[PATCH 2/3 v6] battery: Add the ThinkPad "Not Charging" quirk

[PATCH 2/3 v6] battery: Add the ThinkPad "Not Charging" quirk

[Ognjen Galic]

The EC/ACPI firmware on Lenovo ThinkPads used to report a status
of "Unknown" when the battery is between the charge start and
charge stop thresholds. On Windows, it reports "Not Charging"
so the quirk has been added to also report correctly.

Now the "status" attribute returns "Not Charging" when the
battery on ThinkPads is not physicaly charging.

Tested-by: Kevin Locke <kevin@kevinlocke.name>
Tested-by: Christoph Böhmwalder <christoph@boehmwalder.at>
Signed-off-by: Ognjen Galic <smclt30p@gmail.com>
---
 drivers/acpi/battery.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 2951d07..81e9b4e 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -71,6 +71,7 @@ static async_cookie_t async_cookie;
 static bool battery_driver_registered;
 static int battery_bix_broken_package;
 static int battery_notification_delay_ms;
+static int battery_quirk_thinkpad_notcharging;
 static unsigned int cache_time = 1000;
 module_param(cache_time, uint, 0644);
 MODULE_PARM_DESC(cache_time, "cache time in milliseconds");
@@ -222,6 +223,13 @@ static int acpi_battery_get_property(struct power_supply *psy,
 			val->intval = POWER_SUPPLY_STATUS_CHARGING;
 		else if (acpi_battery_is_charged(battery))
 			val->intval = POWER_SUPPLY_STATUS_FULL;
+		/*
+		 * On the Lenovo ThinkPad ACPI implementation, when
+		 * neither bits 0 or 1 are set, that state is
+		 * considered as "Not Charging".
+		 */
+		else if (battery_quirk_thinkpad_notcharging)
+			val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
 		else
 			val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
 		break;
@@ -1301,6 +1309,13 @@ battery_notification_delay_quirk(const struct dmi_system_id *d)
 	return 0;
 }
 
+static int __init
+battery_quirk_not_charging(const struct dmi_system_id *d)
+{
+	battery_quirk_thinkpad_notcharging = 1;
+	return 0;
+}
+
 static const struct dmi_system_id bat_dmi_table[] __initconst = {
 	{
 		.callback = battery_bix_broken_package_quirk,
@@ -1318,6 +1333,14 @@ static const struct dmi_system_id bat_dmi_table[] __initconst = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "Aspire V5-573G"),
 		},
 	},
+	{
+		.callback = battery_quirk_not_charging,
+		.ident = "Lenovo ThinkPad",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+			DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad"),
+		},
+	},
 	{},
 };
 
-- 
2.7.4

Re: [PATCH 2/3 v6] battery: Add the ThinkPad "Not Charging" quirk

[Rafael J. Wysocki]

On Fri, Dec 15, 2017 at 5:57 PM, Ognjen Galic <smclt30p@gmail.com> wrote:
> The EC/ACPI firmware on Lenovo ThinkPads used to report a status
> of "Unknown" when the battery is between the charge start and
> charge stop thresholds. On Windows, it reports "Not Charging"
> so the quirk has been added to also report correctly.
>
> Now the "status" attribute returns "Not Charging" when the
> battery on ThinkPads is not physicaly charging.
>
> Tested-by: Kevin Locke <kevin@kevinlocke.name>
> Tested-by: Christoph Böhmwalder <christoph@boehmwalder.at>
> Signed-off-by: Ognjen Galic <smclt30p@gmail.com>

It doesn't look like this is related to the [1/3] and [3/3], so why do
you make it part of the series?

> ---
>  drivers/acpi/battery.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
> index 2951d07..81e9b4e 100644
> --- a/drivers/acpi/battery.c
> +++ b/drivers/acpi/battery.c
> @@ -71,6 +71,7 @@ static async_cookie_t async_cookie;
>  static bool battery_driver_registered;
>  static int battery_bix_broken_package;
>  static int battery_notification_delay_ms;
> +static int battery_quirk_thinkpad_notcharging;

Drop "thinkpad" from this name as somebody may need to use the quirk
for a machine from a different vendor in the future.

>  static unsigned int cache_time = 1000;
>  module_param(cache_time, uint, 0644);
>  MODULE_PARM_DESC(cache_time, "cache time in milliseconds");
> @@ -222,6 +223,13 @@ static int acpi_battery_get_property(struct power_supply *psy,
>                         val->intval = POWER_SUPPLY_STATUS_CHARGING;
>                 else if (acpi_battery_is_charged(battery))
>                         val->intval = POWER_SUPPLY_STATUS_FULL;
> +               /*
> +                * On the Lenovo ThinkPad ACPI implementation, when
> +                * neither bits 0 or 1 are set, that state is
> +                * considered as "Not Charging".
> +                */
> +               else if (battery_quirk_thinkpad_notcharging)
> +                       val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
>                 else
>                         val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
>                 break;
> @@ -1301,6 +1309,13 @@ battery_notification_delay_quirk(const struct dmi_system_id *d)
>         return 0;
>  }
>
> +static int __init
> +battery_quirk_not_charging(const struct dmi_system_id *d)

Don't break the above line (it will be over 80 chars long, but that's fine).

> +{
> +       battery_quirk_thinkpad_notcharging = 1;
> +       return 0;
> +}
> +
>  static const struct dmi_system_id bat_dmi_table[] __initconst = {
>         {
>                 .callback = battery_bix_broken_package_quirk,
> @@ -1318,6 +1333,14 @@ static const struct dmi_system_id bat_dmi_table[] __initconst = {
>                         DMI_MATCH(DMI_PRODUCT_NAME, "Aspire V5-573G"),
>                 },
>         },
> +       {

Add a comment to describe this quirk (which it is needed in the first place).

> +               .callback = battery_quirk_not_charging,
> +               .ident = "Lenovo ThinkPad",
> +               .matches = {
> +                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> +                       DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad"),
> +               },
> +       },
>         {},
>  };
>
> --

Thanks,
Rafael

Re: [Devel] [PATCH 2/3 v6] battery: Add the ThinkPad "Not Charging" quirk

[Rafael J. Wysocki]

[-- Attachment #1: Type: text/plain, Size: 3471 bytes --]

On Fri, Dec 15, 2017 at 5:57 PM, Ognjen Galic <smclt30p(a)gmail.com> wrote:
> The EC/ACPI firmware on Lenovo ThinkPads used to report a status
> of "Unknown" when the battery is between the charge start and
> charge stop thresholds. On Windows, it reports "Not Charging"
> so the quirk has been added to also report correctly.
>
> Now the "status" attribute returns "Not Charging" when the
> battery on ThinkPads is not physicaly charging.
>
> Tested-by: Kevin Locke <kevin(a)kevinlocke.name>
> Tested-by: Christoph Böhmwalder <christoph(a)boehmwalder.at>
> Signed-off-by: Ognjen Galic <smclt30p(a)gmail.com>

It doesn't look like this is related to the [1/3] and [3/3], so why do
you make it part of the series?

> ---
>  drivers/acpi/battery.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
> index 2951d07..81e9b4e 100644
> --- a/drivers/acpi/battery.c
> +++ b/drivers/acpi/battery.c
> @@ -71,6 +71,7 @@ static async_cookie_t async_cookie;
>  static bool battery_driver_registered;
>  static int battery_bix_broken_package;
>  static int battery_notification_delay_ms;
> +static int battery_quirk_thinkpad_notcharging;

Drop "thinkpad" from this name as somebody may need to use the quirk
for a machine from a different vendor in the future.

>  static unsigned int cache_time = 1000;
>  module_param(cache_time, uint, 0644);
>  MODULE_PARM_DESC(cache_time, "cache time in milliseconds");
> @@ -222,6 +223,13 @@ static int acpi_battery_get_property(struct power_supply *psy,
>                         val->intval = POWER_SUPPLY_STATUS_CHARGING;
>                 else if (acpi_battery_is_charged(battery))
>                         val->intval = POWER_SUPPLY_STATUS_FULL;
> +               /*
> +                * On the Lenovo ThinkPad ACPI implementation, when
> +                * neither bits 0 or 1 are set, that state is
> +                * considered as "Not Charging".
> +                */
> +               else if (battery_quirk_thinkpad_notcharging)
> +                       val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
>                 else
>                         val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
>                 break;
> @@ -1301,6 +1309,13 @@ battery_notification_delay_quirk(const struct dmi_system_id *d)
>         return 0;
>  }
>
> +static int __init
> +battery_quirk_not_charging(const struct dmi_system_id *d)

Don't break the above line (it will be over 80 chars long, but that's fine).

> +{
> +       battery_quirk_thinkpad_notcharging = 1;
> +       return 0;
> +}
> +
>  static const struct dmi_system_id bat_dmi_table[] __initconst = {
>         {
>                 .callback = battery_bix_broken_package_quirk,
> @@ -1318,6 +1333,14 @@ static const struct dmi_system_id bat_dmi_table[] __initconst = {
>                         DMI_MATCH(DMI_PRODUCT_NAME, "Aspire V5-573G"),
>                 },
>         },
> +       {

Add a comment to describe this quirk (which it is needed in the first place).

> +               .callback = battery_quirk_not_charging,
> +               .ident = "Lenovo ThinkPad",
> +               .matches = {
> +                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> +                       DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad"),
> +               },
> +       },
>         {},
>  };
>
> --

Thanks,
Rafael

Re: [PATCH 2/3 v6] battery: Add the ThinkPad "Not Charging" quirk

[Rafael J. Wysocki]

On Sat, Dec 16, 2017 at 1:33 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Fri, Dec 15, 2017 at 5:57 PM, Ognjen Galic <smclt30p@gmail.com> wrote:
>> The EC/ACPI firmware on Lenovo ThinkPads used to report a status
>> of "Unknown" when the battery is between the charge start and
>> charge stop thresholds. On Windows, it reports "Not Charging"
>> so the quirk has been added to also report correctly.
>>
>> Now the "status" attribute returns "Not Charging" when the
>> battery on ThinkPads is not physicaly charging.
>>
>> Tested-by: Kevin Locke <kevin@kevinlocke.name>
>> Tested-by: Christoph Böhmwalder <christoph@boehmwalder.at>
>> Signed-off-by: Ognjen Galic <smclt30p@gmail.com>
>
> It doesn't look like this is related to the [1/3] and [3/3], so why do
> you make it part of the series?
>
>> ---
>>  drivers/acpi/battery.c | 23 +++++++++++++++++++++++
>>  1 file changed, 23 insertions(+)
>>
>> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
>> index 2951d07..81e9b4e 100644
>> --- a/drivers/acpi/battery.c
>> +++ b/drivers/acpi/battery.c
>> @@ -71,6 +71,7 @@ static async_cookie_t async_cookie;
>>  static bool battery_driver_registered;
>>  static int battery_bix_broken_package;
>>  static int battery_notification_delay_ms;
>> +static int battery_quirk_thinkpad_notcharging;
>
> Drop "thinkpad" from this name as somebody may need to use the quirk
> for a machine from a different vendor in the future.
>
>>  static unsigned int cache_time = 1000;
>>  module_param(cache_time, uint, 0644);
>>  MODULE_PARM_DESC(cache_time, "cache time in milliseconds");
>> @@ -222,6 +223,13 @@ static int acpi_battery_get_property(struct power_supply *psy,
>>                         val->intval = POWER_SUPPLY_STATUS_CHARGING;
>>                 else if (acpi_battery_is_charged(battery))
>>                         val->intval = POWER_SUPPLY_STATUS_FULL;
>> +               /*
>> +                * On the Lenovo ThinkPad ACPI implementation, when
>> +                * neither bits 0 or 1 are set, that state is
>> +                * considered as "Not Charging".
>> +                */
>> +               else if (battery_quirk_thinkpad_notcharging)
>> +                       val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
>>                 else
>>                         val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
>>                 break;
>> @@ -1301,6 +1309,13 @@ battery_notification_delay_quirk(const struct dmi_system_id *d)
>>         return 0;
>>  }
>>
>> +static int __init
>> +battery_quirk_not_charging(const struct dmi_system_id *d)
>
> Don't break the above line (it will be over 80 chars long, but that's fine).
>
>> +{
>> +       battery_quirk_thinkpad_notcharging = 1;
>> +       return 0;
>> +}
>> +
>>  static const struct dmi_system_id bat_dmi_table[] __initconst = {
>>         {
>>                 .callback = battery_bix_broken_package_quirk,
>> @@ -1318,6 +1333,14 @@ static const struct dmi_system_id bat_dmi_table[] __initconst = {
>>                         DMI_MATCH(DMI_PRODUCT_NAME, "Aspire V5-573G"),
>>                 },
>>         },
>> +       {
>
> Add a comment to describe this quirk (which it is needed in the first place).

s/whichj/why/

>
>> +               .callback = battery_quirk_not_charging,
>> +               .ident = "Lenovo ThinkPad",
>> +               .matches = {
>> +                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> +                       DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad"),
>> +               },
>> +       },
>>         {},
>>  };
>>
>> --

Re: [Devel] [PATCH 2/3 v6] battery: Add the ThinkPad "Not Charging" quirk

[Rafael J. Wysocki]

[-- Attachment #1: Type: text/plain, Size: 3645 bytes --]

On Sat, Dec 16, 2017 at 1:33 AM, Rafael J. Wysocki <rafael(a)kernel.org> wrote:
> On Fri, Dec 15, 2017 at 5:57 PM, Ognjen Galic <smclt30p(a)gmail.com> wrote:
>> The EC/ACPI firmware on Lenovo ThinkPads used to report a status
>> of "Unknown" when the battery is between the charge start and
>> charge stop thresholds. On Windows, it reports "Not Charging"
>> so the quirk has been added to also report correctly.
>>
>> Now the "status" attribute returns "Not Charging" when the
>> battery on ThinkPads is not physicaly charging.
>>
>> Tested-by: Kevin Locke <kevin(a)kevinlocke.name>
>> Tested-by: Christoph Böhmwalder <christoph(a)boehmwalder.at>
>> Signed-off-by: Ognjen Galic <smclt30p(a)gmail.com>
>
> It doesn't look like this is related to the [1/3] and [3/3], so why do
> you make it part of the series?
>
>> ---
>>  drivers/acpi/battery.c | 23 +++++++++++++++++++++++
>>  1 file changed, 23 insertions(+)
>>
>> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
>> index 2951d07..81e9b4e 100644
>> --- a/drivers/acpi/battery.c
>> +++ b/drivers/acpi/battery.c
>> @@ -71,6 +71,7 @@ static async_cookie_t async_cookie;
>>  static bool battery_driver_registered;
>>  static int battery_bix_broken_package;
>>  static int battery_notification_delay_ms;
>> +static int battery_quirk_thinkpad_notcharging;
>
> Drop "thinkpad" from this name as somebody may need to use the quirk
> for a machine from a different vendor in the future.
>
>>  static unsigned int cache_time = 1000;
>>  module_param(cache_time, uint, 0644);
>>  MODULE_PARM_DESC(cache_time, "cache time in milliseconds");
>> @@ -222,6 +223,13 @@ static int acpi_battery_get_property(struct power_supply *psy,
>>                         val->intval = POWER_SUPPLY_STATUS_CHARGING;
>>                 else if (acpi_battery_is_charged(battery))
>>                         val->intval = POWER_SUPPLY_STATUS_FULL;
>> +               /*
>> +                * On the Lenovo ThinkPad ACPI implementation, when
>> +                * neither bits 0 or 1 are set, that state is
>> +                * considered as "Not Charging".
>> +                */
>> +               else if (battery_quirk_thinkpad_notcharging)
>> +                       val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
>>                 else
>>                         val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
>>                 break;
>> @@ -1301,6 +1309,13 @@ battery_notification_delay_quirk(const struct dmi_system_id *d)
>>         return 0;
>>  }
>>
>> +static int __init
>> +battery_quirk_not_charging(const struct dmi_system_id *d)
>
> Don't break the above line (it will be over 80 chars long, but that's fine).
>
>> +{
>> +       battery_quirk_thinkpad_notcharging = 1;
>> +       return 0;
>> +}
>> +
>>  static const struct dmi_system_id bat_dmi_table[] __initconst = {
>>         {
>>                 .callback = battery_bix_broken_package_quirk,
>> @@ -1318,6 +1333,14 @@ static const struct dmi_system_id bat_dmi_table[] __initconst = {
>>                         DMI_MATCH(DMI_PRODUCT_NAME, "Aspire V5-573G"),
>>                 },
>>         },
>> +       {
>
> Add a comment to describe this quirk (which it is needed in the first place).

s/whichj/why/

>
>> +               .callback = battery_quirk_not_charging,
>> +               .ident = "Lenovo ThinkPad",
>> +               .matches = {
>> +                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
>> +                       DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad"),
>> +               },
>> +       },
>>         {},
>>  };
>>
>> --

Re: [PATCH 2/3 v6] battery: Add the ThinkPad "Not Charging" quirk

[Ognjen Galic]

On Sat, Dec 16, 2017 at 01:33:55AM +0100, Rafael J. Wysocki wrote:
> On Fri, Dec 15, 2017 at 5:57 PM, Ognjen Galic <smclt30p@gmail.com> wrote:
> > The EC/ACPI firmware on Lenovo ThinkPads used to report a status
> > of "Unknown" when the battery is between the charge start and
> > charge stop thresholds. On Windows, it reports "Not Charging"
> > so the quirk has been added to also report correctly.
> >
> > Now the "status" attribute returns "Not Charging" when the
> > battery on ThinkPads is not physicaly charging.
> >
> > Tested-by: Kevin Locke <kevin@kevinlocke.name>
> > Tested-by: Christoph Böhmwalder <christoph@boehmwalder.at>
> > Signed-off-by: Ognjen Galic <smclt30p@gmail.com>
> 
> It doesn't look like this is related to the [1/3] and [3/3], so why do
> you make it part of the series?
> 

I made it the same series because it is practically the same feature
set. Without this patch and with 1/3 and 3/3 applied, there is a bug
where the status attribute would show "Unknown" for a battery that is
between the start and stop thresholds while attached to AC.

> > ---
> >  drivers/acpi/battery.c | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> >
> > diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
> > index 2951d07..81e9b4e 100644
> > --- a/drivers/acpi/battery.c
> > +++ b/drivers/acpi/battery.c
> > @@ -71,6 +71,7 @@ static async_cookie_t async_cookie;
> >  static bool battery_driver_registered;
> >  static int battery_bix_broken_package;
> >  static int battery_notification_delay_ms;
> > +static int battery_quirk_thinkpad_notcharging;
> 
> Drop "thinkpad" from this name as somebody may need to use the quirk
> for a machine from a different vendor in the future.
> 
> >  static unsigned int cache_time = 1000;
> >  module_param(cache_time, uint, 0644);
> >  MODULE_PARM_DESC(cache_time, "cache time in milliseconds");
> > @@ -222,6 +223,13 @@ static int acpi_battery_get_property(struct power_supply *psy,
> >                         val->intval = POWER_SUPPLY_STATUS_CHARGING;
> >                 else if (acpi_battery_is_charged(battery))
> >                         val->intval = POWER_SUPPLY_STATUS_FULL;
> > +               /*
> > +                * On the Lenovo ThinkPad ACPI implementation, when
> > +                * neither bits 0 or 1 are set, that state is
> > +                * considered as "Not Charging".
> > +                */
> > +               else if (battery_quirk_thinkpad_notcharging)
> > +                       val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
> >                 else
> >                         val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
> >                 break;
> > @@ -1301,6 +1309,13 @@ battery_notification_delay_quirk(const struct dmi_system_id *d)
> >         return 0;
> >  }
> >
> > +static int __init
> > +battery_quirk_not_charging(const struct dmi_system_id *d)
> 
> Don't break the above line (it will be over 80 chars long, but that's fine).
> 
> > +{
> > +       battery_quirk_thinkpad_notcharging = 1;
> > +       return 0;
> > +}
> > +
> >  static const struct dmi_system_id bat_dmi_table[] __initconst = {
> >         {
> >                 .callback = battery_bix_broken_package_quirk,
> > @@ -1318,6 +1333,14 @@ static const struct dmi_system_id bat_dmi_table[] __initconst = {
> >                         DMI_MATCH(DMI_PRODUCT_NAME, "Aspire V5-573G"),
> >                 },
> >         },
> > +       {
> 
> Add a comment to describe this quirk (which it is needed in the first place).
> 
> > +               .callback = battery_quirk_not_charging,
> > +               .ident = "Lenovo ThinkPad",
> > +               .matches = {
> > +                       DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> > +                       DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad"),
> > +               },
> > +       },
> >         {},
> >  };
> >
> > --
> 
> Thanks,
> Rafael

Re: [PATCH 2/3 v6] battery: Add the ThinkPad "Not Charging" quirk

[Rafael J. Wysocki]

On Sat, Dec 16, 2017 at 9:48 AM, Ognjen Galic <smclt30p@gmail.com> wrote:
> On Sat, Dec 16, 2017 at 01:33:55AM +0100, Rafael J. Wysocki wrote:
>> On Fri, Dec 15, 2017 at 5:57 PM, Ognjen Galic <smclt30p@gmail.com> wrote:
>> > The EC/ACPI firmware on Lenovo ThinkPads used to report a status
>> > of "Unknown" when the battery is between the charge start and
>> > charge stop thresholds. On Windows, it reports "Not Charging"
>> > so the quirk has been added to also report correctly.
>> >
>> > Now the "status" attribute returns "Not Charging" when the
>> > battery on ThinkPads is not physicaly charging.
>> >
>> > Tested-by: Kevin Locke <kevin@kevinlocke.name>
>> > Tested-by: Christoph Böhmwalder <christoph@boehmwalder.at>
>> > Signed-off-by: Ognjen Galic <smclt30p@gmail.com>
>>
>> It doesn't look like this is related to the [1/3] and [3/3], so why do
>> you make it part of the series?
>>
>
> I made it the same series because it is practically the same feature
> set. Without this patch and with 1/3 and 3/3 applied, there is a bug
> where the status attribute would show "Unknown" for a battery that is
> between the start and stop thresholds while attached to AC.

OK, fair enough.

I would reorder it as the first patch in the series, then, because the
other two are more related to each other.

Thanks,
Rafael

Re: [Devel] [PATCH 2/3 v6] battery: Add the ThinkPad "Not Charging" quirk

[Rafael J. Wysocki]

[-- Attachment #1: Type: text/plain, Size: 1363 bytes --]

On Sat, Dec 16, 2017 at 9:48 AM, Ognjen Galic <smclt30p(a)gmail.com> wrote:
> On Sat, Dec 16, 2017 at 01:33:55AM +0100, Rafael J. Wysocki wrote:
>> On Fri, Dec 15, 2017 at 5:57 PM, Ognjen Galic <smclt30p(a)gmail.com> wrote:
>> > The EC/ACPI firmware on Lenovo ThinkPads used to report a status
>> > of "Unknown" when the battery is between the charge start and
>> > charge stop thresholds. On Windows, it reports "Not Charging"
>> > so the quirk has been added to also report correctly.
>> >
>> > Now the "status" attribute returns "Not Charging" when the
>> > battery on ThinkPads is not physicaly charging.
>> >
>> > Tested-by: Kevin Locke <kevin(a)kevinlocke.name>
>> > Tested-by: Christoph Böhmwalder <christoph(a)boehmwalder.at>
>> > Signed-off-by: Ognjen Galic <smclt30p(a)gmail.com>
>>
>> It doesn't look like this is related to the [1/3] and [3/3], so why do
>> you make it part of the series?
>>
>
> I made it the same series because it is practically the same feature
> set. Without this patch and with 1/3 and 3/3 applied, there is a bug
> where the status attribute would show "Unknown" for a battery that is
> between the start and stop thresholds while attached to AC.

OK, fair enough.

I would reorder it as the first patch in the series, then, because the
other two are more related to each other.

Thanks,
Rafael

Re: [PATCH 2/3 v6] battery: Add the ThinkPad "Not Charging" quirk

[Ognjen Galić]

On 16/12/2017, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Sat, Dec 16, 2017 at 9:48 AM, Ognjen Galic <smclt30p@gmail.com> wrote:
>> On Sat, Dec 16, 2017 at 01:33:55AM +0100, Rafael J. Wysocki wrote:
>>> On Fri, Dec 15, 2017 at 5:57 PM, Ognjen Galic <smclt30p@gmail.com>
>>> wrote:
>>> > The EC/ACPI firmware on Lenovo ThinkPads used to report a status
>>> > of "Unknown" when the battery is between the charge start and
>>> > charge stop thresholds. On Windows, it reports "Not Charging"
>>> > so the quirk has been added to also report correctly.
>>> >
>>> > Now the "status" attribute returns "Not Charging" when the
>>> > battery on ThinkPads is not physicaly charging.
>>> >
>>> > Tested-by: Kevin Locke <kevin@kevinlocke.name>
>>> > Tested-by: Christoph Böhmwalder <christoph@boehmwalder.at>
>>> > Signed-off-by: Ognjen Galic <smclt30p@gmail.com>
>>>
>>> It doesn't look like this is related to the [1/3] and [3/3], so why do
>>> you make it part of the series?
>>>
>>
>> I made it the same series because it is practically the same feature
>> set. Without this patch and with 1/3 and 3/3 applied, there is a bug
>> where the status attribute would show "Unknown" for a battery that is
>> between the start and stop thresholds while attached to AC.
>
> OK, fair enough.
>
> I would reorder it as the first patch in the series, then, because the
> other two are more related to each other.
>
> Thanks,
> Rafael
>

Do I really need now to re-order and re-send the whole patchset for it
to get merged?

Thanks.

Re: [PATCH 2/3 v6] battery: Add the ThinkPad "Not Charging" quirk

[Andy Shevchenko]

On Mon, Dec 18, 2017 at 12:21 PM, Ognjen Galić <smclt30p@gmail.com> wrote:
> On 16/12/2017, Rafael J. Wysocki <rafael@kernel.org> wrote:

>> I would reorder it as the first patch in the series, then, because the
>> other two are more related to each other.

> Do I really need now to re-order and re-send the whole patchset for it
> to get merged?

Through which tree it's supposed to go?

-- 
With Best Regards,
Andy Shevchenko

Re: [Devel] [PATCH 2/3 v6] battery: Add the ThinkPad "Not Charging" quirk

[Andy Shevchenko]

[-- Attachment #1: Type: text/plain, Size: 448 bytes --]

On Mon, Dec 18, 2017 at 12:21 PM, Ognjen Galić <smclt30p(a)gmail.com> wrote:
> On 16/12/2017, Rafael J. Wysocki <rafael(a)kernel.org> wrote:

>> I would reorder it as the first patch in the series, then, because the
>> other two are more related to each other.

> Do I really need now to re-order and re-send the whole patchset for it
> to get merged?

Through which tree it's supposed to go?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/3 v6] battery: Add the ThinkPad "Not Charging" quirk

[Ognjen Galić]

On 18/12/2017, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> On Mon, Dec 18, 2017 at 12:21 PM, Ognjen Galić <smclt30p@gmail.com> wrote:
>> On 16/12/2017, Rafael J. Wysocki <rafael@kernel.org> wrote:
>
>>> I would reorder it as the first patch in the series, then, because the
>>> other two are more related to each other.
>
>> Do I really need now to re-order and re-send the whole patchset for it
>> to get merged?
>
> Through which tree it's supposed to go?
>
> --
> With Best Regards,
> Andy Shevchenko
>

I'm afraid I don't understand the question. If you mean by Linux tree,
it was written while 4.15-rc2 was active.

Thanks,
Gala

Re: [PATCH 2/3 v6] battery: Add the ThinkPad "Not Charging" quirk

[Rafael J. Wysocki]

On Mon, Dec 18, 2017 at 11:21 AM, Ognjen Galić <smclt30p@gmail.com> wrote:
> On 16/12/2017, Rafael J. Wysocki <rafael@kernel.org> wrote:
>> On Sat, Dec 16, 2017 at 9:48 AM, Ognjen Galic <smclt30p@gmail.com> wrote:
>>> On Sat, Dec 16, 2017 at 01:33:55AM +0100, Rafael J. Wysocki wrote:
>>>> On Fri, Dec 15, 2017 at 5:57 PM, Ognjen Galic <smclt30p@gmail.com>
>>>> wrote:
>>>> > The EC/ACPI firmware on Lenovo ThinkPads used to report a status
>>>> > of "Unknown" when the battery is between the charge start and
>>>> > charge stop thresholds. On Windows, it reports "Not Charging"
>>>> > so the quirk has been added to also report correctly.
>>>> >
>>>> > Now the "status" attribute returns "Not Charging" when the
>>>> > battery on ThinkPads is not physicaly charging.
>>>> >
>>>> > Tested-by: Kevin Locke <kevin@kevinlocke.name>
>>>> > Tested-by: Christoph Böhmwalder <christoph@boehmwalder.at>
>>>> > Signed-off-by: Ognjen Galic <smclt30p@gmail.com>
>>>>
>>>> It doesn't look like this is related to the [1/3] and [3/3], so why do
>>>> you make it part of the series?
>>>>
>>>
>>> I made it the same series because it is practically the same feature
>>> set. Without this patch and with 1/3 and 3/3 applied, there is a bug
>>> where the status attribute would show "Unknown" for a battery that is
>>> between the start and stop thresholds while attached to AC.
>>
>> OK, fair enough.
>>
>> I would reorder it as the first patch in the series, then, because the
>> other two are more related to each other.
>>
>> Thanks,
>> Rafael
>>
>
> Do I really need now to re-order and re-send the whole patchset for it
> to get merged?

I'm not sure I will take anything of it at this time.

Thanks,
Rafael

Re: [Devel] [PATCH 2/3 v6] battery: Add the ThinkPad "Not Charging" quirk

[Rafael J. Wysocki]

[-- Attachment #1: Type: text/plain, Size: 1756 bytes --]

On Mon, Dec 18, 2017 at 11:21 AM, Ognjen Galić <smclt30p(a)gmail.com> wrote:
> On 16/12/2017, Rafael J. Wysocki <rafael(a)kernel.org> wrote:
>> On Sat, Dec 16, 2017 at 9:48 AM, Ognjen Galic <smclt30p(a)gmail.com> wrote:
>>> On Sat, Dec 16, 2017 at 01:33:55AM +0100, Rafael J. Wysocki wrote:
>>>> On Fri, Dec 15, 2017 at 5:57 PM, Ognjen Galic <smclt30p(a)gmail.com>
>>>> wrote:
>>>> > The EC/ACPI firmware on Lenovo ThinkPads used to report a status
>>>> > of "Unknown" when the battery is between the charge start and
>>>> > charge stop thresholds. On Windows, it reports "Not Charging"
>>>> > so the quirk has been added to also report correctly.
>>>> >
>>>> > Now the "status" attribute returns "Not Charging" when the
>>>> > battery on ThinkPads is not physicaly charging.
>>>> >
>>>> > Tested-by: Kevin Locke <kevin(a)kevinlocke.name>
>>>> > Tested-by: Christoph Böhmwalder <christoph(a)boehmwalder.at>
>>>> > Signed-off-by: Ognjen Galic <smclt30p(a)gmail.com>
>>>>
>>>> It doesn't look like this is related to the [1/3] and [3/3], so why do
>>>> you make it part of the series?
>>>>
>>>
>>> I made it the same series because it is practically the same feature
>>> set. Without this patch and with 1/3 and 3/3 applied, there is a bug
>>> where the status attribute would show "Unknown" for a battery that is
>>> between the start and stop thresholds while attached to AC.
>>
>> OK, fair enough.
>>
>> I would reorder it as the first patch in the series, then, because the
>> other two are more related to each other.
>>
>> Thanks,
>> Rafael
>>
>
> Do I really need now to re-order and re-send the whole patchset for it
> to get merged?

I'm not sure I will take anything of it at this time.

Thanks,
Rafael

Re: [PATCH 2/3 v6] battery: Add the ThinkPad "Not Charging" quirk

[Rafael J. Wysocki]

On Mon, Dec 18, 2017 at 1:36 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Mon, Dec 18, 2017 at 12:21 PM, Ognjen Galić <smclt30p@gmail.com> wrote:
>> On 16/12/2017, Rafael J. Wysocki <rafael@kernel.org> wrote:
>
>>> I would reorder it as the first patch in the series, then, because the
>>> other two are more related to each other.
>
>> Do I really need now to re-order and re-send the whole patchset for it
>> to get merged?
>
> Through which tree it's supposed to go?

Wait, wait.  I haven't commented patches [1,3/3] yet, so no ACKs
assumed, please.

Thanks,
Rafael

Re: [Devel] [PATCH 2/3 v6] battery: Add the ThinkPad "Not Charging" quirk

[Rafael J. Wysocki]

[-- Attachment #1: Type: text/plain, Size: 609 bytes --]

On Mon, Dec 18, 2017 at 1:36 PM, Andy Shevchenko
<andy.shevchenko(a)gmail.com> wrote:
> On Mon, Dec 18, 2017 at 12:21 PM, Ognjen Galić <smclt30p(a)gmail.com> wrote:
>> On 16/12/2017, Rafael J. Wysocki <rafael(a)kernel.org> wrote:
>
>>> I would reorder it as the first patch in the series, then, because the
>>> other two are more related to each other.
>
>> Do I really need now to re-order and re-send the whole patchset for it
>> to get merged?
>
> Through which tree it's supposed to go?

Wait, wait.  I haven't commented patches [1,3/3] yet, so no ACKs
assumed, please.

Thanks,
Rafael

Re: [PATCH 2/3 v6] battery: Add the ThinkPad "Not Charging" quirk

[Andy Shevchenko]

On Mon, Dec 18, 2017 at 7:31 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Mon, Dec 18, 2017 at 1:36 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>> On Mon, Dec 18, 2017 at 12:21 PM, Ognjen Galić <smclt30p@gmail.com> wrote:
>>> On 16/12/2017, Rafael J. Wysocki <rafael@kernel.org> wrote:
>>
>>>> I would reorder it as the first patch in the series, then, because the
>>>> other two are more related to each other.
>>
>>> Do I really need now to re-order and re-send the whole patchset for it
>>> to get merged?
>>
>> Through which tree it's supposed to go?
>
> Wait, wait.  I haven't commented patches [1,3/3] yet, so no ACKs
> assumed, please.

I would prefer if you can take it through linux-pm.

Whenever you are okay with the series let me know I would review PDx86 bits.

-- 
With Best Regards,
Andy Shevchenko

Re: [Devel] [PATCH 2/3 v6] battery: Add the ThinkPad "Not Charging" quirk

[Andy Shevchenko]

[-- Attachment #1: Type: text/plain, Size: 869 bytes --]

On Mon, Dec 18, 2017 at 7:31 PM, Rafael J. Wysocki <rafael(a)kernel.org> wrote:
> On Mon, Dec 18, 2017 at 1:36 PM, Andy Shevchenko
> <andy.shevchenko(a)gmail.com> wrote:
>> On Mon, Dec 18, 2017 at 12:21 PM, Ognjen Galić <smclt30p(a)gmail.com> wrote:
>>> On 16/12/2017, Rafael J. Wysocki <rafael(a)kernel.org> wrote:
>>
>>>> I would reorder it as the first patch in the series, then, because the
>>>> other two are more related to each other.
>>
>>> Do I really need now to re-order and re-send the whole patchset for it
>>> to get merged?
>>
>> Through which tree it's supposed to go?
>
> Wait, wait.  I haven't commented patches [1,3/3] yet, so no ACKs
> assumed, please.

I would prefer if you can take it through linux-pm.

Whenever you are okay with the series let me know I would review PDx86 bits.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/3 v6] battery: Add the ThinkPad "Not Charging" quirk

[Rafael J. Wysocki]

On Tue, Dec 19, 2017 at 3:34 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Mon, Dec 18, 2017 at 7:31 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
>> On Mon, Dec 18, 2017 at 1:36 PM, Andy Shevchenko
>> <andy.shevchenko@gmail.com> wrote:
>>> On Mon, Dec 18, 2017 at 12:21 PM, Ognjen Galić <smclt30p@gmail.com> wrote:
>>>> On 16/12/2017, Rafael J. Wysocki <rafael@kernel.org> wrote:
>>>
>>>>> I would reorder it as the first patch in the series, then, because the
>>>>> other two are more related to each other.
>>>
>>>> Do I really need now to re-order and re-send the whole patchset for it
>>>> to get merged?
>>>
>>> Through which tree it's supposed to go?
>>
>> Wait, wait.  I haven't commented patches [1,3/3] yet, so no ACKs
>> assumed, please.
>
> I would prefer if you can take it through linux-pm.

No problem.

> Whenever you are okay with the series let me know I would review PDx86 bits.

OK

Thanks,
Rafael

Re: [Devel] [PATCH 2/3 v6] battery: Add the ThinkPad "Not Charging" quirk

[Rafael J. Wysocki]

[-- Attachment #1: Type: text/plain, Size: 975 bytes --]

On Tue, Dec 19, 2017 at 3:34 PM, Andy Shevchenko
<andy.shevchenko(a)gmail.com> wrote:
> On Mon, Dec 18, 2017 at 7:31 PM, Rafael J. Wysocki <rafael(a)kernel.org> wrote:
>> On Mon, Dec 18, 2017 at 1:36 PM, Andy Shevchenko
>> <andy.shevchenko(a)gmail.com> wrote:
>>> On Mon, Dec 18, 2017 at 12:21 PM, Ognjen Galić <smclt30p(a)gmail.com> wrote:
>>>> On 16/12/2017, Rafael J. Wysocki <rafael(a)kernel.org> wrote:
>>>
>>>>> I would reorder it as the first patch in the series, then, because the
>>>>> other two are more related to each other.
>>>
>>>> Do I really need now to re-order and re-send the whole patchset for it
>>>> to get merged?
>>>
>>> Through which tree it's supposed to go?
>>
>> Wait, wait.  I haven't commented patches [1,3/3] yet, so no ACKs
>> assumed, please.
>
> I would prefer if you can take it through linux-pm.

No problem.

> Whenever you are okay with the series let me know I would review PDx86 bits.

OK

Thanks,
Rafael