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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 12/20/2023 3:06 PM, Dmitry Baryshkov wrote:
On Wed, 20 Dec 2023 at 02:54, Aiqun Yu (Maria) <quic_aiquny@xxxxxxxxxxx> wrote:



On 12/19/2023 6:21 PM, Dmitry Baryshkov wrote:
On Tue, 19 Dec 2023 at 12:09, Aiqun Yu (Maria) <quic_aiquny@xxxxxxxxxxx> 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@xxxxxxxxxx>
Signed-off-by: Tengfei Fan <quic_tengfan@xxxxxxxxxxx>
---

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




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux