On 05/09/2023 04:46, wangweidong.a@xxxxxxxxxx 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
