Re: [PATCH v3 10/12] dt-bindings: mediatek: mt8188: add audio afe document

From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
To: "Trevor Wu (吳文良)" <Trevor.Wu@mediatek.com>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>,
	"broonie@kernel.org" <broonie@kernel.org>,
	"p.zabel@pengutronix.de" <p.zabel@pengutronix.de>,
	"tiwai@suse.com" <tiwai@suse.com>,
	"lgirdwood@gmail.com" <lgirdwood@gmail.com>,
	"krzysztof.kozlowski+dt@linaro.org"
	<krzysztof.kozlowski+dt@linaro.org>,
	"matthias.bgg@gmail.com" <matthias.bgg@gmail.com>,
	"perex@perex.cz" <perex@perex.cz>
Cc: "linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-mediatek@lists.infradead.org" 
	<linux-mediatek@lists.infradead.org>,
	"angelogioacchino.delregno@collabora.com" 
	<angelogioacchino.delregno@collabora.com>,
	"alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
	Project_Global_Chrome_Upstream_Group 
	<Project_Global_Chrome_Upstream_Group@mediatek.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>
Subject: Re: [PATCH v3 10/12] dt-bindings: mediatek: mt8188: add audio afe document
Date: Wed, 14 Dec 2022 13:02:19 +0100	[thread overview]
Message-ID: <dd2a373e-d3a7-cec9-6608-379d61f4228f@linaro.org> (raw)
In-Reply-To: <6eae5d7eef22443b6152c6655c89bdfd70e59089.camel@mediatek.com>

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.

Best regards,
Krzysztof