Message ID | 20190625075746.10439-1-vigneshr@ti.com |
---|---|
Headers | show |
Series | MTD: Add Initial Hyperbus support | expand |
Hi, Thanks for the fix. Reviewed-by: Tokunori Ikegami <ikegami.t@gmail.com> I have just tested the patch quickly on my local environment that uses the cfi_cmdset_0002 flash device but not HyperFlash family. So tested as not affected by the change. On 2019/06/25 16:57, Vignesh Raghavendra wrote: > HyperFlash devices are compliant with CFI AMD/Fujitsu Extended Command > Set (0x0002) for flash operations, therefore > drivers/mtd/chips/cfi_cmdset_0002.c can be used as is. But these devices > do not support DQ polling method of determining chip ready/good status. > These flashes provide Status Register whose bits can be polled to know > status of flash operation. > > Cypress HyperFlash datasheet here[1], talks about CFI Amd/Fujitsu > Extended Query version 1.5. Bit 0 of "Software Features supported" field > of CFI Primary Vendor-Specific Extended Query table indicates > presence/absence of status register and Bit 1 indicates whether or not > DQ polling is supported. Using these bits, its possible to determine > whether flash supports DQ polling or need to use Status Register. > > Add support for polling Status Register to know device ready/status of > erase/write operations when DQ polling is not supported. > Print error messages on erase/program failure by looking at related > Status Register bits. > > [1] https://www.cypress.com/file/213346/download > > Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> > --- > v8: > Fix up status register polling to support banked flashes in patch 1/5. > > > drivers/mtd/chips/cfi_cmdset_0002.c | 130 ++++++++++++++++++++++++---- > include/linux/mtd/cfi.h | 7 ++ > 2 files changed, 120 insertions(+), 17 deletions(-) > > diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c > index c8fa5906bdf9..09f8aaf1e763 100644 > --- a/drivers/mtd/chips/cfi_cmdset_0002.c > +++ b/drivers/mtd/chips/cfi_cmdset_0002.c > @@ -49,6 +49,16 @@ > #define SST49LF008A 0x005a > #define AT49BV6416 0x00d6 > > +/* > + * Status Register bit description. Used by flash devices that don't > + * support DQ polling (e.g. HyperFlash) > + */ > +#define CFI_SR_DRB BIT(7) > +#define CFI_SR_ESB BIT(5) > +#define CFI_SR_PSB BIT(4) > +#define CFI_SR_WBASB BIT(3) > +#define CFI_SR_SLSB BIT(1) > + > static int cfi_amdstd_read (struct mtd_info *, loff_t, size_t, size_t *, u_char *); > static int cfi_amdstd_write_words(struct mtd_info *, loff_t, size_t, size_t *, const u_char *); > static int cfi_amdstd_write_buffers(struct mtd_info *, loff_t, size_t, size_t *, const u_char *); > @@ -97,6 +107,50 @@ static struct mtd_chip_driver cfi_amdstd_chipdrv = { > .module = THIS_MODULE > }; > > +/* > + * Use status register to poll for Erase/write completion when DQ is not > + * supported. This is indicated by Bit[1:0] of SoftwareFeatures field in > + * CFI Primary Vendor-Specific Extended Query table 1.5 > + */ > +static int cfi_use_status_reg(struct cfi_private *cfi) > +{ > + struct cfi_pri_amdstd *extp = cfi->cmdset_priv; > + u8 poll_mask = CFI_POLL_STATUS_REG | CFI_POLL_DQ; > + > + return extp->MinorVersion >= '5' && > + (extp->SoftwareFeatures & poll_mask) == CFI_POLL_STATUS_REG; > +} > + > +static void cfi_check_err_status(struct map_info *map, struct flchip *chip, Just a minor comment. I though that it is better to be called this function in chip_good() instead of to be called separately. But I can understand that the chip_good() is called repeatedly in do_erase_chip(), do_erase_oneblock() and do_write_buffer() until timeout. Also do_write_oneword() is also possible to be changed to call the chip_good() repeatedly instead of chip_ready(). So additional logics is needed to avoid the repeated error messages in the function above. Anyway it is okay for the current implementation to me. Regards, Ikegami > + unsigned long adr) > +{ > + struct cfi_private *cfi = map->fldrv_priv; > + map_word status; > + > + if (!cfi_use_status_reg(cfi)) > + return; > + > + cfi_send_gen_cmd(0x70, cfi->addr_unlock1, chip->start, map, cfi, > + cfi->device_type, NULL); > + status = map_read(map, adr); > + > + if (map_word_bitsset(map, status, CMD(0x3a))) { > + unsigned long chipstatus = MERGESTATUS(status); > + > + if (chipstatus & CFI_SR_ESB) > + pr_err("%s erase operation failed, status %lx\n", > + map->name, chipstatus); > + if (chipstatus & CFI_SR_PSB) > + pr_err("%s program operation failed, status %lx\n", > + map->name, chipstatus); > + if (chipstatus & CFI_SR_WBASB) > + pr_err("%s buffer program command aborted, status %lx\n", > + map->name, chipstatus); > + if (chipstatus & CFI_SR_SLSB) > + pr_err("%s sector write protected, status %lx\n", > + map->name, chipstatus); > + } > +} > > /* #define DEBUG_CFI_FEATURES */ > > @@ -742,10 +796,25 @@ static struct mtd_info *cfi_amdstd_setup(struct mtd_info *mtd) > * correctly and is therefore not done (particularly with interleaved chips > * as each chip must be checked independently of the others). > */ > -static int __xipram chip_ready(struct map_info *map, unsigned long addr) > +static int __xipram chip_ready(struct map_info *map, struct flchip *chip, > + unsigned long addr) > { > + struct cfi_private *cfi = map->fldrv_priv; > map_word d, t; > > + if (cfi_use_status_reg(cfi)) { > + map_word ready = CMD(CFI_SR_DRB); > + /* > + * For chips that support status register, check device > + * ready bit > + */ > + cfi_send_gen_cmd(0x70, cfi->addr_unlock1, chip->start, map, cfi, > + cfi->device_type, NULL); > + d = map_read(map, addr); > + > + return map_word_andequal(map, d, ready, ready); > + } > + > d = map_read(map, addr); > t = map_read(map, addr); > > @@ -767,10 +836,30 @@ static int __xipram chip_ready(struct map_info *map, unsigned long addr) > * as each chip must be checked independently of the others). > * > */ > -static int __xipram chip_good(struct map_info *map, unsigned long addr, map_word expected) > +static int __xipram chip_good(struct map_info *map, struct flchip *chip, > + unsigned long addr, map_word expected) > { > + struct cfi_private *cfi = map->fldrv_priv; > map_word oldd, curd; > > + if (cfi_use_status_reg(cfi)) { > + map_word ready = CMD(CFI_SR_DRB); > + map_word err = CMD(CFI_SR_PSB | CFI_SR_ESB); > + /* > + * For chips that support status register, check device > + * ready bit and Erase/Program status bit to know if > + * operation succeeded. > + */ > + cfi_send_gen_cmd(0x70, cfi->addr_unlock1, chip->start, map, cfi, > + cfi->device_type, NULL); > + curd = map_read(map, addr); > + > + if (map_word_andequal(map, curd, ready, ready)) > + return !map_word_bitsset(map, curd, err); > + > + return 0; > + } > + > oldd = map_read(map, addr); > curd = map_read(map, addr); > > @@ -792,7 +881,7 @@ static int get_chip(struct map_info *map, struct flchip *chip, unsigned long adr > > case FL_STATUS: > for (;;) { > - if (chip_ready(map, adr)) > + if (chip_ready(map, chip, adr)) > break; > > if (time_after(jiffies, timeo)) { > @@ -830,7 +919,7 @@ static int get_chip(struct map_info *map, struct flchip *chip, unsigned long adr > chip->state = FL_ERASE_SUSPENDING; > chip->erase_suspended = 1; > for (;;) { > - if (chip_ready(map, adr)) > + if (chip_ready(map, chip, adr)) > break; > > if (time_after(jiffies, timeo)) { > @@ -1362,7 +1451,7 @@ static int do_otp_lock(struct map_info *map, struct flchip *chip, loff_t adr, > /* wait for chip to become ready */ > timeo = jiffies + msecs_to_jiffies(2); > for (;;) { > - if (chip_ready(map, adr)) > + if (chip_ready(map, chip, adr)) > break; > > if (time_after(jiffies, timeo)) { > @@ -1628,22 +1717,24 @@ static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip, > continue; > } > > - if (time_after(jiffies, timeo) && !chip_ready(map, adr)){ > + if (time_after(jiffies, timeo) && > + !chip_ready(map, chip, adr)) { > xip_enable(map, chip, adr); > printk(KERN_WARNING "MTD %s(): software timeout\n", __func__); > xip_disable(map, chip, adr); > break; > } > > - if (chip_ready(map, adr)) > + if (chip_ready(map, chip, adr)) > break; > > /* Latency issues. Drop the lock, wait a while and retry */ > UDELAY(map, chip, adr, 1); > } > /* Did we succeed? */ > - if (!chip_good(map, adr, datum)) { > + if (!chip_good(map, chip, adr, datum)) { > /* reset on all failures. */ > + cfi_check_err_status(map, chip, adr); > map_write(map, CMD(0xF0), chip->start); > /* FIXME - should have reset delay before continuing */ > > @@ -1881,10 +1972,11 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip, > * We check "time_after" and "!chip_good" before checking "chip_good" to avoid > * the failure due to scheduling. > */ > - if (time_after(jiffies, timeo) && !chip_good(map, adr, datum)) > + if (time_after(jiffies, timeo) && > + !chip_good(map, chip, adr, datum)) > break; > > - if (chip_good(map, adr, datum)) { > + if (chip_good(map, chip, adr, datum)) { > xip_enable(map, chip, adr); > goto op_done; > } > @@ -1901,6 +1993,7 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip, > * See e.g. > * http://www.spansion.com/Support/Application%20Notes/MirrorBit_Write_Buffer_Prog_Page_Buffer_Read_AN.pdf > */ > + cfi_check_err_status(map, chip, adr); > cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi, > cfi->device_type, NULL); > cfi_send_gen_cmd(0x55, cfi->addr_unlock2, chip->start, map, cfi, > @@ -2018,7 +2111,7 @@ static int cfi_amdstd_panic_wait(struct map_info *map, struct flchip *chip, > * If the driver thinks the chip is idle, and no toggle bits > * are changing, then the chip is actually idle for sure. > */ > - if (chip->state == FL_READY && chip_ready(map, adr)) > + if (chip->state == FL_READY && chip_ready(map, chip, adr)) > return 0; > > /* > @@ -2035,7 +2128,7 @@ static int cfi_amdstd_panic_wait(struct map_info *map, struct flchip *chip, > > /* wait for the chip to become ready */ > for (i = 0; i < jiffies_to_usecs(timeo); i++) { > - if (chip_ready(map, adr)) > + if (chip_ready(map, chip, adr)) > return 0; > > udelay(1); > @@ -2099,14 +2192,15 @@ static int do_panic_write_oneword(struct map_info *map, struct flchip *chip, > map_write(map, datum, adr); > > for (i = 0; i < jiffies_to_usecs(uWriteTimeout); i++) { > - if (chip_ready(map, adr)) > + if (chip_ready(map, chip, adr)) > break; > > udelay(1); > } > > - if (!chip_good(map, adr, datum)) { > + if (!chip_good(map, chip, adr, datum)) { > /* reset on all failures. */ > + cfi_check_err_status(map, chip, adr); > map_write(map, CMD(0xF0), chip->start); > /* FIXME - should have reset delay before continuing */ > > @@ -2300,7 +2394,7 @@ static int __xipram do_erase_chip(struct map_info *map, struct flchip *chip) > chip->erase_suspended = 0; > } > > - if (chip_good(map, adr, map_word_ff(map))) > + if (chip_good(map, chip, adr, map_word_ff(map))) > break; > > if (time_after(jiffies, timeo)) { > @@ -2316,6 +2410,7 @@ static int __xipram do_erase_chip(struct map_info *map, struct flchip *chip) > /* Did we succeed? */ > if (ret) { > /* reset on all failures. */ > + cfi_check_err_status(map, chip, adr); > map_write(map, CMD(0xF0), chip->start); > /* FIXME - should have reset delay before continuing */ > > @@ -2396,7 +2491,7 @@ static int __xipram do_erase_oneblock(struct map_info *map, struct flchip *chip, > chip->erase_suspended = 0; > } > > - if (chip_good(map, adr, map_word_ff(map))) > + if (chip_good(map, chip, adr, map_word_ff(map))) > break; > > if (time_after(jiffies, timeo)) { > @@ -2412,6 +2507,7 @@ static int __xipram do_erase_oneblock(struct map_info *map, struct flchip *chip, > /* Did we succeed? */ > if (ret) { > /* reset on all failures. */ > + cfi_check_err_status(map, chip, adr); > map_write(map, CMD(0xF0), chip->start); > /* FIXME - should have reset delay before continuing */ > > @@ -2589,7 +2685,7 @@ static int __maybe_unused do_ppb_xxlock(struct map_info *map, > */ > timeo = jiffies + msecs_to_jiffies(2000); /* 2s max (un)locking */ > for (;;) { > - if (chip_ready(map, adr)) > + if (chip_ready(map, chip, adr)) > break; > > if (time_after(jiffies, timeo)) { > diff --git a/include/linux/mtd/cfi.h b/include/linux/mtd/cfi.h > index 208c87cf2e3e..c98a21108688 100644 > --- a/include/linux/mtd/cfi.h > +++ b/include/linux/mtd/cfi.h > @@ -219,6 +219,13 @@ struct cfi_pri_amdstd { > uint8_t VppMin; > uint8_t VppMax; > uint8_t TopBottom; > + /* Below field are added from version 1.5 */ > + uint8_t ProgramSuspend; > + uint8_t UnlockBypass; > + uint8_t SecureSiliconSector; > + uint8_t SoftwareFeatures; > +#define CFI_POLL_STATUS_REG BIT(0) > +#define CFI_POLL_DQ BIT(1) > } __packed; > > /* Vendor-Specific PRI for Atmel chips (command set 0x0002) */
Hello! On 06/25/2019 10:57 AM, Vignesh Raghavendra wrote: > Cypress' HyperBus is Low Signal Count, High Performance Double Data Rate > Bus interface between a host system master and one or more slave > interfaces. HyperBus is used to connect microprocessor, microcontroller, > or ASIC devices with random access NOR flash memory (called HyperFlash) > or self refresh DRAM (called HyperRAM). > > Its a 8-bit data bus (DQ[7:0]) with Read-Write Data Strobe (RWDS) > signal and either Single-ended clock(3.0V parts) or Differential clock > (1.8V parts). It uses ChipSelect lines to select b/w multiple slaves. > At bus level, it follows a separate protocol described in HyperBus > specification[1]. > > HyperFlash follows CFI AMD/Fujitsu Extended Command Set (0x0002) similar > to that of existing parallel NORs. Since HyperBus is x8 DDR bus, > its equivalent to x16 parallel NOR flash with respect to bits per clock > cycle. But HyperBus operates at >166MHz frequencies. > HyperRAM provides direct random read/write access to flash memory > array. > > But, HyperBus memory controllers seem to abstract implementation details > and expose a simple MMIO interface to access connected flash. > > Add support for registering HyperFlash devices with MTD framework. MTD > maps framework along with CFI chip support framework are used to support > communicating with flash. > > Framework is modelled along the lines of spi-nor framework. HyperBus > memory controller (HBMC) drivers calls hyperbus_register_device() to > register a single HyperFlash device. HyperFlash core parses MMIO access > information from DT, sets up the map_info struct, probes CFI flash and > registers it with MTD framework. > > Some HBMC masters need calibration/training sequence[3] to be carried > out, in order for DLL inside the controller to lock, by reading a known > string/pattern. This is done by repeatedly reading CFI Query > Identification String. Calibration needs to be done before trying to detect > flash as part of CFI flash probe. > > HyperRAM is not supported at the moment. > > HyperBus specification can be found at[1] > HyperFlash datasheet can be found at[2] > > [1] https://www.cypress.com/file/213356/download > [2] https://www.cypress.com/file/213346/download > [3] http://www.ti.com/lit/ug/spruid7b/spruid7b.pdf > Table 12-5741. HyperFlash Access Sequence > > Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> [...] > diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile > index 806287e80e84..62d649a959e2 100644 > --- a/drivers/mtd/Makefile > +++ b/drivers/mtd/Makefile > @@ -34,3 +34,4 @@ obj-y += chips/ lpddr/ maps/ devices/ nand/ tests/ > > obj-$(CONFIG_MTD_SPI_NOR) += spi-nor/ > obj-$(CONFIG_MTD_UBI) += ubi/ > +obj-$(CONFIG_MTD_HYPERBUS) += hyperbus/ > diff --git a/drivers/mtd/hyperbus/Kconfig b/drivers/mtd/hyperbus/Kconfig > new file mode 100644 > index 000000000000..98147e28caa0 > --- /dev/null > +++ b/drivers/mtd/hyperbus/Kconfig > @@ -0,0 +1,11 @@ > +menuconfig MTD_HYPERBUS > + tristate "HyperBus support" > + select MTD_CFI > + select MTD_MAP_BANK_WIDTH_2 > + select MTD_CFI_AMDSTD > + select MTD_COMPLEX_MAPPINGS > + help > + This is the framework for the HyperBus which can be used by > + the HyperBus Controller driver to communicate with > + HyperFlash. See Cypress HyperBus specification for more > + details > diff --git a/drivers/mtd/hyperbus/Makefile b/drivers/mtd/hyperbus/Makefile > new file mode 100644 > index 000000000000..ca61dedd730d > --- /dev/null > +++ b/drivers/mtd/hyperbus/Makefile > @@ -0,0 +1,3 @@ > +# SPDX-License-Identifier: GPL-2.0 > + > +obj-$(CONFIG_MTD_HYPERBUS) += hyperbus-core.o > diff --git a/drivers/mtd/hyperbus/hyperbus-core.c b/drivers/mtd/hyperbus/hyperbus-core.c > new file mode 100644 > index 000000000000..63a9e64895bc > --- /dev/null > +++ b/drivers/mtd/hyperbus/hyperbus-core.c > @@ -0,0 +1,154 @@ [...] > +int hyperbus_register_device(struct hyperbus_device *hbdev) > +{ [...] > + hbdev->mtd = do_map_probe("cfi_probe", map); > + if (!hbdev->mtd) { > + dev_err(dev, "probing of hyperbus device failed\n"); > + return -ENODEV; > + } > + > + hbdev->mtd->dev.parent = dev; > + mtd_set_of_node(hbdev->mtd, np); > + > + ret = mtd_device_register(hbdev->mtd, NULL, 0); > + if (ret) { > + dev_err(dev, "failed to register mtd device\n"); > + map_destroy(hbdev->mtd); > + return ret; > + } > + hbdev->registered = true; I doubt that you actually need this flag... > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(hyperbus_register_device); > + > +int hyperbus_unregister_device(struct hyperbus_device *hbdev) > +{ > + int ret = 0; > + > + if (hbdev && hbdev->mtd && hbdev->registered) { ... as you missed clearing that 'registered' flag. > + ret = mtd_device_unregister(hbdev->mtd); > + map_destroy(hbdev->mtd); > + } > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(hyperbus_unregister_device); [...] MBR, Sergei
On 25/06/19 10:31 PM, Tokunori Ikegami wrote: > Hi, > > Thanks for the fix. > > Reviewed-by: Tokunori Ikegami <ikegami.t@gmail.com> > > I have just tested the patch quickly on my local environment that uses > the cfi_cmdset_0002 flash device but not HyperFlash family. > So tested as not affected by the change. > Thanks for testing! > On 2019/06/25 16:57, Vignesh Raghavendra wrote: >> HyperFlash devices are compliant with CFI AMD/Fujitsu Extended Command >> Set (0x0002) for flash operations, therefore >> drivers/mtd/chips/cfi_cmdset_0002.c can be used as is. But these devices >> do not support DQ polling method of determining chip ready/good status. >> These flashes provide Status Register whose bits can be polled to know >> status of flash operation. >> >> Cypress HyperFlash datasheet here[1], talks about CFI Amd/Fujitsu >> Extended Query version 1.5. Bit 0 of "Software Features supported" field >> of CFI Primary Vendor-Specific Extended Query table indicates >> presence/absence of status register and Bit 1 indicates whether or not >> DQ polling is supported. Using these bits, its possible to determine >> whether flash supports DQ polling or need to use Status Register. >> >> Add support for polling Status Register to know device ready/status of >> erase/write operations when DQ polling is not supported. >> Print error messages on erase/program failure by looking at related >> Status Register bits. >> >> [1] https://www.cypress.com/file/213346/download >> >> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> >> --- >> v8: >> Fix up status register polling to support banked flashes in patch 1/5. >> >> >> drivers/mtd/chips/cfi_cmdset_0002.c | 130 ++++++++++++++++++++++++---- >> include/linux/mtd/cfi.h | 7 ++ >> 2 files changed, 120 insertions(+), 17 deletions(-) >> >> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c >> b/drivers/mtd/chips/cfi_cmdset_0002.c >> index c8fa5906bdf9..09f8aaf1e763 100644 >> --- a/drivers/mtd/chips/cfi_cmdset_0002.c >> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c >> @@ -49,6 +49,16 @@ >> #define SST49LF008A 0x005a >> #define AT49BV6416 0x00d6 >> +/* >> + * Status Register bit description. Used by flash devices that don't >> + * support DQ polling (e.g. HyperFlash) >> + */ >> +#define CFI_SR_DRB BIT(7) >> +#define CFI_SR_ESB BIT(5) >> +#define CFI_SR_PSB BIT(4) >> +#define CFI_SR_WBASB BIT(3) >> +#define CFI_SR_SLSB BIT(1) >> + >> static int cfi_amdstd_read (struct mtd_info *, loff_t, size_t, >> size_t *, u_char *); >> static int cfi_amdstd_write_words(struct mtd_info *, loff_t, size_t, >> size_t *, const u_char *); >> static int cfi_amdstd_write_buffers(struct mtd_info *, loff_t, >> size_t, size_t *, const u_char *); >> @@ -97,6 +107,50 @@ static struct mtd_chip_driver cfi_amdstd_chipdrv = { >> .module = THIS_MODULE >> }; >> +/* >> + * Use status register to poll for Erase/write completion when DQ is not >> + * supported. This is indicated by Bit[1:0] of SoftwareFeatures field in >> + * CFI Primary Vendor-Specific Extended Query table 1.5 >> + */ >> +static int cfi_use_status_reg(struct cfi_private *cfi) >> +{ >> + struct cfi_pri_amdstd *extp = cfi->cmdset_priv; >> + u8 poll_mask = CFI_POLL_STATUS_REG | CFI_POLL_DQ; >> + >> + return extp->MinorVersion >= '5' && >> + (extp->SoftwareFeatures & poll_mask) == CFI_POLL_STATUS_REG; >> +} >> + >> +static void cfi_check_err_status(struct map_info *map, struct flchip >> *chip, > > Just a minor comment. > I though that it is better to be called this function in chip_good() > instead of to be called separately. > But I can understand that the chip_good() is called repeatedly in > do_erase_chip(), do_erase_oneblock() and do_write_buffer() until timeout. > Also do_write_oneword() is also possible to be changed to call the > chip_good() repeatedly instead of chip_ready(). > So additional logics is needed to avoid the repeated error messages in > the function above. > Anyway it is okay for the current implementation to me. > Will do that as an incremental change once base support for HyperFlash is in. Regards Vignesh > Regards, > Ikegami > >> + unsigned long adr) >> +{ >> + struct cfi_private *cfi = map->fldrv_priv; >> + map_word status; >> + >> + if (!cfi_use_status_reg(cfi)) >> + return; >> + >> + cfi_send_gen_cmd(0x70, cfi->addr_unlock1, chip->start, map, cfi, >> + cfi->device_type, NULL); >> + status = map_read(map, adr); >> + >> + if (map_word_bitsset(map, status, CMD(0x3a))) { >> + unsigned long chipstatus = MERGESTATUS(status); >> + >> + if (chipstatus & CFI_SR_ESB) >> + pr_err("%s erase operation failed, status %lx\n", >> + map->name, chipstatus); >> + if (chipstatus & CFI_SR_PSB) >> + pr_err("%s program operation failed, status %lx\n", >> + map->name, chipstatus); >> + if (chipstatus & CFI_SR_WBASB) >> + pr_err("%s buffer program command aborted, status %lx\n", >> + map->name, chipstatus); >> + if (chipstatus & CFI_SR_SLSB) >> + pr_err("%s sector write protected, status %lx\n", >> + map->name, chipstatus); >> + } >> +} >> /* #define DEBUG_CFI_FEATURES */ >> @@ -742,10 +796,25 @@ static struct mtd_info >> *cfi_amdstd_setup(struct mtd_info *mtd) >> * correctly and is therefore not done (particularly with >> interleaved chips >> * as each chip must be checked independently of the others). >> */ >> -static int __xipram chip_ready(struct map_info *map, unsigned long addr) >> +static int __xipram chip_ready(struct map_info *map, struct flchip >> *chip, >> + unsigned long addr) >> { >> + struct cfi_private *cfi = map->fldrv_priv; >> map_word d, t; >> + if (cfi_use_status_reg(cfi)) { >> + map_word ready = CMD(CFI_SR_DRB); >> + /* >> + * For chips that support status register, check device >> + * ready bit >> + */ >> + cfi_send_gen_cmd(0x70, cfi->addr_unlock1, chip->start, map, cfi, >> + cfi->device_type, NULL); >> + d = map_read(map, addr); >> + >> + return map_word_andequal(map, d, ready, ready); >> + } >> + >> d = map_read(map, addr); >> t = map_read(map, addr); >> @@ -767,10 +836,30 @@ static int __xipram chip_ready(struct map_info >> *map, unsigned long addr) >> * as each chip must be checked independently of the others). >> * >> */ >> -static int __xipram chip_good(struct map_info *map, unsigned long >> addr, map_word expected) >> +static int __xipram chip_good(struct map_info *map, struct flchip *chip, >> + unsigned long addr, map_word expected) >> { >> + struct cfi_private *cfi = map->fldrv_priv; >> map_word oldd, curd; >> + if (cfi_use_status_reg(cfi)) { >> + map_word ready = CMD(CFI_SR_DRB); >> + map_word err = CMD(CFI_SR_PSB | CFI_SR_ESB); >> + /* >> + * For chips that support status register, check device >> + * ready bit and Erase/Program status bit to know if >> + * operation succeeded. >> + */ >> + cfi_send_gen_cmd(0x70, cfi->addr_unlock1, chip->start, map, cfi, >> + cfi->device_type, NULL); >> + curd = map_read(map, addr); >> + >> + if (map_word_andequal(map, curd, ready, ready)) >> + return !map_word_bitsset(map, curd, err); >> + >> + return 0; >> + } >> + >> oldd = map_read(map, addr); >> curd = map_read(map, addr); >> @@ -792,7 +881,7 @@ static int get_chip(struct map_info *map, struct >> flchip *chip, unsigned long adr >> case FL_STATUS: >> for (;;) { >> - if (chip_ready(map, adr)) >> + if (chip_ready(map, chip, adr)) >> break; >> if (time_after(jiffies, timeo)) { >> @@ -830,7 +919,7 @@ static int get_chip(struct map_info *map, struct >> flchip *chip, unsigned long adr >> chip->state = FL_ERASE_SUSPENDING; >> chip->erase_suspended = 1; >> for (;;) { >> - if (chip_ready(map, adr)) >> + if (chip_ready(map, chip, adr)) >> break; >> if (time_after(jiffies, timeo)) { >> @@ -1362,7 +1451,7 @@ static int do_otp_lock(struct map_info *map, >> struct flchip *chip, loff_t adr, >> /* wait for chip to become ready */ >> timeo = jiffies + msecs_to_jiffies(2); >> for (;;) { >> - if (chip_ready(map, adr)) >> + if (chip_ready(map, chip, adr)) >> break; >> if (time_after(jiffies, timeo)) { >> @@ -1628,22 +1717,24 @@ static int __xipram do_write_oneword(struct >> map_info *map, struct flchip *chip, >> continue; >> } >> - if (time_after(jiffies, timeo) && !chip_ready(map, adr)){ >> + if (time_after(jiffies, timeo) && >> + !chip_ready(map, chip, adr)) { >> xip_enable(map, chip, adr); >> printk(KERN_WARNING "MTD %s(): software timeout\n", >> __func__); >> xip_disable(map, chip, adr); >> break; >> } >> - if (chip_ready(map, adr)) >> + if (chip_ready(map, chip, adr)) >> break; >> /* Latency issues. Drop the lock, wait a while and retry */ >> UDELAY(map, chip, adr, 1); >> } >> /* Did we succeed? */ >> - if (!chip_good(map, adr, datum)) { >> + if (!chip_good(map, chip, adr, datum)) { >> /* reset on all failures. */ >> + cfi_check_err_status(map, chip, adr); >> map_write(map, CMD(0xF0), chip->start); >> /* FIXME - should have reset delay before continuing */ >> @@ -1881,10 +1972,11 @@ static int __xipram do_write_buffer(struct >> map_info *map, struct flchip *chip, >> * We check "time_after" and "!chip_good" before checking >> "chip_good" to avoid >> * the failure due to scheduling. >> */ >> - if (time_after(jiffies, timeo) && !chip_good(map, adr, datum)) >> + if (time_after(jiffies, timeo) && >> + !chip_good(map, chip, adr, datum)) >> break; >> - if (chip_good(map, adr, datum)) { >> + if (chip_good(map, chip, adr, datum)) { >> xip_enable(map, chip, adr); >> goto op_done; >> } >> @@ -1901,6 +1993,7 @@ static int __xipram do_write_buffer(struct >> map_info *map, struct flchip *chip, >> * See e.g. >> * >> http://www.spansion.com/Support/Application%20Notes/MirrorBit_Write_Buffer_Prog_Page_Buffer_Read_AN.pdf >> >> */ >> + cfi_check_err_status(map, chip, adr); >> cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi, >> cfi->device_type, NULL); >> cfi_send_gen_cmd(0x55, cfi->addr_unlock2, chip->start, map, cfi, >> @@ -2018,7 +2111,7 @@ static int cfi_amdstd_panic_wait(struct map_info >> *map, struct flchip *chip, >> * If the driver thinks the chip is idle, and no toggle bits >> * are changing, then the chip is actually idle for sure. >> */ >> - if (chip->state == FL_READY && chip_ready(map, adr)) >> + if (chip->state == FL_READY && chip_ready(map, chip, adr)) >> return 0; >> /* >> @@ -2035,7 +2128,7 @@ static int cfi_amdstd_panic_wait(struct map_info >> *map, struct flchip *chip, >> /* wait for the chip to become ready */ >> for (i = 0; i < jiffies_to_usecs(timeo); i++) { >> - if (chip_ready(map, adr)) >> + if (chip_ready(map, chip, adr)) >> return 0; >> udelay(1); >> @@ -2099,14 +2192,15 @@ static int do_panic_write_oneword(struct >> map_info *map, struct flchip *chip, >> map_write(map, datum, adr); >> for (i = 0; i < jiffies_to_usecs(uWriteTimeout); i++) { >> - if (chip_ready(map, adr)) >> + if (chip_ready(map, chip, adr)) >> break; >> udelay(1); >> } >> - if (!chip_good(map, adr, datum)) { >> + if (!chip_good(map, chip, adr, datum)) { >> /* reset on all failures. */ >> + cfi_check_err_status(map, chip, adr); >> map_write(map, CMD(0xF0), chip->start); >> /* FIXME - should have reset delay before continuing */ >> @@ -2300,7 +2394,7 @@ static int __xipram do_erase_chip(struct >> map_info *map, struct flchip *chip) >> chip->erase_suspended = 0; >> } >> - if (chip_good(map, adr, map_word_ff(map))) >> + if (chip_good(map, chip, adr, map_word_ff(map))) >> break; >> if (time_after(jiffies, timeo)) { >> @@ -2316,6 +2410,7 @@ static int __xipram do_erase_chip(struct >> map_info *map, struct flchip *chip) >> /* Did we succeed? */ >> if (ret) { >> /* reset on all failures. */ >> + cfi_check_err_status(map, chip, adr); >> map_write(map, CMD(0xF0), chip->start); >> /* FIXME - should have reset delay before continuing */ >> @@ -2396,7 +2491,7 @@ static int __xipram do_erase_oneblock(struct >> map_info *map, struct flchip *chip, >> chip->erase_suspended = 0; >> } >> - if (chip_good(map, adr, map_word_ff(map))) >> + if (chip_good(map, chip, adr, map_word_ff(map))) >> break; >> if (time_after(jiffies, timeo)) { >> @@ -2412,6 +2507,7 @@ static int __xipram do_erase_oneblock(struct >> map_info *map, struct flchip *chip, >> /* Did we succeed? */ >> if (ret) { >> /* reset on all failures. */ >> + cfi_check_err_status(map, chip, adr); >> map_write(map, CMD(0xF0), chip->start); >> /* FIXME - should have reset delay before continuing */ >> @@ -2589,7 +2685,7 @@ static int __maybe_unused do_ppb_xxlock(struct >> map_info *map, >> */ >> timeo = jiffies + msecs_to_jiffies(2000); /* 2s max >> (un)locking */ >> for (;;) { >> - if (chip_ready(map, adr)) >> + if (chip_ready(map, chip, adr)) >> break; >> if (time_after(jiffies, timeo)) { >> diff --git a/include/linux/mtd/cfi.h b/include/linux/mtd/cfi.h >> index 208c87cf2e3e..c98a21108688 100644 >> --- a/include/linux/mtd/cfi.h >> +++ b/include/linux/mtd/cfi.h >> @@ -219,6 +219,13 @@ struct cfi_pri_amdstd { >> uint8_t VppMin; >> uint8_t VppMax; >> uint8_t TopBottom; >> + /* Below field are added from version 1.5 */ >> + uint8_t ProgramSuspend; >> + uint8_t UnlockBypass; >> + uint8_t SecureSiliconSector; >> + uint8_t SoftwareFeatures; >> +#define CFI_POLL_STATUS_REG BIT(0) >> +#define CFI_POLL_DQ BIT(1) >> } __packed; >> /* Vendor-Specific PRI for Atmel chips (command set 0x0002) */
Hi, On 25-Jun-19 1:27 PM, Vignesh Raghavendra wrote: [...] > Vignesh Raghavendra (5): > mtd: cfi_cmdset_0002: Add support for polling status register > dt-bindings: mtd: Add binding documentation for HyperFlash > mtd: Add support for HyperBus memory devices > dt-bindings: mtd: Add bindings for TI's AM654 HyperBus memory > controller > mtd: hyperbus: Add driver for TI's HyperBus memory controller > Fixed comments on patch 3/5 locally and series applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git branch mtd/next. Regards Vignesh > .../bindings/mtd/cypress,hyperflash.txt | 13 ++ > .../devicetree/bindings/mtd/ti,am654-hbmc.txt | 51 ++++++ > MAINTAINERS | 8 + > drivers/mtd/Kconfig | 2 + > drivers/mtd/Makefile | 1 + > drivers/mtd/chips/cfi_cmdset_0002.c | 130 +++++++++++++-- > drivers/mtd/hyperbus/Kconfig | 23 +++ > drivers/mtd/hyperbus/Makefile | 4 + > drivers/mtd/hyperbus/hbmc-am654.c | 141 ++++++++++++++++ > drivers/mtd/hyperbus/hyperbus-core.c | 154 ++++++++++++++++++ > include/linux/mtd/cfi.h | 7 + > include/linux/mtd/hyperbus.h | 86 ++++++++++ > 12 files changed, 603 insertions(+), 17 deletions(-) > create mode 100644 Documentation/devicetree/bindings/mtd/cypress,hyperflash.txt > create mode 100644 Documentation/devicetree/bindings/mtd/ti,am654-hbmc.txt > create mode 100644 drivers/mtd/hyperbus/Kconfig > create mode 100644 drivers/mtd/hyperbus/Makefile > create mode 100644 drivers/mtd/hyperbus/hbmc-am654.c > create mode 100644 drivers/mtd/hyperbus/hyperbus-core.c > create mode 100644 include/linux/mtd/hyperbus.h >
Hello! On 06/25/2019 10:57 AM, Vignesh Raghavendra wrote: > Cypress' HyperBus is Low Signal Count, High Performance Double Data Rate > Bus interface between a host system master and one or more slave > interfaces. HyperBus is used to connect microprocessor, microcontroller, > or ASIC devices with random access NOR flash memory (called HyperFlash) > or self refresh DRAM (called HyperRAM). > > Its a 8-bit data bus (DQ[7:0]) with Read-Write Data Strobe (RWDS) > signal and either Single-ended clock(3.0V parts) or Differential clock > (1.8V parts). It uses ChipSelect lines to select b/w multiple slaves. > At bus level, it follows a separate protocol described in HyperBus > specification[1]. > > HyperFlash follows CFI AMD/Fujitsu Extended Command Set (0x0002) similar > to that of existing parallel NORs. Since HyperBus is x8 DDR bus, > its equivalent to x16 parallel NOR flash with respect to bits per clock > cycle. But HyperBus operates at >166MHz frequencies. > HyperRAM provides direct random read/write access to flash memory > array. > > But, HyperBus memory controllers seem to abstract implementation details > and expose a simple MMIO interface to access connected flash. > > Add support for registering HyperFlash devices with MTD framework. MTD > maps framework along with CFI chip support framework are used to support > communicating with flash. > > Framework is modelled along the lines of spi-nor framework. HyperBus > memory controller (HBMC) drivers calls hyperbus_register_device() to > register a single HyperFlash device. HyperFlash core parses MMIO access > information from DT, sets up the map_info struct, probes CFI flash and > registers it with MTD framework. > > Some HBMC masters need calibration/training sequence[3] to be carried > out, in order for DLL inside the controller to lock, by reading a known > string/pattern. This is done by repeatedly reading CFI Query > Identification String. Calibration needs to be done before trying to detect > flash as part of CFI flash probe. > > HyperRAM is not supported at the moment. > > HyperBus specification can be found at[1] > HyperFlash datasheet can be found at[2] > > [1] https://www.cypress.com/file/213356/download > [2] https://www.cypress.com/file/213346/download > [3] http://www.ti.com/lit/ug/spruid7b/spruid7b.pdf > Table 12-5741. HyperFlash Access Sequence > > Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> [...] I have at least created my HyperBus driver and unfortunately I'm having serious issues with the design of the support core (see below)... [...] > diff --git a/drivers/mtd/hyperbus/hyperbus-core.c b/drivers/mtd/hyperbus/hyperbus-core.c > new file mode 100644 > index 000000000000..63a9e64895bc > --- /dev/null > +++ b/drivers/mtd/hyperbus/hyperbus-core.c > @@ -0,0 +1,154 @@ [...] > +int hyperbus_register_device(struct hyperbus_device *hbdev) > +{ > + const struct hyperbus_ops *ops; > + struct hyperbus_ctlr *ctlr; > + struct device_node *np; > + struct map_info *map; > + struct resource res; > + struct device *dev; > + int ret; > + > + if (!hbdev || !hbdev->np || !hbdev->ctlr || !hbdev->ctlr->dev) { > + pr_err("hyperbus: please fill all the necessary fields!\n"); > + return -EINVAL; > + } > + > + np = hbdev->np; > + ctlr = hbdev->ctlr; > + if (!of_device_is_compatible(np, "cypress,hyperflash")) > + return -ENODEV; > + > + hbdev->memtype = HYPERFLASH; > + > + ret = of_address_to_resource(np, 0, &res); Hm, I doubt that the HB devices are wholly mapped into memory space, that seems like a property of the HB controller. In my case, the flash device in the DT has only single-cell "reg" prop (equal to the chip select #). Then this function returns -EINVAL and the registration fails. Also, in my case such mapping is R/O, not R/W. > + if (ret) > + return ret; > + > + dev = ctlr->dev; > + map = &hbdev->map; > + map->size = resource_size(&res); > + map->virt = devm_ioremap_resource(dev, &res); > + if (IS_ERR(map->virt)) > + return PTR_ERR(map->virt); Again, I doubt that this should be done here, and not in the HB controller driver... [...] MBR, Sergei
On 02/07/19 11:23 PM, Sergei Shtylyov wrote: > Hello! > > On 06/25/2019 10:57 AM, Vignesh Raghavendra wrote: > >> Cypress' HyperBus is Low Signal Count, High Performance Double Data Rate >> Bus interface between a host system master and one or more slave >> interfaces. HyperBus is used to connect microprocessor, microcontroller, >> or ASIC devices with random access NOR flash memory (called HyperFlash) >> or self refresh DRAM (called HyperRAM). >> >> Its a 8-bit data bus (DQ[7:0]) with Read-Write Data Strobe (RWDS) >> signal and either Single-ended clock(3.0V parts) or Differential clock >> (1.8V parts). It uses ChipSelect lines to select b/w multiple slaves. >> At bus level, it follows a separate protocol described in HyperBus >> specification[1]. >> >> HyperFlash follows CFI AMD/Fujitsu Extended Command Set (0x0002) similar >> to that of existing parallel NORs. Since HyperBus is x8 DDR bus, >> its equivalent to x16 parallel NOR flash with respect to bits per clock >> cycle. But HyperBus operates at >166MHz frequencies. >> HyperRAM provides direct random read/write access to flash memory >> array. >> >> But, HyperBus memory controllers seem to abstract implementation details >> and expose a simple MMIO interface to access connected flash. >> >> Add support for registering HyperFlash devices with MTD framework. MTD >> maps framework along with CFI chip support framework are used to support >> communicating with flash. >> >> Framework is modelled along the lines of spi-nor framework. HyperBus >> memory controller (HBMC) drivers calls hyperbus_register_device() to >> register a single HyperFlash device. HyperFlash core parses MMIO access >> information from DT, sets up the map_info struct, probes CFI flash and >> registers it with MTD framework. >> >> Some HBMC masters need calibration/training sequence[3] to be carried >> out, in order for DLL inside the controller to lock, by reading a known >> string/pattern. This is done by repeatedly reading CFI Query >> Identification String. Calibration needs to be done before trying to detect >> flash as part of CFI flash probe. >> >> HyperRAM is not supported at the moment. >> >> HyperBus specification can be found at[1] >> HyperFlash datasheet can be found at[2] >> >> [1] https://www.cypress.com/file/213356/download >> [2] https://www.cypress.com/file/213346/download >> [3] http://www.ti.com/lit/ug/spruid7b/spruid7b.pdf >> Table 12-5741. HyperFlash Access Sequence >> >> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> > [...] > > I have at least created my HyperBus driver and unfortunately I'm having serious > issues with the design of the support core (see below)... > > [...] >> diff --git a/drivers/mtd/hyperbus/hyperbus-core.c b/drivers/mtd/hyperbus/hyperbus-core.c >> new file mode 100644 >> index 000000000000..63a9e64895bc >> --- /dev/null >> +++ b/drivers/mtd/hyperbus/hyperbus-core.c >> @@ -0,0 +1,154 @@ > [...] >> +int hyperbus_register_device(struct hyperbus_device *hbdev) >> +{ >> + const struct hyperbus_ops *ops; >> + struct hyperbus_ctlr *ctlr; >> + struct device_node *np; >> + struct map_info *map; >> + struct resource res; >> + struct device *dev; >> + int ret; >> + >> + if (!hbdev || !hbdev->np || !hbdev->ctlr || !hbdev->ctlr->dev) { >> + pr_err("hyperbus: please fill all the necessary fields!\n"); >> + return -EINVAL; >> + } >> + >> + np = hbdev->np; >> + ctlr = hbdev->ctlr; >> + if (!of_device_is_compatible(np, "cypress,hyperflash")) >> + return -ENODEV; >> + >> + hbdev->memtype = HYPERFLASH; >> + >> + ret = of_address_to_resource(np, 0, &res); > > Hm, I doubt that the HB devices are wholly mapped into memory space, that seems > like a property of the HB controller. In my case, the flash device in the DT has > only single-cell "reg" prop (equal to the chip select #). Then this function returns > -EINVAL and the registration fails. Also, in my case such mapping is R/O, not R/W. > You could declare R/O MMIO region in controla and set up a translation using ranges from slave's reg CS based reg mapping like: + hbmc: hyperbus@47034000 { + compatible = "ti,am654-hbmc"; + reg = <0x0 0x47034000 0x0 0x100>, + <0x5 0x00000000 0x1 0x0000000>; + #address-cells = <2>; + #size-cells = <1>; + ranges = <0x0 0x0 0x5 0x00000000 0x4000000>, /* CS0 - 64MB */ + <0x1 0x0 0x5 0x04000000 0x4000000>; /* CS1 - 64MB */ + + /* Slave flash node */ + flash@0,0 { + compatible = "cypress,hyperflash", "cfi-flash"; + reg = <0x0 0x0 0x4000000>; + }; + }; If you use just CS# how would you handle CS to MMIO region mapping? Does both CS use the same MMIO base for reads? >> + if (ret) >> + return ret; >> + >> + dev = ctlr->dev; >> + map = &hbdev->map; >> + map->size = resource_size(&res); >> + map->virt = devm_ioremap_resource(dev, &res); >> + if (IS_ERR(map->virt)) >> + return PTR_ERR(map->virt); > > Again, I doubt that this should be done here, and not in the HB controller driver... If multiple CS use same MMIO base, then I can make this part of code non fatal when reg entry is a single cell and introduce notion of CS like SPI > > [...] > > MBR, Sergei >
Hello! On 07/03/2019 07:41 AM, Vignesh Raghavendra wrote: >>> Cypress' HyperBus is Low Signal Count, High Performance Double Data Rate >>> Bus interface between a host system master and one or more slave >>> interfaces. HyperBus is used to connect microprocessor, microcontroller, >>> or ASIC devices with random access NOR flash memory (called HyperFlash) >>> or self refresh DRAM (called HyperRAM). >>> >>> Its a 8-bit data bus (DQ[7:0]) with Read-Write Data Strobe (RWDS) >>> signal and either Single-ended clock(3.0V parts) or Differential clock >>> (1.8V parts). It uses ChipSelect lines to select b/w multiple slaves. >>> At bus level, it follows a separate protocol described in HyperBus >>> specification[1]. >>> >>> HyperFlash follows CFI AMD/Fujitsu Extended Command Set (0x0002) similar >>> to that of existing parallel NORs. Since HyperBus is x8 DDR bus, >>> its equivalent to x16 parallel NOR flash with respect to bits per clock >>> cycle. But HyperBus operates at >166MHz frequencies. >>> HyperRAM provides direct random read/write access to flash memory >>> array. >>> >>> But, HyperBus memory controllers seem to abstract implementation details >>> and expose a simple MMIO interface to access connected flash. >>> >>> Add support for registering HyperFlash devices with MTD framework. MTD >>> maps framework along with CFI chip support framework are used to support >>> communicating with flash. >>> >>> Framework is modelled along the lines of spi-nor framework. HyperBus >>> memory controller (HBMC) drivers calls hyperbus_register_device() to >>> register a single HyperFlash device. HyperFlash core parses MMIO access >>> information from DT, sets up the map_info struct, probes CFI flash and >>> registers it with MTD framework. >>> >>> Some HBMC masters need calibration/training sequence[3] to be carried >>> out, in order for DLL inside the controller to lock, by reading a known >>> string/pattern. This is done by repeatedly reading CFI Query >>> Identification String. Calibration needs to be done before trying to detect >>> flash as part of CFI flash probe. >>> >>> HyperRAM is not supported at the moment. >>> >>> HyperBus specification can be found at[1] >>> HyperFlash datasheet can be found at[2] >>> >>> [1] https://www.cypress.com/file/213356/download >>> [2] https://www.cypress.com/file/213346/download >>> [3] http://www.ti.com/lit/ug/spruid7b/spruid7b.pdf >>> Table 12-5741. HyperFlash Access Sequence >>> >>> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> >> [...] >> >> I have at least created my HyperBus driver and unfortunately I'm having serious At last. :-) >> issues with the design of the support core (see below)... >> >> [...] >>> diff --git a/drivers/mtd/hyperbus/hyperbus-core.c b/drivers/mtd/hyperbus/hyperbus-core.c >>> new file mode 100644 >>> index 000000000000..63a9e64895bc >>> --- /dev/null >>> +++ b/drivers/mtd/hyperbus/hyperbus-core.c >>> @@ -0,0 +1,154 @@ >> [...] >>> +int hyperbus_register_device(struct hyperbus_device *hbdev) >>> +{ >>> + const struct hyperbus_ops *ops; >>> + struct hyperbus_ctlr *ctlr; >>> + struct device_node *np; >>> + struct map_info *map; >>> + struct resource res; >>> + struct device *dev; >>> + int ret; >>> + >>> + if (!hbdev || !hbdev->np || !hbdev->ctlr || !hbdev->ctlr->dev) { >>> + pr_err("hyperbus: please fill all the necessary fields!\n"); >>> + return -EINVAL; >>> + } >>> + >>> + np = hbdev->np; >>> + ctlr = hbdev->ctlr; >>> + if (!of_device_is_compatible(np, "cypress,hyperflash")) >>> + return -ENODEV; >>> + >>> + hbdev->memtype = HYPERFLASH; >>> + >>> + ret = of_address_to_resource(np, 0, &res); >> >> Hm, I doubt that the HB devices are wholly mapped into memory space, that seems >> like a property of the HB controller. In my case, the flash device in the DT has >> only single-cell "reg" prop (equal to the chip select #). Then this function returns >> -EINVAL and the registration fails. Also, in my case such mapping is R/O, not R/W. >> > > You could declare R/O MMIO region in controla and set up a translation using ranges > from slave's reg CS based reg mapping like: No, not all HB controllers work the same (simple) way as yours. In case of RPC-IF, the direct read map is a 64 MiB window into a possibly larger flash chip, it has a register supplying address bits 25:31... > + hbmc: hyperbus@47034000 { > + compatible = "ti,am654-hbmc"; > + reg = <0x0 0x47034000 0x0 0x100>, > + <0x5 0x00000000 0x1 0x0000000>; > + #address-cells = <2>; > + #size-cells = <1>; > + ranges = <0x0 0x0 0x5 0x00000000 0x4000000>, /* CS0 - 64MB */ > + <0x1 0x0 0x5 0x04000000 0x4000000>; /* CS1 - 64MB */ > + > + /* Slave flash node */ > + flash@0,0 { > + compatible = "cypress,hyperflash", "cfi-flash"; > + reg = <0x0 0x0 0x4000000>; > + }; > + }; > > If you use just CS# how would you handle CS to MMIO region mapping? > Does both CS use the same MMIO base for reads? The RPC-IF HF mode only has a single CS signal. [...] MBR, Sergei
On 03-Jul-19 11:44 PM, Sergei Shtylyov wrote: > Hello! > > On 07/03/2019 07:41 AM, Vignesh Raghavendra wrote: > >>>> Cypress' HyperBus is Low Signal Count, High Performance Double Data Rate >>>> Bus interface between a host system master and one or more slave >>>> interfaces. HyperBus is used to connect microprocessor, microcontroller, >>>> or ASIC devices with random access NOR flash memory (called HyperFlash) >>>> or self refresh DRAM (called HyperRAM). >>>> >>>> Its a 8-bit data bus (DQ[7:0]) with Read-Write Data Strobe (RWDS) >>>> signal and either Single-ended clock(3.0V parts) or Differential clock >>>> (1.8V parts). It uses ChipSelect lines to select b/w multiple slaves. >>>> At bus level, it follows a separate protocol described in HyperBus >>>> specification[1]. >>>> >>>> HyperFlash follows CFI AMD/Fujitsu Extended Command Set (0x0002) similar >>>> to that of existing parallel NORs. Since HyperBus is x8 DDR bus, >>>> its equivalent to x16 parallel NOR flash with respect to bits per clock >>>> cycle. But HyperBus operates at >166MHz frequencies. >>>> HyperRAM provides direct random read/write access to flash memory >>>> array. >>>> >>>> But, HyperBus memory controllers seem to abstract implementation details >>>> and expose a simple MMIO interface to access connected flash. >>>> >>>> Add support for registering HyperFlash devices with MTD framework. MTD >>>> maps framework along with CFI chip support framework are used to support >>>> communicating with flash. >>>> >>>> Framework is modelled along the lines of spi-nor framework. HyperBus >>>> memory controller (HBMC) drivers calls hyperbus_register_device() to >>>> register a single HyperFlash device. HyperFlash core parses MMIO access >>>> information from DT, sets up the map_info struct, probes CFI flash and >>>> registers it with MTD framework. >>>> >>>> Some HBMC masters need calibration/training sequence[3] to be carried >>>> out, in order for DLL inside the controller to lock, by reading a known >>>> string/pattern. This is done by repeatedly reading CFI Query >>>> Identification String. Calibration needs to be done before trying to detect >>>> flash as part of CFI flash probe. >>>> >>>> HyperRAM is not supported at the moment. >>>> >>>> HyperBus specification can be found at[1] >>>> HyperFlash datasheet can be found at[2] >>>> >>>> [1] https://www.cypress.com/file/213356/download >>>> [2] https://www.cypress.com/file/213346/download >>>> [3] http://www.ti.com/lit/ug/spruid7b/spruid7b.pdf >>>> Table 12-5741. HyperFlash Access Sequence >>>> >>>> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> >>> [...] >>> >>> I have at least created my HyperBus driver and unfortunately I'm having serious > > At last. :-) > So, I guess driver works for limited memory size? >>> issues with the design of the support core (see below)... >>> >>> [...] >>>> diff --git a/drivers/mtd/hyperbus/hyperbus-core.c b/drivers/mtd/hyperbus/hyperbus-core.c >>>> new file mode 100644 >>>> index 000000000000..63a9e64895bc >>>> --- /dev/null >>>> +++ b/drivers/mtd/hyperbus/hyperbus-core.c >>>> @@ -0,0 +1,154 @@ >>> [...] >>>> +int hyperbus_register_device(struct hyperbus_device *hbdev) >>>> +{ >>>> + const struct hyperbus_ops *ops; >>>> + struct hyperbus_ctlr *ctlr; >>>> + struct device_node *np; >>>> + struct map_info *map; >>>> + struct resource res; >>>> + struct device *dev; >>>> + int ret; >>>> + >>>> + if (!hbdev || !hbdev->np || !hbdev->ctlr || !hbdev->ctlr->dev) { >>>> + pr_err("hyperbus: please fill all the necessary fields!\n"); >>>> + return -EINVAL; >>>> + } >>>> + >>>> + np = hbdev->np; >>>> + ctlr = hbdev->ctlr; >>>> + if (!of_device_is_compatible(np, "cypress,hyperflash")) >>>> + return -ENODEV; >>>> + >>>> + hbdev->memtype = HYPERFLASH; >>>> + >>>> + ret = of_address_to_resource(np, 0, &res); >>> >>> Hm, I doubt that the HB devices are wholly mapped into memory space, that seems >>> like a property of the HB controller. In my case, the flash device in the DT has >>> only single-cell "reg" prop (equal to the chip select #). Then this function returns >>> -EINVAL and the registration fails. Also, in my case such mapping is R/O, not R/W. >>> >> >> You could declare R/O MMIO region in controla and set up a translation using ranges >> from slave's reg CS based reg mapping like: > > No, not all HB controllers work the same (simple) way as yours. In case of RPC-IF, > the direct read map is a 64 MiB window into a possibly larger flash chip, it has a > register supplying address bits 25:31... Okay, this limitation was not made clear earlier. I thought RPC-IF also supported MMIO accesses for all reads I will look into changes needed to support HB controllers that don't have MMIO interface next week. Regards Vignesh > >> + hbmc: hyperbus@47034000 { >> + compatible = "ti,am654-hbmc"; >> + reg = <0x0 0x47034000 0x0 0x100>, >> + <0x5 0x00000000 0x1 0x0000000>; >> + #address-cells = <2>; >> + #size-cells = <1>; >> + ranges = <0x0 0x0 0x5 0x00000000 0x4000000>, /* CS0 - 64MB */ >> + <0x1 0x0 0x5 0x04000000 0x4000000>; /* CS1 - 64MB */ >> + >> + /* Slave flash node */ >> + flash@0,0 { >> + compatible = "cypress,hyperflash", "cfi-flash"; >> + reg = <0x0 0x0 0x4000000>; >> + }; >> + }; >> >> If you use just CS# how would you handle CS to MMIO region mapping? >> Does both CS use the same MMIO base for reads? > > The RPC-IF HF mode only has a single CS signal. > I see... > [...] > > MBR, Sergei > Regards Vignesh
Hello! On 06/25/2019 10:57 AM, Vignesh Raghavendra wrote: > Cypress' HyperBus is Low Signal Count, High Performance Double Data Rate > Bus interface between a host system master and one or more slave > interfaces. HyperBus is used to connect microprocessor, microcontroller, > or ASIC devices with random access NOR flash memory (called HyperFlash) > or self refresh DRAM (called HyperRAM). > > Its a 8-bit data bus (DQ[7:0]) with Read-Write Data Strobe (RWDS) > signal and either Single-ended clock(3.0V parts) or Differential clock > (1.8V parts). It uses ChipSelect lines to select b/w multiple slaves. > At bus level, it follows a separate protocol described in HyperBus > specification[1]. > > HyperFlash follows CFI AMD/Fujitsu Extended Command Set (0x0002) similar > to that of existing parallel NORs. Since HyperBus is x8 DDR bus, > its equivalent to x16 parallel NOR flash with respect to bits per clock > cycle. But HyperBus operates at >166MHz frequencies. > HyperRAM provides direct random read/write access to flash memory > array. > > But, HyperBus memory controllers seem to abstract implementation details > and expose a simple MMIO interface to access connected flash. > > Add support for registering HyperFlash devices with MTD framework. MTD > maps framework along with CFI chip support framework are used to support > communicating with flash. > > Framework is modelled along the lines of spi-nor framework. HyperBus > memory controller (HBMC) drivers calls hyperbus_register_device() to > register a single HyperFlash device. HyperFlash core parses MMIO access > information from DT, sets up the map_info struct, probes CFI flash and > registers it with MTD framework. > > Some HBMC masters need calibration/training sequence[3] to be carried > out, in order for DLL inside the controller to lock, by reading a known > string/pattern. This is done by repeatedly reading CFI Query > Identification String. Calibration needs to be done before trying to detect > flash as part of CFI flash probe. > > HyperRAM is not supported at the moment. > > HyperBus specification can be found at[1] > HyperFlash datasheet can be found at[2] > > [1] https://www.cypress.com/file/213356/download > [2] https://www.cypress.com/file/213346/download > [3] http://www.ti.com/lit/ug/spruid7b/spruid7b.pdf > Table 12-5741. HyperFlash Access Sequence > > Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> [...] > diff --git a/drivers/mtd/hyperbus/hyperbus-core.c b/drivers/mtd/hyperbus/hyperbus-core.c > new file mode 100644 > index 000000000000..63a9e64895bc > --- /dev/null > +++ b/drivers/mtd/hyperbus/hyperbus-core.c > @@ -0,0 +1,154 @@ [...] > +int hyperbus_register_device(struct hyperbus_device *hbdev) > +{ [...] > + map->name = dev_name(dev); > + map->bankwidth = 2; I think this should really be 1, judging on the comment to that field (and on Cogent's own RPC-IF HF driver). > + map->device_node = np; [...] MBR, Sergei
On 12/07/19 12:56 AM, Sergei Shtylyov wrote: > Hello! > > On 06/25/2019 10:57 AM, Vignesh Raghavendra wrote: > >> Cypress' HyperBus is Low Signal Count, High Performance Double Data Rate >> Bus interface between a host system master and one or more slave >> interfaces. HyperBus is used to connect microprocessor, microcontroller, >> or ASIC devices with random access NOR flash memory (called HyperFlash) >> or self refresh DRAM (called HyperRAM). >> >> Its a 8-bit data bus (DQ[7:0]) with Read-Write Data Strobe (RWDS) >> signal and either Single-ended clock(3.0V parts) or Differential clock >> (1.8V parts). It uses ChipSelect lines to select b/w multiple slaves. >> At bus level, it follows a separate protocol described in HyperBus >> specification[1]. >> >> HyperFlash follows CFI AMD/Fujitsu Extended Command Set (0x0002) similar >> to that of existing parallel NORs. Since HyperBus is x8 DDR bus, >> its equivalent to x16 parallel NOR flash with respect to bits per clock >> cycle. But HyperBus operates at >166MHz frequencies. >> HyperRAM provides direct random read/write access to flash memory >> array. >> >> But, HyperBus memory controllers seem to abstract implementation details >> and expose a simple MMIO interface to access connected flash. >> >> Add support for registering HyperFlash devices with MTD framework. MTD >> maps framework along with CFI chip support framework are used to support >> communicating with flash. >> >> Framework is modelled along the lines of spi-nor framework. HyperBus >> memory controller (HBMC) drivers calls hyperbus_register_device() to >> register a single HyperFlash device. HyperFlash core parses MMIO access >> information from DT, sets up the map_info struct, probes CFI flash and >> registers it with MTD framework. >> >> Some HBMC masters need calibration/training sequence[3] to be carried >> out, in order for DLL inside the controller to lock, by reading a known >> string/pattern. This is done by repeatedly reading CFI Query >> Identification String. Calibration needs to be done before trying to detect >> flash as part of CFI flash probe. >> >> HyperRAM is not supported at the moment. >> >> HyperBus specification can be found at[1] >> HyperFlash datasheet can be found at[2] >> >> [1] https://www.cypress.com/file/213356/download >> [2] https://www.cypress.com/file/213346/download >> [3] http://www.ti.com/lit/ug/spruid7b/spruid7b.pdf >> Table 12-5741. HyperFlash Access Sequence >> >> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> > [...] > >> diff --git a/drivers/mtd/hyperbus/hyperbus-core.c b/drivers/mtd/hyperbus/hyperbus-core.c >> new file mode 100644 >> index 000000000000..63a9e64895bc >> --- /dev/null >> +++ b/drivers/mtd/hyperbus/hyperbus-core.c >> @@ -0,0 +1,154 @@ > [...] >> +int hyperbus_register_device(struct hyperbus_device *hbdev) >> +{ > [...] >> + map->name = dev_name(dev); >> + map->bankwidth = 2; > > I think this should really be 1, judging on the comment to that field (and on > Cogent's own RPC-IF HF driver). > I agree this setting is a bit confusing because DDR nature. What we have with HyperFlash in DDR mode is equivalent to 16bit flash on a 8bit bus and kind of equal to 2 bus cycles (in this case clock edges), therefore bandwidth would turn out to be 2. Otherwise cfi_build_cmd() would generate wrong addresses and simple map implmention of read/writes would use wrong accessors. Only way I see map->bankwidth = 1 working is if HF is used in SDR mode. So is Cogent's HF in SDR mode? I thought HyperFlash is DDR only but I may be wrong. >> + map->device_node = np; > > [...] > > MBR, Sergei >
Hi Vignesh, On Tue, Jun 25, 2019 at 10:00 AM Vignesh Raghavendra <vigneshr@ti.com> wrote: > Cypress' HyperBus is Low Signal Count, High Performance Double Data Rate > Bus interface between a host system master and one or more slave > interfaces. HyperBus is used to connect microprocessor, microcontroller, > or ASIC devices with random access NOR flash memory (called HyperFlash) > or self refresh DRAM (called HyperRAM). > > Its a 8-bit data bus (DQ[7:0]) with Read-Write Data Strobe (RWDS) > signal and either Single-ended clock(3.0V parts) or Differential clock > (1.8V parts). It uses ChipSelect lines to select b/w multiple slaves. > At bus level, it follows a separate protocol described in HyperBus > specification[1]. > > HyperFlash follows CFI AMD/Fujitsu Extended Command Set (0x0002) similar > to that of existing parallel NORs. Since HyperBus is x8 DDR bus, > its equivalent to x16 parallel NOR flash with respect to bits per clock > cycle. But HyperBus operates at >166MHz frequencies. > HyperRAM provides direct random read/write access to flash memory > array. > > But, HyperBus memory controllers seem to abstract implementation details > and expose a simple MMIO interface to access connected flash. > > Add support for registering HyperFlash devices with MTD framework. MTD > maps framework along with CFI chip support framework are used to support > communicating with flash. > > Framework is modelled along the lines of spi-nor framework. HyperBus > memory controller (HBMC) drivers calls hyperbus_register_device() to > register a single HyperFlash device. HyperFlash core parses MMIO access > information from DT, sets up the map_info struct, probes CFI flash and > registers it with MTD framework. > > Some HBMC masters need calibration/training sequence[3] to be carried > out, in order for DLL inside the controller to lock, by reading a known > string/pattern. This is done by repeatedly reading CFI Query > Identification String. Calibration needs to be done before trying to detect > flash as part of CFI flash probe. > > HyperRAM is not supported at the moment. Thanks for your patch, which is now commit dcc7d3446a0fa19b ("mtd: Add support for HyperBus memory devices") in v5.3. > HyperBus specification can be found at[1] > HyperFlash datasheet can be found at[2] > > [1] https://www.cypress.com/file/213356/download > [2] https://www.cypress.com/file/213346/download > [3] http://www.ti.com/lit/ug/spruid7b/spruid7b.pdf > Table 12-5741. HyperFlash Access Sequence The last link no longer works. Do you have a replacement? Thanks! Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert, On 6/27/2022 8:58 PM, Geert Uytterhoeven wrote: > Hi Vignesh, > > On Tue, Jun 25, 2019 at 10:00 AM Vignesh Raghavendra <vigneshr@ti.com> wrote: >> Cypress' HyperBus is Low Signal Count, High Performance Double Data Rate >> Bus interface between a host system master and one or more slave >> interfaces. HyperBus is used to connect microprocessor, microcontroller, >> or ASIC devices with random access NOR flash memory (called HyperFlash) >> or self refresh DRAM (called HyperRAM). >> >> Its a 8-bit data bus (DQ[7:0]) with Read-Write Data Strobe (RWDS) >> signal and either Single-ended clock(3.0V parts) or Differential clock >> (1.8V parts). It uses ChipSelect lines to select b/w multiple slaves. >> At bus level, it follows a separate protocol described in HyperBus >> specification[1]. >> >> HyperFlash follows CFI AMD/Fujitsu Extended Command Set (0x0002) similar >> to that of existing parallel NORs. Since HyperBus is x8 DDR bus, >> its equivalent to x16 parallel NOR flash with respect to bits per clock >> cycle. But HyperBus operates at >166MHz frequencies. >> HyperRAM provides direct random read/write access to flash memory >> array. >> >> But, HyperBus memory controllers seem to abstract implementation details >> and expose a simple MMIO interface to access connected flash. >> >> Add support for registering HyperFlash devices with MTD framework. MTD >> maps framework along with CFI chip support framework are used to support >> communicating with flash. >> >> Framework is modelled along the lines of spi-nor framework. HyperBus >> memory controller (HBMC) drivers calls hyperbus_register_device() to >> register a single HyperFlash device. HyperFlash core parses MMIO access >> information from DT, sets up the map_info struct, probes CFI flash and >> registers it with MTD framework. >> >> Some HBMC masters need calibration/training sequence[3] to be carried >> out, in order for DLL inside the controller to lock, by reading a known >> string/pattern. This is done by repeatedly reading CFI Query >> Identification String. Calibration needs to be done before trying to detect >> flash as part of CFI flash probe. >> >> HyperRAM is not supported at the moment. > > Thanks for your patch, which is now commit dcc7d3446a0fa19b ("mtd: > Add support for HyperBus memory devices") in v5.3. > >> HyperBus specification can be found at[1] >> HyperFlash datasheet can be found at[2] >> >> [1] https://www.cypress.com/file/213356/download >> [2] https://www.cypress.com/file/213346/download >> [3] http://www.ti.com/lit/ug/spruid7b/spruid7b.pdf >> Table 12-5741. HyperFlash Access Sequence > > The last link no longer works. Do you have a replacement? Looks like I used a link point to specific version instead of top level redirector link. Please use: https://www.ti.com/lit/pdf/spruid7 Regards Vignesh
Hi Vignesh, On Sat, Jul 2, 2022 at 7:10 PM Raghavendra, Vignesh <vigneshr@ti.com> wrote: > On 6/27/2022 8:58 PM, Geert Uytterhoeven wrote: > > On Tue, Jun 25, 2019 at 10:00 AM Vignesh Raghavendra <vigneshr@ti.com> wrote: > >> Cypress' HyperBus is Low Signal Count, High Performance Double Data Rate > >> Bus interface between a host system master and one or more slave > >> interfaces. HyperBus is used to connect microprocessor, microcontroller, > >> or ASIC devices with random access NOR flash memory (called HyperFlash) > >> or self refresh DRAM (called HyperRAM). > >> > >> Its a 8-bit data bus (DQ[7:0]) with Read-Write Data Strobe (RWDS) > >> signal and either Single-ended clock(3.0V parts) or Differential clock > >> (1.8V parts). It uses ChipSelect lines to select b/w multiple slaves. > >> At bus level, it follows a separate protocol described in HyperBus > >> specification[1]. > >> > >> HyperFlash follows CFI AMD/Fujitsu Extended Command Set (0x0002) similar > >> to that of existing parallel NORs. Since HyperBus is x8 DDR bus, > >> its equivalent to x16 parallel NOR flash with respect to bits per clock > >> cycle. But HyperBus operates at >166MHz frequencies. > >> HyperRAM provides direct random read/write access to flash memory > >> array. > >> > >> But, HyperBus memory controllers seem to abstract implementation details > >> and expose a simple MMIO interface to access connected flash. > >> > >> Add support for registering HyperFlash devices with MTD framework. MTD > >> maps framework along with CFI chip support framework are used to support > >> communicating with flash. > >> > >> Framework is modelled along the lines of spi-nor framework. HyperBus > >> memory controller (HBMC) drivers calls hyperbus_register_device() to > >> register a single HyperFlash device. HyperFlash core parses MMIO access > >> information from DT, sets up the map_info struct, probes CFI flash and > >> registers it with MTD framework. > >> > >> Some HBMC masters need calibration/training sequence[3] to be carried > >> out, in order for DLL inside the controller to lock, by reading a known > >> string/pattern. This is done by repeatedly reading CFI Query > >> Identification String. Calibration needs to be done before trying to detect > >> flash as part of CFI flash probe. > >> > >> HyperRAM is not supported at the moment. > > > > Thanks for your patch, which is now commit dcc7d3446a0fa19b ("mtd: > > Add support for HyperBus memory devices") in v5.3. > > > >> HyperBus specification can be found at[1] > >> HyperFlash datasheet can be found at[2] > >> > >> [1] https://www.cypress.com/file/213356/download > >> [2] https://www.cypress.com/file/213346/download > >> [3] http://www.ti.com/lit/ug/spruid7b/spruid7b.pdf > >> Table 12-5741. HyperFlash Access Sequence > > > > The last link no longer works. Do you have a replacement? > > Looks like I used a link point to specific version instead of top level > redirector link. Please use: > > https://www.ti.com/lit/pdf/spruid7 Thank you, that link works for me. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds