| Message ID | alpine.LNX.2.00.0908011635020.26881@banach.math.auburn.edu (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Sat, 1 Aug 2009 16:56:06 -0500 (CDT) Theodore Kilgore <kilgota@banach.math.auburn.edu> wrote: > Several cameras are supported here, which all share the same USB > Vendor:Product number when started up in streaming mode. All of these > cameras use bulk transport for streaming, and all of them produce > frames in JPEG format. Hi Theodore, Your patch seems ok, but: - there is no kfree(sd->jpeg_hdr). Should be in stop0(). - as there is only one vend:prod, one line is enough in gspca.txt. May you fix this and resend? Thanks.
Hello, Theodore On Sun, Aug 2, 2009 at 1:56 AM, Theodore Kilgore<kilgota@banach.math.auburn.edu> wrote: > Several cameras are supported here, which all share the same USB > Vendor:Product number when started up in streaming mode. All of these > cameras use bulk transport for streaming, and all of them produce frames in > JPEG format. > > Patch follows. > > Signed-off-by Theodore Kilgore <kilgota@auburn.edu> > > ------ > diff -r d189fb4be712 linux/Documentation/video4linux/gspca.txt > --- a/linux/Documentation/video4linux/gspca.txt Wed Jul 29 10:01:54 2009 > +0200 > +++ b/linux/Documentation/video4linux/gspca.txt Sat Aug 01 15:42:02 2009 > -0500 > @@ -239,6 +239,9 @@ >  pac7311         093a:2629    Genious iSlim 300 >  pac7311         093a:262a    Webcam 300k >  pac7311         093a:262c    Philips SPC 230 NC > +jeilinj         0979:0280    Cobra DMC 300 > +jeilinj         0979:0280    FlyCamOne > +jeilinj         0979:0280    Sakar 57379 >  zc3xx      0ac8:0302    Z-star Vimicro zc0302 >  vc032x     0ac8:0321    Vimicro generic vc0321 >  vc032x     0ac8:0323    Vimicro Vc0323 > diff -r d189fb4be712 linux/drivers/media/video/gspca/Kconfig > --- a/linux/drivers/media/video/gspca/Kconfig  Wed Jul 29 10:01:54 2009 > +0200 > +++ b/linux/drivers/media/video/gspca/Kconfig  Sat Aug 01 15:42:02 2009 > -0500 > @@ -47,6 +47,15 @@ >      To compile this driver as a module, choose M here: the >      module will be called gspca_finepix. > > +config USB_GSPCA_JEILINJ > +    tristate "Jeilin JPEG USB V4L2 driver" > +    depends on VIDEO_V4L2 && USB_GSPCA > +    help > +     Say Y here if you want support for cameras based on this Jeilin > chip. > + > +     To compile this driver as a module, choose M here: the > +     module will be called gspca_jeilinj. > + >  config USB_GSPCA_MARS >     tristate "Mars USB Camera Driver" >     depends on VIDEO_V4L2 && USB_GSPCA > diff -r d189fb4be712 linux/drivers/media/video/gspca/Makefile > --- a/linux/drivers/media/video/gspca/Makefile  Wed Jul 29 10:01:54 2009 > +0200 > +++ b/linux/drivers/media/video/gspca/Makefile  Sat Aug 01 15:42:02 2009 > -0500 > @@ -2,6 +2,7 @@ >  obj-$(CONFIG_USB_GSPCA_CONEX)   += gspca_conex.o >  obj-$(CONFIG_USB_GSPCA_ETOMS)   += gspca_etoms.o >  obj-$(CONFIG_USB_GSPCA_FINEPIX)  += gspca_finepix.o > +obj-$(CONFIG_USB_GSPCA_JEILINJ)  += gspca_jeilinj.o >  obj-$(CONFIG_USB_GSPCA_MARS)   += gspca_mars.o >  obj-$(CONFIG_USB_GSPCA_MR97310A) += gspca_mr97310a.o >  obj-$(CONFIG_USB_GSPCA_OV519)   += gspca_ov519.o > @@ -30,6 +31,7 @@ >  gspca_conex-objs   := conex.o >  gspca_etoms-objs   := etoms.o >  gspca_finepix-objs  := finepix.o > +gspca_jeilinj-objs  := jeilinj.o >  gspca_mars-objs   := mars.o >  gspca_mr97310a-objs := mr97310a.o >  gspca_ov519-objs   := ov519.o > diff -r d189fb4be712 linux/drivers/media/video/gspca/jeilinj.c > --- /dev/null  Thu Jan 01 00:00:00 1970 +0000 > +++ b/linux/drivers/media/video/gspca/jeilinj.c Sat Aug 01 15:42:02 2009 > -0500 > @@ -0,0 +1,387 @@ > +/* > + * Jeilinj subdriver > + * > + * Supports some Jeilin dual-mode cameras which use bulk transport and > + * download raw JPEG data. > + * > + * Copyright (C) 2009 Theodore Kilgore > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA > + */ > + > +#define MODULE_NAME "jeilinj" > + > +#include <linux/workqueue.h> > +#include "gspca.h" > +#include "jpeg.h" > + > +MODULE_AUTHOR("Theodore Kilgore <kilgota@auburn.edu>"); > +MODULE_DESCRIPTION("GSPCA/JEILINJ USB Camera Driver"); > +MODULE_LICENSE("GPL"); > + > +/* Default timeouts, in ms */ > +#define JEILINJ_CMD_TIMEOUT 500 > +#define JEILINJ_DATA_TIMEOUT 1000 > + > +/* Maximum transfer size to use. */ > +#define JEILINJ_MAX_TRANSFER 0x200 > + > +#define FRAME_HEADER_LEN 0x10 > + > +/* Structure to hold all of our device specific stuff */ > +struct sd { > +    struct gspca_dev gspca_dev;   /* !! must be the first item */ > +    const struct v4l2_pix_format *cap_mode; > +    /* Driver stuff */ > +    struct work_struct work_struct; > +    struct workqueue_struct *work_thread; > +    u8 quality;                /* image quality */ > +    u8 jpegqual;               /* webcam quality */ > +    u8 *jpeg_hdr; > +}; > + > +    struct jlj_command { > +        unsigned char instruction[2]; > +        unsigned char ack_wanted; > +    }; > + > +/* AFAICT these cameras will only do 320x240. */ > +static struct v4l2_pix_format jlj_mode[] = { > +    { 320, 240, V4L2_PIX_FMT_JPEG, V4L2_FIELD_NONE, > +        .bytesperline = 320, > +        .sizeimage = 320 * 240, > +        .colorspace = V4L2_COLORSPACE_JPEG, > +        .priv = 0} > +}; > + > +/* > + * cam uses endpoint 0x03 to send commands, 0x84 for read commands, > + * and 0x82 for bulk transfer. > + */ > + > +/* All commands are two bytes only */ > +static int jlj_write2(struct gspca_dev *gspca_dev, unsigned char *command) > +{ > +    int retval; > + > +    memcpy(gspca_dev->usb_buf, command, 2); > +    retval = usb_bulk_msg(gspca_dev->dev, > +            usb_sndbulkpipe(gspca_dev->dev, 3), > +            gspca_dev->usb_buf, 2, NULL, 500); > +    if (retval < 0) > +        PDEBUG(D_ERR, "command write [%02x] error %d", > +                gspca_dev->usb_buf[0], retval); > +    return retval; > +} > + > +/* Responses are one byte only */ > +static int jlj_read1(struct gspca_dev *gspca_dev, unsigned char response) > +{ > +    int retval; > + > +    retval = usb_bulk_msg(gspca_dev->dev, > +    usb_rcvbulkpipe(gspca_dev->dev, 0x84), > +                gspca_dev->usb_buf, 1, NULL, 500); > +    response = gspca_dev->usb_buf[0]; > +    if (retval < 0) > +        PDEBUG(D_ERR, "read command [%02x] error %d", > +                gspca_dev->usb_buf[0], retval); > +    return retval; > +} > + > +static int jlj_start(struct gspca_dev *gspca_dev) > +{ > +    int i; > +    int retval = -1; > +    u8 response = 0xff; > +    struct jlj_command start_commands[] = { > +        {{0x71, 0x81}, 0}, > +        {{0x70, 0x05}, 0}, > +        {{0x95, 0x70}, 1}, > +        {{0x71, 0x81}, 0}, > +        {{0x70, 0x04}, 0}, > +        {{0x95, 0x70}, 1}, > +        {{0x71, 0x00}, 0}, > +        {{0x70, 0x08}, 0}, > +        {{0x95, 0x70}, 1}, > +        {{0x94, 0x02}, 0}, > +        {{0xde, 0x24}, 0}, > +        {{0x94, 0x02}, 0}, > +        {{0xdd, 0xf0}, 0}, > +        {{0x94, 0x02}, 0}, > +        {{0xe3, 0x2c}, 0}, > +        {{0x94, 0x02}, 0}, > +        {{0xe4, 0x00}, 0}, > +        {{0x94, 0x02}, 0}, > +        {{0xe5, 0x00}, 0}, > +        {{0x94, 0x02}, 0}, > +        {{0xe6, 0x2c}, 0}, > +        {{0x94, 0x03}, 0}, > +        {{0xaa, 0x00}, 0}, > +        {{0x71, 0x1e}, 0}, > +        {{0x70, 0x06}, 0}, > +        {{0x71, 0x80}, 0}, > +        {{0x70, 0x07}, 0} > +    }; > +    for (i = 0; i < ARRAY_SIZE(start_commands); i++) { > +        retval = jlj_write2(gspca_dev, > start_commands[i].instruction); > +        if (retval < 0) > +            return retval; > +        if (start_commands[i].ack_wanted) > +            retval = jlj_read1(gspca_dev, response); > +        if (retval < 0) > +            return retval; > +    } > +    PDEBUG(D_ERR, "jlj_start retval is %d", retval); > +    return retval; > +} > + > +static int jlj_stop(struct gspca_dev *gspca_dev) > +{ > +    int i; > +    int retval; > +    struct jlj_command stop_commands[] = { > +        {{0x71, 0x00}, 0}, > +        {{0x70, 0x09}, 0}, > +        {{0x71, 0x80}, 0}, > +        {{0x70, 0x05}, 0} > +    }; > +    for (i = 0; i < ARRAY_SIZE(stop_commands); i++) { > +        retval = jlj_write2(gspca_dev, > stop_commands[i].instruction); > +        if (retval < 0) > +            return retval; > +    } > +    return retval; > +} > + > +/* This function is called as a workqueue function and runs whenever the > camera > + * is streaming data. Because it is a workqueue function it is allowed to > sleep > + * so we can use synchronous USB calls. To avoid possible collisions with > other > + * threads attempting to use the camera's USB interface the gspca usb_lock > is > + * used when performing the one USB control operation inside the workqueue, > + * which tells the camera to close the stream. In practice the only thing > + * which needs to be protected against is the usb_set_interface call that > + * gspca makes during stream_off. Otherwise the camera doesn't provide any > + * controls that the user could try to change. > + */ > + > +static void jlj_dostream(struct work_struct *work) > +{ > +    struct sd *dev = container_of(work, struct sd, work_struct); > +    struct gspca_dev *gspca_dev = &dev->gspca_dev; > +    struct gspca_frame *frame; > +    int blocks_left; /* 0x200-sized blocks remaining in current frame. > */ > +    int size_in_blocks; > +    int act_len; > +    int discarding = 0; /* true if we failed to get space for frame. */ > +    int packet_type; > +    int ret; > +    u8 *buffer; > + > +    buffer = kmalloc(JEILINJ_MAX_TRANSFER, GFP_KERNEL | GFP_DMA); > +    if (!buffer) { > +        PDEBUG(D_ERR, "Couldn't allocate USB buffer"); > +        goto quit_stream; > +    } This clean up on error path looks bad. On quit_stream you have: > +quit_stream: > + mutex_lock(&gspca_dev->usb_lock); > + if (gspca_dev->present) > + jlj_stop(gspca_dev); > + mutex_unlock(&gspca_dev->usb_lock); > + kfree(buffer); kfree() tries to free null buffer after kmalloc for buffer failed. Please, check if i'm not wrong.
On Sun, 2 Aug 2009, Jean-Francois Moine wrote: > On Sat, 1 Aug 2009 16:56:06 -0500 (CDT) > Theodore Kilgore <kilgota@banach.math.auburn.edu> wrote: > >> Several cameras are supported here, which all share the same USB >> Vendor:Product number when started up in streaming mode. All of these >> cameras use bulk transport for streaming, and all of them produce >> frames in JPEG format. > > Hi Theodore, > > Your patch seems ok, but: > > - there is no kfree(sd->jpeg_hdr). Should be in stop0(). OK. Will do, and resend, as asked. But before I do that, there is the second item. It seems to raise questions of policy. I think it is appropriate to raise that question for general discussion. > > - as there is only one vend:prod, one line is enough in gspca.txt. This is a question about which I have been curious for quite some time, and I think that now is a good time to ask it. Just what policy do we have about this? The information which links brand and model to driver ought to be presented somewhere. If it does not go into gspca.txt then where exactly is the appropriate place to put said information? As to these three particular cameras, perhaps one should take into account here the fact that what we actually have here is three "different" cameras, put on the market by three different vendors, and in at least two different parts of the world. The Sakar camera and the Cobra camera are for sale in the US. The FlyOne camera is for sale in Germany and perhaps in some other parts of Europe. Furthermore, in stillcam mode they are all three of them standard mass storage devices, but they have different IDs from what is listed here (good for them about that!) and the stillcam IDs are distinct, too. So in other words they are not the same camera, at all, in spite of the fact that they all use the same ID when set up in webcam mode. A more general consideration is that the buyer of a camera is not helped at all by knowing that a particular chipset combination is supported. No. The buyer can only go by the make and model which are printed on the outside of the plastic bubble pack. So what exactly are we, the developers, to do in order to make the relevant information available to the public? There is some wiki, of course. But it seems to me the wiki itself ought to refer to the gspca.txt file, among other things. I do think that one of the easy ways to address the above issue of helping the camera purchaser would be to agree that gspca.txt ought to contain the information for each individual camera which is supported. This would make the file longer, but it is in my opinion a small price to pay, when the goal is to have complete information, put in a central place. In going over the gspca.txt file, I also see that the jeilinj entries are the only place where there is a "duplicate" entry, so that I am at this time in non-conformity. But, on checking, I also see that the SQ905 cameras (2770:9120) and the SQ905C cameras (2770:905c) are not listed at all in gspca.txt. This could be claimed as an example of my ignorance that I am supposed to put something there. But it is also related to the fact that -- there are twenty-five (25) cameras listed in libgphoto2/camlibs/sq905/library.c, which were reported to me as working with that particular stillcam driver. This list does not include several other cameras that I only heard about vaguely, or that I do not even know about at all because nobody ever reported them to me. So if I am supposed to pick just one of these, then which one do I list in gspca.txt, when as far as I know they all ought to work with the gspca sq905 driver? and, as for the SQ905C cameras -- there are seventeen (17) cameras listed in libgphoto2/camlibs/digigr8/library.c, which were reported to me as working with that particular stillcam driver. This list does not include several other cameras that I only heard about vaguely, or that I do not even know about at all because nobody ever reported them to me. If I am supposed to pick just one of these, then which one do I list in gspca.txt, when as far as I know they all ought to work with the gspca sq905c driver? For the above reasons, I have up to this point not listed any of these cameras in gspca.txt at all, as supported. Probably that is not the right thing to do, either? Now, in addition to the above, there is another problem which will hit very soon, probably soon after the time that Thomas Kaiser gets back from his vacation and starts to work again. This is the matter of the mr97310a driver, which is almost finished now. What we have there is a list of cameras which are all functionally identical as still cameras, but as webcams the functionality differs in minor details. Here is what happens: 08ca:0111 Aiptek Pencam VGA+ (supported, listed in gspca.txt) 093a:010f several cameras, some functionally identical to the Aiptek camera, and some which as webcams require a different init procedure and use different control procedures! There are two different basic cameras with the same USB number, and it has been necessary to create undocumented commands to be able to tell which kind of camera we have! This has been done, at this point. But as to documentation the question arises, that if we choose just one of these to list in gspca.txt, should we pick one of "Type 1" or one of "Type 2" to list? 093a:010e several cameras with this ID number, which also fall into two disjoint sets when run in webcam mode, again requiring the development of previously undocumented commands to be able to detect and classify which camera it is, before initializing it. Again, which one of these cameras is one supposed to list in gspca.txt? Also the above account of the unexpected complications of the mr97310a driver raises the natural problem that the same thing might happen yet again. The differences in behavior of the different cameras seem to be connected to such external characteristics as the type of LCD display used on the outside of the camera, or other minor details in the construction. Who is to say that there can not be a "Type 3" in either of the two cases where two types have already been discovered? What is the safe thing to do? Well, it seems to me, the only thing appropriate is to list explicitly all the mr97310a cameras currently supported, and in the future to list explicitly any other one which is reported as working. As I said, this question of what to do with the mr97310a documentation will come up very soon, unless one merely avoids the question by refusing to make a choice. So the question of documentation becomes rather serious. I hope that the occasion is now appropriate one deciding this issue of documentation. My opinion is that gspca.txt ought to list individually each camera which is known to work, as well as the driver used and the USB ID. Theodore Kilgore -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, 2 Aug 2009 17:25:29 +0400 Alexey Klimov <klimov.linux@gmail.com> wrote: > > + Â Â Â buffer = kmalloc(JEILINJ_MAX_TRANSFER, GFP_KERNEL | > > GFP_DMA); > > + Â Â Â if (!buffer) { > > + Â Â Â Â Â Â Â PDEBUG(D_ERR, "Couldn't allocate USB buffer"); > > + Â Â Â Â Â Â Â goto quit_stream; > > + Â Â Â } > > This clean up on error path looks bad. On quit_stream you have: > > > +quit_stream: > > + mutex_lock(&gspca_dev->usb_lock); > > + if (gspca_dev->present) > > + jlj_stop(gspca_dev); > > + mutex_unlock(&gspca_dev->usb_lock); > > + kfree(buffer); > > kfree() tries to free null buffer after kmalloc for buffer failed. > Please, check if i'm not wrong. Hi Alexey, AFAIK, kfree() checks the pointer. Cheers.
On Sun, 2 Aug 2009 14:12:28 -0500 (CDT) Theodore Kilgore <kilgota@banach.math.auburn.edu> wrote: [snip] > > - as there is only one vend:prod, one line is enough in gspca.txt. > > This is a question about which I have been curious for quite some > time, and I think that now is a good time to ask it. > > Just what policy do we have about this? The information which links > brand and model to driver ought to be presented somewhere. If it does > not go into gspca.txt then where exactly is the appropriate place to > put said information? [snip] Hi Theodore, gspca.txt has been defined only to know which subdriver has to be generated for a webcam that a user already owns. The trade name of the webcams are often not clear enough (look at all the Creative varieties). So, the user has just to plug her webcam and with the vend:prod ID, she will know which driver she has to generate (you may say that there are already tools which do the job, as easycam, but I do not think they are often used). The device list of the other drivers (CARDLIST.xx) are not sorted and their format (numbered list) does not facilitate this job. So, I prefered a list sorted by vend:prod. In gspca.txt, the 3rd column contains the webcam names. As you can see, it is a comma separated list, so, you may put here all the names you may know. But, is it useful? I think that the webcam names should be only in the file usb.ids which comes with the usbutils. To go further, there should be a general file which should contain all the usb (and pci) devices and their associated drivers. This information exists in /lib/modules/`uname -r`/modules.usbmap when all drivers are generated. So, we just need a tool (and a guy!) to maintain this general file... Best regards.
Em Mon, 3 Aug 2009 08:30:12 +0200 Jean-Francois Moine <moinejf@free.fr> escreveu: > On Sun, 2 Aug 2009 17:25:29 +0400 > Alexey Klimov <klimov.linux@gmail.com> wrote: > > > > + Â Â Â buffer = kmalloc(JEILINJ_MAX_TRANSFER, GFP_KERNEL | > > > GFP_DMA); > > > + Â Â Â if (!buffer) { > > > + Â Â Â Â Â Â Â PDEBUG(D_ERR, "Couldn't allocate USB buffer"); > > > + Â Â Â Â Â Â Â goto quit_stream; > > > + Â Â Â } > > > > This clean up on error path looks bad. On quit_stream you have: > > > > > +quit_stream: > > > + mutex_lock(&gspca_dev->usb_lock); > > > + if (gspca_dev->present) > > > + jlj_stop(gspca_dev); > > > + mutex_unlock(&gspca_dev->usb_lock); > > > + kfree(buffer); > > > > kfree() tries to free null buffer after kmalloc for buffer failed. > > Please, check if i'm not wrong. > > Hi Alexey, > > AFAIK, kfree() checks the pointer. Yeah. Theodore's code is ok. kfree(NULL) is legal. > > Cheers. > Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 3 Aug 2009, Jean-Francois Moine wrote: > On Sun, 2 Aug 2009 14:12:28 -0500 (CDT) > Theodore Kilgore <kilgota@banach.math.auburn.edu> wrote: > > [snip] >>> - as there is only one vend:prod, one line is enough in gspca.txt. >> >> This is a question about which I have been curious for quite some >> time, and I think that now is a good time to ask it. >> >> Just what policy do we have about this? The information which links >> brand and model to driver ought to be presented somewhere. If it does >> not go into gspca.txt then where exactly is the appropriate place to >> put said information? > [snip] > > Hi Theodore, > > gspca.txt has been defined only to know which subdriver has to be > generated for a webcam that a user already owns. > > The trade name of the webcams are often not clear enough (look at all > the Creative varieties). So, the user has just to plug her webcam and > with the vend:prod ID, she will know which driver she has to generate So, from this the general advice to users is 1. Buy a camera 2. Plug it in to see if it has a driver? > (you may say that there are already tools which do the job, as easycam, > but I do not think they are often used). The problem is, the USB ID (and hence the driver) are not publicized by anyone connected to the manufacturer or the retailer. So the result is that there is a lot of hardware out there which is is currently unusable and nobody knows that, and also lots of hardware out there which is perfectly usable because we already supported it, but again nobody knows that. The corollary is that the "trade names" you refer to ought to be listed _somewhere_ as completely, clearly, and precisely as possible. To me, and obviously good place to start with that would be to list somewhere in the kernel documentation the devices supported by a given driver or module. > > The device list of the other drivers (CARDLIST.xx) are not sorted and > their format (numbered list) does not facilitate this job. So, I > prefered a list sorted by vend:prod. > > In gspca.txt, the 3rd column contains the webcam names. As you can see, > it is a comma separated list, so, you may put here all the names you may > know. But, is it useful? I would certainly hope so. Otherwise, why bother to put any names in there at all? I think that the webcam names should be only > in the file usb.ids which comes with the usbutils. That file is hopelessly out of date, by definition, and occasionally seems to me inaccurate in such details as ownership of USB ID X by company Y, for which, for example, see the association of 0x2770 (S&Q Technologies) to the Japanese camera and electronics packager and retailer NHL. More relevant to the present discussion, though, is that even if the usb.ids file were completely up to date it serves an entirely different purpose from what we are discussing here. My understanding is that the usb.ids file is supposed to be nothing but an inventory of the devices that we know about. It was never intended as a list of supported devices and by its nature can not serve that purpose simultaneously. Also, a lot of USB devices come under the category of "who cares what the Vendor and Product number are?" such as standard mass storage pocket flash drives. So I suspect that no one is conscientious about listing them. > > To go further, there should be a general file which should contain all > the usb (and pci) devices and their associated drivers. This > information exists in /lib/modules/`uname -r`/modules.usbmap when all > drivers are generated. So, we just need a tool (and a guy!) to maintain > this general file... Hmm. Yes. The "guy." Well, it is better to figure out a way to make things like that happen automatically, and then one does not need to worry so much about the "guy." I will mention how Gphoto handles this problem, for comparison. It might not be so easy to carry out here, because what finally comes is the command option gphoto2 --list-cameras. That command will print out a (very long) list of all the currently nsupported cameras, by name. At this point, the list has over 1000 entries. It is up to the writer of the camera library to list the cameras which are supported by that particular piece of software. So the entry instead of just looking like this /* Table of supported USB devices */ static const __devinitdata struct usb_device_id device_table[] = { {USB_DEVICE(0x2770, 0x905c)}, {USB_DEVICE(0x2770, 0x9050)}, {USB_DEVICE(0x2770, 0x913d)}, {} }; must contain two other additional fields, describing the name in human-readable form (intended to identify the camera to the extent that one can probably pick it out from others on the shelf) and the current status of the support for the camera. Like this: {"Sakar Micro Digital 2428x", GP_DRIVER_STATUS_EXPERIMENTAL, 0x2770, 0x905c}, {"Jazz JDC9", GP_DRIVER_STATUS_EXPERIMENTAL, 0x2770, 0x905c}, {"Disney pix micro", GP_DRIVER_STATUS_EXPERIMENTAL, 0x2770, 0x9050}, {"Disney pix micro 2", GP_DRIVER_STATUS_EXPERIMENTAL, 0x2770, 0x9052}, {"Suprema Digital Keychain Camera", GP_DRIVER_STATUS_EXPERIMENTAL, 0x2770, 0x913d} If something similar were done here, then perhaps it would be possible to provide a tool which would print out two lists. One of them could be a list of the known and supported devices by gspca, say. The second option could provide a list of the devices which the local kernel supports. The need to rely on "the guy" is greatly reduced if such procedures are put into place. Then it is the responsibility of the person who writes the module to produce the information in the first place, and it is one of the items on the checklist at submission time. It is also very easy for someone else to add to the iist later on, if the occasion arises. Perhaps someone else can come up with a better idea? Theodore Kilgore -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Em Mon, 3 Aug 2009 11:01:56 -0500 (CDT) Theodore Kilgore <kilgota@banach.math.auburn.edu> escreveu: > On Mon, 3 Aug 2009, Jean-Francois Moine wrote: > > > On Sun, 2 Aug 2009 14:12:28 -0500 (CDT) > > Theodore Kilgore <kilgota@banach.math.auburn.edu> wrote: > > > > [snip] > >>> - as there is only one vend:prod, one line is enough in gspca.txt. > >> > >> This is a question about which I have been curious for quite some > >> time, and I think that now is a good time to ask it. > >> > >> Just what policy do we have about this? The information which links > >> brand and model to driver ought to be presented somewhere. If it does > >> not go into gspca.txt then where exactly is the appropriate place to > >> put said information? > > [snip] > > > > Hi Theodore, > > > > gspca.txt has been defined only to know which subdriver has to be > > generated for a webcam that a user already owns. > > > > The trade name of the webcams are often not clear enough (look at all > > the Creative varieties). So, the user has just to plug her webcam and > > with the vend:prod ID, she will know which driver she has to generate > > So, from this the general advice to users is > > 1. Buy a camera > 2. Plug it in to see if it has a driver? > > > (you may say that there are already tools which do the job, as easycam, > > but I do not think they are often used). > > The problem is, the USB ID (and hence the driver) are not publicized by > anyone connected to the manufacturer or the retailer. So the result is > that there is a lot of hardware out there which is is currently unusable > and nobody knows that, and also lots of hardware out there which is > perfectly usable because we already supported it, but again nobody knows > that. The corollary is that the "trade names" you refer to ought to be > listed _somewhere_ as completely, clearly, and precisely as possible. To > me, and obviously good place to start with that would be to list somewhere > in the kernel documentation the devices supported by a given driver or > module. > > > > > > The device list of the other drivers (CARDLIST.xx) are not sorted and > > their format (numbered list) does not facilitate this job. So, I > > prefered a list sorted by vend:prod. > > > > In gspca.txt, the 3rd column contains the webcam names. As you can see, > > it is a comma separated list, so, you may put here all the names you may > > know. But, is it useful? > > I would certainly hope so. Otherwise, why bother to put any names in there > at all? > > > I think that the webcam names should be only > > in the file usb.ids which comes with the usbutils. > > That file is hopelessly out of date, by definition, and occasionally > seems to me inaccurate in such details as ownership of USB ID X by > company Y, for which, for example, see the association of 0x2770 (S&Q > Technologies) to the Japanese camera and electronics packager and > retailer NHL. > > More relevant to the present discussion, though, is that even if the > usb.ids file were completely up to date it serves an entirely different > purpose from what we are discussing here. My understanding is that the > usb.ids file is supposed to be nothing but an inventory of the devices > that we know about. It was never intended as a list of supported devices > and by its nature can not serve that purpose simultaneously. Also, a lot > of USB devices come under the category of "who cares what the Vendor and > Product number are?" such as standard mass storage pocket flash drives. So > I suspect that no one is conscientious about listing them. > > > > > To go further, there should be a general file which should contain all > > the usb (and pci) devices and their associated drivers. This > > information exists in /lib/modules/`uname -r`/modules.usbmap when all > > drivers are generated. So, we just need a tool (and a guy!) to maintain > > this general file... > > Hmm. Yes. The "guy." Well, it is better to figure out a way to make > things like that happen automatically, and then one does not need to worry > so much about the "guy." I will mention how Gphoto handles this problem, > for comparison. It might not be so easy to carry out here, because what > finally comes is the command option gphoto2 --list-cameras. That command > will print out a (very long) list of all the currently nsupported cameras, > by name. At this point, the list has over 1000 entries. It is up to the > writer of the camera library to list the cameras which are supported by > that particular piece of software. So the entry instead of just looking > like this > > /* Table of supported USB devices */ > static const __devinitdata struct usb_device_id device_table[] = { > {USB_DEVICE(0x2770, 0x905c)}, > {USB_DEVICE(0x2770, 0x9050)}, > {USB_DEVICE(0x2770, 0x913d)}, > {} > }; > > must contain two other additional fields, describing the name in > human-readable form (intended to identify the camera to the extent that > one can probably pick it out from others on the shelf) and the current > status of the support for the camera. Like this: > > {"Sakar Micro Digital 2428x", GP_DRIVER_STATUS_EXPERIMENTAL, 0x2770, 0x905c}, > {"Jazz JDC9", GP_DRIVER_STATUS_EXPERIMENTAL, 0x2770, 0x905c}, > {"Disney pix micro", GP_DRIVER_STATUS_EXPERIMENTAL, 0x2770, 0x9050}, > {"Disney pix micro 2", GP_DRIVER_STATUS_EXPERIMENTAL, 0x2770, 0x9052}, > {"Suprema Digital Keychain Camera", GP_DRIVER_STATUS_EXPERIMENTAL, > 0x2770, 0x913d} The struct usb_device_id doesn't have any thing like that, and creating another table repeating the USB ID is very ugly (yet, a few drivers do it). This also means to add 4 extra bytes per camera, just to store a duplicated information. The better way is to do something like: enum { SAKAR_MICRO_DIGITAL_2428X, DISNEY_PIX_MICRO, DISNEY_PIX_MICRO2, SUPREMA_DIGITAL_KEYCHAIN_CAMERA, }; static const __devinitdata struct usb_device_id device_table[] = { {USB_DEVICE(0x2770, 0x905c), .driver_info = SAKAR_MICRO_DIGITAL_2428X}, {USB_DEVICE(0x2770, 0x9050), .driver_info = DISNEY_PIX_MICRO}, {USB_DEVICE(0x2770, 0x9052), .driver_info = DISNEY_PIX_MICRO2}, {USB_DEVICE(0x2770, 0x913d), .driver_info = SUPREMA_DIGITAL_KEYCHAIN_CAMERA}, {} }; struct camera_description { char *name; unsigned int flags; }; static struct camera_description cameras[] = { [SAKAR_MICRO_DIGITAL_2428X] = { .name = "Sakar Micro Digital 2428x/Jazz JDC9", .flags = GP_DRIVER_STATUS_EXPERIMENTAL, }, [DISNEY_PIX_MICRO] = { .name = "Disney pix micro", .flags = GP_DRIVER_STATUS_EXPERIMENTAL, }, [DISNEY_PIX_MICRO2] = { .name = "Disney pix micro 2", .flags = GP_DRIVER_STATUS_EXPERIMENTAL, }, [SUPREMA_DIGITAL_KEYCHAIN_CAMERA] = { .name = "Suprema Digital Keychain Camera", .flags = GP_DRIVER_STATUS_EXPERIMENTAL, }, }; The association between the two tables can easily be done at the .config ops: /* this function is called at probe time */ static int sd_config(struct gspca_dev *gspca_dev, const struct usb_device_id *id) { ... int board_nr = id->driver_info; printf(KERN_WARN "Detected camera %s\n", cameras[board_nr]); ... } A small change at cx88.pl will be enough to auto-generate gspca.txt. > > If something similar were done here, then perhaps it would be possible to > provide a tool which would print out two lists. One of them could be a > list of the known and supported devices by gspca, say. The second option > could provide a list of the devices which the local kernel supports. > > The need to rely on "the guy" is greatly reduced if such procedures are > put into place. Then it is the responsibility of the person who writes the > module to produce the information in the first place, and it is one of the > items on the checklist at submission time. It is also very easy for > someone else to add to the iist later on, if the occasion arises. > > Perhaps someone else can come up with a better idea? > > Theodore Kilgore > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Mauro, Thank you for some very interesting comments. I will have some responses "in-line," also expanding on some of the previous points. On Mon, 3 Aug 2009, Mauro Carvalho Chehab wrote: > Em Mon, 3 Aug 2009 11:01:56 -0500 (CDT) > Theodore Kilgore <kilgota@banach.math.auburn.edu> escreveu: > >> On Mon, 3 Aug 2009, Jean-Francois Moine wrote: >> >>> On Sun, 2 Aug 2009 14:12:28 -0500 (CDT) >>> Theodore Kilgore <kilgota@banach.math.auburn.edu> wrote: >>> >>> [snip] >>>>> - as there is only one vend:prod, one line is enough in gspca.txt. >>>> >>>> This is a question about which I have been curious for quite some >>>> time, and I think that now is a good time to ask it. >>>> >>>> Just what policy do we have about this? The information which links >>>> brand and model to driver ought to be presented somewhere. If it does >>>> not go into gspca.txt then where exactly is the appropriate place to >>>> put said information? >>> [snip] >>> >>> Hi Theodore, >>> >>> gspca.txt has been defined only to know which subdriver has to be >>> generated for a webcam that a user already owns. >>> >>> The trade name of the webcams are often not clear enough (look at all >>> the Creative varieties). My opinion here is that in the interests of general usability the wecam names should be as clear and unambiguous as possible, and most ideally the trade name, if enough of it is quoted, is "clear enough." Unfortunately, there is another problem I did not think of, here, which is an argument against myself. It is that quite often a "manufacturer" switches the chip inside of a device marked with a certain trade name and does not tell us about it. This has happened with several cameras that I myself have supported. But I think we still have to do our best. So, the user has just to plug her webcam and >>> with the vend:prod ID, she will know which driver she has to generate >> >> So, from this the general advice to users is >> >> 1. Buy a camera >> 2. Plug it in to see if it has a driver? >> >>> (you may say that there are already tools which do the job, as easycam, >>> but I do not think they are often used). I did not understand this comment. Where would "tools which do the job" come by the information, anyway? So that is why I had no response to it. >> >> The problem is, the USB ID (and hence the driver) are not publicized by >> anyone connected to the manufacturer or the retailer. So the result is >> that there is a lot of hardware out there which is is currently unusable >> and nobody knows that, and also lots of hardware out there which is >> perfectly usable because we already supported it, but again nobody knows >> that. The corollary is that the "trade names" you refer to ought to be >> listed _somewhere_ as completely, clearly, and precisely as possible. To >> me, and obviously good place to start with that would be to list somewhere >> in the kernel documentation the devices supported by a given driver or >> module. The discussion which comes below makes me suspect (again) that the best way to maintain a list of the supported cameras, which includes the trade names of them, would be to place that information in the Documentation directory, rather than to dirty up the code by making it do the listing. >> >> >>> >>> The device list of the other drivers (CARDLIST.xx) are not sorted and >>> their format (numbered list) does not facilitate this job. So, I >>> prefered a list sorted by vend:prod. >>> >>> In gspca.txt, the 3rd column contains the webcam names. As you can see, >>> it is a comma separated list, so, you may put here all the names you may >>> know. But, is it useful? >> >> I would certainly hope so. Otherwise, why bother to put any names in there >> at all? I certainly stand by this statement. It is not logical to put in one name willy-nilly and not to put in a dozen or so others. Moreover, I do not claim it has happened, but if it has happened it should not have happened, that a name is put there but is incomplete, making it not possible actually to identify the device. Nonetheless, I do agree that a comma-separated list could get quite long. But I think this is the only feasible approach in the long run. >> >> >> I think that the webcam names should be only >>> in the file usb.ids which comes with the usbutils. >> >> That file is hopelessly out of date, by definition, and occasionally >> seems to me inaccurate in such details as ownership of USB ID X by >> company Y, for which, for example, see the association of 0x2770 (S&Q >> Technologies) to the Japanese camera and electronics packager and >> retailer NHL. >> >> More relevant to the present discussion, though, is that even if the >> usb.ids file were completely up to date it serves an entirely different >> purpose from what we are discussing here. My understanding is that the >> usb.ids file is supposed to be nothing but an inventory of the devices >> that we know about. It was never intended as a list of supported devices >> and by its nature can not serve that purpose simultaneously. Also, a lot >> of USB devices come under the category of "who cares what the Vendor and >> Product number are?" such as standard mass storage pocket flash drives. So >> I suspect that no one is conscientious about listing them. Again, the usb.ids file seems to me to serve entirely another purpose. It lists those IDs which are known, reported, and entered into said file. This is important, but it is not the same thing at all as listing _supported_ devices. On the contrary, it has historically listed unsupported devices, too. Also somehow connected with the usb.ids file, as I understand, is the website at qbik.ch which lists all devices reported to it, along with the status of their support. I have an account there and have reported a number of devices. There is a problem there, though. It is the problem that you can not go there and edit an entry unless you yourself have created that entry. There are several devices on that site marked with a big red X (will not work on Linux) for which I myself later on provided the support. I can not change the big red X. I can go and leave a comment which someone can read if curious enough to go past the big red X and click on "show" and I can try to contact the original poster of the report about the device. But the original poster may have disappeared from the face of the earth, and mail to the OP gets bounced, and nevertheless one can not change the original entry. Only the OP can do that. So here is a problem, which in its fundamentals is the problem of "the guy." Who is the "guy"? I have tried to find out, and I still do not know. So one thing I think we all agree about is that it is really not a good idea to rely on "the guy." >> >>> >>> To go further, there should be a general file which should contain all >>> the usb (and pci) devices and their associated drivers. This >>> information exists in /lib/modules/`uname -r`/modules.usbmap when all >>> drivers are generated. Come again? If the information was never there in the first place, how does it now magically get generated. To take the most recent example of the jeilinj driver, I strongly suspect that it can not list all three (known) supported cameras by name. Nor, perhaps, should it. If mixed into module code, such gory details could probably be classified as crud. So, we just need a tool (and a guy!) to maintain >>> this general file... >> >> Hmm. Yes. The "guy." Well, it is better to figure out a way to make >> things like that happen automatically, and then one does not need to worry >> so much about the "guy." I will mention how Gphoto handles this problem, >> for comparison. It might not be so easy to carry out here, because what >> finally comes is the command option gphoto2 --list-cameras. That command >> will print out a (very long) list of all the currently nsupported cameras, >> by name. At this point, the list has over 1000 entries. It is up to the >> writer of the camera library to list the cameras which are supported by >> that particular piece of software. So the entry instead of just looking >> like this >> >> /* Table of supported USB devices */ >> static const __devinitdata struct usb_device_id device_table[] = { >> {USB_DEVICE(0x2770, 0x905c)}, >> {USB_DEVICE(0x2770, 0x9050)}, >> {USB_DEVICE(0x2770, 0x913d)}, >> {} >> }; >> >> must contain two other additional fields, describing the name in >> human-readable form (intended to identify the camera to the extent that >> one can probably pick it out from others on the shelf) and the current >> status of the support for the camera. Like this: >> >> {"Sakar Micro Digital 2428x", GP_DRIVER_STATUS_EXPERIMENTAL, 0x2770, 0x905c}, >> {"Jazz JDC9", GP_DRIVER_STATUS_EXPERIMENTAL, 0x2770, 0x905c}, >> {"Disney pix micro", GP_DRIVER_STATUS_EXPERIMENTAL, 0x2770, 0x9050}, >> {"Disney pix micro 2", GP_DRIVER_STATUS_EXPERIMENTAL, 0x2770, 0x9052}, >> {"Suprema Digital Keychain Camera", GP_DRIVER_STATUS_EXPERIMENTAL, >> 0x2770, 0x913d} > > The struct usb_device_id doesn't have any thing like that, and creating another > table repeating the USB ID is very ugly (yet, a few drivers do it). This also > means to add 4 extra bytes per camera, just to store a duplicated information. > > The better way is to do something like: > > enum { > SAKAR_MICRO_DIGITAL_2428X, > DISNEY_PIX_MICRO, > DISNEY_PIX_MICRO2, > SUPREMA_DIGITAL_KEYCHAIN_CAMERA, > }; > > static const __devinitdata struct usb_device_id device_table[] = { > {USB_DEVICE(0x2770, 0x905c), .driver_info = SAKAR_MICRO_DIGITAL_2428X}, > {USB_DEVICE(0x2770, 0x9050), .driver_info = DISNEY_PIX_MICRO}, > {USB_DEVICE(0x2770, 0x9052), .driver_info = DISNEY_PIX_MICRO2}, > {USB_DEVICE(0x2770, 0x913d), .driver_info = SUPREMA_DIGITAL_KEYCHAIN_CAMERA}, > {} > }; > > struct camera_description { > char *name; > unsigned int flags; > }; > > static struct camera_description cameras[] = { > [SAKAR_MICRO_DIGITAL_2428X] = { > .name = "Sakar Micro Digital 2428x/Jazz JDC9", > .flags = GP_DRIVER_STATUS_EXPERIMENTAL, > }, > [DISNEY_PIX_MICRO] = { > .name = "Disney pix micro", > .flags = GP_DRIVER_STATUS_EXPERIMENTAL, > }, > [DISNEY_PIX_MICRO2] = { > .name = "Disney pix micro 2", > .flags = GP_DRIVER_STATUS_EXPERIMENTAL, > }, > [SUPREMA_DIGITAL_KEYCHAIN_CAMERA] = { > .name = "Suprema Digital Keychain Camera", > .flags = GP_DRIVER_STATUS_EXPERIMENTAL, > }, > }; > > The association between the two tables can easily be done at the .config ops: > > /* this function is called at probe time */ > static int sd_config(struct gspca_dev *gspca_dev, > const struct usb_device_id *id) > { > > ... > int board_nr = id->driver_info; > > printf(KERN_WARN "Detected camera %s\n", cameras[board_nr]); > > ... > } > > A small change at cx88.pl will be enough to auto-generate gspca.txt. Mauro, this is impressive. First, you point out that Gphoto could have done the job more efficiently, and it seems to me that you are right. Then you point out that to make such a change would break compatibility with the Linux kernel's struct usb_device_id so it cannot be done that way. Then you provide a workaround. It is very clever. However, another side of me says that all of this workaround would have to get compiled into the code for every gspca driver, making the code for each driver, and the resulting binary output, too, in turn longer and more complicated. Therefore, the excursion ends up convincing me that in the first place the documentation about which devices are supported, listed by trade name as well as USB ID, really ought to be in some kind of place like gspca.txt. It is after all much easier to edit a text file than it is to write code, and the contents of the text file do not contribute to the growth of size of the resulting binary. And it would be at least as easy, at least from this point forward, to place the responsibility on the author/maintainer of a gspca_* module to provide such a list as part of the procedure for submitting code and patches. Then a perl script could, for example, parse that one file and put the entries into alphabetical order, or any other reasonable and desired order. <snip> As I said already: >> >> The need to rely on "the guy" is greatly reduced if such procedures are >> put into place. Then it is the responsibility of the person who writes the >> module to produce the information in the first place, and it is one of the >> items on the checklist at submission time. It is also very easy for >> someone else to add to the iist later on, if the occasion arises. So this is my suggestion. I think the proper place for the information is a text file, and one already exists which is along those lines but is incomplete. The way to make sure the information gets into the file is then to make that step to be part of the patch procedure for adding drivers. Theodore Kilgore -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 3 Aug 2009, Theodore Kilgore wrote: >> >> enum { >> SAKAR_MICRO_DIGITAL_2428X, >> DISNEY_PIX_MICRO, >> DISNEY_PIX_MICRO2, >> SUPREMA_DIGITAL_KEYCHAIN_CAMERA, >> }; >> >> static const __devinitdata struct usb_device_id device_table[] = { >> {USB_DEVICE(0x2770, 0x905c), .driver_info = >> SAKAR_MICRO_DIGITAL_2428X}, >> {USB_DEVICE(0x2770, 0x9050), .driver_info = DISNEY_PIX_MICRO}, >> {USB_DEVICE(0x2770, 0x9052), .driver_info = DISNEY_PIX_MICRO2}, >> {USB_DEVICE(0x2770, 0x913d), .driver_info = >> SUPREMA_DIGITAL_KEYCHAIN_CAMERA}, >> {} >> }; >> >> struct camera_description { >> char *name; >> unsigned int flags; >> }; >> >> static struct camera_description cameras[] = { >> [SAKAR_MICRO_DIGITAL_2428X] = { >> .name = "Sakar Micro Digital 2428x/Jazz JDC9", >> .flags = GP_DRIVER_STATUS_EXPERIMENTAL, >> }, >> [DISNEY_PIX_MICRO] = { >> .name = "Disney pix micro", >> .flags = GP_DRIVER_STATUS_EXPERIMENTAL, >> }, >> [DISNEY_PIX_MICRO2] = { >> .name = "Disney pix micro 2", >> .flags = GP_DRIVER_STATUS_EXPERIMENTAL, >> }, >> [SUPREMA_DIGITAL_KEYCHAIN_CAMERA] = { >> .name = "Suprema Digital Keychain Camera", >> .flags = GP_DRIVER_STATUS_EXPERIMENTAL, >> }, >> }; >> >> The association between the two tables can easily be done at the .config >> ops: >> >> /* this function is called at probe time */ >> static int sd_config(struct gspca_dev *gspca_dev, >> const struct usb_device_id *id) >> { >> >> ... >> int board_nr = id->driver_info; >> >> printf(KERN_WARN "Detected camera %s\n", cameras[board_nr]); >> >> ... >> } >> >> A small change at cx88.pl will be enough to auto-generate gspca.txt. > > Mauro, this is impressive. First, you point out that Gphoto could have done > the job more efficiently, and it seems to me that you are right. Then you > point out that to make such a change would break compatibility with the Linux > kernel's struct usb_device_id so it cannot be done that way. Then you provide > a workaround. It is very clever. This kind of trick is used already on several other drivers: cx88, bttv, em28xx, usbvision, saa7134, ... > However, another side of me says that all of this workaround would have to > get compiled into the code for every gspca driver, making the code for each > driver, and the resulting binary output, too, in turn longer and more > complicated. Longer: yes. It will generate a longer data segment with the names of all supported webcams. However, This won't increase the complextiy of the driver. As I showed, you can know what device you have by just looking at id.driver_info. Also, as this standard on other drivers, you aren't adding any uncommon weird behavior. Yet, this change would be a big patch, since several drivers already use device_info for something else. The patch would be trivial though, since we just need to move the current driver_info information to something inside the webcam struct. > Therefore, the excursion ends up convincing me that in the first > place the documentation about which devices are supported, listed by trade > name as well as USB ID, really ought to be in some kind of place like > gspca.txt. It is after all much easier to edit a text file than it is to > write code, and the contents of the text file do not contribute to the growth > of size of the resulting binary. And it would be at least as easy, at least > from this point forward, to place the responsibility on the author/maintainer > of a gspca_* module to provide such a list as part of the procedure for > submitting code and patches. Then a perl script could, for example, parse > that one file and put the entries into alphabetical order, or any other > reasonable and desired order. This will only work fine if there are some sanity check script to validate if all USB ID's are present at gspca.txt. Otherwise, we'll always have the risk of not having it properly updated. There is another alternative: add a comment before each new board at gspca's USB ID's entries. Something like: static const __devinitdata struct usb_device_id device_table[] = { /* Webcam: Sakar Micro Digital 2428x / Jazz JDC9 */ {USB_DEVICE(0x2770, 0x905c)}, /* Webcam: Disney pix micro */ {USB_DEVICE(0x2770, 0x9050)}, /* Webcam: Suprema Digital Keychain Camera */ {USB_DEVICE(0x2770, 0x913d)}, {} }; This could be easily parseable by a script that would generate the gspca.txt. Also, the patch for it can be generated by some script. > As I said already: > >> > >> > The need to rely on "the guy" is greatly reduced if such procedures are >> > put into place. Then it is the responsibility of the person who writes >> > the >> > module to produce the information in the first place, and it is one of >> > the >> > items on the checklist at submission time. It is also very easy for >> > someone else to add to the iist later on, if the occasion arises. > > So this is my suggestion. I think the proper place for the information is a > text file, and one already exists which is along those lines but is > incomplete. The way to make sure the information gets into the file is then > to make that step to be part of the patch procedure for adding drivers. > > Theodore Kilgore > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > >
On Mon, 3 Aug 2009, Mauro Carvalho Chehab wrote: > On Mon, 3 Aug 2009, Theodore Kilgore wrote: <snip> >>> static const __devinitdata struct usb_device_id device_table[] = { >>> {USB_DEVICE(0x2770, 0x905c), .driver_info = >>> SAKAR_MICRO_DIGITAL_2428X}, >>> {USB_DEVICE(0x2770, 0x9050), .driver_info = DISNEY_PIX_MICRO}, >>> {USB_DEVICE(0x2770, 0x9052), .driver_info = DISNEY_PIX_MICRO2}, >>> {USB_DEVICE(0x2770, 0x913d), .driver_info = >>> SUPREMA_DIGITAL_KEYCHAIN_CAMERA}, >>> {} >>> }; Incidentally, the above was only a small snippet from the list given in libgphoto2/camlibs/digigr8/library.c. The entire list is quite a bit longer. As I mentioned in a post yesterday, there are seventeen entries in all. That seventeen is a sample of the numbers that one often has to deal with. The reason I say that the entire list ought to be available and publicized somehow is that people simply would not know that these seventeen cameras are all supported in webcam mode (and by the same driver module, too!) unless someone informs them. OK, so here was your first suggestion: >>> >>> struct camera_description { >>> char *name; >>> unsigned int flags; >>> }; >>> >>> static struct camera_description cameras[] = { >>> [SAKAR_MICRO_DIGITAL_2428X] = { >>> .name = "Sakar Micro Digital 2428x/Jazz JDC9", >>> .flags = GP_DRIVER_STATUS_EXPERIMENTAL, >>> }, >>> [DISNEY_PIX_MICRO] = { >>> .name = "Disney pix micro", >>> .flags = GP_DRIVER_STATUS_EXPERIMENTAL, >>> }, >>> [DISNEY_PIX_MICRO2] = { >>> .name = "Disney pix micro 2", >>> .flags = GP_DRIVER_STATUS_EXPERIMENTAL, >>> }, >>> [SUPREMA_DIGITAL_KEYCHAIN_CAMERA] = { >>> .name = "Suprema Digital Keychain Camera", >>> .flags = GP_DRIVER_STATUS_EXPERIMENTAL, >>> }, >>> }; >>> >>> The association between the two tables can easily be done at the .config >>> ops: >>> >>> /* this function is called at probe time */ >>> static int sd_config(struct gspca_dev *gspca_dev, >>> const struct usb_device_id *id) >>> { >>> >>> ... >>> int board_nr = id->driver_info; >>> >>> printf(KERN_WARN "Detected camera %s\n", cameras[board_nr]); Incidentally, if two cameras have the same USB ID _and_ are otherwise functionally identical (these are not necessarily the same thing) then I think the physical camera will not always be the same as the detected camera. So, while it is always nice to have debug messages, that is not otherwise solving very much. >>> >>> ... >>> } >>> >>> A small change at cx88.pl will be enough to auto-generate gspca.txt. >> >> Mauro, this is impressive. First, you point out that Gphoto could have done >> the job more efficiently, and it seems to me that you are right. Then you >> point out that to make such a change would break compatibility with the >> Linux kernel's struct usb_device_id so it cannot be done that way. Then you >> provide a workaround. It is very clever. > > This kind of trick is used already on several other drivers: cx88, bttv, > em28xx, usbvision, saa7134, ... > >> However, another side of me says that all of this workaround would have to >> get compiled into the code for every gspca driver, making the code for each >> driver, and the resulting binary output, too, in turn longer and more >> complicated. > > Longer: yes. It will generate a longer data segment with the names of all > supported webcams. Could it be that some people who are writing code for RTS systems and small single-purpose systems might scream about unnecessary bloat? > > However, This won't increase the complextiy of the driver. As I showed, you > can know what device you have by just looking at id.driver_info. Also, as > this standard on other drivers, you aren't adding any uncommon weird > behavior. > > Yet, this change would be a big patch, since several drivers already use > device_info for something else. The patch would be trivial though, since we > just need to move the current driver_info information to something inside the > webcam struct. This looks to me like an excellent opportunity to get everyone tied in knots at the same time. Gee. That sounds like fun. Perhaps we could get "the guy" to do it :) ? > >> Therefore, the excursion ends up convincing me that in the first place the >> documentation about which devices are supported, listed by trade name as >> well as USB ID, really ought to be in some kind of place like gspca.txt. It >> is after all much easier to edit a text file than it is to write code, and >> the contents of the text file do not contribute to the growth of size of >> the resulting binary. And it would be at least as easy, at least from this >> point forward, to place the responsibility on the author/maintainer of a >> gspca_* module to provide such a list as part of the procedure for >> submitting code and patches. Then a perl script could, for example, parse >> that one file and put the entries into alphabetical order, or any other >> reasonable and desired order. > > This will only work fine if there are some sanity check script to validate if > all USB ID's are present at gspca.txt. Otherwise, we'll always have the risk > of not having it properly updated. The solution for that is simple. From this day on, anybody who adds a new device to any gspca driver provides the entry for gspca.txt or else the patch is not accepted without suitable revision. Just, first, it has to be decided what kind of thing goes in that file. What is missing is a policy and a clear-cut guideline. > > There is another alternative: add a comment before each new board at gspca's > USB ID's entries. Something like: > > static const __devinitdata struct usb_device_id device_table[] = { > /* Webcam: Sakar Micro Digital 2428x / Jazz JDC9 */ > {USB_DEVICE(0x2770, 0x905c)}, > /* Webcam: Disney pix micro */ > {USB_DEVICE(0x2770, 0x9050)}, > /* Webcam: Suprema Digital Keychain Camera */ > {USB_DEVICE(0x2770, 0x913d)}, > {} > }; > > This could be easily parseable by a script that would generate the gspca.txt. Yes. Existing documentation tools certainly can do something like that, and in fact already do, on fairly routine basis. I don't know about you, but I like this much better. It is simpler, does not screw with the code, and is in plain text. But, alas, it still does not resolve the question of exactly what should go into gspca.txt, and what format that ought to have. > Also, the patch for it can be generated by some script. What I said already. A clear-cut policy and a clear-cut procedure and a clear-cut format for the information to go into gspca.txt (or somewhere else, if that is more appropriate) are what is missing. The job has to get done. Very nice if it is done by a script, too, but that seems to me to be quite secondary. Also, to rely totally on a script might make upset anyone who has been a "good citizen" and has already listed his/her device in gspca.txt in a manner which is acceptable, but now gets asked to put the information in a comment in the source file instead. Theodore Kilgore -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Aug 3, 2009 at 10:30 AM, Jean-Francois Moine<moinejf@free.fr> wrote: > On Sun, 2 Aug 2009 17:25:29 +0400 > Alexey Klimov <klimov.linux@gmail.com> wrote: > >> > + Â Â Â buffer = kmalloc(JEILINJ_MAX_TRANSFER, GFP_KERNEL | >> > GFP_DMA); >> > + Â Â Â if (!buffer) { >> > + Â Â Â Â Â Â Â PDEBUG(D_ERR, "Couldn't allocate USB buffer"); >> > + Â Â Â Â Â Â Â goto quit_stream; >> > + Â Â Â } >> >> This clean up on error path looks bad. On quit_stream you have: >> >> > +quit_stream: >> > + Â Â Â mutex_lock(&gspca_dev->usb_lock); >> > + Â Â Â if (gspca_dev->present) >> > + Â Â Â Â Â Â Â jlj_stop(gspca_dev); >> > + Â Â Â mutex_unlock(&gspca_dev->usb_lock); >> > + Â Â Â kfree(buffer); >> >> kfree() tries to free null buffer after kmalloc for buffer failed. >> Please, check if i'm not wrong. > > Hi Alexey, > > AFAIK, kfree() checks the pointer. > > Cheers. Yes, you're right. I checked the code in kfree(). Sorry for doubts.
diff -r d189fb4be712 linux/Documentation/video4linux/gspca.txt --- a/linux/Documentation/video4linux/gspca.txt Wed Jul 29 10:01:54 2009 +0200 +++ b/linux/Documentation/video4linux/gspca.txt Sat Aug 01 15:42:02 2009 -0500 @@ -239,6 +239,9 @@ pac7311 093a:2629 Genious iSlim 300 pac7311 093a:262a Webcam 300k pac7311 093a:262c Philips SPC 230 NC +jeilinj 0979:0280 Cobra DMC 300 +jeilinj 0979:0280 FlyCamOne +jeilinj 0979:0280 Sakar 57379 zc3xx 0ac8:0302 Z-star Vimicro zc0302 vc032x 0ac8:0321 Vimicro generic vc0321 vc032x 0ac8:0323 Vimicro Vc0323 diff -r d189fb4be712 linux/drivers/media/video/gspca/Kconfig --- a/linux/drivers/media/video/gspca/Kconfig Wed Jul 29 10:01:54 2009 +0200 +++ b/linux/drivers/media/video/gspca/Kconfig Sat Aug 01 15:42:02 2009 -0500 @@ -47,6 +47,15 @@ To compile this driver as a module, choose M here: the module will be called gspca_finepix. +config USB_GSPCA_JEILINJ + tristate "Jeilin JPEG USB V4L2 driver" + depends on VIDEO_V4L2 && USB_GSPCA + help + Say Y here if you want support for cameras based on this Jeilin chip. + + To compile this driver as a module, choose M here: the + module will be called gspca_jeilinj. + config USB_GSPCA_MARS tristate "Mars USB Camera Driver" depends on VIDEO_V4L2 && USB_GSPCA diff -r d189fb4be712 linux/drivers/media/video/gspca/Makefile --- a/linux/drivers/media/video/gspca/Makefile Wed Jul 29 10:01:54 2009 +0200 +++ b/linux/drivers/media/video/gspca/Makefile Sat Aug 01 15:42:02 2009 -0500 @@ -2,6 +2,7 @@ obj-$(CONFIG_USB_GSPCA_CONEX) += gspca_conex.o obj-$(CONFIG_USB_GSPCA_ETOMS) += gspca_etoms.o obj-$(CONFIG_USB_GSPCA_FINEPIX) += gspca_finepix.o +obj-$(CONFIG_USB_GSPCA_JEILINJ) += gspca_jeilinj.o obj-$(CONFIG_USB_GSPCA_MARS) += gspca_mars.o obj-$(CONFIG_USB_GSPCA_MR97310A) += gspca_mr97310a.o obj-$(CONFIG_USB_GSPCA_OV519) += gspca_ov519.o @@ -30,6 +31,7 @@ gspca_conex-objs := conex.o gspca_etoms-objs := etoms.o gspca_finepix-objs := finepix.o +gspca_jeilinj-objs := jeilinj.o gspca_mars-objs := mars.o gspca_mr97310a-objs := mr97310a.o gspca_ov519-objs := ov519.o diff -r d189fb4be712 linux/drivers/media/video/gspca/jeilinj.c --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/linux/drivers/media/video/gspca/jeilinj.c Sat Aug 01 15:42:02 2009 -0500 @@ -0,0 +1,387 @@ +/* + * Jeilinj subdriver + * + * Supports some Jeilin dual-mode cameras which use bulk transport and + * download raw JPEG data. + * + * Copyright (C) 2009 Theodore Kilgore + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#define MODULE_NAME "jeilinj" + +#include <linux/workqueue.h> +#include "gspca.h" +#include "jpeg.h" + +MODULE_AUTHOR("Theodore Kilgore <kilgota@auburn.edu>"); +MODULE_DESCRIPTION("GSPCA/JEILINJ USB Camera Driver"); +MODULE_LICENSE("GPL"); + +/* Default timeouts, in ms */ +#define JEILINJ_CMD_TIMEOUT 500 +#define JEILINJ_DATA_TIMEOUT 1000 + +/* Maximum transfer size to use. */ +#define JEILINJ_MAX_TRANSFER 0x200 + +#define FRAME_HEADER_LEN 0x10 + +/* Structure to hold all of our device specific stuff */ +struct sd { + struct gspca_dev gspca_dev; /* !! must be the first item */ + const struct v4l2_pix_format *cap_mode; + /* Driver stuff */ + struct work_struct work_struct; + struct workqueue_struct *work_thread; + u8 quality; /* image quality */ + u8 jpegqual; /* webcam quality */ + u8 *jpeg_hdr; +}; + + struct jlj_command { + unsigned char instruction[2]; + unsigned char ack_wanted; + }; + +/* AFAICT these cameras will only do 320x240. */ +static struct v4l2_pix_format jlj_mode[] = { + { 320, 240, V4L2_PIX_FMT_JPEG, V4L2_FIELD_NONE, + .bytesperline = 320, + .sizeimage = 320 * 240, + .colorspace = V4L2_COLORSPACE_JPEG, + .priv = 0} +}; + +/* + * cam uses endpoint 0x03 to send commands, 0x84 for read commands, + * and 0x82 for bulk transfer. + */ + +/* All commands are two bytes only */ +static int jlj_write2(struct gspca_dev *gspca_dev, unsigned char *command) +{ + int retval; + + memcpy(gspca_dev->usb_buf, command, 2); + retval = usb_bulk_msg(gspca_dev->dev, + usb_sndbulkpipe(gspca_dev->dev, 3), + gspca_dev->usb_buf, 2, NULL, 500); + if (retval < 0) + PDEBUG(D_ERR, "command write [%02x] error %d", + gspca_dev->usb_buf[0], retval); + return retval; +} + +/* Responses are one byte only */ +static int jlj_read1(struct gspca_dev *gspca_dev, unsigned char response) +{ + int retval; + + retval = usb_bulk_msg(gspca_dev->dev, + usb_rcvbulkpipe(gspca_dev->dev, 0x84), + gspca_dev->usb_buf, 1, NULL, 500); + response = gspca_dev->usb_buf[0]; + if (retval < 0) + PDEBUG(D_ERR, "read command [%02x] error %d", + gspca_dev->usb_buf[0], retval); + return retval; +} + +static int jlj_start(struct gspca_dev *gspca_dev) +{ + int i; + int retval = -1; + u8 response = 0xff; + struct jlj_command start_commands[] = { + {{0x71, 0x81}, 0}, + {{0x70, 0x05}, 0}, + {{0x95, 0x70}, 1}, + {{0x71, 0x81}, 0}, + {{0x70, 0x04}, 0}, + {{0x95, 0x70}, 1}, + {{0x71, 0x00}, 0}, + {{0x70, 0x08}, 0}, + {{0x95, 0x70}, 1}, + {{0x94, 0x02}, 0}, + {{0xde, 0x24}, 0}, + {{0x94, 0x02}, 0}, + {{0xdd, 0xf0}, 0}, + {{0x94, 0x02}, 0}, + {{0xe3, 0x2c}, 0}, + {{0x94, 0x02}, 0}, + {{0xe4, 0x00}, 0}, + {{0x94, 0x02}, 0}, + {{0xe5, 0x00}, 0}, + {{0x94, 0x02}, 0}, + {{0xe6, 0x2c}, 0}, + {{0x94, 0x03}, 0}, + {{0xaa, 0x00}, 0}, + {{0x71, 0x1e}, 0}, + {{0x70, 0x06}, 0}, + {{0x71, 0x80}, 0}, + {{0x70, 0x07}, 0} + }; + for (i = 0; i < ARRAY_SIZE(start_commands); i++) { + retval = jlj_write2(gspca_dev, start_commands[i].instruction); + if (retval < 0) + return retval; + if (start_commands[i].ack_wanted) + retval = jlj_read1(gspca_dev, response); + if (retval < 0) + return retval; + } + PDEBUG(D_ERR, "jlj_start retval is %d", retval); + return retval; +} + +static int jlj_stop(struct gspca_dev *gspca_dev) +{ + int i; + int retval; + struct jlj_command stop_commands[] = { + {{0x71, 0x00}, 0}, + {{0x70, 0x09}, 0}, + {{0x71, 0x80}, 0}, + {{0x70, 0x05}, 0} + }; + for (i = 0; i < ARRAY_SIZE(stop_commands); i++) { + retval = jlj_write2(gspca_dev, stop_commands[i].instruction); + if (retval < 0) + return retval; + } + return retval; +} + +/* This function is called as a workqueue function and runs whenever the camera + * is streaming data. Because it is a workqueue function it is allowed to sleep + * so we can use synchronous USB calls. To avoid possible collisions with other + * threads attempting to use the camera's USB interface the gspca usb_lock is + * used when performing the one USB control operation inside the workqueue, + * which tells the camera to close the stream. In practice the only thing + * which needs to be protected against is the usb_set_interface call that + * gspca makes during stream_off. Otherwise the camera doesn't provide any + * controls that the user could try to change. + */ + +static void jlj_dostream(struct work_struct *work) +{ + struct sd *dev = container_of(work, struct sd, work_struct); + struct gspca_dev *gspca_dev = &dev->gspca_dev; + struct gspca_frame *frame; + int blocks_left; /* 0x200-sized blocks remaining in current frame. */ + int size_in_blocks; + int act_len; + int discarding = 0; /* true if we failed to get space for frame. */ + int packet_type; + int ret; + u8 *buffer; + + buffer = kmalloc(JEILINJ_MAX_TRANSFER, GFP_KERNEL | GFP_DMA); + if (!buffer) { + PDEBUG(D_ERR, "Couldn't allocate USB buffer"); + goto quit_stream; + } + while (gspca_dev->present && gspca_dev->streaming) { + if (!gspca_dev->present) + goto quit_stream; + /* Start a new frame, and add the JPEG header, first thing */ + frame = gspca_get_i_frame(gspca_dev); + if (frame && !discarding) + gspca_frame_add(gspca_dev, FIRST_PACKET, frame, + dev->jpeg_hdr, JPEG_HDR_SZ); + else + discarding = 1; + /* + * Now request data block 0. Line 0 reports the size + * to download, in blocks of size 0x200, and also tells the + * "actual" data size, in bytes, which seems best to ignore. + */ + ret = usb_bulk_msg(gspca_dev->dev, + usb_rcvbulkpipe(gspca_dev->dev, 0x82), + buffer, JEILINJ_MAX_TRANSFER, &act_len, + JEILINJ_DATA_TIMEOUT); + PDEBUG(D_STREAM, + "Got %d bytes out of %d for Block 0", + act_len, JEILINJ_MAX_TRANSFER); + if (ret < 0 || act_len < FRAME_HEADER_LEN) + goto quit_stream; + size_in_blocks = buffer[0x0a]; + blocks_left = buffer[0x0a] - 1; + PDEBUG(D_STREAM, "blocks_left = 0x%x", blocks_left); + packet_type = INTER_PACKET; + if (frame && !discarding) + /* Toss line 0 of data block 0, keep the rest. */ + gspca_frame_add(gspca_dev, packet_type, + frame, buffer + FRAME_HEADER_LEN, + JEILINJ_MAX_TRANSFER - FRAME_HEADER_LEN); + else + discarding = 1; + while (blocks_left > 0) { + if (!gspca_dev->present) + goto quit_stream; + ret = usb_bulk_msg(gspca_dev->dev, + usb_rcvbulkpipe(gspca_dev->dev, 0x82), + buffer, JEILINJ_MAX_TRANSFER, &act_len, + JEILINJ_DATA_TIMEOUT); + if (ret < 0 || act_len < JEILINJ_MAX_TRANSFER) + goto quit_stream; + PDEBUG(D_STREAM, + "%d blocks remaining for frame", blocks_left); + blocks_left -= 1; + if (blocks_left == 0) + packet_type = LAST_PACKET; + else + packet_type = INTER_PACKET; + if (frame && !discarding) + gspca_frame_add(gspca_dev, packet_type, + frame, buffer, + JEILINJ_MAX_TRANSFER); + else + discarding = 1; + } + } +quit_stream: + mutex_lock(&gspca_dev->usb_lock); + if (gspca_dev->present) + jlj_stop(gspca_dev); + mutex_unlock(&gspca_dev->usb_lock); + kfree(buffer); +} + +/* This function is called at probe time just before sd_init */ +static int sd_config(struct gspca_dev *gspca_dev, + const struct usb_device_id *id) +{ + struct cam *cam = &gspca_dev->cam; + struct sd *dev = (struct sd *) gspca_dev; + + dev->quality = 85; + dev->jpegqual = 85; + PDEBUG(D_PROBE, + "JEILINJ camera detected" + " (vid/pid 0x%04X:0x%04X)", id->idVendor, id->idProduct); + cam->cam_mode = jlj_mode; + cam->nmodes = 1; + cam->bulk = 1; + /* We don't use the buffer gspca allocates so make it small. */ + cam->bulk_size = 32; + INIT_WORK(&dev->work_struct, jlj_dostream); + return 0; +} + +/* called on streamoff with alt==0 and on disconnect */ +/* the usb_lock is held at entry - restore on exit */ +static void sd_stop0(struct gspca_dev *gspca_dev) +{ + struct sd *dev = (struct sd *) gspca_dev; + + /* wait for the work queue to terminate */ + mutex_unlock(&gspca_dev->usb_lock); + /* This waits for jlj_dostream to finish */ + destroy_workqueue(dev->work_thread); + dev->work_thread = NULL; + mutex_lock(&gspca_dev->usb_lock); +} + +/* this function is called at probe and resume time */ +static int sd_init(struct gspca_dev *gspca_dev) +{ + return 0; +} + +/* Set up for getting frames. */ +static int sd_start(struct gspca_dev *gspca_dev) +{ + struct sd *dev = (struct sd *) gspca_dev; + int ret; + + /* create the JPEG header */ + dev->jpeg_hdr = kmalloc(JPEG_HDR_SZ, GFP_KERNEL); + jpeg_define(dev->jpeg_hdr, gspca_dev->height, gspca_dev->width, + 0x21); /* JPEG 422 */ + jpeg_set_qual(dev->jpeg_hdr, dev->quality); + PDEBUG(D_STREAM, "Start streaming at 320x240"); + ret = jlj_start(gspca_dev); + if (ret < 0) { + PDEBUG(D_ERR, "Start streaming command failed"); + return ret; + } + /* Start the workqueue function to do the streaming */ + dev->work_thread = create_singlethread_workqueue(MODULE_NAME); + queue_work(dev->work_thread, &dev->work_struct); + + return 0; +} + +/* Table of supported USB devices */ +static const __devinitdata struct usb_device_id device_table[] = { + {USB_DEVICE(0x0979, 0x0280)}, + {} +}; + +MODULE_DEVICE_TABLE(usb, device_table); + +/* sub-driver description */ +static const struct sd_desc sd_desc = { + .name = MODULE_NAME, + .config = sd_config, + .init = sd_init, + .start = sd_start, + .stop0 = sd_stop0, +}; + +/* -- device connect -- */ +static int sd_probe(struct usb_interface *intf, + const struct usb_device_id *id) +{ + return gspca_dev_probe(intf, id, + &sd_desc, + sizeof(struct sd), + THIS_MODULE); +} + +static struct usb_driver sd_driver = { + .name = MODULE_NAME, + .id_table = device_table, + .probe = sd_probe, + .disconnect = gspca_disconnect, +#ifdef CONFIG_PM + .suspend = gspca_suspend, + .resume = gspca_resume, +#endif +}; + +/* -- module insert / remove -- */ +static int __init sd_mod_init(void) +{ + int ret; + + ret = usb_register(&sd_driver); + if (ret < 0) + return ret; + PDEBUG(D_PROBE, "registered"); + return 0; +} + +static void __exit sd_mod_exit(void) +{ + usb_deregister(&sd_driver); + PDEBUG(D_PROBE, "deregistered"); +} + +module_init(sd_mod_init); +module_exit(sd_mod_exit);