On 05/09/2023 04:46, wangweidong.a@awinic.com wrote:
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?
Maybe?
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
Hm. So you load user-space provided file and assign it directly, without any validation (aw88395_dev_load_acf_check() checks only for magic), to a kernel structure. Sounds bullet-proof. Why using known kernel interfaces, better to implement some conf-file-parsing.
Best regards, Krzysztof