Re: [PATCH V4 1/2] xen/arm: Add i.MX lpuart driver

[Julien Grall <julien@xxxxxxx>]

Hi,

On 11/04/2022 07:26, Michal Orzel wrote:
> On 08.04.2022 16:09, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@xxxxxxx>
> >
> > The i.MX LPUART Documentation:
> > https://www.nxp.com/webapp/Download?colCode=IMX8QMIEC
> > Chatper 13.6 Low Power Universal Asynchronous Receiver/
> > Transmitter (LPUART)
> >
> > Tested-by: Henry Wang <Henry.Wang@xxxxxxx>
> > Signed-off-by: Peng Fan <peng.fan@xxxxxxx>
> > ---
> >   xen/arch/arm/include/asm/imx-lpuart.h |  64 ++++++
> >   xen/drivers/char/Kconfig              |   7 +
> >   xen/drivers/char/Makefile             |   1 +
> >   xen/drivers/char/imx-lpuart.c         | 276 ++++++++++++++++++++++++++
> >   4 files changed, 348 insertions(+)
> >   create mode 100644 xen/arch/arm/include/asm/imx-lpuart.h
> >   create mode 100644 xen/drivers/char/imx-lpuart.c
> >
> > diff --git a/xen/arch/arm/include/asm/imx-lpuart.h 
> > b/xen/arch/arm/include/asm/imx-lpuart.h
> > new file mode 100644
> > index 0000000000..26e2cf0249
> > --- /dev/null
> > +++ b/xen/arch/arm/include/asm/imx-lpuart.h
> > @@ -0,0 +1,64 @@
> > +/*
> > + * xen/arch/arm/include/asm/imx-lpuart.h
> > + *
> > + * Common constant definition between early printk and the LPUART driver
> > + *
> > + * Peng Fan <peng.fan@xxxxxxx>
> > + * Copyright 2022 NXP
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#ifndef __ASM_ARM_IMX_LPUART_H__
> > +#define __ASM_ARM_IMX_LPUART_H__
> > +
> > +/* 32-bit register definition */
> > +#define UARTBAUD          (0x10)
> > +#define UARTSTAT          (0x14)
> > +#define UARTCTRL          (0x18)
> > +#define UARTDATA          (0x1C)
> > +#define UARTMATCH         (0x20)
> > +#define UARTMODIR         (0x24)
> > +#define UARTFIFO          (0x28)
> > +#define UARTWATER         (0x2c)
> > +
> > +#define UARTSTAT_TDRE     BIT(23, UL)
> > +#define UARTSTAT_TC       BIT(22, UL)
> > +#define UARTSTAT_RDRF     BIT(21, UL)
> > +#define UARTSTAT_OR       BIT(19, UL)
> > +
> > +#define UARTBAUD_OSR_SHIFT (24)
> > +#define UARTBAUD_OSR_MASK (0x1f)
> > +#define UARTBAUD_SBR_MASK (0x1fff)
> > +#define UARTBAUD_BOTHEDGE (0x00020000)
> > +#define UARTBAUD_TDMAE    (0x00800000)
> > +#define UARTBAUD_RDMAE    (0x00200000)It seems like you missed my comment 
> > about alignment from the previous patch version.
> Please keep the same alignment of values within the same section.
I haven't yet reviewed this patch. But if this is the only comment, then 
I can update the alignment on commit.
Cheers,

--
Julien Grall