Add support for BCM2712 SD card controller

Signed-off-by Andrea della Porta <[email protected]>

---
 .../pinctrl/brcm,bcm2712-pinctrl.yaml         | 99 +++++++++++++++++++
 1 file changed, 99 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,bcm2712-pinctrl.yaml

Signed-off-by Andrea della Porta <[email protected]>

---
 .../pinctrl/brcm,bcm2712-pinctrl.yaml         | 99 +++++++++++++++++++
 1 file changed, 99 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,bcm2712-pinctrl.yaml

Documentation/devicetree/bindings/pinctrl/brcm,bcm2712-pinctrl.yaml new file +3 -0

new file mode 100644
index 000000000000..2908dfe99f3e
@@ -0,0 +1,99 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---

Documentation/devicetree/bindings/pinctrl/brcm,bcm2712-pinctrl.yaml new file +56 -0

+
+properties:
+  compatible:
+    enum:
+      - brcm,bcm2712-pinctrl
+      - brcm,bcm2712-aon-pinctrl
+      - brcm,bcm2712c0-pinctrl
+      - brcm,bcm2712c0-aon-pinctrl
+      - brcm,bcm2712d0-pinctrl
+      - brcm,bcm2712d0-aon-pinctrl
+
+  reg:
+    items:
+      - description: pin control registers
+
+allOf:
+  - $ref: pinctrl.yaml#
+
+required:
+  - compatible
+  - reg
+
+additionalProperties:
+  anyOf:
+    - type: object  
+      allOf:
+        - $ref: pincfg-node.yaml#
+        - $ref: pinmux-node.yaml#
+
+      properties:
+        function: 
+          enum: [ gpio, alt1, alt2, alt3, alt4, alt5, alt6, alt7, alt8,
+                 aon_cpu_standbyb, aon_fp_4sec_resetb, aon_gpclk, aon_pwm,
+                 arm_jtag, aud_fs_clk0, avs_pmu_bsc, bsc_m0, bsc_m1, bsc_m2,
+                 bsc_m3, clk_observe, ctl_hdmi_5v, enet0, enet0_mii,
+                 enet0_rgmii, ext_sc_clk, fl0, fl1, gpclk0, gpclk1, gpclk2,
+                 hdmi_tx0_auto_i2c, hdmi_tx0_bsc, hdmi_tx1_auto_i2c,
+                 hdmi_tx1_bsc, i2s_in, i2s_out, ir_in, mtsif, mtsif_alt,
+                 mtsif_alt1, pdm, pkt, pm_led_out, sc0, sd0, sd2, sd_card_a,
+                 sd_card_b, sd_card_c, sd_card_d, sd_card_e, sd_card_f,
+                 sd_card_g, spdif_out, spi_m, spi_s, sr_edm_sense, te0, te1,
+                 tsio, uart0, uart1, uart2, usb_pwr, usb_vbus, uui, vc_i2c0,
+                 vc_i2c3, vc_i2c4, vc_i2c5, vc_i2csl, vc_pcm, vc_pwm0,
+                 vc_pwm1, vc_spi0, vc_spi3, vc_spi4, vc_spi5, vc_uart0,
+                 vc_uart2, vc_uart3, vc_uart4 ]
+
+        pins:
+          items:
+            pattern: "^((aon_)?s?gpio[0-6]?[0-9])|(emmc_(clk|cmd|dat[0-7]|ds))$"
+
+        bias-disable: true
+        bias-pull-down: true
+        bias-pull-up: true
+      additionalProperties: false
+
+    - type: object

Documentation/devicetree/bindings/pinctrl/brcm,bcm2712-pinctrl.yaml new file +6 -0

+          pin_rts {
+            function = "uart0";
+            pins = "gpio24";
+            bias-disable;
+        };
+

Signed-off-by Andrea della Porta <[email protected]>

---
  .../pinctrl/brcm,bcm2712-pinctrl.yaml         | 99 +++++++++++++++++++
  1 file changed, 99 insertions(+)

create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,bcm2712-pinctrl.yaml

Documentation/devicetree/bindings/pinctrl/brcm,bcm2712-pinctrl.yaml new file +0 -0

new file mode 100644
index 000000000000..2908dfe99f3e

Quoted 3 lines · Andrea della Porta

Signed-off-by Andrea della Porta <[email protected]>

+++ b/Documentation/devicetree/bindings/pinctrl/brcm,bcm2712-pinctrl.yaml

Quoted 1 line

+$id: http://devicetree.org/schemas/pinctrl/brcm,bcm2712-pinctrl.yaml#

Quoted 1 line

+title: Broadcom BCM2712 pin controller

This is not strictly speaking BCM2712 specific, the pin controller you describe is a Broadcom STB product line pin controller.

Please describe it as such as and make BCM2712 a specific instance of the chip using that pin controller, see more comments on patch #4.

Quoted 12 lines · Andrea della Porta

Signed-off-by Andrea della Porta <[email protected]>

---
  .../pinctrl/brcm,bcm2712-pinctrl.yaml         | 99 +++++++++++++++++++
  1 file changed, 99 insertions(+)

create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,bcm2712-pinctrl.yaml

Documentation/devicetree/bindings/pinctrl/brcm,bcm2712-pinctrl.yaml new file +0 -0

new file mode 100644
index 000000000000..2908dfe99f3e

Documentation/devicetree/bindings/pinctrl/brcm,bcm2712-pinctrl.yaml line 7

Quoted 7 lines

Documentation/devicetree/bindings/pinctrl/brcm,bcm2712-pinctrl.yaml new file +7 -0

@@ -0,0 +1,99 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/brcm,bcm2712-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom BCM2712 pin controller

This is not strictly speaking BCM2712 specific, the pin controller you describe is a Broadcom STB product line pin controller.

Please describe it as such as and make BCM2712 a specific instance of the chip using that pin controller, see more comments on patch #4.

Signature · 1 line

Florian

Signed-off-by Andrea della Porta <[email protected]>

---
 .../bindings/mmc/brcm,sdhci-brcmstb.yaml      | 51 ++++++++++++++++++-
 1 file changed, 50 insertions(+), 1 deletion(-)

Signed-off-by Andrea della Porta <[email protected]>

---
 .../bindings/mmc/brcm,sdhci-brcmstb.yaml      | 51 ++++++++++++++++++-
 1 file changed, 50 insertions(+), 1 deletion(-)

Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml +6 -1

index cbd3d6c6c77f..6aa137d78e4f 100644
@@ -13,6 +13,7 @@ maintainers:
 properties:
   compatible:
     oneOf:
+      - const: brcm,bcm2712-sdhci
       - items:
           - enum:
               - brcm,bcm7216-sdhci

@@ -26,12 +27,16 @@ properties:
           - const: brcm,sdhci-brcmstb
 
   reg:
-    maxItems: 2
+    minItems: 2
+    maxItems: 4
 
   reg-names:
+    minItems: 2
     items:
       - const: host
       - const: cfg
+      - const: busisol
+      - const: lcpll
 
   interrupts:
     maxItems: 1

Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml +2 -0

   - if:
       properties:

@@ -71,6 +77,28 @@ allOf:
       required:
         - clock-frequency
 
+  - if:
+      properties:

Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml +14 -0

+
+    else:
+      properties:
+        reg:
+          minItems: 2
+          maxItems: 2
+        reg-names:
+          minItems: 2
+          maxItems: 2
+
 required:
   - compatible
   - reg

@@ -114,3 +142,24 @@ examples:
       clocks = <&scmi_clk 245>;
       clock-names = "sw_sdio";
     };
+
+  - |
+    soc {
+      #address-cells = <2>;

Signed-off-by Andrea della Porta <[email protected]>

---
  .../bindings/mmc/brcm,sdhci-brcmstb.yaml      | 51 ++++++++++++++++++-
  1 file changed, 50 insertions(+), 1 deletion(-)

Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml +30 -1

index cbd3d6c6c77f..6aa137d78e4f 100644
@@ -13,6 +13,7 @@ maintainers:
  properties:
    compatible:
      oneOf:
+      - const: brcm,bcm2712-sdhci
        - items:
            - enum:
                - brcm,bcm7216-sdhci

@@ -26,12 +27,16 @@ properties:
            - const: brcm,sdhci-brcmstb
 
    reg:
-    maxItems: 2
+    minItems: 2
+    maxItems: 4
 
    reg-names:
+    minItems: 2
      items:
        - const: host
        - const: cfg
+      - const: busisol
+      - const: lcpll
 
    interrupts:
      maxItems: 1

@@ -60,6 +65,7 @@ properties:
      description: Specifies that controller should use auto CMD12
 
  allOf:
+  - $ref: sdhci-common.yaml
    - $ref: mmc-controller.yaml#
    - if:
        properties:

@@ -71,6 +77,28 @@ allOf:
        required:
          - clock-frequency
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: brcm,bcm2712-sdhci
+
+    then:
+      properties:
+        reg:
+          maxItems: 4
+        clock-names:
+         const: "sw_sdio"
+
+    else:
+      properties:
+        reg:
+          minItems: 2
+          maxItems: 2
+        reg-names:
+          minItems: 2
+          maxItems: 2
+
  required:
    - compatible
    - reg

@@ -114,3 +142,24 @@ examples:
        clocks = <&scmi_clk 245>;
        clock-names = "sw_sdio";
      };
+

Signed-off-by Andrea della Porta <[email protected]>

---
 arch/arm64/boot/dts/broadcom/Makefile         |   1 +
 .../boot/dts/broadcom/bcm2712-rpi-5-b.dts     | 313 +++++++
 arch/arm64/boot/dts/broadcom/bcm2712-rpi.dtsi |  81 ++
 arch/arm64/boot/dts/broadcom/bcm2712.dtsi     | 841 ++++++++++++++++++
 4 files changed, 1236 insertions(+)
 create mode 100644 arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts
 create mode 100644 arch/arm64/boot/dts/broadcom/bcm2712-rpi.dtsi
 create mode 100644 arch/arm64/boot/dts/broadcom/bcm2712.dtsi

arch/arm64/boot/dts/broadcom/Makefile +1 -0

index 8b4591ddd27c..92565e9781ad 100644
@@ -6,6 +6,7 @@ DTC_FLAGS := -@
 dtb-$(CONFIG_ARCH_BCM2835) += bcm2711-rpi-400.dtb \
 			      bcm2711-rpi-4-b.dtb \
 			      bcm2711-rpi-cm4-io.dtb \
+			      bcm2712-rpi-5-b.dtb \
 			      bcm2837-rpi-3-a-plus.dtb \
 			      bcm2837-rpi-3-b.dtb \
 			      bcm2837-rpi-3-b-plus.dtb \

arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts new file +8 -0

new file mode 100644
index 000000000000..2ce180a54e5b
@@ -0,0 +1,313 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/pwm/pwm.h>
+#include <dt-bindings/reset/raspberrypi,firmware-reset.h>
+

arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts new file +10 -0

+	model = "Raspberry Pi 5";
+
+	/* Will be filled by the bootloader */
+	memory@0 {
+		device_type = "memory";
+		reg = <0 0 0x28000000>;
+	};
+
+	leds: leds {
+		compatible = "gpio-leds";

arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts new file +8 -0

+	};
+
+	wl_on_reg: wl_on_reg {
+		compatible = "regulator-fixed";
+		regulator-name = "wl-on-regulator";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		pinctrl-0 = <&wl_on_pins>;

arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts new file +26 -0

+		function = "sd_card_g";
+		pins = "aon_gpio5";
+		bias-pull-up;
+	};
+
+	/* Slight hack - only one PWM pin (status LED) is usable */
+	aon_pwm_1pin: aon_pwm_1pin {
+		function = "aon_pwm";
+		pins = "aon_gpio9";
+	};
+};
+
+&pinctrl {
+	pwr_button_pins: pwr_button_pins {
+		function = "gpio";
+		pins = "gpio20";
+		bias-pull-up;
+	};
+
+	wl_on_pins: wl_on_pins {
+		function = "gpio";
+		pins = "gpio28";
+	};
+
+	bt_shutdown_pins: bt_shutdown_pins {
+		function = "gpio";

arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts new file +4 -0

+
+		// Pad bank0 out to 32 entries
+		"", "", "", "", "", "", "", "", "", "", "", "", "", "", "",
+

arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts new file +4 -0

+		blconfig = &blconfig;
+		blpubkey = &blpubkey;
+		console = &uart10;
+		mailbox = &mailbox;

arch/arm64/boot/dts/broadcom/bcm2712-rpi.dtsi new file +22 -0

@@ -0,0 +1,81 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <dt-bindings/power/raspberrypi-power.h>
+
+&soc {
+	firmware: firmware {
+		compatible = "raspberrypi,bcm2835-firmware", "simple-mfd";
 
 
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		mboxes = <&mailbox>;
+		dma-ranges;
+
+		firmware_clocks: clocks {
+			compatible = "raspberrypi,firmware-clocks";
+			#clock-cells = <1>;
+		};
+
+		reset: reset {
+			compatible = "raspberrypi,firmware-reset";
+			#reset-cells = <1>;
+		};

arch/arm64/boot/dts/broadcom/bcm2712-rpi.dtsi new file +30 -0

+	};
+};
+
+&rmem {
+	/*
+	 * RPi5's co-processor will copy the board's bootloader configuration
+	 * into memory for the OS to consume. It'll also update this node with
+	 * its placement information.
+	 */
+	blconfig: nvram@0 {
+		compatible = "raspberrypi,bootloader-config", "nvmem-rmem";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		reg = <0x0 0x0 0x0>;
+		no-map;
+		status = "disabled";
+	};
+	/*
+	 * RPi5 will copy the binary public key blob (if present) from the bootloader
+	 * into memory for use by the OS.
+	 */
+	blpubkey: nvram@1 {
+		compatible = "raspberrypi,bootloader-public-key", "nvmem-rmem";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		reg = <0x0 0x0 0x0>;
+		no-map;
+		status = "disabled";
+	};
+};

arch/arm64/boot/dts/broadcom/bcm2712.dtsi new file +0 -0

new file mode 100644
index 000000000000..fd5a19f68b49

arch/arm64/boot/dts/broadcom/bcm2712.dtsi new file +52 -0

+		#clock-cells = <0>;
+		compatible = "fixed-clock";
+		clock-frequency = <27000000>;
+		clock-output-names = "27MHz-clock";
+	};
+
+	clk_108MHz: clk-108M {
+		#clock-cells = <0>;
+		compatible = "fixed-clock";
+		clock-frequency = <108000000>;
+		clock-output-names = "108MHz-clock";
+	};
+
+	soc: soc {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		ranges     = <0x7c000000  0x10 0x7c000000  0x04000000>;
+		/* Emulate a contiguous 30-bit address range for DMA */
+		dma-ranges = <0xc0000000  0x00 0x00000000  0x40000000>,
+			     <0x7c000000  0x10 0x7c000000  0x04000000>;
+
+		system_timer: timer@7c003000 {
+			compatible = "brcm,bcm2835-system-timer";
+			reg = <0x7c003000 0x1000>;
+			interrupts = <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>,
+		     		     <GIC_SPI 66 IRQ_TYPE_LEVEL_HIGH>,
+		     		     <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>;
+			clock-frequency = <1000000>;
+		};
+
+		mailbox: mailbox@7c013880 {
+			compatible = "brcm,bcm2835-mbox";
+			reg = <0x7c013880 0x40>;
+			interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
+			#mbox-cells = <0>;
+		};
+
+		disp_intr: interrupt-controller@7c502000 {
+			compatible = "brcm,bcm2711-l2-intc", "brcm,l2-intc";
+			reg = <0x7c502000 0x30>;
+			interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-controller;
+			#interrupt-cells = <1>;
+			status = "disabled";
+		};
+
+		dvp: clock@7c700000 {
+			compatible = "brcm,brcm2711-dvp";
+			reg = <0x7c700000 0x10>;

arch/arm64/boot/dts/broadcom/bcm2712.dtsi new file +31 -0

+			compatible = "brcm,bcm2836-l1-intc";
+			reg = <0x7cd00000 0x100>;
+		};
+
+		uart0: serial@7d001000 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0x7d001000 0x200>;
+			interrupts = <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clk_uart>,
+				 <&clk_vpu>;
+			clock-names = "uartclk", "apb_pclk";
+			arm,primecell-periphid = <0x00241011>;
+			status = "disabled";
+		};
+
+		uart2: serial@7d001400 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0x7d001400 0x200>;
+			interrupts = <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clk_uart>,
+				 <&clk_vpu>;
+			clock-names = "uartclk", "apb_pclk";
+			arm,primecell-periphid = <0x00241011>;
+			status = "disabled";
+		};
+
+		uart5: serial@7d001a00 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0x7d001a00 0x200>;
+			interrupts = <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clk_uart>,

Signed-off-by Andrea della Porta <[email protected]>

Quoted 1 line · Andrea della Porta

Signed-off-by Andrea della Porta <[email protected]>

No commit message given the amount of lines changed?

Please split this patch into multiple series that add basic 2712 support to the mainline kernel.

Signature · 1 line

Florian

Signed-off-by Andrea della Porta <[email protected]>

---
  arch/arm64/boot/dts/broadcom/Makefile         |   1 +
  .../boot/dts/broadcom/bcm2712-rpi-5-b.dts     | 313 +++++++
  arch/arm64/boot/dts/broadcom/bcm2712-rpi.dtsi |  81 ++
  arch/arm64/boot/dts/broadcom/bcm2712.dtsi     | 841 ++++++++++++++++++
  4 files changed, 1236 insertions(+)

create mode 100644 arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts create mode 100644 arch/arm64/boot/dts/broadcom/bcm2712-rpi.dtsi create mode 100644 arch/arm64/boot/dts/broadcom/bcm2712.dtsi

arch/arm64/boot/dts/broadcom/Makefile +1 -0

index 8b4591ddd27c..92565e9781ad 100644
@@ -6,6 +6,7 @@ DTC_FLAGS := -@
  dtb-$(CONFIG_ARCH_BCM2835) += bcm2711-rpi-400.dtb \
  			      bcm2711-rpi-4-b.dtb \
  			      bcm2711-rpi-cm4-io.dtb \
+			      bcm2712-rpi-5-b.dtb \
  			      bcm2837-rpi-3-a-plus.dtb \
  			      bcm2837-rpi-3-b.dtb \
  			      bcm2837-rpi-3-b-plus.dtb \

arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts new file +10 -0

new file mode 100644
index 000000000000..2ce180a54e5b
@@ -0,0 +1,313 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/pwm/pwm.h>
+#include <dt-bindings/reset/raspberrypi,firmware-reset.h>
+
+#define spi0 _spi0
+#define uart0 _uart0

Hi Phil,

Am 14.04.24 um 00:14 schrieb Andrea della Porta:

Quoted 40 lines

Signed-off-by Andrea della Porta <[email protected]>

---
  arch/arm64/boot/dts/broadcom/Makefile         |   1 +
  .../boot/dts/broadcom/bcm2712-rpi-5-b.dts     | 313 +++++++
  arch/arm64/boot/dts/broadcom/bcm2712-rpi.dtsi |  81 ++
  arch/arm64/boot/dts/broadcom/bcm2712.dtsi     | 841 ++++++++++++++++++
  4 files changed, 1236 insertions(+)

create mode 100644 arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts create mode 100644 arch/arm64/boot/dts/broadcom/bcm2712-rpi.dtsi create mode 100644 arch/arm64/boot/dts/broadcom/bcm2712.dtsi

arch/arm64/boot/dts/broadcom/Makefile +1 -0

index 8b4591ddd27c..92565e9781ad 100644
@@ -6,6 +6,7 @@ DTC_FLAGS := -@
  dtb-$(CONFIG_ARCH_BCM2835) += bcm2711-rpi-400.dtb \
                            bcm2711-rpi-4-b.dtb \
                            bcm2711-rpi-cm4-io.dtb \
+                           bcm2712-rpi-5-b.dtb \
                            bcm2837-rpi-3-a-plus.dtb \
                            bcm2837-rpi-3-b.dtb \
                            bcm2837-rpi-3-b-plus.dtb \

arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts new file +10 -0

new file mode 100644
index 000000000000..2ce180a54e5b
@@ -0,0 +1,313 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/pwm/pwm.h>
+#include <dt-bindings/reset/raspberrypi,firmware-reset.h>
+
+#define spi0 _spi0
+#define uart0 _uart0

arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts line 20

Quoted 10 lines

arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts new file +10 -0

+
+#include "bcm2712.dtsi"
+
+#undef spi0
+#undef uart0
+
+/ {
+     compatible = "raspberrypi,5-model-b", "brcm,bcm2712";
+     model = "Raspberry Pi 5";
+

according to this downstream commit [1] it's just called "Raspberry Pi 5" without Model B, but the filename and the compatible says something different. Is there still a chance to get this consistent or is it too late because the firmware expect the compatible?

[1] -
https://github.com/raspberrypi/linux/commit/99e359d2f2da2c820fd2a30b1ad08b32c9549adb

Stefan,

Quoted 58 lines · Stefan Wahren

Hi Phil,

Am 14.04.24 um 00:14 schrieb Andrea della Porta:

Quoted 40 lines

Signed-off-by Andrea della Porta <[email protected]>

---
   arch/arm64/boot/dts/broadcom/Makefile         |   1 +
   .../boot/dts/broadcom/bcm2712-rpi-5-b.dts     | 313 +++++++
   arch/arm64/boot/dts/broadcom/bcm2712-rpi.dtsi |  81 ++
   arch/arm64/boot/dts/broadcom/bcm2712.dtsi     | 841 ++++++++++++++++++
   4 files changed, 1236 insertions(+)

create mode 100644 arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts create mode 100644 arch/arm64/boot/dts/broadcom/bcm2712-rpi.dtsi create mode 100644 arch/arm64/boot/dts/broadcom/bcm2712.dtsi

arch/arm64/boot/dts/broadcom/Makefile +1 -0

index 8b4591ddd27c..92565e9781ad 100644
@@ -6,6 +6,7 @@ DTC_FLAGS := -@
   dtb-$(CONFIG_ARCH_BCM2835) += bcm2711-rpi-400.dtb \
                             bcm2711-rpi-4-b.dtb \
                             bcm2711-rpi-cm4-io.dtb \
+                           bcm2712-rpi-5-b.dtb \
                             bcm2837-rpi-3-a-plus.dtb \
                             bcm2837-rpi-3-b.dtb \
                             bcm2837-rpi-3-b-plus.dtb \

arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts new file +10 -0

new file mode 100644
index 000000000000..2ce180a54e5b
@@ -0,0 +1,313 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/pwm/pwm.h>
+#include <dt-bindings/reset/raspberrypi,firmware-reset.h>
+
+#define spi0 _spi0
+#define uart0 _uart0

arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts line 20

Quoted 10 lines

arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts new file +10 -0

+
+#include "bcm2712.dtsi"
+
+#undef spi0
+#undef uart0
+
+/ {
+     compatible = "raspberrypi,5-model-b", "brcm,bcm2712";
+     model = "Raspberry Pi 5";
+

according to this downstream commit [1] it's just called "Raspberry Pi 5" without Model B, but the filename and the compatible says something different. Is there still a chance to get this consistent or is it too late because the firmware expect the compatible?

Quoted 2 lines

[1] -
https://github.com/raspberrypi/linux/commit/99e359d2f2da2c820fd2a30b1ad08b32c9549adb

Nothing cares about the compatible string, but the product name was changed too late for the firmware, which expects the current DTB file name.

I'm happy with the naming as it stands, since we use Pi 4 to
refer to all the BCM2711-based devices, and Pi 5 can include CM5.

Phil

Hi Phil,

Am 15.04.24 um 10:52 schrieb Phil Elwell:

Quoted 62 lines

Stefan,

Quoted 58 lines · Stefan Wahren

Hi Phil,

Am 14.04.24 um 00:14 schrieb Andrea della Porta:

Quoted 40 lines

Signed-off-by Andrea della Porta <[email protected]>

---
   arch/arm64/boot/dts/broadcom/Makefile         |   1 +
   .../boot/dts/broadcom/bcm2712-rpi-5-b.dts     | 313 +++++++
   arch/arm64/boot/dts/broadcom/bcm2712-rpi.dtsi |  81 ++
   arch/arm64/boot/dts/broadcom/bcm2712.dtsi     | 841 ++++++++++++++++++
   4 files changed, 1236 insertions(+)

create mode 100644 arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts create mode 100644 arch/arm64/boot/dts/broadcom/bcm2712-rpi.dtsi create mode 100644 arch/arm64/boot/dts/broadcom/bcm2712.dtsi

arch/arm64/boot/dts/broadcom/Makefile +1 -0

index 8b4591ddd27c..92565e9781ad 100644
@@ -6,6 +6,7 @@ DTC_FLAGS := -@
   dtb-$(CONFIG_ARCH_BCM2835) += bcm2711-rpi-400.dtb \
                             bcm2711-rpi-4-b.dtb \
                             bcm2711-rpi-cm4-io.dtb \
+                           bcm2712-rpi-5-b.dtb \
                             bcm2837-rpi-3-a-plus.dtb \
                             bcm2837-rpi-3-b.dtb \
                             bcm2837-rpi-3-b-plus.dtb \

arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts new file +10 -0

new file mode 100644
index 000000000000..2ce180a54e5b
@@ -0,0 +1,313 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/pwm/pwm.h>
+#include <dt-bindings/reset/raspberrypi,firmware-reset.h>
+
+#define spi0 _spi0
+#define uart0 _uart0

arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts line 20

Quoted 10 lines

arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts new file +10 -0

+
+#include "bcm2712.dtsi"
+
+#undef spi0
+#undef uart0
+
+/ {
+     compatible = "raspberrypi,5-model-b", "brcm,bcm2712";
+     model = "Raspberry Pi 5";
+

according to this downstream commit [1] it's just called "Raspberry Pi 5" without Model B, but the filename and the compatible says something different. Is there still a chance to get this consistent or is it too late because the firmware expect the compatible?

Quoted 5 lines

Quoted 2 lines

[1] -
https://github.com/raspberrypi/linux/commit/99e359d2f2da2c820fd2a30b1ad08b32c9549adb

Nothing cares about the compatible string, but the product name was changed too late for the firmware, which expects the current DTB file name.

should i send a pull request to address the compatible? This would avoid a little bit confusion in the upstreaming process, because devicetree/bindings/arm/bcm/bcm2835.yaml needs to be updated as well.

Add a pincontrol driver for BCM2712. BCM2712 allows muxing GPIOs and setting configuration on pads.

Originally-by Jonathan Bell <[email protected]>

Originally-by Phil Elwell <[email protected]>

Signed-off-by Andrea della Porta <[email protected]>

---
  drivers/pinctrl/bcm/Kconfig           |    9 +
  drivers/pinctrl/bcm/Makefile          |    1 +
  drivers/pinctrl/bcm/pinctrl-bcm2712.c | 1247 +++++++++++++++++++++++++
  3 files changed, 1257 insertions(+)

create mode 100644 drivers/pinctrl/bcm/pinctrl-bcm2712.c

drivers/pinctrl/bcm/pinctrl-bcm2712.c new file +6 -0

+	/* every pin can do every function */
+	*groups = pc->gpio_groups;
+	*num_groups = pc->pctl_desc.npins;
+
+	return 0;
+}

drivers/pinctrl/bcm/pinctrl-bcm2712.c new file +14 -0

+static int bcm2712_pinconf_get(struct pinctrl_dev *pctldev,
+			unsigned pin, unsigned long *config)
+{
+	struct bcm2712_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev);
+	enum pin_config_param param = pinconf_to_config_param(*config);
+	u32 arg;
+
+	switch (param) {
+	case PIN_CONFIG_BIAS_DISABLE:
+		arg = (bcm2712_pull_config_get(pc, pin) == BCM2712_PULL_NONE);
+		break;
+	case PIN_CONFIG_BIAS_PULL_DOWN:
+		arg = (bcm2712_pull_config_get(pc, pin) == BCM2712_PULL_DOWN);
+		break;

drivers/pinctrl/bcm/pinctrl-bcm2712.c new file +18 -0

+}
+
+static int bcm2712_pinconf_set(struct pinctrl_dev *pctldev,
+			       unsigned int pin, unsigned long *configs,
+			       unsigned int num_configs)
+{
+	struct bcm2712_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev);
+	u32 param, arg;
+	int i;
+
+	for (i = 0; i < num_configs; i++) {
+		param = pinconf_to_config_param(configs[i]);
+		arg = pinconf_to_config_argument(configs[i]);
+
+		switch (param) {
+		case PIN_CONFIG_BIAS_DISABLE:
+			bcm2712_pull_config_set(pc, pin, BCM2712_PULL_NONE);
+			break;

drivers/pinctrl/bcm/pinctrl-bcm2712.c new file +3 -0

+
+	return 0;
+}

drivers/pinctrl/bcm/pinctrl-bcm2712.c new file +13 -0

+static int bcm2712_pinctrl_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	//struct device_node *np = dev->of_node;
+	const struct bcm_plat_data *pdata;
+	//const struct of_device_id *match;
+	struct bcm2712_pinctrl *pc;
+	const char **names;
+	int num_pins, i;
+
+	pdata = device_get_match_data(&pdev->dev);
+	if (!pdata)
+		return -EINVAL;

drivers/pinctrl/bcm/pinctrl-bcm2712.c new file +16 -0

+	pc->base = devm_platform_ioremap_resource(pdev, 0);
+	if (WARN_ON(IS_ERR(pc->base))) {
+		//dev_err(dev, "could not get IO memory\n");
+		return PTR_ERR(pc->base);
+	}
+
+	pc->pctl_desc = *pdata->pctl_desc;
+	num_pins = pc->pctl_desc.npins;
+	names = devm_kmalloc_array(dev, num_pins, sizeof(const char *),
+				   GFP_KERNEL);
+	if (!names)
+		return -ENOMEM;
+	for (i = 0; i < num_pins; i++)
+		names[i] = pc->pctl_desc.pins[i].name;
+	pc->gpio_groups = names;
+	pc->pin_regs = pdata->pin_regs;

Le 14/04/2024 à 00:14, Andrea della Porta a écrit :

Quoted 12 lines

Add a pincontrol driver for BCM2712. BCM2712 allows muxing GPIOs and setting configuration on pads.

Originally-by Jonathan Bell <[email protected]>

Originally-by Phil Elwell <[email protected]>

Signed-off-by Andrea della Porta <[email protected]>

---
  drivers/pinctrl/bcm/Kconfig           |    9 +
  drivers/pinctrl/bcm/Makefile          |    1 +
  drivers/pinctrl/bcm/pinctrl-bcm2712.c | 1247 +++++++++++++++++++++++++
  3 files changed, 1257 insertions(+)

create mode 100644 drivers/pinctrl/bcm/pinctrl-bcm2712.c

...

drivers/pinctrl/bcm/pinctrl-bcm2712.c line 922

Quoted 6 lines

drivers/pinctrl/bcm/pinctrl-bcm2712.c new file +6 -0

+static int bcm2712_pmx_get_function_groups(struct pinctrl_dev *pctldev,
+		unsigned selector,
+		const char * const **groups,
+		unsigned * const num_groups)
+{
+	struct bcm2712_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev);

Missing empty new line.

Quoted 6 lines

drivers/pinctrl/bcm/pinctrl-bcm2712.c new file +6 -0

+	/* every pin can do every function */
+	*groups = pc->gpio_groups;
+	*num_groups = pc->pctl_desc.npins;
+
+	return 0;
+}

...

Quoted 14 lines

drivers/pinctrl/bcm/pinctrl-bcm2712.c new file +14 -0

+static int bcm2712_pinconf_get(struct pinctrl_dev *pctldev,
+			unsigned pin, unsigned long *config)
+{
+	struct bcm2712_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev);
+	enum pin_config_param param = pinconf_to_config_param(*config);
+	u32 arg;
+
+	switch (param) {
+	case PIN_CONFIG_BIAS_DISABLE:
+		arg = (bcm2712_pull_config_get(pc, pin) == BCM2712_PULL_NONE);
+		break;
+	case PIN_CONFIG_BIAS_PULL_DOWN:
+		arg = (bcm2712_pull_config_get(pc, pin) == BCM2712_PULL_DOWN);
+		break;

drivers/pinctrl/bcm/pinctrl-bcm2712.c line 1032

Quoted 10 lines

drivers/pinctrl/bcm/pinctrl-bcm2712.c new file +10 -0

+	case PIN_CONFIG_BIAS_PULL_UP:
+		arg = (bcm2712_pull_config_get(pc, pin) == BCM2712_PULL_UP);
+		break;
+	default:
+		return -ENOTSUPP;
+	}
+
+	*config = pinconf_to_config_packed(param, arg);
+
+	return -ENOTSUPP;

Strange.

	return 0;

?

Quoted 22 lines

drivers/pinctrl/bcm/pinctrl-bcm2712.c new file +22 -0

+}
+
+static int bcm2712_pinconf_set(struct pinctrl_dev *pctldev,
+			       unsigned int pin, unsigned long *configs,
+			       unsigned int num_configs)
+{
+	struct bcm2712_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev);
+	u32 param, arg;
+	int i;
+
+	for (i = 0; i < num_configs; i++) {
+		param = pinconf_to_config_param(configs[i]);
+		arg = pinconf_to_config_argument(configs[i]);
+
+		switch (param) {
+		case PIN_CONFIG_BIAS_DISABLE:
+			bcm2712_pull_config_set(pc, pin, BCM2712_PULL_NONE);
+			break;
+		case PIN_CONFIG_BIAS_PULL_DOWN:
+			bcm2712_pull_config_set(pc, pin, BCM2712_PULL_DOWN);
+			break;
+		case PIN_CONFIG_BIAS_PULL_UP:

drivers/pinctrl/bcm/pinctrl-bcm2712.c line 1060

Quoted 6 lines

drivers/pinctrl/bcm/pinctrl-bcm2712.c new file +6 -0

+			bcm2712_pull_config_set(pc, pin, BCM2712_PULL_UP);
+			break;
+		default:
+			return -ENOTSUPP;
+		}
+	} /* for each config */

This comment is not really usefull, IMHO.

Quoted 4 lines

drivers/pinctrl/bcm/pinctrl-bcm2712.c new file +3 -0

+
+	return 0;
+}

...

Quoted 17 lines

drivers/pinctrl/bcm/pinctrl-bcm2712.c new file +17 -0

+static int bcm2712_pinctrl_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	//struct device_node *np = dev->of_node;
+	const struct bcm_plat_data *pdata;
+	//const struct of_device_id *match;
+	struct bcm2712_pinctrl *pc;
+	const char **names;
+	int num_pins, i;
+
+	pdata = device_get_match_data(&pdev->dev);
+	if (!pdata)
+		return -EINVAL;
+
+	pc = devm_kzalloc(dev, sizeof(*pc), GFP_KERNEL);
+	if (!pc)
+		return -ENOMEM;

drivers/pinctrl/bcm/pinctrl-bcm2712.c line 1211

Quoted 6 lines

drivers/pinctrl/bcm/pinctrl-bcm2712.c new file +6 -0

+
+	platform_set_drvdata(pdev, pc);
+	pc->dev = dev;
+	spin_lock_init(&pc->lock);
+
+	//pc->base = devm_of_iomap(dev, np, 0, NULL);

Any use for this commented code? (and variable declarations above)

CJ

Quoted 26 lines

drivers/pinctrl/bcm/pinctrl-bcm2712.c new file +26 -0

+	pc->base = devm_platform_ioremap_resource(pdev, 0);
+	if (WARN_ON(IS_ERR(pc->base))) {
+		//dev_err(dev, "could not get IO memory\n");
+		return PTR_ERR(pc->base);
+	}
+
+	pc->pctl_desc = *pdata->pctl_desc;
+	num_pins = pc->pctl_desc.npins;
+	names = devm_kmalloc_array(dev, num_pins, sizeof(const char *),
+				   GFP_KERNEL);
+	if (!names)
+		return -ENOMEM;
+	for (i = 0; i < num_pins; i++)
+		names[i] = pc->pctl_desc.pins[i].name;
+	pc->gpio_groups = names;
+	pc->pin_regs = pdata->pin_regs;
+	pc->pin_funcs = pdata->pin_funcs;
+	pc->pctl_dev = devm_pinctrl_register(dev, &pc->pctl_desc, pc);
+	if (IS_ERR(pc->pctl_dev))
+		return PTR_ERR(pc->pctl_dev);
+
+	pc->gpio_range = *pdata->gpio_range;
+	pinctrl_add_gpio_range(pc->pctl_dev, &pc->gpio_range);
+
+	return 0;
+}

...

Add a pincontrol driver for BCM2712. BCM2712 allows muxing GPIOs and setting configuration on pads.

Originally-by Jonathan Bell <[email protected]>

Originally-by Phil Elwell <[email protected]>

Signed-off-by Andrea della Porta <[email protected]>

---

  drivers/pinctrl/bcm/Kconfig           |    9 +
  drivers/pinctrl/bcm/Makefile          |    1 +
  drivers/pinctrl/bcm/pinctrl-bcm2712.c | 1247 +++++++++++++++++++++++++
  3 files changed, 1257 insertions(+)

create mode 100644 drivers/pinctrl/bcm/pinctrl-bcm2712.c

drivers/pinctrl/bcm/Kconfig +0 -0

index 35b51ce4298e..62ede44460bc 100644

drivers/pinctrl/bcm/Kconfig +3 -0

+	help
+	  Say Y here to enable the Broadcom BCM2712 PINCONF driver.
+
  config PINCTRL_BCM281XX
  	bool "Broadcom BCM281xx pinctrl driver"
  	depends on OF && (ARCH_BCM_MOBILE || COMPILE_TEST)

drivers/pinctrl/bcm/Makefile +0 -0

index 82b868ec1471..d298e4785829 100644

drivers/pinctrl/bcm/Makefile +0 -0

  obj-$(CONFIG_PINCTRL_BCM281XX)		+= pinctrl-bcm281xx.o
  obj-$(CONFIG_PINCTRL_BCM2835)		+= pinctrl-bcm2835.o
  obj-$(CONFIG_PINCTRL_BCM4908)		+= pinctrl-bcm4908.o

drivers/pinctrl/bcm/pinctrl-bcm2712.c new file +91 -0

new file mode 100644
index 000000000000..f9359e9eff14
@@ -0,0 +1,1247 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for Broadcom BCM2712 GPIO units (pinctrl only)
+ *
+ * Copyright (C) 2021-3 Raspberry Pi Ltd.
+ * Copyright (C) 2012 Chris Boot, Simon Arlott, Stephen Warren
+ *
+ * Based heavily on the BCM2835 GPIO & pinctrl driver, which was inspired by:
+ * pinctrl-nomadik.c, please see original file for copyright information
+ * pinctrl-tegra.c, please see original file for copyright information
+ */
+
+#include <linux/bitmap.h>
+#include <linux/bug.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/of_address.h>
+#include <linux/of.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/pinctrl/machine.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/platform_device.h>
+#include <linux/seq_file.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+
+#define MODULE_NAME "pinctrl-bcm2712"
+
+/* Register offsets */
+
+#define BCM2712_PULL_NONE	0
+#define BCM2712_PULL_DOWN	1
+#define BCM2712_PULL_UP		2
+#define BCM2712_PULL_MASK	0x3
+
+#define BCM2712_FSEL_COUNT 9
+#define BCM2712_FSEL_MASK  0xf
+
+#define FUNC(f) \
+	[func_##f] = #f
+#define PIN(i, f1, f2, f3, f4, f5, f6, f7, f8) \
+	[i] = { \
+		.funcs = { \
+			func_##f1, \
+			func_##f2, \
+			func_##f3, \
+			func_##f4, \
+			func_##f5, \
+			func_##f6, \
+			func_##f7, \
+			func_##f8, \
+		}, \
+	}
+
+#define MUX_BIT_VALID	0x8000
+#define REG_BIT_INVALID	0xffff
+
+#define BIT_TO_REG(b) (((b) >> 5) << 2)
+#define BIT_TO_SHIFT(b) ((b) & 0x1f)
+
+#define MUX_BIT(mr, mb) (MUX_BIT_VALID + ((mr)*4)*8 + (mb)*4)
+#define GPIO_REGS(n, mr, mb, pr, pb) \
+	[n] = { MUX_BIT(mr, mb), ((pr)*4)*8 + (pb)*2 }
+
+#define EMMC_REGS(n, pr, pb) \
+	[n] = { 0, ((pr)*4)*8 + (pb)*2 }
+
+#define AGPIO_REGS(n, mr, mb, pr, pb) \
+	[n] = { MUX_BIT(mr, mb), ((pr)*4)*8 + (pb)*2 }
+
+#define SGPIO_REGS(n, mr, mb) \
+	[n+32] = { MUX_BIT(mr, mb), REG_BIT_INVALID }
+
+#define GPIO_PIN(a) PINCTRL_PIN(a, "gpio" #a)
+#define AGPIO_PIN(a) PINCTRL_PIN(a, "aon_gpio" #a)
+#define SGPIO_PIN(a) PINCTRL_PIN(a+32, "aon_sgpio" #a)
+
+struct pin_regs {
+	u16 mux_bit;
+	u16 pad_bit;
+};
+
+struct bcm2712_pinctrl {

drivers/pinctrl/bcm/pinctrl-bcm2712.c new file +12 -0

+
+struct bcm_plat_data {
+	const struct pinctrl_desc *pctl_desc;
+	const struct pinctrl_gpio_range *gpio_range;
+	const struct pin_regs *pin_regs;
+	const struct bcm2712_pin_funcs *pin_funcs;
+};
+
+struct bcm2712_pin_funcs {
+	u8 funcs[BCM2712_FSEL_COUNT - 1];
+};
+

drivers/pinctrl/bcm/pinctrl-bcm2712.c new file +13 -0

+static int bcm2712_pinctrl_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	//struct device_node *np = dev->of_node;
+	const struct bcm_plat_data *pdata;
+	//const struct of_device_id *match;
+	struct bcm2712_pinctrl *pc;
+	const char **names;
+	int num_pins, i;
+
+	pdata = device_get_match_data(&pdev->dev);
+	if (!pdata)
+		return -EINVAL;

drivers/pinctrl/bcm/pinctrl-bcm2712.c new file +26 -0

+	pc->base = devm_platform_ioremap_resource(pdev, 0);
+	if (WARN_ON(IS_ERR(pc->base))) {
+		//dev_err(dev, "could not get IO memory\n");
+		return PTR_ERR(pc->base);
+	}
+
+	pc->pctl_desc = *pdata->pctl_desc;
+	num_pins = pc->pctl_desc.npins;
+	names = devm_kmalloc_array(dev, num_pins, sizeof(const char *),
+				   GFP_KERNEL);
+	if (!names)
+		return -ENOMEM;
+	for (i = 0; i < num_pins; i++)
+		names[i] = pc->pctl_desc.pins[i].name;
+	pc->gpio_groups = names;
+	pc->pin_regs = pdata->pin_regs;
+	pc->pin_funcs = pdata->pin_funcs;
+	pc->pctl_dev = devm_pinctrl_register(dev, &pc->pctl_desc, pc);
+	if (IS_ERR(pc->pctl_dev))
+		return PTR_ERR(pc->pctl_dev);
+
+	pc->gpio_range = *pdata->gpio_range;
+	pinctrl_add_gpio_range(pc->pctl_dev, &pc->gpio_range);
+
+	return 0;
+}

Quoted 5 lines · Andrea della Porta

Add a pincontrol driver for BCM2712. BCM2712 allows muxing GPIOs and setting configuration on pads.

Originally-by Jonathan Bell <[email protected]>

Originally-by Phil Elwell <[email protected]>

Is that a new tag in a comment message? Signed-off-by maybe?

Quoted 2 lines

Signed-off-by Andrea della Porta <[email protected]>

---

Was not pinctrl-single usable somehow that we had to go through a dedicated pinctrl driver?

Quoted 11 lines

  drivers/pinctrl/bcm/Kconfig           |    9 +
  drivers/pinctrl/bcm/Makefile          |    1 +
  drivers/pinctrl/bcm/pinctrl-bcm2712.c | 1247 +++++++++++++++++++++++++
  3 files changed, 1257 insertions(+)

create mode 100644 drivers/pinctrl/bcm/pinctrl-bcm2712.c

drivers/pinctrl/bcm/Kconfig +0 -0

index 35b51ce4298e..62ede44460bc 100644

drivers/pinctrl/bcm/Kconfig line 10

Quoted 8 lines

drivers/pinctrl/bcm/Kconfig +6 -0

@@ -3,6 +3,15 @@
  # Broadcom pinctrl drivers
  #
+config PINCTRL_BCM2712
+	bool "Broadcom BCM2712 PINCONF driver"
+	depends on OF && (ARCH_BCM2835 || ARCH_BRCMSTB || COMPILE_TEST)
+	select PINMUX
+	select PINCONF
+	select GENERIC_PINCONF

Rename to PINCTRL_BRCMSTB sicne this is not BCM2712 specific at all.

Quoted 11 lines

drivers/pinctrl/bcm/Kconfig +3 -0

+	help
+	  Say Y here to enable the Broadcom BCM2712 PINCONF driver.
+
  config PINCTRL_BCM281XX
  	bool "Broadcom BCM281xx pinctrl driver"
  	depends on OF && (ARCH_BCM_MOBILE || COMPILE_TEST)

drivers/pinctrl/bcm/Makefile +0 -0

index 82b868ec1471..d298e4785829 100644

drivers/pinctrl/bcm/Makefile line 3

Quoted 3 lines

drivers/pinctrl/bcm/Makefile +1 -0

@@ -1,6 +1,7 @@
  # SPDX-License-Identifier: GPL-2.0
  # Broadcom pinctrl support
+obj-$(CONFIG_PINCTRL_BCM2712)		+= pinctrl-bcm2712.o

Likewise.

Quoted 100 lines

drivers/pinctrl/bcm/Makefile +0 -0

  obj-$(CONFIG_PINCTRL_BCM281XX)		+= pinctrl-bcm281xx.o
  obj-$(CONFIG_PINCTRL_BCM2835)		+= pinctrl-bcm2835.o
  obj-$(CONFIG_PINCTRL_BCM4908)		+= pinctrl-bcm4908.o

drivers/pinctrl/bcm/pinctrl-bcm2712.c new file +91 -0

new file mode 100644
index 000000000000..f9359e9eff14
@@ -0,0 +1,1247 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for Broadcom BCM2712 GPIO units (pinctrl only)
+ *
+ * Copyright (C) 2021-3 Raspberry Pi Ltd.
+ * Copyright (C) 2012 Chris Boot, Simon Arlott, Stephen Warren
+ *
+ * Based heavily on the BCM2835 GPIO & pinctrl driver, which was inspired by:
+ * pinctrl-nomadik.c, please see original file for copyright information
+ * pinctrl-tegra.c, please see original file for copyright information
+ */
+
+#include <linux/bitmap.h>
+#include <linux/bug.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/of_address.h>
+#include <linux/of.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/pinctrl/machine.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/platform_device.h>
+#include <linux/seq_file.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+
+#define MODULE_NAME "pinctrl-bcm2712"
+
+/* Register offsets */
+
+#define BCM2712_PULL_NONE	0
+#define BCM2712_PULL_DOWN	1
+#define BCM2712_PULL_UP		2
+#define BCM2712_PULL_MASK	0x3
+
+#define BCM2712_FSEL_COUNT 9
+#define BCM2712_FSEL_MASK  0xf
+
+#define FUNC(f) \
+	[func_##f] = #f
+#define PIN(i, f1, f2, f3, f4, f5, f6, f7, f8) \
+	[i] = { \
+		.funcs = { \
+			func_##f1, \
+			func_##f2, \
+			func_##f3, \
+			func_##f4, \
+			func_##f5, \
+			func_##f6, \
+			func_##f7, \
+			func_##f8, \
+		}, \
+	}
+
+#define MUX_BIT_VALID	0x8000
+#define REG_BIT_INVALID	0xffff
+
+#define BIT_TO_REG(b) (((b) >> 5) << 2)
+#define BIT_TO_SHIFT(b) ((b) & 0x1f)
+
+#define MUX_BIT(mr, mb) (MUX_BIT_VALID + ((mr)*4)*8 + (mb)*4)
+#define GPIO_REGS(n, mr, mb, pr, pb) \
+	[n] = { MUX_BIT(mr, mb), ((pr)*4)*8 + (pb)*2 }
+
+#define EMMC_REGS(n, pr, pb) \
+	[n] = { 0, ((pr)*4)*8 + (pb)*2 }
+
+#define AGPIO_REGS(n, mr, mb, pr, pb) \
+	[n] = { MUX_BIT(mr, mb), ((pr)*4)*8 + (pb)*2 }
+
+#define SGPIO_REGS(n, mr, mb) \
+	[n+32] = { MUX_BIT(mr, mb), REG_BIT_INVALID }
+
+#define GPIO_PIN(a) PINCTRL_PIN(a, "gpio" #a)
+#define AGPIO_PIN(a) PINCTRL_PIN(a, "aon_gpio" #a)
+#define SGPIO_PIN(a) PINCTRL_PIN(a+32, "aon_sgpio" #a)
+
+struct pin_regs {
+	u16 mux_bit;
+	u16 pad_bit;
+};
+
+struct bcm2712_pinctrl {

drivers/pinctrl/bcm/pinctrl-bcm2712.c line 101

Quoted 10 lines

drivers/pinctrl/bcm/pinctrl-bcm2712.c new file +10 -0

+	struct device *dev;
+	void __iomem *base;
+	struct pinctrl_dev *pctl_dev;
+	struct pinctrl_desc pctl_desc;
+	const struct pin_regs *pin_regs;
+	const struct bcm2712_pin_funcs *pin_funcs;
+	const char *const *gpio_groups;
+	struct pinctrl_gpio_range gpio_range;
+	spinlock_t lock;
+};

Please s/bcm2712/brcmstb/ throughout the driver's structures and any declaration that is not inherently 2712 specific and just make 2712 the first instance using this driver.

Quoted 12 lines

drivers/pinctrl/bcm/pinctrl-bcm2712.c new file +12 -0

+
+struct bcm_plat_data {
+	const struct pinctrl_desc *pctl_desc;
+	const struct pinctrl_gpio_range *gpio_range;
+	const struct pin_regs *pin_regs;
+	const struct bcm2712_pin_funcs *pin_funcs;
+};
+
+struct bcm2712_pin_funcs {
+	u8 funcs[BCM2712_FSEL_COUNT - 1];
+};
+

[snip]

Quoted 13 lines

drivers/pinctrl/bcm/pinctrl-bcm2712.c new file +13 -0

+static int bcm2712_pinctrl_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	//struct device_node *np = dev->of_node;
+	const struct bcm_plat_data *pdata;
+	//const struct of_device_id *match;
+	struct bcm2712_pinctrl *pc;
+	const char **names;
+	int num_pins, i;
+
+	pdata = device_get_match_data(&pdev->dev);
+	if (!pdata)
+		return -EINVAL;

drivers/pinctrl/bcm/pinctrl-bcm2712.c line 1211

Quoted 10 lines

drivers/pinctrl/bcm/pinctrl-bcm2712.c new file +10 -0

+
+	pc = devm_kzalloc(dev, sizeof(*pc), GFP_KERNEL);
+	if (!pc)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, pc);
+	pc->dev = dev;
+	spin_lock_init(&pc->lock);
+
+	//pc->base = devm_of_iomap(dev, np, 0, NULL);

Remove stray commented lines.

Quoted 26 lines

drivers/pinctrl/bcm/pinctrl-bcm2712.c new file +26 -0

+	pc->base = devm_platform_ioremap_resource(pdev, 0);
+	if (WARN_ON(IS_ERR(pc->base))) {
+		//dev_err(dev, "could not get IO memory\n");
+		return PTR_ERR(pc->base);
+	}
+
+	pc->pctl_desc = *pdata->pctl_desc;
+	num_pins = pc->pctl_desc.npins;
+	names = devm_kmalloc_array(dev, num_pins, sizeof(const char *),
+				   GFP_KERNEL);
+	if (!names)
+		return -ENOMEM;
+	for (i = 0; i < num_pins; i++)
+		names[i] = pc->pctl_desc.pins[i].name;
+	pc->gpio_groups = names;
+	pc->pin_regs = pdata->pin_regs;
+	pc->pin_funcs = pdata->pin_funcs;
+	pc->pctl_dev = devm_pinctrl_register(dev, &pc->pctl_desc, pc);
+	if (IS_ERR(pc->pctl_dev))
+		return PTR_ERR(pc->pctl_dev);
+
+	pc->gpio_range = *pdata->gpio_range;
+	pinctrl_add_gpio_range(pc->pctl_dev, &pc->gpio_range);
+
+	return 0;
+}

