2024-03-08 15:31:24

by Frank Li

[permalink] [raw]
Subject: [PATCH v6 0/4] arm64: dts: imx8qxp add asrc and sai

Update binding doc to avoid warning.
Change from v1 to v2
- Fixed dts DTB_CHECK warning

Signed-off-by: Frank Li <[email protected]>
---
Changes in v6:
- Add interrupt description in binding doc according to rob suggestion
- Link to v5: https://lore.kernel.org/r/[email protected]

Changes in v5:
- using rob's suggest logic after fix maxItems.
- sort dts nodes.
- remove spdif1. Add later when do 8qm upstream
- Link to v4: https://lore.kernel.org/r/[email protected]

Changes in v4:
Combine comments' from v2 and v3. I hope I address everythings.

- Krzysztof's comments
- add reson about why change

- rob's comments
using rob's suggest logic to restrict interrupt number
but for dma-names, still need use oneOf to cover 3 case
- [rx, tx]
- [rx]
- [tx]

oneOf
- items:
- tx
- rx
- enums: [rx, tx]

- Conor's comments
- add power-domains required for imx8qxp and imx8qm
- remove dmas descript, not allow use index to get dma-channel. Current
no user using this method.

- Link to v3: https://lore.kernel.org/r/[email protected]

Changes in v3:
- Fixed dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/sound/fsl,spdif.example.dtb: spdif@2004000: interrupts: [[0, 52, 4]] is too short
from schema $id: http://devicetree.org/schemas/sound/fsl,spdif.yaml#

- Link to v2: https://lore.kernel.org/r/[email protected]

---
Frank Li (4):
ASoC: dt-bindings: fsl,imx-asrc/spdif: Add power-domains property
ASoC: dt-bindings: fsl,imx-asrc: update max interrupt numbers
ASoC: dt-bindings: fsl-sai: allow only one dma-names
arm64: dts: imx8qxp: add asrc[0,1], esai0, spdif0 and sai[4,5]

.../devicetree/bindings/sound/fsl,imx-asrc.yaml | 14 ++
.../devicetree/bindings/sound/fsl,sai.yaml | 8 +-
.../devicetree/bindings/sound/fsl,spdif.yaml | 36 ++-
arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi | 267 +++++++++++++++++++++
4 files changed, 319 insertions(+), 6 deletions(-)
---
base-commit: ca301cf599a4eeafed8e3dd87bf8d2fe810e075e
change-id: 20240227-asrc_8qxp-25aa6783840f

Best regards,
--
Frank Li <[email protected]>



2024-03-08 15:31:35

by Frank Li

[permalink] [raw]
Subject: [PATCH v6 1/4] ASoC: dt-bindings: fsl,imx-asrc/spdif: Add power-domains property

Add power-domains property for asrc and spdif since fsl,imx8qm-asrc/spdif
and fsl,imx8qxp-asrc/spdif require 'power-domains'.

Set 'power-domains' as required property for compatible string
fsl,imx8qm-asrc/spdif and fsl,imx8qxp-asrc/spdif.

Acked-by: Conor Dooley <[email protected]>
Signed-off-by: Frank Li <[email protected]>
---
Documentation/devicetree/bindings/sound/fsl,imx-asrc.yaml | 14 ++++++++++++++
Documentation/devicetree/bindings/sound/fsl,spdif.yaml | 15 +++++++++++++++
2 files changed, 29 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/fsl,imx-asrc.yaml b/Documentation/devicetree/bindings/sound/fsl,imx-asrc.yaml
index bfef2fcb75b14..76aa1f2484883 100644
--- a/Documentation/devicetree/bindings/sound/fsl,imx-asrc.yaml
+++ b/Documentation/devicetree/bindings/sound/fsl,imx-asrc.yaml
@@ -74,6 +74,9 @@ properties:
- const: asrck_f
- const: spba

+ power-domains:
+ maxItems: 1
+
fsl,asrc-rate:
$ref: /schemas/types.yaml#/definitions/uint32
description: The mutual sample rate used by DPCM Back Ends
@@ -131,6 +134,17 @@ allOf:
properties:
fsl,asrc-clk-map: false

+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - fsl,imx8qm-asrc
+ - fsl,imx8qxp-asrc
+ then:
+ required:
+ - power-domains
+
additionalProperties: false

examples:
diff --git a/Documentation/devicetree/bindings/sound/fsl,spdif.yaml b/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
index 1d64e8337aa4b..56f8c0c8afdea 100644
--- a/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
+++ b/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
@@ -86,6 +86,9 @@ properties:
registers. Set this flag for HCDs with big endian descriptors and big
endian registers.

+ power-domains:
+ maxItems: 1
+
required:
- compatible
- reg
@@ -97,6 +100,18 @@ required:

additionalProperties: false

