Hi Dan, Thanks for the review.
On 11/08/2018 06:50 PM, Dan Murphy wrote: > Jacek > > On 11/06/2018 04:07 PM, Jacek Anaszewski wrote: [...] >> >> /** >> - * of_led_classdev_register - register a new object of led_classdev class. >> + * led_classdev_register_ext - register a new object of led_classdev class. > > For this should the comment indicate a different between the extended and > non-extended calls? > Like the _ext might say register a new object of led_classdev with init_data? > > Same comments for each addition below. Agreed, I will amend it as you propose. > Otherwise with Baolin's comments for 1/24 > > Acked-by: Dan Murphy <[email protected]> > > Dan > >> * >> * @parent: parent of LED device >> * @led_cdev: the led_classdev structure for this device. >> - * @np: DT node describing this LED >> + * @init_data: LED class device initialization data >> */ >> -int of_led_classdev_register(struct device *parent, struct device_node *np, >> - struct led_classdev *led_cdev) >> +int led_classdev_register_ext(struct device *parent, >> + struct led_classdev *led_cdev, >> + struct led_init_data *init_data) >> { >> char name[LED_MAX_NAME_SIZE]; >> int ret; >> >> + if (init_data && init_data->name) { >> + led_cdev->name = init_data->name; >> + } else { >> + dev_info(parent, "init_data not available"); >> + } >> + >> ret = led_classdev_next_name(led_cdev->name, name, sizeof(name)); >> if (ret < 0) >> return ret; >> @@ -268,7 +276,8 @@ int of_led_classdev_register(struct device *parent, >> struct device_node *np, >> mutex_unlock(&led_cdev->led_access); >> return PTR_ERR(led_cdev->dev); >> } >> - led_cdev->dev->of_node = np; >> + if (init_data && init_data->fwnode) >> + led_cdev->dev->of_node = to_of_node(init_data->fwnode); >> >> if (ret) >> dev_warn(parent, "Led %s renamed to %s due to name collision", >> @@ -313,7 +322,7 @@ int of_led_classdev_register(struct device *parent, >> struct device_node *np, >> >> return 0; >> } >> -EXPORT_SYMBOL_GPL(of_led_classdev_register); >> +EXPORT_SYMBOL_GPL(led_classdev_register_ext); >> >> /** >> * led_classdev_unregister - unregisters a object of led_properties class. >> @@ -358,14 +367,15 @@ static void devm_led_classdev_release(struct device >> *dev, void *res) >> } >> >> /** >> - * devm_of_led_classdev_register - resource managed led_classdev_register() >> + * devm_led_classdev_register_ext - resource managed >> led_classdev_register_ext() >> * >> * @parent: parent of LED device >> * @led_cdev: the led_classdev structure for this device. >> + * @init_data: LED class device initialization data >> */ >> -int devm_of_led_classdev_register(struct device *parent, >> - struct device_node *np, >> - struct led_classdev *led_cdev) >> +int devm_led_classdev_register_ext(struct device *parent, >> + struct led_classdev *led_cdev, >> + struct led_init_data *init_data) >> { >> struct led_classdev **dr; >> int rc; >> @@ -374,7 +384,7 @@ int devm_of_led_classdev_register(struct device *parent, >> if (!dr) >> return -ENOMEM; >> >> - rc = of_led_classdev_register(parent, np, led_cdev); >> + rc = led_classdev_register_ext(parent, led_cdev, init_data); >> if (rc) { >> devres_free(dr); >> return rc; >> @@ -385,7 +395,7 @@ int devm_of_led_classdev_register(struct device *parent, >> >> return 0; >> } >> -EXPORT_SYMBOL_GPL(devm_of_led_classdev_register); >> +EXPORT_SYMBOL_GPL(devm_led_classdev_register_ext); >> >> static int devm_led_classdev_match(struct device *dev, void *res, void >> *data) >> { >> diff --git a/drivers/leds/leds-cr0014114.c b/drivers/leds/leds-cr0014114.c >> index 0e42624..1c82152 100644 >> --- a/drivers/leds/leds-cr0014114.c >> +++ b/drivers/leds/leds-cr0014114.c >> @@ -207,8 +207,7 @@ static int cr0014114_probe_dt(struct cr0014114 *priv) >> led->ldev.max_brightness = CR_MAX_BRIGHTNESS; >> led->ldev.brightness_set_blocking = cr0014114_set_sync; >> >> - ret = devm_of_led_classdev_register(priv->dev, np, >> - &led->ldev); >> + ret = devm_led_classdev_register(priv->dev, &led->ldev); >> if (ret) { >> dev_err(priv->dev, >> "failed to register LED device %s, err %d", >> diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c >> index 32fa752..c87fbd3 100644 >> --- a/drivers/leds/leds-gpio.c >> +++ b/drivers/leds/leds-gpio.c >> @@ -112,7 +112,7 @@ static int create_gpio_led(const struct gpio_led >> *template, >> if (ret < 0) >> return ret; >> >> - return devm_of_led_classdev_register(parent, np, &led_dat->cdev); >> + return devm_led_classdev_register(parent, &led_dat->cdev); >> } >> >> struct gpio_leds_priv { >> diff --git a/include/linux/led-class-flash.h >> b/include/linux/led-class-flash.h >> index 700efaa..28a73d0 100644 >> --- a/include/linux/led-class-flash.h >> +++ b/include/linux/led-class-flash.h >> @@ -90,15 +90,20 @@ static inline struct led_classdev_flash *lcdev_to_flcdev( >> } >> >> /** >> - * led_classdev_flash_register - register a new object of led_classdev class >> - * with support for flash LEDs >> + * fwnode_led_classdev_flash_register - register a new object of >> led_classdev >> + * class with support for flash LEDs >> * @parent: the flash LED to register >> + * @fwnode: the flash LED fwnode handle >> * @fled_cdev: the led_classdev_flash structure for this device >> * >> * Returns: 0 on success or negative error value on failure >> */ >> -extern int led_classdev_flash_register(struct device *parent, >> - struct led_classdev_flash *fled_cdev); >> +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) >> >> /** >> * led_classdev_flash_unregister - unregisters an object of led_classdev >> class >> diff --git a/include/linux/leds.h b/include/linux/leds.h >> index 834683d..646c49a 100644 >> --- a/include/linux/leds.h >> +++ b/include/linux/leds.h >> @@ -20,6 +20,7 @@ >> #include <linux/spinlock.h> >> #include <linux/timer.h> >> #include <linux/workqueue.h> >> +#include <uapi/linux/uleds.h> >> >> struct device; >> /* >> @@ -33,6 +34,13 @@ enum led_brightness { >> LED_FULL = 255, >> }; >> >> +struct led_init_data { >> + /* Device fwnode handle */ >> + struct fwnode_handle *fwnode; >> + /* Requested LED class device name */ >> + char name[LED_MAX_NAME_SIZE]; >> +}; >> + >> struct led_classdev { >> const char *name; >> enum led_brightness brightness; >> @@ -73,6 +81,7 @@ struct led_classdev { >> */ >> int (*brightness_set_blocking)(struct led_classdev *led_cdev, >> enum led_brightness brightness); >> + >> /* Get LED brightness level */ >> enum led_brightness (*brightness_get)(struct led_classdev *led_cdev); >> >> @@ -123,16 +132,16 @@ struct led_classdev { >> struct mutex led_access; >> }; >> >> -extern int of_led_classdev_register(struct device *parent, >> - struct device_node *np, >> - struct led_classdev *led_cdev); >> -#define led_classdev_register(parent, led_cdev) >> \ >> - of_led_classdev_register(parent, NULL, led_cdev) >> -extern int devm_of_led_classdev_register(struct device *parent, >> - struct device_node *np, >> - struct led_classdev *led_cdev); >> -#define devm_led_classdev_register(parent, led_cdev) >> \ >> - devm_of_led_classdev_register(parent, NULL, led_cdev) >> +extern int led_classdev_register_ext(struct device *parent, >> + struct led_classdev *led_cdev, >> + struct led_init_data *init_data); >> +#define led_classdev_register(parent, led_cdev) \ >> + led_classdev_register_ext(parent, led_cdev, NULL) >> +extern int devm_led_classdev_register_ext(struct device *parent, >> + struct led_classdev *led_cdev, >> + struct led_init_data *init_data); >> +#define devm_led_classdev_register(parent, led_cdev) \ >> + devm_led_classdev_register_ext(parent, led_cdev, NULL) >> extern void led_classdev_unregister(struct led_classdev *led_cdev); >> extern void devm_led_classdev_unregister(struct device *parent, >> struct led_classdev *led_cdev); >> > > -- Best regards, Jacek Anaszewski
