devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/7] Raspberry Pi 4 PCIe support
@ 2019-11-26  9:19 Nicolas Saenz Julienne
  2019-11-26  9:19 ` [PATCH v3 1/7] linux/log2.h: Add roundup/rounddown_pow_two64() family of functions Nicolas Saenz Julienne
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Nicolas Saenz Julienne @ 2019-11-26  9:19 UTC (permalink / raw)
  To: andrew.murray, maz, linux-kernel
  Cc: james.quinlan, mbrugger, f.fainelli, phil, wahrenst,
	jeremy.linton, linux-pci, linux-rpi-kernel,
	Nicolas Saenz Julienne, Robin Murphy, linux-arm-kernel,
	bcm-kernel-feedback-list, devicetree, linux-acpi, netdev,
	linux-rdma, linux-rockchip, iommu

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 v2:
  - Redo register access in driver avoiding indirection while keeping
    the naming intact
  - Add patch editing ARM64's config
  - Last MSI cleanups, notably removing MSIX flag
  - Got rid of all _RB writes
  - Got rid of all of_data
  - Overall churn removal
  - Address the rest of Andrew's comments

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 (4):
  linux/log2.h: Add roundup/rounddown_pow_two64() family of functions
  ARM: dts: bcm2711: Enable PCIe controller
  MAINTAINERS: Add brcmstb PCIe controller
  arm64: defconfig: Enable Broadcom's STB PCIe controller

 .../bindings/pci/brcm,stb-pcie.yaml           |   97 ++
 MAINTAINERS                                   |    4 +
 arch/arm/boot/dts/bcm2711.dtsi                |   41 +
 arch/arm64/configs/defconfig                  |    1 +
 drivers/acpi/arm64/iort.c                     |    2 +-
 drivers/net/ethernet/mellanox/mlx4/en_clock.c |    3 +-
 drivers/of/device.c                           |    2 +-
 drivers/pci/controller/Kconfig                |    9 +
 drivers/pci/controller/Makefile               |    1 +
 .../pci/controller/cadence/pcie-cadence-ep.c  |    7 +-
 drivers/pci/controller/cadence/pcie-cadence.c |    7 +-
 drivers/pci/controller/pcie-brcmstb.c         | 1012 +++++++++++++++++
 drivers/pci/controller/pcie-rockchip-ep.c     |    9 +-
 include/linux/log2.h                          |   52 +
 kernel/dma/direct.c                           |    3 +-
 15 files changed, 1230 insertions(+), 20 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
 create mode 100644 drivers/pci/controller/pcie-brcmstb.c

-- 
2.24.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v3 1/7] linux/log2.h: Add roundup/rounddown_pow_two64() family of functions
  2019-11-26  9:19 [PATCH v3 0/7] Raspberry Pi 4 PCIe support Nicolas Saenz Julienne
@ 2019-11-26  9:19 ` Nicolas Saenz Julienne
  2019-11-26 12:51   ` Leon Romanovsky
  2019-11-27 17:36   ` Nicolas Saenz Julienne
  2019-11-26  9:19 ` [PATCH v3 2/7] dt-bindings: PCI: Add bindings for brcmstb's PCIe device Nicolas Saenz Julienne
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 16+ messages in thread
From: Nicolas Saenz Julienne @ 2019-11-26  9:19 UTC (permalink / raw)
  To: andrew.murray, maz, linux-kernel, Lorenzo Pieralisi, Hanjun Guo,
	Sudeep Holla, Tariq Toukan, Rob Herring, Frank Rowand, Shawn Lin,
	Heiko Stuebner, Christoph Hellwig, Marek Szyprowski,
	Robin Murphy
  Cc: james.quinlan, mbrugger, f.fainelli, phil, wahrenst,
	jeremy.linton, linux-pci, linux-rpi-kernel,
	Nicolas Saenz Julienne, Rafael J. Wysocki, Len Brown,
	David S. Miller, Bjorn Helgaas, linux-acpi, linux-arm-kernel,
	netdev, linux-rdma, devicetree, linux-rockchip, iommu

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>

---

Changes since v2:
  - Use u64
  - Rename function to roundup/down_pow_two_u64()
  - Use 1ULL instead of 1UL
  - Include function usage in of/device.c and acpi/arm64/iort.c

 drivers/acpi/arm64/iort.c                     |  2 +-
 drivers/net/ethernet/mellanox/mlx4/en_clock.c |  3 +-
 drivers/of/device.c                           |  2 +-
 .../pci/controller/cadence/pcie-cadence-ep.c  |  7 +--
 drivers/pci/controller/cadence/pcie-cadence.c |  7 +--
 drivers/pci/controller/pcie-rockchip-ep.c     |  9 ++--
 include/linux/log2.h                          | 52 +++++++++++++++++++
 kernel/dma/direct.c                           |  3 +-
 8 files changed, 65 insertions(+), 20 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 33f71983e001..4809d3757d71 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -1090,7 +1090,7 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
 		 * firmware.
 		 */
 		end = dmaaddr + size - 1;
-		mask = DMA_BIT_MASK(ilog2(end) + 1);
+		mask = roundup_pow_of_two_u64(end) - 1;
 		dev->bus_dma_limit = end;
 		dev->coherent_dma_mask = mask;
 		*dev->dma_mask = mask;
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_clock.c b/drivers/net/ethernet/mellanox/mlx4/en_clock.c
index 024788549c25..ba5368e221f3 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_two_u64(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/of/device.c b/drivers/of/device.c
index e9127db7b067..418d7d014af1 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -149,7 +149,7 @@ int of_dma_configure(struct device *dev, struct device_node *np, bool force_dma)
 	 * set by the driver.
 	 */
 	end = dma_addr + size - 1;
-	mask = DMA_BIT_MASK(ilog2(end) + 1);
+	mask = roundup_pow_of_two_u64(end) - 1;
 	dev->coherent_dma_mask &= mask;
 	*dev->dma_mask &= mask;
 	/* ...but only set bus limit if we found valid dma-ranges earlier */
diff --git a/drivers/pci/controller/cadence/pcie-cadence-ep.c b/drivers/pci/controller/cadence/pcie-cadence-ep.c
index 1c173dad67d1..f6e37eb67c68 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-ep.c
+++ b/drivers/pci/controller/cadence/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"
 
@@ -61,11 +62,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_two_u64(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/cadence/pcie-cadence.c b/drivers/pci/controller/cadence/pcie-cadence.c
index cd795f6fc1e2..2ddda5ac60c4 100644
--- a/drivers/pci/controller/cadence/pcie-cadence.c
+++ b/drivers/pci/controller/cadence/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_two_u64(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..343f9683b540 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_two_u64(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_two_u64(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..0db47b78faa2 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_two_u64() - round 64bit value up to nearest power of two
+ * @n: value to round up
+ */
+static inline __attribute__((const)) u64 __roundup_pow_of_two_u64(u64 n)
+{
+	return 1ULL << fls64(n - 1);
+}
+
+/**
+ * __rounddown_pow_of_two_u64() - round 64bit value down to nearest power of two
+ * @n: value to round down
+ */
+static inline __attribute__((const)) u64 __rounddown_pow_of_two_u64(u64 n)
+{
+	return 1ULL << (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_two_u64 - 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_two_u64(n)			\
+(						\
+	__builtin_constant_p(n) ? (		\
+		(n == 1) ? 1 :			\
+		(1UL << (ilog2((n) - 1) + 1))	\
+				   ) :		\
+	__roundup_pow_of_two_u64(n)		\
+)
+
+/**
+ * rounddown_pow_of_two_u64 - 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_two_u64(n)		\
+(						\
+	__builtin_constant_p(n) ? (		\
+		(1UL << ilog2(n))) :		\
+	__rounddown_pow_of_two_u64(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 6af7ae83c4ad..8dd59e51b4dc 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_two_u64(max_dma) * 2 - 1;
 }
 
 static gfp_t __dma_direct_optimal_gfp_mask(struct device *dev, u64 dma_mask,
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 2/7] dt-bindings: PCI: Add bindings for brcmstb's PCIe device
  2019-11-26  9:19 [PATCH v3 0/7] Raspberry Pi 4 PCIe support Nicolas Saenz Julienne
  2019-11-26  9:19 ` [PATCH v3 1/7] linux/log2.h: Add roundup/rounddown_pow_two64() family of functions Nicolas Saenz Julienne
@ 2019-11-26  9:19 ` Nicolas Saenz Julienne
  2019-12-02 16:39   ` Rob Herring
  2019-11-26  9:19 ` [PATCH v3 3/7] ARM: dts: bcm2711: Enable PCIe controller Nicolas Saenz Julienne
  2019-11-26 21:50 ` [PATCH v3 0/7] Raspberry Pi 4 PCIe support Bjorn Helgaas
  3 siblings, 1 reply; 16+ messages in thread
From: Nicolas Saenz Julienne @ 2019-11-26  9:19 UTC (permalink / raw)
  To: andrew.murray, maz, linux-kernel, Florian Fainelli,
	bcm-kernel-feedback-list, Eric Anholt, Stefan Wahren,
	Bjorn Helgaas
  Cc: james.quinlan, mbrugger, phil, jeremy.linton, linux-pci,
	linux-rpi-kernel, Nicolas Saenz Julienne, Rob Herring,
	Mark Rutland, linux-arm-kernel, devicetree

From: Jim Quinlan <james.quinlan@broadcom.com>

The DT bindings description of the brcmstb PCIe device is described.
This node can only be used for now on the Raspberry Pi 4.

Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
Co-developed-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>

---

Changes since v2:
  - Add pci reference schema
  - Drop all default properties
  - Assume msi-controller and msi-parent are properly defined
  - Add num entries on multiple properties
  - use unevaluatedProperties
  - Update required properties
  - Fix license

Changes since v1:
  - Fix commit Subject
  - Remove linux,pci-domain

This was based on Jim's original submission[1], converted to yaml and
adapted to the RPi4 case.

[1] https://patchwork.kernel.org/patch/10605937/

 .../bindings/pci/brcm,stb-pcie.yaml           | 97 +++++++++++++++++++
 1 file changed, 97 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml

diff --git a/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
new file mode 100644
index 000000000000..77d3e81a437b
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
@@ -0,0 +1,97 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/brcm,stb-pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Brcmstb PCIe Host Controller Device Tree Bindings
+
+maintainers:
+  - Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
+
+allOf:
+  - $ref: /schemas/pci/pci-bus.yaml#
+
+properties:
+  compatible:
+    const: brcm,bcm2711-pcie # The Raspberry Pi 4
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    minItems: 1
+    maxItems: 2
+    items:
+      - description: PCIe host controller
+      - description: builtin MSI controller
+
+  interrupt-names:
+    minItems: 1
+    maxItems: 2
+    items:
+      - const: pcie
+      - const: msi
+
+  ranges:
+    maxItems: 1
+
+  dma-ranges:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    items:
+      - const: sw_pcie
+
+  msi-controller:
+    description: Identifies the node as an MSI controller.
+
+  msi-parent:
+    description: MSI controller the device is capable of using.
+
+  brcm,enable-ssc:
+    description: Indicates usage of spread-spectrum clocking.
+    type: boolean
+
+required:
+  - reg
+  - dma-ranges
+  - "#interrupt-cells"
+  - interrupts
+  - interrupt-names
+  - interrupt-map-mask
+  - interrupt-map
+  - msi-controller
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    scb {
+            #address-cells = <2>;
+            #size-cells = <1>;
+            pcie0: pcie@7d500000 {
+                    compatible = "brcm,bcm2711-pcie";
+                    reg = <0x0 0x7d500000 0x9310>;
+                    device_type = "pci";
+                    #address-cells = <3>;
+                    #size-cells = <2>;
+                    #interrupt-cells = <1>;
+                    interrupts = <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>,
+                                 <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>;
+                    interrupt-names = "pcie", "msi";
+                    interrupt-map-mask = <0x0 0x0 0x0 0x7>;
+                    interrupt-map = <0 0 0 1 &gicv2 GIC_SPI 143 IRQ_TYPE_LEVEL_HIGH>;
+                    msi-parent = <&pcie0>;
+                    msi-controller;
+                    ranges = <0x02000000 0x0 0xf8000000 0x6 0x00000000 0x0 0x04000000>;
+                    dma-ranges = <0x02000000 0x0 0x00000000 0x0 0x00000000 0x0 0x80000000>;
+                    brcm,enable-ssc;
+            };
+    };
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 3/7] ARM: dts: bcm2711: Enable PCIe controller
  2019-11-26  9:19 [PATCH v3 0/7] Raspberry Pi 4 PCIe support Nicolas Saenz Julienne
  2019-11-26  9:19 ` [PATCH v3 1/7] linux/log2.h: Add roundup/rounddown_pow_two64() family of functions Nicolas Saenz Julienne
  2019-11-26  9:19 ` [PATCH v3 2/7] dt-bindings: PCI: Add bindings for brcmstb's PCIe device Nicolas Saenz Julienne
