On Fri, Jul 23, 2010 at 11:33 AM, Steve Sakoman <sako...@gmail.com> wrote:
> On Wed, Jul 21, 2010 at 4:53 PM, Nishanth Menon
> <menon.nisha...@gmail.com> wrote:
>
>>>>> diff --git a/drivers/i2c/omap24xx_i2c.c b/drivers/i2c/omap24xx_i2c.c
>>>>> index 3256133..a91fe55 100644
>>>>> --- a/drivers/i2c/omap24xx_i2c.c
>>>>> +++ b/drivers/i2c/omap24xx_i2c.c
>>>>> @@ -41,6 +41,7 @@ void i2c_init (int speed, int slaveadd)
>>>>> int psc, fsscll, fssclh;
>>>>> int hsscll = 0, hssclh = 0;
>>>>> u32 scll, sclh;
>>>>> + int reset_timeout = 10;
>>>>
>>>> I am guessing we can life with a u8, we timeout in 10ms.. would asking
>>>> for a #define I2C_TIMEOUT_RESET is too much here?
>>>
>>> I can't imagine why we would ever want to make this a config option!
>>>
>>> In normal operation the loop is only traversed one time. I only added
>>> the timeout to prevent the possibility of an infinite loop.
>>
>> not as a config option, but a #define in the C file so that it is readable,
>> the 10 makes not much sense at a glance.. but as I mentioned, it is just a
>> nitpick.. it just helps to quickly figure out which are the timeouts used in
>> the driver.. I mean we cant proceed with i2c_init or use it as i2c master
>> (aka no point in using the driver) if that happens in the system rt?
>
> OK, I will submit a version 2 with a #define for the timeout/retry count.
>
>>>>> - printf ("I2C read: addr len %d not supported\n", alen);
>>>>> + printf("I2C write: addr len %d not supported\n", alen);
>>>>
>>>> err.. do we need this change?
>>>
>>> I think so -- otherwise the i2c_write function is printing messages
>>> claiming to come from i2c_read! Same for the other messages you
>>> flagged.
>>>
>>
>> Apologies on not being clear - I dont have a problem with the print as such,
>> BUT, do we need a diff here which changes printf ("... to printf("... ?
>> makes no reason why I need that for reliable OMAP4 operation :D..
>
> The message is clearly wrong, but your point is well taken: an
> erroneous error message fix doesn't really belong in a patch for
> reliable operation on OMAP4.
>
> Version 2 of this patch will address only the reliability issues and
> your request for a timeout #define.
>
> I will then do a subsequent patch (not part of this series) to fix
> erroneous error messages, potential infinite loops, and a variety of
> whitespace and style issues.
alright.. thanks. looking for the v2 of this patch..
>
> Regards,
>
> Steve
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
