Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[PATCH -mm] replace DECLARE_DMA_UNMAP_ADD_{ADDR,LEN} with DEFINE_DMA_UNMAP_ADD_{ADDR,LEN}

0 views
Skip to first unread message

FUJITA Tomonori

unread,
Mar 3, 2010, 12:00:02 AM3/3/10
to
Seems that you missed the following patch?

http://lkml.org/lkml/2010/2/13/2

This can be fold into dma-mappingh-add-the-dma_unmap-state-api.patch.

Thanks,

=
From: FUJITA Tomonori <fujita....@lab.ntt.co.jp>
Subject: [PATCH -mm] replace DECLARE_DMA_UNMAP_ADD_{ADDR,LEN} with DEFINE_DMA_UNMAP_ADD_{ADDR,LEN}

Andrew pointed out:

- adding the semicolons at the end of DECLARE_DMA_UNMAP_{ADDR|LEN}
confuses people.

- they are "definitions", not "declarations".

Signed-off-by: FUJITA Tomonori <fujita....@lab.ntt.co.jp>
---
Documentation/DMA-API.txt | 8 +++-----
include/linux/dma-mapping.h | 8 ++++----
include/linux/pci-dma.h | 12 ++++++------
3 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/Documentation/DMA-API.txt b/Documentation/DMA-API.txt
index d7d9eef..0fc5728 100644
--- a/Documentation/DMA-API.txt
+++ b/Documentation/DMA-API.txt
@@ -494,7 +494,7 @@ portable API) the following facilities are provided.
Actually, instead of describing the macros one by one, we'll
transform some example code.

-1) Use DECLARE_DMA_UNMAP_{ADDR,LEN} in state saving structures.
+1) Use DEFINE_DMA_UNMAP_{ADDR,LEN} in state saving structures.
Example, before:

struct ring_state {
@@ -507,12 +507,10 @@ transform some example code.

struct ring_state {
struct sk_buff *skb;
- DECLARE_DMA_UNMAP_ADDR(mapping)
- DECLARE_DMA_UNMAP_LEN(len)
+ DEFINE_DMA_UNMAP_ADDR(mapping);
+ DEFINE_DMA_UNMAP_LEN(len);
};

-NOTE: DO NOT put a semicolon at the end of the DECLARE_*() macro.
-
2) Use dma_unmap_{addr,len}_set to set these values.
Example, before:

diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 599d8e4..e00c5c9 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -241,15 +241,15 @@ struct dma_attrs;
#endif /* CONFIG_HAVE_DMA_ATTRS */

#ifdef CONFIG_NEED_DMA_MAP_STATE
-#define DECLARE_DMA_UNMAP_ADDR(ADDR_NAME) dma_addr_t ADDR_NAME;
-#define DECLARE_DMA_UNMAP_LEN(LEN_NAME) __u32 LEN_NAME;
+#define DEFINE_DMA_UNMAP_ADDR(ADDR_NAME) dma_addr_t ADDR_NAME
+#define DEFINE_DMA_UNMAP_LEN(LEN_NAME) __u32 LEN_NAME
#define dma_unmap_addr(PTR, ADDR_NAME) ((PTR)->ADDR_NAME)
#define dma_unmap_addr_set(PTR, ADDR_NAME, VAL) (((PTR)->ADDR_NAME) = (VAL))
#define dma_unmap_len(PTR, LEN_NAME) ((PTR)->LEN_NAME)
#define dma_unmap_len_set(PTR, LEN_NAME, VAL) (((PTR)->LEN_NAME) = (VAL))
#else
-#define DECLARE_DMA_MAP_ADDR(ADDR_NAME)
-#define DECLARE_DMA_UNMAP_LEN(LEN_NAME)
+#define DEFINE_DMA_MAP_ADDR(ADDR_NAME)
+#define DEFINE_DMA_UNMAP_LEN(LEN_NAME)
#define dma_unmap_addr(PTR, ADDR_NAME) (0)
#define dma_unmap_addr_set(PTR, ADDR_NAME, VAL) do { } while (0)
#define dma_unmap_len(PTR, LEN_NAME) (0)
diff --git a/include/linux/pci-dma.h b/include/linux/pci-dma.h
index 235a61e..549a041 100644
--- a/include/linux/pci-dma.h
+++ b/include/linux/pci-dma.h
@@ -1,11 +1,11 @@
#ifndef _LINUX_PCI_DMA_H
#define _LINUX_PCI_DMA_H

