[PATCH v2 00/10] USB host support for Raspberry Pi 4 board

[PATCH v2 00/10] USB host support for Raspberry Pi 4 board

[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 32-bit and 64-bit system builds and some related fixes and updates
in the usb/xhci and the pci driver core code.

This iteration includes minor corrections in patches 7, 9, 10 addressing
review comments.

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

Thanks,
Sylwester

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

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 STB PCIe controller

 arch/arm/mach-bcm283x/Kconfig             |   1 +
 arch/arm/mach-bcm283x/include/mach/base.h |   7 +
 arch/arm/mach-bcm283x/init.c              |  72 +++-
 configs/rpi_4_32b_defconfig               |   9 +
 configs/rpi_4_defconfig                   |   9 +
 configs/rpi_arm64_defconfig               |   8 +-
 drivers/pci/Kconfig                       |   6 +
 drivers/pci/Makefile                      |   1 +
 drivers/pci/pci-rcar-gen3.c               |   8 -
 drivers/pci/pcie_brcmstb.c                | 594 ++++++++++++++++++++++++++++++
 drivers/pci/pcie_intel_fpga.c             |   3 -
 drivers/usb/host/xhci-mem.c               |   3 +
 include/linux/bitfield.h                  |  50 +++
 include/pci.h                             |  19 +-
 include/usb/xhci.h                        |   8 -
 15 files changed, 772 insertions(+), 26 deletions(-)
 create mode 100644 drivers/pci/pcie_brcmstb.c

-- 
2.7.4

[PATCH v2 01/10] 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 v2 02/10] 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 v2 03/10] 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 v2 04/10] 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 v2 05/10] 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 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..6a748da 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 MAX_MAP_MAX_ENTRIES (4)
+
+static struct mm_region bcm283x_mem_map[MAX_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[MAX_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 < MAX_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 v2 05/10] rpi4: add a mapping for the PCIe XHCI controller MMIO registers (ARM 64bit)

[Matthias Brugger]

On 04/05/2020 14:45, Sylwester Nawrocki wrote:
> 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 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..6a748da 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 MAX_MAP_MAX_ENTRIES (4)

What stands the second 'MAX' for?

> +
> +static struct mm_region bcm283x_mem_map[MAX_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[MAX_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
>  	}, {

I'd prefer a comment instead of using the BCM2711_RPI4_PCIE_XHCI_MMIO_* defines.

> +		.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 < MAX_MAP_MAX_ENTRIES; i++) {

Variable mem_map points to bcm283x_mem_map which only holds two mm_region's
(plus list terminator). So we have an overflow here. I think we should just
define bcm2711_mem_map and bcm283x_mem_map with a fixed array size of 4 (see
comment on the define naming above).

Regards,
Matthias

>  		mem_map[i].virt = pd[i].virt;
>  		mem_map[i].phys = pd[i].phys;
>  		mem_map[i].size = pd[i].size;
> 

[PATCH v2 05/10] rpi4: add a mapping for the PCIe XHCI controller MMIO registers (ARM 64bit)

[Matthias Brugger]

On 05/05/2020 16:00, Matthias Brugger wrote:
> 
> 
> On 04/05/2020 14:45, Sylwester Nawrocki wrote:
>> 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 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..6a748da 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 MAX_MAP_MAX_ENTRIES (4)
> 
> What stands the second 'MAX' for?
> 
>> +
>> +static struct mm_region bcm283x_mem_map[MAX_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[MAX_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
>>  	}, {
> 
> I'd prefer a comment instead of using the BCM2711_RPI4_PCIE_XHCI_MMIO_* defines.
> 

Ok, I see now you use the defines later on, forget my comment. It's ok as is.

Regards,
Matthias

>> +		.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 < MAX_MAP_MAX_ENTRIES; i++) {
> 
> Variable mem_map points to bcm283x_mem_map which only holds two mm_region's
> (plus list terminator). So we have an overflow here. I think we should just
> define bcm2711_mem_map and bcm283x_mem_map with a fixed array size of 4 (see
> comment on the define naming above).
> 
> Regards,
> Matthias
> 
>>  		mem_map[i].virt = pd[i].virt;
>>  		mem_map[i].phys = pd[i].phys;
>>  		mem_map[i].size = pd[i].size;
>>

[PATCH v2 05/10] rpi4: add a mapping for the PCIe XHCI controller MMIO registers (ARM 64bit)

[Marek Szyprowski]

Hi Matthias,

On 05.05.2020 16:00, Matthias Brugger wrote:
> On 04/05/2020 14:45, Sylwester Nawrocki wrote:
>> 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 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..6a748da 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 MAX_MAP_MAX_ENTRIES (4)
> What stands the second 'MAX' for?
a simple copy/paste issue. I will fix it.
>> +
>> +static struct mm_region bcm283x_mem_map[MAX_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[MAX_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
>>   	}, {
> I'd prefer a comment instead of using the BCM2711_RPI4_PCIE_XHCI_MMIO_* defines.

Those defines are also used in ARM 32bit code.

>> +		.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 < MAX_MAP_MAX_ENTRIES; i++) {
> Variable mem_map points to bcm283x_mem_map which only holds two mm_region's
> (plus list terminator). So we have an overflow here.
Nope, I've changed the bcm283x_mem_map to be large enough, see the above 
diff.
>   I think we should just
> define bcm2711_mem_map and bcm283x_mem_map with a fixed array size of 4 (see
> comment on the define naming above).

That's exactly what I did.

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

[PATCH v2 05/10] rpi4: add a mapping for the PCIe XHCI controller MMIO registers (ARM 64bit)

[Matthias Brugger]

On 05/05/2020 16:10, Marek Szyprowski wrote:
> Hi Matthias,
> 
> On 05.05.2020 16:00, Matthias Brugger wrote:
>> On 04/05/2020 14:45, Sylwester Nawrocki wrote:
>>> 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 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..6a748da 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 MAX_MAP_MAX_ENTRIES (4)
>> What stands the second 'MAX' for?
> a simple copy/paste issue. I will fix it.
>>> +
>>> +static struct mm_region bcm283x_mem_map[MAX_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[MAX_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
>>>   	}, {
>> I'd prefer a comment instead of using the BCM2711_RPI4_PCIE_XHCI_MMIO_* defines.
> 
> Those defines are also used in ARM 32bit code.
> 
>>> +		.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 < MAX_MAP_MAX_ENTRIES; i++) {
>> Variable mem_map points to bcm283x_mem_map which only holds two mm_region's
>> (plus list terminator). So we have an overflow here.
> Nope, I've changed the bcm283x_mem_map to be large enough, see the above 
> diff.
>>   I think we should just
>> define bcm2711_mem_map and bcm283x_mem_map with a fixed array size of 4 (see
>> comment on the define naming above).
> 
> That's exactly what I did.
> 

You are right, sorry for the noise!

Matthias

[PATCH v2 06/10] rpi4: add a mapping for the PCIe XHCI controller MMIO registers (ARM 32bit)

[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. Due to 32bit limit in the CPU virtual address space in ARM
32bit mode, this region is mapped at 0xff800000 CPU virtual address.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
Changes since v1:
 - none.
---
 arch/arm/mach-bcm283x/Kconfig             |  1 +
 arch/arm/mach-bcm283x/include/mach/base.h |  7 +++++
 arch/arm/mach-bcm283x/init.c              | 52 +++++++++++++++++++++++++++++++
 3 files changed, 60 insertions(+)

diff --git a/arch/arm/mach-bcm283x/Kconfig b/arch/arm/mach-bcm283x/Kconfig
index 00419bf..bcb7f1d 100644
--- a/arch/arm/mach-bcm283x/Kconfig
+++ b/arch/arm/mach-bcm283x/Kconfig
@@ -36,6 +36,7 @@ config BCM2711_32B
 	select BCM2711
 	select ARMV7_LPAE
 	select CPU_V7A
+	select PHYS_64BIT
 
 config BCM2711_64B
 	bool "Broadcom BCM2711 SoC 64-bit support"
diff --git a/arch/arm/mach-bcm283x/include/mach/base.h b/arch/arm/mach-bcm283x/include/mach/base.h
index c4ae398..1d10dc9 100644
--- a/arch/arm/mach-bcm283x/include/mach/base.h
+++ b/arch/arm/mach-bcm283x/include/mach/base.h
@@ -6,6 +6,13 @@
 #ifndef _BCM283x_BASE_H_
 #define _BCM283x_BASE_H_
 
+#include <linux/types.h>
+
 extern unsigned long rpi_bcm283x_base;
 
+#ifdef CONFIG_ARMV7_LPAE
+extern void *rpi4_phys_to_virt(phys_addr_t paddr);
+#define phys_to_virt(x) rpi4_phys_to_virt(x)
+#endif
+
 #endif
diff --git a/arch/arm/mach-bcm283x/init.c b/arch/arm/mach-bcm283x/init.c
index 6a748da..5d0d160 100644
--- a/arch/arm/mach-bcm283x/init.c
+++ b/arch/arm/mach-bcm283x/init.c
@@ -145,6 +145,58 @@ int mach_cpu_init(void)
 }
 
 #ifdef CONFIG_ARMV7_LPAE
+
+#define BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT	0xff800000UL
+
+void *rpi4_phys_to_virt(phys_addr_t paddr)
+{
+	if (paddr >= BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS)
+		paddr = paddr - BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS +
+			BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT;
+	return (void *)(unsigned long)paddr;
+}
+
+static void set_section_phys(unsigned int section, phys_addr_t phys,
+			     enum dcache_option option)
+{
+	u64 *page_table = (u64 *)gd->arch.tlb_addr;
+	/* Need to set the access flag to not fault */
+	u64 value = TTB_SECT_AP | TTB_SECT_AF;
+
+	/* Add the page offset */
+	value |= (phys);
+
+	/* Add caching bits */
+	value |= option;
+
+	/* Set PTE */
+	page_table[section] = value;
+}
+
+static void rpi4_create_pcie_xhci_mapping(void)
+{
+	unsigned sect = BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT >> MMU_SECTION_SHIFT;
+	phys_addr_t phys_addr = BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS;
+	unsigned int size = BCM2711_RPI4_PCIE_XHCI_MMIO_SIZE;
+
+	while (size) {
+		set_section_phys(sect, phys_addr, DCACHE_OFF);
+		sect++;
+		phys_addr += MMU_SECTION_SIZE;
+		size -= MMU_SECTION_SIZE;
+	}
+}
+
+void arm_init_domains(void)
+{
+	/*
+	 * Hijack this function to prepare a mappings for the PCIe MMIO
+	 * region for the XHCI controller on RPi4 board.
+	 * This code is called before enabling the MMU in ARM 32bit mode.
+	 */
+	rpi4_create_pcie_xhci_mapping();
+}
+
 void enable_caches(void)
 {
 	dcache_enable();
-- 
2.7.4

[PATCH v2 06/10] rpi4: add a mapping for the PCIe XHCI controller MMIO registers (ARM 32bit)

[Matthias Brugger]

On 04/05/2020 14:45, Sylwester Nawrocki wrote:
> 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. Due to 32bit limit in the CPU virtual address space in ARM
> 32bit mode, this region is mapped at 0xff800000 CPU virtual address.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
> Changes since v1:
>  - none.
> ---
>  arch/arm/mach-bcm283x/Kconfig             |  1 +
>  arch/arm/mach-bcm283x/include/mach/base.h |  7 +++++
>  arch/arm/mach-bcm283x/init.c              | 52 +++++++++++++++++++++++++++++++
>  3 files changed, 60 insertions(+)
> 
> diff --git a/arch/arm/mach-bcm283x/Kconfig b/arch/arm/mach-bcm283x/Kconfig
> index 00419bf..bcb7f1d 100644
> --- a/arch/arm/mach-bcm283x/Kconfig
> +++ b/arch/arm/mach-bcm283x/Kconfig
> @@ -36,6 +36,7 @@ config BCM2711_32B
>  	select BCM2711
>  	select ARMV7_LPAE
>  	select CPU_V7A
> +	select PHYS_64BIT
>  
>  config BCM2711_64B
>  	bool "Broadcom BCM2711 SoC 64-bit support"
> diff --git a/arch/arm/mach-bcm283x/include/mach/base.h b/arch/arm/mach-bcm283x/include/mach/base.h
> index c4ae398..1d10dc9 100644
> --- a/arch/arm/mach-bcm283x/include/mach/base.h
> +++ b/arch/arm/mach-bcm283x/include/mach/base.h
> @@ -6,6 +6,13 @@
>  #ifndef _BCM283x_BASE_H_
>  #define _BCM283x_BASE_H_
>  
> +#include <linux/types.h>
> +
>  extern unsigned long rpi_bcm283x_base;
>  
> +#ifdef CONFIG_ARMV7_LPAE
> +extern void *rpi4_phys_to_virt(phys_addr_t paddr);
> +#define phys_to_virt(x) rpi4_phys_to_virt(x)
> +#endif
> +
>  #endif
> diff --git a/arch/arm/mach-bcm283x/init.c b/arch/arm/mach-bcm283x/init.c
> index 6a748da..5d0d160 100644
> --- a/arch/arm/mach-bcm283x/init.c
> +++ b/arch/arm/mach-bcm283x/init.c
> @@ -145,6 +145,58 @@ int mach_cpu_init(void)
>  }
>  
>  #ifdef CONFIG_ARMV7_LPAE
> +
> +#define BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT	0xff800000UL
> +
> +void *rpi4_phys_to_virt(phys_addr_t paddr)
> +{
> +	if (paddr >= BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS)
> +		paddr = paddr - BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS +
> +			BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT;
> +	return (void *)(unsigned long)paddr;
> +}
> +
> +static void set_section_phys(unsigned int section, phys_addr_t phys,
> +			     enum dcache_option option)
> +{
> +	u64 *page_table = (u64 *)gd->arch.tlb_addr;
> +	/* Need to set the access flag to not fault */
> +	u64 value = TTB_SECT_AP | TTB_SECT_AF;
> +
> +	/* Add the page offset */
> +	value |= (phys);
> +
> +	/* Add caching bits */
> +	value |= option;
> +
> +	/* Set PTE */
> +	page_table[section] = value;
> +}
> +
> +static void rpi4_create_pcie_xhci_mapping(void)
> +{
> +	unsigned sect = BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT >> MMU_SECTION_SHIFT;
> +	phys_addr_t phys_addr = BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS;
> +	unsigned int size = BCM2711_RPI4_PCIE_XHCI_MMIO_SIZE;
> +
> +	while (size) {
> +		set_section_phys(sect, phys_addr, DCACHE_OFF);
> +		sect++;
> +		phys_addr += MMU_SECTION_SIZE;
> +		size -= MMU_SECTION_SIZE;
> +	}

Why can't we reuse mmu_set_region_dcache_behaviour()?

> +}
> +
> +void arm_init_domains(void)
> +{
> +	/*
> +	 * Hijack this function to prepare a mappings for the PCIe MMIO
> +	 * region for the XHCI controller on RPi4 board.
> +	 * This code is called before enabling the MMU in ARM 32bit mode.
> +	 */
> +	rpi4_create_pcie_xhci_mapping();

I think this indirection is not necessary.

Regards,
Matthias

> +}
> +
>  void enable_caches(void)
>  {
>  	dcache_enable();
> 

[PATCH v2 06/10] rpi4: add a mapping for the PCIe XHCI controller MMIO registers (ARM 32bit)

[Marek Szyprowski]

Hi Matthias,

On 05.05.2020 16:25, Matthias Brugger wrote:
> On 04/05/2020 14:45, Sylwester Nawrocki wrote:
>> 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. Due to 32bit limit in the CPU virtual address space in ARM
>> 32bit mode, this region is mapped at 0xff800000 CPU virtual address.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> ---
>> Changes since v1:
>>   - none.
>> ---
>>   arch/arm/mach-bcm283x/Kconfig             |  1 +
>>   arch/arm/mach-bcm283x/include/mach/base.h |  7 +++++
>>   arch/arm/mach-bcm283x/init.c              | 52 +++++++++++++++++++++++++++++++
>>   3 files changed, 60 insertions(+)
>>
>> diff --git a/arch/arm/mach-bcm283x/Kconfig b/arch/arm/mach-bcm283x/Kconfig
>> index 00419bf..bcb7f1d 100644
>> --- a/arch/arm/mach-bcm283x/Kconfig
>> +++ b/arch/arm/mach-bcm283x/Kconfig
>> @@ -36,6 +36,7 @@ config BCM2711_32B
>>   	select BCM2711
>>   	select ARMV7_LPAE
>>   	select CPU_V7A
>> +	select PHYS_64BIT
>>   
>>   config BCM2711_64B
>>   	bool "Broadcom BCM2711 SoC 64-bit support"
>> diff --git a/arch/arm/mach-bcm283x/include/mach/base.h b/arch/arm/mach-bcm283x/include/mach/base.h
>> index c4ae398..1d10dc9 100644
>> --- a/arch/arm/mach-bcm283x/include/mach/base.h
>> +++ b/arch/arm/mach-bcm283x/include/mach/base.h
>> @@ -6,6 +6,13 @@
>>   #ifndef _BCM283x_BASE_H_
>>   #define _BCM283x_BASE_H_
>>   
>> +#include <linux/types.h>
>> +
>>   extern unsigned long rpi_bcm283x_base;
>>   
>> +#ifdef CONFIG_ARMV7_LPAE
>> +extern void *rpi4_phys_to_virt(phys_addr_t paddr);
>> +#define phys_to_virt(x) rpi4_phys_to_virt(x)
>> +#endif
>> +
>>   #endif
>> diff --git a/arch/arm/mach-bcm283x/init.c b/arch/arm/mach-bcm283x/init.c
>> index 6a748da..5d0d160 100644
>> --- a/arch/arm/mach-bcm283x/init.c
>> +++ b/arch/arm/mach-bcm283x/init.c
>> @@ -145,6 +145,58 @@ int mach_cpu_init(void)
>>   }
>>   
>>   #ifdef CONFIG_ARMV7_LPAE
>> +
>> +#define BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT	0xff800000UL
>> +
>> +void *rpi4_phys_to_virt(phys_addr_t paddr)
>> +{
>> +	if (paddr >= BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS)
>> +		paddr = paddr - BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS +
>> +			BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT;
>> +	return (void *)(unsigned long)paddr;
>> +}
>> +
>> +static void set_section_phys(unsigned int section, phys_addr_t phys,
>> +			     enum dcache_option option)
>> +{
>> +	u64 *page_table = (u64 *)gd->arch.tlb_addr;
>> +	/* Need to set the access flag to not fault */
>> +	u64 value = TTB_SECT_AP | TTB_SECT_AF;
>> +
>> +	/* Add the page offset */
>> +	value |= (phys);
>> +
>> +	/* Add caching bits */
>> +	value |= option;
>> +
>> +	/* Set PTE */
>> +	page_table[section] = value;
>> +}
>> +
>> +static void rpi4_create_pcie_xhci_mapping(void)
>> +{
>> +	unsigned sect = BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT >> MMU_SECTION_SHIFT;
>> +	phys_addr_t phys_addr = BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS;
>> +	unsigned int size = BCM2711_RPI4_PCIE_XHCI_MMIO_SIZE;
>> +
>> +	while (size) {
>> +		set_section_phys(sect, phys_addr, DCACHE_OFF);
>> +		sect++;
>> +		phys_addr += MMU_SECTION_SIZE;
>> +		size -= MMU_SECTION_SIZE;
>> +	}
> Why can't we reuse mmu_set_region_dcache_behaviour()?

Because it doesn't allow to set arbirtary physical address - it only 
works for the cases where phys == virt.


>
>> +}
>> +
>> +void arm_init_domains(void)
>> +{
>> +	/*
>> +	 * Hijack this function to prepare a mappings for the PCIe MMIO
>> +	 * region for the XHCI controller on RPi4 board.
>> +	 * This code is called before enabling the MMU in ARM 32bit mode.
>> +	 */
>> +	rpi4_create_pcie_xhci_mapping();
> I think this indirection is not necessary.

Okay.


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

[PATCH v2 06/10] rpi4: add a mapping for the PCIe XHCI controller MMIO registers (ARM 32bit)

[Matthias Brugger]

Adding Tom as he is the arm maintainer.

On 04/05/2020 14:45, Sylwester Nawrocki wrote:
> 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. Due to 32bit limit in the CPU virtual address space in ARM
> 32bit mode, this region is mapped at 0xff800000 CPU virtual address.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> ---
> Changes since v1:
>  - none.
> ---
>  arch/arm/mach-bcm283x/Kconfig             |  1 +
>  arch/arm/mach-bcm283x/include/mach/base.h |  7 +++++
>  arch/arm/mach-bcm283x/init.c              | 52 +++++++++++++++++++++++++++++++
>  3 files changed, 60 insertions(+)
> 
> diff --git a/arch/arm/mach-bcm283x/Kconfig b/arch/arm/mach-bcm283x/Kconfig
> index 00419bf..bcb7f1d 100644
> --- a/arch/arm/mach-bcm283x/Kconfig
> +++ b/arch/arm/mach-bcm283x/Kconfig
> @@ -36,6 +36,7 @@ config BCM2711_32B
>  	select BCM2711
>  	select ARMV7_LPAE
>  	select CPU_V7A
> +	select PHYS_64BIT
>  
>  config BCM2711_64B
>  	bool "Broadcom BCM2711 SoC 64-bit support"
> diff --git a/arch/arm/mach-bcm283x/include/mach/base.h b/arch/arm/mach-bcm283x/include/mach/base.h
> index c4ae398..1d10dc9 100644
> --- a/arch/arm/mach-bcm283x/include/mach/base.h
> +++ b/arch/arm/mach-bcm283x/include/mach/base.h
> @@ -6,6 +6,13 @@
>  #ifndef _BCM283x_BASE_H_
>  #define _BCM283x_BASE_H_
>  
> +#include <linux/types.h>
> +
>  extern unsigned long rpi_bcm283x_base;
>  
> +#ifdef CONFIG_ARMV7_LPAE
> +extern void *rpi4_phys_to_virt(phys_addr_t paddr);
> +#define phys_to_virt(x) rpi4_phys_to_virt(x)
> +#endif
> +
>  #endif
> diff --git a/arch/arm/mach-bcm283x/init.c b/arch/arm/mach-bcm283x/init.c
> index 6a748da..5d0d160 100644
> --- a/arch/arm/mach-bcm283x/init.c
> +++ b/arch/arm/mach-bcm283x/init.c
> @@ -145,6 +145,58 @@ int mach_cpu_init(void)
>  }
>  
>  #ifdef CONFIG_ARMV7_LPAE
> +
> +#define BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT	0xff800000UL
> +
> +void *rpi4_phys_to_virt(phys_addr_t paddr)
> +{
> +	if (paddr >= BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS)
> +		paddr = paddr - BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS +
> +			BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT;
> +	return (void *)(unsigned long)paddr;

I think for this cases we have addrmap_phys_to_virt() which up to now is only
used by powerpc.

> +}
> +
> +static void set_section_phys(unsigned int section, phys_addr_t phys,
> +			     enum dcache_option option)
> +{
> +	u64 *page_table = (u64 *)gd->arch.tlb_addr;
> +	/* Need to set the access flag to not fault */
> +	u64 value = TTB_SECT_AP | TTB_SECT_AF;
> +
> +	/* Add the page offset */
> +	value |= (phys);
> +
> +	/* Add caching bits */
> +	value |= option;
> +
> +	/* Set PTE */
> +	page_table[section] = value;
> +}
> +
> +static void rpi4_create_pcie_xhci_mapping(void)
> +{
> +	unsigned sect = BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT >> MMU_SECTION_SHIFT;
> +	phys_addr_t phys_addr = BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS;
> +	unsigned int size = BCM2711_RPI4_PCIE_XHCI_MMIO_SIZE;
> +
> +	while (size) {
> +		set_section_phys(sect, phys_addr, DCACHE_OFF);
> +		sect++;
> +		phys_addr += MMU_SECTION_SIZE;
> +		size -= MMU_SECTION_SIZE;
> +	}
> +}

I wonder if we can't do all this in the pcie driver probe function. Maybe we can
create a new function like mmu_set_region_dcache_behaviour_phys which allows us
to update a mapping that's not 1:1.

Tom what do you think?

Regards,
Matthias

> +
> +void arm_init_domains(void)
> +{
> +	/*
> +	 * Hijack this function to prepare a mappings for the PCIe MMIO
> +	 * region for the XHCI controller on RPi4 board.
> +	 * This code is called before enabling the MMU in ARM 32bit mode.
> +	 */
> +	rpi4_create_pcie_xhci_mapping();
> +}
> +
>  void enable_caches(void)
>  {
>  	dcache_enable();
> 

[PATCH v2 06/10] rpi4: add a mapping for the PCIe XHCI controller MMIO registers (ARM 32bit)

[Tom Rini]

On Fri, May 08, 2020 at 11:26:36PM +0200, Matthias Brugger wrote:
> Adding Tom as he is the arm maintainer.
> 
> On 04/05/2020 14:45, Sylwester Nawrocki wrote:
> > 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. Due to 32bit limit in the CPU virtual address space in ARM
> > 32bit mode, this region is mapped at 0xff800000 CPU virtual address.
> > 
> > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> > ---
> > Changes since v1:
> >  - none.
> > ---
> >  arch/arm/mach-bcm283x/Kconfig             |  1 +
> >  arch/arm/mach-bcm283x/include/mach/base.h |  7 +++++
> >  arch/arm/mach-bcm283x/init.c              | 52 +++++++++++++++++++++++++++++++
> >  3 files changed, 60 insertions(+)
> > 
> > diff --git a/arch/arm/mach-bcm283x/Kconfig b/arch/arm/mach-bcm283x/Kconfig
> > index 00419bf..bcb7f1d 100644
> > --- a/arch/arm/mach-bcm283x/Kconfig
> > +++ b/arch/arm/mach-bcm283x/Kconfig
> > @@ -36,6 +36,7 @@ config BCM2711_32B
> >  	select BCM2711
> >  	select ARMV7_LPAE
> >  	select CPU_V7A
> > +	select PHYS_64BIT
> >  
> >  config BCM2711_64B
> >  	bool "Broadcom BCM2711 SoC 64-bit support"
> > diff --git a/arch/arm/mach-bcm283x/include/mach/base.h b/arch/arm/mach-bcm283x/include/mach/base.h
> > index c4ae398..1d10dc9 100644
> > --- a/arch/arm/mach-bcm283x/include/mach/base.h
> > +++ b/arch/arm/mach-bcm283x/include/mach/base.h
> > @@ -6,6 +6,13 @@
> >  #ifndef _BCM283x_BASE_H_
> >  #define _BCM283x_BASE_H_
> >  
> > +#include <linux/types.h>
> > +
> >  extern unsigned long rpi_bcm283x_base;
> >  
> > +#ifdef CONFIG_ARMV7_LPAE
> > +extern void *rpi4_phys_to_virt(phys_addr_t paddr);
> > +#define phys_to_virt(x) rpi4_phys_to_virt(x)
> > +#endif
> > +
> >  #endif
> > diff --git a/arch/arm/mach-bcm283x/init.c b/arch/arm/mach-bcm283x/init.c
> > index 6a748da..5d0d160 100644
> > --- a/arch/arm/mach-bcm283x/init.c
> > +++ b/arch/arm/mach-bcm283x/init.c
> > @@ -145,6 +145,58 @@ int mach_cpu_init(void)
> >  }
> >  
> >  #ifdef CONFIG_ARMV7_LPAE
> > +
> > +#define BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT	0xff800000UL
> > +
> > +void *rpi4_phys_to_virt(phys_addr_t paddr)
> > +{
> > +	if (paddr >= BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS)
> > +		paddr = paddr - BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS +
> > +			BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT;
> > +	return (void *)(unsigned long)paddr;
> 
> I think for this cases we have addrmap_phys_to_virt() which up to now is only
> used by powerpc.
> 
> > +}
> > +
> > +static void set_section_phys(unsigned int section, phys_addr_t phys,
> > +			     enum dcache_option option)
> > +{
> > +	u64 *page_table = (u64 *)gd->arch.tlb_addr;
> > +	/* Need to set the access flag to not fault */
> > +	u64 value = TTB_SECT_AP | TTB_SECT_AF;
> > +
> > +	/* Add the page offset */
> > +	value |= (phys);
> > +
> > +	/* Add caching bits */
> > +	value |= option;
> > +
> > +	/* Set PTE */
> > +	page_table[section] = value;
> > +}
> > +
> > +static void rpi4_create_pcie_xhci_mapping(void)
> > +{
> > +	unsigned sect = BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT >> MMU_SECTION_SHIFT;
> > +	phys_addr_t phys_addr = BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS;
> > +	unsigned int size = BCM2711_RPI4_PCIE_XHCI_MMIO_SIZE;
> > +
> > +	while (size) {
> > +		set_section_phys(sect, phys_addr, DCACHE_OFF);
> > +		sect++;
> > +		phys_addr += MMU_SECTION_SIZE;
> > +		size -= MMU_SECTION_SIZE;
> > +	}
> > +}
> 
> I wonder if we can't do all this in the pcie driver probe function. Maybe we can
> create a new function like mmu_set_region_dcache_behaviour_phys which allows us
> to update a mapping that's not 1:1.
> 
> Tom what do you think?

I think a harder look at how PowerPC handled this situation is in order,
and then following / extending that path is in order.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200511/c2452851/attachment.sig>

[PATCH v2 06/10] rpi4: add a mapping for the PCIe XHCI controller MMIO registers (ARM 32bit)

[Matthias Brugger]

On 11/05/2020 21:44, Tom Rini wrote:
> On Fri, May 08, 2020 at 11:26:36PM +0200, Matthias Brugger wrote:
>> Adding Tom as he is the arm maintainer.
>>
>> On 04/05/2020 14:45, Sylwester Nawrocki wrote:
>>> 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. Due to 32bit limit in the CPU virtual address space in ARM
>>> 32bit mode, this region is mapped at 0xff800000 CPU virtual address.
>>>
>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>>> ---
>>> Changes since v1:
>>>  - none.
>>> ---
>>>  arch/arm/mach-bcm283x/Kconfig             |  1 +
>>>  arch/arm/mach-bcm283x/include/mach/base.h |  7 +++++
>>>  arch/arm/mach-bcm283x/init.c              | 52 +++++++++++++++++++++++++++++++
>>>  3 files changed, 60 insertions(+)
>>>
>>> diff --git a/arch/arm/mach-bcm283x/Kconfig b/arch/arm/mach-bcm283x/Kconfig
>>> index 00419bf..bcb7f1d 100644
>>> --- a/arch/arm/mach-bcm283x/Kconfig
>>> +++ b/arch/arm/mach-bcm283x/Kconfig
>>> @@ -36,6 +36,7 @@ config BCM2711_32B
>>>  	select BCM2711
>>>  	select ARMV7_LPAE
>>>  	select CPU_V7A
>>> +	select PHYS_64BIT
>>>  
>>>  config BCM2711_64B
>>>  	bool "Broadcom BCM2711 SoC 64-bit support"
>>> diff --git a/arch/arm/mach-bcm283x/include/mach/base.h b/arch/arm/mach-bcm283x/include/mach/base.h
>>> index c4ae398..1d10dc9 100644
>>> --- a/arch/arm/mach-bcm283x/include/mach/base.h
>>> +++ b/arch/arm/mach-bcm283x/include/mach/base.h
>>> @@ -6,6 +6,13 @@
>>>  #ifndef _BCM283x_BASE_H_
>>>  #define _BCM283x_BASE_H_
>>>  
>>> +#include <linux/types.h>
>>> +
>>>  extern unsigned long rpi_bcm283x_base;
>>>  
>>> +#ifdef CONFIG_ARMV7_LPAE
>>> +extern void *rpi4_phys_to_virt(phys_addr_t paddr);
>>> +#define phys_to_virt(x) rpi4_phys_to_virt(x)
>>> +#endif
>>> +
>>>  #endif
>>> diff --git a/arch/arm/mach-bcm283x/init.c b/arch/arm/mach-bcm283x/init.c
>>> index 6a748da..5d0d160 100644
>>> --- a/arch/arm/mach-bcm283x/init.c
>>> +++ b/arch/arm/mach-bcm283x/init.c
>>> @@ -145,6 +145,58 @@ int mach_cpu_init(void)
>>>  }
>>>  
>>>  #ifdef CONFIG_ARMV7_LPAE
>>> +
>>> +#define BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT	0xff800000UL
>>> +
>>> +void *rpi4_phys_to_virt(phys_addr_t paddr)
>>> +{
>>> +	if (paddr >= BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS)
>>> +		paddr = paddr - BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS +
>>> +			BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT;
>>> +	return (void *)(unsigned long)paddr;
>>
>> I think for this cases we have addrmap_phys_to_virt() which up to now is only
>> used by powerpc.
>>
>>> +}
>>> +
>>> +static void set_section_phys(unsigned int section, phys_addr_t phys,
>>> +			     enum dcache_option option)
>>> +{
>>> +	u64 *page_table = (u64 *)gd->arch.tlb_addr;
>>> +	/* Need to set the access flag to not fault */
>>> +	u64 value = TTB_SECT_AP | TTB_SECT_AF;
>>> +
>>> +	/* Add the page offset */
>>> +	value |= (phys);
>>> +
>>> +	/* Add caching bits */
>>> +	value |= option;
>>> +
>>> +	/* Set PTE */
>>> +	page_table[section] = value;
>>> +}
>>> +
>>> +static void rpi4_create_pcie_xhci_mapping(void)
>>> +{
>>> +	unsigned sect = BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT >> MMU_SECTION_SHIFT;
>>> +	phys_addr_t phys_addr = BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS;
>>> +	unsigned int size = BCM2711_RPI4_PCIE_XHCI_MMIO_SIZE;
>>> +
>>> +	while (size) {
>>> +		set_section_phys(sect, phys_addr, DCACHE_OFF);
>>> +		sect++;
>>> +		phys_addr += MMU_SECTION_SIZE;
>>> +		size -= MMU_SECTION_SIZE;
>>> +	}
>>> +}
>>
>> I wonder if we can't do all this in the pcie driver probe function. Maybe we can
>> create a new function like mmu_set_region_dcache_behaviour_phys which allows us
>> to update a mapping that's not 1:1.
>>
>> Tom what do you think?
> 
> I think a harder look at how PowerPC handled this situation is in order,
> and then following / extending that path is in order.
> 

Thanks Tom for the feedback.
Sylwester, I propose to split the series in two. One for adding the driver to
64-bit U-Boot and another one to add support for rpi_4_32b_defconfig. This way
we could get the driver merged for 2020.07 for sure, while 32-bit parts could
take more cycles to be ready. What do you think?

Regards,
Matthias

[PATCH v2 06/10] rpi4: add a mapping for the PCIe XHCI controller MMIO registers (ARM 32bit)

[Sylwester Nawrocki]

On 11.05.2020 21:47, Matthias Brugger wrote:
>>>> static void rpi4_create_pcie_xhci_mapping(void)
>>>> +{
>>>> +	unsigned sect = BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT >> MMU_SECTION_SHIFT;
>>>> +	phys_addr_t phys_addr = BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS;
>>>> +	unsigned int size = BCM2711_RPI4_PCIE_XHCI_MMIO_SIZE;
>>>> +
>>>> +	while (size) {
>>>> +		set_section_phys(sect, phys_addr, DCACHE_OFF);
>>>> +		sect++;
>>>> +		phys_addr += MMU_SECTION_SIZE;
>>>> +		size -= MMU_SECTION_SIZE;
>>>> +	}
>>>> +}
>>> I wonder if we can't do all this in the pcie driver probe function. Maybe we can
>>> create a new function like mmu_set_region_dcache_behaviour_phys which allows us
>>> to update a mapping that's not 1:1.
>>>
>>> Tom what do you think?
>> I think a harder look at how PowerPC handled this situation is in order,
>> and then following / extending that path is in order.
>>
> Thanks Tom for the feedback.
> Sylwester, I propose to split the series in two. One for adding the driver to
> 64-bit U-Boot and another one to add support for rpi_4_32b_defconfig. This way
> we could get the driver merged for 2020.07 for sure, while 32-bit parts could
> take more cycles to be ready. What do you think?

Sounds good to me, I will split the series and will post the 64-bits part
first, while we work on the 32-bit part according to your suggestions.

-- 
Thanks,
Sylwester

[PATCH v2 06/10] rpi4: add a mapping for the PCIe XHCI controller MMIO registers (ARM 32bit)

[Marek Szyprowski]

Hi Matthias,

On 08.05.2020 23:26, Matthias Brugger wrote:
> Adding Tom as he is the arm maintainer.
>
> On 04/05/2020 14:45, Sylwester Nawrocki wrote:
>> 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. Due to 32bit limit in the CPU virtual address space in ARM
>> 32bit mode, this region is mapped at 0xff800000 CPU virtual address.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>> ---
>> Changes since v1:
>>   - none.
>> ---
>>   arch/arm/mach-bcm283x/Kconfig             |  1 +
>>   arch/arm/mach-bcm283x/include/mach/base.h |  7 +++++
>>   arch/arm/mach-bcm283x/init.c              | 52 +++++++++++++++++++++++++++++++
>>   3 files changed, 60 insertions(+)
>>
>> diff --git a/arch/arm/mach-bcm283x/Kconfig b/arch/arm/mach-bcm283x/Kconfig
>> index 00419bf..bcb7f1d 100644
>> --- a/arch/arm/mach-bcm283x/Kconfig
>> +++ b/arch/arm/mach-bcm283x/Kconfig
>> @@ -36,6 +36,7 @@ config BCM2711_32B
>>   	select BCM2711
>>   	select ARMV7_LPAE
>>   	select CPU_V7A
>> +	select PHYS_64BIT
>>   
>>   config BCM2711_64B
>>   	bool "Broadcom BCM2711 SoC 64-bit support"
>> diff --git a/arch/arm/mach-bcm283x/include/mach/base.h b/arch/arm/mach-bcm283x/include/mach/base.h
>> index c4ae398..1d10dc9 100644
>> --- a/arch/arm/mach-bcm283x/include/mach/base.h
>> +++ b/arch/arm/mach-bcm283x/include/mach/base.h
>> @@ -6,6 +6,13 @@
>>   #ifndef _BCM283x_BASE_H_
>>   #define _BCM283x_BASE_H_
>>   
>> +#include <linux/types.h>
>> +
>>   extern unsigned long rpi_bcm283x_base;
>>   
>> +#ifdef CONFIG_ARMV7_LPAE
>> +extern void *rpi4_phys_to_virt(phys_addr_t paddr);
>> +#define phys_to_virt(x) rpi4_phys_to_virt(x)
>> +#endif
>> +
>>   #endif
>> diff --git a/arch/arm/mach-bcm283x/init.c b/arch/arm/mach-bcm283x/init.c
>> index 6a748da..5d0d160 100644
>> --- a/arch/arm/mach-bcm283x/init.c
>> +++ b/arch/arm/mach-bcm283x/init.c
>> @@ -145,6 +145,58 @@ int mach_cpu_init(void)
>>   }
>>   
>>   #ifdef CONFIG_ARMV7_LPAE
>> +
>> +#define BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT	0xff800000UL
>> +
>> +void *rpi4_phys_to_virt(phys_addr_t paddr)
>> +{
>> +	if (paddr >= BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS)
>> +		paddr = paddr - BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS +
>> +			BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT;
>> +	return (void *)(unsigned long)paddr;
> I think for this cases we have addrmap_phys_to_virt() which up to now is only
> used by powerpc.
>
>> +}
>> +
>> +static void set_section_phys(unsigned int section, phys_addr_t phys,
>> +			     enum dcache_option option)
>> +{
>> +	u64 *page_table = (u64 *)gd->arch.tlb_addr;
>> +	/* Need to set the access flag to not fault */
>> +	u64 value = TTB_SECT_AP | TTB_SECT_AF;
>> +
>> +	/* Add the page offset */
>> +	value |= (phys);
>> +
>> +	/* Add caching bits */
>> +	value |= option;
>> +
>> +	/* Set PTE */
>> +	page_table[section] = value;
>> +}
>> +
>> +static void rpi4_create_pcie_xhci_mapping(void)
>> +{
>> +	unsigned sect = BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT >> MMU_SECTION_SHIFT;
>> +	phys_addr_t phys_addr = BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS;
>> +	unsigned int size = BCM2711_RPI4_PCIE_XHCI_MMIO_SIZE;
>> +
>> +	while (size) {
>> +		set_section_phys(sect, phys_addr, DCACHE_OFF);
>> +		sect++;
>> +		phys_addr += MMU_SECTION_SIZE;
>> +		size -= MMU_SECTION_SIZE;
>> +	}
>> +}
> I wonder if we can't do all this in the pcie driver probe function. Maybe we can
> create a new function like mmu_set_region_dcache_behaviour_phys which allows us
> to update a mapping that's not 1:1.
>
> Tom what do you think?

In theory this could be moved to pcie probe, but then how the pcie 
driver would know WHERE in the virtual address space it should create a 
mapping for the needed MMIO area? IMHO this is something SoC or board 
specific and creating such mapping in board init is justified.

I will adapt the code to use addr_map helpers.

Best regards

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

[RFC PATCH 0/2] ARM: arbitrary virtual-physical mappings for RPi4 XHCI support

[Marek Szyprowski]

Hi All,

This is a result of the following discussion:
https://lists.denx.de/pipermail/u-boot/2020-May/411086.html

Those 2 patches are replacement for the patch discussed there.

Best regards
Marek Szyprowski
Samsung R&D Institute Poland


Marek Szyprowski (2):
  arm: provide a function for boards init code to modify MMU
    virtual-physical map
  rpi4: add a mapping for the PCIe XHCI controller MMIO registers (ARM
    32bit)

 arch/arm/include/asm/mmu.h                |  8 ++++++++
 arch/arm/include/asm/system.h             | 18 ++++++++++++++++--
 arch/arm/lib/cache-cp15.c                 | 18 ++++++++++++------
 arch/arm/mach-bcm283x/Kconfig             |  1 +
 arch/arm/mach-bcm283x/include/mach/base.h |  6 ++++++
 arch/arm/mach-bcm283x/init.c              | 14 ++++++++++++++
 include/configs/rpi.h                     |  5 +++++
 7 files changed, 62 insertions(+), 8 deletions(-)
 create mode 100644 arch/arm/include/asm/mmu.h

-- 
1.9.1

[RFC PATCH 1/2] arm: provide a function for boards init code to modify MMU virtual-physical map

[Marek Szyprowski]

Provide a function for setting arbitrary virtual-physical MMU mapping
for the given region.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/include/asm/mmu.h    |  8 ++++++++
 arch/arm/include/asm/system.h | 18 ++++++++++++++++--
 arch/arm/lib/cache-cp15.c     | 18 ++++++++++++------
 3 files changed, 36 insertions(+), 8 deletions(-)
 create mode 100644 arch/arm/include/asm/mmu.h

diff --git a/arch/arm/include/asm/mmu.h b/arch/arm/include/asm/mmu.h
new file mode 100644
index 0000000..fe3d793
--- /dev/null
+++ b/arch/arm/include/asm/mmu.h
@@ -0,0 +1,8 @@
+#ifndef __ASM_ARM_MMU_H
+#define __ASM_ARM_MMU_H
+
+#ifdef CONFIG_ADDR_MAP
+extern void init_addr_map(void);
+#endif
+
+#endif
diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
index 81ccead..a513f4a 100644
--- a/arch/arm/include/asm/system.h
+++ b/arch/arm/include/asm/system.h
@@ -573,14 +573,28 @@ s32 psci_features(u32 function_id, u32 psci_fid);
 void save_boot_params_ret(void);
 
 /**
+ * Change the virt/phys mapping and cache settings for a region.
+ *
+ * \param virt		virtual start address of memory region to change
+ * \param phys		physical address for the memory region to set
+ * \param size		size of memory region to change
+ * \param option	dcache option to select
+ */
+void mmu_set_region_dcache_behaviour_phys(phys_addr_t virt, phys_addr_t phys,
+					size_t size, enum dcache_option option);
+
+/**
  * Change the cache settings for a region.
  *
  * \param start		start address of memory region to change
  * \param size		size of memory region to change
  * \param option	dcache option to select
  */
-void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size,
-				     enum dcache_option option);
+static inline void mmu_set_region_dcache_behaviour(phys_addr_t start,
+			size_t size, enum dcache_option option)
+{
+	mmu_set_region_dcache_behaviour_phys(start, start, size, option);
+}
 
 #ifdef CONFIG_SYS_NONCACHED_MEMORY
 void noncached_init(void);
diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
index f8d2096..7c14d1d 100644
--- a/arch/arm/lib/cache-cp15.c
+++ b/arch/arm/lib/cache-cp15.c
@@ -24,7 +24,8 @@ __weak void arm_init_domains(void)
 {
 }
 
-void set_section_dcache(int section, enum dcache_option option)
+static void set_section_phys(int section, phys_addr_t phys,
+			     enum dcache_option option)
 {
 #ifdef CONFIG_ARMV7_LPAE
 	u64 *page_table = (u64 *)gd->arch.tlb_addr;
@@ -36,7 +37,7 @@ void set_section_dcache(int section, enum dcache_option option)
 #endif
 
 	/* Add the page offset */
-	value |= ((u32)section << MMU_SECTION_SHIFT);
+	value |= phys;
 
 	/* Add caching bits */
 	value |= option;
@@ -45,13 +46,18 @@ void set_section_dcache(int section, enum dcache_option option)
 	page_table[section] = value;
 }
 
+void set_section_dcache(int section, enum dcache_option option)
+{
+	set_section_phys(section, (u32)section << MMU_SECTION_SHIFT, option);
+}
+
 __weak void mmu_page_table_flush(unsigned long start, unsigned long stop)
 {
 	debug("%s: Warning: not implemented\n", __func__);
 }
 
-void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size,
-				     enum dcache_option option)
+void mmu_set_region_dcache_behaviour_phys(phys_addr_t start, phys_addr_t phys,
+					size_t size, enum dcache_option option)
 {
 #ifdef CONFIG_ARMV7_LPAE
 	u64 *page_table = (u64 *)gd->arch.tlb_addr;
@@ -70,8 +76,8 @@ void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size,
 	debug("%s: start=%pa, size=%zu, option=0x%x\n", __func__, &start, size,
 	      option);
 #endif
-	for (upto = start; upto < end; upto++)
-		set_section_dcache(upto, option);
+	for (upto = start; upto < end; upto++, phys += MMU_SECTION_SIZE)
+		set_section_phys(upto, phys, option);
 
 	/*
 	 * Make sure range is cache line aligned
-- 
1.9.1

[RFC PATCH 1/2] arm: provide a function for boards init code to modify MMU virtual-physical map

[Sylwester Nawrocki]

On 12.05.2020 15:17, Marek Szyprowski wrote:
> -void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size,
> -				     enum dcache_option option);
> +static inline void mmu_set_region_dcache_behaviour(phys_addr_t start,
> +			size_t size, enum dcache_option option)

aarch64 build fails with an error:

arch/arm/cpu/armv8/cache_v8.c:555:6: error: redefinition of ?mmu_set_region_dcache_behaviour?
 void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size,
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ./arch/arm/include/asm/cache.h:11:0,
                 from include/net.h:15,
                 from include/common.h:41,
                 from arch/arm/cpu/armv8/cache_v8.c:10:
./arch/arm/include/asm/system.h:593:20: note: previous definition of ?mmu_set_region_dcache_behaviour? was here
 static inline void mmu_set_region_dcache_behaviour(phys_addr_t start,
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  CC      common/exports.o
scripts/Makefile.build:265: recipe for target 'arch/arm/cpu/armv8/cache_v8.o' failed
make[1]: *** [arch/arm/cpu/armv8/cache_v8.o] Error 1
Makefile:1790: recipe for target 'arch/arm/cpu/armv8' failed
make: *** [arch/arm/cpu/armv8] Error 2

-- 
Thanks,
Sylwester

[RFC PATCH 2/2] rpi4: add a mapping for the PCIe XHCI controller MMIO registers (ARM 32bit)

[Marek Szyprowski]

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. Due to 32bit limit in the CPU virtual address space in ARM
32bit mode, this region is mapped at 0xff800000 CPU virtual address.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/mach-bcm283x/Kconfig             |  1 +
 arch/arm/mach-bcm283x/include/mach/base.h |  6 ++++++
 arch/arm/mach-bcm283x/init.c              | 14 ++++++++++++++
 include/configs/rpi.h                     |  5 +++++
 4 files changed, 26 insertions(+)

diff --git a/arch/arm/mach-bcm283x/Kconfig b/arch/arm/mach-bcm283x/Kconfig
index 00419bf..bcb7f1d 100644
--- a/arch/arm/mach-bcm283x/Kconfig
+++ b/arch/arm/mach-bcm283x/Kconfig
@@ -36,6 +36,7 @@ config BCM2711_32B
 	select BCM2711
 	select ARMV7_LPAE
 	select CPU_V7A
+	select PHYS_64BIT
 
 config BCM2711_64B
 	bool "Broadcom BCM2711 SoC 64-bit support"
diff --git a/arch/arm/mach-bcm283x/include/mach/base.h b/arch/arm/mach-bcm283x/include/mach/base.h
index c4ae398..1bf89db 100644
--- a/arch/arm/mach-bcm283x/include/mach/base.h
+++ b/arch/arm/mach-bcm283x/include/mach/base.h
@@ -8,4 +8,10 @@
 
 extern unsigned long rpi_bcm283x_base;
 
+#ifdef CONFIG_ARMV7_LPAE
+#include <addr_map.h>
+#define phys_to_virt addrmap_phys_to_virt
+#define virt_to_phys addrmap_virt_to_phys
+#endif
+
 #endif
diff --git a/arch/arm/mach-bcm283x/init.c b/arch/arm/mach-bcm283x/init.c
index 6a748da..4b9c831 100644
--- a/arch/arm/mach-bcm283x/init.c
+++ b/arch/arm/mach-bcm283x/init.c
@@ -145,6 +145,20 @@ int mach_cpu_init(void)
 }
 
 #ifdef CONFIG_ARMV7_LPAE
+#define BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT	0xff800000UL
+#include <addr_map.h>
+
+void init_addr_map(void)
+{
+	mmu_set_region_dcache_behaviour_phys(BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT,
+					     BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS,
+					     BCM2711_RPI4_PCIE_XHCI_MMIO_SIZE,
+					     DCACHE_OFF);
+	addrmap_set_entry(BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT,
+			  BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS,
+			  BCM2711_RPI4_PCIE_XHCI_MMIO_SIZE, 0);
+}
+
 void enable_caches(void)
 {
 	dcache_enable();
diff --git a/include/configs/rpi.h b/include/configs/rpi.h
index b53a4b6..7da2cff 100644
--- a/include/configs/rpi.h
+++ b/include/configs/rpi.h
@@ -63,6 +63,11 @@
 #define CONFIG_SYS_BOOTM_LEN		SZ_64M
 #endif
 
+#ifdef CONFIG_ARMV7_LPAE
+#define CONFIG_ADDR_MAP 1
+#define CONFIG_SYS_NUM_ADDR_MAP 2
+#endif
+
 /* Devices */
 /* GPIO */
 #define CONFIG_BCM2835_GPIO
-- 
1.9.1

[RFC PATCH v2 0/2] ARM: arbitrary virtual-physical mappings for RPi4 XHCI support

[Marek Szyprowski]

Hi All,

This is a result of the following discussion:
https://lists.denx.de/pipermail/u-boot/2020-May/411086.html

Those 2 patches are replacement for the patch discussed there.
V2 fixes ARM64 build.

Best regards
Marek Szyprowski
Samsung R&D Institute Poland


Marek Szyprowski (2):
  arm: provide a function for boards init code to modify MMU
    virtual-physical map
  rpi4: add a mapping for the PCIe XHCI controller MMIO registers (ARM
    32bit)

 arch/arm/include/asm/mmu.h                |  8 ++++++++
 arch/arm/include/asm/system.h             | 11 +++++++++++
 arch/arm/lib/cache-cp15.c                 | 24 ++++++++++++++++++------
 arch/arm/mach-bcm283x/Kconfig             |  1 +
 arch/arm/mach-bcm283x/include/mach/base.h |  6 ++++++
 arch/arm/mach-bcm283x/init.c              | 14 ++++++++++++++
 include/configs/rpi.h                     |  5 +++++
 7 files changed, 63 insertions(+), 6 deletions(-)
 create mode 100644 arch/arm/include/asm/mmu.h

-- 
1.9.1

[RFC PATCH v2 1/2] arm: provide a function for boards init code to modify MMU virtual-physical map

[Marek Szyprowski]

Provide a function for setting arbitrary virtual-physical MMU mapping
for the given region.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/include/asm/mmu.h    |  8 ++++++++
 arch/arm/include/asm/system.h | 11 +++++++++++
 arch/arm/lib/cache-cp15.c     | 24 ++++++++++++++++++------
 3 files changed, 37 insertions(+), 6 deletions(-)
 create mode 100644 arch/arm/include/asm/mmu.h

diff --git a/arch/arm/include/asm/mmu.h b/arch/arm/include/asm/mmu.h
new file mode 100644
index 0000000..fe3d793
--- /dev/null
+++ b/arch/arm/include/asm/mmu.h
@@ -0,0 +1,8 @@
+#ifndef __ASM_ARM_MMU_H
+#define __ASM_ARM_MMU_H
+
+#ifdef CONFIG_ADDR_MAP
+extern void init_addr_map(void);
+#endif
+
+#endif
diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
index 81ccead..5b9f31c 100644
--- a/arch/arm/include/asm/system.h
+++ b/arch/arm/include/asm/system.h
@@ -573,6 +573,17 @@ s32 psci_features(u32 function_id, u32 psci_fid);
 void save_boot_params_ret(void);
 
 /**
+ * Change the virt/phys mapping and cache settings for a region.
+ *
+ * \param virt		virtual start address of memory region to change
+ * \param phys		physical address for the memory region to set
+ * \param size		size of memory region to change
+ * \param option	dcache option to select
+ */
+void mmu_set_region_dcache_behaviour_phys(phys_addr_t virt, phys_addr_t phys,
+					size_t size, enum dcache_option option);
+
+/**
  * Change the cache settings for a region.
  *
  * \param start		start address of memory region to change
diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
index f8d2096..84ddad3 100644
--- a/arch/arm/lib/cache-cp15.c
+++ b/arch/arm/lib/cache-cp15.c
@@ -24,7 +24,8 @@ __weak void arm_init_domains(void)
 {
 }
 
-void set_section_dcache(int section, enum dcache_option option)
+static void set_section_phys(int section, phys_addr_t phys,
+			     enum dcache_option option)
 {
 #ifdef CONFIG_ARMV7_LPAE
 	u64 *page_table = (u64 *)gd->arch.tlb_addr;
@@ -36,7 +37,7 @@ void set_section_dcache(int section, enum dcache_option option)
 #endif
 
 	/* Add the page offset */
-	value |= ((u32)section << MMU_SECTION_SHIFT);
+	value |= phys;
 
 	/* Add caching bits */
 	value |= option;
@@ -45,13 +46,18 @@ void set_section_dcache(int section, enum dcache_option option)
 	page_table[section] = value;
 }
 
+void set_section_dcache(int section, enum dcache_option option)
+{
+	set_section_phys(section, (u32)section << MMU_SECTION_SHIFT, option);
+}
+
 __weak void mmu_page_table_flush(unsigned long start, unsigned long stop)
 {
 	debug("%s: Warning: not implemented\n", __func__);
 }
 
-void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size,
-				     enum dcache_option option)
+void mmu_set_region_dcache_behaviour_phys(phys_addr_t start, phys_addr_t phys,
+					size_t size, enum dcache_option option)
 {
 #ifdef CONFIG_ARMV7_LPAE
 	u64 *page_table = (u64 *)gd->arch.tlb_addr;
@@ -70,8 +76,8 @@ void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size,
 	debug("%s: start=%pa, size=%zu, option=0x%x\n", __func__, &start, size,
 	      option);
 #endif
-	for (upto = start; upto < end; upto++)
-		set_section_dcache(upto, option);
+	for (upto = start; upto < end; upto++, phys += MMU_SECTION_SIZE)
+		set_section_phys(upto, phys, option);
 
 	/*
 	 * Make sure range is cache line aligned
@@ -86,6 +92,12 @@ void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size,
 	mmu_page_table_flush(startpt, stoppt);
 }
 
+void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size,
+				     enum dcache_option option)
+{
+	mmu_set_region_dcache_behaviour_phys(start, start, size, option);
+}
+
 __weak void dram_bank_mmu_setup(int bank)
 {
 	bd_t *bd = gd->bd;
-- 
1.9.1

[RFC PATCH v2 2/2] rpi4: add a mapping for the PCIe XHCI controller MMIO registers (ARM 32bit)

[Marek Szyprowski]

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. Due to 32bit limit in the CPU virtual address space in ARM
32bit mode, this region is mapped at 0xff800000 CPU virtual address.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/mach-bcm283x/Kconfig             |  1 +
 arch/arm/mach-bcm283x/include/mach/base.h |  6 ++++++
 arch/arm/mach-bcm283x/init.c              | 14 ++++++++++++++
 include/configs/rpi.h                     |  5 +++++
 4 files changed, 26 insertions(+)

diff --git a/arch/arm/mach-bcm283x/Kconfig b/arch/arm/mach-bcm283x/Kconfig
index 00419bf..bcb7f1d 100644
--- a/arch/arm/mach-bcm283x/Kconfig
+++ b/arch/arm/mach-bcm283x/Kconfig
@@ -36,6 +36,7 @@ config BCM2711_32B
 	select BCM2711
 	select ARMV7_LPAE
 	select CPU_V7A
+	select PHYS_64BIT
 
 config BCM2711_64B
 	bool "Broadcom BCM2711 SoC 64-bit support"
diff --git a/arch/arm/mach-bcm283x/include/mach/base.h b/arch/arm/mach-bcm283x/include/mach/base.h
index c4ae398..1bf89db 100644
--- a/arch/arm/mach-bcm283x/include/mach/base.h
+++ b/arch/arm/mach-bcm283x/include/mach/base.h
@@ -8,4 +8,10 @@
 
 extern unsigned long rpi_bcm283x_base;
 
+#ifdef CONFIG_ARMV7_LPAE
+#include <addr_map.h>
+#define phys_to_virt addrmap_phys_to_virt
+#define virt_to_phys addrmap_virt_to_phys
+#endif
+
 #endif
diff --git a/arch/arm/mach-bcm283x/init.c b/arch/arm/mach-bcm283x/init.c
index 6a748da..4b9c831 100644
--- a/arch/arm/mach-bcm283x/init.c
+++ b/arch/arm/mach-bcm283x/init.c
@@ -145,6 +145,20 @@ int mach_cpu_init(void)
 }
 
 #ifdef CONFIG_ARMV7_LPAE
+#define BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT	0xff800000UL
+#include <addr_map.h>
+
+void init_addr_map(void)
+{
+	mmu_set_region_dcache_behaviour_phys(BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT,
+					     BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS,
+					     BCM2711_RPI4_PCIE_XHCI_MMIO_SIZE,
+					     DCACHE_OFF);
+	addrmap_set_entry(BCM2711_RPI4_PCIE_XHCI_MMIO_VIRT,
+			  BCM2711_RPI4_PCIE_XHCI_MMIO_PHYS,
+			  BCM2711_RPI4_PCIE_XHCI_MMIO_SIZE, 0);
+}
+
 void enable_caches(void)
 {
 	dcache_enable();
diff --git a/include/configs/rpi.h b/include/configs/rpi.h
index b53a4b6..7da2cff 100644
--- a/include/configs/rpi.h
+++ b/include/configs/rpi.h
@@ -63,6 +63,11 @@
 #define CONFIG_SYS_BOOTM_LEN		SZ_64M
 #endif
 
+#ifdef CONFIG_ARMV7_LPAE
+#define CONFIG_ADDR_MAP 1
+#define CONFIG_SYS_NUM_ADDR_MAP 2
+#endif
+
 /* Devices */
 /* GPIO */
 #define CONFIG_BCM2835_GPIO
-- 
1.9.1

[PATCH v2 07/10] 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 v1:
 - added empty lines between functions and macros.

Changes since RFC:
 - new patch.
---
 include/linux/bitfield.h | 50 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
index 8b9d6ff..7acba4c 100644
--- a/include/linux/bitfield.h
+++ b/include/linux/bitfield.h
@@ -103,4 +103,54 @@
 		(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 v2 07/10] linux/bitfield.h: Add primitives for manipulating bitfields both in host- and fixed-endian

[Bin Meng]

On Mon, May 4, 2020 at 8:45 PM Sylwester Nawrocki
<s.nawrocki@samsung.com> 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>
> ---
> Changes since v1:
>  - added empty lines between functions and macros.
>
> Changes since RFC:
>  - new patch.
> ---
>  include/linux/bitfield.h | 50 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[PATCH v2 08/10] 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 v1:
 - none.
Changes since RFC:
 - ensure the entries are added in order, sorted by ascending
   address values.
---
 include/pci.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/pci.h b/include/pci.h
index dfdbb32..66676c0 100644
--- a/include/pci.h
+++ b/include/pci.h
@@ -479,11 +479,17 @@
 #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_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 v2 09/10] pci: Add driver for Broadcom STB PCIe controller

[Sylwester Nawrocki]

This patch adds basic driver for the Broadcom STB PCIe host controller.
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 4GB 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 v1:
 - fixed argument in brcm_pcie_set_ssc() function call
 - changed rc_bar2_size assignment to value 0xC0000000, as in upstream
   devicetre
Changes since RFC:
 - reworked to align with current Linux mainline version and u-boot
   driver by Nicolas Saenz Julienne

brcmstb pcie
---
 drivers/pci/Kconfig        |   6 +
 drivers/pci/Makefile       |   1 +
 drivers/pci/pcie_brcmstb.c | 594 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 601 insertions(+)
 create mode 100644 drivers/pci/pcie_brcmstb.c

diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index 437cd9a..056a021 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -197,4 +197,10 @@ 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 Broadcom STB PCIe controller support.
 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..c6ddf92
--- /dev/null
+++ b/drivers/pci/pcie_brcmstb.c
@@ -0,0 +1,594 @@
+// 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 <asm/io.h>
+#include <common.h>
+#include <dm.h>
+#include <dm/ofnode.h>
+#include <errno.h>
+#include <linux/bitfield.h>
+#include <linux/log2.h>
+#include <pci.h>
+
+/* BRCM_PCIE_CAP_REGS - Offset for the mandatory capability config regs */
+#define BRCM_PCIE_CAP_REGS				0x00ac
+
+/* Broadcom STB PCIe Register Offsets */
+#define PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1				0x0188
+#define  PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1_ENDIAN_MODE_BAR2_MASK	0xc
+#define  PCIE_RC_CFG_VENDOR_SPCIFIC_REG1_LITTLE_ENDIAN			0x0
+
+#define PCIE_RC_CFG_PRIV1_ID_VAL3			0x043c
+#define  PCIE_RC_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  PCIE_MISC_MISC_CTRL_SCB_ACCESS_EN_MASK		0x1000
+#define  PCIE_MISC_MISC_CTRL_CFG_READ_UR_MODE_MASK	0x2000
+#define  PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK	0x300000
+#define  PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_128		0x0
+#define  PCIE_MISC_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  PCIE_MISC_RC_BAR1_CONFIG_LO_SIZE_MASK		0x1f
+
+#define PCIE_MISC_RC_BAR2_CONFIG_LO			0x4034
+#define  PCIE_MISC_RC_BAR2_CONFIG_LO_SIZE_MASK		0x1f
+#define PCIE_MISC_RC_BAR2_CONFIG_HI			0x4038
+
+#define PCIE_MISC_RC_BAR3_CONFIG_LO			0x403c
+#define  PCIE_MISC_RC_BAR3_CONFIG_LO_SIZE_MASK		0x1f
+
+#define PCIE_MISC_PCIE_STATUS				0x4068
+#define  PCIE_MISC_PCIE_STATUS_PCIE_PORT_MASK		0x80
+#define  PCIE_MISC_PCIE_STATUS_PCIE_DL_ACTIVE_MASK	0x20
+#define  PCIE_MISC_PCIE_STATUS_PCIE_PHYLINKUP_MASK	0x10
+#define  PCIE_MISC_PCIE_STATUS_PCIE_LINK_IN_L23_MASK	0x40
+
+#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT		 0x4070
+#define  PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_LIMIT_MASK	 0xfff00000
+#define  PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_BASE_MASK	 0xfff0
+#define  PCIE_MISC_CPU_2_PCIE_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  PCIE_MISC_CPU_2_PCIE_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_MISC_CPU_2_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_MISC_HARD_PCIE_HARD_DEBUG_CLKREQ_DEBUG_ENABLE_MASK 0x2
+#define  PCIE_MISC_HARD_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  PCIE_RGR1_SW_INIT_1_PERST_MASK			0x1
+#define  PCIE_RGR1_SW_INIT_1_INIT_MASK			0x2
+
+/* PCIe parameters */
+#define BRCM_NUM_PCIE_OUT_WINS		0x4
+
+/* MDIO registers */
+#define MDIO_PORT0			0x0
+#define MDIO_DATA_MASK			0x7fffffff
+#define MDIO_PORT_MASK			0xf0000
+#define MDIO_REGAD_MASK			0xffff
+#define MDIO_CMD_MASK			0xfff00000
+#define MDIO_CMD_READ			0x1
+#define MDIO_CMD_WRITE			0x0
+#define MDIO_DATA_DONE_MASK		0x80000000
+#define MDIO_RD_DONE(x)			(((x) & MDIO_DATA_DONE_MASK) ? 1 : 0)
+#define MDIO_WT_DONE(x)			(((x) & MDIO_DATA_DONE_MASK) ? 0 : 1)
+#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_PLL_LOCK_MASK	0x800
+
+struct brcm_pcie {
+	void __iomem		*base;
+
+	int			gen;
+	bool			ssc;
+};
+
+#define msleep(a) udelay((a) * 1000)
+
+/*
+ * This is to convert the size of the inbound "BAR" region to the
+ * non-linear values of PCIE_X_MISC_RC_BAR[123]_CONFIG_LO.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;
+}
+
+/* Configuration space read/write support */
+static inline int brcm_pcie_cfg_index(pci_dev_t bdf, int reg)
+{
+	return (PCI_DEV(bdf) << PCIE_EXT_SLOT_SHIFT)
+		| (PCI_FUNC(bdf) << PCIE_EXT_FUNC_SHIFT)
+		| (PCI_BUS(bdf) << PCIE_EXT_BUSNUM_SHIFT)
+		| (reg & ~3);
+}
+
+/* The controller is capable of serving in both RC and EP roles */
+static bool brcm_pcie_rc_mode(struct brcm_pcie *pcie)
+{
+	u32 val = readl(pcie->base + PCIE_MISC_PCIE_STATUS);
+
+	return !!FIELD_GET(PCIE_MISC_PCIE_STATUS_PCIE_PORT_MASK, val);
+}
+
+static bool brcm_pcie_link_up(struct brcm_pcie *pcie)
+{
+	u32 val = readl(pcie->base + PCIE_MISC_PCIE_STATUS);
+	u32 dla = FIELD_GET(PCIE_MISC_PCIE_STATUS_PCIE_DL_ACTIVE_MASK, val);
+	u32 plu = FIELD_GET(PCIE_MISC_PCIE_STATUS_PCIE_PHYLINKUP_MASK, val);
+
+	return dla && plu;
+}
+
+static int brcm_pcie_config_address(const struct udevice *udev, pci_dev_t bdf,
+				    uint offset, void **paddress)
+{
+	struct brcm_pcie *pcie = dev_get_priv(udev);
+	unsigned int bus = PCI_BUS(bdf);
+	unsigned int dev = PCI_DEV(bdf);
+	int idx;
+
+	/*
+	 * Busses 0 (host PCIe bridge) and 1 (its immediate child)
+	 * are limited to a single device each
+	 */
+	if ((bus == (udev->seq + 1)) && dev > 0)
+		return -ENODEV;
+
+	/* Accesses to the RC go right to the RC registers if PCI device == 0 */
+	if (bus == udev->seq) {
+		if (PCI_DEV(bdf))
+			return -ENODEV;
+
+		*paddress = pcie->base + offset;
+		return 0;
+	}
+
+	/* For devices, write to the config space index register */
+	idx = brcm_pcie_cfg_index(bdf, 0);
+
+	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 s)
+{
+	static const char * const speed_str[] = { "??", "2.5", "5.0", "8.0" };
+
+	if (s >= ARRAY_SIZE(speed_str))
+		s = 0;
+
+	return speed_str[s];
+}
+
+static inline void brcm_pcie_bridge_sw_init_set(struct brcm_pcie *pcie, u32 val)
+{
+	u32 tmp;
+
+	tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1);
+	u32p_replace_bits(&tmp, val, PCIE_RGR1_SW_INIT_1_INIT_MASK);
+	writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1);
+}
+
+static inline void brcm_pcie_perst_set(struct brcm_pcie *pcie, u32 val)
+{
+	u32 tmp;
+
+	tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1);
+	u32p_replace_bits(&tmp, val, PCIE_RGR1_SW_INIT_1_PERST_MASK);
+	writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1);
+}
+
+static u32 brcm_pcie_mdio_form_pkt(int port, int regad, int cmd)
+{
+	u32 pkt = 0;
+
+	pkt |= FIELD_PREP(MDIO_PORT_MASK, port);
+	pkt |= FIELD_PREP(MDIO_REGAD_MASK, regad);
+	pkt |= FIELD_PREP(MDIO_CMD_MASK, cmd);
+
+	return pkt;
+}
+
+/* Negative return value indicates error */
+static int brcm_pcie_mdio_read(void __iomem *base, u8 port, u8 regad, u32 *val)
+{
+	int tries;
+	u32 data;
+
+	writel(brcm_pcie_mdio_form_pkt(port, regad, MDIO_CMD_READ),
+	       base + PCIE_RC_DL_MDIO_ADDR);
+	readl(base + PCIE_RC_DL_MDIO_ADDR);
+
+	data = readl(base + PCIE_RC_DL_MDIO_RD_DATA);
+	for (tries = 0; !MDIO_RD_DONE(data) && tries < 10; tries++) {
+		udelay(10);
+		data = readl(base + PCIE_RC_DL_MDIO_RD_DATA);
+	}
+
+	*val = FIELD_GET(MDIO_DATA_MASK, data);
+	return MDIO_RD_DONE(data) ? 0 : -EIO;
+}
+
+/* Negative return value indicates error */
+static int brcm_pcie_mdio_write(void __iomem *base, u8 port,
+				u8 regad, u16 wrdata)
+{
+	int tries;
+	u32 data;
+
+	writel(brcm_pcie_mdio_form_pkt(port, regad, MDIO_CMD_WRITE),
+	       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);
+
+	data = readl(base + PCIE_RC_DL_MDIO_WR_DATA);
+	for (tries = 0; !MDIO_WT_DONE(data) && tries < 10; tries++) {
+		udelay(10);
+		data = readl(base + PCIE_RC_DL_MDIO_WR_DATA);
+	}
+
+	return MDIO_WT_DONE(data) ? 0 : -EIO;
+}
+
+/*
+ * Configures device for Spread Spectrum Clocking (SSC) mode; negative
+ * return value indicates error.
+ */
+static int brcm_pcie_set_ssc(struct brcm_pcie *pcie)
+{
+	void __iomem *base = pcie->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;
+
+	u32p_replace_bits(&tmp, 1, SSC_CNTL_OVRD_EN_MASK);
+	u32p_replace_bits(&tmp, 1, 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 = FIELD_GET(SSC_STATUS_SSC_MASK, tmp);
+	pll = FIELD_GET(SSC_STATUS_PLL_LOCK_MASK, tmp);
+
+	return ssc && pll ? 0 : -EIO;
+}
+
+/* Limits operation to a specific generation (1, 2, or 3) */
+static void brcm_pcie_set_gen(struct brcm_pcie *pcie, int gen)
+{
+	void __iomem *base = pcie->base;
+
+	u16 lnkctl2 = readw(base + BRCM_PCIE_CAP_REGS + PCI_EXP_LNKCTL2);
+	u32 lnkcap = readl(base + BRCM_PCIE_CAP_REGS + PCI_EXP_LNKCAP);
+
+	lnkcap = (lnkcap & ~PCI_EXP_LNKCAP_SLS) | gen;
+	writel(lnkcap, base + BRCM_PCIE_CAP_REGS + PCI_EXP_LNKCAP);
+
+	lnkctl2 = (lnkctl2 & ~0xf) | gen;
+	writew(lnkctl2, base + BRCM_PCIE_CAP_REGS + 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,
+			  PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_BASE_MASK);
+	u32p_replace_bits(&tmp, limit_addr_mb,
+			  PCIE_MISC_CPU_2_PCIE_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 = PCIE_MISC_CPU_2_PCIE_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,
+			  PCIE_MISC_CPU_2_PCIE_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_MISC_CPU_2_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 */
+	brcm_pcie_bridge_sw_init_set(pcie, 1);
+
+	udelay(150);
+
+	/* Take the bridge out of reset */
+	brcm_pcie_bridge_sw_init_set(pcie, 0);
+
+	tmp = readl(base + PCIE_MISC_HARD_PCIE_HARD_DEBUG);
+	tmp &= ~PCIE_MISC_HARD_PCIE_HARD_DEBUG_SERDES_IDDQ_MASK;
+	writel(tmp, base + PCIE_MISC_HARD_PCIE_HARD_DEBUG);
+	/* Wait for SerDes to be stable */
+	udelay(150);
+
+	/* Set SCB_MAX_BURST_SIZE, CFG_READ_UR_MODE, SCB_ACCESS_EN */
+	u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_SCB_ACCESS_EN_MASK);
+	u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_CFG_READ_UR_MODE_MASK);
+	u32p_replace_bits(&tmp, PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_128,
+			  PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK);
+	writel(tmp, base + PCIE_MISC_MISC_CTRL);
+
+	/*
+	 * 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),
+			  PCIE_MISC_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,
+			  PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK);
+	writel(tmp, base + PCIE_MISC_MISC_CTRL);
+
+	/* Disable the PCIe->GISB memory window (RC_BAR1) */
+	tmp = readl(base + PCIE_MISC_RC_BAR1_CONFIG_LO);
+	tmp &= ~PCIE_MISC_RC_BAR1_CONFIG_LO_SIZE_MASK;
+	writel(tmp, base + PCIE_MISC_RC_BAR1_CONFIG_LO);
+
+	/* Disable the PCIe->SCB memory window (RC_BAR3) */
+	tmp = readl(base + PCIE_MISC_RC_BAR3_CONFIG_LO);
+	tmp &= ~PCIE_MISC_RC_BAR3_CONFIG_LO_SIZE_MASK;
+	writel(tmp, base + PCIE_MISC_RC_BAR3_CONFIG_LO);
+
+	/* 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 */
+	brcm_pcie_perst_set(pcie, 0);
+
+	/* 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)
+		msleep(5);
+
+	if (!brcm_pcie_link_up(pcie)) {
+		printf("PCIe BRCM: link down\n");
+		return -ENODEV;
+	}
+
+	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).
+	 */
+	tmp = readl(base + PCIE_RC_CFG_PRIV1_ID_VAL3);
+	u32p_replace_bits(&tmp, 0x060400,
+			  PCIE_RC_CFG_PRIV1_ID_VAL3_CLASS_CODE_MASK);
+	writel(tmp, base + PCIE_RC_CFG_PRIV1_ID_VAL3);
+
+	if (pcie->ssc) {
+		ret = brcm_pcie_set_ssc(pcie);
+		if (ret == 0)
+			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 = FIELD_GET(PCI_EXP_LNKSTA_CLS, lnksta);
+	nlw = FIELD_GET(PCI_EXP_LNKSTA_NLW, lnksta);
+
+	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 */
+	tmp = readl(base + PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1);
+	u32p_replace_bits(&tmp, PCIE_RC_CFG_VENDOR_SPCIFIC_REG1_LITTLE_ENDIAN,
+		PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1_ENDIAN_MODE_BAR2_MASK);
+	writel(tmp, base + PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1);
+
+	/*
+	 * Refclk from RC should be gated with CLKREQ# input when ASPM L0s,L1
+	 * is enabled => setting the CLKREQ_DEBUG_ENABLE field to 1.
+	 */
+	tmp = readl(base + PCIE_MISC_HARD_PCIE_HARD_DEBUG);
+	tmp |= PCIE_MISC_HARD_PCIE_HARD_DEBUG_CLKREQ_DEBUG_ENABLE_MASK;
+	writel(tmp, base + PCIE_MISC_HARD_PCIE_HARD_DEBUG);
+
+	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 v2 09/10] pci: Add driver for Broadcom STB PCIe controller

[Nicolas Saenz Julienne]

On Mon, 2020-05-04 at 14:45 +0200, Sylwester Nawrocki wrote:
> This patch adds basic driver for the Broadcom STB PCIe host controller.
> 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 4GB 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>

I don't know if it's a little redundant already having a Signed-off-by tag, but
if relevant you can add my:

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

Regards,
Nicolas

> ---
> Changes since v1:
>  - fixed argument in brcm_pcie_set_ssc() function call
>  - changed rc_bar2_size assignment to value 0xC0000000, as in upstream
>    devicetre
> Changes since RFC:
>  - reworked to align with current Linux mainline version and u-boot
>    driver by Nicolas Saenz Julienne
> 
> brcmstb pcie
> ---
>  drivers/pci/Kconfig        |   6 +
>  drivers/pci/Makefile       |   1 +
>  drivers/pci/pcie_brcmstb.c | 594
> +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 601 insertions(+)
>  create mode 100644 drivers/pci/pcie_brcmstb.c
> 
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index 437cd9a..056a021 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -197,4 +197,10 @@ 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 Broadcom STB PCIe controller support.
>  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..c6ddf92
> --- /dev/null
> +++ b/drivers/pci/pcie_brcmstb.c
> @@ -0,0 +1,594 @@
> +// 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 <asm/io.h>
> +#include <common.h>
> +#include <dm.h>
> +#include <dm/ofnode.h>
> +#include <errno.h>
> +#include <linux/bitfield.h>
> +#include <linux/log2.h>
> +#include <pci.h>
> +
> +/* BRCM_PCIE_CAP_REGS - Offset for the mandatory capability config regs */
> +#define BRCM_PCIE_CAP_REGS				0x00ac
> +
> +/* Broadcom STB PCIe Register Offsets */
> +#define PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1				
> 0x0188
> +#define  PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1_ENDIAN_MODE_BAR2_MASK	
> 0xc
> +#define  PCIE_RC_CFG_VENDOR_SPCIFIC_REG1_LITTLE_ENDIAN			
> 0x0
> +
> +#define PCIE_RC_CFG_PRIV1_ID_VAL3			0x043c
> +#define  PCIE_RC_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  PCIE_MISC_MISC_CTRL_SCB_ACCESS_EN_MASK		0x1000
> +#define  PCIE_MISC_MISC_CTRL_CFG_READ_UR_MODE_MASK	0x2000
> +#define  PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK	0x300000
> +#define  PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_128		0x0
> +#define  PCIE_MISC_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  PCIE_MISC_RC_BAR1_CONFIG_LO_SIZE_MASK		0x1f
> +
> +#define PCIE_MISC_RC_BAR2_CONFIG_LO			0x4034
> +#define  PCIE_MISC_RC_BAR2_CONFIG_LO_SIZE_MASK		0x1f
> +#define PCIE_MISC_RC_BAR2_CONFIG_HI			0x4038
> +
> +#define PCIE_MISC_RC_BAR3_CONFIG_LO			0x403c
> +#define  PCIE_MISC_RC_BAR3_CONFIG_LO_SIZE_MASK		0x1f
> +
> +#define PCIE_MISC_PCIE_STATUS				0x4068
> +#define  PCIE_MISC_PCIE_STATUS_PCIE_PORT_MASK		0x80
> +#define  PCIE_MISC_PCIE_STATUS_PCIE_DL_ACTIVE_MASK	0x20
> +#define  PCIE_MISC_PCIE_STATUS_PCIE_PHYLINKUP_MASK	0x10
> +#define  PCIE_MISC_PCIE_STATUS_PCIE_LINK_IN_L23_MASK	0x40
> +
> +#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT		 0x4070
> +#define  PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_LIMIT_MASK	 0xfff00000
> +#define  PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_BASE_MASK	 0xfff0
> +#define  PCIE_MISC_CPU_2_PCIE_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  PCIE_MISC_CPU_2_PCIE_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_MISC_CPU_2_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_MISC_HARD_PCIE_HARD_DEBUG_CLKREQ_DEBUG_ENABLE_MASK 0x2
> +#define  PCIE_MISC_HARD_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  PCIE_RGR1_SW_INIT_1_PERST_MASK			0x1
> +#define  PCIE_RGR1_SW_INIT_1_INIT_MASK			0x2
> +
> +/* PCIe parameters */
> +#define BRCM_NUM_PCIE_OUT_WINS		0x4
> +
> +/* MDIO registers */
> +#define MDIO_PORT0			0x0
> +#define MDIO_DATA_MASK			0x7fffffff
> +#define MDIO_PORT_MASK			0xf0000
> +#define MDIO_REGAD_MASK			0xffff
> +#define MDIO_CMD_MASK			0xfff00000
> +#define MDIO_CMD_READ			0x1
> +#define MDIO_CMD_WRITE			0x0
> +#define MDIO_DATA_DONE_MASK		0x80000000
> +#define MDIO_RD_DONE(x)			(((x) & MDIO_DATA_DONE_MASK) ? 1
> : 0)
> +#define MDIO_WT_DONE(x)			(((x) & MDIO_DATA_DONE_MASK) ? 0
> : 1)
> +#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_PLL_LOCK_MASK	0x800
> +
> +struct brcm_pcie {
> +	void __iomem		*base;
> +
> +	int			gen;
> +	bool			ssc;
> +};
> +
> +#define msleep(a) udelay((a) * 1000)
> +
> +/*
> + * This is to convert the size of the inbound "BAR" region to the
> + * non-linear values of PCIE_X_MISC_RC_BAR[123]_CONFIG_LO.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;
> +}
> +
> +/* Configuration space read/write support */
> +static inline int brcm_pcie_cfg_index(pci_dev_t bdf, int reg)
> +{
> +	return (PCI_DEV(bdf) << PCIE_EXT_SLOT_SHIFT)
> +		| (PCI_FUNC(bdf) << PCIE_EXT_FUNC_SHIFT)
> +		| (PCI_BUS(bdf) << PCIE_EXT_BUSNUM_SHIFT)
> +		| (reg & ~3);
> +}
> +
> +/* The controller is capable of serving in both RC and EP roles */
> +static bool brcm_pcie_rc_mode(struct brcm_pcie *pcie)
> +{
> +	u32 val = readl(pcie->base + PCIE_MISC_PCIE_STATUS);
> +
> +	return !!FIELD_GET(PCIE_MISC_PCIE_STATUS_PCIE_PORT_MASK, val);
> +}
> +
> +static bool brcm_pcie_link_up(struct brcm_pcie *pcie)
> +{
> +	u32 val = readl(pcie->base + PCIE_MISC_PCIE_STATUS);
> +	u32 dla = FIELD_GET(PCIE_MISC_PCIE_STATUS_PCIE_DL_ACTIVE_MASK, val);
> +	u32 plu = FIELD_GET(PCIE_MISC_PCIE_STATUS_PCIE_PHYLINKUP_MASK, val);
> +
> +	return dla && plu;
> +}
> +
> +static int brcm_pcie_config_address(const struct udevice *udev, pci_dev_t
> bdf,
> +				    uint offset, void **paddress)
> +{
> +	struct brcm_pcie *pcie = dev_get_priv(udev);
> +	unsigned int bus = PCI_BUS(bdf);
> +	unsigned int dev = PCI_DEV(bdf);
> +	int idx;
> +
> +	/*
> +	 * Busses 0 (host PCIe bridge) and 1 (its immediate child)
> +	 * are limited to a single device each
> +	 */
> +	if ((bus == (udev->seq + 1)) && dev > 0)
> +		return -ENODEV;
> +
> +	/* Accesses to the RC go right to the RC registers if PCI device == 0 */
> +	if (bus == udev->seq) {
> +		if (PCI_DEV(bdf))
> +			return -ENODEV;
> +
> +		*paddress = pcie->base + offset;
> +		return 0;
> +	}
> +
> +	/* For devices, write to the config space index register */
> +	idx = brcm_pcie_cfg_index(bdf, 0);
> +
> +	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 s)
> +{
> +	static const char * const speed_str[] = { "??", "2.5", "5.0", "8.0" };
> +
> +	if (s >= ARRAY_SIZE(speed_str))
> +		s = 0;
> +
> +	return speed_str[s];
> +}
> +
> +static inline void brcm_pcie_bridge_sw_init_set(struct brcm_pcie *pcie, u32
> val)
> +{
> +	u32 tmp;
> +
> +	tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1);
> +	u32p_replace_bits(&tmp, val, PCIE_RGR1_SW_INIT_1_INIT_MASK);
> +	writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1);
> +}
> +
> +static inline void brcm_pcie_perst_set(struct brcm_pcie *pcie, u32 val)
> +{
> +	u32 tmp;
> +
> +	tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1);
> +	u32p_replace_bits(&tmp, val, PCIE_RGR1_SW_INIT_1_PERST_MASK);
> +	writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1);
> +}
> +
> +static u32 brcm_pcie_mdio_form_pkt(int port, int regad, int cmd)
> +{
> +	u32 pkt = 0;
> +
> +	pkt |= FIELD_PREP(MDIO_PORT_MASK, port);
> +	pkt |= FIELD_PREP(MDIO_REGAD_MASK, regad);
> +	pkt |= FIELD_PREP(MDIO_CMD_MASK, cmd);
> +
> +	return pkt;
> +}
> +
> +/* Negative return value indicates error */
> +static int brcm_pcie_mdio_read(void __iomem *base, u8 port, u8 regad, u32
> *val)
> +{
> +	int tries;
> +	u32 data;
> +
> +	writel(brcm_pcie_mdio_form_pkt(port, regad, MDIO_CMD_READ),
> +	       base + PCIE_RC_DL_MDIO_ADDR);
> +	readl(base + PCIE_RC_DL_MDIO_ADDR);
> +
> +	data = readl(base + PCIE_RC_DL_MDIO_RD_DATA);
> +	for (tries = 0; !MDIO_RD_DONE(data) && tries < 10; tries++) {
> +		udelay(10);
> +		data = readl(base + PCIE_RC_DL_MDIO_RD_DATA);
> +	}
> +
> +	*val = FIELD_GET(MDIO_DATA_MASK, data);
> +	return MDIO_RD_DONE(data) ? 0 : -EIO;
> +}
> +
> +/* Negative return value indicates error */
> +static int brcm_pcie_mdio_write(void __iomem *base, u8 port,
> +				u8 regad, u16 wrdata)
> +{
> +	int tries;
> +	u32 data;
> +
> +	writel(brcm_pcie_mdio_form_pkt(port, regad, MDIO_CMD_WRITE),
> +	       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);
> +
> +	data = readl(base + PCIE_RC_DL_MDIO_WR_DATA);
> +	for (tries = 0; !MDIO_WT_DONE(data) && tries < 10; tries++) {
> +		udelay(10);
> +		data = readl(base + PCIE_RC_DL_MDIO_WR_DATA);
> +	}
> +
> +	return MDIO_WT_DONE(data) ? 0 : -EIO;
> +}
> +
> +/*
> + * Configures device for Spread Spectrum Clocking (SSC) mode; negative
> + * return value indicates error.
> + */
> +static int brcm_pcie_set_ssc(struct brcm_pcie *pcie)
> +{
> +	void __iomem *base = pcie->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;
> +
> +	u32p_replace_bits(&tmp, 1, SSC_CNTL_OVRD_EN_MASK);
> +	u32p_replace_bits(&tmp, 1, 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 = FIELD_GET(SSC_STATUS_SSC_MASK, tmp);
> +	pll = FIELD_GET(SSC_STATUS_PLL_LOCK_MASK, tmp);
> +
> +	return ssc && pll ? 0 : -EIO;
> +}
> +
> +/* Limits operation to a specific generation (1, 2, or 3) */
> +static void brcm_pcie_set_gen(struct brcm_pcie *pcie, int gen)
> +{
> +	void __iomem *base = pcie->base;
> +
> +	u16 lnkctl2 = readw(base + BRCM_PCIE_CAP_REGS + PCI_EXP_LNKCTL2);
> +	u32 lnkcap = readl(base + BRCM_PCIE_CAP_REGS + PCI_EXP_LNKCAP);
> +
> +	lnkcap = (lnkcap & ~PCI_EXP_LNKCAP_SLS) | gen;
> +	writel(lnkcap, base + BRCM_PCIE_CAP_REGS + PCI_EXP_LNKCAP);
> +
> +	lnkctl2 = (lnkctl2 & ~0xf) | gen;
> +	writew(lnkctl2, base + BRCM_PCIE_CAP_REGS + 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,
> +			  PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_BASE_MASK);
> +	u32p_replace_bits(&tmp, limit_addr_mb,
> +			  PCIE_MISC_CPU_2_PCIE_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 =
> PCIE_MISC_CPU_2_PCIE_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,
> +			  PCIE_MISC_CPU_2_PCIE_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_MISC_CPU_2_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 */
> +	brcm_pcie_bridge_sw_init_set(pcie, 1);
> +
> +	udelay(150);
> +
> +	/* Take the bridge out of reset */
> +	brcm_pcie_bridge_sw_init_set(pcie, 0);
> +
> +	tmp = readl(base + PCIE_MISC_HARD_PCIE_HARD_DEBUG);
> +	tmp &= ~PCIE_MISC_HARD_PCIE_HARD_DEBUG_SERDES_IDDQ_MASK;
> +	writel(tmp, base + PCIE_MISC_HARD_PCIE_HARD_DEBUG);
> +	/* Wait for SerDes to be stable */
> +	udelay(150);
> +
> +	/* Set SCB_MAX_BURST_SIZE, CFG_READ_UR_MODE, SCB_ACCESS_EN */
> +	u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_SCB_ACCESS_EN_MASK);
> +	u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_CFG_READ_UR_MODE_MASK);
> +	u32p_replace_bits(&tmp, PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_128,
> +			  PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK);
> +	writel(tmp, base + PCIE_MISC_MISC_CTRL);
> +
> +	/*
> +	 * 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),
> +			  PCIE_MISC_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,
> +			  PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK);
> +	writel(tmp, base + PCIE_MISC_MISC_CTRL);
> +
> +	/* Disable the PCIe->GISB memory window (RC_BAR1) */
> +	tmp = readl(base + PCIE_MISC_RC_BAR1_CONFIG_LO);
> +	tmp &= ~PCIE_MISC_RC_BAR1_CONFIG_LO_SIZE_MASK;
> +	writel(tmp, base + PCIE_MISC_RC_BAR1_CONFIG_LO);
> +
> +	/* Disable the PCIe->SCB memory window (RC_BAR3) */
> +	tmp = readl(base + PCIE_MISC_RC_BAR3_CONFIG_LO);
> +	tmp &= ~PCIE_MISC_RC_BAR3_CONFIG_LO_SIZE_MASK;
> +	writel(tmp, base + PCIE_MISC_RC_BAR3_CONFIG_LO);
> +
> +	/* 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 */
> +	brcm_pcie_perst_set(pcie, 0);
> +
> +	/* 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)
> +		msleep(5);
> +
> +	if (!brcm_pcie_link_up(pcie)) {
> +		printf("PCIe BRCM: link down\n");
> +		return -ENODEV;
> +	}
> +
> +	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).
> +	 */
> +	tmp = readl(base + PCIE_RC_CFG_PRIV1_ID_VAL3);
> +	u32p_replace_bits(&tmp, 0x060400,
> +			  PCIE_RC_CFG_PRIV1_ID_VAL3_CLASS_CODE_MASK);
> +	writel(tmp, base + PCIE_RC_CFG_PRIV1_ID_VAL3);
> +
> +	if (pcie->ssc) {
> +		ret = brcm_pcie_set_ssc(pcie);
> +		if (ret == 0)
> +			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 = FIELD_GET(PCI_EXP_LNKSTA_CLS, lnksta);
> +	nlw = FIELD_GET(PCI_EXP_LNKSTA_NLW, lnksta);
> +
> +	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 */
> +	tmp = readl(base + PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1);
> +	u32p_replace_bits(&tmp, PCIE_RC_CFG_VENDOR_SPCIFIC_REG1_LITTLE_ENDIAN,
> +		PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1_ENDIAN_MODE_BAR2_MASK);
> +	writel(tmp, base + PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1);
> +
> +	/*
> +	 * Refclk from RC should be gated with CLKREQ# input when ASPM L0s,L1
> +	 * is enabled => setting the CLKREQ_DEBUG_ENABLE field to 1.
> +	 */
> +	tmp = readl(base + PCIE_MISC_HARD_PCIE_HARD_DEBUG);
> +	tmp |= PCIE_MISC_HARD_PCIE_HARD_DEBUG_CLKREQ_DEBUG_ENABLE_MASK;
> +	writel(tmp, base + PCIE_MISC_HARD_PCIE_HARD_DEBUG);
> +
> +	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),
> +};

