linux-kernel - Re: [PATCH v5 2/2] drivers: hwmon: sophgo: Add SG2042 external hardware monitor support

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] [day] [month] [year] [list]

Date: Sat, 22 Jun 2024 08:05:17 +0800
From: Inochi Amaoto <inochiama@...look.com>
To: Guenter Roeck <linux@...ck-us.net>, 
	Inochi Amaoto <inochiama@...look.com>
Cc: Jean Delvare <jdelvare@...e.com>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Chen Wang <unicorn_wang@...look.com>, Paul Walmsley <paul.walmsley@...ive.com>, 
	Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>, linux-hwmon@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org
Subject: Re: [PATCH v5 2/2] drivers: hwmon: sophgo: Add SG2042 external
 hardware monitor support

On Fri, Jun 21, 2024 at 04:05:46PM GMT, Guenter Roeck wrote:
> On 6/21/24 15:45, Inochi Amaoto wrote:
> > On Fri, Jun 21, 2024 at 08:19:10AM GMT, Guenter Roeck wrote:
> > > On Sun, May 05, 2024 at 09:18:53AM +0800, Inochi Amaoto wrote:
> > > > SG2042 use an external MCU to provide basic hardware information
> > > > and thermal sensors.
> > > > 
> > > > Add driver support for the onboard MCU of SG2042.
> > > > 
> > > > Signed-off-by: Inochi Amaoto <inochiama@...look.com>
> > > > ---
> > > >   drivers/hwmon/Kconfig            |  11 +
> > > >   drivers/hwmon/Makefile           |   1 +
> > > >   drivers/hwmon/sg2042-hwmon-mcu.c | 531 +++++++++++++++++++++++++++++++
> > > 
> > > Documentation missing.
> > > 
> > > > +config SENSORS_SG2042_HWMON_MCU
> > > 
> > > "SENSORS" and "HWMON" is redundant. Yes, I know, others do it
> > > as well, but that doesn't make it less redundant.
> > > 
> > 
> > OK, I will short it.
> > 
> > > > +static DEVICE_ATTR_RO(board_type);
> > > > +static DEVICE_ATTR_RO(firmware_version);
> > > > +static DEVICE_ATTR_RO(pcb_version);
> > > > +static DEVICE_ATTR_RO(reset_count);
> > > > +static DEVICE_ATTR_RO(uptime);
> > > > +static DEVICE_ATTR_RO(reset_reason);
> > > > +static DEVICE_ATTR_RO(mcu_type);
> > > > +static DEVICE_ATTR_RW(critical_action);
> > > 
> > > None of the above are hardware monitoring attributes. They are not
> > > acceptable as sysfs attributes. Some of them might be acceptable
> > > as debugfs attributes.
> > > 
> > 
> > It are reasonable, I will move the hardware info to debugfs.
> > 
> > > > +static DEVICE_ATTR_RW(repower_temp);
> > > > +
> > > 
> > > For this one, I don't know what it is. If it is a temperature
> > > limit, it should be implemented as one (for example as _emergency
> > > limit).
> > > 
> > > Guenter
> > 
> > "repower_temp" is the highest temperature for SG2042 to perform
> > repower operation. It is not the emergency limit, but a recovery
> > limit when the emergency limit is reached.
> > 
> > For example, when emergency temperature is set to 95 and repower
> > temperature is set to 80. The board is off-power when it reachs
> > 95 degree, and it will power on when the temperature is lower
> > than 80 degree.
> > 
> 
> That would then be the matching _hyst temperature. There is no need
> to invent a non-standard attribute for it.
> 
> Guenter
> 

Thanks for the reminder. I will move to the standard attribute.

Regards,
Inochi