| Message ID | 1480906577-38455-3-git-send-email-shawn.lin@rock-chips.com |
|---|---|
| State | Superseded |
| Headers | show |
On 12/05/2016 03:56 AM, Shawn Lin wrote: > Add rockchip serial flash controller driver > > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> > Looks good, a few nits below. [...] > +static int get_if_type(struct rockchip_sfc *sfc, enum read_mode flash_read) > +{ > + enum rockchip_sfc_iftype if_type; Wouldn't it be shorter if you used if-return below ? Example if (flash_read == SPI_NOR_QUAD) return IF_TYPE_QUAD; if (flash_read == SPI_NOR_DUAL) return IF_TYPE_DUAL; ... dev_err(sfc->dev, "unsupported SPI read mode\n"); return -EINVAL; > + switch (flash_read) { > + case SPI_NOR_DUAL: > + if_type = IF_TYPE_DUAL; > + break; > + case SPI_NOR_QUAD: > + if_type = IF_TYPE_QUAD; > + break; > + case SPI_NOR_NORMAL: > + case SPI_NOR_FAST: > + if_type = IF_TYPE_STD; > + break; > + default: > + dev_err(sfc->dev, "unsupported SPI read mode\n"); > + return -EINVAL; > + } > + > + return if_type; > +} [...] > +static inline void rockchip_sfc_setup_transfer(struct spi_nor *nor, > + loff_t from_to, > + size_t len, u8 op_type) > +{ > + struct rockchip_sfc_chip_priv *priv = nor->priv; > + struct rockchip_sfc *sfc = priv->sfc; > + u32 reg; > + u8 if_type = 0; > + > + if_type = get_if_type(sfc, nor->flash_read); > + writel_relaxed((if_type << SFC_CTRL_DATA_BITS_SHIFT) | > + (if_type << SFC_CTRL_ADDR_BITS_SHIFT) | > + (if_type << SFC_CTRL_CMD_BITS_SHIFT) | Hm, looking at this, does the controller only support n-n-n mode (1-1-1, 2-2-2, 4-4-4) ? Or why don't you allow 1-1-n/1-n-n/2-n-n ? I would like to hear some input from Cyrille on this one. > + (sfc->negative_edge ? SFC_CTRL_PHASE_SEL_NEGETIVE : 0), > + sfc->regbase + SFC_CTRL); > + > + if (op_type == SFC_CMD_DIR_WR) > + reg = nor->program_opcode << SFC_CMD_IDX_SHIFT; > + else > + reg = nor->read_opcode << SFC_CMD_IDX_SHIFT; > + > + reg |= op_type << SFC_CMD_DIR_SHIFT; > + reg |= (nor->addr_width == 4) ? > + SFC_CMD_ADDR_32BITS : SFC_CMD_ADDR_24BITS; > + > + reg |= priv->cs << SFC_CMD_CS_SHIFT; > + reg |= len << SFC_CMD_TRAN_BYTES_SHIFT; > + > + if (op_type == SFC_CMD_DIR_RD) > + reg |= SFC_CMD_DUMMY(nor->read_dummy); > + > + /* Should minus one as 0x0 means 1 bit flash address */ > + writel_relaxed(nor->addr_width * 8 - 1, sfc->regbase + SFC_ABIT); > + writel_relaxed(reg, sfc->regbase + SFC_CMD); > + writel_relaxed(from_to, sfc->regbase + SFC_ADDR); > +} [...] > +static int rockchip_sfc_dma_transfer(struct spi_nor *nor, loff_t from_to, > + size_t len, u_char *buf, u8 op_type) > +{ > + struct rockchip_sfc_chip_priv *priv = nor->priv; > + struct rockchip_sfc *sfc = priv->sfc; > + size_t offset; > + int ret; > + dma_addr_t dma_addr = 0; Nit, you can precalculate the DMA_TO/FROM_DEVICE and store it to variable here, ie. dma_dir = (op_type == SFC_CMD_DIR_RD) ? DMA_FROM_DEVICE : DMA_TO_DEVICE. > + for (offset = 0; offset < len; offset += SFC_DMA_MAX_LEN) { > + size_t trans = min_t(size_t, SFC_DMA_MAX_LEN, len - offset); > + > + if (SFC_CMD_DIR_RD) if (op_type == is missing, but you can drop this, see above. > + dma_addr = dma_map_single(NULL, (void *)buf, > + trans, DMA_FROM_DEVICE); > + else > + dma_addr = dma_map_single(NULL, (void *)buf, > + trans, DMA_TO_DEVICE); You can use dma_dir here ^ and drop the condition. > + if (dma_mapping_error(sfc->dev, dma_addr)) { > + /* > + * If we use pre-allocated dma_buffer, we need to > + * do a copy here. > + */ > + if (op_type == SFC_CMD_DIR_WR) > + memcpy(sfc->buffer, buf + offset, trans); > + > + dma_addr = 0; > + } > + > + if (op_type == SFC_CMD_DIR_WR) > + /* > + * Flush the write data from write_buf to dma_addr > + * if using dynamic allocated dma buffer before dma > + * moves data from dma_addr to fifo. > + */ > + dma_sync_single_for_device(sfc->dev, dma_addr, > + trans, DMA_TO_DEVICE); > + > + > + /* If failing to map dma, use pre-allocated area instead */ > + ret = rockchip_sfc_do_dma_transfer(nor, from_to + offset, > + dma_addr ? dma_addr : > + sfc->dma_buffer, > + trans, op_type); > + > + if (dma_addr) { > + /* > + * Invalidate the read data from dma_addr if using > + * dynamic allocated dma buffer after dma moves data > + * from fifo to dma_addr. > + */ > + if (op_type == SFC_CMD_DIR_RD) { > + dma_sync_single_for_cpu(sfc->dev, dma_addr, > + trans, DMA_FROM_DEVICE); > + dma_unmap_single(NULL, dma_addr, > + trans, DMA_FROM_DEVICE); > + } else { > + dma_unmap_single(NULL, dma_addr, > + trans, DMA_TO_DEVICE); Here as well and it'd be reduced to if (...) dma_sync_single...() dma_unmap( ... , dma_dir); > + } > + } > + > + if (ret) { > + dev_warn(nor->dev, "DMA read timeout\n"); > + return ret; > + } > + /* > + * If we use pre-allocated dma_buffer for read, we need to > + * do a copy here. > + */ > + if (!dma_addr && (op_type == SFC_CMD_DIR_RD)) > + memcpy(buf + offset, sfc->buffer, trans); > + } > + > + return len; > +} > + > +static ssize_t rockchip_sfc_do_rd_wr(struct spi_nor *nor, loff_t from_to, > + size_t len, u_char *buf, u32 op_type) > +{ > + struct rockchip_sfc_chip_priv *priv = nor->priv; > + struct rockchip_sfc *sfc = priv->sfc; > + int ret; > + > + if (!sfc->use_dma) > + goto no_dma; > + > + return rockchip_sfc_dma_transfer(nor, from_to, len, > + buf, op_type); if (likely(sfc->use_dma)) return rockchip_sfc_dma...(); /* Comment saying that we fall back to PIO */ ... pio code ... > +no_dma: > + ret = rockchip_sfc_pio_transfer(nor, from_to, len, > + (u_char *)buf, op_type); > + if (ret) { > + if (op_type == SFC_CMD_DIR_RD) > + dev_warn(nor->dev, "PIO read timeout\n"); > + else > + dev_warn(nor->dev, "PIO write timeout\n"); > + return ret; > + } > + > + return len; > +} [...] > +/** Drop this asterisk unless you document the driver in kerneldoc. > + * Get spi flash device information and register it as a mtd device. > + */ > +static int rockchip_sfc_register(struct device_node *np, > + struct rockchip_sfc *sfc) > +{ > + struct device *dev = sfc->dev; > + struct mtd_info *mtd; > + struct spi_nor *nor; > + int ret; > + > + nor = &(sfc->flash[sfc->num_chip].nor); Parenthesis not needed. > + nor->dev = dev; > + spi_nor_set_flash_node(nor, np); > + > + ret = of_property_read_u8(np, "reg", &sfc->flash[sfc->num_chip].cs); > + if (ret) { > + dev_err(dev, "No reg property for %s\n", > + np->full_name); > + return ret; > + } > + > + ret = of_property_read_u32(np, "spi-max-frequency", > + &sfc->flash[sfc->num_chip].clk_rate); > + if (ret) { > + dev_err(dev, "No spi-max-frequency property for %s\n", > + np->full_name); > + return ret; > + } > + > + sfc->flash[sfc->num_chip].sfc = sfc; > + nor->priv = &(sfc->flash[sfc->num_chip]); > + > + nor->prepare = rockchip_sfc_prep; > + nor->unprepare = rockchip_sfc_unprep; > + nor->read_reg = rockchip_sfc_read_reg; > + nor->write_reg = rockchip_sfc_write_reg; > + nor->read = rockchip_sfc_read; > + nor->write = rockchip_sfc_write; > + nor->erase = NULL; > + ret = spi_nor_scan(nor, NULL, SPI_NOR_QUAD); > + if (ret) > + return ret; > + > + mtd = &(nor->mtd); > + mtd->name = np->name; > + ret = mtd_device_register(mtd, NULL, 0); > + if (ret) > + return ret; > + > + sfc->num_chip++; > + return 0; > +} > + > +static void rockchip_sfc_unregister_all(struct rockchip_sfc *sfc) > +{ > + int i; > + > + for (i = 0; i < sfc->num_chip; i++) > + mtd_device_unregister(&(sfc->flash[i].nor.mtd)); Inner parenthesis not needed IMO > +} > + > +static int rockchip_sfc_register_all(struct rockchip_sfc *sfc) > +{ > + struct device *dev = sfc->dev; > + struct device_node *np; > + int ret; > + > + for_each_available_child_of_node(dev->of_node, np) { > + ret = rockchip_sfc_register(np, sfc); > + if (ret) > + goto fail; > + > + if (sfc->num_chip == SFC_MAX_CHIPSELECT_NUM) { > + dev_warn(dev, "Exceeds the max cs limitation\n"); > + break; > + } > + } > + > + return 0; > + > +fail: > + dev_err(dev, "Failed to register all chips\n"); > + /* Unregister all the _registered_ nor flash */ > + rockchip_sfc_unregister_all(sfc); > + return ret; > +} [...] > +#ifdef CONFIG_PM > +int rockchip_sfc_runtime_suspend(struct device *dev) > +{ > + struct rockchip_sfc *sfc = dev_get_drvdata(dev); > + > + clk_disable_unprepare(sfc->hclk); > + return 0; > +} Was the suspend ever really tested with this block ? Is disabling clock really enough ? > +int rockchip_sfc_runtime_resume(struct device *dev) > +{ > + struct rockchip_sfc *sfc = dev_get_drvdata(dev); > + > + clk_prepare_enable(sfc->hclk); > + return 0; > +} > +#endif /* CONFIG_PM */ [...]
On 2016/12/5 11:40, Marek Vasut wrote: > On 12/05/2016 03:56 AM, Shawn Lin wrote: >> Add rockchip serial flash controller driver >> >> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> >> > > Looks good, a few nits below. > > [...] > >> +static int get_if_type(struct rockchip_sfc *sfc, enum read_mode flash_read) >> +{ >> + enum rockchip_sfc_iftype if_type; > > Wouldn't it be shorter if you used if-return below ? > Example > > if (flash_read == SPI_NOR_QUAD) > return IF_TYPE_QUAD; > > if (flash_read == SPI_NOR_DUAL) > return IF_TYPE_DUAL; > ... ok, will improve. > > dev_err(sfc->dev, "unsupported SPI read mode\n"); > return -EINVAL; > >> + switch (flash_read) { >> + case SPI_NOR_DUAL: >> + if_type = IF_TYPE_DUAL; >> + break; >> + case SPI_NOR_QUAD: >> + if_type = IF_TYPE_QUAD; >> + break; >> + case SPI_NOR_NORMAL: >> + case SPI_NOR_FAST: >> + if_type = IF_TYPE_STD; >> + break; >> + default: >> + dev_err(sfc->dev, "unsupported SPI read mode\n"); >> + return -EINVAL; >> + } >> + >> + return if_type; >> +} > > [...] > >> +static inline void rockchip_sfc_setup_transfer(struct spi_nor *nor, >> + loff_t from_to, >> + size_t len, u8 op_type) >> +{ >> + struct rockchip_sfc_chip_priv *priv = nor->priv; >> + struct rockchip_sfc *sfc = priv->sfc; >> + u32 reg; >> + u8 if_type = 0; >> + >> + if_type = get_if_type(sfc, nor->flash_read); >> + writel_relaxed((if_type << SFC_CTRL_DATA_BITS_SHIFT) | >> + (if_type << SFC_CTRL_ADDR_BITS_SHIFT) | >> + (if_type << SFC_CTRL_CMD_BITS_SHIFT) | > > Hm, looking at this, does the controller only support n-n-n mode (1-1-1, > 2-2-2, 4-4-4) ? Or why don't you allow 1-1-n/1-n-n/2-n-n ? No, it also could support 1-1-n, etc. By looking at the cadence-quadspi.c, it only allows CQSPI_INST_TYPE_SINGLE for f_pdata->addr_width and f_pdata->inst_width, so finally it only supports 1-1-1, 1-1-2, 1-1-4? > I would like to hear some input from Cyrille on this one. > >> + (sfc->negative_edge ? SFC_CTRL_PHASE_SEL_NEGETIVE : 0), >> + sfc->regbase + SFC_CTRL); >> + >> + if (op_type == SFC_CMD_DIR_WR) >> + reg = nor->program_opcode << SFC_CMD_IDX_SHIFT; >> + else >> + reg = nor->read_opcode << SFC_CMD_IDX_SHIFT; >> + >> + reg |= op_type << SFC_CMD_DIR_SHIFT; >> + reg |= (nor->addr_width == 4) ? >> + SFC_CMD_ADDR_32BITS : SFC_CMD_ADDR_24BITS; >> + >> + reg |= priv->cs << SFC_CMD_CS_SHIFT; >> + reg |= len << SFC_CMD_TRAN_BYTES_SHIFT; >> + >> + if (op_type == SFC_CMD_DIR_RD) >> + reg |= SFC_CMD_DUMMY(nor->read_dummy); >> + >> + /* Should minus one as 0x0 means 1 bit flash address */ >> + writel_relaxed(nor->addr_width * 8 - 1, sfc->regbase + SFC_ABIT); >> + writel_relaxed(reg, sfc->regbase + SFC_CMD); >> + writel_relaxed(from_to, sfc->regbase + SFC_ADDR); >> +} > > > [...] > >> +static int rockchip_sfc_dma_transfer(struct spi_nor *nor, loff_t from_to, >> + size_t len, u_char *buf, u8 op_type) >> +{ >> + struct rockchip_sfc_chip_priv *priv = nor->priv; >> + struct rockchip_sfc *sfc = priv->sfc; >> + size_t offset; >> + int ret; >> + dma_addr_t dma_addr = 0; > > Nit, you can precalculate the DMA_TO/FROM_DEVICE and store it to > variable here, ie. > > dma_dir = (op_type == SFC_CMD_DIR_RD) ? DMA_FROM_DEVICE : DMA_TO_DEVICE. > >> + for (offset = 0; offset < len; offset += SFC_DMA_MAX_LEN) { >> + size_t trans = min_t(size_t, SFC_DMA_MAX_LEN, len - offset); >> + >> + if (SFC_CMD_DIR_RD) > > if (op_type == is missing, but you can drop this, see above. okay > >> + dma_addr = dma_map_single(NULL, (void *)buf, >> + trans, DMA_FROM_DEVICE); >> + else >> + dma_addr = dma_map_single(NULL, (void *)buf, >> + trans, DMA_TO_DEVICE); > > You can use dma_dir here ^ and drop the condition. sure > >> + if (dma_mapping_error(sfc->dev, dma_addr)) { >> + /* >> + * If we use pre-allocated dma_buffer, we need to >> + * do a copy here. >> + */ >> + if (op_type == SFC_CMD_DIR_WR) >> + memcpy(sfc->buffer, buf + offset, trans); >> + >> + dma_addr = 0; >> + } >> + >> + if (op_type == SFC_CMD_DIR_WR) >> + /* >> + * Flush the write data from write_buf to dma_addr >> + * if using dynamic allocated dma buffer before dma >> + * moves data from dma_addr to fifo. >> + */ >> + dma_sync_single_for_device(sfc->dev, dma_addr, >> + trans, DMA_TO_DEVICE); >> + >> + >> + /* If failing to map dma, use pre-allocated area instead */ >> + ret = rockchip_sfc_do_dma_transfer(nor, from_to + offset, >> + dma_addr ? dma_addr : >> + sfc->dma_buffer, >> + trans, op_type); >> + >> + if (dma_addr) { >> + /* >> + * Invalidate the read data from dma_addr if using >> + * dynamic allocated dma buffer after dma moves data >> + * from fifo to dma_addr. >> + */ >> + if (op_type == SFC_CMD_DIR_RD) { >> + dma_sync_single_for_cpu(sfc->dev, dma_addr, >> + trans, DMA_FROM_DEVICE); >> + dma_unmap_single(NULL, dma_addr, >> + trans, DMA_FROM_DEVICE); >> + } else { >> + dma_unmap_single(NULL, dma_addr, >> + trans, DMA_TO_DEVICE); > > Here as well and it'd be reduced to > > if (...) > dma_sync_single...() > dma_unmap( ... , dma_dir); sure. > >> + } >> + } >> + >> + if (ret) { >> + dev_warn(nor->dev, "DMA read timeout\n"); >> + return ret; >> + } >> + /* >> + * If we use pre-allocated dma_buffer for read, we need to >> + * do a copy here. >> + */ >> + if (!dma_addr && (op_type == SFC_CMD_DIR_RD)) >> + memcpy(buf + offset, sfc->buffer, trans); >> + } >> + >> + return len; >> +} >> + >> +static ssize_t rockchip_sfc_do_rd_wr(struct spi_nor *nor, loff_t from_to, >> + size_t len, u_char *buf, u32 op_type) >> +{ >> + struct rockchip_sfc_chip_priv *priv = nor->priv; >> + struct rockchip_sfc *sfc = priv->sfc; >> + int ret; >> + >> + if (!sfc->use_dma) >> + goto no_dma; >> + >> + return rockchip_sfc_dma_transfer(nor, from_to, len, >> + buf, op_type); > > if (likely(sfc->use_dma)) > return rockchip_sfc_dma...(); > > /* Comment saying that we fall back to PIO */ > ... pio code ... > sure. >> +no_dma: >> + ret = rockchip_sfc_pio_transfer(nor, from_to, len, >> + (u_char *)buf, op_type); >> + if (ret) { >> + if (op_type == SFC_CMD_DIR_RD) >> + dev_warn(nor->dev, "PIO read timeout\n"); >> + else >> + dev_warn(nor->dev, "PIO write timeout\n"); >> + return ret; >> + } >> + >> + return len; >> +} > > [...] > >> +/** > > Drop this asterisk unless you document the driver in kerneldoc. okay > >> + * Get spi flash device information and register it as a mtd device. >> + */ >> +static int rockchip_sfc_register(struct device_node *np, >> + struct rockchip_sfc *sfc) >> +{ >> + struct device *dev = sfc->dev; >> + struct mtd_info *mtd; >> + struct spi_nor *nor; >> + int ret; >> + >> + nor = &(sfc->flash[sfc->num_chip].nor); > > Parenthesis not needed. sure. > >> + nor->dev = dev; >> + spi_nor_set_flash_node(nor, np); >> + >> + ret = of_property_read_u8(np, "reg", &sfc->flash[sfc->num_chip].cs); >> + if (ret) { >> + dev_err(dev, "No reg property for %s\n", >> + np->full_name); >> + return ret; >> + } >> + >> + ret = of_property_read_u32(np, "spi-max-frequency", >> + &sfc->flash[sfc->num_chip].clk_rate); >> + if (ret) { >> + dev_err(dev, "No spi-max-frequency property for %s\n", >> + np->full_name); >> + return ret; >> + } >> + >> + sfc->flash[sfc->num_chip].sfc = sfc; >> + nor->priv = &(sfc->flash[sfc->num_chip]); >> + >> + nor->prepare = rockchip_sfc_prep; >> + nor->unprepare = rockchip_sfc_unprep; >> + nor->read_reg = rockchip_sfc_read_reg; >> + nor->write_reg = rockchip_sfc_write_reg; >> + nor->read = rockchip_sfc_read; >> + nor->write = rockchip_sfc_write; >> + nor->erase = NULL; >> + ret = spi_nor_scan(nor, NULL, SPI_NOR_QUAD); >> + if (ret) >> + return ret; >> + >> + mtd = &(nor->mtd); >> + mtd->name = np->name; >> + ret = mtd_device_register(mtd, NULL, 0); >> + if (ret) >> + return ret; >> + >> + sfc->num_chip++; >> + return 0; >> +} >> + >> +static void rockchip_sfc_unregister_all(struct rockchip_sfc *sfc) >> +{ >> + int i; >> + >> + for (i = 0; i < sfc->num_chip; i++) >> + mtd_device_unregister(&(sfc->flash[i].nor.mtd)); > > Inner parenthesis not needed IMO okay. > >> +} >> + >> +static int rockchip_sfc_register_all(struct rockchip_sfc *sfc) >> +{ >> + struct device *dev = sfc->dev; >> + struct device_node *np; >> + int ret; >> + >> + for_each_available_child_of_node(dev->of_node, np) { >> + ret = rockchip_sfc_register(np, sfc); >> + if (ret) >> + goto fail; >> + >> + if (sfc->num_chip == SFC_MAX_CHIPSELECT_NUM) { >> + dev_warn(dev, "Exceeds the max cs limitation\n"); >> + break; >> + } >> + } >> + >> + return 0; >> + >> +fail: >> + dev_err(dev, "Failed to register all chips\n"); >> + /* Unregister all the _registered_ nor flash */ >> + rockchip_sfc_unregister_all(sfc); >> + return ret; >> +} > > > [...] > >> +#ifdef CONFIG_PM >> +int rockchip_sfc_runtime_suspend(struct device *dev) >> +{ >> + struct rockchip_sfc *sfc = dev_get_drvdata(dev); >> + >> + clk_disable_unprepare(sfc->hclk); >> + return 0; >> +} > > Was the suspend ever really tested with this block ? Is disabling clock > really enough ? It was tested and we could do more, for instance power off the genpd, but disabling clcok should be enough. > >> +int rockchip_sfc_runtime_resume(struct device *dev) >> +{ >> + struct rockchip_sfc *sfc = dev_get_drvdata(dev); >> + >> + clk_prepare_enable(sfc->hclk); >> + return 0; >> +} >> +#endif /* CONFIG_PM */ > > [...] >
On 12/06/2016 03:56 AM, Shawn Lin wrote: [...] >>> +static inline void rockchip_sfc_setup_transfer(struct spi_nor *nor, >>> + loff_t from_to, >>> + size_t len, u8 op_type) >>> +{ >>> + struct rockchip_sfc_chip_priv *priv = nor->priv; >>> + struct rockchip_sfc *sfc = priv->sfc; >>> + u32 reg; >>> + u8 if_type = 0; >>> + >>> + if_type = get_if_type(sfc, nor->flash_read); >>> + writel_relaxed((if_type << SFC_CTRL_DATA_BITS_SHIFT) | >>> + (if_type << SFC_CTRL_ADDR_BITS_SHIFT) | >>> + (if_type << SFC_CTRL_CMD_BITS_SHIFT) | >> >> Hm, looking at this, does the controller only support n-n-n mode (1-1-1, >> 2-2-2, 4-4-4) ? Or why don't you allow 1-1-n/1-n-n/2-n-n ? > > No, it also could support 1-1-n, etc. > By looking at the cadence-quadspi.c, it only allows > CQSPI_INST_TYPE_SINGLE for f_pdata->addr_width and f_pdata->inst_width, > so finally it only supports 1-1-1, 1-1-2, 1-1-4? > >> I would like to hear some input from Cyrille on this one. The CQSPI driver indeed does only 1-1-x read thus far. I am not sure whether support for the other modes in the SPI NOR subsystem landed already, which is why I'd like to hear from Cyrille here. [...] >>> +#ifdef CONFIG_PM >>> +int rockchip_sfc_runtime_suspend(struct device *dev) >>> +{ >>> + struct rockchip_sfc *sfc = dev_get_drvdata(dev); >>> + >>> + clk_disable_unprepare(sfc->hclk); >>> + return 0; >>> +} >> >> Was the suspend ever really tested with this block ? Is disabling clock >> really enough ? > > It was tested and we could do more, for instance power off the genpd, > but disabling clcok should be enough. What about putting the controller into some reset state , is that possible? >>> +int rockchip_sfc_runtime_resume(struct device *dev) >>> +{ >>> + struct rockchip_sfc *sfc = dev_get_drvdata(dev); >>> + >>> + clk_prepare_enable(sfc->hclk); >>> + return 0; >>> +} >>> +#endif /* CONFIG_PM */ >> >> [...] >> > >
On 2016/12/6 11:08, Marek Vasut wrote: > On 12/06/2016 03:56 AM, Shawn Lin wrote: > > [...] > >>>> +static inline void rockchip_sfc_setup_transfer(struct spi_nor *nor, >>>> + loff_t from_to, >>>> + size_t len, u8 op_type) >>>> +{ >>>> + struct rockchip_sfc_chip_priv *priv = nor->priv; >>>> + struct rockchip_sfc *sfc = priv->sfc; >>>> + u32 reg; >>>> + u8 if_type = 0; >>>> + >>>> + if_type = get_if_type(sfc, nor->flash_read); >>>> + writel_relaxed((if_type << SFC_CTRL_DATA_BITS_SHIFT) | >>>> + (if_type << SFC_CTRL_ADDR_BITS_SHIFT) | >>>> + (if_type << SFC_CTRL_CMD_BITS_SHIFT) | >>> >>> Hm, looking at this, does the controller only support n-n-n mode (1-1-1, >>> 2-2-2, 4-4-4) ? Or why don't you allow 1-1-n/1-n-n/2-n-n ? >> >> No, it also could support 1-1-n, etc. >> By looking at the cadence-quadspi.c, it only allows >> CQSPI_INST_TYPE_SINGLE for f_pdata->addr_width and f_pdata->inst_width, >> so finally it only supports 1-1-1, 1-1-2, 1-1-4? >> >>> I would like to hear some input from Cyrille on this one. > > The CQSPI driver indeed does only 1-1-x read thus far. > I am not sure whether support for the other modes in the SPI NOR > subsystem landed already, which is why I'd like to hear from > Cyrille here. > > [...] > >>>> +#ifdef CONFIG_PM >>>> +int rockchip_sfc_runtime_suspend(struct device *dev) >>>> +{ >>>> + struct rockchip_sfc *sfc = dev_get_drvdata(dev); >>>> + >>>> + clk_disable_unprepare(sfc->hclk); >>>> + return 0; >>>> +} >>> >>> Was the suspend ever really tested with this block ? Is disabling clock >>> really enough ? >> >> It was tested and we could do more, for instance power off the genpd, >> but disabling clcok should be enough. > > What about putting the controller into some reset state , is that possible? yup, there are two reset control for sfc for some Socs. I will include them as optional properties to put the controller into reset state. > >>>> +int rockchip_sfc_runtime_resume(struct device *dev) >>>> +{ >>>> + struct rockchip_sfc *sfc = dev_get_drvdata(dev); >>>> + >>>> + clk_prepare_enable(sfc->hclk); >>>> + return 0; >>>> +} >>>> +#endif /* CONFIG_PM */ >>> >>> [...] >>> >> >> > >
Hi all, Le 06/12/2016 à 04:08, Marek Vasut a écrit : > On 12/06/2016 03:56 AM, Shawn Lin wrote: > > [...] > >>>> +static inline void rockchip_sfc_setup_transfer(struct spi_nor *nor, >>>> + loff_t from_to, >>>> + size_t len, u8 op_type) >>>> +{ >>>> + struct rockchip_sfc_chip_priv *priv = nor->priv; >>>> + struct rockchip_sfc *sfc = priv->sfc; >>>> + u32 reg; >>>> + u8 if_type = 0; >>>> + >>>> + if_type = get_if_type(sfc, nor->flash_read); >>>> + writel_relaxed((if_type << SFC_CTRL_DATA_BITS_SHIFT) | >>>> + (if_type << SFC_CTRL_ADDR_BITS_SHIFT) | >>>> + (if_type << SFC_CTRL_CMD_BITS_SHIFT) | >>> >>> Hm, looking at this, does the controller only support n-n-n mode (1-1-1, >>> 2-2-2, 4-4-4) ? Or why don't you allow 1-1-n/1-n-n/2-n-n ? >> >> No, it also could support 1-1-n, etc. >> By looking at the cadence-quadspi.c, it only allows >> CQSPI_INST_TYPE_SINGLE for f_pdata->addr_width and f_pdata->inst_width, >> so finally it only supports 1-1-1, 1-1-2, 1-1-4? >> >>> I would like to hear some input from Cyrille on this one. > > The CQSPI driver indeed does only 1-1-x read thus far. > I am not sure whether support for the other modes in the SPI NOR > subsystem landed already, which is why I'd like to hear from > Cyrille here. > > [...] > No, the support of SPI protocols other than 1-1-z has not been merged yet into the spi-nor subsystem. I've sent it as part of the SFDP series, since then I've been waiting for more reviews and approvals before merging it because I don't want to force anything and wait to avoid regression. Recently I was thinking about splitting the series into smaller and almost independent topics. For instance the Macronix patch to improve the management of the Quad Enable bit is a stand alone patch. Then the patch about improving support of > 128Mbit memory by using the dedicated 4-byte instruction set only depends on the patch renaming some SPINOR_OP_* macros to unify the use of the "_4B" suffix. Those two patches solve the issue of bootloaders which fail to read from SPI flash when the memory has entered its statefull 4-byte address mode. Next, there are 3 patches to add support to SPI protocols 1-y-z. I guess they are the patches your are talking about. Those patches prepares the move to the SFDP support but actually they can be also be used as is just to use SPI 1-y-z protocol without talking about SFDP. Finally, the last patches introduce the SFDP support. As I said, there are not mandatory for your use case if you only want to test SPI protocols such as 1-4-4. In your case, you might be interested in reviewing/testing: [v4, 4/8] mtd: spi-nor: add support of SPI protocols like SPI 1-2-2 and SPI-1-4-4 http://patchwork.ozlabs.org/patch/697268/ This is the main patch. [v4, 5/8] mtd: spi-nor: remove unused set_quad_mode() function http://patchwork.ozlabs.org/patch/697269/ Only small cleanup, please read the commit message for more explination [v4, 6/8] mtd: m25p80: add support of dual and quad spi protocols to all commands http://patchwork.ozlabs.org/patch/697270/ This one is not need when testing with this rockchip serial flash controller driver as it directly calls spi_nor_scan() from rockchip_sfc_register. Please note there might be a small dependence to the SPINOR_OP_* macro renaming patch: [v4, 2/8] mtd: spi-nor: rename SPINOR_OP_* macros of the 4-byte address op codes http://patchwork.ozlabs.org/patch/697266/ Indeed this patch introduces in spi-nor.h the SPINOR_OP_READ_1_2_2 and SPINOR_OP_READ_1_4_4 macros and their associated op codes. About the support of SPI 2-2-2 and SPI 4-4-4, I also have patches to add support to those two protocols however I decided not to submit them for now for many reasons. First, the series is already long and hard enough to review. Secondly, in most cases the performance increase between SPI 1-4-4 and SPI 4-4-4 isn't worth it when you read 512 byte pages or 64KB sectors. I think it was a mistake to send all those patches in a single big series since actually most of them have nothing to do with SFDP. They just prepare the transition. I understand big series might scare people and discourage them from reviewing or testing. However, if you are interested in some of those features, I think I should send the patches step by step. Best regards, Cyrille >>>> +#ifdef CONFIG_PM >>>> +int rockchip_sfc_runtime_suspend(struct device *dev) >>>> +{ >>>> + struct rockchip_sfc *sfc = dev_get_drvdata(dev); >>>> + >>>> + clk_disable_unprepare(sfc->hclk); >>>> + return 0; >>>> +} >>> >>> Was the suspend ever really tested with this block ? Is disabling clock >>> really enough ? >> >> It was tested and we could do more, for instance power off the genpd, >> but disabling clcok should be enough. > > What about putting the controller into some reset state , is that possible? > >>>> +int rockchip_sfc_runtime_resume(struct device *dev) >>>> +{ >>>> + struct rockchip_sfc *sfc = dev_get_drvdata(dev); >>>> + >>>> + clk_prepare_enable(sfc->hclk); >>>> + return 0; >>>> +} >>>> +#endif /* CONFIG_PM */ >>> >>> [...] >>> >> >> > >
On 2016/12/6 23:44, Cyrille Pitchen wrote: > Hi all, > > Le 06/12/2016 à 04:08, Marek Vasut a écrit : >> On 12/06/2016 03:56 AM, Shawn Lin wrote: >> >> [...] >> >>>>> +static inline void rockchip_sfc_setup_transfer(struct spi_nor *nor, >>>>> + loff_t from_to, >>>>> + size_t len, u8 op_type) >>>>> +{ >>>>> + struct rockchip_sfc_chip_priv *priv = nor->priv; >>>>> + struct rockchip_sfc *sfc = priv->sfc; >>>>> + u32 reg; >>>>> + u8 if_type = 0; >>>>> + >>>>> + if_type = get_if_type(sfc, nor->flash_read); >>>>> + writel_relaxed((if_type << SFC_CTRL_DATA_BITS_SHIFT) | >>>>> + (if_type << SFC_CTRL_ADDR_BITS_SHIFT) | >>>>> + (if_type << SFC_CTRL_CMD_BITS_SHIFT) | >>>> >>>> Hm, looking at this, does the controller only support n-n-n mode (1-1-1, >>>> 2-2-2, 4-4-4) ? Or why don't you allow 1-1-n/1-n-n/2-n-n ? >>> >>> No, it also could support 1-1-n, etc. >>> By looking at the cadence-quadspi.c, it only allows >>> CQSPI_INST_TYPE_SINGLE for f_pdata->addr_width and f_pdata->inst_width, >>> so finally it only supports 1-1-1, 1-1-2, 1-1-4? >>> >>>> I would like to hear some input from Cyrille on this one. >> >> The CQSPI driver indeed does only 1-1-x read thus far. >> I am not sure whether support for the other modes in the SPI NOR >> subsystem landed already, which is why I'd like to hear from >> Cyrille here. >> >> [...] >> > > No, the support of SPI protocols other than 1-1-z has not been merged yet > into the spi-nor subsystem. I've sent it as part of the SFDP series, since > then I've been waiting for more reviews and approvals before merging it > because I don't want to force anything and wait to avoid regression. > > Recently I was thinking about splitting the series into smaller and almost > independent topics. For instance the Macronix patch to improve the > management of the Quad Enable bit is a stand alone patch. > > Then the patch about improving support of > 128Mbit memory by using the > dedicated 4-byte instruction set only depends on the patch renaming some > SPINOR_OP_* macros to unify the use of the "_4B" suffix. Those two patches > solve the issue of bootloaders which fail to read from SPI flash when the > memory has entered its statefull 4-byte address mode. > > Next, there are 3 patches to add support to SPI protocols 1-y-z. I guess > they are the patches your are talking about. Those patches prepares the > move to the SFDP support but actually they can be also be used as is just > to use SPI 1-y-z protocol without talking about SFDP. > > Finally, the last patches introduce the SFDP support. As I said, there are > not mandatory for your use case if you only want to test SPI protocols such > as 1-4-4. > > In your case, you might be interested in reviewing/testing: > Thanks for sharing these patches and I will test them. :) > [v4, 4/8] mtd: spi-nor: add support of SPI protocols like SPI 1-2-2 and > SPI-1-4-4 > http://patchwork.ozlabs.org/patch/697268/ > This is the main patch. > > [v4, 5/8] mtd: spi-nor: remove unused set_quad_mode() function > http://patchwork.ozlabs.org/patch/697269/ > Only small cleanup, please read the commit message for more explination > > [v4, 6/8] mtd: m25p80: add support of dual and quad spi protocols to all > commands > http://patchwork.ozlabs.org/patch/697270/ > This one is not need when testing with this rockchip serial flash > controller driver as it directly calls spi_nor_scan() from > rockchip_sfc_register. > > Please note there might be a small dependence to the SPINOR_OP_* macro > renaming patch: > [v4, 2/8] mtd: spi-nor: rename SPINOR_OP_* macros of the 4-byte address op > codes > http://patchwork.ozlabs.org/patch/697266/ > Indeed this patch introduces in spi-nor.h the SPINOR_OP_READ_1_2_2 and > SPINOR_OP_READ_1_4_4 macros and their associated op codes. > > About the support of SPI 2-2-2 and SPI 4-4-4, I also have patches to add > support to those two protocols however I decided not to submit them for now > for many reasons. First, the series is already long and hard enough to > review. Secondly, in most cases the performance increase between SPI 1-4-4 > and SPI 4-4-4 isn't worth it when you read 512 byte pages or 64KB sectors. > > I think it was a mistake to send all those patches in a single big series > since actually most of them have nothing to do with SFDP. They just prepare > the transition. I understand big series might scare people and discourage > them from reviewing or testing. > > However, if you are interested in some of those features, I think I should > send the patches step by step. > > Best regards, > > Cyrille > >>>>> +#ifdef CONFIG_PM >>>>> +int rockchip_sfc_runtime_suspend(struct device *dev) >>>>> +{ >>>>> + struct rockchip_sfc *sfc = dev_get_drvdata(dev); >>>>> + >>>>> + clk_disable_unprepare(sfc->hclk); >>>>> + return 0; >>>>> +} >>>> >>>> Was the suspend ever really tested with this block ? Is disabling clock >>>> really enough ? >>> >>> It was tested and we could do more, for instance power off the genpd, >>> but disabling clcok should be enough. >> >> What about putting the controller into some reset state , is that possible? >> >>>>> +int rockchip_sfc_runtime_resume(struct device *dev) >>>>> +{ >>>>> + struct rockchip_sfc *sfc = dev_get_drvdata(dev); >>>>> + >>>>> + clk_prepare_enable(sfc->hclk); >>>>> + return 0; >>>>> +} >>>>> +#endif /* CONFIG_PM */ >>>> >>>> [...] >>>> >>> >>> >> >> > > > >
diff --git a/MAINTAINERS b/MAINTAINERS index 1cd38a7..eb7e06d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -10266,6 +10266,14 @@ L: linux-serial@vger.kernel.org S: Odd Fixes F: drivers/tty/serial/rp2.* +ROCKCHIP SERIAL FLASH CONTROLLER DRIVER +M: Shawn Lin <shawn.lin@rock-chips.com> +L: linux-mtd@lists.infradead.org +L: linux-rockchip@lists.infradead.org +S: Maintained +F: Documentation/devicetree/bindings/mtd/rockchip-sfc.txt +F: drivers/mtd/spi-nor/rockchip-sfc.c + ROSE NETWORK LAYER M: Ralf Baechle <ralf@linux-mips.org> L: linux-hams@vger.kernel.org diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig index 4a682ee..bf783a8 100644 --- a/drivers/mtd/spi-nor/Kconfig +++ b/drivers/mtd/spi-nor/Kconfig @@ -76,4 +76,11 @@ config SPI_NXP_SPIFI Flash. Enable this option if you have a device with a SPIFI controller and want to access the Flash as a mtd device. +config SPI_ROCKCHIP_SFC + tristate "Rockchip Serial Flash Controller(SFC)" + depends on ARCH_ROCKCHIP || COMPILE_TEST + depends on HAS_IOMEM && HAS_DMA + help + This enables support for rockchip serial flash controller. + endif # MTD_SPI_NOR diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile index 121695e..364d4c6 100644 --- a/drivers/mtd/spi-nor/Makefile +++ b/drivers/mtd/spi-nor/Makefile @@ -5,3 +5,4 @@ obj-$(CONFIG_SPI_FSL_QUADSPI) += fsl-quadspi.o obj-$(CONFIG_SPI_HISI_SFC) += hisi-sfc.o obj-$(CONFIG_MTD_MT81xx_NOR) += mtk-quadspi.o obj-$(CONFIG_SPI_NXP_SPIFI) += nxp-spifi.o +obj-$(CONFIG_SPI_ROCKCHIP_SFC) += rockchip-sfc.o diff --git a/drivers/mtd/spi-nor/rockchip-sfc.c b/drivers/mtd/spi-nor/rockchip-sfc.c new file mode 100644 index 0000000..454e1bd --- /dev/null +++ b/drivers/mtd/spi-nor/rockchip-sfc.c @@ -0,0 +1,889 @@ +/* + * Rockchip Serial Flash Controller Driver + * + * Copyright (c) 2016, Rockchip Inc. + * Author: Shawn Lin <shawn.lin@rock-chips.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ +#include <linux/bitops.h> +#include <linux/clk.h> +#include <linux/completion.h> +#include <linux/dma-mapping.h> +#include <linux/iopoll.h> +#include <linux/module.h> +#include <linux/mtd/mtd.h> +#include <linux/mtd/spi-nor.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/pm_runtime.h> +#include <linux/slab.h> + +/* System control */ +#define SFC_CTRL 0x0 +#define SFC_CTRL_COMMON_BITS_1 0x0 +#define SFC_CTRL_COMMON_BITS_2 0x1 +#define SFC_CTRL_COMMON_BITS_4 0x2 +#define SFC_CTRL_DATA_BITS_SHIFT 12 +#define SFC_CTRL_ADDR_BITS_SHIFT 10 +#define SFC_CTRL_CMD_BITS_SHIFT 8 +#define SFC_CTRL_PHASE_SEL_NEGETIVE BIT(1) + +/* Interrupt mask */ +#define SFC_IMR 0x4 +#define SFC_IMR_RX_FULL BIT(0) +#define SFC_IMR_RX_UFLOW BIT(1) +#define SFC_IMR_TX_OFLOW BIT(2) +#define SFC_IMR_TX_EMPTY BIT(3) +#define SFC_IMR_TRAN_FINISH BIT(4) +#define SFC_IMR_BUS_ERR BIT(5) +#define SFC_IMR_NSPI_ERR BIT(6) +#define SFC_IMR_DMA BIT(7) +/* Interrupt clear */ +#define SFC_ICLR 0x8 +#define SFC_ICLR_RX_FULL BIT(0) +#define SFC_ICLR_RX_UFLOW BIT(1) +#define SFC_ICLR_TX_OFLOW BIT(2) +#define SFC_ICLR_TX_EMPTY BIT(3) +#define SFC_ICLR_TRAN_FINISH BIT(4) +#define SFC_ICLR_BUS_ERR BIT(5) +#define SFC_ICLR_NSPI_ERR BIT(6) +#define SFC_ICLR_DMA BIT(7) +/* FIFO threshold level */ +#define SFC_FTLR 0xc +#define SFC_FTLR_TX_SHIFT 0 +#define SFC_FTLR_TX_MASK 0x1f +#define SFC_FTLR_RX_SHIFT 8 +#define SFC_FTLR_RX_MASK 0x1f +/* Reset FSM and FIFO */ +#define SFC_RCVR 0x10 +#define SFC_RCVR_RESET BIT(0) +/* Enhanced mode */ +#define SFC_AX 0x14 +/* Address Bit number */ +#define SFC_ABIT 0x18 +/* Interrupt status */ +#define SFC_ISR 0x1c +#define SFC_ISR_RX_FULL_SHIFT BIT(0) +#define SFC_ISR_RX_UFLOW_SHIFT BIT(1) +#define SFC_ISR_TX_OFLOW_SHIFT BIT(2) +#define SFC_ISR_TX_EMPTY_SHIFT BIT(3) +#define SFC_ISR_TX_FINISH_SHIFT BIT(4) +#define SFC_ISR_BUS_ERR_SHIFT BIT(5) +#define SFC_ISR_NSPI_ERR_SHIFT BIT(6) +#define SFC_ISR_DMA_SHIFT BIT(7) +/* FIFO status */ +#define SFC_FSR 0x20 +#define SFC_FSR_TX_IS_FULL BIT(0) +#define SFC_FSR_TX_IS_EMPTY BIT(1) +#define SFC_FSR_RX_IS_EMPTY BIT(2) +#define SFC_FSR_RX_IS_FULL BIT(3) +/* FSM status */ +#define SFC_SR 0x24 +#define SFC_SR_IS_IDLE 0x0 +#define SFC_SR_IS_BUSY 0x1 +/* Raw interrupt status */ +#define SFC_RISR 0x28 +#define SFC_RISR_RX_FULL BIT(0) +#define SFC_RISR_RX_UNDERFLOW BIT(1) +#define SFC_RISR_TX_OVERFLOW BIT(2) +#define SFC_RISR_TX_EMPTY BIT(3) +#define SFC_RISR_TRAN_FINISH BIT(4) +#define SFC_RISR_BUS_ERR BIT(5) +#define SFC_RISR_NSPI_ERR BIT(6) +#define SFC_RISR_DMA BIT(7) +/* Master trigger */ +#define SFC_DMA_TRIGGER 0x80 +/* Src or Dst addr for master */ +#define SFC_DMA_ADDR 0x84 +/* Command */ +#define SFC_CMD 0x100 +#define SFC_CMD_IDX_SHIFT 0 +#define SFC_CMD_DUMMY_SHIFT 8 +#define SFC_CMD_DIR_RD 0 +#define SFC_CMD_DIR_WR 1 +#define SFC_CMD_DIR_SHIFT 12 +#define SFC_CMD_ADDR_ZERO (0x0 << 14) +#define SFC_CMD_ADDR_24BITS (0x1 << 14) +#define SFC_CMD_ADDR_32BITS (0x2 << 14) +#define SFC_CMD_ADDR_FRS (0x3 << 14) +#define SFC_CMD_TRAN_BYTES_SHIFT 16 +#define SFC_CMD_CS_SHIFT 30 +/* Address */ +#define SFC_ADDR 0x104 +/* Data */ +#define SFC_DATA 0x108 + +#define SFC_MAX_CHIPSELECT_NUM 4 +#define SFC_DMA_MAX_LEN 0x4000 +#define SFC_CMD_DUMMY(x) \ + ((x) << SFC_CMD_DUMMY_SHIFT) + +enum rockchip_sfc_iftype { + IF_TYPE_STD, + IF_TYPE_DUAL, + IF_TYPE_QUAD, +}; + +struct rockchip_sfc; +struct rockchip_sfc_chip_priv { + u8 cs; + u32 clk_rate; + struct spi_nor nor; + struct rockchip_sfc *sfc; +}; + +struct rockchip_sfc { + struct device *dev; + struct mutex lock; + void __iomem *regbase; + struct clk *hclk; + struct clk *clk; + /* virtual mapped addr for dma_buffer */ + void *buffer; + dma_addr_t dma_buffer; + struct completion cp; + struct rockchip_sfc_chip_priv flash[SFC_MAX_CHIPSELECT_NUM]; + u32 num_chip; + bool use_dma; + /* use negative edge of hclk to latch data */ + bool negative_edge; +}; + +static int get_if_type(struct rockchip_sfc *sfc, enum read_mode flash_read) +{ + enum rockchip_sfc_iftype if_type; + + switch (flash_read) { + case SPI_NOR_DUAL: + if_type = IF_TYPE_DUAL; + break; + case SPI_NOR_QUAD: + if_type = IF_TYPE_QUAD; + break; + case SPI_NOR_NORMAL: + case SPI_NOR_FAST: + if_type = IF_TYPE_STD; + break; + default: + dev_err(sfc->dev, "unsupported SPI read mode\n"); + return -EINVAL; + } + + return if_type; +} + +static int rockchip_sfc_reset(struct rockchip_sfc *sfc) +{ + int err; + u32 status; + + writel_relaxed(SFC_RCVR_RESET, sfc->regbase + SFC_RCVR); + + err = readl_poll_timeout(sfc->regbase + SFC_RCVR, status, + !(status & SFC_RCVR_RESET), 20, + jiffies_to_usecs(HZ)); + if (err) + dev_err(sfc->dev, "SFC reset never finished\n"); + + /* Still need to clear the masked interrupt from RISR */ + writel_relaxed(SFC_ICLR_RX_FULL | SFC_ICLR_RX_UFLOW | + SFC_ICLR_TX_OFLOW | SFC_ICLR_TX_EMPTY | + SFC_ICLR_TRAN_FINISH | SFC_ICLR_BUS_ERR | + SFC_ICLR_NSPI_ERR | SFC_ICLR_DMA, + sfc->regbase + SFC_ICLR); + return err; +} + +static int rockchip_sfc_init(struct rockchip_sfc *sfc) +{ + int err; + + err = rockchip_sfc_reset(sfc); + if (err) + return err; + + /* Mask all eight interrupts */ + writel_relaxed(0xff, sfc->regbase + SFC_IMR); + + /* + * Phase configure for sfc to latch data by using + * ahb clock, and this configuration should be Soc + * specific. + */ + if (sfc->negative_edge) + writel_relaxed(SFC_CTRL_PHASE_SEL_NEGETIVE, + sfc->regbase + SFC_CTRL); + else + writel_relaxed(0, sfc->regbase + SFC_CTRL); + + return 0; +} + +static int rockchip_sfc_prep(struct spi_nor *nor, enum spi_nor_ops ops) +{ + struct rockchip_sfc_chip_priv *priv = nor->priv; + struct rockchip_sfc *sfc = priv->sfc; + int ret; + + mutex_lock(&sfc->lock); + pm_runtime_get_sync(sfc->dev); + + ret = clk_set_rate(sfc->clk, priv->clk_rate); + if (ret) + goto out; + + ret = clk_prepare_enable(sfc->clk); + if (ret) + goto out; + + return 0; + +out: + mutex_unlock(&sfc->lock); + return ret; +} + +static void rockchip_sfc_unprep(struct spi_nor *nor, enum spi_nor_ops ops) +{ + struct rockchip_sfc_chip_priv *priv = nor->priv; + struct rockchip_sfc *sfc = priv->sfc; + + clk_disable_unprepare(sfc->clk); + mutex_unlock(&sfc->lock); + pm_runtime_mark_last_busy(sfc->dev); + pm_runtime_put_autosuspend(sfc->dev); +} + +static int rockchip_sfc_wait_op_finish(struct rockchip_sfc *sfc) +{ + int err; + u32 status; + + /* + * Note: tx and rx share the same fifo, so the rx's water level + * is the same as rx's, which means this function could be reused + * for checking the read operations as well. + */ + err = readl_poll_timeout(sfc->regbase + SFC_FSR, status, + status & SFC_FSR_TX_IS_EMPTY, + 20, jiffies_to_usecs(2 * HZ)); + if (err) + dev_err(sfc->dev, "SFC fifo never empty\n"); + + return err; +} + +static int rockchip_sfc_op_reg(struct spi_nor *nor, + u8 opcode, int len, u8 optype) +{ + struct rockchip_sfc_chip_priv *priv = nor->priv; + struct rockchip_sfc *sfc = priv->sfc; + u32 reg; + bool tx_no_empty, rx_no_empty, is_busy; + int err; + + reg = readl_relaxed(sfc->regbase + SFC_FSR); + tx_no_empty = !(reg & SFC_FSR_TX_IS_EMPTY); + rx_no_empty = !(reg & SFC_FSR_RX_IS_EMPTY); + + is_busy = readl_relaxed(sfc->regbase + SFC_SR); + + if (tx_no_empty || rx_no_empty || is_busy) { + err = rockchip_sfc_reset(sfc); + if (err) + return err; + } + + reg = opcode << SFC_CMD_IDX_SHIFT; + reg |= len << SFC_CMD_TRAN_BYTES_SHIFT; + reg |= priv->cs << SFC_CMD_CS_SHIFT; + reg |= optype << SFC_CMD_DIR_SHIFT; + + writel_relaxed(reg, sfc->regbase + SFC_CMD); + + return rockchip_sfc_wait_op_finish(sfc); +} + +static void rockchip_sfc_read_fifo(struct rockchip_sfc *sfc, u8 *buf, int len) +{ + u32 tmp, i; + int total_len = len; + + /* 32-bit access only */ + if (len >= 4 && !((u32)buf & 0x03)) { + ioread32_rep(sfc->regbase + SFC_DATA, buf, len >> 2); + len %= 4; + buf += total_len - len; + } + + /* read the rest bytes */ + for (i = 0; i < len; i++) { + if (!(i & 0x03)) + tmp = readl_relaxed(sfc->regbase + SFC_DATA); + buf[i] = (tmp >> ((i & 0x03) * 8)) & 0xff; + } +} + +static int rockchip_sfc_read_reg(struct spi_nor *nor, u8 opcode, + u8 *buf, int len) +{ + struct rockchip_sfc_chip_priv *priv = nor->priv; + struct rockchip_sfc *sfc = priv->sfc; + int ret; + + ret = rockchip_sfc_op_reg(nor, opcode, len, SFC_CMD_DIR_RD); + if (ret) + return ret; + + rockchip_sfc_read_fifo(sfc, buf, len); + + return 0; +} + +static int rockchip_sfc_write_reg(struct spi_nor *nor, u8 opcode, + u8 *buf, int len) +{ + struct rockchip_sfc_chip_priv *priv = nor->priv; + struct rockchip_sfc *sfc = priv->sfc; + u32 dwords; + + /* Align bytes to dwords */ + dwords = DIV_ROUND_UP(len, sizeof(u32)); + iowrite32_rep(sfc->regbase + SFC_DATA, buf, dwords); + + return rockchip_sfc_op_reg(nor, opcode, len, SFC_CMD_DIR_WR); +} + +static inline void rockchip_sfc_setup_transfer(struct spi_nor *nor, + loff_t from_to, + size_t len, u8 op_type) +{ + struct rockchip_sfc_chip_priv *priv = nor->priv; + struct rockchip_sfc *sfc = priv->sfc; + u32 reg; + u8 if_type = 0; + + if_type = get_if_type(sfc, nor->flash_read); + writel_relaxed((if_type << SFC_CTRL_DATA_BITS_SHIFT) | + (if_type << SFC_CTRL_ADDR_BITS_SHIFT) | + (if_type << SFC_CTRL_CMD_BITS_SHIFT) | + (sfc->negative_edge ? SFC_CTRL_PHASE_SEL_NEGETIVE : 0), + sfc->regbase + SFC_CTRL); + + if (op_type == SFC_CMD_DIR_WR) + reg = nor->program_opcode << SFC_CMD_IDX_SHIFT; + else + reg = nor->read_opcode << SFC_CMD_IDX_SHIFT; + + reg |= op_type << SFC_CMD_DIR_SHIFT; + reg |= (nor->addr_width == 4) ? + SFC_CMD_ADDR_32BITS : SFC_CMD_ADDR_24BITS; + + reg |= priv->cs << SFC_CMD_CS_SHIFT; + reg |= len << SFC_CMD_TRAN_BYTES_SHIFT; + + if (op_type == SFC_CMD_DIR_RD) + reg |= SFC_CMD_DUMMY(nor->read_dummy); + + /* Should minus one as 0x0 means 1 bit flash address */ + writel_relaxed(nor->addr_width * 8 - 1, sfc->regbase + SFC_ABIT); + writel_relaxed(reg, sfc->regbase + SFC_CMD); + writel_relaxed(from_to, sfc->regbase + SFC_ADDR); +} + +static int rockchip_sfc_do_dma_transfer(struct spi_nor *nor, loff_t from_to, + dma_addr_t dma_buf, size_t len, + u8 op_type) +{ + struct rockchip_sfc_chip_priv *priv = nor->priv; + struct rockchip_sfc *sfc = priv->sfc; + u32 reg; + int err = 0; + + init_completion(&sfc->cp); + + writel_relaxed(SFC_ICLR_RX_FULL | SFC_ICLR_RX_UFLOW | + SFC_ICLR_TX_OFLOW | SFC_ICLR_TX_EMPTY | + SFC_ICLR_TRAN_FINISH | SFC_ICLR_BUS_ERR | + SFC_ICLR_NSPI_ERR | SFC_ICLR_DMA, + sfc->regbase + SFC_ICLR); + + /* Enable transfer complete interrupt */ + reg = readl_relaxed(sfc->regbase + SFC_IMR); + reg &= ~SFC_IMR_TRAN_FINISH; + writel_relaxed(reg, sfc->regbase + SFC_IMR); + + rockchip_sfc_setup_transfer(nor, from_to, len, op_type); + writel_relaxed(dma_buf, sfc->regbase + SFC_DMA_ADDR); + + /* + * Start dma but note that the sfc->dma_buffer is derived from + * dmam_alloc_coherent so we don't actually need any sync operations + * for coherent dma memory. + */ + writel_relaxed(0x1, sfc->regbase + SFC_DMA_TRIGGER); + + /* Wait for the interrupt. */ + if (!wait_for_completion_timeout(&sfc->cp, msecs_to_jiffies(2000))) { + dev_err(sfc->dev, "DMA wait for transfer finish timeout\n"); + err = -ETIMEDOUT; + } + + /* Disable transfer finish interrupt */ + reg = readl_relaxed(sfc->regbase + SFC_IMR); + reg |= SFC_IMR_TRAN_FINISH; + writel_relaxed(reg, sfc->regbase + SFC_IMR); + + if (err) { + rockchip_sfc_reset(sfc); + return err; + } + + return rockchip_sfc_wait_op_finish(sfc); +} + +static inline int rockchip_sfc_pio_write(struct rockchip_sfc *sfc, u_char *buf, + size_t len) +{ + u32 dwords; + + /* + * Align bytes to dwords, although we will write some extra + * bytes to fifo but the transfer bytes number in SFC_CMD + * register will make sure we just send out the expected + * byte numbers and the extra bytes will be clean before + * setting up the next transfer. We should always round up + * to align to DWORD as the ahb for Rockchip Socs won't + * support non-aligned-to-DWORD transfer. + */ + dwords = DIV_ROUND_UP(len, sizeof(u32)); + iowrite32_rep(sfc->regbase + SFC_DATA, buf, dwords); + + return rockchip_sfc_wait_op_finish(sfc); +} + +static inline int rockchip_sfc_pio_read(struct rockchip_sfc *sfc, u_char *buf, + size_t len) +{ + rockchip_sfc_read_fifo(sfc, buf, len); + + return rockchip_sfc_wait_op_finish(sfc); +} + +static int rockchip_sfc_pio_transfer(struct spi_nor *nor, loff_t from_to, + size_t len, u_char *buf, u8 op_type) +{ + struct rockchip_sfc_chip_priv *priv = nor->priv; + struct rockchip_sfc *sfc = priv->sfc; + + rockchip_sfc_setup_transfer(nor, from_to, len, op_type); + + if (op_type == SFC_CMD_DIR_WR) + return rockchip_sfc_pio_write(sfc, buf, len); + else + return rockchip_sfc_pio_read(sfc, buf, len); +} + +static int rockchip_sfc_dma_transfer(struct spi_nor *nor, loff_t from_to, + size_t len, u_char *buf, u8 op_type) +{ + struct rockchip_sfc_chip_priv *priv = nor->priv; + struct rockchip_sfc *sfc = priv->sfc; + size_t offset; + int ret; + dma_addr_t dma_addr = 0; + + for (offset = 0; offset < len; offset += SFC_DMA_MAX_LEN) { + size_t trans = min_t(size_t, SFC_DMA_MAX_LEN, len - offset); + + if (SFC_CMD_DIR_RD) + dma_addr = dma_map_single(NULL, (void *)buf, + trans, DMA_FROM_DEVICE); + else + dma_addr = dma_map_single(NULL, (void *)buf, + trans, DMA_TO_DEVICE); + + if (dma_mapping_error(sfc->dev, dma_addr)) { + /* + * If we use pre-allocated dma_buffer, we need to + * do a copy here. + */ + if (op_type == SFC_CMD_DIR_WR) + memcpy(sfc->buffer, buf + offset, trans); + + dma_addr = 0; + } + + if (op_type == SFC_CMD_DIR_WR) + /* + * Flush the write data from write_buf to dma_addr + * if using dynamic allocated dma buffer before dma + * moves data from dma_addr to fifo. + */ + dma_sync_single_for_device(sfc->dev, dma_addr, + trans, DMA_TO_DEVICE); + + + /* If failing to map dma, use pre-allocated area instead */ + ret = rockchip_sfc_do_dma_transfer(nor, from_to + offset, + dma_addr ? dma_addr : + sfc->dma_buffer, + trans, op_type); + + if (dma_addr) { + /* + * Invalidate the read data from dma_addr if using + * dynamic allocated dma buffer after dma moves data + * from fifo to dma_addr. + */ + if (op_type == SFC_CMD_DIR_RD) { + dma_sync_single_for_cpu(sfc->dev, dma_addr, + trans, DMA_FROM_DEVICE); + dma_unmap_single(NULL, dma_addr, + trans, DMA_FROM_DEVICE); + } else { + dma_unmap_single(NULL, dma_addr, + trans, DMA_TO_DEVICE); + } + } + + if (ret) { + dev_warn(nor->dev, "DMA read timeout\n"); + return ret; + } + /* + * If we use pre-allocated dma_buffer for read, we need to + * do a copy here. + */ + if (!dma_addr && (op_type == SFC_CMD_DIR_RD)) + memcpy(buf + offset, sfc->buffer, trans); + } + + return len; +} + +static ssize_t rockchip_sfc_do_rd_wr(struct spi_nor *nor, loff_t from_to, + size_t len, u_char *buf, u32 op_type) +{ + struct rockchip_sfc_chip_priv *priv = nor->priv; + struct rockchip_sfc *sfc = priv->sfc; + int ret; + + if (!sfc->use_dma) + goto no_dma; + + return rockchip_sfc_dma_transfer(nor, from_to, len, + buf, op_type); +no_dma: + ret = rockchip_sfc_pio_transfer(nor, from_to, len, + (u_char *)buf, op_type); + if (ret) { + if (op_type == SFC_CMD_DIR_RD) + dev_warn(nor->dev, "PIO read timeout\n"); + else + dev_warn(nor->dev, "PIO write timeout\n"); + return ret; + } + + return len; +} + +static ssize_t rockchip_sfc_read(struct spi_nor *nor, loff_t from, + size_t len, u_char *read_buf) +{ + return rockchip_sfc_do_rd_wr(nor, from, len, + read_buf, SFC_CMD_DIR_RD); +} + +static ssize_t rockchip_sfc_write(struct spi_nor *nor, loff_t to, + size_t len, const u_char *write_buf) +{ + return rockchip_sfc_do_rd_wr(nor, to, len, + (u_char *)write_buf, + SFC_CMD_DIR_WR); +} + +/** + * Get spi flash device information and register it as a mtd device. + */ +static int rockchip_sfc_register(struct device_node *np, + struct rockchip_sfc *sfc) +{ + struct device *dev = sfc->dev; + struct mtd_info *mtd; + struct spi_nor *nor; + int ret; + + nor = &(sfc->flash[sfc->num_chip].nor); + nor->dev = dev; + spi_nor_set_flash_node(nor, np); + + ret = of_property_read_u8(np, "reg", &sfc->flash[sfc->num_chip].cs); + if (ret) { + dev_err(dev, "No reg property for %s\n", + np->full_name); + return ret; + } + + ret = of_property_read_u32(np, "spi-max-frequency", + &sfc->flash[sfc->num_chip].clk_rate); + if (ret) { + dev_err(dev, "No spi-max-frequency property for %s\n", + np->full_name); + return ret; + } + + sfc->flash[sfc->num_chip].sfc = sfc; + nor->priv = &(sfc->flash[sfc->num_chip]); + + nor->prepare = rockchip_sfc_prep; + nor->unprepare = rockchip_sfc_unprep; + nor->read_reg = rockchip_sfc_read_reg; + nor->write_reg = rockchip_sfc_write_reg; + nor->read = rockchip_sfc_read; + nor->write = rockchip_sfc_write; + nor->erase = NULL; + ret = spi_nor_scan(nor, NULL, SPI_NOR_QUAD); + if (ret) + return ret; + + mtd = &(nor->mtd); + mtd->name = np->name; + ret = mtd_device_register(mtd, NULL, 0); + if (ret) + return ret; + + sfc->num_chip++; + return 0; +} + +static void rockchip_sfc_unregister_all(struct rockchip_sfc *sfc) +{ + int i; + + for (i = 0; i < sfc->num_chip; i++) + mtd_device_unregister(&(sfc->flash[i].nor.mtd)); +} + +static int rockchip_sfc_register_all(struct rockchip_sfc *sfc) +{ + struct device *dev = sfc->dev; + struct device_node *np; + int ret; + + for_each_available_child_of_node(dev->of_node, np) { + ret = rockchip_sfc_register(np, sfc); + if (ret) + goto fail; + + if (sfc->num_chip == SFC_MAX_CHIPSELECT_NUM) { + dev_warn(dev, "Exceeds the max cs limitation\n"); + break; + } + } + + return 0; + +fail: + dev_err(dev, "Failed to register all chips\n"); + /* Unregister all the _registered_ nor flash */ + rockchip_sfc_unregister_all(sfc); + return ret; +} + +static irqreturn_t rockchip_sfc_irq_handler(int irq, void *dev_id) +{ + struct rockchip_sfc *sfc = dev_id; + u32 reg; + + reg = readl_relaxed(sfc->regbase + SFC_RISR); + dev_dbg(sfc->dev, "Get irq: 0x%x\n", reg); + + /* Clear interrupt */ + writel_relaxed(reg, sfc->regbase + SFC_ICLR); + + if (reg & SFC_RISR_TRAN_FINISH) + complete(&sfc->cp); + + return IRQ_HANDLED; +} + +static int rockchip_sfc_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct resource *res; + struct rockchip_sfc *sfc; + int ret; + + sfc = devm_kzalloc(dev, sizeof(*sfc), GFP_KERNEL); + if (!sfc) + return -ENOMEM; + + platform_set_drvdata(pdev, sfc); + sfc->dev = dev; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + sfc->regbase = devm_ioremap_resource(dev, res); + if (IS_ERR(sfc->regbase)) + return PTR_ERR(sfc->regbase); + + sfc->clk = devm_clk_get(&pdev->dev, "sfc"); + if (IS_ERR(sfc->clk)) { + dev_err(&pdev->dev, "Failed to get sfc interface clk\n"); + return PTR_ERR(sfc->clk); + } + + sfc->hclk = devm_clk_get(&pdev->dev, "hsfc"); + if (IS_ERR(sfc->hclk)) { + dev_err(&pdev->dev, "Failed to get sfc ahp clk\n"); + return PTR_ERR(sfc->hclk); + } + + ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32)); + if (ret) { + dev_warn(dev, "Unable to set dma mask\n"); + return ret; + } + + sfc->buffer = dmam_alloc_coherent(dev, SFC_DMA_MAX_LEN, + &sfc->dma_buffer, GFP_KERNEL); + if (!sfc->buffer) + return -ENOMEM; + + mutex_init(&sfc->lock); + + ret = clk_prepare_enable(sfc->hclk); + if (ret) { + dev_err(&pdev->dev, "Failed to enable hclk\n"); + goto err_hclk; + } + + ret = clk_prepare_enable(sfc->clk); + if (ret) { + dev_err(&pdev->dev, "Failed to enable clk\n"); + goto err_clk; + } + + sfc->use_dma = !of_property_read_bool(sfc->dev->of_node, + "rockchip,sfc-no-DMA"); + + sfc->negative_edge = of_device_is_compatible(sfc->dev->of_node, + "rockchip,rk1108-sfc"); + /* Find the irq */ + ret = platform_get_irq(pdev, 0); + if (ret < 0) { + dev_err(dev, "Failed to get the irq\n"); + goto err_irq; + } + + ret = devm_request_irq(dev, ret, rockchip_sfc_irq_handler, + 0, pdev->name, sfc); + if (ret) { + dev_err(dev, "Failed to request irq\n"); + goto err_irq; + } + + sfc->num_chip = 0; + ret = rockchip_sfc_init(sfc); + if (ret) + goto err_irq; + + pm_runtime_get_noresume(&pdev->dev); + pm_runtime_set_active(&pdev->dev); + pm_runtime_enable(&pdev->dev); + pm_runtime_set_autosuspend_delay(&pdev->dev, 50); + pm_runtime_use_autosuspend(&pdev->dev); + + ret = rockchip_sfc_register_all(sfc); + if (ret) + goto err_register; + + clk_disable_unprepare(sfc->clk); + pm_runtime_put_autosuspend(&pdev->dev); + return 0; + +err_register: + pm_runtime_disable(&pdev->dev); + pm_runtime_set_suspended(&pdev->dev); + pm_runtime_put_noidle(&pdev->dev); +err_irq: + clk_disable_unprepare(sfc->clk); +err_clk: + clk_disable_unprepare(sfc->hclk); +err_hclk: + mutex_destroy(&sfc->lock); + return ret; +} + +static int rockchip_sfc_remove(struct platform_device *pdev) +{ + struct rockchip_sfc *sfc = platform_get_drvdata(pdev); + + pm_runtime_get_sync(&pdev->dev); + pm_runtime_disable(&pdev->dev); + pm_runtime_put_noidle(&pdev->dev); + + rockchip_sfc_unregister_all(sfc); + mutex_destroy(&sfc->lock); + clk_disable_unprepare(sfc->clk); + clk_disable_unprepare(sfc->hclk); + return 0; +} + +#ifdef CONFIG_PM +int rockchip_sfc_runtime_suspend(struct device *dev) +{ + struct rockchip_sfc *sfc = dev_get_drvdata(dev); + + clk_disable_unprepare(sfc->hclk); + return 0; +} + +int rockchip_sfc_runtime_resume(struct device *dev) +{ + struct rockchip_sfc *sfc = dev_get_drvdata(dev); + + clk_prepare_enable(sfc->hclk); + return 0; +} +#endif /* CONFIG_PM */ + +static const struct of_device_id rockchip_sfc_dt_ids[] = { + { .compatible = "rockchip,sfc"}, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, rockchip_sfc_dt_ids); + +static const struct dev_pm_ops rockchip_sfc_dev_pm_ops = { + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, + pm_runtime_force_resume) + SET_RUNTIME_PM_OPS(rockchip_sfc_runtime_suspend, + rockchip_sfc_runtime_resume, NULL) +}; + +static struct platform_driver rockchip_sfc_driver = { + .driver = { + .name = "rockchip-sfc", + .of_match_table = rockchip_sfc_dt_ids, + .pm = &rockchip_sfc_dev_pm_ops, + }, + .probe = rockchip_sfc_probe, + .remove = rockchip_sfc_remove, +}; +module_platform_driver(rockchip_sfc_driver); + +MODULE_LICENSE("GPL v2"); +MODULE_DESCRIPTION("Rockchip Serial Flash Controller Driver"); +MODULE_AUTHOR("Shawn Lin <shawn.lin@rock-chips.com>");
Add rockchip serial flash controller driver Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> --- Changes in v3: - use io{read32,write32}_rep to simplify the corner cases - remove more unnecessary bit definitions - some minor comment fixes and improvement - fix wrong unregister function - unify more code - use nor to avoid constantly replicating the whole sfc->flash[sfc->num_chip].nor - add email for MODULE_AUTHOR - remove #if 1 --- #endif - extract DMA code to imporve the code structure - reset all when failing to do dma - pass sfc to get_if_type - rename sfc-no-dma to sfc-no-DMA Changes in v2: - fix typos - add some comment for buffer and others operations - rename SFC_MAX_CHIP_NUM to MAX_CHIPSELECT_NUM - use u8 for cs - return -EINVAL for default case of get_if_type - use readl_poll_*() to check timeout cases - simplify and clarify some condition checks - rework the bitshifts to simplify the code - define SFC_CMD_DUMMY(x) - fix ummap for dma read path and finish all the cache maintenance. - rename to rockchip_sfc_chip_priv and embed struct spi_nor in it. - add MODULE_AUTHOR - add runtime PM and general PM support. - Thanks for Marek's comments. Link: http://lists.infradead.org/pipermail/linux-mtd/2016-November/070321.html MAINTAINERS | 8 + drivers/mtd/spi-nor/Kconfig | 7 + drivers/mtd/spi-nor/Makefile | 1 + drivers/mtd/spi-nor/rockchip-sfc.c | 889 +++++++++++++++++++++++++++++++++++++ 4 files changed, 905 insertions(+) create mode 100644 drivers/mtd/spi-nor/rockchip-sfc.c