Re: [PATCH v5 1/6] drivers: phy: add generic PHY framework

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux