Hi Sakari,
On 09/28/2018 05:16 AM, Sakari Ailus wrote:
On Wed, Sep 26, 2018 at 10:49:18AM -0700, Steve Longerbeam wrote:
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().
Fine for me.
Could you address especially the author vs. SoB line difference in the set,
and re-post to the list, please?
Will do!
I've pushed the latest set including my fwnode patches (which you can
ignore) to my linuxtv.org tree v4l2-fwnode branch; feel free to use these
as the basis. I've fixed a few conflicts in there in rebasing on current
media tree master.
Yes in drivers/media/platform/ti-vpe/cal.c, due to
58513d4849 (" media: platform: remove redundant null pointer check
before of_node_put")
I fixed that conflict too, but in a different way that retains the above
change.
Since my conflict fix is different from yours, I will push v7 against
current
media tree master. Let me know if the fixup looks ok to you.
Steve
