2012-05-28 09:59:25

by Roland Stigge

[permalink] [raw]
Subject: [PATCH] serial/of-serial: Add 16654 chip to compatible string list

The (ST)16654 chip was missing in the list of supported chips in of-serial.
This patch adds the string "st16654" to it, and to the documentation.

Signed-off-by: Roland Stigge <[email protected]>

---
Applies to v3.4

Documentation/devicetree/bindings/tty/serial/of-serial.txt | 1 +
drivers/tty/serial/of_serial.c | 1 +
2 files changed, 2 insertions(+)

--- linux-2.6.orig/Documentation/devicetree/bindings/tty/serial/of-serial.txt
+++ linux-2.6/Documentation/devicetree/bindings/tty/serial/of-serial.txt
@@ -6,6 +6,7 @@ Required properties:
- "ns16450"
- "ns16550a"
- "ns16550"
+ - "st16654"
- "ns16750"
- "ns16850"
- "nvidia,tegra20-uart"
--- linux-2.6.orig/drivers/tty/serial/of_serial.c
+++ linux-2.6/drivers/tty/serial/of_serial.c
@@ -179,6 +179,7 @@ static struct of_device_id __devinitdata
{ .compatible = "ns16450", .data = (void *)PORT_16450, },
{ .compatible = "ns16550a", .data = (void *)PORT_16550A, },
{ .compatible = "ns16550", .data = (void *)PORT_16550, },
+ { .compatible = "st16654", .data = (void *)PORT_16654, },
{ .compatible = "ns16750", .data = (void *)PORT_16750, },
{ .compatible = "ns16850", .data = (void *)PORT_16850, },
{ .compatible = "nvidia,tegra20-uart", .data = (void *)PORT_TEGRA, },


2012-05-28 10:03:55

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH] serial/of-serial: Add 16654 chip to compatible string list

On Mon, May 28, 2012 at 11:58:54AM +0200, Roland Stigge wrote:
> The (ST)16654 chip was missing in the list of supported chips in of-serial.
> This patch adds the string "st16654" to it, and to the documentation.

Something which occurs to me. Normally, the digit at the end indicates
how many ports are integrated into the device (0=1port, 2=2port, 4=4port.)
Are you sure there isn't a ST16650 which has the same characteristics?

We really should stick with naming these using the single port versions if
at all possible.

2012-05-28 11:21:02

by Roland Stigge

[permalink] [raw]
Subject: Re: [PATCH] serial/of-serial: Add 16654 chip to compatible string list

On 28/05/12 12:03, Russell King - ARM Linux wrote:
> Something which occurs to me. Normally, the digit at the end indicates
> how many ports are integrated into the device (0=1port, 2=2port, 4=4port.)
> Are you sure there isn't a ST16650 which has the same characteristics?
>
> We really should stick with naming these using the single port versions if
> at all possible.

Right.

This indicates to me that maybe this is not the right solution for my
original problem, anyway. :-)

Initially, with my RFC patch, there was an #ifdef for bigger FIFO in
case of LPC32xx where we have a 16550A variant with 64 byte fifos.

Looking at the 16x50 line:

16550A 16 bytes FIFOs
16650V2 32 bytes FIFOs
16750 64 bytes FIFOs

(?)

So maybe 16750 is the better choice for me, anyway. Already supported in
of-serial. Works for now, but need more testing. Another hint is that
16750 is advertised as "IP core for Soc" which matches the case of LPC32xx.

Maybe NXP can comment on this?

Thanks in advance,

Roland

2012-05-28 15:03:29

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH] serial/of-serial: Add 16654 chip to compatible string list

On Mon, May 28, 2012 at 01:20:55PM +0200, Roland Stigge wrote:
> On 28/05/12 12:03, Russell King - ARM Linux wrote:
> > Something which occurs to me. Normally, the digit at the end indicates
> > how many ports are integrated into the device (0=1port, 2=2port, 4=4port.)
> > Are you sure there isn't a ST16650 which has the same characteristics?
> >
> > We really should stick with naming these using the single port versions if
> > at all possible.
>
> Right.
>
> This indicates to me that maybe this is not the right solution for my
> original problem, anyway. :-)
>
> Initially, with my RFC patch, there was an #ifdef for bigger FIFO in
> case of LPC32xx where we have a 16550A variant with 64 byte fifos.

What are all the differences? Is it just a larger FIFO?

> Looking at the 16x50 line:
>
> 16550A 16 bytes FIFOs
> 16650V2 32 bytes FIFOs
> 16750 64 bytes FIFOs
>
> (?)
>
> So maybe 16750 is the better choice for me, anyway. Already supported in
> of-serial. Works for now, but need more testing. Another hint is that
> 16750 is advertised as "IP core for Soc" which matches the case of LPC32xx.

16750 also has automatic hardware flow control support, selectable through
bit 5 in the MCR register. If your UART has that, then it's probably a
16750 derivative rather than a 16550 or 16650 derivative.

16650s have an EFR register at offset 2, selectable by writing 0xBF into
the LCR register, which the 16750 doesn't have. 16650 also has automatic
hardware flow control, bit this is selected through a couple of bits in
the EFR.

With that information, you should be able to track down which of these
your UART is derived from.

2012-05-28 16:27:25

by Roland Stigge

[permalink] [raw]
Subject: Re: [PATCH] serial/of-serial: Add 16654 chip to compatible string list

On 28/05/12 17:03, Russell King - ARM Linux wrote:
>> Initially, with my RFC patch, there was an #ifdef for bigger FIFO in
>> case of LPC32xx where we have a 16550A variant with 64 byte fifos.
>
> What are all the differences? Is it just a larger FIFO?

Yes, this is how it's summarized in the manual (LPC32xx SoC).

>> So maybe 16750 is the better choice for me, anyway. Already supported in
>> of-serial. Works for now, but need more testing. Another hint is that
>> 16750 is advertised as "IP core for Soc" which matches the case of LPC32xx.
>
> 16750 also has automatic hardware flow control support, selectable through
> bit 5 in the MCR register. If your UART has that, then it's probably a
> 16750 derivative rather than a 16550 or 16650 derivative.
>
> 16650s have an EFR register at offset 2, selectable by writing 0xBF into
> the LCR register, which the 16750 doesn't have. 16650 also has automatic
> hardware flow control, bit this is selected through a couple of bits in
> the EFR.

The 4 LPC32xx's "Standard" UARTs have neither of those.

Is it ok to use "ns16650", i.e. PORT_16650, or do I need to introduce a
FIFO depth configuration?

Thanks in advance,

Roland

2012-05-28 16:32:03

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH] serial/of-serial: Add 16654 chip to compatible string list

On Mon, May 28, 2012 at 06:27:20PM +0200, Roland Stigge wrote:
> On 28/05/12 17:03, Russell King - ARM Linux wrote:
> >> Initially, with my RFC patch, there was an #ifdef for bigger FIFO in
> >> case of LPC32xx where we have a 16550A variant with 64 byte fifos.
> >
> > What are all the differences? Is it just a larger FIFO?
>
> Yes, this is how it's summarized in the manual (LPC32xx SoC).
>
> >> So maybe 16750 is the better choice for me, anyway. Already supported in
> >> of-serial. Works for now, but need more testing. Another hint is that
> >> 16750 is advertised as "IP core for Soc" which matches the case of LPC32xx.
> >
> > 16750 also has automatic hardware flow control support, selectable through
> > bit 5 in the MCR register. If your UART has that, then it's probably a
> > 16750 derivative rather than a 16550 or 16650 derivative.
> >
> > 16650s have an EFR register at offset 2, selectable by writing 0xBF into
> > the LCR register, which the 16750 doesn't have. 16650 also has automatic
> > hardware flow control, bit this is selected through a couple of bits in
> > the EFR.
>
> The 4 LPC32xx's "Standard" UARTs have neither of those.
>
> Is it ok to use "ns16650", i.e. PORT_16650, or do I need to introduce a
> FIFO depth configuration?

I think you need a new type, because as I said above, 16650s have that
additional EFR, and we will attempt to access that register which
isn't present in yours.

It's not solely about fifo depth.

2012-05-28 17:49:05

by Roland Stigge

[permalink] [raw]
Subject: Re: [PATCH] serial/of-serial: Add 16654 chip to compatible string list

On 28/05/12 18:31, Russell King - ARM Linux wrote:
>>>> So maybe 16750 is the better choice for me, anyway. Already supported in
>>>> of-serial. Works for now, but need more testing. Another hint is that
>>>> 16750 is advertised as "IP core for Soc" which matches the case of LPC32xx.
>>>
>>> 16750 also has automatic hardware flow control support, selectable through
>>> bit 5 in the MCR register. If your UART has that, then it's probably a
>>> 16750 derivative rather than a 16550 or 16650 derivative.
>>>
>>> 16650s have an EFR register at offset 2, selectable by writing 0xBF into
>>> the LCR register, which the 16750 doesn't have. 16650 also has automatic
>>> hardware flow control, bit this is selected through a couple of bits in
>>> the EFR.
>>
>> The 4 LPC32xx's "Standard" UARTs have neither of those.
>>
>> Is it ok to use "ns16650", i.e. PORT_16650, or do I need to introduce a
>> FIFO depth configuration?
>
> I think you need a new type, because as I said above, 16650s have that
> additional EFR, and we will attempt to access that register which
> isn't present in yours.

