Hi Andy, On Mon, Sep 24, 2018 at 7:54 AM Banik, Subrata <subrata.ba...@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.shevche...@gmail.com] > Sent: Monday, September 24, 2018 7:30 PM > To: Rajat Jain <raja...@google.com> > Cc: Mika Westerberg <mika.westerb...@linux.intel.com>; Linus Walleij > <linus.wall...@linaro.org>; linux-g...@vger.kernel.org; Linux Kernel Mailing > List <linux-kernel@vger.kernel.org>; Rajat Jain <rajatxj...@gmail.com>; > Banik, Subrata <subrata.ba...@intel.com>; Bohra, Aamir <aamir.bo...@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 <raja...@google.com> wrote: > > > On Fri, Sep 14, 2018 at 2:06 PM Rajat Jain <raja...@google.com> wrote: > > > > On Fri, Sep 14, 2018 at 12:41 AM Andy Shevchenko > > > > <andy.shevche...@gmail.com> wrote: > > > > > On Fri, Sep 14, 2018 at 1:54 AM Rajat Jain <raja...@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 > >
