| Message ID | 20241119221249.539610-3-niklas.soderlund+renesas@ragnatech.se (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: v4l: fwnode: Add support for CSI-2 C-PHY line orders | expand |
Hejssan, On Tue, Nov 19, 2024 at 11:12:47PM +0100, Niklas Söderlund wrote: > Extend the fwnode parsing to validate and fill in the CSI-2 C-PHY > line-orders order properties as defined in MIPI Discovery and > Configuration (DisCo) Specification for Imaging. > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > --- > drivers/media/v4l2-core/v4l2-fwnode.c | 56 ++++++++++++++++++++++++++- > include/media/v4l2-mediabus.h | 21 ++++++++++ > 2 files changed, 76 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c > index f19c8adf2c61..b8b2b7fb685e 100644 > --- a/drivers/media/v4l2-core/v4l2-fwnode.c > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c > @@ -127,7 +127,7 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct fwnode_handle *fwnode, > { > struct v4l2_mbus_config_mipi_csi2 *bus = &vep->bus.mipi_csi2; > bool have_clk_lane = false, have_data_lanes = false, > - have_lane_polarities = false; > + have_lane_polarities = false, have_line_orders = false; > unsigned int flags = 0, lanes_used = 0; > u32 array[1 + V4L2_MBUS_CSI2_MAX_DATA_LANES]; > u32 clock_lane = 0; > @@ -197,6 +197,17 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct fwnode_handle *fwnode, > have_lane_polarities = true; > } > > + rval = fwnode_property_count_u32(fwnode, "line-orders"); > + if (rval > 0) { > + if (rval != num_data_lanes) { > + pr_warn("invalid number of line-orders entries (need %u, got %u)\n", > + num_data_lanes, rval); > + return -EINVAL; > + } > + > + have_line_orders = true; > + } > + > if (!fwnode_property_read_u32(fwnode, "clock-lanes", &v)) { > clock_lane = v; > pr_debug("clock lane position %u\n", v); > @@ -250,6 +261,49 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct fwnode_handle *fwnode, > } else { > pr_debug("no lane polarities defined, assuming not inverted\n"); > } > + > + if (have_line_orders) { > + fwnode_property_read_u32_array(fwnode, > + "line-orders", array, > + num_data_lanes); > + > + for (i = 0; i < num_data_lanes; i++) { > + const char *order; > + > + switch (array[i]) { > + case 0: > + order = "ABC"; > + break; > + case 1: > + order = "ACB"; > + break; > + case 2: > + order = "BAC"; > + break; > + case 3: > + order = "BCA"; > + break; > + case 4: > + order = "CAB"; > + break; > + case 5: > + order = "CBA"; > + break; Please use an array instead. > + default: > + pr_warn("lane %u invalid line-order assuming ABC (got %u)\n", > + i, array[i]); > + bus->line_orders[i] = V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ABC; > + continue; > + } > + bus->line_orders[i] = array[i]; > + pr_debug("lane %u line order %s", i, order); > + } > + } else { > + for (i = 0; i < num_data_lanes; i++) > + bus->line_orders[i] = V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ABC; A few lines could be wrapped above. > + > + pr_debug("no line orders defined, assuming ABC\n"); > + } > } > > return 0; > diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h > index 5bce6e423e94..e7f019f68c8d 100644 > --- a/include/media/v4l2-mediabus.h > +++ b/include/media/v4l2-mediabus.h > @@ -73,6 +73,24 @@ > > #define V4L2_MBUS_CSI2_MAX_DATA_LANES 8 > > +/** > + * enum v4l2_mbus_csi2_cphy_line_orders_type - CSI-2 C-PHY line order > + * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ABC: C-PHY line order ABC (default) > + * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ACB: C-PHY line order ACB > + * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_BAC: C-PHY line order BAC > + * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_BCA: C-PHY line order BCA > + * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_CAB: C-PHY line order CAB > + * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_CBA: C-PHY line order CBA > + */ > +enum v4l2_mbus_csi2_cphy_line_orders_type { > + V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ABC, > + V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ACB, > + V4L2_MBUS_CSI2_CPHY_LINE_ORDER_BAC, > + V4L2_MBUS_CSI2_CPHY_LINE_ORDER_BCA, > + V4L2_MBUS_CSI2_CPHY_LINE_ORDER_CAB, > + V4L2_MBUS_CSI2_CPHY_LINE_ORDER_CBA, > +}; > + > /** > * struct v4l2_mbus_config_mipi_csi2 - MIPI CSI-2 data bus configuration > * @flags: media bus (V4L2_MBUS_*) flags > @@ -81,6 +99,8 @@ > * @num_data_lanes: number of data lanes > * @lane_polarities: polarity of the lanes. The order is the same of > * the physical lanes. > + * @line_orders: line order of the data lanes. The order is the same of the > + * physical lanes. > */ > struct v4l2_mbus_config_mipi_csi2 { > unsigned int flags; > @@ -88,6 +108,7 @@ struct v4l2_mbus_config_mipi_csi2 { > unsigned char clock_lane; > unsigned char num_data_lanes; > bool lane_polarities[1 + V4L2_MBUS_CSI2_MAX_DATA_LANES]; > + enum v4l2_mbus_csi2_cphy_line_orders_type line_orders[V4L2_MBUS_CSI2_MAX_DATA_LANES]; > }; > > /**
Hello Sakari, On 2024-11-20 08:10:42 +0000, Sakari Ailus wrote: > Hejssan, > > On Tue, Nov 19, 2024 at 11:12:47PM +0100, Niklas Söderlund wrote: > > Extend the fwnode parsing to validate and fill in the CSI-2 C-PHY > > line-orders order properties as defined in MIPI Discovery and > > Configuration (DisCo) Specification for Imaging. > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > > --- > > drivers/media/v4l2-core/v4l2-fwnode.c | 56 ++++++++++++++++++++++++++- > > include/media/v4l2-mediabus.h | 21 ++++++++++ > > 2 files changed, 76 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c > > index f19c8adf2c61..b8b2b7fb685e 100644 > > --- a/drivers/media/v4l2-core/v4l2-fwnode.c > > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c > > @@ -127,7 +127,7 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct fwnode_handle *fwnode, > > { > > struct v4l2_mbus_config_mipi_csi2 *bus = &vep->bus.mipi_csi2; > > bool have_clk_lane = false, have_data_lanes = false, > > - have_lane_polarities = false; > > + have_lane_polarities = false, have_line_orders = false; > > unsigned int flags = 0, lanes_used = 0; > > u32 array[1 + V4L2_MBUS_CSI2_MAX_DATA_LANES]; > > u32 clock_lane = 0; > > @@ -197,6 +197,17 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct fwnode_handle *fwnode, > > have_lane_polarities = true; > > } > > > > + rval = fwnode_property_count_u32(fwnode, "line-orders"); > > + if (rval > 0) { > > + if (rval != num_data_lanes) { > > + pr_warn("invalid number of line-orders entries (need %u, got %u)\n", > > + num_data_lanes, rval); > > + return -EINVAL; > > + } > > + > > + have_line_orders = true; > > + } > > + > > if (!fwnode_property_read_u32(fwnode, "clock-lanes", &v)) { > > clock_lane = v; > > pr_debug("clock lane position %u\n", v); > > @@ -250,6 +261,49 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct fwnode_handle *fwnode, > > } else { > > pr_debug("no lane polarities defined, assuming not inverted\n"); > > } > > + > > + if (have_line_orders) { > > + fwnode_property_read_u32_array(fwnode, > > + "line-orders", array, > > + num_data_lanes); > > + > > + for (i = 0; i < num_data_lanes; i++) { > > + const char *order; > > + > > + switch (array[i]) { > > + case 0: > > + order = "ABC"; > > + break; > > + case 1: > > + order = "ACB"; > > + break; > > + case 2: > > + order = "BAC"; > > + break; > > + case 3: > > + order = "BCA"; > > + break; > > + case 4: > > + order = "CAB"; > > + break; > > + case 5: > > + order = "CBA"; > > + break; > > Please use an array instead. > > > + default: > > + pr_warn("lane %u invalid line-order assuming ABC (got %u)\n", > > + i, array[i]); > > + bus->line_orders[i] = V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ABC; > > + continue; > > + } > > + bus->line_orders[i] = array[i]; > > + pr_debug("lane %u line order %s", i, order); > > + } > > + } else { > > + for (i = 0; i < num_data_lanes; i++) > > + bus->line_orders[i] = V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ABC; > > A few lines could be wrapped above. I'm not sure I understand this comment. Do you mean I could loop over num_data_lanes and initialize all lines to ABC before checking have_line_orders and that way avoid having to loop here and set the default ABC if we are out-of bounds in the switch? > > > + > > + pr_debug("no line orders defined, assuming ABC\n"); > > + } > > } > > > > return 0; > > diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h > > index 5bce6e423e94..e7f019f68c8d 100644 > > --- a/include/media/v4l2-mediabus.h > > +++ b/include/media/v4l2-mediabus.h > > @@ -73,6 +73,24 @@ > > > > #define V4L2_MBUS_CSI2_MAX_DATA_LANES 8 > > > > +/** > > + * enum v4l2_mbus_csi2_cphy_line_orders_type - CSI-2 C-PHY line order > > + * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ABC: C-PHY line order ABC (default) > > + * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ACB: C-PHY line order ACB > > + * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_BAC: C-PHY line order BAC > > + * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_BCA: C-PHY line order BCA > > + * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_CAB: C-PHY line order CAB > > + * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_CBA: C-PHY line order CBA > > + */ > > +enum v4l2_mbus_csi2_cphy_line_orders_type { > > + V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ABC, > > + V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ACB, > > + V4L2_MBUS_CSI2_CPHY_LINE_ORDER_BAC, > > + V4L2_MBUS_CSI2_CPHY_LINE_ORDER_BCA, > > + V4L2_MBUS_CSI2_CPHY_LINE_ORDER_CAB, > > + V4L2_MBUS_CSI2_CPHY_LINE_ORDER_CBA, > > +}; > > + > > /** > > * struct v4l2_mbus_config_mipi_csi2 - MIPI CSI-2 data bus configuration > > * @flags: media bus (V4L2_MBUS_*) flags > > @@ -81,6 +99,8 @@ > > * @num_data_lanes: number of data lanes > > * @lane_polarities: polarity of the lanes. The order is the same of > > * the physical lanes. > > + * @line_orders: line order of the data lanes. The order is the same of the > > + * physical lanes. > > */ > > struct v4l2_mbus_config_mipi_csi2 { > > unsigned int flags; > > @@ -88,6 +108,7 @@ struct v4l2_mbus_config_mipi_csi2 { > > unsigned char clock_lane; > > unsigned char num_data_lanes; > > bool lane_polarities[1 + V4L2_MBUS_CSI2_MAX_DATA_LANES]; > > + enum v4l2_mbus_csi2_cphy_line_orders_type line_orders[V4L2_MBUS_CSI2_MAX_DATA_LANES]; > > }; > > > > /** > > -- > Med vänliga hälsningar, > > Sakari Ailus
Hejssan, Niklas! On Wed, Nov 20, 2024 at 10:50:30AM +0100, Niklas Söderlund wrote: > Hello Sakari, > > On 2024-11-20 08:10:42 +0000, Sakari Ailus wrote: > > Hejssan, > > > > On Tue, Nov 19, 2024 at 11:12:47PM +0100, Niklas Söderlund wrote: > > > Extend the fwnode parsing to validate and fill in the CSI-2 C-PHY > > > line-orders order properties as defined in MIPI Discovery and > > > Configuration (DisCo) Specification for Imaging. > > > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > > > --- > > > drivers/media/v4l2-core/v4l2-fwnode.c | 56 ++++++++++++++++++++++++++- > > > include/media/v4l2-mediabus.h | 21 ++++++++++ > > > 2 files changed, 76 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c > > > index f19c8adf2c61..b8b2b7fb685e 100644 > > > --- a/drivers/media/v4l2-core/v4l2-fwnode.c > > > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c > > > @@ -127,7 +127,7 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct fwnode_handle *fwnode, > > > { > > > struct v4l2_mbus_config_mipi_csi2 *bus = &vep->bus.mipi_csi2; > > > bool have_clk_lane = false, have_data_lanes = false, > > > - have_lane_polarities = false; > > > + have_lane_polarities = false, have_line_orders = false; > > > unsigned int flags = 0, lanes_used = 0; > > > u32 array[1 + V4L2_MBUS_CSI2_MAX_DATA_LANES]; > > > u32 clock_lane = 0; > > > @@ -197,6 +197,17 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct fwnode_handle *fwnode, > > > have_lane_polarities = true; > > > } > > > > > > + rval = fwnode_property_count_u32(fwnode, "line-orders"); > > > + if (rval > 0) { > > > + if (rval != num_data_lanes) { > > > + pr_warn("invalid number of line-orders entries (need %u, got %u)\n", > > > + num_data_lanes, rval); > > > + return -EINVAL; > > > + } > > > + > > > + have_line_orders = true; > > > + } > > > + > > > if (!fwnode_property_read_u32(fwnode, "clock-lanes", &v)) { > > > clock_lane = v; > > > pr_debug("clock lane position %u\n", v); > > > @@ -250,6 +261,49 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct fwnode_handle *fwnode, > > > } else { > > > pr_debug("no lane polarities defined, assuming not inverted\n"); > > > } > > > + > > > + if (have_line_orders) { > > > + fwnode_property_read_u32_array(fwnode, > > > + "line-orders", array, > > > + num_data_lanes); > > > + > > > + for (i = 0; i < num_data_lanes; i++) { > > > + const char *order; > > > + > > > + switch (array[i]) { > > > + case 0: > > > + order = "ABC"; > > > + break; > > > + case 1: > > > + order = "ACB"; > > > + break; > > > + case 2: > > > + order = "BAC"; > > > + break; > > > + case 3: > > > + order = "BCA"; > > > + break; > > > + case 4: > > > + order = "CAB"; > > > + break; > > > + case 5: > > > + order = "CBA"; > > > + break; > > > > Please use an array instead. > > > > > + default: > > > + pr_warn("lane %u invalid line-order assuming ABC (got %u)\n", > > > + i, array[i]); > > > + bus->line_orders[i] = V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ABC; > > > + continue; > > > + } > > > + bus->line_orders[i] = array[i]; > > > + pr_debug("lane %u line order %s", i, order); > > > + } > > > + } else { > > > + for (i = 0; i < num_data_lanes; i++) > > > + bus->line_orders[i] = V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ABC; > > > > A few lines could be wrapped above. > > I'm not sure I understand this comment. Do you mean I could loop over > num_data_lanes and initialize all lines to ABC before checking > have_line_orders and that way avoid having to loop here and set the > default ABC if we are out-of bounds in the switch? No, just that you'd wrap lines that are over 80 characters per line, unless there's some tangible reason to have them like that.
diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c index f19c8adf2c61..b8b2b7fb685e 100644 --- a/drivers/media/v4l2-core/v4l2-fwnode.c +++ b/drivers/media/v4l2-core/v4l2-fwnode.c @@ -127,7 +127,7 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct fwnode_handle *fwnode, { struct v4l2_mbus_config_mipi_csi2 *bus = &vep->bus.mipi_csi2; bool have_clk_lane = false, have_data_lanes = false, - have_lane_polarities = false; + have_lane_polarities = false, have_line_orders = false; unsigned int flags = 0, lanes_used = 0; u32 array[1 + V4L2_MBUS_CSI2_MAX_DATA_LANES]; u32 clock_lane = 0; @@ -197,6 +197,17 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct fwnode_handle *fwnode, have_lane_polarities = true; } + rval = fwnode_property_count_u32(fwnode, "line-orders"); + if (rval > 0) { + if (rval != num_data_lanes) { + pr_warn("invalid number of line-orders entries (need %u, got %u)\n", + num_data_lanes, rval); + return -EINVAL; + } + + have_line_orders = true; + } + if (!fwnode_property_read_u32(fwnode, "clock-lanes", &v)) { clock_lane = v; pr_debug("clock lane position %u\n", v); @@ -250,6 +261,49 @@ static int v4l2_fwnode_endpoint_parse_csi2_bus(struct fwnode_handle *fwnode, } else { pr_debug("no lane polarities defined, assuming not inverted\n"); } + + if (have_line_orders) { + fwnode_property_read_u32_array(fwnode, + "line-orders", array, + num_data_lanes); + + for (i = 0; i < num_data_lanes; i++) { + const char *order; + + switch (array[i]) { + case 0: + order = "ABC"; + break; + case 1: + order = "ACB"; + break; + case 2: + order = "BAC"; + break; + case 3: + order = "BCA"; + break; + case 4: + order = "CAB"; + break; + case 5: + order = "CBA"; + break; + default: + pr_warn("lane %u invalid line-order assuming ABC (got %u)\n", + i, array[i]); + bus->line_orders[i] = V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ABC; + continue; + } + bus->line_orders[i] = array[i]; + pr_debug("lane %u line order %s", i, order); + } + } else { + for (i = 0; i < num_data_lanes; i++) + bus->line_orders[i] = V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ABC; + + pr_debug("no line orders defined, assuming ABC\n"); + } } return 0; diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h index 5bce6e423e94..e7f019f68c8d 100644 --- a/include/media/v4l2-mediabus.h +++ b/include/media/v4l2-mediabus.h @@ -73,6 +73,24 @@ #define V4L2_MBUS_CSI2_MAX_DATA_LANES 8 +/** + * enum v4l2_mbus_csi2_cphy_line_orders_type - CSI-2 C-PHY line order + * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ABC: C-PHY line order ABC (default) + * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ACB: C-PHY line order ACB + * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_BAC: C-PHY line order BAC + * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_BCA: C-PHY line order BCA + * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_CAB: C-PHY line order CAB + * @V4L2_MBUS_CSI2_CPHY_LINE_ORDER_CBA: C-PHY line order CBA + */ +enum v4l2_mbus_csi2_cphy_line_orders_type { + V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ABC, + V4L2_MBUS_CSI2_CPHY_LINE_ORDER_ACB, + V4L2_MBUS_CSI2_CPHY_LINE_ORDER_BAC, + V4L2_MBUS_CSI2_CPHY_LINE_ORDER_BCA, + V4L2_MBUS_CSI2_CPHY_LINE_ORDER_CAB, + V4L2_MBUS_CSI2_CPHY_LINE_ORDER_CBA, +}; + /** * struct v4l2_mbus_config_mipi_csi2 - MIPI CSI-2 data bus configuration * @flags: media bus (V4L2_MBUS_*) flags @@ -81,6 +99,8 @@ * @num_data_lanes: number of data lanes * @lane_polarities: polarity of the lanes. The order is the same of * the physical lanes. + * @line_orders: line order of the data lanes. The order is the same of the + * physical lanes. */ struct v4l2_mbus_config_mipi_csi2 { unsigned int flags; @@ -88,6 +108,7 @@ struct v4l2_mbus_config_mipi_csi2 { unsigned char clock_lane; unsigned char num_data_lanes; bool lane_polarities[1 + V4L2_MBUS_CSI2_MAX_DATA_LANES]; + enum v4l2_mbus_csi2_cphy_line_orders_type line_orders[V4L2_MBUS_CSI2_MAX_DATA_LANES]; }; /**
Extend the fwnode parsing to validate and fill in the CSI-2 C-PHY line-orders order properties as defined in MIPI Discovery and Configuration (DisCo) Specification for Imaging. Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> --- drivers/media/v4l2-core/v4l2-fwnode.c | 56 ++++++++++++++++++++++++++- include/media/v4l2-mediabus.h | 21 ++++++++++ 2 files changed, 76 insertions(+), 1 deletion(-)