On Sat, Mar 16, 2013 at 08:36:51AM -0400, Eduardo Valentin wrote: > >But that said, I don't care for the RMW_BITS() very much as a long > >term thing. If we just used pointers instead of passing the offset > >into the bg_ptr->conf->sensors[] array then everything would be a > >lot cleaner. > > > >In other words, instead of this: > > > >static u32 omap_bandgap_read_temp(struct omap_bandgap *bg_ptr, int id) > > > >We would have: > > > >static u32 omap_bandgap_read_temp(struct omap_bandgap *bg_ptr, > > struct temp_sensor_registers *tsr) > > I see. That will require a change in the whole driver though. If you > see, the driver as of today uses the former approach, not only for > read_temp or rmw_bits. Yep. > > > > >If you have the pointer then it's easy write RMW_BITS() as a > >function. > > > >static void rmw_bits(struct omap_bandgap *bg_ptr, u32 reg, u32 mask, u32 val) > >{ > > u32 r; > > > > r = omap_bandgap_readl(bg_ptr, reg); > > r &= ~mask; > > r |= val << __ffs(mask); > > omap_bandgap_writel(bg_ptr, r, reg); > >} > > > >It's called like: > > > > rmw_bits(bg_ptr, tsr->bgap_mask_ctrl, tsr->mask_freeze_mask, 1); > > This is nice, but it will require fetching tsr from .conf before > every call o rmw_bits. And for that you need the sensor index. > No. I'm suggesting that you re-write the driver to pass the tsr pointer directly instead of the index. regards, dan carpenter -- 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