* [RFC PATCH 0/2] HISI LPC: Add PNP device support @ 2018-04-20 10:07 ` John Garry 0 siblings, 0 replies; 38+ messages in thread From: John Garry @ 2018-04-20 10:07 UTC (permalink / raw) To: rjw, andriy.shevchenko, linux-acpi, lenb, mika.westerberg, lorenzo.pieralisi Cc: linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang, John Garry This patchset adds support for creating PNP devices for devices attached to the HiSilicon LPC host bus. Currently an mfd-cell (platform device) is created per device on the bus for using ACPI-based firmware. As such, we need require a platform driver for these devices. However for PNP-compatible devices, it would be better to create a PNP device so that we may use the existing PNP driver. The PNP-compatible device we are interested in is the 8250-compatible UART on the Huawei D03 development board. This uart has the following profile: - IO space iotype - no interrupt, so polling mode required - 16550 compatible Currently no platform driver exists for this. I did send an RFC to add support to the 8250_dw driver, but Andy Shevchenko suggested adding PNP support to the host driver instead so we may use the appropriate PNP driver (8250_pnp) without modification, see here: https://lkml.org/lkml/2018/2/26/354 As for the implementation in this patchset, to avoid the PNP scan from creating and adding the device, we reuse the "enumeration_by_parent" flag to hold off adding the PNP device, so the parent driver can modify the IO resources to translate from the bus address to the logical PIO address, and then add the device for probing. This solution is not good - hence the RFC. But I am looking for suggestions on how to solve this. Any opinions or ideas? Thanks! John Garry (2): ACPI / PNP: Don't add "enumeration_by_parent" devices HISI LPC: Add PNP device support drivers/bus/hisi_lpc.c | 38 +++++++++++++++++++++++++++++++++++++- drivers/pnp/pnpacpi/core.c | 12 ++++++++---- 2 files changed, 45 insertions(+), 5 deletions(-) -- 1.9.1 ^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH 0/2] HISI LPC: Add PNP device support @ 2018-04-20 10:07 ` John Garry 0 siblings, 0 replies; 38+ messages in thread From: John Garry @ 2018-04-20 10:07 UTC (permalink / raw) To: rjw, andriy.shevchenko, linux-acpi, lenb, mika.westerberg, lorenzo.pieralisi Cc: linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang, John Garry This patchset adds support for creating PNP devices for devices attached to the HiSilicon LPC host bus. Currently an mfd-cell (platform device) is created per device on the bus for using ACPI-based firmware. As such, we need require a platform driver for these devices. However for PNP-compatible devices, it would be better to create a PNP device so that we may use the existing PNP driver. The PNP-compatible device we are interested in is the 8250-compatible UART on the Huawei D03 development board. This uart has the following profile: - IO space iotype - no interrupt, so polling mode required - 16550 compatible Currently no platform driver exists for this. I did send an RFC to add support to the 8250_dw driver, but Andy Shevchenko suggested adding PNP support to the host driver instead so we may use the appropriate PNP driver (8250_pnp) without modification, see here: https://lkml.org/lkml/2018/2/26/354 As for the implementation in this patchset, to avoid the PNP scan from creating and adding the device, we reuse the "enumeration_by_parent" flag to hold off adding the PNP device, so the parent driver can modify the IO resources to translate from the bus address to the logical PIO address, and then add the device for probing. This solution is not good - hence the RFC. But I am looking for suggestions on how to solve this. Any opinions or ideas? Thanks! John Garry (2): ACPI / PNP: Don't add "enumeration_by_parent" devices HISI LPC: Add PNP device support drivers/bus/hisi_lpc.c | 38 +++++++++++++++++++++++++++++++++++++- drivers/pnp/pnpacpi/core.c | 12 ++++++++---- 2 files changed, 45 insertions(+), 5 deletions(-) -- 1.9.1 ^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH 1/2] ACPI / PNP: Don't add "enumeration_by_parent" devices 2018-04-20 10:07 ` John Garry @ 2018-04-20 10:07 ` John Garry -1 siblings, 0 replies; 38+ messages in thread From: John Garry @ 2018-04-20 10:07 UTC (permalink / raw) To: rjw, andriy.shevchenko, linux-acpi, lenb, mika.westerberg, lorenzo.pieralisi Cc: linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang, John Garry For ACPI devices with the enumeration_by_parent flag set, we expect the parent device to enumerate the device after the ACPI scan. This patch does partially the same for devices which are enumerated as PNP devices. We still want PNP scan code to create the per-ACPI device PNP device, but hold off adding the device to allow the parent to do this optionally. Flag acpi_device.driver_data is used as temp store as a reference to the PNP device for the parent. Signed-off-by: John Garry <john.garry@huawei.com> --- drivers/pnp/pnpacpi/core.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c index 3a4c1aa..92f9d6f 100644 --- a/drivers/pnp/pnpacpi/core.c +++ b/drivers/pnp/pnpacpi/core.c @@ -285,10 +285,14 @@ static int __init pnpacpi_add_device(struct acpi_device *device) if (!dev->active) pnp_init_resources(dev); - error = pnp_add_device(dev); - if (error) { - put_device(&dev->dev); - return error; + if (!device->flags.enumeration_by_parent) { + error = pnp_add_device(dev); + if (error) { + put_device(&dev->dev); + return error; + } + } else { + device->driver_data = dev; } num++; -- 1.9.1 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* [RFC PATCH 1/2] ACPI / PNP: Don't add "enumeration_by_parent" devices @ 2018-04-20 10:07 ` John Garry 0 siblings, 0 replies; 38+ messages in thread From: John Garry @ 2018-04-20 10:07 UTC (permalink / raw) To: rjw, andriy.shevchenko, linux-acpi, lenb, mika.westerberg, lorenzo.pieralisi Cc: linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang, John Garry For ACPI devices with the enumeration_by_parent flag set, we expect the parent device to enumerate the device after the ACPI scan. This patch does partially the same for devices which are enumerated as PNP devices. We still want PNP scan code to create the per-ACPI device PNP device, but hold off adding the device to allow the parent to do this optionally. Flag acpi_device.driver_data is used as temp store as a reference to the PNP device for the parent. Signed-off-by: John Garry <john.garry@huawei.com> --- drivers/pnp/pnpacpi/core.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c index 3a4c1aa..92f9d6f 100644 --- a/drivers/pnp/pnpacpi/core.c +++ b/drivers/pnp/pnpacpi/core.c @@ -285,10 +285,14 @@ static int __init pnpacpi_add_device(struct acpi_device *device) if (!dev->active) pnp_init_resources(dev); - error = pnp_add_device(dev); - if (error) { - put_device(&dev->dev); - return error; + if (!device->flags.enumeration_by_parent) { + error = pnp_add_device(dev); + if (error) { + put_device(&dev->dev); + return error; + } + } else { + device->driver_data = dev; } num++; -- 1.9.1 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/2] ACPI / PNP: Don't add "enumeration_by_parent" devices 2018-04-20 10:07 ` John Garry (?) @ 2018-04-20 13:07 ` Mika Westerberg 2018-04-20 13:24 ` John Garry -1 siblings, 1 reply; 38+ messages in thread From: Mika Westerberg @ 2018-04-20 13:07 UTC (permalink / raw) To: John Garry Cc: rjw, andriy.shevchenko, linux-acpi, lenb, lorenzo.pieralisi, linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang On Fri, Apr 20, 2018 at 06:07:25PM +0800, John Garry wrote: > + } else { > + device->driver_data = dev; I think this deserves a comment explaining why we (ab)use driver_data like this. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/2] ACPI / PNP: Don't add "enumeration_by_parent" devices 2018-04-20 13:07 ` Mika Westerberg @ 2018-04-20 13:24 ` John Garry 0 siblings, 0 replies; 38+ messages in thread From: John Garry @ 2018-04-20 13:24 UTC (permalink / raw) To: Mika Westerberg Cc: rjw, andriy.shevchenko, linux-acpi, lenb, lorenzo.pieralisi, linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang Hi Mika, On 20/04/2018 14:07, Mika Westerberg wrote: > On Fri, Apr 20, 2018 at 06:07:25PM +0800, John Garry wrote: >> + } else { >> + device->driver_data = dev; > > I think this deserves a comment explaining why we (ab)use driver_data > like this. Sure, could add. I didn't see any other way for the acpi_device structure to reference the derived PNP device. TBH, This overall approach is not good since we are creating the PNP device in the scan, and then leaving the device in limbo, waiting for the parent to add it, if at all. There's no rule for this. So I'm looking for ideas on how to improve this. Thanks, John > > . > ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/2] ACPI / PNP: Don't add "enumeration_by_parent" devices @ 2018-04-20 13:24 ` John Garry 0 siblings, 0 replies; 38+ messages in thread From: John Garry @ 2018-04-20 13:24 UTC (permalink / raw) To: Mika Westerberg Cc: rjw, andriy.shevchenko, linux-acpi, lenb, lorenzo.pieralisi, linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang Hi Mika, On 20/04/2018 14:07, Mika Westerberg wrote: > On Fri, Apr 20, 2018 at 06:07:25PM +0800, John Garry wrote: >> + } else { >> + device->driver_data = dev; > > I think this deserves a comment explaining why we (ab)use driver_data > like this. Sure, could add. I didn't see any other way for the acpi_device structure to reference the derived PNP device. TBH, This overall approach is not good since we are creating the PNP device in the scan, and then leaving the device in limbo, waiting for the parent to add it, if at all. There's no rule for this. So I'm looking for ideas on how to improve this. Thanks, John > > . > ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/2] ACPI / PNP: Don't add "enumeration_by_parent" devices 2018-04-20 13:24 ` John Garry (?) @ 2018-04-20 13:52 ` Mika Westerberg 2018-04-20 14:09 ` John Garry -1 siblings, 1 reply; 38+ messages in thread From: Mika Westerberg @ 2018-04-20 13:52 UTC (permalink / raw) To: John Garry Cc: rjw, andriy.shevchenko, linux-acpi, lenb, lorenzo.pieralisi, linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang On Fri, Apr 20, 2018 at 02:24:18PM +0100, John Garry wrote: > Hi Mika, > > On 20/04/2018 14:07, Mika Westerberg wrote: > > On Fri, Apr 20, 2018 at 06:07:25PM +0800, John Garry wrote: > > > + } else { > > > + device->driver_data = dev; > > > > I think this deserves a comment explaining why we (ab)use driver_data > > like this. > > Sure, could add. I didn't see any other way for the acpi_device structure to > reference the derived PNP device. > > TBH, This overall approach is not good since we are creating the PNP device > in the scan, and then leaving the device in limbo, waiting for the parent to > add it, if at all. There's no rule for this. > > So I'm looking for ideas on how to improve this. One idea is to make pnpacpi_add_device() available outside of PNP and call it directly (or some variation) in hisi_lpc.c when it walks over its children. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/2] ACPI / PNP: Don't add "enumeration_by_parent" devices 2018-04-20 13:52 ` Mika Westerberg @ 2018-04-20 14:09 ` John Garry 0 siblings, 0 replies; 38+ messages in thread From: John Garry @ 2018-04-20 14:09 UTC (permalink / raw) To: Mika Westerberg Cc: rjw, andriy.shevchenko, linux-acpi, lenb, lorenzo.pieralisi, linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang On 20/04/2018 14:52, Mika Westerberg wrote: > On Fri, Apr 20, 2018 at 02:24:18PM +0100, John Garry wrote: >> Hi Mika, >> >> On 20/04/2018 14:07, Mika Westerberg wrote: >>> On Fri, Apr 20, 2018 at 06:07:25PM +0800, John Garry wrote: >>>> + } else { >>>> + device->driver_data = dev; >>> >>> I think this deserves a comment explaining why we (ab)use driver_data >>> like this. >> >> Sure, could add. I didn't see any other way for the acpi_device structure to >> reference the derived PNP device. >> >> TBH, This overall approach is not good since we are creating the PNP device >> in the scan, and then leaving the device in limbo, waiting for the parent to >> add it, if at all. There's no rule for this. >> >> So I'm looking for ideas on how to improve this. > Hi Mika, > One idea is to make pnpacpi_add_device() available outside of PNP and > call it directly (or some variation) in hisi_lpc.c when it walks over > its children. > I did consider this initially and it seems quite straightforward. However I think the problem is that we would need to modify the acpi_device child resources from FW with kernel-specific resources, which does not seem right (I think that is what you meant). As I see, this is one reason that we went in the direction of modelling the host as an MFD, as we could set the resources of the mfd_cells. So adding a variant of pnpacpi_add_device() could work, or modifying pnpacpi_add_device() to accept a callback to translate the resources. But this feature is specific to our very special requirement... Thanks, John > . > ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/2] ACPI / PNP: Don't add "enumeration_by_parent" devices @ 2018-04-20 14:09 ` John Garry 0 siblings, 0 replies; 38+ messages in thread From: John Garry @ 2018-04-20 14:09 UTC (permalink / raw) To: Mika Westerberg Cc: rjw, andriy.shevchenko, linux-acpi, lenb, lorenzo.pieralisi, linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang On 20/04/2018 14:52, Mika Westerberg wrote: > On Fri, Apr 20, 2018 at 02:24:18PM +0100, John Garry wrote: >> Hi Mika, >> >> On 20/04/2018 14:07, Mika Westerberg wrote: >>> On Fri, Apr 20, 2018 at 06:07:25PM +0800, John Garry wrote: >>>> + } else { >>>> + device->driver_data = dev; >>> >>> I think this deserves a comment explaining why we (ab)use driver_data >>> like this. >> >> Sure, could add. I didn't see any other way for the acpi_device structure to >> reference the derived PNP device. >> >> TBH, This overall approach is not good since we are creating the PNP device >> in the scan, and then leaving the device in limbo, waiting for the parent to >> add it, if at all. There's no rule for this. >> >> So I'm looking for ideas on how to improve this. > Hi Mika, > One idea is to make pnpacpi_add_device() available outside of PNP and > call it directly (or some variation) in hisi_lpc.c when it walks over > its children. > I did consider this initially and it seems quite straightforward. However I think the problem is that we would need to modify the acpi_device child resources from FW with kernel-specific resources, which does not seem right (I think that is what you meant). As I see, this is one reason that we went in the direction of modelling the host as an MFD, as we could set the resources of the mfd_cells. So adding a variant of pnpacpi_add_device() could work, or modifying pnpacpi_add_device() to accept a callback to translate the resources. But this feature is specific to our very special requirement... Thanks, John > . > ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/2] ACPI / PNP: Don't add "enumeration_by_parent" devices 2018-04-20 14:09 ` John Garry @ 2018-04-26 13:49 ` John Garry -1 siblings, 0 replies; 38+ messages in thread From: John Garry @ 2018-04-26 13:49 UTC (permalink / raw) To: Mika Westerberg, andriy.shevchenko Cc: rjw, linux-acpi, lenb, lorenzo.pieralisi, linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang, Liguozhu (Kenneth) On 20/04/2018 15:09, John Garry wrote: > On 20/04/2018 14:52, Mika Westerberg wrote: >> On Fri, Apr 20, 2018 at 02:24:18PM +0100, John Garry wrote: >>> Hi Mika, >>> >>> On 20/04/2018 14:07, Mika Westerberg wrote: >>>> On Fri, Apr 20, 2018 at 06:07:25PM +0800, John Garry wrote: >>>>> + } else { >>>>> + device->driver_data = dev; >>>> >>>> I think this deserves a comment explaining why we (ab)use driver_data >>>> like this. >>> >>> Sure, could add. I didn't see any other way for the acpi_device >>> structure to >>> reference the derived PNP device. >>> >>> TBH, This overall approach is not good since we are creating the PNP >>> device >>> in the scan, and then leaving the device in limbo, waiting for the >>> parent to >>> add it, if at all. There's no rule for this. >>> >>> So I'm looking for ideas on how to improve this. >> > > Hi Mika, > >> One idea is to make pnpacpi_add_device() available outside of PNP and >> call it directly (or some variation) in hisi_lpc.c when it walks over >> its children. >> > > I did consider this initially and it seems quite straightforward. > > However I think the problem is that we would need to modify the > acpi_device child resources from FW with kernel-specific resources, > which does not seem right (I think that is what you meant). As I see, > this is one reason that we went in the direction of modelling the host > as an MFD, as we could set the resources of the mfd_cells. > > So adding a variant of pnpacpi_add_device() could work, or modifying > pnpacpi_add_device() to accept a callback to translate the resources. > But this feature is specific to our very special requirement... > Hi Andy, Mika, I have spent a bit of time looking at this PNP support issue, and I still can't find a good solution. So - as discussed - I could add the call to pnpacpi_add_device(), but I would need a method to defer the pnp dev probe before resources fixup. As a alternative solution, I could add a callback pointer to pnpacpi_add_device(), for the caller to do the fixup, but this is quite arbitrary in the PNP code. As an alternative, I am strongly considering this patch instead of adding PNP support: -->8 Subject: [PATCH] HISI LPC: Add special handling for 8250-compatible UART For APCI support, for each each child device on the host LPC bus we create an mfd_cell, and, as such, we create a platform device per ACPI child. This creates a problem in 8250-compatible device support. Currently the kernel does not support an suitable 8250- compatible driver for the UART device on the LPC bus on the Huawei D03 board, which has the following profile/ requirements: - 16550 device - platform_driver for ACPI device - IO space iotype - polling mode In principle we should use the 8250_pnp driver for 8250- compatible devices with ACPI firmware. However the host driver does not support PNP devices, and the work is not worth the effort to rework the host driver and PNP code to support such a device. As a alternate solution, add special UART handling to use the 8250 isa generic driver for this one-off device. Signed-off-by: John Garry <john.garry@huawei.com> diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c index 2d4611e..b04425b 100644 --- a/drivers/bus/hisi_lpc.c +++ b/drivers/bus/hisi_lpc.c @@ -18,6 +18,8 @@ #include <linux/of_platform.h> #include <linux/pci.h> #include <linux/slab.h> +#include <linux/serial_8250.h> +#include "../tty/serial/8250/8250.h" #define DRV_NAME "hisi-lpc" @@ -345,6 +347,7 @@ static void hisi_lpc_comm_outs(void *hostdata, unsigned long pio, #define MFD_CHILD_NAME_LEN (ACPI_ID_LEN + sizeof(MFD_CHILD_NAME_PREFIX) - 1) struct hisi_lpc_mfd_cell { + struct plat_serial8250_port serial8250_port; struct mfd_cell_acpi_match acpi_match; char name[MFD_CHILD_NAME_LEN]; char pnpid[ACPI_ID_LEN]; @@ -513,10 +516,31 @@ static int hisi_lpc_acpi_probe(struct device *hostdev) dev_warn(&child->dev, "set resource fail (%d)\n", ret); return ret; } + if (!strcmp(acpi_device_hid(child), "HISI1031")) { + /* + * Special handling for HISI1031 8250-compatible UART: + * Since currently no platform driver exists for this + * ACPI device, the generic 8250 isa driver instead. + * For this, change cell name and add associated + * serial port data. + */ + const struct resource * const io_base_resource = + mfd_cell->resources; + struct plat_serial8250_port ref = + SERIAL8250_PORT(io_base_resource->start, 0); + + memcpy(&hisi_lpc_mfd_cell->serial8250_port, + &ref, sizeof(ref)); + + mfd_cell->name = "serial8250"; + mfd_cell->platform_data = + &hisi_lpc_mfd_cell->serial8250_port; + mfd_cell->pdata_size = sizeof(ref); + } count++; } - ret = mfd_add_devices(hostdev, PLATFORM_DEVID_NONE, + ret = mfd_add_devices(hostdev, PLATFORM_DEVID_AUTO, mfd_cells, cell_num, NULL, 0, NULL); if (ret) { dev_err(hostdev, "failed to add mfd cells (%d)\n", ret); Any issue? Thanks, John > Thanks, > John > >> . >> > ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/2] ACPI / PNP: Don't add "enumeration_by_parent" devices @ 2018-04-26 13:49 ` John Garry 0 siblings, 0 replies; 38+ messages in thread From: John Garry @ 2018-04-26 13:49 UTC (permalink / raw) To: Mika Westerberg, andriy.shevchenko Cc: rjw, linux-acpi, lenb, lorenzo.pieralisi, linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang, Liguozhu (Kenneth) On 20/04/2018 15:09, John Garry wrote: > On 20/04/2018 14:52, Mika Westerberg wrote: >> On Fri, Apr 20, 2018 at 02:24:18PM +0100, John Garry wrote: >>> Hi Mika, >>> >>> On 20/04/2018 14:07, Mika Westerberg wrote: >>>> On Fri, Apr 20, 2018 at 06:07:25PM +0800, John Garry wrote: >>>>> + } else { >>>>> + device->driver_data = dev; >>>> >>>> I think this deserves a comment explaining why we (ab)use driver_data >>>> like this. >>> >>> Sure, could add. I didn't see any other way for the acpi_device >>> structure to >>> reference the derived PNP device. >>> >>> TBH, This overall approach is not good since we are creating the PNP >>> device >>> in the scan, and then leaving the device in limbo, waiting for the >>> parent to >>> add it, if at all. There's no rule for this. >>> >>> So I'm looking for ideas on how to improve this. >> > > Hi Mika, > >> One idea is to make pnpacpi_add_device() available outside of PNP and >> call it directly (or some variation) in hisi_lpc.c when it walks over >> its children. >> > > I did consider this initially and it seems quite straightforward. > > However I think the problem is that we would need to modify the > acpi_device child resources from FW with kernel-specific resources, > which does not seem right (I think that is what you meant). As I see, > this is one reason that we went in the direction of modelling the host > as an MFD, as we could set the resources of the mfd_cells. > > So adding a variant of pnpacpi_add_device() could work, or modifying > pnpacpi_add_device() to accept a callback to translate the resources. > But this feature is specific to our very special requirement... > Hi Andy, Mika, I have spent a bit of time looking at this PNP support issue, and I still can't find a good solution. So - as discussed - I could add the call to pnpacpi_add_device(), but I would need a method to defer the pnp dev probe before resources fixup. As a alternative solution, I could add a callback pointer to pnpacpi_add_device(), for the caller to do the fixup, but this is quite arbitrary in the PNP code. As an alternative, I am strongly considering this patch instead of adding PNP support: -->8 Subject: [PATCH] HISI LPC: Add special handling for 8250-compatible UART For APCI support, for each each child device on the host LPC bus we create an mfd_cell, and, as such, we create a platform device per ACPI child. This creates a problem in 8250-compatible device support. Currently the kernel does not support an suitable 8250- compatible driver for the UART device on the LPC bus on the Huawei D03 board, which has the following profile/ requirements: - 16550 device - platform_driver for ACPI device - IO space iotype - polling mode In principle we should use the 8250_pnp driver for 8250- compatible devices with ACPI firmware. However the host driver does not support PNP devices, and the work is not worth the effort to rework the host driver and PNP code to support such a device. As a alternate solution, add special UART handling to use the 8250 isa generic driver for this one-off device. Signed-off-by: John Garry <john.garry@huawei.com> diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c index 2d4611e..b04425b 100644 --- a/drivers/bus/hisi_lpc.c +++ b/drivers/bus/hisi_lpc.c @@ -18,6 +18,8 @@ #include <linux/of_platform.h> #include <linux/pci.h> #include <linux/slab.h> +#include <linux/serial_8250.h> +#include "../tty/serial/8250/8250.h" #define DRV_NAME "hisi-lpc" @@ -345,6 +347,7 @@ static void hisi_lpc_comm_outs(void *hostdata, unsigned long pio, #define MFD_CHILD_NAME_LEN (ACPI_ID_LEN + sizeof(MFD_CHILD_NAME_PREFIX) - 1) struct hisi_lpc_mfd_cell { + struct plat_serial8250_port serial8250_port; struct mfd_cell_acpi_match acpi_match; char name[MFD_CHILD_NAME_LEN]; char pnpid[ACPI_ID_LEN]; @@ -513,10 +516,31 @@ static int hisi_lpc_acpi_probe(struct device *hostdev) dev_warn(&child->dev, "set resource fail (%d)\n", ret); return ret; } + if (!strcmp(acpi_device_hid(child), "HISI1031")) { + /* + * Special handling for HISI1031 8250-compatible UART: + * Since currently no platform driver exists for this + * ACPI device, the generic 8250 isa driver instead. + * For this, change cell name and add associated + * serial port data. + */ + const struct resource * const io_base_resource = + mfd_cell->resources; + struct plat_serial8250_port ref = + SERIAL8250_PORT(io_base_resource->start, 0); + + memcpy(&hisi_lpc_mfd_cell->serial8250_port, + &ref, sizeof(ref)); + + mfd_cell->name = "serial8250"; + mfd_cell->platform_data = + &hisi_lpc_mfd_cell->serial8250_port; + mfd_cell->pdata_size = sizeof(ref); + } count++; } - ret = mfd_add_devices(hostdev, PLATFORM_DEVID_NONE, + ret = mfd_add_devices(hostdev, PLATFORM_DEVID_AUTO, mfd_cells, cell_num, NULL, 0, NULL); if (ret) { dev_err(hostdev, "failed to add mfd cells (%d)\n", ret); Any issue? Thanks, John > Thanks, > John > >> . >> > ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/2] ACPI / PNP: Don't add "enumeration_by_parent" devices 2018-04-26 13:49 ` John Garry (?) @ 2018-04-26 14:08 ` Mika Westerberg 2018-04-26 14:23 ` John Garry -1 siblings, 1 reply; 38+ messages in thread From: Mika Westerberg @ 2018-04-26 14:08 UTC (permalink / raw) To: John Garry Cc: andriy.shevchenko, rjw, linux-acpi, lenb, lorenzo.pieralisi, linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang, Liguozhu (Kenneth) On Thu, Apr 26, 2018 at 02:49:49PM +0100, John Garry wrote: > diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c > index 2d4611e..b04425b 100644 > --- a/drivers/bus/hisi_lpc.c > +++ b/drivers/bus/hisi_lpc.c > @@ -18,6 +18,8 @@ > #include <linux/of_platform.h> > #include <linux/pci.h> > #include <linux/slab.h> > +#include <linux/serial_8250.h> > +#include "../tty/serial/8250/8250.h" > > #define DRV_NAME "hisi-lpc" > > @@ -345,6 +347,7 @@ static void hisi_lpc_comm_outs(void *hostdata, unsigned > long pio, > #define MFD_CHILD_NAME_LEN (ACPI_ID_LEN + sizeof(MFD_CHILD_NAME_PREFIX) - > 1) > > struct hisi_lpc_mfd_cell { > + struct plat_serial8250_port serial8250_port; > struct mfd_cell_acpi_match acpi_match; > char name[MFD_CHILD_NAME_LEN]; > char pnpid[ACPI_ID_LEN]; > @@ -513,10 +516,31 @@ static int hisi_lpc_acpi_probe(struct device *hostdev) > dev_warn(&child->dev, "set resource fail (%d)\n", ret); > return ret; > } > + if (!strcmp(acpi_device_hid(child), "HISI1031")) { Hmm, there is a way in struct mfd_cell to match child devices using _HID so is there something preventing you from using that? ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/2] ACPI / PNP: Don't add "enumeration_by_parent" devices 2018-04-26 14:08 ` Mika Westerberg @ 2018-04-26 14:23 ` John Garry 0 siblings, 0 replies; 38+ messages in thread From: John Garry @ 2018-04-26 14:23 UTC (permalink / raw) To: Mika Westerberg Cc: andriy.shevchenko, rjw, linux-acpi, lenb, lorenzo.pieralisi, linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang, Liguozhu (Kenneth) On 26/04/2018 15:08, Mika Westerberg wrote: > On Thu, Apr 26, 2018 at 02:49:49PM +0100, John Garry wrote: >> diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c >> index 2d4611e..b04425b 100644 >> --- a/drivers/bus/hisi_lpc.c >> +++ b/drivers/bus/hisi_lpc.c >> @@ -18,6 +18,8 @@ >> #include <linux/of_platform.h> >> #include <linux/pci.h> >> #include <linux/slab.h> >> +#include <linux/serial_8250.h> >> +#include "../tty/serial/8250/8250.h" >> >> #define DRV_NAME "hisi-lpc" >> >> @@ -345,6 +347,7 @@ static void hisi_lpc_comm_outs(void *hostdata, unsigned >> long pio, >> #define MFD_CHILD_NAME_LEN (ACPI_ID_LEN + sizeof(MFD_CHILD_NAME_PREFIX) - >> 1) >> >> struct hisi_lpc_mfd_cell { >> + struct plat_serial8250_port serial8250_port; >> struct mfd_cell_acpi_match acpi_match; >> char name[MFD_CHILD_NAME_LEN]; >> char pnpid[ACPI_ID_LEN]; >> @@ -513,10 +516,31 @@ static int hisi_lpc_acpi_probe(struct device *hostdev) >> dev_warn(&child->dev, "set resource fail (%d)\n", ret); >> return ret; >> } >> + if (!strcmp(acpi_device_hid(child), "HISI1031")) { > Hi Mika, > Hmm, there is a way in struct mfd_cell to match child devices using _HID > so is there something preventing you from using that? Not that I know about. Can you describe this method? I guess I also don't need to set the mfd_cell pnpid either for this special case device. Thanks, John > > . > ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/2] ACPI / PNP: Don't add "enumeration_by_parent" devices @ 2018-04-26 14:23 ` John Garry 0 siblings, 0 replies; 38+ messages in thread From: John Garry @ 2018-04-26 14:23 UTC (permalink / raw) To: Mika Westerberg Cc: andriy.shevchenko, rjw, linux-acpi, lenb, lorenzo.pieralisi, linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang, Liguozhu (Kenneth) On 26/04/2018 15:08, Mika Westerberg wrote: > On Thu, Apr 26, 2018 at 02:49:49PM +0100, John Garry wrote: >> diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c >> index 2d4611e..b04425b 100644 >> --- a/drivers/bus/hisi_lpc.c >> +++ b/drivers/bus/hisi_lpc.c >> @@ -18,6 +18,8 @@ >> #include <linux/of_platform.h> >> #include <linux/pci.h> >> #include <linux/slab.h> >> +#include <linux/serial_8250.h> >> +#include "../tty/serial/8250/8250.h" >> >> #define DRV_NAME "hisi-lpc" >> >> @@ -345,6 +347,7 @@ static void hisi_lpc_comm_outs(void *hostdata, unsigned >> long pio, >> #define MFD_CHILD_NAME_LEN (ACPI_ID_LEN + sizeof(MFD_CHILD_NAME_PREFIX) - >> 1) >> >> struct hisi_lpc_mfd_cell { >> + struct plat_serial8250_port serial8250_port; >> struct mfd_cell_acpi_match acpi_match; >> char name[MFD_CHILD_NAME_LEN]; >> char pnpid[ACPI_ID_LEN]; >> @@ -513,10 +516,31 @@ static int hisi_lpc_acpi_probe(struct device *hostdev) >> dev_warn(&child->dev, "set resource fail (%d)\n", ret); >> return ret; >> } >> + if (!strcmp(acpi_device_hid(child), "HISI1031")) { > Hi Mika, > Hmm, there is a way in struct mfd_cell to match child devices using _HID > so is there something preventing you from using that? Not that I know about. Can you describe this method? I guess I also don't need to set the mfd_cell pnpid either for this special case device. Thanks, John > > . > ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/2] ACPI / PNP: Don't add "enumeration_by_parent" devices 2018-04-26 14:23 ` John Garry (?) @ 2018-04-26 14:40 ` Mika Westerberg -1 siblings, 0 replies; 38+ messages in thread From: Mika Westerberg @ 2018-04-26 14:40 UTC (permalink / raw) To: John Garry Cc: andriy.shevchenko, rjw, linux-acpi, lenb, lorenzo.pieralisi, linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang, Liguozhu (Kenneth) On Thu, Apr 26, 2018 at 03:23:17PM +0100, John Garry wrote: > Not that I know about. Can you describe this method? I guess I also don't > need to set the mfd_cell pnpid either for this special case device. There is some documentation in "MFD devices" chapter of Documentation/acpi/enumeration.txt at least. ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/2] ACPI / PNP: Don't add "enumeration_by_parent" devices 2018-04-26 14:23 ` John Garry @ 2018-04-27 9:17 ` John Garry -1 siblings, 0 replies; 38+ messages in thread From: John Garry @ 2018-04-27 9:17 UTC (permalink / raw) To: Mika Westerberg Cc: andriy.shevchenko, rjw, linux-acpi, lenb, lorenzo.pieralisi, linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang, Liguozhu (Kenneth), lee.jones On 26/04/2018 15:23, John Garry wrote: + > On 26/04/2018 15:08, Mika Westerberg wrote: >> On Thu, Apr 26, 2018 at 02:49:49PM +0100, John Garry wrote: >>> diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c >>> index 2d4611e..b04425b 100644 >>> --- a/drivers/bus/hisi_lpc.c >>> +++ b/drivers/bus/hisi_lpc.c >>> @@ -18,6 +18,8 @@ >>> #include <linux/of_platform.h> >>> #include <linux/pci.h> >>> #include <linux/slab.h> >>> +#include <linux/serial_8250.h> >>> +#include "../tty/serial/8250/8250.h" >>> >>> #define DRV_NAME "hisi-lpc" >>> >>> @@ -345,6 +347,7 @@ static void hisi_lpc_comm_outs(void *hostdata, >>> unsigned >>> long pio, >>> #define MFD_CHILD_NAME_LEN (ACPI_ID_LEN + >>> sizeof(MFD_CHILD_NAME_PREFIX) - >>> 1) >>> >>> struct hisi_lpc_mfd_cell { >>> + struct plat_serial8250_port serial8250_port; >>> struct mfd_cell_acpi_match acpi_match; >>> char name[MFD_CHILD_NAME_LEN]; >>> char pnpid[ACPI_ID_LEN]; >>> @@ -513,10 +516,31 @@ static int hisi_lpc_acpi_probe(struct device >>> *hostdev) >>> dev_warn(&child->dev, "set resource fail (%d)\n", ret); >>> return ret; >>> } >>> + if (!strcmp(acpi_device_hid(child), "HISI1031")) { >> > > Hi Mika, > >> Hmm, there is a way in struct mfd_cell to match child devices using _HID >> so is there something preventing you from using that? > > Not that I know about. Can you describe this method? I guess I also > don't need to set the mfd_cell pnpid either for this special case device. > So we using the mfd_cell to match child devices using _HID. At a glance, I don't actually see other drivers to use mfd_cell_acpi_match.pnpid . Anyway we don't use static tables as we need to update the resources of the cell dynamically. However I do look at a driver like intel_quark_i2c_gpio.c, and this dynamically modifies the value of global mfd_cell array here: https://elixir.bootlin.com/linux/latest/source/drivers/mfd/intel_quark_i2c_gpio.c#L266 I know the cell array is only used at probe time, but this did not look to be good standard practice to me. Thanks, John > Thanks, > John > > >> >> . >> > ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/2] ACPI / PNP: Don't add "enumeration_by_parent" devices @ 2018-04-27 9:17 ` John Garry 0 siblings, 0 replies; 38+ messages in thread From: John Garry @ 2018-04-27 9:17 UTC (permalink / raw) To: Mika Westerberg Cc: andriy.shevchenko, rjw, linux-acpi, lenb, lorenzo.pieralisi, linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang, Liguozhu (Kenneth), lee.jones On 26/04/2018 15:23, John Garry wrote: + > On 26/04/2018 15:08, Mika Westerberg wrote: >> On Thu, Apr 26, 2018 at 02:49:49PM +0100, John Garry wrote: >>> diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c >>> index 2d4611e..b04425b 100644 >>> --- a/drivers/bus/hisi_lpc.c >>> +++ b/drivers/bus/hisi_lpc.c >>> @@ -18,6 +18,8 @@ >>> #include <linux/of_platform.h> >>> #include <linux/pci.h> >>> #include <linux/slab.h> >>> +#include <linux/serial_8250.h> >>> +#include "../tty/serial/8250/8250.h" >>> >>> #define DRV_NAME "hisi-lpc" >>> >>> @@ -345,6 +347,7 @@ static void hisi_lpc_comm_outs(void *hostdata, >>> unsigned >>> long pio, >>> #define MFD_CHILD_NAME_LEN (ACPI_ID_LEN + >>> sizeof(MFD_CHILD_NAME_PREFIX) - >>> 1) >>> >>> struct hisi_lpc_mfd_cell { >>> + struct plat_serial8250_port serial8250_port; >>> struct mfd_cell_acpi_match acpi_match; >>> char name[MFD_CHILD_NAME_LEN]; >>> char pnpid[ACPI_ID_LEN]; >>> @@ -513,10 +516,31 @@ static int hisi_lpc_acpi_probe(struct device >>> *hostdev) >>> dev_warn(&child->dev, "set resource fail (%d)\n", ret); >>> return ret; >>> } >>> + if (!strcmp(acpi_device_hid(child), "HISI1031")) { >> > > Hi Mika, > >> Hmm, there is a way in struct mfd_cell to match child devices using _HID >> so is there something preventing you from using that? > > Not that I know about. Can you describe this method? I guess I also > don't need to set the mfd_cell pnpid either for this special case device. > So we using the mfd_cell to match child devices using _HID. At a glance, I don't actually see other drivers to use mfd_cell_acpi_match.pnpid . Anyway we don't use static tables as we need to update the resources of the cell dynamically. However I do look at a driver like intel_quark_i2c_gpio.c, and this dynamically modifies the value of global mfd_cell array here: https://elixir.bootlin.com/linux/latest/source/drivers/mfd/intel_quark_i2c_gpio.c#L266 I know the cell array is only used at probe time, but this did not look to be good standard practice to me. Thanks, John > Thanks, > John > > >> >> . >> > ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/2] ACPI / PNP: Don't add "enumeration_by_parent" devices 2018-04-27 9:17 ` John Garry (?) @ 2018-04-30 5:36 ` Lee Jones 2018-04-30 9:00 ` John Garry -1 siblings, 1 reply; 38+ messages in thread From: Lee Jones @ 2018-04-30 5:36 UTC (permalink / raw) To: John Garry Cc: Mika Westerberg, andriy.shevchenko, rjw, linux-acpi, lenb, lorenzo.pieralisi, linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang, Liguozhu (Kenneth) On Fri, 27 Apr 2018, John Garry wrote: > On 26/04/2018 15:23, John Garry wrote: > > On 26/04/2018 15:08, Mika Westerberg wrote: > > > On Thu, Apr 26, 2018 at 02:49:49PM +0100, John Garry wrote: > > > > diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c > > > > index 2d4611e..b04425b 100644 > > > > --- a/drivers/bus/hisi_lpc.c > > > > +++ b/drivers/bus/hisi_lpc.c > > > > @@ -18,6 +18,8 @@ > > > > #include <linux/of_platform.h> > > > > #include <linux/pci.h> > > > > #include <linux/slab.h> > > > > +#include <linux/serial_8250.h> > > > > +#include "../tty/serial/8250/8250.h" > > > > > > > > #define DRV_NAME "hisi-lpc" > > > > > > > > @@ -345,6 +347,7 @@ static void hisi_lpc_comm_outs(void *hostdata, > > > > unsigned > > > > long pio, > > > > #define MFD_CHILD_NAME_LEN (ACPI_ID_LEN + > > > > sizeof(MFD_CHILD_NAME_PREFIX) - > > > > 1) > > > > > > > > struct hisi_lpc_mfd_cell { > > > > + struct plat_serial8250_port serial8250_port; > > > > struct mfd_cell_acpi_match acpi_match; > > > > char name[MFD_CHILD_NAME_LEN]; > > > > char pnpid[ACPI_ID_LEN]; > > > > @@ -513,10 +516,31 @@ static int hisi_lpc_acpi_probe(struct device > > > > *hostdev) > > > > dev_warn(&child->dev, "set resource fail (%d)\n", ret); > > > > return ret; > > > > } > > > > + if (!strcmp(acpi_device_hid(child), "HISI1031")) { > > > > > > > Hi Mika, > > > > > Hmm, there is a way in struct mfd_cell to match child devices using _HID > > > so is there something preventing you from using that? > > > > Not that I know about. Can you describe this method? I guess I also > > don't need to set the mfd_cell pnpid either for this special case device. > > > > So we using the mfd_cell to match child devices using _HID. At a glance, I > don't actually see other drivers to use mfd_cell_acpi_match.pnpid . > > Anyway we don't use static tables as we need to update the resources of the > cell dynamically. However I do look at a driver like intel_quark_i2c_gpio.c, > and this dynamically modifies the value of global mfd_cell array here: > https://elixir.bootlin.com/linux/latest/source/drivers/mfd/intel_quark_i2c_gpio.c#L266 > > I know the cell array is only used at probe time, but this did not look to > be good standard practice to me. Lots of drivers do this to supply dynamic data. If there is no other sane way of providing such data, it's fine to do. Although each situation should be dealt with on a case-by-case basis. -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/2] ACPI / PNP: Don't add "enumeration_by_parent" devices 2018-04-30 5:36 ` Lee Jones @ 2018-04-30 9:00 ` John Garry 0 siblings, 0 replies; 38+ messages in thread From: John Garry @ 2018-04-30 9:00 UTC (permalink / raw) To: Lee Jones Cc: Mika Westerberg, andriy.shevchenko, rjw, linux-acpi, lenb, lorenzo.pieralisi, linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang, Liguozhu (Kenneth) On 30/04/2018 06:36, Lee Jones wrote: > On Fri, 27 Apr 2018, John Garry wrote: >> On 26/04/2018 15:23, John Garry wrote: >>> On 26/04/2018 15:08, Mika Westerberg wrote: >>>> On Thu, Apr 26, 2018 at 02:49:49PM +0100, John Garry wrote: >>>>> diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c >>>>> index 2d4611e..b04425b 100644 >>>>> --- a/drivers/bus/hisi_lpc.c >>>>> +++ b/drivers/bus/hisi_lpc.c >>>>> @@ -18,6 +18,8 @@ >>>>> #include <linux/of_platform.h> >>>>> #include <linux/pci.h> >>>>> #include <linux/slab.h> >>>>> +#include <linux/serial_8250.h> >>>>> +#include "../tty/serial/8250/8250.h" >>>>> >>>>> #define DRV_NAME "hisi-lpc" >>>>> >>>>> @@ -345,6 +347,7 @@ static void hisi_lpc_comm_outs(void *hostdata, >>>>> unsigned >>>>> long pio, >>>>> #define MFD_CHILD_NAME_LEN (ACPI_ID_LEN + >>>>> sizeof(MFD_CHILD_NAME_PREFIX) - >>>>> 1) >>>>> >>>>> struct hisi_lpc_mfd_cell { >>>>> + struct plat_serial8250_port serial8250_port; >>>>> struct mfd_cell_acpi_match acpi_match; >>>>> char name[MFD_CHILD_NAME_LEN]; >>>>> char pnpid[ACPI_ID_LEN]; >>>>> @@ -513,10 +516,31 @@ static int hisi_lpc_acpi_probe(struct device >>>>> *hostdev) >>>>> dev_warn(&child->dev, "set resource fail (%d)\n", ret); >>>>> return ret; >>>>> } >>>>> + if (!strcmp(acpi_device_hid(child), "HISI1031")) { >>>> >>> >>> Hi Mika, >>> >>>> Hmm, there is a way in struct mfd_cell to match child devices using _HID >>>> so is there something preventing you from using that? >>> >>> Not that I know about. Can you describe this method? I guess I also >>> don't need to set the mfd_cell pnpid either for this special case device. >>> >> >> So we using the mfd_cell to match child devices using _HID. At a glance, I >> don't actually see other drivers to use mfd_cell_acpi_match.pnpid . >> >> Anyway we don't use static tables as we need to update the resources of the >> cell dynamically. However I do look at a driver like intel_quark_i2c_gpio.c, >> and this dynamically modifies the value of global mfd_cell array here: >> https://elixir.bootlin.com/linux/latest/source/drivers/mfd/intel_quark_i2c_gpio.c#L266 >> >> I know the cell array is only used at probe time, but this did not look to >> be good standard practice to me. > > Lots of drivers do this to supply dynamic data. If there is no other > sane way of providing such data, it's fine to do. Although each > situation should be dealt with on a case-by-case basis. > Hi Lee, Thanks for your input. I do see others drivers which use dynamic mem for the mfd_cells (like cros_ec_dev.c), so what we're doing in this driver already is not totally unchartered territory. But creating the MFD cells from the ACPI table could be ... Anyway, I'll cc you in my next patchset and maybe you can kindly check it. Cheers, John ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/2] ACPI / PNP: Don't add "enumeration_by_parent" devices @ 2018-04-30 9:00 ` John Garry 0 siblings, 0 replies; 38+ messages in thread From: John Garry @ 2018-04-30 9:00 UTC (permalink / raw) To: Lee Jones Cc: Mika Westerberg, andriy.shevchenko, rjw, linux-acpi, lenb, lorenzo.pieralisi, linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang, Liguozhu (Kenneth) On 30/04/2018 06:36, Lee Jones wrote: > On Fri, 27 Apr 2018, John Garry wrote: >> On 26/04/2018 15:23, John Garry wrote: >>> On 26/04/2018 15:08, Mika Westerberg wrote: >>>> On Thu, Apr 26, 2018 at 02:49:49PM +0100, John Garry wrote: >>>>> diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c >>>>> index 2d4611e..b04425b 100644 >>>>> --- a/drivers/bus/hisi_lpc.c >>>>> +++ b/drivers/bus/hisi_lpc.c >>>>> @@ -18,6 +18,8 @@ >>>>> #include <linux/of_platform.h> >>>>> #include <linux/pci.h> >>>>> #include <linux/slab.h> >>>>> +#include <linux/serial_8250.h> >>>>> +#include "../tty/serial/8250/8250.h" >>>>> >>>>> #define DRV_NAME "hisi-lpc" >>>>> >>>>> @@ -345,6 +347,7 @@ static void hisi_lpc_comm_outs(void *hostdata, >>>>> unsigned >>>>> long pio, >>>>> #define MFD_CHILD_NAME_LEN (ACPI_ID_LEN + >>>>> sizeof(MFD_CHILD_NAME_PREFIX) - >>>>> 1) >>>>> >>>>> struct hisi_lpc_mfd_cell { >>>>> + struct plat_serial8250_port serial8250_port; >>>>> struct mfd_cell_acpi_match acpi_match; >>>>> char name[MFD_CHILD_NAME_LEN]; >>>>> char pnpid[ACPI_ID_LEN]; >>>>> @@ -513,10 +516,31 @@ static int hisi_lpc_acpi_probe(struct device >>>>> *hostdev) >>>>> dev_warn(&child->dev, "set resource fail (%d)\n", ret); >>>>> return ret; >>>>> } >>>>> + if (!strcmp(acpi_device_hid(child), "HISI1031")) { >>>> >>> >>> Hi Mika, >>> >>>> Hmm, there is a way in struct mfd_cell to match child devices using _HID >>>> so is there something preventing you from using that? >>> >>> Not that I know about. Can you describe this method? I guess I also >>> don't need to set the mfd_cell pnpid either for this special case device. >>> >> >> So we using the mfd_cell to match child devices using _HID. At a glance, I >> don't actually see other drivers to use mfd_cell_acpi_match.pnpid . >> >> Anyway we don't use static tables as we need to update the resources of the >> cell dynamically. However I do look at a driver like intel_quark_i2c_gpio.c, >> and this dynamically modifies the value of global mfd_cell array here: >> https://elixir.bootlin.com/linux/latest/source/drivers/mfd/intel_quark_i2c_gpio.c#L266 >> >> I know the cell array is only used at probe time, but this did not look to >> be good standard practice to me. > > Lots of drivers do this to supply dynamic data. If there is no other > sane way of providing such data, it's fine to do. Although each > situation should be dealt with on a case-by-case basis. > Hi Lee, Thanks for your input. I do see others drivers which use dynamic mem for the mfd_cells (like cros_ec_dev.c), so what we're doing in this driver already is not totally unchartered territory. But creating the MFD cells from the ACPI table could be ... Anyway, I'll cc you in my next patchset and maybe you can kindly check it. Cheers, John ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/2] ACPI / PNP: Don't add "enumeration_by_parent" devices 2018-04-30 9:00 ` John Garry (?) @ 2018-04-30 9:26 ` Lee Jones 2018-04-30 9:35 ` John Garry -1 siblings, 1 reply; 38+ messages in thread From: Lee Jones @ 2018-04-30 9:26 UTC (permalink / raw) To: John Garry Cc: Mika Westerberg, andriy.shevchenko, rjw, linux-acpi, lenb, lorenzo.pieralisi, linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang, Liguozhu (Kenneth) On Mon, 30 Apr 2018, John Garry wrote: > On 30/04/2018 06:36, Lee Jones wrote: > > On Fri, 27 Apr 2018, John Garry wrote: > > > On 26/04/2018 15:23, John Garry wrote: > > > > On 26/04/2018 15:08, Mika Westerberg wrote: > > > > > On Thu, Apr 26, 2018 at 02:49:49PM +0100, John Garry wrote: > > > > > > diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c > > > > > > index 2d4611e..b04425b 100644 > > > > > > --- a/drivers/bus/hisi_lpc.c > > > > > > +++ b/drivers/bus/hisi_lpc.c > > > > > > @@ -18,6 +18,8 @@ > > > > > > #include <linux/of_platform.h> > > > > > > #include <linux/pci.h> > > > > > > #include <linux/slab.h> > > > > > > +#include <linux/serial_8250.h> > > > > > > +#include "../tty/serial/8250/8250.h" > > > > > > > > > > > > #define DRV_NAME "hisi-lpc" > > > > > > > > > > > > @@ -345,6 +347,7 @@ static void hisi_lpc_comm_outs(void *hostdata, > > > > > > unsigned > > > > > > long pio, > > > > > > #define MFD_CHILD_NAME_LEN (ACPI_ID_LEN + > > > > > > sizeof(MFD_CHILD_NAME_PREFIX) - > > > > > > 1) > > > > > > > > > > > > struct hisi_lpc_mfd_cell { > > > > > > + struct plat_serial8250_port serial8250_port; > > > > > > struct mfd_cell_acpi_match acpi_match; > > > > > > char name[MFD_CHILD_NAME_LEN]; > > > > > > char pnpid[ACPI_ID_LEN]; > > > > > > @@ -513,10 +516,31 @@ static int hisi_lpc_acpi_probe(struct device > > > > > > *hostdev) > > > > > > dev_warn(&child->dev, "set resource fail (%d)\n", ret); > > > > > > return ret; > > > > > > } > > > > > > + if (!strcmp(acpi_device_hid(child), "HISI1031")) { > > > > > > > > > > > > > Hi Mika, > > > > > > > > > Hmm, there is a way in struct mfd_cell to match child devices using _HID > > > > > so is there something preventing you from using that? > > > > > > > > Not that I know about. Can you describe this method? I guess I also > > > > don't need to set the mfd_cell pnpid either for this special case device. > > > > > > > > > > So we using the mfd_cell to match child devices using _HID. At a glance, I > > > don't actually see other drivers to use mfd_cell_acpi_match.pnpid . > > > > > > Anyway we don't use static tables as we need to update the resources of the > > > cell dynamically. However I do look at a driver like intel_quark_i2c_gpio.c, > > > and this dynamically modifies the value of global mfd_cell array here: > > > https://elixir.bootlin.com/linux/latest/source/drivers/mfd/intel_quark_i2c_gpio.c#L266 > > > > > > I know the cell array is only used at probe time, but this did not look to > > > be good standard practice to me. > > > > Lots of drivers do this to supply dynamic data. If there is no other > > sane way of providing such data, it's fine to do. Although each > > situation should be dealt with on a case-by-case basis. > > > > Hi Lee, > > Thanks for your input. > > I do see others drivers which use dynamic mem for the mfd_cells (like > cros_ec_dev.c), so what we're doing in this driver already is not totally > unchartered territory. But creating the MFD cells from the ACPI table could > be ... Right. I don't normally like mixing platform data technologies (MFD, ACPI and DT). I normally NACK patches which take information from Device Tree and populate MFD cells with it. ACPI would be the same I guess. > Anyway, I'll cc you in my next patchset and maybe you can kindly check it. > -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/2] ACPI / PNP: Don't add "enumeration_by_parent" devices 2018-04-30 9:26 ` Lee Jones @ 2018-04-30 9:35 ` John Garry 0 siblings, 0 replies; 38+ messages in thread From: John Garry @ 2018-04-30 9:35 UTC (permalink / raw) To: Lee Jones Cc: Mika Westerberg, andriy.shevchenko, rjw, linux-acpi, lenb, lorenzo.pieralisi, linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang, Liguozhu (Kenneth) >>>> So we using the mfd_cell to match child devices using _HID. At a glance, I >>>> don't actually see other drivers to use mfd_cell_acpi_match.pnpid . >>>> >>>> Anyway we don't use static tables as we need to update the resources of the >>>> cell dynamically. However I do look at a driver like intel_quark_i2c_gpio.c, >>>> and this dynamically modifies the value of global mfd_cell array here: >>>> https://elixir.bootlin.com/linux/latest/source/drivers/mfd/intel_quark_i2c_gpio.c#L266 >>>> >>>> I know the cell array is only used at probe time, but this did not look to >>>> be good standard practice to me. >>> >>> Lots of drivers do this to supply dynamic data. If there is no other >>> sane way of providing such data, it's fine to do. Although each >>> situation should be dealt with on a case-by-case basis. >>> >> >> Hi Lee, >> >> Thanks for your input. >> >> I do see others drivers which use dynamic mem for the mfd_cells (like >> cros_ec_dev.c), so what we're doing in this driver already is not totally >> unchartered territory. But creating the MFD cells from the ACPI table could >> be ... > > Right. I don't normally like mixing platform data technologies (MFD, > ACPI and DT). I normally NACK patches which take information from > Device Tree and populate MFD cells with it. ACPI would be the same I > guess. Oh, well that is what we have in this driver. So what's the preferred approach? Just not use MFD model at all if ACPI/DT needs to be scanned for data to create the cells? Thanks, John > >> Anyway, I'll cc you in my next patchset and maybe you can kindly check it. >> > ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/2] ACPI / PNP: Don't add "enumeration_by_parent" devices @ 2018-04-30 9:35 ` John Garry 0 siblings, 0 replies; 38+ messages in thread From: John Garry @ 2018-04-30 9:35 UTC (permalink / raw) To: Lee Jones Cc: Mika Westerberg, andriy.shevchenko, rjw, linux-acpi, lenb, lorenzo.pieralisi, linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang, Liguozhu (Kenneth) >>>> So we using the mfd_cell to match child devices using _HID. At a glance, I >>>> don't actually see other drivers to use mfd_cell_acpi_match.pnpid . >>>> >>>> Anyway we don't use static tables as we need to update the resources of the >>>> cell dynamically. However I do look at a driver like intel_quark_i2c_gpio.c, >>>> and this dynamically modifies the value of global mfd_cell array here: >>>> https://elixir.bootlin.com/linux/latest/source/drivers/mfd/intel_quark_i2c_gpio.c#L266 >>>> >>>> I know the cell array is only used at probe time, but this did not look to >>>> be good standard practice to me. >>> >>> Lots of drivers do this to supply dynamic data. If there is no other >>> sane way of providing such data, it's fine to do. Although each >>> situation should be dealt with on a case-by-case basis. >>> >> >> Hi Lee, >> >> Thanks for your input. >> >> I do see others drivers which use dynamic mem for the mfd_cells (like >> cros_ec_dev.c), so what we're doing in this driver already is not totally >> unchartered territory. But creating the MFD cells from the ACPI table could >> be ... > > Right. I don't normally like mixing platform data technologies (MFD, > ACPI and DT). I normally NACK patches which take information from > Device Tree and populate MFD cells with it. ACPI would be the same I > guess. Oh, well that is what we have in this driver. So what's the preferred approach? Just not use MFD model at all if ACPI/DT needs to be scanned for data to create the cells? Thanks, John > >> Anyway, I'll cc you in my next patchset and maybe you can kindly check it. >> > ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/2] ACPI / PNP: Don't add "enumeration_by_parent" devices 2018-04-30 9:35 ` John Garry (?) @ 2018-04-30 10:46 ` Lee Jones 2018-04-30 10:57 ` John Garry -1 siblings, 1 reply; 38+ messages in thread From: Lee Jones @ 2018-04-30 10:46 UTC (permalink / raw) To: John Garry Cc: Mika Westerberg, andriy.shevchenko, rjw, linux-acpi, lenb, lorenzo.pieralisi, linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang, Liguozhu (Kenneth) On Mon, 30 Apr 2018, John Garry wrote: > > > > > So we using the mfd_cell to match child devices using _HID. At a glance, I > > > > > don't actually see other drivers to use mfd_cell_acpi_match.pnpid . > > > > > > > > > > Anyway we don't use static tables as we need to update the resources of the > > > > > cell dynamically. However I do look at a driver like intel_quark_i2c_gpio.c, > > > > > and this dynamically modifies the value of global mfd_cell array here: > > > > > https://elixir.bootlin.com/linux/latest/source/drivers/mfd/intel_quark_i2c_gpio.c#L266 > > > > > > > > > > I know the cell array is only used at probe time, but this did not look to > > > > > be good standard practice to me. > > > > > > > > Lots of drivers do this to supply dynamic data. If there is no other > > > > sane way of providing such data, it's fine to do. Although each > > > > situation should be dealt with on a case-by-case basis. > > > > > > > > > > Hi Lee, > > > > > > Thanks for your input. > > > > > > I do see others drivers which use dynamic mem for the mfd_cells (like > > > cros_ec_dev.c), so what we're doing in this driver already is not totally > > > unchartered territory. But creating the MFD cells from the ACPI table could > > > be ... > > > > Right. I don't normally like mixing platform data technologies (MFD, > > ACPI and DT). I normally NACK patches which take information from > > Device Tree and populate MFD cells with it. ACPI would be the same I > > guess. > > Oh, well that is what we have in this driver. So what's the preferred > approach? Just not use MFD model at all if ACPI/DT needs to be scanned for > data to create the cells? I've just seen the driver - yuk! Why are you using the MFD API outside of MFD anyway? -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/2] ACPI / PNP: Don't add "enumeration_by_parent" devices 2018-04-30 10:46 ` Lee Jones @ 2018-04-30 10:57 ` John Garry 0 siblings, 0 replies; 38+ messages in thread From: John Garry @ 2018-04-30 10:57 UTC (permalink / raw) To: Lee Jones Cc: Mika Westerberg, andriy.shevchenko, rjw, linux-acpi, lenb, lorenzo.pieralisi, linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang, Liguozhu (Kenneth) On 30/04/2018 11:46, Lee Jones wrote: > On Mon, 30 Apr 2018, John Garry wrote: > >>>>>> So we using the mfd_cell to match child devices using _HID. At a glance, I >>>>>> don't actually see other drivers to use mfd_cell_acpi_match.pnpid . >>>>>> >>>>>> Anyway we don't use static tables as we need to update the resources of the >>>>>> cell dynamically. However I do look at a driver like intel_quark_i2c_gpio.c, >>>>>> and this dynamically modifies the value of global mfd_cell array here: >>>>>> https://elixir.bootlin.com/linux/latest/source/drivers/mfd/intel_quark_i2c_gpio.c#L266 >>>>>> >>>>>> I know the cell array is only used at probe time, but this did not look to >>>>>> be good standard practice to me. >>>>> >>>>> Lots of drivers do this to supply dynamic data. If there is no other >>>>> sane way of providing such data, it's fine to do. Although each >>>>> situation should be dealt with on a case-by-case basis. >>>>> >>>> >>>> Hi Lee, >>>> >>>> Thanks for your input. >>>> >>>> I do see others drivers which use dynamic mem for the mfd_cells (like >>>> cros_ec_dev.c), so what we're doing in this driver already is not totally >>>> unchartered territory. But creating the MFD cells from the ACPI table could >>>> be ... >>> >>> Right. I don't normally like mixing platform data technologies (MFD, >>> ACPI and DT). I normally NACK patches which take information from >>> Device Tree and populate MFD cells with it. ACPI would be the same I >>> guess. >> >> Oh, well that is what we have in this driver. So what's the preferred >> approach? Just not use MFD model at all if ACPI/DT needs to be scanned for >> data to create the cells? > > I've just seen the driver - yuk! > > Why are you using the MFD API outside of MFD anyway? Hi Lee, This goes back a bit. The original thread was here: https://lkml.org/lkml/2017/6/13/581 Essentially a method was required to model this host to support platform device children for ACPI FW, and this did the job. In hindsight I think that there was a misunderstanding in recommending MFD since the devices attached are not fixed - hence the dynamic part. Cheers, John > ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 1/2] ACPI / PNP: Don't add "enumeration_by_parent" devices @ 2018-04-30 10:57 ` John Garry 0 siblings, 0 replies; 38+ messages in thread From: John Garry @ 2018-04-30 10:57 UTC (permalink / raw) To: Lee Jones Cc: Mika Westerberg, andriy.shevchenko, rjw, linux-acpi, lenb, lorenzo.pieralisi, linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang, Liguozhu (Kenneth) On 30/04/2018 11:46, Lee Jones wrote: > On Mon, 30 Apr 2018, John Garry wrote: > >>>>>> So we using the mfd_cell to match child devices using _HID. At a glance, I >>>>>> don't actually see other drivers to use mfd_cell_acpi_match.pnpid . >>>>>> >>>>>> Anyway we don't use static tables as we need to update the resources of the >>>>>> cell dynamically. However I do look at a driver like intel_quark_i2c_gpio.c, >>>>>> and this dynamically modifies the value of global mfd_cell array here: >>>>>> https://elixir.bootlin.com/linux/latest/source/drivers/mfd/intel_quark_i2c_gpio.c#L266 >>>>>> >>>>>> I know the cell array is only used at probe time, but this did not look to >>>>>> be good standard practice to me. >>>>> >>>>> Lots of drivers do this to supply dynamic data. If there is no other >>>>> sane way of providing such data, it's fine to do. Although each >>>>> situation should be dealt with on a case-by-case basis. >>>>> >>>> >>>> Hi Lee, >>>> >>>> Thanks for your input. >>>> >>>> I do see others drivers which use dynamic mem for the mfd_cells (like >>>> cros_ec_dev.c), so what we're doing in this driver already is not totally >>>> unchartered territory. But creating the MFD cells from the ACPI table could >>>> be ... >>> >>> Right. I don't normally like mixing platform data technologies (MFD, >>> ACPI and DT). I normally NACK patches which take information from >>> Device Tree and populate MFD cells with it. ACPI would be the same I >>> guess. >> >> Oh, well that is what we have in this driver. So what's the preferred >> approach? Just not use MFD model at all if ACPI/DT needs to be scanned for >> data to create the cells? > > I've just seen the driver - yuk! > > Why are you using the MFD API outside of MFD anyway? Hi Lee, This goes back a bit. The original thread was here: https://lkml.org/lkml/2017/6/13/581 Essentially a method was required to model this host to support platform device children for ACPI FW, and this did the job. In hindsight I think that there was a misunderstanding in recommending MFD since the devices attached are not fixed - hence the dynamic part. Cheers, John > ^ permalink raw reply [flat|nested] 38+ messages in thread
* [RFC PATCH 2/2] HISI LPC: Add PNP device support 2018-04-20 10:07 ` John Garry @ 2018-04-20 10:07 ` John Garry -1 siblings, 0 replies; 38+ messages in thread From: John Garry @ 2018-04-20 10:07 UTC (permalink / raw) To: rjw, andriy.shevchenko, linux-acpi, lenb, mika.westerberg, lorenzo.pieralisi Cc: linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang, John Garry Currently the driver creates an per-ACPI device mfd_cell for child devices. This does not suit devices which are PNP-compatible, as we expect PNP-compatible devices to derive PNP devices. To add PNP device support, we continue to allow the PNP scan code to create the PNP device (which have the enumeration_by_parent flag set), but expect the PNP scan to defer adding the device to allow the host probe code to do this. In addition, no longer do we create an mfd_cell (platform_device) for PNP-compatible devices. We take this approach so that host probe code can translate the IO resources of the PNP device prior to adding the device. Signed-off-by: John Garry <john.garry@huawei.com> --- drivers/bus/hisi_lpc.c | 38 +++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c index 2d4611e..d228bc5 100644 --- a/drivers/bus/hisi_lpc.c +++ b/drivers/bus/hisi_lpc.c @@ -17,8 +17,11 @@ #include <linux/of_address.h> #include <linux/of_platform.h> #include <linux/pci.h> +#include <linux/pnp.h> #include <linux/slab.h> +#include "../pnp/base.h" + #define DRV_NAME "hisi-lpc" /* @@ -469,8 +472,11 @@ static int hisi_lpc_acpi_probe(struct device *hostdev) struct acpi_device *child; int size, ret, count = 0, cell_num = 0; - list_for_each_entry(child, &adev->children, node) + list_for_each_entry(child, &adev->children, node) { + if (acpi_is_pnp_device(child)) + continue; cell_num++; + } /* allocate the mfd cell and companion ACPI info, one per child */ size = sizeof(*mfd_cells) + sizeof(*hisi_lpc_mfd_cells); @@ -492,6 +498,9 @@ static int hisi_lpc_acpi_probe(struct device *hostdev) .pnpid = pnpid, }; + if (acpi_is_pnp_device(child)) + continue; + /* * For any instances of this host controller (Hip06 and Hip07 * are the only chipsets), we would not have multiple slaves @@ -523,6 +532,33 @@ static int hisi_lpc_acpi_probe(struct device *hostdev) return ret; } + list_for_each_entry(child, &adev->children, node) { + struct pnp_resource *pnp_res; + struct pnp_dev *pnp_dev; + int rc; + + if (!acpi_is_pnp_device(child)) + continue; + + pnp_dev = child->driver_data; + + /* + * Prior to adding the device, we need to translate the + * resources to logical PIO addresses. + */ + list_for_each_entry(pnp_res, &pnp_dev->resources, list) { + struct resource *res = &pnp_res->res; + + if (res->flags | IORESOURCE_IO) + hisi_lpc_acpi_xlat_io_res(child, adev, res); + } + rc = pnp_add_device(pnp_dev); + if (rc) { + put_device(&pnp_dev->dev); + return rc; + } + } + return 0; } -- 1.9.1 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* [RFC PATCH 2/2] HISI LPC: Add PNP device support @ 2018-04-20 10:07 ` John Garry 0 siblings, 0 replies; 38+ messages in thread From: John Garry @ 2018-04-20 10:07 UTC (permalink / raw) To: rjw, andriy.shevchenko, linux-acpi, lenb, mika.westerberg, lorenzo.pieralisi Cc: linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang, John Garry Currently the driver creates an per-ACPI device mfd_cell for child devices. This does not suit devices which are PNP-compatible, as we expect PNP-compatible devices to derive PNP devices. To add PNP device support, we continue to allow the PNP scan code to create the PNP device (which have the enumeration_by_parent flag set), but expect the PNP scan to defer adding the device to allow the host probe code to do this. In addition, no longer do we create an mfd_cell (platform_device) for PNP-compatible devices. We take this approach so that host probe code can translate the IO resources of the PNP device prior to adding the device. Signed-off-by: John Garry <john.garry@huawei.com> --- drivers/bus/hisi_lpc.c | 38 +++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c index 2d4611e..d228bc5 100644 --- a/drivers/bus/hisi_lpc.c +++ b/drivers/bus/hisi_lpc.c @@ -17,8 +17,11 @@ #include <linux/of_address.h> #include <linux/of_platform.h> #include <linux/pci.h> +#include <linux/pnp.h> #include <linux/slab.h> +#include "../pnp/base.h" + #define DRV_NAME "hisi-lpc" /* @@ -469,8 +472,11 @@ static int hisi_lpc_acpi_probe(struct device *hostdev) struct acpi_device *child; int size, ret, count = 0, cell_num = 0; - list_for_each_entry(child, &adev->children, node) + list_for_each_entry(child, &adev->children, node) { + if (acpi_is_pnp_device(child)) + continue; cell_num++; + } /* allocate the mfd cell and companion ACPI info, one per child */ size = sizeof(*mfd_cells) + sizeof(*hisi_lpc_mfd_cells); @@ -492,6 +498,9 @@ static int hisi_lpc_acpi_probe(struct device *hostdev) .pnpid = pnpid, }; + if (acpi_is_pnp_device(child)) + continue; + /* * For any instances of this host controller (Hip06 and Hip07 * are the only chipsets), we would not have multiple slaves @@ -523,6 +532,33 @@ static int hisi_lpc_acpi_probe(struct device *hostdev) return ret; } + list_for_each_entry(child, &adev->children, node) { + struct pnp_resource *pnp_res; + struct pnp_dev *pnp_dev; + int rc; + + if (!acpi_is_pnp_device(child)) + continue; + + pnp_dev = child->driver_data; + + /* + * Prior to adding the device, we need to translate the + * resources to logical PIO addresses. + */ + list_for_each_entry(pnp_res, &pnp_dev->resources, list) { + struct resource *res = &pnp_res->res; + + if (res->flags | IORESOURCE_IO) + hisi_lpc_acpi_xlat_io_res(child, adev, res); + } + rc = pnp_add_device(pnp_dev); + if (rc) { + put_device(&pnp_dev->dev); + return rc; + } + } + return 0; } -- 1.9.1 ^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 2/2] HISI LPC: Add PNP device support 2018-04-20 10:07 ` John Garry (?) @ 2018-04-20 12:50 ` Andy Shevchenko 2018-04-20 13:09 ` John Garry -1 siblings, 1 reply; 38+ messages in thread From: Andy Shevchenko @ 2018-04-20 12:50 UTC (permalink / raw) To: John Garry, rjw, linux-acpi, lenb, mika.westerberg, lorenzo.pieralisi Cc: linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang On Fri, 2018-04-20 at 18:07 +0800, John Garry wrote: > Currently the driver creates an per-ACPI device mfd_cell > for child devices. This does not suit devices which are > PNP-compatible, as we expect PNP-compatible devices to > derive PNP devices. > > To add PNP device support, we continue to allow the PNP > scan code to create the PNP device (which have the > enumeration_by_parent flag set), but expect the PNP > scan to defer adding the device to allow the host probe > code to do this. In addition, no longer do we create an > mfd_cell (platform_device) for PNP-compatible devices. > > We take this approach so that host probe code can > translate the IO resources of the PNP device prior > to adding the device. > + list_for_each_entry(child, &adev->children, node) { > + if (acpi_is_pnp_device(child)) > + continue; This is good candidate for a separate helper macro #define for_each_acpi_non_pnp_device(child, adev) \ ... (see, for example, for_each_pci_bridge() implementation as an example) > + list_for_each_entry(child, &adev->children, node) { > + if (!acpi_is_pnp_device(child)) > + continue; Ditto. > + /* > + * Prior to adding the device, we need to translate > the > + * resources to logical PIO addresses. > + */ > + list_for_each_entry(pnp_res, &pnp_dev->resources, > list) { > + struct resource *res = &pnp_res->res; > + > + if (res->flags | IORESOURCE_IO) What does this mean? > + hisi_lpc_acpi_xlat_io_res(child, > adev, res); > + } -- Andy Shevchenko <andriy.shevchenko@linux.intel.com> Intel Finland Oy ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 2/2] HISI LPC: Add PNP device support 2018-04-20 12:50 ` Andy Shevchenko @ 2018-04-20 13:09 ` John Garry 0 siblings, 0 replies; 38+ messages in thread From: John Garry @ 2018-04-20 13:09 UTC (permalink / raw) To: Andy Shevchenko, rjw, linux-acpi, lenb, mika.westerberg, lorenzo.pieralisi Cc: linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang On 20/04/2018 13:50, Andy Shevchenko wrote: > On Fri, 2018-04-20 at 18:07 +0800, John Garry wrote: >> Currently the driver creates an per-ACPI device mfd_cell >> for child devices. This does not suit devices which are >> PNP-compatible, as we expect PNP-compatible devices to >> derive PNP devices. >> >> To add PNP device support, we continue to allow the PNP >> scan code to create the PNP device (which have the >> enumeration_by_parent flag set), but expect the PNP >> scan to defer adding the device to allow the host probe >> code to do this. In addition, no longer do we create an >> mfd_cell (platform_device) for PNP-compatible devices. >> >> We take this approach so that host probe code can >> translate the IO resources of the PNP device prior >> to adding the device. > Hi Andy, Thanks for checking this. >> + list_for_each_entry(child, &adev->children, node) { >> + if (acpi_is_pnp_device(child)) >> + continue; > > This is good candidate for a separate helper macro > > #define for_each_acpi_non_pnp_device(child, adev) \ > ... Right, I did consider this, but was holding off refining until I get past RFC stage. In fact, we could also process each child entry in one loop, like this: list_for_each_entry(child, &adev->children, node) { if (acpi_is_pnp_device(child)) { /* Do PNP compatible device work */ } else { /* otherwise, make an MFD cell ... */ } > > (see, for example, for_each_pci_bridge() implementation as an example) > > >> + list_for_each_entry(child, &adev->children, node) { > >> + if (!acpi_is_pnp_device(child)) >> + continue; > > Ditto. > ok >> + /* >> + * Prior to adding the device, we need to translate >> the >> + * resources to logical PIO addresses. >> + */ >> + list_for_each_entry(pnp_res, &pnp_dev->resources, >> list) { >> + struct resource *res = &pnp_res->res; >> + > >> + if (res->flags | IORESOURCE_IO) > > What does this mean? Here we check the resource flag for each resource for this PNP device - for IO resources we must translate the resource value from the bus address to the logical PIO address. > >> + hisi_lpc_acpi_xlat_io_res(child, >> adev, res); >> + } > Regards, John ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 2/2] HISI LPC: Add PNP device support @ 2018-04-20 13:09 ` John Garry 0 siblings, 0 replies; 38+ messages in thread From: John Garry @ 2018-04-20 13:09 UTC (permalink / raw) To: Andy Shevchenko, rjw, linux-acpi, lenb, mika.westerberg, lorenzo.pieralisi Cc: linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang On 20/04/2018 13:50, Andy Shevchenko wrote: > On Fri, 2018-04-20 at 18:07 +0800, John Garry wrote: >> Currently the driver creates an per-ACPI device mfd_cell >> for child devices. This does not suit devices which are >> PNP-compatible, as we expect PNP-compatible devices to >> derive PNP devices. >> >> To add PNP device support, we continue to allow the PNP >> scan code to create the PNP device (which have the >> enumeration_by_parent flag set), but expect the PNP >> scan to defer adding the device to allow the host probe >> code to do this. In addition, no longer do we create an >> mfd_cell (platform_device) for PNP-compatible devices. >> >> We take this approach so that host probe code can >> translate the IO resources of the PNP device prior >> to adding the device. > Hi Andy, Thanks for checking this. >> + list_for_each_entry(child, &adev->children, node) { >> + if (acpi_is_pnp_device(child)) >> + continue; > > This is good candidate for a separate helper macro > > #define for_each_acpi_non_pnp_device(child, adev) \ > ... Right, I did consider this, but was holding off refining until I get past RFC stage. In fact, we could also process each child entry in one loop, like this: list_for_each_entry(child, &adev->children, node) { if (acpi_is_pnp_device(child)) { /* Do PNP compatible device work */ } else { /* otherwise, make an MFD cell ... */ } > > (see, for example, for_each_pci_bridge() implementation as an example) > > >> + list_for_each_entry(child, &adev->children, node) { > >> + if (!acpi_is_pnp_device(child)) >> + continue; > > Ditto. > ok >> + /* >> + * Prior to adding the device, we need to translate >> the >> + * resources to logical PIO addresses. >> + */ >> + list_for_each_entry(pnp_res, &pnp_dev->resources, >> list) { >> + struct resource *res = &pnp_res->res; >> + > >> + if (res->flags | IORESOURCE_IO) > > What does this mean? Here we check the resource flag for each resource for this PNP device - for IO resources we must translate the resource value from the bus address to the logical PIO address. > >> + hisi_lpc_acpi_xlat_io_res(child, >> adev, res); >> + } > Regards, John ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 2/2] HISI LPC: Add PNP device support 2018-04-20 13:09 ` John Garry (?) @ 2018-04-20 13:28 ` Andy Shevchenko 2018-04-20 13:32 ` John Garry -1 siblings, 1 reply; 38+ messages in thread From: Andy Shevchenko @ 2018-04-20 13:28 UTC (permalink / raw) To: John Garry, rjw, linux-acpi, lenb, mika.westerberg, lorenzo.pieralisi Cc: linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang On Fri, 2018-04-20 at 14:09 +0100, John Garry wrote: > On 20/04/2018 13:50, Andy Shevchenko wrote: > > On Fri, 2018-04-20 at 18:07 +0800, John Garry wrote: > > > + if (res->flags | IORESOURCE_IO) > > > > What does this mean? > > Here we check the resource flag for each resource for this PNP device > - > for IO resources we must translate the resource value from the bus > address to the logical PIO address. Please, re-read your code again. -- Andy Shevchenko <andriy.shevchenko@linux.intel.com> Intel Finland Oy ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 2/2] HISI LPC: Add PNP device support 2018-04-20 13:28 ` Andy Shevchenko @ 2018-04-20 13:32 ` John Garry 0 siblings, 0 replies; 38+ messages in thread From: John Garry @ 2018-04-20 13:32 UTC (permalink / raw) To: Andy Shevchenko, rjw, linux-acpi, lenb, mika.westerberg, lorenzo.pieralisi Cc: linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang On 20/04/2018 14:28, Andy Shevchenko wrote: > On Fri, 2018-04-20 at 14:09 +0100, John Garry wrote: >> On 20/04/2018 13:50, Andy Shevchenko wrote: >>> On Fri, 2018-04-20 at 18:07 +0800, John Garry wrote: > >>>> + if (res->flags | IORESOURCE_IO) >>> >>> What does this mean? >> >> Here we check the resource flag for each resource for this PNP device >> - >> for IO resources we must translate the resource value from the bus >> address to the logical PIO address. > > Please, re-read your code again. > Got it. But then again there is a helper for this, as Mika pointed out... :) Cheers ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 2/2] HISI LPC: Add PNP device support @ 2018-04-20 13:32 ` John Garry 0 siblings, 0 replies; 38+ messages in thread From: John Garry @ 2018-04-20 13:32 UTC (permalink / raw) To: Andy Shevchenko, rjw, linux-acpi, lenb, mika.westerberg, lorenzo.pieralisi Cc: linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang On 20/04/2018 14:28, Andy Shevchenko wrote: > On Fri, 2018-04-20 at 14:09 +0100, John Garry wrote: >> On 20/04/2018 13:50, Andy Shevchenko wrote: >>> On Fri, 2018-04-20 at 18:07 +0800, John Garry wrote: > >>>> + if (res->flags | IORESOURCE_IO) >>> >>> What does this mean? >> >> Here we check the resource flag for each resource for this PNP device >> - >> for IO resources we must translate the resource value from the bus >> address to the logical PIO address. > > Please, re-read your code again. > Got it. But then again there is a helper for this, as Mika pointed out... :) Cheers ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 2/2] HISI LPC: Add PNP device support 2018-04-20 10:07 ` John Garry (?) (?) @ 2018-04-20 13:12 ` Mika Westerberg 2018-04-20 13:36 ` John Garry -1 siblings, 1 reply; 38+ messages in thread From: Mika Westerberg @ 2018-04-20 13:12 UTC (permalink / raw) To: John Garry Cc: rjw, andriy.shevchenko, linux-acpi, lenb, lorenzo.pieralisi, linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang On Fri, Apr 20, 2018 at 06:07:26PM +0800, John Garry wrote: > Currently the driver creates an per-ACPI device mfd_cell > for child devices. This does not suit devices which are > PNP-compatible, as we expect PNP-compatible devices to > derive PNP devices. > > To add PNP device support, we continue to allow the PNP > scan code to create the PNP device (which have the > enumeration_by_parent flag set), but expect the PNP > scan to defer adding the device to allow the host probe > code to do this. In addition, no longer do we create an > mfd_cell (platform_device) for PNP-compatible devices. > > We take this approach so that host probe code can > translate the IO resources of the PNP device prior > to adding the device. > > Signed-off-by: John Garry <john.garry@huawei.com> > --- > drivers/bus/hisi_lpc.c | 38 +++++++++++++++++++++++++++++++++++++- > 1 file changed, 37 insertions(+), 1 deletion(-) > > diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c > index 2d4611e..d228bc5 100644 > --- a/drivers/bus/hisi_lpc.c > +++ b/drivers/bus/hisi_lpc.c > @@ -17,8 +17,11 @@ > #include <linux/of_address.h> > #include <linux/of_platform.h> > #include <linux/pci.h> > +#include <linux/pnp.h> > #include <linux/slab.h> > > +#include "../pnp/base.h" > + > #define DRV_NAME "hisi-lpc" > > /* > @@ -469,8 +472,11 @@ static int hisi_lpc_acpi_probe(struct device *hostdev) > struct acpi_device *child; > int size, ret, count = 0, cell_num = 0; > > - list_for_each_entry(child, &adev->children, node) > + list_for_each_entry(child, &adev->children, node) { > + if (acpi_is_pnp_device(child)) > + continue; > cell_num++; > + } > > /* allocate the mfd cell and companion ACPI info, one per child */ > size = sizeof(*mfd_cells) + sizeof(*hisi_lpc_mfd_cells); > @@ -492,6 +498,9 @@ static int hisi_lpc_acpi_probe(struct device *hostdev) > .pnpid = pnpid, > }; > > + if (acpi_is_pnp_device(child)) > + continue; > + > /* > * For any instances of this host controller (Hip06 and Hip07 > * are the only chipsets), we would not have multiple slaves > @@ -523,6 +532,33 @@ static int hisi_lpc_acpi_probe(struct device *hostdev) > return ret; > } > > + list_for_each_entry(child, &adev->children, node) { > + struct pnp_resource *pnp_res; > + struct pnp_dev *pnp_dev; > + int rc; > + > + if (!acpi_is_pnp_device(child)) > + continue; > + > + pnp_dev = child->driver_data; ...or better yet a PNP helper function that makes this more understandable. > + > + /* > + * Prior to adding the device, we need to translate the > + * resources to logical PIO addresses. > + */ > + list_for_each_entry(pnp_res, &pnp_dev->resources, list) { > + struct resource *res = &pnp_res->res; > + > + if (res->flags | IORESOURCE_IO) I think you should use if (resource_type(res) == IORESOURCE_IO) instead. > + hisi_lpc_acpi_xlat_io_res(child, adev, res); > + } > + rc = pnp_add_device(pnp_dev); > + if (rc) { > + put_device(&pnp_dev->dev); > + return rc; > + } > + } > + > return 0; > } > > -- > 1.9.1 ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 2/2] HISI LPC: Add PNP device support 2018-04-20 13:12 ` Mika Westerberg @ 2018-04-20 13:36 ` John Garry 0 siblings, 0 replies; 38+ messages in thread From: John Garry @ 2018-04-20 13:36 UTC (permalink / raw) To: Mika Westerberg Cc: rjw, andriy.shevchenko, linux-acpi, lenb, lorenzo.pieralisi, linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang Hi Mika, >> /* >> @@ -469,8 +472,11 @@ static int hisi_lpc_acpi_probe(struct device *hostdev) >> struct acpi_device *child; >> int size, ret, count = 0, cell_num = 0; >> >> - list_for_each_entry(child, &adev->children, node) >> + list_for_each_entry(child, &adev->children, node) { >> + if (acpi_is_pnp_device(child)) >> + continue; >> cell_num++; >> + } >> >> /* allocate the mfd cell and companion ACPI info, one per child */ >> size = sizeof(*mfd_cells) + sizeof(*hisi_lpc_mfd_cells); >> @@ -492,6 +498,9 @@ static int hisi_lpc_acpi_probe(struct device *hostdev) >> .pnpid = pnpid, >> }; >> >> + if (acpi_is_pnp_device(child)) >> + continue; >> + >> /* >> * For any instances of this host controller (Hip06 and Hip07 >> * are the only chipsets), we would not have multiple slaves >> @@ -523,6 +532,33 @@ static int hisi_lpc_acpi_probe(struct device *hostdev) >> return ret; >> } >> >> + list_for_each_entry(child, &adev->children, node) { >> + struct pnp_resource *pnp_res; >> + struct pnp_dev *pnp_dev; >> + int rc; >> + >> + if (!acpi_is_pnp_device(child)) >> + continue; >> + >> + pnp_dev = child->driver_data; > > ...or better yet a PNP helper function that makes this more > understandable. Sure, but I would not say the helper function would do the same, due to to (ab)use of driver_data element. As I mentioned in patch 1/2, I couldn't see a current method for the acpi_device to reference the PNP device. > >> + >> + /* >> + * Prior to adding the device, we need to translate the >> + * resources to logical PIO addresses. >> + */ >> + list_for_each_entry(pnp_res, &pnp_dev->resources, list) { >> + struct resource *res = &pnp_res->res; >> + >> + if (res->flags | IORESOURCE_IO) > > I think you should use > > if (resource_type(res) == IORESOURCE_IO) > > instead. Yes, since I don't know the difference between logical OR and logical AND :) > >> + hisi_lpc_acpi_xlat_io_res(child, adev, res); >> + } >> + rc = pnp_add_device(pnp_dev); >> + if (rc) { >> + put_device(&pnp_dev->dev); >> + return rc; >> + } >> + } >> + Cheers, John >> return 0; >> } >> >> -- >> 1.9.1 > > . > ^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [RFC PATCH 2/2] HISI LPC: Add PNP device support @ 2018-04-20 13:36 ` John Garry 0 siblings, 0 replies; 38+ messages in thread From: John Garry @ 2018-04-20 13:36 UTC (permalink / raw) To: Mika Westerberg Cc: rjw, andriy.shevchenko, linux-acpi, lenb, lorenzo.pieralisi, linux-kernel, arnd, graeme.gregory, helgaas, linuxarm, z.liuxinliang Hi Mika, >> /* >> @@ -469,8 +472,11 @@ static int hisi_lpc_acpi_probe(struct device *hostdev) >> struct acpi_device *child; >> int size, ret, count = 0, cell_num = 0; >> >> - list_for_each_entry(child, &adev->children, node) >> + list_for_each_entry(child, &adev->children, node) { >> + if (acpi_is_pnp_device(child)) >> + continue; >> cell_num++; >> + } >> >> /* allocate the mfd cell and companion ACPI info, one per child */ >> size = sizeof(*mfd_cells) + sizeof(*hisi_lpc_mfd_cells); >> @@ -492,6 +498,9 @@ static int hisi_lpc_acpi_probe(struct device *hostdev) >> .pnpid = pnpid, >> }; >> >> + if (acpi_is_pnp_device(child)) >> + continue; >> + >> /* >> * For any instances of this host controller (Hip06 and Hip07 >> * are the only chipsets), we would not have multiple slaves >> @@ -523,6 +532,33 @@ static int hisi_lpc_acpi_probe(struct device *hostdev) >> return ret; >> } >> >> + list_for_each_entry(child, &adev->children, node) { >> + struct pnp_resource *pnp_res; >> + struct pnp_dev *pnp_dev; >> + int rc; >> + >> + if (!acpi_is_pnp_device(child)) >> + continue; >> + >> + pnp_dev = child->driver_data; > > ...or better yet a PNP helper function that makes this more > understandable. Sure, but I would not say the helper function would do the same, due to to (ab)use of driver_data element. As I mentioned in patch 1/2, I couldn't see a current method for the acpi_device to reference the PNP device. > >> + >> + /* >> + * Prior to adding the device, we need to translate the >> + * resources to logical PIO addresses. >> + */ >> + list_for_each_entry(pnp_res, &pnp_dev->resources, list) { >> + struct resource *res = &pnp_res->res; >> + >> + if (res->flags | IORESOURCE_IO) > > I think you should use > > if (resource_type(res) == IORESOURCE_IO) > > instead. Yes, since I don't know the difference between logical OR and logical AND :) > >> + hisi_lpc_acpi_xlat_io_res(child, adev, res); >> + } >> + rc = pnp_add_device(pnp_dev); >> + if (rc) { >> + put_device(&pnp_dev->dev); >> + return rc; >> + } >> + } >> + Cheers, John >> return 0; >> } >> >> -- >> 1.9.1 > > . > ^ permalink raw reply [flat|nested] 38+ messages in thread
end of thread, other threads:[~2018-04-30 10:58 UTC | newest] Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-04-20 10:07 [RFC PATCH 0/2] HISI LPC: Add PNP device support John Garry 2018-04-20 10:07 ` John Garry 2018-04-20 10:07 ` [RFC PATCH 1/2] ACPI / PNP: Don't add "enumeration_by_parent" devices John Garry 2018-04-20 10:07 ` John Garry 2018-04-20 13:07 ` Mika Westerberg 2018-04-20 13:24 ` John Garry 2018-04-20 13:24 ` John Garry 2018-04-20 13:52 ` Mika Westerberg 2018-04-20 14:09 ` John Garry 2018-04-20 14:09 ` John Garry 2018-04-26 13:49 ` John Garry 2018-04-26 13:49 ` John Garry 2018-04-26 14:08 ` Mika Westerberg 2018-04-26 14:23 ` John Garry 2018-04-26 14:23 ` John Garry 2018-04-26 14:40 ` Mika Westerberg 2018-04-27 9:17 ` John Garry 2018-04-27 9:17 ` John Garry 2018-04-30 5:36 ` Lee Jones 2018-04-30 9:00 ` John Garry 2018-04-30 9:00 ` John Garry 2018-04-30 9:26 ` Lee Jones 2018-04-30 9:35 ` John Garry 2018-04-30 9:35 ` John Garry 2018-04-30 10:46 ` Lee Jones 2018-04-30 10:57 ` John Garry 2018-04-30 10:57 ` John Garry 2018-04-20 10:07 ` [RFC PATCH 2/2] HISI LPC: Add PNP device support John Garry 2018-04-20 10:07 ` John Garry 2018-04-20 12:50 ` Andy Shevchenko 2018-04-20 13:09 ` John Garry 2018-04-20 13:09 ` John Garry 2018-04-20 13:28 ` Andy Shevchenko 2018-04-20 13:32 ` John Garry 2018-04-20 13:32 ` John Garry 2018-04-20 13:12 ` Mika Westerberg 2018-04-20 13:36 ` John Garry 2018-04-20 13:36 ` John Garry
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.