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

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

[PATCH v2 00/10] Add sun8i A33 audio driver

[Mylène Josserand]

Hello everyone,

This a V2 of my Allwinner A33 (sun8i) audio codec driver.
Tested on linux-next-20170105 tag with one remaining patch from Chen-Yu's
series:
http://lkml.iu.edu/hypermail/linux/kernel/1611.3/01256.html
Patch to apply before this serie:
https://patchwork.kernel.org/patch/9423999/

Changes since V1:
	- Remove the analog codec driver as a better version has been
	committed by Chen-Yu Tsai and is already merged.
	- Remove the audio-card as simple-card can be used
	- The DMA maxburst is set to 8 in the sun4i-i2s instead of
	adding the maxburst of 4 in Sun6i dma engine.
	- Create a new compatible for sun4i-i2s to handle the reset
	line.
	- Fix various problems in sun8i-codec driver according to V1's
	reviews
	- Add the pm_runtime hooks in sun8i-codec driver to prepare/
	unprepare clocks.
	- Update the DTS according to Chen-Yu's analog codec driver.
	- Rename sun8i-codec's clocks to "bus" and "mod"
	- The first "delay" issue from V1 is fixed by using a delay
	when enabling the headphone amplifier to let the amplifier
	being up.

Patch 1 increases the DMA maxburst value of sun4i-i2s to 8.
See Chen-Yu's commit log for more details:
https://patchwork.kernel.org/patch/9411805/

Patch 2 adds the CLK_SET_RATE_PARENT flag to ccu sun8i-a33 clocks.

Patches 3 and 4 add a new compatible "allwinner,sun6i-a31-i2s"
to handle the reset line for sun4i-i2s driver. It uses a quirk to
use a version with or without reset lines.

Patch 5 adds the sun8i codec driver which represents the digital part
of the A33 codec. It supports only playback features.

Path 6 fixes the previous issue of a "first time delay" in V1 (see cover
letter). Do not hesitate if you have comments on this patch.

Patches 7 adds the dt-bindings documentation for new audio driver
added in this serie (sun8i-codec).

Patch 8 adds the cpu DAI, codec and audio nodes to sun8i-a33 device tree.

Patches 9 and 10 enable the audio on Parrot and Sinlinx's boards.

The DAI for this A33 codec is the same than for A20: "sun4i-i2s".
Currently, all the drivers handle only the playback feature.
The other ones (such as capture) and all other interfaces except
headphone are not supported. I will send a patch to handle the
capture with microphones in next few weeks.

Examples of amixer commands:
	amixer set 'Headphone' 75%
	amixer set 'Headphone' on
	amixer set 'DAC' on
	amixer set 'Right DAC Mixer RSlot 0' on
	amixer set 'Left DAC Mixer LSlot 0' on

It was tested on Parrot and Sinlinx boards.

Let me know if you have any comments on this serie.

Thank you in advance,
Best regards,

Mylène Josserand (10):
  ASoC: sun4i-i2s: Increase DMA max burst to 8
  clk: ccu-sun8i-a33: Add CLK_SET_RATE_PARENT to ac-dig
  dt-bindings: sound: Add new reset compatible for sun4i-i2s
  ASoC: sun4i-i2s: Add quirks to handle new compatible for reset
  ASoC: Add sun8i digital audio codec
  ASoC: sun8i-codec-analog: Add amplifier event to fix first delay
  dt-bindings: sound: Add sun8i audio documentation
  ARM: dts: sun8i: Add audio codec, dai and card for A33
  ARM: dts: sun8i: parrot: Enable audio nodes
  ARM: dts: sun8i: sinlinx: Enable audio nodes

 .../devicetree/bindings/sound/sun4i-i2s.txt        |  23 +
 .../devicetree/bindings/sound/sun8i-codec.txt      |  76 +++
 arch/arm/boot/dts/sun8i-a33-sinlinx-sina33.dts     |   8 +
 arch/arm/boot/dts/sun8i-a33.dtsi                   |  47 ++
 arch/arm/boot/dts/sun8i-r16-parrot.dts             |   8 +
 drivers/clk/sunxi-ng/ccu-sun8i-a33.c               |   2 +-
 sound/soc/sunxi/Kconfig                            |  11 +
 sound/soc/sunxi/Makefile                           |   1 +
 sound/soc/sunxi/sun4i-i2s.c                        |  61 ++-
 sound/soc/sunxi/sun8i-codec-analog.c               |  30 +-
 sound/soc/sunxi/sun8i-codec.c                      | 531 +++++++++++++++++++++
 11 files changed, 791 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/sun8i-codec.txt
 create mode 100644 sound/soc/sunxi/sun8i-codec.c

-- 
2.11.0

[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

[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