[PATCH v2 1/4] leds: lm3532: Fix brightness control for i2c mode

[PATCH v2 1/4] leds: lm3532: Fix brightness control for i2c mode

[Dan Murphy]

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@ucw.cz>
Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v2 - Separated out random fixes and brightness control fix - https://lore.kernel.org/patchwork/patch/1109502/

 drivers/leds/leds-lm3532.c | 56 ++++++++++++++++++++++++--------------
 1 file changed, 36 insertions(+), 20 deletions(-)

diff --git a/drivers/leds/leds-lm3532.c b/drivers/leds/leds-lm3532.c
index 646100724971..b28b7f63198b 100644
--- a/drivers/leds/leds-lm3532.c
+++ b/drivers/leds/leds-lm3532.c
@@ -23,11 +23,11 @@
 #define LM3532_REG_PWM_B_CFG	0x14
 #define LM3532_REG_PWM_C_CFG	0x15
 #define LM3532_REG_ZONE_CFG_A	0x16
-#define LM3532_REG_CTRL_A_BRT	0x17
+#define LM3532_REG_CTRL_A_FS_CURR	0x17
 #define LM3532_REG_ZONE_CFG_B	0x18
-#define LM3532_REG_CTRL_B_BRT	0x19
+#define LM3532_REG_CTRL_B_FS_CURR	0x19
 #define LM3532_REG_ZONE_CFG_C	0x1a
-#define LM3532_REG_CTRL_C_BRT	0x1b
+#define LM3532_REG_CTRL_C_FS_CURR	0x1b
 #define LM3532_REG_ENABLE	0x1d
 #define LM3532_ALS_CONFIG	0x23
 #define LM3532_REG_ZN_0_HI	0x60
@@ -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];
@@ -168,11 +173,11 @@ static const struct reg_default lm3532_reg_defs[] = {
 	{LM3532_REG_PWM_B_CFG, 0x82},
 	{LM3532_REG_PWM_C_CFG, 0x82},
 	{LM3532_REG_ZONE_CFG_A, 0xf1},
-	{LM3532_REG_CTRL_A_BRT, 0xf3},
+	{LM3532_REG_CTRL_A_FS_CURR, 0x13},
 	{LM3532_REG_ZONE_CFG_B, 0xf1},
-	{LM3532_REG_CTRL_B_BRT, 0xf3},
+	{LM3532_REG_CTRL_B_FS_CURR, 0x13},
 	{LM3532_REG_ZONE_CFG_C, 0xf1},
-	{LM3532_REG_CTRL_C_BRT, 0xf3},
+	{LM3532_REG_CTRL_C_FS_CURR, 0x13},
 	{LM3532_REG_ENABLE, 0xf8},
 	{LM3532_ALS_CONFIG, 0x44},
 	{LM3532_REG_ZN_0_HI, 0x35},
@@ -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;
 }
 
-- 
2.22.0.214.g8dca754b1e

[PATCH v2 2/4] leds: lm3532: Fixes for the driver for stability

[Dan Murphy]

Fixed misspelled words, added error check during probe
on the init of the registers, and fixed ALS/I2C control
mode.

Fixes: e37a7f8d77e1 ("leds: lm3532: Introduce the lm3532 LED driver")
Reported-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v2 - New patch due to separating out random fixes and brightness control
fix - https://lore.kernel.org/patchwork/patch/1109502/

 drivers/leds/leds-lm3532.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/leds/leds-lm3532.c b/drivers/leds/leds-lm3532.c
index b28b7f63198b..ef4c74cbcdc0 100644
--- a/drivers/leds/leds-lm3532.c
+++ b/drivers/leds/leds-lm3532.c
@@ -43,7 +43,7 @@
 #define LM3532_REG_ZONE_TRGT_C	0x7a
 #define LM3532_REG_MAX		0x7e
 
-/* Contorl Enable */
+/* Control Enable */
 #define LM3532_CTRL_A_ENABLE	BIT(0)
 #define LM3532_CTRL_B_ENABLE	BIT(1)
 #define LM3532_CTRL_C_ENABLE	BIT(2)
@@ -307,7 +307,7 @@ static int lm3532_led_disable(struct lm3532_led *led_data)
 	int ret;
 
 	ret = regmap_update_bits(led_data->priv->regmap, LM3532_REG_ENABLE,
-					 ctrl_en_val, ~ctrl_en_val);
+					 ctrl_en_val, 0);
 	if (ret) {
 		dev_err(led_data->priv->dev, "Failed to set ctrl:%d\n", ret);
 		return ret;
@@ -326,7 +326,7 @@ static int lm3532_brightness_set(struct led_classdev *led_cdev,
 
 	mutex_lock(&led->priv->lock);
 
-	if (led->mode == LM3532_BL_MODE_ALS) {
+	if (led->mode == LM3532_ALS_CTRL) {
 		if (brt_val > LED_OFF)
 			ret = lm3532_led_enable(led);
 		else
@@ -561,11 +561,14 @@ static int lm3532_parse_node(struct lm3532_data *priv)
 		}
 
 		if (led->mode == LM3532_BL_MODE_ALS) {
+			led->mode = LM3532_ALS_CTRL;
 			ret = lm3532_parse_als(priv);
 			if (ret)
 				dev_err(&priv->client->dev, "Failed to parse als\n");
 			else
 				lm3532_als_configure(priv, led);
+		} else {
+			led->mode = LM3532_I2C_CTRL;
 		}
 
 		led->num_leds = fwnode_property_count_u32(child, "led-sources");
@@ -606,7 +609,13 @@ static int lm3532_parse_node(struct lm3532_data *priv)
 			goto child_out;
 		}
 
-		lm3532_init_registers(led);
+		ret = lm3532_init_registers(led);
+		if (ret) {
+			dev_err(&priv->client->dev, "register init err: %d\n",
+				ret);
+			fwnode_handle_put(child);
+			goto child_out;
+		}
 
 		i++;
 	}
-- 
2.22.0.214.g8dca754b1e

[PATCH v2 3/4] dt: lm3532: Add property for full scale current.

[Dan Murphy]

Add a property for each control bank to configure the
full scale current setting for the device.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v2 - Change ti,fs-current to led-max-microamp - https://lore.kernel.org/patchwork/patch/1109501/

 Documentation/devicetree/bindings/leds/leds-lm3532.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/leds/leds-lm3532.txt b/Documentation/devicetree/bindings/leds/leds-lm3532.txt
index c087f85ddddc..53793213dd52 100644
--- a/Documentation/devicetree/bindings/leds/leds-lm3532.txt
+++ b/Documentation/devicetree/bindings/leds/leds-lm3532.txt
@@ -62,6 +62,9 @@ Optional LED child properties:
 	- label : see Documentation/devicetree/bindings/leds/common.txt
 	- linux,default-trigger :
 	   see Documentation/devicetree/bindings/leds/common.txt
+	- led-max-microamp : Defines the full scale current value for each control
+			  bank.  The range is from 5000uA-29800uA in increments
+			  of 800uA.
 
 Example:
 led-controller@38 {
@@ -85,6 +88,7 @@ led-controller@38 {
 		reg = <0>;
 		led-sources = <2>;
 		ti,led-mode = <1>;
+		led-max-microamp = <21800>;
 		label = ":backlight";
 		linux,default-trigger = "backlight";
 	};
-- 
2.22.0.214.g8dca754b1e

[PATCH v2 4/4] leds: lm3532: Add full scale current configuration

[Dan Murphy]

Allow the full scale current to be configured at init.
Valid rangles are 5mA->29.8mA.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v2 - Change ti,fs-current to led-max-microamp - https://lore.kernel.org/patchwork/patch/1109503/

 drivers/leds/leds-lm3532.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/leds/leds-lm3532.c b/drivers/leds/leds-lm3532.c
index ef4c74cbcdc0..dabf4be1e8c8 100644
--- a/drivers/leds/leds-lm3532.c
+++ b/drivers/leds/leds-lm3532.c
@@ -89,6 +89,10 @@
 #define LM3532_NUM_AVG_VALS	8
 #define LM3532_NUM_IMP_VALS	32
 
+#define LM3532_FS_CURR_MIN	5000
+#define LM3532_FS_CURR_MAX	29800
+#define LM3532_FS_CURR_STEP	800
+
 /*
  * struct lm3532_als_data
  * @config - value of ALS configuration register
@@ -121,6 +125,7 @@ struct lm3532_als_data {
  * @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
+ * @full_scale_current - The full-scale current setting for the current sink.
  * @led_strings - The LED strings supported in this array
  * @label - LED label
  */
@@ -130,8 +135,9 @@ struct lm3532_led {
 
 	int control_bank;
 	int mode;
-	int ctrl_brt_pointer;
 	int num_leds;
+	int ctrl_brt_pointer;
+	int full_scale_current;
 	u32 led_strings[LM3532_MAX_CONTROL_BANKS];
 	char label[LED_MAX_NAME_SIZE];
 };
@@ -363,6 +369,8 @@ static int lm3532_init_registers(struct lm3532_led *led)
 	unsigned int output_cfg_mask = 0;
 	unsigned int brightness_config_reg;
 	unsigned int brightness_config_val;
+	int fs_current_reg;
+	int fs_current_val;
 	int ret, i;
 
 	if (drvdata->enable_gpio)
@@ -385,6 +393,16 @@ static int lm3532_init_registers(struct lm3532_led *led)
 	if (ret)
 		return ret;
 
+	if (led->full_scale_current) {
+		fs_current_reg = LM3532_REG_CTRL_A_FS_CURR + led->control_bank * 2;
+		fs_current_val = led->full_scale_current - LM3532_FS_CURR_MIN /
+				 LM3532_FS_CURR_STEP;
+		ret = regmap_write(drvdata->regmap, fs_current_reg,
+				   fs_current_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);
@@ -560,6 +578,12 @@ static int lm3532_parse_node(struct lm3532_data *priv)
 			goto child_out;
 		}
 
+		ret = fwnode_property_read_u32(child, "led-max-microamp",
+					       &led->full_scale_current);
+
+		if (led->full_scale_current > LM3532_FS_CURR_MAX)
+			led->full_scale_current = LM3532_FS_CURR_MAX;
+
 		if (led->mode == LM3532_BL_MODE_ALS) {
 			led->mode = LM3532_ALS_CTRL;
 			ret = lm3532_parse_als(priv);
-- 
2.22.0.214.g8dca754b1e

Re: [PATCH v2 1/4] leds: lm3532: Fix brightness control for i2c mode

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 801 bytes --]

On Tue 2019-08-13 13:11:51, 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@ucw.cz>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>

I may prefer register renames to come separately, but ...

Acked-by: Pavel Machek <pavel@ucw.cz>
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH v2 2/4] leds: lm3532: Fixes for the driver for stability

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 535 bytes --]

On Tue 2019-08-13 13:11:52, Dan Murphy wrote:
> Fixed misspelled words, added error check during probe
> on the init of the registers, and fixed ALS/I2C control
> mode.
> 
> Fixes: e37a7f8d77e1 ("leds: lm3532: Introduce the lm3532 LED driver")
> Reported-by: Pavel Machek <pavel@ucw.cz>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>

Acked-by: Pavel Machek <pavel@ucw.cz>
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH v2 3/4] dt: lm3532: Add property for full scale current.

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1025 bytes --]

On Tue 2019-08-13 13:11:53, Dan Murphy wrote:
> Add a property for each control bank to configure the
> full scale current setting for the device.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>

Acked-by: Pavel Machek <pavel@ucw.cz>


> @@ -62,6 +62,9 @@ Optional LED child properties:
>  	- label : see Documentation/devicetree/bindings/leds/common.txt
>  	- linux,default-trigger :
>  	   see Documentation/devicetree/bindings/leds/common.txt
> +	- led-max-microamp : Defines the full scale current value for each control
> +			  bank.  The range is from 5000uA-29800uA in increments
> +			  of 800uA.
>  
>  Example:
>  led-controller@38 {
> @@ -85,6 +88,7 @@ led-controller@38 {
>  		reg = <0>;
>  		led-sources = <2>;
>  		ti,led-mode = <1>;
> +		led-max-microamp = <21800>;
>  		label = ":backlight";
>  		linux,default-trigger = "backlight";
>  	};

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH v2 4/4] leds: lm3532: Add full scale current configuration

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1233 bytes --]

Hi!

> Allow the full scale current to be configured at init.
> Valid rangles are 5mA->29.8mA.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>

