linux-spi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/8] spi: Adding support for Microchip Sparx5 SoC
@ 2020-07-02 10:13 Lars Povlsen
  2020-07-02 10:13 ` [PATCH v3 1/8] spi: dw: Add support for RX sample delay register Lars Povlsen
                   ` (7 more replies)
  0 siblings, 8 replies; 17+ messages in thread
From: Lars Povlsen @ 2020-07-02 10:13 UTC (permalink / raw)
  To: Mark Brown, Peter Rosin
  Cc: Lars Povlsen, Microchip Linux Driver Support, linux-spi,
	devicetree, linux-kernel, linux-arm-kernel, Serge Semin,
	Serge Semin

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.

v3 changes:
- Added mux support for controlling SPI bus interface. This is new mux
  driver, bindings and added to sparx5 base DT.
- Removed "microchip,spi-interface2" property in favour of
  "mux-controls" property in SPI controller (sparx5 only).
- Changed dw_spi_sparx5_set_cs() to use the mux control instead of
  directly acessing "mux" register. Associated code/defines moved to mux
  driver.
- Changed dw_spi_sparx5_set_cs() to match other similar functions in
  signature and avoid explicit CS toggling.
- Spun off duplicated NAND device DT chunks into separate DT file.

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 (8):
  spi: dw: Add support for RX sample delay register
  arm64: dts: sparx5: Add SPI controller and SPI mux
  spi: dw: Add Microchip Sparx5 support
  mux: sparx5: Add Sparx5 SPI mux driver
  dt-bindings: snps,dw-apb-ssi: Add sparx5 support, plus
    snps,rx-sample-delay-ns property
  dt-bindings: microchip,sparx5-spi-mux: Add Sparx5 SPI mux driver
    bindings
  arm64: dts: sparx5: Add spi-nor support
  arm64: dts: sparx5: Add spi-nand devices

 .../mux/microchip,sparx5-spi-mux.yaml         |  71 +++++++++
 .../bindings/spi/snps,dw-apb-ssi.yaml         |  28 ++++
 arch/arm64/boot/dts/microchip/sparx5.dtsi     |  43 ++++++
 .../arm64/boot/dts/microchip/sparx5_nand.dtsi |  32 ++++
 .../boot/dts/microchip/sparx5_pcb125.dts      |  16 ++
 .../boot/dts/microchip/sparx5_pcb134.dts      |   1 +
 .../dts/microchip/sparx5_pcb134_board.dtsi    |   9 ++
 .../boot/dts/microchip/sparx5_pcb135.dts      |   1 +
 .../dts/microchip/sparx5_pcb135_board.dtsi    |   9 ++
 drivers/mux/Makefile                          |   2 +
 drivers/mux/sparx5-spi.c                      | 138 ++++++++++++++++++
 drivers/spi/Kconfig                           |   1 +
 drivers/spi/spi-dw-core.c                     |  20 +++
 drivers/spi/spi-dw-mmio.c                     |  79 +++++++++-
 drivers/spi/spi-dw.h                          |   2 +
 15 files changed, 451 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/mux/microchip,sparx5-spi-mux.yaml
 create mode 100644 arch/arm64/boot/dts/microchip/sparx5_nand.dtsi
 create mode 100644 drivers/mux/sparx5-spi.c

--
2.27.0

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

* [PATCH v3 1/8] spi: dw: Add support for RX sample delay register
  2020-07-02 10:13 [PATCH v3 0/8] spi: Adding support for Microchip Sparx5 SoC Lars Povlsen
@ 2020-07-02 10:13 ` Lars Povlsen
  2020-07-02 10:13 ` [PATCH v3 2/8] arm64: dts: sparx5: Add SPI controller and SPI mux Lars Povlsen
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Lars Povlsen @ 2020-07-02 10:13 UTC (permalink / raw)
  To: Mark Brown, Peter Rosin
  Cc: Lars Povlsen, Microchip Linux Driver Support, linux-spi,
	devicetree, linux-kernel, linux-arm-kernel, Serge Semin,
	Serge Semin

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


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

* [PATCH v3 2/8] arm64: dts: sparx5: Add SPI controller and SPI mux
  2020-07-02 10:13 [PATCH v3 0/8] spi: Adding support for Microchip Sparx5 SoC Lars Povlsen
  2020-07-02 10:13 ` [PATCH v3 1/8] spi: dw: Add support for RX sample delay register Lars Povlsen
@ 2020-07-02 10:13 ` Lars Povlsen
  2020-07-02 10:13 ` [PATCH v3 3/8] spi: dw: Add Microchip Sparx5 support Lars Povlsen
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Lars Povlsen @ 2020-07-02 10:13 UTC (permalink / raw)
  To: Mark Brown, Peter Rosin
  Cc: Lars Povlsen, Microchip Linux Driver Support, linux-spi,
	devicetree, linux-kernel, linux-arm-kernel, Serge Semin,
	Serge Semin

This adds a SPI controller to the Microchip Sparx5 SoC, as well as the
SPI bus interface mux, which may optionally be needed for selecting
between alternate SPI bus segments (SPI/SPI2).

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

diff --git a/arch/arm64/boot/dts/microchip/sparx5.dtsi b/arch/arm64/boot/dts/microchip/sparx5.dtsi
index 7e811e24f0e99..2831935a489e1 100644
--- a/arch/arm64/boot/dts/microchip/sparx5.dtsi
+++ b/arch/arm64/boot/dts/microchip/sparx5.dtsi
@@ -14,6 +14,8 @@ / {
 	#size-cells = <1>;
 
 	aliases {
+		mux = &mux;
+		spi0 = &spi0;
 		serial0 = &uart0;
 		serial1 = &uart1;
 	};
@@ -155,6 +157,27 @@ uart1: serial@600102000 {
 			status = "disabled";
 		};
 
+		mux: mux-controller {
+			compatible = "microchip,sparx5-spi-mux";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			#mux-control-cells = <0>;
+		};
+
+		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>;
+			mux-controls = <&mux>;
+			status = "disabled";
+		};
+
 		timer1: timer@600105000 {
 			compatible = "snps,dw-apb-timer";
 			reg = <0x6 0x00105000 0x1000>;
-- 
2.27.0


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

* [PATCH v3 3/8] spi: dw: Add Microchip Sparx5 support
  2020-07-02 10:13 [PATCH v3 0/8] spi: Adding support for Microchip Sparx5 SoC Lars Povlsen
  2020-07-02 10:13 ` [PATCH v3 1/8] spi: dw: Add support for RX sample delay register Lars Povlsen
  2020-07-02 10:13 ` [PATCH v3 2/8] arm64: dts: sparx5: Add SPI controller and SPI mux Lars Povlsen
@ 2020-07-02 10:13 ` Lars Povlsen
  2020-07-02 10:13 ` [PATCH v3 4/8] mux: sparx5: Add Sparx5 SPI mux driver Lars Povlsen
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Lars Povlsen @ 2020-07-02 10:13 UTC (permalink / raw)
  To: Mark Brown, Peter Rosin
  Cc: Lars Povlsen, Microchip Linux Driver Support, linux-spi,
	devicetree, linux-kernel, linux-arm-kernel, Serge Semin,
	Serge Semin

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...). The "mux-controls" property must be
   set, and the mux should be configured with the bus/device mapping
   information.

 * 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.

As Sparx5 requires CONFIG_MULTIPLEXER, this will automatically be
enabled when this driver is selected.

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

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 8f1f8fca79e37..2bc2d42b25120 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -237,6 +237,7 @@ config SPI_DW_PCI
 
 config SPI_DW_MMIO
 	tristate "Memory-mapped io interface driver for DW SPI core"
+	select MULTIPLEXER
 	depends on HAS_IOMEM
 
 endif
diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
index 403403deae664..05bc09be4a8bd 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/mux/consumer.h>
 
 #include "spi-dw.h"
 
@@ -45,6 +46,9 @@ struct dw_spi_mmio {
 #define MSCC_SPI_MST_SW_MODE_SW_PIN_CTRL_MODE	BIT(13)
 #define MSCC_SPI_MST_SW_MODE_SW_SPI_CS(x)	(x << 5)
 
+#define SPARX5_FORCE_ENA			0xa4
+#define SPARX5_FORCE_VAL			0xa8
+
 /*
  * For Keem Bay, CTRLR0[31] is used to select controller mode.
  * 0: SSI is slave
@@ -54,7 +58,8 @@ struct dw_spi_mmio {
 
 struct dw_spi_mscc {
 	struct regmap       *syscon;
-	void __iomem        *spi_mst;
+	void __iomem        *spi_mst; /* Not sparx5 */
+	struct mux_control  *spi_mux; /* Sparx5 bus interface */
 };
 
 /*
@@ -134,6 +139,77 @@ static int dw_spi_mscc_jaguar2_init(struct platform_device *pdev,
 				JAGUAR2_IF_SI_OWNER_OFFSET);
 }
 
+/*
+ * 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 enable)
+{
+	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 (!enable) {
+		/* Drive mux */
+		if (mux_control_select(dwsmscc->spi_mux, cs))
+			dev_err(&spi->dev, "Unable to drive SPI mux\n");
+		/* CS override drive enable */
+		regmap_write(dwsmscc->syscon, SPARX5_FORCE_ENA, 1);
+		/* Now set CSx enabled */
+		regmap_write(dwsmscc->syscon, SPARX5_FORCE_VAL, ~BIT(cs));
+		/* Allow settle */
+		usleep_range(1, 5);
+	} else {
+		/* CS value */
+		regmap_write(dwsmscc->syscon, SPARX5_FORCE_VAL, ~0);
+		/* Allow settle */
+		usleep_range(1, 5);
+		/* CS override drive disable */
+		regmap_write(dwsmscc->syscon, SPARX5_FORCE_ENA, 0);
+		/* Deselect mux */
+		mux_control_deselect(dwsmscc->spi_mux);
+	}
+
+	dw_spi_set_cs(spi, enable);
+}
+
+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 device *dev = &pdev->dev;
+	struct dw_spi_mscc *dwsmscc;
+
+	dwsmscc = devm_kzalloc(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(dev, "No syscon map %s\n", syscon_name);
+		return PTR_ERR(dwsmscc->syscon);
+	}
+
+	/* Get SPI mux */
+	dwsmscc->spi_mux = devm_mux_control_get(dev, NULL);
+	if (IS_ERR(dwsmscc->spi_mux)) {
+		dev_err(dev, "SPI mux is required\n");
+		return PTR_ERR(dwsmscc->spi_mux);
+	}
+
+	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 +373,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


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

* [PATCH v3 4/8] mux: sparx5: Add Sparx5 SPI mux driver
  2020-07-02 10:13 [PATCH v3 0/8] spi: Adding support for Microchip Sparx5 SoC Lars Povlsen
                   ` (2 preceding siblings ...)
  2020-07-02 10:13 ` [PATCH v3 3/8] spi: dw: Add Microchip Sparx5 support Lars Povlsen
@ 2020-07-02 10:13 ` Lars Povlsen
  2020-07-02 11:33   ` Lars Povlsen
  2020-07-02 11:36   ` Peter Rosin
  2020-07-02 10:13 ` [PATCH v3 5/8] dt-bindings: snps,dw-apb-ssi: Add sparx5 support, plus snps,rx-sample-delay-ns property Lars Povlsen
                   ` (3 subsequent siblings)
  7 siblings, 2 replies; 17+ messages in thread
