Hi Juho,

On Friday 28 November 2008, Juho Vähä-Herttua wrote:
> Hi,
>
> First I have to say I'm sorry for a bit slow reply, the patch didn't
> work out of the box and I had to check out where the problem was...

No worries.

> On 26.11.2008, at 5.21, Laurent Pinchart wrote:
> > This is not completely true. Controls are added to the entity
> > controls list in ascending index order, so manual controls will always
> > come before auto controls (at least for the processing unit).
>
> Ah, in that case it is true because of how they are sorted. I didn't
> dare to count on it though...
>
> >> However it works again for me and that's what matters now. I hope
> >> to get this thing resolved some day in the official version too
> >> though. :) 
> >
> > Let's work on this then :-)
> >
> > The patch indeed looked a bit messy at first. I've been trying to rework
> > it for the past hour but ran into various issues and I now understand
> > implementing a clean solution is not as easy as it might seem. 
> >
> > There are several places where control blacklisting can be implemented. If
> > we want to use information taken from uvc_control_info (such as the
> > control selector) the location you picked is the right one. However, the
> > driver then needs to walk the entities and controls lists over again and
> > free data allocated during the initialisation process.
> >
> > Another solution would be to blacklist controls before they are allocated.
> > We would then process the bmControls bitmask instead of the controls list.
> > This approach is more efficient, but probably less generic as it won't
> > allow the driver to blacklist controls based on advanced information taken
> > from uvc_control_info.
> >
> > As both solutions have advantages and drawbacks let's remember Linus
> > Torvald and his famous quote about overdesign. Here's a simplified version
> > of the patch that optionally (based on a device quirk) removes processing
> > unit auto controls when no corresponding manual control is available.
> >
> > Could you please test the patch and report the results ?
>
> I agree that this version is cleaner than mine, at least it doesn't
> free memory blocks and hold the mutex while doing it. :)

It's also much simpler, so I might have to redo it if more complex needs arise 
later.

> Also adding the device quirk was something that I was thinking should be
> added but wanted to ask for comments first.
>
> The latest patch had a small thinko though, you wrote "entity->extension" in
> the controls and size getting when it should be "entity->processing" in this
> case. I'll attach a new tested version of the patch where that is fixed, it
> should have no other changes.

Oops. Thanks for spotting this.

> Thanks again and tell let me know if there's something else I could/should
> do. :) 

You're welcome. I've committed the change to my repository on linuxtv.org and 
will push it upstream soon.

Best regards,

Laurent Pinchart
_______________________________________________
Linux-uvc-devel mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/linux-uvc-devel

Reply via email to