[PATCH v2 0/6] spi: Adding support for Microchip Sparx5 SoC

[PATCH v2 0/6] spi: Adding support for Microchip Sparx5 SoC

[Lars Povlsen]

This is an add-on series to the main SoC Sparx5 series
(Message-ID: <20200615133242.24911-1-lars.povlsen@microchip.com>

The series add support for the Sparx5 SoC SPI controller in the
spi-dw-mmio.c spi driver.

v2 changes:
- Moved all RX sample delay into spi-dw-core.c, using
  the "snps,rx-sample-delay-ns" device property.
- Integrated Sparx5 support directly in spi-dw-mmio.c
- Changed SPI2 configuration to per-slave "microchip,spi-interface2"
  property.
- Added bindings to existing snps,dw-apb-ssi.yaml file
- Dropped patches for polled mode and SPI memory operations.

Lars Povlsen (6):
  spi: dw: Add support for RX sample delay register
  arm64: dts: sparx5: Add SPI controller
  spi: dw: Add Microchip Sparx5 support
  dt-bindings: snps,dw-apb-ssi: Add sparx5, SPI slave
    snps,rx-sample-delay-ns and microchip,spi-interface2 properties.
  arm64: dts: sparx5: Add spi-nor support
  arm64: dts: sparx5: Add spi-nand devices

 .../bindings/spi/snps,dw-apb-ssi.yaml         |  24 ++++
 arch/arm64/boot/dts/microchip/sparx5.dtsi     |  34 ++++++
 .../boot/dts/microchip/sparx5_pcb125.dts      |  16 +++
 .../boot/dts/microchip/sparx5_pcb134.dts      |  22 ++++
 .../dts/microchip/sparx5_pcb134_board.dtsi    |   9 ++
 .../boot/dts/microchip/sparx5_pcb135.dts      |  23 ++++
 .../dts/microchip/sparx5_pcb135_board.dtsi    |   9 ++
 drivers/spi/spi-dw-core.c                     |  20 ++++
 drivers/spi/spi-dw-mmio.c                     | 113 +++++++++++++++++-
 drivers/spi/spi-dw.h                          |   2 +
 10 files changed, 271 insertions(+), 1 deletion(-)

--
2.27.0

[PATCH v2 1/6] spi: dw: Add support for RX sample delay register

[Lars Povlsen]

This add support for the RX_SAMPLE_DLY register. If enabled in the
Designware IP, it allows tuning of the rx data signal by means of an
internal rx sample fifo.

The register is controlled by the snps,rx-sample-delay-ns DT
property, which is defined per SPI slave.

The register is located at offset 0xf0, and if the option is not
enabled in the IP, changing the register will have no effect. The
register will only be written if any slave defines a nonzero value
(after scaling by the clock period).

Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
---
 drivers/spi/spi-dw-core.c | 20 ++++++++++++++++++++
 drivers/spi/spi-dw.h      |  2 ++
 2 files changed, 22 insertions(+)

diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
index 323c66c5db506..d249f25cbff7f 100644
--- a/drivers/spi/spi-dw-core.c
+++ b/drivers/spi/spi-dw-core.c
@@ -12,6 +12,7 @@
 #include <linux/delay.h>
 #include <linux/slab.h>
 #include <linux/spi/spi.h>
+#include <linux/of.h>
 
 #include "spi-dw.h"
 
@@ -26,6 +27,8 @@ struct chip_data {
 
 	u16 clk_div;		/* baud rate divider */
 	u32 speed_hz;		/* baud rate */
+
+	u32 rx_sample_dly;	/* RX sample delay */
 };
 
 #ifdef CONFIG_DEBUG_FS
@@ -52,6 +55,7 @@ static const struct debugfs_reg32 dw_spi_dbgfs_regs[] = {
 	DW_SPI_DBGFS_REG("DMACR", DW_SPI_DMACR),
 	DW_SPI_DBGFS_REG("DMATDLR", DW_SPI_DMATDLR),
 	DW_SPI_DBGFS_REG("DMARDLR", DW_SPI_DMARDLR),
+	DW_SPI_DBGFS_REG("RX_SAMPLE_DLY", DW_SPI_RX_SAMPLE_DLY),
 };
 
 static int dw_spi_debugfs_init(struct dw_spi *dws)
@@ -328,6 +332,12 @@ static int dw_spi_transfer_one(struct spi_controller *master,
 	if (master->can_dma && master->can_dma(master, spi, transfer))
 		dws->dma_mapped = master->cur_msg_mapped;
 
+	/* Update RX sample delay if required */
+	if (dws->curr_rx_sample_dly != chip->rx_sample_dly) {
+		dw_writel(dws, DW_SPI_RX_SAMPLE_DLY, chip->rx_sample_dly);
+		dws->curr_rx_sample_dly = chip->rx_sample_dly;
+	}
+
 	/* For poll mode just disable all interrupts */
 	spi_mask_intr(dws, 0xff);
 
@@ -380,10 +390,20 @@ static int dw_spi_setup(struct spi_device *spi)
 	/* Only alloc on first setup */
 	chip = spi_get_ctldata(spi);
 	if (!chip) {
+		struct dw_spi *dws = spi_controller_get_devdata(spi->controller);
+		u32 rx_sample_dly;
+
 		chip = kzalloc(sizeof(struct chip_data), GFP_KERNEL);
 		if (!chip)
 			return -ENOMEM;
 		spi_set_ctldata(spi, chip);
+		/* Is rx_sample_dly defined for a slave? */
+		if (device_property_read_u32(&spi->dev,
+					     "snps,rx-sample-delay-ns",
+					     &rx_sample_dly) == 0)
+			chip->rx_sample_dly = DIV_ROUND_CLOSEST(rx_sample_dly,
+								NSEC_PER_SEC /
+								dws->max_freq);
 	}
 
 	chip->tmode = SPI_TMOD_TR;
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index 151ba316619e6..f9243bf2a662b 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -34,6 +34,7 @@
 #define DW_SPI_IDR			0x58
 #define DW_SPI_VERSION			0x5c
 #define DW_SPI_DR			0x60
+#define DW_SPI_RX_SAMPLE_DLY		0xf0
 #define DW_SPI_CS_OVERRIDE		0xf4
 
 /* Bit fields in CTRLR0 */
@@ -140,6 +141,7 @@ struct dw_spi {
 	u8			n_bytes;	/* current is a 1/2 bytes op */
 	irqreturn_t		(*transfer_handler)(struct dw_spi *dws);
 	u32			current_freq;	/* frequency in hz */
+	u32			curr_rx_sample_dly;
 
 	/* DMA info */
 	struct dma_chan		*txchan;
-- 
2.27.0

[PATCH v2 2/6] arm64: dts: sparx5: Add SPI controller

[Lars Povlsen]

This adds a SPI controller to the Microchip Sparx5 SoC

Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
---
 arch/arm64/boot/dts/microchip/sparx5.dtsi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm64/boot/dts/microchip/sparx5.dtsi b/arch/arm64/boot/dts/microchip/sparx5.dtsi
index 7e811e24f0e99..2404bcc08b89d 100644
--- a/arch/arm64/boot/dts/microchip/sparx5.dtsi
+++ b/arch/arm64/boot/dts/microchip/sparx5.dtsi
@@ -14,6 +14,7 @@ / {
 	#size-cells = <1>;
 
 	aliases {
+		spi0 = &spi0;
 		serial0 = &uart0;
 		serial1 = &uart1;
 	};
@@ -155,6 +156,19 @@ uart1: serial@600102000 {
 			status = "disabled";
 		};
 
+		spi0: spi@600104000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "microchip,sparx5-spi";
+			reg = <0x6 0x00104000 0x40>;
+			num-cs = <16>;
+			reg-io-width = <4>;
+			reg-shift = <2>;
+			clocks = <&ahb_clk>;
+			interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
+			status = "disabled";
+		};
+
 		timer1: timer@600105000 {
 			compatible = "snps,dw-apb-timer";
 			reg = <0x6 0x00105000 0x1000>;
-- 
2.27.0

[PATCH v2 3/6] spi: dw: Add Microchip Sparx5 support

[Lars Povlsen]

This adds SPI support for the Sparx5 SoC, which is using the MMIO
Designware SPI controller.

The Sparx5 differs from the Ocelot version in these areas:

 * The Sparx5 SPI controller has 2 different SPI bus interfaces on the
   same controller (don't ask...). As each SPI slave is physically
   located on a particular bus, they must be configured
   accordingly. The microchip,spi-interface2 property is used for
   this. Switching between busses also requires specific
   handling/timing.

 * The CS override is controlled by a new set of registers for
   this purpose.

 * The Sparx5 SPI controller has the RX sample delay register, and it
   must be configured for the (SPI NAND) device on SPI2.

Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
---
 drivers/spi/spi-dw-mmio.c | 113 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 112 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
index 403403deae664..78241d93289f5 100644
--- a/drivers/spi/spi-dw-mmio.c
+++ b/drivers/spi/spi-dw-mmio.c
@@ -20,6 +20,7 @@
 #include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/reset.h>
+#include <linux/bitfield.h>
 
 #include "spi-dw.h"
 
@@ -41,6 +42,12 @@ struct dw_spi_mmio {
 #define MSCC_IF_SI_OWNER_SIBM			1
 #define MSCC_IF_SI_OWNER_SIMC			2
 
+#define SPARX5_CPU_SYSTEM_CTRL_GENERAL_CTRL	0x88
+#define SPARX5_IF_SI_OWNER			GENMASK(7, 6)
+#define SPARX5_IF_SI2_OWNER			GENMASK(5, 4)
+#define SPARX5_FORCE_ENA			0xa4
+#define SPARX5_FORCE_VAL			0xa8
+
 #define MSCC_SPI_MST_SW_MODE			0x14
 #define MSCC_SPI_MST_SW_MODE_SW_PIN_CTRL_MODE	BIT(13)
 #define MSCC_SPI_MST_SW_MODE_SW_SPI_CS(x)	(x << 5)
@@ -54,7 +61,8 @@ struct dw_spi_mmio {
 
 struct dw_spi_mscc {
 	struct regmap       *syscon;
-	void __iomem        *spi_mst;
+	void __iomem        *spi_mst; /* Not sparx5 */
+	u32		    if2mask;  /* sparx5 only */
 };
 
 /*
@@ -134,6 +142,108 @@ static int dw_spi_mscc_jaguar2_init(struct platform_device *pdev,
 				JAGUAR2_IF_SI_OWNER_OFFSET);
 }
 
+/*
+ * Set the owner of the SPI interface
+ */
+static void dw_spi_sparx5_set_owner(struct regmap *syscon,
+				    u8 owner, u8 owner2)
+{
+	u32 val, msk;
+
+	val = FIELD_PREP(SPARX5_IF_SI_OWNER, owner) |
+		FIELD_PREP(SPARX5_IF_SI2_OWNER, owner2);
+	msk = SPARX5_IF_SI_OWNER | SPARX5_IF_SI2_OWNER;
+	regmap_update_bits(syscon,
+			   SPARX5_CPU_SYSTEM_CTRL_GENERAL_CTRL,
+			   msk, val);
+}
+
+static void dw_spi_sparx5_set_cs_owner(struct dw_spi_mmio *dwsmmio,
+				       u8 cs, u8 owner)
+{
+	struct dw_spi_mscc *dwsmscc = dwsmmio->priv;
+	u8 other = (owner == MSCC_IF_SI_OWNER_SIBM ?
+		    MSCC_IF_SI_OWNER_SIMC : MSCC_IF_SI_OWNER_SIBM);
+	if (dwsmscc->if2mask & BIT(cs))
+		/* SPI2 */
+		dw_spi_sparx5_set_owner(dwsmscc->syscon, other, owner);
+	else
+		/* SPI1 */
+		dw_spi_sparx5_set_owner(dwsmscc->syscon, owner, other);
+}
+
+/*
+ * The Designware SPI controller (referred to as master in the
+ * documentation) automatically deasserts chip select when the tx fifo
+ * is empty. The chip selects then needs to be driven by a CS override
+ * register. enable is an active low signal.
+ */
+static void dw_spi_sparx5_set_cs(struct spi_device *spi, bool nEnable)
+{
+	struct dw_spi *dws = spi_master_get_devdata(spi->master);
+	struct dw_spi_mmio *dwsmmio = container_of(dws, struct dw_spi_mmio, dws);
+	struct dw_spi_mscc *dwsmscc = dwsmmio->priv;
+	u8 cs = spi->chip_select;
+
+	if (!nEnable)
+		dw_spi_sparx5_set_cs_owner(dwsmmio, cs,
+					   MSCC_IF_SI_OWNER_SIMC);
+
+	if (!nEnable) {
+		/* Ensure CS toggles, so start off all disabled */
+		regmap_write(dwsmscc->syscon, SPARX5_FORCE_VAL, ~0);
+		/* CS override drive enable */
+		regmap_write(dwsmscc->syscon, SPARX5_FORCE_ENA, 1);
+		/* Allow settle */
+		udelay(1);
+		/* Now set CSx enabled */
+		regmap_write(dwsmscc->syscon, SPARX5_FORCE_VAL, ~BIT(cs));
+	} else {
+		/* CS value */
+		regmap_write(dwsmscc->syscon, SPARX5_FORCE_VAL, ~0);
+		/* CS override drive disable */
+		regmap_write(dwsmscc->syscon, SPARX5_FORCE_ENA, 0);
+	}
+
+	dw_spi_set_cs(spi, nEnable);
+}
+
+static int dw_spi_mscc_sparx5_init(struct platform_device *pdev,
+				   struct dw_spi_mmio *dwsmmio)
+{
+	const char *syscon_name = "microchip,sparx5-cpu-syscon";
+	struct dw_spi_mscc *dwsmscc;
+	struct device_node *nc;
+
+	dwsmscc = devm_kzalloc(&pdev->dev, sizeof(*dwsmscc), GFP_KERNEL);
+	if (!dwsmscc)
+		return -ENOMEM;
+
+	dwsmscc->syscon =
+		syscon_regmap_lookup_by_compatible(syscon_name);
+	if (IS_ERR(dwsmscc->syscon)) {
+		dev_err(&pdev->dev, "No syscon map %s\n", syscon_name);
+		return PTR_ERR(dwsmscc->syscon);
+	}
+
+	/* SPI2 mapping bitmask */
+	for_each_available_child_of_node(pdev->dev.of_node, nc) {
+		u32 cs;
+
+		if (of_property_read_u32(nc, "reg", &cs) == 0 &&
+		    of_property_read_bool(nc, "microchip,spi-interface2"))
+			dwsmscc->if2mask |= BIT(cs);
+	}
+
+	dwsmmio->dws.set_cs = dw_spi_sparx5_set_cs;
+	dwsmmio->priv = dwsmscc;
+
+	/* Register hook to configure CTRLR0 */
+	dwsmmio->dws.update_cr0 = dw_spi_update_cr0;
+
+	return 0;
+}
+
 static int dw_spi_alpine_init(struct platform_device *pdev,
 			      struct dw_spi_mmio *dwsmmio)
 {
@@ -297,6 +407,7 @@ static const struct of_device_id dw_spi_mmio_of_match[] = {
 	{ .compatible = "renesas,rzn1-spi", .data = dw_spi_dw_apb_init},
 	{ .compatible = "snps,dwc-ssi-1.01a", .data = dw_spi_dwc_ssi_init},
 	{ .compatible = "intel,keembay-ssi", .data = dw_spi_keembay_init},
+	{ .compatible = "microchip,sparx5-spi", dw_spi_mscc_sparx5_init},
 	{ /* end of table */}
 };
 MODULE_DEVICE_TABLE(of, dw_spi_mmio_of_match);
-- 
2.27.0

[PATCH v2 4/6] dt-bindings: snps,dw-apb-ssi: Add sparx5, SPI slave snps,rx-sample-delay-ns and microchip,spi-interface2 properties.

[Lars Povlsen]

This has the following changes for the snps,dw-apb-ss DT bindings:

- Add "microchip,sparx5-spi" as the compatible for the Sparx5 SoC
controller,

- Add the property "snps,rx-sample-delay-ns" for SPI slaves

- Add the property "microchip,spi-interface2" for SPI slaves

Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
---
 .../bindings/spi/snps,dw-apb-ssi.yaml         | 24 +++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
index c62cbe79f00dd..5bca4f0493bdf 100644
--- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
+++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
@@ -36,6 +36,11 @@ properties:
               - mscc,ocelot-spi
               - mscc,jaguar2-spi
           - const: snps,dw-apb-ssi
+      - description: Microchip Sparx5 SoC SPI Controller
+        items:
+          - enum:
+              - microchip,sparx5-spi
+          - const: snps,dw-apb-ssi
       - description: Amazon Alpine SPI Controller
         const: amazon,alpine-dw-apb-ssi
       - description: Renesas RZ/N1 SPI Controller
@@ -107,6 +112,19 @@ patternProperties:
       spi-tx-bus-width:
         const: 1

+      snps,rx-sample-delay-ns:
+        description: SPI Rx sample delay offset, unit is nanoseconds.
+          The delay from the default sample time before the actual
+          sample of the rxd input signal occurs. The "rx_sample_delay"
+          is an optional feature of the designware controller, and the
+          upper limit is also subject to controller configuration.
+        $ref: /schemas/types.yaml#/definitions/uint32
+
+      microchip,spi-interface2:
+        description: indicates the spi device is placed on a special
+          controller interface of the "microchip,sparx5-spi" controller.
+        type: boolean
+
 unevaluatedProperties: false

 required:
@@ -129,5 +147,11 @@ examples:
       num-cs = <2>;
       cs-gpios = <&gpio0 13 0>,
                  <&gpio0 14 0>;
+        spi-flash@1 {
+          compatible = "spi-nand";
+          reg = <1>;
+          microchip,spi-interface2;
+          snps,rx-sample-delay-ns = <7>;
+        };
     };
 ...
--
2.27.0

[PATCH v2 5/6] arm64: dts: sparx5: Add spi-nor support

[Lars Povlsen]

This add spi-nor device nodes to the Sparx5 reference boards.

Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
---
 arch/arm64/boot/dts/microchip/sparx5_pcb125.dts        | 9 +++++++++
 arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi | 9 +++++++++
 arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi | 9 +++++++++
 3 files changed, 27 insertions(+)

diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts b/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts
index 573309fe45823..d8b5d23abfab0 100644
--- a/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts
+++ b/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts
@@ -39,6 +39,15 @@ &sdhci0 {
 	microchip,clock-delay = <10>;
 };
 
+&spi0 {
+	status = "okay";
+	spi-flash@0 {
+		compatible = "jedec,spi-nor";
+		spi-max-frequency = <8000000>; /* input clock */
+		reg = <0>; /* CS0 */
+	};
+};
+
 &i2c1 {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi b/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi
index 18a535a043686..628a05d3f57ce 100644
--- a/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi
+++ b/arch/arm64/boot/dts/microchip/sparx5_pcb134_board.dtsi
@@ -38,6 +38,15 @@ gpio-restart {
 	};
 };
 
+&spi0 {
+	status = "okay";
+	spi-flash@0 {
+		compatible = "jedec,spi-nor";
+		spi-max-frequency = <8000000>;
+		reg = <0>;
+	};
+};
+
 &gpio {
 	i2cmux_pins_i: i2cmux-pins-i {
 	       pins = "GPIO_16", "GPIO_17", "GPIO_18", "GPIO_19",
diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi b/arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi
index d71f11a10b3d2..fb0bc3b241204 100644
--- a/arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi
+++ b/arch/arm64/boot/dts/microchip/sparx5_pcb135_board.dtsi
@@ -51,6 +51,15 @@ i2cmux_s32: i2cmux-3 {
 	};
 };
 
+&spi0 {
+	status = "okay";
+	spi-flash@0 {
+		compatible = "jedec,spi-nor";
+		spi-max-frequency = <8000000>;
+		reg = <0>;
+	};
+};
+
 &axi {
 	i2c0_imux: i2c0-imux@0 {
 		compatible = "i2c-mux-pinctrl";
-- 
2.27.0

[PATCH v2 6/6] arm64: dts: sparx5: Add spi-nand devices

[Lars Povlsen]

This patch add spi-nand DT nodes to the applicable Sparx5 boards.

Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
---
 arch/arm64/boot/dts/microchip/sparx5.dtsi     | 20 ++++++++++++++++
 .../boot/dts/microchip/sparx5_pcb125.dts      |  7 ++++++
 .../boot/dts/microchip/sparx5_pcb134.dts      | 22 ++++++++++++++++++
 .../boot/dts/microchip/sparx5_pcb135.dts      | 23 +++++++++++++++++++
 4 files changed, 72 insertions(+)

diff --git a/arch/arm64/boot/dts/microchip/sparx5.dtsi b/arch/arm64/boot/dts/microchip/sparx5.dtsi
index 2404bcc08b89d..dd666d185e466 100644
--- a/arch/arm64/boot/dts/microchip/sparx5.dtsi
+++ b/arch/arm64/boot/dts/microchip/sparx5.dtsi
@@ -201,6 +201,26 @@ gpio: pinctrl@6110101e0 {
 			interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>;
 			#interrupt-cells = <2>;
 
+			cs1_pins: cs1-pins {
+				pins = "GPIO_16";
+				function = "si";
+			};
+
+			cs2_pins: cs2-pins {
+				pins = "GPIO_17";
+				function = "si";
+			};
+
+			cs3_pins: cs3-pins {
+				pins = "GPIO_18";
+				function = "si";
+			};
+
+			si2_pins: si2-pins {
+				pins = "GPIO_39", "GPIO_40", "GPIO_41";
+				function = "si2";
+			};
+
 			uart_pins: uart-pins {
 				pins = "GPIO_10", "GPIO_11";
 				function = "uart";
diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts b/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts
index d8b5d23abfab0..94c4c3fd5a786 100644
--- a/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts
+++ b/arch/arm64/boot/dts/microchip/sparx5_pcb125.dts
@@ -46,6 +46,13 @@ spi-flash@0 {
 		spi-max-frequency = <8000000>; /* input clock */
 		reg = <0>; /* CS0 */
 	};
+	spi-flash@1 {
+		compatible = "spi-nand";
+		pinctrl-0 = <&cs1_pins>;
+		pinctrl-names = "default";
+		spi-max-frequency = <8000000>;
+		reg = <1>; /* CS1 */
+	};
 };
 
 &i2c1 {
diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb134.dts b/arch/arm64/boot/dts/microchip/sparx5_pcb134.dts
index feee4e99ff57c..7aee0548e44cb 100644
--- a/arch/arm64/boot/dts/microchip/sparx5_pcb134.dts
+++ b/arch/arm64/boot/dts/microchip/sparx5_pcb134.dts
@@ -15,3 +15,25 @@ memory@0 {
 		reg = <0x00000000 0x00000000 0x10000000>;
 	};
 };
+
+&gpio {
+	cs14_pins: cs14-pins {
+		pins = "GPIO_44";
+		function = "si";
+	};
+};
+
+&spi0 {
+	pinctrl-0 = <&si2_pins>;
+	pinctrl-names = "default";
+	/* Dedicated SPI2 interface */
+	spi-flash@e {
+		compatible = "spi-nand";
+		pinctrl-0 = <&cs14_pins>;
+		pinctrl-names = "default";
+		spi-max-frequency = <42000000>;
+		reg = <14>;
+		microchip,spi-interface2;	/* SPI2 */
+		snps,rx-sample-delay-ns = <7>;  /* Tune for speed */
+	};
+};
diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb135.dts b/arch/arm64/boot/dts/microchip/sparx5_pcb135.dts
index 20e409a9be196..8f2329ce02030 100644
--- a/arch/arm64/boot/dts/microchip/sparx5_pcb135.dts
+++ b/arch/arm64/boot/dts/microchip/sparx5_pcb135.dts
@@ -15,3 +15,26 @@ memory@0 {
 		reg = <0x00000000 0x00000000 0x10000000>;
 	};
 };
+
+&gpio {
+	cs14_pins: cs14-pins {
+		pins = "GPIO_44";
+		function = "si";
+	};
+};
+
+&spi0 {
+	status = "okay";
+	pinctrl-0 = <&si2_pins>;
+	pinctrl-names = "default";
+	/* Dedicated SPI2 interface */
+	spi-flash@e {
+		compatible = "spi-nand";
+		pinctrl-0 = <&cs14_pins>;
+		pinctrl-names = "default";
+		spi-max-frequency = <42000000>;
+		reg = <14>;
+		microchip,spi-interface2;	/* SPI2 */
+		snps,rx-sample-delay-ns = <7>;  /* Tune for speed */
+	};
+};
-- 
2.27.0

Re: [PATCH v2 3/6] spi: dw: Add Microchip Sparx5 support

[Lars Povlsen]

Mark Brown writes:

On Fri, Jun 19, 2020 at 01:31:18PM +0200, Lars Povlsen wrote:

>> +/*
>> + * The Designware SPI controller (referred to as master in the
>> + * documentation) automatically deasserts chip select when the tx fifo
>> + * is empty. The chip selects then needs to be driven by a CS override
>> + * register. enable is an active low signal.
>> + */
>> +static void dw_spi_sparx5_set_cs(struct spi_device *spi, bool nEnable)
>
>The value that is passed in here is the value that should be driven on
>the output pin, the driver should not be interpreting the value in any
>way here.  Documenting it as nEnable adds a layer of confusion, and it
>may not be an active high signal depending on the system.

Ok, I will make the CS function more like the others.

>
>> +	if (!nEnable) {
>> +		/* Ensure CS toggles, so start off all disabled */
>> +		regmap_write(dwsmscc->syscon, SPARX5_FORCE_VAL, ~0);
>> +		/* CS override drive enable */
>> +		regmap_write(dwsmscc->syscon, SPARX5_FORCE_ENA, 1);
>
>This should just be setting the value to whatever the core asked for it
>to be set to, the driver adding extra toggles is likely to disrupt
>things.

I will have a look at this again. But it was added for a reason. The
issue is that we have two different busses in front of the controller,
so we might need more settle time when switching interface.

Thank you for you comments,

Cheers

-- 
Lars Povlsen,
Microchip

Re: [PATCH v2 3/6] spi: dw: Add Microchip Sparx5 support

[Lars Povlsen]

Mark Brown writes:

On Mon, Jun 22, 2020 at 12:46:33PM +0200, Lars Povlsen wrote:
>> On Fri, Jun 19, 2020 at 01:31:18PM +0200, Lars Povlsen wrote:
>
>> >> +	if (!nEnable) {
>> >> +		/* Ensure CS toggles, so start off all disabled */
>> >> +		regmap_write(dwsmscc->syscon, SPARX5_FORCE_VAL, ~0);
>> >> +		/* CS override drive enable */
>> >> +		regmap_write(dwsmscc->syscon, SPARX5_FORCE_ENA, 1);
>
>> >This should just be setting the value to whatever the core asked for it
>> >to be set to, the driver adding extra toggles is likely to disrupt
>> >things.
>
>> I will have a look at this again. But it was added for a reason. The
>> issue is that we have two different busses in front of the controller,
>> so we might need more settle time when switching interface.
>
>If there's a mux that needs to be handled specially that mux should be
>described in DT on the relevant boards, there shouldn't just be
>something hard coded in the controller driver.

I have been able to change this into a straight setting - no
toggling. Just needed a settle delay.

I looked at the spi-mux driver, but that is more for muxing the CS's, as
I understand - not the actual bus segment. I could use it, but it would
require encoding the bus segment into the CS (double the normal
range). Also, selecting the bus interface is tightly coupled to the
controller - its not an externally constructed board mux.

I feel the current implementation is more to the point, and easily
understandable. It just adds the "microchip,spi-interface2" DT property.

It might be that a better way exists using the spi-mux framework, and if
you have some ideas for that I'd be all ears.

>
>BTW please do not CC subsystem patches to soc@kernel.org unless there's
>a specific reason to do so - there's no need for it, these patches won't
>get merged via there unless something is going wrong.  Generally the
>subsystem maintainers take patches for a given subsystem.

Ok, duly noted.

Thank you for the comments.

---Lars


-- 
Lars Povlsen,
Microchip

Re: [PATCH v2 3/6] spi: dw: Add Microchip Sparx5 support

[Mark Brown]

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

On Tue, Jun 23, 2020 at 03:53:22PM +0200, Lars Povlsen wrote:
> Mark Brown writes:

> >If there's a mux that needs to be handled specially that mux should be
> >described in DT on the relevant boards, there shouldn't just be
> >something hard coded in the controller driver.

> I looked at the spi-mux driver, but that is more for muxing the CS's, as
> I understand - not the actual bus segment. I could use it, but it would

It doesn't matter that much exactly what signals get switched I think,
we can't really tell by the time we get back to the controller.

> require encoding the bus segment into the CS (double the normal
> range). Also, selecting the bus interface is tightly coupled to the
> controller - its not an externally constructed board mux.

It sounds like this controller should be describing a mux all the time -
if there's completely separate output buses that you can switch between
then we'll need to know about that if someone wires up a GPIO chip
select.

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

Re: [PATCH v2 3/6] spi: dw: Add Microchip Sparx5 support

[Lars Povlsen]

Mark Brown writes:

> On Tue, Jun 23, 2020 at 03:53:22PM +0200, Lars Povlsen wrote:
> > Mark Brown writes:
> 
> > >If there's a mux that needs to be handled specially that mux should be
> > >described in DT on the relevant boards, there shouldn't just be
> > >something hard coded in the controller driver.
> 
> > I looked at the spi-mux driver, but that is more for muxing the CS's, as
> > I understand - not the actual bus segment. I could use it, but it would
> 
> It doesn't matter that much exactly what signals get switched I think,
> we can't really tell by the time we get back to the controller.
> 
> > require encoding the bus segment into the CS (double the normal
> > range). Also, selecting the bus interface is tightly coupled to the
> > controller - its not an externally constructed board mux.
> 
> It sounds like this controller should be describing a mux all the time -
> if there's completely separate output buses that you can switch between
> then we'll need to know about that if someone wires up a GPIO chip
> select.

Mark,

I had to tinker a bit with this to get my head around it.

I added the mux driver, and made the cs/bus configuration reside here -
all well and done.

For our reference config we have something like:

  mux: mux-controller {
          compatible = "microchip,sparx5-spi-mux";
          #mux-control-cells = <0>;
          mux@0 {
                  reg = <0>;
                  microchip,bus-interface = <0>;
          };
          mux@e {
                  reg = <14>;
                  microchip,bus-interface = <1>;
          };
  };

Then I tried to use the existing spi-mux as you suggested. But I
realized as its really designed for CS muxing I had to instantiate each
SPI device in its own spi-mux instance, repeating the CS (as we don't
want that to change). The result was kinda bulky.

An example would be:

  spi0: spi@600104000 {
          compatible = "microchip,sparx5-spi";
          spi@0 {
                  compatible = "spi-mux";
                  mux-controls = <&mux>;
                  reg = <0>;
                  spi-flash@0 {
                          compatible = "jedec,spi-nor";
                          reg = <0>;
                  };
          };
          spi@e {
                  compatible = "spi-mux";
                  mux-controls = <&mux>;
                  reg = <14>;
                  spi-flash@e {
                          compatible = "spi-nand";
                          reg = <14>;
                  };
          };
  };

I then looked a bit at other users of the mux framework,
drivers/mtd/hyperbus/hbmc-am654.c specifically.

I then added direct use of a mux, by the well-established "mux-controls"
DT property. The result was much cleaner, IMHO, and allows the spi and
mux to be connected directly in the base sparx5 DT. Code impact was
really small.

Both examples use the same mux configuration, and as the "mux-controls"
is established by default in the base spi0 node, this (directly) yields:

&spi0 {
	spi-flash@0 {
		compatible = "jedec,spi-nor";
		reg = <0>;
	};
	spi-flash@e {
		compatible = "spi-nand";
		reg = <14>;
	};
};

I will be sending the new revision of the patches shortly. I look
forward to your comments.

I also CC'ed Peter Rosin as the MUX subsystem maintainer. Peter, sorry
for sticking you halfway into a conversation, but I thought you might
want to be informed. You are also on the recipient list of the v3
patches, so now you know why...

Sincerely,

-- 
Lars Povlsen,
Microchip