paz00 updates for 3.3

This adds a dts file for paz00. As a side effect, this also enables the embedded controller which controls the keyboard, touchpad, power, leds, and some other functions.

arch/arm/boot/dts/tegra-paz00.dts new file +12 -0

+	};
+
+	memory@0 {
+		reg = <0x00000000 0x20000000>;
+	};
+
+	i2c@7000c000 {
+		clock-frequency = <400000>;
+	};
+
+	i2c@7000c400 {
+		clock-frequency = <400000>;

arch/arm/boot/dts/tegra-paz00.dts new file +7 -0

+		reg = <0x7000C500 0x100>;
+		interrupts = <124>;
+		clock-frequency = <80000>;
+		request-gpios = <&gpio 170 0>;
+		slave-addr = <138>;
+	};
 
 
+++ b/arch/arm/mach-tegra/Makefile

Marc Dietrich wrote at Wednesday, October 26, 2011 1:59 PM:

Quoted 3 lines

This adds a dts file for paz00. As a side effect, this also enables the embedded controller which controls the keyboard, touchpad, power, leds, and some other functions.

...

arch/arm/boot/dts/tegra-paz00.dts line 11

Quoted 13 lines

+++ b/arch/arm/boot/dts/tegra-paz00.dts

arch/arm/boot/dts/tegra-paz00.dts new file +11 -0

@@ -0,0 +1,70 @@
+/dts-v1/;
+
+/memreserve/ 0x1c000000 0x04000000;
+/include/ "tegra20.dtsi"
+
+/ {
+	model = "Toshiba AC100 / Dynabook AZ";
+	compatible = "compal,paz00", "nvidia,tegra20";
+
+	chosen {
+		bootargs = "mem=448@0 console=ttyS0,115200n8 root=/dev/mmcblk1p1";

You shouldn't need the mem= parameter here; it wasn't in your first patch set.

Quoted 12 lines

arch/arm/boot/dts/tegra-paz00.dts new file +12 -0

+	};
+
+	memory@0 {
+		reg = <0x00000000 0x20000000>;
+	};
+
+	i2c@7000c000 {
+		clock-frequency = <400000>;
+	};
+
+	i2c@7000c400 {
+		clock-frequency = <400000>;

arch/arm/boot/dts/tegra-paz00.dts line 33

Quoted 10 lines

arch/arm/boot/dts/tegra-paz00.dts new file +10 -0

+	};
+
+	i2c@7000c500 {
+		status = "disable";
+	};
+
+	nvec@7000c500 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "nvidia,nvec";

I'm not convinced that's the correct compatible value, but I should discuss this more in the nvec patch...

Quoted 16 lines

arch/arm/boot/dts/tegra-paz00.dts new file +7 -0

+		reg = <0x7000C500 0x100>;
+		interrupts = <124>;
+		clock-frequency = <80000>;
+		request-gpios = <&gpio 170 0>;
+		slave-addr = <138>;
+	};
 
 
+++ b/arch/arm/mach-tegra/Makefile

@@ -31,6 +31,7 @@ obj-${CONFIG_MACH_SEABOARD}             +=

board-seaboard-pinmux.o>
obj-${CONFIG_MACH_TEGRA_DT} += board-dt.o
obj-${CONFIG_MACH_TEGRA_DT} += board-harmony-pinmux.o

+obj-$(CONFIG_MACH_TEGRA_DT) += board-paz00-pinmux.o

Quoted 1 line

obj-${CONFIG_MACH_TEGRA_DT} += board-seaboard-pinmux.o

