On Sun, Oct 04, 2026 at 07:41:20PM +0300, Svyatoslav Ryhel wrote:
> Conversion of the AAT2870 driver to use OF bindings requires a few complex
> changes that should be done simultaneously.
>
> The AAT2870 essentially provides two functions via child devices:
> backlight and regulators. Both functions are fairly self-sufficient and
> may not be populated on the final board. Consequently, the MFD
> registration API was replaced with of_platform_populate(), and each
> sub-device was given its own compatible string.
>
> Additionally, aat2870-core utilizes an enable GPIO. Obtaining this GPIO
> was converted to use modern gpiod/OF helpers, allowing the redundant
> aat2870_enable() and aat2870_disable() helpers to be removed.
>
> The aat2870-regulator driver was updated to register each of the four LDOs
> from a dedicated OF node.
>
> The aat2870-backlight driver was changed to populate all required
> properties from a dedicated Device Tree node. Its channel map was updated
> to use u8 instead of int. Furthermore, because the maximum current is now
> parsed as an absolute value rather than an enum entry, the calculation and
> application of the maximum current were updated accordingly.
>
> All of the changes above allow platform data to be removed entirely.
>
> Signed-off-by: Svyatoslav Ryhel <[email protected]>
> ---
> drivers/mfd/aat2870-core.c | 115 ++++++--------------------
> drivers/regulator/aat2870-regulator.c | 76 ++++++++++++-----
> drivers/video/backlight/aat2870_bl.c | 55 ++++++------
> include/linux/mfd/aat2870.h | 95 +--------------------
> 4 files changed, 111 insertions(+), 230 deletions(-)
>
> [snip]
>
> diff --git a/drivers/video/backlight/aat2870_bl.c
> b/drivers/video/backlight/aat2870_bl.c
> index 8b790df1e842b..933a8f728f66a 100644
> --- a/drivers/video/backlight/aat2870_bl.c
> +++ b/drivers/video/backlight/aat2870_bl.c
> @@ -15,11 +15,19 @@
> #include <linux/backlight.h>
> #include <linux/mfd/aat2870.h>
>
> +/* Backlight has 8 channels, each bit represents one channel */
> +#define AAT2870_BL_CH_ALL 0xff
> +
> +/* Backlight current magnitude (uA), 450uA current is eq to 0 */
> +#define AAT2870_CURRENT_MIN 450
> +#define AAT2870_CURRENT_MAX 27900
> +#define AAT2870_CURRENT_STEP 900
> +
> struct aat2870_bl_driver_data {
> struct platform_device *pdev;
> struct backlight_device *bd;
>
> - int channels;
> + u8 channels;
> int max_current;
> int brightness; /* current brightness */
> };
> @@ -30,7 +38,7 @@ static inline int aat2870_brightness(struct
> aat2870_bl_driver_data *aat2870_bl,
> struct backlight_device *bd = aat2870_bl->bd;
> int val;
>
> - val = brightness * (aat2870_bl->max_current - 1);
> + val = brightness * aat2870_bl->max_current;
> val /= bd->props.max_brightness;
What is the purpose of max_brightness?
Normally it is used to limit brightness but max_current is already doing
that. Is it just being used to reduce the number of steps in the
brightness scale (and if so, why is that useful)?
>
> return val;
> @@ -42,7 +50,7 @@ static inline int aat2870_bl_enable(struct
> aat2870_bl_driver_data *aat2870_bl)
> = dev_get_drvdata(aat2870_bl->pdev->dev.parent);
>
> return aat2870->write(aat2870, AAT2870_BL_CH_EN,
> - (u8)aat2870_bl->channels);
> + aat2870_bl->channels);
> }
>
> static inline int aat2870_bl_disable(struct aat2870_bl_driver_data
> *aat2870_bl)
> @@ -96,24 +104,12 @@ static const struct backlight_ops aat2870_bl_ops = {
>
> static int aat2870_bl_probe(struct platform_device *pdev)
> {
> - struct aat2870_bl_platform_data *pdata = dev_get_platdata(&pdev->dev);
> struct aat2870_bl_driver_data *aat2870_bl;
> struct backlight_device *bd;
> struct backlight_properties props;
> + u32 max_brightness = 0;
> int ret = 0;
>
> - if (!pdata) {
> - dev_err(&pdev->dev, "No platform data\n");
> - ret = -ENXIO;
> - goto out;
> - }
> -
> - if (pdev->id != AAT2870_ID_BL) {
> - dev_err(&pdev->dev, "Invalid device ID, %d\n", pdev->id);
> - ret = -EINVAL;
> - goto out;
> - }
> -
> aat2870_bl = devm_kzalloc(&pdev->dev,
> sizeof(struct aat2870_bl_driver_data),
> GFP_KERNEL);
> @@ -140,18 +136,18 @@ static int aat2870_bl_probe(struct platform_device
> *pdev)
>
> aat2870_bl->bd = bd;
>
> - if (pdata->channels > 0)
> - aat2870_bl->channels = pdata->channels;
> - else
> - aat2870_bl->channels = AAT2870_BL_CH_ALL;
> + aat2870_bl->channels = AAT2870_BL_CH_ALL;
> + device_property_read_u8(&pdev->dev, "skyworks,channels",
> &aat2870_bl->channels);
>
> - if (pdata->max_current > 0)
> - aat2870_bl->max_current = pdata->max_current;
> - else
> - aat2870_bl->max_current = AAT2870_CURRENT_27_9;
> + device_property_read_u32(&pdev->dev, "led-max-microamp",
> &aat2870_bl->max_current);
> + aat2870_bl->max_current = clamp(aat2870_bl->max_current,
> AAT2870_CURRENT_MIN,
> + AAT2870_CURRENT_MAX);
> + aat2870_bl->max_current /= AAT2870_CURRENT_STEP;
>
> - if (pdata->max_brightness > 0)
> - bd->props.max_brightness = pdata->max_brightness;
> + /* If max-brightness property is missing or set to zero, use chip's max
> value */
> + device_property_read_u32(&pdev->dev, "max-brightness", &max_brightness);
> + if (max_brightness)
> + bd->props.max_brightness = max_brightness;
> else
> bd->props.max_brightness = 255;
IIUC max_current can be zero (since AAT2870_CURRENT_MIN <
AAT2870_CURRENT_STEP). Since that means aat2870_brightness() will
always return 0 then there might need to be a special case
max_brightness for this case.
> @@ -181,9 +177,16 @@ static void aat2870_bl_remove(struct platform_device
> *pdev)
> backlight_update_status(bd);
> }
>
> +static const struct of_device_id aat2870_bl_match_table[] = {
> + { .compatible = "skyworks,aat2870-backlight" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, aat2870_bl_match_table);
> +
> static struct platform_driver aat2870_bl_driver = {
> .driver = {
> .name = "aat2870-backlight",
> + .of_match_table = aat2870_bl_match_table,
> },
> .probe = aat2870_bl_probe,
> .remove = aat2870_bl_remove,
Daniel.
