v4l parent for usb device interface or device?

v4l parent for usb device interface or device?

[Hans de Goede]

<take 2 this time to the new list, hoping it gets some more attention>

Hi,

Today it came to my attention (through a libv4l bugreport) that
the uvc driver and the gspca driver handle the setting of
the v4l parent for usb webcams differently.

The probe function for an usb driver gets passed in a
"struct usb_interface *intf" parameter.

uvc sets parent to:

vdev->parent = &intf->dev;

gspca uses:
struct usb_device *dev = interface_to_usbdev(intf);
vdev.parent = &dev->dev;

Looking at what for example the usb mass-storage driver
does (with my multi function printer/scanner with cardreader),
which matches UVC, and thinking about how this is supposed to
work with multifunction devices in general, I believe the uvc
driver behaviour is correct, but before writing a patch for
gspca, I thought it would be good to first discuss this on the
list.

So what do you think ?

Thanks & Regards,

Hans


p.s.

This mainly influences what the /sys/class/video4linux/video#/device
symlink will point to, which libv4l uses.

Re: v4l parent for usb device interface or device?

[Laurent Pinchart]

Hi Hans,

On Wednesday 25 March 2009 11:18:31 Hans de Goede wrote:
> <take 2 this time to the new list, hoping it gets some more attention>
>
> Hi,
>
> Today it came to my attention (through a libv4l bugreport) that
> the uvc driver and the gspca driver handle the setting of
> the v4l parent for usb webcams differently.
>
> The probe function for an usb driver gets passed in a
> "struct usb_interface *intf" parameter.
>
> uvc sets parent to:
>
> vdev->parent = &intf->dev;
>
> gspca uses:
> struct usb_device *dev = interface_to_usbdev(intf);
> vdev.parent = &dev->dev;
>
> Looking at what for example the usb mass-storage driver
> does (with my multi function printer/scanner with cardreader),
> which matches UVC, and thinking about how this is supposed to
> work with multifunction devices in general, I believe the uvc
> driver behaviour is correct, but before writing a patch for
> gspca, I thought it would be good to first discuss this on the
> list.
>
> So what do you think ?

I obviously agree with you :-)

USB class drivers bind to interfaces instead of devices to support composite 
(multifunction) devices. While drivers for vendor-specific USB devices can 
bind to the device, in which case the parent could be a USB device, we need to 
have some consistency in the sysfs symlinks. Using a USB interface as the 
video device parent regardless of the device type makes sense.

Best regards,

Laurent Pinchart

Re: v4l parent for usb device interface or device?

[Hans Verkuil]

> Hi Hans,
>
> On Wednesday 25 March 2009 11:18:31 Hans de Goede wrote:
>> <take 2 this time to the new list, hoping it gets some more attention>
>>
>> Hi,
>>
>> Today it came to my attention (through a libv4l bugreport) that
>> the uvc driver and the gspca driver handle the setting of
>> the v4l parent for usb webcams differently.
>>
>> The probe function for an usb driver gets passed in a
>> "struct usb_interface *intf" parameter.
>>
>> uvc sets parent to:
>>
>> vdev->parent = &intf->dev;
>>
>> gspca uses:
>> struct usb_device *dev = interface_to_usbdev(intf);
>> vdev.parent = &dev->dev;
>>
>> Looking at what for example the usb mass-storage driver
>> does (with my multi function printer/scanner with cardreader),
>> which matches UVC, and thinking about how this is supposed to
>> work with multifunction devices in general, I believe the uvc
>> driver behaviour is correct, but before writing a patch for
>> gspca, I thought it would be good to first discuss this on the
>> list.
>>
>> So what do you think ?
>
> I obviously agree with you :-)
>
> USB class drivers bind to interfaces instead of devices to support
> composite
> (multifunction) devices. While drivers for vendor-specific USB devices can
> bind to the device, in which case the parent could be a USB device, we
> need to
> have some consistency in the sysfs symlinks. Using a USB interface as the
> video device parent regardless of the device type makes sense.

If the parent should indeed become the usb_interface, then we should make
all v4l usb drivers consistent. And update v4l2-framework.txt. I've
noticed before that it seems to be random what is used as the parent. I'm
no USB expert, so I'm relying on your input.

Regards,

       Hans

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG

Re: v4l parent for usb device interface or device?

[Hans de Goede]

