RE: [PATCH v3 3/3] serial: 8250_em: Add serial_out() to struct serial8250_em_hw_info

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

 



Hi Ilpo Järvinen,

Thanks for feedback.

> -----Original Message-----
> From: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
> Sent: Monday, February 13, 2023 12:05 PM
> To: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> Cc: Jiri Slaby <jirislaby@xxxxxxxxxx>; Geert Uytterhoeven
> <geert+renesas@xxxxxxxxx>; Magnus Damm <magnus.damm@xxxxxxxxx>; Niklas
> Söderlund <niklas.soderlund@xxxxxxxxxxxx>; Greg Kroah-Hartman
> <gregkh@xxxxxxxxxxxxxxxxxxx>; linux-serial <linux-serial@xxxxxxxxxxxxxxx>;
> Fabrizio Castro <fabrizio.castro.jz@xxxxxxxxxxx>; linux-renesas-
> soc@xxxxxxxxxxxxxxx
> Subject: RE: [PATCH v3 3/3] serial: 8250_em: Add serial_out() to struct
> serial8250_em_hw_info
> 
> On Mon, 13 Feb 2023, Biju Das wrote:
> 
> > Hi Ilpo,
> >
> > Thanks for the feedback.
> >
> > > Subject: RE: [PATCH v3 3/3] serial: 8250_em: Add serial_out() to
> > > struct serial8250_em_hw_info
> > >
> > > On Mon, 13 Feb 2023, Biju Das wrote:
> > >
> > > > Hi Jiri Slaby,
> > > >
> > > > Thanks for the feedback.
> > > >
> > > > > Subject: Re: [PATCH v3 3/3] serial: 8250_em: Add serial_out() to
> > > > > struct serial8250_em_hw_info
> > > > >
> > > > > On 13. 02. 23, 10:31, Biju Das wrote:
> > > > > > So looks like similar to other macros, UART_FCR_EM (0x3) is
> > > > > > sensible
> > > one.
> > > > > >
> > > > > > UART_FCR_RO_OFFSET (9)
> > > > > > UART_FCR_RO_EM (UART_FCR_EM + UART_FCR_RO_OFFSET)
> > > > > >
> > > > > >
> > > > > > static unsigned int serial8250_em_serial_in(struct uart_port
> > > > > > *p, int
> > > > > > offset) case UART_FCR_RO_EM:
> > > > > > 	return readl(p->membase + (offset - UART_FCR_RO_OFFSET <<
> > > > > > 2));
> > > > >
> > > > >
> > > > > Please send a complete patch as a reply. I am completely lost now.
> > > >
> > > > Please find the complete patc.
> > > >
> > > >
> > > > From e597ae60eb170c1f1b650e1e533bf4e12c09f822 Mon Sep 17 00:00:00
> > > > 2001
> > > > From: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > > > Date: Tue, 7 Feb 2023 15:07:13 +0000
> > > > Subject: [PATCH] serial: 8250_em: Add serial_out() to struct
> > > > serial8250_em_hw_info
> > > >
> > > > As per RZ/V2M hardware manual(Rev.1.30 Jun, 2022), UART IP has a
> > > > restriction as mentioned below.
> > > >
> > > > 40.6.1 Point for Caution when Changing the Register Settings:
> > > >
> > > > When changing the settings of the following registers, a PRESETn
> > > > master reset or FIFO reset + SW reset (FCR[2],FCR[1], HCR0[7])
> > > > must be input to re-initialize them.
> > > >
> > > > Target Registers: FCR, LCR, MCR, DLL, DLM, HCR0.
> > > >
> > > > 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>
> > > > ---
> > > >  drivers/tty/serial/8250/8250_em.c | 70
> > > > ++++++++++++++++++++++++++++++-
> > > >  1 file changed, 69 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/tty/serial/8250/8250_em.c
> > > > b/drivers/tty/serial/8250/8250_em.c
> > > > index 69cd3b611501..c1c64f48ee7e 100644
> > > > --- a/drivers/tty/serial/8250/8250_em.c
> > > > +++ b/drivers/tty/serial/8250/8250_em.c
> > > > @@ -17,12 +17,23 @@
> > > >
> > > >  #include "8250.h"
> > > >
> > > > +#define UART_FCR_EM 3
> > > >  #define UART_DLL_EM 9
> > > >  #define UART_DLM_EM 10
> > > > +#define UART_HCR0_EM 11
> > > > +
> > > > +#define UART_FCR_R_EM	(UART_FCR_EM + UART_HCR0_EM)
> > >
> > > It's easy to lose track of all this, IMHO this would be simple:
> > >
> > > /*
> > >  * A high value for UART_FCR_EM avoids overlapping with existing
> > > UART_*
> > >  * register defines. UART_FCR_EM_HW is the real HW register offset.
> > >  */
> > > #define UART_FCR_EM 12
> >
> > I will change it to #define UART_FCR_EM 14
> >
> > And will add the below unused HW status registers in the driver.
> >
> > #define UART_HCR2_EM 12 (@30)
> > #define UART_HCR3_EM 13 (@34)
> >
> > Is it ok?
> 
> It's okay, that number is pseudo one anyway so the actual number doesn't
> matter. One could just as well pick some large number such as 0x10003 or so
> if the collision with real regs is a concern.

OK will use 0x10003, as pseudo offset.

Cheers,
Biju




[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux