Am 07.09.2023 um 15:21 hat Mark Brown geschrieben: > On Thu, Sep 07, 2023 at 06:12:05PM +0200, Joerg Schambacher wrote: > > > + if (pcm512x->tas_device) { > > + /* set necessary PLL coeffs for amp > > + * according to spec only 2x and 4x MCLKs > > + * possible > > + */ > > + ret = regmap_write(pcm512x->regmap, > > + PCM512x_PLL_COEFF_0, 0x01); > > + if (mck_rate > 25000000UL) > > + ret = regmap_write(pcm512x->regmap, > > + PCM512x_PLL_COEFF_1, 0x02); > > + else > > + ret = regmap_write(pcm512x->regmap, > > + PCM512x_PLL_COEFF_1, 0x04); > > I would name this quirk something a bit more specific - it seems likely > that there might be future TASxxxx devices which need different quirks. > If it's a limited range of MCLK multipliers perhaps something about how > the PLL is limited, or just make the quirk data being to specify min/max > for the multiplier? The spec allows a maximum input Clk of 50MHz. Useful for the concerned mode are only clocks with 22.5792/45.1584MHz for the 44.1ksps family rates and 24.576/49.152MHz for the 48ksps. The only thing we need to make sure is to divide the master clock by 2 in case of a MCLK input of >25MHz to use the the same settings hereafter. And yes, we cannot be sure that future devices may require different settings, but I can hardly imagine anything completely different than this approach with the usual audio MCLK frequencies. > > > + if (!pcm512x->tas_device) { > > + ret = regmap_update_bits(pcm512x->regmap, > > + PCM512x_PLL_EN, PCM512x_PLLE, 0); > > + } else { > > + /* leave PLL enabled for amp clocking */ > > + ret = regmap_write(pcm512x->regmap, > > + PCM512x_PLL_EN, 0x01); > > + dev_dbg(component->dev, "Enabling PLL for TAS575x\n"); > > + } > > This is probably a separate quirk? I'm a bit concerned about what's > turning the PLL off here... The PLL should not be used in this specific case where all divisions are just divide-by-2's. Your original code does reflect that and turns the PLL off. It improves jitter and finally audio performance. But in the case of the TAS-devices we even then need the PLL to drive the AMP clocks. My code changes only address the case 'TAS-device and external audio MCLK'. All other constellations like clock slave mode or e.g. 12MHz MCLK input require the PLL anyhow. THe 12MHz clock input even requires the FLEX mode through the PLL using the GPIOs as PLL-in and -out. I hope I could clarify things a bit. --