| Message ID | 1353506793-8354-6-git-send-email-m.grzeschik@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Nov 21, 2012 at 03:06:31PM +0100, Michael Grzeschik wrote: > From: Marc Kleine-Budde <mkl@pengutronix.de> > > This patch adds support for a second and third clock to the usbmisc driver. On > modern freescale ARM cores like the imx51, imx53 and imx6q three clocks ("ahb", > "ipg" and "per") must be enabled in order to access the USB core. The imx6q usb's clock structure is different with old mxc serials (mx3x, mx5x) due to different PHY. This patch will cause mx6q's probe fail, the message like below: imx_usb 2184000.usb: Failed to get clock, err=-2 imx_usb: probe of 2184000.usb failed with error -2 The mx5x (50, 51, 53) and mx6q's clock structure like below: mx5x: - usboh3_ipg_ahb: CCGR2(index13), used to access register (like your ahb) - usboh3_60M: CCGR2(index14), used to access register at serial phy mode as the phy is at serial mode, we need to use it at the first before the phy goes to other modes. (like your ipg) usbphy: used to transfer data (like your per) mx6q: - usboh3(Index 162 at DT), used to access register - usbphy(Index 182 for otg port at DT), at phy controller So, we may consider a way to consolidate both mx6q (mx28) and mxc (mx5x, mx3x) platforms. > > ahb - AMBA High-Performance Bus clock domain > ipg - IP-Bus Gate clock domain > per - Peripheral clock domain > > Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> > Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de> > --- > Changes since v1: > * changed patch description > > drivers/usb/chipidea/usbmisc_imx.c | 60 +++++++++++++++++++++++++++--------- > 1 file changed, 45 insertions(+), 15 deletions(-) > > diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c > index d0c7a03..e090e0f 100644 > --- a/drivers/usb/chipidea/usbmisc_imx.c > +++ b/drivers/usb/chipidea/usbmisc_imx.c > @@ -24,7 +24,9 @@ > struct imx_usbmisc { > void __iomem *base; > spinlock_t lock; > - struct clk *clk; > + struct clk *clk_ahb; > + struct clk *clk_ipg; > + struct clk *clk_per; > struct usbmisc_usb_device usbdev[USB_DEV_MAX]; > const struct usbmisc_ops *ops; > }; > @@ -104,38 +106,66 @@ static int __devinit usbmisc_imx_probe(struct platform_device *pdev) > if (!data->base) > return -EADDRNOTAVAIL; > > - data->clk = devm_clk_get(&pdev->dev, NULL); > - if (IS_ERR(data->clk)) { > + data->clk_ahb = devm_clk_get(&pdev->dev, "ahb"); > + if (IS_ERR(data->clk_ahb)) { > dev_err(&pdev->dev, > - "failed to get clock, err=%ld\n", PTR_ERR(data->clk)); > - return PTR_ERR(data->clk); > + "failed to get ahb clock, err=%ld\n", PTR_ERR(data->clk_ahb)); > + return PTR_ERR(data->clk_ahb); > } > > - ret = clk_prepare_enable(data->clk); > - if (ret) { > + data->clk_ipg = devm_clk_get(&pdev->dev, "ipg"); > + if (IS_ERR(data->clk_ipg)) { > dev_err(&pdev->dev, > - "clk_prepare_enable failed, err=%d\n", ret); > - return ret; > + "failed to get ipg clock, err=%ld\n", PTR_ERR(data->clk_ipg)); > + return PTR_ERR(data->clk_ipg); > } > > + data->clk_per = devm_clk_get(&pdev->dev, "per"); > + if (IS_ERR(data->clk_per)) { > + dev_err(&pdev->dev, > + "failed to get per clock, err=%ld\n", PTR_ERR(data->clk_per)); > + return PTR_ERR(data->clk_per); > + } > + > + ret = clk_prepare_enable(data->clk_ahb); > + if (ret) > + return ret; > + > + ret = clk_prepare_enable(data->clk_ipg); > + if (ret) > + goto err_ipg_failed; > + > + ret = clk_prepare_enable(data->clk_per); > + if (ret) > + goto err_per_failed; > + > tmp_dev = (struct of_device_id *) > of_match_device(usbmisc_imx_dt_ids, &pdev->dev); > data->ops = (const struct usbmisc_ops *)tmp_dev->data; > usbmisc = data; > ret = usbmisc_set_ops(data->ops); > - if (ret) { > - usbmisc = NULL; > - clk_disable_unprepare(data->clk); > - return ret; > - } > + if (ret) > + goto err_set_ops_failed; > > return 0; > + > + err_set_ops_failed: > + usbmisc = NULL; > + clk_disable_unprepare(data->clk_per); > + err_per_failed: > + clk_disable_unprepare(data->clk_ipg); > + err_ipg_failed: > + clk_disable_unprepare(data->clk_ahb); > + > + return ret; > } > > static int __devexit usbmisc_imx_remove(struct platform_device *pdev) > { > usbmisc_unset_ops(usbmisc->ops); > - clk_disable_unprepare(usbmisc->clk); > + clk_disable_unprepare(usbmisc->clk_per); > + clk_disable_unprepare(usbmisc->clk_ipg); > + clk_disable_unprepare(usbmisc->clk_ahb); > usbmisc = NULL; > return 0; > } > -- > 1.7.10.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
On Fri, Nov 23, 2012 at 02:51:16PM +0800, Peter Chen wrote: > On Wed, Nov 21, 2012 at 03:06:31PM +0100, Michael Grzeschik wrote: > > From: Marc Kleine-Budde <mkl@pengutronix.de> > > > > This patch adds support for a second and third clock to the usbmisc driver. On > > modern freescale ARM cores like the imx51, imx53 and imx6q three clocks ("ahb", > > "ipg" and "per") must be enabled in order to access the USB core. > > The imx6q usb's clock structure is different with old mxc serials (mx3x, mx5x) > due to different PHY. This patch will cause mx6q's probe fail, the message > like below: > > imx_usb 2184000.usb: Failed to get clock, err=-2 > imx_usb: probe of 2184000.usb failed with error -2 > > The mx5x (50, 51, 53) and mx6q's clock structure like below: > > mx5x: > - usboh3_ipg_ahb: CCGR2(index13), used to access register > (like your ahb) That is probably the one, which already gets enabled inside the ci13xxx_imx code. > - usboh3_60M: CCGR2(index14), used to access register at serial phy mode > as the phy is at serial mode, we need to use it at the first before the phy > goes to other modes. > (like your ipg) > usbphy: used to transfer data > (like your per) The "per" clk is a different clk than the usbphy. We have an usbphy clk aswell on the mx5x. As discussed before [1], we need a solution to enable the usbphy clk. The current approach is to let the nop-xceiver enable this. [1] http://www.spinics.net/lists/linux-usb/msg74489.html > mx6q: > - usboh3(Index 162 at DT), used to access register > - usbphy(Index 182 for otg port at DT), at phy controller > So, we may consider a way to consolidate both mx6q (mx28) and mxc (mx5x, mx3x) > platforms. Yes, we will discuss a real clktree solution in the usbmisc driver for the different cores. The first thoughts are, that the clk configuration has completely to be done inside the usbmisc driver. For this, the usbmisc needs to become a hard dependency for the ci13xxx_imx glue code. Regards, Michael
On Fri, Nov 23, 2012 at 05:20:51PM +0100, Michael Grzeschik wrote: > > usbphy: used to transfer data > > (like your per) > > The "per" clk is a different clk than the usbphy. We have an usbphy clk > aswell on the mx5x. As discussed before [1], we need a solution to enable > the usbphy clk. The current approach is to let the nop-xceiver enable > this. > > [1] http://www.spinics.net/lists/linux-usb/msg74489.html It does not matter which the name is, this clock is related to usb 480M or its source. We use nop-xceiver is just to create struct usb_otg and usb_phy. Clock information can be at imx specific file like cil13xxx_imx. > > > mx6q: > > - usboh3(Index 162 at DT), used to access register > > - usbphy(Index 182 for otg port at DT), at phy controller > > > So, we may consider a way to consolidate both mx6q (mx28) and mxc (mx5x, mx3x) > > platforms. > > Yes, we will discuss a real clktree solution in the usbmisc driver for > the different cores. The first thoughts are, that the clk configuration > has completely to be done inside the usbmisc driver. For this, the > usbmisc needs to become a hard dependency for the ci13xxx_imx glue code. For my mind, imx usb clock handling is better only at place, that is cil13xxx_imx. We can have imx_clk_enable and imx_clk_disable in it, and handling clk in imx_clk_enable using id_table or what else platform specific. > > Regards, > Michael > > -- > 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 Sun, Nov 25, 2012 at 05:18:35PM +0800, Peter Chen wrote: > On Fri, Nov 23, 2012 at 05:20:51PM +0100, Michael Grzeschik wrote: > > > usbphy: used to transfer data > > > (like your per) > > > > The "per" clk is a different clk than the usbphy. We have an usbphy clk > > aswell on the mx5x. As discussed before [1], we need a solution to enable > > the usbphy clk. The current approach is to let the nop-xceiver enable > > this. > > > > [1] http://www.spinics.net/lists/linux-usb/msg74489.html > It does not matter which the name is, this clock is related to usb 480M or > its source. We use nop-xceiver is just to create struct usb_otg > and usb_phy. Clock information can be at imx specific file like > cil13xxx_imx. I think the clock task needs to become a sepreate topic, apart from this series. > > > > > mx6q: > > > - usboh3(Index 162 at DT), used to access register > > > - usbphy(Index 182 for otg port at DT), at phy controller > > > > > So, we may consider a way to consolidate both mx6q (mx28) and mxc (mx5x, mx3x) > > > platforms. > > > > Yes, we will discuss a real clktree solution in the usbmisc driver for > > the different cores. The first thoughts are, that the clk configuration > > has completely to be done inside the usbmisc driver. For this, the > > usbmisc needs to become a hard dependency for the ci13xxx_imx glue code. > > For my mind, imx usb clock handling is better only at place, that is > cil13xxx_imx. We can have imx_clk_enable and imx_clk_disable in it, > and handling clk in imx_clk_enable using id_table or what else platform > specific. As already mentioned, the clock layout differs from SoC to SoC. Therefore the usbmisc can become a mandatory instance over the ci13xxx_imx instances in all imx cases. This controlling interface can define the special clock setups and connection types for the underlying usb cores. I will send the next series without this clock handling. Regards, Michael
diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c index d0c7a03..e090e0f 100644 --- a/drivers/usb/chipidea/usbmisc_imx.c +++ b/drivers/usb/chipidea/usbmisc_imx.c @@ -24,7 +24,9 @@ struct imx_usbmisc { void __iomem *base; spinlock_t lock; - struct clk *clk; + struct clk *clk_ahb; + struct clk *clk_ipg; + struct clk *clk_per; struct usbmisc_usb_device usbdev[USB_DEV_MAX]; const struct usbmisc_ops *ops; }; @@ -104,38 +106,66 @@ static int __devinit usbmisc_imx_probe(struct platform_device *pdev) if (!data->base) return -EADDRNOTAVAIL; - data->clk = devm_clk_get(&pdev->dev, NULL); - if (IS_ERR(data->clk)) { + data->clk_ahb = devm_clk_get(&pdev->dev, "ahb"); + if (IS_ERR(data->clk_ahb)) { dev_err(&pdev->dev, - "failed to get clock, err=%ld\n", PTR_ERR(data->clk)); - return PTR_ERR(data->clk); + "failed to get ahb clock, err=%ld\n", PTR_ERR(data->clk_ahb)); + return PTR_ERR(data->clk_ahb); } - ret = clk_prepare_enable(data->clk); - if (ret) { + data->clk_ipg = devm_clk_get(&pdev->dev, "ipg"); + if (IS_ERR(data->clk_ipg)) { dev_err(&pdev->dev, - "clk_prepare_enable failed, err=%d\n", ret); - return ret; + "failed to get ipg clock, err=%ld\n", PTR_ERR(data->clk_ipg)); + return PTR_ERR(data->clk_ipg); } + data->clk_per = devm_clk_get(&pdev->dev, "per"); + if (IS_ERR(data->clk_per)) { + dev_err(&pdev->dev, + "failed to get per clock, err=%ld\n", PTR_ERR(data->clk_per)); + return PTR_ERR(data->clk_per); + } + + ret = clk_prepare_enable(data->clk_ahb); + if (ret) + return ret; + + ret = clk_prepare_enable(data->clk_ipg); + if (ret) + goto err_ipg_failed; + + ret = clk_prepare_enable(data->clk_per); + if (ret) + goto err_per_failed; + tmp_dev = (struct of_device_id *) of_match_device(usbmisc_imx_dt_ids, &pdev->dev); data->ops = (const struct usbmisc_ops *)tmp_dev->data; usbmisc = data; ret = usbmisc_set_ops(data->ops); - if (ret) { - usbmisc = NULL; - clk_disable_unprepare(data->clk); - return ret; - } + if (ret) + goto err_set_ops_failed; return 0; + + err_set_ops_failed: + usbmisc = NULL; + clk_disable_unprepare(data->clk_per); + err_per_failed: + clk_disable_unprepare(data->clk_ipg); + err_ipg_failed: + clk_disable_unprepare(data->clk_ahb); + + return ret; } static int __devexit usbmisc_imx_remove(struct platform_device *pdev) { usbmisc_unset_ops(usbmisc->ops); - clk_disable_unprepare(usbmisc->clk); + clk_disable_unprepare(usbmisc->clk_per); + clk_disable_unprepare(usbmisc->clk_ipg); + clk_disable_unprepare(usbmisc->clk_ahb); usbmisc = NULL; return 0; }