[PATCH v2 RFC] drivers/usb/host/ehci-fsl: Fix interrupt setup in host mode.

[PATCH v2 RFC] drivers/usb/host/ehci-fsl: Fix interrupt setup in host mode.

[Darren Stevens]

In patch a1a2b7125e10 (Drop static setup of IRQ resource from DT
core) we stopped platform_get_resource() from returning the IRQ, as all
drivers were supposed to have switched to platform_get_irq()
Unfortunately the Freescale EHCI driver in host mode got missed. Fix
it. Also fix allocation of resources to work with current kernel.

Fixes: a1a2b7125e10 (Drop static setup of IRQ resource from DT core)
Reported-by Christian Zigotzky <chzigotzky@xenosoft.de>
Signed-off-by Darren Stevens <darren@stevens-zone.net>
---
 v2 - Fixed coding style, removed a couple of unneeded initializations,
      cc'd Layerscape maintainers.

Tested on AmigaOne X5000/20 and X5000/40 not sure if this is entirely
correct fix though. Contains code by Rob Herring (in fsl-mph-dr-of.c)

diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c
index 385be30..8bd258a 100644
--- a/drivers/usb/host/ehci-fsl.c
+++ b/drivers/usb/host/ehci-fsl.c
@@ -23,6 +23,7 @@
 #include <linux/platform_device.h>
 #include <linux/fsl_devices.h>
 #include <linux/of_platform.h>
+#include <linux/of_address.h>
 #include <linux/io.h>
 
 #include "ehci.h"
@@ -46,9 +47,10 @@ static struct hc_driver __read_mostly fsl_ehci_hc_driver;
  */
 static int fsl_ehci_drv_probe(struct platform_device *pdev)
 {
+	struct device_node *dn = pdev->dev.of_node;
 	struct fsl_usb2_platform_data *pdata;
 	struct usb_hcd *hcd;
-	struct resource *res;
+	struct resource res;
 	int irq;
 	int retval;
 	u32 tmp;
@@ -76,14 +78,9 @@ static int fsl_ehci_drv_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-	if (!res) {
-		dev_err(&pdev->dev,
-			"Found HC with no IRQ. Check %s setup!\n",
-			dev_name(&pdev->dev));
-		return -ENODEV;
-	}
-	irq = res->start;
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return irq;
 
 	hcd = __usb_create_hcd(&fsl_ehci_hc_driver, pdev->dev.parent,
 			       &pdev->dev, dev_name(&pdev->dev), NULL);
@@ -92,15 +89,18 @@ static int fsl_ehci_drv_probe(struct platform_device *pdev)
 		goto err1;
 	}
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	hcd->regs = devm_ioremap_resource(&pdev->dev, res);
+	tmp = of_address_to_resource(dn, 0, &res);
+	if (tmp)
+		return tmp;
+
+	hcd->regs = devm_ioremap_resource(&pdev->dev, &res);
 	if (IS_ERR(hcd->regs)) {
 		retval = PTR_ERR(hcd->regs);
 		goto err2;
 	}
 
-	hcd->rsrc_start = res->start;
-	hcd->rsrc_len = resource_size(res);
+	hcd->rsrc_start = res.start;
+	hcd->rsrc_len = resource_size(&res);
 
 	pdata->regs = hcd->regs;
 
