On Tuesday 09 February 2010, [email protected] wrote:
> This patchset adds include/asm-generic/pci-dma-common.h that defines
> the pci_unmap state API:
>
> DECLARE_PCI_UNMAP_ADDR(ADDR_NAME)
> DECLARE_PCI_UNMAP_LEN(LEN_NAME)
> pci_unmap_addr(PTR, ADDR_NAME)
> pci_unmap_addr_set(PTR, ADDR_NAME, VAL)
> pci_unmap_len(PTR, LEN_NAME)
> pci_unmap_len_set(PTR, LEN_NAME, VAL)
>
> This enables us to remove lots of the duplication in architecture
> implementations since there are only two ways to define the API.
Good stuff, moving this code to a common location is certainly
a step in the right direction.
A few general comments though:
* It seems you left out arch/cris for no good reason.
* The general tendency nowadays is to avoid new HAVE_ARCH_*
macros and instead using either a Kconfig symbol or
checking the presence of the macros themselves. In this case,
the macro would always be present, so CONFIG_PCI_UNMAP
would be reasonable.
* AFAICT, there are no architectures left doing the
non-generic version, so the code could be moved to
a global file in include/linux instead of include/asm-generic.
While include/linux/pci.h is already a bit overloaded, it's
probably the right place for this. Alternatively,
include/linux/pci-dma.h, included from include/linux/pci.h.
> In the long term, it's better to replace the API with the generic
> device model API such as DECLARE_DMA_UNMAP_ADDR. We can map the API to
> the generic one (like dma-mapping-compat.h does). This patch also
> makes the migration process easier.
Agreed.
Arnd
On Tue, 9 Feb 2010 16:23:35 +0100
Arnd Bergmann <[email protected]> wrote:
> On Tuesday 09 February 2010, [email protected] wrote:
> > This patchset adds include/asm-generic/pci-dma-common.h that defines
> > the pci_unmap state API:
> >
> > DECLARE_PCI_UNMAP_ADDR(ADDR_NAME)
> > DECLARE_PCI_UNMAP_LEN(LEN_NAME)
> > pci_unmap_addr(PTR, ADDR_NAME)
> > pci_unmap_addr_set(PTR, ADDR_NAME, VAL)
> > pci_unmap_len(PTR, LEN_NAME)
> > pci_unmap_len_set(PTR, LEN_NAME, VAL)
> >
> > This enables us to remove lots of the duplication in architecture
> > implementations since there are only two ways to define the API.
>
> Good stuff, moving this code to a common location is certainly
> a step in the right direction.
>
> A few general comments though:
>
> * It seems you left out arch/cris for no good reason.
Yeah, somehow I missed it. I'll do next time.
> * The general tendency nowadays is to avoid new HAVE_ARCH_*
> macros and instead using either a Kconfig symbol or
> checking the presence of the macros themselves. In this case,
> the macro would always be present, so CONFIG_PCI_UNMAP
> would be reasonable.
I see. I go with CONFIG_NEED_DMA_MAP_STATE (it's a bit explanatory and
mips already uses CONFIG_DMA_NEED_PCI_MAP_STATE for the same purpose).
> * AFAICT, there are no architectures left doing the
> non-generic version, so the code could be moved to
> a global file in include/linux instead of include/asm-generic.
> While include/linux/pci.h is already a bit overloaded, it's
> probably the right place for this. Alternatively,
> include/linux/pci-dma.h, included from include/linux/pci.h.
Ok, I go with linux/pci-dma.h. I prefer linux/pci.h but as you said,
looks like already too big. We'll remove this file after moving to the
generic device model. So the name shouldn't matter much.
Thanks!