Hi Andy Shevchenko,
Thanks for the feedback.
> Subject: Re: [PATCH 1/3] serial: 8250: Identify Renesas RZ/V2M 16750 UART
>
> On Fri, Feb 10, 2023 at 07:14:54AM +0000, Biju Das wrote:
> > > Subject: Re: [PATCH 1/3] serial: 8250: Identify Renesas RZ/V2M 16750
> > > UART On Thu, Feb 09, 2023 at 02:28:55PM +0000, Biju Das wrote:
> > > > > From: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
> > > > > Sent: Thursday, February 9, 2023 2:09 PM On Thu, 9 Feb 2023,
> > > > > Biju Das wrote:
>
> ...
>
> > > > > > + [PORT_16750] = {
> > > > > > + .name = "Renesas RZ/V2M 16750",
> > > > > > + .fifo_size = 64,
> > > > > > + .tx_loadsz = 64,
> > > > > > + .fcr = UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10 |
> > > > > > + UART_FCR7_64BYTE,
> > > > > > + .rxtrig_bytes = {1, 16, 32, 56},
> > > > > > + .flags = UART_CAP_FIFO | UART_CAP_AFE,
> > > > > > + },
> > > > >
> > > > > Eh, how can you reuse [PORT_16750] again in the initializer like
> that?
> > > >
> > > > Oops. Missed it. Is it ok to introduce PORT_RENESAS_16750_F64
> > > > instead as PORT_16750 is used by TI16750?
> > >
> > > What the difference to the 16750 from TI that prevents you from using
> it?
> >
> > Mostly it is identical.
> >
> > The main difference is detection method, and we don't have UART_IERX_SLEEP
> bit in IER.
> >
> > On TI, it sets bit 5 of the IIR when 64-byte FIFO mode is enabled when
> DLAB is set.
> >
> > Whereas in our case DLAB does n't have any role for Identification,
> >
> > It set bit 5 of the IIR when 64 byte FIFO mode is enabled.
> > and it clears bit 5 of the IIR when 64 byte FIFO mode is disabled.
>
> So the question here is do these minor deviations affect the actual
> functionality?
OK.
>
> Note, on Intel hardware we use directly TI16750 while we have no sleep
> functionality available IIRC. Ilpo may correct me if I'm wrong.
>
> > Other than that, when I use PORT_16750 type and capabilities in
> > 8250_em driver and add identification method for Renesas UART in
> > 8250_port driver,
> >
> > It detected as PORT_16750 UART, but I get below prints during autoconf
> > which is confusing for the end user
> >
> > [ 0.214926] serial8250-em a4040000.serial: detected caps 00000900
> should be 00000d00
> > [ 0.214975] a4040000.serial: ttyS0 at MMIO 0xa4040000 (irq = 24,
> base_baud = 3000000) is a TI16750
> >
> >
> > Modification in 8250_em driver
> >
> > + up.port.type = PORT_16750;
> > + up.port.name = "Renesas RZ/V2M 16750";
> > + up.port.fifosize = 64;
> > + up.tx_loadsz = 64;
> > + up.capabilities = UART_CAP_FIFO | UART_CAP_AFE;
> >
> > Identification method in 8250_port.c driver
> >
> > + /*
> > + * No EFR. Try to detect a Renesas RZ/V2M 16750, which only sets
> bit 5
> > + * of the IIR when 64 byte FIFO mode is enabled.
> > + * Try setting/clear bit5 of FCR.
> > + */
> > + serial_out(up, UART_FCR, UART_FCR_ENABLE_FIFO);
> > + status1 = serial_in(up, UART_IIR) & (UART_IIR_64BYTE_FIFO |
> > + UART_IIR_FIFO_ENABLED);
> > +
> > + serial_out(up, UART_FCR, UART_FCR_ENABLE_FIFO | UART_FCR7_64BYTE);
> > + status2 = serial_in(up, UART_IIR) & (UART_IIR_64BYTE_FIFO |
> > + UART_IIR_FIFO_ENABLED);
> > +
> > + if (status1 == UART_IIR_FIFO_ENABLED_16550A &&
> > + status2 == (UART_IIR_64BYTE_FIFO |
> UART_IIR_FIFO_ENABLED_16550A)) {
> > + up->port.type = PORT_16750;
> > + up->capabilities |= UART_CAP_AFE;
> > + return;
> > + }
>
> What I don't like is increasing quirks in the 8250_port. Can't you simply
> use FIXED_PORT facility?
> Again, look how 8250_mid is written.
Coll. I have referred 8250_mid and added below changes and it is working as expected.
I will sent next version after incorporating comments from Geert and ilpo.
+ up.port.type = PORT_16750;
+ up.port.flags |= UPF_FIXED_TYPE;
a4040000.serial: ttyS0 at MMIO 0xa4040000 (irq = 24, base_baud = 3000000) is a TI16750
root@rzv2m:~# /uart_dump.sh
Read at address 0xA4040000 (0xffff933ec000): 0x00000000
Read at address 0xA4040004 (0xffffb4994004): 0x00000005
Read at address 0xA4040008 (0xffff8041c008): 0x000000E1
Read at address 0xA404000C (0xffffbb62100c): 0x000000A1
Read at address 0xA4040010 (0xffff94158010): 0x00000013
Read at address 0xA4040014 (0xffff8018c014): 0x0000000B
Read at address 0xA4040018 (0xffff971af018): 0x00000060
Read at address 0xA404001C (0xffffb057d01c): 0x00000010
Read at address 0xA4040020 (0xffff8e8b6020): 0x00000000
Read at address 0xA4040024 (0xffff8094a024): 0x0000001A
Read at address 0xA4040028 (0xffffb6e81028): 0x00000000
Read at address 0xA404002C (0xffffa663902c): 0x00000000
root@rzv2m:~# stty -F /dev/ttyS0 115200 crtscts
root@rzv2m:~# /uart_dump.sh
Read at address 0xA4040000 (0xffff9672f000): 0x00000000
Read at address 0xA4040004 (0xffffb4d63004): 0x0000000D
Read at address 0xA4040008 (0xffffbdd07008): 0x000000E1
Read at address 0xA404000C (0xffff9573a00c): 0x000000A1
Read at address 0xA4040010 (0xffffbd5be010): 0x00000013
Read at address 0xA4040014 (0xffff9451c014): 0x0000002B
Read at address 0xA4040018 (0xffffb47cb018): 0x00000060
Read at address 0xA404001C (0xffffa680901c): 0x00000010
Read at address 0xA4040020 (0xffffb76d1020): 0x00000000
Read at address 0xA4040024 (0xffffb8667024): 0x0000001A
Read at address 0xA4040028 (0xffffad9c8028): 0x00000000
Read at address 0xA404002C (0xffff8f46702c): 0x00000000
Cheers,
Biju
