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 linux-cve-announce 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]

Message-ID: <AM6PR05MB5224331ECB7E643BA18C10BDA2E10@AM6PR05MB5224.eurprd05.prod.outlook.com>
Date:   Sun, 23 Jun 2019 16:11:40 +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: Vadim Pasternak
> Sent: Sunday, June 23, 2019 7:01 PM
> To: Andrew Lunn <andrew@...n.ch>; Ido Schimmel <idosch@...sch.org>
> Cc: netdev@...r.kernel.org; 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.
> 
 O, I see what you mentioned.
This is mlxsw_hwmon_module_temp_show().
Yes, right it should be %d.
Thank you.

> >
> > > @@ -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