Re: [Patch v2] i2c: imx: implement bus recovery

["Uwe Kleine-König" <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>]

Hello,

On Wed, Aug 19, 2015 at 07:43:51AM +0000, Gao Pandy wrote:
> > On Wed, Aug 19, 2015 at 03:44:49AM +0000, Gao Pandy wrote:
> > > > >  	/* Set up adapter data */
> > > > >  	i2c_set_adapdata(&i2c_imx->adapter, i2c_imx);
> > > > >
> > > > > +	/* Init recover pins */
> > > > > +	i2c_imx->pins.sda =
> > > > > +		devm_gpiod_get_optional(&pdev->dev, "sda-gpio", 0);
> > > > > +	i2c_imx->pins.scl =
> > > > > +		devm_gpiod_get_optional(&pdev->dev, "scl-gpio", 0);
> > > >
> > > > 0 is wrong here, better use GPIOD_ASIS. Is there a reason not to
> > > > configure the direction as input already here?
> > >
> > > Thanks, 0 is inappropriate here. Actually, we should use
> > > GPIOD_OUT_HIGH to set the gpios high.
> > 
> > Are you sure? Consider the bus is blocked and you switch to gpio mode to
> > do recovery you have a device pulling SDA to 0 and the imx pulling it to
> > 1. That's wrong, isn't it?
> 
> Yes, you are right. The gpios should be set low. Thanks.
Then on switching to the gpio function from an idle bus (which should be
the normal case) you pull both lines low which depending on the timing
might even be recognised as a start condition. I'm convinced the right
thing to select here is GPIOD_IN.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |