| Message ID | 1662030871-182688-6-git-send-email-john.garry@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Misc hisi_lpc changes | expand |
On Thu, Sep 1, 2022 at 2:37 PM John Garry <john.garry@huawei.com> wrote: > > The code to create the child platform device is essentially the same as > what platform_device_register_full() does, so change over to use > that same function to reduce duplication. Thanks! > drivers/bus/hisi_lpc.c | 70 ++++++++++++++++++++++-------------------- > 1 file changed, 36 insertions(+), 34 deletions(-) Looking at the statistics I thought it was a scary "improvement", but... ... > /* ipmi */ > { > .hid = "IPI0001", > - .name = "hisi-lpc-ipmi", > + .pdevinfo = (struct platform_device_info []) { > + { > + .parent = hostdev, > + .fwnode = acpi_fwnode_handle(child), > + > + .name = "hisi-lpc-ipmi", > + .id = PLATFORM_DEVID_AUTO, > + > + .res = res, > + .num_res = num_res, > + }, ...here and in the other cases you have a lot of blank lines, can we just get rid of them? > + }, > }, > /* 8250-compatible uart */ > { > .hid = "HISI1031", > - .name = "serial8250", > - .pdata = (struct plat_serial8250_port []) { > + .pdevinfo = (struct platform_device_info []) { > { > - .iobase = res->start, > - .uartclk = 1843200, > - .iotype = UPIO_PORT, > - .flags = UPF_BOOT_AUTOCONF, > + .parent = hostdev, > + .fwnode = acpi_fwnode_handle(child), > + > + .name = "serial8250", > + .id = PLATFORM_DEVID_AUTO, > + > + .res = res, > + .num_res = num_res, > + > + .data = (struct plat_serial8250_port []) { > + { > + .iobase = res->start, > + .uartclk = 1843200, > + .iotype = UPIO_PORT, > + .flags = UPF_BOOT_AUTOCONF, > + }, > + {} > + }, > + .size_data = 2 * > + sizeof(struct plat_serial8250_port), I believe this can be one line. > }, > },
On 01/09/2022 18:47, Andy Shevchenko wrote: >> + .name = "hisi-lpc-ipmi", >> + .id = PLATFORM_DEVID_AUTO, >> + >> + .res = res, >> + .num_res = num_res, >> + }, > ...here and in the other cases you have a lot of blank lines, can we > just get rid of them? ok > >> + }, >> }, >> /* 8250-compatible uart */ >> { >> .hid = "HISI1031", >> - .name = "serial8250", >> - .pdata = (struct plat_serial8250_port []) { >> + .pdevinfo = (struct platform_device_info []) { >> { >> - .iobase = res->start, >> - .uartclk = 1843200, >> - .iotype = UPIO_PORT, >> - .flags = UPF_BOOT_AUTOCONF, >> + .parent = hostdev, >> + .fwnode = acpi_fwnode_handle(child), >> + >> + .name = "serial8250", >> + .id = PLATFORM_DEVID_AUTO, >> + >> + .res = res, >> + .num_res = num_res, >> + >> + .data = (struct plat_serial8250_port []) { >> + { >> + .iobase = res->start, >> + .uartclk = 1843200, >> + .iotype = UPIO_PORT, >> + .flags = UPF_BOOT_AUTOCONF, >> + }, >> + {} >> + }, >> + .size_data = 2 * >> + sizeof(struct plat_serial8250_port), > I believe this can be one line. I think so, as we're exceeding 80 char width anyway. > >> }, Thanks, John
diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c index 74f4448bff9d..2203ee57eaaf 100644 --- a/drivers/bus/hisi_lpc.c +++ b/drivers/bus/hisi_lpc.c @@ -472,9 +472,7 @@ static int hisi_lpc_acpi_clear_enumerated(struct acpi_device *adev, void *not_us struct hisi_lpc_acpi_cell { const char *hid; - const char *name; - void *pdata; - size_t pdata_size; + const struct platform_device_info *pdevinfo; }; static void hisi_lpc_acpi_remove(struct device *hostdev) @@ -505,28 +503,51 @@ static int hisi_lpc_acpi_add_child(struct acpi_device *child, void *data) /* ipmi */ { .hid = "IPI0001", - .name = "hisi-lpc-ipmi", + .pdevinfo = (struct platform_device_info []) { + { + .parent = hostdev, + .fwnode = acpi_fwnode_handle(child), + + .name = "hisi-lpc-ipmi", + .id = PLATFORM_DEVID_AUTO, + + .res = res, + .num_res = num_res, + }, + }, }, /* 8250-compatible uart */ { .hid = "HISI1031", - .name = "serial8250", - .pdata = (struct plat_serial8250_port []) { + .pdevinfo = (struct platform_device_info []) { { - .iobase = res->start, - .uartclk = 1843200, - .iotype = UPIO_PORT, - .flags = UPF_BOOT_AUTOCONF, + .parent = hostdev, + .fwnode = acpi_fwnode_handle(child), + + .name = "serial8250", + .id = PLATFORM_DEVID_AUTO, + + .res = res, + .num_res = num_res, + + .data = (struct plat_serial8250_port []) { + { + .iobase = res->start, + .uartclk = 1843200, + .iotype = UPIO_PORT, + .flags = UPF_BOOT_AUTOCONF, + }, + {} + }, + .size_data = 2 * + sizeof(struct plat_serial8250_port), }, - {} }, - .pdata_size = 2 * - sizeof(struct plat_serial8250_port), }, {} }; - for (; cell && cell->name; cell++) { + for (; cell && cell->hid; cell++) { if (!strcmp(cell->hid, hid)) { found = true; break; @@ -540,31 +561,12 @@ static int hisi_lpc_acpi_add_child(struct acpi_device *child, void *data) return 0; } - pdev = platform_device_alloc(cell->name, PLATFORM_DEVID_AUTO); + pdev = platform_device_register_full(cell->pdevinfo); if (!pdev) return -ENOMEM; - pdev->dev.parent = hostdev; - ACPI_COMPANION_SET(&pdev->dev, child); - - ret = platform_device_add_resources(pdev, res, num_res); - if (ret) - goto fail; - - ret = platform_device_add_data(pdev, cell->pdata, cell->pdata_size); - if (ret) - goto fail; - - ret = platform_device_add(pdev); - if (ret) - goto fail; - acpi_device_set_enumerated(child); return 0; - -fail: - platform_device_put(pdev); - return ret; } /*
The code to create the child platform device is essentially the same as what platform_device_register_full() does, so change over to use that same function to reduce duplication. Signed-off-by: John Garry <john.garry@huawei.com> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/bus/hisi_lpc.c | 70 ++++++++++++++++++++++-------------------- 1 file changed, 36 insertions(+), 34 deletions(-)