Hi Dmitry, sorry for late reply!
On Sat, Jul 24, 2021 at 3:13 AM Dmitry Torokhov
<dmitry.torokhov@xxxxxxxxx> wrote:
> On Fri, Jun 25, 2021 at 01:34:35PM +0200, Linus Walleij wrote:
> > + /*
> > + * Some older device trees have erroneous names for the regulators,
> > + * so check if "vddo" is present and in that case use these names
> > + * and warn. Else use the proper supply names on the component.
> > + */
> > + if (IS_ENABLED(CONFIG_OF) &&
>
> Why is this check needed? The of_property_*() are stubbed out properly I
> believe. We might need to check that dev->of_node is not NULL, although
> I think of_* API handles this properly.
(...)
> > + of_property_read_bool(dev->of_node, "vddo-supply")) {
>
> If we go with this I do not like using of_property_read_bool() as this
> is not a boolean property, but rather of_find_property().
These comments are fixed up in Nikita's respin of the series:
https://lore.kernel.org/linux-input/20211027181350.91630-4-nikita@xxxxxxx/
> However maybe we should use regulator_get_optional() which will not give
> a dummy regulator? Still quite awkward, a dedicated API to see if a
> regulator is defined would be nice.
I guess the option would be to get all four regulators by name and
optional, but then we don't detect if more than 2 out of 4 are missing.
Not sure, it feels like we have less control over the supplies then.
I guess it sadly gets ugly because making mistakes in bindings is ugly
in the first place.
Yours,
Linus Walleij
