[PATCH 0/2] i2c: pxa: migrate to i2c-core-slave APIs

[PATCH 0/2] i2c: pxa: migrate to i2c-core-slave APIs

[Patrick Williams]

i2c-pxa has its own i2c slave APIs rather than using the i2c-core
APIs.  There are no in-tree drivers currently using this custom
slave API set.

Migrate the i2c-pxa driver from its own i2c slave APIs to the
i2c-core slave APIs so that in-tree slave devices can be used with
the i2c-pxa hardware (ex. i2c-slave-eeprom).

Patrick Williams (2):
  i2c: pxa: migrate to new i2c_slave APIs
  i2c: pxa: remove unused i2c-slave APIs

 drivers/i2c/busses/Kconfig            |  1 +
 drivers/i2c/busses/i2c-pxa.c          | 75 +++++++++++++++++++++------
 include/linux/i2c-pxa.h               | 18 -------
 include/linux/platform_data/i2c-pxa.h |  4 --
 4 files changed, 61 insertions(+), 37 deletions(-)
 delete mode 100644 include/linux/i2c-pxa.h

-- 
2.17.2 (Apple Git-113)

[PATCH 1/2] i2c: pxa: migrate to new i2c_slave APIs

[Patrick Williams]

The i2c subsystem was enhanced circa 2015 to support operating as
an i2c-slave device.  Prior to that, the i2c-pxa driver supported
an i2c-slave but had its own APIs.  There are no existing in-kernel
drivers or platforms that utilize the i2c-pxa APIs.

Migrate the i2c-pxa driver to the general i2c-slave APIs so that
existing drivers, such as the i2c-slave-eeprom, can be used.

This has been tested with a Marvell EspressoBin, using i2c-pxa and
i2c-slave-eeprom, acting as a slave, and a RaspeberryPi 3, using the
at24 driver, acting as a master.

