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
| ||
|
Message-ID: <d6e093d7-0ff3-4545-9ff8-1c342879fe40@roeck-us.net> Date: Wed, 16 Aug 2023 12:25:42 -0700 From: Guenter Roeck <linux@...ck-us.net> To: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@...adcom.com> Cc: Michael Chan <michael.chan@...adcom.com>, davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com, gospo@...adcom.com, Jean Delvare <jdelvare@...e.com>, linux-hwmon@...r.kernel.org Subject: Re: [PATCH net-next 11/12] bnxt_en: Expose threshold temperatures through hwmon On Wed, Aug 16, 2023 at 09:42:17PM +0530, Kalesh Anakkur Purayil wrote: > On Wed, Aug 16, 2023 at 5:43 PM Guenter Roeck <linux@...ck-us.net> wrote: > > > On Wed, Aug 16, 2023 at 03:58:34PM +0530, Kalesh Anakkur Purayil wrote: > > > Thank you Guenter for the review and the suggestions. > > > > > > Please see my response inline. > > > > > > On Tue, Aug 15, 2023 at 8:35 PM Guenter Roeck <linux@...ck-us.net> > > wrote: > > > > > [ ... ] > > > > > > > > > > Hmm, that isn't really the purpose of alarm attributes. The expectation > > > > would be that the chip sets alarm flags and the driver reports it. > > > > I guess there is some value in having it, so I won't object. > > > > > > > > Anyway, the ordering is wrong. max_alarm should be the lowest > > > > alarm level, followed by crit and emergency. So > > > > max_alarm -> temp >= bp->warn_thresh_temp > > > > crit_alarm -> temp >= bp->crit_thresh_temp > > > > emergency_alarm -> temp >= bp->fatal_thresh_temp > > > > or temp >= bp->shutdown_thresh_temp > > > > > > > > There are only three levels of upper temperature alarms. > > > > Abusing lcrit as 4th upper alarm is most definitely wrong. > > > > > > > [Kalesh]: Thank you for the clarification. > > > bnxt_en driver wants to expose 4 threshold temperatures to the user > > through > > > hwmon sysfs. > > > 1. warning threshold temperature > > > 2. critical threshold temperature > > > 3. fatal threshold temperature > > > 4. shutdown threshold temperature > > > > > > I will use the following mapping: > > > > > > hwmon_temp_max : warning threshold temperature > > > hwmon_temp_crit : critical threshold temperature > > > hwmon_temp_emergency : fatal threshold temperature > > > > > > hwmon_temp_max_alarm : temp >= bp->warn_thresh_temp > > > hwmon_temp_crit_alarm : temp >= bp->crit_thresh_temp > > > hwmon_temp_emergency_alarm : temp >= bp->fatal_thresh_temp > > > > > > Is it OK to map the shutdown threshold temperature to "hwmon_temp_fault"? > > > > That is a flag, not a temperature, and it is intended to signal > > a problem ith the sensor. > > > > > If not, can you please suggest an alternative? > > > > > > > The only one I can think of is to add non-standard attributes > > such as temp1_shutdown and temp1_shutdown_alarm. > > > [Kalesh]: Sorry, I don't quite get this part. I was looking at the kernel > hwmon code, but could not find any reference. > It would be non-standard attributes, so, correct, there is no reference. > Can we add new attributes "shutdown" and "shutdown_alarm" for tempX? For > example: > > #define HWMON_T_SHUTDOWN BIT(hwmon_temp_shutdown) > Not for a single driver. You can implement the sysfs attributes directly in the driver and pass an extra attribute group to the hwmon core when registering the hwmon device. Guenter
Powered by blists - more mailing lists