[U-Boot] [PATCH V5] ARM: OMAP: I2C: New read, write and probe functions

[Lubomir Popov]

> Hello Lubomir,
>
> Am 01.06.2013 18:44, schrieb Lubomir Popov:
>> New i2c_read, i2c_write and i2c_probe functions, tested on OMAP4
>> (4430/60/70), OMAP5 (5430) and AM335X (3359); should work on older
>> OMAPs and derivatives as well. The only anticipated exception would
>> be the OMAP2420, which shall require driver modification.
>> - Rewritten i2c_read to operate correctly with all types of chips
>>   (old function could not read consistent data from some I2C slaves).
>> - Optimised i2c_write.
>> - New i2c_probe, performs write access vs read. The old probe could
>>   hang the system under certain conditions (e.g. unconfigured pads).
>> - The read/write/probe functions try to identify unconfigured bus.
>> - Status functions now read irqstatus_raw as per TRM guidelines
>>   (except for OMAP243X and OMAP34XX).
>> - Driver now supports up to I2C5 (OMAP5).
>>
>> Signed-off-by: Lubomir Popov <lpopov at mm-sol.com>
>> ---
>> V5 changes:
>> - Replaced some printf() with puts().
>> - Minor formatting touches, checkpatch-clean.
>> V4 changes:
>> - New i2c_probe is built unconditionally, old code is removed.
>>   CONFIG_I2C_PROBE_WRITE is no longer needed.
>> - Added a small delay to work around breakage in AM335X SPL.
>> - Some whitespace and general formatting cleanup.
>> V3 changes:
>> - Removed old functions and conditional compilation. New functions
>>   are now built unconditionally for all SoCs using this driver. The
>>   only chip that should break is the OMAP2420 dinosaur.
>> - Interrupts are enabled for OMAP243X and OMAP34XX only (where we
>>   don't have an irqstatus_raw register).
>> V2 changes:
>> - Probe tries to identify misconfigured pads as well.
>> - Status is retrieved from irqstatus_raw rather than from stat.
>> - Some minor style & format changes.
>>
>>  drivers/i2c/omap24xx_i2c.c | 490 +++++++++++++++++++++++++++------------------
>>  1 file changed, 299 insertions(+), 191 deletions(-)
>
> Tested on 3 arm335x based boards, which one uses i2c in SPL
> code for getting ram parameters, so:
>
> Tested-by: Heiko Schocher <hs at denx.de>
Many thanks for testing, to Tom as well (he did it on the
Beagleboards, but for one of the older versions, V3 I believe,
right?).
When it comes to versions, I see that V1 and V2 are still listed
in patchwork, probably because of slightly different wording of
the subject:
http://patchwork.ozlabs.org/patch/233823/
http://patchwork.ozlabs.org/patch/246204/
Could you or Tom clean this up, please? Thanks.

>
> Just one comment:
> Your patch has 9 checkpatch warnings which are all lines
> (printf strings) over 80 chars ... some with lines > 110
> characters ... I know, tom gave you a OK for this ... I am
> also unhappy with splitting a printf-string over 2 or more lines ...
> but we have this 80 characters rule ... Wolfgang, what do you
> think? Should we loosen this rule for printf-strings?
Yes, I had the long strings splitted in the older versions, but
then unrolled them back as per Tom's recommendation. IMHO, grep-ability
is worth breaking this particular rule... But perhaps only for pure
strings w/o format placeholders? I mean, strings could be splitted
at the format parameters of printf/sprintf arguments.
>
> bye,
> Heiko
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>
Best regards,
Lubo