Re: [PATCH v3 10/13] backlight: add overview and update existing doc
On Mon, Jun 01, 2020 at 08:52:04AM +0200, Sam Ravnborg wrote:
> Add overview chapter to backlight.c.
> Update existing kernel-doc to follow a more consistent
> style and drop kernel-doc for deprecated functions.
>
> v2:
> - Sevaral editorial corrections that makes reading
> much easier (Daniel)
> - Spelling fixes (Daniel)
> - updated intro chapter with a little more info
>
> Signed-off-by: Sam Ravnborg
Two very small nits... but one will affect formatted output so I had to
raise it. Feel free add my Reviewed-by: when recirculating!
Daniel.
> Cc: Lee Jones
> Cc: Daniel Thompson
> Cc: Jingoo Han
> ---
> drivers/video/backlight/backlight.c | 131 +++-
> 1 file changed, 90 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/video/backlight/backlight.c
> b/drivers/video/backlight/backlight.c
> index 17f04cff50ab..06bcddd76a7e 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -22,6 +22,46 @@
> #include
> #endif
>
> +/**
> + * DOC: overview
> + *
> + * The backlight core supports implementing backlight drivers.
> + *
> + * A backlight driver registers a driver using
> + * devm_backlight_device_register(). The properties of the backlight
> + * driver such as type and max_brightness must be specified.
> + * When the core detect changes in for example brightness or power state
> + * the update_status() operation is called. The backlight driver shall
> + * implement this operation and use it to adjust backlight.
> + *
> + * Several sysfs attributes are provided by the backlight core::
> + *
> + * - brightness R/W, set the requested brightness level
> + * - actual_brighness RO, the brightness level used by the HW
> + * - max_brightness RO, the maximum brightness level supported
> + *
> + * See Documentation/ABI/stable/sysfs-class-backlight for the full list.
> + *
> + * The backlight can be adjusted using the sysfs interface, and
> + * the backlight driver may also support adjusting backlight using
> + * a hot-key or some other platfrom or firmware specific way.
> + *
> + * The driver shall implement the get_brightness() operation if
I overlooked this before but this reads better with "must" rather
than "shall". It's not wrong to use shall but I think it is more
idiomatic to use "must".
> + * the HW do not support all the levels that can be specified in
> + * brightness, thus providing user-space access to the actual level
> + * via the actual_brightness attribute.
> + * When the backlight changes this is reported to user-space using
Missing newline between paragraphs...
> + * an uevent connected to the actual_brightness attribute.
> + * When brightness is set by platform specific means, for example
> + * a hot-key to adjust backlight, the driver must notify the backlight
> + * core that brightness has changed using backlight_force_update().
> + *
> + * The backlight driver core receives notifications from fbdev and
> + * if the event is FB_EVENT_BLANK and if the value of blank, from the
> + * FBIOBLANK ioclt, results in a change in the backlight state the
> + * update_status() operation is called.
> + */
> +
> static struct list_head backlight_dev_list;
> static struct mutex backlight_dev_list_mutex;
> static struct blocking_notifier_head backlight_notifier;
> @@ -40,9 +80,17 @@ static const char *const backlight_scale_types[] = {
>
> #if defined(CONFIG_FB) || (defined(CONFIG_FB_MODULE) && \
> defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE))
> -/* This callback gets called when something important happens inside a
> - * framebuffer driver. We're looking if that important event is blanking,
> - * and if it is and necessary, we're switching backlight power as well ...
> +/*
> + * fb_notifier_callback
> + *
> + * This callback gets called when something important happens inside a
> + * framebuffer driver. The backlight core only cares about FB_BLANK_UNBLANK
> + * which is reported to the driver using backlight_update_status()
> + * as a state change.
> + *
> + * There may be several fbdev's connected to the backlight device,
> + * in which case they are kept track of. A state change is only reported
> + * if there is a change in backlight for the specified fbdev.
> */
> static int fb_notifier_callback(struct notifier_block *self,
> unsigned long event, void *data)
> @@ -318,12 +366,15 @@ static struct attribute *bl_device_attrs[] = {
> ATTRIBUTE_GROUPS(bl_device);
>
> /**
> - * backlight_force_update - tell the backlight subsystem that hardware state
> - * has changed
> + * backlight_force_update - force an update due to a hardware change
> * @bd: the backlight device to update
> + * @reason: the method used for the backlight update
> *
> * Updates the internal state of the backlight in response to a hardware
> event,
> - * and generate a uevent to notify userspace
> + * and generates an uevent to notify userspace. A
[PATCH v3 10/13] backlight: add overview and update existing doc
Add overview chapter to backlight.c.
Update existing kernel-doc to follow a more consistent
style and drop kernel-doc for deprecated functions.
v2:
- Sevaral editorial corrections that makes reading
much easier (Daniel)
- Spelling fixes (Daniel)
- updated intro chapter with a little more info
Signed-off-by: Sam Ravnborg
Cc: Lee Jones
Cc: Daniel Thompson
Cc: Jingoo Han
---
drivers/video/backlight/backlight.c | 131 +++-
1 file changed, 90 insertions(+), 41 deletions(-)
diff --git a/drivers/video/backlight/backlight.c
b/drivers/video/backlight/backlight.c
index 17f04cff50ab..06bcddd76a7e 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -22,6 +22,46 @@
#include
#endif
+/**
+ * DOC: overview
+ *
+ * The backlight core supports implementing backlight drivers.
+ *
+ * A backlight driver registers a driver using
+ * devm_backlight_device_register(). The properties of the backlight
+ * driver such as type and max_brightness must be specified.
+ * When the core detect changes in for example brightness or power state
+ * the update_status() operation is called. The backlight driver shall
+ * implement this operation and use it to adjust backlight.
+ *
+ * Several sysfs attributes are provided by the backlight core::
+ *
+ * - brightness R/W, set the requested brightness level
+ * - actual_brighness RO, the brightness level used by the HW
+ * - max_brightness RO, the maximum brightness level supported
+ *
+ * See Documentation/ABI/stable/sysfs-class-backlight for the full list.
+ *
+ * The backlight can be adjusted using the sysfs interface, and
+ * the backlight driver may also support adjusting backlight using
+ * a hot-key or some other platfrom or firmware specific way.
+ *
+ * The driver shall implement the get_brightness() operation if
+ * the HW do not support all the levels that can be specified in
+ * brightness, thus providing user-space access to the actual level
+ * via the actual_brightness attribute.
+ * When the backlight changes this is reported to user-space using
+ * an uevent connected to the actual_brightness attribute.
+ * When brightness is set by platform specific means, for example
+ * a hot-key to adjust backlight, the driver must notify the backlight
+ * core that brightness has changed using backlight_force_update().
+ *
+ * The backlight driver core receives notifications from fbdev and
+ * if the event is FB_EVENT_BLANK and if the value of blank, from the
+ * FBIOBLANK ioclt, results in a change in the backlight state the
+ * update_status() operation is called.
+ */
+
static struct list_head backlight_dev_list;
static struct mutex backlight_dev_list_mutex;
static struct blocking_notifier_head backlight_notifier;
@@ -40,9 +80,17 @@ static const char *const backlight_scale_types[] = {
#if defined(CONFIG_FB) || (defined(CONFIG_FB_MODULE) && \
defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE))
-/* This callback gets called when something important happens inside a
- * framebuffer driver. We're looking if that important event is blanking,
- * and if it is and necessary, we're switching backlight power as well ...
+/*
+ * fb_notifier_callback
+ *
+ * This callback gets called when something important happens inside a
+ * framebuffer driver. The backlight core only cares about FB_BLANK_UNBLANK
+ * which is reported to the driver using backlight_update_status()
+ * as a state change.
+ *
+ * There may be several fbdev's connected to the backlight device,
+ * in which case they are kept track of. A state change is only reported
+ * if there is a change in backlight for the specified fbdev.
*/
static int fb_notifier_callback(struct notifier_block *self,
unsigned long event, void *data)
@@ -318,12 +366,15 @@ static struct attribute *bl_device_attrs[] = {
ATTRIBUTE_GROUPS(bl_device);
/**
- * backlight_force_update - tell the backlight subsystem that hardware state
- * has changed
+ * backlight_force_update - force an update due to a hardware change
* @bd: the backlight device to update
+ * @reason: the method used for the backlight update
*
* Updates the internal state of the backlight in response to a hardware event,
- * and generate a uevent to notify userspace
+ * and generates an uevent to notify userspace. A backlight driver shall call
+ * backlight_force_update() when the backlight is changed using, for example,
+ * a hot-key. The updated brightness is read using get_brightness() and the
+ * brightness value is reported using an uevent.
*/
void backlight_force_update(struct backlight_device *bd,
enum backlight_update_reason reason)
@@ -336,19 +387,7 @@ void backlight_force_update(struct backlight_device *bd,
}
EXPORT_SYMBOL(backlight_force_update);
-/**
- * backlight_device_register - create and register a new object of
- * backlight_device class.
- * @name: the name of the
