Messages in this thread | | | Date | Mon, 2 Oct 2023 11:59:21 +0200 | Subject | Re: [PATCH 03/13] arm64: dts: qcom: msm8916: Add common msm8916-modem-qdsp6.dtsi | From | Konrad Dybcio <> |
| |
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@gerhold.net> >>>>> --- >>>> 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
| |