ARM: dts: Change i2s compatible string on exynos5250

PATCH New Padmavathi Venna 6 participants 8 messages Aug 16, 2013 – Aug 18, 2013

Subsystems (2)

Device tree bindings Arm Samsung Exynos

lore

Share & tools

lore

Padmavathi Venna

author

2013-08-16 04:25 UTC #0 lore raw

PATCH v5 1/1 ARM: dts: Change i2s compatible string on exynos5250

This patch removes quirks from i2s node and change the i2s compatible names.

Signed-off-by Padmavathi Venna <[email protected]>

---

Changes since V4:
- Mark Brown reverted the below two patches due to below build failure with exynos_defconfig.
"ARM: dts: exynos5250: move common i2s properties to exynos5 dtsi" "ARM: dts: Change i2s compatible string on exynos5250"

build error:
DTC arch/arm/boot/dts/exynos5420-smdk5420.dtb
ERROR (phandle_references): Reference to non-existent node or label "pdma1" ERROR (phandle_references): Reference to non-existent node or label "pdma1" ERROR (phandle_references): Reference to non-existent node or label "pdma0" ERROR (phandle_references): Reference to non-existent node or label "pdma0"

But with out "ARM: dts: Change i2s compatible string on exynos5250" will break the i2s driver.

So posting this patch now and will take care of posting other patch later after dependent patches got merged.

 arch/arm/boot/dts/exynos5250.dtsi |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

arch/arm/boot/dts/exynos5250.dtsi +3 -6

index ef57277..376090f 100644
@@ -405,7 +405,7 @@
 	};
 
 	i2s0: i2s@03830000 {
-		compatible = "samsung,i2s-v5";
+		compatible = "samsung,s5pv210-i2s";
 		reg = <0x03830000 0x100>;
 		dmas = <&pdma0 10
 			&pdma0 9

@@ -415,16 +415,13 @@
 			<&clock_audss EXYNOS_I2S_BUS>,
 			<&clock_audss EXYNOS_SCLK_I2S>;
 		clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
-		samsung,supports-6ch;
-		samsung,supports-rstclr;
-		samsung,supports-secdai;
 		samsung,idma-addr = <0x03000000>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&i2s0_bus>;
 	};
 
 	i2s1: i2s@12D60000 {
-		compatible = "samsung,i2s-v5";
+		compatible = "samsung,s3c6410-i2s";
 		reg = <0x12D60000 0x100>;
 		dmas = <&pdma1 12
 			&pdma1 11>;

@@ -436,7 +433,7 @@
 	};
 
 	i2s2: i2s@12D70000 {
-		compatible = "samsung,i2s-v5";
+		compatible = "samsung,s3c6410-i2s";
 		reg = <0x12D70000 0x100>;
 		dmas = <&pdma0 12
 			&pdma0 11>;

Signature · 1 line

1.7.4.4

Olof Johansson

2013-08-16 04:48 UTC #1 lore raw

PATCH v5 1/1 ARM: dts: Change i2s compatible string on exynos5250

Quoted 33 lines · Padmavathi Venna

This patch removes quirks from i2s node and change the i2s compatible names.
Signed-off-by Padmavathi Venna <[email protected]>
---
Changes since V4:
- Mark Brown reverted the below two patches due to below build failure with exynos_defconfig.
"ARM: dts: exynos5250: move common i2s properties to exynos5 dtsi" "ARM: dts: Change i2s compatible string on exynos5250"
build error:
DTC arch/arm/boot/dts/exynos5420-smdk5420.dtb
ERROR (phandle_references): Reference to non-existent node or label "pdma1" ERROR (phandle_references): Reference to non-existent node or label "pdma1" ERROR (phandle_references): Reference to non-existent node or label "pdma0" ERROR (phandle_references): Reference to non-existent node or label "pdma0"
But with out "ARM: dts: Change i2s compatible string on exynos5250" will break the i2s driver.
So posting this patch now and will take care of posting other patch later after dependent patches got merged.
 arch/arm/boot/dts/exynos5250.dtsi |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)
arch/arm/boot/dts/exynos5250.dtsi +0 -0
index ef57277..376090f 100644

arch/arm/boot/dts/exynos5250.dtsi line 408

Quoted 5 lines

arch/arm/boot/dts/exynos5250.dtsi +1 -1

@@ -405,7 +405,7 @@
        };
 
        i2s0: i2s at 03830000 {
-               compatible = "samsung,i2s-v5";
+               compatible = "samsung,s5pv210-i2s";

Device tree reviewers, this is something to look out for in the future. Some samsung platforms/drivers use "samsung,<chip>-<ip>", others "samsung,<ip>-<chip>". I don't personally care much one way or another, but it really should be consistent.

-Olof

Kukjin Kim

2013-08-16 05:43 UTC #2 lore raw

PATCH v5 1/1 ARM: dts: Change i2s compatible string on exynos5250

Quoted 6 lines · Olof Johansson

Quoted 3 lines · Padmavathi Venna
This patch removes quirks from i2s node and change the i2s compatible names.

Quoted 8 lines

Quoted 5 lines
Signed-off-by Padmavathi Venna <[email protected]>
---
Changes since V4:
- Mark Brown reverted the below two patches due to below build
failure with
Quoted 2 lines
exynos_defconfig.
"ARM: dts: exynos5250: move common i2s properties to

exynos5

Quoted 33 lines

dtsi"
Quoted 5 lines
"ARM: dts: Change i2s compatible string on exynos5250"
build error:
DTC arch/arm/boot/dts/exynos5420-smdk5420.dtb
ERROR (phandle_references): Reference to non-existent node or
label "pdma1"
Quoted 1 line
ERROR (phandle_references): Reference to non-existent node or
label "pdma1"
Quoted 1 line
ERROR (phandle_references): Reference to non-existent node or
label "pdma0"
Quoted 1 line
ERROR (phandle_references): Reference to non-existent node or
label "pdma0"
Quoted 2 lines
But with out "ARM: dts: Change i2s compatible string on exynos5250" will
break the i2s
Quoted 3 lines
driver.
So posting this patch now and will take care of posting other patch
later after dependent
Review
Quoted 6 lines
patches got merged.
 arch/arm/boot/dts/exynos5250.dtsi |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)
(quoted diff) +0 -0
b/arch/arm/boot/dts/exynos5250.dtsi
Quoted 6 lines
index ef57277..376090f 100644
arch/arm/boot/dts/exynos5250.dtsi +0 -0
@@ -405,7 +405,7 @@
        };
 

Quoted 9 lines

arch/arm/boot/dts/exynos5250.dtsi line 408
Quoted 3 lines
arch/arm/boot/dts/exynos5250.dtsi +1 -1
        i2s0: i2s at 03830000 {
-               compatible = "samsung,i2s-v5";
+               compatible = "samsung,s5pv210-i2s";
Device tree reviewers, this is something to look out for in the future. Some samsung platforms/drivers use "samsung,<chip>-<ip>", others "samsung,<ip>-<chip>". I don't personally care much one way or another, but it really should be consistent.

Hmm...I think, if "samsung,<ip name>-<ip version>" is possible, it would be nice. I remember there are no versions in datasheet for some IPs but something have like i2s and mfc. So "samsung,<ip>-<version>" is used for only i2s and mfc. But actually there are versions for Samsung IPs, no comments for that in datasheet. So I think, if Samsung can provide the specific version of Samsung IPs, we can use that like other platforms. I will prepare some table for that after meeting with Samsung hardware IP team so that samsung platform use one format "samsung,<ip>-<version>".

Thanks,
Kukjin

Tomasz Figa

2013-08-16 07:53 UTC #3 lore raw

PATCH v5 1/1 ARM: dts: Change i2s compatible string on exynos5250

Hi Kukjin,

Quoted 86 lines · On Friday 16 of August 2013 14:43:17 Kukjin Kim wrote:

Quoted 7 lines · Olof Johansson
On Thu, Aug 15, 2013 at 9:26 PM, Padmavathi Venna
<[email protected]>
Quoted 3 lines · wrote:
This patch removes quirks from i2s node and change the i2s compatible names.
Quoted 12 lines
Quoted 6 lines
Signed-off-by Padmavathi Venna <[email protected]>
---
Changes since V4:
- Mark Brown reverted the below two patches due to below build
failure with
Quoted 3 lines
exynos_defconfig.
"ARM: dts: exynos5250: move common i2s properties to
exynos5
Quoted 55 lines
dtsi"
Quoted 7 lines
"ARM: dts: Change i2s compatible string on exynos5250"
build error:
DTC arch/arm/boot/dts/exynos5420-smdk5420.dtb
ERROR (phandle_references): Reference to non-existent node or
label "pdma1"
Quoted 2 lines
ERROR (phandle_references): Reference to non-existent node or
label "pdma1"
Quoted 2 lines
ERROR (phandle_references): Reference to non-existent node or
label "pdma0"
Quoted 2 lines
ERROR (phandle_references): Reference to non-existent node or
label "pdma0"
Quoted 2 lines
But with out "ARM: dts: Change i2s compatible string on exynos5250" will>
break the i2s
Quoted 3 lines
driver.
So posting this patch now and will take care of posting other patch
later after dependent
Review
Quoted 6 lines
patches got merged.
 arch/arm/boot/dts/exynos5250.dtsi |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)
