[PATCH 00/16] spi: bcm63xx-hsspi: driver and doc updates

[PATCH 00/16] spi: bcm63xx-hsspi: driver and doc updates

[William Zhang]

This patch series include the accumulative updates and fixes for the
driver from Broadcom. It also added a new driver for the updated SPI
controller found in the new BCMBCA SoC. The device tree document is
converted to yaml format and updated accordingly.


William Zhang (16):
  dt-bindings: spi: Convert bcm63xx-hsspi bindings to json-schema
  dt-bindings: spi: Add bcmbca-hsspi controller support
  dt-bindings: spi: Add spi peripheral specific property
  ARM: dts: broadcom: bcmbca: Add spi controller node
  arm64: dts: broadcom: bcmbca: Add spi controller node
  spi: bcm63xx-hsspi: Endianness fix for ARM based SoC
  spi: bcm63xx-hsspi: Add polling mode support
  spi: bcm63xx-hsspi: Handle cs_change correctly
  spi: bcm63xx-hsspi: Fix multi-bit mode setting
  spi: bcm63xx-hsspi: Make dummy cs workaround as an option
  spi: bcm63xx-hsspi: Add prepend feature support
  spi: bcm63xx-hsspi: Add clock gate disable option support
  spi: spi-mem: Allow controller supporting mem_ops without exec_op
  spi: bcm63xx-hsspi: prepend: Disable spi mem dual io read op support
  spi: bcmbca-hsspi: Add driver for newer HSSPI controller
  MAINTAINERS: Add entry for Broadcom Broadband SoC HS SPI drivers

 .../brcm,bcm63xx-hsspi-peripheral-props.yaml  |  27 +
 .../bindings/spi/brcm,bcm63xx-hsspi.yaml      | 124 ++++
 .../bindings/spi/spi-bcm63xx-hsspi.txt        |  33 -
 .../bindings/spi/spi-peripheral-props.yaml    |   1 +
 MAINTAINERS                                   |  12 +
 arch/arm/boot/dts/bcm47622.dtsi               |  17 +
 arch/arm/boot/dts/bcm63138.dtsi               |  17 +
 arch/arm/boot/dts/bcm63148.dtsi               |  17 +
 arch/arm/boot/dts/bcm63178.dtsi               |  18 +
 arch/arm/boot/dts/bcm6756.dtsi                |  18 +
 arch/arm/boot/dts/bcm6846.dtsi                |  17 +
 arch/arm/boot/dts/bcm6855.dtsi                |  18 +
 arch/arm/boot/dts/bcm6878.dtsi                |  18 +
 arch/arm/boot/dts/bcm947622.dts               |   4 +
 arch/arm/boot/dts/bcm963138.dts               |   4 +
 arch/arm/boot/dts/bcm963138dvt.dts            |   4 +
 arch/arm/boot/dts/bcm963148.dts               |   4 +
 arch/arm/boot/dts/bcm963178.dts               |   4 +
 arch/arm/boot/dts/bcm96756.dts                |   4 +
 arch/arm/boot/dts/bcm96846.dts                |   4 +
 arch/arm/boot/dts/bcm96855.dts                |   4 +
 arch/arm/boot/dts/bcm96878.dts                |   4 +
 .../boot/dts/broadcom/bcmbca/bcm4908.dtsi     |  17 +
 .../boot/dts/broadcom/bcmbca/bcm4912.dtsi     |  19 +
 .../boot/dts/broadcom/bcmbca/bcm63146.dtsi    |  18 +
 .../boot/dts/broadcom/bcmbca/bcm63158.dtsi    |  18 +
 .../boot/dts/broadcom/bcmbca/bcm6813.dtsi     |  19 +
 .../boot/dts/broadcom/bcmbca/bcm6856.dtsi     |  17 +
 .../boot/dts/broadcom/bcmbca/bcm6858.dtsi     |  17 +
 .../boot/dts/broadcom/bcmbca/bcm94908.dts     |   4 +
 .../boot/dts/broadcom/bcmbca/bcm94912.dts     |   4 +
 .../boot/dts/broadcom/bcmbca/bcm963146.dts    |   4 +
 .../boot/dts/broadcom/bcmbca/bcm963158.dts    |   4 +
 .../boot/dts/broadcom/bcmbca/bcm96813.dts     |   4 +
 .../boot/dts/broadcom/bcmbca/bcm96856.dts     |   4 +
 .../boot/dts/broadcom/bcmbca/bcm96858.dts     |   4 +
 drivers/spi/Kconfig                           |   9 +
 drivers/spi/Makefile                          |   1 +
 drivers/spi/spi-bcm63xx-hsspi.c               | 391 ++++++++++-
 drivers/spi/spi-bcmbca-hsspi.c                | 629 ++++++++++++++++++
 drivers/spi/spi-mem.c                         |   2 +-
 drivers/spi/spi.c                             |  13 +-
 42 files changed, 1498 insertions(+), 73 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi-peripheral-props.yaml
 create mode 100644 Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
 delete mode 100644 Documentation/devicetree/bindings/spi/spi-bcm63xx-hsspi.txt
 create mode 100644 drivers/spi/spi-bcmbca-hsspi.c

-- 
2.37.3

[PATCH 01/16] dt-bindings: spi: Convert bcm63xx-hsspi bindings to json-schema

[William Zhang]

This is the preparation for updates on the bcm63xx hsspi driver. Convert
the text based bindings to json-schema per new dts requirement.

Signed-off-by: William Zhang <william.zhang@broadcom.com>
---

 .../bindings/spi/brcm,bcm63xx-hsspi.yaml      | 52 +++++++++++++++++++
 .../bindings/spi/spi-bcm63xx-hsspi.txt        | 33 ------------
 2 files changed, 52 insertions(+), 33 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
 delete mode 100644 Documentation/devicetree/bindings/spi/spi-bcm63xx-hsspi.txt

diff --git a/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
new file mode 100644
index 000000000000..45f1417b1213
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
@@ -0,0 +1,52 @@
+# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/brcm,bcm63xx-hsspi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom BCM6328 High Speed SPI controller
+
+maintainers:
+  - Jonas Gorski <jonas.gorski@gmail.com>
+
+properties:
+  compatible:
+    const: brcm,bcm6328-hsspi
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: spi master reference clock
+      - description: spi master pll clock
+
+  clock-names:
+    items:
+      - const: hsspi
+      - const: pll
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - interrupts
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    spi@10001000 {
+        compatible = "brcm,bcm6328-hsspi";
+        reg = <0x10001000 0x600>;
+        interrupts = <29>;
+        clocks = <&clkctl 9>, <&hsspi_pll>;
+        clock-names = "hsspi", "pll";
+        num-cs = <2>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+    };
diff --git a/Documentation/devicetree/bindings/spi/spi-bcm63xx-hsspi.txt b/Documentation/devicetree/bindings/spi/spi-bcm63xx-hsspi.txt
deleted file mode 100644
index 37b29ee13860..000000000000
--- a/Documentation/devicetree/bindings/spi/spi-bcm63xx-hsspi.txt
+++ /dev/null
@@ -1,33 +0,0 @@
-Binding for Broadcom BCM6328 High Speed SPI controller
-
-Required properties:
-- compatible: must contain of "brcm,bcm6328-hsspi".
-- reg: Base address and size of the controllers memory area.
-- interrupts: Interrupt for the SPI block.
-- clocks: phandles of the SPI clock and the PLL clock.
-- clock-names: must be "hsspi", "pll".
-- #address-cells: <1>, as required by generic SPI binding.
-- #size-cells: <0>, also as required by generic SPI binding.
-
-Optional properties:
-- num-cs: some controllers have less than 8 cs signals. Defaults to 8
-  if absent.
-
-Child nodes as per the generic SPI binding.
-
-Example:
-
-	spi@10001000 {
-		compatible = "brcm,bcm6328-hsspi";
-		reg = <0x10001000 0x600>;
-
-		interrupts = <29>;
-
-		clocks = <&clkctl 9>, <&hsspi_pll>;
-		clock-names = "hsspi", "pll";
-
-		num-cs = <2>;
-
-		#address-cells = <1>;
-		#size-cells = <0>;
-	};
-- 
2.37.3

[PATCH 02/16] dt-bindings: spi: Add bcmbca-hsspi controller support

[William Zhang]

The new Broadcom Broadband BCMBCA SoCs includes a updated HSSPI
controller. Add a new compatible string and required fields for the new
driver.  Also add myself and Kursad as the maintainers.

Signed-off-by: William Zhang <william.zhang@broadcom.com>
---

 .../bindings/spi/brcm,bcm63xx-hsspi.yaml      | 84 +++++++++++++++++--
 1 file changed, 78 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
index 45f1417b1213..56e69d4a1faf 100644
--- a/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
+++ b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
@@ -4,22 +4,51 @@
 $id: http://devicetree.org/schemas/spi/brcm,bcm63xx-hsspi.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Broadcom BCM6328 High Speed SPI controller
+title: Broadcom Broadband SoC High Speed SPI controller
 
 maintainers:
+
+  - William Zhang <william.zhang@broadcom.com>
+  - Kursad Oney <kursad.oney@broadcom.com>
   - Jonas Gorski <jonas.gorski@gmail.com>
 
+description: |
+  Broadcom Broadband SoC supports High Speed SPI master controller since the
+  early MIPS based chips such as BCM6328 and BCM63268.  This controller was
+  carried over to recent ARM based chips, such as BCM63138, BCM4908 and BCM6858.
+
+  It has a limitation that can not keep the chip select line active between
+  the SPI transfers within the same SPI message. This can terminate the
+  transaction to some SPI devices prematurely. The issue can be worked around by
+  either the controller's prepend mode or using the dummy chip select
+  workaround. This controller uses the compatible string brcm,bcm6328-hsspi.
+
+  The newer SoCs such as BCM6756, BCM4912 and BCM6855 include an updated SPI
+  controller that add the capability to allow the driver to control chip select
+  explicitly. This solves the issue in the old controller. This new controller
+  uses the compatible string brcm,bcmbca-hsspi.
+
 properties:
   compatible:
-    const: brcm,bcm6328-hsspi
+    enum:
+      - brcm,bcm6328-hsspi
+      - brcm,bcmbca-hsspi
 
   reg:
-    maxItems: 1
+    items:
+      - description: main registers
+      - description: miscellaneous control registers
+    minItems: 1
+
+  reg-names:
+    items:
+      - const: hsspi
+      - const: spim-ctrl
 
   clocks:
     items:
-      - description: spi master reference clock
-      - description: spi master pll clock
+      - description: SPI master reference clock
+      - description: SPI master pll clock
 
   clock-names:
     items:
@@ -29,12 +58,43 @@ properties:
   interrupts:
     maxItems: 1
 
+  brcm,use-cs-workaround:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description: |
+      Enable dummy chip select workaround for SPI transfers that can not be
+      supported by the default controller's prepend mode, i.e. delay or cs
+      change needed between SPI transfers.
+
 required:
   - compatible
   - reg
   - clocks
   - clock-names
-  - interrupts
+
+allOf:
+  - $ref: "spi-controller.yaml#"
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - brcm,bcm6328-hsspi
+    then:
+      properties:
+        reg:
+          minItems: 1
+          maxItems: 1
+    else:
+      properties:
+        reg:
+          minItems: 2
+          maxItems: 2
+        reg-names:
+          minItems: 2
+          maxItems: 2
+        brcm,use-cs-workaround: false
+      required:
+        - reg-names
 
 unevaluatedProperties: false
 
@@ -50,3 +110,15 @@ examples:
         #address-cells = <1>;
         #size-cells = <0>;
     };
+  - |
+    spi@ff801000 {
+        compatible = "brcm,bcmbca-hsspi";
+        reg = <0xff801000 0x1000>,
+              <0xff802610 0x4>;
+        reg-names = "hsspi", "spim-ctrl";
+        clocks = <&hsspi>, <&hsspi_pll>;
+        clock-names = "hsspi", "pll";
+        num-cs = <8>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+    };
-- 
2.37.3

[PATCH 03/16] dt-bindings: spi: Add spi peripheral specific property

[William Zhang]

brcm,no-clk-gate is a Broadcom Broadband HS SPI controller specific
property for certain SPI device such as Broadcom ISI voice daughtercard
to work properly. It disables the clock gating feature when the chip
select is deasserted for any device that wants to keep the clock
running.

Signed-off-by: William Zhang <william.zhang@broadcom.com>
---

 .../brcm,bcm63xx-hsspi-peripheral-props.yaml  | 27 +++++++++++++++++++
 .../bindings/spi/spi-peripheral-props.yaml    |  1 +
 2 files changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi-peripheral-props.yaml

diff --git a/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi-peripheral-props.yaml
new file mode 100644
index 000000000000..81884e2cc42d
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi-peripheral-props.yaml
@@ -0,0 +1,27 @@
+# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/brcm,bcm63xx-hsspi-peripheral-props.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Peripheral-specific properties for Broadcom Broadband SoC HSSPI controller
+
+description:
+  See spi-peripheral-props.yaml for more info.
+
+maintainers:
+  - William Zhang <william.zhang@broadcom.com>
+  - Kursad Oney <kursad.oney@broadcom.com>
+  - Jonas Gorski <jonas.gorski@gmail.com>
+
+properties:
+  brcm,no-clk-gate:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description:
+      Some SPI device such as Broadcom ISI based voice daughtercard requires SPI
+      clock running even when chip select is deasserted. By default the
+      controller turns off or gate the clock when cs is not active to save
+      power. This flag tells the controller driver to keep the clock running
+      when chip select is not active.
+
+additionalProperties: true
diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
index ead2cccf658f..f85d777c7b67 100644
--- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
+++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
@@ -108,5 +108,6 @@ allOf:
   - $ref: cdns,qspi-nor-peripheral-props.yaml#
   - $ref: samsung,spi-peripheral-props.yaml#
   - $ref: nvidia,tegra210-quad-peripheral-props.yaml#
+  - $ref: brcm,bcm63xx-hsspi-peripheral-props.yaml#
 
 additionalProperties: true
-- 
2.37.3

[PATCH 04/16] ARM: dts: broadcom: bcmbca: Add spi controller node

[William Zhang]

Add support for HSSPI controller in ARMv7 chip dts files.

Signed-off-by: William Zhang <william.zhang@broadcom.com>
---

 arch/arm/boot/dts/bcm47622.dtsi    | 17 +++++++++++++++++
 arch/arm/boot/dts/bcm63138.dtsi    | 17 +++++++++++++++++
 arch/arm/boot/dts/bcm63148.dtsi    | 17 +++++++++++++++++
 arch/arm/boot/dts/bcm63178.dtsi    | 18 ++++++++++++++++++
 arch/arm/boot/dts/bcm6756.dtsi     | 18 ++++++++++++++++++
 arch/arm/boot/dts/bcm6846.dtsi     | 17 +++++++++++++++++
 arch/arm/boot/dts/bcm6855.dtsi     | 18 ++++++++++++++++++
 arch/arm/boot/dts/bcm6878.dtsi     | 18 ++++++++++++++++++
 arch/arm/boot/dts/bcm947622.dts    |  4 ++++
 arch/arm/boot/dts/bcm963138.dts    |  4 ++++
 arch/arm/boot/dts/bcm963138dvt.dts |  4 ++++
 arch/arm/boot/dts/bcm963148.dts    |  4 ++++
 arch/arm/boot/dts/bcm963178.dts    |  4 ++++
 arch/arm/boot/dts/bcm96756.dts     |  4 ++++
 arch/arm/boot/dts/bcm96846.dts     |  4 ++++
 arch/arm/boot/dts/bcm96855.dts     |  4 ++++
 arch/arm/boot/dts/bcm96878.dts     |  4 ++++
 17 files changed, 176 insertions(+)

diff --git a/arch/arm/boot/dts/bcm47622.dtsi b/arch/arm/boot/dts/bcm47622.dtsi
index f4b2db9bc4ab..da4b71ef2471 100644
--- a/arch/arm/boot/dts/bcm47622.dtsi
+++ b/arch/arm/boot/dts/bcm47622.dtsi
@@ -88,6 +88,12 @@ uart_clk: uart-clk {
 			clock-div = <4>;
 			clock-mult = <1>;
 		};
+
+		hsspi_pll: hsspi-pll {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <200000000>;
+		};
 	};
 
 	psci {
@@ -119,6 +125,17 @@ bus@ff800000 {
 		#size-cells = <1>;
 		ranges = <0 0xff800000 0x800000>;
 
+		hsspi: spi@1000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "brcm,bcm6328-hsspi";
+			reg = <0x1000 0x600>;
+			clocks = <&hsspi_pll &hsspi_pll>;
+			clock-names = "hsspi", "pll";
+			num-cs = <8>;
+			status = "disabled";
+		};
+
 		uart0: serial@12000 {
 			compatible = "arm,pl011", "arm,primecell";
 			reg = <0x12000 0x1000>;
diff --git a/arch/arm/boot/dts/bcm63138.dtsi b/arch/arm/boot/dts/bcm63138.dtsi
index b774a8d63813..1631694c0496 100644
--- a/arch/arm/boot/dts/bcm63138.dtsi
+++ b/arch/arm/boot/dts/bcm63138.dtsi
@@ -66,6 +66,12 @@ apb_clk: apb_clk {
 			clock-div = <4>;
 			clock-mult = <1>;
 		};
+
+		hsspi_pll: hsspi-pll {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <400000000>;
+		};
 	};
 
 	/* ARM bus */
@@ -203,6 +209,17 @@ serial1: serial@620 {
 			status = "disabled";
 		};
 
+		hsspi: spi@1000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "brcm,bcm6328-hsspi";
+			reg = <0x1000 0x600>;
+			clocks = <&hsspi_pll &hsspi_pll>;
+			clock-names = "hsspi", "pll";
+			num-cs = <8>;
+			status = "disabled";
+		};
+
 		nand_controller: nand-controller@2000 {
 			#address-cells = <1>;
 			#size-cells = <0>;
diff --git a/arch/arm/boot/dts/bcm63148.dtsi b/arch/arm/boot/dts/bcm63148.dtsi
index 7cd55d64de71..6dccba705f5d 100644
--- a/arch/arm/boot/dts/bcm63148.dtsi
+++ b/arch/arm/boot/dts/bcm63148.dtsi
@@ -60,6 +60,12 @@ periph_clk: periph-clk {
 			#clock-cells = <0>;
 			clock-frequency = <50000000>;
 		};
+
+		hsspi_pll: hsspi-pll {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <400000000>;
+		};
 	};
 
 	psci {
@@ -100,5 +106,16 @@ uart0: serial@600 {
 			clock-names = "refclk";
 			status = "disabled";
 		};
+
+		hsspi: spi@1000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "brcm,bcm6328-hsspi";
+			reg = <0x1000 0x600>;
+			clocks = <&hsspi_pll &hsspi_pll>;
+			clock-names = "hsspi", "pll";
+			num-cs = <8>;
+			status = "disabled";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/bcm63178.dtsi b/arch/arm/boot/dts/bcm63178.dtsi
index 043e699cbc27..8db27e7ac9fd 100644
--- a/arch/arm/boot/dts/bcm63178.dtsi
+++ b/arch/arm/boot/dts/bcm63178.dtsi
@@ -71,6 +71,7 @@ periph_clk: periph-clk {
 			#clock-cells = <0>;
 			clock-frequency = <200000000>;
 		};
+
 		uart_clk: uart-clk {
 			compatible = "fixed-factor-clock";
 			#clock-cells = <0>;
@@ -78,6 +79,12 @@ uart_clk: uart-clk {
 			clock-div = <4>;
 			clock-mult = <1>;
 		};
+
+		hsspi_pll: hsspi-pll {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <200000000>;
+		};
 	};
 
 	psci {
@@ -109,6 +116,17 @@ bus@ff800000 {
 		#size-cells = <1>;
 		ranges = <0 0xff800000 0x800000>;
 
+		hsspi: spi@1000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "brcm,bcm6328-hsspi";
+			reg = <0x1000 0x600>;
+			clocks = <&hsspi_pll &hsspi_pll>;
+			clock-names = "hsspi", "pll";
+			num-cs = <8>;
+			status = "disabled";
+		};
+
 		uart0: serial@12000 {
 			compatible = "arm,pl011", "arm,primecell";
 			reg = <0x12000 0x1000>;
diff --git a/arch/arm/boot/dts/bcm6756.dtsi b/arch/arm/boot/dts/bcm6756.dtsi
index 5c72219bc194..2af35a48b6c3 100644
--- a/arch/arm/boot/dts/bcm6756.dtsi
+++ b/arch/arm/boot/dts/bcm6756.dtsi
@@ -88,6 +88,12 @@ uart_clk: uart-clk {
 			clock-div = <4>;
 			clock-mult = <1>;
 		};
+
+		hsspi_pll: hsspi-pll {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <200000000>;
+		};
 	};
 
 	psci {
@@ -119,6 +125,18 @@ bus@ff800000 {
 		#size-cells = <1>;
 		ranges = <0 0xff800000 0x800000>;
 
+		hsspi: spi@1000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "brcm,bcmbca-hsspi";
+			reg = <0x1000 0x600>, <0x2610 0x4>;
+			reg-names = "hsspi", "spim-ctrl";
+			clocks = <&hsspi_pll &hsspi_pll>;
+			clock-names = "hsspi", "pll";
+			num-cs = <8>;
+			status = "disabled";
+		};
+
 		uart0: serial@12000 {
 			compatible = "arm,pl011", "arm,primecell";
 			reg = <0x12000 0x1000>;
diff --git a/arch/arm/boot/dts/bcm6846.dtsi b/arch/arm/boot/dts/bcm6846.dtsi
index 81513a793815..fa26b2107f93 100644
--- a/arch/arm/boot/dts/bcm6846.dtsi
+++ b/arch/arm/boot/dts/bcm6846.dtsi
@@ -61,6 +61,12 @@ periph_clk: periph-clk {
 			#clock-cells = <0>;
 			clock-frequency = <200000000>;
 		};
+
+		hsspi_pll: hsspi-pll {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <400000000>;
+		};
 	};
 
 	psci {
@@ -100,5 +106,16 @@ uart0: serial@640 {
 			clock-names = "refclk";
 			status = "disabled";
 		};
+
+		hsspi: spi@1000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "brcm,bcm6328-hsspi";
+			reg = <0x1000 0x600>;
+			clocks = <&hsspi_pll &hsspi_pll>;
+			clock-names = "hsspi", "pll";
+			num-cs = <8>;
+			status = "disabled";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/bcm6855.dtsi b/arch/arm/boot/dts/bcm6855.dtsi
index 5fa5feac0e29..bf028f0ad84c 100644
--- a/arch/arm/boot/dts/bcm6855.dtsi
+++ b/arch/arm/boot/dts/bcm6855.dtsi
@@ -78,6 +78,12 @@ uart_clk: uart-clk {
 			clock-div = <4>;
 			clock-mult = <1>;
 		};
+
+		hsspi_pll: hsspi-pll {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <200000000>;
+		};
 	};
 
 	psci {
@@ -109,6 +115,18 @@ bus@ff800000 {
 		#size-cells = <1>;
 		ranges = <0 0xff800000 0x800000>;
 
+		hsspi: spi@1000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "brcm,bcmbca-hsspi";
+			reg = <0x1000 0x600>, <0x2610 0x4>;
+			reg-names = "hsspi", "spim-ctrl";
+			clocks = <&hsspi_pll &hsspi_pll>;
+			clock-names = "hsspi", "pll";
+			num-cs = <8>;
+			status = "disabled";
+		};
+
 		uart0: serial@12000 {
 			compatible = "arm,pl011", "arm,primecell";
 			reg = <0x12000 0x1000>;
diff --git a/arch/arm/boot/dts/bcm6878.dtsi b/arch/arm/boot/dts/bcm6878.dtsi
index 4ec836ac4baf..be7ab5f52da4 100644
--- a/arch/arm/boot/dts/bcm6878.dtsi
+++ b/arch/arm/boot/dts/bcm6878.dtsi
@@ -61,6 +61,7 @@ periph_clk: periph-clk {
 			#clock-cells = <0>;
 			clock-frequency = <200000000>;
 		};
+
 		uart_clk: uart-clk {
 			compatible = "fixed-factor-clock";
 			#clock-cells = <0>;
@@ -68,6 +69,12 @@ uart_clk: uart-clk {
 			clock-div = <4>;
 			clock-mult = <1>;
 		};
+
+		hsspi_pll: hsspi-pll {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <200000000>;
+		};
 	};
 
 	psci {
@@ -100,6 +107,17 @@ bus@ff800000 {
 		#size-cells = <1>;
 		ranges = <0 0xff800000 0x800000>;
 
+		hsspi: spi@1000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "brcm,bcm6328-hsspi";
+			reg = <0x1000 0x600>;
+			clocks = <&hsspi_pll &hsspi_pll>;
+			clock-names = "hsspi", "pll";
+			num-cs = <8>;
+			status = "disabled";
+		};
+
 		uart0: serial@12000 {
 			compatible = "arm,pl011", "arm,primecell";
 			reg = <0x12000 0x1000>;
diff --git a/arch/arm/boot/dts/bcm947622.dts b/arch/arm/boot/dts/bcm947622.dts
index 6f083724ab8e..93b8ce22678d 100644
--- a/arch/arm/boot/dts/bcm947622.dts
+++ b/arch/arm/boot/dts/bcm947622.dts
@@ -28,3 +28,7 @@ memory@0 {
 &uart0 {
 	status = "okay";
 };
+
+&hsspi {
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/bcm963138.dts b/arch/arm/boot/dts/bcm963138.dts
index d28c4f130ca2..1b405c249213 100644
--- a/arch/arm/boot/dts/bcm963138.dts
+++ b/arch/arm/boot/dts/bcm963138.dts
@@ -25,3 +25,7 @@ memory@0 {
 &serial0 {
 	status = "okay";
 };
+
+&hsspi {
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/bcm963138dvt.dts b/arch/arm/boot/dts/bcm963138dvt.dts
index 15bec75be74c..b5af61853a07 100644
--- a/arch/arm/boot/dts/bcm963138dvt.dts
+++ b/arch/arm/boot/dts/bcm963138dvt.dts
@@ -50,3 +50,7 @@ &ahci {
 &sata_phy {
 	status = "okay";
 };
+
+&hsspi {
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/bcm963148.dts b/arch/arm/boot/dts/bcm963148.dts
index 98f6a6d09f50..1f5d6d783f09 100644
--- a/arch/arm/boot/dts/bcm963148.dts
+++ b/arch/arm/boot/dts/bcm963148.dts
@@ -28,3 +28,7 @@ memory@0 {
 &uart0 {
 	status = "okay";
 };
+
+&hsspi {
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/bcm963178.dts b/arch/arm/boot/dts/bcm963178.dts
index fa096e9cde23..d036e99dd8d1 100644
--- a/arch/arm/boot/dts/bcm963178.dts
+++ b/arch/arm/boot/dts/bcm963178.dts
@@ -28,3 +28,7 @@ memory@0 {
 &uart0 {
 	status = "okay";
 };
+
+&hsspi {
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/bcm96756.dts b/arch/arm/boot/dts/bcm96756.dts
index 9a4a87ba9c8a..8b104f3fb14a 100644
--- a/arch/arm/boot/dts/bcm96756.dts
+++ b/arch/arm/boot/dts/bcm96756.dts
@@ -28,3 +28,7 @@ memory@0 {
 &uart0 {
 	status = "okay";
 };
+
+&hsspi {
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/bcm96846.dts b/arch/arm/boot/dts/bcm96846.dts
index c70ebccabc19..55852c229608 100644
--- a/arch/arm/boot/dts/bcm96846.dts
+++ b/arch/arm/boot/dts/bcm96846.dts
@@ -28,3 +28,7 @@ memory@0 {
 &uart0 {
 	status = "okay";
 };
+
+&hsspi {
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/bcm96855.dts b/arch/arm/boot/dts/bcm96855.dts
index 4438152561ac..2ad880af2104 100644
--- a/arch/arm/boot/dts/bcm96855.dts
+++ b/arch/arm/boot/dts/bcm96855.dts
@@ -28,3 +28,7 @@ memory@0 {
 &uart0 {
 	status = "okay";
 };
+
+&hsspi {
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/bcm96878.dts b/arch/arm/boot/dts/bcm96878.dts
index 8fbc175cb452..b7af8ade7a9d 100644
--- a/arch/arm/boot/dts/bcm96878.dts
+++ b/arch/arm/boot/dts/bcm96878.dts
@@ -28,3 +28,7 @@ memory@0 {
 &uart0 {
 	status = "okay";
 };
+
+&hsspi {
+	status = "okay";
+};
-- 
2.37.3

[PATCH 05/16] arm64: dts: broadcom: bcmbca: Add spi controller node

[William Zhang]

Add support for HSSPI controller in ARMv8 chip dts files.

Signed-off-by: William Zhang <william.zhang@broadcom.com>
---

 .../boot/dts/broadcom/bcmbca/bcm4908.dtsi     | 17 +++++++++++++++++
 .../boot/dts/broadcom/bcmbca/bcm4912.dtsi     | 19 +++++++++++++++++++
 .../boot/dts/broadcom/bcmbca/bcm63146.dtsi    | 18 ++++++++++++++++++
 .../boot/dts/broadcom/bcmbca/bcm63158.dtsi    | 18 ++++++++++++++++++
 .../boot/dts/broadcom/bcmbca/bcm6813.dtsi     | 19 +++++++++++++++++++
 .../boot/dts/broadcom/bcmbca/bcm6856.dtsi     | 17 +++++++++++++++++
 .../boot/dts/broadcom/bcmbca/bcm6858.dtsi     | 17 +++++++++++++++++
 .../boot/dts/broadcom/bcmbca/bcm94908.dts     |  4 ++++
 .../boot/dts/broadcom/bcmbca/bcm94912.dts     |  4 ++++
 .../boot/dts/broadcom/bcmbca/bcm963146.dts    |  4 ++++
 .../boot/dts/broadcom/bcmbca/bcm963158.dts    |  4 ++++
 .../boot/dts/broadcom/bcmbca/bcm96813.dts     |  4 ++++
 .../boot/dts/broadcom/bcmbca/bcm96856.dts     |  4 ++++
 .../boot/dts/broadcom/bcmbca/bcm96858.dts     |  4 ++++
 14 files changed, 153 insertions(+)

diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi
index eb2a78f4e033..169045215d91 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi
@@ -107,6 +107,12 @@ periph_clk: periph_clk {
 			clock-frequency = <50000000>;
 			clock-output-names = "periph";
 		};
+
+		hsspi_pll: hsspi-pll {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <400000000>;
+		};
 	};
 
 	soc {
@@ -531,6 +537,17 @@ leds: leds@800 {
 			#size-cells = <0>;
 		};
 
+		hsspi: spi@1000{
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "brcm,bcm6328-hsspi";
+			reg = <0x1000 0x600>;
+			clocks = <&hsspi_pll &hsspi_pll>;
+			clock-names = "hsspi", "pll";
+			num-cs = <8>;
+			status = "disabled";
+		};
+
 		nand-controller@1800 {
 			#address-cells = <1>;
 			#size-cells = <0>;
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4912.dtsi b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4912.dtsi
index d5bc31980f03..ebacfffc4264 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4912.dtsi
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4912.dtsi
@@ -79,6 +79,7 @@ periph_clk: periph-clk {
 			#clock-cells = <0>;
 			clock-frequency = <200000000>;
 		};
+
 		uart_clk: uart-clk {
 			compatible = "fixed-factor-clock";
 			#clock-cells = <0>;
@@ -86,6 +87,12 @@ uart_clk: uart-clk {
 			clock-div = <4>;
 			clock-mult = <1>;
 		};
+
+		hsspi_pll: hsspi-pll {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <200000000>;
+		};
 	};
 
 	psci {
@@ -117,6 +124,18 @@ bus@ff800000 {
 		#size-cells = <1>;
 		ranges = <0x0 0x0 0xff800000 0x800000>;
 
+		hsspi: spi@1000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "brcm,bcmbca-hsspi";
+			reg = <0x1000 0x600>, <0x2610 0x4>;
+			reg-names = "hsspi", "spim-ctrl";
+			clocks = <&hsspi_pll &hsspi_pll>;
+			clock-names = "hsspi", "pll";
+			num-cs = <8>;
+			status = "disabled";
+		};
+
 		uart0: serial@12000 {
 			compatible = "arm,pl011", "arm,primecell";
 			reg = <0x12000 0x1000>;
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm63146.dtsi b/arch/arm64/boot/dts/broadcom/bcmbca/bcm63146.dtsi
index 6f805266d3c9..184f8975d8f2 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm63146.dtsi
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm63146.dtsi
@@ -60,6 +60,7 @@ periph_clk: periph-clk {
 			#clock-cells = <0>;
 			clock-frequency = <200000000>;
 		};
+
 		uart_clk: uart-clk {
 			compatible = "fixed-factor-clock";
 			#clock-cells = <0>;
@@ -67,6 +68,12 @@ uart_clk: uart-clk {
 			clock-div = <4>;
 			clock-mult = <1>;
 		};
+
+		hsspi_pll: hsspi-pll {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <200000000>;
+		};
 	};
 
 	psci {
@@ -99,6 +106,17 @@ bus@ff800000 {
 		#size-cells = <1>;
 		ranges = <0x0 0x0 0xff800000 0x800000>;
 
+		hsspi: spi@1000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "brcm,bcm6328-hsspi";
+			reg = <0x1000 0x600>;
+			clocks = <&hsspi_pll &hsspi_pll>;
+			clock-names = "hsspi", "pll";
+			num-cs = <8>;
+			status = "disabled";
+		};
+
 		uart0: serial@12000 {
 			compatible = "arm,pl011", "arm,primecell";
 			reg = <0x12000 0x1000>;
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm63158.dtsi b/arch/arm64/boot/dts/broadcom/bcmbca/bcm63158.dtsi
index b982249b80a2..4036ddc3c833 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm63158.dtsi
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm63158.dtsi
@@ -79,6 +79,7 @@ periph_clk: periph-clk {
 			#clock-cells = <0>;
 			clock-frequency = <200000000>;
 		};
+
 		uart_clk: uart-clk {
 			compatible = "fixed-factor-clock";
 			#clock-cells = <0>;
@@ -86,6 +87,12 @@ uart_clk: uart-clk {
 			clock-div = <4>;
 			clock-mult = <1>;
 		};
+
+		hsspi_pll: hsspi-pll {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <400000000>;
+		};
 	};
 
 	psci {
@@ -117,6 +124,17 @@ bus@ff800000 {
 		#size-cells = <1>;
 		ranges = <0x0 0x0 0xff800000 0x800000>;
 
+		hsspi: spi@1000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "brcm,bcm6328-hsspi";
+			reg = <0x1000 0x600>;
+			clocks = <&hsspi_pll &hsspi_pll>;
+			clock-names = "hsspi", "pll";
+			num-cs = <8>;
+			status = "disabled";
+		};
+
 		uart0: serial@12000 {
 			compatible = "arm,pl011", "arm,primecell";
 			reg = <0x12000 0x1000>;
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm6813.dtsi b/arch/arm64/boot/dts/broadcom/bcmbca/bcm6813.dtsi
index a996d436e977..d29738e6fd67 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm6813.dtsi
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm6813.dtsi
@@ -79,6 +79,7 @@ periph_clk: periph-clk {
 			#clock-cells = <0>;
 			clock-frequency = <200000000>;
 		};
+
 		uart_clk: uart-clk {
 			compatible = "fixed-factor-clock";
 			#clock-cells = <0>;
@@ -86,6 +87,12 @@ uart_clk: uart-clk {
 			clock-div = <4>;
 			clock-mult = <1>;
 		};
+
+		hsspi_pll: hsspi-pll {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <200000000>;
+		};
 	};
 
 	psci {
@@ -117,6 +124,18 @@ bus@ff800000 {
 		#size-cells = <1>;
 		ranges = <0x0 0x0 0xff800000 0x800000>;
 
+		hsspi: spi@1000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "brcm,bcmbca-hsspi";
+			reg = <0x1000 0x600>, <0x2610 0x4>;
+			reg-names = "hsspi", "spim-ctrl";
+			clocks = <&hsspi_pll &hsspi_pll>;
+			clock-names = "hsspi", "pll";
+			num-cs = <8>;
+			status = "disabled";
+		};
+
 		uart0: serial@12000 {
 			compatible = "arm,pl011", "arm,primecell";
 			reg = <0x12000 0x1000>;
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm6856.dtsi b/arch/arm64/boot/dts/broadcom/bcmbca/bcm6856.dtsi
index 62c530d4b103..6c5faf175551 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm6856.dtsi
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm6856.dtsi
@@ -60,6 +60,12 @@ periph_clk:periph-clk {
 			#clock-cells = <0>;
 			clock-frequency = <200000000>;
 		};
+
+		hsspi_pll: hsspi-pll {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <400000000>;
+		};
 	};
 
 	psci {
@@ -100,5 +106,16 @@ uart0: serial@640 {
 			clock-names = "refclk";
 			status = "disabled";
 		};
+
+		hsspi: spi@1000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "brcm,bcm6328-hsspi";
+			reg = <0x1000 0x600>;
+			clocks = <&hsspi_pll &hsspi_pll>;
+			clock-names = "hsspi", "pll";
+			num-cs = <8>;
+			status = "disabled";
+		};
 	};
 };
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm6858.dtsi b/arch/arm64/boot/dts/broadcom/bcmbca/bcm6858.dtsi
index 34c7b513d363..edc0003457fd 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm6858.dtsi
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm6858.dtsi
@@ -78,6 +78,12 @@ periph_clk:periph-clk {
 			#clock-cells = <0>;
 			clock-frequency = <200000000>;
 		};
+
+		hsspi_pll: hsspi-pll {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <400000000>;
+		};
 	};
 
 	psci {
@@ -137,5 +143,16 @@ uart0: serial@640 {
 			clock-names = "refclk";
 			status = "disabled";
 		};
+
+		hsspi: spi@1000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "brcm,bcm6328-hsspi";
+			reg = <0x1000 0x600>;
+			clocks = <&hsspi_pll &hsspi_pll>;
+			clock-names = "hsspi", "pll";
+			num-cs = <8>;
+			status = "disabled";
+		};
 	};
 };
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm94908.dts b/arch/arm64/boot/dts/broadcom/bcmbca/bcm94908.dts
index fcbd3c430ace..c4e6e71f6310 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm94908.dts
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm94908.dts
@@ -28,3 +28,7 @@ memory@0 {
 &uart0 {
 	status = "okay";
 };
+
+&hsspi {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm94912.dts b/arch/arm64/boot/dts/broadcom/bcmbca/bcm94912.dts
index a3623e6f6919..e69cd683211a 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm94912.dts
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm94912.dts
@@ -28,3 +28,7 @@ memory@0 {
 &uart0 {
 	status = "okay";
 };
+
+&hsspi {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm963146.dts b/arch/arm64/boot/dts/broadcom/bcmbca/bcm963146.dts
index e39f1e6d4774..db2c82d6dfd8 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm963146.dts
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm963146.dts
@@ -28,3 +28,7 @@ memory@0 {
 &uart0 {
 	status = "okay";
 };
+
+&hsspi {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm963158.dts b/arch/arm64/boot/dts/broadcom/bcmbca/bcm963158.dts
index eba07e0b1ca6..25c12bc63545 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm963158.dts
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm963158.dts
@@ -28,3 +28,7 @@ memory@0 {
 &uart0 {
 	status = "okay";
 };
+
+&hsspi {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm96813.dts b/arch/arm64/boot/dts/broadcom/bcmbca/bcm96813.dts
index af17091ae764..faba21f03120 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm96813.dts
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm96813.dts
@@ -28,3 +28,7 @@ memory@0 {
 &uart0 {
 	status = "okay";
 };
+
+&hsspi {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm96856.dts b/arch/arm64/boot/dts/broadcom/bcmbca/bcm96856.dts
index 032aeb75c983..9808331eede2 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm96856.dts
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm96856.dts
@@ -28,3 +28,7 @@ memory@0 {
 &uart0 {
 	status = "okay";
 };
+
+&hsspi {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm96858.dts b/arch/arm64/boot/dts/broadcom/bcmbca/bcm96858.dts
index 0cbf582f5d54..1f561c8e13b0 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm96858.dts
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm96858.dts
@@ -28,3 +28,7 @@ memory@0 {
 &uart0 {
 	status = "okay";
 };
+
+&hsspi {
+	status = "okay";
+};
-- 
2.37.3

Re: [PATCH 03/16] dt-bindings: spi: Add spi peripheral specific property

[Mark Brown]

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

On Fri, Jan 06, 2023 at 12:07:55PM -0800, William Zhang wrote:

> brcm,no-clk-gate is a Broadcom Broadband HS SPI controller specific
> property for certain SPI device such as Broadcom ISI voice daughtercard
> to work properly. It disables the clock gating feature when the chip
> select is deasserted for any device that wants to keep the clock
> running.

Why would this property be Broadcom specific?  Other devices could in
theory implement this.

> +properties:
> +  brcm,no-clk-gate:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description:
> +      Some SPI device such as Broadcom ISI based voice daughtercard requires SPI
> +      clock running even when chip select is deasserted. By default the
> +      controller turns off or gate the clock when cs is not active to save
> +      power. This flag tells the controller driver to keep the clock running
> +      when chip select is not active.

This seems problematic with any host controlled chip select support...

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 03/16] dt-bindings: spi: Add spi peripheral specific property

[William Zhang]

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

Hi Mark,

On 01/06/2023 01:14 PM, Mark Brown wrote:
> On Fri, Jan 06, 2023 at 12:07:55PM -0800, William Zhang wrote:
> 
>> brcm,no-clk-gate is a Broadcom Broadband HS SPI controller specific
>> property for certain SPI device such as Broadcom ISI voice daughtercard
>> to work properly. It disables the clock gating feature when the chip
>> select is deasserted for any device that wants to keep the clock
>> running.
> 
> Why would this property be Broadcom specific?  Other devices could in
> theory implement this.
> 
It does not need to be Broadcom specific if other SoC's SPI bus 
controller support such function. I am not aware of such case but 
certainly I am no expert on other chips. I can put it in the generic 
spi-peripheral-props.yaml if that is what you suggest.

>> +properties:
>> +  brcm,no-clk-gate:
>> +    $ref: /schemas/types.yaml#/definitions/flag
>> +    description:
>> +      Some SPI device such as Broadcom ISI based voice daughtercard requires SPI
>> +      clock running even when chip select is deasserted. By default the
>> +      controller turns off or gate the clock when cs is not active to save
>> +      power. This flag tells the controller driver to keep the clock running
>> +      when chip select is not active.
> 
> This seems problematic with any host controlled chip select support...
> 
Yes those ISI chip based voice cards do need such strange requirement 
and will not work with other controller.  That is one of the reason I 
put this as Broadcom specific option.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

Re: [PATCH 01/16] dt-bindings: spi: Convert bcm63xx-hsspi bindings to json-schema

[Rob Herring]

On Fri, 06 Jan 2023 12:07:53 -0800, William Zhang wrote:
> This is the preparation for updates on the bcm63xx hsspi driver. Convert
> the text based bindings to json-schema per new dts requirement.
> 
> Signed-off-by: William Zhang <william.zhang@broadcom.com>
> ---
> 
>  .../bindings/spi/brcm,bcm63xx-hsspi.yaml      | 52 +++++++++++++++++++
>  .../bindings/spi/spi-bcm63xx-hsspi.txt        | 33 ------------
>  2 files changed, 52 insertions(+), 33 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
>  delete mode 100644 Documentation/devicetree/bindings/spi/spi-bcm63xx-hsspi.txt
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.example.dtb: spi@10001000: Unevaluated properties are not allowed ('#address-cells', '#size-cells', 'num-cs' were unexpected)
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230106200809.330769-2-william.zhang@broadcom.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 01/16] dt-bindings: spi: Convert bcm63xx-hsspi bindings to json-schema

[Krzysztof Kozlowski]

On 06/01/2023 21:07, William Zhang wrote:
> This is the preparation for updates on the bcm63xx hsspi driver. Convert
> the text based bindings to json-schema per new dts requirement.
> 
> Signed-off-by: William Zhang <william.zhang@broadcom.com>
> ---
> 
>  .../bindings/spi/brcm,bcm63xx-hsspi.yaml      | 52 +++++++++++++++++++
>  .../bindings/spi/spi-bcm63xx-hsspi.txt        | 33 ------------
>  2 files changed, 52 insertions(+), 33 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
>  delete mode 100644 Documentation/devicetree/bindings/spi/spi-bcm63xx-hsspi.txt
> 
> diff --git a/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
> new file mode 100644
> index 000000000000..45f1417b1213
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
> @@ -0,0 +1,52 @@
> +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/spi/brcm,bcm63xx-hsspi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Broadcom BCM6328 High Speed SPI controller
> +
> +maintainers:
> +  - Jonas Gorski <jonas.gorski@gmail.com>
> +

Missing reference to spi-controller.

> +properties:
> +  compatible:
> +    const: brcm,bcm6328-hsspi
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: spi master reference clock
> +      - description: spi master pll clock
> +
> +  clock-names:
> +    items:
> +      - const: hsspi
> +      - const: pll
> +
> +  interrupts:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - interrupts
> +
> +unevaluatedProperties: false

This is for cases when you have reference to other schema.


Best regards,
Krzysztof

Re: [PATCH 03/16] dt-bindings: spi: Add spi peripheral specific property

[Rob Herring]

On Fri, Jan 6, 2023 at 9:27 PM William Zhang <william.zhang@broadcom.com> wrote:
>
> Hi Mark,
>
> On 01/06/2023 01:14 PM, Mark Brown wrote:
> > On Fri, Jan 06, 2023 at 12:07:55PM -0800, William Zhang wrote:
> >
> >> brcm,no-clk-gate is a Broadcom Broadband HS SPI controller specific
> >> property for certain SPI device such as Broadcom ISI voice daughtercard
> >> to work properly. It disables the clock gating feature when the chip
> >> select is deasserted for any device that wants to keep the clock
> >> running.
> >
> > Why would this property be Broadcom specific?  Other devices could in
> > theory implement this.
> >
> It does not need to be Broadcom specific if other SoC's SPI bus
> controller support such function. I am not aware of such case but
> certainly I am no expert on other chips. I can put it in the generic
> spi-peripheral-props.yaml if that is what you suggest.
>
> >> +properties:
> >> +  brcm,no-clk-gate:
> >> +    $ref: /schemas/types.yaml#/definitions/flag
> >> +    description:
> >> +      Some SPI device such as Broadcom ISI based voice daughtercard requires SPI
> >> +      clock running even when chip select is deasserted. By default the
> >> +      controller turns off or gate the clock when cs is not active to save
> >> +      power. This flag tells the controller driver to keep the clock running
> >> +      when chip select is not active.
> >
> > This seems problematic with any host controlled chip select support...
> >
> Yes those ISI chip based voice cards do need such strange requirement
> and will not work with other controller.  That is one of the reason I
> put this as Broadcom specific option.

Keeping the clock on or not would affect all devices unless you have a
per device clock you can gate, so making this a per device flag
doesn't make sense.

If this is a requirement of the slave device, then the device's
compatible string can imply the need for this and its driver can tell
the host controller in some way.

Rob

Re: [PATCH 02/16] dt-bindings: spi: Add bcmbca-hsspi controller support

[Krzysztof Kozlowski]

On 06/01/2023 21:07, William Zhang wrote:
> The new Broadcom Broadband BCMBCA SoCs includes a updated HSSPI
> controller. Add a new compatible string and required fields for the new
> driver.  Also add myself and Kursad as the maintainers.
> 
> Signed-off-by: William Zhang <william.zhang@broadcom.com>
> ---
> 
>  .../bindings/spi/brcm,bcm63xx-hsspi.yaml      | 84 +++++++++++++++++--
>  1 file changed, 78 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
> index 45f1417b1213..56e69d4a1faf 100644
> --- a/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
> +++ b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
> @@ -4,22 +4,51 @@
>  $id: http://devicetree.org/schemas/spi/brcm,bcm63xx-hsspi.yaml#
>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>  
> -title: Broadcom BCM6328 High Speed SPI controller
> +title: Broadcom Broadband SoC High Speed SPI controller
>  
>  maintainers:
> +

Drop blank line.

> +  - William Zhang <william.zhang@broadcom.com>
> +  - Kursad Oney <kursad.oney@broadcom.com>
>    - Jonas Gorski <jonas.gorski@gmail.com>

>  
> +description: |
> +  Broadcom Broadband SoC supports High Speed SPI master controller since the
> +  early MIPS based chips such as BCM6328 and BCM63268.  This controller was
> +  carried over to recent ARM based chips, such as BCM63138, BCM4908 and BCM6858.
> +
> +  It has a limitation that can not keep the chip select line active between
> +  the SPI transfers within the same SPI message. This can terminate the
> +  transaction to some SPI devices prematurely. The issue can be worked around by
> +  either the controller's prepend mode or using the dummy chip select
> +  workaround. This controller uses the compatible string brcm,bcm6328-hsspi.
> +
> +  The newer SoCs such as BCM6756, BCM4912 and BCM6855 include an updated SPI
> +  controller that add the capability to allow the driver to control chip select
> +  explicitly. This solves the issue in the old controller. This new controller
> +  uses the compatible string brcm,bcmbca-hsspi.
> +
>  properties:
>    compatible:
> -    const: brcm,bcm6328-hsspi
> +    enum:
> +      - brcm,bcm6328-hsspi
> +      - brcm,bcmbca-hsspi

bca seems quite unspecific. Your description above mentions several
model numbers and "bca" is not listed as model. Compatibles cannot be
generic.

>  
>    reg:
> -    maxItems: 1
> +    items:
> +      - description: main registers
> +      - description: miscellaneous control registers
> +    minItems: 1
> +
> +  reg-names:
> +    items:
> +      - const: hsspi
> +      - const: spim-ctrl

This does not match reg

>  
>    clocks:
>      items:
> -      - description: spi master reference clock
> -      - description: spi master pll clock
> +      - description: SPI master reference clock
> +      - description: SPI master pll clock

Really? You just added it in previous patch, didn't you?

>  
>    clock-names:
>      items:
> @@ -29,12 +58,43 @@ properties:
>    interrupts:
>      maxItems: 1
>  
> +  brcm,use-cs-workaround:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: |
> +      Enable dummy chip select workaround for SPI transfers that can not be
> +      supported by the default controller's prepend mode, i.e. delay or cs
> +      change needed between SPI transfers.

You need to describe what is the workaround.

> +
>  required:
>    - compatible
>    - reg
>    - clocks
>    - clock-names
> -  - interrupts
> +
> +allOf:
> +  - $ref: "spi-controller.yaml#"

No quotes. How this is related to this patch?

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - brcm,bcm6328-hsspi
> +    then:
> +      properties:
> +        reg:
> +          minItems: 1

Drop.

reg-names now do not match.

> +          maxItems: 1
> +    else:
> +      properties:
> +        reg:
> +          minItems: 2
> +          maxItems: 2
> +        reg-names:
> +          minItems: 2
> +          maxItems: 2
> +        brcm,use-cs-workaround: false
> +      required:
> +        - reg-names
Best regards,
Krzysztof

Re: [PATCH 03/16] dt-bindings: spi: Add spi peripheral specific property

[Krzysztof Kozlowski]

On 06/01/2023 21:07, William Zhang wrote:
> brcm,no-clk-gate is a Broadcom Broadband HS SPI controller specific
> property for certain SPI device such as Broadcom ISI voice daughtercard
> to work properly. It disables the clock gating feature when the chip
> select is deasserted for any device that wants to keep the clock
> running.


> +additionalProperties: true
> diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> index ead2cccf658f..f85d777c7b67 100644
> --- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> +++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
> @@ -108,5 +108,6 @@ allOf:
>    - $ref: cdns,qspi-nor-peripheral-props.yaml#
>    - $ref: samsung,spi-peripheral-props.yaml#
>    - $ref: nvidia,tegra210-quad-peripheral-props.yaml#
> +  - $ref: brcm,bcm63xx-hsspi-peripheral-props.yaml#

Don't break the order.

Best regards,
Krzysztof

Re: [PATCH 01/16] dt-bindings: spi: Convert bcm63xx-hsspi bindings to json-schema

[William Zhang]

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



On 01/07/2023 07:32 AM, Krzysztof Kozlowski wrote:
> On 06/01/2023 21:07, William Zhang wrote:
>> This is the preparation for updates on the bcm63xx hsspi driver. Convert
>> the text based bindings to json-schema per new dts requirement.
>>
>> Signed-off-by: William Zhang <william.zhang@broadcom.com>
>> ---
>>
>>   .../bindings/spi/brcm,bcm63xx-hsspi.yaml      | 52 +++++++++++++++++++
>>   .../bindings/spi/spi-bcm63xx-hsspi.txt        | 33 ------------
>>   2 files changed, 52 insertions(+), 33 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
>>   delete mode 100644 Documentation/devicetree/bindings/spi/spi-bcm63xx-hsspi.txt
>>
>> diff --git a/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
>> new file mode 100644
>> index 000000000000..45f1417b1213
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
>> @@ -0,0 +1,52 @@
>> +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/spi/brcm,bcm63xx-hsspi.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Broadcom BCM6328 High Speed SPI controller
>> +
>> +maintainers:
>> +  - Jonas Gorski <jonas.gorski@gmail.com>
>> +
> 
> Missing reference to spi-controller.
> 
This was word to word conversion from the text file. But I will update 
with this required reference.

>> +properties:
>> +  compatible:
>> +    const: brcm,bcm6328-hsspi
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    items:
>> +      - description: spi master reference clock
>> +      - description: spi master pll clock
>> +
>> +  clock-names:
>> +    items:
>> +      - const: hsspi
>> +      - const: pll
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - clocks
>> +  - clock-names
>> +  - interrupts
>> +
>> +unevaluatedProperties: false
> 
> This is for cases when you have reference to other schema.
> 
Will drop here. But will add back in patch 1 which produces the final 
version of this file and need this property.

> 
> Best regards,
> Krzysztof
> 

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

Re: [PATCH 03/16] dt-bindings: spi: Add spi peripheral specific property

[William Zhang]

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

Hi Rob,

On 01/07/2023 07:38 AM, Rob Herring wrote:
> On Fri, Jan 6, 2023 at 9:27 PM William Zhang <william.zhang@broadcom.com> wrote:
>>
>> Hi Mark,
>>
>> On 01/06/2023 01:14 PM, Mark Brown wrote:
>>> On Fri, Jan 06, 2023 at 12:07:55PM -0800, William Zhang wrote:
>>>
>>>> brcm,no-clk-gate is a Broadcom Broadband HS SPI controller specific
>>>> property for certain SPI device such as Broadcom ISI voice daughtercard
>>>> to work properly. It disables the clock gating feature when the chip
>>>> select is deasserted for any device that wants to keep the clock
>>>> running.
>>>
>>> Why would this property be Broadcom specific?  Other devices could in
>>> theory implement this.
>>>
>> It does not need to be Broadcom specific if other SoC's SPI bus
>> controller support such function. I am not aware of such case but
>> certainly I am no expert on other chips. I can put it in the generic
>> spi-peripheral-props.yaml if that is what you suggest.
>>
>>>> +properties:
>>>> +  brcm,no-clk-gate:
>>>> +    $ref: /schemas/types.yaml#/definitions/flag
>>>> +    description:
>>>> +      Some SPI device such as Broadcom ISI based voice daughtercard requires SPI
>>>> +      clock running even when chip select is deasserted. By default the
>>>> +      controller turns off or gate the clock when cs is not active to save
>>>> +      power. This flag tells the controller driver to keep the clock running
>>>> +      when chip select is not active.
>>>
>>> This seems problematic with any host controlled chip select support...
>>>
>> Yes those ISI chip based voice cards do need such strange requirement
>> and will not work with other controller.  That is one of the reason I
>> put this as Broadcom specific option.
> 
> Keeping the clock on or not would affect all devices unless you have a
> per device clock you can gate, so making this a per device flag
> doesn't make sense.
> 
This applies only to each chip select. There is only one device under 
each chip select.  So won't impact any other devices under other cs.

> If this is a requirement of the slave device, then the device's
> compatible string can imply the need for this and its driver can tell
> the host controller in some way.
That is true but spi host controller driver reads and parses these slave 
device flag directly.
> 
> Rob
> 

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

Re: [PATCH 02/16] dt-bindings: spi: Add bcmbca-hsspi controller support

[William Zhang]

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

Hi Krzysztof,

On 01/08/2023 06:51 AM, Krzysztof Kozlowski wrote:
> On 06/01/2023 21:07, William Zhang wrote:
>> The new Broadcom Broadband BCMBCA SoCs includes a updated HSSPI
>> controller. Add a new compatible string and required fields for the new
>> driver.  Also add myself and Kursad as the maintainers.
>>
>> Signed-off-by: William Zhang <william.zhang@broadcom.com>
>> ---
>>
>>   .../bindings/spi/brcm,bcm63xx-hsspi.yaml      | 84 +++++++++++++++++--
>>   1 file changed, 78 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
>> index 45f1417b1213..56e69d4a1faf 100644
>> --- a/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
>> +++ b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
>> @@ -4,22 +4,51 @@
>>   $id: http://devicetree.org/schemas/spi/brcm,bcm63xx-hsspi.yaml#
>>   $schema: http://devicetree.org/meta-schemas/core.yaml#
>>   
>> -title: Broadcom BCM6328 High Speed SPI controller
>> +title: Broadcom Broadband SoC High Speed SPI controller
>>   
>>   maintainers:
>> +
> 
> Drop blank line.
will fix in  v2.

> 
>> +  - William Zhang <william.zhang@broadcom.com>
>> +  - Kursad Oney <kursad.oney@broadcom.com>
>>     - Jonas Gorski <jonas.gorski@gmail.com>
> 
>>   
>> +description: |
>> +  Broadcom Broadband SoC supports High Speed SPI master controller since the
>> +  early MIPS based chips such as BCM6328 and BCM63268.  This controller was
>> +  carried over to recent ARM based chips, such as BCM63138, BCM4908 and BCM6858.
>> +
>> +  It has a limitation that can not keep the chip select line active between
>> +  the SPI transfers within the same SPI message. This can terminate the
>> +  transaction to some SPI devices prematurely. The issue can be worked around by
>> +  either the controller's prepend mode or using the dummy chip select
>> +  workaround. This controller uses the compatible string brcm,bcm6328-hsspi.
>> +
>> +  The newer SoCs such as BCM6756, BCM4912 and BCM6855 include an updated SPI
>> +  controller that add the capability to allow the driver to control chip select
>> +  explicitly. This solves the issue in the old controller. This new controller
>> +  uses the compatible string brcm,bcmbca-hsspi.
>> +
>>   properties:
>>     compatible:
>> -    const: brcm,bcm6328-hsspi
>> +    enum:
>> +      - brcm,bcm6328-hsspi
>> +      - brcm,bcmbca-hsspi
> 
> bca seems quite unspecific. Your description above mentions several
> model numbers and "bca" is not listed as model. Compatibles cannot be
> generic.
"bca" is not model number, rather it is a group (broadband carrier 
access) of chip that share the same spi host controller IP. Agree it is 
not particularly specific but it differentiate from other broadcom spi 
controller ip used by other groups.  We just don't have a specific name 
for this spi host controller but can we treat bcmbca as the ip name? 
Otherwise we will have to have a compatible string with chip model for 
each SoC even they share the same IP. We already have more than ten of 
SoCs and the list will increase.  I don't see this is a good solution too.

> 
>>   
>>     reg:
>> -    maxItems: 1
>> +    items:
>> +      - description: main registers
>> +      - description: miscellaneous control registers
>> +    minItems: 1
>> +
>> +  reg-names:
>> +    items:
>> +      - const: hsspi
>> +      - const: spim-ctrl
> 
> This does not match reg
Do you mean it does not match the description?
> 
>>   
>>     clocks:
>>       items:
>> -      - description: spi master reference clock
>> -      - description: spi master pll clock
>> +      - description: SPI master reference clock
>> +      - description: SPI master pll clock
> 
> Really? You just added it in previous patch, didn't you?
The previous patch was just word to word conversion of the text file.  I 
will update that patch to include this change.

> 
>>   
>>     clock-names:
>>       items:
>> @@ -29,12 +58,43 @@ properties:
>>     interrupts:
>>       maxItems: 1
>>   
>> +  brcm,use-cs-workaround:
>> +    $ref: /schemas/types.yaml#/definitions/flag
>> +    description: |
>> +      Enable dummy chip select workaround for SPI transfers that can not be
>> +      supported by the default controller's prepend mode, i.e. delay or cs
>> +      change needed between SPI transfers.
> 
> You need to describe what is the workaround.
Will do.
> 
>> +
>>   required:
>>     - compatible
>>     - reg
>>     - clocks
>>     - clock-names
>> -  - interrupts
>> +
>> +allOf:
>> +  - $ref: "spi-controller.yaml#"
> 
> No quotes. How this is related to this patch?
Will remove quote and put it in patch 1.
> 
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - brcm,bcm6328-hsspi
>> +    then:
>> +      properties:
>> +        reg:
>> +          minItems: 1
> 
> Drop.
> 
> reg-names now do not match.
Don't quite understand your comment. What do I need to drop and what is 
not matched?

> 
>> +          maxItems: 1
>> +    else:
>> +      properties:
>> +        reg:
>> +          minItems: 2
>> +          maxItems: 2
>> +        reg-names:
>> +          minItems: 2
>> +          maxItems: 2
>> +        brcm,use-cs-workaround: false
>> +      required:
>> +        - reg-names
> Best regards,
> Krzysztof
> 

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

Re: [PATCH 03/16] dt-bindings: spi: Add spi peripheral specific property

[William Zhang]

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



On 01/08/2023 06:52 AM, Krzysztof Kozlowski wrote:
> On 06/01/2023 21:07, William Zhang wrote:
>> brcm,no-clk-gate is a Broadcom Broadband HS SPI controller specific
>> property for certain SPI device such as Broadcom ISI voice daughtercard
>> to work properly. It disables the clock gating feature when the chip
>> select is deasserted for any device that wants to keep the clock
>> running.
> 
> 
>> +additionalProperties: true
>> diff --git a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
>> index ead2cccf658f..f85d777c7b67 100644
>> --- a/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
>> +++ b/Documentation/devicetree/bindings/spi/spi-peripheral-props.yaml
>> @@ -108,5 +108,6 @@ allOf:
>>     - $ref: cdns,qspi-nor-peripheral-props.yaml#
>>     - $ref: samsung,spi-peripheral-props.yaml#
>>     - $ref: nvidia,tegra210-quad-peripheral-props.yaml#
>> +  - $ref: brcm,bcm63xx-hsspi-peripheral-props.yaml#
> 
> Don't break the order.
> 
Will fix in v2

> Best regards,
> Krzysztof
> 

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

Re: [PATCH 01/16] dt-bindings: spi: Convert bcm63xx-hsspi bindings to json-schema

[Krzysztof Kozlowski]

On 09/01/2023 08:52, William Zhang wrote:
> 
> 
> On 01/07/2023 07:32 AM, Krzysztof Kozlowski wrote:
>> On 06/01/2023 21:07, William Zhang wrote:
>>> This is the preparation for updates on the bcm63xx hsspi driver. Convert
>>> the text based bindings to json-schema per new dts requirement.
>>>
>>> Signed-off-by: William Zhang <william.zhang@broadcom.com>
>>> ---
>>>
>>>   .../bindings/spi/brcm,bcm63xx-hsspi.yaml      | 52 +++++++++++++++++++
>>>   .../bindings/spi/spi-bcm63xx-hsspi.txt        | 33 ------------
>>>   2 files changed, 52 insertions(+), 33 deletions(-)
>>>   create mode 100644 Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
>>>   delete mode 100644 Documentation/devicetree/bindings/spi/spi-bcm63xx-hsspi.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
>>> new file mode 100644
>>> index 000000000000..45f1417b1213
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
>>> @@ -0,0 +1,52 @@
>>> +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/spi/brcm,bcm63xx-hsspi.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Broadcom BCM6328 High Speed SPI controller
>>> +
>>> +maintainers:
>>> +  - Jonas Gorski <jonas.gorski@gmail.com>
>>> +
>>
>> Missing reference to spi-controller.
>>
> This was word to word conversion from the text file. But I will update 
> with this required reference.
> 
>>> +properties:
>>> +  compatible:
>>> +    const: brcm,bcm6328-hsspi
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  clocks:
>>> +    items:
>>> +      - description: spi master reference clock
>>> +      - description: spi master pll clock
>>> +
>>> +  clock-names:
>>> +    items:
>>> +      - const: hsspi
>>> +      - const: pll
>>> +
>>> +  interrupts:
>>> +    maxItems: 1
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +  - clocks
>>> +  - clock-names
>>> +  - interrupts
>>> +
>>> +unevaluatedProperties: false
>>
>> This is for cases when you have reference to other schema.
>>
> Will drop here. But will add back in patch 1 which produces the final 
> version of this file and need this property.

When you add reference to spi-controller, keep it here. This was wrong
when the reference was missing.

Best regards,
Krzysztof

Re: [PATCH 02/16] dt-bindings: spi: Add bcmbca-hsspi controller support

[Krzysztof Kozlowski]

On 09/01/2023 09:27, William Zhang wrote:
> Hi Krzysztof,
> 
> On 01/08/2023 06:51 AM, Krzysztof Kozlowski wrote:
>> On 06/01/2023 21:07, William Zhang wrote:
>>> The new Broadcom Broadband BCMBCA SoCs includes a updated HSSPI
>>> controller. Add a new compatible string and required fields for the new
>>> driver.  Also add myself and Kursad as the maintainers.
>>>
>>> Signed-off-by: William Zhang <william.zhang@broadcom.com>
>>> ---
>>>
>>>   .../bindings/spi/brcm,bcm63xx-hsspi.yaml      | 84 +++++++++++++++++--
>>>   1 file changed, 78 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
>>> index 45f1417b1213..56e69d4a1faf 100644
>>> --- a/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
>>> +++ b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
>>> @@ -4,22 +4,51 @@
>>>   $id: http://devicetree.org/schemas/spi/brcm,bcm63xx-hsspi.yaml#
>>>   $schema: http://devicetree.org/meta-schemas/core.yaml#
>>>   
>>> -title: Broadcom BCM6328 High Speed SPI controller
>>> +title: Broadcom Broadband SoC High Speed SPI controller
>>>   
>>>   maintainers:
>>> +
>>
>> Drop blank line.
> will fix in  v2.
> 
>>
>>> +  - William Zhang <william.zhang@broadcom.com>
>>> +  - Kursad Oney <kursad.oney@broadcom.com>
>>>     - Jonas Gorski <jonas.gorski@gmail.com>
>>
>>>   
>>> +description: |
>>> +  Broadcom Broadband SoC supports High Speed SPI master controller since the
>>> +  early MIPS based chips such as BCM6328 and BCM63268.  This controller was
>>> +  carried over to recent ARM based chips, such as BCM63138, BCM4908 and BCM6858.
>>> +
>>> +  It has a limitation that can not keep the chip select line active between
>>> +  the SPI transfers within the same SPI message. This can terminate the
>>> +  transaction to some SPI devices prematurely. The issue can be worked around by
>>> +  either the controller's prepend mode or using the dummy chip select
>>> +  workaround. This controller uses the compatible string brcm,bcm6328-hsspi.
>>> +
>>> +  The newer SoCs such as BCM6756, BCM4912 and BCM6855 include an updated SPI
>>> +  controller that add the capability to allow the driver to control chip select
>>> +  explicitly. This solves the issue in the old controller. This new controller
>>> +  uses the compatible string brcm,bcmbca-hsspi.
>>> +
>>>   properties:
>>>     compatible:
>>> -    const: brcm,bcm6328-hsspi
>>> +    enum:
>>> +      - brcm,bcm6328-hsspi
>>> +      - brcm,bcmbca-hsspi
>>
>> bca seems quite unspecific. Your description above mentions several
>> model numbers and "bca" is not listed as model. Compatibles cannot be
>> generic.
> "bca" is not model number, rather it is a group (broadband carrier 
> access) of chip that share the same spi host controller IP. Agree it is 
> not particularly specific but it differentiate from other broadcom spi 
> controller ip used by other groups.  We just don't have a specific name 
> for this spi host controller but can we treat bcmbca as the ip name? 

No, it is discouraged in such forms. Family or IP block compatibles
should be prepended with a specific compatible. There were many issues
when people insisted on generic or family compatibles...

> Otherwise we will have to have a compatible string with chip model for 
> each SoC even they share the same IP. We already have more than ten of 
> SoCs and the list will increase.  I don't see this is a good solution too.

You will have to do it anyway even with generic fallback, so I don't get
what is here to gain... I also don't get why Broadcom should be here
special, different than others. Why it is not a good solution for
Broadcom SoCs but it is for others?



> 
>>
>>>   
>>>     reg:
>>> -    maxItems: 1
>>> +    items:
>>> +      - description: main registers
>>> +      - description: miscellaneous control registers
>>> +    minItems: 1
>>> +
>>> +  reg-names:
>>> +    items:
>>> +      - const: hsspi
>>> +      - const: spim-ctrl
>>
>> This does not match reg
> Do you mean it does not match the description?

No. reg can be 1 item but you state reg-names cannot. These are always
the same. If one is 1 item, second is as well.

>>
>>>   
>>>     clocks:
>>>       items:
>>> -      - description: spi master reference clock
>>> -      - description: spi master pll clock
>>> +      - description: SPI master reference clock
>>> +      - description: SPI master pll clock
>>
>> Really? You just added it in previous patch, didn't you?
> The previous patch was just word to word conversion of the text file.  I 
> will update that patch to include this change.
> 
>>
>>>   
>>>     clock-names:
>>>       items:
>>> @@ -29,12 +58,43 @@ properties:
>>>     interrupts:
>>>       maxItems: 1
>>>   
>>> +  brcm,use-cs-workaround:
>>> +    $ref: /schemas/types.yaml#/definitions/flag
>>> +    description: |
>>> +      Enable dummy chip select workaround for SPI transfers that can not be
>>> +      supported by the default controller's prepend mode, i.e. delay or cs
>>> +      change needed between SPI transfers.
>>
>> You need to describe what is the workaround.
> Will do.
>>
>>> +
>>>   required:
>>>     - compatible
>>>     - reg
>>>     - clocks
>>>     - clock-names
>>> -  - interrupts
>>> +
>>> +allOf:
>>> +  - $ref: "spi-controller.yaml#"
>>
>> No quotes. How this is related to this patch?
> Will remove quote and put it in patch 1.
>>
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - brcm,bcm6328-hsspi
>>> +    then:
>>> +      properties:
>>> +        reg:
>>> +          minItems: 1
>>
>> Drop.
>>
>> reg-names now do not match.
> Don't quite understand your comment. What do I need to drop and what is 
> not matched?

You need to add constraints for reg-names, same way as for reg.
Disallowing the reg-names also could work, but there won't be benefit in
it. Better to have uniform DTS.

> 
Best regards,
Krzysztof

Re: [PATCH 02/16] dt-bindings: spi: Add bcmbca-hsspi controller support

[William Zhang]

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



On 01/09/2023 12:56 AM, Krzysztof Kozlowski wrote:
> On 09/01/2023 09:27, William Zhang wrote:
>> Hi Krzysztof,
>>
>> On 01/08/2023 06:51 AM, Krzysztof Kozlowski wrote:
>>> On 06/01/2023 21:07, William Zhang wrote:
>>>> The new Broadcom Broadband BCMBCA SoCs includes a updated HSSPI
>>>> controller. Add a new compatible string and required fields for the new
>>>> driver.  Also add myself and Kursad as the maintainers.
>>>>
>>>> Signed-off-by: William Zhang <william.zhang@broadcom.com>
>>>> ---
>>>>
>>>>    .../bindings/spi/brcm,bcm63xx-hsspi.yaml      | 84 +++++++++++++++++--
>>>>    1 file changed, 78 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
>>>> index 45f1417b1213..56e69d4a1faf 100644
>>>> --- a/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
>>>> +++ b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
>>>> @@ -4,22 +4,51 @@
>>>>    $id: http://devicetree.org/schemas/spi/brcm,bcm63xx-hsspi.yaml#
>>>>    $schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>    
>>>> -title: Broadcom BCM6328 High Speed SPI controller
>>>> +title: Broadcom Broadband SoC High Speed SPI controller
>>>>    
>>>>    maintainers:
>>>> +
>>>
>>> Drop blank line.
>> will fix in  v2.
>>
>>>
>>>> +  - William Zhang <william.zhang@broadcom.com>
>>>> +  - Kursad Oney <kursad.oney@broadcom.com>
>>>>      - Jonas Gorski <jonas.gorski@gmail.com>
>>>
>>>>    
>>>> +description: |
>>>> +  Broadcom Broadband SoC supports High Speed SPI master controller since the
>>>> +  early MIPS based chips such as BCM6328 and BCM63268.  This controller was
>>>> +  carried over to recent ARM based chips, such as BCM63138, BCM4908 and BCM6858.
>>>> +
>>>> +  It has a limitation that can not keep the chip select line active between
>>>> +  the SPI transfers within the same SPI message. This can terminate the
>>>> +  transaction to some SPI devices prematurely. The issue can be worked around by
>>>> +  either the controller's prepend mode or using the dummy chip select
>>>> +  workaround. This controller uses the compatible string brcm,bcm6328-hsspi.
>>>> +
>>>> +  The newer SoCs such as BCM6756, BCM4912 and BCM6855 include an updated SPI
>>>> +  controller that add the capability to allow the driver to control chip select
>>>> +  explicitly. This solves the issue in the old controller. This new controller
>>>> +  uses the compatible string brcm,bcmbca-hsspi.
>>>> +
>>>>    properties:
>>>>      compatible:
>>>> -    const: brcm,bcm6328-hsspi
>>>> +    enum:
>>>> +      - brcm,bcm6328-hsspi
>>>> +      - brcm,bcmbca-hsspi
>>>
>>> bca seems quite unspecific. Your description above mentions several
>>> model numbers and "bca" is not listed as model. Compatibles cannot be
>>> generic.
>> "bca" is not model number, rather it is a group (broadband carrier
>> access) of chip that share the same spi host controller IP. Agree it is
>> not particularly specific but it differentiate from other broadcom spi
>> controller ip used by other groups.  We just don't have a specific name
>> for this spi host controller but can we treat bcmbca as the ip name?
> 
> No, it is discouraged in such forms. Family or IP block compatibles
> should be prepended with a specific compatible. There were many issues
> when people insisted on generic or family compatibles...
> 
>> Otherwise we will have to have a compatible string with chip model for
>> each SoC even they share the same IP. We already have more than ten of
>> SoCs and the list will increase.  I don't see this is a good solution too.
> 
> You will have to do it anyway even with generic fallback, so I don't get
> what is here to gain... I also don't get why Broadcom should be here
> special, different than others. Why it is not a good solution for
> Broadcom SoCs but it is for others?
>
I saw a few other vendors like these qcom ones:
  qcom,spi-qup.yaml
      - qcom,spi-qup-v1.1.1 # for 8660, 8960 and 8064
      - qcom,spi-qup-v2.1.1 # for 8974 and later
      - qcom,spi-qup-v2.2.1 # for 8974 v2 and later
  qcom,spi-qup.yaml
      const: qcom,geni-spi

I guess when individual who only has one particular board/chip and is 
not aware of the IP family,  it is understandable to use the chip 
specific compatible string. But when company works on it, we have the 
visibility and access to all the chips and ip blocks in the family and 
IMHO it is very reasonable to use the IP family name for the same IP as 
these examples shows. Are you saying these are not good example to 
follow?  What are the issues with generic or family compatibles?  Could 
you please elaborate?

> 
> 
>>
>>>
>>>>    
>>>>      reg:
>>>> -    maxItems: 1
>>>> +    items:
>>>> +      - description: main registers
>>>> +      - description: miscellaneous control registers
>>>> +    minItems: 1
>>>> +
>>>> +  reg-names:
>>>> +    items:
>>>> +      - const: hsspi
>>>> +      - const: spim-ctrl
>>>
>>> This does not match reg
>> Do you mean it does not match the description?
> 
> No. reg can be 1 item but you state reg-names cannot. These are always
> the same. If one is 1 item, second is as well.
> 
I'll drop the "minItems: 1" from the reg property then.

>>>
>>>>    
>>>>      clocks:
>>>>        items:
>>>> -      - description: spi master reference clock
>>>> -      - description: spi master pll clock
>>>> +      - description: SPI master reference clock
>>>> +      - description: SPI master pll clock
>>>
>>> Really? You just added it in previous patch, didn't you?
>> The previous patch was just word to word conversion of the text file.  I
>> will update that patch to include this change.
>>
>>>
>>>>    
>>>>      clock-names:
>>>>        items:
>>>> @@ -29,12 +58,43 @@ properties:
>>>>      interrupts:
>>>>        maxItems: 1
>>>>    
>>>> +  brcm,use-cs-workaround:
>>>> +    $ref: /schemas/types.yaml#/definitions/flag
>>>> +    description: |
>>>> +      Enable dummy chip select workaround for SPI transfers that can not be
>>>> +      supported by the default controller's prepend mode, i.e. delay or cs
>>>> +      change needed between SPI transfers.
>>>
>>> You need to describe what is the workaround.
>> Will do.
>>>
>>>> +
>>>>    required:
>>>>      - compatible
>>>>      - reg
>>>>      - clocks
>>>>      - clock-names
>>>> -  - interrupts
>>>> +
>>>> +allOf:
>>>> +  - $ref: "spi-controller.yaml#"
>>>
>>> No quotes. How this is related to this patch?
>> Will remove quote and put it in patch 1.
>>>
>>>> +  - if:
>>>> +      properties:
>>>> +        compatible:
>>>> +          contains:
>>>> +            enum:
>>>> +              - brcm,bcm6328-hsspi
>>>> +    then:
>>>> +      properties:
>>>> +        reg:
>>>> +          minItems: 1
>>>
>>> Drop.
>>>
>>> reg-names now do not match.
>> Don't quite understand your comment. What do I need to drop and what is
>> not matched?
> 
> You need to add constraints for reg-names, same way as for reg.
> Disallowing the reg-names also could work, but there won't be benefit in
> it. Better to have uniform DTS.
> 
I agree it is better to have the uniform DTS but the situation here is 
that the brcm,bcm6328-hsspi does not require reg name since there is 
only one register needed and it was already used in many chip dts for 
long time.  If I enforce it to have the corresponding reg name, that 
could potentially break the compatibility of those old device if the 
driver change to use reg name, right?

>>
> Best regards,
> Krzysztof
> 

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

Re: [PATCH 03/16] dt-bindings: spi: Add spi peripheral specific property

[Mark Brown]

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

On Mon, Jan 09, 2023 at 12:06:13AM -0800, William Zhang wrote:
> > On Fri, Jan 6, 2023 at 9:27 PM William Zhang <william.zhang@broadcom.com> wrote:

> > Keeping the clock on or not would affect all devices unless you have a
> > per device clock you can gate, so making this a per device flag
> > doesn't make sense.

> This applies only to each chip select. There is only one device under each
> chip select.  So won't impact any other devices under other cs.

I don't understand how this would work - usually a SPI controller has a
single set of clock, MOSI and MISO lines with the only per device thing
being the chip select.  If the clock line is used by all devices then it
must be kept on for all of them if it's to be kept on for one of them.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 03/16] dt-bindings: spi: Add spi peripheral specific property

[William Zhang]

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

Hi Mark,

On 01/09/2023 11:19 AM, Mark Brown wrote:
> On Mon, Jan 09, 2023 at 12:06:13AM -0800, William Zhang wrote:
>>> On Fri, Jan 6, 2023 at 9:27 PM William Zhang <william.zhang@broadcom.com> wrote:
> 
>>> Keeping the clock on or not would affect all devices unless you have a
>>> per device clock you can gate, so making this a per device flag
>>> doesn't make sense.
> 
>> This applies only to each chip select. There is only one device under each
>> chip select.  So won't impact any other devices under other cs.
> 
> I don't understand how this would work - usually a SPI controller has a
> single set of clock, MOSI and MISO lines with the only per device thing
> being the chip select.  If the clock line is used by all devices then it
> must be kept on for all of them if it's to be kept on for one of them.
> 

This setting is set per spi message for particular chip select of the 
device when starting the message through bcm63xx_hsspi_set_clk function 
and restore to default(clock gating) when message is done through 
bcm63xx_hsspi_restore_clk_gate.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

Re: [PATCH 02/16] dt-bindings: spi: Add bcmbca-hsspi controller support

[Krzysztof Kozlowski]

On 09/01/2023 20:13, William Zhang wrote:
> 
> 
> On 01/09/2023 12:56 AM, Krzysztof Kozlowski wrote:
>> On 09/01/2023 09:27, William Zhang wrote:
>>> Hi Krzysztof,
>>>
>>> On 01/08/2023 06:51 AM, Krzysztof Kozlowski wrote:
>>>> On 06/01/2023 21:07, William Zhang wrote:
>>>>> The new Broadcom Broadband BCMBCA SoCs includes a updated HSSPI
>>>>> controller. Add a new compatible string and required fields for the new
>>>>> driver.  Also add myself and Kursad as the maintainers.
>>>>>
>>>>> Signed-off-by: William Zhang <william.zhang@broadcom.com>
>>>>> ---
>>>>>
>>>>>    .../bindings/spi/brcm,bcm63xx-hsspi.yaml      | 84 +++++++++++++++++--
>>>>>    1 file changed, 78 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
>>>>> index 45f1417b1213..56e69d4a1faf 100644
>>>>> --- a/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
>>>>> +++ b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
>>>>> @@ -4,22 +4,51 @@
>>>>>    $id: http://devicetree.org/schemas/spi/brcm,bcm63xx-hsspi.yaml#
>>>>>    $schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>>    
>>>>> -title: Broadcom BCM6328 High Speed SPI controller
>>>>> +title: Broadcom Broadband SoC High Speed SPI controller
>>>>>    
>>>>>    maintainers:
>>>>> +
>>>>
>>>> Drop blank line.
>>> will fix in  v2.
>>>
>>>>
>>>>> +  - William Zhang <william.zhang@broadcom.com>
>>>>> +  - Kursad Oney <kursad.oney@broadcom.com>
>>>>>      - Jonas Gorski <jonas.gorski@gmail.com>
>>>>
>>>>>    
>>>>> +description: |
>>>>> +  Broadcom Broadband SoC supports High Speed SPI master controller since the
>>>>> +  early MIPS based chips such as BCM6328 and BCM63268.  This controller was
>>>>> +  carried over to recent ARM based chips, such as BCM63138, BCM4908 and BCM6858.
>>>>> +
>>>>> +  It has a limitation that can not keep the chip select line active between
>>>>> +  the SPI transfers within the same SPI message. This can terminate the
>>>>> +  transaction to some SPI devices prematurely. The issue can be worked around by
>>>>> +  either the controller's prepend mode or using the dummy chip select
>>>>> +  workaround. This controller uses the compatible string brcm,bcm6328-hsspi.
>>>>> +
>>>>> +  The newer SoCs such as BCM6756, BCM4912 and BCM6855 include an updated SPI
>>>>> +  controller that add the capability to allow the driver to control chip select
>>>>> +  explicitly. This solves the issue in the old controller. This new controller
>>>>> +  uses the compatible string brcm,bcmbca-hsspi.
>>>>> +
>>>>>    properties:
>>>>>      compatible:
>>>>> -    const: brcm,bcm6328-hsspi
>>>>> +    enum:
>>>>> +      - brcm,bcm6328-hsspi
>>>>> +      - brcm,bcmbca-hsspi
>>>>
>>>> bca seems quite unspecific. Your description above mentions several
>>>> model numbers and "bca" is not listed as model. Compatibles cannot be
>>>> generic.
>>> "bca" is not model number, rather it is a group (broadband carrier
>>> access) of chip that share the same spi host controller IP. Agree it is
>>> not particularly specific but it differentiate from other broadcom spi
>>> controller ip used by other groups.  We just don't have a specific name
>>> for this spi host controller but can we treat bcmbca as the ip name?
>>
>> No, it is discouraged in such forms. Family or IP block compatibles
>> should be prepended with a specific compatible. There were many issues
>> when people insisted on generic or family compatibles...
>>
>>> Otherwise we will have to have a compatible string with chip model for
>>> each SoC even they share the same IP. We already have more than ten of
>>> SoCs and the list will increase.  I don't see this is a good solution too.
>>
>> You will have to do it anyway even with generic fallback, so I don't get
>> what is here to gain... I also don't get why Broadcom should be here
>> special, different than others. Why it is not a good solution for
>> Broadcom SoCs but it is for others?
>>
> I saw a few other vendors like these qcom ones:
>   qcom,spi-qup.yaml
>       - qcom,spi-qup-v1.1.1 # for 8660, 8960 and 8064
>       - qcom,spi-qup-v2.1.1 # for 8974 and later
>       - qcom,spi-qup-v2.2.1 # for 8974 v2 and later
>   qcom,spi-qup.yaml
>       const: qcom,geni-spi

IP block version numbers are allowed when there is clear mapping between
version and SoCs using it. This is the case for Qualcomm because there
is such clear mapping documented and available for Qualcomm engineers
and also some of us (although not public).

> I guess when individual who only has one particular board/chip and is 
> not aware of the IP family,  it is understandable to use the chip 
> specific compatible string.

Family of devices is not a versioned IP block.

> But when company works on it, we have the 
> visibility and access to all the chips and ip blocks in the family and 
> IMHO it is very reasonable to use the IP family name for the same IP as 
> these examples shows. 

No, because family of devices is not a versioned IP block. I wrote
before that families and wildcards are not allowed.

> Are you saying these are not good example to 
> follow?  

It's nothing related to your case.

> What are the issues with generic or family compatibles? 
>  Could 
> you please elaborate?

They stop matching and some point and cause ABI breaks. We had several
cases where engineer believed "I have here family of devices" and then
later it turned out that the family is different.


> 
>>
>>
>>>
>>>>
>>>>>    
>>>>>      reg:
>>>>> -    maxItems: 1
>>>>> +    items:
>>>>> +      - description: main registers
>>>>> +      - description: miscellaneous control registers
>>>>> +    minItems: 1
>>>>> +
>>>>> +  reg-names:
>>>>> +    items:
>>>>> +      - const: hsspi
>>>>> +      - const: spim-ctrl
>>>>
>>>> This does not match reg
>>> Do you mean it does not match the description?
>>
>> No. reg can be 1 item but you state reg-names cannot. These are always
>> the same. If one is 1 item, second is as well.
>>
> I'll drop the "minItems: 1" from the reg property then.

Then it won't be correct, because it would mean two items are required
always.

> 
>>>>
>>>>>    
>>>>>      clocks:
>>>>>        items:
>>>>> -      - description: spi master reference clock
>>>>> -      - description: spi master pll clock
>>>>> +      - description: SPI master reference clock
>>>>> +      - description: SPI master pll clock
>>>>
>>>> Really? You just added it in previous patch, didn't you?
>>> The previous patch was just word to word conversion of the text file.  I
>>> will update that patch to include this change.
>>>
>>>>
>>>>>    
>>>>>      clock-names:
>>>>>        items:
>>>>> @@ -29,12 +58,43 @@ properties:
>>>>>      interrupts:
>>>>>        maxItems: 1
>>>>>    
>>>>> +  brcm,use-cs-workaround:
>>>>> +    $ref: /schemas/types.yaml#/definitions/flag
>>>>> +    description: |
>>>>> +      Enable dummy chip select workaround for SPI transfers that can not be
>>>>> +      supported by the default controller's prepend mode, i.e. delay or cs
>>>>> +      change needed between SPI transfers.
>>>>
>>>> You need to describe what is the workaround.
>>> Will do.
>>>>
>>>>> +
>>>>>    required:
>>>>>      - compatible
>>>>>      - reg
>>>>>      - clocks
>>>>>      - clock-names
>>>>> -  - interrupts
>>>>> +
>>>>> +allOf:
>>>>> +  - $ref: "spi-controller.yaml#"
>>>>
>>>> No quotes. How this is related to this patch?
>>> Will remove quote and put it in patch 1.
>>>>
>>>>> +  - if:
>>>>> +      properties:
>>>>> +        compatible:
>>>>> +          contains:
>>>>> +            enum:
>>>>> +              - brcm,bcm6328-hsspi
>>>>> +    then:
>>>>> +      properties:
>>>>> +        reg:
>>>>> +          minItems: 1
>>>>
>>>> Drop.
>>>>
>>>> reg-names now do not match.
>>> Don't quite understand your comment. What do I need to drop and what is
>>> not matched?
>>
>> You need to add constraints for reg-names, same way as for reg.
>> Disallowing the reg-names also could work, but there won't be benefit in
>> it. Better to have uniform DTS.
>>
> I agree it is better to have the uniform DTS but the situation here is 
> that the brcm,bcm6328-hsspi does not require reg name since there is 
> only one register needed and it was already used in many chip dts for 
> long time.  If I enforce it to have the corresponding reg name, that 

No one told you to enforce to have a reg-names.

> could potentially break the compatibility of those old device if the 
> driver change to use reg name, right?

How compatibility is broken by some optional, unrelated property?

Best regards,
Krzysztof

Re: [PATCH 03/16] dt-bindings: spi: Add spi peripheral specific property

[Mark Brown]

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

On Mon, Jan 09, 2023 at 12:18:09PM -0800, William Zhang wrote:

> This setting is set per spi message for particular chip select of the device
> when starting the message through bcm63xx_hsspi_set_clk function and restore
> to default(clock gating) when message is done through
> bcm63xx_hsspi_restore_clk_gate.

In that case I am extremely confused about what the feature is supposed
to do.  The description says:

+  brcm,no-clk-gate:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description:
+      Some SPI device such as Broadcom ISI based voice daughtercard requires
+SPI
+      clock running even when chip select is deasserted. By default the
+      controller turns off or gate the clock when cs is not active to save
+      power. This flag tells the controller driver to keep the clock running
+      when chip select is not active.


which to me sounds like the clock should never be turned off and instead
left running at all times.  Switching back to clock gating after sending
the message doesn't seem to correspond to the above at all, the message
being done would normally also be the point at which chip select is
deasserted.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 02/16] dt-bindings: spi: Add bcmbca-hsspi controller support

[Florian Fainelli]

On 1/10/23 00:40, Krzysztof Kozlowski wrote:
>>> No, it is discouraged in such forms. Family or IP block compatibles
>>> should be prepended with a specific compatible. There were many issues
>>> when people insisted on generic or family compatibles...
>>>
>>>> Otherwise we will have to have a compatible string with chip model for
>>>> each SoC even they share the same IP. We already have more than ten of
>>>> SoCs and the list will increase.  I don't see this is a good solution too.
>>>
>>> You will have to do it anyway even with generic fallback, so I don't get
>>> what is here to gain... I also don't get why Broadcom should be here
>>> special, different than others. Why it is not a good solution for
>>> Broadcom SoCs but it is for others?
>>>
>> I saw a few other vendors like these qcom ones:
>>    qcom,spi-qup.yaml
>>        - qcom,spi-qup-v1.1.1 # for 8660, 8960 and 8064
>>        - qcom,spi-qup-v2.1.1 # for 8974 and later
>>        - qcom,spi-qup-v2.2.1 # for 8974 v2 and later
>>    qcom,spi-qup.yaml
>>        const: qcom,geni-spi
> 
> IP block version numbers are allowed when there is clear mapping between
> version and SoCs using it. This is the case for Qualcomm because there
> is such clear mapping documented and available for Qualcomm engineers
> and also some of us (although not public).
> 
>> I guess when individual who only has one particular board/chip and is
>> not aware of the IP family,  it is understandable to use the chip
>> specific compatible string.
> 
> Family of devices is not a versioned IP block.

Would it be acceptable to define for instance:

- compatible = "brcm,bcm6868-hsspi", "brcm,bcmbca-hsspi";

in which case, having a fallback compatible on the SoC family that sees 
this IP being deployed is very useful for client programs of the DT 
(u-boot or kernel). As long as the fallback works, we use it, the day it 
stops and a quirk needs to be applied because SoC XYZ has a bug, match 
the SoC XYZ compatible string.

FWIW, and feel free to rant at me, we have adopted this convention a 
while ago for STB chips whereby we want bindings to be defined with:

<chip specific compatible>, <version of the IP>, <fallback>

and the fallback may, or may not be matched, but defining in does not 
hurt at all, in fact it dramatically helps with the boot loader looking 
for specific nodes because it can search for the fallback.

If the version specific compatible is not available, it does not get used.
-- 
Florian

Re: [PATCH 02/16] dt-bindings: spi: Add bcmbca-hsspi controller support

[William Zhang]

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



On 01/10/2023 12:40 AM, Krzysztof Kozlowski wrote:
> On 09/01/2023 20:13, William Zhang wrote:
>>
>>
>> On 01/09/2023 12:56 AM, Krzysztof Kozlowski wrote:
>>> On 09/01/2023 09:27, William Zhang wrote:
>>>> Hi Krzysztof,
>>>>
>>>> On 01/08/2023 06:51 AM, Krzysztof Kozlowski wrote:
>>>>> On 06/01/2023 21:07, William Zhang wrote:
>>>>>> The new Broadcom Broadband BCMBCA SoCs includes a updated HSSPI
>>>>>> controller. Add a new compatible string and required fields for the new
>>>>>> driver.  Also add myself and Kursad as the maintainers.
>>>>>>
>>>>>> Signed-off-by: William Zhang <william.zhang@broadcom.com>
>>>>>> ---
>>>>>>
>>>>>>     .../bindings/spi/brcm,bcm63xx-hsspi.yaml      | 84 +++++++++++++++++--
>>>>>>     1 file changed, 78 insertions(+), 6 deletions(-)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
>>>>>> index 45f1417b1213..56e69d4a1faf 100644
>>>>>> --- a/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
>>>>>> @@ -4,22 +4,51 @@
>>>>>>     $id: http://devicetree.org/schemas/spi/brcm,bcm63xx-hsspi.yaml#
>>>>>>     $schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>>>     
>>>>>> -title: Broadcom BCM6328 High Speed SPI controller
>>>>>> +title: Broadcom Broadband SoC High Speed SPI controller
>>>>>>     
>>>>>>     maintainers:
>>>>>> +
>>>>>
>>>>> Drop blank line.
>>>> will fix in  v2.
>>>>
>>>>>
>>>>>> +  - William Zhang <william.zhang@broadcom.com>
>>>>>> +  - Kursad Oney <kursad.oney@broadcom.com>
>>>>>>       - Jonas Gorski <jonas.gorski@gmail.com>
>>>>>
>>>>>>     
>>>>>> +description: |
>>>>>> +  Broadcom Broadband SoC supports High Speed SPI master controller since the
>>>>>> +  early MIPS based chips such as BCM6328 and BCM63268.  This controller was
>>>>>> +  carried over to recent ARM based chips, such as BCM63138, BCM4908 and BCM6858.
>>>>>> +
>>>>>> +  It has a limitation that can not keep the chip select line active between
>>>>>> +  the SPI transfers within the same SPI message. This can terminate the
>>>>>> +  transaction to some SPI devices prematurely. The issue can be worked around by
>>>>>> +  either the controller's prepend mode or using the dummy chip select
>>>>>> +  workaround. This controller uses the compatible string brcm,bcm6328-hsspi.
>>>>>> +
>>>>>> +  The newer SoCs such as BCM6756, BCM4912 and BCM6855 include an updated SPI
>>>>>> +  controller that add the capability to allow the driver to control chip select
>>>>>> +  explicitly. This solves the issue in the old controller. This new controller
>>>>>> +  uses the compatible string brcm,bcmbca-hsspi.
>>>>>> +
>>>>>>     properties:
>>>>>>       compatible:
>>>>>> -    const: brcm,bcm6328-hsspi
>>>>>> +    enum:
>>>>>> +      - brcm,bcm6328-hsspi
>>>>>> +      - brcm,bcmbca-hsspi
>>>>>
>>>>> bca seems quite unspecific. Your description above mentions several
>>>>> model numbers and "bca" is not listed as model. Compatibles cannot be
>>>>> generic.
>>>> "bca" is not model number, rather it is a group (broadband carrier
>>>> access) of chip that share the same spi host controller IP. Agree it is
>>>> not particularly specific but it differentiate from other broadcom spi
>>>> controller ip used by other groups.  We just don't have a specific name
>>>> for this spi host controller but can we treat bcmbca as the ip name?
>>>
>>> No, it is discouraged in such forms. Family or IP block compatibles
>>> should be prepended with a specific compatible. There were many issues
>>> when people insisted on generic or family compatibles...
>>>
>>>> Otherwise we will have to have a compatible string with chip model for
>>>> each SoC even they share the same IP. We already have more than ten of
>>>> SoCs and the list will increase.  I don't see this is a good solution too.
>>>
>>> You will have to do it anyway even with generic fallback, so I don't get
>>> what is here to gain... I also don't get why Broadcom should be here
>>> special, different than others. Why it is not a good solution for
>>> Broadcom SoCs but it is for others?
>>>
>> I saw a few other vendors like these qcom ones:
>>    qcom,spi-qup.yaml
>>        - qcom,spi-qup-v1.1.1 # for 8660, 8960 and 8064
>>        - qcom,spi-qup-v2.1.1 # for 8974 and later
>>        - qcom,spi-qup-v2.2.1 # for 8974 v2 and later
>>    qcom,spi-qup.yaml
>>        const: qcom,geni-spi
> 
> IP block version numbers are allowed when there is clear mapping between
> version and SoCs using it. This is the case for Qualcomm because there
> is such clear mapping documented and available for Qualcomm engineers
> and also some of us (although not public).
> 
>> I guess when individual who only has one particular board/chip and is
>> not aware of the IP family,  it is understandable to use the chip
>> specific compatible string.
> 
> Family of devices is not a versioned IP block.
> 
>> But when company works on it, we have the
>> visibility and access to all the chips and ip blocks in the family and
>> IMHO it is very reasonable to use the IP family name for the same IP as
>> these examples shows.
> 
> No, because family of devices is not a versioned IP block. I wrote
> before that families and wildcards are not allowed.
> 
>> Are you saying these are not good example to
>> follow?
> 
> It's nothing related to your case.
> 
>> What are the issues with generic or family compatibles?
>>   Could
>> you please elaborate?
> 
> They stop matching and some point and cause ABI breaks. We had several
> cases where engineer believed "I have here family of devices" and then
> later it turned out that the family is different.
> 
> 
>>
>>>
>>>
>>>>
>>>>>
>>>>>>     
>>>>>>       reg:
>>>>>> -    maxItems: 1
>>>>>> +    items:
>>>>>> +      - description: main registers
>>>>>> +      - description: miscellaneous control registers
>>>>>> +    minItems: 1
>>>>>> +
>>>>>> +  reg-names:
>>>>>> +    items:
>>>>>> +      - const: hsspi
>>>>>> +      - const: spim-ctrl
>>>>>
>>>>> This does not match reg
>>>> Do you mean it does not match the description?
>>>
>>> No. reg can be 1 item but you state reg-names cannot. These are always
>>> the same. If one is 1 item, second is as well.
>>>
>> I'll drop the "minItems: 1" from the reg property then.
> 
> Then it won't be correct, because it would mean two items are required
> always.
Ah you are right. Add minItems: 1 for reg-name then.
> 
>>
>>>>>
>>>>>>     
>>>>>>       clocks:
>>>>>>         items:
>>>>>> -      - description: spi master reference clock
>>>>>> -      - description: spi master pll clock
>>>>>> +      - description: SPI master reference clock
>>>>>> +      - description: SPI master pll clock
>>>>>
>>>>> Really? You just added it in previous patch, didn't you?
>>>> The previous patch was just word to word conversion of the text file.  I
>>>> will update that patch to include this change.
>>>>
>>>>>
>>>>>>     
>>>>>>       clock-names:
>>>>>>         items:
>>>>>> @@ -29,12 +58,43 @@ properties:
>>>>>>       interrupts:
>>>>>>         maxItems: 1
>>>>>>     
>>>>>> +  brcm,use-cs-workaround:
>>>>>> +    $ref: /schemas/types.yaml#/definitions/flag
>>>>>> +    description: |
>>>>>> +      Enable dummy chip select workaround for SPI transfers that can not be
>>>>>> +      supported by the default controller's prepend mode, i.e. delay or cs
>>>>>> +      change needed between SPI transfers.
>>>>>
>>>>> You need to describe what is the workaround.
>>>> Will do.
>>>>>
>>>>>> +
>>>>>>     required:
>>>>>>       - compatible
>>>>>>       - reg
>>>>>>       - clocks
>>>>>>       - clock-names
>>>>>> -  - interrupts
>>>>>> +
>>>>>> +allOf:
>>>>>> +  - $ref: "spi-controller.yaml#"
>>>>>
>>>>> No quotes. How this is related to this patch?
>>>> Will remove quote and put it in patch 1.
>>>>>
>>>>>> +  - if:
>>>>>> +      properties:
>>>>>> +        compatible:
>>>>>> +          contains:
>>>>>> +            enum:
>>>>>> +              - brcm,bcm6328-hsspi
>>>>>> +    then:
>>>>>> +      properties:
>>>>>> +        reg:
>>>>>> +          minItems: 1
>>>>>
>>>>> Drop.
>>>>>
>>>>> reg-names now do not match.
>>>> Don't quite understand your comment. What do I need to drop and what is
>>>> not matched?
>>>
>>> You need to add constraints for reg-names, same way as for reg.
>>> Disallowing the reg-names also could work, but there won't be benefit in
>>> it. Better to have uniform DTS.
>>>
>> I agree it is better to have the uniform DTS but the situation here is
>> that the brcm,bcm6328-hsspi does not require reg name since there is
>> only one register needed and it was already used in many chip dts for
>> long time.  If I enforce it to have the corresponding reg name, that
> 
> No one told you to enforce to have a reg-names.
> 
>> could potentially break the compatibility of those old device if the
>> driver change to use reg name, right?
> 
> How compatibility is broken by some optional, unrelated property?
> 
I think I misunderstand what you said.  You basically want the reg-name 
minItem/maxItem constraints for brcm,bcm6328-hsspi compatible as well so 
it is consistent for all the compatibles? I was confused and thought it 
is not needed as reg-name is not required for brcm,bcm6328-hsspi compatible.

> Best regards,
> Krzysztof
> 

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

Re: [PATCH 02/16] dt-bindings: spi: Add bcmbca-hsspi controller support

[William Zhang]

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



On 01/10/2023 02:18 PM, Florian Fainelli wrote:
> On 1/10/23 00:40, Krzysztof Kozlowski wrote:
>>>> No, it is discouraged in such forms. Family or IP block compatibles
>>>> should be prepended with a specific compatible. There were many issues
>>>> when people insisted on generic or family compatibles...
>>>>
>>>>> Otherwise we will have to have a compatible string with chip model for
>>>>> each SoC even they share the same IP. We already have more than ten of
>>>>> SoCs and the list will increase.  I don't see this is a good 
>>>>> solution too.
>>>>
>>>> You will have to do it anyway even with generic fallback, so I don't 
>>>> get
>>>> what is here to gain... I also don't get why Broadcom should be here
>>>> special, different than others. Why it is not a good solution for
>>>> Broadcom SoCs but it is for others?
>>>>
>>> I saw a few other vendors like these qcom ones:
>>>    qcom,spi-qup.yaml
>>>        - qcom,spi-qup-v1.1.1 # for 8660, 8960 and 8064
>>>        - qcom,spi-qup-v2.1.1 # for 8974 and later
>>>        - qcom,spi-qup-v2.2.1 # for 8974 v2 and later
>>>    qcom,spi-qup.yaml
>>>        const: qcom,geni-spi
>>
>> IP block version numbers are allowed when there is clear mapping between
>> version and SoCs using it. This is the case for Qualcomm because there
>> is such clear mapping documented and available for Qualcomm engineers
>> and also some of us (although not public).
>>
>>> I guess when individual who only has one particular board/chip and is
>>> not aware of the IP family,  it is understandable to use the chip
>>> specific compatible string.
>>
>> Family of devices is not a versioned IP block.
> 
> Would it be acceptable to define for instance:
> 
> - compatible = "brcm,bcm6868-hsspi", "brcm,bcmbca-hsspi";
> 
> in which case, having a fallback compatible on the SoC family that sees 
> this IP being deployed is very useful for client programs of the DT 
> (u-boot or kernel). As long as the fallback works, we use it, the day it 
> stops and a quirk needs to be applied because SoC XYZ has a bug, match 
> the SoC XYZ compatible string.
> 
> FWIW, and feel free to rant at me, we have adopted this convention a 
> while ago for STB chips whereby we want bindings to be defined with:
> 
> <chip specific compatible>, <version of the IP>, <fallback>
> 
> and the fallback may, or may not be matched, but defining in does not 
> hurt at all, in fact it dramatically helps with the boot loader looking 
> for specific nodes because it can search for the fallback.
> 
> If the version specific compatible is not available, it does not get used.

Thanks Florian for jumping in! I was thinking to propose something with 
version info:
    brcm,bcmbca-hsspi-v1.0
    brcm,bcmbca-hsspi-v1.1

To meet STB chip convention, then it would be:
compatible = "brcm,bcm63138-hsspi", "brcm,bcmbca-hsspi-v1.0", 
"brcm,bcmbca-hsspi";
compatible = "brcm,bcm6756-hsspi", "brcm,bcmbca-hsspi-v1.1", 
"brcm,bcmbca-hsspi";

Although I am not a fan of having a chip specific compatible while we 
already have IP version,  I am okay to have it to be consistent with 
Broadcom convention. We will need to remember to update this yaml file 
whenever we have a new chip.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

Re: [PATCH 02/16] dt-bindings: spi: Add bcmbca-hsspi controller support

[Krzysztof Kozlowski]

On 11/01/2023 01:59, William Zhang wrote:
>>>> You need to add constraints for reg-names, same way as for reg.
>>>> Disallowing the reg-names also could work, but there won't be benefit in
>>>> it. Better to have uniform DTS.
>>>>
>>> I agree it is better to have the uniform DTS but the situation here is
>>> that the brcm,bcm6328-hsspi does not require reg name since there is
>>> only one register needed and it was already used in many chip dts for
>>> long time.  If I enforce it to have the corresponding reg name, that
>>
>> No one told you to enforce to have a reg-names.
>>
>>> could potentially break the compatibility of those old device if the
>>> driver change to use reg name, right?
>>
>> How compatibility is broken by some optional, unrelated property?
>>
> I think I misunderstand what you said.  You basically want the reg-name 
> minItem/maxItem constraints for brcm,bcm6328-hsspi compatible as well so 
> it is consistent for all the compatibles?

Yes.

Best regards,
Krzysztof

Re: [PATCH 02/16] dt-bindings: spi: Add bcmbca-hsspi controller support

[Krzysztof Kozlowski]

On 10/01/2023 23:18, Florian Fainelli wrote:
> On 1/10/23 00:40, Krzysztof Kozlowski wrote:
>>>> No, it is discouraged in such forms. Family or IP block compatibles
>>>> should be prepended with a specific compatible. There were many issues
>>>> when people insisted on generic or family compatibles...
>>>>
>>>>> Otherwise we will have to have a compatible string with chip model for
>>>>> each SoC even they share the same IP. We already have more than ten of
>>>>> SoCs and the list will increase.  I don't see this is a good solution too.
>>>>
>>>> You will have to do it anyway even with generic fallback, so I don't get
>>>> what is here to gain... I also don't get why Broadcom should be here
>>>> special, different than others. Why it is not a good solution for
>>>> Broadcom SoCs but it is for others?
>>>>
>>> I saw a few other vendors like these qcom ones:
>>>    qcom,spi-qup.yaml
>>>        - qcom,spi-qup-v1.1.1 # for 8660, 8960 and 8064
>>>        - qcom,spi-qup-v2.1.1 # for 8974 and later
>>>        - qcom,spi-qup-v2.2.1 # for 8974 v2 and later
>>>    qcom,spi-qup.yaml
>>>        const: qcom,geni-spi
>>
>> IP block version numbers are allowed when there is clear mapping between
>> version and SoCs using it. This is the case for Qualcomm because there
>> is such clear mapping documented and available for Qualcomm engineers
>> and also some of us (although not public).
>>
>>> I guess when individual who only has one particular board/chip and is
>>> not aware of the IP family,  it is understandable to use the chip
>>> specific compatible string.
>>
>> Family of devices is not a versioned IP block.
> 
> Would it be acceptable to define for instance:
> 
> - compatible = "brcm,bcm6868-hsspi", "brcm,bcmbca-hsspi";

Yes, this is perfectly valid. Although it does not solve William
concerns because it requires defining specific compatibles for all of
the SoCs.

Best regards,
Krzysztof

Re: [PATCH 02/16] dt-bindings: spi: Add bcmbca-hsspi controller support

[William Zhang]

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



On 01/11/2023 01:02 AM, Krzysztof Kozlowski wrote:
> On 10/01/2023 23:18, Florian Fainelli wrote:
>> On 1/10/23 00:40, Krzysztof Kozlowski wrote:
>>>>> No, it is discouraged in such forms. Family or IP block compatibles
>>>>> should be prepended with a specific compatible. There were many issues
>>>>> when people insisted on generic or family compatibles...
>>>>>
>>>>>> Otherwise we will have to have a compatible string with chip model for
>>>>>> each SoC even they share the same IP. We already have more than ten of
>>>>>> SoCs and the list will increase.  I don't see this is a good solution too.
>>>>>
>>>>> You will have to do it anyway even with generic fallback, so I don't get
>>>>> what is here to gain... I also don't get why Broadcom should be here
>>>>> special, different than others. Why it is not a good solution for
>>>>> Broadcom SoCs but it is for others?
>>>>>
>>>> I saw a few other vendors like these qcom ones:
>>>>     qcom,spi-qup.yaml
>>>>         - qcom,spi-qup-v1.1.1 # for 8660, 8960 and 8064
>>>>         - qcom,spi-qup-v2.1.1 # for 8974 and later
>>>>         - qcom,spi-qup-v2.2.1 # for 8974 v2 and later
>>>>     qcom,spi-qup.yaml
>>>>         const: qcom,geni-spi
>>>
>>> IP block version numbers are allowed when there is clear mapping between
>>> version and SoCs using it. This is the case for Qualcomm because there
>>> is such clear mapping documented and available for Qualcomm engineers
>>> and also some of us (although not public).
>>>
>>>> I guess when individual who only has one particular board/chip and is
>>>> not aware of the IP family,  it is understandable to use the chip
>>>> specific compatible string.
>>>
>>> Family of devices is not a versioned IP block.
>>
>> Would it be acceptable to define for instance:
>>
>> - compatible = "brcm,bcm6868-hsspi", "brcm,bcmbca-hsspi";
> 
> Yes, this is perfectly valid. Although it does not solve William
> concerns because it requires defining specific compatibles for all of
> the SoCs.
> 
> Best regards,
> Krzysztof
> 
As I mentioned in another email,  I would be okay to use these 
compatibles to differentiate by ip rev and to conforms to brcm convention:
"brcm,bcmXYZ-hsspi", "brcm,bcmbca-hsspi-v1.0", "brcm,bcmbca-hsspi";
"brcm,bcmXYZ-hsspi", "brcm,bcmbca-hsspi-v1.1", "brcm,bcmbca-hsspi";

In the two drivers I included in this series, it will be bound to 
brcm,bcmbca-hsspi-v1.0 (in additional to brcm,bcm6328-hsspi) and 
brcm,bcmbca-hsspi-v1.1 respectively.  This way we don't need to update 
the driver with a new soc specific compatible whenever a new chips comes 
out.

Does this sound good to you?

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

Re: [PATCH 02/16] dt-bindings: spi: Add bcmbca-hsspi controller support

[Krzysztof Kozlowski]

On 11/01/2023 19:04, William Zhang wrote:
> 
> 
> On 01/11/2023 01:02 AM, Krzysztof Kozlowski wrote:
>> On 10/01/2023 23:18, Florian Fainelli wrote:
>>> On 1/10/23 00:40, Krzysztof Kozlowski wrote:
>>>>>> No, it is discouraged in such forms. Family or IP block compatibles
>>>>>> should be prepended with a specific compatible. There were many issues
>>>>>> when people insisted on generic or family compatibles...
>>>>>>
>>>>>>> Otherwise we will have to have a compatible string with chip model for
>>>>>>> each SoC even they share the same IP. We already have more than ten of
>>>>>>> SoCs and the list will increase.  I don't see this is a good solution too.
>>>>>>
>>>>>> You will have to do it anyway even with generic fallback, so I don't get
>>>>>> what is here to gain... I also don't get why Broadcom should be here
>>>>>> special, different than others. Why it is not a good solution for
>>>>>> Broadcom SoCs but it is for others?
>>>>>>
>>>>> I saw a few other vendors like these qcom ones:
>>>>>     qcom,spi-qup.yaml
>>>>>         - qcom,spi-qup-v1.1.1 # for 8660, 8960 and 8064
>>>>>         - qcom,spi-qup-v2.1.1 # for 8974 and later
>>>>>         - qcom,spi-qup-v2.2.1 # for 8974 v2 and later
>>>>>     qcom,spi-qup.yaml
>>>>>         const: qcom,geni-spi
>>>>
>>>> IP block version numbers are allowed when there is clear mapping between
>>>> version and SoCs using it. This is the case for Qualcomm because there
>>>> is such clear mapping documented and available for Qualcomm engineers
>>>> and also some of us (although not public).
>>>>
>>>>> I guess when individual who only has one particular board/chip and is
>>>>> not aware of the IP family,  it is understandable to use the chip
>>>>> specific compatible string.
>>>>
>>>> Family of devices is not a versioned IP block.
>>>
>>> Would it be acceptable to define for instance:
>>>
>>> - compatible = "brcm,bcm6868-hsspi", "brcm,bcmbca-hsspi";
>>
>> Yes, this is perfectly valid. Although it does not solve William
>> concerns because it requires defining specific compatibles for all of
>> the SoCs.
>>
>> Best regards,
>> Krzysztof
>>
> As I mentioned in another email,  I would be okay to use these 
> compatibles to differentiate by ip rev and to conforms to brcm convention:
> "brcm,bcmXYZ-hsspi", "brcm,bcmbca-hsspi-v1.0", "brcm,bcmbca-hsspi";
> "brcm,bcmXYZ-hsspi", "brcm,bcmbca-hsspi-v1.1", "brcm,bcmbca-hsspi";


Drop the version in such case, no benefits. I assume XYZ is the SoC
model, so for example 6868.

> 
> In the two drivers I included in this series, it will be bound to 
> brcm,bcmbca-hsspi-v1.0 (in additional to brcm,bcm6328-hsspi) and 
> brcm,bcmbca-hsspi-v1.1 respectively.  This way we don't need to update 
> the driver with a new soc specific compatible whenever a new chips comes 
> out.

I don't understand why do you bring it now as an argument. You defined
before that your driver will bind to the generic bcmbca compatible, so
now it is not enough?

Best regards,
Krzysztof

Re: [PATCH 02/16] dt-bindings: spi: Add bcmbca-hsspi controller support

[William Zhang]

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



On 01/11/2023 10:12 AM, Krzysztof Kozlowski wrote:
> On 11/01/2023 19:04, William Zhang wrote:
>>
>>
>> On 01/11/2023 01:02 AM, Krzysztof Kozlowski wrote:
>>> On 10/01/2023 23:18, Florian Fainelli wrote:
>>>> On 1/10/23 00:40, Krzysztof Kozlowski wrote:
>>>>>>> No, it is discouraged in such forms. Family or IP block compatibles
>>>>>>> should be prepended with a specific compatible. There were many issues
>>>>>>> when people insisted on generic or family compatibles...
>>>>>>>
>>>>>>>> Otherwise we will have to have a compatible string with chip model for
>>>>>>>> each SoC even they share the same IP. We already have more than ten of
>>>>>>>> SoCs and the list will increase.  I don't see this is a good solution too.
>>>>>>>
>>>>>>> You will have to do it anyway even with generic fallback, so I don't get
>>>>>>> what is here to gain... I also don't get why Broadcom should be here
>>>>>>> special, different than others. Why it is not a good solution for
>>>>>>> Broadcom SoCs but it is for others?
>>>>>>>
>>>>>> I saw a few other vendors like these qcom ones:
>>>>>>      qcom,spi-qup.yaml
>>>>>>          - qcom,spi-qup-v1.1.1 # for 8660, 8960 and 8064
>>>>>>          - qcom,spi-qup-v2.1.1 # for 8974 and later
>>>>>>          - qcom,spi-qup-v2.2.1 # for 8974 v2 and later
>>>>>>      qcom,spi-qup.yaml
>>>>>>          const: qcom,geni-spi
>>>>>
>>>>> IP block version numbers are allowed when there is clear mapping between
>>>>> version and SoCs using it. This is the case for Qualcomm because there
>>>>> is such clear mapping documented and available for Qualcomm engineers
>>>>> and also some of us (although not public).
>>>>>
>>>>>> I guess when individual who only has one particular board/chip and is
>>>>>> not aware of the IP family,  it is understandable to use the chip
>>>>>> specific compatible string.
>>>>>
>>>>> Family of devices is not a versioned IP block.
>>>>
>>>> Would it be acceptable to define for instance:
>>>>
>>>> - compatible = "brcm,bcm6868-hsspi", "brcm,bcmbca-hsspi";
>>>
>>> Yes, this is perfectly valid. Although it does not solve William
>>> concerns because it requires defining specific compatibles for all of
>>> the SoCs.
>>>
>>> Best regards,
>>> Krzysztof
>>>
>> As I mentioned in another email,  I would be okay to use these
>> compatibles to differentiate by ip rev and to conforms to brcm convention:
>> "brcm,bcmXYZ-hsspi", "brcm,bcmbca-hsspi-v1.0", "brcm,bcmbca-hsspi";
>> "brcm,bcmXYZ-hsspi", "brcm,bcmbca-hsspi-v1.1", "brcm,bcmbca-hsspi";
> 
> 
> Drop the version in such case, no benefits. I assume XYZ is the SoC
> model, so for example 6868.
> 
Yes XYZ is the SoC model
>>
>> In the two drivers I included in this series, it will be bound to
>> brcm,bcmbca-hsspi-v1.0 (in additional to brcm,bcm6328-hsspi) and
>> brcm,bcmbca-hsspi-v1.1 respectively.  This way we don't need to update
>> the driver with a new soc specific compatible whenever a new chips comes
>> out.
> 
> I don't understand why do you bring it now as an argument. You defined
> before that your driver will bind to the generic bcmbca compatible, so
> now it is not enough?
> 
No as we are adding chip model specific info here.  The existing driver 
spi-bcm63xx-hsspi.c only binds to brcm,bcm6328-hsspi. This driver 
supports all the chips with rev1.0 controller so I am using this 6328 
string for other chips with v1.0 in the dts patch, which is not ideal.
Now I have to add more compatible to this driver and for each new chip 
with 1.0 in the future if any.

With all the thoughts from you and Florian,  I think it is better to use 
rev compatible in the driver but add on chip model compatible in the dts.



> Best regards,
> Krzysztof
> 

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

Re: [PATCH 03/16] dt-bindings: spi: Add spi peripheral specific property

[William Zhang]

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



On 01/10/2023 02:01 PM, Mark Brown wrote:
> On Mon, Jan 09, 2023 at 12:18:09PM -0800, William Zhang wrote:
> 
>> This setting is set per spi message for particular chip select of the device
>> when starting the message through bcm63xx_hsspi_set_clk function and restore
>> to default(clock gating) when message is done through
>> bcm63xx_hsspi_restore_clk_gate.
> 
> In that case I am extremely confused about what the feature is supposed
> to do.  The description says:
> 
> +  brcm,no-clk-gate:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description:
> +      Some SPI device such as Broadcom ISI based voice daughtercard requires
> +SPI
> +      clock running even when chip select is deasserted. By default the
> +      controller turns off or gate the clock when cs is not active to save
> +      power. This flag tells the controller driver to keep the clock running
> +      when chip select is not active.
> 
> 
> which to me sounds like the clock should never be turned off and instead
> left running at all times.  Switching back to clock gating after sending
> the message doesn't seem to correspond to the above at all, the message
> being done would normally also be the point at which chip select is
> deasserted.
> 
This feature is used by our voice team and as far I can tell, it is used 
to keep clock running between the transfers within the same message. 
But now that we have prepend mode to combine to one transfer or dummy 
workaround to keep cs always active between transfers, this indeed does 
not seems right.   I will have to talk to the voice team why this is 
still needed and get back here.

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

Re: [PATCH 02/16] dt-bindings: spi: Add bcmbca-hsspi controller support

[Krzysztof Kozlowski]

On 11/01/2023 19:44, William Zhang wrote:
> 
> 
> On 01/11/2023 10:12 AM, Krzysztof Kozlowski wrote:
>> On 11/01/2023 19:04, William Zhang wrote:
>>>
>>>
>>> On 01/11/2023 01:02 AM, Krzysztof Kozlowski wrote:
>>>> On 10/01/2023 23:18, Florian Fainelli wrote:
>>>>> On 1/10/23 00:40, Krzysztof Kozlowski wrote:
>>>>>>>> No, it is discouraged in such forms. Family or IP block compatibles
>>>>>>>> should be prepended with a specific compatible. There were many issues
>>>>>>>> when people insisted on generic or family compatibles...
>>>>>>>>
>>>>>>>>> Otherwise we will have to have a compatible string with chip model for
>>>>>>>>> each SoC even they share the same IP. We already have more than ten of
>>>>>>>>> SoCs and the list will increase.  I don't see this is a good solution too.
>>>>>>>>
>>>>>>>> You will have to do it anyway even with generic fallback, so I don't get
>>>>>>>> what is here to gain... I also don't get why Broadcom should be here
>>>>>>>> special, different than others. Why it is not a good solution for
>>>>>>>> Broadcom SoCs but it is for others?
>>>>>>>>
>>>>>>> I saw a few other vendors like these qcom ones:
>>>>>>>      qcom,spi-qup.yaml
>>>>>>>          - qcom,spi-qup-v1.1.1 # for 8660, 8960 and 8064
>>>>>>>          - qcom,spi-qup-v2.1.1 # for 8974 and later
>>>>>>>          - qcom,spi-qup-v2.2.1 # for 8974 v2 and later
>>>>>>>      qcom,spi-qup.yaml
>>>>>>>          const: qcom,geni-spi
>>>>>>
>>>>>> IP block version numbers are allowed when there is clear mapping between
>>>>>> version and SoCs using it. This is the case for Qualcomm because there
>>>>>> is such clear mapping documented and available for Qualcomm engineers
>>>>>> and also some of us (although not public).
>>>>>>
>>>>>>> I guess when individual who only has one particular board/chip and is
>>>>>>> not aware of the IP family,  it is understandable to use the chip
>>>>>>> specific compatible string.
>>>>>>
>>>>>> Family of devices is not a versioned IP block.
>>>>>
>>>>> Would it be acceptable to define for instance:
>>>>>
>>>>> - compatible = "brcm,bcm6868-hsspi", "brcm,bcmbca-hsspi";
>>>>
>>>> Yes, this is perfectly valid. Although it does not solve William
>>>> concerns because it requires defining specific compatibles for all of
>>>> the SoCs.
>>>>
>>>> Best regards,
>>>> Krzysztof
>>>>
>>> As I mentioned in another email,  I would be okay to use these
>>> compatibles to differentiate by ip rev and to conforms to brcm convention:
>>> "brcm,bcmXYZ-hsspi", "brcm,bcmbca-hsspi-v1.0", "brcm,bcmbca-hsspi";
>>> "brcm,bcmXYZ-hsspi", "brcm,bcmbca-hsspi-v1.1", "brcm,bcmbca-hsspi";
>>
>>
>> Drop the version in such case, no benefits. I assume XYZ is the SoC
>> model, so for example 6868.
>>
> Yes XYZ is the SoC model
>>>
>>> In the two drivers I included in this series, it will be bound to
>>> brcm,bcmbca-hsspi-v1.0 (in additional to brcm,bcm6328-hsspi) and
>>> brcm,bcmbca-hsspi-v1.1 respectively.  This way we don't need to update
>>> the driver with a new soc specific compatible whenever a new chips comes
>>> out.
>>
>> I don't understand why do you bring it now as an argument. You defined
>> before that your driver will bind to the generic bcmbca compatible, so
>> now it is not enough?
>>
> No as we are adding chip model specific info here.  The existing driver 
> spi-bcm63xx-hsspi.c only binds to brcm,bcm6328-hsspi. This driver 
> supports all the chips with rev1.0 controller so I am using this 6328 
> string for other chips with v1.0 in the dts patch, which is not ideal.

Why? This is perfectly ideal and usual case. Why changing it?

> Now I have to add more compatible to this driver and for each new chip 
> with 1.0 in the future if any.

Why you cannot use compatibility with older chipset?


Best regards,
Krzysztof

Re: [PATCH 02/16] dt-bindings: spi: Add bcmbca-hsspi controller support

[William Zhang]

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



On 01/12/2023 12:21 AM, Krzysztof Kozlowski wrote:
> On 11/01/2023 19:44, William Zhang wrote:
>>
>>
>> On 01/11/2023 10:12 AM, Krzysztof Kozlowski wrote:
>>> On 11/01/2023 19:04, William Zhang wrote:
>>>>
>>>>
>>>> On 01/11/2023 01:02 AM, Krzysztof Kozlowski wrote:
>>>>> On 10/01/2023 23:18, Florian Fainelli wrote:
>>>>>> On 1/10/23 00:40, Krzysztof Kozlowski wrote:
>>>>>>>>> No, it is discouraged in such forms. Family or IP block compatibles
>>>>>>>>> should be prepended with a specific compatible. There were many issues
>>>>>>>>> when people insisted on generic or family compatibles...
>>>>>>>>>
>>>>>>>>>> Otherwise we will have to have a compatible string with chip model for
>>>>>>>>>> each SoC even they share the same IP. We already have more than ten of
>>>>>>>>>> SoCs and the list will increase.  I don't see this is a good solution too.
>>>>>>>>>
>>>>>>>>> You will have to do it anyway even with generic fallback, so I don't get
>>>>>>>>> what is here to gain... I also don't get why Broadcom should be here
>>>>>>>>> special, different than others. Why it is not a good solution for
>>>>>>>>> Broadcom SoCs but it is for others?
>>>>>>>>>
>>>>>>>> I saw a few other vendors like these qcom ones:
>>>>>>>>       qcom,spi-qup.yaml
>>>>>>>>           - qcom,spi-qup-v1.1.1 # for 8660, 8960 and 8064
>>>>>>>>           - qcom,spi-qup-v2.1.1 # for 8974 and later
>>>>>>>>           - qcom,spi-qup-v2.2.1 # for 8974 v2 and later
>>>>>>>>       qcom,spi-qup.yaml
>>>>>>>>           const: qcom,geni-spi
>>>>>>>
>>>>>>> IP block version numbers are allowed when there is clear mapping between
>>>>>>> version and SoCs using it. This is the case for Qualcomm because there
>>>>>>> is such clear mapping documented and available for Qualcomm engineers
>>>>>>> and also some of us (although not public).
>>>>>>>
>>>>>>>> I guess when individual who only has one particular board/chip and is
>>>>>>>> not aware of the IP family,  it is understandable to use the chip
>>>>>>>> specific compatible string.
>>>>>>>
>>>>>>> Family of devices is not a versioned IP block.
>>>>>>
>>>>>> Would it be acceptable to define for instance:
>>>>>>
>>>>>> - compatible = "brcm,bcm6868-hsspi", "brcm,bcmbca-hsspi";
>>>>>
>>>>> Yes, this is perfectly valid. Although it does not solve William
>>>>> concerns because it requires defining specific compatibles for all of
>>>>> the SoCs.
>>>>>
>>>>> Best regards,
>>>>> Krzysztof
>>>>>
>>>> As I mentioned in another email,  I would be okay to use these
>>>> compatibles to differentiate by ip rev and to conforms to brcm convention:
>>>> "brcm,bcmXYZ-hsspi", "brcm,bcmbca-hsspi-v1.0", "brcm,bcmbca-hsspi";
>>>> "brcm,bcmXYZ-hsspi", "brcm,bcmbca-hsspi-v1.1", "brcm,bcmbca-hsspi";
>>>
>>>
>>> Drop the version in such case, no benefits. I assume XYZ is the SoC
>>> model, so for example 6868.
>>>
>> Yes XYZ is the SoC model
>>>>
>>>> In the two drivers I included in this series, it will be bound to
>>>> brcm,bcmbca-hsspi-v1.0 (in additional to brcm,bcm6328-hsspi) and
>>>> brcm,bcmbca-hsspi-v1.1 respectively.  This way we don't need to update
>>>> the driver with a new soc specific compatible whenever a new chips comes
>>>> out.
>>>
>>> I don't understand why do you bring it now as an argument. You defined
>>> before that your driver will bind to the generic bcmbca compatible, so
>>> now it is not enough?
>>>
>> No as we are adding chip model specific info here.  The existing driver
>> spi-bcm63xx-hsspi.c only binds to brcm,bcm6328-hsspi. This driver
>> supports all the chips with rev1.0 controller so I am using this 6328
>> string for other chips with v1.0 in the dts patch, which is not ideal.
> 
> Why? This is perfectly ideal and usual case. Why changing it?
> 
>> Now I have to add more compatible to this driver and for each new chip
>> with 1.0 in the future if any.
> 
> Why you cannot use compatibility with older chipset?
> 
IMHO it is really confusing that we have all the SoCs but have to bind 
to an antique SoC's spi controller compatible and people may think it is 
a mistake or typo when they don't know they are actually the same. I 
know there are usage like that but when we have clear knowledge of the 
IP block with rev info, I think it is much better to have a precise SoC 
model number and a general revision info in the compatible. As you know 
they are many usage of IP rev info in the compatible too. 
brcm,bcm6328-hsspi will stay so it does not break any existing dts 
reference to that.

Anyway if you still does not like this idea, I will drop the rev info 
and you have it your way.

> 
> Best regards,
> Krzysztof
> 

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

Re: [PATCH 02/16] dt-bindings: spi: Add bcmbca-hsspi controller support

[Krzysztof Kozlowski]

On 12/01/2023 20:50, William Zhang wrote:
>>> No as we are adding chip model specific info here.  The existing driver
>>> spi-bcm63xx-hsspi.c only binds to brcm,bcm6328-hsspi. This driver
>>> supports all the chips with rev1.0 controller so I am using this 6328
>>> string for other chips with v1.0 in the dts patch, which is not ideal.
>>
>> Why? This is perfectly ideal and usual case. Why changing it?
>>
>>> Now I have to add more compatible to this driver and for each new chip
>>> with 1.0 in the future if any.
>>
>> Why you cannot use compatibility with older chipset?
>>
> IMHO it is really confusing that we have all the SoCs but have to bind 
> to an antique SoC's spi controller compatible and people may think it is 
> a mistake or typo when they don't know they are actually the same.

I am sorry, this is ridiculous argument. It's like saying - people
cannot understand what they are reading, therefore we need to present
them obfuscated information so they will think something else than their
minds created...

> I 
> know there are usage like that but when we have clear knowledge of the 
> IP block with rev info, I think it is much better to have a precise SoC 

No, it's not particularly better and you were questioning it just before...

> model number and a general revision info in the compatible. As you know 
> they are many usage of IP rev info in the compatible too. 
> brcm,bcm6328-hsspi will stay so it does not break any existing dts 
> reference to that.

Anyway your ship sailed - you already have bindings using SoC  versions...

> 
> Anyway if you still does not like this idea, I will drop the rev info 
> and you have it your way.

Best regards,
Krzysztof

Re: [PATCH 02/16] dt-bindings: spi: Add bcmbca-hsspi controller support

[William Zhang]

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



On 01/12/2023 11:41 PM, Krzysztof Kozlowski wrote:
> On 12/01/2023 20:50, William Zhang wrote:
>>>> No as we are adding chip model specific info here.  The existing driver
>>>> spi-bcm63xx-hsspi.c only binds to brcm,bcm6328-hsspi. This driver
>>>> supports all the chips with rev1.0 controller so I am using this 6328
>>>> string for other chips with v1.0 in the dts patch, which is not ideal.
>>>
>>> Why? This is perfectly ideal and usual case. Why changing it?
>>>
>>>> Now I have to add more compatible to this driver and for each new chip
>>>> with 1.0 in the future if any.
>>>
>>> Why you cannot use compatibility with older chipset?
>>>
>> IMHO it is really confusing that we have all the SoCs but have to bind
>> to an antique SoC's spi controller compatible and people may think it is
>> a mistake or typo when they don't know they are actually the same.
> 
> I am sorry, this is ridiculous argument. It's like saying - people
> cannot understand what they are reading, therefore we need to present
> them obfuscated information so they will think something else than their
> minds created...
> 
This is clearly not to obfuscate. Rather it provide more accurate info 
about the binding.  Is it a problem to have the correct and precise info 
to make it easier for people to understand?

>> I
>> know there are usage like that but when we have clear knowledge of the
>> IP block with rev info, I think it is much better to have a precise SoC
> 
> No, it's not particularly better and you were questioning it just before...
> 
Better than using the very old specific chip model number to bind all 
other new chips while I have a chance to update the doc now. I guess we 
have to agree to disagree. Enough discussion and I will send out v2 next 
week.  Thanks for the review.

>> model number and a general revision info in the compatible. As you know
>> they are many usage of IP rev info in the compatible too.
>> brcm,bcm6328-hsspi will stay so it does not break any existing dts
>> reference to that.
> 
> Anyway your ship sailed - you already have bindings using SoC  versions...
> 
>>
>> Anyway if you still does not like this idea, I will drop the rev info
>> and you have it your way.
> 
> Best regards,
> Krzysztof
> 

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

Re: [PATCH 02/16] dt-bindings: spi: Add bcmbca-hsspi controller support

[Krzysztof Kozlowski]

On 14/01/2023 04:17, William Zhang wrote:
>>> I
>>> know there are usage like that but when we have clear knowledge of the
>>> IP block with rev info, I think it is much better to have a precise SoC
>>
>> No, it's not particularly better and you were questioning it just before...
>>
> Better than using the very old specific chip model number to bind all 
> other new chips while I have a chance to update the doc now. 

It will be used to bind them anyway, it's already an ABI.


> I guess we 
> have to agree to disagree. Enough discussion and I will send out v2 next 
> week.  Thanks for the review.
> 
>>> model number and a general revision info in the compatible. As you know
>>> they are many usage of IP rev info in the compatible too.
>>> brcm,bcm6328-hsspi will stay so it does not break any existing dts
>>> reference to that.
>>
>> Anyway your ship sailed - you already have bindings using SoC  versions...

As I said here...

Best regards,
Krzysztof