> -----Original Message----- > From: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx> > Sent: Monday, February 27, 2023 3:38 PM > To: Lee, RyanS <RyanS.Lee@xxxxxxxxxx>; Mark Brown > <broonie@xxxxxxxxxx> > Cc: “Ryan <ryan.lee.analog@xxxxxxxxx>; lgirdwood@xxxxxxxxx; > tiwai@xxxxxxxx; krzysztof.kozlowski@xxxxxxxxxx; rf@xxxxxxxxxxxxxxxxxxxxx; > ckeepax@xxxxxxxxxxxxxxxxxxxxx; herve.codina@xxxxxxxxxxx; > wangweidong.a@xxxxxxxxxx; james.schulman@xxxxxxxxxx; > ajye_huang@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx; shumingf@xxxxxxxxxxx; > povik+lin@xxxxxxxxxxx; flatmax@xxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > alsa-devel@xxxxxxxxxxxxxxxx; robh+dt@xxxxxxxxxx; > devicetree@xxxxxxxxxxxxxxx > Subject: Re: [PATCH 1/2] ASoC: max98363: add soundwire amplifier driver > > [External] > > > >>> 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. > > > > Thanks for the comment. > > The only reason I added standard SoundWire registers to the amp driver > > is to check the values for the debugging purpose because these > > registers values are important to understand the device status, but it > > is not visible from the regmap debugfs if those registers are not included > on the regmap table of the driver. > > The driver never controls the standard SoundWire registers by itself. > > Do you recommend removing the standard SoundWire registers from the > > driver or keeping it non-volatile? > > (The reg_default values in the table are all amp reset values and > > those registers are treated as volatile. I shall clear 'unique ID' > > field because it is determined by the hardware pin connection.) > > We already have debugfs support for those registers, see > sdw_slave_reg_show() in drivers/soundwire/debugfs.c > > It's not the same file as regmap debugfs but the information is already there, > see e.g. an example on the SOF CI devices: > > cd /sys/kernel/debug/soundwire/master-0-1/sdw:1:025d:0700:00 > more registers > > Register Value > > DP0 > 0 0 > 1 0 > 2 0 > 3 0 > 4 0 > 5 1 > Bank0 > 20 0 > 22 0 > 23 0 > 24 0 > 25 0 > 26 0 > 27 XX > 28 XX > Bank1 > 30 0 > 32 0 > 33 0 > 34 0 > 35 0 > 36 0 > 37 XX > 38 XX > > SCP > 40 0 > 41 7 > 42 0 > 43 0 > 44 20 > 45 9 > 46 4 > 47 XX > 48 XX > 49 XX > 4a XX > 4b XX > 50 10 > 51 2 > 52 5d > 53 7 > 54 0 > 55 0 > > DP1 > 100 0 > 101 0 > 102 0 Thank you for the useful information. Then, there is no reason to keep standard registers in the driver. I shall remove stand registers from the amp register map.
