| Message ID | 20220621230941.381f9791@Cyrus.lan (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RFC] drivers/usb/ehci-fsl: Fix interrupt setup in host mode. | expand |
On Tue, Jun 21, 2022 at 11:09:41PM +0100, Darren Stevens wrote: > In patch a1a2b7125e1079 (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:a1a2b7125e1079 (Drop static setup of IRQ resource from DT core) Nit, please put a space after the :. Also not that many characters are needed, as you can see in our documentation, this is the proper format: Fixes: a1a2b7125e10 ("of/platform: 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> > --- > 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..d0bf7fb 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,10 @@ 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 = platform_get_irq(pdev, 0); > + if (irq < 0) { > + return irq; > } Did you run checkpatch on this? Coding style is not correct :( thanks, greg k-h
Darren Stevens <darren@stevens-zone.net> writes: > In patch a1a2b7125e1079 (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:a1a2b7125e1079 (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> > --- > 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) It looks like this driver is used on some arm/arm64 boards: $ git grep -l fsl-usb2-dr arch/arm*/boot/dts arch/arm/boot/dts/ls1021a.dtsi arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi Which is for the "Layerscape-1012A family SoC". Have we heard of any bug reports from users of those boards? Is it wired up differently or otherwise immune to the problem? I've added the Layerscape maintainers to Cc. cheers > 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)
On Tue, Jun 21, 2022 at 4:09 PM Darren Stevens <darren@stevens-zone.net> wrote: > > In patch a1a2b7125e1079 (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:a1a2b7125e1079 (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> > --- > 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..d0bf7fb 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,10 @@ static int fsl_ehci_drv_probe(struct > platform_device *pdev) return -ENODEV; > } > > - res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); Ah, this was the part I was missing... > - if (!res) { > - dev_err(&pdev->dev, > - "Found HC with no IRQ. Check %s setup!\n", > - dev_name(&pdev->dev)); > - return -ENODEV; > + irq = platform_get_irq(pdev, 0); > + if (irq < 0) { > + return irq; > } > - irq = res->start; > > hcd = __usb_create_hcd(&fsl_ehci_hc_driver, pdev->dev.parent, > &pdev->dev, dev_name(&pdev->dev), NULL); > @@ -92,15 +90,21 @@ 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); > + platform_set_drvdata(pdev, hcd); > + pdev->dev.platform_data = pdata; > + > + tmp = of_address_to_resource(dn, 0, &res); There's no need to change this. platform_get_resource() still works for IORESOURCE_MEM. Plus, drivers shouldn't use of_address_to_resource(). > + 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)
diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c index 385be30..d0bf7fb 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,10 @@ 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 = platform_get_irq(pdev, 0); + if (irq < 0) { + return irq; } - irq = res->start; hcd = __usb_create_hcd(&fsl_ehci_hc_driver, pdev->dev.parent, &pdev->dev, dev_name(&pdev->dev), NULL); @@ -92,15 +90,21 @@ 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); + platform_set_drvdata(pdev, hcd); + pdev->dev.platform_data = pdata; + + 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)