On Mon, 13 Feb 2023, Jiri Slaby wrote:
> On 13. 02. 23, 10:22, Ilpo Järvinen wrote:
> > On Mon, 13 Feb 2023, Jiri Slaby wrote:
> >
> > > On 13. 02. 23, 10:10, Jiri Slaby wrote:
> > > > On 13. 02. 23, 9:53, Biju Das wrote:
> > > > > > > +static void serial8250_rzv2m_reg_update(struct uart_port *p, int
> > > > > > > off,
> > > > > > > +int value) {
> > > > > > > + unsigned int ier, fcr, lcr, mcr, hcr0;
> > > > > > > +
> > > > > > > + ier = serial8250_em_serial_in(p, UART_IER);
> > > > > > > + lcr = serial8250_em_serial_in(p, UART_LCR);
> > > > > > > + mcr = serial8250_em_serial_in(p, UART_MCR);
> > > > > > > + hcr0 = serial8250_em_serial_in(p, UART_HCR0);
> > > > > > > + /*
> > > > > > > + * The value of UART_IIR and UART_FCR are 2, but
> > > > > > > corresponding
> > > > > > > + * RZ/V2M address offset are different(0x08 and 0x0c). So we
> > > > > > > need
> > > > > > > to
> > > > > > > + * use readl() here.
> > > > > > > + */
> > > > > > > + fcr = readl(p->membase + ((UART_FCR + 1) << 2));
> > > > > >
> > > > > > I don't get the meaning of that comment. It doesn't seem to match
> > > > > > what
> > > > > > your
> > > > > > code does as the code seemingly has nothing to do with IIR (and none
> > > > > > of
> > > > > > you
> > > > > > changelogs refer to IIR either)?
> > > > >
> > > > > The generic macro UART_IIR and UART_FCR in linux/serial_reg.h has a
> > > > > value
> > > > > of 2.
> > > >
> > > > Sure, IIR is normally WO and FCR RO and share the same register. I would
> > > > simply define UART_FCR_RZ (or alike)
> > >
> > > Or even UART_FCR_RO_RZ?
> > >
> > > > for 0x12.
> > >
> > > I mean 12 or 0xc.
> >
> > Won't that collide with LCR reads then? They are currently mapped by
> > return 0; but after adding a case for UART_FCR_RO_RZ they'll read from
> > what is the FCR register on this HW?
>
> LCR is WO, right? But maybe I'm confused by this really weird HW design?
LCR is not WO and it is read during autoconfig too:
8250_port.c=static int size_fifo(struct uart_8250_port *up)
8250_port.c: old_lcr = serial_in(up, UART_LCR);
8250_port.c=static unsigned int autoconfig_read_divisor_id(struct uart_8250_port *p)
8250_port.c: old_lcr = serial_in(p, UART_LCR);
8250_port.c=static void autoconfig(struct uart_8250_port *up)
8250_port.c: save_lcr = serial_in(up, UART_LCR);
Although the only thing the value is used for is to write it back to LCR.
--
i.
