linux-kernel - RE: [PATCH 1/2] ASoC: max98363: add soundwire amplifier driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <SJ0PR03MB6681558502B972EE6651FA2D8ABA9@SJ0PR03MB6681.namprd03.prod.outlook.com>
Date:   Fri, 10 Mar 2023 01:37:27 +0000
From:   "Lee, RyanS" <RyanS.Lee@...log.com>
To:     Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
        Mark Brown <broonie@...nel.org>
CC:     “Ryan <ryan.lee.analog@...il.com>,
        "lgirdwood@...il.com" <lgirdwood@...il.com>,
        "tiwai@...e.com" <tiwai@...e.com>,
        "krzysztof.kozlowski@...aro.org" <krzysztof.kozlowski@...aro.org>,
        "rf@...nsource.cirrus.com" <rf@...nsource.cirrus.com>,
        "ckeepax@...nsource.cirrus.com" <ckeepax@...nsource.cirrus.com>,
        "herve.codina@...tlin.com" <herve.codina@...tlin.com>,
        "wangweidong.a@...nic.com" <wangweidong.a@...nic.com>,
        "james.schulman@...rus.com" <james.schulman@...rus.com>,
        "ajye_huang@...pal.corp-partner.google.com" 
        <ajye_huang@...pal.corp-partner.google.com>,
        "shumingf@...ltek.com" <shumingf@...ltek.com>,
        "povik+lin@...ebit.org" <povik+lin@...ebit.org>,
        "flatmax@...tmax.com" <flatmax@...tmax.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "alsa-devel@...a-project.org" <alsa-devel@...a-project.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: RE: [PATCH 1/2] ASoC: max98363: add soundwire amplifier driver

> -----Original Message-----
> From: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
> Sent: Monday, February 27, 2023 3:38 PM
> To: Lee, RyanS <RyanS.Lee@...log.com>; Mark Brown
> <broonie@...nel.org>
> Cc: “Ryan <ryan.lee.analog@...il.com>; lgirdwood@...il.com;
> tiwai@...e.com; krzysztof.kozlowski@...aro.org; rf@...nsource.cirrus.com;
> ckeepax@...nsource.cirrus.com; herve.codina@...tlin.com;
> wangweidong.a@...nic.com; james.schulman@...rus.com;
> ajye_huang@...pal.corp-partner.google.com; shumingf@...ltek.com;
> povik+lin@...ebit.org; flatmax@...tmax.com; linux-kernel@...r.kernel.org;
> alsa-devel@...a-project.org; robh+dt@...nel.org;
> devicetree@...r.kernel.org
> 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.