2022-06-22 06:41:44

by Peng Fan (OSS)

[permalink] [raw]
Subject: [PATCH] arm64: dts: imx8mp: drop dmas property for uart console

From: Peng Fan <[email protected]>

Commit 71066545b48e4("driver core: Set fw_devlink.strict=1 by default")
default set fw_devlink to true. This has a side effect to i.MX uart
console. The sdma will make the i.MX8MP uart driver defer probe for some
time (~10s with i.MX8MP-EVK board) until sdma ready, because sdma is a
supplier with property dmas set in device tree node.

Since this uart is for console, we need log printed out as soon as
possible, so remove the dmas property for the uart console node.

Signed-off-by: Peng Fan <[email protected]>
---

V1:
The upper set fw_devlink.strict=1 patch is in linux-next tree.

arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 2 ++
arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp-edimm2.2.dts | 2 ++
arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts | 2 ++
arch/arm64/boot/dts/freescale/imx8mp-venice-gw74xx.dts | 2 ++
4 files changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
index fb11c03bc8b1..3fdb38bc0069 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
@@ -359,6 +359,8 @@ &uart2 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_uart2>;
status = "okay";
+ /delete-property/ dmas;
+ /delete-property/ dma-names;
};

&usb3_phy1 {
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp-edimm2.2.dts b/arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp-edimm2.2.dts
index dd703b6a5e17..fb2b44e94482 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp-edimm2.2.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp-edimm2.2.dts
@@ -69,6 +69,8 @@ &uart2 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_uart2>;
status = "okay";
+ /delete-property/ dmas;
+ /delete-property/ dma-names;
};

&usb3_phy0 {
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
index 6aa720bafe28..68a478151292 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
@@ -99,6 +99,8 @@ &uart1 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_uart1>;
status = "okay";
+ /delete-property/ dmas;
+ /delete-property/ dma-names;
};

/* SD-Card */
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-venice-gw74xx.dts b/arch/arm64/boot/dts/freescale/imx8mp-venice-gw74xx.dts
index 521215520a0f..f90c1ac2791c 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-venice-gw74xx.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mp-venice-gw74xx.dts
@@ -554,6 +554,8 @@ &uart2 {
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_uart2>;
status = "okay";
+ /delete-property/ dmas;
+ /delete-property/ dma-names;
};

&uart4 {
--
2.25.1


2022-06-22 07:48:18

by Sascha Hauer

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: imx8mp: drop dmas property for uart console

+Cc Saravana Kannan <[email protected]>, the author of 71066545b48e4

On Wed, Jun 22, 2022 at 02:20:27PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <[email protected]>
>
> Commit 71066545b48e4("driver core: Set fw_devlink.strict=1 by default")
> default set fw_devlink to true. This has a side effect to i.MX uart
> console. The sdma will make the i.MX8MP uart driver defer probe for some
> time (~10s with i.MX8MP-EVK board) until sdma ready, because sdma is a
> supplier with property dmas set in device tree node.

I just tested this on an i.MX6 board and observed the same behaviour.
The same will happen on any other i.MX board as well. This will also
likely happen on any other SoC on which the UART driver uses dmaengine.

>
> Since this uart is for console, we need log printed out as soon as
> possible, so remove the dmas property for the uart console node.

Fixing this at board level is not really an option because that means
fixing each and every, at least i.MX board in the tree. Furthermore
this would mean to bring the deleted property back in and to remove
another dmas property should a user want to switch to another console
port.

For what it's worth: NACK for 71066545b48e4.

Sascha

>
> Signed-off-by: Peng Fan <[email protected]>
> ---
>
> V1:
> The upper set fw_devlink.strict=1 patch is in linux-next tree.
>
> arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 2 ++
> arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp-edimm2.2.dts | 2 ++
> arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts | 2 ++
> arch/arm64/boot/dts/freescale/imx8mp-venice-gw74xx.dts | 2 ++
> 4 files changed, 8 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> index fb11c03bc8b1..3fdb38bc0069 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> @@ -359,6 +359,8 @@ &uart2 {
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_uart2>;
> status = "okay";
> + /delete-property/ dmas;
> + /delete-property/ dma-names;
> };
>
> &usb3_phy1 {
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp-edimm2.2.dts b/arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp-edimm2.2.dts
> index dd703b6a5e17..fb2b44e94482 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp-edimm2.2.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp-edimm2.2.dts
> @@ -69,6 +69,8 @@ &uart2 {
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_uart2>;
> status = "okay";
> + /delete-property/ dmas;
> + /delete-property/ dma-names;
> };
>
> &usb3_phy0 {
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
> index 6aa720bafe28..68a478151292 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
> @@ -99,6 +99,8 @@ &uart1 {
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_uart1>;
> status = "okay";
> + /delete-property/ dmas;
> + /delete-property/ dma-names;
> };
>
> /* SD-Card */
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-venice-gw74xx.dts b/arch/arm64/boot/dts/freescale/imx8mp-venice-gw74xx.dts
> index 521215520a0f..f90c1ac2791c 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp-venice-gw74xx.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-venice-gw74xx.dts
> @@ -554,6 +554,8 @@ &uart2 {
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_uart2>;
> status = "okay";
> + /delete-property/ dmas;
> + /delete-property/ dma-names;
> };
>
> &uart4 {
> --
> 2.25.1
>
>

--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

2022-06-22 22:41:58

by Peng Fan

[permalink] [raw]
Subject: RE: [PATCH] arm64: dts: imx8mp: drop dmas property for uart console

> Subject: Re: [PATCH] arm64: dts: imx8mp: drop dmas property for uart console
>
> On Wed, Jun 22, 2022 at 12:24 AM Sascha Hauer <[email protected]>
> wrote:
> >
> > +Cc Saravana Kannan <[email protected]>, the author of
> > +71066545b48e4
> >
> > On Wed, Jun 22, 2022 at 02:20:27PM +0800, Peng Fan (OSS) wrote:
> > > From: Peng Fan <[email protected]>
> > >
> > > Commit 71066545b48e4("driver core: Set fw_devlink.strict=1 by
> > > default") default set fw_devlink to true. This has a side effect to
> > > i.MX uart console. The sdma will make the i.MX8MP uart driver defer
> > > probe for some time (~10s with i.MX8MP-EVK board) until sdma ready,
> > > because sdma is a supplier with property dmas set in device tree node.
> >
> > I just tested this on an i.MX6 board and observed the same behaviour.
> > The same will happen on any other i.MX board as well. This will also
> > likely happen on any other SoC on which the UART driver uses dmaengine.
> >
> > >
> > > Since this uart is for console, we need log printed out as soon as
> > > possible, so remove the dmas property for the uart console node.
> >
> > Fixing this at board level is not really an option because that means
> > fixing each and every, at least i.MX board in the tree. Furthermore
> > this would mean to bring the deleted property back in and to remove
> > another dmas property should a user want to switch to another console
> > port.
> >
> > For what it's worth: NACK for 71066545b48e4.
>
> Peng, I sent a fix. Can you give it a shot please?

Thanks for the quick fix. I have replied your patch with my T-b.
My patch could be dropped.

Thanks,
Peng.

>
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.ke
> rnel.org%2Flkml%2F20220622215912.550419-1-saravanak%40google.com%
> 2F&amp;data=05%7C01%7Cpeng.fan%40nxp.com%7C7a841838946c4df32eb
> 408da549efa47%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63
> 7915339019509985%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMD
> AiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C
> &amp;sdata=wO7NEebGVRp1C7LvhRyZnTTzJwma5eHDfDcoAqdLE24%3D&am
> p;reserved=0
>
> -Saravana
>
> >
> > Sascha
> >
> > >
> > > Signed-off-by: Peng Fan <[email protected]>
> > > ---
> > >
> > > V1:
> > > The upper set fw_devlink.strict=1 patch is in linux-next tree.
> > >
> > > arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 2
> ++
> > > arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp-edimm2.2.dts | 2
> > > ++ arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts |
> 2 ++
> > > arch/arm64/boot/dts/freescale/imx8mp-venice-gw74xx.dts | 2
> ++
> > > 4 files changed, 8 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > > b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > > index fb11c03bc8b1..3fdb38bc0069 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > > @@ -359,6 +359,8 @@ &uart2 {
> > > pinctrl-names = "default";
> > > pinctrl-0 = <&pinctrl_uart2>;
> > > status = "okay";
> > > + /delete-property/ dmas;
> > > + /delete-property/ dma-names;
> > > };
> > >
> > > &usb3_phy1 {
> > > diff --git
> > > a/arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp-edimm2.2.dts
> > > b/arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp-edimm2.2.dts
> > > index dd703b6a5e17..fb2b44e94482 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp-edimm2.2.dts
> > > +++
> b/arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp-edimm2.2.dts
> > > @@ -69,6 +69,8 @@ &uart2 {
> > > pinctrl-names = "default";
> > > pinctrl-0 = <&pinctrl_uart2>;
> > > status = "okay";
> > > + /delete-property/ dmas;
> > > + /delete-property/ dma-names;
> > > };
> > >
> > > &usb3_phy0 {
> > > diff --git
> > > a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
> > > b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
> > > index 6aa720bafe28..68a478151292 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
> > > @@ -99,6 +99,8 @@ &uart1 {
> > > pinctrl-names = "default";
> > > pinctrl-0 = <&pinctrl_uart1>;
> > > status = "okay";
> > > + /delete-property/ dmas;
> > > + /delete-property/ dma-names;
> > > };
> > >
> > > /* SD-Card */
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-venice-gw74xx.dts
> > > b/arch/arm64/boot/dts/freescale/imx8mp-venice-gw74xx.dts
> > > index 521215520a0f..f90c1ac2791c 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8mp-venice-gw74xx.dts
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mp-venice-gw74xx.dts
> > > @@ -554,6 +554,8 @@ &uart2 {
> > > pinctrl-names = "default";
> > > pinctrl-0 = <&pinctrl_uart2>;
> > > status = "okay";
> > > + /delete-property/ dmas;
> > > + /delete-property/ dma-names;
> > > };
> > >
> > > &uart4 {
> > > --
> > > 2.25.1
> > >
> > >
> >
> > --
> > Pengutronix e.K. |
> |
> > Steuerwalder Str. 21 |
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.pe
> ngutronix.de%2F&amp;data=05%7C01%7Cpeng.fan%40nxp.com%7C7a84183
> 8946c4df32eb408da549efa47%7C686ea1d3bc2b4c6fa92cd99c5c301635%7
> C0%7C0%7C637915339019509985%7CUnknown%7CTWFpbGZsb3d8eyJWIjoi
> MC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C300
> 0%7C%7C%7C&amp;sdata=h0WK5980JKlkNVax%2F7PopVftgkClnG0SLAvuPqh
> lj0A%3D&amp;reserved=0 |
> > 31137 Hildesheim, Germany | Phone:
> +49-5121-206917-0 |
> > Amtsgericht Hildesheim, HRA 2686 | Fax:
> +49-5121-206917-5555 |

2022-06-22 22:52:34

by Saravana Kannan

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: imx8mp: drop dmas property for uart console

On Wed, Jun 22, 2022 at 12:24 AM Sascha Hauer <[email protected]> wrote:
>
> +Cc Saravana Kannan <[email protected]>, the author of 71066545b48e4
>
> On Wed, Jun 22, 2022 at 02:20:27PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <[email protected]>
> >
> > Commit 71066545b48e4("driver core: Set fw_devlink.strict=1 by default")
> > default set fw_devlink to true. This has a side effect to i.MX uart
> > console. The sdma will make the i.MX8MP uart driver defer probe for some
> > time (~10s with i.MX8MP-EVK board) until sdma ready, because sdma is a
> > supplier with property dmas set in device tree node.
>
> I just tested this on an i.MX6 board and observed the same behaviour.
> The same will happen on any other i.MX board as well. This will also
> likely happen on any other SoC on which the UART driver uses dmaengine.
>
> >
> > Since this uart is for console, we need log printed out as soon as
> > possible, so remove the dmas property for the uart console node.
>
> Fixing this at board level is not really an option because that means
> fixing each and every, at least i.MX board in the tree. Furthermore
> this would mean to bring the deleted property back in and to remove
> another dmas property should a user want to switch to another console
> port.
>
> For what it's worth: NACK for 71066545b48e4.

Peng, I sent a fix. Can you give it a shot please?

https://lore.kernel.org/lkml/[email protected]/

-Saravana

>
> Sascha
>
> >
> > Signed-off-by: Peng Fan <[email protected]>
> > ---
> >
> > V1:
> > The upper set fw_devlink.strict=1 patch is in linux-next tree.
> >
> > arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 2 ++
> > arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp-edimm2.2.dts | 2 ++
> > arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts | 2 ++
> > arch/arm64/boot/dts/freescale/imx8mp-venice-gw74xx.dts | 2 ++
> > 4 files changed, 8 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > index fb11c03bc8b1..3fdb38bc0069 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > @@ -359,6 +359,8 @@ &uart2 {
> > pinctrl-names = "default";
> > pinctrl-0 = <&pinctrl_uart2>;
> > status = "okay";
> > + /delete-property/ dmas;
> > + /delete-property/ dma-names;
> > };
> >
> > &usb3_phy1 {
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp-edimm2.2.dts b/arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp-edimm2.2.dts
> > index dd703b6a5e17..fb2b44e94482 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp-edimm2.2.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp-edimm2.2.dts
> > @@ -69,6 +69,8 @@ &uart2 {
> > pinctrl-names = "default";
> > pinctrl-0 = <&pinctrl_uart2>;
> > status = "okay";
> > + /delete-property/ dmas;
> > + /delete-property/ dma-names;
> > };
> >
> > &usb3_phy0 {
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
> > index 6aa720bafe28..68a478151292 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
> > @@ -99,6 +99,8 @@ &uart1 {
> > pinctrl-names = "default";
> > pinctrl-0 = <&pinctrl_uart1>;
> > status = "okay";
> > + /delete-property/ dmas;
> > + /delete-property/ dma-names;
> > };
> >
> > /* SD-Card */
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-venice-gw74xx.dts b/arch/arm64/boot/dts/freescale/imx8mp-venice-gw74xx.dts
> > index 521215520a0f..f90c1ac2791c 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp-venice-gw74xx.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp-venice-gw74xx.dts
> > @@ -554,6 +554,8 @@ &uart2 {
> > pinctrl-names = "default";
> > pinctrl-0 = <&pinctrl_uart2>;
> > status = "okay";
> > + /delete-property/ dmas;
> > + /delete-property/ dma-names;
> > };
> >
> > &uart4 {
> > --
> > 2.25.1
> >
> >
>
> --
> Pengutronix e.K. | |
> Steuerwalder Str. 21 | http://www.pengutronix.de/ |
> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

2022-06-23 11:42:22

by Fabio Estevam

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: imx8mp: drop dmas property for uart console

Hi Saravana,

On Wed, Jun 22, 2022 at 7:31 PM Saravana Kannan <[email protected]> wrote:

> Peng, I sent a fix. Can you give it a shot please?
>
> https://lore.kernel.org/lkml/[email protected]/

It didn't work for me on an imx6q-sabresd board. I could not find a v2.

Also, would this solution work when stdout-path is not passed in the devicetree?

Thanks

2022-06-23 19:23:13

by Saravana Kannan

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: imx8mp: drop dmas property for uart console

On Thu, Jun 23, 2022 at 11:20 AM Fabio Estevam <[email protected]> wrote:
>
> On Thu, Jun 23, 2022 at 8:35 AM Fabio Estevam <[email protected]> wrote:
>
> > It didn't work for me on an imx6q-sabresd board. I could not find a v2.
>
> Ok, found your v2 and tested it.
>
> It solves the console problem on an imx6q-sabresd.
>
> > Also, would this solution work when stdout-path is not passed in the devicetree?
>
> I have confirmed that v2 does not fix the case when stdout-path is not
> passed in the devicetree.
>
> While it is a good practice to describe stdout-path in dt, not all
> devicetree files do this,

I don't think it'll be hard to fix the console= case either. In the
case where you are not using stdout-path, how are you setting the
console? Just want to make sure I'm handling that case too.

-Saravana

2022-06-23 19:38:34

by Fabio Estevam

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: imx8mp: drop dmas property for uart console

On Thu, Jun 23, 2022 at 8:35 AM Fabio Estevam <[email protected]> wrote:

> It didn't work for me on an imx6q-sabresd board. I could not find a v2.

Ok, found your v2 and tested it.

It solves the console problem on an imx6q-sabresd.

> Also, would this solution work when stdout-path is not passed in the devicetree?

I have confirmed that v2 does not fix the case when stdout-path is not
passed in the devicetree.

While it is a good practice to describe stdout-path in dt, not all
devicetree files do this,
so 71066545b48e4 will cause regressions on several boards.

2022-06-23 20:01:49

by Saravana Kannan

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: imx8mp: drop dmas property for uart console

On Thu, Jun 23, 2022 at 11:43 AM Fabio Estevam <[email protected]> wrote:
>
> On Thu, Jun 23, 2022 at 3:26 PM Saravana Kannan <[email protected]> wrote:
>
> > I don't think it'll be hard to fix the console= case either. In the
> > case where you are not using stdout-path, how are you setting the
> > console? Just want to make sure I'm handling that case too.
>
> When stdout-path is not passed in DT, then the bootloader needs to pass
> console=ttymxc0,115200, for example.
>
> Please copy me in v3 if you send it.

Will do. I'm looking at the serial console code and I don't see a
difference between earlycon= vs console= handling. And I know that
earlycon= doesn't go through the driver core and isn't affected by any
of this. If you have additional pointers on where console= is parsed,
feel free to pass it on. I'll continue poking at this.

-Saravana

2022-06-23 20:02:35

by Fabio Estevam

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: imx8mp: drop dmas property for uart console

On Thu, Jun 23, 2022 at 3:26 PM Saravana Kannan <[email protected]> wrote:

> I don't think it'll be hard to fix the console= case either. In the
> case where you are not using stdout-path, how are you setting the
> console? Just want to make sure I'm handling that case too.

When stdout-path is not passed in DT, then the bootloader needs to pass
console=ttymxc0,115200, for example.

Please copy me in v3 if you send it.

Thanks

2022-06-23 20:59:18

by Fabio Estevam

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: imx8mp: drop dmas property for uart console

On Thu, Jun 23, 2022 at 3:58 PM Saravana Kannan <[email protected]> wrote:

> Will do. I'm looking at the serial console code and I don't see a
> difference between earlycon= vs console= handling. And I know that
> earlycon= doesn't go through the driver core and isn't affected by any
> of this. If you have additional pointers on where console= is parsed,
> feel free to pass it on. I'll continue poking at this.

It seems to be parsed at uart_parse_earlycon() inside
drivers/tty/serial/serial_core.c.

2022-06-23 21:54:24

by Saravana Kannan

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: imx8mp: drop dmas property for uart console

On Thu, Jun 23, 2022 at 1:51 PM Fabio Estevam <[email protected]> wrote:
>
> On Thu, Jun 23, 2022 at 3:58 PM Saravana Kannan <[email protected]> wrote:
>
> > Will do. I'm looking at the serial console code and I don't see a
> > difference between earlycon= vs console= handling. And I know that
> > earlycon= doesn't go through the driver core and isn't affected by any
> > of this. If you have additional pointers on where console= is parsed,
> > feel free to pass it on. I'll continue poking at this.
>
> It seems to be parsed at uart_parse_earlycon() inside
> drivers/tty/serial/serial_core.c.

Turns out it's kernel/printk/printk.c

-Saravana

2022-06-25 07:29:06

by Saravana Kannan

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: imx8mp: drop dmas property for uart console

On Wed, Jun 22, 2022 at 12:24 AM Sascha Hauer <[email protected]> wrote:
>
> +Cc Saravana Kannan <[email protected]>, the author of 71066545b48e4
>
> On Wed, Jun 22, 2022 at 02:20:27PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <[email protected]>
> >
> > Commit 71066545b48e4("driver core: Set fw_devlink.strict=1 by default")
> > default set fw_devlink to true. This has a side effect to i.MX uart
> > console. The sdma will make the i.MX8MP uart driver defer probe for some
> > time (~10s with i.MX8MP-EVK board) until sdma ready, because sdma is a
> > supplier with property dmas set in device tree node.
>
> I just tested this on an i.MX6 board and observed the same behaviour.
> The same will happen on any other i.MX board as well. This will also
> likely happen on any other SoC on which the UART driver uses dmaengine.
>
> >
> > Since this uart is for console, we need log printed out as soon as
> > possible, so remove the dmas property for the uart console node.
>
> Fixing this at board level is not really an option because that means
> fixing each and every, at least i.MX board in the tree. Furthermore
> this would mean to bring the deleted property back in and to remove
> another dmas property should a user want to switch to another console
> port.
>
> For what it's worth: NACK for 71066545b48e4.

Hi Peng/Sascha/Fabio,

I was looking at a fix for this (even when stdout-path isn't set in
DT) and looking at the older code (say, 5.18), and it looks like
before my recent changes the console would still only get probed
during late_initcall(). Does that match your experience?

Thanks,
Saravana

>
> Sascha
>
> >
> > Signed-off-by: Peng Fan <[email protected]>
> > ---
> >
> > V1:
> > The upper set fw_devlink.strict=1 patch is in linux-next tree.
> >
> > arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 2 ++
> > arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp-edimm2.2.dts | 2 ++
> > arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts | 2 ++
> > arch/arm64/boot/dts/freescale/imx8mp-venice-gw74xx.dts | 2 ++
> > 4 files changed, 8 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > index fb11c03bc8b1..3fdb38bc0069 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
> > @@ -359,6 +359,8 @@ &uart2 {
> > pinctrl-names = "default";
> > pinctrl-0 = <&pinctrl_uart2>;
> > status = "okay";
> > + /delete-property/ dmas;
> > + /delete-property/ dma-names;
> > };
> >
> > &usb3_phy1 {
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp-edimm2.2.dts b/arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp-edimm2.2.dts
> > index dd703b6a5e17..fb2b44e94482 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp-edimm2.2.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp-edimm2.2.dts
> > @@ -69,6 +69,8 @@ &uart2 {
> > pinctrl-names = "default";
> > pinctrl-0 = <&pinctrl_uart2>;
> > status = "okay";
> > + /delete-property/ dmas;
> > + /delete-property/ dma-names;
> > };
> >
> > &usb3_phy0 {
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
> > index 6aa720bafe28..68a478151292 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
> > @@ -99,6 +99,8 @@ &uart1 {
> > pinctrl-names = "default";
> > pinctrl-0 = <&pinctrl_uart1>;
> > status = "okay";
> > + /delete-property/ dmas;
> > + /delete-property/ dma-names;
> > };
> >
> > /* SD-Card */
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-venice-gw74xx.dts b/arch/arm64/boot/dts/freescale/imx8mp-venice-gw74xx.dts
> > index 521215520a0f..f90c1ac2791c 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp-venice-gw74xx.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp-venice-gw74xx.dts
> > @@ -554,6 +554,8 @@ &uart2 {
> > pinctrl-names = "default";
> > pinctrl-0 = <&pinctrl_uart2>;
> > status = "okay";
> > + /delete-property/ dmas;
> > + /delete-property/ dma-names;
> > };
> >
> > &uart4 {
> > --
> > 2.25.1
> >
> >
>
> --
> Pengutronix e.K. | |
> Steuerwalder Str. 21 | http://www.pengutronix.de/ |
> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

2022-06-27 08:53:21

by Peng Fan (OSS)

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: imx8mp: drop dmas property for uart console

Hi Saravana,

在 2022/6/25 15:15, Saravana Kannan 写道:
> On Wed, Jun 22, 2022 at 12:24 AM Sascha Hauer <[email protected]> wrote:
>> +Cc Saravana Kannan <[email protected]>, the author of 71066545b48e4
>>
>> On Wed, Jun 22, 2022 at 02:20:27PM +0800, Peng Fan (OSS) wrote:
>>> From: Peng Fan <[email protected]>
>>>
>>> Commit 71066545b48e4("driver core: Set fw_devlink.strict=1 by default")
>>> default set fw_devlink to true. This has a side effect to i.MX uart
>>> console. The sdma will make the i.MX8MP uart driver defer probe for some
>>> time (~10s with i.MX8MP-EVK board) until sdma ready, because sdma is a
>>> supplier with property dmas set in device tree node.
>> I just tested this on an i.MX6 board and observed the same behaviour.
>> The same will happen on any other i.MX board as well. This will also
>> likely happen on any other SoC on which the UART driver uses dmaengine.
>>
>>> Since this uart is for console, we need log printed out as soon as
>>> possible, so remove the dmas property for the uart console node.
>> Fixing this at board level is not really an option because that means
>> fixing each and every, at least i.MX board in the tree. Furthermore
>> this would mean to bring the deleted property back in and to remove
>> another dmas property should a user want to switch to another console
>> port.
>>
>> For what it's worth: NACK for 71066545b48e4.
> Hi Peng/Sascha/Fabio,
>
> I was looking at a fix for this (even when stdout-path isn't set in
> DT) and looking at the older code (say, 5.18), and it looks like
> before my recent changes the console would still only get probed
> during late_initcall(). Does that match your experience?

I could not get you. In driver, it is module init:
module_init(imx_uart_init), not late_initcall.

Thanks,
Peng

>
> Thanks,
> Saravana
>
>> Sascha
>>
>>> Signed-off-by: Peng Fan <[email protected]>
>>> ---
>>>
>>> V1:
>>> The upper set fw_devlink.strict=1 patch is in linux-next tree.
>>>
>>> arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 2 ++
>>> arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp-edimm2.2.dts | 2 ++
>>> arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts | 2 ++
>>> arch/arm64/boot/dts/freescale/imx8mp-venice-gw74xx.dts | 2 ++
>>> 4 files changed, 8 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
>>> index fb11c03bc8b1..3fdb38bc0069 100644
>>> --- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
>>> +++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
>>> @@ -359,6 +359,8 @@ &uart2 {
>>> pinctrl-names = "default";
>>> pinctrl-0 = <&pinctrl_uart2>;
>>> status = "okay";
>>> + /delete-property/ dmas;
>>> + /delete-property/ dma-names;
>>> };
>>>
>>> &usb3_phy1 {
>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp-edimm2.2.dts b/arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp-edimm2.2.dts
>>> index dd703b6a5e17..fb2b44e94482 100644
>>> --- a/arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp-edimm2.2.dts
>>> +++ b/arch/arm64/boot/dts/freescale/imx8mp-icore-mx8mp-edimm2.2.dts
>>> @@ -69,6 +69,8 @@ &uart2 {
>>> pinctrl-names = "default";
>>> pinctrl-0 = <&pinctrl_uart2>;
>>> status = "okay";
>>> + /delete-property/ dmas;
>>> + /delete-property/ dma-names;
>>> };
>>>
>>> &usb3_phy0 {
>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
>>> index 6aa720bafe28..68a478151292 100644
>>> --- a/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
>>> +++ b/arch/arm64/boot/dts/freescale/imx8mp-phyboard-pollux-rdk.dts
>>> @@ -99,6 +99,8 @@ &uart1 {
>>> pinctrl-names = "default";
>>> pinctrl-0 = <&pinctrl_uart1>;
>>> status = "okay";
>>> + /delete-property/ dmas;
>>> + /delete-property/ dma-names;
>>> };
>>>
>>> /* SD-Card */
>>> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-venice-gw74xx.dts b/arch/arm64/boot/dts/freescale/imx8mp-venice-gw74xx.dts
>>> index 521215520a0f..f90c1ac2791c 100644
>>> --- a/arch/arm64/boot/dts/freescale/imx8mp-venice-gw74xx.dts
>>> +++ b/arch/arm64/boot/dts/freescale/imx8mp-venice-gw74xx.dts
>>> @@ -554,6 +554,8 @@ &uart2 {
>>> pinctrl-names = "default";
>>> pinctrl-0 = <&pinctrl_uart2>;
>>> status = "okay";
>>> + /delete-property/ dmas;
>>> + /delete-property/ dma-names;
>>> };
>>>
>>> &uart4 {
>>> --
>>> 2.25.1
>>>
>>>
>> --
>> Pengutronix e.K. | |
>> Steuerwalder Str. 21 | http://www.pengutronix.de/ |
>> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
>> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

2022-06-28 03:03:08

by Saravana Kannan

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: imx8mp: drop dmas property for uart console

On Mon, Jun 27, 2022 at 1:22 AM Peng Fan (OSS) <[email protected]> wrote:
>
> Hi Saravana,
>
> 在 2022/6/25 15:15, Saravana Kannan 写道:
> > On Wed, Jun 22, 2022 at 12:24 AM Sascha Hauer <[email protected]> wrote:
> >> +Cc Saravana Kannan <[email protected]>, the author of 71066545b48e4
> >>
> >> On Wed, Jun 22, 2022 at 02:20:27PM +0800, Peng Fan (OSS) wrote:
> >>> From: Peng Fan <[email protected]>
> >>>
> >>> Commit 71066545b48e4("driver core: Set fw_devlink.strict=1 by default")
> >>> default set fw_devlink to true. This has a side effect to i.MX uart
> >>> console. The sdma will make the i.MX8MP uart driver defer probe for some
> >>> time (~10s with i.MX8MP-EVK board) until sdma ready, because sdma is a
> >>> supplier with property dmas set in device tree node.
> >> I just tested this on an i.MX6 board and observed the same behaviour.
> >> The same will happen on any other i.MX board as well. This will also
> >> likely happen on any other SoC on which the UART driver uses dmaengine.
> >>
> >>> Since this uart is for console, we need log printed out as soon as
> >>> possible, so remove the dmas property for the uart console node.
> >> Fixing this at board level is not really an option because that means
> >> fixing each and every, at least i.MX board in the tree. Furthermore
> >> this would mean to bring the deleted property back in and to remove
> >> another dmas property should a user want to switch to another console
> >> port.
> >>
> >> For what it's worth: NACK for 71066545b48e4.
> > Hi Peng/Sascha/Fabio,
> >
> > I was looking at a fix for this (even when stdout-path isn't set in
> > DT) and looking at the older code (say, 5.18), and it looks like
> > before my recent changes the console would still only get probed
> > during late_initcall(). Does that match your experience?
>
> I could not get you. In driver, it is module init:
> module_init(imx_uart_init), not late_initcall.
>

Correct, the driver does use module_init(). However, because of how
driver_deferred_probe_check_state() is implemented in 5.18, the
console would get deferred probe until late_initcall(). In my earlier
email, I'm trying to get you to confirm my analysis is what you were
seeing in 5.18 (or 5.19-rcX without my series that deletes
driver_deferred_probe_check_state()).

-Saravana