| Message ID | 1322550565-29162-1-git-send-email-peter.chen@freescale.com |
|---|---|
| State | New |
| Headers | show |
Hi, Peter Chen writes: > The patch removes all the uses of cpu_is_mx(). Instead, it utilizes > platform_device_id to distinguish the ehci differences among SoCs. > It can be useful to imx ehci submission and device tree support later. > > As the EHCI core is equivalent on all SoCs but only require different clocks on each platform I think it's overkill to use platform_ids to distinguish the SoCs. Indeed there is actually no need to make any distinction in the driver, as you could simply provide the same set of clocks on all platforms with some of them being dummy clocks on certain platforms: clock-imx25.c and clock-imx35.c: _REGISTER_CLOCK("mxc-ehci.0", "usb", usbotg_clk) _REGISTER_CLOCK("mxc-ehci.0", "system_bus", dummy_clk) _REGISTER_CLOCK("mxc-ehci.0", "usb_phy", dummy_clk) _REGISTER_CLOCK("mxc-ehci.1", "usb", usbotg_clk) _REGISTER_CLOCK("mxc-ehci.1", "system_bus", dummy_clk) _REGISTER_CLOCK("mxc-ehci.1", "usb_phy", dummy_clk) _REGISTER_CLOCK("mxc-ehci.2", "usb", usbotg_clk) _REGISTER_CLOCK("mxc-ehci.2", "system_bus", dummy_clk) _REGISTER_CLOCK("mxc-ehci.2", "usb_phy", dummy_clk) clock-imx27.c: _REGISTER_CLOCK("mxc-ehci.0", "usb", usb_clk) _REGISTER_CLOCK("mxc-ehci.0", "system_bus", usb_clk1) _REGISTER_CLOCK("mxc-ehci.0", "usb_phy", dummy_clk) _REGISTER_CLOCK("mxc-ehci.1", "usb", usb_clk) _REGISTER_CLOCK("mxc-ehci.1", "system_bus", usb_clk1) _REGISTER_CLOCK("mxc-ehci.1", "usb_phy", dummy_clk) _REGISTER_CLOCK("mxc-ehci.2", "usb", usb_clk) _REGISTER_CLOCK("mxc-ehci.2", "system_bus", usb_clk1) _REGISTER_CLOCK("mxc-ehci.2", "usb_phy", dummy_clk) clock-imx31.c: _REGISTER_CLOCK("mxc-ehci.0", "usb", usb_clk1) _REGISTER_CLOCK("mxc-ehci.0", "system_bus", usb_clk2) _REGISTER_CLOCK("mxc-ehci.0", "usb_phy", dummy_clk) _REGISTER_CLOCK("mxc-ehci.1", "usb", usb_clk1) _REGISTER_CLOCK("mxc-ehci.1", "system_bus", usb_clk2) _REGISTER_CLOCK("mxc-ehci.1", "usb_phy", dummy_clk) _REGISTER_CLOCK("mxc-ehci.2", "usb", usb_clk1) _REGISTER_CLOCK("mxc-ehci.2", "system_bus", usb_clk2) _REGISTER_CLOCK("mxc-ehci.2", "usb_phy", dummy_clk) clock-mx51-mx53.c: _REGISTER_CLOCK("mxc-ehci.0", "usb", usboh3_clk) _REGISTER_CLOCK("mxc-ehci.0", "system_bus", usb_ahb_clk) _REGISTER_CLOCK("mxc-ehci.0", "usb_phy", usb_phy1_clk) _REGISTER_CLOCK("mxc-ehci.1", "usb", usboh3_clk) _REGISTER_CLOCK("mxc-ehci.1", "system_bus", usb_ahb_clk) _REGISTER_CLOCK("mxc-ehci.1", "usb_phy", dummy_clk) _REGISTER_CLOCK("mxc-ehci.2", "usb", usboh3_clk) _REGISTER_CLOCK("mxc-ehci.2", "system_bus", usb_ahb_clk) _REGISTER_CLOCK("mxc-ehci.2", "usb_phy", dummy_clk) In the driver you would then do a clk_get() for those three clocks independent of the platform. Lothar Waßmann
On Tue, Nov 29, 2011 at 03:09:25PM +0800, Peter Chen wrote: > The patch removes all the uses of cpu_is_mx(). Instead, it utilizes > platform_device_id to distinguish the ehci differences among SoCs. > It can be useful to imx ehci submission and device tree support later. > > Signed-off-by: Peter Chen <peter.chen@freescale.com> > --- > arch/arm/mach-imx/clock-imx25.c | 6 +- > arch/arm/mach-imx/clock-imx27.c | 12 +- > arch/arm/mach-imx/clock-imx31.c | 12 +- > arch/arm/mach-imx/clock-imx35.c | 6 +- > arch/arm/mach-mx5/clock-mx51-mx53.c | 14 ++-- > arch/arm/plat-mxc/devices/platform-mxc-ehci.c | 40 +++++--- > arch/arm/plat-mxc/include/mach/devices-common.h | 1 + > drivers/usb/host/ehci-mxc.c | 118 ++++++++++++++++++++++- > 8 files changed, 166 insertions(+), 43 deletions(-) > > diff --git a/arch/arm/mach-imx/clock-imx25.c b/arch/arm/mach-imx/clock-imx25.c > index b0fec74..8288b6e 100644 > --- a/arch/arm/mach-imx/clock-imx25.c > +++ b/arch/arm/mach-imx/clock-imx25.c > @@ -279,9 +279,9 @@ static struct clk_lookup lookups[] = { > _REGISTER_CLOCK("imx21-uart.2", NULL, uart3_clk) > _REGISTER_CLOCK("imx21-uart.3", NULL, uart4_clk) > _REGISTER_CLOCK("imx21-uart.4", NULL, uart5_clk) > - _REGISTER_CLOCK("mxc-ehci.0", "usb", usbotg_clk) > - _REGISTER_CLOCK("mxc-ehci.1", "usb", usbotg_clk) > - _REGISTER_CLOCK("mxc-ehci.2", "usb", usbotg_clk) > + _REGISTER_CLOCK("ehci-imx25.0", "usb", usbotg_clk) > + _REGISTER_CLOCK("ehci-imx25.1", "usb", usbotg_clk) > + _REGISTER_CLOCK("ehci-imx25.2", "usb", usbotg_clk) No. Please add a dummy ahb clock for the SoCs which do not have a real one. Then you don't have to care about the differerences between the SoCs in the driver at all. > _REGISTER_CLOCK("fsl-usb2-udc", "usb", usbotg_clk) > _REGISTER_CLOCK("mxc_nand.0", NULL, nfc_clk) > /* i.mx25 has the i.mx35 type cspi */ > diff --git a/arch/arm/mach-imx/clock-imx27.c b/arch/arm/mach-imx/clock-imx27.c > index 88fe00a..fcbf0ec 100644 > --- a/arch/arm/mach-imx/clock-imx27.c > +++ b/arch/arm/mach-imx/clock-imx27.c > @@ -648,12 +648,12 @@ static struct clk_lookup lookups[] = { > _REGISTER_CLOCK("mx2-camera.0", NULL, csi_clk) > _REGISTER_CLOCK("fsl-usb2-udc", "usb", usb_clk) > _REGISTER_CLOCK("fsl-usb2-udc", "usb_ahb", usb_clk1) > - _REGISTER_CLOCK("mxc-ehci.0", "usb", usb_clk) > - _REGISTER_CLOCK("mxc-ehci.0", "usb_ahb", usb_clk1) > - _REGISTER_CLOCK("mxc-ehci.1", "usb", usb_clk) > - _REGISTER_CLOCK("mxc-ehci.1", "usb_ahb", usb_clk1) > - _REGISTER_CLOCK("mxc-ehci.2", "usb", usb_clk) > - _REGISTER_CLOCK("mxc-ehci.2", "usb_ahb", usb_clk1) > + _REGISTER_CLOCK("ehci-imx27.0", "usb", usb_clk) > + _REGISTER_CLOCK("ehci-imx27.0", "usb_ahb", usb_clk1) > + _REGISTER_CLOCK("ehci-imx27.1", "usb", usb_clk) > + _REGISTER_CLOCK("ehci-imx27.1", "usb_ahb", usb_clk1) > + _REGISTER_CLOCK("ehci-imx27.2", "usb", usb_clk) > + _REGISTER_CLOCK("ehci-imx27.2", "usb_ahb", usb_clk1) > _REGISTER_CLOCK("imx-ssi.0", NULL, ssi1_clk) > _REGISTER_CLOCK("imx-ssi.1", NULL, ssi2_clk) > _REGISTER_CLOCK("mxc_nand.0", NULL, nfc_clk) > diff --git a/arch/arm/mach-imx/clock-imx31.c b/arch/arm/mach-imx/clock-imx31.c > index 988a281..cae39b1 100644 > --- a/arch/arm/mach-imx/clock-imx31.c > +++ b/arch/arm/mach-imx/clock-imx31.c > @@ -538,12 +538,12 @@ static struct clk_lookup lookups[] = { > _REGISTER_CLOCK("ipu-core", NULL, ipu_clk) > _REGISTER_CLOCK("mx3_sdc_fb", NULL, ipu_clk) > _REGISTER_CLOCK(NULL, "kpp", kpp_clk) > - _REGISTER_CLOCK("mxc-ehci.0", "usb", usb_clk1) > - _REGISTER_CLOCK("mxc-ehci.0", "usb_ahb", usb_clk2) > - _REGISTER_CLOCK("mxc-ehci.1", "usb", usb_clk1) > - _REGISTER_CLOCK("mxc-ehci.1", "usb_ahb", usb_clk2) > - _REGISTER_CLOCK("mxc-ehci.2", "usb", usb_clk1) > - _REGISTER_CLOCK("mxc-ehci.2", "usb_ahb", usb_clk2) > + _REGISTER_CLOCK("ehci-imx31.0", "usb", usb_clk1) > + _REGISTER_CLOCK("ehci-imx31.0", "usb_ahb", usb_clk2) > + _REGISTER_CLOCK("ehci-imx31.1", "usb", usb_clk1) > + _REGISTER_CLOCK("ehci-imx31.1", "usb_ahb", usb_clk2) > + _REGISTER_CLOCK("ehci-imx31.2", "usb", usb_clk1) > + _REGISTER_CLOCK("ehci-imx31.2", "usb_ahb", usb_clk2) > _REGISTER_CLOCK("fsl-usb2-udc", "usb", usb_clk1) > _REGISTER_CLOCK("fsl-usb2-udc", "usb_ahb", usb_clk2) > _REGISTER_CLOCK("mx3-camera.0", NULL, csi_clk) > diff --git a/arch/arm/mach-imx/clock-imx35.c b/arch/arm/mach-imx/clock-imx35.c > index 8116f11..661b884 100644 > --- a/arch/arm/mach-imx/clock-imx35.c > +++ b/arch/arm/mach-imx/clock-imx35.c > @@ -491,9 +491,9 @@ static struct clk_lookup lookups[] = { > _REGISTER_CLOCK("imx21-uart.0", NULL, uart1_clk) > _REGISTER_CLOCK("imx21-uart.1", NULL, uart2_clk) > _REGISTER_CLOCK("imx21-uart.2", NULL, uart3_clk) > - _REGISTER_CLOCK("mxc-ehci.0", "usb", usbotg_clk) > - _REGISTER_CLOCK("mxc-ehci.1", "usb", usbotg_clk) > - _REGISTER_CLOCK("mxc-ehci.2", "usb", usbotg_clk) > + _REGISTER_CLOCK("ehci-imx35.0", "usb", usbotg_clk) > + _REGISTER_CLOCK("ehci-imx35.1", "usb", usbotg_clk) > + _REGISTER_CLOCK("ehci-imx35.2", "usb", usbotg_clk) > _REGISTER_CLOCK("fsl-usb2-udc", "usb", usbotg_clk) > _REGISTER_CLOCK("fsl-usb2-udc", "usb_ahb", usbahb_clk) > _REGISTER_CLOCK("imx2-wdt.0", NULL, wdog_clk) > diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c b/arch/arm/mach-mx5/clock-mx51-mx53.c > index 4cb2769..56dc34b 100644 > --- a/arch/arm/mach-mx5/clock-mx51-mx53.c > +++ b/arch/arm/mach-mx5/clock-mx51-mx53.c > @@ -1459,13 +1459,13 @@ static struct clk_lookup mx51_lookups[] = { > _REGISTER_CLOCK("imx-i2c.0", NULL, i2c1_clk) > _REGISTER_CLOCK("imx-i2c.1", NULL, i2c2_clk) > _REGISTER_CLOCK("imx-i2c.2", NULL, hsi2c_clk) > - _REGISTER_CLOCK("mxc-ehci.0", "usb", usboh3_clk) > - _REGISTER_CLOCK("mxc-ehci.0", "usb_ahb", usb_ahb_clk) > - _REGISTER_CLOCK("mxc-ehci.0", "usb_phy1", usb_phy1_clk) > - _REGISTER_CLOCK("mxc-ehci.1", "usb", usboh3_clk) > - _REGISTER_CLOCK("mxc-ehci.1", "usb_ahb", usb_ahb_clk) > - _REGISTER_CLOCK("mxc-ehci.2", "usb", usboh3_clk) > - _REGISTER_CLOCK("mxc-ehci.2", "usb_ahb", usb_ahb_clk) > + _REGISTER_CLOCK("ehci-imx51.0", "usb", usboh3_clk) > + _REGISTER_CLOCK("ehci-imx51.0", "usb_ahb", usb_ahb_clk) > + _REGISTER_CLOCK("ehci-imx51.0", "usb_phy1", usb_phy1_clk) The real solution for the phy clock is to handle the phy completely outside the ehci driver. The phy is a resource shared between the ehci and the corresponding gadget driver. I suggest to handle the phy clock in mx51_initialize_usb_hw instead since we have all phy related code in this function. This is a good place until we finally have a real phy driver. > + _REGISTER_CLOCK("ehci-imx51.1", "usb", usboh3_clk) > + _REGISTER_CLOCK("ehci-imx51.1", "usb_ahb", usb_ahb_clk) > + _REGISTER_CLOCK("ehci-imx51.2", "usb", usboh3_clk) > + _REGISTER_CLOCK("ehci-imx51.2", "usb_ahb", usb_ahb_clk) > _REGISTER_CLOCK("fsl-usb2-udc", "usb", usboh3_clk) > _REGISTER_CLOCK("fsl-usb2-udc", "usb_ahb", ahb_clk) > _REGISTER_CLOCK("imx-keypad", NULL, dummy_clk) > diff --git a/arch/arm/plat-mxc/devices/platform-mxc-ehci.c b/arch/arm/plat-mxc/devices/platform-mxc-ehci.c > index 35851d8..d339887 100644 [...] > diff --git a/arch/arm/plat-mxc/include/mach/devices-common.h b/arch/arm/plat-mxc/include/mach/devices-common.h > index def9ba5..15b68e8 100644 > --- a/arch/arm/plat-mxc/include/mach/devices-common.h > +++ b/arch/arm/plat-mxc/include/mach/devices-common.h > @@ -226,6 +226,7 @@ struct platform_device *__init imx_add_mx2_camera( > > #include <mach/mxc_ehci.h> > struct imx_mxc_ehci_data { > + const char *devid; > int id; > resource_size_t iobase; > resource_size_t irq; > diff --git a/drivers/usb/host/ehci-mxc.c b/drivers/usb/host/ehci-mxc.c > index 55978fc..6f6b00a 100644 > --- a/drivers/usb/host/ehci-mxc.c > +++ b/drivers/usb/host/ehci-mxc.c > @@ -23,6 +23,7 @@ > #include <linux/usb/otg.h> > #include <linux/usb/ulpi.h> > #include <linux/slab.h> > +#include <linux/of_device.h> > > #include <mach/hardware.h> > #include <mach/mxc_ehci.h> > @@ -31,10 +32,116 @@ > > #define ULPI_VIEWPORT_OFFSET 0x170 > > +enum mxc_ehci_type { > + IMX23_EHCI, > + IMX25_EHCI, > + IMX28_EHCI, > + IMX31_EHCI, > + IMX35_EHCI, > + IMX50_EHCI, > + IMX51_EHCI, > + IMX53_EHCI, > + IMX6Q_EHCI, > +}; Instead of adding this enum you should introduce a SoC specific driver data struct and add fields to this where you describe the differences between the SoCs. All the if(mxc_*_ehci()) need fixes for each new SoC revision. Do a if(soc_data->has_ahb_clk) instead. Only a hint for next time since I think there are no differences to be handled in the driver once we have dummy ahb clocks. > + > struct ehci_mxc_priv { > struct clk *usbclk, *ahbclk, *phy1clk; > struct usb_hcd *hcd; > + enum mxc_ehci_type devtype; > +}; > + > +static struct platform_device_id mxc_ehci_devtype[] = { > + { > + .name = "ehci-imx23", > + .driver_data = IMX25_EHCI, > + }, { > + .name = "ehci-imx25", > + .driver_data = IMX35_EHCI, > + }, { > + .name = "ehci-imx28", > + .driver_data = IMX35_EHCI, > + }, { > + .name = "ehci-imx31", > + .driver_data = IMX35_EHCI, > + }, { > + .name = "ehci-imx35", > + .driver_data = IMX35_EHCI, > + }, { > + .name = "ehci-imx50", > + .driver_data = IMX51_EHCI, > + }, { > + .name = "ehci-imx51", > + .driver_data = IMX51_EHCI, > + }, { > + .name = "ehci-imx53", > + .driver_data = IMX53_EHCI, > + }, { > + .name = "ehci-imx6q", > + .driver_data = IMX6Q_EHCI, > + }, { > + /* sentinel */ > + } > }; > +MODULE_DEVICE_TABLE(platform, mxc_ehci_devtype); > + > +static const struct of_device_id imx_ehci_dt_ids[] = { > + { .compatible = "fsl,imx23-ehci", .data = &mxc_ehci_devtype[IMX23_EHCI], }, > + { .compatible = "fsl,imx25-ehci", .data = &mxc_ehci_devtype[IMX25_EHCI], }, > + { .compatible = "fsl,imx28-ehci", .data = &mxc_ehci_devtype[IMX28_EHCI], }, > + { .compatible = "fsl,imx31-ehci", .data = &mxc_ehci_devtype[IMX31_EHCI], }, > + { .compatible = "fsl,imx35-ehci", .data = &mxc_ehci_devtype[IMX35_EHCI], }, > + { .compatible = "fsl,imx50-ehci", .data = &mxc_ehci_devtype[IMX50_EHCI], }, > + { .compatible = "fsl,imx51-ehci", .data = &mxc_ehci_devtype[IMX51_EHCI], }, > + { .compatible = "fsl,imx53-ehci", .data = &mxc_ehci_devtype[IMX53_EHCI], }, > + { .compatible = "fsl,imx6q-ehci", .data = &mxc_ehci_devtype[IMX6Q_EHCI], }, > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, imx_ehci_dt_ids); > +
> Peter Chen writes: > > The patch removes all the uses of cpu_is_mx(). Instead, it utilizes > > platform_device_id to distinguish the ehci differences among SoCs. > > It can be useful to imx ehci submission and device tree support later. > > > > > As the EHCI core is equivalent on all SoCs but only require different > clocks on each platform I think it's overkill to use platform_ids to > distinguish the SoCs. Indeed there is actually no need to make any > distinction in the driver, as you could simply provide the same set of > clocks on all platforms with some of them being dummy clocks on > certain platforms: > clock-imx25.c and clock-imx35.c: > _REGISTER_CLOCK("mxc-ehci.0", "usb", usbotg_clk) > _REGISTER_CLOCK("mxc-ehci.0", "system_bus", dummy_clk) > _REGISTER_CLOCK("mxc-ehci.0", "usb_phy", dummy_clk) > _REGISTER_CLOCK("mxc-ehci.1", "usb", usbotg_clk) > _REGISTER_CLOCK("mxc-ehci.1", "system_bus", dummy_clk) > _REGISTER_CLOCK("mxc-ehci.1", "usb_phy", dummy_clk) > _REGISTER_CLOCK("mxc-ehci.2", "usb", usbotg_clk) > _REGISTER_CLOCK("mxc-ehci.2", "system_bus", dummy_clk) > _REGISTER_CLOCK("mxc-ehci.2", "usb_phy", dummy_clk) > clock-imx27.c: > _REGISTER_CLOCK("mxc-ehci.0", "usb", usb_clk) > _REGISTER_CLOCK("mxc-ehci.0", "system_bus", usb_clk1) > _REGISTER_CLOCK("mxc-ehci.0", "usb_phy", dummy_clk) > _REGISTER_CLOCK("mxc-ehci.1", "usb", usb_clk) > _REGISTER_CLOCK("mxc-ehci.1", "system_bus", usb_clk1) > _REGISTER_CLOCK("mxc-ehci.1", "usb_phy", dummy_clk) > _REGISTER_CLOCK("mxc-ehci.2", "usb", usb_clk) > _REGISTER_CLOCK("mxc-ehci.2", "system_bus", usb_clk1) > _REGISTER_CLOCK("mxc-ehci.2", "usb_phy", dummy_clk) > clock-imx31.c: > _REGISTER_CLOCK("mxc-ehci.0", "usb", usb_clk1) > _REGISTER_CLOCK("mxc-ehci.0", "system_bus", usb_clk2) > _REGISTER_CLOCK("mxc-ehci.0", "usb_phy", dummy_clk) > _REGISTER_CLOCK("mxc-ehci.1", "usb", usb_clk1) > _REGISTER_CLOCK("mxc-ehci.1", "system_bus", usb_clk2) > _REGISTER_CLOCK("mxc-ehci.1", "usb_phy", dummy_clk) > _REGISTER_CLOCK("mxc-ehci.2", "usb", usb_clk1) > _REGISTER_CLOCK("mxc-ehci.2", "system_bus", usb_clk2) > _REGISTER_CLOCK("mxc-ehci.2", "usb_phy", dummy_clk) > clock-mx51-mx53.c: > _REGISTER_CLOCK("mxc-ehci.0", "usb", usboh3_clk) > _REGISTER_CLOCK("mxc-ehci.0", "system_bus", usb_ahb_clk) > _REGISTER_CLOCK("mxc-ehci.0", "usb_phy", usb_phy1_clk) > _REGISTER_CLOCK("mxc-ehci.1", "usb", usboh3_clk) > _REGISTER_CLOCK("mxc-ehci.1", "system_bus", usb_ahb_clk) > _REGISTER_CLOCK("mxc-ehci.1", "usb_phy", dummy_clk) > _REGISTER_CLOCK("mxc-ehci.2", "usb", usboh3_clk) > _REGISTER_CLOCK("mxc-ehci.2", "system_bus", usb_ahb_clk) > _REGISTER_CLOCK("mxc-ehci.2", "usb_phy", dummy_clk) > > > In the driver you would then do a clk_get() for those three clocks > independent of the platform. > The reason why I add this patch is not for clk things, it is for: 1. Coming device driver 2. Handle things difference between SoC's. not only clk, others like low power, wakeup, hsic, or other will comes in future. > > Lothar Waßmann > -- > ___________________________________________________________ > > Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen > Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10 > Geschäftsführer: Matthias Kaussen > Handelsregistereintrag: Amtsgericht Aachen, HRB 4996 > > www.karo-electronics.de | info@karo-electronics.de > ___________________________________________________________
Hello, On Tue, Nov 29, 2011 at 08:02:53AM +0000, Chen Peter-B29397 wrote: > > > Peter Chen writes: > > > The patch removes all the uses of cpu_is_mx(). Instead, it utilizes > > > platform_device_id to distinguish the ehci differences among SoCs. > > > It can be useful to imx ehci submission and device tree support later. > > > > > > > > As the EHCI core is equivalent on all SoCs but only require different > > clocks on each platform I think it's overkill to use platform_ids to > > distinguish the SoCs. Indeed there is actually no need to make any > > distinction in the driver, as you could simply provide the same set of > > clocks on all platforms with some of them being dummy clocks on > > certain platforms: > > clock-imx25.c and clock-imx35.c: > > _REGISTER_CLOCK("mxc-ehci.0", "usb", usbotg_clk) > > _REGISTER_CLOCK("mxc-ehci.0", "system_bus", dummy_clk) > > _REGISTER_CLOCK("mxc-ehci.0", "usb_phy", dummy_clk) > > _REGISTER_CLOCK("mxc-ehci.1", "usb", usbotg_clk) > > _REGISTER_CLOCK("mxc-ehci.1", "system_bus", dummy_clk) > > _REGISTER_CLOCK("mxc-ehci.1", "usb_phy", dummy_clk) > > _REGISTER_CLOCK("mxc-ehci.2", "usb", usbotg_clk) > > _REGISTER_CLOCK("mxc-ehci.2", "system_bus", dummy_clk) > > _REGISTER_CLOCK("mxc-ehci.2", "usb_phy", dummy_clk) > > clock-imx27.c: > > _REGISTER_CLOCK("mxc-ehci.0", "usb", usb_clk) > > _REGISTER_CLOCK("mxc-ehci.0", "system_bus", usb_clk1) > > _REGISTER_CLOCK("mxc-ehci.0", "usb_phy", dummy_clk) > > _REGISTER_CLOCK("mxc-ehci.1", "usb", usb_clk) > > _REGISTER_CLOCK("mxc-ehci.1", "system_bus", usb_clk1) > > _REGISTER_CLOCK("mxc-ehci.1", "usb_phy", dummy_clk) > > _REGISTER_CLOCK("mxc-ehci.2", "usb", usb_clk) > > _REGISTER_CLOCK("mxc-ehci.2", "system_bus", usb_clk1) > > _REGISTER_CLOCK("mxc-ehci.2", "usb_phy", dummy_clk) > > clock-imx31.c: > > _REGISTER_CLOCK("mxc-ehci.0", "usb", usb_clk1) > > _REGISTER_CLOCK("mxc-ehci.0", "system_bus", usb_clk2) > > _REGISTER_CLOCK("mxc-ehci.0", "usb_phy", dummy_clk) > > _REGISTER_CLOCK("mxc-ehci.1", "usb", usb_clk1) > > _REGISTER_CLOCK("mxc-ehci.1", "system_bus", usb_clk2) > > _REGISTER_CLOCK("mxc-ehci.1", "usb_phy", dummy_clk) > > _REGISTER_CLOCK("mxc-ehci.2", "usb", usb_clk1) > > _REGISTER_CLOCK("mxc-ehci.2", "system_bus", usb_clk2) > > _REGISTER_CLOCK("mxc-ehci.2", "usb_phy", dummy_clk) > > clock-mx51-mx53.c: > > _REGISTER_CLOCK("mxc-ehci.0", "usb", usboh3_clk) > > _REGISTER_CLOCK("mxc-ehci.0", "system_bus", usb_ahb_clk) > > _REGISTER_CLOCK("mxc-ehci.0", "usb_phy", usb_phy1_clk) > > _REGISTER_CLOCK("mxc-ehci.1", "usb", usboh3_clk) > > _REGISTER_CLOCK("mxc-ehci.1", "system_bus", usb_ahb_clk) > > _REGISTER_CLOCK("mxc-ehci.1", "usb_phy", dummy_clk) > > _REGISTER_CLOCK("mxc-ehci.2", "usb", usboh3_clk) > > _REGISTER_CLOCK("mxc-ehci.2", "system_bus", usb_ahb_clk) > > _REGISTER_CLOCK("mxc-ehci.2", "usb_phy", dummy_clk) > > > > > > In the driver you would then do a clk_get() for those three clocks > > independent of the platform. > > > The reason why I add this patch is not for clk things, it is for: > 1. Coming device driver > 2. Handle things difference between SoC's. not only clk, others > like low power, wakeup, hsic, or other will comes in future. Then are they really different per SoC (in a way not yet handled in the driver)? If not, up to now these devices were named after the first SoC introducing them, so i.MX25 has imx21-uart.0 and imx35-cspi.0. Best regards Uwe
On Tue, Nov 29, 2011 at 08:02:53AM +0000, Chen Peter-B29397 wrote: > > > > In the driver you would then do a clk_get() for those three clocks > > independent of the platform. > > > The reason why I add this patch is not for clk things, it is for: > 1. Coming device driver > 2. Handle things difference between SoC's. not only clk, others > like low power, wakeup, hsic, or other will comes in future. The ehci is a standardized core. Please be *very* careful with what you add to this driver. As said in my other mail, the real differences are in the phys and the phy handling code should not be in the ehci-mxc driver at all. Rather than adding random phy code to the driver you should write phy drivers. If you still think that there is code missing in this driver please explain further. Sascha
> -----Original Message----- > From: s.hauer@pengutronix.de [mailto:s.hauer@pengutronix.de] > Sent: Tuesday, November 29, 2011 4:22 PM > To: Chen Peter-B29397 > Cc: Lothar Waßmann; stern@rowland.harvard.edu; gregkh@suse.de; > amit.kucheria@canonical.com; linux-usb@vger.kernel.org; linux-arm- > kernel@lists.infradead.org; u.kleine-koenig@pengutronix.de > Subject: Re: [PATCH] USB: ehci-mxc: get rid of the uses of cpu_is_mx() > > On Tue, Nov 29, 2011 at 08:02:53AM +0000, Chen Peter-B29397 wrote: > > > > > > In the driver you would then do a clk_get() for those three clocks > > > independent of the platform. > > > > > The reason why I add this patch is not for clk things, it is for: > > 1. Coming device driver > > 2. Handle things difference between SoC's. not only clk, others > > like low power, wakeup, hsic, or other will comes in future. > > The ehci is a standardized core. Please be *very* careful with what you > add to this driver. As said in my other mail, the real differences are > in the phys and the phy handling code should not be in the ehci-mxc > driver at all. > Rather than adding random phy code to the driver you should write phy > drivers. If you still think that there is code missing in this driver > please explain further. > > Sascha > Sascha, thanks for your comments. In fact, this patch does not change any ehci logics. The changes for clock.c are just let probe not return error with clk_get(dev, "usb"), as dev_id is changed. Yes, this patch is not meaningful for current ehci-mxc, I do this patch just for preparing submit further i.mx usb ehci driver. - If I am going to submit mx28 usb ehci driver, the things like cpu_is_mx51 is not defined for mxs platform - So, refer to i.mx spi and sdhci-esdhc changing for platform_device_id, I made this patch. If you think this patch is not meaningful at current stage, I will not go on. For USB PHY's driver, I will discuss with linux-usb list first, then discuss with you how to do it at i.mx platform. My current plan is refine phy.c which is from heikki's RFC, and implement at i.mx51 bbg first, which has two phys (UTMI for OTG, and ULPI for host1). What do you think? > -- > Pengutronix e.K. | > | > Industrial Linux Solutions | http://www.pengutronix.de/ > | > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 > | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 > |
On Tue, Nov 29, 2011 at 10:57:37AM +0000, Chen Peter-B29397 wrote: > > > -----Original Message----- > > From: s.hauer@pengutronix.de [mailto:s.hauer@pengutronix.de] > > Sent: Tuesday, November 29, 2011 4:22 PM > > To: Chen Peter-B29397 > > Cc: Lothar Waßmann; stern@rowland.harvard.edu; gregkh@suse.de; > > amit.kucheria@canonical.com; linux-usb@vger.kernel.org; linux-arm- > > kernel@lists.infradead.org; u.kleine-koenig@pengutronix.de > > Subject: Re: [PATCH] USB: ehci-mxc: get rid of the uses of cpu_is_mx() > > > > On Tue, Nov 29, 2011 at 08:02:53AM +0000, Chen Peter-B29397 wrote: > > > > > > > > In the driver you would then do a clk_get() for those three clocks > > > > independent of the platform. > > > > > > > The reason why I add this patch is not for clk things, it is for: > > > 1. Coming device driver > > > 2. Handle things difference between SoC's. not only clk, others > > > like low power, wakeup, hsic, or other will comes in future. > > > > The ehci is a standardized core. Please be *very* careful with what you > > add to this driver. As said in my other mail, the real differences are > > in the phys and the phy handling code should not be in the ehci-mxc > > driver at all. > > Rather than adding random phy code to the driver you should write phy > > drivers. If you still think that there is code missing in this driver > > please explain further. > > > > Sascha > > > Sascha, thanks for your comments. > > In fact, this patch does not change any ehci logics. The changes for clock.c > are just let probe not return error with clk_get(dev, "usb"), as dev_id is changed. > > Yes, this patch is not meaningful for current ehci-mxc, I do this patch just for > preparing submit further i.mx usb ehci driver. > - If I am going to submit mx28 usb ehci driver, the things like cpu_is_mx51 is not defined > for mxs platform > - So, refer to i.mx spi and sdhci-esdhc changing for platform_device_id, I made this patch. > > If you think this patch is not meaningful at current stage, I will not go on. > > For USB PHY's driver, I will discuss with linux-usb list first, then discuss with you how > to do it at i.mx platform. My current plan is refine phy.c which is from heikki's RFC, and > implement at i.mx51 bbg first, which has two phys (UTMI for OTG, and ULPI for host1). What do you > think? We have to deal with a historically grown wrong hardware abstraction here. What we have from a hardware point of view is a device consisting of: - A ehci core (theoretically optional, but seems to be always present) - A Synopsys USB device core (optional), the one we currently have four different drivers for. - A USB phy (mandatory, but may be nearly invisible from software). The phy may be an external ULPI phy, an internal phy in the SoC, or even selectable between both. Currently we have a ehci-mxc driver which claims resources like mem regions, interrupts and clocks. We also have the gadget driver (currently the fsl_mxc_udc driver) which also claims the same resources. Instead we should have a glue driver which claims the resources and passes control over to either the host or the gadget driver. This glue code should also connect the phy. Looking at the code in mainline the msm driver is closest to what we want to have. It claims the resources in msm_otg.c and passes control over to the ehci or ci13xxx gadget driver depending on the USB id pin or a platform decision. What's missing here is some plugin mechanism for different phys. That's another historically grown thing that there is only a single phy in the Kernel because there can be only one otg instance and phys were thought to be present only in otg cores. Heikkis patches work in the right direction here. As a conclusion I think that there should be no platform or SoC specific code in neither the ehci nor in the gadget driver. Also there shouldn't be all these ehci-$SoC.c files in the tree. The way to get there could be: - Work on Heikkis patches so that we can register usb phys - move the USB phy setup code from arch-imx to drivers/usb/phy - factor out the common code from the msm_otg driver. - create a imx-otg.c which only claims the resources and passes them to the otg common code. Sooner or later not even an imx-otg.c is necessary because we can use a devicetree based driver and then only the phy code will be SoC specific. I'm sorry that there is no easy way to get proper USB support, but we are way beyond the point where the current infrastructure scales. In any way, there is no place for more SoC specific code in ehci-mxc.c. Instead you should work on moving SoC code out of there. Sascha
> > We have to deal with a historically grown wrong hardware abstraction > here. What we have from a hardware point of view is a device consisting > of: > > - A ehci core (theoretically optional, but seems to be always present) > - A Synopsys USB device core (optional), the one we currently have four > different drivers for. > - A USB phy (mandatory, but may be nearly invisible from software). The > phy may be an external ULPI phy, an internal phy in the SoC, or even > selectable between both. > > Currently we have a ehci-mxc driver which claims resources like mem > regions, interrupts and clocks. We also have the gadget driver > (currently the fsl_mxc_udc driver) which also claims the same resources. > Instead we should have a glue driver which claims the resources and > passes control over to either the host or the gadget driver. This glue > code should also connect the phy. > > Looking at the code in mainline the msm driver is closest to what we > want to have. It claims the resources in msm_otg.c and passes control > over to the ehci or ci13xxx gadget driver depending on the USB id pin or > a platform decision. What's missing here is some plugin mechanism for > different phys. That's another historically grown thing that there is > only a single phy in the Kernel because there can be only one otg > instance and phys were thought to be present only in otg cores. Heikkis > patches work in the right direction here. msm_otg.c is really good code which mainlined this year, it considers many user situations, like low power, charger, usb wakeup, id/vbus changes from different hardware/user choice, etc. In fact, the most difficulties for otg driver (host/device share the same registers controller) are clock management, power management, usb wakeup, register access among the three drivers (otg, host and device driver) > > As a conclusion I think that there should be no platform or SoC specific > code in neither the ehci nor in the gadget driver. Also there shouldn't > be all these ehci-$SoC.c files in the tree. The way to get there could > be: > > - Work on Heikkis patches so that we can register usb phys > - move the USB phy setup code from arch-imx to drivers/usb/phy > - factor out the common code from the msm_otg driver. > - create a imx-otg.c which only claims the resources and passes > them to the otg common code. Sooner or later not even an imx-otg.c is > necessary because we can use a devicetree based driver and then only > the phy code will be SoC specific. I agree with these tasks for refine i.mx usb structure. I will try Heikki's patch first. > > I'm sorry that there is no easy way to get proper USB support, but we > are way beyond the point where the current infrastructure scales. > In any way, there is no place for more SoC specific code in ehci-mxc.c. > Instead you should work on moving SoC code out of there. > > Sascha > > > -- > Pengutronix e.K. | > | > Industrial Linux Solutions | http://www.pengutronix.de/ > | > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 > | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 > |
diff --git a/arch/arm/mach-imx/clock-imx25.c b/arch/arm/mach-imx/clock-imx25.c index b0fec74..8288b6e 100644 --- a/arch/arm/mach-imx/clock-imx25.c +++ b/arch/arm/mach-imx/clock-imx25.c @@ -279,9 +279,9 @@ static struct clk_lookup lookups[] = { _REGISTER_CLOCK("imx21-uart.2", NULL, uart3_clk) _REGISTER_CLOCK("imx21-uart.3", NULL, uart4_clk) _REGISTER_CLOCK("imx21-uart.4", NULL, uart5_clk) - _REGISTER_CLOCK("mxc-ehci.0", "usb", usbotg_clk) - _REGISTER_CLOCK("mxc-ehci.1", "usb", usbotg_clk) - _REGISTER_CLOCK("mxc-ehci.2", "usb", usbotg_clk) + _REGISTER_CLOCK("ehci-imx25.0", "usb", usbotg_clk) + _REGISTER_CLOCK("ehci-imx25.1", "usb", usbotg_clk) + _REGISTER_CLOCK("ehci-imx25.2", "usb", usbotg_clk) _REGISTER_CLOCK("fsl-usb2-udc", "usb", usbotg_clk) _REGISTER_CLOCK("mxc_nand.0", NULL, nfc_clk) /* i.mx25 has the i.mx35 type cspi */ diff --git a/arch/arm/mach-imx/clock-imx27.c b/arch/arm/mach-imx/clock-imx27.c index 88fe00a..fcbf0ec 100644 --- a/arch/arm/mach-imx/clock-imx27.c +++ b/arch/arm/mach-imx/clock-imx27.c @@ -648,12 +648,12 @@ static struct clk_lookup lookups[] = { _REGISTER_CLOCK("mx2-camera.0", NULL, csi_clk) _REGISTER_CLOCK("fsl-usb2-udc", "usb", usb_clk) _REGISTER_CLOCK("fsl-usb2-udc", "usb_ahb", usb_clk1) - _REGISTER_CLOCK("mxc-ehci.0", "usb", usb_clk) - _REGISTER_CLOCK("mxc-ehci.0", "usb_ahb", usb_clk1) - _REGISTER_CLOCK("mxc-ehci.1", "usb", usb_clk) - _REGISTER_CLOCK("mxc-ehci.1", "usb_ahb", usb_clk1) - _REGISTER_CLOCK("mxc-ehci.2", "usb", usb_clk) - _REGISTER_CLOCK("mxc-ehci.2", "usb_ahb", usb_clk1) + _REGISTER_CLOCK("ehci-imx27.0", "usb", usb_clk) + _REGISTER_CLOCK("ehci-imx27.0", "usb_ahb", usb_clk1) + _REGISTER_CLOCK("ehci-imx27.1", "usb", usb_clk) + _REGISTER_CLOCK("ehci-imx27.1", "usb_ahb", usb_clk1) + _REGISTER_CLOCK("ehci-imx27.2", "usb", usb_clk) + _REGISTER_CLOCK("ehci-imx27.2", "usb_ahb", usb_clk1) _REGISTER_CLOCK("imx-ssi.0", NULL, ssi1_clk) _REGISTER_CLOCK("imx-ssi.1", NULL, ssi2_clk) _REGISTER_CLOCK("mxc_nand.0", NULL, nfc_clk) diff --git a/arch/arm/mach-imx/clock-imx31.c b/arch/arm/mach-imx/clock-imx31.c index 988a281..cae39b1 100644 --- a/arch/arm/mach-imx/clock-imx31.c +++ b/arch/arm/mach-imx/clock-imx31.c @@ -538,12 +538,12 @@ static struct clk_lookup lookups[] = { _REGISTER_CLOCK("ipu-core", NULL, ipu_clk) _REGISTER_CLOCK("mx3_sdc_fb", NULL, ipu_clk) _REGISTER_CLOCK(NULL, "kpp", kpp_clk) - _REGISTER_CLOCK("mxc-ehci.0", "usb", usb_clk1) - _REGISTER_CLOCK("mxc-ehci.0", "usb_ahb", usb_clk2) - _REGISTER_CLOCK("mxc-ehci.1", "usb", usb_clk1) - _REGISTER_CLOCK("mxc-ehci.1", "usb_ahb", usb_clk2) - _REGISTER_CLOCK("mxc-ehci.2", "usb", usb_clk1) - _REGISTER_CLOCK("mxc-ehci.2", "usb_ahb", usb_clk2) + _REGISTER_CLOCK("ehci-imx31.0", "usb", usb_clk1) + _REGISTER_CLOCK("ehci-imx31.0", "usb_ahb", usb_clk2) + _REGISTER_CLOCK("ehci-imx31.1", "usb", usb_clk1) + _REGISTER_CLOCK("ehci-imx31.1", "usb_ahb", usb_clk2) + _REGISTER_CLOCK("ehci-imx31.2", "usb", usb_clk1) + _REGISTER_CLOCK("ehci-imx31.2", "usb_ahb", usb_clk2) _REGISTER_CLOCK("fsl-usb2-udc", "usb", usb_clk1) _REGISTER_CLOCK("fsl-usb2-udc", "usb_ahb", usb_clk2) _REGISTER_CLOCK("mx3-camera.0", NULL, csi_clk) diff --git a/arch/arm/mach-imx/clock-imx35.c b/arch/arm/mach-imx/clock-imx35.c index 8116f11..661b884 100644 --- a/arch/arm/mach-imx/clock-imx35.c +++ b/arch/arm/mach-imx/clock-imx35.c @@ -491,9 +491,9 @@ static struct clk_lookup lookups[] = { _REGISTER_CLOCK("imx21-uart.0", NULL, uart1_clk) _REGISTER_CLOCK("imx21-uart.1", NULL, uart2_clk) _REGISTER_CLOCK("imx21-uart.2", NULL, uart3_clk) - _REGISTER_CLOCK("mxc-ehci.0", "usb", usbotg_clk) - _REGISTER_CLOCK("mxc-ehci.1", "usb", usbotg_clk) - _REGISTER_CLOCK("mxc-ehci.2", "usb", usbotg_clk) + _REGISTER_CLOCK("ehci-imx35.0", "usb", usbotg_clk) + _REGISTER_CLOCK("ehci-imx35.1", "usb", usbotg_clk) + _REGISTER_CLOCK("ehci-imx35.2", "usb", usbotg_clk) _REGISTER_CLOCK("fsl-usb2-udc", "usb", usbotg_clk) _REGISTER_CLOCK("fsl-usb2-udc", "usb_ahb", usbahb_clk) _REGISTER_CLOCK("imx2-wdt.0", NULL, wdog_clk) diff --git a/arch/arm/mach-mx5/clock-mx51-mx53.c b/arch/arm/mach-mx5/clock-mx51-mx53.c index 4cb2769..56dc34b 100644 --- a/arch/arm/mach-mx5/clock-mx51-mx53.c +++ b/arch/arm/mach-mx5/clock-mx51-mx53.c @@ -1459,13 +1459,13 @@ static struct clk_lookup mx51_lookups[] = { _REGISTER_CLOCK("imx-i2c.0", NULL, i2c1_clk) _REGISTER_CLOCK("imx-i2c.1", NULL, i2c2_clk) _REGISTER_CLOCK("imx-i2c.2", NULL, hsi2c_clk) - _REGISTER_CLOCK("mxc-ehci.0", "usb", usboh3_clk) - _REGISTER_CLOCK("mxc-ehci.0", "usb_ahb", usb_ahb_clk) - _REGISTER_CLOCK("mxc-ehci.0", "usb_phy1", usb_phy1_clk) - _REGISTER_CLOCK("mxc-ehci.1", "usb", usboh3_clk) - _REGISTER_CLOCK("mxc-ehci.1", "usb_ahb", usb_ahb_clk) - _REGISTER_CLOCK("mxc-ehci.2", "usb", usboh3_clk) - _REGISTER_CLOCK("mxc-ehci.2", "usb_ahb", usb_ahb_clk) + _REGISTER_CLOCK("ehci-imx51.0", "usb", usboh3_clk) + _REGISTER_CLOCK("ehci-imx51.0", "usb_ahb", usb_ahb_clk) + _REGISTER_CLOCK("ehci-imx51.0", "usb_phy1", usb_phy1_clk) + _REGISTER_CLOCK("ehci-imx51.1", "usb", usboh3_clk) + _REGISTER_CLOCK("ehci-imx51.1", "usb_ahb", usb_ahb_clk) + _REGISTER_CLOCK("ehci-imx51.2", "usb", usboh3_clk) + _REGISTER_CLOCK("ehci-imx51.2", "usb_ahb", usb_ahb_clk) _REGISTER_CLOCK("fsl-usb2-udc", "usb", usboh3_clk) _REGISTER_CLOCK("fsl-usb2-udc", "usb_ahb", ahb_clk) _REGISTER_CLOCK("imx-keypad", NULL, dummy_clk) diff --git a/arch/arm/plat-mxc/devices/platform-mxc-ehci.c b/arch/arm/plat-mxc/devices/platform-mxc-ehci.c index 35851d8..d339887 100644 --- a/arch/arm/plat-mxc/devices/platform-mxc-ehci.c +++ b/arch/arm/plat-mxc/devices/platform-mxc-ehci.c @@ -10,51 +10,61 @@ #include <mach/hardware.h> #include <mach/devices-common.h> -#define imx_mxc_ehci_data_entry_single(soc, _id, hs) \ +#define imx_mxc_ehci_data_entry_single(soc, _devid, _id, hs) \ { \ + .devid = _devid, \ .id = _id, \ .iobase = soc ## _USB_ ## hs ## _BASE_ADDR, \ .irq = soc ## _INT_USB_ ## hs, \ } +#define imx_mxc_ehci_data_entry(soc, devid, id, hs) \ + [id - 1] = imx_mxc_ehci_data_entry_single(soc, devid, id, hs) + #ifdef CONFIG_SOC_IMX25 const struct imx_mxc_ehci_data imx25_mxc_ehci_otg_data __initconst = - imx_mxc_ehci_data_entry_single(MX25, 0, OTG); + imx_mxc_ehci_data_entry_single(MX25, "ehci-imx25", 0, OTG); const struct imx_mxc_ehci_data imx25_mxc_ehci_hs_data __initconst = - imx_mxc_ehci_data_entry_single(MX25, 1, HS); + imx_mxc_ehci_data_entry_single(MX25, "ehci-imx25", 1, HS); #endif /* ifdef CONFIG_SOC_IMX25 */ #ifdef CONFIG_SOC_IMX27 const struct imx_mxc_ehci_data imx27_mxc_ehci_otg_data __initconst = - imx_mxc_ehci_data_entry_single(MX27, 0, OTG); + imx_mxc_ehci_data_entry_single(MX27, "ehci-imx27", 0, OTG); const struct imx_mxc_ehci_data imx27_mxc_ehci_hs_data[] __initconst = { - imx_mxc_ehci_data_entry_single(MX27, 1, HS1), - imx_mxc_ehci_data_entry_single(MX27, 2, HS2), +#define imx27_imx_mxc_ehci_data_entry(soc, _id, hs) \ + imx_mxc_ehci_data_entry(soc, "ehci-imx27", _id, hs) + imx27_imx_mxc_ehci_data_entry(MX27, 1, HS1), + imx27_imx_mxc_ehci_data_entry(MX27, 2, HS2), }; #endif /* ifdef CONFIG_SOC_IMX27 */ #ifdef CONFIG_SOC_IMX31 const struct imx_mxc_ehci_data imx31_mxc_ehci_otg_data __initconst = - imx_mxc_ehci_data_entry_single(MX31, 0, OTG); + imx_mxc_ehci_data_entry_single(MX31, "ehci-imx31", 0, OTG); const struct imx_mxc_ehci_data imx31_mxc_ehci_hs_data[] __initconst = { - imx_mxc_ehci_data_entry_single(MX31, 1, HS1), - imx_mxc_ehci_data_entry_single(MX31, 2, HS2), +#define imx31_imx_mxc_ehci_data_entry(soc, _id, hs) \ + imx_mxc_ehci_data_entry(soc, "ehci-imx31", _id, hs) + imx31_imx_mxc_ehci_data_entry(MX31, 1, HS1), + imx31_imx_mxc_ehci_data_entry(MX31, 2, HS2), }; #endif /* ifdef CONFIG_SOC_IMX31 */ #ifdef CONFIG_SOC_IMX35 const struct imx_mxc_ehci_data imx35_mxc_ehci_otg_data __initconst = - imx_mxc_ehci_data_entry_single(MX35, 0, OTG); + imx_mxc_ehci_data_entry_single(MX35, "ehci-imx35", 0, OTG); const struct imx_mxc_ehci_data imx35_mxc_ehci_hs_data __initconst = - imx_mxc_ehci_data_entry_single(MX35, 1, HS); + imx_mxc_ehci_data_entry_single(MX35, "ehci-imx35", 1, HS); #endif /* ifdef CONFIG_SOC_IMX35 */ #ifdef CONFIG_SOC_IMX51 const struct imx_mxc_ehci_data imx51_mxc_ehci_otg_data __initconst = - imx_mxc_ehci_data_entry_single(MX51, 0, OTG); + imx_mxc_ehci_data_entry_single(MX51, "ehci-imx51", 0, OTG); const struct imx_mxc_ehci_data imx51_mxc_ehci_hs_data[] __initconst = { - imx_mxc_ehci_data_entry_single(MX51, 1, HS1), - imx_mxc_ehci_data_entry_single(MX51, 2, HS2), +#define imx51_imx_mxc_ehci_data_entry(soc, _id, hs) \ + imx_mxc_ehci_data_entry(soc, "ehci-imx51", _id, hs) + imx51_imx_mxc_ehci_data_entry(MX51, 1, HS1), + imx51_imx_mxc_ehci_data_entry(MX51, 2, HS2), }; #endif /* ifdef CONFIG_SOC_IMX51 */ @@ -73,7 +83,7 @@ struct platform_device *__init imx_add_mxc_ehci( .flags = IORESOURCE_IRQ, }, }; - return imx_add_platform_device_dmamask("mxc-ehci", data->id, + return imx_add_platform_device_dmamask(data->devid, data->id, res, ARRAY_SIZE(res), pdata, sizeof(*pdata), DMA_BIT_MASK(32)); } diff --git a/arch/arm/plat-mxc/include/mach/devices-common.h b/arch/arm/plat-mxc/include/mach/devices-common.h index def9ba5..15b68e8 100644 --- a/arch/arm/plat-mxc/include/mach/devices-common.h +++ b/arch/arm/plat-mxc/include/mach/devices-common.h @@ -226,6 +226,7 @@ struct platform_device *__init imx_add_mx2_camera( #include <mach/mxc_ehci.h> struct imx_mxc_ehci_data { + const char *devid; int id; resource_size_t iobase; resource_size_t irq; diff --git a/drivers/usb/host/ehci-mxc.c b/drivers/usb/host/ehci-mxc.c index 55978fc..6f6b00a 100644 --- a/drivers/usb/host/ehci-mxc.c +++ b/drivers/usb/host/ehci-mxc.c @@ -23,6 +23,7 @@ #include <linux/usb/otg.h> #include <linux/usb/ulpi.h> #include <linux/slab.h> +#include <linux/of_device.h> #include <mach/hardware.h> #include <mach/mxc_ehci.h> @@ -31,10 +32,116 @@ #define ULPI_VIEWPORT_OFFSET 0x170 +enum mxc_ehci_type { + IMX23_EHCI, + IMX25_EHCI, + IMX28_EHCI, + IMX31_EHCI, + IMX35_EHCI, + IMX50_EHCI, + IMX51_EHCI, + IMX53_EHCI, + IMX6Q_EHCI, +}; + struct ehci_mxc_priv { struct clk *usbclk, *ahbclk, *phy1clk; struct usb_hcd *hcd; + enum mxc_ehci_type devtype; +}; + +static struct platform_device_id mxc_ehci_devtype[] = { + { + .name = "ehci-imx23", + .driver_data = IMX25_EHCI, + }, { + .name = "ehci-imx25", + .driver_data = IMX35_EHCI, + }, { + .name = "ehci-imx28", + .driver_data = IMX35_EHCI, + }, { + .name = "ehci-imx31", + .driver_data = IMX35_EHCI, + }, { + .name = "ehci-imx35", + .driver_data = IMX35_EHCI, + }, { + .name = "ehci-imx50", + .driver_data = IMX51_EHCI, + }, { + .name = "ehci-imx51", + .driver_data = IMX51_EHCI, + }, { + .name = "ehci-imx53", + .driver_data = IMX53_EHCI, + }, { + .name = "ehci-imx6q", + .driver_data = IMX6Q_EHCI, + }, { + /* sentinel */ + } }; +MODULE_DEVICE_TABLE(platform, mxc_ehci_devtype); + +static const struct of_device_id imx_ehci_dt_ids[] = { + { .compatible = "fsl,imx23-ehci", .data = &mxc_ehci_devtype[IMX23_EHCI], }, + { .compatible = "fsl,imx25-ehci", .data = &mxc_ehci_devtype[IMX25_EHCI], }, + { .compatible = "fsl,imx28-ehci", .data = &mxc_ehci_devtype[IMX28_EHCI], }, + { .compatible = "fsl,imx31-ehci", .data = &mxc_ehci_devtype[IMX31_EHCI], }, + { .compatible = "fsl,imx35-ehci", .data = &mxc_ehci_devtype[IMX35_EHCI], }, + { .compatible = "fsl,imx50-ehci", .data = &mxc_ehci_devtype[IMX50_EHCI], }, + { .compatible = "fsl,imx51-ehci", .data = &mxc_ehci_devtype[IMX51_EHCI], }, + { .compatible = "fsl,imx53-ehci", .data = &mxc_ehci_devtype[IMX53_EHCI], }, + { .compatible = "fsl,imx6q-ehci", .data = &mxc_ehci_devtype[IMX6Q_EHCI], }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, imx_ehci_dt_ids); + +static inline int is_imx23_ehci(struct ehci_mxc_priv *data) +{ + return data->devtype == IMX23_EHCI; +} + +static inline int is_imx25_ehci(struct ehci_mxc_priv *data) +{ + return data->devtype == IMX25_EHCI; +} + +static inline int is_imx28_ehci(struct ehci_mxc_priv *data) +{ + return data->devtype == IMX28_EHCI; +} + +static inline int is_imx31_ehci(struct ehci_mxc_priv *data) +{ + return data->devtype == IMX31_EHCI; +} + +static inline int is_imx35_ehci(struct ehci_mxc_priv *data) +{ + return data->devtype == IMX35_EHCI; +} + +static inline int is_imx50_ehci(struct ehci_mxc_priv *data) +{ + return data->devtype == IMX50_EHCI; +} + +static inline int is_imx51_ehci(struct ehci_mxc_priv *data) +{ + return data->devtype == IMX51_EHCI; +} + +static inline int is_imx53_ehci(struct ehci_mxc_priv *data) +{ + return data->devtype == IMX53_EHCI; +} + +static inline int is_imx6q_ehci(struct ehci_mxc_priv *data) +{ + return data->devtype == IMX6Q_EHCI; +} /* called during probe() after chip reset completes */ static int ehci_mxc_setup(struct usb_hcd *hcd) @@ -122,6 +229,8 @@ static int ehci_mxc_drv_probe(struct platform_device *pdev) struct ehci_mxc_priv *priv; struct device *dev = &pdev->dev; struct ehci_hcd *ehci; + const struct of_device_id *of_id = + of_match_device(imx_ehci_dt_ids, &pdev->dev); dev_info(&pdev->dev, "initializing i.MX USB Controller\n"); @@ -141,6 +250,9 @@ static int ehci_mxc_drv_probe(struct platform_device *pdev) ret = -ENOMEM; goto err_alloc; } + if (of_id) + pdev->id_entry = of_id->data; + priv->devtype = pdev->id_entry->driver_data; res = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!res) { @@ -173,7 +285,7 @@ static int ehci_mxc_drv_probe(struct platform_device *pdev) } clk_enable(priv->usbclk); - if (!cpu_is_mx35() && !cpu_is_mx25()) { + if (!is_imx35_ehci(priv) && !is_imx25_ehci(priv)) { priv->ahbclk = clk_get(dev, "usb_ahb"); if (IS_ERR(priv->ahbclk)) { ret = PTR_ERR(priv->ahbclk); @@ -183,7 +295,7 @@ static int ehci_mxc_drv_probe(struct platform_device *pdev) } /* "dr" device has its own clock on i.MX51 */ - if (cpu_is_mx51() && (pdev->id == 0)) { + if (is_imx51_ehci(priv) && (pdev->id == 0)) { priv->phy1clk = clk_get(dev, "usb_phy1"); if (IS_ERR(priv->phy1clk)) { ret = PTR_ERR(priv->phy1clk); @@ -192,7 +304,6 @@ static int ehci_mxc_drv_probe(struct platform_device *pdev) clk_enable(priv->phy1clk); } - /* call platform specific init function */ if (pdata->init) { ret = pdata->init(pdev); @@ -336,6 +447,7 @@ static struct platform_driver ehci_mxc_driver = { .probe = ehci_mxc_drv_probe, .remove = __exit_p(ehci_mxc_drv_remove), .shutdown = ehci_mxc_drv_shutdown, + .id_table = mxc_ehci_devtype, .driver = { .name = "mxc-ehci", },
The patch removes all the uses of cpu_is_mx(). Instead, it utilizes platform_device_id to distinguish the ehci differences among SoCs. It can be useful to imx ehci submission and device tree support later. Signed-off-by: Peter Chen <peter.chen@freescale.com> --- arch/arm/mach-imx/clock-imx25.c | 6 +- arch/arm/mach-imx/clock-imx27.c | 12 +- arch/arm/mach-imx/clock-imx31.c | 12 +- arch/arm/mach-imx/clock-imx35.c | 6 +- arch/arm/mach-mx5/clock-mx51-mx53.c | 14 ++-- arch/arm/plat-mxc/devices/platform-mxc-ehci.c | 40 +++++--- arch/arm/plat-mxc/include/mach/devices-common.h | 1 + drivers/usb/host/ehci-mxc.c | 118 ++++++++++++++++++++++- 8 files changed, 166 insertions(+), 43 deletions(-)