[PATCH RESEND v2 0/3] RZ/V2M I2Cdriver clean ups

[PATCH RESEND v2 0/3] RZ/V2M I2Cdriver clean ups

[Biju Das]

This patch series aims to do the following clean ups based on feedback
from Pavel while backporting this driver to 5.10.y-cip kernel.

Drop extra space from config help description, Replace lowercase macros
with static inline functions and finally in probe disable the operation
of unit in case of error similar to remove and suspend callbacks.

v1->v2:
 * Added Rb tag from Geert.
 * Updated commit header and description for patch#2.
 * Replaced macros bit_setl and bit_clrl with static inline functions.
 * Introduced rzv2m_i2c_disable() and shared the code between
   probe error path, remove and suspend callbacks.

Biju Das (3):
  i2c: rzv2m: Drop extra space
  i2c: rzv2m: Replace lowercase macros with static inline functions
  i2c: rzv2m: Disable the operation of unit in case of error

 drivers/i2c/busses/Kconfig     |  2 +-
 drivers/i2c/busses/i2c-rzv2m.c | 43 +++++++++++++++++++++++-----------
 2 files changed, 30 insertions(+), 15 deletions(-)

-- 
2.25.1

[PATCH RESEND v2 1/3] i2c: rzv2m: Drop extra space

[Biju Das]

Drop extra space from the I2C_RZV2M config help description.

Reported-by: Pavel Machek <pavel@denx.de>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v1->v2:
 * Added Rb tag from Geert.
---
 drivers/i2c/busses/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 87600b4aacb3..31c0f54b0b8c 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -1025,7 +1025,7 @@ config I2C_RZV2M
 	depends on ARCH_RENESAS || COMPILE_TEST
 	help
 	  If you say yes to this option, support will be included for the
-	  Renesas RZ/V2M  I2C interface.
+	  Renesas RZ/V2M I2C interface.
 
 	  This driver can also be built as a module.  If so, the module
 	  will be called i2c-rzv2m.
-- 
2.25.1

Re: [PATCH RESEND v2 1/3] i2c: rzv2m: Drop extra space

[Wolfram Sang]

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

On Fri, May 26, 2023 at 02:57:36PM +0100, Biju Das wrote:
> Drop extra space from the I2C_RZV2M config help description.
> 
> Reported-by: Pavel Machek <pavel@denx.de>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Applied to for-next, thanks!


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

[PATCH RESEND v2 2/3] i2c: rzv2m: Replace lowercase macros with static inline functions

[Biju Das]

Convert macros bit_setl and bit_clrl with static inline functions
as normally we'd put macro names in all uppercase.

Reported-by: Pavel Machek <pavel@denx.de>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v1->v2:
 * Updated commit header and description.
 * Replaced macros bit_setl and bit_clrl with static inline functions.
---
 drivers/i2c/busses/i2c-rzv2m.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rzv2m.c b/drivers/i2c/busses/i2c-rzv2m.c
index 56d0faee5c46..b97e29871558 100644
--- a/drivers/i2c/busses/i2c-rzv2m.c
+++ b/drivers/i2c/busses/i2c-rzv2m.c
@@ -50,9 +50,6 @@
 #define IICB0MDSC	BIT(7)		/* Bus Mode */
 #define IICB0SLSE	BIT(1)		/* Start condition output */
 
-#define bit_setl(addr, val)		writel(readl(addr) | (val), (addr))
-#define bit_clrl(addr, val)		writel(readl(addr) & ~(val), (addr))
-
 struct rzv2m_i2c_priv {
 	void __iomem *base;
 	struct i2c_adapter adap;
@@ -78,6 +75,16 @@ static const struct bitrate_config bitrate_configs[] = {
 	[RZV2M_I2C_400K] = { 52, 900 },
 };
 
+static inline void bit_setl(void __iomem *addr, u32 val)
+{
+	writel(readl(addr) | val, addr);
+}
+
+static inline void bit_clrl(void __iomem *addr, u32 val)
+{
+	writel(readl(addr) & ~val, addr);
+}
+
 static irqreturn_t rzv2m_i2c_tia_irq_handler(int this_irq, void *dev_id)
 {
 	struct rzv2m_i2c_priv *priv = dev_id;
-- 
2.25.1

Re: [PATCH RESEND v2 2/3] i2c: rzv2m: Replace lowercase macros with static inline functions

[Wolfram Sang]

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

On Fri, May 26, 2023 at 02:57:37PM +0100, Biju Das wrote:
> Convert macros bit_setl and bit_clrl with static inline functions
> as normally we'd put macro names in all uppercase.
> 
> Reported-by: Pavel Machek <pavel@denx.de>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Applied to for-next, thanks!


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

[PATCH RESEND v2 3/3] i2c: rzv2m: Disable the operation of unit in case of error

[Biju Das]

The remove and suspend callbacks disable the operation of the unit.
Do the same in probe() in case of error.

While at it, introduce a helper function rzv2m_i2c_disable() for
disabling the operation of the unit and this function is shared
between probe error path, remove and suspend callbacks.

Reported-by: Pavel Machek <pavel@denx.de>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v1->v2:
 * Introduced rzv2m_i2c_disable() and shared the code between
   probe error path, remove and suspend callbacks.
 * Updated commit description.
---
 drivers/i2c/busses/i2c-rzv2m.c | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rzv2m.c b/drivers/i2c/busses/i2c-rzv2m.c
index b97e29871558..7acde5133e51 100644
--- a/drivers/i2c/busses/i2c-rzv2m.c
+++ b/drivers/i2c/busses/i2c-rzv2m.c
@@ -389,6 +389,20 @@ static u32 rzv2m_i2c_func(struct i2c_adapter *adap)
 	       I2C_FUNC_10BIT_ADDR;
 }
 
+static int rzv2m_i2c_disable(struct device *dev, struct rzv2m_i2c_priv *priv)
+{
+	int ret;
+
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret < 0)
+		return ret;
+
+	bit_clrl(priv->base + IICB0CTL0, IICB0IICE);
+	pm_runtime_put(dev);
+
+	return 0;
+}
+
 static const struct i2c_adapter_quirks rzv2m_i2c_quirks = {
 	.flags = I2C_AQ_NO_ZERO_LEN,
 };
@@ -461,8 +475,10 @@ static int rzv2m_i2c_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, priv);
 
 	ret = i2c_add_numbered_adapter(adap);
-	if (ret < 0)
+	if (ret < 0) {
+		rzv2m_i2c_disable(dev, priv);
 		pm_runtime_disable(dev);
+	}
 
 	return ret;
 }
@@ -473,7 +489,7 @@ static int rzv2m_i2c_remove(struct platform_device *pdev)
 	struct device *dev = priv->adap.dev.parent;
 
 	i2c_del_adapter(&priv->adap);
-	bit_clrl(priv->base + IICB0CTL0, IICB0IICE);
+	rzv2m_i2c_disable(dev, priv);
 	pm_runtime_disable(dev);
 
 	return 0;
@@ -482,16 +498,8 @@ static int rzv2m_i2c_remove(struct platform_device *pdev)
 static int rzv2m_i2c_suspend(struct device *dev)
 {
 	struct rzv2m_i2c_priv *priv = dev_get_drvdata(dev);
-	int ret;
-
-	ret = pm_runtime_resume_and_get(dev);
-	if (ret < 0)
-		return ret;
-
-	bit_clrl(priv->base + IICB0CTL0, IICB0IICE);
-	pm_runtime_put(dev);
 
-	return 0;
+	return rzv2m_i2c_disable(dev, priv);
 }
 
 static int rzv2m_i2c_resume(struct device *dev)
-- 
2.25.1

Re: [PATCH RESEND v2 3/3] i2c: rzv2m: Disable the operation of unit in case of error

[Wolfram Sang]

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

On Fri, May 26, 2023 at 02:57:38PM +0100, Biju Das wrote:
> The remove and suspend callbacks disable the operation of the unit.
> Do the same in probe() in case of error.
> 
> While at it, introduce a helper function rzv2m_i2c_disable() for
> disabling the operation of the unit and this function is shared
> between probe error path, remove and suspend callbacks.
> 
> Reported-by: Pavel Machek <pavel@denx.de>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Applied to for-next, thanks!

Do you maybe want to add yourself as a MAINTAINER for this driver?


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

RE: [PATCH RESEND v2 3/3] i2c: rzv2m: Disable the operation of unit in case of error

[Biju Das]

Hi Wolfram Sang,

Thanks for the feedback.

> Subject: Re: [PATCH RESEND v2 3/3] i2c: rzv2m: Disable the operation of
> unit in case of error
> 
> On Fri, May 26, 2023 at 02:57:38PM +0100, Biju Das wrote:
> > The remove and suspend callbacks disable the operation of the unit.
> > Do the same in probe() in case of error.
> >
> > While at it, introduce a helper function rzv2m_i2c_disable() for
> > disabling the operation of the unit and this function is shared
> > between probe error path, remove and suspend callbacks.
> >
> > Reported-by: Pavel Machek <pavel@denx.de>
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Applied to for-next, thanks!
> 
> Do you maybe want to add yourself as a MAINTAINER for this driver?

Agreed. But Fabrizio is going to maintain this driver. So will update
MAINTAINER entries for this driver with Fabrizio's name/e-mail address.

Cheers,
Biju