On Wed, 2022-12-14 at 13:02 +0100, Krzysztof Kozlowski wrote: > On 13/12/2022 16:06, Trevor Wu (吳文良) wrote: > > On Mon, 2022-12-12 at 09:40 +0100, Krzysztof Kozlowski wrote: > > > On 12/12/2022 03:43, Trevor Wu (吳文良) wrote: > > > > > > > > > > > > > > + uniqueItems: true > > > > > > > > + items: > > > > > > > > + minimum: 0 > > > > > > > > + maximum: 15 > > > > > > > > + > > > > > > > > + "^mediatek,etdm-in[1-2]-mclk-always-on-rate-hz$": > > > > > > > > + description: Specify etdm in mclk output rate for > > > > > > > > always > > > > > > > > on > > > > > > > > case. > > > > > > > > > > > > > > How is it different than assigned-clock-rates? > > > > > > > > > > > > > > > > > > > This includes clock enabling at init stage. > > > > > > > > > > assigned-clock-rates are also at init stage. I asked what is > > > > > different. > > > > > > > > > > > > > If the property is used, there are three parts included in dai > > > > driver > > > > probe function. > > > > > > > > 1. set clock parent (which APLL) > > > > 2. set clock rate (MCLK rate) > > > > 3. enable clock (MCLK On) > > > > > > > > The first two parts can be done by existing dts clock > > > > properties, > > > > but > > > > the last one can't. > > > > When MCLK should be enabled at boot time and kept on, this > > > > property > > > > is used. That's why I say the property is designed for always- > > > > on > > > > case. > > > > > > Heh, so the "always on case" means this property enables clock? > > > How > > > is > > > this even DT property? That's not how clocks should be kept > > > enabled. > > > You > > > need proper clock provider and consumer. > > > > > > > > > > Hi Krzysztof, > > > > Sorry, I don't know it is not appropriate to notify driver that the > > clock should be ketp enabled after boot. > > > > The original idea is that enabling this clock in the machine > > driver, > > but a property to inform machine driver is also required when the > > machine driver is shared by different codec combination. And it's > > easier to handle set_rate and set_parent in etdm dai driver, so I > > put > > the property here. > > > > Do you mean if the clock consumer(audio codec or external DSP) > > requries > > the clock, the consumer should enable the clock by itself? > > Yes, your clocks should have consumers and they keep the clock > enabled > when needed. Certain clocks can be marked as IGNORE or CRITICAL to > keep > enabled without consumers (or even when consumers disable), but > that's > still not a DT property. > > Hi Krzysztof, Got it. If the implementation is not suggested, I will drop the property in V4 and ask consumer to use existing clock property with clock control API instead when we have such case. Thanks, Trevor >