On 10/22/2012 02:28 PM, Felipe Balbi wrote: > Hi, > > On Mon, Oct 22, 2012 at 02:27:20PM +0200, Benoit Cousson wrote: >> Hi Felipe, >> >> On 10/22/2012 11:46 AM, Felipe Balbi wrote: >>> on OMAP4+ we want to read IRQSTATUS_RAW register, >>> instead of IRQSTATUS. The reason being that IRQSTATUS >>> will only contain the bits which were enabled on >>> IRQENABLE_SET and that will break when we need to >>> poll for a certain bit which wasn't enabled as an >>> IRQ source. >>> >>> One such case is after we finish converting to >>> deferred stop bit, we will have to poll for ARDY >>> bit before returning control for the client driver >>> in order to prevent us from trying to start a >>> transfer on a bus which is already busy. >>> >>> Note, however, that omap-i2c.c needs a big rework >>> on register definition and register access. Such >>> work will be done in a separate series of patches. >> >> Do you need OMAP_I2C_IP_VERSION_3 for OMAP4? >> >> OMAP_I2C_IP_VERSION_2 was already introduced to detect OMAP3630 vs >> omap4430 because they were sharing the same IP version.. >> >> /* I2C controller revisions present on specific hardware */ >> #define OMAP_I2C_REV_ON_2430 0x36 >> #define OMAP_I2C_REV_ON_3430_3530 0x3C >> #define OMAP_I2C_REV_ON_3630_4430 0x40 >> >> So in theory you should not need an extra version. > > are you sure that's how they're used ? Looking at the code where we > choose reg_map_ip_v1 and reg_map_ip_v2: > > 1120 if (dev->dtrev == OMAP_I2C_IP_VERSION_2) > 1121 dev->regs = (u8 *)reg_map_ip_v2; > 1122 else > 1123 dev->regs = (u8 *)reg_map_ip_v1; > > And looking at reg_map_ip_v1: > > 218 static const u8 reg_map_ip_v1[] = { > 219 [OMAP_I2C_REV_REG] = 0x00, > 220 [OMAP_I2C_IE_REG] = 0x01, > 221 [OMAP_I2C_STAT_REG] = 0x02, > 222 [OMAP_I2C_IV_REG] = 0x03, > 223 [OMAP_I2C_WE_REG] = 0x03, > 224 [OMAP_I2C_SYSS_REG] = 0x04, > 225 [OMAP_I2C_BUF_REG] = 0x05, > 226 [OMAP_I2C_CNT_REG] = 0x06, > 227 [OMAP_I2C_DATA_REG] = 0x07, > 228 [OMAP_I2C_SYSC_REG] = 0x08, > 229 [OMAP_I2C_CON_REG] = 0x09, > 230 [OMAP_I2C_OA_REG] = 0x0a, > 231 [OMAP_I2C_SA_REG] = 0x0b, > 232 [OMAP_I2C_PSC_REG] = 0x0c, > 233 [OMAP_I2C_SCLL_REG] = 0x0d, > 234 [OMAP_I2C_SCLH_REG] = 0x0e, > 235 [OMAP_I2C_SYSTEST_REG] = 0x0f, > 236 [OMAP_I2C_BUFSTAT_REG] = 0x10, > 237 }; > > that's really not the register map on OMAP3. That only looks valid for > OMAP1. No that should be valid as well for OMAP2&3 thanks to the OMAP_I2C_FLAG_BUS_SHIFT_2. It was introduced during DT adaptation: #ifdef CONFIG_OF static struct omap_i2c_bus_platform_data omap3_pdata = { .rev = OMAP_I2C_IP_VERSION_1, .flags = OMAP_I2C_FLAG_APPLY_ERRATA_I207 | OMAP_I2C_FLAG_RESET_REGS_POSTIDLE | OMAP_I2C_FLAG_BUS_SHIFT_2, }; static struct omap_i2c_bus_platform_data omap4_pdata = { .rev = OMAP_I2C_IP_VERSION_2, }; static const struct of_device_id omap_i2c_of_match[] = { { .compatible = "ti,omap4-i2c", .data = &omap4_pdata, }, { .compatible = "ti,omap3-i2c", .data = &omap3_pdata, }, { }, }; MODULE_DEVICE_TABLE(of, omap_i2c_of_match); #endif > -ECONFUSED Well, it is confusing... Benoit -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html