Re: [net-next 10/13] igb: Tidy up wrapping for CONFIG_IGB_PTP.
From: Ben Hutchings <hidden>
Date: 2012-08-24 16:51:33
On Fri, 2012-08-24 at 12:10 +0200, Richard Cochran wrote:
On Thu, Aug 23, 2012 at 06:40:25PM +0000, Vick, Matthew wrote:quoted
I tend to agree with Jake here--I like having the information. I'm fine removing them, but I'd like to do it for all CONFIG_IGB_PTP wrapping if we're going to do it. What do you think, Richard?Come to think of it, I never liked the CONFIG_IGB_PTP very much in the first place. These were added after the fact by Jeff Kirsher. He had said off list that there was some issue with CONFIG_PTP_1588_CLOCK and igb as a module, or something like that. At that time I said, just go ahead and fix it up.
If there is some feature of a driver that depends on an optional modularisable subsystem, and you want the driver to be built without that feature when the subsystem is missing, then the feature has to be disabled when the driver is built-in and the subsystem is a module. So in general you need: config DRIVER_FEATURE bool "This is a really neat feature for the driver" depends on DRIVER && SUBSYSTEM && !(DRIVER=y && SUBYSTEM=m)
I think it would be better if the "time stamp all Rx packets" of the 82580 were always available, and that the PHC feature always be compiled when CONFIG_PTP_1588_CLOCK is selected. Maybe you could ask Jeff what the issue was, and then see if there is a way to remove CONFIG_IGB_PTP altogether.
Even if they are to be enabled automatically, CONFIG_IGB_PTP and similar config symbols are important as shorthand. So I think what you want is: config IGB_PTP def_bool y depends on IGB && PTP_1588_CLOCK && !(IGB=y && PTP_1588_CLOCK=m) (But currently it's actually *selecting* PTP_1588_CLOCK. We should probably have drivers consistently select or depend on it, not a mixture.) Ben. -- Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked.