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 */ >>> >>> [...] >>> >> >> > > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html