diff mbox series

[v3,02/12] hwmon: (oxp-sensors) Add all OneXFly variants

Message ID 20250309112114.1177361-3-lkml@antheas.dev (mailing list archive)
State Handled Elsewhere
Headers show
Series hwmon: (oxpsensors) Add devices, features, fix ABI and move to platform/x86 | expand

Commit Message

Antheas Kapenekakis March 9, 2025, 11:21 a.m. UTC
Currently, the driver only has the F1 OneXFly variant, which was based
on the 7000 AMD platform. Add its special editions: F1 EVA-01, F1 OLED.
F1 OLED might have been a dev unit, but it is supported by OneXConsole
with the same features so add it. Then add the F1L variant which is
based on the 8000 AMD platform and the F1Pro and its special edition
EVA-02.

One might ask why not just fuzzy match. Well, EVA-02 is a variant of
F1Pro which is a Strix Point handheld, but does not have F1Pro in its
name. This makes it risky to fuzzy match, as special variants in the
future from different platforms might not have the same feature set
or registers.

By happenstance, all current devices use the same registers. For the
charge limitting feature on this series, only F1Pro/X1 (AMD) were
released with it, but OneXPlayer is providing bios updates for F1, F1L,
X1 Mini units that use the same register, so treat all of them the same.

Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
 drivers/hwmon/oxp-sensors.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

Comments

Guenter Roeck March 9, 2025, 3:24 p.m. UTC | #1
On 3/9/25 04:21, Antheas Kapenekakis wrote:
> Currently, the driver only has the F1 OneXFly variant, which was based
> on the 7000 AMD platform. Add its special editions: F1 EVA-01, F1 OLED.
> F1 OLED might have been a dev unit, but it is supported by OneXConsole
> with the same features so add it. Then add the F1L variant which is
> based on the 8000 AMD platform and the F1Pro and its special edition
> EVA-02.
> 
> One might ask why not just fuzzy match. Well, EVA-02 is a variant of
> F1Pro which is a Strix Point handheld, but does not have F1Pro in its
> name. This makes it risky to fuzzy match, as special variants in the
> future from different platforms might not have the same feature set
> or registers.
> 
> By happenstance, all current devices use the same registers. For the
> charge limitting feature on this series, only F1Pro/X1 (AMD) were
> released with it, but OneXPlayer is providing bios updates for F1, F1L,
> X1 Mini units that use the same register, so treat all of them the same.
> 
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>

Acked-by: Guenter Roeck <linux@roeck-us.net>
Derek John Clark March 10, 2025, 11:03 p.m. UTC | #2
On Sun, Mar 9, 2025 at 4:21 AM Antheas Kapenekakis <lkml@antheas.dev> wrote:
>
> Currently, the driver only has the F1 OneXFly variant, which was based
> on the 7000 AMD platform. Add its special editions: F1 EVA-01, F1 OLED.
> F1 OLED might have been a dev unit, but it is supported by OneXConsole
> with the same features so add it. Then add the F1L variant which is
> based on the 8000 AMD platform and the F1Pro and its special edition
> EVA-02.
>
> One might ask why not just fuzzy match. Well, EVA-02 is a variant of
> F1Pro which is a Strix Point handheld, but does not have F1Pro in its
> name. This makes it risky to fuzzy match, as special variants in the
> future from different platforms might not have the same feature set
> or registers.
>
> By happenstance, all current devices use the same registers. For the
> charge limitting feature on this series, only F1Pro/X1 (AMD) were
> released with it, but OneXPlayer is providing bios updates for F1, F1L,
> X1 Mini units that use the same register, so treat all of them the same.
>
Greeting Antheas,

Do we know the BIOS version(s) that support was added? If so, I think
it makes sense to treat these as separate devices  and check for
device specific BIOS version in an is_visible for the charge limit
attr. I expect that calling the registers when support isn't present
will just be a no-op based on how OXP historically does things, but
having a present attribute that has no effect will probably generate
bug reports. It is also not appropriate to check/fix this in userspace
as some folks might use udev to set it over a program with such
checks.

Cheers,
- Derek

> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
>  drivers/hwmon/oxp-sensors.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
>
> diff --git a/drivers/hwmon/oxp-sensors.c b/drivers/hwmon/oxp-sensors.c
> index 5a4230ad3757..f7a64fbc8f33 100644
> --- a/drivers/hwmon/oxp-sensors.c
> +++ b/drivers/hwmon/oxp-sensors.c
> @@ -188,6 +188,41 @@ static const struct dmi_system_id dmi_table[] = {
>                 },
>                 .driver_data = (void *)oxp_fly,
>         },
> +       {
> +               .matches = {
> +                       DMI_MATCH(DMI_BOARD_VENDOR, "ONE-NETBOOK"),
> +                       DMI_EXACT_MATCH(DMI_BOARD_NAME, "ONEXPLAYER F1 EVA-01"),
> +               },
> +               .driver_data = (void *)oxp_fly,
> +       },
> +       {
> +               .matches = {
> +                       DMI_MATCH(DMI_BOARD_VENDOR, "ONE-NETBOOK"),
> +                       DMI_EXACT_MATCH(DMI_BOARD_NAME, "ONEXPLAYER F1 OLED"),
> +               },
> +               .driver_data = (void *)oxp_fly,
> +       },
> +       {
> +               .matches = {
> +                       DMI_MATCH(DMI_BOARD_VENDOR, "ONE-NETBOOK"),
> +                       DMI_EXACT_MATCH(DMI_BOARD_NAME, "ONEXPLAYER F1L"),
> +               },
> +               .driver_data = (void *)oxp_fly,
> +       },
> +       {
> +               .matches = {
> +                       DMI_MATCH(DMI_BOARD_VENDOR, "ONE-NETBOOK"),
> +                       DMI_EXACT_MATCH(DMI_BOARD_NAME, "ONEXPLAYER F1Pro"),
> +               },
> +               .driver_data = (void *)oxp_fly,
> +       },
> +       {
> +               .matches = {
> +                       DMI_MATCH(DMI_BOARD_VENDOR, "ONE-NETBOOK"),
> +                       DMI_EXACT_MATCH(DMI_BOARD_NAME, "ONEXPLAYER F1 EVA-02"),
> +               },
> +               .driver_data = (void *)oxp_fly,
> +       },
>         {
>                 .matches = {
>                         DMI_MATCH(DMI_BOARD_VENDOR, "ONE-NETBOOK"),
> --
> 2.48.1
>
Antheas Kapenekakis March 10, 2025, 11:19 p.m. UTC | #3
On Tue, 11 Mar 2025 at 00:04, Derek John Clark
<derekjohn.clark@gmail.com> wrote:
>
> On Sun, Mar 9, 2025 at 4:21 AM Antheas Kapenekakis <lkml@antheas.dev> wrote:
> >
> > Currently, the driver only has the F1 OneXFly variant, which was based
> > on the 7000 AMD platform. Add its special editions: F1 EVA-01, F1 OLED.
> > F1 OLED might have been a dev unit, but it is supported by OneXConsole
> > with the same features so add it. Then add the F1L variant which is
> > based on the 8000 AMD platform and the F1Pro and its special edition
> > EVA-02.
> >
> > One might ask why not just fuzzy match. Well, EVA-02 is a variant of
> > F1Pro which is a Strix Point handheld, but does not have F1Pro in its
> > name. This makes it risky to fuzzy match, as special variants in the
> > future from different platforms might not have the same feature set
> > or registers.
> >
> > By happenstance, all current devices use the same registers. For the
> > charge limitting feature on this series, only F1Pro/X1 (AMD) were
> > released with it, but OneXPlayer is providing bios updates for F1, F1L,
> > X1 Mini units that use the same register, so treat all of them the same.
> >
> Greeting Antheas,
>
> Do we know the BIOS version(s) that support was added? If so, I think
> it makes sense to treat these as separate devices  and check for
> device specific BIOS version in an is_visible for the charge limit
> attr. I expect that calling the registers when support isn't present
> will just be a no-op based on how OXP historically does things, but
> having a present attribute that has no effect will probably generate
> bug reports. It is also not appropriate to check/fix this in userspace
> as some folks might use udev to set it over a program with such
> checks.
>
> Cheers,
> - Derek

Unfortunately, I do not know the BIOS versions to check for older
OneXFly devices.

OneXPlayer has informed their users they will need to update their
BIOS and nobody has asked up to now, so I do not expect that to be
that much of an issue. The ones that will need to update know that
their device does not support it currently as they bought it without
the feature. I think we rolled out the GUI for it 3 weeks ago now.

Eileen will know more about that and might be able to provide some
BIOS ranges. I do not expect that to be that much of a problem
though.Yes, it will noop as the register is unused.

> > Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> > ---
> >  drivers/hwmon/oxp-sensors.c | 35 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> >
> > diff --git a/drivers/hwmon/oxp-sensors.c b/drivers/hwmon/oxp-sensors.c
> > index 5a4230ad3757..f7a64fbc8f33 100644
> > --- a/drivers/hwmon/oxp-sensors.c
> > +++ b/drivers/hwmon/oxp-sensors.c
> > @@ -188,6 +188,41 @@ static const struct dmi_system_id dmi_table[] = {
> >                 },
> >                 .driver_data = (void *)oxp_fly,
> >         },
> > +       {
> > +               .matches = {
> > +                       DMI_MATCH(DMI_BOARD_VENDOR, "ONE-NETBOOK"),
> > +                       DMI_EXACT_MATCH(DMI_BOARD_NAME, "ONEXPLAYER F1 EVA-01"),
> > +               },
> > +               .driver_data = (void *)oxp_fly,
> > +       },
> > +       {
> > +               .matches = {
> > +                       DMI_MATCH(DMI_BOARD_VENDOR, "ONE-NETBOOK"),
> > +                       DMI_EXACT_MATCH(DMI_BOARD_NAME, "ONEXPLAYER F1 OLED"),
> > +               },
> > +               .driver_data = (void *)oxp_fly,
> > +       },
> > +       {
> > +               .matches = {
> > +                       DMI_MATCH(DMI_BOARD_VENDOR, "ONE-NETBOOK"),
> > +                       DMI_EXACT_MATCH(DMI_BOARD_NAME, "ONEXPLAYER F1L"),
> > +               },
> > +               .driver_data = (void *)oxp_fly,
> > +       },
> > +       {
> > +               .matches = {
> > +                       DMI_MATCH(DMI_BOARD_VENDOR, "ONE-NETBOOK"),
> > +                       DMI_EXACT_MATCH(DMI_BOARD_NAME, "ONEXPLAYER F1Pro"),
> > +               },
> > +               .driver_data = (void *)oxp_fly,
> > +       },
> > +       {
> > +               .matches = {
> > +                       DMI_MATCH(DMI_BOARD_VENDOR, "ONE-NETBOOK"),
> > +                       DMI_EXACT_MATCH(DMI_BOARD_NAME, "ONEXPLAYER F1 EVA-02"),
> > +               },
> > +               .driver_data = (void *)oxp_fly,
> > +       },
> >         {
> >                 .matches = {
> >                         DMI_MATCH(DMI_BOARD_VENDOR, "ONE-NETBOOK"),
> > --
> > 2.48.1
> >
diff mbox series

Patch

diff --git a/drivers/hwmon/oxp-sensors.c b/drivers/hwmon/oxp-sensors.c
index 5a4230ad3757..f7a64fbc8f33 100644
--- a/drivers/hwmon/oxp-sensors.c
+++ b/drivers/hwmon/oxp-sensors.c
@@ -188,6 +188,41 @@  static const struct dmi_system_id dmi_table[] = {
 		},
 		.driver_data = (void *)oxp_fly,
 	},
+	{
+		.matches = {
+			DMI_MATCH(DMI_BOARD_VENDOR, "ONE-NETBOOK"),
+			DMI_EXACT_MATCH(DMI_BOARD_NAME, "ONEXPLAYER F1 EVA-01"),
+		},
+		.driver_data = (void *)oxp_fly,
+	},
+	{
+		.matches = {
+			DMI_MATCH(DMI_BOARD_VENDOR, "ONE-NETBOOK"),
+			DMI_EXACT_MATCH(DMI_BOARD_NAME, "ONEXPLAYER F1 OLED"),
+		},
+		.driver_data = (void *)oxp_fly,
+	},
+	{
+		.matches = {
+			DMI_MATCH(DMI_BOARD_VENDOR, "ONE-NETBOOK"),
+			DMI_EXACT_MATCH(DMI_BOARD_NAME, "ONEXPLAYER F1L"),
+		},
+		.driver_data = (void *)oxp_fly,
+	},
+	{
+		.matches = {
+			DMI_MATCH(DMI_BOARD_VENDOR, "ONE-NETBOOK"),
+			DMI_EXACT_MATCH(DMI_BOARD_NAME, "ONEXPLAYER F1Pro"),
+		},
+		.driver_data = (void *)oxp_fly,
+	},
+	{
+		.matches = {
+			DMI_MATCH(DMI_BOARD_VENDOR, "ONE-NETBOOK"),
+			DMI_EXACT_MATCH(DMI_BOARD_NAME, "ONEXPLAYER F1 EVA-02"),
+		},
+		.driver_data = (void *)oxp_fly,
+	},
 	{
 		.matches = {
 			DMI_MATCH(DMI_BOARD_VENDOR, "ONE-NETBOOK"),