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.
Changes in v2:
- Update the dts yaml document and all the related dtsi/dts accordingly
- Fix build error for Alpha platform
- Add a new patch for bcm63xx-hsspi driver to support the new compatible
string
- Make interrupt mode required but keep polling mode as default. Also
add a sysfs option wait_mode for run-time mode change
- Remove use_cs_workaround option and change the transfer logic to try
prepend mode first and if not prependable, switch to dummy cs mode with
clock limit at the 25MHz. Add driver sysfs node xfer_mode for run-time
configuration to dummy cs or prepend mode.
- Withdraw SPI device specific clock gate option patch for now
William Zhang (14):
dt-bindings: spi: Convert bcm63xx-hsspi bindings to json-schema
dt-bindings: spi: Add bcmbca-hsspi controller support
ARM: dts: broadcom: bcmbca: Add spi controller node
arm64: dts: broadcom: bcmbca: Add spi controller node
spi: bcm63xx-hsspi: Add new compatible string support
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: Add prepend mode 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
.../bindings/spi/brcm,bcm63xx-hsspi.yaml | 151 ++++
.../bindings/spi/spi-bcm63xx-hsspi.txt | 33 -
MAINTAINERS | 12 +
arch/arm/boot/dts/bcm47622.dtsi | 19 +
arch/arm/boot/dts/bcm63138.dtsi | 19 +
arch/arm/boot/dts/bcm63148.dtsi | 19 +
arch/arm/boot/dts/bcm63178.dtsi | 20 +
arch/arm/boot/dts/bcm6756.dtsi | 20 +
arch/arm/boot/dts/bcm6846.dtsi | 19 +
arch/arm/boot/dts/bcm6855.dtsi | 20 +
arch/arm/boot/dts/bcm6878.dtsi | 20 +
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 | 19 +
.../boot/dts/broadcom/bcmbca/bcm4912.dtsi | 21 +
.../boot/dts/broadcom/bcmbca/bcm63146.dtsi | 20 +
.../boot/dts/broadcom/bcmbca/bcm63158.dtsi | 20 +
.../boot/dts/broadcom/bcmbca/bcm6813.dtsi | 21 +
.../boot/dts/broadcom/bcmbca/bcm6856.dtsi | 19 +
.../boot/dts/broadcom/bcmbca/bcm6858.dtsi | 19 +
.../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 | 453 +++++++++++-
drivers/spi/spi-bcmbca-hsspi.c | 645 ++++++++++++++++++
drivers/spi/spi-mem.c | 2 +-
drivers/spi/spi.c | 13 +-
40 files changed, 1607 insertions(+), 71 deletions(-)
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
Add support for HSSPI controller in ARMv7 chip dts files.
Signed-off-by: William Zhang <[email protected]>
---
Changes in v2:
- Update compatible string with SoC model number, controller version
info and bcmbca fall back name
- Add interrupt property
arch/arm/boot/dts/bcm47622.dtsi | 19 +++++++++++++++++++
arch/arm/boot/dts/bcm63138.dtsi | 19 +++++++++++++++++++
arch/arm/boot/dts/bcm63148.dtsi | 19 +++++++++++++++++++
arch/arm/boot/dts/bcm63178.dtsi | 20 ++++++++++++++++++++
arch/arm/boot/dts/bcm6756.dtsi | 20 ++++++++++++++++++++
arch/arm/boot/dts/bcm6846.dtsi | 19 +++++++++++++++++++
arch/arm/boot/dts/bcm6855.dtsi | 20 ++++++++++++++++++++
arch/arm/boot/dts/bcm6878.dtsi | 20 ++++++++++++++++++++
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, 192 insertions(+)
diff --git a/arch/arm/boot/dts/bcm47622.dtsi b/arch/arm/boot/dts/bcm47622.dtsi
index f4b2db9bc4ab..6de4a2287c90 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,19 @@ bus@ff800000 {
#size-cells = <1>;
ranges = <0 0xff800000 0x800000>;
+ hsspi: spi@1000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "brcm,bcm47622-hsspi", "brcm,bcmbca-hsspi-v1.0",
+ "brcm,bcmbca-hsspi";
+ reg = <0x1000 0x600>;
+ interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
+ 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..2c436b95f874 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,19 @@ serial1: serial@620 {
status = "disabled";
};
+ hsspi: spi@1000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "brcm,bcm63138-hsspi", "brcm,bcmbca-hsspi-v1.0",
+ "brcm,bcmbca-hsspi";
+ reg = <0x1000 0x600>;
+ interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
+ 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..646906c8c260 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,18 @@ uart0: serial@600 {
clock-names = "refclk";
status = "disabled";
};
+
+ hsspi: spi@1000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "brcm,bcm63148-hsspi", "brcm,bcmbca-hsspi-v1.0",
+ "brcm,bcmbca-hsspi";
+ reg = <0x1000 0x600>;
+ interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
+ 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..15f550dadc60 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,19 @@ bus@ff800000 {
#size-cells = <1>;
ranges = <0 0xff800000 0x800000>;
+ hsspi: spi@1000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "brcm,bcm63178-hsspi", "brcm,bcmbca-hsspi-v1.0",
+ "brcm,bcmbca-hsspi";
+ reg = <0x1000 0x600>;
+ interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
+ 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..45c09a54ac20 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,20 @@ bus@ff800000 {
#size-cells = <1>;
ranges = <0 0xff800000 0x800000>;
+ hsspi: spi@1000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "brcm,bcm6756-hsspi", "brcm,bcmbca-hsspi-v1.1",
+ "brcm,bcmbca-hsspi";
+ reg = <0x1000 0x600>, <0x2610 0x4>;
+ reg-names = "hsspi", "spim-ctrl";
+ interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
+ 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..e8895d69e249 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,18 @@ uart0: serial@640 {
clock-names = "refclk";
status = "disabled";
};
+
+ hsspi: spi@1000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "brcm,bcm6846-hsspi", "brcm,bcmbca-hsspi-v1.0",
+ "brcm,bcmbca-hsspi";
+ reg = <0x1000 0x600>;
+ interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
+ 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..6ecfe7ac0554 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,20 @@ bus@ff800000 {
#size-cells = <1>;
ranges = <0 0xff800000 0x800000>;
+ hsspi: spi@1000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "brcm,bcm6855-hsspi", "brcm,bcmbca-hsspi-v1.1",
+ "brcm,bcmbca-hsspi";
+ reg = <0x1000 0x600>, <0x2610 0x4>;
+ reg-names = "hsspi", "spim-ctrl";
+ interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
+ 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..636a6ae416b3 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,19 @@ bus@ff800000 {
#size-cells = <1>;
ranges = <0 0xff800000 0x800000>;
+ hsspi: spi@1000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "brcm,bcm6878-hsspi", "brcm,bcmbca-hsspi-v1.0",
+ "brcm,bcmbca-hsspi";
+ reg = <0x1000 0x600>;
+ interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
+ 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
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 <[email protected]>
---
Changes in v2:
- Add the missing reference to spi-controller which fix the
dt_binding_check error.
- Use SPI intead of spi in the description.
.../bindings/spi/brcm,bcm63xx-hsspi.yaml | 55 +++++++++++++++++++
.../bindings/spi/spi-bcm63xx-hsspi.txt | 33 -----------
2 files changed, 55 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..d1a0c9adee7a
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
@@ -0,0 +1,55 @@
+# 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 <[email protected]>
+
+allOf:
+ - $ref: spi-controller.yaml#
+
+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
Add support for HSSPI controller in ARMv8 chip dts files.
Signed-off-by: William Zhang <[email protected]>
---
Changes in v2:
- Update compatible string with SoC model number, controller version
info and bcmbca fall back name
- Add interrupt property
.../boot/dts/broadcom/bcmbca/bcm4908.dtsi | 19 +++++++++++++++++
.../boot/dts/broadcom/bcmbca/bcm4912.dtsi | 21 +++++++++++++++++++
.../boot/dts/broadcom/bcmbca/bcm63146.dtsi | 20 ++++++++++++++++++
.../boot/dts/broadcom/bcmbca/bcm63158.dtsi | 20 ++++++++++++++++++
.../boot/dts/broadcom/bcmbca/bcm6813.dtsi | 21 +++++++++++++++++++
.../boot/dts/broadcom/bcmbca/bcm6856.dtsi | 19 +++++++++++++++++
.../boot/dts/broadcom/bcmbca/bcm6858.dtsi | 19 +++++++++++++++++
.../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, 167 insertions(+)
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi
index eb2a78f4e033..7818cd562e17 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,19 @@ leds: leds@800 {
#size-cells = <0>;
};
+ hsspi: spi@1000{
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "brcm,bcm4908-hsspi", "brcm,bcmbca-hsspi-v1.0",
+ "brcm,bcmbca-hsspi";
+ reg = <0x1000 0x600>;
+ interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
+ 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..75a63b5156b3 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,20 @@ bus@ff800000 {
#size-cells = <1>;
ranges = <0x0 0x0 0xff800000 0x800000>;
+ hsspi: spi@1000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "brcm,bcm4912-hsspi", "brcm,bcmbca-hsspi-v1.1",
+ "brcm,bcmbca-hsspi";
+ reg = <0x1000 0x600>, <0x2610 0x4>;
+ reg-names = "hsspi", "spim-ctrl";
+ interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
+ 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..b4ef4e1ba7e0 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,19 @@ bus@ff800000 {
#size-cells = <1>;
ranges = <0x0 0x0 0xff800000 0x800000>;
+ hsspi: spi@1000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "brcm,bcm63146-hsspi", "brcm,bcmbca-hsspi-v1.0",
+ "brcm,bcmbca-hsspi";
+ reg = <0x1000 0x600>;
+ interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
+ 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..fc11d85235f7 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,19 @@ bus@ff800000 {
#size-cells = <1>;
ranges = <0x0 0x0 0xff800000 0x800000>;
+ hsspi: spi@1000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "brcm,bcm63158-hsspi", "brcm,bcmbca-hsspi-v1.0",
+ "brcm,bcmbca-hsspi";
+ reg = <0x1000 0x600>;
+ interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
+ 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..412fafaede20 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,20 @@ bus@ff800000 {
#size-cells = <1>;
ranges = <0x0 0x0 0xff800000 0x800000>;
+ hsspi: spi@1000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "brcm,bcm6813-hsspi", "brcm,bcmbca-hsspi-v1.1",
+ "brcm,bcmbca-hsspi";
+ reg = <0x1000 0x600>, <0x2610 0x4>;
+ reg-names = "hsspi", "spim-ctrl";
+ interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
+ 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..930110022260 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,18 @@ uart0: serial@640 {
clock-names = "refclk";
status = "disabled";
};
+
+ hsspi: spi@1000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "brcm,bcm6856-hsspi", "brcm,bcmbca-hsspi-v1.0",
+ "brcm,bcmbca-hsspi";
+ reg = <0x1000 0x600>;
+ interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
+ 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..b9c1f7dad934 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,18 @@ uart0: serial@640 {
clock-names = "refclk";
status = "disabled";
};
+
+ hsspi: spi@1000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "brcm,bcm6858-hsspi", "brcm,bcmbca-hsspi-v1.0",
+ "brcm,bcmbca-hsspi";
+ reg = <0x1000 0x600>;
+ interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
+ 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
The new Broadcom Broadband BCMBCA SoCs includes a updated HSSPI
controller. Add new compatible strings to differentiate the old and new
controller while keeping MIPS based chip with the old compatible. Update
property requirements for these two revisions of the controller. Also
add myself and Kursad as the maintainers.
Signed-off-by: William Zhang <[email protected]>
---
Changes in v2:
- Update new compatible string to follow Broadcom convention <chip
specific compatible>, <version of the IP>, <fallback>
- Add reg-names min/maxItem constraints to be consistent with reg
property
- Make interrupts required property
- Remove double quote from spi-controller.yaml reference
- Remove brcm,use-cs-workaround flag
- Update the example with new compatile and interrupts property
- Update commit message
.../bindings/spi/brcm,bcm63xx-hsspi.yaml | 106 +++++++++++++++++-
1 file changed, 101 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
index d1a0c9adee7a..d39604654c9e 100644
--- a/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
+++ b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
@@ -4,20 +4,73 @@
$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 <[email protected]>
+ - Kursad Oney <[email protected]>
- Jonas Gorski <[email protected]>
-allOf:
- - $ref: spi-controller.yaml#
+description: |
+ Broadcom Broadband SoC supports High Speed SPI master controller since the
+ early MIPS based chips such as BCM6328 and BCM63268. This initial rev 1.0
+ controller was carried over to recent ARM based chips, such as BCM63138,
+ BCM4908 and BCM6858. The old MIPS based chip should continue to use the
+ brcm,bcm6328-hsspi compatible string. The recent ARM based chip is required to
+ use the brcm,bcmbca-hsspi-v1.0 as part of its compatible string list as
+ defined below to match the specific chip along with ip revision info.
+
+ This rev 1.0 controller 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. Driver automatically picks the suitable mode based on
+ transfer type so it is transparent to the user.
+
+ The newer SoCs such as BCM6756, BCM4912 and BCM6855 include an updated SPI
+ controller rev 1.1 that add the capability to allow the driver to control chip
+ select explicitly. This solves the issue in the old controller.
properties:
compatible:
- const: brcm,bcm6328-hsspi
+ oneOf:
+ - const: brcm,bcm6328-hsspi
+ - items:
+ - enum:
+ - brcm,bcm47622-hsspi
+ - brcm,bcm4908-hsspi
+ - brcm,bcm63138-hsspi
+ - brcm,bcm63146-hsspi
+ - brcm,bcm63148-hsspi
+ - brcm,bcm63158-hsspi
+ - brcm,bcm63178-hsspi
+ - brcm,bcm6846-hsspi
+ - brcm,bcm6856-hsspi
+ - brcm,bcm6858-hsspi
+ - brcm,bcm6878-hsspi
+ - const: brcm,bcmbca-hsspi-v1.0
+ - const: brcm,bcmbca-hsspi
+ - items:
+ - enum:
+ - brcm,bcm4912-hsspi
+ - brcm,bcm6756-hsspi
+ - brcm,bcm6813-hsspi
+ - brcm,bcm6855-hsspi
+ - const: brcm,bcmbca-hsspi-v1.1
+ - const: brcm,bcmbca-hsspi
reg:
- maxItems: 1
+ items:
+ - description: main registers
+ - description: miscellaneous control registers
+ minItems: 1
+
+ reg-names:
+ items:
+ - const: hsspi
+ - const: spim-ctrl
+ minItems: 1
clocks:
items:
@@ -39,10 +92,39 @@ required:
- clock-names
- interrupts
+allOf:
+ - $ref: spi-controller.yaml#
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - brcm,bcm6328-hsspi
+ - brcm,bcmbca-hsspi-v1.0
+ then:
+ properties:
+ reg:
+ minItems: 1
+ maxItems: 1
+ reg-names:
+ minItems: 1
+ maxItems: 1
+ else:
+ properties:
+ reg:
+ minItems: 2
+ maxItems: 2
+ reg-names:
+ minItems: 2
+ maxItems: 2
+ required:
+ - reg-names
+
unevaluatedProperties: false
examples:
- |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
spi@10001000 {
compatible = "brcm,bcm6328-hsspi";
reg = <0x10001000 0x600>;
@@ -53,3 +135,17 @@ examples:
#address-cells = <1>;
#size-cells = <0>;
};
+ - |
+ spi@ff801000 {
+ compatible = "brcm,bcm6756-hsspi", "brcm,bcmbca-hsspi-v1.1",
+ "brcm,bcmbca-hsspi";
+ reg = <0xff801000 0x1000>,
+ <0xff802610 0x4>;
+ reg-names = "hsspi", "spim-ctrl";
+ interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&hsspi>, <&hsspi_pll>;
+ clock-names = "hsspi", "pll";
+ num-cs = <8>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
--
2.37.3
New compatible string brcm,bcmbca-hsspi-v1.0 is introduced based on dts
document brcm,bcm63xx-hsspi.yaml. Add it to the driver to support this
new binding.
Signed-off-by: William Zhang <[email protected]>
---
(no changes since v1)
drivers/spi/spi-bcm63xx-hsspi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/spi/spi-bcm63xx-hsspi.c b/drivers/spi/spi-bcm63xx-hsspi.c
index b871fd810d80..01d5acad4a1b 100644
--- a/drivers/spi/spi-bcm63xx-hsspi.c
+++ b/drivers/spi/spi-bcm63xx-hsspi.c
@@ -516,6 +516,7 @@ static SIMPLE_DEV_PM_OPS(bcm63xx_hsspi_pm_ops, bcm63xx_hsspi_suspend,
static const struct of_device_id bcm63xx_hsspi_of_match[] = {
{ .compatible = "brcm,bcm6328-hsspi", },
+ { .compatible = "brcm,bcmbca-hsspi-v1.0", },
{ },
};
MODULE_DEVICE_TABLE(of, bcm63xx_hsspi_of_match);
--
2.37.3
HSSPI controller uses big endian for the opcode in the message to the
controller ping pong buffer. Use cpu_to_be16 to properly handle the
endianness for both big and little endian host.
Fixes: 142168eba9dc ("spi: bcm63xx-hsspi: add bcm63xx HSSPI driver")
Signed-off-by: Kursad Oney <[email protected]>
Signed-off-by: William Zhang <[email protected]>
---
Changes in v2:
- Fix build error for Alpha platform
Reported-by: kernel test robot <[email protected]>
drivers/spi/spi-bcm63xx-hsspi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/spi/spi-bcm63xx-hsspi.c b/drivers/spi/spi-bcm63xx-hsspi.c
index 01d5acad4a1b..a65a0ec67641 100644
--- a/drivers/spi/spi-bcm63xx-hsspi.c
+++ b/drivers/spi/spi-bcm63xx-hsspi.c
@@ -194,7 +194,7 @@ static int bcm63xx_hsspi_do_txrx(struct spi_device *spi, struct spi_transfer *t)
tx += curr_step;
}
- __raw_writew(opcode | curr_step, bs->fifo);
+ __raw_writew((u16)cpu_to_be16(opcode | curr_step), bs->fifo);
/* enable interrupt */
__raw_writel(HSSPI_PINGx_CMD_DONE(0),
--
2.37.3
Polling mode provides better throughput in general by avoiding the
interrupt overhead as the maximum data size one interrupt can handle is
only 512 bytes. So switch to polling mode as the default mode but add
a driver sysfs option wait_mode to allow user manually changing the mode
at run time between interrupt and polling. Also add driver banner
message when the driver is loaded successfully.
When test on a Broadcom BCM47622(ARM A7 dual core) reference board with
WINBOND W25N01GV SPI NAND chip at 100MHz SPI clock using the MTD speed
test suite, it shows about 15% improvement on the write and 30% on
the read:
** Interrupt mode **
mtd_speedtest: MTD device: 0 count: 16
mtd_speedtest: MTD device size 134217728, eraseblock size 131072, page
size 2048, count of eraseblocks 1024, pages per eraseblock 64, OOB size
64
mtd_test: scanning for bad eraseblocks
mtd_test: scanned 16 eraseblocks, 0 are bad
mtd_speedtest: testing eraseblock write speed
mtd_speedtest: eraseblock write speed is 3072 KiB/s
mtd_speedtest: testing eraseblock read speed
mtd_speedtest: eraseblock read speed is 6690 KiB/s
mtd_speedtest: testing page write speed
mtd_speedtest: page write speed is 3066 KiB/s
mtd_speedtest: testing page read speed
mtd_speedtest: page read speed is 6762 KiB/s
mtd_speedtest: testing 2 page write speed
mtd_speedtest: 2 page write speed is 3071 KiB/s
mtd_speedtest: testing 2 page read speed
mtd_speedtest: 2 page read speed is 6772 KiB/s
** Polling mode **
mtd_speedtest: MTD device: 0 count: 16
mtd_speedtest: MTD device size 134217728, eraseblock size 131072, page
size 2048, count of eraseblocks 1024, pages per eraseblock 64, OOB size
64
mtd_test: scanning for bad eraseblocks
mtd_test: scanned 16 eraseblocks, 0 are bad
mtd_speedtest: testing eraseblock write speed
mtd_speedtest: eraseblock write speed is 3542 KiB/s
mtd_speedtest: testing eraseblock read speed
mtd_speedtest: eraseblock read speed is 8825 KiB/s
mtd_speedtest: testing page write speed
mtd_speedtest: page write speed is 3563 KiB/s
mtd_speedtest: testing page read speed
mtd_speedtest: page read speed is 8787 KiB/s
mtd_speedtest: testing 2 page write speed
mtd_speedtest: 2 page write speed is 3572 KiB/s
mtd_speedtest: testing 2 page read speed
mtd_speedtest: 2 page read speed is 8806 KiB/s
Signed-off-by: William Zhang <[email protected]>
---
Changes in v2:
- Make interrupt as required node in the dts
- Use polling mode as default mode
- Add driver sysfs option wait_mode to allow mode change at run time
- Update commit message
drivers/spi/spi-bcm63xx-hsspi.c | 109 ++++++++++++++++++++++++++++----
1 file changed, 98 insertions(+), 11 deletions(-)
diff --git a/drivers/spi/spi-bcm63xx-hsspi.c b/drivers/spi/spi-bcm63xx-hsspi.c
index a65a0ec67641..55cbe7deba08 100644
--- a/drivers/spi/spi-bcm63xx-hsspi.c
+++ b/drivers/spi/spi-bcm63xx-hsspi.c
@@ -57,6 +57,7 @@
#define PINGPONG_CMD_SS_SHIFT 12
#define HSSPI_PINGPONG_STATUS_REG(x) (0x84 + (x) * 0x40)
+#define HSSPI_PINGPONG_STATUS_SRC_BUSY BIT(1)
#define HSSPI_PROFILE_CLK_CTRL_REG(x) (0x100 + (x) * 0x20)
#define CLK_CTRL_FREQ_CTRL_MASK 0x0000ffff
@@ -96,11 +97,16 @@
#define HSSPI_SPI_MAX_CS 8
#define HSSPI_BUS_NUM 1 /* 0 is legacy SPI */
+#define HSSPI_POLL_STATUS_TIMEOUT_MS 100
+
+#define HSSPI_WAIT_MODE_POLLING 0
+#define HSSPI_WAIT_MODE_INTR 1
+#define HSSPI_WAIT_MODE_MAX HSSPI_WAIT_MODE_INTR
struct bcm63xx_hsspi {
struct completion done;
struct mutex bus_mutex;
-
+ struct mutex msg_mutex;
struct platform_device *pdev;
struct clk *clk;
struct clk *pll_clk;
@@ -109,6 +115,52 @@ struct bcm63xx_hsspi {
u32 speed_hz;
u8 cs_polarity;
+ u32 wait_mode;
+};
+
+static ssize_t wait_mode_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct spi_controller *ctrl = dev_get_drvdata(dev);
+ struct bcm63xx_hsspi *bs = spi_master_get_devdata(ctrl);
+
+ return sprintf(buf, "%d\n", bs->wait_mode);
+}
+
+static ssize_t wait_mode_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct spi_controller *ctrl = dev_get_drvdata(dev);
+ struct bcm63xx_hsspi *bs = spi_master_get_devdata(ctrl);
+ u32 val;
+
+ if (kstrtou32(buf, 10, &val))
+ return -EINVAL;
+
+ if (val > HSSPI_WAIT_MODE_MAX) {
+ dev_warn(dev, "invalid wait mode %u\n", val);
+ return -EINVAL;
+ }
+
+ mutex_lock(&bs->msg_mutex);
+ bs->wait_mode = val;
+ /* clear interrupt status to avoid spurious int on next transfer */
+ if (val == HSSPI_WAIT_MODE_INTR)
+ __raw_writel(HSSPI_INT_CLEAR_ALL, bs->regs + HSSPI_INT_STATUS_REG);
+ mutex_unlock(&bs->msg_mutex);
+
+ return count;
+}
+
+static DEVICE_ATTR_RW(wait_mode);
+
+static struct attribute *bcm63xx_hsspi_attrs[] = {
+ &dev_attr_wait_mode.attr,
+ NULL,
+};
+
+static const struct attribute_group bcm63xx_hsspi_group = {
+ .attrs = bcm63xx_hsspi_attrs,
};
static void bcm63xx_hsspi_set_cs(struct bcm63xx_hsspi *bs, unsigned int cs,
@@ -163,6 +215,8 @@ static int bcm63xx_hsspi_do_txrx(struct spi_device *spi, struct spi_transfer *t)
int step_size = HSSPI_BUFFER_LEN;
const u8 *tx = t->tx_buf;
u8 *rx = t->rx_buf;
+ u32 val;
+ unsigned long limit;
bcm63xx_hsspi_set_clk(bs, spi, t->speed_hz);
bcm63xx_hsspi_set_cs(bs, spi->chip_select, true);
@@ -197,8 +251,9 @@ static int bcm63xx_hsspi_do_txrx(struct spi_device *spi, struct spi_transfer *t)
__raw_writew((u16)cpu_to_be16(opcode | curr_step), bs->fifo);
/* enable interrupt */
- __raw_writel(HSSPI_PINGx_CMD_DONE(0),
- bs->regs + HSSPI_INT_MASK_REG);
+ if (bs->wait_mode == HSSPI_WAIT_MODE_INTR)
+ __raw_writel(HSSPI_PINGx_CMD_DONE(0),
+ bs->regs + HSSPI_INT_MASK_REG);
/* start the transfer */
__raw_writel(!chip_select << PINGPONG_CMD_SS_SHIFT |
@@ -206,9 +261,21 @@ static int bcm63xx_hsspi_do_txrx(struct spi_device *spi, struct spi_transfer *t)
PINGPONG_COMMAND_START_NOW,
bs->regs + HSSPI_PINGPONG_COMMAND_REG(0));
- if (wait_for_completion_timeout(&bs->done, HZ) == 0) {
- dev_err(&bs->pdev->dev, "transfer timed out!\n");
- return -ETIMEDOUT;
+ if (bs->wait_mode == HSSPI_WAIT_MODE_INTR) {
+ if (wait_for_completion_timeout(&bs->done, HZ) == 0)
+ goto err_timeout;
+ } else {
+ /* polling mode checks for status busy bit */
+ limit = jiffies + msecs_to_jiffies(HSSPI_POLL_STATUS_TIMEOUT_MS);
+ while (!time_after(jiffies, limit)) {
+ val = __raw_readl(bs->regs + HSSPI_PINGPONG_STATUS_REG(0));
+ if (val & HSSPI_PINGPONG_STATUS_SRC_BUSY)
+ cpu_relax();
+ else
+ break;
+ }
+ if (val & HSSPI_PINGPONG_STATUS_SRC_BUSY)
+ goto err_timeout;
}
if (rx) {
@@ -220,6 +287,10 @@ static int bcm63xx_hsspi_do_txrx(struct spi_device *spi, struct spi_transfer *t)
}
return 0;
+
+err_timeout:
+ dev_err(&bs->pdev->dev, "transfer timed out!\n");
+ return -ETIMEDOUT;
}
static int bcm63xx_hsspi_setup(struct spi_device *spi)
@@ -269,6 +340,7 @@ static int bcm63xx_hsspi_transfer_one(struct spi_master *master,
int dummy_cs;
u32 reg;
+ mutex_lock(&bs->msg_mutex);
/* This controller does not support keeping CS active during idle.
* To work around this, we use the following ugly hack:
*
@@ -306,6 +378,7 @@ static int bcm63xx_hsspi_transfer_one(struct spi_master *master,
__raw_writel(reg, bs->regs + HSSPI_GLOBAL_CTRL_REG);
mutex_unlock(&bs->bus_mutex);
+ mutex_unlock(&bs->msg_mutex);
msg->status = status;
spi_finalize_current_message(master);
@@ -398,8 +471,10 @@ static int bcm63xx_hsspi_probe(struct platform_device *pdev)
bs->regs = regs;
bs->speed_hz = rate;
bs->fifo = (u8 __iomem *)(bs->regs + HSSPI_FIFO_REG(0));
+ bs->wait_mode = HSSPI_WAIT_MODE_POLLING;
mutex_init(&bs->bus_mutex);
+ mutex_init(&bs->msg_mutex);
init_completion(&bs->done);
master->dev.of_node = dev->of_node;
@@ -434,21 +509,32 @@ static int bcm63xx_hsspi_probe(struct platform_device *pdev)
__raw_writel(reg | GLOBAL_CTRL_CLK_GATE_SSOFF,
bs->regs + HSSPI_GLOBAL_CTRL_REG);
- ret = devm_request_irq(dev, irq, bcm63xx_hsspi_interrupt, IRQF_SHARED,
- pdev->name, bs);
+ if (irq > 0) {
+ ret = devm_request_irq(dev, irq, bcm63xx_hsspi_interrupt, IRQF_SHARED,
+ pdev->name, bs);
- if (ret)
- goto out_put_master;
+ if (ret)
+ goto out_put_master;
+ }
pm_runtime_enable(&pdev->dev);
+ if (sysfs_create_group(&pdev->dev.kobj, &bcm63xx_hsspi_group)) {
+ dev_err(&pdev->dev, "couldn't register sysfs group\n");
+ goto out_pm_disable;
+ }
+
/* register and we are done */
ret = devm_spi_register_master(dev, master);
if (ret)
- goto out_pm_disable;
+ goto out_sysgroup_disable;
+
+ dev_info(dev, "Broadcom 63XX High Speed SPI Controller driver");
return 0;
+out_sysgroup_disable:
+ sysfs_remove_group(&pdev->dev.kobj, &bcm63xx_hsspi_group);
out_pm_disable:
pm_runtime_disable(&pdev->dev);
out_put_master:
@@ -470,6 +556,7 @@ static int bcm63xx_hsspi_remove(struct platform_device *pdev)
__raw_writel(0, bs->regs + HSSPI_INT_MASK_REG);
clk_disable_unprepare(bs->pll_clk);
clk_disable_unprepare(bs->clk);
+ sysfs_remove_group(&pdev->dev.kobj, &bcm63xx_hsspi_group);
return 0;
}
--
2.37.3
The kernel SPI interface includes the cs_change flag that alters how
the CS behaves.
If we're in the middle of transfers, it tells us to unselect the
CS momentarily since the target device requires that.
If we're at the end of a transfer, it tells us to keep the CS
selected, perhaps because the next transfer is likely targeted
to the same device.
We implement this scheme in the HSSPI driver in this change.
Prior to this change, the CS would toggle momentarily if cs_change
was set for the last transfer. This can be ignored by some or
most devices, but the Microchip TPM2 device does not ignore it.
With the change, the behavior is corrected and the 'glitch' is
eliminated.
Signed-off-by: Kursad Oney <[email protected]>
Signed-off-by: William Zhang <[email protected]>
---
Changes in v2:
- Fix unused variable ‘reg’ compile warning
drivers/spi/spi-bcm63xx-hsspi.c | 29 +++++++++++++++++++++--------
1 file changed, 21 insertions(+), 8 deletions(-)
diff --git a/drivers/spi/spi-bcm63xx-hsspi.c b/drivers/spi/spi-bcm63xx-hsspi.c
index 55cbe7deba08..696e14abba2d 100644
--- a/drivers/spi/spi-bcm63xx-hsspi.c
+++ b/drivers/spi/spi-bcm63xx-hsspi.c
@@ -338,7 +338,7 @@ static int bcm63xx_hsspi_transfer_one(struct spi_master *master,
struct spi_device *spi = msg->spi;
int status = -EINVAL;
int dummy_cs;
- u32 reg;
+ bool restore_polarity = true;
mutex_lock(&bs->msg_mutex);
/* This controller does not support keeping CS active during idle.
@@ -367,16 +367,29 @@ static int bcm63xx_hsspi_transfer_one(struct spi_master *master,
spi_transfer_delay_exec(t);
- if (t->cs_change)
+ /*
+ * cs_change rules:
+ * (1) cs_change = 0 && last_xfer = 0:
+ * Do not touch the CS. On to the next xfer.
+ * (2) cs_change = 1 && last_xfer = 0:
+ * Set cs = false before the next xfer.
+ * (3) cs_change = 0 && last_xfer = 1:
+ * We want CS to be deactivated. So do NOT set cs = false,
+ * instead just restore the original polarity. This has the
+ * same effect of deactivating the CS.
+ * (4) cs_change = 1 && last_xfer = 1:
+ * We want to keep CS active. So do NOT set cs = false, and
+ * make sure we do NOT reverse polarity.
+ */
+ if (t->cs_change && !list_is_last(&t->transfer_list, &msg->transfers))
bcm63xx_hsspi_set_cs(bs, spi->chip_select, false);
+
+ restore_polarity = !t->cs_change;
}
- mutex_lock(&bs->bus_mutex);
- reg = __raw_readl(bs->regs + HSSPI_GLOBAL_CTRL_REG);
- reg &= ~GLOBAL_CTRL_CS_POLARITY_MASK;
- reg |= bs->cs_polarity;
- __raw_writel(reg, bs->regs + HSSPI_GLOBAL_CTRL_REG);
- mutex_unlock(&bs->bus_mutex);
+ bcm63xx_hsspi_set_cs(bs, dummy_cs, false);
+ if (restore_polarity)
+ bcm63xx_hsspi_set_cs(bs, spi->chip_select, false);
mutex_unlock(&bs->msg_mutex);
msg->status = status;
--
2.37.3
Currently exec_op is always required if controller driver provides
mem_ops. But some controller such as bcm63xx-hsspi may only need to
implement other operation like supports_op and use the default
execution operation. This patch removes this restriction.
Signed-off-by: William Zhang <[email protected]>
---
(no changes since v1)
drivers/spi/spi-mem.c | 2 +-
drivers/spi/spi.c | 13 ++++++-------
2 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
index 0c79193d9697..701838b6f0c4 100644
--- a/drivers/spi/spi-mem.c
+++ b/drivers/spi/spi-mem.c
@@ -325,7 +325,7 @@ int spi_mem_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
if (!spi_mem_internal_supports_op(mem, op))
return -ENOTSUPP;
- if (ctlr->mem_ops && !mem->spi->cs_gpiod) {
+ if (ctlr->mem_ops && ctlr->mem_ops->exec_op && !mem->spi->cs_gpiod) {
ret = spi_mem_access_start(mem);
if (ret)
return ret;
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 3cc7bb4d03de..6faa77592e93 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -3051,15 +3051,14 @@ static int spi_controller_check_ops(struct spi_controller *ctlr)
* The controller may implement only the high-level SPI-memory like
* operations if it does not support regular SPI transfers, and this is
* valid use case.
- * If ->mem_ops is NULL, we request that at least one of the
- * ->transfer_xxx() method be implemented.
+ * If ->mem_ops or ->mem_ops->exec_op is NULL, we request that at least
+ * one of the ->transfer_xxx() method be implemented.
*/
- if (ctlr->mem_ops) {
- if (!ctlr->mem_ops->exec_op)
- return -EINVAL;
- } else if (!ctlr->transfer && !ctlr->transfer_one &&
+ if (!ctlr->mem_ops || (ctlr->mem_ops && !ctlr->mem_ops->exec_op)) {
+ if (!ctlr->transfer && !ctlr->transfer_one &&
!ctlr->transfer_one_message) {
- return -EINVAL;
+ return -EINVAL;
+ }
}
return 0;
--
2.37.3
Due to the controller limitation to keep the chip select low during the
bus idle time between the transfer, a dummy cs workaround was used when
this driver was first upstreamed to the kernel. It basically picks the
dummy cs as !actual_cs so typically dummy cs is 1 when most of the case
only cs 0 is used in the board design. Then invert the polarity of both
cs and tell the controller to start the transfers using dummy cs.
Assuming both cs are active low before the inversion, effectively this
keeps dummy cs high and actual cs low during the transfer and workaround
the issue.
This workaround implies that dummy cs 1 pin has to be set to chip
selection function in the pinmux when the transfer clock is above
25MHz. The old chips likely have default pinmux set to chip select on
the dummy cs pin so it works but this is not case for the new Broadband
BCA chips and this workaround stop working. This is specifically an
issue to support SPI NAND and SPI NOR flash because these flash devices
can typically run at or above 100MHz.
This patch utilizes the prepend feature of the controller to combine the
multiple transfers in the same message to a single transfer when
possible. This way there is no need to keep clock low between transfers
and solve the issue without any hardware requirement.
Multiple transfers within a SPI message may be combined into one
transfer if the following are all true:
* One or more half duplex write transfer in single bit mode
* Optional full duplex read/write at the end
* No delay and cs_change between transfers
Most of the SPI device meets this requirements such as SPI NOR,
SPI NAND flash, Broadcom SPI voice card and etc. For any SPI message
that does not meet the above requirement to combine the transfers, we
switch to original dummy cs mode but limit the clock rate to the safe
25MHz. This is the default auto transfer mode and it makes sure all the
SPI message can be supported automatically under the hood.
This patch also adds the driver sysfs node xfer_mode to provide
the option for overriding the default auto mode and force it to dummy cs
or prepend mode.
Signed-off-by: William Zhang <[email protected]>
---
Changes in v2:
- Fix build error for Alpha platform
Reported-by: kernel test robot <[email protected]>
- Remove use_cs_workaround option from device tree
- Change the transfer logic to use try prepend first and if not
prependable, switch to dummy cs mode with clock limit at the 25MHz
- Add driver sysfs node xfer_mode for the option to override the
transfer mode to dummy cs or prepend mode.
- Add number of bits check in the tranfer for prepend mode eligibility
check
- Update commit message
drivers/spi/spi-bcm63xx-hsspi.c | 332 +++++++++++++++++++++++++++++---
1 file changed, 300 insertions(+), 32 deletions(-)
diff --git a/drivers/spi/spi-bcm63xx-hsspi.c b/drivers/spi/spi-bcm63xx-hsspi.c
index 8f0d31764f98..2a0bef943967 100644
--- a/drivers/spi/spi-bcm63xx-hsspi.c
+++ b/drivers/spi/spi-bcm63xx-hsspi.c
@@ -93,7 +93,11 @@
#define HSSPI_MAX_PREPEND_LEN 15
-#define HSSPI_MAX_SYNC_CLOCK 30000000
+/*
+ * Some chip require 30MHz but other require 25MHz. Use smaller value to cover
+ * both cases.
+ */
+#define HSSPI_MAX_SYNC_CLOCK 25000000
#define HSSPI_SPI_MAX_CS 8
#define HSSPI_BUS_NUM 1 /* 0 is legacy SPI */
@@ -103,6 +107,16 @@
#define HSSPI_WAIT_MODE_INTR 1
#define HSSPI_WAIT_MODE_MAX HSSPI_WAIT_MODE_INTR
+/*
+ * Default transfer mode is auto. If the msg is prependable, use the prepend
+ * mode. If not, falls back to use the dummy cs workaround mode but limit the
+ * clock to 25MHz to make sure it works in all board design.
+ */
+#define HSSPI_XFER_MODE_AUTO 0
+#define HSSPI_XFER_MODE_PREPEND 1
+#define HSSPI_XFER_MODE_DUMMYCS 2
+#define HSSPI_XFER_MODE_MAX HSSPI_XFER_MODE_DUMMYCS
+
struct bcm63xx_hsspi {
struct completion done;
struct mutex bus_mutex;
@@ -116,6 +130,9 @@ struct bcm63xx_hsspi {
u32 speed_hz;
u8 cs_polarity;
u32 wait_mode;
+ u32 xfer_mode;
+ u32 prepend_cnt;
+ u8 *prepend_buf;
};
static ssize_t wait_mode_show(struct device *dev, struct device_attribute *attr,
@@ -154,8 +171,42 @@ static ssize_t wait_mode_store(struct device *dev, struct device_attribute *attr
static DEVICE_ATTR_RW(wait_mode);
+static ssize_t xfer_mode_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct spi_controller *ctrl = dev_get_drvdata(dev);
+ struct bcm63xx_hsspi *bs = spi_master_get_devdata(ctrl);
+
+ return sprintf(buf, "%d\n", bs->xfer_mode);
+}
+
+static ssize_t xfer_mode_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct spi_controller *ctrl = dev_get_drvdata(dev);
+ struct bcm63xx_hsspi *bs = spi_master_get_devdata(ctrl);
+ u32 val;
+
+ if (kstrtou32(buf, 10, &val))
+ return -EINVAL;
+
+ if (val > HSSPI_XFER_MODE_MAX) {
+ dev_warn(dev, "invalid xfer mode %u\n", val);
+ return -EINVAL;
+ }
+
+ mutex_lock(&bs->msg_mutex);
+ bs->xfer_mode = val;
+ mutex_unlock(&bs->msg_mutex);
+
+ return count;
+}
+
+static DEVICE_ATTR_RW(xfer_mode);
+
static struct attribute *bcm63xx_hsspi_attrs[] = {
&dev_attr_wait_mode.attr,
+ &dev_attr_xfer_mode.attr,
NULL,
};
@@ -163,6 +214,208 @@ static const struct attribute_group bcm63xx_hsspi_group = {
.attrs = bcm63xx_hsspi_attrs,
};
+static void bcm63xx_hsspi_set_clk(struct bcm63xx_hsspi *bs,
+ struct spi_device *spi, int hz);
+
+static size_t bcm63xx_hsspi_max_message_size(struct spi_device *spi)
+{
+ return HSSPI_BUFFER_LEN - HSSPI_OPCODE_LEN;
+}
+
+static int bcm63xx_hsspi_wait_cmd(struct bcm63xx_hsspi *bs)
+{
+ unsigned long limit;
+ u32 reg = 0;
+ int rc = 0;
+
+ if (bs->wait_mode == HSSPI_WAIT_MODE_INTR) {
+ if (wait_for_completion_timeout(&bs->done, HZ) == 0)
+ rc = 1;
+ } else {
+ /* polling mode checks for status busy bit */
+ limit = jiffies + msecs_to_jiffies(HSSPI_POLL_STATUS_TIMEOUT_MS);
+
+ while (!time_after(jiffies, limit)) {
+ reg = __raw_readl(bs->regs + HSSPI_PINGPONG_STATUS_REG(0));
+ if (reg & HSSPI_PINGPONG_STATUS_SRC_BUSY)
+ cpu_relax();
+ else
+ break;
+ }
+ if (reg & HSSPI_PINGPONG_STATUS_SRC_BUSY)
+ rc = 1;
+ }
+
+ if (rc)
+ dev_err(&bs->pdev->dev, "transfer timed out!\n");
+
+ return rc;
+}
+
+static bool bcm63xx_check_msg_prependable(struct spi_master *master,
+ struct spi_message *msg,
+ struct spi_transfer *t_prepend)
+{
+
+ struct bcm63xx_hsspi *bs = spi_master_get_devdata(master);
+ bool prepend = false, tx_only = false;
+ struct spi_transfer *t;
+
+ /* If it is forced cs dummy workaround mode, no need to prepend message */
+ if (bs->xfer_mode == HSSPI_XFER_MODE_DUMMYCS)
+ return false;
+
+ /*
+ * Multiple transfers within a message may be combined into one transfer
+ * to the controller using its prepend feature. A SPI message is prependable
+ * only if the following are all true:
+ * 1. One or more half duplex write transfer in single bit mode
+ * 2. Optional full duplex read/write at the end
+ * 3. No delay and cs_change between transfers
+ */
+ bs->prepend_cnt = 0;
+ list_for_each_entry(t, &msg->transfers, transfer_list) {
+ if ((spi_delay_to_ns(&t->delay, t) > 0) || t->cs_change) {
+ dev_warn(&bs->pdev->dev,
+ "Delay or cs change not supported in prepend mode!\n");
+ break;
+ }
+
+ tx_only = false;
+ if (t->tx_buf && !t->rx_buf) {
+ tx_only = true;
+ if (bs->prepend_cnt + t->len >
+ (HSSPI_BUFFER_LEN - HSSPI_OPCODE_LEN)) {
+ dev_warn(&bs->pdev->dev,
+ "exceed max buf len, abort prepending transfers!\n");
+ break;
+ }
+
+ if (t->tx_nbits > SPI_NBITS_SINGLE &&
+ !list_is_last(&t->transfer_list, &msg->transfers)) {
+ dev_warn(&bs->pdev->dev,
+ "multi-bit prepend buf not supported!\n");
+ break;
+ }
+
+ if (t->tx_nbits == SPI_NBITS_SINGLE) {
+ memcpy(bs->prepend_buf + bs->prepend_cnt, t->tx_buf, t->len);
+ bs->prepend_cnt += t->len;
+ }
+ } else {
+ if (!list_is_last(&t->transfer_list, &msg->transfers)) {
+ dev_warn(&bs->pdev->dev,
+ "rx/tx_rx transfer not supported when it is not last one!\n");
+ break;
+ }
+ }
+
+ if (list_is_last(&t->transfer_list, &msg->transfers)) {
+ memcpy(t_prepend, t, sizeof(struct spi_transfer));
+
+ if (tx_only && t->tx_nbits == SPI_NBITS_SINGLE) {
+ /*
+ * if the last one is also a single bit tx only transfer, merge
+ * all of them into one single tx transfer
+ */
+ t_prepend->len = bs->prepend_cnt;
+ t_prepend->tx_buf = bs->prepend_buf;
+ bs->prepend_cnt = 0;
+ } else {
+ /*
+ * if the last one is not a tx only transfer or dual tx xfer, all
+ * the previous transfers are sent through prepend bytes and
+ * make sure it does not exceed the max prepend len
+ */
+ if (bs->prepend_cnt > HSSPI_MAX_PREPEND_LEN) {
+ dev_warn(&bs->pdev->dev,
+ "exceed max prepend len, abort prepending transfers!\n");
+ break;
+ }
+ }
+ prepend = true;
+ }
+ }
+
+ return prepend;
+}
+
+static int bcm63xx_hsspi_do_prepend_txrx(struct spi_device *spi,
+ struct spi_transfer *t)
+{
+ struct bcm63xx_hsspi *bs = spi_master_get_devdata(spi->master);
+ unsigned int chip_select = spi->chip_select;
+ u16 opcode = 0;
+ const u8 *tx = t->tx_buf;
+ u8 *rx = t->rx_buf;
+ u32 reg = 0;
+
+ /*
+ * shouldn't happen as we set the max_message_size in the probe.
+ * but check it again in case some driver does not honor the max size
+ */
+ if (t->len + bs->prepend_cnt > (HSSPI_BUFFER_LEN - HSSPI_OPCODE_LEN)) {
+ dev_warn(&bs->pdev->dev,
+ "Prepend message large than fifo size len %d prepend %d\n",
+ t->len, bs->prepend_cnt);
+ return -EINVAL;
+ }
+
+ bcm63xx_hsspi_set_clk(bs, spi, t->speed_hz);
+
+ if (tx && rx)
+ opcode = HSSPI_OP_READ_WRITE;
+ else if (tx)
+ opcode = HSSPI_OP_WRITE;
+ else if (rx)
+ opcode = HSSPI_OP_READ;
+
+ if ((opcode == HSSPI_OP_READ && t->rx_nbits == SPI_NBITS_DUAL) ||
+ (opcode == HSSPI_OP_WRITE && t->tx_nbits == SPI_NBITS_DUAL)) {
+ opcode |= HSSPI_OP_MULTIBIT;
+
+ if (t->rx_nbits == SPI_NBITS_DUAL) {
+ reg |= 1 << MODE_CTRL_MULTIDATA_RD_SIZE_SHIFT;
+ reg |= bs->prepend_cnt << MODE_CTRL_MULTIDATA_RD_STRT_SHIFT;
+ }
+ if (t->tx_nbits == SPI_NBITS_DUAL) {
+ reg |= 1 << MODE_CTRL_MULTIDATA_WR_SIZE_SHIFT;
+ reg |= bs->prepend_cnt << MODE_CTRL_MULTIDATA_WR_STRT_SHIFT;
+ }
+ }
+
+ reg |= bs->prepend_cnt << MODE_CTRL_PREPENDBYTE_CNT_SHIFT;
+ __raw_writel(reg | 0xff,
+ bs->regs + HSSPI_PROFILE_MODE_CTRL_REG(chip_select));
+
+ reinit_completion(&bs->done);
+ if (bs->prepend_cnt)
+ memcpy_toio(bs->fifo + HSSPI_OPCODE_LEN, bs->prepend_buf,
+ bs->prepend_cnt);
+ if (tx)
+ memcpy_toio(bs->fifo + HSSPI_OPCODE_LEN + bs->prepend_cnt, tx,
+ t->len);
+
+ __raw_writew((u16)cpu_to_be16(opcode | t->len), bs->fifo);
+ /* enable interrupt */
+ if (bs->wait_mode == HSSPI_WAIT_MODE_INTR)
+ __raw_writel(HSSPI_PINGx_CMD_DONE(0), bs->regs + HSSPI_INT_MASK_REG);
+
+ /* start the transfer */
+ reg = chip_select << PINGPONG_CMD_SS_SHIFT |
+ chip_select << PINGPONG_CMD_PROFILE_SHIFT |
+ PINGPONG_COMMAND_START_NOW;
+ __raw_writel(reg, bs->regs + HSSPI_PINGPONG_COMMAND_REG(0));
+
+ if (bcm63xx_hsspi_wait_cmd(bs))
+ return -ETIMEDOUT;
+
+ if (rx)
+ memcpy_fromio(rx, bs->fifo, t->len);
+
+ return 0;
+}
+
static void bcm63xx_hsspi_set_cs(struct bcm63xx_hsspi *bs, unsigned int cs,
bool active)
{
@@ -215,10 +468,10 @@ static int bcm63xx_hsspi_do_txrx(struct spi_device *spi, struct spi_transfer *t)
int step_size = HSSPI_BUFFER_LEN;
const u8 *tx = t->tx_buf;
u8 *rx = t->rx_buf;
- u32 val = 0;
- unsigned long limit;
+ u32 reg = 0;
bcm63xx_hsspi_set_clk(bs, spi, t->speed_hz);
+
bcm63xx_hsspi_set_cs(bs, spi->chip_select, true);
if (tx && rx)
@@ -236,12 +489,12 @@ static int bcm63xx_hsspi_do_txrx(struct spi_device *spi, struct spi_transfer *t)
opcode |= HSSPI_OP_MULTIBIT;
if (t->rx_nbits == SPI_NBITS_DUAL)
- val |= 1 << MODE_CTRL_MULTIDATA_RD_SIZE_SHIFT;
+ reg |= 1 << MODE_CTRL_MULTIDATA_RD_SIZE_SHIFT;
if (t->tx_nbits == SPI_NBITS_DUAL)
- val |= 1 << MODE_CTRL_MULTIDATA_WR_SIZE_SHIFT;
+ reg |= 1 << MODE_CTRL_MULTIDATA_WR_SIZE_SHIFT;
}
- __raw_writel(val | 0xff,
+ __raw_writel(reg | 0xff,
bs->regs + HSSPI_PROFILE_MODE_CTRL_REG(chip_select));
while (pending > 0) {
@@ -260,28 +513,13 @@ static int bcm63xx_hsspi_do_txrx(struct spi_device *spi, struct spi_transfer *t)
__raw_writel(HSSPI_PINGx_CMD_DONE(0),
bs->regs + HSSPI_INT_MASK_REG);
- /* start the transfer */
- __raw_writel(!chip_select << PINGPONG_CMD_SS_SHIFT |
- chip_select << PINGPONG_CMD_PROFILE_SHIFT |
- PINGPONG_COMMAND_START_NOW,
- bs->regs + HSSPI_PINGPONG_COMMAND_REG(0));
+ reg = !chip_select << PINGPONG_CMD_SS_SHIFT |
+ chip_select << PINGPONG_CMD_PROFILE_SHIFT |
+ PINGPONG_COMMAND_START_NOW;
+ __raw_writel(reg, bs->regs + HSSPI_PINGPONG_COMMAND_REG(0));
- if (bs->wait_mode == HSSPI_WAIT_MODE_INTR) {
- if (wait_for_completion_timeout(&bs->done, HZ) == 0)
- goto err_timeout;
- } else {
- /* polling mode checks for status busy bit */
- limit = jiffies + msecs_to_jiffies(HSSPI_POLL_STATUS_TIMEOUT_MS);
- while (!time_after(jiffies, limit)) {
- val = __raw_readl(bs->regs + HSSPI_PINGPONG_STATUS_REG(0));
- if (val & HSSPI_PINGPONG_STATUS_SRC_BUSY)
- cpu_relax();
- else
- break;
- }
- if (val & HSSPI_PINGPONG_STATUS_SRC_BUSY)
- goto err_timeout;
- }
+ if (bcm63xx_hsspi_wait_cmd(bs))
+ return -ETIMEDOUT;
if (rx) {
memcpy_fromio(rx, bs->fifo, curr_step);
@@ -292,10 +530,6 @@ static int bcm63xx_hsspi_do_txrx(struct spi_device *spi, struct spi_transfer *t)
}
return 0;
-
-err_timeout:
- dev_err(&bs->pdev->dev, "transfer timed out!\n");
- return -ETIMEDOUT;
}
static int bcm63xx_hsspi_setup(struct spi_device *spi)
@@ -344,9 +578,23 @@ static int bcm63xx_hsspi_transfer_one(struct spi_master *master,
int status = -EINVAL;
int dummy_cs;
bool restore_polarity = true;
+ struct spi_transfer t_prepend;
mutex_lock(&bs->msg_mutex);
- /* This controller does not support keeping CS active during idle.
+ if (bcm63xx_check_msg_prependable(master, msg, &t_prepend)) {
+ status = bcm63xx_hsspi_do_prepend_txrx(spi, &t_prepend);
+ msg->actual_length += (t_prepend.len + bs->prepend_cnt);
+ goto msg_done;
+ }
+
+ if (bs->xfer_mode == HSSPI_XFER_MODE_PREPEND) {
+ dev_warn(&bs->pdev->dev,
+ "User set prepend mode but msg not prependable! Fail the xfer!\n");
+ goto msg_done;
+ }
+
+ /*
+ * This controller does not support keeping CS active during idle.
* To work around this, we use the following ugly hack:
*
* a. Invert the target chip select's polarity so it will be active.
@@ -364,6 +612,17 @@ static int bcm63xx_hsspi_transfer_one(struct spi_master *master,
bcm63xx_hsspi_set_cs(bs, dummy_cs, true);
list_for_each_entry(t, &msg->transfers, transfer_list) {
+ /*
+ * We are here because one of reasons below:
+ * a. Message is not prependable and in default auto xfer mode. This mean
+ * we fallback to dummy cs mode at maximum 25MHz safe clock rate.
+ * b. User set to use the dummy cs mode.
+ */
+ if (bs->xfer_mode == HSSPI_XFER_MODE_AUTO) {
+ if (t->speed_hz > HSSPI_MAX_SYNC_CLOCK)
+ t->speed_hz = HSSPI_MAX_SYNC_CLOCK;
+ }
+
status = bcm63xx_hsspi_do_txrx(spi, t);
if (status)
break;
@@ -396,6 +655,7 @@ static int bcm63xx_hsspi_transfer_one(struct spi_master *master,
if (restore_polarity)
bcm63xx_hsspi_set_cs(bs, spi->chip_select, false);
+msg_done:
mutex_unlock(&bs->msg_mutex);
msg->status = status;
spi_finalize_current_message(master);
@@ -490,6 +750,11 @@ static int bcm63xx_hsspi_probe(struct platform_device *pdev)
bs->speed_hz = rate;
bs->fifo = (u8 __iomem *)(bs->regs + HSSPI_FIFO_REG(0));
bs->wait_mode = HSSPI_WAIT_MODE_POLLING;
+ bs->prepend_buf = devm_kzalloc(dev, HSSPI_BUFFER_LEN, GFP_KERNEL);
+ if (!bs->prepend_buf) {
+ ret = -ENOMEM;
+ goto out_put_master;
+ }
mutex_init(&bs->bus_mutex);
mutex_init(&bs->msg_mutex);
@@ -508,6 +773,9 @@ static int bcm63xx_hsspi_probe(struct platform_device *pdev)
master->num_chipselect = num_cs;
master->setup = bcm63xx_hsspi_setup;
master->transfer_one_message = bcm63xx_hsspi_transfer_one;
+ master->max_transfer_size = bcm63xx_hsspi_max_message_size;
+ master->max_message_size = bcm63xx_hsspi_max_message_size;
+
master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH |
SPI_RX_DUAL | SPI_TX_DUAL;
master->bits_per_word_mask = SPI_BPW_MASK(8);
--
2.37.3
In general the controller supports SPI dual mode operation but the
particular SPI flash dual io read op switches from single mode in cmd
phase to dual mode in address and data phase. This is not compatible
with prepend operation where cmd and address are sent out through the
prepend buffer and they must use same the number of io pins.
This patch disables these SPI flash dual io read ops through the mem_ops
supports_op interface. This makes sure the SPI flash driver selects the
compatible read ops at run time.
Signed-off-by: William Zhang <[email protected]>
---
Changes in v2:
- Remove the code that uses the deprecated flag use_cs_workaround
- Always disable dual io read ops as prepend is the default mode
drivers/spi/spi-bcm63xx-hsspi.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/spi/spi-bcm63xx-hsspi.c b/drivers/spi/spi-bcm63xx-hsspi.c
index 2a0bef943967..dd320fda7611 100644
--- a/drivers/spi/spi-bcm63xx-hsspi.c
+++ b/drivers/spi/spi-bcm63xx-hsspi.c
@@ -20,6 +20,7 @@
#include <linux/spi/spi.h>
#include <linux/mutex.h>
#include <linux/of.h>
+#include <linux/spi/spi-mem.h>
#include <linux/reset.h>
#include <linux/pm_runtime.h>
@@ -663,6 +664,23 @@ static int bcm63xx_hsspi_transfer_one(struct spi_master *master,
return 0;
}
+static bool bcm63xx_hsspi_mem_supports_op(struct spi_mem *mem,
+ const struct spi_mem_op *op)
+{
+ if (!spi_mem_default_supports_op(mem, op))
+ return false;
+
+ /* Controller doesn't support spi mem dual/quad read cmd in prepend mode */
+ if ((op->cmd.opcode == 0xbb) || (op->cmd.opcode == 0xeb))
+ return false;
+
+ return true;
+}
+
+static const struct spi_controller_mem_ops bcm63xx_hsspi_mem_ops = {
+ .supports_op = bcm63xx_hsspi_mem_supports_op,
+};
+
static irqreturn_t bcm63xx_hsspi_interrupt(int irq, void *dev_id)
{
struct bcm63xx_hsspi *bs = (struct bcm63xx_hsspi *)dev_id;
@@ -760,6 +778,7 @@ static int bcm63xx_hsspi_probe(struct platform_device *pdev)
mutex_init(&bs->msg_mutex);
init_completion(&bs->done);
+ master->mem_ops = &bcm63xx_hsspi_mem_ops;
master->dev.of_node = dev->of_node;
if (!dev->of_node)
master->bus_num = HSSPI_BUS_NUM;
--
2.37.3
Currently the driver always sets the controller to dual data bit mode
for both tx and rx data in the profile mode control register even for
single data bit transfer. Luckily the opcode is set correctly according
to SPI transfer data bit width so it does not actually cause issues.
This change fixes the problem by setting tx and rx data bit mode field
correctly according to the actual SPI transfer tx and rx data bit width.
Fixes: 142168eba9dc ("spi: bcm63xx-hsspi: add bcm63xx HSSPI driver")
Signed-off-by: William Zhang <[email protected]>
---
(no changes since v1)
drivers/spi/spi-bcm63xx-hsspi.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/spi-bcm63xx-hsspi.c b/drivers/spi/spi-bcm63xx-hsspi.c
index 696e14abba2d..8f0d31764f98 100644
--- a/drivers/spi/spi-bcm63xx-hsspi.c
+++ b/drivers/spi/spi-bcm63xx-hsspi.c
@@ -215,7 +215,7 @@ static int bcm63xx_hsspi_do_txrx(struct spi_device *spi, struct spi_transfer *t)
int step_size = HSSPI_BUFFER_LEN;
const u8 *tx = t->tx_buf;
u8 *rx = t->rx_buf;
- u32 val;
+ u32 val = 0;
unsigned long limit;
bcm63xx_hsspi_set_clk(bs, spi, t->speed_hz);
@@ -232,11 +232,16 @@ static int bcm63xx_hsspi_do_txrx(struct spi_device *spi, struct spi_transfer *t)
step_size -= HSSPI_OPCODE_LEN;
if ((opcode == HSSPI_OP_READ && t->rx_nbits == SPI_NBITS_DUAL) ||
- (opcode == HSSPI_OP_WRITE && t->tx_nbits == SPI_NBITS_DUAL))
+ (opcode == HSSPI_OP_WRITE && t->tx_nbits == SPI_NBITS_DUAL)) {
opcode |= HSSPI_OP_MULTIBIT;
- __raw_writel(1 << MODE_CTRL_MULTIDATA_WR_SIZE_SHIFT |
- 1 << MODE_CTRL_MULTIDATA_RD_SIZE_SHIFT | 0xff,
+ if (t->rx_nbits == SPI_NBITS_DUAL)
+ val |= 1 << MODE_CTRL_MULTIDATA_RD_SIZE_SHIFT;
+ if (t->tx_nbits == SPI_NBITS_DUAL)
+ val |= 1 << MODE_CTRL_MULTIDATA_WR_SIZE_SHIFT;
+ }
+
+ __raw_writel(val | 0xff,
bs->regs + HSSPI_PROFILE_MODE_CTRL_REG(chip_select));
while (pending > 0) {
--
2.37.3
The newer BCMBCA 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. Driver can control and keep cs low
between the transfers natively. Hence the dummy cs workaround or prepend
mode found in the bcm63xx-hsspi driver are no longer needed and this new
driver is much cleaner.
Signed-off-by: William Zhang <[email protected]>
---
Changes in v2:
- Fix build error for Alpha platform
Reported-by: kernel test robot <[email protected]>
- Make interrupt as required node in the dts
- Use polling mode as default mode
- Add driver sysfs option wait_mode to allow mode change at run time
- Update the compatible string based on changes in dts document
- Remove clock gate disabling code for now
- Update commit message
drivers/spi/Kconfig | 9 +
drivers/spi/Makefile | 1 +
drivers/spi/spi-bcmbca-hsspi.c | 645 +++++++++++++++++++++++++++++++++
3 files changed, 655 insertions(+)
create mode 100644 drivers/spi/spi-bcmbca-hsspi.c
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 3b1c0878bb85..771244582d03 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -199,6 +199,15 @@ config SPI_BCM_QSPI
based platforms. This driver works for both SPI master for SPI NOR
flash device as well as MSPI device.
+config SPI_BCMBCA_HSSPI
+ tristate "Broadcom BCMBCA HS SPI controller driver"
+ depends on ARCH_BCMBCA || COMPILE_TEST
+ help
+ This enables support for the High Speed SPI controller present on
+ newer Broadcom BCMBCA SoCs. These SoCs include an updated SPI controller
+ that adds the capability to allow the driver to control chip select
+ explicitly.
+
config SPI_BITBANG
tristate "Utilities for Bitbanging SPI masters"
help
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index be9ba40ef8d0..fe92106447c3 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_SPI_BCM2835) += spi-bcm2835.o
obj-$(CONFIG_SPI_BCM2835AUX) += spi-bcm2835aux.o
obj-$(CONFIG_SPI_BCM63XX) += spi-bcm63xx.o
obj-$(CONFIG_SPI_BCM63XX_HSSPI) += spi-bcm63xx-hsspi.o
+obj-$(CONFIG_SPI_BCMBCA_HSSPI) += spi-bcmbca-hsspi.o
obj-$(CONFIG_SPI_BCM_QSPI) += spi-iproc-qspi.o spi-brcmstb-qspi.o spi-bcm-qspi.o
obj-$(CONFIG_SPI_BITBANG) += spi-bitbang.o
obj-$(CONFIG_SPI_BUTTERFLY) += spi-butterfly.o
diff --git a/drivers/spi/spi-bcmbca-hsspi.c b/drivers/spi/spi-bcmbca-hsspi.c
new file mode 100644
index 000000000000..f8da194939c0
--- /dev/null
+++ b/drivers/spi/spi-bcmbca-hsspi.c
@@ -0,0 +1,645 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Broadcom BCMBCA High Speed SPI Controller driver
+ *
+ * Copyright 2000-2010 Broadcom Corporation
+ * Copyright 2012-2013 Jonas Gorski <[email protected]>
+ * Copyright 2019-2022 Broadcom Ltd
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/delay.h>
+#include <linux/dma-mapping.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/spi/spi.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/spi/spi-mem.h>
+#include <linux/pm_runtime.h>
+
+#define HSSPI_GLOBAL_CTRL_REG 0x0
+#define GLOBAL_CTRL_CS_POLARITY_SHIFT 0
+#define GLOBAL_CTRL_CS_POLARITY_MASK 0x000000ff
+#define GLOBAL_CTRL_PLL_CLK_CTRL_SHIFT 8
+#define GLOBAL_CTRL_PLL_CLK_CTRL_MASK 0x0000ff00
+#define GLOBAL_CTRL_CLK_GATE_SSOFF BIT(16)
+#define GLOBAL_CTRL_CLK_POLARITY BIT(17)
+#define GLOBAL_CTRL_MOSI_IDLE BIT(18)
+
+#define HSSPI_GLOBAL_EXT_TRIGGER_REG 0x4
+
+#define HSSPI_INT_STATUS_REG 0x8
+#define HSSPI_INT_STATUS_MASKED_REG 0xc
+#define HSSPI_INT_MASK_REG 0x10
+
+#define HSSPI_PINGx_CMD_DONE(i) BIT((i * 8) + 0)
+#define HSSPI_PINGx_RX_OVER(i) BIT((i * 8) + 1)
+#define HSSPI_PINGx_TX_UNDER(i) BIT((i * 8) + 2)
+#define HSSPI_PINGx_POLL_TIMEOUT(i) BIT((i * 8) + 3)
+#define HSSPI_PINGx_CTRL_INVAL(i) BIT((i * 8) + 4)
+
+#define HSSPI_INT_CLEAR_ALL 0xff001f1f
+
+#define HSSPI_PINGPONG_COMMAND_REG(x) (0x80 + (x) * 0x40)
+#define PINGPONG_CMD_COMMAND_MASK 0xf
+#define PINGPONG_COMMAND_NOOP 0
+#define PINGPONG_COMMAND_START_NOW 1
+#define PINGPONG_COMMAND_START_TRIGGER 2
+#define PINGPONG_COMMAND_HALT 3
+#define PINGPONG_COMMAND_FLUSH 4
+#define PINGPONG_CMD_PROFILE_SHIFT 8
+#define PINGPONG_CMD_SS_SHIFT 12
+
+#define HSSPI_PINGPONG_STATUS_REG(x) (0x84 + (x) * 0x40)
+#define HSSPI_PINGPONG_STATUS_SRC_BUSY BIT(1)
+
+#define HSSPI_PROFILE_CLK_CTRL_REG(x) (0x100 + (x) * 0x20)
+#define CLK_CTRL_FREQ_CTRL_MASK 0x0000ffff
+#define CLK_CTRL_SPI_CLK_2X_SEL BIT(14)
+#define CLK_CTRL_ACCUM_RST_ON_LOOP BIT(15)
+#define CLK_CTRL_CLK_POLARITY BIT(16)
+
+#define HSSPI_PROFILE_SIGNAL_CTRL_REG(x) (0x104 + (x) * 0x20)
+#define SIGNAL_CTRL_LATCH_RISING BIT(12)
+#define SIGNAL_CTRL_LAUNCH_RISING BIT(13)
+#define SIGNAL_CTRL_ASYNC_INPUT_PATH BIT(16)
+
+#define HSSPI_PROFILE_MODE_CTRL_REG(x) (0x108 + (x) * 0x20)
+#define MODE_CTRL_MULTIDATA_RD_STRT_SHIFT 8
+#define MODE_CTRL_MULTIDATA_WR_STRT_SHIFT 12
+#define MODE_CTRL_MULTIDATA_RD_SIZE_SHIFT 16
+#define MODE_CTRL_MULTIDATA_WR_SIZE_SHIFT 18
+#define MODE_CTRL_MODE_3WIRE BIT(20)
+#define MODE_CTRL_PREPENDBYTE_CNT_SHIFT 24
+
+#define HSSPI_FIFO_REG(x) (0x200 + (x) * 0x200)
+
+#define HSSPI_OP_MULTIBIT BIT(11)
+#define HSSPI_OP_CODE_SHIFT 13
+#define HSSPI_OP_SLEEP (0 << HSSPI_OP_CODE_SHIFT)
+#define HSSPI_OP_READ_WRITE (1 << HSSPI_OP_CODE_SHIFT)
+#define HSSPI_OP_WRITE (2 << HSSPI_OP_CODE_SHIFT)
+#define HSSPI_OP_READ (3 << HSSPI_OP_CODE_SHIFT)
+#define HSSPI_OP_SETIRQ (4 << HSSPI_OP_CODE_SHIFT)
+
+#define HSSPI_BUFFER_LEN 512
+#define HSSPI_OPCODE_LEN 2
+
+#define HSSPI_MAX_PREPEND_LEN 15
+
+#define HSSPI_MAX_SYNC_CLOCK 30000000
+
+#define HSSPI_SPI_MAX_CS 8
+#define HSSPI_BUS_NUM 1 /* 0 is legacy SPI */
+#define HSSPI_POLL_STATUS_TIMEOUT_MS 100
+
+#define HSSPI_WAIT_MODE_POLLING 0
+#define HSSPI_WAIT_MODE_INTR 1
+#define HSSPI_WAIT_MODE_MAX HSSPI_WAIT_MODE_INTR
+
+#define SPIM_CTRL_CS_OVERRIDE_SEL_SHIFT 0
+#define SPIM_CTRL_CS_OVERRIDE_SEL_MASK 0xff
+#define SPIM_CTRL_CS_OVERRIDE_VAL_SHIFT 8
+#define SPIM_CTRL_CS_OVERRIDE_VAL_MASK 0xff
+
+struct bcmbca_hsspi {
+ struct completion done;
+ struct mutex bus_mutex;
+ struct mutex msg_mutex;
+ struct platform_device *pdev;
+ struct clk *clk;
+ struct clk *pll_clk;
+ void __iomem *regs;
+ void __iomem *spim_ctrl;
+ u8 __iomem *fifo;
+ u32 speed_hz;
+ u8 cs_polarity;
+ u32 wait_mode;
+};
+
+static ssize_t wait_mode_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct spi_controller *ctrl = dev_get_drvdata(dev);
+ struct bcmbca_hsspi *bs = spi_master_get_devdata(ctrl);
+
+ return sprintf(buf, "%d\n", bs->wait_mode);
+}
+
+static ssize_t wait_mode_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct spi_controller *ctrl = dev_get_drvdata(dev);
+ struct bcmbca_hsspi *bs = spi_master_get_devdata(ctrl);
+ u32 val;
+
+ if (kstrtou32(buf, 10, &val))
+ return -EINVAL;
+
+ if (val > HSSPI_WAIT_MODE_MAX) {
+ dev_warn(dev, "invalid wait mode %u\n", val);
+ return -EINVAL;
+ }
+
+ mutex_lock(&bs->msg_mutex);
+ bs->wait_mode = val;
+ /* clear interrupt status to avoid spurious int on next transfer */
+ if (val == HSSPI_WAIT_MODE_INTR)
+ __raw_writel(HSSPI_INT_CLEAR_ALL, bs->regs + HSSPI_INT_STATUS_REG);
+ mutex_unlock(&bs->msg_mutex);
+
+ return count;
+}
+
+static DEVICE_ATTR_RW(wait_mode);
+
+static struct attribute *bcmbca_hsspi_attrs[] = {
+ &dev_attr_wait_mode.attr,
+ NULL,
+};
+
+static const struct attribute_group bcmbca_hsspi_group = {
+ .attrs = bcmbca_hsspi_attrs,
+};
+
+static void bcmbca_hsspi_set_clk(struct bcmbca_hsspi *bs,
+ struct spi_device *spi, int hz);
+
+static void bcmbca_hsspi_set_cs(struct bcmbca_hsspi *bs, unsigned int cs,
+ bool active)
+{
+ u32 reg;
+
+ /* No cs orerriden needed for SS7 internal cs on pcm based voice dev */
+ if (cs == 7)
+ return;
+
+ mutex_lock(&bs->bus_mutex);
+
+ if (active) {
+ /* activate cs by setting the override bit and active value bit*/
+ reg = __raw_readl(bs->spim_ctrl);
+ reg |= BIT(cs+SPIM_CTRL_CS_OVERRIDE_SEL_SHIFT);
+ reg &= ~BIT(cs+SPIM_CTRL_CS_OVERRIDE_VAL_SHIFT);
+ if (bs->cs_polarity & BIT(cs))
+ reg |= BIT(cs+SPIM_CTRL_CS_OVERRIDE_VAL_SHIFT);
+ __raw_writel(reg, bs->spim_ctrl);
+ } else {
+ /* clear the cs override bit */
+ reg = __raw_readl(bs->spim_ctrl);
+ reg &= ~BIT(cs+SPIM_CTRL_CS_OVERRIDE_SEL_SHIFT);
+ __raw_writel(reg, bs->spim_ctrl);
+ }
+
+ mutex_unlock(&bs->bus_mutex);
+}
+
+static void bcmbca_hsspi_set_clk(struct bcmbca_hsspi *bs,
+ struct spi_device *spi, int hz)
+{
+ unsigned int profile = spi->chip_select;
+ u32 reg;
+
+ reg = DIV_ROUND_UP(2048, DIV_ROUND_UP(bs->speed_hz, hz));
+ __raw_writel(CLK_CTRL_ACCUM_RST_ON_LOOP | reg,
+ bs->regs + HSSPI_PROFILE_CLK_CTRL_REG(profile));
+
+ reg = __raw_readl(bs->regs + HSSPI_PROFILE_SIGNAL_CTRL_REG(profile));
+ if (hz > HSSPI_MAX_SYNC_CLOCK)
+ reg |= SIGNAL_CTRL_ASYNC_INPUT_PATH;
+ else
+ reg &= ~SIGNAL_CTRL_ASYNC_INPUT_PATH;
+ __raw_writel(reg, bs->regs + HSSPI_PROFILE_SIGNAL_CTRL_REG(profile));
+
+ mutex_lock(&bs->bus_mutex);
+ /* setup clock polarity */
+ reg = __raw_readl(bs->regs + HSSPI_GLOBAL_CTRL_REG);
+ reg &= ~GLOBAL_CTRL_CLK_POLARITY;
+ if (spi->mode & SPI_CPOL)
+ reg |= GLOBAL_CTRL_CLK_POLARITY;
+
+ __raw_writel(reg, bs->regs + HSSPI_GLOBAL_CTRL_REG);
+
+ mutex_unlock(&bs->bus_mutex);
+}
+
+static int bcmbca_hsspi_wait_cmd(struct bcmbca_hsspi *bs, unsigned int cs)
+{
+ unsigned long limit;
+ u32 reg = 0;
+ int rc = 0;
+
+ if (bs->wait_mode == HSSPI_WAIT_MODE_INTR) {
+ if (wait_for_completion_timeout(&bs->done, HZ) == 0)
+ rc = 1;
+ } else {
+ limit = jiffies + msecs_to_jiffies(HSSPI_POLL_STATUS_TIMEOUT_MS);
+
+ while (!time_after(jiffies, limit)) {
+ reg = __raw_readl(bs->regs + HSSPI_PINGPONG_STATUS_REG(0));
+ if (reg & HSSPI_PINGPONG_STATUS_SRC_BUSY)
+ cpu_relax();
+ else
+ break;
+ }
+ if (reg & HSSPI_PINGPONG_STATUS_SRC_BUSY)
+ rc = 1;
+ }
+
+ if (rc) {
+ dev_err(&bs->pdev->dev, "transfer timed out!\n");
+ bcmbca_hsspi_set_cs(bs, cs, false);
+ }
+
+ return rc;
+}
+
+static int bcmbca_hsspi_do_txrx(struct spi_device *spi, struct spi_transfer *t,
+ struct spi_message *msg)
+{
+ struct bcmbca_hsspi *bs = spi_master_get_devdata(spi->master);
+ unsigned int chip_select = spi->chip_select;
+ u16 opcode = 0;
+ int pending = t->len;
+ int step_size = HSSPI_BUFFER_LEN;
+ const u8 *tx = t->tx_buf;
+ u8 *rx = t->rx_buf;
+ u32 reg = 0, cs_act = 0;
+
+ bcmbca_hsspi_set_clk(bs, spi, t->speed_hz);
+
+ if (tx && rx)
+ opcode = HSSPI_OP_READ_WRITE;
+ else if (tx)
+ opcode = HSSPI_OP_WRITE;
+ else if (rx)
+ opcode = HSSPI_OP_READ;
+
+ if (opcode != HSSPI_OP_READ)
+ step_size -= HSSPI_OPCODE_LEN;
+
+ if ((opcode == HSSPI_OP_READ && t->rx_nbits == SPI_NBITS_DUAL) ||
+ (opcode == HSSPI_OP_WRITE && t->tx_nbits == SPI_NBITS_DUAL)) {
+ opcode |= HSSPI_OP_MULTIBIT;
+
+ if (t->rx_nbits == SPI_NBITS_DUAL)
+ reg |= 1 << MODE_CTRL_MULTIDATA_RD_SIZE_SHIFT;
+ if (t->tx_nbits == SPI_NBITS_DUAL)
+ reg |= 1 << MODE_CTRL_MULTIDATA_WR_SIZE_SHIFT;
+ }
+
+ __raw_writel(reg | 0xff,
+ bs->regs + HSSPI_PROFILE_MODE_CTRL_REG(chip_select));
+
+ while (pending > 0) {
+ int curr_step = min_t(int, step_size, pending);
+
+ reinit_completion(&bs->done);
+ if (tx) {
+ memcpy_toio(bs->fifo + HSSPI_OPCODE_LEN, tx, curr_step);
+ tx += curr_step;
+ }
+ __raw_writew((u16)cpu_to_be16(opcode | curr_step), bs->fifo);
+
+ /* enable interrupt */
+ if (bs->wait_mode == HSSPI_WAIT_MODE_INTR)
+ __raw_writel(HSSPI_PINGx_CMD_DONE(0),
+ bs->regs + HSSPI_INT_MASK_REG);
+
+ if (!cs_act) {
+ bcmbca_hsspi_set_cs(bs, chip_select, true);
+ cs_act = 1;
+ }
+ reg = chip_select << PINGPONG_CMD_SS_SHIFT |
+ chip_select << PINGPONG_CMD_PROFILE_SHIFT |
+ PINGPONG_COMMAND_START_NOW;
+ __raw_writel(reg, bs->regs + HSSPI_PINGPONG_COMMAND_REG(0));
+
+ if (bcmbca_hsspi_wait_cmd(bs, spi->chip_select))
+ return -ETIMEDOUT;
+
+ pending -= curr_step;
+ if (pending == 0) {
+ if (list_is_last(&t->transfer_list, &msg->transfers)) {
+ if (!t->cs_change)
+ bcmbca_hsspi_set_cs(bs, spi->chip_select, false);
+ } else {
+ if (t->cs_change) {
+ bcmbca_hsspi_set_cs(bs, spi->chip_select, false);
+ udelay(10);
+ bcmbca_hsspi_set_cs(bs, spi->chip_select, true);
+ }
+ }
+ }
+ if (rx) {
+ memcpy_fromio(rx, bs->fifo, curr_step);
+ rx += curr_step;
+ }
+ }
+
+ return 0;
+}
+
+static int bcmbca_hsspi_setup(struct spi_device *spi)
+{
+ struct bcmbca_hsspi *bs = spi_master_get_devdata(spi->master);
+ u32 reg;
+
+ reg = __raw_readl(bs->regs +
+ HSSPI_PROFILE_SIGNAL_CTRL_REG(spi->chip_select));
+ reg &= ~(SIGNAL_CTRL_LAUNCH_RISING | SIGNAL_CTRL_LATCH_RISING);
+ if (spi->mode & SPI_CPHA)
+ reg |= SIGNAL_CTRL_LAUNCH_RISING;
+ else
+ reg |= SIGNAL_CTRL_LATCH_RISING;
+ __raw_writel(reg, bs->regs +
+ HSSPI_PROFILE_SIGNAL_CTRL_REG(spi->chip_select));
+
+ mutex_lock(&bs->bus_mutex);
+ reg = __raw_readl(bs->regs + HSSPI_GLOBAL_CTRL_REG);
+
+ if (spi->mode & SPI_CS_HIGH)
+ reg |= BIT(spi->chip_select);
+ else
+ reg &= ~BIT(spi->chip_select);
+ __raw_writel(reg, bs->regs + HSSPI_GLOBAL_CTRL_REG);
+
+ if (spi->mode & SPI_CS_HIGH)
+ bs->cs_polarity |= BIT(spi->chip_select);
+ else
+ bs->cs_polarity &= ~BIT(spi->chip_select);
+
+ mutex_unlock(&bs->bus_mutex);
+
+ return 0;
+}
+
+static int bcmbca_hsspi_transfer_one(struct spi_master *master,
+ struct spi_message *msg)
+{
+ struct bcmbca_hsspi *bs = spi_master_get_devdata(master);
+ struct spi_transfer *t;
+ struct spi_device *spi = msg->spi;
+ int status = -EINVAL;
+
+ mutex_lock(&bs->msg_mutex);
+ list_for_each_entry(t, &msg->transfers, transfer_list) {
+ status = bcmbca_hsspi_do_txrx(spi, t, msg);
+ if (status)
+ break;
+
+ msg->actual_length += t->len;
+
+ spi_transfer_delay_exec(t);
+ }
+
+ mutex_unlock(&bs->msg_mutex);
+ msg->status = status;
+ spi_finalize_current_message(master);
+
+ return 0;
+}
+
+static irqreturn_t bcmbca_hsspi_interrupt(int irq, void *dev_id)
+{
+ struct bcmbca_hsspi *bs = (struct bcmbca_hsspi *)dev_id;
+
+ if (__raw_readl(bs->regs + HSSPI_INT_STATUS_MASKED_REG) == 0)
+ return IRQ_NONE;
+
+ __raw_writel(HSSPI_INT_CLEAR_ALL, bs->regs + HSSPI_INT_STATUS_REG);
+ __raw_writel(0, bs->regs + HSSPI_INT_MASK_REG);
+
+ complete(&bs->done);
+
+ return IRQ_HANDLED;
+}
+
+static int bcmbca_hsspi_probe(struct platform_device *pdev)
+{
+ struct spi_master *master;
+ struct bcmbca_hsspi *bs;
+ struct resource *res_mem;
+ void __iomem *spim_ctrl;
+ void __iomem *regs;
+ struct device *dev = &pdev->dev;
+ struct clk *clk, *pll_clk = NULL;
+ int irq, ret;
+ u32 reg, rate, num_cs = HSSPI_SPI_MAX_CS;
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0)
+ return irq;
+
+ res_mem = platform_get_resource_byname(pdev, IORESOURCE_MEM, "hsspi");
+ if (!res_mem)
+ return -EINVAL;
+ regs = devm_ioremap_resource(dev, res_mem);
+ if (IS_ERR(regs))
+ return PTR_ERR(regs);
+
+ res_mem = platform_get_resource_byname(pdev, IORESOURCE_MEM, "spim-ctrl");
+ if (!res_mem)
+ return -EINVAL;
+ spim_ctrl = devm_ioremap_resource(dev, res_mem);
+ if (IS_ERR(spim_ctrl))
+ return PTR_ERR(spim_ctrl);
+
+ clk = devm_clk_get(dev, "hsspi");
+ if (IS_ERR(clk))
+ return PTR_ERR(clk);
+
+ ret = clk_prepare_enable(clk);
+ if (ret)
+ return ret;
+
+ rate = clk_get_rate(clk);
+ if (!rate) {
+ pll_clk = devm_clk_get(dev, "pll");
+
+ if (IS_ERR(pll_clk)) {
+ ret = PTR_ERR(pll_clk);
+ goto out_disable_clk;
+ }
+
+ ret = clk_prepare_enable(pll_clk);
+ if (ret)
+ goto out_disable_clk;
+
+ rate = clk_get_rate(pll_clk);
+ if (!rate) {
+ ret = -EINVAL;
+ goto out_disable_pll_clk;
+ }
+ }
+
+ master = spi_alloc_master(&pdev->dev, sizeof(*bs));
+ if (!master) {
+ ret = -ENOMEM;
+ goto out_disable_pll_clk;
+ }
+
+ bs = spi_master_get_devdata(master);
+ bs->pdev = pdev;
+ bs->clk = clk;
+ bs->pll_clk = pll_clk;
+ bs->regs = regs;
+ bs->spim_ctrl = spim_ctrl;
+ bs->speed_hz = rate;
+ bs->fifo = (u8 __iomem *) (bs->regs + HSSPI_FIFO_REG(0));
+ bs->wait_mode = HSSPI_WAIT_MODE_POLLING;
+
+ mutex_init(&bs->bus_mutex);
+ mutex_init(&bs->msg_mutex);
+ init_completion(&bs->done);
+
+ master->dev.of_node = dev->of_node;
+ if (!dev->of_node)
+ master->bus_num = HSSPI_BUS_NUM;
+
+ of_property_read_u32(dev->of_node, "num-cs", &num_cs);
+ if (num_cs > 8) {
+ dev_warn(dev, "unsupported number of cs (%i), reducing to 8\n",
+ num_cs);
+ num_cs = HSSPI_SPI_MAX_CS;
+ }
+ master->num_chipselect = num_cs;
+ master->setup = bcmbca_hsspi_setup;
+ master->transfer_one_message = bcmbca_hsspi_transfer_one;
+ master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH |
+ SPI_RX_DUAL | SPI_TX_DUAL;
+ master->bits_per_word_mask = SPI_BPW_MASK(8);
+ master->auto_runtime_pm = true;
+
+ platform_set_drvdata(pdev, master);
+
+ /* Initialize the hardware */
+ __raw_writel(0, bs->regs + HSSPI_INT_MASK_REG);
+
+ /* clean up any pending interrupts */
+ __raw_writel(HSSPI_INT_CLEAR_ALL, bs->regs + HSSPI_INT_STATUS_REG);
+
+ /* read out default CS polarities */
+ reg = __raw_readl(bs->regs + HSSPI_GLOBAL_CTRL_REG);
+ bs->cs_polarity = reg & GLOBAL_CTRL_CS_POLARITY_MASK;
+ __raw_writel(reg | GLOBAL_CTRL_CLK_GATE_SSOFF,
+ bs->regs + HSSPI_GLOBAL_CTRL_REG);
+
+ if (irq > 0) {
+ ret = devm_request_irq(dev, irq, bcmbca_hsspi_interrupt, IRQF_SHARED,
+ pdev->name, bs);
+ if (ret)
+ goto out_put_master;
+ }
+
+ pm_runtime_enable(&pdev->dev);
+
+ if (sysfs_create_group(&pdev->dev.kobj, &bcmbca_hsspi_group)) {
+ dev_err(&pdev->dev, "couldn't register sysfs group\n");
+ goto out_pm_disable;
+ }
+
+ /* register and we are done */
+ ret = devm_spi_register_master(dev, master);
+ if (ret)
+ goto out_sysgroup_disable;
+
+ dev_info(dev, "Broadcom BCMBCA High Speed SPI Controller driver");
+
+ return 0;
+
+out_sysgroup_disable:
+ sysfs_remove_group(&pdev->dev.kobj, &bcmbca_hsspi_group);
+out_pm_disable:
+ pm_runtime_disable(&pdev->dev);
+out_put_master:
+ spi_master_put(master);
+out_disable_pll_clk:
+ clk_disable_unprepare(pll_clk);
+out_disable_clk:
+ clk_disable_unprepare(clk);
+ return ret;
+}
+
+static int bcmbca_hsspi_remove(struct platform_device *pdev)
+{
+ struct spi_master *master = platform_get_drvdata(pdev);
+ struct bcmbca_hsspi *bs = spi_master_get_devdata(master);
+
+ /* reset the hardware and block queue progress */
+ __raw_writel(0, bs->regs + HSSPI_INT_MASK_REG);
+ clk_disable_unprepare(bs->pll_clk);
+ clk_disable_unprepare(bs->clk);
+ sysfs_remove_group(&pdev->dev.kobj, &bcmbca_hsspi_group);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int bcmbca_hsspi_suspend(struct device *dev)
+{
+ struct spi_master *master = dev_get_drvdata(dev);
+ struct bcmbca_hsspi *bs = spi_master_get_devdata(master);
+
+ spi_master_suspend(master);
+ clk_disable_unprepare(bs->pll_clk);
+ clk_disable_unprepare(bs->clk);
+
+ return 0;
+}
+
+static int bcmbca_hsspi_resume(struct device *dev)
+{
+ struct spi_master *master = dev_get_drvdata(dev);
+ struct bcmbca_hsspi *bs = spi_master_get_devdata(master);
+ int ret;
+
+ ret = clk_prepare_enable(bs->clk);
+ if (ret)
+ return ret;
+
+ if (bs->pll_clk) {
+ ret = clk_prepare_enable(bs->pll_clk);
+ if (ret) {
+ clk_disable_unprepare(bs->clk);
+ return ret;
+ }
+ }
+
+ spi_master_resume(master);
+
+ return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(bcmbca_hsspi_pm_ops, bcmbca_hsspi_suspend,
+ bcmbca_hsspi_resume);
+
+static const struct of_device_id bcmbca_hsspi_of_match[] = {
+ { .compatible = "brcm,bcmbca-hsspi-v1.1", },
+ {},
+};
+
+MODULE_DEVICE_TABLE(of, bcmbca_hsspi_of_match);
+
+static struct platform_driver bcmbca_hsspi_driver = {
+ .driver = {
+ .name = "bcmbca-hsspi",
+ .pm = &bcmbca_hsspi_pm_ops,
+ .of_match_table = bcmbca_hsspi_of_match,
+ },
+ .probe = bcmbca_hsspi_probe,
+ .remove = bcmbca_hsspi_remove,
+};
+
+module_platform_driver(bcmbca_hsspi_driver);
+
+MODULE_ALIAS("platform:bcmbca_hsspi");
+MODULE_DESCRIPTION("Broadcom BCMBCA High Speed SPI Controller driver");
+MODULE_LICENSE("GPL");
--
2.37.3
The driver and device tree doc were originally authored by Jonas Gorski
and it has been updated from Broadcom recently including the dts yaml
file and a new driver for the updated controller. Add Jonas Gorski and
Broadcom engineers William Zhang and Kursad Oney as the maintainers.
Signed-off-by: William Zhang <[email protected]>
---
(no changes since v1)
MAINTAINERS | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 7f0b7181e60a..c7b1d4046940 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4299,6 +4299,18 @@ L: [email protected]
S: Maintained
F: drivers/phy/broadcom/phy-brcm-usb*
+BROADCOM Broadband SoC High Speed SPI Controller DRIVER
+M: William Zhang <[email protected]>
+M: Kursad Oney <[email protected]>
+M: Jonas Gorski <[email protected]>
+R: Broadcom internal kernel review list <[email protected]>
+L: [email protected]
+S: Maintained
+F: Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi-peripheral-props.yaml
+F: Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
+F: drivers/spi/spi-bcm63xx-hsspi.c
+F: drivers/spi/spi-bcmbca-hsspi.c
+
BROADCOM ETHERNET PHY DRIVERS
M: Florian Fainelli <[email protected]>
R: Broadcom internal kernel review list <[email protected]>
--
2.37.3
On 1/24/23 14:12, William Zhang wrote:
> HSSPI controller uses big endian for the opcode in the message to the
> controller ping pong buffer. Use cpu_to_be16 to properly handle the
> endianness for both big and little endian host.
>
> Fixes: 142168eba9dc ("spi: bcm63xx-hsspi: add bcm63xx HSSPI driver")
> Signed-off-by: Kursad Oney <[email protected]>
> Signed-off-by: William Zhang <[email protected]>
Acked-by: Florian Fainelli <[email protected]>
--
Florian
On 1/24/23 14:12, William Zhang wrote:
> The driver and device tree doc were originally authored by Jonas Gorski
> and it has been updated from Broadcom recently including the dts yaml
> file and a new driver for the updated controller. Add Jonas Gorski and
> Broadcom engineers William Zhang and Kursad Oney as the maintainers.
>
> Signed-off-by: William Zhang <[email protected]>
Acked-by: Florian Fainelli <[email protected]>
--
Florian
On 24/01/2023 23:12, 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 <[email protected]>
>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
On 24/01/2023 23:12, William Zhang wrote:
> The new Broadcom Broadband BCMBCA SoCs includes a updated HSSPI
> controller. Add new compatible strings to differentiate the old and new
> controller while keeping MIPS based chip with the old compatible. Update
> property requirements for these two revisions of the controller. Also
> add myself and Kursad as the maintainers.
>
> Signed-off-by: William Zhang <[email protected]>
>
> ---
>
> Changes in v2:
> - Update new compatible string to follow Broadcom convention <chip
> specific compatible>, <version of the IP>, <fallback>
> - Add reg-names min/maxItem constraints to be consistent with reg
> property
> - Make interrupts required property
> - Remove double quote from spi-controller.yaml reference
> - Remove brcm,use-cs-workaround flag
> - Update the example with new compatile and interrupts property
> - Update commit message
>
> .../bindings/spi/brcm,bcm63xx-hsspi.yaml | 106 +++++++++++++++++-
> 1 file changed, 101 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
> index d1a0c9adee7a..d39604654c9e 100644
> --- a/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
> +++ b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
> @@ -4,20 +4,73 @@
> $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:
> +
This is a friendly reminder during the review process.
It seems my previous comments were not fully addressed. Maybe my
feedback got lost between the quotes, maybe you just forgot to apply it.
Please go back to the previous discussion and either implement all
requested changes or keep discussing them.
Thank you.
> + - William Zhang <[email protected]>
> + - Kursad Oney <[email protected]>
> - Jonas Gorski <[email protected]>
>
> -allOf:
> - - $ref: spi-controller.yaml#
In your previous patch, put it already in desired place (after
"required:"), so you will not have to shuffle it.
> +description: |
> + Broadcom Broadband SoC supports High Speed SPI master controller since the
> + early MIPS based chips such as BCM6328 and BCM63268. This initial rev 1.0
> + controller was carried over to recent ARM based chips, such as BCM63138,
> + BCM4908 and BCM6858. The old MIPS based chip should continue to use the
> + brcm,bcm6328-hsspi compatible string. The recent ARM based chip is required to
> + use the brcm,bcmbca-hsspi-v1.0 as part of its compatible string list as
> + defined below to match the specific chip along with ip revision info.
> +
> + This rev 1.0 controller 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. Driver automatically picks the suitable mode based on
> + transfer type so it is transparent to the user.
> +
> + The newer SoCs such as BCM6756, BCM4912 and BCM6855 include an updated SPI
> + controller rev 1.1 that add the capability to allow the driver to control chip
> + select explicitly. This solves the issue in the old controller.
>
> properties:
> compatible:
> - const: brcm,bcm6328-hsspi
> + oneOf:
> + - const: brcm,bcm6328-hsspi
> + - items:
> + - enum:
> + - brcm,bcm47622-hsspi
> + - brcm,bcm4908-hsspi
> + - brcm,bcm63138-hsspi
> + - brcm,bcm63146-hsspi
> + - brcm,bcm63148-hsspi
> + - brcm,bcm63158-hsspi
> + - brcm,bcm63178-hsspi
> + - brcm,bcm6846-hsspi
> + - brcm,bcm6856-hsspi
> + - brcm,bcm6858-hsspi
> + - brcm,bcm6878-hsspi
> + - const: brcm,bcmbca-hsspi-v1.0
> + - const: brcm,bcmbca-hsspi
Why do you need "brcm,bcmbca-hsspi"? Nothing binds to it, so it's
useless and very generic.
> + - items:
> + - enum:
> + - brcm,bcm4912-hsspi
> + - brcm,bcm6756-hsspi
> + - brcm,bcm6813-hsspi
> + - brcm,bcm6855-hsspi
> + - const: brcm,bcmbca-hsspi-v1.1
> + - const: brcm,bcmbca-hsspi
Same here.
>
> reg:
> - maxItems: 1
> + items:
> + - description: main registers
> + - description: miscellaneous control registers
> + minItems: 1
> +
> + reg-names:
> + items:
> + - const: hsspi
> + - const: spim-ctrl
> + minItems: 1
>
> clocks:
> items:
> @@ -39,10 +92,39 @@ required:
> - clock-names
> - interrupts
>
> +allOf:
> + - $ref: spi-controller.yaml#
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - brcm,bcm6328-hsspi
> + - brcm,bcmbca-hsspi-v1.0
> + then:
> + properties:
> + reg:
> + minItems: 1
drop minItems
> + maxItems: 1
> + reg-names:
> + minItems: 1
drop minItems
> + maxItems: 1
> + else:
> + properties:
> + reg:
> + minItems: 2
> + maxItems: 2
> + reg-names:
> + minItems: 2
> + maxItems: 2
> + required:
> + - reg-names
> +
> unevaluatedProperties: false
>
> examples:
> - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> spi@10001000 {
> compatible = "brcm,bcm6328-hsspi";
> reg = <0x10001000 0x600>;
> @@ -53,3 +135,17 @@ examples:
> #address-cells = <1>;
> #size-cells = <0>;
> };
> + - |
> + spi@ff801000 {
> + compatible = "brcm,bcm6756-hsspi", "brcm,bcmbca-hsspi-v1.1",
> + "brcm,bcmbca-hsspi";
> + reg = <0xff801000 0x1000>,
> + <0xff802610 0x4>;
> + reg-names = "hsspi", "spim-ctrl";
> + interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&hsspi>, <&hsspi_pll>;
> + clock-names = "hsspi", "pll";
> + num-cs = <8>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
Drop new example - the difference is only in reg. Or change old example
to have only one (newer, more complex).
Best regards,
Krzysztof
On 24/01/2023 23:12, William Zhang wrote:
> Add support for HSSPI controller in ARMv7 chip dts files.
>
> Signed-off-by: William Zhang <[email protected]>
>
> ---
>
> Changes in v2:
> - Update compatible string with SoC model number, controller version
> info and bcmbca fall back name
> - Add interrupt property
>
> arch/arm/boot/dts/bcm47622.dtsi | 19 +++++++++++++++++++
> arch/arm/boot/dts/bcm63138.dtsi | 19 +++++++++++++++++++
> arch/arm/boot/dts/bcm63148.dtsi | 19 +++++++++++++++++++
> arch/arm/boot/dts/bcm63178.dtsi | 20 ++++++++++++++++++++
> arch/arm/boot/dts/bcm6756.dtsi | 20 ++++++++++++++++++++
> arch/arm/boot/dts/bcm6846.dtsi | 19 +++++++++++++++++++
> arch/arm/boot/dts/bcm6855.dtsi | 20 ++++++++++++++++++++
> arch/arm/boot/dts/bcm6878.dtsi | 20 ++++++++++++++++++++
> 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, 192 insertions(+)
>
> diff --git a/arch/arm/boot/dts/bcm47622.dtsi b/arch/arm/boot/dts/bcm47622.dtsi
> index f4b2db9bc4ab..6de4a2287c90 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,19 @@ bus@ff800000 {
> #size-cells = <1>;
> ranges = <0 0xff800000 0x800000>;
>
> + hsspi: spi@1000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "brcm,bcm47622-hsspi", "brcm,bcmbca-hsspi-v1.0",
> + "brcm,bcmbca-hsspi";
Several of your lines are not properly indented/aligned.
Best regards,
Krzysztof
On 01/24/2023 11:35 PM, Krzysztof Kozlowski wrote:
> On 24/01/2023 23:12, William Zhang wrote:
>> The new Broadcom Broadband BCMBCA SoCs includes a updated HSSPI
>> controller. Add new compatible strings to differentiate the old and new
>> controller while keeping MIPS based chip with the old compatible. Update
>> property requirements for these two revisions of the controller. Also
>> add myself and Kursad as the maintainers.
>>
>> Signed-off-by: William Zhang <[email protected]>
>>
>> ---
>>
>> Changes in v2:
>> - Update new compatible string to follow Broadcom convention <chip
>> specific compatible>, <version of the IP>, <fallback>
>> - Add reg-names min/maxItem constraints to be consistent with reg
>> property
>> - Make interrupts required property
>> - Remove double quote from spi-controller.yaml reference
>> - Remove brcm,use-cs-workaround flag
>> - Update the example with new compatile and interrupts property
>> - Update commit message
>>
>> .../bindings/spi/brcm,bcm63xx-hsspi.yaml | 106 +++++++++++++++++-
>> 1 file changed, 101 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
>> index d1a0c9adee7a..d39604654c9e 100644
>> --- a/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
>> +++ b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
>> @@ -4,20 +4,73 @@
>> $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:
>> +
>
> This is a friendly reminder during the review process.
>
> It seems my previous comments were not fully addressed. Maybe my
> feedback got lost between the quotes, maybe you just forgot to apply it.
> Please go back to the previous discussion and either implement all
> requested changes or keep discussing them.
>
> Thank you.
>
Yeah I forgot to remove the blank line after maintainers tag. Also
regarding the explanation of dummy cs workaround flag, we decided to
remove it as it is not necessary after discussion internally and with
SPI maintainer Mark. Let me know if I missed anything else.
>> + - William Zhang <[email protected]>
>> + - Kursad Oney <[email protected]>
>> - Jonas Gorski <[email protected]>
>>
>> -allOf:
>> - - $ref: spi-controller.yaml#
>
> In your previous patch, put it already in desired place (after
> "required:"), so you will not have to shuffle it.
>
Will update the previous patch in v3
>> +description: |
>> + Broadcom Broadband SoC supports High Speed SPI master controller since the
>> + early MIPS based chips such as BCM6328 and BCM63268. This initial rev 1.0
>> + controller was carried over to recent ARM based chips, such as BCM63138,
>> + BCM4908 and BCM6858. The old MIPS based chip should continue to use the
>> + brcm,bcm6328-hsspi compatible string. The recent ARM based chip is required to
>> + use the brcm,bcmbca-hsspi-v1.0 as part of its compatible string list as
>> + defined below to match the specific chip along with ip revision info.
>> +
>> + This rev 1.0 controller 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. Driver automatically picks the suitable mode based on
>> + transfer type so it is transparent to the user.
>> +
>> + The newer SoCs such as BCM6756, BCM4912 and BCM6855 include an updated SPI
>> + controller rev 1.1 that add the capability to allow the driver to control chip
>> + select explicitly. This solves the issue in the old controller.
>>
>> properties:
>> compatible:
>> - const: brcm,bcm6328-hsspi
>> + oneOf:
>> + - const: brcm,bcm6328-hsspi
>> + - items:
>> + - enum:
>> + - brcm,bcm47622-hsspi
>> + - brcm,bcm4908-hsspi
>> + - brcm,bcm63138-hsspi
>> + - brcm,bcm63146-hsspi
>> + - brcm,bcm63148-hsspi
>> + - brcm,bcm63158-hsspi
>> + - brcm,bcm63178-hsspi
>> + - brcm,bcm6846-hsspi
>> + - brcm,bcm6856-hsspi
>> + - brcm,bcm6858-hsspi
>> + - brcm,bcm6878-hsspi
>> + - const: brcm,bcmbca-hsspi-v1.0
>> + - const: brcm,bcmbca-hsspi
>
> Why do you need "brcm,bcmbca-hsspi"? Nothing binds to it, so it's
> useless and very generic.
>
This was from Florian's suggestion and Broadcom's convention. See [1]
and you are okay with that [2]. I added the rev compatible and you were
not objecting it finally if I understand you correctly.
>> + - items:
>> + - enum:
>> + - brcm,bcm4912-hsspi
>> + - brcm,bcm6756-hsspi
>> + - brcm,bcm6813-hsspi
>> + - brcm,bcm6855-hsspi
>> + - const: brcm,bcmbca-hsspi-v1.1
>> + - const: brcm,bcmbca-hsspi
>
> Same here.
>
>>
>> reg:
>> - maxItems: 1
>> + items:
>> + - description: main registers
>> + - description: miscellaneous control registers
>> + minItems: 1
>> +
>> + reg-names:
>> + items:
>> + - const: hsspi
>> + - const: spim-ctrl
>> + minItems: 1
>>
>> clocks:
>> items:
>> @@ -39,10 +92,39 @@ required:
>> - clock-names
>> - interrupts
>>
>> +allOf:
>> + - $ref: spi-controller.yaml#
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + enum:
>> + - brcm,bcm6328-hsspi
>> + - brcm,bcmbca-hsspi-v1.0
>> + then:
>> + properties:
>> + reg:
>> + minItems: 1
>
> drop minItems
>
Will fix in v3 and the next one as well
>> + maxItems: 1
>> + reg-names:
>> + minItems: 1
>
> drop minItems
>
>> + maxItems: 1
>> + else:
>> + properties:
>> + reg:
>> + minItems: 2
>> + maxItems: 2
>> + reg-names:
>> + minItems: 2
>> + maxItems: 2
>> + required:
>> + - reg-names
>> +
>> unevaluatedProperties: false
>>
>> examples:
>> - |
>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>> spi@10001000 {
>> compatible = "brcm,bcm6328-hsspi";
>> reg = <0x10001000 0x600>;
>> @@ -53,3 +135,17 @@ examples:
>> #address-cells = <1>;
>> #size-cells = <0>;
>> };
>> + - |
>> + spi@ff801000 {
>> + compatible = "brcm,bcm6756-hsspi", "brcm,bcmbca-hsspi-v1.1",
>> + "brcm,bcmbca-hsspi";
>> + reg = <0xff801000 0x1000>,
>> + <0xff802610 0x4>;
>> + reg-names = "hsspi", "spim-ctrl";
>> + interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
>> + clocks = <&hsspi>, <&hsspi_pll>;
>> + clock-names = "hsspi", "pll";
>> + num-cs = <8>;
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + };
>
> Drop new example - the difference is only in reg. Or change old example
> to have only one (newer, more complex).
>
Will replace the old example with this more recent and complex example in v3
> Best regards,
> Krzysztof
>
[1] https://www.spinics.net/lists/devicetree/msg565016.html
[2] https://www.spinics.net/lists/devicetree/msg565197.html
On Wed, Jan 25, 2023 at 11:23:52AM -0800, William Zhang wrote:
>
>
> On 01/24/2023 11:35 PM, Krzysztof Kozlowski wrote:
> > On 24/01/2023 23:12, William Zhang wrote:
> > > The new Broadcom Broadband BCMBCA SoCs includes a updated HSSPI
> > > controller. Add new compatible strings to differentiate the old and new
> > > controller while keeping MIPS based chip with the old compatible. Update
> > > property requirements for these two revisions of the controller. Also
> > > add myself and Kursad as the maintainers.
> > >
> > > Signed-off-by: William Zhang <[email protected]>
> > >
> > > ---
> > >
> > > Changes in v2:
> > > - Update new compatible string to follow Broadcom convention <chip
> > > specific compatible>, <version of the IP>, <fallback>
> > > - Add reg-names min/maxItem constraints to be consistent with reg
> > > property
> > > - Make interrupts required property
> > > - Remove double quote from spi-controller.yaml reference
> > > - Remove brcm,use-cs-workaround flag
> > > - Update the example with new compatile and interrupts property
> > > - Update commit message
> > >
> > > .../bindings/spi/brcm,bcm63xx-hsspi.yaml | 106 +++++++++++++++++-
> > > 1 file changed, 101 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
> > > index d1a0c9adee7a..d39604654c9e 100644
> > > --- a/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
> > > +++ b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
> > > @@ -4,20 +4,73 @@
> > > $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:
> > > +
> >
> > This is a friendly reminder during the review process.
> >
> > It seems my previous comments were not fully addressed. Maybe my
> > feedback got lost between the quotes, maybe you just forgot to apply it.
> > Please go back to the previous discussion and either implement all
> > requested changes or keep discussing them.
> >
> > Thank you.
> >
> Yeah I forgot to remove the blank line after maintainers tag. Also
> regarding the explanation of dummy cs workaround flag, we decided to remove
> it as it is not necessary after discussion internally and with SPI
> maintainer Mark. Let me know if I missed anything else.
>
> > > + - William Zhang <[email protected]>
> > > + - Kursad Oney <[email protected]>
> > > - Jonas Gorski <[email protected]>
> > > -allOf:
> > > - - $ref: spi-controller.yaml#
> >
> > In your previous patch, put it already in desired place (after
> > "required:"), so you will not have to shuffle it.
> >
> Will update the previous patch in v3
>
> > > +description: |
> > > + Broadcom Broadband SoC supports High Speed SPI master controller since the
> > > + early MIPS based chips such as BCM6328 and BCM63268. This initial rev 1.0
> > > + controller was carried over to recent ARM based chips, such as BCM63138,
> > > + BCM4908 and BCM6858. The old MIPS based chip should continue to use the
> > > + brcm,bcm6328-hsspi compatible string. The recent ARM based chip is required to
> > > + use the brcm,bcmbca-hsspi-v1.0 as part of its compatible string list as
> > > + defined below to match the specific chip along with ip revision info.
> > > +
> > > + This rev 1.0 controller 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. Driver automatically picks the suitable mode based on
> > > + transfer type so it is transparent to the user.
> > > +
> > > + The newer SoCs such as BCM6756, BCM4912 and BCM6855 include an updated SPI
> > > + controller rev 1.1 that add the capability to allow the driver to control chip
> > > + select explicitly. This solves the issue in the old controller.
> > > properties:
> > > compatible:
> > > - const: brcm,bcm6328-hsspi
> > > + oneOf:
> > > + - const: brcm,bcm6328-hsspi
> > > + - items:
> > > + - enum:
> > > + - brcm,bcm47622-hsspi
> > > + - brcm,bcm4908-hsspi
> > > + - brcm,bcm63138-hsspi
> > > + - brcm,bcm63146-hsspi
> > > + - brcm,bcm63148-hsspi
> > > + - brcm,bcm63158-hsspi
> > > + - brcm,bcm63178-hsspi
> > > + - brcm,bcm6846-hsspi
> > > + - brcm,bcm6856-hsspi
> > > + - brcm,bcm6858-hsspi
> > > + - brcm,bcm6878-hsspi
> > > + - const: brcm,bcmbca-hsspi-v1.0
> > > + - const: brcm,bcmbca-hsspi
> >
> > Why do you need "brcm,bcmbca-hsspi"? Nothing binds to it, so it's
> > useless and very generic.
> >
> This was from Florian's suggestion and Broadcom's convention. See [1] and
> you are okay with that [2]. I added the rev compatible and you were not
> objecting it finally if I understand you correctly.
Can you have a driver that only understands what 'brcm,bcmbca-hsspi' is
work on all h/w that includes the compatible string? It doesn't seem
like it since v1.1 is a completely new driver. Therefore
'brcm,bcmbca-hsspi' is pretty much useless.
Really, your 'generic' fallback for v1.0 should be 'brcm,bcm6328-hsspi'
because that is the one the OS already supports. I don't know why folks
get so hung up on saying new SoC block is compatible with old SoC's
block.
The rule on using version numbers is they must correspond to something.
FPGA soft IP with released versions is a good example. I always suspect
a v1 or v1.0 is made up by the binding author.
Rob
On 01/25/2023 12:51 PM, Rob Herring wrote:
> On Wed, Jan 25, 2023 at 11:23:52AM -0800, William Zhang wrote:
>>
>>
>> On 01/24/2023 11:35 PM, Krzysztof Kozlowski wrote:
>>> On 24/01/2023 23:12, William Zhang wrote:
>>>> The new Broadcom Broadband BCMBCA SoCs includes a updated HSSPI
>>>> controller. Add new compatible strings to differentiate the old and new
>>>> controller while keeping MIPS based chip with the old compatible. Update
>>>> property requirements for these two revisions of the controller. Also
>>>> add myself and Kursad as the maintainers.
>>>>
>>>> Signed-off-by: William Zhang <[email protected]>
>>>>
>>>> ---
>>>>
>>>> Changes in v2:
>>>> - Update new compatible string to follow Broadcom convention <chip
>>>> specific compatible>, <version of the IP>, <fallback>
>>>> - Add reg-names min/maxItem constraints to be consistent with reg
>>>> property
>>>> - Make interrupts required property
>>>> - Remove double quote from spi-controller.yaml reference
>>>> - Remove brcm,use-cs-workaround flag
>>>> - Update the example with new compatile and interrupts property
>>>> - Update commit message
>>>>
>>>> .../bindings/spi/brcm,bcm63xx-hsspi.yaml | 106 +++++++++++++++++-
>>>> 1 file changed, 101 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
>>>> index d1a0c9adee7a..d39604654c9e 100644
>>>> --- a/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
>>>> +++ b/Documentation/devicetree/bindings/spi/brcm,bcm63xx-hsspi.yaml
>>>> @@ -4,20 +4,73 @@
>>>> $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:
>>>> +
>>>
>>> This is a friendly reminder during the review process.
>>>
>>> It seems my previous comments were not fully addressed. Maybe my
>>> feedback got lost between the quotes, maybe you just forgot to apply it.
>>> Please go back to the previous discussion and either implement all
>>> requested changes or keep discussing them.
>>>
>>> Thank you.
>>>
>> Yeah I forgot to remove the blank line after maintainers tag. Also
>> regarding the explanation of dummy cs workaround flag, we decided to remove
>> it as it is not necessary after discussion internally and with SPI
>> maintainer Mark. Let me know if I missed anything else.
>>
>>>> + - William Zhang <[email protected]>
>>>> + - Kursad Oney <[email protected]>
>>>> - Jonas Gorski <[email protected]>
>>>> -allOf:
>>>> - - $ref: spi-controller.yaml#
>>>
>>> In your previous patch, put it already in desired place (after
>>> "required:"), so you will not have to shuffle it.
>>>
>> Will update the previous patch in v3
>>
>>>> +description: |
>>>> + Broadcom Broadband SoC supports High Speed SPI master controller since the
>>>> + early MIPS based chips such as BCM6328 and BCM63268. This initial rev 1.0
>>>> + controller was carried over to recent ARM based chips, such as BCM63138,
>>>> + BCM4908 and BCM6858. The old MIPS based chip should continue to use the
>>>> + brcm,bcm6328-hsspi compatible string. The recent ARM based chip is required to
>>>> + use the brcm,bcmbca-hsspi-v1.0 as part of its compatible string list as
>>>> + defined below to match the specific chip along with ip revision info.
>>>> +
>>>> + This rev 1.0 controller 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. Driver automatically picks the suitable mode based on
>>>> + transfer type so it is transparent to the user.
>>>> +
>>>> + The newer SoCs such as BCM6756, BCM4912 and BCM6855 include an updated SPI
>>>> + controller rev 1.1 that add the capability to allow the driver to control chip
>>>> + select explicitly. This solves the issue in the old controller.
>>>> properties:
>>>> compatible:
>>>> - const: brcm,bcm6328-hsspi
>>>> + oneOf:
>>>> + - const: brcm,bcm6328-hsspi
>>>> + - items:
>>>> + - enum:
>>>> + - brcm,bcm47622-hsspi
>>>> + - brcm,bcm4908-hsspi
>>>> + - brcm,bcm63138-hsspi
>>>> + - brcm,bcm63146-hsspi
>>>> + - brcm,bcm63148-hsspi
>>>> + - brcm,bcm63158-hsspi
>>>> + - brcm,bcm63178-hsspi
>>>> + - brcm,bcm6846-hsspi
>>>> + - brcm,bcm6856-hsspi
>>>> + - brcm,bcm6858-hsspi
>>>> + - brcm,bcm6878-hsspi
>>>> + - const: brcm,bcmbca-hsspi-v1.0
>>>> + - const: brcm,bcmbca-hsspi
>>>
>>> Why do you need "brcm,bcmbca-hsspi"? Nothing binds to it, so it's
>>> useless and very generic.
>>>
>> This was from Florian's suggestion and Broadcom's convention. See [1] and
>> you are okay with that [2]. I added the rev compatible and you were not
>> objecting it finally if I understand you correctly.
>
> Can you have a driver that only understands what 'brcm,bcmbca-hsspi' is
> work on all h/w that includes the compatible string? It doesn't seem
> like it since v1.1 is a completely new driver. Therefore
> 'brcm,bcmbca-hsspi' is pretty much useless.
>
'brcm,bcmbca-hsspi' should be added to the binding table of
spi-bcm63xx-hsspi.c driver. This is the initial driver that works for
v1.0 controller. For v1.1 controller, yes it can fallback and work with
1.0 driver spi-bcm63xx-hsspi.c simply not using the new feature in
v1.1(chip select signal control through software) and keeping using the
prepend mode or dummy cs workaround supported in 1.0 driver.
> Really, your 'generic' fallback for v1.0 should be 'brcm,bcm6328-hsspi'
> because that is the one the OS already supports. I don't know why folks
> get so hung up on saying new SoC block is compatible with old SoC's
> block.
>
> The rule on using version numbers is they must correspond to something.
> FPGA soft IP with released versions is a good example. I always suspect
> a v1 or v1.0 is made up by the binding author.
>
I had discussion with our ASIC IP team and confirmed we only have two
versions of HSSPI controller among all the BCA chips. Although it does
not have version register in the IP block for now, version 1.0 and 1.1
is what we agreed on and ASIC team was already planning to add rev
register for the new rev of this IP. So I agree it is not perfect like
brcm nand controller and other block that always have rev register but I
think it is good use for software to identify them in dts based on the
fact there are only two revisions.
> Rob
>
On Wed, Jan 25, 2023 at 3:41 PM William Zhang
<[email protected]> wrote:
> On 01/25/2023 12:51 PM, Rob Herring wrote:
> > On Wed, Jan 25, 2023 at 11:23:52AM -0800, William Zhang wrote:
> >> On 01/24/2023 11:35 PM, Krzysztof Kozlowski wrote:
> >>> On 24/01/2023 23:12, William Zhang wrote:
> >>>> The new Broadcom Broadband BCMBCA SoCs includes a updated HSSPI
> >>>> controller. Add new compatible strings to differentiate the old and new
> >>>> controller while keeping MIPS based chip with the old compatible. Update
> >>>> property requirements for these two revisions of the controller. Also
> >>>> add myself and Kursad as the maintainers.
[...]
> >>>> properties:
> >>>> compatible:
> >>>> - const: brcm,bcm6328-hsspi
> >>>> + oneOf:
> >>>> + - const: brcm,bcm6328-hsspi
> >>>> + - items:
> >>>> + - enum:
> >>>> + - brcm,bcm47622-hsspi
> >>>> + - brcm,bcm4908-hsspi
> >>>> + - brcm,bcm63138-hsspi
> >>>> + - brcm,bcm63146-hsspi
> >>>> + - brcm,bcm63148-hsspi
> >>>> + - brcm,bcm63158-hsspi
> >>>> + - brcm,bcm63178-hsspi
> >>>> + - brcm,bcm6846-hsspi
> >>>> + - brcm,bcm6856-hsspi
> >>>> + - brcm,bcm6858-hsspi
> >>>> + - brcm,bcm6878-hsspi
> >>>> + - const: brcm,bcmbca-hsspi-v1.0
> >>>> + - const: brcm,bcmbca-hsspi
> >>>
> >>> Why do you need "brcm,bcmbca-hsspi"? Nothing binds to it, so it's
> >>> useless and very generic.
> >>>
> >> This was from Florian's suggestion and Broadcom's convention. See [1] and
> >> you are okay with that [2]. I added the rev compatible and you were not
> >> objecting it finally if I understand you correctly.
> >
> > Can you have a driver that only understands what 'brcm,bcmbca-hsspi' is
> > work on all h/w that includes the compatible string? It doesn't seem
> > like it since v1.1 is a completely new driver. Therefore
> > 'brcm,bcmbca-hsspi' is pretty much useless.
> >
> 'brcm,bcmbca-hsspi' should be added to the binding table of
> spi-bcm63xx-hsspi.c driver. This is the initial driver that works for
> v1.0 controller. For v1.1 controller, yes it can fallback and work with
> 1.0 driver spi-bcm63xx-hsspi.c simply not using the new feature in
> v1.1(chip select signal control through software) and keeping using the
> prepend mode or dummy cs workaround supported in 1.0 driver.
If v1.1 is compatible with v1.0, then say that:
soc-compat, "brcm,bcmbca-hsspi-v1.1", "brcm,bcmbca-hsspi-v1.0"
IOW, 'brcm,bcmbca-hsspi' is redundant with 'brcm,bcmbca-hsspi-v1.0'.
They have the same meaning. So pick which one you want to use. Not
both.
Also, if that is the case, you shouldn't be introducing a whole new
driver for v1.1.
Rob
On Tue, 24 Jan 2023 at 23:33, William Zhang <[email protected]> wrote:
>
> The kernel SPI interface includes the cs_change flag that alters how
> the CS behaves.
>
> If we're in the middle of transfers, it tells us to unselect the
> CS momentarily since the target device requires that.
>
> If we're at the end of a transfer, it tells us to keep the CS
> selected, perhaps because the next transfer is likely targeted
> to the same device.
>
> We implement this scheme in the HSSPI driver in this change.
>
> Prior to this change, the CS would toggle momentarily if cs_change
> was set for the last transfer. This can be ignored by some or
> most devices, but the Microchip TPM2 device does not ignore it.
>
> With the change, the behavior is corrected and the 'glitch' is
> eliminated.
>
> Signed-off-by: Kursad Oney <[email protected]>
> Signed-off-by: William Zhang <[email protected]>
>
> ---
>
> Changes in v2:
> - Fix unused variable ‘reg’ compile warning
>
> drivers/spi/spi-bcm63xx-hsspi.c | 29 +++++++++++++++++++++--------
> 1 file changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/spi/spi-bcm63xx-hsspi.c b/drivers/spi/spi-bcm63xx-hsspi.c
> index 55cbe7deba08..696e14abba2d 100644
> --- a/drivers/spi/spi-bcm63xx-hsspi.c
> +++ b/drivers/spi/spi-bcm63xx-hsspi.c
> @@ -338,7 +338,7 @@ static int bcm63xx_hsspi_transfer_one(struct spi_master *master,
> struct spi_device *spi = msg->spi;
> int status = -EINVAL;
> int dummy_cs;
> - u32 reg;
> + bool restore_polarity = true;
While restore polarity is how this is implemented, I think using a
more semantic name like keep_cs would be better.
>
> mutex_lock(&bs->msg_mutex);
> /* This controller does not support keeping CS active during idle.
> @@ -367,16 +367,29 @@ static int bcm63xx_hsspi_transfer_one(struct spi_master *master,
>
> spi_transfer_delay_exec(t);
>
> - if (t->cs_change)
> + /*
> + * cs_change rules:
> + * (1) cs_change = 0 && last_xfer = 0:
> + * Do not touch the CS. On to the next xfer.
> + * (2) cs_change = 1 && last_xfer = 0:
> + * Set cs = false before the next xfer.
> + * (3) cs_change = 0 && last_xfer = 1:
> + * We want CS to be deactivated. So do NOT set cs = false,
> + * instead just restore the original polarity. This has the
> + * same effect of deactivating the CS.
> + * (4) cs_change = 1 && last_xfer = 1:
> + * We want to keep CS active. So do NOT set cs = false, and
> + * make sure we do NOT reverse polarity.
> + */
> + if (t->cs_change && !list_is_last(&t->transfer_list, &msg->transfers))
> bcm63xx_hsspi_set_cs(bs, spi->chip_select, false);
> +
> + restore_polarity = !t->cs_change;
> }
I still find setting restore_polarity on each loop iteration when only
its last set value matters confusing and hard to read, so I still
propose keeping close to the generic implementation (
https://elixir.bootlin.com/linux/v6.1.8/source/drivers/spi/spi.c#L1560
) and do
if (t->cs_change) {
if (list_is_last())
restore_polarity = false;
else
bcm63xx_hsspi_set_cs(bs, spi->chip_select, false);
}
While there, you might also want to check the cs_off value(s) as well.
>
> - mutex_lock(&bs->bus_mutex);
> - reg = __raw_readl(bs->regs + HSSPI_GLOBAL_CTRL_REG);
> - reg &= ~GLOBAL_CTRL_CS_POLARITY_MASK;
> - reg |= bs->cs_polarity;
> - __raw_writel(reg, bs->regs + HSSPI_GLOBAL_CTRL_REG);
> - mutex_unlock(&bs->bus_mutex);
> + bcm63xx_hsspi_set_cs(bs, dummy_cs, false);
> + if (restore_polarity)
> + bcm63xx_hsspi_set_cs(bs, spi->chip_select, false);
>
> mutex_unlock(&bs->msg_mutex);
> msg->status = status;
> --
> 2.37.3
>
On Tue, 24 Jan 2023 at 23:33, William Zhang <[email protected]> wrote:
>
> Due to the controller limitation to keep the chip select low during the
> bus idle time between the transfer, a dummy cs workaround was used when
> this driver was first upstreamed to the kernel. It basically picks the
> dummy cs as !actual_cs so typically dummy cs is 1 when most of the case
> only cs 0 is used in the board design. Then invert the polarity of both
> cs and tell the controller to start the transfers using dummy cs.
> Assuming both cs are active low before the inversion, effectively this
> keeps dummy cs high and actual cs low during the transfer and workaround
> the issue.
>
> This workaround implies that dummy cs 1 pin has to be set to chip
> selection function in the pinmux when the transfer clock is above
> 25MHz. The old chips likely have default pinmux set to chip select on
> the dummy cs pin so it works but this is not case for the new Broadband
> BCA chips and this workaround stop working. This is specifically an
> issue to support SPI NAND and SPI NOR flash because these flash devices
> can typically run at or above 100MHz.
>
> This patch utilizes the prepend feature of the controller to combine the
> multiple transfers in the same message to a single transfer when
> possible. This way there is no need to keep clock low between transfers
> and solve the issue without any hardware requirement.
>
> Multiple transfers within a SPI message may be combined into one
> transfer if the following are all true:
> * One or more half duplex write transfer in single bit mode
> * Optional full duplex read/write at the end
> * No delay and cs_change between transfers
>
> Most of the SPI device meets this requirements such as SPI NOR,
> SPI NAND flash, Broadcom SPI voice card and etc. For any SPI message
> that does not meet the above requirement to combine the transfers, we
> switch to original dummy cs mode but limit the clock rate to the safe
> 25MHz. This is the default auto transfer mode and it makes sure all the
> SPI message can be supported automatically under the hood.
>
> This patch also adds the driver sysfs node xfer_mode to provide
> the option for overriding the default auto mode and force it to dummy cs
> or prepend mode.
>
> Signed-off-by: William Zhang <[email protected]>
>
> ---
>
> Changes in v2:
> - Fix build error for Alpha platform
> Reported-by: kernel test robot <[email protected]>
> - Remove use_cs_workaround option from device tree
> - Change the transfer logic to use try prepend first and if not
> prependable, switch to dummy cs mode with clock limit at the 25MHz
> - Add driver sysfs node xfer_mode for the option to override the
> transfer mode to dummy cs or prepend mode.
> - Add number of bits check in the tranfer for prepend mode eligibility
> check
> - Update commit message
>
> drivers/spi/spi-bcm63xx-hsspi.c | 332 +++++++++++++++++++++++++++++---
> 1 file changed, 300 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/spi/spi-bcm63xx-hsspi.c b/drivers/spi/spi-bcm63xx-hsspi.c
> index 8f0d31764f98..2a0bef943967 100644
> --- a/drivers/spi/spi-bcm63xx-hsspi.c
> +++ b/drivers/spi/spi-bcm63xx-hsspi.c
> @@ -93,7 +93,11 @@
>
> #define HSSPI_MAX_PREPEND_LEN 15
>
> -#define HSSPI_MAX_SYNC_CLOCK 30000000
> +/*
> + * Some chip require 30MHz but other require 25MHz. Use smaller value to cover
> + * both cases.
> + */
> +#define HSSPI_MAX_SYNC_CLOCK 25000000
>
> #define HSSPI_SPI_MAX_CS 8
> #define HSSPI_BUS_NUM 1 /* 0 is legacy SPI */
> @@ -103,6 +107,16 @@
> #define HSSPI_WAIT_MODE_INTR 1
> #define HSSPI_WAIT_MODE_MAX HSSPI_WAIT_MODE_INTR
>
> +/*
> + * Default transfer mode is auto. If the msg is prependable, use the prepend
> + * mode. If not, falls back to use the dummy cs workaround mode but limit the
> + * clock to 25MHz to make sure it works in all board design.
> + */
> +#define HSSPI_XFER_MODE_AUTO 0
> +#define HSSPI_XFER_MODE_PREPEND 1
> +#define HSSPI_XFER_MODE_DUMMYCS 2
> +#define HSSPI_XFER_MODE_MAX HSSPI_XFER_MODE_DUMMYCS
> +
> struct bcm63xx_hsspi {
> struct completion done;
> struct mutex bus_mutex;
> @@ -116,6 +130,9 @@ struct bcm63xx_hsspi {
> u32 speed_hz;
> u8 cs_polarity;
> u32 wait_mode;
> + u32 xfer_mode;
> + u32 prepend_cnt;
> + u8 *prepend_buf;
> };
>
> static ssize_t wait_mode_show(struct device *dev, struct device_attribute *attr,
> @@ -154,8 +171,42 @@ static ssize_t wait_mode_store(struct device *dev, struct device_attribute *attr
>
> static DEVICE_ATTR_RW(wait_mode);
>
> +static ssize_t xfer_mode_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct spi_controller *ctrl = dev_get_drvdata(dev);
> + struct bcm63xx_hsspi *bs = spi_master_get_devdata(ctrl);
> +
> + return sprintf(buf, "%d\n", bs->xfer_mode);
> +}
> +
> +static ssize_t xfer_mode_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct spi_controller *ctrl = dev_get_drvdata(dev);
> + struct bcm63xx_hsspi *bs = spi_master_get_devdata(ctrl);
> + u32 val;
> +
> + if (kstrtou32(buf, 10, &val))
> + return -EINVAL;
> +
> + if (val > HSSPI_XFER_MODE_MAX) {
> + dev_warn(dev, "invalid xfer mode %u\n", val);
> + return -EINVAL;
> + }
> +
> + mutex_lock(&bs->msg_mutex);
> + bs->xfer_mode = val;
> + mutex_unlock(&bs->msg_mutex);
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR_RW(xfer_mode);
> +
> static struct attribute *bcm63xx_hsspi_attrs[] = {
> &dev_attr_wait_mode.attr,
> + &dev_attr_xfer_mode.attr,
> NULL,
> };
>
> @@ -163,6 +214,208 @@ static const struct attribute_group bcm63xx_hsspi_group = {
> .attrs = bcm63xx_hsspi_attrs,
> };
>
> +static void bcm63xx_hsspi_set_clk(struct bcm63xx_hsspi *bs,
> + struct spi_device *spi, int hz);
> +
> +static size_t bcm63xx_hsspi_max_message_size(struct spi_device *spi)
> +{
> + return HSSPI_BUFFER_LEN - HSSPI_OPCODE_LEN;
> +}
> +
> +static int bcm63xx_hsspi_wait_cmd(struct bcm63xx_hsspi *bs)
> +{
> + unsigned long limit;
> + u32 reg = 0;
> + int rc = 0;
If the only possible return values are 0 and 1, maybe this should be a bool?
> +
> + if (bs->wait_mode == HSSPI_WAIT_MODE_INTR) {
> + if (wait_for_completion_timeout(&bs->done, HZ) == 0)
> + rc = 1;
> + } else {
> + /* polling mode checks for status busy bit */
> + limit = jiffies + msecs_to_jiffies(HSSPI_POLL_STATUS_TIMEOUT_MS);
> +
> + while (!time_after(jiffies, limit)) {
> + reg = __raw_readl(bs->regs + HSSPI_PINGPONG_STATUS_REG(0));
> + if (reg & HSSPI_PINGPONG_STATUS_SRC_BUSY)
> + cpu_relax();
> + else
> + break;
> + }
> + if (reg & HSSPI_PINGPONG_STATUS_SRC_BUSY)
> + rc = 1;
> + }
> +
> + if (rc)
> + dev_err(&bs->pdev->dev, "transfer timed out!\n");
> +
> + return rc;
> +}
> +
> +static bool bcm63xx_check_msg_prependable(struct spi_master *master,
> + struct spi_message *msg,
> + struct spi_transfer *t_prepend)
This function does more than just checking, so I think a more
appropriate name would be something like
bcm63xx_prepare_prepend_transfer()
> +{
> +
> + struct bcm63xx_hsspi *bs = spi_master_get_devdata(master);
> + bool prepend = false, tx_only = false;
> + struct spi_transfer *t;
> +
> + /* If it is forced cs dummy workaround mode, no need to prepend message */
> + if (bs->xfer_mode == HSSPI_XFER_MODE_DUMMYCS)
> + return false;
That's a weird point for that, why not just move this to the caller
and check it before calling the function.
> +
> + /*
> + * Multiple transfers within a message may be combined into one transfer
> + * to the controller using its prepend feature. A SPI message is prependable
> + * only if the following are all true:
> + * 1. One or more half duplex write transfer in single bit mode
> + * 2. Optional full duplex read/write at the end
> + * 3. No delay and cs_change between transfers
> + */
> + bs->prepend_cnt = 0;
> + list_for_each_entry(t, &msg->transfers, transfer_list) {
> + if ((spi_delay_to_ns(&t->delay, t) > 0) || t->cs_change) {
> + dev_warn(&bs->pdev->dev,
> + "Delay or cs change not supported in prepend mode!\n");
I don't think warn is the right level. If we are on XFER_MODE_AUTO,
this should be _dbg, since we will just fall back to the dummy cs
mode, if we are on XFER_MODE_PREPEND, this should be dev_err, since we
cannot do the message.
cs->change is technically supported when all that's requested is a
between transfer cs toggle (t->cs_change is true, t->cs_off is false
and next transfer's cs_off is also false), which automatically happens
after the transfer. Not sure if it is worth the effort implementing
that though.
> + break;
> + }
> +
> + tx_only = false;
> + if (t->tx_buf && !t->rx_buf) {
> + tx_only = true;
> + if (bs->prepend_cnt + t->len >
> + (HSSPI_BUFFER_LEN - HSSPI_OPCODE_LEN)) {
> + dev_warn(&bs->pdev->dev,
> + "exceed max buf len, abort prepending transfers!\n");
> + break;
why not just return false here directly? And everywhere else where you
decided that you cannot use prepend.
> + }
> +
> + if (t->tx_nbits > SPI_NBITS_SINGLE &&
> + !list_is_last(&t->transfer_list, &msg->transfers)) {
> + dev_warn(&bs->pdev->dev,
> + "multi-bit prepend buf not supported!\n");
> + break;
> + }
> +
> + if (t->tx_nbits == SPI_NBITS_SINGLE) {
> + memcpy(bs->prepend_buf + bs->prepend_cnt, t->tx_buf, t->len);
> + bs->prepend_cnt += t->len;
> + }
> + } else {
> + if (!list_is_last(&t->transfer_list, &msg->transfers)) {
> + dev_warn(&bs->pdev->dev,
> + "rx/tx_rx transfer not supported when it is not last one!\n");
This is only an issue if doing multi-bit RX/TX; for single bit you can
just upgrade the whole transfer/message to duplex, you just need to
pick the read bytes then from the right offsets.
> + break;
> + }
> + }
> +
> + if (list_is_last(&t->transfer_list, &msg->transfers)) {
> + memcpy(t_prepend, t, sizeof(struct spi_transfer));
> +
> + if (tx_only && t->tx_nbits == SPI_NBITS_SINGLE) {
> + /*
> + * if the last one is also a single bit tx only transfer, merge
> + * all of them into one single tx transfer
> + */
> + t_prepend->len = bs->prepend_cnt;
> + t_prepend->tx_buf = bs->prepend_buf;
> + bs->prepend_cnt = 0;
> + } else {
> + /*
> + * if the last one is not a tx only transfer or dual tx xfer, all
> + * the previous transfers are sent through prepend bytes and
> + * make sure it does not exceed the max prepend len
> + */
> + if (bs->prepend_cnt > HSSPI_MAX_PREPEND_LEN) {
> + dev_warn(&bs->pdev->dev,
> + "exceed max prepend len, abort prepending transfers!\n");
> + break;
Likewise, you can merge any amount or rx/tx/rxtx single bit transfers
together as a duplex transfer with prepend len set to 0 (so
technically not a prepend anymore ;-)
> + }
> + }
> + prepend = true;
> + }
> + }
> +
> + return prepend;
and then if you already returned false if you cannot do prepend, you
just need to return true here and don't need the prepend variable.
> +}
> +
> +static int bcm63xx_hsspi_do_prepend_txrx(struct spi_device *spi,
> + struct spi_transfer *t)
> +{
> + struct bcm63xx_hsspi *bs = spi_master_get_devdata(spi->master);
> + unsigned int chip_select = spi->chip_select;
> + u16 opcode = 0;
> + const u8 *tx = t->tx_buf;
> + u8 *rx = t->rx_buf;
> + u32 reg = 0;
> +
> + /*
> + * shouldn't happen as we set the max_message_size in the probe.
> + * but check it again in case some driver does not honor the max size
> + */
> + if (t->len + bs->prepend_cnt > (HSSPI_BUFFER_LEN - HSSPI_OPCODE_LEN)) {
> + dev_warn(&bs->pdev->dev,
> + "Prepend message large than fifo size len %d prepend %d\n",
> + t->len, bs->prepend_cnt);
> + return -EINVAL;
> + }
> +
> + bcm63xx_hsspi_set_clk(bs, spi, t->speed_hz);
> +
> + if (tx && rx)
> + opcode = HSSPI_OP_READ_WRITE;
> + else if (tx)
> + opcode = HSSPI_OP_WRITE;
> + else if (rx)
> + opcode = HSSPI_OP_READ;
> +
> + if ((opcode == HSSPI_OP_READ && t->rx_nbits == SPI_NBITS_DUAL) ||
> + (opcode == HSSPI_OP_WRITE && t->tx_nbits == SPI_NBITS_DUAL)) {
> + opcode |= HSSPI_OP_MULTIBIT;
> +
> + if (t->rx_nbits == SPI_NBITS_DUAL) {
> + reg |= 1 << MODE_CTRL_MULTIDATA_RD_SIZE_SHIFT;
> + reg |= bs->prepend_cnt << MODE_CTRL_MULTIDATA_RD_STRT_SHIFT;
> + }
> + if (t->tx_nbits == SPI_NBITS_DUAL) {
> + reg |= 1 << MODE_CTRL_MULTIDATA_WR_SIZE_SHIFT;
> + reg |= bs->prepend_cnt << MODE_CTRL_MULTIDATA_WR_STRT_SHIFT;
> + }
> + }
> +
> + reg |= bs->prepend_cnt << MODE_CTRL_PREPENDBYTE_CNT_SHIFT;
> + __raw_writel(reg | 0xff,
> + bs->regs + HSSPI_PROFILE_MODE_CTRL_REG(chip_select));
> +
> + reinit_completion(&bs->done);
> + if (bs->prepend_cnt)
> + memcpy_toio(bs->fifo + HSSPI_OPCODE_LEN, bs->prepend_buf,
> + bs->prepend_cnt);
> + if (tx)
> + memcpy_toio(bs->fifo + HSSPI_OPCODE_LEN + bs->prepend_cnt, tx,
> + t->len);
> +
> + __raw_writew((u16)cpu_to_be16(opcode | t->len), bs->fifo);
> + /* enable interrupt */
> + if (bs->wait_mode == HSSPI_WAIT_MODE_INTR)
> + __raw_writel(HSSPI_PINGx_CMD_DONE(0), bs->regs + HSSPI_INT_MASK_REG);
> +
> + /* start the transfer */
> + reg = chip_select << PINGPONG_CMD_SS_SHIFT |
> + chip_select << PINGPONG_CMD_PROFILE_SHIFT |
> + PINGPONG_COMMAND_START_NOW;
> + __raw_writel(reg, bs->regs + HSSPI_PINGPONG_COMMAND_REG(0));
> +
> + if (bcm63xx_hsspi_wait_cmd(bs))
> + return -ETIMEDOUT;
> +
> + if (rx)
> + memcpy_fromio(rx, bs->fifo, t->len);
> +
> + return 0;
> +}
> +
> static void bcm63xx_hsspi_set_cs(struct bcm63xx_hsspi *bs, unsigned int cs,
> bool active)
> {
> @@ -215,10 +468,10 @@ static int bcm63xx_hsspi_do_txrx(struct spi_device *spi, struct spi_transfer *t)
> int step_size = HSSPI_BUFFER_LEN;
> const u8 *tx = t->tx_buf;
> u8 *rx = t->rx_buf;
> - u32 val = 0;
> - unsigned long limit;
> + u32 reg = 0;
>
> bcm63xx_hsspi_set_clk(bs, spi, t->speed_hz);
> +
> bcm63xx_hsspi_set_cs(bs, spi->chip_select, true);
>
> if (tx && rx)
> @@ -236,12 +489,12 @@ static int bcm63xx_hsspi_do_txrx(struct spi_device *spi, struct spi_transfer *t)
> opcode |= HSSPI_OP_MULTIBIT;
>
> if (t->rx_nbits == SPI_NBITS_DUAL)
> - val |= 1 << MODE_CTRL_MULTIDATA_RD_SIZE_SHIFT;
> + reg |= 1 << MODE_CTRL_MULTIDATA_RD_SIZE_SHIFT;
> if (t->tx_nbits == SPI_NBITS_DUAL)
> - val |= 1 << MODE_CTRL_MULTIDATA_WR_SIZE_SHIFT;
> + reg |= 1 << MODE_CTRL_MULTIDATA_WR_SIZE_SHIFT;
> }
>
> - __raw_writel(val | 0xff,
> + __raw_writel(reg | 0xff,
> bs->regs + HSSPI_PROFILE_MODE_CTRL_REG(chip_select));
>
> while (pending > 0) {
> @@ -260,28 +513,13 @@ static int bcm63xx_hsspi_do_txrx(struct spi_device *spi, struct spi_transfer *t)
> __raw_writel(HSSPI_PINGx_CMD_DONE(0),
> bs->regs + HSSPI_INT_MASK_REG);
>
> - /* start the transfer */
> - __raw_writel(!chip_select << PINGPONG_CMD_SS_SHIFT |
> - chip_select << PINGPONG_CMD_PROFILE_SHIFT |
> - PINGPONG_COMMAND_START_NOW,
> - bs->regs + HSSPI_PINGPONG_COMMAND_REG(0));
> + reg = !chip_select << PINGPONG_CMD_SS_SHIFT |
> + chip_select << PINGPONG_CMD_PROFILE_SHIFT |
> + PINGPONG_COMMAND_START_NOW;
> + __raw_writel(reg, bs->regs + HSSPI_PINGPONG_COMMAND_REG(0));
>
> - if (bs->wait_mode == HSSPI_WAIT_MODE_INTR) {
> - if (wait_for_completion_timeout(&bs->done, HZ) == 0)
> - goto err_timeout;
> - } else {
> - /* polling mode checks for status busy bit */
> - limit = jiffies + msecs_to_jiffies(HSSPI_POLL_STATUS_TIMEOUT_MS);
> - while (!time_after(jiffies, limit)) {
> - val = __raw_readl(bs->regs + HSSPI_PINGPONG_STATUS_REG(0));
> - if (val & HSSPI_PINGPONG_STATUS_SRC_BUSY)
> - cpu_relax();
> - else
> - break;
> - }
> - if (val & HSSPI_PINGPONG_STATUS_SRC_BUSY)
> - goto err_timeout;
> - }
> + if (bcm63xx_hsspi_wait_cmd(bs))
> + return -ETIMEDOUT;
>
> if (rx) {
> memcpy_fromio(rx, bs->fifo, curr_step);
> @@ -292,10 +530,6 @@ static int bcm63xx_hsspi_do_txrx(struct spi_device *spi, struct spi_transfer *t)
> }
>
> return 0;
> -
> -err_timeout:
> - dev_err(&bs->pdev->dev, "transfer timed out!\n");
> - return -ETIMEDOUT;
> }
>
> static int bcm63xx_hsspi_setup(struct spi_device *spi)
> @@ -344,9 +578,23 @@ static int bcm63xx_hsspi_transfer_one(struct spi_master *master,
> int status = -EINVAL;
> int dummy_cs;
> bool restore_polarity = true;
> + struct spi_transfer t_prepend;
>
> mutex_lock(&bs->msg_mutex);
> - /* This controller does not support keeping CS active during idle.
> + if (bcm63xx_check_msg_prependable(master, msg, &t_prepend)) {
> + status = bcm63xx_hsspi_do_prepend_txrx(spi, &t_prepend);
> + msg->actual_length += (t_prepend.len + bs->prepend_cnt);
why +=? shouldn't this be the only place in this case where this is set?
> + goto msg_done;
> + }
> +
> + if (bs->xfer_mode == HSSPI_XFER_MODE_PREPEND) {
> + dev_warn(&bs->pdev->dev,
> + "User set prepend mode but msg not prependable! Fail the xfer!\n");
If we are failing, this should be a dev_err, not a dev_warn
> + goto msg_done;
> + }
I think from a readability standpoint it would be better to move the
cs_workaround parts into their own function, and have this as
bool prependable = false;
if (bs->xfer_mode != HSSPI_XFER_MODE_DUMMYCS)
prependable = bcm63xx_prepare_prepend_transfer(...);
if (prependable) {
status = bcm63xx_hsspi_do_prepend_txrx(...);
msg->actual_legth += ...;
} else {
if (bs->xfer_mode == HSSPI_XFER_MODE_PREPEND) {
/* we may not use dummy cs */
dev_err(...);
status = -EINVAL;
} else {
status = bcm63xx_hsspi_do_dummy_cs_txrx(...);
}
}
with (bcm63xx_hsspi_do_dummy_cs_txrx being the proposed function name).
> +
> + /*
> + * This controller does not support keeping CS active during idle.
> * To work around this, we use the following ugly hack:
> *
> * a. Invert the target chip select's polarity so it will be active.
> @@ -364,6 +612,17 @@ static int bcm63xx_hsspi_transfer_one(struct spi_master *master,
> bcm63xx_hsspi_set_cs(bs, dummy_cs, true);
>
> list_for_each_entry(t, &msg->transfers, transfer_list) {
> + /*
> + * We are here because one of reasons below:
> + * a. Message is not prependable and in default auto xfer mode. This mean
> + * we fallback to dummy cs mode at maximum 25MHz safe clock rate.
> + * b. User set to use the dummy cs mode.
> + */
> + if (bs->xfer_mode == HSSPI_XFER_MODE_AUTO) {
> + if (t->speed_hz > HSSPI_MAX_SYNC_CLOCK)
> + t->speed_hz = HSSPI_MAX_SYNC_CLOCK;
OTOH, this may be a point where a dev_warn (once?) might be a good
idea, since the device may depend on a certain speed to avoid buffer
overruns (e.g. audio streams - not sure if that exists), so a warning
that the transfer speed was reduced will help identifying the source.
> + }
> +
> status = bcm63xx_hsspi_do_txrx(spi, t);
> if (status)
> break;
> @@ -396,6 +655,7 @@ static int bcm63xx_hsspi_transfer_one(struct spi_master *master,
> if (restore_polarity)
> bcm63xx_hsspi_set_cs(bs, spi->chip_select, false);
>
> +msg_done:
> mutex_unlock(&bs->msg_mutex);
> msg->status = status;
> spi_finalize_current_message(master);
> @@ -490,6 +750,11 @@ static int bcm63xx_hsspi_probe(struct platform_device *pdev)
> bs->speed_hz = rate;
> bs->fifo = (u8 __iomem *)(bs->regs + HSSPI_FIFO_REG(0));
> bs->wait_mode = HSSPI_WAIT_MODE_POLLING;
> + bs->prepend_buf = devm_kzalloc(dev, HSSPI_BUFFER_LEN, GFP_KERNEL);
> + if (!bs->prepend_buf) {
> + ret = -ENOMEM;
> + goto out_put_master;
> + }
>
> mutex_init(&bs->bus_mutex);
> mutex_init(&bs->msg_mutex);
> @@ -508,6 +773,9 @@ static int bcm63xx_hsspi_probe(struct platform_device *pdev)
> master->num_chipselect = num_cs;
> master->setup = bcm63xx_hsspi_setup;
> master->transfer_one_message = bcm63xx_hsspi_transfer_one;
> + master->max_transfer_size = bcm63xx_hsspi_max_message_size;
> + master->max_message_size = bcm63xx_hsspi_max_message_size;
> +
> master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH |
> SPI_RX_DUAL | SPI_TX_DUAL;
> master->bits_per_word_mask = SPI_BPW_MASK(8);
> --
> 2.37.3
>
On Tue, 24 Jan 2023 at 23:33, William Zhang <[email protected]> wrote:
>
> In general the controller supports SPI dual mode operation but the
> particular SPI flash dual io read op switches from single mode in cmd
> phase to dual mode in address and data phase. This is not compatible
> with prepend operation where cmd and address are sent out through the
> prepend buffer and they must use same the number of io pins.
>
> This patch disables these SPI flash dual io read ops through the mem_ops
> supports_op interface. This makes sure the SPI flash driver selects the
> compatible read ops at run time.
>
> Signed-off-by: William Zhang <[email protected]>
>
> ---
>
> Changes in v2:
> - Remove the code that uses the deprecated flag use_cs_workaround
> - Always disable dual io read ops as prepend is the default mode
>
> drivers/spi/spi-bcm63xx-hsspi.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/spi/spi-bcm63xx-hsspi.c b/drivers/spi/spi-bcm63xx-hsspi.c
> index 2a0bef943967..dd320fda7611 100644
> --- a/drivers/spi/spi-bcm63xx-hsspi.c
> +++ b/drivers/spi/spi-bcm63xx-hsspi.c
> @@ -20,6 +20,7 @@
> #include <linux/spi/spi.h>
> #include <linux/mutex.h>
> #include <linux/of.h>
> +#include <linux/spi/spi-mem.h>
> #include <linux/reset.h>
> #include <linux/pm_runtime.h>
>
> @@ -663,6 +664,23 @@ static int bcm63xx_hsspi_transfer_one(struct spi_master *master,
> return 0;
> }
>
> +static bool bcm63xx_hsspi_mem_supports_op(struct spi_mem *mem,
> + const struct spi_mem_op *op)
> +{
> + if (!spi_mem_default_supports_op(mem, op))
> + return false;
> +
> + /* Controller doesn't support spi mem dual/quad read cmd in prepend mode */
> + if ((op->cmd.opcode == 0xbb) || (op->cmd.opcode == 0xeb))
There are defines in linux/mtd/spi-nor.h you can use:
if ((op->cmd.opcode == SPINOR_OP_READ_1_2_2) || (op->cmd.opcode ==
SPINOR_OP_READ_1_4_4))
Though SPINOR_OP_READ_1_4_4 seems to be redundant, since the
controller does not support quad mode, and does not advertise it, so
it should never even be an option.
Looking at that file, instead what is missing is
SPINOR_OP_READ_1_2_2_4B (0xbc) which shouldn't be usable either.
> + return false;
> +
> + return true;
> +}
> +
> +static const struct spi_controller_mem_ops bcm63xx_hsspi_mem_ops = {
> + .supports_op = bcm63xx_hsspi_mem_supports_op,
> +};
> +
> static irqreturn_t bcm63xx_hsspi_interrupt(int irq, void *dev_id)
> {
> struct bcm63xx_hsspi *bs = (struct bcm63xx_hsspi *)dev_id;
> @@ -760,6 +778,7 @@ static int bcm63xx_hsspi_probe(struct platform_device *pdev)
> mutex_init(&bs->msg_mutex);
> init_completion(&bs->done);
>
> + master->mem_ops = &bcm63xx_hsspi_mem_ops;
> master->dev.of_node = dev->of_node;
> if (!dev->of_node)
> master->bus_num = HSSPI_BUS_NUM;
> --
> 2.37.3
>
On Tue, 24 Jan 2023 at 23:33, William Zhang <[email protected]> wrote:
>
> The newer BCMBCA 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. Driver can control and keep cs low
> between the transfers natively. Hence the dummy cs workaround or prepend
> mode found in the bcm63xx-hsspi driver are no longer needed and this new
> driver is much cleaner.
>
> Signed-off-by: William Zhang <[email protected]>
>
> ---
>
> Changes in v2:
> - Fix build error for Alpha platform
> Reported-by: kernel test robot <[email protected]>
> - Make interrupt as required node in the dts
> - Use polling mode as default mode
> - Add driver sysfs option wait_mode to allow mode change at run time
> - Update the compatible string based on changes in dts document
> - Remove clock gate disabling code for now
> - Update commit message
>
> drivers/spi/Kconfig | 9 +
> drivers/spi/Makefile | 1 +
> drivers/spi/spi-bcmbca-hsspi.c | 645 +++++++++++++++++++++++++++++++++
> 3 files changed, 655 insertions(+)
> create mode 100644 drivers/spi/spi-bcmbca-hsspi.c
>
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index 3b1c0878bb85..771244582d03 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -199,6 +199,15 @@ config SPI_BCM_QSPI
> based platforms. This driver works for both SPI master for SPI NOR
> flash device as well as MSPI device.
>
> +config SPI_BCMBCA_HSSPI
> + tristate "Broadcom BCMBCA HS SPI controller driver"
> + depends on ARCH_BCMBCA || COMPILE_TEST
> + help
> + This enables support for the High Speed SPI controller present on
> + newer Broadcom BCMBCA SoCs. These SoCs include an updated SPI controller
> + that adds the capability to allow the driver to control chip select
> + explicitly.
> +
> config SPI_BITBANG
> tristate "Utilities for Bitbanging SPI masters"
> help
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index be9ba40ef8d0..fe92106447c3 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -30,6 +30,7 @@ obj-$(CONFIG_SPI_BCM2835) += spi-bcm2835.o
> obj-$(CONFIG_SPI_BCM2835AUX) += spi-bcm2835aux.o
> obj-$(CONFIG_SPI_BCM63XX) += spi-bcm63xx.o
> obj-$(CONFIG_SPI_BCM63XX_HSSPI) += spi-bcm63xx-hsspi.o
> +obj-$(CONFIG_SPI_BCMBCA_HSSPI) += spi-bcmbca-hsspi.o
> obj-$(CONFIG_SPI_BCM_QSPI) += spi-iproc-qspi.o spi-brcmstb-qspi.o spi-bcm-qspi.o
> obj-$(CONFIG_SPI_BITBANG) += spi-bitbang.o
> obj-$(CONFIG_SPI_BUTTERFLY) += spi-butterfly.o
> diff --git a/drivers/spi/spi-bcmbca-hsspi.c b/drivers/spi/spi-bcmbca-hsspi.c
> new file mode 100644
> index 000000000000..f8da194939c0
> --- /dev/null
> +++ b/drivers/spi/spi-bcmbca-hsspi.c
> @@ -0,0 +1,645 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Broadcom BCMBCA High Speed SPI Controller driver
> + *
> + * Copyright 2000-2010 Broadcom Corporation
> + * Copyright 2012-2013 Jonas Gorski <[email protected]>
> + * Copyright 2019-2022 Broadcom Ltd
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/clk.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/delay.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/spi/spi.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/spi/spi-mem.h>
> +#include <linux/pm_runtime.h>
> +
> +#define HSSPI_GLOBAL_CTRL_REG 0x0
> +#define GLOBAL_CTRL_CS_POLARITY_SHIFT 0
> +#define GLOBAL_CTRL_CS_POLARITY_MASK 0x000000ff
> +#define GLOBAL_CTRL_PLL_CLK_CTRL_SHIFT 8
> +#define GLOBAL_CTRL_PLL_CLK_CTRL_MASK 0x0000ff00
> +#define GLOBAL_CTRL_CLK_GATE_SSOFF BIT(16)
> +#define GLOBAL_CTRL_CLK_POLARITY BIT(17)
> +#define GLOBAL_CTRL_MOSI_IDLE BIT(18)
> +
> +#define HSSPI_GLOBAL_EXT_TRIGGER_REG 0x4
> +
> +#define HSSPI_INT_STATUS_REG 0x8
> +#define HSSPI_INT_STATUS_MASKED_REG 0xc
> +#define HSSPI_INT_MASK_REG 0x10
> +
> +#define HSSPI_PINGx_CMD_DONE(i) BIT((i * 8) + 0)
> +#define HSSPI_PINGx_RX_OVER(i) BIT((i * 8) + 1)
> +#define HSSPI_PINGx_TX_UNDER(i) BIT((i * 8) + 2)
> +#define HSSPI_PINGx_POLL_TIMEOUT(i) BIT((i * 8) + 3)
> +#define HSSPI_PINGx_CTRL_INVAL(i) BIT((i * 8) + 4)
> +
> +#define HSSPI_INT_CLEAR_ALL 0xff001f1f
> +
> +#define HSSPI_PINGPONG_COMMAND_REG(x) (0x80 + (x) * 0x40)
> +#define PINGPONG_CMD_COMMAND_MASK 0xf
> +#define PINGPONG_COMMAND_NOOP 0
> +#define PINGPONG_COMMAND_START_NOW 1
> +#define PINGPONG_COMMAND_START_TRIGGER 2
> +#define PINGPONG_COMMAND_HALT 3
> +#define PINGPONG_COMMAND_FLUSH 4
> +#define PINGPONG_CMD_PROFILE_SHIFT 8
> +#define PINGPONG_CMD_SS_SHIFT 12
> +
> +#define HSSPI_PINGPONG_STATUS_REG(x) (0x84 + (x) * 0x40)
> +#define HSSPI_PINGPONG_STATUS_SRC_BUSY BIT(1)
> +
> +#define HSSPI_PROFILE_CLK_CTRL_REG(x) (0x100 + (x) * 0x20)
> +#define CLK_CTRL_FREQ_CTRL_MASK 0x0000ffff
> +#define CLK_CTRL_SPI_CLK_2X_SEL BIT(14)
> +#define CLK_CTRL_ACCUM_RST_ON_LOOP BIT(15)
> +#define CLK_CTRL_CLK_POLARITY BIT(16)
> +
> +#define HSSPI_PROFILE_SIGNAL_CTRL_REG(x) (0x104 + (x) * 0x20)
> +#define SIGNAL_CTRL_LATCH_RISING BIT(12)
> +#define SIGNAL_CTRL_LAUNCH_RISING BIT(13)
> +#define SIGNAL_CTRL_ASYNC_INPUT_PATH BIT(16)
> +
> +#define HSSPI_PROFILE_MODE_CTRL_REG(x) (0x108 + (x) * 0x20)
> +#define MODE_CTRL_MULTIDATA_RD_STRT_SHIFT 8
> +#define MODE_CTRL_MULTIDATA_WR_STRT_SHIFT 12
> +#define MODE_CTRL_MULTIDATA_RD_SIZE_SHIFT 16
> +#define MODE_CTRL_MULTIDATA_WR_SIZE_SHIFT 18
> +#define MODE_CTRL_MODE_3WIRE BIT(20)
> +#define MODE_CTRL_PREPENDBYTE_CNT_SHIFT 24
> +
> +#define HSSPI_FIFO_REG(x) (0x200 + (x) * 0x200)
> +
> +#define HSSPI_OP_MULTIBIT BIT(11)
> +#define HSSPI_OP_CODE_SHIFT 13
> +#define HSSPI_OP_SLEEP (0 << HSSPI_OP_CODE_SHIFT)
> +#define HSSPI_OP_READ_WRITE (1 << HSSPI_OP_CODE_SHIFT)
> +#define HSSPI_OP_WRITE (2 << HSSPI_OP_CODE_SHIFT)
> +#define HSSPI_OP_READ (3 << HSSPI_OP_CODE_SHIFT)
> +#define HSSPI_OP_SETIRQ (4 << HSSPI_OP_CODE_SHIFT)
> +
> +#define HSSPI_BUFFER_LEN 512
> +#define HSSPI_OPCODE_LEN 2
> +
> +#define HSSPI_MAX_PREPEND_LEN 15
> +
> +#define HSSPI_MAX_SYNC_CLOCK 30000000
> +
> +#define HSSPI_SPI_MAX_CS 8
> +#define HSSPI_BUS_NUM 1 /* 0 is legacy SPI */
> +#define HSSPI_POLL_STATUS_TIMEOUT_MS 100
> +
> +#define HSSPI_WAIT_MODE_POLLING 0
> +#define HSSPI_WAIT_MODE_INTR 1
> +#define HSSPI_WAIT_MODE_MAX HSSPI_WAIT_MODE_INTR
> +
> +#define SPIM_CTRL_CS_OVERRIDE_SEL_SHIFT 0
> +#define SPIM_CTRL_CS_OVERRIDE_SEL_MASK 0xff
> +#define SPIM_CTRL_CS_OVERRIDE_VAL_SHIFT 8
> +#define SPIM_CTRL_CS_OVERRIDE_VAL_MASK 0xff
> +
> +struct bcmbca_hsspi {
> + struct completion done;
> + struct mutex bus_mutex;
> + struct mutex msg_mutex;
> + struct platform_device *pdev;
> + struct clk *clk;
> + struct clk *pll_clk;
> + void __iomem *regs;
> + void __iomem *spim_ctrl;
> + u8 __iomem *fifo;
> + u32 speed_hz;
> + u8 cs_polarity;
> + u32 wait_mode;
> +};
> +
> +static ssize_t wait_mode_show(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + struct spi_controller *ctrl = dev_get_drvdata(dev);
> + struct bcmbca_hsspi *bs = spi_master_get_devdata(ctrl);
> +
> + return sprintf(buf, "%d\n", bs->wait_mode);
> +}
> +
> +static ssize_t wait_mode_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct spi_controller *ctrl = dev_get_drvdata(dev);
> + struct bcmbca_hsspi *bs = spi_master_get_devdata(ctrl);
> + u32 val;
> +
> + if (kstrtou32(buf, 10, &val))
> + return -EINVAL;
> +
> + if (val > HSSPI_WAIT_MODE_MAX) {
> + dev_warn(dev, "invalid wait mode %u\n", val);
> + return -EINVAL;
> + }
> +
> + mutex_lock(&bs->msg_mutex);
> + bs->wait_mode = val;
> + /* clear interrupt status to avoid spurious int on next transfer */
> + if (val == HSSPI_WAIT_MODE_INTR)
> + __raw_writel(HSSPI_INT_CLEAR_ALL, bs->regs + HSSPI_INT_STATUS_REG);
> + mutex_unlock(&bs->msg_mutex);
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR_RW(wait_mode);
> +
> +static struct attribute *bcmbca_hsspi_attrs[] = {
> + &dev_attr_wait_mode.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group bcmbca_hsspi_group = {
> + .attrs = bcmbca_hsspi_attrs,
> +};
> +
> +static void bcmbca_hsspi_set_clk(struct bcmbca_hsspi *bs,
> + struct spi_device *spi, int hz);
> +
> +static void bcmbca_hsspi_set_cs(struct bcmbca_hsspi *bs, unsigned int cs,
> + bool active)
> +{
> + u32 reg;
> +
> + /* No cs orerriden needed for SS7 internal cs on pcm based voice dev */
> + if (cs == 7)
> + return;
> +
> + mutex_lock(&bs->bus_mutex);
> +
> + if (active) {
> + /* activate cs by setting the override bit and active value bit*/
> + reg = __raw_readl(bs->spim_ctrl);
> + reg |= BIT(cs+SPIM_CTRL_CS_OVERRIDE_SEL_SHIFT);
Doesn't checkpatch complain about missing spaces around the operator (+)?
> + reg &= ~BIT(cs+SPIM_CTRL_CS_OVERRIDE_VAL_SHIFT);
> + if (bs->cs_polarity & BIT(cs))
> + reg |= BIT(cs+SPIM_CTRL_CS_OVERRIDE_VAL_SHIFT);
Does the CS_OVERRIDE_VAL get reset on clearing the OVERRIDE_SEL bit or
it is persistent? If the latter, you could initialize its value in
bcmbca_hsspi_setup() and then this becomes:
reg = _raw_readl(bs->spim_ctrl);
if (active)
reg |= BIT(cs+SPIM_CTRL_CS_OVERRIDE_SEL_SHIFT);
else
reg &= ~BIT(cs + SPIM_CTRL_CS_OVERRIDE_SEL_SHIFT);
__raw_writel(reg, bs->spim_ctrl);
and you can drop the cs_polarity field.
> + __raw_writel(reg, bs->spim_ctrl);
> + } else {
> + /* clear the cs override bit */
> + reg = __raw_readl(bs->spim_ctrl);
> + reg &= ~BIT(cs+SPIM_CTRL_CS_OVERRIDE_SEL_SHIFT);
> + __raw_writel(reg, bs->spim_ctrl);
> + }
> +
> + mutex_unlock(&bs->bus_mutex);
> +}
> +
> +static void bcmbca_hsspi_set_clk(struct bcmbca_hsspi *bs,
> + struct spi_device *spi, int hz)
> +{
> + unsigned int profile = spi->chip_select;
> + u32 reg;
> +
> + reg = DIV_ROUND_UP(2048, DIV_ROUND_UP(bs->speed_hz, hz));
> + __raw_writel(CLK_CTRL_ACCUM_RST_ON_LOOP | reg,
> + bs->regs + HSSPI_PROFILE_CLK_CTRL_REG(profile));
> +
> + reg = __raw_readl(bs->regs + HSSPI_PROFILE_SIGNAL_CTRL_REG(profile));
> + if (hz > HSSPI_MAX_SYNC_CLOCK)
> + reg |= SIGNAL_CTRL_ASYNC_INPUT_PATH;
> + else
> + reg &= ~SIGNAL_CTRL_ASYNC_INPUT_PATH;
> + __raw_writel(reg, bs->regs + HSSPI_PROFILE_SIGNAL_CTRL_REG(profile));
> +
> + mutex_lock(&bs->bus_mutex);
> + /* setup clock polarity */
> + reg = __raw_readl(bs->regs + HSSPI_GLOBAL_CTRL_REG);
> + reg &= ~GLOBAL_CTRL_CLK_POLARITY;
> + if (spi->mode & SPI_CPOL)
> + reg |= GLOBAL_CTRL_CLK_POLARITY;
> +
> + __raw_writel(reg, bs->regs + HSSPI_GLOBAL_CTRL_REG);
> +
> + mutex_unlock(&bs->bus_mutex);
> +}
> +
> +static int bcmbca_hsspi_wait_cmd(struct bcmbca_hsspi *bs, unsigned int cs)
> +{
> + unsigned long limit;
> + u32 reg = 0;
> + int rc = 0;
> +
> + if (bs->wait_mode == HSSPI_WAIT_MODE_INTR) {
> + if (wait_for_completion_timeout(&bs->done, HZ) == 0)
> + rc = 1;
> + } else {
> + limit = jiffies + msecs_to_jiffies(HSSPI_POLL_STATUS_TIMEOUT_MS);
> +
> + while (!time_after(jiffies, limit)) {
> + reg = __raw_readl(bs->regs + HSSPI_PINGPONG_STATUS_REG(0));
> + if (reg & HSSPI_PINGPONG_STATUS_SRC_BUSY)
> + cpu_relax();
> + else
> + break;
> + }
> + if (reg & HSSPI_PINGPONG_STATUS_SRC_BUSY)
> + rc = 1;
> + }
> +
> + if (rc) {
> + dev_err(&bs->pdev->dev, "transfer timed out!\n");
> + bcmbca_hsspi_set_cs(bs, cs, false);
> + }
> +
> + return rc;
> +}
> +
> +static int bcmbca_hsspi_do_txrx(struct spi_device *spi, struct spi_transfer *t,
> + struct spi_message *msg)
> +{
> + struct bcmbca_hsspi *bs = spi_master_get_devdata(spi->master);
> + unsigned int chip_select = spi->chip_select;
> + u16 opcode = 0;
> + int pending = t->len;
> + int step_size = HSSPI_BUFFER_LEN;
> + const u8 *tx = t->tx_buf;
> + u8 *rx = t->rx_buf;
> + u32 reg = 0, cs_act = 0;
> +
> + bcmbca_hsspi_set_clk(bs, spi, t->speed_hz);
> +
> + if (tx && rx)
> + opcode = HSSPI_OP_READ_WRITE;
> + else if (tx)
> + opcode = HSSPI_OP_WRITE;
> + else if (rx)
> + opcode = HSSPI_OP_READ;
> +
> + if (opcode != HSSPI_OP_READ)
> + step_size -= HSSPI_OPCODE_LEN;
> +
> + if ((opcode == HSSPI_OP_READ && t->rx_nbits == SPI_NBITS_DUAL) ||
> + (opcode == HSSPI_OP_WRITE && t->tx_nbits == SPI_NBITS_DUAL)) {
> + opcode |= HSSPI_OP_MULTIBIT;
> +
> + if (t->rx_nbits == SPI_NBITS_DUAL)
> + reg |= 1 << MODE_CTRL_MULTIDATA_RD_SIZE_SHIFT;
> + if (t->tx_nbits == SPI_NBITS_DUAL)
> + reg |= 1 << MODE_CTRL_MULTIDATA_WR_SIZE_SHIFT;
> + }
> +
> + __raw_writel(reg | 0xff,
> + bs->regs + HSSPI_PROFILE_MODE_CTRL_REG(chip_select));
> +
> + while (pending > 0) {
> + int curr_step = min_t(int, step_size, pending);
> +
> + reinit_completion(&bs->done);
> + if (tx) {
> + memcpy_toio(bs->fifo + HSSPI_OPCODE_LEN, tx, curr_step);
> + tx += curr_step;
> + }
> + __raw_writew((u16)cpu_to_be16(opcode | curr_step), bs->fifo);
> +
> + /* enable interrupt */
> + if (bs->wait_mode == HSSPI_WAIT_MODE_INTR)
> + __raw_writel(HSSPI_PINGx_CMD_DONE(0),
> + bs->regs + HSSPI_INT_MASK_REG);
> +
> + if (!cs_act) {
> + bcmbca_hsspi_set_cs(bs, chip_select, true);
> + cs_act = 1;
> + }
> + reg = chip_select << PINGPONG_CMD_SS_SHIFT |
> + chip_select << PINGPONG_CMD_PROFILE_SHIFT |
> + PINGPONG_COMMAND_START_NOW;
> + __raw_writel(reg, bs->regs + HSSPI_PINGPONG_COMMAND_REG(0));
> +
> + if (bcmbca_hsspi_wait_cmd(bs, spi->chip_select))
> + return -ETIMEDOUT;
> +
> + pending -= curr_step;
> + if (pending == 0) {
> + if (list_is_last(&t->transfer_list, &msg->transfers)) {
> + if (!t->cs_change)
> + bcmbca_hsspi_set_cs(bs, spi->chip_select, false);
> + } else {
> + if (t->cs_change) {
> + bcmbca_hsspi_set_cs(bs, spi->chip_select, false);
> + udelay(10);
> + bcmbca_hsspi_set_cs(bs, spi->chip_select, true);
> + }
> + }
> + }
> + if (rx) {
> + memcpy_fromio(rx, bs->fifo, curr_step);
> + rx += curr_step;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int bcmbca_hsspi_setup(struct spi_device *spi)
> +{
> + struct bcmbca_hsspi *bs = spi_master_get_devdata(spi->master);
> + u32 reg;
> +
> + reg = __raw_readl(bs->regs +
> + HSSPI_PROFILE_SIGNAL_CTRL_REG(spi->chip_select));
> + reg &= ~(SIGNAL_CTRL_LAUNCH_RISING | SIGNAL_CTRL_LATCH_RISING);
> + if (spi->mode & SPI_CPHA)
> + reg |= SIGNAL_CTRL_LAUNCH_RISING;
> + else
> + reg |= SIGNAL_CTRL_LATCH_RISING;
> + __raw_writel(reg, bs->regs +
> + HSSPI_PROFILE_SIGNAL_CTRL_REG(spi->chip_select));
> +
> + mutex_lock(&bs->bus_mutex);
> + reg = __raw_readl(bs->regs + HSSPI_GLOBAL_CTRL_REG);
> +
> + if (spi->mode & SPI_CS_HIGH)
> + reg |= BIT(spi->chip_select);
> + else
> + reg &= ~BIT(spi->chip_select);
> + __raw_writel(reg, bs->regs + HSSPI_GLOBAL_CTRL_REG);
> +
> + if (spi->mode & SPI_CS_HIGH)
> + bs->cs_polarity |= BIT(spi->chip_select);
> + else
> + bs->cs_polarity &= ~BIT(spi->chip_select);
> +
> + mutex_unlock(&bs->bus_mutex);
> +
> + return 0;
> +}
> +
> +static int bcmbca_hsspi_transfer_one(struct spi_master *master,
> + struct spi_message *msg)
> +{
> + struct bcmbca_hsspi *bs = spi_master_get_devdata(master);
> + struct spi_transfer *t;
> + struct spi_device *spi = msg->spi;
> + int status = -EINVAL;
> +
> + mutex_lock(&bs->msg_mutex);
> + list_for_each_entry(t, &msg->transfers, transfer_list) {
> + status = bcmbca_hsspi_do_txrx(spi, t, msg);
> + if (status)
> + break;
> +
> + msg->actual_length += t->len;
> +
> + spi_transfer_delay_exec(t);
> + }
> +
> + mutex_unlock(&bs->msg_mutex);
> + msg->status = status;
> + spi_finalize_current_message(master);
> +
> + return 0;
> +}
> +
> +static irqreturn_t bcmbca_hsspi_interrupt(int irq, void *dev_id)
> +{
> + struct bcmbca_hsspi *bs = (struct bcmbca_hsspi *)dev_id;
> +
> + if (__raw_readl(bs->regs + HSSPI_INT_STATUS_MASKED_REG) == 0)
> + return IRQ_NONE;
> +
> + __raw_writel(HSSPI_INT_CLEAR_ALL, bs->regs + HSSPI_INT_STATUS_REG);
> + __raw_writel(0, bs->regs + HSSPI_INT_MASK_REG);
> +
> + complete(&bs->done);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int bcmbca_hsspi_probe(struct platform_device *pdev)
> +{
> + struct spi_master *master;
> + struct bcmbca_hsspi *bs;
> + struct resource *res_mem;
> + void __iomem *spim_ctrl;
> + void __iomem *regs;
> + struct device *dev = &pdev->dev;
> + struct clk *clk, *pll_clk = NULL;
> + int irq, ret;
> + u32 reg, rate, num_cs = HSSPI_SPI_MAX_CS;
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0)
> + return irq;
> +
> + res_mem = platform_get_resource_byname(pdev, IORESOURCE_MEM, "hsspi");
> + if (!res_mem)
> + return -EINVAL;
> + regs = devm_ioremap_resource(dev, res_mem);
> + if (IS_ERR(regs))
> + return PTR_ERR(regs);
> +
> + res_mem = platform_get_resource_byname(pdev, IORESOURCE_MEM, "spim-ctrl");
> + if (!res_mem)
> + return -EINVAL;
> + spim_ctrl = devm_ioremap_resource(dev, res_mem);
> + if (IS_ERR(spim_ctrl))
> + return PTR_ERR(spim_ctrl);
> +
> + clk = devm_clk_get(dev, "hsspi");
> + if (IS_ERR(clk))
> + return PTR_ERR(clk);
> +
> + ret = clk_prepare_enable(clk);
> + if (ret)
> + return ret;
> +
> + rate = clk_get_rate(clk);
> + if (!rate) {
> + pll_clk = devm_clk_get(dev, "pll");
> +
> + if (IS_ERR(pll_clk)) {
> + ret = PTR_ERR(pll_clk);
> + goto out_disable_clk;
> + }
> +
> + ret = clk_prepare_enable(pll_clk);
> + if (ret)
> + goto out_disable_clk;
> +
> + rate = clk_get_rate(pll_clk);
> + if (!rate) {
> + ret = -EINVAL;
> + goto out_disable_pll_clk;
> + }
> + }
> +
> + master = spi_alloc_master(&pdev->dev, sizeof(*bs));
> + if (!master) {
> + ret = -ENOMEM;
> + goto out_disable_pll_clk;
> + }
> +
> + bs = spi_master_get_devdata(master);
> + bs->pdev = pdev;
> + bs->clk = clk;
> + bs->pll_clk = pll_clk;
> + bs->regs = regs;
> + bs->spim_ctrl = spim_ctrl;
> + bs->speed_hz = rate;
> + bs->fifo = (u8 __iomem *) (bs->regs + HSSPI_FIFO_REG(0));
> + bs->wait_mode = HSSPI_WAIT_MODE_POLLING;
> +
> + mutex_init(&bs->bus_mutex);
> + mutex_init(&bs->msg_mutex);
> + init_completion(&bs->done);
> +
> + master->dev.of_node = dev->of_node;
> + if (!dev->of_node)
> + master->bus_num = HSSPI_BUS_NUM;
> +
> + of_property_read_u32(dev->of_node, "num-cs", &num_cs);
> + if (num_cs > 8) {
> + dev_warn(dev, "unsupported number of cs (%i), reducing to 8\n",
> + num_cs);
> + num_cs = HSSPI_SPI_MAX_CS;
> + }
> + master->num_chipselect = num_cs;
> + master->setup = bcmbca_hsspi_setup;
> + master->transfer_one_message = bcmbca_hsspi_transfer_one;
> + master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH |
> + SPI_RX_DUAL | SPI_TX_DUAL;
> + master->bits_per_word_mask = SPI_BPW_MASK(8);
> + master->auto_runtime_pm = true;
> +
> + platform_set_drvdata(pdev, master);
> +
> + /* Initialize the hardware */
> + __raw_writel(0, bs->regs + HSSPI_INT_MASK_REG);
> +
> + /* clean up any pending interrupts */
> + __raw_writel(HSSPI_INT_CLEAR_ALL, bs->regs + HSSPI_INT_STATUS_REG);
> +
> + /* read out default CS polarities */
> + reg = __raw_readl(bs->regs + HSSPI_GLOBAL_CTRL_REG);
> + bs->cs_polarity = reg & GLOBAL_CTRL_CS_POLARITY_MASK;
> + __raw_writel(reg | GLOBAL_CTRL_CLK_GATE_SSOFF,
> + bs->regs + HSSPI_GLOBAL_CTRL_REG);
> +
> + if (irq > 0) {
> + ret = devm_request_irq(dev, irq, bcmbca_hsspi_interrupt, IRQF_SHARED,
> + pdev->name, bs);
> + if (ret)
> + goto out_put_master;
> + }
> +
> + pm_runtime_enable(&pdev->dev);
> +
> + if (sysfs_create_group(&pdev->dev.kobj, &bcmbca_hsspi_group)) {
> + dev_err(&pdev->dev, "couldn't register sysfs group\n");
> + goto out_pm_disable;
> + }
> +
> + /* register and we are done */
> + ret = devm_spi_register_master(dev, master);
> + if (ret)
> + goto out_sysgroup_disable;
> +
> + dev_info(dev, "Broadcom BCMBCA High Speed SPI Controller driver");
> +
> + return 0;
> +
> +out_sysgroup_disable:
> + sysfs_remove_group(&pdev->dev.kobj, &bcmbca_hsspi_group);
> +out_pm_disable:
> + pm_runtime_disable(&pdev->dev);
> +out_put_master:
> + spi_master_put(master);
> +out_disable_pll_clk:
> + clk_disable_unprepare(pll_clk);
> +out_disable_clk:
> + clk_disable_unprepare(clk);
> + return ret;
> +}
> +
> +static int bcmbca_hsspi_remove(struct platform_device *pdev)
> +{
> + struct spi_master *master = platform_get_drvdata(pdev);
> + struct bcmbca_hsspi *bs = spi_master_get_devdata(master);
> +
> + /* reset the hardware and block queue progress */
> + __raw_writel(0, bs->regs + HSSPI_INT_MASK_REG);
> + clk_disable_unprepare(bs->pll_clk);
> + clk_disable_unprepare(bs->clk);
> + sysfs_remove_group(&pdev->dev.kobj, &bcmbca_hsspi_group);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int bcmbca_hsspi_suspend(struct device *dev)
> +{
> + struct spi_master *master = dev_get_drvdata(dev);
> + struct bcmbca_hsspi *bs = spi_master_get_devdata(master);
> +
> + spi_master_suspend(master);
> + clk_disable_unprepare(bs->pll_clk);
> + clk_disable_unprepare(bs->clk);
> +
> + return 0;
> +}
> +
> +static int bcmbca_hsspi_resume(struct device *dev)
> +{
> + struct spi_master *master = dev_get_drvdata(dev);
> + struct bcmbca_hsspi *bs = spi_master_get_devdata(master);
> + int ret;
> +
> + ret = clk_prepare_enable(bs->clk);
> + if (ret)
> + return ret;
> +
> + if (bs->pll_clk) {
> + ret = clk_prepare_enable(bs->pll_clk);
> + if (ret) {
> + clk_disable_unprepare(bs->clk);
> + return ret;
> + }
> + }
> +
> + spi_master_resume(master);
> +
> + return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(bcmbca_hsspi_pm_ops, bcmbca_hsspi_suspend,
> + bcmbca_hsspi_resume);
> +
> +static const struct of_device_id bcmbca_hsspi_of_match[] = {
> + { .compatible = "brcm,bcmbca-hsspi-v1.1", },
> + {},
> +};
> +
> +MODULE_DEVICE_TABLE(of, bcmbca_hsspi_of_match);
> +
> +static struct platform_driver bcmbca_hsspi_driver = {
> + .driver = {
> + .name = "bcmbca-hsspi",
> + .pm = &bcmbca_hsspi_pm_ops,
> + .of_match_table = bcmbca_hsspi_of_match,
> + },
> + .probe = bcmbca_hsspi_probe,
> + .remove = bcmbca_hsspi_remove,
> +};
> +
> +module_platform_driver(bcmbca_hsspi_driver);
> +
> +MODULE_ALIAS("platform:bcmbca_hsspi");
> +MODULE_DESCRIPTION("Broadcom BCMBCA High Speed SPI Controller driver");
> +MODULE_LICENSE("GPL");
> --
> 2.37.3
>
On Thu, Jan 26, 2023 at 04:15:00PM +0100, Jonas Gorski wrote:
> On Tue, 24 Jan 2023 at 23:33, William Zhang <[email protected]> wrote:
> >
> > Due to the controller limitation to keep the chip select low during the
> > bus idle time between the transfer, a dummy cs workaround was used when
> > this driver was first upstreamed to the kernel. It basically picks the
> > dummy cs as !actual_cs so typically dummy cs is 1 when most of the case
Please delete unneeded context from mails when replying. Doing this
makes it much easier to find your reply in the message, helping ensure
it won't be missed by people scrolling through the irrelevant quoted
material.
Hi Jonas, William,
On Thu, Jan 26, 2023 at 10:13 AM Jonas Gorski <[email protected]> wrote:
>
> On Tue, 24 Jan 2023 at 23:33, William Zhang <[email protected]> wrote:
> >
> > The kernel SPI interface includes the cs_change flag that alters how
> > the CS behaves.
> >
> > If we're in the middle of transfers, it tells us to unselect the
> > CS momentarily since the target device requires that.
> >
> > If we're at the end of a transfer, it tells us to keep the CS
> > selected, perhaps because the next transfer is likely targeted
> > to the same device.
> >
> > We implement this scheme in the HSSPI driver in this change.
> >
> > Prior to this change, the CS would toggle momentarily if cs_change
> > was set for the last transfer. This can be ignored by some or
> > most devices, but the Microchip TPM2 device does not ignore it.
> >
> > With the change, the behavior is corrected and the 'glitch' is
> > eliminated.
> >
> > Signed-off-by: Kursad Oney <[email protected]>
> > Signed-off-by: William Zhang <[email protected]>
> >
> > ---
> >
> > Changes in v2:
> > - Fix unused variable ‘reg’ compile warning
> >
> > drivers/spi/spi-bcm63xx-hsspi.c | 29 +++++++++++++++++++++--------
> > 1 file changed, 21 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/spi/spi-bcm63xx-hsspi.c b/drivers/spi/spi-bcm63xx-hsspi.c
> > index 55cbe7deba08..696e14abba2d 100644
> > --- a/drivers/spi/spi-bcm63xx-hsspi.c
> > +++ b/drivers/spi/spi-bcm63xx-hsspi.c
> > @@ -338,7 +338,7 @@ static int bcm63xx_hsspi_transfer_one(struct spi_master *master,
> > struct spi_device *spi = msg->spi;
> > int status = -EINVAL;
> > int dummy_cs;
> > - u32 reg;
> > + bool restore_polarity = true;
>
> While restore polarity is how this is implemented, I think using a
> more semantic name like keep_cs would be better.
This sounds reasonable to me.
>
> >
> > mutex_lock(&bs->msg_mutex);
> > /* This controller does not support keeping CS active during idle.
> > @@ -367,16 +367,29 @@ static int bcm63xx_hsspi_transfer_one(struct spi_master *master,
> >
> > spi_transfer_delay_exec(t);
> >
> > - if (t->cs_change)
> > + /*
> > + * cs_change rules:
> > + * (1) cs_change = 0 && last_xfer = 0:
> > + * Do not touch the CS. On to the next xfer.
> > + * (2) cs_change = 1 && last_xfer = 0:
> > + * Set cs = false before the next xfer.
> > + * (3) cs_change = 0 && last_xfer = 1:
> > + * We want CS to be deactivated. So do NOT set cs = false,
> > + * instead just restore the original polarity. This has the
> > + * same effect of deactivating the CS.
> > + * (4) cs_change = 1 && last_xfer = 1:
> > + * We want to keep CS active. So do NOT set cs = false, and
> > + * make sure we do NOT reverse polarity.
> > + */
> > + if (t->cs_change && !list_is_last(&t->transfer_list, &msg->transfers))
> > bcm63xx_hsspi_set_cs(bs, spi->chip_select, false);
> > +
> > + restore_polarity = !t->cs_change;
> > }
>
> I still find setting restore_polarity on each loop iteration when only
> its last set value matters confusing and hard to read, so I still
> propose keeping close to the generic implementation (
> https://elixir.bootlin.com/linux/v6.1.8/source/drivers/spi/spi.c#L1560
> ) and do
>
> if (t->cs_change) {
> if (list_is_last())
> restore_polarity = false;
> else
> bcm63xx_hsspi_set_cs(bs, spi->chip_select, false);
> }
OK I think this makes sense too but it might be a bit clearer to do:
if (list_is_last()) {
if (cs_change)
keep_cs = false;
else
bcm63xx_hsspi_set_cs(bs, spi->chip_select, false);
}
The gating condition here is when we reach the final transfer. But
list_is_last() is more expensive, so that's another consideration.
>
> While there, you might also want to check the cs_off value(s) as well.
Can you explain this please?
>
>
>
> >
> > - mutex_lock(&bs->bus_mutex);
> > - reg = __raw_readl(bs->regs + HSSPI_GLOBAL_CTRL_REG);
> > - reg &= ~GLOBAL_CTRL_CS_POLARITY_MASK;
> > - reg |= bs->cs_polarity;
> > - __raw_writel(reg, bs->regs + HSSPI_GLOBAL_CTRL_REG);
> > - mutex_unlock(&bs->bus_mutex);
> > + bcm63xx_hsspi_set_cs(bs, dummy_cs, false);
> > + if (restore_polarity)
> > + bcm63xx_hsspi_set_cs(bs, spi->chip_select, false);
> >
> > mutex_unlock(&bs->msg_mutex);
> > msg->status = status;
> > --
> > 2.37.3
> >
Hi Kursad,
On Thu, 26 Jan 2023 at 17:22, Kursad Oney <[email protected]> wrote:
> >
> > While there, you might also want to check the cs_off value(s) as well.
>
> Can you explain this please?
I'm talking about the transfer property cs_off:
" @cs_off: performs the transfer with chipselect off."
See how it is handled in the generic spi_transfer_one_message():
spi_set_cs(msg->spi, !xfer->cs_off, false);
...
list_for_each_entry(xfer, &msg->transfers, transfer_list) {
...
if (xfer->cs_change) {
if (list_is_last(&xfer->transfer_list,
&msg->transfers)) {
keep_cs = true;
} else {
if (!xfer->cs_off)
spi_set_cs(msg->spi, false, false);
_spi_transfer_cs_change_delay(msg, xfer);
if (!list_next_entry(xfer,
transfer_list)->cs_off)
spi_set_cs(msg->spi, true, false);
}
} else if (!list_is_last(&xfer->transfer_list,
&msg->transfers) &&
xfer->cs_off != list_next_entry(xfer,
transfer_list)->cs_off) {
spi_set_cs(msg->spi, xfer->cs_off, false);
}
...
}
if we fix the cs_change handling, we might as well bring it up to state.
In theory I would suggest to switch to implementing the set_cs() /
transfer_one() so you could let the core take care of all of that, but
that wouldn't work with dynamically switching to prepend mode. Might
be something for v1.1 though.
Regards
Jonas
On 01/26/2023 05:56 AM, Rob Herring wrote:
> On Wed, Jan 25, 2023 at 3:41 PM William Zhang
> <[email protected]> wrote:
>> On 01/25/2023 12:51 PM, Rob Herring wrote:
>>> On Wed, Jan 25, 2023 at 11:23:52AM -0800, William Zhang wrote:
>>>> On 01/24/2023 11:35 PM, Krzysztof Kozlowski wrote:
>>>>> On 24/01/2023 23:12, William Zhang wrote:
>>>>>> The new Broadcom Broadband BCMBCA SoCs includes a updated HSSPI
>>>>>> controller. Add new compatible strings to differentiate the old and new
>>>>>> controller while keeping MIPS based chip with the old compatible. Update
>>>>>> property requirements for these two revisions of the controller. Also
>>>>>> add myself and Kursad as the maintainers.
>
> [...]
>
>>>>>> properties:
>>>>>> compatible:
>>>>>> - const: brcm,bcm6328-hsspi
>>>>>> + oneOf:
>>>>>> + - const: brcm,bcm6328-hsspi
>>>>>> + - items:
>>>>>> + - enum:
>>>>>> + - brcm,bcm47622-hsspi
>>>>>> + - brcm,bcm4908-hsspi
>>>>>> + - brcm,bcm63138-hsspi
>>>>>> + - brcm,bcm63146-hsspi
>>>>>> + - brcm,bcm63148-hsspi
>>>>>> + - brcm,bcm63158-hsspi
>>>>>> + - brcm,bcm63178-hsspi
>>>>>> + - brcm,bcm6846-hsspi
>>>>>> + - brcm,bcm6856-hsspi
>>>>>> + - brcm,bcm6858-hsspi
>>>>>> + - brcm,bcm6878-hsspi
>>>>>> + - const: brcm,bcmbca-hsspi-v1.0
>>>>>> + - const: brcm,bcmbca-hsspi
>>>>>
>>>>> Why do you need "brcm,bcmbca-hsspi"? Nothing binds to it, so it's
>>>>> useless and very generic.
>>>>>
>>>> This was from Florian's suggestion and Broadcom's convention. See [1] and
>>>> you are okay with that [2]. I added the rev compatible and you were not
>>>> objecting it finally if I understand you correctly.
>>>
>>> Can you have a driver that only understands what 'brcm,bcmbca-hsspi' is
>>> work on all h/w that includes the compatible string? It doesn't seem
>>> like it since v1.1 is a completely new driver. Therefore
>>> 'brcm,bcmbca-hsspi' is pretty much useless.
>>>
>> 'brcm,bcmbca-hsspi' should be added to the binding table of
>> spi-bcm63xx-hsspi.c driver. This is the initial driver that works for
>> v1.0 controller. For v1.1 controller, yes it can fallback and work with
>> 1.0 driver spi-bcm63xx-hsspi.c simply not using the new feature in
>> v1.1(chip select signal control through software) and keeping using the
>> prepend mode or dummy cs workaround supported in 1.0 driver.
>
> If v1.1 is compatible with v1.0, then say that:
>
> soc-compat, "brcm,bcmbca-hsspi-v1.1", "brcm,bcmbca-hsspi-v1.0"
>
> IOW, 'brcm,bcmbca-hsspi' is redundant with 'brcm,bcmbca-hsspi-v1.0'.
> They have the same meaning. So pick which one you want to use. Not
> both.
>
Agree brcm,bcmbca-hsspi fallback is redundant to brcm,bcmbca-hsspi-v1.0.
I added it to conform Broadcom convention. But I understand your and
Krzystof's concern so I'll drop the brcm,bcmbca-hsspi in v3 to get
things going.
> Also, if that is the case, you shouldn't be introducing a whole new
> driver for v1.1.
>
Technically I can combine the new feature to the existing driver v1.0
but I prefer to start the a new driver so it will be much cleaner and
simpler to follow without all the workarounds and complex logic.
> Rob
>
On 01/26/2023 07:15 AM, Jonas Gorski wrote:
>>
>> +static bool bcm63xx_hsspi_mem_supports_op(struct spi_mem *mem,
>> + const struct spi_mem_op *op)
>> +{
>> + if (!spi_mem_default_supports_op(mem, op))
>> + return false;
>> +
>> + /* Controller doesn't support spi mem dual/quad read cmd in prepend mode */
>> + if ((op->cmd.opcode == 0xbb) || (op->cmd.opcode == 0xeb))
>
> There are defines in linux/mtd/spi-nor.h you can use:
>
> if ((op->cmd.opcode == SPINOR_OP_READ_1_2_2) || (op->cmd.opcode ==
> SPINOR_OP_READ_1_4_4))
>
> Though SPINOR_OP_READ_1_4_4 seems to be redundant, since the
> controller does not support quad mode, and does not advertise it, so
> it should never even be an option.
>
> Looking at that file, instead what is missing is
> SPINOR_OP_READ_1_2_2_4B (0xbc) which shouldn't be usable either.
>
You are right. I was only looking at the spi nand header which does not
have this clear definition. Will use them and drop the quad IO opcode
for the reason you mentioned.
On 01/26/2023 07:16 AM, Jonas Gorski wrote:
>> +static void bcmbca_hsspi_set_cs(struct bcmbca_hsspi *bs, unsigned int cs,
>> + bool active)
>> +{
>> + u32 reg;
>> +
>> + /* No cs orerriden needed for SS7 internal cs on pcm based voice dev */
>> + if (cs == 7)
>> + return;
>> +
>> + mutex_lock(&bs->bus_mutex);
>> +
>> + if (active) {
>> + /* activate cs by setting the override bit and active value bit*/
>> + reg = __raw_readl(bs->spim_ctrl);
>> + reg |= BIT(cs+SPIM_CTRL_CS_OVERRIDE_SEL_SHIFT);
>
> Doesn't checkpatch complain about missing spaces around the operator (+)?
>
Nope...
>> + reg &= ~BIT(cs+SPIM_CTRL_CS_OVERRIDE_VAL_SHIFT);
>> + if (bs->cs_polarity & BIT(cs))
>> + reg |= BIT(cs+SPIM_CTRL_CS_OVERRIDE_VAL_SHIFT);
>
> Does the CS_OVERRIDE_VAL get reset on clearing the OVERRIDE_SEL bit or
> it is persistent? If the latter, you could initialize its value in
> bcmbca_hsspi_setup() and then this becomes:
>
> reg = _raw_readl(bs->spim_ctrl);
> if (active)
> reg |= BIT(cs+SPIM_CTRL_CS_OVERRIDE_SEL_SHIFT);
> else
> reg &= ~BIT(cs + SPIM_CTRL_CS_OVERRIDE_SEL_SHIFT);
> __raw_writel(reg, bs->spim_ctrl);
>
> and you can drop the cs_polarity field.
>
>
No. So I will try this optimization and see if that actually works in
the controller.
>> + __raw_writel(reg, bs->spim_ctrl);
>> + } else {
>> + /* clear the cs override bit */
>> + reg = __raw_readl(bs->spim_ctrl);
>> + reg &= ~BIT(cs+SPIM_CTRL_CS_OVERRIDE_SEL_SHIFT);
>> + __raw_writel(reg, bs->spim_ctrl);
>> + }
>> +
>> + mutex_unlock(&bs->bus_mutex);
>> +}
>> +
On 01/26/2023 07:15 AM, Jonas Gorski wrote:
>> +static int bcm63xx_hsspi_wait_cmd(struct bcm63xx_hsspi *bs)
>> +{
>> + unsigned long limit;
>> + u32 reg = 0;
>> + int rc = 0;
>
> If the only possible return values are 0 and 1, maybe this should be a bool?
>
Well it is possible we may want to return to some specific error code so
I would prefer to keep it as is.
>> +
>> + if (bs->wait_mode == HSSPI_WAIT_MODE_INTR) {
>> + if (wait_for_completion_timeout(&bs->done, HZ) == 0)
>> + rc = 1;
>> + } else {
>> + /* polling mode checks for status busy bit */
>> + limit = jiffies + msecs_to_jiffies(HSSPI_POLL_STATUS_TIMEOUT_MS);
>> +
>> + while (!time_after(jiffies, limit)) {
>> + reg = __raw_readl(bs->regs + HSSPI_PINGPONG_STATUS_REG(0));
>> + if (reg & HSSPI_PINGPONG_STATUS_SRC_BUSY)
>> + cpu_relax();
>> + else
>> + break;
>> + }
>> + if (reg & HSSPI_PINGPONG_STATUS_SRC_BUSY)
>> + rc = 1;
>> + }
>> +
>> + if (rc)
>> + dev_err(&bs->pdev->dev, "transfer timed out!\n");
>> +
>> + return rc;
>> +}
>> +
>> +static bool bcm63xx_check_msg_prependable(struct spi_master *master,
>> + struct spi_message *msg,
>> + struct spi_transfer *t_prepend)
>
> This function does more than just checking, so I think a more
> appropriate name would be something like
>
> bcm63xx_prepare_prepend_transfer()
>
That's reasonable. The function kind of evolved from the checking only.
>> +{
>> +
>> + struct bcm63xx_hsspi *bs = spi_master_get_devdata(master);
>> + bool prepend = false, tx_only = false;
>> + struct spi_transfer *t;
>> +
>> + /* If it is forced cs dummy workaround mode, no need to prepend message */
>> + if (bs->xfer_mode == HSSPI_XFER_MODE_DUMMYCS)
>> + return false;
>
> That's a weird point for that, why not just move this to the caller
> and check it before calling the function.
>
True following your above function name change suggestion.
>> +
>> + /*
>> + * Multiple transfers within a message may be combined into one transfer
>> + * to the controller using its prepend feature. A SPI message is prependable
>> + * only if the following are all true:
>> + * 1. One or more half duplex write transfer in single bit mode
>> + * 2. Optional full duplex read/write at the end
>> + * 3. No delay and cs_change between transfers
>> + */
>> + bs->prepend_cnt = 0;
>> + list_for_each_entry(t, &msg->transfers, transfer_list) {
>> + if ((spi_delay_to_ns(&t->delay, t) > 0) || t->cs_change) {
>> + dev_warn(&bs->pdev->dev,
>> + "Delay or cs change not supported in prepend mode!\n");
>
> I don't think warn is the right level. If we are on XFER_MODE_AUTO,
> this should be _dbg, since we will just fall back to the dummy cs
> mode, if we are on XFER_MODE_PREPEND, this should be dev_err, since we
> cannot do the message.
>
I was relying on this to see the message when we fall back. But
certainly I can fine tune the message level as you suggested
> cs->change is technically supported when all that's requested is a
> between transfer cs toggle (t->cs_change is true, t->cs_off is false
> and next transfer's cs_off is also false), which automatically happens
> after the transfer. Not sure if it is worth the effort implementing
> that though.
>
If this cs toggling is between the transfers that we are merging here,
then no as the cs will be always active during merged transfer in
prepend mode.
>> + break;
>> + }
>> +
>> + tx_only = false;
>> + if (t->tx_buf && !t->rx_buf) {
>> + tx_only = true;
>> + if (bs->prepend_cnt + t->len >
>> + (HSSPI_BUFFER_LEN - HSSPI_OPCODE_LEN)) {
>> + dev_warn(&bs->pdev->dev,
>> + "exceed max buf len, abort prepending transfers!\n");
>> + break;
>
> why not just return false here directly? And everywhere else where you
> decided that you cannot use prepend.
> Sure I can eliminate the prepend variable and return directly
>> + }
>> +
>> + if (t->tx_nbits > SPI_NBITS_SINGLE &&
>> + !list_is_last(&t->transfer_list, &msg->transfers)) {
>> + dev_warn(&bs->pdev->dev,
>> + "multi-bit prepend buf not supported!\n");
>> + break;
>> + }
>> +
>> + if (t->tx_nbits == SPI_NBITS_SINGLE) {
>> + memcpy(bs->prepend_buf + bs->prepend_cnt, t->tx_buf, t->len);
>> + bs->prepend_cnt += t->len;
>> + }
>> + } else {
>> + if (!list_is_last(&t->transfer_list, &msg->transfers)) {
>> + dev_warn(&bs->pdev->dev,
>> + "rx/tx_rx transfer not supported when it is not last one!\n");
>
> This is only an issue if doing multi-bit RX/TX; for single bit you can
> just upgrade the whole transfer/message to duplex, you just need to
> pick the read bytes then from the right offsets.
>
I am not sure if this will work all the case. Considering two transfers
rx 3 bytes then tx 3 bytes in the message(not sure if there is any
device requires this kind of message but just for discussion
purpose...), if I upgrade it to duplex message, the controller will
transfer and receive 6 bytes data in duplex mode where prepend count is
zero. So the extra 3 tx bytes while receiving the first 3 bytes may
disturb the device as they are not expected. It may or may not work. I
would rather just fallback to dummy cs workaround instead of causing
potentially issue.
>> + break;
>> + }
>> + }
>> +
>> + if (list_is_last(&t->transfer_list, &msg->transfers)) {
>> + memcpy(t_prepend, t, sizeof(struct spi_transfer));
>> +
>> + if (tx_only && t->tx_nbits == SPI_NBITS_SINGLE) {
>> + /*
>> + * if the last one is also a single bit tx only transfer, merge
>> + * all of them into one single tx transfer
>> + */
>> + t_prepend->len = bs->prepend_cnt;
>> + t_prepend->tx_buf = bs->prepend_buf;
>> + bs->prepend_cnt = 0;
>> + } else {
>> + /*
>> + * if the last one is not a tx only transfer or dual tx xfer, all
>> + * the previous transfers are sent through prepend bytes and
>> + * make sure it does not exceed the max prepend len
>> + */
>> + if (bs->prepend_cnt > HSSPI_MAX_PREPEND_LEN) {
>> + dev_warn(&bs->pdev->dev,
>> + "exceed max prepend len, abort prepending transfers!\n");
>> + break;
>
> Likewise, you can merge any amount or rx/tx/rxtx single bit transfers
> together as a duplex transfer with prepend len set to 0 (so
> technically not a prepend anymore ;-)
Same here. Let's just fallback to dummy cs workaround mode.
>
>> + }
>> + }
>> + prepend = true;
>> + }
>> + }
>> +
>> + return prepend;
>
> and then if you already returned false if you cannot do prepend, you
> just need to return true here and don't need the prepend variable.
>
>> +}
>> +
>>
>> static int bcm63xx_hsspi_setup(struct spi_device *spi)
>> @@ -344,9 +578,23 @@ static int bcm63xx_hsspi_transfer_one(struct spi_master *master,
>> int status = -EINVAL;
>> int dummy_cs;
>> bool restore_polarity = true;
>> + struct spi_transfer t_prepend;
>>
>> mutex_lock(&bs->msg_mutex);
>> - /* This controller does not support keeping CS active during idle.
>> + if (bcm63xx_check_msg_prependable(master, msg, &t_prepend)) {
>> + status = bcm63xx_hsspi_do_prepend_txrx(spi, &t_prepend);
>> + msg->actual_length += (t_prepend.len + bs->prepend_cnt);
>
> why +=? shouldn't this be the only place in this case where this is set?
>
probably copy/paste error from the dummy cs loop. Will fix.
>> + goto msg_done;
>> + }
>> +
>> + if (bs->xfer_mode == HSSPI_XFER_MODE_PREPEND) {
>> + dev_warn(&bs->pdev->dev,
>> + "User set prepend mode but msg not prependable! Fail the xfer!\n");
>
> If we are failing, this should be a dev_err, not a dev_warn
>
Will fix.
>> + goto msg_done;
>> + }
>
> I think from a readability standpoint it would be better to move the
> cs_workaround parts into their own function, and have this as
>
> bool prependable = false;
>
> if (bs->xfer_mode != HSSPI_XFER_MODE_DUMMYCS)
> prependable = bcm63xx_prepare_prepend_transfer(...);
>
> if (prependable) {
> status = bcm63xx_hsspi_do_prepend_txrx(...);
> msg->actual_legth += ...;
> } else {
> if (bs->xfer_mode == HSSPI_XFER_MODE_PREPEND) {
> /* we may not use dummy cs */
> dev_err(...);
> status = -EINVAL;
> } else {
> status = bcm63xx_hsspi_do_dummy_cs_txrx(...);
> }
> }
>
> with (bcm63xx_hsspi_do_dummy_cs_txrx being the proposed function name).
>
Sound good to me.
>> +
>> + /*
>> + * This controller does not support keeping CS active during idle.
>> * To work around this, we use the following ugly hack:
>> *
>> * a. Invert the target chip select's polarity so it will be active.
>> @@ -364,6 +612,17 @@ static int bcm63xx_hsspi_transfer_one(struct spi_master *master,
>> bcm63xx_hsspi_set_cs(bs, dummy_cs, true);
>>
>> list_for_each_entry(t, &msg->transfers, transfer_list) {
>> + /*
>> + * We are here because one of reasons below:
>> + * a. Message is not prependable and in default auto xfer mode. This mean
>> + * we fallback to dummy cs mode at maximum 25MHz safe clock rate.
>> + * b. User set to use the dummy cs mode.
>> + */
>> + if (bs->xfer_mode == HSSPI_XFER_MODE_AUTO) {
>> + if (t->speed_hz > HSSPI_MAX_SYNC_CLOCK)
>> + t->speed_hz = HSSPI_MAX_SYNC_CLOCK;
>
> OTOH, this may be a point where a dev_warn (once?) might be a good
> idea, since the device may depend on a certain speed to avoid buffer
> overruns (e.g. audio streams - not sure if that exists), so a warning
> that the transfer speed was reduced will help identifying the source.
>
>
That make sense. Should add a warning here.
>
>> + }
>> +
>> status = bcm63xx_hsspi_do_txrx(spi, t);
>> if (status)
>> break;
On 01/26/2023 09:33 AM, Jonas Gorski wrote:
> Hi Kursad,
>
> On Thu, 26 Jan 2023 at 17:22, Kursad Oney <[email protected]> wrote:
>>>
>>> While there, you might also want to check the cs_off value(s) as well.
>>
>> Can you explain this please?
>
> I'm talking about the transfer property cs_off:
>
> " @cs_off: performs the transfer with chipselect off."
>
> See how it is handled in the generic spi_transfer_one_message():
>
> spi_set_cs(msg->spi, !xfer->cs_off, false);
> ...
> list_for_each_entry(xfer, &msg->transfers, transfer_list) {
> ...
> if (xfer->cs_change) {
> if (list_is_last(&xfer->transfer_list,
> &msg->transfers)) {
> keep_cs = true;
> } else {
> if (!xfer->cs_off)
> spi_set_cs(msg->spi, false, false);
> _spi_transfer_cs_change_delay(msg, xfer);
> if (!list_next_entry(xfer,
> transfer_list)->cs_off)
> spi_set_cs(msg->spi, true, false);
> }
> } else if (!list_is_last(&xfer->transfer_list,
> &msg->transfers) &&
> xfer->cs_off != list_next_entry(xfer,
> transfer_list)->cs_off) {
> spi_set_cs(msg->spi, xfer->cs_off, false);
> }
> ...
> }
>
> if we fix the cs_change handling, we might as well bring it up to state.
>
We can blindly port this logic over but this cs_off stuff (from the
spi.h comment @cs_off: performs the transfer with chipselect off) sounds
weird. What kind of device do transfer when cs is off? I don't have any
device like this to test.
> In theory I would suggest to switch to implementing the set_cs() /
> transfer_one() so you could let the core take care of all of that, but
> that wouldn't work with dynamically switching to prepend mode. Might
> be something for v1.1 though.
>
That is good idea and I can certainly try that.
>
> Regards
> Jonas
>