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]
Date:   Sat, 19 May 2018 01:15:30 +0200
From:   Janusz Krzysztofik <jmkrzyszt@...il.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     Tony Lindgren <tony@...mide.com>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Boris Brezillon <boris.brezillon@...tlin.com>,
        Tomi Valkeinen <tomi.valkeinen@...com>,
        Mark Brown <broonie@...nel.org>,
        Aaro Koskinen <aaro.koskinen@....fi>,
        Richard Weinberger <richard@....at>,
        Peter Ujfalusi <peter.ujfalusi@...com>,
        Jarkko Nikula <jarkko.nikula@...mer.com>,
        Liam Girdwood <lgirdwood@...il.com>,
        linux-arm Mailing List <linux-arm-kernel@...ts.infradead.org>,
        Linux OMAP Mailing List <linux-omap@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-input <linux-input@...r.kernel.org>,
        "open list:MEMORY TECHNOLOGY..." <linux-mtd@...ts.infradead.org>,
        linux-fbdev@...r.kernel.org,
        ALSA Development Mailing List <alsa-devel@...a-project.org>
Subject: Re: [PATCH 5/6] mtd: rawnand: ams-delta: use GPIO lookup table

On Friday, May 18, 2018 11:21:14 PM CEST Andy Shevchenko wrote:
> On Sat, May 19, 2018 at 12:09 AM, Janusz Krzysztofik
> 
> <jmkrzyszt@...il.com> wrote:
> > +       gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
> > +       if (!IS_ERR_OR_NULL(gpiod_rdy)) {
> 
> So, is it optional or not at the end?
> If it is, why do we check for NULL?

As far as I can understand, nand_chip->dev_ready() callback is optional. 
That's why I decided to use the _optional variant of devm_gpiod_get(). In case 
of ams-delta, the dev_ready() callback depends on availability of the 'rdy' 
GPIO pin. As a consequence, I'm checking for both NULL and ERR in order to 
decide if dev_ready() will be supported.

I can pretty well replace it with the standard form and check for ERR only if 
the purpose of the _optional form is different.

> >                 this->dev_ready = ams_delta_nand_ready;
> >         
> >         } else {
> >         
> >                 this->dev_ready = NULL;
> >                 pr_notice("Couldn't request gpio for Delta NAND
> >                 ready.\n");
> 
> dev_notice() ?

Sure, but maybe in a separate patch? That's not a new code just being added 
but an existing one, not the merit of the change.

> >         }
> > 
> > +err_gpiod:
> > +       if (err == -ENODEV || err == -ENOENT)
> > +               err = -EPROBE_DEFER;
> 
> Hmm...

Amstrad Delta uses gpio-mmio driver. Unfortunatelty that driver is not 
availble before device init phase, unlike other crucial GPIO drivers which are 
initialized earlier, e.g. during the postcore or at latetst the subsys phase. 
Hence, devices which depend on GPIO pins provided by gpio-mmio must either be 
declared late or fail softly so they get another chance of being probed 
succesfully.

I thought of replacing the gpio-mmio platform driver with bgpio functions it 
exports but for now I haven't implemented it, not even shared the idea.

Does it really hurt to return -EPROBE_DEFER if a GPIO pin can't be obtained?

Thanks,
Janusz



Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