http://bugs.ecos.sourceware.org/show_bug.cgi?id=1000763
Andrew Lunn <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |[email protected] --- Comment #2 from Andrew Lunn <[email protected]> 2009-05-15 20:01:35 --- Hi Vibi It is good to see you don't attempt to implement an interrupt driver I2C driver on the AT91 platforms. That is know not to work. The copyright headers need updating to the latest version. In i2c_at91sam7x_init() you have: HAL_ARM_AT91_GPIO_CFG_PULLUP(AT91_GPIO_PA10,AT91_PIN_PULLUP_DISABLE); HAL_ARM_AT91_GPIO_CFG_PULLUP(AT91_GPIO_PA11,AT91_PIN_PULLUP_DISABLE); You should not have hard coded references to PA10 and PA11. These will not be valid for all AT91SAM devices. volatile cyg_uint32 stat_reg; volatile should not be needed. The macro HAL_READ_UINT32() ensures the value will be read from the register every time. Could you explain this in more detail: // calculate internal address while (i--) tmp |= *tx_data++ << (i << 3); I've no idea what it is doing. The following is not the most readable code: for ( timeout = TIMEOUT,stat_reg = 0; timeout && !(stat_reg & tmp); timeout-- ) I2C_R32 (AT91_TWI_SR,stat_reg); // error condition , return how much data was transferred if (!timeout) return (count - i2c_count); Maybe tmp should be called sr_mask? timeout is not a timeout, it more of a count down. This makes it easy to misread if (!timeout). Most people would think !timeout is being the good case, when in fact it is an error! I would prefer to see this section of code re-written to make it easier to understand. Consider: } while (--i2c_count); return (count - i2c_count); } There is no break in the do {} while loop. So if you got out of the loop it means i2c_count must be 0. So that makes the subtraction for the return value does nothing. There are some indentation issues it would also be nice to clean up, but lets get the other problems addressed first. Some AT91 device have more than one TWI bus, eg the AT91SAM9X devices. Maybe you could think how to handle this? However it is not too important. Andrew -- 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.
