RE: [PATCH] usb: dwc3: Add Exynos Specific Glue layer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux