* [PATCH v2 1/5] bus: hisi_lpc: Don't dereference fwnode handle
2022-09-02 7:47 [PATCH v2 0/5] Misc hisi_lpc changes John Garry
@ 2022-09-02 7:47 ` John Garry
2022-09-02 7:47 ` [PATCH v2 2/5] bus: hisi_lpc: Use devm_platform_ioremap_resource John Garry
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: John Garry @ 2022-09-02 7:47 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 related [flat|nested] 9+ messages in thread* [PATCH v2 2/5] bus: hisi_lpc: Use devm_platform_ioremap_resource
2022-09-02 7:47 [PATCH v2 0/5] Misc hisi_lpc changes John Garry
2022-09-02 7:47 ` [PATCH v2 1/5] bus: hisi_lpc: Don't dereference fwnode handle John Garry
@ 2022-09-02 7:47 ` John Garry
2022-09-02 7:47 ` [PATCH v2 3/5] bus: hisi_lpc: Correct error code for timeout John Garry
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: John Garry @ 2022-09-02 7:47 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 related [flat|nested] 9+ messages in thread* [PATCH v2 3/5] bus: hisi_lpc: Correct error code for timeout
2022-09-02 7:47 [PATCH v2 0/5] Misc hisi_lpc changes John Garry
2022-09-02 7:47 ` [PATCH v2 1/5] bus: hisi_lpc: Don't dereference fwnode handle John Garry
2022-09-02 7:47 ` [PATCH v2 2/5] bus: hisi_lpc: Use devm_platform_ioremap_resource John Garry
@ 2022-09-02 7:47 ` John Garry
2022-09-02 7:47 ` [PATCH v2 4/5] bus: hisi_lpc: Don't guard ACPI IDs with ACPI_PTR() John Garry
2022-09-02 7:47 ` [PATCH v2 5/5] bus: hisi_lpc: Use platform_device_register_full() John Garry
4 siblings, 0 replies; 9+ messages in thread
From: John Garry @ 2022-09-02 7:47 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 related [flat|nested] 9+ messages in thread
* [PATCH v2 4/5] bus: hisi_lpc: Don't guard ACPI IDs with ACPI_PTR()
2022-09-02 7:47 [PATCH v2 0/5] Misc hisi_lpc changes John Garry
` (2 preceding siblings ...)
2022-09-02 7:47 ` [PATCH v2 3/5] bus: hisi_lpc: Correct error code for timeout John Garry
@ 2022-09-02 7:47 ` John Garry
2022-09-02 7:47 ` [PATCH v2 5/5] bus: hisi_lpc: Use platform_device_register_full() John Garry
4 siblings, 0 replies; 9+ messages in thread
From: John Garry @ 2022-09-02 7:47 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 related [flat|nested] 9+ messages in thread* [PATCH v2 5/5] bus: hisi_lpc: Use platform_device_register_full()
2022-09-02 7:47 [PATCH v2 0/5] Misc hisi_lpc changes John Garry
` (3 preceding siblings ...)
2022-09-02 7:47 ` [PATCH v2 4/5] bus: hisi_lpc: Don't guard ACPI IDs with ACPI_PTR() John Garry
@ 2022-09-02 7:47 ` John Garry
2022-09-02 9:18 ` Andy Shevchenko
4 siblings, 1 reply; 9+ messages in thread
From: John Garry @ 2022-09-02 7:47 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 | 64 ++++++++++++++++++++----------------------
1 file changed, 30 insertions(+), 34 deletions(-)
diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c
index 74f4448bff9d..3555a6857214 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,45 @@ 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 +555,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 related [flat|nested] 9+ messages in thread* Re: [PATCH v2 5/5] bus: hisi_lpc: Use platform_device_register_full()
2022-09-02 7:47 ` [PATCH v2 5/5] bus: hisi_lpc: Use platform_device_register_full() John Garry
@ 2022-09-02 9:18 ` Andy Shevchenko
2022-09-02 9:21 ` Andy Shevchenko
0 siblings, 1 reply; 9+ messages in thread
From: Andy Shevchenko @ 2022-09-02 9:18 UTC (permalink / raw)
To: John Garry
Cc: xuwei (O),
Andy Shevchenko, Linuxarm, Rafael J. Wysocki,
Linux Kernel Mailing List, SoC Team, Yang Yingliang
On Fri, Sep 2, 2022 at 11:10 AM 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.
Now statistics plays for you and not against.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: John Garry <john.garry@huawei.com>
> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> drivers/bus/hisi_lpc.c | 64 ++++++++++++++++++++----------------------
> 1 file changed, 30 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/bus/hisi_lpc.c b/drivers/bus/hisi_lpc.c
> index 74f4448bff9d..3555a6857214 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,45 @@ 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 +555,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
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2 5/5] bus: hisi_lpc: Use platform_device_register_full()
2022-09-02 9:18 ` Andy Shevchenko
@ 2022-09-02 9:21 ` Andy Shevchenko
2022-09-02 9:35 ` John Garry
0 siblings, 1 reply; 9+ messages in thread
From: Andy Shevchenko @ 2022-09-02 9:21 UTC (permalink / raw)
To: John Garry
Cc: xuwei (O),
Andy Shevchenko, Linuxarm, Rafael J. Wysocki,
Linux Kernel Mailing List, SoC Team, Yang Yingliang
On Fri, Sep 2, 2022 at 12:18 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Fri, Sep 2, 2022 at 11:10 AM 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.
>
> Now statistics plays for you and not against.
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Too fast to give a tag, see below (keep tag if addressing it)
...
> > - pdev = platform_device_alloc(cell->name, PLATFORM_DEVID_AUTO);
> > + pdev = platform_device_register_full(cell->pdevinfo);
> > if (!pdev)
> > return -ENOMEM;
As per kernel doc:
* Returns &struct platform_device pointer on success, or ERR_PTR() on error.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 5/5] bus: hisi_lpc: Use platform_device_register_full()
2022-09-02 9:21 ` Andy Shevchenko
@ 2022-09-02 9:35 ` John Garry
0 siblings, 0 replies; 9+ messages in thread
From: John Garry @ 2022-09-02 9:35 UTC (permalink / raw)
To: Andy Shevchenko
Cc: xuwei (O),
Andy Shevchenko, Linuxarm, Rafael J. Wysocki,
Linux Kernel Mailing List, SoC Team, Yang Yingliang
On 02/09/2022 10:21, Andy Shevchenko wrote:
>> Reviewed-by: Andy Shevchenko<andy.shevchenko@gmail.com>
> Too fast to give a tag, see below (keep tag if addressing it)
thanks
>
> ...
>
>>> - pdev = platform_device_alloc(cell->name, PLATFORM_DEVID_AUTO);
>>> + pdev = platform_device_register_full(cell->pdevinfo);
>>> if (!pdev)
>>> return -ENOMEM;
> As per kernel doc:
> * Returns &struct platform_device pointer on success, or ERR_PTR() on error.
ah, right, I thought that I got that correct....
John
^ permalink raw reply [flat|nested] 9+ messages in thread