On 03/25/2009 03:58 PM, Hans Verkuil wrote:
>> Hi Hans,
>>
>> On Wednesday 25 March 2009 11:18:31 Hans de Goede wrote:
>>> <take 2 this time to the new list, hoping it gets some more attention>
>>>
>>> Hi,
>>>
>>> Today it came to my attention (through a libv4l bugreport) that
>>> the uvc driver and the gspca driver handle the setting of
>>> the v4l parent for usb webcams differently.
>>>
>>> The probe function for an usb driver gets passed in a
>>> "struct usb_interface *intf" parameter.
>>>
>>> uvc sets parent to:
>>>
>>> vdev->parent =&intf->dev;
>>>
>>> gspca uses:
>>> struct usb_device *dev = interface_to_usbdev(intf);
>>> vdev.parent =&dev->dev;
>>>
>>> Looking at what for example the usb mass-storage driver
>>> does (with my multi function printer/scanner with cardreader),
>>> which matches UVC, and thinking about how this is supposed to
>>> work with multifunction devices in general, I believe the uvc
>>> driver behaviour is correct, but before writing a patch for
>>> gspca, I thought it would be good to first discuss this on the
>>> list.
>>>
>>> So what do you think ?
>> I obviously agree with you :-)
>>
>> USB class drivers bind to interfaces instead of devices to support
>> composite
>> (multifunction) devices. While drivers for vendor-specific USB devices can
>> bind to the device, in which case the parent could be a USB device, we
>> need to
>> have some consistency in the sysfs symlinks. Using a USB interface as the
>> video device parent regardless of the device type makes sense.
>
> If the parent should indeed become the usb_interface, then we should make
> all v4l usb drivers consistent. And update v4l2-framework.txt. I've
> noticed before that it seems to be random what is used as the parent. I'm
> no USB expert, so I'm relying on your input.
>

I believe that what uvc is doing, is the right thing. USB explicitly allows for
multi-function devices, where each function has a seperate interface. So far
example a still camera, with a webcam mode, could have 2 interfaces, a mass
storage interface for the sdcard which stores the pictures and a foo interface,
for the webcam mode. Clearly the right parent for the webcam v4l device then is
the foo interface, and not the entire device (just like the mass storage driver
will use the other interface as its parent).

I think writing some docs about this and making all drivers consistent wrt this,
is a good plan. I will write a driver to make gspca the right thing.

Regards,

Hans

Re: v4l parent for usb device interface or device?

[Theodore Kilgore]

On Thu, 26 Mar 2009, Hans de Goede wrote:

>
>
> On 03/25/2009 03:58 PM, Hans Verkuil wrote:
>>> Hi Hans,
>>> 
>>> On Wednesday 25 March 2009 11:18:31 Hans de Goede wrote:
>>>> <take 2 this time to the new list, hoping it gets some more attention>
>>>> 
>>>> Hi,
>>>> 
>>>> Today it came to my attention (through a libv4l bugreport) that
>>>> the uvc driver and the gspca driver handle the setting of
>>>> the v4l parent for usb webcams differently.
>>>> 
>>>> The probe function for an usb driver gets passed in a
>>>> "struct usb_interface *intf" parameter.
>>>> 
>>>> uvc sets parent to:
>>>> 
>>>> vdev->parent =&intf->dev;
>>>> 
>>>> gspca uses:
>>>> struct usb_device *dev = interface_to_usbdev(intf);
>>>> vdev.parent =&dev->dev;
>>>> 
>>>> Looking at what for example the usb mass-storage driver
>>>> does (with my multi function printer/scanner with cardreader),
>>>> which matches UVC, and thinking about how this is supposed to
>>>> work with multifunction devices in general, I believe the uvc
>>>> driver behaviour is correct, but before writing a patch for
>>>> gspca, I thought it would be good to first discuss this on the
>>>> list.
>>>> 
>>>> So what do you think ?
>>> I obviously agree with you :-)
>>> 
>>> USB class drivers bind to interfaces instead of devices to support
>>> composite
>>> (multifunction) devices. While drivers for vendor-specific USB devices can
>>> bind to the device, in which case the parent could be a USB device, we
>>> need to
>>> have some consistency in the sysfs symlinks. Using a USB interface as the
>>> video device parent regardless of the device type makes sense.
>> 
>> If the parent should indeed become the usb_interface, then we should make
>> all v4l usb drivers consistent. And update v4l2-framework.txt. I've
>> noticed before that it seems to be random what is used as the parent. I'm
>> no USB expert, so I'm relying on your input.
>> 
>
> I believe that what uvc is doing, is the right thing. USB explicitly allows 
> for
> multi-function devices, where each function has a seperate interface. So far
> example a still camera, with a webcam mode, could have 2 interfaces, a mass
> storage interface for the sdcard which stores the pictures and a foo 
> interface,
> for the webcam mode. Clearly the right parent for the webcam v4l device then 
> is
> the foo interface, and not the entire device (just like the mass storage 
> driver
> will use the other interface as its parent).
>
> I think writing some docs about this and making all drivers consistent wrt 
> this,
> is a good plan. I will write a driver to make gspca the right thing.
>
> Regards,
>
> Hans

If it is possible to make the kernel claim the _interface_ and not the 
entire device, this would solve a lot of ancient problems. For example, 
the problem of dual-mode cameras, which are supported as still cameras in 
userspace and in streaming mode, by a kernel module. For another example, 
the problem of those USB DSL modems and similar devices, which come up one 
way as mass storage devices (so one can put config files on them and such) 
and the other way as modems.

