[v10,08/17] mtd: spi-nor: core: use dummy cycle and address width info from SFDP
diff mbox series

Message ID 20200623183030.26591-9-p.yadav@ti.com
State New
Headers show
Series
  • mtd: spi-nor: add xSPI Octal DTR support
Related show

Commit Message

Pratyush Yadav June 23, 2020, 6:30 p.m. UTC
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 <p.yadav@ti.com>
---
 drivers/mtd/spi-nor/core.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

Comments

Tudor Ambarus July 8, 2020, 4:03 p.m. UTC | #1
On 6/23/20 9:30 PM, Pratyush Yadav wrote:
> 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 <p.yadav@ti.com>
> ---
>  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

> 
>         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?

> +                       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?

> 
>         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);
> --
> 2.27.0
>
Pratyush Yadav July 20, 2020, 4:24 p.m. UTC | #2
Hi Tudor,

On 08/07/20 04:03PM, Tudor.Ambarus@microchip.com wrote:
> On 6/23/20 9:30 PM, Pratyush Yadav wrote:
> > 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 <p.yadav@ti.com>
> > ---
> >  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.
 
> > 
> >         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.

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

Patch
diff mbox series

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;
 
 	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;
+			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);
 	}
 
 	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);