Message ID | 20211118105650.207638-3-hdegoede@redhat.com |
---|---|
State | New |
Headers | show |
Series | [v2,1/3] pinctrl: cherryview: Don't use pin/offset 0 to mark an interrupt line as unused | expand |
On Thu, Nov 18, 2021 at 11:56:50AM +0100, Hans de Goede wrote: > Many Cherry Trail DSDTs have an extra INT33FF device with UID 5, > the intel_pinctrl_get_soc_data() call will fail for this extra > unknown UID, leading to the following error in dmesg: > > cherryview-pinctrl: probe of INT33FF:04 failed with error -61 > > Add a check for this extra UID and return -ENODEV for it to > silence this false-positive error message. Hmm... Interesting. Why do they have it? Give me some time to check this...
On Thu, Nov 18, 2021 at 01:28:02PM +0200, Andy Shevchenko wrote: > On Thu, Nov 18, 2021 at 11:56:50AM +0100, Hans de Goede wrote: > > Many Cherry Trail DSDTs have an extra INT33FF device with UID 5, > > the intel_pinctrl_get_soc_data() call will fail for this extra > > unknown UID, leading to the following error in dmesg: > > > > cherryview-pinctrl: probe of INT33FF:04 failed with error -61 > > > > Add a check for this extra UID and return -ENODEV for it to > > silence this false-positive error message. > > Hmm... Interesting. Why do they have it? > Give me some time to check this... _DDN in ACPI describes this as Virtual GPIO. The only documentation at hand right now tells me that this is a "solution" to represent the "virtual GPIO" as fifth community (no connection to any pads, minimum configuration, etc). The goal as far as I can see is "to convert a PME event generated by PCI device to a GPIO interrupt". Seems like we better have a driver for it, but the only purpose of it is to generate interrupts based on PME. I'll try to dig more may be next week, but for now I would like to postpone the patch. Do you agree?
Hi, On 11/26/21 19:12, Andy Shevchenko wrote: > On Thu, Nov 18, 2021 at 01:28:02PM +0200, Andy Shevchenko wrote: >> On Thu, Nov 18, 2021 at 11:56:50AM +0100, Hans de Goede wrote: >>> Many Cherry Trail DSDTs have an extra INT33FF device with UID 5, >>> the intel_pinctrl_get_soc_data() call will fail for this extra >>> unknown UID, leading to the following error in dmesg: >>> >>> cherryview-pinctrl: probe of INT33FF:04 failed with error -61 >>> >>> Add a check for this extra UID and return -ENODEV for it to >>> silence this false-positive error message. >> >> Hmm... Interesting. Why do they have it? >> Give me some time to check this... > > _DDN in ACPI describes this as Virtual GPIO. The only documentation at hand > right now tells me that this is a "solution" to represent the "virtual GPIO" > as fifth community (no connection to any pads, minimum configuration, etc). > > The goal as far as I can see is "to convert a PME event generated by PCI device > to a GPIO interrupt". > > Seems like we better have a driver for it, but the only purpose of it is to > generate interrupts based on PME. > > I'll try to dig more may be next week, but for now I would like to postpone the > patch. Do you agree? Yes postponing merging this is fine. There is no hurry since this does not fix anything broken. I just wanted to get rid of the annoying log message :) Regards, Hans
On Sat, Nov 27, 2021 at 10:49:55PM +0100, Hans de Goede wrote: > On 11/26/21 19:12, Andy Shevchenko wrote: > > On Thu, Nov 18, 2021 at 01:28:02PM +0200, Andy Shevchenko wrote: > >> On Thu, Nov 18, 2021 at 11:56:50AM +0100, Hans de Goede wrote: > >>> Many Cherry Trail DSDTs have an extra INT33FF device with UID 5, > >>> the intel_pinctrl_get_soc_data() call will fail for this extra > >>> unknown UID, leading to the following error in dmesg: > >>> > >>> cherryview-pinctrl: probe of INT33FF:04 failed with error -61 > >>> > >>> Add a check for this extra UID and return -ENODEV for it to > >>> silence this false-positive error message. > >> > >> Hmm... Interesting. Why do they have it? > >> Give me some time to check this... > > > > _DDN in ACPI describes this as Virtual GPIO. The only documentation at hand > > right now tells me that this is a "solution" to represent the "virtual GPIO" > > as fifth community (no connection to any pads, minimum configuration, etc). > > > > The goal as far as I can see is "to convert a PME event generated by PCI device > > to a GPIO interrupt". > > > > Seems like we better have a driver for it, but the only purpose of it is to > > generate interrupts based on PME. > > > > I'll try to dig more may be next week, but for now I would like to postpone the > > patch. Do you agree? > > Yes postponing merging this is fine. There is no hurry since this does > not fix anything broken. I just wanted to get rid of the annoying log message :) So, documentation says the following. "Chassis GPIO does not support the notion of Virtual GPIOs. So a fifth GPIO Community was added to provide virtual GPIOs. This virtual GPIO resides inside PCU." "Table 8‑19: Virtual GPIO Assignments in CHV GPIO-V[x] Usage [7] Reserved for PMC usage [6] PME handling [5] Reserved for PMC usage [4] OTG PME [3:1] In VLV2, was used for tx_modphy_common_mode_en. In CHV, these are reserved for future use. [0] SATA PME" IOAPIC mapping: "108 GPIO_virtual" While at it, in case you want the mapping for direct IRQ: 1) GPIO North: eight interrupts used, connected to IOxAPIC IRQ51 to IRQ58. 2) GPIO Southwest: eight interrupts used, connected to IOxAPIC IRQ59 to IRQ66. 3) GPIO East: all sixteen interrupts used, connected to IOxAPIC IRQ67 to IRQ82. 4) GPIO Southeast: all sixteen interrupt used, connected to IOxAPIC IRQ92 to IRQ107.
diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c index da68f8a849ab..487e343b68e1 100644 --- a/drivers/pinctrl/intel/pinctrl-cherryview.c +++ b/drivers/pinctrl/intel/pinctrl-cherryview.c @@ -1686,6 +1686,10 @@ static int chv_pinctrl_probe(struct platform_device *pdev) acpi_status status; int i, ret, irq; + /* Cherry Trail DSDTs have an extra INT33FF device with UID 5, ignore */ + if (!strcmp(adev->pnp.unique_id, "5")) + return -ENODEV; + soc_data = intel_pinctrl_get_soc_data(pdev); if (IS_ERR(soc_data)) return PTR_ERR(soc_data);