linux-kernel - Re: [PATCH 04/13] arm64: dts: qcom: msm8916-samsung-a2015: Add sound and modem

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: <4d8292ca-c5fe-4270-8d93-66b721903528@linaro.org>
Date:   Tue, 26 Sep 2023 22:29:29 +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>,
        "Lin, Meng-Bo" <linmengbo0689@...tonmail.com>
Subject: Re: [PATCH 04/13] arm64: dts: qcom: msm8916-samsung-a2015: Add sound
 and modem

On 26.09.2023 22:27, Stephan Gerhold wrote:
> On Tue, Sep 26, 2023 at 10:18:22PM +0200, Konrad Dybcio wrote:
>> On 26.09.2023 22:09, Stephan Gerhold wrote:
>>> On Tue, Sep 26, 2023 at 09:56:12PM +0200, Konrad Dybcio wrote:
>>>> [...]
>>>>
>>>>>>> +
>>>>>>>  &blsp_i2c2 {
>>>>>>>  	status = "okay";
>>>>>>>  
>>>>>>> @@ -243,6 +258,13 @@ &gpu {
>>>>>>>  	status = "okay";
>>>>>>>  };
>>>>>>>  
>>>>>>> +&lpass {
>>>>>>> +	dai-link@3 {
>>>>>>> +		reg = <MI2S_QUATERNARY>;
>>>>>>> +		qcom,playback-sd-lines = <1>;
>>>>>>> +	};
>>>>>>> +};
>>>>>> Is that not status = reserved?
>>>>>>
>>>>>
>>>>> Correct. This is here to simplify switching to the modem-bypass audio
>>>>> routing if someone does not need (or want) the modem. The direct audio
>>>>> path with the LPASS drivers tends to be more reliable and configurable
>>>>> (especially wrt bit formats, sampling rates, latency, channels etc).
>>>>> I know that at some point this helped someone who tried to use an old
>>>>> phone as some kind of portable musical instrument / synthesizer.
>>>>>
>>>>> It's not too obvious that these definitions would be needed when making
>>>>> those changes (because devices using the standard SD lines (i.e <0>) do
>>>>> not need it). If you forget about this you get non-functional audio with
>>>>> no error or any hint what could be wrong.
>>>>>
>>>>> To simplify switching between the different audio routing options, the
>>>>> lk2nd bootloader actually has an option to do this transformation in the
>>>>> DTB automagically during boot. It's sort of like a DTB overlay that
>>>>> disables the QDSP6 audio path and enables this node instead. The DAI
>>>>> links are also adjusted where necessary.
>>>>>
>>>>> Do you think a comment would help here?
>>>> I'd say a comment would be necessary here :D
>>>>
>>>
>>> No problem, I will try to add something simple.
>>>
>>>> While I understand this use-case, I believe this is better suited
>>>> for an actual DTBO or something entirely kept inside lk2nd.
>>>> Otherwise this looks very confusing to an outside reader.
>>>>
>>>
>>> Translating from the QDSP6 audio setup to the LPASS one is mostly simple
>>> but not entirely trivial (especially the patching needed for the DAI
>>> links). Main blocker for DTBOs is that you can only add or change, but
>>> AFAIK there is no mechanism to _delete_ or fully recreate nodes.
>> Correct.
>>
>>>
>>> I guess I could maybe derive this from the QDSP6 definitions using
>>> custom magic code, but the code complexity for that is much higher than
>>> adding these nodes here for completeness.
>> I hate to be the bearer of bad news, but this is probably more
>> of a "do we reasonably want this upstream" type question, as
>> you'll probably get some grumpy emails about upstream not caring
>> about what's outside the mainline tree..
>>
> 
> IMHO this is hardly a "downstream" type of situation. The whole point of
> the magic in lk2nd is to make the life for the mainline code base easier
> and simpler, while still allowing the "bypass modem audio" feature with
> an unmodified kernel.
> 
> Before implementing it like this I used to have this directly in the
> kernel tree, by duplicating each device into a normal XYZ.dts and
> "XYZ-no-modem.dts". If you apply this to the total of ~45 different
> MSM8916/MSM8939 DTBs I would like to get upstream this options gets
> extremely ugly. :'D
Maybe a kernel module parameter could be somehow useful here?

Konrad