Den 23.01.2018 18.41, skrev Noralf Trønnes:

Den 23.01.2018 17.55, skrev Meghana Madhyastha:
On Tue, Jan 23, 2018 at 03:37:38PM +0100, Noralf Trønnes wrote:
Den 22.01.2018 15.56, skrev Meghana Madhyastha:
Replace of_find_backlight_by_node and of the code around it
with of_find_backlight helper to avoid repetition of code.

Signed-off-by: Meghana Madhyastha <meghana.madhyas...@gmail.com>
---
Changes in v18:
-Fixed warnings resulting from passing device_node* to of_find_backlight.
  Fixed it by passing struct device* to of_find_backlight

  drivers/gpu/drm/omapdrm/displays/panel-dpi.c | 33 ++++++++++------------------
  1 file changed, 11 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
index ac9596251..93b7a176d 100644
--- a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
+++ b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
@@ -156,14 +156,14 @@ static struct omap_dss_driver panel_dpi_ops = {
  static int panel_dpi_probe_of(struct platform_device *pdev)
  {
      struct panel_drv_data *ddata = platform_get_drvdata(pdev);
+    struct device *dev = &pdev->dev;
      struct device_node *node = pdev->dev.of_node;
-    struct device_node *bl_node;
      struct omap_dss_device *in;
      int r;
      struct display_timing timing;
      struct gpio_desc *gpio;
-    gpio = devm_gpiod_get_optional(&pdev->dev, "enable", GPIOD_OUT_LOW);
+    gpio = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_LOW);
Please don't make unrelated changes like this. It clutters the patch.
You can just as well use &pdev->dev when getting the backlight also.
I had made the change in order to be more consistent with how the other
drivers were doing this. Most of them had a variable struct device *dev.
However, I can undo this if necessary.

It's best to be consistent with the coding style in the driver you're
changing. If you make an extra dev variable or not isn't that important,
unless the driver maintainer have a strict coding style for their driver.

I try to stay on the safe side, change as little as possible and do thing
the way it's done in the driver to increase the change of getting the
patch accepted as-is the first time around.

The important feedback from me is to remove the unrelated changes.


and the use of the devm_ version ofc.

Noralf.


      if (IS_ERR(gpio))
          return PTR_ERR(gpio);
@@ -175,47 +175,36 @@ static int panel_dpi_probe_of(struct platform_device *pdev)        * timing and order relative to the enable gpio. So for now it's just
       * ensured that the reset line isn't active.
       */
-    gpio = devm_gpiod_get_optional(&pdev->dev, "reset", GPIOD_OUT_LOW);
+    gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
      if (IS_ERR(gpio))
          return PTR_ERR(gpio);
-    ddata->vcc_supply = devm_regulator_get(&pdev->dev, "vcc");
+    ddata->vcc_supply = devm_regulator_get(dev, "vcc");
      if (IS_ERR(ddata->vcc_supply))
          return PTR_ERR(ddata->vcc_supply);
-    bl_node = of_parse_phandle(node, "backlight", 0);
-    if (bl_node) {
-        ddata->backlight = of_find_backlight_by_node(bl_node);
-        of_node_put(bl_node);
+    ddata->backlight = of_find_backlight(dev);
Any reason you don't use the devm_ version here?
You do remove error_free_backlight...

With the devm_ version remember to drop the put_device in
panel_dpi_remove().

Noralf.

-        if (!ddata->backlight)
-            return -EPROBE_DEFER;
-    }
+    if (IS_ERR(ddata->backlight))
+        return PTR_ERR(ddata->backlight);
      r = of_get_display_timing(node, "panel-timing", &timing);
      if (r) {
-        dev_err(&pdev->dev, "failed to get video timing\n");
-        goto error_free_backlight;
+        dev_err(dev, "failed to get video timing\n");
+        return r;
      }
      videomode_from_timing(&timing, &ddata->vm);
      in = omapdss_of_find_source_for_first_ep(node);
      if (IS_ERR(in)) {
-        dev_err(&pdev->dev, "failed to find video source\n");
-        r = PTR_ERR(in);
-        goto error_free_backlight;
+        dev_err(dev, "failed to find video source\n");
+        return PTR_ERR(in);
      }
      ddata->in = in;
      return 0;
-
-error_free_backlight:
-    if (ddata->backlight)
-        put_device(&ddata->backlight->dev);
-
-    return r;
  }
  static int panel_dpi_probe(struct platform_device *pdev)

_______________________________________________
dri-devel mailing list
dri-de...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Reply via email to