Hi Dan, Thank you for the update. On 8/20/19 9:53 PM, Dan Murphy wrote: > Fix the brightness control for I2C mode. Instead of > changing the full scale current register update the ALS target > register for the appropriate banks. > > In addition clean up some code errors and random misspellings found > during coding. > > Tested on Droid4 as well as LM3532 EVM connected to a BeagleBoneBlack > > Fixes: e37a7f8d77e1 ("leds: lm3532: Introduce the lm3532 LED driver") > Reported-by: Pavel Machek <pavel@xxxxxx> > Signed-off-by: Dan Murphy <dmurphy@xxxxxx> > --- > > v3 - Removed register define updates - https://lore.kernel.org/patchwork/patch/1114542/ > > drivers/leds/leds-lm3532.c | 44 ++++++++++++++++++++++++++------------ > 1 file changed, 30 insertions(+), 14 deletions(-) > > diff --git a/drivers/leds/leds-lm3532.c b/drivers/leds/leds-lm3532.c > index 646100724971..8132d05f7add 100644 > --- a/drivers/leds/leds-lm3532.c > +++ b/drivers/leds/leds-lm3532.c > @@ -38,6 +38,9 @@ > #define LM3532_REG_ZN_2_LO 0x65 > #define LM3532_REG_ZN_3_HI 0x66 > #define LM3532_REG_ZN_3_LO 0x67 > +#define LM3532_REG_ZONE_TRGT_A 0x70 > +#define LM3532_REG_ZONE_TRGT_B 0x75 > +#define LM3532_REG_ZONE_TRGT_C 0x7a > #define LM3532_REG_MAX 0x7e > > /* Contorl Enable */ > @@ -116,6 +119,7 @@ struct lm3532_als_data { > * @priv - Pointer the device data structure > * @control_bank - Control bank the LED is associated to > * @mode - Mode of the LED string > + * @ctrl_brt_pointer - Zone target register that controls the sink > * @num_leds - Number of LED strings are supported in this array > * @led_strings - The LED strings supported in this array > * @label - LED label > @@ -126,6 +130,7 @@ struct lm3532_led { > > int control_bank; > int mode; > + int ctrl_brt_pointer; > int num_leds; > u32 led_strings[LM3532_MAX_CONTROL_BANKS]; > char label[LED_MAX_NAME_SIZE]; > @@ -339,8 +344,8 @@ static int lm3532_brightness_set(struct led_classdev *led_cdev, > if (ret) > goto unlock; > > - brightness_reg = LM3532_REG_CTRL_A_BRT + led->control_bank * 2; > - brt_val = brt_val / LM3532_BRT_VAL_ADJUST; > + brightness_reg = LM3532_REG_ZONE_TRGT_A + led->control_bank * 5 + > + (led->ctrl_brt_pointer >> 2); > > ret = regmap_write(led->priv->regmap, brightness_reg, brt_val); > > @@ -356,8 +361,30 @@ static int lm3532_init_registers(struct lm3532_led *led) > unsigned int output_cfg_val = 0; > unsigned int output_cfg_shift = 0; > unsigned int output_cfg_mask = 0; > + unsigned int brightness_config_reg; > + unsigned int brightness_config_val; > int ret, i; > > + if (drvdata->enable_gpio) > + gpiod_direction_output(drvdata->enable_gpio, 1); > + > + brightness_config_reg = LM3532_REG_ZONE_CFG_A + led->control_bank * 2; > + /* > + * This could be hard coded to the default value but the control > + * brightness register may have changed during boot. > + */ > + ret = regmap_read(drvdata->regmap, brightness_config_reg, > + &led->ctrl_brt_pointer); > + if (ret) > + return ret; > + > + led->ctrl_brt_pointer &= LM3532_ZONE_MASK; > + brightness_config_val = led->ctrl_brt_pointer | led->mode; > + ret = regmap_write(drvdata->regmap, brightness_config_reg, > + brightness_config_val); > + if (ret) > + return ret; > + > for (i = 0; i < led->num_leds; i++) { > output_cfg_shift = led->led_strings[i] * 2; > output_cfg_val |= (led->control_bank << output_cfg_shift); > @@ -382,7 +409,6 @@ static int lm3532_als_configure(struct lm3532_data *priv, > struct lm3532_als_data *als = priv->als_data; > u32 als_vmin, als_vmax, als_vstep; > int zone_reg = LM3532_REG_ZN_0_HI; > - int brightnes_config_reg; > int ret; > int i; > > @@ -411,14 +437,7 @@ static int lm3532_als_configure(struct lm3532_data *priv, > als->config = (als->als_avrg_time | (LM3532_ENABLE_ALS) | > (als->als_input_mode << LM3532_ALS_SEL_SHIFT)); > > - ret = regmap_write(priv->regmap, LM3532_ALS_CONFIG, als->config); > - if (ret) > - return ret; > - > - brightnes_config_reg = LM3532_REG_ZONE_CFG_A + led->control_bank * 2; > - > - return regmap_update_bits(priv->regmap, brightnes_config_reg, > - LM3532_I2C_CTRL, LM3532_ALS_CTRL); > + return regmap_write(priv->regmap, LM3532_ALS_CONFIG, als->config); > } > > static int lm3532_parse_als(struct lm3532_data *priv) > @@ -634,9 +653,6 @@ static int lm3532_probe(struct i2c_client *client, > return ret; > } > > - if (drvdata->enable_gpio) > - gpiod_direction_output(drvdata->enable_gpio, 1); > - > return ret; > } > > Patch set applied. -- Best regards, Jacek Anaszewski