@ 2019-11-26  9:19 ` Nicolas Saenz Julienne
  2019-11-26  9:37   ` Phil Elwell
  2019-11-26 21:50 ` [PATCH v3 0/7] Raspberry Pi 4 PCIe support Bjorn Helgaas
  3 siblings, 1 reply; 16+ messages in thread
From: Nicolas Saenz Julienne @ 2019-11-26  9:19 UTC (permalink / raw)
  To: andrew.murray, maz, linux-kernel, Rob Herring, Mark Rutland,
	Eric Anholt, Stefan Wahren
  Cc: james.quinlan, mbrugger, f.fainelli, phil, jeremy.linton,
	linux-pci, linux-rpi-kernel, Nicolas Saenz Julienne, devicetree,
	bcm-kernel-feedback-list, linux-arm-kernel

This enables bcm2711's PCIe bus, which is hardwired to a VIA
Technologies XHCI USB 3.0 controller.

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

---

This will likely need a rebase once the RPi GENET patches land.

Changes since v2:
  - Remove unused interrupt-map
  - correct dma-ranges to it's full size, non power of 2 bus DMA
    constraints now supported in linux-next[1]
  - add device_type
  - rename alias from pcie_0 to pcie0

Changes since v1:
  - remove linux,pci-domain

[1] https://lkml.org/lkml/2019/11/21/235

 arch/arm/boot/dts/bcm2711.dtsi | 41 ++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/arch/arm/boot/dts/bcm2711.dtsi b/arch/arm/boot/dts/bcm2711.dtsi
index 667658497898..2e121fc8b3d0 100644
--- a/arch/arm/boot/dts/bcm2711.dtsi
+++ b/arch/arm/boot/dts/bcm2711.dtsi
@@ -288,6 +288,47 @@ IRQ_TYPE_LEVEL_LOW)>,
 		arm,cpu-registers-not-fw-configured;
 	};
 
+	scb {
+		compatible = "simple-bus";
+		#address-cells = <2>;
+		#size-cells = <1>;
+
+		ranges = <0x0 0x7c000000  0x0 0xfc000000  0x03800000>,
+			 <0x6 0x00000000  0x6 0x00000000  0x40000000>;
+
+		pcie0: pcie@7d500000 {
+			compatible = "brcm,bcm2711-pcie";
+			reg = <0x0 0x7d500000 0x9310>;
+			device_type = "pci";
+			#address-cells = <3>;
+			#interrupt-cells = <1>;
+			#size-cells = <2>;
+			interrupts = <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "pcie", "msi";
+			interrupt-map-mask = <0x0 0x0 0x0 0x7>;
+			interrupt-map = <0 0 0 1 &gicv2 GIC_SPI 143
+							IRQ_TYPE_LEVEL_HIGH>;
+			msi-controller;
+			msi-parent = <&pcie0>;
+
+			ranges = <0x02000000 0x0 0xf8000000 0x6 0x00000000
+				  0x0 0x04000000>;
+			/*
+			 * The wrapper around the PCIe block has a bug
+			 * preventing it from accessing beyond the first 3GB of
+			 * memory. As the bus DMA mask is rounded up to the
+			 * closest power of two of the dma-range size, we're
+			 * forced to set the limit at 2GB. This can be
+			 * harmlessly changed in the future once the DMA code
+			 * handles non power of two DMA limits.
+			 */
+			dma-ranges = <0x02000000 0x0 0x00000000 0x0 0x00000000
+				      0x0 0xc0000000>;
+			brcm,enable-ssc;
+		};
+	};
+
 	cpus: cpus {
 		#address-cells = <1>;
 		#size-cells = <0>;
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 3/7] ARM: dts: bcm2711: Enable PCIe controller
  2019-11-26  9:19 ` [PATCH v3 3/7] ARM: dts: bcm2711: Enable PCIe controller Nicolas Saenz Julienne
@ 2019-11-26  9:37   ` Phil Elwell
  2019-11-26  9:43     ` Nicolas Saenz Julienne
  0 siblings, 1 reply; 16+ messages in thread
From: Phil Elwell @ 2019-11-26  9:37 UTC (permalink / raw)
  To: Nicolas Saenz Julienne, andrew.murray, maz, linux-kernel,
	Rob Herring, Mark Rutland, Eric Anholt, Stefan Wahren
  Cc: james.quinlan, mbrugger, f.fainelli, jeremy.linton, linux-pci,
	linux-rpi-kernel, devicetree, bcm-kernel-feedback-list,
	linux-arm-kernel

Hi Nicolas,

On 26/11/2019 09:19, Nicolas Saenz Julienne wrote:
> This enables bcm2711's PCIe bus, which is hardwired to a VIA
> Technologies XHCI USB 3.0 controller.
> 
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> 
> ---
> 
> This will likely need a rebase once the RPi GENET patches land.
> 
> Changes since v2:
>    - Remove unused interrupt-map
>    - correct dma-ranges to it's full size, non power of 2 bus DMA
>      constraints now supported in linux-next[1]
>    - add device_type
>    - rename alias from pcie_0 to pcie0
> 
> Changes since v1:
>    - remove linux,pci-domain
> 
> [1] https://lkml.org/lkml/2019/11/21/235
> 
>   arch/arm/boot/dts/bcm2711.dtsi | 41 ++++++++++++++++++++++++++++++++++
>   1 file changed, 41 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/bcm2711.dtsi b/arch/arm/boot/dts/bcm2711.dtsi
> index 667658497898..2e121fc8b3d0 100644
> --- a/arch/arm/boot/dts/bcm2711.dtsi
> +++ b/arch/arm/boot/dts/bcm2711.dtsi
> @@ -288,6 +288,47 @@ IRQ_TYPE_LEVEL_LOW)>,
>   		arm,cpu-registers-not-fw-configured;
>   	};
>   
> +	scb {
> +		compatible = "simple-bus";
> +		#address-cells = <2>;
> +		#size-cells = <1>;
> +
> +		ranges = <0x0 0x7c000000  0x0 0xfc000000  0x03800000>,
> +			 <0x6 0x00000000  0x6 0x00000000  0x40000000>;
> +
> +		pcie0: pcie@7d500000 {
> +			compatible = "brcm,bcm2711-pcie";
> +			reg = <0x0 0x7d500000 0x9310>;
> +			device_type = "pci";
> +			#address-cells = <3>;
> +			#interrupt-cells = <1>;
> +			#size-cells = <2>;
> +			interrupts = <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "pcie", "msi";
> +			interrupt-map-mask = <0x0 0x0 0x0 0x7>;
> +			interrupt-map = <0 0 0 1 &gicv2 GIC_SPI 143
> +							IRQ_TYPE_LEVEL_HIGH>;
> +			msi-controller;
> +			msi-parent = <&pcie0>;
> +
> +			ranges = <0x02000000 0x0 0xf8000000 0x6 0x00000000
> +				  0x0 0x04000000>;
> +			/*
> +			 * The wrapper around the PCIe block has a bug
> +			 * preventing it from accessing beyond the first 3GB of
> +			 * memory. As the bus DMA mask is rounded up to the
> +			 * closest power of two of the dma-range size, we're
> +			 * forced to set the limit at 2GB. This can be
> +			 * harmlessly changed in the future once the DMA code
> +			 * handles non power of two DMA limits.
> +			 */
> +			dma-ranges = <0x02000000 0x0 0x00000000 0x0 0x00000000
> +				      0x0 0xc0000000>;

The comment doesn't match the data here - I think for now the size field 
needs to be reduced to 2GB to match the comment.

Phil

> +			brcm,enable-ssc;
> +		};
> +	};
> +
>   	cpus: cpus {
>   		#address-cells = <1>;
>   		#size-cells = <0>;
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 3/7] ARM: dts: bcm2711: Enable PCIe controller
  2019-11-26  9:37   ` Phil Elwell
@ 2019-11-26  9:43     ` Nicolas Saenz Julienne
  0 siblings, 0 replies; 16+ messages in thread
From: Nicolas Saenz Julienne @ 2019-11-26  9:43 UTC (permalink / raw)
  To: Phil Elwell, andrew.murray, maz, linux-kernel, Rob Herring,
	Mark Rutland, Eric Anholt, Stefan Wahren
  Cc: james.quinlan, mbrugger, f.fainelli, jeremy.linton, linux-pci,
	linux-rpi-kernel, devicetree, bcm-kernel-feedback-list,
	linux-arm-kernel

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

On Tue, 2019-11-26 at 09:37 +0000, Phil Elwell wrote:
> Hi Nicolas,
> 
> On 26/11/2019 09:19, Nicolas Saenz Julienne wrote:
> > This enables bcm2711's PCIe bus, which is hardwired to a VIA
> > Technologies XHCI USB 3.0 controller.
> > 
> > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> > 
> > ---
> > 
> > This will likely need a rebase once the RPi GENET patches land.
> > 
> > Changes since v2:
> >    - Remove unused interrupt-map
> >    - correct dma-ranges to it's full size, non power of 2 bus DMA
> >      constraints now supported in linux-next[1]
> >    - add device_type
> >    - rename alias from pcie_0 to pcie0
> > 
> > Changes since v1:
> >    - remove linux,pci-domain
> > 
> > [1] https://lkml.org/lkml/2019/11/21/235
> > 
> >   arch/arm/boot/dts/bcm2711.dtsi | 41 ++++++++++++++++++++++++++++++++++
> >   1 file changed, 41 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/bcm2711.dtsi b/arch/arm/boot/dts/bcm2711.dtsi
> > index 667658497898..2e121fc8b3d0 100644
> > --- a/arch/arm/boot/dts/bcm2711.dtsi
> > +++ b/arch/arm/boot/dts/bcm2711.dtsi
> > @@ -288,6 +288,47 @@ IRQ_TYPE_LEVEL_LOW)>,
> >   		arm,cpu-registers-not-fw-configured;
> >   	};
> >   
> > +	scb {
> > +		compatible = "simple-bus";
> > +		#address-cells = <2>;
> > +		#size-cells = <1>;
> > +
> > +		ranges = <0x0 0x7c000000  0x0 0xfc000000  0x03800000>,
> > +			 <0x6 0x00000000  0x6 0x00000000  0x40000000>;
> > +
> > +		pcie0: pcie@7d500000 {
> > +			compatible = "brcm,bcm2711-pcie";
> > +			reg = <0x0 0x7d500000 0x9310>;
> > +			device_type = "pci";
> > +			#address-cells = <3>;
> > +			#interrupt-cells = <1>;
> > +			#size-cells = <2>;
> > +			interrupts = <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>,
> > +				     <GIC_SPI 148 IRQ_TYPE_LEVEL_HIGH>;
> > +			interrupt-names = "pcie", "msi";
> > +			interrupt-map-mask = <0x0 0x0 0x0 0x7>;
> > +			interrupt-map = <0 0 0 1 &gicv2 GIC_SPI 143
> > +							IRQ_TYPE_LEVEL_HIGH>;
> > +			msi-controller;
> > +			msi-parent = <&pcie0>;
> > +
> > +			ranges = <0x02000000 0x0 0xf8000000 0x6 0x00000000
> > +				  0x0 0x04000000>;
> > +			/*
> > +			 * The wrapper around the PCIe block has a bug
> > +			 * preventing it from accessing beyond the first 3GB of
> > +			 * memory. As the bus DMA mask is rounded up to the
> > +			 * closest power of two of the dma-range size, we're
> > +			 * forced to set the limit at 2GB. This can be
> > +			 * harmlessly changed in the future once the DMA code
> > +			 * handles non power of two DMA limits.
> > +			 */
> > +			dma-ranges = <0x02000000 0x0 0x00000000 0x0 0x00000000
> > +				      0x0 0xc0000000>;
> 
> The comment doesn't match the data here - I think for now the size field 
> needs to be reduced to 2GB to match the comment.

You're right, my bad, should've edited it out. The good part is that with this
commit[1], which will soon be in Linus' tree, we don't need to fake dma-ranges
size anymore.

So for the record, the comment should state the following:

	/*
	 * The wrapper around the PCIe block has a bug
	 * preventing it from accessing beyond the first 3GB of
	 * memory.
	 */

Regards,
Nicolas

[1] https://lkml.org/lkml/2019/11/21/235


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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 1/7] linux/log2.h: Add roundup/rounddown_pow_two64() family of functions
  2019-11-26  9:19 ` [PATCH v3 1/7] linux/log2.h: Add roundup/rounddown_pow_two64() family of functions Nicolas Saenz Julienne
@ 2019-11-26 12:51   ` Leon Romanovsky
  2019-11-27 18:06     ` Robin Murphy
  2019-11-27 17:36   ` Nicolas Saenz Julienne
  1 sibling, 1 reply; 16+ messages in thread
From: Leon Romanovsky @ 2019-11-26 12:51 UTC (permalink / raw)
  To: Nicolas Saenz Julienne
  Cc: andrew.murray, maz, linux-kernel, Lorenzo Pieralisi, Hanjun Guo,
	Sudeep Holla, Tariq Toukan, Rob Herring, Frank Rowand, Shawn Lin,
	Heiko Stuebner, Christoph Hellwig, Marek Szyprowski,
	Robin Murphy, james.quinlan, mbrugger, f.fainelli, phil,
	wahrenst, jeremy.linton, linux-pci, linux-rpi-kernel,
	Rafael J. Wysocki, Len Brown, David S. Miller, Bjorn Helgaas,
	linux-acpi, linux-arm-kernel, netdev, linux-rdma, devicetree,
	linux-rockchip, iommu

On Tue, Nov 26, 2019 at 10:19:39AM +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.

Is it possible to create general roundup/rounddown_pow_two() which will
work correctly for any type of variables, instead of creating special
variant for every type?

Thanks

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 0/7] Raspberry Pi 4 PCIe support
  2019-11-26  9:19 [PATCH v3 0/7] Raspberry Pi 4 PCIe support Nicolas Saenz Julienne
                   ` (2 preceding siblings ...)
  2019-11-26  9:19 ` [PATCH v3 3/7] ARM: dts: bcm2711: Enable PCIe controller Nicolas Saenz Julienne
