LinuxLists.cc - [PATCH 1/5] arm64: dts: imx8qxp: Move usdhc clocks assignment to board DT

2019-10-16 10:26:53

Subject: [PATCH 1/5] arm64: dts: imx8qxp: Move usdhc clocks assignment to board DT

2019-10-16 10:28:12

Subject: [PATCH 2/5] arm64: dts: imx8mq: Move usdhc clocks assignment to board DT

usdhc's clock rate is different according to different devices
connected, so clock rate assignment should be placed in board
DT according to different devices connected on each usdhc port.

Signed-off-by: Anson Huang <[email protected]>
---
arch/arm64/boot/dts/freescale/imx8mq-evk.dts | 4 ++++
arch/arm64/boot/dts/freescale/imx8mq-hummingboard-pulse.dts | 2 ++
arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts | 4 ++++
arch/arm64/boot/dts/freescale/imx8mq-nitrogen.dts | 2 ++
arch/arm64/boot/dts/freescale/imx8mq-pico-pi.dts | 4 ++++
arch/arm64/boot/dts/freescale/imx8mq-sr-som.dtsi | 2 ++
arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi | 4 ++++
arch/arm64/boot/dts/freescale/imx8mq.dtsi | 2 --
8 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
index 4e0a281..40fa390 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
@@ -278,6 +278,8 @@
};

&usdhc1 {
+ assigned-clocks = <&clk IMX8MQ_CLK_USDHC1>;
+ assigned-clock-rates = <400000000>;
pinctrl-names = "default", "state_100mhz", "state_200mhz";
pinctrl-0 = <&pinctrl_usdhc1>;
pinctrl-1 = <&pinctrl_usdhc1_100mhz>;
@@ -291,6 +293,8 @@
};

&usdhc2 {
+ assigned-clocks = <&clk IMX8MQ_CLK_USDHC2>;
+ assigned-clock-rates = <200000000>;
pinctrl-names = "default", "state_100mhz", "state_200mhz";
pinctrl-0 = <&pinctrl_usdhc2>;
pinctrl-1 = <&pinctrl_usdhc2_100mhz>;
diff --git a/arch/arm64/boot/dts/freescale/imx8mq-hummingboard-pulse.dts b/arch/arm64/boot/dts/freescale/imx8mq-hummingboard-pulse.dts
index f52e872..b8cb20c 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-hummingboard-pulse.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mq-hummingboard-pulse.dts
@@ -110,6 +110,8 @@
};

&usdhc2 {
+ assigned-clocks = <&clk IMX8MQ_CLK_USDHC2>;
+ assigned-clock-rates = <200000000>;
pinctrl-names = "default", "state_100mhz", "state_200mhz";
pinctrl-0 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_gpio>;
pinctrl-1 = <&pinctrl_usdhc2_100mhz>, <&pinctrl_usdhc2_gpio>;
diff --git a/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts b/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
index 683a110..2a759df 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts
@@ -780,6 +780,8 @@
};

&usdhc1 {
+ assigned-clocks = <&clk IMX8MQ_CLK_USDHC1>;
+ assigned-clock-rates = <400000000>;
pinctrl-names = "default", "state_100mhz", "state_200mhz";
pinctrl-0 = <&pinctrl_usdhc1>;
pinctrl-1 = <&pinctrl_usdhc1_100mhz>;
@@ -790,6 +792,8 @@
};

&usdhc2 {
+ assigned-clocks = <&clk IMX8MQ_CLK_USDHC2>;
+ assigned-clock-rates = <200000000>;
pinctrl-names = "default", "state_100mhz", "state_200mhz";
pinctrl-0 = <&pinctrl_usdhc2>;
pinctrl-1 = <&pinctrl_usdhc2_100mhz>;
diff --git a/arch/arm64/boot/dts/freescale/imx8mq-nitrogen.dts b/arch/arm64/boot/dts/freescale/imx8mq-nitrogen.dts
index c832bf0..81d2692 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-nitrogen.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mq-nitrogen.dts
@@ -191,6 +191,8 @@
};

&usdhc1 {
+ assigned-clocks = <&clk IMX8MQ_CLK_USDHC1>;
+ assigned-clock-rates = <400000000>;
bus-width = <8>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_usdhc1>;
diff --git a/arch/arm64/boot/dts/freescale/imx8mq-pico-pi.dts b/arch/arm64/boot/dts/freescale/imx8mq-pico-pi.dts
index 8a4aee2..59da96b 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-pico-pi.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mq-pico-pi.dts
@@ -207,6 +207,8 @@
};

&usdhc1 {
+ assigned-clocks = <&clk IMX8MQ_CLK_USDHC1>;
+ assigned-clock-rates = <400000000>;
pinctrl-names = "default", "state_100mhz", "state_200mhz";
pinctrl-0 = <&pinctrl_usdhc1>;
pinctrl-1 = <&pinctrl_usdhc1_100mhz>;
@@ -217,6 +219,8 @@
};

&usdhc2 {
+ assigned-clocks = <&clk IMX8MQ_CLK_USDHC2>;
+ assigned-clock-rates = <200000000>;
pinctrl-names = "default", "state_100mhz", "state_200mhz";
pinctrl-0 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_gpio>;
pinctrl-1 = <&pinctrl_usdhc2_100mhz>, <&pinctrl_usdhc2_gpio>;
diff --git a/arch/arm64/boot/dts/freescale/imx8mq-sr-som.dtsi b/arch/arm64/boot/dts/freescale/imx8mq-sr-som.dtsi
index d7f03c6..3dc44114 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-sr-som.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq-sr-som.dtsi
@@ -170,6 +170,8 @@
};

&usdhc1 {
+ assigned-clocks = <&clk IMX8MQ_CLK_USDHC1>;
+ assigned-clock-rates = <400000000>;
pinctrl-names = "default", "state_100mhz", "state_200mhz";
pinctrl-0 = <&pinctrl_usdhc1>;
pinctrl-1 = <&pinctrl_usdhc1_100mhz>;
diff --git a/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi b/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi
index 087b5b6..333c4e2 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi
@@ -486,6 +486,8 @@
};

&usdhc1 {
+ assigned-clocks = <&clk IMX8MQ_CLK_USDHC1>;
+ assigned-clock-rates = <400000000>;
pinctrl-names = "default", "state_100mhz", "state_200mhz";
pinctrl-0 = <&pinctrl_usdhc1>;
pinctrl-1 = <&pinctrl_usdhc1_100mhz>;
@@ -499,6 +501,8 @@
};

