Hello! On 06/11/2019 02:57 PM, 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 wrt 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@xxxxxx> >> [...] >>> diff --git a/drivers/mtd/hyperbus/hyperbus-core.c b/drivers/mtd/hyperbus/hyperbus-core.c >>> new file mode 100644 >>> index 000000000000..df1f75e10b1a >>> --- /dev/null >>> +++ b/drivers/mtd/hyperbus/hyperbus-core.c >>> @@ -0,0 +1,191 @@ >>> +// SPDX-License-Identifier: GPL-2.0 >>> +// >>> +// Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/ >>> +// Author: Vignesh Raghavendra <vigneshr@xxxxxx> >>> + >>> +#include <linux/err.h> >>> +#include <linux/kernel.h> >>> +#include <linux/module.h> >>> +#include <linux/mtd/hyperbus.h> >>> +#include <linux/mtd/map.h> >>> +#include <linux/mtd/mtd.h> >>> +#include <linux/mtd/cfi.h> >>> +#include <linux/of.h> >>> +#include <linux/of_address.h> >>> +#include <linux/types.h> >>> + >>> +#define HYPERBUS_CALIB_COUNT 25 >> >> Mhm, I think I've already protested about this being #define'd here... > > I thought you had agreed that default optional calibration routine can > be part of core code and thus this #define. > > Anyways, what is your preference here? Drop the constant and use a local > variable in hyperbus_calibrate()? > Or are you suggesting to move hyperbus_calibrate() TI's specific driver? I'm just not comfortable with the common HF code using quite an arbitrary constant... >> [...] >>> diff --git a/include/linux/mtd/hyperbus.h b/include/linux/mtd/hyperbus.h >>> new file mode 100644 >>> index 000000000000..ee2eefd822c9 >>> --- /dev/null >>> +++ b/include/linux/mtd/hyperbus.h >>> @@ -0,0 +1,91 @@ [...] >>> + * @mtd: pointer to MTD struct >>> + * @ctlr: pointer to HyperBus controller struct >>> + * @memtype: type of memory device: HyperFlash or HyperRAM >>> + * @registered: flag to indicate whether device is registered with MTD core >>> + */ >>> + >>> +struct hyperbus_device { >>> + struct map_info map; >>> + struct device_node *np; >>> + struct mtd_info *mtd; >>> + struct hyperbus_ctlr *ctlr; >>> + enum hyperbus_memtype memtype; >>> + bool registered; >>> +}; >>> + >>> +/** >>> + * struct hyperbus_ops - struct representing custom HyperBus operations >>> + * @read16: read 16 bit of data, usually from register/ID-CFI space >>> + * @write16: write 16 bit of data, usually to register/ID-CFI space >> >> Usually? How to differ the register/memory transfers if both are possible? > CFI + map framework does not provide a way to differentiate b/w reg > access vs memory access. read16()/write16() is used to either access > registers or for sending various cmds like lock/unlock etc or for > programming a single word. > For regular read/writes copy_from() and copy_to() are used. In my case only copy_from() would exist -- no proper acceleration for writes... > Looking at HyperBus protocol, controllers would not need to > differentiate b/w registers vs memory transfers for HyperFlash devices. > So, I think I can drop read16/write16 and redirect these calls to > copy_from()/copy_to() Doubt it, frankly speaking. > I mainly added these functions keeping HyperRAM in mind. Idea was > drivers would look at hyperbus_device->memtype and set to register > access mode for HyperRAM in case of write16()/read16(). Looks like the > interface is not intuitive enough > So, will drop these and add it back when adding HyperRAM support. > > Does that work for your HW as well? Don't think so... However, my HyperFlash driver could make use of the following #define's in the HyperBus header: #define HF_CMD_CA47 BIT(7) /* Read */ #define HF_CMD_CA46 BIT(6) /* Register space */ #define HF_CMD_CA45 BIT(5) /* Linear burst */ #define HF_CMD_READ_REG (HF_CMD_CA47 | HF_CMD_CA46) #define HF_CMD_READ_MEM HF_CMD_CA47 #define HF_CMD_WRITE_REG HF_CMD_CA46 #define HF_CMD_WRITE_MEM 0 MBR, Sergei
