[PATCH v2 1/2] dt-bindings: display/msm: dsi-controller-main: Fix deprecated QCM2290 compatible

[PATCH v2 1/2] dt-bindings: display/msm: dsi-controller-main: Fix deprecated QCM2290 compatible

[Konrad Dybcio]

SM6115 previously erroneously added just "qcom,dsi-ctrl-6g-qcm2290",
without the generic fallback. Fix the deprecated binding to reflect
that.

Fixes: 0c0f65c6dd44 ("dt-bindings: msm: dsi-controller-main: Add compatible strings for every current SoC")
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
Depends on (and should have been a part of):

https://lore.kernel.org/linux-arm-msm/20230213121012.1768296-1-konrad.dybcio@linaro.org/

v1 -> v2:
New patch

 .../devicetree/bindings/display/msm/dsi-controller-main.yaml     | 1 -
 1 file changed, 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
index 41cdb631d305..ee19d780dea8 100644
--- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
@@ -37,7 +37,6 @@ properties:
       - items:
           - enum:
               - qcom,dsi-ctrl-6g-qcm2290
-          - const: qcom,mdss-dsi-ctrl
         deprecated: true
 
   reg:
-- 
2.39.1

[PATCH v2 2/2] dt-bindings: display: msm: sm6115-mdss: Fix DSI compatible

[Konrad Dybcio]

Since the DSI autodetection is bound to work correctly on 6115 now,
switch to using the correct per-SoC + generic fallback compatible
combo.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
Depends on (and should have been a part of):

https://lore.kernel.org/linux-arm-msm/20230213121012.1768296-1-konrad.dybcio@linaro.org/

v1 -> v2:
- Deprecate the old compatible instead of removing it (Krzysztof)

 .../devicetree/bindings/display/msm/qcom,sm6115-mdss.yaml | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sm6115-mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,sm6115-mdss.yaml
index 2491cb100b33..6fd1703e415f 100644
--- a/Documentation/devicetree/bindings/display/msm/qcom,sm6115-mdss.yaml
+++ b/Documentation/devicetree/bindings/display/msm/qcom,sm6115-mdss.yaml
@@ -40,7 +40,13 @@ patternProperties:
     type: object
     properties:
       compatible:
-        const: qcom,dsi-ctrl-6g-qcm2290
+        oneOf:
+          - items:
+              - const: qcom,sm6115-dsi-ctrl
+              - const: qcom,mdss-dsi-ctrl
+          - description: Old binding with separate driver logic, please don't use
+            deprecated: true
+            const: qcom,dsi-ctrl-6g-qcm2290
 
   "^phy@[0-9a-f]+$":
     type: object
-- 
2.39.1

Re: [PATCH v2 2/2] dt-bindings: display: msm: sm6115-mdss: Fix DSI compatible

[Krzysztof Kozlowski]

On 17/02/2023 12:13, Konrad Dybcio wrote:
> Since the DSI autodetection is bound to work correctly on 6115 now,
> switch to using the correct per-SoC + generic fallback compatible
> combo.
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
> Depends on (and should have been a part of):
> 
> https://lore.kernel.org/linux-arm-msm/20230213121012.1768296-1-konrad.dybcio@linaro.org/
> 
> v1 -> v2:
> - Deprecate the old compatible instead of removing it (Krzysztof)
> 

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: display/msm: dsi-controller-main: Fix deprecated QCM2290 compatible

[Krzysztof Kozlowski]

On 17/02/2023 12:13, Konrad Dybcio wrote:
> SM6115 previously erroneously added just "qcom,dsi-ctrl-6g-qcm2290",
> without the generic fallback. Fix the deprecated binding to reflect
> that.
> 
> Fixes: 0c0f65c6dd44 ("dt-bindings: msm: dsi-controller-main: Add compatible strings for every current SoC")
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
> Depends on (and should have been a part of):
> 
> https://lore.kernel.org/linux-arm-msm/20230213121012.1768296-1-konrad.dybcio@linaro.org/
> 
> v1 -> v2:
> New patch
> 
>  .../devicetree/bindings/display/msm/dsi-controller-main.yaml     | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> index 41cdb631d305..ee19d780dea8 100644
> --- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> @@ -37,7 +37,6 @@ properties:
>        - items:

If this way stays, drop the items as it is just an enum.

>            - enum:
>                - qcom,dsi-ctrl-6g-qcm2290
> -          - const: qcom,mdss-dsi-ctrl

