[PATCH v2 1/2] mtd: spi-nor: Respect flash's hwcaps in spi_nor_adjust_hwcaps()

[PATCH v2 1/2] mtd: spi-nor: Respect flash's hwcaps in spi_nor_adjust_hwcaps()

[Bin Meng]

The smart spi_nor_adjust_hwcaps() does not respect the SPI flash's
hwcaps, and only looks to the controller on what can be supported.

The flash's hwcaps needs to be AND'ed before checking.

Fixes: 71025f013ccb ("mtd: spi-nor-core: Rework hwcaps selection")
Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Pratyush Yadav <p.yadav@ti.com>

---

Changes in v2:
- Update comments per Pratyush's review

 drivers/mtd/spi/spi-nor-core.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index 99e2f16349..c8c3bdd890 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -2858,10 +2858,11 @@ spi_nor_adjust_hwcaps(struct spi_nor *nor,
 	unsigned int cap;
 
 	/*
-	 * Enable all caps by default. We will mask them after checking what's
-	 * really supported using spi_mem_supports_op().
+	 * Start by assuming the controller supports every capability.
+	 * We will mask them after checking what's really supported
+	 * using spi_mem_supports_op().
 	 */
-	*hwcaps = SNOR_HWCAPS_ALL;
+	*hwcaps = SNOR_HWCAPS_ALL & params->hwcaps.mask;
 
 	/* X-X-X modes are not supported yet, mask them all. */
 	*hwcaps &= ~SNOR_HWCAPS_X_X_X;
-- 
2.25.1

[PATCH v2 2/2] mtd: spi-nor: Mask out fast read if not requested in DT

[Bin Meng]

The DT bindings of "jedec,spi-nor" [1] defines "m25p,fast-read" property
to indicate that "fast read" opcode can be used to read data from the
chip instead of the usual "read" opcode.

If this property is not present in DT, mask out fast read in
spi_nor_init_params(). This change mirrors the same logic in
spi_nor_info_init_params() in drivers/mtd/spi-nor/core.c in
the Linux kernel v5.14-rc3.

[1] Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml in the kernel tree

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

---

Changes in v2:
- Guard changes with CONFIG_IS_ENABLED(DM_SPI)

 drivers/mtd/spi/spi-nor-core.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index c8c3bdd890..d5d905fa5a 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -2604,18 +2604,28 @@ static int spi_nor_init_params(struct spi_nor *nor,
 	params->size = info->sector_size * info->n_sectors;
 	params->page_size = info->page_size;
 
+	if (!(info->flags & SPI_NOR_NO_FR)) {
+		/* Default to Fast Read for DT and non-DT platform devices. */
+		params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
+
+		/* Mask out Fast Read if not requested at DT instantiation. */
+#if CONFIG_IS_ENABLED(DM_SPI)
+		if (!ofnode_read_bool(dev_ofnode(nor->spi->dev),
+				      "m25p,fast-read"))
+			params->hwcaps.mask &= ~SNOR_HWCAPS_READ_FAST;
+#endif
+	}
+
 	/* (Fast) Read settings. */
 	params->hwcaps.mask |= SNOR_HWCAPS_READ;
 	spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ],
 				  0, 0, SPINOR_OP_READ,
 				  SNOR_PROTO_1_1_1);
 
-	if (!(info->flags & SPI_NOR_NO_FR)) {
-		params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
+	if (params->hwcaps.mask & SNOR_HWCAPS_READ_FAST)
 		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_FAST],
 					  0, 8, SPINOR_OP_READ_FAST,
 					  SNOR_PROTO_1_1_1);
-	}
 
 	if (info->flags & SPI_NOR_DUAL_READ) {
 		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_2;
-- 
2.25.1

Re: [PATCH v2 2/2] mtd: spi-nor: Mask out fast read if not requested in DT

[Pratyush Yadav]

On 30/07/21 03:20PM, Bin Meng wrote:
> The DT bindings of "jedec,spi-nor" [1] defines "m25p,fast-read" property
> to indicate that "fast read" opcode can be used to read data from the
> chip instead of the usual "read" opcode.
> 
> If this property is not present in DT, mask out fast read in
> spi_nor_init_params(). This change mirrors the same logic in
> spi_nor_info_init_params() in drivers/mtd/spi-nor/core.c in
> the Linux kernel v5.14-rc3.
> 
> [1] Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml in the kernel tree
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> 
> ---
> 
> Changes in v2:
> - Guard changes with CONFIG_IS_ENABLED(DM_SPI)

Hehe, I just commented about this on your v1, and I see the v2 in my 
inbox already ;-)

So this will make sure that dev_ofnode() will be valid, right?

