[sane-devel] MP730 driver broken since pixma-0.12.2
Wade Fitzpatrick
wade.fitzpatrick at gmail.com
Sat Apr 25 12:14:25 UTC 2009
Nicolas
My latest testing was on kernel 2.6.27-gentoo-r8. I don't see why a
different kernel version would change the interface/endpoint numbering
anyway - surely that is hardware specific to the MP730. I have the latest
version of everything: kernel 2.6.27-gentoo-r8, libusb-0.1.12-r5, sane
backends 1.0.20cvs. It's not a version problem, the MP730 has *never* worked
using libsane. It worked enough to do scans using Wittawat Yamwong's
``scan'' utility until pixma-0.12.1 but never using libsane. We just never
got that far in testing it back then.
Ubuntu users also complained about it but obviously never bothered to ask on
sane-devel.
http://ubuntuforums.org/showthread.php?t=592643&highlight=mp730
http://ubuntuforums.org/showthread.php?t=962535&highlight=mp730
I didn't mean to suggest that the patch I have is ready for inclusion in
CVS, only that it should apply to current if somebody wants to use it for
testing. I'm well aware that fixing this properly will mean a significant
rewrite of sanei/sanei_usb.c and will probably touch most if not all backend
drivers if you solve it the way I would, which is to specify the requisite
interfaces and endpoints in the [backend]_config_t structure and pass that
into sanei_usb code.
Let's take the remaining problems into another thread.
Cheers,
Wade.
2009/4/25 Nicolas Martin <nicolas.martin at freesurf.fr>
> Hi Wade,
>
> Sorry if act sometimes like "guardians of the temple", but we need to be
> very accurate on the changes we make into the code, as there are lots of
> shared code in Sane backends (especially pixma) and libs.
>
> - So for points 1 and 2: we can close those.
>
> - Point 3: this is still to investigate, as we must be sure this
> endpoint issue is not brought by kernel 2.6.18-ck1 (BTW, do you know
> what does the -ck1 bring ?). This ck kernel is dated sept-2006 (on
> kernel.org), and after googling a little bit "2.6.18 usb endpoints" it
> looks like there are some issues around that...
> My suggestion would still be to test with a recent kernel, with which we
> are sure Sane and endpoints work fine (recent Ubuntu distros can help a
> lot for that) and check if we can reproduce your endpoints issue.
> My fear here is not to introduce into the Sane code, some turnarounds
> and tweaks only for specific kernels, buggy on usb.
>
> Concerning points 1 and 2 you raise, this is interesting:
>
> The pixma_mp730.c file has been left almost untouched for a while (no
> requests or testing ability so far), so it may be in a less "enhanced"
> state than other ones covering more recent Pixma models, that have been
> recently updated.
> Dennis Lou wrote a while back the pixma_imageclass.c file, based on the
> pixma_mp730.c file, and which covers several Canon ImageClass models in
> the pixma backend. The code has been nicely brushed up, and is now in a
> better shape than mp730's one.
> So for the 2 issues you raise, I'll take a look and compare the
> pixma_mp730.c code with pixma_imageclass.c and pixma_mp150.c , probably
> will help a lot to solve those issues, thanks to your cooperation for
> testing and feedback.
>
> Nicolas
>
>
> Le vendredi 24 avril 2009 à 01:19 +1000, Wade Fitzpatrick a écrit :
> > Hi Nicolas
> >
> > Thanks for reviewing my patches. I have to agree with you... too many
> > late nights... must be coding in my sleep...
> > 1. I had the PIXMA_CAP_EVENTS flag removed at some stage as
> > that's what got the driver working again between pixma-0.12.1
> > and pixma-0.12.2. It doesn't seem to make any difference now
> > and I can't explain why so just ignore it. I don't think the
> > resolution and maximum scan area parameters are correct but
> > that's an issue for another day.
> > 2. I don't remember adding that line but I guess I must have. It
> > may have been an accidental 'p' in vim. Ignore this one too.
> > 3. I checked out a fresh new cvs today and applied just the lines
> > you pasted below to sanei/sanei_usb.c - it works just fine
> > with that change alone. I ran through about 30 successful
> > tests too: using ADF, button-controlled, ADF
> > +button-controlled, Gray, Color and at various resolutions and
> > geometries on and off the platen glass, pressing Cancel during
> > a scan, pressing Ctrl-C during a scan, jamming the paper in
> > the ADF, batch scans etc.
> > The only problems I can find now are:
> > 1. Scanning at 1200dpi Grayscale. Sometimes it hangs depending on
> > the x and y values - the larger the values, the sooner it
> > fails. If it does succeed, the data is always bogus.
> > 2. The return code after a successful scan is always ECANCELED
> > which causes a problem for batch mode as it will only scan one
> > page. It doesn't seem to matter for single scans.
> > Does this hold true for other models too?
> > Does batch mode succeed on other models with ADF?
> >
> > I can't believe I've spent so much time on such a trivial change :(
> >
> > Regards,
> > Wade.
> > 2009/4/23 Nicolas <nicolas0martin at gmail.com>
> > Hi Wade,
> >
> > Good news that you succeeded in having your MP730 work after
> > 2,5 years !
> >
> > I did carefully inspect the small changes you have done to the
> > pixma backend, but there are some points I don't understand
> > clearly, or maybe I've missed sthg, but you can surely give us
> > some more info here:
> >
> > 1/ File backend/pixma_mp730.c
> >
> > I don't understand the change you've brought here individually
> > for each device, the line that you replaced for MP730:
> >
> > - DEVICE ("Canon MultiPASS MP730", "MP730", MP730_PID, 1200,
> > 637, 868, PIXMA_CAP_ADF),
> > + DEVICE ("Canon MultiPASS MP730", "MP730", MP730_PID, 1200,
> > 637, 868, PIXMA_CAP_ADF|PIXMA_CAP_EVENTS),
> >
> > has exactly the same effect than the original one in the
> > device definition:
> > - PIXMA_CAP_GRAY|PIXMA_CAP_EVENTS|cap \
> > + PIXMA_CAP_GRAY|cap /* capabilities */
> > \
> >
> > Here, you move the PIXMA_CAP_EVENTS flag from the global
> > device definition to the individual definition of each device.
> >
> > - For MP730, this looks to have the same effect, or where is
> > the difference ?
> > - But this changes the definition for all other devices here,
> > and I would not like to bring here any regression to other
> > devices.
> >
> > So: could you give the motivation for this modification here ?
> >
> > 2/ file pixma_io_sanei.c
> >
> > The only diff I see here adds a PDBG debug statement:
> > + PDBG (pixma_dbg (3, "sanei_usb_read_bulk returned error
> > code: %i\n", error));
> >
> > The other line is a commented line:
> >
> > + /*error = pixma_map_status_errno (pixma_get_be16
> > (buf)); */
> >
> > And I don't understand the comment you added:
> > "The call to |pixma_map_status_errno() in |
> > backend/pixma_io_sane.c:pixma_read()
> > is in the wrong place as |pixma_read() may be re-entrant so as
> > to read a partial buffer."
> >
> > - There are _no_ calls to |pixma_map_status_errno() in current
> > cvs ||pixma_read()
> > The one here is added as part of you modifications, but is
> > commented out.
> > - This part of the pixma code is common to all pixma devices
> > - The modifications you added here do not bring any functional
> > change.
> >
> > So one again, what are those changes for ?
> >
> > 3/ File sanei_usb.c
> >
> > This is the only file where modifications appear to have a
> > functional impact:
> > |+ /* don't try to read non-scanner device
> > classes */
> > + if (interface->bInterfaceClass == 0x07) {
> > + DBG (3, "sanei_usb_open: skipping Printer
> > interface\n");
> > + continue;
> > + }
> > + if (interface->bInterfaceClass == 0x08) {
> > + DBG (3, "sanei_usb_open: skipping Mass
> > Storage interface\n");
> > + continue;
> > + }
> > + if (interface->bInterfaceClass == 0xff && i
> > == 3) {
> > + DBG (3, "sanei_usb_open: skipping second
> > Vendor Specific interface\n");
> > + continue;
> > + }
> >
> > As Allan pointed out, this may need some deeper analysis to
> > the sanei_usb code, but modifications in this file are very
> > sensitive, as they impact all usb devices
> > supported by Sane, not only pixma.
> > As for the 1.0.20 release, I would not recommend to add this
> > change now, as it probably needs deeper testing and
> > understanding.
> >
> > In other words, to step further, Wade:
> > Could you give a try with the current Sane CVS files and patch
> > only file sanei_usb.c, see if it is still ok?
> > If not, this means there's something I did not catch, so we'll
> > need to talk again to finalize the changes, if any, required
> > for MP730.
> >
> > Nicolas
>
> >
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.alioth.debian.org/pipermail/sane-devel/attachments/20090425/6f8acd30/attachment.htm>
More information about the sane-devel
mailing list