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

From: John Garry
Date: Mon Feb 26 2018 - 06:56:54 EST


Why you can't do properly in ACPI?

No answer here either.

Sorry, but with this level of communication it's no go for the
series.


Sorry if my answers did not tell you want you want to know.

My point was that the 8250_pnp driver would be used for a pnp_device,
but we are creating a platform device for this UART slave so would
require a platform device driver, that which 8250_dw.c is. But I will
check on pnp device support.


Hi Andy,

Perhaps it's not visible, though below is a description of the drivers
we have:

8250_dw - OF/ACPI driver for Synopsys DW (+ DW DMA)
8250_lpss - PCI driver for Synopsys DW (+ DW DMA)
8250_of - generic 8250 compatible driver for OF
8250_pci - generic 8250 compatible driver for PCI
8250_pnp - generic 8250 compatible driver for ACPI

8250_* (except core parts) - custom glue drivers per some IPs

By description you gave your driver fits 8250_pnp if ACPI tables crafted
properly.

Share the ACPI excerpt and we can discuss further how to improve them.


For a bit of background, MFD support was discussed here initially:
https://lkml.org/lkml/2017/6/13/796

Here is the ACPI table:
Scope(_SB) {
Device (LPC0) {
Name (_HID, "HISI0191") // HiSi LPC
Name (_CRS, ResourceTemplate () {
Memory32Fixed (ReadWrite, 0xa01b0000, 0x1000)
})
}

Device (LPC0.CON0) {
Name (_HID, "HISI1031")
// Name (_CID, "PNP0501") // cannot support PNP
Name (LORS, ResourceTemplate() {
QWordIO (
ResourceConsumer,
MinNotFixed, // _MIF
MaxNotFixed, // _MAF
PosDecode,
EntireRange,
0x0, // _GRA
0x2F8, // _MIN
0x3fff, // _MAX
0x0, // _TRA
0x08, // _LEN
, ,
IO02


The latest framework changes and host driver patchset are here:
https://lkml.org/lkml/2018/2/19/465

Here is how we probe the children:
static int hisi_lpc_acpi_set_io_res(struct device *child,
struct device *hostdev,
const struct resource **res,
int *num_res)
{

[ ... In this part we just get the child resources ]


/* translate the I/O resources */
for (i = 0; i < count; i++) {
int ret;

if (!(resources[i].flags & IORESOURCE_IO))
continue;
ret = hisi_lpc_acpi_xlat_io_res(adev, host, &resources[i]);
if (ret) {
dev_err(child, "translate IO range failed(%d)\n", ret);
return ret;
}
}
*res = resources;
*num_res = count;

return 0;
}

static int hisi_lpc_acpi_probe(struct device *hostdev)
{
struct acpi_device *adev = ACPI_COMPANION(hostdev);
struct hisi_lpc_mfd_cell *hisi_lpc_mfd_cells;
struct mfd_cell *mfd_cells;
struct acpi_device *child;
int size, ret, count = 0, cell_num = 0;

list_for_each_entry(child, &adev->children, node)
cell_num++;

/* allocate the mfd cell and companion acpi info, one per child */
size = sizeof(*mfd_cells) + sizeof(*hisi_lpc_mfd_cells);
mfd_cells = devm_kcalloc(hostdev, cell_num, size, GFP_KERNEL);
if (!mfd_cells)
return -ENOMEM;

hisi_lpc_mfd_cells = (struct hisi_lpc_mfd_cell *)
&mfd_cells[cell_num];
/* Only consider the children of the host */
list_for_each_entry(child, &adev->children, node) {
struct mfd_cell *mfd_cell = &mfd_cells[count];
struct hisi_lpc_mfd_cell *hisi_lpc_mfd_cell =
&hisi_lpc_mfd_cells[count];
struct mfd_cell_acpi_match *acpi_match =
&hisi_lpc_mfd_cell->acpi_match;
char *name = hisi_lpc_mfd_cell[count].name;
char *pnpid = hisi_lpc_mfd_cell[count].pnpid;
struct mfd_cell_acpi_match match = {
.pnpid = pnpid,
};

snprintf(name, MFD_CHILD_NAME_LEN, MFD_CHILD_NAME_PREFIX"%s",
acpi_device_hid(child));
snprintf(pnpid, ACPI_ID_LEN, "%s", acpi_device_hid(child));

memcpy(acpi_match, &match, sizeof(*acpi_match));
mfd_cell->name = name;
mfd_cell->acpi_match = acpi_match;

ret = hisi_lpc_acpi_set_io_res(&child->dev, &adev->dev,
&mfd_cell->resources,
&mfd_cell->num_resources);
if (ret) {
dev_warn(&child->dev, "set resource fail(%d)\n", ret);
return ret;
}
count++;
}

ret = mfd_add_devices(hostdev, PLATFORM_DEVID_NONE,
mfd_cells, cell_num, NULL, 0, NULL);
if (ret) {
dev_err(hostdev, "failed to add mfd cells (%d)\n", ret);
return ret;
}

return 0;
}

As you know, this is not accepted upstream yet, but I really hope I'm close. Hence the RFC tag for the UART patchset.

Please let me know if you require more details.

Thanks,
John