| Message ID | 20230217111316.306241-1-konrad.dybcio@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/2] dt-bindings: display/msm: dsi-controller-main: Fix deprecated QCM2290 compatible | expand |
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
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
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 >
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
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 >
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:
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(-)