Messages in this thread | | | Date | Wed, 30 May 2007 11:10:03 +0200 | From | Cornelia Huck <> | Subject | Re: [patch] Introduce CONFIG_HAS_DMA. |
| |
On Fri, 25 May 2007 09:36:31 -0700, "Dan Williams" <dan.j.williams@intel.com> wrote:
> I went back and read the thread leading up to this patch and I am of > the opinion that John's approach (adding more stubs to > dma-mapping-broken.h: > http://marc.info/?l=linux-kernel&m=117219377712232&w=2) is needed in > _addition_ to this Kconfig option. In my particular case I have an > API with a DMA and a non-DMA path written in such a way that the DMA > path can be compiled away. Without dma-mapping-broken.h support the > API is unnecessarily forced to violate point 2 of > Documentation/SubmittingPatches (#ifdefs are ugly).
IMO, well-placed #ifdefs are preferrable to dragging non-working code around. Like:
- put the DMA path in a file only build for MY_STUFF_USE_DMA - let MY_STUFF select MY_STUFF_USE_DMA if HAS_DMA - have a header file that points to the implementation for MY_STUFF_USE_DMA and uses well defined stubs for !MY_STUFF_USE_DMA, like returning ICannotDoThat for check_if_can_do() (kind of like what include/linux/sysfs.h does, for example)
This contains the #ifdefs in a header, doesn't compile stuff that won't work anyway on !HAS_DMA, and adds the ability to disable MY_STUFF_USE_DMA even if HAS_DMA at a later time if someone wants it.
> In other words let CONFIG_HAS_DMA prevent pure DMA code from being > built, but do not preclude "clever" implementations from calling > broken code.
If it calls broken code, it may not be so "clever" after all :)
What I don't like about this is
- compiles stuff that is not needed on !HAS_DMA - worse, compiles stuff that will not work on !HAS_DMA - does not encourage to split code properly into a DMA and a non-DMA part - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |