| Message ID | 20200413011416.2355-2-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] media: staging/imx: Don't assume OF port id equals pad index | expand |
Hi Laurent,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v5.7-rc1 next-20200413]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Laurent-Pinchart/media-imx6-Support-complex-external-topologies/20200413-091602
base: git://linuxtv.org/media_tree.git master
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
cppcheck warnings: (new ones prefixed by >>)
drivers/staging/media/imx/imx-media-of.c:169:6: warning: The scope of the variable 'ret' can be reduced. [variableScope]
int ret;
^
>> drivers/staging/media/imx/imx-media-of.c:110:5: warning: Clarify calculation precedence for '&' and '?'. [clarifyCalculation]
? MEDIA_PAD_FL_SOURCE : MEDIA_PAD_FL_SINK;
^
drivers/staging/media/imx/imx-media-of.c:130:38: warning: Clarify calculation precedence for '&' and '?'. [clarifyCalculation]
pad_flags & MEDIA_PAD_FL_SOURCE ?
^
vim +110 drivers/staging/media/imx/imx-media-of.c
77
78 /*
79 * Create a single media link to/from sd using a fwnode link.
80 *
81 * NOTE: this function assumes that an OF endpoint node is equivalent to a
82 * media link.
83 */
84 static int create_of_link(struct imx_media_dev *imxmd,
85 struct v4l2_subdev *sd,
86 struct v4l2_fwnode_link *link)
87 {
88 struct v4l2_subdev *remote, *src, *sink;
89 int src_pad, sink_pad;
90 int remote_pad;
91 u32 pad_flags;
92
93 if (link->local_port >= sd->entity.num_pads)
94 return -EINVAL;
95
96 remote = imx_media_find_subdev_by_fwnode(imxmd, link->remote_node);
97 if (!remote)
98 return 0;
99
100 /*
101 * Find the remote pad. Try the pad corresponding to the fwnode port id
102 * first. If its direction doesn't correspond to what we expect, use the
103 * first pad that has the right direction.
104 *
105 * FIXME: Media entities should provide an operation to translate from
106 * fwnode port id to pad index.
107 */
108 pad_flags = sd->entity.pads[link->local_port].flags;
109 pad_flags = pad_flags & MEDIA_PAD_FL_SINK
> 110 ? MEDIA_PAD_FL_SOURCE : MEDIA_PAD_FL_SINK;
111
112 if (link->remote_port < remote->entity.num_pads &&
113 remote->entity.pads[link->remote_port].flags & pad_flags) {
114 remote_pad = link->remote_port;
115 } else {
116 unsigned int i;
117
118 remote_pad = -1;
119 for (i = 0; i < remote->entity.num_pads; ++i) {
120 if (remote->entity.pads[i].flags & pad_flags) {
121 remote_pad = i;
122 break;
123 }
124 }
125
126 if (remote_pad == -1) {
127 v4l2_err(sd->v4l2_dev,
128 "remote entity %s has no %s pad\n",
129 remote->name,
130 pad_flags & MEDIA_PAD_FL_SOURCE ?
131 "source" : "sink");
132 return -EINVAL;
133 }
134 }
135
136 /* Mad the local and remote entities to source and sink. */
137 if (pad_flags & MEDIA_PAD_FL_SOURCE) {
138 src = remote;
139 src_pad = remote_pad;
140 sink = sd;
141 sink_pad = link->local_port;
142 } else {
143 src = sd;
144 src_pad = link->local_port;
145 sink = remote;
146 sink_pad = remote_pad;
147 }
148
149 /* Make sure link doesn't already exist before creating it. */
150 if (media_entity_find_link(&src->entity.pads[src_pad],
151 &sink->entity.pads[sink_pad]))
152 return 0;
153
154 v4l2_info(sd->v4l2_dev, "%s:%d -> %s:%d\n",
155 src->name, src_pad, sink->name, sink_pad);
156
157 return media_create_pad_link(&src->entity, src_pad,
158 &sink->entity, sink_pad, 0);
159 }
160
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c index 66468326bcbc..d275decc79be 100644 --- a/drivers/staging/media/imx/imx-media-csi.c +++ b/drivers/staging/media/imx/imx-media-csi.c @@ -199,10 +199,16 @@ static int csi_get_upstream_endpoint(struct csi_priv *priv, sd = media_entity_to_v4l2_subdev(pad->entity); /* - * NOTE: this assumes an OF-graph port id is the same as a - * media pad index. + * Find the port corresponding to the pad. Start by assuming that the + * port id is equal to the pad index. If there's no such port, use the + * first port. + * + * FIXME: Media entities should provide an operation to translate from + * pad index to fwnode port id. */ port = of_graph_get_port_by_id(sd->dev->of_node, pad->index); + if (!port) + port = of_graph_get_port_by_id(sd->dev->of_node, 0); if (!port) return -ENODEV; diff --git a/drivers/staging/media/imx/imx-media-of.c b/drivers/staging/media/imx/imx-media-of.c index 2d3efd2a6dde..f07bd05b8fa5 100644 --- a/drivers/staging/media/imx/imx-media-of.c +++ b/drivers/staging/media/imx/imx-media-of.c @@ -78,9 +78,8 @@ EXPORT_SYMBOL_GPL(imx_media_add_of_subdevs); /* * Create a single media link to/from sd using a fwnode link. * - * NOTE: this function assumes an OF port node is equivalent to - * a media pad (port id equal to media pad index), and that an - * OF endpoint node is equivalent to a media link. + * NOTE: this function assumes that an OF endpoint node is equivalent to a + * media link. */ static int create_of_link(struct imx_media_dev *imxmd, struct v4l2_subdev *sd, @@ -88,6 +87,8 @@ static int create_of_link(struct imx_media_dev *imxmd, { struct v4l2_subdev *remote, *src, *sink; int src_pad, sink_pad; + int remote_pad; + u32 pad_flags; if (link->local_port >= sd->entity.num_pads) return -EINVAL; @@ -96,19 +97,56 @@ static int create_of_link(struct imx_media_dev *imxmd, if (!remote) return 0; - if (sd->entity.pads[link->local_port].flags & MEDIA_PAD_FL_SINK) { + /* + * Find the remote pad. Try the pad corresponding to the fwnode port id + * first. If its direction doesn't correspond to what we expect, use the + * first pad that has the right direction. + * + * FIXME: Media entities should provide an operation to translate from + * fwnode port id to pad index. + */ + pad_flags = sd->entity.pads[link->local_port].flags; + pad_flags = pad_flags & MEDIA_PAD_FL_SINK + ? MEDIA_PAD_FL_SOURCE : MEDIA_PAD_FL_SINK; + + if (link->remote_port < remote->entity.num_pads && + remote->entity.pads[link->remote_port].flags & pad_flags) { + remote_pad = link->remote_port; + } else { + unsigned int i; + + remote_pad = -1; + for (i = 0; i < remote->entity.num_pads; ++i) { + if (remote->entity.pads[i].flags & pad_flags) { + remote_pad = i; + break; + } + } + + if (remote_pad == -1) { + v4l2_err(sd->v4l2_dev, + "remote entity %s has no %s pad\n", + remote->name, + pad_flags & MEDIA_PAD_FL_SOURCE ? + "source" : "sink"); + return -EINVAL; + } + } + + /* Mad the local and remote entities to source and sink. */ + if (pad_flags & MEDIA_PAD_FL_SOURCE) { src = remote; - src_pad = link->remote_port; + src_pad = remote_pad; sink = sd; sink_pad = link->local_port; } else { src = sd; src_pad = link->local_port; sink = remote; - sink_pad = link->remote_port; + sink_pad = remote_pad; } - /* make sure link doesn't already exist before creating */ + /* Make sure link doesn't already exist before creating it. */ if (media_entity_find_link(&src->entity.pads[src_pad], &sink->entity.pads[sink_pad])) return 0;
When parsing the graph and linking entities, the driver assumes that an OF port if is always equal to the entity pad index. This isn't a valid assumption, as for instance a CSI-2 source could be a SMIA++-compatible sensor that creates two or more subdevs but has a single DT node. Media entities should provide an operation to map port ids to pad indexes. Until this is available, work around the issue locally by picking the first pad that has the expected direction if the pad pointed to by the port id doesn't have the right direction. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- drivers/staging/media/imx/imx-media-csi.c | 10 ++++- drivers/staging/media/imx/imx-media-of.c | 52 ++++++++++++++++++++--- 2 files changed, 53 insertions(+), 9 deletions(-)