[PATCH v4 0/9] USB host support for Raspberry Pi 4 board (64-bit)

[PATCH v4 0/9] USB host support for Raspberry Pi 4 board (64-bit)

[Sylwester Nawrocki]

Hi all,

This patch series adds USB host support for Raspberry Pi 4 board. 
It includes the Broadcom STB PCIe controller driver ported from Linux 
kernel, a memory mapping update for the xHCI controller on PCIe bus
for 64-bit builds and some related fixes and updates in the usb/xhci 
and the pci driver core code.

The 32-bit ARM part has be excluded from the series and will be posted
separately.

The difference comparing to previous version is only in the config update
patch "configs: Enable support for the XHCI controller on RPI4 board (ARM
 64-bit)".

The patch series is based on v2020.07-rc1 tree.

Thanks,
Sylwester

Marek Szyprowski (3):
  rpi4: shorten a mapping for the DRAM
  rpi4: add a mapping for the PCIe XHCI controller MMIO registers (ARM
    64bit)
  configs: Enable support for the XHCI controller on RPI4 board (ARM
    64-bit)

Nicolas Saenz Julienne (1):
  linux/bitfield.h: Add primitives for manipulating bitfields both in
    host- and fixed-endian

Sylwester Nawrocki (5):
  usb: xhci: Add missing cache flush in the scratchpad array
    initialization
  usb: xhci: Use only 32-bit accesses in xhci_writeq/xhci_readq
  pci: Move some PCIe register offset definitions to a common header
  pci: Add some PCI Express capability register offset definitions
  pci: Add driver for Broadcom BCM2711 SoC PCIe controller

 arch/arm/mach-bcm283x/init.c  |  20 +-
 configs/rpi_4_defconfig       |   9 +
 configs/rpi_arm64_defconfig   |   8 +-
 drivers/pci/Kconfig           |   9 +
 drivers/pci/Makefile          |   1 +
 drivers/pci/pci-rcar-gen3.c   |   8 -
 drivers/pci/pcie_brcmstb.c    | 623 ++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/pcie_intel_fpga.c |   3 -
 drivers/usb/host/xhci-mem.c   |   3 +
 include/linux/bitfield.h      |  52 ++++
 include/pci.h                 |  22 +-
 include/usb/xhci.h            |   8 -
 12 files changed, 740 insertions(+), 26 deletions(-)
 create mode 100644 drivers/pci/pcie_brcmstb.c

-- 
2.7.4

[PATCH v4 1/9] usb: xhci: Add missing cache flush in the scratchpad array initialization

[Sylwester Nawrocki]

In current code there is no cache flush after initializing the scratchpad
buffer array with the scratchpad buffer pointers. This leads to a failure
of the "slot enable" command on the rpi4 board (Broadcom STB PCIe
controller + VL805 USB hub) - the very first TRB transfer on the command
ring fails and there is a timeout while waiting for the command completion
event. After adding the missing cache flush everything seems to be working
as expected.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
Changes since v1:
 - none.
---
 drivers/usb/host/xhci-mem.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 93450ee..729bdc3 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -393,6 +393,9 @@ static int xhci_scratchpad_alloc(struct xhci_ctrl *ctrl)
 		scratchpad->sp_array[i] = cpu_to_le64(ptr);
 	}
 
+	xhci_flush_cache((uintptr_t)scratchpad->sp_array,
+			 sizeof(u64) * num_sp);
+
 	return 0;
 
 fail_sp3:
-- 
2.7.4

[PATCH v4 2/9] usb: xhci: Use only 32-bit accesses in xhci_writeq/xhci_readq

[Sylwester Nawrocki]

There might be hardware configurations where 64-bit data accesses
to XHCI registers are not supported properly.  This patch removes
the readq/writeq so always two 32-bit accesses are used to read/write
64-bit XHCI registers, similarly as it is done in Linux kernel.

This patch fixes operation of the XHCI controller on RPI4 Broadcom
BCM2711 SoC based board, where the VL805 USB XHCI controller is
connected to the PCIe Root Complex, which is attached to the system
through the SCB bridge.

Even though the architecture is 64-bit the PCIe BAR is 32-bit and likely
the 64-bit wide register accesses initiated by the CPU are not properly
translated to a sequence of 32-bit PCIe accesses.
xhci_readq(), for example, always returns same value in upper and lower
32-bits, e.g. 0xabcd1234abcd1234 instead of 0x00000000abcd1234.

Cc: Sergey Temerkhanov <s.temerkhanov@gmail.com>
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
Changes since v1:
 - none.
Changes since RFC:
 - dropped Kconfig option, switched to not using readq/writeq
   unconditionally.
---
 include/usb/xhci.h | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/include/usb/xhci.h b/include/usb/xhci.h
