| Message ID | 1363204194-19487-4-git-send-email-ldewangan@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 03/13/2013 01:49 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,tegra114-uart", > "nvidia,tegra20-uart" and APB DMA based driver will get enabled with > compatible "nvidia,tegra114-hsuart", "nvidia,tegra30-hsuart". > + /* > + * 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,tegra114-uart", "nvidia,tegra20-uart" and to enable > + * the APB DMA based serial driver, the comptible is > + * "nvidia,tegra114-hsuart", "nvidia,tegra30-hsuart". > + */ Again, that text says you want either of: compatible = "nvidia,tegra114-uart", "nvidia,tegra20-uart"; compatible = "nvidia,tegra114-hsuart", "nvidia,tegra30-hsuart"; (note Tegra20-vs-Tegra30 in the second compatible value) Why isn't it instead: compatible = "nvidia,tegra114-uart", "nvidia,tegra20-uart"; compatible = "nvidia,tegra114-hsuart", "nvidia,tegra20-hsuart"; (note both second compatible values say Tegra20) I assume this is a typo. I suppose I can fix this up when I apply it to avoid a resend, assuming it's wrong.
On 03/13/2013 02:02 PM, Stephen Warren wrote: > On 03/13/2013 01:49 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,tegra114-uart", >> "nvidia,tegra20-uart" and APB DMA based driver will get enabled with >> compatible "nvidia,tegra114-hsuart", "nvidia,tegra30-hsuart". > >> + /* >> + * 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,tegra114-uart", "nvidia,tegra20-uart" and to enable >> + * the APB DMA based serial driver, the comptible is >> + * "nvidia,tegra114-hsuart", "nvidia,tegra30-hsuart". >> + */ > > Again, that text says you want either of: > > compatible = "nvidia,tegra114-uart", "nvidia,tegra20-uart"; > compatible = "nvidia,tegra114-hsuart", "nvidia,tegra30-hsuart"; > > (note Tegra20-vs-Tegra30 in the second compatible value) > > Why isn't it instead: > > compatible = "nvidia,tegra114-uart", "nvidia,tegra20-uart"; > compatible = "nvidia,tegra114-hsuart", "nvidia,tegra20-hsuart"; > > (note both second compatible values say Tegra20) > > I assume this is a typo. > > I suppose I can fix this up when I apply it to avoid a resend, assuming > it's wrong. Since I haven't seen a reply to this, when I apply this, I'm going to change the comment I quoted above to match the values I wrote above under "why isn't it instead:".
On Friday 15 March 2013 11:56 PM, Stephen Warren wrote: > On 03/13/2013 02:02 PM, Stephen Warren wrote: >> On 03/13/2013 01:49 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,tegra114-uart", >>> "nvidia,tegra20-uart" and APB DMA based driver will get enabled with >>> compatible "nvidia,tegra114-hsuart", "nvidia,tegra30-hsuart". >>> + /* >>> + * 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,tegra114-uart", "nvidia,tegra20-uart" and to enable >>> + * the APB DMA based serial driver, the comptible is >>> + * "nvidia,tegra114-hsuart", "nvidia,tegra30-hsuart". >>> + */ >> Again, that text says you want either of: >> >> compatible = "nvidia,tegra114-uart", "nvidia,tegra20-uart"; >> compatible = "nvidia,tegra114-hsuart", "nvidia,tegra30-hsuart"; >> >> (note Tegra20-vs-Tegra30 in the second compatible value) >> >> Why isn't it instead: >> >> compatible = "nvidia,tegra114-uart", "nvidia,tegra20-uart"; >> compatible = "nvidia,tegra114-hsuart", "nvidia,tegra20-hsuart"; >> >> (note both second compatible values say Tegra20) >> >> I assume this is a typo. >> >> I suppose I can fix this up when I apply it to avoid a resend, assuming >> it's wrong. > Since I haven't seen a reply to this, when I apply this, I'm going to > change the comment I quoted above to match the values I wrote above > under "why isn't it instead:". Stephen, Sorry, I missed your comment to reply. I mean was that compatible should be compatible = "nvidia,tegra114-uart", "nvidia,tegra20-uart"; compatible = "nvidia,tegra114-hsuart", "nvidia,tegra30-hsuart"; not compatible = "nvidia,tegra114-uart", "nvidia,tegra20-uart"; compatible = "nvidia,tegra114-hsuart", "nvidia,tegra20-hsuart"; The reason is that, tegra30 has the clock divider in the CAR register set and it is 15.1 which gives more precise baudrate. tegra20 does not have the same. Tegra114 also have the clock divider in the CAR register. All SoCs UART support 16.0 clock divider inside the uart controller as DLL/DLM. Simple uart driver use the uart clock divider and it is fine here. High speed uart driver uses the car register driver for better flexibility and better resolution.
On Friday 15 March 2013 11:56 PM, Stephen Warren wrote: > On 03/13/2013 02:02 PM, Stephen Warren wrote: >> On 03/13/2013 01:49 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,tegra114-uart", >>> "nvidia,tegra20-uart" and APB DMA based driver will get enabled with >>> compatible "nvidia,tegra114-hsuart", "nvidia,tegra30-hsuart". >>> + /* >>> + * 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,tegra114-uart", "nvidia,tegra20-uart" and to enable >>> + * the APB DMA based serial driver, the comptible is >>> + * "nvidia,tegra114-hsuart", "nvidia,tegra30-hsuart". >>> + */ >> Again, that text says you want either of: >> >> compatible = "nvidia,tegra114-uart", "nvidia,tegra20-uart"; >> compatible = "nvidia,tegra114-hsuart", "nvidia,tegra30-hsuart"; >> >> (note Tegra20-vs-Tegra30 in the second compatible value) >> >> Why isn't it instead: >> >> compatible = "nvidia,tegra114-uart", "nvidia,tegra20-uart"; >> compatible = "nvidia,tegra114-hsuart", "nvidia,tegra20-hsuart"; >> >> (note both second compatible values say Tegra20) >> >> I assume this is a typo. >> >> I suppose I can fix this up when I apply it to avoid a resend, assuming >> it's wrong. > Since I haven't seen a reply to this, when I apply this, I'm going to > change the comment I quoted above to match the values I wrote above > under "why isn't it instead:". Stephen, Sorry, I missed your comment to reply. I mean was that compatible should be compatible = "nvidia,tegra114-uart", "nvidia,tegra20-uart"; compatible = "nvidia,tegra114-hsuart", "nvidia,tegra30-hsuart"; not compatible = "nvidia,tegra114-uart", "nvidia,tegra20-uart"; compatible = "nvidia,tegra114-hsuart", "nvidia,tegra20-hsuart"; The reason is that, tegra30 has the clock divider in the CAR register set and it is 15.1 which gives more precise baudrate. tegra20 does not have the same. Tegra114 also have the clock divider in the CAR register. All SoCs UART support 16.0 clock divider inside the uart controller as DLL/DLM. Simple uart driver use the uart clock divider and it is fine here. High speed uart driver uses the car register driver for better flexibility and better resolution.
On 03/15/2013 12:42 PM, Laxman Dewangan wrote: > On Friday 15 March 2013 11:56 PM, Stephen Warren wrote: >> On 03/13/2013 02:02 PM, Stephen Warren wrote: >>> On 03/13/2013 01:49 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,tegra114-uart", >>>> "nvidia,tegra20-uart" and APB DMA based driver will get enabled with >>>> compatible "nvidia,tegra114-hsuart", "nvidia,tegra30-hsuart". >>>> + /* >>>> + * 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,tegra114-uart", "nvidia,tegra20-uart" and to enable >>>> + * the APB DMA based serial driver, the comptible is >>>> + * "nvidia,tegra114-hsuart", "nvidia,tegra30-hsuart". >>>> + */ >>> Again, that text says you want either of: >>> >>> compatible = "nvidia,tegra114-uart", "nvidia,tegra20-uart"; >>> compatible = "nvidia,tegra114-hsuart", "nvidia,tegra30-hsuart"; >>> >>> (note Tegra20-vs-Tegra30 in the second compatible value) >>> >>> Why isn't it instead: >>> >>> compatible = "nvidia,tegra114-uart", "nvidia,tegra20-uart"; >>> compatible = "nvidia,tegra114-hsuart", "nvidia,tegra20-hsuart"; >>> >>> (note both second compatible values say Tegra20) >>> >>> I assume this is a typo. >>> >>> I suppose I can fix this up when I apply it to avoid a resend, assuming >>> it's wrong. >> Since I haven't seen a reply to this, when I apply this, I'm going to >> change the comment I quoted above to match the values I wrote above >> under "why isn't it instead:". > > Stephen, > Sorry, I missed your comment to reply. > I mean was that compatible should be > compatible = "nvidia,tegra114-uart", "nvidia,tegra20-uart"; > compatible = "nvidia,tegra114-hsuart", "nvidia,tegra30-hsuart"; > > not > > compatible = "nvidia,tegra114-uart", "nvidia,tegra20-uart"; > compatible = "nvidia,tegra114-hsuart", "nvidia,tegra20-hsuart"; > > > The reason is that, tegra30 has the clock divider in the CAR register > set and it is 15.1 which gives more precise baudrate. tegra20 does not > have the same. > Tegra114 also have the clock divider in the CAR register. > > All SoCs UART support 16.0 clock divider inside the uart controller as > DLL/DLM. > > Simple uart driver use the uart clock divider and it is fine here. > > High speed uart driver uses the car register driver for better > flexibility and better resolution. OK, so I see that Tegra30 has an enhancement over Tegra20. However, given your description, that enhancement is optional; a driver could simply continue to use /just/ the in-UART divider, and ignore the CAR divider, and still work just fine, albeit with (entirely backwards-compatible) less accuracy than it might achieve if it used the new feature. As such, I think it's correct to mark the device as actually being compatible with all 3: 114 (precise HW model), 30 (base model w/ extra divider), 20 (base model that's compatible, albeit ignoring extra features). That might be a bit excessive though, so I guess I'll just go with the values in your patch. It'd be a good idea if you could post a follow-on patch that updates the DT binding to explain this, and then removes the comments from *.dtsi since this really should be explained in the binding document not the .dtsi files, I think. At most, I'd expect to see the following in the .dtsi files: These nodes can either be compatible with nvidia,tegra114-uart, or nvidia,tegra114-hsuart. See the bindings for details of the difference.
On Saturday 16 March 2013 12:28 AM, Stephen Warren wrote: > On 03/15/2013 12:42 PM, Laxman Dewangan wrote: >> >> >> >> Simple uart driver use the uart clock divider and it is fine here. >> >> High speed uart driver uses the car register driver for better >> flexibility and better resolution. > OK, so I see that Tegra30 has an enhancement over Tegra20. However, > given your description, that enhancement is optional; a driver could > simply continue to use /just/ the in-UART divider, and ignore the CAR > divider, and still work just fine, albeit with (entirely > backwards-compatible) less accuracy than it might achieve if it used the > new feature. > > As such, I think it's correct to mark the device as actually being > compatible with all 3: 114 (precise HW model), 30 (base model w/ extra > divider), 20 (base model that's compatible, albeit ignoring extra features). > > That might be a bit excessive though, so I guess I'll just go with the > values in your patch. It'd be a good idea if you could post a follow-on > patch that updates the DT binding to explain this, and then removes the > comments from *.dtsi since this really should be explained in the > binding document not the .dtsi files, I think. > > At most, I'd expect to see the following in the .dtsi files: > > These nodes can either be compatible with nvidia,tegra114-uart, or > nvidia,tegra114-hsuart. See the bindings for details of the difference. Sure, I will post a patch for this.
diff --git a/arch/arm/boot/dts/tegra114.dtsi b/arch/arm/boot/dts/tegra114.dtsi index ce44938..5a41d04 100644 --- a/arch/arm/boot/dts/tegra114.dtsi +++ b/arch/arm/boot/dts/tegra114.dtsi @@ -4,6 +4,13 @@ compatible = "nvidia,tegra114"; interrupt-parent = <&gic>; + aliases { + serial0 = &uarta; + serial1 = &uartb; + serial2 = &uartc; + serial3 = &uartd; + }; + gic: interrupt-controller { compatible = "arm,cortex-a15-gic"; #interrupt-cells = <3>; @@ -98,35 +105,47 @@ 0x70003000 0x40c>; /* 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,tegra114-uart", "nvidia,tegra20-uart" and to enable + * the APB DMA based serial driver, the comptible is + * "nvidia,tegra114-hsuart", "nvidia,tegra30-hsuart". + */ + uarta: serial@70006000 { compatible = "nvidia,tegra114-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,tegra114-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,tegra114-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,tegra114-uart", "nvidia,tegra20-uart"; reg = <0x70006300 0x100>; reg-shift = <2>; interrupts = <0 90 0x04>; + nvidia,dma-request-selector = <&apbdma 19>; status = "disabled"; };
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,tegra114-uart", "nvidia,tegra20-uart" and APB DMA based driver will get enabled with compatible "nvidia,tegra114-hsuart", "nvidia,tegra30-hsuart". Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com> --- Changes from V1: - Remove UARTB car id correction. Make the compatible with tegra114 and tegra30 for hsuart and tegra20 for simple uart driver. Changes from V2: - Rebase to for-3.10/dt of Stephen's tegra tree. arch/arm/boot/dts/tegra114.dtsi | 27 +++++++++++++++++++++++---- 1 files changed, 23 insertions(+), 4 deletions(-)