"Menon, Nishanth" <nm@xxxxxx> writes: > On Thu, Jun 2, 2011 at 18:45, Kevin Hilman <khilman@xxxxxx> wrote: >> Nishanth Menon <nm@xxxxxx> writes: >> >>> Due to a TRM bug, the current code assumes that channel0(core) is the default >>> channel. With the additional explanation provided by the hardware team, it is >>> clear that PRM_VC_CFG_CHANNEL register allows for flexibility of configuring >>> for various PMIC configurations. For example, the I2C slave address on TWL6030 >>> when using 4430 configuration could potentially use the same slave address for >>> all domains, while in 4460 configuration, we drive MPU using TPS; Core and IVA >>> using TWL6030. To allow this flexibility, we state in existing parameters using >>> a flag to indicate usage of default channel. >>> >>> In addition, the TRM update clears up the confusion on the fact that MPU is >>> infact the default channel on OMAP4. >>> >>> We update the same here. >>> >>> Signed-off-by: Nishanth Menon <nm@xxxxxx> >> >> There's too much going on in this patch not described in the changelog >> (extra error checking, volt & cmd reg checking etc.) >> >> Also, I don't like the USE_DEFAULT_CHANNEL_I2C_PARAM, mainly because it >> adds clutter without any obvious value. ÂTo me, zero is a perfectly good >> value to signify "use default channel value", especially since that's >> the value used by the hardware. > The reason is that 0 is a valid i2c register address. 8 bits is used > in VC and I wanted one bit which was further away, hence the 16 bit. > The usage could be that in .volt_reg_addr or in .cmd_reg_addr I could > set this up with the macro. and bingo, we use the default domain's > configuration. > > This is esp useful if we had a single pmic reg for all domains.. I > mean the VC design allows for it, even though we dont use it > currently. OK. See, it's easy to convince me that something is needed/useful. Of course, I prefer to be conviced by the changelog. :) Please make this feature into a dedicated patch with an appropriately descriptive changelog. Thanks. >> >> Incidentally, adding this doesn't actually change current behavior. >> Currently, I use zero (an unconfigured value in the VC settings) to >> signify it should use default settings. ÂIn your patch, you defined >> USE_DEFAULT_CHANNEL_I2C_PARAM as 0x10000 (17 bits) but assign it to 16 >> bit fields, which means they are just set to zero. :) > > >> So rather than take this patch, I'm just going to fold the following >> diff into "OMAP3+: VC: abstract out channel configuration" in voltdm_b. >> I'll also update the changelog noting that the TRM is wrong here in that >> it describes CORE as the default channel when it's in fact MPU. > > I intend to post a new series including my PMIC changes as well early > next week(mondayish hopefully). Can we hold off review of any further > of my voltage fixes patches till then? > Sure. It's the first time anyone as asked me not to review patches. :) I'll gladly comply. I've already folded the minimal change I proposed into the original patch locally, and will push updated voltdm_* branches by the end of today. Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
