> -----Original Message-----
> From: Andrew Lunn <and...@lunn.ch>
> Sent: Sunday, June 23, 2019 6:44 PM
> To: Ido Schimmel <ido...@idosch.org>
> Cc: netdev@vger.kernel.org; da...@davemloft.net; Jiri Pirko
> <j...@mellanox.com>; mlxsw <ml...@mellanox.com>; Vadim Pasternak
> <vad...@mellanox.com>; Ido Schimmel <ido...@mellanox.com>
> Subject: Re: [PATCH net-next 3/3] mlxsw: core: Add support for negative
> temperature readout
>
> > --- a/drivers/net/ethernet/mellanox/mlxsw/core_hwmon.c
> > +++ b/drivers/net/ethernet/mellanox/mlxsw/core_hwmon.c
> > @@ -52,8 +52,7 @@ static ssize_t mlxsw_hwmon_temp_show(struct device
> *dev,
> > container_of(attr, struct mlxsw_hwmon_attr,
> dev_attr);
> > struct mlxsw_hwmon *mlxsw_hwmon = mlwsw_hwmon_attr->hwmon;
> > char mtmp_pl[MLXSW_REG_MTMP_LEN];
> > - unsigned int temp;
> > - int index;
> > + int temp, index;
> > int err;
> >
> > index = mlxsw_hwmon_get_attr_index(mlwsw_hwmon_attr-
> >type_index,
> > @@ -65,7 +64,7 @@ static ssize_t mlxsw_hwmon_temp_show(struct device
> *dev,
> > return err;
> > }
> > mlxsw_reg_mtmp_unpack(mtmp_pl, &temp, NULL, NULL);
> > - return sprintf(buf, "%u\n", temp);
> > + return sprintf(buf, "%d\n", temp);
> > }
>
> If you had used the hwmon core, rather than implementing it yourself, you
> could
> of avoided this part of the bug.
>
Hi Andrew.
Yes.
But before we handle only positive temperature.
And currently support for the negative readouts has been added.
> > static ssize_t mlxsw_hwmon_temp_rst_store(struct device *dev, @@
> > -215,8 +213,8 @@ static ssize_t mlxsw_hwmon_module_temp_show(struct
> device *dev,
> > container_of(attr, struct mlxsw_hwmon_attr,
> dev_attr);
> > struct mlxsw_hwmon *mlxsw_hwmon = mlwsw_hwmon_attr->hwmon;
> > char mtmp_pl[MLXSW_REG_MTMP_LEN];
> > - unsigned int temp;
> > u8 module;
> > + int temp;
> > int err;
> >
> > module = mlwsw_hwmon_attr->type_index - mlxsw_hwmon-
> >sensor_count;
>
> I think you missed changing the %u to %d in this function.
If I am not wrong, I think you refer to mlxsw_hwmon_fan_rpm_show(),
where it should be %u.
>
> > @@ -519,14 +519,14 @@ static int mlxsw_thermal_module_temp_get(struct
> thermal_zone_device *tzdev,
> > return 0;
> > }
> > mlxsw_reg_mtmp_unpack(mtmp_pl, &temp, NULL, NULL);
> > - *p_temp = (int) temp;
> > + *p_temp = temp;
> >
> > if (!temp)
> > return 0;
> >
> > /* Update trip points. */
> > err = mlxsw_thermal_module_trips_update(dev, thermal->core, tz);
> > - if (!err)
> > + if (!err && temp > 0)
> > mlxsw_thermal_tz_score_update(thermal, tzdev, tz->trips,
> temp);
>
> Why the > 0?
We don't consider negative temperature for thermal control.
>
> Andrew
