Hi, > -----Original Message----- > From: linux-usb-owner@xxxxxxxxxxxxxxx [mailto:linux-usb- > owner@xxxxxxxxxxxxxxx] On Behalf Of Felipe Balbi > Sent: Wednesday, February 15, 2012 5:20 PM > To: Anton Tikhomirov > Cc: linux-usb@xxxxxxxxxxxxxxx; 'Felipe Balbi'; 'Greg KH' > Subject: Re: [PATCH] usb: dwc3: Add Exynos Specific Glue layer > > Hi, > > On Wed, Feb 15, 2012 at 05:04:56PM +0900, Anton Tikhomirov wrote: > > Adds Exynos Specific Glue layer to support USB peripherals > > on Samsung Exynos5 chips. > > > > Signed-off-by: Anton Tikhomirov <av.tikhomirov@xxxxxxxxxxx> > > --- > > drivers/usb/dwc3/Makefile | 1 + > > drivers/usb/dwc3/dwc3-exynos.c | 151 > +++++++++++++++++++++++++++++ > > include/linux/platform_data/dwc3-exynos.h | 24 +++++ > > 3 files changed, 176 insertions(+), 0 deletions(-) > > create mode 100644 drivers/usb/dwc3/dwc3-exynos.c > > create mode 100644 include/linux/platform_data/dwc3-exynos.h > > > > diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile > > index 900ae74..4502648 100644 > > --- a/drivers/usb/dwc3/Makefile > > +++ b/drivers/usb/dwc3/Makefile > > @@ -27,6 +27,7 @@ endif > > ## > > > > obj-$(CONFIG_USB_DWC3) += dwc3-omap.o > > +obj-$(CONFIG_USB_DWC3) += dwc3-exynos.o > > > > ifneq ($(CONFIG_PCI),) > > obj-$(CONFIG_USB_DWC3) += dwc3-pci.o > > diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3- > exynos.c > > new file mode 100644 > > index 0000000..d190301 > > --- /dev/null > > +++ b/drivers/usb/dwc3/dwc3-exynos.c > > @@ -0,0 +1,151 @@ > > +/** > > + * dwc3-exynos.c - Samsung EXYNOS DWC3 Specific Glue layer > > + * > > + * Copyright (c) 2012 Samsung Electronics Co., Ltd. > > + * http://www.samsung.com > > + * > > + * Author: Anton Tikhomirov <av.tikhomirov@xxxxxxxxxxx> > > + * > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License as published by > > + * the Free Software Foundation; either version 2 of the License, or > > + * (at your option) any later version. > > + */ > > + > > +#include <linux/module.h> > > +#include <linux/kernel.h> > > +#include <linux/slab.h> > > +#include <linux/platform_device.h> > > +#include <linux/platform_data/dwc3-exynos.h> > > +#include <linux/dma-mapping.h> > > +#include <linux/module.h> > > +#include <linux/clk.h> > > + > > +#include "core.h" > > + > > +struct dwc3_exynos { > > + struct platform_device *dwc3; > > + struct device *dev; > > + > > + struct clk *clk; > > +}; > > + > > +static int __devinit dwc3_exynos_probe(struct platform_device *pdev) > > +{ > > + struct dwc3_exynos_data *pdata = pdev->dev.platform_data; > > + struct platform_device *dwc3; > > + struct dwc3_exynos *exynos; > > + struct clk *clk; > > + > > + int devid; > > + int ret = -ENOMEM; > > + > > + exynos = kzalloc(sizeof(*exynos), GFP_KERNEL); > > + if (!exynos) { > > + dev_err(&pdev->dev, "not enough memory\n"); > > + goto err0; > > + } > > + > > + platform_set_drvdata(pdev, exynos); > > + > > + devid = dwc3_get_device_id(); > > + if (devid < 0) > > + goto err1; > > + > > + dwc3 = platform_device_alloc("dwc3", devid); > > + if (!dwc3) { > > + dev_err(&pdev->dev, "couldn't allocate dwc3 device\n"); > > + goto err2; > > + } > > + > > + clk = clk_get(&pdev->dev, "usbdrd30"); > > + if (IS_ERR(clk)) { > > + dev_err(&pdev->dev, "couldn't get clock\n"); > > + ret = -EINVAL; > > + goto err3; > > + } > > + > > + dma_set_coherent_mask(&dwc3->dev, pdev->dev.coherent_dma_mask); > > + > > + dwc3->dev.parent = &pdev->dev; > > + dwc3->dev.dma_mask = pdev->dev.dma_mask; > > + dwc3->dev.dma_parms = pdev->dev.dma_parms; > > + exynos->dwc3 = dwc3; > > + exynos->dev = &pdev->dev; > > + exynos->clk = clk; > > + > > + clk_enable(exynos->clk); > > + > > + /* PHY initialization */ > > + if (!pdata) { > > + dev_dbg(&pdev->dev, "missing platform data\n"); > > + } else { > > + if (pdata->phy_init) > > + pdata->phy_init(pdev, pdata->phy_type); > > + } > > + > > + ret = platform_device_add_resources(dwc3, pdev->resource, > > + pdev->num_resources); > > + if (ret) { > > + dev_err(&pdev->dev, "couldn't add resources to dwc3 > device\n"); > > + goto err4; > > + } > > + > > + ret = platform_device_add(dwc3); > > + if (ret) { > > + dev_err(&pdev->dev, "failed to register dwc3 device\n"); > > + goto err4; > > + } > > + > > + return 0; > > + > > +err4: > > + if (pdata && pdata->phy_exit) > > + pdata->phy_exit(pdev, pdata->phy_type); > > + > > + clk_disable(clk); > > + clk_put(clk); > > +err3: > > + platform_device_put(dwc3); > > +err2: > > + dwc3_put_device_id(devid); > > +err1: > > + kfree(exynos); > > +err0: > > + return ret; > > +} > > + > > +static int __devexit dwc3_exynos_remove(struct platform_device *pdev) > > +{ > > + struct dwc3_exynos *exynos = platform_get_drvdata(pdev); > > + struct dwc3_exynos_data *pdata = pdev->dev.platform_data; > > + > > + platform_device_unregister(exynos->dwc3); > > + > > + dwc3_put_device_id(exynos->dwc3->id); > > + > > + if (pdata && pdata->phy_exit) > > + pdata->phy_exit(pdev, pdata->phy_type); > > + > > + clk_disable(exynos->clk); > > + clk_put(exynos->clk); > > + > > + kfree(exynos); > > + > > + return 0; > > +} > > + > > +static struct platform_driver dwc3_exynos_driver = { > > + .probe = dwc3_exynos_probe, > > + .remove = __devexit_p(dwc3_exynos_remove), > > + .driver = { > > + .name = "exynos-dwc3", > > should you add of_match_table here ? Not sure what are your plans to > support devicetree... > Currently we don't care about this. > > diff --git a/include/linux/platform_data/dwc3-exynos.h > b/include/linux/platform_data/dwc3-exynos.h > > new file mode 100644 > > index 0000000..5eb7da9 > > --- /dev/null > > +++ b/include/linux/platform_data/dwc3-exynos.h > > @@ -0,0 +1,24 @@ > > +/** > > + * dwc3-exynos.h - Samsung EXYNOS DWC3 Specific Glue layer, header. > > + * > > + * Copyright (c) 2012 Samsung Electronics Co., Ltd. > > + * http://www.samsung.com > > + * > > + * Author: Anton Tikhomirov <av.tikhomirov@xxxxxxxxxxx> > > + * > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License as published by > > + * the Free Software Foundation; either version 2 of the License, or > > + * (at your option) any later version. > > + */ > > + > > +#ifndef _DWC3_EXYNOS_H_ > > +#define _DWC3_EXYNOS_H_ > > + > > +struct dwc3_exynos_data { > > + int phy_type; > > + int (*phy_init)(struct platform_device *pdev, int type); > > + int (*phy_exit)(struct platform_device *pdev, int type); > > +}; > > This is really my only comment to this patch. Why do you need to pass > phy_type down to driver if it's only gonna be passed back as an argument > to phy_init()/phy_exit() ?? > > I would expect the PHY to know its own type, meaning that: > > pdata->phy_init(pdev); > > should be enough, no ? > I don't like this too, but it is only to comply with the current implementation of USB Phy setup in Exynos platform code (ref. arch/arm/mach-exynos/setup-usb-phy.c). > -- > balbi -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html