lkml.org 
[lkml]   [2017]   [Apr]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2 2/2] mtd: spi-nor: add driver for STM32 quad spi flash controller
From
Date
hi Marek

tomorrow, I send a v3 with your/Rob reviews.

BR

Ludo


On 04/10/2017 06:15 PM, Marek Vasut wrote:
> On 04/10/2017 11:08 AM, Ludovic BARRE wrote:
>> On 04/07/2017 01:55 AM, Marek Vasut wrote:
>>> On 03/31/2017 07:02 PM, Ludovic Barre wrote:
>>>> From: Ludovic Barre <ludovic.barre@st.com>
>>>>
>>>> The quadspi is a specialized communication interface targeting single,
>>>> dual or quad SPI Flash memories.
>>>>
>>>> It can operate in any of the following modes:
>>>> -indirect mode: all the operations are performed using the quadspi
>>>> registers
>>>> -read memory-mapped mode: the external Flash memory is mapped to the
>>>> microcontroller address space and is seen by the system as if it was
>>>> an internal memory
>>>>
>>>> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
>>>> ---
>>>> drivers/mtd/spi-nor/Kconfig | 7 +
>>>> drivers/mtd/spi-nor/Makefile | 1 +
>>>> drivers/mtd/spi-nor/stm32-quadspi.c | 690
>>>> ++++++++++++++++++++++++++++++++++++
>>>> 3 files changed, 698 insertions(+)
>>>> create mode 100644 drivers/mtd/spi-nor/stm32-quadspi.c
>>>>
>>> [...]
>>>
>>>> +struct stm32_qspi_flash {
>>>> + struct spi_nor nor;
>>>> + u32 cs;
>>>> + u32 fsize;
>>>> + u32 presc;
>>>> + struct stm32_qspi *qspi;
>>>> +};
>>> [...]
>>>
>>>> +struct stm32_qspi_cmd {
>>>> + struct {
>>>> + u8 addr_width;
>>>> + u8 dummy;
>>>> + u8 data;
>>>> + } conf;
>>> Is there any benefit in having this structure here or could you just
>>> make the struct stm32_qspi_cmd flat ?
>> no benefit, it was just to regroup, so I can do a flat structure
> Well, as you like, but I think it does make sense to just make it flat.
>
>>>> + u8 opcode;
>>>> + u32 framemode;
>>>> + u32 qspimode;
>>>> + u32 addr;
>>>> + size_t len;
>>>> + void *buf;
>>>> +};
>>> [...]
>>>
>>>> +static ssize_t stm32_qspi_read(struct spi_nor *nor, loff_t from,
>>>> size_t len,
>>>> + u_char *buf)
>>>> +{
>>>> + struct stm32_qspi_flash *flash = nor->priv;
>>>> + struct stm32_qspi *qspi = flash->qspi;
>>>> + struct stm32_qspi_cmd cmd;
>>>> + int err;
>>>> +
>>>> + dev_dbg(qspi->dev, "read(%#.2x): buf:%p from:%#.8x len:%#x\n",
>>>> + nor->read_opcode, buf, (u32)from, len);
>>>> +
>>>> + memset(&cmd, 0, sizeof(cmd));
>>>> + cmd.opcode = nor->read_opcode;
>>>> + cmd.conf.addr_width = nor->addr_width;
>>>> + cmd.addr = (u32)from;
>>> loff_t (from) can be 64bit ... how do we handle this ?
>> I'm surprise by the question,
>> the SPI NOR device uses 3 Bytes or 4 bytes address mode.
>> So, the stm32 qspi controller has a 32 bit register for NOR address.
>> On the other hand the framework and other drivers used this variable
>> (from) like
>> a 32 bits.
> Hmmm, (rhetorical question) then why do we even use loff_t in the
> framework ?
>
> Anyway, this is no problem then.
In fact, the loff_t 64 bit come from mtd interface
(needed to address biggest device constraint) but not needed for spi-nor
devices.
>>>> + cmd.conf.data = 1;
>>>> + cmd.conf.dummy = nor->read_dummy;
>>>> + cmd.len = len;
>>>> + cmd.buf = buf;
>>>> + cmd.qspimode = qspi->read_mode;
>>>> +
>>>> + stm32_qspi_set_framemode(nor, &cmd, true);
>>>> + err = stm32_qspi_send(flash, &cmd);
>>>> +
>>>> + return err ? err : len;
>>>> +}
>>> [...]
>>>
>>>> +static irqreturn_t stm32_qspi_irq(int irq, void *dev_id)
>>>> +{
>>>> + struct stm32_qspi *qspi = (struct stm32_qspi *)dev_id;
>>>> + u32 cr, sr, fcr = 0;
>>>> +
>>>> + cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
>>>> + sr = readl_relaxed(qspi->io_base + QUADSPI_SR);
>>>> +
>>>> + if ((cr & CR_TCIE) && (sr & SR_TCF)) {
>>>> + /* tx complete */
>>>> + fcr |= FCR_CTCF;
>>>> + complete(&qspi->cmd_completion);
>>>> + } else {
>>>> + dev_info(qspi->dev, "spurious interrupt\n");
>>> You probably want to ratelimit this one ...
>> yes it's better if there is an issue.
> Yep
>
>>>> + }
>>>> +
>>>> + writel_relaxed(fcr, qspi->io_base + QUADSPI_FCR);
>>>> +
>>>> + return IRQ_HANDLED;
>>>> +}
>>>> +
>>>> +static int stm32_qspi_prep(struct spi_nor *nor, enum spi_nor_ops ops)
>>>> +{
>>>> + struct stm32_qspi_flash *flash = nor->priv;
>>>> + struct stm32_qspi *qspi = flash->qspi;
>>>> +
>>>> + mutex_lock(&qspi->lock);
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static void stm32_qspi_unprep(struct spi_nor *nor, enum spi_nor_ops
>>>> ops)
>>>> +{
>>>> + struct stm32_qspi_flash *flash = nor->priv;
>>>> + struct stm32_qspi *qspi = flash->qspi;
>>>> +
>>>> + mutex_unlock(&qspi->lock);
>>>> +}
>>>> +
>>>> +static int stm32_qspi_flash_setup(struct stm32_qspi *qspi,
>>>> + struct device_node *np)
>>>> +{
>>>> + u32 width, flash_read, presc, cs_num, max_rate = 0;
>>>> + struct stm32_qspi_flash *flash;
>>>> + struct mtd_info *mtd;
>>>> + int ret;
>>>> +
>>>> + of_property_read_u32(np, "reg", &cs_num);
>>>> + if (cs_num >= STM32_MAX_NORCHIP)
>>>> + return -EINVAL;
>>>> +
>>>> + of_property_read_u32(np, "spi-max-frequency", &max_rate);
>>>> + if (!max_rate)
>>>> + return -EINVAL;
>>>> +
>>>> + presc = DIV_ROUND_UP(qspi->clk_rate, max_rate) - 1;
>>>> +
>>>> + if (of_property_read_u32(np, "spi-rx-bus-width", &width))
>>>> + width = 1;
>>>> +
>>>> + if (width == 4)
>>>> + flash_read = SPI_NOR_QUAD;
>>>> + else if (width == 2)
>>>> + flash_read = SPI_NOR_DUAL;
>>>> + else if (width == 1)
>>>> + flash_read = SPI_NOR_NORMAL;
>>>> + else
>>>> + return -EINVAL;
>>>> +
>>>> + flash = &qspi->flash[cs_num];
>>>> + flash->qspi = qspi;
>>>> + flash->cs = cs_num;
>>>> + flash->presc = presc;
>>>> +
>>>> + flash->nor.dev = qspi->dev;
>>>> + spi_nor_set_flash_node(&flash->nor, np);
>>>> + flash->nor.priv = flash;
>>>> + mtd = &flash->nor.mtd;
>>>> + mtd->priv = &flash->nor;
>>>> +
>>>> + flash->nor.read = stm32_qspi_read;
>>>> + flash->nor.write = stm32_qspi_write;
>>>> + flash->nor.erase = stm32_qspi_erase;
>>>> + flash->nor.read_reg = stm32_qspi_read_reg;
>>>> + flash->nor.write_reg = stm32_qspi_write_reg;
>>>> + flash->nor.prepare = stm32_qspi_prep;
>>>> + flash->nor.unprepare = stm32_qspi_unprep;
>>>> +
>>>> + writel_relaxed(LPTR_DFT_TIMEOUT, qspi->io_base + QUADSPI_LPTR);
>>>> +
>>>> + writel_relaxed(CR_PRESC(presc) | CR_FTHRES(3) | CR_TCEN | CR_SSHIFT
>>>> + | CR_EN, qspi->io_base + QUADSPI_CR);
>>>> +
>>>> + /*
>>>> + * in stm32 qspi controller, QUADSPI_DCR register has a fsize field
>>>> + * which define the size of nor flash.
>>>> + * if fsize is NULL, the controller can't sent spi-nor command.
>>>> + * set a temporary value just to discover the nor flash with
>>>> + * "spi_nor_scan". After, the right value (mtd->size) can be set.
>>>> + */
>>> Is 25 the smallest value ? Use a macro for this ...
>> 25 is an arbitrary choice, I will define a smallest value
> Cool, thanks!
>

\
 
 \ /
  Last update: 2017-04-10 18:57    [W:1.480 / U:0.028 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site