Message-ID: <20250924195358.16bbd90c@kemnade.info>
Date: Wed, 24 Sep 2025 19:53:58 +0200
From: Andreas Kemnade <andreas@...nade.info>
To: Guenter Roeck <linux@...ck-us.net>
Cc: Mark Brown <broonie@...nel.org>, 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 Wed, 24 Sep 2025 00:17:48 -0700
Guenter Roeck <linux@...ck-us.net> wrote:

> 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.
> 
I apparently did not make clear what my question refers to. It was more about separate
hwmon device + mfd vs. integrating everything into the regulator driver.

But since you brought up the topic hwmon vs. iio for temperature. I do not have
a strong opinion here as long as I can somehow live with it. Nothing I want to
fight for. One sensor I use for measuring room temperature is hwmon, another
one is iio. So it is all not that consistent.

But what is the hwmon equivalent for
devm_fwnode_iio_channel_get_by_name() + iio_read_channel_processed()?

I wonder whether I really need a thermal zone. It adds stuff not needed here,
trip points and polling. 

Documentation/hwmon/sy7636a-hwmon.rst seems to be wrong. It is not
SoC-on-die temperature, but temperature from an external NTC. And
that is typically used to tune the EPD refresh to the temperature.
I have a sy7636a connected to a broken display causing excessive current
consumption and heat generation. I looked at it with an IR camera.

Regards,
Andreas

Powered by blists - more mailing lists