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

[Benjamin Tissoires <benjamin.tissoires@redhat.com>]

On Mar 08 2023, Daniel Kaehn wrote:
> On Wed, Mar 8, 2023 at 9:26 AM Benjamin Tissoires
> <benjamin.tissoires@redhat.com> 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;
---

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.

> 
> If that was indeed your problem all along (and I'm not missing
> something else), sorry about that --
> I made a comment above, but didn't add much spacing around it to make
> it stand out (since I noticed you didn't reply to that part in your response)

Yeah, sorry I should have been explicit about this.

For reference, I am appending the full SSDT override which works.

Even if you don't have an i2c-hid device connected, this should at
least call the probe function in i2c-hid-core.c, which is a proof that
the ACPI binding is properly done (the first SMBus read will fail with a
timeout)

Also, I played around with the _DSD that Andy was mentioning (and some
others), hopefully this will help you getting the mapping from the
"cell-names" to the fwnode child index faster.

Cheers,
Benjamin

---
DefinitionBlock ("cp2112.aml", "SSDT", 5, "", "CP2112", 1)
{
  External (_SB_.PCI0, DeviceObj)

  Scope (\_SB_.PCI0)
  {
    Device (USB0)
    {
      Name (_ADR, 0x00010002) // _ADR: Address
      Device (RHUB)
      {
        Name (_ADR, Zero)
        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",
								}
							}
						}
					})

          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
                            "",
                            "",
                            "",
                            ""}},
              }
            })
          }
        }
      }
    }
  }

  Scope (\_SB_.PCI0.USB0.RHUB.CP21.I2C)
  {
    Device (TPD0)
    {
      Name (_HID, "RMI40001")
      Name (_CID, "PNP0C50")
      Name (_STA, 0x0F)

      Name (SBFB, ResourceTemplate ()
      {
          I2cSerialBusV2 (0x2c, ControllerInitiated, 100000,
              AddressingMode7Bit, "\\_SB_.PCI0.USB0.RHUB.CP21.I2C",
              0x00, ResourceConsumer,, Exclusive,
              )
      })
      Name (SBFG, ResourceTemplate ()
      {
          GpioInt (Level, ActiveLow, Exclusive, PullDefault, 0x0000,
              "\\_SB_.PCI0.USB0.RHUB.CP21", 0x00, ResourceConsumer, ,
              )
              {   // Pin list
                  0x0002
              }
      })
      Name (_DSD, Package ()
      {
          ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
          Package ()
          {
              Package () { "irq-gpios", Package () { ^TPD0, 1, 0, 0 } },
          }
      })
      Method(_CRS, 0x0, NotSerialized)
      {
        Return (ConcatenateResTemplate (SBFG, SBFB))
      }

      Method(_DSM, 0x4, Serialized)
      {
        // DSM UUID
        switch (ToBuffer (Arg0))
        {
          // ACPI DSM UUID for HIDI2C
          case (ToUUID ("3CDFF6F7-4267-4555-AD05-B30A3D8938DE"))
          {
              // DSM Function
              switch (ToInteger (Arg2))
              {
                  // Function 0: Query function, return based on revision
                  case(0)
                  {
                      // DSM Revision
                      switch (ToInteger (Arg1))
                      {
                          // Revision 1: Function 1 supported
                          case (1)
                          {
                              Return (Buffer (One) { 0x03 })
                          }

                          default
                          {
                              // Revision 2+: no functions supported
                              Return (Buffer (One) { 0x00 })
                          }
                      }
                  }

                  // Function 1 : HID Function
                  case(1)
                  {
                      // HID Descriptor Address
                      Return (0x0020)
                  }

                  default
                  {
                      // Functions 2+: not supported
                      Return (Buffer (One) { 0x00 })
                  }
              }
          }

          default
          {
              // No other GUIDs supported
              Return (Buffer (One) { 0x00 })
          }
        }
      }
    }
  }
}
---