From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Vasut Date: Wed, 30 Nov 2016 04:04:53 +0100 Subject: [U-Boot] [PATCH 05/14] serial: 16550: Add port type as driver data In-Reply-To: References: <20161125223235.3434-1-marex@denx.de> <20161125223235.3434-5-marex@denx.de> Message-ID: <026837e5-b6da-3b05-bf38-5fbecf8b3ca3@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 11/30/2016 03:26 AM, Simon Glass wrote: > Hi Marek, > > On 29 November 2016 at 18:27, Marek Vasut wrote: >> On 11/30/2016 01:32 AM, Simon Glass wrote: >>> Hi Marek, >>> >>> On 27 November 2016 at 10:07, Marek Vasut wrote: >>>> On 11/27/2016 06:03 PM, Simon Glass wrote: >>>>> Hi Marex, >>>>> >>>>> On 25 November 2016 at 15:32, Marek Vasut wrote: >>>>>> Add driver data to each compatible string to identify the type of >>>>>> the port. Since all the ports in the driver are entirely compatible >>>>>> with 16550 for now, all are marked with PORT_NS16550. But, there >>>>>> are ports which have specific quirks, like the JZ4780 UART, which >>>>>> do not have any DT property to denote the quirks. Instead, Linux >>>>>> uses the compatible string to discern such ports and enable the >>>>>> necessary quirks. >>>>>> >>>>>> Signed-off-by: Marek Vasut >>>>>> Cc: Tom Rini >>>>>> Cc: Simon Glass >>>>>> --- >>>>>> drivers/serial/ns16550.c | 26 ++++++++++++++++---------- >>>>>> 1 file changed, 16 insertions(+), 10 deletions(-) >>>>>> >>>>>> diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c >>>>>> index 3c9f3b0..3130a1d 100644 >>>>>> --- a/drivers/serial/ns16550.c >>>>>> +++ b/drivers/serial/ns16550.c >>>>>> @@ -360,6 +360,12 @@ int ns16550_serial_probe(struct udevice *dev) >>>>>> return 0; >>>>>> } >>>>>> >>>>>> +#if CONFIG_IS_ENABLED(OF_CONTROL) >>>>>> +enum { >>>>>> + PORT_NS16550 = 0, >>>>>> +}; >>>>>> +#endif >>>>>> + >>>>>> #if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA) >>>>>> int ns16550_serial_ofdata_to_platdata(struct udevice *dev) >>>>>> { >>>>>> @@ -453,16 +459,16 @@ const struct dm_serial_ops ns16550_serial_ops = { >>>>>> * compatible string to your dts. >>>>>> */ >>>>>> static const struct udevice_id ns16550_serial_ids[] = { >>>>>> - { .compatible = "ns16550" }, >>>>>> - { .compatible = "ns16550a" }, >>>>>> - { .compatible = "nvidia,tegra20-uart" }, >>>>>> - { .compatible = "snps,dw-apb-uart" }, >>>>>> - { .compatible = "ti,omap2-uart" }, >>>>>> - { .compatible = "ti,omap3-uart" }, >>>>>> - { .compatible = "ti,omap4-uart" }, >>>>>> - { .compatible = "ti,am3352-uart" }, >>>>>> - { .compatible = "ti,am4372-uart" }, >>>>>> - { .compatible = "ti,dra742-uart" }, >>>>>> + { .compatible = "ns16550", .data = PORT_NS16550 }, >>>>>> + { .compatible = "ns16550a", .data = PORT_NS16550 }, >>>>>> + { .compatible = "nvidia,tegra20-uart", .data = PORT_NS16550 }, >>>>>> + { .compatible = "snps,dw-apb-uart", .data = PORT_NS16550 }, >>>>>> + { .compatible = "ti,omap2-uart", .data = PORT_NS16550 }, >>>>>> + { .compatible = "ti,omap3-uart", .data = PORT_NS16550 }, >>>>>> + { .compatible = "ti,omap4-uart", .data = PORT_NS16550 }, >>>>>> + { .compatible = "ti,am3352-uart", .data = PORT_NS16550 }, >>>>>> + { .compatible = "ti,am4372-uart", .data = PORT_NS16550 }, >>>>>> + { .compatible = "ti,dra742-uart", .data = PORT_NS16550 }, >>>>> >>>>> But can we have 0 as the default so we don't need these values? >>>> >>>> PORT_NS16550 is zero anyway, it's just explicitly spelled here. >>> >>> One way might be to use PORT_DEFAULT, set it to 0 and omit it here. It >>> does seem odd to have PORT_NS16550 in the ns16550 driver. >> >> I'd rather be explicit in case we grow this list, it also doesn't hurt >> anything since it is implicitly set to zero. Or is this something more >> than a matter of preference here ? > > Well, at least rename the PORT value to DEFAULT or something like that > if you want to be explicit. Why DEFAULT ? It's NS16550 compatible port and the others are not entirely NS16550 compatible, so I think NS16550 is exactly how it should be named. There never was any DEFAULT label on these chips, but the original 16550 UART chip had 16550 written on it. -- Best regards, Marek Vasut