Re: [PATCH 2/4] hwmon: Add Qualcomm PMIC BCL hardware monitor driver

[Daniel Lezcano]

On 3/20/26 17:59, Guenter Roeck wrote:
> On 3/20/26 09:08, Daniel Lezcano wrote:
> > Hi Guenter,
> >
> > On 3/20/26 16:22, Guenter Roeck wrote:
> > > On 3/20/26 07:52, Daniel Lezcano wrote:
> >
> > [ ... ]
> >
> > > > > +
> > > > > +ADD_BCL_HWMON_ALARM_MAPS(in, min, lcrit);
> > > > > +ADD_BCL_HWMON_ALARM_MAPS(curr, max, crit);
> > > > > +
> > > > > +/* Interrupt names for each alarm level */
> > > > > +static const char * const bcl_int_names[ALARM_MAX] = {
> > > > > +    [LVL0] = "bcl-max-min",
> > > > > +    [LVL1] = "bcl-critical",
> > > > > +};
> > > >
> > > > IIUC there are three levels of alarms but the hwmon only has max/min
> > > > and critical. Would it make sense to do adaptative min / max ? So when
> > >
> > > hwmon has lcrit, min, max, and crit alarms for all sensor types, plus
> > > an additional _cap_alarm for power attributes and _emergency_alarm
> > > for temperature attributes. There is also a generic _alarm attribute
> > > for each sensor, which is supposed to be used if the specific alarm
> > > type is not known.
> > >
> > > What exactly are the three levels of alarms ?
> >
> > Manaf can give more clarifications, but it is like we have yellow, 
> > orange and red alarms. So there is an additional alarm comparing to 
> > what is available in hwmon. The proposed driver maps orange and red 
> > alarms, respectively to bcl-max and bcl-critical.
> >
> > I'm just asking if it is important to have this 'yellow' alarm ? And 
> > as there is a missing alarm to describe it in hwmon, how can we use it ?
>
> Is this for high alarms only or also for low alarms ?

It is high for over current and low for under voltage.

> I would not mind adding _emergency attributes for non-temperature
> alarms if needed. We could also add another low alarm if needed,
> though I don't have a good idea for a good name for that.

May be "warning" ?

Alternatively, could it be interesting to have an alarm with a value 
passed in the event which represent the value in the unit of the 
monitored attribute ? So there is no need to add more alarm files