($INBOX_DIR/description missing)
 help / color / mirror / Atom feed
* [PATCH] net: usb: qmi_wwan: add support for Quectel EG95 LTE modem
From: AceLan Kao @ 2020-07-07  8:14 UTC (permalink / raw)
  To: Bjørn Mork, David S. Miller, Jakub Kicinski, netdev,
	linux-usb, linux-kernel

Add support for Quectel Wireless Solutions Co., Ltd. EG95 LTE modem

T:  Bus=01 Lev=01 Prnt=01 Port=02 Cnt=02 Dev#=  5 Spd=480 MxCh= 0
D:  Ver= 2.00 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs=  1
P:  Vendor=2c7c ProdID=0195 Rev=03.18
S:  Manufacturer=Android
S:  Product=Android
C:  #Ifs= 5 Cfg#= 1 Atr=a0 MxPwr=500mA
I:  If#=0x0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=(none)
I:  If#=0x1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=(none)
I:  If#=0x2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=(none)
I:  If#=0x3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=(none)
I:  If#=0x4 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=(none)

Signed-off-by: AceLan Kao <acelan.kao@canonical.com>
---
 drivers/net/usb/qmi_wwan.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 31b1d4b959f6..07c42c0719f5 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -1370,6 +1370,7 @@ static const struct usb_device_id products[] = {
 	{QMI_QUIRK_SET_DTR(0x1e0e, 0x9001, 5)},	/* SIMCom 7100E, 7230E, 7600E ++ */
 	{QMI_QUIRK_SET_DTR(0x2c7c, 0x0121, 4)},	/* Quectel EC21 Mini PCIe */
 	{QMI_QUIRK_SET_DTR(0x2c7c, 0x0191, 4)},	/* Quectel EG91 */
+	{QMI_QUIRK_SET_DTR(0x2c7c, 0x0195, 4)},	/* Quectel EG95 */
 	{QMI_FIXED_INTF(0x2c7c, 0x0296, 4)},	/* Quectel BG96 */
 	{QMI_QUIRK_SET_DTR(0x2cb7, 0x0104, 4)},	/* Fibocom NL678 series */
 	{QMI_FIXED_INTF(0x0489, 0xe0b4, 0)},	/* Foxconn T77W968 LTE */
-- 
2.25.1


^ permalink raw reply related

* Re: [PATCH 04/30] usb: misc: sisusbvga: sisusb_init: Mark all 'static const' arrays as __maybe_unused
From: Lee Jones @ 2020-07-07  8:03 UTC (permalink / raw)
  To: Joe Perches
  Cc: gregkh, linux-usb, linux-arm-kernel, linux-kernel,
	Thomas Winischhofer
In-Reply-To: <af66b6e94a26ebd500c1376631891b0bdd0912f0.camel@perches.com>

On Fri, 03 Jul 2020, Joe Perches wrote:

> On Fri, 2020-07-03 at 18:41 +0100, Lee Jones wrote:
> > drivers/usb/misc/sisusbvga/sisusb_init.h is included by a few
> > source files.  Most of which do not use the majority of the
> > shared static const arrays which have been declared.  This
> > causes the build system to spew 100's of warnings.
> > 
> > Fixes the following W=1 kernel build warning(s) - and a whole lot more:
> > 
> >  In file included from drivers/usb/misc/sisusbvga/sisusb.c:54:
> >  drivers/usb/misc/sisusbvga/sisusb_init.h:695:34: warning: ‘SiSUSB_VCLKData’ defined but not used [-Wunused-const-variable=]
> >  695 | static const struct SiS_VCLKData SiSUSB_VCLKData[] = {
> >  | ^~~~~~~~~~~~~~~
> > [10's of lines snipped]
> >  drivers/usb/misc/sisusbvga/sisusb_init.h:206:28: warning: ‘SiS_VGA_DAC’ defined but not used [-Wunused-const-variable=]
> >  206 | static const unsigned char SiS_VGA_DAC[] = {
> >  | ^~~~~~~~~~~
> > [10's of lines snipped]
> >  drivers/usb/misc/sisusbvga/sisusb_init.h:171:29: warning: ‘ModeIndex_1280x1024’ defined but not used [-Wunused-const-variable=]
> >  171 | static const unsigned short ModeIndex_1280x1024[] = { 0x3a, 0x4d, 0x00, 0x65 };
> >  | ^~~~~~~~~~~~~~~~~~~
> > [10's of lines snipped]
> 
> They are not __maybe_unused, they _are_ used.

Actually, it looks like all of the ModeIndex_* table are *not* used,
so those will be removed.

> I think these instead should be moved to where
> they are used instead of being declared in an
> #include file.

The remaining tables are many and large, so stuffing them into the
source file does not seem like the correct thing to do.  Instead I
shall move them to their own "sisusb_tables.h" include file where they
can be referenced only by the file(s) which make use of them.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [PATCH v5 9/9] usb: cdns3: host: add xhci_plat_priv quirk XHCI_SKIP_PHY_INIT
From: Peter Chen @ 2020-07-07  7:49 UTC (permalink / raw)
  To: balbi, mathias.nyman, gregkh
  Cc: linux-usb, linux-imx, pawell, rogerq, jun.li, Peter Chen
In-Reply-To: <20200707074941.28078-1-peter.chen@nxp.com>

cdns3 manages PHY by own DRD driver, so skip the management by
HCD core.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/usb/cdns3/host.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/cdns3/host.c b/drivers/usb/cdns3/host.c
index 030d6421abd3..1dfbe23fa089 100644
--- a/drivers/usb/cdns3/host.c
+++ b/drivers/usb/cdns3/host.c
@@ -24,6 +24,7 @@
 #define LPM_2_STB_SWITCH_EN	(1 << 25)
 
 static const struct xhci_plat_priv xhci_plat_cdns3_xhci = {
+	.quirks = XHCI_SKIP_PHY_INIT,
 	.suspend_quirk = xhci_cdns3_suspend_quirk,
 };
 
-- 
2.17.1


^ permalink raw reply related

* [PATCH v5 8/9] usb: cdns3: host: add .suspend_quirk for xhci-plat.c
From: Peter Chen @ 2020-07-07  7:49 UTC (permalink / raw)
  To: balbi, mathias.nyman, gregkh
  Cc: linux-usb, linux-imx, pawell, rogerq, jun.li, Peter Chen
In-Reply-To: <20200707074941.28078-1-peter.chen@nxp.com>

cdns3 has some special PM sequence between xhci_bus_suspend and
xhci_suspend, add quirk to implement it.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/usb/cdns3/host-export.h |  6 +++++
 drivers/usb/cdns3/host.c        | 40 +++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/drivers/usb/cdns3/host-export.h b/drivers/usb/cdns3/host-export.h
index ae11810f8826..26041718a086 100644
--- a/drivers/usb/cdns3/host-export.h
+++ b/drivers/usb/cdns3/host-export.h
@@ -9,9 +9,11 @@
 #ifndef __LINUX_CDNS3_HOST_EXPORT
 #define __LINUX_CDNS3_HOST_EXPORT
 
+struct usb_hcd;
 #ifdef CONFIG_USB_CDNS3_HOST
 
 int cdns3_host_init(struct cdns3 *cdns);
+int xhci_cdns3_suspend_quirk(struct usb_hcd *hcd);
 
 #else
 
@@ -21,6 +23,10 @@ static inline int cdns3_host_init(struct cdns3 *cdns)
 }
 
 static inline void cdns3_host_exit(struct cdns3 *cdns) { }
+static inline int xhci_cdns3_suspend_quirk(struct usb_hcd *hcd)
+{
+	return 0;
+}
 
 #endif /* CONFIG_USB_CDNS3_HOST */
 
diff --git a/drivers/usb/cdns3/host.c b/drivers/usb/cdns3/host.c
index b579ef15f4e0..030d6421abd3 100644
--- a/drivers/usb/cdns3/host.c
+++ b/drivers/usb/cdns3/host.c
@@ -14,6 +14,18 @@
 #include "drd.h"
 #include "host-export.h"
 #include <linux/usb/hcd.h>
+#include "../host/xhci.h"
+#include "../host/xhci-plat.h"
+
+#define XECP_PORT_CAP_REG	0x8000
+#define XECP_AUX_CTRL_REG1	0x8120
+
+#define CFG_RXDET_P3_EN		(1 << 15)
+#define LPM_2_STB_SWITCH_EN	(1 << 25)
+
+static const struct xhci_plat_priv xhci_plat_cdns3_xhci = {
+	.suspend_quirk = xhci_cdns3_suspend_quirk,
+};
 
 static int __cdns3_host_init(struct cdns3 *cdns)
 {
@@ -39,6 +51,11 @@ static int __cdns3_host_init(struct cdns3 *cdns)
 		goto err1;
 	}
 
+	ret = platform_device_add_data(xhci, &xhci_plat_cdns3_xhci,
+			sizeof(struct xhci_plat_priv));
+	if (ret)
+		goto err1;
+
 	ret = platform_device_add(xhci);
 	if (ret) {
 		dev_err(cdns->dev, "failed to register xHCI device\n");
@@ -56,6 +73,29 @@ static int __cdns3_host_init(struct cdns3 *cdns)
 	return ret;
 }
 
+int xhci_cdns3_suspend_quirk(struct usb_hcd *hcd)
+{
+	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
+	u32 value;
+
+	/* set usbcmd.EU3S */
+	value = readl(&xhci->op_regs->command);
+	value |= CMD_PM_INDEX;
+	writel(value, &xhci->op_regs->command);
+
+	if (hcd->regs) {
+		value = readl(hcd->regs + XECP_AUX_CTRL_REG1);
+		value |= CFG_RXDET_P3_EN;
+		writel(value, hcd->regs + XECP_AUX_CTRL_REG1);
+
+		value = readl(hcd->regs + XECP_PORT_CAP_REG);
+		value |= LPM_2_STB_SWITCH_EN;
+		writel(value, hcd->regs + XECP_PORT_CAP_REG);
+	}
+
+	return 0;
+}
+
 static void cdns3_host_exit(struct cdns3 *cdns)
 {
 	platform_device_unregister(cdns->host_dev);
-- 
2.17.1


^ permalink raw reply related

* [PATCH v5 7/9] usb: host: xhci-plat: add priv quirk for skip PHY initialization
From: Peter Chen @ 2020-07-07  7:49 UTC (permalink / raw)
  To: balbi, mathias.nyman, gregkh
  Cc: linux-usb, linux-imx, pawell, rogerq, jun.li, Peter Chen
In-Reply-To: <20200707074941.28078-1-peter.chen@nxp.com>

Some DRD controllers (eg, dwc3 & cdns3) have PHY management at
their own driver to cover both device and host mode, so add one
priv quirk for such users to skip PHY management from HCD core.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/usb/host/xhci-plat.c | 8 ++++++--
 drivers/usb/host/xhci.h      | 1 +
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 03d6bbe51919..cebe24ec80a5 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -183,6 +183,8 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	struct usb_hcd		*hcd;
 	int			ret;
 	int			irq;
+	struct xhci_plat_priv	*priv = NULL;
+
 
 	if (usb_disabled())
 		return -ENODEV;
@@ -280,8 +282,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
 		priv_match = dev_get_platdata(&pdev->dev);
 
 	if (priv_match) {
-		struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
-
+		priv = hcd_to_xhci_priv(hcd);
 		/* Just copy data for now */
 		*priv = *priv_match;
 	}
@@ -329,6 +330,9 @@ static int xhci_plat_probe(struct platform_device *pdev)
 
 	hcd->tpl_support = of_usb_host_tpl_support(sysdev->of_node);
 	xhci->shared_hcd->tpl_support = hcd->tpl_support;
+	if (priv && (priv->quirks & XHCI_SKIP_PHY_INIT))
+		hcd->skip_phy_initialization = 1;
+
 	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
 	if (ret)
 		goto disable_usb_phy;
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index c295e8a7f5ae..07d71fec18d5 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1874,6 +1874,7 @@ struct xhci_hcd {
 #define XHCI_RESET_PLL_ON_DISCONNECT	BIT_ULL(34)
 #define XHCI_SNPS_BROKEN_SUSPEND    BIT_ULL(35)
 #define XHCI_RENESAS_FW_QUIRK	BIT_ULL(36)
+#define XHCI_SKIP_PHY_INIT	BIT_ULL(37)
 
 	unsigned int		num_active_eps;
 	unsigned int		limit_active_eps;
-- 
2.17.1


^ permalink raw reply related

* [PATCH v5 6/9] usb: host: xhci-plat: delete the unnecessary code
From: Peter Chen @ 2020-07-07  7:49 UTC (permalink / raw)
  To: balbi, mathias.nyman, gregkh
  Cc: linux-usb, linux-imx, pawell, rogerq, jun.li, Peter Chen
In-Reply-To: <20200707074941.28078-1-peter.chen@nxp.com>

The if {} condition is duplicated with outer if {} condition.

Reviewed-by: Jun Li <jun.li@nxp.com>
Acked-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/usb/host/xhci-plat.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 51e20464fe0a..03d6bbe51919 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -283,8 +283,7 @@ static int xhci_plat_probe(struct platform_device *pdev)
 		struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
 
 		/* Just copy data for now */
-		if (priv_match)
-			*priv = *priv_match;
+		*priv = *priv_match;
 	}
 
 	device_wakeup_enable(hcd->self.controller);
-- 
2.17.1


^ permalink raw reply related

* [PATCH v5 5/9] usb: host: xhci-plat: add .suspend_quirk for struct xhci_plat_priv
From: Peter Chen @ 2020-07-07  7:49 UTC (permalink / raw)
  To: balbi, mathias.nyman, gregkh
  Cc: linux-usb, linux-imx, pawell, rogerq, jun.li, Peter Chen
In-Reply-To: <20200707074941.28078-1-peter.chen@nxp.com>

Some platforms (eg cdns3) may have special sequences between
xhci_bus_suspend and xhci_suspend, add .suspend_quick for it.

Acked-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/usb/host/xhci-plat.c | 19 +++++++++++++++++++
 drivers/usb/host/xhci-plat.h |  1 +
 2 files changed, 20 insertions(+)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 38bea2b6a17d..51e20464fe0a 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -54,6 +54,16 @@ static int xhci_priv_init_quirk(struct usb_hcd *hcd)
 	return priv->init_quirk(hcd);
 }
 
+static int xhci_priv_suspend_quirk(struct usb_hcd *hcd)
+{
+	struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
+
+	if (!priv->suspend_quirk)
+		return 0;
+
+	return priv->suspend_quirk(hcd);
+}
+
 static int xhci_priv_resume_quirk(struct usb_hcd *hcd)
 {
 	struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
@@ -401,7 +411,11 @@ static int __maybe_unused xhci_plat_suspend(struct device *dev)
 {
 	struct usb_hcd	*hcd = dev_get_drvdata(dev);
 	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
+	int ret;
 
+	ret = xhci_priv_suspend_quirk(hcd);
+	if (ret)
+		return ret;
 	/*
 	 * xhci_suspend() needs `do_wakeup` to know whether host is allowed
 	 * to do wakeup during suspend. Since xhci_plat_suspend is currently
@@ -438,6 +452,11 @@ static int __maybe_unused xhci_plat_runtime_suspend(struct device *dev)
 {
 	struct usb_hcd  *hcd = dev_get_drvdata(dev);
 	struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+	int ret;
+
+	ret = xhci_priv_suspend_quirk(hcd);
+	if (ret)
+		return ret;
 
 	return xhci_suspend(xhci, true);
 }
diff --git a/drivers/usb/host/xhci-plat.h b/drivers/usb/host/xhci-plat.h
index b49f6447bd3a..1fb149d1fbce 100644
--- a/drivers/usb/host/xhci-plat.h
+++ b/drivers/usb/host/xhci-plat.h
@@ -15,6 +15,7 @@ struct xhci_plat_priv {
 	unsigned long long quirks;
 	void (*plat_start)(struct usb_hcd *);
 	int (*init_quirk)(struct usb_hcd *);
+	int (*suspend_quirk)(struct usb_hcd *);
 	int (*resume_quirk)(struct usb_hcd *);
 };
 
-- 
2.17.1


^ permalink raw reply related

* [PATCH v5 4/9] usb: host: xhci-plat: add platform data support
From: Peter Chen @ 2020-07-07  7:49 UTC (permalink / raw)
  To: balbi, mathias.nyman, gregkh
  Cc: linux-usb, linux-imx, pawell, rogerq, jun.li, Peter Chen
In-Reply-To: <20200707074941.28078-1-peter.chen@nxp.com>

Some xhci hosts (eg dwc3 and cdns3) do not use OF to create
platform device, they create xhci-plat platform device runtime.
And these platforms may also have quirks, and the quirks could
be supplied by their parent device through platform data.

Acked-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/usb/host/xhci-plat.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index f6b4089bfc4a..38bea2b6a17d 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -264,7 +264,11 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	if (ret)
 		goto disable_reg_clk;
 
-	priv_match = of_device_get_match_data(&pdev->dev);
+	if (pdev->dev.of_node)
+		priv_match = of_device_get_match_data(&pdev->dev);
+	else
+		priv_match = dev_get_platdata(&pdev->dev);
+
 	if (priv_match) {
 		struct xhci_plat_priv *priv = hcd_to_xhci_priv(hcd);
 
-- 
2.17.1


^ permalink raw reply related

* [PATCH v5 3/9] usb: cdns3: imx: add glue layer runtime pm implementation
From: Peter Chen @ 2020-07-07  7:49 UTC (permalink / raw)
  To: balbi, mathias.nyman, gregkh
  Cc: linux-usb, linux-imx, pawell, rogerq, jun.li, Peter Chen
In-Reply-To: <20200707074941.28078-1-peter.chen@nxp.com>

Add imx glue layer runtime pm implementation, and the runtime
pm is default off.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/usb/cdns3/cdns3-imx.c | 203 ++++++++++++++++++++++++++++++++--
 1 file changed, 192 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/cdns3/cdns3-imx.c b/drivers/usb/cdns3/cdns3-imx.c
index aba988e71958..5d2a4e19fa83 100644
--- a/drivers/usb/cdns3/cdns3-imx.c
+++ b/drivers/usb/cdns3/cdns3-imx.c
@@ -15,6 +15,8 @@
 #include <linux/io.h>
 #include <linux/of_platform.h>
 #include <linux/iopoll.h>
+#include <linux/pm_runtime.h>
+#include "core.h"
 
 #define USB3_CORE_CTRL1    0x00
 #define USB3_CORE_CTRL2    0x04
@@ -32,7 +34,7 @@
 /* Register bits definition */
 
 /* USB3_CORE_CTRL1 */
-#define SW_RESET_MASK	(0x3f << 26)
+#define SW_RESET_MASK	GENMASK(31, 26)
 #define PWR_SW_RESET	BIT(31)
 #define APB_SW_RESET	BIT(30)
 #define AXI_SW_RESET	BIT(29)
@@ -44,17 +46,17 @@
 #define OC_DISABLE	BIT(9)
 #define MDCTRL_CLK_SEL	BIT(7)
 #define MODE_STRAP_MASK	(0x7)
-#define DEV_MODE	(1 << 2)
-#define HOST_MODE	(1 << 1)
-#define OTG_MODE	(1 << 0)
+#define DEV_MODE	BIT(2)
+#define HOST_MODE	BIT(1)
+#define OTG_MODE	BIT(0)
 
 /* USB3_INT_REG */
 #define CLK_125_REQ	BIT(29)
 #define LPM_CLK_REQ	BIT(28)
 #define DEVU3_WAEKUP_EN	BIT(14)
 #define OTG_WAKEUP_EN	BIT(12)
-#define DEV_INT_EN (3 << 8) /* DEV INT b9:8 */
-#define HOST_INT1_EN (1 << 0) /* HOST INT b7:0 */
+#define DEV_INT_EN	GENMASK(9, 8) /* DEV INT b9:8 */
+#define HOST_INT1_EN	BIT(0) /* HOST INT b7:0 */
 
 /* USB3_CORE_STATUS */
 #define MDCTRL_CLK_STATUS	BIT(15)
@@ -62,15 +64,34 @@
 #define HOST_POWER_ON_READY	BIT(12)
 
 /* USB3_SSPHY_STATUS */
-#define CLK_VALID_MASK		(0x3f << 26)
-#define CLK_VALID_COMPARE_BITS	(0xf << 28)
-#define PHY_REFCLK_REQ		(1 << 0)
+#define CLK_VALID_MASK		GENMASK(31, 26)
+#define CLK_VALID_COMPARE_BITS	GENMASK(31, 28)
+#define PHY_REFCLK_REQ		BIT(0)
+
+/* OTG registers definition */
+#define OTGSTS		0x4
+/* OTGSTS */
+#define OTG_NRDY	BIT(11)
+
+/* xHCI registers definition  */
+#define XECP_PM_PMCSR		0x8018
+#define XECP_AUX_CTRL_REG1	0x8120
+
+/* Register bits definition */
+/* XECP_AUX_CTRL_REG1 */
+#define CFG_RXDET_P3_EN		BIT(15)
+
+/* XECP_PM_PMCSR */
+#define PS_MASK			GENMASK(1, 0)
+#define PS_D0			0
+#define PS_D1			1
 
 struct cdns_imx {
 	struct device *dev;
 	void __iomem *noncore;
 	struct clk_bulk_data *clks;
 	int num_clks;
+	struct platform_device *cdns3_pdev;
 };
 
 static inline u32 cdns_imx_readl(struct cdns_imx *data, u32 offset)
@@ -126,6 +147,20 @@ static int cdns_imx_noncore_init(struct cdns_imx *data)
 	return ret;
 }
 
+static int cdns_imx_platform_suspend(struct device *dev,
+	bool suspend, bool wakeup);
+static struct cdns3_platform_data cdns_imx_pdata = {
+	.platform_suspend = cdns_imx_platform_suspend,
+};
+
+static struct of_dev_auxdata cdns_imx_auxdata[] = {
+	{
+	.compatible = "cdns,usb3",
+	.platform_data = &cdns_imx_pdata,
+	},
+	{},
+};
+
 static int cdns_imx_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -162,14 +197,18 @@ static int cdns_imx_probe(struct platform_device *pdev)
 	if (ret)
 		goto err;
 
-	ret = of_platform_populate(node, NULL, NULL, dev);
+	ret = of_platform_populate(node, NULL, cdns_imx_auxdata, dev);
 	if (ret) {
 		dev_err(dev, "failed to create children: %d\n", ret);
 		goto err;
 	}
 
-	return ret;
+	device_set_wakeup_capable(dev, true);
+	pm_runtime_set_active(dev);
+	pm_runtime_enable(dev);
+	pm_runtime_forbid(dev);
 
+	return ret;
 err:
 	clk_bulk_disable_unprepare(data->num_clks, data->clks);
 	return ret;
@@ -194,6 +233,147 @@ static int cdns_imx_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM
+static void cdns3_set_wakeup(struct cdns_imx *data, bool enable)
+{
+	u32 value;
+
+	value = cdns_imx_readl(data, USB3_INT_REG);
+	if (enable)
+		value |= OTG_WAKEUP_EN | DEVU3_WAEKUP_EN;
+	else
+		value &= ~(OTG_WAKEUP_EN | DEVU3_WAEKUP_EN);
+
+	cdns_imx_writel(data, USB3_INT_REG, value);
+}
+
+static int cdns_imx_platform_suspend(struct device *dev,
+		bool suspend, bool wakeup)
+{
+	struct cdns3 *cdns = dev_get_drvdata(dev);
+	struct device *parent = dev->parent;
+	struct cdns_imx *data = dev_get_drvdata(parent);
+	void __iomem *otg_regs = (void *)(cdns->otg_regs);
+	void __iomem *xhci_regs = cdns->xhci_regs;
+	u32 value;
+	int ret = 0;
+
+	if (cdns->role != USB_ROLE_HOST)
+		return 0;
+
+	if (suspend) {
+		/* SW request low power when all usb ports allow to it ??? */
+		value = readl(xhci_regs + XECP_PM_PMCSR);
+		value &= ~PS_MASK;
+		value |= PS_D1;
+		writel(value, xhci_regs + XECP_PM_PMCSR);
+
+		/* mdctrl_clk_sel */
+		value = cdns_imx_readl(data, USB3_CORE_CTRL1);
+		value |= MDCTRL_CLK_SEL;
+		cdns_imx_writel(data, USB3_CORE_CTRL1, value);
+
+		/* wait for mdctrl_clk_status */
+		value = cdns_imx_readl(data, USB3_CORE_STATUS);
+		ret = readl_poll_timeout(data->noncore + USB3_CORE_STATUS, value,
+			(value & MDCTRL_CLK_STATUS) == MDCTRL_CLK_STATUS,
+			10, 100000);
+		if (ret)
+			dev_warn(parent, "wait mdctrl_clk_status timeout\n");
+
+		/* wait lpm_clk_req to be 0 */
+		value = cdns_imx_readl(data, USB3_INT_REG);
+		ret = readl_poll_timeout(data->noncore + USB3_INT_REG, value,
+			(value & LPM_CLK_REQ) != LPM_CLK_REQ,
+			10, 100000);
+		if (ret)
+			dev_warn(parent, "wait lpm_clk_req timeout\n");
+
+		/* wait phy_refclk_req to be 0 */
+		value = cdns_imx_readl(data, USB3_SSPHY_STATUS);
+		ret = readl_poll_timeout(data->noncore + USB3_SSPHY_STATUS, value,
+			(value & PHY_REFCLK_REQ) != PHY_REFCLK_REQ,
+			10, 100000);
+		if (ret)
+			dev_warn(parent, "wait phy_refclk_req timeout\n");
+
+		cdns3_set_wakeup(data, wakeup);
+	} else {
+		cdns3_set_wakeup(data, false);
+
+		/* SW request D0 */
+		value = readl(xhci_regs + XECP_PM_PMCSR);
+		value &= ~PS_MASK;
+		value |= PS_D0;
+		writel(value, xhci_regs + XECP_PM_PMCSR);
+
+		/* clr CFG_RXDET_P3_EN */
+		value = readl(xhci_regs + XECP_AUX_CTRL_REG1);
+		value &= ~CFG_RXDET_P3_EN;
+		writel(value, xhci_regs + XECP_AUX_CTRL_REG1);
+
+		/* clear mdctrl_clk_sel */
+		value = cdns_imx_readl(data, USB3_CORE_CTRL1);
+		value &= ~MDCTRL_CLK_SEL;
+		cdns_imx_writel(data, USB3_CORE_CTRL1, value);
+
+		/* wait CLK_125_REQ to be 1 */
+		value = cdns_imx_readl(data, USB3_INT_REG);
+		ret = readl_poll_timeout(data->noncore + USB3_INT_REG, value,
+			(value & CLK_125_REQ) == CLK_125_REQ,
+			10, 100000);
+		if (ret)
+			dev_warn(parent, "wait CLK_125_REQ timeout\n");
+
+		/* wait for mdctrl_clk_status is cleared */
+		value = cdns_imx_readl(data, USB3_CORE_STATUS);
+		ret = readl_poll_timeout(data->noncore + USB3_CORE_STATUS, value,
+			(value & MDCTRL_CLK_STATUS) != MDCTRL_CLK_STATUS,
+			10, 100000);
+		if (ret)
+			dev_warn(parent, "wait mdctrl_clk_status cleared timeout\n");
+
+		/* Wait until OTG_NRDY is 0 */
+		value = readl(otg_regs + OTGSTS);
+		ret = readl_poll_timeout(otg_regs + OTGSTS, value,
+			(value & OTG_NRDY) != OTG_NRDY,
+			10, 100000);
+		if (ret)
+			dev_warn(parent, "wait OTG ready timeout\n");
+	}
+
+	return ret;
+
+}
+
+static int cdns_imx_resume(struct device *dev)
+{
+	struct cdns_imx *data = dev_get_drvdata(dev);
+
+	return clk_bulk_prepare_enable(data->num_clks, data->clks);
+}
+
+static int cdns_imx_suspend(struct device *dev)
+{
+	struct cdns_imx *data = dev_get_drvdata(dev);
+
+	clk_bulk_disable_unprepare(data->num_clks, data->clks);
+
+	return 0;
+}
+#else
+static int cdns_imx_platform_suspend(struct device *dev,
+	bool suspend, bool wakeup)
+{
+	return 0;
+}
+
+#endif /* CONFIG_PM */
+
+static const struct dev_pm_ops cdns_imx_pm_ops = {
+	SET_RUNTIME_PM_OPS(cdns_imx_suspend, cdns_imx_resume, NULL)
+};
+
 static const struct of_device_id cdns_imx_of_match[] = {
 	{ .compatible = "fsl,imx8qm-usb3", },
 	{},
@@ -206,6 +386,7 @@ static struct platform_driver cdns_imx_driver = {
 	.driver		= {
 		.name	= "cdns3-imx",
 		.of_match_table	= cdns_imx_of_match,
+		.pm	= &cdns_imx_pm_ops,
 	},
 };
 module_platform_driver(cdns_imx_driver);
-- 
2.17.1


^ permalink raw reply related

* [PATCH v5 2/9] usb: cdns3: add runtime PM support
From: Peter Chen @ 2020-07-07  7:49 UTC (permalink / raw)
  To: balbi, mathias.nyman, gregkh
  Cc: linux-usb, linux-imx, pawell, rogerq, jun.li, Peter Chen
In-Reply-To: <20200707074941.28078-1-peter.chen@nxp.com>

Introduce runtime PM and wakeup interrupt handler for cdns3,
the runtime PM is default off since other cdns3 may not
implement glue layer support for runtime PM.

One typical wakeup event use case is xHCI runtime suspend will clear
USBCMD.RS bit, after that the xHCI will not trigger any interrupts,
so its parent (cdns core device) needs to resume xHCI device when
any (wakeup) events occurs at host port.

When the controller is in low power mode, the lpm flag will be set.
The interrupt triggered later than lpm flag is set considers as
wakeup interrupt and handled at cdns_wakeup_irq. Once the wakeup
occurs, it first disables interrupt to avoid later interrupt
occurrence since the controller is in low power mode at that
time, and access registers may be invalid at that time. At wakeup
handler, it will call pm_request_resume to wakeup xHCI device, and
at runtime resume handler, it will enable interrupt again.

The API platform_suspend is introduced for glue layer to implement
platform specific PM sequence.

Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/usb/cdns3/core.c   | 153 ++++++++++++++++++++++++++++++++-----
 drivers/usb/cdns3/core.h   |  16 ++++
 drivers/usb/cdns3/drd.c    |   3 +
 drivers/usb/cdns3/gadget.c |   4 +
 drivers/usb/cdns3/host.c   |   7 ++
 5 files changed, 166 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
index 8818935d157b..f98c3c1a6b26 100644
--- a/drivers/usb/cdns3/core.c
+++ b/drivers/usb/cdns3/core.c
@@ -405,6 +405,30 @@ static void set_phy_power_off(struct cdns3 *cdns)
 	phy_power_off(cdns->usb2_phy);
 }
 
+/**
+ * cdns3_wakeup_irq - interrupt handler for wakeup events
+ *
+ * @irq: irq number for cdns3 core device
+ * @data: structure of cdns3
+ *
+ * Returns IRQ_HANDLED or IRQ_NONE
+ */
+static irqreturn_t cdns3_wakeup_irq(int irq, void *data)
+{
+	struct cdns3 *cdns = data;
+
+	if (cdns->in_lpm) {
+		disable_irq_nosync(irq);
+		cdns->wakeup_pending = true;
+		if ((cdns->role == USB_ROLE_HOST) && cdns->host_dev)
+			pm_request_resume(&cdns->host_dev->dev);
+
+		return IRQ_HANDLED;
+	}
+
+	return IRQ_NONE;
+}
+
 /**
  * cdns3_probe - probe for cdns3 core device
  * @pdev: Pointer to cdns3 core platform device
@@ -431,6 +455,7 @@ static int cdns3_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	cdns->dev = dev;
+	cdns->pdata = dev_get_platdata(dev);
 
 	platform_set_drvdata(pdev, cdns);
 
@@ -480,6 +505,15 @@ static int cdns3_probe(struct platform_device *pdev)
 
 	cdns->otg_res = *res;
 
+	cdns->wakeup_irq = platform_get_irq_byname_optional(pdev, "wakeup");
+	if (cdns->wakeup_irq == -EPROBE_DEFER)
+		return cdns->wakeup_irq;
+
+	if (cdns->wakeup_irq < 0) {
+		dev_dbg(dev, "couldn't get wakeup irq\n");
+		cdns->wakeup_irq = 0x0;
+	}
+
 	mutex_init(&cdns->mutex);
 
 	cdns->usb2_phy = devm_phy_optional_get(dev, "cdns3,usb2-phy");
@@ -516,6 +550,19 @@ static int cdns3_probe(struct platform_device *pdev)
 		goto err3;
 	}
 
+	if (cdns->wakeup_irq) {
+		ret = devm_request_threaded_irq(cdns->dev, cdns->wakeup_irq,
+						cdns3_wakeup_irq,
+						NULL,
+						IRQF_SHARED,
+						dev_name(cdns->dev), cdns);
+
+		if (ret) {
+			dev_err(cdns->dev, "couldn't register wakeup irq handler\n");
+			goto err3;
+		}
+	}
+
 	ret = cdns3_drd_init(cdns);
 	if (ret)
 		goto err4;
@@ -524,9 +571,11 @@ static int cdns3_probe(struct platform_device *pdev)
 	if (ret)
 		goto err4;
 
+	spin_lock_init(&cdns->lock);
 	device_set_wakeup_capable(dev, true);
 	pm_runtime_set_active(dev);
 	pm_runtime_enable(dev);
+	pm_runtime_forbid(dev);
 
 	/*
 	 * The controller needs less time between bus and controller suspend,
@@ -573,52 +622,122 @@ static int cdns3_remove(struct platform_device *pdev)
 	return 0;
 }
 
-#ifdef CONFIG_PM_SLEEP
+#ifdef CONFIG_PM
 
-static int cdns3_suspend(struct device *dev)
+static int cdns3_set_platform_suspend(struct device *dev,
+		bool suspend, bool wakeup)
+{
+	struct cdns3 *cdns = dev_get_drvdata(dev);
+	int ret = 0;
+
+	if (cdns->pdata && cdns->pdata->platform_suspend)
+		ret = cdns->pdata->platform_suspend(dev, suspend, wakeup);
+
+	return ret;
+}
+
+static int cdns3_controller_suspend(struct device *dev, pm_message_t msg)
 {
 	struct cdns3 *cdns = dev_get_drvdata(dev);
+	bool wakeup;
 	unsigned long flags;
 
-	if (cdns->role == USB_ROLE_HOST)
+	if (cdns->in_lpm)
 		return 0;
 
-	if (pm_runtime_status_suspended(dev))
-		pm_runtime_resume(dev);
+	if (PMSG_IS_AUTO(msg))
+		wakeup = true;
+	else
+		wakeup = device_may_wakeup(dev);
 
-	if (cdns->roles[cdns->role]->suspend) {
-		spin_lock_irqsave(&cdns->gadget_dev->lock, flags);
-		cdns->roles[cdns->role]->suspend(cdns, false);
-		spin_unlock_irqrestore(&cdns->gadget_dev->lock, flags);
-	}
+	cdns3_set_platform_suspend(cdns->dev, true, wakeup);
+	set_phy_power_off(cdns);
+	spin_lock_irqsave(&cdns->lock, flags);
+	cdns->in_lpm = true;
+	spin_unlock_irqrestore(&cdns->lock, flags);
+	dev_dbg(cdns->dev, "%s ends\n", __func__);
 
 	return 0;
 }
 
-static int cdns3_resume(struct device *dev)
+static int cdns3_controller_resume(struct device *dev, pm_message_t msg)
 {
 	struct cdns3 *cdns = dev_get_drvdata(dev);
+	int ret;
 	unsigned long flags;
 
-	if (cdns->role == USB_ROLE_HOST)
+	if (!cdns->in_lpm)
 		return 0;
 
-	if (cdns->roles[cdns->role]->resume) {
-		spin_lock_irqsave(&cdns->gadget_dev->lock, flags);
+	ret = set_phy_power_on(cdns);
+	if (ret)
+		return ret;
+
+	cdns3_set_platform_suspend(cdns->dev, false, false);
+
+	spin_lock_irqsave(&cdns->lock, flags);
+	if (cdns->roles[cdns->role]->resume && !PMSG_IS_AUTO(msg))
 		cdns->roles[cdns->role]->resume(cdns, false);
-		spin_unlock_irqrestore(&cdns->gadget_dev->lock, flags);
+
+	cdns->in_lpm = false;
+	spin_unlock_irqrestore(&cdns->lock, flags);
+	if (cdns->wakeup_pending) {
+		cdns->wakeup_pending = false;
+		enable_irq(cdns->wakeup_irq);
+	}
+	dev_dbg(cdns->dev, "%s ends\n", __func__);
+
+	return ret;
+}
+
+static int cdns3_runtime_suspend(struct device *dev)
+{
+	return cdns3_controller_suspend(dev, PMSG_AUTO_SUSPEND);
+}
+
+static int cdns3_runtime_resume(struct device *dev)
+{
+	return cdns3_controller_resume(dev, PMSG_AUTO_RESUME);
+}
+#ifdef CONFIG_PM_SLEEP
+
+static int cdns3_suspend(struct device *dev)
+{
+	struct cdns3 *cdns = dev_get_drvdata(dev);
+	unsigned long flags;
+
+	if (pm_runtime_status_suspended(dev))
+		pm_runtime_resume(dev);
+
+	if (cdns->roles[cdns->role]->suspend) {
+		spin_lock_irqsave(&cdns->lock, flags);
+		cdns->roles[cdns->role]->suspend(cdns, false);
+		spin_unlock_irqrestore(&cdns->lock, flags);
 	}
 
+	return cdns3_controller_suspend(dev, PMSG_SUSPEND);
+}
+
+static int cdns3_resume(struct device *dev)
+{
+	int ret;
+
+	ret = cdns3_controller_resume(dev, PMSG_RESUME);
+	if (ret)
+		return ret;
+
 	pm_runtime_disable(dev);
 	pm_runtime_set_active(dev);
 	pm_runtime_enable(dev);
 
-	return 0;
+	return ret;
 }
-#endif
+#endif /* CONFIG_PM_SLEEP */
+#endif /* CONFIG_PM */
 
 static const struct dev_pm_ops cdns3_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(cdns3_suspend, cdns3_resume)
+	SET_RUNTIME_PM_OPS(cdns3_runtime_suspend, cdns3_runtime_resume, NULL)
 };
 
 #ifdef CONFIG_OF
diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h
index 1ad1f1fe61e9..1b1707796db2 100644
--- a/drivers/usb/cdns3/core.h
+++ b/drivers/usb/cdns3/core.h
@@ -38,6 +38,12 @@ struct cdns3_role_driver {
 };
 
 #define CDNS3_XHCI_RESOURCES_NUM	2
+
+struct cdns3_platform_data {
+	int (*platform_suspend)(struct device *dev,
+			bool suspend, bool wakeup);
+};
+
 /**
  * struct cdns3 - Representation of Cadence USB3 DRD controller.
  * @dev: pointer to Cadence device struct
@@ -50,6 +56,7 @@ struct cdns3_role_driver {
  * @otg_regs: pointer to base of otg registers
  * @otg_irq: irq number for otg controller
  * @dev_irq: irq number for device controller
+ * @wakeup_irq: irq number for wakeup event, it is optional
  * @roles: array of supported roles for this controller
  * @role: current role
  * @host_dev: the child host device pointer for cdns3 core
@@ -62,6 +69,10 @@ struct cdns3_role_driver {
  *           This field based on firmware setting, kernel configuration
  *           and hardware configuration.
  * @role_sw: pointer to role switch object.
+ * @in_lpm: indicate the controller is in low power mode
+ * @wakeup_pending: wakeup interrupt pending
+ * @pdata: platform data from glue layer
+ * @lock: spinlock structure
  */
 struct cdns3 {
 	struct device			*dev;
@@ -79,6 +90,7 @@ struct cdns3 {
 
 	int				otg_irq;
 	int				dev_irq;
+	int				wakeup_irq;
 	struct cdns3_role_driver	*roles[USB_ROLE_DEVICE + 1];
 	enum usb_role			role;
 	struct platform_device		*host_dev;
@@ -89,6 +101,10 @@ struct cdns3 {
 	struct mutex			mutex;
 	enum usb_dr_mode		dr_mode;
 	struct usb_role_switch		*role_sw;
+	bool				in_lpm;
+	bool				wakeup_pending;
+	struct cdns3_platform_data	*pdata;
+	spinlock_t			lock;
 };
 
 int cdns3_hw_role_switch(struct cdns3 *cdns);
diff --git a/drivers/usb/cdns3/drd.c b/drivers/usb/cdns3/drd.c
index 58089841ed52..ac90a484e63c 100644
--- a/drivers/usb/cdns3/drd.c
+++ b/drivers/usb/cdns3/drd.c
@@ -281,6 +281,9 @@ static irqreturn_t cdns3_drd_irq(int irq, void *data)
 	if (cdns->dr_mode != USB_DR_MODE_OTG)
 		return ret;
 
+	if (cdns->in_lpm)
+		return ret;
+
 	reg = readl(&cdns->otg_regs->ivect);
 
 	if (!reg)
diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
index 7c2913bc8bd7..0111fba95797 100644
--- a/drivers/usb/cdns3/gadget.c
+++ b/drivers/usb/cdns3/gadget.c
@@ -1766,9 +1766,13 @@ static void cdns3_check_usb_interrupt_proceed(struct cdns3_device *priv_dev,
 static irqreturn_t cdns3_device_irq_handler(int irq, void *data)
 {
 	struct cdns3_device *priv_dev = data;
+	struct cdns3 *cdns = dev_get_drvdata(priv_dev->dev);
 	irqreturn_t ret = IRQ_NONE;
 	u32 reg;
 
+	if (cdns->in_lpm)
+		return ret;
+
 	/* check USB device interrupt */
 	reg = readl(&priv_dev->regs->usb_ists);
 	if (reg) {
diff --git a/drivers/usb/cdns3/host.c b/drivers/usb/cdns3/host.c
index ad788bf3fe4f..b579ef15f4e0 100644
--- a/drivers/usb/cdns3/host.c
+++ b/drivers/usb/cdns3/host.c
@@ -13,11 +13,13 @@
 #include "core.h"
 #include "drd.h"
 #include "host-export.h"
+#include <linux/usb/hcd.h>
 
 static int __cdns3_host_init(struct cdns3 *cdns)
 {
 	struct platform_device *xhci;
 	int ret;
+	struct usb_hcd *hcd;
 
 	cdns3_drd_switch_host(cdns, 1);
 
@@ -43,6 +45,11 @@ static int __cdns3_host_init(struct cdns3 *cdns)
 		goto err1;
 	}
 
+	/* Glue needs to access xHCI region register for Power management */
+	hcd = platform_get_drvdata(xhci);
+	if (hcd)
+		cdns->xhci_regs = hcd->regs;
+
 	return 0;
 err1:
 	platform_device_put(xhci);
-- 
2.17.1


^ permalink raw reply related

* [PATCH v5 1/9] usb: cdns3: introduce set_phy_power_on{off} APIs
From: Peter Chen @ 2020-07-07  7:49 UTC (permalink / raw)
  To: balbi, mathias.nyman, gregkh
  Cc: linux-usb, linux-imx, pawell, rogerq, jun.li, Peter Chen
In-Reply-To: <20200707074941.28078-1-peter.chen@nxp.com>

Since we have both USB2 and USB3 PHYs for cdns3 controller, it is
better we have unity APIs to handle both USB2 and USB3's power, it
could simplify code for error handling and further power management
implementation.

Reviewed-by: Jun Li <jun.li@nxp.com>
Signed-off-by: Peter Chen <peter.chen@nxp.com>
---
 drivers/usb/cdns3/core.c | 43 ++++++++++++++++++++++++++--------------
 1 file changed, 28 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
index 19bbb5b7e6b6..8818935d157b 100644
--- a/drivers/usb/cdns3/core.c
+++ b/drivers/usb/cdns3/core.c
@@ -384,6 +384,27 @@ static int cdns3_role_set(struct usb_role_switch *sw, enum usb_role role)
 	return ret;
 }
 
+static int set_phy_power_on(struct cdns3 *cdns)
+{
+	int ret;
+
+	ret = phy_power_on(cdns->usb2_phy);
+	if (ret)
+		return ret;
+
+	ret = phy_power_on(cdns->usb3_phy);
+	if (ret)
+		phy_power_off(cdns->usb2_phy);
+
+	return ret;
+}
+
+static void set_phy_power_off(struct cdns3 *cdns)
+{
+	phy_power_off(cdns->usb3_phy);
+	phy_power_off(cdns->usb2_phy);
+}
+
 /**
  * cdns3_probe - probe for cdns3 core device
  * @pdev: Pointer to cdns3 core platform device
@@ -477,14 +498,10 @@ static int cdns3_probe(struct platform_device *pdev)
 	if (ret)
 		goto err1;
 
-	ret = phy_power_on(cdns->usb2_phy);
+	ret = set_phy_power_on(cdns);
 	if (ret)
 		goto err2;
 
-	ret = phy_power_on(cdns->usb3_phy);
-	if (ret)
-		goto err3;
-
 	sw_desc.set = cdns3_role_set;
 	sw_desc.get = cdns3_role_get;
 	sw_desc.allow_userspace_control = true;
@@ -496,16 +513,16 @@ static int cdns3_probe(struct platform_device *pdev)
 	if (IS_ERR(cdns->role_sw)) {
 		ret = PTR_ERR(cdns->role_sw);
 		dev_warn(dev, "Unable to register Role Switch\n");
-		goto err4;
+		goto err3;
 	}
 
 	ret = cdns3_drd_init(cdns);
 	if (ret)
-		goto err5;
+		goto err4;
 
 	ret = cdns3_core_init_role(cdns);
 	if (ret)
-		goto err5;
+		goto err4;
 
 	device_set_wakeup_capable(dev, true);
 	pm_runtime_set_active(dev);
@@ -522,14 +539,11 @@ static int cdns3_probe(struct platform_device *pdev)
 	dev_dbg(dev, "Cadence USB3 core: probe succeed\n");
 
 	return 0;
-err5:
+err4:
 	cdns3_drd_exit(cdns);
 	usb_role_switch_unregister(cdns->role_sw);
-err4:
-	phy_power_off(cdns->usb3_phy);
-
 err3:
-	phy_power_off(cdns->usb2_phy);
+	set_phy_power_off(cdns);
 err2:
 	phy_exit(cdns->usb3_phy);
 err1:
@@ -553,8 +567,7 @@ static int cdns3_remove(struct platform_device *pdev)
 	pm_runtime_put_noidle(&pdev->dev);
 	cdns3_exit_roles(cdns);
 	usb_role_switch_unregister(cdns->role_sw);
-	phy_power_off(cdns->usb2_phy);
-	phy_power_off(cdns->usb3_phy);
+	set_phy_power_off(cdns);
 	phy_exit(cdns->usb2_phy);
 	phy_exit(cdns->usb3_phy);
 	return 0;
-- 
2.17.1


^ permalink raw reply related

* [PATCH v5 0/9] usb: some PM changes for cdns3 and xhci-plat
From: Peter Chen @ 2020-07-07  7:49 UTC (permalink / raw)
  To: balbi, mathias.nyman, gregkh
  Cc: linux-usb, linux-imx, pawell, rogerq, jun.li, Peter Chen

Hi Felipe & Mathias,

In this series, it adds cdns3 runtime PM support as well as some
xhci-plat changes, and verified at NXP i.MX8QM and i.MX8QXP platforms.

Patch 1-3: add main runtime PM support for cdns3 core and imx glue layer
Patch 4-9: add platform data support and two quirks for xhci-plat.c.
- .suspend_quirk for platform special sequence between xhci_bus_suspend
and xhci_suspend.
- add XHCI_SKIP_PHY_INIT quirk for skip PHY management from HCD core.

Changes for v5:
- Address Greg's comments for more obvious PHY power controller APIs [Patch 1/2]
- One build warning from kernel test robot

Changes for v4:
- Address Jun Li's comments for cdns3 core changes [Patch 2]
- Some small fixes for cdns3-imx for CLK_125_REQ bit
- Rebase the latest usb-next

Changes for v3:
Add Jun Li’s reviewed-by [Patch 1 and Patch 6]
Add Mathias’s acked-by [Patch 4-6]
Some wakeup logic improvement [Patch 2]
Add dedicated wakeup interrupt for core, and improve the commit log [Patch]
Fix build error found by kbuild test robot [Patch 3]
Using xhci_plat_priv quirk for skip PHY initialization [patch 7, patch 9]
Some other typo and tiny improvements

Changes for v2:
- Add the 1st patch. Without it, the build on the usb-next will fail.
- Change the subject for cover letter a little to reflect all contents.

Peter Chen (9):
  usb: cdns3: introduce set_phy_power_on{off} APIs
  usb: cdns3: add runtime PM support
  usb: cdns3: imx: add glue layer runtime pm implementation
  usb: host: xhci-plat: add platform data support
  usb: host: xhci-plat: add .suspend_quirk for struct xhci_plat_priv
  usb: host: xhci-plat: delete the unnecessary code
  usb: host: xhci-plat: add priv quirk for skip PHY initialization
  usb: cdns3: host: add .suspend_quirk for xhci-plat.c
  usb: cdns3: host: add xhci_plat_priv quirk XHCI_SKIP_PHY_INIT

 drivers/usb/cdns3/cdns3-imx.c   | 203 ++++++++++++++++++++++++++++++--
 drivers/usb/cdns3/core.c        | 196 +++++++++++++++++++++++++-----
 drivers/usb/cdns3/core.h        |  16 +++
 drivers/usb/cdns3/drd.c         |   3 +
 drivers/usb/cdns3/gadget.c      |   4 +
 drivers/usb/cdns3/host-export.h |   6 +
 drivers/usb/cdns3/host.c        |  48 ++++++++
 drivers/usb/host/xhci-plat.c    |  36 +++++-
 drivers/usb/host/xhci-plat.h    |   1 +
 drivers/usb/host/xhci.h         |   1 +
 10 files changed, 466 insertions(+), 48 deletions(-)

-- 
2.17.1


^ permalink raw reply

* Re: [PATCH 04/30] usb: misc: sisusbvga: sisusb_init: Mark all 'static const' arrays as __maybe_unused
From: Lee Jones @ 2020-07-07  7:49 UTC (permalink / raw)
  To: gregkh, linux-usb; +Cc: linux-arm-kernel, linux-kernel, Thomas Winischhofer
In-Reply-To: <20200703174148.2749969-5-lee.jones@linaro.org>

On Fri, 03 Jul 2020, Lee Jones wrote:

> drivers/usb/misc/sisusbvga/sisusb_init.h is included by a few
> source files.  Most of which do not use the majority of the
> shared static const arrays which have been declared.  This
> causes the build system to spew 100's of warnings.
> 
> Fixes the following W=1 kernel build warning(s) - and a whole lot more:
> 
>  In file included from drivers/usb/misc/sisusbvga/sisusb.c:54:
>  drivers/usb/misc/sisusbvga/sisusb_init.h:695:34: warning: ‘SiSUSB_VCLKData’ defined but not used [-Wunused-const-variable=]
>  695 | static const struct SiS_VCLKData SiSUSB_VCLKData[] = {
>  | ^~~~~~~~~~~~~~~
> [10's of lines snipped]
>  drivers/usb/misc/sisusbvga/sisusb_init.h:206:28: warning: ‘SiS_VGA_DAC’ defined but not used [-Wunused-const-variable=]
>  206 | static const unsigned char SiS_VGA_DAC[] = {
>  | ^~~~~~~~~~~
> [10's of lines snipped]
>  drivers/usb/misc/sisusbvga/sisusb_init.h:171:29: warning: ‘ModeIndex_1280x1024’ defined but not used [-Wunused-const-variable=]
>  171 | static const unsigned short ModeIndex_1280x1024[] = { 0x3a, 0x4d, 0x00, 0x65 };
>  | ^~~~~~~~~~~~~~~~~~~
> [10's of lines snipped]
> 
> Cc: Thomas Winischhofer <thomas@winischhofer.net>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/usb/misc/sisusbvga/sisusb_init.h | 66 ++++++++++++------------
>  1 file changed, 33 insertions(+), 33 deletions(-)

Greg,

Please drop this patch.  I have a new solution.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [PATCH v4] usb: gadget: function: fix missing spinlock in f_uac1_legacy
From: qiang.zhang @ 2020-07-06  5:14 UTC (permalink / raw)
  To: balbi; +Cc: colin.king, gregkh, linux-usb, linux-kernel, stable

From: Zhang Qiang <qiang.zhang@windriver.com>

Add a missing spinlock protection for play_queue, because
the play_queue may be destroyed when the "playback_work"
work func and "f_audio_out_ep_complete" callback func
operate this paly_queue at the same time.

Fixes: c6994e6f067cf ("USB: gadget: add USB Audio Gadget driver")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Zhang Qiang <qiang.zhang@windriver.com>
---
 v1->v2->v3->v4:
 Add changelog text and Cc tags, Fixes tags.

 drivers/usb/gadget/function/f_uac1_legacy.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/gadget/function/f_uac1_legacy.c b/drivers/usb/gadget/function/f_uac1_legacy.c
index 349deae7cabd..e2d7f69128a0 100644
--- a/drivers/usb/gadget/function/f_uac1_legacy.c
+++ b/drivers/usb/gadget/function/f_uac1_legacy.c
@@ -336,7 +336,9 @@ static int f_audio_out_ep_complete(struct usb_ep *ep, struct usb_request *req)
 
 	/* Copy buffer is full, add it to the play_queue */
 	if (audio_buf_size - copy_buf->actual < req->actual) {
+		spin_lock_irq(&audio->lock);
 		list_add_tail(&copy_buf->list, &audio->play_queue);
+		spin_unlock_irq(&audio->lock);
 		schedule_work(&audio->playback_work);
 		copy_buf = f_audio_buffer_alloc(audio_buf_size);
 		if (IS_ERR(copy_buf))
-- 
2.24.1


^ permalink raw reply related

* Re: [BUG] XHCI getting ZONE_DMA32 memory > than its bus_dma_limit
From: Christoph Hellwig @ 2020-07-07  6:55 UTC (permalink / raw)
  To: Nicolas Saenz Julienne
  Cc: David Rientjes, Robin Murphy, Jeremy Linton,
	linux-arm-kernel@lists.infradead.org, linux-mm,
	linux-usb@vger.kernel.org, Christoph Hellwig,
	linux-kernel@vger.kernel.org, linux-rpi-kernel
In-Reply-To: <32ee3bf222b1966caa98b67a9cec8712817a4b52.camel@suse.de>

On Mon, Jul 06, 2020 at 04:09:36PM +0200, Nicolas Saenz Julienne wrote:
> On Sun, 2020-07-05 at 16:41 -0700, David Rientjes wrote:
> > On Fri, 3 Jul 2020, Robin Murphy wrote:
> > > Or perhaps just get rid of atomic_pool_dma32 (and allocate atomic_pool_dma
> > > from ZONE_DMA32 if !ZONE_DMA). That should make it fall pretty much back in
> > > line while still preserving the potential benefit of the kernel pool for
> > > non-address-constrained devices.
> > > 
> > 
> > I assume it depends on how often we have devices where 
> > __dma_direct_alloc_pages() behavior is required, i.e. what requires the 
> > dma_coherent_ok() checks and altering of the gfp flags to get memory that 
> > works.
> > 
> > Is the idea that getting rid of atomic_pool_dma32 would use GFP_KERNEL 
> > (and atomic_pool_kernel) as the default policy here?  That doesn't do any 
> > dma_coherent_ok() checks so dma_direct_alloc_pages would return from 
> > ZONE_NORMAL without a < 3G check?
> 
> IIUC this is not what Robin proposes.
> 
> The idea is to only have one DMA pool, located in ZONE_DMA, if enabled, and
> ZONE_DMA32 otherwise. This way you're always sure the memory is going to be
> good enough for any device while maintaining the benefits of
> atomic_pool_kernel.

That is how I understood the proposal from Robin and I think it is
the right thing to do.

> > It *seems* like we want to check if dma_coherent_ok() succeeds for ret in 
> > dma_direct_alloc_pages() when allocating from the atomic pool and, based 
> > on criteria that allows fallback, just fall into 
> > __dma_direct_alloc_pages()?
> 
> I suspect I don't have enough perspective here but, isn't that breaking the
> point of having an atomic pool? Wouldn't that generate big latency spikes? I
> can see how audio transfers over USB could be affected by this specifically,
> IIRC those are allocated atomically and have timing constraints.
> 
> That said, if Robin solution works for you, I don't mind having a go at it.

We can't just fall back to __dma_direct_alloc_pages when allocation
from the atomic pool fails, as the atomic pool exists for provide
allocations that require sleeping actions for callers that can't
sleep.

^ permalink raw reply

* Re: 回复: [PATCH v3] usb: gadget: function: fix missing spinlock in f_uac1_legacy
From: Greg KH @ 2020-07-07  6:51 UTC (permalink / raw)
  To: Zhang, Qiang
  Cc: balbi@kernel.org, colin.king@canonical.com,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org
In-Reply-To: <BYAPR11MB26323A0A7687EC2CE3C9E17FFF660@BYAPR11MB2632.namprd11.prod.outlook.com>

A: http://en.wikipedia.org/wiki/Top_post
Q: Were do I find info about this thing called top-posting?
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top


On Tue, Jul 07, 2020 at 02:28:36AM +0000, Zhang, Qiang wrote:
> Hi Greg KH
> In the early submission:
> "commit id c6994e6f067cf0fc4c6cca3d164018b1150916f8" which add USB Audio Gadget driver "   
> the "audio->play_queue" was protected from "audio->lock"
> spinlock in "playback_work" func, But in "f_audio_out_ep_complete" func 
> there is no protection for the operation of this "audio->play_queue". there
> are missing spinlock,  Fix tags should add up here commit?

I really do not understand what you are asking here, sorry.

greg k-h

^ permalink raw reply

* Re: [PATCH 9/9] usb: cdns3: Impovement: simplify *switch_gadet and *switch_host
From: Peter Chen @ 2020-07-07  6:41 UTC (permalink / raw)
  To: Pawel Laszczak
  Cc: balbi@kernel.org, gregkh@linuxfoundation.org,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	rogerq@ti.com, dan.carpenter@oracle.com,
	heikki.krogerus@linux.intel.com, colin.king@canonical.com,
	jpawar@cadence.com, ben.dooks@codethink.co.uk,
	kurahul@cadence.com, sparmar@cadence.com
In-Reply-To: <20200701062004.29908-10-pawell@cadence.com>

On 20-07-01 08:20:04, Pawel Laszczak wrote:
> Patch split function cdns3_drd_switch_gadget and
> cdns3_drd_switch_host into:
> - cdns3_drd_host_on
> - cdns3_drd_host_off
> - cdns3_drd_gadget_on
> - cdns3_drd_gadgett_off
> 
> These functions don't have any shared code so it's better to
> have smaller, faster and easier functions.

Changes the patch title, otherwise:

Reviewed-by: Peter Chen <peter.chen@nxp.com>

Peter
> 
> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> ---
>  drivers/usb/cdns3/drd.c    | 124 ++++++++++++++++++++-----------------
>  drivers/usb/cdns3/drd.h    |   6 +-
>  drivers/usb/cdns3/gadget.c |   4 +-
>  drivers/usb/cdns3/host.c   |   4 +-
>  4 files changed, 76 insertions(+), 62 deletions(-)
> 
> diff --git a/drivers/usb/cdns3/drd.c b/drivers/usb/cdns3/drd.c
> index 8e7673da905e..6234bcd6158a 100644
> --- a/drivers/usb/cdns3/drd.c
> +++ b/drivers/usb/cdns3/drd.c
> @@ -124,85 +124,97 @@ static void cdns3_otg_enable_irq(struct cdns3 *cdns)
>  }
>  
>  /**
> - * cdns3_drd_switch_host - start/stop host
> - * @cdns: Pointer to controller context structure
> - * @on: 1 for start, 0 for stop
> + * cdns3_drd_host_on - start host.
> + * @cdns: Pointer to controller context structure.
>   *
> - * Returns 0 on success otherwise negative errno
> + * Returns 0 on success otherwise negative errno.
>   */
> -int cdns3_drd_switch_host(struct cdns3 *cdns, int on)
> +int cdns3_drd_host_on(struct cdns3 *cdns)
>  {
> -	int ret, val;
> +	u32 val;
> +	int ret;
>  
> -	/* switch OTG core */
> -	if (on) {
> -		writel(OTGCMD_HOST_BUS_REQ | OTGCMD_OTG_DIS,
> -		       &cdns->otg_regs->cmd);
> -
> -		dev_dbg(cdns->dev, "Waiting till Host mode is turned on\n");
> -		ret = readl_poll_timeout_atomic(&cdns->otg_regs->sts, val,
> -						val & OTGSTS_XHCI_READY,
> -						1, 100000);
> -		if (ret) {
> -			dev_err(cdns->dev, "timeout waiting for xhci_ready\n");
> -			return ret;
> -		}
> -	} else {
> -		writel(OTGCMD_HOST_BUS_DROP | OTGCMD_DEV_BUS_DROP |
> -		       OTGCMD_DEV_POWER_OFF | OTGCMD_HOST_POWER_OFF,
> -		       &cdns->otg_regs->cmd);
> -		/* Waiting till H_IDLE state.*/
> -		readl_poll_timeout_atomic(&cdns->otg_regs->state, val,
> -					  !(val & OTGSTATE_HOST_STATE_MASK),
> -					  1, 2000000);
> -	}
> +	/* Enable host mode. */
> +	writel(OTGCMD_HOST_BUS_REQ | OTGCMD_OTG_DIS,
> +	       &cdns->otg_regs->cmd);
>  
> -	return 0;
> +	dev_dbg(cdns->dev, "Waiting till Host mode is turned on\n");
> +	ret = readl_poll_timeout_atomic(&cdns->otg_regs->sts, val,
> +					val & OTGSTS_XHCI_READY, 1, 100000);
> +
> +	if (ret)
> +		dev_err(cdns->dev, "timeout waiting for xhci_ready\n");
> +
> +	return ret;
>  }
>  
>  /**
> - * cdns3_drd_switch_gadget - start/stop gadget
> - * @cdns: Pointer to controller context structure
> - * @on: 1 for start, 0 for stop
> + * cdns3_drd_host_off - stop host.
> + * @cdns: Pointer to controller context structure.
> + */
> +void cdns3_drd_host_off(struct cdns3 *cdns)
> +{
> +	u32 val;
> +
> +	writel(OTGCMD_HOST_BUS_DROP | OTGCMD_DEV_BUS_DROP |
> +	       OTGCMD_DEV_POWER_OFF | OTGCMD_HOST_POWER_OFF,
> +	       &cdns->otg_regs->cmd);
> +
> +	/* Waiting till H_IDLE state.*/
> +	readl_poll_timeout_atomic(&cdns->otg_regs->state, val,
> +				  !(val & OTGSTATE_HOST_STATE_MASK),
> +				  1, 2000000);
> +}
> +
> +/**
> + * cdns3_drd_gadget_on - start gadget.
> + * @cdns: Pointer to controller context structure.
>   *
>   * Returns 0 on success otherwise negative errno
>   */
> -int cdns3_drd_switch_gadget(struct cdns3 *cdns, int on)
> +int cdns3_drd_gadget_on(struct cdns3 *cdns)
>  {
>  	int ret, val;
>  	u32 reg = OTGCMD_OTG_DIS;
>  
>  	/* switch OTG core */
> -	if (on) {
> -		writel(OTGCMD_DEV_BUS_REQ | reg, &cdns->otg_regs->cmd);
> +	writel(OTGCMD_DEV_BUS_REQ | reg, &cdns->otg_regs->cmd);
>  
> -		dev_dbg(cdns->dev, "Waiting till Device mode is turned on\n");
> +	dev_dbg(cdns->dev, "Waiting till Device mode is turned on\n");
>  
> -		ret = readl_poll_timeout_atomic(&cdns->otg_regs->sts, val,
> -						val & OTGSTS_DEV_READY,
> -						1, 100000);
> -		if (ret) {
> -			dev_err(cdns->dev, "timeout waiting for dev_ready\n");
> -			return ret;
> -		}
> -	} else {
> -		/*
> -		 * driver should wait at least 10us after disabling Device
> -		 * before turning-off Device (DEV_BUS_DROP)
> -		 */
> -		usleep_range(20, 30);
> -		writel(OTGCMD_HOST_BUS_DROP | OTGCMD_DEV_BUS_DROP |
> -		       OTGCMD_DEV_POWER_OFF | OTGCMD_HOST_POWER_OFF,
> -		       &cdns->otg_regs->cmd);
> -		/* Waiting till DEV_IDLE state.*/
> -		readl_poll_timeout_atomic(&cdns->otg_regs->state, val,
> -					  !(val & OTGSTATE_DEV_STATE_MASK),
> -					  1, 2000000);
> +	ret = readl_poll_timeout_atomic(&cdns->otg_regs->sts, val,
> +					val & OTGSTS_DEV_READY,
> +					1, 100000);
> +	if (ret) {
> +		dev_err(cdns->dev, "timeout waiting for dev_ready\n");
> +		return ret;
>  	}
>  
>  	return 0;
>  }
>  
> +/**
> + * cdns3_drd_gadget_off - stop gadget.
> + * @cdns: Pointer to controller context structure.
> + */
> +void cdns3_drd_gadget_off(struct cdns3 *cdns)
> +{
> +	u32 val;
> +
> +	/*
> +	 * Driver should wait at least 10us after disabling Device
> +	 * before turning-off Device (DEV_BUS_DROP).
> +	 */
> +	usleep_range(20, 30);
> +	writel(OTGCMD_HOST_BUS_DROP | OTGCMD_DEV_BUS_DROP |
> +	       OTGCMD_DEV_POWER_OFF | OTGCMD_HOST_POWER_OFF,
> +	       &cdns->otg_regs->cmd);
> +	/* Waiting till DEV_IDLE state.*/
> +	readl_poll_timeout_atomic(&cdns->otg_regs->state, val,
> +				  !(val & OTGSTATE_DEV_STATE_MASK),
> +				  1, 2000000);
> +}
> +
>  /**
>   * cdns3_init_otg_mode - initialize drd controller
>   * @cdns: Pointer to controller context structure
> diff --git a/drivers/usb/cdns3/drd.h b/drivers/usb/cdns3/drd.h
> index 3889fead9df1..7e7cf7fa2dd3 100644
> --- a/drivers/usb/cdns3/drd.h
> +++ b/drivers/usb/cdns3/drd.h
> @@ -163,8 +163,10 @@ int cdns3_get_vbus(struct cdns3 *cdns);
>  int cdns3_drd_init(struct cdns3 *cdns);
>  int cdns3_drd_exit(struct cdns3 *cdns);
>  int cdns3_drd_update_mode(struct cdns3 *cdns);
> -int cdns3_drd_switch_gadget(struct cdns3 *cdns, int on);
> -int cdns3_drd_switch_host(struct cdns3 *cdns, int on);
> +int cdns3_drd_gadget_on(struct cdns3 *cdns);
> +void cdns3_drd_gadget_off(struct cdns3 *cdns);
> +int cdns3_drd_host_on(struct cdns3 *cdns);
> +void cdns3_drd_host_off(struct cdns3 *cdns);
>  int cdns3_set_mode(struct cdns3 *cdns, enum usb_dr_mode mode);
>  
>  #endif /* __LINUX_CDNS3_DRD */
> diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
> index c303ab7c62d1..6ea6839a2a8c 100644
> --- a/drivers/usb/cdns3/gadget.c
> +++ b/drivers/usb/cdns3/gadget.c
> @@ -3014,7 +3014,7 @@ void cdns3_gadget_exit(struct cdns3 *cdns)
>  	kfree(priv_dev->zlp_buf);
>  	kfree(priv_dev);
>  	cdns->gadget_dev = NULL;
> -	cdns3_drd_switch_gadget(cdns, 0);
> +	cdns3_drd_gadget_off(cdns);
>  }
>  
>  static int cdns3_gadget_start(struct cdns3 *cdns)
> @@ -3145,7 +3145,7 @@ static int __cdns3_gadget_init(struct cdns3 *cdns)
>  		return ret;
>  	}
>  
> -	cdns3_drd_switch_gadget(cdns, 1);
> +	cdns3_drd_gadget_on(cdns);
>  	pm_runtime_get_sync(cdns->dev);
>  
>  	ret = cdns3_gadget_start(cdns);
> diff --git a/drivers/usb/cdns3/host.c b/drivers/usb/cdns3/host.c
> index ad788bf3fe4f..36c63d9ecd37 100644
> --- a/drivers/usb/cdns3/host.c
> +++ b/drivers/usb/cdns3/host.c
> @@ -19,7 +19,7 @@ static int __cdns3_host_init(struct cdns3 *cdns)
>  	struct platform_device *xhci;
>  	int ret;
>  
> -	cdns3_drd_switch_host(cdns, 1);
> +	cdns3_drd_host_on(cdns);
>  
>  	xhci = platform_device_alloc("xhci-hcd", PLATFORM_DEVID_AUTO);
>  	if (!xhci) {
> @@ -53,7 +53,7 @@ static void cdns3_host_exit(struct cdns3 *cdns)
>  {
>  	platform_device_unregister(cdns->host_dev);
>  	cdns->host_dev = NULL;
> -	cdns3_drd_switch_host(cdns, 0);
> +	cdns3_drd_host_off(cdns);
>  }
>  
>  int cdns3_host_init(struct cdns3 *cdns)
> -- 
> 2.17.1
> 

-- 

Thanks,
Peter Chen

^ permalink raw reply

* Re: [PATCH 8/9] usb: cdns3: Improvement: removed overwriting some error code
From: Peter Chen @ 2020-07-07  6:38 UTC (permalink / raw)
  To: Pawel Laszczak
  Cc: balbi@kernel.org, gregkh@linuxfoundation.org,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	rogerq@ti.com, dan.carpenter@oracle.com,
	heikki.krogerus@linux.intel.com, colin.king@canonical.com,
	jpawar@cadence.com, ben.dooks@codethink.co.uk,
	kurahul@cadence.com, sparmar@cadence.com
In-Reply-To: <20200701062004.29908-9-pawell@cadence.com>

On 20-07-01 08:20:03, Pawel Laszczak wrote:
> Some error code can be preserved, so we can remove overwriting
> error code returned by some functions.

Improve the patch title, otherwise:

Reviewed-by: Peter Chen <peter.chen@nxp.com>

Peter
> 
> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> ---
>  drivers/usb/cdns3/core.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
> index 591186987245..01155ab73930 100644
> --- a/drivers/usb/cdns3/core.c
> +++ b/drivers/usb/cdns3/core.c
> @@ -347,7 +347,6 @@ static int cdns3_role_set(struct usb_role_switch *sw, enum usb_role role)
>  		case USB_ROLE_HOST:
>  			break;
>  		default:
> -			ret = -EPERM;
>  			goto pm_put;
>  		}
>  	}
> @@ -358,17 +357,14 @@ static int cdns3_role_set(struct usb_role_switch *sw, enum usb_role role)
>  		case USB_ROLE_DEVICE:
>  			break;
>  		default:
> -			ret = -EPERM;
>  			goto pm_put;
>  		}
>  	}
>  
>  	cdns3_role_stop(cdns);
>  	ret = cdns3_role_start(cdns, role);
> -	if (ret) {
> +	if (ret)
>  		dev_err(cdns->dev, "set role %d has failed\n", role);
> -		ret = -EPERM;
> -	}
>  
>  pm_put:
>  	pm_runtime_put_sync(cdns->dev);
> @@ -393,7 +389,7 @@ static int cdns3_probe(struct platform_device *pdev)
>  	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
>  	if (ret) {
>  		dev_err(dev, "error setting dma mask: %d\n", ret);
> -		return -ENODEV;
> +		return ret;
>  	}
>  
>  	cdns = devm_kzalloc(dev, sizeof(*cdns), GFP_KERNEL);
> -- 
> 2.17.1
> 

-- 

Thanks,
Peter Chen

^ permalink raw reply

* Re: [PATCH 7/9] usb: cdns3: Improvement: removed 'goto not_otg'
From: Peter Chen @ 2020-07-07  6:33 UTC (permalink / raw)
  To: Pawel Laszczak
  Cc: balbi@kernel.org, gregkh@linuxfoundation.org,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	rogerq@ti.com, dan.carpenter@oracle.com,
	heikki.krogerus@linux.intel.com, colin.king@canonical.com,
	jpawar@cadence.com, ben.dooks@codethink.co.uk,
	kurahul@cadence.com, sparmar@cadence.com
In-Reply-To: <20200701062004.29908-8-pawell@cadence.com>

On 20-07-01 08:20:02, Pawel Laszczak wrote:
> Patch removes 'goto not_otg' instruction from
>  cdnsp_hw_role_state_machine function.

Should be cdns3_hw_role_state_machine.

Peter

> 
> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> ---
>  drivers/usb/cdns3/core.c | 20 +++++++++-----------
>  1 file changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
> index c3dac945f63d..591186987245 100644
> --- a/drivers/usb/cdns3/core.c
> +++ b/drivers/usb/cdns3/core.c
> @@ -191,11 +191,17 @@ static int cdns3_core_init_role(struct cdns3 *cdns)
>   */
>  static enum usb_role cdns3_hw_role_state_machine(struct cdns3 *cdns)
>  {
> -	enum usb_role role;
> +	enum usb_role role = USB_ROLE_NONE;
>  	int id, vbus;
>  
> -	if (cdns->dr_mode != USB_DR_MODE_OTG)
> -		goto not_otg;
> +	if (cdns->dr_mode != USB_DR_MODE_OTG) {
> +		if (cdns3_is_host(cdns))
> +			role = USB_ROLE_HOST;
> +		if (cdns3_is_device(cdns))
> +			role = USB_ROLE_DEVICE;
> +
> +		return role;
> +	}
>  
>  	id = cdns3_get_id(cdns);
>  	vbus = cdns3_get_vbus(cdns);
> @@ -232,14 +238,6 @@ static enum usb_role cdns3_hw_role_state_machine(struct cdns3 *cdns)
>  	dev_dbg(cdns->dev, "role %d -> %d\n", cdns->role, role);
>  
>  	return role;
> -
> -not_otg:
> -	if (cdns3_is_host(cdns))
> -		role = USB_ROLE_HOST;
> -	if (cdns3_is_device(cdns))
> -		role = USB_ROLE_DEVICE;
> -
> -	return role;
>  }
>  
>  static int cdns3_idle_role_start(struct cdns3 *cdns)
> -- 
> 2.17.1
> 

-- 

Thanks,
Peter Chen

^ permalink raw reply

* Re: [PATCH 6/9] usb: cdns3: Added CDNS3_ID_PERIPHERAL and CDNS3_ID_HOST
From: Peter Chen @ 2020-07-07  6:30 UTC (permalink / raw)
  To: Pawel Laszczak
  Cc: balbi@kernel.org, gregkh@linuxfoundation.org,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	rogerq@ti.com, dan.carpenter@oracle.com,
	heikki.krogerus@linux.intel.com, colin.king@canonical.com,
	jpawar@cadence.com, ben.dooks@codethink.co.uk,
	kurahul@cadence.com, sparmar@cadence.com
In-Reply-To: <20200701062004.29908-7-pawell@cadence.com>

On 20-07-01 08:20:01, Pawel Laszczak wrote:
> Patch adds 2 definitions that make it easier to understand the code.
> 
> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> ---
>  drivers/usb/cdns3/drd.c | 4 ++--
>  drivers/usb/cdns3/drd.h | 3 +++
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/cdns3/drd.c b/drivers/usb/cdns3/drd.c
> index 6fe092c828b3..8e7673da905e 100644
> --- a/drivers/usb/cdns3/drd.c
> +++ b/drivers/usb/cdns3/drd.c
> @@ -87,7 +87,7 @@ bool cdns3_is_host(struct cdns3 *cdns)
>  {
>  	if (cdns->dr_mode == USB_DR_MODE_HOST)
>  		return true;
> -	else if (!cdns3_get_id(cdns))
> +	else if (cdns3_get_id(cdns) == CDNS3_ID_HOST)
>  		return true;
>  
>  	return false;
> @@ -98,7 +98,7 @@ bool cdns3_is_device(struct cdns3 *cdns)
>  	if (cdns->dr_mode == USB_DR_MODE_PERIPHERAL)
>  		return true;
>  	else if (cdns->dr_mode == USB_DR_MODE_OTG)
> -		if (cdns3_get_id(cdns))
> +		if (cdns3_get_id(cdns) == CDNS3_ID_PERIPHERAL)
>  			return true;
>  
>  	return false;
> diff --git a/drivers/usb/cdns3/drd.h b/drivers/usb/cdns3/drd.h
> index 35b6d459ee58..3889fead9df1 100644
> --- a/drivers/usb/cdns3/drd.h
> +++ b/drivers/usb/cdns3/drd.h
> @@ -153,6 +153,9 @@ struct cdns3_otg_common_regs {
>  /* Only for CDNS3_CONTROLLER_V0 version */
>  #define OVERRIDE_IDPULLUP_V0		BIT(24)
>  
> +#define CDNS3_ID_PERIPHERAL		1
> +#define CDNS3_ID_HOST			0
> +

Instead of adding MACRO, I prefer adding comments at the code to indicate
"ID=0" means it is host mode, "ID=1" means it is device mode.

-- 

Thanks,
Peter Chen

^ permalink raw reply

* Re: [PATCH 3/9] usb: cnds3: Improvement: deleted !=
From: Peter Chen @ 2020-07-07  6:24 UTC (permalink / raw)
  To: Pawel Laszczak
  Cc: balbi@kernel.org, gregkh@linuxfoundation.org,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	rogerq@ti.com, dan.carpenter@oracle.com,
	heikki.krogerus@linux.intel.com, colin.king@canonical.com,
	jpawar@cadence.com, ben.dooks@codethink.co.uk,
	kurahul@cadence.com, sparmar@cadence.com
In-Reply-To: <20200701062004.29908-4-pawell@cadence.com>

On 20-07-01 08:19:58, Pawel Laszczak wrote:
> Patch deletes unnecessary != from condition statement ini cdns3_drd_init
> function.

Change the title.

%s/ini/in

Peter
> 
> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> ---
>  drivers/usb/cdns3/drd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/cdns3/drd.c b/drivers/usb/cdns3/drd.c
> index 4939a568d8a2..6d2da504ad49 100644
> --- a/drivers/usb/cdns3/drd.c
> +++ b/drivers/usb/cdns3/drd.c
> @@ -365,7 +365,7 @@ int cdns3_drd_init(struct cdns3 *cdns)
>  	}
>  
>  	state = readl(&cdns->otg_regs->sts);
> -	if (OTGSTS_OTG_NRDY(state) != 0) {
> +	if (OTGSTS_OTG_NRDY(state)) {
>  		dev_err(cdns->dev, "Cadence USB3 OTG device not ready\n");
>  		return -ENODEV;
>  	}
> -- 
> 2.17.1
> 

-- 

Thanks,
Peter Chen

^ permalink raw reply

* Re: [PATCH 2/9] usb: cdns3: Improvement: removed not needed variables initialization
From: Peter Chen @ 2020-07-07  6:23 UTC (permalink / raw)
  To: Pawel Laszczak
  Cc: balbi@kernel.org, gregkh@linuxfoundation.org,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	rogerq@ti.com, dan.carpenter@oracle.com,
	heikki.krogerus@linux.intel.com, colin.king@canonical.com,
	jpawar@cadence.com, ben.dooks@codethink.co.uk,
	kurahul@cadence.com, sparmar@cadence.com
In-Reply-To: <20200701062004.29908-3-pawell@cadence.com>

On 20-07-01 08:19:57, Pawel Laszczak wrote:
> Patch remove some variables initialization from core.c and drd.c
> file.
> 
> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> ---
>  drivers/usb/cdns3/core.c |  4 ++--
>  drivers/usb/cdns3/drd.c  | 19 +++++++++----------
>  2 files changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
> index eaafa6bd2a50..c3dac945f63d 100644
> --- a/drivers/usb/cdns3/core.c
> +++ b/drivers/usb/cdns3/core.c
> @@ -86,7 +86,7 @@ static int cdns3_core_init_role(struct cdns3 *cdns)
>  	struct device *dev = cdns->dev;
>  	enum usb_dr_mode best_dr_mode;
>  	enum usb_dr_mode dr_mode;
> -	int ret = 0;
> +	int ret;
>  
>  	dr_mode = usb_get_dr_mode(dev);
>  	cdns->role = USB_ROLE_NONE;
> @@ -177,7 +177,7 @@ static int cdns3_core_init_role(struct cdns3 *cdns)
>  		goto err;
>  	}
>  
> -	return ret;
> +	return 0;
>  err:
>  	cdns3_exit_roles(cdns);
>  	return ret;
> diff --git a/drivers/usb/cdns3/drd.c b/drivers/usb/cdns3/drd.c
> index 58089841ed52..4939a568d8a2 100644
> --- a/drivers/usb/cdns3/drd.c
> +++ b/drivers/usb/cdns3/drd.c
> @@ -29,7 +29,6 @@
>   */
>  int cdns3_set_mode(struct cdns3 *cdns, enum usb_dr_mode mode)
>  {
> -	int ret = 0;
>  	u32 reg;
>  
>  	switch (mode) {
> @@ -61,7 +60,7 @@ int cdns3_set_mode(struct cdns3 *cdns, enum usb_dr_mode mode)
>  		return -EINVAL;
>  	}
>  
> -	return ret;
> +	return 0;
>  }
>  
>  int cdns3_get_id(struct cdns3 *cdns)
> @@ -134,11 +133,11 @@ static void cdns3_otg_enable_irq(struct cdns3 *cdns)
>  int cdns3_drd_switch_host(struct cdns3 *cdns, int on)
>  {
>  	int ret, val;
> -	u32 reg = OTGCMD_OTG_DIS;
>  
>  	/* switch OTG core */
>  	if (on) {
> -		writel(OTGCMD_HOST_BUS_REQ | reg, &cdns->otg_regs->cmd);
> +		writel(OTGCMD_HOST_BUS_REQ | OTGCMD_OTG_DIS,
> +		       &cdns->otg_regs->cmd);
>  
>  		dev_dbg(cdns->dev, "Waiting till Host mode is turned on\n");
>  		ret = readl_poll_timeout_atomic(&cdns->otg_regs->sts, val,
> @@ -212,7 +211,7 @@ int cdns3_drd_switch_gadget(struct cdns3 *cdns, int on)
>   */
>  static int cdns3_init_otg_mode(struct cdns3 *cdns)
>  {
> -	int ret = 0;
> +	int ret;
>  
>  	cdns3_otg_disable_irq(cdns);
>  	/* clear all interrupts */
> @@ -223,7 +222,8 @@ static int cdns3_init_otg_mode(struct cdns3 *cdns)
>  		return ret;
>  
>  	cdns3_otg_enable_irq(cdns);
> -	return ret;
> +
> +	return 0;
>  }
>  
>  /**
> @@ -234,7 +234,7 @@ static int cdns3_init_otg_mode(struct cdns3 *cdns)
>   */
>  int cdns3_drd_update_mode(struct cdns3 *cdns)
>  {
> -	int ret = 0;
> +	int ret;
>  
>  	switch (cdns->dr_mode) {
>  	case USB_DR_MODE_PERIPHERAL:
> @@ -307,8 +307,8 @@ static irqreturn_t cdns3_drd_irq(int irq, void *data)
>  int cdns3_drd_init(struct cdns3 *cdns)
>  {
>  	void __iomem *regs;
> -	int ret = 0;
>  	u32 state;
> +	int ret;
>  
>  	regs = devm_ioremap_resource(cdns->dev, &cdns->otg_res);
>  	if (IS_ERR(regs))
> @@ -359,7 +359,6 @@ int cdns3_drd_init(struct cdns3 *cdns)
>  					cdns3_drd_thread_irq,
>  					IRQF_SHARED,
>  					dev_name(cdns->dev), cdns);
> -
>  	if (ret) {
>  		dev_err(cdns->dev, "couldn't get otg_irq\n");
>  		return ret;
> @@ -371,7 +370,7 @@ int cdns3_drd_init(struct cdns3 *cdns)
>  		return -ENODEV;
>  	}
>  
> -	return ret;
> +	return 0;

Is this necessary?

-- 

Thanks,
Peter Chen

^ permalink raw reply

* [PATCH] USB: serial: ch341: fix simulated-break comment
From: Johan Hovold @ 2020-07-07  6:19 UTC (permalink / raw)
  To: linux-usb, Michael Hanselmann; +Cc: Johan Hovold

On devices which do not support break signalling a break condition is
simulated by sending a NUL byte at the lowest possible speed. The break
condition will be 9 bit periods long (start bit and eight data bits),
but the transmission itself also includes the stop bit.

Signed-off-by: Johan Hovold <johan@kernel.org>
---

Hi Michael,

I reread the break-end comment and found it a bit confusing still. The
below seems more correct to me. I'm assuming you did not intend to add
an additional bit period as margin?

Johan



 drivers/usb/serial/ch341.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index 011d7953f087..27a2a62777c9 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -604,9 +604,8 @@ static void ch341_simulate_break(struct tty_struct *tty, int break_state)
 		}
 
 		/*
-		 * Compute expected transmission duration and add a single bit
-		 * of safety margin (the actual NUL byte transmission is 8 bits
-		 * plus one stop bit).
+		 * Compute expected transmission duration (including stop
+		 * bit).
 		 */
 		priv->break_end = jiffies + (10 * HZ / CH341_MIN_BPS);
 
-- 
2.26.2


^ permalink raw reply related

* Re: [PATCH 1/9] usb: cdns3: Improvement: removed cdns3_get_current_role_driver function.
From: Peter Chen @ 2020-07-07  6:20 UTC (permalink / raw)
  To: Pawel Laszczak
  Cc: balbi@kernel.org, gregkh@linuxfoundation.org,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	rogerq@ti.com, dan.carpenter@oracle.com,
	heikki.krogerus@linux.intel.com, colin.king@canonical.com,
	jpawar@cadence.com, ben.dooks@codethink.co.uk,
	kurahul@cadence.com, sparmar@cadence.com
In-Reply-To: <20200701062004.29908-2-pawell@cadence.com>

On 20-07-01 08:19:56, Pawel Laszczak wrote:
> Function is not used in driver so it can be removed.
> 
> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> ---
>  drivers/usb/cdns3/core.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
> index 19bbb5b7e6b6..eaafa6bd2a50 100644
> --- a/drivers/usb/cdns3/core.c
> +++ b/drivers/usb/cdns3/core.c
> @@ -27,13 +27,6 @@
>  
>  static int cdns3_idle_init(struct cdns3 *cdns);
>  
> -static inline
> -struct cdns3_role_driver *cdns3_get_current_role_driver(struct cdns3 *cdns)
> -{
> -	WARN_ON(!cdns->roles[cdns->role]);
> -	return cdns->roles[cdns->role];
> -}
> -

Change the title for the patch, otherwise:
Reviewed-by: Peter Chen <peter.chen@nxp.com>

-- 

Thanks,
Peter Chen

^ permalink raw reply

* Re: [PATCH 0/9] usb: cdns3: Improvements for cdns3 DRD code
From: Peter Chen @ 2020-07-07  6:18 UTC (permalink / raw)
  To: Pawel Laszczak
  Cc: balbi@kernel.org, gregkh@linuxfoundation.org,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	rogerq@ti.com, dan.carpenter@oracle.com,
	heikki.krogerus@linux.intel.com, colin.king@canonical.com,
	jpawar@cadence.com, ben.dooks@codethink.co.uk,
	kurahul@cadence.com, sparmar@cadence.com
In-Reply-To: <20200701062004.29908-1-pawell@cadence.com>

On 20-07-01 08:19:55, Pawel Laszczak wrote:
> Series introduce some improvements to drd.c, drd.h and core.c files of 
> cdns3 driver.
> 
> Except for the first (1/9) patch that removes not needed function,
> the rest patches make improvements suggested by Dan Carpenter
> during reviewing CDNSP driver.
> CDNSP is the next Cadence USBSSP  driver which will be upstreamed.
> The DRD part is similar for both CDNS3 and CDNSP and Greg suggested
> to merge the similar part of DRD code. As first step I want to merge 
> some improvemnts.
> 
> Pawel Laszczak (9):
>   usb: cdns3: Improvement: removed cdns3_get_current_role_driver
>     function.
>   usb: cdns3: Improvement: removed not needed variables initialization
>   usb: cnds3: Improvement: deleted !=
>   usb: cdns3: Improvement: return IRQ_NONE explicitly.
>   usb: cdns3: Improvement: changed return type from int to bool
>   usb: cdns3: Added CDNS3_ID_PERIPHERAL and CDNS3_ID_HOST
>   usb: cdns3: Improvement: removed 'goto not_otg'
>   usb: cdns3: Improvement: removed overwriting some error code
>   usb: cdns3: Impovement: simplify *switch_gadet and *switch_host
> 

There is no part named "Improvement", please replace it with
drd/core/gadget etc.

Peter

>  drivers/usb/cdns3/core.c   |  39 +++------
>  drivers/usb/cdns3/drd.c    | 165 ++++++++++++++++++++-----------------
>  drivers/usb/cdns3/drd.h    |  13 ++-
>  drivers/usb/cdns3/gadget.c |   4 +-
>  drivers/usb/cdns3/host.c   |   4 +-
>  5 files changed, 114 insertions(+), 111 deletions(-)
> 
> -- 
> 2.17.1
> 

-- 

Thanks,
Peter Chen

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox