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

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

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/202306110659.msus3mCo-lkp@intel.com/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/20230610192400.80611-2-samsagax@gmail.com
        # 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 <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306110659.msus3mCo-lkp@intel.com/

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