Re: [PATCH 2/2] macintosh/via-pmu-backlight: Include linux/of.h and uapi/linux/fb.h

[Thomas Zimmermann <tzimmermann@suse.de>]

Hi

Am 04.11.25 um 18:01 schrieb Nathan Chancellor:
> On Tue, Nov 04, 2025 at 01:43:26PM +0100, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 26.09.25 um 01:46 schrieb Nathan Chancellor:
>>> After the recent removal of the fb.h include from backlight.h, which
>>> transitively included of.h, there are several errors from
>>> via-pmu-backlight.c (errors from bl_curve not being properly defined
>>> omitted):
>>>
>>>     drivers/macintosh/via-pmu-backlight.c:22:20: error: 'FB_BACKLIGHT_LEVELS' undeclared here (not in a function)
>>>        22 | static u8 bl_curve[FB_BACKLIGHT_LEVELS];
>>>           |                    ^~~~~~~~~~~~~~~~~~~
>>>     drivers/macintosh/via-pmu-backlight.c: In function 'pmu_backlight_get_level_brightness':
>>>     drivers/macintosh/via-pmu-backlight.c:63:38: error: 'FB_BACKLIGHT_MAX' undeclared (first use in this function); did you mean 'BACKLIGHT_RAW'?
>>>        63 |         pmulevel = bl_curve[level] * FB_BACKLIGHT_MAX / MAX_PMU_LEVEL;
>>>           |                                      ^~~~~~~~~~~~~~~~
>>>           |                                      BACKLIGHT_RAW
>>>     drivers/macintosh/via-pmu-backlight.c: In function 'pmu_backlight_init':
>>>     drivers/macintosh/via-pmu-backlight.c:144:17: error: implicit declaration of function 'of_machine_is_compatible' [-Wimplicit-function-declaration]
>>>       144 |                 of_machine_is_compatible("AAPL,3400/2400") ||
>>>           |                 ^~~~~~~~~~~~~~~~~~~~~~~~
>>>
>>> FB_BACKLIGHT_{LEVELS,MAX} are available from the userspace API fb.h so
>>> just include that avoid dragging in the full fb.h header unnecessarily.
>>> Include linux/of.h for of_machine_is_compatible().
>>>
>>> Fixes: 9f218f9bb9d2 ("backlight: Do not include <linux/fb.h> in header file")
>>> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
>>> ---
>>>    drivers/macintosh/via-pmu-backlight.c | 2 ++
>>>    1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/macintosh/via-pmu-backlight.c b/drivers/macintosh/via-pmu-backlight.c
>>> index 26bd9ed5e664..f7b7853b3802 100644
>>> --- a/drivers/macintosh/via-pmu-backlight.c
>>> +++ b/drivers/macintosh/via-pmu-backlight.c
>>> @@ -11,7 +11,9 @@
>>>    #include <asm/ptrace.h>
>>>    #include <linux/adb.h>
>>>    #include <linux/backlight.h>
>>> +#include <linux/of.h>
>>>    #include <linux/pmu.h>
>>> +#include <uapi/linux/fb.h>
>> Should this not be <linux/fb.h> ?
> It could be but as I mention above, this file only needs the UAPI
> FB_BACKLIGHT_LEVELS and FB_BACKLIGHT_MAX constants so I did not really
> feel like it was worth it to drag in the whole fb.h header.

I mean that I thought that the internal header would be preferred over 
the UAPI header if both exist. But maybe I misremember.

Best regards
Thomas

>
>>>    #include <asm/backlight.h>
>>>    #define MAX_PMU_LEVEL 0xFF
>>>
>> -- 
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
>> GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
>>
>>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)