Hi Jacek,
On Wed, Apr 15, 2015 at 08:48:33AM +0200, Jacek Anaszewski wrote:
...
> +static int max77693_led_parse_dt(struct max77693_led_device *led,
> + struct max77693_led_config_data *cfg)
> +{
> + struct device *dev = &led->pdev->dev;
> + struct max77693_sub_led *sub_leds = led->sub_leds;
> + struct device_node *node = dev->of_node, *child_node;
> + struct property *prop;
> + u32 led_sources[2];
> + int i, ret, fled_id;
> +
> + of_property_read_u32(node, "maxim,boost-mode", &cfg->boost_mode);
> + of_property_read_u32(node, "maxim,boost-mvout", &cfg->boost_vout);
> + of_property_read_u32(node, "maxim,mvsys-min", &cfg->low_vsys);
> +
> + for_each_available_child_of_node(node, child_node) {
> + prop = of_find_property(child_node, "led-sources", NULL);
> + if (prop) {
> + const __be32 *srcs = NULL;
> +
> + for (i = 0; i < ARRAY_SIZE(led_sources); ++i) {
> + srcs = of_prop_next_u32(prop, srcs,
> + &led_sources[i]);
> + if (!srcs)
> + break;
> + }
> + } else {
> + dev_err(dev,
> + "led-sources DT property missing\n");
> + return -EINVAL;
If you exit the loop in the middle, I think you'll need to do
of_node_put(child_node) first.
> + }
> +
> + if (i == 2) {
> + fled_id = FLED1;
> + led->fled_mask = FLED1_IOUT | FLED2_IOUT;
> + } else if (led_sources[0] == FLED1) {
> + fled_id = FLED1;
> + led->fled_mask |= FLED1_IOUT;
> + } else if (led_sources[0] == FLED2) {
> + fled_id = FLED2;
> + led->fled_mask |= FLED2_IOUT;
> + } else {
> + dev_err(dev,
> + "Wrong led-sources DT property value.\n");
> + return -EINVAL;
Same here.
> + }
> +
> + sub_leds[fled_id].fled_id = fled_id;
> +
> + cfg->label[fled_id] =
> + of_get_property(child_node, "label", NULL) ? :
> + child_node->name;
I think you should copy the string here, or keep a reference to child_node.
of_property_read_string() might be useful.
> +
> + ret = of_property_read_u32(child_node, "led-max-microamp",
> + &cfg->iout_torch_max[fled_id]);
> + if (ret < 0) {
> + cfg->iout_torch_max[fled_id] = TORCH_IOUT_MIN;
> + dev_warn(dev, "led-max-microamp DT property missing\n");
> + }
> +
> + ret = of_property_read_u32(child_node, "flash-max-microamp",
> + &cfg->iout_flash_max[fled_id]);
> + if (ret < 0) {
> + cfg->iout_flash_max[fled_id] = FLASH_IOUT_MIN;
> + dev_warn(dev,
> + "flash-max-microamp DT property missing\n");
> + }
> +
> + ret = of_property_read_u32(child_node, "flash-max-timeout-us",
> + &cfg->flash_timeout_max[fled_id]);
> + if (ret < 0) {
> + cfg->flash_timeout_max[fled_id] = FLASH_TIMEOUT_MIN;
> + dev_warn(dev,
> + "flash-max-timeout-us DT property missing\n");
> + }
> +
> + if (++cfg->num_leds == 2 ||
> + (max77693_fled_used(led, FLED1) &&
> + max77693_fled_used(led, FLED2)))
of_node_put(child_node);
> + break;
> + }
> +
> + if (cfg->num_leds == 0) {
> + dev_err(dev, "No DT child node found for connected LED(s).\n");
> + return -EINVAL;
> + }
> +
> + return 0;
With these matters addressed,
Acked-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
--
Kind regards,
Sakari Ailus
e-mail: sakari.ailus@xxxxxx XMPP: sailus@xxxxxxxxxxxxxx
--
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
