On 05/07/2022 14:54, Rafael J. Wysocki wrote:
So we could factor out by dividing acpi_create_platform_device() into 2x
parts: resource get and then platform dev create. But that does not seem
wise as we have 2x parts which don't make sense on their own. Or else
pass a fixup callback into acpi_create_platform_device(). Any other
ideas if we want to go this way?
Personally, I would do the cleanup that can be done without
refactoring the library function as the first step, just to reduce the
amount of changes made in one go if nothing else.
Next, I'd look at introducing something like
acpi_create_platform_device_ops(struct acpi_device *adev, const struct
property_entry *properties, const struct *platform_device_create_ops
*ops);
where ops would be a set of callbacks to invoke as a matter of customization.
Then, acpi_create_platform_device() can be defined as a wrapper around
the above.
.
JFYI, I'm trying out this change and it is looking quite disruptive. The
problems are specifically related to the LPC UART support. Firstly, it
looks like we require this patch (which was never applied):
https://lore.kernel.org/linux-acpi/1524218846-169934-2-git-send-email-john.garry@xxxxxxxxxx/#t
Secondly this code in the hisi lpc driver causes an issue:
static int hisi_lpc_acpi_add_child(struct acpi_device *child, void *data)
{
const char *hid = acpi_device_hid(child);
struct device *hostdev = data;
const struct hisi_lpc_acpi_cell *cell;
struct platform_device *pdev;
const struct resource *res;
bool found = false;
int num_res;
int ret;
ret = hisi_lpc_acpi_set_io_res(child, hostdev, &res, &num_res);
if (ret) {
dev_warn(hostdev, "set resource fail (%d)\n", ret);
return ret;
}
cell = (struct hisi_lpc_acpi_cell []){
...
/* 8250-compatible uart */
{
.hid = "HISI1031",
.name = "serial8250",
.pdata = (struct plat_serial8250_port []) {
{
*** .iobase = res->start,
.uartclk = 1843200,
.iotype = UPIO_PORT,
.flags = UPF_BOOT_AUTOCONF,
},
{}
},
.pdata_size = 2 *
sizeof(struct plat_serial8250_port),
},
{}
};
...
pdev = platform_device_alloc(cell->name, PLATFORM_DEVID_AUTO);
At ***, above, we need to set the platform data plat_serial8250_port
iobase at the translated address, but this can only be done after we
read and translate the resources, which is now all done in the acpi
platform code - so we have an ordering problem.
Anyway, I'll try to get it working and then send out the patches. We may
decide it's just not worth it.
Thanks,
John
