Re: [PATCH 1/6] ASoC: codecs: aw88166: support changing sample rate and bit width

[Aaron Kling <webgeek1234@gmail.com>]

On Fri, Sep 25, 2026 at 9:00 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Fri, Sep 25, 2026 at 02:47:29AM -0500, Aaron Kling via B4 Relay wrote:
>
> > The aw88166 driver only worked with 32-bit 48kHz streams so far due to
> > the lack of a proper PLL initialization sequence. Fix by selecting all
> > the necessary PLL settings based on what was passed to us by the
> > hw_params/set_fmt ops. This replaces the strange downstream routine
> > that tries two divider modes in sequence.
>
> > -     ret = regmap_read(aw_dev->regmap, AW88166_PLLCTRL2_REG, &reg_val);
> > +     /* PLL divider must be used for 8/16/32 kHz modes */
> > +     ret = regmap_update_bits(aw_dev->regmap, AW88166_PLLCTRL1_REG,
> > +                     ~AW88166_CCO_MUX_MASK, aw88166->cco_mux_value);
>
> We seem to just completely drop all reference to PLLCTRL2 and switch
> over to PLLCTRL1, I'm going to guess that this means that something that
> used to work now no longer works...

To be honest, I only understand the audio handling at a high level.
This was copied from the aw88261 change almost entirely as-is, as
mentioned in the cover letter. I did some spot checks of the data
sheet to make sure changed addresses were the same and what I checked
did match the 88261 changes. But for the hardware specific stuff, I
unfortunately don't know. What I can say is that after this change,
the AYN Odin3 aw88166 firmware blob from the stock android image works
as-is, where-as before it required some binary patches to start a
stream. Implying that the firmware defaults to settings that didn't
match what the os was using.

> > +static int aw88166_hw_params(struct snd_pcm_substream *substream,
> > +     struct snd_pcm_hw_params *params,
> > +     struct snd_soc_dai *dai)
> > +{
> > +     struct snd_soc_component *component = dai->component;
> > +     struct aw88166 *aw88166 = snd_soc_component_get_drvdata(component);
> > +
> > +     case 32:
> > +             aw88166->bck_value = AW88166_I2SBCK_64FS_VALUE;
> > +             break;
> > +     default:
> > +             dev_err(aw88166->aw_pa->dev, "unsupported physical bit width %d\n",
> > +                     params_physical_width(params));
> > +             return -EINVAL;
> > +     }
> > +
> > +     return 0;
> > +}
>
> This doesn't touch the hardware, it just writes state so if hw_params()
> is called while things are powered up nothing is going to take effect
> until the next stream.

Mmm. As mentioned above, this copies another change, so if it's wrong,
there's more places that need fixed. If there's a better way to handle
this, I can certainly update this patch.

Aaron