On Mon, Oct 05, 2026 at 08:18:31PM +0300, Svyatoslav Ryhel wrote: > пн, 5 жовт. 2026 р. о 19:51 Daniel Thompson <[email protected]> пише: > > > > 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)? > > > > This is the original driver behavior; I did not modify it.
I can see that but the parameters for these (rather dubious) previous behaviours are now being copied into the devicetree bindings where they become stable ABI. > The AAT2870 backlight intensity is regulated by the Main Backlight > Current Magnitude register. It has 31 steps from 450uA (0) to 27.9mA > (31), so even setting max_current to 0 will not disable the backlight. > The max current value may be limited for some configurations, which is > why this property exists. > > The max_brightness property determines the maximum number of steps the > backlight can have and is mapped to the available current range > provided by the hardware. The backlight is controlled in steps, not in > uA. This behaviour does not match the devicetree bindings documentation for max-brightness: Normally the maximum brightness is determined by the hardware and this property is not required. This property is used to put a software limit on the brightness apart from what the driver says, as it could happen that a LED can be made so bright that it gets damaged or causes damage due to restrictions in a specific system, such as mounting conditions. Hence the question, why is the current behaviour useful? If I understand your summary correctly the hardware has 32 steps including 0) and the effect of max_current is to ban all brightness values greater than max_current / AAT2870_CURRENT_STEP. If that is the case then max_brightness if fundamentally broken because it's default value offers more steps than exists in the hardware. bd->props.max_brightness should be set to max_current / AAT2870_CURRENT_STEP and the whole of aat2870_brightness() should be removed (use backlight_get_brightness() instead). IMHO the patchset will be cleaner if the weird max_brightness logic is removed in a separate before doing the DT changes. Daniel.