-------------- 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/20200505/8f6ea8f3/attachment.sig>

[PATCH v2 09/10] pci: Add driver for Broadcom STB PCIe controller

[Sylwester Nawrocki]

On 05.05.2020 16:13, Nicolas Saenz Julienne wrote:
> On Mon, 2020-05-04 at 14:45 +0200, Sylwester Nawrocki wrote:
>> This patch adds basic driver for the Broadcom STB PCIe host controller.
>> 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 4GB region is used.
                                                 ^^^^^^
      (I will change that to 3GB in next version)
>> 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>

> I don't know if it's a little redundant already having a Signed-off-by tag, 
> but if relevant you can add my:
> 
> Reviewed-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>

Thanks for your review, I think it doesn't hurt to add the tag,
it would indicate your review of final version.

-- 
Regards,
Sylwester

[PATCH v2 09/10] pci: Add driver for Broadcom STB PCIe controller

[Nicolas Saenz Julienne]

Hi All.

On Mon, 2020-05-04 at 14:45 +0200, Sylwester Nawrocki wrote:
> This patch adds basic driver for the Broadcom STB PCIe host controller.
> 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 4GB 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>
> ---

If it's OK with you, as I'm already familiar with the driver from maintaining
it on the Linux kernel, I wouldn't mind doing it here too.

