All of lore.kernel.org
 help / color / mirror / Atom feed
From: Francesco Dolcini <francesco.dolcini@toradex.com>
To: "Oleksandr Suvorov" <oleksandr.suvorov@foundries.io>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Wolfram Sang" <wsa@kernel.org>,
	"Marco Felsch" <m.felsch@pengutronix.de>
Cc: Oleksij Rempel <linux@rempel-privat.de>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	NXP Linux Team <linux-imx@nxp.com>,
	linux-i2c@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v1] i2c: imx: Retry transfer on transient failure
Date: Wed, 13 Jul 2022 13:57:50 +0200	[thread overview]
Message-ID: <20220713115750.GA504159@francesco-nb.int.toradex.com> (raw)
In-Reply-To: <20220712100504.GB319880@francesco-nb.int.toradex.com>

+ oleksandr.suvorov@foundries.io

Hello all,

On Tue, Jul 12, 2022 at 12:05:04PM +0200, Francesco Dolcini wrote:
> On Tue, Jul 12, 2022 at 11:05:14AM +0200, Uwe Kleine-König wrote:
> > In which situations does this help? Please mention these in the
> > commit log.
> I'll do

I did some investigation on this, unfortunately we have this change
laying around since 1 year, it was written by Oleksandr, and in the
meantime he moved to a new company. I added him to this email thread, so
he can comment in case he remembers more.

We introduced this change while working on OV5640 camera sensor on an
apalis-imx6q evaluation board, without this change we had some sporadic
i2c communication issues. Unfortunately I do not have any better
details.

To me looks like having some (3? 5?) retry as a default is somehow
more reasonable than to never retry, not sure if this should be
implemented as a default for all the i2c adapters. From what I was able
to see that would not be a trivial change (the retry parameter is coming
from the i2c_imx driver, there is no obvious way to have a default in
the i2c core).

Would it work for you to keep the change as it is (just getting rid
of the useless define) and add a little bit more blurb to the commit
message to include the various comments collected so far?

Francesco


WARNING: multiple messages have this Message-ID (diff)
From: Francesco Dolcini <francesco.dolcini@toradex.com>
To: "Oleksandr Suvorov" <oleksandr.suvorov@foundries.io>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Wolfram Sang" <wsa@kernel.org>,
	"Marco Felsch" <m.felsch@pengutronix.de>
Cc: Oleksij Rempel <linux@rempel-privat.de>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	NXP Linux Team <linux-imx@nxp.com>,
	linux-i2c@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v1] i2c: imx: Retry transfer on transient failure
Date: Wed, 13 Jul 2022 13:57:50 +0200	[thread overview]
Message-ID: <20220713115750.GA504159@francesco-nb.int.toradex.com> (raw)
In-Reply-To: <20220712100504.GB319880@francesco-nb.int.toradex.com>

+ oleksandr.suvorov@foundries.io

Hello all,

On Tue, Jul 12, 2022 at 12:05:04PM +0200, Francesco Dolcini wrote:
> On Tue, Jul 12, 2022 at 11:05:14AM +0200, Uwe Kleine-König wrote:
> > In which situations does this help? Please mention these in the
> > commit log.
> I'll do

I did some investigation on this, unfortunately we have this change
laying around since 1 year, it was written by Oleksandr, and in the
meantime he moved to a new company. I added him to this email thread, so
he can comment in case he remembers more.

We introduced this change while working on OV5640 camera sensor on an
apalis-imx6q evaluation board, without this change we had some sporadic
i2c communication issues. Unfortunately I do not have any better
details.

To me looks like having some (3? 5?) retry as a default is somehow
more reasonable than to never retry, not sure if this should be
implemented as a default for all the i2c adapters. From what I was able
to see that would not be a trivial change (the retry parameter is coming
from the i2c_imx driver, there is no obvious way to have a default in
the i2c core).

Would it work for you to keep the change as it is (just getting rid
of the useless define) and add a little bit more blurb to the commit
message to include the various comments collected so far?

Francesco


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2022-07-13 11:58 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-12  8:24 [PATCH v1] i2c: imx: Retry transfer on transient failure Francesco Dolcini
2022-07-12  8:24 ` Francesco Dolcini
2022-07-12  8:47 ` Marco Felsch
2022-07-12  8:47   ` Marco Felsch
2022-07-12  9:14   ` Francesco Dolcini
2022-07-12  9:14     ` Francesco Dolcini
2022-07-12 11:32   ` Wolfram Sang
2022-07-12 11:32     ` Wolfram Sang
2022-07-12 12:02     ` Marco Felsch
2022-07-12 12:02       ` Marco Felsch
2022-07-12  9:05 ` Uwe Kleine-König
2022-07-12  9:05   ` Uwe Kleine-König
2022-07-12 10:05   ` Francesco Dolcini
2022-07-12 10:05     ` Francesco Dolcini
2022-07-13 11:57     ` Francesco Dolcini [this message]
2022-07-13 11:57       ` Francesco Dolcini
2022-07-13 13:24       ` Oleksij Rempel
2022-07-13 13:24         ` Oleksij Rempel
2022-07-13 13:43         ` Francesco Dolcini
2022-07-13 13:43           ` Francesco Dolcini
2022-07-13 15:57           ` Oleksij Rempel
2022-07-13 15:57             ` Oleksij Rempel
2022-07-13 20:25             ` Francesco Dolcini
2022-07-13 20:25               ` Francesco Dolcini
2022-07-14  7:07               ` Oleksij Rempel
2022-07-14  7:07                 ` Oleksij Rempel
2022-07-14  7:34                 ` Francesco Dolcini
2022-07-14  7:34                   ` Francesco Dolcini
2022-07-15  6:49                   ` Uwe Kleine-König
2022-07-15  6:49                     ` Uwe Kleine-König
2022-07-15  7:24                     ` Francesco Dolcini
2022-07-15  7:24                       ` Francesco Dolcini
2022-07-15  8:34                       ` Uwe Kleine-König
2022-07-15  8:34                         ` Uwe Kleine-König
2022-07-15 11:45                         ` Francesco Dolcini
2022-07-15 11:45                           ` Francesco Dolcini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220713115750.GA504159@francesco-nb.int.toradex.com \
    --to=francesco.dolcini@toradex.com \
    --cc=festevam@gmail.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-imx@nxp.com \
    --cc=linux@rempel-privat.de \
    --cc=m.felsch@pengutronix.de \
    --cc=oleksandr.suvorov@foundries.io \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.org \
    --cc=u.kleine-koenig@pengutronix.de \
    --cc=wsa@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.