| Message ID | 20190930093900.16524-5-m.felsch@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | TVP5150 Features and fixes | expand |
Hi Marco, On Mon, Sep 30, 2019 at 11:38:49AM +0200, Marco Felsch wrote: > The patch adds the initial connector parsing code, so we can move from a > driver specific parsing code to a generic one. Currently only the > generic fields and the analog-connector specific fields are parsed. Parsing > the other connector specific fields can be added by a simple callbacks. > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > --- > [1] https://patchwork.kernel.org/cover/10794703/ > > v10: > - drop V4L2_CONN_HDMI support > - adapt pr_err msg to reflect new state (-> connector is unkown) > > v9: > - Fix leading semicolon found by kbuild semicolon.cocci > > v8: > - V4L2_CON_* -> V4L2_CONN_* > - tvnorms -> sdtv-standards > - adapt to new v4l2_fwnode_connector_analog member > - return error in case of V4L2_CONN_HDMI > > v7: > @Jacopo: I dropped your r b tag becuase of the amount of changes I > made.. > > - drop unnecessary comments > - fix commet style > - s/v4l2_fwnode_connector_conv.name/v4l2_fwnode_connector_conv.compatible/ > - make label size variable and drop V4L2_CONNECTOR_MAX_LABEL usage > - do not assign a default label in case of no label was specified > - remove useless /* fall through */ comments > - add support for N connector links > - rename local variables to be more meaningful > - adjust kernedoc > - add v4l2_fwnode_connector_free() > - improve error handling (use different error values) > - make use of pr_warn_once() > > v6: > - use unsigned count var > - fix comment and style issues > - place '/* fall through */' to correct places > - fix error handling and cleanup by releasing fwnode > - drop vga and dvi parsing support as those connectors are rarely used > these days > > v5: > - s/strlcpy/strscpy/ > > v2-v4: > - nothing since the patch was squashed from series [1] into this > series. > --- > drivers/media/v4l2-core/v4l2-fwnode.c | 129 ++++++++++++++++++++++++++ > include/media/v4l2-fwnode.h | 38 ++++++++ > 2 files changed, 167 insertions(+) > > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c > index 3bd1888787eb..0bfa7cbf78df 100644 > --- a/drivers/media/v4l2-core/v4l2-fwnode.c > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c > @@ -595,6 +595,135 @@ void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link) > } > EXPORT_SYMBOL_GPL(v4l2_fwnode_put_link); > > +static const struct v4l2_fwnode_connector_conv { > + enum v4l2_connector_type type; > + const char *compatible; > +} connectors[] = { > + { > + .type = V4L2_CONN_COMPOSITE, > + .compatible = "composite-video-connector", > + }, { > + .type = V4L2_CONN_SVIDEO, > + .compatible = "svideo-connector", > + }, > +}; > + > +static enum v4l2_connector_type > +v4l2_fwnode_string_to_connector_type(const char *con_str) > +{ > + unsigned int i; > + > + for (i = 0; i < ARRAY_SIZE(connectors); i++) > + if (!strcmp(con_str, connectors[i].compatible)) > + return connectors[i].type; > + > + return V4L2_CONN_UNKNOWN; > +} > + > +static int > +v4l2_fwnode_connector_parse_analog(struct fwnode_handle *fwnode, > + struct v4l2_fwnode_connector *vc) > +{ > + u32 stds; > + int ret; > + > + ret = fwnode_property_read_u32(fwnode, "sdtv-standards", &stds); > + > + /* The property is optional. */ > + vc->connector.analog.sdtv_stds = ret ? V4L2_STD_ALL : stds; > + > + return 0; > +} > + > +void v4l2_fwnode_connector_free(struct v4l2_fwnode_connector *connector) > +{ > + unsigned int i; > + > + if (IS_ERR_OR_NULL(connector)) > + return; > + > + for (i = 0; i < connector->nr_of_links; i++) > + v4l2_fwnode_put_link(&connector->links[i]); > + kfree(connector->links); Please do set connector->links NULL here. That avoids accidentally double kfree on the pointer. > +} > +EXPORT_SYMBOL_GPL(v4l2_fwnode_connector_free); > + > +int v4l2_fwnode_connector_alloc_parse(struct fwnode_handle *fwnode, > + struct v4l2_fwnode_connector *connector) > +{ > + struct fwnode_handle *remote_pp, *remote_ep; > + const char *type_name; > + unsigned int i = 0, ep_num = 0; > + int err; > + > + memset(connector, 0, sizeof(*connector)); > + > + remote_pp = fwnode_graph_get_remote_port_parent(fwnode); > + if (!remote_pp) > + return -ENOLINK; > + > + /* Parse all common properties first. */ > + fwnode_graph_for_each_endpoint(remote_pp, remote_ep) > + ep_num++; If there are no endpoints, ep_num will be zero and kmalloc_array() will return NULL? There should be a way there are no endpoints, rather than returning -ENOMEM. > + > + connector->nr_of_links = ep_num; > + connector->links = kmalloc_array(ep_num, sizeof(*connector->links), > + GFP_KERNEL); > + if (!connector->links) { > + err = -ENOMEM; > + goto err_put_fwnode; > + } > + > + fwnode_graph_for_each_endpoint(remote_pp, remote_ep) { > + err = v4l2_fwnode_parse_link(remote_ep, &connector->links[i]); If you start parsing a connector starting from another device connected to it, nothing seems to prevent parsing the same links twice, in case the connector is connected to more than one sub-device. Or do I miss something crucial here? > + if (err) { > + fwnode_handle_put(remote_ep); > + goto err_free_links; > + } > + i++; > + } > + > + /* > + * Links reference counting guarantees access -> no duplication needed > + */ > + fwnode_property_read_string(remote_pp, "label", &connector->label); > + > + /* The connector-type is stored within the compatible string. */ > + err = fwnode_property_read_string(remote_pp, "compatible", &type_name); > + if (err) { > + err = -EINVAL; > + goto err_free_links; > + } > + connector->type = v4l2_fwnode_string_to_connector_type(type_name); > + > + /* Now parse the connector specific properties. */ > + switch (connector->type) { > + case V4L2_CONN_COMPOSITE: > + case V4L2_CONN_SVIDEO: > + err = v4l2_fwnode_connector_parse_analog(remote_pp, connector); > + break; > + case V4L2_CONN_UNKNOWN: > + default: > + pr_err("Unknown connector type\n"); > + err = -EINVAL; > + goto err_free_links; > + } > + > + fwnode_handle_put(remote_pp); > + > + return err; > + > +err_free_links: > + for (i = 0; i < ep_num; i++) > + v4l2_fwnode_put_link(&connector->links[i]); > + kfree(connector->links); > +err_put_fwnode: > + fwnode_handle_put(remote_pp); > + > + return err; > +} > +EXPORT_SYMBOL_GPL(v4l2_fwnode_connector_alloc_parse); > + > static int > v4l2_async_notifier_fwnode_parse_endpoint(struct device *dev, > struct v4l2_async_notifier *notifier, > diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h > index 65da646b579e..800302aa84d8 100644 > --- a/include/media/v4l2-fwnode.h > +++ b/include/media/v4l2-fwnode.h > @@ -276,6 +276,44 @@ int v4l2_fwnode_parse_link(struct fwnode_handle *fwnode, > */ > void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link); > > +/** > + * v4l2_fwnode_connector_free() - free the V4L2 connector acquired by > + * v4l2_fwnode_parse_connector() This would be nicer aligned with the text after the dash. > + * @connector: the V4L2 connector the resources of which are to be released > + * > + * Drop references to the connection link partners and free the memory allocated > + * by v4l2_fwnode_parse_connector() call. > + * > + * It is safe to call this function with NULL argument or on a V4L2 connector > + * the parsing of which failed. > + */ > +void v4l2_fwnode_connector_free(struct v4l2_fwnode_connector *connector); > + > +/** > + * v4l2_fwnode_parse_connector() - parse the connector on endpoint The name is different from the actual function. > + * @fwnode: pointer to the endpoint's fwnode handle where the connector is > + * connected to > + * @connector: pointer to the V4L2 fwnode connector data structure > + * > + * Fill the connector data structure with the connector type, label, all found > + * links between the host and the connector as well as connector specific data. > + * Since the label is optional it can be %NULL if no one was found. > + * > + * A reference is taken to both the connector and the connector destination > + * where the connector belongs to. This must be dropped when no longer needed. > + * Also the memory it has allocated to store the variable size data must be > + * freed. Both dropping the references and freeing the memory is done by > + * v4l2_fwnode_connector_free(). > + * > + * Return: > + * * %0 on success or a negative error code on failure: > + * * %-ENOMEM if memory allocation failed > + * * %-ENOLINK if the connector can't be found > + * * %-EINVAL on parsing failure Could this error code tell the endpoint is not connected to a connector? I think the perfectly suitable error code for this would be -ENOTCONN. :-D > + */ > +int v4l2_fwnode_connector_alloc_parse(struct fwnode_handle *fwnode, > + struct v4l2_fwnode_connector *connector); > + > /** > * typedef parse_endpoint_func - Driver's callback function to be called on > * each V4L2 fwnode endpoint. >
Hi Sakari, On 19-11-16 01:34, Sakari Ailus wrote: > Hi Marco, > > On Mon, Sep 30, 2019 at 11:38:49AM +0200, Marco Felsch wrote: > > The patch adds the initial connector parsing code, so we can move from a > > driver specific parsing code to a generic one. Currently only the > > generic fields and the analog-connector specific fields are parsed. Parsing > > the other connector specific fields can be added by a simple callbacks. > > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > > --- > > [1] https://patchwork.kernel.org/cover/10794703/ > > > > v10: > > - drop V4L2_CONN_HDMI support > > - adapt pr_err msg to reflect new state (-> connector is unkown) > > > > v9: > > - Fix leading semicolon found by kbuild semicolon.cocci > > > > v8: > > - V4L2_CON_* -> V4L2_CONN_* > > - tvnorms -> sdtv-standards > > - adapt to new v4l2_fwnode_connector_analog member > > - return error in case of V4L2_CONN_HDMI > > > > v7: > > @Jacopo: I dropped your r b tag becuase of the amount of changes I > > made.. > > > > - drop unnecessary comments > > - fix commet style > > - s/v4l2_fwnode_connector_conv.name/v4l2_fwnode_connector_conv.compatible/ > > - make label size variable and drop V4L2_CONNECTOR_MAX_LABEL usage > > - do not assign a default label in case of no label was specified > > - remove useless /* fall through */ comments > > - add support for N connector links > > - rename local variables to be more meaningful > > - adjust kernedoc > > - add v4l2_fwnode_connector_free() > > - improve error handling (use different error values) > > - make use of pr_warn_once() > > > > v6: > > - use unsigned count var > > - fix comment and style issues > > - place '/* fall through */' to correct places > > - fix error handling and cleanup by releasing fwnode > > - drop vga and dvi parsing support as those connectors are rarely used > > these days > > > > v5: > > - s/strlcpy/strscpy/ > > > > v2-v4: > > - nothing since the patch was squashed from series [1] into this > > series. > > --- > > drivers/media/v4l2-core/v4l2-fwnode.c | 129 ++++++++++++++++++++++++++ > > include/media/v4l2-fwnode.h | 38 ++++++++ > > 2 files changed, 167 insertions(+) > > > > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c > > index 3bd1888787eb..0bfa7cbf78df 100644 > > --- a/drivers/media/v4l2-core/v4l2-fwnode.c > > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c > > @@ -595,6 +595,135 @@ void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link) > > } > > EXPORT_SYMBOL_GPL(v4l2_fwnode_put_link); > > > > +static const struct v4l2_fwnode_connector_conv { > > + enum v4l2_connector_type type; > > + const char *compatible; > > +} connectors[] = { > > + { > > + .type = V4L2_CONN_COMPOSITE, > > + .compatible = "composite-video-connector", > > + }, { > > + .type = V4L2_CONN_SVIDEO, > > + .compatible = "svideo-connector", > > + }, > > +}; > > + > > +static enum v4l2_connector_type > > +v4l2_fwnode_string_to_connector_type(const char *con_str) > > +{ > > + unsigned int i; > > + > > + for (i = 0; i < ARRAY_SIZE(connectors); i++) > > + if (!strcmp(con_str, connectors[i].compatible)) > > + return connectors[i].type; > > + > > + return V4L2_CONN_UNKNOWN; > > +} > > + > > +static int > > +v4l2_fwnode_connector_parse_analog(struct fwnode_handle *fwnode, > > + struct v4l2_fwnode_connector *vc) > > +{ > > + u32 stds; > > + int ret; > > + > > + ret = fwnode_property_read_u32(fwnode, "sdtv-standards", &stds); > > + > > + /* The property is optional. */ > > + vc->connector.analog.sdtv_stds = ret ? V4L2_STD_ALL : stds; > > + > > + return 0; > > +} > > + > > +void v4l2_fwnode_connector_free(struct v4l2_fwnode_connector *connector) > > +{ > > + unsigned int i; > > + > > + if (IS_ERR_OR_NULL(connector)) > > + return; > > + > > + for (i = 0; i < connector->nr_of_links; i++) > > + v4l2_fwnode_put_link(&connector->links[i]); > > + kfree(connector->links); > > Please do set connector->links NULL here. That avoids accidentally double > kfree on the pointer. Okay, I will do that. > > +} > > +EXPORT_SYMBOL_GPL(v4l2_fwnode_connector_free); > > + > > +int v4l2_fwnode_connector_alloc_parse(struct fwnode_handle *fwnode, > > + struct v4l2_fwnode_connector *connector) > > +{ > > + struct fwnode_handle *remote_pp, *remote_ep; > > + const char *type_name; > > + unsigned int i = 0, ep_num = 0; > > + int err; > > + > > + memset(connector, 0, sizeof(*connector)); > > + > > + remote_pp = fwnode_graph_get_remote_port_parent(fwnode); > > + if (!remote_pp) > > + return -ENOLINK; Should I drop this logic here and force the caller to pass the connector endpoint? > > + > > + /* Parse all common properties first. */ > > + fwnode_graph_for_each_endpoint(remote_pp, remote_ep) > > + ep_num++; > > If there are no endpoints, ep_num will be zero and kmalloc_array() will > return NULL? There should be a way there are no endpoints, rather than > returning -ENOMEM. Hm.. using the current implementation (please see my above comment) ensures that there is always one link but if I change that I need to check this, good point. > > + > > + connector->nr_of_links = ep_num; > > + connector->links = kmalloc_array(ep_num, sizeof(*connector->links), > > + GFP_KERNEL); > > + if (!connector->links) { > > + err = -ENOMEM; > > + goto err_put_fwnode; > > + } > > + > > + fwnode_graph_for_each_endpoint(remote_pp, remote_ep) { > > + err = v4l2_fwnode_parse_link(remote_ep, &connector->links[i]); > > If you start parsing a connector starting from another device connected to > it, nothing seems to prevent parsing the same links twice, in case the > connector is connected to more than one sub-device. > > Or do I miss something crucial here? That is true because currently each sub-device implements their own connector parsing/handling stuff.. What i wanted to address with this code is that one sub-device can have multiple links to one connector e.g. s-video connectors (1xluma 1xchroma). I don't know how to handle this without a simple connector device. Regards, Marco > > + if (err) { > > + fwnode_handle_put(remote_ep); > > + goto err_free_links; > > + } > > + i++; > > + } > > + > > + /* > > + * Links reference counting guarantees access -> no duplication needed > > + */ > > + fwnode_property_read_string(remote_pp, "label", &connector->label); > > + > > + /* The connector-type is stored within the compatible string. */ > > + err = fwnode_property_read_string(remote_pp, "compatible", &type_name); > > + if (err) { > > + err = -EINVAL; > > + goto err_free_links; > > + } > > + connector->type = v4l2_fwnode_string_to_connector_type(type_name); > > + > > + /* Now parse the connector specific properties. */ > > + switch (connector->type) { > > + case V4L2_CONN_COMPOSITE: > > + case V4L2_CONN_SVIDEO: > > + err = v4l2_fwnode_connector_parse_analog(remote_pp, connector); > > + break; > > + case V4L2_CONN_UNKNOWN: > > + default: > > + pr_err("Unknown connector type\n"); > > + err = -EINVAL; > > + goto err_free_links; > > + } > > + > > + fwnode_handle_put(remote_pp); > > + > > + return err; > > + > > +err_free_links: > > + for (i = 0; i < ep_num; i++) > > + v4l2_fwnode_put_link(&connector->links[i]); > > + kfree(connector->links); > > +err_put_fwnode: > > + fwnode_handle_put(remote_pp); > > + > > + return err; > > +} > > +EXPORT_SYMBOL_GPL(v4l2_fwnode_connector_alloc_parse); > > + > > static int > > v4l2_async_notifier_fwnode_parse_endpoint(struct device *dev, > > struct v4l2_async_notifier *notifier, > > diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h > > index 65da646b579e..800302aa84d8 100644 > > --- a/include/media/v4l2-fwnode.h > > +++ b/include/media/v4l2-fwnode.h > > @@ -276,6 +276,44 @@ int v4l2_fwnode_parse_link(struct fwnode_handle *fwnode, > > */ > > void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link); > > > > +/** > > + * v4l2_fwnode_connector_free() - free the V4L2 connector acquired by > > + * v4l2_fwnode_parse_connector() > > This would be nicer aligned with the text after the dash. > > > + * @connector: the V4L2 connector the resources of which are to be released > > + * > > + * Drop references to the connection link partners and free the memory allocated > > + * by v4l2_fwnode_parse_connector() call. > > + * > > + * It is safe to call this function with NULL argument or on a V4L2 connector > > + * the parsing of which failed. > > + */ > > +void v4l2_fwnode_connector_free(struct v4l2_fwnode_connector *connector); > > + > > +/** > > + * v4l2_fwnode_parse_connector() - parse the connector on endpoint > > The name is different from the actual function. > > > + * @fwnode: pointer to the endpoint's fwnode handle where the connector is > > + * connected to > > + * @connector: pointer to the V4L2 fwnode connector data structure > > + * > > + * Fill the connector data structure with the connector type, label, all found > > + * links between the host and the connector as well as connector specific data. > > + * Since the label is optional it can be %NULL if no one was found. > > + * > > + * A reference is taken to both the connector and the connector destination > > + * where the connector belongs to. This must be dropped when no longer needed. > > + * Also the memory it has allocated to store the variable size data must be > > + * freed. Both dropping the references and freeing the memory is done by > > + * v4l2_fwnode_connector_free(). > > + * > > + * Return: > > + * * %0 on success or a negative error code on failure: > > + * * %-ENOMEM if memory allocation failed > > + * * %-ENOLINK if the connector can't be found > > + * * %-EINVAL on parsing failure > > Could this error code tell the endpoint is not connected to a connector? > > I think the perfectly suitable error code for this would be -ENOTCONN. :-D > > > + */ > > +int v4l2_fwnode_connector_alloc_parse(struct fwnode_handle *fwnode, > > + struct v4l2_fwnode_connector *connector); > > + > > /** > > * typedef parse_endpoint_func - Driver's callback function to be called on > > * each V4L2 fwnode endpoint. > > > > -- > Regards, > > Sakari Ailus > sakari.ailus@linux.intel.com >
Hi Sakari, gentle ping. Regards, Marco On 19-11-19 12:37, Marco Felsch wrote: > Hi Sakari, > > On 19-11-16 01:34, Sakari Ailus wrote: > > Hi Marco, > > > > On Mon, Sep 30, 2019 at 11:38:49AM +0200, Marco Felsch wrote: > > > The patch adds the initial connector parsing code, so we can move from a > > > driver specific parsing code to a generic one. Currently only the > > > generic fields and the analog-connector specific fields are parsed. Parsing > > > the other connector specific fields can be added by a simple callbacks. > > > > > > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> > > > --- > > > [1] https://patchwork.kernel.org/cover/10794703/ > > > > > > v10: > > > - drop V4L2_CONN_HDMI support > > > - adapt pr_err msg to reflect new state (-> connector is unkown) > > > > > > v9: > > > - Fix leading semicolon found by kbuild semicolon.cocci > > > > > > v8: > > > - V4L2_CON_* -> V4L2_CONN_* > > > - tvnorms -> sdtv-standards > > > - adapt to new v4l2_fwnode_connector_analog member > > > - return error in case of V4L2_CONN_HDMI > > > > > > v7: > > > @Jacopo: I dropped your r b tag becuase of the amount of changes I > > > made.. > > > > > > - drop unnecessary comments > > > - fix commet style > > > - s/v4l2_fwnode_connector_conv.name/v4l2_fwnode_connector_conv.compatible/ > > > - make label size variable and drop V4L2_CONNECTOR_MAX_LABEL usage > > > - do not assign a default label in case of no label was specified > > > - remove useless /* fall through */ comments > > > - add support for N connector links > > > - rename local variables to be more meaningful > > > - adjust kernedoc > > > - add v4l2_fwnode_connector_free() > > > - improve error handling (use different error values) > > > - make use of pr_warn_once() > > > > > > v6: > > > - use unsigned count var > > > - fix comment and style issues > > > - place '/* fall through */' to correct places > > > - fix error handling and cleanup by releasing fwnode > > > - drop vga and dvi parsing support as those connectors are rarely used > > > these days > > > > > > v5: > > > - s/strlcpy/strscpy/ > > > > > > v2-v4: > > > - nothing since the patch was squashed from series [1] into this > > > series. > > > --- > > > drivers/media/v4l2-core/v4l2-fwnode.c | 129 ++++++++++++++++++++++++++ > > > include/media/v4l2-fwnode.h | 38 ++++++++ > > > 2 files changed, 167 insertions(+) > > > > > > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c > > > index 3bd1888787eb..0bfa7cbf78df 100644 > > > --- a/drivers/media/v4l2-core/v4l2-fwnode.c > > > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c > > > @@ -595,6 +595,135 @@ void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link) > > > } > > > EXPORT_SYMBOL_GPL(v4l2_fwnode_put_link); > > > > > > +static const struct v4l2_fwnode_connector_conv { > > > + enum v4l2_connector_type type; > > > + const char *compatible; > > > +} connectors[] = { > > > + { > > > + .type = V4L2_CONN_COMPOSITE, > > > + .compatible = "composite-video-connector", > > > + }, { > > > + .type = V4L2_CONN_SVIDEO, > > > + .compatible = "svideo-connector", > > > + }, > > > +}; > > > + > > > +static enum v4l2_connector_type > > > +v4l2_fwnode_string_to_connector_type(const char *con_str) > > > +{ > > > + unsigned int i; > > > + > > > + for (i = 0; i < ARRAY_SIZE(connectors); i++) > > > + if (!strcmp(con_str, connectors[i].compatible)) > > > + return connectors[i].type; > > > + > > > + return V4L2_CONN_UNKNOWN; > > > +} > > > + > > > +static int > > > +v4l2_fwnode_connector_parse_analog(struct fwnode_handle *fwnode, > > > + struct v4l2_fwnode_connector *vc) > > > +{ > > > + u32 stds; > > > + int ret; > > > + > > > + ret = fwnode_property_read_u32(fwnode, "sdtv-standards", &stds); > > > + > > > + /* The property is optional. */ > > > + vc->connector.analog.sdtv_stds = ret ? V4L2_STD_ALL : stds; > > > + > > > + return 0; > > > +} > > > + > > > +void v4l2_fwnode_connector_free(struct v4l2_fwnode_connector *connector) > > > +{ > > > + unsigned int i; > > > + > > > + if (IS_ERR_OR_NULL(connector)) > > > + return; > > > + > > > + for (i = 0; i < connector->nr_of_links; i++) > > > + v4l2_fwnode_put_link(&connector->links[i]); > > > + kfree(connector->links); > > > > Please do set connector->links NULL here. That avoids accidentally double > > kfree on the pointer. > > Okay, I will do that. > > > > +} > > > +EXPORT_SYMBOL_GPL(v4l2_fwnode_connector_free); > > > + > > > +int v4l2_fwnode_connector_alloc_parse(struct fwnode_handle *fwnode, > > > + struct v4l2_fwnode_connector *connector) > > > +{ > > > + struct fwnode_handle *remote_pp, *remote_ep; > > > + const char *type_name; > > > + unsigned int i = 0, ep_num = 0; > > > + int err; > > > + > > > + memset(connector, 0, sizeof(*connector)); > > > + > > > + remote_pp = fwnode_graph_get_remote_port_parent(fwnode); > > > + if (!remote_pp) > > > + return -ENOLINK; > > Should I drop this logic here and force the caller to pass the connector > endpoint? > > > > + > > > + /* Parse all common properties first. */ > > > + fwnode_graph_for_each_endpoint(remote_pp, remote_ep) > > > + ep_num++; > > > > If there are no endpoints, ep_num will be zero and kmalloc_array() will > > return NULL? There should be a way there are no endpoints, rather than > > returning -ENOMEM. > > Hm.. using the current implementation (please see my above comment) > ensures that there is always one link but if I change that I need to > check this, good point. > > > > + > > > + connector->nr_of_links = ep_num; > > > + connector->links = kmalloc_array(ep_num, sizeof(*connector->links), > > > + GFP_KERNEL); > > > + if (!connector->links) { > > > + err = -ENOMEM; > > > + goto err_put_fwnode; > > > + } > > > + > > > + fwnode_graph_for_each_endpoint(remote_pp, remote_ep) { > > > + err = v4l2_fwnode_parse_link(remote_ep, &connector->links[i]); > > > > If you start parsing a connector starting from another device connected to > > it, nothing seems to prevent parsing the same links twice, in case the > > connector is connected to more than one sub-device. > > > > Or do I miss something crucial here? > > That is true because currently each sub-device implements their own > connector parsing/handling stuff.. What i wanted to address with this > code is that one sub-device can have multiple links to one connector > e.g. s-video connectors (1xluma 1xchroma). I don't know how to handle > this without a simple connector device. > > Regards, > Marco > > > > + if (err) { > > > + fwnode_handle_put(remote_ep); > > > + goto err_free_links; > > > + } > > > + i++; > > > + } > > > + > > > + /* > > > + * Links reference counting guarantees access -> no duplication needed > > > + */ > > > + fwnode_property_read_string(remote_pp, "label", &connector->label); > > > + > > > + /* The connector-type is stored within the compatible string. */ > > > + err = fwnode_property_read_string(remote_pp, "compatible", &type_name); > > > + if (err) { > > > + err = -EINVAL; > > > + goto err_free_links; > > > + } > > > + connector->type = v4l2_fwnode_string_to_connector_type(type_name); > > > + > > > + /* Now parse the connector specific properties. */ > > > + switch (connector->type) { > > > + case V4L2_CONN_COMPOSITE: > > > + case V4L2_CONN_SVIDEO: > > > + err = v4l2_fwnode_connector_parse_analog(remote_pp, connector); > > > + break; > > > + case V4L2_CONN_UNKNOWN: > > > + default: > > > + pr_err("Unknown connector type\n"); > > > + err = -EINVAL; > > > + goto err_free_links; > > > + } > > > + > > > + fwnode_handle_put(remote_pp); > > > + > > > + return err; > > > + > > > +err_free_links: > > > + for (i = 0; i < ep_num; i++) > > > + v4l2_fwnode_put_link(&connector->links[i]); > > > + kfree(connector->links); > > > +err_put_fwnode: > > > + fwnode_handle_put(remote_pp); > > > + > > > + return err; > > > +} > > > +EXPORT_SYMBOL_GPL(v4l2_fwnode_connector_alloc_parse); > > > + > > > static int > > > v4l2_async_notifier_fwnode_parse_endpoint(struct device *dev, > > > struct v4l2_async_notifier *notifier, > > > diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h > > > index 65da646b579e..800302aa84d8 100644 > > > --- a/include/media/v4l2-fwnode.h > > > +++ b/include/media/v4l2-fwnode.h > > > @@ -276,6 +276,44 @@ int v4l2_fwnode_parse_link(struct fwnode_handle *fwnode, > > > */ > > > void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link); > > > > > > +/** > > > + * v4l2_fwnode_connector_free() - free the V4L2 connector acquired by > > > + * v4l2_fwnode_parse_connector() > > > > This would be nicer aligned with the text after the dash. > > > > > + * @connector: the V4L2 connector the resources of which are to be released > > > + * > > > + * Drop references to the connection link partners and free the memory allocated > > > + * by v4l2_fwnode_parse_connector() call. > > > + * > > > + * It is safe to call this function with NULL argument or on a V4L2 connector > > > + * the parsing of which failed. > > > + */ > > > +void v4l2_fwnode_connector_free(struct v4l2_fwnode_connector *connector); > > > + > > > +/** > > > + * v4l2_fwnode_parse_connector() - parse the connector on endpoint > > > > The name is different from the actual function. > > > > > + * @fwnode: pointer to the endpoint's fwnode handle where the connector is > > > + * connected to > > > + * @connector: pointer to the V4L2 fwnode connector data structure > > > + * > > > + * Fill the connector data structure with the connector type, label, all found > > > + * links between the host and the connector as well as connector specific data. > > > + * Since the label is optional it can be %NULL if no one was found. > > > + * > > > + * A reference is taken to both the connector and the connector destination > > > + * where the connector belongs to. This must be dropped when no longer needed. > > > + * Also the memory it has allocated to store the variable size data must be > > > + * freed. Both dropping the references and freeing the memory is done by > > > + * v4l2_fwnode_connector_free(). > > > + * > > > + * Return: > > > + * * %0 on success or a negative error code on failure: > > > + * * %-ENOMEM if memory allocation failed > > > + * * %-ENOLINK if the connector can't be found > > > + * * %-EINVAL on parsing failure > > > > Could this error code tell the endpoint is not connected to a connector? > > > > I think the perfectly suitable error code for this would be -ENOTCONN. :-D > > > > > + */ > > > +int v4l2_fwnode_connector_alloc_parse(struct fwnode_handle *fwnode, > > > + struct v4l2_fwnode_connector *connector); > > > + > > > /** > > > * typedef parse_endpoint_func - Driver's callback function to be called on > > > * each V4L2 fwnode endpoint. > > > > > > > -- > > Regards, > > > > Sakari Ailus > > sakari.ailus@linux.intel.com > > > > -- > Pengutronix e.K. | | > Steuerwalder Str. 21 | http://www.pengutronix.de/ | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | > >
Hi Sakari, On 19-11-16 01:34, Sakari Ailus wrote: > Hi Marco, > > On Mon, Sep 30, 2019 at 11:38:49AM +0200, Marco Felsch wrote: ... > > +int v4l2_fwnode_connector_alloc_parse(struct fwnode_handle *fwnode, > > + struct v4l2_fwnode_connector *connector) > > +{ > > + struct fwnode_handle *remote_pp, *remote_ep; > > + const char *type_name; > > + unsigned int i = 0, ep_num = 0; > > + int err; > > + > > + memset(connector, 0, sizeof(*connector)); > > + > > + remote_pp = fwnode_graph_get_remote_port_parent(fwnode); > > + if (!remote_pp) > > + return -ENOLINK; I will align the API with the v4l2_fwnode_endpoint_alloc_parse() function so the caller need to pass the connector fwnode handle. > > + > > + /* Parse all common properties first. */ > > + fwnode_graph_for_each_endpoint(remote_pp, remote_ep) > > + ep_num++; > > If there are no endpoints, ep_num will be zero and kmalloc_array() will > return NULL? There should be a way there are no endpoints, rather than > returning -ENOMEM. > > > + > > + connector->nr_of_links = ep_num; > > + connector->links = kmalloc_array(ep_num, sizeof(*connector->links), > > + GFP_KERNEL); > > + if (!connector->links) { > > + err = -ENOMEM; > > + goto err_put_fwnode; > > + } > > + > > + fwnode_graph_for_each_endpoint(remote_pp, remote_ep) { > > + err = v4l2_fwnode_parse_link(remote_ep, &connector->links[i]); > > If you start parsing a connector starting from another device connected to > it, nothing seems to prevent parsing the same links twice, in case the > connector is connected to more than one sub-device. > > Or do I miss something crucial here? Yes thats right but it seems that sharing connectors isn't supported at all. All bridge drivers using connectors implementing the connector handling by their self. Anyway, I will add a function parameter to check that we parse only the endpoints connected to the calling sub-dev. Regards, Marco > > + if (err) { > > + fwnode_handle_put(remote_ep); > > + goto err_free_links; > > + } > > + i++; > > + } > > + > > + /* > > + * Links reference counting guarantees access -> no duplication needed > > + */ > > + fwnode_property_read_string(remote_pp, "label", &connector->label); > > + > > + /* The connector-type is stored within the compatible string. */ > > + err = fwnode_property_read_string(remote_pp, "compatible", &type_name); > > + if (err) { > > + err = -EINVAL; > > + goto err_free_links; > > + } > > + connector->type = v4l2_fwnode_string_to_connector_type(type_name); > > + > > + /* Now parse the connector specific properties. */ > > + switch (connector->type) { > > + case V4L2_CONN_COMPOSITE: > > + case V4L2_CONN_SVIDEO: > > + err = v4l2_fwnode_connector_parse_analog(remote_pp, connector); > > + break; > > + case V4L2_CONN_UNKNOWN: > > + default: > > + pr_err("Unknown connector type\n"); > > + err = -EINVAL; > > + goto err_free_links; > > + } > > + > > + fwnode_handle_put(remote_pp); > > + > > + return err; > > + > > +err_free_links: > > + for (i = 0; i < ep_num; i++) > > + v4l2_fwnode_put_link(&connector->links[i]); > > + kfree(connector->links); > > +err_put_fwnode: > > + fwnode_handle_put(remote_pp); > > + > > + return err; > > +} > > +EXPORT_SYMBOL_GPL(v4l2_fwnode_connector_alloc_parse); > > +
Hi Sakari, just a few questions about the below helper. On 20-01-08 16:36, Marco Felsch wrote: > Hi Sakari, > > On 19-11-16 01:34, Sakari Ailus wrote: > > Hi Marco, > > > > On Mon, Sep 30, 2019 at 11:38:49AM +0200, Marco Felsch wrote: > > ... > > > > +int v4l2_fwnode_connector_alloc_parse(struct fwnode_handle *fwnode, > > > + struct v4l2_fwnode_connector *connector) > > > +{ > > > + struct fwnode_handle *remote_pp, *remote_ep; > > > + const char *type_name; > > > + unsigned int i = 0, ep_num = 0; > > > + int err; > > > + > > > + memset(connector, 0, sizeof(*connector)); > > > + > > > + remote_pp = fwnode_graph_get_remote_port_parent(fwnode); > > > + if (!remote_pp) > > > + return -ENOLINK; > > I will align the API with the v4l2_fwnode_endpoint_alloc_parse() > function so the caller need to pass the connector fwnode handle. Should we really align this with v4l2_fwnode_endpoint_alloc_parse()? I have some concerns because this moves the step to the user. Using the current approach makes it easier for the user. Regards, Marco > > > + > > > + /* Parse all common properties first. */ > > > + fwnode_graph_for_each_endpoint(remote_pp, remote_ep) > > > + ep_num++; > > > > If there are no endpoints, ep_num will be zero and kmalloc_array() will > > return NULL? There should be a way there are no endpoints, rather than > > returning -ENOMEM. > > > > > + > > > + connector->nr_of_links = ep_num; > > > + connector->links = kmalloc_array(ep_num, sizeof(*connector->links), > > > + GFP_KERNEL); > > > + if (!connector->links) { > > > + err = -ENOMEM; > > > + goto err_put_fwnode; > > > + } > > > + > > > + fwnode_graph_for_each_endpoint(remote_pp, remote_ep) { > > > + err = v4l2_fwnode_parse_link(remote_ep, &connector->links[i]); > > > > If you start parsing a connector starting from another device connected to > > it, nothing seems to prevent parsing the same links twice, in case the > > connector is connected to more than one sub-device. > > > > Or do I miss something crucial here? > > Yes thats right but it seems that sharing connectors isn't supported at > all. All bridge drivers using connectors implementing the connector > handling by their self. Anyway, I will add a function parameter to check > that we parse only the endpoints connected to the calling sub-dev. > > Regards, > Marco > > > > + if (err) { > > > + fwnode_handle_put(remote_ep); > > > + goto err_free_links; > > > + } > > > + i++; > > > + } > > > + > > > + /* > > > + * Links reference counting guarantees access -> no duplication needed > > > + */ > > > + fwnode_property_read_string(remote_pp, "label", &connector->label); > > > + > > > + /* The connector-type is stored within the compatible string. */ > > > + err = fwnode_property_read_string(remote_pp, "compatible", &type_name); > > > + if (err) { > > > + err = -EINVAL; > > > + goto err_free_links; > > > + } > > > + connector->type = v4l2_fwnode_string_to_connector_type(type_name); > > > + > > > + /* Now parse the connector specific properties. */ > > > + switch (connector->type) { > > > + case V4L2_CONN_COMPOSITE: > > > + case V4L2_CONN_SVIDEO: > > > + err = v4l2_fwnode_connector_parse_analog(remote_pp, connector); > > > + break; > > > + case V4L2_CONN_UNKNOWN: > > > + default: > > > + pr_err("Unknown connector type\n"); > > > + err = -EINVAL; > > > + goto err_free_links; > > > + } > > > + > > > + fwnode_handle_put(remote_pp); > > > + > > > + return err; > > > + > > > +err_free_links: > > > + for (i = 0; i < ep_num; i++) > > > + v4l2_fwnode_put_link(&connector->links[i]); > > > + kfree(connector->links); > > > +err_put_fwnode: > > > + fwnode_handle_put(remote_pp); > > > + > > > + return err; > > > +} > > > +EXPORT_SYMBOL_GPL(v4l2_fwnode_connector_alloc_parse); > > > + > >
Hi Sakari, On 19-11-16 01:34, Sakari Ailus wrote: > Hi Marco, > > On Mon, Sep 30, 2019 at 11:38:49AM +0200, Marco Felsch wrote: ... Let me sum up our irc discussion about that kAPI. Our starting point is a fwnode based subdev which has connectors in front of there pins. Connectors are used to limit the subdev to some device limits e.g. if the device support only PAL-Input streams and the subdev has an buggy autodetect mechanism. In that case the connector can be used by the subdev to set the possible TV-Norms to PAL. Currently the tvp5150 is the only fwnode based subdev implementing connectors. Connectors have common and connector specific properties. All current provided connectors can be found here: Documentation/devicetree/bindings/display/connector/ . Parsing the properties is common to all _upcoming_ fwnode based subdevs so this should be done within the core. So lets move on to the parsing helper. > > +int v4l2_fwnode_connector_alloc_parse(struct fwnode_handle *fwnode, > > + struct v4l2_fwnode_connector *connector) > > +{ This kAPI seems to fit all current use cases. The API is not responsible to alloc the 'struct v4l2_fwnode_connector' instead it is only used to fill this struct. The given 'struct fwnode_handle' should be the subdev local ep-fwnode because the user already has a reference to this ep. This helper has two use-cases: 1) Parsing the connector properties and add the initial (1st) link. 2) Add further n-links upon n-calls to an already parsed connector. Going this way we need to ensure that the caller init the 'struct v4l2_fwnode_connector' to '0' before call this helper. This can be documented within the kAPI doc. Regards, Marco
Hi Marco, On Wed, Jan 15, 2020 at 06:06:21PM +0100, Marco Felsch wrote: > Hi Sakari, > > On 19-11-16 01:34, Sakari Ailus wrote: > > Hi Marco, > > > > On Mon, Sep 30, 2019 at 11:38:49AM +0200, Marco Felsch wrote: > > ... > > Let me sum up our irc discussion about that kAPI. > > Our starting point is a fwnode based subdev which has connectors in > front of there pins. Connectors are used to limit the subdev to some > device limits e.g. if the device support only PAL-Input streams and the > subdev has an buggy autodetect mechanism. In that case the connector can > be used by the subdev to set the possible TV-Norms to PAL. Currently the > tvp5150 is the only fwnode based subdev implementing connectors. > > Connectors have common and connector specific properties. All current > provided connectors can be found here: > Documentation/devicetree/bindings/display/connector/ . > > Parsing the properties is common to all _upcoming_ fwnode based subdevs > so this should be done within the core. So lets move on to the parsing > helper. > > > > +int v4l2_fwnode_connector_alloc_parse(struct fwnode_handle *fwnode, > > > + struct v4l2_fwnode_connector *connector) > > > +{ > > This kAPI seems to fit all current use cases. The API is not responsible > to alloc the 'struct v4l2_fwnode_connector' instead it is only used to > fill this struct. The given 'struct fwnode_handle' should be the subdev > local ep-fwnode because the user already has a reference to this ep. > > This helper has two use-cases: > 1) Parsing the connector properties and add the initial (1st) link. > 2) Add further n-links upon n-calls to an already parsed connector. > > Going this way we need to ensure that the caller init the 'struct > v4l2_fwnode_connector' to '0' before call this helper. This can be > documented within the kAPI doc. The problem with the above is that although the API does not prevent sharing connectors as such, it does not support it either: parsing a connector on another sub-device ends up looking like a new connector, independently of whether one (or more) entities representing the same connector already exist. Either way, I discussed this with Hans, and we concluded it's fine for now. We've dealt with similar changes before, so when someone needs sharing connectors, he'll need to implement it, also changing the kAPI drivers use. Could you address the other comments I've given on the set?
diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c index 3bd1888787eb..0bfa7cbf78df 100644 --- a/drivers/media/v4l2-core/v4l2-fwnode.c +++ b/drivers/media/v4l2-core/v4l2-fwnode.c @@ -595,6 +595,135 @@ void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link) } EXPORT_SYMBOL_GPL(v4l2_fwnode_put_link); +static const struct v4l2_fwnode_connector_conv { + enum v4l2_connector_type type; + const char *compatible; +} connectors[] = { + { + .type = V4L2_CONN_COMPOSITE, + .compatible = "composite-video-connector", + }, { + .type = V4L2_CONN_SVIDEO, + .compatible = "svideo-connector", + }, +}; + +static enum v4l2_connector_type +v4l2_fwnode_string_to_connector_type(const char *con_str) +{ + unsigned int i; + + for (i = 0; i < ARRAY_SIZE(connectors); i++) + if (!strcmp(con_str, connectors[i].compatible)) + return connectors[i].type; + + return V4L2_CONN_UNKNOWN; +} + +static int +v4l2_fwnode_connector_parse_analog(struct fwnode_handle *fwnode, + struct v4l2_fwnode_connector *vc) +{ + u32 stds; + int ret; + + ret = fwnode_property_read_u32(fwnode, "sdtv-standards", &stds); + + /* The property is optional. */ + vc->connector.analog.sdtv_stds = ret ? V4L2_STD_ALL : stds; + + return 0; +} + +void v4l2_fwnode_connector_free(struct v4l2_fwnode_connector *connector) +{ + unsigned int i; + + if (IS_ERR_OR_NULL(connector)) + return; + + for (i = 0; i < connector->nr_of_links; i++) + v4l2_fwnode_put_link(&connector->links[i]); + kfree(connector->links); +} +EXPORT_SYMBOL_GPL(v4l2_fwnode_connector_free); + +int v4l2_fwnode_connector_alloc_parse(struct fwnode_handle *fwnode, + struct v4l2_fwnode_connector *connector) +{ + struct fwnode_handle *remote_pp, *remote_ep; + const char *type_name; + unsigned int i = 0, ep_num = 0; + int err; + + memset(connector, 0, sizeof(*connector)); + + remote_pp = fwnode_graph_get_remote_port_parent(fwnode); + if (!remote_pp) + return -ENOLINK; + + /* Parse all common properties first. */ + fwnode_graph_for_each_endpoint(remote_pp, remote_ep) + ep_num++; + + connector->nr_of_links = ep_num; + connector->links = kmalloc_array(ep_num, sizeof(*connector->links), + GFP_KERNEL); + if (!connector->links) { + err = -ENOMEM; + goto err_put_fwnode; + } + + fwnode_graph_for_each_endpoint(remote_pp, remote_ep) { + err = v4l2_fwnode_parse_link(remote_ep, &connector->links[i]); + if (err) { + fwnode_handle_put(remote_ep); + goto err_free_links; + } + i++; + } + + /* + * Links reference counting guarantees access -> no duplication needed + */ + fwnode_property_read_string(remote_pp, "label", &connector->label); + + /* The connector-type is stored within the compatible string. */ + err = fwnode_property_read_string(remote_pp, "compatible", &type_name); + if (err) { + err = -EINVAL; + goto err_free_links; + } + connector->type = v4l2_fwnode_string_to_connector_type(type_name); + + /* Now parse the connector specific properties. */ + switch (connector->type) { + case V4L2_CONN_COMPOSITE: + case V4L2_CONN_SVIDEO: + err = v4l2_fwnode_connector_parse_analog(remote_pp, connector); + break; + case V4L2_CONN_UNKNOWN: + default: + pr_err("Unknown connector type\n"); + err = -EINVAL; + goto err_free_links; + } + + fwnode_handle_put(remote_pp); + + return err; + +err_free_links: + for (i = 0; i < ep_num; i++) + v4l2_fwnode_put_link(&connector->links[i]); + kfree(connector->links); +err_put_fwnode: + fwnode_handle_put(remote_pp); + + return err; +} +EXPORT_SYMBOL_GPL(v4l2_fwnode_connector_alloc_parse); + static int v4l2_async_notifier_fwnode_parse_endpoint(struct device *dev, struct v4l2_async_notifier *notifier, diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h index 65da646b579e..800302aa84d8 100644 --- a/include/media/v4l2-fwnode.h +++ b/include/media/v4l2-fwnode.h @@ -276,6 +276,44 @@ int v4l2_fwnode_parse_link(struct fwnode_handle *fwnode, */ void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link); +/** + * v4l2_fwnode_connector_free() - free the V4L2 connector acquired by + * v4l2_fwnode_parse_connector() + * @connector: the V4L2 connector the resources of which are to be released + * + * Drop references to the connection link partners and free the memory allocated + * by v4l2_fwnode_parse_connector() call. + * + * It is safe to call this function with NULL argument or on a V4L2 connector + * the parsing of which failed. + */ +void v4l2_fwnode_connector_free(struct v4l2_fwnode_connector *connector); + +/** + * v4l2_fwnode_parse_connector() - parse the connector on endpoint + * @fwnode: pointer to the endpoint's fwnode handle where the connector is + * connected to + * @connector: pointer to the V4L2 fwnode connector data structure + * + * Fill the connector data structure with the connector type, label, all found + * links between the host and the connector as well as connector specific data. + * Since the label is optional it can be %NULL if no one was found. + * + * A reference is taken to both the connector and the connector destination + * where the connector belongs to. This must be dropped when no longer needed. + * Also the memory it has allocated to store the variable size data must be + * freed. Both dropping the references and freeing the memory is done by + * v4l2_fwnode_connector_free(). + * + * Return: + * * %0 on success or a negative error code on failure: + * * %-ENOMEM if memory allocation failed + * * %-ENOLINK if the connector can't be found + * * %-EINVAL on parsing failure + */ +int v4l2_fwnode_connector_alloc_parse(struct fwnode_handle *fwnode, + struct v4l2_fwnode_connector *connector); + /** * typedef parse_endpoint_func - Driver's callback function to be called on * each V4L2 fwnode endpoint.
The patch adds the initial connector parsing code, so we can move from a driver specific parsing code to a generic one. Currently only the generic fields and the analog-connector specific fields are parsed. Parsing the other connector specific fields can be added by a simple callbacks. Signed-off-by: Marco Felsch <m.felsch@pengutronix.de> --- [1] https://patchwork.kernel.org/cover/10794703/ v10: - drop V4L2_CONN_HDMI support - adapt pr_err msg to reflect new state (-> connector is unkown) v9: - Fix leading semicolon found by kbuild semicolon.cocci v8: - V4L2_CON_* -> V4L2_CONN_* - tvnorms -> sdtv-standards - adapt to new v4l2_fwnode_connector_analog member - return error in case of V4L2_CONN_HDMI v7: @Jacopo: I dropped your r b tag becuase of the amount of changes I made.. - drop unnecessary comments - fix commet style - s/v4l2_fwnode_connector_conv.name/v4l2_fwnode_connector_conv.compatible/ - make label size variable and drop V4L2_CONNECTOR_MAX_LABEL usage - do not assign a default label in case of no label was specified - remove useless /* fall through */ comments - add support for N connector links - rename local variables to be more meaningful - adjust kernedoc - add v4l2_fwnode_connector_free() - improve error handling (use different error values) - make use of pr_warn_once() v6: - use unsigned count var - fix comment and style issues - place '/* fall through */' to correct places - fix error handling and cleanup by releasing fwnode - drop vga and dvi parsing support as those connectors are rarely used these days v5: - s/strlcpy/strscpy/ v2-v4: - nothing since the patch was squashed from series [1] into this series. --- drivers/media/v4l2-core/v4l2-fwnode.c | 129 ++++++++++++++++++++++++++ include/media/v4l2-fwnode.h | 38 ++++++++ 2 files changed, 167 insertions(+)