Sylwester, if you're also interested I can send a patch adding both of us.

Regards,
Nicolas

-------------- 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/20200505/a7ccd2ce/attachment.sig>

[PATCH v2 09/10] pci: Add driver for Broadcom STB PCIe controller

[Sylwester Nawrocki]

Hi Nicolas, 

On 05.05.2020 17:45, Nicolas Saenz Julienne wrote:
> If it's OK with you, as I'm already familiar with the driver from maintaining
> it on the Linux kernel, I wouldn't mind doing it here too.
> 
> Sylwester, if you're also interested I can send a patch adding both of us.

Of course, I'm fine with having you as the maintainer of the driver.
You can add me as well, I'm just not sure if a formal separate MAINTAINERS
entry is needed. It looks like it would be first one with two persons for 
one driver.

-- 
Regards,
Sylwester

[PATCH v2 09/10] pci: Add driver for Broadcom STB PCIe controller

[Simon Glass]

Hi Sylwester,

On Mon, 4 May 2020 at 06:45, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
>
> This patch adds basic driver for the Broadcom STB PCIe host controller.
> 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 4GB 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 v1:
>  - fixed argument in brcm_pcie_set_ssc() function call
>  - changed rc_bar2_size assignment to value 0xC0000000, as in upstream
>    devicetre
> Changes since RFC:
>  - reworked to align with current Linux mainline version and u-boot
>    driver by Nicolas Saenz Julienne
>
> brcmstb pcie
> ---
>  drivers/pci/Kconfig        |   6 +
>  drivers/pci/Makefile       |   1 +
>  drivers/pci/pcie_brcmstb.c | 594 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 601 insertions(+)
>  create mode 100644 drivers/pci/pcie_brcmstb.c

