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

[Aaron Kling]

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].
How else is one supposed to indicate that that notable changes have
been made since the original author touched it?

> 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
all. Since this change requires doing so, I figured it was related
enough. But I can split that if desired.

Aaron

[0] https://lore.kernel.org/all/20220107160636.6555-3-ckeepax@xxxxxxxxxxxxxxxxxxxxx/
[1] https://lore.kernel.org/all/20260115164203.811470351@xxxxxxxxxxxxxxxxxxx/
[2] https://lore.kernel.org/all/f7e05205fd33d9e510ec1295e0cc8cfdf395cb89.1756237895.git.osandov@xxxxxxxxxxx/