[PATCH 1/2] mtd: nand: sunxi: update DT bindings

[PATCH 1/2] mtd: nand: sunxi: update DT bindings

[Icenowy Zheng]

Document the reset lines

Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
---
 Documentation/devicetree/bindings/mtd/sunxi-nand.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/sunxi-nand.txt b/Documentation/devicetree/bindings/mtd/sunxi-nand.txt
index 086d6f4..a328fbb 100644
--- a/Documentation/devicetree/bindings/mtd/sunxi-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/sunxi-nand.txt
@@ -15,6 +15,8 @@ Optional children nodes:
 Children nodes represent the available nand chips.
 
 Optional properties:
+- reset : phandle + reset specifier pair
+- reset-names : must contain "ahb"
 - allwinner,rb : shall contain the native Ready/Busy ids.
  or
 - rb-gpios : shall contain the gpios used as R/B pins.
-- 
2.8.1

[PATCH 1/2] mtd: nand: sunxi: update DT bindings

[Icenowy Zheng]

Document the reset lines

Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
---
 Documentation/devicetree/bindings/mtd/sunxi-nand.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/sunxi-nand.txt b/Documentation/devicetree/bindings/mtd/sunxi-nand.txt
index 086d6f4..a328fbb 100644
--- a/Documentation/devicetree/bindings/mtd/sunxi-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/sunxi-nand.txt
@@ -15,6 +15,8 @@ Optional children nodes:
 Children nodes represent the available nand chips.
 
 Optional properties:
+- reset : phandle + reset specifier pair
+- reset-names : must contain "ahb"
 - allwinner,rb : shall contain the native Ready/Busy ids.
  or
 - rb-gpios : shall contain the gpios used as R/B pins.
-- 
2.8.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 2/2] mtd: nand: sunxi: add reset line support

[Icenowy Zheng]

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@aosc.xyz>
---
 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);
+
+	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;
+		}
+	}
+
 	ret = sunxi_nfc_rst(nfc);
 	if (ret)
 		goto out_mod_clk_unprepare;
-- 
2.8.1

[PATCH 2/2] mtd: nand: sunxi: add reset line support

[Icenowy Zheng]

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@aosc.xyz>
---
 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);
+
+	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;
+		}
+	}
+
 	ret = sunxi_nfc_rst(nfc);
 	if (ret)
 		goto out_mod_clk_unprepare;
-- 
2.8.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/2] mtd: nand: sunxi: add reset line support

[Boris Brezillon]

+Philipp

On Sun, 19 Jun 2016 19:37:39 +0800
Icenowy Zheng <icenowy@aosc.xyz> 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@aosc.xyz>
> ---
>  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;
> +		}
> +	}
> +

Re: [PATCH 2/2] mtd: nand: sunxi: add reset line support

[Boris Brezillon]

+Philipp

On Sun, 19 Jun 2016 19:37:39 +0800
Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org> 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-ymACFijhrKM@public.gmane.org>
> ---
>  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;
> +		}
> +	}
> +

Re: [PATCH 2/2] mtd: nand: sunxi: add reset line support

[icenowy]

To be honest, I copied them from sunxi-mmc.c.

What function should be chosen better?


19.06.2016, 20:06, "Boris Brezillon" <boris.brezillon@free-electrons.com>:
> +Philipp
>
> On Sun, 19 Jun 2016 19:37:39 +0800
> Icenowy Zheng <icenowy@aosc.xyz> 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@aosc.xyz>
>>  ---
>>   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;
>>  + }
>>  + }
>>  +

Re: [PATCH 2/2] mtd: nand: sunxi: add reset line support

[icenowy-ymACFijhrKM]

To be honest, I copied them from sunxi-mmc.c.

What function should be chosen better?


19.06.2016, 20:06, "Boris Brezillon" <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>:
> +Philipp
>
> On Sun, 19 Jun 2016 19:37:39 +0800
> Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org> 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-ymACFijhrKM@public.gmane.org>
>>  ---
>>   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;
>>  + }
>>  + }
>>  +

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 2/2] mtd: nand: sunxi: add reset line support

[Boris Brezillon]

On Sun, 19 Jun 2016 20:41:09 +0800
icenowy@aosc.xyz 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@free-electrons.com>:
> > +Philipp
> >
> > On Sun, 19 Jun 2016 19:37:39 +0800
> > Icenowy Zheng <icenowy@aosc.xyz> 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@aosc.xyz>
> >>  ---
> >>   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;
> >>  + }
> >>  + }
> >>  +  

Re: [PATCH 2/2] mtd: nand: sunxi: add reset line support

[Boris Brezillon]

On Sun, 19 Jun 2016 20:41:09 +0800
icenowy-ymACFijhrKM@public.gmane.org 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-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>:
> > +Philipp
> >
> > On Sun, 19 Jun 2016 19:37:39 +0800
> > Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org> 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-ymACFijhrKM@public.gmane.org>
> >>  ---
> >>   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;
> >>  + }
> >>  + }
> >>  +  

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 2/2] mtd: nand: sunxi: add reset line support

[icenowy]

Then I will soon make the v2 patch set, with the error detection part fixed.

But why does sunxi-mmc check only EPROBE_DEFER?

19.06.2016, 20:53, "Boris Brezillon" <boris.brezillon@free-electrons.com>:
> On Sun, 19 Jun 2016 20:41:09 +0800
> icenowy@aosc.xyz 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@free-electrons.com>:
>>  > +Philipp
>>  >
>>  > On Sun, 19 Jun 2016 19:37:39 +0800
>>  > Icenowy Zheng <icenowy@aosc.xyz> 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@aosc.xyz>
>>  >>  ---
>>  >>   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;
>>  >>  + }
>>  >>  + }
>>  >>  +

Re: [PATCH 2/2] mtd: nand: sunxi: add reset line support

[icenowy-ymACFijhrKM]

Then I will soon make the v2 patch set, with the error detection part fixed.

But why does sunxi-mmc check only EPROBE_DEFER?

19.06.2016, 20:53, "Boris Brezillon" <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>:
> On Sun, 19 Jun 2016 20:41:09 +0800
> icenowy-ymACFijhrKM@public.gmane.org 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@free-electrons.com>:
>>  > +Philipp
>>  >
>>  > On Sun, 19 Jun 2016 19:37:39 +0800
>>  > Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org> 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-ymACFijhrKM@public.gmane.org>
>>  >>  ---
>>  >>   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;
>>  >>  + }
>>  >>  + }
>>  >>  +

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 2/2] mtd: nand: sunxi: add reset line support

[Boris Brezillon]

On Sun, 19 Jun 2016 21:11:00 +0800
icenowy@aosc.xyz 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@free-electrons.com>:
> > On Sun, 19 Jun 2016 20:41:09 +0800
> > icenowy@aosc.xyz 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@free-electrons.com>:  
> >>  > +Philipp
> >>  >
> >>  > On Sun, 19 Jun 2016 19:37:39 +0800
> >>  > Icenowy Zheng <icenowy@aosc.xyz> 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@aosc.xyz>
> >>  >>  ---
> >>  >>   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;
> >>  >>  + }
> >>  >>  + }
> >>  >>  +  

Re: [PATCH 2/2] mtd: nand: sunxi: add reset line support

[Boris Brezillon]

On Sun, 19 Jun 2016 21:11:00 +0800
icenowy-ymACFijhrKM@public.gmane.org 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-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>:
> > On Sun, 19 Jun 2016 20:41:09 +0800
> > icenowy-ymACFijhrKM@public.gmane.org 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@free-electrons.com>:  
> >>  > +Philipp
> >>  >
> >>  > On Sun, 19 Jun 2016 19:37:39 +0800
> >>  > Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org> 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-ymACFijhrKM@public.gmane.org>
> >>  >>  ---
> >>  >>   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;
> >>  >>  + }
> >>  >>  + }
> >>  >>  +  

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 2/2] mtd: nand: sunxi: add reset line support

[Philipp Zabel]

Am Sonntag, den 19.06.2016, 14:06 +0200 schrieb Boris Brezillon:
> +Philipp
> 
> On Sun, 19 Jun 2016 19:37:39 +0800
> Icenowy Zheng <icenowy@aosc.xyz> 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@aosc.xyz>
> > ---
> >  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()).

Of course it's possible, -ENOENT is only returned if the reset line is
not defined in the device tree.

Note that gpiod_get_(index_)optional do nothing more that replacing
-ENOENT with NULL. And phydev_optional_get replaces -ENODEV with NULL.
And regulator_get_optional, if I understand it correctly, never returns
NULL.

> Philipp, is there a good reason for doing that?

Historically, NULL has not been a valid value for rstc. I suppose we
could add NULL checks to the reset_control_assert/deassert/reset/status
functions and align the reset API a bit with gpiod. I just wouldn't want
to see any IS_ERR_OR_NULL error handling in the drivers.

regards
Philipp

Re: [PATCH 2/2] mtd: nand: sunxi: add reset line support

[Philipp Zabel]

Am Sonntag, den 19.06.2016, 14:06 +0200 schrieb Boris Brezillon:
> +Philipp
> 
> On Sun, 19 Jun 2016 19:37:39 +0800
> Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org> 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-ymACFijhrKM@public.gmane.org>
> > ---
> >  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()).

Of course it's possible, -ENOENT is only returned if the reset line is
not defined in the device tree.

Note that gpiod_get_(index_)optional do nothing more that replacing
-ENOENT with NULL. And phydev_optional_get replaces -ENODEV with NULL.
And regulator_get_optional, if I understand it correctly, never returns
NULL.

> Philipp, is there a good reason for doing that?

Historically, NULL has not been a valid value for rstc. I suppose we
could add NULL checks to the reset_control_assert/deassert/reset/status
functions and align the reset API a bit with gpiod. I just wouldn't want
to see any IS_ERR_OR_NULL error handling in the drivers.

regards
Philipp

Re: [PATCH 2/2] mtd: nand: sunxi: add reset line support

[Boris Brezillon]

Hi Philipp,

On Mon, 20 Jun 2016 14:05:54 +0200
Philipp Zabel <p.zabel@pengutronix.de> wrote:

> Am Sonntag, den 19.06.2016, 14:06 +0200 schrieb Boris Brezillon:
> > +Philipp
> > 
> > On Sun, 19 Jun 2016 19:37:39 +0800
> > Icenowy Zheng <icenowy@aosc.xyz> 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@aosc.xyz>
> > > ---
> > >  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()).  
> 
> Of course it's possible, -ENOENT is only returned if the reset line is
> not defined in the device tree.

Okay, testing for err != -ENOENT is the right thing to do here. Maybe
this could be documented.

> 
> Note that gpiod_get_(index_)optional do nothing more that replacing
> -ENOENT with NULL. And phydev_optional_get replaces -ENODEV with NULL.
> And regulator_get_optional, if I understand it correctly, never returns
> NULL.
> 
> > Philipp, is there a good reason for doing that?  
> 
> Historically, NULL has not been a valid value for rstc. I suppose we
> could add NULL checks to the reset_control_assert/deassert/reset/status
> functions and align the reset API a bit with gpiod. I just wouldn't want
> to see any IS_ERR_OR_NULL error handling in the drivers.

Well, returning NULL is mainly a convenient way to differentiate real
errors from missing optional reset lines (which are not errors). Now,
if you say checking for -ENOENT is the right thing to do here, I'm
fine with that.

Regards,

Boris

Re: [PATCH 2/2] mtd: nand: sunxi: add reset line support

[Boris Brezillon]

Hi Philipp,

On Mon, 20 Jun 2016 14:05:54 +0200
Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote:

> Am Sonntag, den 19.06.2016, 14:06 +0200 schrieb Boris Brezillon:
> > +Philipp
> > 
> > On Sun, 19 Jun 2016 19:37:39 +0800
> > Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org> 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-ymACFijhrKM@public.gmane.org>
> > > ---
> > >  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()).  
> 
> Of course it's possible, -ENOENT is only returned if the reset line is
> not defined in the device tree.

Okay, testing for err != -ENOENT is the right thing to do here. Maybe
this could be documented.

> 
> Note that gpiod_get_(index_)optional do nothing more that replacing
> -ENOENT with NULL. And phydev_optional_get replaces -ENODEV with NULL.
> And regulator_get_optional, if I understand it correctly, never returns
> NULL.
> 
> > Philipp, is there a good reason for doing that?  
> 
> Historically, NULL has not been a valid value for rstc. I suppose we
> could add NULL checks to the reset_control_assert/deassert/reset/status
> functions and align the reset API a bit with gpiod. I just wouldn't want
> to see any IS_ERR_OR_NULL error handling in the drivers.

Well, returning NULL is mainly a convenient way to differentiate real
errors from missing optional reset lines (which are not errors). Now,
if you say checking for -ENOENT is the right thing to do here, I'm
fine with that.

Regards,

Boris