&usdhc2 {
+ assigned-clocks = <&clk IMX8MQ_CLK_USDHC2>;
+ assigned-clock-rates = <200000000>;
pinctrl-names = "default", "state_100mhz", "state_200mhz";
pinctrl-0 = <&pinctrl_usdhc2>;
pinctrl-1 = <&pinctrl_usdhc2_100mhz>;
diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index a6d57fb..7f93194 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -868,8 +868,6 @@
<&clk IMX8MQ_CLK_NAND_USDHC_BUS>,
<&clk IMX8MQ_CLK_USDHC1_ROOT>;
clock-names = "ipg", "ahb", "per";
- assigned-clocks = <&clk IMX8MQ_CLK_USDHC1>;
- assigned-clock-rates = <400000000>;
fsl,tuning-start-tap = <20>;
fsl,tuning-step = <2>;
bus-width = <4>;
--
2.7.4

2019-10-16 10:28:40

by Anson Huang

[permalink] [raw]

Subject: [PATCH 3/5] arm64: dts: imx8mm: Move usdhc clocks assignment to board DT

2019-10-16 10:54:17

by Anson Huang

[permalink] [raw]

Subject: [PATCH 4/5] arm64: dts: imx8mn: Move usdhc clocks assignment to board DT

2019-10-16 10:54:42

by Anson Huang

[permalink] [raw]

Subject: [PATCH 5/5] ARM: dts: imx7ulp: Move usdhc clocks assignment to board DT

2019-10-20 14:36:34

by Abel Vesa

[permalink] [raw]

Subject: Re: [PATCH 1/5] arm64: dts: imx8qxp: Move usdhc clocks assignment to board DT

On 19-10-16 10:14:23, Anson Huang wrote:
> usdhc's clock rate is different according to different devices
> connected, so clock rate assignment should be placed in board
> DT according to different devices connected on each usdhc port.
>
> Signed-off-by: Anson Huang <[email protected]>

For the entire patchset:

Reviewed-by: Abel Vesa <[email protected]>

> ---
> arch/arm64/boot/dts/freescale/imx8qxp-ai_ml.dts | 4 ++++
> arch/arm64/boot/dts/freescale/imx8qxp-mek.dts | 4 ++++
> arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 6 ------
> 3 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-ai_ml.dts b/arch/arm64/boot/dts/freescale/imx8qxp-ai_ml.dts
> index 91eef97..a3f8cf1 100644
> --- a/arch/arm64/boot/dts/freescale/imx8qxp-ai_ml.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8qxp-ai_ml.dts
> @@ -133,6 +133,8 @@
> &usdhc1 {
> #address-cells = <1>;
> #size-cells = <0>;
> + assigned-clocks = <&clk IMX_CONN_SDHC0_CLK>;
> + assigned-clock-rates = <200000000>;
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_usdhc1>;
> bus-width = <4>;
> @@ -149,6 +151,8 @@
>
> /* SD */
> &usdhc2 {
> + assigned-clocks = <&clk IMX_CONN_SDHC1_CLK>;
> + assigned-clock-rates = <200000000>;
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_usdhc2>;
> bus-width = <4>;
> diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
> index 88dd9132..d3d26cc 100644
> --- a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
> @@ -137,6 +137,8 @@
> };
>
> &usdhc1 {
> + assigned-clocks = <&clk IMX_CONN_SDHC0_CLK>;
> + assigned-clock-rates = <200000000>;
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_usdhc1>;
> bus-width = <8>;
> @@ -147,6 +149,8 @@
> };
>
> &usdhc2 {
> + assigned-clocks = <&clk IMX_CONN_SDHC1_CLK>;
> + assigned-clock-rates = <200000000>;
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_usdhc2>;
> bus-width = <4>;
> diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> index 2d69f1a..9646a41 100644
> --- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> @@ -368,8 +368,6 @@
> <&conn_lpcg IMX_CONN_LPCG_SDHC0_PER_CLK>,
> <&conn_lpcg IMX_CONN_LPCG_SDHC0_HCLK>;
> clock-names = "ipg", "per", "ahb";
> - assigned-clocks = <&clk IMX_CONN_SDHC0_CLK>;
> - assigned-clock-rates = <200000000>;
> power-domains = <&pd IMX_SC_R_SDHC_0>;
> status = "disabled";
> };
> @@ -383,8 +381,6 @@
> <&conn_lpcg IMX_CONN_LPCG_SDHC1_PER_CLK>,
> <&conn_lpcg IMX_CONN_LPCG_SDHC1_HCLK>;
> clock-names = "ipg", "per", "ahb";
> - assigned-clocks = <&clk IMX_CONN_SDHC1_CLK>;
> - assigned-clock-rates = <200000000>;
> power-domains = <&pd IMX_SC_R_SDHC_1>;
> fsl,tuning-start-tap = <20>;
> fsl,tuning-step= <2>;
> @@ -400,8 +396,6 @@
> <&conn_lpcg IMX_CONN_LPCG_SDHC2_PER_CLK>,
> <&conn_lpcg IMX_CONN_LPCG_SDHC2_HCLK>;
> clock-names = "ipg", "per", "ahb";
> - assigned-clocks = <&clk IMX_CONN_SDHC2_CLK>;
> - assigned-clock-rates = <200000000>;
> power-domains = <&pd IMX_SC_R_SDHC_2>;
> status = "disabled";
> };
> --
> 2.7.4
>

2019-10-26 12:11:47

by Shawn Guo

[permalink] [raw]

Subject: Re: [PATCH 1/5] arm64: dts: imx8qxp: Move usdhc clocks assignment to board DT

On Wed, Oct 16, 2019 at 10:14:23AM +0800, Anson Huang wrote:
> usdhc's clock rate is different according to different devices
> connected, so clock rate assignment should be placed in board
> DT according to different devices connected on each usdhc port.

I think it should be fine that we have a reasonable default settings in
soc.dtsi, and boards that need a different setup can overwrite the
settings in board.dts.

Shawn

>
> Signed-off-by: Anson Huang <[email protected]>
> ---
> arch/arm64/boot/dts/freescale/imx8qxp-ai_ml.dts | 4 ++++
> arch/arm64/boot/dts/freescale/imx8qxp-mek.dts | 4 ++++
> arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 6 ------
> 3 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-ai_ml.dts b/arch/arm64/boot/dts/freescale/imx8qxp-ai_ml.dts
> index 91eef97..a3f8cf1 100644
> --- a/arch/arm64/boot/dts/freescale/imx8qxp-ai_ml.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8qxp-ai_ml.dts
> @@ -133,6 +133,8 @@
> &usdhc1 {
> #address-cells = <1>;
> #size-cells = <0>;
> + assigned-clocks = <&clk IMX_CONN_SDHC0_CLK>;
> + assigned-clock-rates = <200000000>;
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_usdhc1>;
> bus-width = <4>;
> @@ -149,6 +151,8 @@
>
> /* SD */
> &usdhc2 {
> + assigned-clocks = <&clk IMX_CONN_SDHC1_CLK>;
> + assigned-clock-rates = <200000000>;
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_usdhc2>;
> bus-width = <4>;
> diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
> index 88dd9132..d3d26cc 100644
> --- a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
> @@ -137,6 +137,8 @@
> };
>
> &usdhc1 {
> + assigned-clocks = <&clk IMX_CONN_SDHC0_CLK>;
> + assigned-clock-rates = <200000000>;
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_usdhc1>;
> bus-width = <8>;
> @@ -147,6 +149,8 @@
> };
>
> &usdhc2 {
> + assigned-clocks = <&clk IMX_CONN_SDHC1_CLK>;
> + assigned-clock-rates = <200000000>;
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_usdhc2>;
> bus-width = <4>;
> diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> index 2d69f1a..9646a41 100644
> --- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
> @@ -368,8 +368,6 @@
> <&conn_lpcg IMX_CONN_LPCG_SDHC0_PER_CLK>,
> <&conn_lpcg IMX_CONN_LPCG_SDHC0_HCLK>;
> clock-names = "ipg", "per", "ahb";
> - assigned-clocks = <&clk IMX_CONN_SDHC0_CLK>;
> - assigned-clock-rates = <200000000>;
> power-domains = <&pd IMX_SC_R_SDHC_0>;
> status = "disabled";
> };
> @@ -383,8 +381,6 @@
> <&conn_lpcg IMX_CONN_LPCG_SDHC1_PER_CLK>,
> <&conn_lpcg IMX_CONN_LPCG_SDHC1_HCLK>;
> clock-names = "ipg", "per", "ahb";
> - assigned-clocks = <&clk IMX_CONN_SDHC1_CLK>;
> - assigned-clock-rates = <200000000>;
> power-domains = <&pd IMX_SC_R_SDHC_1>;
> fsl,tuning-start-tap = <20>;
> fsl,tuning-step= <2>;
> @@ -400,8 +396,6 @@
> <&conn_lpcg IMX_CONN_LPCG_SDHC2_PER_CLK>,
> <&conn_lpcg IMX_CONN_LPCG_SDHC2_HCLK>;
> clock-names = "ipg", "per", "ahb";
> - assigned-clocks = <&clk IMX_CONN_SDHC2_CLK>;
> - assigned-clock-rates = <200000000>;
> power-domains = <&pd IMX_SC_R_SDHC_2>;
> status = "disabled";
> };
> --
> 2.7.4
>

2019-10-28 12:44:31

by Anson Huang

[permalink] [raw]

Subject: RE: [PATCH 1/5] arm64: dts: imx8qxp: Move usdhc clocks assignment to board DT

Hi, Shawn

> On Wed, Oct 16, 2019 at 10:14:23AM +0800, Anson Huang wrote:
> > usdhc's clock rate is different according to different devices
> > connected, so clock rate assignment should be placed in board DT
> > according to different devices connected on each usdhc port.
>
> I think it should be fine that we have a reasonable default settings in soc.dtsi,
> and boards that need a different setup can overwrite the settings in
> board.dts.

Someone was complaining about the usdhc clock assignment in soc.dtsi, because some
usdhc nodes are having clock assignments while some are NOT. That is why I did this
patch set. I agree that we can have default settings in soc.dtsi, so do you think it makes
sense to add default clock assignment to all usdhc nodes? If yes, I will redo the patch
set.

Thanks,
Anson.

2019-10-28 13:14:53

by Shawn Guo

[permalink] [raw]

Subject: Re: [PATCH 1/5] arm64: dts: imx8qxp: Move usdhc clocks assignment to board DT

On Mon, Oct 28, 2019 at 01:29:32AM +0000, Anson Huang wrote:
> Hi, Shawn
>
> > On Wed, Oct 16, 2019 at 10:14:23AM +0800, Anson Huang wrote:
> > > usdhc's clock rate is different according to different devices
> > > connected, so clock rate assignment should be placed in board DT
> > > according to different devices connected on each usdhc port.
> >
> > I think it should be fine that we have a reasonable default settings in soc.dtsi,
> > and boards that need a different setup can overwrite the settings in
> > board.dts.
>
> Someone was complaining about the usdhc clock assignment in soc.dtsi, because some
> usdhc nodes are having clock assignments while some are NOT. That is why I did this
> patch set. I agree that we can have default settings in soc.dtsi, so do you think it makes
> sense to add default clock assignment to all usdhc nodes? If yes, I will redo the patch
> set.

I had a second thought on this. To ease the maintenance of soc.dtsi,
let's do clock assignments in board.dts.

Series applied, thanks.

Shawn