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: cphe...@gmail.com
> Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
> Cc: linux-...@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Andrey Smirnov <andrew.smir...@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.
--
Best Regards,
Peter Chen