Wasn't then intention to deprecate both - qcm2290 and mdss - when used
alone?


Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: display/msm: dsi-controller-main: Fix deprecated QCM2290 compatible

[Konrad Dybcio]

On 17.02.2023 12:30, Krzysztof Kozlowski wrote:
> On 17/02/2023 12:13, Konrad Dybcio wrote:
>> SM6115 previously erroneously added just "qcom,dsi-ctrl-6g-qcm2290",
>> without the generic fallback. Fix the deprecated binding to reflect
>> that.
>>
>> Fixes: 0c0f65c6dd44 ("dt-bindings: msm: dsi-controller-main: Add compatible strings for every current SoC")
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>> ---
>> Depends on (and should have been a part of):
>>
>> https://lore.kernel.org/linux-arm-msm/20230213121012.1768296-1-konrad.dybcio@linaro.org/
>>
>> v1 -> v2:
>> New patch
>>
>>  .../devicetree/bindings/display/msm/dsi-controller-main.yaml     | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
>> index 41cdb631d305..ee19d780dea8 100644
>> --- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
>> +++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
>> @@ -37,7 +37,6 @@ properties:
>>        - items:
> 
> If this way stays, drop the items as it is just an enum.
> 
>>            - enum:
>>                - qcom,dsi-ctrl-6g-qcm2290
>> -          - const: qcom,mdss-dsi-ctrl
> 
> Wasn't then intention to deprecate both - qcm2290 and mdss - when used
> alone?
"qcom,dsi-ctrl-6g-qcm2290", "qcom,mdss-dsi-ctrl"

was never used. The only upstream usage of the 2290 compat
is in sm6115.dtsi:

compatible = "qcom,dsi-ctrl-6g-qcm2290";
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/qcom/sm6115.dtsi?h=next-20230217#n1221

Konrad
> 
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v2 1/2] dt-bindings: display/msm: dsi-controller-main: Fix deprecated QCM2290 compatible

[Krzysztof Kozlowski]

On 17/02/2023 12:32, Konrad Dybcio wrote:
> 
> 
> On 17.02.2023 12:30, Krzysztof Kozlowski wrote:
>> On 17/02/2023 12:13, Konrad Dybcio wrote:
>>> SM6115 previously erroneously added just "qcom,dsi-ctrl-6g-qcm2290",
>>> without the generic fallback. Fix the deprecated binding to reflect
>>> that.
>>>
>>> Fixes: 0c0f65c6dd44 ("dt-bindings: msm: dsi-controller-main: Add compatible strings for every current SoC")
>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>>> ---
>>> Depends on (and should have been a part of):
>>>
>>> https://lore.kernel.org/linux-arm-msm/20230213121012.1768296-1-konrad.dybcio@linaro.org/
>>>
>>> v1 -> v2:
>>> New patch
>>>
>>>  .../devicetree/bindings/display/msm/dsi-controller-main.yaml     | 1 -
>>>  1 file changed, 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
>>> index 41cdb631d305..ee19d780dea8 100644
>>> --- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
>>> +++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
>>> @@ -37,7 +37,6 @@ properties:
>>>        - items:
>>
>> If this way stays, drop the items as it is just an enum.
>>
>>>            - enum:
>>>                - qcom,dsi-ctrl-6g-qcm2290
>>> -          - const: qcom,mdss-dsi-ctrl
>>
>> Wasn't then intention to deprecate both - qcm2290 and mdss - when used
>> alone?
> "qcom,dsi-ctrl-6g-qcm2290", "qcom,mdss-dsi-ctrl"
> 
> was never used. The only upstream usage of the 2290 compat
> is in sm6115.dtsi:
> 
> compatible = "qcom,dsi-ctrl-6g-qcm2290";
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/qcom/sm6115.dtsi?h=next-20230217#n1221

I meant, that original commit wanted to deprecate:
compatible="qcom,dsi-ctrl-6g-qcm2290";
compatible="qcom,mdss-dsi-ctrl";

Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: display/msm: dsi-controller-main: Fix deprecated QCM2290 compatible

[Konrad Dybcio]

