[PATCH 0/5] i3c: dw,ast2600: Add In-Band Interrupt support

[PATCH 0/5] i3c: dw,ast2600: Add In-Band Interrupt support

[Jeremy Kerr]

This series implements In-Band Interrupt (IBI) support for the ast2600
i3c driver, by adding the required generic code in the dw driver, then
enabling that (with a required hardware workaround) in the ast2600
driver.

As always: comments, queries etc. are most welcome.

Cheers,


Jeremy

Jeremy Kerr (5):
  i3c: dw: Create a generic fifo read function
  i3c: dw: Turn DAT array entry into a struct
  i3c: dw: Add support for in-band interrupts
  i3c: dw: Add a platform facility for IBI PEC workarounds
  i3c: ast2600: enable IBI support

 drivers/i3c/master/ast2600-i3c-master.c |  21 ++
 drivers/i3c/master/dw-i3c-master.c      | 336 ++++++++++++++++++++++--
 drivers/i3c/master/dw-i3c-master.h      |  32 ++-
 3 files changed, 370 insertions(+), 19 deletions(-)

-- 
2.39.2


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH 1/5] i3c: dw: Create a generic fifo read function

[Jeremy Kerr]

In a future change we'll want to read from the IBI FIFO too, so turn
dw_i3c_read_rx_fifo() into a generic read with the FIFO register as a
parameter.

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
 drivers/i3c/master/dw-i3c-master.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index 9fc03108a5db..cb38ef95f21a 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -318,18 +318,24 @@ static void dw_i3c_master_wr_tx_fifo(struct dw_i3c_master *master,
 	}
 }
 
-static void dw_i3c_master_read_rx_fifo(struct dw_i3c_master *master,
-				       u8 *bytes, int nbytes)
+static void dw_i3c_master_read_fifo(struct dw_i3c_master *master,
+				    int reg,  u8 *bytes, int nbytes)
 {
-	readsl(master->regs + RX_TX_DATA_PORT, bytes, nbytes / 4);
+	readsl(master->regs + reg, bytes, nbytes / 4);
 	if (nbytes & 3) {
 		u32 tmp;
 
-		readsl(master->regs + RX_TX_DATA_PORT, &tmp, 1);
+		readsl(master->regs + reg, &tmp, 1);
 		memcpy(bytes + (nbytes & ~3), &tmp, nbytes & 3);
 	}
 }
 
+static void dw_i3c_master_read_rx_fifo(struct dw_i3c_master *master,
+				       u8 *bytes, int nbytes)
+{
+	return dw_i3c_master_read_fifo(master, RX_TX_DATA_PORT, bytes, nbytes);
+}
+
 static struct dw_i3c_xfer *
 dw_i3c_master_alloc_xfer(struct dw_i3c_master *master, unsigned int ncmds)
 {
-- 
2.39.2


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 1/5] i3c: dw: Create a generic fifo read function

[Ben Dooks]

On 30/03/2023 08:50, Jeremy Kerr wrote:
> In a future change we'll want to read from the IBI FIFO too, so turn
> dw_i3c_read_rx_fifo() into a generic read with the FIFO register as a
> parameter.
> 
> Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
> ---
>   drivers/i3c/master/dw-i3c-master.c | 14 ++++++++++----
>   1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index 9fc03108a5db..cb38ef95f21a 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
> @@ -318,18 +318,24 @@ static void dw_i3c_master_wr_tx_fifo(struct dw_i3c_master *master,
>   	}
>   }
>   
> -static void dw_i3c_master_read_rx_fifo(struct dw_i3c_master *master,
> -				       u8 *bytes, int nbytes)
> +static void dw_i3c_master_read_fifo(struct dw_i3c_master *master,
> +				    int reg,  u8 *bytes, int nbytes)
>   {
> -	readsl(master->regs + RX_TX_DATA_PORT, bytes, nbytes / 4);
> +	readsl(master->regs + reg, bytes, nbytes / 4);
>   	if (nbytes & 3) {
>   		u32 tmp;
>   
> -		readsl(master->regs + RX_TX_DATA_PORT, &tmp, 1);
> +		readsl(master->regs + reg, &tmp, 1);
>   		memcpy(bytes + (nbytes & ~3), &tmp, nbytes & 3);
>   	}
>   }
>   
> +static void dw_i3c_master_read_rx_fifo(struct dw_i3c_master *master,
> +				       u8 *bytes, int nbytes)
> +{
> +	return dw_i3c_master_read_fifo(master, RX_TX_DATA_PORT, bytes, nbytes);
> +}
> +

Might want to make it inline too.


>   static struct dw_i3c_xfer *
>   dw_i3c_master_alloc_xfer(struct dw_i3c_master *master, unsigned int ncmds)
>   {


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 1/5] i3c: dw: Create a generic fifo read function

[Jeremy Kerr]

Hi Ben,

> > +static void dw_i3c_master_read_rx_fifo(struct dw_i3c_master *master,
> > +                                      u8 *bytes, int nbytes)
> > +{
> > +       return dw_i3c_master_read_fifo(master, RX_TX_DATA_PORT, bytes, nbytes);
> > +}
> > +
> 
> Might want to make it inline too.

The compiler will almost certainly work that out:

    $ arm-linux-gnueabihf-objdump -t drivers/i3c/master/dw-i3c-master.o | grep fifo
    00000b74 l     F .text	000000a4 dw_i3c_master_read_fifo