> @@ -121,6 +125,7 @@ struct lm3532_als_data {
>   * @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
> + * @full_scale_current - The full-scale current setting for the current sink.
>   * @led_strings - The LED strings supported in this array
>   * @label - LED label
>   */
> @@ -130,8 +135,9 @@ struct lm3532_led {
>  
>  	int control_bank;
>  	int mode;
> -	int ctrl_brt_pointer;
>  	int num_leds;
> +	int ctrl_brt_pointer;
> +	int full_scale_current;
>  	u32 led_strings[LM3532_MAX_CONTROL_BANKS];
>  	char label[LED_MAX_NAME_SIZE];
>  };

No need to move ctrl_brt_pointer... to keep order consistent with docs. 

> +		fs_current_val = led->full_scale_current - LM3532_FS_CURR_MIN /
> +				 LM3532_FS_CURR_STEP;

The computation is wrong ... needs () AFAICT.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH v2 4/4] leds: lm3532: Add full scale current configuration

[Dan Murphy]

Pavel

Thanks for the review

On 8/19/19 5:55 AM, Pavel Machek wrote:
> Hi!
>
>> Allow the full scale current to be configured at init.
>> Valid rangles are 5mA->29.8mA.
>>
>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>> @@ -121,6 +125,7 @@ struct lm3532_als_data {
>>    * @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
>> + * @full_scale_current - The full-scale current setting for the current sink.
>>    * @led_strings - The LED strings supported in this array
>>    * @label - LED label
>>    */
>> @@ -130,8 +135,9 @@ struct lm3532_led {
>>   
>>   	int control_bank;
>>   	int mode;
>> -	int ctrl_brt_pointer;
>>   	int num_leds;
>> +	int ctrl_brt_pointer;
>> +	int full_scale_current;
>>   	u32 led_strings[LM3532_MAX_CONTROL_BANKS];
>>   	char label[LED_MAX_NAME_SIZE];
>>   };
> No need to move ctrl_brt_pointer... to keep order consistent with docs.

OK I will reset the patches and get rid of that change.  I think this 
got moved when I applied the v1 patch.


>> +		fs_current_val = led->full_scale_current - LM3532_FS_CURR_MIN /
>> +				 LM3532_FS_CURR_STEP;
> The computation is wrong ... needs () AFAICT.

Hmm. Doesn't order of operations take precedence?

I will add the () unless checkpatch cribs about them

Dan


>
> Best regards,
> 									Pavel

Re: [PATCH v2 1/4] leds: lm3532: Fix brightness control for i2c mode

[Dan Murphy]

Pavel

On 8/19/19 5:48 AM, Pavel Machek wrote:
> On Tue 2019-08-13 13:11:51, 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@ucw.cz>
>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> I may prefer register renames to come separately, but ...

I can separate them into a different patch

Dan


> Acked-by: Pavel Machek <pavel@ucw.cz>
> 								Pavel

Re: [PATCH v2 4/4] leds: lm3532: Add full scale current configuration

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 792 bytes --]

Hi!

> >No need to move ctrl_brt_pointer... to keep order consistent with docs.
> 
> OK I will reset the patches and get rid of that change.  I think this got
> moved when I applied the v1 patch.
> 
> 
> >>+		fs_current_val = led->full_scale_current - LM3532_FS_CURR_MIN /
> >>+				 LM3532_FS_CURR_STEP;
> >The computation is wrong ... needs () AFAICT.
> 
> Hmm. Doesn't order of operations take precedence?
> 
> I will add the () unless checkpatch cribs about them

I may be misunderstanding. What do you expect the computation to be? /
has higher priority than -, right? Can you test it provides expected
results?

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH v2 4/4] leds: lm3532: Add full scale current configuration

[Dan Murphy]

Hello

On 8/20/19 11:29 AM, Pavel Machek wrote:
> Hi!
>
>>> No need to move ctrl_brt_pointer... to keep order consistent with docs.
>> OK I will reset the patches and get rid of that change.  I think this got
>> moved when I applied the v1 patch.
>>
>>
>>>> +		fs_current_val = led->full_scale_current - LM3532_FS_CURR_MIN /
>>>> +				 LM3532_FS_CURR_STEP;
>>> The computation is wrong ... needs () AFAICT.
>> Hmm. Doesn't order of operations take precedence?
>>
>> I will add the () unless checkpatch cribs about them
> I may be misunderstanding. What do you expect the computation to be? /
> has higher priority than -, right? Can you test it provides expected
> results?

Fixed.  I think this is what you expected.

fs_current_val = (led->full_scale_current - LM3532_FS_CURR_MIN) /
                             LM3532_FS_CURR_STEP;


> 									Pavel