On Fri, 28 Oct 2022 11:34:16 +0100, Aidan MacDonald wrote: > The AIC needs only the first two clocks: "aic" is a gate that's used > for gating the I2S controller when it's suspended, and "i2s" is the > system clock, from which the bit and frame clocks are derived. Both > clocks are therefore reasonably part of the AIC and should be passed > to the OS. > > But the "ext" and "pll half" clocks are a little more questionable. > It appears these bindings were introduced when the schema was first > converted to YAML, but weren't present in the original .txt binding. > They are intended to be the possible parent clocks of "i2s". > > The JZ4770 actually has three parents for its "i2s" clock, named > "ext", "pll0", and "pll1" in the Linux driver. The JZ4780 has two > parents but it doesn't have a "pll half" clock, instead it has an > "i2s_pll" clock which behaves much differently to the actual > "pll half" clock found on the JZ4740 & JZ4760. And there are other > Ingenic SoCs that share the JZ4780's clock layout, eg, the X1000. > > Therefore, the bindings aren't really adequate for the JZ4770 and > a bit misleading for the JZ4780. Either we should fix the bindings, > or remove them entirely. > > This patch opts to remove the bindings. There is a good case to be > made that "ext" and "pll half" don't belong here because they aren't > directly used by the AIC. They are only used to set the parent of > the "i2s" clock; they have no other effect on the AIC. > > A good way to think of it is in terms of how the AIC constrains > clocks. The AIC can only generate the bit & frame clocks from the > system clock in certain ratios. Setting the sample rate effectively > constrains the frame clock, which, because of the clock dividers > controlled by the AIC, translates to constraints on the "i2s" clock. > Nothing in the AIC imposes a direct constraint on the parents of > the "i2s" clock, and the AIC does not need to enable or disable > the parents directly, so in principle the AIC doesn't need to be > aware of the parent clocks at all. > > The choice of parent clock is still important, but the AIC doesn't > have enough information to apply such constraints itself. The sound > card does have that information because it knows how the AIC is > connected to other components. We need to use other DT mechanisms > to communicate those constraints at the sound card level, instead > of passing the clocks through to the AIC, and inventing ad-hoc ways > to plumb the constraints around behind the scenes. > > Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@xxxxxxxxx> > --- > .../devicetree/bindings/sound/ingenic,aic.yaml | 10 ++-------- > 1 file changed, 2 insertions(+), 8 deletions(-) > Acked-by: Rob Herring <robh@xxxxxxxxxx>
