linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Revert "PM / devfreq: Modify the device name as devfreq(X) for sysfs"
@ 2020-02-20 17:37 Orson Zhai
  2020-02-20 19:15 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 10+ messages in thread
From: Orson Zhai @ 2020-02-20 17:37 UTC (permalink / raw)
  To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi
  Cc: Greg Kroah-Hartman, John Stultz, mingmin.ling, orsonzhai,
	jingchao.ye, linux-pm, linux-kernel, stable, Orson Zhai

This reverts commit 4585fbcb5331fc910b7e553ad3efd0dd7b320d14.

The name changing as devfreq(X) breaks some user space applications,
such as Android HAL from Unisoc and Hikey [1].
The device name will be changed unexpectly after every boot depending
on module init sequence. It will make trouble to setup some system
configuration like selinux for Android.

So we'd like to revert it back to old naming rule before any better
way being found.

[1] https://lkml.org/lkml/2018/5/8/1042

Cc: John Stultz <john.stultz@linaro.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: stable@vger.kernel.org
Signed-off-by: Orson Zhai <orson.unisoc@gmail.com>

---
 drivers/devfreq/devfreq.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index cceee8b..7dcf209 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -738,7 +738,6 @@ struct devfreq *devfreq_add_device(struct device *dev,
 {
 	struct devfreq *devfreq;
 	struct devfreq_governor *governor;
-	static atomic_t devfreq_no = ATOMIC_INIT(-1);
 	int err = 0;
 
 	if (!dev || !profile || !governor_name) {
@@ -800,8 +799,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
 	devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
 	atomic_set(&devfreq->suspend_count, 0);
 
-	dev_set_name(&devfreq->dev, "devfreq%d",
-				atomic_inc_return(&devfreq_no));
+	dev_set_name(&devfreq->dev, "%s", dev_name(dev));
 	err = device_register(&devfreq->dev);
 	if (err) {
 		mutex_unlock(&devfreq->lock);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] Revert "PM / devfreq: Modify the device name as devfreq(X) for sysfs"
  2020-02-20 17:37 [PATCH] Revert "PM / devfreq: Modify the device name as devfreq(X) for sysfs" Orson Zhai
@ 2020-02-20 19:15 ` Greg Kroah-Hartman
  2020-02-20 19:47   ` John Stultz
  0 siblings, 1 reply; 10+ messages in thread
From: Greg Kroah-Hartman @ 2020-02-20 19:15 UTC (permalink / raw)
  To: Orson Zhai
  Cc: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, John Stultz,
	mingmin.ling, orsonzhai, jingchao.ye, linux-pm, linux-kernel,
	stable

On Fri, Feb 21, 2020 at 01:37:04AM +0800, Orson Zhai wrote:
> This reverts commit 4585fbcb5331fc910b7e553ad3efd0dd7b320d14.
> 
> The name changing as devfreq(X) breaks some user space applications,
> such as Android HAL from Unisoc and Hikey [1].
> The device name will be changed unexpectly after every boot depending
> on module init sequence. It will make trouble to setup some system
> configuration like selinux for Android.
> 
> So we'd like to revert it back to old naming rule before any better
> way being found.
> 
> [1] https://lkml.org/lkml/2018/5/8/1042
> 
> Cc: John Stultz <john.stultz@linaro.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: stable@vger.kernel.org
> Signed-off-by: Orson Zhai <orson.unisoc@gmail.com>
> 
> ---
>  drivers/devfreq/devfreq.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index cceee8b..7dcf209 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -738,7 +738,6 @@ struct devfreq *devfreq_add_device(struct device *dev,
>  {
>  	struct devfreq *devfreq;
>  	struct devfreq_governor *governor;
> -	static atomic_t devfreq_no = ATOMIC_INIT(-1);
>  	int err = 0;
>  
>  	if (!dev || !profile || !governor_name) {
> @@ -800,8 +799,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
>  	devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
>  	atomic_set(&devfreq->suspend_count, 0);
>  
> -	dev_set_name(&devfreq->dev, "devfreq%d",
> -				atomic_inc_return(&devfreq_no));
> +	dev_set_name(&devfreq->dev, "%s", dev_name(dev));
>  	err = device_register(&devfreq->dev);
>  	if (err) {
>  		mutex_unlock(&devfreq->lock);
> -- 
> 2.7.4
> 

Thanks for this, I agree, this needs to get back to the way things were
as it seems to break too many existing systems as-is.

I'll queue this up in my tree now, thanks.

greg k-h

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Revert "PM / devfreq: Modify the device name as devfreq(X) for sysfs"
  2020-02-20 19:15 ` Greg Kroah-Hartman
@ 2020-02-20 19:47   ` John Stultz
  2020-02-20 20:01     ` Greg Kroah-Hartman
  2020-02-21  7:06     ` Greg Kroah-Hartman
  0 siblings, 2 replies; 10+ messages in thread
From: John Stultz @ 2020-02-20 19:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Orson Zhai, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
	mingmin.ling, orsonzhai, jingchao.ye, Linux PM list, lkml,
	stable

On Thu, Feb 20, 2020 at 11:15 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Fri, Feb 21, 2020 at 01:37:04AM +0800, Orson Zhai wrote:
> > This reverts commit 4585fbcb5331fc910b7e553ad3efd0dd7b320d14.
> >
> > The name changing as devfreq(X) breaks some user space applications,
> > such as Android HAL from Unisoc and Hikey [1].
> > The device name will be changed unexpectly after every boot depending
> > on module init sequence. It will make trouble to setup some system
> > configuration like selinux for Android.
> >
> > So we'd like to revert it back to old naming rule before any better
> > way being found.
> >
> > [1] https://lkml.org/lkml/2018/5/8/1042
> >
> > Cc: John Stultz <john.stultz@linaro.org>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Orson Zhai <orson.unisoc@gmail.com>
> >
> > ---
> >  drivers/devfreq/devfreq.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> > index cceee8b..7dcf209 100644
> > --- a/drivers/devfreq/devfreq.c
> > +++ b/drivers/devfreq/devfreq.c
> > @@ -738,7 +738,6 @@ struct devfreq *devfreq_add_device(struct device *dev,
> >  {
> >       struct devfreq *devfreq;
> >       struct devfreq_governor *governor;
> > -     static atomic_t devfreq_no = ATOMIC_INIT(-1);
> >       int err = 0;
> >
> >       if (!dev || !profile || !governor_name) {
> > @@ -800,8 +799,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
> >       devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
> >       atomic_set(&devfreq->suspend_count, 0);
> >
> > -     dev_set_name(&devfreq->dev, "devfreq%d",
> > -                             atomic_inc_return(&devfreq_no));
> > +     dev_set_name(&devfreq->dev, "%s", dev_name(dev));
> >       err = device_register(&devfreq->dev);
> >       if (err) {
> >               mutex_unlock(&devfreq->lock);
> > --
> > 2.7.4
> >
>
> Thanks for this, I agree, this needs to get back to the way things were
> as it seems to break too many existing systems as-is.
>
> I'll queue this up in my tree now, thanks.

Oof this old thing. I unfortunately didn't get back to look at the
devfreq name node issue or the compatibility links, since the impact
of the regression (breaking the powerHAL's interactions with the gpu)
wasn't as big as other problems we had. While the regression was
frustrating, my only hesitancy at this point is that its been this way
since 4.10, so reverting the problematic patch is likely to break any
new users since then.

thanks
-john

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Revert "PM / devfreq: Modify the device name as devfreq(X) for sysfs"
  2020-02-20 19:47   ` John Stultz
@ 2020-02-20 20:01     ` Greg Kroah-Hartman
  2020-02-21  7:06     ` Greg Kroah-Hartman
  1 sibling, 0 replies; 10+ messages in thread
From: Greg Kroah-Hartman @ 2020-02-20 20:01 UTC (permalink / raw)
  To: John Stultz
  Cc: Orson Zhai, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
	mingmin.ling, orsonzhai, jingchao.ye, Linux PM list, lkml,
	stable

On Thu, Feb 20, 2020 at 11:47:41AM -0800, John Stultz wrote:
> On Thu, Feb 20, 2020 at 11:15 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Fri, Feb 21, 2020 at 01:37:04AM +0800, Orson Zhai wrote:
> > > This reverts commit 4585fbcb5331fc910b7e553ad3efd0dd7b320d14.
> > >
> > > The name changing as devfreq(X) breaks some user space applications,
> > > such as Android HAL from Unisoc and Hikey [1].
> > > The device name will be changed unexpectly after every boot depending
> > > on module init sequence. It will make trouble to setup some system
> > > configuration like selinux for Android.
> > >
> > > So we'd like to revert it back to old naming rule before any better
> > > way being found.
> > >
> > > [1] https://lkml.org/lkml/2018/5/8/1042
> > >
> > > Cc: John Stultz <john.stultz@linaro.org>
> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Orson Zhai <orson.unisoc@gmail.com>
> > >
> > > ---
> > >  drivers/devfreq/devfreq.c | 4 +---
> > >  1 file changed, 1 insertion(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> > > index cceee8b..7dcf209 100644
> > > --- a/drivers/devfreq/devfreq.c
> > > +++ b/drivers/devfreq/devfreq.c
> > > @@ -738,7 +738,6 @@ struct devfreq *devfreq_add_device(struct device *dev,
> > >  {
> > >       struct devfreq *devfreq;
> > >       struct devfreq_governor *governor;
> > > -     static atomic_t devfreq_no = ATOMIC_INIT(-1);
> > >       int err = 0;
> > >
> > >       if (!dev || !profile || !governor_name) {
> > > @@ -800,8 +799,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
> > >       devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
> > >       atomic_set(&devfreq->suspend_count, 0);
> > >
> > > -     dev_set_name(&devfreq->dev, "devfreq%d",
> > > -                             atomic_inc_return(&devfreq_no));
> > > +     dev_set_name(&devfreq->dev, "%s", dev_name(dev));
> > >       err = device_register(&devfreq->dev);
> > >       if (err) {
> > >               mutex_unlock(&devfreq->lock);
> > > --
> > > 2.7.4
> > >
> >
> > Thanks for this, I agree, this needs to get back to the way things were
> > as it seems to break too many existing systems as-is.
> >
> > I'll queue this up in my tree now, thanks.
> 
> Oof this old thing. I unfortunately didn't get back to look at the
> devfreq name node issue or the compatibility links, since the impact
> of the regression (breaking the powerHAL's interactions with the gpu)
> wasn't as big as other problems we had. While the regression was
> frustrating, my only hesitancy at this point is that its been this way
> since 4.10, so reverting the problematic patch is likely to break any
> new users since then.

4.11 :)


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Revert "PM / devfreq: Modify the device name as devfreq(X) for sysfs"
  2020-02-20 19:47   ` John Stultz
  2020-02-20 20:01     ` Greg Kroah-Hartman
@ 2020-02-21  7:06     ` Greg Kroah-Hartman
  2020-02-21  8:11       ` Chanwoo Choi
  1 sibling, 1 reply; 10+ messages in thread
From: Greg Kroah-Hartman @ 2020-02-21  7:06 UTC (permalink / raw)
  To: John Stultz
  Cc: Orson Zhai, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
	mingmin.ling, orsonzhai, jingchao.ye, Linux PM list, lkml,
	stable

On Thu, Feb 20, 2020 at 11:47:41AM -0800, John Stultz wrote:
> On Thu, Feb 20, 2020 at 11:15 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Fri, Feb 21, 2020 at 01:37:04AM +0800, Orson Zhai wrote:
> > > This reverts commit 4585fbcb5331fc910b7e553ad3efd0dd7b320d14.
> > >
> > > The name changing as devfreq(X) breaks some user space applications,
> > > such as Android HAL from Unisoc and Hikey [1].
> > > The device name will be changed unexpectly after every boot depending
> > > on module init sequence. It will make trouble to setup some system
> > > configuration like selinux for Android.
> > >
> > > So we'd like to revert it back to old naming rule before any better
> > > way being found.
> > >
> > > [1] https://lkml.org/lkml/2018/5/8/1042
> > >
> > > Cc: John Stultz <john.stultz@linaro.org>
> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Orson Zhai <orson.unisoc@gmail.com>
> > >
> > > ---
> > >  drivers/devfreq/devfreq.c | 4 +---
> > >  1 file changed, 1 insertion(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> > > index cceee8b..7dcf209 100644
> > > --- a/drivers/devfreq/devfreq.c
> > > +++ b/drivers/devfreq/devfreq.c
> > > @@ -738,7 +738,6 @@ struct devfreq *devfreq_add_device(struct device *dev,
> > >  {
> > >       struct devfreq *devfreq;
> > >       struct devfreq_governor *governor;
> > > -     static atomic_t devfreq_no = ATOMIC_INIT(-1);
> > >       int err = 0;
> > >
> > >       if (!dev || !profile || !governor_name) {
> > > @@ -800,8 +799,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
> > >       devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
> > >       atomic_set(&devfreq->suspend_count, 0);
> > >
> > > -     dev_set_name(&devfreq->dev, "devfreq%d",
> > > -                             atomic_inc_return(&devfreq_no));
> > > +     dev_set_name(&devfreq->dev, "%s", dev_name(dev));
> > >       err = device_register(&devfreq->dev);
> > >       if (err) {
> > >               mutex_unlock(&devfreq->lock);
> > > --
> > > 2.7.4
> > >
> >
> > Thanks for this, I agree, this needs to get back to the way things were
> > as it seems to break too many existing systems as-is.
> >
> > I'll queue this up in my tree now, thanks.
> 
> Oof this old thing. I unfortunately didn't get back to look at the
> devfreq name node issue or the compatibility links, since the impact
> of the regression (breaking the powerHAL's interactions with the gpu)
> wasn't as big as other problems we had. While the regression was
> frustrating, my only hesitancy at this point is that its been this way
> since 4.10, so reverting the problematic patch is likely to break any
> new users since then.

Looks like most users just revert that commit in their trees:
	https://source.codeaurora.org/quic/la/kernel/msm-4.14/commit/drivers/devfreq?h=msm-4.14&id=ccf273f6d89ad0fa8032e9225305ad6f62c7770c

So we should be ok here.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Revert "PM / devfreq: Modify the device name as devfreq(X) for sysfs"
  2020-02-21  7:06     ` Greg Kroah-Hartman
@ 2020-02-21  8:11       ` Chanwoo Choi
  2020-02-21 11:13         ` Greg Kroah-Hartman
  0 siblings, 1 reply; 10+ messages in thread
From: Chanwoo Choi @ 2020-02-21  8:11 UTC (permalink / raw)
  To: Greg Kroah-Hartman, John Stultz
  Cc: Orson Zhai, MyungJoo Ham, Kyungmin Park, mingmin.ling, orsonzhai,
	jingchao.ye, Linux PM list, lkml, stable

On 2/21/20 4:06 PM, Greg Kroah-Hartman wrote:
> On Thu, Feb 20, 2020 at 11:47:41AM -0800, John Stultz wrote:
>> On Thu, Feb 20, 2020 at 11:15 AM Greg Kroah-Hartman
>> <gregkh@linuxfoundation.org> wrote:
>>>
>>> On Fri, Feb 21, 2020 at 01:37:04AM +0800, Orson Zhai wrote:
>>>> This reverts commit 4585fbcb5331fc910b7e553ad3efd0dd7b320d14.
>>>>
>>>> The name changing as devfreq(X) breaks some user space applications,
>>>> such as Android HAL from Unisoc and Hikey [1].
>>>> The device name will be changed unexpectly after every boot depending
>>>> on module init sequence. It will make trouble to setup some system
>>>> configuration like selinux for Android.
>>>>
>>>> So we'd like to revert it back to old naming rule before any better
>>>> way being found.
>>>>
>>>> [1] https://protect2.fireeye.com/url?k=00fa721e-5d2a7af6-00fbf951-000babff32e3-95e4b92259b05656&u=https://lkml.org/lkml/2018/5/8/1042
>>>>
>>>> Cc: John Stultz <john.stultz@linaro.org>
>>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>>> Cc: stable@vger.kernel.org
>>>> Signed-off-by: Orson Zhai <orson.unisoc@gmail.com>
>>>>
>>>> ---
>>>>  drivers/devfreq/devfreq.c | 4 +---
>>>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>>>> index cceee8b..7dcf209 100644
>>>> --- a/drivers/devfreq/devfreq.c
>>>> +++ b/drivers/devfreq/devfreq.c
>>>> @@ -738,7 +738,6 @@ struct devfreq *devfreq_add_device(struct device *dev,
>>>>  {
>>>>       struct devfreq *devfreq;
>>>>       struct devfreq_governor *governor;
>>>> -     static atomic_t devfreq_no = ATOMIC_INIT(-1);
>>>>       int err = 0;
>>>>
>>>>       if (!dev || !profile || !governor_name) {
>>>> @@ -800,8 +799,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
>>>>       devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
>>>>       atomic_set(&devfreq->suspend_count, 0);
>>>>
>>>> -     dev_set_name(&devfreq->dev, "devfreq%d",
>>>> -                             atomic_inc_return(&devfreq_no));
>>>> +     dev_set_name(&devfreq->dev, "%s", dev_name(dev));
>>>>       err = device_register(&devfreq->dev);
>>>>       if (err) {
>>>>               mutex_unlock(&devfreq->lock);
>>>> --
>>>> 2.7.4
>>>>
>>>
>>> Thanks for this, I agree, this needs to get back to the way things were
>>> as it seems to break too many existing systems as-is.
>>>
>>> I'll queue this up in my tree now, thanks.
>>
>> Oof this old thing. I unfortunately didn't get back to look at the
>> devfreq name node issue or the compatibility links, since the impact
>> of the regression (breaking the powerHAL's interactions with the gpu)
>> wasn't as big as other problems we had. While the regression was
>> frustrating, my only hesitancy at this point is that its been this way
>> since 4.10, so reverting the problematic patch is likely to break any
>> new users since then.
> 
> Looks like most users just revert that commit in their trees:
> 	https://protect2.fireeye.com/url?k=1012ad0f-4dc2a5e7-10132640-000babff32e3-35779c5ed675ef0f&u=https://source.codeaurora.org/quic/la/kernel/msm-4.14/commit/drivers/devfreq?h=msm-4.14&id=ccf273f6d89ad0fa8032e9225305ad6f62c7770c
> 
> So we should be ok here.

I'm sorry about changing the devfreq node name.

OK. Do you pick this patch to your tree?
or If not, I'll apply it to devfreq-next branch for v5.7-rc1.

And do you apply it to kernel of linux-stable tree since 4.11?

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Revert "PM / devfreq: Modify the device name as devfreq(X) for sysfs"
  2020-02-21  8:11       ` Chanwoo Choi
@ 2020-02-21 11:13         ` Greg Kroah-Hartman
  2020-02-21 23:55           ` Chanwoo Choi
  0 siblings, 1 reply; 10+ messages in thread
From: Greg Kroah-Hartman @ 2020-02-21 11:13 UTC (permalink / raw)
  To: Chanwoo Choi
  Cc: John Stultz, Orson Zhai, MyungJoo Ham, Kyungmin Park,
	mingmin.ling, orsonzhai, jingchao.ye, Linux PM list, lkml,
	stable

On Fri, Feb 21, 2020 at 05:11:02PM +0900, Chanwoo Choi wrote:
> On 2/21/20 4:06 PM, Greg Kroah-Hartman wrote:
> > On Thu, Feb 20, 2020 at 11:47:41AM -0800, John Stultz wrote:
> >> On Thu, Feb 20, 2020 at 11:15 AM Greg Kroah-Hartman
> >> <gregkh@linuxfoundation.org> wrote:
> >>>
> >>> On Fri, Feb 21, 2020 at 01:37:04AM +0800, Orson Zhai wrote:
> >>>> This reverts commit 4585fbcb5331fc910b7e553ad3efd0dd7b320d14.
> >>>>
> >>>> The name changing as devfreq(X) breaks some user space applications,
> >>>> such as Android HAL from Unisoc and Hikey [1].
> >>>> The device name will be changed unexpectly after every boot depending
> >>>> on module init sequence. It will make trouble to setup some system
> >>>> configuration like selinux for Android.
> >>>>
> >>>> So we'd like to revert it back to old naming rule before any better
> >>>> way being found.
> >>>>
> >>>> [1] https://protect2.fireeye.com/url?k=00fa721e-5d2a7af6-00fbf951-000babff32e3-95e4b92259b05656&u=https://lkml.org/lkml/2018/5/8/1042
> >>>>
> >>>> Cc: John Stultz <john.stultz@linaro.org>
> >>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >>>> Cc: stable@vger.kernel.org
> >>>> Signed-off-by: Orson Zhai <orson.unisoc@gmail.com>
> >>>>
> >>>> ---
> >>>>  drivers/devfreq/devfreq.c | 4 +---
> >>>>  1 file changed, 1 insertion(+), 3 deletions(-)
> >>>>
> >>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> >>>> index cceee8b..7dcf209 100644
> >>>> --- a/drivers/devfreq/devfreq.c
> >>>> +++ b/drivers/devfreq/devfreq.c
> >>>> @@ -738,7 +738,6 @@ struct devfreq *devfreq_add_device(struct device *dev,
> >>>>  {
> >>>>       struct devfreq *devfreq;
> >>>>       struct devfreq_governor *governor;
> >>>> -     static atomic_t devfreq_no = ATOMIC_INIT(-1);
> >>>>       int err = 0;
> >>>>
> >>>>       if (!dev || !profile || !governor_name) {
> >>>> @@ -800,8 +799,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
> >>>>       devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
> >>>>       atomic_set(&devfreq->suspend_count, 0);
> >>>>
> >>>> -     dev_set_name(&devfreq->dev, "devfreq%d",
> >>>> -                             atomic_inc_return(&devfreq_no));
> >>>> +     dev_set_name(&devfreq->dev, "%s", dev_name(dev));
> >>>>       err = device_register(&devfreq->dev);
> >>>>       if (err) {
> >>>>               mutex_unlock(&devfreq->lock);
> >>>> --
> >>>> 2.7.4
> >>>>
> >>>
> >>> Thanks for this, I agree, this needs to get back to the way things were
> >>> as it seems to break too many existing systems as-is.
> >>>
> >>> I'll queue this up in my tree now, thanks.
> >>
> >> Oof this old thing. I unfortunately didn't get back to look at the
> >> devfreq name node issue or the compatibility links, since the impact
> >> of the regression (breaking the powerHAL's interactions with the gpu)
> >> wasn't as big as other problems we had. While the regression was
> >> frustrating, my only hesitancy at this point is that its been this way
> >> since 4.10, so reverting the problematic patch is likely to break any
> >> new users since then.
> > 
> > Looks like most users just revert that commit in their trees:
> > 	https://protect2.fireeye.com/url?k=1012ad0f-4dc2a5e7-10132640-000babff32e3-35779c5ed675ef0f&u=https://source.codeaurora.org/quic/la/kernel/msm-4.14/commit/drivers/devfreq?h=msm-4.14&id=ccf273f6d89ad0fa8032e9225305ad6f62c7770c
> > 
> > So we should be ok here.
> 
> I'm sorry about changing the devfreq node name.
> 
> OK. Do you pick this patch to your tree?

Yes, I can do that.

> or If not, I'll apply it to devfreq-next branch for v5.7-rc1.
> 
> And do you apply it to kernel of linux-stable tree since 4.11?

Yeah, I'll mark it for stable.

Can I get an ack from you for this?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Revert "PM / devfreq: Modify the device name as devfreq(X) for sysfs"
  2020-02-21 11:13         ` Greg Kroah-Hartman
@ 2020-02-21 23:55           ` Chanwoo Choi
  2020-02-23 17:54             ` Greg Kroah-Hartman
  0 siblings, 1 reply; 10+ messages in thread
From: Chanwoo Choi @ 2020-02-21 23:55 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Chanwoo Choi, John Stultz, Orson Zhai, MyungJoo Ham,
	Kyungmin Park, mingmin.ling, orsonzhai, jingchao.ye,
	Linux PM list, lkml, stable

On Fri, Feb 21, 2020 at 8:13 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Fri, Feb 21, 2020 at 05:11:02PM +0900, Chanwoo Choi wrote:
> > On 2/21/20 4:06 PM, Greg Kroah-Hartman wrote:
> > > On Thu, Feb 20, 2020 at 11:47:41AM -0800, John Stultz wrote:
> > >> On Thu, Feb 20, 2020 at 11:15 AM Greg Kroah-Hartman
> > >> <gregkh@linuxfoundation.org> wrote:
> > >>>
> > >>> On Fri, Feb 21, 2020 at 01:37:04AM +0800, Orson Zhai wrote:
> > >>>> This reverts commit 4585fbcb5331fc910b7e553ad3efd0dd7b320d14.
> > >>>>
> > >>>> The name changing as devfreq(X) breaks some user space applications,
> > >>>> such as Android HAL from Unisoc and Hikey [1].
> > >>>> The device name will be changed unexpectly after every boot depending
> > >>>> on module init sequence. It will make trouble to setup some system
> > >>>> configuration like selinux for Android.
> > >>>>
> > >>>> So we'd like to revert it back to old naming rule before any better
> > >>>> way being found.
> > >>>>
> > >>>> [1] https://protect2.fireeye.com/url?k=00fa721e-5d2a7af6-00fbf951-000babff32e3-95e4b92259b05656&u=https://lkml.org/lkml/2018/5/8/1042
> > >>>>
> > >>>> Cc: John Stultz <john.stultz@linaro.org>
> > >>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > >>>> Cc: stable@vger.kernel.org
> > >>>> Signed-off-by: Orson Zhai <orson.unisoc@gmail.com>
> > >>>>
> > >>>> ---
> > >>>>  drivers/devfreq/devfreq.c | 4 +---
> > >>>>  1 file changed, 1 insertion(+), 3 deletions(-)
> > >>>>
> > >>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> > >>>> index cceee8b..7dcf209 100644
> > >>>> --- a/drivers/devfreq/devfreq.c
> > >>>> +++ b/drivers/devfreq/devfreq.c
> > >>>> @@ -738,7 +738,6 @@ struct devfreq *devfreq_add_device(struct device *dev,
> > >>>>  {
> > >>>>       struct devfreq *devfreq;
> > >>>>       struct devfreq_governor *governor;
> > >>>> -     static atomic_t devfreq_no = ATOMIC_INIT(-1);
> > >>>>       int err = 0;
> > >>>>
> > >>>>       if (!dev || !profile || !governor_name) {
> > >>>> @@ -800,8 +799,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
> > >>>>       devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
> > >>>>       atomic_set(&devfreq->suspend_count, 0);
> > >>>>
> > >>>> -     dev_set_name(&devfreq->dev, "devfreq%d",
> > >>>> -                             atomic_inc_return(&devfreq_no));
> > >>>> +     dev_set_name(&devfreq->dev, "%s", dev_name(dev));
> > >>>>       err = device_register(&devfreq->dev);
> > >>>>       if (err) {
> > >>>>               mutex_unlock(&devfreq->lock);
> > >>>> --
> > >>>> 2.7.4
> > >>>>
> > >>>
> > >>> Thanks for this, I agree, this needs to get back to the way things were
> > >>> as it seems to break too many existing systems as-is.
> > >>>
> > >>> I'll queue this up in my tree now, thanks.
> > >>
> > >> Oof this old thing. I unfortunately didn't get back to look at the
> > >> devfreq name node issue or the compatibility links, since the impact
> > >> of the regression (breaking the powerHAL's interactions with the gpu)
> > >> wasn't as big as other problems we had. While the regression was
> > >> frustrating, my only hesitancy at this point is that its been this way
> > >> since 4.10, so reverting the problematic patch is likely to break any
> > >> new users since then.
> > >
> > > Looks like most users just revert that commit in their trees:
> > >     https://protect2.fireeye.com/url?k=1012ad0f-4dc2a5e7-10132640-000babff32e3-35779c5ed675ef0f&u=https://source.codeaurora.org/quic/la/kernel/msm-4.14/commit/drivers/devfreq?h=msm-4.14&id=ccf273f6d89ad0fa8032e9225305ad6f62c7770c
> > >
> > > So we should be ok here.
> >
> > I'm sorry about changing the devfreq node name.
> >
> > OK. Do you pick this patch to your tree?
>
> Yes, I can do that.

If you agree, how about merging it to devfreq.git
for preventing the potential merge conflict with other devfreq patches?

>
> > or If not, I'll apply it to devfreq-next branch for v5.7-rc1.
> >
> > And do you apply it to kernel of linux-stable tree since 4.11?
>
> Yeah, I'll mark it for stable.

Thanks.

>
> Can I get an ack from you for this?

OK. but, if it will be merged to devfreq.git,
can I get an ack from you?

>
> thanks,
>
> greg k-h



-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Revert "PM / devfreq: Modify the device name as devfreq(X) for sysfs"
  2020-02-21 23:55           ` Chanwoo Choi
@ 2020-02-23 17:54             ` Greg Kroah-Hartman
  2020-02-24  2:17               ` Chanwoo Choi
  0 siblings, 1 reply; 10+ messages in thread
From: Greg Kroah-Hartman @ 2020-02-23 17:54 UTC (permalink / raw)
  To: Chanwoo Choi
  Cc: Chanwoo Choi, John Stultz, Orson Zhai, MyungJoo Ham,
	Kyungmin Park, mingmin.ling, orsonzhai, jingchao.ye,
	Linux PM list, lkml, stable

On Sat, Feb 22, 2020 at 08:55:22AM +0900, Chanwoo Choi wrote:
> On Fri, Feb 21, 2020 at 8:13 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Fri, Feb 21, 2020 at 05:11:02PM +0900, Chanwoo Choi wrote:
> > > On 2/21/20 4:06 PM, Greg Kroah-Hartman wrote:
> > > > On Thu, Feb 20, 2020 at 11:47:41AM -0800, John Stultz wrote:
> > > >> On Thu, Feb 20, 2020 at 11:15 AM Greg Kroah-Hartman
> > > >> <gregkh@linuxfoundation.org> wrote:
> > > >>>
> > > >>> On Fri, Feb 21, 2020 at 01:37:04AM +0800, Orson Zhai wrote:
> > > >>>> This reverts commit 4585fbcb5331fc910b7e553ad3efd0dd7b320d14.
> > > >>>>
> > > >>>> The name changing as devfreq(X) breaks some user space applications,
> > > >>>> such as Android HAL from Unisoc and Hikey [1].
> > > >>>> The device name will be changed unexpectly after every boot depending
> > > >>>> on module init sequence. It will make trouble to setup some system
> > > >>>> configuration like selinux for Android.
> > > >>>>
> > > >>>> So we'd like to revert it back to old naming rule before any better
> > > >>>> way being found.
> > > >>>>
> > > >>>> [1] https://protect2.fireeye.com/url?k=00fa721e-5d2a7af6-00fbf951-000babff32e3-95e4b92259b05656&u=https://lkml.org/lkml/2018/5/8/1042
> > > >>>>
> > > >>>> Cc: John Stultz <john.stultz@linaro.org>
> > > >>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > >>>> Cc: stable@vger.kernel.org
> > > >>>> Signed-off-by: Orson Zhai <orson.unisoc@gmail.com>
> > > >>>>
> > > >>>> ---
> > > >>>>  drivers/devfreq/devfreq.c | 4 +---
> > > >>>>  1 file changed, 1 insertion(+), 3 deletions(-)
> > > >>>>
> > > >>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> > > >>>> index cceee8b..7dcf209 100644
> > > >>>> --- a/drivers/devfreq/devfreq.c
> > > >>>> +++ b/drivers/devfreq/devfreq.c
> > > >>>> @@ -738,7 +738,6 @@ struct devfreq *devfreq_add_device(struct device *dev,
> > > >>>>  {
> > > >>>>       struct devfreq *devfreq;
> > > >>>>       struct devfreq_governor *governor;
> > > >>>> -     static atomic_t devfreq_no = ATOMIC_INIT(-1);
> > > >>>>       int err = 0;
> > > >>>>
> > > >>>>       if (!dev || !profile || !governor_name) {
> > > >>>> @@ -800,8 +799,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
> > > >>>>       devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
> > > >>>>       atomic_set(&devfreq->suspend_count, 0);
> > > >>>>
> > > >>>> -     dev_set_name(&devfreq->dev, "devfreq%d",
> > > >>>> -                             atomic_inc_return(&devfreq_no));
> > > >>>> +     dev_set_name(&devfreq->dev, "%s", dev_name(dev));
> > > >>>>       err = device_register(&devfreq->dev);
> > > >>>>       if (err) {
> > > >>>>               mutex_unlock(&devfreq->lock);
> > > >>>> --
> > > >>>> 2.7.4
> > > >>>>
> > > >>>
> > > >>> Thanks for this, I agree, this needs to get back to the way things were
> > > >>> as it seems to break too many existing systems as-is.
> > > >>>
> > > >>> I'll queue this up in my tree now, thanks.
> > > >>
> > > >> Oof this old thing. I unfortunately didn't get back to look at the
> > > >> devfreq name node issue or the compatibility links, since the impact
> > > >> of the regression (breaking the powerHAL's interactions with the gpu)
> > > >> wasn't as big as other problems we had. While the regression was
> > > >> frustrating, my only hesitancy at this point is that its been this way
> > > >> since 4.10, so reverting the problematic patch is likely to break any
> > > >> new users since then.
> > > >
> > > > Looks like most users just revert that commit in their trees:
> > > >     https://protect2.fireeye.com/url?k=1012ad0f-4dc2a5e7-10132640-000babff32e3-35779c5ed675ef0f&u=https://source.codeaurora.org/quic/la/kernel/msm-4.14/commit/drivers/devfreq?h=msm-4.14&id=ccf273f6d89ad0fa8032e9225305ad6f62c7770c
> > > >
> > > > So we should be ok here.
> > >
> > > I'm sorry about changing the devfreq node name.
> > >
> > > OK. Do you pick this patch to your tree?
> >
> > Yes, I can do that.
> 
> If you agree, how about merging it to devfreq.git
> for preventing the potential merge conflict with other devfreq patches?

Sure, but it should go for 5.6-final, right?

> > > or If not, I'll apply it to devfreq-next branch for v5.7-rc1.
> > >
> > > And do you apply it to kernel of linux-stable tree since 4.11?
> >
> > Yeah, I'll mark it for stable.
> 
> Thanks.
> 
> >
> > Can I get an ack from you for this?
> 
> OK. but, if it will be merged to devfreq.git,
> can I get an ack from you?

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Revert "PM / devfreq: Modify the device name as devfreq(X) for sysfs"
  2020-02-23 17:54             ` Greg Kroah-Hartman
@ 2020-02-24  2:17               ` Chanwoo Choi
  0 siblings, 0 replies; 10+ messages in thread
From: Chanwoo Choi @ 2020-02-24  2:17 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Chanwoo Choi
  Cc: John Stultz, Orson Zhai, MyungJoo Ham, Kyungmin Park,
	mingmin.ling, orsonzhai, jingchao.ye, Linux PM list, lkml,
	stable

On 2/24/20 2:54 AM, Greg Kroah-Hartman wrote:
> On Sat, Feb 22, 2020 at 08:55:22AM +0900, Chanwoo Choi wrote:
>> On Fri, Feb 21, 2020 at 8:13 PM Greg Kroah-Hartman
>> <gregkh@linuxfoundation.org> wrote:
>>>
>>> On Fri, Feb 21, 2020 at 05:11:02PM +0900, Chanwoo Choi wrote:
>>>> On 2/21/20 4:06 PM, Greg Kroah-Hartman wrote:
>>>>> On Thu, Feb 20, 2020 at 11:47:41AM -0800, John Stultz wrote:
>>>>>> On Thu, Feb 20, 2020 at 11:15 AM Greg Kroah-Hartman
>>>>>> <gregkh@linuxfoundation.org> wrote:
>>>>>>>
>>>>>>> On Fri, Feb 21, 2020 at 01:37:04AM +0800, Orson Zhai wrote:
>>>>>>>> This reverts commit 4585fbcb5331fc910b7e553ad3efd0dd7b320d14.
>>>>>>>>
>>>>>>>> The name changing as devfreq(X) breaks some user space applications,
>>>>>>>> such as Android HAL from Unisoc and Hikey [1].
>>>>>>>> The device name will be changed unexpectly after every boot depending
>>>>>>>> on module init sequence. It will make trouble to setup some system
>>>>>>>> configuration like selinux for Android.
>>>>>>>>
>>>>>>>> So we'd like to revert it back to old naming rule before any better
>>>>>>>> way being found.
>>>>>>>>
>>>>>>>> [1] https://protect2.fireeye.com/url?k=00fa721e-5d2a7af6-00fbf951-000babff32e3-95e4b92259b05656&u=https://lkml.org/lkml/2018/5/8/1042
>>>>>>>>
>>>>>>>> Cc: John Stultz <john.stultz@linaro.org>
>>>>>>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>>>>>>> Cc: stable@vger.kernel.org
>>>>>>>> Signed-off-by: Orson Zhai <orson.unisoc@gmail.com>
>>>>>>>>
>>>>>>>> ---
>>>>>>>>  drivers/devfreq/devfreq.c | 4 +---
>>>>>>>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>>>>>>>> index cceee8b..7dcf209 100644
>>>>>>>> --- a/drivers/devfreq/devfreq.c
>>>>>>>> +++ b/drivers/devfreq/devfreq.c
>>>>>>>> @@ -738,7 +738,6 @@ struct devfreq *devfreq_add_device(struct device *dev,
>>>>>>>>  {
>>>>>>>>       struct devfreq *devfreq;
>>>>>>>>       struct devfreq_governor *governor;
>>>>>>>> -     static atomic_t devfreq_no = ATOMIC_INIT(-1);
>>>>>>>>       int err = 0;
>>>>>>>>
>>>>>>>>       if (!dev || !profile || !governor_name) {
>>>>>>>> @@ -800,8 +799,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
>>>>>>>>       devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
>>>>>>>>       atomic_set(&devfreq->suspend_count, 0);
>>>>>>>>
>>>>>>>> -     dev_set_name(&devfreq->dev, "devfreq%d",
>>>>>>>> -                             atomic_inc_return(&devfreq_no));
>>>>>>>> +     dev_set_name(&devfreq->dev, "%s", dev_name(dev));
>>>>>>>>       err = device_register(&devfreq->dev);
>>>>>>>>       if (err) {
>>>>>>>>               mutex_unlock(&devfreq->lock);
>>>>>>>> --
>>>>>>>> 2.7.4
>>>>>>>>
>>>>>>>
>>>>>>> Thanks for this, I agree, this needs to get back to the way things were
>>>>>>> as it seems to break too many existing systems as-is.
>>>>>>>
>>>>>>> I'll queue this up in my tree now, thanks.
>>>>>>
>>>>>> Oof this old thing. I unfortunately didn't get back to look at the
>>>>>> devfreq name node issue or the compatibility links, since the impact
>>>>>> of the regression (breaking the powerHAL's interactions with the gpu)
>>>>>> wasn't as big as other problems we had. While the regression was
>>>>>> frustrating, my only hesitancy at this point is that its been this way
>>>>>> since 4.10, so reverting the problematic patch is likely to break any
>>>>>> new users since then.
>>>>>
>>>>> Looks like most users just revert that commit in their trees:
>>>>>     https://protect2.fireeye.com/url?k=1012ad0f-4dc2a5e7-10132640-000babff32e3-35779c5ed675ef0f&u=https://source.codeaurora.org/quic/la/kernel/msm-4.14/commit/drivers/devfreq?h=msm-4.14&id=ccf273f6d89ad0fa8032e9225305ad6f62c7770c
>>>>>
>>>>> So we should be ok here.
>>>>
>>>> I'm sorry about changing the devfreq node name.
>>>>
>>>> OK. Do you pick this patch to your tree?
>>>
>>> Yes, I can do that.
>>
>> If you agree, how about merging it to devfreq.git
>> for preventing the potential merge conflict with other devfreq patches?
> 
> Sure, but it should go for 5.6-final, right?

OK.I'll send pull-request of this patch for v5.6-rc4.

> 
>>>> or If not, I'll apply it to devfreq-next branch for v5.7-rc1.
>>>>
>>>> And do you apply it to kernel of linux-stable tree since 4.11?
>>>
>>> Yeah, I'll mark it for stable.
>>
>> Thanks.
>>
>>>
>>> Can I get an ack from you for this?
>>
>> OK. but, if it will be merged to devfreq.git,
>> can I get an ack from you?
> 
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Thanks. I applied it to devfreq-fixes branch for v5.6-rc4.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-02-24  2:08 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-20 17:37 [PATCH] Revert "PM / devfreq: Modify the device name as devfreq(X) for sysfs" Orson Zhai
2020-02-20 19:15 ` Greg Kroah-Hartman
2020-02-20 19:47   ` John Stultz
2020-02-20 20:01     ` Greg Kroah-Hartman
2020-02-21  7:06     ` Greg Kroah-Hartman
2020-02-21  8:11       ` Chanwoo Choi
2020-02-21 11:13         ` Greg Kroah-Hartman
2020-02-21 23:55           ` Chanwoo Choi
2020-02-23 17:54             ` Greg Kroah-Hartman
2020-02-24  2:17               ` Chanwoo Choi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).