| Message ID | 20180618174216.24801-2-krzk@kernel.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On Mon, Jun 18, 2018 at 07:42:16PM +0200, Krzysztof Kozlowski wrote: > The decon, decon_tv and dsi nodes have only one child port so > address/size mappings are not necessary. This fixes DTC warnings like: > > Warning (graph_child_address): /soc/decon@13800000/ports: > graph node has single child node 'port@0', #address-cells/#size-cells are not necessary > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > --- > arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 6 +----- > arch/arm64/boot/dts/exynos/exynos5433.dtsi | 12 ++---------- > 2 files changed, 3 insertions(+), 15 deletions(-) The patch 1/2 should not have any impact but this one could have and I forgot to mention that it was not tested. If someone could provide testing, it would be highly appreciated. Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 18.06.2018 19:42, Krzysztof Kozlowski wrote: > The decon, decon_tv and dsi nodes have only one child port so > address/size mappings are not necessary. This fixes DTC warnings like: > > Warning (graph_child_address): /soc/decon@13800000/ports: > graph node has single child node 'port@0', #address-cells/#size-cells are not necessary DECON nodes according to documentation have only one port so it is OK. DSI has two ports, but since on all Exynos platforms DSI panels/bridges are subnodes of ExynosDSI the 2nd port is not used. So if there will be a platform with DSI panel/bridge controlled via I2C, it should be reverted., but this is theoretical problem. > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> Reviewed-by: Andrzej Hajda <a.hajda@samsung.com> -- Regards Andrzej > --- > arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 6 +----- > arch/arm64/boot/dts/exynos/exynos5433.dtsi | 12 ++---------- > 2 files changed, 3 insertions(+), 15 deletions(-) > > diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi > index a1e3194b7483..0a15ee513f5c 100644 > --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi > +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi > @@ -321,11 +321,7 @@ > status = "okay"; > > ports { > - #address-cells = <1>; > - #size-cells = <0>; > - > - port@0 { > - reg = <0>; > + port { > tv_to_hdmi: endpoint { > remote-endpoint = <&hdmi_to_tv>; > }; > diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi > index 3a9b4c4b9c63..e4367fd39120 100644 > --- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi > +++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi > @@ -850,11 +850,7 @@ > iommu-names = "m0", "m1"; > > ports { > - #address-cells = <1>; > - #size-cells = <0>; > - > - port@0 { > - reg = <0>; > + port { > decon_to_mic: endpoint { > remote-endpoint = > <&mic_to_decon>; > @@ -914,11 +910,7 @@ > #size-cells = <0>; > > ports { > - #address-cells = <1>; > - #size-cells = <0>; > - > - port@0 { > - reg = <0>; > + port { > dsi_to_mic: endpoint { > remote-endpoint = <&mic_to_dsi>; > }; -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Krzysztof, On 2018-06-18 19:42, Krzysztof Kozlowski wrote: > The decon, decon_tv and dsi nodes have only one child port so > address/size mappings are not necessary. This fixes DTC warnings like: > > Warning (graph_child_address): /soc/decon@13800000/ports: > graph node has single child node 'port@0', #address-cells/#size-cells are not necessary > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> Works fine with current Exynos DRM Decon/MIC/DSI drivers. Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 6 +----- > arch/arm64/boot/dts/exynos/exynos5433.dtsi | 12 ++---------- > 2 files changed, 3 insertions(+), 15 deletions(-) > > diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi > index a1e3194b7483..0a15ee513f5c 100644 > --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi > +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi > @@ -321,11 +321,7 @@ > status = "okay"; > > ports { > - #address-cells = <1>; > - #size-cells = <0>; > - > - port@0 { > - reg = <0>; > + port { > tv_to_hdmi: endpoint { > remote-endpoint = <&hdmi_to_tv>; > }; > diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi > index 3a9b4c4b9c63..e4367fd39120 100644 > --- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi > +++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi > @@ -850,11 +850,7 @@ > iommu-names = "m0", "m1"; > > ports { > - #address-cells = <1>; > - #size-cells = <0>; > - > - port@0 { > - reg = <0>; > + port { > decon_to_mic: endpoint { > remote-endpoint = > <&mic_to_decon>; > @@ -914,11 +910,7 @@ > #size-cells = <0>; > > ports { > - #address-cells = <1>; > - #size-cells = <0>; > - > - port@0 { > - reg = <0>; > + port { > dsi_to_mic: endpoint { > remote-endpoint = <&mic_to_dsi>; > }; Best regards
On 19 June 2018 at 09:26, Marek Szyprowski <m.szyprowski@samsung.com> wrote: > Hi Krzysztof, > > On 2018-06-18 19:42, Krzysztof Kozlowski wrote: >> The decon, decon_tv and dsi nodes have only one child port so >> address/size mappings are not necessary. This fixes DTC warnings like: >> >> Warning (graph_child_address): /soc/decon@13800000/ports: >> graph node has single child node 'port@0', #address-cells/#size-cells are not necessary >> >> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > > Works fine with current Exynos DRM Decon/MIC/DSI drivers. > > Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> Thanks for review and testing! Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jun 19, 2018 at 09:59:04AM +0200, Krzysztof Kozlowski wrote: > On 19 June 2018 at 09:26, Marek Szyprowski <m.szyprowski@samsung.com> wrote: > > Hi Krzysztof, > > > > On 2018-06-18 19:42, Krzysztof Kozlowski wrote: > >> The decon, decon_tv and dsi nodes have only one child port so > >> address/size mappings are not necessary. This fixes DTC warnings like: > >> > >> Warning (graph_child_address): /soc/decon@13800000/ports: > >> graph node has single child node 'port@0', #address-cells/#size-cells are not necessary > >> > >> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > > > > Works fine with current Exynos DRM Decon/MIC/DSI drivers. > > > > Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> > > Thanks for review and testing! I have second thoughs whether this patch is correct. AFAIU, the drivers get the remote endpoints by reg==0 (for example the of_graph_get_remote_node() in exynos_dsi_parse_dt()). If the port shall be ignored, then reg==-1 should be passed. Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 20.06.2018 21:34, Krzysztof Kozlowski wrote: > On Tue, Jun 19, 2018 at 09:59:04AM +0200, Krzysztof Kozlowski wrote: >> On 19 June 2018 at 09:26, Marek Szyprowski <m.szyprowski@samsung.com> wrote: >>> Hi Krzysztof, >>> >>> On 2018-06-18 19:42, Krzysztof Kozlowski wrote: >>>> The decon, decon_tv and dsi nodes have only one child port so >>>> address/size mappings are not necessary. This fixes DTC warnings like: >>>> >>>> Warning (graph_child_address): /soc/decon@13800000/ports: >>>> graph node has single child node 'port@0', #address-cells/#size-cells are not necessary >>>> >>>> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> >>> Works fine with current Exynos DRM Decon/MIC/DSI drivers. >>> >>> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> >> Thanks for review and testing! > I have second thoughs whether this patch is correct. AFAIU, the drivers > get the remote endpoints by reg==0 (for example the > of_graph_get_remote_node() in exynos_dsi_parse_dt()). If the port shall > be ignored, then reg==-1 should be passed. All this is about purity, DECON bindings says explicitly that there should be a port with reg=0. So your patch and DTC warnings are incorrect from bindings PoV. On the other side graph bindings are too bloated ( so many lines to describe one connection ) so I am happy if there are shrinking attempts :) Functionally nothing changes, of graph helpers assume reg=0 if it is not present in port/endpoint node. And regarding real issues, DECON could have more ports, possible candidates: - GSCALER0/1/2, - GSD/DSD - interconnect between GSCALERs and DECONs, - SMIES - image enhancer (not implemented), - MIC0/1 - image enhancers, - DSIM0/1 - DSI encoders, - HDMI - HDMI encoder. But since all these connections can be configured dynamically, and more importantly are inside specific SoC I dont think they need of_graphs. In fact I think current of graph is also not necessary, but this is different story, removal is on my long TODO list :) Regards Andrzej > > Best regards, > Krzysztof > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 21 June 2018 at 07:59, Andrzej Hajda <a.hajda@samsung.com> wrote: > On 20.06.2018 21:34, Krzysztof Kozlowski wrote: >> On Tue, Jun 19, 2018 at 09:59:04AM +0200, Krzysztof Kozlowski wrote: >>> On 19 June 2018 at 09:26, Marek Szyprowski <m.szyprowski@samsung.com> wrote: >>>> Hi Krzysztof, >>>> >>>> On 2018-06-18 19:42, Krzysztof Kozlowski wrote: >>>>> The decon, decon_tv and dsi nodes have only one child port so >>>>> address/size mappings are not necessary. This fixes DTC warnings like: >>>>> >>>>> Warning (graph_child_address): /soc/decon@13800000/ports: >>>>> graph node has single child node 'port@0', #address-cells/#size-cells are not necessary >>>>> >>>>> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> >>>> Works fine with current Exynos DRM Decon/MIC/DSI drivers. >>>> >>>> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> >>> Thanks for review and testing! >> I have second thoughs whether this patch is correct. AFAIU, the drivers >> get the remote endpoints by reg==0 (for example the >> of_graph_get_remote_node() in exynos_dsi_parse_dt()). If the port shall >> be ignored, then reg==-1 should be passed. > > All this is about purity, DECON bindings says explicitly that there > should be a port with reg=0. The warnings themself are indeed about purity.... but not having warnings is quite useful. When you have already warnings either in C code or in DTC, it is quite easy to skip new ones. We successfully get rid of all DTC warnings from ARM part. It would be nice to have the same for ARM64... but not with a cost of making bogus changes. > So your patch and DTC warnings are incorrect from bindings PoV. Good point so this patch should come along with changing of bindings (and maybe the code as well to use -1 as port/reg). > On the other side graph bindings are too bloated ( so many lines to > describe one connection ) so I am happy if there are shrinking attempts :) > Functionally nothing changes, of graph helpers assume reg=0 if it is not > present in port/endpoint node. > > And regarding real issues, DECON could have more ports, possible candidates: > - GSCALER0/1/2, > - GSD/DSD - interconnect between GSCALERs and DECONs, > - SMIES - image enhancer (not implemented), > - MIC0/1 - image enhancers, > - DSIM0/1 - DSI encoders, > - HDMI - HDMI encoder. > > But since all these connections can be configured dynamically, and more > importantly are inside specific SoC I dont think they need of_graphs. In > fact I think current of graph is also not necessary, but this is > different story, removal is on my long TODO list :) I keep my fingers crossed for this :) Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi index a1e3194b7483..0a15ee513f5c 100644 --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi @@ -321,11 +321,7 @@ status = "okay"; ports { - #address-cells = <1>; - #size-cells = <0>; - - port@0 { - reg = <0>; + port { tv_to_hdmi: endpoint { remote-endpoint = <&hdmi_to_tv>; }; diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi index 3a9b4c4b9c63..e4367fd39120 100644 --- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi +++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi @@ -850,11 +850,7 @@ iommu-names = "m0", "m1"; ports { - #address-cells = <1>; - #size-cells = <0>; - - port@0 { - reg = <0>; + port { decon_to_mic: endpoint { remote-endpoint = <&mic_to_decon>; @@ -914,11 +910,7 @@ #size-cells = <0>; ports { - #address-cells = <1>; - #size-cells = <0>; - - port@0 { - reg = <0>; + port { dsi_to_mic: endpoint { remote-endpoint = <&mic_to_dsi>; };
The decon, decon_tv and dsi nodes have only one child port so address/size mappings are not necessary. This fixes DTC warnings like: Warning (graph_child_address): /soc/decon@13800000/ports: graph node has single child node 'port@0', #address-cells/#size-cells are not necessary Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> --- arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 6 +----- arch/arm64/boot/dts/exynos/exynos5433.dtsi | 12 ++---------- 2 files changed, 3 insertions(+), 15 deletions(-)