On Sun, 19 Jun 2016 20:41:09 +0800 icenowy@xxxxxxxx wrote: > To be honest, I copied them from sunxi-mmc.c. > > What function should be chosen better? You did the right thing (except for the error detection part). My question was addressed to Philipp (the reset subsystem maintainer). > > > 19.06.2016, 20:06, "Boris Brezillon" <boris.brezillon@xxxxxxxxxxxxxxxxxx>: > > +Philipp > > > > On Sun, 19 Jun 2016 19:37:39 +0800 > > Icenowy Zheng <icenowy@xxxxxxxx> wrote: > > > >> The NAND controller on some sun8i chips needs its reset line to be deasserted > >> before they can enter working state. This commit added the reset line process > >> to the driver. > >> > >> Signed-off-by: Icenowy Zheng <icenowy@xxxxxxxx> > >> --- > >> drivers/mtd/nand/sunxi_nand.c | 14 ++++++++++++++ > >> 1 file changed, 14 insertions(+) > >> > >> diff --git a/drivers/mtd/nand/sunxi_nand.c b/drivers/mtd/nand/sunxi_nand.c > >> index a83a690..1502748 100644 > >> --- a/drivers/mtd/nand/sunxi_nand.c > >> +++ b/drivers/mtd/nand/sunxi_nand.c > >> @@ -39,6 +39,7 @@ > >> #include <linux/gpio.h> > >> #include <linux/interrupt.h> > >> #include <linux/iopoll.h> > >> +#include <linux/reset.h> > >> > >> #define NFC_REG_CTL 0x0000 > >> #define NFC_REG_ST 0x0004 > >> @@ -269,6 +270,7 @@ struct sunxi_nfc { > >> void __iomem *regs; > >> struct clk *ahb_clk; > >> struct clk *mod_clk; > >> + struct reset_control *reset; > >> unsigned long assigned_cs; > >> unsigned long clk_rate; > >> struct list_head chips; > >> @@ -1871,6 +1873,18 @@ static int sunxi_nfc_probe(struct platform_device *pdev) > >> if (ret) > >> goto out_ahb_clk_unprepare; > >> > >> + nfc->reset = devm_reset_control_get_optional(dev, "ahb"); > >> + if (PTR_ERR(nfc->reset) == -EPROBE_DEFER) > >> + return PTR_ERR(nfc->reset); > > > > Actually you should test for != -ENOENT, because all error codes except > > this one should stop the ->probe(). > > > > BTW, this devm_reset_control_get_optional() is really weird. While most > > _optional() methods return NULL when the element is not defined in the > > DT, this one returns -ENOTENT, which makes it impossible to > > differentiate a real error from a undefined reset line (which is a > > valid case for _optional()). > > > > Philipp, is there a good reason for doing that? > > > >> + > >> + if (!IS_ERR(nfc->reset)) { > >> + ret = reset_control_deassert(nfc->reset); > >> + if (ret) { > >> + dev_err(dev, "reset err %d\n", ret); > >> + goto out_mod_clk_unprepare; > >> + } > >> + } > >> + -- 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