On 17.02.2023 12:35, Krzysztof Kozlowski wrote:
> On 17/02/2023 12:32, Konrad Dybcio wrote:
>>
>>
>> On 17.02.2023 12:30, Krzysztof Kozlowski wrote:
>>> On 17/02/2023 12:13, Konrad Dybcio wrote:
>>>> SM6115 previously erroneously added just "qcom,dsi-ctrl-6g-qcm2290",
>>>> without the generic fallback. Fix the deprecated binding to reflect
>>>> that.
>>>>
>>>> Fixes: 0c0f65c6dd44 ("dt-bindings: msm: dsi-controller-main: Add compatible strings for every current SoC")
>>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>>>> ---
>>>> Depends on (and should have been a part of):
>>>>
>>>> https://lore.kernel.org/linux-arm-msm/20230213121012.1768296-1-konrad.dybcio@linaro.org/
>>>>
>>>> v1 -> v2:
>>>> New patch
>>>>
>>>>  .../devicetree/bindings/display/msm/dsi-controller-main.yaml     | 1 -
>>>>  1 file changed, 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
>>>> index 41cdb631d305..ee19d780dea8 100644
>>>> --- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
>>>> +++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
>>>> @@ -37,7 +37,6 @@ properties:
>>>>        - items:
>>>
>>> If this way stays, drop the items as it is just an enum.
>>>
>>>>            - enum:
>>>>                - qcom,dsi-ctrl-6g-qcm2290
>>>> -          - const: qcom,mdss-dsi-ctrl
>>>
>>> Wasn't then intention to deprecate both - qcm2290 and mdss - when used
>>> alone?
>> "qcom,dsi-ctrl-6g-qcm2290", "qcom,mdss-dsi-ctrl"
>>
>> was never used. The only upstream usage of the 2290 compat
>> is in sm6115.dtsi:
>>
>> compatible = "qcom,dsi-ctrl-6g-qcm2290";
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/qcom/sm6115.dtsi?h=next-20230217#n1221
> 
> I meant, that original commit wanted to deprecate:
> compatible="qcom,dsi-ctrl-6g-qcm2290";
> compatible="qcom,mdss-dsi-ctrl";
> 
Okay, so what would be the correct resolution?
Drop this patch and keep 2/2?

Konrad
> Best regards,
> Krzysztof
> 

Re: [PATCH v2 1/2] dt-bindings: display/msm: dsi-controller-main: Fix deprecated QCM2290 compatible

[Krzysztof Kozlowski]

On 17/02/2023 12:36, Konrad Dybcio wrote:
>>>
>>> compatible = "qcom,dsi-ctrl-6g-qcm2290";
>>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/qcom/sm6115.dtsi?h=next-20230217#n1221
>>
>> I meant, that original commit wanted to deprecate:
>> compatible="qcom,dsi-ctrl-6g-qcm2290";
>> compatible="qcom,mdss-dsi-ctrl";
>>
> Okay, so what would be the correct resolution?
> Drop this patch and keep 2/2?

First, it would be nice to know what was the intention of Bryan's commit?

Second, if the intention was to deprecate both of these, then this
commit could stay with changes - make it enum for both compatibles (not
list).

Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: display/msm: dsi-controller-main: Fix deprecated QCM2290 compatible

[Konrad Dybcio]

On 17.02.2023 13:24, Krzysztof Kozlowski wrote:
> On 17/02/2023 12:36, Konrad Dybcio wrote:
>>>>
>>>> compatible = "qcom,dsi-ctrl-6g-qcm2290";
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/qcom/sm6115.dtsi?h=next-20230217#n1221
>>>
>>> I meant, that original commit wanted to deprecate:
>>> compatible="qcom,dsi-ctrl-6g-qcm2290";
>>> compatible="qcom,mdss-dsi-ctrl";
>>>
>> Okay, so what would be the correct resolution?
>> Drop this patch and keep 2/2?
> 
> First, it would be nice to know what was the intention of Bryan's commit?
AFAICT, it was necessary to add per-SoC compatibles to all DSI hosts
to make documenting clocks possible (they differ per-platform).

The qcm2290 deprecation came from the oddity of the compatible name
(it did not match qcom,socname-hw), but he seems to have overlooked
that (at least before my recent patchset [1]), it was necessary as it
needed to circumvent part of the driver's logic. So it was first made
up-to-speed with the rest by adding the fallback common compatible and
then (wrongly) deprecated.


Then, SM6115 DSI DTS part was added parallel to that, so he did not
update it.

With [1] its deprecation is correct and this series tries to complete
it.

Konrad

