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

Re: [Xen-devel] [PATCH V2 1/3] xen/arm: drivers: scif: Add support for SCIFA compatible UARTs

On 18.02.19 16:00, Julien Grall wrote:


On 01/02/2019 12:37, Oleksandr Tyshchenko wrote:
From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>

Extend existing driver to be able to handle SCIFA interface as well.

In general a patch should do only one thing. In this case, this should have been split in 2 patches: one to extend the driver, the second to add support for SCIFA.

Please split the patch accordingly.

Not entirely clear to me how the current patch should be split...

- The first patch will be just a copy of the current patch, but without new compatible string (SCIFA).

- The second patch will add new compatible string.

Did I get your point correctly?

SCIF and SCIFA have lot in common, though SCIFA has different
offsets and bits for some registers.

The "data" field in struct dt_device_match is used for recognizing
what interface is present on a target board.

Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>

     Changes in v2:
         - Name a enum for describing interfaces this driver supports
         - Use local variable for "params" where appropriate
         - Use "data" field in struct dt_device_match instead of calling
         - Don't check for "overrun_reg != status_reg" condition during
  xen/drivers/char/scif-uart.c    | 139 ++++++++++++++++++++++++++++++----------
  xen/include/asm-arm/scif-uart.h |  44 +++++++++++--
  2 files changed, 142 insertions(+), 41 deletions(-)
+static const struct dt_device_match scif_uart_dt_match[] __initconst =
+    { .compatible = "renesas,scif",  .data = (void *)SCIF_PORT },
+    { .compatible = "renesas,scifa", .data = (void *)SCIFA_PORT },

The port_type is pretty much only used in scif_uart_init. So how about passing a pointer to the structure directly?

Agree. Will do.



Oleksandr Tyshchenko

Xen-devel mailing list



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