| Message ID | 20170214133941.GA8469@amd (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Pavel, On Tue, Feb 14, 2017 at 02:39:41PM +0100, Pavel Machek wrote: > From: Sakari Ailus <sakari.ailus@iki.fi> > > The function to parse CSI2 bus parameters was called > v4l2_of_parse_csi_bus(), rename it as v4l2_of_parse_csi2_bus() in > anticipation of CSI1/CCP2 support. > > Obtain data bus type from bus-type property. Only try parsing bus > specific properties in this case. > > Separate lane parsing from CSI-2 bus parameter parsing. The CSI-1 will > need these as well, separate them into a different > function. have_clk_lane and num_data_lanes arguments may be NULL; the > CSI-1 bus will have no use for them. > > Add support for parsing of CSI-1 and CCP2 bus related properties > documented in video-interfaces.txt. > > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi> > Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> > Signed-off-by: Pavel Machek <pavel@ucw.cz> > --- > drivers/media/v4l2-core/v4l2-of.c | 141 ++++++++++++++++++++++++++++++-------- > 1 file changed, 114 insertions(+), 27 deletions(-) > > diff --git a/drivers/media/v4l2-core/v4l2-of.c b/drivers/media/v4l2-core/v4l2-of.c > index 4f59f44..9ffe2d3 100644 > --- a/drivers/media/v4l2-core/v4l2-of.c > +++ b/drivers/media/v4l2-core/v4l2-of.c > @@ -20,64 +20,101 @@ > > #include <media/v4l2-of.h> > > -static int v4l2_of_parse_csi_bus(const struct device_node *node, > - struct v4l2_of_endpoint *endpoint) > +enum v4l2_of_bus_type { > + V4L2_OF_BUS_TYPE_CSI2 = 0, > + V4L2_OF_BUS_TYPE_PARALLEL, > + V4L2_OF_BUS_TYPE_CSI1, > + V4L2_OF_BUS_TYPE_CCP2, > +}; This no longer matches the bus-type values in DT bindings. How about: V4L2_OF_BUS_TYPE_GUESS, /* CSI-2 D-PHY, parallel or Bt.656 */ V4L2_OF_BUS_TYPE_CSI2_CPHY, V4L2_OF_BUS_TYPE_CSI1, V4L2_OF_BUS_TYPE_CCP2 > + > +static int v4l2_of_parse_lanes(const struct device_node *node, > + unsigned char *clock_lane, > + bool *have_clk_lane, > + unsigned char *data_lanes, > + bool *lane_polarities, > + unsigned short *__num_data_lanes, > + unsigned int max_data_lanes) > { > - struct v4l2_of_bus_mipi_csi2 *bus = &endpoint->bus.mipi_csi2; > struct property *prop; > - bool have_clk_lane = false; > - unsigned int flags = 0, lanes_used = 0; > + unsigned int lanes_used = 0; > + short num_data_lanes = 0; > u32 v; > > prop = of_find_property(node, "data-lanes", NULL); > if (prop) { > const __be32 *lane = NULL; > - unsigned int i; > > - for (i = 0; i < ARRAY_SIZE(bus->data_lanes); i++) { > + for (num_data_lanes = 0; num_data_lanes < max_data_lanes; > + num_data_lanes++) { > lane = of_prop_next_u32(prop, lane, &v); > if (!lane) > break; > + data_lanes[num_data_lanes] = v; > > if (lanes_used & BIT(v)) > pr_warn("%s: duplicated lane %u in data-lanes\n", > node->full_name, v); > lanes_used |= BIT(v); > - > - bus->data_lanes[i] = v; > } > - bus->num_data_lanes = i; > } > + if (__num_data_lanes) > + *__num_data_lanes = num_data_lanes; > > prop = of_find_property(node, "lane-polarities", NULL); > if (prop) { > const __be32 *polarity = NULL; > unsigned int i; > > - for (i = 0; i < ARRAY_SIZE(bus->lane_polarities); i++) { > + for (i = 0; i < 1 + max_data_lanes; i++) { > polarity = of_prop_next_u32(prop, polarity, &v); > if (!polarity) > break; > - bus->lane_polarities[i] = v; > + lane_polarities[i] = v; > } > > - if (i < 1 + bus->num_data_lanes /* clock + data */) { > + if (i < 1 + num_data_lanes /* clock + data */) { > pr_warn("%s: too few lane-polarities entries (need %u, got %u)\n", > - node->full_name, 1 + bus->num_data_lanes, i); > + node->full_name, 1 + num_data_lanes, i); > return -EINVAL; > } > } > > + if (have_clk_lane) > + *have_clk_lane = false; > + > if (!of_property_read_u32(node, "clock-lanes", &v)) { > + *clock_lane = v; > + if (have_clk_lane) > + *have_clk_lane = true; > + > if (lanes_used & BIT(v)) > pr_warn("%s: duplicated lane %u in clock-lanes\n", > node->full_name, v); > lanes_used |= BIT(v); > - > - bus->clock_lane = v; > - have_clk_lane = true; > } > > + return 0; > +} > + > +static int v4l2_of_parse_csi2_bus(const struct device_node *node, > + struct v4l2_of_endpoint *endpoint) > +{ > + struct v4l2_of_bus_mipi_csi2 *bus = &endpoint->bus.mipi_csi2; > + bool have_clk_lane = false; > + unsigned int flags = 0; > + int rval; > + u32 v; > + > + rval = v4l2_of_parse_lanes(node, &bus->clock_lane, &have_clk_lane, > + bus->data_lanes, bus->lane_polarities, > + &bus->num_data_lanes, > + ARRAY_SIZE(bus->data_lanes)); > + if (rval) > + return rval; > + > + BUILD_BUG_ON(1 + ARRAY_SIZE(bus->data_lanes) > + != ARRAY_SIZE(bus->lane_polarities)); > + > if (of_get_property(node, "clock-noncontinuous", &v)) > flags |= V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK; > else if (have_clk_lane || bus->num_data_lanes > 0) > @@ -139,6 +176,35 @@ static void v4l2_of_parse_parallel_bus(const struct device_node *node, > > } > > +void v4l2_of_parse_csi1_bus(const struct device_node *node, > + struct v4l2_of_endpoint *endpoint, > + enum v4l2_of_bus_type bus_type) > +{ > + struct v4l2_of_bus_mipi_csi1 *bus = &endpoint->bus.mipi_csi1; > + u32 v; > + > + v4l2_of_parse_lanes(node, &bus->clock_lane, NULL, > + &bus->data_lane, bus->lane_polarity, > + NULL, 1); > + > + if (!of_property_read_u32(node, "clock-inv", &v)) > + bus->clock_inv = v; > + > + if (!of_property_read_u32(node, "strobe", &v)) > + bus->strobe = v; > + > + if (!of_property_read_u32(node, "data-lane", &v)) > + bus->data_lane = v; > + > + if (!of_property_read_u32(node, "clock-lane", &v)) > + bus->clock_lane = v; > + > + if (bus_type == V4L2_OF_BUS_TYPE_CSI1) > + endpoint->bus_type = V4L2_MBUS_CSI1; > + else > + endpoint->bus_type = V4L2_MBUS_CCP2; > +} > + > /** > * v4l2_of_parse_endpoint() - parse all endpoint node properties > * @node: pointer to endpoint device_node > @@ -162,6 +228,7 @@ static void v4l2_of_parse_parallel_bus(const struct device_node *node, > int v4l2_of_parse_endpoint(const struct device_node *node, > struct v4l2_of_endpoint *endpoint) > { > + u32 bus_type; You could assign bus_type as 0 here... > int rval; > > of_graph_parse_endpoint(node, &endpoint->base); > @@ -169,17 +236,37 @@ int v4l2_of_parse_endpoint(const struct device_node *node, > memset(&endpoint->bus_type, 0, sizeof(*endpoint) - > offsetof(typeof(*endpoint), bus_type)); > > - rval = v4l2_of_parse_csi_bus(node, endpoint); > - if (rval) > - return rval; > - /* > - * Parse the parallel video bus properties only if none > - * of the MIPI CSI-2 specific properties were found. > - */ > - if (endpoint->bus.mipi_csi2.flags == 0) > - v4l2_of_parse_parallel_bus(node, endpoint); > + rval = of_property_read_u32(node, "bus-type", &bus_type); > + if (rval < 0) { ...and remove the check. What's below would be done in the _GUESS case. > + endpoint->bus_type = 0; > + rval = v4l2_of_parse_csi2_bus(node, endpoint); > + if (rval) > + return rval; > + /* > + * Parse the parallel video bus properties only if none > + * of the MIPI CSI-2 specific properties were found. > + */ > + if (endpoint->bus.mipi_csi2.flags == 0) > + v4l2_of_parse_parallel_bus(node, endpoint); > + > + return 0; > + } > > - return 0; > + switch (bus_type) { > + case V4L2_OF_BUS_TYPE_CSI2: > + return v4l2_of_parse_csi2_bus(node, endpoint); > + case V4L2_OF_BUS_TYPE_PARALLEL: > + v4l2_of_parse_parallel_bus(node, endpoint); > + return 0; And you can remove CSI2 and PARALLEL cases. > + case V4L2_OF_BUS_TYPE_CSI1: > + case V4L2_OF_BUS_TYPE_CCP2: > + v4l2_of_parse_csi1_bus(node, endpoint, bus_type); > + return 0; > + default: > + pr_warn("bad bus-type %u, device_node \"%s\"\n", > + bus_type, node->full_name); > + return -EINVAL; > + } > } > EXPORT_SYMBOL(v4l2_of_parse_endpoint); >
Hi Pavel, On Tue, Feb 14, 2017 at 02:39:41PM +0100, Pavel Machek wrote: > From: Sakari Ailus <sakari.ailus@iki.fi> > > The function to parse CSI2 bus parameters was called > v4l2_of_parse_csi_bus(), rename it as v4l2_of_parse_csi2_bus() in > anticipation of CSI1/CCP2 support. > > Obtain data bus type from bus-type property. Only try parsing bus > specific properties in this case. > > Separate lane parsing from CSI-2 bus parameter parsing. The CSI-1 will > need these as well, separate them into a different > function. have_clk_lane and num_data_lanes arguments may be NULL; the > CSI-1 bus will have no use for them. > > Add support for parsing of CSI-1 and CCP2 bus related properties > documented in video-interfaces.txt. One more thing: this conflicts badly with the V4L2 fwnode patchset. Assuming things go well and that can be merged somewhat soonish, can I take this and rebase it on the fwnode set? The two first patches in the set look pretty good to me.
Hi!
> And you can remove CSI2 and PARALLEL cases.
Ok, that's all easy enough.
Thanks,
Pavel
Hi! > On Tue, Feb 14, 2017 at 02:39:41PM +0100, Pavel Machek wrote: > > From: Sakari Ailus <sakari.ailus@iki.fi> > > > > The function to parse CSI2 bus parameters was called > > v4l2_of_parse_csi_bus(), rename it as v4l2_of_parse_csi2_bus() in > > anticipation of CSI1/CCP2 support. > > > > Obtain data bus type from bus-type property. Only try parsing bus > > specific properties in this case. > > > > Separate lane parsing from CSI-2 bus parameter parsing. The CSI-1 will > > need these as well, separate them into a different > > function. have_clk_lane and num_data_lanes arguments may be NULL; the > > CSI-1 bus will have no use for them. > > > > Add support for parsing of CSI-1 and CCP2 bus related properties > > documented in video-interfaces.txt. > > One more thing: this conflicts badly with the V4L2 fwnode patchset. > > Assuming things go well and that can be merged somewhat soonish, can I take > this and rebase it on the fwnode set? The two first patches in the set look > pretty good to me. Actually, I'd say that first four patches should be ready. Feel free to take them/rebase them/etc. I can then continue working on the rest.... Best regards, Pavel
diff --git a/drivers/media/v4l2-core/v4l2-of.c b/drivers/media/v4l2-core/v4l2-of.c index 4f59f44..9ffe2d3 100644 --- a/drivers/media/v4l2-core/v4l2-of.c +++ b/drivers/media/v4l2-core/v4l2-of.c @@ -20,64 +20,101 @@ #include <media/v4l2-of.h> -static int v4l2_of_parse_csi_bus(const struct device_node *node, - struct v4l2_of_endpoint *endpoint) +enum v4l2_of_bus_type { + V4L2_OF_BUS_TYPE_CSI2 = 0, + V4L2_OF_BUS_TYPE_PARALLEL, + V4L2_OF_BUS_TYPE_CSI1, + V4L2_OF_BUS_TYPE_CCP2, +}; + +static int v4l2_of_parse_lanes(const struct device_node *node, + unsigned char *clock_lane, + bool *have_clk_lane, + unsigned char *data_lanes, + bool *lane_polarities, + unsigned short *__num_data_lanes, + unsigned int max_data_lanes) { - struct v4l2_of_bus_mipi_csi2 *bus = &endpoint->bus.mipi_csi2; struct property *prop; - bool have_clk_lane = false; - unsigned int flags = 0, lanes_used = 0; + unsigned int lanes_used = 0; + short num_data_lanes = 0; u32 v; prop = of_find_property(node, "data-lanes", NULL); if (prop) { const __be32 *lane = NULL; - unsigned int i; - for (i = 0; i < ARRAY_SIZE(bus->data_lanes); i++) { + for (num_data_lanes = 0; num_data_lanes < max_data_lanes; + num_data_lanes++) { lane = of_prop_next_u32(prop, lane, &v); if (!lane) break; + data_lanes[num_data_lanes] = v; if (lanes_used & BIT(v)) pr_warn("%s: duplicated lane %u in data-lanes\n", node->full_name, v); lanes_used |= BIT(v); - - bus->data_lanes[i] = v; } - bus->num_data_lanes = i; } + if (__num_data_lanes) + *__num_data_lanes = num_data_lanes; prop = of_find_property(node, "lane-polarities", NULL); if (prop) { const __be32 *polarity = NULL; unsigned int i; - for (i = 0; i < ARRAY_SIZE(bus->lane_polarities); i++) { + for (i = 0; i < 1 + max_data_lanes; i++) { polarity = of_prop_next_u32(prop, polarity, &v); if (!polarity) break; - bus->lane_polarities[i] = v; + lane_polarities[i] = v; } - if (i < 1 + bus->num_data_lanes /* clock + data */) { + if (i < 1 + num_data_lanes /* clock + data */) { pr_warn("%s: too few lane-polarities entries (need %u, got %u)\n", - node->full_name, 1 + bus->num_data_lanes, i); + node->full_name, 1 + num_data_lanes, i); return -EINVAL; } } + if (have_clk_lane) + *have_clk_lane = false; + if (!of_property_read_u32(node, "clock-lanes", &v)) { + *clock_lane = v; + if (have_clk_lane) + *have_clk_lane = true; + if (lanes_used & BIT(v)) pr_warn("%s: duplicated lane %u in clock-lanes\n", node->full_name, v); lanes_used |= BIT(v); - - bus->clock_lane = v; - have_clk_lane = true; } + return 0; +} + +static int v4l2_of_parse_csi2_bus(const struct device_node *node, + struct v4l2_of_endpoint *endpoint) +{ + struct v4l2_of_bus_mipi_csi2 *bus = &endpoint->bus.mipi_csi2; + bool have_clk_lane = false; + unsigned int flags = 0; + int rval; + u32 v; + + rval = v4l2_of_parse_lanes(node, &bus->clock_lane, &have_clk_lane, + bus->data_lanes, bus->lane_polarities, + &bus->num_data_lanes, + ARRAY_SIZE(bus->data_lanes)); + if (rval) + return rval; + + BUILD_BUG_ON(1 + ARRAY_SIZE(bus->data_lanes) + != ARRAY_SIZE(bus->lane_polarities)); + if (of_get_property(node, "clock-noncontinuous", &v)) flags |= V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK; else if (have_clk_lane || bus->num_data_lanes > 0) @@ -139,6 +176,35 @@ static void v4l2_of_parse_parallel_bus(const struct device_node *node, } +void v4l2_of_parse_csi1_bus(const struct device_node *node, + struct v4l2_of_endpoint *endpoint, + enum v4l2_of_bus_type bus_type) +{ + struct v4l2_of_bus_mipi_csi1 *bus = &endpoint->bus.mipi_csi1; + u32 v; + + v4l2_of_parse_lanes(node, &bus->clock_lane, NULL, + &bus->data_lane, bus->lane_polarity, + NULL, 1); + + if (!of_property_read_u32(node, "clock-inv", &v)) + bus->clock_inv = v; + + if (!of_property_read_u32(node, "strobe", &v)) + bus->strobe = v; + + if (!of_property_read_u32(node, "data-lane", &v)) + bus->data_lane = v; + + if (!of_property_read_u32(node, "clock-lane", &v)) + bus->clock_lane = v; + + if (bus_type == V4L2_OF_BUS_TYPE_CSI1) + endpoint->bus_type = V4L2_MBUS_CSI1; + else + endpoint->bus_type = V4L2_MBUS_CCP2; +} + /** * v4l2_of_parse_endpoint() - parse all endpoint node properties * @node: pointer to endpoint device_node @@ -162,6 +228,7 @@ static void v4l2_of_parse_parallel_bus(const struct device_node *node, int v4l2_of_parse_endpoint(const struct device_node *node, struct v4l2_of_endpoint *endpoint) { + u32 bus_type; int rval; of_graph_parse_endpoint(node, &endpoint->base); @@ -169,17 +236,37 @@ int v4l2_of_parse_endpoint(const struct device_node *node, memset(&endpoint->bus_type, 0, sizeof(*endpoint) - offsetof(typeof(*endpoint), bus_type)); - rval = v4l2_of_parse_csi_bus(node, endpoint); - if (rval) - return rval; - /* - * Parse the parallel video bus properties only if none - * of the MIPI CSI-2 specific properties were found. - */ - if (endpoint->bus.mipi_csi2.flags == 0) - v4l2_of_parse_parallel_bus(node, endpoint); + rval = of_property_read_u32(node, "bus-type", &bus_type); + if (rval < 0) { + endpoint->bus_type = 0; + rval = v4l2_of_parse_csi2_bus(node, endpoint); + if (rval) + return rval; + /* + * Parse the parallel video bus properties only if none + * of the MIPI CSI-2 specific properties were found. + */ + if (endpoint->bus.mipi_csi2.flags == 0) + v4l2_of_parse_parallel_bus(node, endpoint); + + return 0; + } - return 0; + switch (bus_type) { + case V4L2_OF_BUS_TYPE_CSI2: + return v4l2_of_parse_csi2_bus(node, endpoint); + case V4L2_OF_BUS_TYPE_PARALLEL: + v4l2_of_parse_parallel_bus(node, endpoint); + return 0; + case V4L2_OF_BUS_TYPE_CSI1: + case V4L2_OF_BUS_TYPE_CCP2: + v4l2_of_parse_csi1_bus(node, endpoint, bus_type); + return 0; + default: + pr_warn("bad bus-type %u, device_node \"%s\"\n", + bus_type, node->full_name); + return -EINVAL; + } } EXPORT_SYMBOL(v4l2_of_parse_endpoint);