Re: [PATCH 03/13] arm64: dts: qcom: msm8916: Add common msm8916-modem-qdsp6.dtsi

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 9/26/23 22:17, Stephan Gerhold wrote:
On Tue, Sep 26, 2023 at 10:01:21PM +0200, Konrad Dybcio wrote:
On 26.09.2023 21:06, Stephan Gerhold wrote:
On Tue, Sep 26, 2023 at 08:49:24PM +0200, Konrad Dybcio wrote:
On 26.09.2023 18:51, Stephan Gerhold wrote:
Most MSM8916/MSM8939 devices use very similar setups for the modem,
because most of the device-specific details are abstracted by the modem
firmware. There are several definitions (status switches, DAI links
etc) that will be exactly the same for every board.

Introduce a common msm8916-modem-qdsp6.dtsi include that can be used to
simplify enabling the modem for such devices. By default the
digital/analog codec in the SoC/PMIC is used, but boards can define
additional codecs using the templates for Secondary and Quaternary
MI2S.

Signed-off-by: Stephan Gerhold <stephan@xxxxxxxxxxx>
---
I'd rather see at least one usage so that you aren't introducing
effectively non-compiled code..


There are 10 usages in the rest of the patch series.
Is that enough? :D

IMHO it doesn't make sense to squash this with one of the device
patches, especially considering several of them are primarily authored
by others.
I see..

Well, I guess I don't have better counter-arguments, but please
consider this the next time around.


Will do!

[...]

+&lpass_codec {
+	status = "okay";
+};
Any reason for it to stay disabled?


You mean in msm8916.dtsi?
Yes

For the SoC dtsi we don't make assumptions
what devices use or not. There could be devices that ignore the internal
codec entirely. If there is nothing connected to the codec lpass_codec
should not be enabled (e.g. the msm8916-ufi.dtsi devices).
See my reply to patch 5

[...]


Let's continue discussing that there I guess. :D

+	sound_dai_secondary: mi2s-secondary-dai-link {
+		link-name = "Secondary MI2S";
+		status = "disabled"; /* Needs extra codec configuration */
Hmm.. Potential good user of /omit-if-no-ref/?


AFAICT /omit-if-no-ref/ is for phandle references only. Basically it
would only work if you would somewhere reference the phandle:

	list-of-sound-dais = <&sound_dai_primary &sound_dai_secondary>;

But this doesn't exist so /omit-if-no-ref/ cannot be used here.
Ahh right, this is the one we don't reference.. Too bad,
would be a nice fit :/

I only see one usage of it though (patch 7), perhaps it could
be kept local to that one?


This patch series just contains the initial set of
msm8916-modem-qdsp6.dtsi users (for devices which are already upstream).
We probably have like 20 more that still need to be upstreamed. :D

sound_dai_secondary is fairly rare, but there is at least one more user
that will probably end up upstream soon.
2 users don't sound particularly great in a devicetree included by 20 other non-users

I think the overhead of these template notes is absolutely negligible
compared to all the (potentially) unused SoC nodes we have. :D
Yes, however the unused SoC nodes are mostly standardized and could be used as-they-are on a vast majority of devices

Konrad




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux