[1/2] serial: mxc: support DTE mode

Add DTE mode support via Kconfig on the MXC uart.

Signed-off-by: Ryan Harkin <ryan.harkin@linaro.org>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/serial/Kconfig      |  7 +++++++
 drivers/serial/serial_mxc.c | 10 ++++++++--
 2 files changed, 15 insertions(+), 2 deletions(-)

Hi Ryan,


On 19.01.2018 10:53, Ryan Harkin wrote:
> Add DTE mode support via Kconfig on the MXC uart.

Make use of the driver model, there DTE is supported already today:
https://lists.denx.de/pipermail/u-boot/2016-July/259573.html

--
Stefan

>
> Signed-off-by: Ryan Harkin <ryan.harkin@linaro.org>
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  drivers/serial/Kconfig      |  7 +++++++
>  drivers/serial/serial_mxc.c | 10 ++++++++--
>  2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> index 122b8e7..0df57c0 100644
> --- a/drivers/serial/Kconfig
> +++ b/drivers/serial/Kconfig
> @@ -597,4 +597,11 @@ config SYS_SDMR
>  	depends on MPC8XX_CONS
>  	default 0
>  
> +config SERIAL_MXC_DTE_MODE
> +	bool "Use DTE mode for the MXC UART"
> +	default n
> +	help
> +	  This is used to set DTE mode on the serial console controlled by
> +	  serial_mxc.c.
> +
>  endmenu
> diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c
> index cce80a8..e7ea30c 100644
> --- a/drivers/serial/serial_mxc.c
> +++ b/drivers/serial/serial_mxc.c
> @@ -111,6 +111,12 @@
>  #define TXTL		2  /* reset default */
>  #define RXTL		1  /* reset default */
>  
> +#ifdef CONFIG_SERIAL_MXC_DTE_MODE
> +#define MXC_DTE_MODE	true
> +#else
> +#define MXC_DTE_MODE	false
> +#endif
> +
>  DECLARE_GLOBAL_DATA_PTR;
>  
>  struct mxc_uart {
> @@ -189,7 +195,7 @@ static void mxc_serial_setbrg(void)
>  	if (!gd->baudrate)
>  		gd->baudrate = CONFIG_BAUDRATE;
>  
> -	_mxc_serial_setbrg(mxc_base, clk, gd->baudrate, false);
> +	_mxc_serial_setbrg(mxc_base, clk, gd->baudrate, MXC_DTE_MODE);
>  }
>  
>  static int mxc_serial_getc(void)
> @@ -367,7 +373,7 @@ static inline void _debug_uart_init(void)
>  
>  	_mxc_serial_init(base);
>  	_mxc_serial_setbrg(base, CONFIG_DEBUG_UART_CLOCK,
> -			   CONFIG_BAUDRATE, false);
> +			   CONFIG_BAUDRATE, MXC_DTE_MODE);
>  }
>  
>  static inline void _debug_uart_putc(int ch)

Hi Stefan,

Thanks for looking so quickly.

On 19 January 2018 at 12:23, Stefan Agner <stefan.agner@toradex.com> wrote:

> Hi Ryan,
>
>
> On 19.01.2018 10:53, Ryan Harkin wrote:
> > Add DTE mode support via Kconfig on the MXC uart.
>
> Make use of the driver model, there DTE is supported already today:
> https://lists.denx.de/pipermail/u-boot/2016-July/259573.html


My change would be useful for other non-DM users of serial_mxc.c, of
course. Not just WaRP7.

I don't have any objection to WaRP7 moving to DM, although that isn't my
call, but moving using the driver model is not a straight-forward change,
is it? WaRP7 today doesn't use it.

Do you have an example of a board using this driver that switched using the
driver model? I'd like to see the scale of the changes needed.

Regards,
Ryan.



>
> --
> Stefan
>
> >
> > Signed-off-by: Ryan Harkin <ryan.harkin@linaro.org>
> > Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> > ---
> >  drivers/serial/Kconfig      |  7 +++++++
> >  drivers/serial/serial_mxc.c | 10 ++++++++--
> >  2 files changed, 15 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> > index 122b8e7..0df57c0 100644
> > --- a/drivers/serial/Kconfig
> > +++ b/drivers/serial/Kconfig
> > @@ -597,4 +597,11 @@ config SYS_SDMR
> >       depends on MPC8XX_CONS
> >       default 0
> >
> > +config SERIAL_MXC_DTE_MODE
> > +     bool "Use DTE mode for the MXC UART"
> > +     default n
> > +     help
> > +       This is used to set DTE mode on the serial console controlled by
> > +       serial_mxc.c.
> > +
> >  endmenu
> > diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c
> > index cce80a8..e7ea30c 100644
> > --- a/drivers/serial/serial_mxc.c
> > +++ b/drivers/serial/serial_mxc.c
> > @@ -111,6 +111,12 @@
> >  #define TXTL         2  /* reset default */
> >  #define RXTL         1  /* reset default */
> >
> > +#ifdef CONFIG_SERIAL_MXC_DTE_MODE
> > +#define MXC_DTE_MODE true
> > +#else
> > +#define MXC_DTE_MODE false
> > +#endif
> > +
> >  DECLARE_GLOBAL_DATA_PTR;
> >
> >  struct mxc_uart {
> > @@ -189,7 +195,7 @@ static void mxc_serial_setbrg(void)
> >       if (!gd->baudrate)
> >               gd->baudrate = CONFIG_BAUDRATE;
> >
> > -     _mxc_serial_setbrg(mxc_base, clk, gd->baudrate, false);
> > +     _mxc_serial_setbrg(mxc_base, clk, gd->baudrate, MXC_DTE_MODE);
> >  }
> >
> >  static int mxc_serial_getc(void)
> > @@ -367,7 +373,7 @@ static inline void _debug_uart_init(void)
> >
> >       _mxc_serial_init(base);
> >       _mxc_serial_setbrg(base, CONFIG_DEBUG_UART_CLOCK,
> > -                        CONFIG_BAUDRATE, false);
> > +                        CONFIG_BAUDRATE, MXC_DTE_MODE);
> >  }
> >
> >  static inline void _debug_uart_putc(int ch)
>
>

Hi Ryan,

On 19 January 2018 at 06:21, Ryan Harkin <ryan.harkin@linaro.org> wrote:
> Hi Stefan,
>
> Thanks for looking so quickly.
>
> On 19 January 2018 at 12:23, Stefan Agner <stefan.agner@toradex.com> wrote:
>>
>> Hi Ryan,
>>
>>
>> On 19.01.2018 10:53, Ryan Harkin wrote:
>> > Add DTE mode support via Kconfig on the MXC uart.
>>
>> Make use of the driver model, there DTE is supported already today:
>> https://lists.denx.de/pipermail/u-boot/2016-July/259573.html
>
>
> My change would be useful for other non-DM users of serial_mxc.c, of course.
> Not just WaRP7.
>
> I don't have any objection to WaRP7 moving to DM, although that isn't my
> call, but moving using the driver model is not a straight-forward change, is
> it? WaRP7 today doesn't use it.

We are planning to require that board use CONFIG_BLK fairly soon, and
that likely means conversion to device tree I don't think it makes
sense to accept patches like this. If the board can be converted, then
let's do it!

>
> Do you have an example of a board using this driver that switched using the
> driver model? I'd like to see the scale of the changes needed.

It probably requires:

- Adding a DT (with u-boot,dm-pre-reloc as needed)
- Checking that stdio-path is correct

Regards,
Simon

Hi Simon,

On 22 January 2018 at 00:29, Simon Glass <sjg@chromium.org> wrote:

> Hi Ryan,
>
> On 19 January 2018 at 06:21, Ryan Harkin <ryan.harkin@linaro.org> wrote:
> > Hi Stefan,
> >
> > Thanks for looking so quickly.
> >
> > On 19 January 2018 at 12:23, Stefan Agner <stefan.agner@toradex.com>
> wrote:
> >>
> >> Hi Ryan,
> >>
> >>
> >> On 19.01.2018 10:53, Ryan Harkin wrote:
> >> > Add DTE mode support via Kconfig on the MXC uart.
> >>
> >> Make use of the driver model, there DTE is supported already today:
> >> https://lists.denx.de/pipermail/u-boot/2016-July/259573.html
> >
> >
> > My change would be useful for other non-DM users of serial_mxc.c, of
> course.
> > Not just WaRP7.
> >
> > I don't have any objection to WaRP7 moving to DM, although that isn't my
> > call, but moving using the driver model is not a straight-forward
> change, is
> > it? WaRP7 today doesn't use it.
>
> We are planning to require that board use CONFIG_BLK fairly soon, and
> that likely means conversion to device tree I don't think it makes
> sense to accept patches like this. If the board can be converted, then
> let's do it!
>

I'm not the maintainer of this board. I'm only making this patch so I can
put it into our test farm. But I'm interested in giving it a go. In fact, I
started already after Stefan's email. And bricked my board :-)


> >
> > Do you have an example of a board using this driver that switched using
> the
> > driver model? I'd like to see the scale of the changes needed.
>
> It probably requires:
>
> - Adding a DT (with u-boot,dm-pre-reloc as needed)
>

I take it we add the DT from the upstream linux kernel? The upstream DT
doesn't define UART6, the one I want to use. I have a patch for the kernel
that I have not attempted to send upstream yet.

What approach should I take?
- upstream my patch to the kernel first
- use the DT from upstream kernel as-is and add a separate patch in the
u-boot tree
- use the DT from upstream kernel as-is and squash in my patch

Or something else?

Updating the DT from upstream will possibly mean updating the DTs for all
other iMX7 boards [1], because the include/dt-bindings stuff has changed
slightly, as well as the imx7s.dtsi file. I have no way of testing the
other boards, but I guess their maintainers can help there.


- Checking that stdio-path is correct
>

That's probably what bricked my board...

Cheers,
Ryan.

[1] There only appear to be two iMX7 board in u-boot already:
arch/arm/dts/imx7d.dtsi:44:#include "imx7s.dtsi"
arch/arm/dts/imx7-colibri.dts:9:#include "imx7d.dtsi"
arch/arm/dts/imx7d-sdb.dts:9:#include "imx7d.dtsi"



> Regards,
> Simon
>