Re: [PATCH v3 2/2] mtd: spi-nor: add rockchip serial flash controller driver

On 2016/12/6 11:08, Marek Vasut wrote:

On 12/06/2016 03:56 AM, Shawn Lin wrote:

[...]

quoted

quoted

quoted

+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?

quoted

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.

[...]

quoted

quoted

quoted

+#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.

quoted

quoted

quoted

+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 */

[...]

-- 
Best Regards
Shawn Lin

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html