Re: [PATCH 2/2] arm64: dts: qcom: sc7180: Reorganize trogdor rt5682 audio codec dts

[Konrad Dybcio <konrad.dybcio@linaro.org>]

On 28.08.2023 17:04, Doug Anderson wrote:
> Hi,
> 
> On Sat, Aug 26, 2023 at 2:36 AM Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>
>> On 16.08.2023 20:21, Douglas Anderson wrote:
>>> It was asserted that the "/delete-property/ VBAT-supply;" that we
>>> needed to do in the rt5682s dts fragment was ugly. Let's change up all
>>> the trogdor device trees to make it explicit which version of "rt5682"
>>> we have and avoid the need for the "delete-property".
>>>
>>> As a side effect, this nicely gets rid of the need for a delete-node
>>> in coachz, which doesn't use "rt5682" at all.
>>>
>>> A few notes:
>>> - This doesn't get rid of every "/delete-node/" in trogdor, just the
>>>   one that was used for rt5682s.
>>> - Though we no longer have any "/delete-node/", we do still override
>>>   the "model" in the "sound" node in one case (in pompom) since that
>>>   uses the "2mic" sound setup.
>>>
>>> This is validated to produce the same result (other than a few
>>> properties being reordered) when taking the dtbs generated by the
>>> kernel build and then doing:
>>>
>>>   for dtb in *trogdor*.dtb; do
>>>     dtc -I dtb -O dts $dtb -o out/$dtb.dts;
>>>   done
>>>
>>> Suggested-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>> Signed-off-by: Douglas Anderson <dianders@chromium.org>
>>> ---
>> [...]
>>
>>> +&mdss_dsi0_phy {
>>> +     qcom,phy-rescode-offset-top = /bits/ 8 <31 31 31 31 (-32)>;
>>> +     qcom,phy-rescode-offset-bot = /bits/ 8 <31 31 31 31 (-32)>;
>>> +     qcom,phy-drive-ldo-level = <450>;
>>> +};
>>> +
>>> +&panel {
>>> +     compatible = "boe,tv110c9m-ll3";
>>> +};
>> This is odd
> 
> Can you clarify why it's odd? There are two possible panels used in
> wormdignler, one with an INX display and one with a BOE display. This
> one has the BOE display.
> 
> For the BOE display, there is one with the old Realtek audio codec
> (rt5682i) and one with the new Realtek audio codec (rt5682s).
> 
> Previously, the BOE/rt5682s device tree could just include the
> BOE/rt5682i device tree and override the audio. ...but Krzysztof
> didn't like that. ...so now we have to duplicate the BOE bits in both
> the BOE/rt5682s and BOE/rt5682i files. We could make a fragment for
> just this bit, but that feels overboard for something that's in just
> two files.
I didn't catch that at all, thought it was an unrelated change that
sneaked in..

> 
> As per my "after the cut" description, I'm not really convinced that
> the end result here is easier to understand. Some bits are clearer and
> some more complex. I simply wrote it up because it was requested.
Yeah I'm not sure either. But then I'm not sure that there is any
fantastic way to go about things with this many SKUs, all of them
just tip the dt-number-vs-duplication ratio one way or the other :/

Konrad