Re: [PATCH v2 2/2] serial: 8250_dw: add ACPI support for uart on Hisilicon Hip05 soc

On 2016/6/30 17:54, Andy Shevchenko wrote:

On Thu, 2016-06-30 at 12:38 +0300, Andy Shevchenko wrote:

quoted

On Thu, 2016-06-30 at 09:27 +0800, Kefeng Wang wrote:

quoted

Add ACPI identifier for UART on Hisilicon Hip05 soc, be careful
that it is not 16550 compatibal.

Meanwhile, set dw8250_serial_out32 to keep consistent between
serial_out
and serial_in in ACPI.

Signed-off-by: Kefeng Wang <redacted>
---
 drivers/tty/serial/8250/8250_dw.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c

b/drivers/tty/serial/8250/8250_dw.c
index 65f3da7..096431b 100644

--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c

@@ -272,6 +272,12 @@ static bool dw8250_idma_filter(struct dma_chan

*chan, void *param)
 	return param == chan->device->dev->parent;
 }
 
+/* non 16550 compatible id list*/
+static const struct acpi_device_id non_16550_id_list[] = {
+	{ "HISI0031", 0 },
+	{ },
+};
+

On first glance it looks redundant, see below.

Oh, wait, this is still valid, but the better solution is to use device
properties for the rest except yours!

Do you mean using something like

static struct property_entry dw8250_properties[] = {
        PROPERTY_ENTRY_U32("reg-io-width", 4),
        PROPERTY_ENTRY_U32("reg-shift", 2),
        PROPERTY_ENTRY_BOOL("snps,uart-16550-compatible"),
        { },
};

then use platform_device_add_properties to add it to the device(16500 compatible),

but for hisi, use another property_entry without PROPERTY_ENTRY_BOOL("snps,uart-16550-compatible")?

Not clear about using device properties.

I will prepare patch later this week or at the beginning of next week if
you are in hurry, otherwise I would postpone this a bit (anyway it will
not make v4.8 cycle).

Thanks, hope that it will come out soon.

BRs,
Kefeng