(quoted diff) +0 -0
b/arch/arm/boot/dts/exynos5250.dtsi
Quoted 9 lines
index ef57277..376090f 100644
arch/arm/boot/dts/exynos5250.dtsi +0 -0
@@ -405,7 +405,7 @@
 
        };
 
        i2s0: i2s at 03830000 {
 
Quoted 7 lines
arch/arm/boot/dts/exynos5250.dtsi line 408
Quoted 2 lines
arch/arm/boot/dts/exynos5250.dtsi +1 -1
-               compatible = "samsung,i2s-v5";
+               compatible = "samsung,s5pv210-i2s";
Device tree reviewers, this is something to look out for in the future. Some samsung platforms/drivers use "samsung,<chip>-<ip>", others "samsung,<ip>-<chip>". I don't personally care much one way or another, but it really should be consistent.

Quoted 9 lines

Hmm...I think, if "samsung,<ip name>-<ip version>" is possible, it would be nice. I remember there are no versions in datasheet for some IPs but something have like i2s and mfc. So "samsung,<ip>-<version>" is used for only i2s and mfc. But actually there are versions for Samsung IPs, no comments for that in datasheet. So I think, if Samsung can provide the specific version of Samsung IPs, we can use that like other platforms. I will prepare some table for that after meeting with Samsung hardware IP team so that samsung platform use one format "samsung,<ip>-<version>".

Sorry, I don't think this is a good idea, unless you can force the IP team to release a version table containing version of _every_ IP for _every_ released SoC, including those historical ones, like S3C24xx and S3C64xx. Of course such tables should be available publicly.

In addition, there might be other funny things going on with IPs and their surroundings, that could make a need to create several separate compatible values for the same IP revision, but on different SoCs, because it was integrated in a slightly different way.

I believe we have choses the "samsung,<chip>-<ip>" scheme to avoid being dependent upon data that is not always publicly available, which is more future- (and past-) proof and also solves the integration problem.

Best regards,
Tomasz
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130816/bcd090e8/attachment.sig>

Mark Brown

2013-08-16 09:59 UTC #4 lore raw

PATCH v5 1/1 ARM: dts: Change i2s compatible string on exynos5250

Quoted 4 lines · Tomasz Figa

Sorry, I don't think this is a good idea, unless you can force the IP team to release a version table containing version of _every_ IP for _every_ released SoC, including those historical ones, like S3C24xx and S3C64xx. Of course such tables should be available publicly.

I don't think public availability is much of an issue here, the
datasheets are all NDAed anyway.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130816/122ba697/attachment.sig>

Kukjin Kim

2013-08-18 18:05 UTC #5 lore raw

PATCH v5 1/1 ARM: dts: Change i2s compatible string on exynos5250

In reply to Tomasz Figa

Quoted 2 lines · Tomasz Figa

Hi Kukjin,

[...]

Quoted 18 lines

Quoted 8 lines
Quoted 7 lines
arch/arm/boot/dts/exynos5250.dtsi line 408
Quoted 2 lines
arch/arm/boot/dts/exynos5250.dtsi +1 -1
-               compatible = "samsung,i2s-v5";
+               compatible = "samsung,s5pv210-i2s";
Device tree reviewers, this is something to look out for in the future. Some samsung platforms/drivers use "samsung,<chip>-<ip>", others "samsung,<ip>-<chip>". I don't personally care much one way or another, but it really should be consistent.
Quoted 9 lines
Hmm...I think, if "samsung,<ip name>-<ip version>" is possible, it would be nice. I remember there are no versions in datasheet for some IPs but something have like i2s and mfc. So "samsung,<ip>-<version>" is used for only i2s and mfc. But actually there are versions for Samsung IPs, no comments for that in datasheet. So I think, if Samsung can provide the specific version of Samsung IPs, we can use that like other platforms. I will prepare some table for that after meeting with Samsung hardware IP team so that samsung platform use one format "samsung,<ip>-<version>".

Quoted 5 lines

Sorry, I don't think this is a good idea, unless you can force the IP team to release a version table containing version of _every_ IP for _every_ released SoC, including those historical ones, like S3C24xx and S3C64xx. Of course such tables should be available publicly.

It's different issue and I agree with Mark Brown's comments.

Quoted 9 lines

In addition, there might be other funny things going on with IPs and their surroundings, that could make a need to create several separate compatible values for the same IP revision, but on different SoCs, because it was integrated in a slightly different way.
I believe we have choses the "samsung,<chip>-<ip>" scheme to avoid being dependent upon data that is not always publicly available, which is more future- (and past-) proof and also solves the integration problem.

Well, I don't think so, because Samsung is no more just SoC vendor. Actually Samsung is providing just IP to customers, I think, they don't like to use the name, exynos**** for their SoC or Chip, because exynos, s3c or s5p whatever is Samsung's SoC brand name. If so, I think, my suggestion is more reasonable. See the example of Synopsys and ARM primecell...that's why I will try to do it. But I know I need to get agreement from DT guys and maybe I need more time than I expect ;-)

