| Message ID | 1546516622-46343-1-git-send-email-john.garry@huawei.com |
|---|---|
| State | Accepted |
| Commit | 705c0ee8d4a64b072e324f8daa8767e92560a892 |
| Headers | show |
| Series | HISI LPC: Don't fail probe for unrecognised child devices | expand |
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 > HID. > > 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 > version. > > To provide compatibility support for these dodgy FWs, just discard the > unrecognised HIDs instead of failing the probe altogether. > > Tested-by: Zengruan Ye <yezengruan@huawei.com> > Signed-off-by: John Garry <john.garry@huawei.com> > > 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) { > dev_warn(hostdev, > - "could not find cell for child device (%s)\n", > + "could not find cell for child device (%s), discarding\n", > hid); > - ret = -ENODEV; > - goto fail; > + continue; > } 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. :( 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? -Olof
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 >> HID. >> >> 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 >> version. >> >> To provide compatibility support for these dodgy FWs, just discard the >> unrecognised HIDs instead of failing the probe altogether. >> >> Tested-by: Zengruan Ye <yezengruan@huawei.com> >> Signed-off-by: John Garry <john.garry@huawei.com> >> >> 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) { >> dev_warn(hostdev, >> - "could not find cell for child device (%s)\n", >> + "could not find cell for child device (%s), discarding\n", >> hid); >> - 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: https://lkml.org/lkml/2018/4/20/278 https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/acpi/scan.c?h=v5.0-rc1&id=dfda4492322ed0a1eb9c4d4715c4b90c9af57352 https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/bus/hisi_lpc.c?h=next-20190107&id=e0aa1563f8945d9b8f472426d100bed190a4308f https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/bus/hisi_lpc.c?h=next-20190107&id=adf3457b4ce6940885be3e5ee832c6949fba4166 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. Thanks, John > > > -Olof > >
Hi John, On 1/3/2019 11:57 AM, John Garry wrote: > Currently for ACPI-based FW we fail the probe for an unrecognised child > HID. > > 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 > version. > > To provide compatibility support for these dodgy FWs, just discard the > unrecognised HIDs instead of failing the probe altogether. > > Tested-by: Zengruan Ye <yezengruan@huawei.com> > Signed-off-by: John Garry <john.garry@huawei.com> Updated the subject as "bus: hisi_lpc: xxx" to follow the style and applied to the hisilicon tree. Thanks! Best Regards, Wei > > 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) { > dev_warn(hostdev, > - "could not find cell for child device (%s)\n", > + "could not find cell for child device (%s), discarding\n", > hid); > - ret = -ENODEV; > - goto fail; > + continue; > } > > pdev = platform_device_alloc(cell->name, PLATFORM_DEVID_AUTO); >
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) { dev_warn(hostdev, - "could not find cell for child device (%s)\n", + "could not find cell for child device (%s), discarding\n", hid); - ret = -ENODEV; - goto fail; + continue; } pdev = platform_device_alloc(cell->name, PLATFORM_DEVID_AUTO);