Hi Mark, On Wed, 11 Jan 2023 17:57:01 +0000 Mark Brown <broonie@xxxxxxxxxx> wrote: > On Wed, Jan 11, 2023 at 05:40:22PM +0100, Herve Codina wrote: > > Mark Brown <broonie@xxxxxxxxxx> wrote: > > > On Wed, Jan 11, 2023 at 02:49:04PM +0100, Herve Codina wrote: > > > > Without knowing why things are written in this way or what it's trying > > > to accomplish it's hard to comment in detail on what specifically should > > > be done. > > > Yes, I use regmap to ease the integration of controls and use the > > already defined controls macros but the device registers do not fit > > well with regmap. > > If this doesn't fit into regmap then don't try to shoehorn it into > regmap, that just makes it incredibly hard to follow what's going on. > > > The device registers are not defined as simple as address/value pairs. > > Accesses contains one or more bytes and the signification of the > > data (and bytes) depends on the first bits. > > - 0b10xxxxxx means 'Control register' with some data as xxxxxx > > and one extra byte > > - 0b1101yyyy means 'Configuration register, slic mode' with > > some other data as yyyy and one extra byte > > - 0b1100zzzz means 'Configuration register, gain mode' with > > some other data as zzzz and two extra bytes > > So really the device only has three registers, each of different sizes > and windowed fields within those registers? I love innovation, > innovation is great and it's good that our hardware design colleagues > work so hard to keep us in jobs. It seems hardly worth it to treat them > as registers TBH. This is so far off a register/value type thing that I > just wouldn't even try. > > > Of course, I can describe all of these in details. > > Where do you want to have this information ? All at the top > > of the file ? Each part (low-level, virtual regs, ...) at > > the beginning of each part in the code ? > > I'm not sure what problem it solves to use regmap or have virtual > registers in the first place. I think you would be better off with > custom _EXT controls, you almost have that anway just hidden in the > middle of the fake register stuff instead of directly there. My sense > is that the result would be much less code. If you are trying to map > things onto registers you probably want comments at every level since > you don't know where people are going to end up jumping into the code. > > Perhaps it's possible to write some new SND_SOC_ helpers that work with > just a value in the device's driver data rather than a regmap and have > a callback to trigger a write to the device? I suspect that'd be > generally useful actually... Well, I wil try to use my own .put() and .get() for snd_controls. For DAPM (struct snd_soc_dapm_widget), no kind of .put() and .get() are available. I will use some Ids for the 'reg' value and use the .write() and .read() hooks available in struct snd_soc_component_driver in order to handle these Ids and so perform the accesses. Do you think this can be the right way (at least for a first try) ? Best regards, Hervé -- Hervé Codina, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
