[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(-)

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

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

Hi Olliver,

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

Thanks!
-Joe