Qipeng, can you comment on my understanding of the DSDT and the 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
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
