Re: [PATCH v6 04/20] leds: leds-lp55xx: generalize probe/remove functions

[Lee Jones <lee@kernel.org>]

On Sun, 16 Jun 2024, Christian Marangi wrote:

> Now that stop_all_engine is generalized, probe and remove function are
> the same across every lp55xx based LED driver and can be generalized.
> 
> To permit to use a common probe, make use of the OF match_data and i2c
> driver_data value to store the device_config struct specific for the
> LED.
> 
> Also drop the now unused exported symbol in lp55xx-common and make them
> static.
> 
> Update any lp55xx based LED driver to use the new generic probe/remove.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  drivers/leds/leds-lp5521.c        |  81 +-------------------
>  drivers/leds/leds-lp5523.c        |  85 ++-------------------
>  drivers/leds/leds-lp5562.c        |  80 +------------------
>  drivers/leds/leds-lp55xx-common.c | 123 +++++++++++++++++++++++-------
>  drivers/leds/leds-lp55xx-common.h |  21 +----
>  drivers/leds/leds-lp8501.c        |  81 +-------------------
>  6 files changed, 119 insertions(+), 352 deletions(-)

[...]

> diff --git a/drivers/leds/leds-lp55xx-common.c b/drivers/leds/leds-lp55xx-common.c
> index 2cbc5b302fd4..7e623e4e565c 100644
> --- a/drivers/leds/leds-lp55xx-common.c
> +++ b/drivers/leds/leds-lp55xx-common.c
> @@ -32,6 +32,8 @@
>  /* External clock rate */
>  #define LP55XX_CLK_32K			32768
>  
> +static void lp55xx_deinit_device(struct lp55xx_chip *chip);

No forward declarations please.  Move the function.
> +
>  static struct lp55xx_led *cdev_to_lp55xx_led(struct led_classdev *cdev)
>  {
>  	return container_of(cdev, struct lp55xx_led, cdev);
> @@ -49,7 +51,7 @@ static struct lp55xx_led *mcled_cdev_to_led(struct led_classdev_mc *mc_cdev)
>  
>  static void lp55xx_wait_opmode_done(struct lp55xx_chip *chip)
>  {
> -	struct lp55xx_device_config *cfg = chip->cfg;
> +	const struct lp55xx_device_config *cfg = chip->cfg;
>  	int __always_unused ret;
>  	u8 val;
>  
> @@ -69,7 +71,7 @@ static void lp55xx_wait_opmode_done(struct lp55xx_chip *chip)
>  
>  void lp55xx_stop_all_engine(struct lp55xx_chip *chip)
>  {
> -	struct lp55xx_device_config *cfg = chip->cfg;
> +	const struct lp55xx_device_config *cfg = chip->cfg;
>  
>  	lp55xx_write(chip, cfg->reg_op_mode.addr, LP55xx_MODE_DISABLE_ALL_ENG);
>  	lp55xx_wait_opmode_done(chip);
> @@ -78,7 +80,7 @@ EXPORT_SYMBOL_GPL(lp55xx_stop_all_engine);
>  
>  static void lp55xx_reset_device(struct lp55xx_chip *chip)
>  {
> -	struct lp55xx_device_config *cfg = chip->cfg;
> +	const struct lp55xx_device_config *cfg = chip->cfg;
>  	u8 addr = cfg->reset.addr;
>  	u8 val  = cfg->reset.val;
>  
> @@ -88,7 +90,7 @@ static void lp55xx_reset_device(struct lp55xx_chip *chip)
>  
>  static int lp55xx_detect_device(struct lp55xx_chip *chip)
>  {
> -	struct lp55xx_device_config *cfg = chip->cfg;
> +	const struct lp55xx_device_config *cfg = chip->cfg;
>  	u8 addr = cfg->enable.addr;
>  	u8 val  = cfg->enable.val;
>  	int ret;
> @@ -111,7 +113,7 @@ static int lp55xx_detect_device(struct lp55xx_chip *chip)
>  
>  static int lp55xx_post_init_device(struct lp55xx_chip *chip)
>  {
> -	struct lp55xx_device_config *cfg = chip->cfg;
> +	const struct lp55xx_device_config *cfg = chip->cfg;
>  
>  	if (!cfg->post_init_device)
>  		return 0;
> @@ -176,7 +178,7 @@ static int lp55xx_set_mc_brightness(struct led_classdev *cdev,
>  {
>  	struct led_classdev_mc *mc_dev = lcdev_to_mccdev(cdev);
>  	struct lp55xx_led *led = mcled_cdev_to_led(mc_dev);
> -	struct lp55xx_device_config *cfg = led->chip->cfg;
> +	const struct lp55xx_device_config *cfg = led->chip->cfg;
>  
>  	led_mc_calc_color_components(&led->mc_cdev, brightness);
>  	return cfg->multicolor_brightness_fn(led);
> @@ -187,7 +189,7 @@ static int lp55xx_set_brightness(struct led_classdev *cdev,
>  			     enum led_brightness brightness)
>  {
>  	struct lp55xx_led *led = cdev_to_lp55xx_led(cdev);
> -	struct lp55xx_device_config *cfg = led->chip->cfg;
> +	const struct lp55xx_device_config *cfg = led->chip->cfg;
>  
>  	led->brightness = (u8)brightness;
>  	return cfg->brightness_fn(led);
> @@ -197,7 +199,7 @@ static int lp55xx_init_led(struct lp55xx_led *led,
>  			struct lp55xx_chip *chip, int chan)
>  {
>  	struct lp55xx_platform_data *pdata = chip->pdata;
> -	struct lp55xx_device_config *cfg = chip->cfg;
> +	const struct lp55xx_device_config *cfg = chip->cfg;
>  	struct device *dev = &chip->cl->dev;
>  	int max_channel = cfg->max_channel;
>  	struct mc_subled *mc_led_info;
> @@ -459,10 +461,10 @@ bool lp55xx_is_extclk_used(struct lp55xx_chip *chip)
>  }
>  EXPORT_SYMBOL_GPL(lp55xx_is_extclk_used);
>  
> -int lp55xx_init_device(struct lp55xx_chip *chip)
> +static int lp55xx_init_device(struct lp55xx_chip *chip)
>  {
>  	struct lp55xx_platform_data *pdata;
> -	struct lp55xx_device_config *cfg;
> +	const struct lp55xx_device_config *cfg;
>  	struct device *dev = &chip->cl->dev;
>  	int ret = 0;
>  
> @@ -512,9 +514,8 @@ int lp55xx_init_device(struct lp55xx_chip *chip)
>  err:
>  	return ret;
>  }
> -EXPORT_SYMBOL_GPL(lp55xx_init_device);
>  
> -void lp55xx_deinit_device(struct lp55xx_chip *chip)
> +static void lp55xx_deinit_device(struct lp55xx_chip *chip)
>  {
>  	struct lp55xx_platform_data *pdata = chip->pdata;
>  
> @@ -524,12 +525,11 @@ void lp55xx_deinit_device(struct lp55xx_chip *chip)
>  	if (pdata->enable_gpiod)
>  		gpiod_set_value(pdata->enable_gpiod, 0);
>  }
> -EXPORT_SYMBOL_GPL(lp55xx_deinit_device);

-- 
Lee Jones [李琼斯]