Re: [U-Boot] [PATCH 05/14] serial: 16550: Add port type as driver data

2016-11-29 Thread Marek Vasut 
On 11/30/2016 04:10 AM, Simon Glass wrote:
> 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...

Until someone refactors it or splits it. I wouldn't go with that
assumption. I really dislike having some default here.

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 05/14] serial: 16550: Add port type as driver data

2016-11-29 Thread Simon Glass 
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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 05/14] serial: 16550: Add port type as driver data

2016-11-29 Thread Marek Vasut 
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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 05/14] serial: 16550: Add port type as driver data

2016-11-29 Thread Simon Glass 
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.

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 05/14] serial: 16550: Add port type as driver data

2016-11-29 Thread Marek Vasut 
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 ?

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 05/14] serial: 16550: Add port type as driver data

2016-11-29 Thread Simon Glass 
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.

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 05/14] serial: 16550: Add port type as driver data

2016-11-27 Thread Marek Vasut 
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.

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 05/14] serial: 16550: Add port type as driver data

2016-11-27 Thread Simon Glass 
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?

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 05/14] serial: 16550: Add port type as driver data

2016-11-25 Thread Marek Vasut 
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 },
{}
 };
 #endif
-- 
2.10.2

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot