[PATCH 0/8] Bugs fixes and improvements to Broadcom BDC driver

[PATCH 0/8] Bugs fixes and improvements to Broadcom BDC driver

[Al Cooper]

Bug fixes and improvements to the USB Broadcom Device
Controller (BDC) driver.

Al Cooper (7):
  usb: gadget: bdc: Fix misleading register names
  usb: bdc: Add Device Tree binding document for Broadcom BDC driver
  usb: bdc: Add clock enable for new chips with a separate BDC clock
  usb: bdc: Small code cleanup
  usb: bdc: Add support for suspend/resume
  usb: bdc: fix "xsf for ep not enabled" errror
  usb: bdc: Enable in Kconfig for ARCH_BRCMSTB systems

Florian Fainelli (1):
  usb: gadget: bdc: hook a quick Device Tree compatible string

 .../devicetree/bindings/usb/brcm,bdc-udc.txt       | 28 ++++++++
 drivers/usb/gadget/udc/bdc/Kconfig                 |  1 +
 drivers/usb/gadget/udc/bdc/bdc.h                   | 17 ++---
 drivers/usb/gadget/udc/bdc/bdc_core.c              | 74 ++++++++++++++++++++--
 drivers/usb/gadget/udc/bdc/bdc_dbg.c               | 16 ++---
 drivers/usb/gadget/udc/bdc/bdc_ep.c                |  4 +-
 drivers/usb/gadget/udc/bdc/bdc_udc.c               |  7 +-
 7 files changed, 119 insertions(+), 28 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/brcm,bdc-udc.txt

-- 
1.9.0.138.g2de3478

[PATCH 1/8] usb: gadget: bdc: Fix misleading register names

[Al Cooper]

The BDC endpoint status registers 0-7 were originally each going
to be an array of regsiters. This was later changed to being a
single register. The register definitions are being changed from:
"#define BDC_EPSTS0(n)  (0x60 + (n * 0x10))"
to
"#define BDC_EPSTS0	0x60"
to reflect this change and to avoid future coding mistakes.

Signed-off-by: Al Cooper <alcooperx@gmail.com>
---
 drivers/usb/gadget/udc/bdc/bdc.h     | 16 ++++++++--------
 drivers/usb/gadget/udc/bdc/bdc_dbg.c | 16 ++++++++--------
 drivers/usb/gadget/udc/bdc/bdc_ep.c  |  4 ++--
 3 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/gadget/udc/bdc/bdc.h b/drivers/usb/gadget/udc/bdc/bdc.h
index 916d471..3664808 100644
--- a/drivers/usb/gadget/udc/bdc/bdc.h
+++ b/drivers/usb/gadget/udc/bdc/bdc.h
@@ -83,14 +83,14 @@
 
 #define BDC_DVCSA	0x50
 #define BDC_DVCSB	0x54
-#define BDC_EPSTS0(n)	(0x60 + (n * 0x10))
-#define BDC_EPSTS1(n)	(0x64 + (n * 0x10))
-#define BDC_EPSTS2(n)	(0x68 + (n * 0x10))
-#define BDC_EPSTS3(n)	(0x6c + (n * 0x10))
-#define BDC_EPSTS4(n)	(0x70 + (n * 0x10))
-#define BDC_EPSTS5(n)	(0x74 + (n * 0x10))
-#define BDC_EPSTS6(n)	(0x78 + (n * 0x10))
-#define BDC_EPSTS7(n)	(0x7c + (n * 0x10))
+#define BDC_EPSTS0	0x60
+#define BDC_EPSTS1	0x64
+#define BDC_EPSTS2	0x68
+#define BDC_EPSTS3	0x6c
+#define BDC_EPSTS4	0x70
+#define BDC_EPSTS5	0x74
+#define BDC_EPSTS6	0x78
+#define BDC_EPSTS7	0x7c
 #define BDC_SRRBAL(n)	(0x200 + (n * 0x10))
 #define BDC_SRRBAH(n)	(0x204 + (n * 0x10))
 #define BDC_SRRINT(n)	(0x208 + (n * 0x10))
diff --git a/drivers/usb/gadget/udc/bdc/bdc_dbg.c b/drivers/usb/gadget/udc/bdc/bdc_dbg.c
index 5945dbc..ac98f6f 100644
--- a/drivers/usb/gadget/udc/bdc/bdc_dbg.c
+++ b/drivers/usb/gadget/udc/bdc/bdc_dbg.c
@@ -40,28 +40,28 @@ void bdc_dump_epsts(struct bdc *bdc)
 {
 	u32 temp;
 
-	temp = bdc_readl(bdc->regs, BDC_EPSTS0(0));
+	temp = bdc_readl(bdc->regs, BDC_EPSTS0);
 	dev_vdbg(bdc->dev, "BDC_EPSTS0:0x%08x\n", temp);
 
-	temp = bdc_readl(bdc->regs, BDC_EPSTS1(0));
+	temp = bdc_readl(bdc->regs, BDC_EPSTS1);
 	dev_vdbg(bdc->dev, "BDC_EPSTS1:0x%x\n", temp);
 
-	temp = bdc_readl(bdc->regs, BDC_EPSTS2(0));
+	temp = bdc_readl(bdc->regs, BDC_EPSTS2);
 	dev_vdbg(bdc->dev, "BDC_EPSTS2:0x%08x\n", temp);
 
-	temp = bdc_readl(bdc->regs, BDC_EPSTS3(0));
+	temp = bdc_readl(bdc->regs, BDC_EPSTS3);
 	dev_vdbg(bdc->dev, "BDC_EPSTS3:0x%08x\n", temp);
 
-	temp = bdc_readl(bdc->regs, BDC_EPSTS4(0));
+	temp = bdc_readl(bdc->regs, BDC_EPSTS4);
 	dev_vdbg(bdc->dev, "BDC_EPSTS4:0x%08x\n", temp);
 
-	temp = bdc_readl(bdc->regs, BDC_EPSTS5(0));
+	temp = bdc_readl(bdc->regs, BDC_EPSTS5);
 	dev_vdbg(bdc->dev, "BDC_EPSTS5:0x%08x\n", temp);
 
-	temp = bdc_readl(bdc->regs, BDC_EPSTS6(0));
+	temp = bdc_readl(bdc->regs, BDC_EPSTS6);
 	dev_vdbg(bdc->dev, "BDC_EPSTS6:0x%08x\n", temp);
 
-	temp = bdc_readl(bdc->regs, BDC_EPSTS7(0));
+	temp = bdc_readl(bdc->regs, BDC_EPSTS7);
 	dev_vdbg(bdc->dev, "BDC_EPSTS7:0x%08x\n", temp);
 }
 
diff --git a/drivers/usb/gadget/udc/bdc/bdc_ep.c b/drivers/usb/gadget/udc/bdc/bdc_ep.c
index ff1ef24..bfd8f7a 100644
--- a/drivers/usb/gadget/udc/bdc/bdc_ep.c
+++ b/drivers/usb/gadget/udc/bdc/bdc_ep.c
@@ -777,9 +777,9 @@ static int ep_dequeue(struct bdc_ep *ep, struct bdc_req *req)
 	 */
 
 	/* The current hw dequeue pointer */
-	tmp_32 = bdc_readl(bdc->regs, BDC_EPSTS0(0));
+	tmp_32 = bdc_readl(bdc->regs, BDC_EPSTS0);
 	deq_ptr_64 = tmp_32;
-	tmp_32 = bdc_readl(bdc->regs, BDC_EPSTS1(0));
+	tmp_32 = bdc_readl(bdc->regs, BDC_EPSTS1);
 	deq_ptr_64 |= ((u64)tmp_32 << 32);
 
 	/* we have the dma addr of next bd that will be fetched by hardware */
-- 
1.9.0.138.g2de3478

[PATCH 2/8] usb: bdc: Add Device Tree binding document for Broadcom BDC driver

[Al Cooper]

Add Device Tree binding document for Broadcom USB Device
Controller (BDC).

Signed-off-by: Al Cooper <alcooperx@gmail.com>
---
 .../devicetree/bindings/usb/brcm,bdc-udc.txt       | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/brcm,bdc-udc.txt