+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - fsl,imx8qm-spdif
+ - fsl,imx8qxp-spdif
+ then:
+ required:
+ - power-domains
+
examples:
- |
spdif@2004000 {

--
2.34.1


2024-03-08 15:31:58

by Frank Li

[permalink] [raw]
Subject: [PATCH v6 2/4] ASoC: dt-bindings: fsl,imx-asrc: update max interrupt numbers

fsl,imx8qxp-spdif and fsl,imx8qm-spdif have 2 interrupts. Other platforms
have 1 interrupt.

Increase max interrupt number to 2 and add restriction for platforms except
i.MX8QXP and i.MX8QM.

Acked-by: Conor Dooley <[email protected]>
Signed-off-by: Frank Li <[email protected]>
---
.../devicetree/bindings/sound/fsl,spdif.yaml | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/sound/fsl,spdif.yaml b/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
index 56f8c0c8afdea..a242f68f99f18 100644
--- a/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
+++ b/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
@@ -31,7 +31,11 @@ properties:
maxItems: 1

interrupts:
- maxItems: 1
+ minItems: 1
+ maxItems: 2
+ items:
+ - description: Combined or receive interrupt
+ - description: Transmit interrupt

dmas:
items:
@@ -101,6 +105,21 @@ required:
additionalProperties: false

allOf:
+ - if:
+ properties:
+ compatible:
+ enum:
+ - fsl,imx8qm-spdif
+ - fsl,imx8qxp-spdif
+ then:
+ properties:
+ interrupts:
+ minItems: 2
+ else:
+ properties:
+ interrupts:
+ maxItems: 1
+
- if:
properties:
compatible:

--
2.34.1


2024-03-08 15:32:08

by Frank Li

[permalink] [raw]
Subject: [PATCH v6 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names

Some sai only connect one direction dma (rx/tx) in SOC. For example:
imx8qxp sai5 only connect tx dma channel. So allow only one "rx" or "tx"
for dma-names.

Remove description under dmas because no user use index to get dma channel.
All user use 'dma-names' to get correct dma channel. dma-names already in
'required' list.

Acked-by: Conor Dooley <[email protected]>
Signed-off-by: Frank Li <[email protected]>
---
Documentation/devicetree/bindings/sound/fsl,sai.yaml | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/fsl,sai.yaml b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
index 2456d958adeef..93e7737a49a7b 100644
--- a/Documentation/devicetree/bindings/sound/fsl,sai.yaml
+++ b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
@@ -81,15 +81,13 @@ properties:

dmas:
minItems: 1
- items:
- - description: DMA controller phandle and request line for RX
- - description: DMA controller phandle and request line for TX
+ maxItems: 2

dma-names:
minItems: 1
+ maxItems: 2
items:
- - const: rx
- - const: tx
+ enum: [ rx, tx ]

interrupts:
items:

--
2.34.1


2024-03-08 15:32:25

by Frank Li

[permalink] [raw]
Subject: [PATCH v6 4/4] arm64: dts: imx8qxp: add asrc[0,1], esai0, spdif0 and sai[4,5]

Add asrc[0,1], esai0, spdif0, sai[4,5] and related lpcg node for
imx8 audio subsystem.

Signed-off-by: Frank Li <[email protected]>
---
arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi | 267 +++++++++++++++++++++++
1 file changed, 267 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi b/arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi
index 07afeb78ed564..3c57593687f81 100644
--- a/arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi
@@ -6,6 +6,7 @@

#include <dt-bindings/clock/imx8-clock.h>
#include <dt-bindings/clock/imx8-lpcg.h>
+#include <dt-bindings/dma/fsl-edma.h>
#include <dt-bindings/firmware/imx/rsrc.h>

audio_ipg_clk: clock-audio-ipg {
@@ -119,6 +120,86 @@ audio_subsys: bus@59000000 {
#size-cells = <1>;
ranges = <0x59000000 0x0 0x59000000 0x1000000>;

+ asrc0: asrc@59000000 {
+ compatible = "fsl,imx8qm-asrc";
+ reg = <0x59000000 0x10000>;
+ interrupts = <GIC_SPI 372 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&asrc0_lpcg 0>,
+ <&asrc0_lpcg 0>,
+ <&aud_pll_div0_lpcg 0>,
+ <&aud_pll_div1_lpcg 0>,
+ <&acm IMX_ADMA_ACM_AUD_CLK0_SEL>,
+ <&acm IMX_ADMA_ACM_AUD_CLK1_SEL>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>;
+ clock-names = "mem", "ipg",
+ "asrck_0", "asrck_1", "asrck_2", "asrck_3",
+ "asrck_4", "asrck_5", "asrck_6", "asrck_7",
+ "asrck_8", "asrck_9", "asrck_a", "asrck_b",
+ "asrck_c", "asrck_d", "asrck_e", "asrck_f",
+ "spba";
+ dmas = <&edma0 0 0 0>,
+ <&edma0 1 0 0>,
+ <&edma0 2 0 0>,
+ <&edma0 3 0 FSL_EDMA_RX>,
+ <&edma0 4 0 FSL_EDMA_RX>,
+ <&edma0 5 0 FSL_EDMA_RX>;
+ /* tx* is output channel of asrc, it is rx channel for eDMA */
+ dma-names = "rxa", "rxb", "rxc", "txa", "txb", "txc";
+ fsl,asrc-rate = <8000>;
+ fsl,asrc-width = <16>;
+ fsl,asrc-clk-map = <0>;
+ power-domains = <&pd IMX_SC_R_ASRC_0>;
+ status = "disabled";
+ };
+
+ esai0: esai@59010000 {
+ compatible = "fsl,imx8qm-esai", "fsl,imx6ull-esai";
+ reg = <0x59010000 0x10000>;
+ interrupts = <GIC_SPI 409 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&esai0_lpcg 1>, <&esai0_lpcg 0>, <&esai0_lpcg 1>, <&clk_dummy>;
+ clock-names = "core", "extal", "fsys", "spba";
+ dmas = <&edma0 6 0 FSL_EDMA_RX>, <&edma0 7 0 0>;
+ dma-names = "rx", "tx";
+ power-domains = <&pd IMX_SC_R_ESAI_0>;
+ status = "disabled";
+ };
+
+ spdif0: spdif@59020000 {
+ compatible = "fsl,imx8qm-spdif";
+ reg = <0x59020000 0x10000>;
+ interrupts = <GIC_SPI 456 IRQ_TYPE_LEVEL_HIGH>, /* rx */
+ <GIC_SPI 458 IRQ_TYPE_LEVEL_HIGH>; /* tx */
+ clocks = <&spdif0_lpcg 1>, /* core */
+ <&clk_dummy>, /* rxtx0 */
+ <&spdif0_lpcg 0>, /* rxtx1 */
+ <&clk_dummy>, /* rxtx2 */
+ <&clk_dummy>, /* rxtx3 */
+ <&clk_dummy>, /* rxtx4 */
+ <&audio_ipg_clk>, /* rxtx5 */
+ <&clk_dummy>, /* rxtx6 */
+ <&clk_dummy>, /* rxtx7 */
+ <&clk_dummy>; /* spba */
+ clock-names = "core", "rxtx0", "rxtx1", "rxtx2", "rxtx3", "rxtx4",
+ "rxtx5", "rxtx6", "rxtx7", "spba";
+ dmas = <&edma0 8 0 (FSL_EDMA_MULTI_FIFO | FSL_EDMA_RX)>,
+ <&edma0 9 0 FSL_EDMA_MULTI_FIFO>;
+ dma-names = "rx", "tx";
+ power-domains = <&pd IMX_SC_R_SPDIF_0>;
+ status = "disabled";
+ };
+
sai0: sai@59040000 {
compatible = "fsl,imx8qm-sai";
reg = <0x59040000 0x10000>;
@@ -239,6 +320,40 @@ edma0: dma-controller@591f0000 {
<&pd IMX_SC_R_DMA_0_CH23>;
};

+ asrc0_lpcg: clock-controller@59400000 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x59400000 0x10000>;
+ #clock-cells = <1>;
+ clocks = <&audio_ipg_clk>;
+ clock-indices = <IMX_LPCG_CLK_4>;
+ clock-output-names = "asrc0_lpcg_ipg_clk";
+ power-domains = <&pd IMX_SC_R_ASRC_0>;
+ };
+
+ esai0_lpcg: clock-controller@59410000 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x59410000 0x10000>;
+ #clock-cells = <1>;
+ clocks = <&acm IMX_ADMA_ACM_ESAI0_MCLK_SEL>,
+ <&audio_ipg_clk>;
+ clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
+ clock-output-names = "esai0_lpcg_extal_clk",
+ "esai0_lpcg_ipg_clk";
+ power-domains = <&pd IMX_SC_R_ESAI_0>;
+ };
+
+ spdif0_lpcg: clock-controller@59420000 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x59420000 0x10000>;
+ #clock-cells = <1>;
+ clocks = <&acm IMX_ADMA_ACM_SPDIF0_TX_CLK_SEL>,
+ <&audio_ipg_clk>;
+ clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
+ clock-output-names = "spdif0_lpcg_tx_clk",
+ "spdif0_lpcg_gclkw";
+ power-domains = <&pd IMX_SC_R_SPDIF_0>;
+ };
+
sai0_lpcg: clock-controller@59440000 {
compatible = "fsl,imx8qxp-lpcg";
reg = <0x59440000 0x10000>;
@@ -333,6 +448,102 @@ dsp: dsp@596e8000 {
status = "disabled";
};

+ asrc1: asrc@59800000 {
+ compatible = "fsl,imx8qm-asrc";
+ reg = <0x59800000 0x10000>;
+ interrupts = <GIC_SPI 380 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&asrc1_lpcg 0>,
+ <&asrc1_lpcg 0>,
+ <&aud_pll_div0_lpcg 0>,
+ <&aud_pll_div1_lpcg 0>,
+ <&acm IMX_ADMA_ACM_AUD_CLK0_SEL>,
+ <&acm IMX_ADMA_ACM_AUD_CLK1_SEL>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>,
+ <&clk_dummy>;
+ clock-names = "mem", "ipg",
+ "asrck_0", "asrck_1", "asrck_2", "asrck_3",
+ "asrck_4", "asrck_5", "asrck_6", "asrck_7",
+ "asrck_8", "asrck_9", "asrck_a", "asrck_b",
+ "asrck_c", "asrck_d", "asrck_e", "asrck_f",
+ "spba";
+ dmas = <&edma1 0 0 0>,
+ <&edma1 1 0 0>,
+ <&edma1 2 0 0>,
+ <&edma1 3 0 FSL_EDMA_RX>,
+ <&edma1 4 0 FSL_EDMA_RX>,
+ <&edma1 5 0 FSL_EDMA_RX>;
+ /* tx* is output channel of asrc, it is rx channel for eDMA */
+ dma-names = "rxa", "rxb", "rxc", "txa", "txb", "txc";
+ fsl,asrc-rate = <8000>;
+ fsl,asrc-width = <16>;
+ fsl,asrc-clk-map = <1>;
+ power-domains = <&pd IMX_SC_R_ASRC_1>;
+ status = "disabled";
+ };
+
+ sai4: sai@59820000 {
+ compatible = "fsl,imx8qm-sai";
+ reg = <0x59820000 0x10000>;
+ interrupts = <GIC_SPI 329 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&sai4_lpcg 1>,
+ <&clk_dummy>,
+ <&sai4_lpcg 0>,
+ <&clk_dummy>,
+ <&clk_dummy>;
+ clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3";
+ dmas = <&edma1 8 0 FSL_EDMA_RX>, <&edma1 9 0 0>;
+ dma-names = "rx", "tx";
+ power-domains = <&pd IMX_SC_R_SAI_4>;
+ status = "disabled";
+ };
+
+ sai5: sai@59830000 {
+ compatible = "fsl,imx8qm-sai";
+ reg = <0x59830000 0x10000>;
+ interrupts = <GIC_SPI 331 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&sai5_lpcg 1>,
+ <&clk_dummy>,
+ <&sai5_lpcg 0>,
+ <&clk_dummy>,
+ <&clk_dummy>;
+ clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3";
+ dmas = <&edma1 10 0 0>;
+ dma-names = "tx";
+ power-domains = <&pd IMX_SC_R_SAI_5>;
+ status = "disabled";
+ };
+
+ amix: amix@59840000 {
+ compatible = "fsl,imx8qm-audmix";
+ reg = <0x59840000 0x10000>;
+ clocks = <&amix_lpcg 0>;
+ clock-names = "ipg";
+ power-domains = <&pd IMX_SC_R_AMIX>;
+ dais = <&sai4>, <&sai5>;
+ status = "disabled";
+ };
+
+ mqs: mqs@59850000 {
+ compatible = "fsl,imx8qm-mqs";
+ reg = <0x59850000 0x10000>;
+ clocks = <&mqs0_lpcg 0>,
+ <&mqs0_lpcg 1>;
+ clock-names = "mclk", "core";
+ power-domains = <&pd IMX_SC_R_MQS_0>;
+ status = "disabled";
+ };
+
edma1: dma-controller@599f0000 {
compatible = "fsl,imx8qm-edma";
reg = <0x599f0000 0xc0000>;
@@ -481,4 +692,60 @@ acm: acm@59e00000 {
"sai3_rx_bclk",
"sai4_rx_bclk";
};
+
+ asrc1_lpcg: clock-controller@59c00000 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x59c00000 0x10000>;
+ #clock-cells = <1>;
+ clocks = <&audio_ipg_clk>;
+ clock-indices = <IMX_LPCG_CLK_4>;
+ clock-output-names = "asrc1_lpcg_ipg_clk";
+ power-domains = <&pd IMX_SC_R_ASRC_1>;
+ };
+
+ sai4_lpcg: clock-controller@59c20000 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x59c20000 0x10000>;
+ #clock-cells = <1>;
+ clocks = <&acm IMX_ADMA_ACM_SAI4_MCLK_SEL>,
+ <&audio_ipg_clk>;
+ clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
+ clock-output-names = "sai4_lpcg_mclk",
+ "sai4_lpcg_ipg_clk";
+ power-domains = <&pd IMX_SC_R_SAI_4>;
+ };
+
+ sai5_lpcg: clock-controller@59c30000 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x59c30000 0x10000>;
+ #clock-cells = <1>;
+ clocks = <&acm IMX_ADMA_ACM_SAI5_MCLK_SEL>,
+ <&audio_ipg_clk>;
+ clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
+ clock-output-names = "sai5_lpcg_mclk",
+ "sai5_lpcg_ipg_clk";
+ power-domains = <&pd IMX_SC_R_SAI_5>;
+ };
+
+ amix_lpcg: clock-controller@59c40000 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x59c40000 0x10000>;
+ #clock-cells = <1>;
+ clocks = <&audio_ipg_clk>;
+ clock-indices = <IMX_LPCG_CLK_0>;
+ clock-output-names = "amix_lpcg_ipg_clk";
+ power-domains = <&pd IMX_SC_R_AMIX>;
+ };
+
+ mqs0_lpcg: clock-controller@59c50000 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x59c50000 0x10000>;
+ #clock-cells = <1>;
+ clocks = <&acm IMX_ADMA_ACM_MQS_TX_CLK_SEL>,
+ <&audio_ipg_clk>;
+ clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_4>;
+ clock-output-names = "mqs0_lpcg_mclk",
+ "mqs0_lpcg_ipg_clk";
+ power-domains = <&pd IMX_SC_R_MQS_0>;
+ };
};

