Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Sender: Guenter Roeck <groeck7@gmail.com>
Date:   Thu, 27 Apr 2023 10:35:33 -0700
From:   Guenter Roeck <linux@roeck-us.net>
To:     =?iso-8859-1?Q?Joaqu=EDn_Ignacio_Aramend=EDa?= <samsagax@gmail.com>
Cc:     derekjohn.clark@gmail.com, jdelvare@suse.com,
        linux-hwmon@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] hwmon: (oxp-sensors) Add AYANEO 2 and Geek models
Message-ID: <720c2b48-ca48-4ee3-9158-f453e48cb012@roeck-us.net>
References: <20230426184420.99945-1-samsagax@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20230426184420.99945-1-samsagax@gmail.com>
Precedence: bulk

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;