Re: [PATCH v2 2/3] mtd: spi-nor: add SFDP fixups for Quad Page Program

From: <Tudor.Ambarus@microchip.com>
To: <sudip.mukherjee@sifive.com>, <pratyush@kernel.org>,
	<michael@walle.cc>, <miquel.raynal@bootlin.com>, <richard@nod.at>,
	<vigneshr@ti.com>
Cc: <greentime.hu@sifive.com>, <jude.onyenegecha@sifive.com>,
	<william.salmon@sifive.com>, <adnan.chowdhury@sifive.com>,
	<ben.dooks@sifive.com>, <linux-mtd@lists.infradead.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 2/3] mtd: spi-nor: add SFDP fixups for Quad Page Program
Date: Wed, 10 Aug 2022 08:06:21 +0000	[thread overview]
Message-ID: <361fa56d-617c-ee92-151e-5d8fe0a29e53@microchip.com> (raw)
In-Reply-To: <20220809201428.118523-3-sudip.mukherjee@sifive.com>

On 8/9/22 23:14, Sudip Mukherjee wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> SFDP table of some flash chips do not advertise support of Quad Input
> Page Program even though it has support. Use fixup flags and add hardware
> cap for these chips.
> 
> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
> ---
>  drivers/mtd/spi-nor/core.c | 9 +++++++++
>  drivers/mtd/spi-nor/core.h | 2 ++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index f2c64006f8d7..7542404332a5 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -1962,6 +1962,12 @@ spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps)
>         if (nor->flags & SNOR_F_BROKEN_RESET)
>                 *hwcaps &= ~(SNOR_HWCAPS_X_X_X | SNOR_HWCAPS_X_X_X_DTR);
> 
> +       if (nor->flags & SNOR_F_HAS_QUAD_PP) {
> +               *hwcaps |= SNOR_HWCAPS_PP_1_1_4;
> +               spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP_1_1_4],
> +                                       SPINOR_OP_PP_1_1_4, SNOR_PROTO_1_1_4);
> +       }

setting SPINOR_OP_PP_1_1_4 should be done in spi_nor_late_init_params().
spi_nor_late_init_params() is used to adjust the ops supported by the flash
with the ones supported by the controller.

> +
>         for (cap = 0; cap < sizeof(*hwcaps) * BITS_PER_BYTE; cap++) {
>                 int rdidx, ppidx;
> 
> @@ -2446,6 +2452,9 @@ static void spi_nor_init_fixup_flags(struct spi_nor *nor)
> 
>         if (fixup_flags & SPI_NOR_IO_MODE_EN_VOLATILE)
>                 nor->flags |= SNOR_F_IO_MODE_EN_VOLATILE;
> +
> +       if (fixup_flags & SPI_NOR_QUAD_PP)
> +               nor->flags |= SNOR_F_HAS_QUAD_PP;
>  }
> 
>  /**
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index 85b0cf254e97..7dbdf16a67b4 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -130,6 +130,7 @@ enum spi_nor_option_flags {
>         SNOR_F_IO_MODE_EN_VOLATILE = BIT(11),
>         SNOR_F_SOFT_RESET       = BIT(12),
>         SNOR_F_SWP_IS_VOLATILE  = BIT(13),
> +       SNOR_F_HAS_QUAD_PP      = BIT(14),

you won't need this
>  };
> 
>  struct spi_nor_read_command {
> @@ -520,6 +521,7 @@ struct flash_info {
>         u8 fixup_flags;
>  #define SPI_NOR_4B_OPCODES             BIT(0)
>  #define SPI_NOR_IO_MODE_EN_VOLATILE    BIT(1)
> +#define SPI_NOR_QUAD_PP                        BIT(2)

No, as I previously said, SPI_NOR_QUAD_PP should be declared as a
info->flags, not as info->fixup_flags.

> 
>         u8 mfr_flags;
> 
> --
> 2.30.2
> 

-- 
Cheers,
ta
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/