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. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
