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

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



Hi Julien,

On 11.04.2022 12:20, Julien Grall wrote:
> 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,
> 

All my other findings have already been fixed by Peng. This is the last one as 
it was forgotten.
Patch 2/2 still has sth to be fixed so the alignment can be fixed in the same 
shot as well.

Cheers,
Michal



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.