linux-kernel - Re: [PATCH 3/3] ASoC: codecs: add new pm4125 audio codec driver

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: <DB8GFDXKQ6V1.BXX5KGBJP6YS@linaro.org>
Date: Thu, 10 Jul 2025 15:45:15 +0100
From: "Alexey Klimov" <alexey.klimov@...aro.org>
To: "Mark Brown" <broonie@...nel.org>
Cc: "Srinivas Kandagatla" <srini@...nel.org>, "Liam Girdwood"
 <lgirdwood@...il.com>, "Rob Herring" <robh@...nel.org>, "Krzysztof
 Kozlowski" <krzk+dt@...nel.org>, "Conor Dooley" <conor+dt@...nel.org>,
 "Stephen Boyd" <sboyd@...nel.org>, "Lee Jones" <lee@...nel.org>, "Jaroslav
 Kysela" <perex@...ex.cz>, "Takashi Iwai" <tiwai@...e.com>,
 <linux-arm-msm@...r.kernel.org>, <linux-sound@...r.kernel.org>,
 <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>, "Dmitry
 Baryshkov" <dmitry.baryshkov@....qualcomm.com>, "Srinivas Kandagatla"
 <srinivas.kandagatla@....qualcomm.com>
Subject: Re: [PATCH 3/3] ASoC: codecs: add new pm4125 audio codec driver

On Tue Jul 1, 2025 at 10:04 PM BST, Mark Brown wrote:
> On Tue, Jul 01, 2025 at 08:35:42PM +0100, Alexey Klimov wrote:
>> On Thu Jun 26, 2025 at 12:56 PM BST, Mark Brown wrote:
>> > On Thu, Jun 26, 2025 at 12:50:31AM +0100, Alexey Klimov wrote:
>
>> >> +static int pm4125_micbias_control(struct snd_soc_component *component,
>> >> +				  int micb_num, int req, bool is_dapm)
>> >> +{
>> >> +	return 0;
>> >> +}
>
>> > Why have this empty function which is only called from within the
>> > driver?  At best it's making the callers look like they do something.
>
>> I tried to make a minimal working version that we're going to
>> update with more patches during next submission.
>
> Add the callers when you need them, right now this is just noise.
> Nobody can tell if the callers make sense since the function does
> nothing.

Ok, I cleaned it for the next version. Thanks.

>> >> +#if defined(CONFIG_OF)
>> >> +static const struct of_device_id pm4125_of_match[] = {
>> >> +	{ .compatible = "qcom,pm4125-codec" },
>> >> +	{ }
>> >> +};
>> >> +MODULE_DEVICE_TABLE(of, pm4125_of_match);
>> >> +#endif
>
>> > Why does this compatible exist?  If the driver is instantiated from a
>> > as a Linux software contruct it shouldn't appear in the DT.
>
>> Could you please elaborate a bit more? Should it be instantiated
>> as an MFD device or platform device?
>
> Yes, if it's the child of a MFD then it shouldn't need to be described
> separately in the DT.

Currently, it is going to be described as child/slave device:

spmi_bus {
	pmic@0 {
		pmic4125_codec: codec {
			...
		}
and will go probably in pm4125.dtsi which lists all child nodes with
compatibles. Not sure if it is because each PMIC is customazable or because
of better maintainability.
Also, might need specific description of regulators which may vary from
board to board. Not sure how is that supposed to be done without device
tree description at this point.

Thanks,
Alexey