2024-05-20 15:40:29

by grygorii tertychnyi

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

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 <[email protected]>
Acked-by: Peter Korsgaard <[email protected]>
Cc: [email protected]
---
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



2024-05-20 17:00:37

by Markus Elfring

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


> 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

2024-05-20 18:30:37

by Greg Kroah-Hartman

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

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

2024-06-02 13:07:33

by grygorii tertychnyi

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

Hi,

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

regards

On Mon, May 20, 2024 at 5:40 PM Grygorii Tertychnyi <[email protected]> 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 <[email protected]>
> Acked-by: Peter Korsgaard <[email protected]>
> Cc: [email protected]
> ---
> 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
>