linux-kernel - Re: [PATCH 3/3] ASoC: aw88395: drop undocumented and unused sound-channel property

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230905024659.15071-1-wangweidong.a@awinic.com>
Date:   Tue,  5 Sep 2023 10:46:59 +0800
From:   wangweidong.a@...nic.com
To:     krzysztof.kozlowski@...aro.org
Cc:     alsa-devel@...a-project.org, broonie@...nel.org,
        lgirdwood@...il.com, linux-kernel@...r.kernel.org, perex@...ex.cz,
        tiwai@...e.com, wangweidong.a@...nic.com, yijiangtao@...nic.com
Subject: Re: [PATCH 3/3] ASoC: aw88395: drop undocumented and unused sound-channel property

Thank you very much for the proposed patch, but I have some 
questions that I would like to discuss with you

On Mon, Sep 04, 2023 at 12:53 PM +0200, Krzysztof Kozlowski wrote:
> The Devicetree sound-channel property was never accepted and is not
> allowed by bindings.  It is not used by any upstream user, thus
> considering that it was never documented, should be dropped.

This node property is intended for use with multiple PA, to
load different configurations for different PA.
Can I add this sound-channel in the "awinic,aw88395.yaml" file?

> Even though it does not look like from the diff, the property is not
> actually used by the driver, because once set, it is read only in loops
> depending on ddt_num (prof_hdr->ddt_num, cfg_hdr->ddt_num).  The
> variable ddt_num is never set and is always 0, so the loops do not have
> any iteration.  Dropping sound-channel and ddt_num-related loops allows
> to drop empty functions which in turn drop quite a lot of code.  This
> entire code was not possible to execute. 

The ddt_num variable is not always 0, this variable is defined
in the configuration file. The "prof_hdr" variable is assigned by 
the "cfg_hdr" variable. The "cfg_hdr" variable is assigned by "aw_cfg"
aw_cfg is the data obtained through request_firmware.The specific 
process is as follows:

request_firmware ---> cont->data ---> aw_cfg->data --> cfg_hdr --> prof_hdr


> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>

---

> Not tested on hardware... but feature removed here could not work
> according if conforming to documented interface.
---
>  sound/soc/codecs/aw88261.c                   |  13 -
>  sound/soc/codecs/aw88395/aw88395_data_type.h |   1 -
>  sound/soc/codecs/aw88395/aw88395_device.c    |  21 -
>  sound/soc/codecs/aw88395/aw88395_device.h    |   1 -
>  sound/soc/codecs/aw88395/aw88395_lib.c       | 788 -------------------
>  5 files changed, 824 deletions(-)

> diff --git a/sound/soc/codecs/aw88261.c b/sound/soc/codecs/aw88261.c
> index a5de7c704e73..e0527ce2f83e 100644
> --- a/sound/soc/codecs/aw88261.c
> +++ b/sound/soc/codecs/aw88261.c
> @@ -1171,17 +1171,6 @@ static void aw88261_hw_reset(struct aw88261 *aw88261)
>  	usleep_range(AW88261_1000_US, AW88261_1000_US + 10);
>  }

...

Best Regards,
Weidong Wang