Hi Tudor,

On 08/07/20 04:03PM, Tudor.Ambarus@microchip.com wrote:

On 6/23/20 9:30 PM, Pratyush Yadav wrote:

quoted

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

The xSPI Profile 1.0 table specifies how many dummy cycles and address
bytes are needed for the Read Status Register command in octal DTR mode.
Use that information to send the correct Read SR command.

Signed-off-by: Pratyush Yadav <redacted>
---
 drivers/mtd/spi-nor/core.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 7d24e63fcca8..f2748f1d9957 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c

@@ -357,6 +357,8 @@ int spi_nor_write_disable(struct spi_nor *nor)
 static int spi_nor_read_sr(struct spi_nor *nor, u8 *sr)
 {
        int ret;
+       u8 addr_bytes = nor->params->rdsr_addr_nbytes;
+       u8 dummy = nor->params->rdsr_dummy;

no need to introduce local variables for a single dereference

Ok.
 

quoted

        if (nor->spimem) {
                struct spi_mem_op op =

@@ -365,10 +367,21 @@ static int spi_nor_read_sr(struct spi_nor *nor, u8 *sr)
                                   SPI_MEM_OP_NO_DUMMY,
                                   SPI_MEM_OP_DATA_IN(1, sr, 1));

+               if (spi_nor_protocol_is_dtr(nor->reg_proto)) {
+                       op.addr.nbytes = addr_bytes;
+                       op.addr.val = 0;

isn't addr already initialized to 0?

Yes, it is. But I figured it won't hurt to be explicit about what we 
intend the address to be.

quoted

+                       op.dummy.nbytes = dummy;
+               }
+
+               spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
                ret = spi_mem_exec_op(nor->spimem, &op);
        } else {
-               ret = nor->controller_ops->read_reg(nor, SPINOR_OP_RDSR,
-                                                   sr, 1);
+               if (spi_nor_protocol_is_dtr(nor->reg_proto))
+                       ret = -ENOTSUPP;
+               else
+                       ret = nor->controller_ops->read_reg(nor, SPINOR_OP_RDSR,
+                                                           sr, 1);
        }

doesn't this belong to a previous patch?

It does. Will fix.
 

quoted

        if (ret)

@@ -388,6 +401,8 @@ static int spi_nor_read_sr(struct spi_nor *nor, u8 *sr)
 static int spi_nor_read_fsr(struct spi_nor *nor, u8 *fsr)
 {
        int ret;
+       u8 addr_bytes = nor->params->rdsr_addr_nbytes;
+       u8 dummy = nor->params->rdsr_dummy;

        if (nor->spimem) {
                struct spi_mem_op op =

@@ -396,6 +411,12 @@ static int spi_nor_read_fsr(struct spi_nor *nor, u8 *fsr)
                                   SPI_MEM_OP_NO_DUMMY,
                                   SPI_MEM_OP_DATA_IN(1, fsr, 1));

+               if (spi_nor_protocol_is_dtr(nor->reg_proto)) {
+                       op.addr.nbytes = addr_bytes;
+                       op.addr.val = 0;
+                       op.dummy.nbytes = dummy;
+               }
+
                spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);

                ret = spi_mem_exec_op(nor->spimem, &op);

-- 
Regards,
Pratyush Yadav

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel