[PATCH v2 0/6] ARM: tegra: add nvec keyboard support for paz00

[PATCH v2 0/6] ARM: tegra: add nvec keyboard support for paz00

[Andrey Danin]

This patch series introduces keyboard support for AC100 (board paz00).

 I2C slave mode was implemented for i2c core and tegra-i2c.

 NVEC code from linux kernel was reworked to use tegra-i2c driver.

 Keytable header file is copied from linux kernel but modified
 to fix styles and remove unused code.

 Based on u-boot-tegra/next.

CC: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
CC: Marc Dietrich <marvin24-Mmb7MZpHnFY@public.gmane.org>
CC: Julian Andres Klode <jak-4HMq4SXA452hPH1hqNUYSQ@public.gmane.org>
CC: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
CC: ac100-oU9gvf+ajcQ97yFScArB1dHuzzzSOjJt@public.gmane.org
---
 Changes for v2:
	- I2C slave mode for i2c-core and tegra-i2c implemented
	- Fixed NVEC dt bindings 
	- NVEC driver was reworked to use tegra-i2c
	- fixed incorrect keys handling in nvec-keyboard driver
	- patch is splitted to smaller parts

Andrey Danin (6):
  i2c: add slave mode support
  ARM: tegra: i2c: add slave mode support
  ARM: tegra: i2c: add nvec driver
  ARM: tegra: nvec: add keyboard support
  ARM: tegra: paz00: add dt bindings for nvec
  ARM: tegra: paz00: enable nvec keyboard

 arch/arm/include/asm/arch-tegra/tegra_i2c.h        |    6 +
 arch/arm/include/asm/arch-tegra/tegra_nvec.h       |  130 +++++++++
 .../include/asm/arch-tegra/tegra_nvec_keyboard.h   |  304 ++++++++++++++++++++
 board/compal/dts/tegra20-paz00.dts                 |    8 +-
 board/nvidia/common/board.c                        |   12 +
 drivers/i2c/Makefile                               |    1 +
 drivers/i2c/i2c_core.c                             |   13 +
 drivers/i2c/tegra_i2c.c                            |  199 ++++++++++++-
 drivers/i2c/tegra_nvec.c                           |  294 +++++++++++++++++++
 drivers/input/Makefile                             |    3 +
 drivers/input/tegra-nvec-kbc.c                     |  215 ++++++++++++++
 include/configs/paz00.h                            |    9 +
 include/configs/tegra-common-post.h                |    2 +
 include/fdtdec.h                                   |    1 +
 include/i2c.h                                      |   30 +-
 lib/fdtdec.c                                       |    1 +
 16 files changed, 1223 insertions(+), 5 deletions(-)
 create mode 100644 arch/arm/include/asm/arch-tegra/tegra_nvec.h
 create mode 100644 arch/arm/include/asm/arch-tegra/tegra_nvec_keyboard.h
 create mode 100644 drivers/i2c/tegra_nvec.c
 create mode 100644 drivers/input/tegra-nvec-kbc.c

-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[U-Boot] [PATCH v2 0/6] ARM: tegra: add nvec keyboard support for paz00

[Andrey Danin]

This patch series introduces keyboard support for AC100 (board paz00).

 I2C slave mode was implemented for i2c core and tegra-i2c.

 NVEC code from linux kernel was reworked to use tegra-i2c driver.

 Keytable header file is copied from linux kernel but modified
 to fix styles and remove unused code.

 Based on u-boot-tegra/next.

CC: Stephen Warren <swarren@nvidia.com>
CC: Marc Dietrich <marvin24@gmx.de>
CC: Julian Andres Klode <jak@jak-linux.org>
CC: devicetree at vger.kernel.org
CC: ac100 at lists.launchpad.net
---
 Changes for v2:
	- I2C slave mode for i2c-core and tegra-i2c implemented
	- Fixed NVEC dt bindings 
	- NVEC driver was reworked to use tegra-i2c
	- fixed incorrect keys handling in nvec-keyboard driver
	- patch is splitted to smaller parts

Andrey Danin (6):
  i2c: add slave mode support
  ARM: tegra: i2c: add slave mode support
  ARM: tegra: i2c: add nvec driver
  ARM: tegra: nvec: add keyboard support
  ARM: tegra: paz00: add dt bindings for nvec
  ARM: tegra: paz00: enable nvec keyboard

 arch/arm/include/asm/arch-tegra/tegra_i2c.h        |    6 +
 arch/arm/include/asm/arch-tegra/tegra_nvec.h       |  130 +++++++++
 .../include/asm/arch-tegra/tegra_nvec_keyboard.h   |  304 ++++++++++++++++++++
 board/compal/dts/tegra20-paz00.dts                 |    8 +-
 board/nvidia/common/board.c                        |   12 +
 drivers/i2c/Makefile                               |    1 +
 drivers/i2c/i2c_core.c                             |   13 +
 drivers/i2c/tegra_i2c.c                            |  199 ++++++++++++-
 drivers/i2c/tegra_nvec.c                           |  294 +++++++++++++++++++
 drivers/input/Makefile                             |    3 +
 drivers/input/tegra-nvec-kbc.c                     |  215 ++++++++++++++
 include/configs/paz00.h                            |    9 +
 include/configs/tegra-common-post.h                |    2 +
 include/fdtdec.h                                   |    1 +
 include/i2c.h                                      |   30 +-
 lib/fdtdec.c                                       |    1 +
 16 files changed, 1223 insertions(+), 5 deletions(-)
 create mode 100644 arch/arm/include/asm/arch-tegra/tegra_nvec.h
 create mode 100644 arch/arm/include/asm/arch-tegra/tegra_nvec_keyboard.h
 create mode 100644 drivers/i2c/tegra_nvec.c
 create mode 100644 drivers/input/tegra-nvec-kbc.c

-- 
1.7.9.5

[U-Boot] [PATCH v2 1/6] i2c: add slave mode support

[Andrey Danin]

Signed-off-by: Andrey Danin <danindrey@mail.ru>
CC: Stephen Warren <swarren@nvidia.com>
CC: Marc Dietrich <marvin24@gmx.de>
CC: Julian Andres Klode <jak@jak-linux.org>
CC: ac100 at lists.launchpad.net
---
 drivers/i2c/i2c_core.c |   13 +++++++++++++
 include/i2c.h          |   30 +++++++++++++++++++++++++++++-
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/i2c_core.c b/drivers/i2c/i2c_core.c
index 18d6736..105aa0a 100644
--- a/drivers/i2c/i2c_core.c
+++ b/drivers/i2c/i2c_core.c
@@ -395,6 +395,19 @@ void i2c_reg_write(uint8_t addr, uint8_t reg, uint8_t val)
 	i2c_write(addr, reg, 1, &val, 1);
 }
 
