Re: [PATCH v4 05/20] backlight: improve backlight_device documentation
Hi Jingoo On Sat, Jul 18, 2020 at 05:18:39AM +, Jingoo Han wrote: > On 7/3/20, 2:46 PM, Sam Ravnborg wrote: > > > > Improve the documentation for backlight_device and > > adapt it to kernel-doc style. > > > > The updated documentation is more strict on how locking is used. > > With the update neither update_lock nor ops_lock may be used > > outside the backlight core. > > This restriction was introduced to keep the locking simple > > by keeping it in the core. > > It was verified that this documents the current state by renaming > > update_lock => bl_update_lock and ops_lock => bl_ops_lock. > > The rename did not reveal any uses outside the backlight core. > > The rename is NOT part of this patch. > > > > v3: > > - Update changelog to explain locking details (Daniel) > > > > v2: > > - Add short intro to all fields (Daniel) > > - Updated description of update_lock (Daniel) > > > > Signed-off-by: Sam Ravnborg > > Reviewed-by: Emil Velikov > > Cc: Lee Jones > > Cc: Daniel Thompson > > Cc: Jingoo Han > > It looks good! > Reviewed-by: Jingoo Han Thanks! > > For the rebase, if you don't know which branch of maintainer's git can be > used, > linux-next tree [1] is useful. The linux-next git collects all next branches > from > other maintainers' git every day. > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/ I had used drm-misc-next because the original focus was to clean up drivers in gpu/drm/ - and then I just continued to use this wrong tree. linux-next is indeed a good place to catch the latest and greatest - but as I now have the URL for the backlight tree (thanks to Lee) I will use it here. Will try to find time this weekend so we can land these. Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 05/20] backlight: improve backlight_device documentation
On 7/3/20, 2:46 PM, Sam Ravnborg wrote: > > Improve the documentation for backlight_device and > adapt it to kernel-doc style. > > The updated documentation is more strict on how locking is used. > With the update neither update_lock nor ops_lock may be used > outside the backlight core. > This restriction was introduced to keep the locking simple > by keeping it in the core. > It was verified that this documents the current state by renaming > update_lock => bl_update_lock and ops_lock => bl_ops_lock. > The rename did not reveal any uses outside the backlight core. > The rename is NOT part of this patch. > > v3: > - Update changelog to explain locking details (Daniel) > > v2: > - Add short intro to all fields (Daniel) > - Updated description of update_lock (Daniel) > > Signed-off-by: Sam Ravnborg > Reviewed-by: Emil Velikov > Cc: Lee Jones > Cc: Daniel Thompson > Cc: Jingoo Han It looks good! Reviewed-by: Jingoo Han For the rebase, if you don't know which branch of maintainer's git can be used, linux-next tree [1] is useful. The linux-next git collects all next branches from other maintainers' git every day. [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/ Thank you. Best regards, Jingoo Han > --- > include/linux/backlight.h | 72 ++- > 1 file changed, 49 insertions(+), 23 deletions(-) . ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 05/20] backlight: improve backlight_device documentation
On Thu, 16 Jul 2020, Sam Ravnborg wrote: > Hi Lee. > > On Thu, Jul 16, 2020 at 03:39:41PM +0100, Lee Jones wrote: > > On Fri, 03 Jul 2020, Sam Ravnborg wrote: > > > > > Improve the documentation for backlight_device and > > > adapt it to kernel-doc style. > > > > > > The updated documentation is more strict on how locking is used. > > > With the update neither update_lock nor ops_lock may be used > > > outside the backlight core. > > > This restriction was introduced to keep the locking simple > > > by keeping it in the core. > > > It was verified that this documents the current state by renaming > > > update_lock => bl_update_lock and ops_lock => bl_ops_lock. > > > The rename did not reveal any uses outside the backlight core. > > > The rename is NOT part of this patch. > > > > > > v3: > > > - Update changelog to explain locking details (Daniel) > > > > > > v2: > > > - Add short intro to all fields (Daniel) > > > - Updated description of update_lock (Daniel) > > > > > > Signed-off-by: Sam Ravnborg > > > Reviewed-by: Emil Velikov > > > Cc: Lee Jones > > > Cc: Daniel Thompson > > > Cc: Jingoo Han > > > --- > > > include/linux/backlight.h | 72 ++- > > > 1 file changed, 49 insertions(+), 23 deletions(-) > > > > Some of these do not apply cleanly. > > > > Please collect the *-bys already received, rebase and resubmit. > > Will do. > The patch-set is based on drm-misc-next. Are there another > tree that I should use? I don't have anything to do with that tree. Either Backlight [0] or Next would be fine. [0] https://git.kernel.org/pub/scm/linux/kernel/git/lee/backlight.git/ -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 05/20] backlight: improve backlight_device documentation
Hi Lee. On Thu, Jul 16, 2020 at 03:39:41PM +0100, Lee Jones wrote: > On Fri, 03 Jul 2020, Sam Ravnborg wrote: > > > Improve the documentation for backlight_device and > > adapt it to kernel-doc style. > > > > The updated documentation is more strict on how locking is used. > > With the update neither update_lock nor ops_lock may be used > > outside the backlight core. > > This restriction was introduced to keep the locking simple > > by keeping it in the core. > > It was verified that this documents the current state by renaming > > update_lock => bl_update_lock and ops_lock => bl_ops_lock. > > The rename did not reveal any uses outside the backlight core. > > The rename is NOT part of this patch. > > > > v3: > > - Update changelog to explain locking details (Daniel) > > > > v2: > > - Add short intro to all fields (Daniel) > > - Updated description of update_lock (Daniel) > > > > Signed-off-by: Sam Ravnborg > > Reviewed-by: Emil Velikov > > Cc: Lee Jones > > Cc: Daniel Thompson > > Cc: Jingoo Han > > --- > > include/linux/backlight.h | 72 ++- > > 1 file changed, 49 insertions(+), 23 deletions(-) > > Some of these do not apply cleanly. > > Please collect the *-bys already received, rebase and resubmit. Will do. The patch-set is based on drm-misc-next. Are there another tree that I should use? Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 05/20] backlight: improve backlight_device documentation
On Fri, 03 Jul 2020, Sam Ravnborg wrote: > Improve the documentation for backlight_device and > adapt it to kernel-doc style. > > The updated documentation is more strict on how locking is used. > With the update neither update_lock nor ops_lock may be used > outside the backlight core. > This restriction was introduced to keep the locking simple > by keeping it in the core. > It was verified that this documents the current state by renaming > update_lock => bl_update_lock and ops_lock => bl_ops_lock. > The rename did not reveal any uses outside the backlight core. > The rename is NOT part of this patch. > > v3: > - Update changelog to explain locking details (Daniel) > > v2: > - Add short intro to all fields (Daniel) > - Updated description of update_lock (Daniel) > > Signed-off-by: Sam Ravnborg > Reviewed-by: Emil Velikov > Cc: Lee Jones > Cc: Daniel Thompson > Cc: Jingoo Han > --- > include/linux/backlight.h | 72 ++- > 1 file changed, 49 insertions(+), 23 deletions(-) Some of these do not apply cleanly. Please collect the *-bys already received, rebase and resubmit. -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 05/20] backlight: improve backlight_device documentation
On Fri, Jul 03, 2020 at 08:45:31PM +0200, Sam Ravnborg wrote: > Improve the documentation for backlight_device and > adapt it to kernel-doc style. > > The updated documentation is more strict on how locking is used. > With the update neither update_lock nor ops_lock may be used > outside the backlight core. > This restriction was introduced to keep the locking simple > by keeping it in the core. > It was verified that this documents the current state by renaming > update_lock => bl_update_lock and ops_lock => bl_ops_lock. > The rename did not reveal any uses outside the backlight core. > The rename is NOT part of this patch. > > v3: > - Update changelog to explain locking details (Daniel) > > v2: > - Add short intro to all fields (Daniel) > - Updated description of update_lock (Daniel) > > Signed-off-by: Sam Ravnborg > Reviewed-by: Emil Velikov > Cc: Lee Jones > Cc: Daniel Thompson > Cc: Jingoo Han Reviewed-by: Daniel Thompson Daniel. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 05/20] backlight: improve backlight_device documentation
Improve the documentation for backlight_device and
adapt it to kernel-doc style.
The updated documentation is more strict on how locking is used.
With the update neither update_lock nor ops_lock may be used
outside the backlight core.
This restriction was introduced to keep the locking simple
by keeping it in the core.
It was verified that this documents the current state by renaming
update_lock => bl_update_lock and ops_lock => bl_ops_lock.
The rename did not reveal any uses outside the backlight core.
The rename is NOT part of this patch.
v3:
- Update changelog to explain locking details (Daniel)
v2:
- Add short intro to all fields (Daniel)
- Updated description of update_lock (Daniel)
Signed-off-by: Sam Ravnborg
Reviewed-by: Emil Velikov
Cc: Lee Jones
Cc: Daniel Thompson
Cc: Jingoo Han
---
include/linux/backlight.h | 72 ++-
1 file changed, 49 insertions(+), 23 deletions(-)
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 10518b00b059..7654fe5f1589 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -14,21 +14,6 @@
#include
#include
-/* Notes on locking:
- *
- * backlight_device->ops_lock is an internal backlight lock protecting the
- * ops pointer and no code outside the core should need to touch it.
- *
- * Access to update_status() is serialised by the update_lock mutex since
- * most drivers seem to need this and historically get it wrong.
- *
- * Most drivers don't need locking on their get_brightness() method.
- * If yours does, you need to implement it in the driver. You can use the
- * update_lock mutex if appropriate.
- *
- * Any other use of the locks below is probably wrong.
- */
-
enum backlight_update_reason {
BACKLIGHT_UPDATE_HOTKEY,
BACKLIGHT_UPDATE_SYSFS,
@@ -215,30 +200,71 @@ struct backlight_properties {
enum backlight_scale scale;
};
+/**
+ * struct backlight_device - backlight device data
+ *
+ * This structure holds all data required by a backlight device.
+ */
struct backlight_device {
- /* Backlight properties */
+ /**
+* @props: Backlight properties
+*/
struct backlight_properties props;
- /* Serialise access to update_status method */
+ /**
+* @update_lock: The lock used when calling the update_status()
operation.
+*
+* update_lock is an internal backlight lock that serialise access
+* to the update_status() operation. The backlight core holds the
update_lock
+* when calling the update_status() operation. The update_lock shall not
+* be used by backlight drivers.
+*/
struct mutex update_lock;
- /* This protects the 'ops' field. If 'ops' is NULL, the driver that
- registered this device has been unloaded, and if class_get_devdata()
- points to something in the body of that driver, it is also invalid.
*/
+ /**
+* @ops_lock: The lock used around everything related to backlight_ops.
+*
+* ops_lock is an internal backlight lock that protects the ops pointer
+* and is used around all accesses to ops and when the operations are
+* invoked. The ops_lock shall not be used by backlight drivers.
+*/
struct mutex ops_lock;
+
+ /**
+* @ops: Pointer to the backlight operations.
+*
+* If ops is NULL, the driver that registered this device has been
unloaded,
+* and if class_get_devdata() points to something in the body of that
driver,
+* it is also invalid.
+*/
const struct backlight_ops *ops;
- /* The framebuffer notifier block */
+ /**
+* @fb_notif: The framebuffer notifier block
+*/
struct notifier_block fb_notif;
- /* list entry of all registered backlight devices */
+ /**
+* @entry: List entry of all registered backlight devices
+*/
struct list_head entry;
+ /**
+* @dev: Parent device.
+*/
struct device dev;
- /* Multiple framebuffers may share one backlight device */
+ /**
+* @fb_bl_on: The state of individual fbdev's.
+*
+* Multiple fbdev's may share one backlight device. The fb_bl_on
+* records the state of the individual fbdev.
+*/
bool fb_bl_on[FB_MAX];
+ /**
+* @use_count: The number of uses of fb_bl_on.
+*/
int use_count;
};
--
2.25.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
