[U-Boot] [PATCH v2] OMAP24xx I2C: Add support for 'setspeed'

Wolfgang Denk wd at denx.de
Mon Feb 3 20:06:13 CET 2014


Dear Hannes,

In message <1391444250-29430-1-git-send-email-oe5hpm at oevsv.at> you wrote:
> changes to omap24_i2c_write(...) for polling ARDY Bit from IRQ-Status.
> Otherwise on a subsequent call the transfer of last byte from the
> predecessor is aborted and therefore lost. For exmaple when
> i2c_write(...) is followed by a i2c_setspeed(...) (which has to
> deactivate and activate master for changing psc,...).
> 
> Signed-off-by: Hannes Petermaier <oe5hpm at oevsv.at>
> ---
> Changes for v2:
>    - fixed compile error due to '= ='
>    - removed [PATCH 1/2]: only 1 patch is needed
>    - fixed omap24_i2c_write(...) for waiting until all transfer is finished.

General note: it is always a wise idea to add the responsible custodian
(here Heiko) on Cc: (done here).  [You can automate this by adding a
Cc: line below you Signed-off-by: entry - then git-send-email will
automatically do what is needed.

> +	/* some divisors may cause a precission loss, but shouldn't
> +	 * be a big thing, because i2c_clk is then allready very slow.
> +	 */

This is incorrect multi-line comment format;  it should look like
this:

	/*
	 * some divisors may cause a precission loss, but shouldn't
	 * be a big thing, because i2c_clk is then allready very slow.
	 */

Please fix globally.

> +	omap24_i2c_setspeed(adap, speed);

omap24_i2c_setspeed() returns int, and can return error codes.  These
should be handled - here, and everywhere else where the function is
being called.

> +	/* poll ARDY bit for making sure that last byte really has been
> +	 * transferred on the bus.
> +	 */
> +	do {
> +		status = wait_for_event(adap);
> +	} while (!(status & I2C_STAT_ARDY));

This is potentially an endless loop.  Please make sure that it will
time out (and then issue an error message).

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
What we think, or what we know, or what we believe, is in the end, of
little consequence. The only thing of consequence is what we do.
                                                        - John Ruskin


More information about the U-Boot mailing list