[PATCH v3 0/4] arm: tegra: implement NVEC driver using tegra i2c.

[PATCH v3 0/4] arm: tegra: implement NVEC driver using tegra i2c.

[Andrey Danin]

This version (v3) is for pushing tegra i2c driver to i2c tree.
NVEC driver will be reworked later to use i2c core slave framework.

NVEC driver contains code to manage tegra i2c controller in slave mode.
I2C slave support was implemented in linux kernel. The goal of this
patch serie is to implement I2C slave mode in tegra drived and rework
NVEC driver to use it.

Patches are based on i2c for-next.

Changes for v3:
- rebase on top of i2c for-next tree
- fix 10-bit address condition in tegra i2c driver

Changes for v2:
- rebased on top of new i2c slave framework.
- old code is removed in separate patch
- documentation patch is integrated to main nvec patch


Thanks in advance

*** BLURB HERE ***

Andrey Danin (4):
  i2c: tegra: implement slave mode
  staging/nvec: reimplement on top of tegra i2c driver
  staging/nvec: remove old code
  dt: paz00: define nvec as child of i2c bus

 .../devicetree/bindings/nvec/nvidia,nvec.txt       |  21 +-
 arch/arm/boot/dts/tegra20-paz00.dts                |  22 +-
 drivers/i2c/busses/Kconfig                         |   1 +
 drivers/i2c/busses/i2c-tegra.c                     | 119 ++++++
 drivers/staging/nvec/nvec.c                        | 411 +++++++--------------
 drivers/staging/nvec/nvec.h                        |  10 -
 6 files changed, 269 insertions(+), 315 deletions(-)

-- 
1.9.1

[PATCH v3 1/4] i2c: tegra: implement slave mode

[Andrey Danin]

Initialization code is based on NVEC driver.

There is a HW bug in AP20 that was also mentioned in kernel sources
for Toshiba AC100.

Signed-off-by: Andrey Danin <danindrey-JGs/UdohzUI@public.gmane.org>
---
Changes for v3:
- handle 10-bit clients properly

Changes for v2:
- remove hack from tegra_i2c_clock_disable
- replace slave status helper functions with local variables
- add constant for default delay count value
- add 10-bit address support
- remove read_slave_start_delay init as zero
- don't reset controller during slave registration
- slave isr returns int instead of bool
- make status related variables in slave u32 instead of unsigned int
- enable i2c slave in Kconfig
- rebase on top of new i2c slave framework
- delay workaround was added from nvec

Signed-off-by: Andrey Danin <danindrey-JGs/UdohzUI@public.gmane.org>
---
 drivers/i2c/busses/Kconfig     |   1 +
 drivers/i2c/busses/i2c-tegra.c | 119 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 120 insertions(+)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 0b798ae..3c02041 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -888,6 +888,7 @@ config I2C_SUN6I_P2WI
 config I2C_TEGRA
 	tristate "NVIDIA Tegra internal I2C controller"
 	depends on ARCH_TEGRA
+	select I2C_SLAVE
 	help
 	  If you say yes to this option, support will be included for the
 	  I2C controller embedded in NVIDIA Tegra SOCs
diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 78a3668..f6ecd28 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -42,8 +42,17 @@
 #define I2C_SL_CNFG				0x020
 #define I2C_SL_CNFG_NACK			(1<<1)
 #define I2C_SL_CNFG_NEWSL			(1<<2)
+#define I2C_SL_RCVD				0x024
+#define I2C_SL_STATUS				0x028
+#define I2C_SL_ST_IRQ				(1<<3)
+#define I2C_SL_ST_END_TRANS			(1<<4)
+#define I2C_SL_ST_RCVD				(1<<2)
+#define I2C_SL_ST_RNW				(1<<1)
 #define I2C_SL_ADDR1				0x02c
 #define I2C_SL_ADDR2				0x030
+#define I2C_SL_ADDR2_TEN_BIT_MODE		1
+#define I2C_SL_DELAY_COUNT			0x03c
+#define I2C_SL_DELAY_COUNT_DEFAULT		0x1E
 #define I2C_TX_FIFO				0x050
 #define I2C_RX_FIFO				0x054
 #define I2C_PACKET_TRANSFER_STATUS		0x058
