Re: [PATCH v8 3/3] HID: cp2112: Fwnode Support

[Daniel Kaehn <kaehndan@gmail.com>]

On Mon, Feb 27, 2023 at 5:07 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Mon, Feb 27, 2023 at 08:07:58AM -0600, Danny Kaehn wrote:
> > Bind I2C and GPIO interfaces to subnodes with names
> > "i2c" and "gpio" if they exist, respectively. This
> > allows the GPIO and I2C controllers to be described
> > in firmware as usual. Additionally, support configuring the
> > I2C bus speed from the clock-frequency device property.
>
> A bit shorten indentation...
>
> Nevertheless what I realized now is that this change, despite being OF
> independent by used APIs, still OF-only.

I assumed this would practically be the case -- not because of the casing
reason you gave (wasn't aware of that, thanks for the FYI), but because it
doesn't seem that there's any way to describe USB devices connected to
a USB port in ACPI, at least as far as I can tell (ACPI is still largely a black
box to me). But it seems reasonable that we should try to use the interface
in a way so that it could be described using ACPI at some point (assuming
that it isn't currently possible).

>
> Would it be possible to allow indexed access to child nodes as well, so if
> there are no names, we may still be able to use firmware nodes from the correct
> children?
>

Sure, you mean to fallback to using child nodes by index rather than by name
in the case that that device_get_named_child_node() fails?
Would we need to somehow verify that those nodes are the nodes we expect
them to be? (a.e. node 0 is actually the i2c-controller, node 1 is actually the
gpio-controller).

I don't see a reason why not, though I am curious if there is
precedence for this
strategy, a.e. in other drivers that use named child nodes. In my initial search
through the kernel, I don't think I found anything like this -- does that mean
those drivers also inherently won't work with ACPI?

The only driver I can find which uses device_get_named_child_node and has
an acpi_device_id is drivers/platform/x86/intel/chtwc_int33fe.c

> P.S. The problem with ACPI is that "name" of the child node will be in capital
> letters as it's in accordance with the specification.
>

Knowing that this is the limitation, some other potential resolutions
to potentially
consider might be:

- Uppercase the names of the child nodes for the DT binding -- it appears that
     the child node that chtwc_int33fe.c (the driver mentioned earlier) accesses
     does indeed have an upper-cased name -- though that driver doesn't have
     an of_device_id (makes sense, x86...). It seems named child nodes are
     always lowercase in DT bindings -- not sure if that's a rule, or
just how it
     currently happens to be.
- Do a case invariant compare on the names (and/or check for both lowercase
     and uppercase)
- Remove the use of child nodes, and combine the i2c and gpio nodes into the
    cp2112's fwnode. I didn't do this initially because I wanted to
avoid namespace
    collisions between GPIO hogs and i2c child devices, and thought
that logically
     made sense to keep them separate, but that was before knowing
this limitation
    of ACPI.

What are your / others' thoughts?

Thanks,

Danny Kaehn


> --
> With Best Regards,
> Andy Shevchenko
>
>