2018-06-18 17:45:19

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 1/2] arm64: dts: exynos: Remove leading 0x from unit addresses in Exynos5433

Remove leading 0x from recently introduced unit addresses to fix DTC
warnings:

Warning (unit_address_format): /soc/sysmmu@0x15040000: unit name should not have leading "0x"

Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
arch/arm64/boot/dts/exynos/exynos5433.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
index 038c99792ccb..3a9b4c4b9c63 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
@@ -1171,7 +1171,7 @@
power-domains = <&pd_gscl>;
};

- sysmmu_scaler_0: sysmmu@0x15040000 {
+ sysmmu_scaler_0: sysmmu@15040000 {
compatible = "samsung,exynos-sysmmu";
reg = <0x15040000 0x1000>;
interrupts = <GIC_SPI 404 IRQ_TYPE_LEVEL_HIGH>;
@@ -1182,7 +1182,7 @@
power-domains = <&pd_mscl>;
};

- sysmmu_scaler_1: sysmmu@0x15050000 {
+ sysmmu_scaler_1: sysmmu@15050000 {
compatible = "samsung,exynos-sysmmu";
reg = <0x15050000 0x1000>;
interrupts = <GIC_SPI 406 IRQ_TYPE_LEVEL_HIGH>;
--
2.14.1



2018-06-18 17:44:33

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 2/2] arm64: dts: exynos: Remove unneeded DSI and DECON address/size cells in Exynos5433

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 <[email protected]>
---
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>;
};
--
2.14.1


2018-06-18 17:59:50

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 1/2] arm64: dts: exynos: Remove leading 0x from unit addresses in Exynos5433

