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
