Hi Sakari
On 05/21/2015 12:06 PM, Sakari Ailus wrote:
[...]
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.
Why should I need to refer to the device's root node?
What I meant here was that DT documentation enforces that even if
there is a single LED connected to the device it has to be expressed
as a sub-node anyway. Each LED will have to be matched by the phandle
to the sub-node representing it. This implies that v4l2_subdev->of_node
(related to sub-LED DT node) will be always different from dev->of_node
(related to LED controller DT node).
From driver point of view this makes no difference; it's just easier to
parse if you don't refer to the LEDs separately. I think this is a bit
special case; nowadays many LED flash controllers drive two LEDs.
As I understand, your stance is as follows:
- second argument to v4l2_flash_init needn't always be initialized
because some LEDs could be referred to by the phandle to the parent
node (e.g. flash LED and indicator under common sub-device)
If this is true, than how we would handle the situation where
there is a flash LED controller with two separate flash LEDs
and one of them is associated with indicator LED?
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.
How the sub-device name should look like then? We would have to
concatenate somehow both LED class device names?
It'd be different, i.e. there would be no flash or indicator in the name.
Currently there is no such a requirement too. As we discussed it few
months ago v4l2-flash sub-device name should be composed:
- for I2C devices "<LED class dev name> <i2c_adapter_id>-<i2c_addr>"
- for GPIO driven devices: <LED class dev name>
--
Best Regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
