> -----Original Message----- > From: Sascha Hauer [mailto:s.hauer@xxxxxxxxxxxxxx] > Sent: Wednesday, July 27, 2011 4:03 PM > To: Lin Tony-B19295 > Cc: linux-usb@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; > stern@xxxxxxxxxxxxxxxxxxx; koen.beel.barco@xxxxxxxxx > Subject: Re: [PATCH v3 5/6] ARM: mxs: add usb phy operations > > On Wed, Jul 27, 2011 at 01:29:33PM +0800, Tony Lin wrote: > > add usb phy register definitions and functions usb host driver will > > use these callback functions to initialize usb phy and change working > > mode > > > > Signed-off-by: Tony Lin <tony.lin@xxxxxxxxxxxxx> > > --- > > arch/arm/mach-mxs/Kconfig | 1 + > > arch/arm/mach-mxs/Makefile | 1 + > > arch/arm/mach-mxs/mxs_usb.c | 288 > +++++++++++++++++++++++++++++++++++ > > arch/arm/mach-mxs/regs-usbphy-mxs.h | 240 > > +++++++++++++++++++++++++++++ > > 4 files changed, 530 insertions(+), 0 deletions(-) > > > > +} > > +static int fsl_usb_host_init(struct platform_device *pdev) { > > + struct mxc_usbh_platform_data *pdata = pdev->dev.platform_data; > > + struct mxs_usb_private_date *ppriv = pdata->ppriv; > > + > > + ppriv->phy_regs = ioremap(MX28_USBPHY1_BASE_ADDR, SZ_8K); > > + ppriv->ctrl_regs = ioremap(MX28_USBCTRL1_BASE_ADDR, SZ_8K); > > Did I mention that the i.MX28 has *two* USB controllers? > Yes, i.MX28 has two USB, one is pure USB host, another is USB OTG. I hard code USB1 here just because we want another USB OTG to be a pure USB device. Anyway, I could make it more flexible in later patch. thanks > > + if (!ppriv->phy_regs || !ppriv->ctrl_regs) { > > + iounmap(ppriv->phy_regs); > > + iounmap(ppriv->ctrl_regs); > > + return -ENOMEM; > > + } > > So you call iounmap(0) here. > Yes, iounmap will ignore NULL pointers, right? > > + ppriv->usb_clk = clk_get(&pdev->dev, "usb1"); > > + if (IS_ERR(ppriv->usb_clk)) { > > + iounmap(ppriv->phy_regs); > > + iounmap(ppriv->ctrl_regs); > > + return PTR_ERR(ppriv->usb_clk); > > In the kernel community we are not afraid of 'goto'. > Ok. Will change it. > > + } > > + clk_enable(ppriv->usb_clk); > > + > > + ppriv->usb_phy_clk = clk_get(&pdev->dev, "usb1_phy"); > > + if (IS_ERR(ppriv->usb_phy_clk)) { > > + clk_disable(ppriv->usb_clk); > > + clk_put(ppriv->usb_clk); > > + iounmap(ppriv->phy_regs); > > + iounmap(ppriv->ctrl_regs); > > + return PTR_ERR(ppriv->usb_phy_clk); > > + } > > + clk_enable(ppriv->usb_phy_clk); > > + > > + phy_clock_gate(ppriv, true); > > + return fsl_usbh_init(pdev); > > +} > > + > > +static int fsl_usb_host_uninit(struct platform_device *pdev) { > > + struct mxc_usbh_platform_data *pdata = pdev->dev.platform_data; > > + struct mxs_usb_private_date *ppriv = pdata->ppriv; > > + > > + phy_clock_gate(ppriv, false); > > + clk_disable(ppriv->usb_phy_clk); > > + clk_put(ppriv->usb_phy_clk); > > + clk_disable(ppriv->usb_clk); > > + clk_put(ppriv->usb_clk); > > + iounmap(ppriv->phy_regs); > > + iounmap(ppriv->ctrl_regs); > > + > > + return 0; > > +} > > + > > +static struct mxs_usb_private_date usbh_private = { > > + .internal_phy_clk_already_on = 0, > > +}; > > unitialized fields in static initializers are zero anyway. > Remove all this ppriv crap. clk_enable/disable will do the reference > counting for you. > Ok. Accepted. Thanks Sascha. > 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 > | -- 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
