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: <Y9OU2e6YMIUTtto0@smile.fi.intel.com>
Date:   Fri, 27 Jan 2023 11:09:45 +0200
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     Markuss Broks <markuss.broks@...il.com>
Cc:     linux-kernel@...r.kernel.org, Jonathan Cameron <jic23@...nel.org>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Zhigang Shi <Zhigang.Shi@...eon.com>,
        Dmitry Osipenko <dmitry.osipenko@...labora.com>,
        Paul Gazzillo <paul@...zz.com>,
        Shreeya Patel <shreeya.patel@...labora.com>,
        linux-iio@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH 2/2] iio: light: Add support for AMS TCS3490 light sensor

On Thu, Jan 26, 2023 at 05:54:56PM +0200, Markuss Broks wrote:
> On 1/24/23 12:22, Andy Shevchenko wrote:
> > On Tue, Jan 24, 2023 at 01:10:25AM +0200, Markuss Broks wrote:

First of all, I assume you agree on the comments you left unanswered,
so we will expect them all being addressed in the next version. Is it
correct perception?

...

> > > +static const struct regmap_config tcs3490_regmap_config = {
> > > +	.reg_bits	= 8,
> > > +	.val_bits	= 8,
> > Seems you are using regmap internal serialization, but does it guarantee the
> > serialization on the transaction level? Or why is it not a problem?
> Well, other drivers seem to have it this way too.

They all may be buggy, unclear, or hardware there doesn't require transaction
level locks. It means we have to avoid cargo cult.

> I don't really understand
> why it should be a problem, could you please clarify?

Because one should distinguish IO with register vs. IO transaction.
Imaging two threads which do an IO:

	CPU0					CPU1
						read REG2
						update value
	read REG1
						write REG2
	update value
	write REG1

If in our hypothetical example the writing to REG2 has a side effect on
the values in REG1, we are doomed.

You have to check all possible scenarios and tell if it's a problem or not with
the certain hardware. According to the result, add a corresponding comment to
the code and, if required, change the locking scheme.

> > > +};

-- 
With Best Regards,
Andy Shevchenko


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