| 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" > > 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 > >
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
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 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 >
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(-)