Re: [PATCH v2 1/4] usb: chipidea: add flag for imx hsic implementation

From: Schrempf Frieder <frieder.schrempf@kontron.De>
To: Peter Chen <peter.chen@nxp.com>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>
Cc: dl-linux-imx <linux-imx@nxp.com>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"frieder.schrempf@exceet.de" <frieder.schrempf@exceet.de>,
	"festevam@gmail.com" <festevam@gmail.com>
Subject: Re: [PATCH v2 1/4] usb: chipidea: add flag for imx hsic implementation
Date: Mon, 5 Nov 2018 08:52:26 +0000	[thread overview]
Message-ID: <38c1da4a-6fa7-783a-b600-3be7f8ea42a4@kontron.de> (raw)
In-Reply-To: <20181022054031.14027-2-peter.chen@nxp.com>

On 22.10.18 07:42, Peter Chen wrote:
> NXP (Freecale) imx HSIC design has some special requirements, add
> some flags at host code to handle them.
> 
> Signed-off-by: Peter Chen <peter.chen@nxp.com>
> ---
>   drivers/usb/chipidea/host.c  | 24 ++++++++++++++++++++++++
>   include/linux/usb/chipidea.h |  3 +++
>   2 files changed, 27 insertions(+)
> 
> diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> index d858a82c4f44..d74a13d7c21c 100644
> --- a/drivers/usb/chipidea/host.c
> +++ b/drivers/usb/chipidea/host.c
> @@ -28,6 +28,20 @@ struct ehci_ci_priv {
>   	struct regulator *reg_vbus;
>   };
>   
> +/* This function is used to override WKCN, WKDN, and WKOC */
> +static void ci_ehci_override_wakeup_flag(struct ehci_hcd *ehci,

I think it would be better to rename this function to something like 
ci_ehci_clear_reg_bits() as it is completely generic and just clears 
some bits in reg.

And also remove the argument "set", as currently the only usage of this 
function is with set=false.

> +		u32 __iomem *reg, u32 flags, bool set)
> +{
> +	u32 val = ehci_readl(ehci, reg);
> +
> +	if (set)
> +		val |= flags;
> +	else
> +		val &= ~flags;
> +
> +	ehci_writel(ehci, val, reg);
> +}
> +
>   static int ehci_ci_portpower(struct usb_hcd *hcd, int portnum, bool enable)
>   {
>   	struct ehci_hcd *ehci = hcd_to_ehci(hcd);
> @@ -170,6 +184,11 @@ static int host_start(struct ci_hdrc *ci)
>   			otg->host = &hcd->self;
>   			hcd->self.otg_port = 1;
>   		}
> +
> +		if (ci->platdata->notify_event &&
> +			(ci->platdata->flags & CI_HDRC_IMX_IS_HSIC))
> +			ci->platdata->notify_event
> +				(ci, CI_HDRC_IMX_HSIC_ACTIVE_EVENT);
>   	}
>   
>   	return ret;
> @@ -218,6 +237,8 @@ void ci_hdrc_host_destroy(struct ci_hdrc *ci)
>   static int ci_ehci_bus_suspend(struct usb_hcd *hcd)
>   {
>   	struct ehci_hcd *ehci = hcd_to_ehci(hcd);
> +	struct device *dev = hcd->self.controller;
> +	struct ci_hdrc *ci = dev_get_drvdata(dev);
>   	int port;
>   	u32 tmp;
>   
> @@ -249,6 +270,9 @@ static int ci_ehci_bus_suspend(struct usb_hcd *hcd)
>   			 * It needs a short delay between set RS bit and PHCD.
>   			 */
>   			usleep_range(150, 200);
> +			if (ci->platdata->flags & CI_HDRC_IMX_IS_HSIC)
> +				ci_ehci_override_wakeup_flag(ehci, reg,
> +					PORT_WKDISC_E | PORT_WKCONN_E, false);
>   			break;
>   		}
>   	}
> diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
> index 63758c399e4e..911e05af671e 100644
> --- a/include/linux/usb/chipidea.h
> +++ b/include/linux/usb/chipidea.h
> @@ -60,9 +60,12 @@ struct ci_hdrc_platform_data {
>   #define CI_HDRC_OVERRIDE_RX_BURST	BIT(11)
>   #define CI_HDRC_OVERRIDE_PHY_CONTROL	BIT(12) /* Glue layer manages phy */
>   #define CI_HDRC_REQUIRES_ALIGNED_DMA	BIT(13)
> +#define CI_HDRC_IMX_IS_HSIC		BIT(14)
>   	enum usb_dr_mode	dr_mode;
>   #define CI_HDRC_CONTROLLER_RESET_EVENT		0
>   #define CI_HDRC_CONTROLLER_STOPPED_EVENT	1
> +#define CI_HDRC_IMX_HSIC_ACTIVE_EVENT		2
> +#define CI_HDRC_IMX_HSIC_SUSPEND_EVENT		3
>   	int	(*notify_event) (struct ci_hdrc *ci, unsigned event);
>   	struct regulator	*reg_vbus;
>   	struct usb_otg_caps	ci_otg_caps;