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

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]

Date:   Mon, 2 Oct 2023 11:59:21 +0200
From:   Konrad Dybcio <konrad.dybcio@...aro.org>
To:     Stephan Gerhold <stephan@...hold.net>
Cc:     Bjorn Andersson <andersson@...nel.org>,
        Andy Gross <agross@...nel.org>, linux-arm-msm@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        phone-devel@...r.kernel.org, ~postmarketos/upstreaming@...ts.sr.ht,
        Bryan O'Donoghue <bryan.odonoghue@...aro.org>
Subject: Re: [PATCH 03/13] arm64: dts: qcom: msm8916: Add common
 msm8916-modem-qdsp6.dtsi



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@...hold.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