Groups > linux.kernel > #1564957 > unrolled thread

v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900

Started by	Pavel Machek <pavel@ucw.cz>
First post	2017-01-23 13:50 +0100
Last post	2017-01-23 21:30 +0100
Articles	20 on this page of 43 — 5 participants

Back to article view | Back to linux.kernel

  v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Pavel Machek <pavel@ucw.cz> - 2017-01-23 13:50 +0100
    Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Pali Rohár <pali.rohar@gmail.com> - 2017-01-23 13:50 +0100
      Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Pavel Machek <pavel@ucw.cz> - 2017-01-23 15:30 +0100
    Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Pavel Machek <pavel@ucw.cz> - 2017-01-23 15:20 +0100
      Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Guenter Roeck <linux@roeck-us.net> - 2017-01-23 15:40 +0100
        Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Pavel Machek <pavel@ucw.cz> - 2017-01-23 15:50 +0100
          Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Guenter Roeck <linux@roeck-us.net> - 2017-01-23 18:20 +0100
          Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Pavel Machek <pavel@ucw.cz> - 2017-01-24 00:30 +0100
            Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Guenter Roeck <linux@roeck-us.net> - 2017-01-24 00:50 +0100
              Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Zhang Rui <rui.zhang@intel.com> - 2017-01-24 08:10 +0100
                Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Pavel Machek <pavel@ucw.cz> - 2017-01-24 08:40 +0100
                  Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Guenter Roeck <linux@roeck-us.net> - 2017-01-24 15:20 +0100
                    Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Pavel Machek <pavel@ucw.cz> - 2017-01-24 19:00 +0100
                      Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Guenter Roeck <linux@roeck-us.net> - 2017-01-24 19:50 +0100
                        Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Pavel Machek <pavel@ucw.cz> - 2017-01-24 23:50 +0100
                          Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Fabio Estevam <festevam@gmail.com> - 2017-01-25 00:10 +0100
                            Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Zhang Rui <rui.zhang@intel.com> - 2017-01-25 06:40 +0100
                              Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Pavel Machek <pavel@ucw.cz> - 2017-01-25 11:20 +0100
                        Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Pavel Machek <pavel@ucw.cz> - 2017-01-25 12:20 +0100
                          Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Guenter Roeck <linux@roeck-us.net> - 2017-01-25 13:00 +0100
                          Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Pali Rohár <pali.rohar@gmail.com> - 2017-01-25 13:10 +0100
                            Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Zhang Rui <rui.zhang@intel.com> - 2017-01-27 02:40 +0100
                              Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Guenter Roeck <linux@roeck-us.net> - 2017-01-27 03:10 +0100
                                Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Zhang Rui <rui.zhang@intel.com> - 2017-01-27 04:50 +0100
                                  Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Guenter Roeck <linux@roeck-us.net> - 2017-01-27 08:20 +0100
                                    Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Pavel Machek <pavel@ucw.cz> - 2017-01-27 12:20 +0100
                                      Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Guenter Roeck <linux@roeck-us.net> - 2017-01-27 15:20 +0100
                                        Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Pavel Machek <pavel@ucw.cz> - 2017-01-27 20:10 +0100
                                    Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Zhang Rui <rui.zhang@intel.com> - 2017-01-27 15:50 +0100
                                      Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Guenter Roeck <linux@roeck-us.net> - 2017-01-27 19:30 +0100
                                        Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Pavel Machek <pavel@ucw.cz> - 2017-01-27 20:20 +0100
                                      Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Pavel Machek <pavel@ucw.cz> - 2017-01-27 20:40 +0100
                                  Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Pali Rohár <pali.rohar@gmail.com> - 2017-01-27 09:40 +0100
                                Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Pavel Machek <pavel@ucw.cz> - 2017-01-27 12:30 +0100
                                  Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Guenter Roeck <linux@roeck-us.net> - 2017-01-27 15:20 +0100
                                    Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Pavel Machek <pavel@ucw.cz> - 2017-01-27 20:20 +0100
                            Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Pavel Machek <pavel@ucw.cz> - 2017-01-27 12:40 +0100
                Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Guenter Roeck <linux@roeck-us.net> - 2017-01-24 15:20 +0100
                  Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Pavel Machek <pavel@ucw.cz> - 2017-01-24 15:20 +0100
                    Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Guenter Roeck <linux@roeck-us.net> - 2017-01-24 15:40 +0100
              Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Pavel Machek <pavel@ucw.cz> - 2017-01-24 08:40 +0100
                Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Guenter Roeck <linux@roeck-us.net> - 2017-01-24 15:20 +0100
      Re: v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900 Pavel Machek <pavel@ucw.cz> - 2017-01-23 21:30 +0100

Page 1 of 3 [1] 2 3 Next page →

#1564957 — v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900

From	Pavel Machek <pavel@ucw.cz>
Date	2017-01-23 13:50 +0100
Subject	v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900
Message-ID	<t2Mem-6v8-13@gated-at.bofh.it>

[Multipart message — attachments visible in raw view] — view raw

Hi!

It seems that battery driver stopped working on N900 between -rc4 and
-rc5.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [next] | [standalone]

#1564960

From	Pali Rohár <pali.rohar@gmail.com>
Date	2017-01-23 13:50 +0100
Message-ID	<t2Mem-6v8-17@gated-at.bofh.it>
In reply to	#1564957

On Monday 23 January 2017 13:40:58 Pavel Machek wrote:
> It seems that battery driver stopped working on N900 between -rc4 and
> -rc5.

Which one of those 4 drivers stopped working?

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1565001

From	Pavel Machek <pavel@ucw.cz>
Date	2017-01-23 15:30 +0100
Message-ID	<t2NN8-7xX-15@gated-at.bofh.it>
In reply to	#1564960

[Multipart message — attachments visible in raw view] — view raw

On Mon 2017-01-23 13:42:06, Pali Rohár wrote:
> On Monday 23 January 2017 13:40:58 Pavel Machek wrote:
> > It seems that battery driver stopped working on N900 between -rc4 and
> > -rc5.
> 
> Which one of those 4 drivers stopped working?

n900-battery.

									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1564999

From	Pavel Machek <pavel@ucw.cz>
Date	2017-01-23 15:20 +0100
Message-ID	<t2NDs-7uE-15@gated-at.bofh.it>
In reply to	#1564957

[Multipart message — attachments visible in raw view] — view raw

Hi!

> It seems that battery driver stopped working on N900 between -rc4 and
> -rc5.

pavel@amd:/data/l/linux-n900$ git bisect log
# bad: [41f283973653dc44af47585fa79fb6da6ffdc2e2] Merge
/data//l/clean-cg into mini-v4.10
# good: [4f8d9925eb1de1ec1c4183164f1bc7b01f5ae185] It seems mainline
-rc4 did have a fix for the sound issue. Use that one.
git bisect start 'mini-v4.10'
'4f8d9925eb1de1ec1c4183164f1bc7b01f5ae185'
# good: [e90665a5d38b17fdbe484a85fbba917a7006522d] Merge tag
'ceph-for-4.10-rc5' of git://github.com/ceph/ceph-client
git bisect good e90665a5d38b17fdbe484a85fbba917a7006522d
# good: [83fd57a740bb19286959b3085eb93532f3e7ef2c] Merge tag
'powerpc-4.10-2' of
git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux
git bisect good 83fd57a740bb19286959b3085eb93532f3e7ef2c
# good: [c497f8d17246720afe680ea1a8fa6e48e75af852] Merge tag
'usb-4.10-rc5' of
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb
git bisect good c497f8d17246720afe680ea1a8fa6e48e75af852
# bad: [bb6c01c2dde67b165cf7c808b0f00677b6f94b96] Merge branch
'for-rc' of git://git.kernel.org/pub/scm/linux/kernel/git/rzhang/linux
git bisect bad bb6c01c2dde67b165cf7c808b0f00677b6f94b96
# good: [18591add41ec9558ce0e32ef88626c18cc70c686] thermal: rockchip:
handle set_trips without the trip points
git bisect good 18591add41ec9558ce0e32ef88626c18cc70c686
# bad: [f53345e8cf027d03187b9417f1f8883c516e1a5b] thermal: core: move
tz->device.groups cleanup to thermal_release
git bisect bad f53345e8cf027d03187b9417f1f8883c516e1a5b
# bad: [7611fb68062f8d7f416f3272894d1edf7bbff29c] thermal:
thermal_hwmon: Convert to hwmon_device_register_with_info()
git bisect bad 7611fb68062f8d7f416f3272894d1edf7bbff29c
# first bad commit: [7611fb68062f8d7f416f3272894d1edf7bbff29c]
thermal: thermal_hwmon: Convert to hwmon_device_register_with_info()
pavel@amd:/data/l/linux-n900$

pavel@amd:/data/l/linux-n900$ git bisect log
# bad: [41f283973653dc44af47585fa79fb6da6ffdc2e2] Merge
/data//l/clean-cg into mini-v4.10
# good: [4f8d9925eb1de1ec1c4183164f1bc7b01f5ae185] It seems mainline
-rc4 did have a fix for the sound issue. Use that one.
git bisect start 'mini-v4.10'
'4f8d9925eb1de1ec1c4183164f1bc7b01f5ae185'
# good: [e90665a5d38b17fdbe484a85fbba917a7006522d] Merge tag
'ceph-for-4.10-rc5' of git://github.com/ceph/ceph-client
git bisect good e90665a5d38b17fdbe484a85fbba917a7006522d
# good: [83fd57a740bb19286959b3085eb93532f3e7ef2c] Merge tag
'powerpc-4.10-2' of
git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux
git bisect good 83fd57a740bb19286959b3085eb93532f3e7ef2c
# good: [c497f8d17246720afe680ea1a8fa6e48e75af852] Merge tag
'usb-4.10-rc5' of
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb
git bisect good c497f8d17246720afe680ea1a8fa6e48e75af852
# bad: [bb6c01c2dde67b165cf7c808b0f00677b6f94b96] Merge branch
'for-rc' of git://git.kernel.org/pub/scm/linux/kernel/git/rzhang/linux
git bisect bad bb6c01c2dde67b165cf7c808b0f00677b6f94b96
# good: [18591add41ec9558ce0e32ef88626c18cc70c686] thermal: rockchip:
handle set_trips without the trip points
git bisect good 18591add41ec9558ce0e32ef88626c18cc70c686
# bad: [f53345e8cf027d03187b9417f1f8883c516e1a5b] thermal: core: move
tz->device.groups cleanup to thermal_release
git bisect bad f53345e8cf027d03187b9417f1f8883c516e1a5b
# bad: [7611fb68062f8d7f416f3272894d1edf7bbff29c] thermal:
thermal_hwmon: Convert to hwmon_device_register_with_info()
git bisect bad 7611fb68062f8d7f416f3272894d1edf7bbff29c
# first bad commit: [7611fb68062f8d7f416f3272894d1edf7bbff29c]
thermal: thermal_hwmon: Convert to hwmon_device_register_with_info()
pavel@amd:/data/l/linux-n900$

7611fb68062f8d7f416f3272894d1edf7bbff29c is the first bad commit
commit 7611fb68062f8d7f416f3272894d1edf7bbff29c
Author: Fabio Estevam <fabio.estevam@nxp.com>
Date:   Tue Dec 27 15:31:49 2016 -0200

    thermal: thermal_hwmon: Convert to
    hwmon_device_register_with_info()

    Booting Linux on a mx6q based board leads to the following
    warning:

    (NULL device *): hwmon_device_register() is deprecated. Please
    convert the
        driver to use hwmon_device_register_with_info().

    ,so do as suggested.

    Also, this results in the core taking care of creating the 'name'
        attribute, so drop the code doing that from the thermal
        driver.

 Suggested-by: Guenter Roeck <linux@roeck-us.net>
 Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
 Signed-off-by: Zhang Rui <rui.zhang@intel.com>

:040000 040000 b3f6095ab53677277c2f9d75cb6430afb6825765
156a18e983a22b8ae43c7b312a4e1c63017f0e26 M	drivers

I'll try to revert it on the top of v4.10-rc5 now... and yes, it fixes
the issue.

Any idea what went wrong and how to fix that?

Anyway as we are at -rc5 and this is warning fix that caused a
regression on different hardware... it should be reverted.

Best regards,

								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1565006

From	Guenter Roeck <linux@roeck-us.net>
Date	2017-01-23 15:40 +0100
Message-ID	<t2NWN-7Bg-13@gated-at.bofh.it>
In reply to	#1564999

On 01/23/2017 06:19 AM, Pavel Machek wrote:
> Hi!
>
>> It seems that battery driver stopped working on N900 between -rc4 and
>> -rc5.
>
> pavel@amd:/data/l/linux-n900$ git bisect log
> # bad: [41f283973653dc44af47585fa79fb6da6ffdc2e2] Merge
> /data//l/clean-cg into mini-v4.10
> # good: [4f8d9925eb1de1ec1c4183164f1bc7b01f5ae185] It seems mainline
> -rc4 did have a fix for the sound issue. Use that one.
> git bisect start 'mini-v4.10'
> '4f8d9925eb1de1ec1c4183164f1bc7b01f5ae185'
> # good: [e90665a5d38b17fdbe484a85fbba917a7006522d] Merge tag
> 'ceph-for-4.10-rc5' of git://github.com/ceph/ceph-client
> git bisect good e90665a5d38b17fdbe484a85fbba917a7006522d
> # good: [83fd57a740bb19286959b3085eb93532f3e7ef2c] Merge tag
> 'powerpc-4.10-2' of
> git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux
> git bisect good 83fd57a740bb19286959b3085eb93532f3e7ef2c
> # good: [c497f8d17246720afe680ea1a8fa6e48e75af852] Merge tag
> 'usb-4.10-rc5' of
> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb
> git bisect good c497f8d17246720afe680ea1a8fa6e48e75af852
> # bad: [bb6c01c2dde67b165cf7c808b0f00677b6f94b96] Merge branch
> 'for-rc' of git://git.kernel.org/pub/scm/linux/kernel/git/rzhang/linux
> git bisect bad bb6c01c2dde67b165cf7c808b0f00677b6f94b96
> # good: [18591add41ec9558ce0e32ef88626c18cc70c686] thermal: rockchip:
> handle set_trips without the trip points
> git bisect good 18591add41ec9558ce0e32ef88626c18cc70c686
> # bad: [f53345e8cf027d03187b9417f1f8883c516e1a5b] thermal: core: move
> tz->device.groups cleanup to thermal_release
> git bisect bad f53345e8cf027d03187b9417f1f8883c516e1a5b
> # bad: [7611fb68062f8d7f416f3272894d1edf7bbff29c] thermal:
> thermal_hwmon: Convert to hwmon_device_register_with_info()
> git bisect bad 7611fb68062f8d7f416f3272894d1edf7bbff29c
> # first bad commit: [7611fb68062f8d7f416f3272894d1edf7bbff29c]
> thermal: thermal_hwmon: Convert to hwmon_device_register_with_info()
> pavel@amd:/data/l/linux-n900$
>
> pavel@amd:/data/l/linux-n900$ git bisect log
> # bad: [41f283973653dc44af47585fa79fb6da6ffdc2e2] Merge
> /data//l/clean-cg into mini-v4.10
> # good: [4f8d9925eb1de1ec1c4183164f1bc7b01f5ae185] It seems mainline
> -rc4 did have a fix for the sound issue. Use that one.
> git bisect start 'mini-v4.10'
> '4f8d9925eb1de1ec1c4183164f1bc7b01f5ae185'
> # good: [e90665a5d38b17fdbe484a85fbba917a7006522d] Merge tag
> 'ceph-for-4.10-rc5' of git://github.com/ceph/ceph-client
> git bisect good e90665a5d38b17fdbe484a85fbba917a7006522d
> # good: [83fd57a740bb19286959b3085eb93532f3e7ef2c] Merge tag
> 'powerpc-4.10-2' of
> git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux
> git bisect good 83fd57a740bb19286959b3085eb93532f3e7ef2c
> # good: [c497f8d17246720afe680ea1a8fa6e48e75af852] Merge tag
> 'usb-4.10-rc5' of
> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb
> git bisect good c497f8d17246720afe680ea1a8fa6e48e75af852
> # bad: [bb6c01c2dde67b165cf7c808b0f00677b6f94b96] Merge branch
> 'for-rc' of git://git.kernel.org/pub/scm/linux/kernel/git/rzhang/linux
> git bisect bad bb6c01c2dde67b165cf7c808b0f00677b6f94b96
> # good: [18591add41ec9558ce0e32ef88626c18cc70c686] thermal: rockchip:
> handle set_trips without the trip points
> git bisect good 18591add41ec9558ce0e32ef88626c18cc70c686
> # bad: [f53345e8cf027d03187b9417f1f8883c516e1a5b] thermal: core: move
> tz->device.groups cleanup to thermal_release
> git bisect bad f53345e8cf027d03187b9417f1f8883c516e1a5b
> # bad: [7611fb68062f8d7f416f3272894d1edf7bbff29c] thermal:
> thermal_hwmon: Convert to hwmon_device_register_with_info()
> git bisect bad 7611fb68062f8d7f416f3272894d1edf7bbff29c
> # first bad commit: [7611fb68062f8d7f416f3272894d1edf7bbff29c]
> thermal: thermal_hwmon: Convert to hwmon_device_register_with_info()
> pavel@amd:/data/l/linux-n900$
>
> 7611fb68062f8d7f416f3272894d1edf7bbff29c is the first bad commit
> commit 7611fb68062f8d7f416f3272894d1edf7bbff29c
> Author: Fabio Estevam <fabio.estevam@nxp.com>
> Date:   Tue Dec 27 15:31:49 2016 -0200
>
>     thermal: thermal_hwmon: Convert to
>     hwmon_device_register_with_info()
>
>     Booting Linux on a mx6q based board leads to the following
>     warning:
>
>     (NULL device *): hwmon_device_register() is deprecated. Please
>     convert the
>         driver to use hwmon_device_register_with_info().
>
>     ,so do as suggested.
>
>     Also, this results in the core taking care of creating the 'name'
>         attribute, so drop the code doing that from the thermal
>         driver.
>
>  Suggested-by: Guenter Roeck <linux@roeck-us.net>
>  Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
>  Signed-off-by: Zhang Rui <rui.zhang@intel.com>
>
> :040000 040000 b3f6095ab53677277c2f9d75cb6430afb6825765
> 156a18e983a22b8ae43c7b312a4e1c63017f0e26 M	drivers
>
> I'll try to revert it on the top of v4.10-rc5 now... and yes, it fixes
> the issue.
>
> Any idea what went wrong and how to fix that?
>
> Anyway as we are at -rc5 and this is warning fix that caused a
> regression on different hardware... it should be reverted.
>
Agreed.

