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

[Mylene Josserand <mylene.josserand@free-electrons.com>]

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