netdev - RE: [PATCH net-next 3/3] mlxsw: core: Add support for negative temperature readout

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 23 Jun 2019 16:00:38 +0000
From:   Vadim Pasternak <vadimp@...lanox.com>
To:     Andrew Lunn <andrew@...n.ch>, Ido Schimmel <idosch@...sch.org>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        Jiri Pirko <jiri@...lanox.com>, mlxsw <mlxsw@...lanox.com>,
        Ido Schimmel <idosch@...lanox.com>
Subject: RE: [PATCH net-next 3/3] mlxsw: core: Add support for negative
 temperature readout



> -----Original Message-----
> From: Andrew Lunn <andrew@...n.ch>
> Sent: Sunday, June 23, 2019 6:44 PM
> To: Ido Schimmel <idosch@...sch.org>
> Cc: netdev@...r.kernel.org; davem@...emloft.net; Jiri Pirko
> <jiri@...lanox.com>; mlxsw <mlxsw@...lanox.com>; Vadim Pasternak
> <vadimp@...lanox.com>; Ido Schimmel <idosch@...lanox.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