2017-06-27 18:24:27

by Al Cooper

[permalink] [raw]
Subject: [PATCH 0/8] Bugs fixes and improvements to Broadcom BDC driver

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


2017-06-27 18:24:45

by Al Cooper

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

Signed-off-by: Al Cooper <[email protected]>
---
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

2017-06-27 18:24:44

by Al Cooper

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

Based on a previous commit by Danesh Petigara <[email protected]>
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 <[email protected]>
---
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

2017-06-27 18:24:40

by Al Cooper

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

From: Florian Fainelli <[email protected]>

Allows Device Tree probing

Signed-off-by: Florian Fainelli <[email protected]>
Signed-off-by: Al Cooper <[email protected]>
---
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

2017-06-27 18:25:11

by Al Cooper

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

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

Signed-off-by: Al Cooper <[email protected]>
---
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

2017-06-27 18:25:44

by Al Cooper

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

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 <[email protected]>
---
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

2017-06-27 18:24:33

by Al Cooper

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

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

Signed-off-by: Al Cooper <[email protected]>
Signed-off-by: Florian Fainelli <[email protected]>
---
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

2017-06-27 18:26:38

by Al Cooper

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

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

Signed-off-by: Al Cooper <[email protected]>
---
.../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

2017-06-27 18:26:50

by Al Cooper

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

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 <[email protected]>
---
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

2017-06-28 08:48:12

by David Laight

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

From: Al Cooper
> Sent: 27 June 2017 19:23
> Signed-off-by: Al Cooper <[email protected]>
> ---
> 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

2017-06-28 14:56:16

by Al Cooper

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

On Wed, Jun 28, 2017 at 4:47 AM, David Laight <[email protected]> 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

2017-06-28 23:45:41

by kernel test robot

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

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

2017-06-28 23:45:50

by kernel test robot

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

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 <[email protected]>
Signed-off-by: Fengguang Wu <[email protected]>
---

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,

2017-06-29 14:28:40

by David Laight

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

From: Al Cooper
> Sent: 28 June 2017 15:56
> On Wed, Jun 28, 2017 at 4:47 AM, David Laight <[email protected]> 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


2017-07-06 14:18:46

by Rob Herring (Arm)

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

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 <[email protected]>
> ---
> .../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
>

2017-07-06 14:19:29

by Rob Herring (Arm)

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

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.

2017-07-07 20:15:53

by Rob Herring (Arm)

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

Please only post plain text emails.

On Fri, Jul 7, 2017 at 2:03 PM, Al Cooper <[email protected]> wrote:
>
>
> On Thu, Jul 6, 2017 at 10:18 AM, Rob Herring <[email protected]> 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 <[email protected]>
>> > ---
>> > .../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