+int i2c_slave_io(struct i2c_transaction *trans)
+{
+	struct i2c_adapter *cur = I2C_ADAP;
+
+	if (!cur->slave_io) {
+		printf("Error: slave IO is not supported on adap %d\n",
+		       cur->hwadapnr);
+		return -1;
+	}
+
+	return cur->slave_io(cur, trans);
+}
+
 void __i2c_init(int speed, int slaveaddr)
 {
 	i2c_init_bus(i2c_get_bus_num(), speed, slaveaddr);
diff --git a/include/i2c.h b/include/i2c.h
index f93a183..165b919 100644
--- a/include/i2c.h
+++ b/include/i2c.h
@@ -55,6 +55,20 @@
 #define CONFIG_SYS_SPD_BUS_NUM		0
 #endif
 
+struct i2c_transaction {
+	char rx_buf[34];
+	int rx_pos;
+
+	char tx_buf[34];
+	int tx_pos;
+	int tx_size;
+
+	unsigned int start_timeout;
+	unsigned int timeout;
+
+	int res;
+};
+
 struct i2c_adapter {
 	void		(*init)(struct i2c_adapter *adap, int speed,
 				int slaveaddr);
@@ -65,6 +79,8 @@ struct i2c_adapter {
 	int		(*write)(struct i2c_adapter *adap, uint8_t chip,
 				uint addr, int alen, uint8_t *buffer,
 				int len);
+	int		(*slave_io)(struct i2c_adapter *adap,
+				struct i2c_transaction *trans);
 	uint		(*set_bus_speed)(struct i2c_adapter *adap,
 				uint speed);
 	int		speed;
@@ -81,12 +97,13 @@ struct i2c_adapter {
 		.probe		=	_probe, \
 		.read		=	_read, \
 		.write		=	_write, \
+		.slave_io	=	0, \
 		.set_bus_speed	=	_set_speed, \
 		.speed		=	_speed, \
 		.slaveaddr	=	_slaveaddr, \
 		.init_done	=	0, \
 		.hwadapnr	=	_hwadapnr, \
-		.name		=	#_name \
+		.name		=	#_name, \
 };
 
 #define U_BOOT_I2C_ADAP_COMPLETE(_name, _init, _probe, _read, _write, \
@@ -450,4 +467,15 @@ int i2c_get_bus_num_fdt(int node);
  * @return 0 if port was reset, -1 if not found
  */
 int i2c_reset_port_fdt(const void *blob, int node);
+
+/**
+ * Perform I2C transaction with master device.
+ *
+ * @param trans  I2C transaction object
+ * @return 0 if succeeded, -1 if not supported,
+ *         1 if not ready, 2 if operation timed out,
+ *         3 if not our packet, other - unknown error.
+ */
+int i2c_slave_io(struct i2c_transaction *trans);
+
 #endif	/* _I2C_H_ */
-- 
1.7.9.5

[U-Boot] [Ac100] [PATCH v2 1/6] i2c: add slave mode support

[Stephen Warren]

On 04/26/2014 07:14 PM, Andrey Danin wrote:
> Signed-off-by: Andrey Danin <danindrey@mail.ru>
> CC: Stephen Warren <swarren@nvidia.com>
> CC: Marc Dietrich <marvin24@gmx.de>
> CC: Julian Andres Klode <jak@jak-linux.org>
> CC: ac100 at lists.launchpad.net

Don't you want to CC the I2C maintainer, Heiko Schocher <hs@denx.de>, on
this series?

[U-Boot] [Ac100] [PATCH v2 1/6] i2c: add slave mode support

[Andrey Danin]

On 29.04.2014 3:06, Stephen Warren wrote:
> On 04/26/2014 07:14 PM, Andrey Danin wrote:
>> Signed-off-by: Andrey Danin <danindrey@mail.ru>
>> CC: Stephen Warren <swarren@nvidia.com>
>> CC: Marc Dietrich <marvin24@gmx.de>
>> CC: Julian Andres Klode <jak@jak-linux.org>
>> CC: ac100 at lists.launchpad.net
>
> Don't you want to CC the I2C maintainer, Heiko Schocher <hs@denx.de>, on
> this series?
>
Sorry, I forgot to add him. I will add in v3.

[U-Boot] [PATCH v2 1/6] i2c: add slave mode support

[Heiko Schocher]

Hello Andrey,

Am 27.04.2014 03:14, schrieb Andrey Danin:
> Signed-off-by: Andrey Danin<danindrey@mail.ru>
> CC: Stephen Warren<swarren@nvidia.com>
> CC: Marc Dietrich<marvin24@gmx.de>
> CC: Julian Andres Klode<jak@jak-linux.org>
> CC: ac100 at lists.launchpad.net
> ---
>   drivers/i2c/i2c_core.c |   13 +++++++++++++
>   include/i2c.h          |   30 +++++++++++++++++++++++++++++-
>   2 files changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/i2c_core.c b/drivers/i2c/i2c_core.c
> index 18d6736..105aa0a 100644
> --- a/drivers/i2c/i2c_core.c
> +++ b/drivers/i2c/i2c_core.c
> @@ -395,6 +395,19 @@ void i2c_reg_write(uint8_t addr, uint8_t reg, uint8_t val)
>   	i2c_write(addr, reg, 1,&val, 1);
>   }
>
> +int i2c_slave_io(struct i2c_transaction *trans)
> +{
> +	struct i2c_adapter *cur = I2C_ADAP;
> +
> +	if (!cur->slave_io) {
> +		printf("Error: slave IO is not supported on adap %d\n",
> +		       cur->hwadapnr);
> +		return -1;
> +	}
> +
> +	return cur->slave_io(cur, trans);
> +}
> +
>   void __i2c_init(int speed, int slaveaddr)
>   {
>   	i2c_init_bus(i2c_get_bus_num(), speed, slaveaddr);
> diff --git a/include/i2c.h b/include/i2c.h
> index f93a183..165b919 100644
> --- a/include/i2c.h
> +++ b/include/i2c.h
> @@ -55,6 +55,20 @@
>   #define CONFIG_SYS_SPD_BUS_NUM		0
>   #endif
>
> +struct i2c_transaction {
> +	char rx_buf[34];
> +	int rx_pos;
> +
> +	char tx_buf[34];
> +	int tx_pos;
> +	int tx_size;
> +
> +	unsigned int start_timeout;
> +	unsigned int timeout;
> +
> +	int res;
> +};
> +
>   struct i2c_adapter {
>   	void		(*init)(struct i2c_adapter *adap, int speed,
>   				int slaveaddr);
> @@ -65,6 +79,8 @@ struct i2c_adapter {
>   	int		(*write)(struct i2c_adapter *adap, uint8_t chip,
>   				uint addr, int alen, uint8_t *buffer,
>   				int len);
> +	int		(*slave_io)(struct i2c_adapter *adap,
> +				struct i2c_transaction *trans);
>   	uint		(*set_bus_speed)(struct i2c_adapter *adap,
>   				uint speed);
>   	int		speed;
> @@ -81,12 +97,13 @@ struct i2c_adapter {
>   		.probe		=	_probe, \
>   		.read		=	_read, \
>   		.write		=	_write, \
> +		.slave_io	=	0, \
>   		.set_bus_speed	=	_set_speed, \
>   		.speed		=	_speed, \
>   		.slaveaddr	=	_slaveaddr, \
>   		.init_done	=	0, \
>   		.hwadapnr	=	_hwadapnr, \
> -		.name		=	#_name \
> +		.name		=	#_name, \
>   };
>
>   #define U_BOOT_I2C_ADAP_COMPLETE(_name, _init, _probe, _read, _write, \
> @@ -450,4 +467,15 @@ int i2c_get_bus_num_fdt(int node);
>    * @return 0 if port was reset, -1 if not found
>    */
>   int i2c_reset_port_fdt(const void *blob, int node);
> +
> +/**
> + * Perform I2C transaction with master device.
> + *
> + * @param trans  I2C transaction object
> + * @return 0 if succeeded, -1 if not supported,
> + *         1 if not ready, 2 if operation timed out,
> + *         3 if not our packet, other - unknown error.
> + */
> +int i2c_slave_io(struct i2c_transaction *trans);
> +
>   #endif	/* _I2C_H_ */

Hmm, why you use positiv error codes? Can we use negativ codes here
please? And maybe define somewhere this values as defines? Or can
you use:

include/asm-generic/errno.h

Beside of that, it looks Ok to me.

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH v2 2/6] ARM: tegra: i2c: add slave mode support

[Andrey Danin]

Signed-off-by: Andrey Danin <danindrey@mail.ru>
CC: Stephen Warren <swarren@nvidia.com>
CC: Marc Dietrich <marvin24@gmx.de>
CC: Julian Andres Klode <jak@jak-linux.org>
CC: ac100 at lists.launchpad.net
---
 arch/arm/include/asm/arch-tegra/tegra_i2c.h |    6 +
 drivers/i2c/tegra_i2c.c                     |  199 ++++++++++++++++++++++++++-
 2 files changed, 202 insertions(+), 3 deletions(-)

diff --git a/arch/arm/include/asm/arch-tegra/tegra_i2c.h b/arch/arm/include/asm/arch-tegra/tegra_i2c.h
index 853e59b..f818731 100644
--- a/arch/arm/include/asm/arch-tegra/tegra_i2c.h
+++ b/arch/arm/include/asm/arch-tegra/tegra_i2c.h
@@ -132,13 +132,19 @@ struct i2c_ctlr {
 				(1 << DVC_CTRL_REG3_I2C_HW_SW_PROG_SHIFT)
 
 /* I2C_CNFG */
+#define I2C_CNFG_DEBOUNCE_CNT_SHIFT	12
+#define I2C_CNFG_DEBOUNCE_CNT_MASK	(0x2 << I2C_CNFG_DEBOUNCE_CNT_SHIFT)
 #define I2C_CNFG_NEW_MASTER_FSM_SHIFT	11
 #define I2C_CNFG_NEW_MASTER_FSM_MASK	(1 << I2C_CNFG_NEW_MASTER_FSM_SHIFT)
 #define I2C_CNFG_PACKET_MODE_SHIFT	10
 #define I2C_CNFG_PACKET_MODE_MASK	(1 << I2C_CNFG_PACKET_MODE_SHIFT)
 
 /* I2C_SL_CNFG */
+#define I2C_SL_CNFG_RESP_SHIFT		0
+#define I2C_SL_CNFG_NACK_SHIFT		1
 #define I2C_SL_CNFG_NEWSL_SHIFT		2
+#define I2C_SL_CNFG_IRQ_SHIFT		3
+#define I2C_SL_CNFG_END_TRANS_SHIFT	4
 #define I2C_SL_CNFG_NEWSL_MASK		(1 << I2C_SL_CNFG_NEWSL_SHIFT)
 
 /* I2C_FIFO_STATUS */
diff --git a/drivers/i2c/tegra_i2c.c b/drivers/i2c/tegra_i2c.c
index 594e5dd..287a5df 100644
--- a/drivers/i2c/tegra_i2c.c
+++ b/drivers/i2c/tegra_i2c.c
@@ -22,6 +22,7 @@ DECLARE_GLOBAL_DATA_PTR;
 /* Information about i2c controller */
 struct i2c_bus {
 	int			id;
+	int			node;
 	enum periph_id		periph_id;
 	int			speed;
 	int			pinmux_config;
@@ -30,10 +31,31 @@ struct i2c_bus {
 	int			is_dvc;	/* DVC type, rather than I2C */
 	int			is_scs;	/* single clock source (T114+) */
 	int			inited;	/* bus is inited */
+	int			slave_addr;
 };
 
 static struct i2c_bus i2c_controllers[TEGRA_I2C_NUM_CONTROLLERS];
 
+/**
+ * Init i2c controller to operate in slave mode.
+ *
+ * @param bus	i2c bus/controller state struct
+ */
+static void set_slave_mode(struct i2c_bus *bus)
+{
+	unsigned long val;
+
+	val = I2C_CNFG_NEW_MASTER_FSM_MASK | I2C_CNFG_PACKET_MODE_MASK |
+	    I2C_CNFG_DEBOUNCE_CNT_MASK;
+	writel(val, &bus->regs->cnfg);
+
+	writel(I2C_SL_CNFG_NEWSL_MASK, &bus->regs->sl_cnfg);
+	writel(0x1E, &bus->regs->sl_delay_count);
+
+	writel(bus->slave_addr >> 1, &bus->regs->sl_addr1);
+	writel(0, &bus->regs->sl_addr2);
+}
+
 static void set_packet_mode(struct i2c_bus *i2c_bus)
 {
 	u32 config;
@@ -59,8 +81,12 @@ static void i2c_reset_controller(struct i2c_bus *i2c_bus)
 	/* Reset I2C controller. */
 	reset_periph(i2c_bus->periph_id, 1);
 
-	/* re-program config register to packet mode */
-	set_packet_mode(i2c_bus);
+	if (i2c_bus->slave_addr == 0) {
+		/* re-program config register to packet mode */
+		set_packet_mode(i2c_bus);
+	} else {
+		set_slave_mode(i2c_bus);
+	}
 }
 
 static void i2c_init_controller(struct i2c_bus *i2c_bus)
@@ -193,6 +219,121 @@ static int wait_for_transfer_complete(struct i2c_control *control)
 	return -1;
 }
 
+
+#define I2C_SL_IRQ		(1<<3)
+#define END_TRANS		(1<<4)
+#define RCVD			(1<<2)
+#define RNW			(1<<1)
+
+
+static inline int is_ready(unsigned long status)
+{
+	return status & I2C_SL_IRQ;
+}
+
+static inline int is_read(unsigned long status)
+{
+	return (status & RNW) == 0;
+}
+
+static inline int is_trans_start(unsigned long status)
+{
+	return status & RCVD;
+}
+
+static inline int is_trans_end(unsigned long status)
+{
+	return status & END_TRANS;
+}
+
+
+/**
+ * Send or receive packet in slave mode.
+ *
+ * @param i2c_bus	pointer to bus structure
+ * @param trans		I2C transaction object
+ *
+ * @return	0 if succeeded,
+ *		1 if not ready,
+ *		2 if operation timed out,
+ *		3 if not our packet,
+ *		other - unknown error.
+ */
+static int slave_send_recv_packets(struct i2c_bus *i2c_bus,
+				   struct i2c_transaction *trans)
+{
+	unsigned int poll_start_ms = 0;
+	unsigned long status;
+
+	unsigned int received = 0;
+	unsigned int to_send = 0;
+	unsigned int timer_ms = 0;
+	int addr = -1;
+
+	poll_start_ms = get_timer(0);
+
+	while (1) {
+		status = readl(&i2c_bus->regs->sl_status);
+		if (!is_ready(status)) {
+			timer_ms = get_timer(poll_start_ms);
+			if (addr != i2c_bus->slave_addr &&
+			    trans->start_timeout &&
+			    timer_ms > trans->start_timeout) {
+				trans->res = 1;
+				return 1; /*not ready*/
+			}
+
+			if (timer_ms > trans->timeout) {
+				trans->res = 2;
+				return 2; /*timeout*/
+			}
+
+			udelay(100);
+			continue;
+		}
+
+		if (!is_trans_start(status) && addr != i2c_bus->slave_addr) {
+			trans->res = 3;
+			return 3; /* not our packet, retry */
+		}
+
+		if (is_trans_start(status)) {
+			if (!is_read(status) && addr != i2c_bus->slave_addr) {
+				trans->res = 3;
+				return 3; /* not our packet, retry */
+			}
+			if (is_read(status)) {
+				addr = readl(&i2c_bus->regs->sl_rcvd);
+				trans->rx_buf[trans->rx_pos++] = addr;
+				continue;
+			}
+		}
+
+		if (is_trans_end(status)) {
+			/* Check for repeated start */
+			if (!is_trans_start(status)) {
+				trans->res = 0;
+				return 0;
+			}
+		}
+
+		if (is_read(status)) {
+			/* TODO Check sizes */
+			received = readl(&i2c_bus->regs->sl_rcvd);
+			trans->rx_buf[trans->rx_pos++] = received;
+		} else {
+			/* TODO Check sizes */
+			to_send = trans->tx_buf[trans->tx_pos++];
+			writel(to_send, &i2c_bus->regs->sl_rcvd);
+		}
+	}
+
+	/* not reachable */
+	trans->res = 4;
+	return 4;
+}
+
+
 static int send_recv_packets(struct i2c_bus *i2c_bus,
 			     struct i2c_trans_info *trans)
 {
@@ -343,6 +484,7 @@ static unsigned int tegra_i2c_set_bus_speed(struct i2c_adapter *adap,
 
 static int i2c_get_config(const void *blob, int node, struct i2c_bus *i2c_bus)
 {
+	i2c_bus->node = node;
 	i2c_bus->regs = (struct i2c_ctlr *)fdtdec_get_addr(blob, node, "reg");
 
 	/*
@@ -365,6 +507,8 @@ static int i2c_get_config(const void *blob, int node, struct i2c_bus *i2c_bus)
 	if (i2c_bus->periph_id == -1)
 		return -FDT_ERR_NOTFOUND;
 
+	i2c_bus->slave_addr = fdtdec_get_int(blob, node, "slave-addr", -1);
+
 	return 0;
 }
 
@@ -422,6 +566,28 @@ static int process_nodes(const void *blob, int node_list[], int count,
 	return 0;
 }
 
+static int tegra_i2c_slave_io(struct i2c_adapter *adap,
+			      struct i2c_transaction *trans)
+{
+	struct i2c_bus *bus;
+	debug("tegra_i2c_slave_io: hwadapnr=%d\n", adap->hwadapnr);
+
+	bus = tegra_i2c_get_bus(adap);
+	if (!bus) {
+		error("tegra_i2c_slave_io: no bus for adapter %d\n",
+		      adap->hwadapnr);
+		return -1;
+	}
+
+	if (!bus->slave_addr) {
+		error("tegra_i2c_slave_io: adapter %d isn't in slave mode\n",
+		      adap->hwadapnr);
+		return -2;
+	}
+
+	return slave_send_recv_packets(bus, trans);
+}
+
 /* Sadly there is no error return from this function */
 void i2c_init_board(void)
 {
@@ -453,11 +619,20 @@ void i2c_init_board(void)
 
 static void tegra_i2c_init(struct i2c_adapter *adap, int speed, int slaveaddr)
 {
+	struct i2c_bus *bus;
+
 	/* No i2c support prior to relocation */
 	if (!(gd->flags & GD_FLG_RELOC))
 		return;
 
-	/* This will override the speed selected in the fdt for that port */
+	bus = tegra_i2c_get_bus(adap);
+	if (bus) {
+		adap->slave_io = tegra_i2c_slave_io;
+		debug("i2c_init: ignore static init for adapter %d\n",
+		      adap->hwadapnr);
+		return;
+	}
+
 	debug("i2c_init(speed=%u, slaveaddr=0x%x)\n", speed, slaveaddr);
 	i2c_set_bus_speed(speed);
 }
@@ -614,6 +789,24 @@ int tegra_i2c_get_dvc_bus_num(void)
 	return -1;
 }
 
+/**
+ * Find the I2C bus number by given a FDT I2C node.
+ *
+ * @param blob  Device tree blbo
+ * @param node  FDT I2C node to find
+ * @return the number of I2C bus (zero based), or -1 on error
+ */
+int i2c_get_bus_num_fdt(int node)
+{
+	int i;
+
+	for (i = 0; i < TEGRA_I2C_NUM_CONTROLLERS; ++i)
+		if (i2c_controllers[i].node == node)
+			return i2c_controllers[i].id;
+
+	return -1;
+}
+
 /*
  * Register soft i2c adapters
  */
-- 
1.7.9.5

[U-Boot] [PATCH v2 2/6] ARM: tegra: i2c: add slave mode support

[Heiko Schocher]

Hello Andrey,

Am 27.04.2014 03:14, schrieb Andrey Danin:
> Signed-off-by: Andrey Danin<danindrey@mail.ru>
> CC: Stephen Warren<swarren@nvidia.com>
> CC: Marc Dietrich<marvin24@gmx.de>
> CC: Julian Andres Klode<jak@jak-linux.org>
> CC: ac100 at lists.launchpad.net
> ---
>   arch/arm/include/asm/arch-tegra/tegra_i2c.h |    6 +
>   drivers/i2c/tegra_i2c.c                     |  199 ++++++++++++++++++++++++++-
>   2 files changed, 202 insertions(+), 3 deletions(-)

only some nitpicking comments ...

[...]
> diff --git a/drivers/i2c/tegra_i2c.c b/drivers/i2c/tegra_i2c.c
> index 594e5dd..287a5df 100644
> --- a/drivers/i2c/tegra_i2c.c
> +++ b/drivers/i2c/tegra_i2c.c
> @@ -22,6 +22,7 @@ DECLARE_GLOBAL_DATA_PTR;
>   /* Information about i2c controller */
>   struct i2c_bus {
>   	int			id;
> +	int			node;
>   	enum periph_id		periph_id;
>   	int			speed;
>   	int			pinmux_config;
> @@ -30,10 +31,31 @@ struct i2c_bus {
>   	int			is_dvc;	/* DVC type, rather than I2C */
>   	int			is_scs;	/* single clock source (T114+) */
>   	int			inited;	/* bus is inited */
> +	int			slave_addr;
>   };
>
>   static struct i2c_bus i2c_controllers[TEGRA_I2C_NUM_CONTROLLERS];
>
> +/**
> + * Init i2c controller to operate in slave mode.
> + *
> + * @param bus	i2c bus/controller state struct
> + */
> +static void set_slave_mode(struct i2c_bus *bus)
> +{
> +	unsigned long val;
> +
> +	val = I2C_CNFG_NEW_MASTER_FSM_MASK | I2C_CNFG_PACKET_MODE_MASK |
> +	    I2C_CNFG_DEBOUNCE_CNT_MASK;
> +	writel(val,&bus->regs->cnfg);
> +
> +	writel(I2C_SL_CNFG_NEWSL_MASK,&bus->regs->sl_cnfg);
> +	writel(0x1E,&bus->regs->sl_delay_count);

Space after "," please. Please check globally.

> +
> +	writel(bus->slave_addr>>  1,&bus->regs->sl_addr1);
> +	writel(0,&bus->regs->sl_addr2);
> +}
> +
>   static void set_packet_mode(struct i2c_bus *i2c_bus)
>   {
>   	u32 config;
> @@ -59,8 +81,12 @@ static void i2c_reset_controller(struct i2c_bus *i2c_bus)
>   	/* Reset I2C controller. */
>   	reset_periph(i2c_bus->periph_id, 1);
>
> -	/* re-program config register to packet mode */
> -	set_packet_mode(i2c_bus);
> +	if (i2c_bus->slave_addr == 0) {
> +		/* re-program config register to packet mode */
> +		set_packet_mode(i2c_bus);
> +	} else {
> +		set_slave_mode(i2c_bus);
> +	}
>   }
>
>   static void i2c_init_controller(struct i2c_bus *i2c_bus)
> @@ -193,6 +219,121 @@ static int wait_for_transfer_complete(struct i2c_control *control)
>   	return -1;
>   }
>
> +
> +#define I2C_SL_IRQ		(1<<3)
> +#define END_TRANS		(1<<4)
> +#define RCVD			(1<<2)
> +#define RNW			(1<<1)
> +
> +

only one empty line please. Please check globally.

> +static inline int is_ready(unsigned long status)
> +{
> +	return status&  I2C_SL_IRQ;
> +}
> +
> +static inline int is_read(unsigned long status)
> +{
> +	return (status&  RNW) == 0;
> +}
> +
> +static inline int is_trans_start(unsigned long status)
> +{
> +	return status&  RCVD;
> +}
> +
> +static inline int is_trans_end(unsigned long status)
> +{
> +	return status&  END_TRANS;
> +}
> +
> +
> +/**
> + * Send or receive packet in slave mode.
> + *
> + * @param i2c_bus	pointer to bus structure
> + * @param trans		I2C transaction object
> + *
> + * @return	0 if succeeded,
> + *		1 if not ready,
> + *		2 if operation timed out,
> + *		3 if not our packet,
> + *		other - unknown error.
> + */

See my reply to your patch 1/6. Can we use negativ error codes and
defines?

> +static int slave_send_recv_packets(struct i2c_bus *i2c_bus,
> +				   struct i2c_transaction *trans)
> +{
> +	unsigned int poll_start_ms = 0;
> +	unsigned long status;
> +
> +	unsigned int received = 0;
> +	unsigned int to_send = 0;
> +	unsigned int timer_ms = 0;
> +	int addr = -1;
> +
> +	poll_start_ms = get_timer(0);
> +
> +	while (1) {
> +		status = readl(&i2c_bus->regs->sl_status);
> +		if (!is_ready(status)) {
> +			timer_ms = get_timer(poll_start_ms);
> +			if (addr != i2c_bus->slave_addr&&
> +			    trans->start_timeout&&
> +			    timer_ms>  trans->start_timeout) {

please add a space before "&&" and ">". Please check globally.

> +				trans->res = 1;
> +				return 1; /*not ready*/
> +			}
> +
> +			if (timer_ms>  trans->timeout) {
> +				trans->res = 2;
> +				return 2; /*timeout*/
> +			}
> +
> +			udelay(100);
> +			continue;
> +		}
> +
> +		if (!is_trans_start(status)&&  addr != i2c_bus->slave_addr) {
> +			trans->res = 3;
> +			return 3; /* not our packet, retry */
> +		}
> +
> +		if (is_trans_start(status)) {
> +			if (!is_read(status)&&  addr != i2c_bus->slave_addr) {
> +				trans->res = 3;
> +				return 3; /* not our packet, retry */
> +			}
> +			if (is_read(status)) {
> +				addr = readl(&i2c_bus->regs->sl_rcvd);
> +				trans->rx_buf[trans->rx_pos++] = addr;
> +				continue;
> +			}
> +		}
> +
> +		if (is_trans_end(status)) {
> +			/* Check for repeated start */
> +			if (!is_trans_start(status)) {
> +				trans->res = 0;
> +				return 0;
> +			}
> +		}
> +
> +		if (is_read(status)) {
> +			/* TODO Check sizes */
> +			received = readl(&i2c_bus->regs->sl_rcvd);
> +			trans->rx_buf[trans->rx_pos++] = received;
> +		} else {
> +			/* TODO Check sizes */
> +			to_send = trans->tx_buf[trans->tx_pos++];
> +			writel(to_send,&i2c_bus->regs->sl_rcvd);
> +		}
> +	}
> +
> +	/* not reachable */
> +	trans->res = 4;
> +	return 4;

What is error code 4? Please use defines ...

> +}
> +
> +
>   static int send_recv_packets(struct i2c_bus *i2c_bus,
>   			     struct i2c_trans_info *trans)
>   {
> @@ -343,6 +484,7 @@ static unsigned int tegra_i2c_set_bus_speed(struct i2c_adapter *adap,
>
>   static int i2c_get_config(const void *blob, int node, struct i2c_bus *i2c_bus)
>   {
> +	i2c_bus->node = node;
>   	i2c_bus->regs = (struct i2c_ctlr *)fdtdec_get_addr(blob, node, "reg");
>
>   	/*
> @@ -365,6 +507,8 @@ static int i2c_get_config(const void *blob, int node, struct i2c_bus *i2c_bus)
>   	if (i2c_bus->periph_id == -1)
>   		return -FDT_ERR_NOTFOUND;
>
> +	i2c_bus->slave_addr = fdtdec_get_int(blob, node, "slave-addr", -1);
> +
>   	return 0;
>   }
>
> @@ -422,6 +566,28 @@ static int process_nodes(const void *blob, int node_list[], int count,
>   	return 0;
>   }
>
> +static int tegra_i2c_slave_io(struct i2c_adapter *adap,
> +			      struct i2c_transaction *trans)
> +{
> +	struct i2c_bus *bus;
> +	debug("tegra_i2c_slave_io: hwadapnr=%d\n", adap->hwadapnr);
> +
> +	bus = tegra_i2c_get_bus(adap);
> +	if (!bus) {
> +		error("tegra_i2c_slave_io: no bus for adapter %d\n",
> +		      adap->hwadapnr);
> +		return -1;
> +	}
> +
> +	if (!bus->slave_addr) {
> +		error("tegra_i2c_slave_io: adapter %d isn't in slave mode\n",
> +		      adap->hwadapnr);
> +		return -2;
> +	}
> +
> +	return slave_send_recv_packets(bus, trans);
> +}
> +
>   /* Sadly there is no error return from this function */
>   void i2c_init_board(void)
>   {
> @@ -453,11 +619,20 @@ void i2c_init_board(void)
>
>   static void tegra_i2c_init(struct i2c_adapter *adap, int speed, int slaveaddr)
>   {
> +	struct i2c_bus *bus;
> +
>   	/* No i2c support prior to relocation */
>   	if (!(gd->flags&  GD_FLG_RELOC))
>   		return;
>
> -	/* This will override the speed selected in the fdt for that port */
> +	bus = tegra_i2c_get_bus(adap);
> +	if (bus) {
> +		adap->slave_io = tegra_i2c_slave_io;
> +		debug("i2c_init: ignore static init for adapter %d\n",
> +		      adap->hwadapnr);
> +		return;
> +	}
> +
>   	debug("i2c_init(speed=%u, slaveaddr=0x%x)\n", speed, slaveaddr);
>   	i2c_set_bus_speed(speed);
>   }
> @@ -614,6 +789,24 @@ int tegra_i2c_get_dvc_bus_num(void)
>   	return -1;
>   }
>
> +/**
> + * Find the I2C bus number by given a FDT I2C node.
> + *
> + * @param blob  Device tree blbo
> + * @param node  FDT I2C node to find
> + * @return the number of I2C bus (zero based), or -1 on error
> + */
> +int i2c_get_bus_num_fdt(int node)
> +{
> +	int i;
> +
> +	for (i = 0; i<  TEGRA_I2C_NUM_CONTROLLERS; ++i)
> +		if (i2c_controllers[i].node == node)
> +			return i2c_controllers[i].id;
> +
> +	return -1;
> +}
> +
>   /*
>    * Register soft i2c adapters
>    */

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH v2 3/6] ARM: tegra: i2c: add nvec driver

[Andrey Danin]

Signed-off-by: Andrey Danin <danindrey@mail.ru>
CC: Stephen Warren <swarren@nvidia.com>
CC: Marc Dietrich <marvin24@gmx.de>
CC: Julian Andres Klode <jak@jak-linux.org>
CC: ac100 at lists.launchpad.net
---
 Changes for v2:
	- NVEC driver was reworked to use tegra-i2c

 arch/arm/include/asm/arch-tegra/tegra_nvec.h |  130 ++++++++++++
 board/nvidia/common/board.c                  |   12 ++
 drivers/i2c/Makefile                         |    1 +
 drivers/i2c/tegra_nvec.c                     |  294 ++++++++++++++++++++++++++
 include/fdtdec.h                             |    1 +
 lib/fdtdec.c                                 |    1 +
 6 files changed, 439 insertions(+)
 create mode 100644 arch/arm/include/asm/arch-tegra/tegra_nvec.h
 create mode 100644 drivers/i2c/tegra_nvec.c

diff --git a/arch/arm/include/asm/arch-tegra/tegra_nvec.h b/arch/arm/include/asm/arch-tegra/tegra_nvec.h
new file mode 100644
index 0000000..f1cec0d
--- /dev/null
+++ b/arch/arm/include/asm/arch-tegra/tegra_nvec.h
@@ -0,0 +1,130 @@
+/*
+ * (C) Copyright 2014
+ * Andrey Danin <danindrey@mail.ru>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef _TEGRA_NVEC_H_
+#define _TEGRA_NVEC_H_
+
+#define I2C_CNFG			0x00
+#define I2C_CNFG_PACKET_MODE_EN		(1<<10)
+#define I2C_CNFG_NEW_MASTER_SFM		(1<<11)
+#define I2C_CNFG_DEBOUNCE_CNT_SHIFT	12
+
+#define I2C_SL_CNFG		0x20
+#define I2C_SL_NEWSL		(1<<2)
+#define I2C_SL_NACK		(1<<1)
+#define I2C_SL_RESP		(1<<0)
+#define I2C_SL_IRQ		(1<<3)
+#define END_TRANS		(1<<4)
+#define RCVD			(1<<2)
+#define RNW			(1<<1)
+
+#define I2C_SL_RCVD		0x24
+#define I2C_SL_STATUS		0x28
+#define I2C_SL_ADDR1		0x2c
+#define I2C_SL_ADDR2		0x30
+#define I2C_SL_DELAY_COUNT	0x3c
+
+
+enum nvec_msg_type {
+	NVEC_KEYBOARD = 0,
+	NVEC_SYS = 1,
+	NVEC_BAT,
+	NVEC_GPIO,
+	NVEC_SLEEP,
+	NVEC_KBD,
+	NVEC_PS2,
+	NVEC_CNTL,
+	NVEC_OEM0 = 0x0d,
+	NVEC_KB_EVT = 0x80,
+	NVEC_PS2_EVT,
+	NVEC_LAST_MSG,
+};
+
+enum nvec_event_size {
+	NVEC_2BYTES,
+	NVEC_3BYTES,
+	NVEC_VAR_SIZE,
+};
+
+enum sys_subcmds {
+	SYS_GET_STATUS,
+	SYS_CNFG_EVENT_REPORTING,
+	SYS_ACK_STATUS,
+	SYS_CNFG_WAKE = 0xfd,
+};
+
+enum kbd_subcmds {
+	CNFG_WAKE = 3,
+	CNFG_WAKE_KEY_REPORTING,
+	SET_LEDS = 0xed,
+	ENABLE_KBD = 0xf4,
+	DISABLE_KBD,
+};
+
+enum cntl_subcmds {
+	CNTL_RESET_EC = 0x00,
+	CNTL_SELF_TEST = 0x01,
+	CNTL_NOOP = 0x02,
+	CNTL_GET_EC_SPEC_VER = 0x10,
+	CNTL_GET_FIRMWARE_VERSION = 0x15,
+};
+
+enum nvec_sleep_subcmds {
+	GLOBAL_EVENTS,
+	AP_PWR_DOWN,
+	AP_SUSPEND,
+};
+
+enum ps2_subcmds {
+	MOUSE_SEND_CMD = 0x01
+};
+
+#define MOUSE_RESET 0xff
+
+typedef int (*periph_start)(void);
+typedef int (*periph_process_msg)(const unsigned char *);
+
+struct nvec_periph {
+	periph_start		start;
+	periph_process_msg	process_msg;
+};
+
+
+int board_nvec_init(void);
+
+/**
+ * Read all available events.
+ *
+ * @return count of available events if ok, -1 on error
+ */
+int nvec_read_events(void);
+
+int nvec_msg_is_event(const unsigned char *msg);
+int nvec_msg_event_type(const unsigned char *msg);
+
+/**
+ * Register perepherial device driver.
+ *
+ * @param msg_type	type of messages that divece processes.
+ * @param periph	pointer to device description.
+ *
+ * @return 0 if ok, -1 on error
+ */
+int nvec_register_periph(int msg_type, struct nvec_periph *periph);
+
+/**
+ * Send request and read response. If write or read failed
+ * operation will be repeated NVEC_ATTEMPTS_MAX times.
+ *
+ * @param buf		request data
+ * @param size		request data size
+ * @return 0 if ok, -1 on error
+ */
+int nvec_do_request(char *buf, int size);
+
+
+#endif /* _TEGRA_NVEC_H_ */
diff --git a/board/nvidia/common/board.c b/board/nvidia/common/board.c
index 3b18e28..6d3055c 100644
--- a/board/nvidia/common/board.c
+++ b/board/nvidia/common/board.c
@@ -38,6 +38,9 @@
 #include <asm/arch-tegra/tegra_mmc.h>
 #include <asm/arch-tegra/mmc.h>
 #endif
+#ifdef CONFIG_SYS_I2C_TEGRA_NVEC
+#include <asm/arch-tegra/tegra_nvec.h>
+#endif
 #include <i2c.h>
 #include <spi.h>
 #include "emc.h"
@@ -194,10 +197,19 @@ int board_early_init_f(void)
 
 int board_late_init(void)
 {
+	__maybe_unused int err;
+
 #ifdef CONFIG_LCD
 	/* Make sure we finish initing the LCD */
 	tegra_lcd_check_next_stage(gd->fdt_blob, 1);
 #endif
+
+#ifdef CONFIG_SYS_I2C_TEGRA_NVEC
+	err = board_nvec_init();
+	if (err)
+		debug("NVEC controller init failed: %d\n", err);
+#endif
+
 	return 0;
 }
 
diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
index fa3a875..3041191 100644
--- a/drivers/i2c/Makefile
+++ b/drivers/i2c/Makefile
@@ -27,4 +27,5 @@ obj-$(CONFIG_SYS_I2C_S3C24X0) += s3c24x0_i2c.o
 obj-$(CONFIG_SYS_I2C_SH) += sh_i2c.o
 obj-$(CONFIG_SYS_I2C_SOFT) += soft_i2c.o
 obj-$(CONFIG_SYS_I2C_TEGRA) += tegra_i2c.o
+obj-$(CONFIG_SYS_I2C_TEGRA_NVEC) += tegra_nvec.o
 obj-$(CONFIG_SYS_I2C_ZYNQ) += zynq_i2c.o
diff --git a/drivers/i2c/tegra_nvec.c b/drivers/i2c/tegra_nvec.c
new file mode 100644
index 0000000..b568988
--- /dev/null
+++ b/drivers/i2c/tegra_nvec.c
@@ -0,0 +1,294 @@
+/*
+ * (C) Copyright 2014
+ * Andrey Danin <danindrey@mail.ru>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <fdtdec.h>
+#include <i2c.h>
+#include <asm/io.h>
+#include <asm/gpio.h>
+#include <asm/arch/clock.h>
+#include <asm/arch/funcmux.h>
+#include <asm/arch-tegra/tegra_nvec.h>
+
+#ifndef CONFIG_SYS_I2C_TEGRA_NVEC
+#error "You should enable CONFIG_SYS_I2C_TEGRA_NVEC"
+#endif
+
+DECLARE_GLOBAL_DATA_PTR;
+
+struct nvec_periph devices[NVEC_LAST_MSG];
+
+
+struct nvec_t {
+	int			i2c_bus;
+	struct fdt_gpio_state	gpio;
+} nvec_data;
+
+
+/* nvec commands */
+char noop[] = { NVEC_CNTL, CNTL_NOOP };
+
+void nvec_signal_request(void)
+{
+	gpio_set_value(nvec_data.gpio.gpio, 0);
+}
+
+
+void nvec_clear_request(void)
+{
+	gpio_set_value(nvec_data.gpio.gpio, 1);
+}
+
+
+int nvec_msg_is_event(const unsigned char *msg)
+{
+	return msg[0] >> 7;
+}
+
+
+int nvec_msg_event_type(const unsigned char *msg)
+{
+	return msg[0] & 0x0f;
+}
+
+
+/**
+ * Process incoming io message.
+ * If message is keyboard event then key code will
+ * be added to keys buffer.
+ *
+ * @param nvec	nvec state struct
+ */
+void nvec_process_msg(struct nvec_t *nvec, struct i2c_transaction *trans)
+{
+	(void) nvec;
+
+	const unsigned char *msg = (const unsigned char *)&trans->rx_buf[1];
+	int event_type;
+
+	if (!nvec_msg_is_event(msg))
+		return;
+
+	event_type = nvec_msg_event_type(msg);
+
+	if (event_type < NVEC_KEYBOARD || event_type >= NVEC_LAST_MSG)
+		return;
+
+	if (devices[event_type].process_msg)
+		devices[event_type].process_msg(msg);
+}
+
+
+/**
+ * Perform complete io operation (read or write).
+ * NOTE: function will wait NVEC_TIMEOUT_MIN (20ms)
+ * before status check to avoid nvec hang.
+ *
+ * @param nvec		nvec state struct
+ * @param wait_for_ec	if 1(NVEC_WAIT_FOR_EC) operation
+ *			timeout is NVEC_TIMEOUT_MAX (600ms),
+ *			otherwise function will return if io
+ *			is not ready.
+ *
+ * @return nvec_io_* code
+ */
+int nvec_do_io(struct nvec_t *nvec)
+{
+	static unsigned int prev_timer;
+	unsigned int poll_start_ms = get_timer(prev_timer);
+	struct i2c_transaction trans;
+	int res;
+
+	if (poll_start_ms > 30000) {
+		if (prev_timer != 0)
+			nvec_do_request(noop, sizeof(noop));
+		prev_timer = get_timer(0);
+	}
+
+	memset(&trans, 0, sizeof(trans));
+	trans.start_timeout = 1;
+	trans.timeout = 6000;
+
+	res = i2c_slave_io(&trans);
+	if (res == 0) {
+		nvec_process_msg(nvec, &trans);
+		return 0;
+	}
+
+	if (res != -1)
+		debug("Error: i2c slave io failed with code %d\n", res);
+
+	return -1;
+}
+
+/**
+ * Send request and read response. If write or read failed
+ * operation will be repeated NVEC_ATTEMPTS_MAX times.
+ *
+ * @param buf	request data
+ * @param size	request data size
+ * @return 0 if ok, -1 on error
+ */
+int nvec_do_request(char *buf, int size)
+{
+	int res;
+	struct i2c_transaction trans;
+	int i;
+
+	nvec_signal_request();
+
+	/* Request */
+	for (i = 0; i < 10; ++i) {
+		memset(&trans, 0, sizeof(trans));
+		trans.start_timeout = 600;
+		trans.timeout = 6000;
+
+		trans.tx_buf[0] = (char)size;
+		memcpy(&trans.tx_buf[1], buf, size);
+		trans.tx_size = size + 1;
+
+		res = i2c_slave_io(&trans);
+		if (res == 0) {
+			if (trans.tx_pos == trans.tx_size)
+				break;
+
+			debug("Request was not sent completely");
+		} else if (res != -1) {
+			debug("Unknown error while slave io");
+		}
+	}
+	nvec_clear_request();
+	if (res != 0) {
+		error("nvec failed to perform request\n");
+		return -1;
+	}
+
+	/* Response */
+	for (i = 0; i < 10; ++i) {
+		memset(&trans, 0, sizeof(trans));
+		trans.start_timeout = 600;
+		trans.timeout = 6000;
+
+		res = i2c_slave_io(&trans);
+		if (res == 0)
+			break;
+	}
+	if (res != 0) {
+		error("nvec failed to read response\n");
+		return -1;
+	}
+
+	/* TODO Parse response */
+
+	return 0;
+}
+
+
+/**
+ * Decode the nvec information from the fdt.
+ *
+ * @param blob		fdt blob
+ * @param nvec		nvec device sturct
+ * @return 0 if ok, -ve on error
+ */
+static int nvec_decode_config(const void *blob,
+			      struct nvec_t *nvec)
+{
+	int node, parent;
+	int i2c_bus;
+
+	node = fdtdec_next_compatible(blob, 0, COMPAT_NVIDIA_TEGRA20_NVEC);
+	if (node < 0) {
+		error("Cannot find NVEC node in fdt\n");
+		return node;
+	}
+
+	parent = fdt_parent_offset(blob, node);
+	if (parent < 0) {
+		debug("%s: Cannot find node parent\n", __func__);
+		return -1;
+	}
+
+	i2c_bus = i2c_get_bus_num_fdt(parent);
+	if (i2c_bus < 0)
+		return -1;
+	nvec->i2c_bus = i2c_bus;
+
+	if (fdtdec_decode_gpio(blob, node, "request-gpios",
+			       &nvec->gpio)) {
+		error("No NVEC request gpio\n");
+		return -1;
+	}
+
+	debug("NVEC: i2c:%d, gpio:%s(%u)\n",
+	      nvec->i2c_bus, nvec->gpio.name, nvec->gpio.gpio);
+	return 0;
+}
+
+
+int board_nvec_init(void)
+{
+	int res = 0;
+	int i;
+
+	if (nvec_decode_config(gd->fdt_blob, &nvec_data)) {
+		error("Can't parse NVEC node in device tree\n");
+		return -1;
+	}
+
+	debug("NVEC initialization...\n");
+
+	res = gpio_request(nvec_data.gpio.gpio, NULL);
+	if (res != 0)
+		error("NVEC: err, gpio_request\n");
+	res = gpio_direction_output(nvec_data.gpio.gpio, 1);
+	if (res != 0)
+		error("NVEC: err, gpio_direction\n");
+	res = gpio_set_value(nvec_data.gpio.gpio, 1);
+	if (res != 0)
+		error("NVEC: err, gpio_set_value\n");
+	udelay(100);
+
+	i2c_set_bus_num(nvec_data.i2c_bus);
+
+	for (i = NVEC_KEYBOARD; i < NVEC_LAST_MSG; ++i)
+		if (devices[i].start) {
+			debug("Starting device %d(0x%x)\n", i, i);
+			devices[i].start();
+		}
+
+	return 1;
+}
+
+
+int nvec_read_events(void)
+{
+	int res;
+	int cnt = 0;
+
+	while (++cnt <= 8) {
+		res = nvec_do_io(&nvec_data);
+		if (res)
+			break;
+
+		/* TODO Process nvec communication errors */
+	}
+
+	return cnt;
+}
+
+
+int nvec_register_periph(int msg_type, struct nvec_periph *periph)
+{
+	if (devices[msg_type].start || devices[msg_type].process_msg) {
+		error("Device for msg %d already registered\n", msg_type);
+		return -1;
+	}
+
+	devices[msg_type] = *periph;
+	return 0;
+}
diff --git a/include/fdtdec.h b/include/fdtdec.h
index 19bab79..7b84335 100644
--- a/include/fdtdec.h
+++ b/include/fdtdec.h
@@ -64,6 +64,7 @@ enum fdt_compat_id {
 	COMPAT_NVIDIA_TEGRA20_SDMMC,	/* Tegra20 SDMMC controller */
 	COMPAT_NVIDIA_TEGRA20_SFLASH,	/* Tegra 2 SPI flash controller */
 	COMPAT_NVIDIA_TEGRA20_SLINK,	/* Tegra 2 SPI SLINK controller */
+	COMPAT_NVIDIA_TEGRA20_NVEC,	/* Tegra 2 EC controller */
 	COMPAT_NVIDIA_TEGRA114_SPI,	/* Tegra 114 SPI controller */
 	COMPAT_SMSC_LAN9215,		/* SMSC 10/100 Ethernet LAN9215 */
 	COMPAT_SAMSUNG_EXYNOS5_SROMC,	/* Exynos5 SROMC */
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 1fecab3..82df701 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -37,6 +37,7 @@ static const char * const compat_names[COMPAT_COUNT] = {
 	COMPAT(NVIDIA_TEGRA20_SDMMC, "nvidia,tegra20-sdhci"),
 	COMPAT(NVIDIA_TEGRA20_SFLASH, "nvidia,tegra20-sflash"),
 	COMPAT(NVIDIA_TEGRA20_SLINK, "nvidia,tegra20-slink"),
+	COMPAT(NVIDIA_TEGRA20_NVEC, "nvidia,tegra20-nvec"),
 	COMPAT(NVIDIA_TEGRA114_SPI, "nvidia,tegra114-spi"),
 	COMPAT(SMSC_LAN9215, "smsc,lan9215"),
 	COMPAT(SAMSUNG_EXYNOS5_SROMC, "samsung,exynos-sromc"),
-- 
1.7.9.5

[U-Boot] [PATCH v2 3/6] ARM: tegra: i2c: add nvec driver

[Heiko Schocher]

Hello Andrey,

Am 27.04.2014 03:14, schrieb Andrey Danin:
> Signed-off-by: Andrey Danin<danindrey@mail.ru>
> CC: Stephen Warren<swarren@nvidia.com>
> CC: Marc Dietrich<marvin24@gmx.de>
> CC: Julian Andres Klode<jak@jak-linux.org>
> CC: ac100 at lists.launchpad.net
> ---
>   Changes for v2:
> 	- NVEC driver was reworked to use tegra-i2c
>
>   arch/arm/include/asm/arch-tegra/tegra_nvec.h |  130 ++++++++++++
>   board/nvidia/common/board.c                  |   12 ++
>   drivers/i2c/Makefile                         |    1 +
>   drivers/i2c/tegra_nvec.c                     |  294 ++++++++++++++++++++++++++
>   include/fdtdec.h                             |    1 +
>   lib/fdtdec.c                                 |    1 +
>   6 files changed, 439 insertions(+)
>   create mode 100644 arch/arm/include/asm/arch-tegra/tegra_nvec.h
>   create mode 100644 drivers/i2c/tegra_nvec.c
>
> diff --git a/arch/arm/include/asm/arch-tegra/tegra_nvec.h b/arch/arm/include/asm/arch-tegra/tegra_nvec.h
> new file mode 100644
> index 0000000..f1cec0d
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-tegra/tegra_nvec.h
> @@ -0,0 +1,130 @@
> +/*
> + * (C) Copyright 2014
> + * Andrey Danin<danindrey@mail.ru>
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#ifndef _TEGRA_NVEC_H_
> +#define _TEGRA_NVEC_H_
> +
> +#define I2C_CNFG			0x00
> +#define I2C_CNFG_PACKET_MODE_EN		(1<<10)
> +#define I2C_CNFG_NEW_MASTER_SFM		(1<<11)
> +#define I2C_CNFG_DEBOUNCE_CNT_SHIFT	12
> +
> +#define I2C_SL_CNFG		0x20
> +#define I2C_SL_NEWSL		(1<<2)
> +#define I2C_SL_NACK		(1<<1)
> +#define I2C_SL_RESP		(1<<0)
> +#define I2C_SL_IRQ		(1<<3)
> +#define END_TRANS		(1<<4)
> +#define RCVD			(1<<2)
> +#define RNW			(1<<1)
> +
> +#define I2C_SL_RCVD		0x24
> +#define I2C_SL_STATUS		0x28
> +#define I2C_SL_ADDR1		0x2c
> +#define I2C_SL_ADDR2		0x30
> +#define I2C_SL_DELAY_COUNT	0x3c
> +
> +

please only one line, check globally.

[...]
> diff --git a/drivers/i2c/tegra_nvec.c b/drivers/i2c/tegra_nvec.c
> new file mode 100644
> index 0000000..b568988
> --- /dev/null
> +++ b/drivers/i2c/tegra_nvec.c
> @@ -0,0 +1,294 @@
> +/*
> + * (C) Copyright 2014
> + * Andrey Danin<danindrey@mail.ru>
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#include<common.h>
> +#include<fdtdec.h>
> +#include<i2c.h>
> +#include<asm/io.h>
> +#include<asm/gpio.h>
> +#include<asm/arch/clock.h>
> +#include<asm/arch/funcmux.h>
> +#include<asm/arch-tegra/tegra_nvec.h>
> +
> +#ifndef CONFIG_SYS_I2C_TEGRA_NVEC
> +#error "You should enable CONFIG_SYS_I2C_TEGRA_NVEC"
> +#endif
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +struct nvec_periph devices[NVEC_LAST_MSG];
> +
> +
> +struct nvec_t {
> +	int			i2c_bus;
> +	struct fdt_gpio_state	gpio;
> +} nvec_data;
> +
> +
> +/* nvec commands */
> +char noop[] = { NVEC_CNTL, CNTL_NOOP };
> +
> +void nvec_signal_request(void)
> +{
> +	gpio_set_value(nvec_data.gpio.gpio, 0);
> +}
> +
> +
> +void nvec_clear_request(void)
> +{
> +	gpio_set_value(nvec_data.gpio.gpio, 1);
> +}
> +
> +
> +int nvec_msg_is_event(const unsigned char *msg)
> +{
> +	return msg[0]>>  7;
> +}
> +
> +
> +int nvec_msg_event_type(const unsigned char *msg)
> +{
> +	return msg[0]&  0x0f;
> +}
> +
> +
> +/**
> + * Process incoming io message.
> + * If message is keyboard event then key code will
> + * be added to keys buffer.
> + *
> + * @param nvec	nvec state struct
> + */
> +void nvec_process_msg(struct nvec_t *nvec, struct i2c_transaction *trans)
> +{
> +	(void) nvec;
> +

no empty line needed.

> +	const unsigned char *msg = (const unsigned char *)&trans->rx_buf[1];
> +	int event_type;
> +
> +	if (!nvec_msg_is_event(msg))
> +		return;
> +
> +	event_type = nvec_msg_event_type(msg);
> +
> +	if (event_type<  NVEC_KEYBOARD || event_type>= NVEC_LAST_MSG)
> +		return;
> +
> +	if (devices[event_type].process_msg)
> +		devices[event_type].process_msg(msg);
> +}
> +
> +
> +/**
> + * Perform complete io operation (read or write).
> + * NOTE: function will wait NVEC_TIMEOUT_MIN (20ms)
> + * before status check to avoid nvec hang.
> + *
> + * @param nvec		nvec state struct
> + * @param wait_for_ec	if 1(NVEC_WAIT_FOR_EC) operation
> + *			timeout is NVEC_TIMEOUT_MAX (600ms),
> + *			otherwise function will return if io
> + *			is not ready.
> + *
> + * @return nvec_io_* code
> + */
> +int nvec_do_io(struct nvec_t *nvec)
> +{
> +	static unsigned int prev_timer;
> +	unsigned int poll_start_ms = get_timer(prev_timer);
> +	struct i2c_transaction trans;
> +	int res;
> +
> +	if (poll_start_ms>  30000) {
> +		if (prev_timer != 0)
> +			nvec_do_request(noop, sizeof(noop));
> +		prev_timer = get_timer(0);
> +	}
> +
> +	memset(&trans, 0, sizeof(trans));
> +	trans.start_timeout = 1;
> +	trans.timeout = 6000;
> +
> +	res = i2c_slave_io(&trans);
> +	if (res == 0) {
> +		nvec_process_msg(nvec,&trans);
> +		return 0;
> +	}
> +
> +	if (res != -1)
> +		debug("Error: i2c slave io failed with code %d\n", res);
> +
> +	return -1;
> +}
> +
> +/**
> + * Send request and read response. If write or read failed
> + * operation will be repeated NVEC_ATTEMPTS_MAX times.
> + *
> + * @param buf	request data
> + * @param size	request data size
> + * @return 0 if ok, -1 on error
> + */
> +int nvec_do_request(char *buf, int size)
> +{
> +	int res;
> +	struct i2c_transaction trans;
> +	int i;
> +
> +	nvec_signal_request();
> +
> +	/* Request */
> +	for (i = 0; i<  10; ++i) {
> +		memset(&trans, 0, sizeof(trans));
> +		trans.start_timeout = 600;
> +		trans.timeout = 6000;
> +
> +		trans.tx_buf[0] = (char)size;
> +		memcpy(&trans.tx_buf[1], buf, size);
> +		trans.tx_size = size + 1;
> +
> +		res = i2c_slave_io(&trans);
> +		if (res == 0) {
> +			if (trans.tx_pos == trans.tx_size)
> +				break;
> +
> +			debug("Request was not sent completely");
> +		} else if (res != -1) {
> +			debug("Unknown error while slave io");
> +		}
> +	}
> +	nvec_clear_request();
> +	if (res != 0) {
> +		error("nvec failed to perform request\n");
> +		return -1;
> +	}
> +
> +	/* Response */
> +	for (i = 0; i<  10; ++i) {
> +		memset(&trans, 0, sizeof(trans));
> +		trans.start_timeout = 600;
> +		trans.timeout = 6000;
> +
> +		res = i2c_slave_io(&trans);
> +		if (res == 0)
> +			break;
> +	}
> +	if (res != 0) {
> +		error("nvec failed to read response\n");
> +		return -1;
> +	}
> +
> +	/* TODO Parse response */

?

Is this not a complete driver?

> +
> +	return 0;
> +}
> +
> +
> +/**
> + * Decode the nvec information from the fdt.
> + *
> + * @param blob		fdt blob
> + * @param nvec		nvec device sturct
> + * @return 0 if ok, -ve on error

What is "-ve" ?

> + */
> +static int nvec_decode_config(const void *blob,
> +			      struct nvec_t *nvec)
> +{
> +	int node, parent;
> +	int i2c_bus;
> +
> +	node = fdtdec_next_compatible(blob, 0, COMPAT_NVIDIA_TEGRA20_NVEC);
> +	if (node<  0) {
> +		error("Cannot find NVEC node in fdt\n");
> +		return node;
> +	}
> +
> +	parent = fdt_parent_offset(blob, node);
> +	if (parent<  0) {
> +		debug("%s: Cannot find node parent\n", __func__);
> +		return -1;
> +	}
> +
> +	i2c_bus = i2c_get_bus_num_fdt(parent);
> +	if (i2c_bus<  0)
> +		return -1;
> +	nvec->i2c_bus = i2c_bus;
> +
> +	if (fdtdec_decode_gpio(blob, node, "request-gpios",
> +			&nvec->gpio)) {
> +		error("No NVEC request gpio\n");
> +		return -1;
> +	}
> +
> +	debug("NVEC: i2c:%d, gpio:%s(%u)\n",
> +	      nvec->i2c_bus, nvec->gpio.name, nvec->gpio.gpio);
> +	return 0;
> +}
> +
> +
> +int board_nvec_init(void)
> +{
> +	int res = 0;
> +	int i;
> +
> +	if (nvec_decode_config(gd->fdt_blob,&nvec_data)) {
> +		error("Can't parse NVEC node in device tree\n");
> +		return -1;
> +	}
> +
> +	debug("NVEC initialization...\n");
> +
> +	res = gpio_request(nvec_data.gpio.gpio, NULL);
> +	if (res != 0)
> +		error("NVEC: err, gpio_request\n");
> +	res = gpio_direction_output(nvec_data.gpio.gpio, 1);
> +	if (res != 0)
> +		error("NVEC: err, gpio_direction\n");
> +	res = gpio_set_value(nvec_data.gpio.gpio, 1);
> +	if (res != 0)
> +		error("NVEC: err, gpio_set_value\n");
> +	udelay(100);
> +
> +	i2c_set_bus_num(nvec_data.i2c_bus);
> +
> +	for (i = NVEC_KEYBOARD; i<  NVEC_LAST_MSG; ++i)
> +		if (devices[i].start) {
> +			debug("Starting device %d(0x%x)\n", i, i);
> +			devices[i].start();
> +		}
> +
> +	return 1;

Why "return 1"?

> +}
> +
> +
> +int nvec_read_events(void)
> +{
> +	int res;
> +	int cnt = 0;
> +
> +	while (++cnt<= 8) {
> +		res = nvec_do_io(&nvec_data);
> +		if (res)
> +			break;
> +
> +		/* TODO Process nvec communication errors */

Again a "TODO" ?

> +	}
> +
> +	return cnt;
> +}
> +
> +
> +int nvec_register_periph(int msg_type, struct nvec_periph *periph)
> +{
> +	if (devices[msg_type].start || devices[msg_type].process_msg) {
> +		error("Device for msg %d already registered\n", msg_type);
> +		return -1;
> +	}
> +
> +	devices[msg_type] = *periph;
> +	return 0;
> +}
> diff --git a/include/fdtdec.h b/include/fdtdec.h
> index 19bab79..7b84335 100644
> --- a/include/fdtdec.h
> +++ b/include/fdtdec.h
> @@ -64,6 +64,7 @@ enum fdt_compat_id {
>   	COMPAT_NVIDIA_TEGRA20_SDMMC,	/* Tegra20 SDMMC controller */
>   	COMPAT_NVIDIA_TEGRA20_SFLASH,	/* Tegra 2 SPI flash controller */
>   	COMPAT_NVIDIA_TEGRA20_SLINK,	/* Tegra 2 SPI SLINK controller */
> +	COMPAT_NVIDIA_TEGRA20_NVEC,	/* Tegra 2 EC controller */
>   	COMPAT_NVIDIA_TEGRA114_SPI,	/* Tegra 114 SPI controller */
>   	COMPAT_SMSC_LAN9215,		/* SMSC 10/100 Ethernet LAN9215 */
>   	COMPAT_SAMSUNG_EXYNOS5_SROMC,	/* Exynos5 SROMC */
> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> index 1fecab3..82df701 100644
> --- a/lib/fdtdec.c
> +++ b/lib/fdtdec.c
> @@ -37,6 +37,7 @@ static const char * const compat_names[COMPAT_COUNT] = {
>   	COMPAT(NVIDIA_TEGRA20_SDMMC, "nvidia,tegra20-sdhci"),
>   	COMPAT(NVIDIA_TEGRA20_SFLASH, "nvidia,tegra20-sflash"),
>   	COMPAT(NVIDIA_TEGRA20_SLINK, "nvidia,tegra20-slink"),
> +	COMPAT(NVIDIA_TEGRA20_NVEC, "nvidia,tegra20-nvec"),
>   	COMPAT(NVIDIA_TEGRA114_SPI, "nvidia,tegra114-spi"),
>   	COMPAT(SMSC_LAN9215, "smsc,lan9215"),
>   	COMPAT(SAMSUNG_EXYNOS5_SROMC, "samsung,exynos-sromc"),

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH v2 3/6] ARM: tegra: i2c: add nvec driver

[Andrey Danin]

On 29.04.2014 9:43, Heiko Schocher wrote:
> Hello Andrey,
>
Hello Heiko.

First of all, thank you for the review!

> Am 27.04.2014 03:14, schrieb Andrey Danin:
>> Signed-off-by: Andrey Danin<danindrey@mail.ru>
>> CC: Stephen Warren<swarren@nvidia.com>
>> CC: Marc Dietrich<marvin24@gmx.de>
>> CC: Julian Andres Klode<jak@jak-linux.org>
>> CC: ac100 at lists.launchpad.net
>> ---
>>   Changes for v2:
>>     - NVEC driver was reworked to use tegra-i2c
>>
>>   arch/arm/include/asm/arch-tegra/tegra_nvec.h |  130 ++++++++++++
>>   board/nvidia/common/board.c                  |   12 ++
>>   drivers/i2c/Makefile                         |    1 +
>>   drivers/i2c/tegra_nvec.c                     |  294
>> ++++++++++++++++++++++++++
>>   include/fdtdec.h                             |    1 +
>>   lib/fdtdec.c                                 |    1 +
>>   6 files changed, 439 insertions(+)
>>   create mode 100644 arch/arm/include/asm/arch-tegra/tegra_nvec.h
>>   create mode 100644 drivers/i2c/tegra_nvec.c
>>
>> diff --git a/arch/arm/include/asm/arch-tegra/tegra_nvec.h
>> b/arch/arm/include/asm/arch-tegra/tegra_nvec.h
>> new file mode 100644
>> index 0000000..f1cec0d
>> --- /dev/null
>> +++ b/arch/arm/include/asm/arch-tegra/tegra_nvec.h
>> @@ -0,0 +1,130 @@
>> +/*
>> + * (C) Copyright 2014
>> + * Andrey Danin<danindrey@mail.ru>
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +
>> +#ifndef _TEGRA_NVEC_H_
>> +#define _TEGRA_NVEC_H_
>> +
>> +#define I2C_CNFG            0x00
>> +#define I2C_CNFG_PACKET_MODE_EN        (1<<10)
>> +#define I2C_CNFG_NEW_MASTER_SFM        (1<<11)
>> +#define I2C_CNFG_DEBOUNCE_CNT_SHIFT    12
>> +
>> +#define I2C_SL_CNFG        0x20
>> +#define I2C_SL_NEWSL        (1<<2)
>> +#define I2C_SL_NACK        (1<<1)
>> +#define I2C_SL_RESP        (1<<0)
>> +#define I2C_SL_IRQ        (1<<3)
>> +#define END_TRANS        (1<<4)
>> +#define RCVD            (1<<2)
>> +#define RNW            (1<<1)
>> +
>> +#define I2C_SL_RCVD        0x24
>> +#define I2C_SL_STATUS        0x28
>> +#define I2C_SL_ADDR1        0x2c
>> +#define I2C_SL_ADDR2        0x30
>> +#define I2C_SL_DELAY_COUNT    0x3c
>> +
>> +
>
> please only one line, check globally.
>
I will. Thanks for pointing.

> [...]
>> diff --git a/drivers/i2c/tegra_nvec.c b/drivers/i2c/tegra_nvec.c
>> new file mode 100644
>> index 0000000..b568988
>> --- /dev/null
>> +++ b/drivers/i2c/tegra_nvec.c
>> @@ -0,0 +1,294 @@
>> +/*
>> + * (C) Copyright 2014
>> + * Andrey Danin<danindrey@mail.ru>
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +
>> +#include<common.h>
>> +#include<fdtdec.h>
>> +#include<i2c.h>
>> +#include<asm/io.h>
>> +#include<asm/gpio.h>
>> +#include<asm/arch/clock.h>
>> +#include<asm/arch/funcmux.h>
>> +#include<asm/arch-tegra/tegra_nvec.h>
>> +
>> +#ifndef CONFIG_SYS_I2C_TEGRA_NVEC
>> +#error "You should enable CONFIG_SYS_I2C_TEGRA_NVEC"
>> +#endif
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +struct nvec_periph devices[NVEC_LAST_MSG];
>> +
>> +
>> +struct nvec_t {
>> +    int            i2c_bus;
>> +    struct fdt_gpio_state    gpio;
>> +} nvec_data;
>> +
>> +
>> +/* nvec commands */
>> +char noop[] = { NVEC_CNTL, CNTL_NOOP };
>> +
>> +void nvec_signal_request(void)
>> +{
>> +    gpio_set_value(nvec_data.gpio.gpio, 0);
>> +}
>> +
>> +
>> +void nvec_clear_request(void)
>> +{
>> +    gpio_set_value(nvec_data.gpio.gpio, 1);
>> +}
>> +
>> +
>> +int nvec_msg_is_event(const unsigned char *msg)
>> +{
>> +    return msg[0]>>  7;
>> +}
>> +
>> +
>> +int nvec_msg_event_type(const unsigned char *msg)
>> +{
>> +    return msg[0]&  0x0f;
>> +}
>> +
>> +
>> +/**
>> + * Process incoming io message.
>> + * If message is keyboard event then key code will
>> + * be added to keys buffer.
>> + *
>> + * @param nvec    nvec state struct
>> + */
>> +void nvec_process_msg(struct nvec_t *nvec, struct i2c_transaction
>> *trans)
>> +{
>> +    (void) nvec;
>> +
>
> no empty line needed.
>
>> +    const unsigned char *msg = (const unsigned char *)&trans->rx_buf[1];
>> +    int event_type;
>> +
>> +    if (!nvec_msg_is_event(msg))
>> +        return;
>> +
>> +    event_type = nvec_msg_event_type(msg);
>> +
>> +    if (event_type<  NVEC_KEYBOARD || event_type>= NVEC_LAST_MSG)
>> +        return;
>> +
>> +    if (devices[event_type].process_msg)
>> +        devices[event_type].process_msg(msg);
>> +}
>> +
>> +
>> +/**
>> + * Perform complete io operation (read or write).
>> + * NOTE: function will wait NVEC_TIMEOUT_MIN (20ms)
>> + * before status check to avoid nvec hang.
>> + *
>> + * @param nvec        nvec state struct
>> + * @param wait_for_ec    if 1(NVEC_WAIT_FOR_EC) operation
>> + *            timeout is NVEC_TIMEOUT_MAX (600ms),
>> + *            otherwise function will return if io
>> + *            is not ready.
>> + *
>> + * @return nvec_io_* code
>> + */
>> +int nvec_do_io(struct nvec_t *nvec)
>> +{
>> +    static unsigned int prev_timer;
>> +    unsigned int poll_start_ms = get_timer(prev_timer);
>> +    struct i2c_transaction trans;
>> +    int res;
>> +
>> +    if (poll_start_ms>  30000) {
>> +        if (prev_timer != 0)
>> +            nvec_do_request(noop, sizeof(noop));
>> +        prev_timer = get_timer(0);
>> +    }
>> +
>> +    memset(&trans, 0, sizeof(trans));
>> +    trans.start_timeout = 1;
>> +    trans.timeout = 6000;
>> +
>> +    res = i2c_slave_io(&trans);
>> +    if (res == 0) {
>> +        nvec_process_msg(nvec,&trans);
>> +        return 0;
>> +    }
>> +
>> +    if (res != -1)
>> +        debug("Error: i2c slave io failed with code %d\n", res);
>> +
>> +    return -1;
>> +}
>> +
>> +/**
>> + * Send request and read response. If write or read failed
>> + * operation will be repeated NVEC_ATTEMPTS_MAX times.
>> + *
>> + * @param buf    request data
>> + * @param size    request data size
>> + * @return 0 if ok, -1 on error
>> + */
>> +int nvec_do_request(char *buf, int size)
>> +{
>> +    int res;
>> +    struct i2c_transaction trans;
>> +    int i;
>> +
>> +    nvec_signal_request();
>> +
>> +    /* Request */
>> +    for (i = 0; i<  10; ++i) {
>> +        memset(&trans, 0, sizeof(trans));
>> +        trans.start_timeout = 600;
>> +        trans.timeout = 6000;
>> +
>> +        trans.tx_buf[0] = (char)size;
>> +        memcpy(&trans.tx_buf[1], buf, size);
>> +        trans.tx_size = size + 1;
>> +
>> +        res = i2c_slave_io(&trans);
>> +        if (res == 0) {
>> +            if (trans.tx_pos == trans.tx_size)
>> +                break;
>> +
>> +            debug("Request was not sent completely");
>> +        } else if (res != -1) {
>> +            debug("Unknown error while slave io");
>> +        }
>> +    }
>> +    nvec_clear_request();
>> +    if (res != 0) {
>> +        error("nvec failed to perform request\n");
>> +        return -1;
>> +    }
>> +
>> +    /* Response */
>> +    for (i = 0; i<  10; ++i) {
>> +        memset(&trans, 0, sizeof(trans));
>> +        trans.start_timeout = 600;
>> +        trans.timeout = 6000;
>> +
>> +        res = i2c_slave_io(&trans);
>> +        if (res == 0)
>> +            break;
>> +    }
>> +    if (res != 0) {
>> +        error("nvec failed to read response\n");
>> +        return -1;
>> +    }
>> +
>> +    /* TODO Parse response */
>
> ?
>
> Is this not a complete driver?
>
The driver is complete. This part is not mandatory because request and 
response are processed synchronously. I will write a proper comment here.

>> +
>> +    return 0;
>> +}
>> +
>> +
>> +/**
>> + * Decode the nvec information from the fdt.
>> + *
>> + * @param blob        fdt blob
>> + * @param nvec        nvec device sturct
>> + * @return 0 if ok, -ve on error
>
> What is "-ve" ?
>
It means -error_code.
I will rework return codes, comments, style issues according to all your 
remarks.

>> + */
>> +static int nvec_decode_config(const void *blob,
>> +                  struct nvec_t *nvec)
>> +{
>> +    int node, parent;
>> +    int i2c_bus;
>> +
>> +    node = fdtdec_next_compatible(blob, 0, COMPAT_NVIDIA_TEGRA20_NVEC);
>> +    if (node<  0) {
>> +        error("Cannot find NVEC node in fdt\n");
>> +        return node;
>> +    }
>> +
>> +    parent = fdt_parent_offset(blob, node);
>> +    if (parent<  0) {
>> +        debug("%s: Cannot find node parent\n", __func__);
>> +        return -1;
>> +    }
>> +
>> +    i2c_bus = i2c_get_bus_num_fdt(parent);
>> +    if (i2c_bus<  0)
>> +        return -1;
>> +    nvec->i2c_bus = i2c_bus;
>> +
>> +    if (fdtdec_decode_gpio(blob, node, "request-gpios",
>> +            &nvec->gpio)) {
>> +        error("No NVEC request gpio\n");
>> +        return -1;
>> +    }
>> +
>> +    debug("NVEC: i2c:%d, gpio:%s(%u)\n",
>> +          nvec->i2c_bus, nvec->gpio.name, nvec->gpio.gpio);
>> +    return 0;
>> +}
>> +
>> +
>> +int board_nvec_init(void)
>> +{
>> +    int res = 0;
>> +    int i;
>> +
>> +    if (nvec_decode_config(gd->fdt_blob,&nvec_data)) {
>> +        error("Can't parse NVEC node in device tree\n");
>> +        return -1;
>> +    }
>> +
>> +    debug("NVEC initialization...\n");
>> +
>> +    res = gpio_request(nvec_data.gpio.gpio, NULL);
>> +    if (res != 0)
>> +        error("NVEC: err, gpio_request\n");
>> +    res = gpio_direction_output(nvec_data.gpio.gpio, 1);
>> +    if (res != 0)
>> +        error("NVEC: err, gpio_direction\n");
>> +    res = gpio_set_value(nvec_data.gpio.gpio, 1);
>> +    if (res != 0)
>> +        error("NVEC: err, gpio_set_value\n");
>> +    udelay(100);
>> +
>> +    i2c_set_bus_num(nvec_data.i2c_bus);
>> +
>> +    for (i = NVEC_KEYBOARD; i<  NVEC_LAST_MSG; ++i)
>> +        if (devices[i].start) {
>> +            debug("Starting device %d(0x%x)\n", i, i);
>> +            devices[i].start();
>> +        }
>> +
>> +    return 1;
>
> Why "return 1"?
>
>> +}
>> +
>> +
>> +int nvec_read_events(void)
>> +{
>> +    int res;
>> +    int cnt = 0;
>> +
>> +    while (++cnt<= 8) {
>> +        res = nvec_do_io(&nvec_data);
>> +        if (res)
>> +            break;
>> +
>> +        /* TODO Process nvec communication errors */
>
> Again a "TODO" ?
>
I will implement.

>> +    }
>> +
>> +    return cnt;
>> +}
>> +
>> +
>> +int nvec_register_periph(int msg_type, struct nvec_periph *periph)
>> +{
>> +    if (devices[msg_type].start || devices[msg_type].process_msg) {
>> +        error("Device for msg %d already registered\n", msg_type);
>> +        return -1;
>> +    }
>> +
>> +    devices[msg_type] = *periph;
>> +    return 0;
>> +}
>> diff --git a/include/fdtdec.h b/include/fdtdec.h
>> index 19bab79..7b84335 100644
>> --- a/include/fdtdec.h
>> +++ b/include/fdtdec.h
>> @@ -64,6 +64,7 @@ enum fdt_compat_id {
>>       COMPAT_NVIDIA_TEGRA20_SDMMC,    /* Tegra20 SDMMC controller */
>>       COMPAT_NVIDIA_TEGRA20_SFLASH,    /* Tegra 2 SPI flash controller */
>>       COMPAT_NVIDIA_TEGRA20_SLINK,    /* Tegra 2 SPI SLINK controller */
>> +    COMPAT_NVIDIA_TEGRA20_NVEC,    /* Tegra 2 EC controller */
>>       COMPAT_NVIDIA_TEGRA114_SPI,    /* Tegra 114 SPI controller */
>>       COMPAT_SMSC_LAN9215,        /* SMSC 10/100 Ethernet LAN9215 */
>>       COMPAT_SAMSUNG_EXYNOS5_SROMC,    /* Exynos5 SROMC */
>> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
>> index 1fecab3..82df701 100644
>> --- a/lib/fdtdec.c
>> +++ b/lib/fdtdec.c
>> @@ -37,6 +37,7 @@ static const char * const compat_names[COMPAT_COUNT]
>> = {
>>       COMPAT(NVIDIA_TEGRA20_SDMMC, "nvidia,tegra20-sdhci"),
>>       COMPAT(NVIDIA_TEGRA20_SFLASH, "nvidia,tegra20-sflash"),
>>       COMPAT(NVIDIA_TEGRA20_SLINK, "nvidia,tegra20-slink"),
>> +    COMPAT(NVIDIA_TEGRA20_NVEC, "nvidia,tegra20-nvec"),
>>       COMPAT(NVIDIA_TEGRA114_SPI, "nvidia,tegra114-spi"),
>>       COMPAT(SMSC_LAN9215, "smsc,lan9215"),
>>       COMPAT(SAMSUNG_EXYNOS5_SROMC, "samsung,exynos-sromc"),
>
> bye,
> Heiko

Thanks.

[U-Boot] [PATCH v2 3/6] ARM: tegra: i2c: add nvec driver

[Heiko Schocher]

Hello Andrey,

Am 29.04.2014 09:15, schrieb Andrey Danin:
> On 29.04.2014 9:43, Heiko Schocher wrote:
>> Hello Andrey,
>>
> Hello Heiko.
>
> First of all, thank you for the review!

You are welcome.

>> Am 27.04.2014 03:14, schrieb Andrey Danin:
>>> Signed-off-by: Andrey Danin<danindrey@mail.ru>
>>> CC: Stephen Warren<swarren@nvidia.com>
>>> CC: Marc Dietrich<marvin24@gmx.de>
>>> CC: Julian Andres Klode<jak@jak-linux.org>
>>> CC: ac100 at lists.launchpad.net
>>> ---
>>> Changes for v2:
>>> - NVEC driver was reworked to use tegra-i2c
>>>
>>> arch/arm/include/asm/arch-tegra/tegra_nvec.h | 130 ++++++++++++
>>> board/nvidia/common/board.c | 12 ++
>>> drivers/i2c/Makefile | 1 +
>>> drivers/i2c/tegra_nvec.c | 294
>>> ++++++++++++++++++++++++++
>>> include/fdtdec.h | 1 +
>>> lib/fdtdec.c | 1 +
>>> 6 files changed, 439 insertions(+)
>>> create mode 100644 arch/arm/include/asm/arch-tegra/tegra_nvec.h
>>> create mode 100644 drivers/i2c/tegra_nvec.c
[...]
>>> diff --git a/drivers/i2c/tegra_nvec.c b/drivers/i2c/tegra_nvec.c
>>> new file mode 100644
>>> index 0000000..b568988
>>> --- /dev/null
>>> +++ b/drivers/i2c/tegra_nvec.c
>>> @@ -0,0 +1,294 @@
[...]
>>> + /* TODO Parse response */
>>
>> ?
>>
>> Is this not a complete driver?
>>
> The driver is complete. This part is not mandatory because request and response are processed synchronously. I will write a proper comment here.

Ok, thanks!

>>> +
>>> + return 0;
>>> +}
>>> +
>>> +
>>> +/**
>>> + * Decode the nvec information from the fdt.
>>> + *
>>> + * @param blob fdt blob
>>> + * @param nvec nvec device sturct
>>> + * @return 0 if ok, -ve on error
>>
>> What is "-ve" ?
>>
> It means -error_code.
> I will rework return codes, comments, style issues according to all your remarks.

Thanks!

[...]
>>> +int nvec_read_events(void)
>>> +{
>>> + int res;
>>> + int cnt = 0;
>>> +
>>> + while (++cnt<= 8) {
>>> + res = nvec_do_io(&nvec_data);
>>> + if (res)
>>> + break;
>>> +
>>> + /* TODO Process nvec communication errors */
>>
>> Again a "TODO" ?
>>
> I will implement.

Sounds great, thanks!

[...]

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH v2 4/6] ARM: tegra: nvec: add keyboard support

[Andrey Danin]

Signed-off-by: Andrey Danin <danindrey@mail.ru>
CC: Stephen Warren <swarren@nvidia.com>
CC: Marc Dietrich <marvin24@gmx.de>
CC: Julian Andres Klode <jak@jak-linux.org>
CC: ac100 at lists.launchpad.net
---
 Changes for v2:
	- fixed incorrect keys handling in nvec-keyboard driver

 .../include/asm/arch-tegra/tegra_nvec_keyboard.h   |  304 ++++++++++++++++++++
 drivers/input/Makefile                             |    3 +
 drivers/input/tegra-nvec-kbc.c                     |  215 ++++++++++++++
 include/configs/tegra-common-post.h                |    2 +
 4 files changed, 524 insertions(+)
 create mode 100644 arch/arm/include/asm/arch-tegra/tegra_nvec_keyboard.h
 create mode 100644 drivers/input/tegra-nvec-kbc.c

diff --git a/arch/arm/include/asm/arch-tegra/tegra_nvec_keyboard.h b/arch/arm/include/asm/arch-tegra/tegra_nvec_keyboard.h
new file mode 100644
index 0000000..5f789f4
--- /dev/null
+++ b/arch/arm/include/asm/arch-tegra/tegra_nvec_keyboard.h
@@ -0,0 +1,304 @@
+/*
+ * (C) Copyright 2014
+ * Andrey Danin <danindrey@mail.ru>
+ * (C) Copyright 2009
+ * NVIDIA Corporation.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef _TEGRA_NVEC_KEYBOARD_H_
+#define _TEGRA_NVEC_KEYBOARD_H_
+
+#include <linux/input.h>
+
+
+/* Keytables */
+
+static unsigned short code_tab_102us[] = {
+	/* 0x00 */
+	KEY_GRAVE,
+	KEY_ESC,
+	KEY_1,
+	KEY_2,
+	KEY_3,
+	KEY_4,
+	KEY_5,
+	KEY_6,
+	KEY_7,
+	KEY_8,
+	KEY_9,
+	KEY_0,
+	KEY_MINUS,
+	KEY_EQUAL,
+	KEY_BACKSPACE,
+	KEY_TAB,
+	/* 0x10 */
+	KEY_Q,
+	KEY_W,
+	KEY_E,
+	KEY_R,
+	KEY_T,
+	KEY_Y,
+	KEY_U,
+	KEY_I,
+	KEY_O,
+	KEY_P,
+	KEY_LEFTBRACE,
+	KEY_RIGHTBRACE,
+	KEY_ENTER,
+	KEY_LEFTCTRL,
+	KEY_A,
+	KEY_S,
+	/* 0x20 */
+	KEY_D,
+	KEY_F,
+	KEY_G,
+	KEY_H,
+	KEY_J,
+	KEY_K,
+	KEY_L,
+	KEY_SEMICOLON,
+	KEY_APOSTROPHE,
+	KEY_GRAVE,
+	KEY_LEFTSHIFT,
+	KEY_BACKSLASH,
+	KEY_Z,
+	KEY_X,
+	KEY_C,
+	KEY_V,
+	/* 0x30 */
+	KEY_B,
+	KEY_N,
+	KEY_M,
+	KEY_COMMA,
+	KEY_DOT,
+	KEY_SLASH,
+	KEY_RIGHTSHIFT,
+	KEY_KPASTERISK,
+	KEY_LEFTALT,
+	KEY_SPACE,
+	KEY_CAPSLOCK,
+	KEY_F1,
+	KEY_F2,
+	KEY_F3,
+	KEY_F4,
+	KEY_F5,
+	/* 0x40 */
+	KEY_F6,
+	KEY_F7,
+	KEY_F8,
+	KEY_F9,
+	KEY_F10,
+	KEY_FN,
+	/* VK_SCROLL */
+	0,
+	KEY_KP7,
+	KEY_KP8,
+	KEY_KP9,
+	KEY_KPMINUS,
+	KEY_KP4,
+	KEY_KP5,
+	KEY_KP6,
+	KEY_KPPLUS,
+	KEY_KP1,
+	/* 0x50 */
+	KEY_KP2,
+	KEY_KP3,
+	KEY_KP0,
+	KEY_KPDOT,
+	/* VK_SNAPSHOT */
+	0, /* KEY_MENU, */
+	KEY_POWER,
+	/* VK_OEM_102 */
+	KEY_102ND,
+	KEY_F11,
+	KEY_F12,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	/* 0x60 */
+	0,
+	0,
+	0,
+	0, /* KEY_SEARCH, */
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	/* 0x70 */
+	0,
+	0,
+	0,
+	KEY_KP5,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	KEY_KP9,
+};
+
+static unsigned short extcode_tab_us102[] = {
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	/* 0x10 */
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	/* VK_MEDIA_NEXT_TRACK */
+	0,
+	0,
+	0,
+	/* VK_RETURN */
+	0,
+	KEY_RIGHTCTRL,
+	0,
+	0,
+	/* 0x20 */
+	KEY_MUTE,
+	/* VK_LAUNCH_APP1 */
+	0,
+	/* VK_MEDIA_PLAY_PAUSE */
+	0,
+	0,
+	/* VK_MEDIA_STOP */
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	/* 0x30 */
+	KEY_VOLUMEUP,
+	0,
+	/* VK_BROWSER_HOME */
+	0,
+	0,
+	0,
+	/* VK_DIVIDE */
+	KEY_KPSLASH,
+	0,
+	/* VK_SNAPSHOT */
+	KEY_SYSRQ,
+	/* VK_RMENU */
+	KEY_RIGHTALT,
+	/* VK_OEM_NV_BACKLIGHT_UP */
+	0,
+	/* VK_OEM_NV_BACKLIGHT_DN */
+	0,
+	/* VK_OEM_NV_BACKLIGHT_AUTOTOGGLE */
+	0,
+	/* VK_OEM_NV_POWER_INFO */
+	0,
+	/* VK_OEM_NV_WIFI_TOGGLE */
+	0,
+	/* VK_OEM_NV_DISPLAY_SELECT */
+	0,
+	/* VK_OEM_NV_AIRPLANE_TOGGLE */
+	0,
+	/* 0x40 */
+	0,
+	KEY_LEFT,
+	0,
+	0,
+	0,
+	0,
+	0, /* KEY_CANCEL, */
+	KEY_HOME,
+	KEY_UP,
+	KEY_PAGEUP,
+	0,
+	KEY_LEFT,
+	0,
+	KEY_RIGHT,
+	0,
+	KEY_END,
+	/* 0x50 */
+	KEY_DOWN,
+	KEY_PAGEDOWN,
+	KEY_INSERT,
+	KEY_DELETE,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	KEY_LEFTMETA,
+	0,
+	KEY_ESC,
+	KEY_KPMINUS,
+	0,
+	0,
+	0,
+	0,
+	0,
+	0,
+	/* VK_BROWSER_SEARCH */
+	0,
+	/* VK_BROWSER_FAVORITES */
+	0,
+	/* VK_BROWSER_REFRESH */
+	0,
+	/* VK_BROWSER_STOP */
+	0,
+	/* VK_BROWSER_FORWARD */
+	0,
+	/* VK_BROWSER_BACK */
+	0,
+	/* VK_LAUNCH_APP2 */
+	0,
+	/* VK_LAUNCH_MAIL */
+	0,
+	/* VK_LAUNCH_MEDIA_SELECT */
+	0,
+};
+
+static unsigned short *code_tabs[] = { code_tab_102us, extcode_tab_us102 };
+
+
+#endif /* _TEGRA_NVEC_KEYBOARD_H_ */
diff --git a/drivers/input/Makefile b/drivers/input/Makefile
index a8e9be2..38c576f 100644
--- a/drivers/input/Makefile
+++ b/drivers/input/Makefile
@@ -7,6 +7,9 @@
 
 obj-$(CONFIG_I8042_KBD) += i8042.o
 obj-$(CONFIG_TEGRA_KEYBOARD) += tegra-kbc.o
+ifdef CONFIG_SYS_I2C_TEGRA_NVEC
+obj-$(CONFIG_TEGRA_NVEC_KEYBOARD) += tegra-nvec-kbc.o
+endif
 obj-$(CONFIG_CROS_EC_KEYB) += cros_ec_keyb.o
 ifdef CONFIG_PS2KBD
 obj-y += keyboard.o pc_keyb.o
diff --git a/drivers/input/tegra-nvec-kbc.c b/drivers/input/tegra-nvec-kbc.c
new file mode 100644
index 0000000..f5832b1
--- /dev/null
+++ b/drivers/input/tegra-nvec-kbc.c
@@ -0,0 +1,215 @@
+/*
+ *  (C) Copyright 2014
+ *  Andrey Danin <danindrey@mail.ru>
+ *  (C) Copyright 2011
+ *  NVIDIA Corporation <www.nvidia.com>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <input.h>
+#include <asm/arch-tegra/tegra_nvec.h>
+#include <asm/arch-tegra/tegra_nvec_keyboard.h>
+
+
+enum {
+	KBC_MAX_KPENT = 8,
+	KBC_REPEAT_RATE_MS	= 30,
+	KBC_REPEAT_DELAY_MS	= 240,
+};
+
+/* keyboard config/state */
+static struct keyb {
+	int registered;
+	struct input_config input;	/* The input layer */
+	int pressed_keys[KBC_MAX_KPENT];
+	int pressed_keys_cnt;
+} config;
+
+
+/* 0 - pressed, other - released */
+char keys[256];
+
+
+/* nvec commands */
+static char enable_kbd[] = { NVEC_KBD, ENABLE_KBD };
+static char reset_kbd[] = { NVEC_PS2, MOUSE_SEND_CMD, MOUSE_RESET, 3 };
+static char clear_leds[] = { NVEC_KBD, SET_LEDS, 0 };
+
+
+/**
+ * Check the tegra nvec keyboard, and send any keys that are pressed.
+ *
+ * This is called by input_tstc() and input_getc() when they need more
+ * characters
+ *
+ * @param input		Input configuration
+ * @return 1, to indicate that we have something to look at
+ */
+int tegra_nvec_kbc_check(struct input_config *input)
+{
+	int i;
+
+	nvec_read_events();
+
+	config.pressed_keys_cnt = 0;
+	/* TODO Optimization required */
+	for (i = 0; i < sizeof(keys); ++i) {
+		if (keys[i] == 0 && config.pressed_keys_cnt < KBC_MAX_KPENT) {
+			config.pressed_keys[config.pressed_keys_cnt] = i;
+			++config.pressed_keys_cnt;
+		}
+	}
+
+	input_send_keycodes(input,
+			    config.pressed_keys,
+			    config.pressed_keys_cnt);
+
+	return config.pressed_keys_cnt;
+}
+
+
+/**
+ * Test if keys are available to be read
+ *
+ * @return 0 if no keys available, 1 if keys are available
+ */
+static int kbd_tstc(void)
+{
+	/* Just get input to do this for us */
+	return input_tstc(&config.input);
+}
+
+
+/**
+ * Read a key
+ *
+ * TODO: U-Boot wants 0 for no key, but Ctrl-@ is a valid key...
+ *
+ * @return ASCII key code, or 0 if no key, or -1 if error
+ */
+static int kbd_getc(void)
+{
+	/* Just get input to do this for us */
+	return input_getc(&config.input);
+}
+
+
+int tegra_nvec_process_keyboard_msg(const unsigned char *msg)
+{
+	int code, state;
+	int event_type;
+	int _size;
+	int key;
+
+
+	event_type = nvec_msg_event_type(msg);
+	if (event_type != NVEC_KEYBOARD)
+		return -1;
+
+	_size = (msg[0] & (3 << 5)) >> 5;
+
+	if (_size == NVEC_VAR_SIZE) {
+		debug("Skip unsupported msg (size: %d)\n", _size);
+		return -1;
+	}
+
+	if (_size == NVEC_3BYTES)
+		msg++;
+
+	code = msg[1] & 0x7f;
+	state = msg[1] & 0x80;
+
+	key = code_tabs[_size][code];
+	keys[key] = state;
+
+	return 0;
+}
+
+
+int tegra_nvec_enable_kbd_events(void)
+{
+	if (nvec_do_request(reset_kbd, 4)) {
+		error("NVEC: failed to reset keyboard\n");
+		return -1;
+	}
+	if (nvec_do_request(clear_leds, 3)) {
+		error("NVEC: failed to clear leds\n");
+		return -1;
+	}
+	if (nvec_do_request(enable_kbd, 2)) {
+		error("NVEC: failed to enable keyboard\n");
+		return -1;
+	}
+
+	debug("NVEC: keyboard initialization finished\n");
+
+	return 0;
+}
+
+
+static int tegra_nvec_kbc_start(void)
+{
+	if (config.registered)
+		return 0;
+
+	struct nvec_periph nvec_keyboard;
+	memset(&nvec_keyboard, 0, sizeof(nvec_keyboard));
+	nvec_keyboard.start = tegra_nvec_enable_kbd_events;
+	nvec_keyboard.process_msg = tegra_nvec_process_keyboard_msg;
+
+	if (nvec_register_periph(NVEC_KEYBOARD, &nvec_keyboard)) {
+		error("NVEC: failed to register keyboard perephirial device");
+		return -1;
+	}
+
+	config.registered = 1;
+
+	return 0;
+}
+
+
+int drv_keyboard_init(void)
+{
+	struct stdio_dev dev;
+	int error;
+#ifdef CONFIG_CONSOLE_MUX
+	char *stdinname = getenv("stdin");
+#endif
+
+	memset(keys, 1, sizeof(keys));
+
+	config.registered = 0;
+
+	if (input_init(&config.input, 0)) {
+		printf("nvec kbc: cannot set up input\n");
+		return -1;
+	}
+	input_set_delays(&config.input, KBC_REPEAT_DELAY_MS,
+			 KBC_REPEAT_RATE_MS);
+	config.input.read_keys = tegra_nvec_kbc_check;
+
+	memset(&dev, '\0', sizeof(dev));
+	strcpy(dev.name, "tegra-nvec-kbc");
+	dev.flags = DEV_FLAGS_INPUT | DEV_FLAGS_SYSTEM;
+	dev.getc = kbd_getc;
+	dev.tstc = kbd_tstc;
+	dev.start = tegra_nvec_kbc_start;
+
+	/* Register the device. tegra_nvec_kbc_start() will be called soon */
+	error = input_stdio_register(&dev);
+	if (error) {
+		printf("nvec kbc: failed to register stdio device, %d\n",
+		       error);
+		return error;
+	}
+#ifdef CONFIG_CONSOLE_MUX
+	error = iomux_doenv(stdin, stdinname);
+	if (error) {
+		printf("nvec kbc: iomux_doenv failed, %d\n", error);
+		return error;
+	}
+#endif
+	return 0;
+}
diff --git a/include/configs/tegra-common-post.h b/include/configs/tegra-common-post.h
index e1a3bbc..efc0ef9 100644
--- a/include/configs/tegra-common-post.h
+++ b/include/configs/tegra-common-post.h
@@ -106,6 +106,8 @@
 
 #ifdef CONFIG_TEGRA_KEYBOARD
 #define STDIN_KBD_KBC ",tegra-kbc"
+#elif defined(CONFIG_TEGRA_NVEC_KEYBOARD)
+#define STDIN_KBD_KBC ",tegra-nvec-kbc"
 #else
 #define STDIN_KBD_KBC ""
 #endif
-- 
1.7.9.5

[U-Boot] [Ac100] [PATCH v2 4/6] ARM: tegra: nvec: add keyboard support

[Stephen Warren]

On 04/26/2014 07:14 PM, Andrey Danin wrote:
> Signed-off-by: Andrey Danin <danindrey@mail.ru>
> CC: Stephen Warren <swarren@nvidia.com>
> CC: Marc Dietrich <marvin24@gmx.de>
> CC: Julian Andres Klode <jak@jak-linux.org>
> CC: ac100 at lists.launchpad.net
> ---
>  Changes for v2:
> 	- fixed incorrect keys handling in nvec-keyboard driver

No patch description?

Something isn't quite right with the device registration here. If U-Boot
starts with stdin=serial, and then I "setenv stdin
serial,tegra-nvec-kbc", then I can't use the keyboard. However, if I
save the environment and reset, then nvec input works. Similarly, it
gets confused if I remove and re-add tegra-nvec-kbc to $stdin.

I've also observed quite a stuck keys, hangs, and failed requests:
ERROR: nvec failed to perform request
at .../drivers/i2c/tegra_nvec.c:166/nvec_do_request()

But when it works, it's nice:-)

[PATCH v2 5/6] ARM: tegra: paz00: add dt bindings for nvec

[Andrey Danin]

Signed-off-by: Andrey Danin <danindrey-JGs/UdohzUI@public.gmane.org>
CC: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
CC: Marc Dietrich <marvin24-Mmb7MZpHnFY@public.gmane.org>
CC: Julian Andres Klode <jak-4HMq4SXA452hPH1hqNUYSQ@public.gmane.org>
CC: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
CC: ac100-oU9gvf+ajcQ97yFScArB1dHuzzzSOjJt@public.gmane.org
---
 Changes for v2:
 	- Separated from enabling keyboard patch
	- Changed NVEC dt bindings 

 arch/arm/dts/tegra20-paz00.dts |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm/dts/tegra20-paz00.dts b/arch/arm/dts/tegra20-paz00.dts
index 780203c..9906f3a 100644
--- a/arch/arm/dts/tegra20-paz00.dts
+++ b/arch/arm/dts/tegra20-paz00.dts
@@ -40,7 +40,13 @@
 	};
 
 	i2c@7000c500 {
-		status = "disabled";
+		status = "okay";
+		clock-frequency = <40000>;
+		slave-addr = <138>;
+		nvec {
+			compatible = "nvidia,tegra20-nvec";
+			request-gpios = <&gpio 170 0>; /* gpio PV2 */
+		};
 	};
 
 	i2c@7000d000 {
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[U-Boot] [PATCH v2 5/6] ARM: tegra: paz00: add dt bindings for nvec

[Andrey Danin]

Signed-off-by: Andrey Danin <danindrey@mail.ru>
CC: Stephen Warren <swarren@nvidia.com>
CC: Marc Dietrich <marvin24@gmx.de>
CC: Julian Andres Klode <jak@jak-linux.org>
CC: devicetree at vger.kernel.org
CC: ac100 at lists.launchpad.net
---
 Changes for v2:
 	- Separated from enabling keyboard patch
	- Changed NVEC dt bindings 

 arch/arm/dts/tegra20-paz00.dts |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm/dts/tegra20-paz00.dts b/arch/arm/dts/tegra20-paz00.dts
index 780203c..9906f3a 100644
--- a/arch/arm/dts/tegra20-paz00.dts
+++ b/arch/arm/dts/tegra20-paz00.dts
@@ -40,7 +40,13 @@
 	};
 
 	i2c at 7000c500 {
-		status = "disabled";
+		status = "okay";
+		clock-frequency = <40000>;
+		slave-addr = <138>;
+		nvec {
+			compatible = "nvidia,tegra20-nvec";
+			request-gpios = <&gpio 170 0>; /* gpio PV2 */
+		};
 	};
 
 	i2c at 7000d000 {
-- 
1.7.9.5

Re: [Ac100] [PATCH v2 5/6] ARM: tegra: paz00: add dt bindings for nvec

[Stephen Warren]

On 04/26/2014 07:14 PM, Andrey Danin wrote:

This patch isn't adding DT bindings for NVEC, but rather add DT nodes.
The binding is the schema, not the content.

We need a DT binding document that's been reviewed by the DT binding
maintainers. Can you please first submit a patch to the Linux kernel
that modifies the existing I2C core and Tegra I2C controller binding
documentation to add slave mode support. Once that's fully reviewed and
ack'd, this patch series can implement support for it in U-Boot.

> diff --git a/arch/arm/dts/tegra20-paz00.dts b/arch/arm/dts/tegra20-paz00.dts

>  	i2c@7000c500 {
> -		status = "disabled";
> +		status = "okay";
> +		clock-frequency = <40000>;
> +		slave-addr = <138>;
> +		nvec {
> +			compatible = "nvidia,tegra20-nvec";
> +			request-gpios = <&gpio 170 0>; /* gpio PV2 */

The reg property is missing here. Since the i2c node has
#address-cells/#size-cells, there must be a reg property in the children.

There's nothing here to indicate that this node is a slave device rather
than a master device, and doesn't seem to be any allowance for a single
I2C controller to support both master and slave nodes at the same time
(which I think Tegra's controller can IIRC).

IIRC, I had previously suggested something like encoding master/slave
into the reg property of the I2C child nodes. We could either do:

a) Set some top-bit to indicate a slave device.

b) If #address-cells=<1>, only master devices are present. If
#address-cells=<2>, either master or slave devices could be present.
Cell 0 could be 0==master, 1==slave, and cell 1 the actual I2C bus address.

Either of those approaches would allow representing an I2C controller
that supported multiple slave addresses. Even though I think Tegra's
slave controller doesn't support that, I still think we should use a
generic binding so that I2C slave mode looks the same everywhere.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[U-Boot] [Ac100] [PATCH v2 5/6] ARM: tegra: paz00: add dt bindings for nvec

[Stephen Warren]

On 04/26/2014 07:14 PM, Andrey Danin wrote:

This patch isn't adding DT bindings for NVEC, but rather add DT nodes.
The binding is the schema, not the content.

We need a DT binding document that's been reviewed by the DT binding
maintainers. Can you please first submit a patch to the Linux kernel
that modifies the existing I2C core and Tegra I2C controller binding
documentation to add slave mode support. Once that's fully reviewed and
ack'd, this patch series can implement support for it in U-Boot.

> diff --git a/arch/arm/dts/tegra20-paz00.dts b/arch/arm/dts/tegra20-paz00.dts

>  	i2c at 7000c500 {
> -		status = "disabled";
> +		status = "okay";
> +		clock-frequency = <40000>;
> +		slave-addr = <138>;
> +		nvec {
> +			compatible = "nvidia,tegra20-nvec";
> +			request-gpios = <&gpio 170 0>; /* gpio PV2 */

The reg property is missing here. Since the i2c node has
#address-cells/#size-cells, there must be a reg property in the children.

There's nothing here to indicate that this node is a slave device rather
than a master device, and doesn't seem to be any allowance for a single
I2C controller to support both master and slave nodes at the same time
(which I think Tegra's controller can IIRC).

IIRC, I had previously suggested something like encoding master/slave
into the reg property of the I2C child nodes. We could either do:

a) Set some top-bit to indicate a slave device.

b) If #address-cells=<1>, only master devices are present. If
#address-cells=<2>, either master or slave devices could be present.
Cell 0 could be 0==master, 1==slave, and cell 1 the actual I2C bus address.

Either of those approaches would allow representing an I2C controller
that supported multiple slave addresses. Even though I think Tegra's
slave controller doesn't support that, I still think we should use a
generic binding so that I2C slave mode looks the same everywhere.

Re: [U-Boot] [Ac100] [PATCH v2 5/6] ARM: tegra: paz00: add dtbindings for nvec

[Marc Dietrich]

[-- Attachment #1: Type: text/plain, Size: 3910 bytes --]

Hi,

Am Montag, 28. April 2014, 17:04:13 schrieb Stephen Warren:
> On 04/26/2014 07:14 PM, Andrey Danin wrote:
> 
> This patch isn't adding DT bindings for NVEC, but rather add DT nodes.
> The binding is the schema, not the content.
>
> We need a DT binding document that's been reviewed by the DT binding
> maintainers. Can you please first submit a patch to the Linux kernel
> that modifies the existing I2C core and Tegra I2C controller binding
> documentation to add slave mode support. Once that's fully reviewed and
> ack'd, this patch series can implement support for it in U-Boot.

I'm sorry that I didn't came up with a proper kernel implementation, but while 
we are discussion the binding I just want to give some coins.

> > diff --git a/arch/arm/dts/tegra20-paz00.dts
> > b/arch/arm/dts/tegra20-paz00.dts> 
> >  	i2c@7000c500 {
> > 
> > -		status = "disabled";
> > +		status = "okay";
> > +		clock-frequency = <40000>;
> > +		slave-addr = <138>;
> > +		nvec {
> > +			compatible = "nvidia,tegra20-nvec";
> > +			request-gpios = <&gpio 170 0>; /* gpio PV2 */
> 
> The reg property is missing here. Since the i2c node has
> #address-cells/#size-cells, there must be a reg property in the children.
> 
> There's nothing here to indicate that this node is a slave device rather
> than a master device, and doesn't seem to be any allowance for a single
> I2C controller to support both master and slave nodes at the same time
> (which I think Tegra's controller can IIRC).
> 
> IIRC, I had previously suggested something like encoding master/slave
> into the reg property of the I2C child nodes. We could either do:
> 
> a) Set some top-bit to indicate a slave device.
> 
> b) If #address-cells=<1>, only master devices are present. If
> #address-cells=<2>, either master or slave devices could be present.
> Cell 0 could be 0==master, 1==slave, and cell 1 the actual I2C bus address.

I'm not sure if this is really needed. NVEC knows it has to configure the 
tegra controller as slave. I don't see a reason to double this fact in the 
device tree. I like the idea of how the downstream kernel does it. The driver 
calls something like tegra_i2c_init_slave with the slave address as an 
parameter. This means that the slave address is not a property of the i2c 
(slave-) controller, but of the master because it has the address hard coded 
in its firmware. So reg = <138>; would be sufficient here and it also enables 
multi-slave configs.

i2c-tegra must take care that only one transaction (slave or master) is 
running at a time. The client (e.g. nvec) is free to drop the excluse usage by 
calling tegra_i2c_stop_slave so master mode can be used again or another 
client (master device) can be started where the slave controller gets a 
different address.

Multi-master would require that only one master can hold the lock enabling 
slave mode, but that's all software stuff and not related to the binding.

Also the kernel binding would require that nvec node itself has subdevices for 
e.g. keyboard, mouse, power, ... which are connected to the internal ec bus. 
We can use the nvec protocol identifiers to assign an address here.

Ok, lets take a look at the binding now:

i2c@7000c500 {
		status = "okay";
		clock-frequency = <40000>;
		nvec@138 {
			compatible = "nvidia,nvec, simple-bus";
			#address-cells = <1>
			#size-cells = <0>;
			reg = <138>;
			request-gpios = <&gpio 170 0>; /* gpio PV2 */

			keyboard@5 {
				compatible = "nvidia,nvec-kbd";						
				reg = <5>;
			};
			mouse@6 {
				compatible = "nvidia,nvec-aux";
				reg = <6>;
				packet-size = <6>;
			};
		};
};

Only thing I'm not sure is where to put the clock-frequency because it really 
depends on the i2c clients and if we have multiple masters, we could use 
multiple frequencies. Or maybe the lowest supported must be used instead, in 
which case the clock-frequency is better placed in the controller node.

Marc

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[U-Boot] [Ac100] [PATCH v2 5/6] ARM: tegra: paz00: add dtbindings for nvec

[Marc Dietrich]

Hi,

Am Montag, 28. April 2014, 17:04:13 schrieb Stephen Warren:
> On 04/26/2014 07:14 PM, Andrey Danin wrote:
> 
> This patch isn't adding DT bindings for NVEC, but rather add DT nodes.
> The binding is the schema, not the content.
>
> We need a DT binding document that's been reviewed by the DT binding
> maintainers. Can you please first submit a patch to the Linux kernel
> that modifies the existing I2C core and Tegra I2C controller binding
> documentation to add slave mode support. Once that's fully reviewed and
> ack'd, this patch series can implement support for it in U-Boot.

I'm sorry that I didn't came up with a proper kernel implementation, but while 
we are discussion the binding I just want to give some coins.

> > diff --git a/arch/arm/dts/tegra20-paz00.dts
> > b/arch/arm/dts/tegra20-paz00.dts> 
> >  	i2c at 7000c500 {
> > 
> > -		status = "disabled";
> > +		status = "okay";
> > +		clock-frequency = <40000>;
> > +		slave-addr = <138>;
> > +		nvec {
> > +			compatible = "nvidia,tegra20-nvec";
> > +			request-gpios = <&gpio 170 0>; /* gpio PV2 */
> 
> The reg property is missing here. Since the i2c node has
> #address-cells/#size-cells, there must be a reg property in the children.
> 
> There's nothing here to indicate that this node is a slave device rather
> than a master device, and doesn't seem to be any allowance for a single
> I2C controller to support both master and slave nodes at the same time
> (which I think Tegra's controller can IIRC).
> 
> IIRC, I had previously suggested something like encoding master/slave
> into the reg property of the I2C child nodes. We could either do:
> 
> a) Set some top-bit to indicate a slave device.
> 
> b) If #address-cells=<1>, only master devices are present. If
> #address-cells=<2>, either master or slave devices could be present.
> Cell 0 could be 0==master, 1==slave, and cell 1 the actual I2C bus address.

I'm not sure if this is really needed. NVEC knows it has to configure the 
tegra controller as slave. I don't see a reason to double this fact in the 
device tree. I like the idea of how the downstream kernel does it. The driver 
calls something like tegra_i2c_init_slave with the slave address as an 
parameter. This means that the slave address is not a property of the i2c 
(slave-) controller, but of the master because it has the address hard coded 
in its firmware. So reg = <138>; would be sufficient here and it also enables 
multi-slave configs.

i2c-tegra must take care that only one transaction (slave or master) is 
running at a time. The client (e.g. nvec) is free to drop the excluse usage by 
calling tegra_i2c_stop_slave so master mode can be used again or another 
client (master device) can be started where the slave controller gets a 
different address.

Multi-master would require that only one master can hold the lock enabling 
slave mode, but that's all software stuff and not related to the binding.

Also the kernel binding would require that nvec node itself has subdevices for 
e.g. keyboard, mouse, power, ... which are connected to the internal ec bus. 
We can use the nvec protocol identifiers to assign an address here.

Ok, lets take a look at the binding now:

i2c at 7000c500 {
		status = "okay";
		clock-frequency = <40000>;
		nvec at 138 {
			compatible = "nvidia,nvec, simple-bus";
			#address-cells = <1>
			#size-cells = <0>;
			reg = <138>;
			request-gpios = <&gpio 170 0>; /* gpio PV2 */

			keyboard at 5 {
				compatible = "nvidia,nvec-kbd";						
				reg = <5>;
			};
			mouse at 6 {
				compatible = "nvidia,nvec-aux";
				reg = <6>;
				packet-size = <6>;
			};
		};
};

Only thing I'm not sure is where to put the clock-frequency because it really 
depends on the i2c clients and if we have multiple masters, we could use 
multiple frequencies. Or maybe the lowest supported must be used instead, in 
which case the clock-frequency is better placed in the controller node.

Marc
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140430/a956e2a9/attachment.pgp>

Re: [U-Boot] [Ac100] [PATCH v2 5/6] ARM: tegra: paz00: add dtbindings for nvec

[Stephen Warren]

On 04/30/2014 01:52 AM, Marc Dietrich wrote:
> Hi,
> 
> Am Montag, 28. April 2014, 17:04:13 schrieb Stephen Warren:
>> On 04/26/2014 07:14 PM, Andrey Danin wrote:
>>
>> This patch isn't adding DT bindings for NVEC, but rather add DT nodes.
>> The binding is the schema, not the content.
>>
>> We need a DT binding document that's been reviewed by the DT binding
>> maintainers. Can you please first submit a patch to the Linux kernel
>> that modifies the existing I2C core and Tegra I2C controller binding
>> documentation to add slave mode support. Once that's fully reviewed and
>> ack'd, this patch series can implement support for it in U-Boot.
> 
> I'm sorry that I didn't came up with a proper kernel implementation, but while 
> we are discussion the binding I just want to give some coins.

I'm not looking for a kernel driver implementation, but we do need to
the DT binding fully reviewed and accepted.

>>> diff --git a/arch/arm/dts/tegra20-paz00.dts
>>> b/arch/arm/dts/tegra20-paz00.dts> 
>>>  	i2c@7000c500 {
>>>
>>> -		status = "disabled";
>>> +		status = "okay";
>>> +		clock-frequency = <40000>;
>>> +		slave-addr = <138>;
>>> +		nvec {
>>> +			compatible = "nvidia,tegra20-nvec";
>>> +			request-gpios = <&gpio 170 0>; /* gpio PV2 */
>>
>> The reg property is missing here. Since the i2c node has
>> #address-cells/#size-cells, there must be a reg property in the children.
>>
>> There's nothing here to indicate that this node is a slave device rather
>> than a master device, and doesn't seem to be any allowance for a single
>> I2C controller to support both master and slave nodes at the same time
>> (which I think Tegra's controller can IIRC).
>>
>> IIRC, I had previously suggested something like encoding master/slave
>> into the reg property of the I2C child nodes. We could either do:
>>
>> a) Set some top-bit to indicate a slave device.
>>
>> b) If #address-cells=<1>, only master devices are present. If
>> #address-cells=<2>, either master or slave devices could be present.
>> Cell 0 could be 0==master, 1==slave, and cell 1 the actual I2C bus address.
> 
> I'm not sure if this is really needed. NVEC knows it has to configure the 
> tegra controller as slave. I don't see a reason to double this fact in the 
> device tree. I like the idea of how the downstream kernel does it. The driver 
> calls something like tegra_i2c_init_slave with the slave address as an 
> parameter. This means that the slave address is not a property of the i2c 
> (slave-) controller, but of the master because it has the address hard coded 
> in its firmware. So reg = <138>; would be sufficient here and it also enables 
> multi-slave configs.

The binding in this patch is very special-cased. Instead, we need to
create a generic binding that will work identically across arbitrary I2C
controllers, some of which do support multiple slave addresses. The
issue that caused Linus to blow up about ARM was because everyone went
off and did their own HW-specific stuff without taking a look at the big
picture. That's exactly what the binding proposed in this patch does too.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[U-Boot] [Ac100] [PATCH v2 5/6] ARM: tegra: paz00: add dtbindings for nvec

[Stephen Warren]

On 04/30/2014 01:52 AM, Marc Dietrich wrote:
> Hi,
> 
> Am Montag, 28. April 2014, 17:04:13 schrieb Stephen Warren:
>> On 04/26/2014 07:14 PM, Andrey Danin wrote:
>>
>> This patch isn't adding DT bindings for NVEC, but rather add DT nodes.
>> The binding is the schema, not the content.
>>
>> We need a DT binding document that's been reviewed by the DT binding
>> maintainers. Can you please first submit a patch to the Linux kernel
>> that modifies the existing I2C core and Tegra I2C controller binding
>> documentation to add slave mode support. Once that's fully reviewed and
>> ack'd, this patch series can implement support for it in U-Boot.
> 
> I'm sorry that I didn't came up with a proper kernel implementation, but while 
> we are discussion the binding I just want to give some coins.

I'm not looking for a kernel driver implementation, but we do need to
the DT binding fully reviewed and accepted.

>>> diff --git a/arch/arm/dts/tegra20-paz00.dts
>>> b/arch/arm/dts/tegra20-paz00.dts> 
>>>  	i2c at 7000c500 {
>>>
>>> -		status = "disabled";
>>> +		status = "okay";
>>> +		clock-frequency = <40000>;
>>> +		slave-addr = <138>;
>>> +		nvec {
>>> +			compatible = "nvidia,tegra20-nvec";
>>> +			request-gpios = <&gpio 170 0>; /* gpio PV2 */
>>
>> The reg property is missing here. Since the i2c node has
>> #address-cells/#size-cells, there must be a reg property in the children.
>>
>> There's nothing here to indicate that this node is a slave device rather
>> than a master device, and doesn't seem to be any allowance for a single
>> I2C controller to support both master and slave nodes at the same time
>> (which I think Tegra's controller can IIRC).
>>
>> IIRC, I had previously suggested something like encoding master/slave
>> into the reg property of the I2C child nodes. We could either do:
>>
>> a) Set some top-bit to indicate a slave device.
>>
>> b) If #address-cells=<1>, only master devices are present. If
>> #address-cells=<2>, either master or slave devices could be present.
>> Cell 0 could be 0==master, 1==slave, and cell 1 the actual I2C bus address.
> 
> I'm not sure if this is really needed. NVEC knows it has to configure the 
> tegra controller as slave. I don't see a reason to double this fact in the 
> device tree. I like the idea of how the downstream kernel does it. The driver 
> calls something like tegra_i2c_init_slave with the slave address as an 
> parameter. This means that the slave address is not a property of the i2c 
> (slave-) controller, but of the master because it has the address hard coded 
> in its firmware. So reg = <138>; would be sufficient here and it also enables 
> multi-slave configs.

The binding in this patch is very special-cased. Instead, we need to
create a generic binding that will work identically across arbitrary I2C
controllers, some of which do support multiple slave addresses. The
issue that caused Linus to blow up about ARM was because everyone went
off and did their own HW-specific stuff without taking a look at the big
picture. That's exactly what the binding proposed in this patch does too.

Re: [U-Boot] [Ac100] [PATCH v2 5/6] ARM: tegra: paz00: add dtbindingsfor nvec

[Marc Dietrich]

On Wed, 30 Apr 2014 10:21:07 -0600
Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
> On 04/30/2014 01:52 AM, Marc Dietrich wrote:
> > Am Montag, 28. April 2014, 17:04:13 schrieb Stephen Warren:
> >> On 04/26/2014 07:14 PM, Andrey Danin wrote:
> >>
> >> This patch isn't adding DT bindings for NVEC, but rather add DT nodes.
> >> The binding is the schema, not the content.
> >>
> >> We need a DT binding document that's been reviewed by the DT binding
> >> maintainers. Can you please first submit a patch to the Linux kernel
> >> that modifies the existing I2C core and Tegra I2C controller binding
> >> documentation to add slave mode support. Once that's fully reviewed and
> >> ack'd, this patch series can implement support for it in U-Boot.
> > 
> > I'm sorry that I didn't came up with a proper kernel implementation, but while 
> > we are discussion the binding I just want to give some coins.
> 
> I'm not looking for a kernel driver implementation, but we do need to
> the DT binding fully reviewed and accepted.

well, a kernel implementation would have included the binding.

> >>> diff --git a/arch/arm/dts/tegra20-paz00.dts
> >>> b/arch/arm/dts/tegra20-paz00.dts> 
> >>>  	i2c@7000c500 {
> >>>
> >>> -		status = "disabled";
> >>> +		status = "okay";
> >>> +		clock-frequency = <40000>;
> >>> +		slave-addr = <138>;
> >>> +		nvec {
> >>> +			compatible = "nvidia,tegra20-nvec";
> >>> +			request-gpios = <&gpio 170 0>; /* gpio PV2 */
> >>
> >> The reg property is missing here. Since the i2c node has
> >> #address-cells/#size-cells, there must be a reg property in the children.
> >>
> >> There's nothing here to indicate that this node is a slave device rather
> >> than a master device, and doesn't seem to be any allowance for a single
> >> I2C controller to support both master and slave nodes at the same time
> >> (which I think Tegra's controller can IIRC).
> >>
> >> IIRC, I had previously suggested something like encoding master/slave
> >> into the reg property of the I2C child nodes. We could either do:
> >>
> >> a) Set some top-bit to indicate a slave device.
> >>
> >> b) If #address-cells=<1>, only master devices are present. If
> >> #address-cells=<2>, either master or slave devices could be present.
> >> Cell 0 could be 0==master, 1==slave, and cell 1 the actual I2C bus address.
> > 
> > I'm not sure if this is really needed. NVEC knows it has to configure the 
> > tegra controller as slave. I don't see a reason to double this fact in the 
> > device tree. I like the idea of how the downstream kernel does it. The driver 
> > calls something like tegra_i2c_init_slave with the slave address as an 
> > parameter. This means that the slave address is not a property of the i2c 
> > (slave-) controller, but of the master because it has the address hard coded 
> > in its firmware. So reg = <138>; would be sufficient here and it also enables 
> > multi-slave configs.
> 
> The binding in this patch is very special-cased. Instead, we need to
> create a generic binding that will work identically across arbitrary I2C
> controllers, some of which do support multiple slave addresses. 

I don't see why multi-slave capable controllers won't work. You just
increase the number in the controller's address-cells and add the slave
addresses to the master's reg property. The point here is that the slave
addresses of an i2c controller may be configured at runtime, not at
device initialization via devicetree. This also gives a higher
flexibility for different slave/master combinations.

> The issue that caused Linus to blow up about ARM was because everyone went
> off and did their own HW-specific stuff without taking a look at the big
> picture. That's exactly what the binding proposed in this patch does too.

I can only assume that your are slowly getting tired of this binding
discussion. Still there is no reason for harsh words or to delete my
proposal from the context. IMHO, I tried to be as generic as possible.

Anyway, we should move this discussion to the devicetree ml.

Marc
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[U-Boot] [Ac100] [PATCH v2 5/6] ARM: tegra: paz00: add dtbindingsfor nvec

[Marc Dietrich]

On Wed, 30 Apr 2014 10:21:07 -0600
Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 04/30/2014 01:52 AM, Marc Dietrich wrote:
> > Am Montag, 28. April 2014, 17:04:13 schrieb Stephen Warren:
> >> On 04/26/2014 07:14 PM, Andrey Danin wrote:
> >>
> >> This patch isn't adding DT bindings for NVEC, but rather add DT nodes.
> >> The binding is the schema, not the content.
> >>
> >> We need a DT binding document that's been reviewed by the DT binding
> >> maintainers. Can you please first submit a patch to the Linux kernel
> >> that modifies the existing I2C core and Tegra I2C controller binding
> >> documentation to add slave mode support. Once that's fully reviewed and
> >> ack'd, this patch series can implement support for it in U-Boot.
> > 
> > I'm sorry that I didn't came up with a proper kernel implementation, but while 
> > we are discussion the binding I just want to give some coins.
> 
> I'm not looking for a kernel driver implementation, but we do need to
> the DT binding fully reviewed and accepted.

well, a kernel implementation would have included the binding.

> >>> diff --git a/arch/arm/dts/tegra20-paz00.dts
> >>> b/arch/arm/dts/tegra20-paz00.dts> 
> >>>  	i2c at 7000c500 {
> >>>
> >>> -		status = "disabled";
> >>> +		status = "okay";
> >>> +		clock-frequency = <40000>;
> >>> +		slave-addr = <138>;
> >>> +		nvec {
> >>> +			compatible = "nvidia,tegra20-nvec";
> >>> +			request-gpios = <&gpio 170 0>; /* gpio PV2 */
> >>
> >> The reg property is missing here. Since the i2c node has
> >> #address-cells/#size-cells, there must be a reg property in the children.
> >>
> >> There's nothing here to indicate that this node is a slave device rather
> >> than a master device, and doesn't seem to be any allowance for a single
> >> I2C controller to support both master and slave nodes at the same time
> >> (which I think Tegra's controller can IIRC).
> >>
> >> IIRC, I had previously suggested something like encoding master/slave
> >> into the reg property of the I2C child nodes. We could either do:
> >>
> >> a) Set some top-bit to indicate a slave device.
> >>
> >> b) If #address-cells=<1>, only master devices are present. If
> >> #address-cells=<2>, either master or slave devices could be present.
> >> Cell 0 could be 0==master, 1==slave, and cell 1 the actual I2C bus address.
> > 
> > I'm not sure if this is really needed. NVEC knows it has to configure the 
> > tegra controller as slave. I don't see a reason to double this fact in the 
> > device tree. I like the idea of how the downstream kernel does it. The driver 
> > calls something like tegra_i2c_init_slave with the slave address as an 
> > parameter. This means that the slave address is not a property of the i2c 
> > (slave-) controller, but of the master because it has the address hard coded 
> > in its firmware. So reg = <138>; would be sufficient here and it also enables 
> > multi-slave configs.
> 
> The binding in this patch is very special-cased. Instead, we need to
> create a generic binding that will work identically across arbitrary I2C
> controllers, some of which do support multiple slave addresses. 

I don't see why multi-slave capable controllers won't work. You just
increase the number in the controller's address-cells and add the slave
addresses to the master's reg property. The point here is that the slave
addresses of an i2c controller may be configured at runtime, not at
device initialization via devicetree. This also gives a higher
flexibility for different slave/master combinations.

> The issue that caused Linus to blow up about ARM was because everyone went
> off and did their own HW-specific stuff without taking a look at the big
> picture. That's exactly what the binding proposed in this patch does too.

I can only assume that your are slowly getting tired of this binding
discussion. Still there is no reason for harsh words or to delete my
proposal from the context. IMHO, I tried to be as generic as possible.

Anyway, we should move this discussion to the devicetree ml.

Marc

[U-Boot] [PATCH v2 6/6] ARM: tegra: paz00: enable nvec keyboard

[Andrey Danin]

Signed-off-by: Andrey Danin <danindrey@mail.ru>
CC: Stephen Warren <swarren@nvidia.com>
CC: Marc Dietrich <marvin24@gmx.de>
CC: Julian Andres Klode <jak@jak-linux.org>
CC: ac100 at lists.launchpad.net
---
 Changes for v2:
	- device tree part moved to separate patch
	- added I2C specific defines for initialization

 include/configs/paz00.h |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/configs/paz00.h b/include/configs/paz00.h
index 9e2686a..39dbe1e 100644
--- a/include/configs/paz00.h
+++ b/include/configs/paz00.h
@@ -72,6 +72,15 @@
 #define CONFIG_SYS_WHITE_ON_BLACK
 #define CONFIG_CONSOLE_SCROLL_LINES	10
 
+/* Keyboard support */
+#define CONFIG_KEYBOARD
+#define CONFIG_TEGRA_NVEC_KEYBOARD
+/* NVEC support */
+#define CONFIG_SYS_I2C
+#define CONFIG_SYS_I2C_TEGRA
+#define CONFIG_SYS_I2C_INIT_BOARD
+#define CONFIG_SYS_I2C_TEGRA_NVEC
+
 #include "tegra-common-post.h"
 
 #endif /* __CONFIG_H */
-- 
1.7.9.5