A few small comments.

>
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index 437cd9a..056a021 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -197,4 +197,10 @@ 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 Broadcom STB PCIe controller support.

What is STB? What features does this support? You should get a warning
here to write at least three lines.

>  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..c6ddf92
> --- /dev/null
> +++ b/drivers/pci/pcie_brcmstb.c
> @@ -0,0 +1,594 @@
> +// 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 <asm/io.h>
> +#include <common.h>
> +#include <dm.h>
> +#include <dm/ofnode.h>
> +#include <errno.h>
> +#include <linux/bitfield.h>
> +#include <linux/log2.h>
> +#include <pci.h>

Check ordering of include files:

https://www.denx.de/wiki/U-Boot/CodingStyle

> +
> +/* BRCM_PCIE_CAP_REGS - Offset for the mandatory capability config regs */
> +#define BRCM_PCIE_CAP_REGS                             0x00ac
> +
> +/* Broadcom STB PCIe Register Offsets */
> +#define PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1                                0x0188
> +#define  PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1_ENDIAN_MODE_BAR2_MASK 0xc
> +#define  PCIE_RC_CFG_VENDOR_SPCIFIC_REG1_LITTLE_ENDIAN                 0x0
> +
> +#define PCIE_RC_CFG_PRIV1_ID_VAL3                      0x043c
> +#define  PCIE_RC_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  PCIE_MISC_MISC_CTRL_SCB_ACCESS_EN_MASK                0x1000
> +#define  PCIE_MISC_MISC_CTRL_CFG_READ_UR_MODE_MASK     0x2000
> +#define  PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK       0x300000
> +#define  PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_128                0x0
> +#define  PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK            0xf8000000

