[PATCH 1/2] mtd: hyperbus: Introduce SFDP probe
Tudor Ambarus
tudor.ambarus at linaro.org
Sun Jan 21 21:47:46 PST 2024
Hi, Takahiro!
On 07.04.2023 09:11, tkuw584924 at gmail.com wrote:
> From: Takahiro Kuwano <Takahiro.Kuwano at infineon.com>
>
> Infineon(Cypress) S26HL-T/S26HS-T family of flash devices support both
> legacy SPI mode and HyperBus (JESD251C xSPI profile 2.0) mode. In
> HyperBus mode, it is compatible with Cypress S26KL-S/S26KS-S HyperFlash
> family with some exceptions. One significant difference is that the CFI
> is replaced by SFDP. This patch intends to support Hyperbus devices that
> has SFDP instead of CFI, by adding new probing method that detects SFDP
> signature and setting up CFI structure to make SFDP chips work with
> cfi_cmdset_xxxx.
>
By SFDP you mean the JESD216 SFDP standard, right? Would it make sense
to pull out the SFDP parsing from SPI NOR into its own dedicated entity
so that it's used by hyperbus as well?
> The probe flow implemented in hyperbus_sfdp_probe_chip() is same as
> cfi_probe_chip() in chips/cfi_probe.c. The CFI "QRY" detection is
> replaced by "SFDP" signature detection.
>
> Device identification and setup functions specific for S26Hx-T are also
> implemented in the same source file as SFDP probe and called from probe
> function. Once other devices that supports SFDP come up, we will implemet
> generic detection and setup flow.
What other hyperflash devices are out in the market and which of them
support SFDP? I saw ISSI has hyperflashes:
https://www.issi.com/us/product-flash.shtml
In SPI NOR we worked a lot to separate the core from the manufacturer
drivers. I'd make the split here since the beginning, the code will
become cleaner and easier to maintain.
>
> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano at infineon.com>
> ---
> drivers/mtd/hyperbus/Makefile | 4 +-
> drivers/mtd/hyperbus/hyperbus-core.c | 15 +-
> drivers/mtd/hyperbus/hyperbus-sfdp.c | 346 +++++++++++++++++++++++++++
> 3 files changed, 363 insertions(+), 2 deletions(-)
> create mode 100644 drivers/mtd/hyperbus/hyperbus-sfdp.c
>
> diff --git a/drivers/mtd/hyperbus/Makefile b/drivers/mtd/hyperbus/Makefile
> index 26386158bd1c..03132bc05853 100644
> --- a/drivers/mtd/hyperbus/Makefile
> +++ b/drivers/mtd/hyperbus/Makefile
> @@ -1,9 +1,11 @@
> # SPDX-License-Identifier: GPL-2.0
>
> -obj-$(CONFIG_MTD_HYPERBUS) += hyperbus-core.o
> +obj-$(CONFIG_MTD_HYPERBUS) += hyperbus.o
> obj-$(CONFIG_HBMC_AM654) += hbmc-am654.o
> obj-$(CONFIG_RPCIF_HYPERBUS) += rpc-if.o
>
> +hyperbus-objs := hyperbus-core.o hyperbus-sfdp.o
maybe rename hyperbus-core to core, as we're already in a hyperbus directory
> +
>
>
> obj-y += ifx-hyperbus.o
> diff --git a/drivers/mtd/hyperbus/hyperbus-core.c b/drivers/mtd/hyperbus/hyperbus-core.c
> index 4d8047d43e48..b31a8bea3995 100644
> --- a/drivers/mtd/hyperbus/hyperbus-core.c
> +++ b/drivers/mtd/hyperbus/hyperbus-core.c
> @@ -12,6 +12,19 @@
> #include <linux/of.h>
> #include <linux/types.h>
>
> +struct mtd_info *hyperbus_sfdp_probe(struct map_info *map);
> +
> +static struct mtd_info *hyperbus_map_probe(struct map_info *map)
> +{
> + struct mtd_info *mtd;
> +
> + mtd = do_map_probe("cfi_probe", map);
> + if (!mtd)
> + mtd = hyperbus_sfdp_probe(map);
> +
> + return mtd;
> +}
> +
> static struct hyperbus_device *map_to_hbdev(struct map_info *map)
> {
> return container_of(map, struct hyperbus_device, map);
> @@ -106,7 +119,7 @@ int hyperbus_register_device(struct hyperbus_device *hbdev)
> }
> }
>
> - hbdev->mtd = do_map_probe("cfi_probe", map);
> + hbdev->mtd = hyperbus_map_probe(map);
> if (!hbdev->mtd) {
> dev_err(dev, "probing of hyperbus device failed\n");
> return -ENODEV;
> diff --git a/drivers/mtd/hyperbus/hyperbus-sfdp.c b/drivers/mtd/hyperbus/hyperbus-sfdp.c
> new file mode 100644
> index 000000000000..f292e39856cb
> --- /dev/null
> +++ b/drivers/mtd/hyperbus/hyperbus-sfdp.c
> @@ -0,0 +1,346 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2023, Infineon Technologies.
> + */
> +#include <linux/mtd/xip.h>
> +#include <linux/mtd/gen_probe.h>
> +
> +#ifdef CONFIG_MTD_XIP
> +
> +/* only needed for short periods, so this is rather simple */
> +#define xip_disable() local_irq_disable()
> +
> +#define xip_allowed(base, map) \
> +do { \
> + (void) map_read(map, base); \
> + xip_iprefetch(); \
> + local_irq_enable(); \
> +} while (0)
> +
> +#define xip_enable(base, map, cfi) \
> +do { \
> + hyperbus_sfdp_mode_off(base, map, cfi); \
> + xip_allowed(base, map); \
> +} while (0)
> +
> +#else
> +
> +#define xip_disable() do { } while (0)
> +#define xip_allowed(base, map) do { } while (0)
> +#define xip_enable(base, map, cfi) do { } while (0)
> +> +#endif
Is the xip tested? You haven't specified anything about it in the commit
message. I would get rid of it if possible, and come with a dedicated
patch on top. The simpler a patch is, the more chances it gets to get
integrated.
> +
> +/* HyperBus Commands */
> +#define HYPERBUS_ADDR_UNLOCK1 0x555
> +#define HYPERBUS_ADDR_UNLOCK2 0x2AA
> +#define HYPERBUS_CMD_UNLOCK1 0xAA
> +#define HYPERBUS_CMD_UNLOCK2 0x55
> +#define HYPERBUS_CMD_RDSFDP 0x90
> +#define HYPERBUS_CMD_RDVCR1 0xC7
> +#define HYPERBUS_CMD_RDVCR2 0xC9
> +#define HYPERBUS_CMD_ASOEXT 0xF0
> +#define HYPERBUS_CMD_ERSCTR 0x30
these shall be moved to include/linux/mtd/hyperbus.h
> +
> +/* For Infineon S26Hx family */
> +#define S26HX_ADDR_MFR_ID 0x800
> +#define S26HX_ADDR_DEV_ID1 0x801
> +#define S26HX_ADDR_DEV_ID2 0x802
> +#define S26HX_DEV_ID1_3V0 0x006A
> +#define S26HX_DEV_ID1_1V8 0x007B
> +#define S26HX_DEV_ID2_512 0x001A
> +#define S26HX_DEV_ID2_01G 0x001B
> +#define S26HX_CFR1_UNIFORM BIT(9)
> +#define S26HX_CFR1_TP4KBS BIT(8)
> +#define S26HX_CFR2_SP4KBS BIT(2)
and these to thier own manufacturer file
> +#define ERASEINFO(s, b) (((s) << 8) | ((b) - 1))
can we use FIELD_GET/FIELD_SET for eraseinfo?
> +
> +static inline u16 hyperbus_read(u32 addr, struct map_info *map,
> + struct cfi_private *cfi)
> +{
> + return cfi_read_query16(map, addr * cfi->interleave * cfi->device_type);
> +}
> +
> +static inline void hyperbus_write(u8 cmd, u32 addr, struct map_info *map,
> + struct cfi_private *cfi)
here you ignore the return value of cfi_send_gen_cmd. Shall we change
the return type to u32?
> +{
> + cfi_send_gen_cmd(cmd, addr, 0, map, cfi, cfi->device_type, NULL);> +}
> +
> +static int __xipram hyperbus_sfdp_present(struct map_info *map, __u32 base,
> + struct cfi_private *cfi)
> +{
> + int osf = cfi->interleave * cfi->device_type; /* scale factor */
> + map_word val[2];
> + map_word sfdp[2];
> +
> + sfdp[0] = cfi_build_cmd(0x4653, map, cfi); /* "SF" */
> + sfdp[1] = cfi_build_cmd(0x5044, map, cfi); /* "DP" */
> +
> + val[0] = map_read(map, base + osf * 0x0);
> + val[1] = map_read(map, base + osf * 0x1);
can we read the entire SFDP in one shot?
> +
> + if (!map_word_equal(map, sfdp[0], val[0]))
> + return 0;
> +
> + if (!map_word_equal(map, sfdp[1], val[1]))
> + return 0;
> +
> + return 1; /* "SFDP" found */
return -EINVAL for error and zero for success
> +}
> +
> +static int __xipram hyperbus_sfdp_mode_on(u32 base, struct map_info *map,
> + struct cfi_private *cfi)
base is unused
> +{
> + hyperbus_write(HYPERBUS_CMD_ASOEXT, 0, map, cfi);
> + hyperbus_write(HYPERBUS_CMD_UNLOCK1, HYPERBUS_ADDR_UNLOCK1, map, cfi);
> + hyperbus_write(HYPERBUS_CMD_UNLOCK2, HYPERBUS_ADDR_UNLOCK2, map, cfi);
> + hyperbus_write(HYPERBUS_CMD_RDSFDP, HYPERBUS_ADDR_UNLOCK1, map, cfi);
> + return hyperbus_sfdp_present(map, 0, cfi);
> +}
> +
> +static void __xipram hyperbus_sfdp_mode_off(u32 base, struct map_info *map,
> + struct cfi_private *cfi)
base in unused
> +{
> + hyperbus_write(HYPERBUS_CMD_ASOEXT, 0, map, cfi);
> +}
> +
> +static u16 __xipram hyperbus_s26hx_rdvcr1(struct map_info *map,
> + struct cfi_private *cfi)
> +{
> + hyperbus_write(HYPERBUS_CMD_UNLOCK1, HYPERBUS_ADDR_UNLOCK1, map, cfi);
> + hyperbus_write(HYPERBUS_CMD_UNLOCK2, HYPERBUS_ADDR_UNLOCK2, map, cfi);
> + hyperbus_write(HYPERBUS_CMD_RDVCR1, HYPERBUS_ADDR_UNLOCK1, map, cfi);
> + return hyperbus_read(0, map, cfi);
> +}
> +
> +static u16 __xipram hyperbus_s26hx_rdvcr2(struct map_info *map,
> + struct cfi_private *cfi)
> +{
> + hyperbus_write(HYPERBUS_CMD_UNLOCK1, HYPERBUS_ADDR_UNLOCK1, map, cfi);
> + hyperbus_write(HYPERBUS_CMD_UNLOCK2, HYPERBUS_ADDR_UNLOCK2, map, cfi);
> + hyperbus_write(HYPERBUS_CMD_RDVCR2, HYPERBUS_ADDR_UNLOCK1, map, cfi);
> + return hyperbus_read(0, map, cfi);
> +}
> +
> +static int __xipram hyperbus_s26hx_chip_setup(struct map_info *map,
> + struct cfi_private *cfi)
> +{
> + u16 mfr_id, dev_id1, dev_id2, cfr1v, cfr2v;
> + u8 nregions, nbtm4ks, ntop4ks, n256ks;
> + u32 *erase;
> +
> + /* Read manufacturer and Device ID */
> + mfr_id = hyperbus_read(S26HX_ADDR_MFR_ID, map, cfi);
> + dev_id1 = hyperbus_read(S26HX_ADDR_DEV_ID1, map, cfi);
> + dev_id2 = hyperbus_read(S26HX_ADDR_DEV_ID2, map, cfi);
> +
> + if ((mfr_id != CFI_MFR_CYPRESS) ||
> + (dev_id1 != S26HX_DEV_ID1_3V0 && dev_id1 != S26HX_DEV_ID1_1V8) ||
> + (dev_id2 != S26HX_DEV_ID2_512 && dev_id2 != S26HX_DEV_ID2_01G)) {
> + xip_enable(0, map, cfi);
> + return 0;
> + }
> +
> + /* Exit SFDP ASO then read CFR1V and CFR2V */
> + hyperbus_sfdp_mode_off(0, map, cfi);
> + cfr1v = hyperbus_s26hx_rdvcr1(map, cfi);
> + cfr2v = hyperbus_s26hx_rdvcr2(map, cfi);
> +
> + xip_enable(0, map, cfi);
> +
> + /*
> + * Determine number of regions (nregions), number of bottom 4KB sectors
> + * (nbtm4ks), and number of top 4KB sectors (ntop4ks)
> + */
> + if (cfr1v & S26HX_CFR1_UNIFORM) {
> + nregions = 1;
> + nbtm4ks = 0;
> + ntop4ks = 0;
> + } else if (cfr2v & S26HX_CFR2_SP4KBS) {
> + nregions = 5;
> + nbtm4ks = 16;
> + ntop4ks = 16;
> + } else if (cfr1v & S26HX_CFR1_TP4KBS) {
> + nregions = 3;
> + nbtm4ks = 0;
> + ntop4ks = 32;
> + } else {
> + nregions = 3;
> + nbtm4ks = 32;
> + ntop4ks = 0;
> + }
> +
> + cfi->cfiq = kmalloc(sizeof(struct cfi_ident) + nregions * sizeof(u32),
> + GFP_KERNEL);
> + if (!cfi->cfiq)
> + return 0;
> +
> + memset(cfi->cfiq, 0, sizeof(struct cfi_ident));
> +
> + cfi->mfr = mfr_id;
> + cfi->id = dev_id1 << 8 | dev_id2;
> + cfi->cfi_mode = CFI_MODE_CFI;
> + cfi->addr_unlock1 = HYPERBUS_ADDR_UNLOCK1;
> + cfi->addr_unlock2 = HYPERBUS_ADDR_UNLOCK2;
> + cfi->sector_erase_cmd = CMD(HYPERBUS_CMD_ERSCTR);
> +
> + cfi->cfiq->P_ID = P_ID_AMD_STD;
> + cfi->cfiq->DevSize = dev_id2; /* dev_id2 indicates size in 2^N */
> + cfi->cfiq->MaxBufWriteSize = 9; /* 2^9 = 512 */
> +
> + /*
> + * cfi_cmdset_0002 uses fixed timeout for word write (1ms + 1 jiffies)
> + * and sector erase (20s) that should be enough for s26hx. For buffer
> + * write, 2000us is used if cfiq->BufWriteTimeoutXXX is 0. The typical
> + * and maximum 512B page programming times are 680us and 2175us
> + * respectively. Specify proper values here to extend buffer write
> + * timeout.
> + */
> + cfi->cfiq->BufWriteTimeoutTyp = 10; /* 2^10 = 1024 */
> + cfi->cfiq->BufWriteTimeoutMax = 2; /* 1024 x 2^2 = 4096 */
> +
> + /* Setup erase region info */
> + cfi->cfiq->NumEraseRegions = nregions;
> + erase = cfi->cfiq->EraseRegionInfo;
> +
> + /* Bottom 4KB sectors and remaining portion */
> + if (nbtm4ks) {
> + *erase++ = ERASEINFO(SZ_4K, nbtm4ks);
> + *erase++ = ERASEINFO(SZ_256K - (SZ_4K * nbtm4ks), 1);
> + }
> +
> + /*
> + * The number of uniform 256KB sectors is obtained by dividing 'device
> + * size' by 256K(=2^18). Deduct overlaid sector(s) from uniform number
> + * if top and/or bottom 4KB sectors exist.
> + */
> + n256ks = (1 << (cfi->cfiq->DevSize - 18)) - !!(nbtm4ks) - !!(ntop4ks);
> + *erase++ = ERASEINFO(SZ_256K, n256ks);
> +
> + /* Top 4KB sectors and remaining portion */
> + if (ntop4ks) {
> + *erase++ = ERASEINFO(SZ_256K - (SZ_4K * ntop4ks), 1);
> + *erase = ERASEINFO(SZ_4K, ntop4ks);
> + }
> +
> + return 1;
please return 0 for success, -errno in case of others. Is this method
entirely IFX specific? Let's move it in its own cypress/ifx manufacturer
file.
> +}
> +
> +static int __xipram hyperbus_sfdp_chip_setup(struct map_info *map,
> + struct cfi_private *cfi)
> +{
> + /*
> + * Just call setup for S26Hx since it is only chip supported.
> + * Once other devices come up, we will implement vendor/chip specific
> + * detection and setup flow here.
> + */
> + return hyperbus_s26hx_chip_setup(map, cfi);
> +}
> +
> +static int __xipram hyperbus_sfdp_probe_chip(struct map_info *map, __u32 base,
> + unsigned long *chip_map,
> + struct cfi_private *cfi)
> +{
> + u32 probe_addr;
> + int i;
> +
> + if (base >= map->size) {
> + pr_notice("Probe at base(0x%08x) past the end of the map(0x%08lx)\n",
> + base, map->size - 1);
> + return 0;
> + }
> +
> + probe_addr = base + cfi_build_cmd_addr(HYPERBUS_ADDR_UNLOCK1, map, cfi);
> + if (probe_addr >= map->size) {
> + pr_notice("Probe at base[unlock](0x%08x) past the end of the map(0x%08lx)\n",
> + probe_addr, map->size - 1);
> + return 0;
> + }
> +
> + xip_disable();
> + if (!hyperbus_sfdp_mode_on(base, map, cfi)) {
> + xip_enable(base, map, cfi);
> + return 0;
> + }
> +
> + /*
> + * This is the first time we're called. Set up the CFI stuff accordingly
> + * and return
> + */
> + if (!cfi->numchips)
> + return hyperbus_sfdp_chip_setup(map, cfi);
Ah, I need to study more. Would you please speed me up a little and tell
me more about the multi chip handling? Why is the probe called multiple
times?
Thanks!
ta
More information about the linux-mtd
mailing list