| Message ID | 20230210203439.174913-1-biju.das.jz@bp.renesas.com |
|---|---|
| Headers | show |
| Series | Update Renesas RZ/V2M UART Port type | expand |
On Fri, Feb 10, 2023 at 08:34:39PM +0000, 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. ... > #define UART_DLL_EM 9 > #define UART_DLM_EM 10 > +#define UART_HCR0 11 Can we continue using _EM suffix? ... > + /* > + * 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)); So, what does prevent you to define above #define UART_IIR_EM 3 // or whatever name it should have and use again same helper here?
Hi All, > Subject: RE: [PATCH v3 3/3] serial: 8250_em: Add serial_out() to struct > serial8250_em_hw_info > > Hi Ilpo Järvinen, > > Thanks for feedback. > > > -----Original Message----- > > From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > > Sent: Monday, February 13, 2023 12:05 PM > > To: Biju Das <biju.das.jz@bp.renesas.com> > > Cc: Jiri Slaby <jirislaby@kernel.org>; Geert Uytterhoeven > > <geert+renesas@glider.be>; Magnus Damm <magnus.damm@gmail.com>; Niklas > > Söderlund <niklas.soderlund@ragnatech.se>; Greg Kroah-Hartman > > <gregkh@linuxfoundation.org>; linux-serial > > <linux-serial@vger.kernel.org>; Fabrizio Castro > > <fabrizio.castro.jz@renesas.com>; linux-renesas- soc@vger.kernel.org > > 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@bp.renesas.com> > > > > > 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@bp.renesas.com> > > > > > --- > > > > > 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. I would like to use same Hardware restriction for both RZ/V2M and EMMA-Mobile as both the SoC's have same register sets and it works ok on both these platforms, after this restriction is applied. I guess it is ok for everyone. Cheers, Biju
Hi Jiri Slaby, Thanks for the feedback. > Subject: Re: [PATCH v3 1/3] serial: 8250_em: Use dev_err_probe() > > On 10. 02. 23, 21:34, Biju Das wrote: > > This patch simplifies probe() function by using dev_err_probe() > > instead of dev_err in probe(). > > > > While at it, remove the unused header file slab.h and added a local > > variable 'dev' to replace '&pdev->dev' in probe(). > > > > Also replace devm_clk_get->devm_clk_get_enabled and updated the clk > > handling in probe() and remove(). > > Sorry, this "While at it" and "Also" is not allowed in a single patch. > You do three completely distinct things in a single patch. Please separate > (like prints+&dev, slab.h, and clk changes). OK, will split into 3 patches as mentioned above. > > And pretty please, don't send 3 (!) versions a day. Wait a week or so for > reviews of a single version. No need to haste. OK, next time will take care. Cheers, Biju
The Renesas RZ/V2M UART compatible with the general-purpose 16750 UART chip. This patch updates Renesas RZ/V2M UART type from 16550a->16750 and also enables 64-bytes fifo. This patch series also simplifies 8250_em_probe() and also updates the RZ/V2M specific register handling for the below restriction mentioned in hardware manual 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. v2->v3: * Dropped sclk from priv. * Dropped unneeded clk_disable_unprepare from remove(). * Retained Rb tags from Geert,Andy and Ilpo as the changes are trivial. * Replaced of_device_get_match_data()->device_get_match_data(). * Replaced of_device.h->property.h * Dropped struct serial8250_em_hw_info *info from priv and started using a local variable info in probe(). * Retained Rb tag from Ilpo as changes are trivial. * Replaced readl/writel()->serial8250_em_serial_in/out() in serial8250_rzv2m_ reg_update() to avoid duplication of offset trickery. * Added support for HCR0 read/write in serial8250_em_{serial_in, serial_out} * Added UART_LCR macro support in serial8250_em_serial_in() to read LCR * Reordered serial8250_em_{serial_in, serial_out} above serial8250_rzv2m_ reg_update(). * Replaced priv->info->serial_out to info->serial_out. v1->v2: * Dropped patch#1 from previous series * Replaced devm_clk_get->devm_clk_get_enabled() and updated clk handling in probe(). * Added Rb tag from Geert. * Added patch for updating Renesas RZ/V2M UART type from 16550a->16750 and also enables 64-bytes fifo. * Used .data for taking h/w differences between EMMA mobile and RZ/V2M UART. * Added serial_out() to struct serial8250_em_hw_info for the write register differences between EMMA mobile and RZ/V2M UART. Biju Das (3): serial: 8250_em: Use dev_err_probe() serial: 8250_em: Update RZ/V2M port type as PORT_16750 serial: 8250_em: Add serial_out() to struct serial8250_em_hw_info drivers/tty/serial/8250/8250_em.c | 128 ++++++++++++++++++++++++------ 1 file changed, 102 insertions(+), 26 deletions(-)