linux-kernel - Re: [PATCH 2/2] iio: adc: ad717x: add AD717X driver

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 for Android: free password hash cracker in your pocket

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

Date:   Thu, 10 Aug 2023 18:36:36 +0300
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     Nuno Sá <noname.nuno@...il.com>
Cc:     Dumitru Ceclan <mitrutzceclan@...il.com>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Michael Hennerich <Michael.Hennerich@...log.com>,
        Jonathan Cameron <jic23@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Arnd Bergmann <arnd@...db.de>,
        Cosmin Tanislav <demonsingur@...il.com>,
        Alexander Sverdlin <alexander.sverdlin@...il.com>,
        Hugo Villeneuve <hvilleneuve@...onoff.com>,
        Okan Sahin <okan.sahin@...log.com>,
        Niklas Schnelle <schnelle@...ux.ibm.com>,
        ChiYuan Huang <cy_huang@...htek.com>,
        Ramona Bolboaca <ramona.bolboaca@...log.com>,
        Ibrahim Tilki <Ibrahim.Tilki@...log.com>,
        ChiaEn Wu <chiaen_wu@...htek.com>,
        William Breathitt Gray <william.gray@...aro.org>,
        Lee Jones <lee@...nel.org>, Haibo Chen <haibo.chen@....com>,
        Mike Looijmans <mike.looijmans@...ic.nl>,
        Leonard Göhrs <l.goehrs@...gutronix.de>,
        Ceclan Dumitru <dumitru.ceclan@...log.com>,
        linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] iio: adc: ad717x: add AD717X driver

On Thu, Aug 10, 2023 at 01:57:02PM +0200, Nuno Sá wrote:
> On Thu, 2023-08-10 at 12:33 +0300, Dumitru Ceclan wrote:

...

> Is ad717x_gpio_cleanup() being used anywhere? Moreover I would maybe just get rid of
> the #ifdef wrapper and just select GPIOLIB. How often will it be disabled anyways?

The agreement is that users are depend on and not selecting GPIOLIB.
Any news in these agreement terms?

...

> > +       id &= AD717X_ID_MASK;
> > +       if (id != st->info->id)
> > +               dev_warn(&st->sd.spi->dev, "Unexpected device id: %x, expected:
> > %x\n",
> > +                                           id, st->info->id);
> > +
> 
> Shouldn't we error out?

It seems a new way of thinking about unsupported CHIP ID. Dunno if hw vendors
won't ever do a dirty trick that new ID must be programmed differently and
otherwise burn hardware to a smoke...

I'm with you here, unknown chips mustn't be supported.

...

> > +                               *val = -(1 << (chan->scan_type.realbits - 1));
> 
> nit: I don't expect the driver to really be updated with more devices (it's
> like this for a long time) but the above is not very extensible... Imagine we
> add a device with 32bit channels? We would enter shady waters If I'm not
> missing anything.

Also 1 << 31 is UB in accordance with C standard.

...

> > +       st->info = device_get_match_data(&spi->dev);
> > +       if (!st->info)
> > +               return -ENODEV;
> > +
> 
> spi_get_device_match_data() (not really sure if this is still applicable
> since some work related to this is being done for i2c - and eventually in spi
> I guess).

Still applicable.

-- 
With Best Regards,
Andy Shevchenko