On Tue, Feb 09, 2010 at 15:45:15, Philby John wrote:
> On 02/08/2010 09:33 PM, Nori, Sekhar wrote:
> > On Mon, Feb 08, 2010 at 20:43:27, Philby John wrote:
> >> Hello Sekhar,
> >>
> >> On 02/08/2010 04:05 PM, Nori, Sekhar wrote:
> >>>>>> +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).
> >>
> >>
> >> I think that using davinci_cfg_reg() in generic_i2c_clock_pulse() is the
> >> wrong place to put it. This would require adding davinci_cfg_reg() for
> >> all know davinci platforms. The i2c recovery procedure is correct to
> >> assume that it owns the SCL line at that very moment.
> >>
> >> Instead I believe pinmuxing using davinci_cfg_reg(), should be done way
> >> early, just like we do for DM6446 in devices.c --> davinci_init_i2c(),
> >> for all other platforms. What I could do in function
> >> generic_i2c_clock_pulse() is, set SCL to output, and use gpio_request()
> >> by checking REVID2 register value (0x6) for DA8xx and 0x5 for others.
> >
> > But, the pins should remain as I2C pins till you actually
> > hit a bus lock-up. That's when you need to convert them to GPIO
> > pins and start the recovery by pulsing SCL.
> >
> > It you make them GPIO right at the start, they wont be usable
> > as I2C pins for normal transfers?
>
>
> Right. I was also hoping to rid of cpu_is_xxx usage. The only other way
> I could think of is to add pinmux index into i2c platform data struct.
> What do you think is the best approach?
>
I think passing pinmux index through platform data is fair.
Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
