Re: [PATCH 2/2] ASoC: codecs: aw88166: Support device specific firmware

[Krzysztof Kozlowski]

On 11/03/2026 17:30, Aaron Kling wrote:
> On Wed, Mar 11, 2026 at 8:15 AM Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote:
>>
>> On Tue, Mar 10, 2026 at 11:43:05PM -0500, Aaron Kling wrote:
>>> From: Teguh Sobirin <teguh@xxxxxxxx>
>>>
>>> This driver currently loads firmware from a hardcoded path. Support
>>> loading device specific firmware when provided by the boot firmware.
>>>
>>> Signed-off-by: Teguh Sobirin <teguh@xxxxxxxx>
>>> Co-authored-by: Aaron Kling <webgeek1234@xxxxxxxxx>
>>
>> There is no such tag.
> 
> What do you mean? This tag is used all [0] over [1] the kernel [2].

git grep for it.

Other references are mistakes.

> How else is one supposed to indicate that that notable changes have
> been made since the original author touched it?

The tag which is already documented and explained in submitting patches.

> 
>> Also, incomplete DCO chain.
> 
> Ack, will fix. And somehow b4 didn't catch it, huh. Pretty sure I've
> seen it catch this elsewhere, though.
> 
>>> ---
>>>  sound/soc/codecs/aw88166.c | 19 +++++++++++++++----
>>>  1 file changed, 15 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/sound/soc/codecs/aw88166.c b/sound/soc/codecs/aw88166.c
>>> index daee4de9e3b01fb335975a65456cc79575533d7e..52d33a2f7cb12877138ea5083ad42e2777f8d323 100644
>>> --- a/sound/soc/codecs/aw88166.c
>>> +++ b/sound/soc/codecs/aw88166.c
>>> @@ -1574,18 +1574,22 @@ static int aw88166_dev_init(struct aw88166 *aw88166, struct aw_container *aw_cfg
>>>  static int aw88166_request_firmware_file(struct aw88166 *aw88166)
>>>  {
>>>       const struct firmware *cont = NULL;
>>> +     const char *fw_name;
>>>       int ret;
>>>
>>>       aw88166->aw_pa->fw_status = AW88166_DEV_FW_FAILED;
>>>
>>> -     ret = request_firmware(&cont, AW88166_ACF_FILE, aw88166->aw_pa->dev);
>>> +     if (device_property_read_string(aw88166->aw_pa->dev, "firmware-name", &fw_name) < 0)
>>> +             fw_name = AW88166_ACF_FILE;
>>> +
>>> +     ret = request_firmware(&cont, fw_name, aw88166->aw_pa->dev);
>>>       if (ret) {
>>> -             dev_err(aw88166->aw_pa->dev, "request [%s] failed!\n", AW88166_ACF_FILE);
>>> +             dev_err(aw88166->aw_pa->dev, "request [%s] failed!\n", fw_name);
>>>               return ret;
>>>       }
>>>
>>>       dev_dbg(aw88166->aw_pa->dev, "loaded %s - size: %zu\n",
>>> -                     AW88166_ACF_FILE, cont ? cont->size : 0);
>>> +                     fw_name, cont ? cont->size : 0);
>>>
>>>       aw88166->aw_cfg = devm_kzalloc(aw88166->aw_pa->dev,
>>>                       struct_size(aw88166->aw_cfg, data, cont->size), GFP_KERNEL);
>>> @@ -1599,7 +1603,7 @@ static int aw88166_request_firmware_file(struct aw88166 *aw88166)
>>>
>>>       ret = aw88395_dev_load_acf_check(aw88166->aw_pa, aw88166->aw_cfg);
>>>       if (ret) {
>>> -             dev_err(aw88166->aw_pa->dev, "load [%s] failed!\n", AW88166_ACF_FILE);
>>> +             dev_err(aw88166->aw_pa->dev, "load [%s] failed!\n", fw_name);
>>>               return ret;
>>>       }
>>>
>>> @@ -1802,9 +1806,16 @@ static const struct i2c_device_id aw88166_i2c_id[] = {
>>>  };
>>>  MODULE_DEVICE_TABLE(i2c, aw88166_i2c_id);
>>>
>>> +static const struct of_device_id aw88166_of_match[] = {
>>> +     { .compatible = "awinic,aw88166" },
>>> +     { /* sentinel */ }
>>> +};
>>> +MODULE_DEVICE_TABLE(of, aw88166_of_match);
>>
>> This looks like an unrelated change.
> 
> Without this, I don't think the driver will read from the dt node at

Then how would it probe? If it probes based on DT node, then it matches
the node as well.

> all. Since this change requires doing so, I figured it was related
> enough. But I can split that if desired.



Best regards,
Krzysztof