Date:   Tue, 30 Nov 2021 09:56:18 +0100
From:   Alain Volmat <alain.volmat@foss.st.com>
To:     Wolfram Sang <wsa@kernel.org>, <pierre-yves.mordret@foss.st.com>,
        <alexandre.torgue@foss.st.com>, <linux-i2c@vger.kernel.org>,
        <linux-stm32@st-md-mailman.stormreply.com>,
        <linux-arm-kernel@lists.infradead.org>,
        <linux-kernel@vger.kernel.org>, <fabrice.gasnier@foss.st.com>,
        <amelie.delaunay@foss.st.com>
Subject: Re: [PATCH 2/4] i2c: stm32f7: recover the bus on access timeout
Message-ID: <20211130085618.GA668426@gnbcxd0016.gnb.st.com>
Mail-Followup-To: Wolfram Sang <wsa@kernel.org>,
        pierre-yves.mordret@foss.st.com, alexandre.torgue@foss.st.com,
        linux-i2c@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        fabrice.gasnier@foss.st.com, amelie.delaunay@foss.st.com
References: <1632151292-18503-1-git-send-email-alain.volmat@foss.st.com>
 <1632151292-18503-3-git-send-email-alain.volmat@foss.st.com>
 <YaTE0f9ciy5JRZ3Q@kunai>
 <20211129123302.GB486850@gnbcxd0016.gnb.st.com>
 <YaTNDq7rEyQIopim@kunai>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <YaTNDq7rEyQIopim@kunai>
Precedence: bulk

Hi Alain,

On Mon, Nov 29, 2021 at 01:52:30PM +0100, Wolfram Sang wrote:
> Hi Alain,
> 
> > > > +		stm32f7_i2c_wait_free_bus(i2c_dev);
> > > 
> > > This does only a controller reset, not a bus recovery with 9 toggling
> > > pulses, or?
> > 
> > indeed. I might better rework this and at the same time introduce the
> > bus recovery mechanism via the bus recovery callback in this driver.
> > Please don't merge this patch and I will rework that.
> 
> Wait a sec. Resetting a controller at the end of a failed transfer might
> make sense if the controller is otherwise in an confused state.
> 
> Full bus recovery (9 pulses) should be done at the beginning of a
> transfer when SDA is low, though.
> 
> So, I'd actually suggest to apply this patch and add full bus recovery
> based on SDA low at the beginning of a transfer seperately.
> 
> What doo you think?

I just checked again.  Indeed, this patch is here to handle cases when
communication went bad with a device leading to controller being left in
a confused state.  This is done to put it back in a working state.

I agree with you on the fact to decouple this with the 9 pulses bus
recovery and first apply this one first.

Thanks.
Alain

> 
> All the best,
> 
>    Wolfram
>