On Wed, 27 Sep 2006 13:16:36 +0200 (CEST)
Patrick Boettcher <[EMAIL PROTECTED]> wrote:

> Hi Micheal,
> 
> Attached you'll find my suggestions to your current tree.

-#include "m920x.h"
+#define DVB_USB_LOG_PREFIX "m920x"
+#include "dvb-usb.h"

-#ifndef _DVB_USB_M920X_H_
-#define _DVB_USB_M920X_H_
 
-#define DVB_USB_LOG_PREFIX "m920x"
-#include "dvb-usb.h"
-
-extern int dvb_usb_m920x_debug;
-#define deb_rc(args...)   dprintk(dvb_usb_m920x_debug,0x01,args)
-
-#endif

New defines in m920x.h .

-       if (mutex_lock_interruptible(&d->i2c_mutex) < 0)
+       if (mutex_lock_interruptible(&d->i2c_mutex) < 0) /* I'm not
sure about using the i2c_mutex here is a good idea - maybe it is better
to create m920x_state and put a device-specific mutex or use the
usb_mutex from dvb_usb_device */

You can actually drop this altogether.
M9206_CORE, M9206_I2C, M9206_FILTER and M9206_FW can be accessed
concurrently.

-                       if (msg[i].addr == 0x1e)
+                       if (msg[i].addr == 0x1e) // This screams for a
comment or for another solution.

Given in my patch. I wouldn't try to fix it until we know whether they
are different on different tuners or demods.

-       if (pid == 8192)
+       if (pid == 8192) /* handling this special PID should be in
dvb-usb-dvb and should call the pid_filter_ctrl-ops */

This is now done differently because I think this might fall when both
of the disabling methods are used at the same time. The way pid filters
need to be set seems a bit picky also.


> Patrick.
> 
> On Tue, 26 Sep 2006, Michael Krufky wrote:
> 
> > Patrick,
> > 
> > Could you look over the changesets in this tree and let me know
> > whether or not you approve of this?
> > 
> > Aapo,
> > 
> > After you sent your patch in to the linux-dvb mailing list, Patrick
> > had some comments and requests for certain things to be cleaned
> > up.  I have not seen any response from you about those requests.
> > 
> > In an effort to move things along, I have taken it upon myself to
> > clean up your patch.  I've updated it to comply with some of the
> > recent changes in the dvb-usb structure, and to use the new
> > dvb_attach() method.
> > 
> > I have also separated the qt1010 tuner code into a separate header
> > file. It has come to my attention, thanks to Markus Rechberger,
> > that there may be more fixing to do with regards to the qt1010
> > stuff... but that can be done later, in a separate patch.
> > 
> > I'd like it if you could test the updated tree and confirm that it
> > works as expected with your device.  Please clone the following
> > tree:
> > 
> > http://linuxtv.org/hg/~mkrufky/m920x
> > 
> > You will notice that I have decided to rename the driver from
> > megasky to m920x -- I did this for a few reasons:
> > 
> > 1) There are probably other devices out there based on the m9205,
> > m9206 and m9207 chipsets ... Adding support for those devices will
> > probably be a matter of updating the code in this driver, in which
> > case, the name 'megasky' may be inappropriate.
> > 
> > 2) There is another device out there, called "MSI Megasky 580",
> > based on the gl861 chipset, using the same exact usb id as your
> > device based on the m9206 chipset.  This means that MSI released
> > two completely different devices, each with the same exact name and
> > USB ID!
> > 
> > 3) We should name the driver based on the chipset it uses, rather
> > than the Vendor's retail name of the product.
> > 
> > Please let me know what happens.
> > 
> > Regards,
> > 
> > Michael Krufky
> > 

-- 
Aapo Tahkola

_______________________________________________
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb

Reply via email to