--
2.34.1


2024-03-08 16:58:28

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v6 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names

On Fri, Mar 08, 2024 at 10:30:52AM -0500, Frank Li wrote:
> Some sai only connect one direction dma (rx/tx) in SOC. For example:
> imx8qxp sai5 only connect tx dma channel. So allow only one "rx" or "tx"
> for dma-names.
>
> Remove description under dmas because no user use index to get dma channel.
> All user use 'dma-names' to get correct dma channel. dma-names already in
> 'required' list.
>
> Acked-by: Conor Dooley <[email protected]>
> Signed-off-by: Frank Li <[email protected]>

Please drop my ack from this, this isn't the patch I acked originally
and we were having a conversation as recently as yesterday on v4 about
this patch because Rob didn't like this approach. His suggestion is
better than the one I gave on v4 that you have used here.

Please give it a few days between resubmissions, and do not send new
versions of a series every day. It is very confusing to have different
conversions on three versions of this patch in my inbox all at the same
time.

Thanks,
Conor.

> ---
> Documentation/devicetree/bindings/sound/fsl,sai.yaml | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/sound/fsl,sai.yaml b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> index 2456d958adeef..93e7737a49a7b 100644
> --- a/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> +++ b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> @@ -81,15 +81,13 @@ properties:
>
> dmas:
> minItems: 1
> - items:
> - - description: DMA controller phandle and request line for RX
> - - description: DMA controller phandle and request line for TX
> + maxItems: 2
>
> dma-names:
> minItems: 1
> + maxItems: 2
> items:
> - - const: rx
> - - const: tx
> + enum: [ rx, tx ]
>
> interrupts:
> items:
>
> --
> 2.34.1
>


Attachments:
(No filename) (1.87 kB)
signature.asc (235.00 B)
Download all attachments

2024-03-08 18:11:17

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v6 2/4] ASoC: dt-bindings: fsl,imx-asrc: update max interrupt numbers

On Fri, Mar 08, 2024 at 10:30:51AM -0500, Frank Li wrote:
> fsl,imx8qxp-spdif and fsl,imx8qm-spdif have 2 interrupts. Other platforms
> have 1 interrupt.
>
> Increase max interrupt number to 2 and add restriction for platforms except
> i.MX8QXP and i.MX8QM.
>
> Acked-by: Conor Dooley <[email protected]>
> Signed-off-by: Frank Li <[email protected]>
> ---
> .../devicetree/bindings/sound/fsl,spdif.yaml | 21 ++++++++++++++++++++-
> 1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/sound/fsl,spdif.yaml b/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
> index 56f8c0c8afdea..a242f68f99f18 100644
> --- a/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
> +++ b/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
> @@ -31,7 +31,11 @@ properties:
> maxItems: 1
>
> interrupts:
> - maxItems: 1
> + minItems: 1
> + maxItems: 2
> + items:
> + - description: Combined or receive interrupt
> + - description: Transmit interrupt

Test your patches please because this will have warnings. Or, you can
put in *exactly* what I provided because this is not it.

If you continue to just toss crap at us at the rate you are, the DT
maintainers will either just start ignoring your patches or require some
trusted review by another NXP colleague first (offhand, not sure who
that would be which is part of the problem).

Rob

2024-03-08 19:01:58

by Frank Li

[permalink] [raw]
Subject: Re: [PATCH v6 2/4] ASoC: dt-bindings: fsl,imx-asrc: update max interrupt numbers

On Fri, Mar 08, 2024 at 12:11:03PM -0600, Rob Herring wrote:
> On Fri, Mar 08, 2024 at 10:30:51AM -0500, Frank Li wrote:
> > fsl,imx8qxp-spdif and fsl,imx8qm-spdif have 2 interrupts. Other platforms
> > have 1 interrupt.
> >
> > Increase max interrupt number to 2 and add restriction for platforms except
> > i.MX8QXP and i.MX8QM.
> >
> > Acked-by: Conor Dooley <[email protected]>
> > Signed-off-by: Frank Li <[email protected]>
> > ---
> > .../devicetree/bindings/sound/fsl,spdif.yaml | 21 ++++++++++++++++++++-
> > 1 file changed, 20 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/sound/fsl,spdif.yaml b/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
> > index 56f8c0c8afdea..a242f68f99f18 100644
> > --- a/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
> > +++ b/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
> > @@ -31,7 +31,11 @@ properties:
> > maxItems: 1
> >
> > interrupts:
> > - maxItems: 1
> > + minItems: 1
> > + maxItems: 2
> > + items:
> > + - description: Combined or receive interrupt
> > + - description: Transmit interrupt
>
> Test your patches please because this will have warnings. Or, you can
> put in *exactly* what I provided because this is not it.
>
> If you continue to just toss crap at us at the rate you are, the DT
> maintainers will either just start ignoring your patches or require some
> trusted review by another NXP colleague first (offhand, not sure who
> that would be which is part of the problem).

Sorry, I run wrong command to check another dtb file. So have not catch
this problem.

Frank

>
> Rob

2024-03-08 19:06:31

by Frank Li

[permalink] [raw]
Subject: Re: [PATCH v6 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names

On Fri, Mar 08, 2024 at 04:58:16PM +0000, Conor Dooley wrote:
> On Fri, Mar 08, 2024 at 10:30:52AM -0500, Frank Li wrote:
> > Some sai only connect one direction dma (rx/tx) in SOC. For example:
> > imx8qxp sai5 only connect tx dma channel. So allow only one "rx" or "tx"
> > for dma-names.
> >
> > Remove description under dmas because no user use index to get dma channel.
> > All user use 'dma-names' to get correct dma channel. dma-names already in
> > 'required' list.
> >
> > Acked-by: Conor Dooley <[email protected]>
> > Signed-off-by: Frank Li <[email protected]>
>
> Please drop my ack from this, this isn't the patch I acked originally
> and we were having a conversation as recently as yesterday on v4 about
> this patch because Rob didn't like this approach. His suggestion is
> better than the one I gave on v4 that you have used here.

Why do you think Rob don't like this approach? He just said this is 3rd
method. And it is simple enough and match all restriction.

Frank Li

>
> Please give it a few days between resubmissions, and do not send new
> versions of a series every day. It is very confusing to have different
> conversions on three versions of this patch in my inbox all at the same
> time.

Okay, I think it is just simple fix and almost everything already close to
settle down.

Frank

>
> Thanks,
> Conor.
>
> > ---
> > Documentation/devicetree/bindings/sound/fsl,sai.yaml | 8 +++-----
> > 1 file changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/sound/fsl,sai.yaml b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > index 2456d958adeef..93e7737a49a7b 100644
> > --- a/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > +++ b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > @@ -81,15 +81,13 @@ properties:
> >
> > dmas:
> > minItems: 1
> > - items:
> > - - description: DMA controller phandle and request line for RX
> > - - description: DMA controller phandle and request line for TX
> > + maxItems: 2
> >
> > dma-names:
> > minItems: 1
> > + maxItems: 2
> > items:
> > - - const: rx
> > - - const: tx
> > + enum: [ rx, tx ]
> >
> > interrupts:
> > items:
> >
> > --
> > 2.34.1
> >



2024-03-08 19:11:56

by Frank Li

[permalink] [raw]
Subject: Re: [PATCH v6 2/4] ASoC: dt-bindings: fsl,imx-asrc: update max interrupt numbers

On Fri, Mar 08, 2024 at 02:01:29PM -0500, Frank Li wrote:
> On Fri, Mar 08, 2024 at 12:11:03PM -0600, Rob Herring wrote:
> > On Fri, Mar 08, 2024 at 10:30:51AM -0500, Frank Li wrote:
> > > fsl,imx8qxp-spdif and fsl,imx8qm-spdif have 2 interrupts. Other platforms
> > > have 1 interrupt.
> > >
> > > Increase max interrupt number to 2 and add restriction for platforms except
> > > i.MX8QXP and i.MX8QM.
> > >
> > > Acked-by: Conor Dooley <[email protected]>
> > > Signed-off-by: Frank Li <[email protected]>
> > > ---
> > > .../devicetree/bindings/sound/fsl,spdif.yaml | 21 ++++++++++++++++++++-
> > > 1 file changed, 20 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/sound/fsl,spdif.yaml b/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
> > > index 56f8c0c8afdea..a242f68f99f18 100644
> > > --- a/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
> > > +++ b/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
> > > @@ -31,7 +31,11 @@ properties:
> > > maxItems: 1
> > >
> > > interrupts:
> > > - maxItems: 1
> > > + minItems: 1
> > > + maxItems: 2
> > > + items:
> > > + - description: Combined or receive interrupt
> > > + - description: Transmit interrupt
> >
> > Test your patches please because this will have warnings. Or, you can
> > put in *exactly* what I provided because this is not it.
> >
> > If you continue to just toss crap at us at the rate you are, the DT
> > maintainers will either just start ignoring your patches or require some
> > trusted review by another NXP colleague first (offhand, not sure who
> > that would be which is part of the problem).
>
> Sorry, I run wrong command to check another dtb file. So have not catch
> this problem.

Strange when I run second time, spdif warning disappeared.

make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- -j8 CHECK_DTBS=y freescale/imx8dxl-evk.dtb
DTC_CHK arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb
/home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: /: fixedregulator@101: 'anyOf' conditional failed, one must be fixed:
'reg' is a required property
'ranges' is a required property
from schema $id: http://devicetree.org/schemas/root-node.yaml#
/home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: bus@34000000: clock-cm40-ipg: {'compatible': ['fixed-clock'], '#clock-cells': [[0]], 'clock-frequency': [[132000000]], 'clock-output-names': ['cm40_ipg_clk'], 'phandle': [[15]]} should not be valid under {'type': 'object'}
from schema $id: http://devicetree.org/schemas/simple-bus.yaml#
/home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: intmux@37400000: 'power-domains' does not match any of the regexes: 'pinctrl-[0-9]+'
from schema $id: http://devicetree.org/schemas/interrupt-controller/fsl,intmux.yaml#
/home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: dma-controller@591f0000: 'clocks' is a required property
from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
/home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: dma-controller@591f0000: Unevaluated properties are not allowed ('power-domains' was unexpected)
from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
/home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: dma-controller@599f0000: 'clocks' is a required property
from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
/home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: dma-controller@599f0000: Unevaluated properties are not allowed ('power-domains' was unexpected)
from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
/home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: serial@5a070000: Unevaluated properties are not allowed ('resets' was unexpected)
from schema $id: http://devicetree.org/schemas/serial/fsl-lpuart.yaml#
/home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: dma-controller@5a1f0000: 'clocks' is a required property
from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
/home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: dma-controller@5a1f0000: Unevaluated properties are not allowed ('power-domains' was unexpected)
from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
/home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: dma-controller@5a9f0000: 'clocks' is a required property
from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
/home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: dma-controller@5a9f0000: Unevaluated properties are not allowed ('power-domains' was unexpected)
from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
/home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: bus@5b000000: clock-conn-bch: {'compatible': ['fixed-clock'], '#clock-cells': [[0]], 'clock-frequency': [[400000000]], 'clock-output-names': ['conn_bch_clk']} should not be valid under {'type': 'object'}
from schema $id: http://devicetree.org/schemas/simple-bus.yaml#
/home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: dma-apbh@5b810000: $nodename:0: 'dma-apbh@5b810000' does not match '^dma-controller(@.*)?$'
from schema $id: http://devicetree.org/schemas/dma/fsl,mxs-dma.yaml#
/home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: dma-apbh@5b810000: 'clock-names', 'interrupt-names', 'power-domains' do not match any of the regexes: 'pinctrl-[0-9]+'
from schema $id: http://devicetree.org/schemas/dma/fsl,mxs-dma.yaml#



>
> Frank
>
> >
> > Rob

2024-03-08 19:26:41

by Frank Li

[permalink] [raw]
Subject: Re: [PATCH v6 2/4] ASoC: dt-bindings: fsl,imx-asrc: update max interrupt numbers

On Fri, Mar 08, 2024 at 02:11:34PM -0500, Frank Li wrote:
> On Fri, Mar 08, 2024 at 02:01:29PM -0500, Frank Li wrote:
> > On Fri, Mar 08, 2024 at 12:11:03PM -0600, Rob Herring wrote:
> > > On Fri, Mar 08, 2024 at 10:30:51AM -0500, Frank Li wrote:
> > > > fsl,imx8qxp-spdif and fsl,imx8qm-spdif have 2 interrupts. Other platforms
> > > > have 1 interrupt.
> > > >
> > > > Increase max interrupt number to 2 and add restriction for platforms except
> > > > i.MX8QXP and i.MX8QM.
> > > >
> > > > Acked-by: Conor Dooley <[email protected]>
> > > > Signed-off-by: Frank Li <[email protected]>
> > > > ---
> > > > .../devicetree/bindings/sound/fsl,spdif.yaml | 21 ++++++++++++++++++++-
> > > > 1 file changed, 20 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/sound/fsl,spdif.yaml b/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
> > > > index 56f8c0c8afdea..a242f68f99f18 100644
> > > > --- a/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
> > > > +++ b/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
> > > > @@ -31,7 +31,11 @@ properties:
> > > > maxItems: 1
> > > >
> > > > interrupts:
> > > > - maxItems: 1
> > > > + minItems: 1
> > > > + maxItems: 2
> > > > + items:
> > > > + - description: Combined or receive interrupt
> > > > + - description: Transmit interrupt
> > >
> > > Test your patches please because this will have warnings. Or, you can
> > > put in *exactly* what I provided because this is not it.
> > >
> > > If you continue to just toss crap at us at the rate you are, the DT
> > > maintainers will either just start ignoring your patches or require some
> > > trusted review by another NXP colleague first (offhand, not sure who
> > > that would be which is part of the problem).
> >
> > Sorry, I run wrong command to check another dtb file. So have not catch
> > this problem.
>
> Strange when I run second time, spdif warning disappeared.
>
> make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- -j8 CHECK_DTBS=y freescale/imx8dxl-evk.dtb
> DTC_CHK arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb
> /home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: /: fixedregulator@101: 'anyOf' conditional failed, one must be fixed:
> 'reg' is a required property
> 'ranges' is a required property
> from schema $id: http://devicetree.org/schemas/root-node.yaml#
> /home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: bus@34000000: clock-cm40-ipg: {'compatible': ['fixed-clock'], '#clock-cells': [[0]], 'clock-frequency': [[132000000]], 'clock-output-names': ['cm40_ipg_clk'], 'phandle': [[15]]} should not be valid under {'type': 'object'}
> from schema $id: http://devicetree.org/schemas/simple-bus.yaml#
> /home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: intmux@37400000: 'power-domains' does not match any of the regexes: 'pinctrl-[0-9]+'
> from schema $id: http://devicetree.org/schemas/interrupt-controller/fsl,intmux.yaml#
> /home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: dma-controller@591f0000: 'clocks' is a required property
> from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
> /home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: dma-controller@591f0000: Unevaluated properties are not allowed ('power-domains' was unexpected)
> from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
> /home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: dma-controller@599f0000: 'clocks' is a required property
> from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
> /home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: dma-controller@599f0000: Unevaluated properties are not allowed ('power-domains' was unexpected)
> from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
> /home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: serial@5a070000: Unevaluated properties are not allowed ('resets' was unexpected)
> from schema $id: http://devicetree.org/schemas/serial/fsl-lpuart.yaml#
> /home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: dma-controller@5a1f0000: 'clocks' is a required property
> from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
> /home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: dma-controller@5a1f0000: Unevaluated properties are not allowed ('power-domains' was unexpected)
> from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
> /home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: dma-controller@5a9f0000: 'clocks' is a required property
> from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
> /home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: dma-controller@5a9f0000: Unevaluated properties are not allowed ('power-domains' was unexpected)
> from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
> /home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: bus@5b000000: clock-conn-bch: {'compatible': ['fixed-clock'], '#clock-cells': [[0]], 'clock-frequency': [[400000000]], 'clock-output-names': ['conn_bch_clk']} should not be valid under {'type': 'object'}
> from schema $id: http://devicetree.org/schemas/simple-bus.yaml#
> /home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: dma-apbh@5b810000: $nodename:0: 'dma-apbh@5b810000' does not match '^dma-controller(@.*)?$'
> from schema $id: http://devicetree.org/schemas/dma/fsl,mxs-dma.yaml#
> /home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: dma-apbh@5b810000: 'clock-names', 'interrupt-names', 'power-domains' do not match any of the regexes: 'pinctrl-[0-9]+'
> from schema $id: http://devicetree.org/schemas/dma/fsl,mxs-dma.yaml#
>

I figured out what's happen. CHECK_DTB will check binding doc except
example part. So I skip run dt_binding_check when I have not touch example
part in dt-binding doc.

The problem is
step 1. run make CHECK_DTB=y freescale/imx8ulp-evk.dtb
Then I get some sai error, I found use wrong 8ulp-evk.dtb

step 2. run make CHECK_DTBS=y freescale/imx8dxl-evk.dtb
Then tools not check dt_binding file again even there are warning.

Do you have option let CHECK_DTBS to force check binding doc every time.

Frank

>
>
> >
> > Frank
> >
> > >
> > > Rob

2024-03-08 20:21:37

by Frank Li

[permalink] [raw]
Subject: Re: [PATCH v6 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names

On Fri, Mar 08, 2024 at 02:06:08PM -0500, Frank Li wrote:
> On Fri, Mar 08, 2024 at 04:58:16PM +0000, Conor Dooley wrote:
> > On Fri, Mar 08, 2024 at 10:30:52AM -0500, Frank Li wrote:
> > > Some sai only connect one direction dma (rx/tx) in SOC. For example:
> > > imx8qxp sai5 only connect tx dma channel. So allow only one "rx" or "tx"
> > > for dma-names.
> > >
> > > Remove description under dmas because no user use index to get dma channel.
> > > All user use 'dma-names' to get correct dma channel. dma-names already in
> > > 'required' list.
> > >
> > > Acked-by: Conor Dooley <[email protected]>
> > > Signed-off-by: Frank Li <[email protected]>
> >
> > Please drop my ack from this, this isn't the patch I acked originally
> > and we were having a conversation as recently as yesterday on v4 about
> > this patch because Rob didn't like this approach. His suggestion is
> > better than the one I gave on v4 that you have used here.

I paste your comments here

"What I suggested is different, it is more permissive than what you have
or what Rob suggested. Your original one allows
"rx", "tx" OR "rx" OR "tx"
Rob's allows the same but with a nicer syntax. What that stm binding I
mentioned allows is
"rx", "tx" OR "tx", "rx" OR "rx" OR "tx"
"

Actually:

"rx", "tx" OR "tx", "rx" OR "rx" OR "tx" is exactly what we want.

"rx", "tx" OR "rx" OR "tx" is only feasible, but not perfect. Why need
limited "rx" and "tx" order? It just bring us some noise and no actual
value to do that.

Frank


>
> Why do you think Rob don't like this approach? He just said this is 3rd
> method. And it is simple enough and match all restriction.
>
> Frank Li
>
> >
> > Please give it a few days between resubmissions, and do not send new
> > versions of a series every day. It is very confusing to have different
> > conversions on three versions of this patch in my inbox all at the same
> > time.
>
> Okay, I think it is just simple fix and almost everything already close to
> settle down.
>
> Frank
>
> >
> > Thanks,
> > Conor.
> >
> > > ---
> > > Documentation/devicetree/bindings/sound/fsl,sai.yaml | 8 +++-----
> > > 1 file changed, 3 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/sound/fsl,sai.yaml b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > index 2456d958adeef..93e7737a49a7b 100644
> > > --- a/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > +++ b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > @@ -81,15 +81,13 @@ properties:
> > >
> > > dmas:
> > > minItems: 1
> > > - items:
> > > - - description: DMA controller phandle and request line for RX
> > > - - description: DMA controller phandle and request line for TX
> > > + maxItems: 2
> > >
> > > dma-names:
> > > minItems: 1
> > > + maxItems: 2
> > > items:
> > > - - const: rx
> > > - - const: tx
> > > + enum: [ rx, tx ]
> > >
> > > interrupts:
> > > items:
> > >
> > > --
> > > 2.34.1
> > >
>
>

2024-03-08 21:13:22

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v6 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names

On Fri, Mar 08, 2024 at 03:21:14PM -0500, Frank Li wrote:
> On Fri, Mar 08, 2024 at 02:06:08PM -0500, Frank Li wrote:
> > On Fri, Mar 08, 2024 at 04:58:16PM +0000, Conor Dooley wrote:
> > > On Fri, Mar 08, 2024 at 10:30:52AM -0500, Frank Li wrote:
> > > > Some sai only connect one direction dma (rx/tx) in SOC. For example:
> > > > imx8qxp sai5 only connect tx dma channel. So allow only one "rx" or "tx"
> > > > for dma-names.
> > > >
> > > > Remove description under dmas because no user use index to get dma channel.
> > > > All user use 'dma-names' to get correct dma channel. dma-names already in
> > > > 'required' list.
> > > >
> > > > Acked-by: Conor Dooley <[email protected]>
> > > > Signed-off-by: Frank Li <[email protected]>
> > >
> > > Please drop my ack from this, this isn't the patch I acked originally
> > > and we were having a conversation as recently as yesterday on v4 about
> > > this patch because Rob didn't like this approach. His suggestion is
> > > better than the one I gave on v4 that you have used here.
>
> I paste your comments here
>
> "What I suggested is different, it is more permissive than what you have
> or what Rob suggested. Your original one allows
> "rx", "tx" OR "rx" OR "tx"
> Rob's allows the same but with a nicer syntax. What that stm binding I
> mentioned allows is
> "rx", "tx" OR "tx", "rx" OR "rx" OR "tx"
> "
>
> Actually:
>
> "rx", "tx" OR "tx", "rx" OR "rx" OR "tx" is exactly what we want.

No, it is not.

> "rx", "tx" OR "rx" OR "tx" is only feasible, but not perfect. Why need
> limited "rx" and "tx" order?

First, that's exactly what the binding already had. Why loosen it?
Second, defined order is just the DT way. There is less reason to
support both ways. It is simpler for a client to read properties if it
knows the position of entries.


> It just bring us some noise and no actual
> value to do that.
>
> Frank
>
>
> >
> > Why do you think Rob don't like this approach? He just said this is 3rd
> > method. And it is simple enough and match all restriction.

I don't like the approach. Clear enough?

Rob

2024-03-08 21:14:57

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v6 2/4] ASoC: dt-bindings: fsl,imx-asrc: update max interrupt numbers

On 08/03/2024 19:11, Rob Herring wrote:
>>
>> interrupts:
>> - maxItems: 1
>> + minItems: 1
>> + maxItems: 2
>> + items:
>> + - description: Combined or receive interrupt
>> + - description: Transmit interrupt
>
> Test your patches please because this will have warnings. Or, you can
> put in *exactly* what I provided because this is not it.
>
> If you continue to just toss crap at us at the rate you are, the DT
> maintainers will either just start ignoring your patches or require some

It happened already, at least for me, by default I move them to the end
of the patch/review queue.

> trusted review by another NXP colleague first (offhand, not sure who
> that would be which is part of the problem).



Best regards,
Krzysztof


2024-03-08 21:15:59

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v6 2/4] ASoC: dt-bindings: fsl,imx-asrc: update max interrupt numbers

