netdev - Re: [PATCH] sh_eth: call of_mdiobus

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 18 Feb 2014 21:11:57 +0300
From:	Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
To:	Ben Dooks <ben.dooks@...ethink.co.uk>
CC:	netdev@...r.kernel.org, horms+renesas@...ge.net.au,
	linux-sh@...r.kernel.org, magnus@...nsource.se,
	linux-kernel@...ts.codethink.co.uk
Subject: Re: [PATCH] sh_eth: call of_mdiobus_register() to register phys

Hello.

On 02/18/2014 07:00 PM, Ben Dooks wrote:

>>>>> If the sh_eth device is registered using OF, then the driver

>>>>     Which is not supported yet as my DT patch hasn't been merged.
>>>> This patch seems somewhat premature.

>>> I've got your OF patches in my local tree to test with, this
>>> is what I found during that testing.

>>     The issue is that I didn't post my v3 patch to netdev due to
>> net-next.git repo being closed at this moment and DaveM not wanting to
>> see any patch targeted to it during this time. I've now posted v4 of my
>> Ether DT patch to netdev.

> Ok, I will look for these tomorrow.

    No significant changes there...

>>>>> should call of_mdiobus_register() to register any PHYs connected
>>>>> to the system.

>>>>     That's not necessary (but good to have).

>>> Well, it is necessary if you then want any PHYS bound to
>>> the device to have their OF information to hand,

>>     Ether DT support worked for me without this fragment, at least.

> Yes, it just that the PHY is not being linked to the relevant
> OF node. The PHY gets bound, it will not be able to find the
> DT info passed.

    With no DT support in the PHY driver, I don't see how it matters. Perhaps 
it has to do with your "init-regs" prop patch though...

>>>>> Signed-off-by: Ben Dooks <ben.dooks@...ethink.co.uk>
>>>>> ---
>>>>>   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..165f0c4 100644
>>>>> --- a/drivers/net/ethernet/renesas/sh_eth.c
>>>>> +++ b/drivers/net/ethernet/renesas/sh_eth.c
>>>> [...]
>>>>> @@ -2629,6 +2630,18 @@ static int sh_mdio_init(struct net_device
>>>>> *ndev, int id,
>>>>>       snprintf(mdp->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
>>>>>            mdp->pdev->name, id);
>>>>>
>>>>> +    if (ndev->dev.parent->of_node) {
>>>>> +        dev_set_drvdata(&ndev->dev, mdp->mii_bus);
>>>>> +        ret = of_mdiobus_register(mdp->mii_bus,
>>>>> +                      ndev->dev.parent->of_node);
>>>>> +        if (ret != 0) {
>>>>> +            dev_err(&ndev->dev, "of_mdiobus_register() failed\n");
>>>>> +            goto out_free_bus;
>>>>> +        }

>>> I should probably only set the drvdata if the
>>> of_mdiobus_register() succeeds.

>>     Yes. Probably should use *goto* as well since in that case the
>> success path would be the same as the existing one.

> I will look into that, not the biggest fan of gotos for
> success cases.

    Can also try to use *else* branch to call mdiobus_register()...

>>>>> +        return 0;
>>>>> +    }
>>>>> +
>>>>>       /* PHY IRQ */
>>>>>       mdp->mii_bus->irq = devm_kzalloc(&ndev->dev,
>>>>>                        sizeof(int) * PHY_MAX_ADDR,

[...]

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html