Re: [RFC PATCH 0/2] serial: 8250_dw: IO space + polling mode support

[John Garry]

On 26/02/2018 15:07, John Garry wrote:
> On 26/02/2018 15:02, Andy Shevchenko wrote:
> > On Mon, 2018-02-26 at 13:15 +0000, John Garry wrote:
> > > On 26/02/2018 12:27, Andy Shevchenko wrote:
> > > > On Mon, 2018-02-26 at 14:21 +0200, Andy Shevchenko wrote:
> > > > > On Mon, 2018-02-26 at 11:56 +0000, John Garry wrote:
> > > >
> > > >
> > > > > >    Device (LPC0.CON0) {
> > > > > >      Name (_HID, "HISI1031")
> > > > > >      // Name (_CID, "PNP0501") // cannot support PNP
> > > >
> > > >
> > > > One more question. What is the problem with this CID? Do you have a
> > > > race
> > > > condition in enumeration?
> > >
> > > Hi Andy,
> > >
> > > Not sure if race condition exactly. I tried enabling this CID and a
> > > pnp
> > > device is created in pnpacpi_add_device_handler(), while we have
> > > already
> > > marked the corresponding acpi_device to skip enumeration in ACPI scan
> > > handler (by flagging it as a serial bus slave).
> >
> > Is that code already in upstream?
>
> No, not yet.
>
> > If no, please, Cc next version to me and possible Mika.
>
> Of course. I should be sending it later today.

Hi Andy,

A while ago we discussed on this thread the possibility of adding 
generic 8250 IO space platform driver support for ACPI FW.

In this discussion I mentioned that we require specifically platform 
device support, and not PNP device support, as this is how we enumerate 
the devices in the host controller driver. I think you're familiar with 
this driver - here is the thread posting for reference: 
https://lkml.org/lkml/2018/3/6/230

I would say that there were 2 main takeaway points:
a. for 8250-compatible UART, we should use a PNP driver for ACPI FW
b. you prefered us to change the host driver to use an ACPI handler approach

For b., I was not keen as we already did try the handler in the ACPI 
core code, but this was not so welcome. Reasoning here: 
https://lkml.org/lkml/2018/2/14/532

I did also say that I would prefer not to change approach after a very 
long upstream effort, with no clear end in sight.

However do you have an idea on creating a PNP device in a.? That is, 
enumerate (create) a 8250 PNP device.

If you look at the least driver here:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/bus/hisi_lpc.c

We could have this working with a change in the ACPI probe code, like in 
this code snippet:

list_for_each_entry(child, &adev->children, node) {
	struct resource_entry *rentry;
	LIST_HEAD(resource_list);
        int rc;

        if (!acpi_is_pnp_device(child))
            continue;

        acpi_dev_get_resources(child, &resource_list, NULL, NULL);

        list_for_each_entry(rentry, &resource_list, node) {
            struct resource *res = rentry->res;

            if (res->flags | IORESOURCE_IO)
                hisi_lpc_acpi_xlat_io_res(child, adev, res); /* bad */
        }
        rc = pnpacpi_add_device(child);
        if (rc)
            return rc;
}

Obviously this is not sound as we should not modify the child 
acpi_device resources.

Alternatively, as another approach, I could copy the relevant code 
pnpacpi_add_device() verbatim into this above code, and xlat the 
resource of the PNP device, but it's not good to copy the code like.

Any other ideas?

All the best,
John

> >
> All the best,
> John