Thanks,
Kukjin

Tomasz Figa

2013-08-16 07:48 UTC #6 lore raw

PATCH v5 1/1 ARM: dts: Change i2s compatible string on exynos5250

In reply to Olof Johansson

Hi Olof,

Quoted 1 line · On Thursday 15 of August 2013 21:48:32 Olof Johansson wrote:

On Thu, Aug 15, 2013 at 9:26 PM, Padmavathi Venna <[email protected]>

Quoted 42 lines · wrote:

Quoted 42 lines
This patch removes quirks from i2s node and change the i2s compatible names.
Signed-off-by Padmavathi Venna <[email protected]>
---
Changes since V4:
- Mark Brown reverted the below two patches due to below build failure with>
exynos_defconfig.
"ARM: dts: exynos5250: move common i2s properties to exynos5 dtsi"
"ARM: dts: Change i2s compatible string on exynos5250"
build error:
DTC arch/arm/boot/dts/exynos5420-smdk5420.dtb
ERROR (phandle_references): Reference to non-existent node or label "pdma1" ERROR (phandle_references): Reference to non-existent node or label "pdma1" ERROR (phandle_references): Reference to non-existent node or label "pdma0" ERROR (phandle_references): Reference to non-existent node or label "pdma0">
But with out "ARM: dts: Change i2s compatible string on exynos5250" will break the i2s driver.
So posting this patch now and will take care of posting other patch later after dependent patches got merged.
 arch/arm/boot/dts/exynos5250.dtsi |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)
(quoted diff) +0 -0
b/arch/arm/boot/dts/exynos5250.dtsi index ef57277..376090f 100644
arch/arm/boot/dts/exynos5250.dtsi +0 -0
@@ -405,7 +405,7 @@
 
        };
 
        i2s0: i2s at 03830000 {
 

Quoted 7 lines

arch/arm/boot/dts/exynos5250.dtsi line 408
Quoted 2 lines
arch/arm/boot/dts/exynos5250.dtsi +1 -1
-               compatible = "samsung,i2s-v5";
+               compatible = "samsung,s5pv210-i2s";
Device tree reviewers, this is something to look out for in the future. Some samsung platforms/drivers use "samsung,<chip>-<ip>", others "samsung,<ip>-<chip>". I don't personally care much one way or another, but it really should be consistent.

Hmm, I just did a quick git grep over arch/arm/boot/dts and could not really find the "samsung,<ip>-<chip>" pattern.

Except MFC (and without Padma's patch also I2S), which used "samsung,<ip>- <ip version>, all the compatible values I can see are based on "samsung,<chip>-<ip>" pattern.

I believe we discussed this issue several times already and finally agreed to use "samsung,<chip>-<ip>" for any new compatible values.

Mark Brown

2013-08-16 10:06 UTC #7 lore raw

PATCH v5 1/1 ARM: dts: Change i2s compatible string on exynos5250

In reply to Padmavathi Venna

Quoted 2 lines · Padmavathi Venna

This patch removes quirks from i2s node and change the i2s compatible names.

Applied, thanks.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130816/a5820224/attachment-0001.sig>