On 08/03/2024 20:26, Frank Li wrote:
> On Fri, Mar 08, 2024 at 02:11:34PM -0500, Frank Li wrote:
>> On Fri, Mar 08, 2024 at 02:01:29PM -0500, Frank Li wrote:
>>> On Fri, Mar 08, 2024 at 12:11:03PM -0600, Rob Herring wrote:
>>>> On Fri, Mar 08, 2024 at 10:30:51AM -0500, Frank Li wrote:
>>>>> fsl,imx8qxp-spdif and fsl,imx8qm-spdif have 2 interrupts. Other platforms
>>>>> have 1 interrupt.
>>>>>
>>>>> Increase max interrupt number to 2 and add restriction for platforms except
>>>>> i.MX8QXP and i.MX8QM.
>>>>>
>>>>> Acked-by: Conor Dooley <[email protected]>
>>>>> Signed-off-by: Frank Li <[email protected]>
>>>>> ---
>>>>> .../devicetree/bindings/sound/fsl,spdif.yaml | 21 ++++++++++++++++++++-
>>>>> 1 file changed, 20 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/sound/fsl,spdif.yaml b/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
>>>>> index 56f8c0c8afdea..a242f68f99f18 100644
>>>>> --- a/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
>>>>> +++ b/Documentation/devicetree/bindings/sound/fsl,spdif.yaml
>>>>> @@ -31,7 +31,11 @@ properties:
>>>>> maxItems: 1
>>>>>
>>>>> interrupts:
>>>>> - maxItems: 1
>>>>> + minItems: 1
>>>>> + maxItems: 2
>>>>> + items:
>>>>> + - description: Combined or receive interrupt
>>>>> + - description: Transmit interrupt
>>>>
>>>> Test your patches please because this will have warnings. Or, you can
>>>> put in *exactly* what I provided because this is not it.
>>>>
>>>> If you continue to just toss crap at us at the rate you are, the DT
>>>> maintainers will either just start ignoring your patches or require some
>>>> trusted review by another NXP colleague first (offhand, not sure who
>>>> that would be which is part of the problem).
>>>
>>> Sorry, I run wrong command to check another dtb file. So have not catch
>>> this problem.
>>
>> Strange when I run second time, spdif warning disappeared.
>>
>> make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- -j8 CHECK_DTBS=y freescale/imx8dxl-evk.dtb
>> DTC_CHK arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb
>> /home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: /: fixedregulator@101: 'anyOf' conditional failed, one must be fixed:
>> 'reg' is a required property
>> 'ranges' is a required property
>> from schema $id: http://devicetree.org/schemas/root-node.yaml#
>> /home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: bus@34000000: clock-cm40-ipg: {'compatible': ['fixed-clock'], '#clock-cells': [[0]], 'clock-frequency': [[132000000]], 'clock-output-names': ['cm40_ipg_clk'], 'phandle': [[15]]} should not be valid under {'type': 'object'}
>> from schema $id: http://devicetree.org/schemas/simple-bus.yaml#
>> /home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: intmux@37400000: 'power-domains' does not match any of the regexes: 'pinctrl-[0-9]+'
>> from schema $id: http://devicetree.org/schemas/interrupt-controller/fsl,intmux.yaml#
>> /home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: dma-controller@591f0000: 'clocks' is a required property
>> from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
>> /home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: dma-controller@591f0000: Unevaluated properties are not allowed ('power-domains' was unexpected)
>> from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
>> /home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: dma-controller@599f0000: 'clocks' is a required property
>> from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
>> /home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: dma-controller@599f0000: Unevaluated properties are not allowed ('power-domains' was unexpected)
>> from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
>> /home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: serial@5a070000: Unevaluated properties are not allowed ('resets' was unexpected)
>> from schema $id: http://devicetree.org/schemas/serial/fsl-lpuart.yaml#
>> /home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: dma-controller@5a1f0000: 'clocks' is a required property
>> from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
>> /home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: dma-controller@5a1f0000: Unevaluated properties are not allowed ('power-domains' was unexpected)
>> from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
>> /home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: dma-controller@5a9f0000: 'clocks' is a required property
>> from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
>> /home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: dma-controller@5a9f0000: Unevaluated properties are not allowed ('power-domains' was unexpected)
>> from schema $id: http://devicetree.org/schemas/dma/fsl,edma.yaml#
>> /home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: bus@5b000000: clock-conn-bch: {'compatible': ['fixed-clock'], '#clock-cells': [[0]], 'clock-frequency': [[400000000]], 'clock-output-names': ['conn_bch_clk']} should not be valid under {'type': 'object'}
>> from schema $id: http://devicetree.org/schemas/simple-bus.yaml#
>> /home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: dma-apbh@5b810000: $nodename:0: 'dma-apbh@5b810000' does not match '^dma-controller(@.*)?$'
>> from schema $id: http://devicetree.org/schemas/dma/fsl,mxs-dma.yaml#
>> /home/lizhi/source/linux-upstream-dts/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: dma-apbh@5b810000: 'clock-names', 'interrupt-names', 'power-domains' do not match any of the regexes: 'pinctrl-[0-9]+'
>> from schema $id: http://devicetree.org/schemas/dma/fsl,mxs-dma.yaml#
>>
>
> I figured out what's happen. CHECK_DTB will check binding doc except
> example part. So I skip run dt_binding_check when I have not touch example
> part in dt-binding doc.
>
> The problem is
> step 1. run make CHECK_DTB=y freescale/imx8ulp-evk.dtb
> Then I get some sai error, I found use wrong 8ulp-evk.dtb
>
> step 2. run make CHECK_DTBS=y freescale/imx8dxl-evk.dtb
> Then tools not check dt_binding file again even there are warning.
>
> Do you have option let CHECK_DTBS to force check binding doc every time.

CHECK_DTB is for checking dtbs, not binding. You must run
dt_bindings_check, as asked by every damn document and guide.

Best regards,
Krzysztof