* [PATCH v1] power: supply: surface_{battery,charger}: Consistently define ssam_device_ids using named initializers
@ 2026-06-15 12:51 Uwe Kleine-König (The Capable Hub)
2026-06-21 13:32 ` Maximilian Luz
0 siblings, 1 reply; 3+ messages in thread
From: Uwe Kleine-König (The Capable Hub) @ 2026-06-15 12:51 UTC (permalink / raw)
To: Maximilian Luz, Sebastian Reichel
Cc: linux-pm, platform-driver-x86, linux-kernel
The .driver_data member of the the two struct ssam_device_id arrays were
initialized by list expressions. This isn't easily readable if you don't
work with the Surface System Aggregator core regularily. Using named
initializers is more explicit and thus easier to parse and also more
robust to changes of the struct definition. This robustness is relevant
for a planned change to struct ssam_device_id replacing .driver_data
by an anonymous union.
While touching these arrays, also drop the comma after the list
terminators.
This change doesn't introduce changes to the compiled ssam_device_id
arrays.
Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
---
Hello,
the mentioned change to ssam_device_id is similar to
https://lore.kernel.org/all/cover.1779878004.git.u.kleine-koenig@baylibre.com/.
That allows to get rid of the casts in these two drivers and thus
benefits a bit more of the (admittedly weak) type safety of C.
But IMHO the improved readability alone also justifies this change.
Best regards
Uwe
drivers/power/supply/surface_battery.c | 11 ++++++++---
drivers/power/supply/surface_charger.c | 7 +++++--
2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/power/supply/surface_battery.c b/drivers/power/supply/surface_battery.c
index c759add4df49..1273b6082311 100644
--- a/drivers/power/supply/surface_battery.c
+++ b/drivers/power/supply/surface_battery.c
@@ -852,9 +852,14 @@ static const struct spwr_psy_properties spwr_psy_props_bat2_sb3 = {
};
static const struct ssam_device_id surface_battery_match[] = {
- { SSAM_SDEV(BAT, SAM, 0x01, 0x00), (unsigned long)&spwr_psy_props_bat1 },
- { SSAM_SDEV(BAT, KIP, 0x01, 0x00), (unsigned long)&spwr_psy_props_bat2_sb3 },
- { },
+ {
+ SSAM_SDEV(BAT, SAM, 0x01, 0x00),
+ .driver_data = (unsigned long)&spwr_psy_props_bat1,
+ }, {
+ SSAM_SDEV(BAT, KIP, 0x01, 0x00),
+ .driver_data = (unsigned long)&spwr_psy_props_bat2_sb3,
+ },
+ { }
};
MODULE_DEVICE_TABLE(ssam, surface_battery_match);
diff --git a/drivers/power/supply/surface_charger.c b/drivers/power/supply/surface_charger.c
index 90b823848c99..d4bba6b41794 100644
--- a/drivers/power/supply/surface_charger.c
+++ b/drivers/power/supply/surface_charger.c
@@ -260,8 +260,11 @@ static const struct spwr_psy_properties spwr_psy_props_adp1 = {
};
static const struct ssam_device_id surface_ac_match[] = {
- { SSAM_SDEV(BAT, SAM, 0x01, 0x01), (unsigned long)&spwr_psy_props_adp1 },
- { },
+ {
+ SSAM_SDEV(BAT, SAM, 0x01, 0x01),
+ .driver_data = (unsigned long)&spwr_psy_props_adp1,
+ },
+ { }
};
MODULE_DEVICE_TABLE(ssam, surface_ac_match);
base-commit: c425609d6ac4012c8bbf01ec2e10e801b1923a7b
--
2.47.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v1] power: supply: surface_{battery,charger}: Consistently define ssam_device_ids using named initializers
2026-06-15 12:51 [PATCH v1] power: supply: surface_{battery,charger}: Consistently define ssam_device_ids using named initializers Uwe Kleine-König (The Capable Hub)
@ 2026-06-21 13:32 ` Maximilian Luz
2026-06-22 9:37 ` Uwe Kleine-König (The Capable Hub)
0 siblings, 1 reply; 3+ messages in thread
From: Maximilian Luz @ 2026-06-21 13:32 UTC (permalink / raw)
To: Uwe Kleine-König (The Capable Hub), Sebastian Reichel
Cc: linux-pm, platform-driver-x86, linux-kernel
Am 15.06.2026 um 14:51 schrieb Uwe Kleine-König (The Capable Hub):
> The .driver_data member of the the two struct ssam_device_id arrays were
> initialized by list expressions. This isn't easily readable if you don't
> work with the Surface System Aggregator core regularily. Using named
> initializers is more explicit and thus easier to parse and also more
> robust to changes of the struct definition. This robustness is relevant
> for a planned change to struct ssam_device_id replacing .driver_data
> by an anonymous union.
>
> While touching these arrays, also drop the comma after the list
> terminators.
>
> This change doesn't introduce changes to the compiled ssam_device_id
> arrays.
>
> Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
> ---
> Hello,
>
> the mentioned change to ssam_device_id is similar to
> https://lore.kernel.org/all/cover.1779878004.git.u.kleine-koenig@baylibre.com/.
>
> That allows to get rid of the casts in these two drivers and thus
> benefits a bit more of the (admittedly weak) type safety of C.
As in the prior patch: I assume that will be a separate change or am I missing something in this patch?
> But IMHO the improved readability alone also justifies this change.
>
> Best regards
> Uwe
Looks good to me.
Reviewed-by: Maximilian Luz <luzmaximilian@gmail.com>
>
> drivers/power/supply/surface_battery.c | 11 ++++++++---
> drivers/power/supply/surface_charger.c | 7 +++++--
> 2 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/power/supply/surface_battery.c b/drivers/power/supply/surface_battery.c
> index c759add4df49..1273b6082311 100644
> --- a/drivers/power/supply/surface_battery.c
> +++ b/drivers/power/supply/surface_battery.c
> @@ -852,9 +852,14 @@ static const struct spwr_psy_properties spwr_psy_props_bat2_sb3 = {
> };
>
> static const struct ssam_device_id surface_battery_match[] = {
> - { SSAM_SDEV(BAT, SAM, 0x01, 0x00), (unsigned long)&spwr_psy_props_bat1 },
> - { SSAM_SDEV(BAT, KIP, 0x01, 0x00), (unsigned long)&spwr_psy_props_bat2_sb3 },
> - { },
> + {
> + SSAM_SDEV(BAT, SAM, 0x01, 0x00),
> + .driver_data = (unsigned long)&spwr_psy_props_bat1,
> + }, {
> + SSAM_SDEV(BAT, KIP, 0x01, 0x00),
> + .driver_data = (unsigned long)&spwr_psy_props_bat2_sb3,
> + },
> + { }
> };
> MODULE_DEVICE_TABLE(ssam, surface_battery_match);
>
> diff --git a/drivers/power/supply/surface_charger.c b/drivers/power/supply/surface_charger.c
> index 90b823848c99..d4bba6b41794 100644
> --- a/drivers/power/supply/surface_charger.c
> +++ b/drivers/power/supply/surface_charger.c
> @@ -260,8 +260,11 @@ static const struct spwr_psy_properties spwr_psy_props_adp1 = {
> };
>
> static const struct ssam_device_id surface_ac_match[] = {
> - { SSAM_SDEV(BAT, SAM, 0x01, 0x01), (unsigned long)&spwr_psy_props_adp1 },
> - { },
> + {
> + SSAM_SDEV(BAT, SAM, 0x01, 0x01),
> + .driver_data = (unsigned long)&spwr_psy_props_adp1,
> + },
> + { }
> };
> MODULE_DEVICE_TABLE(ssam, surface_ac_match);
>
>
> base-commit: c425609d6ac4012c8bbf01ec2e10e801b1923a7b
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v1] power: supply: surface_{battery,charger}: Consistently define ssam_device_ids using named initializers
2026-06-21 13:32 ` Maximilian Luz
@ 2026-06-22 9:37 ` Uwe Kleine-König (The Capable Hub)
0 siblings, 0 replies; 3+ messages in thread
From: Uwe Kleine-König (The Capable Hub) @ 2026-06-22 9:37 UTC (permalink / raw)
To: Maximilian Luz
Cc: Sebastian Reichel, linux-pm, platform-driver-x86, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1528 bytes --]
Hello Maximilian,
On Sun, Jun 21, 2026 at 03:32:45PM +0200, Maximilian Luz wrote:
> Am 15.06.2026 um 14:51 schrieb Uwe Kleine-König (The Capable Hub):
> > The .driver_data member of the the two struct ssam_device_id arrays were
> > initialized by list expressions. This isn't easily readable if you don't
> > work with the Surface System Aggregator core regularily. Using named
> > initializers is more explicit and thus easier to parse and also more
> > robust to changes of the struct definition. This robustness is relevant
> > for a planned change to struct ssam_device_id replacing .driver_data
> > by an anonymous union.
> >
> > While touching these arrays, also drop the comma after the list
> > terminators.
> >
> > This change doesn't introduce changes to the compiled ssam_device_id
> > arrays.
> >
> > Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
> > ---
> > Hello,
> >
> > the mentioned change to ssam_device_id is similar to
> > https://lore.kernel.org/all/cover.1779878004.git.u.kleine-koenig@baylibre.com/.
> >
> > That allows to get rid of the casts in these two drivers and thus
> > benefits a bit more of the (admittedly weak) type safety of C.
>
> As in the prior patch: I assume that will be a separate change or am I
> missing something in this patch?
Yes, this can only happen when all drivers use a named initializer for
.driver_data. As this requires adaptions to many subsystems, it's not
included here.
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-06-22 9:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-15 12:51 [PATCH v1] power: supply: surface_{battery,charger}: Consistently define ssam_device_ids using named initializers Uwe Kleine-König (The Capable Hub)
2026-06-21 13:32 ` Maximilian Luz
2026-06-22 9:37 ` Uwe Kleine-König (The Capable Hub)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox