linux-kernel - Re: [PATCH v6 5/5] mfd: max77541: Add ADI MAX77541/MAX77540 PMIC 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: <be16ef0f88e74e3dc25d6ba08f384eb182ddd240.camel@gmail.com>
Date:   Thu, 30 Mar 2023 09:43:32 +0200
From:   Nuno Sá <noname.nuno@...il.com>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        "Sahin, Okan" <Okan.Sahin@...log.com>
Cc:     Mark Brown <broonie@...nel.org>, Lee Jones <lee@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        Jonathan Cameron <jic23@...nel.org>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Cosmin Tanislav <demonsingur@...il.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Caleb Connolly <caleb.connolly@...aro.org>,
        Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>,
        "Bolboaca, Ramona" <Ramona.Bolboaca@...log.com>,
        ChiYuan Huang <cy_huang@...htek.com>,
        "Tilki, Ibrahim" <Ibrahim.Tilki@...log.com>,
        William Breathitt Gray <william.gray@...aro.org>,
        Arnd Bergmann <arnd@...db.de>,
        ChiaEn Wu <chiaen_wu@...htek.com>,
        Haibo Chen <haibo.chen@....com>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>
Subject: Re: [PATCH v6 5/5]  mfd: max77541: Add ADI MAX77541/MAX77540 PMIC
 Support

On Wed, 2023-03-29 at 17:11 +0300, Andy Shevchenko wrote:
> On Wed, Mar 29, 2023 at 05:08:44PM +0300, Andy Shevchenko wrote:
> > On Tue, Mar 28, 2023 at 04:01:21PM +0000, Sahin, Okan wrote:
> > > > On Tue, Mar 28, 2023 at 04:51:18PM +0200, Nuno Sá wrote:
> > > > > On Tue, 2023-03-28 at 17:35 +0300, Andy Shevchenko wrote:
> > > > > > On Tue, Mar 28, 2023 at 04:18:30PM +0200, Nuno Sá wrote:
> > > > > > > On Tue, 2023-03-28 at 14:46 +0100, Mark Brown wrote:
> > > > > > > > On Tue, Mar 28, 2023 at 03:26:44PM +0200, Nuno Sá
> > > > > > > > wrote:
> > > > > > > > 
> > > > > > > > > IIRC, regmap_read() is not really reentrant and it is
> > > > > > > > > used in
> > > > > > > > > the IIO driver on the sysfs interface. So, yeah, I
> > > > > > > > > think you
> > > > > > > > > need the regmap lock and better just leave the config
> > > > > > > > > as is.
> > > > > > > > > Yes, the lock is
> > > > > > > > > opt-
> > > > > > > > > out
> > > > > > > > > so let's not disable it :)
> > > > > > > > 
> > > > > > > > All the regmap operations are fully thread safe.
> > > > > > > 
> > > > > > > Even if 'config->disable_locking' is set? I think that is
> > > > > > > what's
> > > > > > > being discussed in here...
> > > > > > 
> > > > > > In case the driver has its own lock to serialize IO how on
> > > > > > earth the
> > > > > > regmap lock is needed. That's what I asked the author of
> > > > > > the driver.
> > > > > > He told the code
> > > > > 
> > > > > Well, if the driver has it's own locking, then sure we do not
> > > > > need
> > > > > regmap's lock...
> > > > > 
> > > > > > doesn't require the regmap lock, and I tend to believe the
> > > > > > author.
> > > > > > So, why to
> > > > > > keep it?
> > > > > 
> > > > > However, if you look at the adc driver, I can see plain
> > > > > regmap_read()
> > > > > calls without any "outside" locking.
> > > > 
> > > > Then author of the code should know what they are doing. Right?
> > 
> > > Actually, I do not want to disable regmap lock that's why I did
> > > not update it.
> > 
> > If you have something like 
> > 
> > func1()
> >         regmap_read(reg1)
> >         regmap_read/write(reg2)
> > 

I would also add that as soon as you have more than one access to
regmap (or spi, or i2c...) in one func and that function is accessible
through sysfs (and likely any other userspace interface), then you
should already take care. So, it might be even the case that func1()
and func2() don't have to run in parallel.

- Nuno Sá
>