diff --git a/Documentation/devicetree/bindings/usb/brcm,bdc-udc.txt b/Documentation/devicetree/bindings/usb/brcm,bdc-udc.txt
new file mode 100644
index 0000000..4eeaddb
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/brcm,bdc-udc.txt
@@ -0,0 +1,28 @@
+Broadcom USB Device Controller (BDC)
+====================================
+
+Required properties:
+
+- compatible: must be one of:
+                "brcm,bdc-udc-v0.16"
+                "brcm,bdc-udc"
+- reg: the base register address and length
+- interrupts: ther interrupt line for this controller
+
+Optional properties:
+
+On Broadcom STB platforms, these properties are required:
+
+- phys: phandle to the USB PHY blocks
+- clocks: phandle to the functional clock of this block
+
+Example:
+
+        udc@f0b02000 {
+                status = "disabled";
+                compatible = "brcm,bdc-udc-v0.16";
+                reg = <0xf0b02000 0xfc4>;
+                interrupts = <0x0 0x60 0x0>;
+                phys = <&usbphy_0 0x0>;
+                clocks = <&sw_usbd>;
+        };
-- 
1.9.0.138.g2de3478

[PATCH 3/8] usb: bdc: Add clock enable for new chips with a separate BDC clock

[Al Cooper]

Newer SoC's have added a BDC clock to the Device Tree, so get
and enable it.

Signed-off-by: Al Cooper <alcooperx@gmail.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/usb/gadget/udc/bdc/bdc_core.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/usb/gadget/udc/bdc/bdc_core.c b/drivers/usb/gadget/udc/bdc/bdc_core.c
index ccb9c21..3bd82d2 100644
--- a/drivers/usb/gadget/udc/bdc/bdc_core.c
+++ b/drivers/usb/gadget/udc/bdc/bdc_core.c
@@ -27,6 +27,7 @@
 #include <linux/moduleparam.h>
 #include <linux/usb/ch9.h>
 #include <linux/usb/gadget.h>
+#include <linux/clk.h>
 
 #include "bdc.h"
 #include "bdc_dbg.h"
@@ -452,8 +453,22 @@ static int bdc_probe(struct platform_device *pdev)
 	int irq;
 	u32 temp;
 	struct device *dev = &pdev->dev;
+	struct clk *clk;
 
 	dev_dbg(dev, "%s()\n", __func__);
+
+	clk = devm_clk_get(dev, "sw_usbd");
+	if (IS_ERR(clk)) {
+		dev_info(dev, "Clock not found in Device Tree\n");
+		clk = NULL;
+	}
+
+	ret = clk_prepare_enable(clk);
+	if (ret) {
+		dev_err(dev, "could not enable clock\n");
+		return ret;
+	}
+
 	bdc = devm_kzalloc(dev, sizeof(*bdc), GFP_KERNEL);
 	if (!bdc)
 		return -ENOMEM;
-- 
1.9.0.138.g2de3478

[PATCH 4/8] usb: bdc: Small code cleanup

[Al Cooper]

Signed-off-by: Al Cooper <alcooperx@gmail.com>
---
 drivers/usb/gadget/udc/bdc/bdc_core.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/gadget/udc/bdc/bdc_core.c b/drivers/usb/gadget/udc/bdc/bdc_core.c
index 3bd82d2..621328f 100644
--- a/drivers/usb/gadget/udc/bdc/bdc_core.c
+++ b/drivers/usb/gadget/udc/bdc/bdc_core.c
@@ -488,28 +488,29 @@ static int bdc_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, bdc);
 	bdc->irq = irq;
 	bdc->dev = dev;
-	dev_dbg(bdc->dev, "bdc->regs: %p irq=%d\n", bdc->regs, bdc->irq);
+	dev_dbg(dev, "bdc->regs: %p irq=%d\n", bdc->regs, bdc->irq);
 
 	temp = bdc_readl(bdc->regs, BDC_BDCSC);
 	if ((temp & BDC_P64) &&
 			!dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64))) {
-		dev_dbg(bdc->dev, "Using 64-bit address\n");
+		dev_dbg(dev, "Using 64-bit address\n");
 	} else {
-		ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
+		ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
 		if (ret) {
-			dev_err(bdc->dev, "No suitable DMA config available, abort\n");
+			dev_err(dev,
+				"No suitable DMA config available, abort\n");
 			return -ENOTSUPP;
 		}
-		dev_dbg(bdc->dev, "Using 32-bit address\n");
+		dev_dbg(dev, "Using 32-bit address\n");
 	}
 	ret = bdc_hw_init(bdc);
 	if (ret) {
-		dev_err(bdc->dev, "BDC init failure:%d\n", ret);
+		dev_err(dev, "BDC init failure:%d\n", ret);
 		return ret;
 	}
 	ret = bdc_udc_init(bdc);
 	if (ret) {
-		dev_err(bdc->dev, "BDC Gadget init failure:%d\n", ret);
+		dev_err(dev, "BDC Gadget init failure:%d\n", ret);
 		goto cleanup;
 	}
 	return 0;
-- 
1.9.0.138.g2de3478

[PATCH 5/8] usb: gadget: bdc: hook a quick Device Tree compatible string

[Al Cooper]

From: Florian Fainelli <f.fainelli@gmail.com>

Allows Device Tree probing

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Al Cooper <alcooperx@gmail.com>
---
 drivers/usb/gadget/udc/bdc/bdc_core.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/usb/gadget/udc/bdc/bdc_core.c b/drivers/usb/gadget/udc/bdc/bdc_core.c
index 621328f..1714bd3 100644
--- a/drivers/usb/gadget/udc/bdc/bdc_core.c
+++ b/drivers/usb/gadget/udc/bdc/bdc_core.c
@@ -533,9 +533,17 @@ static int bdc_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id bdc_of_match[] = {
+	{ .compatible = "brcm,bdc-udc-v0.16" },
+	{ .compatible = "brcm,bdc-udc" },
+	{ /* sentinel */ }
+};
+
 static struct platform_driver bdc_driver = {
 	.driver		= {
 		.name	= BRCM_BDC_NAME,
+		.owner	= THIS_MODULE,
+		.of_match_table	= bdc_of_match,
 	},
 	.probe		= bdc_probe,
 	.remove		= bdc_remove,
-- 
1.9.0.138.g2de3478

[PATCH 6/8] usb: bdc: Add support for suspend/resume

[Al Cooper]

Based on a previous commit by Danesh Petigara <dpetigara@broadcom.com>
that added resume to solve the following problem:
"The BDC driver will fail after resuming from S3 suspend and this
will cause any upper layer gadget driver to fail."
This commit also adds support for suspend and manages the clock during
suspend/resume.

Signed-off-by: Al Cooper <alcooperx@gmail.com>
---
 drivers/usb/gadget/udc/bdc/bdc.h      |  1 +
 drivers/usb/gadget/udc/bdc/bdc_core.c | 36 +++++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/drivers/usb/gadget/udc/bdc/bdc.h b/drivers/usb/gadget/udc/bdc/bdc.h
index 3664808..f838647 100644
--- a/drivers/usb/gadget/udc/bdc/bdc.h
+++ b/drivers/usb/gadget/udc/bdc/bdc.h
@@ -454,6 +454,7 @@ struct bdc {
 	 * Func Wake packet every 2.5 secs. Refer to USB3 spec section 8.5.6.4
 	 */
 	struct delayed_work	func_wake_notify;
+	struct clk		*clk;
 };
 
 static inline u32 bdc_readl(void __iomem *base, u32 offset)
diff --git a/drivers/usb/gadget/udc/bdc/bdc_core.c b/drivers/usb/gadget/udc/bdc/bdc_core.c
index 1714bd3..021c0e7 100644
--- a/drivers/usb/gadget/udc/bdc/bdc_core.c
+++ b/drivers/usb/gadget/udc/bdc/bdc_core.c
@@ -473,6 +473,8 @@ static int bdc_probe(struct platform_device *pdev)
 	if (!bdc)
 		return -ENOMEM;
 
+	bdc->clk = clk;
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	bdc->regs = devm_ioremap_resource(dev, res);
 	if (IS_ERR(bdc->regs)) {
@@ -529,10 +531,43 @@ static int bdc_remove(struct platform_device *pdev)
 	dev_dbg(bdc->dev, "%s ()\n", __func__);
 	bdc_udc_exit(bdc);
 	bdc_hw_exit(bdc);
+	clk_disable_unprepare(bdc->clk);
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int bdc_suspend(struct device *dev)
+{
+	struct bdc *bdc = dev_get_drvdata(dev);
+
+	clk_disable_unprepare(bdc->clk);
+	return 0;
+}
+
+static int bdc_resume(struct device *dev)
+{
+	struct bdc *bdc = dev_get_drvdata(dev);
+	int ret;
+
+	ret = clk_prepare_enable(bdc->clk);
+	if (ret) {
+		dev_err(bdc->dev, "err enabling the clock\n");
+		return ret;
+	}
+	ret = bdc_reinit(bdc);
+	if (ret) {
+		dev_err(bdc->dev, "err in bdc reinit\n");
+		return ret;
+	}
 
 	return 0;
 }
 
+#endif /* CONFIG_PM_SLEEP */
+
+static SIMPLE_DEV_PM_OPS(bdc_pm_ops, bdc_suspend,
+		bdc_resume);
+
 static const struct of_device_id bdc_of_match[] = {
 	{ .compatible = "brcm,bdc-udc-v0.16" },
 	{ .compatible = "brcm,bdc-udc" },
@@ -543,6 +578,7 @@ static int bdc_remove(struct platform_device *pdev)
 	.driver		= {
 		.name	= BRCM_BDC_NAME,
 		.owner	= THIS_MODULE,
+		.pm = &bdc_pm_ops,
 		.of_match_table	= bdc_of_match,
 	},
 	.probe		= bdc_probe,
-- 
1.9.0.138.g2de3478

[PATCH 7/8] usb: bdc: fix "xsf for ep not enabled" errror

[Al Cooper]

This patch essentially clears the port status change bits at the
correct times. It is necessary because the driver was not handling
the change bits correctly for events during device
connection/disconnection and bus enumeration. So, one of them (PCC)
was left stuck sometimes causing the "xsf for ep not enabled"
error we get on first connection. This was found by the Android team.
This was debugged and fixed by Sasi Kumar.

Signed-off-by: Al Cooper <alcooperx@gmail.com>
---
 drivers/usb/gadget/udc/bdc/bdc_udc.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/udc/bdc/bdc_udc.c b/drivers/usb/gadget/udc/bdc/bdc_udc.c
index aae7458..c843461 100644
--- a/drivers/usb/gadget/udc/bdc/bdc_udc.c
+++ b/drivers/usb/gadget/udc/bdc/bdc_udc.c
@@ -249,6 +249,7 @@ void bdc_sr_uspc(struct bdc *bdc, struct bdc_sr *sreport)
 			disconn = true;
 		else if ((uspc & BDC_PCS) && !BDC_PST(uspc))
 			connected = true;
+		clear_flags |= BDC_PCC;
 	}
 
 	/* Change in VBus and VBus is present */
@@ -259,16 +260,16 @@ void bdc_sr_uspc(struct bdc *bdc, struct bdc_sr *sreport)
 			bdc_softconn(bdc);
 			usb_gadget_set_state(&bdc->gadget, USB_STATE_POWERED);
 		}
-		clear_flags = BDC_VBC;
+		clear_flags |= BDC_VBC;
 	} else if ((uspc & BDC_PRS) || (uspc & BDC_PRC) || disconn) {
 		/* Hot reset, warm reset, 2.0 bus reset or disconn */
 		dev_dbg(bdc->dev, "Port reset or disconn\n");
 		bdc_uspc_disconnected(bdc, disconn);
-		clear_flags = BDC_PCC|BDC_PCS|BDC_PRS|BDC_PRC;
+		clear_flags |= BDC_PRC;
 	} else if ((uspc & BDC_PSC) && (uspc & BDC_PCS)) {
 		/* Change in Link state */
 		handle_link_state_change(bdc, uspc);
-		clear_flags = BDC_PSC|BDC_PCS;
+		clear_flags |= BDC_PSC;
 	}
 
 	/*
-- 
1.9.0.138.g2de3478

[PATCH 8/8] usb: bdc: Enable in Kconfig for ARCH_BRCMSTB systems

[Al Cooper]

Many ARM based Broadcom STB SoC's have a USB BDC controller so
enable this driver for these systems.

Signed-off-by: Al Cooper <alcooperx@gmail.com>
---
 drivers/usb/gadget/udc/bdc/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/gadget/udc/bdc/Kconfig b/drivers/usb/gadget/udc/bdc/Kconfig
index eb8b553..c74ac25 100644
--- a/drivers/usb/gadget/udc/bdc/Kconfig
+++ b/drivers/usb/gadget/udc/bdc/Kconfig
@@ -1,6 +1,7 @@
 config USB_BDC_UDC
 	tristate "Broadcom USB3.0 device controller IP driver(BDC)"
 	depends on USB_GADGET && HAS_DMA
+	default ARCH_BRCMSTB
 
 	help
 	BDC is Broadcom's USB3.0 device controller IP. If your SOC has a BDC IP
-- 
1.9.0.138.g2de3478

RE: [PATCH 4/8] usb: bdc: Small code cleanup

[David Laight]

From: Al Cooper
> Sent: 27 June 2017 19:23
> Signed-off-by: Al Cooper <alcooperx@gmail.com>
> ---
>  drivers/usb/gadget/udc/bdc/bdc_core.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/usb/gadget/udc/bdc/bdc_core.c b/drivers/usb/gadget/udc/bdc/bdc_core.c
> index 3bd82d2..621328f 100644
> --- a/drivers/usb/gadget/udc/bdc/bdc_core.c
> +++ b/drivers/usb/gadget/udc/bdc/bdc_core.c
> @@ -488,28 +488,29 @@ static int bdc_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, bdc);
>  	bdc->irq = irq;
>  	bdc->dev = dev;
> -	dev_dbg(bdc->dev, "bdc->regs: %p irq=%d\n", bdc->regs, bdc->irq);
> +	dev_dbg(dev, "bdc->regs: %p irq=%d\n", bdc->regs, bdc->irq);

The compiler will use the value without re-reading it.
In the other places it makes very little difference.
The changed code might require one less memory read, but if the extra
'live' local variable causes gcc to save registers to stack all
bets are off.

The more explicit bdc->dev is probably more readable.

> 
>  	temp = bdc_readl(bdc->regs, BDC_BDCSC);
>  	if ((temp & BDC_P64) &&
>  			!dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64))) {
> -		dev_dbg(bdc->dev, "Using 64-bit address\n");
> +		dev_dbg(dev, "Using 64-bit address\n");
>  	} else {
> -		ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> +		ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));

That just wrong...
Or was wrong before.

...

	David

Re: [PATCH 4/8] usb: bdc: Small code cleanup

[Al Cooper]

On Wed, Jun 28, 2017 at 4:47 AM, David Laight <David.Laight@aculab.com> wrote:
>>
>>       temp = bdc_readl(bdc->regs, BDC_BDCSC);
>>       if ((temp & BDC_P64) &&
>>                       !dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64))) {
>> -             dev_dbg(bdc->dev, "Using 64-bit address\n");
>> +             dev_dbg(dev, "Using 64-bit address\n");
>>       } else {
>> -             ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
>> +             ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
>
> That just wrong...
> Or was wrong before.

Why is this wrong?

Al

Re: [PATCH 5/8] usb: gadget: bdc: hook a quick Device Tree compatible string

[kbuild test robot]

Hi Florian,

[auto build test WARNING on linus/master]
[also build test WARNING on v4.12-rc7 next-20170628]
[cannot apply to balbi-usb/next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Al-Cooper/Bugs-fixes-and-improvements-to-Broadcom-BDC-driver/20170628-234153


coccinelle warnings: (new ones prefixed by >>)

>> drivers/usb/gadget/udc/bdc/bdc_core.c:545:3-8: No need to set .owner here. The core will do it.

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[PATCH] usb: gadget: bdc: fix platform_no_drv_owner.cocci warnings

[kbuild test robot]

drivers/usb/gadget/udc/bdc/bdc_core.c:545:3-8: No need to set .owner here. The core will do it.

 Remove .owner field if calls are used which set it automatically

Generated by: scripts/coccinelle/api/platform_no_drv_owner.cocci

Fixes: 34bf991059e5 ("usb: gadget: bdc: hook a quick Device Tree compatible string")
CC: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---

 bdc_core.c |    1 -
 1 file changed, 1 deletion(-)

--- a/drivers/usb/gadget/udc/bdc/bdc_core.c
+++ b/drivers/usb/gadget/udc/bdc/bdc_core.c
@@ -542,7 +542,6 @@ static const struct of_device_id bdc_of_
 static struct platform_driver bdc_driver = {
 	.driver		= {
 		.name	= BRCM_BDC_NAME,
-		.owner	= THIS_MODULE,
 		.of_match_table	= bdc_of_match,
 	},
 	.probe		= bdc_probe,

RE: [PATCH 4/8] usb: bdc: Small code cleanup

[David Laight]

From: Al Cooper
> Sent: 28 June 2017 15:56
> On Wed, Jun 28, 2017 at 4:47 AM, David Laight <David.Laight@aculab.com> wrote:
> >>
> >>       temp = bdc_readl(bdc->regs, BDC_BDCSC);
> >>       if ((temp & BDC_P64) &&
> >>                       !dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64))) {
> >> -             dev_dbg(bdc->dev, "Using 64-bit address\n");
> >> +             dev_dbg(dev, "Using 64-bit address\n");
> >>       } else {
> >> -             ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> >> +             ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> >
> > That just wrong...
> > Or was wrong before.
> 
> Why is this wrong?

It isn't obvious that &pdev->dev is bdc->dev and hence dev.

	David

Re: [PATCH 2/8] usb: bdc: Add Device Tree binding document for Broadcom BDC driver

[Rob Herring]

On Tue, Jun 27, 2017 at 02:23:20PM -0400, Al Cooper wrote:
> Add Device Tree binding document for Broadcom USB Device
> Controller (BDC).
> 
> Signed-off-by: Al Cooper <alcooperx@gmail.com>
> ---
>  .../devicetree/bindings/usb/brcm,bdc-udc.txt       | 28 ++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/brcm,bdc-udc.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/brcm,bdc-udc.txt b/Documentation/devicetree/bindings/usb/brcm,bdc-udc.txt
> new file mode 100644
> index 0000000..4eeaddb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/brcm,bdc-udc.txt
> @@ -0,0 +1,28 @@
> +Broadcom USB Device Controller (BDC)
> +====================================
> +
> +Required properties:
> +
> +- compatible: must be one of:
> +                "brcm,bdc-udc-v0.16"
> +                "brcm,bdc-udc"

Isn't "bdc-udc" redundant?

Where does the version number come from?

> +- reg: the base register address and length
> +- interrupts: ther interrupt line for this controller

s/ther/the/

> +
> +Optional properties:
> +
> +On Broadcom STB platforms, these properties are required:
> +
> +- phys: phandle to the USB PHY blocks

How many?

> +- clocks: phandle to the functional clock of this block
> +
> +Example:
> +
> +        udc@f0b02000 {
> +                status = "disabled";

Don't show status in examples.

> +                compatible = "brcm,bdc-udc-v0.16";
> +                reg = <0xf0b02000 0xfc4>;
> +                interrupts = <0x0 0x60 0x0>;
> +                phys = <&usbphy_0 0x0>;
> +                clocks = <&sw_usbd>;
> +        };
> -- 
> 1.9.0.138.g2de3478
> 

Re: [PATCH 2/8] usb: bdc: Add Device Tree binding document for Broadcom BDC driver

[Rob Herring]

On Tue, Jun 27, 2017 at 02:23:20PM -0400, Al Cooper wrote:
> Add Device Tree binding document for Broadcom USB Device
> Controller (BDC).

Also, "dt-bindings: usb: ..." for the subject.

Re: [PATCH 2/8] usb: bdc: Add Device Tree binding document for Broadcom BDC driver

[Rob Herring]

Please only post plain text emails.

On Fri, Jul 7, 2017 at 2:03 PM, Al Cooper <al.cooper@broadcom.com> wrote:
>
>
> On Thu, Jul 6, 2017 at 10:18 AM, Rob Herring <robh@kernel.org> wrote:
>>
>> On Tue, Jun 27, 2017 at 02:23:20PM -0400, Al Cooper wrote:
>> > Add Device Tree binding document for Broadcom USB Device
>> > Controller (BDC).
>> >
>> > Signed-off-by: Al Cooper <alcooperx@gmail.com>
>> > ---
>> >  .../devicetree/bindings/usb/brcm,bdc-udc.txt       | 28 ++++++++++++++++++++++
>> >  1 file changed, 28 insertions(+)
>> >  create mode 100644 Documentation/devicetree/bindings/usb/brcm,bdc-udc.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/usb/brcm,bdc-udc.txt b/Documentation/devicetree/bindings/usb/brcm,bdc-udc.txt
>> > new file mode 100644
>> > index 0000000..4eeaddb
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/usb/brcm,bdc-udc.txt
>> > @@ -0,0 +1,28 @@
>> > +Broadcom USB Device Controller (BDC)
>> > +====================================
>> > +
>> > +Required properties:
>> > +
>> > +- compatible: must be one of:
>> > +                "brcm,bdc-udc-v0.16"
>> > +                "brcm,bdc-udc"
>>
>> Isn't "bdc-udc" redundant?
>
>
> I'll change the base name to "brcm-udc"

Well, it should be "brcm,<something>" with the vendor prefix followed
by a comma. If the block is known as the BDC, then I'd use that.

Rob