[U-Boot] [PATCHv2] net: zynq_gem: Do not return -ENOSYS on success

The .read_rom_hwaddr net_ops hook does not check the return value, which is why it was never caught that we are currently returning 0 if the read_rom_hwaddr function return -ENOSYS and -ENOSYS otherwise.
In this case we can simplify this by just returning the result of the function.
Signed-off-by: Olliver Schinagl oliver@schinagl.nl --- Changes since v1: Check pdata for NULL before dereferencing.
drivers/net/zynq_gem.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index 86dd03feda..52beedfad0 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -589,14 +589,12 @@ __weak int zynq_board_read_rom_ethaddr(unsigned char *ethaddr)
static int zynq_gem_read_rom_mac(struct udevice *dev) { - int retval; struct eth_pdata *pdata = dev_get_platdata(dev);
- retval = zynq_board_read_rom_ethaddr(pdata->enetaddr); - if (retval == -ENOSYS) - retval = 0; + if (!pdata) + return -ENOSYS;
- return retval; + return zynq_board_read_rom_ethaddr(pdata->enetaddr); }
static int zynq_gem_miiphy_read(struct mii_dev *bus, int addr,

On Mon, Apr 3, 2017 at 9:18 AM, Olliver Schinagl oliver@schinagl.nl wrote:
The .read_rom_hwaddr net_ops hook does not check the return value, which is why it was never caught that we are currently returning 0 if the read_rom_hwaddr function return -ENOSYS and -ENOSYS otherwise.
In this case we can simplify this by just returning the result of the function.
Signed-off-by: Olliver Schinagl oliver@schinagl.nl
Acked-by: Joe Hershberger joe.hershberger@ni.com

On 4.4.2017 19:53, Joe Hershberger wrote:
On Mon, Apr 3, 2017 at 9:18 AM, Olliver Schinagl oliver@schinagl.nl wrote:
The .read_rom_hwaddr net_ops hook does not check the return value, which is why it was never caught that we are currently returning 0 if the read_rom_hwaddr function return -ENOSYS and -ENOSYS otherwise.
In this case we can simplify this by just returning the result of the function.
Signed-off-by: Olliver Schinagl oliver@schinagl.nl
Acked-by: Joe Hershberger joe.hershberger@ni.com
Acked-by: Michal Simek michal.simek@xilinx.com
Joe: Can you please take this via your tree?
Thanks, Michal

Hi Michal,
On Wed, Apr 5, 2017 at 4:17 AM, Michal Simek michal.simek@xilinx.com wrote:
On 4.4.2017 19:53, Joe Hershberger wrote:
On Mon, Apr 3, 2017 at 9:18 AM, Olliver Schinagl oliver@schinagl.nl wrote:
The .read_rom_hwaddr net_ops hook does not check the return value, which is why it was never caught that we are currently returning 0 if the read_rom_hwaddr function return -ENOSYS and -ENOSYS otherwise.
In this case we can simplify this by just returning the result of the function.
Signed-off-by: Olliver Schinagl oliver@schinagl.nl
Acked-by: Joe Hershberger joe.hershberger@ni.com
Acked-by: Michal Simek michal.simek@xilinx.com
Joe: Can you please take this via your tree?
Will do.
-Joe

On 3.4.2017 16:18, Olliver Schinagl wrote:
The .read_rom_hwaddr net_ops hook does not check the return value, which is why it was never caught that we are currently returning 0 if the read_rom_hwaddr function return -ENOSYS and -ENOSYS otherwise.
In this case we can simplify this by just returning the result of the function.
Signed-off-by: Olliver Schinagl oliver@schinagl.nl
Changes since v1: Check pdata for NULL before dereferencing.
drivers/net/zynq_gem.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index 86dd03feda..52beedfad0 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -589,14 +589,12 @@ __weak int zynq_board_read_rom_ethaddr(unsigned char *ethaddr)
static int zynq_gem_read_rom_mac(struct udevice *dev) {
int retval; struct eth_pdata *pdata = dev_get_platdata(dev);
retval = zynq_board_read_rom_ethaddr(pdata->enetaddr);
if (retval == -ENOSYS)
retval = 0;
- if (!pdata)
return -ENOSYS;
- return retval;
- return zynq_board_read_rom_ethaddr(pdata->enetaddr);
}
static int zynq_gem_miiphy_read(struct mii_dev *bus, int addr,
I have seen one series about mac address which should just replace the whole this function. Not sure if it was applied or not but worth to check because if this is in tree the whole this function should go away.
M

Not yet, i rebased against u-boot-net/master. As it is it is a bug however so we should just put the fix in for now.
On April 5, 2017 8:29:22 AM CEST, Michal Simek michal.simek@xilinx.com wrote:
On 3.4.2017 16:18, Olliver Schinagl wrote:
The .read_rom_hwaddr net_ops hook does not check the return value,
which
is why it was never caught that we are currently returning 0 if the read_rom_hwaddr function return -ENOSYS and -ENOSYS otherwise.
In this case we can simplify this by just returning the result of the function.
Signed-off-by: Olliver Schinagl oliver@schinagl.nl
Changes since v1: Check pdata for NULL before dereferencing.
drivers/net/zynq_gem.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index 86dd03feda..52beedfad0 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -589,14 +589,12 @@ __weak int zynq_board_read_rom_ethaddr(unsigned
char *ethaddr)
static int zynq_gem_read_rom_mac(struct udevice *dev) {
int retval; struct eth_pdata *pdata = dev_get_platdata(dev);
retval = zynq_board_read_rom_ethaddr(pdata->enetaddr);
if (retval == -ENOSYS)
retval = 0;
- if (!pdata)
return -ENOSYS;
- return retval;
- return zynq_board_read_rom_ethaddr(pdata->enetaddr);
}
static int zynq_gem_miiphy_read(struct mii_dev *bus, int addr,
I have seen one series about mac address which should just replace the whole this function. Not sure if it was applied or not but worth to check because if this is in tree the whole this function should go away.
M

participants (4)
-
Joe Hershberger
-
Joe Hershberger
-
Michal Simek
-
Olliver Schinagl