From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Markuss Broks <markuss.broks@gmail.com>
Cc: linux-kernel@vger.kernel.org, Jonathan Cameron <jic23@kernel.org>,
Lars-Peter Clausen <lars@metafoo.de>,
Rob Herring <robh+dt@kernel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
Zhigang Shi <Zhigang.Shi@liteon.com>,
Dmitry Osipenko <dmitry.osipenko@collabora.com>,
Paul Gazzillo <paul@pgazz.com>,
Shreeya Patel <shreeya.patel@collabora.com>,
linux-iio@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH 2/2] iio: light: Add support for AMS TCS3490 light sensor
Date: Fri, 27 Jan 2023 11:09:45 +0200 [thread overview]
Message-ID: <Y9OU2e6YMIUTtto0@smile.fi.intel.com> (raw)
In-Reply-To: <a782a0d9-c47e-4328-774f-6bef78161d81@gmail.com>
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
next prev parent reply other threads:[~2023-01-27 9:09 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-23 23:10 [PATCH 0/2] Add a driver for AMS TCS3490 light sensor Markuss Broks
2023-01-23 23:10 ` [PATCH 1/2] dt-bindings: iio: tcs3490: Add bindings " Markuss Broks
2023-01-24 10:32 ` Krzysztof Kozlowski
2023-01-23 23:10 ` [PATCH 2/2] iio: light: Add support " Markuss Broks
2023-01-24 10:22 ` Andy Shevchenko
2023-01-26 15:54 ` Markuss Broks
2023-01-26 17:41 ` Jonathan Cameron
2023-01-27 9:09 ` Andy Shevchenko [this message]
2023-01-28 17:33 ` Jonathan Cameron
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y9OU2e6YMIUTtto0@smile.fi.intel.com \
--to=andriy.shevchenko@linux.intel.com \
--cc=Zhigang.Shi@liteon.com \
--cc=devicetree@vger.kernel.org \
--cc=dmitry.osipenko@collabora.com \
--cc=jic23@kernel.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=lars@metafoo.de \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=markuss.broks@gmail.com \
--cc=paul@pgazz.com \
--cc=robh+dt@kernel.org \
--cc=shreeya.patel@collabora.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.