[PATCH v2] i2c: ocores: set IACK bit after core is enabled

[PATCH v2] i2c: ocores: set IACK bit after core is enabled

[Grygorii Tertychnyi]

Setting IACK bit when core is disabled does not clear the "Interrupt Flag"
bit in the status register, and the interrupt remains pending.

Sometimes it causes failure for the very first message transfer, that is
usually a device probe.

Hence, set IACK bit after core is enabled to clear pending interrupt.

Fixes: 18f98b1e3147 ("[PATCH] i2c: New bus driver for the OpenCores I2C controller")
Signed-off-by: Grygorii Tertychnyi <grygorii.tertychnyi@leica-geosystems.com>
Acked-by: Peter Korsgaard <peter@korsgaard.com>
Cc: stable@vger.kernel.org
---
V1 -> V2: Added "Acked-by:", "Fixes:" and "Cc:" tags

 drivers/i2c/busses/i2c-ocores.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index e106af83cef4..350ccfbe8634 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -442,8 +442,8 @@ static int ocores_init(struct device *dev, struct ocores_i2c *i2c)
 	oc_setreg(i2c, OCI2C_PREHIGH, prescale >> 8);
 
 	/* Init the device */
-	oc_setreg(i2c, OCI2C_CMD, OCI2C_CMD_IACK);
 	oc_setreg(i2c, OCI2C_CONTROL, ctrl | OCI2C_CTRL_EN);
+	oc_setreg(i2c, OCI2C_CMD, OCI2C_CMD_IACK);
 
 	return 0;
 }
-- 
2.43.0

Re: [PATCH v2] i2c: ocores: set IACK bit after core is enabled

[Markus Elfring]

…
> Sometimes it causes failure for the very first message transfer, …

Would it be helpful to mention the term “data corruption” here?
https://lore.kernel.org/lkml/CAGFuAuwot_7+R=J4NC=0Z_48YZ-RTJjRUoQnSjZUvpt=AWF39Q@mail.gmail.com/
https://lkml.org/lkml/2024/5/20/549


> Fixes: 18f98b1e3147 ("[PATCH] i2c: New bus driver for the OpenCores I2C controller")

Will the text “[PATCH] ” be omitted from this tag for the final commit?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.9#n145

Regards,
Markus

Re: [PATCH v2] i2c: ocores: set IACK bit after core is enabled

[Greg KH]

On Mon, May 20, 2024 at 07:00:08PM +0200, Markus Elfring wrote:
> …
> > Sometimes it causes failure for the very first message transfer, …
> 
> Would it be helpful to mention the term “data corruption” here?
> https://lore.kernel.org/lkml/CAGFuAuwot_7+R=J4NC=0Z_48YZ-RTJjRUoQnSjZUvpt=AWF39Q@mail.gmail.com/
> https://lkml.org/lkml/2024/5/20/549
> 
> 
> > Fixes: 18f98b1e3147 ("[PATCH] i2c: New bus driver for the OpenCores I2C controller")
> 
> Will the text “[PATCH] ” be omitted from this tag for the final commit?
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.9#n145
> 
> Regards,
> Markus
> 

Hi,

This is the semi-friendly patch-bot of Greg Kroah-Hartman.

Markus, you seem to have sent a nonsensical or otherwise pointless
review comment to a patch submission on a Linux kernel developer mailing
list.  I strongly suggest that you not do this anymore.  Please do not
bother developers who are actively working to produce patches and
features with comments that, in the end, are a waste of time.

Patch submitter, please ignore Markus's suggestion; you do not need to
follow it at all.  The person/bot/AI that sent it is being ignored by
almost all Linux kernel maintainers for having a persistent pattern of
behavior of producing distracting and pointless commentary, and
inability to adapt to feedback.  Please feel free to also ignore emails
from them.

thanks,

greg k-h's patch email bot

Re: [PATCH v2] i2c: ocores: set IACK bit after core is enabled

[grygorii tertychnyi]

Hi,

just a gentle ping...  Is there anything missing?

regards

On Mon, May 20, 2024 at 5:40 PM Grygorii Tertychnyi <grembeter@gmail.com> wrote:
>
> Setting IACK bit when core is disabled does not clear the "Interrupt Flag"
> bit in the status register, and the interrupt remains pending.
>
> Sometimes it causes failure for the very first message transfer, that is
> usually a device probe.
>
> Hence, set IACK bit after core is enabled to clear pending interrupt.
>
> Fixes: 18f98b1e3147 ("[PATCH] i2c: New bus driver for the OpenCores I2C controller")
> Signed-off-by: Grygorii Tertychnyi <grygorii.tertychnyi@leica-geosystems.com>
> Acked-by: Peter Korsgaard <peter@korsgaard.com>
> Cc: stable@vger.kernel.org
> ---
> V1 -> V2: Added "Acked-by:", "Fixes:" and "Cc:" tags
>
>  drivers/i2c/busses/i2c-ocores.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
> index e106af83cef4..350ccfbe8634 100644
> --- a/drivers/i2c/busses/i2c-ocores.c
> +++ b/drivers/i2c/busses/i2c-ocores.c
> @@ -442,8 +442,8 @@ static int ocores_init(struct device *dev, struct ocores_i2c *i2c)
>         oc_setreg(i2c, OCI2C_PREHIGH, prescale >> 8);
>
>         /* Init the device */
> -       oc_setreg(i2c, OCI2C_CMD, OCI2C_CMD_IACK);
>         oc_setreg(i2c, OCI2C_CONTROL, ctrl | OCI2C_CTRL_EN);
> +       oc_setreg(i2c, OCI2C_CMD, OCI2C_CMD_IACK);
>
>         return 0;
>  }
> --
> 2.43.0
>

Re: [PATCH v2] i2c: ocores: set IACK bit after core is enabled

[grygorii tertychnyi]

Hi,

just another gentle ping...  Not sure if I need to rebase it on the
latest master?

regards

On Mon, May 20, 2024 at 5:40 PM Grygorii Tertychnyi <grembeter@gmail.com> wrote:
>
> Setting IACK bit when core is disabled does not clear the "Interrupt Flag"
> bit in the status register, and the interrupt remains pending.
>
> Sometimes it causes failure for the very first message transfer, that is
> usually a device probe.
>
> Hence, set IACK bit after core is enabled to clear pending interrupt.
>
> Fixes: 18f98b1e3147 ("[PATCH] i2c: New bus driver for the OpenCores I2C controller")
> Signed-off-by: Grygorii Tertychnyi <grygorii.tertychnyi@leica-geosystems.com>
> Acked-by: Peter Korsgaard <peter@korsgaard.com>
> Cc: stable@vger.kernel.org
> ---
> V1 -> V2: Added "Acked-by:", "Fixes:" and "Cc:" tags
>
>  drivers/i2c/busses/i2c-ocores.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
> index e106af83cef4..350ccfbe8634 100644
> --- a/drivers/i2c/busses/i2c-ocores.c
> +++ b/drivers/i2c/busses/i2c-ocores.c
> @@ -442,8 +442,8 @@ static int ocores_init(struct device *dev, struct ocores_i2c *i2c)
>         oc_setreg(i2c, OCI2C_PREHIGH, prescale >> 8);
>
>         /* Init the device */
> -       oc_setreg(i2c, OCI2C_CMD, OCI2C_CMD_IACK);
>         oc_setreg(i2c, OCI2C_CONTROL, ctrl | OCI2C_CTRL_EN);
> +       oc_setreg(i2c, OCI2C_CMD, OCI2C_CMD_IACK);
>
>         return 0;
>  }
> --
> 2.43.0
>

Re: [PATCH v2] i2c: ocores: set IACK bit after core is enabled

[Andi Shyti]

Hi

On Mon, 20 May 2024 17:39:32 +0200, Grygorii Tertychnyi wrote:
> Setting IACK bit when core is disabled does not clear the "Interrupt Flag"
> bit in the status register, and the interrupt remains pending.
> 
> Sometimes it causes failure for the very first message transfer, that is
> usually a device probe.
> 
> Hence, set IACK bit after core is enabled to clear pending interrupt.
> 
> [...]

Applied to i2c/i2c-host-next on

git://git.kernel.org/pub/scm/linux/kernel/git/local tree

Thank you,
Andi

Patches applied
===============
[1/1] i2c: ocores: set IACK bit after core is enabled
      commit: 5a72477273066b5b357801ab2d315ef14949d402