* [PATCH leds v3 1/9] leds: lm36274: cosmetic: rename lm36274_data to chip
2020-09-19 18:02 [PATCH leds v3 0/9] Start moving parsing of `linux,default-trigger` to LED core (a cleanup of LED drivers) Marek Behún
@ 2020-09-19 18:02 ` Marek Behún
2020-09-22 15:39 ` Dan Murphy
2020-09-19 18:02 ` [PATCH leds v3 2/9] leds: lm36274: don't iterate through children since there is only one Marek Behún
` (7 subsequent siblings)
8 siblings, 1 reply; 23+ messages in thread
From: Marek Behún @ 2020-09-19 18:02 UTC (permalink / raw)
To: linux-leds; +Cc: Pavel Machek, Dan Murphy, Marek Behún
Rename this variable so that it is easier to read and easier to write in
80 columns. Also rename variable of this type in lm36274_brightness_set
from led to chip, to be consistent.
Signed-off-by: Marek Behún <marek.behun@nic.cz>
Cc: Dan Murphy <dmurphy@ti.com>
---
drivers/leds/leds-lm36274.c | 82 ++++++++++++++++++-------------------
1 file changed, 40 insertions(+), 42 deletions(-)
diff --git a/drivers/leds/leds-lm36274.c b/drivers/leds/leds-lm36274.c
index bfeee03a0053c..4a9f786bb9727 100644
--- a/drivers/leds/leds-lm36274.c
+++ b/drivers/leds/leds-lm36274.c
@@ -41,37 +41,36 @@ struct lm36274 {
};
static int lm36274_brightness_set(struct led_classdev *led_cdev,
- enum led_brightness brt_val)
+ enum led_brightness brt_val)
{
- struct lm36274 *led = container_of(led_cdev, struct lm36274, led_dev);
+ struct lm36274 *chip = container_of(led_cdev, struct lm36274, led_dev);
- return ti_lmu_common_set_brightness(&led->lmu_data, brt_val);
+ return ti_lmu_common_set_brightness(&chip->lmu_data, brt_val);
}
-static int lm36274_init(struct lm36274 *lm36274_data)
+static int lm36274_init(struct lm36274 *chip)
{
int enable_val = 0;
int i;
- for (i = 0; i < lm36274_data->num_leds; i++)
- enable_val |= (1 << lm36274_data->led_sources[i]);
+ for (i = 0; i < chip->num_leds; i++)
+ enable_val |= (1 << chip->led_sources[i]);
if (!enable_val) {
- dev_err(lm36274_data->dev, "No LEDs were enabled\n");
+ dev_err(chip->dev, "No LEDs were enabled\n");
return -EINVAL;
}
enable_val |= LM36274_BL_EN;
- return regmap_write(lm36274_data->regmap, LM36274_REG_BL_EN,
- enable_val);
+ return regmap_write(chip->regmap, LM36274_REG_BL_EN, enable_val);
}
-static int lm36274_parse_dt(struct lm36274 *lm36274_data)
+static int lm36274_parse_dt(struct lm36274 *chip)
{
struct fwnode_handle *child = NULL;
char label[LED_MAX_NAME_SIZE];
- struct device *dev = &lm36274_data->pdev->dev;
+ struct device *dev = &chip->pdev->dev;
const char *name;
int child_cnt;
int ret = -EINVAL;
@@ -84,37 +83,37 @@ static int lm36274_parse_dt(struct lm36274 *lm36274_data)
device_for_each_child_node(dev, child) {
ret = fwnode_property_read_string(child, "label", &name);
if (ret)
- snprintf(label, sizeof(label),
- "%s::", lm36274_data->pdev->name);
+ snprintf(label, sizeof(label), "%s::",
+ chip->pdev->name);
else
- snprintf(label, sizeof(label),
- "%s:%s", lm36274_data->pdev->name, name);
+ snprintf(label, sizeof(label), "%s:%s",
+ chip->pdev->name, name);
- lm36274_data->num_leds = fwnode_property_count_u32(child, "led-sources");
- if (lm36274_data->num_leds <= 0)
+ chip->num_leds = fwnode_property_count_u32(child, "led-sources");
+ if (chip->num_leds <= 0)
return -ENODEV;
ret = fwnode_property_read_u32_array(child, "led-sources",
- lm36274_data->led_sources,
- lm36274_data->num_leds);
+ chip->led_sources,
+ chip->num_leds);
if (ret) {
dev_err(dev, "led-sources property missing\n");
return ret;
}
fwnode_property_read_string(child, "linux,default-trigger",
- &lm36274_data->led_dev.default_trigger);
+ &chip->led_dev.default_trigger);
}
- lm36274_data->lmu_data.regmap = lm36274_data->regmap;
- lm36274_data->lmu_data.max_brightness = MAX_BRIGHTNESS_11BIT;
- lm36274_data->lmu_data.msb_brightness_reg = LM36274_REG_BRT_MSB;
- lm36274_data->lmu_data.lsb_brightness_reg = LM36274_REG_BRT_LSB;
+ chip->lmu_data.regmap = chip->regmap;
+ chip->lmu_data.max_brightness = MAX_BRIGHTNESS_11BIT;
+ chip->lmu_data.msb_brightness_reg = LM36274_REG_BRT_MSB;
+ chip->lmu_data.lsb_brightness_reg = LM36274_REG_BRT_LSB;
- lm36274_data->led_dev.name = label;
- lm36274_data->led_dev.max_brightness = MAX_BRIGHTNESS_11BIT;
- lm36274_data->led_dev.brightness_set_blocking = lm36274_brightness_set;
+ chip->led_dev.name = label;
+ chip->led_dev.max_brightness = MAX_BRIGHTNESS_11BIT;
+ chip->led_dev.brightness_set_blocking = lm36274_brightness_set;
return 0;
}
@@ -122,39 +121,38 @@ static int lm36274_parse_dt(struct lm36274 *lm36274_data)
static int lm36274_probe(struct platform_device *pdev)
{
struct ti_lmu *lmu = dev_get_drvdata(pdev->dev.parent);
- struct lm36274 *lm36274_data;
+ struct lm36274 *chip;
int ret;
- lm36274_data = devm_kzalloc(&pdev->dev, sizeof(*lm36274_data),
- GFP_KERNEL);
- if (!lm36274_data)
+ chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
+ if (!chip)
return -ENOMEM;
- lm36274_data->pdev = pdev;
- lm36274_data->dev = lmu->dev;
- lm36274_data->regmap = lmu->regmap;
- platform_set_drvdata(pdev, lm36274_data);
+ chip->pdev = pdev;
+ chip->dev = lmu->dev;
+ chip->regmap = lmu->regmap;
+ platform_set_drvdata(pdev, chip);
- ret = lm36274_parse_dt(lm36274_data);
+ ret = lm36274_parse_dt(chip);
if (ret) {
- dev_err(lm36274_data->dev, "Failed to parse DT node\n");
+ dev_err(chip->dev, "Failed to parse DT node\n");
return ret;
}
- ret = lm36274_init(lm36274_data);
+ ret = lm36274_init(chip);
if (ret) {
- dev_err(lm36274_data->dev, "Failed to init the device\n");
+ dev_err(chip->dev, "Failed to init the device\n");
return ret;
}
- return led_classdev_register(lm36274_data->dev, &lm36274_data->led_dev);
+ return led_classdev_register(chip->dev, &chip->led_dev);
}
static int lm36274_remove(struct platform_device *pdev)
{
- struct lm36274 *lm36274_data = platform_get_drvdata(pdev);
+ struct lm36274 *chip = platform_get_drvdata(pdev);
- led_classdev_unregister(&lm36274_data->led_dev);
+ led_classdev_unregister(&chip->led_dev);
return 0;
}
--
2.26.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH leds v3 2/9] leds: lm36274: don't iterate through children since there is only one
2020-09-19 18:02 [PATCH leds v3 0/9] Start moving parsing of `linux,default-trigger` to LED core (a cleanup of LED drivers) Marek Behún
2020-09-19 18:02 ` [PATCH leds v3 1/9] leds: lm36274: cosmetic: rename lm36274_data to chip Marek Behún
@ 2020-09-19 18:02 ` Marek Behún
2020-09-22 15:42 ` Dan Murphy
2020-09-19 18:02 ` [PATCH leds v3 3/9] leds: lm36274: use struct led_init_data when registering Marek Behún
` (6 subsequent siblings)
8 siblings, 1 reply; 23+ messages in thread
From: Marek Behún @ 2020-09-19 18:02 UTC (permalink / raw)
To: linux-leds; +Cc: Pavel Machek, Dan Murphy, Marek Behún
Do not use device_for_each_child_node. Since this driver works only with
once child node present, use device_get_next_child_node instead.
This also saves one level of indentation.
Signed-off-by: Marek Behún <marek.behun@nic.cz>
Cc: Dan Murphy <dmurphy@ti.com>
---
drivers/leds/leds-lm36274.c | 50 +++++++++++++++++--------------------
1 file changed, 23 insertions(+), 27 deletions(-)
diff --git a/drivers/leds/leds-lm36274.c b/drivers/leds/leds-lm36274.c
index 4a9f786bb9727..e0fce74a76675 100644
--- a/drivers/leds/leds-lm36274.c
+++ b/drivers/leds/leds-lm36274.c
@@ -72,40 +72,36 @@ static int lm36274_parse_dt(struct lm36274 *chip)
char label[LED_MAX_NAME_SIZE];
struct device *dev = &chip->pdev->dev;
const char *name;
- int child_cnt;
- int ret = -EINVAL;
+ int ret;
/* There should only be 1 node */
- child_cnt = device_get_child_node_count(dev);
- if (child_cnt != 1)
+ if (device_get_child_node_count(dev) != 1)
return -EINVAL;
- device_for_each_child_node(dev, child) {
- ret = fwnode_property_read_string(child, "label", &name);
- if (ret)
- snprintf(label, sizeof(label), "%s::",
- chip->pdev->name);
- else
- snprintf(label, sizeof(label), "%s:%s",
- chip->pdev->name, name);
-
- chip->num_leds = fwnode_property_count_u32(child, "led-sources");
- if (chip->num_leds <= 0)
- return -ENODEV;
-
- ret = fwnode_property_read_u32_array(child, "led-sources",
- chip->led_sources,
- chip->num_leds);
- if (ret) {
- dev_err(dev, "led-sources property missing\n");
- return ret;
- }
-
- fwnode_property_read_string(child, "linux,default-trigger",
- &chip->led_dev.default_trigger);
+ child = device_get_next_child_node(dev, NULL);
+
+ ret = fwnode_property_read_string(child, "label", &name);
+ if (ret)
+ snprintf(label, sizeof(label), "%s::", chip->pdev->name);
+ else
+ snprintf(label, sizeof(label), "%s:%s", chip->pdev->name, name);
+ chip->num_leds = fwnode_property_count_u32(child, "led-sources");
+ if (chip->num_leds <= 0)
+ return -ENODEV;
+
+ ret = fwnode_property_read_u32_array(child, "led-sources",
+ chip->led_sources, chip->num_leds);
+ if (ret) {
+ dev_err(dev, "led-sources property missing\n");
+ return ret;
}
+ fwnode_property_read_string(child, "linux,default-trigger",
+ &chip->led_dev.default_trigger);
+
+ fwnode_handle_put(child);
+
chip->lmu_data.regmap = chip->regmap;
chip->lmu_data.max_brightness = MAX_BRIGHTNESS_11BIT;
chip->lmu_data.msb_brightness_reg = LM36274_REG_BRT_MSB;
--
2.26.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH leds v3 2/9] leds: lm36274: don't iterate through children since there is only one
2020-09-19 18:02 ` [PATCH leds v3 2/9] leds: lm36274: don't iterate through children since there is only one Marek Behún
@ 2020-09-22 15:42 ` Dan Murphy
2020-09-22 15:58 ` Marek Behun
0 siblings, 1 reply; 23+ messages in thread
From: Dan Murphy @ 2020-09-22 15:42 UTC (permalink / raw)
To: Marek Behún, linux-leds; +Cc: Pavel Machek
Hello
On 9/19/20 1:02 PM, Marek Behún wrote:
> Do not use device_for_each_child_node. Since this driver works only with
> once child node present, use device_get_next_child_node instead.
> This also saves one level of indentation.
>
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> Cc: Dan Murphy <dmurphy@ti.com>
> ---
> drivers/leds/leds-lm36274.c | 50 +++++++++++++++++--------------------
> 1 file changed, 23 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/leds/leds-lm36274.c b/drivers/leds/leds-lm36274.c
> index 4a9f786bb9727..e0fce74a76675 100644
> --- a/drivers/leds/leds-lm36274.c
> +++ b/drivers/leds/leds-lm36274.c
> @@ -72,40 +72,36 @@ static int lm36274_parse_dt(struct lm36274 *chip)
> char label[LED_MAX_NAME_SIZE];
> struct device *dev = &chip->pdev->dev;
> const char *name;
> - int child_cnt;
> - int ret = -EINVAL;
> + int ret;
>
> /* There should only be 1 node */
> - child_cnt = device_get_child_node_count(dev);
> - if (child_cnt != 1)
> + if (device_get_child_node_count(dev) != 1)
> return -EINVAL;
>
> - device_for_each_child_node(dev, child) {
> - ret = fwnode_property_read_string(child, "label", &name);
> - if (ret)
> - snprintf(label, sizeof(label), "%s::",
> - chip->pdev->name);
> - else
> - snprintf(label, sizeof(label), "%s:%s",
> - chip->pdev->name, name);
> -
> - chip->num_leds = fwnode_property_count_u32(child, "led-sources");
> - if (chip->num_leds <= 0)
> - return -ENODEV;
> -
> - ret = fwnode_property_read_u32_array(child, "led-sources",
> - chip->led_sources,
> - chip->num_leds);
> - if (ret) {
> - dev_err(dev, "led-sources property missing\n");
> - return ret;
> - }
> -
> - fwnode_property_read_string(child, "linux,default-trigger",
> - &chip->led_dev.default_trigger);
> + child = device_get_next_child_node(dev, NULL);
> +
> + ret = fwnode_property_read_string(child, "label", &name);
> + if (ret)
> + snprintf(label, sizeof(label), "%s::", chip->pdev->name);
> + else
> + snprintf(label, sizeof(label), "%s:%s", chip->pdev->name, name);
>
> + chip->num_leds = fwnode_property_count_u32(child, "led-sources");
> + if (chip->num_leds <= 0)
> + return -ENODEV;
> +
> + ret = fwnode_property_read_u32_array(child, "led-sources",
> + chip->led_sources, chip->num_leds);
> + if (ret) {
> + dev_err(dev, "led-sources property missing\n");
> + return ret;
> }
>
> + fwnode_property_read_string(child, "linux,default-trigger",
> + &chip->led_dev.default_trigger);
> +
> + fwnode_handle_put(child);
> +
> chip->lmu_data.regmap = chip->regmap;
> chip->lmu_data.max_brightness = MAX_BRIGHTNESS_11BIT;
> chip->lmu_data.msb_brightness_reg = LM36274_REG_BRT_MSB;
Question is this device on a piece of hardware you are testing on?
Just wondering how you functionally tested all these changes you submitted
Reviewed-by: Dan Murphy <dmurphy@ti.com>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH leds v3 2/9] leds: lm36274: don't iterate through children since there is only one
2020-09-22 15:42 ` Dan Murphy
@ 2020-09-22 15:58 ` Marek Behun
2020-09-22 19:12 ` Dan Murphy
0 siblings, 1 reply; 23+ messages in thread
From: Marek Behun @ 2020-09-22 15:58 UTC (permalink / raw)
To: Dan Murphy; +Cc: linux-leds, Pavel Machek
On Tue, 22 Sep 2020 10:42:49 -0500
Dan Murphy <dmurphy@ti.com> wrote:
> Hello
>
> On 9/19/20 1:02 PM, Marek Behún wrote:
> > Do not use device_for_each_child_node. Since this driver works only with
> > once child node present, use device_get_next_child_node instead.
> > This also saves one level of indentation.
> >
> > Signed-off-by: Marek Behún <marek.behun@nic.cz>
> > Cc: Dan Murphy <dmurphy@ti.com>
> > ---
> > drivers/leds/leds-lm36274.c | 50 +++++++++++++++++--------------------
> > 1 file changed, 23 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/leds/leds-lm36274.c b/drivers/leds/leds-lm36274.c
> > index 4a9f786bb9727..e0fce74a76675 100644
> > --- a/drivers/leds/leds-lm36274.c
> > +++ b/drivers/leds/leds-lm36274.c
> > @@ -72,40 +72,36 @@ static int lm36274_parse_dt(struct lm36274 *chip)
> > char label[LED_MAX_NAME_SIZE];
> > struct device *dev = &chip->pdev->dev;
> > const char *name;
> > - int child_cnt;
> > - int ret = -EINVAL;
> > + int ret;
> >
> > /* There should only be 1 node */
> > - child_cnt = device_get_child_node_count(dev);
> > - if (child_cnt != 1)
> > + if (device_get_child_node_count(dev) != 1)
> > return -EINVAL;
> >
> > - device_for_each_child_node(dev, child) {
> > - ret = fwnode_property_read_string(child, "label", &name);
> > - if (ret)
> > - snprintf(label, sizeof(label), "%s::",
> > - chip->pdev->name);
> > - else
> > - snprintf(label, sizeof(label), "%s:%s",
> > - chip->pdev->name, name);
> > -
> > - chip->num_leds = fwnode_property_count_u32(child, "led-sources");
> > - if (chip->num_leds <= 0)
> > - return -ENODEV;
> > -
> > - ret = fwnode_property_read_u32_array(child, "led-sources",
> > - chip->led_sources,
> > - chip->num_leds);
> > - if (ret) {
> > - dev_err(dev, "led-sources property missing\n");
> > - return ret;
> > - }
> > -
> > - fwnode_property_read_string(child, "linux,default-trigger",
> > - &chip->led_dev.default_trigger);
> > + child = device_get_next_child_node(dev, NULL);
> > +
> > + ret = fwnode_property_read_string(child, "label", &name);
> > + if (ret)
> > + snprintf(label, sizeof(label), "%s::", chip->pdev->name);
> > + else
> > + snprintf(label, sizeof(label), "%s:%s", chip->pdev->name, name);
> >
> > + chip->num_leds = fwnode_property_count_u32(child, "led-sources");
> > + if (chip->num_leds <= 0)
> > + return -ENODEV;
> > +
> > + ret = fwnode_property_read_u32_array(child, "led-sources",
> > + chip->led_sources, chip->num_leds);
> > + if (ret) {
> > + dev_err(dev, "led-sources property missing\n");
> > + return ret;
> > }
> >
> > + fwnode_property_read_string(child, "linux,default-trigger",
> > + &chip->led_dev.default_trigger);
> > +
> > + fwnode_handle_put(child);
> > +
> > chip->lmu_data.regmap = chip->regmap;
> > chip->lmu_data.max_brightness = MAX_BRIGHTNESS_11BIT;
> > chip->lmu_data.msb_brightness_reg = LM36274_REG_BRT_MSB;
>
> Question is this device on a piece of hardware you are testing on?
No, unfortunately. But this driver is rather simple, in comparison to
the others.
As Linus said:
"If it compiles, it is good; if it boots up, it is perfect."
:D
So if someone tested it, it would be perfect.
Marek
> Just wondering how you functionally tested all these changes you submitted
>
> Reviewed-by: Dan Murphy <dmurphy@ti.com>
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH leds v3 2/9] leds: lm36274: don't iterate through children since there is only one
2020-09-22 15:58 ` Marek Behun
@ 2020-09-22 19:12 ` Dan Murphy
0 siblings, 0 replies; 23+ messages in thread
From: Dan Murphy @ 2020-09-22 19:12 UTC (permalink / raw)
To: Marek Behun; +Cc: linux-leds, Pavel Machek
Marek
On 9/22/20 10:58 AM, Marek Behun wrote:
> On Tue, 22 Sep 2020 10:42:49 -0500
> Dan Murphy <dmurphy@ti.com> wrote:
>
>> <snip>
>> Question is this device on a piece of hardware you are testing on?
> No, unfortunately. But this driver is rather simple, in comparison to
> the others.
>
> As Linus said:
> "If it compiles, it is good; if it boots up, it is perfect."
> :D
>
> So if someone tested it, it would be perfect.
Not sure how a comment made in 1998 applies to the state of the kernel
today.
With this much change to the driver there should have been some level of
functional testing.
So I pulled out my hardware and gave it a whirl. Gave my TB on the
LM36274 patches.
Dan
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH leds v3 3/9] leds: lm36274: use struct led_init_data when registering
2020-09-19 18:02 [PATCH leds v3 0/9] Start moving parsing of `linux,default-trigger` to LED core (a cleanup of LED drivers) Marek Behún
2020-09-19 18:02 ` [PATCH leds v3 1/9] leds: lm36274: cosmetic: rename lm36274_data to chip Marek Behún
2020-09-19 18:02 ` [PATCH leds v3 2/9] leds: lm36274: don't iterate through children since there is only one Marek Behún
@ 2020-09-19 18:02 ` Marek Behún
2020-09-24 12:06 ` Pavel Machek
2020-09-19 18:02 ` [PATCH leds v3 4/9] leds: lm36274: do not set chip settings in DT parsing function Marek Behún
` (5 subsequent siblings)
8 siblings, 1 reply; 23+ messages in thread
From: Marek Behún @ 2020-09-19 18:02 UTC (permalink / raw)
To: linux-leds; +Cc: Pavel Machek, Dan Murphy, Marek Behún
By using struct led_init_data when registering we do not need to parse
`label` DT property. Moreover `label` is deprecated and if it is not
present but `color` and `function` are, LED core will compose a name
from these properties instead.
Previously if the `label` DT property was not present, the code composed
name for the LED in the form
"parent_name::"
For backwards compatibility we therefore set
init_data->default_label = ":";
so that the LED will not get a different name if `label` property is not
present, nor are `color` and `function`.
Signed-off-by: Marek Behún <marek.behun@nic.cz>
Cc: Dan Murphy <dmurphy@ti.com>
---
drivers/leds/leds-lm36274.c | 42 ++++++++++++++++++++++---------------
1 file changed, 25 insertions(+), 17 deletions(-)
diff --git a/drivers/leds/leds-lm36274.c b/drivers/leds/leds-lm36274.c
index e0fce74a76675..ee4f90f04f195 100644
--- a/drivers/leds/leds-lm36274.c
+++ b/drivers/leds/leds-lm36274.c
@@ -66,12 +66,11 @@ static int lm36274_init(struct lm36274 *chip)
return regmap_write(chip->regmap, LM36274_REG_BL_EN, enable_val);
}
-static int lm36274_parse_dt(struct lm36274 *chip)
+static int lm36274_parse_dt(struct lm36274 *chip,
+ struct led_init_data *init_data)
{
- struct fwnode_handle *child = NULL;
- char label[LED_MAX_NAME_SIZE];
struct device *dev = &chip->pdev->dev;
- const char *name;
+ struct fwnode_handle *child;
int ret;
/* There should only be 1 node */
@@ -80,43 +79,45 @@ static int lm36274_parse_dt(struct lm36274 *chip)
child = device_get_next_child_node(dev, NULL);
- ret = fwnode_property_read_string(child, "label", &name);
- if (ret)
- snprintf(label, sizeof(label), "%s::", chip->pdev->name);
- else
- snprintf(label, sizeof(label), "%s:%s", chip->pdev->name, name);
+ init_data->fwnode = child;
+ init_data->devicename = chip->pdev->name;
+ /* for backwards compatibility when `label` property is not present */
+ init_data->default_label = ":";
chip->num_leds = fwnode_property_count_u32(child, "led-sources");
- if (chip->num_leds <= 0)
- return -ENODEV;
+ if (chip->num_leds <= 0) {
+ ret = -ENODEV;
+ goto err;
+ }
ret = fwnode_property_read_u32_array(child, "led-sources",
chip->led_sources, chip->num_leds);
if (ret) {
dev_err(dev, "led-sources property missing\n");
- return ret;
+ goto err;
}
fwnode_property_read_string(child, "linux,default-trigger",
&chip->led_dev.default_trigger);
- fwnode_handle_put(child);
-
chip->lmu_data.regmap = chip->regmap;
chip->lmu_data.max_brightness = MAX_BRIGHTNESS_11BIT;
chip->lmu_data.msb_brightness_reg = LM36274_REG_BRT_MSB;
chip->lmu_data.lsb_brightness_reg = LM36274_REG_BRT_LSB;
- chip->led_dev.name = label;
chip->led_dev.max_brightness = MAX_BRIGHTNESS_11BIT;
chip->led_dev.brightness_set_blocking = lm36274_brightness_set;
return 0;
+err:
+ fwnode_handle_put(child);
+ return ret;
}
static int lm36274_probe(struct platform_device *pdev)
{
struct ti_lmu *lmu = dev_get_drvdata(pdev->dev.parent);
+ struct led_init_data init_data = {};
struct lm36274 *chip;
int ret;
@@ -129,7 +130,7 @@ static int lm36274_probe(struct platform_device *pdev)
chip->regmap = lmu->regmap;
platform_set_drvdata(pdev, chip);
- ret = lm36274_parse_dt(chip);
+ ret = lm36274_parse_dt(chip, &init_data);
if (ret) {
dev_err(chip->dev, "Failed to parse DT node\n");
return ret;
@@ -141,7 +142,14 @@ static int lm36274_probe(struct platform_device *pdev)
return ret;
}
- return led_classdev_register(chip->dev, &chip->led_dev);
+ ret = led_classdev_register_ext(chip->dev, &chip->led_dev, &init_data);
+ if (ret)
+ dev_err(chip->dev, "Failed to register LED for node %pfw\n",
+ init_data.fwnode);
+
+ fwnode_handle_put(init_data.fwnode);
+
+ return ret;
}
static int lm36274_remove(struct platform_device *pdev)
--
2.26.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH leds v3 4/9] leds: lm36274: do not set chip settings in DT parsing function
2020-09-19 18:02 [PATCH leds v3 0/9] Start moving parsing of `linux,default-trigger` to LED core (a cleanup of LED drivers) Marek Behún
` (2 preceding siblings ...)
2020-09-19 18:02 ` [PATCH leds v3 3/9] leds: lm36274: use struct led_init_data when registering Marek Behún
@ 2020-09-19 18:02 ` Marek Behún
2020-09-19 18:03 ` [PATCH leds v3 5/9] leds: lm36274: use platform device as parent of LED Marek Behún
` (4 subsequent siblings)
8 siblings, 0 replies; 23+ messages in thread
From: Marek Behún @ 2020-09-19 18:02 UTC (permalink / raw)
To: linux-leds; +Cc: Pavel Machek, Dan Murphy, Marek Behún
These settings are not parsed from DT and therefore semantically should
not be set in function with a name lm36274_parse_dt.
Signed-off-by: Marek Behún <marek.behun@nic.cz>
Cc: Dan Murphy <dmurphy@ti.com>
---
drivers/leds/leds-lm36274.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/leds/leds-lm36274.c b/drivers/leds/leds-lm36274.c
index ee4f90f04f195..85a58a5cbdf9c 100644
--- a/drivers/leds/leds-lm36274.c
+++ b/drivers/leds/leds-lm36274.c
@@ -100,14 +100,6 @@ static int lm36274_parse_dt(struct lm36274 *chip,
fwnode_property_read_string(child, "linux,default-trigger",
&chip->led_dev.default_trigger);
- chip->lmu_data.regmap = chip->regmap;
- chip->lmu_data.max_brightness = MAX_BRIGHTNESS_11BIT;
- chip->lmu_data.msb_brightness_reg = LM36274_REG_BRT_MSB;
- chip->lmu_data.lsb_brightness_reg = LM36274_REG_BRT_LSB;
-
- chip->led_dev.max_brightness = MAX_BRIGHTNESS_11BIT;
- chip->led_dev.brightness_set_blocking = lm36274_brightness_set;
-
return 0;
err:
fwnode_handle_put(child);
@@ -142,6 +134,14 @@ static int lm36274_probe(struct platform_device *pdev)
return ret;
}
+ chip->lmu_data.regmap = chip->regmap;
+ chip->lmu_data.max_brightness = MAX_BRIGHTNESS_11BIT;
+ chip->lmu_data.msb_brightness_reg = LM36274_REG_BRT_MSB;
+ chip->lmu_data.lsb_brightness_reg = LM36274_REG_BRT_LSB;
+
+ chip->led_dev.max_brightness = MAX_BRIGHTNESS_11BIT;
+ chip->led_dev.brightness_set_blocking = lm36274_brightness_set;
+
ret = led_classdev_register_ext(chip->dev, &chip->led_dev, &init_data);
if (ret)
dev_err(chip->dev, "Failed to register LED for node %pfw\n",
--
2.26.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH leds v3 5/9] leds: lm36274: use platform device as parent of LED
2020-09-19 18:02 [PATCH leds v3 0/9] Start moving parsing of `linux,default-trigger` to LED core (a cleanup of LED drivers) Marek Behún
` (3 preceding siblings ...)
2020-09-19 18:02 ` [PATCH leds v3 4/9] leds: lm36274: do not set chip settings in DT parsing function Marek Behún
@ 2020-09-19 18:03 ` Marek Behún
2020-09-19 18:03 ` [PATCH leds v3 6/9] leds: lm36274: use devres LED registering function Marek Behún
` (3 subsequent siblings)
8 siblings, 0 replies; 23+ messages in thread
From: Marek Behún @ 2020-09-19 18:03 UTC (permalink / raw)
To: linux-leds; +Cc: Pavel Machek, Dan Murphy, Marek Behún
Instead of registering LED under the MFD device, this driver sets the
parent of the LED it is registering to the parent of the MFD device (the
I2C client device).
Because of this we cannot use devres for LED registration, since it can
result in use-after-free, see commit
a0972fff0947 ("leds: lm36274: fix use-after-free on unbind").
The only other in-tree driver that also registers under the MFD device
(drivers/regulator/lm363x-regulator.c) sets the parent to the MFD
device.
Set the parent of this LED to the MFD device, instead of the I2C client
device.
Signed-off-by: Marek Behún <marek.behun@nic.cz>
Cc: Dan Murphy <dmurphy@ti.com>
---
drivers/leds/leds-lm36274.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/leds/leds-lm36274.c b/drivers/leds/leds-lm36274.c
index 85a58a5cbdf9c..74c236d1a60c8 100644
--- a/drivers/leds/leds-lm36274.c
+++ b/drivers/leds/leds-lm36274.c
@@ -69,7 +69,7 @@ static int lm36274_init(struct lm36274 *chip)
static int lm36274_parse_dt(struct lm36274 *chip,
struct led_init_data *init_data)
{
- struct device *dev = &chip->pdev->dev;
+ struct device *dev = chip->dev;
struct fwnode_handle *child;
int ret;
@@ -118,7 +118,7 @@ static int lm36274_probe(struct platform_device *pdev)
return -ENOMEM;
chip->pdev = pdev;
- chip->dev = lmu->dev;
+ chip->dev = &pdev->dev;
chip->regmap = lmu->regmap;
platform_set_drvdata(pdev, chip);
--
2.26.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH leds v3 6/9] leds: lm36274: use devres LED registering function
2020-09-19 18:02 [PATCH leds v3 0/9] Start moving parsing of `linux,default-trigger` to LED core (a cleanup of LED drivers) Marek Behún
` (4 preceding siblings ...)
2020-09-19 18:03 ` [PATCH leds v3 5/9] leds: lm36274: use platform device as parent of LED Marek Behún
@ 2020-09-19 18:03 ` Marek Behún
2020-09-20 21:45 ` Pavel Machek
2020-09-19 18:03 ` [PATCH leds v3 7/9] leds: lm3532: don't parse label DT property Marek Behún
` (2 subsequent siblings)
8 siblings, 1 reply; 23+ messages in thread
From: Marek Behún @ 2020-09-19 18:03 UTC (permalink / raw)
To: linux-leds; +Cc: Pavel Machek, Dan Murphy, Marek Behún
Now that the potential use-after-free issue is resolved we can use
devres for LED registration in this driver.
By using devres version of LED registering function we can remove the
.remove method from this driver.
Signed-off-by: Marek Behún <marek.behun@nic.cz>
Cc: Dan Murphy <dmurphy@ti.com>
---
drivers/leds/leds-lm36274.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/drivers/leds/leds-lm36274.c b/drivers/leds/leds-lm36274.c
index 74c236d1a60c8..10a63b7f2ecce 100644
--- a/drivers/leds/leds-lm36274.c
+++ b/drivers/leds/leds-lm36274.c
@@ -142,7 +142,8 @@ static int lm36274_probe(struct platform_device *pdev)
chip->led_dev.max_brightness = MAX_BRIGHTNESS_11BIT;
chip->led_dev.brightness_set_blocking = lm36274_brightness_set;
- ret = led_classdev_register_ext(chip->dev, &chip->led_dev, &init_data);
+ ret = devm_led_classdev_register_ext(chip->dev, &chip->led_dev,
+ &init_data);
if (ret)
dev_err(chip->dev, "Failed to register LED for node %pfw\n",
init_data.fwnode);
@@ -152,15 +153,6 @@ static int lm36274_probe(struct platform_device *pdev)
return ret;
}
-static int lm36274_remove(struct platform_device *pdev)
-{
- struct lm36274 *chip = platform_get_drvdata(pdev);
-
- led_classdev_unregister(&chip->led_dev);
-
- return 0;
-}
-
static const struct of_device_id of_lm36274_leds_match[] = {
{ .compatible = "ti,lm36274-backlight", },
{},
@@ -169,7 +161,6 @@ MODULE_DEVICE_TABLE(of, of_lm36274_leds_match);
static struct platform_driver lm36274_driver = {
.probe = lm36274_probe,
- .remove = lm36274_remove,
.driver = {
.name = "lm36274-leds",
},
--
2.26.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH leds v3 6/9] leds: lm36274: use devres LED registering function
2020-09-19 18:03 ` [PATCH leds v3 6/9] leds: lm36274: use devres LED registering function Marek Behún
@ 2020-09-20 21:45 ` Pavel Machek
2020-09-20 21:54 ` Marek Behun
0 siblings, 1 reply; 23+ messages in thread
From: Pavel Machek @ 2020-09-20 21:45 UTC (permalink / raw)
To: Marek Behún; +Cc: linux-leds, Dan Murphy
[-- Attachment #1: Type: text/plain, Size: 569 bytes --]
Hi!
> Now that the potential use-after-free issue is resolved we can use
> devres for LED registration in this driver.
>
> By using devres version of LED registering function we can remove the
> .remove method from this driver.
>
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> Cc: Dan Murphy <dmurphy@ti.com>
AFAICT this one is buggy, I sent explanation before. Why are you
resubmitting it?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH leds v3 6/9] leds: lm36274: use devres LED registering function
2020-09-20 21:45 ` Pavel Machek
@ 2020-09-20 21:54 ` Marek Behun
2020-09-22 16:38 ` Pavel Machek
0 siblings, 1 reply; 23+ messages in thread
From: Marek Behun @ 2020-09-20 21:54 UTC (permalink / raw)
To: Pavel Machek; +Cc: linux-leds, Dan Murphy
On Sun, 20 Sep 2020 23:45:32 +0200
Pavel Machek <pavel@ucw.cz> wrote:
> Hi!
>
> > Now that the potential use-after-free issue is resolved we can use
> > devres for LED registration in this driver.
> >
> > By using devres version of LED registering function we can remove the
> > .remove method from this driver.
> >
> > Signed-off-by: Marek Behún <marek.behun@nic.cz>
> > Cc: Dan Murphy <dmurphy@ti.com>
>
> AFAICT this one is buggy, I sent explanation before. Why are you
> resubmitting it?
>
> Pavel
The previous patch in this series (v3 5/9) should solve this issue and
th commit message explains how.
Marek
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH leds v3 6/9] leds: lm36274: use devres LED registering function
2020-09-20 21:54 ` Marek Behun
@ 2020-09-22 16:38 ` Pavel Machek
2020-09-22 16:58 ` Marek Behun
0 siblings, 1 reply; 23+ messages in thread
From: Pavel Machek @ 2020-09-22 16:38 UTC (permalink / raw)
To: Marek Behun; +Cc: linux-leds, Dan Murphy
[-- Attachment #1: Type: text/plain, Size: 1080 bytes --]
On Sun 2020-09-20 23:54:36, Marek Behun wrote:
> On Sun, 20 Sep 2020 23:45:32 +0200
> Pavel Machek <pavel@ucw.cz> wrote:
>
> > Hi!
> >
> > > Now that the potential use-after-free issue is resolved we can use
> > > devres for LED registration in this driver.
> > >
> > > By using devres version of LED registering function we can remove the
> > > .remove method from this driver.
> > >
> > > Signed-off-by: Marek Behún <marek.behun@nic.cz>
> > > Cc: Dan Murphy <dmurphy@ti.com>
> >
> > AFAICT this one is buggy, I sent explanation before. Why are you
> > resubmitting it?
>
> The previous patch in this series (v3 5/9) should solve this issue and
> th commit message explains how.
Aha, let me see.
Will 5/9 have some side-effects, like device appearing at different
place in sysfs?
First few patches look ok, but it would be really nice someone tested
complete sereies.
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH leds v3 6/9] leds: lm36274: use devres LED registering function
2020-09-22 16:38 ` Pavel Machek
@ 2020-09-22 16:58 ` Marek Behun
2020-09-22 19:09 ` Dan Murphy
0 siblings, 1 reply; 23+ messages in thread
From: Marek Behun @ 2020-09-22 16:58 UTC (permalink / raw)
To: Pavel Machek; +Cc: linux-leds, Dan Murphy
On Tue, 22 Sep 2020 18:38:42 +0200
Pavel Machek <pavel@ucw.cz> wrote:
> On Sun 2020-09-20 23:54:36, Marek Behun wrote:
> > On Sun, 20 Sep 2020 23:45:32 +0200
> > Pavel Machek <pavel@ucw.cz> wrote:
> >
> > > Hi!
> > >
> > > > Now that the potential use-after-free issue is resolved we can use
> > > > devres for LED registration in this driver.
> > > >
> > > > By using devres version of LED registering function we can remove the
> > > > .remove method from this driver.
> > > >
> > > > Signed-off-by: Marek Behún <marek.behun@nic.cz>
> > > > Cc: Dan Murphy <dmurphy@ti.com>
> > >
> > > AFAICT this one is buggy, I sent explanation before. Why are you
> > > resubmitting it?
> >
> > The previous patch in this series (v3 5/9) should solve this issue and
> > th commit message explains how.
>
> Aha, let me see.
>
> Will 5/9 have some side-effects, like device appearing at different
> place in sysfs?
Yes, unfortunately. Before this path the led should be in
/sys/devices/.../i2c-client/leds/led
or somthing like that, and after
/sys/devices/..c/i2c-client/mfd/leds/led
But it should have been this way from beginning, I think. The other
driver, regulator, registers its device under the mfd device.
The question is whether this will break something for someone. I don't
think so, but...
>
> First few patches look ok, but it would be really nice someone tested
> complete sereies.
>
> Best regards,
> Pavel
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH leds v3 6/9] leds: lm36274: use devres LED registering function
2020-09-22 16:58 ` Marek Behun
@ 2020-09-22 19:09 ` Dan Murphy
0 siblings, 0 replies; 23+ messages in thread
From: Dan Murphy @ 2020-09-22 19:09 UTC (permalink / raw)
To: Marek Behun, Pavel Machek; +Cc: linux-leds
Pavel
On 9/22/20 11:58 AM, Marek Behun wrote:
> On Tue, 22 Sep 2020 18:38:42 +0200
> Pavel Machek <pavel@ucw.cz> wrote:
>
>> On Sun 2020-09-20 23:54:36, Marek Behun wrote:
>>> On Sun, 20 Sep 2020 23:45:32 +0200
>>> Pavel Machek <pavel@ucw.cz> wrote:
>>>
>>>> Hi!
>>>>
>>>>> Now that the potential use-after-free issue is resolved we can use
>>>>> devres for LED registration in this driver.
>>>>>
>>>>> By using devres version of LED registering function we can remove the
>>>>> .remove method from this driver.
>>>>>
>>>>> Signed-off-by: Marek Behún <marek.behun@nic.cz>
>>>>> Cc: Dan Murphy <dmurphy@ti.com>
>>>> AFAICT this one is buggy, I sent explanation before. Why are you
>>>> resubmitting it?
>>> The previous patch in this series (v3 5/9) should solve this issue and
>>> th commit message explains how.
>> Aha, let me see.
>>
>> Will 5/9 have some side-effects, like device appearing at different
>> place in sysfs?
> Yes, unfortunately. Before this path the led should be in
> /sys/devices/.../i2c-client/leds/led
> or somthing like that, and after
> /sys/devices/..c/i2c-client/mfd/leds/led
>
> But it should have been this way from beginning, I think. The other
> driver, regulator, registers its device under the mfd device.
>
> The question is whether this will break something for someone. I don't
> think so, but...
>
>> First few patches look ok, but it would be really nice someone tested
>> complete sereies.
For the LM36274 patches
Tested-by: Dan Murphy <dmurphy@ti.com>
Also found some other legacy issues I sent patches for.
Dan
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH leds v3 7/9] leds: lm3532: don't parse label DT property
2020-09-19 18:02 [PATCH leds v3 0/9] Start moving parsing of `linux,default-trigger` to LED core (a cleanup of LED drivers) Marek Behún
` (5 preceding siblings ...)
2020-09-19 18:03 ` [PATCH leds v3 6/9] leds: lm36274: use devres LED registering function Marek Behún
@ 2020-09-19 18:03 ` Marek Behún
2020-09-19 18:03 ` [PATCH leds v3 8/9] leds: syscon: use struct led_init_data when registering Marek Behún
2020-09-19 18:03 ` [PATCH leds v3 9/9] leds: parse linux,default-trigger DT property in LED core Marek Behún
8 siblings, 0 replies; 23+ messages in thread
From: Marek Behún @ 2020-09-19 18:03 UTC (permalink / raw)
To: linux-leds; +Cc: Pavel Machek, Dan Murphy, Marek Behún
This driver uses extended LED registration, so we do not need to parse
the `label` DT property on our own.
Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
drivers/leds/leds-lm3532.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/drivers/leds/leds-lm3532.c b/drivers/leds/leds-lm3532.c
index 946ad67eaecb7..9b6973217cc0b 100644
--- a/drivers/leds/leds-lm3532.c
+++ b/drivers/leds/leds-lm3532.c
@@ -129,7 +129,6 @@ struct lm3532_als_data {
* @full_scale_current - The full-scale current setting for the current sink.
* @led_strings - The LED strings supported in this array
* @enabled - Enabled status
- * @label - LED label
*/
struct lm3532_led {
struct led_classdev led_dev;
@@ -142,7 +141,6 @@ struct lm3532_led {
int full_scale_current;
unsigned int enabled:1;
u32 led_strings[LM3532_MAX_CONTROL_BANKS];
- char label[LED_MAX_NAME_SIZE];
};
/**
@@ -548,7 +546,6 @@ static int lm3532_parse_node(struct lm3532_data *priv)
{
struct fwnode_handle *child = NULL;
struct lm3532_led *led;
- const char *name;
int control_bank;
u32 ramp_time;
size_t i = 0;
@@ -646,16 +643,7 @@ static int lm3532_parse_node(struct lm3532_data *priv)
fwnode_property_read_string(child, "linux,default-trigger",
&led->led_dev.default_trigger);
- ret = fwnode_property_read_string(child, "label", &name);
- if (ret)
- snprintf(led->label, sizeof(led->label),
- "%s::", priv->client->name);
- else
- snprintf(led->label, sizeof(led->label),
- "%s:%s", priv->client->name, name);
-
led->priv = priv;
- led->led_dev.name = led->label;
led->led_dev.brightness_set_blocking = lm3532_brightness_set;
ret = devm_led_classdev_register_ext(priv->dev, &led->led_dev, &idata);
--
2.26.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH leds v3 8/9] leds: syscon: use struct led_init_data when registering
2020-09-19 18:02 [PATCH leds v3 0/9] Start moving parsing of `linux,default-trigger` to LED core (a cleanup of LED drivers) Marek Behún
` (6 preceding siblings ...)
2020-09-19 18:03 ` [PATCH leds v3 7/9] leds: lm3532: don't parse label DT property Marek Behún
@ 2020-09-19 18:03 ` Marek Behún
2020-09-29 13:17 ` Linus Walleij
2020-09-19 18:03 ` [PATCH leds v3 9/9] leds: parse linux,default-trigger DT property in LED core Marek Behún
8 siblings, 1 reply; 23+ messages in thread
From: Marek Behún @ 2020-09-19 18:03 UTC (permalink / raw)
To: linux-leds; +Cc: Pavel Machek, Dan Murphy, Marek Behún, Linus Walleij
By using struct led_init_data when registering we do not need to parse
`label` DT property. Moreover `label` is deprecated and if it is not
present but `color` and `function` are, LED core will compose a name
from these properties instead.
Signed-off-by: Marek Behún <marek.behun@nic.cz>
Cc: Linus Walleij <linus.walleij@linaro.org>
---
drivers/leds/leds-syscon.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/leds/leds-syscon.c b/drivers/leds/leds-syscon.c
index d7230da815430..f54935fa650a7 100644
--- a/drivers/leds/leds-syscon.c
+++ b/drivers/leds/leds-syscon.c
@@ -55,6 +55,7 @@ static void syscon_led_set(struct led_classdev *led_cdev,
static int syscon_led_probe(struct platform_device *pdev)
{
+ struct led_init_data init_data = {};
struct device *dev = &pdev->dev;
struct device_node *np = dev_of_node(dev);
struct device *parent;
@@ -84,8 +85,6 @@ static int syscon_led_probe(struct platform_device *pdev)
return -EINVAL;
if (of_property_read_u32(np, "mask", &sled->mask))
return -EINVAL;
- sled->cdev.name =
- of_get_property(np, "label", NULL) ? : np->name;
sled->cdev.default_trigger =
of_get_property(np, "linux,default-trigger", NULL);
@@ -115,7 +114,9 @@ static int syscon_led_probe(struct platform_device *pdev)
}
sled->cdev.brightness_set = syscon_led_set;
- ret = devm_led_classdev_register(dev, &sled->cdev);
+ init_data.fwnode = of_fwnode_handle(np);
+
+ ret = devm_led_classdev_register_ext(dev, &sled->cdev, &init_data);
if (ret < 0)
return ret;
--
2.26.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH leds v3 8/9] leds: syscon: use struct led_init_data when registering
2020-09-19 18:03 ` [PATCH leds v3 8/9] leds: syscon: use struct led_init_data when registering Marek Behún
@ 2020-09-29 13:17 ` Linus Walleij
0 siblings, 0 replies; 23+ messages in thread
From: Linus Walleij @ 2020-09-29 13:17 UTC (permalink / raw)
To: Marek Behún; +Cc: Linux LED Subsystem, Pavel Machek, Dan Murphy
On Sat, Sep 19, 2020 at 8:03 PM Marek Behún <marek.behun@nic.cz> wrote:
> By using struct led_init_data when registering we do not need to parse
> `label` DT property. Moreover `label` is deprecated and if it is not
> present but `color` and `function` are, LED core will compose a name
> from these properties instead.
>
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> Cc: Linus Walleij <linus.walleij@linaro.org>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH leds v3 9/9] leds: parse linux,default-trigger DT property in LED core
2020-09-19 18:02 [PATCH leds v3 0/9] Start moving parsing of `linux,default-trigger` to LED core (a cleanup of LED drivers) Marek Behún
` (7 preceding siblings ...)
2020-09-19 18:03 ` [PATCH leds v3 8/9] leds: syscon: use struct led_init_data when registering Marek Behún
@ 2020-09-19 18:03 ` Marek Behún
2020-09-24 12:10 ` Pavel Machek
8 siblings, 1 reply; 23+ messages in thread
From: Marek Behún @ 2020-09-19 18:03 UTC (permalink / raw)
To: linux-leds; +Cc: Pavel Machek, Dan Murphy, Marek Behún
Do the parsing of `linux,default-trigger` DT property to LED core.
Currently it is done in many different drivers and the code is repeated.
This patch removes the parsing from 23 drivers:
an30259a, aw2013, bcm6328, bcm6358, cr0014114, el15203000, gpio,
is31fl32xx, lm3532, lm36274, lm3692x, lm3697, lp50xx, lp8860, lt3593,
max77650, mt6323, ns2, pm8058, pwm, syscon, tlc591xx and turris-omnia.
There is one driver in drivers/input which parses this property on it's
own. I shall send a separate patch there after this is applied.
There are still 8 drivers that parse this property on their own because
they do not pass the led_init_data structure to the registering
function. I will try to refactor those in the future.
Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
drivers/leds/led-class.c | 5 +++++
drivers/leds/leds-an30259a.c | 3 ---
drivers/leds/leds-aw2013.c | 3 ---
drivers/leds/leds-bcm6328.c | 4 ----
drivers/leds/leds-bcm6358.c | 4 ----
drivers/leds/leds-cr0014114.c | 3 ---
drivers/leds/leds-el15203000.c | 3 ---
drivers/leds/leds-gpio.c | 3 ---
drivers/leds/leds-is31fl32xx.c | 3 ---
drivers/leds/leds-lm3532.c | 3 ---
drivers/leds/leds-lm36274.c | 3 ---
drivers/leds/leds-lm3692x.c | 3 ---
drivers/leds/leds-lm3697.c | 3 ---
drivers/leds/leds-lp50xx.c | 3 ---
drivers/leds/leds-lp8860.c | 4 ----
drivers/leds/leds-lt3593.c | 3 ---
drivers/leds/leds-max77650.c | 3 ---
drivers/leds/leds-mt6323.c | 4 ----
drivers/leds/leds-ns2.c | 3 ---
drivers/leds/leds-pm8058.c | 2 --
drivers/leds/leds-pwm.c | 5 -----
drivers/leds/leds-syscon.c | 2 --
drivers/leds/leds-tlc591xx.c | 3 ---
drivers/leds/leds-turris-omnia.c | 2 --
24 files changed, 5 insertions(+), 72 deletions(-)
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index cc3929f858b68..131ca83f5fb38 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -354,6 +354,11 @@ int led_classdev_register_ext(struct device *parent,
ret = led_compose_name(parent, init_data, composed_name);
if (ret < 0)
return ret;
+
+ if (init_data->fwnode)
+ fwnode_property_read_string(init_data->fwnode,
+ "linux,default-trigger",
+ &led_cdev->default_trigger);
} else {
proposed_name = led_cdev->name;
}
diff --git a/drivers/leds/leds-an30259a.c b/drivers/leds/leds-an30259a.c
index 9749f1cc3e15f..a0df1fb28774d 100644
--- a/drivers/leds/leds-an30259a.c
+++ b/drivers/leds/leds-an30259a.c
@@ -238,9 +238,6 @@ static int an30259a_dt_init(struct i2c_client *client,
led->default_state = STATE_OFF;
}
- of_property_read_string(child, "linux,default-trigger",
- &led->cdev.default_trigger);
-
i++;
}
diff --git a/drivers/leds/leds-aw2013.c b/drivers/leds/leds-aw2013.c
index 927c5ba32592f..80d937454aeef 100644
--- a/drivers/leds/leds-aw2013.c
+++ b/drivers/leds/leds-aw2013.c
@@ -297,9 +297,6 @@ static int aw2013_probe_dt(struct aw2013 *chip)
"DT property led-max-microamp is missing\n");
}
- of_property_read_string(child, "linux,default-trigger",
- &led->cdev.default_trigger);
-
led->cdev.brightness_set_blocking = aw2013_brightness_set;
led->cdev.blink_set = aw2013_blink_set;
diff --git a/drivers/leds/leds-bcm6328.c b/drivers/leds/leds-bcm6328.c
index d79aeb956c9b6..226d17d253ed3 100644
--- a/drivers/leds/leds-bcm6328.c
+++ b/drivers/leds/leds-bcm6328.c
@@ -346,10 +346,6 @@ static int bcm6328_led(struct device *dev, struct device_node *nc, u32 reg,
if (of_property_read_bool(nc, "active-low"))
led->active_low = true;
- led->cdev.default_trigger = of_get_property(nc,
- "linux,default-trigger",
- NULL);
-
if (!of_property_read_string(nc, "default-state", &state)) {
if (!strcmp(state, "on")) {
led->cdev.brightness = LED_FULL;
diff --git a/drivers/leds/leds-bcm6358.c b/drivers/leds/leds-bcm6358.c
index 0fd495103a4d9..9d2e487fa08a3 100644
--- a/drivers/leds/leds-bcm6358.c
+++ b/drivers/leds/leds-bcm6358.c
@@ -110,10 +110,6 @@ static int bcm6358_led(struct device *dev, struct device_node *nc, u32 reg,
if (of_property_read_bool(nc, "active-low"))
led->active_low = true;
- led->cdev.default_trigger = of_get_property(nc,
- "linux,default-trigger",
- NULL);
-
if (!of_property_read_string(nc, "default-state", &state)) {
if (!strcmp(state, "on")) {
led->cdev.brightness = LED_FULL;
diff --git a/drivers/leds/leds-cr0014114.c b/drivers/leds/leds-cr0014114.c
index 2da448ae718e9..d03cfd3c0bfbe 100644
--- a/drivers/leds/leds-cr0014114.c
+++ b/drivers/leds/leds-cr0014114.c
@@ -188,9 +188,6 @@ static int cr0014114_probe_dt(struct cr0014114 *priv)
device_for_each_child_node(priv->dev, child) {
led = &priv->leds[i];
- fwnode_property_read_string(child, "linux,default-trigger",
- &led->ldev.default_trigger);
-
led->priv = priv;
led->ldev.max_brightness = CR_MAX_BRIGHTNESS;
led->ldev.brightness_set_blocking = cr0014114_set_sync;
diff --git a/drivers/leds/leds-el15203000.c b/drivers/leds/leds-el15203000.c
index 298b13e4807a8..6ca47f2a20041 100644
--- a/drivers/leds/leds-el15203000.c
+++ b/drivers/leds/leds-el15203000.c
@@ -263,9 +263,6 @@ static int el15203000_probe_dt(struct el15203000 *priv)
return -EINVAL;
}
- fwnode_property_read_string(child, "linux,default-trigger",
- &led->ldev.default_trigger);
-
led->priv = priv;
led->ldev.max_brightness = LED_ON;
led->ldev.brightness_set_blocking = el15203000_set_blocking;
diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index cf84096d88cec..93f5b1b60fdec 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -160,9 +160,6 @@ static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
led_dat->gpiod = led.gpiod;
- fwnode_property_read_string(child, "linux,default-trigger",
- &led.default_trigger);
-
if (!fwnode_property_read_string(child, "default-state",
&state)) {
if (!strcmp(state, "keep"))
diff --git a/drivers/leds/leds-is31fl32xx.c b/drivers/leds/leds-is31fl32xx.c
index ea589d1a89540..2180255ad3393 100644
--- a/drivers/leds/leds-is31fl32xx.c
+++ b/drivers/leds/leds-is31fl32xx.c
@@ -341,9 +341,6 @@ static int is31fl32xx_parse_child_dt(const struct device *dev,
}
led_data->channel = reg;
- of_property_read_string(child, "linux,default-trigger",
- &cdev->default_trigger);
-
cdev->brightness_set_blocking = is31fl32xx_brightness_set;
return 0;
diff --git a/drivers/leds/leds-lm3532.c b/drivers/leds/leds-lm3532.c
index 9b6973217cc0b..10ded9192fb13 100644
--- a/drivers/leds/leds-lm3532.c
+++ b/drivers/leds/leds-lm3532.c
@@ -640,9 +640,6 @@ static int lm3532_parse_node(struct lm3532_data *priv)
goto child_out;
}
- fwnode_property_read_string(child, "linux,default-trigger",
- &led->led_dev.default_trigger);
-
led->priv = priv;
led->led_dev.brightness_set_blocking = lm3532_brightness_set;
diff --git a/drivers/leds/leds-lm36274.c b/drivers/leds/leds-lm36274.c
index 10a63b7f2ecce..582c6a821dc85 100644
--- a/drivers/leds/leds-lm36274.c
+++ b/drivers/leds/leds-lm36274.c
@@ -97,9 +97,6 @@ static int lm36274_parse_dt(struct lm36274 *chip,
goto err;
}
- fwnode_property_read_string(child, "linux,default-trigger",
- &chip->led_dev.default_trigger);
-
return 0;
err:
fwnode_handle_put(child);
diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c
index 1d7ea1b76a125..e945de45388ca 100644
--- a/drivers/leds/leds-lm3692x.c
+++ b/drivers/leds/leds-lm3692x.c
@@ -433,9 +433,6 @@ static int lm3692x_probe_dt(struct lm3692x_led *led)
return -ENODEV;
}
- fwnode_property_read_string(child, "linux,default-trigger",
- &led->led_dev.default_trigger);
-
ret = fwnode_property_read_u32(child, "reg", &led->led_enable);
if (ret) {
dev_err(&led->client->dev, "reg DT property missing\n");
diff --git a/drivers/leds/leds-lm3697.c b/drivers/leds/leds-lm3697.c
index a7779ce931ab3..64c0794801e6d 100644
--- a/drivers/leds/leds-lm3697.c
+++ b/drivers/leds/leds-lm3697.c
@@ -268,9 +268,6 @@ static int lm3697_probe_dt(struct lm3697 *priv)
if (ret)
dev_warn(dev, "runtime-ramp properties missing\n");
- fwnode_property_read_string(child, "linux,default-trigger",
- &led->led_dev.default_trigger);
-
init_data.fwnode = child;
init_data.devicename = priv->client->name;
/* for backwards compatibility if `label` is not present */
diff --git a/drivers/leds/leds-lp50xx.c b/drivers/leds/leds-lp50xx.c
index 47144a37cb945..5fb4f24aeb2e8 100644
--- a/drivers/leds/leds-lp50xx.c
+++ b/drivers/leds/leds-lp50xx.c
@@ -508,9 +508,6 @@ static int lp50xx_probe_dt(struct lp50xx *priv)
led_cdev = &led->mc_cdev.led_cdev;
led_cdev->brightness_set_blocking = lp50xx_brightness_set;
- fwnode_property_read_string(child, "linux,default-trigger",
- &led_cdev->default_trigger);
-
ret = devm_led_classdev_multicolor_register_ext(&priv->client->dev,
&led->mc_cdev,
&init_data);
diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c
index e9b16f3358d6f..f0533a337bc15 100644
--- a/drivers/leds/leds-lp8860.c
+++ b/drivers/leds/leds-lp8860.c
@@ -392,10 +392,6 @@ static int lp8860_probe(struct i2c_client *client,
if (!child_node)
return -EINVAL;
- led->led_dev.default_trigger = of_get_property(child_node,
- "linux,default-trigger",
- NULL);
-
led->enable_gpio = devm_gpiod_get_optional(&client->dev,
"enable", GPIOD_OUT_LOW);
if (IS_ERR(led->enable_gpio)) {
diff --git a/drivers/leds/leds-lt3593.c b/drivers/leds/leds-lt3593.c
index 061f02e3995ae..68e06434ac087 100644
--- a/drivers/leds/leds-lt3593.c
+++ b/drivers/leds/leds-lt3593.c
@@ -86,9 +86,6 @@ static int lt3593_led_probe(struct platform_device *pdev)
child = device_get_next_child_node(dev, NULL);
- fwnode_property_read_string(child, "linux,default-trigger",
- &led_data->cdev.default_trigger);
-
if (!fwnode_property_read_string(child, "default-state", &tmp)) {
if (!strcmp(tmp, "on"))
state = LEDS_GPIO_DEFSTATE_ON;
diff --git a/drivers/leds/leds-max77650.c b/drivers/leds/leds-max77650.c
index acc1174197c35..1eeac56b00146 100644
--- a/drivers/leds/leds-max77650.c
+++ b/drivers/leds/leds-max77650.c
@@ -100,9 +100,6 @@ static int max77650_led_probe(struct platform_device *pdev)
led->cdev.brightness_set_blocking = max77650_led_brightness_set;
led->cdev.max_brightness = MAX77650_LED_MAX_BRIGHTNESS;
- fwnode_property_read_string(child, "linux,default-trigger",
- &led->cdev.default_trigger);
-
init_data.fwnode = child;
init_data.devicename = "max77650";
/* for backwards compatibility if `label` is not present */
diff --git a/drivers/leds/leds-mt6323.c b/drivers/leds/leds-mt6323.c
index a1fceeb41d7e6..f59e0e8bda8bb 100644
--- a/drivers/leds/leds-mt6323.c
+++ b/drivers/leds/leds-mt6323.c
@@ -342,10 +342,6 @@ static int mt6323_led_set_dt_default(struct led_classdev *cdev,
const char *state;
int ret = 0;
- led->cdev.default_trigger = of_get_property(np,
- "linux,default-trigger",
- NULL);
-
state = of_get_property(np, "default-state", NULL);
if (state) {
if (!strcmp(state, "keep")) {
diff --git a/drivers/leds/leds-ns2.c b/drivers/leds/leds-ns2.c
index 030f426af3d75..e1ec5cbed07e8 100644
--- a/drivers/leds/leds-ns2.c
+++ b/drivers/leds/leds-ns2.c
@@ -185,9 +185,6 @@ static int ns2_led_register(struct device *dev, struct device_node *np,
if (IS_ERR(led->slow))
return PTR_ERR(led->slow);
- of_property_read_string(np, "linux,default-trigger",
- &led->cdev.default_trigger);
-
ret = of_property_count_u32_elems(np, "modes-map");
if (ret < 0 || ret % 3) {
dev_err(dev, "Missing or malformed modes-map for %pOF\n", np);
diff --git a/drivers/leds/leds-pm8058.c b/drivers/leds/leds-pm8058.c
index 5247b04abb7ef..fb2ab72c0c40c 100644
--- a/drivers/leds/leds-pm8058.c
+++ b/drivers/leds/leds-pm8058.c
@@ -117,8 +117,6 @@ static int pm8058_led_probe(struct platform_device *pdev)
return -EINVAL;
}
- led->cdev.default_trigger =
- of_get_property(np, "linux,default-trigger", NULL);
led->cdev.brightness_set = pm8058_led_set;
led->cdev.brightness_get = pm8058_led_get;
if (led->ledtype == PM8058_LED_TYPE_COMMON)
diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
index 4a014d2964123..2a16ae0bf022b 100644
--- a/drivers/leds/leds-pwm.c
+++ b/drivers/leds/leds-pwm.c
@@ -20,7 +20,6 @@
struct led_pwm {
const char *name;
- const char *default_trigger;
u8 active_low;
unsigned int max_brightness;
};
@@ -70,7 +69,6 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
led_data->active_low = led->active_low;
led_data->cdev.name = led->name;
- led_data->cdev.default_trigger = led->default_trigger;
led_data->cdev.brightness = LED_OFF;
led_data->cdev.max_brightness = led->max_brightness;
led_data->cdev.flags = LED_CORE_SUSPENDRESUME;
@@ -124,9 +122,6 @@ static int led_pwm_create_fwnode(struct device *dev, struct led_pwm_priv *priv)
return -EINVAL;
}
- fwnode_property_read_string(fwnode, "linux,default-trigger",
- &led.default_trigger);
-
led.active_low = fwnode_property_read_bool(fwnode,
"active-low");
fwnode_property_read_u32(fwnode, "max-brightness",
diff --git a/drivers/leds/leds-syscon.c b/drivers/leds/leds-syscon.c
index f54935fa650a7..7eddb8ecb44ec 100644
--- a/drivers/leds/leds-syscon.c
+++ b/drivers/leds/leds-syscon.c
@@ -85,8 +85,6 @@ static int syscon_led_probe(struct platform_device *pdev)
return -EINVAL;
if (of_property_read_u32(np, "mask", &sled->mask))
return -EINVAL;
- sled->cdev.default_trigger =
- of_get_property(np, "linux,default-trigger", NULL);
state = of_get_property(np, "default-state", NULL);
if (state) {
diff --git a/drivers/leds/leds-tlc591xx.c b/drivers/leds/leds-tlc591xx.c
index 5e84a0c7aacbd..f24271337bd84 100644
--- a/drivers/leds/leds-tlc591xx.c
+++ b/drivers/leds/leds-tlc591xx.c
@@ -199,9 +199,6 @@ tlc591xx_probe(struct i2c_client *client,
led = &priv->leds[reg];
led->active = true;
- led->ldev.default_trigger =
- of_get_property(child, "linux,default-trigger", NULL);
-
led->priv = priv;
led->led_no = reg;
led->ldev.brightness_set_blocking = tlc591xx_brightness_set;
diff --git a/drivers/leds/leds-turris-omnia.c b/drivers/leds/leds-turris-omnia.c
index 117976cf75c8a..8c5bdc3847ee7 100644
--- a/drivers/leds/leds-turris-omnia.c
+++ b/drivers/leds/leds-turris-omnia.c
@@ -121,8 +121,6 @@ static int omnia_led_register(struct i2c_client *client, struct omnia_led *led,
cdev->max_brightness = 255;
cdev->brightness_set_blocking = omnia_led_brightness_set_blocking;
- of_property_read_string(np, "linux,default-trigger", &cdev->default_trigger);
-
/* put the LED into software mode */
ret = i2c_smbus_write_byte_data(client, CMD_LED_MODE,
CMD_LED_MODE_LED(led->reg) |
--
2.26.2
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH leds v3 9/9] leds: parse linux,default-trigger DT property in LED core
2020-09-19 18:03 ` [PATCH leds v3 9/9] leds: parse linux,default-trigger DT property in LED core Marek Behún
@ 2020-09-24 12:10 ` Pavel Machek
0 siblings, 0 replies; 23+ messages in thread
From: Pavel Machek @ 2020-09-24 12:10 UTC (permalink / raw)
To: Marek Behún; +Cc: linux-leds, Dan Murphy
[-- Attachment #1: Type: text/plain, Size: 1037 bytes --]
Hi!
> Do the parsing of `linux,default-trigger` DT property to LED core.
> Currently it is done in many different drivers and the code is repeated.
>
> This patch removes the parsing from 23 drivers:
> an30259a, aw2013, bcm6328, bcm6358, cr0014114, el15203000, gpio,
> is31fl32xx, lm3532, lm36274, lm3692x, lm3697, lp50xx, lp8860, lt3593,
> max77650, mt6323, ns2, pm8058, pwm, syscon, tlc591xx and turris-omnia.
>
> There is one driver in drivers/input which parses this property on it's
> own. I shall send a separate patch there after this is applied.
>
> There are still 8 drivers that parse this property on their own because
> they do not pass the led_init_data structure to the registering
> function. I will try to refactor those in the future.
>
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
Thanks, I applied the series.
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread