* Re: [PATCH] ARM: dts: imx: Add mclk0 clock for SAI
[not found] <VE1PR04MB647951B20E0F334FF1CC3B7AE3220@VE1PR04MB6479.eurprd04.prod.outlook.com>
@ 2019-04-22 4:25 ` Nicolin Chen
0 siblings, 0 replies; 3+ messages in thread
From: Nicolin Chen @ 2019-04-22 4:25 UTC (permalink / raw)
To: S.j. Wang
Cc: mark.rutland@arm.com, Aisheng Dong, Peng Fan, festevam@gmail.com,
Anson Huang, devicetree@vger.kernel.org, Daniel Baluta,
s.hauer@pengutronix.de, linux-kernel@vger.kernel.org,
robh+dt@kernel.org, dl-linux-imx, kernel@pengutronix.de,
shawnguo@kernel.org, linux-arm-kernel@lists.infradead.org
On Mon, Apr 22, 2019 at 03:30:26AM +0000, S.j. Wang wrote:
> > > SAI has 4 clock sources, which can be selected using MSEL bit of SAI
> > > TCR2 register.
> >
> > I have a doubt at this statement. As far as I can understand, this MSEL is
> > probably used by its internal clock MUX, so it's not really proving that SAI
> > has 4 MCLK inputs. What I know is that SAI block itself only has 3 MCLK
> > inputs as we defined in DT. It's just internally connects bus clock or MCLK1
> > to input0 of clock MUX's and connects MCLK[1-3] to input[1-3]. So adding an
> > MCLK0 here doesn't sound a right way to me. Unless someone can justify
> > for it, I think we should just fix it from driver side.
> >
> > Thanks
> > Nicolin
> >
>
> The MSEL bit width is 2 bit, so there is 4 options, the MCLK0 maybe the same input as
> MCLK1 or bus clock as you said, so we think may be better to show this relation in DT,
> And this is DT's capability. Driver don't care about which clock connect to MCLK0,
> it only need to know there is 4 MCLK from DT.
I know what it is. But it feels weird that we add an MCLK0 just
because of what a register filed has, and there's no "MCLK0" be
mentioned in the RM at all. My point is that if SAI doesn't have
a port named "MCLK0", I don't feel it's that convincing to have
it in the DT.
Usually in DT we define the clock sources of an entire IP block
in audio use cases, not for an internal clock MUX. But taking a
step back, it might not be really wrong to do so, since the MUX
is a part of the hardware. If we redefine the MCLK[0-4] as "four
clock sources of SAI's clock MUX selecting a clock for bit clock
and frame clock providing" in the binding doc, I feel it'd make
a lot of sense.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] ARM: dts: imx: Add mclk0 clock for SAI
@ 2019-04-20 9:12 Daniel Baluta
2019-04-21 8:20 ` Nicolin Chen
0 siblings, 1 reply; 3+ messages in thread
From: Daniel Baluta @ 2019-04-20 9:12 UTC (permalink / raw)
To: shawnguo@kernel.org
Cc: mark.rutland@arm.com, Aisheng Dong, Peng Fan, Anson Huang,
devicetree@vger.kernel.org, Daniel Baluta, s.hauer@pengutronix.de,
linux-kernel@vger.kernel.org, robh+dt@kernel.org, dl-linux-imx,
kernel@pengutronix.de, festevam@gmail.com, S.j. Wang,
linux-arm-kernel@lists.infradead.org
From: Shengjiu Wang <shengjiu.wang@nxp.com>
SAI has 4 clock sources, which can be selected using MSEL
bit of SAI TCR2 register.
On imx6/7 mclk0 and mclk1 always point to the same clock
source. Anyhow, this is no longer true for imx8.
For this reason, we need to add mclk0 and handle it
in a generic way in SAI driver.
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
arch/arm/boot/dts/imx6sx.dtsi | 6 ++++--
arch/arm/boot/dts/imx6ul.dtsi | 9 ++++++---
arch/arm/boot/dts/imx7s.dtsi | 9 ++++++---
3 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index b16a123990a2..682207b5d868 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -1071,9 +1071,10 @@
reg = <0x021d4000 0x4000>;
interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clks IMX6SX_CLK_SAI1_IPG>,
+ <&clks IMX6SX_CLK_SAI1>,
<&clks IMX6SX_CLK_SAI1>,
<&clks 0>, <&clks 0>;
- clock-names = "bus", "mclk1", "mclk2", "mclk3";
+ clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3";
dma-names = "rx", "tx";
dmas = <&sdma 31 24 0>, <&sdma 32 24 0>;
status = "disabled";
@@ -1090,9 +1091,10 @@
reg = <0x021dc000 0x4000>;
interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clks IMX6SX_CLK_SAI2_IPG>,
+ <&clks IMX6SX_CLK_SAI2>,
<&clks IMX6SX_CLK_SAI2>,
<&clks 0>, <&clks 0>;
- clock-names = "bus", "mclk1", "mclk2", "mclk3";
+ clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3";
dma-names = "rx", "tx";
dmas = <&sdma 33 24 0>, <&sdma 34 24 0>;
status = "disabled";
diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
index bbf010c73336..e9691306f557 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -304,9 +304,10 @@
reg = <0x02028000 0x4000>;
interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clks IMX6UL_CLK_SAI1_IPG>,
+ <&clks IMX6UL_CLK_SAI1>,
<&clks IMX6UL_CLK_SAI1>,
<&clks IMX6UL_CLK_DUMMY>, <&clks IMX6UL_CLK_DUMMY>;
- clock-names = "bus", "mclk1", "mclk2", "mclk3";
+ clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3";
dmas = <&sdma 35 24 0>,
<&sdma 36 24 0>;
dma-names = "rx", "tx";
@@ -319,9 +320,10 @@
reg = <0x0202c000 0x4000>;
interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clks IMX6UL_CLK_SAI2_IPG>,
+ <&clks IMX6UL_CLK_SAI2>,
<&clks IMX6UL_CLK_SAI2>,
<&clks IMX6UL_CLK_DUMMY>, <&clks IMX6UL_CLK_DUMMY>;
- clock-names = "bus", "mclk1", "mclk2", "mclk3";
+ clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3";
dmas = <&sdma 37 24 0>,
<&sdma 38 24 0>;
dma-names = "rx", "tx";
@@ -334,9 +336,10 @@
reg = <0x02030000 0x4000>;
interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clks IMX6UL_CLK_SAI3_IPG>,
+ <&clks IMX6UL_CLK_SAI3>,
<&clks IMX6UL_CLK_SAI3>,
<&clks IMX6UL_CLK_DUMMY>, <&clks IMX6UL_CLK_DUMMY>;
- clock-names = "bus", "mclk1", "mclk2", "mclk3";
+ clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3";
dmas = <&sdma 39 24 0>,
<&sdma 40 24 0>;
dma-names = "rx", "tx";
diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index 0b01109ac0a9..693b88e37799 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -884,10 +884,11 @@
reg = <0x308a0000 0x10000>;
interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clks IMX7D_SAI1_IPG_CLK>,
+ <&clks IMX7D_SAI1_ROOT_CLK>,
<&clks IMX7D_SAI1_ROOT_CLK>,
<&clks IMX7D_CLK_DUMMY>,
<&clks IMX7D_CLK_DUMMY>;
- clock-names = "bus", "mclk1", "mclk2", "mclk3";
+ clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3";
dma-names = "rx", "tx";
dmas = <&sdma 8 24 0>, <&sdma 9 24 0>;
status = "disabled";
@@ -899,10 +900,11 @@
reg = <0x308b0000 0x10000>;
interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clks IMX7D_SAI2_IPG_CLK>,
+ <&clks IMX7D_SAI2_ROOT_CLK>,
<&clks IMX7D_SAI2_ROOT_CLK>,
<&clks IMX7D_CLK_DUMMY>,
<&clks IMX7D_CLK_DUMMY>;
- clock-names = "bus", "mclk1", "mclk2", "mclk3";
+ clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3";
dma-names = "rx", "tx";
dmas = <&sdma 10 24 0>, <&sdma 11 24 0>;
status = "disabled";
@@ -914,10 +916,11 @@
reg = <0x308c0000 0x10000>;
interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clks IMX7D_SAI3_IPG_CLK>,
+ <&clks IMX7D_SAI3_ROOT_CLK>,
<&clks IMX7D_SAI3_ROOT_CLK>,
<&clks IMX7D_CLK_DUMMY>,
<&clks IMX7D_CLK_DUMMY>;
- clock-names = "bus", "mclk1", "mclk2", "mclk3";
+ clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3";
dma-names = "rx", "tx";
dmas = <&sdma 12 24 0>, <&sdma 13 24 0>;
status = "disabled";
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] ARM: dts: imx: Add mclk0 clock for SAI
2019-04-20 9:12 Daniel Baluta
@ 2019-04-21 8:20 ` Nicolin Chen
0 siblings, 0 replies; 3+ messages in thread
From: Nicolin Chen @ 2019-04-21 8:20 UTC (permalink / raw)
To: Daniel Baluta
Cc: mark.rutland@arm.com, Aisheng Dong, Peng Fan, Anson Huang,
devicetree@vger.kernel.org, shawnguo@kernel.org,
s.hauer@pengutronix.de, linux-kernel@vger.kernel.org,
robh+dt@kernel.org, dl-linux-imx, kernel@pengutronix.de,
festevam@gmail.com, S.j. Wang,
linux-arm-kernel@lists.infradead.org
On Sat, Apr 20, 2019 at 09:12:52AM +0000, Daniel Baluta wrote:
> From: Shengjiu Wang <shengjiu.wang@nxp.com>
>
> SAI has 4 clock sources, which can be selected using MSEL
> bit of SAI TCR2 register.
I have a doubt at this statement. As far as I can understand,
this MSEL is probably used by its internal clock MUX, so it's
not really proving that SAI has 4 MCLK inputs. What I know is
that SAI block itself only has 3 MCLK inputs as we defined in
DT. It's just internally connects bus clock or MCLK1 to input0
of clock MUX's and connects MCLK[1-3] to input[1-3]. So adding
an MCLK0 here doesn't sound a right way to me. Unless someone
can justify for it, I think we should just fix it from driver
side.
Thanks
Nicolin
>
> On imx6/7 mclk0 and mclk1 always point to the same clock
> source. Anyhow, this is no longer true for imx8.
>
> For this reason, we need to add mclk0 and handle it
> in a generic way in SAI driver.
>
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
> ---
> arch/arm/boot/dts/imx6sx.dtsi | 6 ++++--
> arch/arm/boot/dts/imx6ul.dtsi | 9 ++++++---
> arch/arm/boot/dts/imx7s.dtsi | 9 ++++++---
> 3 files changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
> index b16a123990a2..682207b5d868 100644
> --- a/arch/arm/boot/dts/imx6sx.dtsi
> +++ b/arch/arm/boot/dts/imx6sx.dtsi
> @@ -1071,9 +1071,10 @@
> reg = <0x021d4000 0x4000>;
> interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>;
> clocks = <&clks IMX6SX_CLK_SAI1_IPG>,
> + <&clks IMX6SX_CLK_SAI1>,
> <&clks IMX6SX_CLK_SAI1>,
> <&clks 0>, <&clks 0>;
> - clock-names = "bus", "mclk1", "mclk2", "mclk3";
> + clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3";
> dma-names = "rx", "tx";
> dmas = <&sdma 31 24 0>, <&sdma 32 24 0>;
> status = "disabled";
> @@ -1090,9 +1091,10 @@
> reg = <0x021dc000 0x4000>;
> interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
> clocks = <&clks IMX6SX_CLK_SAI2_IPG>,
> + <&clks IMX6SX_CLK_SAI2>,
> <&clks IMX6SX_CLK_SAI2>,
> <&clks 0>, <&clks 0>;
> - clock-names = "bus", "mclk1", "mclk2", "mclk3";
> + clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3";
> dma-names = "rx", "tx";
> dmas = <&sdma 33 24 0>, <&sdma 34 24 0>;
> status = "disabled";
> diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
> index bbf010c73336..e9691306f557 100644
> --- a/arch/arm/boot/dts/imx6ul.dtsi
> +++ b/arch/arm/boot/dts/imx6ul.dtsi
> @@ -304,9 +304,10 @@
> reg = <0x02028000 0x4000>;
> interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>;
> clocks = <&clks IMX6UL_CLK_SAI1_IPG>,
> + <&clks IMX6UL_CLK_SAI1>,
> <&clks IMX6UL_CLK_SAI1>,
> <&clks IMX6UL_CLK_DUMMY>, <&clks IMX6UL_CLK_DUMMY>;
> - clock-names = "bus", "mclk1", "mclk2", "mclk3";
> + clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3";
> dmas = <&sdma 35 24 0>,
> <&sdma 36 24 0>;
> dma-names = "rx", "tx";
> @@ -319,9 +320,10 @@
> reg = <0x0202c000 0x4000>;
> interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
> clocks = <&clks IMX6UL_CLK_SAI2_IPG>,
> + <&clks IMX6UL_CLK_SAI2>,
> <&clks IMX6UL_CLK_SAI2>,
> <&clks IMX6UL_CLK_DUMMY>, <&clks IMX6UL_CLK_DUMMY>;
> - clock-names = "bus", "mclk1", "mclk2", "mclk3";
> + clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3";
> dmas = <&sdma 37 24 0>,
> <&sdma 38 24 0>;
> dma-names = "rx", "tx";
> @@ -334,9 +336,10 @@
> reg = <0x02030000 0x4000>;
> interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
> clocks = <&clks IMX6UL_CLK_SAI3_IPG>,
> + <&clks IMX6UL_CLK_SAI3>,
> <&clks IMX6UL_CLK_SAI3>,
> <&clks IMX6UL_CLK_DUMMY>, <&clks IMX6UL_CLK_DUMMY>;
> - clock-names = "bus", "mclk1", "mclk2", "mclk3";
> + clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3";
> dmas = <&sdma 39 24 0>,
> <&sdma 40 24 0>;
> dma-names = "rx", "tx";
> diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
> index 0b01109ac0a9..693b88e37799 100644
> --- a/arch/arm/boot/dts/imx7s.dtsi
> +++ b/arch/arm/boot/dts/imx7s.dtsi
> @@ -884,10 +884,11 @@
> reg = <0x308a0000 0x10000>;
> interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>;
> clocks = <&clks IMX7D_SAI1_IPG_CLK>,
> + <&clks IMX7D_SAI1_ROOT_CLK>,
> <&clks IMX7D_SAI1_ROOT_CLK>,
> <&clks IMX7D_CLK_DUMMY>,
> <&clks IMX7D_CLK_DUMMY>;
> - clock-names = "bus", "mclk1", "mclk2", "mclk3";
> + clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3";
> dma-names = "rx", "tx";
> dmas = <&sdma 8 24 0>, <&sdma 9 24 0>;
> status = "disabled";
> @@ -899,10 +900,11 @@
> reg = <0x308b0000 0x10000>;
> interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>;
> clocks = <&clks IMX7D_SAI2_IPG_CLK>,
> + <&clks IMX7D_SAI2_ROOT_CLK>,
> <&clks IMX7D_SAI2_ROOT_CLK>,
> <&clks IMX7D_CLK_DUMMY>,
> <&clks IMX7D_CLK_DUMMY>;
> - clock-names = "bus", "mclk1", "mclk2", "mclk3";
> + clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3";
> dma-names = "rx", "tx";
> dmas = <&sdma 10 24 0>, <&sdma 11 24 0>;
> status = "disabled";
> @@ -914,10 +916,11 @@
> reg = <0x308c0000 0x10000>;
> interrupts = <GIC_SPI 50 IRQ_TYPE_LEVEL_HIGH>;
> clocks = <&clks IMX7D_SAI3_IPG_CLK>,
> + <&clks IMX7D_SAI3_ROOT_CLK>,
> <&clks IMX7D_SAI3_ROOT_CLK>,
> <&clks IMX7D_CLK_DUMMY>,
> <&clks IMX7D_CLK_DUMMY>;
> - clock-names = "bus", "mclk1", "mclk2", "mclk3";
> + clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3";
> dma-names = "rx", "tx";
> dmas = <&sdma 12 24 0>, <&sdma 13 24 0>;
> status = "disabled";
> --
> 2.17.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-04-22 4:25 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <VE1PR04MB647951B20E0F334FF1CC3B7AE3220@VE1PR04MB6479.eurprd04.prod.outlook.com>
2019-04-22 4:25 ` [PATCH] ARM: dts: imx: Add mclk0 clock for SAI Nicolin Chen
2019-04-20 9:12 Daniel Baluta
2019-04-21 8:20 ` Nicolin Chen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).