On Wed, Mar 08, 2023 at 04:55:27PM +0100, Benjamin Tissoires wrote:
> On Mar 08 2023, Daniel Kaehn wrote:
> > On Wed, Mar 8, 2023 at 9:26 AM Benjamin Tissoires
> > <benjamin.tissoires@xxxxxxxxxx> wrote:
> > > But if I refer "\\_SB_.PCI0.USB0.RHUB.CP21.GPIO", the IRQ is never assigned.
> > > With the parent (CP21), it works.
> > >
> > > So I wonder if the cp2112 driver is correctly assigning the gc->parent
> > > field.
> > Did you make a change to the CP2112 driver patch to look for uppercase
> > "I2C" and "GPIO"?
>
> yes, sorry I should have mentioned it. This is the only modification I
> have compared to the upstream kernel plus your patch series.
>
> > Otherwise, it won't assign those child nodes appropriately, and the
> > gpiochip code will use
> > the parent node by default if the gpiochip's fwnode isn't assigned (I believe).
>
> I don't think it's a fwnode issue, but a problem with the assignment of
> the parent of the gc:
> ---
> dev->gc.parent = &hdev->dev;
> ---
I don't think so. The parent should point to the _physical_ device, which is
CP2112, which is correct in my opinion.
> Because the function acpi_gpiochip_find() in drivers/gpio/gpiolib-acpi.c
> compares the acpi handle returned by fetching the ACPI path
> ("\\_SB_.PCI0.USB0.RHUB.CP21.GPIO") and the one of gc->parent, which in
> the hid-cp2112 case is the HID device itself.
We have specifically gc->fwnode for cases like this.
...
> Device (CP21) // the USB-hid & CP2112 shared node
> {
> Name (_ADR, One)
> Name (_DSD, Package ()
> {
> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> Package () {
> Package () { "cell-names", Package () { "i2c", "gpio" }
> }
Yeah, looking at this, I think it still fragile. First of all, either this is
missing, or simply wrong. We would need to access indices. ACPI _ADR is in the
specification. As much as with PCI it may be considered reliable.
So, that said, forget about it, and simply use _ADR as indicator of the node.
See how MFD (in the Linux kernel) cares about this. Ex. Diolan DLN-2 driver.
> })
>
> Device (I2C)
> {
> Name (_ADR, Zero)
> Name (_STA, 0x0F)
> }
>
> Device (GPIO)
> {
> Name (_ADR, One)
> Name (_STA, 0x0F)
>
> Name (_DSD, Package () {
> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> Package () {
> Package () { "gpio-hog", 1 },
> Package () { "gpios", Package () { 4, 0 } },
> Package () { "output-high", 1 },
> Package () { "line-name", "gpio4-pullup" },
> },
> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> Package () {
> Package () { "gpio-line-names", Package () {
> "",
> "",
> "irq-rmi4",
> "",
> "power", // set to 1 with gpio-hog above
> "",
> "",
> "",
> ""}},
> }
> })
> }
> }
--
With Best Regards,
Andy Shevchenko
