[PATCH 0/2] ARM: tegra: Enable high speed UARTs of the Jetson TK1

[PATCH 0/2] ARM: tegra: Enable high speed UARTs of the Jetson TK1

[Ralf Ramsauer]

Hi everyone,

this one enables the high speed UARTs of the Jetson TK1 (and a not
noteworthy typo). Those additional UARTs are located at the
expansion port of the board.

So far, they were only enabled in NVidia's official BSP.

  Ralf

Ralf Ramsauer (2):
  ARM: tegra: Fixed typo
  ARM: tegra: Add high speed UARTs to Jetson TK1 device tree

 arch/arm/boot/dts/tegra124-jetson-tk1.dts | 23 +++++++++++++++++++++++
 arch/arm/boot/dts/tegra124.dtsi           |  2 +-
 2 files changed, 24 insertions(+), 1 deletion(-)

-- 
2.4.10

[PATCH 1/2] ARM: tegra: Fixed typo

[Ralf Ramsauer]

Signed-off-by: Ralf Ramsauer <ralf-ex4EvEgDZR1o7HI0AxOeTKwczVv4G7md@public.gmane.org>
---
 arch/arm/boot/dts/tegra124.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/tegra124.dtsi b/arch/arm/boot/dts/tegra124.dtsi
index 68669f7..995289b 100644
--- a/arch/arm/boot/dts/tegra124.dtsi
+++ b/arch/arm/boot/dts/tegra124.dtsi
@@ -322,7 +322,7 @@
 	 * driver and APB DMA based serial driver for higher baudrate
 	 * and performace. To enable the 8250 based driver, the compatible
 	 * is "nvidia,tegra124-uart", "nvidia,tegra20-uart" and to enable
-	 * the APB DMA based serial driver, the comptible is
+	 * the APB DMA based serial driver, the compatible is
 	 * "nvidia,tegra124-hsuart", "nvidia,tegra30-hsuart".
 	 */
 	uarta: serial@0,70006000 {
-- 
2.4.10

Re: [PATCH 1/2] ARM: tegra: Fixed typo

[Stephen Warren]

On 01/19/2016 06:12 AM, Ralf Ramsauer wrote:

> diff --git a/arch/arm/boot/dts/tegra124.dtsi b/arch/arm/boot/dts/tegra124.dtsi

> -	 * the APB DMA based serial driver, the comptible is
> +	 * the APB DMA based serial driver, the compatible is

Presumably this typo exists in tegra*.dtsi not just tegra124.dtsi. 
Perhaps you could fix it there too?

BTW, you didn't Cc any Tegra maintainers, so Thierry (who applies the 
patches) probably isn't going to notice that you sent this patch. Please 
also Cc the ARM kernel mailing list on Tegra patches. For reference, see 
the output of "./scripts/get_maintainer.pl *.patch".

Re: [PATCH 1/2] ARM: tegra: Fixed typo

[Ralf Ramsauer]

On 01/19/16 17:29, Stephen Warren wrote:
> On 01/19/2016 06:12 AM, Ralf Ramsauer wrote:
>
>> diff --git a/arch/arm/boot/dts/tegra124.dtsi
>> b/arch/arm/boot/dts/tegra124.dtsi
>
>> -     * the APB DMA based serial driver, the comptible is
>> +     * the APB DMA based serial driver, the compatible is
>
> Presumably this typo exists in tegra*.dtsi not just tegra124.dtsi.
> Perhaps you could fix it there too?
I'll check it.
>
> BTW, you didn't Cc any Tegra maintainers, so Thierry (who applies the
> patches) probably isn't going to notice that you sent this patch.
> Please also Cc the ARM kernel mailing list on Tegra patches. For
> reference, see the output of "./scripts/get_maintainer.pl *.patch".
Sure, I'll prepare and send a v2.

  Ralf
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
Ralf Ramsauer
GPG: 0x8F10049B

[PATCH 2/2] ARM: tegra: Add high speed UARTs to Jetson TK1 device tree

[Ralf Ramsauer]

This patch enables the APB DMA high speed UARTs of the Jetson TK1.

Signed-off-by: Ralf Ramsauer <ralf-ex4EvEgDZR1o7HI0AxOeTKwczVv4G7md@public.gmane.org>
---
 arch/arm/boot/dts/tegra124-jetson-tk1.dts | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/arch/arm/boot/dts/tegra124-jetson-tk1.dts b/arch/arm/boot/dts/tegra124-jetson-tk1.dts
index 66b4451..0bd9be7 100644
--- a/arch/arm/boot/dts/tegra124-jetson-tk1.dts
+++ b/arch/arm/boot/dts/tegra124-jetson-tk1.dts
@@ -12,7 +12,12 @@
 	aliases {
 		rtc0 = "/i2c@0,7000d000/pmic@40";
 		rtc1 = "/rtc@0,7000e000";
+
+		/* This order keeps the mapping DB9 connector <-> ttyS0 */
 		serial0 = &uartd;
+		serial1 = &uarta;
+		serial2 = &uartb;
+		serial3 = &uartc;
 	};
 
 	memory {
@@ -1367,6 +1372,24 @@
 		};
 	};
 
+	/* First high speed UART */
+	serial@0,70006000 {
+		compatible = "nvidia,tegra124-hsuart", "nvidia,tegra30-hsuart";
+		status = "okay";
+	};
+
+	/* Second high speed UART */
+	serial@0,70006040 {
+		compatible = "nvidia,tegra124-hsuart", "nvidia,tegra30-hsuart";
+		status = "okay";
+	};
+
+	/* Third high speed UART */
+	serial@0,70006200 {
+		compatible = "nvidia,tegra124-hsuart", "nvidia,tegra30-hsuart";
+		status = "okay";
+	};
+
 	/* DB9 serial port */
 	serial@0,70006300 {
 		status = "okay";
-- 
2.4.10

Re: [PATCH 2/2] ARM: tegra: Add high speed UARTs to Jetson TK1 device tree

[Stephen Warren]

On 01/19/2016 06:12 AM, Ralf Ramsauer wrote:
> This patch enables the APB DMA high speed UARTs of the Jetson TK1.

In the cover letter, you mentioned that these ports are exposed on the 
expansion connector. It'd be useful to mention that fact here too, since 
the cover letter doesn't make it into the git commit log.

However, given that, I don't think we should apply this patch. We can't 
know ahead of time what hardware is connected to the expansion header; 
the pins aren't dedicated to be serial ports (they could be GPIOs and 
probably other functions too depending on the pinmux settings), and 
different people will connect different hardware or none at all. Hence, 
the default DT should not enable IO controllers for the pins on the 
expansion connector. The best approach these days is to use DT overlays. 
You may need to add some "hooks" into the Tegra base DT files to allow 
overlays to be used; they weren't written with overlays in mind.

Re: [PATCH 2/2] ARM: tegra: Add high speed UARTs to Jetson TK1 device tree

[Ralf Ramsauer]

Thanks for review, Stephen.

On 01/19/16 17:32, Stephen Warren wrote:
> On 01/19/2016 06:12 AM, Ralf Ramsauer wrote:
>> This patch enables the APB DMA high speed UARTs of the Jetson TK1.
>
> In the cover letter, you mentioned that these ports are exposed on the
> expansion connector. It'd be useful to mention that fact here too,
> since the cover letter doesn't make it into the git commit log.
Good point.
>
> However, given that, I don't think we should apply this patch. We
> can't know ahead of time what hardware is connected to the expansion
> header; the pins aren't dedicated to be serial ports (they could be
> GPIOs and probably other functions too depending on the pinmux
> settings), and different people will connect different hardware or
> none at all. Hence, the default DT should not enable IO controllers
> for the pins on the expansion connector. The best approach these days
> is to use DT overlays. You may need to add some "hooks" into the Tegra
> base DT files to allow overlays to be used; they weren't written with
> overlays in mind.
If the argument is that those pins could also differently be used, then
the same would also apply to I2C busses, which are apparently _all_
exported in the current device tree, even if no hardware is connected by
default. Those pins are also available on the expansion headers.

You are right that those pins of the CPU can generally be used as GPIOs,
but regarding the Jetson TK1 board, the meaning of those pins on the
expansion header is the UART functionality. This is also what the
official documentation of Nvidia states on those pins [2]. So if you do
want to have a special non-UART setup, then you should have to disable
them on your own, by default they should be enabled.

On the Jetson TK1 board, only GPIO_PU* and GPIO_PH* should be used as
GPIOs [2].

  Ralf

[1]
http://developer.download.nvidia.com/embedded/jetson/TK1/2014-03-24/JetsonTK1_ModuleSpecification_PM375_V1.0.pdf
[2] http://elinux.org/Jetson/GPIO#Jetson_TK1_GPIO_pinouts
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Ralf Ramsauer
GPG: 0x8F10049B

Re: [PATCH 2/2] ARM: tegra: Add high speed UARTs to Jetson TK1 device tree

[Stephen Warren]