What branch is your patch based on? Those lines use braces not brackets in the latest code (incorrect yes, but that's what's there; Olof said he'll fix it for the next merge window). Do you have some local patches in your branch before this patchset? Note: Feel free to add your lines using the correct syntax; I'm just wondering about the existing context in your patch.

Am Donnerstag, 27. Oktober 2011, 09:50:03 schrieb Stephen Warren:

Quoted 5 lines

Marc Dietrich wrote at Wednesday, October 26, 2011 1:59 PM:

Quoted 3 lines

This adds a dts file for paz00. As a side effect, this also enables the embedded controller which controls the keyboard, touchpad, power, leds, and some other functions.

...

Quoted 15 lines

arch/arm/boot/dts/tegra-paz00.dts line 11

Quoted 13 lines

+++ b/arch/arm/boot/dts/tegra-paz00.dts

arch/arm/boot/dts/tegra-paz00.dts new file +11 -0

@@ -0,0 +1,70 @@
+/dts-v1/;
+
+/memreserve/ 0x1c000000 0x04000000;
+/include/ "tegra20.dtsi"
+
+/ {
+	model = "Toshiba AC100 / Dynabook AZ";
+	compatible = "compal,paz00", "nvidia,tegra20";
+
+	chosen {
+		bootargs = "mem=448 at 0 console=ttyS0,115200n8 root=/dev/mmcblk1p1";

You shouldn't need the mem= parameter here; it wasn't in your first patch set.

that's because I forgot it. Sorry, I didn't mentioned it in the changelog. I wonder why mem= is still needed.

I've seen patches on the chromeos tree which try to reserve the gpu memory on demand. While we are at it, what is the vmalloc=192M used by most other boards for?

When boards boot from DT, there is no fixup function to override the bootloader's ATAGs. I also see a bunch of code to set up the memory information from DT e.g. setup_machine_fdt()'s call to:

	of_scan_flat_dt(early_init_dt_scan_memory, NULL);

... but I assume that happens before the ATAGs are processed, and the buggy ATAGs end up overriding the information in the DT file.

Quoted 3 lines

I've seen patches on the chromeos tree which try to reserve the gpu memory on demand. While we are at it, what is the vmalloc=192M used by most other boards for?

I'm not sure what vmalloc= does exactly. I somewhat doubt it's necessary until we have code that uses the GPU in mainline.

Quoted 8 lines · Stephen Warren

When boards boot from DT, there is no fixup function to override the bootloader's ATAGs. I also see a bunch of code to set up the memory information from DT e.g. setup_machine_fdt()'s call to:

	of_scan_flat_dt(early_init_dt_scan_memory, NULL);

... but I assume that happens before the ATAGs are processed, and the buggy ATAGs end up overriding the information in the DT file.

As far as the uncompressed kernel is concerned, there is either ATAG or DT information, never both. If the boot loader provides ATAGs and the zImage has a DT appended to it, the zImage decompressor merges the ATAGs into the appended DT and passes the DT to the kernel.

So anyone who currently 'fixes' their broken boot loader via the fixup function by directly manipulating the ATAGS is going to hit the DT image instead.

Quoted 15 lines · Russell King - ARM Linux

Quoted 8 lines · Stephen Warren

When boards boot from DT, there is no fixup function to override the bootloader's ATAGs. I also see a bunch of code to set up the memory information from DT e.g. setup_machine_fdt()'s call to:

	of_scan_flat_dt(early_init_dt_scan_memory, NULL);

... but I assume that happens before the ATAGs are processed, and the buggy ATAGs end up overriding the information in the DT file.

As far as the uncompressed kernel is concerned, there is either ATAG or DT information, never both. If the boot loader provides ATAGs and the zImage has a DT appended to it, the zImage decompressor merges the ATAGs into the appended DT and passes the DT to the kernel.

Quoted 3 lines

So anyone who currently 'fixes' their broken boot loader via the fixup function by directly manipulating the ATAGS is going to hit the DT image instead.

Ugh. Not pretty. A platform could still have board specific fixup code that matches on the compatible string that deals with broken firmware ATAGs, but I don't think that is what we want to do.

Quoted 8 lines

When boards boot from DT, there is no fixup function to override the bootloader's ATAGs. I also see a bunch of code to set up the memory information from DT e.g. setup_machine_fdt()'s call to:

	of_scan_flat_dt(early_init_dt_scan_memory, NULL);

... but I assume that happens before the ATAGs are processed, and the buggy ATAGs end up overriding the information in the DT file.

As far as the uncompressed kernel is concerned, there is either ATAG or DT information, never both. If the boot loader provides ATAGs and the zImage has a DT appended to it, the zImage decompressor merges the ATAGs into the appended DT and passes the DT to the kernel.

So anyone who currently 'fixes' their broken boot loader via the fixup function by directly manipulating the ATAGS is going to hit the DT image instead.

Marc Dietrich wrote at Friday, October 28, 2011 4:30 AM:

Quoted 27 lines

Am Donnerstag, 27. Oktober 2011, 09:50:03 schrieb Stephen Warren:

Quoted 18 lines

Marc Dietrich wrote at Wednesday, October 26, 2011 1:59 PM:

Quoted 5 lines

This adds a dts file for paz00. As a side effect, this also enables
the embedded controller which controls the keyboard, touchpad, power,
leds, and some other functions.

...

Quoted 9 lines

+++ b/arch/arm/boot/dts/tegra-paz00.dts

arch/arm/boot/dts/tegra-paz00.dts new file +7 -0

@@ -0,0 +1,70 @@
+/dts-v1/;
+
+/memreserve/ 0x1c000000 0x04000000;
+/include/ "tegra20.dtsi"
+
+/ {
+	model = "Toshiba AC100 / Dynabook AZ";

Quoted 8 lines

Quoted 5 lines

arch/arm/boot/dts/tegra-paz00.dts new file +4 -0

+	compatible = "compal,paz00", "nvidia,tegra20";
+
+	chosen {
+		bootargs = "mem=448 at 0 console=ttyS0,115200n8

root=/dev/mmcblk1p1";

You shouldn't need the mem= parameter here; it wasn't in your first patch set.>

Quoted 2 lines

that's because I forgot it. Sorry, I didn't mentioned it in the changelog. I wonder why mem= is still needed.

I wonder if this is some conflict between ATAGs and the DT-specified memory node.

As far as I can tell, the kernel's memory information typically comes from ATAGs. Some boards override what's in the ATAGs in their machine descriptor's fixup routine, e.g. see tegra_paz00_fixup(). Presumably, this is because of buggy bootloaders sending incorrect memory information in the ATAGs. Do you have any way to check the ATAGs that the bootloader is sending? Probably, you could enable DEBUG_LL and using the low-level debugging functions to dump some of that information to the UART early during boot.

When boards boot from DT, there is no fixup function to override the bootloader's ATAGs.

I also see a bunch of code to set up the memory information from DT e.g. setup_machine_fdt()'s call to:

	of_scan_flat_dt(early_init_dt_scan_memory, NULL);

... but I assume that happens before the ATAGs are processed, and the buggy ATAGs end up overriding the information in the DT file. And further, I assume that specifying "mem=" on the command-line overrides that, thus solving the problem.

It'd be awesome if you could validate this; the simplest way is probably to:

a) Remove mem= from the command-line

b) Modify arch/arm/kernel/setup.c:parse_tag_mem32() to do nothing; comment out the call to arm_add_memory()

c) Test booting, and check what RAM size the kernel thinks you have.

If that works, then ATAGs are the problem. We probably need to modify the core ARM code not to use memory ATAGs when there is a DT?

I'm mainly pushing on this because adding "mem=" to the command-line in the DT file isn't a great solution; when people start using bootloaders that rewrite the DT to include the user-specified command-line, then every user is going to have to start specifying "mem=" in their command-lines.

Quoted 3 lines

I've seen patches on the chromeos tree which try to reserve the gpu memory on demand. While we are at it, what is the vmalloc=192M used by most other boards for?

I'm not sure what vmalloc= does exactly. I somewhat doubt it's necessary until we have code that uses the GPU in mainline. The same goes for the /memreserve/ DT entries. I've been thinking of submitting a patch to remove this cruft, but haven't gotten around to it yet.

Quoted 10 lines · On Friday 28 October 2011 09:49:49 Stephen Warren wrote:

I also see a bunch of code to set up the memory information from DT e.g. setup_machine_fdt()'s call to:

	of_scan_flat_dt(early_init_dt_scan_memory, NULL);

... but I assume that happens before the ATAGs are processed, and the buggy ATAGs end up overriding the information in the DT file. And further, I assume that specifying "mem=" on the command-line overrides that, thus solving the problem.

Quoted 4 lines

It'd be awesome if you could validate this; the simplest way is probably to:

a) Remove mem= from the command-line

I tested several variations. Without DT, the fixup can compensate a missing mem entry in the kernel command line, but with a mem= from the kernel command line, a fixup is not needed.

With DT, the command line specified from nvflash is ignored and the one from DT comes into the game. If it is also missing there, system detects 512 MB (which is physical right, but we cannot reserve memory for the gpu). The fixup is indeed ignored in this case.

Quoted 3 lines

b) Modify arch/arm/kernel/setup.c:parse_tag_mem32() to do nothing; comment out the call to arm_add_memory()

I leave this out because the bootloader does not send memory info in the ATAGS.

Quoted 1 line

c) Test booting, and check what RAM size the kernel thinks you have.

see above. RAM detection works, but it's not what we want ...

Quoted 7 lines

If that works, then ATAGs are the problem. We probably need to modify the core ARM code not to use memory ATAGs when there is a DT?

I'm mainly pushing on this because adding "mem=" to the command-line in the DT file isn't a great solution; when people start using bootloaders that rewrite the DT to include the user-specified command-line, then every user is going to have to start specifying "mem=" in their command-lines.

Well, I see two options for our case:

a) use the mem=448M at 0 in the DT so the gpu can get its memory
b) upstream the chromeos changes to reserve gpu memory "on the fly" from
the autodetected 512M (as it should be IMHO).

Quoted 34 lines

Marc Dietrich wrote at Friday, October 28, 2011 4:30 AM:

Quoted 27 lines

Am Donnerstag, 27. Oktober 2011, 09:50:03 schrieb Stephen Warren:

Quoted 18 lines

Marc Dietrich wrote at Wednesday, October 26, 2011 1:59 PM:

Quoted 5 lines

This adds a dts file for paz00. As a side effect, this also enables
the embedded controller which controls the keyboard, touchpad, power,
leds, and some other functions.

...

Quoted 9 lines

+++ b/arch/arm/boot/dts/tegra-paz00.dts

(quoted diff) +7 -0

@@ -0,0 +1,70 @@
+/dts-v1/;
+
+/memreserve/ 0x1c000000 0x04000000;
+/include/ "tegra20.dtsi"
+
+/ {
+	model = "Toshiba AC100 / Dynabook AZ";

Quoted 8 lines

Quoted 5 lines

(quoted diff) +4 -0

+	compatible = "compal,paz00", "nvidia,tegra20";
+
+	chosen {
+		bootargs = "mem=448 at 0 console=ttyS0,115200n8

root=/dev/mmcblk1p1";

You shouldn't need the mem= parameter here; it wasn't in your first patch set.

Quoted 2 lines

that's because I forgot it. Sorry, I didn't mentioned it in the changelog. I wonder why mem= is still needed.

I wonder if this is some conflict between ATAGs and the DT-specified memory node.

Quoted 8 lines

As far as I can tell, the kernel's memory information typically comes from ATAGs. Some boards override what's in the ATAGs in their machine descriptor's fixup routine, e.g. see tegra_paz00_fixup(). Presumably, this is because of buggy bootloaders sending incorrect memory information in the ATAGs. Do you have any way to check the ATAGs that the bootloader is sending? Probably, you could enable DEBUG_LL and using the low-level debugging functions to dump some of that information to the UART early during boot.

I got the ATAGS from /proc/atags and there is no memory entry there, only initrd and cmdline (which is the one from nvflash).

The machine also has a fixup routine, but it also boots without (I'm sure it was necessary in the past),

I tested several variations. Without DT, the fixup can compensate a missing mem entry in the kernel command line, but with a mem= from the kernel command line, a fixup is not needed.

With DT, the command line specified from nvflash is ignored and the one from DT comes into the game.

If it is also missing there, system detects 512 MB.

(which is physical right, but we cannot reserve memory for the gpu). The fixup is indeed ignored in this case.

Quoted 2 lines

b) Modify arch/arm/kernel/setup.c:parse_tag_mem32() to do nothing; comment out the call to arm_add_memory()

I leave this out because the bootloader does not send memory info in the ATAGS.

Quoted 1 line

c) Test booting, and check what RAM size the kernel thinks you have.

see above. RAM detection works, but it's not what we want ...

Marc Dietrich wrote at Sunday, October 30, 2011 2:40 PM:

Quoted 47 lines · On Friday 28 October 2011 09:49:49 Stephen Warren wrote:

Quoted 31 lines

Marc Dietrich wrote at Friday, October 28, 2011 4:30 AM:

Quoted 24 lines

Am Donnerstag, 27. Oktober 2011, 09:50:03 schrieb Stephen Warren:

Quoted 14 lines

Marc Dietrich wrote at Wednesday, October 26, 2011 1:59 PM:

Quoted 13 lines

This adds a dts file for paz00. As a side effect, this also enables the embedded controller which controls the keyboard, touchpad, power, leds, and some other functions.
...
+++ b/arch/arm/boot/dts/tegra-paz00.dts

(quoted diff) +7 -0

@@ -0,0 +1,70 @@
+/dts-v1/;
+
+/memreserve/ 0x1c000000 0x04000000;
+/include/ "tegra20.dtsi"
+
+/ {
+	model = "Toshiba AC100 / Dynabook AZ";

Quoted 8 lines

Quoted 5 lines

(quoted diff) +4 -0

+	compatible = "compal,paz00", "nvidia,tegra20";
+
+	chosen {
+		bootargs = "mem=448 at 0 console=ttyS0,115200n8

root=/dev/mmcblk1p1";

You shouldn't need the mem= parameter here; it wasn't in your first patch set.

Quoted 2 lines

that's because I forgot it. Sorry, I didn't mentioned it in the changelog. I wonder why mem= is still needed.

I wonder if this is some conflict between ATAGs and the DT-specified memory node.

Quoted 8 lines

As far as I can tell, the kernel's memory information typically comes from ATAGs. Some boards override what's in the ATAGs in their machine descriptor's fixup routine, e.g. see tegra_paz00_fixup(). Presumably, this is because of buggy bootloaders sending incorrect memory information in the ATAGs. Do you have any way to check the ATAGs that the bootloader is sending? Probably, you could enable DEBUG_LL and using the low-level debugging functions to dump some of that information to the UART early during boot.

I got the ATAGS from /proc/atags and there is no memory entry there, only
initrd and cmdline (which is the one from nvflash).

The machine also has a fixup routine, but it also boots without (I'm sure it was necessary in the past),

...

Quoted 3 lines

I tested several variations. Without DT, the fixup can compensate a missing mem entry in the kernel command line, but with a mem= from the kernel command line, a fixup is not needed.

OK, that makes sense.

Quoted 2 lines

With DT, the command line specified from nvflash is ignored and the one from DT comes into the game.

I assume you're using CONFIG_ARM_APPENDED_DTB but not CONFIG_ARM_ATAG_DTB_COMPAT.

Quoted 1 line

If it is also missing there, system detects 512 MB.

That makes sense; that's the value in the .dts file you posted.

Quoted 2 lines

(which is physical right, but we cannot reserve memory for the gpu). The fixup is indeed ignored in this case.

It's not "ignored" as such.

When booting without DT, the machine descriptor in board-paz00.c is used, since the descriptor is selected based on the machine ID in the descriptor, and that descriptor includes:

.fixup = tegra_paz00_fixup,

... which causes that fixup function to be called.

When booting with DT, the machine descriptor in board-dt.c is used, since the descriptor is selected based on the DT's overall compatible property, and that descriptor doesn't refer to tegra_paz00_fixup() at all, so it's not called.

Quoted 4 lines

...

Quoted 1 line

c) Test booting, and check what RAM size the kernel thinks you have.

Quoted 1 line

see above. RAM detection works, but it's not what we want ...

At this point, I'd argue that exposing the full 512M to the kernel /is/ what we want. There is no Tegra GPU support in the mainline kernel at present. As such, I'd argue that we should give the entire RAM space to the kernel to use. As and when we add GPU support to the kernel, we can use an appropriate mechanism to take RAM away from the kernel for that purpose.

So, I'm planning to remove all the /memreserve/ entries from the Tegra .dts files as soon as I can find a minute to do so.

But, as Grant mentioned, the /memreserve/ .dts directive should work right now to reserve memory. I'm not sure if you'd tried that and it didn't work? If so, it'd be good to debug that just to make sure the mechanism works, even if we don't intend to use it.

This adds device tree support to the nvec driver. By using this method it is no longer necessary to specify platform data through a board file.

Marc Dietrich wrote at Wednesday, October 26, 2011 1:59 PM:

Quoted 3 lines

This adds device tree support to the nvec driver. By using this method it is no longer necessary to specify platform data through a board file.

You should document the binding in Documentation/devicetree/bindings.

Quoted 4 lines

drivers/staging/nvec/nvec.c +0 -0

@@ -892,6 +915,17 @@ static int tegra_nvec_resume(struct platform_device *pdev)
 #define tegra_nvec_resume NULL
 #endif
 

drivers/staging/nvec/nvec.c static int tegra_nvec_resume(struct platform_device *pdev) · line 918

Quoted 1 line

drivers/staging/nvec/nvec.c +1 -0

+#if defined(CONFIG_OF)

I think you can just remove the ifdef and always include this code. Yes, it'll result in slightly more rodata when !CONFIG_OF, but !CONFIG_OF isn't going to exist or be useful for Tegra for that much longer.

drivers/staging/nvec/nvec.c static int tegra_nvec_resume(struct platform_device *pdev) · line 921

Quoted 3 lines

drivers/staging/nvec/nvec.c +3 -0

+/* Match table for of_platform binding */
+static const struct of_device_id nvidia_nvec_of_match[] __devinitconst = {
+	{ .compatible = "nvidia,nvec", },

I'm not sure that nvidia,nvec is the right value, but need a little more background.

It's my understanding that how this works is a little micro-controller exists on the board, handles various devices like the keyboard, and sends data to Tegra by making I2C master transactions. Isn't it the case that the micro-controller (or at least the SW running on it) is board-specific, and the same for the I2C protocol? If so, nvidia,nvec is a little generic; we probably need to name it compal,paz00-ec or something like that?

Marc Dietrich wrote at Wednesday, October 26, 2011 1:59 PM:

Quoted 3 lines

This adds device tree support to the nvec driver. By using this method it is no longer necessary to specify platform data through a board file.

You should document the binding in Documentation/devicetree/bindings.

drivers/staging/nvec/nvec.c static int tegra_nvec_resume(struct platform_device *pdev) · line 919

Quoted 6 lines

drivers/staging/nvec/nvec.c +1 -0

@@ -892,6 +915,17 @@ static int tegra_nvec_resume(struct platform_device *pdev)
 
 #define tegra_nvec_resume NULL
 #endif
 
+#if defined(CONFIG_OF)

I think you can just remove the ifdef and always include this code. Yes, it'll result in slightly more rodata when !CONFIG_OF, but !CONFIG_OF isn't going to exist or be useful for Tegra for that much longer.

drivers/staging/nvec/nvec.c static int tegra_nvec_resume(struct platform_device *pdev) · line 921

Quoted 4 lines

drivers/staging/nvec/nvec.c +3 -0

+/* Match table for of_platform binding */
+static const struct of_device_id nvidia_nvec_of_match[] __devinitconst = {
+	{ .compatible = "nvidia,nvec", },

I'm not sure that nvidia,nvec is the right value, but need a little more background.

It's my understanding that how this works is a little micro-controller exists on the board, handles various devices like the keyboard, and sends data to Tegra by making I2C master transactions. Isn't it the case that the micro-controller (or at least the SW running on it) is board-specific, and the same for the I2C protocol? If so, nvidia,nvec is a little generic; we probably need to name it compal,paz00-ec or something like that?

Either way, we should probably include some kind of version number in the compatible property so we can support upgrades to the protocol if needed.

Am Donnerstag, 27. Oktober 2011, 12:17:25 schrieb Stephen Warren:

Quoted 4 lines

Marc Dietrich wrote at Wednesday, October 26, 2011 1:59 PM:

Quoted 3 lines

This adds device tree support to the nvec driver. By using this method it is no longer necessary to specify platform data through a board file.

Quoted 7 lines

drivers/staging/nvec/nvec.c static int tegra_nvec_resume(struct platform_device *pdev) · line 921

Quoted 3 lines

drivers/staging/nvec/nvec.c +3 -0

+/* Match table for of_platform binding */
+static const struct of_device_id nvidia_nvec_of_match[] __devinitconst = {
+	{ .compatible = "nvidia,nvec", },

I'm not sure that nvidia,nvec is the right value, but need a little more background.

Quoted 6 lines

It's my understanding that how this works is a little micro-controller exists on the board, handles various devices like the keyboard, and sends data to Tegra by making I2C master transactions. Isn't it the case that the micro-controller (or at least the SW running on it) is board-specific, and the same for the I2C protocol? If so, nvidia,nvec is a little generic; we probably need to name it compal,paz00-ec or something like that?

The firmware (for the 8051 mc inside the keyboard controller) is likely made by Compal, but as Julian already said, the EC protocol definition is very likely from NVIDIA itself. Compal just implemented it for the master. You may refer to
<http://nv-
tegra.nvidia.com/gitweb/?p=linux-2.6.git;a=commitdiff;h=12114faf442a8c6aac81a9702712077364db0e82> Also this protocol is not board specific as many first generation boards/device use it, so "nvidia,nvec" should be correct here.

Quoted 3 lines

Either way, we should probably include some kind of version number in the compatible property so we can support upgrades to the protocol if needed.

You may ask your colleagues on that topic, but it seems that the protocol is dead already, e.g. it wasn't implemented for the new-world kernels (>= .36) anymore.

Marc Dietrich wrote at Friday, October 28, 2011 5:02 AM:

Quoted 5 lines

Am Donnerstag, 27. Oktober 2011, 12:17:25 schrieb Stephen Warren:

Quoted 4 lines

Marc Dietrich wrote at Wednesday, October 26, 2011 1:59 PM:

Quoted 3 lines

This adds device tree support to the nvec driver. By using this method it is no longer necessary to specify platform data through a board file.

...

Quoted 30 lines

Quoted 7 lines

Quoted 3 lines

(quoted diff) +2 -0

+/* Match table for of_platform binding */
+static const struct of_device_id nvidia_nvec_of_match[]

__devinitconst = { + { .compatible = "nvidia,nvec", },

I'm not sure that nvidia,nvec is the right value, but need a little more background.

Quoted 8 lines

It's my understanding that how this works is a little micro-controller
exists on the board, handles various devices like the keyboard, and sends data to Tegra by making I2C master transactions. Isn't it the case that the micro-controller (or at least the SW running on it) is board-specific, and the same for the I2C protocol? If so, nvidia,nvec is a little generic; we probably need to name it compal,paz00-ec or something like that?>

The firmware (for the 8051 mc inside the keyboard controller) is likely made by Compal, but as Julian already said, the EC protocol definition is very likely from NVIDIA itself. Compal just implemented it for the master. You may refer to <http://nv-
tegra.nvidia.com/gitweb/?p=linux-2.6.git;a=commitdiff;h=12114faf442a8c6a ac81a9702712077364db0e82> Also this protocol is not board specific as many first generation boards/device use it, so "nvidia,nvec" should be correct here.

Quoted 5 lines

Either way, we should probably include some kind of version number in
the compatible property so we can support upgrades to the protocol if
needed.

Quoted 3 lines

You may ask your colleagues on that topic, but it seems that the protocol is dead already, e.g. it wasn't implemented for the new-world kernels (>= .36) anymore.

OK, I asked internally and it sounds like this is /probably/ standardized.

That said, there are apparently some OEMs who did change the protocol and do something slightly different. I'm trying to confirm whether PAZ00 was one of them. I guess not if PAZ00 works with the standard driver that you linked to.

So the good news is that there's an internal specification for this protocol, and we might be able to release it. I'll let you know if/when there are updates on this.

I'd like to call this "nvidia,nvec-1.0" to version this compatible property; that's the specification version in the latest document that I saw. While we do seemed to have abandoned this approach, I want to make sure this is extensible if someone suddenly decides to go back to it and creates a 2.0 in the future. Does that seem reasonable?

Quoted 12 lines

Marc Dietrich wrote at Friday, October 28, 2011 5:02 AM:

Quoted 5 lines

Am Donnerstag, 27. Oktober 2011, 12:17:25 schrieb Stephen Warren:

Quoted 4 lines

Marc Dietrich wrote at Wednesday, October 26, 2011 1:59 PM:

Quoted 3 lines

This adds device tree support to the nvec driver. By using this method it is no longer necessary to specify platform data through a board file.

...

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

(quoted diff) +2 -0

+/* Match table for of_platform binding */
+static const struct of_device_id nvidia_nvec_of_match[]

__devinitconst = { + { .compatible = "nvidia,nvec", },

Quoted 5 lines

I'd like to call this "nvidia,nvec-1.0" to version this compatible property; that's the specification version in the latest document that I saw. While we do seemed to have abandoned this approach, I want to make sure this is extensible if someone suddenly decides to go back to it and creates a 2.0 in the future. Does that seem reasonable?

mmh, I can't see why we should add it now. There is no V2 I can see in my limited view. If your company plans to expand the protocol you can either enhance our driver or create a new one (nvec2), which can add a nvidia,nvec-2 compatibility property (we can also change ours to nvidia,nvec-1 at the same time, but that's not required).

Quoted 4 lines

That said, there are apparently some OEMs who did change the protocol and do something slightly different. I'm trying to confirm whether PAZ00 was one of them. I guess not if PAZ00 works with the standard driver that you linked to.

There are so called OEM commands which we will move to a board specific nvec file (e.g. nvec_paz00.c). We haven't got the chance to test it on other boards using it yet (e.g. toshiba folio, advent vega, ...). The tablets are mostly using it for power control and maybe also leds/switches. I don't think any other board uses keyboard / mouse functions.