Hi Felipe, Thank you for the quick review.
On Monday 06 October 2014 11:06:27 Felipe Balbi wrote: > On Mon, Oct 06, 2014 at 06:55:00PM +0300, Laurent Pinchart wrote: > > Move core device initialization to a central location in order to share > > it with the device mode implementation. > > > > Signed-off-by: Laurent Pinchart <[email protected]> > > --- > > > > drivers/usb/host/Makefile | 2 +- > > drivers/usb/host/isp1760-core.c | 90 ++++++++++++++++++++++++++++++++++++ > > drivers/usb/host/isp1760-core.h | 37 +++++++++++++++++ > > drivers/usb/host/isp1760-hcd.c | 73 ++++++++------------------------- > > drivers/usb/host/isp1760-hcd.h | 8 ++-- > > drivers/usb/host/isp1760-if.c | 2 +- > > 6 files changed, 150 insertions(+), 62 deletions(-) > > create mode 100644 drivers/usb/host/isp1760-core.c > > create mode 100644 drivers/usb/host/isp1760-core.h > > > > diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile > > index 144c038..ecba12c 100644 > > --- a/drivers/usb/host/Makefile > > +++ b/drivers/usb/host/Makefile > > @@ -5,7 +5,7 @@ > > > > # tell define_trace.h where to find the xhci trace header > > CFLAGS_xhci-trace.o := -I$(src) > > > > -isp1760-y := isp1760-hcd.o isp1760-if.o > > +isp1760-y := isp1760-core.o isp1760-hcd.o isp1760-if.o > > > > fhci-y := fhci-hcd.o fhci-hub.o fhci-q.o > > fhci-y += fhci-mem.o fhci-tds.o fhci-sched.o > > > > diff --git a/drivers/usb/host/isp1760-core.c > > b/drivers/usb/host/isp1760-core.c new file mode 100644 > > index 0000000..28f8290 > > --- /dev/null > > +++ b/drivers/usb/host/isp1760-core.c > > @@ -0,0 +1,90 @@ > > +/* > > + * Driver for the NXP ISP1760 chip > > + * > > + * Copyright 2014 Laurent Pinchart > > + * Copyright 2007 Sebastian Siewior > > + * > > + * Contacts: > > + * Sebastian Siewior <[email protected]> > > + * Laurent Pinchart <[email protected]> > > + * > > + * This program is free software; you can redistribute it and/or > > + * modify it under the terms of the GNU General Public License > > + * version 2 as published by the Free Software Foundation. > > + */ > > + > > +#include <linux/gpio.h> > > +#include <linux/io.h> > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > +#include <linux/slab.h> > > +#include <linux/usb.h> > > + > > +#include "isp1760-core.h" > > +#include "isp1760-hcd.h" > > + > > +int isp1760_register(struct resource *mem, int irq, unsigned long > > irqflags, + int rst_gpio, struct device *dev, unsigned int > > devflags) > > +{ > > + struct isp1760_device *isp; > > + int ret; > > + > > + if (usb_disabled()) > > + return -ENODEV; > > + > > + /* prevent usb-core allocating DMA pages */ > > + dev->dma_mask = NULL; > > + > > + isp = kzalloc(sizeof(*isp), GFP_KERNEL); > > + if (!isp) > > + return -ENOMEM; > > + > > + isp->rst_gpio = rst_gpio; > > + > > + isp->mem_start = mem->start; > > + isp->mem_size = resource_size(mem); > > + > > + isp->regs = ioremap(isp->mem_start, isp->mem_size); > > + if (!isp->regs) { > > + ret = -EIO; > > + goto error; > > + } > > where did request_mem_region() go ? It's still in the glue code. > > + isp->hcd.rst_gpio = rst_gpio; > > + ret = isp1760_hcd_register(&isp->hcd, isp->regs, mem, irq, irqflags, > > + dev, devflags); > > + if (ret < 0) > > + goto error; > > + > > + dev_set_drvdata(dev, isp); > > + > > + return 0; > > + > > +error: > > + if (isp->regs) > > + iounmap(isp->regs); > > + > > + kfree(isp); > > + > > + return ret; > > +} > > + > > +void isp1760_unregister(struct device *dev) > > +{ > > + struct isp1760_device *isp = dev_get_drvdata(dev); > > + > > + release_mem_region(isp->mem_start, isp->mem_size); > > only after iounmap() Sure, will fix that. I'll actually fix it in patch 02/17 and fix the rebase conflict here. > > + isp1760_hcd_unregister(&isp->hcd); > > + > > + iounmap(isp->regs); > > + > > + if (gpio_is_valid(isp->rst_gpio)) > > + gpio_free(isp->rst_gpio); > > + > > + kfree(isp); > > +} > > + > > +MODULE_DESCRIPTION("Driver for the ISP1760 USB-controller from NXP"); > > +MODULE_AUTHOR("Sebastian Siewior <[email protected]>"); > > +MODULE_LICENSE("GPL v2"); > > diff --git a/drivers/usb/host/isp1760-core.h > > b/drivers/usb/host/isp1760-core.h new file mode 100644 > > index 0000000..8bd997c > > --- /dev/null > > +++ b/drivers/usb/host/isp1760-core.h > > @@ -0,0 +1,37 @@ > > +/* > > + * Driver for the NXP ISP1760 chip > > + * > > + * Copyright 2014 Laurent Pinchart > > + * Copyright 2007 Sebastian Siewior > > + * > > + * Contacts: > > + * Sebastian Siewior <[email protected]> > > + * Laurent Pinchart <[email protected]> > > + * > > + * This program is free software; you can redistribute it and/or > > + * modify it under the terms of the GNU General Public License > > + * version 2 as published by the Free Software Foundation. > > + */ > > + > > +#ifndef _ISP1760_CORE_H_ > > +#define _ISP1760_CORE_H_ > > + > > +#include <linux/ioport.h> > > + > > +#include "isp1760-hcd.h" > > + > > +struct isp1760_device { > > + resource_size_t mem_start; > > + resource_size_t mem_size; > > + > > + void __iomem *regs; > > + int rst_gpio; > > + > > + struct isp1760_hcd hcd; > > +}; > > + > > +int isp1760_register(struct resource *mem, int irq, unsigned long > > irqflags, + int rst_gpio, struct device *dev, unsigned int > > devflags); > > +void isp1760_unregister(struct device *dev); > > + > > +#endif > > diff --git a/drivers/usb/host/isp1760-hcd.c > > b/drivers/usb/host/isp1760-hcd.c index 9c8cd37..f903ad9 100644 > > --- a/drivers/usb/host/isp1760-hcd.c > > +++ b/drivers/usb/host/isp1760-hcd.c > > @@ -2237,81 +2237,42 @@ void isp1760_deinit_kmem_cache(void) > > > > kmem_cache_destroy(urb_listitem_cachep); > > > > } > > > > -int isp1760_register(struct resource *mem, int irq, unsigned long > > irqflags, - int rst_gpio, struct device *dev, unsigned int > > devflags) > > +int isp1760_hcd_register(struct isp1760_hcd *priv, void __iomem *regs, > > + struct resource *mem, int irq, unsigned long irqflags, > > + struct device *dev, unsigned int devflags) > > > > { > > > > - struct usb_hcd *hcd = NULL; > > - struct isp1760_hcd *priv; > > + struct usb_hcd *hcd; > > > > int ret; > > > > - if (usb_disabled()) > > - return -ENODEV; > > - > > - priv = kzalloc(sizeof(*priv), GFP_KERNEL); > > - if (!priv) > > - return -ENOMEM; > > - > > - /* prevent usb-core allocating DMA pages */ > > - dev->dma_mask = NULL; > > - > > > > hcd = usb_create_hcd(&isp1760_hc_driver, dev, dev_name(dev)); > > > > - if (!hcd) { > > - ret = -ENOMEM; > > - goto err_put; > > - } > > + if (!hcd) > > + return -ENOMEM; > > > > - priv->hcd = hcd; > > > > *(struct isp1760_hcd **)hcd->hcd_priv = priv; > > > > + priv->hcd = hcd; > > > > priv->devflags = devflags; > > > > - priv->rst_gpio = rst_gpio; > > + > > > > init_memory(priv); > > > > - hcd->regs = ioremap(mem->start, resource_size(mem)); > > - if (!hcd->regs) { > > - ret = -EIO; > > - goto err_put; > > - } > > > > hcd->irq = irq; > > > > + hcd->regs = regs; > > > > hcd->rsrc_start = mem->start; > > hcd->rsrc_len = resource_size(mem); > > > > ret = usb_add_hcd(hcd, irq, irqflags); > > > > - if (ret) > > - goto err_unmap; > > - device_wakeup_enable(hcd->self.controller); > > + if (ret) { > > + usb_put_hcd(hcd); > > + return ret; > > + } > > > > - dev_set_drvdata(dev, priv); > > + device_wakeup_enable(hcd->self.controller); > > > > return 0; > > > > - > > -err_unmap: > > - iounmap(hcd->regs); > > - > > -err_put: > > - usb_put_hcd(hcd); > > - kfree(priv); > > - > > - return ret; > > > > } > > > > -void isp1760_unregister(struct device *dev) > > +void isp1760_hcd_unregister(struct isp1760_hcd *priv) > > > > { > > > > - struct isp1760_hcd *priv = dev_get_drvdata(dev); > > - struct usb_hcd *hcd = priv->hcd; > > - > > - release_mem_region(hcd->rsrc_start, hcd->rsrc_len); > > - > > - usb_remove_hcd(hcd); > > - iounmap(hcd->regs); > > - usb_put_hcd(hcd); > > - > > - if (gpio_is_valid(priv->rst_gpio)) > > - gpio_free(priv->rst_gpio); > > - > > - kfree(priv); > > + usb_remove_hcd(priv->hcd); > > + usb_put_hcd(priv->hcd); > > > > } > > > > - > > -MODULE_DESCRIPTION("Driver for the ISP1760 USB-controller from NXP"); > > -MODULE_AUTHOR("Sebastian Siewior <[email protected]>"); > > -MODULE_LICENSE("GPL v2"); > > diff --git a/drivers/usb/host/isp1760-hcd.h > > b/drivers/usb/host/isp1760-hcd.h index 3ece078..51996a2 100644 > > --- a/drivers/usb/host/isp1760-hcd.h > > +++ b/drivers/usb/host/isp1760-hcd.h > > @@ -84,10 +84,10 @@ struct isp1760_hcd { > > > > int rst_gpio; > > > > }; > > > > -/* exports for if */ > > -int isp1760_register(struct resource *mem, int irq, unsigned long > > irqflags, - int rst_gpio, struct device *dev, unsigned int > > devflags); > > -void isp1760_unregister(struct device *dev); > > +int isp1760_hcd_register(struct isp1760_hcd *priv, void __iomem *regs, > > + struct resource *mem, int irq, unsigned long irqflags, > > + struct device *dev, unsigned int devflags); > > +void isp1760_hcd_unregister(struct isp1760_hcd *priv); > > > > int isp1760_init_kmem_once(void); > > void isp1760_deinit_kmem_cache(void); > > > > diff --git a/drivers/usb/host/isp1760-if.c b/drivers/usb/host/isp1760-if.c > > index 2880604..16db1be 100644 > > --- a/drivers/usb/host/isp1760-if.c > > +++ b/drivers/usb/host/isp1760-if.c > > @@ -16,7 +16,7 @@ > > > > #include <linux/usb/isp1760.h> > > #include <linux/usb/hcd.h> > > > > -#include "isp1760-hcd.h" > > +#include "isp1760-core.h" > > > > #include "isp1760-regs.h" > > > > #if defined(CONFIG_OF) && defined(CONFIG_OF_IRQ) -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