From: Lars Povlsen @ 2020-07-02 10:13 UTC (permalink / raw)
  To: Mark Brown, Peter Rosin
  Cc: Lars Povlsen, Microchip Linux Driver Support, linux-spi,
	devicetree, linux-kernel, linux-arm-kernel, Serge Semin,
	Serge Semin

The Sparx5 mux driver may be used to control selecting between two
alternate SPI bus segments connected to the SPI controller
(spi-dw-mmio).

Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
---
 drivers/mux/Makefile     |   2 +
 drivers/mux/sparx5-spi.c | 138 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 140 insertions(+)
 create mode 100644 drivers/mux/sparx5-spi.c

diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile
index 6e9fa47daf566..18c3ae3582ece 100644
--- a/drivers/mux/Makefile
+++ b/drivers/mux/Makefile
@@ -8,9 +8,11 @@ mux-adg792a-objs		:= adg792a.o
 mux-adgs1408-objs		:= adgs1408.o
 mux-gpio-objs			:= gpio.o
 mux-mmio-objs			:= mmio.o
+mux-sparx5-objs			:= sparx5-spi.o
 
 obj-$(CONFIG_MULTIPLEXER)	+= mux-core.o
 obj-$(CONFIG_MUX_ADG792A)	+= mux-adg792a.o
 obj-$(CONFIG_MUX_ADGS1408)	+= mux-adgs1408.o
 obj-$(CONFIG_MUX_GPIO)		+= mux-gpio.o
 obj-$(CONFIG_MUX_MMIO)		+= mux-mmio.o
+obj-$(CONFIG_SPI_DW_MMIO)	+= mux-sparx5.o
diff --git a/drivers/mux/sparx5-spi.c b/drivers/mux/sparx5-spi.c
new file mode 100644
index 0000000000000..5fe9025b96a5e
--- /dev/null
+++ b/drivers/mux/sparx5-spi.c
@@ -0,0 +1,138 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Sparx5 SPI MUX driver
+ *
+ * Copyright (c) 2019 Microsemi Corporation
+ *
+ * Author: Lars Povlsen <lars.povlsen@microchip.com>
+ */
+
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/mux/driver.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/mux/driver.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
+#include <linux/bitfield.h>
+
+#define MSCC_IF_SI_OWNER_SISL			0
+#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_MAX_CS	16
+
+struct mux_sparx5 {
+	struct regmap *syscon;
+	u8 bus[SPARX5_MAX_CS];
+	int cur_bus;
+};
+
+/*
+ * Set the owner of the SPI interfaces
+ */
+static void mux_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 mux_sparx5_set_cs_owner(struct mux_sparx5 *mux_sparx5,
+				    u8 cs, u8 owner)
+{
+	u8 other = (owner == MSCC_IF_SI_OWNER_SIBM ?
+		    MSCC_IF_SI_OWNER_SIMC : MSCC_IF_SI_OWNER_SIBM);
+	if (mux_sparx5->bus[cs])
+		/* SPI2 */
+		mux_sparx5_set_owner(mux_sparx5->syscon, other, owner);
+	else
+		/* SPI1 */
+		mux_sparx5_set_owner(mux_sparx5->syscon, owner, other);
+}
+
+static int mux_sparx5_set(struct mux_control *mux, int state)
+{
+	struct mux_sparx5 *mux_sparx5 = mux_chip_priv(mux->chip);
+
+	mux_sparx5_set_cs_owner(mux_sparx5, state, MSCC_IF_SI_OWNER_SIMC);
+
+	return 0;
+}
+
+static const struct mux_control_ops mux_sparx5_ops = {
+	.set = mux_sparx5_set,
+};
+
+static const struct of_device_id mux_sparx5_dt_ids[] = {
+	{ .compatible = "microchip,sparx5-spi-mux", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mux_sparx5_dt_ids);
+
+static int mux_sparx5_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct mux_chip *mux_chip;
+	struct mux_sparx5 *mux_sparx5;
+	struct device_node *nc;
+	const char *syscon_name = "microchip,sparx5-cpu-syscon";
+	int ret;
+
+	mux_chip = devm_mux_chip_alloc(dev, 1, sizeof(*mux_sparx5));
+	if (IS_ERR(mux_chip))
+		return PTR_ERR(mux_chip);
+
+	mux_sparx5 = mux_chip_priv(mux_chip);
+	mux_chip->ops = &mux_sparx5_ops;
+
+	mux_sparx5->syscon =
+		syscon_regmap_lookup_by_compatible(syscon_name);
+	if (IS_ERR(mux_sparx5->syscon)) {
+		dev_err(dev, "No syscon map %s\n", syscon_name);
+		return PTR_ERR(mux_sparx5->syscon);
+	}
+
+	/* Get bus interface mapping */
+	for_each_available_child_of_node(dev->of_node, nc) {
+		u32 cs, bus;
+
+		if (of_property_read_u32(nc, "reg", &cs) == 0 &&
+		    cs < SPARX5_MAX_CS &&
+		    of_property_read_u32(nc, "microchip,bus-interface",
+					 &bus) == 0)
+			mux_sparx5->bus[cs] = bus;
+	}
+
+	mux_chip->mux->states = SPARX5_MAX_CS;
+
+	ret = devm_mux_chip_register(dev, mux_chip);
+	if (ret < 0)
+		return ret;
+
+	dev_info(dev, "%u-way mux-controller registered\n",
+		 mux_chip->mux->states);
+
+	return 0;
+}
+
+static struct platform_driver mux_sparx5_driver = {
+	.driver = {
+		.name = "sparx5-mux",
+		.of_match_table	= of_match_ptr(mux_sparx5_dt_ids),
+	},
+	.probe = mux_sparx5_probe,
+};
+module_platform_driver(mux_sparx5_driver);
-- 
2.27.0


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

* [PATCH v3 5/8] dt-bindings: snps,dw-apb-ssi: Add sparx5 support, plus snps,rx-sample-delay-ns property
  2020-07-02 10:13 [PATCH v3 0/8] spi: Adding support for Microchip Sparx5 SoC Lars Povlsen
                   ` (3 preceding siblings ...)
  2020-07-02 10:13 ` [PATCH v3 4/8] mux: sparx5: Add Sparx5 SPI mux driver Lars Povlsen
@ 2020-07-02 10:13 ` Lars Povlsen
  2020-07-13 19:22   ` Rob Herring
  2020-07-02 10:13 ` [PATCH v3 6/8] dt-bindings: microchip,sparx5-spi-mux: Add Sparx5 SPI mux driver bindings Lars Povlsen
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Lars Povlsen @ 2020-07-02 10:13 UTC (permalink / raw)
  To: Mark Brown, Peter Rosin, Rob Herring
  Cc: Lars Povlsen, Microchip Linux Driver Support, linux-spi,
	devicetree, linux-kernel, linux-arm-kernel, Serge Semin,
	Serge Semin

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 "mux-controls" for the above compatible string

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

Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
---
 .../bindings/spi/snps,dw-apb-ssi.yaml         | 28 +++++++++++++++++++
 1 file changed, 28 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..9d9208391fae3 100644
--- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
+++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
@@ -36,6 +36,8 @@ properties:
               - mscc,ocelot-spi
               - mscc,jaguar2-spi
           - const: snps,dw-apb-ssi
+      - description: Microchip Sparx5 SoC SPI Controller
+        const: microchip,sparx5-spi
       - description: Amazon Alpine SPI Controller
         const: amazon,alpine-dw-apb-ssi
       - description: Renesas RZ/N1 SPI Controller
@@ -93,6 +95,19 @@ properties:
       - const: tx
       - const: rx

+if:
+  properties:
+    compatible:
+      contains:
+        const: microchip,sparx5-spi
+
+then:
+  properties:
+    mux-controls:
+      description: A mux controller node for selecting SPI bus interface.
+      maxItems: 1
+      $ref: '/schemas/types.yaml#/definitions/phandle'
+
 patternProperties:
   "^.*@[0-9a-f]+$":
     type: object
@@ -107,6 +122,14 @@ 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
+
 unevaluatedProperties: false

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

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

* [PATCH v3 6/8] dt-bindings: microchip,sparx5-spi-mux: Add Sparx5 SPI mux driver bindings
  2020-07-02 10:13 [PATCH v3 0/8] spi: Adding support for Microchip Sparx5 SoC Lars Povlsen
                   ` (4 preceding siblings ...)
  2020-07-02 10:13 ` [PATCH v3 5/8] dt-bindings: snps,dw-apb-ssi: Add sparx5 support, plus snps,rx-sample-delay-ns property Lars Povlsen
@ 2020-07-02 10:13 ` Lars Povlsen
  2020-07-13 19:29   ` Rob Herring
  2020-07-02 10:13 ` [PATCH v3 7/8] arm64: dts: sparx5: Add spi-nor support Lars Povlsen
  2020-07-02 10:13 ` [PATCH v3 8/8] arm64: dts: sparx5: Add spi-nand devices Lars Povlsen
  7 siblings, 1 reply; 17+ messages in thread
From: Lars Povlsen @ 2020-07-02 10:13 UTC (permalink / raw)
  To: Mark Brown, Peter Rosin, Rob Herring
  Cc: Lars Povlsen, Microchip Linux Driver Support, linux-spi,
	devicetree, linux-kernel, linux-arm-kernel, Serge Semin,
	Serge Semin

The Microchip Sparx5 SPI controller has two bus segments, and use this
mux to control the bus interface mapping for any chip selects. This
decribes the bindings used to configure the mux driver.

Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
---
 .../mux/microchip,sparx5-spi-mux.yaml         | 71 +++++++++++++++++++
 1 file changed, 71 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mux/microchip,sparx5-spi-mux.yaml

diff --git a/Documentation/devicetree/bindings/mux/microchip,sparx5-spi-mux.yaml b/Documentation/devicetree/bindings/mux/microchip,sparx5-spi-mux.yaml
new file mode 100644
index 0000000000000..b0ce3b15a69e5
--- /dev/null
+++ b/Documentation/devicetree/bindings/mux/microchip,sparx5-spi-mux.yaml
@@ -0,0 +1,71 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mux/microchip,sparx5-spi-mux.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip Sparx5 SPI mux
+
+maintainers:
+  - Lars Povlsen <lars.povlsen@microchip.com>
+
+description: |
+  The Microchip Sparx5 SPI controller has two bus segments. In order
+  to switch between the appropriate bus for any given SPI slave
+  (defined by a chip select), this mux driver is used. The device tree
+  node for the mux will define the bus mapping for any chip
+  selects. The default bus mapping for any chip select is "0", such
+  that only non-default mappings need to be explicitly defined.
+
+properties:
+  compatible:
+    enum:
+      - microchip,sparx5-spi-mux
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+  '#mux-control-cells':
+    const: 0
+
+required:
+  - compatible
+
+additionalProperties: false
+
+patternProperties:
+  "^mux@[0-9a-f]$":
+    type: object
+
+    properties:
+      reg:
+        description:
+          Chip select to define bus mapping for.
+        minimum: 0
+        maximum: 15
+
+      microchip,bus-interface:
+        description:
+          The bus interface to use for this chip select.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        enum: [0, 1]
+
+    required:
+      - reg
+      - microchip,bus-interface
+
+examples:
+  - |
+    mux: mux-controller {
+      compatible = "microchip,sparx5-spi-mux";
+      #address-cells = <1>;
+      #size-cells = <0>;
+      #mux-control-cells = <0>;
+      mux@e {
+        reg = <14>;
+        microchip,bus-interface = <1>;
+      };
+    };
--
2.27.0

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

* [PATCH v3 7/8] arm64: dts: sparx5: Add spi-nor support
  2020-07-02 10:13 [PATCH v3 0/8] spi: Adding support for Microchip Sparx5 SoC Lars Povlsen
                   ` (5 preceding siblings ...)
  2020-07-02 10:13 ` [PATCH v3 6/8] dt-bindings: microchip,sparx5-spi-mux: Add Sparx5 SPI mux driver bindings Lars Povlsen
@ 2020-07-02 10:13 ` Lars Povlsen
  2020-07-02 10:13 ` [PATCH v3 8/8] arm64: dts: sparx5: Add spi-nand devices Lars Povlsen
  7 siblings, 0 replies; 17+ messages in thread
From: Lars Povlsen @ 2020-07-02 10:13 UTC (permalink / raw)
  To: Mark Brown, Peter Rosin
  Cc: Lars Povlsen, Microchip Linux Driver Support, linux-spi,
	devicetree, linux-kernel, linux-arm-kernel, Serge Semin,
	Serge Semin

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


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

* [PATCH v3 8/8] arm64: dts: sparx5: Add spi-nand devices
  2020-07-02 10:13 [PATCH v3 0/8] spi: Adding support for Microchip Sparx5 SoC Lars Povlsen
                   ` (6 preceding siblings ...)
  2020-07-02 10:13 ` [PATCH v3 7/8] arm64: dts: sparx5: Add spi-nor support Lars Povlsen
@ 2020-07-02 10:13 ` Lars Povlsen
  7 siblings, 0 replies; 17+ messages in thread
From: Lars Povlsen @ 2020-07-02 10:13 UTC (permalink / raw)
  To: Mark Brown, Peter Rosin
  Cc: Lars Povlsen, Microchip Linux Driver Support, linux-spi,
	devicetree, linux-kernel, linux-arm-kernel, Serge Semin,
	Serge Semin

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 ++++++++++++
 .../arm64/boot/dts/microchip/sparx5_nand.dtsi | 32 +++++++++++++++++++
 .../boot/dts/microchip/sparx5_pcb125.dts      |  7 ++++
 .../boot/dts/microchip/sparx5_pcb134.dts      |  1 +
 .../boot/dts/microchip/sparx5_pcb135.dts      |  1 +
 5 files changed, 61 insertions(+)
 create mode 100644 arch/arm64/boot/dts/microchip/sparx5_nand.dtsi

diff --git a/arch/arm64/boot/dts/microchip/sparx5.dtsi b/arch/arm64/boot/dts/microchip/sparx5.dtsi
index 2831935a489e1..21a85359d3492 100644
--- a/arch/arm64/boot/dts/microchip/sparx5.dtsi
+++ b/arch/arm64/boot/dts/microchip/sparx5.dtsi
@@ -210,6 +210,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_nand.dtsi b/arch/arm64/boot/dts/microchip/sparx5_nand.dtsi
new file mode 100644
index 0000000000000..fd9523d9efbe3
--- /dev/null
+++ b/arch/arm64/boot/dts/microchip/sparx5_nand.dtsi
@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2020 Microchip Technology Inc. and its subsidiaries.
+ */
+
+&gpio {
+	cs14_pins: cs14-pins {
+		pins = "GPIO_44";
+		function = "si";
+	};
+};
+
+&mux {
+	/* CS14 (NAND) is on SPI2 */
+	mux@e {
+		reg = <14>;
+		microchip,bus-interface = <1>;
+	};
+};
+
+&spi0 {
+	pinctrl-0 = <&si2_pins>;
+	pinctrl-names = "default";
+	spi-flash@e {
+		compatible = "spi-nand";
+		pinctrl-0 = <&cs14_pins>;
+		pinctrl-names = "default";
+		spi-max-frequency = <42000000>;
+		reg = <14>;
+		snps,rx-sample-delay-ns = <7>;  /* Tune for speed */
+	};
+};
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..45ca1af7e8500 100644
--- a/arch/arm64/boot/dts/microchip/sparx5_pcb134.dts
+++ b/arch/arm64/boot/dts/microchip/sparx5_pcb134.dts
@@ -5,6 +5,7 @@
 
 /dts-v1/;
 #include "sparx5_pcb134_board.dtsi"
+#include "sparx5_nand.dtsi"
 
 / {
 	model = "Sparx5 PCB134 Reference Board (NAND)";
diff --git a/arch/arm64/boot/dts/microchip/sparx5_pcb135.dts b/arch/arm64/boot/dts/microchip/sparx5_pcb135.dts
index 20e409a9be196..647cdb38b1130 100644
--- a/arch/arm64/boot/dts/microchip/sparx5_pcb135.dts
+++ b/arch/arm64/boot/dts/microchip/sparx5_pcb135.dts
@@ -5,6 +5,7 @@
 
 /dts-v1/;
 #include "sparx5_pcb135_board.dtsi"
+#include "sparx5_nand.dtsi"
 
 / {
 	model = "Sparx5 PCB135 Reference Board (NAND)";
-- 
2.27.0


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

* Re: [PATCH v3 4/8] mux: sparx5: Add Sparx5 SPI mux driver
  2020-07-02 10:13 ` [PATCH v3 4/8] mux: sparx5: Add Sparx5 SPI mux driver Lars Povlsen
@ 2020-07-02 11:33   ` Lars Povlsen
  2020-07-02 11:36   ` Peter Rosin
  1 sibling, 0 replies; 17+ messages in thread
From: Lars Povlsen @ 2020-07-02 11:33 UTC (permalink / raw)
  To: Lars Povlsen
  Cc: Mark Brown, Peter Rosin, Microchip Linux Driver Support,
	linux-spi, devicetree, linux-kernel, linux-arm-kernel,
	Serge Semin, Serge Semin


Lars Povlsen writes:

> The Sparx5 mux driver may be used to control selecting between two
> alternate SPI bus segments connected to the SPI controller
> (spi-dw-mmio).
>
> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
> ---
>  drivers/mux/Makefile     |   2 +
>  drivers/mux/sparx5-spi.c | 138 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 140 insertions(+)
>  create mode 100644 drivers/mux/sparx5-spi.c
>
...
> +
> +#define SPARX5_MAX_CS	16
> +
> +struct mux_sparx5 {
> +	struct regmap *syscon;
> +	u8 bus[SPARX5_MAX_CS];
> +	int cur_bus;
> +};

Yeah, "cur_bus" is a leftover. I'll remove it.

-- 
Lars Povlsen,
Microchip

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

* Re: [PATCH v3 4/8] mux: sparx5: Add Sparx5 SPI mux driver
  2020-07-02 10:13 ` [PATCH v3 4/8] mux: sparx5: Add Sparx5 SPI mux driver Lars Povlsen
  2020-07-02 11:33   ` Lars Povlsen
@ 2020-07-02 11:36   ` Peter Rosin
  2020-07-03  9:14     ` Lars Povlsen
  1 sibling, 1 reply; 17+ messages in thread
From: Peter Rosin @ 2020-07-02 11:36 UTC (permalink / raw)
  To: Lars Povlsen, Mark Brown
  Cc: Microchip Linux Driver Support, linux-spi, devicetree,
	linux-kernel, linux-arm-kernel, Serge Semin, Serge Semin

Hi!

On 2020-07-02 12:13, Lars Povlsen wrote:
> The Sparx5 mux driver may be used to control selecting between two
> alternate SPI bus segments connected to the SPI controller
> (spi-dw-mmio).
> 
> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
> ---
>  drivers/mux/Makefile     |   2 +
>  drivers/mux/sparx5-spi.c | 138 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 140 insertions(+)
>  create mode 100644 drivers/mux/sparx5-spi.c
> 
> diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile
> index 6e9fa47daf566..18c3ae3582ece 100644
> --- a/drivers/mux/Makefile
> +++ b/drivers/mux/Makefile
> @@ -8,9 +8,11 @@ mux-adg792a-objs		:= adg792a.o
>  mux-adgs1408-objs		:= adgs1408.o
>  mux-gpio-objs			:= gpio.o
>  mux-mmio-objs			:= mmio.o
> +mux-sparx5-objs			:= sparx5-spi.o
>  
>  obj-$(CONFIG_MULTIPLEXER)	+= mux-core.o
>  obj-$(CONFIG_MUX_ADG792A)	+= mux-adg792a.o
>  obj-$(CONFIG_MUX_ADGS1408)	+= mux-adgs1408.o
>  obj-$(CONFIG_MUX_GPIO)		+= mux-gpio.o
>  obj-$(CONFIG_MUX_MMIO)		+= mux-mmio.o
> +obj-$(CONFIG_SPI_DW_MMIO)	+= mux-sparx5.o
> diff --git a/drivers/mux/sparx5-spi.c b/drivers/mux/sparx5-spi.c
> new file mode 100644
> index 0000000000000..5fe9025b96a5e
> --- /dev/null
> +++ b/drivers/mux/sparx5-spi.c
> @@ -0,0 +1,138 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Sparx5 SPI MUX driver
> + *
> + * Copyright (c) 2019 Microsemi Corporation
> + *
> + * Author: Lars Povlsen <lars.povlsen@microchip.com>
> + */
> +
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/mux/driver.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/mux/driver.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/regmap.h>
> +#include <linux/bitfield.h>
> +
> +#define MSCC_IF_SI_OWNER_SISL			0
> +#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_MAX_CS	16
> +
> +struct mux_sparx5 {
> +	struct regmap *syscon;
> +	u8 bus[SPARX5_MAX_CS];
> +	int cur_bus;

Surplus unused variable?

> +};
> +
> +/*
> + * Set the owner of the SPI interfaces
> + */
> +static void mux_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 mux_sparx5_set_cs_owner(struct mux_sparx5 *mux_sparx5,
> +				    u8 cs, u8 owner)
> +{
> +	u8 other = (owner == MSCC_IF_SI_OWNER_SIBM ?
> +		    MSCC_IF_SI_OWNER_SIMC : MSCC_IF_SI_OWNER_SIBM);

Empty line missing here.

> +	if (mux_sparx5->bus[cs])
> +		/* SPI2 */
> +		mux_sparx5_set_owner(mux_sparx5->syscon, other, owner);
> +	else
> +		/* SPI1 */
> +		mux_sparx5_set_owner(mux_sparx5->syscon, owner, other);
> +}


This smells like there are only two states for this mux control, and that
the whole point of this driver is to make the exact numbering selectable.
I don't see the point of that. To me, it looks like the pre-existing
mmio-mux should be able to work. Something like this? Untested of course,
and I might easily have misunderstood something...

mux: mux-controller {
	compatible = "mmio-mux"
	#mux-control-cells = <1>;

	/* SI_OWNER and SI2_OWNER in GENERAL_CTRL */
	mux-reg-masks = <0x88 0xf0>;
};


spi0: spi@600104000 {
	compatible = "microchip,sparx5-spi";
	spi@0 {
		compatible = "spi-mux";
		mux-controls = <&mux 0>;
		reg = <0>;
		/* SI_OWNER = SIMC, SI2_OWNER = SIBM  --->  mux value 9 */
		spi-flash@9 {
			compatible = "jedec,spi-nor";
			reg = <9>;
		};
	};
	spi@e {
		compatible = "spi-mux";
		mux-controls = <&mux 0>;
		reg = <14>;
		/* SI_OWNER = SIBM, SI2_OWNER = SIMC  --->  mux value 6 */
		spi-flash@6 {
			compatible = "spi-nand";
			reg = <6>;
		};
	};
};

> +
> +static int mux_sparx5_set(struct mux_control *mux, int state)
> +{
> +	struct mux_sparx5 *mux_sparx5 = mux_chip_priv(mux->chip);
> +
> +	mux_sparx5_set_cs_owner(mux_sparx5, state, MSCC_IF_SI_OWNER_SIMC);
> +
> +	return 0;
> +}
> +
> +static const struct mux_control_ops mux_sparx5_ops = {
> +	.set = mux_sparx5_set,
> +};
> +
> +static const struct of_device_id mux_sparx5_dt_ids[] = {
> +	{ .compatible = "microchip,sparx5-spi-mux", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, mux_sparx5_dt_ids);
> +
> +static int mux_sparx5_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct mux_chip *mux_chip;
> +	struct mux_sparx5 *mux_sparx5;
> +	struct device_node *nc;
> +	const char *syscon_name = "microchip,sparx5-cpu-syscon";
> +	int ret;
> +
> +	mux_chip = devm_mux_chip_alloc(dev, 1, sizeof(*mux_sparx5));
> +	if (IS_ERR(mux_chip))
> +		return PTR_ERR(mux_chip);
> +
> +	mux_sparx5 = mux_chip_priv(mux_chip);
> +	mux_chip->ops = &mux_sparx5_ops;
> +
> +	mux_sparx5->syscon =
> +		syscon_regmap_lookup_by_compatible(syscon_name);
> +	if (IS_ERR(mux_sparx5->syscon)) {
> +		dev_err(dev, "No syscon map %s\n", syscon_name);
> +		return PTR_ERR(mux_sparx5->syscon);
> +	}
> +
> +	/* Get bus interface mapping */
> +	for_each_available_child_of_node(dev->of_node, nc) {
> +		u32 cs, bus;
> +
> +		if (of_property_read_u32(nc, "reg", &cs) == 0 &&
> +		    cs < SPARX5_MAX_CS &&
> +		    of_property_read_u32(nc, "microchip,bus-interface",
> +					 &bus) == 0)
> +			mux_sparx5->bus[cs] = bus;

The above if is a mess. The kernel model is to handle the exceptional cases
first and break/goto/continue/return/whatever so that the interesting code
can happen at lower indentation level.

		if (of_property_read_u32(nc, "reg", &cs))
			continue;
		if (cs >= SPARX5_MAX_CS)
			continue;
		if (of_property_read_u32(nc, "microchip,bus-interface", &bus))
			continue;

		mux_sparc5->bus[cs] = bus;

Cheers,
Peter

> +	}
> +
> +	mux_chip->mux->states = SPARX5_MAX_CS;
> +
> +	ret = devm_mux_chip_register(dev, mux_chip);
> +	if (ret < 0)
> +		return ret;
> +
> +	dev_info(dev, "%u-way mux-controller registered\n",
> +		 mux_chip->mux->states);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver mux_sparx5_driver = {
> +	.driver = {
> +		.name = "sparx5-mux",
> +		.of_match_table	= of_match_ptr(mux_sparx5_dt_ids),
> +	},
> +	.probe = mux_sparx5_probe,
> +};
> +module_platform_driver(mux_sparx5_driver);
> 

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

* Re: [PATCH v3 4/8] mux: sparx5: Add Sparx5 SPI mux driver
  2020-07-02 11:36   ` Peter Rosin
@ 2020-07-03  9:14     ` Lars Povlsen
  0 siblings, 0 replies; 17+ messages in thread
From: Lars Povlsen @ 2020-07-03  9:14 UTC (permalink / raw)
  To: Peter Rosin
  Cc: Lars Povlsen, Mark Brown, Microchip Linux Driver Support,
	linux-spi, devicetree, linux-kernel, linux-arm-kernel,
	Serge Semin, Serge Semin


Peter Rosin writes:

> Hi!
>
> On 2020-07-02 12:13, Lars Povlsen wrote:
>> The Sparx5 mux driver may be used to control selecting between two
>> alternate SPI bus segments connected to the SPI controller
>> (spi-dw-mmio).
>>
>> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
>> ---
>>  drivers/mux/Makefile     |   2 +
>>  drivers/mux/sparx5-spi.c | 138 +++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 140 insertions(+)
>>  create mode 100644 drivers/mux/sparx5-spi.c
>>
>> diff --git a/drivers/mux/Makefile b/drivers/mux/Makefile
>> index 6e9fa47daf566..18c3ae3582ece 100644
>> --- a/drivers/mux/Makefile
>> +++ b/drivers/mux/Makefile
>> @@ -8,9 +8,11 @@ mux-adg792a-objs             := adg792a.o
>>  mux-adgs1408-objs            := adgs1408.o
>>  mux-gpio-objs                        := gpio.o
>>  mux-mmio-objs                        := mmio.o
>> +mux-sparx5-objs                      := sparx5-spi.o
>>
>>  obj-$(CONFIG_MULTIPLEXER)    += mux-core.o
>>  obj-$(CONFIG_MUX_ADG792A)    += mux-adg792a.o
>>  obj-$(CONFIG_MUX_ADGS1408)   += mux-adgs1408.o
>>  obj-$(CONFIG_MUX_GPIO)               += mux-gpio.o
>>  obj-$(CONFIG_MUX_MMIO)               += mux-mmio.o
>> +obj-$(CONFIG_SPI_DW_MMIO)    += mux-sparx5.o
>> diff --git a/drivers/mux/sparx5-spi.c b/drivers/mux/sparx5-spi.c
>> new file mode 100644
>> index 0000000000000..5fe9025b96a5e
>> --- /dev/null
>> +++ b/drivers/mux/sparx5-spi.c
>> @@ -0,0 +1,138 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Sparx5 SPI MUX driver
>> + *
>> + * Copyright (c) 2019 Microsemi Corporation
>> + *
>> + * Author: Lars Povlsen <lars.povlsen@microchip.com>
>> + */
>> +
>> +#include <linux/err.h>
>> +#include <linux/module.h>
>> +#include <linux/mux/driver.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/property.h>
>> +#include <linux/mux/driver.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/regmap.h>
>> +#include <linux/bitfield.h>
>> +
>> +#define MSCC_IF_SI_OWNER_SISL                        0
>> +#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_MAX_CS        16
>> +
>> +struct mux_sparx5 {
>> +     struct regmap *syscon;
>> +     u8 bus[SPARX5_MAX_CS];
>> +     int cur_bus;
>
> Surplus unused variable?
>
>> +};
>> +
>> +/*
>> + * Set the owner of the SPI interfaces
>> + */
>> +static void mux_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 mux_sparx5_set_cs_owner(struct mux_sparx5 *mux_sparx5,
>> +                                 u8 cs, u8 owner)
>> +{
>> +     u8 other = (owner == MSCC_IF_SI_OWNER_SIBM ?
>> +                 MSCC_IF_SI_OWNER_SIMC : MSCC_IF_SI_OWNER_SIBM);
>
> Empty line missing here.
>
>> +     if (mux_sparx5->bus[cs])
>> +             /* SPI2 */
>> +             mux_sparx5_set_owner(mux_sparx5->syscon, other, owner);
>> +     else
>> +             /* SPI1 */
>> +             mux_sparx5_set_owner(mux_sparx5->syscon, owner, other);
>> +}
>
>
> This smells like there are only two states for this mux control, and that
> the whole point of this driver is to make the exact numbering selectable.
> I don't see the point of that. To me, it looks like the pre-existing
> mmio-mux should be able to work. Something like this? Untested of course,
> and I might easily have misunderstood something...
>

Peter,

Good suggestion with "mmio-mux" - I overlooked it actually supports
regmap. It makes DT configuration a little less intuitive, but removes
the baggage of the dedicated sparx5 mux driver and bindings.

It also pushes the solution towards using "spi-mux", but at least the CS
in the DT does not have to be repeated.

So a little more DT stuff, but less new code.

I will try to implement the "mmio-mux"/"spi-mux" solution in a rev4.

Any comments from Mark?

Anyway, Peter, thank you for your comments.

(Your driver comments are all valid, but it appears the driver isn't
needed after all)

Cheers,

---Lars

> mux: mux-controller {
>         compatible = "mmio-mux"
>         #mux-control-cells = <1>;
>
>         /* SI_OWNER and SI2_OWNER in GENERAL_CTRL */
>         mux-reg-masks = <0x88 0xf0>;
> };
>
>
> spi0: spi@600104000 {
>         compatible = "microchip,sparx5-spi";
>         spi@0 {
>                 compatible = "spi-mux";
>                 mux-controls = <&mux 0>;
>                 reg = <0>;
>                 /* SI_OWNER = SIMC, SI2_OWNER = SIBM  --->  mux value 9 */
>                 spi-flash@9 {
>                         compatible = "jedec,spi-nor";
>                         reg = <9>;
>                 };
>         };
>         spi@e {
>                 compatible = "spi-mux";
>                 mux-controls = <&mux 0>;
>                 reg = <14>;
>                 /* SI_OWNER = SIBM, SI2_OWNER = SIMC  --->  mux value 6 */
>                 spi-flash@6 {
>                         compatible = "spi-nand";
>                         reg = <6>;
>                 };
>         };
> };
>
>> +
>> +static int mux_sparx5_set(struct mux_control *mux, int state)
>> +{
>> +     struct mux_sparx5 *mux_sparx5 = mux_chip_priv(mux->chip);
>> +
>> +     mux_sparx5_set_cs_owner(mux_sparx5, state, MSCC_IF_SI_OWNER_SIMC);
>> +
>> +     return 0;
>> +}
>> +
>> +static const struct mux_control_ops mux_sparx5_ops = {
>> +     .set = mux_sparx5_set,
>> +};
>> +
>> +static const struct of_device_id mux_sparx5_dt_ids[] = {
>> +     { .compatible = "microchip,sparx5-spi-mux", },
>> +     { /* sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(of, mux_sparx5_dt_ids);
>> +
>> +static int mux_sparx5_probe(struct platform_device *pdev)
>> +{
>> +     struct device *dev = &pdev->dev;
>> +     struct mux_chip *mux_chip;
>> +     struct mux_sparx5 *mux_sparx5;
>> +     struct device_node *nc;
>> +     const char *syscon_name = "microchip,sparx5-cpu-syscon";
>> +     int ret;
>> +
>> +     mux_chip = devm_mux_chip_alloc(dev, 1, sizeof(*mux_sparx5));
>> +     if (IS_ERR(mux_chip))
>> +             return PTR_ERR(mux_chip);
>> +
>> +     mux_sparx5 = mux_chip_priv(mux_chip);
>> +     mux_chip->ops = &mux_sparx5_ops;
>> +
>> +     mux_sparx5->syscon =
>> +             syscon_regmap_lookup_by_compatible(syscon_name);
>> +     if (IS_ERR(mux_sparx5->syscon)) {
>> +             dev_err(dev, "No syscon map %s\n", syscon_name);
>> +             return PTR_ERR(mux_sparx5->syscon);
>> +     }
>> +
>> +     /* Get bus interface mapping */
>> +     for_each_available_child_of_node(dev->of_node, nc) {
>> +             u32 cs, bus;
>> +
>> +             if (of_property_read_u32(nc, "reg", &cs) == 0 &&
>> +                 cs < SPARX5_MAX_CS &&
>> +                 of_property_read_u32(nc, "microchip,bus-interface",
>> +                                      &bus) == 0)
>> +                     mux_sparx5->bus[cs] = bus;
>
> The above if is a mess. The kernel model is to handle the exceptional cases
> first and break/goto/continue/return/whatever so that the interesting code
> can happen at lower indentation level.
>
>                 if (of_property_read_u32(nc, "reg", &cs))
>                         continue;
>                 if (cs >= SPARX5_MAX_CS)
>                         continue;
>                 if (of_property_read_u32(nc, "microchip,bus-interface", &bus))
>                         continue;
>
>                 mux_sparc5->bus[cs] = bus;
>
> Cheers,
> Peter
>
>> +     }
>> +
>> +     mux_chip->mux->states = SPARX5_MAX_CS;
>> +
>> +     ret = devm_mux_chip_register(dev, mux_chip);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     dev_info(dev, "%u-way mux-controller registered\n",
>> +              mux_chip->mux->states);
>> +
>> +     return 0;
>> +}
>> +
>> +static struct platform_driver mux_sparx5_driver = {
>> +     .driver = {
>> +             .name = "sparx5-mux",
>> +             .of_match_table = of_match_ptr(mux_sparx5_dt_ids),
>> +     },
>> +     .probe = mux_sparx5_probe,
>> +};
>> +module_platform_driver(mux_sparx5_driver);
>>

-- 
Lars Povlsen,
Microchip

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

* Re: [PATCH v3 5/8] dt-bindings: snps,dw-apb-ssi: Add sparx5 support, plus snps,rx-sample-delay-ns property
  2020-07-02 10:13 ` [PATCH v3 5/8] dt-bindings: snps,dw-apb-ssi: Add sparx5 support, plus snps,rx-sample-delay-ns property Lars Povlsen
@ 2020-07-13 19:22   ` Rob Herring
  2020-07-13 19:52     ` Serge Semin
  0 siblings, 1 reply; 17+ messages in thread
From: Rob Herring @ 2020-07-13 19:22 UTC (permalink / raw)
  To: Lars Povlsen
  Cc: Mark Brown, Peter Rosin, Microchip Linux Driver Support,
	linux-spi, devicetree, linux-kernel, linux-arm-kernel,
	Serge Semin, Serge Semin

On Thu, Jul 02, 2020 at 12:13:28PM +0200, Lars Povlsen wrote:
> 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 "mux-controls" for the above compatible string
> 
> - Add the property "snps,rx-sample-delay-ns" for SPI slaves
> 
> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
> ---
>  .../bindings/spi/snps,dw-apb-ssi.yaml         | 28 +++++++++++++++++++
>  1 file changed, 28 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..9d9208391fae3 100644
> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> @@ -36,6 +36,8 @@ properties:
>                - mscc,ocelot-spi
>                - mscc,jaguar2-spi
>            - const: snps,dw-apb-ssi
> +      - description: Microchip Sparx5 SoC SPI Controller
> +        const: microchip,sparx5-spi
>        - description: Amazon Alpine SPI Controller
>          const: amazon,alpine-dw-apb-ssi
>        - description: Renesas RZ/N1 SPI Controller
> @@ -93,6 +95,19 @@ properties:
>        - const: tx
>        - const: rx
> 
> +if:
> +  properties:
> +    compatible:
> +      contains:
> +        const: microchip,sparx5-spi
> +
> +then:
> +  properties:
> +    mux-controls:
> +      description: A mux controller node for selecting SPI bus interface.
> +      maxItems: 1
> +      $ref: '/schemas/types.yaml#/definitions/phandle'

Can drop the type. You can assume common properties already have a 
defined type.

> +
>  patternProperties:
>    "^.*@[0-9a-f]+$":
>      type: object
> @@ -107,6 +122,14 @@ patternProperties:
>        spi-tx-bus-width:
>          const: 1
> 
> +      snps,rx-sample-delay-ns:

We already have 'rx-sample-delay-ns' from Rockchip SPI, so use that. But 
note that it applies to the SPI node. Does this need to be per SPI 
child?

BTW, the Rockchip controller appears to be a version of the DW 
controller.

> +        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
> +
>  unevaluatedProperties: false
> 
>  required:
> @@ -129,5 +152,10 @@ examples:
>        num-cs = <2>;
>        cs-gpios = <&gpio0 13 0>,
>                   <&gpio0 14 0>;
> +      spi-flash@1 {
> +        compatible = "spi-nand";
> +        reg = <1>;
> +        snps,rx-sample-delay-ns = <7>;
> +      };
>      };
>  ...
> --
> 2.27.0

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

* Re: [PATCH v3 6/8] dt-bindings: microchip,sparx5-spi-mux: Add Sparx5 SPI mux driver bindings
  2020-07-02 10:13 ` [PATCH v3 6/8] dt-bindings: microchip,sparx5-spi-mux: Add Sparx5 SPI mux driver bindings Lars Povlsen
@ 2020-07-13 19:29   ` Rob Herring
  2020-07-14  8:52     ` Lars Povlsen
  0 siblings, 1 reply; 17+ messages in thread
From: Rob Herring @ 2020-07-13 19:29 UTC (permalink / raw)
  To: Lars Povlsen
  Cc: Mark Brown, Peter Rosin, Microchip Linux Driver Support,
	linux-spi, devicetree, linux-kernel, linux-arm-kernel,
	Serge Semin, Serge Semin

On Thu, Jul 02, 2020 at 12:13:29PM +0200, Lars Povlsen wrote:
> The Microchip Sparx5 SPI controller has two bus segments, and use this
> mux to control the bus interface mapping for any chip selects. This
> decribes the bindings used to configure the mux driver.
> 
> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
> ---
>  .../mux/microchip,sparx5-spi-mux.yaml         | 71 +++++++++++++++++++
>  1 file changed, 71 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mux/microchip,sparx5-spi-mux.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mux/microchip,sparx5-spi-mux.yaml b/Documentation/devicetree/bindings/mux/microchip,sparx5-spi-mux.yaml
> new file mode 100644
> index 0000000000000..b0ce3b15a69e5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mux/microchip,sparx5-spi-mux.yaml
> @@ -0,0 +1,71 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mux/microchip,sparx5-spi-mux.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip Sparx5 SPI mux
> +
> +maintainers:
> +  - Lars Povlsen <lars.povlsen@microchip.com>
> +
> +description: |
> +  The Microchip Sparx5 SPI controller has two bus segments. In order
> +  to switch between the appropriate bus for any given SPI slave
> +  (defined by a chip select), this mux driver is used. The device tree
> +  node for the mux will define the bus mapping for any chip
> +  selects. The default bus mapping for any chip select is "0", such
> +  that only non-default mappings need to be explicitly defined.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - microchip,sparx5-spi-mux
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +  '#mux-control-cells':
> +    const: 0
> +
> +required:
> +  - compatible
> +
> +additionalProperties: false
> +
> +patternProperties:
> +  "^mux@[0-9a-f]$":
> +    type: object
> +
> +    properties:
> +      reg:
> +        description:
> +          Chip select to define bus mapping for.
> +        minimum: 0
> +        maximum: 15
> +
> +      microchip,bus-interface:
> +        description:
> +          The bus interface to use for this chip select.
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        enum: [0, 1]
> +
> +    required:
> +      - reg
> +      - microchip,bus-interface
> +
> +examples:
> +  - |
> +    mux: mux-controller {
> +      compatible = "microchip,sparx5-spi-mux";
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      #mux-control-cells = <0>;

How is this mux accessed? You have no control interface defined.

> +      mux@e {
> +        reg = <14>;
> +        microchip,bus-interface = <1>;

This looks odd. I take it that there's 2 muxes for this h/w? If so then 
#mux-control-cells should be 1 and the cell value can be whatever you 
want that is meaningful for the mux controller. Could be 0,1 or perhaps 
0xe if that's more useful.

Rob

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

* Re: [PATCH v3 5/8] dt-bindings: snps,dw-apb-ssi: Add sparx5 support, plus snps,rx-sample-delay-ns property
  2020-07-13 19:22   ` Rob Herring
@ 2020-07-13 19:52     ` Serge Semin
  2020-07-14  8:30       ` Lars Povlsen
  0 siblings, 1 reply; 17+ messages in thread
From: Serge Semin @ 2020-07-13 19:52 UTC (permalink / raw)
  To: Rob Herring
  Cc: Serge Semin, Lars Povlsen, Mark Brown, Peter Rosin,
	Microchip Linux Driver Support, linux-spi, devicetree,
	linux-kernel, linux-arm-kernel

On Mon, Jul 13, 2020 at 01:22:59PM -0600, Rob Herring wrote:
> On Thu, Jul 02, 2020 at 12:13:28PM +0200, Lars Povlsen wrote:
...
> 
> > +
> >  patternProperties:
> >    "^.*@[0-9a-f]+$":
> >      type: object
> > @@ -107,6 +122,14 @@ patternProperties:
> >        spi-tx-bus-width:
> >          const: 1
> > 
> > +      snps,rx-sample-delay-ns:
> 

> We already have 'rx-sample-delay-ns' from Rockchip SPI, so use that. But 
> note that it applies to the SPI node. Does this need to be per SPI 
> child?

It was me, who suggested to Lars to have that parameter moved to the SPI
sub-nodes. As I see it the property is highly dependent on the SPI slave device
the controller is communicating to. Some of the them may need the delay some
may not. It's not always the capacitance thing, but also depends on how good
the MISO signal a particular slave generates. So IMO the Rockchip SPI driver
developer should have moved that property to the sub-nodes too.

On the other hand if the Rx errors are caused by the MISO lane capacitance,
then it will be cumbersome to have the same property duplicated for each
sub-node. Then what about having the property supported by both the SPI
controller and the SPI-child nodes? For instance the SPI-controller
"rx-sample-delay-ns" will provide a default sample delay for each sub-node
instead of zero by default, while the individual sub-node "rx-sample-delay-ns"
property can be used to override the default value.

-Sergey

> 
> BTW, the Rockchip controller appears to be a version of the DW 
> controller.
> 
> > +        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
> > +
> >  unevaluatedProperties: false
> > 
> >  required:
> > @@ -129,5 +152,10 @@ examples:
> >        num-cs = <2>;
> >        cs-gpios = <&gpio0 13 0>,
> >                   <&gpio0 14 0>;
> > +      spi-flash@1 {
> > +        compatible = "spi-nand";
> > +        reg = <1>;
> > +        snps,rx-sample-delay-ns = <7>;
> > +      };
> >      };
> >  ...
> > --
> > 2.27.0

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

* Re: [PATCH v3 5/8] dt-bindings: snps,dw-apb-ssi: Add sparx5 support, plus snps,rx-sample-delay-ns property
  2020-07-13 19:52     ` Serge Semin
@ 2020-07-14  8:30       ` Lars Povlsen
  0 siblings, 0 replies; 17+ messages in thread
From: Lars Povlsen @ 2020-07-14  8:30 UTC (permalink / raw)
  To: Serge Semin, Mark Brown
  Cc: Rob Herring, Serge Semin, Lars Povlsen, Peter Rosin,
	Microchip Linux Driver Support, linux-spi, devicetree,
	linux-kernel, linux-arm-kernel


Serge Semin writes:

> On Mon, Jul 13, 2020 at 01:22:59PM -0600, Rob Herring wrote:
>> On Thu, Jul 02, 2020 at 12:13:28PM +0200, Lars Povlsen wrote:
> ...
>>
>> > +
>> >  patternProperties:
>> >    "^.*@[0-9a-f]+$":
>> >      type: object
>> > @@ -107,6 +122,14 @@ patternProperties:
>> >        spi-tx-bus-width:
>> >          const: 1
>> >
>> > +      snps,rx-sample-delay-ns:
>>
>
>> We already have 'rx-sample-delay-ns' from Rockchip SPI, so use that. But
>> note that it applies to the SPI node. Does this need to be per SPI
>> child?
>
> It was me, who suggested to Lars to have that parameter moved to the SPI
> sub-nodes. As I see it the property is highly dependent on the SPI slave device
> the controller is communicating to. Some of the them may need the delay some
> may not. It's not always the capacitance thing, but also depends on how good
> the MISO signal a particular slave generates. So IMO the Rockchip SPI driver
> developer should have moved that property to the sub-nodes too.
>

