Re: [PATCH RFC 3/4] xhci: Tune PHY for the DWC3-Exynos host controller
Hi,
On Tue, Apr 15, 2014 at 06:24:11PM +0530, Vivek Gautam wrote:
> I had seen your patches in the mailing list, but i don't see any
> updated version of these patches.
> Are you planning to work on this above mentioned patch-series any time soon ?
I'm sorry, I forgot this completely. I have not prepared new version
of those patches as the drivers I need them for are not ready yet, but
I guess I can also use this case as justification for them.
> Or, should i try to find a different approach for handling the phy
> from the host controller (child of DWC3 in this case, which has the
> phys).
Well, there is now an issue with the lookup method I'm suggesting in
this case. Since the device ID is now generated automatically for
xhci-hcd in dwc3, we don't know the xhci-hcd device name before
platform_device_add(), and that is too late. I don't see why the
device could not be named when platform_device_alloc() is called, so I
think I'll suggest something like the attached patch to fix this
issue.
In any case, I'll send an updated version of the phy patches soon.
Thanks,
--
heikki
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index e714709..13f8edb 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -169,11 +169,47 @@ struct platform_object {
*/
void platform_device_put(struct platform_device *pdev)
{
- if (pdev)
- put_device(&pdev->dev);
+ if (!pdev)
+ return;
+
+ if (pdev->id_auto) {
+ ida_simple_remove(&platform_devid_ida, pdev->id);
+ pdev->id = PLATFORM_DEVID_AUTO;
+ }
+
+ put_device(&pdev->dev);
}
EXPORT_SYMBOL_GPL(platform_device_put);
+static int pdev_set_name(struct platform_device *pdev)
+{
+ int ret;
+
+ switch (pdev->id) {
+ default:
+ dev_set_name(&pdev->dev, "%s.%d", pdev->name, pdev->id);
+ break;
+ case PLATFORM_DEVID_NONE:
+ dev_set_name(&pdev->dev, "%s", pdev->name);
+ break;
+ case PLATFORM_DEVID_AUTO:
+ /*
+ * Automatically allocated device ID. We mark it as such so
+ * that we remember it must be freed, and we append a suffix
+ * to avoid namespace collision with explicit IDs.
+ */
+ ret = ida_simple_get(&platform_devid_ida, 0, 0, GFP_KERNEL);
+ if (ret < 0)
+ return ret;
+ pdev->id = ret;
+ pdev->id_auto = true;
+ dev_set_name(&pdev->dev, "%s.%d.auto", pdev->name, pdev->id);
+ break;
+ }
+
+ return 0;
+}
+
static void platform_device_release(struct device *dev)
{
struct platform_object *pa = container_of(dev, struct platform_object,
@@ -206,6 +242,10 @@ struct platform_device *platform_device_alloc(const char *name, int id)
device_initialize(&pa->pdev.dev);
pa->pdev.dev.release = platform_device_release;
arch_setup_pdev_archdata(&pa->pdev);
+ if (pdev_set_name(&pa->pdev)) {
+ kfree(pa);
+ return NULL;
+ }
}
return pa ? &pa->pdev : NULL;
@@ -286,28 +326,6 @@ int platform_device_add(struct platform_device *pdev)
pdev->dev.bus = &platform_bus_type;
- switch (pdev->id) {
- default:
- dev_set_name(&pdev->dev, "%s.%d", pdev->name, pdev->id);
- break;
- case PLATFORM_DEVID_NONE:
- dev_set_name(&pdev->dev, "%s", pdev->name);
- break;
- case PLATFORM_DEVID_AUTO:
- /*
- * Automatically allocated device ID. We mark it as such so
- * that we remember it must be freed, and we append a suffix
- * to avoid namespace collision with explicit IDs.
- */
- ret = ida_simple_get(&platform_devid_ida, 0, 0, GFP_KERNEL);
- if (ret < 0)
- goto err_out;
- pdev->id = ret;
- pdev->id_auto = true;
- dev_set_name(&pdev->dev, "%s.%d.auto", pdev->name, pdev->id);
- break;
- }
-
for (i = 0; i < pdev->num_resources; i++) {
struct resource *p, *r = &pdev->resource[i];
@@ -350,7 +368,6 @@ int platform_device_add(struct platform_device *pdev)
release_resource(r);
}
- err_out:
return ret;
}
EXPORT_SYMBOL_GPL(platform_device_add);
@@ -370,11 +387,6 @@ void platform_device_del(struct platform_device *pdev)
if (pdev) {
device_del(&pdev->dev);
- if (pdev->id_auto) {
- ida_simple_remove(&platform_devid_ida, pdev->id);
- pdev->id = PLATFORM_DEVID_AUTO;
- }
-
for (i = 0; i < pdev->num_resources; i++) {
struct resource *r = &pdev->resource[i];
unsigned long type = resource_type(r);
@@ -392,8 +404,15 @@ EXPORT_SYMBOL_GPL(platform_device_del);
*/
int platform_device_register(struct platform_device *pdev)
{
+ int ret;
+
device_initialize(&pdev->dev);
arch_setup_pdev_archdata(pdev);
+
+ ret = pdev_set_name(pdev);
+ if (ret)
+ return ret;
+
return platform_device_add(pdev);
}
EXPORT_SYMBOL_GPL(platform_device_register);
Re: [PATCH 4/5] arm: omap3: twl: use the new lookup method with usb phy
On Wed, Jan 15, 2014 at 07:41:55PM +0530, Kishon Vijay Abraham I wrote: > The point I'm trying to make is that we won't 'always' know the device names > in > advance. In which cases do we not know the device names, and please note, cases where we would need to use the lookup? The normal cases we would use it are.. 1) A driver creating a child device, like dwc3 when it creates the xhci. There the parent just delivers the phys it has to the child, so both the device name and the phys are known. 2) Platform code. Hopefully we can get rid of the platform code one day, but in any case, when we use it, we always know at least how many users a phy has, and if there is only a singe user, we can rely con_id to do the matching. Though I still don't really see much risk in using the controller device name also there. The lookup table should in any case be made in the place where the phy devices are created, so the phy name is definitely always known. 3) Phys part of something like mfd. This is in practice the same as the platform code case. For example, with twl we always know there is only one user for the phy, which is musb. So we can just use musb's con_id if it's to scary to use "musb-hdrc.0". In any other case, you get the phy from DT, and there is no need to use the lookup when linking the phys and the users. > Btw how are you planning to differentiate between multiple controllers of the > same type with con_id? You don't rely on the con_id alone in those cases. You then use the device name. The con_id is just one parameter you can use to do the matching. > Maybe I'm missing the actual intention of this patch. Do you face problem if > the PHY's have to know about it's consumers in ACPI? > >> I would rather leave the way it is modelled now. > > > > Do you mean, leave it in the platform code? Why? We would reduce the > > platform code by moving it to the mfd driver. Or did I misunderstood > > something? > > In this case, the lookup table will be used only for non-dt boot so don't see > much use in moving to mfd driver. > I meant if the new method is not solving any problem then I would prefer the > way it is modelled now where the PHY's know about it's consumers. OK, so that's what you meant. Things like that init_data promotes use of platform data in the drivers, something that we really should get rid of. Is that not a problem to you? The phy drivers simply should not need to care about the consumers. They should not need to care about anything DT, ACPI, platform specific if it's possible, and here there is nothing preventing it. Let me clarify.. The plan is that ultimately the drivers (meaning all the drivers, not just phy) don't need to care about things like DT properties, ACPI properties or anything DT, ACPI, platform specific. We will have generic driver properties and capabilities and the upper layers will take care of delivering the correct values from the source at hand. The drivers can be made truly platform agnostic. That is the goal, and we should make everything new by keeping that in mind. Many existing frameworks are being converted to that direction, like the gpio framework and DMA Engine API. The way you force the phy drivers the deliver the consumers is taking a step backwards. > Btw where does device gets created in ACPI boot? drivers/acpi/acpi_platform.c Thanks, -- heikki -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] arm: omap3: twl: use the new lookup method with usb phy
On Tue, Jan 07, 2014 at 06:31:52PM +0530, Kishon Vijay Abraham I wrote: > > In any case, having two device names to deal with does not add any > > more risk. These associations should always be made in the place where > > the phy device is created so you will always know it's device name. > > huh.. we should also know the 'controller' device name while defining these > associations and in some cases the controller device and phy device are > created > in entirely different places. If you don't want to use the controller device name to do the matching, we can use the con_id string as well. I believe the lookup method I use in this set just needs a small change. Is that OK with you? > > Normally the platform code creates these associations in the same > > place it creates the platform devices, so you definitely know what the > > device names will be. > > > > In this case it's actually created in drivers/mfd/twl-core.c, so I do > > need to update this and move the lookup table there. We can still > > deliver the user name as platform data, though I believe it's always > > "musb". Maybe we could actually skip that and just hard code the name? > > I would rather leave the way it is modelled now. Do you mean, leave it in the platform code? Why? We would reduce the platform code by moving it to the mfd driver. Or did I misunderstood something? Hope I'm not forgetting something we have talked before my vacation. Thanks, -- heikki -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] phy: add support for indexed lookup
Hi Kishon, And happy new year.. On Tue, Jan 07, 2014 at 07:10:36PM +0530, Kishon Vijay Abraham I wrote: > >>> /** > >>> - * phy_get() - lookup and obtain a reference to a phy. > >>> + * phy_get_index() - obtain a phy based on index > >> > >> NAK. It still takes a 'char' argument and the name is misleading. > >> Btw are you replacing phy_get() or adding a new API in addition to > >> phy_get()? > > > > Additional API. The phy_get() would in practice act as a wrapper after > > In this patch it looks like you've replaced phy_get(). > > this. It could actually be just a #define macro in the include file. > > The function naming I just copied straight from gpiolib.c. I did not > > have the imagination for anything fancier. > > > > I would like to be able to use some function like phy_get_index() and > > be able to deliver it both the name and the index. With DT you guys > > will always be able to use the name (and the string will always > > supersede the index if we do it like this), but with ACPI, and possibly > > the platform lookup tables, the index can be used... > > I think in that case, we should drop the 'string' from phy_get_index since we > have the other API to handle that? I don't know about ACPI, but is it not > possible to use strings with ACPI? No unfortunately. We just have what the ACPI tables provide. The PHYs would be "child" device entries under the controller and we can only get handle to them based on the index. I think I'll skip this patch from this set. Let's wait until we have an actual ACPI DSDT describing some PHYs. Thanks, -- heikki -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] arm: omap3: twl: use the new lookup method with usb phy
Hi,
On Mon, Dec 16, 2013 at 04:34:35PM +0530, Kishon Vijay Abraham I wrote:
> On Monday 09 December 2013 08:38 PM, Heikki Krogerus wrote:
> > Provide a complete association for the phy and it's user
> > (musb) with the new phy_lookup_table.
> >
> > Signed-off-by: Heikki Krogerus
> > ---
> > arch/arm/mach-omap2/twl-common.c | 15 ++-
> > 1 file changed, 6 insertions(+), 9 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/twl-common.c
> > b/arch/arm/mach-omap2/twl-common.c
> > index b0d54da..972430b 100644
> > --- a/arch/arm/mach-omap2/twl-common.c
> > +++ b/arch/arm/mach-omap2/twl-common.c
> > @@ -91,18 +91,13 @@ void __init omap_pmic_late_init(void)
> > }
> >
> > #if defined(CONFIG_ARCH_OMAP3)
> > -struct phy_consumer consumers[] = {
> > - PHY_CONSUMER("musb-hdrc.0", "usb"),
> > -};
> > -
> > -struct phy_init_data init_data = {
> > - .consumers = consumers,
> > - .num_consumers = ARRAY_SIZE(consumers),
> > +static struct phy_lookup_table twl4030_usb_lookup = {
> > + .dev_id = "musb-hdrc.0",
> > + .table = PHY_LOOKUP("phy-twl4030_usb.0", NULL),
> > };
>
> had a similar approach during the initial version of phy framework [1] (see
> phy_bind) but changed it later [2] . Here you should know the device names of
> two devices and even if one of them started using DEVID_AUTO, it'll start
> breaking. Such an approach needs to reviewed carefully.
If somebody creates a regression by changing something like the
platform device id without checking all the users, he deserves to get
into big trouble. I don't see why an individual framework should
provide protection against such cases.
In any case, having two device names to deal with does not add any
more risk. These associations should always be made in the place where
the phy device is created so you will always know it's device name.
Normally the platform code creates these associations in the same
place it creates the platform devices, so you definitely know what the
device names will be.
In this case it's actually created in drivers/mfd/twl-core.c, so I do
need to update this and move the lookup table there. We can still
deliver the user name as platform data, though I believe it's always
"musb". Maybe we could actually skip that and just hard code the name?
The name of the phy we will of course know as the platform device is
created there and then, so the two device names don't create any more
risk even in this case.
Thanks,
--
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] phy: add support for indexed lookup
Hi Kishon,
On Mon, Dec 16, 2013 at 04:32:58PM +0530, Kishon Vijay Abraham I wrote:
> On Monday 09 December 2013 08:38 PM, Heikki Krogerus wrote:
> > Removes the need for the consumer drivers requesting the
> > phys to provide name for the phy. This should ease the use
> > of the framework considerable when using only one phy, which
> > is usually the case when except with USB, but it can also
> > be useful with multiple phys.
>
> If index has to be used with multiple PHYs, the controller should be aware of
> the order in which it is populated in dt data. That's not good.
The Idea is not to replace the name based lookup. Just to provide
possibility for index based lookup.
With ACPI, if we get the device entries for PHYs, the order will be
fixed and we will not have any other reference to the phys. In case
of USB, the first one should always be USB2 PHY and the second the
USB3 PHY.
> > This will also reduce noise from the framework when there is
> > no phy by changing warnings to debug messages.
> >
> > Signed-off-by: Heikki Krogerus
> > ---
> > drivers/phy/phy-core.c | 106
> > ++--
> > include/linux/phy/phy.h | 14 +++
> > 2 files changed, 89 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> > index 1102aef..99dc046 100644
> > --- a/drivers/phy/phy-core.c
> > +++ b/drivers/phy/phy-core.c
> > @@ -53,7 +53,8 @@ static int devm_phy_match(struct device *dev, void *res,
> > void *match_data)
> > return res == match_data;
> > }
> >
> > -static struct phy *phy_lookup(struct device *device, const char *con_id)
> > +static struct phy *phy_lookup(struct device *device, const char *con_id,
> > + unsigned int idx)
> > {
> > unsigned int count;
> > struct phy *phy;
> > @@ -67,6 +68,10 @@ static struct phy *phy_lookup(struct device *device,
> > const char *con_id)
> > count = phy->init_data->num_consumers;
> > consumers = phy->init_data->consumers;
> > while (count--) {
> > + /* index must always match exactly */
> > + if (!con_id)
> > + if (idx != count)
> > + continue;
> > if (!strcmp(consumers->dev_name, dev_name(device)) &&
> > !strcmp(consumers->port, con_id)) {
> > class_dev_iter_exit(&iter);
> > @@ -242,7 +247,8 @@ EXPORT_SYMBOL_GPL(phy_power_off);
> > /**
> > * of_phy_get() - lookup and obtain a reference to a phy by phandle
> > * @dev: device that requests this phy
> > - * @index: the index of the phy
> > + * @con_id: name of the phy from device's point of view
> > + * @idx: the index of the phy if name is not used
> > *
> > * Returns the phy associated with the given phandle value,
> > * after getting a refcount to it or -ENODEV if there is no such phy or
> > @@ -250,12 +256,20 @@ EXPORT_SYMBOL_GPL(phy_power_off);
> > * not yet loaded. This function uses of_xlate call back function provided
> > * while registering the phy_provider to find the phy instance.
> > */
> > -static struct phy *of_phy_get(struct device *dev, int index)
> > +static struct phy *of_phy_get(struct device *dev, const char *con_id,
> > + unsigned int idx)
> > {
> > int ret;
> > struct phy_provider *phy_provider;
> > struct phy *phy = NULL;
> > struct of_phandle_args args;
> > + int index;
> > +
> > + if (!con_id)
> > + index = idx;
> > + else
> > + index = of_property_match_string(dev->of_node, "phy-names",
> > +con_id);
> >
> > ret = of_parse_phandle_with_args(dev->of_node, "phys", "#phy-cells",
> > index, &args);
> > @@ -348,38 +362,36 @@ struct phy *of_phy_simple_xlate(struct device *dev,
> > struct of_phandle_args
> > EXPORT_SYMBOL_GPL(of_phy_simple_xlate);
> >
> > /**
> > - * phy_get() - lookup and obtain a reference to a phy.
> > + * phy_get_index() - obtain a phy based on index
>
> NAK. It still takes a 'char' argument and the name is misleading.
> Btw are you replacing phy_get() or adding a new API in addition to phy_get()?
Additional API. The phy_get() would in practice act as a wrapper after
this. It could actually be just a #de
Re: [PATCH RFC 1/4] phy: Add provision for tuning phy.
Hi again, On Wed, Dec 11, 2013 at 02:02:43PM +0530, Vivek Gautam wrote: > On Wed, Dec 11, 2013 at 1:39 PM, Heikki Krogerus > wrote: > > On Wed, Dec 11, 2013 at 12:08:04PM +0530, Vivek Gautam wrote: > >> On Tue, Dec 10, 2013 at 7:31 PM, Heikki Krogerus > >> > I think "setup" instead of "tune" is much more clear and reusable. > >> > >> I think "setup" will look more like first time setting up the phy, > >> which is rather served by "init" callback. > >> This i thought would serve the purpose of over-riding certain PHY > >> parameters, which would not have been > >> possible at "init" time. > >> Please correct my thinking if i am unable to understand your point here. > > > > OK, sorry I was not clear on this. I'm thinking the same, that this is > > something that is called later, for example when the controller is > > ready. Some ULPI phys need to be initialized, but since the controller > > provides the interface, it's usually not possible during init time. > > This hook could be used in that case as well. > > > > All I'm saying is that "tune" is a poor expression. You will need to > > add a comment explaining what the hook does in any case, so you'll > > have something like "this is something that is called when the > > controller is ready" or something similar. That will make it clear > > what it's meant for. > > Ok, i think i should have kept a comment atleast :-( > I will add proper comments above, and as suggested in the mail by > Kishon, may be name it calibrate ? > What do you think ? Sure, I'm fine with that. Thanks, -- heikki -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC 1/4] phy: Add provision for tuning phy.
Hi, On Wed, Dec 11, 2013 at 12:08:04PM +0530, Vivek Gautam wrote: > On Tue, Dec 10, 2013 at 7:31 PM, Heikki Krogerus > > I think "setup" instead of "tune" is much more clear and reusable. > > I think "setup" will look more like first time setting up the phy, > which is rather served by "init" callback. > This i thought would serve the purpose of over-riding certain PHY > parameters, which would not have been > possible at "init" time. > Please correct my thinking if i am unable to understand your point here. OK, sorry I was not clear on this. I'm thinking the same, that this is something that is called later, for example when the controller is ready. Some ULPI phys need to be initialized, but since the controller provides the interface, it's usually not possible during init time. This hook could be used in that case as well. All I'm saying is that "tune" is a poor expression. You will need to add a comment explaining what the hook does in any case, so you'll have something like "this is something that is called when the controller is ready" or something similar. That will make it clear what it's meant for. Thanks, -- heikki -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC 1/4] phy: Add provision for tuning phy.
Hi,
On Tue, Dec 10, 2013 at 04:25:23PM +0530, Vivek Gautam wrote:
> Some PHY controllers may need to tune PHY post-initialization,
> so that the PHY consumers can call phy-tuning at appropriate
> point of time.
>
> Signed-off-by: vivek Gautam
> ---
> drivers/phy/phy-core.c | 20
> include/linux/phy/phy.h |7 +++
> 2 files changed, 27 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index 03cf8fb..68dbb90 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -239,6 +239,26 @@ out:
> }
> EXPORT_SYMBOL_GPL(phy_power_off);
>
> +int phy_tune(struct phy *phy)
> +{
> + int ret = -ENOTSUPP;
> +
> + mutex_lock(&phy->mutex);
> + if (phy->ops->tune) {
> + ret = phy->ops->tune(phy);
> + if (ret < 0) {
> + dev_err(&phy->dev, "phy tuning failed --> %d\n", ret);
> + goto out;
> + }
> + }
> +
> +out:
> + mutex_unlock(&phy->mutex);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(phy_tune);
I think "setup" instead of "tune" is much more clear and reusable.
Thanks,
--
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC 3/4] xhci: Tune PHY for the DWC3-Exynos host controller
Hi,
On Tue, Dec 10, 2013 at 04:25:25PM +0530, Vivek Gautam wrote:
> @@ -170,6 +189,15 @@ static int xhci_plat_probe(struct platform_device *pdev)
> }
>
> /*
> + * The parent of the xhci-plat device may pass in a PHY via
> + * platform data. If it exists, store it in our struct usb_hcd
> + * so that we can use it later.
> + */
> + phy_generic = dev_get_platdata(&pdev->dev);
> + if (phy_generic)
> + xhci->shared_hcd->phy_generic = *phy_generic;
Getting the handle to the phy from platform data like this is not
going to work for long. It should be possible to get it normally with
phy_get(). It's not going to be possible to get the handle from the
platform data like this if the xhci-hcd platform device is created
from ACPI or DT. You are also not considering case where you have two
phys.
Vivek, I have made a patch set for the phy framework allowing
associations between the phys and their users to be made in same way
gpios and clk make them. With those you should be able to create a
lookup entry to the phy framework in drivers/usb/dwc3/host.c. Then we
could use phy_get() here already. Please check them. Subject of the
thread:
"phy: remove the need for the phys to know about their users"
The lookup table can then be added in drivers/usb/dwc3/host.c with
something like this:
int dwc3_host_init(struct dwc3 *dwc)
{
struct platform_device *xhci;
struct phy_lookup_table *table;
...
table->dev_id = dev_name(&xhci->dev);
if (dwc->usb2_generic_phy)
table->table[0].phy_name =
dev_name(&dwc->usb2_generic_phy->dev);
if (dwc->usb3_generic_phy)
table->table[1].phy_name =
dev_name(&dwc->usb3_generic_phy->dev);
phy_add_lookup_table(table);
...
Br,
--
heikki
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] phy: add support for indexed lookup
Removes the need for the consumer drivers requesting the
phys to provide name for the phy. This should ease the use
of the framework considerable when using only one phy, which
is usually the case when except with USB, but it can also
be useful with multiple phys.
This will also reduce noise from the framework when there is
no phy by changing warnings to debug messages.
Signed-off-by: Heikki Krogerus
---
drivers/phy/phy-core.c | 106 ++--
include/linux/phy/phy.h | 14 +++
2 files changed, 89 insertions(+), 31 deletions(-)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 1102aef..99dc046 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -53,7 +53,8 @@ static int devm_phy_match(struct device *dev, void *res, void
*match_data)
return res == match_data;
}
-static struct phy *phy_lookup(struct device *device, const char *con_id)
+static struct phy *phy_lookup(struct device *device, const char *con_id,
+ unsigned int idx)
{
unsigned int count;
struct phy *phy;
@@ -67,6 +68,10 @@ static struct phy *phy_lookup(struct device *device, const
char *con_id)
count = phy->init_data->num_consumers;
consumers = phy->init_data->consumers;
while (count--) {
+ /* index must always match exactly */
+ if (!con_id)
+ if (idx != count)
+ continue;
if (!strcmp(consumers->dev_name, dev_name(device)) &&
!strcmp(consumers->port, con_id)) {
class_dev_iter_exit(&iter);
@@ -242,7 +247,8 @@ EXPORT_SYMBOL_GPL(phy_power_off);
/**
* of_phy_get() - lookup and obtain a reference to a phy by phandle
* @dev: device that requests this phy
- * @index: the index of the phy
+ * @con_id: name of the phy from device's point of view
+ * @idx: the index of the phy if name is not used
*
* Returns the phy associated with the given phandle value,
* after getting a refcount to it or -ENODEV if there is no such phy or
@@ -250,12 +256,20 @@ EXPORT_SYMBOL_GPL(phy_power_off);
* not yet loaded. This function uses of_xlate call back function provided
* while registering the phy_provider to find the phy instance.
*/
-static struct phy *of_phy_get(struct device *dev, int index)
+static struct phy *of_phy_get(struct device *dev, const char *con_id,
+ unsigned int idx)
{
int ret;
struct phy_provider *phy_provider;
struct phy *phy = NULL;
struct of_phandle_args args;
+ int index;
+
+ if (!con_id)
+ index = idx;
+ else
+ index = of_property_match_string(dev->of_node, "phy-names",
+con_id);
ret = of_parse_phandle_with_args(dev->of_node, "phys", "#phy-cells",
index, &args);
@@ -348,38 +362,36 @@ struct phy *of_phy_simple_xlate(struct device *dev,
struct of_phandle_args
EXPORT_SYMBOL_GPL(of_phy_simple_xlate);
/**
- * phy_get() - lookup and obtain a reference to a phy.
+ * phy_get_index() - obtain a phy based on index
* @dev: device that requests this phy
* @con_id: name of the phy from device's point of view
+ * @idx: index of the phy to obtain in the consumer
*
- * Returns the phy driver, after getting a refcount to it; or
- * -ENODEV if there is no such phy. The caller is responsible for
- * calling phy_put() to release that count.
+ * This variant of phy_get() allows to access PHYs other than the first
+ * defined one for functions that define several PHYs.
*/
-struct phy *phy_get(struct device *dev, const char *con_id)
+struct phy *phy_get_index(struct device *dev, const char *con_id,
+ unsigned int idx)
{
- int index = 0;
struct phy *phy = NULL;
- if (con_id == NULL) {
- dev_WARN(dev, "missing string\n");
- return ERR_PTR(-EINVAL);
+ if (dev->of_node) {
+ dev_dbg(dev, "using device tree for PHY lookup\n");
+ phy = of_phy_get(dev, con_id, idx);
}
- if (dev->of_node) {
- index = of_property_match_string(dev->of_node, "phy-names",
-con_id);
- phy = of_phy_get(dev, index);
- if (IS_ERR(phy)) {
- dev_err(dev, "unable to find phy\n");
- return phy;
- }
- } else {
- phy = phy_lookup(dev, con_id);
- if (IS_ERR(phy)) {
- dev_err(dev, "unable to find phy\n");
- return phy;
-
[PATCH 1/5] phy: unify the phy name parameters
Replace "string" and "port" that are used as phy name
parameter for various functions with "con_id" which is
commonly used in other frameworks.
Signed-off-by: Heikki Krogerus
---
drivers/phy/phy-core.c | 22 ++
include/linux/phy/phy.h | 8
2 files changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 03cf8fb..1102aef 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -53,7 +53,7 @@ static int devm_phy_match(struct device *dev, void *res, void
*match_data)
return res == match_data;
}
-static struct phy *phy_lookup(struct device *device, const char *port)
+static struct phy *phy_lookup(struct device *device, const char *con_id)
{
unsigned int count;
struct phy *phy;
@@ -68,7 +68,7 @@ static struct phy *phy_lookup(struct device *device, const
char *port)
consumers = phy->init_data->consumers;
while (count--) {
if (!strcmp(consumers->dev_name, dev_name(device)) &&
- !strcmp(consumers->port, port)) {
+ !strcmp(consumers->port, con_id)) {
class_dev_iter_exit(&iter);
return phy;
}
@@ -350,33 +350,32 @@ EXPORT_SYMBOL_GPL(of_phy_simple_xlate);
/**
* phy_get() - lookup and obtain a reference to a phy.
* @dev: device that requests this phy
- * @string: the phy name as given in the dt data or the name of the controller
- * port for non-dt case
+ * @con_id: name of the phy from device's point of view
*
* Returns the phy driver, after getting a refcount to it; or
* -ENODEV if there is no such phy. The caller is responsible for
* calling phy_put() to release that count.
*/
-struct phy *phy_get(struct device *dev, const char *string)
+struct phy *phy_get(struct device *dev, const char *con_id)
{
int index = 0;
struct phy *phy = NULL;
- if (string == NULL) {
+ if (con_id == NULL) {
dev_WARN(dev, "missing string\n");
return ERR_PTR(-EINVAL);
}
if (dev->of_node) {
index = of_property_match_string(dev->of_node, "phy-names",
- string);
+con_id);
phy = of_phy_get(dev, index);
if (IS_ERR(phy)) {
dev_err(dev, "unable to find phy\n");
return phy;
}
} else {
- phy = phy_lookup(dev, string);
+ phy = phy_lookup(dev, con_id);
if (IS_ERR(phy)) {
dev_err(dev, "unable to find phy\n");
return phy;
@@ -395,14 +394,13 @@ EXPORT_SYMBOL_GPL(phy_get);
/**
* devm_phy_get() - lookup and obtain a reference to a phy.
* @dev: device that requests this phy
- * @string: the phy name as given in the dt data or phy device name
- * for non-dt case
+ * @con_id: name of the phy from device's point of view
*
* Gets the phy using phy_get(), and associates a device with it using
* devres. On driver detach, release function is invoked on the devres data,
* then, devres data is freed.
*/
-struct phy *devm_phy_get(struct device *dev, const char *string)
+struct phy *devm_phy_get(struct device *dev, const char *con_id)
{
struct phy **ptr, *phy;
@@ -410,7 +408,7 @@ struct phy *devm_phy_get(struct device *dev, const char
*string)
if (!ptr)
return ERR_PTR(-ENOMEM);
- phy = phy_get(dev, string);
+ phy = phy_get(dev, con_id);
if (!IS_ERR(phy)) {
*ptr = phy;
devres_add(dev, ptr);
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 6d72269..d67dcbf 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -127,8 +127,8 @@ int phy_init(struct phy *phy);
int phy_exit(struct phy *phy);
int phy_power_on(struct phy *phy);
int phy_power_off(struct phy *phy);
-struct phy *phy_get(struct device *dev, const char *string);
-struct phy *devm_phy_get(struct device *dev, const char *string);
+struct phy *phy_get(struct device *dev, const char *con_id);
+struct phy *devm_phy_get(struct device *dev, const char *con_id);
void phy_put(struct phy *phy);
void devm_phy_put(struct device *dev, struct phy *phy);
struct phy *of_phy_simple_xlate(struct device *dev,
@@ -199,12 +199,12 @@ static inline int phy_power_off(struct phy *phy)
return -ENOSYS;
}
-static inline struct phy *phy_get(struct device *dev, const char *string)
+static inline struct phy *phy_get(struct device *dev, const char *con_id)
{
return ERR_PTR(-ENOSYS);
}
-static inline struct phy *devm_phy_get(struct device *dev, cons
[PATCH 3/5] phy: improved lookup method
Removes the need for the phys to be aware of their users
even when not using DT. The method is copied from gpiolib.c.
Signed-off-by: Heikki Krogerus
---
drivers/phy/phy-core.c | 91 -
include/linux/phy/phy.h | 48 ++
2 files changed, 138 insertions(+), 1 deletion(-)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 99dc046..05792d0 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -25,6 +25,7 @@
static struct class *phy_class;
static DEFINE_MUTEX(phy_provider_mutex);
static LIST_HEAD(phy_provider_list);
+static LIST_HEAD(phy_lookup_list);
static DEFINE_IDA(phy_ida);
static void devm_phy_release(struct device *dev, void *res)
@@ -85,6 +86,94 @@ static struct phy *phy_lookup(struct device *device, const
char *con_id,
return ERR_PTR(-ENODEV);
}
+/**
+ * phy_add_table() - register PHY/device association to the lookup list
+ * @table: association to register
+ */
+void phy_add_lookup_table(struct phy_lookup_table *table)
+{
+ mutex_lock(&phy_provider_mutex);
+ list_add_tail(&table->list, &phy_lookup_list);
+ mutex_unlock(&phy_provider_mutex);
+}
+
+/**
+ * phy_add_table() - remove PHY/device association from the lookup list
+ * @table: association to be removed
+ */
+void phy_del_lookup_table(struct phy_lookup_table *table)
+{
+ mutex_lock(&phy_provider_mutex);
+ list_del(&table->list);
+ mutex_unlock(&phy_provider_mutex);
+}
+
+static struct phy *find_phy_by_name(const char *name)
+{
+ struct class_dev_iter iter;
+ struct phy *phy = NULL;
+ struct device *dev;
+
+ class_dev_iter_init(&iter, phy_class, NULL, NULL);
+ while ((dev = class_dev_iter_next(&iter))) {
+ if (!strcmp(dev_name(dev), name)) {
+ phy = to_phy(dev);
+ break;
+ }
+ }
+ class_dev_iter_exit(&iter);
+
+ return phy;
+}
+
+static struct phy_lookup_table *phy_get_lookup_table(struct device *dev)
+{
+ const char *dev_id = dev ? dev_name(dev) : NULL;
+ struct phy_lookup_table *table;
+
+ mutex_lock(&phy_provider_mutex);
+ list_for_each_entry(table, &phy_lookup_list, list)
+ if (!strcmp(table->dev_id, dev_id))
+ goto out;
+ table = NULL;
+out:
+ mutex_unlock(&phy_provider_mutex);
+ return table;
+}
+
+static struct phy *phy_find(struct device *dev, const char *con_id,
+ unsigned int idx)
+{
+ struct phy_lookup_table *table;
+ struct phy_lookup *p;
+ struct phy *phy;
+
+ table = phy_get_lookup_table(dev);
+ if (!table)
+ /* fall-back to the old lookup method for now */
+ return phy_lookup(dev, con_id, idx);
+
+ for (p = &table->table[0]; p->phy_name; p++) {
+ /* index must always match exactly */
+ if (idx != p->idx)
+ continue;
+
+ /* If the lookup entry has a con_id, require exact match */
+ if (p->con_id && (!con_id || strcmp(p->con_id, con_id)))
+ continue;
+
+ phy = find_phy_by_name(p->phy_name);
+ if (!phy) {
+ dev_warn(dev, "no PHY by the name %s\n", p->phy_name);
+ return ERR_PTR(-ENODEV);
+ }
+
+ return phy;
+ }
+
+ return ERR_PTR(-ENODEV);
+}
+
static struct phy_provider *of_phy_provider_lookup(struct device_node *node)
{
struct phy_provider *phy_provider;
@@ -386,7 +475,7 @@ struct phy *phy_get_index(struct device *dev, const char
*con_id,
*/
if (!phy || IS_ERR(phy)) {
dev_dbg(dev, "using lookup tables for PHY lookup");
- phy = phy_lookup(dev, con_id, idx);
+ phy = phy_find(dev, con_id, idx);
}
if (IS_ERR(phy)) {
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 43d1a23..cca363a 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -98,6 +98,54 @@ struct phy_init_data {
.port = _port,\
}
+/**
+ * struct phy_lookup - Lookup entry for associating PHYs
+ * @phy_name: device name of the PHY
+ * @con_id: name of the PHY from device's point of view
+ * @idx: index of the PHY when name is not used
+ */
+struct phy_lookup {
+ const char *phy_name;
+ const char *con_id;
+ unsigned int idx;
+};
+
+/**
+ * struct phy_lookup_table - association of PHYs to specific device
+ * @list: entry in the lookup list
+ * @dev_id: the name of the device
+ * @table: table of PHYs attached to this device
+ */
+struct phy_lookup_table {
+ struct list_head list;
+ const char *dev_id;
+
[PATCH 4/5] arm: omap3: twl: use the new lookup method with usb phy
Provide a complete association for the phy and it's user
(musb) with the new phy_lookup_table.
Signed-off-by: Heikki Krogerus
---
arch/arm/mach-omap2/twl-common.c | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/arch/arm/mach-omap2/twl-common.c b/arch/arm/mach-omap2/twl-common.c
index b0d54da..972430b 100644
--- a/arch/arm/mach-omap2/twl-common.c
+++ b/arch/arm/mach-omap2/twl-common.c
@@ -91,18 +91,13 @@ void __init omap_pmic_late_init(void)
}
#if defined(CONFIG_ARCH_OMAP3)
-struct phy_consumer consumers[] = {
- PHY_CONSUMER("musb-hdrc.0", "usb"),
-};
-
-struct phy_init_data init_data = {
- .consumers = consumers,
- .num_consumers = ARRAY_SIZE(consumers),
+static struct phy_lookup_table twl4030_usb_lookup = {
+ .dev_id = "musb-hdrc.0",
+ .table = PHY_LOOKUP("phy-twl4030_usb.0", NULL),
};
static struct twl4030_usb_data omap3_usb_pdata = {
.usb_mode = T2_USB_MODE_ULPI,
- .init_data = &init_data,
};
static int omap3_batt_table[] = {
@@ -225,8 +220,10 @@ void __init omap3_pmic_get_config(struct
twl4030_platform_data *pmic_data,
}
/* Common platform data configurations */
- if (pdata_flags & TWL_COMMON_PDATA_USB && !pmic_data->usb)
+ if (pdata_flags & TWL_COMMON_PDATA_USB && !pmic_data->usb) {
+ phy_add_lookup_table(&twl4030_usb_lookup);
pmic_data->usb = &omap3_usb_pdata;
+ }
if (pdata_flags & TWL_COMMON_PDATA_BCI && !pmic_data->bci)
pmic_data->bci = &omap3_bci_pdata;
--
1.8.5.1
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] phy: remove the old lookup method
The users of the old method are now converted to the new one.
Signed-off-by: Heikki Krogerus
---
Documentation/phy.txt | 100 ++--
drivers/phy/phy-core.c | 46 ++---
drivers/phy/phy-exynos-dp-video.c | 2 +-
drivers/phy/phy-exynos-mipi-video.c | 2 +-
drivers/phy/phy-omap-usb2.c | 2 +-
drivers/phy/phy-twl4030-usb.c | 4 +-
include/linux/phy/phy.h | 36 ++---
7 files changed, 73 insertions(+), 119 deletions(-)
diff --git a/Documentation/phy.txt b/Documentation/phy.txt
index 0103e4b..cfff1a2 100644
--- a/Documentation/phy.txt
+++ b/Documentation/phy.txt
@@ -53,17 +53,13 @@ unregister the PHY.
The PHY driver should create the PHY in order for other peripheral controllers
to make use of it. The PHY framework provides 2 APIs to create the PHY.
-struct phy *phy_create(struct device *dev, const struct phy_ops *ops,
-struct phy_init_data *init_data);
-struct phy *devm_phy_create(struct device *dev, const struct phy_ops *ops,
- struct phy_init_data *init_data);
+struct phy *phy_create(struct device *dev, const struct phy_ops *ops);
+struct phy *devm_phy_create(struct device *dev, const struct phy_ops *ops);
The PHY drivers can use one of the above 2 APIs to create the PHY by passing
-the device pointer, phy ops and init_data.
+the device pointer, phy ops.
phy_ops is a set of function pointers for performing PHY operations such as
-init, exit, power_on and power_off. *init_data* is mandatory to get a reference
-to the PHY in the case of non-dt boot. See section *Board File Initialization*
-on how init_data should be used.
+init, exit, power_on and power_off.
Inorder to dereference the private data (in phy_ops), the phy provider driver
can use phy_set_drvdata() after creating the PHY and use phy_get_drvdata() in
@@ -74,16 +70,21 @@ phy_ops to get back the private data.
Before the controller can make use of the PHY, it has to get a reference to
it. This framework provides the following APIs to get a reference to the PHY.
-struct phy *phy_get(struct device *dev, const char *string);
-struct phy *devm_phy_get(struct device *dev, const char *string);
+struct phy *phy_get(struct device *dev, const char *con_id);
+struct phy *devm_phy_get(struct device *dev, const char *con_id);
phy_get and devm_phy_get can be used to get the PHY. In the case of dt boot,
-the string arguments should contain the phy name as given in the dt data and
+the con_id arguments should contain the phy name as given in the dt data and
in the case of non-dt boot, it should contain the label of the PHY.
The only difference between the two APIs is that devm_phy_get associates the
device with the PHY using devres on successful PHY get. On driver detach,
release function is invoked on the the devres data and devres data is freed.
+Indexed lookup is also possible when device has multiple PHYs attached to it.
+The framework provides variants for phy_get() functions called phy_get_index()
+and devm_phy_get_index(). They take the same parameters as do the normal
+phy_get() plus the index number of the PHY as the last parameter.
+
5. Releasing a reference to the PHY
When the controller no longer needs the PHY, it has to release the reference
@@ -123,42 +124,63 @@ There are exported APIs like phy_pm_runtime_get,
phy_pm_runtime_get_sync,
phy_pm_runtime_put, phy_pm_runtime_put_sync, phy_pm_runtime_allow and
phy_pm_runtime_forbid for performing PM operations.
-8. Board File Initialization
+8. Platform Data binding
-Certain board file initialization is necessary in order to get a reference
-to the PHY in the case of non-dt boot.
-Say we have a single device that implements 3 PHYs that of USB, SATA and PCIe,
-then in the board file the following initialization should be done.
+PHY's are mapped by the means of tables of lookups, containing instances of the
+phy_lookup structure. Two macros are defined to help declaring such mappings:
-struct phy_consumer consumers[] = {
- PHY_CONSUMER("dwc3.0", "usb"),
- PHY_CONSUMER("pcie.0", "pcie"),
- PHY_CONSUMER("sata.0", "sata"),
-};
-PHY_CONSUMER takes 2 parameters, first is the device name of the controller
-(PHY consumer) and second is the port name.
+ PHY_LOOKUP(phy_name, con_id)
+ PHY_LOOKUP_IDX(phy_name, con_id, idx)
+
+where
-struct phy_init_data init_data = {
- .consumers = consumers,
- .num_consumers = ARRAY_SIZE(consumers),
+ - phy_name identifiers of the phy device
+ - con_id is the name of the PHY from the device point of view. It can be
NULL.
+ - idx is the index of the PHY within the function.
+
+A lookup table can then be defined as follows, with an empty entry defining its
+end:
+
+struct phy_lookup_table phys_table = {
+ .dev_id = "usb_controller.0",
+ .table = {
+ PHY_LOOKUP_IDX("phy-u
[PATCH 0/5] phy: remove the need for the phys to know about their users
Hi, This replaces the consumer & init_data structures with a lookup table that contains complete associations with the phys and their users, removing the need for the phy drivers themselves to care about their users even when not using DT. The lookup method is copied from the way the gpio descriptor lookup is now handled in gpiolib.c. Heikki Krogerus (5): phy: unify the phy name parameters phy: add support for indexed lookup phy: improved lookup method arm: omap3: twl: use the new lookup method with usb phy phy: remove the old lookup method arch/arm/mach-omap2/twl-common.c| 15 ++- drivers/phy/phy-core.c | 209 ++-- drivers/phy/phy-exynos-dp-video.c | 2 +- drivers/phy/phy-exynos-mipi-video.c | 2 +- drivers/phy/phy-omap-usb2.c | 2 +- drivers/phy/phy-twl4030-usb.c | 4 +- include/linux/phy/phy.h | 86 ++- 7 files changed, 221 insertions(+), 99 deletions(-) -- 1.8.5.1 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
