Re: [PATCH v19 03/10] video: backlight: Add of_find_backlight helper in backlight.c

[Thierry Reding]

On Wed, Jan 24, 2018 at 04:35:30PM +0000, Meghana Madhyastha wrote:
> Add of_find_backlight, a helper function which is a generic version
> of tinydrm_of_find_backlight that can be used by other drivers to avoid
> repetition of code and simplify things.
> 
> Acked-by: Daniel Thompson <daniel.thompson@xxxxxxxxxx>
> Reviewed-by: Noralf TrÃnnes <noralf@xxxxxxxxxxx>
> Reviewed-by: Sean Paul<seanpaul@xxxxxxxxxxxx>
> Signed-off-by: Meghana Madhyastha <meghana.madhyastha@xxxxxxxxx>
> ---
>  drivers/video/backlight/backlight.c | 43 +++++++++++++++++++++++++++++++++++++
>  include/linux/backlight.h           | 19 ++++++++++++++++
>  2 files changed, 62 insertions(+)
> 
> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
> index 8049e7656..553bf5c48 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -580,6 +580,49 @@ struct backlight_device *of_find_backlight_by_node(struct device_node *node)
>  EXPORT_SYMBOL(of_find_backlight_by_node);
>  #endif
>  
> +/**
> + * of_find_backlight - Get backlight device
> + * @dev: Device
> + *
> + * This function looks for a property named 'backlight' on the DT node
> + * connected to @dev and looks up the backlight device.
> + *
> + * Call backlight_put() to drop the reference on the backlight device.
> + *
> + * Returns:
> + * A pointer to the backlight device if found.
> + * Error pointer -EPROBE_DEFER if the DT property is set, but no backlight
> + * device is found.
> + * NULL if there's no backlight property.
> + */
> +struct backlight_device *of_find_backlight(struct device *dev)

Just a nit: there isn't really anything OF specific about this. It would
be if this took a struct device_node * as parameter. So technically you
could implement this for ACPI or whatever other firmware interface as
well. So I think a more idiomatic name for this would be:

	struct backlight_device *backlight_get(struct device *dev);

which would match the:

	void backlight_put(struct backlight_device *bd);

and be consistent with how other resources are obtained. Not a big deal,
though. It can always be changed later on.

Reviewed-by: Thierry Reding <treding@xxxxxxxxxx>

Attachment: signature.asc
Description: PGP signature