| Message ID | 20250212075314.2291135-1-bingbu.cao@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | media: intel/ipu6: set the bus_info of the v4l2_capability | expand |
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. > > return 0; > }
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. >> >> return 0; >> } >
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.
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)); return 0; }