diff mbox series

[v2] platform/x86: asus-wmi: Support laptops where the first battery is named BATT

Message ID 20200223181832.17131-1-kristian@klausen.dk (mailing list archive)
State Superseded, archived
Headers show
Series [v2] platform/x86: asus-wmi: Support laptops where the first battery is named BATT | expand

Commit Message

Kristian Klausen Feb. 23, 2020, 6:18 p.m. UTC 
The WMI method to set the charge threshold does not provide a
way to specific a battery, so we assume it is the first/primary
battery (by checking if the name is BAT0).
On some newer ASUS laptops (Zenbook UM431DA) though, the
primary/first battery isn't named BAT0 but BATT, so we need
to support that case.

Signed-off-by: Kristian Klausen <kristian@klausen.dk>
Cc: stable@vger.kernel.org
---
I'm not sure if this is candidate for -stable, it fix a real bug
(charge threshold doesn't work on newer ASUS laptops) which has been
reported by a user[1], but is that enough?
I had a quick look at[2], can this be considered a "something
critical"? It "bothers people"[1]. My point: I'm not sure..

I'm unsure if there is a bettery way to fix this. Maybe a counter
would be better (+1 for every new battery)? It would probably need
to be atomic to prevent race condition (I'm not sure how this code
is run), but this "fix" is way simpler.

Please do not accept this patch just yet, I'm waiting for the tester
to either confirm or deny credit[3].

[1] https://gist.github.com/klausenbusk/643f15320ae8997427155c38be13e445#gistcomment-3186025
[2] https://www.kernel.org/doc/html/v5.5/process/stable-kernel-rules.html
[3] https://gist.github.com/klausenbusk/643f15320ae8997427155c38be13e445#gistcomment-3186429

v2:
Add base commit
Remove uneeded parenthesis and adjust styling

 drivers/platform/x86/asus-wmi.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)


base-commit: 11a48a5a18c63fd7621bb050228cebf13566e4d8

Comments

Sasha Levin Feb. 24, 2020, 1:10 a.m. UTC | #1 
Hi,

[This is an automated email]

This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all

The bot has tested the following trees: v5.5.5, v5.4.21, v4.19.105, v4.14.171, v4.9.214, v4.4.214.

v5.5.5: Build OK!
v5.4.21: Build OK!
v4.19.105: Failed to apply! Possible dependencies:
    11e87702be65 ("PCI: pciehp: Differentiate between surprise and safe removal")
    125450f81441 ("PCI: hotplug: Embed hotplug_slot")
    5790a9c78e78 ("PCI: pciehp: Unify controller and slot structs")
    7973353e92ee ("platform/x86: asus-wmi: Refactor charge threshold to use the battery hooking API")
    80696f991424 ("PCI: pciehp: Tolerate Presence Detect hardwired to zero")
    81c4b5bf30de ("PCI: hotplug: Constify hotplug_slot_ops")
    a7da21613c4e ("PCI: hotplug: Drop hotplug_slot_info")
    b096f626a682 ("platform/x86: asus-wmi: Switch fan boost mode")
    eee6e273843d ("PCI: pciehp: Drop hotplug_slot_ops wrappers")

v4.14.171: Failed to apply! Possible dependencies:
    125450f81441 ("PCI: hotplug: Embed hotplug_slot")
    4aed1cd6fb95 ("PCI: pciehp: Document struct slot and struct controller")
    51bbf9bee34f ("PCI: hotplug: Demidlayer registration with the core")
    5790a9c78e78 ("PCI: pciehp: Unify controller and slot structs")
    7973353e92ee ("platform/x86: asus-wmi: Refactor charge threshold to use the battery hooking API")
    81c4b5bf30de ("PCI: hotplug: Constify hotplug_slot_ops")
    97c6f25d5828 ("PCI/hotplug: ppc: correct a php_slot usage after free")
    a7da21613c4e ("PCI: hotplug: Drop hotplug_slot_info")
    b096f626a682 ("platform/x86: asus-wmi: Switch fan boost mode")
    c7abb2352c29 ("PCI: Remove unnecessary messages for memory allocation failures")
    dbb3d78f61ba ("platform/x86: asus-wmi: Call led hw_changed API on kbd brightness change")
    ed99d29b2b15 ("platform/x86: asus-wmi: Add keyboard backlight toggle support")

