| Message ID | 20191220002430.11995-1-cw00.choi@samsung.com |
|---|---|
| Headers | show |
| Series | PM / devfreq: Remove deprecated 'devfreq' and 'devfreq-events' properties | expand |
On 20.12.2019 02:18, Chanwoo Choi wrote: > Previously, devfreq core support 'devfreq' property in order to get > the devfreq device by phandle. But, 'devfreq' property name is not proper > on devicetree binding because this name doesn't mean the any h/w attribute. > > The devfreq core hand over the right to decide the property name > for getting the devfreq device on devicetree. Each devfreq driver > will decide the property name on devicetree binding and then get > the devfreq device by using devfreq_get_devfreq_by_node(). > > Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com> > --- > drivers/devfreq/devfreq.c | 35 ----------------------------------- > drivers/devfreq/exynos-bus.c | 12 +++++++++++- > include/linux/devfreq.h | 8 -------- > 3 files changed, 11 insertions(+), 44 deletions(-) > > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c > index cb8ca81c8973..c3d3c7c802a0 100644 > --- a/drivers/devfreq/devfreq.c > +++ b/drivers/devfreq/devfreq.c > @@ -991,48 +991,13 @@ struct devfreq *devfreq_get_devfreq_by_node(struct device_node *node) > > return ERR_PTR(-ENODEV); > } > - > -/* > - * devfreq_get_devfreq_by_phandle - Get the devfreq device from devicetree > - * @dev - instance to the given device > - * @index - index into list of devfreq > - * > - * return the instance of devfreq device > - */ > -struct devfreq *devfreq_get_devfreq_by_phandle(struct device *dev, int index) > -{ > - struct device_node *node; > - struct devfreq *devfreq; > - > - if (!dev) > - return ERR_PTR(-EINVAL); > - > - if (!dev->of_node) > - return ERR_PTR(-EINVAL); > - > - node = of_parse_phandle(dev->of_node, "devfreq", index); > - if (!node) > - return ERR_PTR(-ENODEV); > - > - devfreq = devfreq_get_devfreq_by_node(node); > - of_node_put(node); > - > - return devfreq; > -} > - > #else > struct devfreq *devfreq_get_devfreq_by_node(struct device_node *node) > { > return ERR_PTR(-ENODEV); > } > - > -struct devfreq *devfreq_get_devfreq_by_phandle(struct device *dev, int index) > -{ > - return ERR_PTR(-ENODEV); > -} > #endif /* CONFIG_OF */ > EXPORT_SYMBOL_GPL(devfreq_get_devfreq_by_node); > -EXPORT_SYMBOL_GPL(devfreq_get_devfreq_by_phandle); > > /** > * devm_devfreq_remove_device() - Resource-managed devfreq_remove_device() > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c > index 7f5917d59072..1bc4e3c81115 100644 > --- a/drivers/devfreq/exynos-bus.c > +++ b/drivers/devfreq/exynos-bus.c > @@ -86,6 +86,16 @@ static int exynos_bus_get_event(struct exynos_bus *bus, > return ret; > } > > +static struct devfreq *exynos_bus_get_parent_devfreq(struct device_node *np) > +{ > + struct device_node *node = of_parse_phandle(np, "devfreq", 0); > + > + if (!node) > + return ERR_PTR(-ENODEV); > + > + return devfreq_get_devfreq_by_node(node); You need to call of_node_put(node) here and in several other places. The old devfreq_get_devfreq_by_phandle API handled this internally but devfreq_get_devfreq_by_node doesn't. Maybe the _by_phandle API could be kept and just take the property name instead of always using "devfreq"? > +} > + > /* > * devfreq function for both simple-ondemand and passive governor > */ > @@ -353,7 +363,7 @@ static int exynos_bus_profile_init_passive(struct exynos_bus *bus, > profile->exit = exynos_bus_passive_exit; > > /* Get the instance of parent devfreq device */ > - parent_devfreq = devfreq_get_devfreq_by_phandle(dev, 0); > + parent_devfreq = exynos_bus_get_parent_devfreq(dev->of_node); > if (IS_ERR(parent_devfreq)) > return -EPROBE_DEFER; > > diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h > index 1dccc47acbce..a4351698fb64 100644 > --- a/include/linux/devfreq.h > +++ b/include/linux/devfreq.h > @@ -254,8 +254,6 @@ extern void devm_devfreq_unregister_notifier(struct device *dev, > struct notifier_block *nb, > unsigned int list); > extern struct devfreq *devfreq_get_devfreq_by_node(struct device_node *node); > -extern struct devfreq *devfreq_get_devfreq_by_phandle(struct device *dev, > - int index); > > #if IS_ENABLED(CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND) > /** > @@ -413,12 +411,6 @@ static inline struct devfreq *devfreq_get_devfreq_by_node(struct device_node *no > return ERR_PTR(-ENODEV); > } > > -static inline struct devfreq *devfreq_get_devfreq_by_phandle(struct device *dev, > - int index) > -{ > - return ERR_PTR(-ENODEV); > -} > - > static inline int devfreq_update_stats(struct devfreq *df) > { > return -EINVAL; >
On 12/20/19 9:46 AM, Leonard Crestez wrote: > On 20.12.2019 02:18, Chanwoo Choi wrote: >> Previously, devfreq core support 'devfreq' property in order to get >> the devfreq device by phandle. But, 'devfreq' property name is not proper >> on devicetree binding because this name doesn't mean the any h/w attribute. >> >> The devfreq core hand over the right to decide the property name >> for getting the devfreq device on devicetree. Each devfreq driver >> will decide the property name on devicetree binding and then get >> the devfreq device by using devfreq_get_devfreq_by_node(). >> >> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com> >> --- >> drivers/devfreq/devfreq.c | 35 ----------------------------------- >> drivers/devfreq/exynos-bus.c | 12 +++++++++++- >> include/linux/devfreq.h | 8 -------- >> 3 files changed, 11 insertions(+), 44 deletions(-) >> >> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c >> index cb8ca81c8973..c3d3c7c802a0 100644 >> --- a/drivers/devfreq/devfreq.c >> +++ b/drivers/devfreq/devfreq.c >> @@ -991,48 +991,13 @@ struct devfreq *devfreq_get_devfreq_by_node(struct device_node *node) >> >> return ERR_PTR(-ENODEV); >> } >> - >> -/* >> - * devfreq_get_devfreq_by_phandle - Get the devfreq device from devicetree >> - * @dev - instance to the given device >> - * @index - index into list of devfreq >> - * >> - * return the instance of devfreq device >> - */ >> -struct devfreq *devfreq_get_devfreq_by_phandle(struct device *dev, int index) >> -{ >> - struct device_node *node; >> - struct devfreq *devfreq; >> - >> - if (!dev) >> - return ERR_PTR(-EINVAL); >> - >> - if (!dev->of_node) >> - return ERR_PTR(-EINVAL); >> - >> - node = of_parse_phandle(dev->of_node, "devfreq", index); >> - if (!node) >> - return ERR_PTR(-ENODEV); >> - >> - devfreq = devfreq_get_devfreq_by_node(node); >> - of_node_put(node); >> - >> - return devfreq; >> -} >> - >> #else >> struct devfreq *devfreq_get_devfreq_by_node(struct device_node *node) >> { >> return ERR_PTR(-ENODEV); >> } >> - >> -struct devfreq *devfreq_get_devfreq_by_phandle(struct device *dev, int index) >> -{ >> - return ERR_PTR(-ENODEV); >> -} >> #endif /* CONFIG_OF */ >> EXPORT_SYMBOL_GPL(devfreq_get_devfreq_by_node); >> -EXPORT_SYMBOL_GPL(devfreq_get_devfreq_by_phandle); >> >> /** >> * devm_devfreq_remove_device() - Resource-managed devfreq_remove_device() >> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c >> index 7f5917d59072..1bc4e3c81115 100644 >> --- a/drivers/devfreq/exynos-bus.c >> +++ b/drivers/devfreq/exynos-bus.c >> @@ -86,6 +86,16 @@ static int exynos_bus_get_event(struct exynos_bus *bus, >> return ret; >> } >> >> +static struct devfreq *exynos_bus_get_parent_devfreq(struct device_node *np) >> +{ >> + struct device_node *node = of_parse_phandle(np, "devfreq", 0); >> + >> + if (!node) >> + return ERR_PTR(-ENODEV); >> + >> + return devfreq_get_devfreq_by_node(node); > > You need to call of_node_put(node) here and in several other places. > > The old devfreq_get_devfreq_by_phandle API handled this internally but > devfreq_get_devfreq_by_node doesn't. Thanks. I'll fix it. > > Maybe the _by_phandle API could be kept and just take the property name > instead of always using "devfreq"? Do you mean like below? devfreq_get_devfreq_by_phandle(struct device *dev, int index) -> devfreq_get_devfreq_by_phandle(struct device *dev, char *property_name, int index) In case of devfreq-event.c, struct devfreq_event_dev *devfreq_event_get_edev_by_phandle( struct device *dev, char property_name, int index) int devfreq_event_get_edev_count(struct device *dev, char *property_name) > >> +} >> + >> /* >> * devfreq function for both simple-ondemand and passive governor >> */ >> @@ -353,7 +363,7 @@ static int exynos_bus_profile_init_passive(struct exynos_bus *bus, >> profile->exit = exynos_bus_passive_exit; >> >> /* Get the instance of parent devfreq device */ >> - parent_devfreq = devfreq_get_devfreq_by_phandle(dev, 0); >> + parent_devfreq = exynos_bus_get_parent_devfreq(dev->of_node); >> if (IS_ERR(parent_devfreq)) >> return -EPROBE_DEFER; >> >> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h >> index 1dccc47acbce..a4351698fb64 100644 >> --- a/include/linux/devfreq.h >> +++ b/include/linux/devfreq.h >> @@ -254,8 +254,6 @@ extern void devm_devfreq_unregister_notifier(struct device *dev, >> struct notifier_block *nb, >> unsigned int list); >> extern struct devfreq *devfreq_get_devfreq_by_node(struct device_node *node); >> -extern struct devfreq *devfreq_get_devfreq_by_phandle(struct device *dev, >> - int index); >> >> #if IS_ENABLED(CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND) >> /** >> @@ -413,12 +411,6 @@ static inline struct devfreq *devfreq_get_devfreq_by_node(struct device_node *no >> return ERR_PTR(-ENODEV); >> } >> >> -static inline struct devfreq *devfreq_get_devfreq_by_phandle(struct device *dev, >> - int index) >> -{ >> - return ERR_PTR(-ENODEV); >> -} >> - >> static inline int devfreq_update_stats(struct devfreq *df) >> { >> return -EINVAL; >> > > >
On 2019-12-20 2:54 AM, Chanwoo Choi wrote: > On 12/20/19 9:46 AM, Leonard Crestez wrote: >> On 20.12.2019 02:18, Chanwoo Choi wrote: >>> Previously, devfreq core support 'devfreq' property in order to get >>> the devfreq device by phandle. But, 'devfreq' property name is not proper >>> on devicetree binding because this name doesn't mean the any h/w attribute. >>> >>> The devfreq core hand over the right to decide the property name >>> for getting the devfreq device on devicetree. Each devfreq driver >>> will decide the property name on devicetree binding and then get >>> the devfreq device by using devfreq_get_devfreq_by_node(). >>> >>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com> >>> --- >>> drivers/devfreq/devfreq.c | 35 ----------------------------------- >>> drivers/devfreq/exynos-bus.c | 12 +++++++++++- >>> include/linux/devfreq.h | 8 -------- >>> 3 files changed, 11 insertions(+), 44 deletions(-) >>> >>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c >>> index cb8ca81c8973..c3d3c7c802a0 100644 >>> --- a/drivers/devfreq/devfreq.c >>> +++ b/drivers/devfreq/devfreq.c >>> @@ -991,48 +991,13 @@ struct devfreq *devfreq_get_devfreq_by_node(struct device_node *node) >>> >>> return ERR_PTR(-ENODEV); >>> } >>> - >>> -/* >>> - * devfreq_get_devfreq_by_phandle - Get the devfreq device from devicetree >>> - * @dev - instance to the given device >>> - * @index - index into list of devfreq >>> - * >>> - * return the instance of devfreq device >>> - */ >>> -struct devfreq *devfreq_get_devfreq_by_phandle(struct device *dev, int index) >>> -{ >>> - struct device_node *node; >>> - struct devfreq *devfreq; >>> - >>> - if (!dev) >>> - return ERR_PTR(-EINVAL); >>> - >>> - if (!dev->of_node) >>> - return ERR_PTR(-EINVAL); >>> - >>> - node = of_parse_phandle(dev->of_node, "devfreq", index); >>> - if (!node) >>> - return ERR_PTR(-ENODEV); >>> - >>> - devfreq = devfreq_get_devfreq_by_node(node); >>> - of_node_put(node); >>> - >>> - return devfreq; >>> -} >>> - >>> #else >>> struct devfreq *devfreq_get_devfreq_by_node(struct device_node *node) >>> { >>> return ERR_PTR(-ENODEV); >>> } >>> - >>> -struct devfreq *devfreq_get_devfreq_by_phandle(struct device *dev, int index) >>> -{ >>> - return ERR_PTR(-ENODEV); >>> -} >>> #endif /* CONFIG_OF */ >>> EXPORT_SYMBOL_GPL(devfreq_get_devfreq_by_node); >>> -EXPORT_SYMBOL_GPL(devfreq_get_devfreq_by_phandle); >>> >>> /** >>> * devm_devfreq_remove_device() - Resource-managed devfreq_remove_device() >>> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c >>> index 7f5917d59072..1bc4e3c81115 100644 >>> --- a/drivers/devfreq/exynos-bus.c >>> +++ b/drivers/devfreq/exynos-bus.c >>> @@ -86,6 +86,16 @@ static int exynos_bus_get_event(struct exynos_bus *bus, >>> return ret; >>> } >>> >>> +static struct devfreq *exynos_bus_get_parent_devfreq(struct device_node *np) >>> +{ >>> + struct device_node *node = of_parse_phandle(np, "devfreq", 0); >>> + >>> + if (!node) >>> + return ERR_PTR(-ENODEV); >>> + >>> + return devfreq_get_devfreq_by_node(node); >> >> You need to call of_node_put(node) here and in several other places. >> >> The old devfreq_get_devfreq_by_phandle API handled this internally but >> devfreq_get_devfreq_by_node doesn't. > > Thanks. I'll fix it. > >> >> Maybe the _by_phandle API could be kept and just take the property name >> instead of always using "devfreq"? > > Do you mean like below? > devfreq_get_devfreq_by_phandle(struct device *dev, int index) > -> devfreq_get_devfreq_by_phandle(struct device *dev, char *property_name, int index) > > In case of devfreq-event.c, > struct devfreq_event_dev *devfreq_event_get_edev_by_phandle( > struct device *dev, > char property_name, > int index) > int devfreq_event_get_edev_count(struct device *dev, char *property_name) Yes. These helpers would avoid the need for explicit of_node_put. > >> >>> +} >>> + >>> /* >>> * devfreq function for both simple-ondemand and passive governor >>> */ >>> @@ -353,7 +363,7 @@ static int exynos_bus_profile_init_passive(struct exynos_bus *bus, >>> profile->exit = exynos_bus_passive_exit; >>> >>> /* Get the instance of parent devfreq device */ >>> - parent_devfreq = devfreq_get_devfreq_by_phandle(dev, 0); >>> + parent_devfreq = exynos_bus_get_parent_devfreq(dev->of_node); >>> if (IS_ERR(parent_devfreq)) >>> return -EPROBE_DEFER; >>> >>> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h >>> index 1dccc47acbce..a4351698fb64 100644 >>> --- a/include/linux/devfreq.h >>> +++ b/include/linux/devfreq.h >>> @@ -254,8 +254,6 @@ extern void devm_devfreq_unregister_notifier(struct device *dev, >>> struct notifier_block *nb, >>> unsigned int list); >>> extern struct devfreq *devfreq_get_devfreq_by_node(struct device_node *node); >>> -extern struct devfreq *devfreq_get_devfreq_by_phandle(struct device *dev, >>> - int index); >>> >>> #if IS_ENABLED(CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND) >>> /** >>> @@ -413,12 +411,6 @@ static inline struct devfreq *devfreq_get_devfreq_by_node(struct device_node *no >>> return ERR_PTR(-ENODEV); >>> } >>> >>> -static inline struct devfreq *devfreq_get_devfreq_by_phandle(struct device *dev, >>> - int index) >>> -{ >>> - return ERR_PTR(-ENODEV); >>> -} >>> - >>> static inline int devfreq_update_stats(struct devfreq *df) >>> { >>> return -EINVAL;
On 12/20/19 10:40 AM, Leonard Crestez wrote: > On 2019-12-20 2:54 AM, Chanwoo Choi wrote: >> On 12/20/19 9:46 AM, Leonard Crestez wrote: >>> On 20.12.2019 02:18, Chanwoo Choi wrote: >>>> Previously, devfreq core support 'devfreq' property in order to get >>>> the devfreq device by phandle. But, 'devfreq' property name is not proper >>>> on devicetree binding because this name doesn't mean the any h/w attribute. >>>> >>>> The devfreq core hand over the right to decide the property name >>>> for getting the devfreq device on devicetree. Each devfreq driver >>>> will decide the property name on devicetree binding and then get >>>> the devfreq device by using devfreq_get_devfreq_by_node(). >>>> >>>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com> >>>> --- >>>> drivers/devfreq/devfreq.c | 35 ----------------------------------- >>>> drivers/devfreq/exynos-bus.c | 12 +++++++++++- >>>> include/linux/devfreq.h | 8 -------- >>>> 3 files changed, 11 insertions(+), 44 deletions(-) >>>> >>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c >>>> index cb8ca81c8973..c3d3c7c802a0 100644 >>>> --- a/drivers/devfreq/devfreq.c >>>> +++ b/drivers/devfreq/devfreq.c >>>> @@ -991,48 +991,13 @@ struct devfreq *devfreq_get_devfreq_by_node(struct device_node *node) >>>> >>>> return ERR_PTR(-ENODEV); >>>> } >>>> - >>>> -/* >>>> - * devfreq_get_devfreq_by_phandle - Get the devfreq device from devicetree >>>> - * @dev - instance to the given device >>>> - * @index - index into list of devfreq >>>> - * >>>> - * return the instance of devfreq device >>>> - */ >>>> -struct devfreq *devfreq_get_devfreq_by_phandle(struct device *dev, int index) >>>> -{ >>>> - struct device_node *node; >>>> - struct devfreq *devfreq; >>>> - >>>> - if (!dev) >>>> - return ERR_PTR(-EINVAL); >>>> - >>>> - if (!dev->of_node) >>>> - return ERR_PTR(-EINVAL); >>>> - >>>> - node = of_parse_phandle(dev->of_node, "devfreq", index); >>>> - if (!node) >>>> - return ERR_PTR(-ENODEV); >>>> - >>>> - devfreq = devfreq_get_devfreq_by_node(node); >>>> - of_node_put(node); >>>> - >>>> - return devfreq; >>>> -} >>>> - >>>> #else >>>> struct devfreq *devfreq_get_devfreq_by_node(struct device_node *node) >>>> { >>>> return ERR_PTR(-ENODEV); >>>> } >>>> - >>>> -struct devfreq *devfreq_get_devfreq_by_phandle(struct device *dev, int index) >>>> -{ >>>> - return ERR_PTR(-ENODEV); >>>> -} >>>> #endif /* CONFIG_OF */ >>>> EXPORT_SYMBOL_GPL(devfreq_get_devfreq_by_node); >>>> -EXPORT_SYMBOL_GPL(devfreq_get_devfreq_by_phandle); >>>> >>>> /** >>>> * devm_devfreq_remove_device() - Resource-managed devfreq_remove_device() >>>> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c >>>> index 7f5917d59072..1bc4e3c81115 100644 >>>> --- a/drivers/devfreq/exynos-bus.c >>>> +++ b/drivers/devfreq/exynos-bus.c >>>> @@ -86,6 +86,16 @@ static int exynos_bus_get_event(struct exynos_bus *bus, >>>> return ret; >>>> } >>>> >>>> +static struct devfreq *exynos_bus_get_parent_devfreq(struct device_node *np) >>>> +{ >>>> + struct device_node *node = of_parse_phandle(np, "devfreq", 0); >>>> + >>>> + if (!node) >>>> + return ERR_PTR(-ENODEV); >>>> + >>>> + return devfreq_get_devfreq_by_node(node); >>> >>> You need to call of_node_put(node) here and in several other places. >>> >>> The old devfreq_get_devfreq_by_phandle API handled this internally but >>> devfreq_get_devfreq_by_node doesn't. >> >> Thanks. I'll fix it. >> >>> >>> Maybe the _by_phandle API could be kept and just take the property name >>> instead of always using "devfreq"? >> >> Do you mean like below? >> devfreq_get_devfreq_by_phandle(struct device *dev, int index) >> -> devfreq_get_devfreq_by_phandle(struct device *dev, char *property_name, int index) >> >> In case of devfreq-event.c, >> struct devfreq_event_dev *devfreq_event_get_edev_by_phandle( >> struct device *dev, >> char property_name, >> int index) >> int devfreq_event_get_edev_count(struct device *dev, char *property_name) > > Yes. These helpers would avoid the need for explicit of_node_put. OK. Instead of removing devfreq_event_get_edev_by_phandle, change the function property of devfreq_event_get_edev_by_phandle on v3. After getting the review for dt-binding patch, I'll send v3 patches. > >> >>> >>>> +} >>>> + >>>> /* >>>> * devfreq function for both simple-ondemand and passive governor >>>> */ >>>> @@ -353,7 +363,7 @@ static int exynos_bus_profile_init_passive(struct exynos_bus *bus, >>>> profile->exit = exynos_bus_passive_exit; >>>> >>>> /* Get the instance of parent devfreq device */ >>>> - parent_devfreq = devfreq_get_devfreq_by_phandle(dev, 0); >>>> + parent_devfreq = exynos_bus_get_parent_devfreq(dev->of_node); >>>> if (IS_ERR(parent_devfreq)) >>>> return -EPROBE_DEFER; >>>> >>>> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h >>>> index 1dccc47acbce..a4351698fb64 100644 >>>> --- a/include/linux/devfreq.h >>>> +++ b/include/linux/devfreq.h >>>> @@ -254,8 +254,6 @@ extern void devm_devfreq_unregister_notifier(struct device *dev, >>>> struct notifier_block *nb, >>>> unsigned int list); >>>> extern struct devfreq *devfreq_get_devfreq_by_node(struct device_node *node); >>>> -extern struct devfreq *devfreq_get_devfreq_by_phandle(struct device *dev, >>>> - int index); >>>> >>>> #if IS_ENABLED(CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND) >>>> /** >>>> @@ -413,12 +411,6 @@ static inline struct devfreq *devfreq_get_devfreq_by_node(struct device_node *no >>>> return ERR_PTR(-ENODEV); >>>> } >>>> >>>> -static inline struct devfreq *devfreq_get_devfreq_by_phandle(struct device *dev, >>>> - int index) >>>> -{ >>>> - return ERR_PTR(-ENODEV); >>>> -} >>>> - >>>> static inline int devfreq_update_stats(struct devfreq *df) >>>> { >>>> return -EINVAL; > > >
Hi Chanwoo, On 12/20/19 12:24 AM, Chanwoo Choi wrote: > From: Leonard Crestez <leonard.crestez@nxp.com> > > Split off part of devfreq_get_devfreq_by_phandle into a separate > function. This allows callers to fetch devfreq instances by enumerating > devicetree instead of explicit phandles. > > [lkp: Reported the build error] > Reported-by: kbuild test robot <lkp@intel.com> > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> > [cw00.choi: Export devfreq_get_devfreq_by_node function and > add function to devfreq.h when CONFIG_PM_DEVFREQ is enabled.] > Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com> > --- > drivers/devfreq/devfreq.c | 46 +++++++++++++++++++++++++++++---------- > include/linux/devfreq.h | 6 +++++ > 2 files changed, 41 insertions(+), 11 deletions(-) > > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c > index 89260b17598f..cb8ca81c8973 100644 > --- a/drivers/devfreq/devfreq.c > +++ b/drivers/devfreq/devfreq.c > @@ -966,6 +966,32 @@ struct devfreq *devm_devfreq_add_device(struct device *dev, > EXPORT_SYMBOL(devm_devfreq_add_device); > > #ifdef CONFIG_OF > +/* > + * devfreq_get_devfreq_by_node - Get the devfreq device from devicetree > + * @node - pointer to device_node > + * > + * return the instance of devfreq device > + */ > +struct devfreq *devfreq_get_devfreq_by_node(struct device_node *node) > +{ > + struct devfreq *devfreq; > + > + if (!node) > + return ERR_PTR(-EINVAL); > + > + mutex_lock(&devfreq_list_lock); > + list_for_each_entry(devfreq, &devfreq_list, node) { > + if (devfreq->dev.parent > + && devfreq->dev.parent->of_node == node) { > + mutex_unlock(&devfreq_list_lock); > + return devfreq; > + } > + } > + mutex_unlock(&devfreq_list_lock); > + > + return ERR_PTR(-ENODEV); > +} > + > /* > * devfreq_get_devfreq_by_phandle - Get the devfreq device from devicetree > * @dev - instance to the given device > @@ -988,26 +1014,24 @@ struct devfreq *devfreq_get_devfreq_by_phandle(struct device *dev, int index) > if (!node) > return ERR_PTR(-ENODEV); > > - mutex_lock(&devfreq_list_lock); > - list_for_each_entry(devfreq, &devfreq_list, node) { > - if (devfreq->dev.parent > - && devfreq->dev.parent->of_node == node) { > - mutex_unlock(&devfreq_list_lock); > - of_node_put(node); > - return devfreq; > - } > - } > - mutex_unlock(&devfreq_list_lock); > + devfreq = devfreq_get_devfreq_by_node(node); > of_node_put(node); > > - return ERR_PTR(-EPROBE_DEFER); > + return devfreq; > } > + > #else > +struct devfreq *devfreq_get_devfreq_by_node(struct device_node *node) > +{ > + return ERR_PTR(-ENODEV); > +} > + > struct devfreq *devfreq_get_devfreq_by_phandle(struct device *dev, int index) > { > return ERR_PTR(-ENODEV); > } > #endif /* CONFIG_OF */ > +EXPORT_SYMBOL_GPL(devfreq_get_devfreq_by_node); > EXPORT_SYMBOL_GPL(devfreq_get_devfreq_by_phandle); > > /** > diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h > index c6f82d4bec9f..1dccc47acbce 100644 > --- a/include/linux/devfreq.h > +++ b/include/linux/devfreq.h > @@ -253,6 +253,7 @@ extern void devm_devfreq_unregister_notifier(struct device *dev, > struct devfreq *devfreq, > struct notifier_block *nb, > unsigned int list); > +extern struct devfreq *devfreq_get_devfreq_by_node(struct device_node *node); It can go without 'extern' in the header. > extern struct devfreq *devfreq_get_devfreq_by_phandle(struct device *dev, > int index); > > @@ -407,6 +408,11 @@ static inline void devm_devfreq_unregister_notifier(struct device *dev, > { > } > > +static inline struct devfreq *devfreq_get_devfreq_by_node(struct device_node *node) > +{ > + return ERR_PTR(-ENODEV); > +} > + > static inline struct devfreq *devfreq_get_devfreq_by_phandle(struct device *dev, > int index) > { > Apart from this minor thing, looks good to me. When you fix it, feel free to add Reviewed-by: Lukasz Luba <lukasz.luba@arm.com> Regards, Lukasz
Hi Chanwoo, On 12/20/19 12:24 AM, Chanwoo Choi wrote: > Previously, devfreq core support 'devfreq' property in order to get > the devfreq device by phandle. But, 'devfreq' property name is not proper > on devicetree binding because this name doesn't mean the any h/w attribute. > > The devfreq core hand over the right to decide the property name > for getting the devfreq device on devicetree. Each devfreq driver > will decide the property name on devicetree binding and then get > the devfreq device by using devfreq_get_devfreq_by_node(). > > Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com> > --- > drivers/devfreq/devfreq.c | 35 ----------------------------------- > drivers/devfreq/exynos-bus.c | 12 +++++++++++- > include/linux/devfreq.h | 8 -------- > 3 files changed, 11 insertions(+), 44 deletions(-) > > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c > index cb8ca81c8973..c3d3c7c802a0 100644 > --- a/drivers/devfreq/devfreq.c > +++ b/drivers/devfreq/devfreq.c > @@ -991,48 +991,13 @@ struct devfreq *devfreq_get_devfreq_by_node(struct device_node *node) > > return ERR_PTR(-ENODEV); > } > - > -/* > - * devfreq_get_devfreq_by_phandle - Get the devfreq device from devicetree > - * @dev - instance to the given device > - * @index - index into list of devfreq > - * > - * return the instance of devfreq device > - */ > -struct devfreq *devfreq_get_devfreq_by_phandle(struct device *dev, int index) > -{ > - struct device_node *node; > - struct devfreq *devfreq; > - > - if (!dev) > - return ERR_PTR(-EINVAL); > - > - if (!dev->of_node) > - return ERR_PTR(-EINVAL); > - > - node = of_parse_phandle(dev->of_node, "devfreq", index); > - if (!node) > - return ERR_PTR(-ENODEV); > - > - devfreq = devfreq_get_devfreq_by_node(node); > - of_node_put(node); > - > - return devfreq; > -} > - > #else > struct devfreq *devfreq_get_devfreq_by_node(struct device_node *node) > { > return ERR_PTR(-ENODEV); > } > - > -struct devfreq *devfreq_get_devfreq_by_phandle(struct device *dev, int index) > -{ > - return ERR_PTR(-ENODEV); > -} > #endif /* CONFIG_OF */ > EXPORT_SYMBOL_GPL(devfreq_get_devfreq_by_node); > -EXPORT_SYMBOL_GPL(devfreq_get_devfreq_by_phandle); > > /** > * devm_devfreq_remove_device() - Resource-managed devfreq_remove_device() > diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c > index 7f5917d59072..1bc4e3c81115 100644 > --- a/drivers/devfreq/exynos-bus.c > +++ b/drivers/devfreq/exynos-bus.c > @@ -86,6 +86,16 @@ static int exynos_bus_get_event(struct exynos_bus *bus, > return ret; > } > > +static struct devfreq *exynos_bus_get_parent_devfreq(struct device_node *np) > +{ > + struct device_node *node = of_parse_phandle(np, "devfreq", 0); > + > + if (!node) > + return ERR_PTR(-ENODEV); > + > + return devfreq_get_devfreq_by_node(node); > +} > + > /* > * devfreq function for both simple-ondemand and passive governor > */ > @@ -353,7 +363,7 @@ static int exynos_bus_profile_init_passive(struct exynos_bus *bus, > profile->exit = exynos_bus_passive_exit; > > /* Get the instance of parent devfreq device */ > - parent_devfreq = devfreq_get_devfreq_by_phandle(dev, 0); > + parent_devfreq = exynos_bus_get_parent_devfreq(dev->of_node); > if (IS_ERR(parent_devfreq)) > return -EPROBE_DEFER; > These changes won't apply, probably I need some base for it. Regards, Lukasz
On 1/9/20 7:37 PM, Lukasz Luba wrote: > Hi Chanwoo, > > On 12/20/19 12:24 AM, Chanwoo Choi wrote: >> Previously, devfreq core support 'devfreq' property in order to get >> the devfreq device by phandle. But, 'devfreq' property name is not proper >> on devicetree binding because this name doesn't mean the any h/w attribute. >> >> The devfreq core hand over the right to decide the property name >> for getting the devfreq device on devicetree. Each devfreq driver >> will decide the property name on devicetree binding and then get >> the devfreq device by using devfreq_get_devfreq_by_node(). >> >> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com> >> --- >> drivers/devfreq/devfreq.c | 35 ----------------------------------- >> drivers/devfreq/exynos-bus.c | 12 +++++++++++- >> include/linux/devfreq.h | 8 -------- >> 3 files changed, 11 insertions(+), 44 deletions(-) >> >> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c >> index cb8ca81c8973..c3d3c7c802a0 100644 >> --- a/drivers/devfreq/devfreq.c >> +++ b/drivers/devfreq/devfreq.c >> @@ -991,48 +991,13 @@ struct devfreq *devfreq_get_devfreq_by_node(struct device_node *node) >> return ERR_PTR(-ENODEV); >> } >> - >> -/* >> - * devfreq_get_devfreq_by_phandle - Get the devfreq device from devicetree >> - * @dev - instance to the given device >> - * @index - index into list of devfreq >> - * >> - * return the instance of devfreq device >> - */ >> -struct devfreq *devfreq_get_devfreq_by_phandle(struct device *dev, int index) >> -{ >> - struct device_node *node; >> - struct devfreq *devfreq; >> - >> - if (!dev) >> - return ERR_PTR(-EINVAL); >> - >> - if (!dev->of_node) >> - return ERR_PTR(-EINVAL); >> - >> - node = of_parse_phandle(dev->of_node, "devfreq", index); >> - if (!node) >> - return ERR_PTR(-ENODEV); >> - >> - devfreq = devfreq_get_devfreq_by_node(node); >> - of_node_put(node); >> - >> - return devfreq; >> -} >> - >> #else >> struct devfreq *devfreq_get_devfreq_by_node(struct device_node *node) >> { >> return ERR_PTR(-ENODEV); >> } >> - >> -struct devfreq *devfreq_get_devfreq_by_phandle(struct device *dev, int index) >> -{ >> - return ERR_PTR(-ENODEV); >> -} >> #endif /* CONFIG_OF */ >> EXPORT_SYMBOL_GPL(devfreq_get_devfreq_by_node); >> -EXPORT_SYMBOL_GPL(devfreq_get_devfreq_by_phandle); >> /** >> * devm_devfreq_remove_device() - Resource-managed devfreq_remove_device() >> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c >> index 7f5917d59072..1bc4e3c81115 100644 >> --- a/drivers/devfreq/exynos-bus.c >> +++ b/drivers/devfreq/exynos-bus.c >> @@ -86,6 +86,16 @@ static int exynos_bus_get_event(struct exynos_bus *bus, >> return ret; >> } >> +static struct devfreq *exynos_bus_get_parent_devfreq(struct device_node *np) >> +{ >> + struct device_node *node = of_parse_phandle(np, "devfreq", 0); >> + >> + if (!node) >> + return ERR_PTR(-ENODEV); >> + >> + return devfreq_get_devfreq_by_node(node); >> +} >> + >> /* >> * devfreq function for both simple-ondemand and passive governor >> */ >> @@ -353,7 +363,7 @@ static int exynos_bus_profile_init_passive(struct exynos_bus *bus, >> profile->exit = exynos_bus_passive_exit; >> /* Get the instance of parent devfreq device */ >> - parent_devfreq = devfreq_get_devfreq_by_phandle(dev, 0); >> + parent_devfreq = exynos_bus_get_parent_devfreq(dev->of_node); >> if (IS_ERR(parent_devfreq)) >> return -EPROBE_DEFER; >> > > These changes won't apply, probably I need some base for it. I developed it on devfreq-next branch[1] [1] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-next And I try to apply these patchset to linux-next[2] with tags/next-20200109. But, patch10/11 of deviceetree has some merge conflict because patch[3] related to exynos-bus was merged. [2] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/log/ [3] https://patchwork.kernel.org/cover/11303235/ - [v2,0/2] Exynos5422: fix bus related OPPs for Odroid XU3/XU4/HC1 On next version, I'll rebase it on latest patches. > > Regards, > Lukasz > > >
On 1/9/20 10:54 AM, Chanwoo Choi wrote: > On 1/9/20 7:37 PM, Lukasz Luba wrote: >> Hi Chanwoo, >> >> On 12/20/19 12:24 AM, Chanwoo Choi wrote: >>> Previously, devfreq core support 'devfreq' property in order to get >>> the devfreq device by phandle. But, 'devfreq' property name is not proper >>> on devicetree binding because this name doesn't mean the any h/w attribute. >>> >>> The devfreq core hand over the right to decide the property name >>> for getting the devfreq device on devicetree. Each devfreq driver >>> will decide the property name on devicetree binding and then get >>> the devfreq device by using devfreq_get_devfreq_by_node(). >>> >>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com> >>> --- >>> drivers/devfreq/devfreq.c | 35 ----------------------------------- >>> drivers/devfreq/exynos-bus.c | 12 +++++++++++- >>> include/linux/devfreq.h | 8 -------- >>> 3 files changed, 11 insertions(+), 44 deletions(-) >>> >>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c >>> index cb8ca81c8973..c3d3c7c802a0 100644 >>> --- a/drivers/devfreq/devfreq.c >>> +++ b/drivers/devfreq/devfreq.c >>> @@ -991,48 +991,13 @@ struct devfreq *devfreq_get_devfreq_by_node(struct device_node *node) >>> return ERR_PTR(-ENODEV); >>> } >>> - >>> -/* >>> - * devfreq_get_devfreq_by_phandle - Get the devfreq device from devicetree >>> - * @dev - instance to the given device >>> - * @index - index into list of devfreq >>> - * >>> - * return the instance of devfreq device >>> - */ >>> -struct devfreq *devfreq_get_devfreq_by_phandle(struct device *dev, int index) >>> -{ >>> - struct device_node *node; >>> - struct devfreq *devfreq; >>> - >>> - if (!dev) >>> - return ERR_PTR(-EINVAL); >>> - >>> - if (!dev->of_node) >>> - return ERR_PTR(-EINVAL); >>> - >>> - node = of_parse_phandle(dev->of_node, "devfreq", index); >>> - if (!node) >>> - return ERR_PTR(-ENODEV); >>> - >>> - devfreq = devfreq_get_devfreq_by_node(node); >>> - of_node_put(node); >>> - >>> - return devfreq; >>> -} >>> - >>> #else >>> struct devfreq *devfreq_get_devfreq_by_node(struct device_node *node) >>> { >>> return ERR_PTR(-ENODEV); >>> } >>> - >>> -struct devfreq *devfreq_get_devfreq_by_phandle(struct device *dev, int index) >>> -{ >>> - return ERR_PTR(-ENODEV); >>> -} >>> #endif /* CONFIG_OF */ >>> EXPORT_SYMBOL_GPL(devfreq_get_devfreq_by_node); >>> -EXPORT_SYMBOL_GPL(devfreq_get_devfreq_by_phandle); >>> /** >>> * devm_devfreq_remove_device() - Resource-managed devfreq_remove_device() >>> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c >>> index 7f5917d59072..1bc4e3c81115 100644 >>> --- a/drivers/devfreq/exynos-bus.c >>> +++ b/drivers/devfreq/exynos-bus.c >>> @@ -86,6 +86,16 @@ static int exynos_bus_get_event(struct exynos_bus *bus, >>> return ret; >>> } >>> +static struct devfreq *exynos_bus_get_parent_devfreq(struct device_node *np) >>> +{ >>> + struct device_node *node = of_parse_phandle(np, "devfreq", 0); >>> + >>> + if (!node) >>> + return ERR_PTR(-ENODEV); >>> + >>> + return devfreq_get_devfreq_by_node(node); >>> +} >>> + >>> /* >>> * devfreq function for both simple-ondemand and passive governor >>> */ >>> @@ -353,7 +363,7 @@ static int exynos_bus_profile_init_passive(struct exynos_bus *bus, >>> profile->exit = exynos_bus_passive_exit; >>> /* Get the instance of parent devfreq device */ >>> - parent_devfreq = devfreq_get_devfreq_by_phandle(dev, 0); >>> + parent_devfreq = exynos_bus_get_parent_devfreq(dev->of_node); >>> if (IS_ERR(parent_devfreq)) >>> return -EPROBE_DEFER; >>> >> >> These changes won't apply, probably I need some base for it. > > I developed it on devfreq-next branch[1] > [1] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-next > > And I try to apply these patchset to linux-next[2] with tags/next-20200109. > But, patch10/11 of deviceetree has some merge conflict > because patch[3] related to exynos-bus was merged. > [2] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/log/ > [3] https://patchwork.kernel.org/cover/11303235/ > - [v2,0/2] Exynos5422: fix bus related OPPs for Odroid XU3/XU4/HC1 > > On next version, I'll rebase it on latest patches. Thank you for the information. I will update the base and continue the review. Lukasz
From: Chanwoo Choi <cw00.choi@samsun.com> The devfreq and devfreq-event subsystem provided the following two properties: - Provide 'devfreq' property in order to get the parent devfreq device by devfreq_get_devfreq_by_phandle() if devfreq device use passive governor. - Provide 'devfreq-events' property in order to get the devfreq-event device by devfreq_event_get_edev_by_phandle(). But, two properties name is not proper expressing the h/w and 'devfreq' word is name of linux subsystem instead of any h/w name. Hand over the rights for deciding the property name for getting the devfreq/devfreq-event device on devicetree, to each devfreq driver. So, replace 'devfreq' and 'devfreq-events' property with following property name according to each devfreq driver: -------------------------------------------------------------------- Old property | New property | Device driver name | -------------------------------------------------------------------- devfreq | exynos,parent-bus | exynos-bus.c | | | | devfreq-events| exynos,ppmu-device | exynos-bus.c, exynos5422-dmc.c| | rockchip,dfi-device| rk3399_dmc.c | -------------------------------------------------------------------- Changes from v1: - Edit function name by removing '_by_node' postfix. - Split out dt-binding patch to make it the separte patch.j - Add Lukasz's tag for exynos5422-dmc Chanwoo Choi (10): PM / devfreq: Remove devfreq_get_devfreq_by_phandle function PM / devfreq: event: Add devfreq_event_get_edev_by_node function dt-bindings: devfreq: exynos-bus: Replace deprecated 'devfreq' and 'devfreq-events' property dt-bindings: devfreq: rk3399_dmc: Replace deprecated 'devfreq-events' property dt-bindings: memory: exynos5422-dmc: Replace the deprecated 'devfreq-events' property PM / devfreq: exynos-bus: Replace the deprecated 'devfreq' and 'devfreq-events' property PM / devfreq: rk3399_dmc: Replace the deprecated 'devfreq-events' property memory: samsung: exynos5422-dmc: Replace the deprecated 'devfreq-events' property ARM: dts: exynos: Replace deprecated property for Exynos bus and DMC arm64: dts: exynos: Replace deprecated property for Exynos bus on TM2 Leonard Crestez (1): PM / devfreq: Add devfreq_get_devfreq_by_node function .../bindings/devfreq/exynos-bus.txt | 22 +++---- .../bindings/devfreq/rk3399_dmc.txt | 4 +- .../memory-controllers/exynos5422-dmc.txt | 8 +-- arch/arm/boot/dts/exynos3250-monk.dts | 2 +- arch/arm/boot/dts/exynos3250-rinato.dts | 18 +++--- .../boot/dts/exynos4412-itop-scp-core.dtsi | 16 ++--- arch/arm/boot/dts/exynos4412-midas.dtsi | 18 +++--- .../boot/dts/exynos4412-odroid-common.dtsi | 18 +++--- arch/arm/boot/dts/exynos5422-odroid-core.dtsi | 34 +++++------ .../dts/exynos/exynos5433-tm2-common.dtsi | 20 +++---- drivers/devfreq/devfreq-event.c | 53 +++-------------- drivers/devfreq/devfreq.c | 25 +++----- drivers/devfreq/exynos-bus.c | 58 ++++++++++++++++--- drivers/devfreq/rk3399_dmc.c | 16 ++++- drivers/memory/samsung/exynos5422-dmc.c | 37 ++++++++++-- include/linux/devfreq-event.h | 14 ++--- include/linux/devfreq.h | 6 +- 17 files changed, 198 insertions(+), 171 deletions(-)