[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240620153846.GN3029315@google.com>
Date: Thu, 20 Jun 2024 16:38:46 +0100
From: Lee Jones <lee@...nel.org>
To: Christian Marangi <ansuelsmth@...il.com>
Cc: Pavel Machek <pavel@....cz>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>,
Jacek Anaszewski <jacek.anaszewski@...il.com>,
linux-leds@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 04/20] leds: leds-lp55xx: generalize probe/remove
functions
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@...il.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 [李琼斯]
Powered by blists - more mailing lists