Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1495322 > unrolled thread

[PATCH 00/14] ASoc: sunxi: Add Allwinner A33 codec driver

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/14] ASoc: sunxi: Add Allwinner A33 codec driver Mylène Josserand          <mylene.josserand@free-electrons.com> - 2016-10-04 11:50 +0200
    [PATCH 08/14] dt-bindings: sound: Add sun8i analog codec documentation Mylène Josserand          <mylene.josserand@free-electrons.com> - 2016-10-04 11:50 +0200
      Re: [PATCH 08/14] dt-bindings: sound: Add sun8i analog codec  documentation Mark Brown <broonie@kernel.org> - 2016-10-04 12:40 +0200
      Re: [PATCH 08/14] dt-bindings: sound: Add sun8i analog codec  documentation Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-10-04 18:30 +0200
        Re: [PATCH 08/14] dt-bindings: sound: Add sun8i analog codec documentation Chen-Yu Tsai <wens@csie.org> - 2016-10-05 05:10 +0200
          Re: [PATCH 08/14] dt-bindings: sound: Add sun8i analog codec  documentation Mylene Josserand <mylene.josserand@free-electrons.com> - 2016-10-05 14:10 +0200
    [PATCH 11/14] ARM: dts: sun8i: Add analog codec on prcm node Mylène Josserand          <mylene.josserand@free-electrons.com> - 2016-10-04 11:50 +0200
    [PATCH 01/14] dma: sun6i-dma: Add burst case of 4 Mylène Josserand          <mylene.josserand@free-electrons.com> - 2016-10-04 11:50 +0200
      Re: [PATCH 01/14] dma: sun6i-dma: Add burst case of 4 Jean-Francois Moine <moinejf@free.fr> - 2016-10-04 12:50 +0200
        Re: [PATCH 01/14] dma: sun6i-dma: Add burst case of 4 Thomas Petazzoni <thomas.petazzoni@free-electrons.com> - 2016-10-04 14:20 +0200
          Re: [PATCH 01/14] dma: sun6i-dma: Add burst case of 4 Jean-Francois Moine <moinejf@free.fr> - 2016-10-04 15:50 +0200
            Re: [PATCH 01/14] dma: sun6i-dma: Add burst case of 4 Vinod Koul <vinod.koul@intel.com> - 2016-10-04 17:40 +0200
        Re: [PATCH 01/14] dma: sun6i-dma: Add burst case of 4 Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-10-04 19:00 +0200
    [PATCH 13/14] ARM: dts: sun8i: parrot: Enable audio nodes Mylène Josserand          <mylene.josserand@free-electrons.com> - 2016-10-04 11:50 +0200
    [PATCH 10/14] dt-bindings: sound: Add sun8i audio card documentation Mylène Josserand          <mylene.josserand@free-electrons.com> - 2016-10-04 11:50 +0200
      Re: [PATCH 10/14] dt-bindings: sound: Add sun8i audio card  documentation Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-10-04 18:40 +0200
    [PATCH 12/14] ARM: dts: sun8i: Add audio codec, dai and card for A33 Mylène Josserand          <mylene.josserand@free-electrons.com> - 2016-10-04 11:50 +0200
    [PATCH 03/14] ASoC: sun4i-i2s: Add apb reset Mylène Josserand          <mylene.josserand@free-electrons.com> - 2016-10-04 11:50 +0200
      Re: [PATCH 03/14] ASoC: sun4i-i2s: Add apb reset Thomas Petazzoni <thomas.petazzoni@free-electrons.com> - 2016-10-04 14:20 +0200
        Re: [PATCH 03/14] ASoC: sun4i-i2s: Add apb reset Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-10-04 17:50 +0200
          Re: [PATCH 03/14] ASoC: sun4i-i2s: Add apb reset Mylene Josserand <mylene.josserand@free-electrons.com> - 2016-10-05 11:50 +0200
      Re: [PATCH 03/14] ASoC: sun4i-i2s: Add apb reset Code Kipper <codekipper@gmail.com> - 2016-10-04 14:30 +0200
    [PATCH 02/14] clk: ccu-sun8i-a33: Add CLK_SET_RATE_PARENT to ac-dig Mylène Josserand          <mylene.josserand@free-electrons.com> - 2016-10-04 12:00 +0200
      Re: [PATCH 02/14] clk: ccu-sun8i-a33: Add CLK_SET_RATE_PARENT to  ac-dig Thomas Petazzoni <thomas.petazzoni@free-electrons.com> - 2016-10-04 14:20 +0200
        Re: [PATCH 02/14] clk: ccu-sun8i-a33: Add CLK_SET_RATE_PARENT to  ac-dig Mylene Josserand <mylene.josserand@free-electrons.com> - 2016-10-05 11:40 +0200
    [PATCH 05/14] mfd: sun6i-prcm: Add sun8i analog codec as subnode Mylène Josserand          <mylene.josserand@free-electrons.com> - 2016-10-04 12:00 +0200
      Re: [PATCH 05/14] mfd: sun6i-prcm: Add sun8i analog codec as  subnode Jean-Francois Moine <moinejf@free.fr> - 2016-10-04 13:00 +0200
    [PATCH 04/14] ASoC: Add sun8i analog codec driver Mylène Josserand          <mylene.josserand@free-electrons.com> - 2016-10-04 12:00 +0200
      Re: [PATCH 04/14] ASoC: Add sun8i analog codec driver Code Kipper <codekipper@gmail.com> - 2016-10-04 12:30 +0200
        Re: [PATCH 04/14] ASoC: Add sun8i analog codec driver Chen-Yu Tsai <wens@csie.org> - 2016-10-04 13:00 +0200
    [PATCH 07/14] ASoC: Add sun8i audio card Mylène Josserand          <mylene.josserand@free-electrons.com> - 2016-10-04 12:00 +0200
      Re: [PATCH 07/14] ASoC: Add sun8i audio card Code Kipper <codekipper@gmail.com> - 2016-10-04 12:20 +0200
        Re: [PATCH 07/14] ASoC: Add sun8i audio card Chen-Yu Tsai <wens@csie.org> - 2016-10-04 13:10 +0200
      Re: [PATCH 07/14] ASoC: Add sun8i audio card Thomas Petazzoni <thomas.petazzoni@free-electrons.com> - 2016-10-04 14:30 +0200
      Re: [PATCH 07/14] ASoC: Add sun8i audio card Code Kipper <codekipper@gmail.com> - 2016-10-05 08:10 +0200
        Re: [PATCH 07/14] ASoC: Add sun8i audio card Jean-Francois Moine <moinejf@free.fr> - 2016-10-05 12:10 +0200
    [PATCH 06/14] ASoC: Add sun8i digital audio codec Mylène Josserand          <mylene.josserand@free-electrons.com> - 2016-10-04 12:00 +0200
      Re: [PATCH 06/14] ASoC: Add sun8i digital audio codec Thomas Petazzoni <thomas.petazzoni@free-electrons.com> - 2016-10-04 14:50 +0200
        Re: [PATCH 06/14] ASoC: Add sun8i digital audio codec Mark Brown <broonie@kernel.org> - 2016-10-04 15:10 +0200
          Re: [PATCH 06/14] ASoC: Add sun8i digital audio codec Thomas Petazzoni <thomas.petazzoni@free-electrons.com> - 2016-10-04 15:20 +0200
        Re: [PATCH 06/14] ASoC: Add sun8i digital audio codec Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-10-04 18:20 +0200
        Re: [PATCH 06/14] ASoC: Add sun8i digital audio codec Mylene Josserand <mylene.josserand@free-electrons.com> - 2016-10-05 14:00 +0200
      Re: [PATCH 06/14] ASoC: Add sun8i digital audio codec Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-10-04 18:20 +0200
        Re: [PATCH 06/14] ASoC: Add sun8i digital audio codec Alexandre Belloni <alexandre.belloni@free-electrons.com> - 2016-10-06 20:30 +0200
    [PATCH 09/14] dt-bindings: sound: Add sun8i codec documentation Mylène Josserand          <mylene.josserand@free-electrons.com> - 2016-10-04 12:00 +0200
      Re: [PATCH 09/14] dt-bindings: sound: Add sun8i codec documentation Maxime Ripard <maxime.ripard@free-electrons.com> - 2016-10-04 18:30 +0200

Page 1 of 3  [1] 2 3  Next page →

#1495322 — [PATCH 00/14] ASoc: sunxi: Add Allwinner A33 codec driver

Hi everyone,


This patchset add the audio codec for Allwinner A33 (sun8i) SoC.

It adds different drivers:
   - sun8i-codec-analog (patch 4): This driver implements the analog part
   of the audio codec. The analog part is handled in PRCM registers so this
   driver must be added as prcm's subnode (patch 5).
   - sun8i-codec (patch 6): This driver implements the digital part of the
   A33 codec.
   - sun8i (patch 7): This driver implements a sound card for A33.
   It links the DAI and the audio codec. The analog codec driver is handled
   as an "aux_device".

The DAI for this codec is the same than for A20: "sun4i-i2s".
The digital codec code is coming from Allwinner's BSP (after some
cleanup and DAPM conversion) [1]
The analog codec driver is coming from Chen-Yu Tsai's driver [2]
with some modifications (such as read/write regmap functions).

Currently, all the drivers handle only the playback feature.
The other ones (such as capture) and all other interfaces except
headphone are not supported.

These drivers are functional except for one issue. When playing a sound
for the first time, a short delay can be noticed. On a second play
(right after), the sound is played correctly. If we wait a short time
(~5 sec), the delay is back.
There is the same behavior for left/right channel. On the first
time, a left sound is played on the left channel but in the second
time, the sound will be played on wrong channel.

These issues will be fixed in a second time. Is someone have suggestions
about it?

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
	amixer set 'DAC Reversed Right' on
	amixer set 'DAC Reversed Left' on

It was tested on Parrot and Sinlinx board where device tree's modifications
are added (patch 11 to 14).

Thank you in advance,
Best regards,

[1]: https://github.com/allwinner-zh/linux-3.4-sunxi/blob/master/sound/soc/sunxi/audiocodec/sun8iw5_sndcodec.c
[2]: https://github.com/wens/linux/tree/a31-audio

Mylène Josserand (14):
  dma: sun6i-dma: Add burst case of 4
  clk: ccu-sun8i-a33: Add CLK_SET_RATE_PARENT to ac-dig
  ASoC: sun4i-i2s: Add apb reset
  ASoC: Add sun8i analog codec driver
  mfd: sun6i-prcm: Add sun8i analog codec as subnode
  ASoC: Add sun8i digital audio codec
  ASoC: Add sun8i audio card
  dt-bindings: sound: Add sun8i analog codec documentation
  dt-bindings: sound: Add sun8i codec documentation
  dt-bindings: sound: Add sun8i audio card documentation
  ARM: dts: sun8i: Add analog codec on prcm node
  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/sun8i-audio.txt      |  17 +
 .../bindings/sound/sun8i-codec-analog.txt          |  20 +
 .../devicetree/bindings/sound/sun8i-codec.txt      |  24 +
 arch/arm/boot/dts/sun8i-a23-a33.dtsi               |   7 +
 arch/arm/boot/dts/sun8i-a33-sinlinx-sina33.dts     |   8 +
 arch/arm/boot/dts/sun8i-a33.dtsi                   |  33 ++
 arch/arm/boot/dts/sun8i-r16-parrot.dts             |   8 +
 drivers/clk/sunxi-ng/ccu-sun8i-a33.c               |   2 +-
 drivers/dma/sun6i-dma.c                            |   2 +
 drivers/mfd/sun6i-prcm.c                           |  16 +
 sound/soc/sunxi/Kconfig                            |  30 ++
 sound/soc/sunxi/Makefile                           |   3 +
 sound/soc/sunxi/sun4i-i2s.c                        |  16 +-
 sound/soc/sunxi/sun8i-codec-analog.c               | 305 +++++++++++++
 sound/soc/sunxi/sun8i-codec.c                      | 492 +++++++++++++++++++++
 sound/soc/sunxi/sun8i.c                            | 101 +++++
 16 files changed, 1082 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/sun8i-audio.txt
 create mode 100644 Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt
 create mode 100644 Documentation/devicetree/bindings/sound/sun8i-codec.txt
 create mode 100644 sound/soc/sunxi/sun8i-codec-analog.c
 create mode 100644 sound/soc/sunxi/sun8i-codec.c
 create mode 100644 sound/soc/sunxi/sun8i.c

-- 
2.9.3

[toc] | [next] | [standalone]

#1495323 — [PATCH 08/14] dt-bindings: sound: Add sun8i analog codec documentation

Add the documentation for dt-binding of the analog audiocodec
driver for SUN8I SoC.

Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
---
 .../devicetree/bindings/sound/sun8i-codec-analog.txt | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt

diff --git a/Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt b/Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt
new file mode 100644
index 0000000..a03ec20
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt
@@ -0,0 +1,20 @@
+* Allwinner A23/A33 Analog Codec
+
+This codec must be handled as a PRCM subnode.
+
+Required properties:
+- compatible: must be either "allwinner,sun8i-codec-analog"
+- interrupts: must contain the codec interrupt
+- clocks: a list of phandle + clock-specifer pairs, one for each entry
+  in clock-names.
+- clock-names: should contain followings:
+   - "apb": the parent APB clock for this controller
+   - "codec": the parent module clock
+
+Example, in your prcm subnode:
+codec_analog: codec_analog {
+	compatible = "allwinner,sun8i-codec-analog";
+	interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
+	clocks = <&ccu CLK_BUS_CODEC>, <&ccu CLK_AC_DIG>;
+	clock-names = "apb", "codec";
+};
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1495351 — Re: [PATCH 08/14] dt-bindings: sound: Add sun8i analog codec documentation

[Multipart message — attachments visible in raw view] — view raw

On Tue, Oct 04, 2016 at 11:46:21AM +0200, Mylène Josserand wrote:

> +* Allwinner A23/A33 Analog Codec
> +
> +This codec must be handled as a PRCM subnode.

What does this mean - how does one handle something as a "PRCM subnode"?

Please use subject lines matching the style for the subsystem.  This
makes it easier for people to identify relevant patches.

[toc] | [prev] | [next] | [standalone]

#1495504 — Re: [PATCH 08/14] dt-bindings: sound: Add sun8i analog codec documentation

[Multipart message — attachments visible in raw view] — view raw

Hi,

On Tue, Oct 04, 2016 at 11:46:21AM +0200, Mylène Josserand wrote:
> Add the documentation for dt-binding of the analog audiocodec
> driver for SUN8I SoC.
> 
> Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
> ---
>  .../devicetree/bindings/sound/sun8i-codec-analog.txt | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt
> 
> diff --git a/Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt b/Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt
> new file mode 100644
> index 0000000..a03ec20
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt
> @@ -0,0 +1,20 @@
> +* Allwinner A23/A33 Analog Codec
> +
> +This codec must be handled as a PRCM subnode.

Like Mark was saying, you should probably reference the sun6i-prcm.txt
binding here

> +Required properties:
> +- compatible: must be either "allwinner,sun8i-codec-analog"

Our compatible prefix is <family>-<soc>, and using the older SoC that
introduced that block.

In this case, that would be sun6i-a31, I think?

Thanks,
Maxime

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

[toc] | [prev] | [next] | [standalone]

#1495678 — Re: [PATCH 08/14] dt-bindings: sound: Add sun8i analog codec documentation

On Wed, Oct 5, 2016 at 12:24 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi,
>
> On Tue, Oct 04, 2016 at 11:46:21AM +0200, Mylène Josserand wrote:
>> Add the documentation for dt-binding of the analog audiocodec
>> driver for SUN8I SoC.
>>
>> Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
>> ---
>>  .../devicetree/bindings/sound/sun8i-codec-analog.txt | 20 ++++++++++++++++++++
>>  1 file changed, 20 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt
>>
>> diff --git a/Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt b/Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt
>> new file mode 100644
>> index 0000000..a03ec20
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt
>> @@ -0,0 +1,20 @@
>> +* Allwinner A23/A33 Analog Codec
>> +
>> +This codec must be handled as a PRCM subnode.
>
> Like Mark was saying, you should probably reference the sun6i-prcm.txt
> binding here
>
>> +Required properties:
>> +- compatible: must be either "allwinner,sun8i-codec-analog"
>
> Our compatible prefix is <family>-<soc>, and using the older SoC that
> introduced that block.
>
> In this case, that would be sun6i-a31, I think?

sun6i-a31s actually, but a31s has extra line out controls,
so the right one would be sun8i-a23. Both are listed in my
original driver.

ChenYu

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

[toc] | [prev] | [next] | [standalone]

#1495835 — Re: [PATCH 08/14] dt-bindings: sound: Add sun8i analog codec documentation

Hello,


On 05/10/2016 04:59, Chen-Yu Tsai wrote:
> On Wed, Oct 5, 2016 at 12:24 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
>> Hi,
>>
>> On Tue, Oct 04, 2016 at 11:46:21AM +0200, Mylène Josserand wrote:
>>> Add the documentation for dt-binding of the analog audiocodec
>>> driver for SUN8I SoC.
>>>
>>> Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
>>> ---
>>>  .../devicetree/bindings/sound/sun8i-codec-analog.txt | 20 ++++++++++++++++++++
>>>  1 file changed, 20 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt b/Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt
>>> new file mode 100644
>>> index 0000000..a03ec20
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/sound/sun8i-codec-analog.txt
>>> @@ -0,0 +1,20 @@
>>> +* Allwinner A23/A33 Analog Codec
>>> +
>>> +This codec must be handled as a PRCM subnode.
>>
>> Like Mark was saying, you should probably reference the sun6i-prcm.txt
>> binding here


Okay, I will explain more how it works.


>>
>>> +Required properties:
>>> +- compatible: must be either "allwinner,sun8i-codec-analog"
>>
>> Our compatible prefix is <family>-<soc>, and using the older SoC that
>> introduced that block.
>>
>> In this case, that would be sun6i-a31, I think?
>
> sun6i-a31s actually, but a31s has extra line out controls,
> so the right one would be sun8i-a23. Both are listed in my
> original driver.


It is noted.

Thanks!


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

[toc] | [prev] | [next] | [standalone]

#1495325 — [PATCH 11/14] ARM: dts: sun8i: Add analog codec on prcm node

The analog codec for sun8i used PRCM registers so it should be added
in the device tree as a subnode of the prcm entry.

Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
---
 arch/arm/boot/dts/sun8i-a23-a33.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a23-a33.dtsi b/arch/arm/boot/dts/sun8i-a23-a33.dtsi
index 48fc24f..5197812 100644
--- a/arch/arm/boot/dts/sun8i-a23-a33.dtsi
+++ b/arch/arm/boot/dts/sun8i-a23-a33.dtsi
@@ -549,6 +549,13 @@
 				compatible = "allwinner,sun6i-a31-clock-reset";
 				#reset-cells = <1>;
 			};
+
+			codec_analog: codec_analog {
+				compatible = "allwinner,sun8i-codec-analog";
+				interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&ccu CLK_BUS_CODEC>, <&ccu CLK_AC_DIG>;
+				clock-names = "apb", "codec";
+			};
 		};
 
 		cpucfg@01f01c00 {
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1495326 — [PATCH 01/14] dma: sun6i-dma: Add burst case of 4

Add the case of a burst of 4 which is handled by the SoC.

Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
---
 drivers/dma/sun6i-dma.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 8346199..0485204 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -240,6 +240,8 @@ static inline s8 convert_burst(u32 maxburst)
 	switch (maxburst) {
 	case 1:
 		return 0;
+	case 4:
+		return 1;
 	case 8:
 		return 2;
 	default:
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1495353 — Re: [PATCH 01/14] dma: sun6i-dma: Add burst case of 4

On Tue,  4 Oct 2016 11:46:14 +0200
Mylène Josserand <mylene.josserand@free-electrons.com> wrote:

> Add the case of a burst of 4 which is handled by the SoC.
> 
> Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
> ---
>  drivers/dma/sun6i-dma.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index 8346199..0485204 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -240,6 +240,8 @@ static inline s8 convert_burst(u32 maxburst)
>  	switch (maxburst) {
>  	case 1:
>  		return 0;
> +	case 4:
> +		return 1;
>  	case 8:
>  		return 2;
>  	default:
> -- 
> 2.9.3

This patch has already been rejected by Maxime in the threads
	http://www.spinics.net/lists/dmaengine/msg08610.html
and
	http://www.spinics.net/lists/dmaengine/msg08719.html

I hope you will find the way he wants for this maxburst to be added.

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

[toc] | [prev] | [next] | [standalone]

#1495398 — Re: [PATCH 01/14] dma: sun6i-dma: Add burst case of 4

Hello,

On Tue, 4 Oct 2016 12:40:11 +0200, Jean-Francois Moine wrote:

> > Add the case of a burst of 4 which is handled by the SoC.
> > 
> > Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
> > ---
> >  drivers/dma/sun6i-dma.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> > index 8346199..0485204 100644
> > --- a/drivers/dma/sun6i-dma.c
> > +++ b/drivers/dma/sun6i-dma.c
> > @@ -240,6 +240,8 @@ static inline s8 convert_burst(u32 maxburst)
> >  	switch (maxburst) {
> >  	case 1:
> >  		return 0;
> > +	case 4:
> > +		return 1;
> >  	case 8:
> >  		return 2;
> >  	default:
> > -- 
> > 2.9.3  
> 
> This patch has already been rejected by Maxime in the threads
> 	http://www.spinics.net/lists/dmaengine/msg08610.html
> and
> 	http://www.spinics.net/lists/dmaengine/msg08719.html
> 
> I hope you will find the way he wants for this maxburst to be added.

I was about to reply to Mylene's e-mail, suggesting that she should add
a comment in the code (and maybe in the commit log) to explain why this
addition is needed, and also that even though the schematics say that
value "1" (max burst size of 4 bytes) is reserved, it is in fact
incorrect. The Allwinner BSP code is really using this value, and it's
the value that makes audio work, so we believe the datasheet is simply
incorrect.

We already discussed it with Maxime, so I believe he should agree this
time. But I would suggest to have such details explained in the commit
log and in a comment in the code.

Best regards,

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

[toc] | [prev] | [next] | [standalone]

#1495442 — Re: [PATCH 01/14] dma: sun6i-dma: Add burst case of 4

On Tue, 4 Oct 2016 14:12:21 +0200
Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:

> > > Add the case of a burst of 4 which is handled by the SoC.
> > > 
> > > Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
> > > ---
> > >  drivers/dma/sun6i-dma.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> > > index 8346199..0485204 100644
> > > --- a/drivers/dma/sun6i-dma.c
> > > +++ b/drivers/dma/sun6i-dma.c
> > > @@ -240,6 +240,8 @@ static inline s8 convert_burst(u32 maxburst)
> > >  	switch (maxburst) {
> > >  	case 1:
> > >  		return 0;
> > > +	case 4:
> > > +		return 1;
> > >  	case 8:
> > >  		return 2;
> > >  	default:
> > > -- 
> > > 2.9.3  
> > 
> > This patch has already been rejected by Maxime in the threads
> > 	http://www.spinics.net/lists/dmaengine/msg08610.html
> > and
> > 	http://www.spinics.net/lists/dmaengine/msg08719.html
> > 
> > I hope you will find the way he wants for this maxburst to be added.
> 
> I was about to reply to Mylene's e-mail, suggesting that she should add
> a comment in the code (and maybe in the commit log) to explain why this
> addition is needed, and also that even though the schematics say that
> value "1" (max burst size of 4 bytes) is reserved, it is in fact
> incorrect. The Allwinner BSP code is really using this value, and it's
> the value that makes audio work, so we believe the datasheet is simply
> incorrect.
> 
> We already discussed it with Maxime, so I believe he should agree this
> time. But I would suggest to have such details explained in the commit
> log and in a comment in the code.

Strange. Looking at the datasheets of the A23, A31, A33, A83T and H3
(these are the SoCs using the DMA sun6i), only the H3 can have 4 as the
burst size (the doc is unclear for the A31).

Well, I was submitting for the H3, Mylène is submitting for the A33.
So, what about the A23, A31 and A83T?

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

[toc] | [prev] | [next] | [standalone]

#1495479 — Re: [PATCH 01/14] dma: sun6i-dma: Add burst case of 4

On Tue, Oct 04, 2016 at 03:46:51PM +0200, Jean-Francois Moine wrote:
> On Tue, 4 Oct 2016 14:12:21 +0200
> Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:
> 
> > > > Add the case of a burst of 4 which is handled by the SoC.
> > > > 
> > > > Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
> > > > ---
> > > >  drivers/dma/sun6i-dma.c | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > > 
> > > > diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> > > > index 8346199..0485204 100644
> > > > --- a/drivers/dma/sun6i-dma.c
> > > > +++ b/drivers/dma/sun6i-dma.c
> > > > @@ -240,6 +240,8 @@ static inline s8 convert_burst(u32 maxburst)
> > > >  	switch (maxburst) {
> > > >  	case 1:
> > > >  		return 0;
> > > > +	case 4:
> > > > +		return 1;
> > > >  	case 8:
> > > >  		return 2;
> > > >  	default:
> > > > -- 
> > > > 2.9.3  
> > > 
> > > This patch has already been rejected by Maxime in the threads
> > > 	http://www.spinics.net/lists/dmaengine/msg08610.html
> > > and
> > > 	http://www.spinics.net/lists/dmaengine/msg08719.html
> > > 
> > > I hope you will find the way he wants for this maxburst to be added.
> > 
> > I was about to reply to Mylene's e-mail, suggesting that she should add
> > a comment in the code (and maybe in the commit log) to explain why this
> > addition is needed, and also that even though the schematics say that
> > value "1" (max burst size of 4 bytes) is reserved, it is in fact
> > incorrect. The Allwinner BSP code is really using this value, and it's
> > the value that makes audio work, so we believe the datasheet is simply
> > incorrect.
> > 
> > We already discussed it with Maxime, so I believe he should agree this
> > time. But I would suggest to have such details explained in the commit
> > log and in a comment in the code.
> 
> Strange. Looking at the datasheets of the A23, A31, A33, A83T and H3
> (these are the SoCs using the DMA sun6i), only the H3 can have 4 as the
> burst size (the doc is unclear for the A31).
> 
> Well, I was submitting for the H3, Mylène is submitting for the A33.
> So, what about the A23, A31 and A83T?

Since these are device properties, I feel we should move this to DT. That
way any new controller can have any variation based on the mood of hw
designer that day and we can hopefully cope with it :-)

But yes we would need to set the bursts supported in driver and allow above
for supported bursts only.

-- 
~Vinod

[toc] | [prev] | [next] | [standalone]

#1495527 — Re: [PATCH 01/14] dma: sun6i-dma: Add burst case of 4

[Multipart message — attachments visible in raw view] — view raw

On Tue, Oct 04, 2016 at 12:40:11PM +0200, Jean-Francois Moine wrote:
> On Tue,  4 Oct 2016 11:46:14 +0200
> Mylène Josserand <mylene.josserand@free-electrons.com> wrote:
> 
> > Add the case of a burst of 4 which is handled by the SoC.
> > 
> > Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
> > ---
> >  drivers/dma/sun6i-dma.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> > index 8346199..0485204 100644
> > --- a/drivers/dma/sun6i-dma.c
> > +++ b/drivers/dma/sun6i-dma.c
> > @@ -240,6 +240,8 @@ static inline s8 convert_burst(u32 maxburst)
> >  	switch (maxburst) {
> >  	case 1:
> >  		return 0;
> > +	case 4:
> > +		return 1;
> >  	case 8:
> >  		return 2;
> >  	default:
> > -- 
> > 2.9.3
> 
> This patch has already been rejected by Maxime in the threads
> 	http://www.spinics.net/lists/dmaengine/msg08610.html
> and
> 	http://www.spinics.net/lists/dmaengine/msg08719.html
> 
> I hope you will find the way he wants for this maxburst to be added.

I was talking about something along these lines (not tested):

-------8<---------
diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 83461994e418..573ac4608293 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -240,6 +240,8 @@ static inline s8 convert_burst(u32 maxburst)
 	switch (maxburst) {
 	case 1:
 		return 0;
+	case 4:
+		return 1;
 	case 8:
 		return 2;
 	default:
@@ -1110,11 +1112,19 @@ static int sun6i_dma_probe(struct platform_device *pdev)
 	sdc->slave.dst_addr_widths		= BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
 						  BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
 						  BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
+	sdc->slave.dst_bursts			= BIT(1) | BIT(8);
+	sdc->slave.src_bursts			= BIT(1) | BIT(8);
 	sdc->slave.directions			= BIT(DMA_DEV_TO_MEM) |
 						  BIT(DMA_MEM_TO_DEV);
 	sdc->slave.residue_granularity		= DMA_RESIDUE_GRANULARITY_BURST;
 	sdc->slave.dev = &pdev->dev;
 
+	if (of_device_is_compatible(pdev->dev.of_node,
+				    "allwinner,sun8i-h3-dma")) {
+		sdc->slave.dst_bursts |= BIT(4);
+		sdc->slave.src_bursts |= BIT(4);
+	}
+
 	sdc->pchans = devm_kcalloc(&pdev->dev, sdc->cfg->nr_max_channels,
 				   sizeof(struct sun6i_pchan), GFP_KERNEL);
 	if (!sdc->pchans)
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index cc535a478bae..f7bbec24bb58 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -673,6 +673,8 @@ struct dma_filter {
  * 	each type of direction, the dma controller should fill (1 <<
  * 	<TYPE>) and same should be checked by controller as well
  * @max_burst: max burst capability per-transfer
+ * @dst_bursts: bitfield of the available burst sizes for the destination
+ * @src_bursts: bitfield of the available burst sizes for the source
  * @residue_granularity: granularity of the transfer residue reported
  *	by tx_status
  * @device_alloc_chan_resources: allocate resources and return the
@@ -800,6 +802,14 @@ struct dma_device {
 static inline int dmaengine_slave_config(struct dma_chan *chan,
 					  struct dma_slave_config *config)
 {
+	if (config->src_maxburst && config->device->src_bursts &&
+	    !(BIT(config->src_maxburst) & config->device->src_bursts))
+		return -EINVAL;
+
+	if (config->dst_maxburst && config->device->dst_bursts &&
+	    !(BIT(config->dst_maxburst) & config->device->dst_bursts))
+		return -EINVAL;
+
 	if (chan->device->device_config)
 		return chan->device->device_config(chan, config);
-------8<------------ 

Maxime

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

[toc] | [prev] | [next] | [standalone]

#1495327 — [PATCH 13/14] ARM: dts: sun8i: parrot: Enable audio nodes

Enable the audio codec and the audio dai for the sun8i R16 Parrot board.

Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
---
 arch/arm/boot/dts/sun8i-r16-parrot.dts | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-r16-parrot.dts b/arch/arm/boot/dts/sun8i-r16-parrot.dts
index 47553e5..7becead 100644
--- a/arch/arm/boot/dts/sun8i-r16-parrot.dts
+++ b/arch/arm/boot/dts/sun8i-r16-parrot.dts
@@ -84,6 +84,14 @@
 
 };
 
+&codec {
+	status = "okay";
+};
+
+&dai {
+	status = "okay";
+};
+
 &ehci0 {
 	status = "okay";
 };
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1495328 — [PATCH 10/14] dt-bindings: sound: Add sun8i audio card documentation

Add the documentation for dt-binding of the audio card driver
for sun8i SoC.

Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
---
 Documentation/devicetree/bindings/sound/sun8i-audio.txt | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/sun8i-audio.txt

diff --git a/Documentation/devicetree/bindings/sound/sun8i-audio.txt b/Documentation/devicetree/bindings/sound/sun8i-audio.txt
new file mode 100644
index 0000000..2403983
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/sun8i-audio.txt
@@ -0,0 +1,17 @@
+* Allwinner A23/A33 audio card
+
+This binding implements the A33 audio card.
+
+Required properties:
+- compatible: must be "allwinner,sun8i-audio"
+- allwinner,audio-codec: must have the phandle of the audio codec
+  ("sun8i-a33-codec", for example).
+- allwinner,i2s-controller: must have the phandle of the DAI
+  ("allwinner,sun4i-a10-i2s", for example)
+
+Example:
+sound {
+	compatible = "allwinner,sun8i-audio";
+	allwinner,audio-codec = <&codec>;
+	allwinner,i2s-controller = <&dai>;
+};
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1495515 — Re: [PATCH 10/14] dt-bindings: sound: Add sun8i audio card documentation

[Multipart message — attachments visible in raw view] — view raw

On Tue, Oct 04, 2016 at 11:46:23AM +0200, Mylène Josserand wrote:
> Add the documentation for dt-binding of the audio card driver
> for sun8i SoC.
> 
> Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
> ---
>  Documentation/devicetree/bindings/sound/sun8i-audio.txt | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/sun8i-audio.txt
> 
> diff --git a/Documentation/devicetree/bindings/sound/sun8i-audio.txt b/Documentation/devicetree/bindings/sound/sun8i-audio.txt
> new file mode 100644
> index 0000000..2403983
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/sun8i-audio.txt
> @@ -0,0 +1,17 @@
> +* Allwinner A23/A33 audio card
> +
> +This binding implements the A33 audio card.
> +
> +Required properties:
> +- compatible: must be "allwinner,sun8i-audio"
> +- allwinner,audio-codec: must have the phandle of the audio codec
> +  ("sun8i-a33-codec", for example).
> +- allwinner,i2s-controller: must have the phandle of the DAI
> +  ("allwinner,sun4i-a10-i2s", for example)

You should probably have a link to the PRCM too, instead of relying on
the name of the device in your card, which is quite fragile.

Also, I'm wondering, shouldn't all these nodes be part of a single
MFD? They share the same address space (even though it's split
nicely), the same clocks, and really are just one big device. Chen-Yu,
Mark, any opinion?

Thanks,
Maxime

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

[toc] | [prev] | [next] | [standalone]

#1495329 — [PATCH 12/14] ARM: dts: sun8i: Add audio codec, dai and card for A33

Add the audio codec, dai and a sun8i card to be able to use the
audio stream of the builtin codec on sun8i SoC.

Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
---
 arch/arm/boot/dts/sun8i-a33.dtsi | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a33.dtsi b/arch/arm/boot/dts/sun8i-a33.dtsi
index fd1e1cd..4f8b442 100644
--- a/arch/arm/boot/dts/sun8i-a33.dtsi
+++ b/arch/arm/boot/dts/sun8i-a33.dtsi
@@ -43,6 +43,7 @@
  */
 
 #include "sun8i-a23-a33.dtsi"
+#include <dt-bindings/dma/sun4i-a10.h>
 
 / {
 	cpus {
@@ -69,6 +70,12 @@
 		reg = <0x40000000 0x80000000>;
 	};
 
+	sound {
+		compatible = "allwinner,sun8i-audio";
+		allwinner,audio-codec = <&codec>;
+		allwinner,i2s-controller = <&dai>;
+	};
+
 	soc@01c00000 {
 		tcon0: lcd-controller@01c0c000 {
 			compatible = "allwinner,sun8i-a33-tcon";
@@ -116,6 +123,32 @@
 			reset-names = "ahb";
 		};
 
+		dai: dai@01c22c00 {
+			#sound-dai-cells = <0>;
+			compatible = "allwinner,sun4i-a10-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 = "apb_reset";
+			dmas = <&dma 15>, /* AUDIO_CODEC port */
+				<&dma 15>; /* AUDIO_CODEC port */
+			dma-names = "rx", "tx";
+			status = "disabled";
+		};
+
+		codec: codec@01c22e00 {
+			#sound-dai-cells = <0>;
+			compatible = "allwinner,sun8i-a33-codec";
+			reg = <0x01c22e00 0x400>; /* SUNXI_AUDIO_PBASE + 0x200 */
+			reg-names = "audio";
+			interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_CODEC>, <&ccu CLK_AC_DIG>;
+			clock-names = "apb", "codec";
+			status = "disabled";
+		};
+
 		fe0: display-frontend@01e00000 {
 			compatible = "allwinner,sun8i-a33-display-frontend";
 			reg = <0x01e00000 0x20000>;
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1495330 — [PATCH 03/14] ASoC: sun4i-i2s: Add apb reset

Add APB deassert function for sun4i-i2s driver.

Signed-off-by: Mylène Josserand <mylene.josserand@free-electrons.com>
---
 sound/soc/sunxi/sun4i-i2s.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index 687a8f8..f3f7026 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -17,6 +17,7 @@
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
+#include <linux/reset.h>
 
 #include <sound/dmaengine_pcm.h>
 #include <sound/pcm_params.h>
@@ -589,6 +590,7 @@ static int sun4i_i2s_probe(struct platform_device *pdev)
 {
 	struct sun4i_i2s *i2s;
 	struct resource *res;
+	struct reset_control *reset_apb;
 	void __iomem *regs;
 	int irq, ret;
 
@@ -626,7 +628,19 @@ static int sun4i_i2s_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "Can't get our mod clock\n");
 		return PTR_ERR(i2s->mod_clk);
 	}
-	
+
+	reset_apb = devm_reset_control_get(&pdev->dev, "apb_reset");
+	if (IS_ERR(reset_apb)) {
+		dev_err(&pdev->dev, "Can't get apb reset\n");
+		return PTR_ERR(i2s->mod_clk);
+	}
+
+	ret = reset_control_deassert(reset_apb);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Can't deassert apb reset (%d)\n", ret);
+		return ret;
+	}
+
 	i2s->playback_dma_data.addr = res->start + SUN4I_I2S_FIFO_TX_REG;
 	i2s->playback_dma_data.maxburst = 4;
 
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1495396 — Re: [PATCH 03/14] ASoC: sun4i-i2s: Add apb reset

Hello,

On Tue,  4 Oct 2016 11:46:16 +0200, Mylène Josserand wrote:

>  #include <sound/dmaengine_pcm.h>
>  #include <sound/pcm_params.h>
> @@ -589,6 +590,7 @@ static int sun4i_i2s_probe(struct platform_device *pdev)
>  {
>  	struct sun4i_i2s *i2s;
>  	struct resource *res;
> +	struct reset_control *reset_apb;
>  	void __iomem *regs;
>  	int irq, ret;
>  
> @@ -626,7 +628,19 @@ static int sun4i_i2s_probe(struct platform_device *pdev)
>  		dev_err(&pdev->dev, "Can't get our mod clock\n");
>  		return PTR_ERR(i2s->mod_clk);
>  	}
> -	
> +
> +	reset_apb = devm_reset_control_get(&pdev->dev, "apb_reset");

I believe this is a change in the Device Tree binding, since you're
adding support for a new resource. Perhaps the Device Tree binding
documentation should be updated accordingly?

> +	if (IS_ERR(reset_apb)) {
> +		dev_err(&pdev->dev, "Can't get apb reset\n");
> +		return PTR_ERR(i2s->mod_clk);

This should be:

		return PTR_ERR(reset_apb);

> +	}
> +
> +	ret = reset_control_deassert(reset_apb);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "Can't deassert apb reset (%d)\n", ret);
> +		return ret;
> +	}

Do you need to re-assert the reset line in the ->remove() hook?

Best regards,

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

[toc] | [prev] | [next] | [standalone]

#1495487 — Re: [PATCH 03/14] ASoC: sun4i-i2s: Add apb reset

[Multipart message — attachments visible in raw view] — view raw

Hi,

On Tue, Oct 04, 2016 at 02:15:16PM +0200, Thomas Petazzoni wrote:
> Hello,
> 
> On Tue,  4 Oct 2016 11:46:16 +0200, Mylène Josserand wrote:
> 
> >  #include <sound/dmaengine_pcm.h>
> >  #include <sound/pcm_params.h>
> > @@ -589,6 +590,7 @@ static int sun4i_i2s_probe(struct platform_device *pdev)
> >  {
> >  	struct sun4i_i2s *i2s;
> >  	struct resource *res;
> > +	struct reset_control *reset_apb;
> >  	void __iomem *regs;
> >  	int irq, ret;
> >  
> > @@ -626,7 +628,19 @@ static int sun4i_i2s_probe(struct platform_device *pdev)
> >  		dev_err(&pdev->dev, "Can't get our mod clock\n");
> >  		return PTR_ERR(i2s->mod_clk);
> >  	}
> > -	
> > +
> > +	reset_apb = devm_reset_control_get(&pdev->dev, "apb_reset");
> 
> I believe this is a change in the Device Tree binding, since you're
> adding support for a new resource. Perhaps the Device Tree binding
> documentation should be updated accordingly?

Indeed.

You have two solutions to do that:
  - Either mark it as optional and use reset_control_get_optional
    (because here, you broke the other SoCs that have that controller
    but no reset line)
  - Or introduce a new compatible, and make the reset property
    mandatory for that new compatible.

I prefer the latter, since you get a stricter error check, and you
cannot end up in a situation where your driver probes but is
useless. But you'll find both in our drivers.

> > +	}
> > +
> > +	ret = reset_control_deassert(reset_apb);
> > +	if (ret < 0) {
> > +		dev_err(&pdev->dev, "Can't deassert apb reset (%d)\n", ret);
> > +		return ret;
> > +	}
> 
> Do you need to re-assert the reset line in the ->remove() hook?

Even better, you can add it to the runtime_pm hooks! :)

Maxime

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

[toc] | [prev] | [next] | [standalone]

Page 1 of 3  [1] 2 3  Next page →

Back to top | Article view | linux.kernel

csiph-web