If these problems can be solved, then it is really great, and any effort 
spent is well spent.

Good luck, and if I could do anything at all to help, I would be very glad 
to participate.

Theodore Kilgore

Re: v4l parent for usb device interface or device?

[Janne Grunau]

[cc-ing linux-usb]

On Thu, Mar 26, 2009 at 01:29:57PM +0100, Hans de Goede wrote:
> 
> On 03/25/2009 03:58 PM, Hans Verkuil wrote:
> >> Hi Hans,
> >>
> >> On Wednesday 25 March 2009 11:18:31 Hans de Goede wrote:
> >>> <take 2 this time to the new list, hoping it gets some more attention>
> >>>
> >>> Hi,
> >>>
> >>> Today it came to my attention (through a libv4l bugreport) that
> >>> the uvc driver and the gspca driver handle the setting of
> >>> the v4l parent for usb webcams differently.
> >>>
> >>> The probe function for an usb driver gets passed in a
> >>> "struct usb_interface *intf" parameter.
> >>>
> >>> uvc sets parent to:
> >>>
> >>> vdev->parent =&intf->dev;
> >>>
> >>> gspca uses:
> >>> struct usb_device *dev = interface_to_usbdev(intf);
> >>> vdev.parent =&dev->dev;
> >>>
> >>> Looking at what for example the usb mass-storage driver
> >>> does (with my multi function printer/scanner with cardreader),
> >>> which matches UVC, and thinking about how this is supposed to
> >>> work with multifunction devices in general, I believe the uvc
> >>> driver behaviour is correct, but before writing a patch for
> >>> gspca, I thought it would be good to first discuss this on the
> >>> list.
> >>>
> >>> So what do you think ?
> >> I obviously agree with you :-)
> >>
> >> USB class drivers bind to interfaces instead of devices to support
> >> composite
> >> (multifunction) devices. While drivers for vendor-specific USB devices can
> >> bind to the device, in which case the parent could be a USB device, we
> >> need to
> >> have some consistency in the sysfs symlinks. Using a USB interface as the
> >> video device parent regardless of the device type makes sense.
> >
> > If the parent should indeed become the usb_interface, then we should make
> > all v4l usb drivers consistent. And update v4l2-framework.txt. I've
> > noticed before that it seems to be random what is used as the parent. I'm
> > no USB expert, so I'm relying on your input.
> >
> 
> I believe that what uvc is doing, is the right thing. USB explicitly allows for
> multi-function devices, where each function has a seperate interface. So far
> example a still camera, with a webcam mode, could have 2 interfaces, a mass
> storage interface for the sdcard which stores the pictures and a foo interface,
> for the webcam mode. Clearly the right parent for the webcam v4l device then is
> the foo interface, and not the entire device (just like the mass storage driver
> will use the other interface as its parent).
> 
> I think writing some docs about this and making all drivers consistent wrt this,
> is a good plan. I will write a driver to make gspca the right thing.
> 
While I generally agree that setting parent to the usb interface is
probably correct for multifunction devices I noticed a problem after
changing the hdpvr driver accordingly.

With parent set to the usb interface there is no longer easy access to
the usb device properties like the serial number through sysfs. I know
that a couple of user with more than one device use the serial number
to set static device nodes through udev.

Regards,

Janne

Re: v4l parent for usb device interface or device?

[David Brownell]

On Thursday 26 March 2009, Janne Grunau wrote:
> 
> While I generally agree that setting parent to the usb interface is
> probably correct for multifunction devices

Make that "all" devices.  :)


> I noticed a problem after 
> changing the hdpvr driver accordingly.
> 
> With parent set to the usb interface there is no longer easy access to
> the usb device properties like the serial number through sysfs. I know
> that a couple of user with more than one device use the serial number
> to set static device nodes through udev.

The serial number is still available, but it's coupled to the USB
device not its interface.  Make your udev script hop up a level or
two in the driver model tree, as appropriate.

Re: v4l parent for usb device interface or device?

[Janne Grunau]

On Thu, Mar 26, 2009 at 06:31:55PM -0700, David Brownell wrote:
> On Thursday 26 March 2009, Janne Grunau wrote:
> 
> > I noticed a problem after 
> > changing the hdpvr driver accordingly.
> > 
> > With parent set to the usb interface there is no longer easy access to
> > the usb device properties like the serial number through sysfs. I know
> > that a couple of user with more than one device use the serial number
> > to set static device nodes through udev.
> 
> The serial number is still available, but it's coupled to the USB
> device not its interface.  Make your udev script hop up a level or
> two in the driver model tree, as appropriate.

yes, ATTRS{} still matches. There are udev howtos around which suggest
ATTRS{} just matches one level up and not the entire path. I tried it but
had unfortunately a typo in the udev rule. Sorry for the noise.

Janne