What exactly does "stopped working" mean ? That might help understanding
what went wrong.

Thanks,
Guenter

[toc] | [prev] | [next] | [standalone]

#1565013

From	Pavel Machek <pavel@ucw.cz>
Date	2017-01-23 15:50 +0100
Message-ID	<t2O6u-7EI-19@gated-at.bofh.it>
In reply to	#1565006

[Multipart message — attachments visible in raw view] — view raw

On Mon 2017-01-23 06:31:31, Guenter Roeck wrote:
> On 01/23/2017 06:19 AM, Pavel Machek wrote:
> >Hi!
> >
> >>It seems that battery driver stopped working on N900 between -rc4 and
> >>-rc5.
> >
> >pavel@amd:/data/l/linux-n900$ git bisect log
> ># bad: [41f283973653dc44af47585fa79fb6da6ffdc2e2] Merge
> >/data//l/clean-cg into mini-v4.10
> ># good: [4f8d9925eb1de1ec1c4183164f1bc7b01f5ae185] It seems mainline
> >-rc4 did have a fix for the sound issue. Use that one.
> >git bisect start 'mini-v4.10'
> >'4f8d9925eb1de1ec1c4183164f1bc7b01f5ae185'
> ># good: [e90665a5d38b17fdbe484a85fbba917a7006522d] Merge tag
> >'ceph-for-4.10-rc5' of git://github.com/ceph/ceph-client
> >git bisect good e90665a5d38b17fdbe484a85fbba917a7006522d
> ># good: [83fd57a740bb19286959b3085eb93532f3e7ef2c] Merge tag
> >'powerpc-4.10-2' of
> >git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux
> >git bisect good 83fd57a740bb19286959b3085eb93532f3e7ef2c
> ># good: [c497f8d17246720afe680ea1a8fa6e48e75af852] Merge tag
> >'usb-4.10-rc5' of
> >git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb
> >git bisect good c497f8d17246720afe680ea1a8fa6e48e75af852
> ># bad: [bb6c01c2dde67b165cf7c808b0f00677b6f94b96] Merge branch
> >'for-rc' of git://git.kernel.org/pub/scm/linux/kernel/git/rzhang/linux
> >git bisect bad bb6c01c2dde67b165cf7c808b0f00677b6f94b96
> ># good: [18591add41ec9558ce0e32ef88626c18cc70c686] thermal: rockchip:
> >handle set_trips without the trip points
> >git bisect good 18591add41ec9558ce0e32ef88626c18cc70c686
> ># bad: [f53345e8cf027d03187b9417f1f8883c516e1a5b] thermal: core: move
> >tz->device.groups cleanup to thermal_release
> >git bisect bad f53345e8cf027d03187b9417f1f8883c516e1a5b
> ># bad: [7611fb68062f8d7f416f3272894d1edf7bbff29c] thermal:
> >thermal_hwmon: Convert to hwmon_device_register_with_info()
> >git bisect bad 7611fb68062f8d7f416f3272894d1edf7bbff29c
> ># first bad commit: [7611fb68062f8d7f416f3272894d1edf7bbff29c]
> >thermal: thermal_hwmon: Convert to hwmon_device_register_with_info()
> >pavel@amd:/data/l/linux-n900$
> >
> >pavel@amd:/data/l/linux-n900$ git bisect log
> ># bad: [41f283973653dc44af47585fa79fb6da6ffdc2e2] Merge
> >/data//l/clean-cg into mini-v4.10
> ># good: [4f8d9925eb1de1ec1c4183164f1bc7b01f5ae185] It seems mainline
> >-rc4 did have a fix for the sound issue. Use that one.
> >git bisect start 'mini-v4.10'
> >'4f8d9925eb1de1ec1c4183164f1bc7b01f5ae185'
> ># good: [e90665a5d38b17fdbe484a85fbba917a7006522d] Merge tag
> >'ceph-for-4.10-rc5' of git://github.com/ceph/ceph-client
> >git bisect good e90665a5d38b17fdbe484a85fbba917a7006522d
> ># good: [83fd57a740bb19286959b3085eb93532f3e7ef2c] Merge tag
> >'powerpc-4.10-2' of
> >git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux
> >git bisect good 83fd57a740bb19286959b3085eb93532f3e7ef2c
> ># good: [c497f8d17246720afe680ea1a8fa6e48e75af852] Merge tag
> >'usb-4.10-rc5' of
> >git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb
> >git bisect good c497f8d17246720afe680ea1a8fa6e48e75af852
> ># bad: [bb6c01c2dde67b165cf7c808b0f00677b6f94b96] Merge branch
> >'for-rc' of git://git.kernel.org/pub/scm/linux/kernel/git/rzhang/linux
> >git bisect bad bb6c01c2dde67b165cf7c808b0f00677b6f94b96
> ># good: [18591add41ec9558ce0e32ef88626c18cc70c686] thermal: rockchip:
> >handle set_trips without the trip points
> >git bisect good 18591add41ec9558ce0e32ef88626c18cc70c686
> ># bad: [f53345e8cf027d03187b9417f1f8883c516e1a5b] thermal: core: move
> >tz->device.groups cleanup to thermal_release
> >git bisect bad f53345e8cf027d03187b9417f1f8883c516e1a5b
> ># bad: [7611fb68062f8d7f416f3272894d1edf7bbff29c] thermal:
> >thermal_hwmon: Convert to hwmon_device_register_with_info()
> >git bisect bad 7611fb68062f8d7f416f3272894d1edf7bbff29c
> ># first bad commit: [7611fb68062f8d7f416f3272894d1edf7bbff29c]
> >thermal: thermal_hwmon: Convert to hwmon_device_register_with_info()
> >pavel@amd:/data/l/linux-n900$
> >
> >7611fb68062f8d7f416f3272894d1edf7bbff29c is the first bad commit
> >commit 7611fb68062f8d7f416f3272894d1edf7bbff29c
> >Author: Fabio Estevam <fabio.estevam@nxp.com>
> >Date:   Tue Dec 27 15:31:49 2016 -0200
> >
> >    thermal: thermal_hwmon: Convert to
> >    hwmon_device_register_with_info()
> >
> >    Booting Linux on a mx6q based board leads to the following
> >    warning:
> >
> >    (NULL device *): hwmon_device_register() is deprecated. Please
> >    convert the
> >        driver to use hwmon_device_register_with_info().
> >
> >    ,so do as suggested.
> >
> >    Also, this results in the core taking care of creating the 'name'
> >        attribute, so drop the code doing that from the thermal
> >        driver.
> >
> > Suggested-by: Guenter Roeck <linux@roeck-us.net>
> > Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
> > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> >
> >:040000 040000 b3f6095ab53677277c2f9d75cb6430afb6825765
> >156a18e983a22b8ae43c7b312a4e1c63017f0e26 M	drivers
> >
> >I'll try to revert it on the top of v4.10-rc5 now... and yes, it fixes
> >the issue.
> >
> >Any idea what went wrong and how to fix that?
> >
> >Anyway as we are at -rc5 and this is warning fix that caused a
> >regression on different hardware... it should be reverted.
> >
> Agreed.
> 
> What exactly does "stopped working" mean ? That might help understanding
> what went wrong.

/sys files related to battery no longer appear. I beieve this has
something to do with it:

[    2.374877] of_get_named_gpiod_flags: parsed 'reset-gpios' property
of node '/ocp@68000000/spi@48098000/tsc2005@0[0]' - status (0)
[    2.375946] input: TSC2005 touchscreen as
/devices/platform/68000000.ocp/48098000.spi/spi_master/spi1/spi1.0/input/input5
[    2.392120] rx51-battery: probe of n900-battery failed with error
-22
[    2.399902] omap_hsmmc 4809c000.mmc: GPIO lookup for consumer cd

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1565171

From	Guenter Roeck <linux@roeck-us.net>
Date	2017-01-23 18:20 +0100
Message-ID	<t2QrF-Nm-43@gated-at.bofh.it>
In reply to	#1565013

