[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <515C3D94.1060000@ti.com>
Date: Wed, 3 Apr 2013 20:02:52 +0530
From: Kishon Vijay Abraham I <kishon@...com>
To: <balbi@...com>
CC: <gregkh@...uxfoundation.org>, <arnd@...db.de>,
<akpm@...ux-foundation.org>, <swarren@...dotorg.org>,
<sylvester.nawrocki@...il.com>, <rob@...dley.net>,
<netdev@...r.kernel.org>, <davem@...emloft.net>,
<cesarb@...arb.net>, <linux-usb@...r.kernel.org>,
<linux-omap@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<tony@...mide.com>, <grant.likely@...retlab.ca>,
<rob.herring@...xeda.com>, <b-cousson@...com>,
<linux@....linux.org.uk>, <eballetbo@...il.com>,
<javier@...hile0.org>, <mchehab@...hat.com>,
<santosh.shilimkar@...com>, <broonie@...nsource.wolfsonmicro.com>,
<swarren@...dia.com>, <linux-doc@...r.kernel.org>,
<devicetree-discuss@...ts.ozlabs.org>,
<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v5 1/6] drivers: phy: add generic PHY framework
Hi,
On Wednesday 03 April 2013 07:57 PM, Felipe Balbi wrote:
> hi,
>
> On Wed, Apr 03, 2013 at 07:48:42PM +0530, Kishon Vijay Abraham I wrote:
>>>> +struct phy *of_phy_xlate(struct phy *phy, struct of_phandle_args *args)
>>>> +{
>>>> + return phy;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(of_phy_xlate);
>>>
>>> so you get a PHY and just return it ? What gives ?? (maybe I skipped
>>> some of the discussion...)
>>
>> hmm.. this is for the common case where the PHY provider implements
>> only one PHY. And both phy provider and phy_instance is represented
>> by struct phy *.
>>
>> For the case where PHY provider implements multiple PHYs (here it
>> will have a single dt node), the PHY provider will implement it's own
>> version of of_xlate that takes *of_phandle_args* as argument and
>> finds the appropriate PHY.
>
> got it.
>
>>>> +struct phy *of_phy_get(struct device *dev, int index)
>>>> +{
>>>> + int ret;
>>>> + struct phy *phy = NULL;
>>>> + struct phy_bind *phy_map = NULL;
>>>> + struct of_phandle_args args;
>>>> + struct device_node *node;
>>>> +
>>>> + if (!dev->of_node) {
>>>> + dev_dbg(dev, "device does not have a device node entry\n");
>>>> + return ERR_PTR(-EINVAL);
>>>> + }
>>>> +
>>>> + ret = of_parse_phandle_with_args(dev->of_node, "phys", "#phy-cells",
>>>> + index, &args);
>>>> + if (ret) {
>>>> + dev_dbg(dev, "failed to get phy in %s node\n",
>>>> + dev->of_node->full_name);
>>>> + return ERR_PTR(-ENODEV);
>>>> + }
>>>> +
>>>> + phy = of_phy_lookup(args.np);
>>>> + if (IS_ERR(phy) || !try_module_get(phy->ops->owner)) {
>>>> + phy = ERR_PTR(-EPROBE_DEFER);
>>>> + goto err0;
>>>> + }
>>>> +
>>>> + phy = phy->ops->of_xlate(phy, &args);
>>>
>>> alright, so of_xlate() is optional, am I right ? How about not
>>
>> Not really. of_xlate is mandatory (it's even checked in phy_create).
>> Either the PHY provider can implement it's own version or use the
>> implementation above (by filling the function pointer).
>
> alright.
>
>>> implementing the above and have a check for of_xlate() being a valid
>>> pointer here ?
>>
>> Having the way it is actually mandates the PHY providers to always
>> provide of_xlate which IMO is better since some PHY providers wont
>> accidentally be using the default implementation.
>
> ok cool, thanks for clarifying.
>
>>>> + ret = -EINVAL;
>>>> + goto err0;
>>>> + }
>>>> +
>>>> + if (!phy_class)
>>>> + phy_core_init();
>>>
>>> why don't you setup the class on module_init ? Then this would be a
>>> terrible error condition here :-)
>>
>> This is for the case where the PHY driver gets loaded before the PHY
>> framework. I could have returned EPROBE_DEFER here instead I thought
>> will have it this way.
>
> looks a bit weird IMO. Is it really possible for PHY to load before ?
yeah. it actually happened when I tried with beagle and had all the
modules as built-in. Because twl4030 has subsys_initcall(), it loads
before PHY framework.
Thanks
Kishon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists