* [PATCH 1/5] bus: hisi_lpc: Don't dereference fwnode handle
2022-09-01 11:14 [PATCH 0/5] Misc hisi_lpc changes John Garry
@ 2022-09-01 11:14 ` John Garry
2022-09-01 11:14 ` [PATCH 2/5] bus: hisi_lpc: Use devm_platform_ioremap_resource John Garry
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: John Garry @ 2022-09-01 11:14 UTC (permalink / raw)
To: xuwei5
Cc: andriy.shevchenko, linuxarm, rafael.j.wysocki, linux-kernel, soc,
yangyingliang, John Garry
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Use dev_fwnode() and acpi_fwnode_handle() instead of dereferencing
an fwnode handle directly, which is a better coding practice.
While at it, reuse fwnode instead of ACPI_COMPANION().
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: John Garry <john.garry@huawei.com>
---
drivers/bus/hisi_lpc.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c
index 2e564803e786..6d432a07cbba 100644
--- a/drivers/bus/hisi_lpc.c
+++ b/drivers/bus/hisi_lpc.c
@@ -347,7 +347,7 @@ static int hisi_lpc_acpi_xlat_io_res(struct acpi_device *adev,
unsigned long sys_port;
resource_size_t len = resource_size(res);
- sys_port = logic_pio_trans_hwaddr(&host->fwnode, res->start, len);
+ sys_port = logic_pio_trans_hwaddr(acpi_fwnode_handle(host), res->start, len);
if (sys_port == ~0UL)
return -EFAULT;
@@ -615,7 +615,6 @@ static void hisi_lpc_acpi_remove(struct device *hostdev)
static int hisi_lpc_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
- struct acpi_device *acpi_device = ACPI_COMPANION(dev);
struct logic_pio_hwaddr *range;
struct hisi_lpc_dev *lpcdev;
resource_size_t io_end;
@@ -637,7 +636,7 @@ static int hisi_lpc_probe(struct platform_device *pdev)
if (!range)
return -ENOMEM;
- range->fwnode = dev->fwnode;
+ range->fwnode = dev_fwnode(dev);
range->flags = LOGIC_PIO_INDIRECT;
range->size = PIO_INDIRECT_SIZE;
range->hostdata = lpcdev;
@@ -651,7 +650,7 @@ static int hisi_lpc_probe(struct platform_device *pdev)
}
/* register the LPC host PIO resources */
- if (acpi_device)
+ if (is_acpi_device_node(range->fwnode))
ret = hisi_lpc_acpi_probe(dev);
else
ret = of_platform_populate(dev->of_node, NULL, NULL, dev);
@@ -672,11 +671,10 @@ static int hisi_lpc_probe(struct platform_device *pdev)
static int hisi_lpc_remove(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
- struct acpi_device *acpi_device = ACPI_COMPANION(dev);
struct hisi_lpc_dev *lpcdev = dev_get_drvdata(dev);
struct logic_pio_hwaddr *range = lpcdev->io_host;
- if (acpi_device)
+ if (is_acpi_device_node(range->fwnode))
hisi_lpc_acpi_remove(dev);
else
of_platform_depopulate(dev);
--
2.35.3
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 2/5] bus: hisi_lpc: Use devm_platform_ioremap_resource
2022-09-01 11:14 [PATCH 0/5] Misc hisi_lpc changes John Garry
2022-09-01 11:14 ` [PATCH 1/5] bus: hisi_lpc: Don't dereference fwnode handle John Garry
@ 2022-09-01 11:14 ` John Garry
2022-09-01 11:14 ` [PATCH 3/5] bus: hisi_lpc: Correct error code for timeout John Garry
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: John Garry @ 2022-09-01 11:14 UTC (permalink / raw)
To: xuwei5
Cc: andriy.shevchenko, linuxarm, rafael.j.wysocki, linux-kernel, soc,
yangyingliang, John Garry
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
The struct resource is not used for anything else, so we can simplify
the code a bit by using the helper function.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: John Garry <john.garry@huawei.com>
---
drivers/bus/hisi_lpc.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c
index 6d432a07cbba..03d4d96ff794 100644
--- a/drivers/bus/hisi_lpc.c
+++ b/drivers/bus/hisi_lpc.c
@@ -618,7 +618,6 @@ static int hisi_lpc_probe(struct platform_device *pdev)
struct logic_pio_hwaddr *range;
struct hisi_lpc_dev *lpcdev;
resource_size_t io_end;
- struct resource *res;
int ret;
lpcdev = devm_kzalloc(dev, sizeof(*lpcdev), GFP_KERNEL);
@@ -627,8 +626,7 @@ static int hisi_lpc_probe(struct platform_device *pdev)
spin_lock_init(&lpcdev->cycle_lock);
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- lpcdev->membase = devm_ioremap_resource(dev, res);
+ lpcdev->membase = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(lpcdev->membase))
return PTR_ERR(lpcdev->membase);
--
2.35.3
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 3/5] bus: hisi_lpc: Correct error code for timeout
2022-09-01 11:14 [PATCH 0/5] Misc hisi_lpc changes John Garry
2022-09-01 11:14 ` [PATCH 1/5] bus: hisi_lpc: Don't dereference fwnode handle John Garry
2022-09-01 11:14 ` [PATCH 2/5] bus: hisi_lpc: Use devm_platform_ioremap_resource John Garry
@ 2022-09-01 11:14 ` John Garry
2022-09-01 11:14 ` [PATCH 4/5] bus: hisi_lpc: Don't guard ACPI IDs with ACPI_PTR() John Garry
2022-09-01 11:14 ` [PATCH 5/5] bus: hisi_lpc: Use platform_device_register_full() John Garry
4 siblings, 0 replies; 8+ messages in thread
From: John Garry @ 2022-09-01 11:14 UTC (permalink / raw)
To: xuwei5
Cc: andriy.shevchenko, linuxarm, rafael.j.wysocki, linux-kernel, soc,
yangyingliang, John Garry
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
The usual error code is -ETIMEDOUT, the currently used -ETIME is specific
for timers.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: John Garry <john.garry@huawei.com>
---
drivers/bus/hisi_lpc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c
index 03d4d96ff794..a6513a571d7b 100644
--- a/drivers/bus/hisi_lpc.c
+++ b/drivers/bus/hisi_lpc.c
@@ -85,7 +85,7 @@ static int wait_lpc_idle(void __iomem *mbase, unsigned int waitcnt)
ndelay(LPC_NSEC_PERWAIT);
} while (--waitcnt);
- return -ETIME;
+ return -ETIMEDOUT;
}
/*
--
2.35.3
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 4/5] bus: hisi_lpc: Don't guard ACPI IDs with ACPI_PTR()
2022-09-01 11:14 [PATCH 0/5] Misc hisi_lpc changes John Garry
` (2 preceding siblings ...)
2022-09-01 11:14 ` [PATCH 3/5] bus: hisi_lpc: Correct error code for timeout John Garry
@ 2022-09-01 11:14 ` John Garry
2022-09-01 11:14 ` [PATCH 5/5] bus: hisi_lpc: Use platform_device_register_full() John Garry
4 siblings, 0 replies; 8+ messages in thread
From: John Garry @ 2022-09-01 11:14 UTC (permalink / raw)
To: xuwei5
Cc: andriy.shevchenko, linuxarm, rafael.j.wysocki, linux-kernel, soc,
yangyingliang, John Garry
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
The OF ID table is not guarded, and the ACPI table does not needs it either.
The IDs do not depend on the configuration. Hence drop ACPI_PTR() from the
code and move ID table closer to its user.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: John Garry <john.garry@huawei.com>
---
drivers/bus/hisi_lpc.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c
index a6513a571d7b..74f4448bff9d 100644
--- a/drivers/bus/hisi_lpc.c
+++ b/drivers/bus/hisi_lpc.c
@@ -589,11 +589,6 @@ static int hisi_lpc_acpi_probe(struct device *hostdev)
return ret;
}
-
-static const struct acpi_device_id hisi_lpc_acpi_match[] = {
- {"HISI0191"},
- {}
-};
#else
static int hisi_lpc_acpi_probe(struct device *dev)
{
@@ -688,11 +683,16 @@ static const struct of_device_id hisi_lpc_of_match[] = {
{}
};
+static const struct acpi_device_id hisi_lpc_acpi_match[] = {
+ {"HISI0191"},
+ {}
+};
+
static struct platform_driver hisi_lpc_driver = {
.driver = {
.name = DRV_NAME,
.of_match_table = hisi_lpc_of_match,
- .acpi_match_table = ACPI_PTR(hisi_lpc_acpi_match),
+ .acpi_match_table = hisi_lpc_acpi_match,
},
.probe = hisi_lpc_probe,
.remove = hisi_lpc_remove,
--
2.35.3
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 5/5] bus: hisi_lpc: Use platform_device_register_full()
2022-09-01 11:14 [PATCH 0/5] Misc hisi_lpc changes John Garry
` (3 preceding siblings ...)
2022-09-01 11:14 ` [PATCH 4/5] bus: hisi_lpc: Don't guard ACPI IDs with ACPI_PTR() John Garry
@ 2022-09-01 11:14 ` John Garry
2022-09-01 17:47 ` Andy Shevchenko
4 siblings, 1 reply; 8+ messages in thread
From: John Garry @ 2022-09-01 11:14 UTC (permalink / raw)
To: xuwei5
Cc: andriy.shevchenko, linuxarm, rafael.j.wysocki, linux-kernel, soc,
yangyingliang, John Garry
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(-)
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;
}
/*
--
2.35.3
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 5/5] bus: hisi_lpc: Use platform_device_register_full()
2022-09-01 11:14 ` [PATCH 5/5] bus: hisi_lpc: Use platform_device_register_full() John Garry
@ 2022-09-01 17:47 ` Andy Shevchenko
2022-09-02 6:46 ` John Garry
0 siblings, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2022-09-01 17:47 UTC (permalink / raw)
To: John Garry
Cc: xuwei (O),
Andy Shevchenko, Linuxarm, Rafael J. Wysocki,
Linux Kernel Mailing List, SoC Team, Yang Yingliang
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.
> },
> },
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 5/5] bus: hisi_lpc: Use platform_device_register_full()
2022-09-01 17:47 ` Andy Shevchenko
@ 2022-09-02 6:46 ` John Garry
0 siblings, 0 replies; 8+ messages in thread
From: John Garry @ 2022-09-02 6:46 UTC (permalink / raw)
To: Andy Shevchenko
Cc: xuwei (O),
Andy Shevchenko, Linuxarm, Rafael J. Wysocki,
Linux Kernel Mailing List, SoC Team, Yang Yingliang
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
^ permalink raw reply [flat|nested] 8+ messages in thread