From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,EXCUSE_4,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 50D9BC433E0 for ; Mon, 18 May 2020 15:53:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 29B86207FB for ; Mon, 18 May 2020 15:53:31 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="S9NQFwn8" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728504AbgERPxa (ORCPT ); Mon, 18 May 2020 11:53:30 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42620 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728463AbgERPxa (ORCPT ); Mon, 18 May 2020 11:53:30 -0400 Received: from mail-wr1-x443.google.com (mail-wr1-x443.google.com [IPv6:2a00:1450:4864:20::443]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 25405C061A0C for ; Mon, 18 May 2020 08:53:30 -0700 (PDT) Received: by mail-wr1-x443.google.com with SMTP id w7so12415813wre.13 for ; Mon, 18 May 2020 08:53:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=1MOGVGDs5OeGfwi3VK8Xlt79QAxduVVC7NWl7I/nzsM=; b=S9NQFwn8O8M7YdxD6fWzC1AmG7i86Q4EXgHk8ojNzkC3dPe8+/OSHtV/OnUOIDek+w UXa/JRwiUrejrJTZz7opZIteAnbXftSoS5eCLVCrn/lTpkmuhqsRYq2giOBTYSlZoJez xkO/VO5J62rqmTQlA1rBqDEWPf0unLjozixBIKO8pBkVQcPzV+DPRVjGa7bi6Vh0zwlS FDtw4i1eWK0ESp9tXdu31UWdtoYYzR68oVj3BZZMxO0o+AeUQvJB6oML5TpGrag6YUhw 8D5K6ON5x42tIjgaw+HNuZ98FG07B0JmNCcUZ1e13JBD/r6jgsfv+Hp8rRD6Csp+Flqq 5Wlw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=1MOGVGDs5OeGfwi3VK8Xlt79QAxduVVC7NWl7I/nzsM=; b=cghiZa7ekF+qrxcaKabVl2HNobei1HWHbp3aEsJG3bRoA8sCDm8YRfCE8em7yGUh1c q7G5PoYN5PPKIRC0GN3qbeZy8qaERKOkgwCrJ8badtRWlhTE0fDnFK96ngIUgxI7jbmT pWJ5idPt844Hkmvda7gqd+zNU6tD+VliQFC5QmwZZJnMv54mWXJgKGNLyd9p3O+Xg89o nYPMTBgWkwRnlbgDRDkk1W0yQHCI1GaV2gXp6vpDz+EjSYcyy6wjmveDqIJEYmaXQ/cD dB81999rvevINlezJRwzTJwoo2iwMjruk6eovqLpNLsEa9z0KmTFZuygFp7wQb8/rXO9 cSPQ== X-Gm-Message-State: AOAM532SQ2jBN/ijOJvY8xw6vyKNHcO1HprJNyYOqKSK1j3eZfqcgX1+ c0NZo4cNWhe84K6pABmSg+NyMw== X-Google-Smtp-Source: ABdhPJyHnttLi85aikrcbMslC+yDiCMUkFPHFALi19qf+ZfR5WFpz/Wcso0LxGP5WIJfH2AgY4oYpg== X-Received: by 2002:a05:6000:1244:: with SMTP id j4mr21070478wrx.189.1589817208789; Mon, 18 May 2020 08:53:28 -0700 (PDT) Received: from holly.lan (cpc141214-aztw34-2-0-cust773.18-1.cable.virginm.net. [86.9.19.6]) by smtp.gmail.com with ESMTPSA id 128sm17837626wme.39.2020.05.18.08.53.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 18 May 2020 08:53:28 -0700 (PDT) Date: Mon, 18 May 2020 16:53:26 +0100 From: Daniel Thompson To: Sam Ravnborg Cc: dri-devel@lists.freedesktop.org, Jingoo Han , Lee Jones , Peter Ujfalusi , Tomi Valkeinen , Andy Gross , Bartlomiej Zolnierkiewicz , Bjorn Andersson , Daniel Vetter , David Airlie , Douglas Anderson , Jani Nikula , Jonathan Corbet , Linus Walleij , linux-arm-msm@vger.kernel.org, linux-pwm@vger.kernel.org, Maarten Lankhorst , Maxime Ripard , Michael Hennerich , patches@opensource.cirrus.com, Russell King , Support Opensource , Thierry Reding , Thomas Zimmermann , Uwe Kleine-Konig Subject: Re: [PATCH v2 05/16] backlight: improve backlight_properties documentation Message-ID: <20200518155326.a35lny7xtsvynibo@holly.lan> References: <20200517190139.740249-1-sam@ravnborg.org> <20200517190139.740249-6-sam@ravnborg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200517190139.740249-6-sam@ravnborg.org> Sender: linux-arm-msm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org On Sun, May 17, 2020 at 09:01:28PM +0200, Sam Ravnborg wrote: > Improve the documentation for backlight_properties and > adapt it to kernel-doc style. > > Signed-off-by: Sam Ravnborg > Cc: Lee Jones > Cc: Daniel Thompson > Cc: Jingoo Han Overall looks good but enough nits that I felt compelled to comment! > --- > include/linux/backlight.h | 101 +++++++++++++++++++++++++++++++++----- > 1 file changed, 90 insertions(+), 11 deletions(-) > > diff --git a/include/linux/backlight.h b/include/linux/backlight.h > index 519dc61ce7e4..7f9cef299d6e 100644 > --- a/include/linux/backlight.h > +++ b/include/linux/backlight.h > @@ -118,28 +118,107 @@ struct backlight_ops { > int (*check_fb)(struct backlight_device *bd, struct fb_info *info); > }; > > -/* This structure defines all the properties of a backlight */ > +/** > + * struct backlight_properties - backlight properties > + * > + * This structure defines all the properties of a backlight. > + */ > struct backlight_properties { > - /* Current User requested brightness (0 - max_brightness) */ > + /** > + * @brightness: > + * > + * The current requested brightness by the user. This applies throughout this file (and perhaps I overlooked it in the previous patc too) but having line breaks after @brightness: differs from the canonical description of a kerneldoc command in: https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#in-line-member-documentation-comments Also: s/requested brightness/brightness requested/ > + * The backlight core makes sure the range is (0 - max_brightness) I know this is a copy of the original text but I'd prefer the range not to use the subtract operator ;-). Maybe 0..max_brightness like the ranges below? > + * when the brightness is set via the sysfs attribute: > + * /sys/class/backlight//brightness. > + * > + * This value can be set in the backlight_properties passed > + * to devm_backlight_device_register() to set a default brightness > + * value. > + */ > int brightness; > - /* Maximal value for brightness (read-only) */ > + > + /** > + * @max_brightness: > + * > + * The maximum brightness value. > + * > + * This value must be set in the backlight_properties passed > + * to devm_backlight_device_register(). > + * > + * This property must not be modified by a driver except > + * before registering the backlight device as explained above. Perhaps combine these (rather than "as explained above"): This value must be set in the backlight_properties passed to devm_backlight_device_register() and shall not be modified by the driver after registration. > + */ > int max_brightness; > - /* Current FB Power mode (0: full on, 1..3: power saving > - modes; 4: full off), see FB_BLANK_XXX */ > + > + /** > + * @power: > + * > + * The current power mode. User space configure the power mode using s/configure/can configure/ > + * the sysfs attribute: /sys/class/backlight//bl_power > + * When the power property is updated update_status() is called. > + * > + * The possible values are: (0: full on, 1..3: power saving > + * modes; 4: full off), see FB_BLANK_XXX. > + * > + * When the backlight device is enabled @power is set > + * to FB_BLANK_UNBLANK. When the backlight device is disabled > + * @power is set to FB_BLANK_POWERDOWN. > + */ > int power; > - /* FB Blanking active? (values as for power) */ > - /* Due to be removed, please use (state & BL_CORE_FBBLANK) */ > + > + /** > + * @fb_blank: > + * > + * When the FBIOBLANK ioctl is called fb_blank is set to the > + * blank parameter and the update_status() operation is called. > + * > + * When the backlight device is enabled @fb_blank is set > + * to FB_BLANK_UNBLANK. When the backlight device is disabled > + * @fb_blank is set to FB_BLANK_POWERDOWN. > + * > + * This property must not be modified by a driver. > + * The backlight driver shall never read this variable, > + * as the necessary info is avaialble via the state. I'd rather be told what to do that what not to do! Maybe. Backlight drivers should avoid using this property. It has been replaced by state & BL_CORE_FBLANK (although most drivers should use backlight_is_blank() as the preferred means to get the blank state. Daniel. > + * > + * fb_blank is deprecated and will be removed. > + */ > int fb_blank; > - /* Backlight type */ > + > + /** > + * @type: > + * > + * The type of backlight supported. > + * The backlight type allows userspace to make appropriate > + * policy desicions based on the backlight type. > + * > + * This value must be set in the backlight_properties > + * passed to devm_backlight_device_register(). > + */ > enum backlight_type type; > - /* Flags used to signal drivers of state changes */ > + > + /** > + * @state: > + * > + * The state property is used to inform drivers of state changes > + * when the update_status() operation is called. > + * The state is a bitmask. BL_CORE_FBBLANK is set when the display > + * is expected to be blank. BL_CORE_SUSPENDED is set when the > + * driver is suspended. > + * > + * This property must not be modified by a driver. > + */ > unsigned int state; > - /* Type of the brightness scale (linear, non-linear, ...) */ > - enum backlight_scale scale; > > #define BL_CORE_SUSPENDED (1 << 0) /* backlight is suspended */ > #define BL_CORE_FBBLANK (1 << 1) /* backlight is under an fb blank event */ > > + /** > + * @scale: > + * > + * The type of the brightness scale (linear, non-linear, ...) > + */ > + enum backlight_scale scale; > }; > > struct backlight_device { > -- > 2.25.1 >