diff mbox

[review] gspca - mr97310a: return error instead of -1 in sd_mod_init

Message ID 1238170102.3791.8.camel@tux.localhost (mailing list archive)
State RFC
Headers show

Commit Message

Alexey Klimov March 27, 2009, 4:08 p.m. UTC
Hello, Jean-Francois

What do you think about such small cleanup ?

---
Patch reformats sd_mod_init in the way to make it return error code from
usb_register instead of -1.

Signed-off-by: Alexey Klimov <klimov.linux@gmail.com>

--

Comments

Jean-Francois Moine March 27, 2009, 7:01 p.m. UTC | #1
On Fri, 27 Mar 2009 19:08:22 +0300
Alexey Klimov <klimov.linux@gmail.com> wrote:

> Hello, Jean-Francois
> 
> What do you think about such small cleanup ?
> 
> ---
> Patch reformats sd_mod_init in the way to make it return error code
> from usb_register instead of -1.
> 
> Signed-off-by: Alexey Klimov <klimov.linux@gmail.com>
	[snip]

Applied. Thanks.
kilgota@banach.math.auburn.edu March 28, 2009, 8:08 p.m. UTC | #2
I notice that sq905.c and sq905c.c have now been added to the gspca tree.

But now a question about the documentation. None of that has been 
addressed, as yet, neither for the sq905 nor for the sq905c cameras. The 
code has been added to the tree, but the accompanying documentation 
is, as yet, missing. I would be glad to provide it, but it seems to me 
that a policy question comes up, about just what to add. There are lots of 
cameras.


Here is the problem:

The sq905 module supports, as far as I know, the entire list of cameras 
which are supported by libgphoto2/camlibs/sq905, and probably more which I 
did not list there because I got tired of adding yet another camera when, 
in fact, functionally they were all pretty much equivalent (My bad. I 
know that I missed a few of them, but I was more inexperienced back 
then). In any event, the support for 24 cameras is explicitly listed 
there.

The sq905c module supports, as far as I know, the entire list of cameras 
which are supported by libgphoto2/camlibs/digigr8. The support for 16 
cameras (or 17, if the line in libgphoto2/camlibs/digigr8/library.c 
"Sakar 28290 and 28292  Digital Concepts Styleshot"
which refers to two cameras differing only in the color of the case 
is counted as referring to two cameras).

Thus, if the documentation would be provided in 
linux/Documentation/video4linux/gspca.txt it would amount to a total of 41 
new entries, for these two new modules alone. Should all 41 of them be 
added? I would think that they all ought to be listed somewhere, but 
should the somewhere be there, or somewhere else? That is the question.

One possibility might be to refer the curious reader to the existing list 
in the relevant file in libgphoto2, for example, since these are all 
dual-mode cameras. If that were done, then it would be only needed to put 
the USB Vendor:Product number into the gspca.txt file, along with a 
pointer to the full information.

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
diff mbox

Patch

diff -r 56cf0f1772f7 linux/drivers/media/video/gspca/mr97310a.c
--- a/linux/drivers/media/video/gspca/mr97310a.c	Mon Mar 23 19:18:34 2009 -0300
+++ b/linux/drivers/media/video/gspca/mr97310a.c	Fri Mar 27 01:42:28 2009 +0300
@@ -347,8 +347,10 @@ 
 /* -- module insert / remove -- */
 static int __init sd_mod_init(void)
 {
-	if (usb_register(&sd_driver) < 0)
-		return -1;
+	int ret;
+	ret = usb_register(&sd_driver);
+	if (ret < 0)
+		return ret;
 	PDEBUG(D_PROBE, "registered");
 	return 0;
 }