[U-Boot] [PATCH 0/2] Add Broadcom XHCI driver for iproc platforms

[U-Boot] [PATCH 0/2] Add Broadcom XHCI driver for iproc platforms

[Vladimir Olovyannikov]

This patchset adds Broadcom XHCI driver for iproc platforms
This USB controller is based on the Broadcom USB3 IP Core.
Supports USB2/3 functionality.

Bharat Kumar Reddy Gooty (2):
  dt-bindings: Documentation on BRCM xHCI controller
  drivers: usb: host: Add BRCM xHCI driver

 .../bindings/usb/brcm,generic-xhci.txt        |  12 ++
 drivers/usb/host/Kconfig                      |   8 ++
 drivers/usb/host/Makefile                     |   1 +
 drivers/usb/host/xhci-brcm.c                  | 103 ++++++++++++++++++
 4 files changed, 124 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/brcm,generic-xhci.txt
 create mode 100644 drivers/usb/host/xhci-brcm.c

-- 
2.17.1

[U-Boot] [PATCH 1/2] dt-bindings: Documentation on BRCM xHCI controller

[Vladimir Olovyannikov]

From: Bharat Kumar Reddy Gooty <bharat.gooty@broadcom.com>

DT bindings document for Broadcom xHCI controller.

Signed-off-by: Bharat Kumar Reddy Gooty <bharat.gooty@broadcom.com>
Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
---
 .../devicetree/bindings/usb/brcm,generic-xhci.txt    | 12 ++++++++++++
 1 file changed, 12 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/brcm,generic-xhci.txt

diff --git a/Documentation/devicetree/bindings/usb/brcm,generic-xhci.txt b/Documentation/devicetree/bindings/usb/brcm,generic-xhci.txt
new file mode 100644
index 0000000000..621b99c9f3
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/brcm,generic-xhci.txt
@@ -0,0 +1,12 @@
+Broadcom USB xHCI Controller
+
+Required properties:
+  - compatible: "brcm,generic-xhci"
+  - reg: Base address and length of the standard xHCI register set
+
+Example:
+
+	xhci0: usb at 68501000 {
+		compatible = "brcm,generic-xhci";
+		reg = <0x68501000 0x1000>;
+	};
-- 
2.17.1

[U-Boot] [PATCH 2/2] drivers: usb: host: Add BRCM xHCI driver

[Vladimir Olovyannikov]

From: Bharat Kumar Reddy Gooty <bharat.gooty@broadcom.com>

Base driver for Broadcom xHCI controllers

Signed-off-by: Bharat Kumar Reddy Gooty <bharat.gooty@broadcom.com>
Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
---
 drivers/usb/host/Kconfig     |   8 +++
 drivers/usb/host/Makefile    |   1 +
 drivers/usb/host/xhci-brcm.c | 103 +++++++++++++++++++++++++++++++++++
 3 files changed, 112 insertions(+)
 create mode 100644 drivers/usb/host/xhci-brcm.c

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 0987ff25b1..94ac969058 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -88,6 +88,14 @@ config USB_XHCI_FSL
 	depends on !SPL_NO_USB
 	help
 	  Enables support for the on-chip xHCI controller on NXP Layerscape SoCs.
+
+config USB_XHCI_BRCM
+	bool "Broadcom USB3 Host XHCI controller"
+	depends on DM_USB
+	help
+	  USB controller based on the Broadcom USB3 IP Core.
+	  Supports USB2/3 functionality.
+
 endif # USB_XHCI_HCD
 
 config USB_EHCI_HCD
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index 7feeff679c..b62f346094 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -44,6 +44,7 @@ obj-$(CONFIG_USB_EHCI_RMOBILE) += ehci-rmobile.o
 obj-$(CONFIG_USB_EHCI_ZYNQ) += ehci-zynq.o
 
 # xhci
+obj-$(CONFIG_USB_XHCI_BRCM) += xhci-brcm.o
 obj-$(CONFIG_USB_XHCI_HCD) += xhci.o xhci-mem.o xhci-ring.o
 obj-$(CONFIG_USB_XHCI_DWC3) += xhci-dwc3.o
 obj-$(CONFIG_USB_XHCI_DWC3_OF_SIMPLE) += dwc3-of-simple.o
diff --git a/drivers/usb/host/xhci-brcm.c b/drivers/usb/host/xhci-brcm.c
new file mode 100644
index 0000000000..84ec95f804
--- /dev/null
+++ b/drivers/usb/host/xhci-brcm.c
@@ -0,0 +1,103 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2019 Broadcom.
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <fdtdec.h>
+#include <usb.h>
+#include <asm/io.h>
+#include "xhci.h"
+
+#define DRD2U3H_XHC_REGS_AXIWRA	0xC08
+#define DRD2U3H_XHC_REGS_AXIRDA	0xC0C
+
+#define USBAXI_AWCACHE		0xF
+#define USBAXI_ARCACHE		0xF
+#define USBAXI_AWPROT		0x8
+#define USBAXI_ARPROT		0x8
+#define USBAXIWR_SA_VAL		((USBAXI_AWCACHE << 4 | USBAXI_AWPROT) << 0)
+#define USBAXIWR_SA_MASK	(0xff)
+#define USBAXIWR_UA_VAL		((USBAXI_AWCACHE << 4 | USBAXI_AWPROT) << 16)
+#define USBAXIWR_UA_MASK	((0xff) << 16)
+#define USBAXIRD_SA_VAL		((USBAXI_ARCACHE << 4 | USBAXI_ARPROT) << 0)
+#define USBAXIRD_SA_MASK	(0xff)
+#define USBAXIRD_UA_VAL		((USBAXI_ARCACHE << 4 | USBAXI_ARPROT) << 16)
+#define USBAXIRD_UA_MASK	((0xff) << 16)
+
+struct brcm_xhci_platdata {
+	unsigned int arcache;
+	unsigned int awcache;
+};
+
+static int xhci_brcm_probe(struct udevice *dev)
+{
+	struct xhci_hccr *hcd;
+	struct xhci_hcor *hcor;
+	struct brcm_xhci_platdata *plat = dev_get_platdata(dev);
+	int len, ret = 0;
+
+	if (!plat) {
+		dev_err(dev, "Can't get xHCI Plat data\n");
+		return -ENOMEM;
+	}
+
+	hcd = dev_read_addr_ptr(dev);
+	if (!hcd) {
+		dev_err(dev, "Can't get the xHCI register base address\n");
+		return -ENXIO;
+	}
+
+	len = HC_LENGTH(xhci_readl(&(hcd)->cr_capbase));
+	hcor = (struct xhci_hcor *)((uintptr_t)hcd + len);
+
+	/* Save the default values of AXI read and write attributes */
+	plat->awcache = readl((uintptr_t)hcd + DRD2U3H_XHC_REGS_AXIWRA);
+	plat->arcache = readl((uintptr_t)hcd + DRD2U3H_XHC_REGS_AXIRDA);
+
+	/* Enable AXI read and write attributes. */
+	clrsetbits_le32(((uintptr_t)hcd + DRD2U3H_XHC_REGS_AXIWRA),
+			(USBAXIWR_UA_MASK | USBAXIWR_SA_MASK),
+			(USBAXIWR_UA_VAL | USBAXIWR_SA_VAL));
+	clrsetbits_le32(((uintptr_t)hcd + DRD2U3H_XHC_REGS_AXIRDA),
+			(USBAXIRD_UA_MASK | USBAXIRD_SA_MASK),
+			(USBAXIRD_UA_VAL | USBAXIRD_SA_VAL));
+
+	ret = xhci_register(dev, hcd, hcor);
+	if (ret)
+		dev_err(dev, "Failed to register xHCI\n");
+
+	return ret;
+}
+
+static int xhci_brcm_deregister(struct udevice *dev)
+{
+	struct xhci_hccr *hcd;
+	struct brcm_xhci_platdata *plat = dev_get_platdata(dev);
+
+	hcd = (struct xhci_hccr *)((uintptr_t)dev_read_addr(dev));
+
+	/* Restore the default values for AXI read and write attributes */
+	writel(plat->awcache, ((uintptr_t)hcd + DRD2U3H_XHC_REGS_AXIWRA));
+	writel(plat->arcache, ((uintptr_t)hcd + DRD2U3H_XHC_REGS_AXIRDA));
+
+	return xhci_deregister(dev);
+}
+
+static const struct udevice_id xhci_brcm_ids[] = {
+	{ .compatible = "brcm,generic-xhci" },
+	{ }
+};
+
+U_BOOT_DRIVER(usb_xhci) = {
+	.name				= "xhci_brcm",
+	.id				= UCLASS_USB,
+	.probe				= xhci_brcm_probe,
+	.remove				= xhci_brcm_deregister,
+	.ops				= &xhci_usb_ops,
+	.of_match			= xhci_brcm_ids,
+	.platdata_auto_alloc_size	= sizeof(struct brcm_xhci_platdata),
+	.priv_auto_alloc_size		= sizeof(struct xhci_ctrl),
+	.flags				= DM_FLAG_ALLOC_PRIV_DMA,
+};
-- 
2.17.1

[U-Boot] [PATCH 2/2] drivers: usb: host: Add BRCM xHCI driver

[Marek Vasut]

On 11/23/19 12:31 AM, Vladimir Olovyannikov wrote:
[...]

> +#define USBAXI_AWCACHE		0xF
> +#define USBAXI_ARCACHE		0xF
> +#define USBAXI_AWPROT		0x8
> +#define USBAXI_ARPROT		0x8
> +#define USBAXIWR_SA_VAL		((USBAXI_AWCACHE << 4 | USBAXI_AWPROT) << 0)

Are the parenthesis correct here ?
Might make sense to rewrite it as ((AWCACHE << 4) | AWPROT)

> +#define USBAXIWR_SA_MASK	(0xff)

Here the parenthesis are not needed.

> +#define USBAXIWR_UA_VAL		((USBAXI_AWCACHE << 4 | USBAXI_AWPROT) << 16)
> +#define USBAXIWR_UA_MASK	((0xff) << 16)
> +#define USBAXIRD_SA_VAL		((USBAXI_ARCACHE << 4 | USBAXI_ARPROT) << 0)
> +#define USBAXIRD_SA_MASK	(0xff)
> +#define USBAXIRD_UA_VAL		((USBAXI_ARCACHE << 4 | USBAXI_ARPROT) << 16)
> +#define USBAXIRD_UA_MASK	((0xff) << 16)
> +
> +struct brcm_xhci_platdata {
> +	unsigned int arcache;
> +	unsigned int awcache;
> +};
> +
> +static int xhci_brcm_probe(struct udevice *dev)
> +{
> +	struct xhci_hccr *hcd;
> +	struct xhci_hcor *hcor;
> +	struct brcm_xhci_platdata *plat = dev_get_platdata(dev);
> +	int len, ret = 0;
> +
> +	if (!plat) {
> +		dev_err(dev, "Can't get xHCI Plat data\n");
> +		return -ENOMEM;
> +	}
> +
> +	hcd = dev_read_addr_ptr(dev);
> +	if (!hcd) {
> +		dev_err(dev, "Can't get the xHCI register base address\n");
> +		return -ENXIO;
> +	}
> +
> +	len = HC_LENGTH(xhci_readl(&(hcd)->cr_capbase));
> +	hcor = (struct xhci_hcor *)((uintptr_t)hcd + len);

Please clean up the extraneous parenthesis ^

> +	/* Save the default values of AXI read and write attributes */
> +	plat->awcache = readl((uintptr_t)hcd + DRD2U3H_XHC_REGS_AXIWRA);
> +	plat->arcache = readl((uintptr_t)hcd + DRD2U3H_XHC_REGS_AXIRDA);
> +
> +	/* Enable AXI read and write attributes. */
> +	clrsetbits_le32(((uintptr_t)hcd + DRD2U3H_XHC_REGS_AXIWRA),
> +			(USBAXIWR_UA_MASK | USBAXIWR_SA_MASK),
> +			(USBAXIWR_UA_VAL | USBAXIWR_SA_VAL));
> +	clrsetbits_le32(((uintptr_t)hcd + DRD2U3H_XHC_REGS_AXIRDA),
> +			(USBAXIRD_UA_MASK | USBAXIRD_SA_MASK),
> +			(USBAXIRD_UA_VAL | USBAXIRD_SA_VAL));
> +
> +	ret = xhci_register(dev, hcd, hcor);
> +	if (ret)
> +		dev_err(dev, "Failed to register xHCI\n");
> +
> +	return ret;
> +}
> +
> +static int xhci_brcm_deregister(struct udevice *dev)
> +{
> +	struct xhci_hccr *hcd;
> +	struct brcm_xhci_platdata *plat = dev_get_platdata(dev);
> +
> +	hcd = (struct xhci_hccr *)((uintptr_t)dev_read_addr(dev));
> +
> +	/* Restore the default values for AXI read and write attributes */
> +	writel(plat->awcache, ((uintptr_t)hcd + DRD2U3H_XHC_REGS_AXIWRA));
> +	writel(plat->arcache, ((uintptr_t)hcd + DRD2U3H_XHC_REGS_AXIRDA));

Here too.

Looks good otherwise, thanks.

[U-Boot] [PATCH 2/2] drivers: usb: host: Add BRCM xHCI driver

[Vladimir Olovyannikov]

HI Marek,

> On 11/23/19 12:31 AM, Vladimir Olovyannikov wrote:
> [...]
>
> > +#define USBAXI_AWCACHE		0xF
> > +#define USBAXI_ARCACHE		0xF
> > +#define USBAXI_AWPROT		0x8
> > +#define USBAXI_ARPROT		0x8
> > +#define USBAXIWR_SA_VAL		((USBAXI_AWCACHE << 4 |
> USBAXI_AWPROT) << 0)
>
> Are the parenthesis correct here ?
> Might make sense to rewrite it as ((AWCACHE << 4) | AWPROT)
>
> > +#define USBAXIWR_SA_MASK	(0xff)
>
> Here the parenthesis are not needed.
>
> > +#define USBAXIWR_UA_VAL		((USBAXI_AWCACHE << 4 |
> USBAXI_AWPROT) << 16)
> > +#define USBAXIWR_UA_MASK	((0xff) << 16)
> > +#define USBAXIRD_SA_VAL		((USBAXI_ARCACHE << 4 |
> USBAXI_ARPROT) << 0)
> > +#define USBAXIRD_SA_MASK	(0xff)
> > +#define USBAXIRD_UA_VAL		((USBAXI_ARCACHE << 4 |
> USBAXI_ARPROT) << 16)
> > +#define USBAXIRD_UA_MASK	((0xff) << 16)
> > +
> > +struct brcm_xhci_platdata {
> > +	unsigned int arcache;
> > +	unsigned int awcache;
> > +};
> > +
> > +static int xhci_brcm_probe(struct udevice *dev) {
> > +	struct xhci_hccr *hcd;
> > +	struct xhci_hcor *hcor;
> > +	struct brcm_xhci_platdata *plat = dev_get_platdata(dev);
> > +	int len, ret = 0;
> > +
> > +	if (!plat) {
> > +		dev_err(dev, "Can't get xHCI Plat data\n");
> > +		return -ENOMEM;
> > +	}
> > +
> > +	hcd = dev_read_addr_ptr(dev);
> > +	if (!hcd) {
> > +		dev_err(dev, "Can't get the xHCI register base address\n");
> > +		return -ENXIO;
> > +	}
> > +
> > +	len = HC_LENGTH(xhci_readl(&(hcd)->cr_capbase));
> > +	hcor = (struct xhci_hcor *)((uintptr_t)hcd + len);
>
> Please clean up the extraneous parenthesis ^
>
> > +	/* Save the default values of AXI read and write attributes */
> > +	plat->awcache = readl((uintptr_t)hcd +
> DRD2U3H_XHC_REGS_AXIWRA);
> > +	plat->arcache = readl((uintptr_t)hcd +
> DRD2U3H_XHC_REGS_AXIRDA);
> > +
> > +	/* Enable AXI read and write attributes. */
> > +	clrsetbits_le32(((uintptr_t)hcd + DRD2U3H_XHC_REGS_AXIWRA),
> > +			(USBAXIWR_UA_MASK | USBAXIWR_SA_MASK),
> > +			(USBAXIWR_UA_VAL | USBAXIWR_SA_VAL));
> > +	clrsetbits_le32(((uintptr_t)hcd + DRD2U3H_XHC_REGS_AXIRDA),
> > +			(USBAXIRD_UA_MASK | USBAXIRD_SA_MASK),
> > +			(USBAXIRD_UA_VAL | USBAXIRD_SA_VAL));
> > +
> > +	ret = xhci_register(dev, hcd, hcor);
> > +	if (ret)
> > +		dev_err(dev, "Failed to register xHCI\n");
> > +
> > +	return ret;
> > +}
> > +
> > +static int xhci_brcm_deregister(struct udevice *dev) {
> > +	struct xhci_hccr *hcd;
> > +	struct brcm_xhci_platdata *plat = dev_get_platdata(dev);
> > +
> > +	hcd = (struct xhci_hccr *)((uintptr_t)dev_read_addr(dev));
> > +
> > +	/* Restore the default values for AXI read and write attributes */
> > +	writel(plat->awcache, ((uintptr_t)hcd +
> DRD2U3H_XHC_REGS_AXIWRA));
> > +	writel(plat->arcache, ((uintptr_t)hcd +
> DRD2U3H_XHC_REGS_AXIRDA));
>
> Here too.
>
> Looks good otherwise, thanks.
Thanks for review Marek,
Issues will be resolved with the next patch set.