meye driver update

PATCH In review Stelian Pop 3 participants 6 messages Jun 15, 2003 – Jun 16, 2003 Media (V4L/DVB)

lore

Share & tools

lore

Stelian Pop

author

2003-06-15 16:18 UTC #0 lore raw

PATCH 1/1 meye driver update

Hi,

Here is my latest meye driver version, ready to be integrated in the next -pre cycle.

The attached patch:
* replaces the pci_alloc_consistent calls with dma_alloc_coherent because we want to do the allocations at GFP_KERNEL priority and not GFP_ATOMIC because we request quite a bit of memory and the allocation fails quite frequently.

It would be better to have a pci_alloc_consistent with an extra parameter (priority) but since we haven't, and the meye driver is supposed to work only on ix86 platforms we can safely do the change. Moreover, in 2.4 we don't even have a dma_alloc_coherent function, so the meye driver is carrying a local version, backported from 2.5.

* fixes the DMA engine stop request. The hard freezes encountered when using this driver and repeatedly opening/closing the device have been tracked down to this particular piece of code. The new version seems to work way better, I haven't had a single freeze since the change.

Marcelo, please apply.

Stelian.

===== drivers/media/video/meye.h 1.7 vs edited =====

drivers/media/video/meye.h +1 -1

@@ -31,7 +31,7 @@
 #define _MEYE_PRIV_H_
 
 #define MEYE_DRIVER_MAJORVERSION	1
-#define MEYE_DRIVER_MINORVERSION	6
+#define MEYE_DRIVER_MINORVERSION	7
 
 #include <linux/config.h>
 #include <linux/types.h>

===== drivers/media/video/meye.c 1.13 vs edited =====

drivers/media/video/meye.c +59 ~~-29~~

146 lines not on this page · raw message

Signature · 1 line

Stelian Pop <[email protected]>

Michael Buesch

2003-06-15 16:53 UTC #1 lore raw

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 1 line · Stelian Pop

Hi,

Hi Stelian,

Quoted 1 line

+void dma_free_coherent(struct pci_dev *dev, size_t size,

                          ^^^^^^^^^^^^^^^^^^^

Quoted 1 line

+ void *vaddr, dma_addr_t dma_handle)

                                         ^^^^^^^^^^^^^^^^^^^^^

Why do you define these unused parameters?

drivers/media/video/meye.c line 190

Quoted 3 lines

drivers/media/video/meye.c +3 -0

+{
+        free_pages((unsigned long)vaddr, get_order(size));
+}

And why are they defined in 2.5, too, althought unused. Is there some reason?

- --
Regards Michael Büsch
http://www.8ung.at/tuxsoft
19:00:48 up 2:45, 2 users, load average: 1.37, 1.13, 1.08

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)

iD8DBQE+7KexoxoigfggmSgRAsWrAKCBMmD+2hp5CIE00eoZNNNsXNKLdgCgitmg NI+nrOdvLnEXmHFv7hopBNk=
=DaIp
-----END PGP SIGNATURE-----

Stelian Pop

author

2003-06-15 21:15 UTC #2 lore raw

Quoted 6 lines · Michael Buesch

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Quoted 1 line · Stelian Pop
Hi,

Quoted 7 lines

Hi Stelian,
Quoted 1 line
+void dma_free_coherent(struct pci_dev *dev, size_t size,
                          ^^^^^^^^^^^^^^^^^^^
Quoted 1 line
+ void *vaddr, dma_addr_t dma_handle)
                                         ^^^^^^^^^^^^^^^^^^^^^
Why do you define these unused parameters?

Because it's backported from 2.5, and I took it as it, without editing.

Quoted 6 lines

drivers/media/video/meye.c line 190
Quoted 3 lines
drivers/media/video/meye.c +3 -0
+{
+        free_pages((unsigned long)vaddr, get_order(size));
+}
And why are they defined in 2.5, too, althought unused. Is there some reason?

Unused of ix86 because bus addresses are the same as virtual addresses. This is not however the case on other architetures, see Documentation/DMA-mapping.txt.

Stelian.

Signature · 1 line

Stelian Pop <[email protected]>

Stelian Pop

2003-06-16 05:51 UTC #3 lore raw

Quoted 2 lines · Stelian Pop

Unused of ix86 because bus addresses are the same as virtual addresses.

Of course I meant that bus addresses are the same as the _physical_ addresses...

I should really sleep first, then reply, not the other way around...

Stelian.

Signature · 2 lines

Stelian Pop <[email protected]>
Alcove - http://www.alcove.com

Michael Buesch

2003-06-16 14:57 UTC #4 lore raw

In reply to Stelian Pop

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 28 lines · Stelian Pop

Quoted 8 lines · Michael Buesch
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Quoted 1 line · Stelian Pop
Hi,
Hi Stelian,
Quoted 8 lines
Quoted 1 line
+void dma_free_coherent(struct pci_dev *dev, size_t size,
                          ^^^^^^^^^^^^^^^^^^^
Quoted 1 line
+ void *vaddr, dma_addr_t dma_handle)
                                         ^^^^^^^^^^^^^^^^^^^^^
Why do you define these unused parameters?
Because it's backported from 2.5, and I took it as it, without editing.
Quoted 6 lines
drivers/media/video/meye.c line 190
Quoted 3 lines
drivers/media/video/meye.c +3 -0
+{
+        free_pages((unsigned long)vaddr, get_order(size));
+}
And why are they defined in 2.5, too, althought unused. Is there some reason?

Quoted 3 lines

Unused of ix86 because bus addresses are the same as virtual addresses. This is not however the case on other architetures, see Documentation/DMA-mapping.txt.

Yes, I now understand, why it is in 2.5, but isn't it better, to remove these parameters from your patch, because it is only a local copy in your driver and not used by any other drivers. And your driver is (if I didn't miss something) for running on i386 only; there is no other version of this function for other architectures, that need these parameters.
So wouldn't it be better to simply remove them, because they are completely useless. IMHO their
just confusing.

Quoted 2 lines

Stelian.

- --
Regards Michael Büsch
http://www.8ung.at/tuxsoft
16:49:26 up 8 min, 1 user, load average: 1.07, 0.96, 0.49

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)

iD8DBQE+7do8oxoigfggmSgRAjbxAJsF5CkukHoBH5OEWE3ZRRDtfQKMGQCgjLTK LWBs80NR2u/VcXW80FEfXY8=
=7/iJ
-----END PGP SIGNATURE-----

Stelian Pop

2003-06-16 15:23 UTC #5 lore raw

Quoted 4 lines · Michael Buesch

Quoted 3 lines
Unused of ix86 because bus addresses are the same as virtual addresses. This is not however the case on other architetures, see Documentation/DMA-mapping.txt.

Quoted 9 lines

Yes, I now understand, why it is in 2.5, but isn't it better, to remove these parameters from your patch, because it is only a local copy in your driver and not used by any other drivers. And your driver is (if I didn't miss something) for running on i386 only; there is no other version of this function for other architectures, that need these parameters.
So wouldn't it be better to simply remove them, because they are completely useless. IMHO their
just confusing.

No, I hope that the DMA allocation function will be backported in the near future from 2.5 into 2.4. If I get some time, I may even do it myself and propose a patch.

And when it's done, I'll just have to remove the two definitions without changing anything in my code and it will just work.

Besides, having two versions (2.4 and 2.5) which don't diverge to much is much easier for the maintainer.

Stelian.

Signature · 2 lines

Stelian Pop <[email protected]>
Alcove - http://www.alcove.com