diff mbox series

arm64: dts: imx8mp: drop dmas property for uart console

Message ID 20220622062027.994614-1-peng.fan@oss.nxp.com (mailing list archive)
State New, archived
Headers show
Series arm64: dts: imx8mp: drop dmas property for uart console | expand

Commit Message

Peng Fan (OSS) June 22, 2022, 6:20 a.m. UTC
From: Peng Fan <peng.fan@nxp.com>

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 <peng.fan@nxp.com>
---

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(+)

Comments

Sascha Hauer June 22, 2022, 7:24 a.m. UTC | #1
+Cc Saravana Kannan <saravanak@google.com>, the author of 71066545b48e4

On Wed, Jun 22, 2022 at 02:20:27PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> 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 <peng.fan@nxp.com>
> ---
> 
> 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
> 
>
Saravana Kannan June 22, 2022, 10:31 p.m. UTC | #2
On Wed, Jun 22, 2022 at 12:24 AM Sascha Hauer <s.hauer@pengutronix.de> wrote:
>
> +Cc Saravana Kannan <saravanak@google.com>, the author of 71066545b48e4
>
> On Wed, Jun 22, 2022 at 02:20:27PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > 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/20220622215912.550419-1-saravanak@google.com/

-Saravana

>
> Sascha
>
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >
> > 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 |
Peng Fan June 22, 2022, 10:33 p.m. UTC | #3
> Subject: Re: [PATCH] arm64: dts: imx8mp: drop dmas property for uart console
> 
> On Wed, Jun 22, 2022 at 12:24 AM Sascha Hauer <s.hauer@pengutronix.de>
> wrote:
> >
> > +Cc Saravana Kannan <saravanak@google.com>, the author of
> > +71066545b48e4
> >
> > On Wed, Jun 22, 2022 at 02:20:27PM +0800, Peng Fan (OSS) wrote:
> > > From: Peng Fan <peng.fan@nxp.com>
> > >
> > > 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 <peng.fan@nxp.com>
> > > ---
> > >
> > > 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 |
Fabio Estevam June 23, 2022, 11:35 a.m. UTC | #4
Hi Saravana,

On Wed, Jun 22, 2022 at 7:31 PM Saravana Kannan <saravanak@google.com> wrote:

> Peng, I sent a fix. Can you give it a shot please?
>
> https://lore.kernel.org/lkml/20220622215912.550419-1-saravanak@google.com/

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
Fabio Estevam June 23, 2022, 6:20 p.m. UTC | #5
On Thu, Jun 23, 2022 at 8:35 AM Fabio Estevam <festevam@gmail.com> 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.
Saravana Kannan June 23, 2022, 6:25 p.m. UTC | #6
On Thu, Jun 23, 2022 at 11:20 AM Fabio Estevam <festevam@gmail.com> wrote:
>
> On Thu, Jun 23, 2022 at 8:35 AM Fabio Estevam <festevam@gmail.com> 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
Fabio Estevam June 23, 2022, 6:43 p.m. UTC | #7
On Thu, Jun 23, 2022 at 3:26 PM Saravana Kannan <saravanak@google.com> 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
Saravana Kannan June 23, 2022, 6:58 p.m. UTC | #8
On Thu, Jun 23, 2022 at 11:43 AM Fabio Estevam <festevam@gmail.com> wrote:
>
> On Thu, Jun 23, 2022 at 3:26 PM Saravana Kannan <saravanak@google.com> 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
Fabio Estevam June 23, 2022, 8:51 p.m. UTC | #9
On Thu, Jun 23, 2022 at 3:58 PM Saravana Kannan <saravanak@google.com> 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.
Saravana Kannan June 23, 2022, 9:28 p.m. UTC | #10
On Thu, Jun 23, 2022 at 1:51 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> On Thu, Jun 23, 2022 at 3:58 PM Saravana Kannan <saravanak@google.com> 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
Saravana Kannan June 25, 2022, 7:15 a.m. UTC | #11
On Wed, Jun 22, 2022 at 12:24 AM Sascha Hauer <s.hauer@pengutronix.de> wrote:
>
> +Cc Saravana Kannan <saravanak@google.com>, the author of 71066545b48e4
>
> On Wed, Jun 22, 2022 at 02:20:27PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > 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 <peng.fan@nxp.com>
> > ---
> >
> > 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 |
Peng Fan (OSS) June 27, 2022, 8:22 a.m. UTC | #12
Hi Saravana,

在 2022/6/25 15:15, Saravana Kannan 写道:
> On Wed, Jun 22, 2022 at 12:24 AM Sascha Hauer <s.hauer@pengutronix.de> wrote:
>> +Cc Saravana Kannan <saravanak@google.com>, the author of 71066545b48e4
>>
>> On Wed, Jun 22, 2022 at 02:20:27PM +0800, Peng Fan (OSS) wrote:
>>> From: Peng Fan <peng.fan@nxp.com>
>>>
>>> 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 <peng.fan@nxp.com>
>>> ---
>>>
>>> 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 |
Saravana Kannan June 28, 2022, 3 a.m. UTC | #13
On Mon, Jun 27, 2022 at 1:22 AM Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote:
>
> Hi Saravana,
>
> 在 2022/6/25 15:15, Saravana Kannan 写道:
> > On Wed, Jun 22, 2022 at 12:24 AM Sascha Hauer <s.hauer@pengutronix.de> wrote:
> >> +Cc Saravana Kannan <saravanak@google.com>, the author of 71066545b48e4
> >>
> >> On Wed, Jun 22, 2022 at 02:20:27PM +0800, Peng Fan (OSS) wrote:
> >>> From: Peng Fan <peng.fan@nxp.com>
> >>>
> >>> 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
diff mbox series

Patch

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 {