On Mon, Oct 30, 2023 at 11:05:39AM -0500, Pierre-Louis Bossart wrote:
> > +static bool tas2783_readable_register(struct device *dev, unsigned int reg)
> > +{
> > + switch (reg) {
> > + case 0x000 ... 0x080: /* Data port 0. */
> No, this is wrong. All the data port 'standard' registers are "owned" by
> the SoundWire core and handled during the port prepare/configure/bank
> switch routines. Do not use them for regmap.
> In other words, you *shall* only define vendor-specific registers in
> this codec driver.
This seems to come up a moderate amount and is an understandable thing
to do - could you (or someone else who knows SoundWire) perhaps send a
patch for the regmap SoundWire integration which does some validation
here during registration and at least prints a warning?
Also worth noting that the default is going to be that the registers are
readable if the driver doesn't configure anything at all so perhaps at
least for just readability this might be worth revisiting.
> > +static const struct snd_soc_dapm_widget tasdevice_dapm_widgets[] = {
> > + SND_SOC_DAPM_AIF_IN("ASI", "ASI Playback", 0, SND_SOC_NOPM, 0, 0),
> > + SND_SOC_DAPM_AIF_OUT("ASI OUT", "ASI Capture", 0, SND_SOC_NOPM,
> > + 0, 0),
> > + SND_SOC_DAPM_SPK("SPK", NULL),
> > + SND_SOC_DAPM_OUTPUT("OUT"),
> > + SND_SOC_DAPM_INPUT("DMIC")
> > +};
> Can you clarify what "ASI" is?
ASI seems to be a fairly commonly used name in TI devices... In general
naming that corresponds to the datasheet should be fine, especially for
internal only things like this sort of DAPM widget. I'd guess it's
something like Audio Serial Interface but not actually gone and looked.
Attachment:
signature.asc
Description: PGP signature