v4.9.214: Failed to apply! Possible dependencies:
    125450f81441 ("PCI: hotplug: Embed hotplug_slot")
    4aed1cd6fb95 ("PCI: pciehp: Document struct slot and struct controller")
    51bbf9bee34f ("PCI: hotplug: Demidlayer registration with the core")
    5790a9c78e78 ("PCI: pciehp: Unify controller and slot structs")
    7973353e92ee ("platform/x86: asus-wmi: Refactor charge threshold to use the battery hooking API")
    81c4b5bf30de ("PCI: hotplug: Constify hotplug_slot_ops")
    97c6f25d5828 ("PCI/hotplug: ppc: correct a php_slot usage after free")
    a7da21613c4e ("PCI: hotplug: Drop hotplug_slot_info")
    b096f626a682 ("platform/x86: asus-wmi: Switch fan boost mode")
    c7abb2352c29 ("PCI: Remove unnecessary messages for memory allocation failures")
    dbb3d78f61ba ("platform/x86: asus-wmi: Call led hw_changed API on kbd brightness change")
    ed99d29b2b15 ("platform/x86: asus-wmi: Add keyboard backlight toggle support")

v4.4.214: Failed to apply! Possible dependencies:
    125450f81441 ("PCI: hotplug: Embed hotplug_slot")
    2ac83cccabbc ("PCI: hotplug: Use list_for_each_entry() to simplify code")
    51bbf9bee34f ("PCI: hotplug: Demidlayer registration with the core")
    66725152fb9f ("PCI/hotplug: PowerPC PowerNV PCI hotplug driver")
    7973353e92ee ("platform/x86: asus-wmi: Refactor charge threshold to use the battery hooking API")
    89379f165a1b ("PCI/hotplug: pnv_php: export symbols and move struct types needed by cxl")
    97c6f25d5828 ("PCI/hotplug: ppc: correct a php_slot usage after free")
    a7da21613c4e ("PCI: hotplug: Drop hotplug_slot_info")
    b096f626a682 ("platform/x86: asus-wmi: Switch fan boost mode")
    dbb3d78f61ba ("platform/x86: asus-wmi: Call led hw_changed API on kbd brightness change")
    ed99d29b2b15 ("platform/x86: asus-wmi: Add keyboard backlight toggle support")
    ef69b03dfd32 ("MAINTAINERS: Add powerpc drivers to the powerpc section")
    ff3ce480e8b5 ("PCI: Fix all whitespace issues")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?
Kristian Klausen Feb. 24, 2020, 1:14 a.m. UTC | #2 
On 24.02.2020 02.10, Sasha Levin wrote:
> Hi,
>
> [This is an automated email]
>
> This commit has been processed because it contains a -stable tag.
> The stable tag indicates that it's relevant for the following trees: all
>
> The bot has tested the following trees: v5.5.5, v5.4.21, v4.19.105, v4.14.171, v4.9.214, v4.4.214.
>
> v5.5.5: Build OK!
> v5.4.21: Build OK!
> v4.19.105: Failed to apply! Possible dependencies:
>      11e87702be65 ("PCI: pciehp: Differentiate between surprise and safe removal")
>      125450f81441 ("PCI: hotplug: Embed hotplug_slot")
>      5790a9c78e78 ("PCI: pciehp: Unify controller and slot structs")
>      7973353e92ee ("platform/x86: asus-wmi: Refactor charge threshold to use the battery hooking API")
>      80696f991424 ("PCI: pciehp: Tolerate Presence Detect hardwired to zero")
>      81c4b5bf30de ("PCI: hotplug: Constify hotplug_slot_ops")
>      a7da21613c4e ("PCI: hotplug: Drop hotplug_slot_info")
>      b096f626a682 ("platform/x86: asus-wmi: Switch fan boost mode")
>      eee6e273843d ("PCI: pciehp: Drop hotplug_slot_ops wrappers")
>
> v4.14.171: Failed to apply! Possible dependencies:
>      125450f81441 ("PCI: hotplug: Embed hotplug_slot")
>      4aed1cd6fb95 ("PCI: pciehp: Document struct slot and struct controller")
>      51bbf9bee34f ("PCI: hotplug: Demidlayer registration with the core")
>      5790a9c78e78 ("PCI: pciehp: Unify controller and slot structs")
>      7973353e92ee ("platform/x86: asus-wmi: Refactor charge threshold to use the battery hooking API")
>      81c4b5bf30de ("PCI: hotplug: Constify hotplug_slot_ops")
>      97c6f25d5828 ("PCI/hotplug: ppc: correct a php_slot usage after free")
>      a7da21613c4e ("PCI: hotplug: Drop hotplug_slot_info")
>      b096f626a682 ("platform/x86: asus-wmi: Switch fan boost mode")
>      c7abb2352c29 ("PCI: Remove unnecessary messages for memory allocation failures")
>      dbb3d78f61ba ("platform/x86: asus-wmi: Call led hw_changed API on kbd brightness change")
>      ed99d29b2b15 ("platform/x86: asus-wmi: Add keyboard backlight toggle support")
>
> v4.9.214: Failed to apply! Possible dependencies:
>      125450f81441 ("PCI: hotplug: Embed hotplug_slot")
>      4aed1cd6fb95 ("PCI: pciehp: Document struct slot and struct controller")
>      51bbf9bee34f ("PCI: hotplug: Demidlayer registration with the core")
>      5790a9c78e78 ("PCI: pciehp: Unify controller and slot structs")
>      7973353e92ee ("platform/x86: asus-wmi: Refactor charge threshold to use the battery hooking API")
>      81c4b5bf30de ("PCI: hotplug: Constify hotplug_slot_ops")
>      97c6f25d5828 ("PCI/hotplug: ppc: correct a php_slot usage after free")
>      a7da21613c4e ("PCI: hotplug: Drop hotplug_slot_info")
>      b096f626a682 ("platform/x86: asus-wmi: Switch fan boost mode")
>      c7abb2352c29 ("PCI: Remove unnecessary messages for memory allocation failures")
>      dbb3d78f61ba ("platform/x86: asus-wmi: Call led hw_changed API on kbd brightness change")
>      ed99d29b2b15 ("platform/x86: asus-wmi: Add keyboard backlight toggle support")
>
> v4.4.214: Failed to apply! Possible dependencies:
>      125450f81441 ("PCI: hotplug: Embed hotplug_slot")
>      2ac83cccabbc ("PCI: hotplug: Use list_for_each_entry() to simplify code")
>      51bbf9bee34f ("PCI: hotplug: Demidlayer registration with the core")
>      66725152fb9f ("PCI/hotplug: PowerPC PowerNV PCI hotplug driver")
>      7973353e92ee ("platform/x86: asus-wmi: Refactor charge threshold to use the battery hooking API")
>      89379f165a1b ("PCI/hotplug: pnv_php: export symbols and move struct types needed by cxl")
>      97c6f25d5828 ("PCI/hotplug: ppc: correct a php_slot usage after free")
>      a7da21613c4e ("PCI: hotplug: Drop hotplug_slot_info")
>      b096f626a682 ("platform/x86: asus-wmi: Switch fan boost mode")
>      dbb3d78f61ba ("platform/x86: asus-wmi: Call led hw_changed API on kbd brightness change")
>      ed99d29b2b15 ("platform/x86: asus-wmi: Add keyboard backlight toggle support")
>      ef69b03dfd32 ("MAINTAINERS: Add powerpc drivers to the powerpc section")
>      ff3ce480e8b5 ("PCI: Fix all whitespace issues")
>
>
> NOTE: The patch will not be queued to stable trees until it is upstream.
>
> How should we proceed with this patch?

The patch should only be applied to the v5.4 and v5.5 trees.
- Kristian
Kristian Klausen Feb. 25, 2020, 9:15 a.m. UTC | #3 
On 23.02.2020 19.18, Kristian Klausen wrote:
> The WMI method to set the charge threshold does not provide a
> way to specific a battery, so we assume it is the first/primary
> battery (by checking if the name is BAT0).
> On some newer ASUS laptops (Zenbook UM431DA) though, the
> primary/first battery isn't named BAT0 but BATT, so we need
> to support that case.
>
> Signed-off-by: Kristian Klausen <kristian@klausen.dk>
> Cc: stable@vger.kernel.org
> ---
> I'm not sure if this is candidate for -stable, it fix a real bug
> (charge threshold doesn't work on newer ASUS laptops) which has been
> reported by a user[1], but is that enough?
> I had a quick look at[2], can this be considered a "something
> critical"? It "bothers people"[1]. My point: I'm not sure..
>
> I'm unsure if there is a bettery way to fix this. Maybe a counter
> would be better (+1 for every new battery)? It would probably need
> to be atomic to prevent race condition (I'm not sure how this code
> is run), but this "fix" is way simpler.
>
> Please do not accept this patch just yet, I'm waiting for the tester
> to either confirm or deny credit[3].

Please add:
Reported-by: Alberto Gomez Marin
Tested-by: Alberto Gomez Marin

Is a email required? The tester prefer not to disclose his email 
address[1]. The tester didn't test this exact patch, but the only change 
is a line break and no parenthesis, so it should be the same[2].
[1] 
https://gist.github.com/klausenbusk/643f15320ae8997427155c38be13e445#gistcomment-3187929
[2] 
https://gist.github.com/klausenbusk/643f15320ae8997427155c38be13e445#gistcomment-3185442

>
> [1] https://gist.github.com/klausenbusk/643f15320ae8997427155c38be13e445#gistcomment-3186025
> [2] https://www.kernel.org/doc/html/v5.5/process/stable-kernel-rules.html
> [3] https://gist.github.com/klausenbusk/643f15320ae8997427155c38be13e445#gistcomment-3186429
>
> v2:
> Add base commit
> Remove uneeded parenthesis and adjust styling
>
>   drivers/platform/x86/asus-wmi.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index 612ef5526226..01becbe2a9a8 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -426,8 +426,11 @@ static int asus_wmi_battery_add(struct power_supply *battery)
>   {
>   	/* The WMI method does not provide a way to specific a battery, so we
>   	 * just assume it is the first battery.
> +	 * Note: On some newer ASUS laptops (Zenbook UM431DA), the primary/first
> +	 * battery is named BATT.
>   	 */
> -	if (strcmp(battery->desc->name, "BAT0") != 0)
> +	if (strcmp(battery->desc->name, "BAT0") != 0 &&
> +	    strcmp(battery->desc->name, "BATT") != 0)
>   		return -ENODEV;
>   
>   	if (device_create_file(&battery->dev,
>
> base-commit: 11a48a5a18c63fd7621bb050228cebf13566e4d8
Andy Shevchenko Feb. 25, 2020, 9:30 a.m. UTC | #4 
On Mon, Feb 24, 2020 at 3:15 AM Kristian Klausen <kristian@klausen.dk> wrote:
> On 24.02.2020 02.10, Sasha Levin wrote:

> > NOTE: The patch will not be queued to stable trees until it is upstream.
> >
> > How should we proceed with this patch?
>
> The patch should only be applied to the v5.4 and v5.5 trees.

I'm not sure I got this right. Do we have already this change in upstream?
I don't see it there. So, why there is no mention of the v5.6 and
current upstream in above list?
Kristian Klausen Feb. 25, 2020, 9:51 a.m. UTC | #5 
On 25.02.2020 10.30, Andy Shevchenko wrote:
> On Mon, Feb 24, 2020 at 3:15 AM Kristian Klausen <kristian@klausen.dk> wrote:
>> On 24.02.2020 02.10, Sasha Levin wrote:
>>> NOTE: The patch will not be queued to stable trees until it is upstream.
>>>
>>> How should we proceed with this patch?
>> The patch should only be applied to the v5.4 and v5.5 trees.
> I'm not sure I got this right. Do we have already this change in upstream?
> I don't see it there. So, why there is no mention of the v5.6 and
> current upstream in above list?

Sorry about that, my response does not make any sense.
The change isn't upstream yet, and should be applied upstream first and 
the 5.4 and 5.6 tree tree if possible.
Was I wrong CC'ing stable@vger.kernel.org? (suggested by git send-email 
due to "Cc: stable@vger.kernel.org")
Andy Shevchenko Feb. 25, 2020, 9:55 a.m. UTC | #6 
On Tue, Feb 25, 2020 at 11:51 AM Kristian Klausen <kristian@klausen.dk> wrote:
> On 25.02.2020 10.30, Andy Shevchenko wrote:
> > On Mon, Feb 24, 2020 at 3:15 AM Kristian Klausen <kristian@klausen.dk> wrote:
> >> On 24.02.2020 02.10, Sasha Levin wrote:
> >>> NOTE: The patch will not be queued to stable trees until it is upstream.
> >>>
> >>> How should we proceed with this patch?
> >> The patch should only be applied to the v5.4 and v5.5 trees.
> > I'm not sure I got this right. Do we have already this change in upstream?
> > I don't see it there. So, why there is no mention of the v5.6 and
> > current upstream in above list?
>
> Sorry about that, my response does not make any sense.
> The change isn't upstream yet, and should be applied upstream first and
> the 5.4 and 5.6 tree tree if possible.

The usual pattern is to add Fixes tag and Cc: stable@.

> Was I wrong CC'ing stable@vger.kernel.org? (suggested by git send-email
> due to "Cc: stable@vger.kernel.org")
Andy Shevchenko March 3, 2020, 2:46 p.m. UTC | #7 
On Tue, Feb 25, 2020 at 11:55 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Tue, Feb 25, 2020 at 11:51 AM Kristian Klausen <kristian@klausen.dk> wrote:

...

> > Sorry about that, my response does not make any sense.
> > The change isn't upstream yet, and should be applied upstream first and
> > the 5.4 and 5.6 tree tree if possible.
>
> The usual pattern is to add Fixes tag and Cc: stable@.
>
> > Was I wrong CC'ing stable@vger.kernel.org? (suggested by git send-email
> > due to "Cc: stable@vger.kernel.org")

Kristian, to be clear, I'm waiting for v3 with appropriate tag and Cc
list applied.
Kristian Klausen March 3, 2020, 6:03 p.m. UTC | #8 
On 03.03.2020 15.46, Andy Shevchenko wrote:
> On Tue, Feb 25, 2020 at 11:55 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>> On Tue, Feb 25, 2020 at 11:51 AM Kristian Klausen <kristian@klausen.dk> wrote:
> ...
>
>>> Sorry about that, my response does not make any sense.
>>> The change isn't upstream yet, and should be applied upstream first and
>>> the 5.4 and 5.6 tree tree if possible.
>> The usual pattern is to add Fixes tag and Cc: stable@.
>>
>>> Was I wrong CC'ing stable@vger.kernel.org? (suggested by git send-email
>>> due to "Cc: stable@vger.kernel.org")
> Kristian, to be clear, I'm waiting for v3 with appropriate tag and Cc
> list applied.

Oh, I forgot. I just sent a V3 with "Fixes".
diff mbox series

Patch

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 612ef5526226..01becbe2a9a8 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -426,8 +426,11 @@  static int asus_wmi_battery_add(struct power_supply *battery)
 {
 	/* The WMI method does not provide a way to specific a battery, so we
 	 * just assume it is the first battery.
+	 * Note: On some newer ASUS laptops (Zenbook UM431DA), the primary/first
+	 * battery is named BATT.
 	 */
-	if (strcmp(battery->desc->name, "BAT0") != 0)
+	if (strcmp(battery->desc->name, "BAT0") != 0 &&
+	    strcmp(battery->desc->name, "BATT") != 0)
 		return -ENODEV;
 
 	if (device_create_file(&battery->dev,