diff mbox series

[v2,1/5] media: cadence: csi2rx: use match fwnode for media link

Message ID 20231218095604.1950737-2-julien.massot@collabora.com (mailing list archive)
State New, archived
Headers show
Series misc improvements for j721e-csi2rx | expand

Commit Message

Julien Massot Dec. 18, 2023, 9:56 a.m. UTC
On some subdev the fwnode is the device node and not the endpoint node.
Using the subdev fwnode doesn't allow to fetch the subdev
pad we are connected to when the remote subdev has multiple
output pads.

Fixes: 1fc3b37f34f69 ("media: v4l: cadence: Add Cadence MIPI-CSI2 RX driver")
Signed-off-by: Julien Massot <julien.massot@collabora.com>
Reviewed-by: Jai Luthra <j-luthra@ti.com>
---
 drivers/media/platform/cadence/cdns-csi2rx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Sakari Ailus Dec. 18, 2023, 11:09 a.m. UTC | #1
Hi Julien,

Thanks for the patch.

On Mon, Dec 18, 2023 at 10:56:00AM +0100, Julien Massot wrote:
> On some subdev the fwnode is the device node and not the endpoint node.
> Using the subdev fwnode doesn't allow to fetch the subdev
> pad we are connected to when the remote subdev has multiple
> output pads.

I'd change the commit message, endpoint-to-endpoint matching used to be the
preferred way to do things not long ago.

> 
> Fixes: 1fc3b37f34f69 ("media: v4l: cadence: Add Cadence MIPI-CSI2 RX driver")

I think the driver was originally fine but I missed making the below change
when changing async matching. Instead I think you should have here:

Fixes: 1029939b3782 ("media: v4l: async: Simplify async sub-device fwnode matching")

IOW applying your patch without 1029939b3782 will also break cdns-csi2rx
driver.

> Signed-off-by: Julien Massot <julien.massot@collabora.com>
> Reviewed-by: Jai Luthra <j-luthra@ti.com>
> ---
>  drivers/media/platform/cadence/cdns-csi2rx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c
> index 91ef22e9109a..e83b877c48da 100644
> --- a/drivers/media/platform/cadence/cdns-csi2rx.c
> +++ b/drivers/media/platform/cadence/cdns-csi2rx.c
> @@ -467,7 +467,7 @@ static int csi2rx_async_bound(struct v4l2_async_notifier *notifier,
>  	struct csi2rx_priv *csi2rx = v4l2_subdev_to_csi2rx(subdev);
>  
>  	csi2rx->source_pad = media_entity_get_fwnode_pad(&s_subdev->entity,
> -							 s_subdev->fwnode,
> +							 asd->match.fwnode,
>  							 MEDIA_PAD_FL_SOURCE);
>  	if (csi2rx->source_pad < 0) {
>  		dev_err(csi2rx->dev, "Couldn't find output pad for subdev %s\n",
Julien Massot Jan. 5, 2024, 9:04 a.m. UTC | #2
Hi Sakari

On 12/18/23 12:09, Sakari Ailus wrote:
> Hi Julien,
> 
> Thanks for the patch.
> 
> On Mon, Dec 18, 2023 at 10:56:00AM +0100, Julien Massot wrote:
>> On some subdev the fwnode is the device node and not the endpoint node.
>> Using the subdev fwnode doesn't allow to fetch the subdev
>> pad we are connected to when the remote subdev has multiple
>> output pads.
> 
> I'd change the commit message, endpoint-to-endpoint matching used to be the
> preferred way to do things not long ago.
> 
>>
>> Fixes: 1fc3b37f34f69 ("media: v4l: cadence: Add Cadence MIPI-CSI2 RX driver")
> 
> I think the driver was originally fine but I missed making the below change
> when changing async matching. Instead I think you should have here:
> 
> Fixes: 1029939b3782 ("media: v4l: async: Simplify async sub-device fwnode matching")
> 
> IOW applying your patch without 1029939b3782 will also break cdns-csi2rx
> driver.

I just replaced the fix tag, and the commit message as suggested in V3.

Thank you,
Julien
diff mbox series

Patch

diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c
index 91ef22e9109a..e83b877c48da 100644
--- a/drivers/media/platform/cadence/cdns-csi2rx.c
+++ b/drivers/media/platform/cadence/cdns-csi2rx.c
@@ -467,7 +467,7 @@  static int csi2rx_async_bound(struct v4l2_async_notifier *notifier,
 	struct csi2rx_priv *csi2rx = v4l2_subdev_to_csi2rx(subdev);
 
 	csi2rx->source_pad = media_entity_get_fwnode_pad(&s_subdev->entity,
-							 s_subdev->fwnode,
+							 asd->match.fwnode,
 							 MEDIA_PAD_FL_SOURCE);
 	if (csi2rx->source_pad < 0) {
 		dev_err(csi2rx->dev, "Couldn't find output pad for subdev %s\n",