Re: [PATCH v2 02/10] hwmon: Add Qualcomm PMIC BCL driver

[Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>]

Hi Guenter,

On 7/23/2026 12:29 AM, Guenter Roeck wrote:
> On 7/22/26 11:19, Manaf Meethalavalappu Pallikunhi wrote:
>> Hi Guenter,
>>
>> On 7/22/2026 11:16 PM, Guenter Roeck wrote:
>>> On 7/22/26 10:38, Manaf Meethalavalappu Pallikunhi wrote:
>>> ...
>>>>>
>>>>> I am curious: Why not use static initialization and use the is_visible
>>>>> function to determine if an attribute is visible or not ?
>>>>
>>>> ACK. There is already a comment in v1 suggesting the use of dynamic 
>>>> allocation based on the available attributes rather than static 
>>>> initialization. The intent is to avoid having to perform enable flag 
>>>> checks in multiple places.
>>>> . >
>>>
>>> You lost me, sorry. There is only a single is_visible function,
>>> and its intent is exactly to handle situations where some sensors
>>> are not always available.
>>>
>>> What would be those "multiple places" ?
>>
>> Understood, thanks for the clarification. I'll revert to static 
>> attribute initialization in the next revision and use is_visible() to 
>> control attribute visibility where needed.
>>
>>>
>>>>>  From the earlier exchange I had the impression that there is a large
>>>>> number of current and voltage channels, but it looks like there is 
>>>>> only
>>>>> one each. That doesn't really warrant or need all this dynamic code
>>>>
>>>> Yes, this hardware only supports one or two channels (voltage and 
>>>> current). What we discussed earlier was that each channel can have 
>>>> up to three threshold alarms (warning, critical, and emergency). 
>>>> However, the hwmon framework currently supports only two limit 
>>>> alarms for these sensor types. I have not added support for the 
>>>> third alarm threshold in this series to keep the initial driver 
>>>> support aligned with the existing hwmon capabilities. Once the basic 
>>>> driver support is accepted, I can queue a follow-up series to add 
>>>> support for the third limit alarm.
>>>>
>>>
>>> What does that have to do with attribute visibility ?
>>
>> I was only clarifying that the channel count has always been small 
>> (maximum of two channels) and has not changed since v1. The earlier 
>> discussion was primarily around the limit alarm attributes. That said, 
>> you're right that this is unrelated to attribute visibility.
>>
> 
> Feel free to use (and test)
> https://lore.kernel.org/linux-hwmon/20260722185749.2313572-1- 
> linux@roeck-us.net/T/#u

Thanks for working on this. I'll use that change as the base, rebase my 
series on top of it, and add support for the third alarm level.
By the way, for the voltage channel, the hardware monitors undervoltage 
conditions and provides three threshold levels. At the moment, I am mapping:

LVL0 → min
LVL1 → lcrit

For LVL2, would it be possible to introduce a corresponding low-voltage 
emergency threshold, such as lemergency (or another more appropriate 
name), to represent the third alarm level for voltage sensors ?

Thanks,
Manaf

> 
> Guenter
>