Re: [PATCH v2 2/5] mfd: lochnagar: Add support for the Cirrus Logic Lochnagar

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2626 bytes --]

On Mon, Oct 29, 2018 at 11:04:39AM +0000, Lee Jones wrote:
> On Fri, 26 Oct 2018, Mark Brown wrote:

> > There's no *requirement* to provide the data even if you're using the
> > cache (and the cache support is entirely optional), there's just costs
> > to not providing it in terms of what features you can get from the
> > regmap API and the performance of the system.  Not every device is going
> > to be bothered by those costs, many devices don't provide all of the
> > data they could.

> So what do we do in the case where, due to the size of the device, the
> amount of lines required by these tables go from crazy to grotesque?

My recommendation when the tables get in the way of the rest of the
driver is to move them into a separate file that contains only tables,
these get big but they're sitting in a separate file that only contains
big data tables so they're fairly simple to look at (and generate or
whatever) and people working on the active code don't need to look at
them.

> > That doesn't work, we need to know both if the register has a default
> > value and what that value is - there's no great value in only supplying
> > the defaults for registers with non-zero values.

> All registers have a default value.  Why can't we assume that if a
> register is writable and a default value was omitted then the default
> is zero?

There are volatile registers which can't be part of the cache as the
hardware might change the state, and we have things like device
identification registers which are fixed but which we want to read from
the device.  This second set of registers can usually be modelled quite
happily as volatile registers though, we don't tend to read them often.

There's also registers where the user just didn't tell us what's going
on, either through oversight or because there's some code that touches
undocumented test registers at runtime for quirk reasons using a read,
modify write cycle.  We could try to insist that the device drivers
always provide a default or mark things as volatile but that's likely to
be an uphill struggle and more error prone.

> Ah wait!  As I was typing the above, I just had a thought.  We don't
> actually provide a list of writable registers do we?  Only a the
> ability to verify if one is writable (presumably) before a write.

> That's frustrating!

You can provide the writability information either with the function or
with data.  The tables code doesn't currently scale very well to large,
sparse register maps but that could in theory be optimized.  Few devices
bother providing distinct writability information as it's not often an
issue.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]