On 28/03/2022 19:16, Vinod Koul wrote:
> On 28-03-22, 19:43, Dmitry Baryshkov wrote:
>> On Mon, 28 Mar 2022 at 18:30, Krzysztof Kozlowski
>> <[email protected]> wrote:
>>>
>>> The DSI node is not a bus and the children do not have unit addresses.
>>>
>>> Reported-by: Vinod Koul <[email protected]>
>>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
>>
>> NAK.
>> DSI panels are children of the DSI device tree node with the reg = <0>; address.
>> This is the convention used by other platforms too (see e.g.
>> arch/arm64/boot/dts/freescale/imx8mq-evk.dts).
>
> So we should add reg = 0, i will update my dtsi fix
>
To "ports" node? No. The reg=0 is for children of the bus, so the
panels. How to combine both without warnings - ports and panel@0 - I
don't know yet...
Best regards,
Krzysztof
On Mon, Mar 28, 2022 at 12:18 PM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 28/03/2022 19:16, Vinod Koul wrote:
> > On 28-03-22, 19:43, Dmitry Baryshkov wrote:
> >> On Mon, 28 Mar 2022 at 18:30, Krzysztof Kozlowski
> >> <[email protected]> wrote:
> >>>
> >>> The DSI node is not a bus and the children do not have unit addresses.
> >>>
> >>> Reported-by: Vinod Koul <[email protected]>
> >>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> >>
> >> NAK.
> >> DSI panels are children of the DSI device tree node with the reg = <0>; address.
> >> This is the convention used by other platforms too (see e.g.
> >> arch/arm64/boot/dts/freescale/imx8mq-evk.dts).
> >
> > So we should add reg = 0, i will update my dtsi fix
> >
>
> To "ports" node? No. The reg=0 is for children of the bus, so the
> panels. How to combine both without warnings - ports and panel@0 - I
> don't know yet...
I don't think that should case a warning. Or at least it's one we turn off.
Rob
On 28-03-22, 13:21, Rob Herring wrote:
> On Mon, Mar 28, 2022 at 12:18 PM Krzysztof Kozlowski
> <[email protected]> wrote:
> >
> > On 28/03/2022 19:16, Vinod Koul wrote:
> > > On 28-03-22, 19:43, Dmitry Baryshkov wrote:
> > >> On Mon, 28 Mar 2022 at 18:30, Krzysztof Kozlowski
> > >> <[email protected]> wrote:
> > >>>
> > >>> The DSI node is not a bus and the children do not have unit addresses.
> > >>>
> > >>> Reported-by: Vinod Koul <[email protected]>
> > >>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> > >>
> > >> NAK.
> > >> DSI panels are children of the DSI device tree node with the reg = <0>; address.
> > >> This is the convention used by other platforms too (see e.g.
> > >> arch/arm64/boot/dts/freescale/imx8mq-evk.dts).
> > >
> > > So we should add reg = 0, i will update my dtsi fix
> > >
> >
> > To "ports" node? No. The reg=0 is for children of the bus, so the
> > panels. How to combine both without warnings - ports and panel@0 - I
> > don't know yet...
>
> I don't think that should case a warning. Or at least it's one we turn off.
Well in this case I think we might need a fix:
Here is the example quoted in the binding. We have ports{} and then the
two port@0 and port@1 underneath.
So it should be okay to drop #address-cells/#size-cells from dsi node
but keep in ports node...
Thoughts...?
dsi@ae94000 {
compatible = "qcom,mdss-dsi-ctrl";
reg = <0x0ae94000 0x400>;
reg-names = "dsi_ctrl";
#address-cells = <1>;
#size-cells = <0>;
interrupt-parent = <&mdss>;
interrupts = <4>;
clocks = <&dispcc DISP_CC_MDSS_BYTE0_CLK>,
<&dispcc DISP_CC_MDSS_BYTE0_INTF_CLK>,
<&dispcc DISP_CC_MDSS_PCLK0_CLK>,
<&dispcc DISP_CC_MDSS_ESC0_CLK>,
<&dispcc DISP_CC_MDSS_AHB_CLK>,
<&dispcc DISP_CC_MDSS_AXI_CLK>;
clock-names = "byte",
"byte_intf",
"pixel",
"core",
"iface",
"bus";
phys = <&dsi0_phy>;
phy-names = "dsi";
assigned-clocks = <&dispcc DISP_CC_MDSS_BYTE0_CLK_SRC>, <&dispcc DISP_CC_MDSS_PCLK0_CLK_SRC>;
assigned-clock-parents = <&dsi_phy 0>, <&dsi_phy 1>;
power-domains = <&rpmhpd SC7180_CX>;
operating-points-v2 = <&dsi_opp_table>;
ports {
#address-cells = <1>;
#size-cells = <0>;
port@0 {
reg = <0>;
dsi0_in: endpoint {
remote-endpoint = <&dpu_intf1_out>;
};
};
port@1 {
reg = <1>;
dsi0_out: endpoint {
remote-endpoint = <&sn65dsi86_in>;
data-lanes = <0 1 2 3>;
};
};
};
};
>
> Rob
--
~Vinod
On Tue, Mar 29, 2022 at 12:01:52PM +0530, Vinod Koul wrote:
> On 28-03-22, 13:21, Rob Herring wrote:
> > On Mon, Mar 28, 2022 at 12:18 PM Krzysztof Kozlowski
> > <[email protected]> wrote:
> > >
> > > On 28/03/2022 19:16, Vinod Koul wrote:
> > > > On 28-03-22, 19:43, Dmitry Baryshkov wrote:
> > > >> On Mon, 28 Mar 2022 at 18:30, Krzysztof Kozlowski
> > > >> <[email protected]> wrote:
> > > >>>
> > > >>> The DSI node is not a bus and the children do not have unit addresses.
> > > >>>
> > > >>> Reported-by: Vinod Koul <[email protected]>
> > > >>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> > > >>
> > > >> NAK.
> > > >> DSI panels are children of the DSI device tree node with the reg = <0>; address.
> > > >> This is the convention used by other platforms too (see e.g.
> > > >> arch/arm64/boot/dts/freescale/imx8mq-evk.dts).
> > > >
> > > > So we should add reg = 0, i will update my dtsi fix
> > > >
> > >
> > > To "ports" node? No. The reg=0 is for children of the bus, so the
> > > panels. How to combine both without warnings - ports and panel@0 - I
> > > don't know yet...
> >
> > I don't think that should case a warning. Or at least it's one we turn off.
>
> Well in this case I think we might need a fix:
> Here is the example quoted in the binding. We have ports{} and then the
> two port@0 and port@1 underneath.
It's the #address-cells/#size-cells under 'ports' that applies to 'port'
nodes. As 'ports' has no address (reg) itself, it doesn't need
#address-cells/#size-cells in its parent node.
>
> So it should be okay to drop #address-cells/#size-cells from dsi node
> but keep in ports node...
Yes.
> Thoughts...?
But I thought a panel@0 node was being added? If so then you need to add
them back.
Rob
On 29-03-22, 10:52, Rob Herring wrote:
> On Tue, Mar 29, 2022 at 12:01:52PM +0530, Vinod Koul wrote:
> > On 28-03-22, 13:21, Rob Herring wrote:
> > > On Mon, Mar 28, 2022 at 12:18 PM Krzysztof Kozlowski
> > > <[email protected]> wrote:
> > > >
> > > > On 28/03/2022 19:16, Vinod Koul wrote:
> > > > > On 28-03-22, 19:43, Dmitry Baryshkov wrote:
> > > > >> On Mon, 28 Mar 2022 at 18:30, Krzysztof Kozlowski
> > > > >> <[email protected]> wrote:
> > > > >>>
> > > > >>> The DSI node is not a bus and the children do not have unit addresses.
> > > > >>>
> > > > >>> Reported-by: Vinod Koul <[email protected]>
> > > > >>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> > > > >>
> > > > >> NAK.
> > > > >> DSI panels are children of the DSI device tree node with the reg = <0>; address.
> > > > >> This is the convention used by other platforms too (see e.g.
> > > > >> arch/arm64/boot/dts/freescale/imx8mq-evk.dts).
> > > > >
> > > > > So we should add reg = 0, i will update my dtsi fix
> > > > >
> > > >
> > > > To "ports" node? No. The reg=0 is for children of the bus, so the
> > > > panels. How to combine both without warnings - ports and panel@0 - I
> > > > don't know yet...
> > >
> > > I don't think that should case a warning. Or at least it's one we turn off.
> >
> > Well in this case I think we might need a fix:
> > Here is the example quoted in the binding. We have ports{} and then the
> > two port@0 and port@1 underneath.
>
> It's the #address-cells/#size-cells under 'ports' that applies to 'port'
> nodes. As 'ports' has no address (reg) itself, it doesn't need
> #address-cells/#size-cells in its parent node.
>
> >
> > So it should be okay to drop #address-cells/#size-cells from dsi node
> > but keep in ports node...
>
> Yes.
>
> > Thoughts...?
>
> But I thought a panel@0 node was being added? If so then you need to add
> them back.
I guess we should make this optional, keep it when adding panel@0 node
and skip for rest where not applicable..? Dmitry is that fine with you?
--
~Vinod
On Thu, 31 Mar 2022 at 09:05, Vinod Koul <[email protected]> wrote:
>
> On 29-03-22, 10:52, Rob Herring wrote:
> > On Tue, Mar 29, 2022 at 12:01:52PM +0530, Vinod Koul wrote:
> > > On 28-03-22, 13:21, Rob Herring wrote:
> > > > On Mon, Mar 28, 2022 at 12:18 PM Krzysztof Kozlowski
> > > > <[email protected]> wrote:
> > > > >
> > > > > On 28/03/2022 19:16, Vinod Koul wrote:
> > > > > > On 28-03-22, 19:43, Dmitry Baryshkov wrote:
> > > > > >> On Mon, 28 Mar 2022 at 18:30, Krzysztof Kozlowski
> > > > > >> <[email protected]> wrote:
> > > > > >>>
> > > > > >>> The DSI node is not a bus and the children do not have unit addresses.
> > > > > >>>
> > > > > >>> Reported-by: Vinod Koul <[email protected]>
> > > > > >>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> > > > > >>
> > > > > >> NAK.
> > > > > >> DSI panels are children of the DSI device tree node with the reg = <0>; address.
> > > > > >> This is the convention used by other platforms too (see e.g.
> > > > > >> arch/arm64/boot/dts/freescale/imx8mq-evk.dts).
> > > > > >
> > > > > > So we should add reg = 0, i will update my dtsi fix
> > > > > >
> > > > >
> > > > > To "ports" node? No. The reg=0 is for children of the bus, so the
> > > > > panels. How to combine both without warnings - ports and panel@0 - I
> > > > > don't know yet...
> > > >
> > > > I don't think that should case a warning. Or at least it's one we turn off.
> > >
> > > Well in this case I think we might need a fix:
> > > Here is the example quoted in the binding. We have ports{} and then the
> > > two port@0 and port@1 underneath.
> >
> > It's the #address-cells/#size-cells under 'ports' that applies to 'port'
> > nodes. As 'ports' has no address (reg) itself, it doesn't need
> > #address-cells/#size-cells in its parent node.
> >
> > >
> > > So it should be okay to drop #address-cells/#size-cells from dsi node
> > > but keep in ports node...
> >
> > Yes.
> >
> > > Thoughts...?
> >
> > But I thought a panel@0 node was being added? If so then you need to add
> > them back.
>
> I guess we should make this optional, keep it when adding panel@0 node
> and skip for rest where not applicable..? Dmitry is that fine with you?
This sounds like a workaround. When a panel node is added together
with the '#address-cells' / '#size-cells' properties, we will get
warnings for the 'ports' node.
I'd prefer to leave things to pinpoint that the problem is generic
rather than being specific to several device trees with the DSI panel
nodes.
How do other platforms solve the issue?
In fact we can try shifting to the following dts schema:
dsi@ae940000 {
compatible = "qcom,mdss-dsi-ctrl";
ports {
#adress-cells = <1>;
#size-cells = <0>;
port@0 {
reg = <0>;
dsi0_in: endpoint {};
};
port@1 {
reg = <1>;
dsi0_out: endpoint {
remote-endpoint = <&panel_in>;
};
};
/* dsi-bus is a generic part */
dsi-bus {
#adress-cells = <1>;
#size-cells = <0>;
/* panel@0 goes to the board file */
panel@0 {
compatible = "vendor,some-panel";
ports {
#adress-cells = <1>;
#size-cells = <0>;
port@0 {
reg = <0>;
panel_in: endpoint {
remote-endpoint = <&dsi0_out>;
};
};
};
};
};
WDYT?
--
With best wishes
Dmitry
On Thu, Mar 31, 2022 at 4:35 AM Dmitry Baryshkov
<[email protected]> wrote:
>
> On Thu, 31 Mar 2022 at 09:05, Vinod Koul <[email protected]> wrote:
> >
> > On 29-03-22, 10:52, Rob Herring wrote:
> > > On Tue, Mar 29, 2022 at 12:01:52PM +0530, Vinod Koul wrote:
> > > > On 28-03-22, 13:21, Rob Herring wrote:
> > > > > On Mon, Mar 28, 2022 at 12:18 PM Krzysztof Kozlowski
> > > > > <[email protected]> wrote:
> > > > > >
> > > > > > On 28/03/2022 19:16, Vinod Koul wrote:
> > > > > > > On 28-03-22, 19:43, Dmitry Baryshkov wrote:
> > > > > > >> On Mon, 28 Mar 2022 at 18:30, Krzysztof Kozlowski
> > > > > > >> <[email protected]> wrote:
> > > > > > >>>
> > > > > > >>> The DSI node is not a bus and the children do not have unit addresses.
> > > > > > >>>
> > > > > > >>> Reported-by: Vinod Koul <[email protected]>
> > > > > > >>> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> > > > > > >>
> > > > > > >> NAK.
> > > > > > >> DSI panels are children of the DSI device tree node with the reg = <0>; address.
> > > > > > >> This is the convention used by other platforms too (see e.g.
> > > > > > >> arch/arm64/boot/dts/freescale/imx8mq-evk.dts).
> > > > > > >
> > > > > > > So we should add reg = 0, i will update my dtsi fix
> > > > > > >
> > > > > >
> > > > > > To "ports" node? No. The reg=0 is for children of the bus, so the
> > > > > > panels. How to combine both without warnings - ports and panel@0 - I
> > > > > > don't know yet...
> > > > >
> > > > > I don't think that should case a warning. Or at least it's one we turn off.
> > > >
> > > > Well in this case I think we might need a fix:
> > > > Here is the example quoted in the binding. We have ports{} and then the
> > > > two port@0 and port@1 underneath.
> > >
> > > It's the #address-cells/#size-cells under 'ports' that applies to 'port'
> > > nodes. As 'ports' has no address (reg) itself, it doesn't need
> > > #address-cells/#size-cells in its parent node.
> > >
> > > >
> > > > So it should be okay to drop #address-cells/#size-cells from dsi node
> > > > but keep in ports node...
> > >
> > > Yes.
> > >
> > > > Thoughts...?
> > >
> > > But I thought a panel@0 node was being added? If so then you need to add
> > > them back.
> >
> > I guess we should make this optional, keep it when adding panel@0 node
> > and skip for rest where not applicable..? Dmitry is that fine with you?
>
> This sounds like a workaround. When a panel node is added together
> with the '#address-cells' / '#size-cells' properties, we will get
> warnings for the 'ports' node.
What warning exactly? Is that with W=1?
Some warnings are more "don't do this on new designs" rather than
never allowed and need to fix current bindings/dts. As such, these
warnings will probably never be enabled by default.
Rob