[PATCH] arm64: dts: rockchip: more user friendly name of sound nodes

2021-01-10 Thread Katsuhiro Suzuki
This patch changes device name to more user friendly name of
Analog and SPDIF sound nodes for rk3399-rockpro64.

Signed-off-by: Katsuhiro Suzuki 
---
 arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
index 58097245994a..5ab0b9edfc88 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
@@ -72,13 +72,13 @@ sdio_pwrseq: sdio-pwrseq {
 
sound {
compatible = "audio-graph-card";
-   label = "rockchip,rk3399";
+   label = "Analog";
dais = <_p0>;
};
 
sound-dit {
compatible = "audio-graph-card";
-   label = "rockchip,rk3399";
+   label = "SPDIF";
dais = <_p0>;
};
 
-- 
2.29.2



Re: [PATCH v3] arm64: dts: rockchip: add SPDIF node for rk3399-rockpro64

2020-11-08 Thread Katsuhiro Suzuki

Hello Heiko, Johan,

This is ping... How about Johan's question?


Question for the maintainer:
Should we add a SPDIF node if the connector is not physical on a board,
only a header?


If we don't need to add nodes for sound I/F that has only header not
physical on a board, we should drop this patch and also remove I2S0
because I2S0 has no physical connector on a board.

Additionally, I2S1 share the I2S_CLK pin with I2S0. So ROCKPro64 users
perhaps cannot use I2S0 and I2S1 (for ES8316) at same time. If users
try to use I2S1 after I2S0, I2S1 overwrites clock setting and I2S0
will face strange behavior.

I don't know why ROCKPro64 has such strange pin implementation...


For Johan:

Sorry for late reply. Thank you for your review.
And I agree with you renaming sound nodes.

- HDMI
- ES8316 (or "Analog" is more useful for media player SW??)
- SPDIF (if we need)


Best Regards,
Katsuhiro Suzuki


On 2020/10/06 18:51, Johan Jonker wrote:

Hi Katsuhiro, Heiko,

Question for the maintainer:
Should we add a SPDIF node if the connector is not physical on a board,
only a header?

Thanks Katsuhiro for the "aplay -l" screen print.

 List of PLAYBACK Hardware Devices 
card 0: hdmisound [hdmi-sound], device 0: ff8a.i2s-i2s-hifi
i2s-hifi-0 [ff8a.i2s-i2s-hifi i2s-hifi-0]
   Subdevices: 1/1
   Subdevice #0: subdevice #0
card 1: rockchiprk3399 [rockchip,rk3399], device 0: ff89.i2s-ES8316
HiFi ES8316 HiFi-0 [ff89.i2s-ES8316 HiFi ES8316 HiFi-0]
   Subdevices: 1/1
   Subdevice #0: subdevice #0
card 2: rockchiprk339_1 [rockchip,rk3399], device 0:
ff87.spdif-dit-hifi dit-hifi-0 [ff87.spdif-dit-hifi dit-hifi-0]
   Subdevices: 1/1
   Subdevice #0: subdevice #0


On 10/5/20 4:03 PM, Katsuhiro Suzuki wrote:

This patch adds 'disabled' SPDIF sound node and related settings
of SPDIF for rk3399-rockpro64.

RockPro64 has output pins for SPDIF Tx. But RK3399 does not have
enough DMA channel for enabling SPDIF tx. Current settings are:

   - I2S0 (Req num 0, 1): Enabled : Output to 40pins header CON40
   - I2S1 (Req num 2, 3): Enabled : Output to ES8316 on board
   - I2S2 (Req num 4, 5): Enabled : Output to internal HDMI core
   - SPDIF Tx (Req num 7)   : Disabled: Output to connector J10

If users want to enable ALL sound I/Os, we need 7 DMA channels for
it. But unfortunately, RK3399 has only 6 DMA channels. So users have
to choose from the following:

   - Disable one of I2S (Ex. I2S0) and enable SPDIF tx
   - Keep enable I2S0/1/2 and disable SPDIF tx

Signed-off-by: Katsuhiro Suzuki 

---

Changes in v3:
   - Refine commit description why adding disabled node

Changes in v2:
   - Remove redundant status property
---
  .../boot/dts/rockchip/rk3399-rockpro64.dtsi   | 27 +++
  1 file changed, 27 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
index 6e553ff47534..58097245994a 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
@@ -76,6 +76,23 @@ sound {
dais = <_p0>;
};
  



hdmi_sound: hdmi-sound {
compatible = "simple-audio-card";
simple-audio-card,name = "hdmi-sound";

Maybe rename to "HDMI"?

[..]
};

sound {
compatible = "audio-graph-card";
label = "rockchip,rk3399";

Maybe change this to "ES8316" to prevent confusion?

dais = <_p0>;
};



+   sound-dit {
+   compatible = "audio-graph-card"
+   label = "rockchip,rk3399";


This would be the second sound card with the same label.
It seems that aplay/linux? adds "-1" to it and removes the comma, so we get:

hdmisound
rockchiprk3399
rockchiprk339_1

Shouldn't we label it with something that reflect the function/output.
Shouldn't we standardize to SPDIF, HDMI and Analog similar to rk3318/rk3328?
Make a shorter label without spaces or special chars, so that chars
don't get removed?

Proposal:

HDMI
ES8316
SPDIF


+   dais = <_p0>;


Maybe disable too?

The "sound-dit" node is standard enabled and will start some process
with a dia in a node that is disabled.



+   };
+
+   spdif-dit {
+   compatible = "linux,spdif-dit";
+   #sound-dai-cells = <0>;


Maybe disable too?


+
+   port {
+   dit_p0_0: endpoint {



+   remote-endpoint = <_p0_0>;


This also points to something that's disabled.


+   };
+   };
+   };
+
vcc12v_dcin: vcc12v-dcin {
compatible = "regulator-fixed";
regulator-name = "vcc12v_dcin";
@@ -698,6 +715,16 @@  {
status = "okay&q

[PATCH v3] arm64: dts: rockchip: add SPDIF node for rk3399-rockpro64

2020-10-05 Thread Katsuhiro Suzuki
This patch adds 'disabled' SPDIF sound node and related settings
of SPDIF for rk3399-rockpro64.

RockPro64 has output pins for SPDIF Tx. But RK3399 does not have
enough DMA channel for enabling SPDIF tx. Current settings are:

  - I2S0 (Req num 0, 1): Enabled : Output to 40pins header CON40
  - I2S1 (Req num 2, 3): Enabled : Output to ES8316 on board
  - I2S2 (Req num 4, 5): Enabled : Output to internal HDMI core
  - SPDIF Tx (Req num 7)   : Disabled: Output to connector J10

If users want to enable ALL sound I/Os, we need 7 DMA channels for
it. But unfortunately, RK3399 has only 6 DMA channels. So users have
to choose from the following:

  - Disable one of I2S (Ex. I2S0) and enable SPDIF tx
  - Keep enable I2S0/1/2 and disable SPDIF tx

Signed-off-by: Katsuhiro Suzuki 

---

Changes in v3:
  - Refine commit description why adding disabled node

Changes in v2:
  - Remove redundant status property
---
 .../boot/dts/rockchip/rk3399-rockpro64.dtsi   | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
index 6e553ff47534..58097245994a 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
@@ -76,6 +76,23 @@ sound {
dais = <_p0>;
};
 
+   sound-dit {
+   compatible = "audio-graph-card";
+   label = "rockchip,rk3399";
+   dais = <_p0>;
+   };
+
+   spdif-dit {
+   compatible = "linux,spdif-dit";
+   #sound-dai-cells = <0>;
+
+   port {
+   dit_p0_0: endpoint {
+   remote-endpoint = <_p0_0>;
+   };
+   };
+   };
+
vcc12v_dcin: vcc12v-dcin {
compatible = "regulator-fixed";
regulator-name = "vcc12v_dcin";
@@ -698,6 +715,16 @@  {
status = "okay";
 };
 
+ {
+   pinctrl-0 = <_bus_1>;
+
+   spdif_p0: port {
+   spdif_p0_0: endpoint {
+   remote-endpoint = <_p0_0>;
+   };
+   };
+};
+
  {
status = "okay";
 
-- 
2.28.0



Re: [PATCH v2] arm64: dts: rockchip: enable HDMI sound nodes for rk3328-rock64

2020-08-30 Thread Katsuhiro Suzuki

On 2020/08/31 4:16, Heiko Stuebner wrote:

Hi,

Am Sonntag, 2. August 2020, 17:42:31 CEST schrieb Katsuhiro Suzuki:

This patch enables HDMI sound (I2S0) and Analog sound (I2S1) which
are defined in rk3328.dtsi, and replace SPDIF nodes.

We can use SPDIF pass-through with suitable ALSA settings and on
mpv or other media players.
   - Settings: 
https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Rockchip/filesystem/usr/share/alsa/cards/SPDIF.conf
   - Ex.: mpv foo.ac3 --audio-spdif=ac3 
--audio-device='alsa/SPDIF.pcm.iec958.0:SPDIF'

[Why use simple-audio-card for SPDIF?]

For newly adding nodes, ASoC guys recommend to use audio-graph-card.
But all other sound nodes for rk3328 have already been defined by
simple-audio-card. In this time, I chose for consistent sound nodes.

[DMA allocation problem]

After this patch is applied, UART2 will fail to allocate DMA resources
but UART driver can work fine without DMA.

This error is related to the DMAC of rk3328 (pl330 or compatible).
DMAC connected to 16 DMA sources. Each sources have ID number that is
called 'Req number' in rk3328 TRM. After this patch is applied total 7
of DMA sources will be activated as follows:

| Req number | Source | Required  |
||| channels  |
|++---|
|  8,  9 | SPI0   | 2ch   |
| 11, 12 | I2S0   | 2ch   |
| 14, 15 | I2S1   | 2ch   |
| 10 | SPDIF  | 1ch   |
|++---|
|| Total  | 7ch   |
|++---|
|  6,  7 | UART2  | 2ch   | -> cannot get DMA channels

Due to rk3328 DMAC specification we can use max 8 channels at same
time. If SPI0/I2S0/I2S1/SPDIF will be activated by this patch,
required DMAC channels reach to 7. So the last two channels (for
UART2) cannot get DMA resources.


Wouldn't the dma allocation depend on the probe ordering?
Or is this predetermined, so that always uart2 looses its dmas?



Ah, it's depends on probe ordering when users use kernel modules...

It's better to disable DMA channels for UART2 for avoiding problem.
I'll fix and resend patch.



Heiko





Best Regards,
Katsuhiro Suzuki


Re: [PATCH v2] arm64: dts: rockchip: add SPDIF node for rk3399-rockpro64

2020-08-30 Thread Katsuhiro Suzuki

Hello Heiko,

On 2020/08/31 4:08, Heiko Stuebner wrote:

Hi,

Am Montag, 10. August 2020, 11:16:19 CEST schrieb Katsuhiro Suzuki:

This patch adds 'disabled' SPDIF sound node and related settings
for rk3399-rockpro64.

There are 2 reasons:
   - All RK3399 dma-bus channels have been already used by I2S0/1/2
   - RockPro64 does not have SPDIF optical nor coaxial connector,
 just have 3pins


I don't really understand what you mean here.

Like is there spdif on the board or not? Because you call it "disabled"
and also indicate that no i2s is available anymore, yet you mention 3 pins.
What do they do then?



RockPro64 has output pins for SPDIF Tx. But RK3399 does not have enough DMA
channel for enabling SPDIF tx. Current settings are:

  - I2S0 (Req number 0, 1): Enabled  : Output to 40pin headers (CON40)
  - I2S1 (Req number 2, 3): Enabled  : Output to ES8316 on board
  - I2S2 (Req number 4, 5): Enabled  : Output to internal HDMI core
  - SPDIF Tx (Req number 7)   : Disabled : Output to other connector (J10)

If we want to enable ALL sound I/Os, we need 7 DMA channels for it.
But... Unfortunately, RK3399 has only 6 DMA channels for sounds, PWM and SPIs.

So users of RockPro64 have to choose from the following:

  - Disable one of I2S (Ex. I2S0) and enable SPDIF tx
  - Keep enable I2S0/1/2 and disable SPDIF tx


Thanks
Heiko




Best Regards,
Katsuhiro Suzuki



Signed-off-by: Katsuhiro Suzuki 

---

Changes in v2:
   - Remove redundant status property
---
  .../boot/dts/rockchip/rk3399-rockpro64.dtsi   | 27 +++
  1 file changed, 27 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
index 6e553ff47534..58097245994a 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
@@ -76,6 +76,23 @@ sound {
dais = <_p0>;
};
  
+	sound-dit {

+   compatible = "audio-graph-card";
+   label = "rockchip,rk3399";
+   dais = <_p0>;
+   };
+
+   spdif-dit {
+   compatible = "linux,spdif-dit";
+   #sound-dai-cells = <0>;
+
+   port {
+   dit_p0_0: endpoint {
+   remote-endpoint = <_p0_0>;
+   };
+   };
+   };
+
vcc12v_dcin: vcc12v-dcin {
compatible = "regulator-fixed";
regulator-name = "vcc12v_dcin";
@@ -698,6 +715,16 @@  {
status = "okay";
  };
  
+ {

+   pinctrl-0 = <_bus_1>;
+
+   spdif_p0: port {
+   spdif_p0_0: endpoint {
+   remote-endpoint = <_p0_0>;
+   };
+   };
+};
+
   {
status = "okay";
  










[PATCH v2] arm64: dts: rockchip: add SPDIF node for rk3399-rockpro64

2020-08-10 Thread Katsuhiro Suzuki
This patch adds 'disabled' SPDIF sound node and related settings
for rk3399-rockpro64.

There are 2 reasons:
  - All RK3399 dma-bus channels have been already used by I2S0/1/2
  - RockPro64 does not have SPDIF optical nor coaxial connector,
just have 3pins

Signed-off-by: Katsuhiro Suzuki 

---

Changes in v2:
  - Remove redundant status property
---
 .../boot/dts/rockchip/rk3399-rockpro64.dtsi   | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
index 6e553ff47534..58097245994a 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
@@ -76,6 +76,23 @@ sound {
dais = <_p0>;
};
 
+   sound-dit {
+   compatible = "audio-graph-card";
+   label = "rockchip,rk3399";
+   dais = <_p0>;
+   };
+
+   spdif-dit {
+   compatible = "linux,spdif-dit";
+   #sound-dai-cells = <0>;
+
+   port {
+   dit_p0_0: endpoint {
+   remote-endpoint = <_p0_0>;
+   };
+   };
+   };
+
vcc12v_dcin: vcc12v-dcin {
compatible = "regulator-fixed";
regulator-name = "vcc12v_dcin";
@@ -698,6 +715,16 @@  {
status = "okay";
 };
 
+ {
+   pinctrl-0 = <_bus_1>;
+
+   spdif_p0: port {
+   spdif_p0_0: endpoint {
+   remote-endpoint = <_p0_0>;
+   };
+   };
+};
+
  {
status = "okay";
 
-- 
2.28.0



Re: [PATCH] arm64: dts: rockchip: add SPDIF node for rk3399-rockpro64

2020-08-10 Thread Katsuhiro Suzuki

Hello Johan,

Sorry for late and thanks for your comment!

On 2020/08/03 2:52, Johan Jonker wrote:

Hi Katsuhiro,

On 8/1/20 7:49 PM, Katsuhiro Suzuki wrote:

This patch adds 'disabled' SPDIF sound node and related settings
for rk3399-rockpro64.

There are 2 reasons:
   - All RK3399 dma-bus channels have been already used by I2S0/1/2
   - RockPro64 does not have SPDIF optical nor coaxial connector,
 just have 3pins

Signed-off-by: Katsuhiro Suzuki 
---
  .../boot/dts/rockchip/rk3399-rockpro64.dtsi   | 28 +++
  1 file changed, 28 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
index 6e553ff47534..e35b30f8a46e 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
@@ -76,6 +76,23 @@ sound {
dais = <_p0>;
};
  
+	sound-dit {

+   compatible = "audio-graph-card";
+   label = "rockchip,rk3399";
+   dais = <_p0>;
+   };
+
+   spdif-dit {
+   compatible = "linux,spdif-dit";
+   #sound-dai-cells = <0>;
+
+   port {
+   dit_p0_0: endpoint {
+   remote-endpoint = <_p0_0>;
+   };
+   };
+   };
+
vcc12v_dcin: vcc12v-dcin {
compatible = "regulator-fixed";
regulator-name = "vcc12v_dcin";
@@ -698,6 +715,17 @@  {
status = "okay";
  };
  
+ {

+   pinctrl-0 = <_bus_1>;



+   status = "disabled";


The status is already disabled.
Adding more unused nodes and properties doesn't make sense here.

spdif: spdif@ff87 {

compatible = "rockchip,rk3399-spdif";
reg = <0x0 0xff87 0x0 0x1000>;
interrupts = ;
dmas = <_bus 7>;
dma-names = "tx";
clock-names = "mclk", "hclk";
clocks = < SCLK_SPDIF_8CH>, < HCLK_SPDIF>;
pinctrl-names = "default";
pinctrl-0 = <_bus>;
power-domains = < RK3399_PD_SDIOAUDIO>;
#sound-dai-cells = <0>;
status = "disabled";
};



I see, I will remove this line and send V2 patch.

Best Regards,
Katsuhiro Suzuki



+
+   spdif_p0: port {
+   spdif_p0_0: endpoint {
+   remote-endpoint = <_p0_0>;
+   };
+   };
+};
+
   {
status = "okay";
  







[PATCH v2] arm64: dts: rockchip: enable HDMI sound nodes for rk3328-rock64

2020-08-02 Thread Katsuhiro Suzuki
This patch enables HDMI sound (I2S0) and Analog sound (I2S1) which
are defined in rk3328.dtsi, and replace SPDIF nodes.

We can use SPDIF pass-through with suitable ALSA settings and on
mpv or other media players.
  - Settings: 
https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Rockchip/filesystem/usr/share/alsa/cards/SPDIF.conf
  - Ex.: mpv foo.ac3 --audio-spdif=ac3 
--audio-device='alsa/SPDIF.pcm.iec958.0:SPDIF'

[Why use simple-audio-card for SPDIF?]

For newly adding nodes, ASoC guys recommend to use audio-graph-card.
But all other sound nodes for rk3328 have already been defined by
simple-audio-card. In this time, I chose for consistent sound nodes.

[DMA allocation problem]

After this patch is applied, UART2 will fail to allocate DMA resources
but UART driver can work fine without DMA.

This error is related to the DMAC of rk3328 (pl330 or compatible).
DMAC connected to 16 DMA sources. Each sources have ID number that is
called 'Req number' in rk3328 TRM. After this patch is applied total 7
of DMA sources will be activated as follows:

| Req number | Source | Required  |
||| channels  |
|++---|
|  8,  9 | SPI0   | 2ch   |
| 11, 12 | I2S0   | 2ch   |
| 14, 15 | I2S1   | 2ch   |
| 10 | SPDIF  | 1ch   |
|++---|
|| Total  | 7ch   |
|++---|
|  6,  7 | UART2  | 2ch   | -> cannot get DMA channels

Due to rk3328 DMAC specification we can use max 8 channels at same
time. If SPI0/I2S0/I2S1/SPDIF will be activated by this patch,
required DMAC channels reach to 7. So the last two channels (for
UART2) cannot get DMA resources.

Virt-dma mechanism for pl0330 DMAC driver is needed to fix this
problem.

Signed-off-by: Katsuhiro Suzuki 
---
 .../arm64/boot/dts/rockchip/rk3328-rock64.dts | 56 ---
 1 file changed, 24 insertions(+), 32 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts 
b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
index 86cfb5c50a94..c984662043da 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
@@ -84,34 +84,32 @@ standby_led: led-1 {
};
};
 
-   sound {
-   compatible = "audio-graph-card";
-   label = "rockchip,rk3328";
-   dais = <_p0
-   _p0>;
+   spdif_sound: spdif-sound {
+   compatible = "simple-audio-card";
+   simple-audio-card,name = "SPDIF";
+
+   simple-audio-card,cpu {
+   sound-dai = <>;
+   };
+
+   simple-audio-card,codec {
+   sound-dai = <_dit>;
+   };
};
 
-   spdif-dit {
+   spdif_dit: spdif-dit {
compatible = "linux,spdif-dit";
#sound-dai-cells = <0>;
-
-   port {
-   dit_p0_0: endpoint {
-   remote-endpoint = <_p0_0>;
-   };
-   };
};
 };
 
+_sound {
+   status = "okay";
+};
+
  {
mute-gpios = <_gpio 0 GPIO_ACTIVE_LOW>;
status = "okay";
-
-   port@0 {
-   codec_p0_0: endpoint {
-   remote-endpoint = <_p0_0>;
-   };
-   };
 };
 
  {
@@ -163,6 +161,10 @@  {
status = "okay";
 };
 
+_sound {
+   status = "okay";
+};
+
  {
status = "okay";
 };
@@ -278,16 +280,12 @@ regulator-state-mem {
};
 };
 
- {
+ {
status = "okay";
+};
 
-   i2s1_p0: port {
-   i2s1_p0_0: endpoint {
-   dai-format = "i2s";
-   mclk-fs = <256>;
-   remote-endpoint = <_p0_0>;
-   };
-   };
+ {
+   status = "okay";
 };
 
 _domains {
@@ -337,12 +335,6 @@  {
  {
pinctrl-0 = <_tx>;
status = "okay";
-
-   spdif_p0: port {
-   spdif_p0_0: endpoint {
-   remote-endpoint = <_p0_0>;
-   };
-   };
 };
 
  {
-- 
2.27.0



[PATCH] arm64: dts: rockchip: enable HDMI sound nodes for rk3328-rock64

2020-08-01 Thread Katsuhiro Suzuki
This patch enables HDMI sound (I2S0) and Analog sound (I2S1) which
are defined in rk3328.dtsi, and replace SPDIF nodes.

We can use SPDIF passthrough with suitable ALSA settings and on
mpv or other media players.
  - Settings: 
https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Rockchip/filesystem/usr/share/alsa/cards/SPDIF.conf
  - Ex.: mpv foo.ac3 --audio-spdif=ac3 
--audio-device='alsa/SPDIF.pcm.iec958.0:SPDIF'

[Why use simple-audio-card for SPDIF?]

For newly adding nodes, ASoC guys recommend to use audio-graph-card.
But all other sound nodes for rk3328 have already been defined by
simple-audio-card. In this time, I chose consistency of sound nodes.

[DMA allocation problem]

After apply this patch, UART2 will fail to allocate DMA resources
but UART driver can work fine without DMA.

This error is related to the DMAC of rk3328 (pl330 or compatible).
DMAC connected to 16 DMA sources. Each sources have ID number that is
called 'Req number' in rk3328 TRM. After apply this patch total 7
sources will be activated as follows:

| Req number | Source | Required  |
||| channels  |
|++---|
|  8,  9 | SPI0   | 2ch   |
| 11, 12 | I2S0   | 2ch   |
| 14, 15 | I2S1   | 2ch   |
| 10 | SPDIF  | 1ch   |
|++---|
|| Total  | 7ch   |
|++---|
|  6,  7 | UART2  | 2ch   | -> cannot get DMA channels

Due to rk3328 DMAC specification we can use max 8 channels at same
time. If SPI0/I2S0/I2S1/SPDIF will be activated by this patch,
required DMAC channels reach to 7. So last two channels (for UART2)
cannot get DMA resources.

Virt-dma mechanism for pl0330 DMAC driver is needed to fix this
problem.

Signed-off-by: Katsuhiro Suzuki 
---
 .../arm64/boot/dts/rockchip/rk3328-rock64.dts | 58 +--
 1 file changed, 26 insertions(+), 32 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts 
b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
index 86cfb5c50a94..4608f8fc6ff3 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
@@ -84,34 +84,34 @@ standby_led: led-1 {
};
};
 
-   sound {
-   compatible = "audio-graph-card";
-   label = "rockchip,rk3328";
-   dais = <_p0
-   _p0>;
+   spdif_sound: spdif-sound {
+   compatible = "simple-audio-card";
+   simple-audio-card,name = "SPDIF";
+   status = "okay";
+
+   simple-audio-card,cpu {
+   sound-dai = <>;
+   };
+
+   simple-audio-card,codec {
+   sound-dai = <_dit>;
+   };
};
 
-   spdif-dit {
+   spdif_dit: spdif-dit {
compatible = "linux,spdif-dit";
#sound-dai-cells = <0>;
-
-   port {
-   dit_p0_0: endpoint {
-   remote-endpoint = <_p0_0>;
-   };
-   };
+   status = "okay";
};
 };
 
+_sound {
+   status = "okay";
+};
+
  {
mute-gpios = <_gpio 0 GPIO_ACTIVE_LOW>;
status = "okay";
-
-   port@0 {
-   codec_p0_0: endpoint {
-   remote-endpoint = <_p0_0>;
-   };
-   };
 };
 
  {
@@ -163,6 +163,10 @@  {
status = "okay";
 };
 
+_sound {
+   status = "okay";
+};
+
  {
status = "okay";
 };
@@ -278,16 +282,12 @@ regulator-state-mem {
};
 };
 
- {
+ {
status = "okay";
+};
 
-   i2s1_p0: port {
-   i2s1_p0_0: endpoint {
-   dai-format = "i2s";
-   mclk-fs = <256>;
-   remote-endpoint = <_p0_0>;
-   };
-   };
+ {
+   status = "okay";
 };
 
 _domains {
@@ -337,12 +337,6 @@  {
  {
pinctrl-0 = <_tx>;
status = "okay";
-
-   spdif_p0: port {
-   spdif_p0_0: endpoint {
-   remote-endpoint = <_p0_0>;
-   };
-   };
 };
 
  {
-- 
2.27.0



[PATCH] arm64: dts: rockchip: add SPDIF node for rk3399-rockpro64

2020-08-01 Thread Katsuhiro Suzuki
This patch adds 'disabled' SPDIF sound node and related settings
for rk3399-rockpro64.

There are 2 reasons:
  - All RK3399 dma-bus channels have been already used by I2S0/1/2
  - RockPro64 does not have SPDIF optical nor coaxial connector,
just have 3pins

Signed-off-by: Katsuhiro Suzuki 
---
 .../boot/dts/rockchip/rk3399-rockpro64.dtsi   | 28 +++
 1 file changed, 28 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
index 6e553ff47534..e35b30f8a46e 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dtsi
@@ -76,6 +76,23 @@ sound {
dais = <_p0>;
};
 
+   sound-dit {
+   compatible = "audio-graph-card";
+   label = "rockchip,rk3399";
+   dais = <_p0>;
+   };
+
+   spdif-dit {
+   compatible = "linux,spdif-dit";
+   #sound-dai-cells = <0>;
+
+   port {
+   dit_p0_0: endpoint {
+   remote-endpoint = <_p0_0>;
+   };
+   };
+   };
+
vcc12v_dcin: vcc12v-dcin {
compatible = "regulator-fixed";
regulator-name = "vcc12v_dcin";
@@ -698,6 +715,17 @@  {
status = "okay";
 };
 
+ {
+   pinctrl-0 = <_bus_1>;
+   status = "disabled";
+
+   spdif_p0: port {
+   spdif_p0_0: endpoint {
+   remote-endpoint = <_p0_0>;
+   };
+   };
+};
+
  {
status = "okay";
 
-- 
2.27.0



[PATCH v2] dt-bindings: sound: convert Everest ES8316 binding to yaml

2020-07-24 Thread Katsuhiro Suzuki
This patch converts Everest Semiconductor ES8316 low power audio
CODEC binding to DT schema.

Signed-off-by: Katsuhiro Suzuki 

---

Changes in v2:
  - Change maintainers from Mark to Daniel and me
---
 .../bindings/sound/everest,es8316.txt | 23 -
 .../bindings/sound/everest,es8316.yaml| 50 +++
 2 files changed, 50 insertions(+), 23 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/sound/everest,es8316.txt
 create mode 100644 Documentation/devicetree/bindings/sound/everest,es8316.yaml

diff --git a/Documentation/devicetree/bindings/sound/everest,es8316.txt 
b/Documentation/devicetree/bindings/sound/everest,es8316.txt
deleted file mode 100644
index 1bf03c5f2af4..
--- a/Documentation/devicetree/bindings/sound/everest,es8316.txt
+++ /dev/null
@@ -1,23 +0,0 @@
-Everest ES8316 audio CODEC
-
-This device supports both I2C and SPI.
-
-Required properties:
-
-  - compatible  : should be "everest,es8316"
-  - reg : the I2C address of the device for I2C
-
-Optional properties:
-
-  - clocks : a list of phandle, should contain entries for clock-names
-  - clock-names : should include as follows:
- "mclk" : master clock (MCLK) of the device
-
-Example:
-
-es8316: codec@11 {
-   compatible = "everest,es8316";
-   reg = <0x11>;
-   clocks = < 10>;
-   clock-names = "mclk";
-};
diff --git a/Documentation/devicetree/bindings/sound/everest,es8316.yaml 
b/Documentation/devicetree/bindings/sound/everest,es8316.yaml
new file mode 100644
index ..3b752bba748b
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/everest,es8316.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/everest,es8316.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Everest ES8316 audio CODEC
+
+maintainers:
+  - Daniel Drake 
+  - Katsuhiro Suzuki 
+
+properties:
+  compatible:
+const: everest,es8316
+
+  reg:
+maxItems: 1
+
+  clocks:
+items:
+  - description: clock for master clock (MCLK)
+
+  clock-names:
+items:
+  - const: mclk
+
+  "#sound-dai-cells":
+const: 0
+
+required:
+  - compatible
+  - reg
+  - "#sound-dai-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+i2c0 {
+  #address-cells = <1>;
+  #size-cells = <0>;
+  es8316: codec@11 {
+compatible = "everest,es8316";
+reg = <0x11>;
+clocks = < 10>;
+clock-names = "mclk";
+#sound-dai-cells = <0>;
+  };
+};
-- 
2.27.0



Re: [PATCH] dt-bindings: sound: convert Everest ES8316 binding to yaml

2020-07-24 Thread Katsuhiro Suzuki

Hello Rob,

Thank you for review.

On 2020/07/24 6:26, Rob Herring wrote:

On Thu, Jul 23, 2020 at 03:07:28AM +0900, Katsuhiro Suzuki wrote:

This patch converts Everest Semiconductor ES8316 low power audio
CODEC binding to DT schema.

Signed-off-by: Katsuhiro Suzuki 
---
  .../bindings/sound/everest,es8316.txt | 23 -
  .../bindings/sound/everest,es8316.yaml| 49 +++
  2 files changed, 49 insertions(+), 23 deletions(-)
  delete mode 100644 Documentation/devicetree/bindings/sound/everest,es8316.txt
  create mode 100644 Documentation/devicetree/bindings/sound/everest,es8316.yaml

diff --git a/Documentation/devicetree/bindings/sound/everest,es8316.txt 
b/Documentation/devicetree/bindings/sound/everest,es8316.txt
deleted file mode 100644
index 1bf03c5f2af4..
--- a/Documentation/devicetree/bindings/sound/everest,es8316.txt
+++ /dev/null
@@ -1,23 +0,0 @@
-Everest ES8316 audio CODEC
-
-This device supports both I2C and SPI.
-
-Required properties:
-
-  - compatible  : should be "everest,es8316"
-  - reg : the I2C address of the device for I2C
-
-Optional properties:
-
-  - clocks : a list of phandle, should contain entries for clock-names
-  - clock-names : should include as follows:
- "mclk" : master clock (MCLK) of the device
-
-Example:
-
-es8316: codec@11 {
-   compatible = "everest,es8316";
-   reg = <0x11>;
-   clocks = < 10>;
-   clock-names = "mclk";
-};
diff --git a/Documentation/devicetree/bindings/sound/everest,es8316.yaml 
b/Documentation/devicetree/bindings/sound/everest,es8316.yaml
new file mode 100644
index ..b713404dac4f
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/everest,es8316.yaml
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/everest,es8316.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Everest ES8316 audio CODEC
+
+maintainers:
+  - Mark Brown 


Should be someone who knows and cares about the h/w which is not Mark.



OK, so set first committer Daniel and me (I can check codes on real device)
to maintainers.

Best Regards,
Katsuhiro Suzuki



+
+properties:
+  compatible:
+const: everest,es8316
+
+  reg:
+maxItems: 1
+
+  clocks:
+items:
+  - description: clock for master clock (MCLK)
+
+  clock-names:
+items:
+  - const: mclk
+
+  "#sound-dai-cells":
+const: 0
+
+required:
+  - compatible
+  - reg
+  - "#sound-dai-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+i2c0 {
+  #address-cells = <1>;
+  #size-cells = <0>;
+  es8316: codec@11 {
+compatible = "everest,es8316";
+reg = <0x11>;
+clocks = < 10>;
+clock-names = "mclk";
+#sound-dai-cells = <0>;
+  };
+};
--
2.27.0





Re: [PATCH] dt-bindings: sound: convert ROHM BD28623 amplifier binding to yaml

2020-07-22 Thread Katsuhiro Suzuki

Hello Rob,

Thanks a lot for your review!

Best Regards,
Katsuhiro Suzuki

On 2020/07/21 11:12, Rob Herring wrote:

On Tue, Jul 14, 2020 at 05:09:59PM +0900, Katsuhiro Suzuki wrote:

This patch converts ROHM BD28623UMV class D speaker amplifier binding
to DT schema.

Signed-off-by: Katsuhiro Suzuki 
---
  .../bindings/sound/rohm,bd28623.txt   | 29 -
  .../bindings/sound/rohm,bd28623.yaml  | 65 +++
  2 files changed, 65 insertions(+), 29 deletions(-)
  delete mode 100644 Documentation/devicetree/bindings/sound/rohm,bd28623.txt
  create mode 100644 Documentation/devicetree/bindings/sound/rohm,bd28623.yaml

diff --git a/Documentation/devicetree/bindings/sound/rohm,bd28623.txt 
b/Documentation/devicetree/bindings/sound/rohm,bd28623.txt
deleted file mode 100644
index d84557c2686e..
--- a/Documentation/devicetree/bindings/sound/rohm,bd28623.txt
+++ /dev/null
@@ -1,29 +0,0 @@
-ROHM BD28623MUV Class D speaker amplifier for digital input
-
-This codec does not have any control buses such as I2C, it detect format and
-rate of I2S signal automatically. It has two signals that can be connected
-to GPIOs: reset and mute.
-
-Required properties:
-- compatible  : should be "rohm,bd28623"
-- #sound-dai-cells: should be 0.
-- VCCA-supply : regulator phandle for the VCCA supply
-- VCCP1-supply: regulator phandle for the VCCP1 supply
-- VCCP2-supply: regulator phandle for the VCCP2 supply
-
-Optional properties:
-- reset-gpios : GPIO specifier for the active low reset line
-- mute-gpios  : GPIO specifier for the active low mute line
-
-Example:
-
-   codec {
-   compatible = "rohm,bd28623";
-   #sound-dai-cells = <0>;
-
-   VCCA-supply = <_reg>;
-   VCCP1-supply = <_reg>;
-   VCCP2-supply = <_reg>;
-   reset-gpios = < 0 GPIO_ACTIVE_LOW>;
-   mute-gpios = < 1 GPIO_ACTIVE_LOW>;
-   };
diff --git a/Documentation/devicetree/bindings/sound/rohm,bd28623.yaml 
b/Documentation/devicetree/bindings/sound/rohm,bd28623.yaml
new file mode 100644
index ..acd8609252b4
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/rohm,bd28623.yaml
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/rohm,bd28623.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ROHM BD28623MUV Class D speaker amplifier for digital input
+
+description:
+  This codec does not have any control buses such as I2C, it detect
+  format and rate of I2S signal automatically. It has two signals
+  that can be connected to GPIOs reset and mute.
+
+maintainers:
+  - Katsuhiro Suzuki 
+
+properties:
+  compatible:
+const: rohm,bd28623
+
+  "#sound-dai-cells":
+const: 0
+
+  VCCA-supply:
+description:
+  regulator phandle for the VCCA (for analog) power supply
+
+  VCCP1-supply:
+description:
+  regulator phandle for the VCCP1 (for ch1) power supply
+
+  VCCP2-supply:
+description:
+  regulator phandle for the VCCP2 (for ch2) power supply
+
+  reset-gpios:
+maxItems: 1
+description:
+  GPIO specifier for the active low reset line
+
+  mute-gpios:
+maxItems: 1
+description:
+  GPIO specifier for the active low mute line
+
+required:
+  - compatible
+  - VCCA-supply
+  - VCCP1-supply
+  - VCCP2-supply
+  - "#sound-dai-cells"


Needs an:

additionalProperties: false

With that,

Reviewed-by: Rob Herring 


+
+examples:
+  - |
+#include 
+codec {
+  compatible = "rohm,bd28623";
+  #sound-dai-cells = <0>;
+
+  VCCA-supply = <_reg>;
+  VCCP1-supply = <_reg>;
+  VCCP2-supply = <_reg>;
+  reset-gpios = < 0 GPIO_ACTIVE_LOW>;
+  mute-gpios = < 1 GPIO_ACTIVE_LOW>;
+};
--
2.27.0





[PATCH] dt-bindings: sound: convert Everest ES8316 binding to yaml

2020-07-22 Thread Katsuhiro Suzuki
This patch converts Everest Semiconductor ES8316 low power audio
CODEC binding to DT schema.

Signed-off-by: Katsuhiro Suzuki 
---
 .../bindings/sound/everest,es8316.txt | 23 -
 .../bindings/sound/everest,es8316.yaml| 49 +++
 2 files changed, 49 insertions(+), 23 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/sound/everest,es8316.txt
 create mode 100644 Documentation/devicetree/bindings/sound/everest,es8316.yaml

diff --git a/Documentation/devicetree/bindings/sound/everest,es8316.txt 
b/Documentation/devicetree/bindings/sound/everest,es8316.txt
deleted file mode 100644
index 1bf03c5f2af4..
--- a/Documentation/devicetree/bindings/sound/everest,es8316.txt
+++ /dev/null
@@ -1,23 +0,0 @@
-Everest ES8316 audio CODEC
-
-This device supports both I2C and SPI.
-
-Required properties:
-
-  - compatible  : should be "everest,es8316"
-  - reg : the I2C address of the device for I2C
-
-Optional properties:
-
-  - clocks : a list of phandle, should contain entries for clock-names
-  - clock-names : should include as follows:
- "mclk" : master clock (MCLK) of the device
-
-Example:
-
-es8316: codec@11 {
-   compatible = "everest,es8316";
-   reg = <0x11>;
-   clocks = < 10>;
-   clock-names = "mclk";
-};
diff --git a/Documentation/devicetree/bindings/sound/everest,es8316.yaml 
b/Documentation/devicetree/bindings/sound/everest,es8316.yaml
new file mode 100644
index ..b713404dac4f
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/everest,es8316.yaml
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/everest,es8316.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Everest ES8316 audio CODEC
+
+maintainers:
+  - Mark Brown 
+
+properties:
+  compatible:
+const: everest,es8316
+
+  reg:
+maxItems: 1
+
+  clocks:
+items:
+  - description: clock for master clock (MCLK)
+
+  clock-names:
+items:
+  - const: mclk
+
+  "#sound-dai-cells":
+const: 0
+
+required:
+  - compatible
+  - reg
+  - "#sound-dai-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+i2c0 {
+  #address-cells = <1>;
+  #size-cells = <0>;
+  es8316: codec@11 {
+compatible = "everest,es8316";
+reg = <0x11>;
+clocks = < 10>;
+clock-names = "mclk";
+#sound-dai-cells = <0>;
+  };
+};
-- 
2.27.0



[PATCH v2] dt-bindings: sound: convert ROHM BD28623 amplifier binding to yaml

2020-07-22 Thread Katsuhiro Suzuki
This patch converts ROHM BD28623UMV class D speaker amplifier binding
to DT schema.

Signed-off-by: Katsuhiro Suzuki 
Reviewed-by: Rob Herring 

---

Changes in v2:
  - add additionalProperties:false
  - add Rob's Reviewed-by
---
 .../bindings/sound/rohm,bd28623.txt   | 29 
 .../bindings/sound/rohm,bd28623.yaml  | 67 +++
 2 files changed, 67 insertions(+), 29 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/sound/rohm,bd28623.txt
 create mode 100644 Documentation/devicetree/bindings/sound/rohm,bd28623.yaml

diff --git a/Documentation/devicetree/bindings/sound/rohm,bd28623.txt 
b/Documentation/devicetree/bindings/sound/rohm,bd28623.txt
deleted file mode 100644
index d84557c2686e..
--- a/Documentation/devicetree/bindings/sound/rohm,bd28623.txt
+++ /dev/null
@@ -1,29 +0,0 @@
-ROHM BD28623MUV Class D speaker amplifier for digital input
-
-This codec does not have any control buses such as I2C, it detect format and
-rate of I2S signal automatically. It has two signals that can be connected
-to GPIOs: reset and mute.
-
-Required properties:
-- compatible  : should be "rohm,bd28623"
-- #sound-dai-cells: should be 0.
-- VCCA-supply : regulator phandle for the VCCA supply
-- VCCP1-supply: regulator phandle for the VCCP1 supply
-- VCCP2-supply: regulator phandle for the VCCP2 supply
-
-Optional properties:
-- reset-gpios : GPIO specifier for the active low reset line
-- mute-gpios  : GPIO specifier for the active low mute line
-
-Example:
-
-   codec {
-   compatible = "rohm,bd28623";
-   #sound-dai-cells = <0>;
-
-   VCCA-supply = <_reg>;
-   VCCP1-supply = <_reg>;
-   VCCP2-supply = <_reg>;
-   reset-gpios = < 0 GPIO_ACTIVE_LOW>;
-   mute-gpios = < 1 GPIO_ACTIVE_LOW>;
-   };
diff --git a/Documentation/devicetree/bindings/sound/rohm,bd28623.yaml 
b/Documentation/devicetree/bindings/sound/rohm,bd28623.yaml
new file mode 100644
index ..859ce64da152
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/rohm,bd28623.yaml
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/rohm,bd28623.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ROHM BD28623MUV Class D speaker amplifier for digital input
+
+description:
+  This codec does not have any control buses such as I2C, it detect
+  format and rate of I2S signal automatically. It has two signals
+  that can be connected to GPIOs reset and mute.
+
+maintainers:
+  - Katsuhiro Suzuki 
+
+properties:
+  compatible:
+const: rohm,bd28623
+
+  "#sound-dai-cells":
+const: 0
+
+  VCCA-supply:
+description:
+  regulator phandle for the VCCA (for analog) power supply
+
+  VCCP1-supply:
+description:
+  regulator phandle for the VCCP1 (for ch1) power supply
+
+  VCCP2-supply:
+description:
+  regulator phandle for the VCCP2 (for ch2) power supply
+
+  reset-gpios:
+maxItems: 1
+description:
+  GPIO specifier for the active low reset line
+
+  mute-gpios:
+maxItems: 1
+description:
+  GPIO specifier for the active low mute line
+
+required:
+  - compatible
+  - VCCA-supply
+  - VCCP1-supply
+  - VCCP2-supply
+  - "#sound-dai-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+#include 
+codec {
+  compatible = "rohm,bd28623";
+  #sound-dai-cells = <0>;
+
+  VCCA-supply = <_reg>;
+  VCCP1-supply = <_reg>;
+  VCCP2-supply = <_reg>;
+  reset-gpios = < 0 GPIO_ACTIVE_LOW>;
+  mute-gpios = < 1 GPIO_ACTIVE_LOW>;
+};
-- 
2.27.0



[PATCH] dt-bindings: sound: convert ROHM BD28623 amplifier binding to yaml

2020-07-14 Thread Katsuhiro Suzuki
This patch converts ROHM BD28623UMV class D speaker amplifier binding
to DT schema.

Signed-off-by: Katsuhiro Suzuki 
---
 .../bindings/sound/rohm,bd28623.txt   | 29 -
 .../bindings/sound/rohm,bd28623.yaml  | 65 +++
 2 files changed, 65 insertions(+), 29 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/sound/rohm,bd28623.txt
 create mode 100644 Documentation/devicetree/bindings/sound/rohm,bd28623.yaml

diff --git a/Documentation/devicetree/bindings/sound/rohm,bd28623.txt 
b/Documentation/devicetree/bindings/sound/rohm,bd28623.txt
deleted file mode 100644
index d84557c2686e..
--- a/Documentation/devicetree/bindings/sound/rohm,bd28623.txt
+++ /dev/null
@@ -1,29 +0,0 @@
-ROHM BD28623MUV Class D speaker amplifier for digital input
-
-This codec does not have any control buses such as I2C, it detect format and
-rate of I2S signal automatically. It has two signals that can be connected
-to GPIOs: reset and mute.
-
-Required properties:
-- compatible  : should be "rohm,bd28623"
-- #sound-dai-cells: should be 0.
-- VCCA-supply : regulator phandle for the VCCA supply
-- VCCP1-supply: regulator phandle for the VCCP1 supply
-- VCCP2-supply: regulator phandle for the VCCP2 supply
-
-Optional properties:
-- reset-gpios : GPIO specifier for the active low reset line
-- mute-gpios  : GPIO specifier for the active low mute line
-
-Example:
-
-   codec {
-   compatible = "rohm,bd28623";
-   #sound-dai-cells = <0>;
-
-   VCCA-supply = <_reg>;
-   VCCP1-supply = <_reg>;
-   VCCP2-supply = <_reg>;
-   reset-gpios = < 0 GPIO_ACTIVE_LOW>;
-   mute-gpios = < 1 GPIO_ACTIVE_LOW>;
-   };
diff --git a/Documentation/devicetree/bindings/sound/rohm,bd28623.yaml 
b/Documentation/devicetree/bindings/sound/rohm,bd28623.yaml
new file mode 100644
index ..acd8609252b4
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/rohm,bd28623.yaml
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/rohm,bd28623.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ROHM BD28623MUV Class D speaker amplifier for digital input
+
+description:
+  This codec does not have any control buses such as I2C, it detect
+  format and rate of I2S signal automatically. It has two signals
+  that can be connected to GPIOs reset and mute.
+
+maintainers:
+  - Katsuhiro Suzuki 
+
+properties:
+  compatible:
+const: rohm,bd28623
+
+  "#sound-dai-cells":
+const: 0
+
+  VCCA-supply:
+description:
+  regulator phandle for the VCCA (for analog) power supply
+
+  VCCP1-supply:
+description:
+  regulator phandle for the VCCP1 (for ch1) power supply
+
+  VCCP2-supply:
+description:
+  regulator phandle for the VCCP2 (for ch2) power supply
+
+  reset-gpios:
+maxItems: 1
+description:
+  GPIO specifier for the active low reset line
+
+  mute-gpios:
+maxItems: 1
+description:
+  GPIO specifier for the active low mute line
+
+required:
+  - compatible
+  - VCCA-supply
+  - VCCP1-supply
+  - VCCP2-supply
+  - "#sound-dai-cells"
+
+examples:
+  - |
+#include 
+codec {
+  compatible = "rohm,bd28623";
+  #sound-dai-cells = <0>;
+
+  VCCA-supply = <_reg>;
+  VCCP1-supply = <_reg>;
+  VCCP2-supply = <_reg>;
+  reset-gpios = < 0 GPIO_ACTIVE_LOW>;
+  mute-gpios = < 1 GPIO_ACTIVE_LOW>;
+};
-- 
2.27.0



Re: [PATCH] arm64: dts: rockchip: add analog audio nodes on rk3399-rockpro64

2019-10-04 Thread Katsuhiro Suzuki

Past about 1 month, so I send a ping...

On 2019/09/08 2:48, Katsuhiro Suzuki wrote:

This patch adds audio codec (Everest ES8316) and I2S audio nodes for
RK3399 RockPro64.

Signed-off-by: Katsuhiro Suzuki 
---
  .../boot/dts/rockchip/rk3399-rockpro64.dts| 28 +++
  1 file changed, 28 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts 
b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
index 0401d4ec1f45..8b1e6382b140 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
@@ -81,6 +81,12 @@
reset-gpios = < RK_PB2 GPIO_ACTIVE_LOW>;
};
  
+	sound {

+   compatible = "audio-graph-card";
+   label = "rockchip,rk3399";
+   dais = <_p0>;
+   };
+
vcc12v_dcin: vcc12v-dcin {
compatible = "regulator-fixed";
regulator-name = "vcc12v_dcin";
@@ -470,6 +476,20 @@
i2c-scl-rising-time-ns = <300>;
i2c-scl-falling-time-ns = <15>;
status = "okay";
+
+   es8316: codec@11 {
+   compatible = "everest,es8316";
+   reg = <0x11>;
+   clocks = < SCLK_I2S_8CH_OUT>;
+   clock-names = "mclk";
+   #sound-dai-cells = <0>;
+
+   port {
+   es8316_p0_0: endpoint {
+   remote-endpoint = <_p0_0>;
+   };
+   };
+   };
  };
  
   {

@@ -505,6 +525,14 @@
rockchip,playback-channels = <2>;
rockchip,capture-channels = <2>;
status = "okay";
+
+   i2s1_p0: port {
+   i2s1_p0_0: endpoint {
+   dai-format = "i2s";
+   mclk-fs = <256>;
+   remote-endpoint = <_p0_0>;
+   };
+   };
  };
  
   {






Re: [PATCH] SoC: simple-card-utils: set 0Hz to sysclk when shutdown

2019-09-08 Thread Katsuhiro Suzuki

On 2019/09/09 9:31, Kuninori Morimoto wrote:


Hi Katsuhiro


This patch set 0Hz to sysclk when shutdown the card.

Some codecs set rate constraints that derives from sysclk. This
mechanism works correctly if machine drivers give fixed frequency.

But simple-audio and audio-graph card set variable clock rate if
'mclk-fs' property exists. In this case, rate constraints will go
bad scenario. For example a codec accepts three limited rates
(mclk / 256, mclk / 384, mclk / 512).

Bad scenario as follows (mclk-fs = 256):
- Initialize sysclk by correct value (Ex. 12.288MHz)
  - Codec set constraints of PCM rate by sysclk
48kHz (1/256), 32kHz (1/384), 24kHz (1/512)
- Play 48kHz sound, it's acceptable
- Sysclk is not changed

- Play 32kHz sound, it's acceptable
- Set sysclk to 8.192MHz (= fs * mclk-fs = 32k * 256)
  - Codec set constraints of PCM rate by sysclk
32kHz (1/256), 21.33kHz (1/384), 16kHz (1/512)

- Play 48kHz again, but it's NOT acceptable because constraints
  do not allow 48kHz

So codecs treat 0Hz sysclk as signal of applying no constraints to
avoid this problem.

Signed-off-by: Katsuhiro Suzuki 
---


I'm not 100% understand your issue.
.hw_params (= set mclk/sysclk) is not called in bad case ??
Or it is called but Codec driver ignores it somehow ??



Ah, sorry for confusing. It's not either. hw_params() of machine
driver has been called even if constraints don't have a requested
PCM rate. But it's not expected.

For example, if constraints are 32k, 21.33k, 16k, hw_params() will
be called with 32k when an user requests to play 48k sounds.



Thank you for your help !!
Best regards
---
Kuninori Morimoto



Best Regards,
Katsuhiro Suzuki


[PATCH] arm64: dts: rockchip: add analog audio nodes on rk3399-rockpro64

2019-09-07 Thread Katsuhiro Suzuki
This patch adds audio codec (Everest ES8316) and I2S audio nodes for
RK3399 RockPro64.

Signed-off-by: Katsuhiro Suzuki 
---
 .../boot/dts/rockchip/rk3399-rockpro64.dts| 28 +++
 1 file changed, 28 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts 
b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
index 0401d4ec1f45..8b1e6382b140 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
@@ -81,6 +81,12 @@
reset-gpios = < RK_PB2 GPIO_ACTIVE_LOW>;
};
 
+   sound {
+   compatible = "audio-graph-card";
+   label = "rockchip,rk3399";
+   dais = <_p0>;
+   };
+
vcc12v_dcin: vcc12v-dcin {
compatible = "regulator-fixed";
regulator-name = "vcc12v_dcin";
@@ -470,6 +476,20 @@
i2c-scl-rising-time-ns = <300>;
i2c-scl-falling-time-ns = <15>;
status = "okay";
+
+   es8316: codec@11 {
+   compatible = "everest,es8316";
+   reg = <0x11>;
+   clocks = < SCLK_I2S_8CH_OUT>;
+   clock-names = "mclk";
+   #sound-dai-cells = <0>;
+
+   port {
+   es8316_p0_0: endpoint {
+   remote-endpoint = <_p0_0>;
+   };
+   };
+   };
 };
 
  {
@@ -505,6 +525,14 @@
rockchip,playback-channels = <2>;
rockchip,capture-channels = <2>;
status = "okay";
+
+   i2s1_p0: port {
+   i2s1_p0_0: endpoint {
+   dai-format = "i2s";
+   mclk-fs = <256>;
+   remote-endpoint = <_p0_0>;
+   };
+   };
 };
 
  {
-- 
2.23.0.rc1



[PATCH] SoC: simple-card-utils: set 0Hz to sysclk when shutdown

2019-09-07 Thread Katsuhiro Suzuki
This patch set 0Hz to sysclk when shutdown the card.

Some codecs set rate constraints that derives from sysclk. This
mechanism works correctly if machine drivers give fixed frequency.

But simple-audio and audio-graph card set variable clock rate if
'mclk-fs' property exists. In this case, rate constraints will go
bad scenario. For example a codec accepts three limited rates
(mclk / 256, mclk / 384, mclk / 512).

Bad scenario as follows (mclk-fs = 256):
   - Initialize sysclk by correct value (Ex. 12.288MHz)
 - Codec set constraints of PCM rate by sysclk
   48kHz (1/256), 32kHz (1/384), 24kHz (1/512)
   - Play 48kHz sound, it's acceptable
   - Sysclk is not changed

   - Play 32kHz sound, it's acceptable
   - Set sysclk to 8.192MHz (= fs * mclk-fs = 32k * 256)
 - Codec set constraints of PCM rate by sysclk
   32kHz (1/256), 21.33kHz (1/384), 16kHz (1/512)

   - Play 48kHz again, but it's NOT acceptable because constraints
 do not allow 48kHz

So codecs treat 0Hz sysclk as signal of applying no constraints to
avoid this problem.

Signed-off-by: Katsuhiro Suzuki 
---
 sound/soc/generic/simple-card-utils.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/sound/soc/generic/simple-card-utils.c 
b/sound/soc/generic/simple-card-utils.c
index 556b1a789629..9b794775df53 100644
--- a/sound/soc/generic/simple-card-utils.c
+++ b/sound/soc/generic/simple-card-utils.c
@@ -213,10 +213,17 @@ EXPORT_SYMBOL_GPL(asoc_simple_startup);
 void asoc_simple_shutdown(struct snd_pcm_substream *substream)
 {
struct snd_soc_pcm_runtime *rtd = substream->private_data;
+   struct snd_soc_dai *codec_dai = rtd->codec_dai;
+   struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
struct asoc_simple_priv *priv = snd_soc_card_get_drvdata(rtd->card);
struct simple_dai_props *dai_props =
simple_priv_to_props(priv, rtd->num);
 
+   if (dai_props->mclk_fs) {
+   snd_soc_dai_set_sysclk(codec_dai, 0, 0, SND_SOC_CLOCK_IN);
+   snd_soc_dai_set_sysclk(cpu_dai, 0, 0, SND_SOC_CLOCK_OUT);
+   }
+
asoc_simple_clk_disable(dai_props->cpu_dai);
 
asoc_simple_clk_disable(dai_props->codec_dai);
-- 
2.23.0.rc1



[PATCH] ASoC: rockchip: ignore 0Hz sysclk

2019-09-07 Thread Katsuhiro Suzuki
This patch ignores sysclk setting if it is 0Hz.

Some codecs treat 0Hz sysclk as signal of applying no constraints.
This driver does not have such feature but current implementation
outputs 'Failed to set mclk' error message if machine driver sets
0Hz sysclk to this driver.

Signed-off-by: Katsuhiro Suzuki 
---
 sound/soc/rockchip/rockchip_i2s.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/soc/rockchip/rockchip_i2s.c 
b/sound/soc/rockchip/rockchip_i2s.c
index 88ebaf6e1880..af2d5a6124c8 100644
--- a/sound/soc/rockchip/rockchip_i2s.c
+++ b/sound/soc/rockchip/rockchip_i2s.c
@@ -419,6 +419,9 @@ static int rockchip_i2s_set_sysclk(struct snd_soc_dai 
*cpu_dai, int clk_id,
struct rk_i2s_dev *i2s = to_info(cpu_dai);
int ret;
 
+   if (freq == 0)
+   return 0;
+
ret = clk_set_rate(i2s->mclk, freq);
if (ret)
dev_err(i2s->dev, "Fail to set mclk %d\n", ret);
-- 
2.23.0.rc1



[PATCH 2/2] ASoC: es8316: support fixed and variable both clock rates

2019-09-07 Thread Katsuhiro Suzuki
This patch supports some type of machine drivers that set 0 to mclk
when sound device goes to idle state. After applied this patch,
sysclk == 0 means there is no constraint of sound rate and other
values will set constraints which is derived by sysclk setting.

Original code refuses sysclk == 0 setting. But some boards and SoC
(such as RockPro64 and RockChip I2S) has connected SoC MCLK out to
ES8316 MCLK in. In this case, SoC side I2S will choose suitable
frequency of MCLK such as fs * mclk-fs when user starts playing or
capturing.

Bad scenario as follows (mclk-fs = 256):
  - Initialize sysclk by correct value (Ex. 12.288MHz)
- ES8316 set constraints of PCM rate by sysclk
  48kHz (1/256), 32kHz (1/384), 30.720kHz (1/400),
  24kHz (1/512), 16kHz (1/768), 12kHz (1/1024)
  - Play 48kHz sound, it's acceptable
  - Sysclk is not changed

  - Play 32kHz sound, it's acceptable
  - Set sysclk by 8.192MHz (= fs * mclk-fs = 32k * 256)
- ES8316 set constraints of PCM rate by sysclk
  32kHz (1/256), 21.33kHz (1/384), 20.48kHz (1/400),
  16kHz (1/512), 10.66kHz (1/768), 8kHz (1/1024)

  - Play 48kHz again, but it's NOT acceptable because constraints
list does not allow 48kHz

Signed-off-by: Katsuhiro Suzuki 
---
 sound/soc/codecs/es8316.c | 35 ---
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/sound/soc/codecs/es8316.c b/sound/soc/codecs/es8316.c
index 1424ecd6952c..36eef1fb3d18 100644
--- a/sound/soc/codecs/es8316.c
+++ b/sound/soc/codecs/es8316.c
@@ -370,8 +370,12 @@ static int es8316_set_dai_sysclk(struct snd_soc_dai 
*codec_dai,
 
es8316->sysclk = freq;
 
-   if (freq == 0)
+   if (freq == 0) {
+   es8316->sysclk_constraints.list = NULL;
+   es8316->sysclk_constraints.count = 0;
+
return 0;
+   }
 
ret = clk_set_rate(es8316->mclk, freq);
if (ret)
@@ -450,17 +454,10 @@ static int es8316_pcm_startup(struct snd_pcm_substream 
*substream,
struct snd_soc_component *component = dai->component;
struct es8316_priv *es8316 = snd_soc_component_get_drvdata(component);
 
-   if (es8316->sysclk == 0) {
-   dev_err(component->dev, "No sysclk provided\n");
-   return -EINVAL;
-   }
-
-   /* The set of sample rates that can be supported depends on the
-* MCLK supplied to the CODEC.
-*/
-   snd_pcm_hw_constraint_list(substream->runtime, 0,
-  SNDRV_PCM_HW_PARAM_RATE,
-  >sysclk_constraints);
+   if (es8316->sysclk_constraints.list)
+   snd_pcm_hw_constraint_list(substream->runtime, 0,
+  SNDRV_PCM_HW_PARAM_RATE,
+  >sysclk_constraints);
 
return 0;
 }
@@ -472,11 +469,19 @@ static int es8316_pcm_hw_params(struct snd_pcm_substream 
*substream,
struct snd_soc_component *component = dai->component;
struct es8316_priv *es8316 = snd_soc_component_get_drvdata(component);
u8 wordlen = 0;
+   int i;
 
-   if (!es8316->sysclk) {
-   dev_err(component->dev, "No MCLK configured\n");
-   return -EINVAL;
+   /* Validate supported sample rates that are autodetected from MCLK */
+   for (i = 0; i < NR_SUPPORTED_MCLK_LRCK_RATIOS; i++) {
+   const unsigned int ratio = supported_mclk_lrck_ratios[i];
+
+   if (es8316->sysclk % ratio != 0)
+   continue;
+   if (es8316->sysclk / ratio == params_rate(params))
+   break;
}
+   if (i == NR_SUPPORTED_MCLK_LRCK_RATIOS)
+   return -EINVAL;
 
switch (params_format(params)) {
case SNDRV_PCM_FORMAT_S16_LE:
-- 
2.23.0.rc1



[PATCH 1/2] ASoC: es8316: fix redundant codes of clock

2019-09-07 Thread Katsuhiro Suzuki
This patch removes redundant null checks for optional MCLK clock.
And fix DT binding document for changing clock property to optional
from required.

Signed-off-by: Katsuhiro Suzuki 
---
 .../bindings/sound/everest,es8316.txt |  3 ++
 sound/soc/codecs/es8316.c | 31 ---
 2 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/everest,es8316.txt 
b/Documentation/devicetree/bindings/sound/everest,es8316.txt
index aefcff9c48a2..1bf03c5f2af4 100644
--- a/Documentation/devicetree/bindings/sound/everest,es8316.txt
+++ b/Documentation/devicetree/bindings/sound/everest,es8316.txt
@@ -6,6 +6,9 @@ Required properties:
 
   - compatible  : should be "everest,es8316"
   - reg : the I2C address of the device for I2C
+
+Optional properties:
+
   - clocks : a list of phandle, should contain entries for clock-names
   - clock-names : should include as follows:
  "mclk" : master clock (MCLK) of the device
diff --git a/sound/soc/codecs/es8316.c b/sound/soc/codecs/es8316.c
index d07d50f51b28..1424ecd6952c 100644
--- a/sound/soc/codecs/es8316.c
+++ b/sound/soc/codecs/es8316.c
@@ -373,11 +373,9 @@ static int es8316_set_dai_sysclk(struct snd_soc_dai 
*codec_dai,
if (freq == 0)
return 0;
 
-   if (es8316->mclk) {
-   ret = clk_set_rate(es8316->mclk, freq);
-   if (ret)
-   return ret;
-   }
+   ret = clk_set_rate(es8316->mclk, freq);
+   if (ret)
+   return ret;
 
/* Limit supported sample rates to ones that can be autodetected
 * by the codec running in slave mode.
@@ -712,20 +710,18 @@ static int es8316_probe(struct snd_soc_component 
*component)
 
es8316->component = component;
 
-   es8316->mclk = devm_clk_get(component->dev, "mclk");
-   if (PTR_ERR(es8316->mclk) == -EPROBE_DEFER)
-   return -EPROBE_DEFER;
+   es8316->mclk = devm_clk_get_optional(component->dev, "mclk");
if (IS_ERR(es8316->mclk)) {
-   dev_err(component->dev, "clock is invalid, ignored\n");
-   es8316->mclk = NULL;
+   dev_err(component->dev, "unable to get mclk\n");
+   return PTR_ERR(es8316->mclk);
}
+   if (!es8316->mclk)
+   dev_warn(component->dev, "assuming static mclk\n");
 
-   if (es8316->mclk) {
-   ret = clk_prepare_enable(es8316->mclk);
-   if (ret) {
-   dev_err(component->dev, "unable to enable clock\n");
-   return ret;
-   }
+   ret = clk_prepare_enable(es8316->mclk);
+   if (ret) {
+   dev_err(component->dev, "unable to enable mclk\n");
+   return ret;
}
 
/* Reset codec and enable current state machine */
@@ -754,8 +750,7 @@ static void es8316_remove(struct snd_soc_component 
*component)
 {
struct es8316_priv *es8316 = snd_soc_component_get_drvdata(component);
 
-   if (es8316->mclk)
-   clk_disable_unprepare(es8316->mclk);
+   clk_disable_unprepare(es8316->mclk);
 }
 
 static const struct snd_soc_component_driver soc_component_dev_es8316 = {
-- 
2.23.0.rc1



Re: [PATCH v3 1/4] ASoC: es8316: judge PCM rate at later timing

2019-09-04 Thread Katsuhiro Suzuki

On 2019/09/05 0:30, Mark Brown wrote:

On Thu, Sep 05, 2019 at 12:06:23AM +0900, Katsuhiro Suzuki wrote:


Would you tell me one more thing. I don't understand who sets MCLK to 0.
Is it needed original machine driver instead of audio-graph-card?



On my test environment (audio-graph-card + Rockchip I2S + ES8316), it
seems audio-graph-card has never called set_sysclk() with freq = 0 after
stop play/capture sound. So my env will go to bad scenario as I described in
this patch.


You shouldn't need a custom machine driver - you'll just be the first
person who ran into this with audio-graph-card.  I'd just add this
support to the audio-graph-card, either with custom startup and shutdown
callbacks or using a set_bias_level() callback (both get used, I'd guess
the set_bias_level() is easier since you don't need to reference count
anything).



Oh, I understand current situation. I'll try it.
Thanks for your support!

Best Regards,
Katsuhiro Suzuki


Re: [PATCH v3 2/4] ASoC: es8316: add clock control of MCLK

2019-09-04 Thread Katsuhiro Suzuki

Hello Andy,

Thank you for reviewing.

On 2019/09/04 23:37, Andy Shevchenko wrote:

On Tue, Sep 3, 2019 at 7:54 PM Katsuhiro Suzuki  wrote:


This patch introduce clock property for MCLK master freq control.
Driver will set rate of MCLK master if set_sysclk is called and
changing sysclk by board driver.

Signed-off-by: Katsuhiro Suzuki 




+   if (es8316->mclk) {


You don't need this if clock has been requested as optional
(clk_get_optional() or so).


+   ret = clk_set_rate(es8316->mclk, freq);
+   if (ret)
+   return ret;
+   }



+   es8316->mclk = devm_clk_get(component->dev, "mclk");
+   if (PTR_ERR(es8316->mclk) == -EPROBE_DEFER)
+   return -EPROBE_DEFER;
+   if (IS_ERR(es8316->mclk)) {
+   dev_err(component->dev, "clock is invalid, ignored\n");
+   es8316->mclk = NULL;
+   }


devm_clk_get_optional()


+   if (es8316->mclk) {


Ditto as above.


+   ret = clk_prepare_enable(es8316->mclk);
+   if (ret) {
+   dev_err(component->dev, "unable to enable clock\n");
+   return ret;
+   }
+   }



+   if (es8316->mclk)


Ditto.


+   clk_disable_unprepare(es8316->mclk);
+}





Indeed, NULL check of MCLK is not needed.
I'll make and send fixup patch.

Best Regards,
Katsuhiro Suzuki


Re: [PATCH v3 1/4] ASoC: es8316: judge PCM rate at later timing

2019-09-04 Thread Katsuhiro Suzuki

Hello Mark,

On 2019/09/04 2:48, Mark Brown wrote:

On Wed, Sep 04, 2019 at 01:53:19AM +0900, Katsuhiro Suzuki wrote:


Root cause of this strange behavior is changing constraints list at
set_sysclk timing. It seems that is too early to determine. So this
patch does not use constraints list and check PCM rate limit more
later timing at hw_params.


hw_params is a bit late to impose constraints, you want them to be
available to be available to the application before it gets as far as
picking the parameters it wants so that you don't get hw_params failing
due to an invalid configuration.  That makes everything run more
smoothly, applications should be able to trust the constraints they got
and some will not handle failures well.

The way this works with the variable MCLKs is that you end up in one of
two cases (wm8731 and wm8741 do this):

1. The system is idle, MCLK is set to 0.  In this case no constraints
   are set and we just set MCLK to whatever is required in hw_params()
   in the machine driver.
2. One direction is active, MCLK is set to whatever that needed.  In
   this case startup() sets constraints derived from the MCLK.

There are races in this if streams are being started and torn down
simultaneously, there's not much we can do about them with the API the
way it is so we do have to validate in hw_params() anyway but it should
be validation not constraint imposition.

If the system has a fixed MCLK it just sets that on probe then we always
get the constraints applied on startup through the same code that
handles case 2.



Thank you for explanation. I agree with apply no constraints if MCLK is
set to 0, and suitable constraints if MCLK is set to other values like
as wm8731 and wm8741 drivers.

I'll change my patch set and send.


Would you tell me one more thing. I don't understand who sets MCLK to 0.
Is it needed original machine driver instead of audio-graph-card?

On my test environment (audio-graph-card + Rockchip I2S + ES8316), it
seems audio-graph-card has never called set_sysclk() with freq = 0 after
stop play/capture sound. So my env will go to bad scenario as I 
described in this patch.


Best Regards,
Katsuhiro Suzuki


[PATCH v3 4/4] ASoC: es8316: add DT-bindings

2019-09-03 Thread Katsuhiro Suzuki
This patch adds missing DT-bindings document for Everest ES8316.

Signed-off-by: Katsuhiro Suzuki 
---
 .../bindings/sound/everest,es8316.txt | 20 +++
 1 file changed, 20 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/everest,es8316.txt

diff --git a/Documentation/devicetree/bindings/sound/everest,es8316.txt 
b/Documentation/devicetree/bindings/sound/everest,es8316.txt
new file mode 100644
index ..aefcff9c48a2
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/everest,es8316.txt
@@ -0,0 +1,20 @@
+Everest ES8316 audio CODEC
+
+This device supports both I2C and SPI.
+
+Required properties:
+
+  - compatible  : should be "everest,es8316"
+  - reg : the I2C address of the device for I2C
+  - clocks : a list of phandle, should contain entries for clock-names
+  - clock-names : should include as follows:
+ "mclk" : master clock (MCLK) of the device
+
+Example:
+
+es8316: codec@11 {
+   compatible = "everest,es8316";
+   reg = <0x11>;
+   clocks = < 10>;
+   clock-names = "mclk";
+};
-- 
2.23.0.rc1



[PATCH v3 3/4] ASoC: es8316: support fixed clock rate

2019-09-03 Thread Katsuhiro Suzuki
This patch supports some type of machine drivers that use fixed clock
rate. After applied this patch, sysclk == 0 means there is no
constraint of sound rate and other values will set constraints which
is derived by sysclk setting.

Signed-off-by: Katsuhiro Suzuki 

---

Changes from v2:
  - Newly added
---
 sound/soc/codecs/es8316.c | 24 
 1 file changed, 4 insertions(+), 20 deletions(-)

diff --git a/sound/soc/codecs/es8316.c b/sound/soc/codecs/es8316.c
index ab41f2c056bd..bf390bc64177 100644
--- a/sound/soc/codecs/es8316.c
+++ b/sound/soc/codecs/es8316.c
@@ -432,20 +432,6 @@ static int es8316_set_dai_fmt(struct snd_soc_dai 
*codec_dai,
return 0;
 }
 
-static int es8316_pcm_startup(struct snd_pcm_substream *substream,
- struct snd_soc_dai *dai)
-{
-   struct snd_soc_component *component = dai->component;
-   struct es8316_priv *es8316 = snd_soc_component_get_drvdata(component);
-
-   if (es8316->sysclk == 0) {
-   dev_err(component->dev, "No sysclk provided\n");
-   return -EINVAL;
-   }
-
-   return 0;
-}
-
 static int es8316_pcm_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params,
struct snd_soc_dai *dai)
@@ -455,17 +441,16 @@ static int es8316_pcm_hw_params(struct snd_pcm_substream 
*substream,
u8 wordlen = 0;
int i;
 
-   if (!es8316->sysclk) {
-   dev_err(component->dev, "No MCLK configured\n");
-   return -EINVAL;
-   }
-
/* Limit supported sample rates to ones that can be autodetected
 * by the codec running in slave mode.
 */
for (i = 0; i < NR_SUPPORTED_MCLK_LRCK_RATIOS; i++) {
const unsigned int ratio = supported_mclk_lrck_ratios[i];
 
+   /* Accept any rates if sysclk is 0. */
+   if (es8316->sysclk == 0)
+   break;
+
if (es8316->sysclk % ratio != 0)
continue;
if (es8316->sysclk / ratio == params_rate(params))
@@ -509,7 +494,6 @@ static int es8316_mute(struct snd_soc_dai *dai, int mute)
SNDRV_PCM_FMTBIT_S24_LE)
 
 static const struct snd_soc_dai_ops es8316_ops = {
-   .startup = es8316_pcm_startup,
.hw_params = es8316_pcm_hw_params,
.set_fmt = es8316_set_dai_fmt,
.set_sysclk = es8316_set_dai_sysclk,
-- 
2.23.0.rc1



[PATCH v3 1/4] ASoC: es8316: judge PCM rate at later timing

2019-09-03 Thread Katsuhiro Suzuki
This patch change the judge timing about playing/capturing PCM rate.

Original code set constraints list of PCM rate limits at set_sysclk.
This strategy works well if system is using fixed rate clock.

But some boards and SoC (such as RockPro64 and RockChip I2S) has
connected SoC MCLK out to ES8316 MCLK in. In this case, SoC side I2S
will choose suitable frequency of MCLK such as fs * mclk-fs when
user starts playing or capturing.

Bad scenario as follows (mclk-fs = 256):
  - Initialize sysclk by correct value (Ex. 12.288MHz)
- ES8316 set constraints of PCM rate by sysclk
  48kHz (1/256), 32kHz (1/384), 30.720kHz (1/400),
  24kHz (1/512), 16kHz (1/768), 12kHz (1/1024)
  - Play 48kHz sound, it's acceptable
  - Sysclk is not changed

  - Play 32kHz sound, it's acceptable
  - Set sysclk by 8.192MHz (= fs * mclk-fs = 32k * 256)
- ES8316 set constraints of PCM rate by sysclk
  32kHz (1/256), 21.33kHz (1/384), 20.48kHz (1/400),
  16kHz (1/512), 10.66kHz (1/768), 8kHz (1/1024)

  - Play 48kHz again, but it's NOT acceptable because constraints
list does not allow 48kHz

Root cause of this strange behavior is changing constraints list at
set_sysclk timing. It seems that is too early to determine. So this
patch does not use constraints list and check PCM rate limit more
later timing at hw_params.

Signed-off-by: Katsuhiro Suzuki 
---
 sound/soc/codecs/es8316.c | 37 +++--
 1 file changed, 15 insertions(+), 22 deletions(-)

diff --git a/sound/soc/codecs/es8316.c b/sound/soc/codecs/es8316.c
index ed2959dbe1fb..229808fa627c 100644
--- a/sound/soc/codecs/es8316.c
+++ b/sound/soc/codecs/es8316.c
@@ -363,27 +363,12 @@ static int es8316_set_dai_sysclk(struct snd_soc_dai 
*codec_dai,
 {
struct snd_soc_component *component = codec_dai->component;
struct es8316_priv *es8316 = snd_soc_component_get_drvdata(component);
-   int i;
-   int count = 0;
 
es8316->sysclk = freq;
 
if (freq == 0)
return 0;
 
-   /* Limit supported sample rates to ones that can be autodetected
-* by the codec running in slave mode.
-*/
-   for (i = 0; i < NR_SUPPORTED_MCLK_LRCK_RATIOS; i++) {
-   const unsigned int ratio = supported_mclk_lrck_ratios[i];
-
-   if (freq % ratio == 0)
-   es8316->allowed_rates[count++] = freq / ratio;
-   }
-
-   es8316->sysclk_constraints.list = es8316->allowed_rates;
-   es8316->sysclk_constraints.count = count;
-
return 0;
 }
 
@@ -449,13 +434,6 @@ static int es8316_pcm_startup(struct snd_pcm_substream 
*substream,
return -EINVAL;
}
 
-   /* The set of sample rates that can be supported depends on the
-* MCLK supplied to the CODEC.
-*/
-   snd_pcm_hw_constraint_list(substream->runtime, 0,
-  SNDRV_PCM_HW_PARAM_RATE,
-  >sysclk_constraints);
-
return 0;
 }
 
@@ -466,12 +444,27 @@ static int es8316_pcm_hw_params(struct snd_pcm_substream 
*substream,
struct snd_soc_component *component = dai->component;
struct es8316_priv *es8316 = snd_soc_component_get_drvdata(component);
u8 wordlen = 0;
+   int i;
 
if (!es8316->sysclk) {
dev_err(component->dev, "No MCLK configured\n");
return -EINVAL;
}
 
+   /* Limit supported sample rates to ones that can be autodetected
+* by the codec running in slave mode.
+*/
+   for (i = 0; i < NR_SUPPORTED_MCLK_LRCK_RATIOS; i++) {
+   const unsigned int ratio = supported_mclk_lrck_ratios[i];
+
+   if (es8316->sysclk % ratio != 0)
+   continue;
+   if (es8316->sysclk / ratio == params_rate(params))
+   break;
+   }
+   if (i == NR_SUPPORTED_MCLK_LRCK_RATIOS)
+   return -EINVAL;
+
switch (params_format(params)) {
case SNDRV_PCM_FORMAT_S16_LE:
wordlen = ES8316_SERDATA2_LEN_16;
-- 
2.23.0.rc1



[PATCH v3 2/4] ASoC: es8316: add clock control of MCLK

2019-09-03 Thread Katsuhiro Suzuki
This patch introduce clock property for MCLK master freq control.
Driver will set rate of MCLK master if set_sysclk is called and
changing sysclk by board driver.

Signed-off-by: Katsuhiro Suzuki 

---

Changes from v1:
  - Output logs if clock error is found
  - Disable and unprepare mclk when remove this driver
---
 sound/soc/codecs/es8316.c | 35 +++
 1 file changed, 35 insertions(+)

diff --git a/sound/soc/codecs/es8316.c b/sound/soc/codecs/es8316.c
index 229808fa627c..ab41f2c056bd 100644
--- a/sound/soc/codecs/es8316.c
+++ b/sound/soc/codecs/es8316.c
@@ -9,6 +9,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -33,6 +34,7 @@ static const unsigned int supported_mclk_lrck_ratios[] = {
 
 struct es8316_priv {
struct mutex lock;
+   struct clk *mclk;
struct regmap *regmap;
struct snd_soc_component *component;
struct snd_soc_jack *jack;
@@ -363,12 +365,19 @@ static int es8316_set_dai_sysclk(struct snd_soc_dai 
*codec_dai,
 {
struct snd_soc_component *component = codec_dai->component;
struct es8316_priv *es8316 = snd_soc_component_get_drvdata(component);
+   int ret;
 
es8316->sysclk = freq;
 
if (freq == 0)
return 0;
 
+   if (es8316->mclk) {
+   ret = clk_set_rate(es8316->mclk, freq);
+   if (ret)
+   return ret;
+   }
+
return 0;
 }
 
@@ -693,9 +702,26 @@ static int es8316_set_jack(struct snd_soc_component 
*component,
 static int es8316_probe(struct snd_soc_component *component)
 {
struct es8316_priv *es8316 = snd_soc_component_get_drvdata(component);
+   int ret;
 
es8316->component = component;
 
+   es8316->mclk = devm_clk_get(component->dev, "mclk");
+   if (PTR_ERR(es8316->mclk) == -EPROBE_DEFER)
+   return -EPROBE_DEFER;
+   if (IS_ERR(es8316->mclk)) {
+   dev_err(component->dev, "clock is invalid, ignored\n");
+   es8316->mclk = NULL;
+   }
+
+   if (es8316->mclk) {
+   ret = clk_prepare_enable(es8316->mclk);
+   if (ret) {
+   dev_err(component->dev, "unable to enable clock\n");
+   return ret;
+   }
+   }
+
/* Reset codec and enable current state machine */
snd_soc_component_write(component, ES8316_RESET, 0x3f);
usleep_range(5000, 5500);
@@ -718,8 +744,17 @@ static int es8316_probe(struct snd_soc_component 
*component)
return 0;
 }
 
+static void es8316_remove(struct snd_soc_component *component)
+{
+   struct es8316_priv *es8316 = snd_soc_component_get_drvdata(component);
+
+   if (es8316->mclk)
+   clk_disable_unprepare(es8316->mclk);
+}
+
 static const struct snd_soc_component_driver soc_component_dev_es8316 = {
.probe  = es8316_probe,
+   .remove = es8316_remove,
.set_jack   = es8316_set_jack,
.controls   = es8316_snd_controls,
.num_controls   = ARRAY_SIZE(es8316_snd_controls),
-- 
2.23.0.rc1



Re: [PATCH v2 1/3] ASoC: es8316: judge PCM rate at later timing

2019-09-03 Thread Katsuhiro Suzuki

Hello Mark,

On 2019/09/03 20:11, Mark Brown wrote:

On Tue, Sep 03, 2019 at 04:19:10AM +0900, Katsuhiro Suzuki wrote:

On 2019/09/02 21:02, Mark Brown wrote:



The best way to handle this is to try to support both fixed and variable
clock rates, some other drivers do this by setting constraints based on
MCLK only if the MCLK has been set to a non-zero value.  They have the
machine drivers reset the clock rate to 0 when it goes idle so that no
constraints are applied then.  This means that if the machine has a



In my understanding, fixed and variable clock both use set_sysclk() for
telling their MCLK to codec driver. For fixed MCLK cases we need to
apply constraint but for variable MCLK cases we should not set
constraints at set_sysclk(). How can we identify these two cases...?


Like I say it's usually done by settingthe MCLK to 0 when not in use and
then not applying any constraints if there's no MCLK set.



Ah... I understand. Current implementation refuses MCLK == 0.
I'll change to accept MCLK == 0 for fixed clock users and send v3 patch.

Best Regards,
Katsuhiro Suzuki


Re: [PATCH v2 1/3] ASoC: es8316: judge PCM rate at later timing

2019-09-02 Thread Katsuhiro Suzuki

Hello Mark,

Thanks a lot for your comments.

On 2019/09/02 21:02, Mark Brown wrote:

On Sun, Sep 01, 2019 at 01:26:48AM +0900, Katsuhiro Suzuki wrote:

This patch change the judge timing about playing/capturing PCM rate.

Original code set constraints list of PCM rate limits at set_sysclk.
This strategy works well if system is using fixed rate clock.

But some boards and SoC (such as RockPro64 and RockChip I2S) has
connected SoC MCLK out to ES8316 MCLK in. In this case, SoC side I2S
will choose suitable frequency of MCLK such as fs * mclk-fs when
user starts playing or capturing.


The best way to handle this is to try to support both fixed and variable
clock rates, some other drivers do this by setting constraints based on
MCLK only if the MCLK has been set to a non-zero value.  They have the
machine drivers reset the clock rate to 0 when it goes idle so that no
constraints are applied then.  This means that if the machine has a
fixed clock there will be constraints, and that constraints get applied
if one direction has started and fixed the clock, but still allows the
clock to be varied where possible.



In my understanding, fixed and variable clock both use set_sysclk() for 
telling their MCLK to codec driver. For fixed MCLK cases we need to

apply constraint but for variable MCLK cases we should not set
constraints at set_sysclk(). How can we identify these two cases...?

For example, if machine sets very low MCLK once, the driver applies low
Fs constraints which I2S driver cannot support to. After that this sound
card cannot play/capture any Fs rate. It seems set_sysclk() is not
called if Fs does not match constraints. So we have no chance to
reconfigure MCLK by set_sysclk().


Best Regards,
Katsuhiro Suzuki


[PATCH v2 2/3] ASoC: es8316: Add clock control of MCLK

2019-08-31 Thread Katsuhiro Suzuki
This patch introduce clock property for MCLK master freq control.
Driver will set rate of MCLK master if set_sysclk is called and
changing sysclk by board driver.

Signed-off-by: Katsuhiro Suzuki 

---

Changes from v1:
  - Output logs if clock error is found
  - Disable and unprepare mclk when remove this driver
---
 sound/soc/codecs/es8316.c | 35 +++
 1 file changed, 35 insertions(+)

diff --git a/sound/soc/codecs/es8316.c b/sound/soc/codecs/es8316.c
index 229808fa627c..ab41f2c056bd 100644
--- a/sound/soc/codecs/es8316.c
+++ b/sound/soc/codecs/es8316.c
@@ -9,6 +9,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -33,6 +34,7 @@ static const unsigned int supported_mclk_lrck_ratios[] = {
 
 struct es8316_priv {
struct mutex lock;
+   struct clk *mclk;
struct regmap *regmap;
struct snd_soc_component *component;
struct snd_soc_jack *jack;
@@ -363,12 +365,19 @@ static int es8316_set_dai_sysclk(struct snd_soc_dai 
*codec_dai,
 {
struct snd_soc_component *component = codec_dai->component;
struct es8316_priv *es8316 = snd_soc_component_get_drvdata(component);
+   int ret;
 
es8316->sysclk = freq;
 
if (freq == 0)
return 0;
 
+   if (es8316->mclk) {
+   ret = clk_set_rate(es8316->mclk, freq);
+   if (ret)
+   return ret;
+   }
+
return 0;
 }
 
@@ -693,9 +702,26 @@ static int es8316_set_jack(struct snd_soc_component 
*component,
 static int es8316_probe(struct snd_soc_component *component)
 {
struct es8316_priv *es8316 = snd_soc_component_get_drvdata(component);
+   int ret;
 
es8316->component = component;
 
+   es8316->mclk = devm_clk_get(component->dev, "mclk");
+   if (PTR_ERR(es8316->mclk) == -EPROBE_DEFER)
+   return -EPROBE_DEFER;
+   if (IS_ERR(es8316->mclk)) {
+   dev_err(component->dev, "clock is invalid, ignored\n");
+   es8316->mclk = NULL;
+   }
+
+   if (es8316->mclk) {
+   ret = clk_prepare_enable(es8316->mclk);
+   if (ret) {
+   dev_err(component->dev, "unable to enable clock\n");
+   return ret;
+   }
+   }
+
/* Reset codec and enable current state machine */
snd_soc_component_write(component, ES8316_RESET, 0x3f);
usleep_range(5000, 5500);
@@ -718,8 +744,17 @@ static int es8316_probe(struct snd_soc_component 
*component)
return 0;
 }
 
+static void es8316_remove(struct snd_soc_component *component)
+{
+   struct es8316_priv *es8316 = snd_soc_component_get_drvdata(component);
+
+   if (es8316->mclk)
+   clk_disable_unprepare(es8316->mclk);
+}
+
 static const struct snd_soc_component_driver soc_component_dev_es8316 = {
.probe  = es8316_probe,
+   .remove = es8316_remove,
.set_jack   = es8316_set_jack,
.controls   = es8316_snd_controls,
.num_controls   = ARRAY_SIZE(es8316_snd_controls),
-- 
2.23.0.rc1



[PATCH v2 3/3] ASoC: es8316: add DT-bindings

2019-08-31 Thread Katsuhiro Suzuki
This patch adds missing DT-bindings document for Everest ES8316.

Signed-off-by: Katsuhiro Suzuki 
---
 .../bindings/sound/everest,es8316.txt | 20 +++
 1 file changed, 20 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/everest,es8316.txt

diff --git a/Documentation/devicetree/bindings/sound/everest,es8316.txt 
b/Documentation/devicetree/bindings/sound/everest,es8316.txt
new file mode 100644
index ..aefcff9c48a2
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/everest,es8316.txt
@@ -0,0 +1,20 @@
+Everest ES8316 audio CODEC
+
+This device supports both I2C and SPI.
+
+Required properties:
+
+  - compatible  : should be "everest,es8316"
+  - reg : the I2C address of the device for I2C
+  - clocks : a list of phandle, should contain entries for clock-names
+  - clock-names : should include as follows:
+ "mclk" : master clock (MCLK) of the device
+
+Example:
+
+es8316: codec@11 {
+   compatible = "everest,es8316";
+   reg = <0x11>;
+   clocks = < 10>;
+   clock-names = "mclk";
+};
-- 
2.23.0.rc1



[PATCH v2 1/3] ASoC: es8316: judge PCM rate at later timing

2019-08-31 Thread Katsuhiro Suzuki
This patch change the judge timing about playing/capturing PCM rate.

Original code set constraints list of PCM rate limits at set_sysclk.
This strategy works well if system is using fixed rate clock.

But some boards and SoC (such as RockPro64 and RockChip I2S) has
connected SoC MCLK out to ES8316 MCLK in. In this case, SoC side I2S
will choose suitable frequency of MCLK such as fs * mclk-fs when
user starts playing or capturing.

Bad scenario as follows (mclk-fs = 256):
  - Initialize sysclk by correct value (Ex. 12.288MHz)
- ES8316 set constraints of PCM rate by sysclk
  48kHz (1/256), 32kHz (1/384), 30.720kHz (1/400),
  24kHz (1/512), 16kHz (1/768), 12kHz (1/1024)
  - Play 48kHz sound, it's acceptable
  - Sysclk is not changed

  - Play 32kHz sound, it's acceptable
  - Set sysclk by 8.192MHz (= fs * mclk-fs = 32k * 256)
- ES8316 set constraints of PCM rate by sysclk
  32kHz (1/256), 21.33kHz (1/384), 20.48kHz (1/400),
  16kHz (1/512), 10.66kHz (1/768), 8kHz (1/1024)

  - Play 48kHz again, but it's NOT acceptable because constraints
list does not allow 48kHz

Root cause of this strange behavior is changing constraints list at
set_sysclk timing. It seems that is too early to determine. So this
patch does not use constraints list and check PCM rate limit more
later timing at hw_params.

Signed-off-by: Katsuhiro Suzuki 
---
 sound/soc/codecs/es8316.c | 37 +++--
 1 file changed, 15 insertions(+), 22 deletions(-)

diff --git a/sound/soc/codecs/es8316.c b/sound/soc/codecs/es8316.c
index ed2959dbe1fb..229808fa627c 100644
--- a/sound/soc/codecs/es8316.c
+++ b/sound/soc/codecs/es8316.c
@@ -363,27 +363,12 @@ static int es8316_set_dai_sysclk(struct snd_soc_dai 
*codec_dai,
 {
struct snd_soc_component *component = codec_dai->component;
struct es8316_priv *es8316 = snd_soc_component_get_drvdata(component);
-   int i;
-   int count = 0;
 
es8316->sysclk = freq;
 
if (freq == 0)
return 0;
 
-   /* Limit supported sample rates to ones that can be autodetected
-* by the codec running in slave mode.
-*/
-   for (i = 0; i < NR_SUPPORTED_MCLK_LRCK_RATIOS; i++) {
-   const unsigned int ratio = supported_mclk_lrck_ratios[i];
-
-   if (freq % ratio == 0)
-   es8316->allowed_rates[count++] = freq / ratio;
-   }
-
-   es8316->sysclk_constraints.list = es8316->allowed_rates;
-   es8316->sysclk_constraints.count = count;
-
return 0;
 }
 
@@ -449,13 +434,6 @@ static int es8316_pcm_startup(struct snd_pcm_substream 
*substream,
return -EINVAL;
}
 
-   /* The set of sample rates that can be supported depends on the
-* MCLK supplied to the CODEC.
-*/
-   snd_pcm_hw_constraint_list(substream->runtime, 0,
-  SNDRV_PCM_HW_PARAM_RATE,
-  >sysclk_constraints);
-
return 0;
 }
 
@@ -466,12 +444,27 @@ static int es8316_pcm_hw_params(struct snd_pcm_substream 
*substream,
struct snd_soc_component *component = dai->component;
struct es8316_priv *es8316 = snd_soc_component_get_drvdata(component);
u8 wordlen = 0;
+   int i;
 
if (!es8316->sysclk) {
dev_err(component->dev, "No MCLK configured\n");
return -EINVAL;
}
 
+   /* Limit supported sample rates to ones that can be autodetected
+* by the codec running in slave mode.
+*/
+   for (i = 0; i < NR_SUPPORTED_MCLK_LRCK_RATIOS; i++) {
+   const unsigned int ratio = supported_mclk_lrck_ratios[i];
+
+   if (es8316->sysclk % ratio != 0)
+   continue;
+   if (es8316->sysclk / ratio == params_rate(params))
+   break;
+   }
+   if (i == NR_SUPPORTED_MCLK_LRCK_RATIOS)
+   return -EINVAL;
+
switch (params_format(params)) {
case SNDRV_PCM_FORMAT_S16_LE:
wordlen = ES8316_SERDATA2_LEN_16;
-- 
2.23.0.rc1



Re: [alsa-devel] [PATCH 2/3] ASoC: es8316: Add clock control of MCLK

2019-08-31 Thread Katsuhiro Suzuki

Hello Mark,

On 2019/08/30 20:18, Mark Brown wrote:

On Fri, Aug 30, 2019 at 02:32:04AM +0900, Katsuhiro Suzuki wrote:


+   es8316->mclk = devm_clk_get(component->dev, "mclk");
+   if (PTR_ERR(es8316->mclk) == -EPROBE_DEFER)
+   return -EPROBE_DEFER;


If we don't get a clock it'd be nice to at least log that in case
there's something wrong with the clock driver so that people have more
of a hint as to why things might be breaking.



OK, to change more user friendly.



+
+   if (es8316->mclk) {
+   ret = clk_prepare_enable(es8316->mclk);
+   if (ret)
+   return ret;
+   }
+


There's nothing that disables the clock on remove.

Otherwise this looks good.


Thank you for reviewing. I'll fix it and send V2 patch set.




___
Alsa-devel mailing list
alsa-de...@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel



Best Regards,
Katsuhiro Suzuki


[PATCH 2/3] ASoC: es8316: Add clock control of MCLK

2019-08-29 Thread Katsuhiro Suzuki
This patch introduce clock property for MCLK master freq control.
Driver will set rate of MCLK master if set_sysclk is called and
changing sysclk by board driver.

Signed-off-by: Katsuhiro Suzuki 
---
 sound/soc/codecs/es8316.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/sound/soc/codecs/es8316.c b/sound/soc/codecs/es8316.c
index 229808fa627c..9ed564eac202 100644
--- a/sound/soc/codecs/es8316.c
+++ b/sound/soc/codecs/es8316.c
@@ -9,6 +9,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -33,6 +34,7 @@ static const unsigned int supported_mclk_lrck_ratios[] = {
 
 struct es8316_priv {
struct mutex lock;
+   struct clk *mclk;
struct regmap *regmap;
struct snd_soc_component *component;
struct snd_soc_jack *jack;
@@ -363,12 +365,19 @@ static int es8316_set_dai_sysclk(struct snd_soc_dai 
*codec_dai,
 {
struct snd_soc_component *component = codec_dai->component;
struct es8316_priv *es8316 = snd_soc_component_get_drvdata(component);
+   int ret;
 
es8316->sysclk = freq;
 
if (freq == 0)
return 0;
 
+   if (es8316->mclk) {
+   ret = clk_set_rate(es8316->mclk, freq);
+   if (ret)
+   return ret;
+   }
+
return 0;
 }
 
@@ -693,9 +702,20 @@ static int es8316_set_jack(struct snd_soc_component 
*component,
 static int es8316_probe(struct snd_soc_component *component)
 {
struct es8316_priv *es8316 = snd_soc_component_get_drvdata(component);
+   int ret;
 
es8316->component = component;
 
+   es8316->mclk = devm_clk_get(component->dev, "mclk");
+   if (PTR_ERR(es8316->mclk) == -EPROBE_DEFER)
+   return -EPROBE_DEFER;
+
+   if (es8316->mclk) {
+   ret = clk_prepare_enable(es8316->mclk);
+   if (ret)
+   return ret;
+   }
+
/* Reset codec and enable current state machine */
snd_soc_component_write(component, ES8316_RESET, 0x3f);
usleep_range(5000, 5500);
-- 
2.23.0.rc1



[PATCH 1/3] ASoC: es8316: judge PCM rate at later timing

2019-08-29 Thread Katsuhiro Suzuki
This patch change the judge timing about playing/capturing PCM rate.

Original code set constraints list of PCM rate limits at set_sysclk.
This strategy works well if system is using fixed rate clock.

But some boards and SoC (such as RockPro64 and RockChip I2S) has
connected SoC MCLK out to ES8316 MCLK in. In this case, SoC side I2S
will choose suitable frequency of MCLK such as fs * mclk-fs when
user starts playing or capturing.

Bad scenario as follows (mclk-fs = 256):
  - Initialize sysclk by correct value (Ex. 12.288MHz)
- ES8316 set constraints of PCM rate by sysclk
  48kHz (1/256), 32kHz (1/384), 30.720kHz (1/400),
  24kHz (1/512), 16kHz (1/768), 12kHz (1/1024)
  - Play 48kHz sound, it's acceptable
  - Sysclk is not changed

  - Play 32kHz sound, it's acceptable
  - Set sysclk by 8.192MHz (= fs * mclk-fs = 32k * 256)
- ES8316 set constraints of PCM rate by sysclk
  32kHz (1/256), 21.33kHz (1/384), 20.48kHz (1/400),
  16kHz (1/512), 10.66kHz (1/768), 8kHz (1/1024)

  - Play 48kHz again, but it's NOT acceptable because constraints
list does not allow 48kHz

Root cause of this strange behavior is changing constraints list at
set_sysclk timing. It seems that is too early to determine. So this
patch does not use constraints list and check PCM rate limit more
later timing at hw_params.

Signed-off-by: Katsuhiro Suzuki 
---
 sound/soc/codecs/es8316.c | 37 +++--
 1 file changed, 15 insertions(+), 22 deletions(-)

diff --git a/sound/soc/codecs/es8316.c b/sound/soc/codecs/es8316.c
index ed2959dbe1fb..229808fa627c 100644
--- a/sound/soc/codecs/es8316.c
+++ b/sound/soc/codecs/es8316.c
@@ -363,27 +363,12 @@ static int es8316_set_dai_sysclk(struct snd_soc_dai 
*codec_dai,
 {
struct snd_soc_component *component = codec_dai->component;
struct es8316_priv *es8316 = snd_soc_component_get_drvdata(component);
-   int i;
-   int count = 0;
 
es8316->sysclk = freq;
 
if (freq == 0)
return 0;
 
-   /* Limit supported sample rates to ones that can be autodetected
-* by the codec running in slave mode.
-*/
-   for (i = 0; i < NR_SUPPORTED_MCLK_LRCK_RATIOS; i++) {
-   const unsigned int ratio = supported_mclk_lrck_ratios[i];
-
-   if (freq % ratio == 0)
-   es8316->allowed_rates[count++] = freq / ratio;
-   }
-
-   es8316->sysclk_constraints.list = es8316->allowed_rates;
-   es8316->sysclk_constraints.count = count;
-
return 0;
 }
 
@@ -449,13 +434,6 @@ static int es8316_pcm_startup(struct snd_pcm_substream 
*substream,
return -EINVAL;
}
 
-   /* The set of sample rates that can be supported depends on the
-* MCLK supplied to the CODEC.
-*/
-   snd_pcm_hw_constraint_list(substream->runtime, 0,
-  SNDRV_PCM_HW_PARAM_RATE,
-  >sysclk_constraints);
-
return 0;
 }
 
@@ -466,12 +444,27 @@ static int es8316_pcm_hw_params(struct snd_pcm_substream 
*substream,
struct snd_soc_component *component = dai->component;
struct es8316_priv *es8316 = snd_soc_component_get_drvdata(component);
u8 wordlen = 0;
+   int i;
 
if (!es8316->sysclk) {
dev_err(component->dev, "No MCLK configured\n");
return -EINVAL;
}
 
+   /* Limit supported sample rates to ones that can be autodetected
+* by the codec running in slave mode.
+*/
+   for (i = 0; i < NR_SUPPORTED_MCLK_LRCK_RATIOS; i++) {
+   const unsigned int ratio = supported_mclk_lrck_ratios[i];
+
+   if (es8316->sysclk % ratio != 0)
+   continue;
+   if (es8316->sysclk / ratio == params_rate(params))
+   break;
+   }
+   if (i == NR_SUPPORTED_MCLK_LRCK_RATIOS)
+   return -EINVAL;
+
switch (params_format(params)) {
case SNDRV_PCM_FORMAT_S16_LE:
wordlen = ES8316_SERDATA2_LEN_16;
-- 
2.23.0.rc1



[PATCH 3/3] ASoC: es8316: add DT-bindings

2019-08-29 Thread Katsuhiro Suzuki
This patch adds missing DT-bindings document for Everest ES8316.

Signed-off-by: Katsuhiro Suzuki 
---
 .../bindings/sound/everest,es8316.txt | 20 +++
 1 file changed, 20 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/everest,es8316.txt

diff --git a/Documentation/devicetree/bindings/sound/everest,es8316.txt 
b/Documentation/devicetree/bindings/sound/everest,es8316.txt
new file mode 100644
index ..aefcff9c48a2
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/everest,es8316.txt
@@ -0,0 +1,20 @@
+Everest ES8316 audio CODEC
+
+This device supports both I2C and SPI.
+
+Required properties:
+
+  - compatible  : should be "everest,es8316"
+  - reg : the I2C address of the device for I2C
+  - clocks : a list of phandle, should contain entries for clock-names
+  - clock-names : should include as follows:
+ "mclk" : master clock (MCLK) of the device
+
+Example:
+
+es8316: codec@11 {
+   compatible = "everest,es8316";
+   reg = <0x11>;
+   clocks = < 10>;
+   clock-names = "mclk";
+};
-- 
2.23.0.rc1



[PATCH v2 1/2] ASoC: es8316: fix headphone mixer volume table

2019-08-26 Thread Katsuhiro Suzuki
This patch fix setting table of Headphone mixer volume.
Current code uses 4 ... 7 values but these values are prohibited.

Correct settings are the following:
   -12dB
  0001 -10.5dB
  0010 -9dB
  0011 -7.5dB
  0100 -6dB
  1000 -4.5dB
  1001 -3dB
  1010 -1.5dB
  1011 0dB

Signed-off-by: Katsuhiro Suzuki 
---
 sound/soc/codecs/es8316.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/es8316.c b/sound/soc/codecs/es8316.c
index 8dfb5dbeebbf..f97d9c5210f0 100644
--- a/sound/soc/codecs/es8316.c
+++ b/sound/soc/codecs/es8316.c
@@ -53,7 +53,10 @@ static const SNDRV_CTL_TLVD_DECLARE_DB_SCALE(adc_vol_tlv, 
-9600, 50, 1);
 static const SNDRV_CTL_TLVD_DECLARE_DB_SCALE(alc_max_gain_tlv, -650, 150, 0);
 static const SNDRV_CTL_TLVD_DECLARE_DB_SCALE(alc_min_gain_tlv, -1200, 150, 0);
 static const SNDRV_CTL_TLVD_DECLARE_DB_SCALE(alc_target_tlv, -1650, 150, 0);
-static const SNDRV_CTL_TLVD_DECLARE_DB_SCALE(hpmixer_gain_tlv, -1200, 150, 0);
+static const SNDRV_CTL_TLVD_DECLARE_DB_RANGE(hpmixer_gain_tlv,
+   0, 4, TLV_DB_SCALE_ITEM(-1200, 150, 0),
+   8, 11, TLV_DB_SCALE_ITEM(-450, 150, 0),
+);
 
 static const SNDRV_CTL_TLVD_DECLARE_DB_RANGE(adc_pga_gain_tlv,
0, 0, TLV_DB_SCALE_ITEM(-350, 0, 0),
@@ -91,7 +94,7 @@ static const struct snd_kcontrol_new es8316_snd_controls[] = {
SOC_DOUBLE_TLV("Headphone Playback Volume", ES8316_CPHP_ICAL_VOL,
   4, 0, 3, 1, hpout_vol_tlv),
SOC_DOUBLE_TLV("Headphone Mixer Volume", ES8316_HPMIX_VOL,
-  0, 4, 7, 0, hpmixer_gain_tlv),
+  0, 4, 11, 0, hpmixer_gain_tlv),
 
SOC_ENUM("Playback Polarity", dacpol),
SOC_DOUBLE_R_TLV("DAC Playback Volume", ES8316_DAC_VOLL,
-- 
2.23.0.rc1



[PATCH v2 2/2] ASoC: es8316: fix inverted L/R of headphone mixer volume

2019-08-26 Thread Katsuhiro Suzuki
This patch fixes inverted Left-Right channel of headphone mixer
volume by wrong shift_left, shift_right values.

Signed-off-by: Katsuhiro Suzuki 
---
 sound/soc/codecs/es8316.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/codecs/es8316.c b/sound/soc/codecs/es8316.c
index f97d9c5210f0..e0da24611800 100644
--- a/sound/soc/codecs/es8316.c
+++ b/sound/soc/codecs/es8316.c
@@ -94,7 +94,7 @@ static const struct snd_kcontrol_new es8316_snd_controls[] = {
SOC_DOUBLE_TLV("Headphone Playback Volume", ES8316_CPHP_ICAL_VOL,
   4, 0, 3, 1, hpout_vol_tlv),
SOC_DOUBLE_TLV("Headphone Mixer Volume", ES8316_HPMIX_VOL,
-  0, 4, 11, 0, hpmixer_gain_tlv),
+  4, 0, 11, 0, hpmixer_gain_tlv),
 
SOC_ENUM("Playback Polarity", dacpol),
SOC_DOUBLE_R_TLV("DAC Playback Volume", ES8316_DAC_VOLL,
-- 
2.23.0.rc1



Re: [PATCH] ASoC: es8316: limit headphone mixer volume

2019-08-26 Thread Katsuhiro Suzuki

Hello Hans, Daniel,

Thank you for reviewing and comment.

On 2019/08/26 18:09, Hans de Goede wrote:

Hi,

On 26-08-19 04:53, Daniel Drake wrote:
On Mon, Aug 26, 2019 at 1:38 AM Hans de Goede  
wrote:

On 24-08-19 23:04, Katsuhiro Suzuki wrote:

This patch limits Headphone mixer volume to 4 from 7.
Because output sound suddenly becomes very loudly with many noise if
set volume over 4.


That sounds like something that should be limited in UCM.


Higher then 4 not working matches my experience, see this comment from
the UCM file: alsa-lib/src/conf/ucm/codecs/es8316/EnableSeq.conf :

# Set HP mixer vol to -6 dB (4/7) louder does not work
cset "name='Headphone Mixer Volume' 4"


What does "does not work" mean more precisely?


IIRC garbled sound.

I checked the spec, there is indeed something wrong in the kernel 
driver here.

The db scale is not a simple scale as the kernel source suggests.

Instead it is:
 – -12dB
0001 – -10.5dB
0010 – -9dB
0011 – -7.5dB
0100 – -6dB
1000 – -4.5dB
1001 – -3dB
1010 – -1.5dB
1011 – 0dB

So perhaps we can fix the kernel to follow this table and then use UCM
to limit the volume if its too high on a given platform?


Yes that sounds like the right thing to do. Katsuhiro can you confirm that
using this table allows using the full scale ? note that the full scale now
has 9 steps rather then 8.



I've finished testing this table on my board (RockPro64).
Every values work well without garbled sound.

I checked address 0x16 register via /sys/kernel/debug/regmap too.
The register values and dB (get from alsamixer) are the following if
I increase headphone volume to max from min.

  reg   dB
  0x16  scale
  
  0x00  -12.00
  0x11  -10.50
  0x22  -9.00
  0x33  -7.50
  0x44  -6.00
  0x88  -4.50
  0x99  -3.00
  0xaa  -1.50
  0xbb  0.00


And I found other problem, current code is inverted L/R volume.
It's only in Headphone "mixer" volume. It seems Headphone "master"
volume works correctly.

I'll fix these problems and send V2 patch set.



Regards,

Hans




Best Regards,
Katsuhiro Suzuki


Re: [PATCH] ASoC: es8316: limit headphone mixer volume

2019-08-26 Thread Katsuhiro Suzuki

Hello,

Oops... I got mistake,

>  SOC_DOUBLE_TLV("Headphone Mixer Volume", ES8316_HPMIX_VOL,
> 0, 4, 15, 0, hpmixer_gain_tlv),

is wrong,

>  SOC_DOUBLE_TLV("Headphone Mixer Volume", ES8316_HPMIX_VOL,
> 0, 4, 11, 0, hpmixer_gain_tlv),

is correct.

Best Regards,
Katsuhiro Suzuki


On 2019/08/26 17:41, Katsuhiro Suzuki wrote:

Hello Daniel,

On 2019/08/26 11:53, Daniel Drake wrote:
On Mon, Aug 26, 2019 at 1:38 AM Hans de Goede  
wrote:

On 24-08-19 23:04, Katsuhiro Suzuki wrote:

This patch limits Headphone mixer volume to 4 from 7.
Because output sound suddenly becomes very loudly with many noise if
set volume over 4.


That sounds like something that should be limited in UCM.


Higher then 4 not working matches my experience, see this comment from
the UCM file: alsa-lib/src/conf/ucm/codecs/es8316/EnableSeq.conf :

# Set HP mixer vol to -6 dB (4/7) louder does not work
cset "name='Headphone Mixer Volume' 4"


What does "does not work" mean more precisely?

I checked the spec, there is indeed something wrong in the kernel 
driver here.

The db scale is not a simple scale as the kernel source suggests.

Instead it is:
 – -12dB
0001 – -10.5dB
0010 – -9dB
0011 – -7.5dB
0100 – -6dB
1000 – -4.5dB
1001 – -3dB
1010 – -1.5dB
1011 – 0dB


 > So perhaps we can fix the kernel to follow this table and then use UCM
 > to limit the volume if its too high on a given platform?
 >

Thank you very important information. So you mean value 5, 6, 7 are
illegal settings for ES8316. Correct codes are

static const SNDRV_CTL_TLVD_DECLARE_DB_RANGE(hpmixer_gain_tlv,
 0, 4, TLV_DB_SCALE_ITEM(-1200, 150, 0),
 8, 11, TLV_DB_SCALE_ITEM(-450, 150, 0),
);

and...

 SOC_DOUBLE_TLV("Headphone Mixer Volume", ES8316_HPMIX_VOL,
    0, 4, 15, 0, hpmixer_gain_tlv),

Is my understanding correct? If so I'll test it on my board
(RockPro64) and re-send patch.

BTW, do you know how to get ES8316 I2C registers spec?
I want to see it for understanding current code, but I cannot find...



Thanks
Daniel




Best Regards,
Katsuhiro Suzuki





Re: [PATCH] ASoC: es8316: limit headphone mixer volume

2019-08-26 Thread Katsuhiro Suzuki

Hello Daniel,

On 2019/08/26 11:53, Daniel Drake wrote:

On Mon, Aug 26, 2019 at 1:38 AM Hans de Goede  wrote:

On 24-08-19 23:04, Katsuhiro Suzuki wrote:

This patch limits Headphone mixer volume to 4 from 7.
Because output sound suddenly becomes very loudly with many noise if
set volume over 4.


That sounds like something that should be limited in UCM.


Higher then 4 not working matches my experience, see this comment from
the UCM file: alsa-lib/src/conf/ucm/codecs/es8316/EnableSeq.conf :

# Set HP mixer vol to -6 dB (4/7) louder does not work
cset "name='Headphone Mixer Volume' 4"


What does "does not work" mean more precisely?

I checked the spec, there is indeed something wrong in the kernel driver here.
The db scale is not a simple scale as the kernel source suggests.

Instead it is:
 – -12dB
0001 – -10.5dB
0010 – -9dB
0011 – -7.5dB
0100 – -6dB
1000 – -4.5dB
1001 – -3dB
1010 – -1.5dB
1011 – 0dB


> So perhaps we can fix the kernel to follow this table and then use UCM
> to limit the volume if its too high on a given platform?
>

Thank you very important information. So you mean value 5, 6, 7 are
illegal settings for ES8316. Correct codes are

static const SNDRV_CTL_TLVD_DECLARE_DB_RANGE(hpmixer_gain_tlv,
0, 4, TLV_DB_SCALE_ITEM(-1200, 150, 0),
8, 11, TLV_DB_SCALE_ITEM(-450, 150, 0),
);

and...

SOC_DOUBLE_TLV("Headphone Mixer Volume", ES8316_HPMIX_VOL,
   0, 4, 15, 0, hpmixer_gain_tlv),

Is my understanding correct? If so I'll test it on my board
(RockPro64) and re-send patch.

BTW, do you know how to get ES8316 I2C registers spec?
I want to see it for understanding current code, but I cannot find...



Thanks
Daniel




Best Regards,
Katsuhiro Suzuki


[PATCH] ASoC: es8316: limit headphone mixer volume

2019-08-24 Thread Katsuhiro Suzuki
This patch limits Headphone mixer volume to 4 from 7.
Because output sound suddenly becomes very loudly with many noise if
set volume over 4.

Signed-off-by: Katsuhiro Suzuki 
---
 sound/soc/codecs/es8316.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/codecs/es8316.c b/sound/soc/codecs/es8316.c
index 8dfb5dbeebbf..bc4141e1eb7f 100644
--- a/sound/soc/codecs/es8316.c
+++ b/sound/soc/codecs/es8316.c
@@ -91,7 +91,7 @@ static const struct snd_kcontrol_new es8316_snd_controls[] = {
SOC_DOUBLE_TLV("Headphone Playback Volume", ES8316_CPHP_ICAL_VOL,
   4, 0, 3, 1, hpout_vol_tlv),
SOC_DOUBLE_TLV("Headphone Mixer Volume", ES8316_HPMIX_VOL,
-  0, 4, 7, 0, hpmixer_gain_tlv),
+  0, 4, 4, 0, hpmixer_gain_tlv),
 
SOC_ENUM("Playback Polarity", dacpol),
SOC_DOUBLE_R_TLV("DAC Playback Volume", ES8316_DAC_VOLL,
-- 
2.23.0.rc1



Re: [PATCH net-next] net: stmmac: Fix the case when PHY handle is not present

2019-06-25 Thread Katsuhiro Suzuki

Hello Andrew,

On 2019/06/25 23:51, Andrew Lunn wrote:

On Tue, Jun 25, 2019 at 11:40:00PM +0900, Katsuhiro Suzuki wrote:

Hello Jose,

This patch works fine with my Tinker Board. Thanks a lot!

Tested-by: Katsuhiro Suzuki 


BTW, from network guys point of view, is it better to add a phy node
into device trees that have no phy node such as the Tinker Board?


Hi Katsuhiro

It makes it less ambiguous if there is a phy-handle. It is then very
clear which PHY should be used. For a development board, which people
can be tinkering around with, there is a chance they add a second PHY
to the MDIO bus, or an Ethernet switch, etc. Without explicitly
listing the PHY, it might get the wrong one. However this is generally
a problem if phy_find_first() is used. I think in this case, something
is setting priv->plat->phy_addr, so it is also clearly defined which
PHY to use.

  Andrew



Hmm, I see. This stmmac driver can choose PHY by the kernel module
parameter 'phyaddr' (if no one set this parameter, priv->plat->phy_addr
goes to 0). So there is no ambiguous in this case and need no changes
for device tree.

Thank you for your comment.

Best Regards,
Katsuhiro Suzuki


Re: [PATCH net-next] net: stmmac: Fix the case when PHY handle is not present

2019-06-25 Thread Katsuhiro Suzuki

Hello Jose,

This patch works fine with my Tinker Board. Thanks a lot!

Tested-by: Katsuhiro Suzuki 


BTW, from network guys point of view, is it better to add a phy node
into device trees that have no phy node such as the Tinker Board?


Best Regards,
Katsuhiro Suzuki


On 2019/06/25 22:11, Jose Abreu wrote:

++ Katsuhiro

From: Jose Abreu 


Some DT bindings do not have the PHY handle. Let's fallback to manually
discovery in case phylink_of_phy_connect() fails.

Reported-by: Katsuhiro Suzuki 
Fixes: 74371272f97f ("net: stmmac: Convert to phylink and remove phylib logic")
Signed-off-by: Jose Abreu 
Cc: Joao Pinto 
Cc: David S. Miller 
Cc: Giuseppe Cavallaro 
Cc: Alexandre Torgue 
---
Hello Katsuhiro,

Can you please test this patch ?
---
  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 7 +--
  1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index a48751989fa6..f4593d2d9d20 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -950,9 +950,12 @@ static int stmmac_init_phy(struct net_device *dev)
  
  	node = priv->plat->phylink_node;
  
-	if (node) {

+   if (node)
ret = phylink_of_phy_connect(priv->phylink, node, 0);
-   } else {
+
+   /* Some DT bindings do not set-up the PHY handle. Let's try to
+* manually parse it */
+   if (!node || ret) {
int addr = priv->plat->phy_addr;
struct phy_device *phydev;
  
--

2.7.4









stmmac regression on ASUS TinkerBoard

2019-06-23 Thread Katsuhiro Suzuki

Hello stmmac maintainers,

I found this commit and that has some regressions:
  74371272f97f net: stmmac: Convert to phylink and remove phylib logic


My environment is:
  - ASUS TinkerBoard
  - SoC is RK3288
  - Using STMMAC driver
drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
  - Using this device-tree
arch/arm/boot/dts/rk3288.dtsi ('gmac: ethernet@ff29' node)

Current linux-next on my environment, 'ifconfig eth0 up' does not work
correctly with following message...

-
root@linaro-alip:~# ifconfig eth0 up
[  105.028916] rk_gmac-dwmac ff29.ethernet eth0: stmmac_open: Cannot 
attach to PHY (error: -19)

SIOCSIFFLAGS: No such device
-

I checked drivers/net/ethernet/stmicro/stmmac/stmmac_main.c and found
stmmac_init_phy() is going to fail if ethernet device node does not
have following property:
  - phy-handle
  - phy
  - phy-device

This commit broke the device-trees such as TinkerBoard. The mdio
subnode creating a mdio bus is changed to required or still optional?


Best Regards,
Katsuhiro Suzuki


Re: [PATCH] ARM: dts: rockchip: add ethernet phy node for tinker board

2019-06-23 Thread Katsuhiro Suzuki

Hello Heiko, Andrew,

Thank you for comments. I found the commit that has regression:
  74371272f97f net: stmmac: Convert to phylink and remove phylib logic

So I'll report it to netdev and stmmac guys.

Best Regards,
---
Katsuhiro Suzuki


On 2019/06/23 2:55, Andrew Lunn wrote:

On Sat, Jun 22, 2019 at 11:50:10PM +0900, Katsuhiro Suzuki wrote:

Hello,


Hi Katsuhiro

Please also report this to netdev, and the stmmac maintainers.

./scripts/get_maintainer.pl -f drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
Giuseppe Cavallaro  (supporter:STMMAC ETHERNET DRIVER)
Alexandre Torgue  (supporter:STMMAC ETHERNET DRIVER)
Jose Abreu  (supporter:STMMAC ETHERNET DRIVER)
"David S. Miller"  (odd fixer:NETWORKING DRIVERS)
Maxime Coquelin  (maintainer:ARM/STM32 ARCHITECTURE)
net...@vger.kernel.org (open list:STMMAC ETHERNET DRIVER)
linux-st...@st-md-mailman.stormreply.com (moderated list:ARM/STM32 ARCHITECTURE)
linux-arm-ker...@lists.infradead.org (moderated list:ARM/STM32 ARCHITECTURE)
linux-kernel@vger.kernel.org (open list)


I have not bisect commit of root cause yet... Is it better to bisect
and find problem instead of sending this patch?


My guess is that it is one of these three which broken it:

74371272f97f net: stmmac: Convert to phylink and remove phylib logic
eeef2f6b9f6e net: stmmac: Start adding phylink support
9ad372fc5aaf net: stmmac: Prepare to convert to phylink

 Andrew





Re: [PATCH] ARM: dts: rockchip: add ethernet phy node for tinker board

2019-06-22 Thread Katsuhiro Suzuki

Hello,

Current linux-next on my environment, 'ifconfig eth0 up' does not
work correctly with following message...

-
root@linaro-alip:~# ifconfig eth0 up
[  105.028916] rk_gmac-dwmac ff29.ethernet eth0: stmmac_open: Cannot 
attach to PHY (error: -19)

SIOCSIFFLAGS: No such device
-

I checked drivers/net/ethernet/stmicro/stmmac/stmmac_main.c and found
stmmac_init_phy() is going to fail if ethernet device node does not
have following property:
  - phy-handle
  - phy
  - phy-device

I salvaged old version of linux-next kernel (5.2.0-rc1-20190523),
network device of my Tinker Board worked correctly if use it.

I have not bisect commit of root cause yet... Is it better to bisect
and find problem instead of sending this patch?

Best Regards,
---
Katsuhiro Suzuki


On 2019/06/22 17:33, Heiko Stuebner wrote:

Hi,

Am Freitag, 21. Juni 2019, 20:00:17 CEST schrieb Katsuhiro Suzuki:

This patch adds missing mdio and ethernet PHY nodes for rk3328 ASUS
tinker board.

Signed-off-by: Katsuhiro Suzuki 


just for my understanding, which problem does this solve?
Normally the gmac can establish connections just fine on
the rk3288 by probing the phy in the automatic way.

And I also don't see any additional properties like phy
interrupt line below.


Thanks
Heiko


---
  arch/arm/boot/dts/rk3288-tinker.dtsi | 12 
  1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288-tinker.dtsi 
b/arch/arm/boot/dts/rk3288-tinker.dtsi
index 293576869546..3190817e8d5d 100644
--- a/arch/arm/boot/dts/rk3288-tinker.dtsi
+++ b/arch/arm/boot/dts/rk3288-tinker.dtsi
@@ -117,6 +117,7 @@
assigned-clocks = < SCLK_MAC>;
assigned-clock-parents = <_gmac>;
clock_in_out = "input";
+   phy-handle = <>;
phy-mode = "rgmii";
phy-supply = <_lan>;
pinctrl-names = "default";
@@ -127,6 +128,17 @@
tx_delay = <0x30>;
rx_delay = <0x10>;
status = "ok";
+
+   mdio0 {
+   compatible = "snps,dwmac-mdio";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   phy0: ethernet-phy@0 {
+   compatible = "ethernet-phy-ieee802.3-c22";
+   reg = <0>;
+   };
+   };
  };
  
   {












[PATCH] ARM: dts: rockchip: add ethernet phy node for tinker board

2019-06-21 Thread Katsuhiro Suzuki
This patch adds missing mdio and ethernet PHY nodes for rk3328 ASUS
tinker board.

Signed-off-by: Katsuhiro Suzuki 
---
 arch/arm/boot/dts/rk3288-tinker.dtsi | 12 
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288-tinker.dtsi 
b/arch/arm/boot/dts/rk3288-tinker.dtsi
index 293576869546..3190817e8d5d 100644
--- a/arch/arm/boot/dts/rk3288-tinker.dtsi
+++ b/arch/arm/boot/dts/rk3288-tinker.dtsi
@@ -117,6 +117,7 @@
assigned-clocks = < SCLK_MAC>;
assigned-clock-parents = <_gmac>;
clock_in_out = "input";
+   phy-handle = <>;
phy-mode = "rgmii";
phy-supply = <_lan>;
pinctrl-names = "default";
@@ -127,6 +128,17 @@
tx_delay = <0x30>;
rx_delay = <0x10>;
status = "ok";
+
+   mdio0 {
+   compatible = "snps,dwmac-mdio";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   phy0: ethernet-phy@0 {
+   compatible = "ethernet-phy-ieee802.3-c22";
+   reg = <0>;
+   };
+   };
 };
 
  {
-- 
2.20.1



[PATCH] arm64: dts: rockchip: add PCIe nodes on rk3399-rockpro64

2019-05-09 Thread Katsuhiro Suzuki
This patch adds PCIe, PCIe phy and pinctrl (for PERST#) nodes for
RockPro64 board.

Signed-off-by: Katsuhiro Suzuki 
---
 .../boot/dts/rockchip/rk3399-rockpro64.dts | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts 
b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
index 20ec7d1c25d7..56d7eeedf07c 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
@@ -513,6 +513,20 @@
gpio1830-supply = <_3v0>;
 };
 
+ {
+   ep-gpios = < RK_PD4 GPIO_ACTIVE_HIGH>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_perst>;
+   vpcie12v-supply = <_dcin>;
+   vpcie3v3-supply = <_pcie>;
+   num-lanes = <4>;
+   status = "okay";
+};
+
+_phy {
+   status = "okay";
+};
+
 _io_domains {
pmu1830-supply = <_3v0>;
status = "okay";
@@ -545,6 +559,10 @@
pcie_pwr_en: pcie-pwr-en {
rockchip,pins = <1 RK_PD0 RK_FUNC_GPIO _pull_none>;
};
+
+   pcie_perst: pcie-perst {
+   rockchip,pins = <2 RK_PD4 RK_FUNC_GPIO _pull_none>;
+   };
};
 
pmic {
-- 
2.20.1



Re: [PATCH] arm64: dts: rockchip: fix IO domain voltage setting of APIO5

2019-04-15 Thread Katsuhiro Suzuki

ping...

On 2019/03/27 21:03, Katsuhiro Suzuki wrote:

This patch fixes IO domain voltage setting that is related to
audio_gpio3d4a_ms (bit 1) of GRF_IO_VSEL.

This is because RockPro64 schematics P.16 says that regulator
supplies 3.0V power to APIO5_VDD. So audio_gpio3d4a_ms bit should
be clear (means 3.0V). Power domain map is saying different thing
(supplies 1.8V) but I believe P.16 is actual connectings.

Signed-off-by: Katsuhiro Suzuki 
Suggested-by: Robin Murphy 
---
  arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts 
b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
index 791fb0ee9722..20ec7d1c25d7 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
@@ -508,7 +508,7 @@
status = "okay";
  
  	bt656-supply = <_dvp>;

-   audio-supply = <_codec>;
+   audio-supply = <_3v0>;
sdmmc-supply = <_sdio>;
gpio1830-supply = <_3v0>;
  };





[PATCH] arm64: dts: rockchip: fix cts, rts pin assign of UART3 for rk3399

2019-04-06 Thread Katsuhiro Suzuki
This patch fixes pin assign of cts and rts signal of UART3.

Currently GPIO3_C2 and C3 pins are assigned but TRM says that
GPIO3_C0 and C1 are correct.

Refer:
  RK3399 TRM v1.4 - Table 19-1 UART Interface Description

Signed-off-by: Katsuhiro Suzuki 
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index f54c855f8cdf..196ac9b78076 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -2543,12 +2543,12 @@
 
uart3_cts: uart3-cts {
rockchip,pins =
-   <3 RK_PC2 2 _pull_none>;
+   <3 RK_PC0 2 _pull_none>;
};
 
uart3_rts: uart3-rts {
rockchip,pins =
-   <3 RK_PC3 2 _pull_none>;
+   <3 RK_PC1 2 _pull_none>;
};
};
 
-- 
2.20.1



[PATCH] arm64: dts: rockchip: fix IO domain voltage setting of APIO5

2019-03-27 Thread Katsuhiro Suzuki
This patch fixes IO domain voltage setting that is related to
audio_gpio3d4a_ms (bit 1) of GRF_IO_VSEL.

This is because RockPro64 schematics P.16 says that regulator
supplies 3.0V power to APIO5_VDD. So audio_gpio3d4a_ms bit should
be clear (means 3.0V). Power domain map is saying different thing
(supplies 1.8V) but I believe P.16 is actual connectings.

Signed-off-by: Katsuhiro Suzuki 
Suggested-by: Robin Murphy 
---
 arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts 
b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
index 791fb0ee9722..20ec7d1c25d7 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
@@ -508,7 +508,7 @@
status = "okay";
 
bt656-supply = <_dvp>;
-   audio-supply = <_codec>;
+   audio-supply = <_3v0>;
sdmmc-supply = <_sdio>;
gpio1830-supply = <_3v0>;
 };
-- 
2.20.1



Re: [PATCH v2] arm64: dts: rockchip: add rk3399 UART DMAs

2019-03-26 Thread Katsuhiro Suzuki

Hello Robin,

Sorry for inconvenience. Since I don't adhere enabling DMA for UARTs,
please revert my patch if you need.

BTW, there are DMA properties in RK3328 device-tree like as this patch.
RK3328 UART DMA could not work correctly too...??

Best Regards,
Katsuhiro Suzuki


On 2019/03/26 20:48, Robin Murphy wrote:

On 25/03/2019 12:34, Heiko Stuebner wrote:

Am Donnerstag, 21. März 2019, 17:22:44 CET schrieb Katsuhiro Suzuki:

Add UART dma channels as specified by the rk3399 TRM.

Refer:
RK3399 TRM V1.4: Chapter 12 DMA Controller

Signed-off-by: Katsuhiro Suzuki 


applied for 5.2


As a heads-up, I did manage to try my board with this patch applied over 
the weekend, and it makes probing the bluetooth adapter fail with 
communication errors, so I'm not sure the 8250 and pl330 drivers are 
really cooperating well enough :(


Robin.





Re: [PATCH] arm64: dts: rockchip: decrease rising edge time of UART2

2019-03-26 Thread Katsuhiro Suzuki

Hello Tony, Robin,

I continue to investigate UART TX rising time problem. Recently, I found
one of cause of this problem.

In my environment, this problem occurred on linux-next only. U-Boot does
not face it. So I compared settings between U-Boot and linux-next. After
I found GRF_IO_VSEL (address 0xff77e640) register is differ.


Would you tell me what value is stored into this register?


My RockPro64, initially 0x is set but 0x0003 is set during
linux boot. UART TX rising time becomes fine if set both bit 1 and bit 3
or clear both bits.


Best Regards,


On 2019/03/04 22:59, Katsuhiro Suzuki wrote:

Hello Tony, Robin,

On 2019/03/04 5:41, Tony McKahan wrote:

On Sun, Mar 3, 2019 at 2:51 PM Robin Murphy  wrote:


On 2019-03-03 6:45 pm, Tony McKahan wrote:

Hello Katsushiro,

On Sun, Mar 3, 2019 at 12:31 PM Katsuhiro Suzuki
 wrote:


Hello Tony,

On 2019/03/04 0:13, Tony McKahan wrote:
On Sun, Mar 3, 2019 at 9:04 AM Katsuhiro Suzuki 
 wrote:


Hello Heiko,

Thank you for comments.

On 2019/03/03 22:19, Heiko Stuebner wrote:

Hi,

Am Sonntag, 3. März 2019, 13:27:05 CET schrieb Katsuhiro Suzuki:

This patch increases drive strength of UART2 from 3mA to 12mA for
getting more faster rising edge.

RockPro64 is using a very high speed rate (1.5Mbps) for UART2. In
this setting, a bit width of UART is about 667ns.

In my environment (RockPro64 UART2 with FTDI FT232RL UART-USB
converter), falling time of RockPro64 UART2 is 40ns, but riging 
time
is over 650ns. So UART receiver will get wrong data, because 
receiver

read intermediate data of rising edge.

Rising time becomes 300ns from 650ns if apply this patch. This 
is not

perfect solution but better than now.

Signed-off-by: Katsuhiro Suzuki 
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)


your changing a core rk3399 property here, so I'd really like to 
get

input from other board stakeholders on this before applying a core
change.

Could you either include the submitters of other rk3399-boards 
in the
recipient list so that they're aware or limit the change to 
rockpro64 for
the time being (aka overriding the property in the board-dts) 
please?




OK, I'm adding other boards members.
by ./scripts/get_maintainer.pl 
arch/arm64/boot/dts/rockchip/rk3399-*.dts



RockPro64 directly connect UART2 pins of RK3399 to external 
connector.
I think maybe other RK3399 boards are facing same problem, but I 
cannot

check it because I have RockPro64 only...

I'm happy if someone tell me other boards situation.


I'm pulling out other rockchip boards momentarily to see what kind of
population we have.

Note these are not all running 5.x kernels, however none of them have
the UART2 drive levels modified to my knowledge, and regardless, none
show over 100 ns.

board:    rise/fall

rk3399-roc-pc: 90ns/90ns
rk3399-rockpro64 V2.0:  90ns/45ns
rk3399-rockpro64 V2.1:  40ns/41ns


I'm having to eyeball off a 20MHz analog scope (thank goodness for "yes"
being able to generate a nice periodic signal), but for my NanoPC-T4
with a cheap eBay FT232R adapter both rise and fall times are certainly
<100ns. FWIW I've not noticed any issues with letting the Bluetooth
interface on UART0 run flat-out at 4Mbaud either.



Robin, Thanks a lot. Your results show that cold solder (or some
electric parts on board) of my RockPro64 is broken or something wrong.




Please make sure there's not a large amount of flux or something
around the terminals on your board, that seems excessively high.



Thank you for valuable information. For more deeply discussion,
I tried other conditions and watch the rise/fall times.

1) Not connect
The rise/fall times are 40ns/5ns when nothing connect (impedance is
very high) to external pin of RockPro64.

What UART device are you using with RockPro64? If you use some device
with RockPro64 and board shows rise/fall times = 90ns/45ns, my device
is not suitable for RockPro64 by some reason. So it's better to drop
my patch.


The adapter is an FTDI FT232RL breakout board, attached with some
generic Dupont connector jumpers.
Interesting your RockPro is showing this symptom, perhaps there is a
cold solder joint somewhere introducing resistance?



2) Other SoC
I have other SoC board rk3328-rock64, Rock64 shows rise/fall times =
90ns/80ns when same UART-USB device is connected to UART pin.


I measured similar on my Rock64 as well.



Tony, thanks for your info about environment.

It seems my RockPro64 problem. I don't have enough electronic knowledge,
but try to check my RockPro64 as much as I can.




I think it shows rk3399's (or RockPro64's?) drive strength is a little
weak. So it's better to increase the drive strength of UART of rk3399.


I do not think this is a bad idea generally, it simply allows for more
available current from the interface.  I'll let others be the judge of
that, however.


There may be RK3399 users who would care about the possible EMI impact,
so 

[PATCH v2] arm64: dts: rockchip: add rk3399 UART DMAs

2019-03-21 Thread Katsuhiro Suzuki
Add UART dma channels as specified by the rk3399 TRM.

Refer:
RK3399 TRM V1.4: Chapter 12 DMA Controller

Signed-off-by: Katsuhiro Suzuki 

---

Changes from v1:
  - Add dma property for UART4
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 0301e3e01b38..31e487202676 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -635,6 +635,8 @@
clocks = < SCLK_UART0>, < PCLK_UART0>;
clock-names = "baudclk", "apb_pclk";
interrupts = ;
+   dmas = <_peri 0>, <_peri 1>;
+   dma-names = "tx", "rx";
reg-shift = <2>;
reg-io-width = <4>;
pinctrl-names = "default";
@@ -648,6 +650,8 @@
clocks = < SCLK_UART1>, < PCLK_UART1>;
clock-names = "baudclk", "apb_pclk";
interrupts = ;
+   dmas = <_peri 2>, <_peri 3>;
+   dma-names = "tx", "rx";
reg-shift = <2>;
reg-io-width = <4>;
pinctrl-names = "default";
@@ -661,6 +665,8 @@
clocks = < SCLK_UART2>, < PCLK_UART2>;
clock-names = "baudclk", "apb_pclk";
interrupts = ;
+   dmas = <_peri 4>, <_peri 5>;
+   dma-names = "tx", "rx";
reg-shift = <2>;
reg-io-width = <4>;
pinctrl-names = "default";
@@ -674,6 +680,8 @@
clocks = < SCLK_UART3>, < PCLK_UART3>;
clock-names = "baudclk", "apb_pclk";
interrupts = ;
+   dmas = <_peri 6>, <_peri 7>;
+   dma-names = "tx", "rx";
reg-shift = <2>;
reg-io-width = <4>;
pinctrl-names = "default";
@@ -1152,6 +1160,8 @@
clocks = < SCLK_UART4_PMU>, < PCLK_UART4_PMU>;
clock-names = "baudclk", "apb_pclk";
interrupts = ;
+   dmas = <_peri 8>, <_peri 9>;
+   dma-names = "tx", "rx";
reg-shift = <2>;
reg-io-width = <4>;
pinctrl-names = "default";
-- 
2.20.1



Re: [PATCH] arm64: dts: rockchip: add rk3399 UART DMAs

2019-03-21 Thread Katsuhiro Suzuki

Hello Robin,

On 2019/03/22 0:21, Robin Murphy wrote:

On 21/03/2019 14:40, Katsuhiro Suzuki wrote:

Add UART dma channels as specified by the rk3399 TRM.


No UART4? That's arguably one of the more useful ones, since 1 and 3 
often end up muxed to ethernet instead.




Oops... I forgot to add for UART4. Thank you for pointing out.


Also, does UART DMA actually work yet? I guess I can try for myself once 
I get home, but I remember last time I looked it was bailing out because 
the 8250 driver wanted a dmaengine feature that the pl330 driver didn't 
implement.




I think it works correctly. Because pl330_irq_handler() that is
IRQ handler of PL330 is called when my patch applied and handler
is not called if not applied.


Best Regards,
Katsuhiro Suzuki


Robin.


Refer:
RK3399 TRM V1.4: Chapter 12 DMA Controller

Signed-off-by: Katsuhiro Suzuki 
---
  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 8 
  1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi

index 0301e3e01b38..4eed7aae6989 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -635,6 +635,8 @@
  clocks = < SCLK_UART0>, < PCLK_UART0>;
  clock-names = "baudclk", "apb_pclk";
  interrupts = ;
+    dmas = <_peri 0>, <_peri 1>;
+    dma-names = "tx", "rx";
  reg-shift = <2>;
  reg-io-width = <4>;
  pinctrl-names = "default";
@@ -648,6 +650,8 @@
  clocks = < SCLK_UART1>, < PCLK_UART1>;
  clock-names = "baudclk", "apb_pclk";
  interrupts = ;
+    dmas = <_peri 2>, <_peri 3>;
+    dma-names = "tx", "rx";
  reg-shift = <2>;
  reg-io-width = <4>;
  pinctrl-names = "default";
@@ -661,6 +665,8 @@
  clocks = < SCLK_UART2>, < PCLK_UART2>;
  clock-names = "baudclk", "apb_pclk";
  interrupts = ;
+    dmas = <_peri 4>, <_peri 5>;
+    dma-names = "tx", "rx";
  reg-shift = <2>;
  reg-io-width = <4>;
  pinctrl-names = "default";
@@ -674,6 +680,8 @@
  clocks = < SCLK_UART3>, < PCLK_UART3>;
  clock-names = "baudclk", "apb_pclk";
  interrupts = ;
+    dmas = <_peri 6>, <_peri 7>;
+    dma-names = "tx", "rx";
  reg-shift = <2>;
  reg-io-width = <4>;
  pinctrl-names = "default";







Re: [PATCH 2/2] arm64: dts: rockchip: fix cts, rts pin assign of UART3 for rk3399

2019-03-21 Thread Katsuhiro Suzuki

ping again...

Please let me know if there is problem in this patch.


On 2019/03/04 23:00, Katsuhiro Suzuki wrote:

ping...

On 2019/02/19 23:08, Katsuhiro Suzuki wrote:

This patch fixes pin assign of cts and rts signal of UART3.

Currently GPIO3_C2 and C3 pins are assigned but TRM says that
GPIO3_C0 and C1 are correct.

Refer:
   RK3399 TRM v1.4 - Table 19-1 UART Interface Description

Signed-off-by: Katsuhiro Suzuki 
---
  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi

index 76fc2c7af80a..beaa92744a64 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -2535,12 +2535,12 @@
  uart3_cts: uart3-cts {
  rockchip,pins =
-    <3 RK_PC2 RK_FUNC_2 _pull_none>;
+    <3 RK_PC0 RK_FUNC_2 _pull_none>;
  };
  uart3_rts: uart3-rts {
  rockchip,pins =
-    <3 RK_PC3 RK_FUNC_2 _pull_none>;
+    <3 RK_PC1 RK_FUNC_2 _pull_none>;
  };
  };








Re: [PATCH v3 2/2] arm64: dts: rockchip: add HDMI sound node for rk3328-rock64

2019-03-21 Thread Katsuhiro Suzuki

Hello Jonas,

Thank you for reply.

On 2019/03/17 21:33, Jonas Karlman wrote:

Hello Katsuhiro,

Sorry for the delay, I have not been able to fully test this yet but will do 
more testing later tonight.

My concern are regarding how to configure a single graph card node to add a 
hdmi: (and iec958: for spdif) variants using an alsa config file.
In LibreELEC I expose HDMI and SPDIF as two different card nodes to simplify 
applying alsa config files, see [1] and [2] for the alsa config files I use.
Do you know how I can configure the hdmi: and iec958: cards and add a "IEC958 
Playback" ctl with the combined graph card node?



Currently I don't know how to and I don't have test environment for
IEC958 sound. Sorry...



I am also working on NL-PCM/HBR-audio support for dw-hdmi-i2s and it will require 
"IEC958 Playback" ctl and players such as kodi and mpv only sets AES0-3 for 
hdmi: and iec958: cards.

[1] 
https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Rockchip/filesystem/usr/share/alsa/cards/HDMI.conf
[2] 
https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Rockchip/filesystem/usr/share/alsa/cards/SPDIF.conf



Ah, I don't consider about Non-LPCM sound. I understand your concern.

My patch should be keeping until this problem is solved.


I'm going to find test environment and try KODI or mpv. Thank you for
great information!


Best Regards,
Katsuhiro Suzuki



Regards,
Jonas

On 2019-03-17 10:54, Katsuhiro Suzuki wrote:

Hello Jonas,

How about this topic?

I think this patch does not have bad effect for multi channel
sound of HDMI. If you don't think so, please tell me.
I wait your sound patch and after re-check this patch.

Best Regards,
Katsuhiro Suzuki

On 2019/03/03 4:26, Katsuhiro Suzuki wrote:

Hello Jonas,

Thanks for your comments.

On 2019/03/03 2:20, Jonas Karlman wrote:

On 2019-03-02 15:19, Katsuhiro Suzuki wrote:

Ping...

On 2019/02/18 2:34, Katsuhiro Suzuki wrote:

This patch adds HDMI sound (I2S0) node for rock64.

After apply this patch, UART2 will fail to allocate DMA resources
but UART driver can work fine without DMA.

This error is related to the DMAC of rk3328 (pl330 or compatible).
DMAC connected to 16 DMA sources. Each sources have ID number that is
called 'Req number' in rk3328 TRM. Currently total 7 sources has been
activated as follows:

| Req number | Source | Required  |
|    |    | channels  |
|++---|
| 14, 15 | I2S1   | 2ch   |
|  6,  7 | UART2  | 2ch   |
| 10 | SPDIF  | 1ch   |
|  8,  9 | SPI0   | 2ch   |
|++---|
|    | Total  | 7ch   |

HDMI audio needs to activate new source I2S0 (Req number 11 and 12).
I2S0 can work concurrently with other sources, but rk3328 DMAC can
use max 8 channels at same time. If I2S0 is simply activated by this
patch, required DMAC channels will be 9. So last one (UART2) cannot
allocate the DMA resources.

Virt-dma mechanism for pl0330 DMAC driver is needed to fix this
problem.

Signed-off-by: Katsuhiro Suzuki 
---
    .../arm64/boot/dts/rockchip/rk3328-rock64.dts | 21
++-
    1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
index 2157a528276b..bfc0930d245c 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
@@ -68,7 +68,8 @@
    sound {
    compatible = "audio-graph-card";
    label = "rockchip,rk3328";
-    dais = <_p0
+    dais = <_p0
+    _p0
    _p0>;

I am working on multi-channel hdmi audio support for dw-hdmi at [1]
and are wondering if having multiple dais on one graph card
instead of a separate hdmi sound card will affect the ability to
define a proper alsa config for multi channel hdmi sound.

[1]
https://github.com/Kwiboo/linux-rockchip/compare/8874c206d613dc575f5cb6e385e7a866020138d0...92b20eaa6b6dd2cf3418a428f905d10bbc62724f



It seems a part of multi channel (5.1 ch) has already been supported
by rockchip-i2s and dw-hdmi. So I applied this patch and tried below
command.
    speaker-test -D hw:0,0 -f 48000 -c 6
It does not return errors.

Would you tell me more details you worried about if something wrong?


FYI: Rock64 PCM devices are as follows after applied this patch.
~
$ cat /proc/asound/pcm
00-00: ff00.i2s-i2s-hifi i2s-hifi-0 :  : playback 1
00-01: ff01.i2s-rk3328-hifi ff41.codec-1 :  : playback 1 :
capture 1
00-02: ff03.spdif-dit-hifi dit-hifi-2 :  : playback 1
~

Best Regards,
Katsuhiro Suzuki



Regards,
Jonas


    };
@@ -141,6 +142,12 @@
     {
    status = "okay";
+
+    port@0 {
+    hdmi_p0_0: endpoint {
+    remote-endpoint = <_p0_0>;
+    };
+    };
    };
     {
@@ -256,6 +263,18 @@
    };
    };
+ {
+    status = "okay&qu

[PATCH] arm64: dts: rockchip: add rk3399 UART DMAs

2019-03-21 Thread Katsuhiro Suzuki
Add UART dma channels as specified by the rk3399 TRM.

Refer:
RK3399 TRM V1.4: Chapter 12 DMA Controller

Signed-off-by: Katsuhiro Suzuki 
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 8 
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 0301e3e01b38..4eed7aae6989 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -635,6 +635,8 @@
clocks = < SCLK_UART0>, < PCLK_UART0>;
clock-names = "baudclk", "apb_pclk";
interrupts = ;
+   dmas = <_peri 0>, <_peri 1>;
+   dma-names = "tx", "rx";
reg-shift = <2>;
reg-io-width = <4>;
pinctrl-names = "default";
@@ -648,6 +650,8 @@
clocks = < SCLK_UART1>, < PCLK_UART1>;
clock-names = "baudclk", "apb_pclk";
interrupts = ;
+   dmas = <_peri 2>, <_peri 3>;
+   dma-names = "tx", "rx";
reg-shift = <2>;
reg-io-width = <4>;
pinctrl-names = "default";
@@ -661,6 +665,8 @@
clocks = < SCLK_UART2>, < PCLK_UART2>;
clock-names = "baudclk", "apb_pclk";
interrupts = ;
+   dmas = <_peri 4>, <_peri 5>;
+   dma-names = "tx", "rx";
reg-shift = <2>;
reg-io-width = <4>;
pinctrl-names = "default";
@@ -674,6 +680,8 @@
clocks = < SCLK_UART3>, < PCLK_UART3>;
clock-names = "baudclk", "apb_pclk";
interrupts = ;
+   dmas = <_peri 6>, <_peri 7>;
+   dma-names = "tx", "rx";
reg-shift = <2>;
reg-io-width = <4>;
pinctrl-names = "default";
-- 
2.20.1



[PATCH v2] dmaengine: pl330: introduce debugfs interface

2019-03-17 Thread Katsuhiro Suzuki
This patch adds debugfs interface to show the relationship between
DMA threads (hardware resource for transferring data) and DMA
channel ID of DMA slave.

Typically, PL330 has many slaves than number of DMA threads.
So sometimes PL330 cannot allocate DMA threads for all slaves even
if a user specify DMA channel ID in devicetree. This interface will
be useful for checking that DMA threads are allocated or not.

Below is an output sample:

$ sudo cat /sys/kernel/debug/ff1f.dmac
PL330 physical channels:
THREAD: CHANNEL:
-
0   8
1   9
2   11
3   12
4   14
5   15
6   10
7   --

Signed-off-by: Katsuhiro Suzuki 
---
 drivers/dma/pl330.c | 51 +
 1 file changed, 51 insertions(+)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index eec79fdf27a5..c72f6fd79c43 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -11,6 +11,7 @@
  * (at your option) any later version.
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -2896,6 +2897,55 @@ static irqreturn_t pl330_irq_handler(int irq, void *data)
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) | \
BIT(DMA_SLAVE_BUSWIDTH_8_BYTES)
 
+#ifdef CONFIG_DEBUG_FS
+static int pl330_debugfs_show(struct seq_file *s, void *data)
+{
+   struct pl330_dmac *pl330 = s->private;
+   int chans, pchs, ch, pr;
+
+   chans = pl330->pcfg.num_chan;
+   pchs = pl330->num_peripherals;
+
+   seq_puts(s, "PL330 physical channels:\n");
+   seq_puts(s, "THREAD:\t\tCHANNEL:\n");
+   seq_puts(s, "\t-\n");
+   for (ch = 0; ch < chans; ch++) {
+   struct pl330_thread *thrd = >channels[ch];
+   int found = -1;
+
+   for (pr = 0; pr < pchs; pr++) {
+   struct dma_pl330_chan *pch = >peripherals[pr];
+
+   if (!pch->thread || thrd->id != pch->thread->id)
+   continue;
+
+   found = pr;
+   }
+
+   seq_printf(s, "%d\t\t", thrd->id);
+   if (found == -1)
+   seq_puts(s, "--\n");
+   else
+   seq_printf(s, "%d\n", found);
+   }
+
+   return 0;
+}
+
+DEFINE_SHOW_ATTRIBUTE(pl330_debugfs);
+
+static inline void init_pl330_debugfs(struct pl330_dmac *pl330)
+{
+   debugfs_create_file(dev_name(pl330->ddma.dev),
+   S_IFREG | 0444, NULL, pl330,
+   _debugfs_fops);
+}
+#else
+static inline void init_pl330_debugfs(struct pl330_dmac *pl330)
+{
+}
+#endif
+
 /*
  * Runtime PM callbacks are provided by amba/bus.c driver.
  *
@@ -3082,6 +3132,7 @@ pl330_probe(struct amba_device *adev, const struct 
amba_id *id)
dev_err(>dev, "unable to set the seg size\n");
 
 
+   init_pl330_debugfs(pl330);
dev_info(>dev,
"Loaded driver for PL330 DMAC-%x\n", adev->periphid);
dev_info(>dev,
-- 
2.20.1



Re: [PATCH] dmaengine: pl330: introduce debugfs interface

2019-03-17 Thread Katsuhiro Suzuki

Hello,

Sorry, I got mistake in title of this patch...
Please ignore this patch.

Best Regards,
Katsuhiro Suzuki

On 2019/03/17 19:00, Katsuhiro Suzuki wrote:

This patch adds debugfs interface to show the relationship between
DMA threads (hardware resource for transferring data) and DMA
channel ID of DMA slave.

Typically, PL330 has many slaves than number of DMA threads.
So sometimes PL330 cannot allocate DMA threads for all slaves even
if a user specify DMA channel ID in devicetree. This interface will
be useful for checking that DMA threads are allocated or not.

Below is an output sample:

$ sudo cat /sys/kernel/debug/ff1f.dmac
PL330 physical channels:
THREAD: CHANNEL:
-
0   8
1   9
2   11
3   12
4   14
5   15
6   10
7   --

Signed-off-by: Katsuhiro Suzuki 
---
  drivers/dma/pl330.c | 51 +
  1 file changed, 51 insertions(+)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index eec79fdf27a5..c72f6fd79c43 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -11,6 +11,7 @@
   * (at your option) any later version.
   */
  
+#include 

  #include 
  #include 
  #include 
@@ -2896,6 +2897,55 @@ static irqreturn_t pl330_irq_handler(int irq, void *data)
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) | \
BIT(DMA_SLAVE_BUSWIDTH_8_BYTES)
  
+#ifdef CONFIG_DEBUG_FS

+static int pl330_debugfs_show(struct seq_file *s, void *data)
+{
+   struct pl330_dmac *pl330 = s->private;
+   int chans, pchs, ch, pr;
+
+   chans = pl330->pcfg.num_chan;
+   pchs = pl330->num_peripherals;
+
+   seq_puts(s, "PL330 physical channels:\n");
+   seq_puts(s, "THREAD:\t\tCHANNEL:\n");
+   seq_puts(s, "\t-\n");
+   for (ch = 0; ch < chans; ch++) {
+   struct pl330_thread *thrd = >channels[ch];
+   int found = -1;
+
+   for (pr = 0; pr < pchs; pr++) {
+   struct dma_pl330_chan *pch = >peripherals[pr];
+
+   if (!pch->thread || thrd->id != pch->thread->id)
+   continue;
+
+   found = pr;
+   }
+
+   seq_printf(s, "%d\t\t", thrd->id);
+   if (found == -1)
+   seq_puts(s, "--\n");
+   else
+   seq_printf(s, "%d\n", found);
+   }
+
+   return 0;
+}
+
+DEFINE_SHOW_ATTRIBUTE(pl330_debugfs);
+
+static inline void init_pl330_debugfs(struct pl330_dmac *pl330)
+{
+   debugfs_create_file(dev_name(pl330->ddma.dev),
+   S_IFREG | 0444, NULL, pl330,
+   _debugfs_fops);
+}
+#else
+static inline void init_pl330_debugfs(struct pl330_dmac *pl330)
+{
+}
+#endif
+
  /*
   * Runtime PM callbacks are provided by amba/bus.c driver.
   *
@@ -3082,6 +3132,7 @@ pl330_probe(struct amba_device *adev, const struct 
amba_id *id)
dev_err(>dev, "unable to set the seg size\n");
  
  
+	init_pl330_debugfs(pl330);

dev_info(>dev,
"Loaded driver for PL330 DMAC-%x\n", adev->periphid);
dev_info(>dev,





[PATCH] dmaengine: pl330: introduce debugfs interface

2019-03-17 Thread Katsuhiro Suzuki
This patch adds debugfs interface to show the relationship between
DMA threads (hardware resource for transferring data) and DMA
channel ID of DMA slave.

Typically, PL330 has many slaves than number of DMA threads.
So sometimes PL330 cannot allocate DMA threads for all slaves even
if a user specify DMA channel ID in devicetree. This interface will
be useful for checking that DMA threads are allocated or not.

Below is an output sample:

$ sudo cat /sys/kernel/debug/ff1f.dmac
PL330 physical channels:
THREAD: CHANNEL:
-
0   8
1   9
2   11
3   12
4   14
5   15
6   10
7   --

Signed-off-by: Katsuhiro Suzuki 
---
 drivers/dma/pl330.c | 51 +
 1 file changed, 51 insertions(+)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index eec79fdf27a5..c72f6fd79c43 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -11,6 +11,7 @@
  * (at your option) any later version.
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -2896,6 +2897,55 @@ static irqreturn_t pl330_irq_handler(int irq, void *data)
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) | \
BIT(DMA_SLAVE_BUSWIDTH_8_BYTES)
 
+#ifdef CONFIG_DEBUG_FS
+static int pl330_debugfs_show(struct seq_file *s, void *data)
+{
+   struct pl330_dmac *pl330 = s->private;
+   int chans, pchs, ch, pr;
+
+   chans = pl330->pcfg.num_chan;
+   pchs = pl330->num_peripherals;
+
+   seq_puts(s, "PL330 physical channels:\n");
+   seq_puts(s, "THREAD:\t\tCHANNEL:\n");
+   seq_puts(s, "\t-\n");
+   for (ch = 0; ch < chans; ch++) {
+   struct pl330_thread *thrd = >channels[ch];
+   int found = -1;
+
+   for (pr = 0; pr < pchs; pr++) {
+   struct dma_pl330_chan *pch = >peripherals[pr];
+
+   if (!pch->thread || thrd->id != pch->thread->id)
+   continue;
+
+   found = pr;
+   }
+
+   seq_printf(s, "%d\t\t", thrd->id);
+   if (found == -1)
+   seq_puts(s, "--\n");
+   else
+   seq_printf(s, "%d\n", found);
+   }
+
+   return 0;
+}
+
+DEFINE_SHOW_ATTRIBUTE(pl330_debugfs);
+
+static inline void init_pl330_debugfs(struct pl330_dmac *pl330)
+{
+   debugfs_create_file(dev_name(pl330->ddma.dev),
+   S_IFREG | 0444, NULL, pl330,
+   _debugfs_fops);
+}
+#else
+static inline void init_pl330_debugfs(struct pl330_dmac *pl330)
+{
+}
+#endif
+
 /*
  * Runtime PM callbacks are provided by amba/bus.c driver.
  *
@@ -3082,6 +3132,7 @@ pl330_probe(struct amba_device *adev, const struct 
amba_id *id)
dev_err(>dev, "unable to set the seg size\n");
 
 
+   init_pl330_debugfs(pl330);
dev_info(>dev,
"Loaded driver for PL330 DMAC-%x\n", adev->periphid);
dev_info(>dev,
-- 
2.20.1



Re: [PATCH] dmaengine: pl330: introduce debugfs interface

2019-03-17 Thread Katsuhiro Suzuki

Hello Vinod,

Thank you for your comment.
I fix it all and re-post v2 patch.


On 2019/03/16 2:00, Vinod Koul wrote:

On 15-03-19, 03:49, Katsuhiro Suzuki wrote:

This patch adds debugfs interface to show the relationship between
DMA threads (hardware resource for transferring data) and DMA
channel ID of DMA slave.

Typically, PL330 has many slaves than number of DMA threads.
So sometimes PL330 cannot allocate DMA threads for all slaves even
if an user specify DMA channel ID in devicetree. This interface will


a user :)


be useful for checking that DMA threads are allocated or not.

Below is an output sample:

$ sudo cat /sys/kernel/debug/ff1f.dmac
PL330 physical channels:
THREAD: CHANNEL:
-
0   8
1   9
2   11
3   12
4   14
5   15
6   10
7   --

Signed-off-by: Katsuhiro Suzuki 
---
  drivers/dma/pl330.c | 51 +
  1 file changed, 51 insertions(+)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index eec79fdf27a5..aab71863757c 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -11,6 +11,7 @@
   * (at your option) any later version.
   */
  
+#include 

  #include 
  #include 
  #include 
@@ -2896,6 +2897,55 @@ static irqreturn_t pl330_irq_handler(int irq, void *data)
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) | \
BIT(DMA_SLAVE_BUSWIDTH_8_BYTES)
  
+#ifdef CONFIG_DEBUG_FS

+static int pl330_debugfs_show(struct seq_file *s, void *data)
+{
+   struct pl330_dmac *pl330 = s->private;
+   int chans, pchs, ch, pr, found;
+
+   chans = pl330->pcfg.num_chan;
+   pchs = pl330->num_peripherals;
+
+   seq_puts(s, "PL330 physical channels:\n");
+   seq_puts(s, "THREAD:\t\tCHANNEL:\n");
+   seq_puts(s, "\t-\n");
+   for (ch = 0; ch < chans; ch++) {
+   struct pl330_thread *thrd = >channels[ch];
+
+   found = -1;


found can be uninitialized if we skip this for, which should not be an
issue as chans> 0!



Exactly. Thanks, 'found' should be changed to for-loop local variable.



+   for (pr = 0; pr < pchs; pr++) {
+   struct dma_pl330_chan *pch = >peripherals[pr];
+
+   if (!pch->thread || thrd->id != pch->thread->id)
+   continue;
+
+   found = pr;
+   }
+
+   seq_printf(s, "%d\t\t", thrd->id);
+   if (found == -1)
+   seq_puts(s, "--\n");
+   else
+   seq_printf(s, "%d\n", found);
+   }
+
+   return 0;
+}
+
+DEFINE_SHOW_ATTRIBUTE(pl330_debugfs);
+
+static inline void init_pl330_debugfs(struct pl330_dmac *pl330)
+{
+   debugfs_create_file(dev_name(pl330->ddma.dev),
+   S_IFREG | S_IRUGO, NULL, pl330,


are we not supposed to use octal permissions?


+   _debugfs_fops);
+}
+#else
+static inline void init_pl330_debugfs(struct pl330_dmac *pl330)
+{
+}
+#endif
+
  /*
   * Runtime PM callbacks are provided by amba/bus.c driver.
   *
@@ -3082,6 +3132,7 @@ pl330_probe(struct amba_device *adev, const struct 
amba_id *id)
dev_err(>dev, "unable to set the seg size\n");
  
  
+	init_pl330_debugfs(pl330);

dev_info(>dev,
        "Loaded driver for PL330 DMAC-%x\n", adev->periphid);
dev_info(>dev,
--
2.20.1




Best Regards,
Katsuhiro Suzuki


Re: [PATCH v3 2/2] arm64: dts: rockchip: add HDMI sound node for rk3328-rock64

2019-03-17 Thread Katsuhiro Suzuki

Hello Jonas,

How about this topic?

I think this patch does not have bad effect for multi channel
sound of HDMI. If you don't think so, please tell me.
I wait your sound patch and after re-check this patch.

Best Regards,
Katsuhiro Suzuki

On 2019/03/03 4:26, Katsuhiro Suzuki wrote:

Hello Jonas,

Thanks for your comments.

On 2019/03/03 2:20, Jonas Karlman wrote:

On 2019-03-02 15:19, Katsuhiro Suzuki wrote:

Ping...

On 2019/02/18 2:34, Katsuhiro Suzuki wrote:

This patch adds HDMI sound (I2S0) node for rock64.

After apply this patch, UART2 will fail to allocate DMA resources
but UART driver can work fine without DMA.

This error is related to the DMAC of rk3328 (pl330 or compatible).
DMAC connected to 16 DMA sources. Each sources have ID number that is
called 'Req number' in rk3328 TRM. Currently total 7 sources has been
activated as follows:

| Req number | Source | Required  |
|    |    | channels  |
|++---|
| 14, 15 | I2S1   | 2ch   |
|  6,  7 | UART2  | 2ch   |
| 10 | SPDIF  | 1ch   |
|  8,  9 | SPI0   | 2ch   |
|++---|
|    | Total  | 7ch   |

HDMI audio needs to activate new source I2S0 (Req number 11 and 12).
I2S0 can work concurrently with other sources, but rk3328 DMAC can
use max 8 channels at same time. If I2S0 is simply activated by this
patch, required DMAC channels will be 9. So last one (UART2) cannot
allocate the DMA resources.

Virt-dma mechanism for pl0330 DMAC driver is needed to fix this
problem.

Signed-off-by: Katsuhiro Suzuki 
---
   .../arm64/boot/dts/rockchip/rk3328-rock64.dts | 21 
++-

   1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts 
b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts

index 2157a528276b..bfc0930d245c 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
@@ -68,7 +68,8 @@
   sound {
   compatible = "audio-graph-card";
   label = "rockchip,rk3328";
-    dais = <_p0
+    dais = <_p0
+    _p0
   _p0>;


I am working on multi-channel hdmi audio support for dw-hdmi at [1] 
and are wondering if having multiple dais on one graph card
instead of a separate hdmi sound card will affect the ability to 
define a proper alsa config for multi channel hdmi sound.


[1] 
https://github.com/Kwiboo/linux-rockchip/compare/8874c206d613dc575f5cb6e385e7a866020138d0...92b20eaa6b6dd2cf3418a428f905d10bbc62724f 





It seems a part of multi channel (5.1 ch) has already been supported
by rockchip-i2s and dw-hdmi. So I applied this patch and tried below
command.
   speaker-test -D hw:0,0 -f 48000 -c 6
It does not return errors.

Would you tell me more details you worried about if something wrong?


FYI: Rock64 PCM devices are as follows after applied this patch.
~
$ cat /proc/asound/pcm
00-00: ff00.i2s-i2s-hifi i2s-hifi-0 :  : playback 1
00-01: ff01.i2s-rk3328-hifi ff41.codec-1 :  : playback 1 : 
capture 1

00-02: ff03.spdif-dit-hifi dit-hifi-2 :  : playback 1
~~~~~

Best Regards,
Katsuhiro Suzuki



Regards,
Jonas


   };
@@ -141,6 +142,12 @@
    {
   status = "okay";
+
+    port@0 {
+    hdmi_p0_0: endpoint {
+    remote-endpoint = <_p0_0>;
+    };
+    };
   };
    {
@@ -256,6 +263,18 @@
   };
   };
+ {
+    status = "okay";
+
+    i2s0_p0: port {
+    i2s0_p0_0: endpoint {
+    dai-format = "i2s";
+    mclk-fs = <256>;
+    remote-endpoint = <_p0_0>;
+    };
+    };
+};
+
    {
   status = "okay";



___
Linux-rockchip mailing list
linux-rockc...@lists.infradead.org
https://nam03.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists.infradead.org%2Fmailman%2Flistinfo%2Flinux-rockchipdata=02%7C01%7C%7Cfcb0d6c43ded4d5e7d4c08d69f1a226d%7C84df9e7fe9f640afb435%7C1%7C0%7C636871331922090426sdata=ABb%2Fo%2FMAGvFBH%2B37uQr6rzn%2B%2FXBAXyiGfv2%2BMO0RQoQ%3Dreserved=0 












[PATCH] dmaengine: pl330: introduce debugfs interface

2019-03-14 Thread Katsuhiro Suzuki
This patch adds debugfs interface to show the relationship between
DMA threads (hardware resource for transferring data) and DMA
channel ID of DMA slave.

Typically, PL330 has many slaves than number of DMA threads.
So sometimes PL330 cannot allocate DMA threads for all slaves even
if an user specify DMA channel ID in devicetree. This interface will
be useful for checking that DMA threads are allocated or not.

Below is an output sample:

$ sudo cat /sys/kernel/debug/ff1f.dmac
PL330 physical channels:
THREAD: CHANNEL:
-
0   8
1   9
2   11
3   12
4   14
5   15
6   10
7   --

Signed-off-by: Katsuhiro Suzuki 
---
 drivers/dma/pl330.c | 51 +
 1 file changed, 51 insertions(+)

diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index eec79fdf27a5..aab71863757c 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -11,6 +11,7 @@
  * (at your option) any later version.
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -2896,6 +2897,55 @@ static irqreturn_t pl330_irq_handler(int irq, void *data)
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) | \
BIT(DMA_SLAVE_BUSWIDTH_8_BYTES)
 
+#ifdef CONFIG_DEBUG_FS
+static int pl330_debugfs_show(struct seq_file *s, void *data)
+{
+   struct pl330_dmac *pl330 = s->private;
+   int chans, pchs, ch, pr, found;
+
+   chans = pl330->pcfg.num_chan;
+   pchs = pl330->num_peripherals;
+
+   seq_puts(s, "PL330 physical channels:\n");
+   seq_puts(s, "THREAD:\t\tCHANNEL:\n");
+   seq_puts(s, "\t-\n");
+   for (ch = 0; ch < chans; ch++) {
+   struct pl330_thread *thrd = >channels[ch];
+
+   found = -1;
+   for (pr = 0; pr < pchs; pr++) {
+   struct dma_pl330_chan *pch = >peripherals[pr];
+
+   if (!pch->thread || thrd->id != pch->thread->id)
+   continue;
+
+   found = pr;
+   }
+
+   seq_printf(s, "%d\t\t", thrd->id);
+   if (found == -1)
+   seq_puts(s, "--\n");
+   else
+   seq_printf(s, "%d\n", found);
+   }
+
+   return 0;
+}
+
+DEFINE_SHOW_ATTRIBUTE(pl330_debugfs);
+
+static inline void init_pl330_debugfs(struct pl330_dmac *pl330)
+{
+   debugfs_create_file(dev_name(pl330->ddma.dev),
+   S_IFREG | S_IRUGO, NULL, pl330,
+   _debugfs_fops);
+}
+#else
+static inline void init_pl330_debugfs(struct pl330_dmac *pl330)
+{
+}
+#endif
+
 /*
  * Runtime PM callbacks are provided by amba/bus.c driver.
  *
@@ -3082,6 +3132,7 @@ pl330_probe(struct amba_device *adev, const struct 
amba_id *id)
dev_err(>dev, "unable to set the seg size\n");
 
 
+   init_pl330_debugfs(pl330);
dev_info(>dev,
"Loaded driver for PL330 DMAC-%x\n", adev->periphid);
dev_info(>dev,
-- 
2.20.1



Re: [PATCH] arm64: dts: rockchip: Fix vcc_host1_5v GPIO polarity on rk3328-rock64

2019-03-08 Thread Katsuhiro Suzuki

Hello Mayama-san,

On 2019/03/08 1:18, Tomohiro Mayama wrote:

This patch makes USB ports functioning again.

Signed-off-by: Tomohiro Mayama 
---
  arch/arm64/boot/dts/rockchip/rk3328-rock64.dts | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts 
b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
index 21107d0f4378..7f365e448867 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
@@ -45,8 +45,7 @@
  
  	vcc_host1_5v: vcc_otg_5v: vcc-host1-5v-regulator {

compatible = "regulator-fixed";
-   enable-active-high;
-   gpio = < RK_PA2 GPIO_ACTIVE_HIGH>;
+   gpio = < RK_PA2 GPIO_ACTIVE_LOW>;
pinctrl-names = "default";
pinctrl-0 = <_host_drv>;
regulator-name = "vcc_host1_5v";



After this patch applied, USB and S/PDIF both work well.

Tested-by: Katsuhiro Suzuki 


$ lsusb
Bus 003 Device 001: ID 1d6b:0001 Linux Foundation 1.1 root hub
Bus 002 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 001 Device 002: ID 046d:c526 Logitech, Inc. Nano Receiver
Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub


Best Regards,
Katsuhiro Suzuki


Re: [PATCH 2/2] arm64: dts: rockchip: fix cts, rts pin assign of UART3 for rk3399

2019-03-04 Thread Katsuhiro Suzuki

ping...

On 2019/02/19 23:08, Katsuhiro Suzuki wrote:

This patch fixes pin assign of cts and rts signal of UART3.

Currently GPIO3_C2 and C3 pins are assigned but TRM says that
GPIO3_C0 and C1 are correct.

Refer:
   RK3399 TRM v1.4 - Table 19-1 UART Interface Description

Signed-off-by: Katsuhiro Suzuki 
---
  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 76fc2c7af80a..beaa92744a64 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -2535,12 +2535,12 @@
  
  			uart3_cts: uart3-cts {

rockchip,pins =
-   <3 RK_PC2 RK_FUNC_2 _pull_none>;
+   <3 RK_PC0 RK_FUNC_2 _pull_none>;
};
  
  			uart3_rts: uart3-rts {

rockchip,pins =
-   <3 RK_PC3 RK_FUNC_2 _pull_none>;
+   <3 RK_PC1 RK_FUNC_2 _pull_none>;
};
};
  





Re: [PATCH v3 1/2] arm64: dts: rockchip: add #sound-dai-cells to HDMI of rk3328

2019-03-04 Thread Katsuhiro Suzuki

Hello Heiko,

On 2019/03/03 22:55, Heiko Stuebner wrote:

Am Sonntag, 17. Februar 2019, 18:34:06 CET schrieb Katsuhiro Suzuki:

This patch adds #sound-dai-cells to use HDMI node as audio
codec from device tree of rk3328 boards.

Signed-off-by: Katsuhiro Suzuki 


applied for 5.2 .

I'm holding off of patch2 for a bit until you and Jonas can clarify that
all is right as it is :-)


Thanks and I understand about patch 2.

Best Regards,
Katsuhiro Suzuki




Heiko







Re: [PATCH] arm64: dts: rockchip: decrease rising edge time of UART2

2019-03-04 Thread Katsuhiro Suzuki

Hello Tony, Robin,

On 2019/03/04 5:41, Tony McKahan wrote:

On Sun, Mar 3, 2019 at 2:51 PM Robin Murphy  wrote:


On 2019-03-03 6:45 pm, Tony McKahan wrote:

Hello Katsushiro,

On Sun, Mar 3, 2019 at 12:31 PM Katsuhiro Suzuki
 wrote:


Hello Tony,

On 2019/03/04 0:13, Tony McKahan wrote:

On Sun, Mar 3, 2019 at 9:04 AM Katsuhiro Suzuki  wrote:


Hello Heiko,

Thank you for comments.

On 2019/03/03 22:19, Heiko Stuebner wrote:

Hi,

Am Sonntag, 3. März 2019, 13:27:05 CET schrieb Katsuhiro Suzuki:

This patch increases drive strength of UART2 from 3mA to 12mA for
getting more faster rising edge.

RockPro64 is using a very high speed rate (1.5Mbps) for UART2. In
this setting, a bit width of UART is about 667ns.

In my environment (RockPro64 UART2 with FTDI FT232RL UART-USB
converter), falling time of RockPro64 UART2 is 40ns, but riging time
is over 650ns. So UART receiver will get wrong data, because receiver
read intermediate data of rising edge.

Rising time becomes 300ns from 650ns if apply this patch. This is not
perfect solution but better than now.

Signed-off-by: Katsuhiro Suzuki 
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)


your changing a core rk3399 property here, so I'd really like to get
input from other board stakeholders on this before applying a core
change.

Could you either include the submitters of other rk3399-boards in the
recipient list so that they're aware or limit the change to rockpro64 for
the time being (aka overriding the property in the board-dts) please?



OK, I'm adding other boards members.
by ./scripts/get_maintainer.pl arch/arm64/boot/dts/rockchip/rk3399-*.dts


RockPro64 directly connect UART2 pins of RK3399 to external connector.
I think maybe other RK3399 boards are facing same problem, but I cannot
check it because I have RockPro64 only...

I'm happy if someone tell me other boards situation.


I'm pulling out other rockchip boards momentarily to see what kind of
population we have.

Note these are not all running 5.x kernels, however none of them have
the UART2 drive levels modified to my knowledge, and regardless, none
show over 100 ns.

board:rise/fall

rk3399-roc-pc: 90ns/90ns
rk3399-rockpro64 V2.0:  90ns/45ns
rk3399-rockpro64 V2.1:  40ns/41ns


I'm having to eyeball off a 20MHz analog scope (thank goodness for "yes"
being able to generate a nice periodic signal), but for my NanoPC-T4
with a cheap eBay FT232R adapter both rise and fall times are certainly
<100ns. FWIW I've not noticed any issues with letting the Bluetooth
interface on UART0 run flat-out at 4Mbaud either.



Robin, Thanks a lot. Your results show that cold solder (or some
electric parts on board) of my RockPro64 is broken or something wrong.




Please make sure there's not a large amount of flux or something
around the terminals on your board, that seems excessively high.



Thank you for valuable information. For more deeply discussion,
I tried other conditions and watch the rise/fall times.

1) Not connect
The rise/fall times are 40ns/5ns when nothing connect (impedance is
very high) to external pin of RockPro64.

What UART device are you using with RockPro64? If you use some device
with RockPro64 and board shows rise/fall times = 90ns/45ns, my device
is not suitable for RockPro64 by some reason. So it's better to drop
my patch.


The adapter is an FTDI FT232RL breakout board, attached with some
generic Dupont connector jumpers.
Interesting your RockPro is showing this symptom, perhaps there is a
cold solder joint somewhere introducing resistance?



2) Other SoC
I have other SoC board rk3328-rock64, Rock64 shows rise/fall times =
90ns/80ns when same UART-USB device is connected to UART pin.


I measured similar on my Rock64 as well.



Tony, thanks for your info about environment.

It seems my RockPro64 problem. I don't have enough electronic knowledge,
but try to check my RockPro64 as much as I can.




I think it shows rk3399's (or RockPro64's?) drive strength is a little
weak. So it's better to increase the drive strength of UART of rk3399.


I do not think this is a bad idea generally, it simply allows for more
available current from the interface.  I'll let others be the judge of
that, however.


There may be RK3399 users who would care about the possible EMI impact,
so it's probably still best to limit such a change to boards which
definitely need it.



Ah, good point.

As to speeds achievable, I've only run into drive level issues with
SD/SDIO, but that's all the way up at 25-50 MHz.


My patch has bad effects for EMI issues of all RK3399 boards.

So conclusion is, my patch should be dropped. Sorry for confusing.

Best Regards,
Katsuhiro Suzuki




Tony


Robin.





Best Regards,
Katsuhiro Suzuki



Best Regards,
Katsuhiro Suzuki



Thanks
Heiko




diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index beaa92744a64..e3c8f91ead50 100644
--- a/ar

Re: [PATCH] arm64: dts: rockchip: decrease rising edge time of UART2

2019-03-04 Thread Katsuhiro Suzuki



No problem! Thank you for discussion :)

On 2019/03/04 4:12, Tony McKahan wrote:

On Sun, Mar 3, 2019 at 1:45 PM Tony McKahan  wrote:


Hello Katsushiro,


And apologies for the extra "s", typing too quickly I'm afraid.


On Sun, Mar 3, 2019 at 12:31 PM Katsuhiro Suzuki
 wrote:


Hello Tony,

On 2019/03/04 0:13, Tony McKahan wrote:

On Sun, Mar 3, 2019 at 9:04 AM Katsuhiro Suzuki  wrote:


Hello Heiko,

Thank you for comments.

On 2019/03/03 22:19, Heiko Stuebner wrote:

Hi,

Am Sonntag, 3. März 2019, 13:27:05 CET schrieb Katsuhiro Suzuki:

This patch increases drive strength of UART2 from 3mA to 12mA for
getting more faster rising edge.

RockPro64 is using a very high speed rate (1.5Mbps) for UART2. In
this setting, a bit width of UART is about 667ns.

In my environment (RockPro64 UART2 with FTDI FT232RL UART-USB
converter), falling time of RockPro64 UART2 is 40ns, but riging time
is over 650ns. So UART receiver will get wrong data, because receiver
read intermediate data of rising edge.

Rising time becomes 300ns from 650ns if apply this patch. This is not
perfect solution but better than now.

Signed-off-by: Katsuhiro Suzuki 
---
arch/arm64/boot/dts/rockchip/rk3399.dtsi | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)


your changing a core rk3399 property here, so I'd really like to get
input from other board stakeholders on this before applying a core
change.

Could you either include the submitters of other rk3399-boards in the
recipient list so that they're aware or limit the change to rockpro64 for
the time being (aka overriding the property in the board-dts) please?



OK, I'm adding other boards members.
by ./scripts/get_maintainer.pl arch/arm64/boot/dts/rockchip/rk3399-*.dts


RockPro64 directly connect UART2 pins of RK3399 to external connector.
I think maybe other RK3399 boards are facing same problem, but I cannot
check it because I have RockPro64 only...

I'm happy if someone tell me other boards situation.


I'm pulling out other rockchip boards momentarily to see what kind of
population we have.

Note these are not all running 5.x kernels, however none of them have
the UART2 drive levels modified to my knowledge, and regardless, none
show over 100 ns.

board:rise/fall

rk3399-roc-pc: 90ns/90ns
rk3399-rockpro64 V2.0:  90ns/45ns
rk3399-rockpro64 V2.1:  40ns/41ns

Please make sure there's not a large amount of flux or something
around the terminals on your board, that seems excessively high.



Thank you for valuable information. For more deeply discussion,
I tried other conditions and watch the rise/fall times.

1) Not connect
The rise/fall times are 40ns/5ns when nothing connect (impedance is
very high) to external pin of RockPro64.

What UART device are you using with RockPro64? If you use some device
with RockPro64 and board shows rise/fall times = 90ns/45ns, my device
is not suitable for RockPro64 by some reason. So it's better to drop
my patch.


The adapter is an FTDI FT232RL breakout board, attached with some
generic Dupont connector jumpers.
Interesting your RockPro is showing this symptom, perhaps there is a
cold solder joint somewhere introducing resistance?



2) Other SoC
I have other SoC board rk3328-rock64, Rock64 shows rise/fall times =
90ns/80ns when same UART-USB device is connected to UART pin.


I measured similar on my Rock64 as well.



I think it shows rk3399's (or RockPro64's?) drive strength is a little
weak. So it's better to increase the drive strength of UART of rk3399.


I do not think this is a bad idea generally, it simply allows for more
available current from the interface.  I'll let others be the judge of
that, however.



Best Regards,
Katsuhiro Suzuki



Best Regards,
Katsuhiro Suzuki



Thanks
Heiko




diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index beaa92744a64..e3c8f91ead50 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -2000,6 +2000,11 @@
   drive-strength = <8>;
   };

+pcfg_pull_up_12ma: pcfg-pull-up-12ma {
+bias-pull-up;
+drive-strength = <12>;
+};
+
   pcfg_pull_up_18ma: pcfg-pull-up-18ma {
   bias-pull-up;
   drive-strength = <18>;
@@ -2521,8 +2526,8 @@
   uart2c {
   uart2c_xfer: uart2c-xfer {
   rockchip,pins =
-<4 RK_PC3 RK_FUNC_1 _pull_up>,
-<4 RK_PC4 RK_FUNC_1 _pull_none>;
+<4 RK_PC3 RK_FUNC_1 _pull_up_12ma>,
+<4 RK_PC4 RK_FUNC_1 _pull_none_12ma>;
   };
   };












___
Linux-rockchip mailing list
linux-rockc...@lists.infra

Re: [PATCH] arm64: dts: rockchip: decrease rising edge time of UART2

2019-03-03 Thread Katsuhiro Suzuki

Hello Tony,

On 2019/03/04 0:13, Tony McKahan wrote:

On Sun, Mar 3, 2019 at 9:04 AM Katsuhiro Suzuki  wrote:


Hello Heiko,

Thank you for comments.

On 2019/03/03 22:19, Heiko Stuebner wrote:

Hi,

Am Sonntag, 3. März 2019, 13:27:05 CET schrieb Katsuhiro Suzuki:

This patch increases drive strength of UART2 from 3mA to 12mA for
getting more faster rising edge.

RockPro64 is using a very high speed rate (1.5Mbps) for UART2. In
this setting, a bit width of UART is about 667ns.

In my environment (RockPro64 UART2 with FTDI FT232RL UART-USB
converter), falling time of RockPro64 UART2 is 40ns, but riging time
is over 650ns. So UART receiver will get wrong data, because receiver
read intermediate data of rising edge.

Rising time becomes 300ns from 650ns if apply this patch. This is not
perfect solution but better than now.

Signed-off-by: Katsuhiro Suzuki 
---
   arch/arm64/boot/dts/rockchip/rk3399.dtsi | 9 +++--
   1 file changed, 7 insertions(+), 2 deletions(-)


your changing a core rk3399 property here, so I'd really like to get
input from other board stakeholders on this before applying a core
change.

Could you either include the submitters of other rk3399-boards in the
recipient list so that they're aware or limit the change to rockpro64 for
the time being (aka overriding the property in the board-dts) please?



OK, I'm adding other boards members.
by ./scripts/get_maintainer.pl arch/arm64/boot/dts/rockchip/rk3399-*.dts


RockPro64 directly connect UART2 pins of RK3399 to external connector.
I think maybe other RK3399 boards are facing same problem, but I cannot
check it because I have RockPro64 only...

I'm happy if someone tell me other boards situation.


I'm pulling out other rockchip boards momentarily to see what kind of
population we have.

Note these are not all running 5.x kernels, however none of them have
the UART2 drive levels modified to my knowledge, and regardless, none
show over 100 ns.

board:rise/fall

rk3399-roc-pc: 90ns/90ns
rk3399-rockpro64 V2.0:  90ns/45ns
rk3399-rockpro64 V2.1:  40ns/41ns

Please make sure there's not a large amount of flux or something
around the terminals on your board, that seems excessively high.



Thank you for valuable information. For more deeply discussion,
I tried other conditions and watch the rise/fall times.

1) Not connect
The rise/fall times are 40ns/5ns when nothing connect (impedance is
very high) to external pin of RockPro64.

What UART device are you using with RockPro64? If you use some device
with RockPro64 and board shows rise/fall times = 90ns/45ns, my device
is not suitable for RockPro64 by some reason. So it's better to drop
my patch.

2) Other SoC
I have other SoC board rk3328-rock64, Rock64 shows rise/fall times =
90ns/80ns when same UART-USB device is connected to UART pin.

I think it shows rk3399's (or RockPro64's?) drive strength is a little
weak. So it's better to increase the drive strength of UART of rk3399.

Best Regards,
Katsuhiro Suzuki



Best Regards,
Katsuhiro Suzuki



Thanks
Heiko




diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index beaa92744a64..e3c8f91ead50 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -2000,6 +2000,11 @@
  drive-strength = <8>;
  };

+pcfg_pull_up_12ma: pcfg-pull-up-12ma {
+bias-pull-up;
+drive-strength = <12>;
+};
+
  pcfg_pull_up_18ma: pcfg-pull-up-18ma {
  bias-pull-up;
  drive-strength = <18>;
@@ -2521,8 +2526,8 @@
  uart2c {
  uart2c_xfer: uart2c-xfer {
  rockchip,pins =
-<4 RK_PC3 RK_FUNC_1 _pull_up>,
-<4 RK_PC4 RK_FUNC_1 _pull_none>;
+<4 RK_PC3 RK_FUNC_1 _pull_up_12ma>,
+<4 RK_PC4 RK_FUNC_1 _pull_none_12ma>;
  };
  };












___
Linux-rockchip mailing list
linux-rockc...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip







Re: [PATCH] arm64: dts: rockchip: decrease rising edge time of UART2

2019-03-03 Thread Katsuhiro Suzuki

Hello Heiko,

Thank you for comments.

On 2019/03/03 22:19, Heiko Stuebner wrote:

Hi,

Am Sonntag, 3. März 2019, 13:27:05 CET schrieb Katsuhiro Suzuki:

This patch increases drive strength of UART2 from 3mA to 12mA for
getting more faster rising edge.

RockPro64 is using a very high speed rate (1.5Mbps) for UART2. In
this setting, a bit width of UART is about 667ns.

In my environment (RockPro64 UART2 with FTDI FT232RL UART-USB
converter), falling time of RockPro64 UART2 is 40ns, but riging time
is over 650ns. So UART receiver will get wrong data, because receiver
read intermediate data of rising edge.

Rising time becomes 300ns from 650ns if apply this patch. This is not
perfect solution but better than now.

Signed-off-by: Katsuhiro Suzuki 
---
  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 9 +++--
  1 file changed, 7 insertions(+), 2 deletions(-)


your changing a core rk3399 property here, so I'd really like to get
input from other board stakeholders on this before applying a core
change.

Could you either include the submitters of other rk3399-boards in the
recipient list so that they're aware or limit the change to rockpro64 for
the time being (aka overriding the property in the board-dts) please?



OK, I'm adding other boards members.
by ./scripts/get_maintainer.pl arch/arm64/boot/dts/rockchip/rk3399-*.dts


RockPro64 directly connect UART2 pins of RK3399 to external connector.
I think maybe other RK3399 boards are facing same problem, but I cannot
check it because I have RockPro64 only...

I'm happy if someone tell me other boards situation.

Best Regards,
Katsuhiro Suzuki



Thanks
Heiko




diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index beaa92744a64..e3c8f91ead50 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -2000,6 +2000,11 @@
drive-strength = <8>;
};
  
+		pcfg_pull_up_12ma: pcfg-pull-up-12ma {

+   bias-pull-up;
+   drive-strength = <12>;
+   };
+
pcfg_pull_up_18ma: pcfg-pull-up-18ma {
bias-pull-up;
drive-strength = <18>;
@@ -2521,8 +2526,8 @@
uart2c {
uart2c_xfer: uart2c-xfer {
rockchip,pins =
-   <4 RK_PC3 RK_FUNC_1 _pull_up>,
-   <4 RK_PC4 RK_FUNC_1 _pull_none>;
+   <4 RK_PC3 RK_FUNC_1 _pull_up_12ma>,
+   <4 RK_PC4 RK_FUNC_1 
_pull_none_12ma>;
};
};
  












[PATCH] arm64: dts: rockchip: decrease rising edge time of UART2

2019-03-03 Thread Katsuhiro Suzuki
This patch increases drive strength of UART2 from 3mA to 12mA for
getting more faster rising edge.

RockPro64 is using a very high speed rate (1.5Mbps) for UART2. In
this setting, a bit width of UART is about 667ns.

In my environment (RockPro64 UART2 with FTDI FT232RL UART-USB
converter), falling time of RockPro64 UART2 is 40ns, but riging time
is over 650ns. So UART receiver will get wrong data, because receiver
read intermediate data of rising edge.

Rising time becomes 300ns from 650ns if apply this patch. This is not
perfect solution but better than now.

Signed-off-by: Katsuhiro Suzuki 
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index beaa92744a64..e3c8f91ead50 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -2000,6 +2000,11 @@
drive-strength = <8>;
};
 
+   pcfg_pull_up_12ma: pcfg-pull-up-12ma {
+   bias-pull-up;
+   drive-strength = <12>;
+   };
+
pcfg_pull_up_18ma: pcfg-pull-up-18ma {
bias-pull-up;
drive-strength = <18>;
@@ -2521,8 +2526,8 @@
uart2c {
uart2c_xfer: uart2c-xfer {
rockchip,pins =
-   <4 RK_PC3 RK_FUNC_1 _pull_up>,
-   <4 RK_PC4 RK_FUNC_1 _pull_none>;
+   <4 RK_PC3 RK_FUNC_1 _pull_up_12ma>,
+   <4 RK_PC4 RK_FUNC_1 
_pull_none_12ma>;
};
};
 
-- 
2.20.1



Re: [PATCH v3 2/2] arm64: dts: rockchip: add HDMI sound node for rk3328-rock64

2019-03-02 Thread Katsuhiro Suzuki

Hello Jonas,

Thanks for your comments.

On 2019/03/03 2:20, Jonas Karlman wrote:

On 2019-03-02 15:19, Katsuhiro Suzuki wrote:

Ping...

On 2019/02/18 2:34, Katsuhiro Suzuki wrote:

This patch adds HDMI sound (I2S0) node for rock64.

After apply this patch, UART2 will fail to allocate DMA resources
but UART driver can work fine without DMA.

This error is related to the DMAC of rk3328 (pl330 or compatible).
DMAC connected to 16 DMA sources. Each sources have ID number that is
called 'Req number' in rk3328 TRM. Currently total 7 sources has been
activated as follows:

| Req number | Source | Required  |
||| channels  |
|++---|
| 14, 15 | I2S1   | 2ch   |
|  6,  7 | UART2  | 2ch   |
| 10 | SPDIF  | 1ch   |
|  8,  9 | SPI0   | 2ch   |
|++---|
|| Total  | 7ch   |

HDMI audio needs to activate new source I2S0 (Req number 11 and 12).
I2S0 can work concurrently with other sources, but rk3328 DMAC can
use max 8 channels at same time. If I2S0 is simply activated by this
patch, required DMAC channels will be 9. So last one (UART2) cannot
allocate the DMA resources.

Virt-dma mechanism for pl0330 DMAC driver is needed to fix this
problem.

Signed-off-by: Katsuhiro Suzuki 
---
   .../arm64/boot/dts/rockchip/rk3328-rock64.dts | 21 ++-
   1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts 
b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
index 2157a528276b..bfc0930d245c 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
@@ -68,7 +68,8 @@
sound {
compatible = "audio-graph-card";
label = "rockchip,rk3328";
-   dais = <_p0
+   dais = <_p0
+   _p0
_p0>;


I am working on multi-channel hdmi audio support for dw-hdmi at [1] and are 
wondering if having multiple dais on one graph card
instead of a separate hdmi sound card will affect the ability to define a 
proper alsa config for multi channel hdmi sound.

[1] 
https://github.com/Kwiboo/linux-rockchip/compare/8874c206d613dc575f5cb6e385e7a866020138d0...92b20eaa6b6dd2cf3418a428f905d10bbc62724f



It seems a part of multi channel (5.1 ch) has already been supported
by rockchip-i2s and dw-hdmi. So I applied this patch and tried below
command.
  speaker-test -D hw:0,0 -f 48000 -c 6
It does not return errors.

Would you tell me more details you worried about if something wrong?


FYI: Rock64 PCM devices are as follows after applied this patch.
~
$ cat /proc/asound/pcm
00-00: ff00.i2s-i2s-hifi i2s-hifi-0 :  : playback 1
00-01: ff01.i2s-rk3328-hifi ff41.codec-1 :  : playback 1 : capture 1
00-02: ff03.spdif-dit-hifi dit-hifi-2 :  : playback 1
~~~~~

Best Regards,
Katsuhiro Suzuki



Regards,
Jonas


};
   
@@ -141,6 +142,12 @@
   
{

status = "okay";
+
+   port@0 {
+   hdmi_p0_0: endpoint {
+   remote-endpoint = <_p0_0>;
+   };
+   };
   };
   
{

@@ -256,6 +263,18 @@
};
   };
   
+ {

+   status = "okay";
+
+   i2s0_p0: port {
+   i2s0_p0_0: endpoint {
+   dai-format = "i2s";
+   mclk-fs = <256>;
+   remote-endpoint = <_p0_0>;
+   };
+   };
+};
+
{
status = "okay";
   



___
Linux-rockchip mailing list
linux-rockc...@lists.infradead.org
https://nam03.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists.infradead.org%2Fmailman%2Flistinfo%2Flinux-rockchipdata=02%7C01%7C%7Cfcb0d6c43ded4d5e7d4c08d69f1a226d%7C84df9e7fe9f640afb435%7C1%7C0%7C636871331922090426sdata=ABb%2Fo%2FMAGvFBH%2B37uQr6rzn%2B%2FXBAXyiGfv2%2BMO0RQoQ%3Dreserved=0








Re: [PATCH v3 2/2] arm64: dts: rockchip: add HDMI sound node for rk3328-rock64

2019-03-02 Thread Katsuhiro Suzuki

Ping...

On 2019/02/18 2:34, Katsuhiro Suzuki wrote:

This patch adds HDMI sound (I2S0) node for rock64.

After apply this patch, UART2 will fail to allocate DMA resources
but UART driver can work fine without DMA.

This error is related to the DMAC of rk3328 (pl330 or compatible).
DMAC connected to 16 DMA sources. Each sources have ID number that is
called 'Req number' in rk3328 TRM. Currently total 7 sources has been
activated as follows:

| Req number | Source | Required  |
||| channels  |
|++---|
| 14, 15 | I2S1   | 2ch   |
|  6,  7 | UART2  | 2ch   |
| 10 | SPDIF  | 1ch   |
|  8,  9 | SPI0   | 2ch   |
|++---|
|| Total  | 7ch   |

HDMI audio needs to activate new source I2S0 (Req number 11 and 12).
I2S0 can work concurrently with other sources, but rk3328 DMAC can
use max 8 channels at same time. If I2S0 is simply activated by this
patch, required DMAC channels will be 9. So last one (UART2) cannot
allocate the DMA resources.

Virt-dma mechanism for pl0330 DMAC driver is needed to fix this
problem.

Signed-off-by: Katsuhiro Suzuki 
---
  .../arm64/boot/dts/rockchip/rk3328-rock64.dts | 21 ++-
  1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts 
b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
index 2157a528276b..bfc0930d245c 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
@@ -68,7 +68,8 @@
sound {
compatible = "audio-graph-card";
label = "rockchip,rk3328";
-   dais = <_p0
+   dais = <_p0
+   _p0
_p0>;
};
  
@@ -141,6 +142,12 @@
  
   {

status = "okay";
+
+   port@0 {
+   hdmi_p0_0: endpoint {
+   remote-endpoint = <_p0_0>;
+   };
+   };
  };
  
   {

@@ -256,6 +263,18 @@
};
  };
  
+ {

+   status = "okay";
+
+   i2s0_p0: port {
+   i2s0_p0_0: endpoint {
+   dai-format = "i2s";
+   mclk-fs = <256>;
+   remote-endpoint = <_p0_0>;
+   };
+   };
+};
+
   {
status = "okay";
  





Re: [PATCH v2] clk: fractional-divider: check parent rate only if flag is set

2019-02-20 Thread Katsuhiro Suzuki

Ping...

On 2019/02/11 0:38, Katsuhiro Suzuki wrote:

Custom approximation of fractional-divider may not need parent clock
rate checking. For example Rockchip SoCs work fine using grand parent
clock rate even if target rate is greater than parent.

This patch checks parent clock rate only if CLK_SET_RATE_PARENT flag
is set.

For detailed example, clock tree of Rockchip I2S audio hardware.
   - Clock rate of CPLL is 1.2GHz, GPLL is 491.52MHz.
   - i2s1_div is integer divider can divide N (N is 1~128).
 Input clock is CPLL or GPLL. Initial divider value is N = 1.
 Ex) PLL = CPLL, N = 10, i2s1_div output rate is
   CPLL / 10 = 1.2GHz / 10 = 120MHz
   - i2s1_frac is fractional divider can divide input to x/y, x and
 y are 16bit integer.

CPLL --> | selector | ---> i2s1_div -+--> | selector | --> I2S1 MCLK
GPLL --> |  | ,--'|  |
   `--> i2s1_frac ---> |  |

Clock mux system try to choose suitable one from i2s1_div and
i2s1_frac for master clock (MCLK) of I2S1.

Bad scenario as follows:
   - Try to set MCLK to 8.192MHz (32kHz audio replay)
 Candidate setting is
 - i2s1_div: GPLL / 60 = 8.192MHz
 i2s1_div candidate is exactly same as target clock rate, so mux
 choose this clock source. i2s1_div output rate is changed
 491.52MHz -> 8.192MHz

   - After that try to set to 11.2896MHz (44.1kHz audio replay)
 Candidate settings are
 - i2s1_div : CPLL / 107 = 11.214945MHz
 - i2s1_frac: i2s1_div   = 8.192MHz
   This is because clk_fd_round_rate() thinks target rate
   (11.2896MHz) is higher than parent rate (i2s1_div = 8.192MHz)
   and returns parent clock rate.

Above is current upstreamed behavior. Clock mux system choose
i2s1_div, but this clock rate is not acceptable for I2S driver, so
users cannot replay audio.

Expected behavior is:
   - Try to set master clock to 11.2896MHz (44.1kHz audio replay)
 Candidate settings are
 - i2s1_div : CPLL / 107  = 11.214945MHz
 - i2s1_frac: i2s1_div * 147/6400 = 11.2896MHz
  Change i2s1_div to GPLL / 1 = 491.52MHz at same
  time.

If apply this commit, clk_fd_round_rate() calls custom approximate
function of Rockchip even if target rate is higher than parent.
Custom function changes both grand parent (i2s1_div) and parent
(i2s_frac) settings at same time. Clock mux system can choose
i2s1_frac and audio works fine.

Signed-off-by: Katsuhiro Suzuki 
---
  drivers/clk/clk-fractional-divider.c | 2 +-
  drivers/clk/clk.c| 8 
  include/linux/clk-provider.h | 1 +
  3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk-fractional-divider.c 
b/drivers/clk/clk-fractional-divider.c
index 545dceec0bbf..fdfe2e423d15 100644
--- a/drivers/clk/clk-fractional-divider.c
+++ b/drivers/clk/clk-fractional-divider.c
@@ -79,7 +79,7 @@ static long clk_fd_round_rate(struct clk_hw *hw, unsigned 
long rate,
unsigned long m, n;
u64 ret;
  
-	if (!rate || rate >= *parent_rate)

+   if (!rate || (!clk_hw_can_set_rate_parent(hw) && rate >= *parent_rate))
return *parent_rate;
  
  	if (fd->approximation)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index d2477a5058ac..070c0cb29ee8 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -518,6 +518,14 @@ void clk_hw_set_rate_range(struct clk_hw *hw, unsigned 
long min_rate,
  }
  EXPORT_SYMBOL_GPL(clk_hw_set_rate_range);
  
+bool clk_hw_can_set_rate_parent(struct clk_hw *hw)

+{
+   unsigned long flags = clk_hw_get_flags(hw);
+
+   return flags & CLK_SET_RATE_PARENT;
+}
+EXPORT_SYMBOL_GPL(clk_hw_can_set_rate_parent);
+
  /*
   * Helper for finding best parent to provide a given frequency. This can be 
used
   * directly as a determine_rate callback (e.g. for a mux), or from a more
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index e443fa9fa859..693a51937ded 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -808,6 +808,7 @@ int clk_mux_determine_rate_flags(struct clk_hw *hw,
  void clk_hw_reparent(struct clk_hw *hw, struct clk_hw *new_parent);
  void clk_hw_set_rate_range(struct clk_hw *hw, unsigned long min_rate,
   unsigned long max_rate);
+bool clk_hw_can_set_rate_parent(struct clk_hw *hw);
  
  static inline void __clk_hw_set_clk(struct clk_hw *dst, struct clk_hw *src)

  {





[PATCH] arm64: dts: rockchip: enable hdmi audio out for rk3399-rockpro64

2019-02-20 Thread Katsuhiro Suzuki
The rockpro64 has hdmi support. So this patch enables hdmi audio
feature that is defined in rk3399 devicetree.

Signed-off-by: Katsuhiro Suzuki 
---
 arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts | 4 
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts 
b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
index 1f2394e0587d..cae7d4d01e0c 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3399-rockpro64.dts
@@ -215,6 +215,10 @@
status = "okay";
 };
 
+_sound {
+   status = "okay";
+};
+
  {
ddc-i2c-bus = <>;
pinctrl-names = "default";
-- 
2.20.1



[PATCH 1/2] arm64: dts: rockchip: replace GPIO number into macro for rk3399

2019-02-19 Thread Katsuhiro Suzuki
This patch just replace some absolute GPIO numbers of I2C, I2S, UART,
SPDIF, SPI and so on for rk3399 devicetree into RK_PXn macro for more
readability.

Signed-off-by: Katsuhiro Suzuki 
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 236 +++
 1 file changed, 118 insertions(+), 118 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index db9d948c0b03..76fc2c7af80a 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -2045,14 +2045,14 @@
 
clock {
clk_32k: clk-32k {
-   rockchip,pins = <0 0 RK_FUNC_2 _pull_none>;
+   rockchip,pins = <0 RK_PA0 RK_FUNC_2 
_pull_none>;
};
};
 
edp {
edp_hpd: edp-hpd {
rockchip,pins =
-   <4 23 RK_FUNC_2 _pull_none>;
+   <4 RK_PC7 RK_FUNC_2 _pull_none>;
};
};
 
@@ -2060,167 +2060,167 @@
rgmii_pins: rgmii-pins {
rockchip,pins =
/* mac_txclk */
-   <3 17 RK_FUNC_1 _pull_none_13ma>,
+   <3 RK_PC1 RK_FUNC_1 
_pull_none_13ma>,
/* mac_rxclk */
-   <3 14 RK_FUNC_1 _pull_none>,
+   <3 RK_PB6 RK_FUNC_1 _pull_none>,
/* mac_mdio */
-   <3 13 RK_FUNC_1 _pull_none>,
+   <3 RK_PB5 RK_FUNC_1 _pull_none>,
/* mac_txen */
-   <3 12 RK_FUNC_1 _pull_none_13ma>,
+   <3 RK_PB4 RK_FUNC_1 
_pull_none_13ma>,
/* mac_clk */
-   <3 11 RK_FUNC_1 _pull_none>,
+   <3 RK_PB3 RK_FUNC_1 _pull_none>,
/* mac_rxdv */
-   <3 9 RK_FUNC_1 _pull_none>,
+   <3 RK_PB1 RK_FUNC_1 _pull_none>,
/* mac_mdc */
-   <3 8 RK_FUNC_1 _pull_none>,
+   <3 RK_PB0 RK_FUNC_1 _pull_none>,
/* mac_rxd1 */
-   <3 7 RK_FUNC_1 _pull_none>,
+   <3 RK_PA7 RK_FUNC_1 _pull_none>,
/* mac_rxd0 */
-   <3 6 RK_FUNC_1 _pull_none>,
+   <3 RK_PA6 RK_FUNC_1 _pull_none>,
/* mac_txd1 */
-   <3 5 RK_FUNC_1 _pull_none_13ma>,
+   <3 RK_PA5 RK_FUNC_1 
_pull_none_13ma>,
/* mac_txd0 */
-   <3 4 RK_FUNC_1 _pull_none_13ma>,
+   <3 RK_PA4 RK_FUNC_1 
_pull_none_13ma>,
/* mac_rxd3 */
-   <3 3 RK_FUNC_1 _pull_none>,
+   <3 RK_PA3 RK_FUNC_1 _pull_none>,
/* mac_rxd2 */
-   <3 2 RK_FUNC_1 _pull_none>,
+   <3 RK_PA2 RK_FUNC_1 _pull_none>,
/* mac_txd3 */
-   <3 1 RK_FUNC_1 _pull_none_13ma>,
+   <3 RK_PA1 RK_FUNC_1 
_pull_none_13ma>,
/* mac_txd2 */
-   <3 0 RK_FUNC_1 _pull_none_13ma>;
+   <3 RK_PA0 RK_FUNC_1 
_pull_none_13ma>;
};
 
rmii_pins: rmii-pins {
rockchip,pins =
/* mac_mdio */
-   <3 13 RK_FUNC_1 _pull_none>,
+   <3 RK_PB5 RK_FUNC_1 _pull_none>,
/* mac_txen */
-   <3 12 RK_FUNC_1 _pull_none_13ma>,
+  

[PATCH 2/2] arm64: dts: rockchip: fix cts, rts pin assign of UART3 for rk3399

2019-02-19 Thread Katsuhiro Suzuki
This patch fixes pin assign of cts and rts signal of UART3.

Currently GPIO3_C2 and C3 pins are assigned but TRM says that
GPIO3_C0 and C1 are correct.

Refer:
  RK3399 TRM v1.4 - Table 19-1 UART Interface Description

Signed-off-by: Katsuhiro Suzuki 
---
 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index 76fc2c7af80a..beaa92744a64 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -2535,12 +2535,12 @@
 
uart3_cts: uart3-cts {
rockchip,pins =
-   <3 RK_PC2 RK_FUNC_2 _pull_none>;
+   <3 RK_PC0 RK_FUNC_2 _pull_none>;
};
 
uart3_rts: uart3-rts {
rockchip,pins =
-   <3 RK_PC3 RK_FUNC_2 _pull_none>;
+   <3 RK_PC1 RK_FUNC_2 _pull_none>;
};
};
 
-- 
2.20.1



[PATCH v3 1/2] arm64: dts: rockchip: add #sound-dai-cells to HDMI of rk3328

2019-02-17 Thread Katsuhiro Suzuki
This patch adds #sound-dai-cells to use HDMI node as audio
codec from device tree of rk3328 boards.

Signed-off-by: Katsuhiro Suzuki 
---
 arch/arm64/boot/dts/rockchip/rk3328.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
index 84f14b132e8f..374b5da93a35 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
@@ -665,6 +665,7 @@
pinctrl-names = "default";
pinctrl-0 = <_cec _xfer _hpd>;
rockchip,grf = <>;
+   #sound-dai-cells = <0>;
status = "disabled";
 
ports {
-- 
2.20.1



[PATCH v3 2/2] arm64: dts: rockchip: add HDMI sound node for rk3328-rock64

2019-02-17 Thread Katsuhiro Suzuki
This patch adds HDMI sound (I2S0) node for rock64.

After apply this patch, UART2 will fail to allocate DMA resources
but UART driver can work fine without DMA.

This error is related to the DMAC of rk3328 (pl330 or compatible).
DMAC connected to 16 DMA sources. Each sources have ID number that is
called 'Req number' in rk3328 TRM. Currently total 7 sources has been
activated as follows:

| Req number | Source | Required  |
||| channels  |
|++---|
| 14, 15 | I2S1   | 2ch   |
|  6,  7 | UART2  | 2ch   |
| 10 | SPDIF  | 1ch   |
|  8,  9 | SPI0   | 2ch   |
|++---|
|| Total  | 7ch   |

HDMI audio needs to activate new source I2S0 (Req number 11 and 12).
I2S0 can work concurrently with other sources, but rk3328 DMAC can
use max 8 channels at same time. If I2S0 is simply activated by this
patch, required DMAC channels will be 9. So last one (UART2) cannot
allocate the DMA resources.

Virt-dma mechanism for pl0330 DMAC driver is needed to fix this
problem.

Signed-off-by: Katsuhiro Suzuki 
---
 .../arm64/boot/dts/rockchip/rk3328-rock64.dts | 21 ++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts 
b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
index 2157a528276b..bfc0930d245c 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
@@ -68,7 +68,8 @@
sound {
compatible = "audio-graph-card";
label = "rockchip,rk3328";
-   dais = <_p0
+   dais = <_p0
+   _p0
_p0>;
};
 
@@ -141,6 +142,12 @@
 
  {
status = "okay";
+
+   port@0 {
+   hdmi_p0_0: endpoint {
+   remote-endpoint = <_p0_0>;
+   };
+   };
 };
 
  {
@@ -256,6 +263,18 @@
};
 };
 
+ {
+   status = "okay";
+
+   i2s0_p0: port {
+   i2s0_p0_0: endpoint {
+   dai-format = "i2s";
+   mclk-fs = <256>;
+   remote-endpoint = <_p0_0>;
+   };
+   };
+};
+
  {
status = "okay";
 
-- 
2.20.1



Re: [PATCH] arm64: dts: rockchip: add HDMI sound node for rk3328-rock64

2019-02-17 Thread Katsuhiro Suzuki

Hello Heiko,

On 2019/02/12 20:12, Heiko Stübner wrote:

Hi,

Am Montag, 4. Februar 2019, 13:59:37 CET schrieb Katsuhiro Suzuki:

On 2019/02/03 18:06, Heiko Stuebner wrote:

Am Samstag, 2. Februar 2019, 05:34:44 CET schrieb Katsuhiro Suzuki:

This patch adds HDMI sound (I2S0) node and remove dma properties
from UART2 node for rock64.

The DMAC of rk3328 can use 8 channels at same time. Currently, total

7 channels are used as follows:
- I2S1  2ch
- UART2 2ch
- SPDIF 1ch
- SPI0  2ch

HDMI audio using I2S0 that requires 2ch but DMAC has only 1 channel.

UART2 can work without DMA resources, so this patch removes dma
allocation for UART2 and reuses it to I2S0.


I don't follow that description. How can i2s0 re-use the uart2 dma
channels? Looking at the dma table in the TRM, uart2 has channels 6+7
while i2s0 uses channels 11+12. They should just run concurrently?


Sorry for confusing... 6 or 7 is as ID number of slave DMA channel.
TRM calls it 'Req number'. Req number 6+7 and 11+12 can work
concurrently but TRM says DMAC can transfer 8 DMA channels at same
time. So all 16 Req numbers cannot activate at same time. It should
be keep less or equal than 8 numbers.


But that "shortcoming" of having more requests than channels is not
something specific to the pl330, instead most dma controllers have that
"problem", which seems to get solved by the virt-dma mechanism of
dmaengine - which pl330 doesn't use so far. (but see pl080 for example)



I understand. I drop these patches.



The devicetree only describes the hardware and is never meant as a
configuration space for kernel-code shortcomings.



Yes, right. I don't want to use device-tree as configuration space,
so which is better?

- Fix the pl330 driver first and after that add HDMI-sound node
  to device-tree.
- Just add HDMI-sound node to device-tree. If someone (include me)
  want to support virt-dma on pl330 driver, they will fix it.
  (PL330 will face error but all sounds works fine and UART still
  works fine with DMA error)



Heiko





Best Regards,
Katsuhiro Suzuki


Re: [PATCH] clk: fractional-divider: check parent rate only for general approximation

2019-02-10 Thread Katsuhiro Suzuki

Hello Stephen,

Thanks a lot for your review. I choose clk_hw_can_set_parent_rate()
macro and fix typo and patch description. And I post v2 patch.

Best Regards,
Katsuhiro Suzuki

On 2019/02/07 4:40, Stephen Boyd wrote:

+Heiko because it mentions Rockchip

Quoting Katsuhiro Suzuki (2019-01-30 15:50:22)

Custom approximation of fractional-divider may not need parent clock
rate checking. For example Rockchip SoCs work fine using grand parent
clock rate even if target rate is greater than parent.

This patch removes parent clock rate check from custom approximation.

For detailied example, clock tree of Rockchip I2S audio hardware.


s/detailied/detailed/


   - A i2s1_div is integer divider can divide input clock 1/1 ~ 1/16.
 Initialize divider value is 1.


s/Initialize/Initial/


   - A i2s1_frac is fractional divider can divide input to x/y, x and
 y are 16bit integer.
   - Clock rate of CPLL is 1.2GHz, GPLL is 491.52MHz.

CPLL --> | selector | ---> i2s1_div -+--> | selector | --> I2S1 MCLK
GPLL --> |  | ,--'|  |
   `--> i2s1_frac ---> |  |

Clock mux system try to choose suitable one from i2s1_div and
i2s1_frac for master clock (MCLK) of I2S1.

Bad scenario as follows:
   - Try to set MCLK to 8.192MHz (32kHz audio replay)
 Candidate setting is
 - i2s1_div : GPLL / 60   = 8.192MHz
 i2s1_div is same as target clock rate, so mux choose this.
 i2s1_div output rate is changed 491.52MHz -> 8.192MHz

   - After that try to set to 11.2896MHz (44.1kHz audio replay)
 Candidate settings are
 - i2s1_div : CPLL / 107 = 11.214945MHz
 - i2s1_frac: i2s1_div   = 8.192MHz
   This is because clk_fd_round_rate() thinks target rate
   (11.2896MHz) is higher than parent rate (i2s1_div = 8.192MHz)
   and returns parent clock rate.
 Clock mux system choose i2s1_div, but this clock rate is not
 acceptable for I2S driver, so users cannot replay audio.

Expected behavior is:
   - Try to set master clock to 11.2896MHz (44.1kHz audio replay)
 Candidate settings are
 - i2s1_div : CPLL / 107  = 11.214945MHz
 - i2s1_frac: i2s1_div * 147/6400 = 11.2896MHz
  (change i2s1_div to GPLL / 1 = 491.52MHz)
   If apply this commit, clk_fd_round_rate() calls custom
   approximate function of Rockchip even if target rate is higher
   than parent. Custom function changes both grand parent
   (i2s1_div) and parent (i2s_frac) settings at same time.


Can you indicate what that function is? Is this merged upstream or
something you're developing now?


 Clock mux system can choose i2s1_frac and audio works fine.


I think this last paragraph from "If apply this commit" can be
unindented from the expected behavior section.



Signed-off-by: Katsuhiro Suzuki 
---
  drivers/clk/clk-fractional-divider.c | 10 +++---
  1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/clk-fractional-divider.c 
b/drivers/clk/clk-fractional-divider.c
index 545dceec0bbf..b0fc5509e0ff 100644
--- a/drivers/clk/clk-fractional-divider.c
+++ b/drivers/clk/clk-fractional-divider.c
@@ -79,13 +79,17 @@ static long clk_fd_round_rate(struct clk_hw *hw, unsigned 
long rate,
 unsigned long m, n;
 u64 ret;
  
-   if (!rate || rate >= *parent_rate)

+   if (!rate)
 return *parent_rate;


Ok. I think it would be clearer if we had

unsigned long flags = clk_hw_get_flags()

if (!rate ||
!(flags & CLK_SET_PARENT_RATE) && rate >= *parent_rate)

indicating that the parent of the clk isn't expected to change rate so
we can only achieve that much frequency. Plus some sort of comment to
this effect would be helpful too.

We could also introduce macros to check clk flags. Then it would read
nicer:

if (!rate || clk_hw_can_set_parent_rate(hw) && rate >= ...)







[PATCH v2] clk: fractional-divider: check parent rate only if flag is set

2019-02-10 Thread Katsuhiro Suzuki
Custom approximation of fractional-divider may not need parent clock
rate checking. For example Rockchip SoCs work fine using grand parent
clock rate even if target rate is greater than parent.

This patch checks parent clock rate only if CLK_SET_RATE_PARENT flag
is set.

For detailed example, clock tree of Rockchip I2S audio hardware.
  - Clock rate of CPLL is 1.2GHz, GPLL is 491.52MHz.
  - i2s1_div is integer divider can divide N (N is 1~128).
Input clock is CPLL or GPLL. Initial divider value is N = 1.
Ex) PLL = CPLL, N = 10, i2s1_div output rate is
  CPLL / 10 = 1.2GHz / 10 = 120MHz
  - i2s1_frac is fractional divider can divide input to x/y, x and
y are 16bit integer.

CPLL --> | selector | ---> i2s1_div -+--> | selector | --> I2S1 MCLK
GPLL --> |  | ,--'|  |
  `--> i2s1_frac ---> |  |

Clock mux system try to choose suitable one from i2s1_div and
i2s1_frac for master clock (MCLK) of I2S1.

Bad scenario as follows:
  - Try to set MCLK to 8.192MHz (32kHz audio replay)
Candidate setting is
- i2s1_div: GPLL / 60 = 8.192MHz
i2s1_div candidate is exactly same as target clock rate, so mux
choose this clock source. i2s1_div output rate is changed
491.52MHz -> 8.192MHz

  - After that try to set to 11.2896MHz (44.1kHz audio replay)
Candidate settings are
- i2s1_div : CPLL / 107 = 11.214945MHz
- i2s1_frac: i2s1_div   = 8.192MHz
  This is because clk_fd_round_rate() thinks target rate
  (11.2896MHz) is higher than parent rate (i2s1_div = 8.192MHz)
  and returns parent clock rate.

Above is current upstreamed behavior. Clock mux system choose
i2s1_div, but this clock rate is not acceptable for I2S driver, so
users cannot replay audio.

Expected behavior is:
  - Try to set master clock to 11.2896MHz (44.1kHz audio replay)
Candidate settings are
- i2s1_div : CPLL / 107  = 11.214945MHz
- i2s1_frac: i2s1_div * 147/6400 = 11.2896MHz
 Change i2s1_div to GPLL / 1 = 491.52MHz at same
 time.

If apply this commit, clk_fd_round_rate() calls custom approximate
function of Rockchip even if target rate is higher than parent.
Custom function changes both grand parent (i2s1_div) and parent
(i2s_frac) settings at same time. Clock mux system can choose
i2s1_frac and audio works fine.

Signed-off-by: Katsuhiro Suzuki 
---
 drivers/clk/clk-fractional-divider.c | 2 +-
 drivers/clk/clk.c| 8 
 include/linux/clk-provider.h | 1 +
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk-fractional-divider.c 
b/drivers/clk/clk-fractional-divider.c
index 545dceec0bbf..fdfe2e423d15 100644
--- a/drivers/clk/clk-fractional-divider.c
+++ b/drivers/clk/clk-fractional-divider.c
@@ -79,7 +79,7 @@ static long clk_fd_round_rate(struct clk_hw *hw, unsigned 
long rate,
unsigned long m, n;
u64 ret;
 
-   if (!rate || rate >= *parent_rate)
+   if (!rate || (!clk_hw_can_set_rate_parent(hw) && rate >= *parent_rate))
return *parent_rate;
 
if (fd->approximation)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index d2477a5058ac..070c0cb29ee8 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -518,6 +518,14 @@ void clk_hw_set_rate_range(struct clk_hw *hw, unsigned 
long min_rate,
 }
 EXPORT_SYMBOL_GPL(clk_hw_set_rate_range);
 
+bool clk_hw_can_set_rate_parent(struct clk_hw *hw)
+{
+   unsigned long flags = clk_hw_get_flags(hw);
+
+   return flags & CLK_SET_RATE_PARENT;
+}
+EXPORT_SYMBOL_GPL(clk_hw_can_set_rate_parent);
+
 /*
  * Helper for finding best parent to provide a given frequency. This can be 
used
  * directly as a determine_rate callback (e.g. for a mux), or from a more
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index e443fa9fa859..693a51937ded 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -808,6 +808,7 @@ int clk_mux_determine_rate_flags(struct clk_hw *hw,
 void clk_hw_reparent(struct clk_hw *hw, struct clk_hw *new_parent);
 void clk_hw_set_rate_range(struct clk_hw *hw, unsigned long min_rate,
   unsigned long max_rate);
+bool clk_hw_can_set_rate_parent(struct clk_hw *hw);
 
 static inline void __clk_hw_set_clk(struct clk_hw *dst, struct clk_hw *src)
 {
-- 
2.20.1



[PATCH v2 2/2] arm64: dts: rockchip: add HDMI sound node for rk3328-rock64

2019-02-06 Thread Katsuhiro Suzuki
This patch adds HDMI sound (I2S0) node and remove dma properties
from UART2 node for rock64.

The DMAC of rk3328 connected to 16 DMA sources. Each sources have ID
number that is called 'Req number' in rk3328 TRM. Currently, total
7 sources has been activated as follows:

| Req number | Source | Required  |
||| channels  |
|++---|
| 14, 15 | I2S1   | 2ch   |
|  6,  7 | UART2  | 2ch   |
| 10 | SPDIF  | 1ch   |
|  8,  9 | SPI0   | 2ch   |
|++---|
|| Total  | 7ch   |

HDMI audio needs to activate new source I2S0 (Req number 11 and 12).
I2S0 can work concurrently with other sources, but rk3328 DMAC can
use max 8 channels at same time. If I2S0 is simply activated,
required DMAC channels are 9. It does not work.

UART2 can work without DMA resources, so this patch removes 2 DMA
channel allocation for UART2. These released channels can be used
for I2S0.

Signed-off-by: Katsuhiro Suzuki 
---
 .../arm64/boot/dts/rockchip/rk3328-rock64.dts | 24 ++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts 
b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
index 2157a528276b..e21645aa3fa5 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
@@ -68,7 +68,8 @@
sound {
compatible = "audio-graph-card";
label = "rockchip,rk3328";
-   dais = <_p0
+   dais = <_p0
+   _p0
_p0>;
};
 
@@ -141,6 +142,12 @@
 
  {
status = "okay";
+
+   port@0 {
+   hdmi_p0_0: endpoint {
+   remote-endpoint = <_p0_0>;
+   };
+   };
 };
 
  {
@@ -256,6 +263,18 @@
};
 };
 
+ {
+   status = "okay";
+
+   i2s0_p0: port {
+   i2s0_p0_0: endpoint {
+   dai-format = "i2s";
+   mclk-fs = <256>;
+   remote-endpoint = <_p0_0>;
+   };
+   };
+};
+
  {
status = "okay";
 
@@ -343,6 +362,9 @@
 
  {
status = "okay";
+
+   /delete-property/ dmas;
+   /delete-property/ dma-names;
 };
 
  {
-- 
2.20.1



[PATCH v2 1/2] arm64: dts: rockchip: add #sound-dai-cells to HDMI of rk3328

2019-02-06 Thread Katsuhiro Suzuki
This patch adds #sound-dai-cells to use HDMI node as audio
codec from device tree of rk3328 boards.

Signed-off-by: Katsuhiro Suzuki 
---
 arch/arm64/boot/dts/rockchip/rk3328.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
index 84f14b132e8f..374b5da93a35 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
@@ -665,6 +665,7 @@
pinctrl-names = "default";
pinctrl-0 = <_cec _xfer _hpd>;
rockchip,grf = <>;
+   #sound-dai-cells = <0>;
status = "disabled";
 
ports {
-- 
2.20.1



Re: [PATCH] arm64: dts: rockchip: add HDMI sound node for rk3328-rock64

2019-02-04 Thread Katsuhiro Suzuki

Hello Heiko,

On 2019/02/03 18:06, Heiko Stuebner wrote:

Am Samstag, 2. Februar 2019, 05:34:44 CET schrieb Katsuhiro Suzuki:

This patch adds HDMI sound (I2S0) node and remove dma properties
from UART2 node for rock64.

The DMAC of rk3328 can use 8 channels at same time. Currently, total
7 channels are used as follows:
   - I2S1  2ch
   - UART2 2ch
   - SPDIF 1ch
   - SPI0  2ch

HDMI audio using I2S0 that requires 2ch but DMAC has only 1 channel.

UART2 can work without DMA resources, so this patch removes dma
allocation for UART2 and reuses it to I2S0.


I don't follow that description. How can i2s0 re-use the uart2 dma channels?
Looking at the dma table in the TRM, uart2 has channels 6+7 while i2s0
uses channels 11+12. They should just run concurrently?



Sorry for confusing... 6 or 7 is as ID number of slave DMA channel.
TRM calls it 'Req number'. Req number 6+7 and 11+12 can work
concurrently but TRM says DMAC can transfer 8 DMA channels at same
time. So all 16 Req numbers cannot activate at same time. It should
be keep less or equal than 8 numbers.

For details, DMAC of RK3328 is ARM PL330 (or compatible IP).
  - Local variable 'chan_id' of of_dma_pl330_xlate() is Req number.
This is from Device-Tree info.
  - Array 'channels' of struct pl330_dmac is DMA channels of DMAC.
pl330_request_channel() allocate DMA channel that is requested from
other drivers. Local variable 'chans' has max channels can run
concurrently.

Current setting:
  channels  chan_id
  0 8
  1 9
  2 14
  3 15
  4 10
  5 6
  6 7
  7 (not used)

Best Regards,
Katsuhiro Suzuki




Signed-off-by: Katsuhiro Suzuki 
---
  .../arm64/boot/dts/rockchip/rk3328-rock64.dts | 24 ++-
  arch/arm64/boot/dts/rockchip/rk3328.dtsi  |  1 +
  2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts 
b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
index 2157a528276b..e21645aa3fa5 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
@@ -68,7 +68,8 @@
sound {
compatible = "audio-graph-card";
label = "rockchip,rk3328";
-   dais = <_p0
+   dais = <_p0
+   _p0
_p0>;
};
  
@@ -141,6 +142,12 @@
  
   {

status = "okay";
+
+   port@0 {
+   hdmi_p0_0: endpoint {
+   remote-endpoint = <_p0_0>;
+   };
+   };
  };
  
   {

@@ -256,6 +263,18 @@
};
  };
  
+ {

+   status = "okay";
+
+   i2s0_p0: port {
+   i2s0_p0_0: endpoint {
+   dai-format = "i2s";
+   mclk-fs = <256>;
+   remote-endpoint = <_p0_0>;
+   };
+   };
+};
+
   {
status = "okay";
  
@@ -343,6 +362,9 @@
  
   {

status = "okay";
+
+   /delete-property/ dmas;
+   /delete-property/ dma-names;
  };
  
   {

diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
index 84f14b132e8f..374b5da93a35 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
@@ -665,6 +665,7 @@
pinctrl-names = "default";
pinctrl-0 = <_cec _xfer _hpd>;
rockchip,grf = <>;
+   #sound-dai-cells = <0>;


please make that a separate patch


Heiko







[PATCH] arm64: dts: rockchip: add HDMI sound node for rk3328-rock64

2019-02-01 Thread Katsuhiro Suzuki
This patch adds HDMI sound (I2S0) node and remove dma properties
from UART2 node for rock64.

The DMAC of rk3328 can use 8 channels at same time. Currently, total
7 channels are used as follows:
  - I2S1  2ch
  - UART2 2ch
  - SPDIF 1ch
  - SPI0  2ch

HDMI audio using I2S0 that requires 2ch but DMAC has only 1 channel.

UART2 can work without DMA resources, so this patch removes dma
allocation for UART2 and reuses it to I2S0.

Signed-off-by: Katsuhiro Suzuki 
---
 .../arm64/boot/dts/rockchip/rk3328-rock64.dts | 24 ++-
 arch/arm64/boot/dts/rockchip/rk3328.dtsi  |  1 +
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts 
b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
index 2157a528276b..e21645aa3fa5 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
@@ -68,7 +68,8 @@
sound {
compatible = "audio-graph-card";
label = "rockchip,rk3328";
-   dais = <_p0
+   dais = <_p0
+   _p0
_p0>;
};
 
@@ -141,6 +142,12 @@
 
  {
status = "okay";
+
+   port@0 {
+   hdmi_p0_0: endpoint {
+   remote-endpoint = <_p0_0>;
+   };
+   };
 };
 
  {
@@ -256,6 +263,18 @@
};
 };
 
+ {
+   status = "okay";
+
+   i2s0_p0: port {
+   i2s0_p0_0: endpoint {
+   dai-format = "i2s";
+   mclk-fs = <256>;
+   remote-endpoint = <_p0_0>;
+   };
+   };
+};
+
  {
status = "okay";
 
@@ -343,6 +362,9 @@
 
  {
status = "okay";
+
+   /delete-property/ dmas;
+   /delete-property/ dma-names;
 };
 
  {
diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
index 84f14b132e8f..374b5da93a35 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
@@ -665,6 +665,7 @@
pinctrl-names = "default";
pinctrl-0 = <_cec _xfer _hpd>;
rockchip,grf = <>;
+   #sound-dai-cells = <0>;
status = "disabled";
 
ports {
-- 
2.20.1



[PATCH] clk: fractional-divider: check parent rate only for general approximation

2019-01-30 Thread Katsuhiro Suzuki
Custom approximation of fractional-divider may not need parent clock
rate checking. For example Rockchip SoCs work fine using grand parent
clock rate even if target rate is greater than parent.

This patch removes parent clock rate check from custom approximation.

For detailied example, clock tree of Rockchip I2S audio hardware.
  - A i2s1_div is integer divider can divide input clock 1/1 ~ 1/16.
Initialize divider value is 1.
  - A i2s1_frac is fractional divider can divide input to x/y, x and
y are 16bit integer.
  - Clock rate of CPLL is 1.2GHz, GPLL is 491.52MHz.

CPLL --> | selector | ---> i2s1_div -+--> | selector | --> I2S1 MCLK
GPLL --> |  | ,--'|  |
  `--> i2s1_frac ---> |  |

Clock mux system try to choose suitable one from i2s1_div and
i2s1_frac for master clock (MCLK) of I2S1.

Bad scenario as follows:
  - Try to set MCLK to 8.192MHz (32kHz audio replay)
Candidate setting is
- i2s1_div : GPLL / 60   = 8.192MHz
i2s1_div is same as target clock rate, so mux choose this.
i2s1_div output rate is changed 491.52MHz -> 8.192MHz

  - After that try to set to 11.2896MHz (44.1kHz audio replay)
Candidate settings are
- i2s1_div : CPLL / 107 = 11.214945MHz
- i2s1_frac: i2s1_div   = 8.192MHz
  This is because clk_fd_round_rate() thinks target rate
  (11.2896MHz) is higher than parent rate (i2s1_div = 8.192MHz)
  and returns parent clock rate.
Clock mux system choose i2s1_div, but this clock rate is not
acceptable for I2S driver, so users cannot replay audio.

Expected behavior is:
  - Try to set master clock to 11.2896MHz (44.1kHz audio replay)
Candidate settings are
- i2s1_div : CPLL / 107  = 11.214945MHz
- i2s1_frac: i2s1_div * 147/6400 = 11.2896MHz
 (change i2s1_div to GPLL / 1 = 491.52MHz)
  If apply this commit, clk_fd_round_rate() calls custom
  approximate function of Rockchip even if target rate is higher
  than parent. Custom function changes both grand parent
  (i2s1_div) and parent (i2s_frac) settings at same time.
Clock mux system can choose i2s1_frac and audio works fine.

Signed-off-by: Katsuhiro Suzuki 
---
 drivers/clk/clk-fractional-divider.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/clk-fractional-divider.c 
b/drivers/clk/clk-fractional-divider.c
index 545dceec0bbf..b0fc5509e0ff 100644
--- a/drivers/clk/clk-fractional-divider.c
+++ b/drivers/clk/clk-fractional-divider.c
@@ -79,13 +79,17 @@ static long clk_fd_round_rate(struct clk_hw *hw, unsigned 
long rate,
unsigned long m, n;
u64 ret;
 
-   if (!rate || rate >= *parent_rate)
+   if (!rate)
return *parent_rate;
 
-   if (fd->approximation)
+   if (fd->approximation) {
fd->approximation(hw, rate, parent_rate, , );
-   else
+   } else {
+   if (rate >= *parent_rate)
+   return *parent_rate;
+
clk_fd_general_approximation(hw, rate, parent_rate, , );
+   }
 
ret = (u64)*parent_rate * m;
do_div(ret, n);
-- 
2.20.1



Re: [PATCH] clk: fractional-divider: check parent rate only for general approximation

2019-01-30 Thread Katsuhiro Suzuki

Hello Stephen,

Thank you for your comment.
I'll add detail description and send it.

Best Regards,
Katsuhiro Suzuki

On 2019/01/30 6:55, Stephen Boyd wrote:

Quoting Katsuhiro Suzuki (2019-01-07 05:21:24)

Custom approximation of fractional-divider may not need parent clock
rate checking. For example Rockchip SoCs work fine using grand parent
clock rate evne if target rate is greater than parent.

This patch removes parent clock rate check from custom approximation.

Signed-off-by: Katsuhiro Suzuki 
---
  drivers/clk/clk-fractional-divider.c | 10 +++---
  1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/clk-fractional-divider.c 
b/drivers/clk/clk-fractional-divider.c
index 545dceec0bbf..b0fc5509e0ff 100644
--- a/drivers/clk/clk-fractional-divider.c
+++ b/drivers/clk/clk-fractional-divider.c
@@ -79,13 +79,17 @@ static long clk_fd_round_rate(struct clk_hw *hw, unsigned 
long rate,
 unsigned long m, n;
 u64 ret;
  
-   if (!rate || rate >= *parent_rate)

+   if (!rate)


Is your fraction multiplying the rate up? If it's a divider it shouldn't
be increasing the rate more than whatever the parent is supplying so
this looks odd. How does the grandparent matter here? Maybe you can show
the example in the commit text so we can all understand what's going on
here.


 return *parent_rate;
  







Re: [PATCH] clk: fractional-divider: check parent rate only for general approximation

2019-01-26 Thread Katsuhiro Suzuki

Ping...

On 2019/01/07 22:21, Katsuhiro Suzuki wrote:

Custom approximation of fractional-divider may not need parent clock
rate checking. For example Rockchip SoCs work fine using grand parent
clock rate evne if target rate is greater than parent.

This patch removes parent clock rate check from custom approximation.

Signed-off-by: Katsuhiro Suzuki 
---
  drivers/clk/clk-fractional-divider.c | 10 +++---
  1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/clk-fractional-divider.c 
b/drivers/clk/clk-fractional-divider.c
index 545dceec0bbf..b0fc5509e0ff 100644
--- a/drivers/clk/clk-fractional-divider.c
+++ b/drivers/clk/clk-fractional-divider.c
@@ -79,13 +79,17 @@ static long clk_fd_round_rate(struct clk_hw *hw, unsigned 
long rate,
unsigned long m, n;
u64 ret;
  
-	if (!rate || rate >= *parent_rate)

+   if (!rate)
return *parent_rate;
  
-	if (fd->approximation)

+   if (fd->approximation) {
fd->approximation(hw, rate, parent_rate, , );
-   else
+   } else {
+   if (rate >= *parent_rate)
+   return *parent_rate;
+
clk_fd_general_approximation(hw, rate, parent_rate, , );
+   }
  
  	ret = (u64)*parent_rate * m;

do_div(ret, n);





[PATCH] clk: fractional-divider: check parent rate only for general approximation

2019-01-07 Thread Katsuhiro Suzuki
Custom approximation of fractional-divider may not need parent clock
rate checking. For example Rockchip SoCs work fine using grand parent
clock rate evne if target rate is greater than parent.

This patch removes parent clock rate check from custom approximation.

Signed-off-by: Katsuhiro Suzuki 
---
 drivers/clk/clk-fractional-divider.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/clk-fractional-divider.c 
b/drivers/clk/clk-fractional-divider.c
index 545dceec0bbf..b0fc5509e0ff 100644
--- a/drivers/clk/clk-fractional-divider.c
+++ b/drivers/clk/clk-fractional-divider.c
@@ -79,13 +79,17 @@ static long clk_fd_round_rate(struct clk_hw *hw, unsigned 
long rate,
unsigned long m, n;
u64 ret;
 
-   if (!rate || rate >= *parent_rate)
+   if (!rate)
return *parent_rate;
 
-   if (fd->approximation)
+   if (fd->approximation) {
fd->approximation(hw, rate, parent_rate, , );
-   else
+   } else {
+   if (rate >= *parent_rate)
+   return *parent_rate;
+
clk_fd_general_approximation(hw, rate, parent_rate, , );
+   }
 
ret = (u64)*parent_rate * m;
do_div(ret, n);
-- 
2.20.1



Re: [PATCH] arm64: dts: rockchip: enable analog audio node for rock64

2018-12-24 Thread Katsuhiro Suzuki

Hello Heiko,

I understand. Thanks a lot!

Best Regards,
Katsuhiro Suzuki

On 2018/12/24 16:16, Heiko Stuebner wrote:

Hi,

Am Sonntag, 23. Dezember 2018, 11:10:26 CET schrieb Katsuhiro Suzuki:

The Rock64 boards has analog audio jack on it. RK3328 can output
analog audio signal using I2S1 and ACODEC core.

This patch adds sound node for analog audio for Rock64.

Signed-off-by: Katsuhiro Suzuki 


I've put a patch collecting the sound-dai-cells in rk3328.dtsi
before this one, dropped the sound-dai-cells from here and
applied the patch for 4.22.

Thanks
Heiko







[PATCH] arm64: dts: rockchip: enable analog audio node for rock64

2018-12-23 Thread Katsuhiro Suzuki
The Rock64 boards has analog audio jack on it. RK3328 can output
analog audio signal using I2S1 and ACODEC core.

This patch adds sound node for analog audio for Rock64.

Signed-off-by: Katsuhiro Suzuki 
---

This patch depends on the following ACODEC node patch. If ACODEC
patch is not good to you, please drop this patch too.
  https://lkml.org/lkml/2018/12/23/53

 .../arm64/boot/dts/rockchip/rk3328-rock64.dts | 27 ++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts 
b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
index bd937d68ca3b..0a71e1287a1b 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
@@ -66,7 +66,8 @@
sound {
compatible = "audio-graph-card";
label = "rockchip,rk3328";
-   dais = <_p0>;
+   dais = <_p0
+   _p0>;
};
 
spdif-dit {
@@ -81,6 +82,17 @@
};
 };
 
+ {
+   status = "okay";
+   #sound-dai-cells = <0>;
+
+   port@0 {
+   codec_p0_0: endpoint {
+   remote-endpoint = <_p0_0>;
+   };
+   };
+};
+
  {
cpu-supply = <_arm>;
 };
@@ -243,6 +255,19 @@
};
 };
 
+ {
+   status = "okay";
+   #sound-dai-cells = <0>;
+
+   i2s1_p0: port {
+   i2s1_p0_0: endpoint {
+   dai-format = "i2s";
+   mclk-fs = <256>;
+   remote-endpoint = <_p0_0>;
+   };
+   };
+};
+
 _domains {
status = "okay";
 
-- 
2.19.2



[PATCH] arm64: dts: rockchip: add rk3328 ACODEC node

2018-12-23 Thread Katsuhiro Suzuki
This patch adds audio codec (ACODEC) node that converts to analog
audio signal from I2S for rk3328.

Signed-off-by: Katsuhiro Suzuki 
---

This patch depends on Rockcihp RK3328 ACODEC driver patches that were
applied in ALSA SoC tree. We can see the patches on this thread.
  https://lkml.org/lkml/2018/12/20/665

 arch/arm64/boot/dts/rockchip/rk3328.dtsi | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3328.dtsi 
b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
index ecd7f19c3542..0f02b1dc87e3 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3328.dtsi
@@ -672,6 +672,16 @@
};
};
 
+   codec: codec@ff41 {
+   compatible = "rockchip,rk3328-codec";
+   reg = <0x0 0xff41 0x0 0x1000>;
+   rockchip,grf = <>;
+   clocks = < PCLK_ACODECPHY>, < SCLK_I2S1>;
+   clock-names = "pclk", "mclk";
+   #sound-dai-cells = <0>;
+   status = "disabled";
+   };
+
hdmiphy: phy@ff43 {
compatible = "rockchip,rk3328-hdmi-phy";
reg = <0x0 0xff43 0x0 0x1>;
-- 
2.19.2



[PATCH] clk: rockchip: fix frac settings of GPLL clock for rk3328

2018-12-22 Thread Katsuhiro Suzuki
This patch fixes settings of GPLL frequency in fractional mode for
rk3328. In this mode, FOUTVCO is calcurated by following formula:
  FOUTVCO = FREF * FBDIV / REFDIV + ((FREF * FRAC / REFDIV) >> 24)

The problem is in FREF * FRAC >> 24 term. This result always lacks
one from target value is specified by rate member. For example first
itme of rk3328_pll_frac_rate originally has
  - rate  : 1016064000
  - refdiv: 3
  - fbdiv : 127
  - frac  : 134217
  - FREF * FBDIV / REFDIV= 101600
  - (FREF * FRAC / REFDIV) >> 24 = 63999
Thus calculated rate is 1016063999. It seems wrong.

If frac has 134218 (it is increased 1 from original value), second
term is 64000. All other items have same situation. So this patch
adds 1 to frac member in all items of rk3328_pll_frac_rate.

Signed-off-by: Katsuhiro Suzuki 
---
 drivers/clk/rockchip/clk-rk3328.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/rockchip/clk-rk3328.c 
b/drivers/clk/rockchip/clk-rk3328.c
index faa94adb2a37..65ab5c2f48b0 100644
--- a/drivers/clk/rockchip/clk-rk3328.c
+++ b/drivers/clk/rockchip/clk-rk3328.c
@@ -78,17 +78,17 @@ static struct rockchip_pll_rate_table rk3328_pll_rates[] = {
 
 static struct rockchip_pll_rate_table rk3328_pll_frac_rates[] = {
/* _mhz, _refdiv, _fbdiv, _postdiv1, _postdiv2, _dsmpd, _frac */
-   RK3036_PLL_RATE(1016064000, 3, 127, 1, 1, 0, 134217),
+   RK3036_PLL_RATE(1016064000, 3, 127, 1, 1, 0, 134218),
/* vco = 1016064000 */
-   RK3036_PLL_RATE(98304, 24, 983, 1, 1, 0, 671088),
+   RK3036_PLL_RATE(98304, 24, 983, 1, 1, 0, 671089),
/* vco = 98304 */
-   RK3036_PLL_RATE(49152, 24, 983, 2, 1, 0, 671088),
+   RK3036_PLL_RATE(49152, 24, 983, 2, 1, 0, 671089),
/* vco = 98304 */
-   RK3036_PLL_RATE(6144, 6, 215, 7, 2, 0, 671088),
+   RK3036_PLL_RATE(6144, 6, 215, 7, 2, 0, 671089),
/* vco = 860156000 */
-   RK3036_PLL_RATE(56448000, 12, 451, 4, 4, 0, 9797894),
+   RK3036_PLL_RATE(56448000, 12, 451, 4, 4, 0, 9797895),
/* vco = 903168000 */
-   RK3036_PLL_RATE(4096, 12, 409, 4, 5, 0, 10066329),
+   RK3036_PLL_RATE(4096, 12, 409, 4, 5, 0, 10066330),
/* vco = 81920 */
{ /* sentinel */ },
 };
-- 
2.19.2



[PATCH 2/2] ASoC: rockchip: add workaround for silence of rk3288 ACODEC

2018-12-20 Thread Katsuhiro Suzuki
This patch adds reset and precharge in shutdown of PCM device.

ACODEC goes to silence if we change Fs to 44.1kHz from 48kHz. This
workaround seems to work but I don't know this workaround is correct
sequence or not for ACODEC.

Signed-off-by: Katsuhiro Suzuki 
---
 sound/soc/codecs/rk3328_codec.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/rk3328_codec.c b/sound/soc/codecs/rk3328_codec.c
index 71f3fc2d970c..f3442a2283ea 100644
--- a/sound/soc/codecs/rk3328_codec.c
+++ b/sound/soc/codecs/rk3328_codec.c
@@ -261,9 +261,12 @@ static int rk3328_codec_close_playback(struct 
rk3328_codec_priv *rk3328)
mdelay(1);
}
 
+   /* Workaround for silence when changed Fs 48 -> 44.1kHz */
+   rk3328_codec_reset(rk3328);
+
regmap_update_bits(rk3328->regmap, DAC_PRECHARGE_CTRL,
   DAC_CHARGE_CURRENT_ALL_MASK,
-  DAC_CHARGE_CURRENT_I);
+  DAC_CHARGE_CURRENT_ALL_ON);
 
return 0;
 }
-- 
2.19.2



  1   2   3   4   >