> -----Original Message----- > From: Mark Brown [mailto:broonie@xxxxxxxxxx] > Sent: 09 November 2022 11:09 PM > To: Padmanabhan Rajanbabu <p.rajanbabu@xxxxxxxxxxx> > Cc: lgirdwood@xxxxxxxxx; robh+dt@xxxxxxxxxx; > krzysztof.kozlowski+dt@xxxxxxxxxx; s.nawrocki@xxxxxxxxxxx; > perex@xxxxxxxx; tiwai@xxxxxxxx; pankaj.dubey@xxxxxxxxxxx; > alim.akhtar@xxxxxxxxxxx; rcsekar@xxxxxxxxxxx; > aswani.reddy@xxxxxxxxxxx; alsa-devel@xxxxxxxxxxxxxxxx; > devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-samsung- > soc@xxxxxxxxxxxxxxx > Subject: Re: [PATCH 2/6] ASoC: samsung: i2s: configure PSR from sound card > > On Tue, Nov 08, 2022 at 10:53:40AM +0530, Padmanabhan Rajanbabu wrote: > > > > > We can overcome this scenario to an extent if we can get a > > > > flexibility to Configure both PSR as well as RFS. > > > > Why does it make sense for the machine driver to worry about this > > > rather than having the I2S controller driver configure the clock tree? > > > _____ | __ > > | > > | | | | \ > > | > > |CMU| | | \ > > | > > |FSD |- |---|-|--------->| \ _________ _________ > > | > > |___ | | | |op_clk0| | | | | > > | | > > | | | |MUX|----| PSR |----| RFS > > |--cdclk | > > | | | | | |_______| |_______| > > | > > | | |--------->| / > > | > > | | op_clk1 | / > > | > > | | |_ / > > | > > | |___________________________________________| > > | > > |-----> To other FSD SoC Peripherals > > > In FSD I2S, the clock source is not an independent source but a common > > clock source being shared by many IPs in the same domain. > > > Changing the clock tree will impact other IPs in the domain as they > > are dependent on the same source for functionality. > > I'm not sure I follow. Perhaps your diagram is unclear but it looks like PSR and > RFS are both after a mux which appears to select which clock is going to be > used by the I2S controller? The usage by other clocks appears to be > upstream of the mux and dividers. > > > We can understand your point to bring the PSR changes under the I2S > > CPU DAI driver by adding a separate compatible and data for the FSD > > SoC. But If we take the example of existing sound cards such as > > sound/soc/samsung/tm2_wm5110.c, the op_clk is supplied via external > > audio pll to the controller and PLL configuration is taken care by the > > sound card. Since the configuration of PLL is more specific to the tm2 > > platform, it makes use of the flexibility of changing the RFS and BFS > > using the sysclk and clkdiv hooks provided by exynos7-i2s CPU DAI > > along with PLL tuning for precise sampling frequency. > > The big reason for the clocking control (and indeed having a custom machine > driver) with the WM5110 is that it has multiple clocks to control and a good > deal of flexibility with placing them in clock domains and so on which have > power and performance impacts. It's frankly a bit unclear to me if the CPU > I2S controller even needs the bitclock configuring given that the clocks are > being driven by the CODEC there, but regardless it's not clear to me why the > I2S controller would need anything other than the input clock to the block > configuring? > > > Similar to the above example, the choice of clock source under > > discussion is not a limitation of exynos7-i2s controller, but instead > > is a limitation on the FSD SoC. > > By using the proposed change, we can ensure that the exynos CPU DAI > > driver is giving additional hooks similar to existing hooks for BFS, > > RFS and CDCLK direction so that sound cards can use > > snd_soc_dai_set_sysclk and snd_soc_dai_set_clkdiv to customize the > > same. > > I'm still not seeing anything that articulates why pushing the configuration of > the dividers within the block into the machine driver solves a problem here. > Again, what's the upside to configuring clocks that are purely within the > block? Okay, I can understand the reason for de-linking these changes from the machine Driver. I'll post the v2 patches integrating the PSR changes into cpu dai driver. Thanks, Padmanabhan R.