On 25/05/15 17:14, Jacek Anaszewski wrote:
> This patch adds support for external v4l2-flash devices.
> The support includes parsing "camera-flashes" DT property

"samsung,camera-flashes" ?

> and asynchronous sub-device registration.

> +static int fimc_md_register_flash_entities(struct fimc_md *fmd)
> +{
> +     struct device_node *parent = fmd->pdev->dev.of_node, *np_sensor,
> +             *np_flash;
> +     struct v4l2_async_notifier *notifier = &fmd->subdev_notifier;
> +     struct v4l2_async_subdev *asd;
> +     int i, j, num_flashes = 0, num_elems;
> +
> +     num_elems = of_property_count_elems_of_size(parent,
> +                             "samsung,camera-flashes", sizeof(np_flash));

I think this should be of_property_count_u32_elems(), phandle is always
a 32-bit value [1], while size of a pointer depends on the architecture.


> +     /* samsung,camera-flashes property is optional */
> +     if (num_elems < 0)
> +             return 0;
> +
> +     /* samsung,camera-flashes array must have even number of elements */
> +     if ((num_elems & 1) || (num_elems > FIMC_MAX_SENSORS * 2))
> +             return -EINVAL;
> +
> +     for (i = 0; i < num_elems; i += 2) {
> +             np_sensor = of_parse_phandle(parent,
> +                                          "samsung,camera-flashes", i);
> +
> +             for (j = 0; j < fmd->num_sensors; j++)
> +                     if (fmd->async_subdevs.sensors[j].match.of.node ==
> +                         np_sensor)
> +                             break;
> +
> +             of_node_put(np_sensor);

Would be good to add some comment here, why is the sensor required.
It's just a DT correctness check? Couldn't we carry on with the flash
registration after just emitting some warning?

> +             if (j == fmd->num_sensors)
> +                     continue;
> +
> +             np_flash = of_parse_phandle(parent, "samsung,camera-flashes",
> +                                             i + 1);
> +
> +             asd = &fmd->async_subdevs.flashes[num_flashes++];
> +             asd->match_type = V4L2_ASYNC_MATCH_OF;
> +             asd->match.of.node = np_flash;
> +             notifier->subdevs[notifier->num_subdevs++] = asd;
> +
> +             of_node_put(np_flash);
> +     }
> +
> +     return 0;
> +}

Otherwise looks good to me.

-- 
Thanks,
Sylwester

[1]
http://lxr.free-electrons.com/source/Documentation/devicetree/booting-without-of.txt#L545

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to