Re: [PATCH V5 Resend 1/2] i2c/adapter: Add bus recovery infrastructure

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

On Thu, Oct 04, 2012 at 03:02:26PM +0530, Viresh Kumar wrote:
> On 4 October 2012 14:50, Uwe Kleine-König
> <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote:
> 
> >> So, we actually need to do "Low-High" 9 times instead of "High-Low".
> >> So, initializing val to 0 should fix it?
> > I don't think this is enough. If you cut off the last half clock of the
> > first sequence above doing 9 times low-high isn't enough. So you have to
> > do high + 9x low-high to assert 9 full cycles.
> 
> I am not cutting the last half clock. val is the variable which keeps
> track of value to be
> set on the line. I am asking to start from zero.
I meant the sequence that created the stall, not the one intending to
clear it. If you remove the last rising edge from that the SCL line is
initially low.
 
> You mean, we should do a high first and then 9 low-high? But this line was high
> initially.. what difference will it make by making it high again?
> 
> Sorry, i am not a I2C expert, so need some help :)
:(

> >> >> + * @scl_gpio_flags: flag for gpio_request_one of scl_gpio. 0 implies
> >> >> + *   GPIOF_OUT_INIT_LOW.
> >> >> + * @sda_gpio_flags: flag for gpio_request_one of sda_gpio. 0 implies
> >> >> + *   GPIOF_OUT_INIT_LOW.
> >>
> >> > Didn't you want to change this to GPIOF_OUT_INIT_HIGH |
> >> > GPIOF_OPEN_DRAIN? Hmm, I just wonder how to distinguish
> >> > GPIOF_OUT_INIT_LOW from unset. Hmm, maybe assume unset because _LOW is
> >> > wrong?
> >>
> >> Hmmm.... Hmmmm... Hmmm... :)
> >> Two things:
> >> - Why should we default it to GPIOF_OPEN_DRAIN? Open drain would mean,
> >> gpio_set_value() will not write one for it, but would put it in input mode.
> >> I don't think that would be correct, as an generic case.
> > As the i2c-bus is open drain and multi-master capable it's wrong to pull
> > it to 1 because this can result in a short circuit. Even in a
> > single-master scenario (where you can pull SCL in both directions) you
> > must not drive SDA to 1.
> 
> Hmm... Hopefully i understood it well this time.
> - We actually don't need these flags then.
> 
> Always pass:
> - scl_flags: GPIOF_OPEN_DRAIN | GPIOF_OUT_INIT_HIGH
> - sda_flags: GPIOF_IN
> 
> Why would anybody else require something different for any SoC?
IMHO this should work, yes. (At least conceptually, not necessary for
all implementations.)

Best regards
Uwe

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