diff --git a/drivers/usb/host/fsl-mph-dr-of.c b/drivers/usb/host/fsl-mph-dr-of.c
index 44a7e58..766e4ab 100644
--- a/drivers/usb/host/fsl-mph-dr-of.c
+++ b/drivers/usb/host/fsl-mph-dr-of.c
@@ -80,8 +80,6 @@ static struct platform_device *fsl_usb2_device_register(
 					const char *name, int id)
 {
 	struct platform_device *pdev;
-	const struct resource *res = ofdev->resource;
-	unsigned int num = ofdev->num_resources;
 	int retval;
 
 	pdev = platform_device_alloc(name, id);
@@ -106,11 +104,8 @@ static struct platform_device *fsl_usb2_device_register(
 	if (retval)
 		goto error;
 
-	if (num) {
-		retval = platform_device_add_resources(pdev, res, num);
-		if (retval)
-			goto error;
-	}
+	pdev->dev.of_node = ofdev->dev.of_node;
+	pdev->dev.of_node_reused = true;
 
 	retval = platform_device_add(pdev);
 	if (retval)

Re: [PATCH v2 RFC] drivers/usb/host/ehci-fsl: Fix interrupt setup in host mode.

[Sergei Shtylyov]

Hello!

On 6/25/22 11:41 PM, Darren Stevens wrote:

> In patch a1a2b7125e10 (Drop static setup of IRQ resource from DT
> core) we stopped platform_get_resource() from returning the IRQ, as all

In commit a1a2b7125e10 ("Drop static setup of IRQ resource from DT core")

> drivers were supposed to have switched to platform_get_irq()
> Unfortunately the Freescale EHCI driver in host mode got missed. Fix
> it. Also fix allocation of resources to work with current kernel.

   The basic rule (especially for the fixes) is "do one thing per patch".

> Fixes: a1a2b7125e10 (Drop static setup of IRQ resource from DT core)
> Reported-by Christian Zigotzky <chzigotzky@xenosoft.de>
> Signed-off-by Darren Stevens <darren@stevens-zone.net>
> ---
>  v2 - Fixed coding style, removed a couple of unneeded initializations,
>       cc'd Layerscape maintainers.
> 
> Tested on AmigaOne X5000/20 and X5000/40 not sure if this is entirely
> correct fix though. Contains code by Rob Herring (in fsl-mph-dr-of.c)
> 
> diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c
> index 385be30..8bd258a 100644
> --- a/drivers/usb/host/ehci-fsl.c
> +++ b/drivers/usb/host/ehci-fsl.c
[...]
> @@ -92,15 +89,18 @@ static int fsl_ehci_drv_probe(struct platform_device *pdev)
>  		goto err1;
>  	}
>  
> -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	hcd->regs = devm_ioremap_resource(&pdev->dev, res);
> +	tmp = of_address_to_resource(dn, 0, &res);

   Hm, why? What does this fix?

[...]

MBR, Sergey

Re: drivers/usb/host/ehci-fsl: Fix interrupt setup in host mode.

[Darren Stevens]

Hello Sergei

On 26/06/2022, Sergei Shtylyov wrote:
> Hello!
>
> On 6/25/22 11:41 PM, Darren Stevens wrote:
>
>> In patch a1a2b7125e10 (Drop static setup of IRQ resource from DT
>> core) we stopped platform_get_resource() from returning the IRQ, as all
>
> In commit a1a2b7125e10 ("Drop static setup of IRQ resource from DT core")
>
>> drivers were supposed to have switched to platform_get_irq()
>> Unfortunately the Freescale EHCI driver in host mode got missed. Fix
>> it. Also fix allocation of resources to work with current kernel.
>
>    The basic rule (especially for the fixes) is "do one thing per patch".

I thought I'd done that, this is the minimum amount of changes that fix what changed in the specified commit. 

> [...]
>> @@ -92,15 +89,18 @@ static int fsl_ehci_drv_probe(struct platform_device *pdev)
>>          goto err1;
>>      }
>>  
>> -    res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> -    hcd->regs = devm_ioremap_resource(&pdev->dev, res);
>> +    tmp = of_address_to_resource(dn, 0, &res);
>
>    Hm, why? What does this fix?

With baseline the mouse and keyboard on our machines don't work - dmesg reports no interrupt. Fixing the interrupt detection throws a 'invalid resoure' error instead (No idea why), which these lines fix. Both problems disappear if we revert the 'fixes' patch.

Hmmm, perhaps title shoud be 'fix resource detection in host mode'?

Regards
Darren

Re: drivers/usb/host/ehci-fsl: Fix interrupt setup in host mode.

[Rob Herring]

On Sun, Jun 26, 2022 at 2:03 PM Darren Stevens <darren@stevens-zone.net> wrote:
>
> Hello Sergei
>
> On 26/06/2022, Sergei Shtylyov wrote:
> > Hello!
> >
> > On 6/25/22 11:41 PM, Darren Stevens wrote:
> >
> >> In patch a1a2b7125e10 (Drop static setup of IRQ resource from DT
> >> core) we stopped platform_get_resource() from returning the IRQ, as all
> >
> > In commit a1a2b7125e10 ("Drop static setup of IRQ resource from DT core")
> >
> >> drivers were supposed to have switched to platform_get_irq()
> >> Unfortunately the Freescale EHCI driver in host mode got missed. Fix
> >> it. Also fix allocation of resources to work with current kernel.
> >
> >    The basic rule (especially for the fixes) is "do one thing per patch".
>
> I thought I'd done that, this is the minimum amount of changes that fix what changed in the specified commit.
>
> > [...]
> >> @@ -92,15 +89,18 @@ static int fsl_ehci_drv_probe(struct platform_device *pdev)
> >>          goto err1;
> >>      }
> >>
> >> -    res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >> -    hcd->regs = devm_ioremap_resource(&pdev->dev, res);
> >> +    tmp = of_address_to_resource(dn, 0, &res);
> >
> >    Hm, why? What does this fix?
>
> With baseline the mouse and keyboard on our machines don't work - dmesg reports no interrupt. Fixing the interrupt detection throws a 'invalid resoure' error instead (No idea why), which these lines fix. Both problems disappear if we revert the 'fixes' patch.
>

I see the problem. You need to keep the
platform_device_add_resources() call in fsl-mph-dr-of.c so that the
memory resource is copied from the parent to the child device.

Rob