Re: [PATCH v2 02/24] backlight: Add DECLARE_* macro for device registration

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Aug 23, 2020 at 12:45:10PM +0200, Sam Ravnborg wrote:
> Device registration almost always uses a struct backlight_properties
> variable to pass config info. Make it simpler and less error prone
> by the introduction of a number of macros.
> 
> There is one macro for each type of backlight {firmware, platform, raw}.
> All members in struct backlight_properties are initialized.
> 
> Signed-off-by: Sam Ravnborg <sam@xxxxxxxxxxxx>
> Cc: Lee Jones <lee.jones@xxxxxxxxxx>
> Cc: Daniel Thompson <daniel.thompson@xxxxxxxxxx>
> Cc: Jingoo Han <jingoohan1@xxxxxxxxx>
> ---
>  include/linux/backlight.h | 63 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 63 insertions(+)
> 
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index 190963ffb7fc..93a47a6cf681 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -272,6 +272,69 @@ struct backlight_properties {
>  	enum backlight_scale scale;
>  };
>  
> +/**
> + * BACKLIGHT_PROPS - init backlight_properties with default values
> + *
> + * This macro is used to initialize backlight_properties with default
> + * values. It is intended to be used when registering a backlight device
> + * and the properties needs to be adjusted at run-time, for example
> + * when the max_brightness is configurable.
> + *
> + * .. code-block:: c

sphinx markup in kernel-doc comments is pretty rare at the moment (and
presumably it does odd things to direct man page generation). Has it
been discussed and approved of by doc maintainers or is it just creeping
organically?


> + *
> + *	struct backlight_properties props = {
> + *		BACKLIGHT_PROPS(0, 255, BACKLIGHT_RAW)
> + *	};
> + *	...
> + *	props.max_brightness = new_max;
> + *	err = devm_backlight_device_register(,,,, props);
> + *
> + */
> +#define BACKLIGHT_PROPS(_brightness, _max_brightness, _type)	\
> +	.brightness = _brightness,				\
> +	.max_brightness = _max_brightness,			\
> +	.power = FB_BLANK_POWERDOWN,				\
> +	.type = _type,						\
> +	.fb_blank = 0,						\
> +	.state = 0,						\
> +	.scale = BACKLIGHT_SCALE_UNKNOWN,

Hmnnn... not sure I like seeing this buried.

BACKLIGHT_SCALE_UNKNOWN is not a sane default... it is pure legacy
so it would be good to force drivers to declare this explicitly
(since it would require new drivers to think about the correct value).

It then also becomes a good git grep target to help identify drivers
whose scale hasn't been reviewed and recorded yet...


Daniel.


> +
> +/**
> + * DECLARE_BACKLIGHT_INIT_RAW - backlight_properties to init a raw
> + *                              backlight device
> + *
> + * This macro is used to initialize backlight_properties that is used when
> + * registering a raw backlight device.
> + */
> +#define DECLARE_BACKLIGHT_INIT_RAW(name, _brightness, _max_brightness)		\
> +	const struct backlight_properties name = {				\
> +		BACKLIGHT_PROPS(_brightness, _max_brightness, BACKLIGHT_RAW)	\
> +	}
> +
> +/**
> + * DECLARE_BACKLIGHT_INIT_PLATFORM - backlight_properties to init a platform
> + *                                   backlight device
> + *
> + * This macro is used to initialize backlight_properties that is used when
> + * registering a platform backlight device.
> + */
> +#define DECLARE_BACKLIGHT_INIT_PLATFORM(name, _brightness, _max_brightness)		\
> +	const struct backlight_properties name = {					\
> +		BACKLIGHT_PROPS(_brightness, _max_brightness, BACKLIGHT_PLATFORM)	\
> +	}
> +
> +/**
> + * DECLARE_BACKLIGHT_INIT_FIRMWARE - backlight_properties to init a firmware
> + *                                   backlight device
> + *
> + * This macro is used to initialize backlight_properties that is used when
> + * registering a firmware backlight device.
> + */
> +#define DECLARE_BACKLIGHT_INIT_FIRMWARE(name, _brightness, _max_brightness)		\
> +	const struct backlight_properties name = {					\
> +		BACKLIGHT_PROPS(_brightness, _max_brightness, BACKLIGHT_FIRMWARE)	\
> +	}
> +
>  /**
>   * struct backlight_device - backlight device data
>   *
> -- 
> 2.25.1
> 



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux