On Mon 02 Mar 04:55 PST 2020, Kiran Gunda wrote: > diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c [..] > @@ -147,14 +187,39 @@ struct wled { > u32 max_brightness; > u32 short_count; > u32 auto_detect_count; > + u32 version; > bool disabled_by_short; > bool has_short_detect; > + bool cabc_disabled; > int short_irq; > int ovp_irq; > > struct wled_config cfg; > struct delayed_work ovp_work; > int (*wled_set_brightness)(struct wled *wled, u16 brightness); > + int (*cabc_config)(struct wled *wled, bool enable); > + int (*wled_sync_toggle)(struct wled *wled); Please split this patch in one that adds these and breaks out the wled3 support, and then a second patch that adds wled5. > +}; > + [..] > +static int wled5_set_brightness(struct wled *wled, u16 brightness) > +{ > + int rc, offset; > + u16 low_limit = wled->max_brightness * 1 / 1000; > + u8 v[2], brightness_msb_mask; > + > + /* WLED5's lower limit is 0.1% */ > + if (brightness > 0 && brightness < low_limit) > + brightness = low_limit; > + > + brightness_msb_mask = 0xf; > + if (wled->max_brightness == WLED5_SINK_REG_BRIGHT_MAX_15B) > + brightness_msb_mask = 0x7f; Why not just brightness &= wled->max_brightness? But given that it seems like the framework ensures that brightness <= max_brightness, why not skip this altogether? > + > + v[0] = brightness & 0xff; > + v[1] = (brightness >> 8) & brightness_msb_mask; > + > + offset = wled5_brightness_reg[wled->cfg.mod_sel]; > + rc = regmap_bulk_write(wled->regmap, wled->sink_addr + offset, > + v, 2); > + return rc; > +} > + > static int wled4_set_brightness(struct wled *wled, u16 brightness) > { > int rc, i; > @@ -237,7 +325,28 @@ static int wled_module_enable(struct wled *wled, int val) > return 0; > } > > -static int wled_sync_toggle(struct wled *wled) > +static int wled5_sync_toggle(struct wled *wled) > +{ > + int rc; > + u8 val; > + > + val = (wled->cfg.mod_sel == MOD_A) ? WLED5_SINK_REG_SYNC_MOD_A_BIT : > + WLED5_SINK_REG_SYNC_MOD_B_BIT; > + rc = regmap_update_bits(wled->regmap, > + wled->sink_addr + WLED5_SINK_REG_MOD_SYNC_BIT, > + WLED5_SINK_REG_SYNC_MASK, val); > + if (rc < 0) > + return rc; > + > + val = 0; Just plug 0 in the function call. > + rc = regmap_update_bits(wled->regmap, > + wled->sink_addr + WLED5_SINK_REG_MOD_SYNC_BIT, > + WLED5_SINK_REG_SYNC_MASK, val); > + > + return rc; And return regmap_update_bits(...); > +} > + Regards, Bjorn _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel