Re: [PATCH] [PATCH v2] sh_eth: call of_mdiobus_register() to register phys

[Ben Dooks <ben.dooks@codethink.co.uk>]

On 17/02/14 21:09, Sergei Shtylyov wrote:
> Hello.
>
> On 02/17/2014 07:57 PM, Ben Dooks wrote:
>
>> If the sh_eth device is registered using OF, then the driver
>> should call of_mdiobus_register() to register any PHYs connected
>> to the system.
>
>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>
>> --
>> v2:
>>     - allocate mdio->irq array at init time
>
>     Did this also fix something?
>
>>     - set devdata after succesful mdio registration
>> ---
>>   drivers/net/ethernet/renesas/sh_eth.c | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/renesas/sh_eth.c
>> b/drivers/net/ethernet/renesas/sh_eth.c
>> index 06970ac..1244509 100644
>> --- a/drivers/net/ethernet/renesas/sh_eth.c
>> +++ b/drivers/net/ethernet/renesas/sh_eth.c
>> @@ -40,6 +40,7 @@
>>   #include <linux/if_vlan.h>
>>   #include <linux/clk.h>
>>   #include <linux/sh_eth.h>
>> +#include <linux/of_mdio.h>
>>
>>   #include "sh_eth.h"
>>
>> @@ -2638,6 +2639,18 @@ static int sh_mdio_init(struct net_device
>> *ndev, int id,
>>           goto out_free_bus;
>>       }
>>
>> +    if (ndev->dev.parent->of_node) {
>> +        ret = of_mdiobus_register(mdp->mii_bus,
>> +                      ndev->dev.parent->of_node);
>> +        if (ret != 0) {
>
>     Why not just (ret)?
>
>> +            dev_err(&ndev->dev, "of_mdiobus_register() failed\n");
>> +            goto out_free_bus;
>> +        }
>> +
>> +        dev_set_drvdata(&ndev->dev, mdp->mii_bus);
>> +        return 0;
>> +    }
>> +
>>       for (i = 0; i < PHY_MAX_ADDR; i++)
>>           mdp->mii_bus->irq[i] = PHY_POLL;
>>       if (pd->phy_irq > 0)
>
>     With this code I don't have to call irq_of_parse_and_map() myself
> when parsing the device node, it seems. What if you insert your code
> after these initializers? Or could you please base off my patch and
> remove that superfluous irq_of_parse_and_map() call from
> sh_eth_parse_dt()? I'm going to post v4 on netdev RSN.

Aha, I will look into that and adding some LED properties to
the KSZ8041 micrel PHYs.

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius