linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] spi: Fix FSI-attached controller and AT25 drivers
@ 2020-08-20 17:02 Eddie James
  2020-08-20 17:02 ` [PATCH 1/7] spi: fsi: Handle 9 to 15 byte transfers lengths Eddie James
                   ` (6 more replies)
  0 siblings, 7 replies; 17+ messages in thread
From: Eddie James @ 2020-08-20 17:02 UTC (permalink / raw)
  To: linux-spi
  Cc: linux-kernel, devicetree, eajames, joel, bradleyb, broonie,
	robh+dt, arnd

This series implements a number of fixes for the FSI-attached SPI controller
driver and the AT25 eeprom driver.

Brad Bishop (4):
  spi: fsi: Handle 9 to 15 byte transfers lengths
  spi: fsi: Fix clock running too fast
  spi: fsi: Fix use of the bneq+ sequencer instruction
  eeprom: at25: Split reads into chunks and cap write size

Eddie James (3):
  dt-bindings: fsi: fsi2spi: Document new restricted property
  spi: fsi: Implement restricted size for certain controllers
  spi: fsi: Check mux status before transfers

 .../devicetree/bindings/fsi/ibm,fsi2spi.yaml  |  10 ++
 drivers/misc/eeprom/at25.c                    |  94 +++++++-----
 drivers/spi/spi-fsi.c                         | 139 ++++++++++++++----
 3 files changed, 172 insertions(+), 71 deletions(-)

-- 
2.26.2


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

* [PATCH 1/7] spi: fsi: Handle 9 to 15 byte transfers lengths
  2020-08-20 17:02 [PATCH 0/7] spi: Fix FSI-attached controller and AT25 drivers Eddie James
@ 2020-08-20 17:02 ` Eddie James
  2020-08-20 17:02 ` [PATCH 2/7] spi: fsi: Fix clock running too fast Eddie James
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Eddie James @ 2020-08-20 17:02 UTC (permalink / raw)
  To: linux-spi
  Cc: linux-kernel, devicetree, eajames, joel, bradleyb, broonie,
	robh+dt, arnd

From: Brad Bishop <bradleyb@fuzziesquirrel.com>

The trailing <len> - 8 bytes of transfer data in this size range is no
longer ignored.

Fixes: bbb6b2f9865b ("spi: Add FSI-attached SPI controller driver")
Signed-off-by: Brad Bishop <bradleyb@fuzziesquirrel.com>
Signed-off-by: Eddie James <eajames@linux.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/spi/spi-fsi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-fsi.c b/drivers/spi/spi-fsi.c
index 37a3e0f8e752..8f64af0140e0 100644
--- a/drivers/spi/spi-fsi.c
+++ b/drivers/spi/spi-fsi.c
@@ -258,15 +258,15 @@ static int fsi_spi_sequence_transfer(struct fsi_spi *ctx,
 	if (loops > 1) {
 		fsi_spi_sequence_add(seq, SPI_FSI_SEQUENCE_BRANCH(idx));
 
-		if (rem)
-			fsi_spi_sequence_add(seq, rem);
-
 		rc = fsi_spi_write_reg(ctx, SPI_FSI_COUNTER_CFG,
 				       SPI_FSI_COUNTER_CFG_LOOPS(loops - 1));
 		if (rc)
 			return rc;
 	}
 
+	if (rem)
+		fsi_spi_sequence_add(seq, rem);
+
 	return 0;
 }
 
-- 
2.26.2


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

* [PATCH 2/7] spi: fsi: Fix clock running too fast
  2020-08-20 17:02 [PATCH 0/7] spi: Fix FSI-attached controller and AT25 drivers Eddie James
  2020-08-20 17:02 ` [PATCH 1/7] spi: fsi: Handle 9 to 15 byte transfers lengths Eddie James
@ 2020-08-20 17:02 ` Eddie James
  2020-08-20 17:12   ` Mark Brown
  2020-08-20 17:02 ` [PATCH 3/7] spi: fsi: Fix use of the bneq+ sequencer instruction Eddie James
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Eddie James @ 2020-08-20 17:02 UTC (permalink / raw)
  To: linux-spi
  Cc: linux-kernel, devicetree, eajames, joel, bradleyb, broonie,
	robh+dt, arnd

From: Brad Bishop <bradleyb@fuzziesquirrel.com>

Use a clock divider tuned to a 200MHz FSI clock.  Use of the previous
divider at 200MHz results in corrupt data from endpoint devices. Ideally
the clock divider would be calculated from the FSI clock, but that
would require some significant work on the FSI driver.

Signed-off-by: Brad Bishop <bradleyb@fuzziesquirrel.com>
Signed-off-by: Eddie James <eajames@linux.ibm.com>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/spi/spi-fsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-fsi.c b/drivers/spi/spi-fsi.c
index 8f64af0140e0..559d0ff981f3 100644
--- a/drivers/spi/spi-fsi.c
+++ b/drivers/spi/spi-fsi.c
@@ -350,7 +350,7 @@ static int fsi_spi_transfer_init(struct fsi_spi *ctx)
 	u64 status = 0ULL;
 	u64 wanted_clock_cfg = SPI_FSI_CLOCK_CFG_ECC_DISABLE |
 		SPI_FSI_CLOCK_CFG_SCK_NO_DEL |
-		FIELD_PREP(SPI_FSI_CLOCK_CFG_SCK_DIV, 4);
+		FIELD_PREP(SPI_FSI_CLOCK_CFG_SCK_DIV, 19);
 
 	end = jiffies + msecs_to_jiffies(SPI_FSI_INIT_TIMEOUT_MS);
 	do {
-- 
2.26.2


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

* [PATCH 3/7] spi: fsi: Fix use of the bneq+ sequencer instruction
  2020-08-20 17:02 [PATCH 0/7] spi: Fix FSI-attached controller and AT25 drivers Eddie James
  2020-08-20 17:02 ` [PATCH 1/7] spi: fsi: Handle 9 to 15 byte transfers lengths Eddie James
  2020-08-20 17:02 ` [PATCH 2/7] spi: fsi: Fix clock running too fast Eddie James
@ 2020-08-20 17:02 ` Eddie James
  2020-08-20 17:02 ` [PATCH 4/7] dt-bindings: fsi: fsi2spi: Document new restricted property Eddie James
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Eddie James @ 2020-08-20 17:02 UTC (permalink / raw)
  To: linux-spi
  Cc: linux-kernel, devicetree, eajames, joel, bradleyb, broonie,
	robh+dt, arnd

From: Brad Bishop <bradleyb@fuzziesquirrel.com>

All of the switches in N2_count_control in the counter configuration are
required to make the branch if not equal and increment command work.
Set them when using bneq+.

A side effect of this mode requires a dummy write to TDR when both
transmitting and receiving otherwise the controller won't start shifting
receive data.

It is likely not possible to avoid TDR underrun errors in this mode and
they are harmless, so do not check for them.

Fixes: bbb6b2f9865b ("spi: Add FSI-attached SPI controller driver")
Signed-off-by: Brad Bishop <bradleyb@fuzziesquirrel.com>
Signed-off-by: Eddie James <eajames@linux.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/spi/spi-fsi.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-fsi.c b/drivers/spi/spi-fsi.c
index 559d0ff981f3..c31a852b6a3e 100644
--- a/drivers/spi/spi-fsi.c
+++ b/drivers/spi/spi-fsi.c
@@ -29,6 +29,10 @@
 #define SPI_FSI_ERROR			0x0
 #define SPI_FSI_COUNTER_CFG		0x1
 #define  SPI_FSI_COUNTER_CFG_LOOPS(x)	 (((u64)(x) & 0xffULL) << 32)
+#define  SPI_FSI_COUNTER_CFG_N2_RX	 BIT_ULL(8)
+#define  SPI_FSI_COUNTER_CFG_N2_TX	 BIT_ULL(9)
+#define  SPI_FSI_COUNTER_CFG_N2_IMPLICIT BIT_ULL(10)
+#define  SPI_FSI_COUNTER_CFG_N2_RELOAD	 BIT_ULL(11)
 #define SPI_FSI_CFG1			0x2
 #define SPI_FSI_CLOCK_CFG		0x3
 #define  SPI_FSI_CLOCK_CFG_MM_ENABLE	 BIT_ULL(32)
@@ -61,7 +65,7 @@
 #define  SPI_FSI_STATUS_RDR_OVERRUN	 BIT_ULL(62)
 #define  SPI_FSI_STATUS_RDR_FULL	 BIT_ULL(63)
 #define  SPI_FSI_STATUS_ANY_ERROR	 \
-	(SPI_FSI_STATUS_ERROR | SPI_FSI_STATUS_TDR_UNDERRUN | \
+	(SPI_FSI_STATUS_ERROR | \
 	 SPI_FSI_STATUS_TDR_OVERRUN | SPI_FSI_STATUS_RDR_UNDERRUN | \
 	 SPI_FSI_STATUS_RDR_OVERRUN)
 #define SPI_FSI_PORT_CTRL		0x9
@@ -238,6 +242,7 @@ static int fsi_spi_sequence_transfer(struct fsi_spi *ctx,
 	int rc;
 	u8 len = min(transfer->len, 8U);
 	u8 rem = transfer->len % len;
+	u64 cfg = 0ULL;
 
 	loops = transfer->len / len;
 
@@ -258,8 +263,14 @@ static int fsi_spi_sequence_transfer(struct fsi_spi *ctx,
 	if (loops > 1) {
 		fsi_spi_sequence_add(seq, SPI_FSI_SEQUENCE_BRANCH(idx));
 
-		rc = fsi_spi_write_reg(ctx, SPI_FSI_COUNTER_CFG,
-				       SPI_FSI_COUNTER_CFG_LOOPS(loops - 1));
+		cfg = SPI_FSI_COUNTER_CFG_LOOPS(loops - 1);
+		if (transfer->rx_buf)
+			cfg |= SPI_FSI_COUNTER_CFG_N2_RX |
+				SPI_FSI_COUNTER_CFG_N2_TX |
+				SPI_FSI_COUNTER_CFG_N2_IMPLICIT |
+				SPI_FSI_COUNTER_CFG_N2_RELOAD;
+
+		rc = fsi_spi_write_reg(ctx, SPI_FSI_COUNTER_CFG, cfg);
 		if (rc)
 			return rc;
 	}
@@ -275,6 +286,7 @@ static int fsi_spi_transfer_data(struct fsi_spi *ctx,
 {
 	int rc = 0;
 	u64 status = 0ULL;
+	u64 cfg = 0ULL;
 
 	if (transfer->tx_buf) {
 		int nb;
@@ -312,6 +324,16 @@ static int fsi_spi_transfer_data(struct fsi_spi *ctx,
 		u64 in = 0ULL;
 		u8 *rx = transfer->rx_buf;
 
+		rc = fsi_spi_read_reg(ctx, SPI_FSI_COUNTER_CFG, &cfg);
+		if (rc)
+			return rc;
+
+		if (cfg & SPI_FSI_COUNTER_CFG_N2_IMPLICIT) {
+			rc = fsi_spi_write_reg(ctx, SPI_FSI_DATA_TX, 0);
+			if (rc)
+				return rc;
+		}
+
 		while (transfer->len > recv) {
 			do {
 				rc = fsi_spi_read_reg(ctx, SPI_FSI_STATUS,
-- 
2.26.2


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

* [PATCH 4/7] dt-bindings: fsi: fsi2spi: Document new restricted property
  2020-08-20 17:02 [PATCH 0/7] spi: Fix FSI-attached controller and AT25 drivers Eddie James
                   ` (2 preceding siblings ...)
  2020-08-20 17:02 ` [PATCH 3/7] spi: fsi: Fix use of the bneq+ sequencer instruction Eddie James
@ 2020-08-20 17:02 ` Eddie James
  2020-08-20 17:14   ` Mark Brown
  2020-09-08 20:44   ` Rob Herring
  2020-08-20 17:02 ` [PATCH 5/7] spi: fsi: Implement restricted size for certain controllers Eddie James
                   ` (2 subsequent siblings)
  6 siblings, 2 replies; 17+ messages in thread
From: Eddie James @ 2020-08-20 17:02 UTC (permalink / raw)
  To: linux-spi
  Cc: linux-kernel, devicetree, eajames, joel, bradleyb, broonie,
	robh+dt, arnd

Add documentation for the "fsi2spi,restricted" property which indicates
a controller shouldn't sequence loops and therefore has a smaller
transfer size.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
Acked-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 Documentation/devicetree/bindings/fsi/ibm,fsi2spi.yaml | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/fsi/ibm,fsi2spi.yaml b/Documentation/devicetree/bindings/fsi/ibm,fsi2spi.yaml
index b26d4b4be743..0d2fb071fd00 100644
--- a/Documentation/devicetree/bindings/fsi/ibm,fsi2spi.yaml
+++ b/Documentation/devicetree/bindings/fsi/ibm,fsi2spi.yaml
@@ -24,6 +24,16 @@ properties:
     items:
       - description: FSI slave address
 
+patternProperties:
+  "^spi(@.*|-[0-9a-f])*$":
+    type: object
+
+    properties:
+      fsi2spi,restricted:
+        description: indicates the controller should not use looping in the
+          sequencer and therefore has a smaller maximum transfer size
+        type: boolean
+
 required:
   - compatible
   - reg
-- 
2.26.2


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

* [PATCH 5/7] spi: fsi: Implement restricted size for certain controllers
  2020-08-20 17:02 [PATCH 0/7] spi: Fix FSI-attached controller and AT25 drivers Eddie James
                   ` (3 preceding siblings ...)
  2020-08-20 17:02 ` [PATCH 4/7] dt-bindings: fsi: fsi2spi: Document new restricted property Eddie James
@ 2020-08-20 17:02 ` Eddie James
  2020-08-20 17:19   ` Mark Brown
  2020-08-20 17:02 ` [PATCH 6/7] spi: fsi: Check mux status before transfers Eddie James
  2020-08-20 17:02 ` [PATCH 7/7] eeprom: at25: Split reads into chunks and cap write size Eddie James
  6 siblings, 1 reply; 17+ messages in thread
From: Eddie James @ 2020-08-20 17:02 UTC (permalink / raw)
  To: linux-spi
  Cc: linux-kernel, devicetree, eajames, joel, bradleyb, broonie,
	robh+dt, arnd

Some of the FSI-attached SPI controllers cannot use the loop command in
programming the sequencer due to security requirements. Add a boolean
devicetree property that describes this condition and restrict the
size for these controllers. Also, add more transfers directly in the
sequence up to the length of the sequence register.

Fixes: bbb6b2f9865b ("spi: Add FSI-attached SPI controller driver")
Signed-off-by: Eddie James <eajames@linux.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/spi/spi-fsi.c | 65 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 53 insertions(+), 12 deletions(-)

diff --git a/drivers/spi/spi-fsi.c b/drivers/spi/spi-fsi.c
index c31a852b6a3e..53cfa201e187 100644
--- a/drivers/spi/spi-fsi.c
+++ b/drivers/spi/spi-fsi.c
@@ -24,7 +24,8 @@
 
 #define SPI_FSI_BASE			0x70000
 #define SPI_FSI_INIT_TIMEOUT_MS		1000
-#define SPI_FSI_MAX_TRANSFER_SIZE	2048
+#define SPI_FSI_MAX_XFR_SIZE		2048
+#define SPI_FSI_MAX_XFR_SIZE_RESTRICTED	32
 
 #define SPI_FSI_ERROR			0x0
 #define SPI_FSI_COUNTER_CFG		0x1
@@ -74,6 +75,8 @@ struct fsi_spi {
 	struct device *dev;	/* SPI controller device */
 	struct fsi_device *fsi;	/* FSI2SPI CFAM engine device */
 	u32 base;
+	size_t max_xfr_size;
+	bool restricted;
 };
 
 struct fsi_spi_sequence {
@@ -209,8 +212,12 @@ static int fsi_spi_reset(struct fsi_spi *ctx)
 	if (rc)
 		return rc;
 
-	return fsi_spi_write_reg(ctx, SPI_FSI_CLOCK_CFG,
-				 SPI_FSI_CLOCK_CFG_RESET2);
+	rc = fsi_spi_write_reg(ctx, SPI_FSI_CLOCK_CFG,
+			       SPI_FSI_CLOCK_CFG_RESET2);
+	if (rc)
+		return rc;
+
+	return fsi_spi_write_reg(ctx, SPI_FSI_STATUS, 0ULL);
 }
 
 static int fsi_spi_sequence_add(struct fsi_spi_sequence *seq, u8 val)
@@ -218,8 +225,8 @@ static int fsi_spi_sequence_add(struct fsi_spi_sequence *seq, u8 val)
 	/*
 	 * Add the next byte of instruction to the 8-byte sequence register.
 	 * Then decrement the counter so that the next instruction will go in
-	 * the right place. Return the number of "slots" left in the sequence
-	 * register.
+	 * the right place. Return the index of the slot we just filled in the
+	 * sequence register.
 	 */
 	seq->data |= (u64)val << seq->bit;
 	seq->bit -= 8;
@@ -237,9 +244,11 @@ static int fsi_spi_sequence_transfer(struct fsi_spi *ctx,
 				     struct fsi_spi_sequence *seq,
 				     struct spi_transfer *transfer)
 {
+	bool docfg = false;
 	int loops;
 	int idx;
 	int rc;
+	u8 val = 0;
 	u8 len = min(transfer->len, 8U);
 	u8 rem = transfer->len % len;
 	u64 cfg = 0ULL;
@@ -247,22 +256,42 @@ static int fsi_spi_sequence_transfer(struct fsi_spi *ctx,
 	loops = transfer->len / len;
 
 	if (transfer->tx_buf) {
-		idx = fsi_spi_sequence_add(seq,
-					   SPI_FSI_SEQUENCE_SHIFT_OUT(len));
+		val = SPI_FSI_SEQUENCE_SHIFT_OUT(len);
+		idx = fsi_spi_sequence_add(seq, val);
+
 		if (rem)
 			rem = SPI_FSI_SEQUENCE_SHIFT_OUT(rem);
 	} else if (transfer->rx_buf) {
-		idx = fsi_spi_sequence_add(seq,
-					   SPI_FSI_SEQUENCE_SHIFT_IN(len));
+		val = SPI_FSI_SEQUENCE_SHIFT_IN(len);
+		idx = fsi_spi_sequence_add(seq, val);
+
 		if (rem)
 			rem = SPI_FSI_SEQUENCE_SHIFT_IN(rem);
 	} else {
 		return -EINVAL;
 	}
 
+	if (ctx->restricted) {
+		const int eidx = rem ? 5 : 6;
+
+		while (loops > 1 && idx <= eidx) {
+			idx = fsi_spi_sequence_add(seq, val);
+			loops--;
+			docfg = true;
+		}
+
+		if (loops > 1) {
+			dev_warn(ctx->dev, "No sequencer slots; aborting.\n");
+			return -EINVAL;
+		}
+	}
+
 	if (loops > 1) {
 		fsi_spi_sequence_add(seq, SPI_FSI_SEQUENCE_BRANCH(idx));
+		docfg = true;
+	}
 
+	if (docfg) {
 		cfg = SPI_FSI_COUNTER_CFG_LOOPS(loops - 1);
 		if (transfer->rx_buf)
 			cfg |= SPI_FSI_COUNTER_CFG_N2_RX |
@@ -273,6 +302,8 @@ static int fsi_spi_sequence_transfer(struct fsi_spi *ctx,
 		rc = fsi_spi_write_reg(ctx, SPI_FSI_COUNTER_CFG, cfg);
 		if (rc)
 			return rc;
+	} else {
+		fsi_spi_write_reg(ctx, SPI_FSI_COUNTER_CFG, 0ULL);
 	}
 
 	if (rem)
@@ -429,7 +460,7 @@ static int fsi_spi_transfer_one_message(struct spi_controller *ctlr,
 
 		/* Sequencer must do shift out (tx) first. */
 		if (!transfer->tx_buf ||
-		    transfer->len > SPI_FSI_MAX_TRANSFER_SIZE) {
+		    transfer->len > (ctx->max_xfr_size + 8)) {
 			rc = -EINVAL;
 			goto error;
 		}
@@ -453,7 +484,7 @@ static int fsi_spi_transfer_one_message(struct spi_controller *ctlr,
 
 			/* Sequencer can only do shift in (rx) after tx. */
 			if (next->rx_buf) {
-				if (next->len > SPI_FSI_MAX_TRANSFER_SIZE) {
+				if (next->len > ctx->max_xfr_size) {
 					rc = -EINVAL;
 					goto error;
 				}
@@ -498,7 +529,9 @@ static int fsi_spi_transfer_one_message(struct spi_controller *ctlr,
 
 static size_t fsi_spi_max_transfer_size(struct spi_device *spi)
 {
-	return SPI_FSI_MAX_TRANSFER_SIZE;
+	struct fsi_spi *ctx = spi_controller_get_devdata(spi->controller);
+
+	return ctx->max_xfr_size;
 }
 
 static int fsi_spi_probe(struct device *dev)
@@ -546,6 +579,14 @@ static int fsi_spi_probe(struct device *dev)
 		ctx->fsi = fsi;
 		ctx->base = base + SPI_FSI_BASE;
 
+		if (of_property_read_bool(np, "fsi2spi,restricted")) {
+			ctx->restricted = true;
+			ctx->max_xfr_size = SPI_FSI_MAX_XFR_SIZE_RESTRICTED;
+		} else {
+			ctx->restricted = false;
+			ctx->max_xfr_size = SPI_FSI_MAX_XFR_SIZE;
+		}
+
 		rc = devm_spi_register_controller(dev, ctlr);
 		if (rc)
 			spi_controller_put(ctlr);
-- 
2.26.2


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

* [PATCH 6/7] spi: fsi: Check mux status before transfers
  2020-08-20 17:02 [PATCH 0/7] spi: Fix FSI-attached controller and AT25 drivers Eddie James
                   ` (4 preceding siblings ...)
  2020-08-20 17:02 ` [PATCH 5/7] spi: fsi: Implement restricted size for certain controllers Eddie James
@ 2020-08-20 17:02 ` Eddie James
  2020-08-20 17:02 ` [PATCH 7/7] eeprom: at25: Split reads into chunks and cap write size Eddie James
  6 siblings, 0 replies; 17+ messages in thread
From: Eddie James @ 2020-08-20 17:02 UTC (permalink / raw)
  To: linux-spi
  Cc: linux-kernel, devicetree, eajames, joel, bradleyb, broonie,
	robh+dt, arnd

The SPI controllers are not accessible if the mux isn't set. Therefore,
check the mux status before starting a transfer and fail out if it isn't
set.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/spi/spi-fsi.c | 40 +++++++++++++++++++++++++++-------------
 1 file changed, 27 insertions(+), 13 deletions(-)

diff --git a/drivers/spi/spi-fsi.c b/drivers/spi/spi-fsi.c
index 53cfa201e187..9c0a4413ec3d 100644
--- a/drivers/spi/spi-fsi.c
+++ b/drivers/spi/spi-fsi.c
@@ -12,6 +12,7 @@
 
 #define FSI_ENGID_SPI			0x23
 #define FSI_MBOX_ROOT_CTRL_8		0x2860
+#define  FSI_MBOX_ROOT_CTRL_8_SPI_MUX	 0xf0000000
 
 #define FSI2SPI_DATA0			0x00
 #define FSI2SPI_DATA1			0x04
@@ -84,6 +85,26 @@ struct fsi_spi_sequence {
 	u64 data;
 };
 
+static int fsi_spi_check_mux(struct fsi_device *fsi, struct device *dev)
+{
+	int rc;
+	u32 root_ctrl_8;
+	__be32 root_ctrl_8_be;
+
+	rc = fsi_slave_read(fsi->slave, FSI_MBOX_ROOT_CTRL_8, &root_ctrl_8_be,
+			    sizeof(root_ctrl_8_be));
+	if (rc)
+		return rc;
+
+	root_ctrl_8 = be32_to_cpu(root_ctrl_8_be);
+	dev_dbg(dev, "Root control register 8: %08x\n", root_ctrl_8);
+	if ((root_ctrl_8 & FSI_MBOX_ROOT_CTRL_8_SPI_MUX) ==
+	     FSI_MBOX_ROOT_CTRL_8_SPI_MUX)
+		return 0;
+
+	return -ENOLINK;
+}
+
 static int fsi_spi_check_status(struct fsi_spi *ctx)
 {
 	int rc;
@@ -449,11 +470,15 @@ static int fsi_spi_transfer_init(struct fsi_spi *ctx)
 static int fsi_spi_transfer_one_message(struct spi_controller *ctlr,
 					struct spi_message *mesg)
 {
-	int rc = 0;
+	int rc;
 	u8 seq_slave = SPI_FSI_SEQUENCE_SEL_SLAVE(mesg->spi->chip_select + 1);
 	struct spi_transfer *transfer;
 	struct fsi_spi *ctx = spi_controller_get_devdata(ctlr);
 
+	rc = fsi_spi_check_mux(ctx->fsi, ctx->dev);
+	if (rc)
+		return rc;
+
 	list_for_each_entry(transfer, &mesg->transfers, transfer_list) {
 		struct fsi_spi_sequence seq;
 		struct spi_transfer *next = NULL;
@@ -537,24 +562,13 @@ static size_t fsi_spi_max_transfer_size(struct spi_device *spi)
 static int fsi_spi_probe(struct device *dev)
 {
 	int rc;
-	u32 root_ctrl_8;
 	struct device_node *np;
 	int num_controllers_registered = 0;
 	struct fsi_device *fsi = to_fsi_dev(dev);
 
-	/*
-	 * Check the SPI mux before attempting to probe. If the mux isn't set
-	 * then the SPI controllers can't access their slave devices.
-	 */
-	rc = fsi_slave_read(fsi->slave, FSI_MBOX_ROOT_CTRL_8, &root_ctrl_8,
-			    sizeof(root_ctrl_8));
+	rc = fsi_spi_check_mux(fsi, dev);
 	if (rc)
-		return rc;
-
-	if (!root_ctrl_8) {
-		dev_dbg(dev, "SPI mux not set, aborting probe.\n");
 		return -ENODEV;
-	}
 
 	for_each_available_child_of_node(dev->of_node, np) {
 		u32 base;
-- 
2.26.2


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

* [PATCH 7/7] eeprom: at25: Split reads into chunks and cap write size
  2020-08-20 17:02 [PATCH 0/7] spi: Fix FSI-attached controller and AT25 drivers Eddie James
                   ` (5 preceding siblings ...)
  2020-08-20 17:02 ` [PATCH 6/7] spi: fsi: Check mux status before transfers Eddie James
@ 2020-08-20 17:02 ` Eddie James
  2020-08-20 17:20   ` Mark Brown
  6 siblings, 1 reply; 17+ messages in thread
From: Eddie James @ 2020-08-20 17:02 UTC (permalink / raw)
  To: linux-spi
  Cc: linux-kernel, devicetree, eajames, joel, bradleyb, broonie,
	robh+dt, arnd

From: Brad Bishop <bradleyb@fuzziesquirrel.com>

Make use of spi_max_transfer_size to avoid requesting transfers that are
too large for some spi controllers.

Signed-off-by: Brad Bishop <bradleyb@fuzziesquirrel.com>
Signed-off-by: Eddie James <eajames@linux.ibm.com>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/misc/eeprom/at25.c | 94 ++++++++++++++++++++++----------------
 1 file changed, 54 insertions(+), 40 deletions(-)

diff --git a/drivers/misc/eeprom/at25.c b/drivers/misc/eeprom/at25.c
index ed8d38b09925..30f0704ab2d4 100644
--- a/drivers/misc/eeprom/at25.c
+++ b/drivers/misc/eeprom/at25.c
@@ -64,12 +64,17 @@ static int at25_ee_read(void *priv, unsigned int offset,
 {
 	struct at25_data *at25 = priv;
 	char *buf = val;
+	size_t max_chunk = spi_max_transfer_size(at25->spi);
+	size_t num_msgs = DIV_ROUND_UP(count, max_chunk);
+	size_t			nr_bytes = 0;
 	u8			command[EE_MAXADDRLEN + 1];
 	u8			*cp;
 	ssize_t			status;
 	struct spi_transfer	t[2];
 	struct spi_message	m;
 	u8			instr;
+	unsigned int		msg_offset;
+	size_t			msg_count;
 
 	if (unlikely(offset >= at25->chip.byte_len))
 		return -EINVAL;
@@ -78,57 +83,64 @@ static int at25_ee_read(void *priv, unsigned int offset,
 	if (unlikely(!count))
 		return -EINVAL;
 
-	cp = command;
-
-	instr = AT25_READ;
-	if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR)
-		if (offset >= (1U << (at25->addrlen * 8)))
-			instr |= AT25_INSTR_BIT3;
-	*cp++ = instr;
-
-	/* 8/16/24-bit address is written MSB first */
-	switch (at25->addrlen) {
-	default:	/* case 3 */
-		*cp++ = offset >> 16;
-		fallthrough;
-	case 2:
-		*cp++ = offset >> 8;
-		fallthrough;
-	case 1:
-	case 0:	/* can't happen: for better codegen */
-		*cp++ = offset >> 0;
-	}
+	msg_offset = (unsigned int)offset;
+	msg_count = min(count, max_chunk);
+	while (num_msgs) {
+		cp = command;
+
+		instr = AT25_READ;
+		if (at25->chip.flags & EE_INSTR_BIT3_IS_ADDR)
+			if (msg_offset >= (1U << (at25->addrlen * 8)))
+				instr |= AT25_INSTR_BIT3;
+		*cp++ = instr;
+
+		/* 8/16/24-bit address is written MSB first */
+		switch (at25->addrlen) {
+		default:	/* case 3 */
+			*cp++ = msg_offset >> 16;
+			fallthrough;
+		case 2:
+			*cp++ = msg_offset >> 8;
+			fallthrough;
+		case 1:
+		case 0:	/* can't happen: for better codegen */
+			*cp++ = msg_offset >> 0;
+		}
 
-	spi_message_init(&m);
-	memset(t, 0, sizeof(t));
+		spi_message_init(&m);
+		memset(t, 0, sizeof(t));
 
-	t[0].tx_buf = command;
-	t[0].len = at25->addrlen + 1;
-	spi_message_add_tail(&t[0], &m);
+		t[0].tx_buf = command;
+		t[0].len = at25->addrlen + 1;
+		spi_message_add_tail(&t[0], &m);
 
-	t[1].rx_buf = buf;
-	t[1].len = count;
-	spi_message_add_tail(&t[1], &m);
+		t[1].rx_buf = buf + nr_bytes;
+		t[1].len = msg_count;
+		spi_message_add_tail(&t[1], &m);
 
-	mutex_lock(&at25->lock);
+		mutex_lock(&at25->lock);
 
-	/* Read it all at once.
-	 *
-	 * REVISIT that's potentially a problem with large chips, if
-	 * other devices on the bus need to be accessed regularly or
-	 * this chip is clocked very slowly
-	 */
-	status = spi_sync(at25->spi, &m);
-	dev_dbg(&at25->spi->dev, "read %zu bytes at %d --> %zd\n",
-		count, offset, status);
+		status = spi_sync(at25->spi, &m);
 
-	mutex_unlock(&at25->lock);
-	return status;
+		mutex_unlock(&at25->lock);
+
+		if (status)
+			return status;
+
+		--num_msgs;
+		msg_offset += msg_count;
+		nr_bytes += msg_count;
+	}
+
+	dev_dbg(&at25->spi->dev, "read %zu bytes at %d\n",
+		count, offset);
+	return 0;
 }
 
 static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
 {
 	struct at25_data *at25 = priv;
+	size_t maxsz = spi_max_transfer_size(at25->spi);
 	const char *buf = val;
 	int			status = 0;
 	unsigned		buf_size;
@@ -191,6 +203,8 @@ static int at25_ee_write(void *priv, unsigned int off, void *val, size_t count)
 		segment = buf_size - (offset % buf_size);
 		if (segment > count)
 			segment = count;
+		if (segment > maxsz)
+			segment = maxsz;
 		memcpy(cp, buf, segment);
 		status = spi_write(at25->spi, bounce,
 				segment + at25->addrlen + 1);
-- 
2.26.2


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

* Re: [PATCH 2/7] spi: fsi: Fix clock running too fast
  2020-08-20 17:02 ` [PATCH 2/7] spi: fsi: Fix clock running too fast Eddie James
@ 2020-08-20 17:12   ` Mark Brown
  2020-08-20 21:06     ` Eddie James
  0 siblings, 1 reply; 17+ messages in thread
From: Mark Brown @ 2020-08-20 17:12 UTC (permalink / raw)
  To: Eddie James
  Cc: linux-spi, linux-kernel, devicetree, joel, bradleyb, robh+dt, arnd

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

On Thu, Aug 20, 2020 at 12:02:23PM -0500, Eddie James wrote:
> From: Brad Bishop <bradleyb@fuzziesquirrel.com>
> 
> Use a clock divider tuned to a 200MHz FSI clock.  Use of the previous
> divider at 200MHz results in corrupt data from endpoint devices. Ideally
> the clock divider would be calculated from the FSI clock, but that
> would require some significant work on the FSI driver.

Presumably this divider was chosen for FSI clocks that aren't 200MHz -
how will those be handled?

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

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

* Re: [PATCH 4/7] dt-bindings: fsi: fsi2spi: Document new restricted property
  2020-08-20 17:02 ` [PATCH 4/7] dt-bindings: fsi: fsi2spi: Document new restricted property Eddie James
@ 2020-08-20 17:14   ` Mark Brown
  2020-08-20 21:07     ` Eddie James
  2020-09-08 20:44   ` Rob Herring
  1 sibling, 1 reply; 17+ messages in thread
From: Mark Brown @ 2020-08-20 17:14 UTC (permalink / raw)
  To: Eddie James
  Cc: linux-spi, linux-kernel, devicetree, joel, bradleyb, robh+dt, arnd

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

On Thu, Aug 20, 2020 at 12:02:25PM -0500, Eddie James wrote:
> Add documentation for the "fsi2spi,restricted" property which indicates
> a controller shouldn't sequence loops and therefore has a smaller
> transfer size.

In what situation might someone set this?  It sounds like a
configuration option rather than a description of the hardware.

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

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

* Re: [PATCH 5/7] spi: fsi: Implement restricted size for certain controllers
  2020-08-20 17:02 ` [PATCH 5/7] spi: fsi: Implement restricted size for certain controllers Eddie James
@ 2020-08-20 17:19   ` Mark Brown
  0 siblings, 0 replies; 17+ messages in thread
From: Mark Brown @ 2020-08-20 17:19 UTC (permalink / raw)
  To: Eddie James
  Cc: linux-spi, linux-kernel, devicetree, joel, bradleyb, robh+dt, arnd

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

On Thu, Aug 20, 2020 at 12:02:26PM -0500, Eddie James wrote:
> Some of the FSI-attached SPI controllers cannot use the loop command in
> programming the sequencer due to security requirements. Add a boolean
> devicetree property that describes this condition and restrict the
> size for these controllers. Also, add more transfers directly in the
> sequence up to the length of the sequence register.

If there is one difference between implementations of the controller
there are likely to be more - why have a property here rather than use
compatible strings for the different hardware variants?  Compatible
strings mean that as more differences are discovered existing DTs
continue to work.

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

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

* Re: [PATCH 7/7] eeprom: at25: Split reads into chunks and cap write size
  2020-08-20 17:02 ` [PATCH 7/7] eeprom: at25: Split reads into chunks and cap write size Eddie James
@ 2020-08-20 17:20   ` Mark Brown
  0 siblings, 0 replies; 17+ messages in thread
From: Mark Brown @ 2020-08-20 17:20 UTC (permalink / raw)
  To: Eddie James
  Cc: linux-spi, linux-kernel, devicetree, joel, bradleyb, robh+dt, arnd

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

On Thu, Aug 20, 2020 at 12:02:28PM -0500, Eddie James wrote:
> From: Brad Bishop <bradleyb@fuzziesquirrel.com>
> 
> Make use of spi_max_transfer_size to avoid requesting transfers that are
> too large for some spi controllers.

This is an unrelated patch to a separate subsystem and should be sent
separately to the maintainers of that subsystem.

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

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

* Re: [PATCH 2/7] spi: fsi: Fix clock running too fast
  2020-08-20 17:12   ` Mark Brown
@ 2020-08-20 21:06     ` Eddie James
  2020-08-25  7:10       ` Joel Stanley
  0 siblings, 1 reply; 17+ messages in thread
From: Eddie James @ 2020-08-20 21:06 UTC (permalink / raw)
  To: Mark Brown
  Cc: linux-spi, linux-kernel, devicetree, joel, bradleyb, robh+dt, arnd


On 8/20/20 12:12 PM, Mark Brown wrote:
> On Thu, Aug 20, 2020 at 12:02:23PM -0500, Eddie James wrote:
>> From: Brad Bishop <bradleyb@fuzziesquirrel.com>
>>
>> Use a clock divider tuned to a 200MHz FSI clock.  Use of the previous
>> divider at 200MHz results in corrupt data from endpoint devices. Ideally
>> the clock divider would be calculated from the FSI clock, but that
>> would require some significant work on the FSI driver.
> Presumably this divider was chosen for FSI clocks that aren't 200MHz -
> how will those be handled?


They aren't handled at the moment, but 200MHz FSI represents the worst 
case, as it's the maximum. Slower FSI clocks will simply result in 
slower SPI clocks.

Thanks,

Eddie



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

* Re: [PATCH 4/7] dt-bindings: fsi: fsi2spi: Document new restricted property
  2020-08-20 17:14   ` Mark Brown
@ 2020-08-20 21:07     ` Eddie James
  2020-08-21 12:03       ` Mark Brown
  0 siblings, 1 reply; 17+ messages in thread
From: Eddie James @ 2020-08-20 21:07 UTC (permalink / raw)
  To: Mark Brown
  Cc: linux-spi, linux-kernel, devicetree, joel, bradleyb, robh+dt, arnd


On 8/20/20 12:14 PM, Mark Brown wrote:
> On Thu, Aug 20, 2020 at 12:02:25PM -0500, Eddie James wrote:
>> Add documentation for the "fsi2spi,restricted" property which indicates
>> a controller shouldn't sequence loops and therefore has a smaller
>> transfer size.
> In what situation might someone set this?  It sounds like a
> configuration option rather than a description of the hardware.


It is a description of the configuration of the hardware. For 
controllers that are configured to be restricted for security purposes, 
this property will be set.

Thanks,

Eddie



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

* Re: [PATCH 4/7] dt-bindings: fsi: fsi2spi: Document new restricted property
  2020-08-20 21:07     ` Eddie James
@ 2020-08-21 12:03       ` Mark Brown
  0 siblings, 0 replies; 17+ messages in thread
From: Mark Brown @ 2020-08-21 12:03 UTC (permalink / raw)
  To: Eddie James
  Cc: linux-spi, linux-kernel, devicetree, joel, bradleyb, robh+dt, arnd

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

On Thu, Aug 20, 2020 at 04:07:41PM -0500, Eddie James wrote:
> On 8/20/20 12:14 PM, Mark Brown wrote:
> > On Thu, Aug 20, 2020 at 12:02:25PM -0500, Eddie James wrote:

> > > Add documentation for the "fsi2spi,restricted" property which indicates
> > > a controller shouldn't sequence loops and therefore has a smaller
> > > transfer size.

> > In what situation might someone set this?  It sounds like a
> > configuration option rather than a description of the hardware.

> It is a description of the configuration of the hardware. For controllers
> that are configured to be restricted for security purposes, this property
> will be set.

The documentation needs to be a *lot* clearer - users need to be able to
relate this to whatever system configuration this corresponds to.  Right
now it says that the controller shouldn't sequence loops but there's no
indication of why this might be.

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

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

* Re: [PATCH 2/7] spi: fsi: Fix clock running too fast
  2020-08-20 21:06     ` Eddie James
@ 2020-08-25  7:10       ` Joel Stanley
  0 siblings, 0 replies; 17+ messages in thread
From: Joel Stanley @ 2020-08-25  7:10 UTC (permalink / raw)
  To: Eddie James
  Cc: Mark Brown, linux-spi, Linux Kernel Mailing List, devicetree,
	Brad Bishop, Rob Herring, Arnd Bergmann

On Thu, 20 Aug 2020 at 21:06, Eddie James <eajames@linux.ibm.com> wrote:
>
>
> On 8/20/20 12:12 PM, Mark Brown wrote:
> > On Thu, Aug 20, 2020 at 12:02:23PM -0500, Eddie James wrote:
> >> From: Brad Bishop <bradleyb@fuzziesquirrel.com>
> >>
> >> Use a clock divider tuned to a 200MHz FSI clock.  Use of the previous
> >> divider at 200MHz results in corrupt data from endpoint devices. Ideally
> >> the clock divider would be calculated from the FSI clock, but that
> >> would require some significant work on the FSI driver.
> > Presumably this divider was chosen for FSI clocks that aren't 200MHz -
> > how will those be handled?
>
>
> They aren't handled at the moment, but 200MHz FSI represents the worst
> case, as it's the maximum. Slower FSI clocks will simply result in
> slower SPI clocks.

That would be a good addition to the commit message, as I had the same
question too.

Cheers,

Joel

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

* Re: [PATCH 4/7] dt-bindings: fsi: fsi2spi: Document new restricted property
  2020-08-20 17:02 ` [PATCH 4/7] dt-bindings: fsi: fsi2spi: Document new restricted property Eddie James
  2020-08-20 17:14   ` Mark Brown
@ 2020-09-08 20:44   ` Rob Herring
  1 sibling, 0 replies; 17+ messages in thread
From: Rob Herring @ 2020-09-08 20:44 UTC (permalink / raw)
  To: Eddie James
  Cc: linux-spi, linux-kernel, devicetree, joel, bradleyb, broonie, arnd

On Thu, Aug 20, 2020 at 12:02:25PM -0500, Eddie James wrote:
> Add documentation for the "fsi2spi,restricted" property which indicates
> a controller shouldn't sequence loops and therefore has a smaller
> transfer size.
> 
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> Acked-by: Joel Stanley <joel@jms.id.au>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  Documentation/devicetree/bindings/fsi/ibm,fsi2spi.yaml | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/fsi/ibm,fsi2spi.yaml b/Documentation/devicetree/bindings/fsi/ibm,fsi2spi.yaml
> index b26d4b4be743..0d2fb071fd00 100644
> --- a/Documentation/devicetree/bindings/fsi/ibm,fsi2spi.yaml
> +++ b/Documentation/devicetree/bindings/fsi/ibm,fsi2spi.yaml
> @@ -24,6 +24,16 @@ properties:
>      items:
>        - description: FSI slave address
>  
> +patternProperties:
> +  "^spi(@.*|-[0-9a-f])*$":

Doesn't FSI define the unit-address format already? No need to do it 
again here. Plus the form 'spi-[0-9a-f]' isn't valid. Just "^spi@" 
should be enough.

> +    type: object
> +
> +    properties:
> +      fsi2spi,restricted:

[<vendor>],<property-name>

fsi2spi is not a vendor.

> +        description: indicates the controller should not use looping in the
> +          sequencer and therefore has a smaller maximum transfer size
> +        type: boolean
> +
>  required:
>    - compatible
>    - reg
> -- 
> 2.26.2
> 

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

end of thread, other threads:[~2020-09-08 20:44 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-20 17:02 [PATCH 0/7] spi: Fix FSI-attached controller and AT25 drivers Eddie James
2020-08-20 17:02 ` [PATCH 1/7] spi: fsi: Handle 9 to 15 byte transfers lengths Eddie James
2020-08-20 17:02 ` [PATCH 2/7] spi: fsi: Fix clock running too fast Eddie James
2020-08-20 17:12   ` Mark Brown
2020-08-20 21:06     ` Eddie James
2020-08-25  7:10       ` Joel Stanley
2020-08-20 17:02 ` [PATCH 3/7] spi: fsi: Fix use of the bneq+ sequencer instruction Eddie James
2020-08-20 17:02 ` [PATCH 4/7] dt-bindings: fsi: fsi2spi: Document new restricted property Eddie James
2020-08-20 17:14   ` Mark Brown
2020-08-20 21:07     ` Eddie James
2020-08-21 12:03       ` Mark Brown
2020-09-08 20:44   ` Rob Herring
2020-08-20 17:02 ` [PATCH 5/7] spi: fsi: Implement restricted size for certain controllers Eddie James
2020-08-20 17:19   ` Mark Brown
2020-08-20 17:02 ` [PATCH 6/7] spi: fsi: Check mux status before transfers Eddie James
2020-08-20 17:02 ` [PATCH 7/7] eeprom: at25: Split reads into chunks and cap write size Eddie James
2020-08-20 17:20   ` Mark Brown

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