Re: [PATCH] media: intel/ipu6: set the bus_info of the v4l2_capability

[Bingbu Cao <bingbu.cao@linux.intel.com>]

Sakari,

On 2/12/25 5:39 PM, Sakari Ailus wrote:
> Hi Bingbu,
> 
> On Wed, Feb 12, 2025 at 05:20:54PM +0800, Bingbu Cao wrote:
>>
>> On 2/12/25 5:14 PM, Bingbu Cao wrote:
>>>
>>>
>>> On 2/12/25 4:25 PM, Sakari Ailus wrote:
>>>> Hi Bingbu,
>>>>
>>>> Thanks for the patch.
>>>>
>>>> On Wed, Feb 12, 2025 at 03:53:14PM +0800, bingbu.cao@intel.com wrote:
>>>>> From: Bingbu Cao <bingbu.cao@intel.com>
>>>>>
>>>>> IPU6 isys driver missed setting the bus_info of its v4l2_capability.
>>>>> `v4l2-ctl --all` cannot show the bus_info. This patch copy the bus_info
>>>>> from the media device to fill the v4l2_capability.
>>>>>
>>>>> Fixes: 3c1dfb5a69cf ("media: intel/ipu6: input system video nodes and buffer queues")
>>>>> Signed-off-by: Hidenori Kobayashi <hidenorik@chromium.org>
>>>>> Signed-off-by: Bingbu Cao <bingbu.cao@intel.com>
>>>>> ---
>>>>>  drivers/media/pci/intel/ipu6/ipu6-isys-video.c | 2 ++
>>>>>  1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-video.c b/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
>>>>> index 387963529adb..3ca3f44da387 100644
>>>>> --- a/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
>>>>> +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
>>>>> @@ -146,6 +146,8 @@ static int ipu6_isys_vidioc_querycap(struct file *file, void *fh,
>>>>>  
>>>>>  	strscpy(cap->driver, IPU6_ISYS_NAME, sizeof(cap->driver));
>>>>>  	strscpy(cap->card, av->isys->media_dev.model, sizeof(cap->card));
>>>>> +	strscpy(cap->bus_info, av->isys->media_dev.bus_info,
>>>>> +		sizeof(cap->bus_info));
>>>>
>>>> Is there a need to do this? The bus_info is set by the framework based on
>>>> struct video_device dev_parent field and that comes from struct v4l2_device
>>>> dev field.
>>>>
>>>
>>> The v4l2_device.dev filed is set to the dev_parent which is auxdev.dev
>>> instead of pdev. So the bus_info was not set by framework.
>>>
>>
>> I remember we made a change in isys_register_devices() :
>>
>> - 	ret = v4l2_device_register(dev->parent, &isys->v4l2_dev);
>> +	ret = v4l2_device_register(dev, &isys->v4l2_dev);
> 
> Right. I vaguely recall there were changes in device assignments over the
> course of upstreaming, due to the sub-device driver module refcounting. So
> I think this needs to stay as-is.
> 
> Instead, would assigning the dev_parent field in struct video_device
> achieve this or would there be side effects? The other alternative is to
> use media_set_bus_info().
>

I prefer assigning the dev_parent. I did some simple test and did not
observe side effects. Let me do more tests and then send v2.

-- 
Best regards,
Bingbu Cao