[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Pine.LNX.4.44L0.1901081118121.1355-100000@iolanthe.rowland.org>
Date: Tue, 8 Jan 2019 11:20:01 -0500 (EST)
From: Alan Stern <stern@...land.harvard.edu>
To: Ran Wang <ran.wang_1@....com>
cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/3] usb: ehci: fsl: Update register accessing for
arm/arm64 platforms
On Tue, 8 Jan 2019, Ran Wang wrote:
> arm/arm64's io.h doesn't define clrbits32() and clrsetbits_be32(), which
> causing compile failure on some Layerscape Platforms (such as LS1021A and
> LS2012A which also integrates FSL EHCI controller). So use
> ioread32be()/iowrite32be() instead to make it workable on both
> powerpc and arm.
>
> Signed-off-by: Ran Wang <ran.wang_1@....com>
> ---
> drivers/usb/host/ehci-fsl.c | 64 ++++++++++++++++++++++++++++---------------
> 1 files changed, 42 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c
> index 0a9fd20..59ebe1b 100644
> --- a/drivers/usb/host/ehci-fsl.c
> +++ b/drivers/usb/host/ehci-fsl.c
> @@ -23,6 +23,7 @@
> #include <linux/platform_device.h>
> #include <linux/fsl_devices.h>
> #include <linux/of_platform.h>
> +#include <linux/io.h>
>
> #include "ehci.h"
> #include "ehci-fsl.h"
> @@ -50,6 +51,7 @@ static int fsl_ehci_drv_probe(struct platform_device *pdev)
> struct resource *res;
> int irq;
> int retval;
> + u32 tmp;
>
> pr_debug("initializing FSL-SOC USB Controller\n");
>
> @@ -114,18 +116,23 @@ static int fsl_ehci_drv_probe(struct platform_device *pdev)
> }
>
> /* Enable USB controller, 83xx or 8536 */
> - if (pdata->have_sysif_regs && pdata->controller_ver < FSL_USB_VER_1_6)
> - clrsetbits_be32(hcd->regs + FSL_SOC_USB_CTRL,
> - CONTROL_REGISTER_W1C_MASK, 0x4);
> -
> + if (pdata->have_sysif_regs && pdata->controller_ver < FSL_USB_VER_1_6) {
> + tmp = ioread32be(hcd->regs + FSL_SOC_USB_CTRL);
> + tmp &= ~CONTROL_REGISTER_W1C_MASK;
> + tmp |= 0x4;
> + iowrite32be(tmp, hcd->regs + FSL_SOC_USB_CTRL);
> + }
> /*
> * Enable UTMI phy and program PTS field in UTMI mode before asserting
> * controller reset for USB Controller version 2.5
> */
> if (pdata->has_fsl_erratum_a007792) {
> - clrsetbits_be32(hcd->regs + FSL_SOC_USB_CTRL,
> - CONTROL_REGISTER_W1C_MASK, CTRL_UTMI_PHY_EN);
> - writel(PORT_PTS_UTMI, hcd->regs + FSL_SOC_USB_PORTSC1);
> + tmp = ioread32be(hcd->regs + FSL_SOC_USB_CTRL);
> + tmp &= ~CONTROL_REGISTER_W1C_MASK;
> + tmp |= CTRL_UTMI_PHY_EN;
> + iowrite32be(tmp, hcd->regs + FSL_SOC_USB_CTRL);
> +
> + iowrite32be(PORT_PTS_UTMI, hcd->regs + FSL_SOC_USB_PORTSC1);
Why do you change this writel() into iowrite32be() but leave other
instances of writel() unchanged? Was this a mistake?
> }
>
> /* Don't need to set host mode here. It will be done by tdi_reset() */
> @@ -174,7 +181,7 @@ static int ehci_fsl_setup_phy(struct usb_hcd *hcd,
> enum fsl_usb2_phy_modes phy_mode,
> unsigned int port_offset)
> {
> - u32 portsc;
> + u32 portsc, tmp;
> struct ehci_hcd *ehci = hcd_to_ehci(hcd);
> void __iomem *non_ehci = hcd->regs;
> struct device *dev = hcd->self.controller;
> @@ -192,11 +199,16 @@ static int ehci_fsl_setup_phy(struct usb_hcd *hcd,
> case FSL_USB2_PHY_ULPI:
> if (pdata->have_sysif_regs && pdata->controller_ver) {
> /* controller version 1.6 or above */
> - clrbits32(non_ehci + FSL_SOC_USB_CTRL,
> - CONTROL_REGISTER_W1C_MASK | UTMI_PHY_EN);
> - clrsetbits_be32(non_ehci + FSL_SOC_USB_CTRL,
> - CONTROL_REGISTER_W1C_MASK,
> - ULPI_PHY_CLK_SEL | USB_CTRL_USB_EN);
> + /* turn off UTMI PHY first */
> + tmp = ioread32be(non_ehci + FSL_SOC_USB_CTRL);
> + tmp &= ~(CONTROL_REGISTER_W1C_MASK | UTMI_PHY_EN);
> + iowrite32be(tmp, non_ehci + FSL_SOC_USB_CTRL);
> +
> + /* then turn on ULPI and enable USB controller */
> + tmp = ioread32be(non_ehci + FSL_SOC_USB_CTRL);
> + tmp &= ~(CONTROL_REGISTER_W1C_MASK);
Unnecessary parens.
> + tmp |= ULPI_PHY_CLK_SEL | USB_CTRL_USB_EN;
> + iowrite32be(tmp, non_ehci + FSL_SOC_USB_CTRL);
> }
> portsc |= PORT_PTS_ULPI;
> break;
> @@ -210,16 +222,21 @@ static int ehci_fsl_setup_phy(struct usb_hcd *hcd,
> case FSL_USB2_PHY_UTMI_DUAL:
> if (pdata->have_sysif_regs && pdata->controller_ver) {
> /* controller version 1.6 or above */
> - clrsetbits_be32(non_ehci + FSL_SOC_USB_CTRL,
> - CONTROL_REGISTER_W1C_MASK, UTMI_PHY_EN);
> + tmp = ioread32be(non_ehci + FSL_SOC_USB_CTRL);
> + tmp &= ~(CONTROL_REGISTER_W1C_MASK);
Unnecessary parens.
> + tmp |= UTMI_PHY_EN;
> + iowrite32be(tmp, non_ehci + FSL_SOC_USB_CTRL);
> +
> mdelay(FSL_UTMI_PHY_DLY); /* Delay for UTMI PHY CLK to
> become stable - 10ms*/
> }
> /* enable UTMI PHY */
> - if (pdata->have_sysif_regs)
> - clrsetbits_be32(non_ehci + FSL_SOC_USB_CTRL,
> - CONTROL_REGISTER_W1C_MASK,
> - CTRL_UTMI_PHY_EN);
> + if (pdata->have_sysif_regs) {
> + tmp = ioread32be(non_ehci + FSL_SOC_USB_CTRL);
> + tmp &= ~CONTROL_REGISTER_W1C_MASK;
> + tmp |= CTRL_UTMI_PHY_EN;
> + iowrite32be(tmp, non_ehci + FSL_SOC_USB_CTRL);
> + }
> portsc |= PORT_PTS_UTMI;
> break;
> case FSL_USB2_PHY_NONE:
> @@ -241,9 +258,12 @@ static int ehci_fsl_setup_phy(struct usb_hcd *hcd,
>
> ehci_writel(ehci, portsc, &ehci->regs->port_status[port_offset]);
>
> - if (phy_mode != FSL_USB2_PHY_ULPI && pdata->have_sysif_regs)
> - clrsetbits_be32(non_ehci + FSL_SOC_USB_CTRL,
> - CONTROL_REGISTER_W1C_MASK, USB_CTRL_USB_EN);
> + if (phy_mode != FSL_USB2_PHY_ULPI && pdata->have_sysif_regs) {
> + tmp = ioread32be(non_ehci + FSL_SOC_USB_CTRL);
> + tmp &= ~CONTROL_REGISTER_W1C_MASK;
> + tmp |= USB_CTRL_USB_EN;
> + iowrite32be(tmp, non_ehci + FSL_SOC_USB_CTRL);
> + }
>
> return 0;
> }
Alan Stern
Powered by blists - more mailing lists