index 6017504..c16106a 100644
--- a/include/usb/xhci.h
+++ b/include/usb/xhci.h
@@ -1111,28 +1111,20 @@ static inline void xhci_writel(uint32_t volatile *regs, const unsigned int val)
  */
 static inline u64 xhci_readq(__le64 volatile *regs)
 {
-#if BITS_PER_LONG == 64
-	return readq(regs);
-#else
 	__u32 *ptr = (__u32 *)regs;
 	u64 val_lo = readl(ptr);
 	u64 val_hi = readl(ptr + 1);
 	return val_lo + (val_hi << 32);
-#endif
 }
 
 static inline void xhci_writeq(__le64 volatile *regs, const u64 val)
 {
-#if BITS_PER_LONG == 64
-	writeq(val, regs);
-#else
 	__u32 *ptr = (__u32 *)regs;
 	u32 val_lo = lower_32_bits(val);
 	/* FIXME */
 	u32 val_hi = upper_32_bits(val);
 	writel(val_lo, ptr);
 	writel(val_hi, ptr + 1);
-#endif
 }
 
 int xhci_hcd_init(int index, struct xhci_hccr **ret_hccr,
-- 
2.7.4

[PATCH v4 3/9] pci: Move some PCIe register offset definitions to a common header

[Sylwester Nawrocki]

Some PCI Express register offsets are currently defined in multiple
drivers, move them to a common header to avoid re-definitions and
as a pre-requisite for adding new PCIe driver.
While at it replace some spaces with tabs.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
Changes since v1:
 - none.
Changes since RFC:
 - whitespace clean up.
---
 drivers/pci/pci-rcar-gen3.c   |  8 --------
 drivers/pci/pcie_intel_fpga.c |  3 ---
 include/pci.h                 | 13 +++++++++++--
 3 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/pci-rcar-gen3.c b/drivers/pci/pci-rcar-gen3.c
index 30eff67..393f1c9 100644
--- a/drivers/pci/pci-rcar-gen3.c
+++ b/drivers/pci/pci-rcar-gen3.c
@@ -117,14 +117,6 @@
 #define RCAR_PCI_MAX_RESOURCES	4
 #define MAX_NR_INBOUND_MAPS	6
 
-#define PCI_EXP_FLAGS		2		/* Capabilities register */
-#define PCI_EXP_FLAGS_TYPE	0x00f0		/* Device/Port type */
-#define PCI_EXP_TYPE_ROOT_PORT	0x4		/* Root Port */
-#define PCI_EXP_LNKCAP		12		/* Link Capabilities */
-#define PCI_EXP_LNKCAP_DLLLARC	0x00100000	/* Data Link Layer Link Active Reporting Capable */
-#define PCI_EXP_SLTCAP		20		/* Slot Capabilities */
-#define PCI_EXP_SLTCAP_PSN	0xfff80000	/* Physical Slot Number */
-
 enum {
 	RCAR_PCI_ACCESS_READ,
 	RCAR_PCI_ACCESS_WRITE,
diff --git a/drivers/pci/pcie_intel_fpga.c b/drivers/pci/pcie_intel_fpga.c
index 6a9f29c..69363a0 100644
--- a/drivers/pci/pcie_intel_fpga.c
+++ b/drivers/pci/pcie_intel_fpga.c
@@ -65,9 +65,6 @@
 #define IS_ROOT_PORT(pcie, bdf)				\
 		((PCI_BUS(bdf) == pcie->first_busno) ? true : false)
 
-#define PCI_EXP_LNKSTA		18	/* Link Status */
-#define PCI_EXP_LNKSTA_DLLLA	0x2000	/* Data Link Layer Link Active */
-
 /**
  * struct intel_fpga_pcie - Intel FPGA PCIe controller state
  * @bus: Pointer to the PCI bus
diff --git a/include/pci.h b/include/pci.h
index aff56b2..dfdbb32 100644
--- a/include/pci.h
+++ b/include/pci.h
@@ -471,10 +471,19 @@
 #define  PCI_EA_FIELD_MASK	0xfffffffc	/* For Base & Max Offset */
 
 /* PCI Express capabilities */
+#define PCI_EXP_FLAGS		2	/* Capabilities register */
+#define  PCI_EXP_FLAGS_TYPE	0x00f0	/* Device/Port type */
+#define  PCI_EXP_TYPE_ROOT_PORT 0x4	/* Root Port */
 #define PCI_EXP_DEVCAP		4	/* Device capabilities */
-#define  PCI_EXP_DEVCAP_FLR     0x10000000 /* Function Level Reset */
+#define  PCI_EXP_DEVCAP_FLR	0x10000000 /* Function Level Reset */
 #define PCI_EXP_DEVCTL		8	/* Device Control */
-#define  PCI_EXP_DEVCTL_BCR_FLR 0x8000  /* Bridge Configuration Retry / FLR */
+#define  PCI_EXP_DEVCTL_BCR_FLR	0x8000  /* Bridge Configuration Retry / FLR */
+#define PCI_EXP_LNKCAP		12	/* Link Capabilities */
+#define  PCI_EXP_LNKCAP_DLLLARC	0x00100000 /* Data Link Layer Link Active Reporting Capable */
+#define PCI_EXP_LNKSTA		18	/* Link Status */
+#define  PCI_EXP_LNKSTA_DLLLA	0x2000	/* Data Link Layer Link Active */
+#define PCI_EXP_SLTCAP		20	/* Slot Capabilities */
+#define  PCI_EXP_SLTCAP_PSN	0xfff80000 /* Physical Slot Number */
 
 /* Include the ID list */
 
-- 
2.7.4

[PATCH v4 4/9] rpi4: shorten a mapping for the DRAM

[Sylwester Nawrocki]

From: Marek Szyprowski <m.szyprowski@samsung.com>

Remove the overlap between DRAM and device's IO area.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Reviewed-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
Changes since v1:
 - none.
---
 arch/arm/mach-bcm283x/init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-bcm283x/init.c b/arch/arm/mach-bcm283x/init.c
index 9966d6c..4295356 100644
--- a/arch/arm/mach-bcm283x/init.c
+++ b/arch/arm/mach-bcm283x/init.c
@@ -38,7 +38,7 @@ static struct mm_region bcm2711_mem_map[] = {
 	{
 		.virt = 0x00000000UL,
 		.phys = 0x00000000UL,
-		.size = 0xfe000000UL,
+		.size = 0xfc000000UL,
 		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
 			 PTE_BLOCK_INNER_SHARE
 	}, {
-- 
2.7.4

[PATCH v4 5/9] rpi4: add a mapping for the PCIe XHCI controller MMIO registers (ARM 64bit)

[Sylwester Nawrocki]

From: Marek Szyprowski <m.szyprowski@samsung.com>

Create a non-cacheable mapping for the 0x600000000 physical memory region,
where MMIO registers for the PCIe XHCI controller are instantiated by the
PCIe bridge.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Reviewed-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
Changes since v3:
 - none.
Changes since v2:
 -  fixed typo MAX_MAP_MAX_ENTRIES -> MEM_MAP_MAX_ENTRIES
Changes since v1:
 - none.
---
 arch/arm/mach-bcm283x/init.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-bcm283x/init.c b/arch/arm/mach-bcm283x/init.c
index 4295356..9f5bca3 100644
--- a/arch/arm/mach-bcm283x/init.c
+++ b/arch/arm/mach-bcm283x/init.c
@@ -11,10 +11,15 @@
 #include <dm/device.h>
 #include <fdt_support.h>
 
+#define BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS	0x600000000UL
+#define BCM2711_RPI4_PCIE_XHCI_MMIO_SIZE	0x800000UL
+
 #ifdef CONFIG_ARM64
 #include <asm/armv8/mmu.h>
 
-static struct mm_region bcm283x_mem_map[] = {
+#define MEM_MAP_MAX_ENTRIES (4)
+
+static struct mm_region bcm283x_mem_map[MEM_MAP_MAX_ENTRIES] = {
 	{
 		.virt = 0x00000000UL,
 		.phys = 0x00000000UL,
@@ -34,7 +39,7 @@ static struct mm_region bcm283x_mem_map[] = {
 	}
 };
 
-static struct mm_region bcm2711_mem_map[] = {
+static struct mm_region bcm2711_mem_map[MEM_MAP_MAX_ENTRIES] = {
 	{
 		.virt = 0x00000000UL,
 		.phys = 0x00000000UL,
@@ -49,6 +54,13 @@ static struct mm_region bcm2711_mem_map[] = {
 			 PTE_BLOCK_NON_SHARE |
 			 PTE_BLOCK_PXN | PTE_BLOCK_UXN
 	}, {
+		.virt = BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS,
+		.phys = BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS,
+		.size = BCM2711_RPI4_PCIE_XHCI_MMIO_SIZE,
+		.attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+			 PTE_BLOCK_NON_SHARE |
+			 PTE_BLOCK_PXN | PTE_BLOCK_UXN
+	}, {
 		/* List terminator */
 		0,
 	}
@@ -71,7 +83,7 @@ static void _rpi_update_mem_map(struct mm_region *pd)
 {
 	int i;
 
-	for (i = 0; i < 2; i++) {
+	for (i = 0; i < MEM_MAP_MAX_ENTRIES; i++) {
 		mem_map[i].virt = pd[i].virt;
 		mem_map[i].phys = pd[i].phys;
 		mem_map[i].size = pd[i].size;
-- 
2.7.4

[PATCH v4 6/9] linux/bitfield.h: Add primitives for manipulating bitfields both in host- and fixed-endian

[Sylwester Nawrocki]

From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>

Imports Al Viro's original Linux commit 00b0c9b82663a, which contains
an in depth explanation and two fixes from Johannes Berg:
 e7d4a95da86e0 "bitfield: fix *_encode_bits()",
 37a3862e12382 "bitfield: add u8 helpers".

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
[s.nawrocki: added empty lines between functions and macros]
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
Changes since v3:
 - none.
Changes since v2:
 - added some more missing whitespaces as pointed out by checkpatch.pl.
Changes since v1:
 - added empty lines between functions and macros.
Changes since RFC:
 - new patch.
---
 include/linux/bitfield.h | 52 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
index 8b9d6ff..7ad8b08 100644
--- a/include/linux/bitfield.h
+++ b/include/linux/bitfield.h
@@ -103,4 +103,56 @@
 		(typeof(_mask))(((_reg) & (_mask)) >> __bf_shf(_mask));	\
 	})
 
+extern void __compiletime_error("value doesn't fit into mask")
+__field_overflow(void);
+extern void __compiletime_error("bad bitfield mask")
+__bad_mask(void);
+
+static __always_inline u64 field_multiplier(u64 field)
+{
+	if ((field | (field - 1)) & ((field | (field - 1)) + 1))
+		__bad_mask();
+	return field & -field;
+}
+
+static __always_inline u64 field_mask(u64 field)
+{
+	return field / field_multiplier(field);
+}
+
+#define ____MAKE_OP(type, base, to, from)				\
+static __always_inline __##type type##_encode_bits(base v, base field)	\
+{									\
+	if (__builtin_constant_p(v) && (v & ~field_mask(field)))	\
+		__field_overflow();					\
+	return to((v & field_mask(field)) * field_multiplier(field));	\
+}									\
+static __always_inline __##type type##_replace_bits(__##type old,	\
+					base val, base field)		\
+{									\
+	return (old & ~to(field)) | type##_encode_bits(val, field);	\
+}									\
+static __always_inline void type##p_replace_bits(__##type * p,		\
+					base val, base field)		\
+{									\
+	*p = (*p & ~to(field)) | type##_encode_bits(val, field);	\
+}									\
+static __always_inline base type##_get_bits(__##type v, base field)	\
+{									\
+	return (from(v) & field) / field_multiplier(field);		\
+}
+
+#define __MAKE_OP(size)							\
+	____MAKE_OP(le##size, u##size, cpu_to_le##size, le##size##_to_cpu) \
+	____MAKE_OP(be##size, u##size, cpu_to_be##size, be##size##_to_cpu) \
+	____MAKE_OP(u##size, u##size, ,)
+
+____MAKE_OP(u8, u8, ,)
+__MAKE_OP(16)
+__MAKE_OP(32)
+__MAKE_OP(64)
+
+#undef __MAKE_OP
+#undef ____MAKE_OP
+
 #endif
-- 
2.7.4

[PATCH v4 7/9] pci: Add some PCI Express capability register offset definitions

[Sylwester Nawrocki]

Add PCI Express capability definitions required by the Broadcom
STB PCIe controller driver.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
Changes since v3:
 - none.
Changes since v2:
 - added Current Link Speed defines.
Changes since v1:
 - none.
Changes since RFC:
 - ensure the entries are added in order, sorted by ascending
   address values.
---
 include/pci.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/pci.h b/include/pci.h
index dfdbb32..ff5f620 100644
--- a/include/pci.h
+++ b/include/pci.h
@@ -479,11 +479,20 @@
 #define PCI_EXP_DEVCTL		8	/* Device Control */
 #define  PCI_EXP_DEVCTL_BCR_FLR	0x8000  /* Bridge Configuration Retry / FLR */
 #define PCI_EXP_LNKCAP		12	/* Link Capabilities */
+#define  PCI_EXP_LNKCAP_SLS	0x0000000f /* Supported Link Speeds */
+#define  PCI_EXP_LNKCAP_MLW	0x000003f0 /* Maximum Link Width */
 #define  PCI_EXP_LNKCAP_DLLLARC	0x00100000 /* Data Link Layer Link Active Reporting Capable */
 #define PCI_EXP_LNKSTA		18	/* Link Status */
+#define  PCI_EXP_LNKSTA_CLS	0x000f	/* Current Link Speed */
+#define  PCI_EXP_LNKSTA_CLS_2_5GB 0x0001 /* Current Link Speed 2.5GT/s */
+#define  PCI_EXP_LNKSTA_CLS_5_0GB 0x0002 /* Current Link Speed 5.0GT/s */
+#define  PCI_EXP_LNKSTA_CLS_8_0GB 0x0003 /* Current Link Speed 8.0GT/s */
+#define  PCI_EXP_LNKSTA_NLW	0x03f0	/* Negotiated Link Width */
+#define  PCI_EXP_LNKSTA_NLW_SHIFT 4	/* start of NLW mask in link status */
 #define  PCI_EXP_LNKSTA_DLLLA	0x2000	/* Data Link Layer Link Active */
 #define PCI_EXP_SLTCAP		20	/* Slot Capabilities */
 #define  PCI_EXP_SLTCAP_PSN	0xfff80000 /* Physical Slot Number */
+#define PCI_EXP_LNKCTL2		48	/* Link Control 2 */
 
 /* Include the ID list */
 
-- 
2.7.4

[PATCH v4 8/9] pci: Add driver for Broadcom BCM2711 SoC PCIe controller

[Sylwester Nawrocki]

This patch adds basic driver PCI Express controller found on Broadcom
set-top-box SoCs, e.g. BCM2711.
The code is based on Linux upstream driver (pcie-brcmstb.c) with MSI
handling removed. The inbound access memory region is not currently
parsed from dma-ranges DT property and a fixed 3GB region is used.

The patch has been tested on RPI4 board, i.e. on BCM2711 SoC with VL805
USB Host Controller.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
Changes since v3:
 - none.
Changes since v2 (mostly addressing Simon's review comments):
 - removed MDO_RD_DONE, MDIO_WT_DONE macro definitions,
 - updated the Kconfig entry help text,
 - reordered #include entries to match the coding style,
 - s/udev/dev,
 - s/ENODEV/EINVAL in brcm_pcie_probe() and brcm_pcie_config_address()
   functions,
 - Simplified brcm_pcie_mdio_{read, write} functions (readl_poll_timeout),
 - shortened register bit fields macro definitions,
 - dropped brcm_pcie_perst_set() and brcm_pcie_bridge_sw_init_set()
   function in favour of direct clrbits_le32/setbits_le32 calls,
 - use setbits_le32/clrbits_le32/clrsetbits_le32 instead of
   readl(), u32p_replace_bits(), writel() sequence
 - simplified brcm_pcie_config_address(), brcm_pcie_set_gen() functions,
 - changed reset pulse delay to 100 us,
 - Replaced FIELD_GET() with open coded bitwise operations,
 - brcm_cpie_cfg_index() function merged into brcm_pcie_config_address(),
 - use standard PCI PCI_EXP_LNKSTA_CLS_* link speed defines
 - added kernel-doc function comments.

Changes since v1:
 - fixed argument in brcm_pcie_set_ssc() function call,
 - changed rc_bar2_size assignment to value 0xC0000000, as in upstream
   devicetree.
Changes since RFC:
 - reworked to align with current Linux mainline version and u-boot
   driver by Nicolas Saenz Julienne.
---
 drivers/pci/Kconfig        |   9 +
 drivers/pci/Makefile       |   1 +
 drivers/pci/pcie_brcmstb.c | 623 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 633 insertions(+)
 create mode 100644 drivers/pci/pcie_brcmstb.c

diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index 437cd9a..543bd46 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -197,4 +197,13 @@ config PCIE_MEDIATEK
 	  Say Y here if you want to enable Gen2 PCIe controller,
 	  which could be found on MT7623 SoC family.
 
+config PCI_BRCMSTB
+	bool "Broadcom STB PCIe controller"
+	depends on DM_PCI
+	depends on ARCH_BCM283X
+	help
+	  Say Y here if you want to enable support for PCIe controller
+	  on Broadcom set-top-box (STB) SoCs.
+	  This driver currently supports only BCM2711 SoC and RC mode
+	  of the controller.
 endif
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index c051ecc..3e53b1f 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -43,3 +43,4 @@ obj-$(CONFIG_PCI_PHYTIUM) += pcie_phytium.o
 obj-$(CONFIG_PCIE_INTEL_FPGA) += pcie_intel_fpga.o
 obj-$(CONFIG_PCI_KEYSTONE) += pcie_dw_ti.o
 obj-$(CONFIG_PCIE_MEDIATEK) += pcie_mediatek.o
+obj-$(CONFIG_PCI_BRCMSTB) += pcie_brcmstb.o
diff --git a/drivers/pci/pcie_brcmstb.c b/drivers/pci/pcie_brcmstb.c
new file mode 100644
index 0000000..dade79e
--- /dev/null
+++ b/drivers/pci/pcie_brcmstb.c
@@ -0,0 +1,623 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Broadcom STB PCIe controller driver
+ *
+ * Copyright (C) 2020 Samsung Electronics Co., Ltd.
+ *
+ * Based on upstream Linux kernel driver:
+ * drivers/pci/controller/pcie-brcmstb.c
+ * Copyright (C) 2009 - 2017 Broadcom
+ *
+ * Based driver by Nicolas Saenz Julienne
+ * Copyright (C) 2020 Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
+ */
+
+#include <common.h>
+#include <errno.h>
+#include <dm.h>
+#include <dm/ofnode.h>
+#include <pci.h>
+#include <asm/io.h>
+#include <linux/bitfield.h>
+#include <linux/log2.h>
+#include <linux/iopoll.h>
+
+/* Offset of the mandatory PCIe capability config registers */
+#define BRCM_PCIE_CAP_REGS				0x00ac
+
+/* The PCIe controller register offsets */
+#define PCIE_RC_CFG_VENDOR_SPECIFIC_REG1		0x0188
+#define  VENDOR_SPECIFIC_REG1_ENDIAN_MODE_BAR2_MASK	0xc
+#define  VENDOR_SPECIFIC_REG1_LITTLE_ENDIAN		0x0
+
+#define PCIE_RC_CFG_PRIV1_ID_VAL3			0x043c
+#define  CFG_PRIV1_ID_VAL3_CLASS_CODE_MASK		0xffffff
+
+#define PCIE_RC_DL_MDIO_ADDR				0x1100
+#define PCIE_RC_DL_MDIO_WR_DATA				0x1104
+#define PCIE_RC_DL_MDIO_RD_DATA				0x1108
+
+#define PCIE_MISC_MISC_CTRL				0x4008
+#define  MISC_CTRL_SCB_ACCESS_EN_MASK			0x1000
+#define  MISC_CTRL_CFG_READ_UR_MODE_MASK		0x2000
+#define  MISC_CTRL_MAX_BURST_SIZE_MASK			0x300000
+#define  MISC_CTRL_MAX_BURST_SIZE_128			0x0
+#define  MISC_CTRL_SCB0_SIZE_MASK			0xf8000000
+
+#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LO		0x400c
+#define PCIE_MEM_WIN0_LO(win)	\
+		PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LO + ((win) * 4)
+
+#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_HI		0x4010
+#define PCIE_MEM_WIN0_HI(win)	\
+		PCIE_MISC_CPU_2_PCIE_MEM_WIN0_HI + ((win) * 4)
+
+#define PCIE_MISC_RC_BAR1_CONFIG_LO			0x402c
+#define  RC_BAR1_CONFIG_LO_SIZE_MASK			0x1f
+
+#define PCIE_MISC_RC_BAR2_CONFIG_LO			0x4034
+#define  RC_BAR2_CONFIG_LO_SIZE_MASK			0x1f
+#define PCIE_MISC_RC_BAR2_CONFIG_HI			0x4038
+
+#define PCIE_MISC_RC_BAR3_CONFIG_LO			0x403c
+#define  RC_BAR3_CONFIG_LO_SIZE_MASK			0x1f
+
+#define PCIE_MISC_PCIE_STATUS				0x4068
+#define  STATUS_PCIE_PORT_MASK				0x80
+#define  STATUS_PCIE_PORT_SHIFT				7
+#define  STATUS_PCIE_DL_ACTIVE_MASK			0x20
+#define  STATUS_PCIE_DL_ACTIVE_SHIFT			5
+#define  STATUS_PCIE_PHYLINKUP_MASK			0x10
+#define  STATUS_PCIE_PHYLINKUP_SHIFT			4
+
+#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT	0x4070
+#define  MEM_WIN0_BASE_LIMIT_LIMIT_MASK			0xfff00000
+#define  MEM_WIN0_BASE_LIMIT_BASE_MASK			0xfff0
+#define  MEM_WIN0_BASE_LIMIT_BASE_HI_SHIFT		12
+#define PCIE_MEM_WIN0_BASE_LIMIT(win)	\
+	 PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT + ((win) * 4)
+
+#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_HI		0x4080
+#define  MEM_WIN0_BASE_HI_BASE_MASK			0xff
+#define PCIE_MEM_WIN0_BASE_HI(win)	\
+	 PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_HI + ((win) * 8)
+
+#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LIMIT_HI		0x4084
+#define  PCIE_MEM_WIN0_LIMIT_HI_LIMIT_MASK		0xff
+#define PCIE_MEM_WIN0_LIMIT_HI(win)	\
+	 PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LIMIT_HI + ((win) * 8)
+
+#define PCIE_MISC_HARD_PCIE_HARD_DEBUG			0x4204
+#define  PCIE_HARD_DEBUG_CLKREQ_DEBUG_ENABLE_MASK	0x2
+#define  PCIE_HARD_DEBUG_SERDES_IDDQ_MASK		0x08000000
+
+#define PCIE_MSI_INTR2_CLR				0x4508
+#define PCIE_MSI_INTR2_MASK_SET				0x4510
+
+#define PCIE_EXT_CFG_DATA				0x8000
+
+#define PCIE_EXT_CFG_INDEX				0x9000
+#define  PCIE_EXT_BUSNUM_SHIFT				20
+#define  PCIE_EXT_SLOT_SHIFT				15
+#define  PCIE_EXT_FUNC_SHIFT				12
+
+#define PCIE_RGR1_SW_INIT_1				0x9210
+#define  RGR1_SW_INIT_1_PERST_MASK			0x1
+#define  RGR1_SW_INIT_1_INIT_MASK			0x2
+
+/* PCIe parameters */
+#define BRCM_NUM_PCIE_OUT_WINS				4
+
+/* MDIO registers */
+#define MDIO_PORT0					0x0
+#define MDIO_DATA_MASK					0x7fffffff
+#define MDIO_DATA_SHIFT					0
+#define MDIO_PORT_MASK					0xf0000
+#define MDIO_PORT_SHIFT					16
+#define MDIO_REGAD_MASK					0xffff
+#define MDIO_REGAD_SHIFT				0
+#define MDIO_CMD_MASK					0xfff00000
+#define MDIO_CMD_SHIFT					20
+#define MDIO_CMD_READ					0x1
+#define MDIO_CMD_WRITE					0x0
+#define MDIO_DATA_DONE_MASK				0x80000000
+#define SSC_REGS_ADDR					0x1100
+#define SET_ADDR_OFFSET					0x1f
+#define SSC_CNTL_OFFSET					0x2
+#define SSC_CNTL_OVRD_EN_MASK				0x8000
+#define SSC_CNTL_OVRD_VAL_MASK				0x4000
+#define SSC_STATUS_OFFSET				0x1
+#define SSC_STATUS_SSC_MASK				0x400
+#define SSC_STATUS_SSC_SHIFT				10
+#define SSC_STATUS_PLL_LOCK_MASK			0x800
+#define SSC_STATUS_PLL_LOCK_SHIFT			11
+
+/**
+ * struct brcm_pcie - the PCIe controller state
+ * @base: Base address of memory mapped IO registers of the controller
+ * @gen: Non-zero value indicates limitation of the PCIe controller operation
+ *       to a specific generation (1, 2 or 3)
+ * @ssc: true indicates active Spread Spectrum Clocking operation
+ */
+struct brcm_pcie {
+	void __iomem		*base;
+
+	int			gen;
+	bool			ssc;
+};
+
+/**
+ * brcm_pcie_encode_ibar_size() - Encode the inbound "BAR" region size
+ * @size: The inbound region size
+ *
+ * This function converts size of the inbound "BAR" region to the non-linear
+ * values of the PCIE_MISC_RC_BAR[123]_CONFIG_LO register SIZE field.
+ *
+ * Return: The encoded inbound region size
+ */
+static int brcm_pcie_encode_ibar_size(u64 size)
+{
+	int log2_in = ilog2(size);
+
+	if (log2_in >= 12 && log2_in <= 15)
+		/* Covers 4KB to 32KB (inclusive) */
+		return (log2_in - 12) + 0x1c;
+	else if (log2_in >= 16 && log2_in <= 37)
+		/* Covers 64KB to 32GB, (inclusive) */
+		return log2_in - 15;
+
+	/* Something is awry so disable */
+	return 0;
+}
+
+/**
+ * brcm_pcie_rc_mode() - Check if PCIe controller is in RC mode
+ * @pcie: Pointer to the PCIe controller state
+ *
+ * The controller is capable of serving in both RC and EP roles.
+ *
+ * Return: true for RC mode, false for EP mode.
+ */
+static bool brcm_pcie_rc_mode(struct brcm_pcie *pcie)
+{
+	u32 val;
+
+	val = readl(pcie->base + PCIE_MISC_PCIE_STATUS);
+
+	return (val & STATUS_PCIE_PORT_MASK) >> STATUS_PCIE_PORT_SHIFT;
+}
+
+/**
+ * brcm_pcie_link_up() - Check whether the PCIe link is up
+ * @pcie: Pointer to the PCIe controller state
+ *
+ * Return: true if the link is up, false otherwise.
+ */
+static bool brcm_pcie_link_up(struct brcm_pcie *pcie)
+{
+	u32 val, dla, plu;
+
+	val = readl(pcie->base + PCIE_MISC_PCIE_STATUS);
+	dla = (val & STATUS_PCIE_DL_ACTIVE_MASK) >> STATUS_PCIE_DL_ACTIVE_SHIFT;
+	plu = (val & STATUS_PCIE_PHYLINKUP_MASK) >> STATUS_PCIE_PHYLINKUP_SHIFT;
+
+	return dla && plu;
+}
+
+static int brcm_pcie_config_address(const struct udevice *dev, pci_dev_t bdf,
+				    uint offset, void **paddress)
+{
+	struct brcm_pcie *pcie = dev_get_priv(dev);
+	unsigned int pci_bus = PCI_BUS(bdf);
+	unsigned int pci_dev = PCI_DEV(bdf);
+	unsigned int pci_func = PCI_FUNC(bdf);
+	int idx;
+
+	/*
+	 * Busses 0 (host PCIe bridge) and 1 (its immediate child)
+	 * are limited to a single device each
+	 */
+	if (pci_bus < 2 && pci_dev > 0)
+		return -EINVAL;
+
+	/* Accesses to the RC go right to the RC registers */
+	if (pci_bus == 0) {
+		*paddress = pcie->base + offset;
+		return 0;
+	}
+
+	/* For devices, write to the config space index register */
+	idx = (pci_bus << PCIE_EXT_BUSNUM_SHIFT)
+		| (pci_dev << PCIE_EXT_SLOT_SHIFT)
+		| (pci_func << PCIE_EXT_FUNC_SHIFT);
+
+	writel(idx, pcie->base + PCIE_EXT_CFG_INDEX);
+	*paddress = pcie->base + PCIE_EXT_CFG_DATA + offset;
+
+	return 0;
+}
+
+static int brcm_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
+				 uint offset, ulong *valuep,
+				 enum pci_size_t size)
+{
+	return pci_generic_mmap_read_config(bus, brcm_pcie_config_address,
+					    bdf, offset, valuep, size);
+}
+
+static int brcm_pcie_write_config(struct udevice *bus, pci_dev_t bdf,
+				  uint offset, ulong value,
+				  enum pci_size_t size)
+{
+	return pci_generic_mmap_write_config(bus, brcm_pcie_config_address,
+					     bdf, offset, value, size);
+}
+
+static const char *link_speed_to_str(unsigned int cls)
+{
+	switch (cls) {
+	case PCI_EXP_LNKSTA_CLS_2_5GB: return "2.5";
+	case PCI_EXP_LNKSTA_CLS_5_0GB: return "5.0";
+	case PCI_EXP_LNKSTA_CLS_8_0GB: return "8.0";
+	default:
+		break;
+	}
+
+	return "??";
+}
+
+static u32 brcm_pcie_mdio_form_pkt(unsigned int port, unsigned int regad,
+				   unsigned int cmd)
+{
+	u32 pkt;
+
+	pkt = (port << MDIO_PORT_SHIFT) & MDIO_PORT_MASK;
+	pkt |= (regad << MDIO_REGAD_SHIFT) & MDIO_REGAD_MASK;
+	pkt |= (cmd << MDIO_CMD_SHIFT) & MDIO_CMD_MASK;
+
+	return pkt;
+}
+
+/**
+ * brcm_pcie_mdio_read() - Perform a register read on the internal MDIO bus
+ * @base: Pointer to the PCIe controller IO registers
+ * @port: The MDIO port number
+ * @regad: The register address
+ * @val: A pointer@which to store the read value
+ *
+ * Return: 0 on success and register value in @val, negative error value
+ *         on failure.
+ */
+static int brcm_pcie_mdio_read(void __iomem *base, unsigned int port,
+			       unsigned int regad, u32 *val)
+{
+	u32 data, addr;
+	int ret;
+
+	addr = brcm_pcie_mdio_form_pkt(port, regad, MDIO_CMD_READ);
+	writel(addr, base + PCIE_RC_DL_MDIO_ADDR);
+	readl(base + PCIE_RC_DL_MDIO_ADDR);
+
+	ret = readl_poll_timeout(base + PCIE_RC_DL_MDIO_RD_DATA, data,
+				 (data & MDIO_DATA_DONE_MASK), 100);
+
+	*val = data & MDIO_DATA_MASK;
+
+	return ret;
+}
+
+/**
+ * brcm_pcie_mdio_write() - Perform a register write on the internal MDIO bus
+ * @base: Pointer to the PCIe controller IO registers
+ * @port: The MDIO port number
+ * @regad: Address of the register
+ * @wrdata: The value to write
+ *
+ * Return: 0 on success, negative error value on failure.
+ */
+static int brcm_pcie_mdio_write(void __iomem *base, unsigned int port,
+				unsigned int regad, u16 wrdata)
+{
+	u32 data, addr;
+
+	addr = brcm_pcie_mdio_form_pkt(port, regad, MDIO_CMD_WRITE);
+	writel(addr, base + PCIE_RC_DL_MDIO_ADDR);
+	readl(base + PCIE_RC_DL_MDIO_ADDR);
+	writel(MDIO_DATA_DONE_MASK | wrdata, base + PCIE_RC_DL_MDIO_WR_DATA);
+
+	return readl_poll_timeout(base + PCIE_RC_DL_MDIO_WR_DATA, data,
+				  !(data & MDIO_DATA_DONE_MASK), 100);
+}
+
+/**
+ * brcm_pcie_set_ssc() - Configure the controller for Spread Spectrum Clocking
+ * @base: pointer to the PCIe controller IO registers
+ *
+ * Return: 0 on success, negative error value on failure.
+ */
+static int brcm_pcie_set_ssc(void __iomem *base)
+{
+	int pll, ssc;
+	int ret;
+	u32 tmp;
+
+	ret = brcm_pcie_mdio_write(base, MDIO_PORT0, SET_ADDR_OFFSET,
+				   SSC_REGS_ADDR);
+	if (ret < 0)
+		return ret;
+
+	ret = brcm_pcie_mdio_read(base, MDIO_PORT0, SSC_CNTL_OFFSET, &tmp);
+	if (ret < 0)
+		return ret;
+
+	tmp |= (SSC_CNTL_OVRD_EN_MASK | SSC_CNTL_OVRD_VAL_MASK);
+
+	ret = brcm_pcie_mdio_write(base, MDIO_PORT0, SSC_CNTL_OFFSET, tmp);
+	if (ret < 0)
+		return ret;
+
+	udelay(1000);
+	ret = brcm_pcie_mdio_read(base, MDIO_PORT0, SSC_STATUS_OFFSET, &tmp);
+	if (ret < 0)
+		return ret;
+
+	ssc = (tmp & SSC_STATUS_SSC_MASK) >> SSC_STATUS_SSC_SHIFT;
+	pll = (tmp & SSC_STATUS_PLL_LOCK_MASK) >> SSC_STATUS_PLL_LOCK_SHIFT;
+
+	return ssc && pll ? 0 : -EIO;
+}
+
+/**
+ * brcm_pcie_set_gen() - Limits operation to a specific generation (1, 2 or 3)
+ * @pcie: pointer to the PCIe controller state
+ * @gen: PCIe generation to limit the controller's operation to
+ */
+static void brcm_pcie_set_gen(struct brcm_pcie *pcie, unsigned int gen)
+{
+	void __iomem *cap_base = pcie->base + BRCM_PCIE_CAP_REGS;
+
+	u16 lnkctl2 = readw(cap_base + PCI_EXP_LNKCTL2);
+	u32 lnkcap = readl(cap_base + PCI_EXP_LNKCAP);
+
+	lnkcap = (lnkcap & ~PCI_EXP_LNKCAP_SLS) | gen;
+	writel(lnkcap, cap_base + PCI_EXP_LNKCAP);
+
+	lnkctl2 = (lnkctl2 & ~0xf) | gen;
+	writew(lnkctl2, cap_base + PCI_EXP_LNKCTL2);
+}
+
+static void brcm_pcie_set_outbound_win(struct brcm_pcie *pcie,
+				       unsigned int win, u64 phys_addr,
+				       u64 pcie_addr, u64 size)
+{
+	void __iomem *base = pcie->base;
+	u32 phys_addr_mb_high, limit_addr_mb_high;
+	phys_addr_t phys_addr_mb, limit_addr_mb;
+	int high_addr_shift;
+	u32 tmp;
+
+	/* Set the base of the pcie_addr window */
+	writel(lower_32_bits(pcie_addr), base + PCIE_MEM_WIN0_LO(win));
+	writel(upper_32_bits(pcie_addr), base + PCIE_MEM_WIN0_HI(win));
+
+	/* Write the addr base & limit lower bits (in MBs) */
+	phys_addr_mb = phys_addr / SZ_1M;
+	limit_addr_mb = (phys_addr + size - 1) / SZ_1M;
+
+	tmp = readl(base + PCIE_MEM_WIN0_BASE_LIMIT(win));
+	u32p_replace_bits(&tmp, phys_addr_mb,
+			  MEM_WIN0_BASE_LIMIT_BASE_MASK);
+	u32p_replace_bits(&tmp, limit_addr_mb,
+			  MEM_WIN0_BASE_LIMIT_LIMIT_MASK);
+	writel(tmp, base + PCIE_MEM_WIN0_BASE_LIMIT(win));
+
+	/* Write the cpu & limit addr upper bits */
+	high_addr_shift = MEM_WIN0_BASE_LIMIT_BASE_HI_SHIFT;
+	phys_addr_mb_high = phys_addr_mb >> high_addr_shift;
+	tmp = readl(base + PCIE_MEM_WIN0_BASE_HI(win));
+	u32p_replace_bits(&tmp, phys_addr_mb_high,
+			  MEM_WIN0_BASE_HI_BASE_MASK);
+	writel(tmp, base + PCIE_MEM_WIN0_BASE_HI(win));
+
+	limit_addr_mb_high = limit_addr_mb >> high_addr_shift;
+	tmp = readl(base + PCIE_MEM_WIN0_LIMIT_HI(win));
+	u32p_replace_bits(&tmp, limit_addr_mb_high,
+			  PCIE_MEM_WIN0_LIMIT_HI_LIMIT_MASK);
+	writel(tmp, base + PCIE_MEM_WIN0_LIMIT_HI(win));
+}
+
+static int brcm_pcie_probe(struct udevice *dev)
+{
+	struct udevice *ctlr = pci_get_controller(dev);
+	struct pci_controller *hose = dev_get_uclass_priv(ctlr);
+	struct brcm_pcie *pcie = dev_get_priv(dev);
+	void __iomem *base = pcie->base;
+	bool ssc_good = false;
+	int num_out_wins = 0;
+	u64 rc_bar2_offset, rc_bar2_size;
+	unsigned int scb_size_val;
+	int i, ret;
+	u16 nlw, cls, lnksta;
+	u32 tmp;
+
+	/*
+	 * Reset the bridge, assert the fundamental reset. Note for some SoCs,
+	 * e.g. BCM7278, the fundamental reset should not be asserted here.
+	 * This will need to be changed when support for other SoCs is added.
+	 */
+	setbits_le32(base + PCIE_RGR1_SW_INIT_1,
+		     RGR1_SW_INIT_1_INIT_MASK | RGR1_SW_INIT_1_PERST_MASK);
+	/*
+	 * The delay is a safety precaution to preclude the reset signal
+	 * from looking like a glitch.
+	 */
+	udelay(100);
+
+	/* Take the bridge out of reset */
+	clrbits_le32(base + PCIE_RGR1_SW_INIT_1, RGR1_SW_INIT_1_INIT_MASK);
+
+	clrbits_le32(base + PCIE_MISC_HARD_PCIE_HARD_DEBUG,
+		     PCIE_HARD_DEBUG_SERDES_IDDQ_MASK);
+
+	/* Wait for SerDes to be stable */
+	udelay(100);
+
+	/* Set SCB_MAX_BURST_SIZE, CFG_READ_UR_MODE, SCB_ACCESS_EN */
+	clrsetbits_le32(base + PCIE_MISC_MISC_CTRL,
+			MISC_CTRL_MAX_BURST_SIZE_MASK,
+			MISC_CTRL_SCB_ACCESS_EN_MASK |
+			MISC_CTRL_CFG_READ_UR_MODE_MASK |
+			MISC_CTRL_MAX_BURST_SIZE_128);
+	/*
+	 * TODO: When support for other SoCs than BCM2711 is added we may
+	 * need to use the base address and size(s) provided in the dma-ranges
+	 * property.
+	 */
+	rc_bar2_offset = 0;
+	rc_bar2_size = 0xc0000000;
+
+	tmp = lower_32_bits(rc_bar2_offset);
+	u32p_replace_bits(&tmp, brcm_pcie_encode_ibar_size(rc_bar2_size),
+			  RC_BAR2_CONFIG_LO_SIZE_MASK);
+	writel(tmp, base + PCIE_MISC_RC_BAR2_CONFIG_LO);
+	writel(upper_32_bits(rc_bar2_offset),
+	       base + PCIE_MISC_RC_BAR2_CONFIG_HI);
+
+	scb_size_val = rc_bar2_size ?
+		       ilog2(rc_bar2_size) - 15 : 0xf; /* 0xf is 1GB */
+
+	tmp = readl(base + PCIE_MISC_MISC_CTRL);
+	u32p_replace_bits(&tmp, scb_size_val,
+			  MISC_CTRL_SCB0_SIZE_MASK);
+	writel(tmp, base + PCIE_MISC_MISC_CTRL);
+
+	/* Disable the PCIe->GISB memory window (RC_BAR1) */
+	clrbits_le32(base + PCIE_MISC_RC_BAR1_CONFIG_LO,
+		     RC_BAR1_CONFIG_LO_SIZE_MASK);
+
+	/* Disable the PCIe->SCB memory window (RC_BAR3) */
+	clrbits_le32(base + PCIE_MISC_RC_BAR3_CONFIG_LO,
+		     RC_BAR3_CONFIG_LO_SIZE_MASK);
+
+	/* Mask all interrupts since we are not handling any yet */
+	writel(0xffffffff, base + PCIE_MSI_INTR2_MASK_SET);
+
+	/* Clear any interrupts we find on boot */
+	writel(0xffffffff, base + PCIE_MSI_INTR2_CLR);
+
+	if (pcie->gen)
+		brcm_pcie_set_gen(pcie, pcie->gen);
+
+	/* Unassert the fundamental reset */
+	clrbits_le32(pcie->base + PCIE_RGR1_SW_INIT_1,
+		     RGR1_SW_INIT_1_PERST_MASK);
+
+	/* Give the RC/EP time to wake up, before trying to configure RC.
+	 * Intermittently check status for link-up, up to a total of 100ms.
+	 */
+	for (i = 0; i < 100 && !brcm_pcie_link_up(pcie); i += 5)
+		mdelay(5);
+
+	if (!brcm_pcie_link_up(pcie)) {
+		printf("PCIe BRCM: link down\n");
+		return -EINVAL;
+	}
+
+	if (!brcm_pcie_rc_mode(pcie)) {
+		printf("PCIe misconfigured; is in EP mode\n");
+		return -EINVAL;
+	}
+
+	for (i = 0; i < hose->region_count; i++) {
+		struct pci_region *reg = &hose->regions[i];
+
+		if (reg->flags != PCI_REGION_MEM)
+			continue;
+
+		if (num_out_wins >= BRCM_NUM_PCIE_OUT_WINS)
+			return -EINVAL;
+
+		brcm_pcie_set_outbound_win(pcie, num_out_wins, reg->phys_start,
+					   reg->bus_start, reg->size);
+
+		num_out_wins++;
+	}
+
+	/*
+	 * For config space accesses on the RC, show the right class for
+	 * a PCIe-PCIe bridge (the default setting is to be EP mode).
+	 */
+	clrsetbits_le32(base + PCIE_RC_CFG_PRIV1_ID_VAL3,
+			CFG_PRIV1_ID_VAL3_CLASS_CODE_MASK, 0x060400);
+
+	if (pcie->ssc) {
+		ret = brcm_pcie_set_ssc(pcie->base);
+		if (!ret)
+			ssc_good = true;
+		else
+			printf("PCIe BRCM: failed attempt to enter SSC mode\n");
+	}
+
+	lnksta = readw(base + BRCM_PCIE_CAP_REGS + PCI_EXP_LNKSTA);
+	cls = lnksta & PCI_EXP_LNKSTA_CLS;
+	nlw = (lnksta & PCI_EXP_LNKSTA_NLW) >> PCI_EXP_LNKSTA_NLW_SHIFT;
+
+	printf("PCIe BRCM: link up, %s Gbps x%u %s\n", link_speed_to_str(cls),
+	       nlw, ssc_good ? "(SSC)" : "(!SSC)");
+
+	/* PCIe->SCB endian mode for BAR */
+	clrsetbits_le32(base + PCIE_RC_CFG_VENDOR_SPECIFIC_REG1,
+			VENDOR_SPECIFIC_REG1_ENDIAN_MODE_BAR2_MASK,
+			VENDOR_SPECIFIC_REG1_LITTLE_ENDIAN);
+	/*
+	 * Refclk from RC should be gated with CLKREQ# input when ASPM L0s,L1
+	 * is enabled => setting the CLKREQ_DEBUG_ENABLE field to 1.
+	 */
+	setbits_le32(base + PCIE_MISC_HARD_PCIE_HARD_DEBUG,
+		     PCIE_HARD_DEBUG_CLKREQ_DEBUG_ENABLE_MASK);
+
+	return 0;
+}
+
+static int brcm_pcie_ofdata_to_platdata(struct udevice *dev)
+{
+	struct brcm_pcie *pcie = dev_get_priv(dev);
+	ofnode dn = dev_ofnode(dev);
+	u32 max_link_speed;
+	int ret;
+
+	/* Get the controller base address */
+	pcie->base = dev_read_addr_ptr(dev);
+	if (!pcie->base)
+		return -EINVAL;
+
+	pcie->ssc = ofnode_read_bool(dn, "brcm,enable-ssc");
+
+	ret = ofnode_read_u32(dn, "max-link-speed", &max_link_speed);
+	if (ret < 0 || max_link_speed > 4)
+		pcie->gen = 0;
+	else
+		pcie->gen = max_link_speed;
+
+	return 0;
+}
+
+static const struct dm_pci_ops brcm_pcie_ops = {
+	.read_config	= brcm_pcie_read_config,
+	.write_config	= brcm_pcie_write_config,
+};
+
+static const struct udevice_id brcm_pcie_ids[] = {
+	{ .compatible = "brcm,bcm2711-pcie" },
+	{ }
+};
+
+U_BOOT_DRIVER(pcie_brcm_base) = {
+	.name			= "pcie_brcm",
+	.id			= UCLASS_PCI,
+	.ops			= &brcm_pcie_ops,
+	.of_match		= brcm_pcie_ids,
+	.probe			= brcm_pcie_probe,
+	.ofdata_to_platdata	= brcm_pcie_ofdata_to_platdata,
+	.priv_auto_alloc_size	= sizeof(struct brcm_pcie),
+};
-- 
2.7.4

[PATCH v4 9/9] configs: Enable support for the XHCI controller on RPI4 board (ARM 64-bit)

[Sylwester Nawrocki]

From: Marek Szyprowski <m.szyprowski@samsung.com>

This requires enabling BRCMSTB PCIe and XHCI_PCI drivers as well as PCI
and USB commands. To get it working one has to call the following commands:
"pci enum; usb start;", thus such commands have been added to the default
"preboot" environment variable. One has to update their environment if it
is already configured to get this feature working out of the box.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
Changes since v3:
 - added missing rpi_4_defconfig changes
Changes since v2:
 - rpi_4_32b_defconfig, rpi_4_defconfig changes moved to separate
   patch
Changes since v1:
 - removed unneeded CONFIG_XHCI_64BIT_DWORD_ACCESS_ONLY entry.
Changes since RFC:
 - none.
---
 configs/rpi_4_defconfig     | 9 +++++++++
 configs/rpi_arm64_defconfig | 8 +++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/configs/rpi_4_defconfig b/configs/rpi_4_defconfig
index 6d148da..3f1a1b5 100644
--- a/configs/rpi_4_defconfig
+++ b/configs/rpi_4_defconfig
@@ -8,6 +8,8 @@ CONFIG_NR_DRAM_BANKS=2
 CONFIG_DISTRO_DEFAULTS=y
 CONFIG_OF_BOARD_SETUP=y
 # CONFIG_ARCH_FIXUP_FDT_MEMORY is not set
+CONFIG_USE_PREBOOT=y
+CONFIG_PREBOOT="pci enum; usb start;"
 CONFIG_MISC_INIT_R=y
 # CONFIG_DISPLAY_CPUINFO is not set
 # CONFIG_DISPLAY_BOARDINFO is not set
@@ -15,6 +17,8 @@ CONFIG_SYS_PROMPT="U-Boot> "
 CONFIG_CMD_DFU=y
 CONFIG_CMD_GPIO=y
 CONFIG_CMD_MMC=y
+CONFIG_CMD_PCI=y
+CONFIG_CMD_USB=y
 CONFIG_CMD_FS_UUID=y
 CONFIG_OF_BOARD=y
 CONFIG_ENV_FAT_INTERFACE="mmc"
@@ -28,12 +32,17 @@ CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_BCM2835=y
 CONFIG_DM_ETH=y
 CONFIG_BCMGENET=y
+CONFIG_PCI=y
+CONFIG_DM_PCI=y
+CONFIG_PCI_BRCMSTB=y
 CONFIG_PINCTRL=y
 # CONFIG_PINCTRL_GENERIC is not set
 # CONFIG_REQUIRE_SERIAL_CONSOLE is not set
 CONFIG_USB=y
 CONFIG_DM_USB=y
 CONFIG_DM_USB_GADGET=y
+CONFIG_USB_XHCI_HCD=y
+CONFIG_USB_XHCI_PCI=y
 CONFIG_USB_GADGET=y
 CONFIG_USB_GADGET_MANUFACTURER="FSL"
 CONFIG_USB_GADGET_VENDOR_NUM=0x0525
diff --git a/configs/rpi_arm64_defconfig b/configs/rpi_arm64_defconfig
index fea86be..f12d1e3 100644
--- a/configs/rpi_arm64_defconfig
+++ b/configs/rpi_arm64_defconfig
@@ -7,13 +7,14 @@ CONFIG_NR_DRAM_BANKS=2
 CONFIG_DISTRO_DEFAULTS=y
 CONFIG_OF_BOARD_SETUP=y
 CONFIG_USE_PREBOOT=y
-CONFIG_PREBOOT="usb start"
+CONFIG_PREBOOT="pci enum; usb start;"
 CONFIG_MISC_INIT_R=y
 # CONFIG_DISPLAY_CPUINFO is not set
 # CONFIG_DISPLAY_BOARDINFO is not set
 CONFIG_SYS_PROMPT="U-Boot> "
 CONFIG_CMD_GPIO=y
 CONFIG_CMD_MMC=y
+CONFIG_CMD_PCI=y
 CONFIG_CMD_USB=y
 CONFIG_CMD_FS_UUID=y
 CONFIG_OF_BOARD=y
@@ -26,11 +27,16 @@ CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_BCM2835=y
 CONFIG_DM_ETH=y
 CONFIG_BCMGENET=y
+CONFIG_PCI=y
+CONFIG_DM_PCI=y
+CONFIG_PCI_BRCMSTB=y
 CONFIG_PINCTRL=y
 # CONFIG_PINCTRL_GENERIC is not set
 # CONFIG_REQUIRE_SERIAL_CONSOLE is not set
 CONFIG_USB=y
 CONFIG_DM_USB=y
+CONFIG_USB_XHCI_HCD=y
+CONFIG_USB_XHCI_PCI=y
 CONFIG_USB_DWC2=y
 CONFIG_USB_KEYBOARD=y
 CONFIG_USB_HOST_ETHER=y
-- 
2.7.4

[PATCH v4 2/9] usb: xhci: Use only 32-bit accesses in xhci_writeq/xhci_readq

[Simon Glass]

Hi Sylwester,

On Mon, 25 May 2020 at 05:40, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
>
> There might be hardware configurations where 64-bit data accesses
> to XHCI registers are not supported properly.  This patch removes
> the readq/writeq so always two 32-bit accesses are used to read/write
> 64-bit XHCI registers, similarly as it is done in Linux kernel.
>
> This patch fixes operation of the XHCI controller on RPI4 Broadcom
> BCM2711 SoC based board, where the VL805 USB XHCI controller is
> connected to the PCIe Root Complex, which is attached to the system
> through the SCB bridge.
>
> Even though the architecture is 64-bit the PCIe BAR is 32-bit and likely
> the 64-bit wide register accesses initiated by the CPU are not properly
> translated to a sequence of 32-bit PCIe accesses.
> xhci_readq(), for example, always returns same value in upper and lower
> 32-bits, e.g. 0xabcd1234abcd1234 instead of 0x00000000abcd1234.
>
> Cc: Sergey Temerkhanov <s.temerkhanov@gmail.com>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> Reviewed-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> ---
> Changes since v1:
>  - none.
> Changes since RFC:
>  - dropped Kconfig option, switched to not using readq/writeq
>    unconditionally.
> ---
>  include/usb/xhci.h | 8 --------
>  1 file changed, 8 deletions(-)

Then I think this should be done with a quirk flag, enabled for this
particular device via the compatible string. It should not be an #if,
but an if().

Regards,
Simon

[PATCH v4 2/9] usb: xhci: Use only 32-bit accesses in xhci_writeq/xhci_readq

[Sylwester Nawrocki]

Hi Simon,

On 25.05.2020 16:57, Simon Glass wrote:
> On Mon, 25 May 2020 at 05:40, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
>>
>> There might be hardware configurations where 64-bit data accesses
>> to XHCI registers are not supported properly.  This patch removes
>> the readq/writeq so always two 32-bit accesses are used to read/write
>> 64-bit XHCI registers, similarly as it is done in Linux kernel.
>>
>> This patch fixes operation of the XHCI controller on RPI4 Broadcom
>> BCM2711 SoC based board, where the VL805 USB XHCI controller is
>> connected to the PCIe Root Complex, which is attached to the system
>> through the SCB bridge.
>>
>> Even though the architecture is 64-bit the PCIe BAR is 32-bit and likely
>> the 64-bit wide register accesses initiated by the CPU are not properly
>> translated to a sequence of 32-bit PCIe accesses.
>> xhci_readq(), for example, always returns same value in upper and lower
>> 32-bits, e.g. 0xabcd1234abcd1234 instead of 0x00000000abcd1234.

> Then I think this should be done with a quirk flag, enabled for this
> particular device via the compatible string. It should not be an #if,
> but an if().

Thanks for your comments. I will check and see how this could be done.
It might not be so straightforward since the XHCI controller is a PCI 
device matched by the pci_device_id so we would need to be looking 
at the compatible string of the PCI controller to set the quirk in 
the xhci layer. It's the PCI bridge that introduces the limitation, 
not the VL805 XHCI controller chip.

-- 
Regards,
Sylwester

[PATCH v4 2/9] usb: xhci: Use only 32-bit accesses in xhci_writeq/xhci_readq

[Simon Glass]

Hi Sylwester,

On Mon, 25 May 2020 at 10:57, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
>
> Hi Simon,
>
> On 25.05.2020 16:57, Simon Glass wrote:
> > On Mon, 25 May 2020 at 05:40, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
> >>
> >> There might be hardware configurations where 64-bit data accesses
> >> to XHCI registers are not supported properly.  This patch removes
> >> the readq/writeq so always two 32-bit accesses are used to read/write
> >> 64-bit XHCI registers, similarly as it is done in Linux kernel.
> >>
> >> This patch fixes operation of the XHCI controller on RPI4 Broadcom
> >> BCM2711 SoC based board, where the VL805 USB XHCI controller is
> >> connected to the PCIe Root Complex, which is attached to the system
> >> through the SCB bridge.
> >>
> >> Even though the architecture is 64-bit the PCIe BAR is 32-bit and likely
> >> the 64-bit wide register accesses initiated by the CPU are not properly
> >> translated to a sequence of 32-bit PCIe accesses.
> >> xhci_readq(), for example, always returns same value in upper and lower
> >> 32-bits, e.g. 0xabcd1234abcd1234 instead of 0x00000000abcd1234.
>
> > Then I think this should be done with a quirk flag, enabled for this
> > particular device via the compatible string. It should not be an #if,
> > but an if().
>
> Thanks for your comments. I will check and see how this could be done.
> It might not be so straightforward since the XHCI controller is a PCI
> device matched by the pci_device_id so we would need to be looking
> at the compatible string of the PCI controller to set the quirk in
> the xhci layer. It's the PCI bridge that introduces the limitation,
> not the VL805 XHCI controller chip.

OK then it should be modelled as such.

How is this done in Linux?

You can add a quirk in the PCI controller and then XHCI can check its
parent's platdata to see the flag, perhaps, since the parent will
always be UCLASS_PCI.

You can always add the device to the devicetree if needed, and then
you get a compatible string.

Regards,
Simon

[PATCH v4 2/9] usb: xhci: Use only 32-bit accesses in xhci_writeq/xhci_readq

[Sylwester Nawrocki]

Hi Simon,

On 25.05.2020 19:04, Simon Glass wrote:
> On Mon, 25 May 2020 at 10:57, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
>> On 25.05.2020 16:57, Simon Glass wrote:
>>> On Mon, 25 May 2020 at 05:40, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
>>>>
>>>> There might be hardware configurations where 64-bit data accesses
>>>> to XHCI registers are not supported properly.  This patch removes
>>>> the readq/writeq so always two 32-bit accesses are used to read/write
>>>> 64-bit XHCI registers, similarly as it is done in Linux kernel.
>>>>
>>>> This patch fixes operation of the XHCI controller on RPI4 Broadcom
>>>> BCM2711 SoC based board, where the VL805 USB XHCI controller is
>>>> connected to the PCIe Root Complex, which is attached to the system
>>>> through the SCB bridge.
>>>>
>>>> Even though the architecture is 64-bit the PCIe BAR is 32-bit and likely
>>>> the 64-bit wide register accesses initiated by the CPU are not properly
>>>> translated to a sequence of 32-bit PCIe accesses.
>>>> xhci_readq(), for example, always returns same value in upper and lower
>>>> 32-bits, e.g. 0xabcd1234abcd1234 instead of 0x00000000abcd1234.
>>
>>> Then I think this should be done with a quirk flag, enabled for this
>>> particular device via the compatible string. It should not be an #if,
>>> but an if().
>>
>> Thanks for your comments. I will check and see how this could be done.
>> It might not be so straightforward since the XHCI controller is a PCI
>> device matched by the pci_device_id so we would need to be looking
>> at the compatible string of the PCI controller to set the quirk in
>> the xhci layer. It's the PCI bridge that introduces the limitation,
>> not the VL805 XHCI controller chip.
> 
> OK then it should be modelled as such.
> 
> How is this done in Linux?

In Linux simply always two 32-bit accesses are used for 64-bit registers
read/write.

And the quirks in the generic PCI XHCI driver are based on the PCI vendor
and the PCI device ID, so it's not helpful. I couldn't find any reference 
to the parent PCI bridge there.

> You can add a quirk in the PCI controller and then XHCI can check its
> parent's platdata to see the flag, perhaps, since the parent will
> always be UCLASS_PCI.

OK, I imagined something like that.

> You can always add the device to the devicetree if needed, and then
> you get a compatible string.

Will have a look, I wasn't aware we could add a node just for such purpose
without negative side effects.

-- 
Regards,
Sylwester

[PATCH v4 2/9] usb: xhci: Use only 32-bit accesses in xhci_writeq/xhci_readq

[Simon Glass]

Hi Sylwester,

On Mon, 25 May 2020 at 11:42, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
>
> Hi Simon,
>
> On 25.05.2020 19:04, Simon Glass wrote:
> > On Mon, 25 May 2020 at 10:57, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
> >> On 25.05.2020 16:57, Simon Glass wrote:
> >>> On Mon, 25 May 2020 at 05:40, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
> >>>>
> >>>> There might be hardware configurations where 64-bit data accesses
> >>>> to XHCI registers are not supported properly.  This patch removes
> >>>> the readq/writeq so always two 32-bit accesses are used to read/write
> >>>> 64-bit XHCI registers, similarly as it is done in Linux kernel.
> >>>>
> >>>> This patch fixes operation of the XHCI controller on RPI4 Broadcom
> >>>> BCM2711 SoC based board, where the VL805 USB XHCI controller is
> >>>> connected to the PCIe Root Complex, which is attached to the system
> >>>> through the SCB bridge.
> >>>>
> >>>> Even though the architecture is 64-bit the PCIe BAR is 32-bit and likely
> >>>> the 64-bit wide register accesses initiated by the CPU are not properly
> >>>> translated to a sequence of 32-bit PCIe accesses.
> >>>> xhci_readq(), for example, always returns same value in upper and lower
> >>>> 32-bits, e.g. 0xabcd1234abcd1234 instead of 0x00000000abcd1234.
> >>
> >>> Then I think this should be done with a quirk flag, enabled for this
> >>> particular device via the compatible string. It should not be an #if,
> >>> but an if().
> >>
> >> Thanks for your comments. I will check and see how this could be done.
> >> It might not be so straightforward since the XHCI controller is a PCI
> >> device matched by the pci_device_id so we would need to be looking
> >> at the compatible string of the PCI controller to set the quirk in
> >> the xhci layer. It's the PCI bridge that introduces the limitation,
> >> not the VL805 XHCI controller chip.
> >
> > OK then it should be modelled as such.
> >
> > How is this done in Linux?
>
> In Linux simply always two 32-bit accesses are used for 64-bit registers
> read/write.

Well the USB maintainer (Marek) might be OK with that, not sure.

>
> And the quirks in the generic PCI XHCI driver are based on the PCI vendor
> and the PCI device ID, so it's not helpful. I couldn't find any reference
> to the parent PCI bridge there.

In xhci_pci_probe() you can look at the PCI vendor/device  with something like:


struct pci_child_platdata *plat = dev_get_parent_platdata(dev);   //
see comments for that struct in pci.h

int quirks = 0;
if (plat->vendor == xxx && plat->device == xxx)
   quirks |= SOMETHING
xhci_register(...., quirks);  // add a new param

in xhci_register() you can store the quirk in ctrl.

>
> > You can add a quirk in the PCI controller and then XHCI can check its
> > parent's platdata to see the flag, perhaps, since the parent will
> > always be UCLASS_PCI.
>
> OK, I imagined something like that.
>
> > You can always add the device to the devicetree if needed, and then
> > you get a compatible string.
>
> Will have a look, I wasn't aware we could add a node just for such purpose
> without negative side effects.

So long as you get the 'reg' property correct (i.e. same bus, device,
function) then you are OK. See pci-info.rst for docs

Regards,
Simon

[PATCH v4 2/9] usb: xhci: Use only 32-bit accesses in xhci_writeq/xhci_readq

[Marek Szyprowski]

Hi Simon,

On 25.05.2020 23:40, Simon Glass wrote:
> On Mon, 25 May 2020 at 11:42, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
>> On 25.05.2020 19:04, Simon Glass wrote:
>>> On Mon, 25 May 2020 at 10:57, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
>>>> On 25.05.2020 16:57, Simon Glass wrote:
>>>>> On Mon, 25 May 2020 at 05:40, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
>>>>>> There might be hardware configurations where 64-bit data accesses
>>>>>> to XHCI registers are not supported properly.  This patch removes
>>>>>> the readq/writeq so always two 32-bit accesses are used to read/write
>>>>>> 64-bit XHCI registers, similarly as it is done in Linux kernel.
>>>>>>
>>>>>> This patch fixes operation of the XHCI controller on RPI4 Broadcom
>>>>>> BCM2711 SoC based board, where the VL805 USB XHCI controller is
>>>>>> connected to the PCIe Root Complex, which is attached to the system
>>>>>> through the SCB bridge.
>>>>>>
>>>>>> Even though the architecture is 64-bit the PCIe BAR is 32-bit and likely
>>>>>> the 64-bit wide register accesses initiated by the CPU are not properly
>>>>>> translated to a sequence of 32-bit PCIe accesses.
>>>>>> xhci_readq(), for example, always returns same value in upper and lower
>>>>>> 32-bits, e.g. 0xabcd1234abcd1234 instead of 0x00000000abcd1234.
>>>>> Then I think this should be done with a quirk flag, enabled for this
>>>>> particular device via the compatible string. It should not be an #if,
>>>>> but an if().
>>>> Thanks for your comments. I will check and see how this could be done.
>>>> It might not be so straightforward since the XHCI controller is a PCI
>>>> device matched by the pci_device_id so we would need to be looking
>>>> at the compatible string of the PCI controller to set the quirk in
>>>> the xhci layer. It's the PCI bridge that introduces the limitation,
>>>> not the VL805 XHCI controller chip.
>>> OK then it should be modelled as such.
>>>
>>> How is this done in Linux?
>> In Linux simply always two 32-bit accesses are used for 64-bit registers
>> read/write.
> Well the USB maintainer (Marek) might be OK with that, not sure.

32bit access is always safe according to the XHCI specification and such 
one is always used by the Linux kernel. 64bit access might give a few 
CPU cycles of performance improvement, but in case of typical u-boot use 
case this is simply negligible. Do we really need to make the code 
over-engineered for no good reason?

>> And the quirks in the generic PCI XHCI driver are based on the PCI vendor
>> and the PCI device ID, so it's not helpful. I couldn't find any reference
>> to the parent PCI bridge there.
> In xhci_pci_probe() you can look at the PCI vendor/device  with something like:
>
>
> struct pci_child_platdata *plat = dev_get_parent_platdata(dev);   //
> see comments for that struct in pci.h
>
> int quirks = 0;
> if (plat->vendor == xxx && plat->device == xxx)
>     quirks |= SOMETHING
> xhci_register(...., quirks);  // add a new param
>
> in xhci_register() you can store the quirk in ctrl.
>
>>> You can add a quirk in the PCI controller and then XHCI can check its
>>> parent's platdata to see the flag, perhaps, since the parent will
>>> always be UCLASS_PCI.
>> OK, I imagined something like that.
>>
>>> You can always add the device to the devicetree if needed, and then
>>> you get a compatible string.
>> Will have a look, I wasn't aware we could add a node just for such purpose
>> without negative side effects.
> So long as you get the 'reg' property correct (i.e. same bus, device,
> function) then you are OK. See pci-info.rst for docs

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

[PATCH v4 2/9] usb: xhci: Use only 32-bit accesses in xhci_writeq/xhci_readq

[Bin Meng]

Hi Simon,

On Tue, May 26, 2020 at 5:41 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Sylwester,
>
> On Mon, 25 May 2020 at 11:42, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
> >
> > Hi Simon,
> >
> > On 25.05.2020 19:04, Simon Glass wrote:
> > > On Mon, 25 May 2020 at 10:57, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
> > >> On 25.05.2020 16:57, Simon Glass wrote:
> > >>> On Mon, 25 May 2020 at 05:40, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
> > >>>>
> > >>>> There might be hardware configurations where 64-bit data accesses
> > >>>> to XHCI registers are not supported properly.  This patch removes
> > >>>> the readq/writeq so always two 32-bit accesses are used to read/write
> > >>>> 64-bit XHCI registers, similarly as it is done in Linux kernel.
> > >>>>
> > >>>> This patch fixes operation of the XHCI controller on RPI4 Broadcom
> > >>>> BCM2711 SoC based board, where the VL805 USB XHCI controller is
> > >>>> connected to the PCIe Root Complex, which is attached to the system
> > >>>> through the SCB bridge.
> > >>>>
> > >>>> Even though the architecture is 64-bit the PCIe BAR is 32-bit and likely
> > >>>> the 64-bit wide register accesses initiated by the CPU are not properly
> > >>>> translated to a sequence of 32-bit PCIe accesses.
> > >>>> xhci_readq(), for example, always returns same value in upper and lower
> > >>>> 32-bits, e.g. 0xabcd1234abcd1234 instead of 0x00000000abcd1234.
> > >>
> > >>> Then I think this should be done with a quirk flag, enabled for this
> > >>> particular device via the compatible string. It should not be an #if,
> > >>> but an if().
> > >>
> > >> Thanks for your comments. I will check and see how this could be done.
> > >> It might not be so straightforward since the XHCI controller is a PCI
> > >> device matched by the pci_device_id so we would need to be looking
> > >> at the compatible string of the PCI controller to set the quirk in
> > >> the xhci layer. It's the PCI bridge that introduces the limitation,
> > >> not the VL805 XHCI controller chip.
> > >
> > > OK then it should be modelled as such.
> > >
> > > How is this done in Linux?
> >
> > In Linux simply always two 32-bit accesses are used for 64-bit registers
> > read/write.

This was discussed during review of the previous version of this
patch, and I think aligning to what Linux does is fine. Previously we
discussed adding an Kconfig option to control this, but I feel that's
not good. Having a quirk flag to detect this is a dynamic approach,
compared to the static Kconfig option, but overall I don't see a need
not to align with Linux xHCI driver.

>
> Well the USB maintainer (Marek) might be OK with that, not sure.
>
> >
> > And the quirks in the generic PCI XHCI driver are based on the PCI vendor
> > and the PCI device ID, so it's not helpful. I couldn't find any reference
> > to the parent PCI bridge there.
>
> In xhci_pci_probe() you can look at the PCI vendor/device  with something like:
>
>
> struct pci_child_platdata *plat = dev_get_parent_platdata(dev);   //
> see comments for that struct in pci.h
>
> int quirks = 0;
> if (plat->vendor == xxx && plat->device == xxx)
>    quirks |= SOMETHING
> xhci_register(...., quirks);  // add a new param
>
> in xhci_register() you can store the quirk in ctrl.
>
> >
> > > You can add a quirk in the PCI controller and then XHCI can check its
> > > parent's platdata to see the flag, perhaps, since the parent will
> > > always be UCLASS_PCI.
> >
> > OK, I imagined something like that.
> >
> > > You can always add the device to the devicetree if needed, and then
> > > you get a compatible string.
> >
> > Will have a look, I wasn't aware we could add a node just for such purpose
> > without negative side effects.
>
> So long as you get the 'reg' property correct (i.e. same bus, device,
> function) then you are OK. See pci-info.rst for docs
>

Regards,
Bin

[PATCH v4 2/9] usb: xhci: Use only 32-bit accesses in xhci_writeq/xhci_readq

[Sylwester Nawrocki]

Hi,

On 26.05.2020 09:49, Marek Szyprowski wrote:
> On 25.05.2020 23:40, Simon Glass wrote:
>> On Mon, 25 May 2020 at 11:42, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
>>> On 25.05.2020 19:04, Simon Glass wrote:
>>>> On Mon, 25 May 2020 at 10:57, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
>>>>> On 25.05.2020 16:57, Simon Glass wrote:
>>>>>> On Mon, 25 May 2020 at 05:40, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
>>>>>>> There might be hardware configurations where 64-bit data accesses
>>>>>>> to XHCI registers are not supported properly.  This patch removes
>>>>>>> the readq/writeq so always two 32-bit accesses are used to read/write
>>>>>>> 64-bit XHCI registers, similarly as it is done in Linux kernel.
>>>>>>>
>>>>>>> This patch fixes operation of the XHCI controller on RPI4 Broadcom
>>>>>>> BCM2711 SoC based board, where the VL805 USB XHCI controller is
>>>>>>> connected to the PCIe Root Complex, which is attached to the system
>>>>>>> through the SCB bridge.
>>>>>>>
>>>>>>> Even though the architecture is 64-bit the PCIe BAR is 32-bit and likely
>>>>>>> the 64-bit wide register accesses initiated by the CPU are not properly
>>>>>>> translated to a sequence of 32-bit PCIe accesses.
>>>>>>> xhci_readq(), for example, always returns same value in upper and lower
>>>>>>> 32-bits, e.g. 0xabcd1234abcd1234 instead of 0x00000000abcd1234.
>>>>>> Then I think this should be done with a quirk flag, enabled for this
>>>>>> particular device via the compatible string. It should not be an #if,
>>>>>> but an if().
>>>>> Thanks for your comments. I will check and see how this could be done.
>>>>> It might not be so straightforward since the XHCI controller is a PCI
>>>>> device matched by the pci_device_id so we would need to be looking
>>>>> at the compatible string of the PCI controller to set the quirk in
>>>>> the xhci layer. It's the PCI bridge that introduces the limitation,
>>>>> not the VL805 XHCI controller chip.
>>>> OK then it should be modelled as such.
>>>>
>>>> How is this done in Linux?
>>> In Linux simply always two 32-bit accesses are used for 64-bit registers
>>> read/write.

>> Well the USB maintainer (Marek) might be OK with that, not sure.
> 
> 32bit access is always safe according to the XHCI specification and such 
> one is always used by the Linux kernel. 64bit access might give a few 
> CPU cycles of performance improvement, but in case of typical u-boot use 
> case this is simply negligible. Do we really need to make the code 
> over-engineered for no good reason?

I didn't have a chance to measure it but I believe in practice there would be 
no any significant performance improvement. There are only few xhci_readq, 
xhci_writeq calls in the code and most of them are in a one off initialization
procedure. The only one that would matter is in xhci_acknowledge_event(),
but if we add a new (ctrl) argument to xhci_{readq,writeq} and testing of the
quirk it will likely be not much different as if we just used two 32-bit 
reads/writes unconditionally, as far as performance is concerned.

>>> And the quirks in the generic PCI XHCI driver are based on the PCI vendor
>>> and the PCI device ID, so it's not helpful. I couldn't find any reference
>>> to the parent PCI bridge there.

>> In xhci_pci_probe() you can look at the PCI vendor/device  with something like:
>>
>> struct pci_child_platdata *plat = dev_get_parent_platdata(dev);   //
>> see comments for that struct in pci.h

We need to test the PCI bridge so we could use dev->parent rather than dev
here. I'm not sure if the 64-bit data access issue is inherent to the PCI
controller itself or rather to its integration logic within the SoC 
(the SCB bridge). In other words, if in other Broadcom SoC same PCI vendor/
device controller might not need the quirk. I guess such approach would be
good enough. 

The question is whether we want to introduce the quirks mechanism now for
the readq/writeq issue. My feeling is that it's better to align with what
Linux does and the quirks could be useful for other purposes.

>> int quirks = 0;
>> if (plat->vendor == xxx && plat->device == xxx)
>>     quirks |= SOMETHING
>> xhci_register(...., quirks);  // add a new param
>>
>> in xhci_register() you can store the quirk in ctrl.
>>>> You can add a quirk in the PCI controller and then XHCI can check its
>>>> parent's platdata to see the flag, perhaps, since the parent will
>>>> always be UCLASS_PCI.
>>> OK, I imagined something like that.
>>>
>>>> You can always add the device to the devicetree if needed, and then
>>>> you get a compatible string.
>>> Will have a look, I wasn't aware we could add a node just for such purpose
>>> without negative side effects.
>> So long as you get the 'reg' property correct (i.e. same bus, device,
>> function) then you are OK. See pci-info.rst for docs

--
Regards, 
Sylwester

[PATCH v4 2/9] usb: xhci: Use only 32-bit accesses in xhci_writeq/xhci_readq

[Matthias Brugger]

On 26/05/2020 10:07, Bin Meng wrote:
> Hi Simon,
> 
> On Tue, May 26, 2020 at 5:41 AM Simon Glass <sjg@chromium.org> wrote:
>>
>> Hi Sylwester,
>>
>> On Mon, 25 May 2020 at 11:42, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
>>>
>>> Hi Simon,
>>>
>>> On 25.05.2020 19:04, Simon Glass wrote:
>>>> On Mon, 25 May 2020 at 10:57, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
>>>>> On 25.05.2020 16:57, Simon Glass wrote:
>>>>>> On Mon, 25 May 2020 at 05:40, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
>>>>>>>
>>>>>>> There might be hardware configurations where 64-bit data accesses
>>>>>>> to XHCI registers are not supported properly.  This patch removes
>>>>>>> the readq/writeq so always two 32-bit accesses are used to read/write
>>>>>>> 64-bit XHCI registers, similarly as it is done in Linux kernel.
>>>>>>>
>>>>>>> This patch fixes operation of the XHCI controller on RPI4 Broadcom
>>>>>>> BCM2711 SoC based board, where the VL805 USB XHCI controller is
>>>>>>> connected to the PCIe Root Complex, which is attached to the system
>>>>>>> through the SCB bridge.
>>>>>>>
>>>>>>> Even though the architecture is 64-bit the PCIe BAR is 32-bit and likely
>>>>>>> the 64-bit wide register accesses initiated by the CPU are not properly
>>>>>>> translated to a sequence of 32-bit PCIe accesses.
>>>>>>> xhci_readq(), for example, always returns same value in upper and lower
>>>>>>> 32-bits, e.g. 0xabcd1234abcd1234 instead of 0x00000000abcd1234.
>>>>>
>>>>>> Then I think this should be done with a quirk flag, enabled for this
>>>>>> particular device via the compatible string. It should not be an #if,
>>>>>> but an if().
>>>>>
>>>>> Thanks for your comments. I will check and see how this could be done.
>>>>> It might not be so straightforward since the XHCI controller is a PCI
>>>>> device matched by the pci_device_id so we would need to be looking
>>>>> at the compatible string of the PCI controller to set the quirk in
>>>>> the xhci layer. It's the PCI bridge that introduces the limitation,
>>>>> not the VL805 XHCI controller chip.
>>>>
>>>> OK then it should be modelled as such.
>>>>
>>>> How is this done in Linux?
>>>
>>> In Linux simply always two 32-bit accesses are used for 64-bit registers
>>> read/write.
> 
> This was discussed during review of the previous version of this
> patch, and I think aligning to what Linux does is fine. Previously we
> discussed adding an Kconfig option to control this, but I feel that's
> not good. Having a quirk flag to detect this is a dynamic approach,
> compared to the static Kconfig option, but overall I don't see a need
> not to align with Linux xHCI driver.
> 

My understanding is, that we will keep the approach of 32-bit accesses. My plan
was to take the whole series through my treen.

Bin is that fine with you?

Regards,
Matthias

>>
>> Well the USB maintainer (Marek) might be OK with that, not sure.
>>
>>>
>>> And the quirks in the generic PCI XHCI driver are based on the PCI vendor
>>> and the PCI device ID, so it's not helpful. I couldn't find any reference
>>> to the parent PCI bridge there.
>>
>> In xhci_pci_probe() you can look at the PCI vendor/device  with something like:
>>
>>
>> struct pci_child_platdata *plat = dev_get_parent_platdata(dev);   //
>> see comments for that struct in pci.h
>>
>> int quirks = 0;
>> if (plat->vendor == xxx && plat->device == xxx)
>>    quirks |= SOMETHING
>> xhci_register(...., quirks);  // add a new param
>>
>> in xhci_register() you can store the quirk in ctrl.
>>
>>>
>>>> You can add a quirk in the PCI controller and then XHCI can check its
>>>> parent's platdata to see the flag, perhaps, since the parent will
>>>> always be UCLASS_PCI.
>>>
>>> OK, I imagined something like that.
>>>
>>>> You can always add the device to the devicetree if needed, and then
>>>> you get a compatible string.
>>>
>>> Will have a look, I wasn't aware we could add a node just for such purpose
>>> without negative side effects.
>>
>> So long as you get the 'reg' property correct (i.e. same bus, device,
>> function) then you are OK. See pci-info.rst for docs
>>
> 
> Regards,
> Bin
> 

[PATCH v4 2/9] usb: xhci: Use only 32-bit accesses in xhci_writeq/xhci_readq

[Bin Meng]

Hi Matthias,

On Wed, May 27, 2020 at 8:20 PM Matthias Brugger <mbrugger@suse.com> wrote:
>
>
>
> On 26/05/2020 10:07, Bin Meng wrote:
> > Hi Simon,
> >
> > On Tue, May 26, 2020 at 5:41 AM Simon Glass <sjg@chromium.org> wrote:
> >>
> >> Hi Sylwester,
> >>
> >> On Mon, 25 May 2020 at 11:42, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
> >>>
> >>> Hi Simon,
> >>>
> >>> On 25.05.2020 19:04, Simon Glass wrote:
> >>>> On Mon, 25 May 2020 at 10:57, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
> >>>>> On 25.05.2020 16:57, Simon Glass wrote:
> >>>>>> On Mon, 25 May 2020 at 05:40, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
> >>>>>>>
> >>>>>>> There might be hardware configurations where 64-bit data accesses
> >>>>>>> to XHCI registers are not supported properly.  This patch removes
> >>>>>>> the readq/writeq so always two 32-bit accesses are used to read/write
> >>>>>>> 64-bit XHCI registers, similarly as it is done in Linux kernel.
> >>>>>>>
> >>>>>>> This patch fixes operation of the XHCI controller on RPI4 Broadcom
> >>>>>>> BCM2711 SoC based board, where the VL805 USB XHCI controller is
> >>>>>>> connected to the PCIe Root Complex, which is attached to the system
> >>>>>>> through the SCB bridge.
> >>>>>>>
> >>>>>>> Even though the architecture is 64-bit the PCIe BAR is 32-bit and likely
> >>>>>>> the 64-bit wide register accesses initiated by the CPU are not properly
> >>>>>>> translated to a sequence of 32-bit PCIe accesses.
> >>>>>>> xhci_readq(), for example, always returns same value in upper and lower
> >>>>>>> 32-bits, e.g. 0xabcd1234abcd1234 instead of 0x00000000abcd1234.
> >>>>>
> >>>>>> Then I think this should be done with a quirk flag, enabled for this
> >>>>>> particular device via the compatible string. It should not be an #if,
> >>>>>> but an if().
> >>>>>
> >>>>> Thanks for your comments. I will check and see how this could be done.
> >>>>> It might not be so straightforward since the XHCI controller is a PCI
> >>>>> device matched by the pci_device_id so we would need to be looking
> >>>>> at the compatible string of the PCI controller to set the quirk in
> >>>>> the xhci layer. It's the PCI bridge that introduces the limitation,
> >>>>> not the VL805 XHCI controller chip.
> >>>>
> >>>> OK then it should be modelled as such.
> >>>>
> >>>> How is this done in Linux?
> >>>
> >>> In Linux simply always two 32-bit accesses are used for 64-bit registers
> >>> read/write.
> >
> > This was discussed during review of the previous version of this
> > patch, and I think aligning to what Linux does is fine. Previously we
> > discussed adding an Kconfig option to control this, but I feel that's
> > not good. Having a quirk flag to detect this is a dynamic approach,
> > compared to the static Kconfig option, but overall I don't see a need
> > not to align with Linux xHCI driver.
> >
>
> My understanding is, that we will keep the approach of 32-bit accesses. My plan
> was to take the whole series through my treen.
>
> Bin is that fine with you?

No problem. Thanks!

Regards,
Bin

[PATCH v4 0/9] USB host support for Raspberry Pi 4 board (64-bit)

[Matthias Brugger]

Hi Sylwester,

On 25/05/2020 13:39, Sylwester Nawrocki wrote:
> Hi all,
> 
> This patch series adds USB host support for Raspberry Pi 4 board. 
> It includes the Broadcom STB PCIe controller driver ported from Linux 
> kernel, a memory mapping update for the xHCI controller on PCIe bus
> for 64-bit builds and some related fixes and updates in the usb/xhci 
> and the pci driver core code.
> 
> The 32-bit ARM part has be excluded from the series and will be posted
> separately.
> 
> The difference comparing to previous version is only in the config update
> patch "configs: Enable support for the XHCI controller on RPI4 board (ARM
>  64-bit)".
> 
> The patch series is based on v2020.07-rc1 tree.
> 
> Thanks,
> Sylwester
> 
> Marek Szyprowski (3):
>   rpi4: shorten a mapping for the DRAM
>   rpi4: add a mapping for the PCIe XHCI controller MMIO registers (ARM
>     64bit)
>   configs: Enable support for the XHCI controller on RPI4 board (ARM
>     64-bit)
> 
> Nicolas Saenz Julienne (1):
>   linux/bitfield.h: Add primitives for manipulating bitfields both in
>     host- and fixed-endian
> 
> Sylwester Nawrocki (5):
>   usb: xhci: Add missing cache flush in the scratchpad array
>     initialization
>   usb: xhci: Use only 32-bit accesses in xhci_writeq/xhci_readq
>   pci: Move some PCIe register offset definitions to a common header
>   pci: Add some PCI Express capability register offset definitions
>   pci: Add driver for Broadcom BCM2711 SoC PCIe controller
> 

Series looks good to me. The problem is that on RPi4 with 8 GB the USB FW get's
loaded via the RPi FW. Nicolas provided a series which is under review right
now. Without this series the 8 GB RPi4 won't boot. As soon as the series is
ready I'll take all the RPi4 PCI/USB related patches into the next branch.

Regards,
Matthias

>  arch/arm/mach-bcm283x/init.c  |  20 +-
>  configs/rpi_4_defconfig       |   9 +
>  configs/rpi_arm64_defconfig   |   8 +-
>  drivers/pci/Kconfig           |   9 +
>  drivers/pci/Makefile          |   1 +
>  drivers/pci/pci-rcar-gen3.c   |   8 -
>  drivers/pci/pcie_brcmstb.c    | 623 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/pci/pcie_intel_fpga.c |   3 -
>  drivers/usb/host/xhci-mem.c   |   3 +
>  include/linux/bitfield.h      |  52 ++++
>  include/pci.h                 |  22 +-
>  include/usb/xhci.h            |   8 -
>  12 files changed, 740 insertions(+), 26 deletions(-)
>  create mode 100644 drivers/pci/pcie_brcmstb.c
> 

[PATCH v4 0/9] USB host support for Raspberry Pi 4 board (64-bit)

[Matthias Brugger]

On 25/05/2020 13:39, Sylwester Nawrocki wrote:
> Hi all,
> 
> This patch series adds USB host support for Raspberry Pi 4 board.
> It includes the Broadcom STB PCIe controller driver ported from Linux
> kernel, a memory mapping update for the xHCI controller on PCIe bus
> for 64-bit builds and some related fixes and updates in the usb/xhci
> and the pci driver core code.
> 
> The 32-bit ARM part has be excluded from the series and will be posted
> separately.
> 
> The difference comparing to previous version is only in the config update
> patch "configs: Enable support for the XHCI controller on RPI4 board (ARM
>   64-bit)".
> 
> The patch series is based on v2020.07-rc1 tree.
> 
> Thanks,
> Sylwester
> 
> Marek Szyprowski (3):
>    rpi4: shorten a mapping for the DRAM
>    rpi4: add a mapping for the PCIe XHCI controller MMIO registers (ARM
>      64bit)
>    configs: Enable support for the XHCI controller on RPI4 board (ARM
>      64-bit)
> 
> Nicolas Saenz Julienne (1):
>    linux/bitfield.h: Add primitives for manipulating bitfields both in
>      host- and fixed-endian
> 
> Sylwester Nawrocki (5):
>    usb: xhci: Add missing cache flush in the scratchpad array
>      initialization
>    usb: xhci: Use only 32-bit accesses in xhci_writeq/xhci_readq
>    pci: Move some PCIe register offset definitions to a common header
>    pci: Add some PCI Express capability register offset definitions
>    pci: Add driver for Broadcom BCM2711 SoC PCIe controller

Applied now to rpi-next

Thanks!

> 
>   arch/arm/mach-bcm283x/init.c  |  20 +-
>   configs/rpi_4_defconfig       |   9 +
>   configs/rpi_arm64_defconfig   |   8 +-
>   drivers/pci/Kconfig           |   9 +
>   drivers/pci/Makefile          |   1 +
>   drivers/pci/pci-rcar-gen3.c   |   8 -
>   drivers/pci/pcie_brcmstb.c    | 623 ++++++++++++++++++++++++++++++++++++++++++
>   drivers/pci/pcie_intel_fpga.c |   3 -
>   drivers/usb/host/xhci-mem.c   |   3 +
>   include/linux/bitfield.h      |  52 ++++
>   include/pci.h                 |  22 +-
>   include/usb/xhci.h            |   8 -
>   12 files changed, 740 insertions(+), 26 deletions(-)
>   create mode 100644 drivers/pci/pcie_brcmstb.c
> 

[PATCH v4 6/9] linux/bitfield.h: Add primitives for manipulating bitfields both in host- and fixed-endian

[Matthias Brugger]

On 25/05/2020 13:39, Sylwester Nawrocki wrote:
> From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> 
> Imports Al Viro's original Linux commit 00b0c9b82663a, which contains
> an in depth explanation and two fixes from Johannes Berg:
>   e7d4a95da86e0 "bitfield: fix *_encode_bits()",
>   37a3862e12382 "bitfield: add u8 helpers".
> 
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> [s.nawrocki: added empty lines between functions and macros]
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

This seems to break compilation of
arch/arm/mach-uniphier/clk/pll-base-ld20.c

https://gitlab.denx.de/u-boot/custodians/u-boot-raspberrypi/-/jobs/121248

Nicolas, Sylwester, can you please have a look.

Regards,
Matthias

> ---
> Changes since v3:
>   - none.
> Changes since v2:
>   - added some more missing whitespaces as pointed out by checkpatch.pl.
> Changes since v1:
>   - added empty lines between functions and macros.
> Changes since RFC:
>   - new patch.
> ---
>   include/linux/bitfield.h | 52 ++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 52 insertions(+)
> 
> diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
> index 8b9d6ff..7ad8b08 100644
> --- a/include/linux/bitfield.h
> +++ b/include/linux/bitfield.h
> @@ -103,4 +103,56 @@
>   		(typeof(_mask))(((_reg) & (_mask)) >> __bf_shf(_mask));	\
>   	})
>   
> +extern void __compiletime_error("value doesn't fit into mask")
> +__field_overflow(void);
> +extern void __compiletime_error("bad bitfield mask")
> +__bad_mask(void);
> +
> +static __always_inline u64 field_multiplier(u64 field)
> +{
> +	if ((field | (field - 1)) & ((field | (field - 1)) + 1))
> +		__bad_mask();
> +	return field & -field;
> +}
> +
> +static __always_inline u64 field_mask(u64 field)
> +{
> +	return field / field_multiplier(field);
> +}
> +
> +#define ____MAKE_OP(type, base, to, from)				\
> +static __always_inline __##type type##_encode_bits(base v, base field)	\
> +{									\
> +	if (__builtin_constant_p(v) && (v & ~field_mask(field)))	\
> +		__field_overflow();					\
> +	return to((v & field_mask(field)) * field_multiplier(field));	\
> +}									\
> +static __always_inline __##type type##_replace_bits(__##type old,	\
> +					base val, base field)		\
> +{									\
> +	return (old & ~to(field)) | type##_encode_bits(val, field);	\
> +}									\
> +static __always_inline void type##p_replace_bits(__##type * p,		\
> +					base val, base field)		\
> +{									\
> +	*p = (*p & ~to(field)) | type##_encode_bits(val, field);	\
> +}									\
> +static __always_inline base type##_get_bits(__##type v, base field)	\
> +{									\
> +	return (from(v) & field) / field_multiplier(field);		\
> +}
> +
> +#define __MAKE_OP(size)							\
> +	____MAKE_OP(le##size, u##size, cpu_to_le##size, le##size##_to_cpu) \
> +	____MAKE_OP(be##size, u##size, cpu_to_be##size, be##size##_to_cpu) \
> +	____MAKE_OP(u##size, u##size, ,)
> +
> +____MAKE_OP(u8, u8, ,)
> +__MAKE_OP(16)
> +__MAKE_OP(32)
> +__MAKE_OP(64)
> +
> +#undef __MAKE_OP
> +#undef ____MAKE_OP
> +
>   #endif
> 

[PATCH v4 6/9] linux/bitfield.h: Add primitives for manipulating bitfields both in host- and fixed-endian

[Matthias Brugger]

On 09/07/2020 17:35, Matthias Brugger wrote:
> 
> 
> On 25/05/2020 13:39, Sylwester Nawrocki wrote:
>> From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>>
>> Imports Al Viro's original Linux commit 00b0c9b82663a, which contains
>> an in depth explanation and two fixes from Johannes Berg:
>> ? e7d4a95da86e0 "bitfield: fix *_encode_bits()",
>> ? 37a3862e12382 "bitfield: add u8 helpers".
>>
>> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>> [s.nawrocki: added empty lines between functions and macros]
>> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> 
> This seems to break compilation of
> arch/arm/mach-uniphier/clk/pll-base-ld20.c
> 
> https://gitlab.denx.de/u-boot/custodians/u-boot-raspberrypi/-/jobs/121248
> 
> Nicolas, Sylwester, can you please have a look.
> 

Just in case you can't access the URL:
https://pastebin.com/KhDHf8e1

> Regards,
> Matthias
> 
>> ---
>> Changes since v3:
>> ? - none.
>> Changes since v2:
>> ? - added some more missing whitespaces as pointed out by checkpatch.pl.
>> Changes since v1:
>> ? - added empty lines between functions and macros.
>> Changes since RFC:
>> ? - new patch.
>> ---
>> ? include/linux/bitfield.h | 52 ++++++++++++++++++++++++++++++++++++++++++++++++
>> ? 1 file changed, 52 insertions(+)
>>
>> diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
>> index 8b9d6ff..7ad8b08 100644
>> --- a/include/linux/bitfield.h
>> +++ b/include/linux/bitfield.h
>> @@ -103,4 +103,56 @@
>> ????????? (typeof(_mask))(((_reg) & (_mask)) >> __bf_shf(_mask));??? \
>> ????? })
>> +extern void __compiletime_error("value doesn't fit into mask")
>> +__field_overflow(void);
>> +extern void __compiletime_error("bad bitfield mask")
>> +__bad_mask(void);
>> +
>> +static __always_inline u64 field_multiplier(u64 field)
>> +{
>> +??? if ((field | (field - 1)) & ((field | (field - 1)) + 1))
>> +??????? __bad_mask();
>> +??? return field & -field;
>> +}
>> +
>> +static __always_inline u64 field_mask(u64 field)
>> +{
>> +??? return field / field_multiplier(field);
>> +}
>> +
>> +#define ____MAKE_OP(type, base, to, from)??????????????? \
>> +static __always_inline __##type type##_encode_bits(base v, base field)??? \
>> +{??????????????????????????????????? \
>> +??? if (__builtin_constant_p(v) && (v & ~field_mask(field)))??? \
>> +??????? __field_overflow();??????????????????? \
>> +??? return to((v & field_mask(field)) * field_multiplier(field));??? \
>> +}??????????????????????????????????? \
>> +static __always_inline __##type type##_replace_bits(__##type old,??? \
>> +??????????????????? base val, base field)??????? \
>> +{??????????????????????????????????? \
>> +??? return (old & ~to(field)) | type##_encode_bits(val, field);??? \
>> +}??????????????????????????????????? \
>> +static __always_inline void type##p_replace_bits(__##type * p,??????? \
>> +??????????????????? base val, base field)??????? \
>> +{??????????????????????????????????? \
>> +??? *p = (*p & ~to(field)) | type##_encode_bits(val, field);??? \
>> +}??????????????????????????????????? \
>> +static __always_inline base type##_get_bits(__##type v, base field)??? \
>> +{??????????????????????????????????? \
>> +??? return (from(v) & field) / field_multiplier(field);??????? \
>> +}
>> +
>> +#define __MAKE_OP(size)??????????????????????????? \
>> +??? ____MAKE_OP(le##size, u##size, cpu_to_le##size, le##size##_to_cpu) \
>> +??? ____MAKE_OP(be##size, u##size, cpu_to_be##size, be##size##_to_cpu) \
>> +??? ____MAKE_OP(u##size, u##size, ,)
>> +
>> +____MAKE_OP(u8, u8, ,)
>> +__MAKE_OP(16)
>> +__MAKE_OP(32)
>> +__MAKE_OP(64)
>> +
>> +#undef __MAKE_OP
>> +#undef ____MAKE_OP
>> +
>> ? #endif
>>

[PATCH] include/bitfield.h: include byteorder.h

[Nicolas Saenz Julienne]

This is needed in order to use the cpu_to_*()/*_to_cpu() family of
functions.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---

IMO the ideal thing would be to squash this into the original patch.

 include/linux/bitfield.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
index 7ad8b088ed..7ad02f8cbb 100644
--- a/include/linux/bitfield.h
+++ b/include/linux/bitfield.h
@@ -16,6 +16,7 @@
 #define _LINUX_BITFIELD_H
 
 #include <linux/bug.h>
+#include <asm/byteorder.h>
 
 /*
  * Bitfield access macros
-- 
2.27.0



-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200709/19ad489b/attachment.sig>

[PATCH] include/bitfield.h: include byteorder.h

[Matthias Brugger]

On 09/07/2020 19:41, Nicolas Saenz Julienne wrote:
> This is needed in order to use the cpu_to_*()/*_to_cpu() family of
> functions.
> 
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> ---
> 
> IMO the ideal thing would be to squash this into the original patch.

I'll test and squash, thanks!

> 
>   include/linux/bitfield.h | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
> index 7ad8b088ed..7ad02f8cbb 100644
> --- a/include/linux/bitfield.h
> +++ b/include/linux/bitfield.h
> @@ -16,6 +16,7 @@
>   #define _LINUX_BITFIELD_H
>   
>   #include <linux/bug.h>
> +#include <asm/byteorder.h>
>   
>   /*
>    * Bitfield access macros
>