On Mon, Jan 23, 2017 at 03:40:31PM +0100, Pavel Machek wrote:
> On Mon 2017-01-23 06:31:31, Guenter Roeck wrote:
> > On 01/23/2017 06:19 AM, Pavel Machek wrote:
> > >Hi!
> > >
> > >>It seems that battery driver stopped working on N900 between -rc4 and
> > >>-rc5.
> > >
> > >pavel@amd:/data/l/linux-n900$ git bisect log
> > ># bad: [41f283973653dc44af47585fa79fb6da6ffdc2e2] Merge
> > >/data//l/clean-cg into mini-v4.10
> > ># good: [4f8d9925eb1de1ec1c4183164f1bc7b01f5ae185] It seems mainline
> > >-rc4 did have a fix for the sound issue. Use that one.
> > >git bisect start 'mini-v4.10'
> > >'4f8d9925eb1de1ec1c4183164f1bc7b01f5ae185'
> > ># good: [e90665a5d38b17fdbe484a85fbba917a7006522d] Merge tag
> > >'ceph-for-4.10-rc5' of git://github.com/ceph/ceph-client
> > >git bisect good e90665a5d38b17fdbe484a85fbba917a7006522d
> > ># good: [83fd57a740bb19286959b3085eb93532f3e7ef2c] Merge tag
> > >'powerpc-4.10-2' of
> > >git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux
> > >git bisect good 83fd57a740bb19286959b3085eb93532f3e7ef2c
> > ># good: [c497f8d17246720afe680ea1a8fa6e48e75af852] Merge tag
> > >'usb-4.10-rc5' of
> > >git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb
> > >git bisect good c497f8d17246720afe680ea1a8fa6e48e75af852
> > ># bad: [bb6c01c2dde67b165cf7c808b0f00677b6f94b96] Merge branch
> > >'for-rc' of git://git.kernel.org/pub/scm/linux/kernel/git/rzhang/linux
> > >git bisect bad bb6c01c2dde67b165cf7c808b0f00677b6f94b96
> > ># good: [18591add41ec9558ce0e32ef88626c18cc70c686] thermal: rockchip:
> > >handle set_trips without the trip points
> > >git bisect good 18591add41ec9558ce0e32ef88626c18cc70c686
> > ># bad: [f53345e8cf027d03187b9417f1f8883c516e1a5b] thermal: core: move
> > >tz->device.groups cleanup to thermal_release
> > >git bisect bad f53345e8cf027d03187b9417f1f8883c516e1a5b
> > ># bad: [7611fb68062f8d7f416f3272894d1edf7bbff29c] thermal:
> > >thermal_hwmon: Convert to hwmon_device_register_with_info()
> > >git bisect bad 7611fb68062f8d7f416f3272894d1edf7bbff29c
> > ># first bad commit: [7611fb68062f8d7f416f3272894d1edf7bbff29c]
> > >thermal: thermal_hwmon: Convert to hwmon_device_register_with_info()
> > >pavel@amd:/data/l/linux-n900$
> > >
> > >pavel@amd:/data/l/linux-n900$ git bisect log
> > ># bad: [41f283973653dc44af47585fa79fb6da6ffdc2e2] Merge
> > >/data//l/clean-cg into mini-v4.10
> > ># good: [4f8d9925eb1de1ec1c4183164f1bc7b01f5ae185] It seems mainline
> > >-rc4 did have a fix for the sound issue. Use that one.
> > >git bisect start 'mini-v4.10'
> > >'4f8d9925eb1de1ec1c4183164f1bc7b01f5ae185'
> > ># good: [e90665a5d38b17fdbe484a85fbba917a7006522d] Merge tag
> > >'ceph-for-4.10-rc5' of git://github.com/ceph/ceph-client
> > >git bisect good e90665a5d38b17fdbe484a85fbba917a7006522d
> > ># good: [83fd57a740bb19286959b3085eb93532f3e7ef2c] Merge tag
> > >'powerpc-4.10-2' of
> > >git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux
> > >git bisect good 83fd57a740bb19286959b3085eb93532f3e7ef2c
> > ># good: [c497f8d17246720afe680ea1a8fa6e48e75af852] Merge tag
> > >'usb-4.10-rc5' of
> > >git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb
> > >git bisect good c497f8d17246720afe680ea1a8fa6e48e75af852
> > ># bad: [bb6c01c2dde67b165cf7c808b0f00677b6f94b96] Merge branch
> > >'for-rc' of git://git.kernel.org/pub/scm/linux/kernel/git/rzhang/linux
> > >git bisect bad bb6c01c2dde67b165cf7c808b0f00677b6f94b96
> > ># good: [18591add41ec9558ce0e32ef88626c18cc70c686] thermal: rockchip:
> > >handle set_trips without the trip points
> > >git bisect good 18591add41ec9558ce0e32ef88626c18cc70c686
> > ># bad: [f53345e8cf027d03187b9417f1f8883c516e1a5b] thermal: core: move
> > >tz->device.groups cleanup to thermal_release
> > >git bisect bad f53345e8cf027d03187b9417f1f8883c516e1a5b
> > ># bad: [7611fb68062f8d7f416f3272894d1edf7bbff29c] thermal:
> > >thermal_hwmon: Convert to hwmon_device_register_with_info()
> > >git bisect bad 7611fb68062f8d7f416f3272894d1edf7bbff29c
> > ># first bad commit: [7611fb68062f8d7f416f3272894d1edf7bbff29c]
> > >thermal: thermal_hwmon: Convert to hwmon_device_register_with_info()
> > >pavel@amd:/data/l/linux-n900$
> > >
> > >7611fb68062f8d7f416f3272894d1edf7bbff29c is the first bad commit
> > >commit 7611fb68062f8d7f416f3272894d1edf7bbff29c
> > >Author: Fabio Estevam <fabio.estevam@nxp.com>
> > >Date:   Tue Dec 27 15:31:49 2016 -0200
> > >
> > >    thermal: thermal_hwmon: Convert to
> > >    hwmon_device_register_with_info()
> > >
> > >    Booting Linux on a mx6q based board leads to the following
> > >    warning:
> > >
> > >    (NULL device *): hwmon_device_register() is deprecated. Please
> > >    convert the
> > >        driver to use hwmon_device_register_with_info().
> > >
> > >    ,so do as suggested.
> > >
> > >    Also, this results in the core taking care of creating the 'name'
> > >        attribute, so drop the code doing that from the thermal
> > >        driver.
> > >
> > > Suggested-by: Guenter Roeck <linux@roeck-us.net>
> > > Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
> > > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > >
> > >:040000 040000 b3f6095ab53677277c2f9d75cb6430afb6825765
> > >156a18e983a22b8ae43c7b312a4e1c63017f0e26 M	drivers
> > >
> > >I'll try to revert it on the top of v4.10-rc5 now... and yes, it fixes
> > >the issue.
> > >
> > >Any idea what went wrong and how to fix that?
> > >
> > >Anyway as we are at -rc5 and this is warning fix that caused a
> > >regression on different hardware... it should be reverted.
> > >
> > Agreed.
> > 
> > What exactly does "stopped working" mean ? That might help understanding
> > what went wrong.
> 
> /sys files related to battery no longer appear. I beieve this has
> something to do with it:
> 
> [    2.374877] of_get_named_gpiod_flags: parsed 'reset-gpios' property
> of node '/ocp@68000000/spi@48098000/tsc2005@0[0]' - status (0)
> [    2.375946] input: TSC2005 touchscreen as
> /devices/platform/68000000.ocp/48098000.spi/spi_master/spi1/spi1.0/input/input5
> [    2.392120] rx51-battery: probe of n900-battery failed with error
> -22
> [    2.399902] omap_hsmmc 4809c000.mmc: GPIO lookup for consumer cd
> 

Only problem I can imagine is that power_supply_register() fails registering
the thermal zone, which in turn would suggest that the offending patch might
not work at all, at least not under some circumstances. So it should
definitely be reverted until we figure out the problem.

Guenter

[toc] | [prev] | [next] | [standalone]

#1565364

From	Pavel Machek <pavel@ucw.cz>
Date	2017-01-24 00:30 +0100
Message-ID	<t2WdH-4AA-9@gated-at.bofh.it>
In reply to	#1565013

[Multipart message — attachments visible in raw view] — view raw

Hi!

> > >I'll try to revert it on the top of v4.10-rc5 now... and yes, it fixes
> > >the issue.
> > >
> > >Any idea what went wrong and how to fix that?
> > >
> > >Anyway as we are at -rc5 and this is warning fix that caused a
> > >regression on different hardware... it should be reverted.
> > >
> > Agreed.
> > 
> > What exactly does "stopped working" mean ? That might help understanding
> > what went wrong.
> 
> /sys files related to battery no longer appear. I beieve this has
> something to do with it:
> 
> [    2.374877] of_get_named_gpiod_flags: parsed 'reset-gpios' property
> of node '/ocp@68000000/spi@48098000/tsc2005@0[0]' - status (0)
> [    2.375946] input: TSC2005 touchscreen as
> /devices/platform/68000000.ocp/48098000.spi/spi_master/spi1/spi1.0/input/input5
> [    2.392120] rx51-battery: probe of n900-battery failed with error
> -22

Mystery solved:

diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
index 3932f92..fe5ec82 100644
--- a/drivers/hwmon/hwmon.c
+++ b/drivers/hwmon/hwmon.c
@@ -545,8 +545,10 @@ __hwmon_device_register(struct device *dev, const char *name, void *drvdata,
 	int i, j, err, id;
 
 	/* Do not accept invalid characters in hwmon name attribute */
-	if (name && (!strlen(name) || strpbrk(name, "-* \t\n")))
+	if (name && (!strlen(name) || strpbrk(name, "-* \t\n"))) {
+		printk("hwmon: Invalid character detected: %s\n", name);
 		return ERR_PTR(-EINVAL);
+	}
 
 	id = ida_simple_get(&hwmon_ida, 0, 0, GFP_KERNEL);
 	if (id < 0)


pavel@n900:~$ dmesg | grep -5 Invalid
[    0.829650] of_get_named_gpiod_flags: parsed 'gpio-reset' property
of node '/ocp@68000000/i2c@48072000/tlv320aic3x@19[0]' - status (0)
[    0.833831] tsl2563 2-0029: model 7, rev. 0
[    0.837768] of_get_named_gpiod_flags: parsed 'enable-gpio' property
of node '/ocp@68000000/i2c@48072000/lp5523@32[0]' - status (0)
[    1.921417] omap_i2c 48072000.i2c: controller timed out
[    2.056823] lp5523x 2-0032: lp5523 Programmable led chip found
[    2.064147] hwmon: Invalid character detected: bq27200-0
[    2.064544] bq27xxx-battery 2-0055: failed to register battery
[    2.064605] bq27xxx-battery: probe of 2-0055 failed with error -22
[    2.065368] of_get_named_gpiod_flags: parsed 'power-gpio' property
of node '/ocp@68000000/i2c@48072000/tpa6130a2@60[0]' - status (0)
[    2.083221] bq2415x-charger 2-006b: automode supported, waiting for
events
[    2.084442] bq2415x-charger 2-006b: driver registered
--
[    2.369842] g_ether gadget: g_ether ready
[    2.377197] tsc2005 spi1.0: GPIO lookup for consumer reset
[    2.377227] tsc2005 spi1.0: using device tree for GPIO lookup
[    2.377288] of_get_named_gpiod_flags: parsed 'reset-gpios' property
of node '/ocp@68000000/spi@48098000/tsc2005@0[0]' - status (0)
[    2.378936] input: TSC2005 touchscreen as
/devices/platform/68000000.ocp/48098000.spi/spi_master/spi1/spi1.0/input/input5
[    2.395111] hwmon: Invalid character detected: rx51-battery
[    2.402587] rx51-battery: probe of n900-battery failed with error
-22
[    2.410247] omap_hsmmc 4809c000.mmc: GPIO lookup for consumer cd
[    2.410247] omap_hsmmc 4809c000.mmc: using device tree for GPIO
lookup
[    2.410278] of_get_named_gpiod_flags: can't parse 'cd-gpios'
property of node '/ocp@68000000/mmc@4809c000[0]'
[    2.410278] of_get_named_gpiod_flags: can't parse 'cd-gpio'
property of node '/ocp@68000000/mmc@4809c000[0]'

									Pavel


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1565371

From	Guenter Roeck <linux@roeck-us.net>
Date	2017-01-24 00:50 +0100
Message-ID	<t2Wx3-4Jj-5@gated-at.bofh.it>
In reply to	#1565364

On Tue, Jan 24, 2017 at 12:26:54AM +0100, Pavel Machek wrote:
> Hi!
> 
> > > >I'll try to revert it on the top of v4.10-rc5 now... and yes, it fixes
> > > >the issue.
> > > >
> > > >Any idea what went wrong and how to fix that?
> > > >
> > > >Anyway as we are at -rc5 and this is warning fix that caused a
> > > >regression on different hardware... it should be reverted.
> > > >
> > > Agreed.
> > > 
> > > What exactly does "stopped working" mean ? That might help understanding
> > > what went wrong.
> > 
> > /sys files related to battery no longer appear. I beieve this has
> > something to do with it:
> > 
> > [    2.374877] of_get_named_gpiod_flags: parsed 'reset-gpios' property
> > of node '/ocp@68000000/spi@48098000/tsc2005@0[0]' - status (0)
> > [    2.375946] input: TSC2005 touchscreen as
> > /devices/platform/68000000.ocp/48098000.spi/spi_master/spi1/spi1.0/input/input5
> > [    2.392120] rx51-battery: probe of n900-battery failed with error
> > -22
> 
> Mystery solved:
> 
> diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
> index 3932f92..fe5ec82 100644
> --- a/drivers/hwmon/hwmon.c
> +++ b/drivers/hwmon/hwmon.c
> @@ -545,8 +545,10 @@ __hwmon_device_register(struct device *dev, const char *name, void *drvdata,
>  	int i, j, err, id;
>  
>  	/* Do not accept invalid characters in hwmon name attribute */
> -	if (name && (!strlen(name) || strpbrk(name, "-* \t\n")))
> +	if (name && (!strlen(name) || strpbrk(name, "-* \t\n"))) {
> +		printk("hwmon: Invalid character detected: %s\n", name);
>  		return ERR_PTR(-EINVAL);
> +	}
>  
>  	id = ida_simple_get(&hwmon_ida, 0, 0, GFP_KERNEL);
>  	if (id < 0)
> 
> 
> pavel@n900:~$ dmesg | grep -5 Invalid
> [    0.829650] of_get_named_gpiod_flags: parsed 'gpio-reset' property
> of node '/ocp@68000000/i2c@48072000/tlv320aic3x@19[0]' - status (0)
> [    0.833831] tsl2563 2-0029: model 7, rev. 0
> [    0.837768] of_get_named_gpiod_flags: parsed 'enable-gpio' property
> of node '/ocp@68000000/i2c@48072000/lp5523@32[0]' - status (0)
> [    1.921417] omap_i2c 48072000.i2c: controller timed out
> [    2.056823] lp5523x 2-0032: lp5523 Programmable led chip found
> [    2.064147] hwmon: Invalid character detected: bq27200-0

So the problem is really that the thermal driver needs to create a valid name.

Guenter

> [    2.064544] bq27xxx-battery 2-0055: failed to register battery
> [    2.064605] bq27xxx-battery: probe of 2-0055 failed with error -22
> [    2.065368] of_get_named_gpiod_flags: parsed 'power-gpio' property
> of node '/ocp@68000000/i2c@48072000/tpa6130a2@60[0]' - status (0)
> [    2.083221] bq2415x-charger 2-006b: automode supported, waiting for
> events
> [    2.084442] bq2415x-charger 2-006b: driver registered
> --
> [    2.369842] g_ether gadget: g_ether ready
> [    2.377197] tsc2005 spi1.0: GPIO lookup for consumer reset
> [    2.377227] tsc2005 spi1.0: using device tree for GPIO lookup
> [    2.377288] of_get_named_gpiod_flags: parsed 'reset-gpios' property
> of node '/ocp@68000000/spi@48098000/tsc2005@0[0]' - status (0)
> [    2.378936] input: TSC2005 touchscreen as
> /devices/platform/68000000.ocp/48098000.spi/spi_master/spi1/spi1.0/input/input5
> [    2.395111] hwmon: Invalid character detected: rx51-battery
> [    2.402587] rx51-battery: probe of n900-battery failed with error
> -22
> [    2.410247] omap_hsmmc 4809c000.mmc: GPIO lookup for consumer cd
> [    2.410247] omap_hsmmc 4809c000.mmc: using device tree for GPIO
> lookup
> [    2.410278] of_get_named_gpiod_flags: can't parse 'cd-gpios'
> property of node '/ocp@68000000/mmc@4809c000[0]'
> [    2.410278] of_get_named_gpiod_flags: can't parse 'cd-gpio'
> property of node '/ocp@68000000/mmc@4809c000[0]'
> 
> 									Pavel
> 
> 
> -- 
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1565510

From	Zhang Rui <rui.zhang@intel.com>
Date	2017-01-24 08:10 +0100
Message-ID	<t33oS-1bE-9@gated-at.bofh.it>
In reply to	#1565371

On Mon, Jan 23, 2017 at 03:49:12PM -0800, Guenter Roeck wrote:
> On Tue, Jan 24, 2017 at 12:26:54AM +0100, Pavel Machek wrote:
> > Hi!
> > 
> > > > >I'll try to revert it on the top of v4.10-rc5 now... and yes, it fixes
> > > > >the issue.
> > > > >
> > > > >Any idea what went wrong and how to fix that?
> > > > >
> > > > >Anyway as we are at -rc5 and this is warning fix that caused a
> > > > >regression on different hardware... it should be reverted.
> > > > >
> > > > Agreed.
> > > > 
> > > > What exactly does "stopped working" mean ? That might help understanding
> > > > what went wrong.
> > > 
> > > /sys files related to battery no longer appear. I beieve this has
> > > something to do with it:
> > > 
> > > [    2.374877] of_get_named_gpiod_flags: parsed 'reset-gpios' property
> > > of node '/ocp@68000000/spi@48098000/tsc2005@0[0]' - status (0)
> > > [    2.375946] input: TSC2005 touchscreen as
> > > /devices/platform/68000000.ocp/48098000.spi/spi_master/spi1/spi1.0/input/input5
> > > [    2.392120] rx51-battery: probe of n900-battery failed with error
> > > -22
> > 
> > Mystery solved:
> > 
> > diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
> > index 3932f92..fe5ec82 100644
> > --- a/drivers/hwmon/hwmon.c
> > +++ b/drivers/hwmon/hwmon.c
> > @@ -545,8 +545,10 @@ __hwmon_device_register(struct device *dev, const char *name, void *drvdata,
> >  	int i, j, err, id;
> >  
> >  	/* Do not accept invalid characters in hwmon name attribute */
> > -	if (name && (!strlen(name) || strpbrk(name, "-* \t\n")))
> > +	if (name && (!strlen(name) || strpbrk(name, "-* \t\n"))) {
> > +		printk("hwmon: Invalid character detected: %s\n", name);
> >  		return ERR_PTR(-EINVAL);
> > +	}
> >  
> >  	id = ida_simple_get(&hwmon_ida, 0, 0, GFP_KERNEL);
> >  	if (id < 0)
> > 
> > 
> > pavel@n900:~$ dmesg | grep -5 Invalid
> > [    0.829650] of_get_named_gpiod_flags: parsed 'gpio-reset' property
> > of node '/ocp@68000000/i2c@48072000/tlv320aic3x@19[0]' - status (0)
> > [    0.833831] tsl2563 2-0029: model 7, rev. 0
> > [    0.837768] of_get_named_gpiod_flags: parsed 'enable-gpio' property
> > of node '/ocp@68000000/i2c@48072000/lp5523@32[0]' - status (0)
> > [    1.921417] omap_i2c 48072000.i2c: controller timed out
> > [    2.056823] lp5523x 2-0032: lp5523 Programmable led chip found
> > [    2.064147] hwmon: Invalid character detected: bq27200-0
> 
> So the problem is really that the thermal driver needs to create a valid name.
>
Right.

Before reverting, can you please try if this patch works or not?

thanks,
rui

From 79320ea3721314ec29ed6cdd6987ff922b11d6f9 Mon Sep 17 00:00:00 2001
From: Zhang Rui <rui.zhang@intel.com>
Date: Tue, 24 Jan 2017 14:11:03 +0800
Subject: [PATCH] thermal: fix parameter when registering hwmon

commit 7611fb68062f ("thermal: thermal_hwmon: Convert to
hwmon_device_register_with_info()") converts thermal core to use
hwmon_device_register_with_info() to register to hwmon instead of
deprecated hwmon_device_register().
But at the same time, the name of the hwmon device created is changed to
the thermal zone device name in this commit, which may contain
incompatible characters for hwmon.

Fixes it by using exactly the same parameters as before this commit.

Fixes: 7611fb68062f ("thermal: thermal_hwmon: Convert to hwmon_devce_register_with_info()")
Reported-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/thermal/thermal_hwmon.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/thermal_hwmon.c b/drivers/thermal/thermal_hwmon.c
index c4a508a..0c6789f 100644
--- a/drivers/thermal/thermal_hwmon.c
+++ b/drivers/thermal/thermal_hwmon.c
@@ -157,8 +157,8 @@ int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
 
 	INIT_LIST_HEAD(&hwmon->tz_list);
 	strlcpy(hwmon->type, tz->type, THERMAL_NAME_LENGTH);
-	hwmon->device = hwmon_device_register_with_info(NULL, hwmon->type,
-							hwmon, NULL, NULL);
+	hwmon->device = hwmon_device_register_with_info(NULL, NULL, NULL,
+							NULL, NULL);
 	if (IS_ERR(hwmon->device)) {
 		result = PTR_ERR(hwmon->device);
 		goto free_mem;
-- 
2.7.4

 
> Guenter
> 
> > [    2.064544] bq27xxx-battery 2-0055: failed to register battery
> > [    2.064605] bq27xxx-battery: probe of 2-0055 failed with error -22
> > [    2.065368] of_get_named_gpiod_flags: parsed 'power-gpio' property
> > of node '/ocp@68000000/i2c@48072000/tpa6130a2@60[0]' - status (0)
> > [    2.083221] bq2415x-charger 2-006b: automode supported, waiting for
> > events
> > [    2.084442] bq2415x-charger 2-006b: driver registered
> > --
> > [    2.369842] g_ether gadget: g_ether ready
> > [    2.377197] tsc2005 spi1.0: GPIO lookup for consumer reset
> > [    2.377227] tsc2005 spi1.0: using device tree for GPIO lookup
> > [    2.377288] of_get_named_gpiod_flags: parsed 'reset-gpios' property
> > of node '/ocp@68000000/spi@48098000/tsc2005@0[0]' - status (0)
> > [    2.378936] input: TSC2005 touchscreen as
> > /devices/platform/68000000.ocp/48098000.spi/spi_master/spi1/spi1.0/input/input5
> > [    2.395111] hwmon: Invalid character detected: rx51-battery
> > [    2.402587] rx51-battery: probe of n900-battery failed with error
> > -22
> > [    2.410247] omap_hsmmc 4809c000.mmc: GPIO lookup for consumer cd
> > [    2.410247] omap_hsmmc 4809c000.mmc: using device tree for GPIO
> > lookup
> > [    2.410278] of_get_named_gpiod_flags: can't parse 'cd-gpios'
> > property of node '/ocp@68000000/mmc@4809c000[0]'
> > [    2.410278] of_get_named_gpiod_flags: can't parse 'cd-gpio'
> > property of node '/ocp@68000000/mmc@4809c000[0]'
> > 
> > 									Pavel
> > 
> > 
> > -- 
> > (english) http://www.livejournal.com/~pavelmachek
> > (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
> 
>

[toc] | [prev] | [next] | [standalone]

#1565516

From	Pavel Machek <pavel@ucw.cz>
Date	2017-01-24 08:40 +0100
Message-ID	<t33RU-1lS-13@gated-at.bofh.it>
In reply to	#1565510

[Multipart message — attachments visible in raw view] — view raw

On Tue 2017-01-24 15:06:39, Zhang Rui wrote:
> On Mon, Jan 23, 2017 at 03:49:12PM -0800, Guenter Roeck wrote:
> > On Tue, Jan 24, 2017 at 12:26:54AM +0100, Pavel Machek wrote:
> > > Hi!
> > > 
> > > > > >I'll try to revert it on the top of v4.10-rc5 now... and yes, it fixes
> > > > > >the issue.
> > > > > >
> > > > > >Any idea what went wrong and how to fix that?
> > > > > >
> > > > > >Anyway as we are at -rc5 and this is warning fix that caused a
> > > > > >regression on different hardware... it should be reverted.
> > > > > >
> > > > > Agreed.
> > > > > 
> > > > > What exactly does "stopped working" mean ? That might help understanding
> > > > > what went wrong.
> > > > 
> > > > /sys files related to battery no longer appear. I beieve this has
> > > > something to do with it:
> > > > 
> > > > [    2.374877] of_get_named_gpiod_flags: parsed 'reset-gpios' property
> > > > of node '/ocp@68000000/spi@48098000/tsc2005@0[0]' - status (0)
> > > > [    2.375946] input: TSC2005 touchscreen as
> > > > /devices/platform/68000000.ocp/48098000.spi/spi_master/spi1/spi1.0/input/input5
> > > > [    2.392120] rx51-battery: probe of n900-battery failed with error
> > > > -22
> > > 
> > > Mystery solved:
> > > 
> > > diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
> > > index 3932f92..fe5ec82 100644
> > > --- a/drivers/hwmon/hwmon.c
> > > +++ b/drivers/hwmon/hwmon.c
> > > @@ -545,8 +545,10 @@ __hwmon_device_register(struct device *dev, const char *name, void *drvdata,
> > >  	int i, j, err, id;
> > >  
> > >  	/* Do not accept invalid characters in hwmon name attribute */
> > > -	if (name && (!strlen(name) || strpbrk(name, "-* \t\n")))
> > > +	if (name && (!strlen(name) || strpbrk(name, "-* \t\n"))) {
> > > +		printk("hwmon: Invalid character detected: %s\n", name);
> > >  		return ERR_PTR(-EINVAL);
> > > +	}
> > >  
> > >  	id = ida_simple_get(&hwmon_ida, 0, 0, GFP_KERNEL);
> > >  	if (id < 0)
> > > 
> > > 
> > > pavel@n900:~$ dmesg | grep -5 Invalid
> > > [    0.829650] of_get_named_gpiod_flags: parsed 'gpio-reset' property
> > > of node '/ocp@68000000/i2c@48072000/tlv320aic3x@19[0]' - status (0)
> > > [    0.833831] tsl2563 2-0029: model 7, rev. 0
> > > [    0.837768] of_get_named_gpiod_flags: parsed 'enable-gpio' property
> > > of node '/ocp@68000000/i2c@48072000/lp5523@32[0]' - status (0)
> > > [    1.921417] omap_i2c 48072000.i2c: controller timed out
> > > [    2.056823] lp5523x 2-0032: lp5523 Programmable led chip found
> > > [    2.064147] hwmon: Invalid character detected: bq27200-0
> > 
> > So the problem is really that the thermal driver needs to create a valid name.
> >
> Right.
> 
> Before reverting, can you please try if this patch works or not?

Not really. Revert now. Sorry.

Are you sure? This does not look equivalent to me at all.

"name" file handling moved from drivers to the core, which added some
crazy checks what name can contain. Even if this "works", what is the
expected effect on the "name" file?

								Pavel

> thanks,
> rui
> 
> >From 79320ea3721314ec29ed6cdd6987ff922b11d6f9 Mon Sep 17 00:00:00 2001
> From: Zhang Rui <rui.zhang@intel.com>
> Date: Tue, 24 Jan 2017 14:11:03 +0800
> Subject: [PATCH] thermal: fix parameter when registering hwmon
> 
> commit 7611fb68062f ("thermal: thermal_hwmon: Convert to
> hwmon_device_register_with_info()") converts thermal core to use
> hwmon_device_register_with_info() to register to hwmon instead of
> deprecated hwmon_device_register().
> But at the same time, the name of the hwmon device created is changed to
> the thermal zone device name in this commit, which may contain
> incompatible characters for hwmon.
> 
> Fixes it by using exactly the same parameters as before this commit.
> 
> Fixes: 7611fb68062f ("thermal: thermal_hwmon: Convert to hwmon_devce_register_with_info()")
> Reported-by: Pavel Machek <pavel@ucw.cz>
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
>  drivers/thermal/thermal_hwmon.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/thermal/thermal_hwmon.c b/drivers/thermal/thermal_hwmon.c
> index c4a508a..0c6789f 100644
> --- a/drivers/thermal/thermal_hwmon.c
> +++ b/drivers/thermal/thermal_hwmon.c
> @@ -157,8 +157,8 @@ int thermal_add_hwmon_sysfs(struct thermal_zone_device *tz)
>  
>  	INIT_LIST_HEAD(&hwmon->tz_list);
>  	strlcpy(hwmon->type, tz->type, THERMAL_NAME_LENGTH);
> -	hwmon->device = hwmon_device_register_with_info(NULL, hwmon->type,
> -							hwmon, NULL, NULL);
> +	hwmon->device = hwmon_device_register_with_info(NULL, NULL, NULL,
> +							NULL, NULL);
>  	if (IS_ERR(hwmon->device)) {
>  		result = PTR_ERR(hwmon->device);
>  		goto free_mem;

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1565889

From	Guenter Roeck <linux@roeck-us.net>
Date	2017-01-24 15:20 +0100
Message-ID	<t3a6Z-5lN-9@gated-at.bofh.it>
In reply to	#1565516

On 01/23/2017 11:37 PM, Pavel Machek wrote:
> On Tue 2017-01-24 15:06:39, Zhang Rui wrote:
>> On Mon, Jan 23, 2017 at 03:49:12PM -0800, Guenter Roeck wrote:
>>> On Tue, Jan 24, 2017 at 12:26:54AM +0100, Pavel Machek wrote:
>>>> Hi!
>>>>
>>>>>>> I'll try to revert it on the top of v4.10-rc5 now... and yes, it fixes
>>>>>>> the issue.
>>>>>>>
>>>>>>> Any idea what went wrong and how to fix that?
>>>>>>>
>>>>>>> Anyway as we are at -rc5 and this is warning fix that caused a
>>>>>>> regression on different hardware... it should be reverted.
>>>>>>>
>>>>>> Agreed.
>>>>>>
>>>>>> What exactly does "stopped working" mean ? That might help understanding
>>>>>> what went wrong.
>>>>>
>>>>> /sys files related to battery no longer appear. I beieve this has
>>>>> something to do with it:
>>>>>
>>>>> [    2.374877] of_get_named_gpiod_flags: parsed 'reset-gpios' property
>>>>> of node '/ocp@68000000/spi@48098000/tsc2005@0[0]' - status (0)
>>>>> [    2.375946] input: TSC2005 touchscreen as
>>>>> /devices/platform/68000000.ocp/48098000.spi/spi_master/spi1/spi1.0/input/input5
>>>>> [    2.392120] rx51-battery: probe of n900-battery failed with error
>>>>> -22
>>>>
>>>> Mystery solved:
>>>>
>>>> diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
>>>> index 3932f92..fe5ec82 100644
>>>> --- a/drivers/hwmon/hwmon.c
>>>> +++ b/drivers/hwmon/hwmon.c
>>>> @@ -545,8 +545,10 @@ __hwmon_device_register(struct device *dev, const char *name, void *drvdata,
>>>>  	int i, j, err, id;
>>>>
>>>>  	/* Do not accept invalid characters in hwmon name attribute */
>>>> -	if (name && (!strlen(name) || strpbrk(name, "-* \t\n")))
>>>> +	if (name && (!strlen(name) || strpbrk(name, "-* \t\n"))) {
>>>> +		printk("hwmon: Invalid character detected: %s\n", name);
>>>>  		return ERR_PTR(-EINVAL);
>>>> +	}
>>>>
>>>>  	id = ida_simple_get(&hwmon_ida, 0, 0, GFP_KERNEL);
>>>>  	if (id < 0)
>>>>
>>>>
>>>> pavel@n900:~$ dmesg | grep -5 Invalid
>>>> [    0.829650] of_get_named_gpiod_flags: parsed 'gpio-reset' property
>>>> of node '/ocp@68000000/i2c@48072000/tlv320aic3x@19[0]' - status (0)
>>>> [    0.833831] tsl2563 2-0029: model 7, rev. 0
>>>> [    0.837768] of_get_named_gpiod_flags: parsed 'enable-gpio' property
>>>> of node '/ocp@68000000/i2c@48072000/lp5523@32[0]' - status (0)
>>>> [    1.921417] omap_i2c 48072000.i2c: controller timed out
>>>> [    2.056823] lp5523x 2-0032: lp5523 Programmable led chip found
>>>> [    2.064147] hwmon: Invalid character detected: bq27200-0
>>>
>>> So the problem is really that the thermal driver needs to create a valid name.
>>>
>> Right.
>>
>> Before reverting, can you please try if this patch works or not?
>
> Not really. Revert now. Sorry.
>
> Are you sure? This does not look equivalent to me at all.
>
> "name" file handling moved from drivers to the core, which added some
> crazy checks what name can contain. Even if this "works", what is the
> expected effect on the "name" file?
>
The hwmon name attribute must not include '-', as documented in
Documentation/hwmon/sysfs-interface. Is enforcing that 'crazy' ?
Maybe in your world, but not in mine.

Guenter

[toc] | [prev] | [next] | [standalone]

#1566031

From	Pavel Machek <pavel@ucw.cz>
Date	2017-01-24 19:00 +0100
Message-ID	<t3dxU-7k1-19@gated-at.bofh.it>
In reply to	#1565889

[Multipart message — attachments visible in raw view] — view raw

> >>Right.
> >>
> >>Before reverting, can you please try if this patch works or not?
> >
> >Not really. Revert now. Sorry.
> >
> >Are you sure? This does not look equivalent to me at all.
> >
> >"name" file handling moved from drivers to the core, which added some
> >crazy checks what name can contain. Even if this "works", what is the
> >expected effect on the "name" file?
> >
> The hwmon name attribute must not include '-', as documented in
> Documentation/hwmon/sysfs-interface. Is enforcing that 'crazy' ?
> Maybe in your world, but not in mine.

Well, lets revert the patch and then we can discuss what to do with
the "name" problem.

Unfortunately, code enforces different rules than documentation says,
and it is all visible to userspace.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1566058

From	Guenter Roeck <linux@roeck-us.net>
Date	2017-01-24 19:50 +0100
Message-ID	<t3ekh-7Q2-11@gated-at.bofh.it>
In reply to	#1566031

On Tue, Jan 24, 2017 at 06:58:01PM +0100, Pavel Machek wrote:
> 
> > >>Right.
> > >>
> > >>Before reverting, can you please try if this patch works or not?
> > >
> > >Not really. Revert now. Sorry.
> > >
> > >Are you sure? This does not look equivalent to me at all.
> > >
> > >"name" file handling moved from drivers to the core, which added some
> > >crazy checks what name can contain. Even if this "works", what is the
> > >expected effect on the "name" file?
> > >
> > The hwmon name attribute must not include '-', as documented in
> > Documentation/hwmon/sysfs-interface. Is enforcing that 'crazy' ?
> > Maybe in your world, but not in mine.
> 
> Well, lets revert the patch and then we can discuss what to do with
> the "name" problem.
> 
Not sure what the problem to be discussed is. Please provide details.

> Unfortunately, code enforces different rules than documentation says,

Please specify your problem with rules vs. documentation. You mean the
additional checks which also block wildcards and spaces/newline in the
name ? Are you having a problem with those ? What are those problems ?
I'll be more than happy to update the documentation if your problem
is with that mismatch.

To provide some detail: libsensors gets just as confused with wildcards
and whitespace/newline as it does with '-' in the reported name, which
is why those are blocked by the new API.

> and it is all visible to userspace.
> 
We are talking about a value reported by an attribute, not about an attribute.
Maybe the ABI Police thinks differently, but I do not count fixing the value
reported by an attribute as an ABI change.

Effecively, what this enforcement does is to make userspace actually work.
Ultimately that means, per your standard, that fixing values reported by the
kernel is not permitted under any circumstances, even if such changes result
in userspace actually working as intended as a result of that change.
I would argue that such an enforcement goes a little bit too far.

In addition, there is actually another problem solved by the patch you want to
have reverted: The current thermal code has another problem, which is that it
creates the name attribute _after_ registering the hwmon device. The udev event
for creating the hwmon device thus occurs before its mandatory name attribute
(and any other attribute, for that matter) exists, meaning that any userspace
handler acting on it would be completely unpredictable. Now _this_ is a real
ABI "change", since it fixes a race condition. I guess you are going to argue
that this isn't permitted either ?

Guenter

[toc] | [prev] | [next] | [standalone]

#1566216

From	Pavel Machek <pavel@ucw.cz>
Date	2017-01-24 23:50 +0100
Message-ID	<t3i4x-1UH-13@gated-at.bofh.it>
In reply to	#1566058

[Multipart message — attachments visible in raw view] — view raw

On Tue 2017-01-24 10:45:26, Guenter Roeck wrote:
> On Tue, Jan 24, 2017 at 06:58:01PM +0100, Pavel Machek wrote:
> > 
> > > >>Right.
> > > >>
> > > >>Before reverting, can you please try if this patch works or not?
> > > >
> > > >Not really. Revert now. Sorry.
> > > >
> > > >Are you sure? This does not look equivalent to me at all.
> > > >
> > > >"name" file handling moved from drivers to the core, which added some
> > > >crazy checks what name can contain. Even if this "works", what is the
> > > >expected effect on the "name" file?
> > > >
> > > The hwmon name attribute must not include '-', as documented in
> > > Documentation/hwmon/sysfs-interface. Is enforcing that 'crazy' ?
> > > Maybe in your world, but not in mine.
> > 
> > Well, lets revert the patch and then we can discuss what to do with
> > the "name" problem.
> > 
> Not sure what the problem to be discussed is. Please provide
> details.

See below.

> > Unfortunately, code enforces different rules than documentation says,
> 
> Please specify your problem with rules vs. documentation. You mean the
> additional checks which also block wildcards and spaces/newline in the
> name ? Are you having a problem with those ? What are those problems ?
> I'll be more than happy to update the documentation if your problem
> is with that mismatch.

Yes, it sounds like documentation should be updated.

> To provide some detail: libsensors gets just as confused with wildcards
> and whitespace/newline as it does with '-' in the reported name, which
> is why those are blocked by the new API.

> > and it is all visible to userspace.
> > 
> We are talking about a value reported by an attribute, not about an attribute.
> Maybe the ABI Police thinks differently, but I do not count fixing the value
> reported by an attribute as an ABI change.

The rule is not what you think it is. It does not matter if it is
attribute or value. It is about breaking userspace.

And this is the attribute userland will use to search for sensor it
works with, right? So changing this attribute will break userspace if
someone uses it. N900 already > 2 temperature sensors on, so someone
may actually be using it.

> Effecively, what this enforcement does is to make userspace actually work.
> Ultimately that means, per your standard, that fixing values reported by the
> kernel is not permitted under any circumstances, even if such changes result
> in userspace actually working as intended as a result of that change.
> I would argue that such an enforcement goes a little bit too far.

You can argue whatever you want, but the rule is "no regressions". You
may not break userspace app A to fix userspace app B.

> In addition, there is actually another problem solved by the patch you want to
> have reverted: The current thermal code has another problem, which is that it
> creates the name attribute _after_ registering the hwmon device. The udev event
> for creating the hwmon device thus occurs before its mandatory name attribute
> (and any other attribute, for that matter) exists, meaning that any userspace
> handler acting on it would be completely unpredictable. Now _this_ is a real
> ABI "change", since it fixes a race condition. I guess you are going to argue
> that this isn't permitted either ?

You guessed wrong. It would only be problem if somone relied on that
race condition.

Anyway, I reported a rather serious regression in -rc5. The original
patch was obviously not understood enough, and it broke charging on my
hardware. I'd argue it had no reason to be in -rc5 in the first place.

I believe solution is obvious. Revert the damn patch. I actually
wonder why we are discussing it.

Maintainer suggested different solution, but you said it was broken.

You are suggesting modification of two driver names on N900. You don't
know what sideeffects it might have. You don't know if there are more
driver names to fix. You did not offer a patch.

It is an ugly regression and it is -rc5 time. Actually not your
problem, but Fabio Estevam or Zhang Rui has to deal with it soon.

Thanks,

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1566222

From	Fabio Estevam <festevam@gmail.com>
Date	2017-01-25 00:10 +0100
Message-ID	<t3inU-2hx-29@gated-at.bofh.it>
In reply to	#1566216

On Tue, Jan 24, 2017 at 8:46 PM, Pavel Machek <pavel@ucw.cz> wrote:

> It is an ugly regression and it is -rc5 time. Actually not your
> problem, but Fabio Estevam or Zhang Rui has to deal with it soon.

I have already sent the revert and it was acked-by you and Guenter.

[toc] | [prev] | [next] | [standalone]

#1566330

From	Zhang Rui <rui.zhang@intel.com>
Date	2017-01-25 06:40 +0100
Message-ID	<t3otj-66T-7@gated-at.bofh.it>
In reply to	#1566222

On Tue, 2017-01-24 at 21:07 -0200, Fabio Estevam wrote:
> On Tue, Jan 24, 2017 at 8:46 PM, Pavel Machek <pavel@ucw.cz> wrote:
> 
> > 
> > It is an ugly regression and it is -rc5 time. Actually not your
> > problem, but Fabio Estevam or Zhang Rui has to deal with it soon.
> I have already sent the revert and it was acked-by you and Guenter.

Revert patch has been applied and queued for next -rc.

thanks,
rui

[toc] | [prev] | [next] | [standalone]

#1566450

From	Pavel Machek <pavel@ucw.cz>
Date	2017-01-25 11:20 +0100
Message-ID	<t3sQi-sW-21@gated-at.bofh.it>
In reply to	#1566330

[Multipart message — attachments visible in raw view] — view raw

On Wed 2017-01-25 13:29:52, Zhang Rui wrote:
> On Tue, 2017-01-24 at 21:07 -0200, Fabio Estevam wrote:
> > On Tue, Jan 24, 2017 at 8:46 PM, Pavel Machek <pavel@ucw.cz> wrote:
> > 
> > > 
> > > It is an ugly regression and it is -rc5 time. Actually not your
> > > problem, but Fabio Estevam or Zhang Rui has to deal with it soon.
> > I have already sent the revert and it was acked-by you and Guenter.
> 
> Revert patch has been applied and queued for next -rc.

Thanks!
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1566503

From	Pavel Machek <pavel@ucw.cz>
Date	2017-01-25 12:20 +0100
Message-ID	<t3tMl-136-7@gated-at.bofh.it>
In reply to	#1566058

[Multipart message — attachments visible in raw view] — view raw

Hi!

> > > >>Right.
> > > >>
> > > >>Before reverting, can you please try if this patch works or not?
> > > >
> > > >Not really. Revert now. Sorry.
> > > >
> > > >Are you sure? This does not look equivalent to me at all.
> > > >
> > > >"name" file handling moved from drivers to the core, which added some
> > > >crazy checks what name can contain. Even if this "works", what is the
> > > >expected effect on the "name" file?
> > > >
> > > The hwmon name attribute must not include '-', as documented in
> > > Documentation/hwmon/sysfs-interface. Is enforcing that 'crazy' ?
> > > Maybe in your world, but not in mine.
> > 
> > Well, lets revert the patch and then we can discuss what to do with
> > the "name" problem.

Ok, so the patch is on the way in. What to do next?

pavel@n900:/sys/class/hwmon$ cat hwmon0/name
bq27200-0
pavel@n900:/sys/class/hwmon$ cat hwmon1/name
rx51-battery

> To provide some detail: libsensors gets just as confused with wildcards
> and whitespace/newline as it does with '-' in the reported name, which
> is why those are blocked by the new API.

Ok... Question is "does someone actually use hwmon*/name on N900"? If
so, we can't change it, but it is well possible that noone is.

Next question is .. are there other drivers affected? Do we want to do
'-' -> '_' in the core or somewhere in the drivers? We might want to
do the change in early in 4.11 and see what breaks....

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1566535

From	Guenter Roeck <linux@roeck-us.net>
Date	2017-01-25 13:00 +0100
Message-ID	<t3up4-1i3-33@gated-at.bofh.it>
In reply to	#1566503

On 01/25/2017 03:12 AM, Pavel Machek wrote:
> Hi!
>
>>>>>> Right.
>>>>>>
>>>>>> Before reverting, can you please try if this patch works or not?
>>>>>
>>>>> Not really. Revert now. Sorry.
>>>>>
>>>>> Are you sure? This does not look equivalent to me at all.
>>>>>
>>>>> "name" file handling moved from drivers to the core, which added some
>>>>> crazy checks what name can contain. Even if this "works", what is the
>>>>> expected effect on the "name" file?
>>>>>
>>>> The hwmon name attribute must not include '-', as documented in
>>>> Documentation/hwmon/sysfs-interface. Is enforcing that 'crazy' ?
>>>> Maybe in your world, but not in mine.
>>>
>>> Well, lets revert the patch and then we can discuss what to do with
>>> the "name" problem.
>
> Ok, so the patch is on the way in. What to do next?
>
> pavel@n900:/sys/class/hwmon$ cat hwmon0/name
> bq27200-0
> pavel@n900:/sys/class/hwmon$ cat hwmon1/name
> rx51-battery
>

The 'name' parameter will be mandatory for hwmon_device_register_with_groups()
and hwmon_device_register_with_info() starting with v4.11, and the API
documentation will be updated to list all invalid characters.

The thermal subsystem doesn't play well when it comes to hwmon userspace
interaction (generating and removing sensor attributes dynamically
messes up pretty much everything, and generating the name attribute after
hwmon registration for sure messes up any udev listener). libsensors gets
confused by '-' in the 'name' attribute. libsensors also doesn't expect
sensor attributes to be dynamically generated and removed, and may
react unpredictably if they are. This all means that any existing users
of hwmon devices created by the thermal subsystem will most likely not
use libsensors nor udev listeners. At the same time, it seems to be quite
important at least to you that the '-' in the name is retained.

Given all that, maybe thermal should just stick with the deprecated API
and accept the deprecated warning message (or maybe drop hwmon support
altogether).

Guenter

[toc] | [prev] | [next] | [standalone]

Page 1 of 3 [1] 2 3 Next page →

csiph-web

v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900

Contents

#1564957 — v4.10-rc4 to v4.10-rc5: battery regression on Nokia N900

#1564960

#1565001

#1564999

#1565006

#1565013

#1565171

#1565364

#1565371

#1565510

#1565516

#1565889

#1566031

#1566058

#1566216

#1566222

#1566330

#1566450

#1566503

#1566535