lkml.org 
[lkml]   [2023]   [Jan]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/3] ASoC: codecs: Add support for the Renesas IDT821034 codec
Hi Mark,

On Wed, 11 Jan 2023 17:57:01 +0000
Mark Brown <broonie@kernel.org> wrote:

> On Wed, Jan 11, 2023 at 05:40:22PM +0100, Herve Codina wrote:
> > Mark Brown <broonie@kernel.org> 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

\
 
 \ /
  Last update: 2023-03-26 23:39    [W:1.006 / U:1.348 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site