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
condition in enumeration?

Hi Andy,

Not sure if race condition exactly. I tried enabling this CID and a
device is created in pnpacpi_add_device_handler(), while we have
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:

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;
        int rc;

        if (!acpi_is_pnp_device(child))

        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,

All the best,

Reply via email to