Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
Date:   Tue, 26 Sep 2023 22:17:35 +0200
From:   Stephan Gerhold <stephan@gerhold.net>
To:     Konrad Dybcio <konrad.dybcio@linaro.org>
Cc:     Bjorn Andersson <andersson@kernel.org>,
        Andy Gross <agross@kernel.org>, linux-arm-msm@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        phone-devel@vger.kernel.org, ~postmarketos/upstreaming@lists.sr.ht,
        Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Subject: Re: [PATCH 03/13] arm64: dts: qcom: msm8916: Add common
 msm8916-modem-qdsp6.dtsi
Message-ID: <ZRM8XzmAz8RAqkUg@gerhold.net>
References: <20230926-msm8916-modem-v1-0-398eec74bac9@gerhold.net>
 <20230926-msm8916-modem-v1-3-398eec74bac9@gerhold.net>
 <45665b43-3be9-4f27-aa88-12cdef56346d@linaro.org>
 <ZRMrqsZ0QeDNFHFj@gerhold.net>
 <1be747ae-1d80-4ebc-9841-c0e98e64a0d1@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1be747ae-1d80-4ebc-9841-c0e98e64a0d1@linaro.org>
Content-Transfer-Encoding: 7bit
Precedence: bulk

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.

I think the overhead of these template notes is absolutely negligible
compared to all the (potentially) unused SoC nodes we have. :D

Thanks,
Stephan