diff mbox series

[U-Boot,v1,1/2] pinctrl: nxp: mx6: DM_FLAG_PRE_RELOC by default

Message ID 20190715141947.20689-2-igor.opaniuk@gmail.com
State Superseded, archived
Delegated to: Stefano Babic
Headers show
Series imx6ull: Fix missing initial output from UART | expand

Commit Message

Igor Opaniuk July 15, 2019, 2:19 p.m. UTC 
From: Igor Opaniuk <igor.opaniuk@toradex.com>

For Colibri iMX6ULL we have to set pinmux configuration ASAP (ideally
before relocation) to get serial console working. Without this we miss
almost the half of output (U-boot version, CPU defails, Reset cause,
DRAM details etc.).

To achieve this we need to force pinctrl-mx6 to get probed before
relocation. Setting pinmux configuration for UART before was done
in board_early_init_f().

Fixes: cd69e8ef9b ("colibri-imx6ull: migrate pinctrl and regulators to dtb/dm")
Signed-off-by: Igor Opaniuk <igor.opaniuk@toradex.com>
---

 drivers/pinctrl/nxp/pinctrl-imx6.c | 2 --
 1 file changed, 2 deletions(-)

Comments

Fabio Estevam July 15, 2019, 7:10 p.m. UTC | #1 
Hi Igor,

