Messages in this thread | | | Date | Mon, 27 Feb 2023 18:44:32 +0000 | From | Mark Brown <> | Subject | Re: [PATCH 1/2] ASoC: max98363: add soundwire amplifier driver |
| |
On Mon, Feb 27, 2023 at 01:19:15PM -0500, Pierre-Louis Bossart wrote: > On 2/27/23 12:47, Mark Brown wrote: > > On Mon, Feb 27, 2023 at 10:17:45AM -0500, Pierre-Louis Bossart wrote:
> >> That seems wrong, why would you declare standard registers that are > >> known to the bus and required to be implemented?
> > This is the register defaults table, it gets used to initialise the > > register cache and optimise resync after suspend - all this does is > > supply defaults for the cache. That said...
> > I would suggest it's better to not supply defaults for ID registers and > > read them back from the device otherwise things might get confused.
> The 'device_id' register is the good counter example: it includes a > 'unique_id' field to deal with cases where there are identical devices > on the same link. The unique_id is usually set with board-specific > pin-strapping, so there's no good default value here. In previous Maxim > 98373 amplifier configurations the unique IDs were 3 and 7 IIRC. The > codec driver should not, rather shall not, assume any specific value here.
Yes, as I said above ID registers in particular are often better off handled as volatile even ignoring any potential for them to show variable configuration information.
> > ...if there's an issue with the SoundWire core modifying the registers > > directly then the driver would need to mark all the core registers as > > volatile so that they're not cached otherwise there will be collisions. > > Or is it the case that we always need to go via the SoundWire core for > > the generic registers, so they should just never be written at all?
> It's really that the SoundWire core will 'own' or take care of all > 'standard' programming registers. There is no good reason for a codec > driver to interfere with standard port programming or clock stop. The > bus provides a set of callbacks that can be used for vendor-specific > registers and sequences.
> Put differently, SoundWire codec drivers should only deal with > non-standard vendor-specific registers.
OK, it'd be good to be clear about what the issue is when reviewing things. The registers *are* in the device's register map but the driver shouldn't be referencing them at all and should instead be going via the SoundWire core for anything in there. [unhandled content-type:application/pgp-signature] | |