In the case with sparx5, having two bus interfaces, spi slaves may be on
different busses - making it obvious why you may need different settings.

But I guess even on the same bus the physical length of SPI connections
and device response delay to each device could play in as well.

Nevertheless, it *does* make sense to be able to specify per slave, but
a default could make the DT more terse. Should I add this to my patch?

I will remove the "snps," prefix now that the property is globally
established.

---Lars

> On the other hand if the Rx errors are caused by the MISO lane capacitance,
> then it will be cumbersome to have the same property duplicated for each
> sub-node. Then what about having the property supported by both the SPI
> controller and the SPI-child nodes? For instance the SPI-controller
> "rx-sample-delay-ns" will provide a default sample delay for each sub-node
> instead of zero by default, while the individual sub-node "rx-sample-delay-ns"
> property can be used to override the default value.
>
> -Sergey
>
>>
>> BTW, the Rockchip controller appears to be a version of the DW
>> controller.
>>
>> > +        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
>> > +
>> >  unevaluatedProperties: false
>> >
>> >  required:
>> > @@ -129,5 +152,10 @@ examples:
>> >        num-cs = <2>;
>> >        cs-gpios = <&gpio0 13 0>,
>> >                   <&gpio0 14 0>;
>> > +      spi-flash@1 {
>> > +        compatible = "spi-nand";
>> > +        reg = <1>;
>> > +        snps,rx-sample-delay-ns = <7>;
>> > +      };
>> >      };
>> >  ...
>> > --
>> > 2.27.0

-- 
Lars Povlsen,
Microchip

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

* Re: [PATCH v3 6/8] dt-bindings: microchip,sparx5-spi-mux: Add Sparx5 SPI mux driver bindings
  2020-07-13 19:29   ` Rob Herring
@ 2020-07-14  8:52     ` Lars Povlsen
  0 siblings, 0 replies; 17+ messages in thread
From: Lars Povlsen @ 2020-07-14  8:52 UTC (permalink / raw)
  To: Rob Herring
  Cc: Lars Povlsen, Mark Brown, Peter Rosin,
	Microchip Linux Driver Support, linux-spi, devicetree,
	linux-kernel, linux-arm-kernel, Serge Semin, Serge Semin


Rob Herring writes:

> On Thu, Jul 02, 2020 at 12:13:29PM +0200, Lars Povlsen wrote:
>> The Microchip Sparx5 SPI controller has two bus segments, and use this
>> mux to control the bus interface mapping for any chip selects. This
>> decribes the bindings used to configure the mux driver.
>>
>> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
>> ---
>>  .../mux/microchip,sparx5-spi-mux.yaml         | 71 +++++++++++++++++++
>>  1 file changed, 71 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mux/microchip,sparx5-spi-mux.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/mux/microchip,sparx5-spi-mux.yaml b/Documentation/devicetree/bindings/mux/microchip,sparx5-spi-mux.yaml
>> new file mode 100644
>> index 0000000000000..b0ce3b15a69e5
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mux/microchip,sparx5-spi-mux.yaml
>> @@ -0,0 +1,71 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/mux/microchip,sparx5-spi-mux.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Microchip Sparx5 SPI mux
>> +
>> +maintainers:
>> +  - Lars Povlsen <lars.povlsen@microchip.com>
>> +
>> +description: |
>> +  The Microchip Sparx5 SPI controller has two bus segments. In order
>> +  to switch between the appropriate bus for any given SPI slave
>> +  (defined by a chip select), this mux driver is used. The device tree
>> +  node for the mux will define the bus mapping for any chip
>> +  selects. The default bus mapping for any chip select is "0", such
>> +  that only non-default mappings need to be explicitly defined.
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - microchip,sparx5-spi-mux
>> +
>> +  '#address-cells':
>> +    const: 1
>> +
>> +  '#size-cells':
>> +    const: 0
>> +
>> +  '#mux-control-cells':
>> +    const: 0
>> +
>> +required:
>> +  - compatible
>> +
>> +additionalProperties: false
>> +
>> +patternProperties:
>> +  "^mux@[0-9a-f]$":
>> +    type: object
>> +
>> +    properties:
>> +      reg:
>> +        description:
>> +          Chip select to define bus mapping for.
>> +        minimum: 0
>> +        maximum: 15
>> +
>> +      microchip,bus-interface:
>> +        description:
>> +          The bus interface to use for this chip select.
>> +        $ref: /schemas/types.yaml#/definitions/uint32
>> +        enum: [0, 1]
>> +
>> +    required:
>> +      - reg
>> +      - microchip,bus-interface
>> +
>> +examples:
>> +  - |
>> +    mux: mux-controller {
>> +      compatible = "microchip,sparx5-spi-mux";
>> +      #address-cells = <1>;
>> +      #size-cells = <0>;
>> +      #mux-control-cells = <0>;
>
> How is this mux accessed? You have no control interface defined.
>
>> +      mux@e {
>> +        reg = <14>;
>> +        microchip,bus-interface = <1>;
>
> This looks odd. I take it that there's 2 muxes for this h/w? If so then
> #mux-control-cells should be 1 and the cell value can be whatever you
> want that is meaningful for the mux controller. Could be 0,1 or perhaps
> 0xe if that's more useful.
>

Rob,

The intended use was for the SPI driver to use mux_control_select(mux,
<cs>) and then have the mux driver translate the <cs> to the right bus
interface, according to its configuration. The SPI driver would have a
"mux-controls" property bound to this mux.

Anyway, I am getting pushed in the direction of using the "mux-mmio" and
"spi-mux" combo, so this driver and bindings are being dropped again.

I am currently awaiting the "rx-sample-delay-ns" issue to be clarified
such that I can refresh the series.

Thank you for your comments!

> Rob

-- 
Lars Povlsen,
Microchip

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

end of thread, other threads:[~2020-07-14  8:52 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-02 10:13 [PATCH v3 0/8] spi: Adding support for Microchip Sparx5 SoC Lars Povlsen
2020-07-02 10:13 ` [PATCH v3 1/8] spi: dw: Add support for RX sample delay register Lars Povlsen
2020-07-02 10:13 ` [PATCH v3 2/8] arm64: dts: sparx5: Add SPI controller and SPI mux Lars Povlsen
2020-07-02 10:13 ` [PATCH v3 3/8] spi: dw: Add Microchip Sparx5 support Lars Povlsen
2020-07-02 10:13 ` [PATCH v3 4/8] mux: sparx5: Add Sparx5 SPI mux driver Lars Povlsen
2020-07-02 11:33   ` Lars Povlsen
2020-07-02 11:36   ` Peter Rosin
2020-07-03  9:14     ` Lars Povlsen
2020-07-02 10:13 ` [PATCH v3 5/8] dt-bindings: snps,dw-apb-ssi: Add sparx5 support, plus snps,rx-sample-delay-ns property Lars Povlsen
2020-07-13 19:22   ` Rob Herring
2020-07-13 19:52     ` Serge Semin
2020-07-14  8:30       ` Lars Povlsen
2020-07-02 10:13 ` [PATCH v3 6/8] dt-bindings: microchip,sparx5-spi-mux: Add Sparx5 SPI mux driver bindings Lars Povlsen
2020-07-13 19:29   ` Rob Herring
2020-07-14  8:52     ` Lars Povlsen
2020-07-02 10:13 ` [PATCH v3 7/8] arm64: dts: sparx5: Add spi-nor support Lars Povlsen
2020-07-02 10:13 ` [PATCH v3 8/8] arm64: dts: sparx5: Add spi-nand devices Lars Povlsen

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