diff mbox

ARM: tegra: dts: add aliases and DMA requestor for serial controller

Message ID 1355898671-3960-1-git-send-email-ldewangan@nvidia.com (mailing list archive)
State New, archived
Headers show

Commit Message

Laxman Dewangan Dec. 19, 2012, 6:31 a.m. UTC
Add APB DMA requestor and serial aliases for serial controller.
There will be two serial driver i.e. 8250 based simple serial driver
and APB DMA based serial driver for higher baudrate and performace.

The simple serial driver get enabled with compatible nvidia,tegra20-uart
and APB DMA based driver will get enabled with compatible
nvidia,tegra20-hsuart.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 arch/arm/boot/dts/tegra20.dtsi |   30 +++++++++++++++++++++++++-----
 arch/arm/boot/dts/tegra30.dtsi |   31 ++++++++++++++++++++++++++-----
 2 files changed, 51 insertions(+), 10 deletions(-)

Comments

Stephen Warren Dec. 19, 2012, 5 p.m. UTC | #1
On 12/18/2012 11:31 PM, Laxman Dewangan wrote:
> Add APB DMA requestor and serial aliases for serial controller.
> There will be two serial driver i.e. 8250 based simple serial driver
> and APB DMA based serial driver for higher baudrate and performace.
> 
> The simple serial driver get enabled with compatible nvidia,tegra20-uart
> and APB DMA based driver will get enabled with compatible
> nvidia,tegra20-hsuart.

This patch seems to do two things:

1) Add a comment describing how to select the basic or DMA-based driver.
I can see why this is useful.

2) Add aliases for the serial ports. I have no idea why this is useful.
Laxman Dewangan Dec. 20, 2012, 6:15 a.m. UTC | #2
On Wednesday 19 December 2012 10:30 PM, Stephen Warren wrote:
> On 12/18/2012 11:31 PM, Laxman Dewangan wrote:
>> Add APB DMA requestor and serial aliases for serial controller.
>> There will be two serial driver i.e. 8250 based simple serial driver
>> and APB DMA based serial driver for higher baudrate and performace.
>>
>> The simple serial driver get enabled with compatible nvidia,tegra20-uart
>> and APB DMA based driver will get enabled with compatible
>> nvidia,tegra20-hsuart.
> This patch seems to do two things:
>
> 1) Add a comment describing how to select the basic or DMA-based driver.
> I can see why this is useful.
>
> 2) Add aliases for the serial ports. I have no idea why this is useful.

Serial aliases are added because we want to get the port number like 
/dev/ttyTHS0,  /d/ttyTHS2 etc. The uarta should be /dev/ttyTHS0 and 
uartc should be /dev/ttyTHS2. The port number can be found using aliases 
and of_alias_get_id() in driver file:

port_number = of_alias_get_id(np, "serial");
Stephen Warren Dec. 20, 2012, 5:03 p.m. UTC | #3
On 12/19/2012 11:15 PM, Laxman Dewangan wrote:
> On Wednesday 19 December 2012 10:30 PM, Stephen Warren wrote:
>> On 12/18/2012 11:31 PM, Laxman Dewangan wrote:
>>> Add APB DMA requestor and serial aliases for serial controller.
>>> There will be two serial driver i.e. 8250 based simple serial driver
>>> and APB DMA based serial driver for higher baudrate and performace.
>>>
>>> The simple serial driver get enabled with compatible nvidia,tegra20-uart
>>> and APB DMA based driver will get enabled with compatible
>>> nvidia,tegra20-hsuart.
>> This patch seems to do two things:
>>
>> 1) Add a comment describing how to select the basic or DMA-based driver.
>> I can see why this is useful.
>>
>> 2) Add aliases for the serial ports. I have no idea why this is useful.
> 
> Serial aliases are added because we want to get the port number like
> /dev/ttyTHS0,  /d/ttyTHS2 etc. The uarta should be /dev/ttyTHS0 and
> uartc should be /dev/ttyTHS2. The port number can be found using aliases
> and of_alias_get_id() in driver file:
> 
> port_number = of_alias_get_id(np, "serial");

Oh, I see. I guess the aliases make sense then.

