| Message ID | 20170126131259.5621-1-niklas.soderlund+renesas@ragnatech.se (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Niklas, On Thu, Jan 26, 2017 at 2:12 PM, Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> wrote: > diff --git a/drivers/media/v4l2-core/v4l2-of.c b/drivers/media/v4l2-core/v4l2-of.c > index 93b33681776c..1042db6bb996 100644 > --- a/drivers/media/v4l2-core/v4l2-of.c > +++ b/drivers/media/v4l2-core/v4l2-of.c > @@ -32,12 +32,19 @@ static int v4l2_of_parse_csi_bus(const struct device_node *node, > prop = of_find_property(node, "data-lanes", NULL); > if (prop) { > const __be32 *lane = NULL; > - unsigned int i; > + unsigned int i, n; Not "j"? > for (i = 0; i < ARRAY_SIZE(bus->data_lanes); i++) { > lane = of_prop_next_u32(prop, lane, &v); > if (!lane) > break; > + for (n = 0; n < i; n++) { I'm not used seeing for loops with an index named "n", and limit named "i" ;-) > + if (bus->data_lanes[n] == v) { > + pr_warn("%s: duplicated lane %u in data-lanes\n", > + node->full_name, v); > + return -EINVAL; > + } > + } > bus->data_lanes[i] = v; > } > bus->num_data_lanes = i; > @@ -63,6 +70,15 @@ static int v4l2_of_parse_csi_bus(const struct device_node *node, > } > > if (!of_property_read_u32(node, "clock-lanes", &v)) { > + unsigned int n; Likewise. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Niklas, On Thu, Jan 26, 2017 at 02:12:59PM +0100, Niklas Söderlund wrote: > All lines in data-lanes and clock-lanes properties must be unique. > Instead of drivers checking for this add it to the generic parser. > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > --- > drivers/media/v4l2-core/v4l2-of.c | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/v4l2-core/v4l2-of.c b/drivers/media/v4l2-core/v4l2-of.c > index 93b33681776c..1042db6bb996 100644 > --- a/drivers/media/v4l2-core/v4l2-of.c > +++ b/drivers/media/v4l2-core/v4l2-of.c > @@ -32,12 +32,19 @@ static int v4l2_of_parse_csi_bus(const struct device_node *node, > prop = of_find_property(node, "data-lanes", NULL); > if (prop) { > const __be32 *lane = NULL; > - unsigned int i; > + unsigned int i, n; > > for (i = 0; i < ARRAY_SIZE(bus->data_lanes); i++) { > lane = of_prop_next_u32(prop, lane, &v); > if (!lane) > break; > + for (n = 0; n < i; n++) { > + if (bus->data_lanes[n] == v) { > + pr_warn("%s: duplicated lane %u in data-lanes\n", > + node->full_name, v); > + return -EINVAL; > + } > + } In some cases it's just the number of lanes that matter, not their order, as the hardware cannot reorder them. I might still just print a warning but not return an error. Same below. Although then hardware that actually can reorder the lanes might have issues if such a bug exists in DTB. Presumably the DTB would have been tested at some point though. > bus->data_lanes[i] = v; > } > bus->num_data_lanes = i; > @@ -63,6 +70,15 @@ static int v4l2_of_parse_csi_bus(const struct device_node *node, > } > > if (!of_property_read_u32(node, "clock-lanes", &v)) { > + unsigned int n; > + > + for (n = 0; n < bus->num_data_lanes; n++) { > + if (bus->data_lanes[n] == v) { > + pr_warn("%s: duplicated lane %u in clock-lanes\n", > + node->full_name, v); > + return -EINVAL; > + } > + } > bus->clock_lane = v; > have_clk_lane = true; > }
Hi Niklas, Thank you for the patch. On Thursday 26 Jan 2017 14:12:59 Niklas Söderlund wrote: > All lines in data-lanes and clock-lanes properties must be unique. > Instead of drivers checking for this add it to the generic parser. > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > --- > drivers/media/v4l2-core/v4l2-of.c | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/v4l2-core/v4l2-of.c > b/drivers/media/v4l2-core/v4l2-of.c index 93b33681776c..1042db6bb996 100644 > --- a/drivers/media/v4l2-core/v4l2-of.c > +++ b/drivers/media/v4l2-core/v4l2-of.c > @@ -32,12 +32,19 @@ static int v4l2_of_parse_csi_bus(const struct > device_node *node, prop = of_find_property(node, "data-lanes", NULL); > if (prop) { > const __be32 *lane = NULL; > - unsigned int i; > + unsigned int i, n; > > for (i = 0; i < ARRAY_SIZE(bus->data_lanes); i++) { > lane = of_prop_next_u32(prop, lane, &v); > if (!lane) > break; > + for (n = 0; n < i; n++) { > + if (bus->data_lanes[n] == v) { > + pr_warn("%s: duplicated lane %u in data-lanes\n", > + node->full_name, v); > + return -EINVAL; > + } If you used a bitmask to store the already used lanes you could avoid the nested loops. Apart from that, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + } > bus->data_lanes[i] = v; > } > bus->num_data_lanes = i; > @@ -63,6 +70,15 @@ static int v4l2_of_parse_csi_bus(const struct device_node > *node, } > > if (!of_property_read_u32(node, "clock-lanes", &v)) { > + unsigned int n; > + > + for (n = 0; n < bus->num_data_lanes; n++) { > + if (bus->data_lanes[n] == v) { > + pr_warn("%s: duplicated lane %u in clock- lanes\n", > + node->full_name, v); > + return -EINVAL; > + } > + } > bus->clock_lane = v; > have_clk_lane = true; > }
diff --git a/drivers/media/v4l2-core/v4l2-of.c b/drivers/media/v4l2-core/v4l2-of.c index 93b33681776c..1042db6bb996 100644 --- a/drivers/media/v4l2-core/v4l2-of.c +++ b/drivers/media/v4l2-core/v4l2-of.c @@ -32,12 +32,19 @@ static int v4l2_of_parse_csi_bus(const struct device_node *node, prop = of_find_property(node, "data-lanes", NULL); if (prop) { const __be32 *lane = NULL; - unsigned int i; + unsigned int i, n; for (i = 0; i < ARRAY_SIZE(bus->data_lanes); i++) { lane = of_prop_next_u32(prop, lane, &v); if (!lane) break; + for (n = 0; n < i; n++) { + if (bus->data_lanes[n] == v) { + pr_warn("%s: duplicated lane %u in data-lanes\n", + node->full_name, v); + return -EINVAL; + } + } bus->data_lanes[i] = v; } bus->num_data_lanes = i; @@ -63,6 +70,15 @@ static int v4l2_of_parse_csi_bus(const struct device_node *node, } if (!of_property_read_u32(node, "clock-lanes", &v)) { + unsigned int n; + + for (n = 0; n < bus->num_data_lanes; n++) { + if (bus->data_lanes[n] == v) { + pr_warn("%s: duplicated lane %u in clock-lanes\n", + node->full_name, v); + return -EINVAL; + } + } bus->clock_lane = v; have_clk_lane = true; }
All lines in data-lanes and clock-lanes properties must be unique. Instead of drivers checking for this add it to the generic parser. Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> --- drivers/media/v4l2-core/v4l2-of.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)