Signed-off-by: Patrick Williams <alpawi@amazon.com>
---
 drivers/i2c/busses/Kconfig   |  1 +
 drivers/i2c/busses/i2c-pxa.c | 74 +++++++++++++++++++++++++++++-------
 2 files changed, 61 insertions(+), 14 deletions(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 146ce40d8e0a..d0c79ac9ffdb 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -874,6 +874,7 @@ config I2C_PXA_PCI
 config I2C_PXA_SLAVE
 	bool "Intel PXA2XX I2C Slave comms support"
 	depends on I2C_PXA && !X86_32
+	select I2C_SLAVE
 	help
 	  Support I2C slave mode communications on the PXA I2C bus.  This
 	  is necessary for systems where the PXA may be a target on the
diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index 2c3c3d6935c0..c811646e809f 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -180,7 +180,7 @@ struct pxa_i2c {
 	struct i2c_adapter	adap;
 	struct clk		*clk;
 #ifdef CONFIG_I2C_PXA_SLAVE
-	struct i2c_slave_client *slave;
+	struct i2c_client	*slave;
 #endif
 
 	unsigned int		irqlogidx;
@@ -544,22 +544,23 @@ static void i2c_pxa_slave_txempty(struct pxa_i2c *i2c, u32 isr)
 	if (isr & ISR_BED) {
 		/* what should we do here? */
 	} else {
-		int ret = 0;
+		u8 byte = 0;
 
 		if (i2c->slave != NULL)
-			ret = i2c->slave->read(i2c->slave->data);
+			i2c_slave_event(i2c->slave, I2C_SLAVE_READ_PROCESSED,
+					&byte);
 
-		writel(ret, _IDBR(i2c));
+		writel(byte, _IDBR(i2c));
 		writel(readl(_ICR(i2c)) | ICR_TB, _ICR(i2c));   /* allow next byte */
 	}
 }
 
 static void i2c_pxa_slave_rxfull(struct pxa_i2c *i2c, u32 isr)
 {
-	unsigned int byte = readl(_IDBR(i2c));
+	u8 byte = readl(_IDBR(i2c));
 
 	if (i2c->slave != NULL)
-		i2c->slave->write(i2c->slave->data, byte);
+		i2c_slave_event(i2c->slave, I2C_SLAVE_WRITE_RECEIVED, &byte);
 
 	writel(readl(_ICR(i2c)) | ICR_TB, _ICR(i2c));
 }
@@ -572,9 +573,18 @@ static void i2c_pxa_slave_start(struct pxa_i2c *i2c, u32 isr)
 		dev_dbg(&i2c->adap.dev, "SAD, mode is slave-%cx\n",
 		       (isr & ISR_RWM) ? 'r' : 't');
 
-	if (i2c->slave != NULL)
-		i2c->slave->event(i2c->slave->data,
-				 (isr & ISR_RWM) ? I2C_SLAVE_EVENT_START_READ : I2C_SLAVE_EVENT_START_WRITE);
+	if (i2c->slave != NULL) {
+		if (isr & ISR_RWM) {
+			u8 byte = 0;
+
+			i2c_slave_event(i2c->slave, I2C_SLAVE_READ_REQUESTED,
+					&byte);
+			writel(byte, _IDBR(i2c));
+		} else {
+			i2c_slave_event(i2c->slave, I2C_SLAVE_WRITE_REQUESTED,
+					NULL);
+		}
+	}
 
 	/*
 	 * slave could interrupt in the middle of us generating a
@@ -607,7 +617,7 @@ static void i2c_pxa_slave_stop(struct pxa_i2c *i2c)
 		dev_dbg(&i2c->adap.dev, "ISR: SSD (Slave Stop)\n");
 
 	if (i2c->slave != NULL)
-		i2c->slave->event(i2c->slave->data, I2C_SLAVE_EVENT_STOP);
+		i2c_slave_event(i2c->slave, I2C_SLAVE_STOP, NULL);
 
 	if (i2c_debug > 2)
 		dev_dbg(&i2c->adap.dev, "ISR: SSD (Slave Stop) acked\n");
@@ -619,6 +629,38 @@ static void i2c_pxa_slave_stop(struct pxa_i2c *i2c)
 	if (i2c->msg)
 		i2c_pxa_master_complete(i2c, I2C_RETRY);
 }
+
+static int i2c_pxa_slave_reg(struct i2c_client *slave)
+{
+	struct pxa_i2c *i2c = slave->adapter->algo_data;
+
+	if (i2c->slave)
+		return -EBUSY;
+
+	if (!i2c->reg_isar)
+		return -EAFNOSUPPORT;
+
+	i2c->slave = slave;
+	i2c->slave_addr = slave->addr;
+
+	writel(i2c->slave_addr, _ISAR(i2c));
+
+	return 0;
+}
+
+static int i2c_pxa_slave_unreg(struct i2c_client *slave)
+{
+	struct pxa_i2c *i2c = slave->adapter->algo_data;
+
+	WARN_ON(!i2c->slave);
+
+	i2c->slave_addr = I2C_PXA_SLAVE_ADDR;
+	writel(i2c->slave_addr, _ISAR(i2c));
+
+	i2c->slave = NULL;
+
+	return 0;
+}
 #else
 static void i2c_pxa_slave_txempty(struct pxa_i2c *i2c, u32 isr)
 {
@@ -1141,11 +1183,19 @@ static u32 i2c_pxa_functionality(struct i2c_adapter *adap)
 static const struct i2c_algorithm i2c_pxa_algorithm = {
 	.master_xfer	= i2c_pxa_xfer,
 	.functionality	= i2c_pxa_functionality,
+#ifdef CONFIG_I2C_PXA_SLAVE
+	.reg_slave	= i2c_pxa_slave_reg,
+	.unreg_slave	= i2c_pxa_slave_unreg,
+#endif
 };
 
 static const struct i2c_algorithm i2c_pxa_pio_algorithm = {
 	.master_xfer	= i2c_pxa_pio_xfer,
 	.functionality	= i2c_pxa_functionality,
+#ifdef CONFIG_I2C_PXA_SLAVE
+	.reg_slave	= i2c_pxa_slave_reg,
+	.unreg_slave	= i2c_pxa_slave_unreg,
+#endif
 };
 
 static const struct of_device_id i2c_pxa_dt_ids[] = {
@@ -1270,10 +1320,6 @@ static int i2c_pxa_probe(struct platform_device *dev)
 	i2c->highmode_enter = false;
 
 	if (plat) {
-#ifdef CONFIG_I2C_PXA_SLAVE
-		i2c->slave_addr = plat->slave_addr;
-		i2c->slave = plat->slave;
-#endif
 		i2c->adap.class = plat->class;
 	}
 
-- 
2.17.2 (Apple Git-113)

[PATCH 2/2] i2c: pxa: remove unused i2c-slave APIs

[Patrick Williams]

With the i2c-pxa driver migrated to the standard i2c-slave
APIs, the custom APIs and structures are no longer needed
or used.  Remove them.

Signed-off-by: Patrick Williams <alpawi@amazon.com>
---
 drivers/i2c/busses/i2c-pxa.c          |  1 -
 include/linux/i2c-pxa.h               | 18 ------------------
 include/linux/platform_data/i2c-pxa.h |  4 ----
 3 files changed, 23 deletions(-)
 delete mode 100644 include/linux/i2c-pxa.h

diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index c811646e809f..466e4f681d7a 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -25,7 +25,6 @@
 #include <linux/delay.h>
 #include <linux/errno.h>
 #include <linux/interrupt.h>
-#include <linux/i2c-pxa.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
diff --git a/include/linux/i2c-pxa.h b/include/linux/i2c-pxa.h
deleted file mode 100644
index a897e2b507b6..000000000000
--- a/include/linux/i2c-pxa.h
+++ /dev/null
@@ -1,18 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef _LINUX_I2C_ALGO_PXA_H
-#define _LINUX_I2C_ALGO_PXA_H
-
-typedef enum i2c_slave_event_e {
-	I2C_SLAVE_EVENT_START_READ,
-	I2C_SLAVE_EVENT_START_WRITE,
-	I2C_SLAVE_EVENT_STOP
-} i2c_slave_event_t;
-
-struct i2c_slave_client {
-	void *data;
-	void (*event)(void *ptr, i2c_slave_event_t event);
-	int  (*read) (void *ptr);
-	void (*write)(void *ptr, unsigned int val);
-};
-
-#endif /* _LINUX_I2C_ALGO_PXA_H */
diff --git a/include/linux/platform_data/i2c-pxa.h b/include/linux/platform_data/i2c-pxa.h
index cb290092599c..6a9b28399b39 100644
--- a/include/linux/platform_data/i2c-pxa.h
+++ b/include/linux/platform_data/i2c-pxa.h
@@ -55,11 +55,7 @@
  */
 #define I2C_ISR_INIT	0x7FF  /* status register init */
 
-struct i2c_slave_client;
-
 struct i2c_pxa_platform_data {
-	unsigned int		slave_addr;
-	struct i2c_slave_client	*slave;
 	unsigned int		class;
 	unsigned int		use_pio :1;
 	unsigned int		fast_mode :1;
-- 
2.17.2 (Apple Git-113)

Re: [PATCH 1/2] i2c: pxa: migrate to new i2c_slave APIs

[Andy Shevchenko]

On Tue, Oct 01, 2019 at 10:59:59AM -0500, Patrick Williams wrote:
> The i2c subsystem was enhanced circa 2015 to support operating as
> an i2c-slave device.  Prior to that, the i2c-pxa driver supported
> an i2c-slave but had its own APIs.  There are no existing in-kernel
> drivers or platforms that utilize the i2c-pxa APIs.
> 
> Migrate the i2c-pxa driver to the general i2c-slave APIs so that
> existing drivers, such as the i2c-slave-eeprom, can be used.
> 
> This has been tested with a Marvell EspressoBin, using i2c-pxa and
> i2c-slave-eeprom, acting as a slave, and a RaspeberryPi 3, using the
> at24 driver, acting as a master.

There are quite a few people in the Cc list. I'm not sure they all are
interested in this. I deliberately dropped few names, sorry, if I was mistaken.

> +		if (isr & ISR_RWM) {
> +			u8 byte = 0;
> +
> +			i2c_slave_event(i2c->slave, I2C_SLAVE_READ_REQUESTED,
> +					&byte);
> +			writel(byte, _IDBR(i2c));
> +		} else {
> +			i2c_slave_event(i2c->slave, I2C_SLAVE_WRITE_REQUESTED,
> +					NULL);
> +		}

Hmm... Perhaps

		u8 byte = 0;

		i2c_slave_event(i2c->slave, I2C_SLAVE_READ_REQUESTED, &byte);
		if (isr & ISR_RWM)
			writel(byte, _IDBR(i2c));

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] i2c: pxa: migrate to new i2c_slave APIs

[Patrick Williams]

Thanks for the review Andy.

On Tue, Oct 01, 2019 at 07:29:13PM +0300, Andy Shevchenko wrote:
> 
> 
> On Tue, Oct 01, 2019 at 10:59:59AM -0500, Patrick Williams wrote:
> There are quite a few people in the Cc list. I'm not sure they all are
> interested in this. I deliberately dropped few names, sorry, if I was mistaken.

Agree it was kind of a big list.  Just chose what was given to me by
get_maintainer.pl.  It seems like there isn't a direct identified
maintainer of this file.

> 
> > +		if (isr & ISR_RWM) {
> > +			u8 byte = 0;
> > +
> > +			i2c_slave_event(i2c->slave, I2C_SLAVE_READ_REQUESTED,
> > +					&byte);
> > +			writel(byte, _IDBR(i2c));
> > +		} else {
> > +			i2c_slave_event(i2c->slave, I2C_SLAVE_WRITE_REQUESTED,
> > +					NULL);
> > +		}
> 
> Hmm... Perhaps
> 
> 		u8 byte = 0;
> 
> 		i2c_slave_event(i2c->slave, I2C_SLAVE_READ_REQUESTED, &byte);
> 		if (isr & ISR_RWM)
> 			writel(byte, _IDBR(i2c));
> 

The two different paths also require READ_REQUEST vs WRITE_REQUESTED.  I
could do a ternary there but it seemed more obvious to just unroll the
logic.

-- 
- Patrick

Re: [PATCH 1/2] i2c: pxa: migrate to new i2c_slave APIs

[Wolfram Sang]

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

On Tue, Oct 01, 2019 at 10:59:59AM -0500, Patrick Williams wrote:
> The i2c subsystem was enhanced circa 2015 to support operating as
> an i2c-slave device.  Prior to that, the i2c-pxa driver supported
> an i2c-slave but had its own APIs.  There are no existing in-kernel
> drivers or platforms that utilize the i2c-pxa APIs.
> 
> Migrate the i2c-pxa driver to the general i2c-slave APIs so that
> existing drivers, such as the i2c-slave-eeprom, can be used.
> 
> This has been tested with a Marvell EspressoBin, using i2c-pxa and
> i2c-slave-eeprom, acting as a slave, and a RaspeberryPi 3, using the
> at24 driver, acting as a master.
> 
> Signed-off-by: Patrick Williams <alpawi@amazon.com>

Awesome! This was so needed but I always wondered if PXA was still
around...

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]