On 01/19/2016 10:16 AM, Ralf Ramsauer wrote:
> Thanks for review, Stephen.
>
> On 01/19/16 17:32, Stephen Warren wrote:
>> On 01/19/2016 06:12 AM, Ralf Ramsauer wrote:
>>> This patch enables the APB DMA high speed UARTs of the Jetson TK1.
>>
>> In the cover letter, you mentioned that these ports are exposed on the
>> expansion connector. It'd be useful to mention that fact here too,
>> since the cover letter doesn't make it into the git commit log.
> Good point.
>>
>> However, given that, I don't think we should apply this patch. We
>> can't know ahead of time what hardware is connected to the expansion
>> header; the pins aren't dedicated to be serial ports (they could be
>> GPIOs and probably other functions too depending on the pinmux
>> settings), and different people will connect different hardware or
>> none at all. Hence, the default DT should not enable IO controllers
>> for the pins on the expansion connector. The best approach these days
>> is to use DT overlays. You may need to add some "hooks" into the Tegra
>> base DT files to allow overlays to be used; they weren't written with
>> overlays in mind.
 >
> If the argument is that those pins could also differently be used, then
> the same would also apply to I2C busses, which are apparently _all_
> exported in the current device tree, even if no hardware is connected by
> default. Those pins are also available on the expansion headers.
>
> You are right that those pins of the CPU can generally be used as GPIOs,
> but regarding the Jetson TK1 board, the meaning of those pins on the
> expansion header is the UART functionality. This is also what the
> official documentation of Nvidia states on those pins [2]. So if you do
> want to have a special non-UART setup, then you should have to disable
> them on your own, by default they should be enabled.
>
> On the Jetson TK1 board, only GPIO_PU* and GPIO_PH* should be used as
> GPIOs [2].

