[PATCH 1/5] i2c: imx-lpi2c: add target mode support

[PATCH 1/5] i2c: imx-lpi2c: add target mode support

[carlos.song]

From: Carlos Song <carlos.song@nxp.com>

Add target mode support for LPI2C.

Signed-off-by: Carlos Song <carlos.song@nxp.com>
---
 drivers/i2c/busses/i2c-imx-lpi2c.c | 253 ++++++++++++++++++++++++++++-
 1 file changed, 249 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
index 530ca5d76403..c8f3dadfb36e 100644
--- a/drivers/i2c/busses/i2c-imx-lpi2c.c
+++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
@@ -43,6 +43,20 @@
 #define LPI2C_MTDR	0x60	/* i2c master TX data register */
 #define LPI2C_MRDR	0x70	/* i2c master RX data register */
 
+#define LPI2C_SCR	0x110	/* i2c target contrl register */
+#define LPI2C_SSR	0x114	/* i2c target status register */
+#define LPI2C_SIER	0x118	/* i2c target interrupt enable */
+#define LPI2C_SDER	0x11C	/* i2c target DMA enable */
+#define LPI2C_SCFGR0	0x120	/* i2c target configuration */
+#define LPI2C_SCFGR1	0x124	/* i2c target configuration */
+#define LPI2C_SCFGR2	0x128	/* i2c target configuration */
+#define LPI2C_SAMR	0x140	/* i2c target address match */
+#define LPI2C_SASR	0x150	/* i2c target address status */
+#define LPI2C_STAR	0x154	/* i2c target transmit ACK */
+#define LPI2C_STDR	0x160	/* i2c target transmit data */
+#define LPI2C_SRDR	0x170	/* i2c target receive data */
+#define LPI2C_SRDROR	0x178	/* i2c target receive data read only */
+
 /* i2c command */
 #define TRAN_DATA	0X00
 #define RECV_DATA	0X01
@@ -76,6 +90,42 @@
 #define MDER_TDDE	BIT(0)
 #define MDER_RDDE	BIT(1)
 
+#define SCR_SEN		BIT(0)
+#define SCR_RST		BIT(1)
+#define SCR_FILTEN	BIT(4)
+#define SCR_RTF		BIT(8)
+#define SCR_RRF		BIT(9)
+#define SCFGR1_RXSTALL	BIT(1)
+#define SCFGR1_TXDSTALL	BIT(2)
+#define SCFGR2_FILTSDA_SHIFT	24
+#define SCFGR2_FILTSCL_SHIFT	16
+#define SCFGR2_CLKHOLD(x)	(x)
+#define SCFGR2_FILTSDA(x)	((x) << SCFGR2_FILTSDA_SHIFT)
+#define SCFGR2_FILTSCL(x)	((x) << SCFGR2_FILTSCL_SHIFT)
+#define SSR_TDF		BIT(0)
+#define SSR_RDF		BIT(1)
+#define SSR_AVF		BIT(2)
+#define SSR_TAF		BIT(3)
+#define SSR_RSF		BIT(8)
+#define SSR_SDF		BIT(9)
+#define SSR_BEF		BIT(10)
+#define SSR_FEF		BIT(11)
+#define SSR_SBF		BIT(24)
+#define SSR_BBF		BIT(25)
+#define SSR_CLEAR_BITS	(SSR_RSF | SSR_SDF | SSR_BEF | SSR_FEF)
+#define SIER_TDIE	BIT(0)
+#define SIER_RDIE	BIT(1)
+#define SIER_AVIE	BIT(2)
+#define SIER_TAIE	BIT(3)
+#define SIER_RSIE	BIT(8)
+#define SIER_SDIE	BIT(9)
+#define SIER_BEIE	BIT(10)
+#define SIER_FEIE	BIT(11)
+#define SIER_AM0F	BIT(12)
+#define SASR_READ_REQ	0x1
+#define SLAVE_INT_FLAG	(SIER_TDIE | SIER_RDIE | SIER_AVIE | \
+						SIER_SDIE | SIER_BEIE)
+
 #define I2C_CLK_RATIO	2
 #define CHUNK_DATA	256
 
@@ -133,6 +183,7 @@ struct lpi2c_imx_struct {
 	struct i2c_bus_recovery_info rinfo;
 	bool			can_use_dma;
 	struct lpi2c_imx_dma	*dma;
+	struct i2c_client	*target;
 };
 
 static void lpi2c_imx_intctrl(struct lpi2c_imx_struct *lpi2c_imx,
@@ -952,9 +1003,57 @@ static int lpi2c_imx_xfer(struct i2c_adapter *adapter,
 	return (result < 0) ? result : num;
 }
 
-static irqreturn_t lpi2c_imx_isr(int irq, void *dev_id)
+static irqreturn_t lpi2c_imx_target_isr(struct lpi2c_imx_struct *lpi2c_imx,
+					   u32 ssr, u32 sier_filter)
+{
+	u8 value;
+	u32 sasr;
+
+	/* Arbitration lost */
+	if (sier_filter & SSR_BEF) {
+		writel(0, lpi2c_imx->base + LPI2C_SIER);
+		return IRQ_HANDLED;
+	}
+
+	/* Address detected */
+	if (sier_filter & SSR_AVF) {
+		sasr = readl(lpi2c_imx->base + LPI2C_SASR);
+		if (SASR_READ_REQ & sasr) {
+			/* Read request */
+			i2c_slave_event(lpi2c_imx->target, I2C_SLAVE_READ_REQUESTED, &value);
+			writel(value, lpi2c_imx->base + LPI2C_STDR);
+			goto ret;
+		} else {
+			/* Write request */
+			i2c_slave_event(lpi2c_imx->target, I2C_SLAVE_WRITE_REQUESTED, &value);
+		}
+	}
+
+	if (sier_filter & SSR_SDF) {
+		/* STOP */
+		i2c_slave_event(lpi2c_imx->target, I2C_SLAVE_STOP, &value);
+	}
+
+	if (sier_filter & SSR_TDF) {
+		/* Target send data */
+		i2c_slave_event(lpi2c_imx->target, I2C_SLAVE_READ_PROCESSED, &value);
+		writel(value, lpi2c_imx->base + LPI2C_STDR);
+	}
+
+	if (sier_filter & SSR_RDF) {
+		/* Target receive data */
+		value = readl(lpi2c_imx->base + LPI2C_SRDR);
+		i2c_slave_event(lpi2c_imx->target, I2C_SLAVE_WRITE_RECEIVED, &value);
+	}
+
+ret:
+	/* Clear SSR */
+	writel(ssr & SSR_CLEAR_BITS, lpi2c_imx->base + LPI2C_SSR);
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t lpi2c_imx_master_isr(struct lpi2c_imx_struct *lpi2c_imx)
 {
-	struct lpi2c_imx_struct *lpi2c_imx = dev_id;
 	unsigned int enabled;
 	unsigned int temp;
 
@@ -974,6 +1073,119 @@ static irqreturn_t lpi2c_imx_isr(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static irqreturn_t lpi2c_imx_isr(int irq, void *dev_id)
+{
+	struct lpi2c_imx_struct *lpi2c_imx = dev_id;
+	u32 ssr, sier_filter;
+	unsigned int scr;
+
+	if (lpi2c_imx->target) {
+		scr = readl(lpi2c_imx->base + LPI2C_SCR);
+		ssr = readl(lpi2c_imx->base + LPI2C_SSR);
+		sier_filter = ssr & readl(lpi2c_imx->base + LPI2C_SIER);
+		if ((scr & SCR_SEN) && sier_filter)
+			return lpi2c_imx_target_isr(lpi2c_imx, ssr, sier_filter);
+		else
+			return lpi2c_imx_master_isr(lpi2c_imx);
+	} else {
+		return lpi2c_imx_master_isr(lpi2c_imx);
+	}
+}
+
+static void lpi2c_imx_target_init(struct lpi2c_imx_struct *lpi2c_imx)
+{
+	int temp;
+
+	/* reset target module */
+	writel(SCR_RST, lpi2c_imx->base + LPI2C_SCR);
+	writel(0, lpi2c_imx->base + LPI2C_SCR);
+
+	/* Set target addr */
+	writel((lpi2c_imx->target->addr << 1), lpi2c_imx->base + LPI2C_SAMR);
+
+	writel(SCFGR1_RXSTALL | SCFGR1_TXDSTALL, lpi2c_imx->base + LPI2C_SCFGR1);
+
+	/*
+	 * set SCFGR2: FILTSDA, FILTSCL and CLKHOLD
+	 *
+	 * FILTSCL/FILTSDA can eliminate signal skew. It should generally be
+	 * set to the same value and should be set >= 50ns.
+	 *
+	 * CLKHOLD is only used when clock stretching is enabled, but it will
+	 * extend the clock stretching to ensure there is an additional delay
+	 * between the target driving SDA and the target releasing the SCL pin.
+	 *
+	 * CLKHOLD setting is crucial for lpi2c target. When master read data
+	 * from target, if there is a delay caused by cpu idle, excessive load,
+	 * or other delays between two bytes in one message transmission. so it
+	 * will cause a short interval time between the driving SDA signal and
+	 * releasing SCL signal. Lpi2c master will mistakenly think it is a stop
+	 * signal resulting in an arbitration failure. This issue can be avoided
+	 * by setting CLKHOLD.
+	 *
+	 * In order to ensure lpi2c function normally when the lpi2c speed is as
+	 * low as 100kHz, CLKHOLD should be set 3 and it is also compatible with
+	 * higher clock frequency like 400kHz and 1MHz.
+	 */
+	temp = SCFGR2_FILTSDA(2) | SCFGR2_FILTSCL(2) | SCFGR2_CLKHOLD(3);
+	writel(temp, lpi2c_imx->base + LPI2C_SCFGR2);
+
+	/*
+	 * Enable module:
+	 * SCR_FILTEN can enable digital filter and output delay counter for LPI2C
+	 * target mode. So SCR_FILTEN need be asserted when enable SDA/SCL FILTER
+	 * and CLKHOLD.
+	 */
+	writel(SCR_SEN | SCR_FILTEN, lpi2c_imx->base + LPI2C_SCR);
+
+	/* Enable interrupt from i2c module */
+	writel(SLAVE_INT_FLAG, lpi2c_imx->base + LPI2C_SIER);
+}
+
+static int lpi2c_imx_reg_target(struct i2c_client *client)
+{
+	struct lpi2c_imx_struct *lpi2c_imx = i2c_get_adapdata(client->adapter);
+	int ret;
+
+	if (lpi2c_imx->target)
+		return -EBUSY;
+
+	lpi2c_imx->target = client;
+
+	ret = pm_runtime_resume_and_get(lpi2c_imx->adapter.dev.parent);
+	if (ret < 0) {
+		dev_err(&lpi2c_imx->adapter.dev, "failed to resume i2c controller");
+		return ret;
+	}
+
+	lpi2c_imx_target_init(lpi2c_imx);
+
+	return 0;
+}
+
+static int lpi2c_imx_unreg_target(struct i2c_client *client)
+{
+	struct lpi2c_imx_struct *lpi2c_imx = i2c_get_adapdata(client->adapter);
+	int ret;
+
+	if (!lpi2c_imx->target)
+		return -EINVAL;
+
+	/* Reset target address. */
+	writel(0, lpi2c_imx->base + LPI2C_SAMR);
+
+	writel(SCR_RST, lpi2c_imx->base + LPI2C_SCR);
+	writel(0, lpi2c_imx->base + LPI2C_SCR);
+
+	lpi2c_imx->target = NULL;
+
+	ret = pm_runtime_put_sync(lpi2c_imx->adapter.dev.parent);
+	if (ret < 0)
+		dev_err(&lpi2c_imx->adapter.dev, "failed to suspend i2c controller");
+
+	return ret;
+}
+
 static int lpi2c_imx_init_recovery_info(struct lpi2c_imx_struct *lpi2c_imx,
 				  struct platform_device *pdev)
 {
@@ -1049,6 +1261,8 @@ static u32 lpi2c_imx_func(struct i2c_adapter *adapter)
 static const struct i2c_algorithm lpi2c_imx_algo = {
 	.master_xfer	= lpi2c_imx_xfer,
 	.functionality	= lpi2c_imx_func,
+	.reg_slave		= lpi2c_imx_reg_target,
+	.unreg_slave	= lpi2c_imx_unreg_target,
 };
 
 static const struct of_device_id lpi2c_imx_of_match[] = {
@@ -1199,9 +1413,40 @@ static int __maybe_unused lpi2c_runtime_resume(struct device *dev)
 	return 0;
 }
 
+static int lpi2c_suspend_noirq(struct device *dev)
+{
+	struct lpi2c_imx_struct *lpi2c_imx = dev_get_drvdata(dev);
+	int ret;
+
+	ret = pm_runtime_force_suspend(dev);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int lpi2c_resume_noirq(struct device *dev)
+{
+	struct lpi2c_imx_struct *lpi2c_imx = dev_get_drvdata(dev);
+	int ret;
+
+	ret = pm_runtime_force_resume(dev);
+	if (ret)
+		return ret;
+
+	/*
+	 * If i2c module powered down in system suspend, register
+	 * value will lose. So reinit target when system resume.
+	 */
+	if (lpi2c_imx->target)
+		lpi2c_imx_target_init(lpi2c_imx);
+
+	return 0;
+}
+
 static const struct dev_pm_ops lpi2c_pm_ops = {
-	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
-				      pm_runtime_force_resume)
+	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(lpi2c_suspend_noirq,
+				      lpi2c_resume_noirq)
 	SET_RUNTIME_PM_OPS(lpi2c_runtime_suspend,
 			   lpi2c_runtime_resume, NULL)
 };
-- 
2.34.1

[PATCH 2/5] i2c: imx-lpi2c: add IRQF_NO_SUSPEND IRQ flag

[carlos.song]

From: Carlos Song <carlos.song@nxp.com>

Set IRQF_NO_SUSPEND flag when request_irq(). Some devices such
as extend GPIO will need i2c transfer during the entire system
suspend and resume period so keep it enabled.

Signed-off-by: Carlos Song <carlos.song@nxp.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/i2c/busses/i2c-imx-lpi2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
index c8f3dadfb36e..23f83f10d5f6 100644
--- a/drivers/i2c/busses/i2c-imx-lpi2c.c
+++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
@@ -1309,7 +1309,7 @@ static int lpi2c_imx_probe(struct platform_device *pdev)
 	if (ret)
 		lpi2c_imx->bitrate = I2C_MAX_STANDARD_MODE_FREQ;
 
-	ret = devm_request_irq(&pdev->dev, irq, lpi2c_imx_isr, 0,
+	ret = devm_request_irq(&pdev->dev, irq, lpi2c_imx_isr, IRQF_NO_SUSPEND,
 			       pdev->name, lpi2c_imx);
 	if (ret)
 		return dev_err_probe(&pdev->dev, ret, "can't claim irq %d\n", irq);
-- 
2.34.1

[PATCH 3/5] i2c: imx-lpi2c: manage IRQ request/release in runtime pm

[carlos.song]

From: Carlos Song <carlos.song@nxp.com>

Request the parent interrupt during runtime resume and release the
parent interrupt dunring runtime suspend for allowing the parent
interrupt controller to enter runtime suspend if there are no active
users.

No visible impact if the parent controller is the GIC, but it has
significant power savings for parent IRQ controllers like IRQSteer
inside a subsystem on i.MX8 SoCs. Releasing the parent IRQ provides
an opportunity for the subsystem to enter suspend states if there are
no active users.

Signed-off-by: Carlos Song <carlos.song@nxp.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/i2c/busses/i2c-imx-lpi2c.c | 40 ++++++++++++++++++++++++------
 1 file changed, 33 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
index 23f83f10d5f6..0159ade235ef 100644
--- a/drivers/i2c/busses/i2c-imx-lpi2c.c
+++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
@@ -168,6 +168,7 @@ struct lpi2c_imx_struct {
 	struct i2c_adapter	adapter;
 	int			num_clks;
 	struct clk_bulk_data	*clks;
+	int			irq;
 	void __iomem		*base;
 	__u8			*rx_buf;
 	__u8			*tx_buf;
@@ -1252,6 +1253,27 @@ static int lpi2c_dma_init(struct device *dev, dma_addr_t phy_addr)
 	return ret;
 }
 
+static int lpi2c_manage_irq_handler(struct lpi2c_imx_struct *lpi2c_imx, bool enable)
+{
+	int ret;
+
+	if (enable) {
+		ret = devm_request_irq(lpi2c_imx->adapter.dev.parent, lpi2c_imx->irq,
+				   lpi2c_imx_isr, IRQF_NO_SUSPEND,
+				   dev_name(lpi2c_imx->adapter.dev.parent),
+				   lpi2c_imx);
+		if (ret) {
+			dev_err(lpi2c_imx->adapter.dev.parent, "can't claim irq %d\n",
+				   lpi2c_imx->irq);
+			return ret;
+		}
+	} else {
+		devm_free_irq(lpi2c_imx->adapter.dev.parent, lpi2c_imx->irq, lpi2c_imx);
+	}
+
+	return 0;
+}
+
 static u32 lpi2c_imx_func(struct i2c_adapter *adapter)
 {
 	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
@@ -1277,7 +1299,7 @@ static int lpi2c_imx_probe(struct platform_device *pdev)
 	struct resource *res;
 	dma_addr_t phy_addr;
 	unsigned int temp;
-	int irq, ret;
+	int ret;
 
 	lpi2c_imx = devm_kzalloc(&pdev->dev, sizeof(*lpi2c_imx), GFP_KERNEL);
 	if (!lpi2c_imx)
@@ -1287,9 +1309,9 @@ static int lpi2c_imx_probe(struct platform_device *pdev)
 	if (IS_ERR(lpi2c_imx->base))
 		return PTR_ERR(lpi2c_imx->base);
 
-	irq = platform_get_irq(pdev, 0);
-	if (irq < 0)
-		return irq;
+	lpi2c_imx->irq = platform_get_irq(pdev, 0);
+	if (lpi2c_imx->irq < 0)
+		return lpi2c_imx->irq;
 
 	lpi2c_imx->adapter.owner	= THIS_MODULE;
 	lpi2c_imx->adapter.algo		= &lpi2c_imx_algo;
@@ -1309,10 +1331,9 @@ static int lpi2c_imx_probe(struct platform_device *pdev)
 	if (ret)
 		lpi2c_imx->bitrate = I2C_MAX_STANDARD_MODE_FREQ;
 
-	ret = devm_request_irq(&pdev->dev, irq, lpi2c_imx_isr, IRQF_NO_SUSPEND,
-			       pdev->name, lpi2c_imx);
+	ret = lpi2c_manage_irq_handler(lpi2c_imx, true);
 	if (ret)
-		return dev_err_probe(&pdev->dev, ret, "can't claim irq %d\n", irq);
+		return ret;
 
 	i2c_set_adapdata(&lpi2c_imx->adapter, lpi2c_imx);
 	platform_set_drvdata(pdev, lpi2c_imx);
@@ -1392,6 +1413,7 @@ static int __maybe_unused lpi2c_runtime_suspend(struct device *dev)
 {
 	struct lpi2c_imx_struct *lpi2c_imx = dev_get_drvdata(dev);
 
+	lpi2c_manage_irq_handler(lpi2c_imx, false);
 	clk_bulk_disable(lpi2c_imx->num_clks, lpi2c_imx->clks);
 	pinctrl_pm_select_sleep_state(dev);
 
@@ -1410,6 +1432,10 @@ static int __maybe_unused lpi2c_runtime_resume(struct device *dev)
 		return ret;
 	}
 
+	ret = lpi2c_manage_irq_handler(lpi2c_imx, true);
+	if (ret)
+		return ret;
+
 	return 0;
 }
 
-- 
2.34.1

[PATCH 4/5] i2c: imx-lpi2c: improve i2c driver probe priority

[carlos.song]

From: Carlos Song <carlos.song@nxp.com>

Some i2c devices such as PMICs need i2c bus available early.
Use subsys_initcall to improve i2c driver probe priority.

Signed-off-by: Carlos Song <carlos.song@nxp.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/i2c/busses/i2c-imx-lpi2c.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
index 0159ade235ef..210d505db76d 100644
--- a/drivers/i2c/busses/i2c-imx-lpi2c.c
+++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
@@ -1487,7 +1487,17 @@ static struct platform_driver lpi2c_imx_driver = {
 	},
 };
 
-module_platform_driver(lpi2c_imx_driver);
+static int __init lpi2c_imx_init(void)
+{
+	return platform_driver_register(&lpi2c_imx_driver);
+}
+subsys_initcall(lpi2c_imx_init);
+
+static void __exit lpi2c_imx_exit(void)
+{
+	platform_driver_unregister(&lpi2c_imx_driver);
+}
+module_exit(lpi2c_imx_exit);
 
 MODULE_AUTHOR("Gao Pan <pandy.gao@nxp.com>");
 MODULE_DESCRIPTION("I2C adapter driver for LPI2C bus");
-- 
2.34.1

[PATCH 5/5] i2c: imx-lpi2c: increase PM timeout to avoid operate clk frequently

[carlos.song]

From: Clark Wang <xiaoning.wang@nxp.com>

Switching the clock frequently will affect the data transmission
efficiency, and prolong the timeout to reduce autosuspend times for
lpi2c.

Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
Signed-off-by: Carlos Song <carlos.song@nxp.com>
---
 drivers/i2c/busses/i2c-imx-lpi2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
index 210d505db76d..cc5e5d96aacd 100644
--- a/drivers/i2c/busses/i2c-imx-lpi2c.c
+++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
@@ -129,7 +129,7 @@
 #define I2C_CLK_RATIO	2
 #define CHUNK_DATA	256
 
-#define I2C_PM_TIMEOUT		10 /* ms */
+#define I2C_PM_TIMEOUT		1000 /* ms */
 #define I2C_DMA_THRESHOLD	8 /* bytes */
 
 enum lpi2c_imx_mode {
-- 
2.34.1

Re: [PATCH 4/5] i2c: imx-lpi2c: improve i2c driver probe priority

[Stefan Wahren]

Hi Carlos,

Am 29.08.24 um 11:37 schrieb carlos.song@nxp.com:
> From: Carlos Song <carlos.song@nxp.com>
>
> Some i2c devices such as PMICs need i2c bus available early.
> Use subsys_initcall to improve i2c driver probe priority.
thanks for providing this patch.

Please try to be more specific, which devices/platform has been effected
by this issue. It would be nice to provide to kind of
link/reference/discussion.

Best regards
>
> Signed-off-by: Carlos Song <carlos.song@nxp.com>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>   drivers/i2c/busses/i2c-imx-lpi2c.c | 12 +++++++++++-
>   1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
> index 0159ade235ef..210d505db76d 100644
> --- a/drivers/i2c/busses/i2c-imx-lpi2c.c
> +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> @@ -1487,7 +1487,17 @@ static struct platform_driver lpi2c_imx_driver = {
>   	},
>   };
>
> -module_platform_driver(lpi2c_imx_driver);
> +static int __init lpi2c_imx_init(void)
> +{
> +	return platform_driver_register(&lpi2c_imx_driver);
> +}
> +subsys_initcall(lpi2c_imx_init);
> +
> +static void __exit lpi2c_imx_exit(void)
> +{
> +	platform_driver_unregister(&lpi2c_imx_driver);
> +}
> +module_exit(lpi2c_imx_exit);
>
>   MODULE_AUTHOR("Gao Pan <pandy.gao@nxp.com>");
>   MODULE_DESCRIPTION("I2C adapter driver for LPI2C bus");

Re: [PATCH 2/5] i2c: imx-lpi2c: add IRQF_NO_SUSPEND IRQ flag

[Stefan Wahren]

Hi Carlos,

Am 29.08.24 um 11:37 schrieb carlos.song@nxp.com:
> From: Carlos Song <carlos.song@nxp.com>
>
> Set IRQF_NO_SUSPEND flag when request_irq(). Some devices such
> as extend GPIO will need i2c transfer during the entire system
> suspend and resume period so keep it enabled.
do you mean GPIO expander which are connected to I2C?

Do the mentioned expander have a dedicated IRQ line or does the host
needs to poll I2C?

Best regards

>
> Signed-off-by: Carlos Song <carlos.song@nxp.com>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>   drivers/i2c/busses/i2c-imx-lpi2c.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
> index c8f3dadfb36e..23f83f10d5f6 100644
> --- a/drivers/i2c/busses/i2c-imx-lpi2c.c
> +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> @@ -1309,7 +1309,7 @@ static int lpi2c_imx_probe(struct platform_device *pdev)
>   	if (ret)
>   		lpi2c_imx->bitrate = I2C_MAX_STANDARD_MODE_FREQ;
>
> -	ret = devm_request_irq(&pdev->dev, irq, lpi2c_imx_isr, 0,
> +	ret = devm_request_irq(&pdev->dev, irq, lpi2c_imx_isr, IRQF_NO_SUSPEND,
>   			       pdev->name, lpi2c_imx);
>   	if (ret)
>   		return dev_err_probe(&pdev->dev, ret, "can't claim irq %d\n", irq);

Re: [PATCH 5/5] i2c: imx-lpi2c: increase PM timeout to avoid operate clk frequently

[Alexander Stein]

Hi,

Am Donnerstag, 29. August 2024, 11:37:22 CEST schrieb carlos.song@nxp.com:
> From: Clark Wang <xiaoning.wang@nxp.com>
> 
> Switching the clock frequently will affect the data transmission
> efficiency, and prolong the timeout to reduce autosuspend times for
> lpi2c.

Efficiency as in throughput or total time per transfer? Do you have any
numbers?

Best regards,
Alexander

> Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
> Signed-off-by: Carlos Song <carlos.song@nxp.com>
> ---
>  drivers/i2c/busses/i2c-imx-lpi2c.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
> index 210d505db76d..cc5e5d96aacd 100644
> --- a/drivers/i2c/busses/i2c-imx-lpi2c.c
> +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> @@ -129,7 +129,7 @@
>  #define I2C_CLK_RATIO	2
>  #define CHUNK_DATA	256
>  
> -#define I2C_PM_TIMEOUT		10 /* ms */
> +#define I2C_PM_TIMEOUT		1000 /* ms */
>  #define I2C_DMA_THRESHOLD	8 /* bytes */
>  
>  enum lpi2c_imx_mode {
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

RE: [EXT] Re: [PATCH 4/5] i2c: imx-lpi2c: improve i2c driver probe priority

[Carlos Song]

> -----Original Message-----
> From: Stefan Wahren <wahrenst@gmx.net>
> Sent: Thursday, August 29, 2024 6:16 PM
> To: Carlos Song <carlos.song@nxp.com>; Aisheng Dong
> <aisheng.dong@nxp.com>; andi.shyti@kernel.org; shawnguo@kernel.org;
> s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com
> Cc: linux-i2c@vger.kernel.org; imx@lists.linux.dev;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: [EXT] Re: [PATCH 4/5] i2c: imx-lpi2c: improve i2c driver probe priority
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report this
> email' button
> 
> 
> Hi Carlos,
> 
> Am 29.08.24 um 11:37 schrieb carlos.song@nxp.com:
> > From: Carlos Song <carlos.song@nxp.com>
> >
> > Some i2c devices such as PMICs need i2c bus available early.
> > Use subsys_initcall to improve i2c driver probe priority.
> thanks for providing this patch.
> 
> Please try to be more specific, which devices/platform has been effected by this
> issue. It would be nice to provide to kind of link/reference/discussion.
>
Hi, Thanks you!

Some I2C peripherals, like PMICs for voltage and power supply adjustment, need to be probed early
in the boot process to configure the system. Additionally, some boards have switches ,where some device need to be
chosen by a GPIO expander manipulating the corresponding GPIO pad. So GPIO expander also needs to be probed early
before these switched devices' driver probing. To guarantee their correct function, this patch was introduced.

This patch has been in our local repository for a long time, so I don't find any detailed documentation about its purpose or the platforms it affects.
Sorry about it. Since I2C is an important basic bus, many devices rely on it, so probed early seems reasonable, then I send it out.

> Best regards
> >
> > Signed-off-by: Carlos Song <carlos.song@nxp.com>
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >   drivers/i2c/busses/i2c-imx-lpi2c.c | 12 +++++++++++-
> >   1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c
> > b/drivers/i2c/busses/i2c-imx-lpi2c.c
> > index 0159ade235ef..210d505db76d 100644
> > --- a/drivers/i2c/busses/i2c-imx-lpi2c.c
> > +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> > @@ -1487,7 +1487,17 @@ static struct platform_driver lpi2c_imx_driver = {
> >       },
> >   };
> >
> > -module_platform_driver(lpi2c_imx_driver);
> > +static int __init lpi2c_imx_init(void) {
> > +     return platform_driver_register(&lpi2c_imx_driver);
> > +}
> > +subsys_initcall(lpi2c_imx_init);
> > +
> > +static void __exit lpi2c_imx_exit(void) {
> > +     platform_driver_unregister(&lpi2c_imx_driver);
> > +}
> > +module_exit(lpi2c_imx_exit);
> >
> >   MODULE_AUTHOR("Gao Pan <pandy.gao@nxp.com>");
> >   MODULE_DESCRIPTION("I2C adapter driver for LPI2C bus");

RE: [EXT] Re: [PATCH 2/5] i2c: imx-lpi2c: add IRQF_NO_SUSPEND IRQ flag

[Carlos Song]

> -----Original Message-----
> From: Stefan Wahren <wahrenst@gmx.net>
> Sent: Thursday, August 29, 2024 6:53 PM
> To: Carlos Song <carlos.song@nxp.com>; Aisheng Dong
> <aisheng.dong@nxp.com>; andi.shyti@kernel.org; shawnguo@kernel.org;
> s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com
> Cc: linux-i2c@vger.kernel.org; imx@lists.linux.dev;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: [EXT] Re: [PATCH 2/5] i2c: imx-lpi2c: add IRQF_NO_SUSPEND IRQ flag
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report this
> email' button
> 
> 
> Hi Carlos,
> 
> Am 29.08.24 um 11:37 schrieb carlos.song@nxp.com:
> > From: Carlos Song <carlos.song@nxp.com>
> >
> > Set IRQF_NO_SUSPEND flag when request_irq(). Some devices such as
> > extend GPIO will need i2c transfer during the entire system suspend
> > and resume period so keep it enabled.
> do you mean GPIO expander which are connected to I2C?
> 
> Do the mentioned expander have a dedicated IRQ line or does the host needs to
> poll I2C?
> 
Hi,

Yes, we have both gpio expanders. But devices polling i2c may have a stronger need for this.
Like pmic, it may use i2c transfer at any time regardless of whether the system enters suspend or disables irq.
This flag is added to make the I2c irq won't be mask in the system suspend.

But I find that it is not enough if just add this flag. Because lpi2c clock is enabled at rpm function. If rpm is disabled,
the lpi2c still can not work. So I will add an extra patch to make lpi2c can work also at no irq stage. I will send this patch
with this in next new patch serials.

> Best regards
> 
> >
> > Signed-off-by: Carlos Song <carlos.song@nxp.com>
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >   drivers/i2c/busses/i2c-imx-lpi2c.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c
> > b/drivers/i2c/busses/i2c-imx-lpi2c.c
> > index c8f3dadfb36e..23f83f10d5f6 100644
> > --- a/drivers/i2c/busses/i2c-imx-lpi2c.c
> > +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> > @@ -1309,7 +1309,7 @@ static int lpi2c_imx_probe(struct platform_device
> *pdev)
> >       if (ret)
> >               lpi2c_imx->bitrate = I2C_MAX_STANDARD_MODE_FREQ;
> >
> > -     ret = devm_request_irq(&pdev->dev, irq, lpi2c_imx_isr, 0,
> > +     ret = devm_request_irq(&pdev->dev, irq, lpi2c_imx_isr,
> > + IRQF_NO_SUSPEND,
> >                              pdev->name, lpi2c_imx);
> >       if (ret)
> >               return dev_err_probe(&pdev->dev, ret, "can't claim irq
> > %d\n", irq);

RE: [EXT] Re: [PATCH 5/5] i2c: imx-lpi2c: increase PM timeout to avoid operate clk frequently

[Carlos Song]

> -----Original Message-----
> From: Alexander Stein <alexander.stein@ew.tq-group.com>
> Sent: Thursday, August 29, 2024 6:55 PM
> To: Aisheng Dong <aisheng.dong@nxp.com>; andi.shyti@kernel.org;
> shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> festevam@gmail.com; linux-arm-kernel@lists.infradead.org
> Cc: linux-i2c@vger.kernel.org; imx@lists.linux.dev;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Carlos Song
> <carlos.song@nxp.com>
> Subject: [EXT] Re: [PATCH 5/5] i2c: imx-lpi2c: increase PM timeout to avoid
> operate clk frequently
>
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report this
> email' button
>
>
> Hi,
>
> Am Donnerstag, 29. August 2024, 11:37:22 CEST schrieb carlos.song@nxp.com:
> > From: Clark Wang <xiaoning.wang@nxp.com>
> >
> > Switching the clock frequently will affect the data transmission
> > efficiency, and prolong the timeout to reduce autosuspend times for
> > lpi2c.
>
> Efficiency as in throughput or total time per transfer? Do you have any numbers?
>

Hi, Thank your for your quick ack!

Apologies for the unclear explanation. The efficiency I'm referring to isn't just for I2C. For platforms with SC firmware
like the 8X series, every i2c transfer to enable or disable the clock notifies the SC firmware to perform a clock operation.
So if the autosuspend time is short, i2c may enable and disable clock frequently, it will occupy resources of the SC firmware.

Therefore, we add this patch to minimize the excessive sc firmware resource waste caused by frequent I2C clock enable/disable operations.


> Best regards,
> Alexander
>
> > Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
> > Signed-off-by: Carlos Song <carlos.song@nxp.com>
> > ---
> >  drivers/i2c/busses/i2c-imx-lpi2c.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c
> > b/drivers/i2c/busses/i2c-imx-lpi2c.c
> > index 210d505db76d..cc5e5d96aacd 100644
> > --- a/drivers/i2c/busses/i2c-imx-lpi2c.c
> > +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> > @@ -129,7 +129,7 @@
> >  #define I2C_CLK_RATIO        2
> >  #define CHUNK_DATA   256
> >
> > -#define I2C_PM_TIMEOUT               10 /* ms */
> > +#define I2C_PM_TIMEOUT               1000 /* ms */
> >  #define I2C_DMA_THRESHOLD    8 /* bytes */
> >
> >  enum lpi2c_imx_mode {
> >
>
>
> --
> TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> Amtsgericht München, HRB 105018
> Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> http://www.tq-g/
> roup.com%2F&data=05%7C02%7Ccarlos.song%40nxp.com%7C85ca11fd9cff416
> ea43108dcc8191786%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C
> 638605257330352810%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMD
> AiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdat
> a=AV343gx9HRiXKLNQk5kBKj%2FfJM0p%2FSLKG%2FQFuQg64I0%3D&reserved
> =0
>

Re: [PATCH 5/5] i2c: imx-lpi2c: increase PM timeout to avoid operate clk frequently

[Alexander Stein]

Hi,

Am Freitag, 30. August 2024, 10:53:16 CEST schrieb Carlos Song:
> > -----Original Message-----
> > From: Alexander Stein <alexander.stein@ew.tq-group.com>
> > Sent: Thursday, August 29, 2024 6:55 PM
> > To: Aisheng Dong <aisheng.dong@nxp.com>; andi.shyti@kernel.org;
> > shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> > festevam@gmail.com; linux-arm-kernel@lists.infradead.org
> > Cc: linux-i2c@vger.kernel.org; imx@lists.linux.dev;
> > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Carlos Song
> > <carlos.song@nxp.com>
> > Subject: [EXT] Re: [PATCH 5/5] i2c: imx-lpi2c: increase PM timeout to avoid
> > operate clk frequently
> >
> > Caution: This is an external email. Please take care when clicking links or
> > opening attachments. When in doubt, report the message using the 'Report this
> > email' button
> >
> >
> > Hi,
> >
> > Am Donnerstag, 29. August 2024, 11:37:22 CEST schrieb carlos.song@nxp.com:
> > > From: Clark Wang <xiaoning.wang@nxp.com>
> > >
> > > Switching the clock frequently will affect the data transmission
> > > efficiency, and prolong the timeout to reduce autosuspend times for
> > > lpi2c.
> >
> > Efficiency as in throughput or total time per transfer? Do you have any numbers?
> >
> 
> Hi, Thank your for your quick ack!
> 
> Apologies for the unclear explanation. The efficiency I'm referring to isn't just for I2C. For platforms with SC firmware
> like the 8X series, every i2c transfer to enable or disable the clock notifies the SC firmware to perform a clock operation.
> So if the autosuspend time is short, i2c may enable and disable clock frequently, it will occupy resources of the SC firmware.
> 
> Therefore, we add this patch to minimize the excessive sc firmware resource waste caused by frequent I2C clock enable/disable operations.

Thanks for the explanation. So this delay only occurs on systems using an SC (imx8qm/qxp) or SM (imx95), right?
Is there a chance to detect this kind of clock provider and make this timeout configurable?

Best regards,
Alexander
-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/