[1] https://lore.kernel.org/linux-arm-msm/20230213121012.1768296-1-konrad.dybcio@linaro.org/
> 
> Second, if the intention was to deprecate both of these, then this
> commit could stay with changes - make it enum for both compatibles (not
> list).
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v2 1/2] dt-bindings: display/msm: dsi-controller-main: Fix deprecated QCM2290 compatible

[Bryan O'Donoghue]

On 17/02/2023 12:24, Krzysztof Kozlowski wrote:
> First, it would be nice to know what was the intention of Bryan's commit?

Sorry I've been grazing this thread but, not responding.

- qcom,dsi-ctrl-6g-qcm2290

is non-compliant with qcom,socid-dsi-ctrl which is our desired naming 
convention, so that's what the deprecation is about i.e. moving this 
compat to "qcom,qcm2290-dsi-ctrl"

Actually I have the question why we are deciding to go with "sm6115" 
instead of "qcm2290" ?

The stamp on the package you receive from Thundercomm says "qcm2290" not 
"sm6115"

?

---
bod

Re: [PATCH v2 1/2] dt-bindings: display/msm: dsi-controller-main: Fix deprecated QCM2290 compatible

[Konrad Dybcio]

On 17.02.2023 22:13, Bryan O'Donoghue wrote:
> On 17/02/2023 12:24, Krzysztof Kozlowski wrote:
>> First, it would be nice to know what was the intention of Bryan's commit?
> 
> Sorry I've been grazing this thread but, not responding.
> 
> - qcom,dsi-ctrl-6g-qcm2290
> 
> is non-compliant with qcom,socid-dsi-ctrl which is our desired naming convention, so that's what the deprecation is about i.e. moving this compat to "qcom,qcm2290-dsi-ctrl"
> 
> Actually I have the question why we are deciding to go with "sm6115" instead of "qcm2290" ?
> 
> The stamp on the package you receive from Thundercomm says "qcm2290" not "sm6115"
Correct, but QCM2290 is not supported upstream yet.

SM6115 (a different SoC) however is, but it used the qcm2290 compatible
as it was a convenient hack to get the DSI host ID recognized based on
the (identical-to-qcm2290) base register without additional driver changes.
We're now trying to untangle that mess..

Konrad
> 
> ?
> 
> ---
> bod
> 
> 

Re: [PATCH v2 1/2] dt-bindings: display/msm: dsi-controller-main: Fix deprecated QCM2290 compatible

[Bryan O'Donoghue]

On 17/02/2023 21:16, Konrad Dybcio wrote:
> Correct, but QCM2290 is not supported upstream yet.
> 
> SM6115 (a different SoC) however is, but it used the qcm2290 compatible
> as it was a convenient hack to get the DSI host ID recognized based on
> the (identical-to-qcm2290) base register without additional driver changes.
> We're now trying to untangle that mess..

Gand so what we want documented is:

compatible = "qcom,qcs2290-dsi-ctrl", qcom,mdss-dsi-ctrl";
compatible = "qcom,sm6115-dsi-ctrl", qcom,mdss-dsi-ctrl";

with the old compatible = "qcom,dsi-ctrl-6g-qcm2290"; clanger continuing 
to be deprecated.

---
bod

Re: [PATCH v2 1/2] dt-bindings: display/msm: dsi-controller-main: Fix deprecated QCM2290 compatible

[Konrad Dybcio]

On 17.02.2023 22:20, Bryan O'Donoghue wrote:
> On 17/02/2023 21:16, Konrad Dybcio wrote:
>> Correct, but QCM2290 is not supported upstream yet.
>>
>> SM6115 (a different SoC) however is, but it used the qcm2290 compatible
>> as it was a convenient hack to get the DSI host ID recognized based on
>> the (identical-to-qcm2290) base register without additional driver changes.
>> We're now trying to untangle that mess..
> 
> Gand so what we want documented is:
> 
> compatible = "qcom,qcs2290-dsi-ctrl", qcom,mdss-dsi-ctrl";
qcm* yes, this became documented with your original cleanup

> compatible = "qcom,sm6115-dsi-ctrl", qcom,mdss-dsi-ctrl";
and yes this became documented (well, in the DSI binding) in
my other patch series and is finished being documented in this one

> 
> with the old compatible = "qcom,dsi-ctrl-6g-qcm2290"; clanger continuing to be deprecated.
correct, we still have to note it but keep it deprecated

Konrad
> 
> ---
> bod

Re: [PATCH v2 1/2] dt-bindings: display/msm: dsi-controller-main: Fix deprecated QCM2290 compatible

[Bryan O'Donoghue]

On 17/02/2023 21:23, Konrad Dybcio wrote:
> 
> 
> On 17.02.2023 22:20, Bryan O'Donoghue wrote:
>> On 17/02/2023 21:16, Konrad Dybcio wrote:
>>> Correct, but QCM2290 is not supported upstream yet.
>>>
>>> SM6115 (a different SoC) however is, but it used the qcm2290 compatible
>>> as it was a convenient hack to get the DSI host ID recognized based on
>>> the (identical-to-qcm2290) base register without additional driver changes.
>>> We're now trying to untangle that mess..
>>
>> Gand so what we want documented is:
>>
>> compatible = "qcom,qcs2290-dsi-ctrl", qcom,mdss-dsi-ctrl";
> qcm* yes, this became documented with your original cleanup
> 
>> compatible = "qcom,sm6115-dsi-ctrl", qcom,mdss-dsi-ctrl";
> and yes this became documented (well, in the DSI binding) in
> my other patch series and is finished being documented in this one
> 
>>
>> with the old compatible = "qcom,dsi-ctrl-6g-qcm2290"; clanger continuing to be deprecated.
> correct, we still have to note it but keep it deprecated
> 
> Konrad
>>
>> ---
>> bod

Cool.

That maps to my understanding & the intention of the deprecation.

---
bod

Re: [PATCH v2 1/2] dt-bindings: display/msm: dsi-controller-main: Fix deprecated QCM2290 compatible

[Krzysztof Kozlowski]

On 17/02/2023 22:13, Bryan O'Donoghue wrote:
> On 17/02/2023 12:24, Krzysztof Kozlowski wrote:
>> First, it would be nice to know what was the intention of Bryan's commit?
> 
> Sorry I've been grazing this thread but, not responding.
> 
> - qcom,dsi-ctrl-6g-qcm2290
> 
> is non-compliant with qcom,socid-dsi-ctrl which is our desired naming 
> convention, so that's what the deprecation is about i.e. moving this 
> compat to "qcom,qcm2290-dsi-ctrl"

OK, then there was no intention to deprecate qcom,mdss-dsi-ctrl and it
should be left as allowed compatible.

Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: display/msm: dsi-controller-main: Fix deprecated QCM2290 compatible

[Konrad Dybcio]

On 18.02.2023 11:14, Krzysztof Kozlowski wrote:
> On 17/02/2023 22:13, Bryan O'Donoghue wrote:
>> On 17/02/2023 12:24, Krzysztof Kozlowski wrote:
>>> First, it would be nice to know what was the intention of Bryan's commit?
>>
>> Sorry I've been grazing this thread but, not responding.
>>
>> - qcom,dsi-ctrl-6g-qcm2290
>>
>> is non-compliant with qcom,socid-dsi-ctrl which is our desired naming 
>> convention, so that's what the deprecation is about i.e. moving this 
>> compat to "qcom,qcm2290-dsi-ctrl"
> 
> OK, then there was no intention to deprecate qcom,mdss-dsi-ctrl and it
> should be left as allowed compatible.
Not sure if we're on the same page.

It wasn't intended to deprecate [1] "qcom,qcm2290-dsi-ctrl", "qcom-mdss-dsi-ctrl";
(newly-introduced in Bryan's cleanup patchset) but it was intended to deprecate
[2] "qcom,dsi-ctrl-6g-qcm2290"; which was introduced long before that *and* used in
the 6115 dt (and it still is in linux-next today, as my cleanup hasn't landed yet).

[3] "qcom,dsi-ctrl-6g-qcm2290", "qcom,mdss-dsi-ctrl" was never used (and should never
be, considering there's a proper compatible [1] now) so adding it to bindings
didn't solve the undocumented-ness issue. Plus the fallback would have never
worked back then, as the DSI hw revision check would spit out 2.4.1 or 2.4.
which is SC7180 or SDM845 and then it would never match the base register, as
they're waay different.

Konrad
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v2 1/2] dt-bindings: display/msm: dsi-controller-main: Fix deprecated QCM2290 compatible

[Krzysztof Kozlowski]

On 18/02/2023 12:23, Konrad Dybcio wrote:
> 
> 
> On 18.02.2023 11:14, Krzysztof Kozlowski wrote:
>> On 17/02/2023 22:13, Bryan O'Donoghue wrote:
>>> On 17/02/2023 12:24, Krzysztof Kozlowski wrote:
>>>> First, it would be nice to know what was the intention of Bryan's commit?
>>>
>>> Sorry I've been grazing this thread but, not responding.
>>>
>>> - qcom,dsi-ctrl-6g-qcm2290
>>>
>>> is non-compliant with qcom,socid-dsi-ctrl which is our desired naming 
>>> convention, so that's what the deprecation is about i.e. moving this 
>>> compat to "qcom,qcm2290-dsi-ctrl"
>>
>> OK, then there was no intention to deprecate qcom,mdss-dsi-ctrl and it
>> should be left as allowed compatible.
> Not sure if we're on the same page.

We are.

> 
> It wasn't intended to deprecate [1] "qcom,qcm2290-dsi-ctrl", "qcom-mdss-dsi-ctrl";
> (newly-introduced in Bryan's cleanup patchset) but it was intended to deprecate
> [2] "qcom,dsi-ctrl-6g-qcm2290"; which was introduced long before that *and* used in
> the 6115 dt (and it still is in linux-next today, as my cleanup hasn't landed yet).
> 
> [3] "qcom,dsi-ctrl-6g-qcm2290", "qcom,mdss-dsi-ctrl" was never used (and should never
> be, considering there's a proper compatible [1] now) so adding it to bindings
> didn't solve the undocumented-ness issue. Plus the fallback would have never
> worked back then, as the DSI hw revision check would spit out 2.4.1 or 2.4.
> which is SC7180 or SDM845 and then it would never match the base register, as
> they're waay different.

All these were known. I was asking about "qcom,mdss-dsi-ctrl", because
the original intention also affects the way we want to keep it now
(unless there are other reasons).

Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: display/msm: dsi-controller-main: Fix deprecated QCM2290 compatible

[Konrad Dybcio]

On 18.02.2023 15:49, Krzysztof Kozlowski wrote:
> On 18/02/2023 12:23, Konrad Dybcio wrote:
>>
>>
>> On 18.02.2023 11:14, Krzysztof Kozlowski wrote:
>>> On 17/02/2023 22:13, Bryan O'Donoghue wrote:
>>>> On 17/02/2023 12:24, Krzysztof Kozlowski wrote:
>>>>> First, it would be nice to know what was the intention of Bryan's commit?
>>>>
>>>> Sorry I've been grazing this thread but, not responding.
>>>>
>>>> - qcom,dsi-ctrl-6g-qcm2290
>>>>
>>>> is non-compliant with qcom,socid-dsi-ctrl which is our desired naming 
>>>> convention, so that's what the deprecation is about i.e. moving this 
>>>> compat to "qcom,qcm2290-dsi-ctrl"
>>>
>>> OK, then there was no intention to deprecate qcom,mdss-dsi-ctrl and it
>>> should be left as allowed compatible.
>> Not sure if we're on the same page.
> 
> We are.
> 
>>
>> It wasn't intended to deprecate [1] "qcom,qcm2290-dsi-ctrl", "qcom-mdss-dsi-ctrl";
>> (newly-introduced in Bryan's cleanup patchset) but it was intended to deprecate
>> [2] "qcom,dsi-ctrl-6g-qcm2290"; which was introduced long before that *and* used in
>> the 6115 dt (and it still is in linux-next today, as my cleanup hasn't landed yet).
>>
>> [3] "qcom,dsi-ctrl-6g-qcm2290", "qcom,mdss-dsi-ctrl" was never used (and should never
>> be, considering there's a proper compatible [1] now) so adding it to bindings
>> didn't solve the undocumented-ness issue. Plus the fallback would have never
>> worked back then, as the DSI hw revision check would spit out 2.4.1 or 2.4.
>> which is SC7180 or SDM845 and then it would never match the base register, as
>> they're waay different.
> 
> All these were known. I was asking about "qcom,mdss-dsi-ctrl", because
> the original intention also affects the way we want to keep it now
> (unless there are other reasons).
Okay, so we want to deprecate:

"qcom,dsi-ctrl-6g-qcm2290", "qcom,mdss-dsi-ctrl"

because it is:

1) non-compliant with the qcom,socname-hwblock formula
2) replaceable since we rely on the fallback compatible
3) "qcom,dsi-ctrl-6g-qcm2290" alone would have been expected to
   be fixed in the DTSI similar to other SoCs

Is that correct?

Because 2) doesn't hold, as - at the time of the introduction
of Bryan's patchset - the fallback compatible would not have
been sufficient from the Linux POV [1], though it would have been
sufficient from the hardware description POV, as the hardware
on the SoC *is* essentially what qcom,mdss-dsi-ctrl refers to.

[1] The driver would simply not probe. It *would be* Linux-correct
after my code-fixing series was applied, but I think I'm just failing
to comprehend what sort of ABI we're trying to preserve here :/

Konrad

> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v2 1/2] dt-bindings: display/msm: dsi-controller-main: Fix deprecated QCM2290 compatible

[Krzysztof Kozlowski]

On 20/02/2023 11:24, Konrad Dybcio wrote:
> 
> 
> On 18.02.2023 15:49, Krzysztof Kozlowski wrote:
>> On 18/02/2023 12:23, Konrad Dybcio wrote:
>>>
>>>
>>> On 18.02.2023 11:14, Krzysztof Kozlowski wrote:
>>>> On 17/02/2023 22:13, Bryan O'Donoghue wrote:
>>>>> On 17/02/2023 12:24, Krzysztof Kozlowski wrote:
>>>>>> First, it would be nice to know what was the intention of Bryan's commit?
>>>>>
>>>>> Sorry I've been grazing this thread but, not responding.
>>>>>
>>>>> - qcom,dsi-ctrl-6g-qcm2290
>>>>>
>>>>> is non-compliant with qcom,socid-dsi-ctrl which is our desired naming 
>>>>> convention, so that's what the deprecation is about i.e. moving this 
>>>>> compat to "qcom,qcm2290-dsi-ctrl"
>>>>
>>>> OK, then there was no intention to deprecate qcom,mdss-dsi-ctrl and it
>>>> should be left as allowed compatible.
>>> Not sure if we're on the same page.
>>
>> We are.
>>
>>>
>>> It wasn't intended to deprecate [1] "qcom,qcm2290-dsi-ctrl", "qcom-mdss-dsi-ctrl";
>>> (newly-introduced in Bryan's cleanup patchset) but it was intended to deprecate
>>> [2] "qcom,dsi-ctrl-6g-qcm2290"; which was introduced long before that *and* used in
>>> the 6115 dt (and it still is in linux-next today, as my cleanup hasn't landed yet).
>>>
>>> [3] "qcom,dsi-ctrl-6g-qcm2290", "qcom,mdss-dsi-ctrl" was never used (and should never
>>> be, considering there's a proper compatible [1] now) so adding it to bindings
>>> didn't solve the undocumented-ness issue. Plus the fallback would have never
>>> worked back then, as the DSI hw revision check would spit out 2.4.1 or 2.4.
>>> which is SC7180 or SDM845 and then it would never match the base register, as
>>> they're waay different.
>>
>> All these were known. I was asking about "qcom,mdss-dsi-ctrl", because
>> the original intention also affects the way we want to keep it now
>> (unless there are other reasons).
> Okay, so we want to deprecate:
> 
> "qcom,dsi-ctrl-6g-qcm2290", "qcom,mdss-dsi-ctrl"

No, we don't want to deprecate it. Such compatible was never existing
originally and was only introduced by mistake. We want to correct the
mistake, but we don't want to deprecate such list.

> 
> because it is:
> 
> 1) non-compliant with the qcom,socname-hwblock formula
> 2) replaceable since we rely on the fallback compatible
> 3) "qcom,dsi-ctrl-6g-qcm2290" alone would have been expected to
>    be fixed in the DTSI similar to other SoCs
> 
> Is that correct?

No. So again, I am talking only about qcom,mdss-dsi-ctrl. Since
beginning of this thread:

"Wasn't then intention to deprecate both - qcm2290 and mdss - when used
alone?"

Why do you bring the list to the topic? The list was created by mistake
and Bryan confirmed that it was never his intention.

> 
> Because 2) doesn't hold, as - at the time of the introduction
> of Bryan's patchset - the fallback compatible would not have
> been sufficient from the Linux POV [1]

There was no fallback compatible at that time.

> , though it would have been
> sufficient from the hardware description POV, as the hardware
> on the SoC *is* essentially what qcom,mdss-dsi-ctrl refers to.
> 
> [1] The driver would simply not probe. It *would be* Linux-correct
> after my code-fixing series was applied, but I think I'm just failing
> to comprehend what sort of ABI we're trying to preserve here :/

Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: display/msm: dsi-controller-main: Fix deprecated QCM2290 compatible

[Konrad Dybcio]

On 20.02.2023 11:31, Krzysztof Kozlowski wrote:
> On 20/02/2023 11:24, Konrad Dybcio wrote:
>>
>>
>> On 18.02.2023 15:49, Krzysztof Kozlowski wrote:
>>> On 18/02/2023 12:23, Konrad Dybcio wrote:
>>>>
>>>>
>>>> On 18.02.2023 11:14, Krzysztof Kozlowski wrote:
>>>>> On 17/02/2023 22:13, Bryan O'Donoghue wrote:
>>>>>> On 17/02/2023 12:24, Krzysztof Kozlowski wrote:
>>>>>>> First, it would be nice to know what was the intention of Bryan's commit?
>>>>>>
>>>>>> Sorry I've been grazing this thread but, not responding.
>>>>>>
>>>>>> - qcom,dsi-ctrl-6g-qcm2290
>>>>>>
>>>>>> is non-compliant with qcom,socid-dsi-ctrl which is our desired naming 
>>>>>> convention, so that's what the deprecation is about i.e. moving this 
>>>>>> compat to "qcom,qcm2290-dsi-ctrl"
>>>>>
>>>>> OK, then there was no intention to deprecate qcom,mdss-dsi-ctrl and it
>>>>> should be left as allowed compatible.
>>>> Not sure if we're on the same page.
>>>
>>> We are.
>>>
>>>>
>>>> It wasn't intended to deprecate [1] "qcom,qcm2290-dsi-ctrl", "qcom-mdss-dsi-ctrl";
>>>> (newly-introduced in Bryan's cleanup patchset) but it was intended to deprecate
>>>> [2] "qcom,dsi-ctrl-6g-qcm2290"; which was introduced long before that *and* used in
>>>> the 6115 dt (and it still is in linux-next today, as my cleanup hasn't landed yet).
>>>>
>>>> [3] "qcom,dsi-ctrl-6g-qcm2290", "qcom,mdss-dsi-ctrl" was never used (and should never
>>>> be, considering there's a proper compatible [1] now) so adding it to bindings
>>>> didn't solve the undocumented-ness issue. Plus the fallback would have never
>>>> worked back then, as the DSI hw revision check would spit out 2.4.1 or 2.4.
>>>> which is SC7180 or SDM845 and then it would never match the base register, as
>>>> they're waay different.
>>>
>>> All these were known. I was asking about "qcom,mdss-dsi-ctrl", because
>>> the original intention also affects the way we want to keep it now
>>> (unless there are other reasons).
>> Okay, so we want to deprecate:
>>
>> "qcom,dsi-ctrl-6g-qcm2290", "qcom,mdss-dsi-ctrl"
> 
> No, we don't want to deprecate it. Such compatible was never existing
> originally and was only introduced by mistake. We want to correct the
> mistake, but we don't want to deprecate such list.
> 
>>
>> because it is:
>>
>> 1) non-compliant with the qcom,socname-hwblock formula
>> 2) replaceable since we rely on the fallback compatible
>> 3) "qcom,dsi-ctrl-6g-qcm2290" alone would have been expected to
>>    be fixed in the DTSI similar to other SoCs
>>
>> Is that correct?
> 
> No. So again, I am talking only about qcom,mdss-dsi-ctrl. Since
> beginning of this thread:
> 
> "Wasn't then intention to deprecate both - qcm2290 and mdss - when used
> alone?"
> 
> Why do you bring the list to the topic? The list was created by mistake
> and Bryan confirmed that it was never his intention.
Ugh.. I think I just misread your message in your second reply
counting from the beginning of the thread.. Things are much
clearer now that I re-read it..

So, just to confirm..

This patch, with the items: level dropped, is fine?

Konrad
> 
>>
>> Because 2) doesn't hold, as - at the time of the introduction
>> of Bryan's patchset - the fallback compatible would not have
>> been sufficient from the Linux POV [1]
> 
> There was no fallback compatible at that time.
> 
>> , though it would have been
>> sufficient from the hardware description POV, as the hardware
>> on the SoC *is* essentially what qcom,mdss-dsi-ctrl refers to.
>>
>> [1] The driver would simply not probe. It *would be* Linux-correct
>> after my code-fixing series was applied, but I think I'm just failing
>> to comprehend what sort of ABI we're trying to preserve here :/
> 
> Best regards,
> Krzysztof
>