Sorry, you're correct for this board. On other board I've dealt with 
(e.g. RPi, and NVIDIA's more recent Jetson TX1) the designers have been 
more careful not to commit any of the expansion pins to a particular use 
without end-user configuration.

I'll go back and re-review this commit with this in mind.

Re: [PATCH 2/2] ARM: tegra: Add high speed UARTs to Jetson TK1 device tree

[Stephen Warren]

On 01/19/2016 06:12 AM, Ralf Ramsauer wrote:
> This patch enables the APB DMA high speed UARTs of the Jetson TK1.

> diff --git a/arch/arm/boot/dts/tegra124-jetson-tk1.dts b/arch/arm/boot/dts/tegra124-jetson-tk1.dts

> +	/* First high speed UART */
> +	serial@0,70006000 {
> +		compatible = "nvidia,tegra124-hsuart", "nvidia,tegra30-hsuart";
> +		status = "okay";
> +	};

It would be nice if the comment described the HW connectivity, i.e. 
which signals the UART was connected to on the board, just like the 
comments for other IO controllers already enabled in the DT file. I'd 
suggest replacing the comment above with:

/* Expansion BR_UART1_RXD/_TXD */

> +	/* Second high speed UART */
> +	serial@0,70006040 {
> +		compatible = "nvidia,tegra124-hsuart", "nvidia,tegra30-hsuart";
> +		status = "okay";
> +	};

... and that commetn with:

/* Expansion UART2_RXD/_TXD/_RTS/_CTS */

> +
> +	/* Third high speed UART */
> +	serial@0,70006200 {
> +		compatible = "nvidia,tegra124-hsuart", "nvidia,tegra30-hsuart";
> +		status = "okay";
> +	};

That UART doesn't seem to be used at all according to the schematics and 
pinmux spreadsheet. Do you have any reference to the contrary aside from 
the L4T DT file? I believe it shouldn't be enabled.

Re: [PATCH 2/2] ARM: tegra: Add high speed UARTs to Jetson TK1 device tree

[Ralf Ramsauer]

Hi Stephen,

On 01/21/16 00:13, Stephen Warren wrote:
> On 01/19/2016 06:12 AM, Ralf Ramsauer wrote:
>> This patch enables the APB DMA high speed UARTs of the Jetson TK1.
>
>> diff --git a/arch/arm/boot/dts/tegra124-jetson-tk1.dts
>> b/arch/arm/boot/dts/tegra124-jetson-tk1.dts
>
>> +    /* First high speed UART */
>> +    serial@0,70006000 {
>> +        compatible = "nvidia,tegra124-hsuart", "nvidia,tegra30-hsuart";
>> +        status = "okay";
>> +    };
>
> It would be nice if the comment described the HW connectivity, i.e.
> which signals the UART was connected to on the board, just like the
> comments for other IO controllers already enabled in the DT file. I'd
> suggest replacing the comment above with:
>
> /* Expansion BR_UART1_RXD/_TXD */
I'll be more verbose with commenting in the next round
>
>> +    /* Second high speed UART */
>> +    serial@0,70006040 {
>> +        compatible = "nvidia,tegra124-hsuart", "nvidia,tegra30-hsuart";
>> +        status = "okay";
>> +    };
>
> ... and that commetn with:
>
> /* Expansion UART2_RXD/_TXD/_RTS/_CTS */
Ack
>
>> +
>> +    /* Third high speed UART */
>> +    serial@0,70006200 {
>> +        compatible = "nvidia,tegra124-hsuart", "nvidia,tegra30-hsuart";
>> +        status = "okay";
>> +    };
>
> That UART doesn't seem to be used at all according to the schematics
> and pinmux spreadsheet. Do you have any reference to the contrary
> aside from the L4T DT file? I believe it shouldn't be enabled.
Just checked it, and yes, you're absolutely right. I used the L4T device
tree as reference. Can't tell you why, but it appears that they
activated all uarts.
This is a short excerpt of Nvidias official DT:

            serial@70006000 {
                    compatible = "nvidia,tegra114-hsuart";
                    status = "okay";
            };

            serial@70006040 {
                    compatible = "nvidia,tegra114-hsuart";
                    status = "okay";
            };

            serial@70006200 {
                    compatible = "nvidia,tegra114-hsuart";
                    status = "okay";
            };

        serial@70006300 {
            compatible = "nvidia,tegra20-uart", "nvidia,tegra114-hsuart";
            console-port;
            sqa-automation-port;
            status = "okay";
        };


This is a short excerpt of /proc/iomem of a Jetson TK1 running latest
stock L4T linux:

    70006000-7000603f : /serial@70006000
    70006040-7000607f : /serial@70006040
    70006200-7000623f : /serial@70006200
    70006300-7000631f : serial

Let me doublecheck that again - in fact only two of three high speed
UARTs are actually exposed to the expansion header. (besides the strange
fact, that nvidia seems to have all uarts enabled...)

According to [1]:
uart1 - expansion connector
uart2 - expansion connector
uart3 - ???
uart4 - DB9 RS232 connector

I'll check this in a few days again. Thanks for review, Stephen!

Ralf

[1]
http://developer.download.nvidia.com/embedded/jetson/TK1/2014-03-24/JetsonTK1_ModuleSpecification_PM375_V1.0.pdf

-- 
Ralf Ramsauer
GPG: 0x8F10049B

Re: [PATCH 2/2] ARM: tegra: Add high speed UARTs to Jetson TK1 device tree

[Ralf Ramsauer]

Hi,

Now it's for sure: According to the TK1 schematics [1], uart3 is not
connected. Hence it makes no sense to enable it.
I'll prepare a next patch round.

Cheers
  Ralf

[1]
https://developer.nvidia.com/rdp/assets/jetson-tk1-datasheet-orcad-schematics
(warning, restricted access)

On 01/21/16 00:13, Stephen Warren wrote:
> On 01/19/2016 06:12 AM, Ralf Ramsauer wrote:
>> This patch enables the APB DMA high speed UARTs of the Jetson TK1.
>
>> diff --git a/arch/arm/boot/dts/tegra124-jetson-tk1.dts
>> b/arch/arm/boot/dts/tegra124-jetson-tk1.dts
>
>> +    /* First high speed UART */
>> +    serial@0,70006000 {
>> +        compatible = "nvidia,tegra124-hsuart", "nvidia,tegra30-hsuart";
>> +        status = "okay";
>> +    };
>
> It would be nice if the comment described the HW connectivity, i.e.
> which signals the UART was connected to on the board, just like the
> comments for other IO controllers already enabled in the DT file. I'd
> suggest replacing the comment above with:
>
> /* Expansion BR_UART1_RXD/_TXD */
>
>> +    /* Second high speed UART */
>> +    serial@0,70006040 {
>> +        compatible = "nvidia,tegra124-hsuart", "nvidia,tegra30-hsuart";
>> +        status = "okay";
>> +    };
>
> ... and that commetn with:
>
> /* Expansion UART2_RXD/_TXD/_RTS/_CTS */
>
>> +
>> +    /* Third high speed UART */
>> +    serial@0,70006200 {
>> +        compatible = "nvidia,tegra124-hsuart", "nvidia,tegra30-hsuart";
>> +        status = "okay";
>> +    };
>
> That UART doesn't seem to be used at all according to the schematics
> and pinmux spreadsheet. Do you have any reference to the contrary
> aside from the L4T DT file? I believe it shouldn't be enabled.
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Ralf Ramsauer
GPG: 0x8F10049B