From: b32955@freescale•com (Huang Shijie)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH 0/4] mtd: spi-nor: add a new framework for SPI NOR
Date: Mon, 2 Dec 2013 18:06:55 +0800 [thread overview]
Message-ID: <529C5BBF.6000604@freescale.com> (raw)
In-Reply-To: <5298AA23.7080404@st.com>
Hi Angus:
thanks for the long suggestion. it's very useful.
Let's push the spi nor framework.:)
> Hi Huang Shijie,
>
> On 11/27/2013 04:32 AM, Brian Norris wrote:
>> + Lee Jones, others
>>
>> I'd like to keep a similar CC list for the various conversations going
>> on about SPI NOR frameworks.
>>
>> On Tue, Nov 26, 2013 at 02:32:51PM +0800, Huang Shijie wrote:
>>> 1.) Why add a new framework for SPI NOR?
>>> The SPI-NOR controller such as Freescale's Quadspi controller is working
>>> in a different way from the SPI bus. It should knows the NOR commands to
>>> find the right LUT sequence. Unfortunately, the current code can not meet
>>> this requirement.
> I fully support your argument for introducing a new framework to support Serial
> Flash controllers. Reiterating my previous comment:
>
> "The kernel offers many examples of others who have struggled with the same
> problem. Some have chosen to write self-contained drivers (e.g.
> drivers/mtd/devices/spear_smi.c and drivers/mtd/devices/sst251.c) duplicating
> much of m25p80.c, while others have attempted to squeeze into the SPI framework
> (e.g. drivers/spi/spi-tegra20-sflash.c and drivers/spi/spi-falcon.c), relying on
> fragile heuristics to recreate the semantics of m25p80 that is lost over the ,
> current circumstances SPI framework ."
>
> However, having now been through your proposed framework, it would not seem to
> provide a generally applicable solution. Indeed, other than making the Serial
> Flash commands and the device table available, it is not obvious how it improves
> the situation for your own specific controller either; the
> read/write/read_reg/write_reg callbacks do not offer a great deal more than
> spi_read_then_write(). (Perhaps all will become clear when you submit the
> fsl-quadspi driver.)
> As I see it, the main problem with the current support is that dedicated Serial
> Flash Controllers require a level of semantics that cannot be conveyed over the
> generic SPI framework. With this in mind, I would start by defining something
> along the lines of a "Serial Flash transfer". Some initial ramblings of my mind:
>
> /**
> * struct spi_nor_xfer_cfg - Structure for defining a Serial Flash transfer
> * @wren: command for "Write Enable", or 0x00 for not required
why this wren is needed?
> * @cmd: command for operation
> * @cmd_pins: number of pins to send @cmd (1, 2, 4)
> * @addr: address for operation
> * @addr_pins: number of pins to send @addr (1, 2, 4)
> * @addr_width: number of address bytes (3,4, or 0 for address not required)
this field has been in the spi_nor{} , it should be a fixed value.
so i think we do not need this argument.
> * @mode: mode data
> * @mode_pins: number of pins to send @mode (1, 2, 4)
> * @mode_cycles: number of mode cycles (0 for mode not required)
> * @dummy_cycles: number of dummy cycles (0 for dummy not required)
> */
> struct spi_nor_xfer_cfg {
> uint8_t wren;
> uint8_t cmd;
> int cmd_pins;
> uint32_t addr;
> int addr_pins;
> int addr_width;
> uint32_t mode;
> int mode_pins;
> int mode_cycles;
> int dummy_cycles;
> };
>
> We could then build up layers of functionality, based on two fundamental primitives:
>
> int (*read_xfer)(struct spi_nor_info *info,
> struct spi_nor_xfer_cfg *cfg,
> uint8_t *buf, size_t len);
>
> int (*write_xfer)(struct spi_nor_info *info,
> struct spi_nor_xfer_cfg *cfg,
> uint8_t *buf, size_t len);
>
> Default implementations for standard operations could follow:
>
> int (*read_reg)(struct spi_nor_info *info,
> uint8_t cmd, uint8_t *reg, int len);
> int (*write_reg)(struct spi_nor_info *info,
> uint8_t cmd, uint8_t *reg, int len,
> int wren, int wtr);
>
> int (*readid)(struct spi_nor_info *info, uint8_t *readid);
> int (*wait_till_ready)(struct spi_nor_info *info);
currently, we poll the status register.
why this hook is needed?
>
> int (*read)(struct spi_nor_info *info, uint8_t buf, off_t offs, size_t len);
> int (*write)(struct spi_nor_info *info, off_t offs, size_t len);
> int (*erase_sector)(struct spi_nor_info *info, off_t offs);
I also confused at this hook.
if we need erase_sector, do we still need the erase_chip()?
> Each driver would be required to implement read_xfer() and write_xfer(), and
> then free to either use the default implementations, or override with
> driver-optimised implementations.
>
> In the case of a pure SPI Controller, read_xfer() and write_xfer() would simply
> flatten to spi_messages. The key benefit is that dedicated Serial Flash
> Controllers would be able to take advantage of the richer semantics offered by
> the 'spi_nor_xfer_cfg' data.
>
> I would also expect the spi-nor framework to provide the following services,
> applicable to all Serial Flash drivers:
>
> - determine devices properties at runtime, based on the READID data (and
> perhaps SFDP, if and when it matures!). This should include supported opcodes
> (e.g. READ_1_1_4, READ_1_4_4, READ4B_1_1_4...), operating frequencies, mode and
> dummy requirements, means of setting Quad Enable, entering/exiting 4-byte
> addressing etc.
>
> - provide optimal read, write and erase configurations, based on the combined
> capabilities of the Serial Flash device and the H/W Controller.
>
> - device specific configuration (e.g. setting QE, unlock BPx bits, DYB unlocking).
>
Does Jones have any opinion about this?
thanks
Huang Shijie
next prev parent reply other threads:[~2013-12-02 10:06 UTC|newest]
Thread overview: 64+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-26 6:32 [PATCH 0/4] mtd: spi-nor: add a new framework for SPI NOR Huang Shijie
2013-11-26 6:32 ` [PATCH 1/4] mtd: spi-nor: move the SPI NOR commands to a new header file Huang Shijie
2013-11-26 7:42 ` Gupta, Pekon
2013-11-26 8:53 ` Huang Shijie
2013-11-26 14:48 ` Angus Clark
2013-11-26 6:32 ` [PATCH 2/4] mtd: spi-nor: add a new data structrue spi_nor{} Huang Shijie
2013-11-26 11:42 ` Gupta, Pekon
2013-11-27 4:35 ` Huang Shijie
2013-11-27 9:32 ` Marek Vasut
2013-11-27 10:24 ` Huang Shijie
2013-11-27 10:27 ` Marek Vasut
2013-11-26 6:32 ` [PATCH 3/4] mtd: spi-nor: add the framework for SPI NOR Huang Shijie
2013-11-26 10:03 ` Gupta, Pekon
2013-11-27 9:39 ` Marek Vasut
2013-11-26 6:32 ` [PATCH 4/4] mtd: m25p80: use the new spi-nor APIs Huang Shijie
2013-11-26 12:57 ` [PATCH 0/4] mtd: spi-nor: add a new framework for SPI NOR Angus Clark
2013-11-27 4:32 ` Brian Norris
2013-11-27 4:39 ` Huang Shijie
2013-11-29 14:52 ` Angus Clark
2013-12-02 10:06 ` Huang Shijie [this message]
2013-12-02 11:01 ` Gupta, Pekon
2013-12-02 11:19 ` Angus Clark
2013-12-03 6:20 ` Huang Shijie
2013-12-03 8:23 ` Lee Jones
2013-12-10 8:25 ` Brian Norris
2013-12-10 10:00 ` Lee Jones
2013-12-03 0:32 ` Marek Vasut
2013-12-03 10:36 ` Huang Shijie
2013-12-03 14:51 ` David Woodhouse
2013-12-04 18:44 ` Brian Norris
2013-12-05 7:12 ` Huang Shijie
2013-12-05 8:11 ` Brian Norris
2013-12-05 7:59 ` Huang Shijie
2013-12-05 9:20 ` Brian Norris
2013-12-06 3:07 ` Huang Shijie
2013-12-05 14:35 ` Angus Clark
2013-12-06 8:18 ` Huang Shijie
2013-12-10 9:08 ` Brian Norris
2013-12-04 2:46 ` Huang Shijie
2013-12-04 6:58 ` Angus Clark
2013-12-04 7:19 ` Gupta, Pekon
2013-12-04 8:21 ` Angus Clark
2013-12-04 15:36 ` Marek Vasut
2013-12-05 2:42 ` Huang Shijie
2013-12-05 5:43 ` Gupta, Pekon
2013-12-05 7:33 ` Huang Shijie
2013-11-27 9:27 ` Marek Vasut
2013-11-27 9:47 ` Sourav Poddar
2013-11-27 10:06 ` Marek Vasut
2013-11-27 10:56 ` Sourav Poddar
2013-12-02 23:59 ` Marek Vasut
2013-12-03 10:01 ` Sourav Poddar
2013-12-03 13:42 ` Marek Vasut
2013-12-03 13:50 ` Sourav Poddar
2013-12-03 14:19 ` Marek Vasut
2013-12-03 14:31 ` Sourav Poddar
2013-12-03 15:09 ` Marek Vasut
2013-12-03 15:16 ` Sourav Poddar
2013-12-03 15:35 ` Marek Vasut
2013-12-03 15:23 ` David Woodhouse
2013-12-03 18:28 ` Brian Norris
2013-12-03 23:41 ` Marek Vasut
2013-11-27 10:19 ` Huang Shijie
2013-12-03 0:00 ` Marek Vasut
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=529C5BBF.6000604@freescale.com \
--to=b32955@freescale$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox