[PATCH v2 0/6] Raspberry Pi 4 PCIe support

[PATCH v2 0/6] Raspberry Pi 4 PCIe support

[Nicolas Saenz Julienne]

This series aims at providing support for Raspberry Pi 4's PCIe
controller, which is also shared with the Broadcom STB family of
devices.

There was a previous attempt to upstream this some years ago[1] but was
blocked as most STB PCIe integrations have a sparse DMA mapping[2] which
is something currently not supported by the kernel.  Luckily this is not
the case for the Raspberry Pi 4.

Note that the driver code is to be based on top of Rob Herring's series
simplifying inbound and outbound range parsing.

[1] https://patchwork.kernel.org/cover/10605933/
[2] https://patchwork.kernel.org/patch/10605957/

---

Changes since v1:
  - add generic rounddown/roundup_pow_two64() patch
  - Add MAINTAINERS patch
  - Fix Kconfig
  - Cleanup probe, use up to date APIs, exit on MSI failure
  - Get rid of linux,pci-domain and other unused constructs
  - Use edge triggered setup for MSI
  - Cleanup MSI implementation
  - Fix multiple cosmetic issues
  - Remove supend/resume code

Jim Quinlan (3):
  dt-bindings: PCI: Add bindings for brcmstb's PCIe device
  PCI: brcmstb: add Broadcom STB PCIe host controller driver
  PCI: brcmstb: add MSI capability

Nicolas Saenz Julienne (3):
  linux/log2.h: Add roundup/rounddown_pow_two64() family of functions
  ARM: dts: bcm2711: Enable PCIe controller
  MAINTAINERS: Add brcmstb PCIe controller

 .../bindings/pci/brcm,stb-pcie.yaml           |  110 ++
 MAINTAINERS                                   |    4 +
 arch/arm/boot/dts/bcm2711.dtsi                |   46 +
 drivers/net/ethernet/mellanox/mlx4/en_clock.c |    3 +-
 drivers/pci/controller/Kconfig                |    9 +
 drivers/pci/controller/Makefile               |    1 +
 drivers/pci/controller/pcie-brcmstb.c         | 1179 +++++++++++++++++
 drivers/pci/controller/pcie-cadence-ep.c      |    7 +-
 drivers/pci/controller/pcie-cadence.c         |    7 +-
 drivers/pci/controller/pcie-rockchip-ep.c     |    9 +-
 include/linux/log2.h                          |   52 +
 kernel/dma/direct.c                           |    3 +-
 12 files changed, 1412 insertions(+), 18 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
 create mode 100644 drivers/pci/controller/pcie-brcmstb.c

-- 
2.24.0

[PATCH v2 1/6] linux/log2.h: Add roundup/rounddown_pow_two64() family of functions

[Nicolas Saenz Julienne]

Some users need to make sure their rounding function accepts and returns
64bit long variables regardless of the architecture. Sadly
roundup/rounddown_pow_two() takes and returns unsigned longs. Create a
new generic 64bit variant of the function and cleanup rougue custom
implementations.

Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
---
 drivers/net/ethernet/mellanox/mlx4/en_clock.c |  3 +-
 drivers/pci/controller/pcie-cadence-ep.c      |  7 +--
 drivers/pci/controller/pcie-cadence.c         |  7 +--
 drivers/pci/controller/pcie-rockchip-ep.c     |  9 ++--
 include/linux/log2.h                          | 52 +++++++++++++++++++
 kernel/dma/direct.c                           |  3 +-
 6 files changed, 63 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_clock.c b/drivers/net/ethernet/mellanox/mlx4/en_clock.c
index 024788549c25..027bd72505e2 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_clock.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_clock.c
@@ -33,6 +33,7 @@
 
 #include <linux/mlx4/device.h>
 #include <linux/clocksource.h>
+#include <linux/log2.h>
 
 #include "mlx4_en.h"
 
@@ -252,7 +253,7 @@ static u32 freq_to_shift(u16 freq)
 {
 	u32 freq_khz = freq * 1000;
 	u64 max_val_cycles = freq_khz * 1000 * MLX4_EN_WRAP_AROUND_SEC;
-	u64 max_val_cycles_rounded = 1ULL << fls64(max_val_cycles - 1);
+	u64 max_val_cycles_rounded = roundup_pow_of_two64(max_val_cycles);
 	/* calculate max possible multiplier in order to fit in 64bit */
 	u64 max_mul = div64_u64(ULLONG_MAX, max_val_cycles_rounded);
 
diff --git a/drivers/pci/controller/pcie-cadence-ep.c b/drivers/pci/controller/pcie-cadence-ep.c
index def7820cb824..26ff424b16f5 100644
--- a/drivers/pci/controller/pcie-cadence-ep.c
+++ b/drivers/pci/controller/pcie-cadence-ep.c
@@ -10,6 +10,7 @@
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/sizes.h>
+#include <linux/log2.h>
 
 #include "pcie-cadence.h"
 
@@ -90,11 +91,7 @@ static int cdns_pcie_ep_set_bar(struct pci_epc *epc, u8 fn,
 
 	/* BAR size is 2^(aperture + 7) */
 	sz = max_t(size_t, epf_bar->size, CDNS_PCIE_EP_MIN_APERTURE);
-	/*
-	 * roundup_pow_of_two() returns an unsigned long, which is not suited
-	 * for 64bit values.
-	 */
-	sz = 1ULL << fls64(sz - 1);
+	sz = roundup_pow_of_two64(sz);
 	aperture = ilog2(sz) - 7; /* 128B -> 0, 256B -> 1, 512B -> 2, ... */
 
 	if ((flags & PCI_BASE_ADDRESS_SPACE) == PCI_BASE_ADDRESS_SPACE_IO) {
diff --git a/drivers/pci/controller/pcie-cadence.c b/drivers/pci/controller/pcie-cadence.c
index cd795f6fc1e2..b2278e6b955c 100644
--- a/drivers/pci/controller/pcie-cadence.c
+++ b/drivers/pci/controller/pcie-cadence.c
@@ -4,6 +4,7 @@
 // Author: Cyrille Pitchen <cyrille.pitchen@free-electrons.com>
 
 #include <linux/kernel.h>
+#include <linux/log2.h>
 
 #include "pcie-cadence.h"
 
@@ -11,11 +12,7 @@ void cdns_pcie_set_outbound_region(struct cdns_pcie *pcie, u8 fn,
 				   u32 r, bool is_io,
 				   u64 cpu_addr, u64 pci_addr, size_t size)
 {
-	/*
-	 * roundup_pow_of_two() returns an unsigned long, which is not suited
-	 * for 64bit values.
-	 */
-	u64 sz = 1ULL << fls64(size - 1);
+	u64 sz = roundup_pow_of_two64(size);
 	int nbits = ilog2(sz);
 	u32 addr0, addr1, desc0, desc1;
 
diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
index d743b0a48988..ed50aaf27784 100644
--- a/drivers/pci/controller/pcie-rockchip-ep.c
+++ b/drivers/pci/controller/pcie-rockchip-ep.c
@@ -16,6 +16,7 @@
 #include <linux/platform_device.h>
 #include <linux/pci-epf.h>
 #include <linux/sizes.h>
+#include <linux/log2.h>
 
 #include "pcie-rockchip.h"
 
@@ -70,7 +71,7 @@ static void rockchip_pcie_prog_ep_ob_atu(struct rockchip_pcie *rockchip, u8 fn,
 					 u32 r, u32 type, u64 cpu_addr,
 					 u64 pci_addr, size_t size)
 {
-	u64 sz = 1ULL << fls64(size - 1);
+	u64 sz = roundup_pow_of_two64(size);
 	int num_pass_bits = ilog2(sz);
 	u32 addr0, addr1, desc0, desc1;
 	bool is_nor_msg = (type == AXI_WRAPPER_NOR_MSG);
@@ -172,11 +173,7 @@ static int rockchip_pcie_ep_set_bar(struct pci_epc *epc, u8 fn,
 	/* BAR size is 2^(aperture + 7) */
 	sz = max_t(size_t, epf_bar->size, MIN_EP_APERTURE);
 
-	/*
-	 * roundup_pow_of_two() returns an unsigned long, which is not suited
-	 * for 64bit values.
-	 */
-	sz = 1ULL << fls64(sz - 1);
+	sz = roundup_pow_of_two64(sz);
 	aperture = ilog2(sz) - 7; /* 128B -> 0, 256B -> 1, 512B -> 2, ... */
 
 	if ((flags & PCI_BASE_ADDRESS_SPACE) == PCI_BASE_ADDRESS_SPACE_IO) {
diff --git a/include/linux/log2.h b/include/linux/log2.h
index 83a4a3ca3e8a..db12d92ab6eb 100644
--- a/include/linux/log2.h
+++ b/include/linux/log2.h
@@ -67,6 +67,24 @@ unsigned long __rounddown_pow_of_two(unsigned long n)
 	return 1UL << (fls_long(n) - 1);
 }
 
+/**
+ * __roundup_pow_of_two64() - round 64bit value up to nearest power of two
+ * @n: value to round up
+ */
+static inline __attribute__((const)) __u64 __roundup_pow_of_two64(__u64 n)
+{
+	return 1UL << fls64(n - 1);
+}
+
+/**
+ * __rounddown_pow_of_two64() - round 64bit value down to nearest power of two
+ * @n: value to round down
+ */
+static inline __attribute__((const)) __u64 __rounddown_pow_of_two64(__u64 n)
+{
+	return 1UL << (fls64(n) - 1);
+}
+
 /**
  * const_ilog2 - log base 2 of 32-bit or a 64-bit constant unsigned value
  * @n: parameter
@@ -194,6 +212,40 @@ unsigned long __rounddown_pow_of_two(unsigned long n)
 	__rounddown_pow_of_two(n)		\
  )
 
+/**
+ * roundup_pow_of_two64 - round the given 64bit value up to nearest power of
+ * two
+ * @n: parameter
+ *
+ * round the given value up to the nearest power of two
+ * - the result is undefined when n == 0
+ * - this can be used to initialise global variables from constant data
+ */
+#define roundup_pow_of_two64(n)			\
+(						\
+	__builtin_constant_p(n) ? (		\
+		(n == 1) ? 1 :			\
+		(1UL << (ilog2((n) - 1) + 1))	\
+				   ) :		\
+	__roundup_pow_of_two64(n)		\
+)
+
+/**
+ * rounddown_pow_of_two64 - round the given 64bit value down to nearest power
+ * of two
+ * @n: parameter
+ *
+ * round the given value down to the nearest power of two
+ * - the result is undefined when n == 0
+ * - this can be used to initialise global variables from constant data
+ */
+#define rounddown_pow_of_two64(n)		\
+(						\
+	__builtin_constant_p(n) ? (		\
+		(1UL << ilog2(n))) :		\
+	__rounddown_pow_of_two64(n)		\
+)
+
 static inline __attribute_const__
 int __order_base_2(unsigned long n)
 {
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index b9e1744999d9..a419530abd3e 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -15,6 +15,7 @@
 #include <linux/vmalloc.h>
 #include <linux/set_memory.h>
 #include <linux/swiotlb.h>
+#include <linux/log2.h>
 
 /*
  * Most architectures use ZONE_DMA for the first 16 Megabytes, but some use it
@@ -53,7 +54,7 @@ u64 dma_direct_get_required_mask(struct device *dev)
 {
 	u64 max_dma = phys_to_dma_direct(dev, (max_pfn - 1) << PAGE_SHIFT);
 
-	return (1ULL << (fls64(max_dma) - 1)) * 2 - 1;
+	return rounddown_pow_of_two64(max_dma) * 2 - 1;
 }
 
 static gfp_t __dma_direct_optimal_gfp_mask(struct device *dev, u64 dma_mask,
-- 
2.24.0

Re: [PATCH v2 1/6] linux/log2.h: Add roundup/rounddown_pow_two64() family of functions

[Andrew Murray]

On Tue, Nov 12, 2019 at 04:59:20PM +0100, Nicolas Saenz Julienne wrote:
> Some users need to make sure their rounding function accepts and returns
> 64bit long variables regardless of the architecture. Sadly
> roundup/rounddown_pow_two() takes and returns unsigned longs. Create a
> new generic 64bit variant of the function and cleanup rougue custom
> implementations.
> 
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> ---
>  drivers/net/ethernet/mellanox/mlx4/en_clock.c |  3 +-
>  drivers/pci/controller/pcie-cadence-ep.c      |  7 +--
>  drivers/pci/controller/pcie-cadence.c         |  7 +--
>  drivers/pci/controller/pcie-rockchip-ep.c     |  9 ++--

Thanks for making this change. See comments inline...

>  include/linux/log2.h                          | 52 +++++++++++++++++++
>  kernel/dma/direct.c                           |  3 +-
>  6 files changed, 63 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_clock.c b/drivers/net/ethernet/mellanox/mlx4/en_clock.c
> index 024788549c25..027bd72505e2 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_clock.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_clock.c
> @@ -33,6 +33,7 @@
>  
>  #include <linux/mlx4/device.h>
>  #include <linux/clocksource.h>
> +#include <linux/log2.h>
>  
>  #include "mlx4_en.h"
>  
> @@ -252,7 +253,7 @@ static u32 freq_to_shift(u16 freq)
>  {
>  	u32 freq_khz = freq * 1000;
>  	u64 max_val_cycles = freq_khz * 1000 * MLX4_EN_WRAP_AROUND_SEC;
> -	u64 max_val_cycles_rounded = 1ULL << fls64(max_val_cycles - 1);
> +	u64 max_val_cycles_rounded = roundup_pow_of_two64(max_val_cycles);
>  	/* calculate max possible multiplier in order to fit in 64bit */
>  	u64 max_mul = div64_u64(ULLONG_MAX, max_val_cycles_rounded);
>  
> diff --git a/drivers/pci/controller/pcie-cadence-ep.c b/drivers/pci/controller/pcie-cadence-ep.c
> index def7820cb824..26ff424b16f5 100644
> --- a/drivers/pci/controller/pcie-cadence-ep.c
> +++ b/drivers/pci/controller/pcie-cadence-ep.c
> @@ -10,6 +10,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/sizes.h>
> +#include <linux/log2.h>
>  
>  #include "pcie-cadence.h"
>  
> @@ -90,11 +91,7 @@ static int cdns_pcie_ep_set_bar(struct pci_epc *epc, u8 fn,
>  
>  	/* BAR size is 2^(aperture + 7) */
>  	sz = max_t(size_t, epf_bar->size, CDNS_PCIE_EP_MIN_APERTURE);
> -	/*
> -	 * roundup_pow_of_two() returns an unsigned long, which is not suited
> -	 * for 64bit values.
> -	 */
> -	sz = 1ULL << fls64(sz - 1);
> +	sz = roundup_pow_of_two64(sz);
>  	aperture = ilog2(sz) - 7; /* 128B -> 0, 256B -> 1, 512B -> 2, ... */
>  
>  	if ((flags & PCI_BASE_ADDRESS_SPACE) == PCI_BASE_ADDRESS_SPACE_IO) {
> diff --git a/drivers/pci/controller/pcie-cadence.c b/drivers/pci/controller/pcie-cadence.c
> index cd795f6fc1e2..b2278e6b955c 100644
> --- a/drivers/pci/controller/pcie-cadence.c
> +++ b/drivers/pci/controller/pcie-cadence.c
> @@ -4,6 +4,7 @@
>  // Author: Cyrille Pitchen <cyrille.pitchen@free-electrons.com>
>  
>  #include <linux/kernel.h>
> +#include <linux/log2.h>
>  
>  #include "pcie-cadence.h"
>  
> @@ -11,11 +12,7 @@ void cdns_pcie_set_outbound_region(struct cdns_pcie *pcie, u8 fn,
>  				   u32 r, bool is_io,
>  				   u64 cpu_addr, u64 pci_addr, size_t size)
>  {
> -	/*
> -	 * roundup_pow_of_two() returns an unsigned long, which is not suited
> -	 * for 64bit values.
> -	 */
> -	u64 sz = 1ULL << fls64(size - 1);
> +	u64 sz = roundup_pow_of_two64(size);
>  	int nbits = ilog2(sz);
>  	u32 addr0, addr1, desc0, desc1;
>  
> diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
> index d743b0a48988..ed50aaf27784 100644
> --- a/drivers/pci/controller/pcie-rockchip-ep.c
> +++ b/drivers/pci/controller/pcie-rockchip-ep.c
> @@ -16,6 +16,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/pci-epf.h>
>  #include <linux/sizes.h>
> +#include <linux/log2.h>
>  
>  #include "pcie-rockchip.h"
>  
> @@ -70,7 +71,7 @@ static void rockchip_pcie_prog_ep_ob_atu(struct rockchip_pcie *rockchip, u8 fn,
>  					 u32 r, u32 type, u64 cpu_addr,
>  					 u64 pci_addr, size_t size)
>  {
> -	u64 sz = 1ULL << fls64(size - 1);
> +	u64 sz = roundup_pow_of_two64(size);
>  	int num_pass_bits = ilog2(sz);
>  	u32 addr0, addr1, desc0, desc1;
>  	bool is_nor_msg = (type == AXI_WRAPPER_NOR_MSG);
> @@ -172,11 +173,7 @@ static int rockchip_pcie_ep_set_bar(struct pci_epc *epc, u8 fn,
>  	/* BAR size is 2^(aperture + 7) */
>  	sz = max_t(size_t, epf_bar->size, MIN_EP_APERTURE);
>  
> -	/*
> -	 * roundup_pow_of_two() returns an unsigned long, which is not suited
> -	 * for 64bit values.
> -	 */
> -	sz = 1ULL << fls64(sz - 1);
> +	sz = roundup_pow_of_two64(sz);
>  	aperture = ilog2(sz) - 7; /* 128B -> 0, 256B -> 1, 512B -> 2, ... */
>  
>  	if ((flags & PCI_BASE_ADDRESS_SPACE) == PCI_BASE_ADDRESS_SPACE_IO) {
> diff --git a/include/linux/log2.h b/include/linux/log2.h
> index 83a4a3ca3e8a..db12d92ab6eb 100644
> --- a/include/linux/log2.h
> +++ b/include/linux/log2.h
> @@ -67,6 +67,24 @@ unsigned long __rounddown_pow_of_two(unsigned long n)
>  	return 1UL << (fls_long(n) - 1);
>  }
>  
> +/**
> + * __roundup_pow_of_two64() - round 64bit value up to nearest power of two
> + * @n: value to round up
> + */
> +static inline __attribute__((const)) __u64 __roundup_pow_of_two64(__u64 n)

To be consistent with other functions in the same file (__ilog_u64) you may
want to rename this to __roundup_pow_of_two_u64.

Also do you know why u64 is used in some places and __u64 in others?

> +{
> +	return 1UL << fls64(n - 1);

Does this need to be (and for the others):

return 1ULL << fls64(n - 1);

Notice that the PCI drivers you convert, all use 1ULL.

Thanks,

Andrew Murray 


> +}
> +
> +/**
> + * __rounddown_pow_of_two64() - round 64bit value down to nearest power of two
> + * @n: value to round down
> + */
> +static inline __attribute__((const)) __u64 __rounddown_pow_of_two64(__u64 n)
> +{
> +	return 1UL << (fls64(n) - 1);
> +}
> +
>  /**
>   * const_ilog2 - log base 2 of 32-bit or a 64-bit constant unsigned value
>   * @n: parameter
> @@ -194,6 +212,40 @@ unsigned long __rounddown_pow_of_two(unsigned long n)
>  	__rounddown_pow_of_two(n)		\
>   )
>  
> +/**
> + * roundup_pow_of_two64 - round the given 64bit value up to nearest power of
> + * two
> + * @n: parameter
> + *
> + * round the given value up to the nearest power of two
> + * - the result is undefined when n == 0
> + * - this can be used to initialise global variables from constant data
> + */
> +#define roundup_pow_of_two64(n)			\
> +(						\
> +	__builtin_constant_p(n) ? (		\
> +		(n == 1) ? 1 :			\
> +		(1UL << (ilog2((n) - 1) + 1))	\
> +				   ) :		\
> +	__roundup_pow_of_two64(n)		\
> +)
> +
> +/**
> + * rounddown_pow_of_two64 - round the given 64bit value down to nearest power
> + * of two
> + * @n: parameter
> + *
> + * round the given value down to the nearest power of two
> + * - the result is undefined when n == 0
> + * - this can be used to initialise global variables from constant data
> + */
> +#define rounddown_pow_of_two64(n)		\
> +(						\
> +	__builtin_constant_p(n) ? (		\
> +		(1UL << ilog2(n))) :		\
> +	__rounddown_pow_of_two64(n)		\
> +)
> +
>  static inline __attribute_const__
>  int __order_base_2(unsigned long n)
>  {
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index b9e1744999d9..a419530abd3e 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -15,6 +15,7 @@
>  #include <linux/vmalloc.h>
>  #include <linux/set_memory.h>
>  #include <linux/swiotlb.h>
> +#include <linux/log2.h>
>  
>  /*
>   * Most architectures use ZONE_DMA for the first 16 Megabytes, but some use it
> @@ -53,7 +54,7 @@ u64 dma_direct_get_required_mask(struct device *dev)
>  {
>  	u64 max_dma = phys_to_dma_direct(dev, (max_pfn - 1) << PAGE_SHIFT);
>  
> -	return (1ULL << (fls64(max_dma) - 1)) * 2 - 1;
> +	return rounddown_pow_of_two64(max_dma) * 2 - 1;
>  }
>  
>  static gfp_t __dma_direct_optimal_gfp_mask(struct device *dev, u64 dma_mask,
> -- 
> 2.24.0
> 

Re: [PATCH v2 0/6] Raspberry Pi 4 PCIe support

[Andrew Murray]

On Tue, Nov 12, 2019 at 04:59:19PM +0100, Nicolas Saenz Julienne wrote:
> This series aims at providing support for Raspberry Pi 4's PCIe
> controller, which is also shared with the Broadcom STB family of
> devices.
> 
> There was a previous attempt to upstream this some years ago[1] but was
> blocked as most STB PCIe integrations have a sparse DMA mapping[2] which
> is something currently not supported by the kernel.  Luckily this is not
> the case for the Raspberry Pi 4.
> 
> Note that the driver code is to be based on top of Rob Herring's series
> simplifying inbound and outbound range parsing.
> 
> [1] https://patchwork.kernel.org/cover/10605933/
> [2] https://patchwork.kernel.org/patch/10605957/
> 

What happened to patch 3? I can't see it on the list or in patchwork?

Thanks,

Andrew Murray

> ---
> 
> Changes since v1:
>   - add generic rounddown/roundup_pow_two64() patch
>   - Add MAINTAINERS patch
>   - Fix Kconfig
>   - Cleanup probe, use up to date APIs, exit on MSI failure
>   - Get rid of linux,pci-domain and other unused constructs
>   - Use edge triggered setup for MSI
>   - Cleanup MSI implementation
>   - Fix multiple cosmetic issues
>   - Remove supend/resume code
> 
> Jim Quinlan (3):
>   dt-bindings: PCI: Add bindings for brcmstb's PCIe device
>   PCI: brcmstb: add Broadcom STB PCIe host controller driver
>   PCI: brcmstb: add MSI capability
> 
> Nicolas Saenz Julienne (3):
>   linux/log2.h: Add roundup/rounddown_pow_two64() family of functions
>   ARM: dts: bcm2711: Enable PCIe controller
>   MAINTAINERS: Add brcmstb PCIe controller
> 
>  .../bindings/pci/brcm,stb-pcie.yaml           |  110 ++
>  MAINTAINERS                                   |    4 +
>  arch/arm/boot/dts/bcm2711.dtsi                |   46 +
>  drivers/net/ethernet/mellanox/mlx4/en_clock.c |    3 +-
>  drivers/pci/controller/Kconfig                |    9 +
>  drivers/pci/controller/Makefile               |    1 +
>  drivers/pci/controller/pcie-brcmstb.c         | 1179 +++++++++++++++++
>  drivers/pci/controller/pcie-cadence-ep.c      |    7 +-
>  drivers/pci/controller/pcie-cadence.c         |    7 +-
>  drivers/pci/controller/pcie-rockchip-ep.c     |    9 +-
>  include/linux/log2.h                          |   52 +
>  kernel/dma/direct.c                           |    3 +-
>  12 files changed, 1412 insertions(+), 18 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
>  create mode 100644 drivers/pci/controller/pcie-brcmstb.c
> 
> -- 
> 2.24.0
> 

Re: [PATCH v2 1/6] linux/log2.h: Add roundup/rounddown_pow_two64() family of functions

[Nicolas Saenz Julienne]

[-- Attachment #1: Type: text/plain, Size: 758 bytes --]

Hi Andrew, thanks for the review.
> > +/**
> > + * __roundup_pow_of_two64() - round 64bit value up to nearest power of two
> > + * @n: value to round up
> > + */
> > +static inline __attribute__((const)) __u64 __roundup_pow_of_two64(__u64 n)
> 
> To be consistent with other functions in the same file (__ilog_u64) you may
> want to rename this to __roundup_pow_of_two_u64.

Sounds good to me.

> Also do you know why u64 is used in some places and __u64 in others?

That's unwarranted, it should be __u64 everywhere.

> > +{
> > +	return 1UL << fls64(n - 1);
> 
> Does this need to be (and for the others):
> 
> return 1ULL << fls64(n - 1);
> 
> Notice that the PCI drivers you convert, all use 1ULL.

Noted

Regards,
Nicolas


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 0/6] Raspberry Pi 4 PCIe support

[Nicolas Saenz Julienne]

[-- Attachment #1: Type: text/plain, Size: 1368 bytes --]

On Tue, 2019-11-19 at 11:18 +0000, Andrew Murray wrote:
> On Tue, Nov 12, 2019 at 04:59:19PM +0100, Nicolas Saenz Julienne wrote:
> > This series aims at providing support for Raspberry Pi 4's PCIe
> > controller, which is also shared with the Broadcom STB family of
> > devices.
> > 
> > There was a previous attempt to upstream this some years ago[1] but was
> > blocked as most STB PCIe integrations have a sparse DMA mapping[2] which
> > is something currently not supported by the kernel.  Luckily this is not
> > the case for the Raspberry Pi 4.
> > 
> > Note that the driver code is to be based on top of Rob Herring's series
> > simplifying inbound and outbound range parsing.
> > 
> > [1] https://patchwork.kernel.org/cover/10605933/
> > [2] https://patchwork.kernel.org/patch/10605957/
> > 
> 
> What happened to patch 3? I can't see it on the list or in patchwork?

For some reason the script I use to call get_maintainer.sh or git send-mail
failed to add linux-pci@vger.kernel.org and linux-kernel@vger.kernel.org as
recipients. I didn't do anything different between v1 and v2 as far as mailing
is concerned.

Nevertheless it's here: https://www.spinics.net/lists/arm-kernel/msg768461.html
and should be present in the linux-arm-kernel list.

I'll look in to it and make sure this doesn't happen in v3.

Regards,
Nicolas


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/6] linux/log2.h: Add roundup/rounddown_pow_two64() family of functions

[Nicolas Saenz Julienne]

[-- Attachment #1: Type: text/plain, Size: 958 bytes --]

On Tue, 2019-11-19 at 12:30 +0100, Nicolas Saenz Julienne wrote:
> Hi Andrew, thanks for the review.
> > > +/**
> > > + * __roundup_pow_of_two64() - round 64bit value up to nearest power of
> > > two
> > > + * @n: value to round up
> > > + */
> > > +static inline __attribute__((const)) __u64 __roundup_pow_of_two64(__u64
> > > n)
> > 
> > To be consistent with other functions in the same file (__ilog_u64) you may
> > want to rename this to __roundup_pow_of_two_u64.
> 
> Sounds good to me.
> 
> > Also do you know why u64 is used in some places and __u64 in others?
> 
> That's unwarranted, it should be __u64 everywhere.

Sorry, now that I look deeper into it, it should be u64.

> > > +{
> > > +	return 1UL << fls64(n - 1);
> > 
> > Does this need to be (and for the others):
> > 
> > return 1ULL << fls64(n - 1);
> > 
> > Notice that the PCI drivers you convert, all use 1ULL.
> 
> Noted
> 
> Regards,
> Nicolas
> 


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/6] linux/log2.h: Add roundup/rounddown_pow_two64() family of functions

[Andrew Murray]

On Tue, Nov 19, 2019 at 01:43:39PM +0100, Nicolas Saenz Julienne wrote:
> On Tue, 2019-11-19 at 12:30 +0100, Nicolas Saenz Julienne wrote:
> > Hi Andrew, thanks for the review.
> > > > +/**
> > > > + * __roundup_pow_of_two64() - round 64bit value up to nearest power of
> > > > two
> > > > + * @n: value to round up
> > > > + */
> > > > +static inline __attribute__((const)) __u64 __roundup_pow_of_two64(__u64
> > > > n)
> > > 
> > > To be consistent with other functions in the same file (__ilog_u64) you may
> > > want to rename this to __roundup_pow_of_two_u64.
> > 
> > Sounds good to me.
> > 
> > > Also do you know why u64 is used in some places and __u64 in others?
> > 
> > That's unwarranted, it should be __u64 everywhere.
> 
> Sorry, now that I look deeper into it, it should be u64.

Do you know the reason why? I'd be interested to know.

Thanks,

Andrew Murray

> 
> > > > +{
> > > > +	return 1UL << fls64(n - 1);
> > > 
> > > Does this need to be (and for the others):
> > > 
> > > return 1ULL << fls64(n - 1);
> > > 
> > > Notice that the PCI drivers you convert, all use 1ULL.
> > 
> > Noted
> > 
> > Regards,
> > Nicolas
> > 
> 

Re: [PATCH v2 1/6] linux/log2.h: Add roundup/rounddown_pow_two64() family of functions

[Jason Gunthorpe]

On Tue, Nov 19, 2019 at 04:28:50PM +0000, Andrew Murray wrote:
> On Tue, Nov 19, 2019 at 01:43:39PM +0100, Nicolas Saenz Julienne wrote:
> > On Tue, 2019-11-19 at 12:30 +0100, Nicolas Saenz Julienne wrote:
> > > Hi Andrew, thanks for the review.
> > > > > +/**
> > > > > + * __roundup_pow_of_two64() - round 64bit value up to nearest power of
> > > > > two
> > > > > + * @n: value to round up
> > > > > + */
> > > > > +static inline __attribute__((const)) __u64 __roundup_pow_of_two64(__u64
> > > > > n)
> > > > 
> > > > To be consistent with other functions in the same file (__ilog_u64) you may
> > > > want to rename this to __roundup_pow_of_two_u64.
> > > 
> > > Sounds good to me.
> > > 
> > > > Also do you know why u64 is used in some places and __u64 in others?
> > > 
> > > That's unwarranted, it should be __u64 everywhere.
> > 
> > Sorry, now that I look deeper into it, it should be u64.
> 
> Do you know the reason why? I'd be interested to know.

__u64 must be used in header files that are under uapi - ie it is the
name of the symbol in userspace, and u64 does not exist.

u64 should be used in all code that is only inside the kernel, ie .c
files, internal headers, etc

I routinely discourage use of __uXX in kernel native code.

Jason

Re: [PATCH v2 1/6] linux/log2.h: Add roundup/rounddown_pow_two64() family of functions

[Andrew Murray]

On Tue, Nov 19, 2019 at 12:55:02PM -0400, Jason Gunthorpe wrote:
> On Tue, Nov 19, 2019 at 04:28:50PM +0000, Andrew Murray wrote:
> > On Tue, Nov 19, 2019 at 01:43:39PM +0100, Nicolas Saenz Julienne wrote:
> > > On Tue, 2019-11-19 at 12:30 +0100, Nicolas Saenz Julienne wrote:
> > > > Hi Andrew, thanks for the review.
> > > > > > +/**
> > > > > > + * __roundup_pow_of_two64() - round 64bit value up to nearest power of
> > > > > > two
> > > > > > + * @n: value to round up
> > > > > > + */
> > > > > > +static inline __attribute__((const)) __u64 __roundup_pow_of_two64(__u64
> > > > > > n)
> > > > > 
> > > > > To be consistent with other functions in the same file (__ilog_u64) you may
> > > > > want to rename this to __roundup_pow_of_two_u64.
> > > > 
> > > > Sounds good to me.
> > > > 
> > > > > Also do you know why u64 is used in some places and __u64 in others?
> > > > 
> > > > That's unwarranted, it should be __u64 everywhere.
> > > 
> > > Sorry, now that I look deeper into it, it should be u64.
> > 
> > Do you know the reason why? I'd be interested to know.
> 
> __u64 must be used in header files that are under uapi - ie it is the
> name of the symbol in userspace, and u64 does not exist.
> 
> u64 should be used in all code that is only inside the kernel, ie .c
> files, internal headers, etc
> 
> I routinely discourage use of __uXX in kernel native code.

Thanks for this, much appreciated!

Andrew Murray

> 
> Jason

Re: [PATCH v2 0/6] Raspberry Pi 4 PCIe support

[Andrew Murray]

On Tue, Nov 19, 2019 at 12:49:24PM +0100, Nicolas Saenz Julienne wrote:
> On Tue, 2019-11-19 at 11:18 +0000, Andrew Murray wrote:
> > On Tue, Nov 12, 2019 at 04:59:19PM +0100, Nicolas Saenz Julienne wrote:
> > > This series aims at providing support for Raspberry Pi 4's PCIe
> > > controller, which is also shared with the Broadcom STB family of
> > > devices.
> > > 
> > > There was a previous attempt to upstream this some years ago[1] but was
> > > blocked as most STB PCIe integrations have a sparse DMA mapping[2] which
> > > is something currently not supported by the kernel.  Luckily this is not
> > > the case for the Raspberry Pi 4.
> > > 
> > > Note that the driver code is to be based on top of Rob Herring's series
> > > simplifying inbound and outbound range parsing.
> > > 
> > > [1] https://patchwork.kernel.org/cover/10605933/
> > > [2] https://patchwork.kernel.org/patch/10605957/
> > > 
> > 
> > What happened to patch 3? I can't see it on the list or in patchwork?
> 
> For some reason the script I use to call get_maintainer.sh or git send-mail
> failed to add linux-pci@vger.kernel.org and linux-kernel@vger.kernel.org as
> recipients. I didn't do anything different between v1 and v2 as far as mailing
> is concerned.
> 
> Nevertheless it's here: https://www.spinics.net/lists/arm-kernel/msg768461.html
> and should be present in the linux-arm-kernel list.
> 
> I'll look in to it and make sure this doesn't happen in v3.

No problem.

Thanks,

Andrew Murray

> 
> Regards,
> Nicolas
>