Hi Bjorn
Thanks for the review.
Reply inline.
On 2018-04-16 09:41, Bjorn Andersson wrote:
On Sat 14 Apr 00:25 PDT 2018, Abhinav Kumar wrote:
Register truly panel as a backlight led device and
provide methods to control its backlight operation.
Changes in v2:
- Removed redundant NULL checks
- Arranged headers alphabetically
- Formatting fixes
The change log goes below the "---" line.
[Abhinav] As sean mentioned, its quite common to list it to view it in
the log.
This practice has been followed by quite a few in DRM
Another reference here https://patchwork.freedesktop.org/patch/211361/
Signed-off-by: Abhinav Kumar <abhinavk@xxxxxxxxxxxxxx>
---
[..]
+static int truly_backlight_setup(struct truly_wqxga *ctx)
+{
+ struct backlight_properties props;
+ char bl_node_name[BL_NODE_NAME_SIZE];
+
+ if (!ctx->backlight) {
+ memset(&props, 0, sizeof(props));
+ props.type = BACKLIGHT_RAW;
+ props.power = FB_BLANK_UNBLANK;
+ props.max_brightness = 4096;
+
+ snprintf(bl_node_name, BL_NODE_NAME_SIZE, "panel%u-backlight",
+ PRIM_DISPLAY_NODE);
+
+ ctx->backlight = backlight_device_register(bl_node_name,
+ ctx->dev, ctx,
+ &truly_backlight_device_ops, &props);
+
+ if (IS_ERR_OR_NULL(ctx->backlight)) {
+ pr_err("Failed to register backlight\n");
+ ctx->backlight = NULL;
+ return -ENODEV;
+ }
+
+ /* Register with the LED driver interface */
+ led_trigger_register_simple("bkl-trigger", &ctx->wled);
+
+ if (!ctx->wled) {
+ pr_err("backlight led registration failed\n");
+ return -ENODEV;
+ }
It seems like you're registering a backlight driver for the sake of
invoking the LED backlight trigger to control the WLED.
The WLED is a backlight driver, so all you should have to do is add the
following line to your probe:
ctx->backlight = devm_of_find_backlight(dev);
and then add "backlight = <&wled>" to your dt node.
Regards,
Bjorn
[Abhinav] Thats not the only purpose of backlight_device_register().
We want to hook up our panel with the parent backlight driver in
drivers/video/backlight/backlight.c and also route all the
update_backlight_status()
calls through the sysfs of the newly registered node.
The of_find_backlight() method doesnt seem to allow us to register our
own
sysfs method.
BTW, this isnt something which we are doing uniquely.
There are other panels which seem to be doing this :
drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c
Can you please comment on whether we can have our own sysfs without
the device_register()?
_______________________________________________
Freedreno mailing list
Freedreno@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/freedreno
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html