From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Tue, 29 Nov 2016 20:10:35 -0700 Subject: [U-Boot] [PATCH 05/14] serial: 16550: Add port type as driver data In-Reply-To: <026837e5-b6da-3b05-bf38-5fbecf8b3ca3@denx.de> References: <20161125223235.3434-1-marex@denx.de> <20161125223235.3434-5-marex@denx.de> <026837e5-b6da-3b05-bf38-5fbecf8b3ca3@denx.de> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Marek, On 29 November 2016 at 20:04, Marek Vasut wrote: > 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. Because the driver is called ns16550. So one can assume that this is the base case... Regards, Simon