Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1390483 > unrolled thread

Re: [PATCH] net: davinci_mdio: Set of_node in the mdio bus

Started byDavid Miller <davem@davemloft.net>
First post2016-04-28 21:50 +0200
Last post2016-04-29 01:20 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH] net: davinci_mdio: Set of_node in the mdio bus David Miller <davem@davemloft.net> - 2016-04-28 21:50 +0200
    Re: [PATCH] net: davinci_mdio: Set of_node in the mdio bus David Miller <davem@davemloft.net> - 2016-04-28 23:50 +0200
    Re: [PATCH] net: davinci_mdio: Set of_node in the mdio bus Andrew Lunn <andrew@lunn.ch> - 2016-04-29 01:20 +0200

#1390483 — Re: [PATCH] net: davinci_mdio: Set of_node in the mdio bus

FromDavid Miller <davem@davemloft.net>
Date2016-04-28 21:50 +0200
SubjectRe: [PATCH] net: davinci_mdio: Set of_node in the mdio bus
Message-ID<rt06L-1ay-15@gated-at.bofh.it>
From: "J.D. Schroeder" <Linux.HWI@garmin.com>
Date: Mon, 25 Apr 2016 09:46:11 -0500

> From: "J.D. Schroeder" <jay.schroeder@garmin.com>
> 
> Assigns the of_node from the platform device to the of_node of the
> mdio bus so that it can be used in the mdio driver to properly match
> a bus in the DT with a phandle in of_mdio_find_bus().
> 
> Signed-off-by: J.D. Schroeder <jay.schroeder@garmin.com>
> Signed-off-by: Ben McCauley <ben.mccauley@garmin.com>
> ---
>  drivers/net/ethernet/ti/davinci_mdio.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
> index 4e7c9b9..b5e5f37 100644
> --- a/drivers/net/ethernet/ti/davinci_mdio.c
> +++ b/drivers/net/ethernet/ti/davinci_mdio.c
> @@ -343,6 +343,7 @@ static int davinci_mdio_probe(struct platform_device *pdev)
>  		if (davinci_mdio_probe_dt(&data->pdata, pdev))
>  			data->pdata = default_pdata;
>  		snprintf(data->bus->id, MII_BUS_ID_SIZE, "%s", pdev->name);
> +		data->bus->dev.of_node = dev->of_node;
>  	} else {
>  		data->pdata = pdata ? (*pdata) : default_pdata;
>  		snprintf(data->bus->id, MII_BUS_ID_SIZE, "%s-%x",

You can't do this.

First of all, of_node objects are reference counted.  So even if this was a
legal thing to do you would have to drop the reference to the existing of_node
pointer and gain a reference to dev->of_node.

But even more importantly, it is the job of the bus driver to set that
bus->dev.of_node correctly, you should never override it in a driver like
this.

I'm not applying this, sorry.

[toc] | [next] | [standalone]


#1390565

FromDavid Miller <davem@davemloft.net>
Date2016-04-28 23:50 +0200
Message-ID<rt1YS-2LF-7@gated-at.bofh.it>
In reply to#1390483
From: "J.D. Schroeder" <Linux.HWI@garmin.com>
Date: Thu, 28 Apr 2016 16:39:36 -0500

> On 04/28/2016 02:44 PM, David Miller wrote:
>>> --- a/drivers/net/ethernet/ti/davinci_mdio.c
>>> +++ b/drivers/net/ethernet/ti/davinci_mdio.c
>>> @@ -343,6 +343,7 @@ static int davinci_mdio_probe(struct platform_device *pdev)
>>>  		if (davinci_mdio_probe_dt(&data->pdata, pdev))
>>>  			data->pdata = default_pdata;
>>>  		snprintf(data->bus->id, MII_BUS_ID_SIZE, "%s", pdev->name);
>>> +		data->bus->dev.of_node = dev->of_node;
>>>  	} else {
>>>  		data->pdata = pdata ? (*pdata) : default_pdata;
>>>  		snprintf(data->bus->id, MII_BUS_ID_SIZE, "%s-%x",
>> 
>> You can't do this.
>> 
>> First of all, of_node objects are reference counted.  So even if this was a
>> legal thing to do you would have to drop the reference to the existing of_node
>> pointer and gain a reference to dev->of_node.
>> 
>> But even more importantly, it is the job of the bus driver to set that
>> bus->dev.of_node correctly, you should never override it in a driver like
>> this.
> 
> David, thanks for your review. I understand your point about the reference count.
> 
> One thing to note is that it is always null for the davinci mdio bus when
> going through this path. I'm not trying to override it. I'm trying to make
> sure it has a way to find the davinci mdio bus. Do you see the problem I'm
> trying to solve?
> 
> Is there another way to be able to make the of_mdio_find_bus() call be able to
> find the davinci mdio bus?

You should reference the device which actually has an OF node attached with it,
rather than pretending that the MDIO bus does.

Don't fake this stuff, reference the proper device to get the OF node.

Thanks.

[toc] | [prev] | [next] | [standalone]


#1390625

FromAndrew Lunn <andrew@lunn.ch>
Date2016-04-29 01:20 +0200
Message-ID<rt3nY-3WT-29@gated-at.bofh.it>
In reply to#1390483
> Is there another way to be able to make the of_mdio_find_bus() call be able to
> find the davinci mdio bus?

I missed the first post, and i cannot find it in the archive. Can you
explain what your problem is please.

So long as you call of_mdiobus_register() passing the correct device
node, it should all work.

      Andrew

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web