2008-06-29 06:22:54

by Matti Linnanvuori

Subject: [patch] x86: add compilation checks to pci_unmap_ macros

From: Matti Linnanvuori <[email protected]>

Add compilation checks to pci_unmap_ macros.

Signed-off-by: Matti Linnanvuori <[email protected]>

---

--- a/include/asm-x86/pci_32.h 2008-06-29 08:15:20.129045000 +0300
+++ b/include/asm-x86/pci_32.h 2008-06-29 09:03:29.623927000 +0300
@@ -18,12 +18,14 @@ struct pci_dev;
#define PCI_DMA_BUS_IS_PHYS (1)

/* pci_unmap_{page,single} is a nop so... */
-#define DECLARE_PCI_UNMAP_ADDR(ADDR_NAME)
-#define DECLARE_PCI_UNMAP_LEN(LEN_NAME)
-#define pci_unmap_addr(PTR, ADDR_NAME) (0)
-#define pci_unmap_addr_set(PTR, ADDR_NAME, VAL) do { } while (0)
-#define pci_unmap_len(PTR, LEN_NAME) (0)
-#define pci_unmap_len_set(PTR, LEN_NAME, VAL) do { } while (0)
+#define DECLARE_PCI_UNMAP_ADDR(ADDR_NAME) dma_addr_t ADDR_NAME[0];
+#define DECLARE_PCI_UNMAP_LEN(LEN_NAME) unsigned LEN_NAME[0];
+#define pci_unmap_addr(PTR, ADDR_NAME) sizeof((PTR)->ADDR_NAME)
+#define pci_unmap_addr_set(PTR, ADDR_NAME, VAL) \
+ do { break; } while (pci_unmap_addr(PTR, ADDR_NAME))
+#define pci_unmap_len(PTR, LEN_NAME) sizeof((PTR)->LEN_NAME)
+#define pci_unmap_len_set(PTR, LEN_NAME, VAL) \
+ do { break; } while (pci_unmap_len(PTR, LEN_NAME))


#endif /* __KERNEL__ */

2008-06-29 12:52:48

by Jan Engelhardt

Subject: Re: [patch] x86: add compilation checks to pci_unmap_ macros

On Sunday 2008-06-29 08:22, Matti Linnanvuori wrote:
>+#define DECLARE_PCI_UNMAP_ADDR(ADDR_NAME) dma_addr_t ADDR_NAME[0];
>+#define DECLARE_PCI_UNMAP_LEN(LEN_NAME) unsigned LEN_NAME[0];
>+#define pci_unmap_addr(PTR, ADDR_NAME) sizeof((PTR)->ADDR_NAME)
>+#define pci_unmap_addr_set(PTR, ADDR_NAME, VAL) \
>+ do { break; } while (pci_unmap_addr(PTR, ADDR_NAME))

Mh. If addr_name happens to be non-0 for some reason (like, a typo),
then this just introduces a lockup.
(Better to have a no-op with a typo than a sudden lockup. Given
that these are no-ops means they are deprecated, is not it the case?)

2008-06-30 06:35:00

by Matti Linnanvuori

Subject: Re: [patch] x86: add compilation checks to pci_unmap_ macros

2008/6/29 Jan Engelhardt <[email protected]>:
>
> On Sunday 2008-06-29 08:22, Matti Linnanvuori wrote:
>>+#define DECLARE_PCI_UNMAP_ADDR(ADDR_NAME) dma_addr_t ADDR_NAME[0];
>>+#define DECLARE_PCI_UNMAP_LEN(LEN_NAME) unsigned LEN_NAME[0];
>>+#define pci_unmap_addr(PTR, ADDR_NAME) sizeof((PTR)->ADDR_NAME)
>>+#define pci_unmap_addr_set(PTR, ADDR_NAME, VAL) \
>>+ do { break; } while (pci_unmap_addr(PTR, ADDR_NAME))
>
> Mh. If addr_name happens to be non-0 for some reason (like, a typo),
> then this just introduces a lockup.
> (Better to have a no-op with a typo than a sudden lockup.

No, it does not lock up because there is a break statement in the do block.

> Given
> that these are no-ops means they are deprecated, is not it the case?)

I don't think they are deprecated. There is no text about the
deprecation in kernel documents.

2008-06-30 10:18:46

by Ingo Molnar

Subject: Re: [patch] x86: add compilation checks to pci_unmap_ macros

* Jan Engelhardt <[email protected]> wrote:

> [...] Given that these are no-ops means they are deprecated, is not it
> the case?)

see higher up in the file:

/* Dynamic DMA mapping stuff.
* i386 has everything mapped statically.
*/

because everything is 1:1 mapped on 32-bit it's a NOP.

Ingo

2008-06-30 10:23:56

by Ingo Molnar

Subject: Re: [patch] x86: add compilation checks to pci_unmap_ macros

* Matti Linnanvuori <[email protected]> wrote:

> From: Matti Linnanvuori <[email protected]>
>
> Add compilation checks to pci_unmap_ macros.

> -#define DECLARE_PCI_UNMAP_ADDR(ADDR_NAME)
> -#define DECLARE_PCI_UNMAP_LEN(LEN_NAME)
> -#define pci_unmap_addr(PTR, ADDR_NAME) (0)
> -#define pci_unmap_addr_set(PTR, ADDR_NAME, VAL) do { } while (0)
> -#define pci_unmap_len(PTR, LEN_NAME) (0)
> -#define pci_unmap_len_set(PTR, LEN_NAME, VAL) do { } while (0)
> +#define DECLARE_PCI_UNMAP_ADDR(ADDR_NAME) dma_addr_t ADDR_NAME[0];
> +#define DECLARE_PCI_UNMAP_LEN(LEN_NAME) unsigned LEN_NAME[0];
> +#define pci_unmap_addr(PTR, ADDR_NAME) sizeof((PTR)->ADDR_NAME)
> +#define pci_unmap_addr_set(PTR, ADDR_NAME, VAL) \
> + do { break; } while (pci_unmap_addr(PTR, ADDR_NAME))
> +#define pci_unmap_len(PTR, LEN_NAME) sizeof((PTR)->LEN_NAME)
> +#define pci_unmap_len_set(PTR, LEN_NAME, VAL) \
> + do { break; } while (pci_unmap_len(PTR, LEN_NAME))

applied to tip/x86/cleanups - thanks Matti.

Would you be interested in doing a small cleanup as well and convert the
parameter names to non-shouting lower-case letters? (If you do it then
please do it as a delta patch as i've already applied your current
patch.)

Ingo