On 06/02/2014 06:26 PM, Stephen Warren wrote:
+ toradex,colibri_t30
+ toradex,colibri_t30-eval-v3
Those don't seem to be related to Apalis support.
Yes, that's why I mentioned it in the commit message as follows:
>> While at it also add the device tree binding documentation for Apalis
>> T30 as well as the previously missing one for the recently added
>> Colibri T30.
If it is preferred having this as a separate commit I can ask Stefan to
submit one once he returns from his vacation next week.
diff --git a/arch/arm/boot/dts/tegra30-apalis-eval.dts b/arch/arm/boot/dts/tegra30-apalis-eval.dts
+ host1x@50000000 {
+ dc@54200000 {
+ rgb {
+ status = "okay";
+ nvidia,panel = <&panel>;
+ };
+ };
+ hdmi@54280000 {
Nit: Add a blank line between the nodes. Check elsewhere too.
Sure, I actually checked elsewhere as it is 1-to-1 how it got accepted
for the Colibri T30.
+ serial@70006040 {
+ compatible = "nvidia,tegra30-hsuart";
+ status = "okay";
+ };
Nit: Put the status property first followed by new/overridden
properties, to be consistent with other Tegra DTs. Check elsewhere too.
Dito.
+ /* SPI1: Apalis SPI1 */
+ spi@7000d400 {
+ status = "okay";
+ spi-max-frequency = <25000000>;
+ spidev0: spidev@1 {
Nit: Add a blank line between properties and nodes. Check elsewhere too.
Dito.
Please excuse our ignorance. I will fix it and ask Stefan to do the same
for the Colibri T30 DT.
+ sd1: sdhci@78000000 {
...
+ mmc1: sdhci@78000400 {
Do those nodes really need labels? Nothing appears to reference them,
and I can't see why anything would.
Yes, you are absolutely right. This is a remnant of our tries to use
aliases thereof to solve the predominant MMC block device ordering
issue. I will remove them.
Should the mmc1 node be non-removable? It seems a bit odd for a
removable device to have an 8-bit data bus.
No, not odd at all. Our Apalis Evaluation Board does actually feature an
8-bit MMCplus socket which while such cards are rather rare can be used
to test this functionality in preparation of maybe designing an
additional albeit then non-removable eMMC onto their custom carrier
boards. So nothing wrong with that I believe.
+ backlight: backlight {
+ compatible = "pwm-backlight";
+
+ /* PWM0 */
Nit: No need for a blank line between a bunch of related properties.
Check elsewhere too.
Sure, dito Colibri T30 DT again.
+ pwmleds {
+ compatible = "pwm-leds";
+
+ pwm3 {
+ label = "PWM3";
+ pwms = <&pwm 1 19600>;
+ max-brightness = <255>;
+ };
+ pwm2 {
+ label = "PWM2";
+ pwms = <&pwm 2 19600>;
+ max-brightness = <255>;
+ };
+ pwm1 {
+ label = "PWM1";
+ pwms = <&pwm 3 19600>;
+ max-brightness = <255>;
+ };
Nit: Why not sort those nodes in numerical order?
Sure, the only question is ordering based on what. I choose the actual
Tegra PWM instance while you propose to use our Apalis instance
numbering which in this particular case turns out to be the opposite but
makes us even more happy to comply.
+ regulators {
+ sys_5v0_reg: regulator@1 {
Nit: Why not start the numbering at 0?
Good question which I asked Stefan earlier as well. He proposed to start
with 101 in the module dtsi while starting with 1 in the board dts. But
I guess starting with 100 resp. 0 might be more C programmer friendly.
diff --git a/arch/arm/boot/dts/tegra30-apalis.dtsi b/arch/arm/boot/dts/tegra30-apalis.dtsi
+ pinmux@70000868 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&state_default>;
+
+ state_default: pinmux {
It might make sense to add all the pinmux data to
https://github.com/NVIDIA/tegra-pinmux-scripts, so that both the kernel
and U-Boot pinmux initialization tables can be auto-generated from a
single data-structure. I think that'll get a small amount of
error-/consistency-checking of the data too.
Yes, that makes sense. Please understand that our current mainlining
effort started long before we even learned about the existence of those
scripts. Let me look into adding this there as well.
--
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
