Re: [PATCH v1 2/5] ASoC: cs35l56: Add struct to index firmware registers
From: Richard Fitzgerald
Date: Wed Apr 09 2025 - 07:34:18 EST
On 08/04/2025 5:00 pm, Mark Brown wrote:
On Tue, Apr 08, 2025 at 01:58:23PM +0100, Richard Fitzgerald wrote:
On 07/04/2025 8:16 pm, Mark Brown wrote:
This is fine but note that this is the use case that the regmap_field_
APIs were created for, that also helps deal with things if anyone is
clever and resizes fields or shifts within registers. It's purely a
question of taste if you want to use that.
The regmap_field stuff looks like a lot of unnecessary complexity and
overhead just for 6 registers with alternate addresses.
Yeah, like I say it's a taste thing.
If you would prefer us to use it we'll make the changes.
If not, then we'd like to keep the code as it is for now and in
future we'll have a look at regmap_field.
(In fact, the regmap_field stuff looks like an over-complex way of
solving a non-problem. The original commit is talking about replacing
masks and shifts "all over the code" to make the code neater. But
really, all those extra structs and pointers and allocated memory just
to replace a logical & or | ? Every struct regmap_field has a pointer
to the struct regmap !!?! So if I've got 100 fields there are 100 copies
of the struct regmap pointer that my driver already has.)
It was for cases where the shifts and widths may also change, there's a
bit more than applying a mask. Like you say it's got some overhead
hence the taste thing.
Yes. If we had registers where the fields were moving around then there
would be better justification for using regmap_field.