RE: [PATCH 3/3] serial: 8250_em: Add serial8250_rzv2m_reg_update()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Ilpo,

Thanks for the feedback.

> Subject: Re: [PATCH 3/3] serial: 8250_em: Add serial8250_rzv2m_reg_update()
> 
> On Thu, 9 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 serial8250_rzv2m_reg_update() to handle it.
> >
> > 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>
> > ---
> >  drivers/tty/serial/8250/8250_em.c | 49
> > +++++++++++++++++++++++++++++++
> >  1 file changed, 49 insertions(+)
> >
> > diff --git a/drivers/tty/serial/8250/8250_em.c
> > b/drivers/tty/serial/8250/8250_em.c
> > index 3a45aa066d3d..a1e42b8ef99d 100644
> > --- a/drivers/tty/serial/8250/8250_em.c
> > +++ b/drivers/tty/serial/8250/8250_em.c
> > @@ -9,6 +9,7 @@
> >  #include <linux/io.h>
> >  #include <linux/module.h>
> >  #include <linux/mod_devicetable.h>
> > +#include <linux/of.h>
> >  #include <linux/serial_8250.h>
> >  #include <linux/serial_reg.h>
> >  #include <linux/platform_device.h>
> > @@ -18,14 +19,53 @@
> >
> >  #define UART_DLL_EM 9
> >  #define UART_DLM_EM 10
> > +#define UART_HCR0 11
> > +
> > +#define UART_HCR0_SW_RESET	BIT(7) /* SW Reset */
> >
> >  struct serial8250_em_priv {
> >  	struct clk *sclk;
> >  	int line;
> > +	bool is_rzv2m;
> >  };
> >
> > +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)); }
> > +
> >  static void serial8250_em_serial_out(struct uart_port *p, int offset,
> > int value)  {
> > +	struct serial8250_em_priv *priv = p->private_data;
> > +
> >  	switch (offset) {
> >  	case UART_TX: /* TX @ 0x00 */
> >  		writeb(value, p->membase);
> > @@ -33,6 +73,11 @@ static void serial8250_em_serial_out(struct uart_port
> *p, int offset, int value)
> >  	case UART_FCR: /* FCR @ 0x0c (+1) */
> >  	case UART_LCR: /* LCR @ 0x10 (+1) */
> >  	case UART_MCR: /* MCR @ 0x14 (+1) */
> > +		if (priv->is_rzv2m)
> > +			serial8250_rzv2m_reg_update(p, offset, value);
> > +		else
> > +			writel(value, p->membase + ((offset + 1) << 2));
> > +		break;
> 
> Create serial8250_em_rzv2m_serial_out() that does the necessary magic and
> calls serial8250_em_serial_out() in other cases.
> 
> I think you can use .data in of_device_id table to pick the correct
> .serial_out function so you don't need to add that bool at all.

OK, will add .data and will make use a feature flag to handle this
as suggested by Geert.

> 
> >  	case UART_SCR: /* SCR @ 0x20 (+1) */
> >  		writel(value, p->membase + ((offset + 1) << 2));
> >  		break;
> > @@ -111,6 +156,10 @@ static int serial8250_em_probe(struct platform_device
> *pdev)
> >  	up.port.uartclk = clk_get_rate(priv->sclk);
> >
> >  	up.port.iotype = UPIO_MEM32;
> > +
> > +	if (of_device_is_compatible(dev->of_node, "renesas,r9a09g011-uart"))
> > +		priv->is_rzv2m = true;
> > +
> >  	up.port.serial_in = serial8250_em_serial_in;
> >  	up.port.serial_out = serial8250_em_serial_out;
> >  	up.dl_read = serial8250_em_serial_dl_read;
> >
> 
> I'm bit lost why you need patch 1/3 and cannot set the port type and
> capabilities here?

As per Andy's suggestion I have tried this based on 8250_mid.c and
It works as expected.

So plan is create separate patch for changing port type from 16550A to
16750.

Cheers,
Biju




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux