Hi Philby,
On Fri, Feb 05, 2010 at 19:23:43, Philby John wrote:
> Hello Sekhar,
>
[...]
> >> +/* Generate a pulse on the i2c clock pin. */
> >> +static void generic_i2c_clock_pulse(unsigned int scl_pin)
> >> +{
> >> + u16 i;
> >> +
> >> + if (scl_pin) {
> >> + /* Send high and low on the SCL line */
> >> + for (i = 0; i < 9; i++) {
> >> + gpio_set_value(scl_pin, 0);
> >> + udelay(20);
> >> + gpio_set_value(scl_pin, 1);
> >> + udelay(20);
> >> + }
> >
> > Before using the pins as GPIO, you would have to set the
> > functionality of these pins as GPIO. You had this code in
> > previous incarnations of this patch - not sure why it is
> > dropped now.
> >
>
> Don't seem to remember having the code in the old versions at least
> not in generic_i2c_clock_pulse(). The functions disable_i2c_pins() and
> enable_i2c_pins() were discarded as the i2c protocol spec. did not
> specify the need. Moreover bus recovered without it. (Tested on DM355
> and Dm6446).
Yes, I was referring to the davinci_cfg_reg() calls in
{disable|enable}_i2c_pins() functions. Per the specification
of the DaVinci devices, a pin needs to be muxed as 'GPIO' if
it is to be used as GPIO controlled by GPIO module. It may
have worked on couple of devices but cannot be guaranteed to
work on all DaVinci devices (esp. DA8XX ones).
>
> > Couple of good to haves:
> >
[...]
> >
> > The I2C peripheral on da8xx itself contains a mode where its
> > pins could be used as GPIO - so no need for SoC level muxing
> > and need for the platform data. This seems to be missing from
> > DM355 though. Thankfully there is a revision id within the I2C
> > memory map which will help you differentiate the two cases
> > (revision 0x5 vs 0x6)
>
> I did not entirely follow your above statement. Will usage of
> gpio_request() solve the problem for da8xx and DM355 or does it
> require a if else condition?
You require special casing for I2C version 0x6.
Have a look here: http://focus.ti.com/lit/ug/sprufl9a/sprufl9a.pdf
I was referring to registers described in sections
3.15-3.20
> A reminder that the present code will
> only work for DM6446 and DM355. I do not have a DA8xx to test specific
> functionality if it were to be added.
Right. It is only an enhancement (and only good
to have at that). This should not stop the current
patch from getting in.
Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
