On Thu, Dec 26, 2024 at 11:56:12PM +0100, Tobias Jakobi wrote: > > On 12/26/24 21:52, Guenter Roeck wrote: > > On Thu, Dec 26, 2024 at 06:00:18PM +0100, tjakobi@xxxxxxxxxxxxxxxxxxxxx wrote: > > > From: Tobias Jakobi <tjakobi@xxxxxxxxxxxxxxxxxxxxx> > > > > > > Despite what the current comment says, the register is used > > > both for reading and writing the PWM value. > > > > > > Signed-off-by: Tobias Jakobi <tjakobi@xxxxxxxxxxxxxxxxxxxxx> > > > --- > > > drivers/hwmon/oxp-sensors.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/hwmon/oxp-sensors.c b/drivers/hwmon/oxp-sensors.c > > > index fbd1463d1494..8089349fa508 100644 > > > --- a/drivers/hwmon/oxp-sensors.c > > > +++ b/drivers/hwmon/oxp-sensors.c > > > @@ -46,14 +46,14 @@ static bool unlock_global_acpi_lock(void) > > > #define OXP_SENSOR_FAN_REG 0x76 /* Fan reading is 2 registers long */ > > > #define OXP_2_SENSOR_FAN_REG 0x58 /* 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 */ > > > +#define OXP_SENSOR_PWM_REG 0x4B /* PWM control is 1 register long */ > > > > I think that, if anything, this is more confusing than before. > > "control" is, for example, enabling or disabling pwm management, > > setting automatic or manual mode, or setting the pwm polarity. > > Together ith the next two defines, "control" would suggest that > > PWM_MODE_AUTO and PWM_MODE_MANUAL are set through that register - > > which is not the case. "value" maybe, but "control" is just wrong. > Noted. What do you think about "target" then? > > My main point here was that reading implies that this register is read-only. > Which it clearly isn't. And the documentation (which could be certainly be > improved in general) should reflect that. > "reading and writing the PWM value". "target" implies writing. "register" implies neither, so you could use that. Guenter
