From: Pavel Machek <pavel@ucw.cz>
To: Gabriel David <ultracoolguy4@protonmail.com>
Cc: dmurphy@ti.com, kabel@blackhole.sk, linux-leds@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] leds: lm3697: Rename struct into more appropriate name
Date: Sat, 10 Oct 2020 20:57:00 +0200 [thread overview]
Message-ID: <20201010185700.GA8218@amd> (raw)
In-Reply-To: <nH0fAuRxkhh0jdtlck5LucnuXiEY2wfpLF8C8spK8hebUUZ75xQOe-PjBtR7F8jEZ84l-o9rVJ3z9xvatOAJMjvbH5qCQIO5MuSOmpWr0ZQ=@protonmail.com>
[-- Attachment #1: Type: text/plain, Size: 8818 bytes --]
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@protonmail.com>
> ---
> Yes, this is the same Gabriel David from ultracoolguy@tutanota.org and
> ultracoolguy@disroot.org. 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 #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
next prev parent reply other threads:[~2020-10-10 23:07 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-09 15:51 [PATCH v2] leds: lm3697: Rename struct into more appropriate name Gabriel David
2020-10-10 18:57 ` Pavel Machek [this message]
2020-10-10 21:50 ` Marek Behun
2020-10-12 14:27 ` Dan Murphy
2020-11-25 11:25 ` Pavel Machek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201010185700.GA8218@amd \
--to=pavel@ucw.cz \
--cc=dmurphy@ti.com \
--cc=kabel@blackhole.sk \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-leds@vger.kernel.org \
--cc=ultracoolguy4@protonmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).