linux-kernel - Re: [PATCH 2/2] arm64: dts: qcom: x1e80100-crd: Drop duplicate DMIC supplies

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6vfrlwir6sfommhn3met6wnjm76lnnxw4rdwzq75b7lzcy4jep@2cbcfvb3tvr2>
Date: Wed, 8 Jan 2025 17:07:47 -0600
From: Bjorn Andersson <andersson@...nel.org>
To: Stephan Gerhold <stephan.gerhold@...aro.org>
Cc: Konrad Dybcio <konrad.dybcio@....qualcomm.com>, 
	Srinivas Kandagatla <srinivas.kandagatla@...aro.org>, Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>, 
	Konrad Dybcio <konradybcio@...nel.org>, Rob Herring <robh@...nel.org>, 
	Conor Dooley <conor+dt@...nel.org>, linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Abel Vesa <abel.vesa@...aro.org>, 
	Johan Hovold <johan@...nel.org>
Subject: Re: [PATCH 2/2] arm64: dts: qcom: x1e80100-crd: Drop duplicate DMIC
 supplies

On Thu, Dec 05, 2024 at 07:46:28PM +0100, Stephan Gerhold wrote:
> On Thu, Dec 05, 2024 at 06:11:47PM +0100, Konrad Dybcio wrote:
> > On 4.12.2024 9:33 AM, Stephan Gerhold wrote:
> > > On Wed, Dec 04, 2024 at 08:20:15AM +0100, Krzysztof Kozlowski wrote:
> > >> On 03/12/2024 18:44, Stephan Gerhold wrote:
> > >>> The WCD938x codec provides two controls for each of the MIC_BIASn outputs:
> > >>>
> > >>>  - "MIC BIASn" enables an internal regulator to generate the output
> > >>>    with a configurable voltage (qcom,micbiasN-microvolt).
> > >>>
> > >>>  - "VA MIC BIASn" enables "pull-up mode" that bypasses the internal
> > >>>    regulator and directly outputs fixed 1.8V from the VDD_PX pin.
> > >>>    This is intended for low-power VA (voice activation) use cases.
> > >>>
> > >>> The audio-routing setup for the X1E80100 CRD currently specifies both
> > >>> as power supplies for the DMICs, but only one of them can be active
> > >>> at the same time. In practice, only the internal regulator is used
> > >>> with the current setup because the driver prefers it over pull-up mode.
> > >>>
> > >>> Make this more clear by dropping the redundant routes to the pull-up
> > >>> "VA MIC BIASn" supply. There is no functional difference except that we
> > >>> skip briefly switching to pull-up mode when shutting down the microphone.
> > >>>
> > >>> Fixes: 4442a67eedc1 ("arm64: dts: qcom: x1e80100-crd: add sound card")
> > >>
> > >> If there is no functional difference and this is just redundant, then
> > >> there is nothing to fix, so drop the tag. But the point is that users
> > >> might want the low-power VA. You claim they don't want... sure, I am
> > >> fine with that but there is nothing to fix in such case.
> > >>
> > > 
> > > The fix here is that two mutually exclusive power supplies for the DMIC
> > > are specified in the device tree. You can only have one of them active
> > > at the same time. The Linux driver handles that gracefully, but the
> > > device tree is still wrong and IMO deserves a fixes tag.
> > > 
> > > The functional difference is that we skip briefly switching to pull-up
> > > mode when shutting down the microphone. Users won't notice that, but
> > > it's not the intended behavior.
> > > 
> > > I don't claim that users don't want to switch to the low-power pull-up
> > > mode (VA MIC BIASn). However, we would need a different mechanism to
> > > give them the option to switch at runtime. "audio-routing" just
> > > specifies static routes, so the current description does not allow
> > > switching between the two modes either.
> > 
> > Is there no existing mechanism to alter this at runtime?
> > 
> 
> I don't think so... Since it's currently exposed as two separate DAPM
> supplies (instead of a mux or similar) you can only choose between one
> of them in the static routes specified by "audio-routing" in the DT.
> 
> I tried looking at how downstream handles this, but this left me even
> more confused than I was before. :-) On CRD we currently have the
> following routes in DT:
> 
> 	"VA DMIC0", "MIC BIAS3",
> 	"VA DMIC1", "MIC BIAS3",
> 	"VA DMIC2", "MIC BIAS1",
> 	"VA DMIC3", "MIC BIAS1",
> 	"VA DMIC0", "VA MIC BIAS3",
> 	"VA DMIC1", "VA MIC BIAS3",
> 	"VA DMIC2", "VA MIC BIAS1",
> 	"VA DMIC3", "VA MIC BIAS1",
> 
> MIC BIAS and VA MIC BIAS are mutually exclusive, so this is not correct.
> But if you look at e.g. SM8550 downstream they have:
> 
> 	"TX DMIC0", "MIC BIAS3",
> 	"TX DMIC1", "MIC BIAS3",
> 	"TX DMIC2", "MIC BIAS1",
> 	"TX DMIC3", "MIC BIAS1",
> 	"VA DMIC0", "VA MIC BIAS3",
> 	"VA DMIC1", "VA MIC BIAS3",
> 	"VA DMIC2", "VA MIC BIAS1",
> 	"VA DMIC3", "VA MIC BIAS1";
> 
> Note the TX DMIC vs VA DMIC. So they specify one of the supplies for the
> TX macro DMIC, and the low-power one for the VA macro DMIC. That would
> be fine.
> 
> Now the question is: If we can use the DMIC through both the TX and the
> VA macro, and we're not doing voice activation, why are we using the VA
> macro in the first place?
> 
> @Srini: Do you remember why?
> 

What's the verdict regarding this?

Regards,
Bjorn