Add support for handhelds with same EC registers
- AYANEO 2
- AYANEO GEEK
All functionality tests succeed on AYANEO 2 by "pastaq" user on Discord
and AYANEO GEEK tested by "oneoc" Discord user.
Signed-off-by: Joaquín Ignacio Aramendía <[email protected]>
---
Documentation/hwmon/oxp-sensors.rst | 2 ++
drivers/hwmon/oxp-sensors.c | 20 ++++++++++++++++++++
2 files changed, 22 insertions(+)
diff --git a/Documentation/hwmon/oxp-sensors.rst b/Documentation/hwmon/oxp-sensors.rst
index 566a8d5bde08..4ab442301415 100644
--- a/Documentation/hwmon/oxp-sensors.rst
+++ b/Documentation/hwmon/oxp-sensors.rst
@@ -25,8 +25,10 @@ Supported devices
Currently the driver supports the following handhelds:
- AOK ZOE A1
+ - Aya Neo 2
- Aya Neo AIR
- Aya Neo AIR Pro
+ - Aya Neo Geek
- OneXPlayer AMD
- OneXPlayer mini AMD
- OneXPlayer mini AMD PRO
diff --git a/drivers/hwmon/oxp-sensors.c b/drivers/hwmon/oxp-sensors.c
index ae67207030e8..9093c608dee0 100644
--- a/drivers/hwmon/oxp-sensors.c
+++ b/drivers/hwmon/oxp-sensors.c
@@ -42,8 +42,10 @@ static bool unlock_global_acpi_lock(void)
enum oxp_board {
aok_zoe_a1 = 1,
+ aya_neo_2,
aya_neo_air,
aya_neo_air_pro,
+ aya_neo_geek,
oxp_mini_amd,
oxp_mini_amd_pro,
};
@@ -62,6 +64,13 @@ static const struct dmi_system_id dmi_table[] = {
},
.driver_data = (void *) &(enum oxp_board) {aok_zoe_a1},
},
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "AYANEO"),
+ DMI_EXACT_MATCH(DMI_BOARD_NAME, "AYANEO 2"),
+ },
+ .driver_data = (void *) &(enum oxp_board) {aya_neo_2},
+ },
{
.matches = {
DMI_MATCH(DMI_BOARD_VENDOR, "AYANEO"),
@@ -76,6 +85,13 @@ static const struct dmi_system_id dmi_table[] = {
},
.driver_data = (void *) &(enum oxp_board) {aya_neo_air_pro},
},
+ {
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "AYANEO"),
+ DMI_EXACT_MATCH(DMI_BOARD_NAME, "GEEK"),
+ },
+ .driver_data = (void *) &(enum oxp_board) {aya_neo_geek},
+ },
{
.matches = {
DMI_MATCH(DMI_BOARD_VENDOR, "ONE-NETBOOK"),
@@ -178,8 +194,10 @@ static int oxp_platform_read(struct device *dev, enum hwmon_sensor_types type,
if (ret)
return ret;
switch (board) {
+ case aya_neo_2:
case aya_neo_air:
case aya_neo_air_pro:
+ case aya_neo_geek:
case oxp_mini_amd:
*val = (*val * 255) / 100;
break;
@@ -217,8 +235,10 @@ static int oxp_platform_write(struct device *dev, enum hwmon_sensor_types type,
if (val < 0 || val > 255)
return -EINVAL;
switch (board) {
+ case aya_neo_2:
case aya_neo_air:
case aya_neo_air_pro:
+ case aya_neo_geek:
case oxp_mini_amd:
val = (val * 100) / 255;
break;
--
2.40.1
On Wed, Apr 26, 2023 at 03:44:20PM -0300, Joaqu?n Ignacio Aramend?a wrote:
> Add support for handhelds with same EC registers
> - AYANEO 2
> - AYANEO GEEK
>
> All functionality tests succeed on AYANEO 2 by "pastaq" user on Discord
> and AYANEO GEEK tested by "oneoc" Discord user.
>
> Signed-off-by: Joaqu?n Ignacio Aramend?a <[email protected]>
CHECK: No space is necessary after a cast
#130: FILE: drivers/hwmon/oxp-sensors.c:72:
+ .driver_data = (void *) &(enum oxp_board) {aya_neo_2},
CHECK: No space is necessary after a cast
#144: FILE: drivers/hwmon/oxp-sensors.c:93:
+ .driver_data = (void *) &(enum oxp_board) {aya_neo_geek},
Please run checkpatch --strict on your patches. Never mind, I'll apply the
patch anyway - I see the other entries are the same.
That makes me have a closer look at the code. What is the purpose of the
odd typecast anyway ? Why not just
.driver_data = (void *)aya_neo_2,
and
board = (enum oxp_board)dmi_entry->driver_data;
?
Guenter
> ---
> Documentation/hwmon/oxp-sensors.rst | 2 ++
> drivers/hwmon/oxp-sensors.c | 20 ++++++++++++++++++++
> 2 files changed, 22 insertions(+)
>
> diff --git a/Documentation/hwmon/oxp-sensors.rst b/Documentation/hwmon/oxp-sensors.rst
> index 566a8d5bde08..4ab442301415 100644
> --- a/Documentation/hwmon/oxp-sensors.rst
> +++ b/Documentation/hwmon/oxp-sensors.rst
> @@ -25,8 +25,10 @@ Supported devices
> Currently the driver supports the following handhelds:
>
> - AOK ZOE A1
> + - Aya Neo 2
> - Aya Neo AIR
> - Aya Neo AIR Pro
> + - Aya Neo Geek
> - OneXPlayer AMD
> - OneXPlayer mini AMD
> - OneXPlayer mini AMD PRO
> diff --git a/drivers/hwmon/oxp-sensors.c b/drivers/hwmon/oxp-sensors.c
> index ae67207030e8..9093c608dee0 100644
> --- a/drivers/hwmon/oxp-sensors.c
> +++ b/drivers/hwmon/oxp-sensors.c
> @@ -42,8 +42,10 @@ static bool unlock_global_acpi_lock(void)
>
> enum oxp_board {
> aok_zoe_a1 = 1,
> + aya_neo_2,
> aya_neo_air,
> aya_neo_air_pro,
> + aya_neo_geek,
> oxp_mini_amd,
> oxp_mini_amd_pro,
> };
> @@ -62,6 +64,13 @@ static const struct dmi_system_id dmi_table[] = {
> },
> .driver_data = (void *) &(enum oxp_board) {aok_zoe_a1},
> },
> + {
> + .matches = {
> + DMI_MATCH(DMI_BOARD_VENDOR, "AYANEO"),
> + DMI_EXACT_MATCH(DMI_BOARD_NAME, "AYANEO 2"),
> + },
> + .driver_data = (void *) &(enum oxp_board) {aya_neo_2},
> + },
> {
> .matches = {
> DMI_MATCH(DMI_BOARD_VENDOR, "AYANEO"),
> @@ -76,6 +85,13 @@ static const struct dmi_system_id dmi_table[] = {
> },
> .driver_data = (void *) &(enum oxp_board) {aya_neo_air_pro},
> },
> + {
> + .matches = {
> + DMI_MATCH(DMI_BOARD_VENDOR, "AYANEO"),
> + DMI_EXACT_MATCH(DMI_BOARD_NAME, "GEEK"),
> + },
> + .driver_data = (void *) &(enum oxp_board) {aya_neo_geek},
> + },
> {
> .matches = {
> DMI_MATCH(DMI_BOARD_VENDOR, "ONE-NETBOOK"),
> @@ -178,8 +194,10 @@ static int oxp_platform_read(struct device *dev, enum hwmon_sensor_types type,
> if (ret)
> return ret;
> switch (board) {
> + case aya_neo_2:
> case aya_neo_air:
> case aya_neo_air_pro:
> + case aya_neo_geek:
> case oxp_mini_amd:
> *val = (*val * 255) / 100;
> break;
> @@ -217,8 +235,10 @@ static int oxp_platform_write(struct device *dev, enum hwmon_sensor_types type,
> if (val < 0 || val > 255)
> return -EINVAL;
> switch (board) {
> + case aya_neo_2:
> case aya_neo_air:
> case aya_neo_air_pro:
> + case aya_neo_geek:
> case oxp_mini_amd:
> val = (val * 100) / 255;
> break;
Hello Guenter and thanks for your quick review.
> Please run checkpatch --strict on your patches. Never mind, I'll apply the
> patch anyway - I see the other entries are the same.
I've run it before on the other patches... did something change in the
checkpatch?
Nevermid, After this one I may submit a patch to fix all styling in one go
> That makes me have a closer look at the code. What is the purpose of the
> odd typecast anyway ? Why not just
> .driver_data = (void *)aya_neo_2,
> and
> board = (enum oxp_board)dmi_entry->driver_data;
> ?
I don't know why but the compiler would complain with the casting from
enum to void*.
Found out that explicitly casting the enum literal before casting it
to void* works and
the compiler stopped complaining so I went with it.
As a follow up question, since this driver has become more AYANEO than
OXP since its inception,
wouldn't it be better to change the description?
Joaquín Aramendía
On 4/27/23 16:16, Joaquin Aramendia wrote:
> Hello Guenter and thanks for your quick review.
>
>> Please run checkpatch --strict on your patches. Never mind, I'll apply the
>> patch anyway - I see the other entries are the same.
> I've run it before on the other patches... did something change in the
> checkpatch?
> Nevermid, After this one I may submit a patch to fix all styling in one go
>
>> That makes me have a closer look at the code. What is the purpose of the
>> odd typecast anyway ? Why not just
>> .driver_data = (void *)aya_neo_2,
>> and
>> board = (enum oxp_board)dmi_entry->driver_data;
>> ?
> I don't know why but the compiler would complain with the casting from
> enum to void*.
Really ? I tried with both 32 bit and 64 bit targets, and the above worked
just fine (with W=1). I tried with gcc 10.3 as well as 11.3. What is
your compiler version, what exactly is the warning/error message you
observed, and how exactly did your code look like ?
> Found out that explicitly casting the enum literal before casting it
> to void* works and
> the compiler stopped complaining so I went with it.
>
You are not casting the enum literal, but a pointer to it.
Guenter
> As a follow up question, since this driver has become more AYANEO than
> OXP since its inception,
> wouldn't it be better to change the description?
>
> Joaquín Aramendía
> > I don't know why but the compiler would complain with the casting from
> > enum to void*.
>
> Really ? I tried with both 32 bit and 64 bit targets, and the above worked
> just fine (with W=1). I tried with gcc 10.3 as well as 11.3. What is
> your compiler version, what exactly is the warning/error message you
> observed, and how exactly did your code look like ?
Said something about an invalid cast of an lvalue? I can´t remember exactly
and couldn't reproduce it again. Will change all to your form after
this patch gets
merged if you like, just to keep things atomic.
>
> > Found out that explicitly casting the enum literal before casting it
> > to void* works and
> > the compiler stopped complaining so I went with it.
> >
>
> You are not casting the enum literal, but a pointer to it.
Yes. I'm taking an enum literal and taking its pointer to cast it to a void*.
A little cumbersome, but maybe it can be avoided by using your proposed
form directly and after making sure it works.
--
Joaquín I. Aramendía
On 4/28/23 12:15, Joaquin Aramendia wrote:
>>> I don't know why but the compiler would complain with the casting from
>>> enum to void*.
>>
>> Really ? I tried with both 32 bit and 64 bit targets, and the above worked
>> just fine (with W=1). I tried with gcc 10.3 as well as 11.3. What is
>> your compiler version, what exactly is the warning/error message you
>> observed, and how exactly did your code look like ?
>
> Said something about an invalid cast of an lvalue? I can´t remember exactly
Like this, maybe ?
drivers/hwmon/oxp-sensors.c:63:26: error: lvalue required as unary ‘&’ operand
That would have been something along the line of
.driver_data = (void *)&aok_zoe_a1,
or
driver_data = &aok_zoe_a1,
> and couldn't reproduce it again. Will change all to your form after
> this patch gets
> merged if you like, just to keep things atomic.
>
Please do.
Thanks,
Guenter
>>
>>> Found out that explicitly casting the enum literal before casting it
>>> to void* works and
>>> the compiler stopped complaining so I went with it.
>>>
>>
>> You are not casting the enum literal, but a pointer to it.
>
> Yes. I'm taking an enum literal and taking its pointer to cast it to a void*.
> A little cumbersome, but maybe it can be avoided by using your proposed
> form directly and after making sure it works.
>