On Sun, 19 Jun 2016 21:11:00 +0800 icenowy@xxxxxxxx wrote: > Then I will soon make the v2 patch set, with the error detection part fixed. > > But why does sunxi-mmc check only EPROBE_DEFER? I guess someone had a probe-dependency problem and fixed this case but ignored all the possible errors. But there may be other reasons for devm_reset_control_get_optional() to fail. The only one that is really reflecting that the reset line is not defined in the DT is -ENOENT. > > 19.06.2016, 20:53, "Boris Brezillon" <boris.brezillon@xxxxxxxxxxxxxxxxxx>: > > 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