Re: [PATCH] HISI LPC: Don't fail probe for unrecognised child devices

From: John Garry
Date: Mon Jan 07 2019 - 05:32:07 EST

On 04/01/2019 22:25, Olof Johansson wrote:
On Thu, Jan 03, 2019 at 07:57:02PM +0800, John Garry wrote:
Currently for ACPI-based FW we fail the probe for an unrecognised child

However, there is FW in the field with LPC child devices having fake HIDs,
namely "IPI0002", which was an IPMI device invented to support the
initial out-of-tree LPC host driver, different from the final mainline

To provide compatibility support for these dodgy FWs, just discard the
unrecognised HIDs instead of failing the probe altogether.

Tested-by: Zengruan Ye <yezengruan@xxxxxxxxxx>
Signed-off-by: John Garry <john.garry@xxxxxxxxxx>

diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c
index d5f8545..19d7b6f 100644
--- a/drivers/bus/hisi_lpc.c
+++ b/drivers/bus/hisi_lpc.c
@@ -522,10 +522,9 @@ static int hisi_lpc_acpi_probe(struct device *hostdev)

if (!found) {
- "could not find cell for child device (%s)\n",
+ "could not find cell for child device (%s), discarding\n",
- ret = -ENODEV;
- goto fail;
+ continue;

Hi Olof,

This driver is the equivalent of a board file. Wasn't ACPI supposed to
spare us from these platform device tables? It even has hardcoded clock
information in it. :(

For sure, we should not need a look-up table like this. The background is complex. I think that if you check these thread+patches then you may get a better idea of why we require the table:

To summarize:
For child devices, we use indirect-PIO method to access. In this, we need to create a new device with updated resources (see call to hisi_lpc_acpi_set_io_res() call and drivers/acpi/scan.c:acpi_is_indirect_io_slave()) for logical PIO space.

One of the child devices is a 8250-compatible UART. For ACPI, we should use the 8250 PNP driver for this device, i.e. use PNP0501. However PNP code does not support this indirect-PIO child probe. So we use the generic 8250 platform device driver instead. Hence the look-up table.

We saw this as the least disruptive method to support this legacy host controller.

As for the clock info in the driver, we're just setting some driver wait times depending on fixed clock information. So not configuring clocks or the like.

Also, we were told that there'll be expectations for users to update
their ACPI tables if they're incompatible our out of date. Can that be done
here as well?

If you're referring to this comment "To provide compatibility support for these dodgy FWs", we're saying that if the FW has IPI0001 and IPI0002 (this being invalid) child devices, then we can handle it by just discarding IPI0002.