| Message ID | 20250313-extend_ec_hwmon_fan-v1-2-5c566776f2c4@chromium.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Export the target RPM fan control by ChromeOS EC under hwmon | expand |
On 2025-03-13 12:47:43+0800, Sung-Chi Li wrote: > Implement the functionality of reading the target fan RPM setting from > ChromeOS embedded controller under framework. > > Signed-off-by: Sung-Chi Li <lschyi@chromium.org> > --- > drivers/hwmon/cros_ec_hwmon.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/drivers/hwmon/cros_ec_hwmon.c b/drivers/hwmon/cros_ec_hwmon.c > index b2fec0768301f116f49c57b8dbfb042b98a573e1..73bfcbbaf9531be6b753cfef8045fd5dab5b2ab3 100644 > --- a/drivers/hwmon/cros_ec_hwmon.c > +++ b/drivers/hwmon/cros_ec_hwmon.c > @@ -36,6 +36,19 @@ static int cros_ec_hwmon_read_fan_speed(struct cros_ec_device *cros_ec, u8 index > return 0; > } > > +static int cros_ec_hwmon_read_fan_target(struct cros_ec_device *cros_ec, u8 index, int32_t *speed) int32_t is a userspace type. In the kernel use i32, or even better u32. > +{ > + int ret; > + struct ec_response_pwm_get_fan_rpm r; Switch the variable declarations around. Also call the request "req". > + > + ret = cros_ec_cmd(cros_ec, 0, EC_CMD_PWM_GET_FAN_TARGET_RPM, NULL, 0, &r, sizeof(r)); > + if (ret < 0) > + return ret; > + > + *speed = le32_to_cpu(r.rpm); r.rpm is not marked as __le32, I'm not sure if sparse will complain about the usage of le32_to_cpu(). > + return 0; > +} > + > static int cros_ec_hwmon_read_temp(struct cros_ec_device *cros_ec, u8 index, u8 *temp) > { > unsigned int offset; > @@ -95,6 +108,7 @@ static int cros_ec_hwmon_read(struct device *dev, enum hwmon_sensor_types type, > { > struct cros_ec_hwmon_priv *priv = dev_get_drvdata(dev); > int ret = -EOPNOTSUPP; > + int32_t target_rpm; Also u32. > u16 speed; > u8 temp; > > @@ -111,6 +125,10 @@ static int cros_ec_hwmon_read(struct device *dev, enum hwmon_sensor_types type, > ret = cros_ec_hwmon_read_fan_speed(priv->cros_ec, channel, &speed); > if (ret == 0) > *val = cros_ec_hwmon_is_error_fan(speed); > + } else if (attr == hwmon_fan_target) { > + ret = cros_ec_hwmon_read_fan_target(priv->cros_ec, channel, &target_rpm); > + if (ret == 0) > + *val = target_rpm; > } > } else if (type == hwmon_temp) { > if (attr == hwmon_temp_input) { > > -- > 2.49.0.rc0.332.g42c0ae87b1-goog >
On 3/13/25 09:24, Thomas Weißschuh wrote: > On 2025-03-13 12:47:43+0800, Sung-Chi Li wrote: >> Implement the functionality of reading the target fan RPM setting from >> ChromeOS embedded controller under framework. >> >> Signed-off-by: Sung-Chi Li <lschyi@chromium.org> >> --- >> drivers/hwmon/cros_ec_hwmon.c | 18 ++++++++++++++++++ >> 1 file changed, 18 insertions(+) >> >> diff --git a/drivers/hwmon/cros_ec_hwmon.c b/drivers/hwmon/cros_ec_hwmon.c >> index b2fec0768301f116f49c57b8dbfb042b98a573e1..73bfcbbaf9531be6b753cfef8045fd5dab5b2ab3 100644 >> --- a/drivers/hwmon/cros_ec_hwmon.c >> +++ b/drivers/hwmon/cros_ec_hwmon.c >> @@ -36,6 +36,19 @@ static int cros_ec_hwmon_read_fan_speed(struct cros_ec_device *cros_ec, u8 index >> return 0; >> } >> >> +static int cros_ec_hwmon_read_fan_target(struct cros_ec_device *cros_ec, u8 index, int32_t *speed) > > int32_t is a userspace type. In the kernel use i32, or even better u32. > Seems to be pretty widely used to complain about. $ git grep int32_t drivers/ | wc 43662 192381 3555402 Also, in comparison: $ git grep i32 drivers/ | wc 820 4009 68486 Guenter
On 2025-03-13 16:36:54-0700, Guenter Roeck wrote: > On 3/13/25 09:24, Thomas Weißschuh wrote: > > On 2025-03-13 12:47:43+0800, Sung-Chi Li wrote: > > > Implement the functionality of reading the target fan RPM setting from > > > ChromeOS embedded controller under framework. > > > > > > Signed-off-by: Sung-Chi Li <lschyi@chromium.org> > > > --- > > > drivers/hwmon/cros_ec_hwmon.c | 18 ++++++++++++++++++ > > > 1 file changed, 18 insertions(+) > > > > > > diff --git a/drivers/hwmon/cros_ec_hwmon.c b/drivers/hwmon/cros_ec_hwmon.c > > > index b2fec0768301f116f49c57b8dbfb042b98a573e1..73bfcbbaf9531be6b753cfef8045fd5dab5b2ab3 100644 > > > --- a/drivers/hwmon/cros_ec_hwmon.c > > > +++ b/drivers/hwmon/cros_ec_hwmon.c > > > @@ -36,6 +36,19 @@ static int cros_ec_hwmon_read_fan_speed(struct cros_ec_device *cros_ec, u8 index > > > return 0; > > > } > > > +static int cros_ec_hwmon_read_fan_target(struct cros_ec_device *cros_ec, u8 index, int32_t *speed) > > > > int32_t is a userspace type. In the kernel use i32, or even better u32. > > > > Seems to be pretty widely used to complain about. There is even a checkpatch.pl test for it, which should have triggered: PREFER_KERNEL_TYPES. > $ git grep int32_t drivers/ | wc > 43662 192381 3555402 33k of those are in generated amdgpu headers. This search also happens to include the more frequently used uint32_t. > Also, in comparison: > > $ git grep i32 drivers/ | wc > 820 4009 68486 The numbers for u32 look a bit different: $ git grep u32 drivers/ | wc 234768 1137059 17410570 Also this specific driver already consistently uses uNN. This does look wrong: int32_t target_rpm; u16 speed; u8 temp; Thomas
Hi Sung-Chi, kernel test robot noticed the following build warnings: [auto build test WARNING on 9fbcd7b32bf7c0a5bda0f22c25df29d00a872017] url: https://github.com/intel-lab-lkp/linux/commits/Sung-Chi-Li/hwmon-cros_ec-Add-setting-target-fan-RPM-function/20250313-125018 base: 9fbcd7b32bf7c0a5bda0f22c25df29d00a872017 patch link: https://lore.kernel.org/r/20250313-extend_ec_hwmon_fan-v1-2-5c566776f2c4%40chromium.org patch subject: [PATCH 2/3] hwmon: (cros_ec) Add reading target fan RPM function config: x86_64-randconfig-121-20250314 (https://download.01.org/0day-ci/archive/20250314/202503141908.rieksBce-lkp@intel.com/config) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250314/202503141908.rieksBce-lkp@intel.com/reproduce) 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/202503141908.rieksBce-lkp@intel.com/ sparse warnings: (new ones prefixed by >>) >> drivers/hwmon/cros_ec_hwmon.c:48:18: sparse: sparse: cast to restricted __le32 vim +48 drivers/hwmon/cros_ec_hwmon.c 38 39 static int cros_ec_hwmon_read_fan_target(struct cros_ec_device *cros_ec, u8 index, int32_t *speed) 40 { 41 int ret; 42 struct ec_response_pwm_get_fan_rpm r; 43 44 ret = cros_ec_cmd(cros_ec, 0, EC_CMD_PWM_GET_FAN_TARGET_RPM, NULL, 0, &r, sizeof(r)); 45 if (ret < 0) 46 return ret; 47 > 48 *speed = le32_to_cpu(r.rpm); 49 return 0; 50 } 51
diff --git a/drivers/hwmon/cros_ec_hwmon.c b/drivers/hwmon/cros_ec_hwmon.c index b2fec0768301f116f49c57b8dbfb042b98a573e1..73bfcbbaf9531be6b753cfef8045fd5dab5b2ab3 100644 --- a/drivers/hwmon/cros_ec_hwmon.c +++ b/drivers/hwmon/cros_ec_hwmon.c @@ -36,6 +36,19 @@ static int cros_ec_hwmon_read_fan_speed(struct cros_ec_device *cros_ec, u8 index return 0; } +static int cros_ec_hwmon_read_fan_target(struct cros_ec_device *cros_ec, u8 index, int32_t *speed) +{ + int ret; + struct ec_response_pwm_get_fan_rpm r; + + ret = cros_ec_cmd(cros_ec, 0, EC_CMD_PWM_GET_FAN_TARGET_RPM, NULL, 0, &r, sizeof(r)); + if (ret < 0) + return ret; + + *speed = le32_to_cpu(r.rpm); + return 0; +} + static int cros_ec_hwmon_read_temp(struct cros_ec_device *cros_ec, u8 index, u8 *temp) { unsigned int offset; @@ -95,6 +108,7 @@ static int cros_ec_hwmon_read(struct device *dev, enum hwmon_sensor_types type, { struct cros_ec_hwmon_priv *priv = dev_get_drvdata(dev); int ret = -EOPNOTSUPP; + int32_t target_rpm; u16 speed; u8 temp; @@ -111,6 +125,10 @@ static int cros_ec_hwmon_read(struct device *dev, enum hwmon_sensor_types type, ret = cros_ec_hwmon_read_fan_speed(priv->cros_ec, channel, &speed); if (ret == 0) *val = cros_ec_hwmon_is_error_fan(speed); + } else if (attr == hwmon_fan_target) { + ret = cros_ec_hwmon_read_fan_target(priv->cros_ec, channel, &target_rpm); + if (ret == 0) + *val = target_rpm; } } else if (type == hwmon_temp) { if (attr == hwmon_temp_input) {
Implement the functionality of reading the target fan RPM setting from ChromeOS embedded controller under framework. Signed-off-by: Sung-Chi Li <lschyi@chromium.org> --- drivers/hwmon/cros_ec_hwmon.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)