Hi,
On Fri, 20 Nov 2009 08:14:10 +0100
Németh Márton <[email protected]> wrote:
> Hans de Goede wrote:
> > On 11/19/2009 08:46 AM, Németh Márton wrote:
> >> Add helper functions for interrupt endpoint based input handling.
> > First of all many many thanks for doing this!
>
> You are welcome :-) . My goal is to just make my webcam working
> properly...
Many thanks from me too. This job will be useful for other webcams.
[snip]
> > I'm personally not a big fan of adding more configuration options,
> > what should be done instead is make the compilation dependent on the
> > CONFIG_INPUT kernel config option, I see no reason not to enable
> > this when CONFIG_INPUT is enabled.
>
> I added dependency on CONFIG_INPUT.
The option USB_GSPCA_SN9C20X_EVDEV should be removed too.
> > Some other remarks, you are using:
> > printk(KERN_DEBUG
> > In various places, please use
> > PDEBUG(D_FOO
> > instead so that the output can be controlled using the gspca
> > module's debug parameter.
>
> I created a PDEBUG_INPUT() for this otherwise there is a circular
> dependency between gspca_main and gspca_input because of the variable
> gspca_debug.
That is because you created a separate module.
> > And in gspca_input_connect() you are setting name to "pac7302", this
> > needs to be generalized somehow,
>
> I use now gspca_dev->sd_desc->name.
OK for me.
> > and also you are not setting the
> > input device's parent there, I think we need to fix that too
> > (although I'm not sure what it should be set to).
>
> I don't know what to use there, maybe somebody on the linux-input
> mailing list could tell.
sn9c20x sets it to &gspca_dev->dev->dev.
> Also, I am not sure about setting of input_dev->id.version.
It seems it can be EV_VERSION only.
> Unfortunately I still get the following error when I start streaming,
> stop streaming or unplug the device:
>
> [ 6876.780726] uhci_hcd 0000:00:10.1: dma_pool_free buffer-32,
> de0ad168/1e0ad168 (bad dma)
As there is no 'break' in gspca_input_create_urb(), many URBs are
created.
> Please find the new version of this patch later in this mail.
Here are some other remarks:
- As the input functions are called from the gspca main only, and as
they cannot be used by other drivers, there is no need to have a
separate module.
- Almost all other webcams who have buttons ask for polling. So, the
'int_urb' should be pac7302 dependent (in 'struct sd' and not in
'struct gspca_dev').
Cheers.
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