drivers/pinctrl/bcm/pinctrl-bcm2712.c line 1247

Quoted 10 lines

drivers/pinctrl/bcm/pinctrl-bcm2712.c new file +10 -0

+
+static struct platform_driver bcm2712_pinctrl_driver = {
+	.probe = bcm2712_pinctrl_probe,
+	.driver = {
+		.name = MODULE_NAME,
+		.of_match_table = bcm2712_pinctrl_match,
+		.suppress_bind_attrs = true,
+	},
+};
+builtin_platform_driver(bcm2712_pinctrl_driver);

There is no MODULE_LICENSE(), MODULE_AUTHOR() or MODULE_DESCRIPTION(), please provide some.

Signature · 1 line

Florian

Hi Andrea,

thanks for your patch!

Some comments apart from was said already.

drivers/pinctrl/bcm/pinctrl-bcm2712.c line 24

Quoted 2 lines · Andrea della Porta

drivers/pinctrl/bcm/pinctrl-bcm2712.c new file +2 -0

+#include <linux/pinctrl/consumer.h>
+#include <linux/pinctrl/machine.h>

Really? Why?

drivers/pinctrl/bcm/pinctrl-bcm2712.c line 28

Quoted 4 lines

drivers/pinctrl/bcm/pinctrl-bcm2712.c new file +4 -0

+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/pinctrl/pinconf-generic.h>

I would just expect these.

drivers/pinctrl/bcm/pinctrl-bcm2712.c line 1194

Quoted 6 lines

drivers/pinctrl/bcm/pinctrl-bcm2712.c new file +6 -0

+static int bcm2712_pinctrl_probe(struct platform_device *pdev)
+{
+       struct device *dev = &pdev->dev;
+       //struct device_node *np = dev->of_node;
+       const struct bcm_plat_data *pdata;
+       //const struct of_device_id *match;

I don't know if others commented on it but drop all commented-out code or make use of it.

Yours,
Linus Walleij

Broadcom BCM2712 SoC has an SDHCI card controller using the SDIO CFG register block present on other STB chips. Add support for BCM2712 SD capabilities of this chipset.
The silicon is SD Express capable but this driver port does not currently include that feature yet.
Based on downstream driver by raspberry foundation maintained kernel.

Signed-off-by Andrea della Porta <[email protected]>

---
 drivers/mmc/host/sdhci-brcmstb.c | 130 +++++++++++++++++++++++++++++++
 1 file changed, 130 insertions(+)

drivers/mmc/host/sdhci-brcmstb.c +28 -0

index 9053526fa212..907a4947abe5 100644
@@ -12,6 +12,8 @@
 #include <linux/of.h>
 #include <linux/bitops.h>
 #include <linux/delay.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/regulator/consumer.h>
 
 #include "sdhci-cqhci.h"
 #include "sdhci-pltfm.h"

@@ -30,15 +32,31 @@
 
 #define SDHCI_ARASAN_CQE_BASE_ADDR		0x200
 
+#define SDIO_CFG_CTRL				0x0
+#define  SDIO_CFG_CTRL_SDCD_N_TEST_EN		BIT(31)
+#define  SDIO_CFG_CTRL_SDCD_N_TEST_LEV		BIT(30)
+
+#define SDIO_CFG_SD_PIN_SEL			0x44
+#define  SDIO_CFG_SD_PIN_SEL_MASK		0x3
+#define  SDIO_CFG_SD_PIN_SEL_SD			BIT(1)
+#define  SDIO_CFG_SD_PIN_SEL_MMC		BIT(0)
+
+#define SDIO_CFG_MAX_50MHZ_MODE			0x1ac
+#define  SDIO_CFG_MAX_50MHZ_MODE_STRAP_OVERRIDE	BIT(31)
+#define  SDIO_CFG_MAX_50MHZ_MODE_ENABLE		BIT(0)
+
 struct sdhci_brcmstb_priv {
 	void __iomem *cfg_regs;
 	unsigned int flags;
 	struct clk *base_clk;
 	u32 base_freq_hz;
+	struct pinctrl *pinctrl;
+	struct pinctrl_state *pins_default;
 };
 
 struct brcmstb_match_priv {
 	void (*hs400es)(struct mmc_host *mmc, struct mmc_ios *ios);
+	void (*cfginit)(struct sdhci_host *host);
 	struct sdhci_ops *ops;
 	const unsigned int flags;
 };

@@ -124,6 +142,42 @@ static void sdhci_brcmstb_hs400es(struct mmc_host *mmc, struct mmc_ios *ios)
 	writel(reg, host->ioaddr + SDHCI_VENDOR);
 }
 
+static void sdhci_bcm2712_set_clock(struct sdhci_host *host, unsigned int clock)
+{
+	u16 clk;
+	u32 reg;
+	bool is_emmc_rate = false;
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_brcmstb_priv *brcmstb_priv = sdhci_pltfm_priv(pltfm_host);
+
+	host->mmc->actual_clock = 0;
+

drivers/mmc/host/sdhci-brcmstb.c +71 -0

+
+	reg = readl(brcmstb_priv->cfg_regs + SDIO_CFG_SD_PIN_SEL);
+	reg &= ~SDIO_CFG_SD_PIN_SEL_MASK;
+	if (is_emmc_rate)
+		reg |= SDIO_CFG_SD_PIN_SEL_MMC;
+	else
+		reg |= SDIO_CFG_SD_PIN_SEL_SD;
+	writel(reg, brcmstb_priv->cfg_regs + SDIO_CFG_SD_PIN_SEL);
+
+	if (clock == 0)
+		return;
+
+	clk = sdhci_calc_clk(host, clock, &host->mmc->actual_clock);
+	sdhci_enable_clk(host, clk);
+}
+
 static void sdhci_brcmstb_set_clock(struct sdhci_host *host, unsigned int clock)
 {
 	u16 clk;

@@ -139,6 +193,17 @@ static void sdhci_brcmstb_set_clock(struct sdhci_host *host, unsigned int clock)
 	sdhci_enable_clk(host, clk);
 }
 
+static void sdhci_brcmstb_set_power(struct sdhci_host *host, unsigned char mode,
+				  unsigned short vdd)
+{
+	if (!IS_ERR(host->mmc->supply.vmmc)) {
+		struct mmc_host *mmc = host->mmc;
+
+		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
+	}
+	sdhci_set_power_noreg(host, mode, vdd);
+}
+
 static void sdhci_brcmstb_set_uhs_signaling(struct sdhci_host *host,
 					    unsigned int timing)
 {

@@ -168,6 +233,36 @@ static void sdhci_brcmstb_set_uhs_signaling(struct sdhci_host *host,
 	sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
 }
 
+static void sdhci_brcmstb_cfginit_2712(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_brcmstb_priv *brcmstb_priv = sdhci_pltfm_priv(pltfm_host);
+	u32 uhs_mask = (MMC_CAP_UHS_SDR50 | MMC_CAP_UHS_SDR104);
+	u32 hsemmc_mask = (MMC_CAP2_HS200_1_8V_SDR | MMC_CAP2_HS200_1_2V_SDR |
+			   MMC_CAP2_HS400_1_8V | MMC_CAP2_HS400_1_2V);
+	u32 reg;
+
+	/*
+	* If we support a speed that requires tuning,
+	* then select the delay line PHY as the clock source.
+	*/
+	if ((host->mmc->caps & uhs_mask) || (host->mmc->caps2 & hsemmc_mask)) {
+		reg = readl(brcmstb_priv->cfg_regs + SDIO_CFG_MAX_50MHZ_MODE);
+		reg &= ~SDIO_CFG_MAX_50MHZ_MODE_ENABLE;
+		reg |= SDIO_CFG_MAX_50MHZ_MODE_STRAP_OVERRIDE;
+		writel(reg, brcmstb_priv->cfg_regs + SDIO_CFG_MAX_50MHZ_MODE);
+	}
+
+	if ((host->mmc->caps & MMC_CAP_NONREMOVABLE) ||
+	    (host->mmc->caps & MMC_CAP_NEEDS_POLL)) {
+		/* Force presence */
+		reg = readl(brcmstb_priv->cfg_regs + SDIO_CFG_CTRL);
+		reg &= ~SDIO_CFG_CTRL_SDCD_N_TEST_LEV;
+		reg |= SDIO_CFG_CTRL_SDCD_N_TEST_EN;
+		writel(reg, brcmstb_priv->cfg_regs + SDIO_CFG_CTRL);
+	}
+}
+
 static void sdhci_brcmstb_dumpregs(struct mmc_host *mmc)
 {
 	sdhci_dumpregs(mmc_priv(mmc));

@@ -200,6 +295,14 @@ static struct sdhci_ops sdhci_brcmstb_ops = {
 	.set_uhs_signaling = sdhci_set_uhs_signaling,
 };
 
+static struct sdhci_ops sdhci_brcmstb_ops_2712 = {
+	.set_clock = sdhci_bcm2712_set_clock,
+	.set_power = sdhci_brcmstb_set_power,
+	.set_bus_width = sdhci_set_bus_width,
+	.reset = sdhci_reset,
+	.set_uhs_signaling = sdhci_set_uhs_signaling,
+};
+
 static struct sdhci_ops sdhci_brcmstb_ops_7216 = {
 	.set_clock = sdhci_brcmstb_set_clock,
 	.set_bus_width = sdhci_set_bus_width,

@@ -237,7 +340,13 @@ static struct brcmstb_match_priv match_priv_74165b0 = {
 	.ops = &sdhci_brcmstb_ops_74165b0,
 };
 
+static const struct brcmstb_match_priv match_priv_2712 = {
+	.cfginit = sdhci_brcmstb_cfginit_2712,
+	.ops = &sdhci_brcmstb_ops_2712,
+};
+
 static const struct of_device_id __maybe_unused sdhci_brcm_of_match[] = {
+	{ .compatible = "brcm,bcm2712-sdhci", .data = &match_priv_2712 },
 	{ .compatible = "brcm,bcm7425-sdhci", .data = &match_priv_7425 },
 	{ .compatible = "brcm,bcm7445-sdhci", .data = &match_priv_7445 },
 	{ .compatible = "brcm,bcm7216-sdhci", .data = &match_priv_7216 },

drivers/mmc/host/sdhci-brcmstb.c +16 -0

+		dev_err(&pdev->dev, "fail to get matching of_match struct\n");
+		return -EINVAL;
+	}
 	match_priv = match->data;
 
 	dev_dbg(&pdev->dev, "Probe found match for %s\n",  match->compatible);

@@ -354,6 +468,19 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
 	if (res)
 		goto err;
 
+	priv->pinctrl = devm_pinctrl_get(&pdev->dev);
+	if (IS_ERR(priv->pinctrl)) {
+			no_pinctrl = true;
+	}
+	priv->pins_default = pinctrl_lookup_state(priv->pinctrl, "default");
+	if (IS_ERR(priv->pins_default)) {
+			dev_dbg(&pdev->dev, "No pinctrl default state\n");
+			no_pinctrl = true;
+	}
+
+	if (no_pinctrl )
+		priv->pinctrl = NULL;
+
 	/*
 	 * Automatic clock gating does not work for SD cards that may
 	 * voltage switch so only enable it for non-removable devices.

Broadcom BCM2712 SoC has an SDHCI card controller using the SDIO CFG register block present on other STB chips. Add support for BCM2712 SD capabilities of this chipset.
The silicon is SD Express capable but this driver port does not currently include that feature yet.
Based on downstream driver by raspberry foundation maintained kernel.

Signed-off-by Andrea della Porta <andrea.porta-IBi9RG/[email protected]>

---
  drivers/mmc/host/sdhci-brcmstb.c | 130 +++++++++++++++++++++++++++++++
  1 file changed, 130 insertions(+)

drivers/mmc/host/sdhci-brcmstb.c +6 -0

+
+		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
+	}
+	sdhci_set_power_noreg(host, mode, vdd);
+}
+
  static void sdhci_brcmstb_set_uhs_signaling(struct sdhci_host *host,
  					    unsigned int timing)
  {

@@ -168,6 +233,36 @@ static void sdhci_brcmstb_set_uhs_signaling(struct sdhci_host *host,
  	sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
  }

drivers/mmc/host/sdhci-brcmstb.c +5 -0

+	}
+
+	if (no_pinctrl )
+		priv->pinctrl = NULL;
+
  	/*
  	 * Automatic clock gating does not work for SD cards that may
  	 * voltage switch so only enable it for non-removable devices.

Broadcom BCM2712 SoC has an SDHCI card controller using the SDIO CFG register block present on other STB chips. Add support for BCM2712 SD capabilities of this chipset.
The silicon is SD Express capable but this driver port does not currently include that feature yet.
Based on downstream driver by raspberry foundation maintained kernel.

Signed-off-by Andrea della Porta <[email protected]>

---
  drivers/mmc/host/sdhci-brcmstb.c | 130 +++++++++++++++++++++++++++++++
  1 file changed, 130 insertions(+)

drivers/mmc/host/sdhci-brcmstb.c +18 -0

index 9053526fa212..907a4947abe5 100644
@@ -12,6 +12,8 @@
  #include <linux/of.h>
  #include <linux/bitops.h>
  #include <linux/delay.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/regulator/consumer.h>
 
  #include "sdhci-cqhci.h"
  #include "sdhci-pltfm.h"

@@ -30,15 +32,31 @@
 
  #define SDHCI_ARASAN_CQE_BASE_ADDR		0x200
 
+#define SDIO_CFG_CTRL				0x0
+#define  SDIO_CFG_CTRL_SDCD_N_TEST_EN		BIT(31)
+#define  SDIO_CFG_CTRL_SDCD_N_TEST_LEV		BIT(30)
+
+#define SDIO_CFG_SD_PIN_SEL			0x44
+#define  SDIO_CFG_SD_PIN_SEL_MASK		0x3
+#define  SDIO_CFG_SD_PIN_SEL_SD			BIT(1)
+#define  SDIO_CFG_SD_PIN_SEL_MMC		BIT(0)
+
+#define SDIO_CFG_MAX_50MHZ_MODE			0x1ac
+#define  SDIO_CFG_MAX_50MHZ_MODE_STRAP_OVERRIDE	BIT(31)
+#define  SDIO_CFG_MAX_50MHZ_MODE_ENABLE		BIT(0)
+
  struct sdhci_brcmstb_priv {
  	void __iomem *cfg_regs;
  	unsigned int flags;
  	struct clk *base_clk;
  	u32 base_freq_hz;
+	struct pinctrl *pinctrl;
+	struct pinctrl_state *pins_default;
  };
 
  struct brcmstb_match_priv {
  	void (*hs400es)(struct mmc_host *mmc, struct mmc_ios *ios);
+	void (*cfginit)(struct sdhci_host *host);
  	struct sdhci_ops *ops;
  	const unsigned int flags;
  };

drivers/mmc/host/sdhci-brcmstb.c +77 -0

+	}
+
+	reg = readl(brcmstb_priv->cfg_regs + SDIO_CFG_SD_PIN_SEL);
+	reg &= ~SDIO_CFG_SD_PIN_SEL_MASK;
+	if (is_emmc_rate)
+		reg |= SDIO_CFG_SD_PIN_SEL_MMC;
+	else
+		reg |= SDIO_CFG_SD_PIN_SEL_SD;
+	writel(reg, brcmstb_priv->cfg_regs + SDIO_CFG_SD_PIN_SEL);
+
+	if (clock == 0)
+		return;
+
+	clk = sdhci_calc_clk(host, clock, &host->mmc->actual_clock);
+	sdhci_enable_clk(host, clk);
+}
+
  static void sdhci_brcmstb_set_clock(struct sdhci_host *host, unsigned int clock)
  {
  	u16 clk;

@@ -139,6 +193,17 @@ static void sdhci_brcmstb_set_clock(struct sdhci_host *host, unsigned int clock)
  	sdhci_enable_clk(host, clk);
  }
 
+static void sdhci_brcmstb_set_power(struct sdhci_host *host, unsigned char mode,
+				  unsigned short vdd)
+{
+	if (!IS_ERR(host->mmc->supply.vmmc)) {
+		struct mmc_host *mmc = host->mmc;
+
+		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, vdd);
+	}
+	sdhci_set_power_noreg(host, mode, vdd);
+}
+
  static void sdhci_brcmstb_set_uhs_signaling(struct sdhci_host *host,
  					    unsigned int timing)
  {

@@ -168,6 +233,36 @@ static void sdhci_brcmstb_set_uhs_signaling(struct sdhci_host *host,
  	sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
  }
 
+static void sdhci_brcmstb_cfginit_2712(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_brcmstb_priv *brcmstb_priv = sdhci_pltfm_priv(pltfm_host);
+	u32 uhs_mask = (MMC_CAP_UHS_SDR50 | MMC_CAP_UHS_SDR104);
+	u32 hsemmc_mask = (MMC_CAP2_HS200_1_8V_SDR | MMC_CAP2_HS200_1_2V_SDR |
+			   MMC_CAP2_HS400_1_8V | MMC_CAP2_HS400_1_2V);
+	u32 reg;
+
+	/*
+	* If we support a speed that requires tuning,
+	* then select the delay line PHY as the clock source.
+	*/
+	if ((host->mmc->caps & uhs_mask) || (host->mmc->caps2 & hsemmc_mask)) {
+		reg = readl(brcmstb_priv->cfg_regs + SDIO_CFG_MAX_50MHZ_MODE);
+		reg &= ~SDIO_CFG_MAX_50MHZ_MODE_ENABLE;
+		reg |= SDIO_CFG_MAX_50MHZ_MODE_STRAP_OVERRIDE;
+		writel(reg, brcmstb_priv->cfg_regs + SDIO_CFG_MAX_50MHZ_MODE);
+	}
+
+	if ((host->mmc->caps & MMC_CAP_NONREMOVABLE) ||
+	    (host->mmc->caps & MMC_CAP_NEEDS_POLL)) {
+		/* Force presence */
+		reg = readl(brcmstb_priv->cfg_regs + SDIO_CFG_CTRL);
+		reg &= ~SDIO_CFG_CTRL_SDCD_N_TEST_LEV;
+		reg |= SDIO_CFG_CTRL_SDCD_N_TEST_EN;
+		writel(reg, brcmstb_priv->cfg_regs + SDIO_CFG_CTRL);
+	}
+}
+
  static void sdhci_brcmstb_dumpregs(struct mmc_host *mmc)
  {
  	sdhci_dumpregs(mmc_priv(mmc));

@@ -200,6 +295,14 @@ static struct sdhci_ops sdhci_brcmstb_ops = {
  	.set_uhs_signaling = sdhci_set_uhs_signaling,
  };
 
+static struct sdhci_ops sdhci_brcmstb_ops_2712 = {
+	.set_clock = sdhci_bcm2712_set_clock,
+	.set_power = sdhci_brcmstb_set_power,
+	.set_bus_width = sdhci_set_bus_width,
+	.reset = sdhci_reset,
+	.set_uhs_signaling = sdhci_set_uhs_signaling,
+};
+
  static struct sdhci_ops sdhci_brcmstb_ops_7216 = {
  	.set_clock = sdhci_brcmstb_set_clock,
  	.set_bus_width = sdhci_set_bus_width,

@@ -237,7 +340,13 @@ static struct brcmstb_match_priv match_priv_74165b0 = {
  	.ops = &sdhci_brcmstb_ops_74165b0,
  };
 
+static const struct brcmstb_match_priv match_priv_2712 = {
+	.cfginit = sdhci_brcmstb_cfginit_2712,
+	.ops = &sdhci_brcmstb_ops_2712,
+};
+
  static const struct of_device_id __maybe_unused sdhci_brcm_of_match[] = {
+	{ .compatible = "brcm,bcm2712-sdhci", .data = &match_priv_2712 },
  	{ .compatible = "brcm,bcm7425-sdhci", .data = &match_priv_7425 },
  	{ .compatible = "brcm,bcm7445-sdhci", .data = &match_priv_7445 },
  	{ .compatible = "brcm,bcm7216-sdhci", .data = &match_priv_7216 },

@@ -314,11 +423,16 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
  	struct sdhci_brcmstb_priv *priv;
  	u32 actual_clock_mhz;
  	struct sdhci_host *host;
+	bool no_pinctrl = false;
  	struct clk *clk;
  	struct clk *base_clk = NULL;
  	int res;
 
  	match = of_match_node(sdhci_brcm_of_match, pdev->dev.of_node);
+	if (!match) {
+		dev_err(&pdev->dev, "fail to get matching of_match struct\n");
+		return -EINVAL;
+	}
  	match_priv = match->data;
 
  	dev_dbg(&pdev->dev, "Probe found match for %s\n",  match->compatible);

Broadcom BCM2712 SDHCI controller is SD Express capable. Add support for sde capability where the implementation is based on downstream driver, diverging from it in the way that init_sd_express callback is invoked:
in downstream the sdhci_ops structure has been augmented with a new function ptr 'init_sd_express' that just propagate the call to the driver specific callback so that the callstack from a structure standpoint is mmc_host_ops -> sdhci_ops. The drawback here is in the added level of indirection (the newly added init_sd_express is redundant) and the sdhci_ops structure declaration has to be changed. To overcome this the presented approach consist in patching the mmc_host_ops init_sd_express callback to point directly to the custom function defined in this driver (see struct brcmstb_match_priv).

Signed-off-by Andrea della Porta <andrea.porta-IBi9RG/[email protected]>

---
  drivers/mmc/host/Kconfig         |   1 +
  drivers/mmc/host/sdhci-brcmstb.c | 147 ++++++++++++++++++++++++++++++-
  2 files changed, 147 insertions(+), 1 deletion(-)

drivers/mmc/host/sdhci-brcmstb.c +12 -0

+	if (brcmstb_priv->sde_pcie) {
+		struct of_changeset changeset;
+		static struct property okay_property = {
+			.name = "status",
+			.value = "okay",
+			.length = 5,
+		};
+
+		/* Enable the pcie controller */
+		of_changeset_init(&changeset);
+		ret = of_changeset_update_property(&changeset,
+						   brcmstb_priv->sde_pcie,

drivers/mmc/host/sdhci-brcmstb.c +3 -0

+	return ret;
+}
+

drivers/mmc/host/sdhci-brcmstb.c +6 -0

@@ -468,6 +581,24 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
  	if (res)
  		goto err;
 
+	priv->sde_1v8 = devm_regulator_get_optional(&pdev->dev, "sde-1v8");
+	if (IS_ERR(priv->sde_1v8))
+		priv->flags &= ~BRCMSTB_PRIV_FLAGS_HAS_SD_EXPRESS;
+
+	iomem = platform_get_resource(pdev, IORESOURCE_MEM, 2);
+	if (iomem) {

drivers/mmc/host/sdhci-brcmstb.c +2 -0

+	}
+
  	priv->pinctrl = devm_pinctrl_get(&pdev->dev);
  	if (IS_ERR(priv->pinctrl)) {
  			no_pinctrl = true;

drivers/mmc/host/sdhci-brcmstb.c +5 -0

+	}
+
+	if (no_pinctrl || !priv->sde_ioaddr || !priv->sde_ioaddr2) {
  		priv->pinctrl = NULL;
+		priv->flags &= ~BRCMSTB_PRIV_FLAGS_HAS_SD_EXPRESS;
+	}
 
  	/*
  	 * Automatic clock gating does not work for SD cards that may

Le 14/04/2024 à 00:14, Andrea della Porta a écrit :

Quoted 18 lines

Broadcom BCM2712 SDHCI controller is SD Express capable. Add support for sde capability where the implementation is based on downstream driver, diverging from it in the way that init_sd_express callback is invoked:
in downstream the sdhci_ops structure has been augmented with a new function ptr 'init_sd_express' that just propagate the call to the driver specific callback so that the callstack from a structure standpoint is mmc_host_ops -> sdhci_ops. The drawback here is in the added level of indirection (the newly added init_sd_express is redundant) and the sdhci_ops structure declaration has to be changed. To overcome this the presented approach consist in patching the mmc_host_ops init_sd_express callback to point directly to the custom function defined in this driver (see struct brcmstb_match_priv).

Signed-off-by Andrea della Porta <andrea.porta-IBi9RG/[email protected]>

---
  drivers/mmc/host/Kconfig         |   1 +
  drivers/mmc/host/sdhci-brcmstb.c | 147 ++++++++++++++++++++++++++++++-
  2 files changed, 147 insertions(+), 1 deletion(-)

...

Quoted 16 lines

drivers/mmc/host/sdhci-brcmstb.c +16 -0

+	if (brcmstb_priv->sde_pcie) {
+		struct of_changeset changeset;
+		static struct property okay_property = {
+			.name = "status",
+			.value = "okay",
+			.length = 5,
+		};
+
+		/* Enable the pcie controller */
+		of_changeset_init(&changeset);
+		ret = of_changeset_update_property(&changeset,
+						   brcmstb_priv->sde_pcie,
+						   &okay_property);
+		if (ret) {
+			dev_err(dev, "%s: failed to update property - %d\n", __func__,
+			       ret);

drivers/mmc/host/sdhci-brcmstb.c static void sdhci_brcmstb_cfginit_2712(struct sdhci_host *host) · line 368

Quoted 6 lines

drivers/mmc/host/sdhci-brcmstb.c +6 -0

+			return -ENODEV;
+		}
+		ret = of_changeset_apply(&changeset);
+	}
+
+	dev_dbg(dev, "%s -> %d\n", __func__, ret);

Is this really useful?

Quoted 4 lines

drivers/mmc/host/sdhci-brcmstb.c +3 -0

+	return ret;
+}
+

...

Quoted 9 lines

drivers/mmc/host/sdhci-brcmstb.c +6 -0

@@ -468,6 +581,24 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
  	if (res)
  		goto err;
+	priv->sde_1v8 = devm_regulator_get_optional(&pdev->dev, "sde-1v8");
+	if (IS_ERR(priv->sde_1v8))
+		priv->flags &= ~BRCMSTB_PRIV_FLAGS_HAS_SD_EXPRESS;
+
+	iomem = platform_get_resource(pdev, IORESOURCE_MEM, 2);
+	if (iomem) {

drivers/mmc/host/sdhci-brcmstb.c static int sdhci_brcmstb_probe(struct platform_device *pdev) · line 598

Quoted 10 lines

drivers/mmc/host/sdhci-brcmstb.c +10 -0

+		priv->sde_ioaddr = devm_ioremap_resource(&pdev->dev, iomem);
+		if (IS_ERR(priv->sde_ioaddr))
+			priv->sde_ioaddr = NULL;
+	}
+
+	iomem = platform_get_resource(pdev, IORESOURCE_MEM, 3);
+	if (iomem) {
+		priv->sde_ioaddr2 = devm_ioremap_resource(&pdev->dev, iomem);
+		if (IS_ERR(priv->sde_ioaddr2))
+			priv->sde_ioaddr = NULL;

sde_ioaddr2 ?

drivers/mmc/host/sdhci-brcmstb.c static int sdhci_brcmstb_probe(struct platform_device *pdev) · line 614

Quoted 13 lines

drivers/mmc/host/sdhci-brcmstb.c +6 -1

+	}
+
  	priv->pinctrl = devm_pinctrl_get(&pdev->dev);
  	if (IS_ERR(priv->pinctrl)) {
  			no_pinctrl = true;

@@ -478,8 +609,16 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
  			no_pinctrl = true;
  	}
-	if (no_pinctrl )
+	priv->pins_sdex = pinctrl_lookup_state(priv->pinctrl, "sd-express");
+	if (IS_ERR(priv->pins_sdex)) {
+			dev_dbg(&pdev->dev, "No pinctrl sd-express state\n");
+			no_pinctrl = true;

Indentation looks too large.

Quoted 9 lines

drivers/mmc/host/sdhci-brcmstb.c +5 -0

+	}
+
+	if (no_pinctrl || !priv->sde_ioaddr || !priv->sde_ioaddr2) {
  		priv->pinctrl = NULL;
+		priv->flags &= ~BRCMSTB_PRIV_FLAGS_HAS_SD_EXPRESS;
+	}
  	/*
  	 * Automatic clock gating does not work for SD cards that may

...

Broadcom BCM2712 SDHCI controller is SD Express capable. Add support for sde capability where the implementation is based on downstream driver, diverging from it in the way that init_sd_express callback is invoked:
in downstream the sdhci_ops structure has been augmented with a new function ptr 'init_sd_express' that just propagate the call to the driver specific callback so that the callstack from a structure standpoint is mmc_host_ops -> sdhci_ops. The drawback here is in the added level of indirection (the newly added init_sd_express is redundant) and the sdhci_ops structure declaration has to be changed. To overcome this the presented approach consist in patching the mmc_host_ops init_sd_express callback to point directly to the custom function defined in this driver (see struct brcmstb_match_priv).

Signed-off-by Andrea della Porta <[email protected]>

---
  drivers/mmc/host/Kconfig         |   1 +
  drivers/mmc/host/sdhci-brcmstb.c | 147 ++++++++++++++++++++++++++++++-
  2 files changed, 147 insertions(+), 1 deletion(-)

drivers/mmc/host/Kconfig +1 -0

index aebc587f77a7..343ccac1a4e4 100644
@@ -1018,6 +1018,7 @@ config MMC_SDHCI_BRCMSTB
  	depends on ARCH_BRCMSTB || BMIPS_GENERIC || COMPILE_TEST
  	depends on MMC_SDHCI_PLTFM
  	select MMC_CQHCI
+	select OF_DYNAMIC
  	default ARCH_BRCMSTB || BMIPS_GENERIC
  	help
  	  This selects support for the SDIO/SD/MMC Host Controller on

drivers/mmc/host/sdhci-brcmstb.c +24 -0

index 907a4947abe5..56fb34a75ec2 100644
@@ -29,6 +29,7 @@
 
  #define BRCMSTB_PRIV_FLAGS_HAS_CQE		BIT(0)
  #define BRCMSTB_PRIV_FLAGS_GATE_CLOCK		BIT(1)
+#define BRCMSTB_PRIV_FLAGS_HAS_SD_EXPRESS	BIT(2)
 
  #define SDHCI_ARASAN_CQE_BASE_ADDR		0x200
 

@@ -50,13 +51,19 @@ struct sdhci_brcmstb_priv {
  	unsigned int flags;
  	struct clk *base_clk;
  	u32 base_freq_hz;
+	struct regulator *sde_1v8;
+	struct device_node *sde_pcie;
+	void *__iomem sde_ioaddr;
+	void *__iomem sde_ioaddr2;
  	struct pinctrl *pinctrl;
  	struct pinctrl_state *pins_default;
+	struct pinctrl_state *pins_sdex;
  };
 
  struct brcmstb_match_priv {
  	void (*hs400es)(struct mmc_host *mmc, struct mmc_ios *ios);
  	void (*cfginit)(struct sdhci_host *host);
+	int (*init_sd_express)(struct mmc_host *mmc, struct mmc_ios *ios);
  	struct sdhci_ops *ops;
  	const unsigned int flags;
  };

@@ -263,6 +270,105 @@ static void sdhci_brcmstb_cfginit_2712(struct sdhci_host *host)
  	}
  }
 
+static int bcm2712_init_sd_express(struct mmc_host *mmc, struct mmc_ios *ios)
+{
+	struct sdhci_host *host = mmc_priv(mmc);
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_brcmstb_priv *brcmstb_priv = sdhci_pltfm_priv(pltfm_host);
+	struct device *dev = host->mmc->parent;
+	u32 ctrl_val;
+	u32 present_state;
+	int ret;
+
+	if (!brcmstb_priv->sde_ioaddr || !brcmstb_priv->sde_ioaddr2)
+		return -EINVAL;
+
+	if (!brcmstb_priv->pinctrl)
+		return -EINVAL;
+
+	/* Turn off the SD clock first */