Jacek Anaszewski <jacek.anaszewski@xxxxxxxxx> 於 2020年11月24日 週二 上午5:07寫道: > > On 11/23/20 4:20 AM, Gene Chen wrote: > > Jacek Anaszewski <jacek.anaszewski@xxxxxxxxx> 於 2020年11月20日 週五 上午6:29寫道: > >> > >> Hi Gene, > >> > >> On 11/18/20 11:47 AM, Gene Chen wrote: > >>> From: Gene Chen <gene_chen@xxxxxxxxxxx> > >>> > >>> Add flash registration with undefined CONFIG_LEDS_CLASS_FLASH > >>> > >>> Signed-off-by: Gene Chen <gene_chen@xxxxxxxxxxx> > >>> --- > >>> include/linux/led-class-flash.h | 36 ++++++++++++++++++++++++++++++++++++ > >>> 1 file changed, 36 insertions(+) > >>> > >>> diff --git a/include/linux/led-class-flash.h b/include/linux/led-class-flash.h > >>> index 21a3358..4f56c28 100644 > >>> --- a/include/linux/led-class-flash.h > >>> +++ b/include/linux/led-class-flash.h > >>> @@ -85,6 +85,7 @@ static inline struct led_classdev_flash *lcdev_to_flcdev( > >>> return container_of(lcdev, struct led_classdev_flash, led_cdev); > >>> } > >>> > >>> +#if IS_ENABLED(CONFIG_LEDS_CLASS_FLASH) > >>> /** > >>> * led_classdev_flash_register_ext - register a new object of LED class with > >>> * init data and with support for flash LEDs > >>> @@ -127,6 +128,41 @@ static inline int devm_led_classdev_flash_register(struct device *parent, > >>> void devm_led_classdev_flash_unregister(struct device *parent, > >>> struct led_classdev_flash *fled_cdev); > >>> > >>> +#else > >>> + > >>> +static inline int led_classdev_flash_register_ext(struct device *parent, > >>> + struct led_classdev_flash *fled_cdev, > >>> + struct led_init_data *init_data) > >>> +{ > >>> + return -EINVAL; > >> > >> s/-EINVAL/0/ > >> > >> The goal here is to assure that client will not fail when using no-op. > >> > >>> +} > >>> + > >>> +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); > >>> +} > >> > >> This function should be placed after #ifdef block because its > >> shape is the same for both cases. > >> > >>> +static inline void led_classdev_flash_unregister(struct led_classdev_flash *fled_cdev) {}; > >>> +static inline int devm_led_classdev_flash_register_ext(struct device *parent, > >>> + struct led_classdev_flash *fled_cdev, > >>> + struct led_init_data *init_data) > >>> +{ > >>> + return -EINVAL; > >> > >> /-EINVAL/0/ > >> > >> Please do the same fix in all no-ops in the led-class-multicolor.h, > >> as we've discussed. > >> > > > > I think return -EINVAL is correct, because I should register flash > > light device if I define FLED in DTS node. > > I don't quite follow your logic here. > > No-op function's purpose is to simplify the code on the caller's side. > Therefore it should report success. > > Please return 0 from it. > Just like those functions in led-class-multicolor.h, caller may use return value to check whether FLED is registered successfully or not. For this case, is returning 0 a little bit misleading? > -- > Best regards, > Jacek Anaszewski