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;
---
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 })
}
}
}
}
}
}
---
