[PATCH v3 0/1] arm64: dts: qcom: sm8550: remove

[PATCH v3 0/1] arm64: dts: qcom: sm8550: remove

[Tengfei Fan]

The address/size-cells in mdss_dsi1 node have not ranges and child also
have not reg, then this leads to dtc W=1 warnings:

  sm8550.dtsi:2937.27-2992.6: Warning (avoid_unnecessary_addr_size): /soc@0/display-subsystem@ae00000/dsi@ae96000:
    unnecessary #address-cells/#size-cells without "ranges" or child "reg" property

Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com>
---

v2 -> v3:
  - separate this patch from other patch series
v1 -> v2:
  - add reviewed-by tage due to patch review done

previous discussion here:
[1] v2: https://lore.kernel.org/linux-arm-msm/20231207092801.7506-4-quic_tengfan@quicinc.com
[2] v1: https://lore.kernel.org/linux-arm-msm/20231117101817.4401-6-quic_tengfan@quicinc.com

Tengfei Fan (1):
  arm64: dts: qcom: sm8550: remove address/size-cells from mdss_dsi1

 arch/arm64/boot/dts/qcom/sm8550.dtsi | 3 ---
 1 file changed, 3 deletions(-)


base-commit: ceb2fe0d438644e1de06b9a6468a1fb8e2199c70
-- 
2.17.1

[PATCH v3 1/1] arm64: dts: qcom: sm8550: remove address/size-cells from mdss_dsi1

[Tengfei Fan]

The address/size-cells in mdss_dsi1 node have not ranges and child also
have not reg, then this leads to dtc W=1 warnings:

  sm8550.dtsi:2937.27-2992.6: Warning (avoid_unnecessary_addr_size): /soc@0/display-subsystem@ae00000/dsi@ae96000:
    unnecessary #address-cells/#size-cells without "ranges" or child "reg" property


Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sm8550.dtsi | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/dts/qcom/sm8550.dtsi
index d707d15cea5b..cce3e63a2599 100644
--- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
@@ -2967,9 +2967,6 @@
 				phys = <&mdss_dsi1_phy>;
 				phy-names = "dsi";
 
-				#address-cells = <1>;
-				#size-cells = <0>;
-
 				status = "disabled";
 
 				ports {
-- 
2.17.1

Re: [PATCH v3 1/1] arm64: dts: qcom: sm8550: remove address/size-cells from mdss_dsi1

[Krzysztof Kozlowski]

On 19/12/2023 01:31, Tengfei Fan wrote:
> The address/size-cells in mdss_dsi1 node have not ranges and child also
> have not reg, then this leads to dtc W=1 warnings:

I cannot parse it. Address/size cells never have ranges or children.
They cannot have. These are uint32 properties.

> 
>   sm8550.dtsi:2937.27-2992.6: Warning (avoid_unnecessary_addr_size): /soc@0/display-subsystem@ae00000/dsi@ae96000:
>     unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
> 
> 
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com>
> ---

I disagreed with the patch before. You resubmit it without really
addressing my concerns.

I am not sure if this is correct fix and I want to fix all of such
errors (there are multiple of them) in the same way. Feel free to
propose common solution based on arguments.

I don't really want to NAKit but since you are resending without
finishing the discussion, which is quite impolite, then let's be like that:

NAK

Best regards,
Krzysztof

Re: [PATCH v3 1/1] arm64: dts: qcom: sm8550: remove address/size-cells from mdss_dsi1

[Tengfei Fan]

在 12/19/2023 3:17 PM, Krzysztof Kozlowski 写道:
> On 19/12/2023 01:31, Tengfei Fan wrote:
>> The address/size-cells in mdss_dsi1 node have not ranges and child also
>> have not reg, then this leads to dtc W=1 warnings:
> 
> I cannot parse it. Address/size cells never have ranges or children.
> They cannot have. These are uint32 properties.
> 
>>
>>    sm8550.dtsi:2937.27-2992.6: Warning (avoid_unnecessary_addr_size): /soc@0/display-subsystem@ae00000/dsi@ae96000:
>>      unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
>>
>>
>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com>
>> ---
> 
> I disagreed with the patch before. You resubmit it without really
> addressing my concerns.
> 
> I am not sure if this is correct fix and I want to fix all of such
> errors (there are multiple of them) in the same way. Feel free to
> propose common solution based on arguments.
> 
> I don't really want to NAKit but since you are resending without
> finishing the discussion, which is quite impolite, then let's be like that:
> 
> NAK
Hi Krzysztof,
Have sent this patch series before finding your latest comments in the 
previous patch series.
I sincerely apologize to you! This is my fault, please ignore this patch 
series before solve your concerns.
> 
> Best regards,
> Krzysztof
> 

-- 
Thx and BRs,
Tengfei Fan

Re: [PATCH v3 1/1] arm64: dts: qcom: sm8550: remove address/size-cells from mdss_dsi1

[Aiqun Yu (Maria)]

On 12/19/2023 3:17 PM, Krzysztof Kozlowski wrote:
> On 19/12/2023 01:31, Tengfei Fan wrote:
>> The address/size-cells in mdss_dsi1 node have not ranges and child also
>> have not reg, then this leads to dtc W=1 warnings:
> 
Comments can be more readable:
"mdss_dsi1" node don't have "ranges" or child "reg" property, while it 
have address/size-cells properties. This caused 
"avoid_unnecessary_addr_size" warning from dtb check.
Remove address/size-cells properties for "mdss_dsi1" node.

> I cannot parse it. Address/size cells never have ranges or children.
> They cannot have. These are uint32 properties.
Pls help to comment on the revised commit message. Every time I write a 
commit message, also takes a while for me to double confirm whether 
others can understand me correctly as well. Feel free to let us know if 
it is not readable to you. It will help us as non-English native developers.
> 
>>
>>    sm8550.dtsi:2937.27-2992.6: Warning (avoid_unnecessary_addr_size): /soc@0/display-subsystem@ae00000/dsi@ae96000:
>>      unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
>>
>>
>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com>
>> ---
> 
> I disagreed with the patch before. You resubmit it without really
> addressing my concerns.
> 
> I am not sure if this is correct fix and I want to fix all of such
> errors (there are multiple of them) in the same way. Feel free to
> propose common solution based on arguments.
Per my understanding, "qcom,mdss-dsi-ctrl" driver node like "mdss_dsi1" 
don't need to have address/size-cells properties.
Feel free to let us know whether there is different idea of 
"address/size-cells" needed for the "qcom,mdss-dsi-ctrl" driver node.

If there is no valid reason to add "address/size-cells" properties for 
"qcom,mdss-dsi-ctrl" driver node, the plan is that: Remove 
address/size-cells properties for "qcom,mdss-dsi-ctrl" compatible node.
While there are about 22 different soc dtsi and it's document binding 
files needed to be fixed.Shall we fix it for all qcom related soc usage 
in one patch, or we'd better to split into different patches according 
to soc specifically?
> 
> I don't really want to NAKit but since you are resending without
> finishing the discussion, which is quite impolite, then let's be like that:
Acked your feelings. Feel free to NAK when you thought it is right thing 
to do. :)
> 
> NAK
> 
> Best regards,
> Krzysztof
> 

-- 
Thx and BRs,
Aiqun(Maria) Yu

Re: [PATCH v3 1/1] arm64: dts: qcom: sm8550: remove address/size-cells from mdss_dsi1

[Krzysztof Kozlowski]

On 19/12/2023 10:36, Aiqun Yu (Maria) wrote:
> 
> 
> On 12/19/2023 3:17 PM, Krzysztof Kozlowski wrote:
>> On 19/12/2023 01:31, Tengfei Fan wrote:
>>> The address/size-cells in mdss_dsi1 node have not ranges and child also
>>> have not reg, then this leads to dtc W=1 warnings:
>>
> Comments can be more readable:
> "mdss_dsi1" node don't have "ranges" or child "reg" property, while it 
> have address/size-cells properties. This caused 
> "avoid_unnecessary_addr_size" warning from dtb check.
> Remove address/size-cells properties for "mdss_dsi1" node.
> 
>> I cannot parse it. Address/size cells never have ranges or children.
>> They cannot have. These are uint32 properties.
> Pls help to comment on the revised commit message. Every time I write a 
> commit message, also takes a while for me to double confirm whether 
> others can understand me correctly as well. Feel free to let us know if 
> it is not readable to you. It will help us as non-English native developers.
>>
>>>
>>>    sm8550.dtsi:2937.27-2992.6: Warning (avoid_unnecessary_addr_size): /soc@0/display-subsystem@ae00000/dsi@ae96000:
>>>      unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
>>>
>>>
>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com>
>>> ---
>>
>> I disagreed with the patch before. You resubmit it without really
>> addressing my concerns.
>>
>> I am not sure if this is correct fix and I want to fix all of such
>> errors (there are multiple of them) in the same way. Feel free to
>> propose common solution based on arguments.
> Per my understanding, "qcom,mdss-dsi-ctrl" driver node like "mdss_dsi1" 
> don't need to have address/size-cells properties.

Just because dtc says so? And what about bindings?

> Feel free to let us know whether there is different idea of 
> "address/size-cells" needed for the "qcom,mdss-dsi-ctrl" driver node.

The bindings expressed that idea. If the binding is incorrect, fix the
binding and the DTS. If the binding is correct, provide rationale why it
somehow does not apply here etc.


Best regards,
Krzysztof

Re: [PATCH v3 1/1] arm64: dts: qcom: sm8550: remove address/size-cells from mdss_dsi1

[Aiqun Yu (Maria)]

On 12/19/2023 5:41 PM, Krzysztof Kozlowski wrote:
> On 19/12/2023 10:36, Aiqun Yu (Maria) wrote:
>>
>>
>> On 12/19/2023 3:17 PM, Krzysztof Kozlowski wrote:
>>> On 19/12/2023 01:31, Tengfei Fan wrote:
>>>> The address/size-cells in mdss_dsi1 node have not ranges and child also
>>>> have not reg, then this leads to dtc W=1 warnings:
>>>
>> Comments can be more readable:
>> "mdss_dsi1" node don't have "ranges" or child "reg" property, while it
>> have address/size-cells properties. This caused
>> "avoid_unnecessary_addr_size" warning from dtb check.
>> Remove address/size-cells properties for "mdss_dsi1" node.
>>
>>> I cannot parse it. Address/size cells never have ranges or children.
>>> They cannot have. These are uint32 properties.
>> Pls help to comment on the revised commit message. Every time I write a
>> commit message, also takes a while for me to double confirm whether
>> others can understand me correctly as well. Feel free to let us know if
>> it is not readable to you. It will help us as non-English native developers.
>>>
>>>>
>>>>     sm8550.dtsi:2937.27-2992.6: Warning (avoid_unnecessary_addr_size): /soc@0/display-subsystem@ae00000/dsi@ae96000:
>>>>       unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
>>>>
>>>>
>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>> Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com>
>>>> ---
>>>
>>> I disagreed with the patch before. You resubmit it without really
>>> addressing my concerns.
>>>
>>> I am not sure if this is correct fix and I want to fix all of such
>>> errors (there are multiple of them) in the same way. Feel free to
>>> propose common solution based on arguments.
>> Per my understanding, "qcom,mdss-dsi-ctrl" driver node like "mdss_dsi1"
>> don't need to have address/size-cells properties.
> 
> Just because dtc says so? And what about bindings?
I don't find any reason why "qcom,mdss-dsi-ctrl" driver node need to 
have address/size-cells properties. Document Bindings should also be fixed.
> 
>> Feel free to let us know whether there is different idea of
>> "address/size-cells" needed for the "qcom,mdss-dsi-ctrl" driver node.
> 
> The bindings expressed that idea. If the binding is incorrect, fix the
> binding and the DTS. If the binding is correct, provide rationale why it
> somehow does not apply here etc.
Our plan is to fix the bindings as well.

In case you have missed the question, I just re-place it here:
While there are about 22 different soc dtsi and it's document binding 
files needed to be fixed. Shall we fix it for all qcom related soc usage 
in one patch, or we'd better to split into different patches according 
to soc specifically?

> 
> 
> Best regards,
> Krzysztof
> 

-- 
Thx and BRs,
Aiqun(Maria) Yu

Re: [PATCH v3 1/1] arm64: dts: qcom: sm8550: remove address/size-cells from mdss_dsi1

[Krzysztof Kozlowski]

On 19/12/2023 11:09, Aiqun Yu (Maria) wrote:
>>>>>
>>>>>     sm8550.dtsi:2937.27-2992.6: Warning (avoid_unnecessary_addr_size): /soc@0/display-subsystem@ae00000/dsi@ae96000:
>>>>>       unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
>>>>>
>>>>>
>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>>> Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com>
>>>>> ---
>>>>
>>>> I disagreed with the patch before. You resubmit it without really
>>>> addressing my concerns.
>>>>
>>>> I am not sure if this is correct fix and I want to fix all of such
>>>> errors (there are multiple of them) in the same way. Feel free to
>>>> propose common solution based on arguments.
>>> Per my understanding, "qcom,mdss-dsi-ctrl" driver node like "mdss_dsi1"
>>> don't need to have address/size-cells properties.
>>
>> Just because dtc says so? And what about bindings?
> I don't find any reason why "qcom,mdss-dsi-ctrl" driver node need to 
> have address/size-cells properties. Document Bindings should also be fixed.

Hm, maybe we misunderstand each other but I found clear reason:
referencing common binding which mentions panels. Now, that's the reason
for DTS but of course maybe hardware is different. I did not investigate
that.

>>
>>> Feel free to let us know whether there is different idea of
>>> "address/size-cells" needed for the "qcom,mdss-dsi-ctrl" driver node.
>>
>> The bindings expressed that idea. If the binding is incorrect, fix the
>> binding and the DTS. If the binding is correct, provide rationale why it
>> somehow does not apply here etc.
> Our plan is to fix the bindings as well.
> 
> In case you have missed the question, I just re-place it here:
> While there are about 22 different soc dtsi and it's document binding 
> files needed to be fixed. Shall we fix it for all qcom related soc usage 
> in one patch, or we'd better to split into different patches according 
> to soc specifically?

Both options work for me.

Best regards,
Krzysztof

Re: [PATCH v3 1/1] arm64: dts: qcom: sm8550: remove address/size-cells from mdss_dsi1

[Dmitry Baryshkov]

On Tue, 19 Dec 2023 at 12:09, Aiqun Yu (Maria) <quic_aiquny@quicinc.com> wrote:
>
>
>
> On 12/19/2023 5:41 PM, Krzysztof Kozlowski wrote:
> > On 19/12/2023 10:36, Aiqun Yu (Maria) wrote:
> >>
> >>
> >> On 12/19/2023 3:17 PM, Krzysztof Kozlowski wrote:
> >>> On 19/12/2023 01:31, Tengfei Fan wrote:
> >>>> The address/size-cells in mdss_dsi1 node have not ranges and child also
> >>>> have not reg, then this leads to dtc W=1 warnings:
> >>>
> >> Comments can be more readable:
> >> "mdss_dsi1" node don't have "ranges" or child "reg" property, while it
> >> have address/size-cells properties. This caused
> >> "avoid_unnecessary_addr_size" warning from dtb check.
> >> Remove address/size-cells properties for "mdss_dsi1" node.
> >>
> >>> I cannot parse it. Address/size cells never have ranges or children.
> >>> They cannot have. These are uint32 properties.
> >> Pls help to comment on the revised commit message. Every time I write a
> >> commit message, also takes a while for me to double confirm whether
> >> others can understand me correctly as well. Feel free to let us know if
> >> it is not readable to you. It will help us as non-English native developers.
> >>>
> >>>>
> >>>>     sm8550.dtsi:2937.27-2992.6: Warning (avoid_unnecessary_addr_size): /soc@0/display-subsystem@ae00000/dsi@ae96000:
> >>>>       unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
> >>>>
> >>>>
> >>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >>>> Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com>
> >>>> ---
> >>>
> >>> I disagreed with the patch before. You resubmit it without really
> >>> addressing my concerns.
> >>>
> >>> I am not sure if this is correct fix and I want to fix all of such
> >>> errors (there are multiple of them) in the same way. Feel free to
> >>> propose common solution based on arguments.
> >> Per my understanding, "qcom,mdss-dsi-ctrl" driver node like "mdss_dsi1"
> >> don't need to have address/size-cells properties.
> >
> > Just because dtc says so? And what about bindings?
> I don't find any reason why "qcom,mdss-dsi-ctrl" driver node need to
> have address/size-cells properties. Document Bindings should also be fixed.
> >
> >> Feel free to let us know whether there is different idea of
> >> "address/size-cells" needed for the "qcom,mdss-dsi-ctrl" driver node.
> >
> > The bindings expressed that idea. If the binding is incorrect, fix the
> > binding and the DTS. If the binding is correct, provide rationale why it
> > somehow does not apply here etc.
> Our plan is to fix the bindings as well.
>
> In case you have missed the question, I just re-place it here:
> While there are about 22 different soc dtsi and it's document binding
> files needed to be fixed. Shall we fix it for all qcom related soc usage
> in one patch, or we'd better to split into different patches according
> to soc specifically?

Don't touch the bindings unless you understand what you are doing.
Your patch will be NAKed. There can be a DSI panel attached to the DSI
host, which means there is a need for #address-cells / #size-cells.

Please stop wasting the time on dtc warning. The bindings (and the
file) are correct.

-- 
With best wishes
Dmitry

Re: [PATCH v3 1/1] arm64: dts: qcom: sm8550: remove address/size-cells from mdss_dsi1

[Aiqun Yu (Maria)]

On 12/19/2023 6:21 PM, Dmitry Baryshkov wrote:
> On Tue, 19 Dec 2023 at 12:09, Aiqun Yu (Maria) <quic_aiquny@quicinc.com> wrote:
>>
>>
>>
>> On 12/19/2023 5:41 PM, Krzysztof Kozlowski wrote:
>>> On 19/12/2023 10:36, Aiqun Yu (Maria) wrote:
>>>>
>>>>
>>>> On 12/19/2023 3:17 PM, Krzysztof Kozlowski wrote:
>>>>> On 19/12/2023 01:31, Tengfei Fan wrote:
>>>>>> The address/size-cells in mdss_dsi1 node have not ranges and child also
>>>>>> have not reg, then this leads to dtc W=1 warnings:
>>>>>
>>>> Comments can be more readable:
>>>> "mdss_dsi1" node don't have "ranges" or child "reg" property, while it
>>>> have address/size-cells properties. This caused
>>>> "avoid_unnecessary_addr_size" warning from dtb check.
>>>> Remove address/size-cells properties for "mdss_dsi1" node.
>>>>
>>>>> I cannot parse it. Address/size cells never have ranges or children.
>>>>> They cannot have. These are uint32 properties.
>>>> Pls help to comment on the revised commit message. Every time I write a
>>>> commit message, also takes a while for me to double confirm whether
>>>> others can understand me correctly as well. Feel free to let us know if
>>>> it is not readable to you. It will help us as non-English native developers.
>>>>>
>>>>>>
>>>>>>      sm8550.dtsi:2937.27-2992.6: Warning (avoid_unnecessary_addr_size): /soc@0/display-subsystem@ae00000/dsi@ae96000:
>>>>>>        unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
>>>>>>
>>>>>>
>>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>>>> Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com>
>>>>>> ---
>>>>>
>>>>> I disagreed with the patch before. You resubmit it without really
>>>>> addressing my concerns.
>>>>>
>>>>> I am not sure if this is correct fix and I want to fix all of such
>>>>> errors (there are multiple of them) in the same way. Feel free to
>>>>> propose common solution based on arguments.
>>>> Per my understanding, "qcom,mdss-dsi-ctrl" driver node like "mdss_dsi1"
>>>> don't need to have address/size-cells properties.
>>>
>>> Just because dtc says so? And what about bindings?
>> I don't find any reason why "qcom,mdss-dsi-ctrl" driver node need to
>> have address/size-cells properties. Document Bindings should also be fixed.
>>>
>>>> Feel free to let us know whether there is different idea of
>>>> "address/size-cells" needed for the "qcom,mdss-dsi-ctrl" driver node.
>>>
>>> The bindings expressed that idea. If the binding is incorrect, fix the
>>> binding and the DTS. If the binding is correct, provide rationale why it
>>> somehow does not apply here etc.
>> Our plan is to fix the bindings as well.
>>
>> In case you have missed the question, I just re-place it here:
>> While there are about 22 different soc dtsi and it's document binding
>> files needed to be fixed. Shall we fix it for all qcom related soc usage
>> in one patch, or we'd better to split into different patches according
>> to soc specifically?
> 
> Don't touch the bindings unless you understand what you are doing.
> Your patch will be NAKed. There can be a DSI panel attached to the DSI
> host, which means there is a need for #address-cells / #size-cells.
> 
Could you please help to elaborate more on details? Like what's the 
right example here for the "qcom,mdss-dsi-ctrl" driver node needed to 
have "#address-cells"/"#size-cells".

Thx to chime in that we have put a good amount of time here.
> Please stop wasting the time on dtc warning. The bindings (and the
> file) are correct.
I don't agree here.
Either it is a wrong usage of "#address-cells"/"#size-cells", or dtc 
warning should be fixed with this usage take into account.
"dtb check" will be a good guideline for developers to follow, I don't 
think it is wasting time here.
> 

-- 
Thx and BRs,
Aiqun(Maria) Yu

Re: [PATCH v3 1/1] arm64: dts: qcom: sm8550: remove address/size-cells from mdss_dsi1

[Dmitry Baryshkov]

On Wed, 20 Dec 2023 at 02:54, Aiqun Yu (Maria) <quic_aiquny@quicinc.com> wrote:
>
>
>
> On 12/19/2023 6:21 PM, Dmitry Baryshkov wrote:
> > On Tue, 19 Dec 2023 at 12:09, Aiqun Yu (Maria) <quic_aiquny@quicinc.com> wrote:
> >>
> >>
> >>
> >> On 12/19/2023 5:41 PM, Krzysztof Kozlowski wrote:
> >>> On 19/12/2023 10:36, Aiqun Yu (Maria) wrote:
> >>>>
> >>>>
> >>>> On 12/19/2023 3:17 PM, Krzysztof Kozlowski wrote:
> >>>>> On 19/12/2023 01:31, Tengfei Fan wrote:
> >>>>>> The address/size-cells in mdss_dsi1 node have not ranges and child also
> >>>>>> have not reg, then this leads to dtc W=1 warnings:
> >>>>>
> >>>> Comments can be more readable:
> >>>> "mdss_dsi1" node don't have "ranges" or child "reg" property, while it
> >>>> have address/size-cells properties. This caused
> >>>> "avoid_unnecessary_addr_size" warning from dtb check.
> >>>> Remove address/size-cells properties for "mdss_dsi1" node.
> >>>>
> >>>>> I cannot parse it. Address/size cells never have ranges or children.
> >>>>> They cannot have. These are uint32 properties.
> >>>> Pls help to comment on the revised commit message. Every time I write a
> >>>> commit message, also takes a while for me to double confirm whether
> >>>> others can understand me correctly as well. Feel free to let us know if
> >>>> it is not readable to you. It will help us as non-English native developers.
> >>>>>
> >>>>>>
> >>>>>>      sm8550.dtsi:2937.27-2992.6: Warning (avoid_unnecessary_addr_size): /soc@0/display-subsystem@ae00000/dsi@ae96000:
> >>>>>>        unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
> >>>>>>
> >>>>>>
> >>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >>>>>> Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com>
> >>>>>> ---
> >>>>>
> >>>>> I disagreed with the patch before. You resubmit it without really
> >>>>> addressing my concerns.
> >>>>>
> >>>>> I am not sure if this is correct fix and I want to fix all of such
> >>>>> errors (there are multiple of them) in the same way. Feel free to
> >>>>> propose common solution based on arguments.
> >>>> Per my understanding, "qcom,mdss-dsi-ctrl" driver node like "mdss_dsi1"
> >>>> don't need to have address/size-cells properties.
> >>>
> >>> Just because dtc says so? And what about bindings?
> >> I don't find any reason why "qcom,mdss-dsi-ctrl" driver node need to
> >> have address/size-cells properties. Document Bindings should also be fixed.
> >>>
> >>>> Feel free to let us know whether there is different idea of
> >>>> "address/size-cells" needed for the "qcom,mdss-dsi-ctrl" driver node.
> >>>
> >>> The bindings expressed that idea. If the binding is incorrect, fix the
> >>> binding and the DTS. If the binding is correct, provide rationale why it
> >>> somehow does not apply here etc.
> >> Our plan is to fix the bindings as well.
> >>
> >> In case you have missed the question, I just re-place it here:
> >> While there are about 22 different soc dtsi and it's document binding
> >> files needed to be fixed. Shall we fix it for all qcom related soc usage
> >> in one patch, or we'd better to split into different patches according
> >> to soc specifically?
> >
> > Don't touch the bindings unless you understand what you are doing.
> > Your patch will be NAKed. There can be a DSI panel attached to the DSI
> > host, which means there is a need for #address-cells / #size-cells.
> >
> Could you please help to elaborate more on details? Like what's the
> right example here for the "qcom,mdss-dsi-ctrl" driver node needed to
> have "#address-cells"/"#size-cells".

As I wrote, the attached DSI panels make use of #address-cells / #size-cells.

Please take a look at the sdm845-mtp.dts, which provides a complex
enough example (a panel which is attached to both DSI0 and DSI1
hosts).

> Thx to chime in that we have put a good amount of time here.

Can't quite catch you here.

> > Please stop wasting the time on dtc warning. The bindings (and the
> > file) are correct.
> I don't agree here.
> Either it is a wrong usage of "#address-cells"/"#size-cells", or dtc
> warning should be fixed with this usage take into account.
> "dtb check" will be a good guideline for developers to follow, I don't
> think it is wasting time here.

It is a guideline, but not a rule. No warnings by default is more of
the rule. W=1 enables warnings that developers have to classify and
cope with.

E.g. I don't think dtc correctly handles the case when there are both
with-address and no-address nodes (e.g. 'panel@0' and 'ports'). Note,
I might be mistaken there.

> >
>
> --
> Thx and BRs,
> Aiqun(Maria) Yu



-- 
With best wishes
Dmitry

Re: [PATCH v3 1/1] arm64: dts: qcom: sm8550: remove address/size-cells from mdss_dsi1

[Aiqun Yu (Maria)]

On 12/20/2023 3:06 PM, Dmitry Baryshkov wrote:
> On Wed, 20 Dec 2023 at 02:54, Aiqun Yu (Maria) <quic_aiquny@quicinc.com> wrote:
>>
>>
>>
>> On 12/19/2023 6:21 PM, Dmitry Baryshkov wrote:
>>> On Tue, 19 Dec 2023 at 12:09, Aiqun Yu (Maria) <quic_aiquny@quicinc.com> wrote:
>>>>
>>>>
>>>>
>>>> On 12/19/2023 5:41 PM, Krzysztof Kozlowski wrote:
>>>>> On 19/12/2023 10:36, Aiqun Yu (Maria) wrote:
>>>>>>
>>>>>>
>>>>>> On 12/19/2023 3:17 PM, Krzysztof Kozlowski wrote:
>>>>>>> On 19/12/2023 01:31, Tengfei Fan wrote:
>>>>>>>> The address/size-cells in mdss_dsi1 node have not ranges and child also
>>>>>>>> have not reg, then this leads to dtc W=1 warnings:
>>>>>>>
>>>>>> Comments can be more readable:
>>>>>> "mdss_dsi1" node don't have "ranges" or child "reg" property, while it
>>>>>> have address/size-cells properties. This caused
>>>>>> "avoid_unnecessary_addr_size" warning from dtb check.
>>>>>> Remove address/size-cells properties for "mdss_dsi1" node.
>>>>>>
>>>>>>> I cannot parse it. Address/size cells never have ranges or children.
>>>>>>> They cannot have. These are uint32 properties.
>>>>>> Pls help to comment on the revised commit message. Every time I write a
>>>>>> commit message, also takes a while for me to double confirm whether
>>>>>> others can understand me correctly as well. Feel free to let us know if
>>>>>> it is not readable to you. It will help us as non-English native developers.
>>>>>>>
>>>>>>>>
>>>>>>>>       sm8550.dtsi:2937.27-2992.6: Warning (avoid_unnecessary_addr_size): /soc@0/display-subsystem@ae00000/dsi@ae96000:
>>>>>>>>         unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
>>>>>>>>
>>>>>>>>
>>>>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>>>>>> Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com>
>>>>>>>> ---
>>>>>>>
>>>>>>> I disagreed with the patch before. You resubmit it without really
>>>>>>> addressing my concerns.
>>>>>>>
>>>>>>> I am not sure if this is correct fix and I want to fix all of such
>>>>>>> errors (there are multiple of them) in the same way. Feel free to
>>>>>>> propose common solution based on arguments.
>>>>>> Per my understanding, "qcom,mdss-dsi-ctrl" driver node like "mdss_dsi1"
>>>>>> don't need to have address/size-cells properties.
>>>>>
>>>>> Just because dtc says so? And what about bindings?
>>>> I don't find any reason why "qcom,mdss-dsi-ctrl" driver node need to
>>>> have address/size-cells properties. Document Bindings should also be fixed.
>>>>>
>>>>>> Feel free to let us know whether there is different idea of
>>>>>> "address/size-cells" needed for the "qcom,mdss-dsi-ctrl" driver node.
>>>>>
>>>>> The bindings expressed that idea. If the binding is incorrect, fix the
>>>>> binding and the DTS. If the binding is correct, provide rationale why it
>>>>> somehow does not apply here etc.
>>>> Our plan is to fix the bindings as well.
>>>>
>>>> In case you have missed the question, I just re-place it here:
>>>> While there are about 22 different soc dtsi and it's document binding
>>>> files needed to be fixed. Shall we fix it for all qcom related soc usage
>>>> in one patch, or we'd better to split into different patches according
>>>> to soc specifically?
>>>
>>> Don't touch the bindings unless you understand what you are doing.
>>> Your patch will be NAKed. There can be a DSI panel attached to the DSI
>>> host, which means there is a need for #address-cells / #size-cells.
>>>
>> Could you please help to elaborate more on details? Like what's the
>> right example here for the "qcom,mdss-dsi-ctrl" driver node needed to
>> have "#address-cells"/"#size-cells".
> 
> As I wrote, the attached DSI panels make use of #address-cells / #size-cells.
> 
> Please take a look at the sdm845-mtp.dts, which provides a complex
> enough example (a panel which is attached to both DSI0 and DSI1
> hosts).
I can see the panel example now.
While panel@0 likely node is not at in the binding that I've checked. 
There are quite a few of binding document about the same driver. I 
checked 5 of the bindings document and moste of them are alike, and 
don't have the panel example.:(
> 
>> Thx to chime in that we have put a good amount of time here.
> 
> Can't quite catch you here.
> 
>>> Please stop wasting the time on dtc warning. The bindings (and the
>>> file) are correct.
>> I don't agree here.
>> Either it is a wrong usage of "#address-cells"/"#size-cells", or dtc
>> warning should be fixed with this usage take into account.
>> "dtb check" will be a good guideline for developers to follow, I don't
>> think it is wasting time here.
> 
> It is a guideline, but not a rule. No warnings by default is more of
> the rule. W=1 enables warnings that developers have to classify and
> cope with.
> 
> E.g. I don't think dtc correctly handles the case when there are both
> with-address and no-address nodes (e.g. 'panel@0' and 'ports'). Note,
> I might be mistaken there.
Now I understand the issue, here is some thinking from my end, and 
welcome others to chime in with more ideas:
1. Only put "#address-cells" "#size-cells" right before node of panel@0.
2. Align current binding document with "panel@0" examples.
3. Too many bindings files for driver "qcom,mdss-dsi-ctrl", shall we 
align them into 1 binding files.
4. enhance the dtc warning check if we still want to have 
"#address-cells" "#size-cells" even if there is no "panel@0" attached.

@krzy here I try to answer your comments here as well.
I am disagree on leave the warning as it is. And want to do something to 
improve this. Ideas above.
Let me know if any comments is not right addressed from your comments.
> 
>>>
>>
>> --
>> Thx and BRs,
>> Aiqun(Maria) Yu
> 
> 
> 

-- 
Thx and BRs,
Aiqun(Maria) Yu

Re: [PATCH v3 1/1] arm64: dts: qcom: sm8550: remove address/size-cells from mdss_dsi1

[Dmitry Baryshkov]

On 20 December 2023 12:33:07 EET, "Aiqun Yu (Maria)" <quic_aiquny@quicinc.com> wrote:
>
>
>On 12/20/2023 3:06 PM, Dmitry Baryshkov wrote:
>> On Wed, 20 Dec 2023 at 02:54, Aiqun Yu (Maria) <quic_aiquny@quicinc.com> wrote:
>>> 
>>> 
>>> 
>>> On 12/19/2023 6:21 PM, Dmitry Baryshkov wrote:
>>>> On Tue, 19 Dec 2023 at 12:09, Aiqun Yu (Maria) <quic_aiquny@quicinc.com> wrote:
>>>>> 
>>>>> 
>>>>> 
>>>>> On 12/19/2023 5:41 PM, Krzysztof Kozlowski wrote:
>>>>>> On 19/12/2023 10:36, Aiqun Yu (Maria) wrote:
>>>>>>> 
>>>>>>> 
>>>>>>> On 12/19/2023 3:17 PM, Krzysztof Kozlowski wrote:
>>>>>>>> On 19/12/2023 01:31, Tengfei Fan wrote:
>>>>>>>>> The address/size-cells in mdss_dsi1 node have not ranges and child also
>>>>>>>>> have not reg, then this leads to dtc W=1 warnings:
>>>>>>>> 
>>>>>>> Comments can be more readable:
>>>>>>> "mdss_dsi1" node don't have "ranges" or child "reg" property, while it
>>>>>>> have address/size-cells properties. This caused
>>>>>>> "avoid_unnecessary_addr_size" warning from dtb check.
>>>>>>> Remove address/size-cells properties for "mdss_dsi1" node.
>>>>>>> 
>>>>>>>> I cannot parse it. Address/size cells never have ranges or children.
>>>>>>>> They cannot have. These are uint32 properties.
>>>>>>> Pls help to comment on the revised commit message. Every time I write a
>>>>>>> commit message, also takes a while for me to double confirm whether
>>>>>>> others can understand me correctly as well. Feel free to let us know if
>>>>>>> it is not readable to you. It will help us as non-English native developers.
>>>>>>>> 
>>>>>>>>> 
>>>>>>>>>       sm8550.dtsi:2937.27-2992.6: Warning (avoid_unnecessary_addr_size): /soc@0/display-subsystem@ae00000/dsi@ae96000:
>>>>>>>>>         unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>>>>>>> Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com>
>>>>>>>>> ---
>>>>>>>> 
>>>>>>>> I disagreed with the patch before. You resubmit it without really
>>>>>>>> addressing my concerns.
>>>>>>>> 
>>>>>>>> I am not sure if this is correct fix and I want to fix all of such
>>>>>>>> errors (there are multiple of them) in the same way. Feel free to
>>>>>>>> propose common solution based on arguments.
>>>>>>> Per my understanding, "qcom,mdss-dsi-ctrl" driver node like "mdss_dsi1"
>>>>>>> don't need to have address/size-cells properties.
>>>>>> 
>>>>>> Just because dtc says so? And what about bindings?
>>>>> I don't find any reason why "qcom,mdss-dsi-ctrl" driver node need to
>>>>> have address/size-cells properties. Document Bindings should also be fixed.
>>>>>> 
>>>>>>> Feel free to let us know whether there is different idea of
>>>>>>> "address/size-cells" needed for the "qcom,mdss-dsi-ctrl" driver node.
>>>>>> 
>>>>>> The bindings expressed that idea. If the binding is incorrect, fix the
>>>>>> binding and the DTS. If the binding is correct, provide rationale why it
>>>>>> somehow does not apply here etc.
>>>>> Our plan is to fix the bindings as well.
>>>>> 
>>>>> In case you have missed the question, I just re-place it here:
>>>>> While there are about 22 different soc dtsi and it's document binding
>>>>> files needed to be fixed. Shall we fix it for all qcom related soc usage
>>>>> in one patch, or we'd better to split into different patches according
>>>>> to soc specifically?
>>>> 
>>>> Don't touch the bindings unless you understand what you are doing.
>>>> Your patch will be NAKed. There can be a DSI panel attached to the DSI
>>>> host, which means there is a need for #address-cells / #size-cells.
>>>> 
>>> Could you please help to elaborate more on details? Like what's the
>>> right example here for the "qcom,mdss-dsi-ctrl" driver node needed to
>>> have "#address-cells"/"#size-cells".
>> 
>> As I wrote, the attached DSI panels make use of #address-cells / #size-cells.
>> 
>> Please take a look at the sdm845-mtp.dts, which provides a complex
>> enough example (a panel which is attached to both DSI0 and DSI1
>> hosts).
>I can see the panel example now.
>While panel@0 likely node is not at in the binding that I've checked. There are quite a few of binding document about the same driver. I checked 5 of the bindings document and moste of them are alike, and don't have the panel example.:(

There is a single bindings documents describing MSM DSI controller, display/MSM/dsi-controller-main.yaml . It explicitly includes ../dsi-controller.yaml, which describes generic DSI host controller. The latter one includes an example of the DSI panel. MSM DSI bindings do not have to include one, there is nothing platform specific there.


>> 
>>> Thx to chime in that we have put a good amount of time here.
>> 
>> Can't quite catch you here.
>> 
>>>> Please stop wasting the time on dtc warning. The bindings (and the
>>>> file) are correct.
>>> I don't agree here.
>>> Either it is a wrong usage of "#address-cells"/"#size-cells", or dtc
>>> warning should be fixed with this usage take into account.
>>> "dtb check" will be a good guideline for developers to follow, I don't
>>> think it is wasting time here.
>> 
>> It is a guideline, but not a rule. No warnings by default is more of
>> the rule. W=1 enables warnings that developers have to classify and
>> cope with.
>> 
>> E.g. I don't think dtc correctly handles the case when there are both
>> with-address and no-address nodes (e.g. 'panel@0' and 'ports'). Note,
>> I might be mistaken there.
>Now I understand the issue, here is some thinking from my end, and welcome others to chime in with more ideas:
>1. Only put "#address-cells" "#size-cells" right before node of panel@0

No. Cells specification is a property of the host/bus. As such they do not belong to the board DT file.

>2. Align current binding document with "panel@0" examples.

There is already a panel in dsi-controller.yaml. Adding another example is optional. Do you also need an example with the external DSI bridge? 


>3. Too many bindings files for driver "qcom,mdss-dsi-ctrl", shall we align them into 1 binding files.

There is just one.


>4. enhance the dtc warning check if we still want to have "#address-cells" "#size-cells" even if there is no "panel@0" attached.

In my opinion this is a way to go, if any. Did you include devicetree@ ML and the corresponding maintainers into the discussion?

>
>@krzy here I try to answer your comments here as well.
>I am disagree on leave the warning as it is. And want to do something to improve this. Ideas above.
>Let me know if any comments is not right addressed from your comments.
>> 
>>>> 
>>> 
>>> --
>>> Thx and BRs,
>>> Aiqun(Maria) Yu
>> 
>> 
>> 
>

Re: [PATCH v3 1/1] arm64: dts: qcom: sm8550: remove address/size-cells from mdss_dsi1

[Aiqun Yu (Maria)]

On 12/20/2023 7:10 PM, Dmitry Baryshkov wrote:
> On 20 December 2023 12:33:07 EET, "Aiqun Yu (Maria)" <quic_aiquny@quicinc.com> wrote:
>>
>>
>> On 12/20/2023 3:06 PM, Dmitry Baryshkov wrote:
>>> On Wed, 20 Dec 2023 at 02:54, Aiqun Yu (Maria) <quic_aiquny@quicinc.com> wrote:
>>>>
>>>>
>>>>
>>>> On 12/19/2023 6:21 PM, Dmitry Baryshkov wrote:
>>>>> On Tue, 19 Dec 2023 at 12:09, Aiqun Yu (Maria) <quic_aiquny@quicinc.com> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 12/19/2023 5:41 PM, Krzysztof Kozlowski wrote:
>>>>>>> On 19/12/2023 10:36, Aiqun Yu (Maria) wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 12/19/2023 3:17 PM, Krzysztof Kozlowski wrote:
>>>>>>>>> On 19/12/2023 01:31, Tengfei Fan wrote:
>>>>>>>>>> The address/size-cells in mdss_dsi1 node have not ranges and child also
>>>>>>>>>> have not reg, then this leads to dtc W=1 warnings:
>>>>>>>>>
>>>>>>>> Comments can be more readable:
>>>>>>>> "mdss_dsi1" node don't have "ranges" or child "reg" property, while it
>>>>>>>> have address/size-cells properties. This caused
>>>>>>>> "avoid_unnecessary_addr_size" warning from dtb check.
>>>>>>>> Remove address/size-cells properties for "mdss_dsi1" node.
>>>>>>>>
>>>>>>>>> I cannot parse it. Address/size cells never have ranges or children.
>>>>>>>>> They cannot have. These are uint32 properties.
>>>>>>>> Pls help to comment on the revised commit message. Every time I write a
>>>>>>>> commit message, also takes a while for me to double confirm whether
>>>>>>>> others can understand me correctly as well. Feel free to let us know if
>>>>>>>> it is not readable to you. It will help us as non-English native developers.
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>        sm8550.dtsi:2937.27-2992.6: Warning (avoid_unnecessary_addr_size): /soc@0/display-subsystem@ae00000/dsi@ae96000:
>>>>>>>>>>          unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>>>>>>>> Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com>
>>>>>>>>>> ---
>>>>>>>>>
>>>>>>>>> I disagreed with the patch before. You resubmit it without really
>>>>>>>>> addressing my concerns.
>>>>>>>>>
>>>>>>>>> I am not sure if this is correct fix and I want to fix all of such
>>>>>>>>> errors (there are multiple of them) in the same way. Feel free to
>>>>>>>>> propose common solution based on arguments.
>>>>>>>> Per my understanding, "qcom,mdss-dsi-ctrl" driver node like "mdss_dsi1"
>>>>>>>> don't need to have address/size-cells properties.
>>>>>>>
>>>>>>> Just because dtc says so? And what about bindings?
>>>>>> I don't find any reason why "qcom,mdss-dsi-ctrl" driver node need to
>>>>>> have address/size-cells properties. Document Bindings should also be fixed.
>>>>>>>
>>>>>>>> Feel free to let us know whether there is different idea of
>>>>>>>> "address/size-cells" needed for the "qcom,mdss-dsi-ctrl" driver node.
>>>>>>>
>>>>>>> The bindings expressed that idea. If the binding is incorrect, fix the
>>>>>>> binding and the DTS. If the binding is correct, provide rationale why it
>>>>>>> somehow does not apply here etc.
>>>>>> Our plan is to fix the bindings as well.
>>>>>>
>>>>>> In case you have missed the question, I just re-place it here:
>>>>>> While there are about 22 different soc dtsi and it's document binding
>>>>>> files needed to be fixed. Shall we fix it for all qcom related soc usage
>>>>>> in one patch, or we'd better to split into different patches according
>>>>>> to soc specifically?
>>>>>
>>>>> Don't touch the bindings unless you understand what you are doing.
>>>>> Your patch will be NAKed. There can be a DSI panel attached to the DSI
>>>>> host, which means there is a need for #address-cells / #size-cells.
>>>>>
>>>> Could you please help to elaborate more on details? Like what's the
>>>> right example here for the "qcom,mdss-dsi-ctrl" driver node needed to
>>>> have "#address-cells"/"#size-cells".
>>>
>>> As I wrote, the attached DSI panels make use of #address-cells / #size-cells.
>>>
>>> Please take a look at the sdm845-mtp.dts, which provides a complex
>>> enough example (a panel which is attached to both DSI0 and DSI1
>>> hosts).
>> I can see the panel example now.
>> While panel@0 likely node is not at in the binding that I've checked. There are quite a few of binding document about the same driver. I checked 5 of the bindings document and moste of them are alike, and don't have the panel example.:(
> 
> There is a single bindings documents describing MSM DSI controller, display/MSM/dsi-controller-main.yaml . It explicitly includes ../dsi-controller.yaml, which describes generic DSI host controller. The latter one includes an example of the DSI panel. MSM DSI bindings do not have to include one, there is nothing platform specific there.
> 
> 
>>>
>>>> Thx to chime in that we have put a good amount of time here.
>>>
>>> Can't quite catch you here.
>>>
>>>>> Please stop wasting the time on dtc warning. The bindings (and the
>>>>> file) are correct.
>>>> I don't agree here.
>>>> Either it is a wrong usage of "#address-cells"/"#size-cells", or dtc
>>>> warning should be fixed with this usage take into account.
>>>> "dtb check" will be a good guideline for developers to follow, I don't
>>>> think it is wasting time here.
>>>
>>> It is a guideline, but not a rule. No warnings by default is more of
>>> the rule. W=1 enables warnings that developers have to classify and
>>> cope with.
>>>
>>> E.g. I don't think dtc correctly handles the case when there are both
>>> with-address and no-address nodes (e.g. 'panel@0' and 'ports'). Note,
>>> I might be mistaken there.
>> Now I understand the issue, here is some thinking from my end, and welcome others to chime in with more ideas:
>> 1. Only put "#address-cells" "#size-cells" right before node of panel@0
> 
> No. Cells specification is a property of the host/bus. As such they do not belong to the board DT file.
As "#address-cells" "#size-cells" is not marked as required properties 
in the Document dsi-controller.yaml. Did it really needed even 
"panel@[0-3]" is not present at current dsi node?
That's good that comes to the initial discussion of current patch here. :)

I can understand that "#address-cells" "#size-cells" cannot be device 
tree overlayed by dtbo. While when there is no "panel@[0-3]" nodes shall 
we also remove "#address-cells" "#size-cells" properties for dsi node?
> 
>> 2. Align current binding document with "panel@0" examples.
> 
> There is already a panel in dsi-controller.yaml. Adding another example is optional. Do you also need an example with the external DSI bridge?
Current binding I am talking about is current patch binding file:
qcom,sm8550-mdss.yaml

It have a ref to mdss-common.yaml,  but I cannot find the ref direct me 
to dsi-controller.yaml.
In my opinion if the example have "#address-cells" "#size-cells", then 
it's better to also include "panel@0" with "reg = <0>" to not confuse.
> 
> 
>> 3. Too many bindings files for driver "qcom,mdss-dsi-ctrl", shall we align them into 1 binding files.
> 
> There is just one.
Currently I mentioned bindings files was searched the compatible 
"qcom,mdss-dsi-ctrl", and find binding docs like "qcom,sm8550-mdss.yaml" 
"qcom,sm8450-mdss.yaml" etc.
There is duplicate information on "qcom,sm8550-mdss.yaml" etc, while 
"qcom,mdss-common.yaml" is not common enough for my understanding.
> 
> 
>> 4. enhance the dtc warning check if we still want to have "#address-cells" "#size-cells" even if there is no "panel@0" attached.
> 
> In my opinion this is a way to go, if any. Did you include devicetree@ ML and the corresponding maintainers into the discussion?
Already included devicetree@ ML at the very beginning.
If the required properties part in each yaml is marked good enough, I 
think it can be an input for avoid unnecessary dtc warnings.
> 
>>
>> @krzy here I try to answer your comments here as well.
>> I am disagree on leave the warning as it is. And want to do something to improve this. Ideas above.
>> Let me know if any comments is not right addressed from your comments.
>>>
>>>>>
>>>>
>>>> --
>>>> Thx and BRs,
>>>> Aiqun(Maria) Yu
>>>
>>>
>>>
>>
> 

-- 
Thx and BRs,
Aiqun(Maria) Yu

Re: [PATCH v3 1/1] arm64: dts: qcom: sm8550: remove address/size-cells from mdss_dsi1

[Dmitry Baryshkov]

On Thu, 21 Dec 2023 at 03:57, Aiqun Yu (Maria) <quic_aiquny@quicinc.com> wrote:
>
>
>
> On 12/20/2023 7:10 PM, Dmitry Baryshkov wrote:
> > On 20 December 2023 12:33:07 EET, "Aiqun Yu (Maria)" <quic_aiquny@quicinc.com> wrote:
> >>
> >>
> >> On 12/20/2023 3:06 PM, Dmitry Baryshkov wrote:
> >>> On Wed, 20 Dec 2023 at 02:54, Aiqun Yu (Maria) <quic_aiquny@quicinc.com> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On 12/19/2023 6:21 PM, Dmitry Baryshkov wrote:
> >>>>> On Tue, 19 Dec 2023 at 12:09, Aiqun Yu (Maria) <quic_aiquny@quicinc.com> wrote:
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> On 12/19/2023 5:41 PM, Krzysztof Kozlowski wrote:
> >>>>>>> On 19/12/2023 10:36, Aiqun Yu (Maria) wrote:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On 12/19/2023 3:17 PM, Krzysztof Kozlowski wrote:
> >>>>>>>>> On 19/12/2023 01:31, Tengfei Fan wrote:
> >>>>>>>>>> The address/size-cells in mdss_dsi1 node have not ranges and child also
> >>>>>>>>>> have not reg, then this leads to dtc W=1 warnings:
> >>>>>>>>>
> >>>>>>>> Comments can be more readable:
> >>>>>>>> "mdss_dsi1" node don't have "ranges" or child "reg" property, while it
> >>>>>>>> have address/size-cells properties. This caused
> >>>>>>>> "avoid_unnecessary_addr_size" warning from dtb check.
> >>>>>>>> Remove address/size-cells properties for "mdss_dsi1" node.
> >>>>>>>>
> >>>>>>>>> I cannot parse it. Address/size cells never have ranges or children.
> >>>>>>>>> They cannot have. These are uint32 properties.
> >>>>>>>> Pls help to comment on the revised commit message. Every time I write a
> >>>>>>>> commit message, also takes a while for me to double confirm whether
> >>>>>>>> others can understand me correctly as well. Feel free to let us know if
> >>>>>>>> it is not readable to you. It will help us as non-English native developers.
> >>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>        sm8550.dtsi:2937.27-2992.6: Warning (avoid_unnecessary_addr_size): /soc@0/display-subsystem@ae00000/dsi@ae96000:
> >>>>>>>>>>          unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >>>>>>>>>> Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com>
> >>>>>>>>>> ---
> >>>>>>>>>
> >>>>>>>>> I disagreed with the patch before. You resubmit it without really
> >>>>>>>>> addressing my concerns.
> >>>>>>>>>
> >>>>>>>>> I am not sure if this is correct fix and I want to fix all of such
> >>>>>>>>> errors (there are multiple of them) in the same way. Feel free to
> >>>>>>>>> propose common solution based on arguments.
> >>>>>>>> Per my understanding, "qcom,mdss-dsi-ctrl" driver node like "mdss_dsi1"
> >>>>>>>> don't need to have address/size-cells properties.
> >>>>>>>
> >>>>>>> Just because dtc says so? And what about bindings?
> >>>>>> I don't find any reason why "qcom,mdss-dsi-ctrl" driver node need to
> >>>>>> have address/size-cells properties. Document Bindings should also be fixed.
> >>>>>>>
> >>>>>>>> Feel free to let us know whether there is different idea of
> >>>>>>>> "address/size-cells" needed for the "qcom,mdss-dsi-ctrl" driver node.
> >>>>>>>
> >>>>>>> The bindings expressed that idea. If the binding is incorrect, fix the
> >>>>>>> binding and the DTS. If the binding is correct, provide rationale why it
> >>>>>>> somehow does not apply here etc.
> >>>>>> Our plan is to fix the bindings as well.
> >>>>>>
> >>>>>> In case you have missed the question, I just re-place it here:
> >>>>>> While there are about 22 different soc dtsi and it's document binding
> >>>>>> files needed to be fixed. Shall we fix it for all qcom related soc usage
> >>>>>> in one patch, or we'd better to split into different patches according
> >>>>>> to soc specifically?
> >>>>>
> >>>>> Don't touch the bindings unless you understand what you are doing.
> >>>>> Your patch will be NAKed. There can be a DSI panel attached to the DSI
> >>>>> host, which means there is a need for #address-cells / #size-cells.
> >>>>>
> >>>> Could you please help to elaborate more on details? Like what's the
> >>>> right example here for the "qcom,mdss-dsi-ctrl" driver node needed to
> >>>> have "#address-cells"/"#size-cells".
> >>>
> >>> As I wrote, the attached DSI panels make use of #address-cells / #size-cells.
> >>>
> >>> Please take a look at the sdm845-mtp.dts, which provides a complex
> >>> enough example (a panel which is attached to both DSI0 and DSI1
> >>> hosts).
> >> I can see the panel example now.
> >> While panel@0 likely node is not at in the binding that I've checked. There are quite a few of binding document about the same driver. I checked 5 of the bindings document and moste of them are alike, and don't have the panel example.:(
> >
> > There is a single bindings documents describing MSM DSI controller, display/MSM/dsi-controller-main.yaml . It explicitly includes ../dsi-controller.yaml, which describes generic DSI host controller. The latter one includes an example of the DSI panel. MSM DSI bindings do not have to include one, there is nothing platform specific there.
> >
> >
> >>>
> >>>> Thx to chime in that we have put a good amount of time here.
> >>>
> >>> Can't quite catch you here.
> >>>
> >>>>> Please stop wasting the time on dtc warning. The bindings (and the
> >>>>> file) are correct.
> >>>> I don't agree here.
> >>>> Either it is a wrong usage of "#address-cells"/"#size-cells", or dtc
> >>>> warning should be fixed with this usage take into account.
> >>>> "dtb check" will be a good guideline for developers to follow, I don't
> >>>> think it is wasting time here.
> >>>
> >>> It is a guideline, but not a rule. No warnings by default is more of
> >>> the rule. W=1 enables warnings that developers have to classify and
> >>> cope with.
> >>>
> >>> E.g. I don't think dtc correctly handles the case when there are both
> >>> with-address and no-address nodes (e.g. 'panel@0' and 'ports'). Note,
> >>> I might be mistaken there.
> >> Now I understand the issue, here is some thinking from my end, and welcome others to chime in with more ideas:
> >> 1. Only put "#address-cells" "#size-cells" right before node of panel@0
> >
> > No. Cells specification is a property of the host/bus. As such they do not belong to the board DT file.
> As "#address-cells" "#size-cells" is not marked as required properties
> in the Document dsi-controller.yaml. Did it really needed even
> "panel@[0-3]" is not present at current dsi node?
> That's good that comes to the initial discussion of current patch here. :)

The #address-cells / #size-cells describe the addressing of the bus.
The bus still continues to exist even with no panel being attached.

>
> I can understand that "#address-cells" "#size-cells" cannot be device
> tree overlayed by dtbo. While when there is no "panel@[0-3]" nodes shall
> we also remove "#address-cells" "#size-cells" properties for dsi node?

But why?

> >
> >> 2. Align current binding document with "panel@0" examples.
> >
> > There is already a panel in dsi-controller.yaml. Adding another example is optional. Do you also need an example with the external DSI bridge?
> Current binding I am talking about is current patch binding file:
> qcom,sm8550-mdss.yaml

Why do you call it a patch? Also if you have read the description, you
would have known that the bindings describe the Mobile Display
Subsystem (MDSS) itself. And then it explicitly tells that the MDSS on
that platform has (among others) DSI blocks with proper compatibles.

> It have a ref to mdss-common.yaml,  but I cannot find the ref direct me
> to dsi-controller.yaml.

Because qcom,sm8550-mdss.yaml doesn't describe the DSI controller. I
think I have already mentioned a file that describes the DSI
controller, it is called display/msm/dsi-controller-main.yaml. Not the
best name, but it is quickly revealed by grepping for either of the
DSI compatible strings. And the dsi-controller-main.yaml has `  -
$ref: ../dsi-controller.yaml#` string inside.

> In my opinion if the example have "#address-cells" "#size-cells", then
> it's better to also include "panel@0" with "reg = <0>" to not confuse.

It is already there, see dsi-controller.yaml.

> >> 3. Too many bindings files for driver "qcom,mdss-dsi-ctrl", shall we align them into 1 binding files.
> >
> > There is just one.
> Currently I mentioned bindings files was searched the compatible
> "qcom,mdss-dsi-ctrl", and find binding docs like "qcom,sm8550-mdss.yaml"
> "qcom,sm8450-mdss.yaml" etc.
> There is duplicate information on "qcom,sm8550-mdss.yaml" etc, while
> "qcom,mdss-common.yaml" is not common enough for my understanding.

If you had compared the qcom,SOC-mdss.yaml, you would have seen that
they provide tight binding between compatible strings used for all the
subblocks. The `mdss-common.yaml` describes MDSS common properties. It
describes everything except the platform specifics. It can not be made
more common. And there is no duplication.

If you think you can improve the bindings, please send the patches.
They must pass the `make dt_binding_check` check.

> >> 4. enhance the dtc warning check if we still want to have "#address-cells" "#size-cells" even if there is no "panel@0" attached.
> >
> > In my opinion this is a way to go, if any. Did you include devicetree@ ML and the corresponding maintainers into the discussion?
> Already included devicetree@ ML at the very beginning.

Good, thanks for the confirmation.

> If the required properties part in each yaml is marked good enough, I
> think it can be an input for avoid unnecessary dtc warnings.

Patches are welcome.

> >
> >>
> >> @krzy here I try to answer your comments here as well.
> >> I am disagree on leave the warning as it is. And want to do something to improve this. Ideas above.
> >> Let me know if any comments is not right addressed from your comments.

-- 
With best wishes
Dmitry

Re: [PATCH v3 1/1] arm64: dts: qcom: sm8550: remove address/size-cells from mdss_dsi1

[Aiqun Yu (Maria)]

On 12/21/2023 2:59 PM, Dmitry Baryshkov wrote:
> On Thu, 21 Dec 2023 at 03:57, Aiqun Yu (Maria) <quic_aiquny@quicinc.com> wrote:
>>
>>
>>
>> On 12/20/2023 7:10 PM, Dmitry Baryshkov wrote:
>>> On 20 December 2023 12:33:07 EET, "Aiqun Yu (Maria)" <quic_aiquny@quicinc.com> wrote:
>>>>
>>>>
>>>> On 12/20/2023 3:06 PM, Dmitry Baryshkov wrote:
>>>>> On Wed, 20 Dec 2023 at 02:54, Aiqun Yu (Maria) <quic_aiquny@quicinc.com> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 12/19/2023 6:21 PM, Dmitry Baryshkov wrote:
>>>>>>> On Tue, 19 Dec 2023 at 12:09, Aiqun Yu (Maria) <quic_aiquny@quicinc.com> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 12/19/2023 5:41 PM, Krzysztof Kozlowski wrote:
>>>>>>>>> On 19/12/2023 10:36, Aiqun Yu (Maria) wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 12/19/2023 3:17 PM, Krzysztof Kozlowski wrote:
>>>>>>>>>>> On 19/12/2023 01:31, Tengfei Fan wrote:
>>>>>>>>>>>> The address/size-cells in mdss_dsi1 node have not ranges and child also
>>>>>>>>>>>> have not reg, then this leads to dtc W=1 warnings:
>>>>>>>>>>>
>>>>>>>>>> Comments can be more readable:
>>>>>>>>>> "mdss_dsi1" node don't have "ranges" or child "reg" property, while it
>>>>>>>>>> have address/size-cells properties. This caused
>>>>>>>>>> "avoid_unnecessary_addr_size" warning from dtb check.
>>>>>>>>>> Remove address/size-cells properties for "mdss_dsi1" node.
>>>>>>>>>>
>>>>>>>>>>> I cannot parse it. Address/size cells never have ranges or children.
>>>>>>>>>>> They cannot have. These are uint32 properties.
>>>>>>>>>> Pls help to comment on the revised commit message. Every time I write a
>>>>>>>>>> commit message, also takes a while for me to double confirm whether
>>>>>>>>>> others can understand me correctly as well. Feel free to let us know if
>>>>>>>>>> it is not readable to you. It will help us as non-English native developers.
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>         sm8550.dtsi:2937.27-2992.6: Warning (avoid_unnecessary_addr_size): /soc@0/display-subsystem@ae00000/dsi@ae96000:
>>>>>>>>>>>>           unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>>>>>>>>>> Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com>
>>>>>>>>>>>> ---
>>>>>>>>>>>
>>>>>>>>>>> I disagreed with the patch before. You resubmit it without really
>>>>>>>>>>> addressing my concerns.
>>>>>>>>>>>
>>>>>>>>>>> I am not sure if this is correct fix and I want to fix all of such
>>>>>>>>>>> errors (there are multiple of them) in the same way. Feel free to
>>>>>>>>>>> propose common solution based on arguments.
>>>>>>>>>> Per my understanding, "qcom,mdss-dsi-ctrl" driver node like "mdss_dsi1"
>>>>>>>>>> don't need to have address/size-cells properties.
>>>>>>>>>
>>>>>>>>> Just because dtc says so? And what about bindings?
>>>>>>>> I don't find any reason why "qcom,mdss-dsi-ctrl" driver node need to
>>>>>>>> have address/size-cells properties. Document Bindings should also be fixed.
>>>>>>>>>
>>>>>>>>>> Feel free to let us know whether there is different idea of
>>>>>>>>>> "address/size-cells" needed for the "qcom,mdss-dsi-ctrl" driver node.
>>>>>>>>>
>>>>>>>>> The bindings expressed that idea. If the binding is incorrect, fix the
>>>>>>>>> binding and the DTS. If the binding is correct, provide rationale why it
>>>>>>>>> somehow does not apply here etc.
>>>>>>>> Our plan is to fix the bindings as well.
>>>>>>>>
>>>>>>>> In case you have missed the question, I just re-place it here:
>>>>>>>> While there are about 22 different soc dtsi and it's document binding
>>>>>>>> files needed to be fixed. Shall we fix it for all qcom related soc usage
>>>>>>>> in one patch, or we'd better to split into different patches according
>>>>>>>> to soc specifically?
>>>>>>>
>>>>>>> Don't touch the bindings unless you understand what you are doing.
>>>>>>> Your patch will be NAKed. There can be a DSI panel attached to the DSI
>>>>>>> host, which means there is a need for #address-cells / #size-cells.
>>>>>>>
>>>>>> Could you please help to elaborate more on details? Like what's the
>>>>>> right example here for the "qcom,mdss-dsi-ctrl" driver node needed to
>>>>>> have "#address-cells"/"#size-cells".
>>>>>
>>>>> As I wrote, the attached DSI panels make use of #address-cells / #size-cells.
>>>>>
>>>>> Please take a look at the sdm845-mtp.dts, which provides a complex
>>>>> enough example (a panel which is attached to both DSI0 and DSI1
>>>>> hosts).
>>>> I can see the panel example now.
>>>> While panel@0 likely node is not at in the binding that I've checked. There are quite a few of binding document about the same driver. I checked 5 of the bindings document and moste of them are alike, and don't have the panel example.:(
>>>
>>> There is a single bindings documents describing MSM DSI controller, display/MSM/dsi-controller-main.yaml . It explicitly includes ../dsi-controller.yaml, which describes generic DSI host controller. The latter one includes an example of the DSI panel. MSM DSI bindings do not have to include one, there is nothing platform specific there.
>>>
>>>
>>>>>
>>>>>> Thx to chime in that we have put a good amount of time here.
>>>>>
>>>>> Can't quite catch you here.
>>>>>
>>>>>>> Please stop wasting the time on dtc warning. The bindings (and the
>>>>>>> file) are correct.
>>>>>> I don't agree here.
>>>>>> Either it is a wrong usage of "#address-cells"/"#size-cells", or dtc
>>>>>> warning should be fixed with this usage take into account.
>>>>>> "dtb check" will be a good guideline for developers to follow, I don't
>>>>>> think it is wasting time here.
>>>>>
>>>>> It is a guideline, but not a rule. No warnings by default is more of
>>>>> the rule. W=1 enables warnings that developers have to classify and
>>>>> cope with.
>>>>>
>>>>> E.g. I don't think dtc correctly handles the case when there are both
>>>>> with-address and no-address nodes (e.g. 'panel@0' and 'ports'). Note,
>>>>> I might be mistaken there.
>>>> Now I understand the issue, here is some thinking from my end, and welcome others to chime in with more ideas:
>>>> 1. Only put "#address-cells" "#size-cells" right before node of panel@0
>>>
>>> No. Cells specification is a property of the host/bus. As such they do not belong to the board DT file.
>> As "#address-cells" "#size-cells" is not marked as required properties
>> in the Document dsi-controller.yaml. Did it really needed even
>> "panel@[0-3]" is not present at current dsi node?
>> That's good that comes to the initial discussion of current patch here. :)
> 
> The #address-cells / #size-cells describe the addressing of the bus.
> The bus still continues to exist even with no panel being attached.
While even empty "panel@[0-3]" is not required to be written as long as 
there is no panel being attached. Although the bus do have such kind of 
2 bits allocation.
> 
>>
>> I can understand that "#address-cells" "#size-cells" cannot be device
>> tree overlayed by dtbo. While when there is no "panel@[0-3]" nodes shall
>> we also remove "#address-cells" "#size-cells" properties for dsi node?
> 
> But why?
The reason is that "#address-cells" "#size-cells" are not marked as 
"required" properties in dsi-controller.yaml.
Also referenced the other bindings which "$ref:dsi-controller.yaml", 
some of them also not have those 2 properties in dsi node.

Take below 2 examples(there are more of cause):
[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/display/amlogic,meson-g12a-dw-mipi-dsi.yaml
[2] 
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml

> 
>>>
>>>> 2. Align current binding document with "panel@0" examples.
>>>
>>> There is already a panel in dsi-controller.yaml. Adding another example is optional. Do you also need an example with the external DSI bridge?
>> Current binding I am talking about is current patch binding file:
>> qcom,sm8550-mdss.yaml
> 
> Why do you call it a patch? Also if you have read the description, you
> would have known that the bindings describe the Mobile Display
> Subsystem (MDSS) itself. And then it explicitly tells that the MDSS on
> that platform has (among others) DSI blocks with proper compatibles.
> 
>> It have a ref to mdss-common.yaml,  but I cannot find the ref direct me
>> to dsi-controller.yaml.
> 
> Because qcom,sm8550-mdss.yaml doesn't describe the DSI controller. I
> think I have already mentioned a file that describes the DSI
> controller, it is called display/msm/dsi-controller-main.yaml. Not the
> best name, but it is quickly revealed by grepping for either of the
> DSI compatible strings. And the dsi-controller-main.yaml has `  -
> $ref: ../dsi-controller.yaml#` string inside.
Let me try to describe this in a code way.
"qcom,sm8550-mdss.yaml" binding document is confusing me when it also 
have same compatible string support in the binding. So I will suggest to 
directly reference the "dsi-controller-main.yaml" in file 
"qcom,sm8550-mdss.yaml" instead.

For example:

--- a/Documentation/devicetree/bindings/display/msm/qcom,sm8550-mdss.yaml
+++ b/Documentation/devicetree/bindings/display/msm/qcom,sm8550-mdss.yaml

@@ -55,14 +50,7 @@ patternProperties:
            - const: qcom,sm8350-dp

    "^dsi@[0-9a-f]+$":
-    type: object
-    additionalProperties: true
-
-    properties:
-      compatible:
-        items:
-          - const: qcom,sm8550-dsi-ctrl
-          - const: qcom,mdss-dsi-ctrl
+    $ref: ../dsi-controller-main.yaml#

With above unified reference change, it will be easier for other 
developers to reference bindings files next time.
Also dsi@[0-9a-f] node in mdss node will be correctly fully described.
> 
>> In my opinion if the example have "#address-cells" "#size-cells", then
>> it's better to also include "panel@0" with "reg = <0>" to not confuse.
> 
> It is already there, see dsi-controller.yaml.
> 
>>>> 3. Too many bindings files for driver "qcom,mdss-dsi-ctrl", shall we align them into 1 binding files.
>>>
>>> There is just one.
>> Currently I mentioned bindings files was searched the compatible
>> "qcom,mdss-dsi-ctrl", and find binding docs like "qcom,sm8550-mdss.yaml"
>> "qcom,sm8450-mdss.yaml" etc.
>> There is duplicate information on "qcom,sm8550-mdss.yaml" etc, while
>> "qcom,mdss-common.yaml" is not common enough for my understanding.
> 
> If you had compared the qcom,SOC-mdss.yaml, you would have seen that
> they provide tight binding between compatible strings used for all the
> subblocks. The `mdss-common.yaml` describes MDSS common properties. It
> describes everything except the platform specifics. It can not be made
> more common. And there is no duplication.
> 
> If you think you can improve the bindings, please send the patches.
I am thinking of a unified qcom,mdss.yaml instead of "qcom,*each 
SOC*-mdss.yaml". I will try to have a patch.
> They must pass the `make dt_binding_check` check.
Thx for the remind.
> 
>>>> 4. enhance the dtc warning check if we still want to have "#address-cells" "#size-cells" even if there is no "panel@0" attached.
>>>
>>> In my opinion this is a way to go, if any. Did you include devicetree@ ML and the corresponding maintainers into the discussion?
>> Already included devicetree@ ML at the very beginning.
> 
> Good, thanks for the confirmation.
> 
>> If the required properties part in each yaml is marked good enough, I
>> think it can be an input for avoid unnecessary dtc warnings.
> 
> Patches are welcome.
Improving developer efficiency with unnecessary warnings is one of my 
interest as well.
First of all, I'd better to make sure "Required properties" attribute in 
current bindings are good enough. Let me try to get back on this in a 
separate discuss session.
> 
>>>
>>>>
>>>> @krzy here I try to answer your comments here as well.
>>>> I am disagree on leave the warning as it is. And want to do something to improve this. Ideas above.
>>>> Let me know if any comments is not right addressed from your comments.
> 

-- 
Thx and BRs,
Aiqun(Maria) Yu

Re: [PATCH v3 1/1] arm64: dts: qcom: sm8550: remove address/size-cells from mdss_dsi1

[Krzysztof Kozlowski]

On 21/12/2023 09:49, Aiqun Yu (Maria) wrote:
> For example:
> 
> --- a/Documentation/devicetree/bindings/display/msm/qcom,sm8550-mdss.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/qcom,sm8550-mdss.yaml
> 
> @@ -55,14 +50,7 @@ patternProperties:
>             - const: qcom,sm8350-dp
> 
>     "^dsi@[0-9a-f]+$":
> -    type: object
> -    additionalProperties: true
> -
> -    properties:
> -      compatible:
> -        items:
> -          - const: qcom,sm8550-dsi-ctrl
> -          - const: qcom,mdss-dsi-ctrl
> +    $ref: ../dsi-controller-main.yaml#
> 
> With above unified reference change, it will be easier for other 
> developers to reference bindings files next time.
> Also dsi@[0-9a-f] node in mdss node will be correctly fully described.

No, this does not make sense and allows anything as dsi. It is opposite
of what we want in bindings, so NAK.

>>
>>> In my opinion if the example have "#address-cells" "#size-cells", then
>>> it's better to also include "panel@0" with "reg = <0>" to not confuse.
>>
>> It is already there, see dsi-controller.yaml.
>>
>>>>> 3. Too many bindings files for driver "qcom,mdss-dsi-ctrl", shall we align them into 1 binding files.
>>>>
>>>> There is just one.
>>> Currently I mentioned bindings files was searched the compatible
>>> "qcom,mdss-dsi-ctrl", and find binding docs like "qcom,sm8550-mdss.yaml"
>>> "qcom,sm8450-mdss.yaml" etc.
>>> There is duplicate information on "qcom,sm8550-mdss.yaml" etc, while
>>> "qcom,mdss-common.yaml" is not common enough for my understanding.
>>
>> If you had compared the qcom,SOC-mdss.yaml, you would have seen that
>> they provide tight binding between compatible strings used for all the
>> subblocks. The `mdss-common.yaml` describes MDSS common properties. It
>> describes everything except the platform specifics. It can not be made
>> more common. And there is no duplication.
>>
>> If you think you can improve the bindings, please send the patches.
> I am thinking of a unified qcom,mdss.yaml instead of "qcom,*each 
> SOC*-mdss.yaml". I will try to have a patch.

I asked first of you to read previous discussions. If you still insist
on sending patch for this, it means you did not read them.

How you wrote this idea, sounds like exactly opposite of what we were
doing and what I was recommending few times on the list, so it is very
likely I will NAK it.

>> They must pass the `make dt_binding_check` check.
> Thx for the remind.

And follow bindings guidelines.

>>
>>>>> 4. enhance the dtc warning check if we still want to have "#address-cells" "#size-cells" even if there is no "panel@0" attached.
>>>>
>>>> In my opinion this is a way to go, if any. Did you include devicetree@ ML and the corresponding maintainers into the discussion?
>>> Already included devicetree@ ML at the very beginning.
>>
>> Good, thanks for the confirmation.
>>
>>> If the required properties part in each yaml is marked good enough, I
>>> think it can be an input for avoid unnecessary dtc warnings.
>>
>> Patches are welcome.
> Improving developer efficiency with unnecessary warnings is one of my 
> interest as well.
> First of all, I'd better to make sure "Required properties" attribute in 
> current bindings are good enough. Let me try to get back on this in a 

I don't understand why do you keep mentioning the "required properties".
They have nothing to do with any of this here.



Best regards,
Krzysztof

Re: [PATCH v3 1/1] arm64: dts: qcom: sm8550: remove address/size-cells from mdss_dsi1

[Aiqun Yu (Maria)]

On 12/21/2023 4:57 PM, Krzysztof Kozlowski wrote:
> On 21/12/2023 09:49, Aiqun Yu (Maria) wrote:
>> For example:
>>
>> --- a/Documentation/devicetree/bindings/display/msm/qcom,sm8550-mdss.yaml
>> +++ b/Documentation/devicetree/bindings/display/msm/qcom,sm8550-mdss.yaml
>>
>> @@ -55,14 +50,7 @@ patternProperties:
>>              - const: qcom,sm8350-dp
>>
>>      "^dsi@[0-9a-f]+$":
>> -    type: object
>> -    additionalProperties: true
>> -
>> -    properties:
>> -      compatible:
>> -        items:
>> -          - const: qcom,sm8550-dsi-ctrl
>> -          - const: qcom,mdss-dsi-ctrl
>> +    $ref: ../dsi-controller-main.yaml#
>>
>> With above unified reference change, it will be easier for other
>> developers to reference bindings files next time.
>> Also dsi@[0-9a-f] node in mdss node will be correctly fully described.
> 
> No, this does not make sense and allows anything as dsi. It is opposite
> of what we want in bindings, so NAK.
> 
>>>
>>>> In my opinion if the example have "#address-cells" "#size-cells", then
>>>> it's better to also include "panel@0" with "reg = <0>" to not confuse.
>>>
>>> It is already there, see dsi-controller.yaml.
>>>
>>>>>> 3. Too many bindings files for driver "qcom,mdss-dsi-ctrl", shall we align them into 1 binding files.
>>>>>
>>>>> There is just one.
>>>> Currently I mentioned bindings files was searched the compatible
>>>> "qcom,mdss-dsi-ctrl", and find binding docs like "qcom,sm8550-mdss.yaml"
>>>> "qcom,sm8450-mdss.yaml" etc.
>>>> There is duplicate information on "qcom,sm8550-mdss.yaml" etc, while
>>>> "qcom,mdss-common.yaml" is not common enough for my understanding.
>>>
>>> If you had compared the qcom,SOC-mdss.yaml, you would have seen that
>>> they provide tight binding between compatible strings used for all the
>>> subblocks. The `mdss-common.yaml` describes MDSS common properties. It
>>> describes everything except the platform specifics. It can not be made
>>> more common. And there is no duplication.
>>>
>>> If you think you can improve the bindings, please send the patches.
>> I am thinking of a unified qcom,mdss.yaml instead of "qcom,*each
>> SOC*-mdss.yaml". I will try to have a patch.
> 
> I asked first of you to read previous discussions. If you still insist
> on sending patch for this, it means you did not read them.
Do you have the previous discussion title/link that you are refereed 
here pls?
> 
> How you wrote this idea, sounds like exactly opposite of what we were
> doing and what I was recommending few times on the list, so it is very
> likely I will NAK it.
> 
>>> They must pass the `make dt_binding_check` check.
>> Thx for the remind.
> 
> And follow bindings guidelines.
> 
>>>
>>>>>> 4. enhance the dtc warning check if we still want to have "#address-cells" "#size-cells" even if there is no "panel@0" attached.
>>>>>
>>>>> In my opinion this is a way to go, if any. Did you include devicetree@ ML and the corresponding maintainers into the discussion?
>>>> Already included devicetree@ ML at the very beginning.
>>>
>>> Good, thanks for the confirmation.
>>>
>>>> If the required properties part in each yaml is marked good enough, I
>>>> think it can be an input for avoid unnecessary dtc warnings.
>>>
>>> Patches are welcome.
>> Improving developer efficiency with unnecessary warnings is one of my
>> interest as well.
>> First of all, I'd better to make sure "Required properties" attribute in
>> current bindings are good enough. Let me try to get back on this in a
> 
> I don't understand why do you keep mentioning the "required properties".
> They have nothing to do with any of this here.
> 
> 
> 
> Best regards,
> Krzysztof
> 

-- 
Thx and BRs,
Aiqun(Maria) Yu

Re: [PATCH v3 1/1] arm64: dts: qcom: sm8550: remove address/size-cells from mdss_dsi1

[Krzysztof Kozlowski]

On 22/12/2023 10:10, Aiqun Yu (Maria) wrote:
>>>>>> There is just one.
>>>>> Currently I mentioned bindings files was searched the compatible
>>>>> "qcom,mdss-dsi-ctrl", and find binding docs like "qcom,sm8550-mdss.yaml"
>>>>> "qcom,sm8450-mdss.yaml" etc.
>>>>> There is duplicate information on "qcom,sm8550-mdss.yaml" etc, while
>>>>> "qcom,mdss-common.yaml" is not common enough for my understanding.
>>>>
>>>> If you had compared the qcom,SOC-mdss.yaml, you would have seen that
>>>> they provide tight binding between compatible strings used for all the
>>>> subblocks. The `mdss-common.yaml` describes MDSS common properties. It
>>>> describes everything except the platform specifics. It can not be made
>>>> more common. And there is no duplication.
>>>>
>>>> If you think you can improve the bindings, please send the patches.
>>> I am thinking of a unified qcom,mdss.yaml instead of "qcom,*each
>>> SOC*-mdss.yaml". I will try to have a patch.
>>
>> I asked first of you to read previous discussions. If you still insist
>> on sending patch for this, it means you did not read them.
> Do you have the previous discussion title/link that you are refereed 
> here pls?

No, I don't have it. You can find it the same as me and it is not the
job of reviewer to find them for you.

Best regards,
Krzysztof

Re: [PATCH v3 1/1] arm64: dts: qcom: sm8550: remove address/size-cells from mdss_dsi1

[Dmitry Baryshkov]

On Thu, 21 Dec 2023 at 10:49, Aiqun Yu (Maria) <quic_aiquny@quicinc.com> wrote:
>
>
>
> On 12/21/2023 2:59 PM, Dmitry Baryshkov wrote:
> > On Thu, 21 Dec 2023 at 03:57, Aiqun Yu (Maria) <quic_aiquny@quicinc.com> wrote:
> >>
> >>
> >>
> >> On 12/20/2023 7:10 PM, Dmitry Baryshkov wrote:
> >>> On 20 December 2023 12:33:07 EET, "Aiqun Yu (Maria)" <quic_aiquny@quicinc.com> wrote:
> >>>>
> >>>>
> >>>> On 12/20/2023 3:06 PM, Dmitry Baryshkov wrote:
> >>>>> On Wed, 20 Dec 2023 at 02:54, Aiqun Yu (Maria) <quic_aiquny@quicinc.com> wrote:
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> On 12/19/2023 6:21 PM, Dmitry Baryshkov wrote:
> >>>>>>> On Tue, 19 Dec 2023 at 12:09, Aiqun Yu (Maria) <quic_aiquny@quicinc.com> wrote:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On 12/19/2023 5:41 PM, Krzysztof Kozlowski wrote:
> >>>>>>>>> On 19/12/2023 10:36, Aiqun Yu (Maria) wrote:
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> On 12/19/2023 3:17 PM, Krzysztof Kozlowski wrote:
> >>>>>>>>>>> On 19/12/2023 01:31, Tengfei Fan wrote:
> >>>>>>>>>>>> The address/size-cells in mdss_dsi1 node have not ranges and child also
> >>>>>>>>>>>> have not reg, then this leads to dtc W=1 warnings:
> >>>>>>>>>>>
> >>>>>>>>>> Comments can be more readable:
> >>>>>>>>>> "mdss_dsi1" node don't have "ranges" or child "reg" property, while it
> >>>>>>>>>> have address/size-cells properties. This caused
> >>>>>>>>>> "avoid_unnecessary_addr_size" warning from dtb check.
> >>>>>>>>>> Remove address/size-cells properties for "mdss_dsi1" node.
> >>>>>>>>>>
> >>>>>>>>>>> I cannot parse it. Address/size cells never have ranges or children.
> >>>>>>>>>>> They cannot have. These are uint32 properties.
> >>>>>>>>>> Pls help to comment on the revised commit message. Every time I write a
> >>>>>>>>>> commit message, also takes a while for me to double confirm whether
> >>>>>>>>>> others can understand me correctly as well. Feel free to let us know if
> >>>>>>>>>> it is not readable to you. It will help us as non-English native developers.
> >>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>>         sm8550.dtsi:2937.27-2992.6: Warning (avoid_unnecessary_addr_size): /soc@0/display-subsystem@ae00000/dsi@ae96000:
> >>>>>>>>>>>>           unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >>>>>>>>>>>> Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com>
> >>>>>>>>>>>> ---
> >>>>>>>>>>>
> >>>>>>>>>>> I disagreed with the patch before. You resubmit it without really
> >>>>>>>>>>> addressing my concerns.
> >>>>>>>>>>>
> >>>>>>>>>>> I am not sure if this is correct fix and I want to fix all of such
> >>>>>>>>>>> errors (there are multiple of them) in the same way. Feel free to
> >>>>>>>>>>> propose common solution based on arguments.
> >>>>>>>>>> Per my understanding, "qcom,mdss-dsi-ctrl" driver node like "mdss_dsi1"
> >>>>>>>>>> don't need to have address/size-cells properties.
> >>>>>>>>>
> >>>>>>>>> Just because dtc says so? And what about bindings?
> >>>>>>>> I don't find any reason why "qcom,mdss-dsi-ctrl" driver node need to
> >>>>>>>> have address/size-cells properties. Document Bindings should also be fixed.
> >>>>>>>>>
> >>>>>>>>>> Feel free to let us know whether there is different idea of
> >>>>>>>>>> "address/size-cells" needed for the "qcom,mdss-dsi-ctrl" driver node.
> >>>>>>>>>
> >>>>>>>>> The bindings expressed that idea. If the binding is incorrect, fix the
> >>>>>>>>> binding and the DTS. If the binding is correct, provide rationale why it
> >>>>>>>>> somehow does not apply here etc.
> >>>>>>>> Our plan is to fix the bindings as well.
> >>>>>>>>
> >>>>>>>> In case you have missed the question, I just re-place it here:
> >>>>>>>> While there are about 22 different soc dtsi and it's document binding
> >>>>>>>> files needed to be fixed. Shall we fix it for all qcom related soc usage
> >>>>>>>> in one patch, or we'd better to split into different patches according
> >>>>>>>> to soc specifically?
> >>>>>>>
> >>>>>>> Don't touch the bindings unless you understand what you are doing.
> >>>>>>> Your patch will be NAKed. There can be a DSI panel attached to the DSI
> >>>>>>> host, which means there is a need for #address-cells / #size-cells.
> >>>>>>>
> >>>>>> Could you please help to elaborate more on details? Like what's the
> >>>>>> right example here for the "qcom,mdss-dsi-ctrl" driver node needed to
> >>>>>> have "#address-cells"/"#size-cells".
> >>>>>
> >>>>> As I wrote, the attached DSI panels make use of #address-cells / #size-cells.
> >>>>>
> >>>>> Please take a look at the sdm845-mtp.dts, which provides a complex
> >>>>> enough example (a panel which is attached to both DSI0 and DSI1
> >>>>> hosts).
> >>>> I can see the panel example now.
> >>>> While panel@0 likely node is not at in the binding that I've checked. There are quite a few of binding document about the same driver. I checked 5 of the bindings document and moste of them are alike, and don't have the panel example.:(
> >>>
> >>> There is a single bindings documents describing MSM DSI controller, display/MSM/dsi-controller-main.yaml . It explicitly includes ../dsi-controller.yaml, which describes generic DSI host controller. The latter one includes an example of the DSI panel. MSM DSI bindings do not have to include one, there is nothing platform specific there.
> >>>
> >>>
> >>>>>
> >>>>>> Thx to chime in that we have put a good amount of time here.
> >>>>>
> >>>>> Can't quite catch you here.
> >>>>>
> >>>>>>> Please stop wasting the time on dtc warning. The bindings (and the
> >>>>>>> file) are correct.
> >>>>>> I don't agree here.
> >>>>>> Either it is a wrong usage of "#address-cells"/"#size-cells", or dtc
> >>>>>> warning should be fixed with this usage take into account.
> >>>>>> "dtb check" will be a good guideline for developers to follow, I don't
> >>>>>> think it is wasting time here.
> >>>>>
> >>>>> It is a guideline, but not a rule. No warnings by default is more of
> >>>>> the rule. W=1 enables warnings that developers have to classify and
> >>>>> cope with.
> >>>>>
> >>>>> E.g. I don't think dtc correctly handles the case when there are both
> >>>>> with-address and no-address nodes (e.g. 'panel@0' and 'ports'). Note,
> >>>>> I might be mistaken there.
> >>>> Now I understand the issue, here is some thinking from my end, and welcome others to chime in with more ideas:
> >>>> 1. Only put "#address-cells" "#size-cells" right before node of panel@0
> >>>
> >>> No. Cells specification is a property of the host/bus. As such they do not belong to the board DT file.
> >> As "#address-cells" "#size-cells" is not marked as required properties
> >> in the Document dsi-controller.yaml. Did it really needed even
> >> "panel@[0-3]" is not present at current dsi node?
> >> That's good that comes to the initial discussion of current patch here. :)
> >
> > The #address-cells / #size-cells describe the addressing of the bus.
> > The bus still continues to exist even with no panel being attached.
> While even empty "panel@[0-3]" is not required to be written as long as
> there is no panel being attached. Although the bus do have such kind of
> 2 bits allocation.
> >
> >>
> >> I can understand that "#address-cells" "#size-cells" cannot be device
> >> tree overlayed by dtbo. While when there is no "panel@[0-3]" nodes shall
> >> we also remove "#address-cells" "#size-cells" properties for dsi node?
> >
> > But why?
> The reason is that "#address-cells" "#size-cells" are not marked as
> "required" properties in dsi-controller.yaml.
> Also referenced the other bindings which "$ref:dsi-controller.yaml",
> some of them also not have those 2 properties in dsi node.

And others (like ste,mcde.yaml or brcm,bcm2835-dsi0.yaml) explicitly
have #cells in the example.

>
> Take below 2 examples(there are more of cause):
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/display/amlogic,meson-g12a-dw-mipi-dsi.yaml
> [2]
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml
>
> >
> >>>
> >>>> 2. Align current binding document with "panel@0" examples.
> >>>
> >>> There is already a panel in dsi-controller.yaml. Adding another example is optional. Do you also need an example with the external DSI bridge?
> >> Current binding I am talking about is current patch binding file:
> >> qcom,sm8550-mdss.yaml
> >
> > Why do you call it a patch? Also if you have read the description, you
> > would have known that the bindings describe the Mobile Display
> > Subsystem (MDSS) itself. And then it explicitly tells that the MDSS on
> > that platform has (among others) DSI blocks with proper compatibles.
> >
> >> It have a ref to mdss-common.yaml,  but I cannot find the ref direct me
> >> to dsi-controller.yaml.
> >
> > Because qcom,sm8550-mdss.yaml doesn't describe the DSI controller. I
> > think I have already mentioned a file that describes the DSI
> > controller, it is called display/msm/dsi-controller-main.yaml. Not the
> > best name, but it is quickly revealed by grepping for either of the
> > DSI compatible strings. And the dsi-controller-main.yaml has `  -
> > $ref: ../dsi-controller.yaml#` string inside.
> Let me try to describe this in a code way.
> "qcom,sm8550-mdss.yaml" binding document is confusing me when it also
> have same compatible string support in the binding.

Yes, it has. But there is a huge difference. The bindings file
describes and qcom,sm8550-mdss device and then enforces that DSI
controllers should be compatible with "qcom,sm8550-dsi-ctrl",
"qcom,mdss-dsi-ctrl". In the same way it also binds SoC-specific
compatible strings to all other subnodes.

> So I will suggest to
> directly reference the "dsi-controller-main.yaml" in file
> "qcom,sm8550-mdss.yaml" instead.
>
> For example:
>
> --- a/Documentation/devicetree/bindings/display/msm/qcom,sm8550-mdss.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/qcom,sm8550-mdss.yaml
>
> @@ -55,14 +50,7 @@ patternProperties:
>             - const: qcom,sm8350-dp
>
>     "^dsi@[0-9a-f]+$":
> -    type: object
> -    additionalProperties: true
> -
> -    properties:
> -      compatible:
> -        items:
> -          - const: qcom,sm8550-dsi-ctrl
> -          - const: qcom,mdss-dsi-ctrl
> +    $ref: ../dsi-controller-main.yaml#

Note, there is no ../dsi-controller-main.yaml, so you didn't check
your patch (despite my request to do so).
I can only hope that you meant `$ref: dsi-controller-main.yaml`
instead of `$ref: ../dsi-controller.yaml`.

>
> With above unified reference change, it will be easier for other
> developers to reference bindings files next time.
> Also dsi@[0-9a-f] node in mdss node will be correctly fully described.

If you cared to read mailing list discussions related to the
corresponding patches, you would have noticed that this approach was
considered and then abandoned.
Two main reasons:
- This causes dsi-controller-main.yaml schema to be evaluated twice
for each of the DSI controller nodes.
- This doesn't have the SoC binding. Thus with such schema it is
possible to have top-level qcom,sm8550-mdss and then
qcom,sdm845-dsi-ctrl beneath.

> >
> >> In my opinion if the example have "#address-cells" "#size-cells", then
> >> it's better to also include "panel@0" with "reg = <0>" to not confuse.
> >
> > It is already there, see dsi-controller.yaml.
> >
> >>>> 3. Too many bindings files for driver "qcom,mdss-dsi-ctrl", shall we align them into 1 binding files.
> >>>
> >>> There is just one.
> >> Currently I mentioned bindings files was searched the compatible
> >> "qcom,mdss-dsi-ctrl", and find binding docs like "qcom,sm8550-mdss.yaml"
> >> "qcom,sm8450-mdss.yaml" etc.
> >> There is duplicate information on "qcom,sm8550-mdss.yaml" etc, while
> >> "qcom,mdss-common.yaml" is not common enough for my understanding.
> >
> > If you had compared the qcom,SOC-mdss.yaml, you would have seen that
> > they provide tight binding between compatible strings used for all the
> > subblocks. The `mdss-common.yaml` describes MDSS common properties. It
> > describes everything except the platform specifics. It can not be made
> > more common. And there is no duplication.
> >
> > If you think you can improve the bindings, please send the patches.
> I am thinking of a unified qcom,mdss.yaml instead of "qcom,*each
> SOC*-mdss.yaml". I will try to have a patch.

Please. Read the mailing list archives first.

> > They must pass the `make dt_binding_check` check.
> Thx for the remind.
> >
> >>>> 4. enhance the dtc warning check if we still want to have "#address-cells" "#size-cells" even if there is no "panel@0" attached.
> >>>
> >>> In my opinion this is a way to go, if any. Did you include devicetree@ ML and the corresponding maintainers into the discussion?
> >> Already included devicetree@ ML at the very beginning.
> >
> > Good, thanks for the confirmation.
> >
> >> If the required properties part in each yaml is marked good enough, I
> >> think it can be an input for avoid unnecessary dtc warnings.
> >
> > Patches are welcome.
> Improving developer efficiency with unnecessary warnings is one of my
> interest as well.
> First of all, I'd better to make sure "Required properties" attribute in
> current bindings are good enough. Let me try to get back on this in a
> separate discuss session.

Let me put some kind of a point here. Show us your code. Otherwise
it's just an endless discussion.

-- 
With best wishes
Dmitry

Re: [PATCH v3 1/1] arm64: dts: qcom: sm8550: remove address/size-cells from mdss_dsi1

[Krzysztof Kozlowski]

On 20/12/2023 11:33, Aiqun Yu (Maria) wrote:
>>>>
>>>> Don't touch the bindings unless you understand what you are doing.
>>>> Your patch will be NAKed. There can be a DSI panel attached to the DSI
>>>> host, which means there is a need for #address-cells / #size-cells.
>>>>
>>> Could you please help to elaborate more on details? Like what's the
>>> right example here for the "qcom,mdss-dsi-ctrl" driver node needed to
>>> have "#address-cells"/"#size-cells".
>>
>> As I wrote, the attached DSI panels make use of #address-cells / #size-cells.
>>
>> Please take a look at the sdm845-mtp.dts, which provides a complex
>> enough example (a panel which is attached to both DSI0 and DSI1
>> hosts).
> I can see the panel example now.
> While panel@0 likely node is not at in the binding that I've checked. 

"Likely not" is not the same as "cannot".

> There are quite a few of binding document about the same driver. I 

You keep mixing drivers, bindings and devices which does not help this
discussion. I really do not understand above sentence.

> checked 5 of the bindings document and moste of them are alike, and 
> don't have the panel example.:(

Example like the example DTS in the binding? What does it have to do
with anything here? What are we talking here about?

>>
>>> Thx to chime in that we have put a good amount of time here.
>>
>> Can't quite catch you here.
>>
>>>> Please stop wasting the time on dtc warning. The bindings (and the
>>>> file) are correct.
>>> I don't agree here.
>>> Either it is a wrong usage of "#address-cells"/"#size-cells", or dtc
>>> warning should be fixed with this usage take into account.
>>> "dtb check" will be a good guideline for developers to follow, I don't
>>> think it is wasting time here.
>>
>> It is a guideline, but not a rule. No warnings by default is more of
>> the rule. W=1 enables warnings that developers have to classify and
>> cope with.
>>
>> E.g. I don't think dtc correctly handles the case when there are both
>> with-address and no-address nodes (e.g. 'panel@0' and 'ports'). Note,
>> I might be mistaken there.
> Now I understand the issue, here is some thinking from my end, and 
> welcome others to chime in with more ideas:
> 1. Only put "#address-cells" "#size-cells" right before node of panel@0.
> 2. Align current binding document with "panel@0" examples.

Examples? Again, about what examples are you talking? Mixing terminology
does not help this discussion, so let's be specific:
Touching examples just because you want, without valid reason: no

> 3. Too many bindings files for driver "qcom,mdss-dsi-ctrl", shall we 
> align them into 1 binding files.

You are joking right? First of all, there is no driver
"qcom,mdss-dsi-ctrl". Don't mix the terms, because it does not help the
discussion. Second, please read previous discussions related to these
bindings.

> 4. enhance the dtc warning check if we still want to have 
> "#address-cells" "#size-cells" even if there is no "panel@0" attached.
> 
> @krzy here I try to answer your comments here as well.
> I am disagree on leave the warning as it is. And want to do something to 
> improve this. Ideas above.
> Let me know if any comments is not right addressed from your comments.

You want to do something without understanding the problem which results
in random band-aids over some warning. Instead, please dig deeper to
understand the problem and then propose solution.

Best regards,
Krzysztof

Re: [PATCH v3 1/1] arm64: dts: qcom: sm8550: remove address/size-cells from mdss_dsi1

[Krzysztof Kozlowski]

On 20/12/2023 01:53, Aiqun Yu (Maria) wrote:
> 
> 
> On 12/19/2023 6:21 PM, Dmitry Baryshkov wrote:
>> On Tue, 19 Dec 2023 at 12:09, Aiqun Yu (Maria) <quic_aiquny@quicinc.com> wrote:
>>>
>>>
>>>
>>> On 12/19/2023 5:41 PM, Krzysztof Kozlowski wrote:
>>>> On 19/12/2023 10:36, Aiqun Yu (Maria) wrote:
>>>>>
>>>>>
>>>>> On 12/19/2023 3:17 PM, Krzysztof Kozlowski wrote:
>>>>>> On 19/12/2023 01:31, Tengfei Fan wrote:
>>>>>>> The address/size-cells in mdss_dsi1 node have not ranges and child also
>>>>>>> have not reg, then this leads to dtc W=1 warnings:
>>>>>>
>>>>> Comments can be more readable:
>>>>> "mdss_dsi1" node don't have "ranges" or child "reg" property, while it
>>>>> have address/size-cells properties. This caused
>>>>> "avoid_unnecessary_addr_size" warning from dtb check.
>>>>> Remove address/size-cells properties for "mdss_dsi1" node.
>>>>>
>>>>>> I cannot parse it. Address/size cells never have ranges or children.
>>>>>> They cannot have. These are uint32 properties.
>>>>> Pls help to comment on the revised commit message. Every time I write a
>>>>> commit message, also takes a while for me to double confirm whether
>>>>> others can understand me correctly as well. Feel free to let us know if
>>>>> it is not readable to you. It will help us as non-English native developers.
>>>>>>
>>>>>>>
>>>>>>>      sm8550.dtsi:2937.27-2992.6: Warning (avoid_unnecessary_addr_size): /soc@0/display-subsystem@ae00000/dsi@ae96000:
>>>>>>>        unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
>>>>>>>
>>>>>>>
>>>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>>>>> Signed-off-by: Tengfei Fan <quic_tengfan@quicinc.com>
>>>>>>> ---
>>>>>>
>>>>>> I disagreed with the patch before. You resubmit it without really
>>>>>> addressing my concerns.
>>>>>>
>>>>>> I am not sure if this is correct fix and I want to fix all of such
>>>>>> errors (there are multiple of them) in the same way. Feel free to
>>>>>> propose common solution based on arguments.
>>>>> Per my understanding, "qcom,mdss-dsi-ctrl" driver node like "mdss_dsi1"
>>>>> don't need to have address/size-cells properties.
>>>>
>>>> Just because dtc says so? And what about bindings?
>>> I don't find any reason why "qcom,mdss-dsi-ctrl" driver node need to
>>> have address/size-cells properties. Document Bindings should also be fixed.
>>>>
>>>>> Feel free to let us know whether there is different idea of
>>>>> "address/size-cells" needed for the "qcom,mdss-dsi-ctrl" driver node.
>>>>
>>>> The bindings expressed that idea. If the binding is incorrect, fix the
>>>> binding and the DTS. If the binding is correct, provide rationale why it
>>>> somehow does not apply here etc.
>>> Our plan is to fix the bindings as well.
>>>
>>> In case you have missed the question, I just re-place it here:
>>> While there are about 22 different soc dtsi and it's document binding
>>> files needed to be fixed. Shall we fix it for all qcom related soc usage
>>> in one patch, or we'd better to split into different patches according
>>> to soc specifically?
>>
>> Don't touch the bindings unless you understand what you are doing.
>> Your patch will be NAKed. There can be a DSI panel attached to the DSI
>> host, which means there is a need for #address-cells / #size-cells.
>>
> Could you please help to elaborate more on details? Like what's the 
> right example here for the "qcom,mdss-dsi-ctrl" driver node needed to 
> have "#address-cells"/"#size-cells".

Isn't the binding describing such example?

> 
> Thx to chime in that we have put a good amount of time here.
>> Please stop wasting the time on dtc warning. The bindings (and the
>> file) are correct.
> I don't agree here.
> Either it is a wrong usage of "#address-cells"/"#size-cells", or dtc 
> warning should be fixed with this usage take into account.
> "dtb check" will be a good guideline for developers to follow, I don't 
> think it is wasting time here.

You don't agree but you don't know how this binding works?

Best regards,
Krzysztof

Re: (subset) [PATCH v3 0/1] arm64: dts: qcom: sm8550: remove

[Bjorn Andersson]

On Tue, 19 Dec 2023 08:31:05 +0800, Tengfei Fan wrote:
> The address/size-cells in mdss_dsi1 node have not ranges and child also
> have not reg, then this leads to dtc W=1 warnings:
> 
>   sm8550.dtsi:2937.27-2992.6: Warning (avoid_unnecessary_addr_size): /soc@0/display-subsystem@ae00000/dsi@ae96000:
>     unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
> 
> 
> [...]

Applied, thanks!

[1/1] arm64: dts: qcom: sm8550: remove address/size-cells from mdss_dsi1
      commit: 53081095936cdb1501d6bcf6cb703fdd3ac71b85

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>

Re: (subset) [PATCH v3 0/1] arm64: dts: qcom: sm8550: remove

[Aiqun Yu (Maria)]

On 12/20/2023 3:33 AM, Bjorn Andersson wrote:
> 
> On Tue, 19 Dec 2023 08:31:05 +0800, Tengfei Fan wrote:
>> The address/size-cells in mdss_dsi1 node have not ranges and child also
>> have not reg, then this leads to dtc W=1 warnings:
>>
>>    sm8550.dtsi:2937.27-2992.6: Warning (avoid_unnecessary_addr_size): /soc@0/display-subsystem@ae00000/dsi@ae96000:
>>      unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
>>
>>
>> [...]
> 
> Applied, thanks!
Hi Bjorn,

Discussion is still on going.
Dmitry dropped the previous review-by.
> 
> [1/1] arm64: dts: qcom: sm8550: remove address/size-cells from mdss_dsi1
>        commit: 53081095936cdb1501d6bcf6cb703fdd3ac71b85
> 
> Best regards,

-- 
Thx and BRs,
Aiqun(Maria) Yu

Re: (subset) [PATCH v3 0/1] arm64: dts: qcom: sm8550: remove

[Krzysztof Kozlowski]

On 19/12/2023 20:33, Bjorn Andersson wrote:
> 
> On Tue, 19 Dec 2023 08:31:05 +0800, Tengfei Fan wrote:
>> The address/size-cells in mdss_dsi1 node have not ranges and child also
>> have not reg, then this leads to dtc W=1 warnings:
>>
>>   sm8550.dtsi:2937.27-2992.6: Warning (avoid_unnecessary_addr_size): /soc@0/display-subsystem@ae00000/dsi@ae96000:
>>     unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
>>
>>
>> [...]
> 
> Applied, thanks!
> 
> [1/1] arm64: dts: qcom: sm8550: remove address/size-cells from mdss_dsi1
>       commit: 53081095936cdb1501d6bcf6cb703fdd3ac71b85

?!? I explicitly disagreed with this patch here and in the previous thread!

Best regards,
Krzysztof

Re: (subset) [PATCH v3 0/1] arm64: dts: qcom: sm8550: remove

[Dmitry Baryshkov]

On Tue, 19 Dec 2023 at 21:36, Bjorn Andersson <andersson@kernel.org> wrote:
>
>
> On Tue, 19 Dec 2023 08:31:05 +0800, Tengfei Fan wrote:
> > The address/size-cells in mdss_dsi1 node have not ranges and child also
> > have not reg, then this leads to dtc W=1 warnings:
> >
> >   sm8550.dtsi:2937.27-2992.6: Warning (avoid_unnecessary_addr_size): /soc@0/display-subsystem@ae00000/dsi@ae96000:
> >     unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
> >
> >
> > [...]
>
> Applied, thanks!
>
> [1/1] arm64: dts: qcom: sm8550: remove address/size-cells from mdss_dsi1
>       commit: 53081095936cdb1501d6bcf6cb703fdd3ac71b85

Bjorn, but there was a NAK If I remember correctly.


--
With best wishes
Dmitry

Re: (subset) [PATCH v3 0/1] arm64: dts: qcom: sm8550: remove

[Bjorn Andersson]

On Tue, Dec 19, 2023 at 01:33:37PM -0600, Bjorn Andersson wrote:
> 
> On Tue, 19 Dec 2023 08:31:05 +0800, Tengfei Fan wrote:
> > The address/size-cells in mdss_dsi1 node have not ranges and child also
> > have not reg, then this leads to dtc W=1 warnings:
> > 
> >   sm8550.dtsi:2937.27-2992.6: Warning (avoid_unnecessary_addr_size): /soc@0/display-subsystem@ae00000/dsi@ae96000:
> >     unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
> > 
> > 
> > [...]
> 
> Applied, thanks!
> 
> [1/1] arm64: dts: qcom: sm8550: remove address/size-cells from mdss_dsi1
>       commit: 53081095936cdb1501d6bcf6cb703fdd3ac71b85
> 

Sorry, I thought this was removed from my queue again. I've dropped the
patch again. Please address Krzysztof's concerns.

Regards,
Bjorn

> Best regards,
> -- 
> Bjorn Andersson <andersson@kernel.org>
>