I actually meant 16750 instead of 16650, sorry, but this basically means
the same - I would refer to extensions that are actually not there...

Now, introducing a new type, can I add to 8250.c's uart_config[] by
introducing a new type (no. 22) after PORT_XR17D15X? Unfortunately,
there are the "ARM specific type numbers" after current PORT_MAX_8250
(21), but those are not listed in 8250.c's uart_config[]. Or how am I
supposed to add a new type?

Thanks in advance,

Roland

2012-05-28 18:01:51

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH] serial/of-serial: Add 16654 chip to compatible string list

On Mon, May 28, 2012 at 07:48:57PM +0200, Roland Stigge wrote:
> On 28/05/12 18:31, Russell King - ARM Linux wrote:
> >>>> So maybe 16750 is the better choice for me, anyway. Already supported in
> >>>> of-serial. Works for now, but need more testing. Another hint is that
> >>>> 16750 is advertised as "IP core for Soc" which matches the case of LPC32xx.
> >>>
> >>> 16750 also has automatic hardware flow control support, selectable through
> >>> bit 5 in the MCR register. If your UART has that, then it's probably a
> >>> 16750 derivative rather than a 16550 or 16650 derivative.
> >>>
> >>> 16650s have an EFR register at offset 2, selectable by writing 0xBF into
> >>> the LCR register, which the 16750 doesn't have. 16650 also has automatic
> >>> hardware flow control, bit this is selected through a couple of bits in
> >>> the EFR.
> >>
> >> The 4 LPC32xx's "Standard" UARTs have neither of those.
> >>
> >> Is it ok to use "ns16650", i.e. PORT_16650, or do I need to introduce a
> >> FIFO depth configuration?
> >
> > I think you need a new type, because as I said above, 16650s have that
> > additional EFR, and we will attempt to access that register which
> > isn't present in yours.
>
> I actually meant 16750 instead of 16650, sorry, but this basically means
> the same - I would refer to extensions that are actually not there...
>
> Now, introducing a new type, can I add to 8250.c's uart_config[] by
> introducing a new type (no. 22) after PORT_XR17D15X? Unfortunately,
> there are the "ARM specific type numbers" after current PORT_MAX_8250
> (21), but those are not listed in 8250.c's uart_config[]. Or how am I
> supposed to add a new type?

If it's 8250, stick it in with the group, otherwise the array will
become stupidly large. That's why there's a separation of the two.

2012-05-28 18:18:03

by Roland Stigge

[permalink] [raw]
Subject: Re: [PATCH] serial/of-serial: Add 16654 chip to compatible string list

On 28/05/12 20:01, Russell King - ARM Linux wrote:
>> Now, introducing a new type, can I add to 8250.c's uart_config[] by
>> introducing a new type (no. 22) after PORT_XR17D15X? Unfortunately,
>> there are the "ARM specific type numbers" after current PORT_MAX_8250
>> (21), but those are not listed in 8250.c's uart_config[]. Or how am I
>> supposed to add a new type?
>
> If it's 8250, stick it in with the group, otherwise the array will
> become stupidly large. That's why there's a separation of the two.

Do you mean adding one element to uart_config[] and increasing
PORT_MAX_8250 (actually, the size of uart_config[]) by one? I would also
need to increase the indices of all the following "ARM specific type
numbers" by one (the second group).

Or did I get sth. wrong here?

Thanks in advance,

Roland

2012-05-28 18:39:32

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH] serial/of-serial: Add 16654 chip to compatible string list

On Mon, May 28, 2012 at 08:17:58PM +0200, Roland Stigge wrote:
> On 28/05/12 20:01, Russell King - ARM Linux wrote:
> >> Now, introducing a new type, can I add to 8250.c's uart_config[] by
> >> introducing a new type (no. 22) after PORT_XR17D15X? Unfortunately,
> >> there are the "ARM specific type numbers" after current PORT_MAX_8250
> >> (21), but those are not listed in 8250.c's uart_config[]. Or how am I
> >> supposed to add a new type?
> >
> > If it's 8250, stick it in with the group, otherwise the array will
> > become stupidly large. That's why there's a separation of the two.
>
> Do you mean adding one element to uart_config[] and increasing
> PORT_MAX_8250 (actually, the size of uart_config[]) by one? I would also
> need to increase the indices of all the following "ARM specific type
> numbers" by one (the second group).

#define PORT_XR17D15X 21 /* Exar XR17D15x UART */
#define PORT_MAX_8250 21 /* max port ID */

#define PORT_PXA 31

There's space between 21 and 31...