* Warning since "power: supply: add charge_behaviour attributes"
@ 2022-01-04 23:03 Heiner Kallweit
2022-01-05 6:42 ` [PATCH 1/2] power: supply: fix charge_behaviour attribute initialization Thomas Weißschuh
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Heiner Kallweit @ 2022-01-04 23:03 UTC (permalink / raw)
To: Sebastian Reichel, Thomas Weißschuh, Hans de Goede; +Cc: Linux PM
Since 1b0b6cc8030d ("power: supply: add charge_behaviour attributes") I get the following warning:
power_supply_init_attrs: Property 37 skipped because it is missing from power_supply_attrs
Seems the patch misses the following:
diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
index 2cfce2b2e..ef5109102 100644
--- a/drivers/power/supply/power_supply_sysfs.c
+++ b/drivers/power/supply/power_supply_sysfs.c
@@ -179,6 +179,7 @@ static struct power_supply_attr power_supply_attrs[] = {
POWER_SUPPLY_ATTR(CHARGE_CONTROL_LIMIT_MAX),
POWER_SUPPLY_ATTR(CHARGE_CONTROL_START_THRESHOLD),
POWER_SUPPLY_ATTR(CHARGE_CONTROL_END_THRESHOLD),
+ POWER_SUPPLY_ENUM_ATTR(CHARGE_BEHAVIOUR),
POWER_SUPPLY_ATTR(INPUT_CURRENT_LIMIT),
POWER_SUPPLY_ATTR(INPUT_VOLTAGE_LIMIT),
POWER_SUPPLY_ATTR(INPUT_POWER_LIMIT),
Didn't this show up when testing before submitting the patch?
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 1/2] power: supply: fix charge_behaviour attribute initialization
2022-01-04 23:03 Warning since "power: supply: add charge_behaviour attributes" Heiner Kallweit
@ 2022-01-05 6:42 ` Thomas Weißschuh
2022-01-05 10:30 ` Hans de Goede
2022-01-05 6:42 ` [PATCH 2/2] power: supply: validate size of power_supply_attrs at compiletime Thomas Weißschuh
2022-01-05 10:28 ` Warning since "power: supply: add charge_behaviour attributes" Hans de Goede
2 siblings, 1 reply; 6+ messages in thread
From: Thomas Weißschuh @ 2022-01-05 6:42 UTC (permalink / raw)
To: Heiner Kallweit, Sebastian Reichel, Hans de Goede
Cc: Thomas Weißschuh, linux-pm, linux-kernel
All properties have to be added to power_supply_attrs which was missed
before.
Fixes: 1b0b6cc8030d ("power: supply: add charge_behaviour attributes")
Reported-by: Heiner Kallweit <hkallweit1@gmail.com>
Suggested-by: Heiner Kallweit <hkallweit1@gmail.com>
Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
drivers/power/supply/power_supply_sysfs.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
index 5e3b8c15ddbe..491ffec7bf47 100644
--- a/drivers/power/supply/power_supply_sysfs.c
+++ b/drivers/power/supply/power_supply_sysfs.c
@@ -178,6 +178,7 @@ static struct power_supply_attr power_supply_attrs[] = {
POWER_SUPPLY_ATTR(CHARGE_CONTROL_LIMIT_MAX),
POWER_SUPPLY_ATTR(CHARGE_CONTROL_START_THRESHOLD),
POWER_SUPPLY_ATTR(CHARGE_CONTROL_END_THRESHOLD),
+ POWER_SUPPLY_ENUM_ATTR(CHARGE_BEHAVIOUR),
POWER_SUPPLY_ATTR(INPUT_CURRENT_LIMIT),
POWER_SUPPLY_ATTR(INPUT_VOLTAGE_LIMIT),
POWER_SUPPLY_ATTR(INPUT_POWER_LIMIT),
base-commit: 998e7ea8c641fc6bbca1acd478c6824733ac9851
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] power: supply: fix charge_behaviour attribute initialization
2022-01-05 6:42 ` [PATCH 1/2] power: supply: fix charge_behaviour attribute initialization Thomas Weißschuh
@ 2022-01-05 10:30 ` Hans de Goede
0 siblings, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2022-01-05 10:30 UTC (permalink / raw)
To: Thomas Weißschuh, Heiner Kallweit, Sebastian Reichel
Cc: linux-pm, linux-kernel
Hi,
On 1/5/22 07:42, Thomas Weißschuh wrote:
> All properties have to be added to power_supply_attrs which was missed
> before.
>
> Fixes: 1b0b6cc8030d ("power: supply: add charge_behaviour attributes")
> Reported-by: Heiner Kallweit <hkallweit1@gmail.com>
> Suggested-by: Heiner Kallweit <hkallweit1@gmail.com>
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
Thank you for the quick fix.
Since the base commit for this is from pdx86/for-next I'm merging this
fix through the pdx86 tree too:
Thank you for your patch, I've applied this patch to my review-hans
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.
Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.
Regards,
Hans
> ---
> drivers/power/supply/power_supply_sysfs.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
> index 5e3b8c15ddbe..491ffec7bf47 100644
> --- a/drivers/power/supply/power_supply_sysfs.c
> +++ b/drivers/power/supply/power_supply_sysfs.c
> @@ -178,6 +178,7 @@ static struct power_supply_attr power_supply_attrs[] = {
> POWER_SUPPLY_ATTR(CHARGE_CONTROL_LIMIT_MAX),
> POWER_SUPPLY_ATTR(CHARGE_CONTROL_START_THRESHOLD),
> POWER_SUPPLY_ATTR(CHARGE_CONTROL_END_THRESHOLD),
> + POWER_SUPPLY_ENUM_ATTR(CHARGE_BEHAVIOUR),
> POWER_SUPPLY_ATTR(INPUT_CURRENT_LIMIT),
> POWER_SUPPLY_ATTR(INPUT_VOLTAGE_LIMIT),
> POWER_SUPPLY_ATTR(INPUT_POWER_LIMIT),
>
> base-commit: 998e7ea8c641fc6bbca1acd478c6824733ac9851
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] power: supply: validate size of power_supply_attrs at compiletime
2022-01-04 23:03 Warning since "power: supply: add charge_behaviour attributes" Heiner Kallweit
2022-01-05 6:42 ` [PATCH 1/2] power: supply: fix charge_behaviour attribute initialization Thomas Weißschuh
@ 2022-01-05 6:42 ` Thomas Weißschuh
2022-01-05 6:49 ` Thomas Weißschuh
2022-01-05 10:28 ` Warning since "power: supply: add charge_behaviour attributes" Hans de Goede
2 siblings, 1 reply; 6+ messages in thread
From: Thomas Weißschuh @ 2022-01-05 6:42 UTC (permalink / raw)
To: Heiner Kallweit, Sebastian Reichel, Hans de Goede
Cc: Thomas Weißschuh, linux-pm, linux-kernel
For each member of enum power_supply_property a matching entry in
power_supply_attrs is needed.
Add a basic test at compiletime to validate this in addition to the
existing runtime testing.
Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
drivers/power/supply/power_supply_sysfs.c | 2 ++
include/linux/power_supply.h | 2 ++
2 files changed, 4 insertions(+)
diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
index 491ffec7bf47..2565052a7a8c 100644
--- a/drivers/power/supply/power_supply_sysfs.c
+++ b/drivers/power/supply/power_supply_sysfs.c
@@ -403,6 +403,8 @@ void power_supply_init_attrs(struct device_type *dev_type)
{
int i;
+ BUILD_BUG_ON(ARRAY_SIZE(power_supply_attrs) != __POWER_SUPPLY_PROP_CNT);
+
dev_type->groups = power_supply_attr_groups;
for (i = 0; i < ARRAY_SIZE(power_supply_attrs); i++) {
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 71f0379c2af8..60853f26e25f 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -172,6 +172,8 @@ enum power_supply_property {
POWER_SUPPLY_PROP_MODEL_NAME,
POWER_SUPPLY_PROP_MANUFACTURER,
POWER_SUPPLY_PROP_SERIAL_NUMBER,
+
+ __POWER_SUPPLY_PROP_CNT
};
enum power_supply_type {
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] power: supply: validate size of power_supply_attrs at compiletime
2022-01-05 6:42 ` [PATCH 2/2] power: supply: validate size of power_supply_attrs at compiletime Thomas Weißschuh
@ 2022-01-05 6:49 ` Thomas Weißschuh
0 siblings, 0 replies; 6+ messages in thread
From: Thomas Weißschuh @ 2022-01-05 6:49 UTC (permalink / raw)
To: Heiner Kallweit, Sebastian Reichel, Hans de Goede; +Cc: linux-pm, linux-kernel
Hi Sebastian, Hans,
On 2022-01-05 07:42+0100, Thomas Weißschuh wrote:
> For each member of enum power_supply_property a matching entry in
> power_supply_attrs is needed.
> Add a basic test at compiletime to validate this in addition to the
> existing runtime testing.
>
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
> drivers/power/supply/power_supply_sysfs.c | 2 ++
> include/linux/power_supply.h | 2 ++
> 2 files changed, 4 insertions(+)
>
> diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
> index 491ffec7bf47..2565052a7a8c 100644
> --- a/drivers/power/supply/power_supply_sysfs.c
> +++ b/drivers/power/supply/power_supply_sysfs.c
> @@ -403,6 +403,8 @@ void power_supply_init_attrs(struct device_type *dev_type)
> {
> int i;
>
> + BUILD_BUG_ON(ARRAY_SIZE(power_supply_attrs) != __POWER_SUPPLY_PROP_CNT);
> +
> dev_type->groups = power_supply_attr_groups;
>
> for (i = 0; i < ARRAY_SIZE(power_supply_attrs); i++) {
> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> index 71f0379c2af8..60853f26e25f 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -172,6 +172,8 @@ enum power_supply_property {
> POWER_SUPPLY_PROP_MODEL_NAME,
> POWER_SUPPLY_PROP_MANUFACTURER,
> POWER_SUPPLY_PROP_SERIAL_NUMBER,
> +
> + __POWER_SUPPLY_PROP_CNT
> };
>
> enum power_supply_type {
> --
> 2.34.1
>
Please ignore this patch. It does not do what is tries to do. Sorry for the
noise.
Thomas
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Warning since "power: supply: add charge_behaviour attributes"
2022-01-04 23:03 Warning since "power: supply: add charge_behaviour attributes" Heiner Kallweit
2022-01-05 6:42 ` [PATCH 1/2] power: supply: fix charge_behaviour attribute initialization Thomas Weißschuh
2022-01-05 6:42 ` [PATCH 2/2] power: supply: validate size of power_supply_attrs at compiletime Thomas Weißschuh
@ 2022-01-05 10:28 ` Hans de Goede
2 siblings, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2022-01-05 10:28 UTC (permalink / raw)
To: Heiner Kallweit, Sebastian Reichel, Thomas Weißschuh; +Cc: Linux PM
Hi Heiner,
On 1/5/22 00:03, Heiner Kallweit wrote:
> Since 1b0b6cc8030d ("power: supply: add charge_behaviour attributes") I get the following warning:
> power_supply_init_attrs: Property 37 skipped because it is missing from power_supply_attrs
> Seems the patch misses the following:
>
> diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
> index 2cfce2b2e..ef5109102 100644
> --- a/drivers/power/supply/power_supply_sysfs.c
> +++ b/drivers/power/supply/power_supply_sysfs.c
> @@ -179,6 +179,7 @@ static struct power_supply_attr power_supply_attrs[] = {
> POWER_SUPPLY_ATTR(CHARGE_CONTROL_LIMIT_MAX),
> POWER_SUPPLY_ATTR(CHARGE_CONTROL_START_THRESHOLD),
> POWER_SUPPLY_ATTR(CHARGE_CONTROL_END_THRESHOLD),
> + POWER_SUPPLY_ENUM_ATTR(CHARGE_BEHAVIOUR),
> POWER_SUPPLY_ATTR(INPUT_CURRENT_LIMIT),
> POWER_SUPPLY_ATTR(INPUT_VOLTAGE_LIMIT),
> POWER_SUPPLY_ATTR(INPUT_POWER_LIMIT),
>
> Didn't this show up when testing before submitting the patch?
Thank you for reporting this.
The warning did show up in dmesg, but the attribute still works despite the warning
because it is injected as an "extra" property through the drivers/acpi/battery.c
battery_hook_register() mechanism which uses device_add_groups() to add extra
properties handled by vendor specific extensions.
These extra properties do not use the drivers/power/supply/power_supply_sysfs.c
registered attributed handlers and this caused us to miss this, since everything
still works fine for the current user of the new charge_behaviour attribute
despite the warning.
And we completely missed the warning in dmesg.
Regards,
Hans
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-01-05 10:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-04 23:03 Warning since "power: supply: add charge_behaviour attributes" Heiner Kallweit
2022-01-05 6:42 ` [PATCH 1/2] power: supply: fix charge_behaviour attribute initialization Thomas Weißschuh
2022-01-05 10:30 ` Hans de Goede
2022-01-05 6:42 ` [PATCH 2/2] power: supply: validate size of power_supply_attrs at compiletime Thomas Weißschuh
2022-01-05 6:49 ` Thomas Weißschuh
2022-01-05 10:28 ` Warning since "power: supply: add charge_behaviour attributes" Hans de Goede
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.