* [PATCH 2/5] leds: flash: Convert non extended registration to inline
2019-10-01 18:04 [PATCH 1/5] leds: Kconfig: Be consistent with the usage of "LED" Dan Murphy
@ 2019-10-01 18:04 ` Dan Murphy
2019-10-01 18:04 ` [PATCH 3/5] leds: flash: Remove extern from the header file Dan Murphy
` (3 subsequent siblings)
4 siblings, 0 replies; 15+ messages in thread
From: Dan Murphy @ 2019-10-01 18:04 UTC (permalink / raw)
To: jacek.anaszewski, pavel; +Cc: linux-leds, linux-kernel, Dan Murphy
Convert the #define non-extended registration API to an
inline function.
Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 3/5] leds: flash: Remove extern from the header file
2019-10-01 18:04 [PATCH 1/5] leds: Kconfig: Be consistent with the usage of "LED" Dan Murphy
2019-10-01 18:04 ` [PATCH 2/5] leds: flash: Convert non extended registration to inline Dan Murphy
@ 2019-10-01 18:04 ` Dan Murphy
2019-10-01 20:57 ` Jacek Anaszewski
2019-10-01 18:04 ` [PATCH 4/5] leds: flash: Add devm_* functions to the flash class Dan Murphy
` (2 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Dan Murphy @ 2019-10-01 18:04 UTC (permalink / raw)
To: jacek.anaszewski, pavel; +Cc: linux-leds, linux-kernel, Dan Murphy
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 <dmurphy@ti.com>
---
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 3/5] leds: flash: Remove extern from the header file
2019-10-01 18:04 ` [PATCH 3/5] leds: flash: Remove extern from the header file Dan Murphy
@ 2019-10-01 20:57 ` Jacek Anaszewski
2019-10-02 11:56 ` Dan Murphy
0 siblings, 1 reply; 15+ messages in thread
From: Jacek Anaszewski @ 2019-10-01 20:57 UTC (permalink / raw)
To: Dan Murphy, pavel; +Cc: linux-leds, linux-kernel
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 <dmurphy@ti.com>
> ---
> 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 */
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 3/5] leds: flash: Remove extern from the header file
2019-10-01 20:57 ` Jacek Anaszewski
@ 2019-10-02 11:56 ` Dan Murphy
2019-10-02 19:53 ` Jacek Anaszewski
0 siblings, 1 reply; 15+ messages in thread
From: Dan Murphy @ 2019-10-02 11:56 UTC (permalink / raw)
To: Jacek Anaszewski, pavel; +Cc: linux-leds, linux-kernel
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/5] leds: flash: Remove extern from the header file
2019-10-02 11:56 ` Dan Murphy
@ 2019-10-02 19:53 ` Jacek Anaszewski
0 siblings, 0 replies; 15+ messages in thread
From: Jacek Anaszewski @ 2019-10-02 19:53 UTC (permalink / raw)
To: Dan Murphy, pavel; +Cc: linux-leds, linux-kernel
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 4/5] leds: flash: Add devm_* functions to the flash class
2019-10-01 18:04 [PATCH 1/5] leds: Kconfig: Be consistent with the usage of "LED" Dan Murphy
2019-10-01 18:04 ` [PATCH 2/5] leds: flash: Convert non extended registration to inline Dan Murphy
2019-10-01 18:04 ` [PATCH 3/5] leds: flash: Remove extern from the header file Dan Murphy
@ 2019-10-01 18:04 ` Dan Murphy
2019-10-01 21:06 ` Jacek Anaszewski
2019-10-01 18:04 ` [PATCH 5/5] leds: lm3601x: Convert class registration to device managed Dan Murphy
2019-10-01 21:16 ` [PATCH 1/5] leds: Kconfig: Be consistent with the usage of "LED" Jacek Anaszewski
4 siblings, 1 reply; 15+ messages in thread
From: Dan Murphy @ 2019-10-01 18:04 UTC (permalink / raw)
To: jacek.anaszewski, pavel; +Cc: linux-leds, linux-kernel, Dan Murphy
Add the missing device managed API for registration and
unregistration for the LED flash class.
Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 4/5] leds: flash: Add devm_* functions to the flash class
2019-10-01 18:04 ` [PATCH 4/5] leds: flash: Add devm_* functions to the flash class Dan Murphy
@ 2019-10-01 21:06 ` Jacek Anaszewski
2019-10-01 21:10 ` Jacek Anaszewski
2019-10-02 12:04 ` Dan Murphy
0 siblings, 2 replies; 15+ messages in thread
From: Jacek Anaszewski @ 2019-10-01 21:06 UTC (permalink / raw)
To: Dan Murphy, pavel; +Cc: linux-leds, linux-kernel
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 <dmurphy@ti.com>
> ---
> 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
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 4/5] leds: flash: Add devm_* functions to the flash class
2019-10-01 21:06 ` Jacek Anaszewski
@ 2019-10-01 21:10 ` Jacek Anaszewski
2019-10-02 12:04 ` Dan Murphy
1 sibling, 0 replies; 15+ messages in thread
From: Jacek Anaszewski @ 2019-10-01 21:10 UTC (permalink / raw)
To: Dan Murphy, pavel; +Cc: linux-leds, linux-kernel
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 <dmurphy@ti.com>
>> ---
>> 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
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 4/5] leds: flash: Add devm_* functions to the flash class
2019-10-01 21:06 ` Jacek Anaszewski
2019-10-01 21:10 ` Jacek Anaszewski
@ 2019-10-02 12:04 ` Dan Murphy
2019-10-02 19:55 ` Jacek Anaszewski
1 sibling, 1 reply; 15+ messages in thread
From: Dan Murphy @ 2019-10-02 12:04 UTC (permalink / raw)
To: Jacek Anaszewski, pavel; +Cc: linux-leds, linux-kernel
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 <dmurphy@ti.com>
>> ---
>> 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
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 4/5] leds: flash: Add devm_* functions to the flash class
2019-10-02 12:04 ` Dan Murphy
@ 2019-10-02 19:55 ` Jacek Anaszewski
0 siblings, 0 replies; 15+ messages in thread
From: Jacek Anaszewski @ 2019-10-02 19:55 UTC (permalink / raw)
To: Dan Murphy, pavel; +Cc: linux-leds, linux-kernel
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 <dmurphy@ti.com>
>>> ---
>>> 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 5/5] leds: lm3601x: Convert class registration to device managed
2019-10-01 18:04 [PATCH 1/5] leds: Kconfig: Be consistent with the usage of "LED" Dan Murphy
` (2 preceding siblings ...)
2019-10-01 18:04 ` [PATCH 4/5] leds: flash: Add devm_* functions to the flash class Dan Murphy
@ 2019-10-01 18:04 ` Dan Murphy
2019-10-01 19:39 ` Jacek Anaszewski
2019-10-01 21:16 ` [PATCH 1/5] leds: Kconfig: Be consistent with the usage of "LED" Jacek Anaszewski
4 siblings, 1 reply; 15+ messages in thread
From: Dan Murphy @ 2019-10-01 18:04 UTC (permalink / raw)
To: jacek.anaszewski, pavel; +Cc: linux-leds, linux-kernel, Dan Murphy
Convert LED flash class registration to device managed class
registration API.
Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 5/5] leds: lm3601x: Convert class registration to device managed
2019-10-01 18:04 ` [PATCH 5/5] leds: lm3601x: Convert class registration to device managed Dan Murphy
@ 2019-10-01 19:39 ` Jacek Anaszewski
2019-10-01 20:11 ` Dan Murphy
0 siblings, 1 reply; 15+ messages in thread
From: Jacek Anaszewski @ 2019-10-01 19:39 UTC (permalink / raw)
To: Dan Murphy, pavel; +Cc: linux-leds, linux-kernel
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 <dmurphy@ti.com>
> ---
> 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 5/5] leds: lm3601x: Convert class registration to device managed
2019-10-01 19:39 ` Jacek Anaszewski
@ 2019-10-01 20:11 ` Dan Murphy
0 siblings, 0 replies; 15+ messages in thread
From: Dan Murphy @ 2019-10-01 20:11 UTC (permalink / raw)
To: Jacek Anaszewski, pavel; +Cc: linux-leds, linux-kernel
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 <dmurphy@ti.com>
>> ---
>> 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.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/5] leds: Kconfig: Be consistent with the usage of "LED"
2019-10-01 18:04 [PATCH 1/5] leds: Kconfig: Be consistent with the usage of "LED" Dan Murphy
` (3 preceding siblings ...)
2019-10-01 18:04 ` [PATCH 5/5] leds: lm3601x: Convert class registration to device managed Dan Murphy
@ 2019-10-01 21:16 ` Jacek Anaszewski
4 siblings, 0 replies; 15+ messages in thread
From: Jacek Anaszewski @ 2019-10-01 21:16 UTC (permalink / raw)
To: Dan Murphy, pavel; +Cc: linux-leds, linux-kernel
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 <dmurphy@ti.com>
> ---
> 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
^ permalink raw reply [flat|nested] 15+ messages in thread