Subject: [PATCH v2] hwmon: (oxp-sensors) Add tt_toggle attribute on supported boards

OneXPlayer boards from the last generation (both for OneXPlayer and AOK
ZOE brands) have a toggle in the EC to switch the "Turbo/Silent" button
into a different keyboard event.

Add a means to use that "Turbo button takeover" function and expose it
to userspace in a custom sysfs `tt_toggle` attribute. It can be read to
take the current state. Write 1|0 to activate the function. The specific
keycode is dependent on the board but can be checked by running
`evtest` utility.

Newer BIOS on the OneXPlayer added this function aside from string changes.
Add a board enum to differentiate it from the old OneXplayer Mini AMD BIOS.

Currently known supported boards:
- AOK ZOE A1
- OneXPlayer Mini AMD (only newer BIOS version supported)
- OneXPlayer Mini Pro

Signed-off-by: Joaquín Ignacio Aramendía <[email protected]>
---
v2 changes:
- Attach the attribute to the platform device as per review
- Make the attribute return status 0|1 instead of read value
---
Documentation/hwmon/oxp-sensors.rst | 16 ++++
drivers/hwmon/oxp-sensors.c | 142 +++++++++++++++++++++++++++-
2 files changed, 157 insertions(+), 1 deletion(-)

diff --git a/Documentation/hwmon/oxp-sensors.rst b/Documentation/hwmon/oxp-sensors.rst
index 4ab442301415..131c89fad03a 100644
--- a/Documentation/hwmon/oxp-sensors.rst
+++ b/Documentation/hwmon/oxp-sensors.rst
@@ -19,6 +19,11 @@ out the EC registers and values to write to since the EC layout and model is
different. Aya Neo devices preceding the AIR may not be supportable as the EC
model is different and do not appear to have manual control capabilities.

+Some models have a toggle for changing the behaviour of the "Turbo/Silent"
+button of the device. It will change the key event that it triggers with
+a flip of the `tt_toggle` attribute. See below for boards that support this
+function.
+
Supported devices
-----------------

@@ -33,6 +38,11 @@ Currently the driver supports the following handhelds:
- OneXPlayer mini AMD
- OneXPlayer mini AMD PRO

+"Turbo/Silent" button behaviour toggle is only supported on:
+ - AOK ZOE A1
+ - OneXPlayer mini AMD (only with updated alpha BIOS)
+ - OneXPlayer mini AMD PRO
+
Sysfs entries
-------------

@@ -49,3 +59,9 @@ pwm1
Read Write. Read this attribute to see current duty cycle in the range [0-255].
When pwm1_enable is set to "1" (manual) write any value in the range [0-255]
to set fan speed.
+
+tt_toggle
+ Read Write. Read this attribute to check the status of the turbo/silent
+ button behaviour function. Write "1" to activate the switch and "0" to
+ deactivate it. The specific keycodes and behaviour is specific to the device
+ both with this function on and off.
diff --git a/drivers/hwmon/oxp-sensors.c b/drivers/hwmon/oxp-sensors.c
index 0ec7588610ad..59e4e906ced5 100644
--- a/drivers/hwmon/oxp-sensors.c
+++ b/drivers/hwmon/oxp-sensors.c
@@ -47,15 +47,29 @@ enum oxp_board {
aya_neo_air_pro,
aya_neo_geek,
oxp_mini_amd,
+ oxp_mini_amd_a07,
oxp_mini_amd_pro,
};

static enum oxp_board board;

+/* Fan reading and PWM */
#define OXP_SENSOR_FAN_REG 0x76 /* Fan reading is 2 registers long */
#define OXP_SENSOR_PWM_ENABLE_REG 0x4A /* PWM enable is 1 register long */
#define OXP_SENSOR_PWM_REG 0x4B /* PWM reading is 1 register long */

+/* Turbo button takeover function
+ * Older boards have different values and EC registers
+ * for the same function
+ */
+#define OXP_OLD_TURBO_SWITCH_REG 0x1E
+#define OXP_OLD_TURBO_TAKE_VAL 0x01
+#define OXP_OLD_TURBO_RETURN_VAL 0x00
+
+#define OXP_TURBO_SWITCH_REG 0xF1
+#define OXP_TURBO_TAKE_VAL 0x40
+#define OXP_TURBO_RETURN_VAL 0x00
+
static const struct dmi_system_id dmi_table[] = {
{
.matches = {
@@ -104,7 +118,7 @@ static const struct dmi_system_id dmi_table[] = {
DMI_MATCH(DMI_BOARD_VENDOR, "ONE-NETBOOK"),
DMI_EXACT_MATCH(DMI_BOARD_NAME, "ONEXPLAYER mini A07"),
},
- .driver_data = (void *)oxp_mini_amd,
+ .driver_data = (void *)oxp_mini_amd_a07,
},
{
.matches = {
@@ -156,6 +170,112 @@ static int write_to_ec(u8 reg, u8 value)
return ret;
}

+/* Turbo button toggle functions */
+static int tt_toggle_enable(void)
+{
+ u8 reg;
+ u8 val;
+
+ switch (board) {
+ case oxp_mini_amd_a07:
+ reg = OXP_OLD_TURBO_SWITCH_REG;
+ val = OXP_OLD_TURBO_TAKE_VAL;
+ break;
+ case oxp_mini_amd_pro:
+ case aok_zoe_a1:
+ reg = OXP_TURBO_SWITCH_REG;
+ val = OXP_TURBO_TAKE_VAL;
+ break;
+ default:
+ return -EINVAL;
+ }
+ return write_to_ec(reg, val);
+}
+
+static int tt_toggle_disable(void)
+{
+ u8 reg;
+ u8 val;
+
+ switch (board) {
+ case oxp_mini_amd_a07:
+ reg = OXP_OLD_TURBO_SWITCH_REG;
+ val = OXP_OLD_TURBO_RETURN_VAL;
+ break;
+ case oxp_mini_amd_pro:
+ case aok_zoe_a1:
+ reg = OXP_TURBO_SWITCH_REG;
+ val = OXP_TURBO_RETURN_VAL;
+ break;
+ default:
+ return -EINVAL;
+ }
+ return write_to_ec(reg, val);
+}
+
+/* Callbacks for turbo toggle attribute */
+static ssize_t tt_toggle_store(struct device *dev,
+ struct device_attribute *attr, const char *buf,
+ size_t count)
+{
+ int rval;
+ unsigned int value;
+
+ rval = kstrtouint(buf, 10, &value);
+ if (rval)
+ return rval;
+
+ switch (value) {
+ case 0:
+ rval = tt_toggle_disable();
+ if (rval)
+ return rval;
+ return count;
+ case 1:
+ rval = tt_toggle_enable();
+ if (rval)
+ return rval;
+ return count;
+ default:
+ return -EINVAL;
+ }
+}
+
+static ssize_t tt_toggle_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int retval;
+ u8 reg;
+ long val;
+ int status;
+
+ switch (board) {
+ case oxp_mini_amd_a07:
+ reg = OXP_OLD_TURBO_SWITCH_REG;
+ break;
+ case oxp_mini_amd_pro:
+ case aok_zoe_a1:
+ reg = OXP_TURBO_SWITCH_REG;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ retval = read_from_ec(reg, 1, &val);
+ if (retval)
+ return retval;
+
+ if (val)
+ status = 1;
+ else
+ status = 0;
+
+ return sysfs_emit(buf, "%d\n", status);
+}
+
+static DEVICE_ATTR_RW(tt_toggle);
+
+/* PWM enable/disable functions */
static int oxp_pwm_enable(void)
{
return write_to_ec(OXP_SENSOR_PWM_ENABLE_REG, 0x01);
@@ -206,6 +326,7 @@ static int oxp_platform_read(struct device *dev, enum hwmon_sensor_types type,
case aya_neo_air_pro:
case aya_neo_geek:
case oxp_mini_amd:
+ case oxp_mini_amd_a07:
*val = (*val * 255) / 100;
break;
case oxp_mini_amd_pro:
@@ -247,6 +368,7 @@ static int oxp_platform_write(struct device *dev, enum hwmon_sensor_types type,
case aya_neo_air_pro:
case aya_neo_geek:
case oxp_mini_amd:
+ case oxp_mini_amd_a07:
val = (val * 100) / 255;
break;
case aok_zoe_a1:
@@ -274,6 +396,13 @@ static const struct hwmon_channel_info * const oxp_platform_sensors[] = {
NULL,
};

+static struct attribute *oxp_ec_attrs[] = {
+ &dev_attr_tt_toggle.attr,
+ NULL
+};
+
+ATTRIBUTE_GROUPS(oxp_ec);
+
static const struct hwmon_ops oxp_ec_hwmon_ops = {
.is_visible = oxp_ec_hwmon_is_visible,
.read = oxp_platform_read,
@@ -291,6 +420,7 @@ static int oxp_platform_probe(struct platform_device *pdev)
const struct dmi_system_id *dmi_entry;
struct device *dev = &pdev->dev;
struct device *hwdev;
+ int ret;

/*
* Have to check for AMD processor here because DMI strings are the
@@ -305,6 +435,16 @@ static int oxp_platform_probe(struct platform_device *pdev)

board = (enum oxp_board)(unsigned long)dmi_entry->driver_data;

+ switch (board) {
+ case aok_zoe_a1:
+ case oxp_mini_amd_a07:
+ case oxp_mini_amd_pro:
+ ret = devm_device_add_groups(dev, oxp_ec_groups);
+ if (ret)
+ return ret;
+ default:
+ }
+
hwdev = devm_hwmon_device_register_with_info(dev, "oxpec", NULL,
&oxp_ec_chip_info, NULL);

--
2.41.0



2023-06-10 22:06:20

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH v2] hwmon: (oxp-sensors) Add tt_toggle attribute on supported boards

On 6/10/23 12:23, Joaquín Ignacio Aramendía wrote:
> OneXPlayer boards from the last generation (both for OneXPlayer and AOK
> ZOE brands) have a toggle in the EC to switch the "Turbo/Silent" button
> into a different keyboard event.
>
> Add a means to use that "Turbo button takeover" function and expose it
> to userspace in a custom sysfs `tt_toggle` attribute. It can be read to
> take the current state. Write 1|0 to activate the function. The specific
> keycode is dependent on the board but can be checked by running
> `evtest` utility.
>
> Newer BIOS on the OneXPlayer added this function aside from string changes.
> Add a board enum to differentiate it from the old OneXplayer Mini AMD BIOS.
>
> Currently known supported boards:
> - AOK ZOE A1
> - OneXPlayer Mini AMD (only newer BIOS version supported)
> - OneXPlayer Mini Pro
>
> Signed-off-by: Joaquín Ignacio Aramendía <[email protected]>
> ---
> v2 changes:
> - Attach the attribute to the platform device as per review
> - Make the attribute return status 0|1 instead of read value
> ---
> Documentation/hwmon/oxp-sensors.rst | 16 ++++
> drivers/hwmon/oxp-sensors.c | 142 +++++++++++++++++++++++++++-
> 2 files changed, 157 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/hwmon/oxp-sensors.rst b/Documentation/hwmon/oxp-sensors.rst
> index 4ab442301415..131c89fad03a 100644
> --- a/Documentation/hwmon/oxp-sensors.rst
> +++ b/Documentation/hwmon/oxp-sensors.rst
> @@ -19,6 +19,11 @@ out the EC registers and values to write to since the EC layout and model is
> different. Aya Neo devices preceding the AIR may not be supportable as the EC
> model is different and do not appear to have manual control capabilities.
>
> +Some models have a toggle for changing the behaviour of the "Turbo/Silent"
> +button of the device. It will change the key event that it triggers with
> +a flip of the `tt_toggle` attribute. See below for boards that support this
> +function.
> +
> Supported devices
> -----------------
>
> @@ -33,6 +38,11 @@ Currently the driver supports the following handhelds:
> - OneXPlayer mini AMD
> - OneXPlayer mini AMD PRO
>
> +"Turbo/Silent" button behaviour toggle is only supported on:
> + - AOK ZOE A1
> + - OneXPlayer mini AMD (only with updated alpha BIOS)
> + - OneXPlayer mini AMD PRO
> +
> Sysfs entries
> -------------
>
> @@ -49,3 +59,9 @@ pwm1
> Read Write. Read this attribute to see current duty cycle in the range [0-255].
> When pwm1_enable is set to "1" (manual) write any value in the range [0-255]
> to set fan speed.
> +
> +tt_toggle
> + Read Write. Read this attribute to check the status of the turbo/silent
> + button behaviour function. Write "1" to activate the switch and "0" to
> + deactivate it. The specific keycodes and behaviour is specific to the device
> + both with this function on and off.

Mention that this attribute is attached to the platform device.

> diff --git a/drivers/hwmon/oxp-sensors.c b/drivers/hwmon/oxp-sensors.c
> index 0ec7588610ad..59e4e906ced5 100644
> --- a/drivers/hwmon/oxp-sensors.c
> +++ b/drivers/hwmon/oxp-sensors.c
> @@ -47,15 +47,29 @@ enum oxp_board {
> aya_neo_air_pro,
> aya_neo_geek,
> oxp_mini_amd,
> + oxp_mini_amd_a07,
> oxp_mini_amd_pro,
> };
>
> static enum oxp_board board;
>
> +/* Fan reading and PWM */
> #define OXP_SENSOR_FAN_REG 0x76 /* Fan reading is 2 registers long */
> #define OXP_SENSOR_PWM_ENABLE_REG 0x4A /* PWM enable is 1 register long */
> #define OXP_SENSOR_PWM_REG 0x4B /* PWM reading is 1 register long */
>
> +/* Turbo button takeover function
> + * Older boards have different values and EC registers
> + * for the same function
> + */
> +#define OXP_OLD_TURBO_SWITCH_REG 0x1E
> +#define OXP_OLD_TURBO_TAKE_VAL 0x01
> +#define OXP_OLD_TURBO_RETURN_VAL 0x00
> +
> +#define OXP_TURBO_SWITCH_REG 0xF1
> +#define OXP_TURBO_TAKE_VAL 0x40
> +#define OXP_TURBO_RETURN_VAL 0x00
> +
> static const struct dmi_system_id dmi_table[] = {
> {
> .matches = {
> @@ -104,7 +118,7 @@ static const struct dmi_system_id dmi_table[] = {
> DMI_MATCH(DMI_BOARD_VENDOR, "ONE-NETBOOK"),
> DMI_EXACT_MATCH(DMI_BOARD_NAME, "ONEXPLAYER mini A07"),
> },
> - .driver_data = (void *)oxp_mini_amd,
> + .driver_data = (void *)oxp_mini_amd_a07,
> },
> {
> .matches = {
> @@ -156,6 +170,112 @@ static int write_to_ec(u8 reg, u8 value)
> return ret;
> }
>
> +/* Turbo button toggle functions */
> +static int tt_toggle_enable(void)
> +{
> + u8 reg;
> + u8 val;
> +
> + switch (board) {
> + case oxp_mini_amd_a07:
> + reg = OXP_OLD_TURBO_SWITCH_REG;
> + val = OXP_OLD_TURBO_TAKE_VAL;
> + break;
> + case oxp_mini_amd_pro:
> + case aok_zoe_a1:
> + reg = OXP_TURBO_SWITCH_REG;
> + val = OXP_TURBO_TAKE_VAL;
> + break;
> + default:
> + return -EINVAL;
> + }
> + return write_to_ec(reg, val);
> +}
> +
> +static int tt_toggle_disable(void)
> +{
> + u8 reg;
> + u8 val;
> +
> + switch (board) {
> + case oxp_mini_amd_a07:
> + reg = OXP_OLD_TURBO_SWITCH_REG;
> + val = OXP_OLD_TURBO_RETURN_VAL;
> + break;
> + case oxp_mini_amd_pro:
> + case aok_zoe_a1:
> + reg = OXP_TURBO_SWITCH_REG;
> + val = OXP_TURBO_RETURN_VAL;
> + break;
> + default:
> + return -EINVAL;
> + }
> + return write_to_ec(reg, val);
> +}
> +
> +/* Callbacks for turbo toggle attribute */
> +static ssize_t tt_toggle_store(struct device *dev,
> + struct device_attribute *attr, const char *buf,
> + size_t count)
> +{
> + int rval;
> + unsigned int value;
> +
> + rval = kstrtouint(buf, 10, &value);
> + if (rval)
> + return rval;

Please use kstrtobool().

> +
> + switch (value) {
> + case 0:
> + rval = tt_toggle_disable();
> + if (rval)
> + return rval;
> + return count;
> + case 1:
> + rval = tt_toggle_enable();
> + if (rval)
> + return rval;
> + return count;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static ssize_t tt_toggle_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + int retval;
> + u8 reg;
> + long val;
> + int status;
> +
> + switch (board) {
> + case oxp_mini_amd_a07:
> + reg = OXP_OLD_TURBO_SWITCH_REG;
> + break;
> + case oxp_mini_amd_pro:
> + case aok_zoe_a1:
> + reg = OXP_TURBO_SWITCH_REG;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + retval = read_from_ec(reg, 1, &val);
> + if (retval)
> + return retval;
> +
> + if (val)
> + status = 1;
> + else
> + status = 0;
> +
> + return sysfs_emit(buf, "%d\n", status);

return sysfs_emit(buf, "%d\n", !!val);

is much simpler.

> +}
> +
> +static DEVICE_ATTR_RW(tt_toggle);
> +
> +/* PWM enable/disable functions */
> static int oxp_pwm_enable(void)
> {
> return write_to_ec(OXP_SENSOR_PWM_ENABLE_REG, 0x01);
> @@ -206,6 +326,7 @@ static int oxp_platform_read(struct device *dev, enum hwmon_sensor_types type,
> case aya_neo_air_pro:
> case aya_neo_geek:
> case oxp_mini_amd:
> + case oxp_mini_amd_a07:
> *val = (*val * 255) / 100;
> break;
> case oxp_mini_amd_pro:
> @@ -247,6 +368,7 @@ static int oxp_platform_write(struct device *dev, enum hwmon_sensor_types type,
> case aya_neo_air_pro:
> case aya_neo_geek:
> case oxp_mini_amd:
> + case oxp_mini_amd_a07:
> val = (val * 100) / 255;
> break;
> case aok_zoe_a1:
> @@ -274,6 +396,13 @@ static const struct hwmon_channel_info * const oxp_platform_sensors[] = {
> NULL,
> };
>
> +static struct attribute *oxp_ec_attrs[] = {
> + &dev_attr_tt_toggle.attr,
> + NULL
> +};
> +
> +ATTRIBUTE_GROUPS(oxp_ec);
> +
> static const struct hwmon_ops oxp_ec_hwmon_ops = {
> .is_visible = oxp_ec_hwmon_is_visible,
> .read = oxp_platform_read,
> @@ -291,6 +420,7 @@ static int oxp_platform_probe(struct platform_device *pdev)
> const struct dmi_system_id *dmi_entry;
> struct device *dev = &pdev->dev;
> struct device *hwdev;
> + int ret;
>
> /*
> * Have to check for AMD processor here because DMI strings are the
> @@ -305,6 +435,16 @@ static int oxp_platform_probe(struct platform_device *pdev)
>
> board = (enum oxp_board)(unsigned long)dmi_entry->driver_data;
>
> + switch (board) {
> + case aok_zoe_a1:
> + case oxp_mini_amd_a07:
> + case oxp_mini_amd_pro:
> + ret = devm_device_add_groups(dev, oxp_ec_groups);
> + if (ret)
> + return ret;
break;

> + default:

break;

> + }
> +
> hwdev = devm_hwmon_device_register_with_info(dev, "oxpec", NULL,
> &oxp_ec_chip_info, NULL);
>


2023-06-11 00:20:53

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v2] hwmon: (oxp-sensors) Add tt_toggle attribute on supported boards

Hi Joaqu?n,

kernel test robot noticed the following build errors:

[auto build test ERROR on groeck-staging/hwmon-next]
[also build test ERROR on next-20230609]
[cannot apply to linus/master v6.4-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Joaqu-n-Ignacio-Aramend-a/hwmon-oxp-sensors-Add-tt_toggle-attribute-on-supported-boards/20230611-032531
base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
patch link: https://lore.kernel.org/r/20230610192400.80611-2-samsagax%40gmail.com
patch subject: [PATCH v2] hwmon: (oxp-sensors) Add tt_toggle attribute on supported boards
config: x86_64-randconfig-a013-20230611 (https://download.01.org/0day-ci/archive/20230611/[email protected]/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project.git 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce (this is a W=1 build):
mkdir -p ~/bin
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
git remote add groeck-staging https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git
git fetch groeck-staging hwmon-next
git checkout groeck-staging/hwmon-next
b4 shazam https://lore.kernel.org/r/[email protected]
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

>> drivers/hwmon/oxp-sensors.c:445:10: error: label at end of compound statement: expected statement
default:
^
;
1 error generated.


vim +445 drivers/hwmon/oxp-sensors.c

416
417 /* Initialization logic */
418 static int oxp_platform_probe(struct platform_device *pdev)
419 {
420 const struct dmi_system_id *dmi_entry;
421 struct device *dev = &pdev->dev;
422 struct device *hwdev;
423 int ret;
424
425 /*
426 * Have to check for AMD processor here because DMI strings are the
427 * same between Intel and AMD boards, the only way to tell them apart
428 * is the CPU.
429 * Intel boards seem to have different EC registers and values to
430 * read/write.
431 */
432 dmi_entry = dmi_first_match(dmi_table);
433 if (!dmi_entry || boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
434 return -ENODEV;
435
436 board = (enum oxp_board)(unsigned long)dmi_entry->driver_data;
437
438 switch (board) {
439 case aok_zoe_a1:
440 case oxp_mini_amd_a07:
441 case oxp_mini_amd_pro:
442 ret = devm_device_add_groups(dev, oxp_ec_groups);
443 if (ret)
444 return ret;
> 445 default:
446 }
447
448 hwdev = devm_hwmon_device_register_with_info(dev, "oxpec", NULL,
449 &oxp_ec_chip_info, NULL);
450
451 return PTR_ERR_OR_ZERO(hwdev);
452 }
453

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki