On 02.07.2015 10:40, Peter Chen wrote:
> On Wed, Jul 01, 2015 at 12:06:19PM -0300, Fabio Estevam wrote:
>> From: Fabio Estevam <fabio.estevam@xxxxxxxxxxxxx>
>>
>> This reverts commit 73dea4a912b2bfe955305de4891018f9e71e399d.
>>
>> Since commit 73dea4a912b2("usb: chipidea: usbmisc_imx: delete clock
>> information") it is not possible to boot a kernel on a mx27pdk:
>>
>> usbcore: registered new interface driver usb-storage
>> Unhandled fault: external abort on non-linefetch (0x008) at 0xf4424600
>> pgd = c0004000
>> [f4424600] *pgd=10000452(bad)
>> Internal error: : 8 [#1] PREEMPT ARM
>> Modules linked in:
>> CPU: 0 PID: 1 Comm: swapper Not tainted 4.1.0-next-20150701-dirty #3089
>> Hardware name: Freescale i.MX27 (Device Tree Support)
>> task: c7832b60 ti: c783e000 task.ti: c783e000
>> PC is at usbmisc_imx27_init+0x4c/0xbc
>> LR is at usbmisc_imx27_init+0x40/0xbc
>> pc : [<c03cb5c0>] lr : [<c03cb5b4>] psr: 60000093
>> sp : c783fe08 ip : 00000000 fp : 00000000
>> r10: c0576434 r9 : 0000009c r8 : c7a773a0
>> r7 : 01000000 r6 : 60000013 r5 : c7a776f0 r4 : c7a773f0
>> r3 : f4424600 r2 : 00000000 r1 : 00000001 r0 : 00000001
>> Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment kernel
>> Control: 0005317f Table: a0004000 DAC: 00000017
>> Process swapper (pid: 1, stack limit = 0xc783e190)
>> Stack: (0xc783fe08 to 0xc7840000)
>>
>> This commit also breaks the booting of imx6q-udoo board [1], so revert
>> it to avoid these regressions.
>>
>> [1] http://www.spinics.net/lists/linux-usb/msg125597.html
>>
>> Signed-off-by: Fabio Estevam <fabio.estevam@xxxxxxxxxxxxx>
>> ---
>> drivers/usb/chipidea/usbmisc_imx.c | 19 +++++++++++++++++++
>> 1 file changed, 19 insertions(+)
>>
>> diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
>> index 140945c..8f4cd92 100644
>> --- a/drivers/usb/chipidea/usbmisc_imx.c
>> +++ b/drivers/usb/chipidea/usbmisc_imx.c
>> @@ -11,6 +11,7 @@
>>
>> #include <linux/module.h>
>> #include <linux/of_platform.h>
>> +#include <linux/clk.h>
>> #include <linux/err.h>
>> #include <linux/io.h>
>> #include <linux/delay.h>
>> @@ -83,6 +84,7 @@ struct usbmisc_ops {
>> struct imx_usbmisc {
>> void __iomem *base;
>> spinlock_t lock;
>> + struct clk *clk;
>> const struct usbmisc_ops *ops;
>> };
>>
>> @@ -426,6 +428,7 @@ static int usbmisc_imx_probe(struct platform_device *pdev)
>> {
>> struct resource *res;
>> struct imx_usbmisc *data;
>> + int ret;
>> struct of_device_id *tmp_dev;
>>
>> data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
>> @@ -439,6 +442,20 @@ static int usbmisc_imx_probe(struct platform_device *pdev)
>> if (IS_ERR(data->base))
>> return PTR_ERR(data->base);
>>
>> + data->clk = devm_clk_get(&pdev->dev, NULL);
>> + if (IS_ERR(data->clk)) {
>> + dev_err(&pdev->dev,
>> + "failed to get clock, err=%ld\n", PTR_ERR(data->clk));
>> + return PTR_ERR(data->clk);
>> + }
>> +
>> + ret = clk_prepare_enable(data->clk);
>> + if (ret) {
>> + dev_err(&pdev->dev,
>> + "clk_prepare_enable failed, err=%d\n", ret);
>> + return ret;
>> + }
>> +
>> tmp_dev = (struct of_device_id *)
>> of_match_device(usbmisc_imx_dt_ids, &pdev->dev);
>> data->ops = (const struct usbmisc_ops *)tmp_dev->data;
>> @@ -449,6 +466,8 @@ static int usbmisc_imx_probe(struct platform_device *pdev)
>>
>> static int usbmisc_imx_remove(struct platform_device *pdev)
>> {
>> + struct imx_usbmisc *usbmisc = dev_get_drvdata(&pdev->dev);
>> + clk_disable_unprepare(usbmisc->clk);
>> return 0;
>> }
>>
>> --
>> 1.9.1
>>
>
> Sorry, I can't accept it, and it will remain the clock on, and break
> runtime pm feature.
>
> In fact, the core and non-core use the same clocks, and at imx27, it
> may need two or three clocks to let the controller work for imx27.
> I can accept handle three clocks like below at ci_hdrc_imx.c to fix
> this issue.
>
> arch/arm/boot/dts/imx25.dtsi
> clocks = <&clks 9>, <&clks 70>, <&clks 8>;
> clock-names = "ipg", "ahb", "per";
>
This would also work for UDOO board (one clock then can be used for hub),
but there might be a problem if such clock would be disabled by
runtime PM.
The hub chip (USB2514) datasheet is silent whether the clock could
be stopped, for example during suspend, so one has to assume that
it shouldn't be done.
Best regards,
Maciej Szmigiero
--
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
