Hi Mauro, Sakari,
On 09/26/2018 03:40 AM, Sakari Ailus wrote:
Hi Mauro, Steve,
On Wed, Sep 26, 2018 at 06:33:35AM -0300, Mauro Carvalho Chehab wrote:
Em Tue, 25 Sep 2018 18:05:36 -0700
Steve Longerbeam <steve_longerbeam@xxxxxxxxxx> escreveu:
On 09/25/2018 03:20 PM, Mauro Carvalho Chehab wrote:
Em Tue, 25 Sep 2018 14:04:21 -0700
Steve Longerbeam <steve_longerbeam@xxxxxxxxxx> escreveu:
@@ -392,12 +406,11 @@ static int __v4l2_async_notifier_register(struct v4l2_async_notifier *notifier)
case V4L2_ASYNC_MATCH_CUSTOM:
case V4L2_ASYNC_MATCH_DEVNAME:
case V4L2_ASYNC_MATCH_I2C:
- break;
case V4L2_ASYNC_MATCH_FWNODE:
- if (v4l2_async_notifier_fwnode_has_async_subdev(
- notifier, asd->match.fwnode, i)) {
+ if (v4l2_async_notifier_has_async_subdev(
+ notifier, asd, i)) {
dev_err(dev,
- "fwnode has already been registered or in notifier's subdev list\n");
+ "asd has already been registered or in notifier's subdev list\n");
Please, never use "asd" on messages printed to the user. While someone
may understand it while reading the source code, for a poor use,
"asd" is just a random sequence of 3 characters.
I will change the message to read:
"subdev descriptor already listed in this or other notifiers".
Perfect!
But the error message is removed in the subsequent patch
"[PATCH 03/17] media: v4l2: async: Add v4l2_async_notifier_add_subdev".
I could bring it back as a dev_dbg() in v4l2_async_notifier_asd_valid(), but
this shouldn't be a dev_err() anymore since it is up to the media platform
to decide whether an already existing subdev descriptor is an error.
Hmm... that's an interesting discussion... what cases do you think it
would be fine to try to register twice an asd notifier?
It should be a fairly common case that a sub-device has multiple fwnode
output ports. In that case it's possible multiple sub-devices downstream
from it will each encounter it when parsing the fwnode graph, and attempt
to add it to their notifiers asd_list multiple times. That isn't an
error, any
attempt to add it after the first add should be ignored.
imx-media is an example, there is a CSI-2 transmitter with four fwnode
output ports for each CSI-2 virtual channel. Those channels each go to
one of four Camera Sensor Interface in the imx6 IPU. So each CSI will
encounter the CSI-2 transmitter when parsing its fwnode ports.
Only the error message is removed; this case is still considered an error.
I think it'd be better to keep this error message; it helps debugging.
Ok I will add it back, but it should be a dev_dbg().
Steve
Haven't write myself any piece of code using async framework, on a first
glance, trying to register twice sounds like an error to me.
Sakari, what do you think?