> 
>  drivers/mtd/spi/spi-nor-core.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index c8c3bdd890..d5d905fa5a 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -2604,18 +2604,28 @@ static int spi_nor_init_params(struct spi_nor *nor,
>  	params->size = info->sector_size * info->n_sectors;
>  	params->page_size = info->page_size;
>  
> +	if (!(info->flags & SPI_NOR_NO_FR)) {
> +		/* Default to Fast Read for DT and non-DT platform devices. */
> +		params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
> +
> +		/* Mask out Fast Read if not requested at DT instantiation. */
> +#if CONFIG_IS_ENABLED(DM_SPI)
> +		if (!ofnode_read_bool(dev_ofnode(nor->spi->dev),

Let's avoid the #if here and do

  if (CONFIG_IS_ENABLED(DM_SPI) && !ofnode_read_bool())

> +				      "m25p,fast-read"))
> +			params->hwcaps.mask &= ~SNOR_HWCAPS_READ_FAST;
> +#endif
> +	}
> +
>  	/* (Fast) Read settings. */
>  	params->hwcaps.mask |= SNOR_HWCAPS_READ;
>  	spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ],
>  				  0, 0, SPINOR_OP_READ,
>  				  SNOR_PROTO_1_1_1);
>  
> -	if (!(info->flags & SPI_NOR_NO_FR)) {
> -		params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
> +	if (params->hwcaps.mask & SNOR_HWCAPS_READ_FAST)
>  		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_FAST],
>  					  0, 8, SPINOR_OP_READ_FAST,
>  					  SNOR_PROTO_1_1_1);
> -	}
>  
>  	if (info->flags & SPI_NOR_DUAL_READ) {
>  		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_2;

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

Re: [PATCH v2 2/2] mtd: spi-nor: Mask out fast read if not requested in DT

[Bin Meng]

Hi Pratyush,

On Fri, Jul 30, 2021 at 3:31 PM Pratyush Yadav <p.yadav@ti.com> wrote:
>
> On 30/07/21 03:20PM, Bin Meng wrote:
> > The DT bindings of "jedec,spi-nor" [1] defines "m25p,fast-read" property
> > to indicate that "fast read" opcode can be used to read data from the
> > chip instead of the usual "read" opcode.
> >
> > If this property is not present in DT, mask out fast read in
> > spi_nor_init_params(). This change mirrors the same logic in
> > spi_nor_info_init_params() in drivers/mtd/spi-nor/core.c in
> > the Linux kernel v5.14-rc3.
> >
> > [1] Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml in the kernel tree
> >
> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> >
> > ---
> >
> > Changes in v2:
> > - Guard changes with CONFIG_IS_ENABLED(DM_SPI)
>
> Hehe, I just commented about this on your v1, and I see the v2 in my
> inbox already ;-)
>
> So this will make sure that dev_ofnode() will be valid, right?

This is to avoid build error for non-DM_SPI platforms.

For dev_ofnode() I think it's always valid, as spi_nor_init_params()
is called by spi_nor_scan() in sf_probe.c which does not support
OF_PLATDATA.

>
> >
> >  drivers/mtd/spi/spi-nor-core.c | 16 +++++++++++++---
> >  1 file changed, 13 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> > index c8c3bdd890..d5d905fa5a 100644
> > --- a/drivers/mtd/spi/spi-nor-core.c
> > +++ b/drivers/mtd/spi/spi-nor-core.c
> > @@ -2604,18 +2604,28 @@ static int spi_nor_init_params(struct spi_nor *nor,
> >       params->size = info->sector_size * info->n_sectors;
> >       params->page_size = info->page_size;
> >
> > +     if (!(info->flags & SPI_NOR_NO_FR)) {
> > +             /* Default to Fast Read for DT and non-DT platform devices. */
> > +             params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
> > +
> > +             /* Mask out Fast Read if not requested at DT instantiation. */
> > +#if CONFIG_IS_ENABLED(DM_SPI)
> > +             if (!ofnode_read_bool(dev_ofnode(nor->spi->dev),
>
> Let's avoid the #if here and do
>
>   if (CONFIG_IS_ENABLED(DM_SPI) && !ofnode_read_bool())

This does not work because nor->spi->dev has to be inside
CONFIG_IS_ENABLED(DM_SPI) as well.

>
> > +                                   "m25p,fast-read"))
> > +                     params->hwcaps.mask &= ~SNOR_HWCAPS_READ_FAST;
> > +#endif
> > +     }
> > +
> >       /* (Fast) Read settings. */
> >       params->hwcaps.mask |= SNOR_HWCAPS_READ;
> >       spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ],
> >                                 0, 0, SPINOR_OP_READ,
> >                                 SNOR_PROTO_1_1_1);
> >
> > -     if (!(info->flags & SPI_NOR_NO_FR)) {
> > -             params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
> > +     if (params->hwcaps.mask & SNOR_HWCAPS_READ_FAST)
> >               spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_FAST],
> >                                         0, 8, SPINOR_OP_READ_FAST,
> >                                         SNOR_PROTO_1_1_1);
> > -     }
> >
> >       if (info->flags & SPI_NOR_DUAL_READ) {
> >               params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_2;
>

Regards,
Bin

Re: [PATCH v2 2/2] mtd: spi-nor: Mask out fast read if not requested in DT

[Jagan Teki]

On Fri, Jul 30, 2021 at 12:50 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> The DT bindings of "jedec,spi-nor" [1] defines "m25p,fast-read" property
> to indicate that "fast read" opcode can be used to read data from the
> chip instead of the usual "read" opcode.
>
> If this property is not present in DT, mask out fast read in
> spi_nor_init_params(). This change mirrors the same logic in
> spi_nor_info_init_params() in drivers/mtd/spi-nor/core.c in
> the Linux kernel v5.14-rc3.
>
> [1] Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml in the kernel tree
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

Applied to u-boot-spi/master

Re: [PATCH v2 1/2] mtd: spi-nor: Respect flash's hwcaps in spi_nor_adjust_hwcaps()

[Jagan Teki]

On Fri, Jul 30, 2021 at 12:50 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> The smart spi_nor_adjust_hwcaps() does not respect the SPI flash's
> hwcaps, and only looks to the controller on what can be supported.
>
> The flash's hwcaps needs to be AND'ed before checking.
>
> Fixes: 71025f013ccb ("mtd: spi-nor-core: Rework hwcaps selection")
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
>
> ---

Applied to u-boot-spi/master