[PATCH v6 1/2] drm/tinydrm: Move tinydrm_of_find_backlight into drm_of.c
Noralf Trønnes
noralf at tronnes.org
Sun Oct 1 13:26:36 UTC 2017
Den 01.10.2017 06.14, skrev Meghana Madhyastha:
> On Sat, Sep 30, 2017 at 09:04:57PM +0200, Noralf Trønnes wrote:
>> Den 30.09.2017 19.12, skrev Meghana Madhyastha:
>>> Rename tinydrm_of_find_backlight to drm_of_find_backlight
>>> and move it into drm_of.c from tinydrm-helpers.c. This is
>>> because other drivers in the drm subsystem might need to call
>>> this function. In that case and otherwise, it is better from
>>> an organizational point of view to move it into drm_of.c along
>>> with the other _of.c functions.
>>>
>>> Signed-off-by: Meghana Madhyastha <meghana.madhyastha at gmail.com>
>>> ---
>>> Changes in v6:
>>> -Move function declarations to linux/backlight.h to resolve
>>> module dependency issues.
>>>
>>> drivers/gpu/drm/drm_of.c | 44 ++++++++++++++++++++++++++
>>> drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 40 -----------------------
>>> drivers/gpu/drm/tinydrm/mi0283qt.c | 2 +-
>>> include/drm/tinydrm/tinydrm-helpers.h | 1 -
>>> include/linux/backlight.h | 11 +++++++
>>> 5 files changed, 56 insertions(+), 42 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
>>> index 8dafbdf..d878d3a 100644
>>> --- a/drivers/gpu/drm/drm_of.c
>>> +++ b/drivers/gpu/drm/drm_of.c
>>> @@ -1,6 +1,7 @@
>>> #include <linux/component.h>
>>> #include <linux/export.h>
>>> #include <linux/list.h>
>>> +#include <linux/backlight.h>
>>> #include <linux/of_graph.h>
>>> #include <drm/drmP.h>
>>> #include <drm/drm_bridge.h>
>>> @@ -260,3 +261,46 @@ int drm_of_find_panel_or_bridge(const struct device_node *np,
>>> return ret;
>>> }
>>> EXPORT_SYMBOL_GPL(drm_of_find_panel_or_bridge);
>>> +
>>> +/**
>>> + * drm_of_find_backlight - Find backlight device in device-tree
>>> + * @dev: Device
>>> + *
>>> + * This function looks for a DT node pointed to by a property named 'backlight'
>>> + * and uses of_find_backlight_by_node() to get the backlight device.
>>> + * Additionally if the brightness property is zero, it is set to
>>> + * max_brightness.
>>> + *
>>> + * Note: It is the responsibility of the caller to call put_device() when
>>> + * releasing the resource.
>>> + *
>>> + * Returns:
>>> + * NULL if there's no backlight property.
>>> + * Error pointer -EPROBE_DEFER if the DT node is found, but no backlight device
>>> + * is found.
>>> + * If the backlight device is found, a pointer to the structure is returned.
>>> + */
>>> +struct backlight_device *drm_of_find_backlight(struct device *dev)
>>> +{
>>> + struct backlight_device *backlight;
>>> + struct device_node *np;
>>> +
>>> + np = of_parse_phandle(dev->of_node, "backlight", 0);
>>> + if (!np)
>>> + return NULL;
>>> +
>>> + backlight = of_find_backlight_by_node(np);
>>> + of_node_put(np);
>>> +
>>> + if (!backlight)
>>> + return ERR_PTR(-EPROBE_DEFER);
>>> +
>>> + if (!backlight->props.brightness) {
>>> + backlight->props.brightness = backlight->props.max_brightness;
>>> + DRM_DEBUG_KMS("Backlight brightness set to %d\n",
>>> + backlight->props.brightness);
>>> + }
>>> +
>>> + return backlight;
>>> +}
>>> +EXPORT_SYMBOL(drm_of_find_backlight);
>>> diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
>>> index bd6cce0..cd4c6a5 100644
>>> --- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
>>> +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
>>> @@ -237,46 +237,6 @@ void tinydrm_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
>>> EXPORT_SYMBOL(tinydrm_xrgb8888_to_gray8);
>>> /**
>>> - * tinydrm_of_find_backlight - Find backlight device in device-tree
>>> - * @dev: Device
>>> - *
>>> - * This function looks for a DT node pointed to by a property named 'backlight'
>>> - * and uses of_find_backlight_by_node() to get the backlight device.
>>> - * Additionally if the brightness property is zero, it is set to
>>> - * max_brightness.
>>> - *
>>> - * Returns:
>>> - * NULL if there's no backlight property.
>>> - * Error pointer -EPROBE_DEFER if the DT node is found, but no backlight device
>>> - * is found.
>>> - * If the backlight device is found, a pointer to the structure is returned.
>>> - */
>>> -struct backlight_device *tinydrm_of_find_backlight(struct device *dev)
>>> -{
>>> - struct backlight_device *backlight;
>>> - struct device_node *np;
>>> -
>>> - np = of_parse_phandle(dev->of_node, "backlight", 0);
>>> - if (!np)
>>> - return NULL;
>>> -
>>> - backlight = of_find_backlight_by_node(np);
>>> - of_node_put(np);
>>> -
>>> - if (!backlight)
>>> - return ERR_PTR(-EPROBE_DEFER);
>>> -
>>> - if (!backlight->props.brightness) {
>>> - backlight->props.brightness = backlight->props.max_brightness;
>>> - DRM_DEBUG_KMS("Backlight brightness set to %d\n",
>>> - backlight->props.brightness);
>>> - }
>>> -
>>> - return backlight;
>>> -}
>>> -EXPORT_SYMBOL(tinydrm_of_find_backlight);
>>> -
>>> -/**
>>> * tinydrm_enable_backlight - Enable backlight helper
>>> * @backlight: Backlight device
>>> *
>>> diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c
>>> index 7e5bb7d..682b4db 100644
>>> --- a/drivers/gpu/drm/tinydrm/mi0283qt.c
>>> +++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
>>> @@ -189,7 +189,7 @@ static int mi0283qt_probe(struct spi_device *spi)
>>> if (IS_ERR(mipi->regulator))
>>> return PTR_ERR(mipi->regulator);
>>> - mipi->backlight = tinydrm_of_find_backlight(dev);
>>> + mipi->backlight = drm_of_find_backlight(dev);
>>> if (IS_ERR(mipi->backlight))
>>> return PTR_ERR(mipi->backlight);
>>> diff --git a/include/drm/tinydrm/tinydrm-helpers.h b/include/drm/tinydrm/tinydrm-helpers.h
>>> index d554ded..e40ef2d 100644
>>> --- a/include/drm/tinydrm/tinydrm-helpers.h
>>> +++ b/include/drm/tinydrm/tinydrm-helpers.h
>>> @@ -46,7 +46,6 @@ void tinydrm_xrgb8888_to_rgb565(u16 *dst, void *vaddr,
>>> void tinydrm_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
>>> struct drm_clip_rect *clip);
>>> -struct backlight_device *tinydrm_of_find_backlight(struct device *dev);
>>> int tinydrm_enable_backlight(struct backlight_device *backlight);
>>> int tinydrm_disable_backlight(struct backlight_device *backlight);
>>> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
>>> index 5f2fd61..47a095e 100644
>>> --- a/include/linux/backlight.h
>>> +++ b/include/linux/backlight.h
>>> @@ -172,4 +172,15 @@ of_find_backlight_by_node(struct device_node *node)
>>> }
>>> #endif
>>> +#if defined(CONFIG_OF) && IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
>>> +struct backlight_device *
>>> +drm_of_find_backlight(struct device *dev);
>>> +#else
>>> +static inline struct backlight_device *
>>> +drm_of_find_backlight(struct device *dev)
>>> +{
>>> + return NULL;
>>> +}
>>> +#endif
>>> +
>>> #endif
>> This isn't what I meant, you only change the of_find_backlight_by_node()
>> declaration/definition. It should be like this:
>>
>> #if defined(CONFIG_OF) && IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
>> struct backlight_device *of_find_backlight_by_node(struct device_node
>> *node);
>> #else
>> static inline struct backlight_device *
>> of_find_backlight_by_node(struct device_node *node)
>> {
>> return NULL;
>> }
>> #endif
> I see. So basically add the function declaration of drm_of_find_backlight
> and devm_drm_of_find_backlight in drm_of.h and this function declaration
> need not be inside the #ifdef CONFIG / #else part?
> So there need not be a dummy version of drm_of_find_backlight here
> because we are anyway taking care of that in of_find_backliht by node?
> Sorry for the many versions and thank you for the clarifications.
You need the dummy part in drm_of.h because you need to handle the
CONFIG_OF option here as well.
If drm_of_find_backlight() was only 2-3 lines of code, you could have
made a static inline function in drm_of.h outside the #if/#else/#endif,
but since it's longer than that it should be in drm_of.c.
CONFIG_OF decides if drm_of.c is built (as part of drm.o) or not:
drivers/gpu/drm/Makefile:
drm-$(CONFIG_OF) += drm_of.o
This means we need a function prototype declaration for the CONFIG_OF=y
case, and an inline function definition for the CONFIG_OF=n case.
Noralf.
> Regards,
> Meghana
>> And this change needs to be a patch on it's own. The backlight subsystem
>> has it's own maintainer that will review this change.
>> It's best to send him/them the whole patchset so they see the context.
>>
>> Noralf.
>>
More information about the dri-devel
mailing list