linux-kernel - Re: [PATCH RFC 1/2] hwmon: (sy7636a) fix races during probe of mfd subdevices

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: <b68d40d9-6ea9-49a9-8a2e-8b899f33340d@roeck-us.net>
Date: Wed, 24 Sep 2025 00:17:48 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Andreas Kemnade <andreas@...nade.info>, Mark Brown <broonie@...nel.org>
Cc: jdelvare@...e.com, lgirdwood@...il.com, linux-hwmon@...r.kernel.org,
 linux-kernel@...r.kernel.org, Alistair Francis <alistair@...stair23.me>
Subject: Re: [PATCH RFC 1/2] hwmon: (sy7636a) fix races during probe of mfd
 subdevices

On 9/24/25 00:00, Andreas Kemnade wrote:
> On Sat, 20 Sep 2025 23:18:59 +0100
> Mark Brown <broonie@...nel.org> wrote:
> 
>> On Sat, Sep 20, 2025 at 11:33:07PM +0200, Andreas Kemnade wrote:
>>
>>> Just for learning, yes, it is an abuse of the _optional for non-optional
>>> things, so a dirty hack which should not go in, therefore RFC. But what
>>> happens more than having the hwmon device endlessly deferred at worst?
>>
>> There's also the fact that this API is so frequently abused for bad and
>> broken reasons that I regularly audit users and try to fix them, I'd
>> rather not see any new users that don't have a really strong reason to
>> use it.
>>
>>> The wanted regulator is the one defined in sy7636a-regulator.c. So it
>>> is all an issue internal to the sy7636a.
>>
>>> Both subdevices are instantiated via drivers/simple-mfd-i2c.c.
>>> I see several other solutions:
>>> a) call device_is_bound() on every other children of dev->parent, if not
>>> bound defer.
>>> b) do not care about the regulator api at all, just check whether
>>>     the corresponding bit is set before reading temperature, return
>>>     -ENODATA if not, some mutex is probably needed.
>>> c) do not care about the regulator api at all, just set the
>>>     corresponding bit (together with some mutex locking and counting).
>>
>> I assume this is using the regulator API because someone might use an
>> external regulator in a system design for some reason (better quality,
>> power efficiency or a shared reference between multiple devices I
>> guess?), or because the supply might also be used by external devices?
>>
>>> d) copy the of_node pointer from the parent, add a regulator phandle property
>>>     to the node pointing to the regulator in the node itself.
>>>     That sounds like your idea but is against the current dt binding for
>>>     this device and afaik it is uncommon to have mfd-internal things wired
>>>     up this way
>>>
>>> e) something clean, simple I miss
>>
>> The idea is that the relationship between the devices should be
>> registered before the devices, that's how the regulator knows to defer.
>> We used to have an API for doing this for board files which might fit
>> here, but it got removed since nobody wants board files any more.  If
>> you're allocating the devices dynamically that's annoying to implement
>> though...
> 
> looking a bit around:
> max5970-regulator.c has hwmon integrated and no extra device. That would
> simplify things. Although it does not report temperature. Some
> touchscreens have temperature via hwmon, some others have temperature
> via iio, directly in one device without mfd. Maybe that is also
> the better way here?
> 

Touchscreens reporting temperature via iio is in general the wrong thing to do.
Touchscreens report the temperature for monitoring reasons, after all.
But then, sure, if you insist. I am getting tired of arguing.

FWIW, the proper implementation would probably have been to implement
the hwmon device as auxiliary device.

Guenter