Re: [PATCH 4/5] leds: aat1290: Pass dev and dev->of_node to v4l2_flash_init()

From: Sakari Ailus <sakari.ailus@iki.fi>
To: Jacek Anaszewski <j.anaszewski@samsung.com>
Cc: linux-media@vger.kernel.org, linux-leds@vger.kernel.org,
	cooloney@gmail.com, g.liakhovetski@gmx.de,
	s.nawrocki@samsung.com, laurent.pinchart@ideasonboard.com,
	mchehab@osg.samsung.com
Subject: Re: [PATCH 4/5] leds: aat1290: Pass dev and dev->of_node to v4l2_flash_init()
Date: Wed, 20 May 2015 17:31:43 +0300	[thread overview]
Message-ID: <20150520143143.GA8601@valkosipuli.retiisi.org.uk> (raw)
In-Reply-To: <555C906D.4030902@samsung.com>

Hi Jacek,

On Wed, May 20, 2015 at 03:47:25PM +0200, Jacek Anaszewski wrote:
...
> >>>>--- a/drivers/leds/leds-aat1290.c
> >>>>+++ b/drivers/leds/leds-aat1290.c
> >>>>@@ -524,9 +524,8 @@ static int aat1290_led_probe(struct
> >>>>platform_device *pdev)
> >>>>      led_cdev->dev->of_node = sub_node;
> >>>>
> >>>>      /* Create V4L2 Flash subdev. */
> >>>>-    led->v4l2_flash = v4l2_flash_init(fled_cdev,
> >>>>-                      &v4l2_flash_ops,
> >>>>-                      &v4l2_sd_cfg);
> >>>>+    led->v4l2_flash = v4l2_flash_init(dev, NULL, fled_cdev,
> >>>>+                      &v4l2_flash_ops, &v4l2_sd_cfg);
> >>>
> >>>Here the first argument should be led_cdev->dev, not dev, which is
> >>>&pdev->dev, whereas led_cdev->dev is returned by
> >>>device_create_with_groups (it takes dev as a parent) called from
> >>>led_classdev_register.
> >>
> >>The reason for this is the fact that pdev->dev has its of_node
> >>field initialized, which makes v4l2_async trying to match
> >>subdev by parent node of a LED device, not by sub-LED related
> >>DT node.
> >
> >If v4l2_subdev->of_node is set, then it won't be replaced with one from
> >struct device. I.e. you need to provide of_node pointer only if it's
> >different from dev->of_node.
> >
> 
> It will always be different since dev->of_node pointer is related
> to the main DT node of LED device, whereas each LED connected to it
> must be expressed in the form of sub-node, as
> Documentation/devicetree/bindings/leds/common.txt DT states.

You can still refer to the device's root device_node using a phandle.

Say, if you have a LED flash controller with an indicator. It's intended to
be used together with the flash LED, and the existing as3645a driver exposes
it through the same sub-device. I think that'd make sense with LED class
driver as well (i.e. you'd have two LED class devices but a single
sub-device). Small changes to the wrapper would be needed.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk