On Thu, Mar 13, 2025 at 05:24:28PM +0100, 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@xxxxxxxxxxxx>
> > ---
> > 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.
>
Sorry for missing this important detail, I will not use userspace type
for following changes.
> > +{
> > + 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().
>
It did. Currently, all devices are running little endians on both AP and EC, so
I think it is ok not to explicitly call the 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.
>
Same for the kernel type changes.
> > 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
> >
