Messages in this thread |  | | Subject | Re: [PATCH 2/2] leds: lm3692x: Allow to set ovp and brigthness mode | From | Dan Murphy <> | Date | Tue, 17 Dec 2019 06:53:45 -0600 |
| |
Guido
On 12/16/19 6:28 AM, Guido Günther wrote: > Overvoltage protection and brightness mode are currently hardcoded > as disabled in the driver. Make these configurable via DT.
Can we split these up to two separate patch series?
We are adding 2 separate features and if something is incorrect with one of the changes it is a bit hard to debug.
> > Signed-off-by: Guido Günther <agx@sigxcpu.org> > --- > drivers/leds/leds-lm3692x.c | 43 +++++++++++++++++++++++++++++++------ > 1 file changed, 37 insertions(+), 6 deletions(-) > > diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c > index 8b408102e138..2c084b333628 100644 > --- a/drivers/leds/leds-lm3692x.c > +++ b/drivers/leds/leds-lm3692x.c > @@ -114,6 +114,7 @@ struct lm3692x_led { > struct regulator *regulator; > int led_enable; > int model_id; > + u8 boost_ctrl, brightness_ctrl; > }; > > static const struct reg_default lm3692x_reg_defs[] = { > @@ -249,10 +250,7 @@ static int lm3692x_init(struct lm3692x_led *led) > if (ret) > goto out; > > - ret = regmap_write(led->regmap, LM3692X_BOOST_CTRL, > - LM3692X_BOOST_SW_1MHZ | > - LM3692X_BOOST_SW_NO_SHIFT | > - LM3692X_OCP_PROT_1_5A); > + ret = regmap_write(led->regmap, LM3692X_BOOST_CTRL, led->boost_ctrl); > if (ret) > goto out;
regmap_update_bits
> > @@ -268,8 +266,7 @@ static int lm3692x_init(struct lm3692x_led *led) > if (ret) > goto out; > > - ret = regmap_write(led->regmap, LM3692X_BRT_CTRL, > - LM3692X_BL_ADJ_POL | LM3692X_RAMP_EN); > + ret = regmap_write(led->regmap, LM3692X_BRT_CTRL, led->brightness_ctrl); > if (ret) > goto out; regmap_update_bits > > @@ -326,6 +323,8 @@ static int lm3692x_probe_dt(struct lm3692x_led *led) > { > struct fwnode_handle *child = NULL; > struct led_init_data init_data = {}; > + u32 ovp = 0; > + bool exp_mode; > int ret; > > led->enable_gpio = devm_gpiod_get_optional(&led->client->dev, > @@ -350,6 +349,38 @@ static int lm3692x_probe_dt(struct lm3692x_led *led) > led->regulator = NULL; > } > > + led->boost_ctrl = LM3692X_BOOST_SW_1MHZ | > + LM3692X_BOOST_SW_NO_SHIFT | > + LM3692X_OCP_PROT_1_5A; Make this a #define and then it can be reused as a mask for regmap_update_bits > + ret = device_property_read_u32(&led->client->dev, > + "ti,overvoltage-volts", &ovp); > + if (!ret) {
if (ret)
set boost_ctrl to default value since the default is not 0
led->boost_ctrl |= LM3692X_OVP_29V;
else
do case
> + switch (ovp) { > + case 0: > + break; > + case 22: If the value is 21v why is this case 22? DT binding says 21 is the first value > + led->boost_ctrl |= LM3692X_OVP_21V; > + break; > + case 25: > + led->boost_ctrl |= LM3692X_OVP_25V; > + break; > + case 29: > + led->boost_ctrl |= LM3692X_OVP_29V; > + break; > + default: > + dev_err(&led->client->dev, "Invalid OVP %d\n", ovp); > + return -EINVAL; > + } > + } > + dev_dbg(&led->client->dev, "OVP: %dV", ovp); > + extra debug statement > + led->brightness_ctrl = LM3692X_BL_ADJ_POL | LM3692X_RAMP_EN; Same comment as before on the #define > + exp_mode = device_property_read_bool(&led->client->dev, > + "ti,brightness-mapping-exponential"); > + dev_dbg(&led->client->dev, "Exponential brightness: %d", exp_mode);
extra debug statement
Dan
|  |