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

From: "Trevor Wu (吳文良)" <Trevor.Wu@mediatek.com>
To: "robh+dt@kernel.org" <robh+dt@kernel.org>,
	"krzysztof.kozlowski@linaro.org" <krzysztof.kozlowski@linaro.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: Mon, 12 Dec 2022 02:43:47 +0000	[thread overview]
Message-ID: <e08fab2a694313abc12e0f8303de3e2e704b388c.camel@mediatek.com> (raw)
In-Reply-To: <fb34e446-2904-a357-e54b-3e82a2b0745c@linaro.org>

On Fri, 2022-12-09 at 15:56 +0100, Krzysztof Kozlowski wrote:
> On 09/12/2022 11:56, Trevor Wu (吳文良) wrote:
> > > > +
> > > > +patternProperties:
> > > > +  "^mediatek,etdm-in[1-2]-chn-disabled$":
> > > > +    $ref: /schemas/types.yaml#/definitions/uint8-array
> > > > +    minItems: 1
> > > > +    maxItems: 16
> > > > +    description:
> > > > +      By default, all data received from ETDM pins will be
> > > > outputed to
> > > > +      memory. etdm in supports disable_out in direct mode(w/o
> > > > interconn).
> > > > +      User can specify the channel ID which they hope dropping
> > > > and
> > > > then
> > > > +      the specified channel won't be seen on memory.
> > > 
> > > So we know what are the IDs but it's a mystery what are the
> > > values.
> > > Especially with unique values - how any of these should case that
> > > channel "won't be seen in memory"?
> > > 
> > 
> > For example,
> > FE can support 14ch, but BE(etdm) can't support 14ch(it can support
> > 16ch).
> > In the case, we can configure 16ch to etdm and make use of the
> > property
> > to disable the last two channels.
> > 
> > mediatek,etdm-in1-chn-disabled = /bits/ 8 <0xE 0xF>;
> 
> Your description should explain that this is a list of channel IDs
> which
> should be disabled.
> 
Hi Krzysztof,

Thanks for your suggestion.
I'll put it into the description.

> > 
> > 
> > > > +    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.

Thanks,
Trevor.
> > 
> > > > +
> > > > +  "^mediatek,etdm-out[1-3]-mclk-always-on-rate-hz$":
> > > > +    description: Specify etdm out mclk output rate for always
> > > > on
> > > > case.
> > > > +
> > > > +  "^mediatek,etdm-in[1-2]-multi-pin-mode$":
> > > > +    type: boolean
> > > > +    description: if present, the etdm data mode is I2S.
> > > > +
> > > > +  "^mediatek,etdm-out[1-3]-multi-pin-mode$":
> > > > +    type: boolean
> > > > +    description: if present, the etdm data mode is I2S.
> > > > +
> > > > +  "^mediatek,etdm-in[1-2]-cowork-source$":
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > +    description:
> > > > +      etdm modules can share the same external clock pin.
> > > > Specify
> > > > +      which etdm clock source is required by this etdm in
> > > > moudule.
> > > 
> > > typo: module
> > > 
> 
> Best regards,
> Krzysztof
> 