>>Qipeng, can you comment on my understanding of the DSDT and the driver?
>> > // Memory32Fixed (ReadWrite, 0x00000000, 0x00001000, BAR1) // SSRAM
>> > Memory32Fixed (ReadWrite, 0x00000000, 0x00001000, MDAT) // PUnit BIOS mailbox Data
>> size: 0x1000
>> And this would be res1 in the driver?
>> > Memory32Fixed (ReadWrite, 0x00000000, 0x00001000, MINF) // PUnit BIOS mailbox Interface and GTD/ISPD mailbox
When boot up, BIOS will rewrite the size of these entries, actually the size for this entry will change to 4B, not the default 0x1000.
This is real strange implementation for us, as before mentioned, BIOS implement like this to make it compatible for wos driver.
On Wed, Oct 14, 2015 at 10:16:06PM -0700, Darren Hart wrote:
> On Sat, Oct 10, 2015 at 03:07:53AM +0000, Zha, Qipeng wrote:
> >
> > >Everything is quite okay, except this BAR thingy.
> >
> > >Can you provide a DSDT excerpt for the device to see what is there?
> >
> > >I can't find such device (by ACPI id) in the tables of the accessible hardware in our lab.
> >
> > Please check below acpi device definition from BIOS.
> > Punit device is created in pmc driver, since BIOS finally reject to create a separate device for Punit.
> >
>
> Qipeng, sorry for the delay. I've been under a rather absurd load. I'd
> like to close on this so we can get this into -next ASAP. Thank you
> for posting the DSDT.
>
> Help us parse what this means so we're all seeing the same thing. I
> see a total of 3 memory addresses and the following IO base address.
> Unfortunately, the comments don't map directly to the driver code, so
> I need to piece this together.
>
> The code in question from the driver is:
>
> #define MAILBOX_REGISTER_SPACE 0x10
>
> +static int intel_punit_get_bars(struct platform_device *pdev) {
> + struct resource *res0, *res1;
> + void __iomem *addr;
> + int size;
> +
> + res0 = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res0) {
> + dev_err(&pdev->dev, "Failed to get iomem resource\n");
> + return -ENXIO;
> + }
> + size = resource_size(res0);
> + if (!devm_request_mem_region(&pdev->dev, res0->start,
> + size, pdev->name)) {
> + dev_err(&pdev->dev, "Failed to request iomem resouce\n");
> + return -EBUSY;
> + }
> +
> + res1 = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + if (!res1) {
> + dev_err(&pdev->dev, "Failed to get iomem resource1\n");
> + return -ENXIO;
> + }
> + size = resource_size(res1);
> + if (!devm_request_mem_region(&pdev->dev, res1->start,
> + size, pdev->name)) {
> + dev_err(&pdev->dev, "Failed to request iomem resouce1\n");
> + return -EBUSY;
> + }
> +
> + addr = ioremap_nocache(res0->start,
> + resource_size(res0) + resource_size(res1));
> + if (!addr) {
> + dev_err(&pdev->dev, "Failed to ioremap ipc base\n");
> + return -ENOMEM;
> + }
> + punit_ipcdev->base[BIOS_IPC] = addr;
> + addr += MAILBOX_REGISTER_SPACE;
> + punit_ipcdev->base[GTDRIVER_IPC] = addr;
> + addr += MAILBOX_REGISTER_SPACE;
> + punit_ipcdev->base[ISPDRIVER_IPC] = addr;
> +
> + return 0;
> +}
>
>
> > Scope (\_SB) {
> > Device(IPC1)
> > {
> > …
> > Name (RBUF, ResourceTemplate ()
> > {
> > Memory32Fixed (ReadWrite, 0x00000000, 0x00001000, BAR0) // IPC1 Bar
>
> size: 0x1000
> IPC1 BAR, I presume this maps to res0 in the driver?
>
> Then the start of this 4096 byte region is assigned by:
>
> punit_ipcdev->base[BIOS_IPC] = addr;
>
> And everything else assigned by intel_punit_get_bards is contained
> within this addr since MAILBOX_REGISTER_SPACE is only 0x10.
>
> Do I have that correct?
>
> > // Memory32Fixed (ReadWrite, 0x00000000, 0x00001000, BAR1) // SSRAM
> > Memory32Fixed (ReadWrite, 0x00000000, 0x00001000, MDAT) // PUnit BIOS mailbox Data
>
> size: 0x1000
> And this would be res1 in the driver?
>
> > Memory32Fixed (ReadWrite, 0x00000000, 0x00001000, MINF) // PUnit BIOS mailbox Interface and GTD/ISPD mailbox
>
> size: 0x1000
>
> This seems odd, especially with the comments in the DSD. A more natural mapping would have been each of the Memory32Fixed macros mapping to the BIOS_IPC, GTDRIVER_IPC, and ISPDRIVER_IPC BARs in the driver. This would indeed be the case if MAILBOX_REGISTER_SPACE was 0x1000 instead of 0x10. Also, if that were the case, then those BARs should be set by using the mapped addr for the 3 separate resources - which is what Andy was getting at.
>
> Qipeng, I assume I misinterpretted something above. Can you point out where I've got this wrong and how the driver is doing the only thing it can given the resources provided by the DSDT?
>
> Thanks,
>
>
> > IO (Decode16, 0x400, 0x480, 0x4, 0x80) // ACPI IO Base address
> > Interrupt (ResourceConsumer, Level, ActiveLow, Exclusive, , , ) {40} // IPC1 IRQ
> > })
> >
> > …
> > }
> > }//end scope
>
> --
> Darren Hart
> Intel Open Source Technology Center
--
Darren Hart
Intel Open Source Technology Center
��.n��������+%������w��{.n������_���v��z����n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�
