Re: [PATCH v2] leds: lm3697: Rename struct into more appropriate name

[Pavel Machek]

On Fri 2020-10-09 15:51:35, Gabriel David wrote:
> The mentioned struct, lm3697_led, was renamed to lm3697_bank since the
> structure is representing the control banks. This name, in my opinion,
> is more semantically correct. The pointers referring to it were also
> renamed.

> Signed-off-by: Gabriel David <ultracoolguy4@xxxxxxxxxxxxxx>
> ---
> Yes, this is the same Gabriel David from ultracoolguy@xxxxxxxxxxxx and
> ultracoolguy@xxxxxxxxxxx. If you want me to confirm it I'll gladly do
> it.

No problem with that, and no need to resend. This can proably wait
for 5.11...

I'd like some comment from Dan... and perhaps I'd want to understand
what the difference between LED and bank is.

...there can be more than one LED connected to the given bank, that's
what you are pointing out?

...but these LEDs will always work in unison, and they are handled as
single LED by Linux, right?

Best regards,
								Pavel

>  drivers/leds/leds-lm3697.c | 90 +++++++++++++++++++-------------------
>  1 file changed, 45 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/leds/leds-lm3697.c b/drivers/leds/leds-lm3697.c
> index 31f5ed486839..c62f95fc17e8 100644
> --- a/drivers/leds/leds-lm3697.c
> +++ b/drivers/leds/leds-lm3697.c
> @@ -39,7 +39,7 @@
>  #define LM3697_MAX_CONTROL_BANKS 2
> 
>  /**
> - * struct lm3697_led -
> + * struct lm3697_bank -
>   * @hvled_strings: Array of LED strings associated with a control bank
>   * @label: LED label
>   * @led_dev: LED class device
> @@ -48,7 +48,7 @@
>   * @control_bank: Control bank the LED is associated to. 0 is control bank A
>   *		   1 is control bank B
>   */
> -struct lm3697_led {
> +struct lm3697_bank {
>  	u32 hvled_strings[LM3697_MAX_LED_STRINGS];
>  	char label[LED_MAX_NAME_SIZE];
>  	struct led_classdev led_dev;
> @@ -80,7 +80,7 @@ struct lm3697 {
>  	int bank_cfg;
>  	int num_banks;
> 
> -	struct lm3697_led leds[];
> +	struct lm3697_bank banks[];
>  };
> 
>  static const struct reg_default lm3697_reg_defs[] = {
> @@ -113,52 +113,52 @@ static const struct regmap_config lm3697_regmap_config = {
>  static int lm3697_brightness_set(struct led_classdev *led_cdev,
>  				enum led_brightness brt_val)
>  {
> -	struct lm3697_led *led = container_of(led_cdev, struct lm3697_led,
> +	struct lm3697_bank *bank = container_of(led_cdev, struct lm3697_bank,
>  					      led_dev);
> -	int ctrl_en_val = (1 << led->control_bank);
> +	int ctrl_en_val = (1 << bank->control_bank);
>  	int ret;
> 
> -	mutex_lock(&led->priv->lock);
> +	mutex_lock(&bank->priv->lock);
> 
>  	if (brt_val == LED_OFF) {
> -		ret = regmap_update_bits(led->priv->regmap, LM3697_CTRL_ENABLE,
> +		ret = regmap_update_bits(bank->priv->regmap, LM3697_CTRL_ENABLE,
>  					 ctrl_en_val, ~ctrl_en_val);
>  		if (ret) {
> -			dev_err(&led->priv->client->dev, "Cannot write ctrl register\n");
> +			dev_err(&bank->priv->client->dev, "Cannot write ctrl register\n");
>  			goto brightness_out;
>  		}
> 
> -		led->enabled = LED_OFF;
> +		bank->enabled = LED_OFF;
>  	} else {
> -		ret = ti_lmu_common_set_brightness(&led->lmu_data, brt_val);
> +		ret = ti_lmu_common_set_brightness(&bank->lmu_data, brt_val);
>  		if (ret) {
> -			dev_err(&led->priv->client->dev,
> +			dev_err(&bank->priv->client->dev,
>  				"Cannot write brightness\n");
>  			goto brightness_out;
>  		}
> 
> -		if (!led->enabled) {
> -			ret = regmap_update_bits(led->priv->regmap,
> +		if (!bank->enabled) {
> +			ret = regmap_update_bits(bank->priv->regmap,
>  						 LM3697_CTRL_ENABLE,
>  						 ctrl_en_val, ctrl_en_val);
>  			if (ret) {
> -				dev_err(&led->priv->client->dev,
> +				dev_err(&bank->priv->client->dev,
>  					"Cannot enable the device\n");
>  				goto brightness_out;
>  			}
> 
> -			led->enabled = brt_val;
> +			bank->enabled = brt_val;
>  		}
>  	}
> 
>  brightness_out:
> -	mutex_unlock(&led->priv->lock);
> +	mutex_unlock(&bank->priv->lock);
>  	return ret;
>  }
> 
>  static int lm3697_init(struct lm3697 *priv)
>  {
> -	struct lm3697_led *led;
> +	struct lm3697_bank *bank;
>  	int i, ret;
> 
>  	if (priv->enable_gpio) {
> @@ -182,8 +182,8 @@ static int lm3697_init(struct lm3697 *priv)
>  		dev_err(&priv->client->dev, "Cannot write OUTPUT config\n");
> 
>  	for (i = 0; i < priv->num_banks; i++) {
> -		led = &priv->leds[i];
> -		ret = ti_lmu_common_set_ramp(&led->lmu_data);
> +		bank = &priv->banks[i];
> +		ret = ti_lmu_common_set_ramp(&bank->lmu_data);
>  		if (ret)
>  			dev_err(&priv->client->dev, "Setting the ramp rate failed\n");
>  	}
> @@ -194,7 +194,7 @@ static int lm3697_init(struct lm3697 *priv)
>  static int lm3697_probe_dt(struct lm3697 *priv)
>  {
>  	struct fwnode_handle *child = NULL;
> -	struct lm3697_led *led;
> +	struct lm3697_bank *bank;
>  	const char *name;
>  	int control_bank;
>  	size_t i = 0;
> @@ -229,63 +229,63 @@ static int lm3697_probe_dt(struct lm3697 *priv)
>  			goto child_out;
>  		}
> 
> -		led = &priv->leds[i];
> +		bank = &priv->banks[i];
> 
>  		ret = ti_lmu_common_get_brt_res(&priv->client->dev,
> -						child, &led->lmu_data);
> +						child, &bank->lmu_data);
>  		if (ret)
>  			dev_warn(&priv->client->dev, "brightness resolution property missing\n");
> 
> -		led->control_bank = control_bank;
> -		led->lmu_data.regmap = priv->regmap;
> -		led->lmu_data.runtime_ramp_reg = LM3697_CTRL_A_RAMP +
> +		bank->control_bank = control_bank;
> +		bank->lmu_data.regmap = priv->regmap;
> +		bank->lmu_data.runtime_ramp_reg = LM3697_CTRL_A_RAMP +
>  						 control_bank;
> -		led->lmu_data.msb_brightness_reg = LM3697_CTRL_A_BRT_MSB +
> -						   led->control_bank * 2;
> -		led->lmu_data.lsb_brightness_reg = LM3697_CTRL_A_BRT_LSB +
> -						   led->control_bank * 2;
> +		bank->lmu_data.msb_brightness_reg = LM3697_CTRL_A_BRT_MSB +
> +						   bank->control_bank * 2;
> +		bank->lmu_data.lsb_brightness_reg = LM3697_CTRL_A_BRT_LSB +
> +						   bank->control_bank * 2;
> 
> -		led->num_leds = fwnode_property_count_u32(child, "led-sources");
> -		if (led->num_leds > LM3697_MAX_LED_STRINGS) {
> +		bank->num_leds = fwnode_property_count_u32(child, "led-sources");
> +		if (bank->num_leds > LM3697_MAX_LED_STRINGS) {
>  			dev_err(&priv->client->dev, "Too many LED strings defined\n");
>  			continue;
>  		}
> 
>  		ret = fwnode_property_read_u32_array(child, "led-sources",
> -						    led->hvled_strings,
> -						    led->num_leds);
> +						    bank->hvled_strings,
> +						    bank->num_leds);
>  		if (ret) {
>  			dev_err(&priv->client->dev, "led-sources property missing\n");
>  			fwnode_handle_put(child);
>  			goto child_out;
>  		}
> 
> -		for (j = 0; j < led->num_leds; j++)
> +		for (j = 0; j < bank->num_leds; j++)
>  			priv->bank_cfg |=
> -				(led->control_bank << led->hvled_strings[j]);
> +				(bank->control_bank << bank->hvled_strings[j]);
> 
>  		ret = ti_lmu_common_get_ramp_params(&priv->client->dev,
> -						    child, &led->lmu_data);
> +						    child, &bank->lmu_data);
>  		if (ret)
>  			dev_warn(&priv->client->dev, "runtime-ramp properties missing\n");
> 
>  		fwnode_property_read_string(child, "linux,default-trigger",
> -					    &led->led_dev.default_trigger);
> +					    &bank->led_dev.default_trigger);
> 
>  		ret = fwnode_property_read_string(child, "label", &name);
>  		if (ret)
> -			snprintf(led->label, sizeof(led->label),
> +			snprintf(bank->label, sizeof(bank->label),
>  				"%s::", priv->client->name);
>  		else
> -			snprintf(led->label, sizeof(led->label),
> +			snprintf(bank->label, sizeof(bank->label),
>  				 "%s:%s", priv->client->name, name);
> 
> -		led->priv = priv;
> -		led->led_dev.name = led->label;
> -		led->led_dev.max_brightness = led->lmu_data.max_brightness;
> -		led->led_dev.brightness_set_blocking = lm3697_brightness_set;
> +		bank->priv = priv;
> +		bank->led_dev.name = bank->label;
> +		bank->led_dev.max_brightness = bank->lmu_data.max_brightness;
> +		bank->led_dev.brightness_set_blocking = lm3697_brightness_set;
> 
> -		ret = devm_led_classdev_register(priv->dev, &led->led_dev);
> +		ret = devm_led_classdev_register(priv->dev, &bank->led_dev);
>  		if (ret) {
>  			dev_err(&priv->client->dev, "led register err: %d\n",
>  				ret);
> @@ -313,7 +313,7 @@ static int lm3697_probe(struct i2c_client *client,
>  		return -ENODEV;
>  	}
> 
> -	led = devm_kzalloc(&client->dev, struct_size(led, leds, count),
> +	led = devm_kzalloc(&client->dev, struct_size(led, banks, count),
>  			   GFP_KERNEL);
>  	if (!led)
>  		return -ENOMEM;
> --
> 2.28.0
> 

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

Attachment: signature.asc
Description: Digital signature