* [PATCH] pinctrl: icelake: Fix the resource number for community-4/5 @ 2018-09-13 22:31 Rajat Jain 2018-09-14 7:33 ` Andy Shevchenko 0 siblings, 1 reply; 12+ messages in thread From: Rajat Jain @ 2018-09-13 22:31 UTC (permalink / raw) To: Mika Westerberg, Andy Shevchenko, Linus Walleij, linux-gpio, linux-kernel Cc: rajatxjain, subrata.banik, aamir.bohra, Rajat Jain The Icelake does not have a community-3, and the memory resources are laid out in the following order in the ACPI: resource-0: community-0 registers resource-1: community-1 registers resource-2: community-2 registers resource-3: community-4 registers resource-4: community-5 registers (EDS also describes the communities in the above order). Since the pinctrl driver exposes communities 0, 1, 4, 5, it needs to get the corresponding community registers by getting the resourse number right. Currently the resourse number is not correct for community 4 and 5, thus fix that. Signed-off-by: Rajat Jain <rajatja@google.com> --- drivers/pinctrl/intel/pinctrl-icelake.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/pinctrl/intel/pinctrl-icelake.c b/drivers/pinctrl/intel/pinctrl-icelake.c index 630b966ce081..5b4eaf7c90df 100644 --- a/drivers/pinctrl/intel/pinctrl-icelake.c +++ b/drivers/pinctrl/intel/pinctrl-icelake.c @@ -331,8 +331,8 @@ static const struct intel_padgroup icllp_community5_gpps[] = { static const struct intel_community icllp_communities[] = { ICL_COMMUNITY(0, 0, 58, icllp_community0_gpps), ICL_COMMUNITY(1, 59, 152, icllp_community1_gpps), - ICL_COMMUNITY(2, 153, 215, icllp_community4_gpps), - ICL_COMMUNITY(3, 216, 240, icllp_community5_gpps), + ICL_COMMUNITY(3, 153, 215, icllp_community4_gpps), + ICL_COMMUNITY(4, 216, 240, icllp_community5_gpps), }; static const unsigned int icllp_spi0_pins[] = { 22, 23, 24, 25, 26 }; -- 2.19.0.397.gdd90340f6a-goog ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] pinctrl: icelake: Fix the resource number for community-4/5 2018-09-13 22:31 [PATCH] pinctrl: icelake: Fix the resource number for community-4/5 Rajat Jain @ 2018-09-14 7:33 ` Andy Shevchenko 2018-09-14 21:06 ` Rajat Jain 0 siblings, 1 reply; 12+ messages in thread From: Andy Shevchenko @ 2018-09-14 7:33 UTC (permalink / raw) To: Rajat Jain Cc: Mika Westerberg, Andy Shevchenko, Linus Walleij, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List, rajatxjain, subrata.banik, aamir.bohra On Fri, Sep 14, 2018 at 1:54 AM Rajat Jain <rajatja@google.com> wrote: > > The Icelake does not have a community-3, and the memory resources are > laid out in the following order in the ACPI: > > resource-0: community-0 registers > resource-1: community-1 registers > resource-2: community-2 registers > resource-3: community-4 registers > resource-4: community-5 registers > > (EDS also describes the communities in the above order). > > Since the pinctrl driver exposes communities 0, 1, 4, 5, it needs to get > the corresponding community registers by getting the resourse number right. > Currently the resourse number is not correct for community 4 and 5, thus > fix that. > Can you share link to the ACPI dump of the tables? (you may get one by running `acpidump -o tables.dat`) > Signed-off-by: Rajat Jain <rajatja@google.com> > --- > drivers/pinctrl/intel/pinctrl-icelake.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/pinctrl/intel/pinctrl-icelake.c b/drivers/pinctrl/intel/pinctrl-icelake.c > index 630b966ce081..5b4eaf7c90df 100644 > --- a/drivers/pinctrl/intel/pinctrl-icelake.c > +++ b/drivers/pinctrl/intel/pinctrl-icelake.c > @@ -331,8 +331,8 @@ static const struct intel_padgroup icllp_community5_gpps[] = { > static const struct intel_community icllp_communities[] = { > ICL_COMMUNITY(0, 0, 58, icllp_community0_gpps), > ICL_COMMUNITY(1, 59, 152, icllp_community1_gpps), > - ICL_COMMUNITY(2, 153, 215, icllp_community4_gpps), > - ICL_COMMUNITY(3, 216, 240, icllp_community5_gpps), > + ICL_COMMUNITY(3, 153, 215, icllp_community4_gpps), > + ICL_COMMUNITY(4, 216, 240, icllp_community5_gpps), > }; > > static const unsigned int icllp_spi0_pins[] = { 22, 23, 24, 25, 26 }; > -- > 2.19.0.397.gdd90340f6a-goog > -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] pinctrl: icelake: Fix the resource number for community-4/5 2018-09-14 7:33 ` Andy Shevchenko @ 2018-09-14 21:06 ` Rajat Jain 2018-09-14 21:38 ` Rajat Jain 0 siblings, 1 reply; 12+ messages in thread From: Rajat Jain @ 2018-09-14 21:06 UTC (permalink / raw) To: Andy Shevchenko Cc: Mika Westerberg, Andy Shevchenko, linus.walleij, linux-gpio, Linux Kernel Mailing List, Rajat Jain, Banik, Subrata, aamir.bohra On Fri, Sep 14, 2018 at 12:41 AM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Fri, Sep 14, 2018 at 1:54 AM Rajat Jain <rajatja@google.com> wrote: > > > > The Icelake does not have a community-3, and the memory resources are > > laid out in the following order in the ACPI: > > > > resource-0: community-0 registers > > resource-1: community-1 registers > > resource-2: community-2 registers > > resource-3: community-4 registers > > resource-4: community-5 registers > > > > (EDS also describes the communities in the above order). > > > > Since the pinctrl driver exposes communities 0, 1, 4, 5, it needs to get > > the corresponding community registers by getting the resourse number right. > > Currently the resourse number is not correct for community 4 and 5, thus > > fix that. > > > > Can you share link to the ACPI dump of the tables? (you may get one by > running `acpidump -o tables.dat`) I don't have that command on my system, but I took /sys/firmware/acpi/tables/DSDT from the system and disassembled it using Intel disassembler (iasl -d) and here is the relevant portion that describes the GPIO controller. The port IDs for communities can be seen in the below output (i.e. the PCRB()): Device (GPIO) { Name (_HID, "INT3455") // _HID: Hardware ID Name (_UID, Zero) // _UID: Unique ID Name (_DDN, "GPIO Controller") // _DDN: DOS Device Name Name (RBUF, ResourceTemplate () { Memory32Fixed (ReadWrite, 0x00000000, // Address Base 0x00000000, // Address Length _Y06) Memory32Fixed (ReadWrite, 0x00000000, // Address Base 0x00000000, // Address Length _Y07) Memory32Fixed (ReadWrite, 0x00000000, // Address Base 0x00000000, // Address Length _Y08) Memory32Fixed (ReadWrite, 0x00000000, // Address Base 0x00000000, // Address Length _Y09) Memory32Fixed (ReadWrite, 0x00000000, // Address Base 0x00000000, // Address Length _Y0A) Interrupt (ResourceConsumer, Level, ActiveLow, Shared, ,, ) { 0x0000000E, } }) Method (_CRS, 0, NotSerialized) // _CRS: Current Resource Settings { CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y06._BAS, BAS0) // _BAS: Base Address CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y06._LEN, LEN0) // _LEN: Length BAS0 = PCRB (0x6E) LEN0 = 0x00010000 CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y07._BAS, BAS1) // _BAS: Base Address CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y07._LEN, LEN1) // _LEN: Length BAS1 = PCRB (0x6D) LEN1 = 0x00010000 CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y08._BAS, BAS2) // _BAS: Base Address CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y08._LEN, LEN2) // _LEN: Length BAS2 = PCRB (0x6C) LEN2 = 0x00010000 CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y09._BAS, BAS4) // _BAS: Base Address CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y09._LEN, LEN4) // _LEN: Length BAS4 = PCRB (0x6A) LEN4 = 0x00010000 CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y0A._BAS, BAS5) // _BAS: Base Address CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y0A._LEN, LEN5) // _LEN: Length BAS5 = PCRB (0x69) LEN5 = 0x00010000 Return (RBUF) /* \_SB_.PCI0.GPIO.RBUF */ } Method (_STA, 0, NotSerialized) // _STA: Status { Return (0x0F) } } Please let me know if this helps, or if you need more info. Thanks & Best Regards, Rajat > > > Signed-off-by: Rajat Jain <rajatja@google.com> > > --- > > drivers/pinctrl/intel/pinctrl-icelake.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/pinctrl/intel/pinctrl-icelake.c b/drivers/pinctrl/intel/pinctrl-icelake.c > > index 630b966ce081..5b4eaf7c90df 100644 > > --- a/drivers/pinctrl/intel/pinctrl-icelake.c > > +++ b/drivers/pinctrl/intel/pinctrl-icelake.c > > @@ -331,8 +331,8 @@ static const struct intel_padgroup icllp_community5_gpps[] = { > > static const struct intel_community icllp_communities[] = { > > ICL_COMMUNITY(0, 0, 58, icllp_community0_gpps), > > ICL_COMMUNITY(1, 59, 152, icllp_community1_gpps), > > - ICL_COMMUNITY(2, 153, 215, icllp_community4_gpps), > > - ICL_COMMUNITY(3, 216, 240, icllp_community5_gpps), > > + ICL_COMMUNITY(3, 153, 215, icllp_community4_gpps), > > + ICL_COMMUNITY(4, 216, 240, icllp_community5_gpps), > > }; > > > > static const unsigned int icllp_spi0_pins[] = { 22, 23, 24, 25, 26 }; > > -- > > 2.19.0.397.gdd90340f6a-goog > > > > > -- > With Best Regards, > Andy Shevchenko ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] pinctrl: icelake: Fix the resource number for community-4/5 2018-09-14 21:06 ` Rajat Jain @ 2018-09-14 21:38 ` Rajat Jain 2018-09-20 17:19 ` Rajat Jain 0 siblings, 1 reply; 12+ messages in thread From: Rajat Jain @ 2018-09-14 21:38 UTC (permalink / raw) To: Andy Shevchenko Cc: Mika Westerberg, Andy Shevchenko, linus.walleij, linux-gpio, Linux Kernel Mailing List, Rajat Jain, Banik, Subrata, aamir.bohra On Fri, Sep 14, 2018 at 2:06 PM Rajat Jain <rajatja@google.com> wrote: > > On Fri, Sep 14, 2018 at 12:41 AM Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > > > > On Fri, Sep 14, 2018 at 1:54 AM Rajat Jain <rajatja@google.com> wrote: > > > > > > The Icelake does not have a community-3, and the memory resources are > > > laid out in the following order in the ACPI: > > > > > > resource-0: community-0 registers > > > resource-1: community-1 registers > > > resource-2: community-2 registers > > > resource-3: community-4 registers > > > resource-4: community-5 registers > > > > > > (EDS also describes the communities in the above order). > > > > > > Since the pinctrl driver exposes communities 0, 1, 4, 5, it needs to get > > > the corresponding community registers by getting the resourse number right. > > > Currently the resourse number is not correct for community 4 and 5, thus > > > fix that. > > > > > > > Can you share link to the ACPI dump of the tables? (you may get one by > > running `acpidump -o tables.dat`) > > I don't have that command on my system, but I took > /sys/firmware/acpi/tables/DSDT from the system and disassembled it > using Intel disassembler (iasl -d) and here is the relevant portion > that describes the GPIO controller. The port IDs for communities can > be seen in the below output (i.e. the PCRB()): I realized PCRB() is an ACPI method defined in the same disasembly: Method (PCRB, 1, NotSerialized) { Return ((0xFD000000 + (Arg0 << 0x10))) } > > Device (GPIO) > { > Name (_HID, "INT3455") // _HID: Hardware ID > Name (_UID, Zero) // _UID: Unique ID > Name (_DDN, "GPIO Controller") // _DDN: DOS Device Name > Name (RBUF, ResourceTemplate () > { > Memory32Fixed (ReadWrite, > 0x00000000, // Address Base > 0x00000000, // Address Length > _Y06) > Memory32Fixed (ReadWrite, > 0x00000000, // Address Base > 0x00000000, // Address Length > _Y07) > Memory32Fixed (ReadWrite, > 0x00000000, // Address Base > 0x00000000, // Address Length > _Y08) > Memory32Fixed (ReadWrite, > 0x00000000, // Address Base > 0x00000000, // Address Length > _Y09) > Memory32Fixed (ReadWrite, > 0x00000000, // Address Base > 0x00000000, // Address Length > _Y0A) > Interrupt (ResourceConsumer, Level, ActiveLow, Shared, ,, ) > { > 0x0000000E, > } > }) > Method (_CRS, 0, NotSerialized) // _CRS: Current > Resource Settings > { > CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y06._BAS, > BAS0) // _BAS: Base Address > CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y06._LEN, > LEN0) // _LEN: Length > BAS0 = PCRB (0x6E) > LEN0 = 0x00010000 > CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y07._BAS, > BAS1) // _BAS: Base Address > CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y07._LEN, > LEN1) // _LEN: Length > BAS1 = PCRB (0x6D) > LEN1 = 0x00010000 > CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y08._BAS, > BAS2) // _BAS: Base Address > CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y08._LEN, > LEN2) // _LEN: Length > BAS2 = PCRB (0x6C) > LEN2 = 0x00010000 > CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y09._BAS, > BAS4) // _BAS: Base Address > CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y09._LEN, > LEN4) // _LEN: Length > BAS4 = PCRB (0x6A) > LEN4 = 0x00010000 > CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y0A._BAS, > BAS5) // _BAS: Base Address > CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y0A._LEN, > LEN5) // _LEN: Length > BAS5 = PCRB (0x69) > LEN5 = 0x00010000 > Return (RBUF) /* \_SB_.PCI0.GPIO.RBUF */ > } > > Method (_STA, 0, NotSerialized) // _STA: Status > { > Return (0x0F) > } > } > > Please let me know if this helps, or if you need more info. > > Thanks & Best Regards, > > Rajat > > > > > > Signed-off-by: Rajat Jain <rajatja@google.com> > > > --- > > > drivers/pinctrl/intel/pinctrl-icelake.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/pinctrl/intel/pinctrl-icelake.c b/drivers/pinctrl/intel/pinctrl-icelake.c > > > index 630b966ce081..5b4eaf7c90df 100644 > > > --- a/drivers/pinctrl/intel/pinctrl-icelake.c > > > +++ b/drivers/pinctrl/intel/pinctrl-icelake.c > > > @@ -331,8 +331,8 @@ static const struct intel_padgroup icllp_community5_gpps[] = { > > > static const struct intel_community icllp_communities[] = { > > > ICL_COMMUNITY(0, 0, 58, icllp_community0_gpps), > > > ICL_COMMUNITY(1, 59, 152, icllp_community1_gpps), > > > - ICL_COMMUNITY(2, 153, 215, icllp_community4_gpps), > > > - ICL_COMMUNITY(3, 216, 240, icllp_community5_gpps), > > > + ICL_COMMUNITY(3, 153, 215, icllp_community4_gpps), > > > + ICL_COMMUNITY(4, 216, 240, icllp_community5_gpps), > > > }; > > > > > > static const unsigned int icllp_spi0_pins[] = { 22, 23, 24, 25, 26 }; > > > -- > > > 2.19.0.397.gdd90340f6a-goog > > > > > > > > > -- > > With Best Regards, > > Andy Shevchenko ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] pinctrl: icelake: Fix the resource number for community-4/5 2018-09-14 21:38 ` Rajat Jain @ 2018-09-20 17:19 ` Rajat Jain 2018-09-24 13:59 ` Andy Shevchenko 0 siblings, 1 reply; 12+ messages in thread From: Rajat Jain @ 2018-09-20 17:19 UTC (permalink / raw) To: Andy Shevchenko Cc: Mika Westerberg, Andy Shevchenko, Linus Walleij, linux-gpio, Linux Kernel Mailing List, Rajat Jain, Banik, Subrata, Bohra, Aamir On Fri, Sep 14, 2018 at 2:38 PM Rajat Jain <rajatja@google.com> wrote: > > On Fri, Sep 14, 2018 at 2:06 PM Rajat Jain <rajatja@google.com> wrote: > > > > On Fri, Sep 14, 2018 at 12:41 AM Andy Shevchenko > > <andy.shevchenko@gmail.com> wrote: > > > > > > On Fri, Sep 14, 2018 at 1:54 AM Rajat Jain <rajatja@google.com> wrote: > > > > > > > > The Icelake does not have a community-3, and the memory resources are > > > > laid out in the following order in the ACPI: > > > > > > > > resource-0: community-0 registers > > > > resource-1: community-1 registers > > > > resource-2: community-2 registers > > > > resource-3: community-4 registers > > > > resource-4: community-5 registers > > > > > > > > (EDS also describes the communities in the above order). > > > > > > > > Since the pinctrl driver exposes communities 0, 1, 4, 5, it needs to get > > > > the corresponding community registers by getting the resourse number right. > > > > Currently the resourse number is not correct for community 4 and 5, thus > > > > fix that. > > > > > > > > > > Can you share link to the ACPI dump of the tables? (you may get one by > > > running `acpidump -o tables.dat`) Hello Andy, Any feedback on this patch? I provided a dump of the ACPI below, please let me know if you need more info. Thanks, Rajat > > > > I don't have that command on my system, but I took > > /sys/firmware/acpi/tables/DSDT from the system and disassembled it > > using Intel disassembler (iasl -d) and here is the relevant portion > > that describes the GPIO controller. The port IDs for communities can > > be seen in the below output (i.e. the PCRB()): > > I realized PCRB() is an ACPI method defined in the same disasembly: > > Method (PCRB, 1, NotSerialized) > { > Return ((0xFD000000 + (Arg0 << 0x10))) > } > > > > > Device (GPIO) > > { > > Name (_HID, "INT3455") // _HID: Hardware ID > > Name (_UID, Zero) // _UID: Unique ID > > Name (_DDN, "GPIO Controller") // _DDN: DOS Device Name > > Name (RBUF, ResourceTemplate () > > { > > Memory32Fixed (ReadWrite, > > 0x00000000, // Address Base > > 0x00000000, // Address Length > > _Y06) > > Memory32Fixed (ReadWrite, > > 0x00000000, // Address Base > > 0x00000000, // Address Length > > _Y07) > > Memory32Fixed (ReadWrite, > > 0x00000000, // Address Base > > 0x00000000, // Address Length > > _Y08) > > Memory32Fixed (ReadWrite, > > 0x00000000, // Address Base > > 0x00000000, // Address Length > > _Y09) > > Memory32Fixed (ReadWrite, > > 0x00000000, // Address Base > > 0x00000000, // Address Length > > _Y0A) > > Interrupt (ResourceConsumer, Level, ActiveLow, Shared, ,, ) > > { > > 0x0000000E, > > } > > }) > > Method (_CRS, 0, NotSerialized) // _CRS: Current > > Resource Settings > > { > > CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y06._BAS, > > BAS0) // _BAS: Base Address > > CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y06._LEN, > > LEN0) // _LEN: Length > > BAS0 = PCRB (0x6E) > > LEN0 = 0x00010000 > > CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y07._BAS, > > BAS1) // _BAS: Base Address > > CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y07._LEN, > > LEN1) // _LEN: Length > > BAS1 = PCRB (0x6D) > > LEN1 = 0x00010000 > > CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y08._BAS, > > BAS2) // _BAS: Base Address > > CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y08._LEN, > > LEN2) // _LEN: Length > > BAS2 = PCRB (0x6C) > > LEN2 = 0x00010000 > > CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y09._BAS, > > BAS4) // _BAS: Base Address > > CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y09._LEN, > > LEN4) // _LEN: Length > > BAS4 = PCRB (0x6A) > > LEN4 = 0x00010000 > > CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y0A._BAS, > > BAS5) // _BAS: Base Address > > CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y0A._LEN, > > LEN5) // _LEN: Length > > BAS5 = PCRB (0x69) > > LEN5 = 0x00010000 > > Return (RBUF) /* \_SB_.PCI0.GPIO.RBUF */ > > } > > > > Method (_STA, 0, NotSerialized) // _STA: Status > > { > > Return (0x0F) > > } > > } > > > > Please let me know if this helps, or if you need more info. > > > > Thanks & Best Regards, > > > > Rajat > > > > > > > > > Signed-off-by: Rajat Jain <rajatja@google.com> > > > > --- > > > > drivers/pinctrl/intel/pinctrl-icelake.c | 4 ++-- > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/pinctrl/intel/pinctrl-icelake.c b/drivers/pinctrl/intel/pinctrl-icelake.c > > > > index 630b966ce081..5b4eaf7c90df 100644 > > > > --- a/drivers/pinctrl/intel/pinctrl-icelake.c > > > > +++ b/drivers/pinctrl/intel/pinctrl-icelake.c > > > > @@ -331,8 +331,8 @@ static const struct intel_padgroup icllp_community5_gpps[] = { > > > > static const struct intel_community icllp_communities[] = { > > > > ICL_COMMUNITY(0, 0, 58, icllp_community0_gpps), > > > > ICL_COMMUNITY(1, 59, 152, icllp_community1_gpps), > > > > - ICL_COMMUNITY(2, 153, 215, icllp_community4_gpps), > > > > - ICL_COMMUNITY(3, 216, 240, icllp_community5_gpps), > > > > + ICL_COMMUNITY(3, 153, 215, icllp_community4_gpps), > > > > + ICL_COMMUNITY(4, 216, 240, icllp_community5_gpps), > > > > }; > > > > > > > > static const unsigned int icllp_spi0_pins[] = { 22, 23, 24, 25, 26 }; > > > > -- > > > > 2.19.0.397.gdd90340f6a-goog > > > > > > > > > > > > > -- > > > With Best Regards, > > > Andy Shevchenko ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] pinctrl: icelake: Fix the resource number for community-4/5 2018-09-20 17:19 ` Rajat Jain @ 2018-09-24 13:59 ` Andy Shevchenko 2018-09-24 14:50 ` Banik, Subrata 0 siblings, 1 reply; 12+ messages in thread From: Andy Shevchenko @ 2018-09-24 13:59 UTC (permalink / raw) To: Rajat Jain Cc: Mika Westerberg, Linus Walleij, linux-gpio, Linux Kernel Mailing List, Rajat Jain, Banik, Subrata, Bohra, Aamir On Thu, Sep 20, 2018 at 10:19:34AM -0700, Rajat Jain wrote: > On Fri, Sep 14, 2018 at 2:38 PM Rajat Jain <rajatja@google.com> wrote: > > On Fri, Sep 14, 2018 at 2:06 PM Rajat Jain <rajatja@google.com> wrote: > > > On Fri, Sep 14, 2018 at 12:41 AM Andy Shevchenko > > > <andy.shevchenko@gmail.com> wrote: > > > > On Fri, Sep 14, 2018 at 1:54 AM Rajat Jain <rajatja@google.com> wrote: > > > > > > > > > > The Icelake does not have a community-3, and the memory resources are > > > > > laid out in the following order in the ACPI: > > > > > > > > > > resource-0: community-0 registers > > > > > resource-1: community-1 registers > > > > > resource-2: community-2 registers > > > > > resource-3: community-4 registers > > > > > resource-4: community-5 registers > > > > > > > > > > (EDS also describes the communities in the above order). > > > > > > > > > > Since the pinctrl driver exposes communities 0, 1, 4, 5, it needs to get > > > > > the corresponding community registers by getting the resourse number right. > > > > > Currently the resourse number is not correct for community 4 and 5, thus > > > > > fix that. > > > > > > > > > > > > > Can you share link to the ACPI dump of the tables? (you may get one by > > > > running `acpidump -o tables.dat`) > > Hello Andy, > > Any feedback on this patch? I provided a dump of the ACPI below, > please let me know if you need more info. Sorry it took a while (I had to get tested on our reference BIOS(es) on Ice Lake platforms we have). See my reply below. > > > I don't have that command on my system, but I took > > > /sys/firmware/acpi/tables/DSDT from the system and disassembled it > > > using Intel disassembler (iasl -d) and here is the relevant portion > > > that describes the GPIO controller. The port IDs for communities can > > > be seen in the below output (i.e. the PCRB()): > > > > I realized PCRB() is an ACPI method defined in the same disasembly: > > > > Method (PCRB, 1, NotSerialized) > > { > > Return ((0xFD000000 + (Arg0 << 0x10))) > > } > > > > > > > > Device (GPIO) > > > { > > > Name (_HID, "INT3455") // _HID: Hardware ID > > > Name (_UID, Zero) // _UID: Unique ID > > > Name (_DDN, "GPIO Controller") // _DDN: DOS Device Name > > > Name (RBUF, ResourceTemplate () > > > { > > > Memory32Fixed (ReadWrite, > > > 0x00000000, // Address Base > > > 0x00000000, // Address Length > > > _Y06) > > > Memory32Fixed (ReadWrite, > > > 0x00000000, // Address Base > > > 0x00000000, // Address Length > > > _Y07) > > > Memory32Fixed (ReadWrite, > > > 0x00000000, // Address Base > > > 0x00000000, // Address Length > > > _Y08) > > > Memory32Fixed (ReadWrite, > > > 0x00000000, // Address Base > > > 0x00000000, // Address Length > > > _Y09) > > > Memory32Fixed (ReadWrite, > > > 0x00000000, // Address Base > > > 0x00000000, // Address Length > > > _Y0A) > > > Interrupt (ResourceConsumer, Level, ActiveLow, Shared, ,, ) > > > { > > > 0x0000000E, > > > } > > > }) > > > Method (_CRS, 0, NotSerialized) // _CRS: Current > > > Resource Settings > > > { > > > CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y06._BAS, > > > BAS0) // _BAS: Base Address > > > CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y06._LEN, > > > LEN0) // _LEN: Length > > > BAS0 = PCRB (0x6E) > > > LEN0 = 0x00010000 > > > CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y07._BAS, > > > BAS1) // _BAS: Base Address > > > CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y07._LEN, > > > LEN1) // _LEN: Length > > > BAS1 = PCRB (0x6D) > > > LEN1 = 0x00010000 > > > CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y08._BAS, > > > BAS2) // _BAS: Base Address > > > CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y08._LEN, > > > LEN2) // _LEN: Length > > > BAS2 = PCRB (0x6C) > > > LEN2 = 0x00010000 > > > CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y09._BAS, > > > BAS4) // _BAS: Base Address > > > CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y09._LEN, > > > LEN4) // _LEN: Length > > > BAS4 = PCRB (0x6A) > > > LEN4 = 0x00010000 > > > CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y0A._BAS, > > > BAS5) // _BAS: Base Address > > > CreateDWordField (RBUF, \_SB.PCI0.GPIO._Y0A._LEN, > > > LEN5) // _LEN: Length > > > BAS5 = PCRB (0x69) > > > LEN5 = 0x00010000 > > > Return (RBUF) /* \_SB_.PCI0.GPIO.RBUF */ > > > } > > > > > > Method (_STA, 0, NotSerialized) // _STA: Status > > > { > > > Return (0x0F) > > > } > > > } > > > > > > Please let me know if this helps, or if you need more info. First of all, this is pre-production chip, so, I don't think there is a bug in the driver (yet) discovered. Looking to the above ASL code I may conclude that is definitely is *not* from our reference BIOS. I have checked two versions of it and found that in both we have the following mapping: for LP variant: there are only 4 communities are exported for H variant: there are only 5 communities are exported So, I guess the problem is in ASL code you provided. It simple should not export that community at all. In case you need to do so, there are ways: - contact Intel and ask for a change in reference BIOS - acquire another ACPI ID for the case, or, perhaps use special constants like _HRV for that purpose (also need to contact Intel while doing that) P.S. I think EDS covers it as it present in HW, though not exported by FW. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH] pinctrl: icelake: Fix the resource number for community-4/5 2018-09-24 13:59 ` Andy Shevchenko @ 2018-09-24 14:50 ` Banik, Subrata 2018-09-24 17:04 ` Rajat Jain 2018-09-24 21:05 ` Andy Shevchenko 0 siblings, 2 replies; 12+ messages in thread From: Banik, Subrata @ 2018-09-24 14:50 UTC (permalink / raw) To: Andy Shevchenko, Rajat Jain Cc: Mika Westerberg, Linus Walleij, linux-gpio@vger.kernel.org, Linux Kernel Mailing List, Rajat Jain, Bohra, Aamir HI Andy, You are right that this ASL code is not same with Intel reference BIOS code because BIOS sources are different between what you are looking vs what Chrome OS is using. In Coreboot BIOS, we are more relying on EDS spec and as COM 3 is dedicated for CPU GPIO hence we are mapping, 0/1/2/4/5 (whatever present in EDS vol 1.1) We have ensured that PCR ID and offsets are correct in ASL code for respective community, I don't think anything else really matter from BIOS unless you tell me, that you are having any required that your drive code will only probe for 4 COMM for LP and 5 for ICL-H? Thanks, Subrata -----Original Message----- From: Andy Shevchenko [mailto:andy.shevchenko@gmail.com] Sent: Monday, September 24, 2018 7:30 PM To: Rajat Jain <rajatja@google.com> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>; Linus Walleij <linus.walleij@linaro.org>; linux-gpio@vger.kernel.org; Linux Kernel Mailing List <linux-kernel@vger.kernel.org>; Rajat Jain <rajatxjain@gmail.com>; Banik, Subrata <subrata.banik@intel.com>; Bohra, Aamir <aamir.bohra@intel.com> Subject: Re: [PATCH] pinctrl: icelake: Fix the resource number for community-4/5 On Thu, Sep 20, 2018 at 10:19:34AM -0700, Rajat Jain wrote: > On Fri, Sep 14, 2018 at 2:38 PM Rajat Jain <rajatja@google.com> wrote: > > On Fri, Sep 14, 2018 at 2:06 PM Rajat Jain <rajatja@google.com> wrote: > > > On Fri, Sep 14, 2018 at 12:41 AM Andy Shevchenko > > > <andy.shevchenko@gmail.com> wrote: > > > > On Fri, Sep 14, 2018 at 1:54 AM Rajat Jain <rajatja@google.com> wrote: > > > > > > > > > > The Icelake does not have a community-3, and the memory > > > > > resources are laid out in the following order in the ACPI: > > > > > > > > > > resource-0: community-0 registers > > > > > resource-1: community-1 registers > > > > > resource-2: community-2 registers > > > > > resource-3: community-4 registers > > > > > resource-4: community-5 registers > > > > > > > > > > (EDS also describes the communities in the above order). > > > > > > > > > > Since the pinctrl driver exposes communities 0, 1, 4, 5, it > > > > > needs to get the corresponding community registers by getting the resourse number right. > > > > > Currently the resourse number is not correct for community 4 > > > > > and 5, thus fix that. > > > > > > > > > > > > > Can you share link to the ACPI dump of the tables? (you may get > > > > one by running `acpidump -o tables.dat`) > > Hello Andy, > > Any feedback on this patch? I provided a dump of the ACPI below, > please let me know if you need more info. Sorry it took a while (I had to get tested on our reference BIOS(es) on Ice Lake platforms we have). See my reply below. > > > I don't have that command on my system, but I took > > > /sys/firmware/acpi/tables/DSDT from the system and disassembled it > > > using Intel disassembler (iasl -d) and here is the relevant > > > portion that describes the GPIO controller. The port IDs for > > > communities can be seen in the below output (i.e. the PCRB()): > > > > I realized PCRB() is an ACPI method defined in the same disasembly: > > > > Method (PCRB, 1, NotSerialized) > > { > > Return ((0xFD000000 + (Arg0 << 0x10))) > > } > > > > > > > > Device (GPIO) > > > { > > > Name (_HID, "INT3455") // _HID: Hardware ID > > > Name (_UID, Zero) // _UID: Unique ID > > > Name (_DDN, "GPIO Controller") // _DDN: DOS Device Name > > > Name (RBUF, ResourceTemplate () > > > { > > > Memory32Fixed (ReadWrite, > > > 0x00000000, // Address Base > > > 0x00000000, // Address Length > > > _Y06) > > > Memory32Fixed (ReadWrite, > > > 0x00000000, // Address Base > > > 0x00000000, // Address Length > > > _Y07) > > > Memory32Fixed (ReadWrite, > > > 0x00000000, // Address Base > > > 0x00000000, // Address Length > > > _Y08) > > > Memory32Fixed (ReadWrite, > > > 0x00000000, // Address Base > > > 0x00000000, // Address Length > > > _Y09) > > > Memory32Fixed (ReadWrite, > > > 0x00000000, // Address Base > > > 0x00000000, // Address Length > > > _Y0A) > > > Interrupt (ResourceConsumer, Level, ActiveLow, Shared, ,, ) > > > { > > > 0x0000000E, > > > } > > > }) > > > Method (_CRS, 0, NotSerialized) // _CRS: Current > > > Resource Settings > > > { > > > CreateDWordField (RBUF, > > > \_SB.PCI0.GPIO._Y06._BAS, > > > BAS0) // _BAS: Base Address > > > CreateDWordField (RBUF, > > > \_SB.PCI0.GPIO._Y06._LEN, > > > LEN0) // _LEN: Length > > > BAS0 = PCRB (0x6E) > > > LEN0 = 0x00010000 > > > CreateDWordField (RBUF, > > > \_SB.PCI0.GPIO._Y07._BAS, > > > BAS1) // _BAS: Base Address > > > CreateDWordField (RBUF, > > > \_SB.PCI0.GPIO._Y07._LEN, > > > LEN1) // _LEN: Length > > > BAS1 = PCRB (0x6D) > > > LEN1 = 0x00010000 > > > CreateDWordField (RBUF, > > > \_SB.PCI0.GPIO._Y08._BAS, > > > BAS2) // _BAS: Base Address > > > CreateDWordField (RBUF, > > > \_SB.PCI0.GPIO._Y08._LEN, > > > LEN2) // _LEN: Length > > > BAS2 = PCRB (0x6C) > > > LEN2 = 0x00010000 > > > CreateDWordField (RBUF, > > > \_SB.PCI0.GPIO._Y09._BAS, > > > BAS4) // _BAS: Base Address > > > CreateDWordField (RBUF, > > > \_SB.PCI0.GPIO._Y09._LEN, > > > LEN4) // _LEN: Length > > > BAS4 = PCRB (0x6A) > > > LEN4 = 0x00010000 > > > CreateDWordField (RBUF, > > > \_SB.PCI0.GPIO._Y0A._BAS, > > > BAS5) // _BAS: Base Address > > > CreateDWordField (RBUF, > > > \_SB.PCI0.GPIO._Y0A._LEN, > > > LEN5) // _LEN: Length > > > BAS5 = PCRB (0x69) > > > LEN5 = 0x00010000 > > > Return (RBUF) /* \_SB_.PCI0.GPIO.RBUF */ > > > } > > > > > > Method (_STA, 0, NotSerialized) // _STA: Status > > > { > > > Return (0x0F) > > > } > > > } > > > > > > Please let me know if this helps, or if you need more info. First of all, this is pre-production chip, so, I don't think there is a bug in the driver (yet) discovered. Looking to the above ASL code I may conclude that is definitely is *not* from our reference BIOS. I have checked two versions of it and found that in both we have the following mapping: for LP variant: there are only 4 communities are exported for H variant: there are only 5 communities are exported So, I guess the problem is in ASL code you provided. It simple should not export that community at all. In case you need to do so, there are ways: - contact Intel and ask for a change in reference BIOS - acquire another ACPI ID for the case, or, perhaps use special constants like _HRV for that purpose (also need to contact Intel while doing that) P.S. I think EDS covers it as it present in HW, though not exported by FW. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] pinctrl: icelake: Fix the resource number for community-4/5 2018-09-24 14:50 ` Banik, Subrata @ 2018-09-24 17:04 ` Rajat Jain 2018-09-24 21:09 ` Andy Shevchenko 2018-09-24 21:05 ` Andy Shevchenko 1 sibling, 1 reply; 12+ messages in thread From: Rajat Jain @ 2018-09-24 17:04 UTC (permalink / raw) To: Banik, Subrata Cc: Andy Shevchenko, Mika Westerberg, Linus Walleij, linux-gpio, Linux Kernel Mailing List, Rajat Jain, Bohra, Aamir Hi Andy, On Mon, Sep 24, 2018 at 7:54 AM Banik, Subrata <subrata.banik@intel.com> wrote: > > HI Andy, > > You are right that this ASL code is not same with Intel reference BIOS code because BIOS sources are different between what you are looking vs what Chrome OS is using. In Coreboot BIOS, we are more relying on EDS spec and as COM 3 is dedicated for CPU GPIO hence we are mapping, 0/1/2/4/5 (whatever present in EDS vol 1.1) > > We have ensured that PCR ID and offsets are correct in ASL code for respective community, I don't think anything else really matter from BIOS unless you tell me, that you are having any required that your drive code will only probe for 4 COMM for LP and 5 for ICL-H? > > Thanks, > Subrata > > -----Original Message----- > From: Andy Shevchenko [mailto:andy.shevchenko@gmail.com] > Sent: Monday, September 24, 2018 7:30 PM > To: Rajat Jain <rajatja@google.com> > Cc: Mika Westerberg <mika.westerberg@linux.intel.com>; Linus Walleij <linus.walleij@linaro.org>; linux-gpio@vger.kernel.org; Linux Kernel Mailing List <linux-kernel@vger.kernel.org>; Rajat Jain <rajatxjain@gmail.com>; Banik, Subrata <subrata.banik@intel.com>; Bohra, Aamir <aamir.bohra@intel.com> > Subject: Re: [PATCH] pinctrl: icelake: Fix the resource number for community-4/5 > > On Thu, Sep 20, 2018 at 10:19:34AM -0700, Rajat Jain wrote: > > On Fri, Sep 14, 2018 at 2:38 PM Rajat Jain <rajatja@google.com> wrote: > > > On Fri, Sep 14, 2018 at 2:06 PM Rajat Jain <rajatja@google.com> wrote: > > > > On Fri, Sep 14, 2018 at 12:41 AM Andy Shevchenko > > > > <andy.shevchenko@gmail.com> wrote: > > > > > On Fri, Sep 14, 2018 at 1:54 AM Rajat Jain <rajatja@google.com> wrote: > > > > > > > > > > > > The Icelake does not have a community-3, and the memory > > > > > > resources are laid out in the following order in the ACPI: > > > > > > > > > > > > resource-0: community-0 registers > > > > > > resource-1: community-1 registers > > > > > > resource-2: community-2 registers > > > > > > resource-3: community-4 registers > > > > > > resource-4: community-5 registers > > > > > > > > > > > > (EDS also describes the communities in the above order). > > > > > > > > > > > > Since the pinctrl driver exposes communities 0, 1, 4, 5, it > > > > > > needs to get the corresponding community registers by getting the resourse number right. > > > > > > Currently the resourse number is not correct for community 4 > > > > > > and 5, thus fix that. > > > > > > > > > > > > > > > > Can you share link to the ACPI dump of the tables? (you may get > > > > > one by running `acpidump -o tables.dat`) > > > > Hello Andy, > > > > Any feedback on this patch? I provided a dump of the ACPI below, > > please let me know if you need more info. > > Sorry it took a while (I had to get tested on our reference BIOS(es) on Ice Lake platforms we have). > See my reply below. > > > > > I don't have that command on my system, but I took > > > > /sys/firmware/acpi/tables/DSDT from the system and disassembled it > > > > using Intel disassembler (iasl -d) and here is the relevant > > > > portion that describes the GPIO controller. The port IDs for > > > > communities can be seen in the below output (i.e. the PCRB()): > > > > > > I realized PCRB() is an ACPI method defined in the same disasembly: > > > > > > Method (PCRB, 1, NotSerialized) > > > { > > > Return ((0xFD000000 + (Arg0 << 0x10))) > > > } > > > > > > > > > > > Device (GPIO) > > > > { > > > > Name (_HID, "INT3455") // _HID: Hardware ID > > > > Name (_UID, Zero) // _UID: Unique ID > > > > Name (_DDN, "GPIO Controller") // _DDN: DOS Device Name > > > > Name (RBUF, ResourceTemplate () > > > > { > > > > Memory32Fixed (ReadWrite, > > > > 0x00000000, // Address Base > > > > 0x00000000, // Address Length > > > > _Y06) > > > > Memory32Fixed (ReadWrite, > > > > 0x00000000, // Address Base > > > > 0x00000000, // Address Length > > > > _Y07) > > > > Memory32Fixed (ReadWrite, > > > > 0x00000000, // Address Base > > > > 0x00000000, // Address Length > > > > _Y08) > > > > Memory32Fixed (ReadWrite, > > > > 0x00000000, // Address Base > > > > 0x00000000, // Address Length > > > > _Y09) > > > > Memory32Fixed (ReadWrite, > > > > 0x00000000, // Address Base > > > > 0x00000000, // Address Length > > > > _Y0A) > > > > Interrupt (ResourceConsumer, Level, ActiveLow, Shared, ,, ) > > > > { > > > > 0x0000000E, > > > > } > > > > }) > > > > Method (_CRS, 0, NotSerialized) // _CRS: Current > > > > Resource Settings > > > > { > > > > CreateDWordField (RBUF, > > > > \_SB.PCI0.GPIO._Y06._BAS, > > > > BAS0) // _BAS: Base Address > > > > CreateDWordField (RBUF, > > > > \_SB.PCI0.GPIO._Y06._LEN, > > > > LEN0) // _LEN: Length > > > > BAS0 = PCRB (0x6E) > > > > LEN0 = 0x00010000 > > > > CreateDWordField (RBUF, > > > > \_SB.PCI0.GPIO._Y07._BAS, > > > > BAS1) // _BAS: Base Address > > > > CreateDWordField (RBUF, > > > > \_SB.PCI0.GPIO._Y07._LEN, > > > > LEN1) // _LEN: Length > > > > BAS1 = PCRB (0x6D) > > > > LEN1 = 0x00010000 > > > > CreateDWordField (RBUF, > > > > \_SB.PCI0.GPIO._Y08._BAS, > > > > BAS2) // _BAS: Base Address > > > > CreateDWordField (RBUF, > > > > \_SB.PCI0.GPIO._Y08._LEN, > > > > LEN2) // _LEN: Length > > > > BAS2 = PCRB (0x6C) > > > > LEN2 = 0x00010000 > > > > CreateDWordField (RBUF, > > > > \_SB.PCI0.GPIO._Y09._BAS, > > > > BAS4) // _BAS: Base Address > > > > CreateDWordField (RBUF, > > > > \_SB.PCI0.GPIO._Y09._LEN, > > > > LEN4) // _LEN: Length > > > > BAS4 = PCRB (0x6A) > > > > LEN4 = 0x00010000 > > > > CreateDWordField (RBUF, > > > > \_SB.PCI0.GPIO._Y0A._BAS, > > > > BAS5) // _BAS: Base Address > > > > CreateDWordField (RBUF, > > > > \_SB.PCI0.GPIO._Y0A._LEN, > > > > LEN5) // _LEN: Length > > > > BAS5 = PCRB (0x69) > > > > LEN5 = 0x00010000 > > > > Return (RBUF) /* \_SB_.PCI0.GPIO.RBUF */ > > > > } > > > > > > > > Method (_STA, 0, NotSerialized) // _STA: Status > > > > { > > > > Return (0x0F) > > > > } > > > > } > > > > > > > > Please let me know if this helps, or if you need more info. > > First of all, this is pre-production chip, so, I don't think there is a bug in the driver (yet) discovered. > > Looking to the above ASL code I may conclude that is definitely is *not* from our reference BIOS. > I have checked two versions of it and found that in both we have the following mapping: > for LP variant: there are only 4 communities are exported for H variant: there are only 5 communities are exported > > So, I guess the problem is in ASL code you provided. It simple should not export that community at all. > > In case you need to do so, there are ways: > - contact Intel and ask for a change in reference BIOS > - acquire another ACPI ID for the case, or, perhaps use special constants like > _HRV for that purpose (also need to contact Intel while doing that) As Subrata clarified (Subrata & Aamir are from Intel's Coreboot team), that is not *the* Intel reference BIOS that you are using, but it is *an* Intel generated BIOS/Coreboot image. Andy, can you please let us know in what order are the resources laid out in your reference BIOS for the case when it exports 5 communities (i.e. community-0-2, 4-5)? Thanks, Rajat > > P.S. I think EDS covers it as it present in HW, though not exported by FW. > > -- > With Best Regards, > Andy Shevchenko > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] pinctrl: icelake: Fix the resource number for community-4/5 2018-09-24 17:04 ` Rajat Jain @ 2018-09-24 21:09 ` Andy Shevchenko 2018-09-24 21:31 ` Rajat Jain 0 siblings, 1 reply; 12+ messages in thread From: Andy Shevchenko @ 2018-09-24 21:09 UTC (permalink / raw) To: Rajat Jain Cc: subrata.banik, Mika Westerberg, Linus Walleij, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List, Rajat Jain, aamir.bohra On Mon, Sep 24, 2018 at 8:04 PM Rajat Jain <rajatja@google.com> wrote: > On Mon, Sep 24, 2018 at 7:54 AM Banik, Subrata <subrata.banik@intel.com> wrote: > > First of all, this is pre-production chip, so, I don't think there is a bug in the driver (yet) discovered. > > > > Looking to the above ASL code I may conclude that is definitely is *not* from our reference BIOS. > > I have checked two versions of it and found that in both we have the following mapping: > > for LP variant: there are only 4 communities are exported for H variant: there are only 5 communities are exported > > > > So, I guess the problem is in ASL code you provided. It simple should not export that community at all. > > > > In case you need to do so, there are ways: > > - contact Intel and ask for a change in reference BIOS > > - acquire another ACPI ID for the case, or, perhaps use special constants like > > _HRV for that purpose (also need to contact Intel while doing that) > > As Subrata clarified (Subrata & Aamir are from Intel's Coreboot team), > that is not *the* Intel reference BIOS that you are using, but it is > *an* Intel generated BIOS/Coreboot image. Ah, interesting. > Andy, can you please let us know in what order are the resources laid > out in your reference BIOS for the case when it exports 5 communities > (i.e. community-0-2, 4-5)? We have no hardware with such PCH to answer to this question. LP version exports only 4 communities and order of them as per existing driver. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] pinctrl: icelake: Fix the resource number for community-4/5 2018-09-24 21:09 ` Andy Shevchenko @ 2018-09-24 21:31 ` Rajat Jain 2018-09-25 8:31 ` Andy Shevchenko 0 siblings, 1 reply; 12+ messages in thread From: Rajat Jain @ 2018-09-24 21:31 UTC (permalink / raw) To: Andy Shevchenko Cc: Banik, Subrata, Mika Westerberg, Linus Walleij, linux-gpio, Linux Kernel Mailing List, Rajat Jain, Bohra, Aamir On Mon, Sep 24, 2018 at 2:09 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Mon, Sep 24, 2018 at 8:04 PM Rajat Jain <rajatja@google.com> wrote: > > On Mon, Sep 24, 2018 at 7:54 AM Banik, Subrata <subrata.banik@intel.com> wrote: > > > > First of all, this is pre-production chip, so, I don't think there is a bug in the driver (yet) discovered. > > > > > > Looking to the above ASL code I may conclude that is definitely is *not* from our reference BIOS. > > > I have checked two versions of it and found that in both we have the following mapping: > > > for LP variant: there are only 4 communities are exported for H variant: there are only 5 communities are exported > > > > > > So, I guess the problem is in ASL code you provided. It simple should not export that community at all. > > > > > > In case you need to do so, there are ways: > > > - contact Intel and ask for a change in reference BIOS > > > - acquire another ACPI ID for the case, or, perhaps use special constants like > > > _HRV for that purpose (also need to contact Intel while doing that) > > > > As Subrata clarified (Subrata & Aamir are from Intel's Coreboot team), > > that is not *the* Intel reference BIOS that you are using, but it is > > *an* Intel generated BIOS/Coreboot image. > > Ah, interesting. > > > Andy, can you please let us know in what order are the resources laid > > out in your reference BIOS for the case when it exports 5 communities > > (i.e. community-0-2, 4-5)? > > We have no hardware with such PCH to answer to this question. LP > version exports only 4 communities and order of them as per existing > driver. Got it, thanks. From your response earlier: > Looking to the above ASL code I may conclude that is definitely is *not* from our reference BIOS. > I have checked two versions of it and found that in both we have the following mapping: > for LP variant: there are only 4 communities are exported > for H variant: there are only 5 communities are exported 1) Do you know or recall, the order of communities in ACPI in the H variant? Of course, this is a request for help for getting the info. 2) Trying to understand how would the kernel support both LP and H variant. Is it the assumption that the H variant will have a different ACPI ID than the LP variant (i.e. not "INT3455")? Because it will also have the same problem that we are seeing I think. Thanks for all your help, Rajat > > > -- > With Best Regards, > Andy Shevchenko ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] pinctrl: icelake: Fix the resource number for community-4/5 2018-09-24 21:31 ` Rajat Jain @ 2018-09-25 8:31 ` Andy Shevchenko 0 siblings, 0 replies; 12+ messages in thread From: Andy Shevchenko @ 2018-09-25 8:31 UTC (permalink / raw) To: Rajat Jain Cc: subrata.banik, Mika Westerberg, Linus Walleij, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List, Rajat Jain, aamir.bohra On Tue, Sep 25, 2018 at 12:32 AM Rajat Jain <rajatja@google.com> wrote: > On Mon, Sep 24, 2018 at 2:09 PM Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > > On Mon, Sep 24, 2018 at 8:04 PM Rajat Jain <rajatja@google.com> wrote: > > > On Mon, Sep 24, 2018 at 7:54 AM Banik, Subrata <subrata.banik@intel.com> wrote: > > > > First of all, this is pre-production chip, so, I don't think there is a bug in the driver (yet) discovered. > > > > > > > > Looking to the above ASL code I may conclude that is definitely is *not* from our reference BIOS. > > > > I have checked two versions of it and found that in both we have the following mapping: > > > > for LP variant: there are only 4 communities are exported for H variant: there are only 5 communities are exported > > > > > > > > So, I guess the problem is in ASL code you provided. It simple should not export that community at all. > > > > > > > > In case you need to do so, there are ways: > > > > - contact Intel and ask for a change in reference BIOS > > > > - acquire another ACPI ID for the case, or, perhaps use special constants like > > > > _HRV for that purpose (also need to contact Intel while doing that) > > > Andy, can you please let us know in what order are the resources laid > > > out in your reference BIOS for the case when it exports 5 communities > > > (i.e. community-0-2, 4-5)? > > > > We have no hardware with such PCH to answer to this question. LP > > version exports only 4 communities and order of them as per existing > > driver. > > Got it, thanks. From your response earlier: > > > Looking to the above ASL code I may conclude that is definitely is *not* from our reference BIOS. > > I have checked two versions of it and found that in both we have the following mapping: > > for LP variant: there are only 4 communities are exported > > for H variant: there are only 5 communities are exported > > 1) Do you know or recall, the order of communities in ACPI in the H > variant? Of course, this is a request for help for getting the info. I can't say anything about hardware I didn't see. Even if BIOS code has something, it's not fully guaranteed that in production it will be same. Better to ask Intel BIOS team for the details. > 2) Trying to understand how would the kernel support both LP and H > variant. Is it the assumption that the H variant will have a different > ACPI ID than the LP variant (i.e. not "INT3455")? Because it will > also have the same problem that we are seeing I think. Yes, definitely they have different IDs. You may consult with pinctrl-cannonlake.c driver (better from latest possible kernel version like v4.19-rc5) to see how it's implemented. Moreover, LP and H variants have different pin lists inside communities, so, they can't be substituted and one may consider them as different IPs. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] pinctrl: icelake: Fix the resource number for community-4/5 2018-09-24 14:50 ` Banik, Subrata 2018-09-24 17:04 ` Rajat Jain @ 2018-09-24 21:05 ` Andy Shevchenko 1 sibling, 0 replies; 12+ messages in thread From: Andy Shevchenko @ 2018-09-24 21:05 UTC (permalink / raw) To: subrata.banik Cc: Rajat Jain, Mika Westerberg, Linus Walleij, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List, Rajat Jain, aamir.bohra Please do not top post in the open source mailing lists. On Mon, Sep 24, 2018 at 5:50 PM Banik, Subrata <subrata.banik@intel.com> wrote: > You are right that this ASL code is not same with Intel reference BIOS code because BIOS sources are different between what you are looking vs what Chrome OS is using. In Coreboot BIOS, we are more relying on EDS spec and as COM 3 is dedicated for CPU GPIO hence we are mapping, 0/1/2/4/5 (whatever present in EDS vol 1.1) But how would it possible to make interoperability work if there are *different* firmwares for the *same* ACPI HID? ACPI table is a part of protocol by which firmware sends data to the software. If one breaks this protocol they must use another ACPI HID for that kind of device. > We have ensured that PCR ID and offsets are correct in ASL code for respective community, I don't think anything else really matter from BIOS unless you tell me, that you are having any required that your drive code will only probe for 4 COMM for LP and 5 for ICL-H? See above. So, I do not see any bug in the driver, but in firmware. In this case FW may not use INT3455 HID. This is my understanding, if you would like more details, ask BIOS and ACPI teams. > First of all, this is pre-production chip, so, I don't think there is a bug in the driver (yet) discovered. > > Looking to the above ASL code I may conclude that is definitely is *not* from our reference BIOS. > I have checked two versions of it and found that in both we have the following mapping: > for LP variant: there are only 4 communities are exported for H variant: there are only 5 communities are exported > > So, I guess the problem is in ASL code you provided. It simple should not export that community at all. > > In case you need to do so, there are ways: > - contact Intel and ask for a change in reference BIOS > - acquire another ACPI ID for the case, or, perhaps use special constants like > _HRV for that purpose (also need to contact Intel while doing that) -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-09-25 8:31 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-09-13 22:31 [PATCH] pinctrl: icelake: Fix the resource number for community-4/5 Rajat Jain 2018-09-14 7:33 ` Andy Shevchenko 2018-09-14 21:06 ` Rajat Jain 2018-09-14 21:38 ` Rajat Jain 2018-09-20 17:19 ` Rajat Jain 2018-09-24 13:59 ` Andy Shevchenko 2018-09-24 14:50 ` Banik, Subrata 2018-09-24 17:04 ` Rajat Jain 2018-09-24 21:09 ` Andy Shevchenko 2018-09-24 21:31 ` Rajat Jain 2018-09-25 8:31 ` Andy Shevchenko 2018-09-24 21:05 ` Andy Shevchenko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).