On Mon, 2018-06-18 at 19:42 +0200, Krzysztof Kozlowski wrote:
> Remove leading 0x from recently introduced unit addresses to fix DTC
> warnings:
>
> Warning (unit_address_format): /soc/sysmmu@0x15040000: unit name should not have leading "0x"
[]
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
[]
> @@ -1171,7 +1171,7 @@
> power-domains = <&pd_gscl>;
> };
>
> - sysmmu_scaler_0: sysmmu@0x15040000 {
> + sysmmu_scaler_0: sysmmu@15040000 {
> compatible = "samsung,exynos-sysmmu";
> reg = <0x15040000 0x1000>;
> interrupts = <GIC_SPI 404 IRQ_TYPE_LEVEL_HIGH>;
> @@ -1182,7 +1182,7 @@
> power-domains = <&pd_mscl>;
> };
>
> - sysmmu_scaler_1: sysmmu@0x15050000 {
> + sysmmu_scaler_1: sysmmu@15050000 {
> compatible = "samsung,exynos-sysmmu";
> reg = <0x15050000 0x1000>;
> interrupts = <GIC_SPI 406 IRQ_TYPE_LEVEL_HIGH>;

Presumably these 3 other entries too?

$ git grep -P "\b\w+:\s+\w+@0x" -- "*.dt*"
arch/arc/boot/dts/abilis_tb10x.dtsi: spi0: spi@0xFE010000 {
arch/arc/boot/dts/abilis_tb10x.dtsi: spi1: spi@0xFE011000 {
arch/arc/boot/dts/vdk_axs10x_mb.dtsi: uio_ev: uio@0xD0000000 {
arch/arm64/boot/dts/exynos/exynos5433.dtsi: sysmmu_scaler_0: sysmmu@0x15040000 {
arch/arm64/boot/dts/exynos/exynos5433.dtsi: sysmmu_scaler_1: sysmmu@0x15050000 {

2018-06-18 18:40:00

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm64: dts: exynos: Remove unneeded DSI and DECON address/size cells in Exynos5433

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 <[email protected]>
> ---
> 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


2018-06-18 19:06:22

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/2] arm64: dts: exynos: Remove leading 0x from unit addresses in Exynos5433

On 18 June 2018 at 19:57, Joe Perches <[email protected]> wrote:
> On Mon, 2018-06-18 at 19:42 +0200, Krzysztof Kozlowski wrote:
>> Remove leading 0x from recently introduced unit addresses to fix DTC
>> warnings:
>>
>> Warning (unit_address_format): /soc/sysmmu@0x15040000: unit name should not have leading "0x"
> []
>> diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> []
>> @@ -1171,7 +1171,7 @@
>> power-domains = <&pd_gscl>;
>> };
>>
>> - sysmmu_scaler_0: sysmmu@0x15040000 {
>> + sysmmu_scaler_0: sysmmu@15040000 {
>> compatible = "samsung,exynos-sysmmu";
>> reg = <0x15040000 0x1000>;
>> interrupts = <GIC_SPI 404 IRQ_TYPE_LEVEL_HIGH>;
>> @@ -1182,7 +1182,7 @@
>> power-domains = <&pd_mscl>;
>> };
>>
>> - sysmmu_scaler_1: sysmmu@0x15050000 {
>> + sysmmu_scaler_1: sysmmu@15050000 {
>> compatible = "samsung,exynos-sysmmu";
>> reg = <0x15050000 0x1000>;
>> interrupts = <GIC_SPI 406 IRQ_TYPE_LEVEL_HIGH>;
>
> Presumably these 3 other entries too?
>
> $ git grep -P "\b\w+:\s+\w+@0x" -- "*.dt*"
> arch/arc/boot/dts/abilis_tb10x.dtsi: spi0: spi@0xFE010000 {
> arch/arc/boot/dts/abilis_tb10x.dtsi: spi1: spi@0xFE011000 {
> arch/arc/boot/dts/vdk_axs10x_mb.dtsi: uio_ev: uio@0xD0000000 {

True. That's an "arc" but I can fix it as well with your reported-by.

Best regards,
Krzysztof

2018-06-19 07:28:26

by Andrzej Hajda

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm64: dts: exynos: Remove unneeded DSI and DECON address/size cells in Exynos5433

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 <[email protected]>

Reviewed-by: Andrzej Hajda <[email protected]>

 --
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>;
> };



2018-06-19 07:29:19

by Marek Szyprowski

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm64: dts: exynos: Remove unneeded DSI and DECON address/size cells in Exynos5433

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 <[email protected]>

Works fine with current Exynos DRM Decon/MIC/DSI drivers.

Tested-by: Marek Szyprowski <[email protected]>

> ---
> 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
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland


2018-06-19 08:00:21

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm64: dts: exynos: Remove unneeded DSI and DECON address/size cells in Exynos5433

On 19 June 2018 at 09:26, Marek Szyprowski <[email protected]> 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 <[email protected]>
>
> Works fine with current Exynos DRM Decon/MIC/DSI drivers.
>
> Tested-by: Marek Szyprowski <[email protected]>

Thanks for review and testing!

Best regards,
Krzysztof

2018-06-20 19:35:55

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm64: dts: exynos: Remove unneeded DSI and DECON address/size cells in Exynos5433

On Tue, Jun 19, 2018 at 09:59:04AM +0200, Krzysztof Kozlowski wrote:
> On 19 June 2018 at 09:26, Marek Szyprowski <[email protected]> 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 <[email protected]>
> >
> > Works fine with current Exynos DRM Decon/MIC/DSI drivers.
> >
> > Tested-by: Marek Szyprowski <[email protected]>
>
> 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


2018-06-21 06:01:07

by Andrzej Hajda

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm64: dts: exynos: Remove unneeded DSI and DECON address/size cells in Exynos5433

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 <[email protected]> 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 <[email protected]>
>>> Works fine with current Exynos DRM Decon/MIC/DSI drivers.
>>>
>>> Tested-by: Marek Szyprowski <[email protected]>
>> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>


2018-06-21 07:23:21

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/2] arm64: dts: exynos: Remove unneeded DSI and DECON address/size cells in Exynos5433

On 21 June 2018 at 07:59, Andrzej Hajda <[email protected]> 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 <[email protected]> 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 <[email protected]>
>>>> Works fine with current Exynos DRM Decon/MIC/DSI drivers.
>>>>
>>>> Tested-by: Marek Szyprowski <[email protected]>
>>> 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