* [PATCH linux dev-5.7 0/6] spi: Fix FSI-attached controller and AT25 drivers
@ 2020-07-29 20:45 Eddie James
2020-07-29 20:45 ` [PATCH linux dev-5.7 1/6] spi: fsi: Handle 9 to 15 byte transfers lengths Eddie James
` (7 more replies)
0 siblings, 8 replies; 15+ messages in thread
From: Eddie James @ 2020-07-29 20:45 UTC (permalink / raw)
To: openbmc; +Cc: joel, Eddie James
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 (2):
dt-bindings: fsi: fsi2spi: Document new restricted property
spi: fsi: Implement restricted size for certain controllers
.../devicetree/bindings/fsi/ibm,fsi2spi.yaml | 10 ++
drivers/misc/eeprom/at25.c | 94 ++++++++++--------
drivers/spi/spi-fsi.c | 99 +++++++++++++++----
3 files changed, 145 insertions(+), 58 deletions(-)
--
2.24.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH linux dev-5.7 1/6] spi: fsi: Handle 9 to 15 byte transfers lengths
2020-07-29 20:45 [PATCH linux dev-5.7 0/6] spi: Fix FSI-attached controller and AT25 drivers Eddie James
@ 2020-07-29 20:45 ` Eddie James
2020-07-29 23:41 ` Joel Stanley
2020-07-29 20:45 ` [PATCH linux dev-5.7 2/6] spi: fsi: Fix clock running too fast Eddie James
` (6 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Eddie James @ 2020-07-29 20:45 UTC (permalink / raw)
To: openbmc; +Cc: joel, Brad Bishop, Eddie James
From: Brad Bishop <bradleyb@fuzziesquirrel.com>
The trailing <len> - 8 bytes of transfer data in this size range is no
longer ignored.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
Signed-off-by: Brad Bishop <bradleyb@fuzziesquirrel.com>
---
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.24.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH linux dev-5.7 2/6] spi: fsi: Fix clock running too fast
2020-07-29 20:45 [PATCH linux dev-5.7 0/6] spi: Fix FSI-attached controller and AT25 drivers Eddie James
2020-07-29 20:45 ` [PATCH linux dev-5.7 1/6] spi: fsi: Handle 9 to 15 byte transfers lengths Eddie James
@ 2020-07-29 20:45 ` Eddie James
2020-07-29 23:25 ` Joel Stanley
2020-07-29 20:45 ` [PATCH linux dev-5.7 3/6] spi: fsi: Fix use of the bneq+ sequencer instruction Eddie James
` (5 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Eddie James @ 2020-07-29 20:45 UTC (permalink / raw)
To: openbmc; +Cc: joel, Brad Bishop, Eddie James
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: Eddie James <eajames@linux.ibm.com>
Signed-off-by: Brad Bishop <bradleyb@fuzziesquirrel.com>
---
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.24.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH linux dev-5.7 3/6] spi: fsi: Fix use of the bneq+ sequencer instruction
2020-07-29 20:45 [PATCH linux dev-5.7 0/6] spi: Fix FSI-attached controller and AT25 drivers Eddie James
2020-07-29 20:45 ` [PATCH linux dev-5.7 1/6] spi: fsi: Handle 9 to 15 byte transfers lengths Eddie James
2020-07-29 20:45 ` [PATCH linux dev-5.7 2/6] spi: fsi: Fix clock running too fast Eddie James
@ 2020-07-29 20:45 ` Eddie James
2020-07-29 23:27 ` Joel Stanley
2020-07-29 20:45 ` [PATCH linux dev-5.7 4/6] dt-bindings: fsi: fsi2spi: Document new restricted property Eddie James
` (4 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Eddie James @ 2020-07-29 20:45 UTC (permalink / raw)
To: openbmc; +Cc: joel, Brad Bishop, Eddie James
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.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
Signed-off-by: Brad Bishop <bradleyb@fuzziesquirrel.com>
---
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.24.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH linux dev-5.7 4/6] dt-bindings: fsi: fsi2spi: Document new restricted property
2020-07-29 20:45 [PATCH linux dev-5.7 0/6] spi: Fix FSI-attached controller and AT25 drivers Eddie James
` (2 preceding siblings ...)
2020-07-29 20:45 ` [PATCH linux dev-5.7 3/6] spi: fsi: Fix use of the bneq+ sequencer instruction Eddie James
@ 2020-07-29 20:45 ` Eddie James
2020-07-29 23:27 ` Joel Stanley
2020-07-29 20:45 ` [PATCH linux dev-5.7 5/6] spi: fsi: Implement restricted size for certain controllers Eddie James
` (3 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Eddie James @ 2020-07-29 20:45 UTC (permalink / raw)
To: openbmc; +Cc: joel, Eddie James
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>
---
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 893d81e54caa..ff16797061ec 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.24.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH linux dev-5.7 5/6] spi: fsi: Implement restricted size for certain controllers
2020-07-29 20:45 [PATCH linux dev-5.7 0/6] spi: Fix FSI-attached controller and AT25 drivers Eddie James
` (3 preceding siblings ...)
2020-07-29 20:45 ` [PATCH linux dev-5.7 4/6] dt-bindings: fsi: fsi2spi: Document new restricted property Eddie James
@ 2020-07-29 20:45 ` Eddie James
2020-07-29 23:33 ` Joel Stanley
2020-07-29 20:45 ` [PATCH linux dev-5.7 6/6] eeprom: at25: Split reads into chunks and cap write size Eddie James
` (2 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Eddie James @ 2020-07-29 20:45 UTC (permalink / raw)
To: openbmc; +Cc: joel, Eddie James
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.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
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.24.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH linux dev-5.7 6/6] eeprom: at25: Split reads into chunks and cap write size
2020-07-29 20:45 [PATCH linux dev-5.7 0/6] spi: Fix FSI-attached controller and AT25 drivers Eddie James
` (4 preceding siblings ...)
2020-07-29 20:45 ` [PATCH linux dev-5.7 5/6] spi: fsi: Implement restricted size for certain controllers Eddie James
@ 2020-07-29 20:45 ` Eddie James
2020-07-29 23:28 ` Milton Miller II
2020-07-29 23:35 ` [PATCH linux dev-5.7 0/6] spi: Fix FSI-attached controller and AT25 drivers Joel Stanley
7 siblings, 0 replies; 15+ messages in thread
From: Eddie James @ 2020-07-29 20:45 UTC (permalink / raw)
To: openbmc; +Cc: joel, Brad Bishop, Eddie James
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: Eddie James <eajames@linux.ibm.com>
Signed-off-by: Brad Bishop <bradleyb@fuzziesquirrel.com>
---
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 cde9a2fc1325..3ed041cb3083 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 = count / max_chunk + (bool)(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 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;
- /* fall through */
- case 2:
- *cp++ = offset >> 8;
- /* fall through */
- case 1:
- case 0: /* can't happen: for better codegen */
- *cp++ = offset >> 0;
- }
+ msg_offset = (unsigned) offset;
+ msg_count = min(count, max_chunk);
+ while (num_msgs) {
+ cp = command;
- spi_message_init(&m);
- memset(t, 0, sizeof(t));
+ 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;
- t[0].tx_buf = command;
- t[0].len = at25->addrlen + 1;
- spi_message_add_tail(&t[0], &m);
+ /* 8/16/24-bit address is written MSB first */
+ switch (at25->addrlen) {
+ default: /* case 3 */
+ *cp++ = msg_offset >> 16;
+ /* fall through */
+ case 2:
+ *cp++ = msg_offset >> 8;
+ /* fall through */
+ case 1:
+ case 0: /* can't happen: for better codegen */
+ *cp++ = msg_offset >> 0;
+ }
- t[1].rx_buf = buf;
- t[1].len = count;
- spi_message_add_tail(&t[1], &m);
+ spi_message_init(&m);
+ memset(t, 0, sizeof(t));
- mutex_lock(&at25->lock);
+ t[0].tx_buf = command;
+ t[0].len = at25->addrlen + 1;
+ spi_message_add_tail(&t[0], &m);
- /* 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);
+ t[1].rx_buf = buf + nr_bytes;
+ t[1].len = msg_count;
+ spi_message_add_tail(&t[1], &m);
- mutex_unlock(&at25->lock);
- return status;
+ mutex_lock(&at25->lock);
+
+ status = spi_sync(at25->spi, &m);
+
+ 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.24.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH linux dev-5.7 2/6] spi: fsi: Fix clock running too fast
2020-07-29 20:45 ` [PATCH linux dev-5.7 2/6] spi: fsi: Fix clock running too fast Eddie James
@ 2020-07-29 23:25 ` Joel Stanley
2020-07-30 21:31 ` Eddie James
0 siblings, 1 reply; 15+ messages in thread
From: Joel Stanley @ 2020-07-29 23:25 UTC (permalink / raw)
To: Eddie James, Dean Sanner, Joachim Fenkes; +Cc: OpenBMC Maillist, Brad Bishop
On Wed, 29 Jul 2020 at 20:45, Eddie James <eajames@linux.ibm.com> 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.
This sounds like something we should fix, especially if we're
experimenting between 200/166/100 MHz FSI clocks?
>
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> Signed-off-by: Brad Bishop <bradleyb@fuzziesquirrel.com>
> ---
> 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.24.0
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH linux dev-5.7 3/6] spi: fsi: Fix use of the bneq+ sequencer instruction
2020-07-29 20:45 ` [PATCH linux dev-5.7 3/6] spi: fsi: Fix use of the bneq+ sequencer instruction Eddie James
@ 2020-07-29 23:27 ` Joel Stanley
0 siblings, 0 replies; 15+ messages in thread
From: Joel Stanley @ 2020-07-29 23:27 UTC (permalink / raw)
To: Eddie James; +Cc: OpenBMC Maillist, Brad Bishop
On Wed, 29 Jul 2020 at 20:45, Eddie James <eajames@linux.ibm.com> wrote:
>
> 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.
>
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> Signed-off-by: Brad Bishop <bradleyb@fuzziesquirrel.com>
Fixes: bbb6b2f9865b ("spi: Add FSI-attached SPI controller driver")
Reviewed-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.24.0
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH linux dev-5.7 4/6] dt-bindings: fsi: fsi2spi: Document new restricted property
2020-07-29 20:45 ` [PATCH linux dev-5.7 4/6] dt-bindings: fsi: fsi2spi: Document new restricted property Eddie James
@ 2020-07-29 23:27 ` Joel Stanley
0 siblings, 0 replies; 15+ messages in thread
From: Joel Stanley @ 2020-07-29 23:27 UTC (permalink / raw)
To: Eddie James; +Cc: OpenBMC Maillist
On Wed, 29 Jul 2020 at 20:45, Eddie James <eajames@linux.ibm.com> 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>
> ---
> 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 893d81e54caa..ff16797061ec 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.24.0
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH linux dev-5.7 6/6] eeprom: at25: Split reads into chunks and cap write size
2020-07-29 20:45 [PATCH linux dev-5.7 0/6] spi: Fix FSI-attached controller and AT25 drivers Eddie James
` (5 preceding siblings ...)
2020-07-29 20:45 ` [PATCH linux dev-5.7 6/6] eeprom: at25: Split reads into chunks and cap write size Eddie James
@ 2020-07-29 23:28 ` Milton Miller II
2020-07-29 23:35 ` [PATCH linux dev-5.7 0/6] spi: Fix FSI-attached controller and AT25 drivers Joel Stanley
7 siblings, 0 replies; 15+ messages in thread
From: Milton Miller II @ 2020-07-29 23:28 UTC (permalink / raw)
To: Eddie James; +Cc: openbmc, Brad Bishop
On July 20,2020 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.
>
>Signed-off-by: Eddie James <eajames@linux.ibm.com>
>Signed-off-by: Brad Bishop <bradleyb@fuzziesquirrel.com>
When forwarding patches from others your SOB goes after theirs.
>---> 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 cde9a2fc1325..3ed041cb3083 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 = count / max_chunk + (bool)(count % max_chunk);
Casting to bool to use result as 1 or 0 ? Obsfucated.
Use DIV_ROUND_UP instead.
>+ 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 msg_offset;
I expect checkpatch to complain
# check for declarations of signed or unsigned without int
>+ 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;
>- /* fall through */
>- case 2:
>- *cp++ = offset >> 8;
>- /* fall through */
>- case 1:
>- case 0: /* can't happen: for better codegen */
>- *cp++ = offset >> 0;
>- }
>+ msg_offset = (unsigned) offset;
>+ msg_count = min(count, max_chunk);
>+ while (num_msgs) {
>+ cp = command;
>
>- spi_message_init(&m);
>- memset(t, 0, sizeof(t));
>+ 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;
>
>- t[0].tx_buf = command;
>- t[0].len = at25->addrlen + 1;
>- spi_message_add_tail(&t[0], &m);
>+ /* 8/16/24-bit address is written MSB first */
>+ switch (at25->addrlen) {
>+ default: /* case 3 */
>+ *cp++ = msg_offset >> 16;
>+ /* fall through */
>+ case 2:
>+ *cp++ = msg_offset >> 8;
>+ /* fall through */
>+ case 1:
>+ case 0: /* can't happen: for better codegen */
>+ *cp++ = msg_offset >> 0;
>+ }
>
>- t[1].rx_buf = buf;
>- t[1].len = count;
>- spi_message_add_tail(&t[1], &m);
>+ spi_message_init(&m);
>+ memset(t, 0, sizeof(t));
>
>- mutex_lock(&at25->lock);
>+ t[0].tx_buf = command;
>+ t[0].len = at25->addrlen + 1;
>+ spi_message_add_tail(&t[0], &m);
>
>- /* 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);
>+ t[1].rx_buf = buf + nr_bytes;
>+ t[1].len = msg_count;
>+ spi_message_add_tail(&t[1], &m);
>
>- mutex_unlock(&at25->lock);
>- return status;
>+ mutex_lock(&at25->lock);
>+
>+ status = spi_sync(at25->spi, &m);
>+
>+ 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.24.0
>
>
Regards,
milton
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH linux dev-5.7 5/6] spi: fsi: Implement restricted size for certain controllers
2020-07-29 20:45 ` [PATCH linux dev-5.7 5/6] spi: fsi: Implement restricted size for certain controllers Eddie James
@ 2020-07-29 23:33 ` Joel Stanley
0 siblings, 0 replies; 15+ messages in thread
From: Joel Stanley @ 2020-07-29 23:33 UTC (permalink / raw)
To: Eddie James; +Cc: OpenBMC Maillist
On Wed, 29 Jul 2020 at 20:45, Eddie James <eajames@linux.ibm.com> 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.
>
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
Fixes: bbb6b2f9865b ("spi: Add FSI-attached SPI controller driver")
Reviewed-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.24.0
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH linux dev-5.7 0/6] spi: Fix FSI-attached controller and AT25 drivers
2020-07-29 20:45 [PATCH linux dev-5.7 0/6] spi: Fix FSI-attached controller and AT25 drivers Eddie James
` (6 preceding siblings ...)
2020-07-29 23:28 ` Milton Miller II
@ 2020-07-29 23:35 ` Joel Stanley
7 siblings, 0 replies; 15+ messages in thread
From: Joel Stanley @ 2020-07-29 23:35 UTC (permalink / raw)
To: Eddie James; +Cc: OpenBMC Maillist
Hi Eddie,
On Wed, 29 Jul 2020 at 20:45, Eddie James <eajames@linux.ibm.com> wrote:
>
> This series implements a number of fixes for the FSI-attached SPI controller
> driver and the AT25 eeprom driver.
Thanks for sending these out. I'll give you a chance to respond to the
comments Milton and I have made, and then merge them to the openbmc
tree.
Can you then send them upstream (keep my r-b and fixes tags)?
I'd be interested to see what the upstream maintainers say about the
at25 patch. I suspect we will have to fix that in our controller.
>
> 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 (2):
> dt-bindings: fsi: fsi2spi: Document new restricted property
> spi: fsi: Implement restricted size for certain controllers
>
> .../devicetree/bindings/fsi/ibm,fsi2spi.yaml | 10 ++
> drivers/misc/eeprom/at25.c | 94 ++++++++++--------
> drivers/spi/spi-fsi.c | 99 +++++++++++++++----
> 3 files changed, 145 insertions(+), 58 deletions(-)
>
> --
> 2.24.0
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH linux dev-5.7 1/6] spi: fsi: Handle 9 to 15 byte transfers lengths
2020-07-29 20:45 ` [PATCH linux dev-5.7 1/6] spi: fsi: Handle 9 to 15 byte transfers lengths Eddie James
@ 2020-07-29 23:41 ` Joel Stanley
0 siblings, 0 replies; 15+ messages in thread
From: Joel Stanley @ 2020-07-29 23:41 UTC (permalink / raw)
To: Eddie James; +Cc: OpenBMC Maillist, Brad Bishop
On Wed, 29 Jul 2020 at 20:45, Eddie James <eajames@linux.ibm.com> wrote:
>
> From: Brad Bishop <bradleyb@fuzziesquirrel.com>
>
> The trailing <len> - 8 bytes of transfer data in this size range is no
> longer ignored.
>
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> Signed-off-by: Brad Bishop <bradleyb@fuzziesquirrel.com>
Phew, that's subtle :)
Fixes: bbb6b2f9865b ("spi: Add FSI-attached SPI controller driver")
Reviewed-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.24.0
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH linux dev-5.7 2/6] spi: fsi: Fix clock running too fast
2020-07-29 23:25 ` Joel Stanley
@ 2020-07-30 21:31 ` Eddie James
0 siblings, 0 replies; 15+ messages in thread
From: Eddie James @ 2020-07-30 21:31 UTC (permalink / raw)
To: Joel Stanley, Dean Sanner, Joachim Fenkes; +Cc: OpenBMC Maillist, Brad Bishop
On 7/29/20 6:25 PM, Joel Stanley wrote:
> On Wed, 29 Jul 2020 at 20:45, Eddie James <eajames@linux.ibm.com> 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.
> This sounds like something we should fix, especially if we're
> experimenting between 200/166/100 MHz FSI clocks?
Yes, but I figured that can be done later. Since 200 is the fastest,
then using 19 will be safe until we can find the time to implement the
clock division properly.
Thanks,
Eddie
>
>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>> Signed-off-by: Brad Bishop <bradleyb@fuzziesquirrel.com>
>> ---
>> 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.24.0
>>
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2020-07-30 21:31 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-29 20:45 [PATCH linux dev-5.7 0/6] spi: Fix FSI-attached controller and AT25 drivers Eddie James
2020-07-29 20:45 ` [PATCH linux dev-5.7 1/6] spi: fsi: Handle 9 to 15 byte transfers lengths Eddie James
2020-07-29 23:41 ` Joel Stanley
2020-07-29 20:45 ` [PATCH linux dev-5.7 2/6] spi: fsi: Fix clock running too fast Eddie James
2020-07-29 23:25 ` Joel Stanley
2020-07-30 21:31 ` Eddie James
2020-07-29 20:45 ` [PATCH linux dev-5.7 3/6] spi: fsi: Fix use of the bneq+ sequencer instruction Eddie James
2020-07-29 23:27 ` Joel Stanley
2020-07-29 20:45 ` [PATCH linux dev-5.7 4/6] dt-bindings: fsi: fsi2spi: Document new restricted property Eddie James
2020-07-29 23:27 ` Joel Stanley
2020-07-29 20:45 ` [PATCH linux dev-5.7 5/6] spi: fsi: Implement restricted size for certain controllers Eddie James
2020-07-29 23:33 ` Joel Stanley
2020-07-29 20:45 ` [PATCH linux dev-5.7 6/6] eeprom: at25: Split reads into chunks and cap write size Eddie James
2020-07-29 23:28 ` Milton Miller II
2020-07-29 23:35 ` [PATCH linux dev-5.7 0/6] spi: Fix FSI-attached controller and AT25 drivers Joel Stanley
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.