linux-kernel - Re: [PATCH v1 2/2] gpio: ds4520: Add ADI DS4520 Regulator 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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAHp75VcxscqmLFRKJ8apLsY3Dsih=FMQn65cRGTUSeCZ-qLy9Q@mail.gmail.com>
Date:   Tue, 11 Apr 2023 17:11:15 +0300
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     "Sahin, Okan" <Okan.Sahin@...log.com>
Cc:     Linus Walleij <linus.walleij@...aro.org>,
        Michael Walle <michael@...le.cc>,
        Bartosz Golaszewski <brgl@...ev.pl>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 2/2] gpio: ds4520: Add ADI DS4520 Regulator Support

On Sun, Apr 9, 2023 at 5:25 PM Sahin, Okan <Okan.Sahin@...log.com> wrote:
> >Fri, Apr 07, 2023 at 03:48:25PM +0200, Linus Walleij kirjoitti:
> >> On Wed, Apr 5, 2023 at 3:57 PM Michael Walle <michael@...le.cc> wrote:

...

> >> If the chip is *only* doing GPIO and nvram it can be a GPIO-only
> >> device I think.

I have read a datasheet, it has the pre-boot settings, but it doesn't
matter from the Linux point of view. The only thing which we need to
take care of is to have the EEPROM disabled during GPIO interaction.
However, there is a question as to how this device should actually
commit into EEPROM the state to survive the cold/warm power cycle.
What is the persistent flag for, btw, I haven't been familiar with it?

> >> The precedent is a ton of ethernet drivers with nvram for storing e.g.
> >> the MAC address. We don't make all of those into MFDs, as the nvram is
> >> closely tied to the one and only function of the block.
> >
> >I agree with Linus. This should be part of the actual (main) driver for the chip as many
> >do (like USB to serial adapters that have GPIO capability).
> >Also this code lacks of proper locking and has style issues.

...

> Could you give more detail related to locking and style issues?

You use a few IO accessors in a row without locking, which means at
any point of this flow some other CPUs may access the chip using the
same or other APIs of this driver.

-- 
With Best Regards,
Andy Shevchenko