Re: [PATCH 2/8] platform/x86: int3472: Init LED registers using platform data

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 3/22/23 17:09, Daniel Scally wrote:
> Check platform data to discover the appropriate settings for the PMIC's
> WLED registers and set them during probe.
> 
> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>

As mentioned already the subsys-prefix for this patch is wrong
and maybe also in the subject do s/LED/WLED/, so we get:

[PATCH 2/8] leds: tps68470: Init WLED registers using platform data

although since the pdata is shared for all tps68470 LEDs
I guess without the W it makes sense too.

With the Subject subsys prefix fixed this is:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans



> ---
>  drivers/leds/leds-tps68470.c | 51 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/drivers/leds/leds-tps68470.c b/drivers/leds/leds-tps68470.c
> index 35aeb5db89c8..d2060fe4259d 100644
> --- a/drivers/leds/leds-tps68470.c
> +++ b/drivers/leds/leds-tps68470.c
> @@ -11,6 +11,7 @@
>  #include <linux/leds.h>
>  #include <linux/mfd/tps68470.h>
>  #include <linux/module.h>
> +#include <linux/platform_data/tps68470.h>
>  #include <linux/platform_device.h>
>  #include <linux/property.h>
>  #include <linux/regmap.h>
> @@ -113,6 +114,52 @@ static int tps68470_ledb_current_init(struct platform_device *pdev,
>  	return ret;
>  }
>  
> +static int tps68470_leds_init(struct tps68470_device *tps68470)
> +{
> +	struct tps68470_led_platform_data *pdata = tps68470->dev->platform_data;
> +	int ret;
> +
> +	if (!pdata)
> +		return 0;
> +
> +	ret = regmap_write(tps68470->regmap, TPS68470_REG_ILEDCTL, pdata->iledctl_ctrlb);
> +	if (ret)
> +		return dev_err_probe(tps68470->dev, ret, "failed to set ILED CTRLB\n");
> +
> +	ret = regmap_write(tps68470->regmap, TPS68470_REG_WLEDMAXF,
> +			   pdata->wledmaxf & TPS68470_WLEDMAXF_MAX_CUR_MASK);
> +	if (ret)
> +		return dev_err_probe(tps68470->dev, ret, "failed to set WLEDMAXF\n");
> +
> +	ret = regmap_write(tps68470->regmap, TPS68470_REG_WLEDTO, pdata->wledto);
> +	if (ret)
> +		return dev_err_probe(tps68470->dev, ret, "failed to set WLEDTO\n");
> +
> +	ret = regmap_write(tps68470->regmap, TPS68470_REG_WLEDC1,
> +			   pdata->wledc1 & TPS68470_WLEDC_ILED_MASK);
> +	if (ret)
> +		return dev_err_probe(tps68470->dev, ret, "failed to set WLEDC1\n");
> +
> +	ret = regmap_write(tps68470->regmap, TPS68470_REG_WLEDC2,
> +			   pdata->wledc2 & TPS68470_WLEDC_ILED_MASK);
> +	if (ret)
> +		return dev_err_probe(tps68470->dev, ret, "failed to set WLEDC2\n");
> +
> +	ret = regmap_update_bits(tps68470->regmap, TPS68470_REG_WLEDCTL,
> +				 TPS68470_WLED_DISLED1,
> +				 pdata->wledctl_disled1 ? TPS68470_WLED_DISLED1 : 0);
> +	if (ret)
> +		return dev_err_probe(tps68470->dev, ret, "failed to set DISLED1\n");
> +
> +	ret = regmap_update_bits(tps68470->regmap, TPS68470_REG_WLEDCTL,
> +				 TPS68470_WLED_DISLED2,
> +				 pdata->wledctl_disled2 ? TPS68470_WLED_DISLED2 : 0);
> +	if (ret)
> +		dev_err_probe(tps68470->dev, ret, "failed to set DISLED2\n");
> +
> +	return 0;
> +}
> +
>  static int tps68470_leds_probe(struct platform_device *pdev)
>  {
>  	int i = 0;
> @@ -160,6 +207,10 @@ static int tps68470_leds_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	ret = tps68470_leds_init(tps68470);
> +	if (ret)
> +		goto err_exit;
> +
>  err_exit:
>  	if (ret) {
>  		for (i = 0; i < TPS68470_NUM_LEDS; i++) {