This is the mail archive of the ecos-patches@sourceware.org mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[Bug 1000763] I2C Driver for at91sam7x


http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000763





--- Comment #7 from Andrew Lunn <andrew.lunn@ascom.ch>  2009-05-19 08:53:42 ---
The code looks better. Thanks for the changes. I have a few more minor
requests/questions.

    // enable multi-drain
    HAL_WRITE_UINT32((AT91_PIOA + AT91_PIO_MDER),(AT91_PIO_PSR_TWD
        |AT91_PIO_PSR_TWCK));

To make the code more portable to other AT91 platforms, it would be better to
add a HAL_ARM_AT91_GPIO_MULTIDRAIN(__pin__, __enable__) to var_io.h following
the HAL_ARM_AT91_GPIO_CFG_PULLUP macro.

HAL_ARM_AT91_GPIO_CFG_PULLUP() takes an AT91_PIN macro, not AT91_PIO_PSR_*
macro.

These macros are important when the pin is actually on PIOB.

As you suggested, breaking up the for loop for waiting for completion would be
nice. I find the following much more readable (note: Untested):

    for (success = COUNT_DOWN, success, success--) {
        IC2_R32(AT91_TWI_SR,stat_reg);
        if (stat_req & sr_mask)
            break;
    }
    // error condition ,ie count down ended before txr complete flag is set
    // return how much data was transferred
    if (!success)
        break;


Before commiting to anoncvs your cyg_i2c_at91_fram_dev needs removing from both
the .c and .h file.

Apart from these comments the code looks good.


-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]