| Message ID | 20230426184420.99945-1-samsagax@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | hwmon: (oxp-sensors) Add AYANEO 2 and Geek models | expand |
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;
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.
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. >
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;
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(+)