On February 22, 2025 8:18:18 AM PST, Antheas Kapenekakis <lkml@xxxxxxxxxxx> wrote: >Currently, this driver breaks sysfs by using auto as 0 and manual as 1. It breaks hwmon ABI convention, the sysfs is fully functional. Please be more accurate as maintainers may misunderstand the problem here. This comment applies to 8/12 as well. You should probably link the discussion where this was identified as well for context. <https://lore.kernel.org/linux-hwmon/20241027174836.8588-1-derekjohn.clark@xxxxxxxxx/T/#u> >However, for pwm_enable, 0 is full speed, 1 is manual, and 2 is auto. >For the correction to be possible, this means that the pwm_enable >endpoint will need access to both pwm enable and value (as for >the 0th value, the fan needs to be set to full power). > >Therefore, begin by moving the current pwm_enable read to its own >function, oxp_pwm_enable. > >Signed-off-by: Antheas Kapenekakis <lkml@xxxxxxxxxxx> >--- > drivers/hwmon/oxp-sensors.c | 50 ++++++++++++++++++++----------------- > 1 file changed, 27 insertions(+), 23 deletions(-) > >diff --git a/drivers/hwmon/oxp-sensors.c b/drivers/hwmon/oxp-sensors.c >index 9c43ec0fc994..1da1e1655f96 100644 >--- a/drivers/hwmon/oxp-sensors.c >+++ b/drivers/hwmon/oxp-sensors.c >@@ -762,6 +762,32 @@ static int oxp_pwm_disable(void) > } > } > >+static int oxp_pwm_read(long *val) >+{ >+ switch (board) { >+ case orange_pi_neo: >+ return read_from_ec(ORANGEPI_SENSOR_PWM_ENABLE_REG, 1, val); >+ case aok_zoe_a1: >+ case aya_neo_2: >+ case aya_neo_air: >+ case aya_neo_air_1s: >+ case aya_neo_air_plus_mendo: >+ case aya_neo_air_pro: >+ case aya_neo_flip: >+ case aya_neo_geek: >+ case aya_neo_kun: >+ case oxp_2: >+ case oxp_fly: >+ case oxp_mini_amd: >+ case oxp_mini_amd_a07: >+ case oxp_mini_amd_pro: >+ case oxp_x1: >+ return read_from_ec(OXP_SENSOR_PWM_ENABLE_REG, 1, val); >+ default: >+ return -EOPNOTSUPP; >+ } >+} >+ > /* Callbacks for hwmon interface */ > static umode_t oxp_ec_hwmon_is_visible(const void *drvdata, > enum hwmon_sensor_types type, u32 attr, int channel) >@@ -859,29 +885,7 @@ static int oxp_platform_read(struct device *dev, enum hwmon_sensor_types type, > } > return 0; > case hwmon_pwm_enable: >- switch (board) { >- case orange_pi_neo: >- return read_from_ec(ORANGEPI_SENSOR_PWM_ENABLE_REG, 1, val); >- case aok_zoe_a1: >- case aya_neo_2: >- case aya_neo_air: >- case aya_neo_air_1s: >- case aya_neo_air_plus_mendo: >- case aya_neo_air_pro: >- case aya_neo_flip: >- case aya_neo_geek: >- case aya_neo_kun: >- case oxp_2: >- case oxp_fly: >- case oxp_mini_amd: >- case oxp_mini_amd_a07: >- case oxp_mini_amd_pro: >- case oxp_x1: >- return read_from_ec(OXP_SENSOR_PWM_ENABLE_REG, 1, val); >- default: >- break; >- } >- break; >+ return oxp_pwm_read(val); > default: > break; > } - Derek