@@ -125,6 +134,8 @@ enum msg_end_type {
  * @clk_divisor_std_fast_mode: Clock divisor in standard/fast mode. It is
  *		applicable if there is no fast clock source i.e. single clock
  *		source.
+ * @slave_read_start_delay: Workaround for AP20 I2C Slave Controller bug. Delay
+ *              before writing data byte into register I2C_SL_RCVD.
  */
 
 struct tegra_i2c_hw_feature {
@@ -133,6 +144,7 @@ struct tegra_i2c_hw_feature {
 	bool has_single_clk_source;
 	int clk_divisor_hs_mode;
 	int clk_divisor_std_fast_mode;
+	int slave_read_start_delay;
 };
 
 /**
@@ -173,6 +185,7 @@ struct tegra_i2c_dev {
 	int msg_read;
 	u32 bus_clk_rate;
 	bool is_suspended;
+	struct i2c_client *slave;
 };
 
 static void dvc_writel(struct tegra_i2c_dev *i2c_dev, u32 val, unsigned long reg)
@@ -461,12 +474,78 @@ static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev)
 	return err;
 }
 
+static void tegra_i2c_slave_write(struct tegra_i2c_dev *i2c_dev, u32 val)
+{
+	i2c_writel(i2c_dev, val, I2C_SL_RCVD);
+
+	/*
+	 * TODO: A correct fix needs to be found for this.
+	 *
+	 * We experience less incomplete messages with this delay than without
+	 * it, but we don't know why. Help is appreciated.
+	 */
+	udelay(100);
+}
+
+static int tegra_i2c_slave_isr(int irq, struct tegra_i2c_dev *i2c_dev)
+{
+	u32 status;
+	u8 value;
+	u8 dummy;
+	u32 is_slave_irq, is_read, is_trans_start, is_trans_end;
+
+	if (!i2c_dev->slave || !i2c_dev->slave->slave_cb)
+		return -EINVAL;
+
+	status = i2c_readl(i2c_dev, I2C_SL_STATUS);
+
+	is_slave_irq = (status & I2C_SL_ST_IRQ);
+	is_read = (status & I2C_SL_ST_RNW);
+	is_trans_start = (status & I2C_SL_ST_RCVD);
+	is_trans_end = (status & I2C_SL_ST_END_TRANS);
+
+	if (!is_slave_irq)
+		return -EINVAL;
+
+	/* master sent stop */
+	if (is_trans_end) {
+		i2c_slave_event(i2c_dev->slave, I2C_SLAVE_STOP, &dummy);
+		if (!is_trans_start)
+			return 0;
+	}
+
+	if (is_read) {
+		/* i2c master reads data from us */
+		i2c_slave_event(i2c_dev->slave,
+				is_trans_start ? I2C_SLAVE_READ_REQUESTED
+					       : I2C_SLAVE_READ_PROCESSED,
+				&value);
+		if (is_trans_start && i2c_dev->hw->slave_read_start_delay)
+			udelay(i2c_dev->hw->slave_read_start_delay);
+		tegra_i2c_slave_write(i2c_dev, value);
+	} else {
+		/* i2c master sends data to us */
+		value = i2c_readl(i2c_dev, I2C_SL_RCVD);
+		if (is_trans_start)
+			tegra_i2c_slave_write(i2c_dev, 0);
+		i2c_slave_event(i2c_dev->slave,
+				is_trans_start ? I2C_SLAVE_WRITE_REQUESTED
+					       : I2C_SLAVE_WRITE_RECEIVED,
+				&value);
+	}
+
+	return 0;
+}
+
 static irqreturn_t tegra_i2c_isr(int irq, void *dev_id)
 {
 	u32 status;
 	const u32 status_err = I2C_INT_NO_ACK | I2C_INT_ARBITRATION_LOST;
 	struct tegra_i2c_dev *i2c_dev = dev_id;
 
+	if (!tegra_i2c_slave_isr(irq, i2c_dev))
+		return IRQ_HANDLED;
+
 	status = i2c_readl(i2c_dev, I2C_INT_STATUS);
 
 	if (status == 0) {
@@ -664,9 +743,48 @@ static u32 tegra_i2c_func(struct i2c_adapter *adap)
 	return ret;
 }
 
+static int tegra_reg_slave(struct i2c_client *slave)
+{
+	struct tegra_i2c_dev *i2c_dev = i2c_get_adapdata(slave->adapter);
+	int addr2 = 0;
+
+	if (i2c_dev->slave)
+		return -EBUSY;
+
+	i2c_dev->slave = slave;
+
+	tegra_i2c_clock_enable(i2c_dev);
+
+	i2c_writel(i2c_dev, I2C_SL_CNFG_NEWSL, I2C_SL_CNFG);
+	i2c_writel(i2c_dev, I2C_SL_DELAY_COUNT_DEFAULT, I2C_SL_DELAY_COUNT);
+
+	if (slave->flags & I2C_CLIENT_TEN)
+		addr2 = (slave->addr >> 7) | I2C_SL_ADDR2_TEN_BIT_MODE;
+
+	i2c_writel(i2c_dev, slave->addr, I2C_SL_ADDR1);
+	i2c_writel(i2c_dev, addr2, I2C_SL_ADDR2);
+
+	return 0;
+}
+
+static int tegra_unreg_slave(struct i2c_client *slave)
+{
+	struct tegra_i2c_dev *i2c_dev = i2c_get_adapdata(slave->adapter);
+
+	WARN_ON(!i2c_dev->slave);
+
+	i2c_writel(i2c_dev, 0, I2C_SL_CNFG);
+
+	i2c_dev->slave = NULL;
+
+	return 0;
+}
+
 static const struct i2c_algorithm tegra_i2c_algo = {
 	.master_xfer	= tegra_i2c_xfer,
 	.functionality	= tegra_i2c_func,
+	.reg_slave	= tegra_reg_slave,
+	.unreg_slave	= tegra_unreg_slave,
 };
 
 /* payload size is only 12 bit */
@@ -681,6 +799,7 @@ static const struct tegra_i2c_hw_feature tegra20_i2c_hw = {
 	.has_single_clk_source = false,
 	.clk_divisor_hs_mode = 3,
 	.clk_divisor_std_fast_mode = 0,
+	.slave_read_start_delay = 8,
 };
 
 static const struct tegra_i2c_hw_feature tegra30_i2c_hw = {
-- 
1.9.1

Re: [PATCH v3 1/4] i2c: tegra: implement slave mode

[Wolfram Sang]

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

Hi Andrey,

On Mon, Jul 20, 2015 at 11:35:43PM +0300, Andrey Danin wrote:
> Initialization code is based on NVEC driver.
> 
> There is a HW bug in AP20 that was also mentioned in kernel sources
> for Toshiba AC100.
> 
> Signed-off-by: Andrey Danin <danindrey@mail.ru>

Still doesn't work for me and I think I understand why. Do you run your
I2C controller in slave mode only? That might work, but using it in
master/slave mode simultanously won't work yet as I see it:

* After every transfer (as master), clocks get disabled. I assume the IP
  core won't be able to detect its own address then.

* There is this code in tegra_i2c_init():

	if (!i2c_dev->is_dvc) {
		u32 sl_cfg = i2c_readl(i2c_dev, I2C_SL_CNFG);
		sl_cfg |= I2C_SL_CNFG_NACK | I2C_SL_CNFG_NEWSL;
		i2c_writel(i2c_dev, sl_cfg, I2C_SL_CNFG);
		i2c_writel(i2c_dev, 0xfc, I2C_SL_ADDR1);
		i2c_writel(i2c_dev, 0x00, I2C_SL_ADDR2);

	}

  It probably messes up the slave initialization in tegra_reg_slave().
  At least I see that the slave address gets overwritten when I peek
  the register after boot.

Does that make sense to you?

Thanks,

   Wolfram

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v3 1/4] i2c: tegra: implement slave mode

[Andrey Danin]

On 24.07.2015 12:27, Wolfram Sang wrote:
> Still doesn't work for me and I think I understand why. Do you run your
> I2C controller in slave mode only?

Yes.

> That might work, but using it in
> master/slave mode simultanously won't work yet as I see it:
>
> * After every transfer (as master), clocks get disabled. I assume the IP
>    core won't be able to detect its own address then.

At the begin of my work on this patchset I even denied clock disable 
call if slave is registered (to minimize code that can affect transfer). 
If only slave mode is used, then this logic is not needed.

>
> * There is this code in tegra_i2c_init():
>
> 	if (!i2c_dev->is_dvc) {
> 		u32 sl_cfg = i2c_readl(i2c_dev, I2C_SL_CNFG);
> 		sl_cfg |= I2C_SL_CNFG_NACK | I2C_SL_CNFG_NEWSL;
> 		i2c_writel(i2c_dev, sl_cfg, I2C_SL_CNFG);
> 		i2c_writel(i2c_dev, 0xfc, I2C_SL_ADDR1);
> 		i2c_writel(i2c_dev, 0x00, I2C_SL_ADDR2);
>
> 	}
>
>    It probably messes up the slave initialization in tegra_reg_slave().
>    At least I see that the slave address gets overwritten when I peek
>    the register after boot.
>

tegra_i2c_init is called on probe and resume. Also it is called in case 
of xfer fail. If xfer is ok, then I think slave addr must be kept unchanged.

> Does that make sense to you?

As far as I understand it is a loopback mode. Probably it will not work 
(Stephen Warren already mentioned this).
But we can try to run it.

Re: [PATCH v3 1/4] i2c: tegra: implement slave mode

[Wolfram Sang]

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


> At the begin of my work on this patchset I even denied clock disable call if
> slave is registered (to minimize code that can affect transfer).

I hacked something like this, but it seems it was not enough.

> If only slave mode is used, then this logic is not needed.

This is not sufficent. We shouldn't break being a master only because we
also listen to a slave address (as long as the HW supports that of
course).

> tegra_i2c_init is called on probe and resume. Also it is called in case of
> xfer fail. If xfer is ok, then I think slave addr must be kept unchanged.

This is fragile. Try scanning the bus with i2cdetect and slave setup
will be gone.

> As far as I understand it is a loopback mode. Probably it will not work
> (Stephen Warren already mentioned this).

Just to make clear: I am not saying we should support talking to our own
slave address. But it should still be possible to communicate with other
remote devices on the bus.

Thanks,

   Wolfram

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v3 1/4] i2c: tegra: implement slave mode

[Andrey Danin]

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

On 24.07.2015 13:52, Wolfram Sang wrote:
>
>> At the begin of my work on this patchset I even denied clock disable call if
>> slave is registered (to minimize code that can affect transfer).
>
> I hacked something like this, but it seems it was not enough.
>
>> If only slave mode is used, then this logic is not needed.
>
> This is not sufficent. We shouldn't break being a master only because we
> also listen to a slave address (as long as the HW supports that of
> course).
>
>> tegra_i2c_init is called on probe and resume. Also it is called in case of
>> xfer fail. If xfer is ok, then I think slave addr must be kept unchanged.
>
> This is fragile. Try scanning the bus with i2cdetect and slave setup
> will be gone.
>
>> As far as I understand it is a loopback mode. Probably it will not work
>> (Stephen Warren already mentioned this).
>
> Just to make clear: I am not saying we should support talking to our own
> slave address. But it should still be possible to communicate with other
> remote devices on the bus.

Sorry for the long delay. I tried to analyze the issue. Attached patch 
works on AC100 (Misha Komarovsky helped me with testing).

Wolfram could you please try the patch with your environment?


Thanks.


[-- Attachment #2: 0001-i2c-tegra-don-t-reset-I2C-slave-address-on-init.patch --]
[-- Type: text/plain, Size: 3018 bytes --]

>From 0927b4007786b19e51415c4900863dd4e74fa034 Mon Sep 17 00:00:00 2001
From: Andrey Danin <danindrey-JGs/UdohzUI@public.gmane.org>
Date: Thu, 20 Aug 2015 00:41:39 +0300
Subject: [PATCH] i2c: tegra: don't reset I2C slave address on init

Init function is called multuple times. If I2C controller works
in slave mode, then driver must keep slave registers otherwise
slave configuration will be reseted.

Signed-off-by: Andrey Danin <danindrey-JGs/UdohzUI@public.gmane.org>
---
 drivers/i2c/busses/i2c-tegra.c |   42 +++++++++++++++++++++++++--------------
 1 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 6467ce0..50250a1 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -402,6 +402,22 @@ static void tegra_dvc_init(struct tegra_i2c_dev *i2c_dev)
 	dvc_writel(i2c_dev, val, DVC_CTRL_REG1);
 }
 
+static int tegra_i2c_init_slave(struct tegra_i2c_dev *i2c_dev, u32 addr, u32 flags)
+{
+	int addr2 = 0;
+
+	i2c_writel(i2c_dev, I2C_SL_CNFG_NEWSL, I2C_SL_CNFG);
+	i2c_writel(i2c_dev, I2C_SL_DELAY_COUNT_DEFAULT, I2C_SL_DELAY_COUNT);
+
+	if (flags & I2C_CLIENT_TEN)
+		addr2 = (addr >> 7) | I2C_SL_ADDR2_TEN_BIT_MODE;
+
+	i2c_writel(i2c_dev, addr, I2C_SL_ADDR1);
+	i2c_writel(i2c_dev, addr2, I2C_SL_ADDR2);
+
+	return 0;
+}
+
 static inline int tegra_i2c_clock_enable(struct tegra_i2c_dev *i2c_dev)
 {
 	int ret;
@@ -461,12 +477,16 @@ static int tegra_i2c_init(struct tegra_i2c_dev *i2c_dev)
 	i2c_writel(i2c_dev, clk_divisor, I2C_CLK_DIVISOR);
 
 	if (!i2c_dev->is_dvc) {
-		u32 sl_cfg = i2c_readl(i2c_dev, I2C_SL_CNFG);
-		sl_cfg |= I2C_SL_CNFG_NACK | I2C_SL_CNFG_NEWSL;
-		i2c_writel(i2c_dev, sl_cfg, I2C_SL_CNFG);
-		i2c_writel(i2c_dev, 0xfc, I2C_SL_ADDR1);
-		i2c_writel(i2c_dev, 0x00, I2C_SL_ADDR2);
-
+		if (i2c_dev->slave) {
+			tegra_i2c_init_slave(i2c_dev, i2c_dev->slave->addr,
+					i2c_dev->slave->flags);
+		} else {
+			u32 sl_cfg = i2c_readl(i2c_dev, I2C_SL_CNFG);
+			sl_cfg |= I2C_SL_CNFG_NACK | I2C_SL_CNFG_NEWSL;
+			i2c_writel(i2c_dev, sl_cfg, I2C_SL_CNFG);
+			i2c_writel(i2c_dev, 0xfc, I2C_SL_ADDR1);
+			i2c_writel(i2c_dev, 0x00, I2C_SL_ADDR2);
+		}
 	}
 
 	val = 7 << I2C_FIFO_CONTROL_TX_TRIG_SHIFT |
@@ -767,7 +787,6 @@ static u32 tegra_i2c_func(struct i2c_adapter *adap)
 static int tegra_reg_slave(struct i2c_client *slave)
 {
 	struct tegra_i2c_dev *i2c_dev = i2c_get_adapdata(slave->adapter);
-	int addr2 = 0;
 
 	if (i2c_dev->slave)
 		return -EBUSY;
@@ -776,14 +795,7 @@ static int tegra_reg_slave(struct i2c_client *slave)
 
 	tegra_i2c_clock_enable(i2c_dev);
 
-	i2c_writel(i2c_dev, I2C_SL_CNFG_NEWSL, I2C_SL_CNFG);
-	i2c_writel(i2c_dev, I2C_SL_DELAY_COUNT_DEFAULT, I2C_SL_DELAY_COUNT);
-
-	if (slave->flags & I2C_CLIENT_TEN)
-		addr2 = (slave->addr >> 7) | I2C_SL_ADDR2_TEN_BIT_MODE;
-
-	i2c_writel(i2c_dev, slave->addr, I2C_SL_ADDR1);
-	i2c_writel(i2c_dev, addr2, I2C_SL_ADDR2);
+	tegra_i2c_init_slave(i2c_dev, slave->addr, slave->flags);
 
 	return 0;
 }
-- 
1.7.1

Re: [PATCH v3 1/4] i2c: tegra: implement slave mode

[Wolfram Sang]

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


> Sorry for the long delay. I tried to analyze the issue. Attached patch works
> on AC100 (Misha Komarovsky helped me with testing).
> 
> Wolfram could you please try the patch with your environment?

No change, sadly. I don't get slave interrupts.

> Init function is called multuple times. If I2C controller works
> in slave mode, then driver must keep slave registers otherwise
> slave configuration will be reseted.

This patch does not tackle the main issue, though. There should not be a
"slave mode" for the controller. It can be a master and slave
simultaneously and should do the right thing depending on what's
happening on the bus. The Tegra2 manual I have says "The Master can
address the internal slave (for basic testing) or an external 7-bit or
10-bit addressed Slave device." So even a loopback should be possible
(if we trust the manual ;)).


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v3 1/4] i2c: tegra: implement slave mode

