| Message ID | 1426885630-32429-2-git-send-email-arun.ramamurthy@broadcom.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Arun, On Fri, Mar 20, 2015 at 02:07:08PM -0700, Arun Ramamurthy wrote: > Adding devm_of_phy_get_by_index to get phys by supplying an index > and not a phy name when multiple phys are declared > > Reviewed-by: Ray Jui <rjui@broadcom.com> > Reviewed-by: Scott Branden <sbranden@broadcom.com> > Signed-off-by: Arun Ramamurthy <arun.ramamurthy@broadcom.com> > --- > drivers/phy/phy-core.c | 30 ++++++++++++++++++++++++++++++ > include/linux/phy/phy.h | 2 ++ > 2 files changed, 32 insertions(+) > > diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c > index a12d353..0c03876 100644 > --- a/drivers/phy/phy-core.c > +++ b/drivers/phy/phy-core.c > @@ -622,6 +622,36 @@ struct phy *devm_of_phy_get(struct device *dev, struct device_node *np, > EXPORT_SYMBOL_GPL(devm_of_phy_get); > > /** > + * devm_of_phy_get_by_index() - lookup and obtain a reference to a phy by index. > + * @dev: device that requests this phy > + * @np: node containing the phy > + * @index: index of the phy > + * > + * Gets the phy using _of_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_of_phy_get_by_index(struct device *dev, struct device_node *np, > + int index) > +{ > + struct phy **ptr, *phy; > + > + ptr = devres_alloc(devm_phy_release, sizeof(*ptr), GFP_KERNEL); > + if (!ptr) > + return ERR_PTR(-ENOMEM); > + > + phy = _of_phy_get(np, index); > + if (!IS_ERR(phy)) { > + *ptr = phy; > + devres_add(dev, ptr); > + } else { > + devres_free(ptr); > + } > + > + return phy; > +} You want EXPORT_SYMBOL_GPL(devm_of_phy_get_by_index); here. > +/** > * phy_create() - create a new phy > * @dev: device that is creating the new phy > * @node: device node of the phy > diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h > index a0197fa..ae2ffaf 100644 > --- a/include/linux/phy/phy.h > +++ b/include/linux/phy/phy.h > @@ -133,6 +133,8 @@ struct phy *devm_phy_get(struct device *dev, const char *string); > struct phy *devm_phy_optional_get(struct device *dev, const char *string); > struct phy *devm_of_phy_get(struct device *dev, struct device_node *np, > const char *con_id); > +struct phy *devm_of_phy_get_by_index(struct device *dev, struct device_node *np, > + int index); > void phy_put(struct phy *phy); > void devm_phy_put(struct device *dev, struct phy *phy); > struct phy *of_phy_get(struct device_node *np, const char *con_id); > -- > 2.3.2 >
On 15-03-20 02:26 PM, Dmitry Torokhov wrote: > Hi Arun, > > On Fri, Mar 20, 2015 at 02:07:08PM -0700, Arun Ramamurthy wrote: >> Adding devm_of_phy_get_by_index to get phys by supplying an index >> and not a phy name when multiple phys are declared >> >> Reviewed-by: Ray Jui <rjui@broadcom.com> >> Reviewed-by: Scott Branden <sbranden@broadcom.com> >> Signed-off-by: Arun Ramamurthy <arun.ramamurthy@broadcom.com> >> --- >> drivers/phy/phy-core.c | 30 ++++++++++++++++++++++++++++++ >> include/linux/phy/phy.h | 2 ++ >> 2 files changed, 32 insertions(+) >> >> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c >> index a12d353..0c03876 100644 >> --- a/drivers/phy/phy-core.c >> +++ b/drivers/phy/phy-core.c >> @@ -622,6 +622,36 @@ struct phy *devm_of_phy_get(struct device *dev, struct device_node *np, >> EXPORT_SYMBOL_GPL(devm_of_phy_get); >> >> /** >> + * devm_of_phy_get_by_index() - lookup and obtain a reference to a phy by index. >> + * @dev: device that requests this phy >> + * @np: node containing the phy >> + * @index: index of the phy >> + * >> + * Gets the phy using _of_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_of_phy_get_by_index(struct device *dev, struct device_node *np, >> + int index) >> +{ >> + struct phy **ptr, *phy; >> + >> + ptr = devres_alloc(devm_phy_release, sizeof(*ptr), GFP_KERNEL); >> + if (!ptr) >> + return ERR_PTR(-ENOMEM); >> + >> + phy = _of_phy_get(np, index); >> + if (!IS_ERR(phy)) { >> + *ptr = phy; >> + devres_add(dev, ptr); >> + } else { >> + devres_free(ptr); >> + } >> + >> + return phy; >> +} > > You want EXPORT_SYMBOL_GPL(devm_of_phy_get_by_index); here. > Ah, I missed that! Thanks Dmitry, Will update in next patch after any other comments. >> +/** >> * phy_create() - create a new phy >> * @dev: device that is creating the new phy >> * @node: device node of the phy >> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h >> index a0197fa..ae2ffaf 100644 >> --- a/include/linux/phy/phy.h >> +++ b/include/linux/phy/phy.h >> @@ -133,6 +133,8 @@ struct phy *devm_phy_get(struct device *dev, const char *string); >> struct phy *devm_phy_optional_get(struct device *dev, const char *string); >> struct phy *devm_of_phy_get(struct device *dev, struct device_node *np, >> const char *con_id); >> +struct phy *devm_of_phy_get_by_index(struct device *dev, struct device_node *np, >> + int index); >> void phy_put(struct phy *phy); >> void devm_phy_put(struct device *dev, struct phy *phy); >> struct phy *of_phy_get(struct device_node *np, const char *con_id); >> -- >> 2.3.2 >> >
Hi, On Saturday 21 March 2015 02:59 AM, Arun Ramamurthy wrote: > > > On 15-03-20 02:26 PM, Dmitry Torokhov wrote: >> Hi Arun, >> >> On Fri, Mar 20, 2015 at 02:07:08PM -0700, Arun Ramamurthy wrote: >>> Adding devm_of_phy_get_by_index to get phys by supplying an index >>> and not a phy name when multiple phys are declared I think a bit more explanation on why get_by_index is needed here. >>> >>> Reviewed-by: Ray Jui <rjui@broadcom.com> >>> Reviewed-by: Scott Branden <sbranden@broadcom.com> >>> Signed-off-by: Arun Ramamurthy <arun.ramamurthy@broadcom.com> >>> --- >>> drivers/phy/phy-core.c | 30 ++++++++++++++++++++++++++++++ >>> include/linux/phy/phy.h | 2 ++ >>> 2 files changed, 32 insertions(+) >>> >>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c >>> index a12d353..0c03876 100644 >>> --- a/drivers/phy/phy-core.c >>> +++ b/drivers/phy/phy-core.c >>> @@ -622,6 +622,36 @@ struct phy *devm_of_phy_get(struct device *dev, struct >>> device_node *np, >>> EXPORT_SYMBOL_GPL(devm_of_phy_get); >>> >>> /** >>> + * devm_of_phy_get_by_index() - lookup and obtain a reference to a phy by >>> index. >>> + * @dev: device that requests this phy >>> + * @np: node containing the phy >>> + * @index: index of the phy >>> + * >>> + * Gets the phy using _of_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_of_phy_get_by_index(struct device *dev, struct device_node >>> *np, >>> + int index) >>> +{ >>> + struct phy **ptr, *phy; >>> + >>> + ptr = devres_alloc(devm_phy_release, sizeof(*ptr), GFP_KERNEL); >>> + if (!ptr) >>> + return ERR_PTR(-ENOMEM); >>> + >>> + phy = _of_phy_get(np, index); >>> + if (!IS_ERR(phy)) { >>> + *ptr = phy; >>> + devres_add(dev, ptr); >>> + } else { >>> + devres_free(ptr); >>> + } >>> + >>> + return phy; >>> +} >> >> You want EXPORT_SYMBOL_GPL(devm_of_phy_get_by_index); here. Also update the Documentation/phy.txt. -Kishon
On 15-03-25 03:03 PM, Kishon Vijay Abraham I wrote: > Hi, > > On Saturday 21 March 2015 02:59 AM, Arun Ramamurthy wrote: >> >> >> On 15-03-20 02:26 PM, Dmitry Torokhov wrote: >>> Hi Arun, >>> >>> On Fri, Mar 20, 2015 at 02:07:08PM -0700, Arun Ramamurthy wrote: >>>> Adding devm_of_phy_get_by_index to get phys by supplying an index >>>> and not a phy name when multiple phys are declared > > I think a bit more explanation on why get_by_index is needed here. Thanks Kison. Can you be more specific? I am unsure of what more I can explain here. >>>> >>>> Reviewed-by: Ray Jui <rjui@broadcom.com> >>>> Reviewed-by: Scott Branden <sbranden@broadcom.com> >>>> Signed-off-by: Arun Ramamurthy <arun.ramamurthy@broadcom.com> >>>> --- >>>> drivers/phy/phy-core.c | 30 ++++++++++++++++++++++++++++++ >>>> include/linux/phy/phy.h | 2 ++ >>>> 2 files changed, 32 insertions(+) >>>> >>>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c >>>> index a12d353..0c03876 100644 >>>> --- a/drivers/phy/phy-core.c >>>> +++ b/drivers/phy/phy-core.c >>>> @@ -622,6 +622,36 @@ struct phy *devm_of_phy_get(struct device *dev, struct >>>> device_node *np, >>>> EXPORT_SYMBOL_GPL(devm_of_phy_get); >>>> >>>> /** >>>> + * devm_of_phy_get_by_index() - lookup and obtain a reference to a phy by >>>> index. >>>> + * @dev: device that requests this phy >>>> + * @np: node containing the phy >>>> + * @index: index of the phy >>>> + * >>>> + * Gets the phy using _of_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_of_phy_get_by_index(struct device *dev, struct device_node >>>> *np, >>>> + int index) >>>> +{ >>>> + struct phy **ptr, *phy; >>>> + >>>> + ptr = devres_alloc(devm_phy_release, sizeof(*ptr), GFP_KERNEL); >>>> + if (!ptr) >>>> + return ERR_PTR(-ENOMEM); >>>> + >>>> + phy = _of_phy_get(np, index); >>>> + if (!IS_ERR(phy)) { >>>> + *ptr = phy; >>>> + devres_add(dev, ptr); >>>> + } else { >>>> + devres_free(ptr); >>>> + } >>>> + >>>> + return phy; >>>> +} >>> >>> You want EXPORT_SYMBOL_GPL(devm_of_phy_get_by_index); here. > > Also update the Documentation/phy.txt. > Ok will do. > -Kishon >
On Wed, Mar 25, 2015 at 05:04:32PM -0700, Arun Ramamurthy wrote: > > > On 15-03-25 03:03 PM, Kishon Vijay Abraham I wrote: > >Hi, > > > >On Saturday 21 March 2015 02:59 AM, Arun Ramamurthy wrote: > >> > >> > >>On 15-03-20 02:26 PM, Dmitry Torokhov wrote: > >>>Hi Arun, > >>> > >>>On Fri, Mar 20, 2015 at 02:07:08PM -0700, Arun Ramamurthy wrote: > >>>>Adding devm_of_phy_get_by_index to get phys by supplying an index > >>>>and not a phy name when multiple phys are declared > > > >I think a bit more explanation on why get_by_index is needed here. > Thanks Kison. Can you be more specific? I am unsure of what more I > can explain here. We just need to mention that some generic drivers, such as ehci, may use multiple phys, and for such drivers referencing phy(s) by name(s) does not make sense. Instead of inventing elaborate naming schemes and producing custom code to iterate over names, such drivers are better of using nameless phy bindings and using this newly introduced API to iterate through them. Thanks.
On Friday 27 March 2015 04:37 AM, Dmitry Torokhov wrote: > On Wed, Mar 25, 2015 at 05:04:32PM -0700, Arun Ramamurthy wrote: >> >> >> On 15-03-25 03:03 PM, Kishon Vijay Abraham I wrote: >>> Hi, >>> >>> On Saturday 21 March 2015 02:59 AM, Arun Ramamurthy wrote: >>>> >>>> >>>> On 15-03-20 02:26 PM, Dmitry Torokhov wrote: >>>>> Hi Arun, >>>>> >>>>> On Fri, Mar 20, 2015 at 02:07:08PM -0700, Arun Ramamurthy wrote: >>>>>> Adding devm_of_phy_get_by_index to get phys by supplying an index >>>>>> and not a phy name when multiple phys are declared >>> >>> I think a bit more explanation on why get_by_index is needed here. >> Thanks Kison. Can you be more specific? I am unsure of what more I >> can explain here. > > We just need to mention that some generic drivers, such as ehci, may use > multiple phys, and for such drivers referencing phy(s) by name(s) does > not make sense. Instead of inventing elaborate naming schemes and > producing custom code to iterate over names, such drivers are better of > using nameless phy bindings and using this newly introduced API to > iterate through them. +1 -Kishon
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c index a12d353..0c03876 100644 --- a/drivers/phy/phy-core.c +++ b/drivers/phy/phy-core.c @@ -622,6 +622,36 @@ struct phy *devm_of_phy_get(struct device *dev, struct device_node *np, EXPORT_SYMBOL_GPL(devm_of_phy_get); /** + * devm_of_phy_get_by_index() - lookup and obtain a reference to a phy by index. + * @dev: device that requests this phy + * @np: node containing the phy + * @index: index of the phy + * + * Gets the phy using _of_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_of_phy_get_by_index(struct device *dev, struct device_node *np, + int index) +{ + struct phy **ptr, *phy; + + ptr = devres_alloc(devm_phy_release, sizeof(*ptr), GFP_KERNEL); + if (!ptr) + return ERR_PTR(-ENOMEM); + + phy = _of_phy_get(np, index); + if (!IS_ERR(phy)) { + *ptr = phy; + devres_add(dev, ptr); + } else { + devres_free(ptr); + } + + return phy; +} +/** * phy_create() - create a new phy * @dev: device that is creating the new phy * @node: device node of the phy diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h index a0197fa..ae2ffaf 100644 --- a/include/linux/phy/phy.h +++ b/include/linux/phy/phy.h @@ -133,6 +133,8 @@ struct phy *devm_phy_get(struct device *dev, const char *string); struct phy *devm_phy_optional_get(struct device *dev, const char *string); struct phy *devm_of_phy_get(struct device *dev, struct device_node *np, const char *con_id); +struct phy *devm_of_phy_get_by_index(struct device *dev, struct device_node *np, + int index); void phy_put(struct phy *phy); void devm_phy_put(struct device *dev, struct phy *phy); struct phy *of_phy_get(struct device_node *np, const char *con_id);