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.
Attachment:
signature.asc
Description: PGP signature