But, wouldn't you only add aliases for the specific UARTs that are in
use on a particular board (so put them in tegraNN-board.dts) rather than
all UARTs on Tegra (i.e. the current patch which puts them into
tegraNN.dtsi)?
Laxman Dewangan Dec. 21, 2012, 4:18 a.m. UTC | #4
On Thursday 20 December 2012 10:33 PM, Stephen Warren wrote:
> On 12/19/2012 11:15 PM, Laxman Dewangan wrote:
>> On Wednesday 19 December 2012 10:30 PM, Stephen Warren wrote:
>>> On 12/18/2012 11:31 PM, Laxman Dewangan wrote:
>>>> Add APB DMA requestor and serial aliases for serial controller.
>>>> There will be two serial driver i.e. 8250 based simple serial driver
>>>> and APB DMA based serial driver for higher baudrate and performace.
>>>>
>>>> The simple serial driver get enabled with compatible nvidia,tegra20-uart
>>>> and APB DMA based driver will get enabled with compatible
>>>> nvidia,tegra20-hsuart.
>>> This patch seems to do two things:
>>>
>>> 1) Add a comment describing how to select the basic or DMA-based driver.
>>> I can see why this is useful.
>>>
>>> 2) Add aliases for the serial ports. I have no idea why this is useful.
>> Serial aliases are added because we want to get the port number like
>> /dev/ttyTHS0,  /d/ttyTHS2 etc. The uarta should be /dev/ttyTHS0 and
>> uartc should be /dev/ttyTHS2. The port number can be found using aliases
>> and of_alias_get_id() in driver file:
>>
>> port_number = of_alias_get_id(np, "serial");
> Oh, I see. I guess the aliases make sense then.
>
> But, wouldn't you only add aliases for the specific UARTs that are in
> use on a particular board (so put them in tegraNN-board.dts) rather than
> all UARTs on Tegra (i.e. the current patch which puts them into
> tegraNN.dtsi)?

I like to have this in tegraN N.dtsi file becasue:
- This policy is not going to change the board to board. For high speed 
driver the uarta will be ttyTHS0 and uartb ttyTHS1.
- In board file I will only enable driver and make compatible, not worry 
about the making aliases.

This approach will simplify the instantiation. This will also avoid 
confusion like the aliases get added or not when we create board dts file.
This will be on same line we have with board files where pdev->id is 
fixed for all board in common place i.e. device.c which decide the 
port-number.
Stephen Warren Dec. 21, 2012, 8:50 p.m. UTC | #5
On 12/20/2012 09:18 PM, Laxman Dewangan wrote:
> On Thursday 20 December 2012 10:33 PM, Stephen Warren wrote:
>> On 12/19/2012 11:15 PM, Laxman Dewangan wrote:
>>> On Wednesday 19 December 2012 10:30 PM, Stephen Warren wrote:
>>>> On 12/18/2012 11:31 PM, Laxman Dewangan wrote:
>>>>> Add APB DMA requestor and serial aliases for serial controller.
>>>>> There will be two serial driver i.e. 8250 based simple serial driver
>>>>> and APB DMA based serial driver for higher baudrate and performace.
...
>>>> This patch seems to do two things:
...
>>>> 2) Add aliases for the serial ports. I have no idea why this is useful.
>>> Serial aliases are added because we want to get the port number like
>>> /dev/ttyTHS0,  /d/ttyTHS2 etc. The uarta should be /dev/ttyTHS0 and
>>> uartc should be /dev/ttyTHS2. The port number can be found using aliases
>>> and of_alias_get_id() in driver file:
>>>
>>> port_number = of_alias_get_id(np, "serial");
>>
>> Oh, I see. I guess the aliases make sense then.
>>
>> But, wouldn't you only add aliases for the specific UARTs that are in
>> use on a particular board (so put them in tegraNN-board.dts) rather than
>> all UARTs on Tegra (i.e. the current patch which puts them into
>> tegraNN.dtsi)?
> 
> I like to have this in tegraN N.dtsi file becasue:
> - This policy is not going to change the board to board. For high speed
> driver the uarta will be ttyTHS0 and uartb ttyTHS1.

Sure it is. Since the board files select which UARTs use the low-speed
driver vs. the high-speed driver by overriding the compatible value,
each board will have a different subset of UARTs which use the high
speed driver and hence which use the aliases. Don't you want to end up
with /dev/ttyHS0 on each board, rather than ending up with a "random"
device node name depending on which UART the board designer picked for
the high speed device?
Laxman Dewangan Dec. 22, 2012, 8:15 p.m. UTC | #6
On Saturday 22 December 2012 02:20 AM, Stephen Warren wrote:
> On 12/20/2012 09:18 PM, Laxman Dewangan wrote:
>> On Thursday 20 December 2012 10:33 PM, Stephen Warren wrote:
>>> On 12/19/2012 11:15 PM, Laxman Dewangan wrote:
>>>> On Wednesday 19 December 2012 10:30 PM, Stephen Warren wrote:
>>>>> On 12/18/2012 11:31 PM, Laxman Dewangan wrote:
>>>>>> Add APB DMA requestor and serial aliases for serial controller.
>>>>>> There will be two serial driver i.e. 8250 based simple serial driver
>>>>>> and APB DMA based serial driver for higher baudrate and performace.
> ...
>>>>> This patch seems to do two things:
> ...
>>>>> 2) Add aliases for the serial ports. I have no idea why this is useful.
>>>> Serial aliases are added because we want to get the port number like
>>>> /dev/ttyTHS0,  /d/ttyTHS2 etc. The uarta should be /dev/ttyTHS0 and
>>>> uartc should be /dev/ttyTHS2. The port number can be found using aliases
>>>> and of_alias_get_id() in driver file:
>>>>
>>>> port_number = of_alias_get_id(np, "serial");
>>> Oh, I see. I guess the aliases make sense then.
>>>
>>> But, wouldn't you only add aliases for the specific UARTs that are in
>>> use on a particular board (so put them in tegraNN-board.dts) rather than
>>> all UARTs on Tegra (i.e. the current patch which puts them into
>>> tegraNN.dtsi)?
>> I like to have this in tegraN N.dtsi file becasue:
>> - This policy is not going to change the board to board. For high speed
>> driver the uarta will be ttyTHS0 and uartb ttyTHS1.
> Sure it is. Since the board files select which UARTs use the low-speed
> driver vs. the high-speed driver by overriding the compatible value,
> each board will have a different subset of UARTs which use the high
> speed driver and hence which use the aliases. Don't you want to end up
> with /dev/ttyHS0 on each board, rather than ending up with a "random"
> device node name depending on which UART the board designer picked for
> the high speed device?

We had this in past where we started the high speed driver always from 0 
like in one board uartc become ttyHS0 and another board uarta become 
ttyHS0 and uartc become ttyHS1.
This confused more to other team (BT and modem team) as they need to 
know all uart instance and their corresponding driver.  So we kept uarta 
will be always 0 and uartc will be always 2. This way it is straight 
forward for the uart user to directly see the uart instance on board and 
decide the port number rather than going to what are other uart and 
their corresponding driver.


So we kept the
Laxman Dewangan Dec. 22, 2012, 8:15 p.m. UTC | #7
On Saturday 22 December 2012 02:20 AM, Stephen Warren wrote:
> On 12/20/2012 09:18 PM, Laxman Dewangan wrote:
>> On Thursday 20 December 2012 10:33 PM, Stephen Warren wrote:
>>> On 12/19/2012 11:15 PM, Laxman Dewangan wrote:
>>>> On Wednesday 19 December 2012 10:30 PM, Stephen Warren wrote:
>>>>> On 12/18/2012 11:31 PM, Laxman Dewangan wrote:
>>>>>> Add APB DMA requestor and serial aliases for serial controller.
>>>>>> There will be two serial driver i.e. 8250 based simple serial driver
>>>>>> and APB DMA based serial driver for higher baudrate and performace.
> ...
>>>>> This patch seems to do two things:
> ...
>>>>> 2) Add aliases for the serial ports. I have no idea why this is useful.
>>>> Serial aliases are added because we want to get the port number like
>>>> /dev/ttyTHS0,  /d/ttyTHS2 etc. The uarta should be /dev/ttyTHS0 and
>>>> uartc should be /dev/ttyTHS2. The port number can be found using aliases
>>>> and of_alias_get_id() in driver file:
>>>>
>>>> port_number = of_alias_get_id(np, "serial");
>>> Oh, I see. I guess the aliases make sense then.
>>>
>>> But, wouldn't you only add aliases for the specific UARTs that are in
>>> use on a particular board (so put them in tegraNN-board.dts) rather than
>>> all UARTs on Tegra (i.e. the current patch which puts them into
>>> tegraNN.dtsi)?
>> I like to have this in tegraN N.dtsi file becasue:
>> - This policy is not going to change the board to board. For high speed
>> driver the uarta will be ttyTHS0 and uartb ttyTHS1.
> Sure it is. Since the board files select which UARTs use the low-speed
> driver vs. the high-speed driver by overriding the compatible value,
> each board will have a different subset of UARTs which use the high
> speed driver and hence which use the aliases. Don't you want to end up
> with /dev/ttyHS0 on each board, rather than ending up with a "random"
> device node name depending on which UART the board designer picked for
> the high speed device?

We had this in past where we started the high speed driver always from 0 
like in one board uartc become ttyHS0 and another board uarta become 
ttyHS0 and uartc become ttyHS1.
This confused more to other team (BT and modem team) as they need to 
know all uart instance and their corresponding driver.  So we kept uarta 
will be always 0 and uartc will be always 2. This way it is straight 
forward for the uart user to directly see the uart instance on board and 
decide the port number rather than going to what are other uart and 
their corresponding driver.


So we kept the
Stephen Warren Dec. 25, 2012, 5:33 a.m. UTC | #8
On 12/22/2012 01:15 PM, Laxman Dewangan wrote:
> On Saturday 22 December 2012 02:20 AM, Stephen Warren wrote:
>> On 12/20/2012 09:18 PM, Laxman Dewangan wrote:
>>> On Thursday 20 December 2012 10:33 PM, Stephen Warren wrote:
>>>> On 12/19/2012 11:15 PM, Laxman Dewangan wrote:
>>>>> On Wednesday 19 December 2012 10:30 PM, Stephen Warren wrote:
>>>>>> On 12/18/2012 11:31 PM, Laxman Dewangan wrote:
>>>>>>> Add APB DMA requestor and serial aliases for serial controller.
>>>>>>> There will be two serial driver i.e. 8250 based simple serial driver
>>>>>>> and APB DMA based serial driver for higher baudrate and performace.
>> ...
>>>>>> This patch seems to do two things:
>> ...
>>>>>> 2) Add aliases for the serial ports. I have no idea why this is
>>>>>> useful.
>>>>> Serial aliases are added because we want to get the port number like
>>>>> /dev/ttyTHS0,  /d/ttyTHS2 etc. The uarta should be /dev/ttyTHS0 and
>>>>> uartc should be /dev/ttyTHS2. The port number can be found using
>>>>> aliases
>>>>> and of_alias_get_id() in driver file:
>>>>>
>>>>> port_number = of_alias_get_id(np, "serial");
>>>> Oh, I see. I guess the aliases make sense then.
>>>>
>>>> But, wouldn't you only add aliases for the specific UARTs that are in
>>>> use on a particular board (so put them in tegraNN-board.dts) rather
>>>> than
>>>> all UARTs on Tegra (i.e. the current patch which puts them into
>>>> tegraNN.dtsi)?
>>> I like to have this in tegraN N.dtsi file becasue:
>>> - This policy is not going to change the board to board. For high speed
>>> driver the uarta will be ttyTHS0 and uartb ttyTHS1.
>> Sure it is. Since the board files select which UARTs use the low-speed
>> driver vs. the high-speed driver by overriding the compatible value,
>> each board will have a different subset of UARTs which use the high
>> speed driver and hence which use the aliases. Don't you want to end up
>> with /dev/ttyHS0 on each board, rather than ending up with a "random"
>> device node name depending on which UART the board designer picked for
>> the high speed device?
> 
> We had this in past where we started the high speed driver always from 0
> like in one board uartc become ttyHS0 and another board uarta become
> ttyHS0 and uartc become ttyHS1.
> This confused more to other team (BT and modem team) as they need to
> know all uart instance and their corresponding driver.  So we kept uarta

That doesn't really make sense; The BT and MODEM team should only care
about the specific UARTs they use.

> will be always 0 and uartc will be always 2.

That seems a little unlikely; why can't different boards use different
UARTs for different purposes; after all, UARTA is often a debug UART not
a BT or MODEM UART.

> This way it is straight
> forward for the uart user to directly see the uart instance on board and
> decide the port number rather than going to what are other uart and
> their corresponding driver.

Still, there is some precedent in the kernel for your approach. So, I
guess we can go ahead with using the aliases as-is.

Your approach breaks down in the following scenarios though:

a) Somebody (random customer board designer or SW engineer, who NVIDIA
can't control) wants the console UART to always have alias serial0;
there are examples of this in the kernel. On just the boards we support
upstream, some use UART A for the debug/console and some use UART D. I
know of others that use UART B.

b) The board contains UARTs (e.g. PCIe) that aren't the Tegra-internal
UARTs, so the alias IDs can't all map to Tegra UART IDs.

c) Conceptually, the purpose of aliases seems to be to provide useful
names (numbers) for the relevant ports, and naming/numbering
non-existent ports doesn't seem particularly useful.
Laxman Dewangan Dec. 25, 2012, 9:43 a.m. UTC | #9
On Tuesday 25 December 2012 11:03 AM, Stephen Warren wrote:
> On 12/22/2012 01:15 PM, Laxman Dewangan wrote:
>
>> This way it is straight
>> forward for the uart user to directly see the uart instance on board and
>> decide the port number rather than going to what are other uart and
>> their corresponding driver.
> Still, there is some precedent in the kernel for your approach. So, I
> guess we can go ahead with using the aliases as-is.
>

So can I assume this patch is fine and I need not to re-spin it for 
removing aliases?
Stephen Warren Jan. 2, 2013, 7:19 p.m. UTC | #10
On 12/18/2012 11:31 PM, Laxman Dewangan wrote:
> Add APB DMA requestor and serial aliases for serial controller.
> There will be two serial driver i.e. 8250 based simple serial driver
> and APB DMA based serial driver for higher baudrate and performace.
> 
> The simple serial driver get enabled with compatible nvidia,tegra20-uart
> and APB DMA based driver will get enabled with compatible
> nvidia,tegra20-hsuart.

I've applied this to Tegra's for-3.9/dt branch.
diff mbox

Patch

diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index b8effa1..fe35c72 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -4,6 +4,14 @@ 
 	compatible = "nvidia,tegra20";
 	interrupt-parent = <&intc>;
 
+	aliases {
+		serial0 = &uarta;
+		serial1 = &uartb;
+		serial2 = &uartc;
+		serial3 = &uartd;
+		serial4 = &uarte;
+	};
+
 	host1x {
 		compatible = "nvidia,tegra20-host1x", "simple-bus";
 		reg = <0x50000000 0x00024000>;
@@ -194,43 +202,55 @@ 
 		status = "disabled";
 	};
 
-	serial@70006000 {
+	/*
+	 * There are two serial driver i.e. 8250 based simple serial
+	 * driver and APB DMA based serial driver for higher baudrate
+	 * and performace. To enable the 8250 based driver, the compatible
+	 * is "nvidia,tegra20-uart" and to enable the APB DMA based serial
+	 * driver, the comptible is "nvidia,tegra20-hsuart".
+	 */
+	uarta: serial@70006000 {
 		compatible = "nvidia,tegra20-uart";
 		reg = <0x70006000 0x40>;
 		reg-shift = <2>;
 		interrupts = <0 36 0x04>;
+		nvidia,dma-request-selector = <&apbdma 8>;
 		status = "disabled";
 	};
 
-	serial@70006040 {
+	uartb: serial@70006040 {
 		compatible = "nvidia,tegra20-uart";
 		reg = <0x70006040 0x40>;
 		reg-shift = <2>;
 		interrupts = <0 37 0x04>;
+		nvidia,dma-request-selector = <&apbdma 9>;
 		status = "disabled";
 	};
 
-	serial@70006200 {
+	uartc: serial@70006200 {
 		compatible = "nvidia,tegra20-uart";
 		reg = <0x70006200 0x100>;
 		reg-shift = <2>;
 		interrupts = <0 46 0x04>;
+		nvidia,dma-request-selector = <&apbdma 10>;
 		status = "disabled";
 	};
 
-	serial@70006300 {
+	uartd: serial@70006300 {
 		compatible = "nvidia,tegra20-uart";
 		reg = <0x70006300 0x100>;
 		reg-shift = <2>;
 		interrupts = <0 90 0x04>;
+		nvidia,dma-request-selector = <&apbdma 19>;
 		status = "disabled";
 	};
 
-	serial@70006400 {
+	uarte: serial@70006400 {
 		compatible = "nvidia,tegra20-uart";
 		reg = <0x70006400 0x100>;
 		reg-shift = <2>;
 		interrupts = <0 91 0x04>;
+		nvidia,dma-request-selector = <&apbdma 20>;
 		status = "disabled";
 	};
 
diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
index 529fdb8..80a0edf 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -4,6 +4,14 @@ 
 	compatible = "nvidia,tegra30";
 	interrupt-parent = <&intc>;
 
+	aliases {
+		serial0 = &uarta;
+		serial1 = &uartb;
+		serial2 = &uartc;
+		serial3 = &uartd;
+		serial4 = &uarte;
+	};
+
 	host1x {
 		compatible = "nvidia,tegra30-host1x", "simple-bus";
 		reg = <0x50000000 0x00024000>;
@@ -190,43 +198,56 @@ 
 		       0x70003000 0x3e4>; /* Mux registers */
 	};
 
-	serial@70006000 {
+	/*
+	 * There are two serial driver i.e. 8250 based simple serial
+	 * driver and APB DMA based serial driver for higher baudrate
+	 * and performace. To enable the 8250 based driver, the compatible
+	 * is "nvidia,tegra30-uart", "nvidia,tegra20-uart" and to enable
+	 * the APB DMA based serial driver, the comptible is
+	 * "nvidia,tegra30-hsuart", "nvidia,tegra20-hsuart".
+	 */
+	uarta: serial@70006000 {
 		compatible = "nvidia,tegra30-uart", "nvidia,tegra20-uart";
 		reg = <0x70006000 0x40>;
 		reg-shift = <2>;
 		interrupts = <0 36 0x04>;
+		nvidia,dma-request-selector = <&apbdma 8>;
 		status = "disabled";
 	};
 
-	serial@70006040 {
+	uartb: serial@70006040 {
 		compatible = "nvidia,tegra30-uart", "nvidia,tegra20-uart";
 		reg = <0x70006040 0x40>;
 		reg-shift = <2>;
 		interrupts = <0 37 0x04>;
+		nvidia,dma-request-selector = <&apbdma 9>;
 		status = "disabled";
 	};
 
-	serial@70006200 {
+	uartc: serial@70006200 {
 		compatible = "nvidia,tegra30-uart", "nvidia,tegra20-uart";
 		reg = <0x70006200 0x100>;
 		reg-shift = <2>;
 		interrupts = <0 46 0x04>;
+		nvidia,dma-request-selector = <&apbdma 10>;
 		status = "disabled";
 	};
 
-	serial@70006300 {
+	uartd: serial@70006300 {
 		compatible = "nvidia,tegra30-uart", "nvidia,tegra20-uart";
 		reg = <0x70006300 0x100>;
 		reg-shift = <2>;
 		interrupts = <0 90 0x04>;
+		nvidia,dma-request-selector = <&apbdma 19>;
 		status = "disabled";
 	};
 
-	serial@70006400 {
+	uarte: serial@70006400 {
 		compatible = "nvidia,tegra30-uart", "nvidia,tegra20-uart";
 		reg = <0x70006400 0x100>;
 		reg-shift = <2>;
 		interrupts = <0 91 0x04>;
+		nvidia,dma-request-selector = <&apbdma 20>;
 		status = "disabled";
 	};