Hi Geert,
Thanks for the feedback.
> Subject: Re: [PATCH v2 3/3] serial: 8250_em: Add serial_out() to struct
> serial8250_em_hw_info
>
> Hi Biju,
>
> On Fri, Feb 10, 2023 at 5:01 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > > On Fri, 10 Feb 2023, Biju Das wrote:
> > > > As per HW manual section 40.6.1, we need to perform FIFO reset +
> > > > SW reset before updating the below registers.
> > > >
> > > > FCR[7:5], FCR[3:0], LCR[7][5:0], MCR[6:4], DLL[7:0], DLM[7:0] and
> > > > HCR0[6:5][3:2].
> > > >
> > > > This patch adds serial_out() to struct serial8250_em_hw_info to
> > > > handle this difference between emma mobile and rz/v2m.
> > > >
> > > > DLL/DLM register can be updated only by setting LCR[7]. So the
> > > > updation of LCR[7] will perform reset for DLL/DLM register changes.
> > > >
> > > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
>
> > > > --- a/drivers/tty/serial/8250/8250_em.c
> > > > +++ b/drivers/tty/serial/8250/8250_em.c
> > > > @@ -31,6 +35,40 @@ struct serial8250_em_priv {
> > > > const struct serial8250_em_hw_info *info; };
> > > >
> > > > +static void serial8250_rzv2m_reg_update(struct uart_port *p, int
> > > > +off, int value) {
> > > > + unsigned int ier, fcr, lcr, mcr, hcr0;
> > > > +
> > > > + ier = readl(p->membase + (UART_IER << 2));
> > > > + hcr0 = readl(p->membase + (UART_HCR0 << 2));
> > > > + fcr = readl(p->membase + ((UART_FCR + 1) << 2));
> > > > + lcr = readl(p->membase + ((UART_LCR + 1) << 2));
> > > > + mcr = readl(p->membase + ((UART_MCR + 1) << 2));
> > > > +
> > > > + writel(fcr | UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT,
> > > > + p->membase + ((UART_FCR + 1) << 2));
> > > > + writel(hcr0 | UART_HCR0_SW_RESET, p->membase + (UART_HCR0 << 2));
> > > > + writel(hcr0 & ~UART_HCR0_SW_RESET, p->membase + (UART_HCR0 <<
> > > > + 2));
> > > > +
> > > > + switch (off) {
> > > > + case UART_FCR:
> > > > + fcr = value;
> > > > + break;
> > > > + case UART_LCR:
> > > > + lcr = value;
> > > > + break;
> > > > + case UART_MCR:
> > > > + mcr = value;
> > > > + break;
> > > > + }
> > > > +
> > > > + writel(ier, p->membase + (UART_IER << 2));
> > > > + writel(fcr, p->membase + ((UART_FCR + 1) << 2));
> > > > + writel(mcr, p->membase + ((UART_MCR + 1) << 2));
> > > > + writel(lcr, p->membase + ((UART_LCR + 1) << 2));
> > > > + writel(hcr0, p->membase + (UART_HCR0 << 2));
> > >
> > > Perhaps it would make sense to instead of using readl/writel()
> > > directly to call serial8250_em_serial_in/out() so all the offset
> > > trickery wouldn't need to be duplicated inside this function?
> >
> > HCR0 register is not available for emma mobile. Is it ok if I just do
> > readl/writel for that register and rest will use
> serial8250_em_serial_in/out()??
>
> According to R19UH0040EJ0400 Rev.4.00 it is available on EMEV2, and the
> layout looks identical to RZ/V2M.
OK, will add HCR0 as well. Now only issue is related to fcr as
the value of UART_FCR and UART_IIR are 2. But their address offsets
are at 0x8 and 0xc. So I need to use readl(p->membase + ((UART_FCR + 1) << 2));
for reading FCR and I will add a comment above it.
/*
* The value of UART_IIR and UART_FCR are 2, but the corresponding
* RZ/V2M address offset are different(0x08 and 0x0c). So, we need to use
* readl() here.
*/
Cheers,
Biju
