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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1466424354.6070.41.camel@pengutronix.de>
Date:	Mon, 20 Jun 2016 14:05:54 +0200
From:	Philipp Zabel <p.zabel@...gutronix.de>
To:	Boris Brezillon <boris.brezillon@...e-electrons.com>
Cc:	Icenowy Zheng <icenowy@...c.xyz>, maxime.ripard@...e-electrons.com,
	wens@...e.org, robh+dt@...nel.org, richard@....at,
	dwmw2@...radead.org, computersforpeace@...il.com,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-mtd@...ts.infradead.org, linux-sunxi@...glegroups.com
Subject: Re: [PATCH 2/2] mtd: nand: sunxi: add reset line support

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@...c.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@...c.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