-#define DECLARE_PCI_UNMAP_ADDR DECLARE_DMA_UNMAP_ADDR
-#define DECLARE_PCI_UNMAP_LEN DECLARE_DMA_UNMAP_LEN
-#define pci_unmap_addr dma_unmap_addr
-#define pci_unmap_addr_set dma_unmap_addr_set
-#define pci_unmap_len dma_unmap_len
-#define pci_unmap_len_set dma_unmap_len_set
+#define DECLARE_PCI_UNMAP_ADDR(ADDR_NAME) DEFINE_DMA_UNMAP_ADDR(ADDR_NAME);
+#define DECLARE_PCI_UNMAP_LEN(LEN_NAME) DEFINE_DMA_UNMAP_LEN(LEN_NAME);
+#define pci_unmap_addr dma_unmap_addr
+#define pci_unmap_addr_set dma_unmap_addr_set
+#define pci_unmap_len dma_unmap_len
+#define pci_unmap_len_set dma_unmap_len_set

#endif
--
1.6.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Randy Dunlap

unread,
Mar 3, 2010, 6:20:03 PM3/3/10
to
On 03/02/10 20:50, FUJITA Tomonori wrote:
> Seems that you missed the following patch?
>
> http://lkml.org/lkml/2010/2/13/2
>
> This can be fold into dma-mappingh-add-the-dma_unmap-state-api.patch.
>
> Thanks,
>
> =
> From: FUJITA Tomonori <fujita....@lab.ntt.co.jp>
> Subject: [PATCH -mm] replace DECLARE_DMA_UNMAP_ADD_{ADDR,LEN} with DEFINE_DMA_UNMAP_ADD_{ADDR,LEN}
>
> Andrew pointed out:
>
> - adding the semicolons at the end of DECLARE_DMA_UNMAP_{ADDR|LEN}
> confuses people.

Hm. I'm (still) confused. Why does this patch add semi-colons at the end
of DECLARE_PCI_UNMAP_{ADDR|LEN} below then? (other than it is spelled with
letters "PCI" instead of "DMA")

Thanks.


--
~Randy

FUJITA Tomonori

unread,
Mar 3, 2010, 6:30:02 PM3/3/10
to
On Wed, 03 Mar 2010 15:16:05 -0800
Randy Dunlap <rdu...@xenotime.net> wrote:

> On 03/02/10 20:50, FUJITA Tomonori wrote:
> > Seems that you missed the following patch?
> >
> > http://lkml.org/lkml/2010/2/13/2
> >
> > This can be fold into dma-mappingh-add-the-dma_unmap-state-api.patch.
> >
> > Thanks,
> >
> > =
> > From: FUJITA Tomonori <fujita....@lab.ntt.co.jp>
> > Subject: [PATCH -mm] replace DECLARE_DMA_UNMAP_ADD_{ADDR,LEN} with DEFINE_DMA_UNMAP_ADD_{ADDR,LEN}
> >
> > Andrew pointed out:
> >
> > - adding the semicolons at the end of DECLARE_DMA_UNMAP_{ADDR|LEN}
> > confuses people.
>
> Hm. I'm (still) confused. Why does this patch add semi-colons at the end
> of DECLARE_PCI_UNMAP_{ADDR|LEN} below then? (other than it is spelled with
> letters "PCI" instead of "DMA")

Are you taking about this part, right?

+#define DECLARE_PCI_UNMAP_ADDR (ADDR_NAME) DEFINE_DMA_UNMAP_ADDR(ADDR_NAME);
+#define DECLARE_PCI_UNMAP_LEN(LEN_NAME) DEFINE_DMA_UNMAP_LEN(LEN_NAME);

Currently, we have:

#define DECLARE_PCI_UNMAP_ADDR(ADDR_NAME) dma_addr_t ADDR_NAME;

And PCI-DMA-mapping.txt says:

struct ring_state {
struct sk_buff *skb;

DECLARE_PCI_UNMAP_ADDR(mapping)
DECLARE_PCI_UNMAP_LEN(len)
};

NOTE: DO NOT put a semicolon at the end of the DECLARE_*()
macro.

