[PATCH v2 1/4] bus: hisi_lpc: Don't dereference fwnode handle

[PATCH v2 1/4] bus: hisi_lpc: Don't dereference fwnode handle

[Andy Shevchenko]

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>
Acked-by: John Garry <john.garry@huawei.com>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
v2: added tags (John, Rafael), tweaked commit message (John)
 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.1

[PATCH v2 2/4] bus: hisi_lpc: Use devm_platform_ioremap_resource

[Andy Shevchenko]

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>
Acked-by: John Garry <john.garry@huawei.com>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
v2: added tags
 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.1

[PATCH v2 3/4] bus: hisi_lpc: Correct error code for timeout

[Andy Shevchenko]

The usual error code is -ETIMEDOUT, the currently used -ETIME is specific
for timers.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: John Garry <john.garry@huawei.com>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
v2: added tags (John, Rafael)
 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.1

[PATCH v2 4/4] bus: hisi_lpc: Don't guard ACPI IDs with ACPI_PTR()

[Andy Shevchenko]

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>
Acked-by: John Garry <john.garry@huawei.com>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
v2: added tags (John, Rafael), improved commit message (John)
 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.1

Re: [PATCH v2 1/4] bus: hisi_lpc: Don't dereference fwnode handle

[John Garry]

Andy, Thanks for this work. I am not sure if you are hoping that Rafael 
picks up this series also. JFYI, I would normally route any changes to 
this driver through the arm soc tree via xuwei5@huawei.com, but if we 
want to try that it may take an extra cycle now.

Thanks,
John

Re: [PATCH v2 1/4] bus: hisi_lpc: Don't dereference fwnode handle

[Andy Shevchenko]

On Mon, Jul 11, 2022 at 10:17 AM John Garry <john.garry@huawei.com> wrote:
>
> Andy, Thanks for this work. I am not sure if you are hoping that Rafael
> picks up this series also. JFYI, I would normally route any changes to
> this driver through the arm soc tree via xuwei5@huawei.com, but if we
> want to try that it may take an extra cycle now.

The series has been inspired by the recent work Rafael has done
regarding some ACPI API changes. It's not critical per se and can be
routed as the best for all maintainers.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/4] bus: hisi_lpc: Don't dereference fwnode handle

[John Garry]

On 11/07/2022 09:59, Andy Shevchenko wrote:
> On Mon, Jul 11, 2022 at 10:17 AM John Garry <john.garry@huawei.com> wrote:
>>
>> Andy, Thanks for this work. I am not sure if you are hoping that Rafael
>> picks up this series also. JFYI, I would normally route any changes to
>> this driver through the arm soc tree via xuwei5@huawei.com, but if we
>> want to try that it may take an extra cycle now.
> 
> The series has been inspired by the recent work Rafael has done
> regarding some ACPI API changes. It's not critical per se and can be
> routed as the best for all maintainers.
> 
> 

Hi Andy,

I'll pick these up to be sent upstream by xu wei through the arm-soc 
tree. I will also do 2x things:
a. Send separate PoC series factor out the ACPI platform code, which I 
assume you and Rafael will agree is not worth pursuing
b. Add a patch to convert this driver to use 
platform_device_register_full(). This obviously conflicts with a.

Thanks,
John

BTW, A copy letter helps in scenarios like this....

Re: [PATCH v2 1/4] bus: hisi_lpc: Don't dereference fwnode handle

[Andy Shevchenko]

On Mon, Aug 15, 2022 at 05:16:48PM +0100, John Garry wrote:
> On 11/07/2022 09:59, Andy Shevchenko wrote:
> > On Mon, Jul 11, 2022 at 10:17 AM John Garry <john.garry@huawei.com> wrote:
> > > 
> > > Andy, Thanks for this work. I am not sure if you are hoping that Rafael
> > > picks up this series also. JFYI, I would normally route any changes to
> > > this driver through the arm soc tree via xuwei5@huawei.com, but if we
> > > want to try that it may take an extra cycle now.
> > 
> > The series has been inspired by the recent work Rafael has done
> > regarding some ACPI API changes. It's not critical per se and can be
> > routed as the best for all maintainers.
> 
> I'll pick these up to be sent upstream by xu wei through the arm-soc tree.

Thanks!

...

> b. Add a patch to convert this driver to use
> platform_device_register_full(). This obviously conflicts with a.

This one can be done beforehand, because it doesn't change flow, correct?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/4] bus: hisi_lpc: Don't dereference fwnode handle

[John Garry]

On 23/08/2022 14:27, Andy Shevchenko wrote:
>> I'll pick these up to be sent upstream by xu wei through the arm-soc tree.
> Thanks!
> 
> ...
> 
>> b. Add a patch to convert this driver to use
>> platform_device_register_full(). This obviously conflicts with a.
> This one can be done beforehand, because it doesn't change flow, correct?

The general flow is the same. It may be previewed here:
https://github.com/hisilicon/kernel-dev/commits/private-topic-lpc-6.0-platform-full

Thanks,
John