- and I'd prefer to not override any compiler decision not to inline, if
that works out better in specific circumstances or with any future
changes.

Cheers,


Jeremy

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH 2/5] i3c: dw: Turn DAT array entry into a struct

[Jeremy Kerr]

In an upcoming change, we will want to store additional data about the
devices we have in the data address table.

Change the type of the DAT entries into a struct, which currently just
has the address data.

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
 drivers/i3c/master/dw-i3c-master.c | 22 +++++++++++-----------
 drivers/i3c/master/dw-i3c-master.h | 11 ++++++++++-
 2 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index cb38ef95f21a..a086d9c5ff35 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -291,7 +291,7 @@ static int dw_i3c_master_get_addr_pos(struct dw_i3c_master *master, u8 addr)
 	int pos;
 
 	for (pos = 0; pos < master->maxdevs; pos++) {
-		if (addr == master->addrs[pos])
+		if (addr == master->devs[pos].addr)
 			return pos;
 	}
 
@@ -769,7 +769,7 @@ static int dw_i3c_master_daa(struct i3c_master_controller *m)
 		if (ret < 0)
 			return -ENOSPC;
 
-		master->addrs[pos] = ret;
+		master->devs[pos].addr = ret;
 		p = even_parity(ret);
 		last_addr = ret;
 		ret |= (p << 7);
@@ -806,7 +806,7 @@ static int dw_i3c_master_daa(struct i3c_master_controller *m)
 
 	for (pos = 0; pos < master->maxdevs; pos++) {
 		if (newdevs & BIT(pos))
-			i3c_master_add_i3c_dev_locked(m, master->addrs[pos]);
+			i3c_master_add_i3c_dev_locked(m, master->devs[pos].addr);
 	}
 
 	dw_i3c_master_free_xfer(xfer);
@@ -905,11 +905,11 @@ static int dw_i3c_master_reattach_i3c_dev(struct i3c_dev_desc *dev,
 		       master->regs +
 		       DEV_ADDR_TABLE_LOC(master->datstartaddr, data->index));
 
-		master->addrs[data->index] = 0;
+		master->devs[data->index].addr = 0;
 		master->free_pos |= BIT(data->index);
 
 		data->index = pos;
-		master->addrs[pos] = dev->info.dyn_addr;
+		master->devs[pos].addr = dev->info.dyn_addr;
 		master->free_pos &= ~BIT(pos);
 	}
 
@@ -917,7 +917,7 @@ static int dw_i3c_master_reattach_i3c_dev(struct i3c_dev_desc *dev,
 	       master->regs +
 	       DEV_ADDR_TABLE_LOC(master->datstartaddr, data->index));
 
-	master->addrs[data->index] = dev->info.dyn_addr;
+	master->devs[data->index].addr = dev->info.dyn_addr;
 
 	return 0;
 }
@@ -938,11 +938,11 @@ static int dw_i3c_master_attach_i3c_dev(struct i3c_dev_desc *dev)
 		return -ENOMEM;
 
 	data->index = pos;
-	master->addrs[pos] = dev->info.dyn_addr ? : dev->info.static_addr;
+	master->devs[pos].addr = dev->info.dyn_addr ? : dev->info.static_addr;
 	master->free_pos &= ~BIT(pos);
 	i3c_dev_set_master_data(dev, data);
 
-	writel(DEV_ADDR_TABLE_DYNAMIC_ADDR(master->addrs[pos]),
+	writel(DEV_ADDR_TABLE_DYNAMIC_ADDR(master->devs[pos].addr),
 	       master->regs +
 	       DEV_ADDR_TABLE_LOC(master->datstartaddr, data->index));
 
@@ -960,7 +960,7 @@ static void dw_i3c_master_detach_i3c_dev(struct i3c_dev_desc *dev)
 	       DEV_ADDR_TABLE_LOC(master->datstartaddr, data->index));
 
 	i3c_dev_set_master_data(dev, NULL);
-	master->addrs[data->index] = 0;
+	master->devs[data->index].addr = 0;
 	master->free_pos |= BIT(data->index);
 	kfree(data);
 }
@@ -1046,7 +1046,7 @@ static int dw_i3c_master_attach_i2c_dev(struct i2c_dev_desc *dev)
 		return -ENOMEM;
 
 	data->index = pos;
-	master->addrs[pos] = dev->addr;
+	master->devs[pos].addr = dev->addr;
 	master->free_pos &= ~BIT(pos);
 	i2c_dev_set_master_data(dev, data);
 
@@ -1069,7 +1069,7 @@ static void dw_i3c_master_detach_i2c_dev(struct i2c_dev_desc *dev)
 	       DEV_ADDR_TABLE_LOC(master->datstartaddr, data->index));
 
 	i2c_dev_set_master_data(dev, NULL);
-	master->addrs[data->index] = 0;
+	master->devs[data->index].addr = 0;
 	master->free_pos |= BIT(data->index);
 	kfree(data);
 }
diff --git a/drivers/i3c/master/dw-i3c-master.h b/drivers/i3c/master/dw-i3c-master.h
index 9f1e48211aa4..dd0b77e2c66a 100644
--- a/drivers/i3c/master/dw-i3c-master.h
+++ b/drivers/i3c/master/dw-i3c-master.h
@@ -17,6 +17,10 @@ struct dw_i3c_master_caps {
 	u8 datafifodepth;
 };
 
+struct dw_i3c_dat_entry {
+	u8 addr;
+};
+
 struct dw_i3c_master {
 	struct i3c_master_controller base;
 	u16 maxdevs;
@@ -33,7 +37,12 @@ struct dw_i3c_master {
 	struct clk *core_clk;
 	char version[5];
 	char type[5];
-	u8 addrs[DW_I3C_MAX_DEVS];
+
+	/*
+	 * Per-device hardware data, used to manage the device address table
+	 * (DAT)
+	 */
+	struct dw_i3c_dat_entry devs[DW_I3C_MAX_DEVS];
 
 	/* platform-specific data */
 	const struct dw_i3c_platform_ops *platform_ops;
-- 
2.39.2


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH 3/5] i3c: dw: Add support for in-band interrupts

[Jeremy Kerr]

This change adds support for receiving and dequeueing i3c IBIs.

By setting struct dw_i3c_master->ibi_capable before probe, a platform
implementation can select the IBI-enabled version of the i3c_master_ops,
enabling the global IBI infrastrcture for that controller.

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
 drivers/i3c/master/dw-i3c-master.c | 281 ++++++++++++++++++++++++++++-
 drivers/i3c/master/dw-i3c-master.h |  11 ++
 2 files changed, 289 insertions(+), 3 deletions(-)

diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index a086d9c5ff35..c49c5fa01a26 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -76,7 +76,22 @@
 
 #define RX_TX_DATA_PORT			0x14
 #define IBI_QUEUE_STATUS		0x18
+#define IBI_QUEUE_STATUS_IBI_ID(x)	(((x) & GENMASK(15, 8)) >> 8)
+#define IBI_QUEUE_STATUS_DATA_LEN(x)	((x) & GENMASK(7, 0))
+#define IBI_QUEUE_IBI_ADDR(x)		(IBI_QUEUE_STATUS_IBI_ID(x) >> 1)
+#define IBI_QUEUE_IBI_RNW(x)		(IBI_QUEUE_STATUS_IBI_ID(x) & BIT(0))
+#define IBI_TYPE_MR(x)                                                         \
+	((IBI_QUEUE_IBI_ADDR(x) != I3C_HOT_JOIN_ADDR) && !IBI_QUEUE_IBI_RNW(x))
+#define IBI_TYPE_HJ(x)                                                         \
+	((IBI_QUEUE_IBI_ADDR(x) == I3C_HOT_JOIN_ADDR) && !IBI_QUEUE_IBI_RNW(x))
+#define IBI_TYPE_SIRQ(x)                                                        \
+	((IBI_QUEUE_IBI_ADDR(x) != I3C_HOT_JOIN_ADDR) && IBI_QUEUE_IBI_RNW(x))
+
 #define QUEUE_THLD_CTRL			0x1c
+#define QUEUE_THLD_CTRL_IBI_STAT_MASK	GENMASK(31, 24)
+#define QUEUE_THLD_CTRL_IBI_STAT(x)	(((x) - 1) << 24)
+#define QUEUE_THLD_CTRL_IBI_DATA_MASK	GENMASK(20, 16)
+#define QUEUE_THLD_CTRL_IBI_DATA(x)	((x) << 16)
 #define QUEUE_THLD_CTRL_RESP_BUF_MASK	GENMASK(15, 8)
 #define QUEUE_THLD_CTRL_RESP_BUF(x)	(((x) - 1) << 8)
 
@@ -186,6 +201,8 @@
 #define EXTENDED_CAPABILITY		0xe8
 #define SLAVE_CONFIG			0xec
 
+#define DEV_ADDR_TABLE_IBI_MDB		BIT(12)
+#define DEV_ADDR_TABLE_SIR_REJECT	BIT(13)
 #define DEV_ADDR_TABLE_LEGACY_I2C_DEV	BIT(31)
 #define DEV_ADDR_TABLE_DYNAMIC_ADDR(x)	(((x) << 16) & GENMASK(23, 16))
 #define DEV_ADDR_TABLE_STATIC_ADDR(x)	((x) & GENMASK(6, 0))
@@ -221,6 +238,7 @@ struct dw_i3c_xfer {
 
 struct dw_i3c_i2c_dev_data {
 	u8 index;
+	struct i3c_generic_ibi_pool *ibi_pool;
 };
 
 static u8 even_parity(u8 p)
@@ -336,6 +354,12 @@ static void dw_i3c_master_read_rx_fifo(struct dw_i3c_master *master,
 	return dw_i3c_master_read_fifo(master, RX_TX_DATA_PORT, bytes, nbytes);
 }
 
+static void dw_i3c_master_read_ibi_fifo(struct dw_i3c_master *master,
+					u8 *bytes, int nbytes)
+{
+	return dw_i3c_master_read_fifo(master, IBI_QUEUE_STATUS, bytes, nbytes);
+}
+
 static struct dw_i3c_xfer *
 dw_i3c_master_alloc_xfer(struct dw_i3c_master *master, unsigned int ncmds)
 {
@@ -605,7 +629,11 @@ static int dw_i3c_master_bus_init(struct i3c_master_controller *m)
 	}
 
 	thld_ctrl = readl(master->regs + QUEUE_THLD_CTRL);
-	thld_ctrl &= ~QUEUE_THLD_CTRL_RESP_BUF_MASK;
+	thld_ctrl &= ~(QUEUE_THLD_CTRL_RESP_BUF_MASK |
+		       QUEUE_THLD_CTRL_IBI_STAT_MASK |
+		       QUEUE_THLD_CTRL_IBI_STAT_MASK);
+	thld_ctrl |= QUEUE_THLD_CTRL_IBI_STAT(1) |
+		QUEUE_THLD_CTRL_IBI_DATA(31);
 	writel(thld_ctrl, master->regs + QUEUE_THLD_CTRL);
 
 	thld_ctrl = readl(master->regs + DATA_BUFFER_THLD_CTRL);
@@ -1074,6 +1102,226 @@ static void dw_i3c_master_detach_i2c_dev(struct i2c_dev_desc *dev)
 	kfree(data);
 }
 
+static int dw_i3c_master_request_ibi(struct i3c_dev_desc *dev,
+				     const struct i3c_ibi_setup *req)
+{
+	struct dw_i3c_i2c_dev_data *data = i3c_dev_get_master_data(dev);
+	struct i3c_master_controller *m = i3c_dev_get_master(dev);
+	struct dw_i3c_master *master = to_dw_i3c_master(m);
+	unsigned long flags;
+
+	data->ibi_pool = i3c_generic_ibi_alloc_pool(dev, req);
+	if (IS_ERR(data->ibi_pool))
+		return PTR_ERR(data->ibi_pool);
+
+	spin_lock_irqsave(&master->devs_lock, flags);
+	master->devs[data->index].ibi_dev = dev;
+	spin_unlock_irqrestore(&master->devs_lock, flags);
+
+	return 0;
+}
+
+static void dw_i3c_master_free_ibi(struct i3c_dev_desc *dev)
+{
+	struct dw_i3c_i2c_dev_data *data = i3c_dev_get_master_data(dev);
+	struct i3c_master_controller *m = i3c_dev_get_master(dev);
+	struct dw_i3c_master *master = to_dw_i3c_master(m);
+	unsigned long flags;
+
+	spin_lock_irqsave(&master->devs_lock, flags);
+	master->devs[data->index].ibi_dev = NULL;
+	spin_unlock_irqrestore(&master->devs_lock, flags);
+
+	i3c_generic_ibi_free_pool(data->ibi_pool);
+	data->ibi_pool = NULL;
+}
+
+static void dw_i3c_master_set_sir_enabled(struct dw_i3c_master *master,
+					  struct i3c_dev_desc *dev,
+					  u8 idx, bool enable)
+{
+	unsigned long flags;
+	u32 dat_entry, reg;
+	bool global;
+
+	dat_entry = DEV_ADDR_TABLE_LOC(master->datstartaddr, idx);
+
+	spin_lock_irqsave(&master->devs_lock, flags);
+	reg = readl(master->regs + dat_entry);
+	if (enable) {
+		reg &= ~DEV_ADDR_TABLE_SIR_REJECT;
+		if (dev->info.bcr & I3C_BCR_IBI_PAYLOAD)
+			reg |= DEV_ADDR_TABLE_IBI_MDB;
+	} else {
+		reg |= DEV_ADDR_TABLE_SIR_REJECT;
+	}
+	writel(reg, master->regs + dat_entry);
+
+	reg = readl(master->regs + IBI_SIR_REQ_REJECT);
+	if (enable) {
+		global = reg == 0xffffffff;
+		reg &= ~BIT(idx);
+	} else {
+		global = reg == 0;
+		reg |= BIT(idx);
+	}
+	writel(reg, master->regs + IBI_SIR_REQ_REJECT);
+
+	if (global) {
+		reg = readl(master->regs + INTR_STATUS_EN);
+		reg &= ~INTR_IBI_THLD_STAT;
+		if (enable)
+			reg |= INTR_IBI_THLD_STAT;
+		writel(reg, master->regs + INTR_STATUS_EN);
+
+		reg = readl(master->regs + INTR_SIGNAL_EN);
+		reg &= ~INTR_IBI_THLD_STAT;
+		if (enable)
+			reg |= INTR_IBI_THLD_STAT;
+		writel(reg, master->regs + INTR_SIGNAL_EN);
+	}
+
+	spin_unlock_irqrestore(&master->devs_lock, flags);
+}
+
+static int dw_i3c_master_enable_ibi(struct i3c_dev_desc *dev)
+{
+	struct dw_i3c_i2c_dev_data *data = i3c_dev_get_master_data(dev);
+	struct i3c_master_controller *m = i3c_dev_get_master(dev);
+	struct dw_i3c_master *master = to_dw_i3c_master(m);
+	int rc;
+
+	dw_i3c_master_set_sir_enabled(master, dev, data->index, true);
+
+	rc = i3c_master_enec_locked(m, dev->info.dyn_addr, I3C_CCC_EVENT_SIR);
+
+	if (rc)
+		dw_i3c_master_set_sir_enabled(master, dev, data->index, false);
+
+	return rc;
+}
+
+static int dw_i3c_master_disable_ibi(struct i3c_dev_desc *dev)
+{
+	struct dw_i3c_i2c_dev_data *data = i3c_dev_get_master_data(dev);
+	struct i3c_master_controller *m = i3c_dev_get_master(dev);
+	struct dw_i3c_master *master = to_dw_i3c_master(m);
+	int rc;
+
+	rc = i3c_master_disec_locked(m, dev->info.dyn_addr, I3C_CCC_EVENT_SIR);
+	if (rc)
+		return rc;
+
+	dw_i3c_master_set_sir_enabled(master, dev, data->index, false);
+
+	return 0;
+}
+
+static void dw_i3c_master_recycle_ibi_slot(struct i3c_dev_desc *dev,
+					   struct i3c_ibi_slot *slot)
+{
+	struct dw_i3c_i2c_dev_data *data = i3c_dev_get_master_data(dev);
+
+	i3c_generic_ibi_recycle_slot(data->ibi_pool, slot);
+}
+
+static void dw_i3c_master_drain_ibi_queue(struct dw_i3c_master *master,
+					  int len)
+{
+	int i;
+
+	for (i = 0; i < DIV_ROUND_UP(len, 4); i++)
+		readl(master->regs + IBI_QUEUE_STATUS);
+}
+
+static void dw_i3c_master_handle_ibi_sir(struct dw_i3c_master *master,
+					 u32 status)
+{
+	struct dw_i3c_i2c_dev_data *data;
+	struct i3c_ibi_slot *slot;
+	struct i3c_dev_desc *dev;
+	unsigned long flags;
+	u8 addr, len;
+	int idx;
+
+	addr = IBI_QUEUE_IBI_ADDR(status);
+	len = IBI_QUEUE_STATUS_DATA_LEN(status);
+
+	spin_lock_irqsave(&master->devs_lock, flags);
+	idx = dw_i3c_master_get_addr_pos(master, addr);
+	if (idx < 0) {
+		dev_dbg_ratelimited(&master->base.dev,
+			 "IBI from unknown addr 0x%x\n", addr);
+		goto err_drain;
+	}
+
+	dev = master->devs[idx].ibi_dev;
+	if (!dev || !dev->ibi) {
+		dev_dbg_ratelimited(&master->base.dev,
+			 "IBI from non-requested dev idx %d\n", idx);
+		goto err_drain;
+	}
+
+	data = i3c_dev_get_master_data(dev);
+	slot = i3c_generic_ibi_get_free_slot(data->ibi_pool);
+	if (!slot) {
+		dev_dbg_ratelimited(&master->base.dev,
+				    "No IBI slots available\n");
+		goto err_drain;
+	}
+
+	if (dev->ibi->max_payload_len < len) {
+		dev_dbg_ratelimited(&master->base.dev,
+				    "IBI payload len %d greater than max %d\n",
+				    len, dev->ibi->max_payload_len);
+		goto err_drain;
+	}
+
+	if (len) {
+		dw_i3c_master_read_ibi_fifo(master, slot->data, len);
+		slot->len = len;
+	}
+	i3c_master_queue_ibi(dev, slot);
+
+	spin_unlock_irqrestore(&master->devs_lock, flags);
+
+	return;
+
+err_drain:
+	dw_i3c_master_drain_ibi_queue(master, len);
+
+	spin_unlock_irqrestore(&master->devs_lock, flags);
+}
+
+/* "ibis": referring to In-Band Interrupts, and not
+ * https://en.wikipedia.org/wiki/Australian_white_ibis. The latter should
+ * not be handled.
+ */
+static void dw_i3c_master_irq_handle_ibis(struct dw_i3c_master *master)
+{
+	unsigned int i, len, n_ibis;
+	u32 reg;
+
+	reg = readl(master->regs + QUEUE_STATUS_LEVEL);
+	n_ibis = QUEUE_STATUS_IBI_STATUS_CNT(reg);
+	if (!n_ibis)
+		return;
+
+	for (i = 0; i < n_ibis; i++) {
+		reg = readl(master->regs + IBI_QUEUE_STATUS);
+
+		if (IBI_TYPE_SIRQ(reg)) {
+			dw_i3c_master_handle_ibi_sir(master, reg);
+		} else {
+			len = IBI_QUEUE_STATUS_DATA_LEN(reg);
+			dev_info(&master->base.dev,
+				 "unsupported IBI type 0x%lx len %d\n",
+				 IBI_QUEUE_STATUS_IBI_ID(reg), len);
+			dw_i3c_master_drain_ibi_queue(master, len);
+		}
+	}
+}
+
 static irqreturn_t dw_i3c_master_irq_handler(int irq, void *dev_id)
 {
 	struct dw_i3c_master *master = dev_id;
@@ -1092,6 +1340,9 @@ static irqreturn_t dw_i3c_master_irq_handler(int irq, void *dev_id)
 		writel(INTR_TRANSFER_ERR_STAT, master->regs + INTR_STATUS);
 	spin_unlock(&master->xferqueue.lock);
 
+	if (status & INTR_IBI_THLD_STAT)
+		dw_i3c_master_irq_handle_ibis(master);
+
 	return IRQ_HANDLED;
 }
 
@@ -1110,6 +1361,26 @@ static const struct i3c_master_controller_ops dw_mipi_i3c_ops = {
 	.i2c_xfers = dw_i3c_master_i2c_xfers,
 };
 
+static const struct i3c_master_controller_ops dw_mipi_i3c_ibi_ops = {
+	.bus_init = dw_i3c_master_bus_init,
+	.bus_cleanup = dw_i3c_master_bus_cleanup,
+	.attach_i3c_dev = dw_i3c_master_attach_i3c_dev,
+	.reattach_i3c_dev = dw_i3c_master_reattach_i3c_dev,
+	.detach_i3c_dev = dw_i3c_master_detach_i3c_dev,
+	.do_daa = dw_i3c_master_daa,
+	.supports_ccc_cmd = dw_i3c_master_supports_ccc_cmd,
+	.send_ccc_cmd = dw_i3c_master_send_ccc_cmd,
+	.priv_xfers = dw_i3c_master_priv_xfers,
+	.attach_i2c_dev = dw_i3c_master_attach_i2c_dev,
+	.detach_i2c_dev = dw_i3c_master_detach_i2c_dev,
+	.i2c_xfers = dw_i3c_master_i2c_xfers,
+	.request_ibi = dw_i3c_master_request_ibi,
+	.free_ibi = dw_i3c_master_free_ibi,
+	.enable_ibi = dw_i3c_master_enable_ibi,
+	.disable_ibi = dw_i3c_master_disable_ibi,
+	.recycle_ibi_slot = dw_i3c_master_recycle_ibi_slot,
+};
+
 /* default platform ops implementations */
 static int dw_i3c_platform_init_nop(struct dw_i3c_master *i3c)
 {
@@ -1123,6 +1394,7 @@ static const struct dw_i3c_platform_ops dw_i3c_platform_ops_default = {
 int dw_i3c_common_probe(struct dw_i3c_master *master,
 			struct platform_device *pdev)
 {
+	const struct i3c_master_controller_ops *ops;
 	int ret, irq;
 
 	if (!master->platform_ops)
@@ -1172,8 +1444,11 @@ int dw_i3c_common_probe(struct dw_i3c_master *master,
 	master->maxdevs = ret >> 16;
 	master->free_pos = GENMASK(master->maxdevs - 1, 0);
 
-	ret = i3c_master_register(&master->base, &pdev->dev,
-				  &dw_mipi_i3c_ops, false);
+	ops = &dw_mipi_i3c_ops;
+	if (master->ibi_capable)
+		ops = &dw_mipi_i3c_ibi_ops;
+
+	ret = i3c_master_register(&master->base, &pdev->dev, ops, false);
 	if (ret)
 		goto err_assert_rst;
 
diff --git a/drivers/i3c/master/dw-i3c-master.h b/drivers/i3c/master/dw-i3c-master.h
index dd0b77e2c66a..0096942649a3 100644
--- a/drivers/i3c/master/dw-i3c-master.h
+++ b/drivers/i3c/master/dw-i3c-master.h
@@ -19,6 +19,7 @@ struct dw_i3c_master_caps {
 
 struct dw_i3c_dat_entry {
 	u8 addr;
+	struct i3c_dev_desc *ibi_dev;
 };
 
 struct dw_i3c_master {
@@ -37,12 +38,22 @@ struct dw_i3c_master {
 	struct clk *core_clk;
 	char version[5];
 	char type[5];
+	bool ibi_capable;
 
 	/*
 	 * Per-device hardware data, used to manage the device address table
 	 * (DAT)
+	 *
+	 * Locking: the devs array may be referenced in IRQ context while
+	 * processing an IBI. However, IBIs (for a specific device, which
+	 * implies a specific DAT entry) can only happen while interrupts are
+	 * requested for that device, which is serialised against other
+	 * insertions/removals from the array by the global i3c infrastructure.
+	 * So, devs_lock protects against concurrent updates to devs->ibi_dev
+	 * between request_ibi/free_ibi and the IBI irq event.
 	 */
 	struct dw_i3c_dat_entry devs[DW_I3C_MAX_DEVS];
+	spinlock_t devs_lock;
 
 	/* platform-specific data */
 	const struct dw_i3c_platform_ops *platform_ops;
-- 
2.39.2


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH 4/5] i3c: dw: Add a platform facility for IBI PEC workarounds

[Jeremy Kerr]

On the AST2600 i3c controller, we'll need to apply a workaround for a
hardware issue with IBI payloads.

Introduce a platform hook to allow dw i3c platform implementations to
modify the DAT entry in IBI enable/disable to allow this workaround in a
future change.

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
 drivers/i3c/master/dw-i3c-master.c | 19 +++++++++++++++++++
 drivers/i3c/master/dw-i3c-master.h | 10 ++++++++++
 2 files changed, 29 insertions(+)

diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index c49c5fa01a26..df8bf985abda 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -1155,6 +1155,7 @@ static void dw_i3c_master_set_sir_enabled(struct dw_i3c_master *master,
 	} else {
 		reg |= DEV_ADDR_TABLE_SIR_REJECT;
 	}
+	master->platform_ops->set_dat_ibi(master, dev, enable, &reg);
 	writel(reg, master->regs + dat_entry);
 
 	reg = readl(master->regs + IBI_SIR_REQ_REJECT);
@@ -1247,6 +1248,17 @@ static void dw_i3c_master_handle_ibi_sir(struct dw_i3c_master *master,
 	addr = IBI_QUEUE_IBI_ADDR(status);
 	len = IBI_QUEUE_STATUS_DATA_LEN(status);
 
+	/*
+	 * We be tempted to check the error status in bit 30; however, due
+	 * to the PEC errata workaround on some platform implementations (see
+	 * ast2600_i3c_set_dat_ibi()), those will almost always have a PEC
+	 * error on IBI payload data, as well as losing the last byte of
+	 * payload.
+	 *
+	 * If we implement error status checking on that bit, we may need
+	 * a new platform op to validate it.
+	 */
+
 	spin_lock_irqsave(&master->devs_lock, flags);
 	idx = dw_i3c_master_get_addr_pos(master, addr);
 	if (idx < 0) {
@@ -1387,8 +1399,15 @@ static int dw_i3c_platform_init_nop(struct dw_i3c_master *i3c)
 	return 0;
 }
 
+static void dw_i3c_platform_set_dat_ibi_nop(struct dw_i3c_master *i3c,
+					struct i3c_dev_desc *dev,
+					bool enable, u32 *dat)
+{
+}
+
 static const struct dw_i3c_platform_ops dw_i3c_platform_ops_default = {
 	.init = dw_i3c_platform_init_nop,
+	.set_dat_ibi = dw_i3c_platform_set_dat_ibi_nop,
 };
 
 int dw_i3c_common_probe(struct dw_i3c_master *master,
diff --git a/drivers/i3c/master/dw-i3c-master.h b/drivers/i3c/master/dw-i3c-master.h
index 0096942649a3..379300b7c405 100644
--- a/drivers/i3c/master/dw-i3c-master.h
+++ b/drivers/i3c/master/dw-i3c-master.h
@@ -67,6 +67,16 @@ struct dw_i3c_platform_ops {
 	 * perform actual device enabling with the i3c core ready.
 	 */
 	int (*init)(struct dw_i3c_master *i3c);
+
+	/*
+	 * Initialise a DAT entry to enable/disable IBIs. Allows the platform
+	 * to perform any device workarounds on the DAT entry before
+	 * inserting into the hardware table.
+	 *
+	 * Called with the DAT lock held; must not sleep.
+	 */
+	void (*set_dat_ibi)(struct dw_i3c_master *i3c,
+			    struct i3c_dev_desc *dev, bool enable, u32 *reg);
 };
 
 extern int dw_i3c_common_probe(struct dw_i3c_master *master,
-- 
2.39.2


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH 5/5] i3c: ast2600: enable IBI support

[Jeremy Kerr]

The ast2600 i3c hardware is capable of IBIs, but we need a workaround
for a hardware issue with the I3C state machine handling IBI payloads
of specific lengths when PEC is not enabled. To avoid this, we need to
unconditionally enable PECs, at the consquence of losing a byte of data
when the device does not send a PEC.

Enable IBIs on the ast2600 platform, including an implementation of the
PEC workaround, which prints a warning when triggered.

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
 drivers/i3c/master/ast2600-i3c-master.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/i3c/master/ast2600-i3c-master.c b/drivers/i3c/master/ast2600-i3c-master.c
index d3d7b7d19f22..52d9474a7651 100644
--- a/drivers/i3c/master/ast2600-i3c-master.c
+++ b/drivers/i3c/master/ast2600-i3c-master.c
@@ -36,6 +36,8 @@
 
 #define AST2600_DEFAULT_SDA_PULLUP_OHMS		2000
 
+#define DEV_ADDR_TABLE_IBI_PEC			BIT(11)
+
 struct ast2600_i3c_platform_data {
 	struct regmap *global_regs;
 	unsigned int global_idx;
@@ -93,8 +95,26 @@ static int ast2600_i3c_init(struct dw_i3c_master *master)
 	return rc;
 }
 
+static void ast2600_i3c_set_dat_ibi(struct dw_i3c_master *i3c,
+				    struct i3c_dev_desc *dev,
+				    bool enable, u32 *dat)
+{
+	/*
+	 * The ast2600 i3c controller will lock up on receiving 4n+1-byte IBIs
+	 * if the PEC is disabled. We have no way to restrict the length of
+	 * IBIs sent to the controller, so we need to unconditionally enable
+	 * PEC checking, which means we drop a byte of payload data
+	 */
+	if (enable && dev->info.bcr & I3C_BCR_IBI_PAYLOAD) {
+		dev_warn_once(&i3c->base.dev,
+		      "Enabling PEC workaround. IBI payloads will be truncated\n");
+		*dat |= DEV_ADDR_TABLE_IBI_PEC;
+	}
+}
+
 const struct dw_i3c_platform_ops ast2600_i3c_ops = {
 	.init = ast2600_i3c_init,
+	.set_dat_ibi = ast2600_i3c_set_dat_ibi,
 };
 
 static int ast2600_i3c_probe(struct platform_device *pdev)
@@ -136,6 +156,7 @@ static int ast2600_i3c_probe(struct platform_device *pdev)
 
 	dw_i3c->platform_ops = &ast2600_i3c_ops;
 	dw_i3c->platform_data = pdata;
+	dw_i3c->ibi_capable = true;
 	rc = dw_i3c_common_probe(dw_i3c, pdev);
 
 	return rc;
-- 
2.39.2


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 0/5] i3c: dw,ast2600: Add In-Band Interrupt support

[Joel Stanley]

On Thu, 30 Mar 2023 at 07:50, Jeremy Kerr <jk@codeconstruct.com.au> wrote:
>
> This series implements In-Band Interrupt (IBI) support for the ast2600
> i3c driver, by adding the required generic code in the dw driver, then
> enabling that (with a required hardware workaround) in the ast2600
> driver.
>
> As always: comments, queries etc. are most welcome.

I am not fluent in i3c but the changes look okay to me.

Reviewed-by: Joel Stanley <joel@jms.id.au>

Cheers,

Joel

>
> Cheers,
>
>
> Jeremy
>
> Jeremy Kerr (5):
>   i3c: dw: Create a generic fifo read function
>   i3c: dw: Turn DAT array entry into a struct
>   i3c: dw: Add support for in-band interrupts
>   i3c: dw: Add a platform facility for IBI PEC workarounds
>   i3c: ast2600: enable IBI support
>
>  drivers/i3c/master/ast2600-i3c-master.c |  21 ++
>  drivers/i3c/master/dw-i3c-master.c      | 336 ++++++++++++++++++++++--
>  drivers/i3c/master/dw-i3c-master.h      |  32 ++-
>  3 files changed, 370 insertions(+), 19 deletions(-)
>
> --
> 2.39.2
>

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 0/5] i3c: dw,ast2600: Add In-Band Interrupt support

[Alexandre Belloni]

I'm sorry about the late reply, I've been super busy this month. I'm
happy to take this series but it doesn't apply on top of v3 of your
other series. I guess this will have to wait for the next release.

On 30/03/2023 15:50:31+0800, Jeremy Kerr wrote:
> This series implements In-Band Interrupt (IBI) support for the ast2600
> i3c driver, by adding the required generic code in the dw driver, then
> enabling that (with a required hardware workaround) in the ast2600
> driver.
> 
> As always: comments, queries etc. are most welcome.
> 
> Cheers,
> 
> 
> Jeremy
> 
> Jeremy Kerr (5):
>   i3c: dw: Create a generic fifo read function
>   i3c: dw: Turn DAT array entry into a struct
>   i3c: dw: Add support for in-band interrupts
>   i3c: dw: Add a platform facility for IBI PEC workarounds
>   i3c: ast2600: enable IBI support
> 
>  drivers/i3c/master/ast2600-i3c-master.c |  21 ++
>  drivers/i3c/master/dw-i3c-master.c      | 336 ++++++++++++++++++++++--
>  drivers/i3c/master/dw-i3c-master.h      |  32 ++-
>  3 files changed, 370 insertions(+), 19 deletions(-)
> 
> -- 
> 2.39.2
> 
> 
> -- 
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 0/5] i3c: dw,ast2600: Add In-Band Interrupt support

[Jeremy Kerr]

Hi Aledandre,

> I'm sorry about the late reply, I've been super busy this month. I'm
> happy to take this series but it doesn't apply on top of v3 of your
> other series. I guess this will have to wait for the next release.

That was originally based on v3 - I think you've applied v2 of the
ast2600 series. 

In:

 https://git.kernel.org/pub/scm/linux/kernel/git/i3c/linux.git/commit/?id=7dcbb33d3b2b8bd903d3157fc7249a4cb4f6820d

your linux-i3c tree has the v2 dw struct, with:

+	/* platform-specific data */
+	const struct dw_i3c_platform_ops *platform_ops;
+	void *platform_data;
+};

(platform_data was removed in v3).

The Link: tag is correct though.

Cheers,


Jeremy


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 0/5] i3c: dw,ast2600: Add In-Band Interrupt support

[Alexandre Belloni]

On 28/04/2023 08:01:39+0800, Jeremy Kerr wrote:
> Hi Aledandre,
> 
> > I'm sorry about the late reply, I've been super busy this month. I'm
> > happy to take this series but it doesn't apply on top of v3 of your
> > other series. I guess this will have to wait for the next release.
> 
> That was originally based on v3 - I think you've applied v2 of the
> ast2600 series. 
> 
> In:
> 
>  https://git.kernel.org/pub/scm/linux/kernel/git/i3c/linux.git/commit/?id=7dcbb33d3b2b8bd903d3157fc7249a4cb4f6820d
> 
> your linux-i3c tree has the v2 dw struct, with:
> 
> +	/* platform-specific data */
> +	const struct dw_i3c_platform_ops *platform_ops;
> +	void *platform_data;
> +};
> 
> (platform_data was removed in v3).
> 
> The Link: tag is correct though.

Indeed, I fixed that but this wasn't the issue. Patch 5/5 doesn't apply.
If you look at the context, it has "struct ast2600_i3c_platform_data {"
which is "struct ast2600_i3c {" in your v3. Also the chunk in
ast2600_i3c_probe() can't apply.

I fixed it up and pushed, please check.


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 0/5] i3c: dw,ast2600: Add In-Band Interrupt support

[Jeremy Kerr]

Hi Alexandre,

> Indeed, I fixed that but this wasn't the issue. Patch 5/5 doesn't
> apply. If you look at the context, it has "struct
> ast2600_i3c_platform_data {" which is "struct ast2600_i3c {" in your
> v3. Also the chunk in ast2600_i3c_probe() can't apply.

Ah yep, looks like I'd got out of sync with the series for 5/5.

> I fixed it up and pushed, please check.

Looks good to me, thanks for sorting that out. I'll do some testing here
in the meantime too.

Cheers,


Jeremy

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c