Re: [PATCH/RFC 1/1] recover from davinci i2c time out conditions

[Philby John <pjohn-k0rHJ+Hhz/SB+jHODAdFcQ@public.gmane.org>]

Hello David,

On Wed, 2009-07-15 at 10:10 -0700, David Brownell wrote:
> On Wednesday 15 July 2009, Philby John wrote:
> >  /*
> > + * Configure the i2c data pin as a GPIO input and the i2c clock pin as a
> > + * high GPIO output.
> > + */
> > +static void disable_i2c_pins(void)
> > +{
> > +       unsigned long flags;
> > +
> > +       local_irq_save(flags);
> > +       if (cpu_is_davinci_dm355()) {
> > +               gpio_direction_input(15);
> > +               gpio_direction_output(14, 0);
> > +               gpio_set_value(14, 1);
> 
> As noted:  gpio_direction_output(14, 1) is better.

Now Corrected!

> 
> > +               davinci_cfg_reg(DM355_I2C_SDA);
> > +               davinci_cfg_reg(DM355_I2C_SCL);
> > +       }
> > +       local_irq_restore(flags);
> > +}
> > +
> > +/* Connect the i2c pins to the i2c controller. */
> > +static void enable_i2c_pins(void)
> > +{
> > +       unsigned long flags;
> > +
> > +       local_irq_save(flags);
> > +       if (cpu_is_davinci_dm355()) {
> > +               davinci_cfg_reg(DM355_I2C_SDA);
> > +               davinci_cfg_reg(DM355_I2C_SCL);
> > +       }
> > +       local_irq_restore(flags);
> > +}
> > +
> > +
> > +/* Generate a pulse on the i2c clock pin. */
> > +static void pulse_i2c_clock(void)
> > +{
> > +       if (cpu_is_davinci_dm355()) {
> > +               gpio_set_value(14, 0);
> > +               udelay(20);
> > +               gpio_set_value(14, 1);
> > +               udelay(20);
> > +       }
> > +}
> > +
> 
> The general problem with how that's done is that the SoC-specific
> bits should not be in this file.  The mach-davinc/dm355.c file
> should hold that configuration; likewise for other SoCs.

Not quite sure what you were expecting here. What I could do is to set
sda_pin and scl_pin in struct davinci_i2c_platform_data, for all davinci
platforms there is, rather than make davinci specific changes to
i2c-gpio.c I would also prefer not to make changes to
board-dm646x-evm.c/board-dm365-evm.c/board-dm355-evm.c etc...
Please let me know what you think.

> 
> > +                       /* Disable i2c */
> > +                       disable_i2c_pins();
> > +                       for (i = 0; i < 10; i++)
> > +                               pulse_i2c_clock();
> > +                       /* Re-enable i2c */
> > +                       enable_i2c_pins();
> 
> A better description of this is that you're trying to follow
> the I2Cv3 instructions about how to *reset* the bus, yes?

This is correct. To be precise, I am following the bus recovery
procedure as outlined in AN10216-01 I2C Manual. Reproducing here as
general information.

•SDA line is then non usable anymore because of the
“Slave-Transmitter”mode.
•Methods to recover the SDA line are:
–Reset the slave device (assuming the device has a Reset pin)
–Use a bus recovery sequence to leave the “Slave-Transmitter” mode
•Bus recovery sequence is done as following:
1-Send 9 clock pulses on SCL line
2-Ask the master to keep SDA High until the “Slave-Transmitter” releases
the SDA line to perform the ACK operation
3-Keeping SDA High during the ACK means that the “Master-Receiver”does
not acknowledge the previous byte receive
4-The “Slave-Transmitter” then goes in an idle state
5-The master then sends a STOP command initializing completely the bus


Thank you for your comments.
Regards,
Philby