Messages in this thread | | | Date | Mon, 10 Jul 2006 20:32:36 +0800 | From | "Antonino A. Daplas" <> | Subject | Re: [PATCH v2] cirrus-logic-framebuffer-i2c-support.patch |
| |
Krzysztof Halasa wrote: > "Antonino A. Daplas" <adaplas@pol.net> writes: > >>> Feel free to add another patch, while I don't see a need I have nothing >>> against :-) >> No, you fix the patch. > > Look, I don't feel my patch needs such "fix". So if you think it does, > you have to do it.
Eventually, I'm the one who's going to maintain the code, most of the drivers in the video directory are practically abandoned. Additionally, this is mentioned in Documentation/CodingStyle.
2) #ifdefs are ugly
Code cluttered with ifdefs is difficult to read and maintain. Don't do it. Instead, put your ifdefs in a header, and conditionally define 'static inline' functions, or macros, which are used in the code. Let the compiler optimize away the "no-op" case.
Simple example, of poor code:
dev = alloc_etherdev (sizeof(struct funky_private)); if (!dev) return -ENODEV; #ifdef CONFIG_NET_FUNKINESS init_funky_net(dev); #endif
> >> And while your at it, check your Kconfig >> dependencies, ie check for impossible combinations such as CONFIG_I2C=m, >> CONFIG_FB_CIRRUS=y. > > You're right here, I don't know why I assumed DEPENDS does it > automatically.
Use select.
Tony - 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/
| |