* [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,
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 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
* 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
end of thread, other threads:[~2018-09-25 8:32 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).