* [PATCH] media: v4l2-async: Use endpoints in __v4l2_async_nf_add_fwnode_remote() @ 2022-03-21 14:51 ` Laurent Pinchart 2022-04-09 19:35 ` Kieran Bingham 2022-09-22 14:06 ` Marek Szyprowski 0 siblings, 2 replies; 7+ messages in thread From: Laurent Pinchart @ 2022-03-21 14:51 UTC (permalink / raw) To: linux-media Cc: Sakari Ailus, Jacopo Mondi, Kieran Bingham, Niklas Söderlund, Laurent Pinchart From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> Matching on device fwnode handles is deprecated in favour of endpoint fwnode handles. Switch the __v4l2_async_nf_add_fwnode_remote() function to use the latter. The match code handles backward compatibility by falling by to the device fwnode handle, so this shouldn't introduce any regression. Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> --- drivers/media/v4l2-core/v4l2-async.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c index 0404267f1ae4..67d7f445d429 100644 --- a/drivers/media/v4l2-core/v4l2-async.c +++ b/drivers/media/v4l2-core/v4l2-async.c @@ -662,7 +662,7 @@ __v4l2_async_nf_add_fwnode_remote(struct v4l2_async_notifier *notif, struct v4l2_async_subdev *asd; struct fwnode_handle *remote; - remote = fwnode_graph_get_remote_port_parent(endpoint); + remote = fwnode_graph_get_remote_endpoint(endpoint); if (!remote) return ERR_PTR(-ENOTCONN); -- Regards, Laurent Pinchart ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] media: v4l2-async: Use endpoints in __v4l2_async_nf_add_fwnode_remote() 2022-03-21 14:51 ` [PATCH] media: v4l2-async: Use endpoints in __v4l2_async_nf_add_fwnode_remote() Laurent Pinchart @ 2022-04-09 19:35 ` Kieran Bingham 2022-04-10 15:08 ` Laurent Pinchart 2022-09-22 14:06 ` Marek Szyprowski 1 sibling, 1 reply; 7+ messages in thread From: Kieran Bingham @ 2022-04-09 19:35 UTC (permalink / raw) To: Laurent Pinchart, linux-media Cc: Sakari Ailus, Jacopo Mondi, Niklas Söderlund, Laurent Pinchart Quoting Laurent Pinchart (2022-03-21 14:51:34) > From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > Matching on device fwnode handles is deprecated in favour of endpoint > fwnode handles. Switch the __v4l2_async_nf_add_fwnode_remote() function > to use the latter. The match code handles backward compatibility by > falling by to the device fwnode handle, so this shouldn't introduce any s/by/back/ ? It sounds like this helps move other devices towards endpoint matching more generically too, but I expect there will still be more specifics to handle in receiver drivers? And indeed the whole point of the fallback case was to make them compatible... so: Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> But I suspect this needs some testing on platforms that haven't yet moved to endpoint matching.. ? > regression. > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > --- > drivers/media/v4l2-core/v4l2-async.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c > index 0404267f1ae4..67d7f445d429 100644 > --- a/drivers/media/v4l2-core/v4l2-async.c > +++ b/drivers/media/v4l2-core/v4l2-async.c > @@ -662,7 +662,7 @@ __v4l2_async_nf_add_fwnode_remote(struct v4l2_async_notifier *notif, > struct v4l2_async_subdev *asd; > struct fwnode_handle *remote; > > - remote = fwnode_graph_get_remote_port_parent(endpoint); > + remote = fwnode_graph_get_remote_endpoint(endpoint); > if (!remote) > return ERR_PTR(-ENOTCONN); > > -- > Regards, > > Laurent Pinchart > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] media: v4l2-async: Use endpoints in __v4l2_async_nf_add_fwnode_remote() 2022-04-09 19:35 ` Kieran Bingham @ 2022-04-10 15:08 ` Laurent Pinchart 0 siblings, 0 replies; 7+ messages in thread From: Laurent Pinchart @ 2022-04-10 15:08 UTC (permalink / raw) To: Kieran Bingham Cc: linux-media, Sakari Ailus, Jacopo Mondi, Niklas Söderlund Hi Kieran, On Sat, Apr 09, 2022 at 08:35:48PM +0100, Kieran Bingham wrote: > Quoting Laurent Pinchart (2022-03-21 14:51:34) > > From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > > > Matching on device fwnode handles is deprecated in favour of endpoint > > fwnode handles. Switch the __v4l2_async_nf_add_fwnode_remote() function > > to use the latter. The match code handles backward compatibility by > > falling by to the device fwnode handle, so this shouldn't introduce any > > s/by/back/ ? Indeed. > It sounds like this helps move other devices towards endpoint matching > more generically too, but I expect there will still be more specifics to > handle in receiver drivers? > > And indeed the whole point of the fallback case was to make them > compatible... so: > > Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > > But I suspect this needs some testing on platforms that haven't yet > moved to endpoint matching.. ? The more testing the better :-) > > regression. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > --- > > drivers/media/v4l2-core/v4l2-async.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c > > index 0404267f1ae4..67d7f445d429 100644 > > --- a/drivers/media/v4l2-core/v4l2-async.c > > +++ b/drivers/media/v4l2-core/v4l2-async.c > > @@ -662,7 +662,7 @@ __v4l2_async_nf_add_fwnode_remote(struct v4l2_async_notifier *notif, > > struct v4l2_async_subdev *asd; > > struct fwnode_handle *remote; > > > > - remote = fwnode_graph_get_remote_port_parent(endpoint); > > + remote = fwnode_graph_get_remote_endpoint(endpoint); > > if (!remote) > > return ERR_PTR(-ENOTCONN); > > -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] media: v4l2-async: Use endpoints in __v4l2_async_nf_add_fwnode_remote() 2022-03-21 14:51 ` [PATCH] media: v4l2-async: Use endpoints in __v4l2_async_nf_add_fwnode_remote() Laurent Pinchart 2022-04-09 19:35 ` Kieran Bingham @ 2022-09-22 14:06 ` Marek Szyprowski 2022-09-22 22:48 ` Sakari Ailus 1 sibling, 1 reply; 7+ messages in thread From: Marek Szyprowski @ 2022-09-22 14:06 UTC (permalink / raw) To: Laurent Pinchart, linux-media, 'Linux Samsung SOC' Cc: Sakari Ailus, Jacopo Mondi, Kieran Bingham, Niklas Söderlund, Laurent Pinchart, Sylwester Nawrocki Hi Laurent, On 21.03.2022 15:51, Laurent Pinchart wrote: > From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > Matching on device fwnode handles is deprecated in favour of endpoint > fwnode handles. Switch the __v4l2_async_nf_add_fwnode_remote() function > to use the latter. The match code handles backward compatibility by > falling by to the device fwnode handle, so this shouldn't introduce any > regression. > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> I love the last sentence of the patch description. :) Unfortunately, recently I found that this patch breaks Exynos4 IS (FIMC) driver operation on Trats2 board (exynos4412-trats2.dts). After merging this patch I see the following errors related to the camera sensors: [ 16.038705] S5C73M3: S5C73M3 SPI probed successfully [ 16.097399] S5C73M3: Sensor type: CML0801-M017, FW version: GDFD01 [ 16.106842] S5C73M3 0-003c: Consider updating driver S5C73M3 to match on endpoints [ 16.298323] S5C73M3: probe of 0-003c failed with error -22 [ 16.343173] S5K6A3 15-0010: Consider updating driver S5K6A3 to match on endpoints [ 16.434968] S5K6A3: probe of 15-0010 failed with error -22 I'm a bit puzzled, because I don't see anything related to endpoint matching in the sensor drivers. Instead I only found that v4l2_async_nf_add_fwnode_remote() function (which is modified by this patch) is called from the drivers/media/platform/samsung/exynos4-is/media-dev.c code. Do you have any idea what is broken after this change? Could you help fixing this issue? Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] media: v4l2-async: Use endpoints in __v4l2_async_nf_add_fwnode_remote() 2022-09-22 14:06 ` Marek Szyprowski @ 2022-09-22 22:48 ` Sakari Ailus 2022-09-22 23:19 ` Daniel Scally 0 siblings, 1 reply; 7+ messages in thread From: Sakari Ailus @ 2022-09-22 22:48 UTC (permalink / raw) To: Marek Szyprowski Cc: Laurent Pinchart, linux-media, 'Linux Samsung SOC', Jacopo Mondi, Kieran Bingham, Niklas Söderlund, Laurent Pinchart, Sylwester Nawrocki Hi, Marek! Good to hear from you! And it's so nice you're testing the Samsung Exynos drivers! :-) On Thu, Sep 22, 2022 at 04:06:58PM +0200, Marek Szyprowski wrote: > Hi Laurent, > > On 21.03.2022 15:51, Laurent Pinchart wrote: > > From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > > > Matching on device fwnode handles is deprecated in favour of endpoint > > fwnode handles. Switch the __v4l2_async_nf_add_fwnode_remote() function > > to use the latter. The match code handles backward compatibility by > > falling by to the device fwnode handle, so this shouldn't introduce any > > regression. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > I love the last sentence of the patch description. :) > > Unfortunately, recently I found that this patch breaks Exynos4 IS (FIMC) > driver operation on Trats2 board (exynos4412-trats2.dts). After merging > this patch I see the following errors related to the camera sensors: > > [ 16.038705] S5C73M3: S5C73M3 SPI probed successfully > [ 16.097399] S5C73M3: Sensor type: CML0801-M017, FW version: GDFD01 > [ 16.106842] S5C73M3 0-003c: Consider updating driver S5C73M3 to match > on endpoints > [ 16.298323] S5C73M3: probe of 0-003c failed with error -22 > [ 16.343173] S5K6A3 15-0010: Consider updating driver S5K6A3 to match > on endpoints > [ 16.434968] S5K6A3: probe of 15-0010 failed with error -22 Have you checked what exactly caused the probe to fail? Laurent's patch changes how matching works but if that fails, the result should be a bunch of waiting async sub-devices and notifier(s), not a failure on probe. > > I'm a bit puzzled, because I don't see anything related to endpoint > matching in the sensor drivers. Instead I only found that > v4l2_async_nf_add_fwnode_remote() function (which is modified by this > patch) is called from the > drivers/media/platform/samsung/exynos4-is/media-dev.c code. Do you have > any idea what is broken after this change? Could you help fixing this issue? You can assign the endpoint node to subdev->fwnode instead of the device fwnode. No regular sensor driver appears to be doing that though. -- Kind regards, Sakari Ailus ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] media: v4l2-async: Use endpoints in __v4l2_async_nf_add_fwnode_remote() 2022-09-22 22:48 ` Sakari Ailus @ 2022-09-22 23:19 ` Daniel Scally 2022-09-23 9:27 ` Marek Szyprowski 0 siblings, 1 reply; 7+ messages in thread From: Daniel Scally @ 2022-09-22 23:19 UTC (permalink / raw) To: Sakari Ailus, Marek Szyprowski Cc: Laurent Pinchart, linux-media, 'Linux Samsung SOC', Jacopo Mondi, Kieran Bingham, Niklas Söderlund, Laurent Pinchart, Sylwester Nawrocki Hi all On 22/09/2022 23:48, Sakari Ailus wrote: > Hi, Marek! > > Good to hear from you! And it's so nice you're testing the Samsung Exynos > drivers! :-) > > On Thu, Sep 22, 2022 at 04:06:58PM +0200, Marek Szyprowski wrote: >> Hi Laurent, >> >> On 21.03.2022 15:51, Laurent Pinchart wrote: >>> From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> >>> >>> Matching on device fwnode handles is deprecated in favour of endpoint >>> fwnode handles. Switch the __v4l2_async_nf_add_fwnode_remote() function >>> to use the latter. The match code handles backward compatibility by >>> falling by to the device fwnode handle, so this shouldn't introduce any >>> regression. >>> >>> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> >> I love the last sentence of the patch description. :) >> >> Unfortunately, recently I found that this patch breaks Exynos4 IS (FIMC) >> driver operation on Trats2 board (exynos4412-trats2.dts). After merging >> this patch I see the following errors related to the camera sensors: >> >> [ 16.038705] S5C73M3: S5C73M3 SPI probed successfully >> [ 16.097399] S5C73M3: Sensor type: CML0801-M017, FW version: GDFD01 >> [ 16.106842] S5C73M3 0-003c: Consider updating driver S5C73M3 to match >> on endpoints >> [ 16.298323] S5C73M3: probe of 0-003c failed with error -22 >> [ 16.343173] S5K6A3 15-0010: Consider updating driver S5K6A3 to match >> on endpoints >> [ 16.434968] S5K6A3: probe of 15-0010 failed with error -22 > Have you checked what exactly caused the probe to fail? Laurent's patch > changes how matching works but if that fails, the result should be a bunch > of waiting async sub-devices and notifier(s), not a failure on probe. Might be some other effects too...I can't test it but in subdev_notifier_bound() in drivers/media/platform/samsung/exynos4-is/media-dev.c there's the following check: /* Find platform data for this sensor subdev */ for (i = 0; i < ARRAY_SIZE(fmd->sensor); i++) if (fmd->sensor[i].asd && fmd->sensor[i].asd->match.fwnode == of_fwnode_handle(subdev->dev->of_node)) si = &fmd->sensor[i]; if (si == NULL) return -EINVAL; And I think following that patch of Laurent's asd->match.fwnode will never be the dev->of_node anymore, because it's now the endpoint that's assigned as asd->match.fwnode rather than that of the device. That will return -EINVAL for the notifier's .bound() callback...I'm not sure if that would cause the whole probe to fail, but thought it might be worth mentioning :) > >> I'm a bit puzzled, because I don't see anything related to endpoint >> matching in the sensor drivers. Instead I only found that >> v4l2_async_nf_add_fwnode_remote() function (which is modified by this >> patch) is called from the >> drivers/media/platform/samsung/exynos4-is/media-dev.c code. Do you have >> any idea what is broken after this change? Could you help fixing this issue? > You can assign the endpoint node to subdev->fwnode instead of the device > fwnode. No regular sensor driver appears to be doing that though. > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] media: v4l2-async: Use endpoints in __v4l2_async_nf_add_fwnode_remote() 2022-09-22 23:19 ` Daniel Scally @ 2022-09-23 9:27 ` Marek Szyprowski 0 siblings, 0 replies; 7+ messages in thread From: Marek Szyprowski @ 2022-09-23 9:27 UTC (permalink / raw) To: Daniel Scally, Sakari Ailus Cc: Laurent Pinchart, linux-media, 'Linux Samsung SOC', Jacopo Mondi, Kieran Bingham, Niklas Söderlund, Laurent Pinchart, Sylwester Nawrocki Hi Daniel, On 23.09.2022 01:19, Daniel Scally wrote: > On 22/09/2022 23:48, Sakari Ailus wrote: >> Good to hear from you! And it's so nice you're testing the Samsung >> Exynos >> drivers! :-) >> >> On Thu, Sep 22, 2022 at 04:06:58PM +0200, Marek Szyprowski wrote: >>> On 21.03.2022 15:51, Laurent Pinchart wrote: >>>> From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> >>>> >>>> Matching on device fwnode handles is deprecated in favour of endpoint >>>> fwnode handles. Switch the __v4l2_async_nf_add_fwnode_remote() >>>> function >>>> to use the latter. The match code handles backward compatibility by >>>> falling by to the device fwnode handle, so this shouldn't introduce >>>> any >>>> regression. >>>> >>>> Signed-off-by: Laurent Pinchart >>>> <laurent.pinchart+renesas@ideasonboard.com> >>> I love the last sentence of the patch description. :) >>> >>> Unfortunately, recently I found that this patch breaks Exynos4 IS >>> (FIMC) >>> driver operation on Trats2 board (exynos4412-trats2.dts). After merging >>> this patch I see the following errors related to the camera sensors: >>> >>> [ 16.038705] S5C73M3: S5C73M3 SPI probed successfully >>> [ 16.097399] S5C73M3: Sensor type: CML0801-M017, FW version: GDFD01 >>> [ 16.106842] S5C73M3 0-003c: Consider updating driver S5C73M3 to >>> match >>> on endpoints >>> [ 16.298323] S5C73M3: probe of 0-003c failed with error -22 >>> [ 16.343173] S5K6A3 15-0010: Consider updating driver S5K6A3 to match >>> on endpoints >>> [ 16.434968] S5K6A3: probe of 15-0010 failed with error -22 >> Have you checked what exactly caused the probe to fail? Laurent's patch >> changes how matching works but if that fails, the result should be a >> bunch >> of waiting async sub-devices and notifier(s), not a failure on probe. > > > Might be some other effects too...I can't test it but in > subdev_notifier_bound() in > drivers/media/platform/samsung/exynos4-is/media-dev.c there's the > following check: > > > /* Find platform data for this sensor subdev */ > for (i = 0; i < ARRAY_SIZE(fmd->sensor); i++) > if (fmd->sensor[i].asd && > fmd->sensor[i].asd->match.fwnode == > of_fwnode_handle(subdev->dev->of_node)) > si = &fmd->sensor[i]; > > if (si == NULL) > return -EINVAL; > > > And I think following that patch of Laurent's asd->match.fwnode will > never be the dev->of_node anymore, because it's now the endpoint > that's assigned as asd->match.fwnode rather than that of the device. > That will return -EINVAL for the notifier's .bound() callback...I'm > not sure if that would cause the whole probe to fail, but thought it > might be worth mentioning :) Good catch, thanks! I have no idea why that loop compared the asd->match.fwnode and subdev->dev->of_node, maybe it some kind of historical left-over or cargo-cult. The asd pointer is already available there and can be used to find the proper sensor. This loop works both with the new and old version: /* Find platform data for this sensor subdev */ for (i = 0; i < ARRAY_SIZE(fmd->sensor); i++) if (fmd->sensor[i].asd == asd) si = &fmd->sensor[i]; I will prepare a patch in a few minutes. >>> I'm a bit puzzled, because I don't see anything related to endpoint >>> matching in the sensor drivers. Instead I only found that >>> v4l2_async_nf_add_fwnode_remote() function (which is modified by this >>> patch) is called from the >>> drivers/media/platform/samsung/exynos4-is/media-dev.c code. Do you have >>> any idea what is broken after this change? Could you help fixing >>> this issue? >> You can assign the endpoint node to subdev->fwnode instead of the device >> fwnode. No regular sensor driver appears to be doing that though. >> > Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-09-23 9:28 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20220922140658eucas1p29f9d609e795ed064e51694c0e7be80d3@eucas1p2.samsung.com>
2022-03-21 14:51 ` [PATCH] media: v4l2-async: Use endpoints in __v4l2_async_nf_add_fwnode_remote() Laurent Pinchart
2022-04-09 19:35 ` Kieran Bingham
2022-04-10 15:08 ` Laurent Pinchart
2022-09-22 14:06 ` Marek Szyprowski
2022-09-22 22:48 ` Sakari Ailus
2022-09-22 23:19 ` Daniel Scally
2022-09-23 9:27 ` Marek Szyprowski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox