Re: [PATCH 1/2] ASoC: max98363: add soundwire amplifier driver

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

>>> 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