[Andrey Danin]

Wolfram, thanks!

On 08.09.2015 14:46, Wolfram Sang wrote:
>
>> Sorry for the long delay. I tried to analyze the issue. Attached patch works
>> on AC100 (Misha Komarovsky helped me with testing).
>>
>> Wolfram could you please try the patch with your environment?
>
> No change, sadly. I don't get slave interrupts.

Slave ISR is called only if slave device is registered on a bus. Do you 
get master interrupts ?

>> Init function is called multuple times. If I2C controller works
>> in slave mode, then driver must keep slave registers otherwise
>> slave configuration will be reseted.
>
> This patch does not tackle the main issue, though. There should not be a
> "slave mode" for the controller.

Looks like my commit message is not clear enough :(
 >> If I2C controller works in slave mode, then ...
I mean something like this:
"If slave functionality is enabled, then ..."

> It can be a master and slave
> simultaneously and should do the right thing depending on what's
> happening on the bus. The Tegra2 manual I have says "The Master can
> address the internal slave (for basic testing) or an external 7-bit or
> 10-bit addressed Slave device." So even a loopback should be possible
> (if we trust the manual ;)).

Slave logic is not enabled by default (we don't set up proper 
configuration and slave address). We can enable it by default but it is 
useless without driver that will handle requests.

We used i2cdetect on the I2C bus where master NVEC controller is 
connected. i2cdetect found devices on the bus. Also keyboard and mouse 
was running fine after that (slave logic was not disabled).

[PATCH v3 2/4] staging/nvec: reimplement on top of tegra i2c driver

[Andrey Danin]

Remove i2c controller related code and use tegra i2c driver in slave mode.
Update nvec documentation.

Signed-off-by: Andrey Danin <danindrey-JGs/UdohzUI@public.gmane.org>
---
Changes for v3:
- resolve conflict: 'nvec != NULL' changed to '!nvec'

Changes for v2:
- remove extra new line
- keep old functions to simplify review
- move nvec_state enum to nvec.c
- use nvec-slave instead of nvec in dts to keep ABI compatibility
- rebased on top of new i2c slave framework
- delay workaround moved to tegra-i2c
- documentation patch is integrated in this commit
- reverted a log message to minimize changes

Signed-off-by: Andrey Danin <danindrey-JGs/UdohzUI@public.gmane.org>
---
 .../devicetree/bindings/nvec/nvidia,nvec.txt       |  21 +-
 drivers/staging/nvec/nvec.c                        | 258 +++++++++++++--------
 drivers/staging/nvec/nvec.h                        |  10 -
 3 files changed, 169 insertions(+), 120 deletions(-)

diff --git a/Documentation/devicetree/bindings/nvec/nvidia,nvec.txt b/Documentation/devicetree/bindings/nvec/nvidia,nvec.txt
index 5ae601e..aba34095 100644
--- a/Documentation/devicetree/bindings/nvec/nvidia,nvec.txt
+++ b/Documentation/devicetree/bindings/nvec/nvidia,nvec.txt
@@ -1,21 +1,6 @@
 NVIDIA compliant embedded controller
 
 Required properties:
-- compatible : should be "nvidia,nvec".
-- reg : the iomem of the i2c slave controller
-- interrupts : the interrupt line of the i2c slave controller
-- clock-frequency : the frequency of the i2c bus
-- gpios : the gpio used for ec request
-- slave-addr: the i2c address of the slave controller
-- clocks : Must contain an entry for each entry in clock-names.
-  See ../clocks/clock-bindings.txt for details.
-- clock-names : Must include the following entries:
-  Tegra20/Tegra30:
-  - div-clk
-  - fast-clk
-  Tegra114:
-  - div-clk
-- resets : Must contain an entry for each entry in reset-names.
-  See ../reset/reset.txt for details.
-- reset-names : Must include the following entries:
-  - i2c
+- compatible : should be "nvidia,nvec-slave".
+- reg: the i2c address of the slave controller
+- request-gpios : the gpio used for ec request
diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c
index 164634d..7da3dfe 100644
--- a/drivers/staging/nvec/nvec.c
+++ b/drivers/staging/nvec/nvec.c
@@ -25,8 +25,8 @@
 #include <linux/err.h>
 #include <linux/gpio.h>
 #include <linux/interrupt.h>
+#include <linux/i2c.h>
 #include <linux/io.h>
-#include <linux/irq.h>
 #include <linux/of.h>
 #include <linux/of_gpio.h>
 #include <linux/list.h>
@@ -39,25 +39,19 @@
 
 #include "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
+
+#define I2C_SL_ST_END_TRANS			(1<<4)
+#define I2C_SL_ST_IRQ				(1<<3)
+#define I2C_SL_ST_RCVD				(1<<2)
+#define I2C_SL_ST_RNW				(1<<1)
+
+
+enum nvec_state {
+	ST_NONE,
+	ST_RX,
+	ST_TX,
+	ST_TRANS_START,
+};
 
 /**
  * enum nvec_msg_category - Message categories for nvec_msg_alloc()
@@ -479,7 +473,7 @@ static void nvec_tx_completed(struct nvec_chip *nvec)
 		nvec->tx->pos = 0;
 		nvec_gpio_set_value(nvec, 0);
 	} else {
-		nvec->state = 0;
+		nvec->state = ST_NONE;
 	}
 }
 
@@ -497,7 +491,7 @@ static void nvec_rx_completed(struct nvec_chip *nvec)
 			   (uint) nvec->rx->pos);
 
 		nvec_msg_free(nvec, nvec->rx);
-		nvec->state = 0;
+		nvec->state = ST_NONE;
 
 		/* Battery quirk - Often incomplete, and likes to crash */
 		if (nvec->rx->data[0] == NVEC_BAT)
@@ -514,7 +508,7 @@ static void nvec_rx_completed(struct nvec_chip *nvec)
 
 	spin_unlock(&nvec->rx_lock);
 
-	nvec->state = 0;
+	nvec->state = ST_NONE;
 
 	if (!nvec_msg_is_event(nvec->rx))
 		complete(&nvec->ec_transfer);
@@ -522,6 +516,7 @@ static void nvec_rx_completed(struct nvec_chip *nvec)
 	schedule_work(&nvec->rx_work);
 }
 
