Update the Kconfig to be consistent in the case of using
"LED" in the Kconfig. LED is an acronym and should be
capitalized.
Signed-off-by: Dan Murphy <[email protected]>
---
drivers/leds/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 6e7703fd03d0..4b68520ac251 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -17,7 +17,7 @@ if NEW_LEDS
config LEDS_CLASS
tristate "LED Class Support"
help
- This option enables the led sysfs class in /sys/class/leds. You'll
+ This option enables the LED sysfs class in /sys/class/leds. You'll
need this to do anything useful with LEDs. If unsure, say N.
config LEDS_CLASS_FLASH
@@ -35,7 +35,7 @@ config LEDS_BRIGHTNESS_HW_CHANGED
depends on LEDS_CLASS
help
This option enables support for the brightness_hw_changed attribute
- for led sysfs class devices under /sys/class/leds.
+ for LED sysfs class devices under /sys/class/leds.
See Documentation/ABI/testing/sysfs-class-led for details.
--
2.22.0.214.g8dca754b1e
Add the missing device managed API for registration and
unregistration for the LED flash class.
Signed-off-by: Dan Murphy <[email protected]>
---
drivers/leds/led-class-flash.c | 50 +++++++++++++++++++++++++++++++++
include/linux/led-class-flash.h | 14 +++++++++
2 files changed, 64 insertions(+)
diff --git a/drivers/leds/led-class-flash.c b/drivers/leds/led-class-flash.c
index 60c3de5c6b9f..c2b4fd02a1bc 100644
--- a/drivers/leds/led-class-flash.c
+++ b/drivers/leds/led-class-flash.c
@@ -327,6 +327,56 @@ void led_classdev_flash_unregister(struct led_classdev_flash *fled_cdev)
}
EXPORT_SYMBOL_GPL(led_classdev_flash_unregister);
+static void devm_led_classdev_flash_release(struct device *dev, void *res)
+{
+ led_classdev_flash_unregister(*(struct led_classdev_flash **)res);
+}
+
+int devm_led_classdev_flash_register_ext(struct device *parent,
+ struct led_classdev_flash *fled_cdev,
+ struct led_init_data *init_data)
+{
+ struct led_classdev_flash **dr;
+ int ret;
+
+ dr = devres_alloc(devm_led_classdev_flash_release, sizeof(*dr),
+ GFP_KERNEL);
+ if (!dr)
+ return -ENOMEM;
+
+ ret = led_classdev_flash_register_ext(parent, fled_cdev, init_data);
+ if (ret) {
+ devres_free(dr);
+ return ret;
+ }
+
+ *dr = fled_cdev;
+ devres_add(parent, dr);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(devm_led_classdev_flash_register_ext);
+
+static int devm_led_classdev_flash_match(struct device *dev,
+ void *res, void *data)
+{
+ struct fled_cdev **p = res;
+
+ if (WARN_ON(!p || !*p))
+ return 0;
+
+ return *p == data;
+}
+
+void devm_led_classdev_flash_unregister(struct device *dev,
+ struct led_classdev_flash *fled_cdev)
+{
+ WARN_ON(devres_release(dev,
+ devm_led_classdev_flash_release,
+ devm_led_classdev_flash_match, fled_cdev));
+}
+EXPORT_SYMBOL_GPL(devm_led_classdev_flash_unregister);
+
static void led_clamp_align(struct led_flash_setting *s)
{
u32 v, offset;
diff --git a/include/linux/led-class-flash.h b/include/linux/led-class-flash.h
index 1bd83159fa4c..21a3358a1731 100644
--- a/include/linux/led-class-flash.h
+++ b/include/linux/led-class-flash.h
@@ -113,6 +113,20 @@ static inline int led_classdev_flash_register(struct device *parent,
*/
void led_classdev_flash_unregister(struct led_classdev_flash *fled_cdev);
+int devm_led_classdev_flash_register_ext(struct device *parent,
+ struct led_classdev_flash *fled_cdev,
+ struct led_init_data *init_data);
+
+
+static inline int devm_led_classdev_flash_register(struct device *parent,
+ struct led_classdev_flash *fled_cdev)
+{
+ return devm_led_classdev_flash_register_ext(parent, fled_cdev, NULL);
+}
+
+void devm_led_classdev_flash_unregister(struct device *parent,
+ struct led_classdev_flash *fled_cdev);
+
/**
* led_set_flash_strobe - setup flash strobe
* @fled_cdev: the flash LED to set strobe on
--
2.22.0.214.g8dca754b1e
Convert the #define non-extended registration API to an
inline function.
Signed-off-by: Dan Murphy <[email protected]>
---
include/linux/led-class-flash.h | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/include/linux/led-class-flash.h b/include/linux/led-class-flash.h
index 1e824963af17..7ff287a9e2a2 100644
--- a/include/linux/led-class-flash.h
+++ b/include/linux/led-class-flash.h
@@ -98,8 +98,11 @@ extern int led_classdev_flash_register_ext(struct device *parent,
struct led_classdev_flash *fled_cdev,
struct led_init_data *init_data);
-#define led_classdev_flash_register(parent, fled_cdev) \
- led_classdev_flash_register_ext(parent, fled_cdev, NULL)
+static inline int led_classdev_flash_register(struct device *parent,
+ struct led_classdev_flash *fled_cdev)
+{
+ return led_classdev_flash_register_ext(parent, fled_cdev, NULL);
+}
/**
* led_classdev_flash_unregister - unregisters an object of led_classdev class
--
2.22.0.214.g8dca754b1e
Convert LED flash class registration to device managed class
registration API.
Signed-off-by: Dan Murphy <[email protected]>
---
drivers/leds/leds-lm3601x.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/leds/leds-lm3601x.c b/drivers/leds/leds-lm3601x.c
index b02972f1a341..a68e4f97739c 100644
--- a/drivers/leds/leds-lm3601x.c
+++ b/drivers/leds/leds-lm3601x.c
@@ -350,8 +350,7 @@ static int lm3601x_register_leds(struct lm3601x_led *led,
init_data.devicename = led->client->name;
init_data.default_label = (led->led_mode == LM3601X_LED_TORCH) ?
"torch" : "infrared";
-
- return led_classdev_flash_register_ext(&led->client->dev,
+ return devm_led_classdev_flash_register_ext(&led->client->dev,
&led->fled_cdev, &init_data);
}
--
2.22.0.214.g8dca754b1e
extern is implied and is not needed in the header file.
Remove the extern keyword and re-align the code.
Signed-off-by: Dan Murphy <[email protected]>
---
include/linux/led-class-flash.h | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/include/linux/led-class-flash.h b/include/linux/led-class-flash.h
index 7ff287a9e2a2..1bd83159fa4c 100644
--- a/include/linux/led-class-flash.h
+++ b/include/linux/led-class-flash.h
@@ -94,12 +94,12 @@ static inline struct led_classdev_flash *lcdev_to_flcdev(
*
* Returns: 0 on success or negative error value on failure
*/
-extern int led_classdev_flash_register_ext(struct device *parent,
- struct led_classdev_flash *fled_cdev,
- struct led_init_data *init_data);
+int led_classdev_flash_register_ext(struct device *parent,
+ struct led_classdev_flash *fled_cdev,
+ struct led_init_data *init_data);
static inline int led_classdev_flash_register(struct device *parent,
- struct led_classdev_flash *fled_cdev)
+ struct led_classdev_flash *fled_cdev)
{
return led_classdev_flash_register_ext(parent, fled_cdev, NULL);
}
@@ -111,7 +111,7 @@ static inline int led_classdev_flash_register(struct device *parent,
*
* Unregister a previously registered via led_classdev_flash_register object
*/
-extern void led_classdev_flash_unregister(struct led_classdev_flash *fled_cdev);
+void led_classdev_flash_unregister(struct led_classdev_flash *fled_cdev);
/**
* led_set_flash_strobe - setup flash strobe
@@ -159,8 +159,8 @@ static inline int led_get_flash_strobe(struct led_classdev_flash *fled_cdev,
*
* Returns: 0 on success or negative error value on failure
*/
-extern int led_set_flash_brightness(struct led_classdev_flash *fled_cdev,
- u32 brightness);
+int led_set_flash_brightness(struct led_classdev_flash *fled_cdev,
+ u32 brightness);
/**
* led_update_flash_brightness - update flash LED brightness
@@ -171,7 +171,7 @@ extern int led_set_flash_brightness(struct led_classdev_flash *fled_cdev,
*
* Returns: 0 on success or negative error value on failure
*/
-extern int led_update_flash_brightness(struct led_classdev_flash *fled_cdev);
+int led_update_flash_brightness(struct led_classdev_flash *fled_cdev);
/**
* led_set_flash_timeout - set flash LED timeout
@@ -182,8 +182,7 @@ extern int led_update_flash_brightness(struct led_classdev_flash *fled_cdev);
*
* Returns: 0 on success or negative error value on failure
*/
-extern int led_set_flash_timeout(struct led_classdev_flash *fled_cdev,
- u32 timeout);
+int led_set_flash_timeout(struct led_classdev_flash *fled_cdev, u32 timeout);
/**
* led_get_flash_fault - get the flash LED fault
@@ -194,7 +193,6 @@ extern int led_set_flash_timeout(struct led_classdev_flash *fled_cdev,
*
* Returns: 0 on success or negative error value on failure
*/
-extern int led_get_flash_fault(struct led_classdev_flash *fled_cdev,
- u32 *fault);
+int led_get_flash_fault(struct led_classdev_flash *fled_cdev, u32 *fault);
#endif /* __LINUX_FLASH_LEDS_H_INCLUDED */
--
2.22.0.214.g8dca754b1e
Hi Dan,
Thank you for the patch.
On 10/1/19 8:04 PM, Dan Murphy wrote:
> Convert LED flash class registration to device managed class
> registration API.
>
> Signed-off-by: Dan Murphy <[email protected]>
> ---
> drivers/leds/leds-lm3601x.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/leds/leds-lm3601x.c b/drivers/leds/leds-lm3601x.c
> index b02972f1a341..a68e4f97739c 100644
> --- a/drivers/leds/leds-lm3601x.c
> +++ b/drivers/leds/leds-lm3601x.c
> @@ -350,8 +350,7 @@ static int lm3601x_register_leds(struct lm3601x_led *led,
> init_data.devicename = led->client->name;
> init_data.default_label = (led->led_mode == LM3601X_LED_TORCH) ?
> "torch" : "infrared";
> -
> - return led_classdev_flash_register_ext(&led->client->dev,
> + return devm_led_classdev_flash_register_ext(&led->client->dev,
> &led->fled_cdev, &init_data);
You need to remove led_classdev_flash_unregister(&led->fled_cdev) from
lm3601x_remove() to complete this improvement.
--
Best regards,
Jacek Anaszewski
Jacek
On 10/1/19 2:39 PM, Jacek Anaszewski wrote:
> Hi Dan,
>
> Thank you for the patch.
>
> On 10/1/19 8:04 PM, Dan Murphy wrote:
>> Convert LED flash class registration to device managed class
>> registration API.
>>
>> Signed-off-by: Dan Murphy <[email protected]>
>> ---
>> drivers/leds/leds-lm3601x.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/leds/leds-lm3601x.c b/drivers/leds/leds-lm3601x.c
>> index b02972f1a341..a68e4f97739c 100644
>> --- a/drivers/leds/leds-lm3601x.c
>> +++ b/drivers/leds/leds-lm3601x.c
>> @@ -350,8 +350,7 @@ static int lm3601x_register_leds(struct lm3601x_led *led,
>> init_data.devicename = led->client->name;
>> init_data.default_label = (led->led_mode == LM3601X_LED_TORCH) ?
>> "torch" : "infrared";
>> -
>> - return led_classdev_flash_register_ext(&led->client->dev,
>> + return devm_led_classdev_flash_register_ext(&led->client->dev,
>> &led->fled_cdev, &init_data);
> You need to remove led_classdev_flash_unregister(&led->fled_cdev) from
> lm3601x_remove() to complete this improvement.
>
Ack.
Dan,
Thank you for the patch.
Could we have similar patch for leds.h when we are at it,
if you wouldn't mind?
--
Best regards,
Jacek Anaszewski
On 10/1/19 8:04 PM, Dan Murphy wrote:
> extern is implied and is not needed in the header file.
> Remove the extern keyword and re-align the code.
>
> Signed-off-by: Dan Murphy <[email protected]>
> ---
> include/linux/led-class-flash.h | 22 ++++++++++------------
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/include/linux/led-class-flash.h b/include/linux/led-class-flash.h
> index 7ff287a9e2a2..1bd83159fa4c 100644
> --- a/include/linux/led-class-flash.h
> +++ b/include/linux/led-class-flash.h
> @@ -94,12 +94,12 @@ static inline struct led_classdev_flash *lcdev_to_flcdev(
> *
> * Returns: 0 on success or negative error value on failure
> */
> -extern int led_classdev_flash_register_ext(struct device *parent,
> - struct led_classdev_flash *fled_cdev,
> - struct led_init_data *init_data);
> +int led_classdev_flash_register_ext(struct device *parent,
> + struct led_classdev_flash *fled_cdev,
> + struct led_init_data *init_data);
>
> static inline int led_classdev_flash_register(struct device *parent,
> - struct led_classdev_flash *fled_cdev)
> + struct led_classdev_flash *fled_cdev)
> {
> return led_classdev_flash_register_ext(parent, fled_cdev, NULL);
> }
> @@ -111,7 +111,7 @@ static inline int led_classdev_flash_register(struct device *parent,
> *
> * Unregister a previously registered via led_classdev_flash_register object
> */
> -extern void led_classdev_flash_unregister(struct led_classdev_flash *fled_cdev);
> +void led_classdev_flash_unregister(struct led_classdev_flash *fled_cdev);
>
> /**
> * led_set_flash_strobe - setup flash strobe
> @@ -159,8 +159,8 @@ static inline int led_get_flash_strobe(struct led_classdev_flash *fled_cdev,
> *
> * Returns: 0 on success or negative error value on failure
> */
> -extern int led_set_flash_brightness(struct led_classdev_flash *fled_cdev,
> - u32 brightness);
> +int led_set_flash_brightness(struct led_classdev_flash *fled_cdev,
> + u32 brightness);
>
> /**
> * led_update_flash_brightness - update flash LED brightness
> @@ -171,7 +171,7 @@ extern int led_set_flash_brightness(struct led_classdev_flash *fled_cdev,
> *
> * Returns: 0 on success or negative error value on failure
> */
> -extern int led_update_flash_brightness(struct led_classdev_flash *fled_cdev);
> +int led_update_flash_brightness(struct led_classdev_flash *fled_cdev);
>
> /**
> * led_set_flash_timeout - set flash LED timeout
> @@ -182,8 +182,7 @@ extern int led_update_flash_brightness(struct led_classdev_flash *fled_cdev);
> *
> * Returns: 0 on success or negative error value on failure
> */
> -extern int led_set_flash_timeout(struct led_classdev_flash *fled_cdev,
> - u32 timeout);
> +int led_set_flash_timeout(struct led_classdev_flash *fled_cdev, u32 timeout);
>
> /**
> * led_get_flash_fault - get the flash LED fault
> @@ -194,7 +193,6 @@ extern int led_set_flash_timeout(struct led_classdev_flash *fled_cdev,
> *
> * Returns: 0 on success or negative error value on failure
> */
> -extern int led_get_flash_fault(struct led_classdev_flash *fled_cdev,
> - u32 *fault);
> +int led_get_flash_fault(struct led_classdev_flash *fled_cdev, u32 *fault);
>
> #endif /* __LINUX_FLASH_LEDS_H_INCLUDED */
>
Moreover, checkpatch.pl complains about whitespaces
in two places of this patch.
On 10/1/19 11:06 PM, Jacek Anaszewski wrote:
> Dan,
>
> Thank you for the patch. One funny omission caught my
> eye here and in led-class.c when making visual comparison.
> Please refer below.
>
> On 10/1/19 8:04 PM, Dan Murphy wrote:
>> Add the missing device managed API for registration and
>> unregistration for the LED flash class.
>>
>> Signed-off-by: Dan Murphy <[email protected]>
>> ---
>> drivers/leds/led-class-flash.c | 50 +++++++++++++++++++++++++++++++++
>> include/linux/led-class-flash.h | 14 +++++++++
>> 2 files changed, 64 insertions(+)
>>
>> diff --git a/drivers/leds/led-class-flash.c b/drivers/leds/led-class-flash.c
>> index 60c3de5c6b9f..c2b4fd02a1bc 100644
>> --- a/drivers/leds/led-class-flash.c
>> +++ b/drivers/leds/led-class-flash.c
>> @@ -327,6 +327,56 @@ void led_classdev_flash_unregister(struct led_classdev_flash *fled_cdev)
>> }
>> EXPORT_SYMBOL_GPL(led_classdev_flash_unregister);
>>
>> +static void devm_led_classdev_flash_release(struct device *dev, void *res)
>> +{
>> + led_classdev_flash_unregister(*(struct led_classdev_flash **)res);
>> +}
>> +
>> +int devm_led_classdev_flash_register_ext(struct device *parent,
>> + struct led_classdev_flash *fled_cdev,
>> + struct led_init_data *init_data)
>> +{
>> + struct led_classdev_flash **dr;
>> + int ret;
>> +
>> + dr = devres_alloc(devm_led_classdev_flash_release, sizeof(*dr),
>> + GFP_KERNEL);
>> + if (!dr)
>> + return -ENOMEM;
>> +
>> + ret = led_classdev_flash_register_ext(parent, fled_cdev, init_data);
>> + if (ret) {
>> + devres_free(dr);
>> + return ret;
>> + }
>> +
>> + *dr = fled_cdev;
>> + devres_add(parent, dr);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(devm_led_classdev_flash_register_ext);
>> +
>> +static int devm_led_classdev_flash_match(struct device *dev,
>> + void *res, void *data)
>> +{
>> + struct fled_cdev **p = res;
>
> We don't have struct fled_cdev. This name is used for variables
> of struct led_clssdev_flash type in drivers.
>
> We don't get even compiler warning here because this is cast
> from void pointer.
>
> Funny thing is that you seem to have followed similar flaw in
> devm_led_classdev_match() where struct led_cdev has been
> introduced.
>
> We need to fix both cases.
>
>> +
>> + if (WARN_ON(!p || !*p))
>> + return 0;
>> +
>> + return *p == data;
>> +}
>> +
>> +void devm_led_classdev_flash_unregister(struct device *dev,
>> + struct led_classdev_flash *fled_cdev)
>> +{
>> + WARN_ON(devres_release(dev,
>> + devm_led_classdev_flash_release,
>> + devm_led_classdev_flash_match, fled_cdev));
>> +}
>> +EXPORT_SYMBOL_GPL(devm_led_classdev_flash_unregister);
>> +
>> static void led_clamp_align(struct led_flash_setting *s)
>> {
>> u32 v, offset;
>> diff --git a/include/linux/led-class-flash.h b/include/linux/led-class-flash.h
>> index 1bd83159fa4c..21a3358a1731 100644
>> --- a/include/linux/led-class-flash.h
>> +++ b/include/linux/led-class-flash.h
>> @@ -113,6 +113,20 @@ static inline int led_classdev_flash_register(struct device *parent,
>> */
>> void led_classdev_flash_unregister(struct led_classdev_flash *fled_cdev);
>>
>> +int devm_led_classdev_flash_register_ext(struct device *parent,
>> + struct led_classdev_flash *fled_cdev,
>> + struct led_init_data *init_data);
>> +
>> +
>> +static inline int devm_led_classdev_flash_register(struct device *parent,
>> + struct led_classdev_flash *fled_cdev)
>> +{
>> + return devm_led_classdev_flash_register_ext(parent, fled_cdev, NULL);
>> +}
>> +
>> +void devm_led_classdev_flash_unregister(struct device *parent,
>> + struct led_classdev_flash *fled_cdev);
>> +
>> /**
>> * led_set_flash_strobe - setup flash strobe
>> * @fled_cdev: the flash LED to set strobe on
>>
>
--
Best regards,
Jacek Anaszewski
Dan,
Thank you for the patch. One funny omission caught my
eye here and in led-class.c when making visual comparison.
Please refer below.
On 10/1/19 8:04 PM, Dan Murphy wrote:
> Add the missing device managed API for registration and
> unregistration for the LED flash class.
>
> Signed-off-by: Dan Murphy <[email protected]>
> ---
> drivers/leds/led-class-flash.c | 50 +++++++++++++++++++++++++++++++++
> include/linux/led-class-flash.h | 14 +++++++++
> 2 files changed, 64 insertions(+)
>
> diff --git a/drivers/leds/led-class-flash.c b/drivers/leds/led-class-flash.c
> index 60c3de5c6b9f..c2b4fd02a1bc 100644
> --- a/drivers/leds/led-class-flash.c
> +++ b/drivers/leds/led-class-flash.c
> @@ -327,6 +327,56 @@ void led_classdev_flash_unregister(struct led_classdev_flash *fled_cdev)
> }
> EXPORT_SYMBOL_GPL(led_classdev_flash_unregister);
>
> +static void devm_led_classdev_flash_release(struct device *dev, void *res)
> +{
> + led_classdev_flash_unregister(*(struct led_classdev_flash **)res);
> +}
> +
> +int devm_led_classdev_flash_register_ext(struct device *parent,
> + struct led_classdev_flash *fled_cdev,
> + struct led_init_data *init_data)
> +{
> + struct led_classdev_flash **dr;
> + int ret;
> +
> + dr = devres_alloc(devm_led_classdev_flash_release, sizeof(*dr),
> + GFP_KERNEL);
> + if (!dr)
> + return -ENOMEM;
> +
> + ret = led_classdev_flash_register_ext(parent, fled_cdev, init_data);
> + if (ret) {
> + devres_free(dr);
> + return ret;
> + }
> +
> + *dr = fled_cdev;
> + devres_add(parent, dr);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(devm_led_classdev_flash_register_ext);
> +
> +static int devm_led_classdev_flash_match(struct device *dev,
> + void *res, void *data)
> +{
> + struct fled_cdev **p = res;
We don't have struct fled_cdev. This name is used for variables
of struct led_clssdev_flash type in drivers.
We don't get even compiler warning here because this is cast
from void pointer.
Funny thing is that you seem to have followed similar flaw in
devm_led_classdev_match() where struct led_cdev has been
introduced.
We need to fix both cases.
> +
> + if (WARN_ON(!p || !*p))
> + return 0;
> +
> + return *p == data;
> +}
> +
> +void devm_led_classdev_flash_unregister(struct device *dev,
> + struct led_classdev_flash *fled_cdev)
> +{
> + WARN_ON(devres_release(dev,
> + devm_led_classdev_flash_release,
> + devm_led_classdev_flash_match, fled_cdev));
> +}
> +EXPORT_SYMBOL_GPL(devm_led_classdev_flash_unregister);
> +
> static void led_clamp_align(struct led_flash_setting *s)
> {
> u32 v, offset;
> diff --git a/include/linux/led-class-flash.h b/include/linux/led-class-flash.h
> index 1bd83159fa4c..21a3358a1731 100644
> --- a/include/linux/led-class-flash.h
> +++ b/include/linux/led-class-flash.h
> @@ -113,6 +113,20 @@ static inline int led_classdev_flash_register(struct device *parent,
> */
> void led_classdev_flash_unregister(struct led_classdev_flash *fled_cdev);
>
> +int devm_led_classdev_flash_register_ext(struct device *parent,
> + struct led_classdev_flash *fled_cdev,
> + struct led_init_data *init_data);
> +
> +
> +static inline int devm_led_classdev_flash_register(struct device *parent,
> + struct led_classdev_flash *fled_cdev)
> +{
> + return devm_led_classdev_flash_register_ext(parent, fled_cdev, NULL);
> +}
> +
> +void devm_led_classdev_flash_unregister(struct device *parent,
> + struct led_classdev_flash *fled_cdev);
> +
> /**
> * led_set_flash_strobe - setup flash strobe
> * @fled_cdev: the flash LED to set strobe on
>
--
Best regards,
Jacek Anaszewski
Dan,
On 10/1/19 8:04 PM, Dan Murphy wrote:
> Update the Kconfig to be consistent in the case of using
> "LED" in the Kconfig. LED is an acronym and should be
> capitalized.
>
> Signed-off-by: Dan Murphy <[email protected]>
> ---
> drivers/leds/Kconfig | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 6e7703fd03d0..4b68520ac251 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -17,7 +17,7 @@ if NEW_LEDS
> config LEDS_CLASS
> tristate "LED Class Support"
> help
> - This option enables the led sysfs class in /sys/class/leds. You'll
> + This option enables the LED sysfs class in /sys/class/leds. You'll
> need this to do anything useful with LEDs. If unsure, say N.
>
> config LEDS_CLASS_FLASH
> @@ -35,7 +35,7 @@ config LEDS_BRIGHTNESS_HW_CHANGED
> depends on LEDS_CLASS
> help
> This option enables support for the brightness_hw_changed attribute
> - for led sysfs class devices under /sys/class/leds.
> + for LED sysfs class devices under /sys/class/leds.
>
> See Documentation/ABI/testing/sysfs-class-led for details.
>
>
I've applied patches 1/5 and 2/5 from this patch set. Thanks.
--
Best regards,
Jacek Anaszewski
Jacek
On 10/1/19 3:57 PM, Jacek Anaszewski wrote:
> Dan,
>
> Thank you for the patch.
>
> Could we have similar patch for leds.h when we are at it,
> if you wouldn't mind?
>
Sure do you want it in this patch or a separate patch?
Dan
Jacek
On 10/1/19 4:06 PM, Jacek Anaszewski wrote:
> Dan,
>
> Thank you for the patch. One funny omission caught my
> eye here and in led-class.c when making visual comparison.
> Please refer below.
>
> On 10/1/19 8:04 PM, Dan Murphy wrote:
>> Add the missing device managed API for registration and
>> unregistration for the LED flash class.
>>
>> Signed-off-by: Dan Murphy <[email protected]>
>> ---
>> drivers/leds/led-class-flash.c | 50 +++++++++++++++++++++++++++++++++
>> include/linux/led-class-flash.h | 14 +++++++++
>> 2 files changed, 64 insertions(+)
>>
>> diff --git a/drivers/leds/led-class-flash.c b/drivers/leds/led-class-flash.c
>> index 60c3de5c6b9f..c2b4fd02a1bc 100644
>> --- a/drivers/leds/led-class-flash.c
>> +++ b/drivers/leds/led-class-flash.c
>> @@ -327,6 +327,56 @@ void led_classdev_flash_unregister(struct led_classdev_flash *fled_cdev)
>> }
>> EXPORT_SYMBOL_GPL(led_classdev_flash_unregister);
>>
>> +static void devm_led_classdev_flash_release(struct device *dev, void *res)
>> +{
>> + led_classdev_flash_unregister(*(struct led_classdev_flash **)res);
>> +}
>> +
>> +int devm_led_classdev_flash_register_ext(struct device *parent,
>> + struct led_classdev_flash *fled_cdev,
>> + struct led_init_data *init_data)
>> +{
>> + struct led_classdev_flash **dr;
>> + int ret;
>> +
>> + dr = devres_alloc(devm_led_classdev_flash_release, sizeof(*dr),
>> + GFP_KERNEL);
>> + if (!dr)
>> + return -ENOMEM;
>> +
>> + ret = led_classdev_flash_register_ext(parent, fled_cdev, init_data);
>> + if (ret) {
>> + devres_free(dr);
>> + return ret;
>> + }
>> +
>> + *dr = fled_cdev;
>> + devres_add(parent, dr);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(devm_led_classdev_flash_register_ext);
>> +
>> +static int devm_led_classdev_flash_match(struct device *dev,
>> + void *res, void *data)
>> +{
>> + struct fled_cdev **p = res;
> We don't have struct fled_cdev. This name is used for variables
> of struct led_clssdev_flash type in drivers.
>
> We don't get even compiler warning here because this is cast
> from void pointer.
>
> Funny thing is that you seem to have followed similar flaw in
> devm_led_classdev_match() where struct led_cdev has been
> introduced.
>
> We need to fix both cases.
OK I will fix the leds-class in a separate patch in case it causes issues.
Dan
Dan,
On 10/2/19 2:04 PM, Dan Murphy wrote:
> Jacek
>
> On 10/1/19 4:06 PM, Jacek Anaszewski wrote:
>> Dan,
>>
>> Thank you for the patch. One funny omission caught my
>> eye here and in led-class.c when making visual comparison.
>> Please refer below.
>>
>> On 10/1/19 8:04 PM, Dan Murphy wrote:
>>> Add the missing device managed API for registration and
>>> unregistration for the LED flash class.
>>>
>>> Signed-off-by: Dan Murphy <[email protected]>
>>> ---
>>> drivers/leds/led-class-flash.c | 50 +++++++++++++++++++++++++++++++++
>>> include/linux/led-class-flash.h | 14 +++++++++
>>> 2 files changed, 64 insertions(+)
>>>
>>> diff --git a/drivers/leds/led-class-flash.c
>>> b/drivers/leds/led-class-flash.c
>>> index 60c3de5c6b9f..c2b4fd02a1bc 100644
>>> --- a/drivers/leds/led-class-flash.c
>>> +++ b/drivers/leds/led-class-flash.c
>>> @@ -327,6 +327,56 @@ void led_classdev_flash_unregister(struct
>>> led_classdev_flash *fled_cdev)
>>> }
>>> EXPORT_SYMBOL_GPL(led_classdev_flash_unregister);
>>> +static void devm_led_classdev_flash_release(struct device *dev,
>>> void *res)
>>> +{
>>> + led_classdev_flash_unregister(*(struct led_classdev_flash **)res);
>>> +}
>>> +
>>> +int devm_led_classdev_flash_register_ext(struct device *parent,
>>> + struct led_classdev_flash *fled_cdev,
>>> + struct led_init_data *init_data)
>>> +{
>>> + struct led_classdev_flash **dr;
>>> + int ret;
>>> +
>>> + dr = devres_alloc(devm_led_classdev_flash_release, sizeof(*dr),
>>> + GFP_KERNEL);
>>> + if (!dr)
>>> + return -ENOMEM;
>>> +
>>> + ret = led_classdev_flash_register_ext(parent, fled_cdev,
>>> init_data);
>>> + if (ret) {
>>> + devres_free(dr);
>>> + return ret;
>>> + }
>>> +
>>> + *dr = fled_cdev;
>>> + devres_add(parent, dr);
>>> +
>>> + return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(devm_led_classdev_flash_register_ext);
>>> +
>>> +static int devm_led_classdev_flash_match(struct device *dev,
>>> + void *res, void *data)
>>> +{
>>> + struct fled_cdev **p = res;
>> We don't have struct fled_cdev. This name is used for variables
>> of struct led_clssdev_flash type in drivers.
>>
>> We don't get even compiler warning here because this is cast
>> from void pointer.
>>
>> Funny thing is that you seem to have followed similar flaw in
>> devm_led_classdev_match() where struct led_cdev has been
>> introduced.
>>
>> We need to fix both cases.
>
> OK I will fix the leds-class in a separate patch in case it causes issues.
It doesn't cause issues now but is misleading.
--
Best regards,
Jacek Anaszewski
On 10/2/19 1:56 PM, Dan Murphy wrote:
> Jacek
>
> On 10/1/19 3:57 PM, Jacek Anaszewski wrote:
>> Dan,
>>
>> Thank you for the patch.
>>
>> Could we have similar patch for leds.h when we are at it,
>> if you wouldn't mind?
>>
> Sure do you want it in this patch or a separate patch?
Separate please. Thanks!
--
Best regards,
Jacek Anaszewski