[PATCH] hwmon: (oxp-sensors) Add AYANEO 2 and Geek models

[PATCH] hwmon: (oxp-sensors) Add AYANEO 2 and Geek models

[Joaquín Ignacio Aramendía]

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 <samsagax@gmail.com>
---
 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

Re: [PATCH] hwmon: (oxp-sensors) Add AYANEO 2 and Geek models

[Guenter Roeck]

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 <samsagax@gmail.com>

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;

Re: [PATCH] hwmon: (oxp-sensors) Add AYANEO 2 and Geek models

[Joaquin Aramendia]

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

Re: [PATCH] hwmon: (oxp-sensors) Add AYANEO 2 and Geek models

[Guenter Roeck]

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

Re: [PATCH] hwmon: (oxp-sensors) Add AYANEO 2 and Geek models

[Joaquin Aramendia]

> > 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

Re: [PATCH] hwmon: (oxp-sensors) Add AYANEO 2 and Geek models

[Guenter Roeck]

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.
>