+#if 0
 /**
  * nvec_invalid_flags - Send an error message about invalid flags and jump
  * @nvec: The nvec device
@@ -536,6 +531,7 @@ static void nvec_invalid_flags(struct nvec_chip *nvec, unsigned int status,
 	if (reset)
 		nvec->state = 0;
 }
+#endif /* FIXME: remove old code */
 
 /**
  * nvec_tx_set - Set the message to transfer (nvec->tx)
@@ -566,6 +562,8 @@ static void nvec_tx_set(struct nvec_chip *nvec)
 		(uint)nvec->tx->size, nvec->tx->data[1]);
 }
 
+
+#if 0
 /**
  * nvec_interrupt - Interrupt handler
  * @irq: The IRQ
@@ -755,6 +753,115 @@ static void nvec_disable_i2c_slave(struct nvec_chip *nvec)
 	clk_disable_unprepare(nvec->i2c_clk);
 }
 #endif
+#endif /* FIXME: remove old code. */
+
+
+/**
+ * nvec_slave_cb - I2C slave callback
+ *
+ * This callback fills our RX buffers and empties our TX
+ * buffers. This uses a finite state machine.
+ */
+static int nvec_slave_cb(struct i2c_client *client,
+		enum i2c_slave_event event, u8 *val)
+{
+	struct nvec_chip *nvec = i2c_get_clientdata(client);
+
+	switch (event) {
+	case I2C_SLAVE_WRITE_REQUESTED:
+		/* Alloc new msg only if prev transaction finished */
+		if (nvec->state == ST_NONE)
+			nvec->rx = nvec_msg_alloc(nvec, NVEC_MSG_RX);
+
+		/* Should not happen in a normal world */
+		if (unlikely(nvec->rx == NULL)) {
+			nvec->state = ST_NONE;
+			return -1;
+		}
+		nvec->rx->pos = 0;
+
+		if (client->addr != ((*val) >> 1)) {
+			dev_err(&client->dev,
+				"received address 0x%02x, expected 0x%02x\n",
+				((*val) >> 1), client->addr);
+			return -1;
+		}
+		nvec->state = ST_TRANS_START;
+		break;
+
+	case I2C_SLAVE_WRITE_RECEIVED:
+		if (nvec->state != ST_TRANS_START &&
+		    nvec->state != ST_RX) {
+			dev_err(&client->dev,
+				"unexpected write: state %d\n",
+				nvec->state);
+			return -1;
+		}
+		if (nvec->rx->pos >= NVEC_MSG_SIZE) {
+			dev_err(&client->dev,
+				"write buffer overflow: state %d\n",
+				nvec->state);
+			return -1;
+		}
+		nvec->rx->data[nvec->rx->pos++] = *val;
+		nvec->state = ST_RX;
+		break;
+
+	case I2C_SLAVE_READ_REQUESTED:
+		if (nvec->state != ST_RX) {
+			dev_err(&client->dev,
+				"unexpected read request: state %d\n",
+				nvec->state);
+			return -1;
+		}
+		nvec_msg_free(nvec, nvec->rx);
+		nvec_tx_set(nvec);
+		/* Should not happen in a normal world */
+		if (!nvec->tx) {
+			dev_err(&client->dev,
+				"can't alloc tx msg: state %d\n",
+				nvec->state);
+			return -1;
+		}
+		nvec_gpio_set_value(nvec, 1);
+		/* fallthrough */
+	case I2C_SLAVE_READ_PROCESSED:
+		if (nvec->state != ST_RX &&
+		    nvec->state != ST_TX) {
+			dev_err(&client->dev,
+				"unexpected read: state %d\n",
+				nvec->state);
+			return -1;
+		}
+
+		if (!nvec->tx || nvec->tx->pos >= nvec->tx->size) {
+			dev_err(nvec->dev,
+				"tx buffer underflow on %p (%u > %u)\n",
+				nvec->tx,
+				(uint) (nvec->tx ? nvec->tx->pos : 0),
+				(uint) (nvec->tx ? nvec->tx->size : 0));
+			nvec->state = ST_NONE;
+			break;
+		}
+
+		nvec->state = ST_TX;
+		*val = nvec->tx->data[nvec->tx->pos++];
+		break;
+
+	case I2C_SLAVE_STOP:
+		if (nvec->state == ST_TX)
+			nvec_tx_completed(nvec);
+		else if (nvec->state == ST_RX)
+			nvec_rx_completed(nvec);
+		nvec->state = ST_NONE;
+		break;
+
+	default:
+		return 0;
+	}
+
+	return 0;
+}
 
 static void nvec_power_off(void)
 {
@@ -767,7 +874,7 @@ static void nvec_power_off(void)
 /*
  *  Parse common device tree data
  */
-static int nvec_i2c_parse_dt_pdata(struct nvec_chip *nvec)
+static int nvec_i2c_parse_dt(struct nvec_chip *nvec)
 {
 	nvec->gpio = of_get_named_gpio(nvec->dev->of_node, "request-gpios", 0);
 
@@ -776,68 +883,39 @@ static int nvec_i2c_parse_dt_pdata(struct nvec_chip *nvec)
 		return -ENODEV;
 	}
 
-	if (of_property_read_u32(nvec->dev->of_node, "slave-addr",
-				&nvec->i2c_addr)) {
-		dev_err(nvec->dev, "no i2c address specified");
-		return -ENODEV;
-	}
-
 	return 0;
 }
 
-static int tegra_nvec_probe(struct platform_device *pdev)
+static int nvec_probe(struct i2c_client *client, const struct i2c_device_id *id)
 {
-	int err, ret;
-	struct clk *i2c_clk;
+	int ret;
 	struct nvec_chip *nvec;
 	struct nvec_msg *msg;
-	struct resource *res;
-	void __iomem *base;
-	char	get_firmware_version[] = { NVEC_CNTL, GET_FIRMWARE_VERSION },
+	char    get_firmware_version[] = { NVEC_CNTL, GET_FIRMWARE_VERSION },
 		unmute_speakers[] = { NVEC_OEM0, 0x10, 0x59, 0x95 },
 		enable_event[7] = { NVEC_SYS, CNF_EVENT_REPORTING, true };
 
-	if (!pdev->dev.of_node) {
-		dev_err(&pdev->dev, "must be instantiated using device tree\n");
+
+	if (!client->dev.of_node) {
+		dev_err(&client->dev, "must be instantiated using device tree\n");
 		return -ENODEV;
 	}
 
-	nvec = devm_kzalloc(&pdev->dev, sizeof(struct nvec_chip), GFP_KERNEL);
+	nvec = devm_kzalloc(&client->dev, sizeof(struct nvec_chip), GFP_KERNEL);
 	if (!nvec)
 		return -ENOMEM;
 
-	platform_set_drvdata(pdev, nvec);
-	nvec->dev = &pdev->dev;
+	nvec->dev = &client->dev;
+	ret = nvec_i2c_parse_dt(nvec);
+	if (ret < 0)
+		return ret;
 
-	err = nvec_i2c_parse_dt_pdata(nvec);
-	if (err < 0)
-		return err;
+	i2c_set_clientdata(client, nvec);
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	base = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(base))
-		return PTR_ERR(base);
+	ret = i2c_slave_register(client, nvec_slave_cb);
+	if (ret < 0)
+		return ret;
 
-	nvec->irq = platform_get_irq(pdev, 0);
-	if (nvec->irq < 0) {
-		dev_err(&pdev->dev, "no irq resource?\n");
-		return -ENODEV;
-	}
-
-	i2c_clk = devm_clk_get(&pdev->dev, "div-clk");
-	if (IS_ERR(i2c_clk)) {
-		dev_err(nvec->dev, "failed to get controller clock\n");
-		return -ENODEV;
-	}
-
-	nvec->rst = devm_reset_control_get(&pdev->dev, "i2c");
-	if (IS_ERR(nvec->rst)) {
-		dev_err(nvec->dev, "failed to get controller reset\n");
-		return PTR_ERR(nvec->rst);
-	}
-
-	nvec->base = base;
-	nvec->i2c_clk = i2c_clk;
 	nvec->rx = &nvec->msg_pool[0];
 
 	ATOMIC_INIT_NOTIFIER_HEAD(&nvec->notifier_list);
@@ -852,23 +930,14 @@ static int tegra_nvec_probe(struct platform_device *pdev)
 	INIT_WORK(&nvec->rx_work, nvec_dispatch);
 	INIT_WORK(&nvec->tx_work, nvec_request_master);
 
-	err = devm_gpio_request_one(&pdev->dev, nvec->gpio, GPIOF_OUT_INIT_HIGH,
-					"nvec gpio");
-	if (err < 0) {
+	ret = devm_gpio_request_one(&client->dev, nvec->gpio,
+			GPIOF_OUT_INIT_HIGH,
+			"nvec gpio");
+	if (ret < 0) {
 		dev_err(nvec->dev, "couldn't request gpio\n");
 		return -ENODEV;
 	}
 
-	err = devm_request_irq(&pdev->dev, nvec->irq, nvec_interrupt, 0,
-				"nvec", nvec);
-	if (err) {
-		dev_err(nvec->dev, "couldn't request irq\n");
-		return -ENODEV;
-	}
-	disable_irq(nvec->irq);
-
-	tegra_init_i2c_slave(nvec);
-
 	/* enable event reporting */
 	nvec_toggle_global_events(nvec, true);
 
@@ -907,9 +976,10 @@ static int tegra_nvec_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int tegra_nvec_remove(struct platform_device *pdev)
+
+static int nvec_remove(struct i2c_client *client)
 {
-	struct nvec_chip *nvec = platform_get_drvdata(pdev);
+	struct nvec_chip *nvec = i2c_get_clientdata(client);
 
 	nvec_toggle_global_events(nvec, false);
 	mfd_remove_devices(nvec->dev);
@@ -938,8 +1008,6 @@ static int nvec_suspend(struct device *dev)
 	msg = nvec_write_sync(nvec, ap_suspend, sizeof(ap_suspend));
 	nvec_msg_free(nvec, msg);
 
-	nvec_disable_i2c_slave(nvec);
-
 	return 0;
 }
 
@@ -949,7 +1017,6 @@ static int nvec_resume(struct device *dev)
 	struct nvec_chip *nvec = platform_get_drvdata(pdev);
 
 	dev_dbg(nvec->dev, "resuming\n");
-	tegra_init_i2c_slave(nvec);
 	nvec_toggle_global_events(nvec, true);
 
 	return 0;
@@ -960,14 +1027,21 @@ static SIMPLE_DEV_PM_OPS(nvec_pm_ops, nvec_suspend, nvec_resume);
 
 /* Match table for of_platform binding */
 static const struct of_device_id nvidia_nvec_of_match[] = {
-	{ .compatible = "nvidia,nvec", },
+	{ .compatible = "nvidia,nvec-slave", },
 	{},
 };
 MODULE_DEVICE_TABLE(of, nvidia_nvec_of_match);
 
-static struct platform_driver nvec_device_driver = {
-	.probe   = tegra_nvec_probe,
-	.remove  = tegra_nvec_remove,
+static const struct i2c_device_id nvidia_nvec_i2c_table[] = {
+	{ "nvec-slave", 0 },
+	{},
+};
+MODULE_DEVICE_TABLE(i2c, nvidia_nvec_i2c_table);
+
+static struct i2c_driver i2c_nvec_device_driver = {
+	.probe   = nvec_probe,
+	.remove  = nvec_remove,
+	.id_table = nvidia_nvec_i2c_table,
 	.driver  = {
 		.name = "nvec",
 		.pm = &nvec_pm_ops,
@@ -975,9 +1049,9 @@ static struct platform_driver nvec_device_driver = {
 	}
 };
 
-module_platform_driver(nvec_device_driver);
+module_i2c_driver(i2c_nvec_device_driver);
 
-MODULE_ALIAS("platform:nvec");
+MODULE_ALIAS("i2c:nvec");
 MODULE_DESCRIPTION("NVIDIA compliant embedded controller interface");
 MODULE_AUTHOR("Marc Dietrich <marvin24-Mmb7MZpHnFY@public.gmane.org>");
 MODULE_LICENSE("GPL");
diff --git a/drivers/staging/nvec/nvec.h b/drivers/staging/nvec/nvec.h
index e271375..ad6a556 100644
--- a/drivers/staging/nvec/nvec.h
+++ b/drivers/staging/nvec/nvec.h
@@ -107,11 +107,6 @@ struct nvec_msg {
  * struct nvec_chip - A single connection to an NVIDIA Embedded controller
  * @dev: The device
  * @gpio: The same as for &struct nvec_platform_data
- * @irq: The IRQ of the I2C device
- * @i2c_addr: The address of the I2C slave
- * @base: The base of the memory mapped region of the I2C device
- * @i2c_clk: The clock of the I2C device
- * @rst: The reset of the I2C device
  * @notifier_list: Notifiers to be called on received messages, see
  *                 nvec_register_notifier()
  * @rx_data: Received messages that have to be processed
@@ -137,11 +132,6 @@ struct nvec_msg {
 struct nvec_chip {
 	struct device *dev;
 	int gpio;
-	int irq;
-	int i2c_addr;
-	void __iomem *base;
-	struct clk *i2c_clk;
-	struct reset_control *rst;
 	struct atomic_notifier_head notifier_list;
 	struct list_head rx_data, tx_data;
 	struct notifier_block nvec_status_notifier;
-- 
1.9.1

Re: [PATCH v3 2/4] staging/nvec: reimplement on top of tegra i2c driver

[Stephen Warren]

On 07/20/2015 02:35 PM, Andrey Danin wrote:
> Remove i2c controller related code and use tegra i2c driver in slave mode.
> Update nvec documentation.

> diff --git a/Documentation/devicetree/bindings/nvec/nvidia,nvec.txt b/Documentation/devicetree/bindings/nvec/nvidia,nvec.txt

I would expect this patch to add a new binding file 
nvidia,nvec-slave.txt so that the filename continues to match the 
compatible value it documents. This patch introduces a new binding for 
the NVEC slave as opposed to modifying the existing binding.

> +- compatible : should be "nvidia,nvec-slave".
> +- reg: the i2c address of the slave controller

I think "the I2C address to respond to" would be clearer? You might also 
mention that this needs to include flags from <dt-bindings/i2c/ic2.h>.

[PATCH v3 3/4] staging/nvec: remove old code

[Andrey Danin]

Signed-off-by: Andrey Danin <danindrey-JGs/UdohzUI@public.gmane.org>
---
No changes for v3

No changes for v2

Signed-off-by: Andrey Danin <danindrey-JGs/UdohzUI@public.gmane.org>
---
 drivers/staging/nvec/nvec.c | 211 --------------------------------------------
 1 file changed, 211 deletions(-)

diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c
index 7da3dfe..fc0ee5c 100644
--- a/drivers/staging/nvec/nvec.c
+++ b/drivers/staging/nvec/nvec.c
@@ -516,23 +516,6 @@ static void nvec_rx_completed(struct nvec_chip *nvec)
 	schedule_work(&nvec->rx_work);
 }
 
-#if 0
-/**
- * nvec_invalid_flags - Send an error message about invalid flags and jump
- * @nvec: The nvec device
- * @status: The status flags
- * @reset: Whether we shall jump to state 0.
- */
-static void nvec_invalid_flags(struct nvec_chip *nvec, unsigned int status,
-			       bool reset)
-{
-	dev_err(nvec->dev, "unexpected status flags 0x%02x during state %i\n",
-		status, nvec->state);
-	if (reset)
-		nvec->state = 0;
-}
-#endif /* FIXME: remove old code */
-
 /**
  * nvec_tx_set - Set the message to transfer (nvec->tx)
  * @nvec: A &struct nvec_chip
@@ -562,200 +545,6 @@ static void nvec_tx_set(struct nvec_chip *nvec)
 		(uint)nvec->tx->size, nvec->tx->data[1]);
 }
 
-
-#if 0
-/**
- * nvec_interrupt - Interrupt handler
- * @irq: The IRQ
- * @dev: The nvec device
- *
- * Interrupt handler that fills our RX buffers and empties our TX
- * buffers. This uses a finite state machine with ridiculous amounts
- * of error checking, in order to be fairly reliable.
- */
-static irqreturn_t nvec_interrupt(int irq, void *dev)
-{
-	unsigned long status;
-	unsigned int received = 0;
-	unsigned char to_send = 0xff;
-	const unsigned long irq_mask = I2C_SL_IRQ | END_TRANS | RCVD | RNW;
-	struct nvec_chip *nvec = dev;
-	unsigned int state = nvec->state;
-
-	status = readl(nvec->base + I2C_SL_STATUS);
-
-	/* Filter out some errors */
-	if ((status & irq_mask) == 0 && (status & ~irq_mask) != 0) {
-		dev_err(nvec->dev, "unexpected irq mask %lx\n", status);
-		return IRQ_HANDLED;
-	}
-	if ((status & I2C_SL_IRQ) == 0) {
-		dev_err(nvec->dev, "Spurious IRQ\n");
-		return IRQ_HANDLED;
-	}
-
-	/* The EC did not request a read, so it send us something, read it */
-	if ((status & RNW) == 0) {
-		received = readl(nvec->base + I2C_SL_RCVD);
-		if (status & RCVD)
-			writel(0, nvec->base + I2C_SL_RCVD);
-	}
-
-	if (status == (I2C_SL_IRQ | RCVD))
-		nvec->state = 0;
-
-	switch (nvec->state) {
-	case 0:		/* Verify that its a transfer start, the rest later */
-		if (status != (I2C_SL_IRQ | RCVD))
-			nvec_invalid_flags(nvec, status, false);
-		break;
-	case 1:		/* command byte */
-		if (status != I2C_SL_IRQ) {
-			nvec_invalid_flags(nvec, status, true);
-		} else {
-			nvec->rx = nvec_msg_alloc(nvec, NVEC_MSG_RX);
-			/* Should not happen in a normal world */
-			if (unlikely(nvec->rx == NULL)) {
-				nvec->state = 0;
-				break;
-			}
-			nvec->rx->data[0] = received;
-			nvec->rx->pos = 1;
-			nvec->state = 2;
-		}
-		break;
-	case 2:		/* first byte after command */
-		if (status == (I2C_SL_IRQ | RNW | RCVD)) {
-			udelay(33);
-			if (nvec->rx->data[0] != 0x01) {
-				dev_err(nvec->dev,
-					"Read without prior read command\n");
-				nvec->state = 0;
-				break;
-			}
-			nvec_msg_free(nvec, nvec->rx);
-			nvec->state = 3;
-			nvec_tx_set(nvec);
-			BUG_ON(nvec->tx->size < 1);
-			to_send = nvec->tx->data[0];
-			nvec->tx->pos = 1;
-		} else if (status == (I2C_SL_IRQ)) {
-			BUG_ON(nvec->rx == NULL);
-			nvec->rx->data[1] = received;
-			nvec->rx->pos = 2;
-			nvec->state = 4;
-		} else {
-			nvec_invalid_flags(nvec, status, true);
-		}
-		break;
-	case 3:		/* EC does a block read, we transmit data */
-		if (status & END_TRANS) {
-			nvec_tx_completed(nvec);
-		} else if ((status & RNW) == 0 || (status & RCVD)) {
-			nvec_invalid_flags(nvec, status, true);
-		} else if (nvec->tx && nvec->tx->pos < nvec->tx->size) {
-			to_send = nvec->tx->data[nvec->tx->pos++];
-		} else {
-			dev_err(nvec->dev, "tx buffer underflow on %p (%u > %u)\n",
-				nvec->tx,
-				(uint) (nvec->tx ? nvec->tx->pos : 0),
-				(uint) (nvec->tx ? nvec->tx->size : 0));
-			nvec->state = 0;
-		}
-		break;
-	case 4:		/* EC does some write, we read the data */
-		if ((status & (END_TRANS | RNW)) == END_TRANS)
-			nvec_rx_completed(nvec);
-		else if (status & (RNW | RCVD))
-			nvec_invalid_flags(nvec, status, true);
-		else if (nvec->rx && nvec->rx->pos < NVEC_MSG_SIZE)
-			nvec->rx->data[nvec->rx->pos++] = received;
-		else
-			dev_err(nvec->dev,
-				"RX buffer overflow on %p: Trying to write byte %u of %u\n",
-				nvec->rx, nvec->rx ? nvec->rx->pos : 0,
-				NVEC_MSG_SIZE);
-		break;
-	default:
-		nvec->state = 0;
-	}
-
-	/* If we are told that a new transfer starts, verify it */
-	if ((status & (RCVD | RNW)) == RCVD) {
-		if (received != nvec->i2c_addr)
-			dev_err(nvec->dev,
-			"received address 0x%02x, expected 0x%02x\n",
-			received, nvec->i2c_addr);
-		nvec->state = 1;
-	}
-
-	/* Send data if requested, but not on end of transmission */
-	if ((status & (RNW | END_TRANS)) == RNW)
-		writel(to_send, nvec->base + I2C_SL_RCVD);
-
-	/* If we have send the first byte */
-	if (status == (I2C_SL_IRQ | RNW | RCVD))
-		nvec_gpio_set_value(nvec, 1);
-
-	dev_dbg(nvec->dev,
-		"Handled: %s 0x%02x, %s 0x%02x in state %u [%s%s%s]\n",
-		(status & RNW) == 0 ? "received" : "R=",
-		received,
-		(status & (RNW | END_TRANS)) ? "sent" : "S=",
-		to_send,
-		state,
-		status & END_TRANS ? " END_TRANS" : "",
-		status & RCVD ? " RCVD" : "",
-		status & RNW ? " RNW" : "");
-
-
-	/*
-	 * TODO: A correct fix needs to be found for this.
-	 *
-	 * We experience less incomplete messages with this delay than without
-	 * it, but we don't know why. Help is appreciated.
-	 */
-	udelay(100);
-
-	return IRQ_HANDLED;
-}
-
-static void tegra_init_i2c_slave(struct nvec_chip *nvec)
-{
-	u32 val;
-
-	clk_prepare_enable(nvec->i2c_clk);
-
-	reset_control_assert(nvec->rst);
-	udelay(2);
-	reset_control_deassert(nvec->rst);
-
-	val = I2C_CNFG_NEW_MASTER_SFM | I2C_CNFG_PACKET_MODE_EN |
-	    (0x2 << I2C_CNFG_DEBOUNCE_CNT_SHIFT);
-	writel(val, nvec->base + I2C_CNFG);
-
-	clk_set_rate(nvec->i2c_clk, 8 * 80000);
-
-	writel(I2C_SL_NEWSL, nvec->base + I2C_SL_CNFG);
-	writel(0x1E, nvec->base + I2C_SL_DELAY_COUNT);
-
-	writel(nvec->i2c_addr>>1, nvec->base + I2C_SL_ADDR1);
-	writel(0, nvec->base + I2C_SL_ADDR2);
-
-	enable_irq(nvec->irq);
-}
-
-#ifdef CONFIG_PM_SLEEP
-static void nvec_disable_i2c_slave(struct nvec_chip *nvec)
-{
-	disable_irq(nvec->irq);
-	writel(I2C_SL_NEWSL | I2C_SL_NACK, nvec->base + I2C_SL_CNFG);
-	clk_disable_unprepare(nvec->i2c_clk);
-}
-#endif
-#endif /* FIXME: remove old code. */
-
-
 /**
  * nvec_slave_cb - I2C slave callback
  *
-- 
1.9.1

[PATCH v3 4/4] dt: paz00: define nvec as child of i2c bus

[Andrey Danin]

NVEC driver was reimplemented to use tegra i2c. Use common i2c bindings
for NVEC node.

Signed-off-by: Andrey Danin <danindrey-JGs/UdohzUI@public.gmane.org>
---
No changes for v3:

Changes for v2:
- swap reg and request-gpios properties
- use nvec-slave instead of nvec to keep ABI compatibility
- place doc in separate patch

Signed-off-by: Andrey Danin <danindrey-JGs/UdohzUI@public.gmane.org>
---
 arch/arm/boot/dts/tegra20-paz00.dts | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/arch/arm/boot/dts/tegra20-paz00.dts b/arch/arm/boot/dts/tegra20-paz00.dts
index ed7e100..cd5e6ef 100644
--- a/arch/arm/boot/dts/tegra20-paz00.dts
+++ b/arch/arm/boot/dts/tegra20-paz00.dts
@@ -288,20 +288,16 @@
 		clock-frequency = <100000>;
 	};
 
-	nvec@7000c500 {
-		compatible = "nvidia,nvec";
-		reg = <0x7000c500 0x100>;
-		interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_HIGH>;
-		#address-cells = <1>;
-		#size-cells = <0>;
+	i2c@7000c500 {
+		status = "okay";
 		clock-frequency = <80000>;
-		request-gpios = <&gpio TEGRA_GPIO(V, 2) GPIO_ACTIVE_HIGH>;
-		slave-addr = <138>;
-		clocks = <&tegra_car TEGRA20_CLK_I2C3>,
-		         <&tegra_car TEGRA20_CLK_PLL_P_OUT3>;
-		clock-names = "div-clk", "fast-clk";
-		resets = <&tegra_car 67>;
-		reset-names = "i2c";
+
+		nvec: nvec@45 {
+			compatible = "nvidia,nvec-slave";
+			reg = <0x45>;
+			request-gpios = <&gpio TEGRA_GPIO(V, 2)
+				GPIO_ACTIVE_HIGH>;
+		};
 	};
 
 	i2c@7000d000 {
-- 
1.9.1

Re: [PATCH v3 4/4] dt: paz00: define nvec as child of i2c bus

[Stephen Warren]

On 07/20/2015 02:35 PM, Andrey Danin wrote:
> NVEC driver was reimplemented to use tegra i2c. Use common i2c bindings
> for NVEC node.

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

> +		nvec: nvec@45 {
> +			compatible = "nvidia,nvec-slave";
> +			reg = <0x45>;

I think you need to or in I2C_OWN_SLAVE_ADDRESS from 
<dt-bindings/i2c/i2c.h> here?

Re: [PATCH v3 4/4] dt: paz00: define nvec as child of i2c bus

[Andrey Danin]

On 21.07.2015 1:19, Stephen Warren wrote:
> On 07/20/2015 02:35 PM, Andrey Danin wrote:
>> NVEC driver was reimplemented to use tegra i2c. Use common i2c bindings
>> for NVEC node.
>
>> diff --git a/arch/arm/boot/dts/tegra20-paz00.dts
>> b/arch/arm/boot/dts/tegra20-paz00.dts
>
>> +        nvec: nvec@45 {
>> +            compatible = "nvidia,nvec-slave";
>> +            reg = <0x45>;
>
> I think you need to or in I2C_OWN_SLAVE_ADDRESS from
> <dt-bindings/i2c/i2c.h> here?

Sorry, I mentioned it in letter 0 only.
I will rework nvec driver and device tree according to i2c core slave 
implementation later. v3 of this patchset is for fixing tegra i2c driver 
only.

Re: [PATCH v3 4/4] dt: paz00: define nvec as child of i2c bus

[Marc Dietrich]

[-- Attachment #1.1: Type: text/plain, Size: 1132 bytes --]

Am Dienstag, 21. Juli 2015, 09:35:21 schrieb Andrey Danin:
> On 21.07.2015 1:19, Stephen Warren wrote:
> > On 07/20/2015 02:35 PM, Andrey Danin wrote:
> >> NVEC driver was reimplemented to use tegra i2c. Use common i2c bindings
> >> for NVEC node.
> >> 
> >> diff --git a/arch/arm/boot/dts/tegra20-paz00.dts
> >> b/arch/arm/boot/dts/tegra20-paz00.dts
> >> 
> >> +        nvec: nvec@45 {
> >> +            compatible = "nvidia,nvec-slave";
> >> +            reg = <0x45>;
> > 
> > I think you need to or in I2C_OWN_SLAVE_ADDRESS from
> > <dt-bindings/i2c/i2c.h> here?
> 
> Sorry, I mentioned it in letter 0 only.
> I will rework nvec driver and device tree according to i2c core slave
> implementation later. v3 of this patchset is for fixing tegra i2c driver
> only.

I think in this case it would be better to leave nvec and dt as it is for now, 
and just add the slave function to tegra-i2c. Otherwise we will again have two 
different "nvidia,nvec-slave" bindings (one for the intermediate hack and one 
for the final representation). As an alternative, you could also add slave 
function and port nvec in the same series.

Marc

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

[-- Attachment #2: Type: text/plain, Size: 169 bytes --]

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 4/4] dt: paz00: define nvec as child of i2c bus

[Andrey Danin]

On 21.07.2015 11:25, Marc Dietrich wrote:
> I think in this case it would be better to leave nvec and dt as it is for now,
> and just add the slave function to tegra-i2c. Otherwise we will again have two
> different "nvidia,nvec-slave" bindings (one for the intermediate hack and one
> for the final representation). As an alternative, you could also add slave
> function and port nvec in the same series.
>
First patch only adds slave functionality to tegra-i2c driver. I sent v3 
to fix only tegra-i2c as Wolfram suggested.

Unfortunately I haven't fixed all defects and I will resend patch(es).
I can resend only first patch (for tegra-i2c) if it is more obvious for 
everyone.

Re: [PATCH v3 4/4] dt: paz00: define nvec as child of i2c bus

[Marc Dietrich]

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

Am Dienstag, 21. Juli 2015, 11:51:15 schrieb Andrey Danin:
> On 21.07.2015 11:25, Marc Dietrich wrote:
> > I think in this case it would be better to leave nvec and dt as it is for
> > now, and just add the slave function to tegra-i2c. Otherwise we will
> > again have two different "nvidia,nvec-slave" bindings (one for the
> > intermediate hack and one for the final representation). As an
> > alternative, you could also add slave function and port nvec in the same
> > series.
> 
> First patch only adds slave functionality to tegra-i2c driver. I sent v3
> to fix only tegra-i2c as Wolfram suggested.
> 
> Unfortunately I haven't fixed all defects and I will resend patch(es).
> I can resend only first patch (for tegra-i2c) if it is more obvious for
> everyone.

It's up to you. I think if the NV guys are ok with the tegra i2c change, 
Wolfram can merge it right away for 4.3. Not sure about the state of the nvec 
changes though and if they can be ready soon.

Marc

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

Re: [PATCH v3 4/4] dt: paz00: define nvec as child of i2c bus

[Wolfram Sang]

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


> It's up to you. I think if the NV guys are ok with the tegra i2c change, 
> Wolfram can merge it right away for 4.3.

I asked him to resend, because I *want* to merge it for 4.3 :) Only the
slave-mode enablement for i2c, of course. The other patches need to go
via some other tree (when they are done).

So, V4 with only this one patch is a good idea in my book.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v3 0/4] arm: tegra: implement NVEC driver using tegra i2c.

[Andrey Danin]

On 20.07.2015 23:35, Andrey Danin wrote:
> This version (v3) is for pushing tegra i2c driver to i2c tree.
> NVEC driver will be reworked later to use i2c core slave framework.
>
> NVEC driver contains code to manage tegra i2c controller in slave mode.
> I2C slave support was implemented in linux kernel. The goal of this
> patch serie is to implement I2C slave mode in tegra drived and rework
> NVEC driver to use it.
>
> Patches are based on i2c for-next.
>
> Changes for v3:
> - rebase on top of i2c for-next tree
> - fix 10-bit address condition in tegra i2c driver
>
Sorry, I didn't fix all comments in tegra i2c driver. I will send v4 soon.