Re: [PATCH 2/4] i2c: at91: implement i2c bus recovery

[<Ludovic.Desroches@microchip.com>]

On Thu, Oct 24, 2019 at 02:29:13PM +0200, Kamel Bouhara wrote:
> 
> On 10/10/2019 08:54, Ludovic Desroches wrote:
> > On Wed, Oct 09, 2019 at 04:01:47PM +0200, Alexandre Belloni wrote:
> > > 
> > > On 09/10/2019 15:55:00+0200, Ludovic Desroches wrote:
> > > > On Wed, Oct 02, 2019 at 04:46:56PM +0200, Kamel Bouhara wrote:
> > > > > External E-Mail
> > > > > 
> > > > > 
> > > > > Implement i2c bus recovery when slaves devices might hold SDA low.
> > > > > In this case re-assign SCL/SDA to gpios and issue 9 dummy clock pulses
> > > > > until the slave release SDA.
> > > > > 
> > > > 
> > > > Hi Kamel,
> > > > 
> Hi Ludovic,
> 
> > > > Thanks for adding this new feature. As I see patches only for sama5d3 and
> > > > sama5d4, I assume it has not been tested with a sama5d2, isn't it?
> > > > 
> > > 
> > > I there a point having it on sama5d2 as the controller already supports
> > > this feature?
> > > 
> > 
> > Right, I was focused on pinctrl and forget we have this feature
> > supported by the IP.
> > 
> > > > I doubt it works with a sama5d2 because of the pinctrl. I also wonder if it can
> > > > work if we add .strict = true to pinmux_ops which is something plan for the
> > > > future...
> > > > 
> > > 
> > > I don't see why it wouldn't work with strict as this is switching muxing
> > > properly instead of using the pins for two functions at the same time.
> > > 
> > 
> > Not sure devm_gpiod_get won't fail with strict.
> > 
> Actually the strict flag is checked in the pinmux core to allow the pin
> request.
> 
> What is the purpose to enable it ? It seems to break a lot things, eg. on
> sama5d3 :

Hi Kamel,

First, to be clear, I am not against this patch, I'll ack the new
version. My goal is only to be aware if the use of strict can have side
effects.

I am more used to the pio4 but I assume it's the same with the older
one. As you notice enabling it should break many things. It involves DT
changes for pins used as gpio. They have to be removed from the
pinmuxing or to find a way to allow a gpio request on a pin muxed as a
gpio.

I would like to enable it, because, at the moment, if you request a gpio,
for example using the sysfs, you can change the muxing of the pin and
breaks a device using it. If strict is enabled, this change will be
rejected and it's probably safer.

> 
> # dmesg |grep pin
> pinctrl-at91 ahb:apb:pinctrl@fffff200: pin pioA18 already requested by
> f801c000.i2c; cannot claim for fffff200.gpio:18
> pinctrl-at91 ahb:apb:pinctrl@fffff200: pin-18 (fffff200.gpio:18) status -22
> pinctrl-at91 ahb:apb:pinctrl@fffff200: pin pioA19 already requested by
> f801c000.i2c; cannot claim for fffff200.gpio:19
> pinctrl-at91 ahb:apb:pinctrl@fffff200: pin-19 (fffff200.gpio:19) status -22

Thanks for testing it, it confirms what I had in mind.

Regards

Ludovic

> pinctrl-at91 ahb:apb:pinctrl@fffff200: pin pioE9 already requested by
> 500000.gadget; cannot claim for fffffa00.gpio:137
> pinctrl-at91 ahb:apb:pinctrl@fffff200: pin-137 (fffffa00.gpio:137) status
> -22
> pinctrl-at91 ahb:apb:pinctrl@fffff200: pin pioE0 already requested by
> f0000000.mmc; cannot claim for fffffa00.gpio:128
> pinctrl-at91 ahb:apb:pinctrl@fffff200: pin-128 (fffffa00.gpio:128) status
> -22
> pinctrl-at91 ahb:apb:pinctrl@fffff200: pin pioE1 already requested by
> f8000000.mmc; cannot claim for fffffa00.gpio:129
> pinctrl-at91 ahb:apb:pinctrl@fffff200: pin-129 (fffffa00.gpio:129) status
> -22
> pinctrl-at91 ahb:apb:pinctrl@fffff200: pin pioE29 already requested by
> gpio_keys; cannot claim for fffffa00.gpio:157
> pinctrl-at91 ahb:apb:pinctrl@fffff200: pin-157 (fffffa00.gpio:157) status
> -22
> 
> > Ludovic
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> > 
> 
> -- 
> Kamel Bouhara, Bootlin
> Embedded Linux and kernel engineering
> https://bootlin.com
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

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