However, this confuses people. Some drivers already use this macro
with a semicolon like, DECLARE_PCI_UNMAP_ADDR(mapping);

To avoid the above confusion, the new DEFINE_DMA_ macros doesn't have
a semicolon (users need to use a semicolon):

#define DEFINE_DMA_UNMAP_ADDR(ADDR_NAME) dma_addr_t ADDR_NAME
#define DEFINE_DMA_UNMAP_LEN(LEN_NAME) __u32 LEN_NAME

So we need to define DECLARE_PCI_* macro in the following way
(otherwise we break the existing drivers):

#define DECLARE_PCI_UNMAP_ADDR (ADDR_NAME) DEFINE_DMA_UNMAP_ADDR(ADDR_NAME);
#define DECLARE_PCI_UNMAP_LEN(LEN_NAME) DEFINE_DMA_UNMAP_LEN(LEN_NAME);


In the long term, I'll remove DECLARE_PCI_* macros.

Randy Dunlap

unread,
Mar 3, 2010, 6:40:01 PM3/3/10
to
On 03/03/10 15:28, FUJITA Tomonori wrote:
> On Wed, 03 Mar 2010 15:16:05 -0800
> Randy Dunlap <rdu...@xenotime.net> wrote:
>
>> On 03/02/10 20:50, FUJITA Tomonori wrote:
>>> Seems that you missed the following patch?
>>>
>>> http://lkml.org/lkml/2010/2/13/2
>>>
>>> This can be fold into dma-mappingh-add-the-dma_unmap-state-api.patch.
>>>
>>> Thanks,
>>>
>>> =
>>> From: FUJITA Tomonori <fujita....@lab.ntt.co.jp>
>>> Subject: [PATCH -mm] replace DECLARE_DMA_UNMAP_ADD_{ADDR,LEN} with DEFINE_DMA_UNMAP_ADD_{ADDR,LEN}
>>>
>>> Andrew pointed out:
>>>
>>> - adding the semicolons at the end of DECLARE_DMA_UNMAP_{ADDR|LEN}
>>> confuses people.
>>
>> Hm. I'm (still) confused. Why does this patch add semi-colons at the end
>> of DECLARE_PCI_UNMAP_{ADDR|LEN} below then? (other than it is spelled with
>> letters "PCI" instead of "DMA")
>
> Are you taking about this part, right?

Yes.

> +#define DECLARE_PCI_UNMAP_ADDR (ADDR_NAME) DEFINE_DMA_UNMAP_ADDR(ADDR_NAME);
> +#define DECLARE_PCI_UNMAP_LEN(LEN_NAME) DEFINE_DMA_UNMAP_LEN(LEN_NAME);
>
> Currently, we have:
>
> #define DECLARE_PCI_UNMAP_ADDR(ADDR_NAME) dma_addr_t ADDR_NAME;
>
> And PCI-DMA-mapping.txt says:
>
> struct ring_state {
> struct sk_buff *skb;
> DECLARE_PCI_UNMAP_ADDR(mapping)
> DECLARE_PCI_UNMAP_LEN(len)
> };
>
> NOTE: DO NOT put a semicolon at the end of the DECLARE_*()
> macro.

Argh. That is just wrong.
Thanks for cleaning it all up.


> However, this confuses people. Some drivers already use this macro
> with a semicolon like, DECLARE_PCI_UNMAP_ADDR(mapping);
>
> To avoid the above confusion, the new DEFINE_DMA_ macros doesn't have
> a semicolon (users need to use a semicolon):
>
> #define DEFINE_DMA_UNMAP_ADDR(ADDR_NAME) dma_addr_t ADDR_NAME
> #define DEFINE_DMA_UNMAP_LEN(LEN_NAME) __u32 LEN_NAME
>
> So we need to define DECLARE_PCI_* macro in the following way
> (otherwise we break the existing drivers):
>
> #define DECLARE_PCI_UNMAP_ADDR (ADDR_NAME) DEFINE_DMA_UNMAP_ADDR(ADDR_NAME);
> #define DECLARE_PCI_UNMAP_LEN(LEN_NAME) DEFINE_DMA_UNMAP_LEN(LEN_NAME);
>
>
> In the long term, I'll remove DECLARE_PCI_* macros.

I see. That's good, although I don't quite see the problem with doing
it all at once.

0 new messages