Messages in this thread | | | From | Andrey Smirnov <> | Date | Tue, 9 May 2017 11:12:15 -0700 | Subject | Re: [PATCH] usb: chipidea: imx: Do not access CLKONOFF on i.MX51 |
| |
On Mon, May 8, 2017 at 5:47 PM, Peter Chen <hzpeterchen@gmail.com> wrote: > On Mon, May 08, 2017 at 07:15:01AM -0700, Andrey Smirnov wrote: >> Unlike i.MX53, i.MX51's USBOH3 register file does not implemenent >> registers past offset 0x018, which includes >> MX53_USB_CLKONOFF_CTRL_OFFSET and trying to access that register on >> said platform results in external abort. >> >> Fix it by enabling CLKONOFF accessing codepath only for i.MX53. >> >> Fixes 3be3251db0887f721f9c110e1966900922fc4ff4 >> ("usb: chipidea: imx: Disable internal 60Mhz clock with ULPI PHY") >> >> Cc: cphealy@gmail.com >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >> Cc: linux-usb@vger.kernel.org >> Cc: linux-kernel@vger.kernel.org >> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com> >> --- >> drivers/usb/chipidea/usbmisc_imx.c | 44 +++++++++++++++++++++++++++++--------- >> 1 file changed, 34 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c >> index e77a4ed..fc21f92 100644 >> --- a/drivers/usb/chipidea/usbmisc_imx.c >> +++ b/drivers/usb/chipidea/usbmisc_imx.c >> @@ -195,7 +195,7 @@ static int usbmisc_imx27_init(struct imx_usbmisc_data *data) >> return 0; >> } >> >> -static int usbmisc_imx53_init(struct imx_usbmisc_data *data) >> +static int usbmisc_imx5x_init(struct imx_usbmisc_data *data, bool has_clkonoff) >> { >> struct imx_usbmisc *usbmisc = dev_get_drvdata(data->dev); >> void __iomem *reg = NULL; >> @@ -242,10 +242,15 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) >> val = readl(reg) | MX53_USB_UHx_CTRL_WAKE_UP_EN >> | MX53_USB_UHx_CTRL_ULPI_INT_EN; >> writel(val, reg); >> - /* Disable internal 60Mhz clock */ >> - reg = usbmisc->base + MX53_USB_CLKONOFF_CTRL_OFFSET; >> - val = readl(reg) | MX53_USB_CLKONOFF_CTRL_H2_INT60CKOFF; >> - writel(val, reg); >> + if (has_clkonoff) { >> + /* Disable internal 60Mhz clock */ >> + reg = usbmisc->base + >> + MX53_USB_CLKONOFF_CTRL_OFFSET; >> + val = readl(reg) | >> + MX53_USB_CLKONOFF_CTRL_H2_INT60CKOFF; >> + writel(val, reg); >> + } >> + >> } >> if (data->disable_oc) { >> reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; >> @@ -267,10 +272,15 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) >> val = readl(reg) | MX53_USB_UHx_CTRL_WAKE_UP_EN >> | MX53_USB_UHx_CTRL_ULPI_INT_EN; >> writel(val, reg); >> - /* Disable internal 60Mhz clock */ >> - reg = usbmisc->base + MX53_USB_CLKONOFF_CTRL_OFFSET; >> - val = readl(reg) | MX53_USB_CLKONOFF_CTRL_H3_INT60CKOFF; >> - writel(val, reg); >> + >> + if (has_clkonoff) { >> + /* Disable internal 60Mhz clock */ >> + reg = usbmisc->base + >> + MX53_USB_CLKONOFF_CTRL_OFFSET; >> + val = readl(reg) | >> + MX53_USB_CLKONOFF_CTRL_H3_INT60CKOFF; >> + writel(val, reg); >> + } >> } >> if (data->disable_oc) { >> reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET; >> @@ -285,6 +295,16 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) >> return 0; >> } >> >> +static int usbmisc_imx51_init(struct imx_usbmisc_data *data) >> +{ >> + return usbmisc_imx5x_init(data, false); >> +} >> + >> +static int usbmisc_imx53_init(struct imx_usbmisc_data *data) >> +{ >> + return usbmisc_imx5x_init(data, true); >> +} >> + >> static int usbmisc_imx6q_set_wakeup >> (struct imx_usbmisc_data *data, bool enabled) >> { >> @@ -456,6 +476,10 @@ static const struct usbmisc_ops imx27_usbmisc_ops = { >> .init = usbmisc_imx27_init, >> }; >> >> +static const struct usbmisc_ops imx51_usbmisc_ops = { >> + .init = usbmisc_imx51_init, >> +}; >> + >> static const struct usbmisc_ops imx53_usbmisc_ops = { >> .init = usbmisc_imx53_init, >> }; >> @@ -536,7 +560,7 @@ static const struct of_device_id usbmisc_imx_dt_ids[] = { >> }, >> { >> .compatible = "fsl,imx51-usbmisc", >> - .data = &imx53_usbmisc_ops, >> + .data = &imx51_usbmisc_ops, >> }, >> { >> .compatible = "fsl,imx53-usbmisc", > > Thanks for fixing it. > > I prefer using is_imx53_usbmisc stands for if it is imx53 usbmisc > at usbmisc_imx53_init to fix it, you can prefer to: > drivers/usb/phy/phy-mxs-usb.c. >
OK, sounds good, I'll update and resubmit the patch.
Thanks, Andrey Smirnov
| |