On Fri, Mar 15, 2013 at 08:59:55AM -0400, Eduardo Valentin wrote:
> This patch introduce a macro to read, update, write bitfields.
> It will be specific to bandgap data structures.
>
> Signed-off-by: Eduardo Valentin <eduardo.valentin@xxxxxx>
> ---
> drivers/staging/omap-thermal/omap-bandgap.c | 178 +++++++--------------------
> 1 files changed, 46 insertions(+), 132 deletions(-)
>
> diff --git a/drivers/staging/omap-thermal/omap-bandgap.c b/drivers/staging/omap-thermal/omap-bandgap.c
> index 9f2d7cc..1c1b905 100644
> --- a/drivers/staging/omap-thermal/omap-bandgap.c
> +++ b/drivers/staging/omap-thermal/omap-bandgap.c
> @@ -52,25 +52,29 @@ static void omap_bandgap_writel(struct omap_bandgap *bg_ptr, u32 val, u32 reg)
> writel(val, bg_ptr->base + reg);
> }
>
> +/* update bits, value will be shifted */
> +#define RMW_BITS(bg_ptr, id, reg, mask, val) \
> +do { \
> + struct temp_sensor_registers *t; \
> + u32 r; \
> + \
> + t = bg_ptr->conf->sensors[(id)].registers; \
> + r = omap_bandgap_readl(bg_ptr, t->reg); \
> + r &= ~t->mask; \
> + r |= (val) << __ffs(t->mask); \
> + omap_bandgap_writel(bg_ptr, r, t->reg); \
> +} while (0)
> +
> static int omap_bandgap_power(struct omap_bandgap *bg_ptr, bool on)
> {
> - struct temp_sensor_registers *tsr;
> int i;
> - u32 ctrl;
>
> if (!OMAP_BANDGAP_HAS(bg_ptr, POWER_SWITCH))
> return 0;
>
> - for (i = 0; i < bg_ptr->conf->sensor_count; i++) {
> - tsr = bg_ptr->conf->sensors[i].registers;
> - ctrl = omap_bandgap_readl(bg_ptr, tsr->temp_sensor_ctrl);
> - ctrl &= ~tsr->bgap_tempsoff_mask;
> + for (i = 0; i < bg_ptr->conf->sensor_count; i++)
> /* active on 0 */
> - ctrl |= !on << __ffs(tsr->bgap_tempsoff_mask);
> -
> - /* write BGAP_TEMPSOFF should be reset to 0 */
> - omap_bandgap_writel(bg_ptr, ctrl, tsr->temp_sensor_ctrl);
> - }
> + RMW_BITS(bg_ptr, i, temp_sensor_ctrl, bgap_tempsoff_mask, !on);
>
> return 0;
> }
> @@ -78,15 +82,13 @@ static int omap_bandgap_power(struct omap_bandgap *bg_ptr, bool on)
> static u32 omap_bandgap_read_temp(struct omap_bandgap *bg_ptr, int id)
> {
This patch is fine and it makes it cleaner than before.
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)
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);
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
