[PATCH v2 03/10] dt-bindings: sound: Add new reset compatible for sun4i-i2s

[PATCH v2 03/10] dt-bindings: sound: Add new reset compatible for sun4i-i2s

[Mylène Josserand]

Add a new compatible for sun4i-i2s driver to handle some
SoCs that have a reset line that must be asserted/deasserted.

This new compatible, "allwinner,sun6i-a31-i2s", requires two
properties:
	- resets: phandle to the reset line
	- reset-names: the name of the reset line ("rst").
Except these differences, the compatible is identical to previous one
which will not handle a reset line.

Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
---
 .../devicetree/bindings/sound/sun4i-i2s.txt        | 23 ++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
index 7a2c0945fd22..f673206e309b 100644
--- a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
+++ b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
@@ -7,6 +7,7 @@ Required properties:
 
 - compatible: should be one of the following:
    - "allwinner,sun4i-a10-i2s"
+   - "allwinner,sun6i-a31-i2s" for controller with reset lines
 - reg: physical base address of the controller and length of memory mapped
   region.
 - interrupts: should contain the I2S interrupt.
@@ -19,7 +20,13 @@ Required properties:
    - "mod" : module clock for the I2S controller
 - #sound-dai-cells : Must be equal to 0
 
+Required properties for the following compatibles:
+		- "allwinner,sun6i-a31-i2s"
+- resets: phandle to the reset line for this codec
+- reset-names: Contains the reset signal name "rst"
+
 Example:
+For "allwinner,sun4i-a10-i2s":
 
 i2s0: i2s@01c22400 {
 	#sound-dai-cells = <0>;
@@ -32,3 +39,19 @@ i2s0: i2s@01c22400 {
 	       <&dma SUN4I_DMA_NORMAL 3>;
 	dma-names = "rx", "tx";
 };
+
+For "allwinner,sun6i-a31-i2s":
+
+dai: dai@01c22c00 {
+	#sound-dai-cells = <0>;
+	compatible = "allwinner,sun6i-a31-i2s";
+	reg = <0x01c22c00 0x200>;
+	interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
+	clocks = <&ccu CLK_BUS_CODEC>, <&ccu CLK_AC_DIG>;
+	clock-names = "apb", "mod";
+	resets = <&ccu RST_BUS_CODEC>;
+	reset-names = "rst";
+	dmas = <&dma 15>, /* AUDIO_CODEC port */
+		<&dma 15>; /* AUDIO_CODEC port */
+	dma-names = "rx", "tx";
+};
-- 
2.11.0

Re: [PATCH v2 03/10] dt-bindings: sound: Add new reset compatible for sun4i-i2s

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2822 bytes --]

On Tue, Jan 17, 2017 at 03:02:23PM +0100, Mylène Josserand wrote:
> Add a new compatible for sun4i-i2s driver to handle some
> SoCs that have a reset line that must be asserted/deasserted.
> 
> This new compatible, "allwinner,sun6i-a31-i2s", requires two
> properties:
> 	- resets: phandle to the reset line
> 	- reset-names: the name of the reset line ("rst").
> Except these differences, the compatible is identical to previous one
> which will not handle a reset line.
> 
> Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
> ---
>  .../devicetree/bindings/sound/sun4i-i2s.txt        | 23 ++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
> index 7a2c0945fd22..f673206e309b 100644
> --- a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
> +++ b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
> @@ -7,6 +7,7 @@ Required properties:
>  
>  - compatible: should be one of the following:
>     - "allwinner,sun4i-a10-i2s"
> +   - "allwinner,sun6i-a31-i2s" for controller with reset lines

That's not only for controllers with reset lines, but for the
controllers found in the A31 (and later). I'd simply drop the last
part of that line.

>  - reg: physical base address of the controller and length of memory mapped
>    region.
>  - interrupts: should contain the I2S interrupt.
> @@ -19,7 +20,13 @@ Required properties:
>     - "mod" : module clock for the I2S controller
>  - #sound-dai-cells : Must be equal to 0
>  
> +Required properties for the following compatibles:
> +		- "allwinner,sun6i-a31-i2s"
> +- resets: phandle to the reset line for this codec
> +- reset-names: Contains the reset signal name "rst"

You don't need reset-names if there's a single reset line.

> +
>  Example:
> +For "allwinner,sun4i-a10-i2s":
>  
>  i2s0: i2s@01c22400 {
>  	#sound-dai-cells = <0>;
> @@ -32,3 +39,19 @@ i2s0: i2s@01c22400 {
>  	       <&dma SUN4I_DMA_NORMAL 3>;
>  	dma-names = "rx", "tx";
>  };
> +
> +For "allwinner,sun6i-a31-i2s":
> +
> +dai: dai@01c22c00 {
> +	#sound-dai-cells = <0>;
> +	compatible = "allwinner,sun6i-a31-i2s";
> +	reg = <0x01c22c00 0x200>;
> +	interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
> +	clocks = <&ccu CLK_BUS_CODEC>, <&ccu CLK_AC_DIG>;
> +	clock-names = "apb", "mod";
> +	resets = <&ccu RST_BUS_CODEC>;
> +	reset-names = "rst";
> +	dmas = <&dma 15>, /* AUDIO_CODEC port */
> +		<&dma 15>; /* AUDIO_CODEC port */
> +	dma-names = "rx", "tx";
> +};

And we already have an example, so there's no need to add a new one
either.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH v2 03/10] dt-bindings: sound: Add new reset compatible for sun4i-i2s

[Icenowy Zheng]

CjIwMTflubQx5pyIMTfml6UgMjI6MDLkuo4gTXlsw6huZSBKb3NzZXJhbmQgPG15bGVuZS5qb3Nz
ZXJhbmRAZnJlZS1lbGVjdHJvbnMuY29tPuWGmemBk++8mgo+Cj4gQWRkIGEgbmV3IGNvbXBhdGli
bGUgZm9yIHN1bjRpLWkycyBkcml2ZXIgdG8gaGFuZGxlIHNvbWUKPiBTb0NzIHRoYXQgaGF2ZSBh
IHJlc2V0IGxpbmUgdGhhdCBtdXN0IGJlIGFzc2VydGVkL2RlYXNzZXJ0ZWQuCj4KPiBUaGlzIG5l
dyBjb21wYXRpYmxlLCAiYWxsd2lubmVyLHN1bjZpLWEzMS1pMnMiLCByZXF1aXJlcyB0d28KPiBw
cm9wZXJ0aWVzOgo+IC0gcmVzZXRzOiBwaGFuZGxlIHRvIHRoZSByZXNldCBsaW5lCj4gLSByZXNl
dC1uYW1lczogdGhlIG5hbWUgb2YgdGhlIHJlc2V0IGxpbmUgKCJyc3QiKS4KPiBFeGNlcHQgdGhl
c2UgZGlmZmVyZW5jZXMsIHRoZSBjb21wYXRpYmxlIGlzIGlkZW50aWNhbCB0byBwcmV2aW91cyBv
bmUKPiB3aGljaCB3aWxsIG5vdCBoYW5kbGUgYSByZXNldCBsaW5lLgoKQnV0IEkgdGhpbmsgdGhl
IElQIGJsb2NrIGlzIGlkZW50aWNhbCwgcmlnaHQ/CgpTaG91bGQgYSBuZXcgY29tcGF0aWJsZSBi
ZSBhZGRlZCBvbmx5IGZvciByZXNldC1saW5lPwoKPgo+IFNpZ25lZC1vZmYtYnk6IE15bMOobmUg
Sm9zc2VyYW5kIDxteWxlbmUuam9zc2VyYW5kQGZyZWUtZWxlY3Ryb25zLmNvbT4KPiAtLS0KPiAu
Li4vZGV2aWNldHJlZS9iaW5kaW5ncy9zb3VuZC9zdW40aS1pMnMudHh0wqDCoMKgwqDCoMKgwqAg
fCAyMyArKysrKysrKysrKysrKysrKysrKysrCj4gMSBmaWxlIGNoYW5nZWQsIDIzIGluc2VydGlv
bnMoKykKPgo+IGRpZmYgLS1naXQgYS9Eb2N1bWVudGF0aW9uL2RldmljZXRyZWUvYmluZGluZ3Mv
c291bmQvc3VuNGktaTJzLnR4dCBiL0RvY3VtZW50YXRpb24vZGV2aWNldHJlZS9iaW5kaW5ncy9z
b3VuZC9zdW40aS1pMnMudHh0Cj4gaW5kZXggN2EyYzA5NDVmZDIyLi5mNjczMjA2ZTMwOWIgMTAw
NjQ0Cj4gLS0tIGEvRG9jdW1lbnRhdGlvbi9kZXZpY2V0cmVlL2JpbmRpbmdzL3NvdW5kL3N1bjRp
LWkycy50eHQKPiArKysgYi9Eb2N1bWVudGF0aW9uL2RldmljZXRyZWUvYmluZGluZ3Mvc291bmQv
c3VuNGktaTJzLnR4dAo+IEBAIC03LDYgKzcsNyBAQCBSZXF1aXJlZCBwcm9wZXJ0aWVzOgo+Cj4g
LSBjb21wYXRpYmxlOiBzaG91bGQgYmUgb25lIG9mIHRoZSBmb2xsb3dpbmc6Cj4gwqDCoMKgIC0g
ImFsbHdpbm5lcixzdW40aS1hMTAtaTJzIgo+ICvCoMKgIC0gImFsbHdpbm5lcixzdW42aS1hMzEt
aTJzIiBmb3IgY29udHJvbGxlciB3aXRoIHJlc2V0IGxpbmVzCj4gLSByZWc6IHBoeXNpY2FsIGJh
c2UgYWRkcmVzcyBvZiB0aGUgY29udHJvbGxlciBhbmQgbGVuZ3RoIG9mIG1lbW9yeSBtYXBwZWQK
PiDCoMKgIHJlZ2lvbi4KPiAtIGludGVycnVwdHM6IHNob3VsZCBjb250YWluIHRoZSBJMlMgaW50
ZXJydXB0Lgo+IEBAIC0xOSw3ICsyMCwxMyBAQCBSZXF1aXJlZCBwcm9wZXJ0aWVzOgo+IMKgwqDC
oCAtICJtb2QiIDogbW9kdWxlIGNsb2NrIGZvciB0aGUgSTJTIGNvbnRyb2xsZXIKPiAtICNzb3Vu
ZC1kYWktY2VsbHMgOiBNdXN0IGJlIGVxdWFsIHRvIDAKPgo+ICtSZXF1aXJlZCBwcm9wZXJ0aWVz
IGZvciB0aGUgZm9sbG93aW5nIGNvbXBhdGlibGVzOgo+ICsgLSAiYWxsd2lubmVyLHN1bjZpLWEz
MS1pMnMiCj4gKy0gcmVzZXRzOiBwaGFuZGxlIHRvIHRoZSByZXNldCBsaW5lIGZvciB0aGlzIGNv
ZGVjCj4gKy0gcmVzZXQtbmFtZXM6IENvbnRhaW5zIHRoZSByZXNldCBzaWduYWwgbmFtZSAicnN0
Igo+ICsKPiBFeGFtcGxlOgo+ICtGb3IgImFsbHdpbm5lcixzdW40aS1hMTAtaTJzIjoKPgo+IGky
czA6IGkyc0AwMWMyMjQwMCB7Cj4gI3NvdW5kLWRhaS1jZWxscyA9IDwwPjsKPiBAQCAtMzIsMyAr
MzksMTkgQEAgaTJzMDogaTJzQDAxYzIyNDAwIHsKPiDCoMKgwqDCoMKgwqAgPCZkbWEgU1VONElf
RE1BX05PUk1BTCAzPjsKPiBkbWEtbmFtZXMgPSAicngiLCAidHgiOwo+IH07Cj4gKwo+ICtGb3Ig
ImFsbHdpbm5lcixzdW42aS1hMzEtaTJzIjoKPiArCj4gK2RhaTogZGFpQDAxYzIyYzAwIHsKPiAr
ICNzb3VuZC1kYWktY2VsbHMgPSA8MD47Cj4gKyBjb21wYXRpYmxlID0gImFsbHdpbm5lcixzdW42
aS1hMzEtaTJzIjsKPiArIHJlZyA9IDwweDAxYzIyYzAwIDB4MjAwPjsKPiArIGludGVycnVwdHMg
PSA8R0lDX1NQSSAyOSBJUlFfVFlQRV9MRVZFTF9ISUdIPjsKPiArIGNsb2NrcyA9IDwmY2N1IENM
S19CVVNfQ09ERUM+LCA8JmNjdSBDTEtfQUNfRElHPjsKPiArIGNsb2NrLW5hbWVzID0gImFwYiIs
ICJtb2QiOwo+ICsgcmVzZXRzID0gPCZjY3UgUlNUX0JVU19DT0RFQz47Cj4gKyByZXNldC1uYW1l
cyA9ICJyc3QiOwo+ICsgZG1hcyA9IDwmZG1hIDE1PiwgLyogQVVESU9fQ09ERUMgcG9ydCAqLwo+
ICsgPCZkbWEgMTU+OyAvKiBBVURJT19DT0RFQyBwb3J0ICovCj4gKyBkbWEtbmFtZXMgPSAicngi
LCAidHgiOwo+ICt9Owo+IC0tIAo+IDIuMTEuMAo+Cj4gX19fX19fX19fX19fX19fX19fX19fX19f
X19fX19fX19fX19fX19fX19fX19fX18KPiBsaW51eC1hcm0ta2VybmVsIG1haWxpbmcgbGlzdAo+
IGxpbnV4LWFybS1rZXJuZWxAbGlzdHMuaW5mcmFkZWFkLm9yZwo+IGh0dHA6Ly9saXN0cy5pbmZy
YWRlYWQub3JnL21haWxtYW4vbGlzdGluZm8vbGludXgtYXJtLWtlcm5lbAo=

Re: [PATCH v2 03/10] dt-bindings: sound: Add new reset compatible for sun4i-i2s

[Thomas Petazzoni]

Hello,

On Wed, 18 Jan 2017 01:10:00 +0800, Icenowy Zheng wrote:

> > Add a new compatible for sun4i-i2s driver to handle some
> > SoCs that have a reset line that must be asserted/deasserted.
> >
> > This new compatible, "allwinner,sun6i-a31-i2s", requires two
> > properties:
> > - resets: phandle to the reset line
> > - reset-names: the name of the reset line ("rst").
> > Except these differences, the compatible is identical to previous one
> > which will not handle a reset line.  
> 
> But I think the IP block is identical, right?
> 
> Should a new compatible be added only for reset-line?

Having a different compatible in this case allows to make some stricter
error checking: the driver can make sure that if the compatible string
is sun6i-a31-i2s there *IS* a reset line specified in the DT. Without a
separate compatible value, such a check is not possible, and the reset
line is just optional. This can lead to people being confused if they
forget to specify the reset line.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH v2 03/10] dt-bindings: sound: Add new reset compatible for sun4i-i2s

[Mylene Josserand]

Hi,

On 17/01/2017 17:46, Maxime Ripard wrote:
> On Tue, Jan 17, 2017 at 03:02:23PM +0100, Mylène Josserand wrote:
>> Add a new compatible for sun4i-i2s driver to handle some
>> SoCs that have a reset line that must be asserted/deasserted.
>>
>> This new compatible, "allwinner,sun6i-a31-i2s", requires two
>> properties:
>> 	- resets: phandle to the reset line
>> 	- reset-names: the name of the reset line ("rst").
>> Except these differences, the compatible is identical to previous one
>> which will not handle a reset line.
>>
>> Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
>> ---
>>  .../devicetree/bindings/sound/sun4i-i2s.txt        | 23 ++++++++++++++++++++++
>>  1 file changed, 23 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
>> index 7a2c0945fd22..f673206e309b 100644
>> --- a/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
>> +++ b/Documentation/devicetree/bindings/sound/sun4i-i2s.txt
>> @@ -7,6 +7,7 @@ Required properties:
>>
>>  - compatible: should be one of the following:
>>     - "allwinner,sun4i-a10-i2s"
>> +   - "allwinner,sun6i-a31-i2s" for controller with reset lines
>
> That's not only for controllers with reset lines, but for the
> controllers found in the A31 (and later). I'd simply drop the last
> part of that line.

ACK

>
>>  - reg: physical base address of the controller and length of memory mapped
>>    region.
>>  - interrupts: should contain the I2S interrupt.
>> @@ -19,7 +20,13 @@ Required properties:
>>     - "mod" : module clock for the I2S controller
>>  - #sound-dai-cells : Must be equal to 0
>>
>> +Required properties for the following compatibles:
>> +		- "allwinner,sun6i-a31-i2s"
>> +- resets: phandle to the reset line for this codec
>> +- reset-names: Contains the reset signal name "rst"
>
> You don't need reset-names if there's a single reset line.
>

ACK

>> +
>>  Example:
>> +For "allwinner,sun4i-a10-i2s":
>>
>>  i2s0: i2s@01c22400 {
>>  	#sound-dai-cells = <0>;
>> @@ -32,3 +39,19 @@ i2s0: i2s@01c22400 {
>>  	       <&dma SUN4I_DMA_NORMAL 3>;
>>  	dma-names = "rx", "tx";
>>  };
>> +
>> +For "allwinner,sun6i-a31-i2s":
>> +
>> +dai: dai@01c22c00 {
>> +	#sound-dai-cells = <0>;
>> +	compatible = "allwinner,sun6i-a31-i2s";
>> +	reg = <0x01c22c00 0x200>;
>> +	interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
>> +	clocks = <&ccu CLK_BUS_CODEC>, <&ccu CLK_AC_DIG>;
>> +	clock-names = "apb", "mod";
>> +	resets = <&ccu RST_BUS_CODEC>;
>> +	reset-names = "rst";
>> +	dmas = <&dma 15>, /* AUDIO_CODEC port */
>> +		<&dma 15>; /* AUDIO_CODEC port */
>> +	dma-names = "rx", "tx";
>> +};
>
> And we already have an example, so there's no need to add a new one
> either.

I will remove it on a V3.

Thank you the review!

-- 
Mylène Josserand, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com