[PATCH 0/3] paz00 updates for 3.3

[PATCH 0/3] paz00 updates for 3.3

[Marc Dietrich]

Hi,

the following three patches are intended for the 3.3 merge window and are based
on linux-next.

The first one adds support for the embedded controller (which handles keyboard,
touchpad, leds, and suspend functions - and some other stuff). The way it is
initialized will probably change in the future, but we like to include it for
now as it makes debugging more easier.

The second patch adds wakeup support similar to seaboard, but with the
difference that the wakeup gpio is connected to the embedded controller instead
of a real gpio key.

The final patch adds a dts file for paz00. I also like add paz00 to board-dt.c,
but I couldn't find an up to date version of it (which includes all the patches
on the tegra-ml). I'll update this one as soon as it emerges. So take it as some
kind of "preview".

Thanks

Marc

Marc Dietrich (3):
  ARM: tegra: paz00: add support for the embedded controller
  ARM: tegra: paz00: add support for wakeup gpio key
  arm/dt: tegra: add dts file for paz00

 arch/arm/boot/dts/tegra-paz00.dts        |   51 ++++++++++++++++++++++++++++++
 arch/arm/mach-tegra/board-paz00-pinmux.c |    1 +
 arch/arm/mach-tegra/board-paz00.c        |   40 +++++++++++++++++++++++-
 arch/arm/mach-tegra/board-paz00.h        |    6 +++
 4 files changed, 97 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/boot/dts/tegra-paz00.dts

-- 
1.7.5.4

[PATCH 1/3] ARM: tegra: paz00: add support for the embedded controller

[Marc Dietrich]

This adds support for the embedded controller known as NVEC. The driver
lives currently in the staging tree and we aim to promote it one level
higher in the near future.

The NVEC driver uses the I2C resources of the master controller for now
until slave support is added to the i2c-tegra driver.

Signed-off-by: Marc Dietrich <marvin24-Mmb7MZpHnFY@public.gmane.org>
---
 arch/arm/mach-tegra/board-paz00-pinmux.c |    1 +
 arch/arm/mach-tegra/board-paz00.c        |   11 +++++++++++
 arch/arm/mach-tegra/board-paz00.h        |    3 +++
 3 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-tegra/board-paz00-pinmux.c b/arch/arm/mach-tegra/board-paz00-pinmux.c
index fb20894..0a0e27a 100644
--- a/arch/arm/mach-tegra/board-paz00-pinmux.c
+++ b/arch/arm/mach-tegra/board-paz00-pinmux.c
@@ -154,6 +154,7 @@ static struct tegra_gpio_table gpio_table[] = {
 	{ .gpio = TEGRA_WIFI_PWRN,	.enable = true },
 	{ .gpio = TEGRA_WIFI_RST,	.enable = true },
 	{ .gpio = TEGRA_WIFI_LED,	.enable = true },
+	{ .gpio = TEGRA_NVEC_REQ,	.enable = true },
 };
 
 void paz00_pinmux_init(void)
diff --git a/arch/arm/mach-tegra/board-paz00.c b/arch/arm/mach-tegra/board-paz00.c
index 602f8dd..3f46b37 100644
--- a/arch/arm/mach-tegra/board-paz00.c
+++ b/arch/arm/mach-tegra/board-paz00.c
@@ -44,6 +44,8 @@
 #include "devices.h"
 #include "gpio-names.h"
 
+#include "../../../drivers/staging/nvec/nvec.h"
+
 static struct plat_serial8250_port debug_uart_platform_data[] = {
 	{
 		/* serial port on JP1 */
@@ -114,6 +116,11 @@ static struct platform_device leds_gpio = {
         },
 };
 
+static struct nvec_platform_data nvec_pdata = {
+	.i2c_addr	= 0x8a,
+	.gpio		= TEGRA_NVEC_REQ,
+};
+
 static struct platform_device *paz00_devices[] __initdata = {
 	&debug_uart,
 	&tegra_sdhci_device4,
@@ -127,6 +134,10 @@ static void paz00_i2c_init(void)
 	platform_device_register(&tegra_i2c_device1);
 	platform_device_register(&tegra_i2c_device2);
 	platform_device_register(&tegra_i2c_device4);
+
+	tegra_i2c_device3.name = "nvec";
+	tegra_i2c_device3.dev.platform_data = &nvec_pdata;
+	platform_device_register(&tegra_i2c_device3);
 }
 
 static void paz00_usb_init(void)
diff --git a/arch/arm/mach-tegra/board-paz00.h b/arch/arm/mach-tegra/board-paz00.h
index 2dc1899..7e978f3 100644
--- a/arch/arm/mach-tegra/board-paz00.h
+++ b/arch/arm/mach-tegra/board-paz00.h
@@ -32,6 +32,9 @@
 #define TEGRA_WIFI_RST		TEGRA_GPIO_PD1
 #define TEGRA_WIFI_LED		TEGRA_GPIO_PD0
 
+/* EC */
+#define TEGRA_NVEC_REQ		TEGRA_GPIO_PV2
+
 void paz00_pinmux_init(void);
 
 #endif
-- 
1.7.5.4

[PATCH 2/3] ARM: tegra: paz00: add support for wakeup gpio key

[Marc Dietrich]

This adds support for a wakeup gpio which is connected to the
embedded controller. This will be used later on for wakeup from suspend.

Signed-off-by: Marc Dietrich <marvin24-Mmb7MZpHnFY@public.gmane.org>
---
 arch/arm/mach-tegra/board-paz00.c |   29 ++++++++++++++++++++++++++++-
 arch/arm/mach-tegra/board-paz00.h |    3 +++
 2 files changed, 31 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-tegra/board-paz00.c b/arch/arm/mach-tegra/board-paz00.c
index 3f46b37..8a973d1 100644
--- a/arch/arm/mach-tegra/board-paz00.c
+++ b/arch/arm/mach-tegra/board-paz00.c
@@ -23,8 +23,11 @@
 #include <linux/serial_8250.h>
 #include <linux/clk.h>
 #include <linux/dma-mapping.h>
+#include <linux/gpio.h>
+#include <linux/gpio_keys.h>
 #include <linux/pda_power.h>
 #include <linux/io.h>
+#include <linux/input.h>
 #include <linux/i2c.h>
 #include <linux/rfkill-gpio.h>
 
@@ -36,7 +39,6 @@
 #include <mach/iomap.h>
 #include <mach/irqs.h>
 #include <mach/sdhci.h>
-#include <mach/gpio.h>
 
 #include "board.h"
 #include "board-paz00.h"
@@ -121,12 +123,37 @@ static struct nvec_platform_data nvec_pdata = {
 	.gpio		= TEGRA_NVEC_REQ,
 };
 
+static struct gpio_keys_button paz00_gpio_keys_buttons[] = {
+	{
+		.code		= KEY_POWER,
+		.gpio		= TEGRA_GPIO_POWERKEY,
+		.active_low	= 1,
+		.desc		= "Power",
+		.type		= EV_KEY,
+		.wakeup		= 1,
+	},
+};
+
+static struct gpio_keys_platform_data paz00_gpio_keys = {
+	.buttons	= paz00_gpio_keys_buttons,
+	.nbuttons	= ARRAY_SIZE(paz00_gpio_keys_buttons),
+};
+
+static struct platform_device gpio_keys_device = {
+	.name	= "gpio-keys",
+	.id	= -1,
+	.dev	= {
+		.platform_data = &paz00_gpio_keys,
+	},
+};
+
 static struct platform_device *paz00_devices[] __initdata = {
 	&debug_uart,
 	&tegra_sdhci_device4,
 	&tegra_sdhci_device1,
 	&wifi_rfkill_device,
 	&leds_gpio,
+	&gpio_keys_device,
 };
 
 static void paz00_i2c_init(void)
diff --git a/arch/arm/mach-tegra/board-paz00.h b/arch/arm/mach-tegra/board-paz00.h
index 7e978f3..e88f42b 100644
--- a/arch/arm/mach-tegra/board-paz00.h
+++ b/arch/arm/mach-tegra/board-paz00.h
@@ -35,6 +35,9 @@
 /* EC */
 #define TEGRA_NVEC_REQ		TEGRA_GPIO_PV2
 
+/* WakeUp */
+#define TEGRA_GPIO_POWERKEY	TEGRA_GPIO_PJ7
+
 void paz00_pinmux_init(void);
 
 #endif
-- 
1.7.5.4

[PATCH 3/3] arm/dt: tegra: add dts file for paz00

[Marc Dietrich]

* NOT FOR COMMIT *

This just adds the dts for paz00. I like to add it to board-dt as
well as soon as I find out which tree to base it on.

Signed-off-by: Marc Dietrich <marvin24-Mmb7MZpHnFY@public.gmane.org>
---
 arch/arm/boot/dts/tegra-paz00.dts |   51 +++++++++++++++++++++++++++++++++++++
 1 files changed, 51 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/boot/dts/tegra-paz00.dts

diff --git a/arch/arm/boot/dts/tegra-paz00.dts b/arch/arm/boot/dts/tegra-paz00.dts
new file mode 100644
index 0000000..2114026
--- /dev/null
+++ b/arch/arm/boot/dts/tegra-paz00.dts
@@ -0,0 +1,51 @@
+/dts-v1/;
+
+/memreserve/ 0x1c000000 0x04000000;
+/include/ "tegra20.dtsi"
+
+/ {
+	model = "Toshiba AC100 / Dynabook AZ";
+	compatible = "compal,paz00", "nvidia,tegra20";
+
+	chosen {
+		bootargs = "console=ttyS0,115200n8 root=/dev/mmcblk1p1";
+	};
+
+	memory@0 {
+		reg = < 0x00000000 0x20000000 >;
+	};
+
+	i2c@7000c000 {
+		clock-frequency = <400000>;
+	};
+
+	i2c@7000c400 {
+		clock-frequency = <400000>;
+	};
+
+	i2c@7000c500 {
+		clock-frequency = <80000>;
+	};
+
+	i2c@7000d000 {
+		clock-frequency = <400000>;
+	};
+
+	serial@70006000 {
+		clock-frequency = < 216000000 >;
+	};
+
+	serial@70006300 {
+		clock-frequency = < 216000000 >;
+	};
+
+	sdhci@c8000000 {
+		cd-gpios = <&gpio 173 0>; /* gpio PV5 */
+		wp-gpios = <&gpio 57 0>;  /* gpio PH1 */
+		power-gpios = <&gpio 155 0>; /* gpio PT3 */
+	};
+
+	sdhci@c8000600 {
+		support-8bit;
+	};
+};
-- 
1.7.5.4

Re: [PATCH 1/3] ARM: tegra: paz00: add support for the embedded controller

[Olof Johansson]

On Sat, Oct 22, 2011 at 1:16 PM, Marc Dietrich <marvin24-Mmb7MZpHnFY@public.gmane.org> wrote:
> This adds support for the embedded controller known as NVEC. The driver
> lives currently in the staging tree and we aim to promote it one level
> higher in the near future.
>
> The NVEC driver uses the I2C resources of the master controller for now
> until slave support is added to the i2c-tegra driver.

> diff --git a/arch/arm/mach-tegra/board-paz00.c b/arch/arm/mach-tegra/board-paz00.c
> index 602f8dd..3f46b37 100644
> --- a/arch/arm/mach-tegra/board-paz00.c
> +++ b/arch/arm/mach-tegra/board-paz00.c
> @@ -44,6 +44,8 @@
>  #include "devices.h"
>  #include "gpio-names.h"
>
> +#include "../../../drivers/staging/nvec/nvec.h"

Ick, no! Move the header file containing platform data to
include/linux/platform_data instead (or break it off in a separate
header file).



>  static struct platform_device *paz00_devices[] __initdata = {
>        &debug_uart,
>        &tegra_sdhci_device4,
> @@ -127,6 +134,10 @@ static void paz00_i2c_init(void)
>        platform_device_register(&tegra_i2c_device1);
>        platform_device_register(&tegra_i2c_device2);
>        platform_device_register(&tegra_i2c_device4);
> +
> +       tegra_i2c_device3.name = "nvec";
> +       tegra_i2c_device3.dev.platform_data = &nvec_pdata;
> +       platform_device_register(&tegra_i2c_device3);

Please define a separate platform_device instead of hijacking the
current one, please.



-Olof

Re: [PATCH 1/3] ARM: tegra: paz00: add support for the embedded controller

[Marc Dietrich]

On Saturday 22 October 2011 13:27:12 Olof Johansson wrote:
> On Sat, Oct 22, 2011 at 1:16 PM, Marc Dietrich <marvin24-Mmb7MZpHnFY@public.gmane.org> wrote:
> > This adds support for the embedded controller known as NVEC. The driver
> > lives currently in the staging tree and we aim to promote it one level
> > higher in the near future.
> > 
> > The NVEC driver uses the I2C resources of the master controller for now
> > until slave support is added to the i2c-tegra driver.
> > 
> > diff --git a/arch/arm/mach-tegra/board-paz00.c
> > b/arch/arm/mach-tegra/board-paz00.c index 602f8dd..3f46b37 100644
> > --- a/arch/arm/mach-tegra/board-paz00.c
> > +++ b/arch/arm/mach-tegra/board-paz00.c
> > @@ -44,6 +44,8 @@
> >  #include "devices.h"
> >  #include "gpio-names.h"
> > 
> > +#include "../../../drivers/staging/nvec/nvec.h"
> 
> Ick, no! Move the header file containing platform data to
> include/linux/platform_data instead (or break it off in a separate
> header file).

I know this looks ugly, but it is AFAIK the only (and the common) way for a 
staging driver to be used. Of course the header will be moved to e.g. to 
include/linux/mfd once the driver is ready for mainline, but till that we just
cannot write somewhere outside of the staging dir.
 
> >  static struct platform_device *paz00_devices[] __initdata = {
> >        &debug_uart,
> >        &tegra_sdhci_device4,
> > @@ -127,6 +134,10 @@ static void paz00_i2c_init(void)
> >        platform_device_register(&tegra_i2c_device1);
> >        platform_device_register(&tegra_i2c_device2);
> >        platform_device_register(&tegra_i2c_device4);
> > +
> > +       tegra_i2c_device3.name = "nvec";
> > +       tegra_i2c_device3.dev.platform_data = &nvec_pdata;
> > +       platform_device_register(&tegra_i2c_device3);
> 
> Please define a separate platform_device instead of hijacking the
> current one, please.

ok, I just wanted to keep the patch small ;-)

Marc

Re: [PATCH 1/3] ARM: tegra: paz00: add support for the embedded controller

[Olof Johansson]

[+gregkh]

On Sat, Oct 22, 2011 at 10:49:42PM +0200, Marc Dietrich wrote:
> On Saturday 22 October 2011 13:27:12 Olof Johansson wrote:
> > On Sat, Oct 22, 2011 at 1:16 PM, Marc Dietrich <marvin24-Mmb7MZpHnFY@public.gmane.org> wrote:
> > > This adds support for the embedded controller known as NVEC. The driver
> > > lives currently in the staging tree and we aim to promote it one level
> > > higher in the near future.
> > > 
> > > The NVEC driver uses the I2C resources of the master controller for now
> > > until slave support is added to the i2c-tegra driver.
> > > 
> > > diff --git a/arch/arm/mach-tegra/board-paz00.c
> > > b/arch/arm/mach-tegra/board-paz00.c index 602f8dd..3f46b37 100644
> > > --- a/arch/arm/mach-tegra/board-paz00.c
> > > +++ b/arch/arm/mach-tegra/board-paz00.c
> > > @@ -44,6 +44,8 @@
> > >  #include "devices.h"
> > >  #include "gpio-names.h"
> > > 
> > > +#include "../../../drivers/staging/nvec/nvec.h"
> > 
> > Ick, no! Move the header file containing platform data to
> > include/linux/platform_data instead (or break it off in a separate
> > header file).
> 
> I know this looks ugly, but it is AFAIK the only (and the common) way for a 
> staging driver to be used. Of course the header will be moved to e.g. to 
> include/linux/mfd once the driver is ready for mainline, but till that we just
> cannot write somewhere outside of the staging dir.

Actually, you should be OK with adding it to include/linux/platform_data.

But if you're just about to add device tree, it might make sense to do
a device tree binding instead and only do device-tree configuration of the
device instead. Care to cook up a patch for that?

> > > +       tegra_i2c_device3.name = "nvec";
> > > +       tegra_i2c_device3.dev.platform_data = &nvec_pdata;
> > > +       platform_device_register(&tegra_i2c_device3);
> > 
> > Please define a separate platform_device instead of hijacking the
> > current one, please.
> 
> ok, I just wanted to keep the patch small ;-)

Keeping the code clean is more important than keeping the change small.


-Olof

RE: [PATCH 1/3] ARM: tegra: paz00: add support for the embedded controller

[Stephen Warren]

Marc Dietrich wrote at Saturday, October 22, 2011 2:17 PM:
> This adds support for the embedded controller known as NVEC. The driver
> lives currently in the staging tree and we aim to promote it one level
> higher in the near future.
> 
> The NVEC driver uses the I2C resources of the master controller for now
> until slave support is added to the i2c-tegra driver.

I'm in two minds about this:

The fact that the nvec driver accesses the I2C HW directly is a hack; that
should all happen in the I2C driver, and the nvec code should /just/ handle
the protocol to talk to the microcontroller.

To that end, we're starting to bring on some more engineers internally who
will aim to upstream more of our downstream kernel. Part of this includes
upstreaming the I2C slave support in a more palatable fashion. I know those
engineers are keen in general to get started, but I can't yet estimate a
timescale for when this particular change will be ready.

That said, I'm not sure that I want to block progress on getting nvec
and the AC100 support going. Olof, do you have any input here? Thanks.

-- 
nvpublic

RE: [PATCH 2/3] ARM: tegra: paz00: add support for wakeup gpio key

[Stephen Warren]

Marc Dietrich wrote at Saturday, October 22, 2011 2:17 PM:
> This adds support for a wakeup gpio which is connected to the
> embedded controller. This will be used later on for wakeup from suspend.

I think this seems reasonable, but I'm not really familiar with gpio-keys,
so additional review definitely appreciated:-) Anyway,

Acked-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

-- 
nvpublic

RE: [PATCH 3/3] arm/dt: tegra: add dts file for paz00

[Stephen Warren]

Marc Dietrich wrote at Saturday, October 22, 2011 2:17 PM:
> * NOT FOR COMMIT *
> 
> This just adds the dts for paz00. I like to add it to board-dt as
> well as soon as I find out which tree to base it on.
...
> +++ b/arch/arm/boot/dts/tegra-paz00.dts

One thing that cross my mind when writing the TrimSlice support and I
forgot to mention: Should these files be named ${soc}-${board}.dts or
${vendor}-${board}.dts (tegra-paz00.dts or toshiba-paz00.dts or
compal-paz00.dts). I'm OK either way; just want to follow any outside
expectations.

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

The other boards just have "memory" here; which is wrong?

...
> +	i2c@7000c500 {
> +		clock-frequency = <80000>;

That's a pretty odd I2C frequency...

-- 
nvpublic

Re: [PATCH 1/3] ARM: tegra: paz00: add support for the embedded controller

[Olof Johansson]

On Tue, Oct 25, 2011 at 7:06 AM, Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> Marc Dietrich wrote at Saturday, October 22, 2011 2:17 PM:
>> This adds support for the embedded controller known as NVEC. The driver
>> lives currently in the staging tree and we aim to promote it one level
>> higher in the near future.
>>
>> The NVEC driver uses the I2C resources of the master controller for now
>> until slave support is added to the i2c-tegra driver.
>
> I'm in two minds about this:
>
> The fact that the nvec driver accesses the I2C HW directly is a hack; that
> should all happen in the I2C driver, and the nvec code should /just/ handle
> the protocol to talk to the microcontroller.
>
> To that end, we're starting to bring on some more engineers internally who
> will aim to upstream more of our downstream kernel. Part of this includes
> upstreaming the I2C slave support in a more palatable fashion. I know those
> engineers are keen in general to get started, but I can't yet estimate a
> timescale for when this particular change will be ready.
>
> That said, I'm not sure that I want to block progress on getting nvec
> and the AC100 support going. Olof, do you have any input here? Thanks.

Yeah, let's not hold up ac100 based on not-yet-ready code, especially
since the driver is in staging. If the i2c slave driver could be done
in time to be there before nvec graduates from staging, that would
probably be ideal.

That being said, let's do it reasonably clean -- Marc, if you guys are
up for trying to define device-tree bindings for this that would be
awesome, and move to that for driver probing. That avoids adding an
include/linux/platform_data header file.


-Olof

Re: [PATCH 3/3] arm/dt: tegra: add dts file for paz00

[Marc Dietrich]

On Monday 24 October 2011 22:16:15 Stephen Warren wrote:
> Marc Dietrich wrote at Saturday, October 22, 2011 2:17 PM:
> > * NOT FOR COMMIT *
> > 
> > This just adds the dts for paz00. I like to add it to board-dt as
> > well as soon as I find out which tree to base it on.
> 
> ...
> 
> > +++ b/arch/arm/boot/dts/tegra-paz00.dts
> 
> One thing that cross my mind when writing the TrimSlice support and I
> forgot to mention: Should these files be named ${soc}-${board}.dts or
> ${vendor}-${board}.dts (tegra-paz00.dts or toshiba-paz00.dts or
> compal-paz00.dts). I'm OK either way; just want to follow any outside
> expectations.

I also don't know. I prefer compal for paz00 because Toshiba's code name was 
procyon (I think) and I don't want to change all the naming. Also Toshiba 
showed no interest in helping us, so I don't see why they should get the fame.

> > @@ -0,0 +1,51 @@
> > +/dts-v1/;
> > +
> > +/memreserve/ 0x1c000000 0x04000000;
> > +/include/ "tegra20.dtsi"
> > +
> > +/ {
> > +	model = "Toshiba AC100 / Dynabook AZ";
> > +	compatible = "compal,paz00", "nvidia,tegra20";
> > +
> > +	chosen {
> > +		bootargs = "console=ttyS0,115200n8 root=/dev/mmcblk1p1";
> > +	};
> > +
> > +	memory@0 {
> 
> The other boards just have "memory" here; which is wrong?

I just copy-pasted from harmony, I guess it doesn't matter.

> 
> ...
> 
> > +	i2c@7000c500 {
> > +		clock-frequency = <80000>;
> 
> That's a pretty odd I2C frequency...

no, 80000 is even ;-) Seriously, it just reflects the fact that the embedded 
controller bus runs at 80kHz. Even more seriously, reading "booting-without-
of.txt" in the Documentation/devicetree dir,

      | o PowerPC,970@0
      |   |- name = "PowerPC,970"
      |   |- device_type = "cpu"
      |   |- reg = <0>
      |   |- clock-frequency = <5f5e1000>
      |   |- 64-bit
      |   |- linux,phandle = <2>

 it looks like all clock-frequency values are hexadecimal. In this case even 
<400000> would be wrong (or the docu is --- ok, the docu is wrong).

Marc

Re: [PATCH 1/3] ARM: tegra: paz00: add support for the embeddedcontroller

[Marc Dietrich]

On Monday 24 October 2011 22:06:46 Stephen Warren wrote:
> Marc Dietrich wrote at Saturday, October 22, 2011 2:17 PM:
> > This adds support for the embedded controller known as NVEC. The driver
> > lives currently in the staging tree and we aim to promote it one level
> > higher in the near future.
> > 
> > The NVEC driver uses the I2C resources of the master controller for now
> > until slave support is added to the i2c-tegra driver.
> 
> I'm in two minds about this:
> 
> The fact that the nvec driver accesses the I2C HW directly is a hack; that
> should all happen in the I2C driver, and the nvec code should /just/ handle
> the protocol to talk to the microcontroller.
> 
> To that end, we're starting to bring on some more engineers internally who
> will aim to upstream more of our downstream kernel. Part of this includes
> upstreaming the I2C slave support in a more palatable fashion. I know those
> engineers are keen in general to get started, but I can't yet estimate a
> timescale for when this particular change will be ready.

yeah, we just found out by chance that there exists a slave implementation at 
NVIDIA since last year :-(

<http://nv-
tegra.nvidia.com/gitweb/?p=linux-2.6.git;a=commit;h=fe0261ca61a134fc13ae9c0b2a70fd63804a516e>

but it doesn't seem to be upstream ready yet (maybe should be merged with i2c-
tegra driver).

Re: [PATCH 3/3] arm/dt: tegra: add dts file for paz00

[Rob Herring]

On 10/25/2011 12:16 AM, Stephen Warren wrote:
> Marc Dietrich wrote at Saturday, October 22, 2011 2:17 PM:
>> * NOT FOR COMMIT *
>>
>> This just adds the dts for paz00. I like to add it to board-dt as
>> well as soon as I find out which tree to base it on.
> ...
>> +++ b/arch/arm/boot/dts/tegra-paz00.dts
> 
> One thing that cross my mind when writing the TrimSlice support and I
> forgot to mention: Should these files be named ${soc}-${board}.dts or
> ${vendor}-${board}.dts (tegra-paz00.dts or toshiba-paz00.dts or
> compal-paz00.dts). I'm OK either way; just want to follow any outside
> expectations.
> 
>> @@ -0,0 +1,51 @@
>> +/dts-v1/;
>> +
>> +/memreserve/ 0x1c000000 0x04000000;
>> +/include/ "tegra20.dtsi"
>> +
>> +/ {
>> +	model = "Toshiba AC100 / Dynabook AZ";
>> +	compatible = "compal,paz00", "nvidia,tegra20";
>> +
>> +	chosen {
>> +		bootargs = "console=ttyS0,115200n8 root=/dev/mmcblk1p1";
>> +	};
>> +
>> +	memory@0 {
> 
> The other boards just have "memory" here; which is wrong?
> 

Both... I believe it should be "memory@<base address>".

Rob

Re: [PATCH 1/3] ARM: tegra: paz00: add support for the embedded controller

[Marc Dietrich]

On Tuesday 25 October 2011 09:16:54 Olof Johansson wrote:
> On Tue, Oct 25, 2011 at 7:06 AM, Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> > Marc Dietrich wrote at Saturday, October 22, 2011 2:17 PM:
> >> This adds support for the embedded controller known as NVEC. The
> >> driver
> >> lives currently in the staging tree and we aim to promote it one level
> >> higher in the near future.
> >> 
> >> The NVEC driver uses the I2C resources of the master controller for
> >> now
> >> until slave support is added to the i2c-tegra driver.
> > 
> > I'm in two minds about this:
> > 
> > The fact that the nvec driver accesses the I2C HW directly is a hack;
> > that should all happen in the I2C driver, and the nvec code should
> > /just/ handle the protocol to talk to the microcontroller.
> > 
> > To that end, we're starting to bring on some more engineers internally
> > who will aim to upstream more of our downstream kernel. Part of this
> > includes upstreaming the I2C slave support in a more palatable fashion.
> > I know those engineers are keen in general to get started, but I can't
> > yet estimate a timescale for when this particular change will be ready.
> > 
> > That said, I'm not sure that I want to block progress on getting nvec
> > and the AC100 support going. Olof, do you have any input here? Thanks.
> 
> Yeah, let's not hold up ac100 based on not-yet-ready code, especially
> since the driver is in staging. If the i2c slave driver could be done
> in time to be there before nvec graduates from staging, that would
> probably be ideal.

Thanks Olof!
 
> That being said, let's do it reasonably clean -- Marc, if you guys are
> up for trying to define device-tree bindings for this that would be
> awesome, and move to that for driver probing. That avoids adding an
> include/linux/platform_data header file.

I'm just checking what's the best way to do this. 

But first, something else: booting with device tree enabled gives me the wrong 
order of the mmc's again (or wrong oder of sdhci initialization). Also I only 
need controller 4 and 1 (in that order), how to disable 2 and 3? Same for the 
i2c3 controller, it should not be initialized at all or it will conflict with 
the nvec.

For the nvec, if we only need the platform data, something like

nvec@8a {
		gpio = <0xaa>
}

came into my mind (8a is the slave address). Later I felt this is wrong 
because 8a is not a memory address, and there should be the address of the 
slave controller instead (e.g. 7000c500), but that would require to move all 
of the resources to device tree:

nvec@7000c500 {
		#address-cells = <1>;
		#size-cells = <0>;
		compatible = "nvidia,???";
		reg = <0x7000C500 0x100>;
		interrupts = < 124 >;
		cock-frequency = <80000>
		gpio = <0xaa>
		slave-address = <0x8a>
}

Looking at i2c-tegra I don't see where the memory address is used at all. It 
is specified in the device tree, but the real memory resource still comes from 
the board file, so it seems to be incomplete.

Given that the nvec is a kind of device connected to the i2c bus and has 
devices connected to it, something like this would also make sense:

i2c@7000c500 {
		cock-frequency = <80000>
		is_slave;

		nvec {
				addr = <0x8a>
				gpio = <0xaa>
				
				keyboard {
						cell-index = <0>
				}
				mouse {
						cell-index = <0>
				}
				power {						/* for AC  */
						cell-index = <0>
				}
				power {						/* for battery */
						cell-index = <1>
				}
				leds {
						cell-index = <0>
				}

		}
}

But this is more like it should look like after slave support is added to i2c-
tegra.

I'm a device tree newbie, so any idea what's best?

And finally, looks like linux-next does a minute's silence before booting up 
now. Is this a known issue?

Marc

RE: [PATCH 3/3] arm/dt: tegra: add dts file for paz00

[Stephen Warren]

Rob Herring wrote at Tuesday, October 25, 2011 1:52 PM:
> On 10/25/2011 12:16 AM, Stephen Warren wrote:
> > Marc Dietrich wrote at Saturday, October 22, 2011 2:17 PM:
> >> * NOT FOR COMMIT *
> >>
> >> This just adds the dts for paz00. I like to add it to board-dt as
> >> well as soon as I find out which tree to base it on.
> > ...
> >> +++ b/arch/arm/boot/dts/tegra-paz00.dts
> >
> > One thing that cross my mind when writing the TrimSlice support and I
> > forgot to mention: Should these files be named ${soc}-${board}.dts or
> > ${vendor}-${board}.dts (tegra-paz00.dts or toshiba-paz00.dts or
> > compal-paz00.dts). I'm OK either way; just want to follow any outside
> > expectations.
> >
> >> @@ -0,0 +1,51 @@
> >> +/dts-v1/;
> >> +
> >> +/memreserve/ 0x1c000000 0x04000000;
> >> +/include/ "tegra20.dtsi"
> >> +
> >> +/ {
> >> +	model = "Toshiba AC100 / Dynabook AZ";
> >> +	compatible = "compal,paz00", "nvidia,tegra20";
> >> +
> >> +	chosen {
> >> +		bootargs = "console=ttyS0,115200n8 root=/dev/mmcblk1p1";
> >> +	};
> >> +
> >> +	memory@0 {
> >
> > The other boards just have "memory" here; which is wrong?
> 
> Both... I believe it should be "memory@<base address>".

Tegra20's memory base address is 0, so I assume the quoted example above
is correct then.

-- 
nvpublic

RE: [PATCH 3/3] arm/dt: tegra: add dts file for paz00

[Stephen Warren]

Marc Dietrich wrote at Tuesday, October 25, 2011 1:05 PM:
> On Monday 24 October 2011 22:16:15 Stephen Warren wrote:
> > Marc Dietrich wrote at Saturday, October 22, 2011 2:17 PM:
> > > * NOT FOR COMMIT *
> > >
> > > This just adds the dts for paz00. I like to add it to board-dt as
> > > well as soon as I find out which tree to base it on.
> >
> > ...
> >
> > > +++ b/arch/arm/boot/dts/tegra-paz00.dts
> >
> > One thing that cross my mind when writing the TrimSlice support and I
> > forgot to mention: Should these files be named ${soc}-${board}.dts or
> > ${vendor}-${board}.dts (tegra-paz00.dts or toshiba-paz00.dts or
> > compal-paz00.dts). I'm OK either way; just want to follow any outside
> > expectations.
> 
> I also don't know. I prefer compal for paz00 because Toshiba's code name was
> procyon (I think) and I don't want to change all the naming. Also Toshiba
> showed no interest in helping us, so I don't see why they should get the fame.

Well, it's more about correctly defining what the HW is than advertising,
fame, or support!
 
-- 
nvpublic

RE: [PATCH 1/3] ARM: tegra: paz00: add support for the embedded controller

[Stephen Warren]

Marc Dietrich wrote at Tuesday, October 25, 2011 3:04 PM:
> On Tuesday 25 October 2011 09:16:54 Olof Johansson wrote:
> > On Tue, Oct 25, 2011 at 7:06 AM, Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> > > Marc Dietrich wrote at Saturday, October 22, 2011 2:17 PM:
> > >> This adds support for the embedded controller known as NVEC. The
> > >> driver
> > >> lives currently in the staging tree and we aim to promote it one level
> > >> higher in the near future.
> > >>
> > >> The NVEC driver uses the I2C resources of the master controller for
> > >> now
> > >> until slave support is added to the i2c-tegra driver.
> > >
> > > I'm in two minds about this:
...
> > Yeah, let's not hold up ac100 based on not-yet-ready code, especially
> > since the driver is in staging. If the i2c slave driver could be done
> > in time to be there before nvec graduates from staging, that would
> > probably be ideal.
> 
> Thanks Olof!
> 
> > That being said, let's do it reasonably clean -- Marc, if you guys are
> > up for trying to define device-tree bindings for this that would be
> > awesome, and move to that for driver probing. That avoids adding an
> > include/linux/platform_data header file.
> 
> I'm just checking what's the best way to do this.
> 
> But first, something else: booting with device tree enabled gives me the wrong
> order of the mmc's again (or wrong oder of sdhci initialization). Also I only
> need controller 4 and 1 (in that order), how to disable 2 and 3?

To disable devices, add property status = "disabled".

I'm not sure yet hw to solve the ordering issue. Grant had mentioned
using an aliases node to help with this, but I haven't had a chance to
look into that and see how it'd work.

> For the nvec, if we only need the platform data, something like
> 
> nvec@8a {
> 		gpio = <0xaa>
> }
> 
> came into my mind (8a is the slave address). Later I felt this is wrong
> because 8a is not a memory address, and there should be the address of the
> slave controller instead (e.g. 7000c500), but that would require to move all
> of the resources to device tree:
> 
> nvec@7000c500 {
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 		compatible = "nvidia,???";
> 		reg = <0x7000C500 0x100>;
> 		interrupts = < 124 >;
> 		cock-frequency = <80000>
> 		gpio = <0xaa>
> 		slave-address = <0x8a>
> }

Yes, that looks more correct; the address in the node's name should be where
that node exists on the bus that is its parent; the 0x8a address is essentially
some internal implementation detail in this case, since it's a slave.

> Looking at i2c-tegra I don't see where the memory address is used at all. It
> is specified in the device tree, but the real memory resource still comes from
> the board file, so it seems to be incomplete.

The DT reg property is converted to platform resources during DT parsing/probing,
so it looks like the driveris using APIs to get resources from the board data and
devices.c, but it's actually getting the data from DT.

> Given that the nvec is a kind of device connected to the i2c bus and has
> devices connected to it, something like this would also make sense:
> 
> i2c@7000c500 {
> 		cock-frequency = <80000>
> 		is_slave;
> 
> 		nvec {
> 				addr = <0x8a>
> 				gpio = <0xaa>
> 
> 				keyboard {
> 						cell-index = <0>
> 				}
> 				mouse {
> 						cell-index = <0>
> 				}
> 				power {						/* for AC  */
> 						cell-index = <0>
> 				}
> 				power {						/* for battery */
> 						cell-index = <1>
> 				}
> 				leds {
> 						cell-index = <0>
> 				}
> 
> 		}
> }
> 
> But this is more like it should look like after slave support is added to i2c-
> tegra.
> 
> I'm a device tree newbie, so any idea what's best?

I'd go with the nvec@7000c500 a little above for now. The correct binding once
the I2C driver is fixed needs a little more though (well, at least I need to
think more; it may already be obvious to those more experienced with DT!)

> And finally, looks like linux-next does a minute's silence before booting up
> now. Is this a known issue?

That's odd; I certainly haven't noticed it, but I haven't pulled in the latest
linux-next or a half week or so.

-- 
nvpublic

Re: [PATCH 3/3] arm/dt: tegra: add dts file for paz00

[Marc Dietrich]

On Tuesday 25 October 2011 23:43:06 Stephen Warren wrote:
> Marc Dietrich wrote at Tuesday, October 25, 2011 1:05 PM:
> > On Monday 24 October 2011 22:16:15 Stephen Warren wrote:
> > > Marc Dietrich wrote at Saturday, October 22, 2011 2:17 PM:
> > > > * NOT FOR COMMIT *
> > > > 
> > > > This just adds the dts for paz00. I like to add it to board-dt
> > > > as
> > > > well as soon as I find out which tree to base it on.
> > > 
> > > ...
> > > 
> > > > +++ b/arch/arm/boot/dts/tegra-paz00.dts
> > > 
> > > One thing that cross my mind when writing the TrimSlice support and
> > > I
> > > forgot to mention: Should these files be named ${soc}-${board}.dts
> > > or
> > > ${vendor}-${board}.dts (tegra-paz00.dts or toshiba-paz00.dts or
> > > compal-paz00.dts). I'm OK either way; just want to follow any
> > > outside
> > > expectations.
> > 
> > I also don't know. I prefer compal for paz00 because Toshiba's code name
> > was procyon (I think) and I don't want to change all the naming. Also
> > Toshiba showed no interest in helping us, so I don't see why they
> > should get the fame.
> Well, it's more about correctly defining what the HW is than advertising,
> fame, or support!

define "what the HW is". Warning: the following list is purely fictitious. Any 
similarities between the companies or products mentiond, real or imaginary, 
are coincidental.

	- company A creates a cpu (only the IP) and license it to company N.
	- company N adds some more stuff around it (interfaces, gpu, ...) and 
creates real cpu hw.
	- company T likes the cpu and asks company C to build a computer around it
	- company C takes the evaluation board from company N, modifies it a bit, 
and builds a whole netbook around it
	- company T gets the netbook from company C and adds a operating system 
which will displeased their customers

So how would you define that hw? Or the real question, who made it? And please 
don't take my second argument too serious. I just expressed my anger in an 
improper way at an improper place. Sorry for that.

Marc