2020-01-29 21:16:21

by Thomas Weißschuh

[permalink] [raw]
Subject: [PATCH 0/3] platform/x86: thinkpad_acpi: use standard charge control attribute names

This patch series switches over the battery control sysfs attributes to their
standard names as documented in Documentation/ABI/testing/sysfs-class-power.

If backwards compatability is not required please drop patch 3 of this series.
The old names were not documented explicitly and new generic software should
automatically use the new attributes, which may allow us to drop the old names.

Thomas Weißschuh (3):
platform/x86: thinkpad_acpi: remove unused defines
platform/x86: thinkpad_acpi: use standard charge control attribute
names
platform/x86: thinkpad_acpi: restore old battery charge attributes

drivers/platform/x86/thinkpad_acpi.c | 29 +++++++++++++++++++---------
1 file changed, 20 insertions(+), 9 deletions(-)

--
2.25.0


2020-01-31 14:38:52

by Thomas Weißschuh

[permalink] [raw]
Subject: [PATCH v2 0/3] platform/x86: thinkpad_acpi: use standard charge control attribute names

This patch series switches the battery control sysfs attributes to their
standard names as documented in Documentation/ABI/testing/sysfs-class-power.

If backwards compatability is not required please drop patch 3 of this series.
The old names were not documented explicitly and new generic software should
automatically use the new attributes, which may allow to drop the old names.

Changes since v1:
* Corrected charge_control_stop_threshold to charge_control_end_threshold.

Thomas Weißschuh (3):
platform/x86: thinkpad_acpi: remove unused defines
platform/x86: thinkpad_acpi: use standard charge control attribute
names
platform/x86: thinkpad_acpi: restore old battery charge attributes

drivers/platform/x86/thinkpad_acpi.c | 29 +++++++++++++++++++---------
1 file changed, 20 insertions(+), 9 deletions(-)

--
2.25.0

2020-01-31 14:39:35

by Thomas Weißschuh

[permalink] [raw]
Subject: [PATCH v2 1/3] platform/x86: thinkpad_acpi: remove unused defines

They were never used.

Signed-off-by: Thomas Weißschuh <[email protected]>
---
drivers/platform/x86/thinkpad_acpi.c | 3 ---
1 file changed, 3 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index da794dcfdd92..2d3a99e3efb7 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -9323,9 +9323,6 @@ static struct ibm_struct mute_led_driver_data = {
#define GET_STOP "BCSG"
#define SET_STOP "BCSS"

-#define START_ATTR "charge_start_threshold"
-#define STOP_ATTR "charge_stop_threshold"
-
enum {
BAT_ANY = 0,
BAT_PRIMARY = 1,
--
2.25.0

2020-01-31 14:39:59

by Thomas Weißschuh

[permalink] [raw]
Subject: [PATCH v2 2/3] platform/x86: thinkpad_acpi: use standard charge control attribute names

The standard attributes were only introduced after the ones from
thinkpad_acpi in commit 813cab8f3994 ("power: supply: core: Add CHARGE_CONTROL_{START_THRESHOLD,END_THRESHOLD} properties").

Signed-off-by: Thomas Weißschuh <[email protected]>
---
drivers/platform/x86/thinkpad_acpi.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 2d3a99e3efb7..5f0e3299778a 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -9608,40 +9608,40 @@ static ssize_t tpacpi_battery_show(int what,
return sprintf(buf, "%d\n", ret);
}

-static ssize_t charge_start_threshold_show(struct device *device,
+static ssize_t charge_control_start_threshold_show(struct device *device,
struct device_attribute *attr,
char *buf)
{
return tpacpi_battery_show(THRESHOLD_START, device, buf);
}

-static ssize_t charge_stop_threshold_show(struct device *device,
+static ssize_t charge_control_end_threshold_show(struct device *device,
struct device_attribute *attr,
char *buf)
{
return tpacpi_battery_show(THRESHOLD_STOP, device, buf);
}

-static ssize_t charge_start_threshold_store(struct device *dev,
+static ssize_t charge_control_start_threshold_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
return tpacpi_battery_store(THRESHOLD_START, dev, buf, count);
}

-static ssize_t charge_stop_threshold_store(struct device *dev,
+static ssize_t charge_control_end_threshold_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
return tpacpi_battery_store(THRESHOLD_STOP, dev, buf, count);
}

-static DEVICE_ATTR_RW(charge_start_threshold);
-static DEVICE_ATTR_RW(charge_stop_threshold);
+static DEVICE_ATTR_RW(charge_control_start_threshold);
+static DEVICE_ATTR_RW(charge_control_end_threshold);

static struct attribute *tpacpi_battery_attrs[] = {
- &dev_attr_charge_start_threshold.attr,
- &dev_attr_charge_stop_threshold.attr,
+ &dev_attr_charge_control_start_threshold.attr,
+ &dev_attr_charge_control_end_threshold.attr,
NULL,
};

--
2.25.0

2020-02-03 11:23:34

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] platform/x86: thinkpad_acpi: remove unused defines

On Fri, Jan 31, 2020 at 03:36:48PM +0100, Thomas Wei?schuh wrote:
> They were never used.

Thanks for the series.

Unfortunately I may not proceed it since it misses our patchwork [1]
for some reason.

Besides that, this patch is okay, but the rest two must be:
- unified together to avoid regression in the middle
- done other way around, simple add aliases to *old* ones

Don't forget to update any documentation if needed.

[1]: https://patchwork.kernel.org/project/platform-driver-x86/list/

>
> Signed-off-by: Thomas Wei?schuh <[email protected]>
> ---
> drivers/platform/x86/thinkpad_acpi.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index da794dcfdd92..2d3a99e3efb7 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -9323,9 +9323,6 @@ static struct ibm_struct mute_led_driver_data = {
> #define GET_STOP "BCSG"
> #define SET_STOP "BCSS"
>
> -#define START_ATTR "charge_start_threshold"
> -#define STOP_ATTR "charge_stop_threshold"
> -
> enum {
> BAT_ANY = 0,
> BAT_PRIMARY = 1,

--
With Best Regards,
Andy Shevchenko


2020-02-03 11:23:34

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] platform/x86: thinkpad_acpi: remove unused defines

On Mon, Feb 03, 2020 at 12:00:43PM +0200, Andy Shevchenko wrote:
> On Fri, Jan 31, 2020 at 03:36:48PM +0100, Thomas Wei?schuh wrote:
> > They were never used.
>
> Thanks for the series.
>
> Unfortunately I may not proceed it since it misses our patchwork [1]
> for some reason.

Oh, sorry! It misses our *mailing list* while patchwork is good.

> Besides that, this patch is okay, but the rest two must be:
> - unified together to avoid regression in the middle
> - done other way around, simple add aliases to *old* ones
>
> Don't forget to update any documentation if needed.
>
> [1]: https://patchwork.kernel.org/project/platform-driver-x86/list/

--
With Best Regards,
Andy Shevchenko


2020-02-03 11:27:54

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] platform/x86: thinkpad_acpi: remove unused defines

On Mon, Feb 03, 2020 at 12:02:11PM +0200, Andy Shevchenko wrote:
> On Mon, Feb 03, 2020 at 12:00:43PM +0200, Andy Shevchenko wrote:
> > On Fri, Jan 31, 2020 at 03:36:48PM +0100, Thomas Wei?schuh wrote:
> > > They were never used.
> >
> > Thanks for the series.
> >
> > Unfortunately I may not proceed it since it misses our patchwork [1]
> > for some reason.
>
> Oh, sorry! It misses our *mailing list* while patchwork is good.

Found your mails in archive (on the Web), so, it seems Google (gmail) doesn't
like them.

It seems no issue on your side. Anyway, I'm waiting for v3 as commented
previously.

> > Besides that, this patch is okay, but the rest two must be:
> > - unified together to avoid regression in the middle
> > - done other way around, simple add aliases to *old* ones
> >
> > Don't forget to update any documentation if needed.
> >
> > [1]: https://patchwork.kernel.org/project/platform-driver-x86/list/

--
With Best Regards,
Andy Shevchenko


2020-02-03 14:03:25

by Thomas Weißschuh

[permalink] [raw]
Subject: [PATCH v3 0/2] platform/x86: thinkpad_acpi: use standard charge control attribute names

This patch series switches the battery control sysfs attributes to their
standard names as documented in Documentation/ABI/testing/sysfs-class-power.

If backwards compatability is not required please drop patch 3 of this series.
The old names were not documented explicitly and new generic software should
automatically use the new attributes, which may allow to drop the old names.

Changes since v2:
* Preserved API backwards compat after each patch by merging the patches that
switch over to the new API and reintroduce the compat aliases.

Thomas Weißschuh (2):
platform/x86: thinkpad_acpi: remove unused defines
platform/x86: thinkpad_acpi: use standard charge control attribute
names

drivers/platform/x86/thinkpad_acpi.c | 29 +++++++++++++++++++---------
1 file changed, 20 insertions(+), 9 deletions(-)

--
2.25.0

2020-02-03 14:03:28

by Thomas Weißschuh

[permalink] [raw]
Subject: [PATCH v3 1/2] platform/x86: thinkpad_acpi: remove unused defines

They were never used.

Signed-off-by: Thomas Weißschuh <[email protected]>
---
drivers/platform/x86/thinkpad_acpi.c | 3 ---
1 file changed, 3 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index da794dcfdd92..2d3a99e3efb7 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -9323,9 +9323,6 @@ static struct ibm_struct mute_led_driver_data = {
#define GET_STOP "BCSG"
#define SET_STOP "BCSS"

-#define START_ATTR "charge_start_threshold"
-#define STOP_ATTR "charge_stop_threshold"
-
enum {
BAT_ANY = 0,
BAT_PRIMARY = 1,
--
2.25.0

2020-02-03 14:15:31

by Thomas Weißschuh

[permalink] [raw]
Subject: [PATCH v3 2/2] platform/x86: thinkpad_acpi: use standard charge control attribute names

The standard attributes were only introduced after the ones from
thinkpad_acpi in commit 813cab8f3994 ("power: supply: core:
Add CHARGE_CONTROL_{START_THRESHOLD,END_THRESHOLD} properties").

The new standard attributes are aliased to their previous names,
preserving backwards compatibility.

Signed-off-by: Thomas Weißschuh <[email protected]>
---
drivers/platform/x86/thinkpad_acpi.c | 26 ++++++++++++++++++++------
1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 2d3a99e3efb7..2cbcd2e3092f 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -9608,38 +9608,52 @@ static ssize_t tpacpi_battery_show(int what,
return sprintf(buf, "%d\n", ret);
}

-static ssize_t charge_start_threshold_show(struct device *device,
+static ssize_t charge_control_start_threshold_show(struct device *device,
struct device_attribute *attr,
char *buf)
{
return tpacpi_battery_show(THRESHOLD_START, device, buf);
}

-static ssize_t charge_stop_threshold_show(struct device *device,
+static ssize_t charge_control_end_threshold_show(struct device *device,
struct device_attribute *attr,
char *buf)
{
return tpacpi_battery_show(THRESHOLD_STOP, device, buf);
}

-static ssize_t charge_start_threshold_store(struct device *dev,
+static ssize_t charge_control_start_threshold_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
return tpacpi_battery_store(THRESHOLD_START, dev, buf, count);
}

-static ssize_t charge_stop_threshold_store(struct device *dev,
+static ssize_t charge_control_end_threshold_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t count)
{
return tpacpi_battery_store(THRESHOLD_STOP, dev, buf, count);
}

-static DEVICE_ATTR_RW(charge_start_threshold);
-static DEVICE_ATTR_RW(charge_stop_threshold);
+static DEVICE_ATTR_RW(charge_control_start_threshold);
+static DEVICE_ATTR_RW(charge_control_end_threshold);
+struct device_attribute dev_attr_charge_start_threshold = __ATTR(
+ charge_start_threshold,
+ 0644,
+ charge_control_start_threshold_show,
+ charge_control_start_threshold_store
+);
+struct device_attribute dev_attr_charge_stop_threshold = __ATTR(
+ charge_stop_threshold,
+ 0644,
+ charge_control_end_threshold_show,
+ charge_control_end_threshold_store
+);

static struct attribute *tpacpi_battery_attrs[] = {
+ &dev_attr_charge_control_start_threshold.attr,
+ &dev_attr_charge_control_end_threshold.attr,
&dev_attr_charge_start_threshold.attr,
&dev_attr_charge_stop_threshold.attr,
NULL,
--
2.25.0