Re: [PATCH] serial: msm: Configure correct working mode before starting earlycon

[Stephan Gerhold <stephan.gerhold@linaro.org>]

On Wed, Apr 09, 2025 at 11:11:00AM -0700, Stephen Boyd wrote:
> Quoting Stephan Gerhold (2025-04-08 10:22:47)
> > diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
> > index 1b137e06844425584afe5d3f647e9537c6e2d658..3449945493ceb42369d2acafca925350fccc4f82 100644
> > --- a/drivers/tty/serial/msm_serial.c
> > +++ b/drivers/tty/serial/msm_serial.c
> > @@ -1746,6 +1746,12 @@ msm_serial_early_console_setup_dm(struct earlycon_device *device,
> >         if (!device->port.membase)
> >                 return -ENODEV;
> >  
> > +       /* Disable DM / single-character modes */
> > +       msm_write(&device->port, 0, UARTDM_DMEN);
> > +       msm_write(&device->port, MSM_UART_CR_CMD_RESET_RX, MSM_UART_CR);
> > +       msm_write(&device->port, MSM_UART_CR_CMD_RESET_TX, MSM_UART_CR);
> > +       msm_write(&device->port, MSM_UART_CR_TX_ENABLE, MSM_UART_CR);
> 
> In msm_complete_tx_dma() these are under an if condition checking the
> version of uartdm. Do we need that here? Although I also see that
> MSM_UART_CR_CMD_RESET_TX is unconditionally written in msm_reset() but
> not MSM_UART_CR_TX_ENABLE so maybe the condition check is wrong or the
> bit doesn't exist in earlier versions of the hardware so it doesn't
> really matter.

msm_reset() is called from msm_set_baud_rate(), and that one does

  msm_write(port, MSM_UART_CR_TX_ENABLE | MSM_UART_CR_RX_ENABLE, MSM_UART_CR);

unconditionally immediately after, so what I'm doing here matches what
the driver anyway does for all the IP versions.

I'm not sure why we have version checks to perform the reset/enable in
*some* code paths only for *some* IP versions. All of this feels
obsolete at this point, since "msm_port->is_uartdm > UARTDM_1P3" covers
all SoCs we still support upstream. There are no users of the v1.1 and
v1.2 compatibles upstream, even MSM8660 has v1.3 already. We might as
well drop those conditions at some point.

Even better, we still have code for some super old controller before all
the DM variants (qcom,msm-uart instead of qcom,msm-uart*dm*). No users
of that upstream either. :-)

Thanks,
Stephan