Received-SPF: pass (google.com: domain of linux-kernel+bounces-21020-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
From: Rui Miguel Silva <rmfrfs@gmail.com>
To: Arnd Bergmann <arnd@kernel.org>, Pavel Machek <pavel@ucw.cz>, Lee Jones
 <lee@kernel.org>
Cc: Andrew Lunn <andrew@lunn.ch>, Arnd Bergmann <arnd@arndb.de>, Heiner
 Kallweit <hkallweit1@gmail.com>, Hans de Goede <hdegoede@redhat.com>,
 Jean-Jacques Hiblot <jjhiblot@traphandler.com>,
 linux-leds@vger.kernel.org, linux-kernel@vger.kernel.org, Johan Hovold
 <johan@kernel.org>, Alex Elder <elder@kernel.org>, Greg Kroah-Hartman
 <gregkh@linuxfoundation.org>, Andy Shevchenko
 <andriy.shevchenko@linux.intel.com>, Andreas Kemnade
 <andreas@kemnade.info>, greybus-dev@lists.linaro.org,
 linux-staging@lists.linux.dev
Subject: Re: [PATCH 2/2] leds: make flash and multicolor dependencies
 unconditional
In-Reply-To: <20240109090715.982332-2-arnd@kernel.org>
References: <20240109090715.982332-1-arnd@kernel.org>
 <20240109090715.982332-2-arnd@kernel.org>
Date: Tue, 09 Jan 2024 15:01:18 +0000
Message-ID: <m3mstey2ip.fsf@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain

Hey Arnd,
Many thanks for the patch.

Arnd Bergmann <arnd@kernel.org> writes:

> From: Arnd Bergmann <arnd@arndb.de>
>
> Along the same lines as making devm_led_classdev_register() declared
> extern unconditional, do the same thing for the two sub-classes
> that have similar stubs.
>
> The users of these interfaces go to great lengths to allow building
> with both the generic leds API and the extended version, but realistically
> there is not much use in this, so just simplify it to always rely
> on it and remove the confusing fallback logic.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/leds/Kconfig                 |  4 ++--
>  drivers/leds/flash/Kconfig           |  4 ++--
>  drivers/staging/greybus/Kconfig      |  2 +-
>  drivers/staging/greybus/light.c      | 21 --------------------
>  include/linux/led-class-flash.h      | 24 -----------------------
>  include/linux/led-class-multicolor.h | 29 ----------------------------
>  6 files changed, 5 insertions(+), 79 deletions(-)
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index d721b254e1e4..9613a45a35bd 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -395,7 +395,7 @@ config LEDS_LP3952
>  config LEDS_LP50XX
>  	tristate "LED Support for TI LP5036/30/24/18/12/09 LED driver chip"
>  	depends on LEDS_CLASS && REGMAP_I2C
> -	depends on LEDS_CLASS_MULTICOLOR || !LEDS_CLASS_MULTICOLOR
> +	depends on LEDS_CLASS_MULTICOLOR
>  	help
>  	  If you say yes here you get support for the Texas Instruments
>  	  LP5036, LP5030, LP5024, LP5018, LP5012 and LP5009 LED driver.
> @@ -406,7 +406,7 @@ config LEDS_LP50XX
>  config LEDS_LP55XX_COMMON
>  	tristate "Common Driver for TI/National LP5521/5523/55231/5562/8501"
>  	depends on LEDS_CLASS
> -	depends on LEDS_CLASS_MULTICOLOR || !LEDS_CLASS_MULTICOLOR
> +	depends on LEDS_CLASS_MULTICOLOR
>  	depends on OF
>  	depends on I2C
>  	select FW_LOADER
> diff --git a/drivers/leds/flash/Kconfig b/drivers/leds/flash/Kconfig
> index 4e08dbc05709..b95f90cd5749 100644
> --- a/drivers/leds/flash/Kconfig
> +++ b/drivers/leds/flash/Kconfig
> @@ -51,8 +51,8 @@ config LEDS_MAX77693
>  config LEDS_MT6360
>  	tristate "LED Support for Mediatek MT6360 PMIC"
>  	depends on LEDS_CLASS && OF
> -	depends on LEDS_CLASS_FLASH || !LEDS_CLASS_FLASH
> -	depends on LEDS_CLASS_MULTICOLOR || !LEDS_CLASS_MULTICOLOR
> +	depends on LEDS_CLASS_FLASH
> +	depends on LEDS_CLASS_MULTICOLOR
>  	depends on V4L2_FLASH_LED_CLASS || !V4L2_FLASH_LED_CLASS
>  	depends on MFD_MT6360
>  	help
> diff --git a/drivers/staging/greybus/Kconfig b/drivers/staging/greybus/Kconfig
> index 927cfa4bc989..1e745a8d439c 100644
> --- a/drivers/staging/greybus/Kconfig
> +++ b/drivers/staging/greybus/Kconfig
> @@ -64,7 +64,7 @@ config GREYBUS_HID
>  
>  config GREYBUS_LIGHT
>  	tristate "Greybus LED Class driver"
> -	depends on LEDS_CLASS
> +	depends on LEDS_CLASS_FLASH

Agree with the change for the greybus driver, one note, since you are
cleaning up this, maybe also remove the other #if down when setting
the flash specific operations, right?

#if IS_REACHABLE(CONFIG_LEDS_CLASS_FLASH)
/* Flash specific operations */
static int gb_lights_flash_intensity_set(struct led_classdev_flash *fcdev,
					 u32 brightness)
{
	struct gb_channel *channel = container_of(fcdev, struct gb_channel,
						  fled);
	int ret;
.
.
.
.

#else
static int gb_lights_channel_flash_config(struct gb_channel *channel)
{
	struct gb_connection *connection = get_conn_from_channel(channel);

	dev_err(&connection->bundle->dev, "no support for flash devices\n");
	return 0;
}
.
.
.


Thanks again.

Cheers,
    Rui

>  	help
>  	  Select this option if you have a device that follows the
>  	  Greybus LED Class specification.
> diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c
> index 87d36948c610..d62f97249aca 100644
> --- a/drivers/staging/greybus/light.c
> +++ b/drivers/staging/greybus/light.c
> @@ -29,13 +29,9 @@ struct gb_channel {
>  	struct attribute_group		*attr_group;
>  	const struct attribute_group	**attr_groups;
>  	struct led_classdev		*led;
> -#if IS_REACHABLE(CONFIG_LEDS_CLASS_FLASH)
>  	struct led_classdev_flash	fled;
>  	struct led_flash_setting	intensity_uA;
>  	struct led_flash_setting	timeout_us;
> -#else
> -	struct led_classdev		cled;
> -#endif
>  	struct gb_light			*light;
>  	bool				is_registered;
>  	bool				releasing;
> @@ -84,7 +80,6 @@ static bool is_channel_flash(struct gb_channel *channel)
>  				   | GB_CHANNEL_MODE_INDICATOR));
>  }
>  
> -#if IS_REACHABLE(CONFIG_LEDS_CLASS_FLASH)
>  static struct gb_channel *get_channel_from_cdev(struct led_classdev *cdev)
>  {
>  	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(cdev);
> @@ -153,22 +148,6 @@ static int __gb_lights_flash_brightness_set(struct gb_channel *channel)
>  
>  	return __gb_lights_flash_intensity_set(channel, intensity);
>  }
> -#else
> -static struct gb_channel *get_channel_from_cdev(struct led_classdev *cdev)
> -{
> -	return container_of(cdev, struct gb_channel, cled);
> -}
> -
> -static struct led_classdev *get_channel_cdev(struct gb_channel *channel)
> -{
> -	return &channel->cled;
> -}
> -
> -static int __gb_lights_flash_brightness_set(struct gb_channel *channel)
> -{
> -	return 0;
> -}
> -#endif
>  
>  static int gb_lights_color_set(struct gb_channel *channel, u32 color);
>  static int gb_lights_fade_set(struct gb_channel *channel);
> diff --git a/include/linux/led-class-flash.h b/include/linux/led-class-flash.h
> index 612b4cab3819..36df927ec4b7 100644
> --- a/include/linux/led-class-flash.h
> +++ b/include/linux/led-class-flash.h
> @@ -85,7 +85,6 @@ 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
> @@ -116,29 +115,6 @@ int devm_led_classdev_flash_register_ext(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 0;
> -}
> -
> -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 0;
> -}
> -
> -static inline void devm_led_classdev_flash_unregister(struct device *parent,
> -					struct led_classdev_flash *fled_cdev)
> -{};
> -
> -#endif  /* IS_ENABLED(CONFIG_LEDS_CLASS_FLASH) */
> -
>  static inline int led_classdev_flash_register(struct device *parent,
>  					   struct led_classdev_flash *fled_cdev)
>  {
> diff --git a/include/linux/led-class-multicolor.h b/include/linux/led-class-multicolor.h
> index 210d57bcd767..db9f34c6736e 100644
> --- a/include/linux/led-class-multicolor.h
> +++ b/include/linux/led-class-multicolor.h
> @@ -30,7 +30,6 @@ static inline struct led_classdev_mc *lcdev_to_mccdev(
>  	return container_of(led_cdev, struct led_classdev_mc, led_cdev);
>  }
>  
> -#if IS_ENABLED(CONFIG_LEDS_CLASS_MULTICOLOR)
>  /**
>   * led_classdev_multicolor_register_ext - register a new object of led_classdev
>   *				      class with support for multicolor LEDs
> @@ -64,34 +63,6 @@ int devm_led_classdev_multicolor_register_ext(struct device *parent,
>  
>  void devm_led_classdev_multicolor_unregister(struct device *parent,
>  					    struct led_classdev_mc *mcled_cdev);
> -#else
> -
> -static inline int led_classdev_multicolor_register_ext(struct device *parent,
> -					    struct led_classdev_mc *mcled_cdev,
> -					    struct led_init_data *init_data)
> -{
> -	return 0;
> -}
> -
> -static inline void led_classdev_multicolor_unregister(struct led_classdev_mc *mcled_cdev) {};
> -static inline int led_mc_calc_color_components(struct led_classdev_mc *mcled_cdev,
> -					       enum led_brightness brightness)
> -{
> -	return 0;
> -}
> -
> -static inline int devm_led_classdev_multicolor_register_ext(struct device *parent,
> -					  struct led_classdev_mc *mcled_cdev,
> -					  struct led_init_data *init_data)
> -{
> -	return 0;
> -}
> -
> -static inline void devm_led_classdev_multicolor_unregister(struct device *parent,
> -					    struct led_classdev_mc *mcled_cdev)
> -{};
> -
> -#endif  /* IS_ENABLED(CONFIG_LEDS_CLASS_MULTICOLOR) */
>  
>  static inline int led_classdev_multicolor_register(struct device *parent,
>  					    struct led_classdev_mc *mcled_cdev)
> -- 
> 2.39.2