On Sat, 1 Mar 2025 at 16:22, Derek J. Clark <derekjohn.clark@xxxxxxxxx> wrote: > > > > 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> Sure, I can add a closes and tweak the wording. > >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