On Mon, Jul 15, 2019 at 11:20 AM Igor Opaniuk <igor.opaniuk@gmail.com> wrote:
>
> From: Igor Opaniuk <igor.opaniuk@toradex.com>
>
> For Colibri iMX6ULL we have to set pinmux configuration ASAP (ideally
> before relocation) to get serial console working. Without this we miss
> almost the half of output (U-boot version, CPU defails, Reset cause,
> DRAM details etc.).
>
> To achieve this we need to force pinctrl-mx6 to get probed before
> relocation. Setting pinmux configuration for UART before was done
> in board_early_init_f().
>
> Fixes: cd69e8ef9b ("colibri-imx6ull: migrate pinctrl and regulators to dtb/dm")
> Signed-off-by: Igor Opaniuk <igor.opaniuk@toradex.com>
> ---
>
>  drivers/pinctrl/nxp/pinctrl-imx6.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/pinctrl/nxp/pinctrl-imx6.c b/drivers/pinctrl/nxp/pinctrl-imx6.c
> index 0c1e7a9c05..aafa3057ad 100644
> --- a/drivers/pinctrl/nxp/pinctrl-imx6.c
> +++ b/drivers/pinctrl/nxp/pinctrl-imx6.c
> @@ -49,7 +49,5 @@ U_BOOT_DRIVER(imx6_pinctrl) = {
>         .remove = imx_pinctrl_remove,
>         .priv_auto_alloc_size = sizeof(struct imx_pinctrl_priv),
>         .ops = &imx_pinctrl_ops,
> -#if !CONFIG_IS_ENABLED(OF_CONTROL)
>         .flags = DM_FLAG_PRE_RELOC,
> -#endif

It seems that this is not i.MX6 specific (maybe not even i.MX
specific. I noticed this issue on i.MX7 too).

Could this be handled in the drivers/pinctrl core instead?

Just wanted to see if we can get a more generic solution for this issue.

Thanks
Igor Opaniuk July 16, 2019, 12:44 p.m. UTC | #2 
Hi Fabio,

On Mon, Jul 15, 2019 at 10:10 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Igor,
>
> On Mon, Jul 15, 2019 at 11:20 AM Igor Opaniuk <igor.opaniuk@gmail.com> wrote:
> >
> > From: Igor Opaniuk <igor.opaniuk@toradex.com>
> >
> > For Colibri iMX6ULL we have to set pinmux configuration ASAP (ideally
> > before relocation) to get serial console working. Without this we miss
> > almost the half of output (U-boot version, CPU defails, Reset cause,
> > DRAM details etc.).
> >
> > To achieve this we need to force pinctrl-mx6 to get probed before
> > relocation. Setting pinmux configuration for UART before was done
> > in board_early_init_f().
> >
> > Fixes: cd69e8ef9b ("colibri-imx6ull: migrate pinctrl and regulators to dtb/dm")
> > Signed-off-by: Igor Opaniuk <igor.opaniuk@toradex.com>
> > ---
> >
> >  drivers/pinctrl/nxp/pinctrl-imx6.c | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/drivers/pinctrl/nxp/pinctrl-imx6.c b/drivers/pinctrl/nxp/pinctrl-imx6.c
> > index 0c1e7a9c05..aafa3057ad 100644
> > --- a/drivers/pinctrl/nxp/pinctrl-imx6.c
> > +++ b/drivers/pinctrl/nxp/pinctrl-imx6.c
> > @@ -49,7 +49,5 @@ U_BOOT_DRIVER(imx6_pinctrl) = {
> >         .remove = imx_pinctrl_remove,
> >         .priv_auto_alloc_size = sizeof(struct imx_pinctrl_priv),
> >         .ops = &imx_pinctrl_ops,
> > -#if !CONFIG_IS_ENABLED(OF_CONTROL)
> >         .flags = DM_FLAG_PRE_RELOC,
> > -#endif
>
> It seems that this is not i.MX6 specific (maybe not even i.MX
> specific. I noticed this issue on i.MX7 too).

Right. Currently there are no any output issues on
Colibri iMX7 because board_early_init_f() does apply initial
pinmux configuration for uart [1].

>
> Could this be handled in the drivers/pinctrl core instead?
>
> Just wanted to see if we can get a more generic solution for this issue.
>
> Thanks

AFAIK, DM_FLAG_PRE_RELOC flag can be set for a driver,
not for the whole uclass (but I might be mistaken). Also taking
into account memory limitations (small malloc pool etc) before
U-boot is relocated, I would force driver to be bounded only
in the cases when it's really vital (like this particular case
with serial console issue).

Thanks!

[1] board/toradex/colibri_imx7/colibri_imx7.c

--
Best regards - Freundliche Grüsse - Meilleures salutations

Igor Opaniuk

mailto: igor.opaniuk@gmail.com
skype: igor.opanyuk
+380 (93) 836 40 67
http://ua.linkedin.com/in/iopaniuk
Igor Opaniuk July 24, 2019, 1:58 p.m. UTC | #3 
Hi Fabio,

On Tue, Jul 16, 2019 at 3:44 PM Igor Opaniuk <igor.opaniuk@gmail.com> wrote:
>
> Hi Fabio,
>
> On Mon, Jul 15, 2019 at 10:10 PM Fabio Estevam <festevam@gmail.com> wrote:
> >
> > Hi Igor,
> >
> > On Mon, Jul 15, 2019 at 11:20 AM Igor Opaniuk <igor.opaniuk@gmail.com> wrote:
> > >
> > > From: Igor Opaniuk <igor.opaniuk@toradex.com>
> > >
> > > For Colibri iMX6ULL we have to set pinmux configuration ASAP (ideally
> > > before relocation) to get serial console working. Without this we miss
> > > almost the half of output (U-boot version, CPU defails, Reset cause,
> > > DRAM details etc.).
> > >
> > > To achieve this we need to force pinctrl-mx6 to get probed before
> > > relocation. Setting pinmux configuration for UART before was done
> > > in board_early_init_f().
> > >
> > > Fixes: cd69e8ef9b ("colibri-imx6ull: migrate pinctrl and regulators to dtb/dm")
> > > Signed-off-by: Igor Opaniuk <igor.opaniuk@toradex.com>
> > > ---
> > >
> > >  drivers/pinctrl/nxp/pinctrl-imx6.c | 2 --
> > >  1 file changed, 2 deletions(-)
> > >
> > > diff --git a/drivers/pinctrl/nxp/pinctrl-imx6.c b/drivers/pinctrl/nxp/pinctrl-imx6.c
> > > index 0c1e7a9c05..aafa3057ad 100644
> > > --- a/drivers/pinctrl/nxp/pinctrl-imx6.c
> > > +++ b/drivers/pinctrl/nxp/pinctrl-imx6.c
> > > @@ -49,7 +49,5 @@ U_BOOT_DRIVER(imx6_pinctrl) = {
> > >         .remove = imx_pinctrl_remove,
> > >         .priv_auto_alloc_size = sizeof(struct imx_pinctrl_priv),
> > >         .ops = &imx_pinctrl_ops,
> > > -#if !CONFIG_IS_ENABLED(OF_CONTROL)
> > >         .flags = DM_FLAG_PRE_RELOC,
> > > -#endif
> >
> > It seems that this is not i.MX6 specific (maybe not even i.MX
> > specific. I noticed this issue on i.MX7 too).
>
> Right. Currently there are no any output issues on
> Colibri iMX7 because board_early_init_f() does apply initial
> pinmux configuration for uart [1].
>
> >
> > Could this be handled in the drivers/pinctrl core instead?
> >
> > Just wanted to see if we can get a more generic solution for this issue.
> >
> > Thanks
>
> AFAIK, DM_FLAG_PRE_RELOC flag can be set for a driver,
> not for the whole uclass (but I might be mistaken). Also taking
> into account memory limitations (small malloc pool etc) before
> U-boot is relocated, I would force driver to be bounded only
> in the cases when it's really vital (like this particular case
> with serial console issue).
I'm currently in the middle of preparing v2, just wanted to know if you do agree
with this point and in general have any additional suggestions regarding this?

>
> Thanks!
>
> [1] board/toradex/colibri_imx7/colibri_imx7.c
>
> --
> Best regards - Freundliche Grüsse - Meilleures salutations
>
> Igor Opaniuk
>
> mailto: igor.opaniuk@gmail.com
> skype: igor.opanyuk
> +380 (93) 836 40 67
> http://ua.linkedin.com/in/iopaniuk

Thanks
Fabio Estevam July 24, 2019, 2:06 p.m. UTC | #4 
Hi Igor,

On Wed, Jul 24, 2019 at 10:58 AM Igor Opaniuk <igor.opaniuk@toradex.com> wrote:

> I'm currently in the middle of preparing v2, just wanted to know if you do agree
> with this point and in general have any additional suggestions regarding this?

My only suggestion is to do a single patch that touches all the
drivers/pinctrl/nxp/ drivers, not ony the mx6 one.

Thanks
diff mbox series

Patch

diff --git a/drivers/pinctrl/nxp/pinctrl-imx6.c b/drivers/pinctrl/nxp/pinctrl-imx6.c
index 0c1e7a9c05..aafa3057ad 100644
--- a/drivers/pinctrl/nxp/pinctrl-imx6.c
+++ b/drivers/pinctrl/nxp/pinctrl-imx6.c
@@ -49,7 +49,5 @@  U_BOOT_DRIVER(imx6_pinctrl) = {
 	.remove = imx_pinctrl_remove,
 	.priv_auto_alloc_size = sizeof(struct imx_pinctrl_priv),
 	.ops = &imx_pinctrl_ops,
-#if !CONFIG_IS_ENABLED(OF_CONTROL)
 	.flags = DM_FLAG_PRE_RELOC,
-#endif
 };