Re: [PATCH 5/5] Net: ath5k, kconfig changes
2007/8/31, Nick Kossifidis <[EMAIL PROTECTED]>: > 2007/8/30, John W. Linville <[EMAIL PROTECTED]>: > > On Thu, Aug 30, 2007 at 04:38:09AM +0300, Nick Kossifidis wrote: > > > 2007/8/28, Christoph Hellwig <[EMAIL PROTECTED]>: > > > > > > Also this whole patch seems rather pointless. It saves only > > > > very little and turns the driver into a complete ifdef maze. > > > > > Also most > > > people will use 5212 code only, 5211 cards are on some old laptops and > > > 5210, well i couldn't even find a 5210 for actual testing :P > > > > FWIW, I'd bet dollars to donuts that distros will enable them all > > together. > > > > Is saving code space the only reason to turn these off? How much > > space do you save? > > > > Is there some way you can isolate and/or limit the number of ifdef > > blocks further? If so, we might consider a version of this patch > > that depends on EMBEDDED or somesuch...? > > > > John > > O.K. as a first step i'll limit 5210 code only then, just an option > like "support older 5210 chipsets" which is going to be off by default > instead of 3 options. It's not just saving space, it's also saving > some runtime checks. It's not really a gain in performance though, > most checks are done during initialization and dfs setup, i just > thought it would be usefull to save as much cpu as possible. > Well after some thought i removed them all, there is no real gain from this in most cases (that ppl will use newer 5212 chips and combatibles). -- GPG ID: 0xD21DB2DB As you read this post global entropy rises. Have Fun ;-) Nick - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] Net: ath5k, kconfig changes
On Fri, 2007-08-31 at 09:30 -0400, Jeff Garzik wrote: > Dan Williams wrote: > > On Thu, 2007-08-30 at 08:36 -0400, John W. Linville wrote: > >> On Thu, Aug 30, 2007 at 04:38:09AM +0300, Nick Kossifidis wrote: > >>> 2007/8/28, Christoph Hellwig <[EMAIL PROTECTED]>: > Also this whole patch seems rather pointless. It saves only > very little and turns the driver into a complete ifdef maze. > >>> Also most > >>> people will use 5212 code only, 5211 cards are on some old laptops and > >>> 5210, well i couldn't even find a 5210 for actual testing :P > >> FWIW, I'd bet dollars to donuts that distros will enable them all > >> together. > > > > I would certainly _hope_ that distros enable everything -that is in the > > kernel- that they can get their hands on, otherwise when you stick a > > card in, it doesn't just work. > > Distros definitely -do not- do this. Plenty of ancient ISA drivers are > disabled at build time, for example, in many distros. Ok, so let me qualify to "within reason". All 802.11-compliant wireless cards would fall within the "within reason" IMHO, but, for example, older non 802.11 wireless cards (early Aironet products for example) probably don't. ISA clearly does not for mainstream distros. Dan - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] Net: ath5k, kconfig changes
Dan Williams wrote: On Thu, 2007-08-30 at 08:36 -0400, John W. Linville wrote: On Thu, Aug 30, 2007 at 04:38:09AM +0300, Nick Kossifidis wrote: 2007/8/28, Christoph Hellwig <[EMAIL PROTECTED]>: Also this whole patch seems rather pointless. It saves only very little and turns the driver into a complete ifdef maze. Also most people will use 5212 code only, 5211 cards are on some old laptops and 5210, well i couldn't even find a 5210 for actual testing :P FWIW, I'd bet dollars to donuts that distros will enable them all together. I would certainly _hope_ that distros enable everything -that is in the kernel- that they can get their hands on, otherwise when you stick a card in, it doesn't just work. Distros definitely -do not- do this. Plenty of ancient ISA drivers are disabled at build time, for example, in many distros. Jeff - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] Net: ath5k, kconfig changes
On Thu, 2007-08-30 at 08:36 -0400, John W. Linville wrote: > On Thu, Aug 30, 2007 at 04:38:09AM +0300, Nick Kossifidis wrote: > > 2007/8/28, Christoph Hellwig <[EMAIL PROTECTED]>: > > > > Also this whole patch seems rather pointless. It saves only > > > very little and turns the driver into a complete ifdef maze. > > > Also most > > people will use 5212 code only, 5211 cards are on some old laptops and > > 5210, well i couldn't even find a 5210 for actual testing :P > > FWIW, I'd bet dollars to donuts that distros will enable them all > together. I would certainly _hope_ that distros enable everything -that is in the kernel- that they can get their hands on, otherwise when you stick a card in, it doesn't just work. Dan > Is saving code space the only reason to turn these off? How much > space do you save? > > Is there some way you can isolate and/or limit the number of ifdef > blocks further? If so, we might consider a version of this patch > that depends on EMBEDDED or somesuch...? > > John - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] Net: ath5k, kconfig changes
2007/8/30, John W. Linville <[EMAIL PROTECTED]>: > On Thu, Aug 30, 2007 at 04:38:09AM +0300, Nick Kossifidis wrote: > > 2007/8/28, Christoph Hellwig <[EMAIL PROTECTED]>: > > > > Also this whole patch seems rather pointless. It saves only > > > very little and turns the driver into a complete ifdef maze. > > > Also most > > people will use 5212 code only, 5211 cards are on some old laptops and > > 5210, well i couldn't even find a 5210 for actual testing :P > > FWIW, I'd bet dollars to donuts that distros will enable them all > together. > > Is saving code space the only reason to turn these off? How much > space do you save? > > Is there some way you can isolate and/or limit the number of ifdef > blocks further? If so, we might consider a version of this patch > that depends on EMBEDDED or somesuch...? > > John O.K. as a first step i'll limit 5210 code only then, just an option like "support older 5210 chipsets" which is going to be off by default instead of 3 options. It's not just saving space, it's also saving some runtime checks. It's not really a gain in performance though, most checks are done during initialization and dfs setup, i just thought it would be usefull to save as much cpu as possible. -- GPG ID: 0xD21DB2DB As you read this post global entropy rises. Have Fun ;-) Nick - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] Net: ath5k, kconfig changes
On Thu, Aug 30, 2007 at 04:38:09AM +0300, Nick Kossifidis wrote: > 2007/8/28, Christoph Hellwig <[EMAIL PROTECTED]>: > > Also this whole patch seems rather pointless. It saves only > > very little and turns the driver into a complete ifdef maze. > Also most > people will use 5212 code only, 5211 cards are on some old laptops and > 5210, well i couldn't even find a 5210 for actual testing :P FWIW, I'd bet dollars to donuts that distros will enable them all together. Is saving code space the only reason to turn these off? How much space do you save? Is there some way you can isolate and/or limit the number of ifdef blocks further? If so, we might consider a version of this patch that depends on EMBEDDED or somesuch...? John -- John W. Linville [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] Net: ath5k, kconfig changes
On Thu, Aug 30, 2007 at 04:38:09AM +0300, Nick Kossifidis wrote: > It saves big chunks of code (not only initial register settings > arrays) and we'll extend it's use more inside ath5k_hw.c Trust me this > is a very useful step, eg. check out descriptor processing / setup or > PHY functions (calibrate/channel set etc). For example AR5210 mac chip > only comes with RF5110 phy chip so we can get rid of RF5111/RF5112 > code, AR5211 comes with RF5111 so we can get rid of RF5110 and RF5112 > code and AR5212 comes with RF5111 or RF5112 so we get rid of RF5110. > This thing also saves lots of checks during runtime (some of them > happen verry frequently, eg. durring descriptor processing). Also most > people will use 5212 code only, 5211 cards are on some old laptops and > 5210, well i couldn't even find a 5210 for actual testing :P If you're doing these checks in a hotpath something is badly wrong with your architecture. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] Net: ath5k, kconfig changes
2007/8/28, Christoph Hellwig <[EMAIL PROTECTED]>: > On Tue, Aug 28, 2007 at 12:01:30PM -0400, Jiri Slaby wrote: > > +config ATH5K_AR5210 > > + bool "Support AR5210" > > + depends on ATH5K > > + default y > > + > > +config ATH5K_AR5211 > > + bool "Support AR5211" > > + depends on ATH5K > > + default y > > + > > +config ATH5K_AR5212 > > + bool "Support AR5212" > > + depends on ATH5K > > + default y > > Please don't add more default statements. > > Also this whole patch seems rather pointless. It saves only > very little and turns the driver into a complete ifdef maze. It saves big chunks of code (not only initial register settings arrays) and we'll extend it's use more inside ath5k_hw.c Trust me this is a very useful step, eg. check out descriptor processing / setup or PHY functions (calibrate/channel set etc). For example AR5210 mac chip only comes with RF5110 phy chip so we can get rid of RF5111/RF5112 code, AR5211 comes with RF5111 so we can get rid of RF5110 and RF5112 code and AR5212 comes with RF5111 or RF5112 so we get rid of RF5110. This thing also saves lots of checks during runtime (some of them happen verry frequently, eg. durring descriptor processing). Also most people will use 5212 code only, 5211 cards are on some old laptops and 5210, well i couldn't even find a 5210 for actual testing :P -- GPG ID: 0xD21DB2DB As you read this post global entropy rises. Have Fun ;-) Nick - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] Net: ath5k, kconfig changes
On Tue, Aug 28, 2007 at 12:01:30PM -0400, Jiri Slaby wrote: > +config ATH5K_AR5210 > + bool "Support AR5210" > + depends on ATH5K > + default y > + > +config ATH5K_AR5211 > + bool "Support AR5211" > + depends on ATH5K > + default y > + > +config ATH5K_AR5212 > + bool "Support AR5212" > + depends on ATH5K > + default y Please don't add more default statements. Also this whole patch seems rather pointless. It saves only very little and turns the driver into a complete ifdef maze. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
