On 01/21/2013 12:35 PM, Hans Verkuil wrote:
[...]
>> +/**
>> + * v4l2_of_parse_mipi_csi2() - parse MIPI CSI-2 bus properties
>> + * @node: pointer to endpoint device_node
>> + * @endpoint: pointer to v4l2_of_endpoint data structure
>> + *
>> + * Return: 0 on success or negative error value otherwise.
>> + */
>> +int v4l2_of_parse_mipi_csi2(const struct device_node *node,
>> + struct v4l2_of_endpoint *endpoint)
>> +{
>> + struct v4l2_of_mipi_csi2 *mipi_csi2 = &endpoint->mipi_csi_2;
>> + u32 data_lanes[ARRAY_SIZE(mipi_csi2->data_lanes)];
>> + struct property *prop;
>> + const __be32 *lane = NULL;
>> + u32 v;
>> + int i = 0;
>> +
>> + prop = of_find_property(node, "data-lanes", NULL);
>> + if (!prop)
>> + return -EINVAL;
>> + do {
>> + lane = of_prop_next_u32(prop, lane, &data_lanes[i]);
>> + } while (lane && i++ < ARRAY_SIZE(data_lanes));
>> +
>> + mipi_csi2->num_data_lanes = i;
>> + while (i--)
>> + mipi_csi2->data_lanes[i] = data_lanes[i];
>> +
>> + if (!of_property_read_u32(node, "clock-lanes", &v))
>> + mipi_csi2->clock_lane = v;
>
> Hmm, the property name is 'clock-lanes', but only one lane is obtained here.
>
> Why is the property name plural in that case?
This is how we agreed it with Guennadi, the argumentation was that it is
more consistent with 'data-lanes'. Also I think it is better to use plural
form right from the beginning, rather than introducing another 'clock-lanes'
property along 'clock-lane' when there would be a need to handle busses
with more than one clock lane in the future.
[...]
>> +/**
>> + * v4l2_of_parse_endpoint() - parse all endpoint node properties
>> + * @node: pointer to endpoint device_node
>> + * @endpoint: pointer to v4l2_of_endpoint data structure
>> + *
>> + * All properties are optional. If none are found, we don't set any flags.
>> + * This means the port has a static configuration and no properties have
>> + * to be specified explicitly.
>> + * If any properties that identify the bus as parallel are found and
>> + * slave-mode isn't set, we set V4L2_MBUS_MASTER. Similarly, if we recognise
>> + * the bus as serial CSI-2 and clock-noncontinuous isn't set, we set the
>> + * V4L2_MBUS_CSI2_CONTINUOUS_CLOCK flag.
>> + * The caller should hold a reference to @node.
>> + */
>> +void v4l2_of_parse_endpoint(const struct device_node *node,
>> + struct v4l2_of_endpoint *endpoint)
>> +{
>> + const struct device_node *port_node = of_get_parent(node);
>> + bool data_lanes_present = false;
>> +
>> + memset(endpoint, 0, sizeof(*endpoint));
>> +
>> + endpoint->local_node = node;
>> +
>> + /* Doesn't matter, whether the below two calls succeed */
>
> 'It doesn't matter whether the two calls below succeed. If they don't
> then the default value 0 is used.'
>
> At least, that's how I understand the code.
Yeah, it's more precise this way. I'll amend it, thanks!
>> + of_property_read_u32(port_node, "reg", &endpoint->port);
>> + of_property_read_u32(node, "reg", &endpoint->addr);
--
Regards,
Sylwester
--
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