If you have a _MASK, don't you need a _SHIFT to allow you to read from
the field?

Can you drop the PCIE_MISC prefix? These are very long and local to this file.

> +
> +#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  PCIE_MISC_RC_BAR1_CONFIG_LO_SIZE_MASK         0x1f
> +
> +#define PCIE_MISC_RC_BAR2_CONFIG_LO                    0x4034
> +#define  PCIE_MISC_RC_BAR2_CONFIG_LO_SIZE_MASK         0x1f
> +#define PCIE_MISC_RC_BAR2_CONFIG_HI                    0x4038
> +
> +#define PCIE_MISC_RC_BAR3_CONFIG_LO                    0x403c
> +#define  PCIE_MISC_RC_BAR3_CONFIG_LO_SIZE_MASK         0x1f
> +
> +#define PCIE_MISC_PCIE_STATUS                          0x4068
> +#define  PCIE_MISC_PCIE_STATUS_PCIE_PORT_MASK          0x80
> +#define  PCIE_MISC_PCIE_STATUS_PCIE_DL_ACTIVE_MASK     0x20
> +#define  PCIE_MISC_PCIE_STATUS_PCIE_PHYLINKUP_MASK     0x10
> +#define  PCIE_MISC_PCIE_STATUS_PCIE_LINK_IN_L23_MASK   0x40
> +
> +#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT                0x4070
> +#define  PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_LIMIT_MASK    0xfff00000
> +#define  PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_BASE_MASK     0xfff0
> +#define  PCIE_MISC_CPU_2_PCIE_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  PCIE_MISC_CPU_2_PCIE_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_MISC_CPU_2_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_MISC_HARD_PCIE_HARD_DEBUG_CLKREQ_DEBUG_ENABLE_MASK 0x2
> +#define  PCIE_MISC_HARD_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  PCIE_RGR1_SW_INIT_1_PERST_MASK                        0x1
> +#define  PCIE_RGR1_SW_INIT_1_INIT_MASK                 0x2
> +
> +/* PCIe parameters */
> +#define BRCM_NUM_PCIE_OUT_WINS         0x4
> +
> +/* MDIO registers */
> +#define MDIO_PORT0                     0x0
> +#define MDIO_DATA_MASK                 0x7fffffff
> +#define MDIO_PORT_MASK                 0xf0000
> +#define MDIO_REGAD_MASK                        0xffff
> +#define MDIO_CMD_MASK                  0xfff00000
> +#define MDIO_CMD_READ                  0x1
> +#define MDIO_CMD_WRITE                 0x0
> +#define MDIO_DATA_DONE_MASK            0x80000000
> +#define MDIO_RD_DONE(x)                        (((x) & MDIO_DATA_DONE_MASK) ? 1 : 0)
> +#define MDIO_WT_DONE(x)                        (((x) & MDIO_DATA_DONE_MASK) ? 0 : 1)

Are these two worth it? You can do this in your code:

if (readl(xxx) & MDIO_DATA_DONE_MASK)

If you must use these, then I think true/false are better than 1/0.

> +#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_PLL_LOCK_MASK       0x800
> +
> +struct brcm_pcie {

struct comment

> +       void __iomem            *base;
> +
> +       int                     gen;
> +       bool                    ssc;
> +};
> +
> +#define msleep(a) udelay((a) * 1000)

This is already defined in U-Boot.

> +
> +/*
> + * This is to convert the size of the inbound "BAR" region to the
> + * non-linear values of PCIE_X_MISC_RC_BAR[123]_CONFIG_LO.SIZE

Please use a proper function comments with args and return value.

> + */
> +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 */

blank line always before return at end of function


> +       return 0;
> +}
> +
> +/* Configuration space read/write support */
> +static inline int brcm_pcie_cfg_index(pci_dev_t bdf, int reg)
> +{
> +       return (PCI_DEV(bdf) << PCIE_EXT_SLOT_SHIFT)
> +               | (PCI_FUNC(bdf) << PCIE_EXT_FUNC_SHIFT)
> +               | (PCI_BUS(bdf) << PCIE_EXT_BUSNUM_SHIFT)
> +               | (reg & ~3);
> +}
> +
> +/* The controller is capable of serving in both RC and EP roles */
> +static bool brcm_pcie_rc_mode(struct brcm_pcie *pcie)
> +{
> +       u32 val = readl(pcie->base + PCIE_MISC_PCIE_STATUS);

Consider using a struct for your registers which would reduce the size
of this code.

> +
> +       return !!FIELD_GET(PCIE_MISC_PCIE_STATUS_PCIE_PORT_MASK, val);

What is FIELD_GET? We have this sort of thing rejected in the past.

Here, since you return a bool, the !! should not be necessary.

> +}
> +
> +static bool brcm_pcie_link_up(struct brcm_pcie *pcie)
> +{
> +       u32 val = readl(pcie->base + PCIE_MISC_PCIE_STATUS);
> +       u32 dla = FIELD_GET(PCIE_MISC_PCIE_STATUS_PCIE_DL_ACTIVE_MASK, val);
> +       u32 plu = FIELD_GET(PCIE_MISC_PCIE_STATUS_PCIE_PHYLINKUP_MASK, val);

PCIE_MISC_PCIE_STATUS_PCIE_PHYLINKUP_MASK is way too long.

How about STATUS_PCIE_PHYLINKUP_MASK?

> +
> +       return dla && plu;
> +}
> +
> +static int brcm_pcie_config_address(const struct udevice *udev, pci_dev_t bdf,

Please use 'dev' instead of udev for the device.

> +                                   uint offset, void **paddress)
> +{
> +       struct brcm_pcie *pcie = dev_get_priv(udev);
> +       unsigned int bus = PCI_BUS(bdf);
> +       unsigned int dev = PCI_DEV(bdf);
> +       int idx;
> +
> +       /*
> +        * Busses 0 (host PCIe bridge) and 1 (its immediate child)
> +        * are limited to a single device each
> +        */
> +       if ((bus == (udev->seq + 1)) && dev > 0)
> +               return -ENODEV;
> +
> +       /* Accesses to the RC go right to the RC registers if PCI device == 0 */
> +       if (bus == udev->seq) {
> +               if (PCI_DEV(bdf))
> +                       return -ENODEV;

You can't return that as there is a device. Perhaps ENXIO? Does this
indicate an internal error? If so you could add a comment here as to
how to fix it.

> +
> +               *paddress = pcie->base + offset;
> +               return 0;
> +       }
> +
> +       /* For devices, write to the config space index register */
> +       idx = brcm_pcie_cfg_index(bdf, 0);
> +
> +       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 s)
> +{
> +       static const char * const speed_str[] = { "??", "2.5", "5.0", "8.0" };
> +
> +       if (s >= ARRAY_SIZE(speed_str))
> +               s = 0;
> +
> +       return speed_str[s];
> +}
> +
> +static inline void brcm_pcie_bridge_sw_init_set(struct brcm_pcie *pcie, u32 val)
> +{
> +       u32 tmp;
> +
> +       tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1);
> +       u32p_replace_bits(&tmp, val, PCIE_RGR1_SW_INIT_1_INIT_MASK);
> +       writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1);

See clrsetbits_le32(), etc. Below alos.

> +}
> +
> +static inline void brcm_pcie_perst_set(struct brcm_pcie *pcie, u32 val)
> +{
> +       u32 tmp;
> +
> +       tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1);
> +       u32p_replace_bits(&tmp, val, PCIE_RGR1_SW_INIT_1_PERST_MASK);
> +       writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1);
> +}
> +
> +static u32 brcm_pcie_mdio_form_pkt(int port, int regad, int cmd)
> +{
> +       u32 pkt = 0;
> +
> +       pkt |= FIELD_PREP(MDIO_PORT_MASK, port);
> +       pkt |= FIELD_PREP(MDIO_REGAD_MASK, regad);
> +       pkt |= FIELD_PREP(MDIO_CMD_MASK, cmd);
> +
> +       return pkt;
> +}
> +
> +/* Negative return value indicates error */

This would be incorporated in a full function comment

> +static int brcm_pcie_mdio_read(void __iomem *base, u8 port, u8 regad, u32 *val)
> +{
> +       int tries;
> +       u32 data;
> +
> +       writel(brcm_pcie_mdio_form_pkt(port, regad, MDIO_CMD_READ),
> +              base + PCIE_RC_DL_MDIO_ADDR);
> +       readl(base + PCIE_RC_DL_MDIO_ADDR);
> +
> +       data = readl(base + PCIE_RC_DL_MDIO_RD_DATA);
> +       for (tries = 0; !MDIO_RD_DONE(data) && tries < 10; tries++) {
> +               udelay(10);
> +               data = readl(base + PCIE_RC_DL_MDIO_RD_DATA);
> +       }
> +
> +       *val = FIELD_GET(MDIO_DATA_MASK, data);
> +       return MDIO_RD_DONE(data) ? 0 : -EIO;
> +}
> +
> +/* Negative return value indicates error */
> +static int brcm_pcie_mdio_write(void __iomem *base, u8 port,
> +                               u8 regad, u16 wrdata)
> +{
> +       int tries;
> +       u32 data;
> +
> +       writel(brcm_pcie_mdio_form_pkt(port, regad, MDIO_CMD_WRITE),
> +              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);
> +
> +       data = readl(base + PCIE_RC_DL_MDIO_WR_DATA);
> +       for (tries = 0; !MDIO_WT_DONE(data) && tries < 10; tries++) {
> +               udelay(10);
> +               data = readl(base + PCIE_RC_DL_MDIO_WR_DATA);
> +       }
> +
> +       return MDIO_WT_DONE(data) ? 0 : -EIO;
> +}
> +
> +/*
> + * Configures device for Spread Spectrum Clocking (SSC) mode; negative
> + * return value indicates error.
> + */
> +static int brcm_pcie_set_ssc(struct brcm_pcie *pcie)
> +{
> +       void __iomem *base = pcie->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;
> +
> +       u32p_replace_bits(&tmp, 1, SSC_CNTL_OVRD_EN_MASK);
> +       u32p_replace_bits(&tmp, 1, 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 = FIELD_GET(SSC_STATUS_SSC_MASK, tmp);
> +       pll = FIELD_GET(SSC_STATUS_PLL_LOCK_MASK, tmp);
> +
> +       return ssc && pll ? 0 : -EIO;
> +}
> +
> +/* Limits operation to a specific generation (1, 2, or 3) */
> +static void brcm_pcie_set_gen(struct brcm_pcie *pcie, int gen)
> +{
> +       void __iomem *base = pcie->base;
> +
> +       u16 lnkctl2 = readw(base + BRCM_PCIE_CAP_REGS + PCI_EXP_LNKCTL2);
> +       u32 lnkcap = readl(base + BRCM_PCIE_CAP_REGS + PCI_EXP_LNKCAP);
> +
> +       lnkcap = (lnkcap & ~PCI_EXP_LNKCAP_SLS) | gen;
> +       writel(lnkcap, base + BRCM_PCIE_CAP_REGS + PCI_EXP_LNKCAP);
> +
> +       lnkctl2 = (lnkctl2 & ~0xf) | gen;
> +       writew(lnkctl2, base + BRCM_PCIE_CAP_REGS + 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,
> +                         PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_BASE_MASK);
> +       u32p_replace_bits(&tmp, limit_addr_mb,
> +                         PCIE_MISC_CPU_2_PCIE_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 = PCIE_MISC_CPU_2_PCIE_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,
> +                         PCIE_MISC_CPU_2_PCIE_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_MISC_CPU_2_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 */
> +       brcm_pcie_bridge_sw_init_set(pcie, 1);
> +
> +       udelay(150);

Please add a comment as to how you chose the value, and below.

> +
> +       /* Take the bridge out of reset */
> +       brcm_pcie_bridge_sw_init_set(pcie, 0);
> +
> +       tmp = readl(base + PCIE_MISC_HARD_PCIE_HARD_DEBUG);
> +       tmp &= ~PCIE_MISC_HARD_PCIE_HARD_DEBUG_SERDES_IDDQ_MASK;
> +       writel(tmp, base + PCIE_MISC_HARD_PCIE_HARD_DEBUG);
> +       /* Wait for SerDes to be stable */
> +       udelay(150);
> +
> +       /* Set SCB_MAX_BURST_SIZE, CFG_READ_UR_MODE, SCB_ACCESS_EN */
> +       u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_SCB_ACCESS_EN_MASK);
> +       u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_CFG_READ_UR_MODE_MASK);
> +       u32p_replace_bits(&tmp, PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_128,
> +                         PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK);
> +       writel(tmp, base + PCIE_MISC_MISC_CTRL);
> +
> +       /*
> +        * 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),
> +                         PCIE_MISC_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,
> +                         PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK);
> +       writel(tmp, base + PCIE_MISC_MISC_CTRL);
> +
> +       /* Disable the PCIe->GISB memory window (RC_BAR1) */
> +       tmp = readl(base + PCIE_MISC_RC_BAR1_CONFIG_LO);
> +       tmp &= ~PCIE_MISC_RC_BAR1_CONFIG_LO_SIZE_MASK;
> +       writel(tmp, base + PCIE_MISC_RC_BAR1_CONFIG_LO);
> +
> +       /* Disable the PCIe->SCB memory window (RC_BAR3) */
> +       tmp = readl(base + PCIE_MISC_RC_BAR3_CONFIG_LO);
> +       tmp &= ~PCIE_MISC_RC_BAR3_CONFIG_LO_SIZE_MASK;
> +       writel(tmp, base + PCIE_MISC_RC_BAR3_CONFIG_LO);
> +
> +       /* 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 */
> +       brcm_pcie_perst_set(pcie, 0);
> +
> +       /* 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)
> +               msleep(5);
> +
> +       if (!brcm_pcie_link_up(pcie)) {
> +               printf("PCIe BRCM: link down\n");
> +               return -ENODEV;

Again, cannot return that error

> +       }
> +
> +       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).
> +        */
> +       tmp = readl(base + PCIE_RC_CFG_PRIV1_ID_VAL3);
> +       u32p_replace_bits(&tmp, 0x060400,
> +                         PCIE_RC_CFG_PRIV1_ID_VAL3_CLASS_CODE_MASK);
> +       writel(tmp, base + PCIE_RC_CFG_PRIV1_ID_VAL3);
> +
> +       if (pcie->ssc) {
> +               ret = brcm_pcie_set_ssc(pcie);
> +               if (ret == 0)
> +                       ssc_good = true;
> +               else
> +                       printf("PCIe BRCM: failed attempt to enter SSC mode\n");

Should this return an error?

> +       }
> +
> +       lnksta = readw(base + BRCM_PCIE_CAP_REGS + PCI_EXP_LNKSTA);
> +       cls = FIELD_GET(PCI_EXP_LNKSTA_CLS, lnksta);
> +       nlw = FIELD_GET(PCI_EXP_LNKSTA_NLW, lnksta);
> +
> +       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 */
> +       tmp = readl(base + PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1);
> +       u32p_replace_bits(&tmp, PCIE_RC_CFG_VENDOR_SPCIFIC_REG1_LITTLE_ENDIAN,
> +               PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1_ENDIAN_MODE_BAR2_MASK);
> +       writel(tmp, base + PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1);
> +
> +       /*
> +        * Refclk from RC should be gated with CLKREQ# input when ASPM L0s,L1
> +        * is enabled => setting the CLKREQ_DEBUG_ENABLE field to 1.
> +        */
> +       tmp = readl(base + PCIE_MISC_HARD_PCIE_HARD_DEBUG);
> +       tmp |= PCIE_MISC_HARD_PCIE_HARD_DEBUG_CLKREQ_DEBUG_ENABLE_MASK;
> +       writel(tmp, base + PCIE_MISC_HARD_PCIE_HARD_DEBUG);
> +
> +       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
>

Regards,
Simon

[PATCH v2 09/10] pci: Add driver for Broadcom STB PCIe controller

[Nicolas Saenz Julienne]

Hi, I'll try to add my two cents.

On Wed, 2020-05-06 at 08:47 -0600, Simon Glass wrote:
> > +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 Broadcom STB PCIe controller
> > support.
> 
> What is STB? What features does this support? You should get a warning
> here to write at least three lines.

STB is short for set top box, which is the wider family of SoCs bcm2711 is part
of[1].

This implementation is, for now, tailor made for bcm2711. The bus not being
exposed, only the relevant bits to enabling xHCI compatibility, which is
hardwired into the RPi4 board, have been tested. For example there is so far no
support for IO memory mappings. Note that some work is already being done on
the Linux version of this driver to incorporate other devices, which will be
easily ported here, as for now, both are pretty much the same (minus interrupt
handling).

[1] https://www.broadcom.com/products/broadband/set-top-box

[...]

> > +#define PCIE_MISC_MISC_CTRL                            0x4008
> > +#define  PCIE_MISC_MISC_CTRL_SCB_ACCESS_EN_MASK                0x1000
> > +#define  PCIE_MISC_MISC_CTRL_CFG_READ_UR_MODE_MASK     0x2000
> > +#define  PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK       0x300000
> > +#define  PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_128                0x0
> > +#define  PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK            0xf8000000
> 
> If you have a _MASK, don't you need a _SHIFT to allow you to read from
> the field?

See patch #7, where we import some bitfield helpers from Linux. The shift
calculation happens there.

> Can you drop the PCIE_MISC prefix? These are very long and local to this file.

This was also discussed when upstreaming this in Linux, Broadcom engineers
asked to keep the naming scheme as is, convoluted as it is, to help with
debugging in the future. I added Jim and Florian in to the conversation in case
they want to clarify something.

[...]

> > +
> > +       return !!FIELD_GET(PCIE_MISC_PCIE_STATUS_PCIE_PORT_MASK, val);
> 
> What is FIELD_GET? We have this sort of thing rejected in the past.

This is also part of the bitfield helpers introduced in patch #7

[...]

> > +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 */
> > +       brcm_pcie_bridge_sw_init_set(pcie, 1);
> > +
> > +       udelay(150);
> 
> Please add a comment as to how you chose the value, and below.

This was picked from Jim Quinlan's original code submission:
https://lkml.org/lkml/2018/9/19/642

Sadly there isn't any comment there.

Regards,
Nicolas

-------------- 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/20200508/8e017400/attachment.sig>

[PATCH v2 09/10] pci: Add driver for Broadcom STB PCIe controller

[Jim Quinlan]

On Fri, May 8, 2020 at 5:50 AM Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> Hi, I'll try to add my two cents.
>
> On Wed, 2020-05-06 at 08:47 -0600, Simon Glass wrote:
> > > +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 Broadcom STB PCIe controller
> > > support.
> >
> > What is STB? What features does this support? You should get a warning
> > here to write at least three lines.
>
> STB is short for set top box, which is the wider family of SoCs bcm2711 is part
> of[1].
>
> This implementation is, for now, tailor made for bcm2711. The bus not being
> exposed, only the relevant bits to enabling xHCI compatibility, which is
> hardwired into the RPi4 board, have been tested. For example there is so far no
> support for IO memory mappings. Note that some work is already being done on
> the Linux version of this driver to incorporate other devices, which will be
> easily ported here, as for now, both are pretty much the same (minus interrupt
> handling).
>
> [1] https://www.broadcom.com/products/broadband/set-top-box
>
> [...]
>
> > > +#define PCIE_MISC_MISC_CTRL                            0x4008
> > > +#define  PCIE_MISC_MISC_CTRL_SCB_ACCESS_EN_MASK                0x1000
> > > +#define  PCIE_MISC_MISC_CTRL_CFG_READ_UR_MODE_MASK     0x2000
> > > +#define  PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK       0x300000
> > > +#define  PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_128                0x0
> > > +#define  PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK            0xf8000000
> >
> > If you have a _MASK, don't you need a _SHIFT to allow you to read from
> > the field?
>
> See patch #7, where we import some bitfield helpers from Linux. The shift
> calculation happens there.
>
> > Can you drop the PCIE_MISC prefix? These are very long and local to this file.
>
> This was also discussed when upstreaming this in Linux, Broadcom engineers
> asked to keep the naming scheme as is, convoluted as it is, to help with
> debugging in the future. I added Jim and Florian in to the conversation in case
> they want to clarify something.
>
> [...]
>
> > > +
> > > +       return !!FIELD_GET(PCIE_MISC_PCIE_STATUS_PCIE_PORT_MASK, val);
> >
> > What is FIELD_GET? We have this sort of thing rejected in the past.
>
> This is also part of the bitfield helpers introduced in patch #7
>
> [...]
>
> > > +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 */
> > > +       brcm_pcie_bridge_sw_init_set(pcie, 1);
> > > +
> > > +       udelay(150);
> >
> > Please add a comment as to how you chose the value, and below.
>
> This was picked from Jim Quinlan's original code submission:
> https://lkml.org/lkml/2018/9/19/642
>
> Sadly there isn't any comment there.

The bridge is being reset and then un-reset.  The delay is a safety
precaution to preclude the reset signal from looking like a glitch.
BTW, what is the context here?  Aren't these patches already upstream?
Regards,
Jim

>
> Regards,
> Nicolas
>

[PATCH v2 09/10] pci: Add driver for Broadcom STB PCIe controller

[Nicolas Saenz Julienne]

Hi Jim,

On Fri, 2020-05-08 at 10:25 -0400, Jim Quinlan wrote:
> > > > +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 */
> > > > +       brcm_pcie_bridge_sw_init_set(pcie, 1);
> > > > +
> > > > +       udelay(150);
> > > 
> > > Please add a comment as to how you chose the value, and below.
> > 
> > This was picked from Jim Quinlan's original code submission:
> > https://lkml.org/lkml/2018/9/19/642
> > 
> > Sadly there isn't any comment there.
> 
> The bridge is being reset and then un-reset.  The delay is a safety
> precaution to preclude the reset signal from looking like a glitch.
> BTW, what is the context here?  Aren't these patches already upstream?

This is the submission for u-boot.

Regards,
Nicolas

-------------- 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/20200508/64122183/attachment.sig>

[PATCH v2 09/10] pci: Add driver for Broadcom STB PCIe controller

[Simon Glass]

Hi Jim,

On Fri, 8 May 2020 at 08:25, Jim Quinlan <james.quinlan@broadcom.com> wrote:
>
> On Fri, May 8, 2020 at 5:50 AM Nicolas Saenz Julienne
> <nsaenzjulienne@suse.de> wrote:
> >
> > Hi, I'll try to add my two cents.
> >
> > On Wed, 2020-05-06 at 08:47 -0600, Simon Glass wrote:
> > > > +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 Broadcom STB PCIe controller
> > > > support.
> > >
> > > What is STB? What features does this support? You should get a warning
> > > here to write at least three lines.
> >
> > STB is short for set top box, which is the wider family of SoCs bcm2711 is part
> > of[1].
> >
> > This implementation is, for now, tailor made for bcm2711. The bus not being
> > exposed, only the relevant bits to enabling xHCI compatibility, which is
> > hardwired into the RPi4 board, have been tested. For example there is so far no
> > support for IO memory mappings. Note that some work is already being done on
> > the Linux version of this driver to incorporate other devices, which will be
> > easily ported here, as for now, both are pretty much the same (minus interrupt
> > handling).
> >
> > [1] https://www.broadcom.com/products/broadband/set-top-box
> >
> > [...]
> >
> > > > +#define PCIE_MISC_MISC_CTRL                            0x4008
> > > > +#define  PCIE_MISC_MISC_CTRL_SCB_ACCESS_EN_MASK                0x1000
> > > > +#define  PCIE_MISC_MISC_CTRL_CFG_READ_UR_MODE_MASK     0x2000
> > > > +#define  PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK       0x300000
> > > > +#define  PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_128                0x0
> > > > +#define  PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK            0xf8000000
> > >
> > > If you have a _MASK, don't you need a _SHIFT to allow you to read from
> > > the field?
> >
> > See patch #7, where we import some bitfield helpers from Linux. The shift
> > calculation happens there.
> >
> > > Can you drop the PCIE_MISC prefix? These are very long and local to this file.
> >
> > This was also discussed when upstreaming this in Linux, Broadcom engineers
> > asked to keep the naming scheme as is, convoluted as it is, to help with
> > debugging in the future. I added Jim and Florian in to the conversation in case
> > they want to clarify something.
> >
> > [...]
> >
> > > > +
> > > > +       return !!FIELD_GET(PCIE_MISC_PCIE_STATUS_PCIE_PORT_MASK, val);
> > >
> > > What is FIELD_GET? We have this sort of thing rejected in the past.
> >
> > This is also part of the bitfield helpers introduced in patch #7
> >
> > [...]
> >
> > > > +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 */
> > > > +       brcm_pcie_bridge_sw_init_set(pcie, 1);
> > > > +
> > > > +       udelay(150);
> > >
> > > Please add a comment as to how you chose the value, and below.
> >
> > This was picked from Jim Quinlan's original code submission:
> > https://lkml.org/lkml/2018/9/19/642
> >
> > Sadly there isn't any comment there.
>
> The bridge is being reset and then un-reset.  The delay is a safety
> precaution to preclude the reset signal from looking like a glitch.

OK well that is a reasonable comment. It is important to put comments
on delays since people coming later may make changes and not know what
the delays are for.

> BTW, what is the context here?  Aren't these patches already upstream?

Not yet.

Regards,
Simon

[PATCH v2 09/10] pci: Add driver for Broadcom STB PCIe controller

[Sylwester Nawrocki]

Hi Jim,

On 08.05.2020 16:25, Jim Quinlan wrote:
>>>> 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 */
>>>> +       brcm_pcie_bridge_sw_init_set(pcie, 1);
>>>> +
>>>> +       udelay(150);
>>> Please add a comment as to how you chose the value, and below.
>> This was picked from Jim Quinlan's original code submission:
>> https://protect2.fireeye.com/url?k=9d9c41ed-c002ef77-9d9dcaa2-0cc47a336fae-205e162c16256602&q=1&u=https%3A%2F%2Flkml.org%2Flkml%2F2018%2F9%2F19%2F642
>>
>> Sadly there isn't any comment there.

> The bridge is being reset and then un-reset.  The delay is a safety
> precaution to preclude the reset signal from looking like a glitch.

If you don't mind I will add that sentence as a comment for the reset
delay.

-- 
Regards,
Sylwester

[PATCH v2 09/10] pci: Add driver for Broadcom STB PCIe controller

[Jim Quinlan]

On Tue, May 12, 2020 at 12:42 PM Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
>
> Hi Jim,
>
> On 08.05.2020 16:25, Jim Quinlan wrote:
> >>>> 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 */
> >>>> +       brcm_pcie_bridge_sw_init_set(pcie, 1);
> >>>> +
> >>>> +       udelay(150);
> >>> Please add a comment as to how you chose the value, and below.
> >> This was picked from Jim Quinlan's original code submission:
> >> https://protect2.fireeye.com/url?k=9d9c41ed-c002ef77-9d9dcaa2-0cc47a336fae-205e162c16256602&q=1&u=https%3A%2F%2Flkml.org%2Flkml%2F2018%2F9%2F19%2F642
> >>
> >> Sadly there isn't any comment there.
>
> > The bridge is being reset and then un-reset.  The delay is a safety
> > precaution to preclude the reset signal from looking like a glitch.
>
> If you don't mind I will add that sentence as a comment for the reset
> delay.
Yes, please do.  Thanks, Jim
>
> --
> Regards,
> Sylwester

[PATCH v2 09/10] pci: Add driver for Broadcom STB PCIe controller

[Sylwester Nawrocki]

Hi Simon,

On 06.05.2020 16:47, Simon Glass wrote:
> On Mon, 4 May 2020 at 06:45, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:

>> ---
>>  drivers/pci/Kconfig        |   6 +
>>  drivers/pci/Makefile       |   1 +
>>  drivers/pci/pcie_brcmstb.c | 594 +++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 601 insertions(+)
>>  create mode 100644 drivers/pci/pcie_brcmstb.c
> 
> A few small comments.

Thank you for time and a valuable review.

>> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig

>> +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 Broadcom STB PCIe controller support.
> 
> What is STB? What features does this support? You should get a warning
> here to write at least three lines.

I'm going to change that help text to something along the lines of:

  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.

>> diff --git a/drivers/pci/pcie_brcmstb.c b/drivers/pci/pcie_brcmstb.c
>> new file mode 100644
>> index 0000000..c6ddf92
>> --- /dev/null
>> +++ b/drivers/pci/pcie_brcmstb.c
>> @@ -0,0 +1,594 @@

>> +#include <asm/io.h>
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <dm/ofnode.h>
>> +#include <errno.h>
>> +#include <linux/bitfield.h>
>> +#include <linux/log2.h>
>> +#include <pci.h>
> 
> Check ordering of include files:
> 
> https://protect2.fireeye.com/url?k=c3a0292d-9e737093-c3a1a262-0cc47a31ba82-690f9f10b3970f9d&q=1&u=https%3A%2F%2Fwww.denx.de%2Fwiki%2FU-Boot%2FCodingStyle

Thanks for the hint, it felt there was something wrong with the ordering.

>> +#define PCIE_MISC_MISC_CTRL                            0x4008
>> +#define  PCIE_MISC_MISC_CTRL_SCB_ACCESS_EN_MASK                0x1000
>> +#define  PCIE_MISC_MISC_CTRL_CFG_READ_UR_MODE_MASK     0x2000
>> +#define  PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK       0x300000
>> +#define  PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_128                0x0
>> +#define  PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK            0xf8000000
> 
> If you have a _MASK, don't you need a _SHIFT to allow you to read from
> the field?

I had shift definitions originally but these got removed when we started to
use the FIELD_GET macro and similar. Shifts are retrieved there from a mask 
by ffs() call (find first bit set in a word). If that's not preferred in 
u-boot I will switch back to using explicit shift definitions.

> Can you drop the PCIE_MISC prefix? These are very long and local to this file.

Yes, will do. It seems many of these bit field macro definitions could have 
shorter names. I revisited them all again and shortened most of them.
>> +#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)

>> +/* MDIO registers */

>> +#define MDIO_RD_DONE(x)                        (((x) & MDIO_DATA_DONE_MASK) ? 1 : 0)
>> +#define MDIO_WT_DONE(x)                        (((x) & MDIO_DATA_DONE_MASK) ? 0 : 1)
> 
> Are these two worth it? You can do this in your code> if (readl(xxx) & MDIO_DATA_DONE_MASK)

I will drop these macros, have copied them from the Linux driver without
much thought. They are not really necessary, especially when we use
readl_poll_timout().
 
> If you must use these, then I think true/false are better than 1/0.

>> +struct brcm_pcie {
> 
> struct comment

Ok.
>> +       void __iomem            *base;
>> +
>> +       int                     gen;
>> +       bool                    ssc;
>> +};
>> +
>> +#define msleep(a) udelay((a) * 1000)
> 
> This is already defined in U-Boot.

Indeed, I should have grepped for mdelay rather than msleep. 

>> +/*
>> + * This is to convert the size of the inbound "BAR" region to the
>> + * non-linear values of PCIE_X_MISC_RC_BAR[123]_CONFIG_LO.SIZE
> 
> Please use a proper function comments with args and return value.

Will improve that.

>> + */
>> +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 */
> 
> blank line always before return at end of function

>> +       return 0;
>> +}

>> +/* The controller is capable of serving in both RC and EP roles */
>> +static bool brcm_pcie_rc_mode(struct brcm_pcie *pcie)
>> +{
>> +       u32 val = readl(pcie->base + PCIE_MISC_PCIE_STATUS);
> 
> Consider using a struct for your registers which would reduce the size
> of this code.

I agree with all your other comments but here I have some doubts.
The register offsets used are sparse and there will be many gaps in a struct. 
Anyway, I will give it a try and see how it looks.

>> +       return !!FIELD_GET(PCIE_MISC_PCIE_STATUS_PCIE_PORT_MASK, val);
> 
> What is FIELD_GET? We have this sort of thing rejected in the past.

It's a common macro from linux/bitfield.h for masking and shifting a bitfield.
If its use is not preferred I will switch to using explicit _SHIFTs as 
I mentioned above.

> Here, since you return a bool, the !! should not be necessary.
> 
>> +}
>> +
>> +static bool brcm_pcie_link_up(struct brcm_pcie *pcie)
>> +{
>> +       u32 val = readl(pcie->base + PCIE_MISC_PCIE_STATUS);
>> +       u32 dla = FIELD_GET(PCIE_MISC_PCIE_STATUS_PCIE_DL_ACTIVE_MASK, val);
>> +       u32 plu = FIELD_GET(PCIE_MISC_PCIE_STATUS_PCIE_PHYLINKUP_MASK, val);
> 
> PCIE_MISC_PCIE_STATUS_PCIE_PHYLINKUP_MASK is way too long.
> 
> How about STATUS_PCIE_PHYLINKUP_MASK?

Yes, that really looks much more reasonable.

>> +       return dla && plu;
>> +}
>> +
>> +static int brcm_pcie_config_address(const struct udevice *udev, pci_dev_t bdf,
> 
> Please use 'dev' instead of udev for the device.

Fixed.

>> +                                   uint offset, void **paddress)
>> +{
>> +       struct brcm_pcie *pcie = dev_get_priv(udev);
>> +       unsigned int bus = PCI_BUS(bdf);
>> +       unsigned int dev = PCI_DEV(bdf);
>> +       int idx;
>> +
>> +       /*
>> +        * Busses 0 (host PCIe bridge) and 1 (its immediate child)
>> +        * are limited to a single device each
>> +        */
>> +       if ((bus == (udev->seq + 1)) && dev > 0)
>> +               return -ENODEV;
>> +
>> +       /* Accesses to the RC go right to the RC registers if PCI device == 0 */
>> +       if (bus == udev->seq) {
>> +               if (PCI_DEV(bdf))
>> +                       return -ENODEV;
> 
> You can't return that as there is a device. Perhaps ENXIO? Does this
> indicate an internal error? If so you could add a comment here as to
> how to fix it.

The function is supposed to return 0 when @bdf is valid and a non 0 value
otherwise. The pci core code calls it with a range of PCI bus/device/func 
and we have to return 0 only when a PCI bus/device/function is valid. 

Exact error code from this function is not propagated further as can be 
seen in pci_generic_mmap_read_config(). This function only gets called 
by below two read/write function callbacks.

I just followed what is done in other PCIe drivers and used ENODEV.
I'm going to change the error code to EINVAL. I have also already simplified
this function so a non-zero value is returned in one place at the beginning 
only.

>> +               *paddress = pcie->base + offset;
>> +               return 0;
>> +       }
>> +
>> +       /* For devices, write to the config space index register */
>> +       idx = brcm_pcie_cfg_index(bdf, 0);
>> +
>> +       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 inline void brcm_pcie_bridge_sw_init_set(struct brcm_pcie *pcie, u32 val)
>> +{
>> +       u32 tmp;
>> +
>> +       tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1);
>> +       u32p_replace_bits(&tmp, val, PCIE_RGR1_SW_INIT_1_INIT_MASK);
>> +       writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1);
> 
> See clrsetbits_le32(), etc. Below alos.

Yes, thanks, that's really what I was missing but somehow overlooked.

>> +}

>> +/* Negative return value indicates error */
> 
> This would be incorporated in a full function comment

OK, will improve that.
 
>> +static int brcm_pcie_mdio_read(void __iomem *base, u8 port, u8 regad, u32 *val)
>> +{
>> +       int tries;
>> +       u32 data;
>> +
>> +       writel(brcm_pcie_mdio_form_pkt(port, regad, MDIO_CMD_READ),
>> +              base + PCIE_RC_DL_MDIO_ADDR);
>> +       readl(base + PCIE_RC_DL_MDIO_ADDR);
>> +
>> +       data = readl(base + PCIE_RC_DL_MDIO_RD_DATA);
>> +       for (tries = 0; !MDIO_RD_DONE(data) && tries < 10; tries++) {
>> +               udelay(10);
>> +               data = readl(base + PCIE_RC_DL_MDIO_RD_DATA);
>> +       }
>> +
>> +       *val = FIELD_GET(MDIO_DATA_MASK, data);
>> +       return MDIO_RD_DONE(data) ? 0 : -EIO;
>> +}

>> +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 */
>> +       brcm_pcie_bridge_sw_init_set(pcie, 1);
>> +
>> +       udelay(150);
> 
> Please add a comment as to how you chose the value, and below.

I'm not sure what are exact requirements for these delays, in downstream 
Linux code usleep_range(100, 200) is used, so I picked 150 us value.

>> +       /* Take the bridge out of reset */
>> +       brcm_pcie_bridge_sw_init_set(pcie, 0);
>> +
>> +       tmp = readl(base + PCIE_MISC_HARD_PCIE_HARD_DEBUG);
>> +       tmp &= ~PCIE_MISC_HARD_PCIE_HARD_DEBUG_SERDES_IDDQ_MASK;
>> +       writel(tmp, base + PCIE_MISC_HARD_PCIE_HARD_DEBUG);
>> +       /* Wait for SerDes to be stable */
>> +       udelay(150);

>> +       if (!brcm_pcie_link_up(pcie)) {
>> +               printf("PCIe BRCM: link down\n");
>> +               return -ENODEV;
> 
> Again, cannot return that error

Will make it EINVAL as is done below.

>> +       }
>> +
>> +       if (!brcm_pcie_rc_mode(pcie)) {
>> +               printf("PCIe misconfigured; is in EP mode\n");
>> +               return -EINVAL;
>> +       }
>> +

>> +       if (pcie->ssc) {
>> +               ret = brcm_pcie_set_ssc(pcie);
>> +               if (ret == 0)
>> +                       ssc_good = true;
>> +               else
>> +                       printf("PCIe BRCM: failed attempt to enter SSC mode\n");
> 
> Should this return an error?

In Linux driver any errors here are also ignored.  AFAIU that Spread 
Spectrum Clocking is not critical for the PCIe controller's operation, 
it's just for EMI reduction.  I don't have detailed documentation 
of the controller and I'm not sure what comment could be added here.

-- 
Regards,
Sylwester

[PATCH v2 09/10] pci: Add driver for Broadcom STB PCIe controller

[Simon Glass]

Hi Sylwester,

On Fri, 8 May 2020 at 05:46, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
>
> Hi Simon,
>
> On 06.05.2020 16:47, Simon Glass wrote:
> > On Mon, 4 May 2020 at 06:45, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote:
>
> >> ---
> >>  drivers/pci/Kconfig        |   6 +
> >>  drivers/pci/Makefile       |   1 +
> >>  drivers/pci/pcie_brcmstb.c | 594 +++++++++++++++++++++++++++++++++++++++++++++
> >>  3 files changed, 601 insertions(+)
> >>  create mode 100644 drivers/pci/pcie_brcmstb.c
> >
> > A few small comments.
>
> Thank you for time and a valuable review.
>
> >> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
>
> >> +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 Broadcom STB PCIe controller support.
> >
> > What is STB? What features does this support? You should get a warning
> > here to write at least three lines.
>
> I'm going to change that help text to something along the lines of:
>
>   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.
>
> >> diff --git a/drivers/pci/pcie_brcmstb.c b/drivers/pci/pcie_brcmstb.c
> >> new file mode 100644
> >> index 0000000..c6ddf92
> >> --- /dev/null
> >> +++ b/drivers/pci/pcie_brcmstb.c
> >> @@ -0,0 +1,594 @@
>
> >> +#include <asm/io.h>
> >> +#include <common.h>
> >> +#include <dm.h>
> >> +#include <dm/ofnode.h>
> >> +#include <errno.h>
> >> +#include <linux/bitfield.h>
> >> +#include <linux/log2.h>
> >> +#include <pci.h>
> >
> > Check ordering of include files:
> >
> > https://protect2.fireeye.com/url?k=c3a0292d-9e737093-c3a1a262-0cc47a31ba82-690f9f10b3970f9d&q=1&u=https%3A%2F%2Fwww.denx.de%2Fwiki%2FU-Boot%2FCodingStyle
>
> Thanks for the hint, it felt there was something wrong with the ordering.
>
> >> +#define PCIE_MISC_MISC_CTRL                            0x4008
> >> +#define  PCIE_MISC_MISC_CTRL_SCB_ACCESS_EN_MASK                0x1000
> >> +#define  PCIE_MISC_MISC_CTRL_CFG_READ_UR_MODE_MASK     0x2000
> >> +#define  PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK       0x300000
> >> +#define  PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_128                0x0
> >> +#define  PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK            0xf8000000
> >
> > If you have a _MASK, don't you need a _SHIFT to allow you to read from
> > the field?
>
> I had shift definitions originally but these got removed when we started to
> use the FIELD_GET macro and similar. Shifts are retrieved there from a mask
> by ffs() call (find first bit set in a word). If that's not preferred in
> u-boot I will switch back to using explicit shift definitions.

+Tom Rini

We've had a policy of avoiding these for some time, but now that Linux
has them, should we pull this in and start using it? It is not that
widely used in Linux but has been there for a few years.

Regards,
Simon

[PATCH v2 10/10] config: Enable support for the XHCI controller on RPI4 board

[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 v1:
 - removed unneeded CONFIG_XHCI_64BIT_DWORD_ACCESS_ONLY entry.

Changes since RFC:
 - none.
---
 configs/rpi_4_32b_defconfig | 9 +++++++++
 configs/rpi_4_defconfig     | 9 +++++++++
 configs/rpi_arm64_defconfig | 8 +++++++-
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/configs/rpi_4_32b_defconfig b/configs/rpi_4_32b_defconfig
index 72cda5d..0dd763f 100644
--- a/configs/rpi_4_32b_defconfig
+++ b/configs/rpi_4_32b_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_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 v2 10/10] config: Enable support for the XHCI controller on RPI4 board

[Bin Meng]

On Mon, May 4, 2020 at 8:45 PM Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
>
> 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 v1:
>  - removed unneeded CONFIG_XHCI_64BIT_DWORD_ACCESS_ONLY entry.
>
> Changes since RFC:
>  - none.
> ---
>  configs/rpi_4_32b_defconfig | 9 +++++++++
>  configs/rpi_4_defconfig     | 9 +++++++++
>  configs/rpi_arm64_defconfig | 8 +++++++-
>  3 files changed, 25 insertions(+), 1 deletion(-)
>

What are these 3 defconfigs files for? Seems there is no MAINTAINERS
mentioning them.

I assume rpi_4_32b_defconfig is for 32-bit U-Boot and rpi_4_defconfig
is for 64-bit U-Boot for RPi 4? What is rpi_arm64_defconfig?

Regards,
Bin

[PATCH v2 10/10] config: Enable support for the XHCI controller on RPI4 board

[Nicolas Saenz Julienne]

[ Adding Matthias as he's the board maintainer ]

On Tue, 2020-05-05 at 19:15 +0800, Bin Meng wrote:
> On Mon, May 4, 2020 at 8:45 PM Sylwester Nawrocki
> <s.nawrocki@samsung.com> wrote:
> > 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 v1:
> >  - removed unneeded CONFIG_XHCI_64BIT_DWORD_ACCESS_ONLY entry.
> > 
> > Changes since RFC:
> >  - none.
> > ---
> >  configs/rpi_4_32b_defconfig | 9 +++++++++
> >  configs/rpi_4_defconfig     | 9 +++++++++
> >  configs/rpi_arm64_defconfig | 8 +++++++-
> >  3 files changed, 25 insertions(+), 1 deletion(-)
> > 
> 
> What are these 3 defconfigs files for? Seems there is no MAINTAINERS
> mentioning them.
> 
> I assume rpi_4_32b_defconfig is for 32-bit U-Boot and rpi_4_defconfig
> is for 64-bit U-Boot for RPi 4? What is rpi_arm64_defconfig?

I think rpi_arm64_defconfig is for both rpi3 & rpi4.

Regards,
Nicolas

-------------- 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/20200505/274a0129/attachment.sig>

[PATCH v2 10/10] config: Enable support for the XHCI controller on RPI4 board

[Marek Szyprowski]

Hi

On 05.05.2020 13:15, Bin Meng wrote:
> On Mon, May 4, 2020 at 8:45 PM Sylwester Nawrocki
> <s.nawrocki@samsung.com> wrote:
>> 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 v1:
>>   - removed unneeded CONFIG_XHCI_64BIT_DWORD_ACCESS_ONLY entry.
>>
>> Changes since RFC:
>>   - none.
>> ---
>>   configs/rpi_4_32b_defconfig | 9 +++++++++
>>   configs/rpi_4_defconfig     | 9 +++++++++
>>   configs/rpi_arm64_defconfig | 8 +++++++-
>>   3 files changed, 25 insertions(+), 1 deletion(-)
>>
> What are these 3 defconfigs files for? Seems there is no MAINTAINERS
> mentioning them.
>
> I assume rpi_4_32b_defconfig is for 32-bit U-Boot and rpi_4_defconfig
> is for 64-bit U-Boot for RPi 4? What is rpi_arm64_defconfig?

rpi_arm64_defconfig is for common image, which works on both RPi 3 and 
RPi 4.

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

[PATCH v2 00/10] USB host support for Raspberry Pi 4 board

[Nicolas Saenz Julienne]

On Mon, 2020-05-04 at 14:45 +0200, 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 32-bit and 64-bit system builds and some related fixes and updates
> in the usb/xhci and the pci driver core code.
> 
> This iteration includes minor corrections in patches 7, 9, 10 addressing
> review comments.
> 
> The patch series is based on v2020.07-rc1 tree.
> 
> Thanks,
> Sylwester

I tested the whole series with rpi_4_defconfig and rpi_arm64_defconfig.

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

Regards,
Nicolas

> Marek Szyprowski (4):
>   rpi4: shorten a mapping for the DRAM
>   rpi4: add a mapping for the PCIe XHCI controller MMIO registers (ARM
>     64bit)
>   rpi4: add a mapping for the PCIe XHCI controller MMIO registers (ARM
>     32bit)
>   config: Enable support for the XHCI controller on RPI4 board
> 
> 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 STB PCIe controller
> 
>  arch/arm/mach-bcm283x/Kconfig             |   1 +
>  arch/arm/mach-bcm283x/include/mach/base.h |   7 +
>  arch/arm/mach-bcm283x/init.c              |  72 +++-
>  configs/rpi_4_32b_defconfig               |   9 +
>  configs/rpi_4_defconfig                   |   9 +
>  configs/rpi_arm64_defconfig               |   8 +-
>  drivers/pci/Kconfig                       |   6 +
>  drivers/pci/Makefile                      |   1 +
>  drivers/pci/pci-rcar-gen3.c               |   8 -
>  drivers/pci/pcie_brcmstb.c                | 594
> ++++++++++++++++++++++++++++++
>  drivers/pci/pcie_intel_fpga.c             |   3 -
>  drivers/usb/host/xhci-mem.c               |   3 +
>  include/linux/bitfield.h                  |  50 +++
>  include/pci.h                             |  19 +-
>  include/usb/xhci.h                        |   8 -
>  15 files changed, 772 insertions(+), 26 deletions(-)
>  create mode 100644 drivers/pci/pcie_brcmstb.c
> 

-------------- 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/20200505/229cc9aa/attachment.sig>