Re: [PATCH v4 06/17] media: imx: mipi csi-2: Implement get_fwnode_pad op

[Steve Longerbeam <slongerbeam@gmail.com>]

Hi Laurent,

On 4/14/20 4:07 PM, Laurent Pinchart wrote:
> Hi Steve,
>
> Thank you for the patch.
>
> On Tue, Mar 03, 2020 at 03:42:45PM -0800, Steve Longerbeam wrote:
>> Implement get_fwnode_pad operation. If the endpoint is owned by the MIPI
>> CSI-2 receiver, return the endpoint's port number. The MIPI CSI-2 receiver
>> maps port numbers and pad indexes 1:1.
>>
>> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
>> ---
>>   drivers/staging/media/imx/imx6-mipi-csi2.c | 28 ++++++++++++++++++++++
>>   1 file changed, 28 insertions(+)
>>
>> diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c b/drivers/staging/media/imx/imx6-mipi-csi2.c
>> index fdd763587e6c..8500207e5ea9 100644
>> --- a/drivers/staging/media/imx/imx6-mipi-csi2.c
>> +++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
>> @@ -507,9 +507,37 @@ static int csi2_registered(struct v4l2_subdev *sd)
>>   				      640, 480, 0, V4L2_FIELD_NONE, NULL);
>>   }
>>   
>> +static int csi2_get_fwnode_pad(struct media_entity *entity,
>> +			       struct fwnode_endpoint *endpoint)
>> +{
>> +	struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
>> +	struct csi2_dev *csi2 = sd_to_dev(sd);
>> +	struct fwnode_handle *csi2_ep;
>> +
>> +	/*
>> +	 * If the endpoint is one of ours, return the endpoint's port
>> +	 * number. This device maps port numbers and pad indexes 1:1.
>> +	 */
>> +	fwnode_graph_for_each_endpoint(dev_fwnode(csi2->dev), csi2_ep) {
>> +		if (endpoint->local_fwnode == csi2_ep) {
>> +			struct fwnode_endpoint fwep;
>> +			int ret;
>> +
>> +			ret = fwnode_graph_parse_endpoint(csi2_ep, &fwep);
>> +
>> +			fwnode_handle_put(csi2_ep);
>> +
>> +			return ret ? ret : fwep.port;
>> +		}
>> +	}
>> +
>> +	return -ENXIO;
>> +}
> As the 1:1 mapping is the common case, would it make sense to modify
> media_entity_get_fwnode_pad() accordingly when .get_fwnode_pad is not
> set ?

Yes! In fact that was a previous revision of this patchset (v2). But I 
dropped that idea in v3 and v4 because it didn't seem to be getting any 
traction.

I guess I'll try again. I'll bring back [1] in v5.

Steve


[1] https://patchwork.linuxtv.org/patch/60312/

>   The current behaviour is to return the first pad that matches the
> requested direction, which could be preserved as a second-level fallback
> if the 1:1 mapping doesn't give the right direction (but I'm not sure
> there's a use case for that, the 1:1 mapping seems to be all we need if
> there's no specific .get_fwnode_pad implementation).
>
>> +
>>   static const struct media_entity_operations csi2_entity_ops = {
>>   	.link_setup = csi2_link_setup,
>>   	.link_validate = v4l2_subdev_link_validate,
>> +	.get_fwnode_pad = csi2_get_fwnode_pad,
>>   };
>>   
>>   static const struct v4l2_subdev_video_ops csi2_video_ops = {