[14/16] spi: bcm63xx-hsspi: prepend: Disable spi mem dual io read op support

Message ID 20230106200809.330769-15-william.zhang@broadcom.com
State New
Headers
Series spi: bcm63xx-hsspi: driver and doc updates |

Commit Message

William Zhang Jan. 6, 2023, 8:08 p.m. UTC
  In general the controller supports SPI dual mode operation but the
particular SPI flash dual io read op switches from single mode in cmd
phase to dual mode in address and data phase. This is not compatible
with prepend operation where cmd and address are sent out through the
prepend buffer and they must use same the number of io pins.

This patch disables these SPI flash dual io read ops through the mem_ops
supports_op interface when prepend mode is used. This makes sure the SPI
flash driver selects the compatible read ops at run time.

Signed-off-by: William Zhang <william.zhang@broadcom.com>
---

 drivers/spi/spi-bcm63xx-hsspi.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
  

Comments

Mark Brown Jan. 6, 2023, 10:07 p.m. UTC | #1
On Fri, Jan 06, 2023 at 12:08:06PM -0800, William Zhang wrote:
> In general the controller supports SPI dual mode operation but the
> particular SPI flash dual io read op switches from single mode in cmd
> phase to dual mode in address and data phase. This is not compatible
> with prepend operation where cmd and address are sent out through the
> prepend buffer and they must use same the number of io pins.
> 
> This patch disables these SPI flash dual io read ops through the mem_ops
> supports_op interface when prepend mode is used. This makes sure the SPI
> flash driver selects the compatible read ops at run time.

This suggests that your prepend mode is attempting to combine
incompatible transfers doesn't it?  Presumably other devices could
generate similar access patterns...
  
William Zhang Jan. 7, 2023, 3:57 a.m. UTC | #2
On 01/06/2023 02:07 PM, Mark Brown wrote:
> On Fri, Jan 06, 2023 at 12:08:06PM -0800, William Zhang wrote:
>> In general the controller supports SPI dual mode operation but the
>> particular SPI flash dual io read op switches from single mode in cmd
>> phase to dual mode in address and data phase. This is not compatible
>> with prepend operation where cmd and address are sent out through the
>> prepend buffer and they must use same the number of io pins.
>>
>> This patch disables these SPI flash dual io read ops through the mem_ops
>> supports_op interface when prepend mode is used. This makes sure the SPI
>> flash driver selects the compatible read ops at run time.
> 
> This suggests that your prepend mode is attempting to combine
> incompatible transfers doesn't it?  Presumably other devices could
> generate similar access patterns...
> 
As far as I can see, only spi mem op has such pattern but agree it is 
possible that other device can generate such usage. I can add a check 
for this condition(all the prepend data must use same io width) so error 
can be printed and user can switch to dummy cs workaround.

Thank you for your feedbacks so far and have a nice weekend!
  

Patch

diff --git a/drivers/spi/spi-bcm63xx-hsspi.c b/drivers/spi/spi-bcm63xx-hsspi.c
index be4ca01f332a..7ede7f02ba2c 100644
--- a/drivers/spi/spi-bcm63xx-hsspi.c
+++ b/drivers/spi/spi-bcm63xx-hsspi.c
@@ -20,6 +20,7 @@ 
 #include <linux/spi/spi.h>
 #include <linux/mutex.h>
 #include <linux/of.h>
+#include <linux/spi/spi-mem.h>
 #include <linux/reset.h>
 #include <linux/pm_runtime.h>
 
@@ -596,6 +597,26 @@  static int bcm63xx_hsspi_transfer_one(struct spi_master *master,
 	return 0;
 }
 
+static bool bcm63xx_hsspi_mem_supports_op(struct spi_mem *mem,
+			    const struct spi_mem_op *op)
+{
+	struct bcm63xx_hsspi *bs = spi_master_get_devdata(mem->spi->master);
+
+	if (!spi_mem_default_supports_op(mem, op))
+		return false;
+
+	/* Controller doesn't support spi mem dual/quad read cmd in prepend mode */
+	if (!bs->use_cs_workaround &&
+		  ((op->cmd.opcode == 0xbb) || (op->cmd.opcode == 0xeb)))
+		return false;
+
+	return true;
+}
+
+static const struct spi_controller_mem_ops bcm63xx_hsspi_mem_ops = {
+	.supports_op = bcm63xx_hsspi_mem_supports_op,
+};
+
 static irqreturn_t bcm63xx_hsspi_interrupt(int irq, void *dev_id)
 {
 	struct bcm63xx_hsspi *bs = (struct bcm63xx_hsspi *)dev_id;
@@ -692,6 +713,7 @@  static int bcm63xx_hsspi_probe(struct platform_device *pdev)
 	mutex_init(&bs->bus_mutex);
 	init_completion(&bs->done);
 
+	master->mem_ops = &bcm63xx_hsspi_mem_ops;
 	master->dev.of_node = dev->of_node;
 	if (!dev->of_node) {
 		master->bus_num = HSSPI_BUS_NUM;