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

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

[Olliver Schinagl]

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,
-- 
2.11.0

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

[Joe Hershberger]

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>

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

[Michal Simek]

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

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

[Joe Hershberger]

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

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

[Michal Simek]

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

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

[Olliver Schinagl]

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

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

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

[Joe Hershberger]

Hi Olliver,

https://patchwork.ozlabs.org/patch/746814/ was applied to u-boot-net.git.

Thanks!
-Joe