@ 2019-11-26 21:50 ` Bjorn Helgaas
  2019-11-27 17:28   ` Nicolas Saenz Julienne
  3 siblings, 1 reply; 16+ messages in thread
From: Bjorn Helgaas @ 2019-11-26 21:50 UTC (permalink / raw)
  To: Nicolas Saenz Julienne
  Cc: andrew.murray, maz, linux-kernel, james.quinlan, mbrugger,
	f.fainelli, phil, wahrenst, jeremy.linton, linux-pci,
	linux-rpi-kernel, Robin Murphy, linux-arm-kernel,
	bcm-kernel-feedback-list, devicetree, linux-acpi, netdev,
	linux-rdma, linux-rockchip, iommu

On Tue, Nov 26, 2019 at 10:19:38AM +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.

> 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

Please update these subjects to match the others, i.e., capitalize
"Add".  Also, I think "Add MSI capability" really means "Add support
for MSI ..."; in PCIe terms the "MSI Capability" is a structure in
config space and it's there whether the OS supports it or not.

No need to repost just for this.

> Nicolas Saenz Julienne (4):
>   linux/log2.h: Add roundup/rounddown_pow_two64() family of functions
>   ARM: dts: bcm2711: Enable PCIe controller
>   MAINTAINERS: Add brcmstb PCIe controller
>   arm64: defconfig: Enable Broadcom's STB PCIe controller

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 0/7] Raspberry Pi 4 PCIe support
  2019-11-26 21:50 ` [PATCH v3 0/7] Raspberry Pi 4 PCIe support Bjorn Helgaas
@ 2019-11-27 17:28   ` Nicolas Saenz Julienne
  0 siblings, 0 replies; 16+ messages in thread
From: Nicolas Saenz Julienne @ 2019-11-27 17:28 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: andrew.murray, maz, linux-kernel, james.quinlan, mbrugger,
	f.fainelli, phil, wahrenst, jeremy.linton, linux-pci,
	linux-rpi-kernel, Robin Murphy, linux-arm-kernel,
	bcm-kernel-feedback-list, devicetree, linux-acpi, netdev,
	linux-rdma, linux-rockchip, iommu

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

Hi Bjorn,

On Tue, 2019-11-26 at 15:50 -0600, Bjorn Helgaas wrote:
> On Tue, Nov 26, 2019 at 10:19:38AM +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.
> > 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
> 
> Please update these subjects to match the others, i.e., capitalize
> "Add".  Also, I think "Add MSI capability" really means "Add support
> for MSI ..."; in PCIe terms the "MSI Capability" is a structure in
> config space and it's there whether the OS supports it or not.
> 
> No need to repost just for this.

Noted, I'll update them.

Regards,
Nicolas


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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 1/7] linux/log2.h: Add roundup/rounddown_pow_two64() family of functions
  2019-11-26  9:19 ` [PATCH v3 1/7] linux/log2.h: Add roundup/rounddown_pow_two64() family of functions Nicolas Saenz Julienne
  2019-11-26 12:51   ` Leon Romanovsky
@ 2019-11-27 17:36   ` Nicolas Saenz Julienne
  1 sibling, 0 replies; 16+ messages in thread
From: Nicolas Saenz Julienne @ 2019-11-27 17:36 UTC (permalink / raw)
  To: andrew.murray, maz, linux-kernel, Lorenzo Pieralisi, Hanjun Guo,
	Sudeep Holla, Tariq Toukan, Rob Herring, Frank Rowand, Shawn Lin,
	Heiko Stuebner, Christoph Hellwig, Marek Szyprowski,
	Robin Murphy
  Cc: james.quinlan, mbrugger, f.fainelli, phil, wahrenst,
	jeremy.linton, linux-pci, linux-rpi-kernel, Rafael J. Wysocki,
	Len Brown, David S. Miller, Bjorn Helgaas, linux-acpi,
	linux-arm-kernel, netdev, linux-rdma, devicetree, linux-rockchip,
	iommu

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

On Tue, 2019-11-26 at 10:19 +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>

Small Nit: I corrected the patch subject for next version.

linux/log2.h: Add roundup/rounddown_pow_two_u64() family of functions

Note the change here:                      ^^^^

Regards,
Nicolas


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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 1/7] linux/log2.h: Add roundup/rounddown_pow_two64() family of functions
  2019-11-26 12:51   ` Leon Romanovsky
@ 2019-11-27 18:06     ` Robin Murphy
  2019-11-27 18:24       ` Nicolas Saenz Julienne
  0 siblings, 1 reply; 16+ messages in thread
From: Robin Murphy @ 2019-11-27 18:06 UTC (permalink / raw)
  To: Leon Romanovsky, Nicolas Saenz Julienne
  Cc: andrew.murray, maz, linux-kernel, Lorenzo Pieralisi, Hanjun Guo,
	Sudeep Holla, Tariq Toukan, Rob Herring, Frank Rowand, Shawn Lin,
	Heiko Stuebner, Christoph Hellwig, Marek Szyprowski,
	james.quinlan, mbrugger, f.fainelli, phil, wahrenst,
	jeremy.linton, linux-pci, linux-rpi-kernel, Rafael J. Wysocki,
	Len Brown, David S. Miller, Bjorn Helgaas, linux-acpi,
	linux-arm-kernel, netdev, linux-rdma, devicetree, linux-rockchip,
	iommu

On 26/11/2019 12:51 pm, Leon Romanovsky wrote:
> On Tue, Nov 26, 2019 at 10:19:39AM +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.
> 
> Is it possible to create general roundup/rounddown_pow_two() which will
> work correctly for any type of variables, instead of creating special
> variant for every type?

In fact, that is sort of the case already - roundup_pow_of_two() itself 
wraps ilog2() such that the constant case *is* type-independent. And 
since ilog2() handles non-constant values anyway, might it be reasonable 
to just take the strongly-typed __roundup_pow_of_two() helper out of the 
loop as below?

Robin

----->8-----
diff --git a/include/linux/log2.h b/include/linux/log2.h
index 83a4a3ca3e8a..e825f8a6e8b5 100644
--- a/include/linux/log2.h
+++ b/include/linux/log2.h
@@ -172,11 +172,8 @@ unsigned long __rounddown_pow_of_two(unsigned long n)
   */
  #define roundup_pow_of_two(n)			\
  (						\
-	__builtin_constant_p(n) ? (		\
-		(n == 1) ? 1 :			\
-		(1UL << (ilog2((n) - 1) + 1))	\
-				   ) :		\
-	__roundup_pow_of_two(n)			\
+	(__builtin_constant_p(n) && (n == 1)) ?	\
+	1 : (1UL << (ilog2((n) - 1) + 1))	\
   )

  /**

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 1/7] linux/log2.h: Add roundup/rounddown_pow_two64() family of functions
  2019-11-27 18:06     ` Robin Murphy
@ 2019-11-27 18:24       ` Nicolas Saenz Julienne
  2019-11-27 19:06         ` Robin Murphy
  0 siblings, 1 reply; 16+ messages in thread
From: Nicolas Saenz Julienne @ 2019-11-27 18:24 UTC (permalink / raw)
  To: Robin Murphy, Leon Romanovsky
  Cc: andrew.murray, maz, linux-kernel, Lorenzo Pieralisi, Hanjun Guo,
	Sudeep Holla, Tariq Toukan, Rob Herring, Frank Rowand, Shawn Lin,
	Heiko Stuebner, Christoph Hellwig, Marek Szyprowski,
	james.quinlan, mbrugger, f.fainelli, phil, wahrenst,
	jeremy.linton, linux-pci, linux-rpi-kernel, Rafael J. Wysocki,
	Len Brown, David S. Miller, Bjorn Helgaas, linux-acpi,
	linux-arm-kernel, netdev, linux-rdma, devicetree, linux-rockchip,
	iommu

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

On Wed, 2019-11-27 at 18:06 +0000, Robin Murphy wrote:
> On 26/11/2019 12:51 pm, Leon Romanovsky wrote:
> > On Tue, Nov 26, 2019 at 10:19:39AM +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.
> > 
> > Is it possible to create general roundup/rounddown_pow_two() which will
> > work correctly for any type of variables, instead of creating special
> > variant for every type?
> 
> In fact, that is sort of the case already - roundup_pow_of_two() itself 
> wraps ilog2() such that the constant case *is* type-independent. And 
> since ilog2() handles non-constant values anyway, might it be reasonable 
> to just take the strongly-typed __roundup_pow_of_two() helper out of the 
> loop as below?
> 
> Robin
> 

That looks way better that's for sure. Some questions.

> ----->8-----
> diff --git a/include/linux/log2.h b/include/linux/log2.h
> index 83a4a3ca3e8a..e825f8a6e8b5 100644
> --- a/include/linux/log2.h
> +++ b/include/linux/log2.h
> @@ -172,11 +172,8 @@ unsigned long __rounddown_pow_of_two(unsigned long n)
>    */
>   #define roundup_pow_of_two(n)			\
>   (						\
> -	__builtin_constant_p(n) ? (		\
> -		(n == 1) ? 1 :			\
> -		(1UL << (ilog2((n) - 1) + 1))	\
> -				   ) :		\
> -	__roundup_pow_of_two(n)			\
> +	(__builtin_constant_p(n) && (n == 1)) ?	\
> +	1 : (1UL << (ilog2((n) - 1) + 1))	\

Then here you'd have to use ULL instead of UL, right? I want my 64bit value
everywhere regardless of the CPU arch. The downside is that would affect
performance to some extent (i.e. returning a 64bit value where you used to have
a 32bit one)?

Also, what about callers to this function on platforms with 32bit 'unsigned
longs' that happen to input a 64bit value into this. IIUC we'd have a change of
behaviour.

Regards,
Nicolas


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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 1/7] linux/log2.h: Add roundup/rounddown_pow_two64() family of functions
  2019-11-27 18:24       ` Nicolas Saenz Julienne
@ 2019-11-27 19:06         ` Robin Murphy
  2019-11-27 19:12           ` Leon Romanovsky
  2019-11-27 19:16           ` Nicolas Saenz Julienne
  0 siblings, 2 replies; 16+ messages in thread
From: Robin Murphy @ 2019-11-27 19:06 UTC (permalink / raw)
  To: Nicolas Saenz Julienne, Leon Romanovsky
  Cc: andrew.murray, maz, linux-kernel, Lorenzo Pieralisi, Hanjun Guo,
	Sudeep Holla, Tariq Toukan, Rob Herring, Frank Rowand, Shawn Lin,
	Heiko Stuebner, Christoph Hellwig, Marek Szyprowski,
	james.quinlan, mbrugger, f.fainelli, phil, wahrenst,
	jeremy.linton, linux-pci, linux-rpi-kernel, Rafael J. Wysocki,
	Len Brown, David S. Miller, Bjorn Helgaas, linux-acpi,
	linux-arm-kernel, netdev, linux-rdma, devicetree, linux-rockchip,
	iommu

On 27/11/2019 6:24 pm, Nicolas Saenz Julienne wrote:
> On Wed, 2019-11-27 at 18:06 +0000, Robin Murphy wrote:
>> On 26/11/2019 12:51 pm, Leon Romanovsky wrote:
>>> On Tue, Nov 26, 2019 at 10:19:39AM +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.
>>>
>>> Is it possible to create general roundup/rounddown_pow_two() which will
>>> work correctly for any type of variables, instead of creating special
>>> variant for every type?
>>
>> In fact, that is sort of the case already - roundup_pow_of_two() itself
>> wraps ilog2() such that the constant case *is* type-independent. And
>> since ilog2() handles non-constant values anyway, might it be reasonable
>> to just take the strongly-typed __roundup_pow_of_two() helper out of the
>> loop as below?
>>
>> Robin
>>
> 
> That looks way better that's for sure. Some questions.
> 
>> ----->8-----
>> diff --git a/include/linux/log2.h b/include/linux/log2.h
>> index 83a4a3ca3e8a..e825f8a6e8b5 100644
>> --- a/include/linux/log2.h
>> +++ b/include/linux/log2.h
>> @@ -172,11 +172,8 @@ unsigned long __rounddown_pow_of_two(unsigned long n)
>>     */
>>    #define roundup_pow_of_two(n)			\
>>    (						\
>> -	__builtin_constant_p(n) ? (		\
>> -		(n == 1) ? 1 :			\
>> -		(1UL << (ilog2((n) - 1) + 1))	\
>> -				   ) :		\
>> -	__roundup_pow_of_two(n)			\
>> +	(__builtin_constant_p(n) && (n == 1)) ?	\
>> +	1 : (1UL << (ilog2((n) - 1) + 1))	\
> 
> Then here you'd have to use ULL instead of UL, right? I want my 64bit value
> everywhere regardless of the CPU arch. The downside is that would affect
> performance to some extent (i.e. returning a 64bit value where you used to have
> a 32bit one)?

True, although it's possible that 1ULL might result in the same codegen 
if the compiler can see that the result is immediately truncated back to 
long anyway. Or at worst, I suppose "(typeof(n))1" could suffice, 
however ugly. Either way, this diff was only an illustration rather than 
a concrete proposal, but it might be an interesting diversion to 
investigate.

On that note, though, you should probably be using ULL in your current 
patch too.

> Also, what about callers to this function on platforms with 32bit 'unsigned
> longs' that happen to input a 64bit value into this. IIUC we'd have a change of
> behaviour.

Indeed, although the change in such a case would be "start getting the 
expected value instead of nonsense", so it might very well be welcome ;)

Robin.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 1/7] linux/log2.h: Add roundup/rounddown_pow_two64() family of functions
  2019-11-27 19:06         ` Robin Murphy
@ 2019-11-27 19:12           ` Leon Romanovsky
  2019-11-27 19:16           ` Nicolas Saenz Julienne
  1 sibling, 0 replies; 16+ messages in thread
From: Leon Romanovsky @ 2019-11-27 19:12 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Nicolas Saenz Julienne, andrew.murray, maz, linux-kernel,
	Lorenzo Pieralisi, Hanjun Guo, Sudeep Holla, Tariq Toukan,
	Rob Herring, Frank Rowand, Shawn Lin, Heiko Stuebner,
	Christoph Hellwig, Marek Szyprowski, james.quinlan, mbrugger,
	f.fainelli, phil, wahrenst, jeremy.linton, linux-pci,
	linux-rpi-kernel, Rafael J. Wysocki, Len Brown, David S. Miller,
	Bjorn Helgaas, linux-acpi, linux-arm-kernel, netdev, linux-rdma,
	devicetree, linux-rockchip, iommu

On Wed, Nov 27, 2019 at 07:06:12PM +0000, Robin Murphy wrote:
> On 27/11/2019 6:24 pm, Nicolas Saenz Julienne wrote:
> > On Wed, 2019-11-27 at 18:06 +0000, Robin Murphy wrote:
> > > On 26/11/2019 12:51 pm, Leon Romanovsky wrote:
> > > > On Tue, Nov 26, 2019 at 10:19:39AM +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.
> > > >
> > > > Is it possible to create general roundup/rounddown_pow_two() which will
> > > > work correctly for any type of variables, instead of creating special
> > > > variant for every type?
> > >
> > > In fact, that is sort of the case already - roundup_pow_of_two() itself
> > > wraps ilog2() such that the constant case *is* type-independent. And
> > > since ilog2() handles non-constant values anyway, might it be reasonable
> > > to just take the strongly-typed __roundup_pow_of_two() helper out of the
> > > loop as below?
> > >
> > > Robin
> > >
> >
> > That looks way better that's for sure. Some questions.
> >
> > > ----->8-----
> > > diff --git a/include/linux/log2.h b/include/linux/log2.h
> > > index 83a4a3ca3e8a..e825f8a6e8b5 100644
> > > --- a/include/linux/log2.h
> > > +++ b/include/linux/log2.h
> > > @@ -172,11 +172,8 @@ unsigned long __rounddown_pow_of_two(unsigned long n)
> > >     */
> > >    #define roundup_pow_of_two(n)			\
> > >    (						\
> > > -	__builtin_constant_p(n) ? (		\
> > > -		(n == 1) ? 1 :			\
> > > -		(1UL << (ilog2((n) - 1) + 1))	\
> > > -				   ) :		\
> > > -	__roundup_pow_of_two(n)			\
> > > +	(__builtin_constant_p(n) && (n == 1)) ?	\
> > > +	1 : (1UL << (ilog2((n) - 1) + 1))	\
> >
> > Then here you'd have to use ULL instead of UL, right? I want my 64bit value
> > everywhere regardless of the CPU arch. The downside is that would affect
> > performance to some extent (i.e. returning a 64bit value where you used to have
> > a 32bit one)?
>
> True, although it's possible that 1ULL might result in the same codegen if
> the compiler can see that the result is immediately truncated back to long
> anyway. Or at worst, I suppose "(typeof(n))1" could suffice, however ugly.
> Either way, this diff was only an illustration rather than a concrete
> proposal, but it might be an interesting diversion to investigate.
>
> On that note, though, you should probably be using ULL in your current patch
> too.
>
> > Also, what about callers to this function on platforms with 32bit 'unsigned
> > longs' that happen to input a 64bit value into this. IIUC we'd have a change of
> > behaviour.
>
> Indeed, although the change in such a case would be "start getting the
> expected value instead of nonsense", so it might very well be welcome ;)

Agree, if code overflowed with 32 bits before this change, the code was already
broken. At least now, it won't overflow.

>
> Robin.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 1/7] linux/log2.h: Add roundup/rounddown_pow_two64() family of functions
  2019-11-27 19:06         ` Robin Murphy
  2019-11-27 19:12           ` Leon Romanovsky
@ 2019-11-27 19:16           ` Nicolas Saenz Julienne
  1 sibling, 0 replies; 16+ messages in thread
From: Nicolas Saenz Julienne @ 2019-11-27 19:16 UTC (permalink / raw)
  To: Robin Murphy, Leon Romanovsky
  Cc: andrew.murray, maz, linux-kernel, Lorenzo Pieralisi, Hanjun Guo,
	Sudeep Holla, Tariq Toukan, Rob Herring, Frank Rowand, Shawn Lin,
	Heiko Stuebner, Christoph Hellwig, Marek Szyprowski,
	james.quinlan, mbrugger, f.fainelli, phil, wahrenst,
	jeremy.linton, linux-pci, linux-rpi-kernel, Rafael J. Wysocki,
	Len Brown, David S. Miller, Bjorn Helgaas, linux-acpi,
	linux-arm-kernel, netdev, linux-rdma, devicetree, linux-rockchip,
	iommu

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

On Wed, 2019-11-27 at 19:06 +0000, Robin Murphy wrote:
> On 27/11/2019 6:24 pm, Nicolas Saenz Julienne wrote:
> > On Wed, 2019-11-27 at 18:06 +0000, Robin Murphy wrote:
> > > On 26/11/2019 12:51 pm, Leon Romanovsky wrote:
> > > > On Tue, Nov 26, 2019 at 10:19:39AM +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.
> > > > 
> > > > Is it possible to create general roundup/rounddown_pow_two() which will
> > > > work correctly for any type of variables, instead of creating special
> > > > variant for every type?
> > > 
> > > In fact, that is sort of the case already - roundup_pow_of_two() itself
> > > wraps ilog2() such that the constant case *is* type-independent. And
> > > since ilog2() handles non-constant values anyway, might it be reasonable
> > > to just take the strongly-typed __roundup_pow_of_two() helper out of the
> > > loop as below?
> > > 
> > > Robin
> > > 
> > 
> > That looks way better that's for sure. Some questions.
> > 
> > > ----->8-----
> > > diff --git a/include/linux/log2.h b/include/linux/log2.h
> > > index 83a4a3ca3e8a..e825f8a6e8b5 100644
> > > --- a/include/linux/log2.h
> > > +++ b/include/linux/log2.h
> > > @@ -172,11 +172,8 @@ unsigned long __rounddown_pow_of_two(unsigned long n)
> > >     */
> > >    #define roundup_pow_of_two(n)			\
> > >    (						\
> > > -	__builtin_constant_p(n) ? (		\
> > > -		(n == 1) ? 1 :			\
> > > -		(1UL << (ilog2((n) - 1) + 1))	\
> > > -				   ) :		\
> > > -	__roundup_pow_of_two(n)			\
> > > +	(__builtin_constant_p(n) && (n == 1)) ?	\
> > > +	1 : (1UL << (ilog2((n) - 1) + 1))	\
> > 
> > Then here you'd have to use ULL instead of UL, right? I want my 64bit value
> > everywhere regardless of the CPU arch. The downside is that would affect
> > performance to some extent (i.e. returning a 64bit value where you used to
> > have
> > a 32bit one)?
> 
> True, although it's possible that 1ULL might result in the same codegen 
> if the compiler can see that the result is immediately truncated back to 
> long anyway. Or at worst, I suppose "(typeof(n))1" could suffice, 
> however ugly. Either way, this diff was only an illustration rather than 
> a concrete proposal, but it might be an interesting diversion to 
> investigate.
> 
> On that note, though, you should probably be using ULL in your current 
> patch too.

I actually meant to, the fix got lost. Thanks for pointing it out.

As I see Leon also likes this, I'll try out this implementation and come back
with some results.

Regards,
Nicolas


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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 2/7] dt-bindings: PCI: Add bindings for brcmstb's PCIe device
  2019-11-26  9:19 ` [PATCH v3 2/7] dt-bindings: PCI: Add bindings for brcmstb's PCIe device Nicolas Saenz Julienne
@ 2019-12-02 16:39   ` Rob Herring
  0 siblings, 0 replies; 16+ messages in thread
From: Rob Herring @ 2019-12-02 16:39 UTC (permalink / raw)
  To: Nicolas Saenz Julienne
  Cc: Andrew Murray, Marc Zyngier, linux-kernel, Florian Fainelli,
	maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE, Eric Anholt,
	Stefan Wahren, Bjorn Helgaas, james.quinlan, Matthias Brugger,
	Phil Elwell, Jeremy Linton, PCI,
	moderated list:BROADCOM BCM2835 ARM ARCHITECTURE, Mark Rutland,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	devicetree

On Tue, Nov 26, 2019 at 3:20 AM Nicolas Saenz Julienne
<nsaenzjulienne@suse.de> wrote:
>
> From: Jim Quinlan <james.quinlan@broadcom.com>
>
> The DT bindings description of the brcmstb PCIe device is described.
> This node can only be used for now on the Raspberry Pi 4.
>
> Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
> Co-developed-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
>
> ---
>
> Changes since v2:
>   - Add pci reference schema
>   - Drop all default properties
>   - Assume msi-controller and msi-parent are properly defined
>   - Add num entries on multiple properties
>   - use unevaluatedProperties
>   - Update required properties
>   - Fix license
>
> Changes since v1:
>   - Fix commit Subject
>   - Remove linux,pci-domain
>
> This was based on Jim's original submission[1], converted to yaml and
> adapted to the RPi4 case.
>
> [1] https://patchwork.kernel.org/patch/10605937/
>
>  .../bindings/pci/brcm,stb-pcie.yaml           | 97 +++++++++++++++++++
>  1 file changed, 97 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2019-12-02 16:39 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-26  9:19 [PATCH v3 0/7] Raspberry Pi 4 PCIe support Nicolas Saenz Julienne
2019-11-26  9:19 ` [PATCH v3 1/7] linux/log2.h: Add roundup/rounddown_pow_two64() family of functions Nicolas Saenz Julienne
2019-11-26 12:51   ` Leon Romanovsky
2019-11-27 18:06     ` Robin Murphy
2019-11-27 18:24       ` Nicolas Saenz Julienne
2019-11-27 19:06         ` Robin Murphy
2019-11-27 19:12           ` Leon Romanovsky
2019-11-27 19:16           ` Nicolas Saenz Julienne
2019-11-27 17:36   ` Nicolas Saenz Julienne
2019-11-26  9:19 ` [PATCH v3 2/7] dt-bindings: PCI: Add bindings for brcmstb's PCIe device Nicolas Saenz Julienne
2019-12-02 16:39   ` Rob Herring
2019-11-26  9:19 ` [PATCH v3 3/7] ARM: dts: bcm2711: Enable PCIe controller Nicolas Saenz Julienne
2019-11-26  9:37   ` Phil Elwell
2019-11-26  9:43     ` Nicolas Saenz Julienne
2019-11-26 21:50 ` [PATCH v3 0/7] Raspberry Pi 4 PCIe support Bjorn Helgaas
2019-11-27 17:28   ` Nicolas Saenz Julienne

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).