On Thu, Sep 10, 2009 at 11:46 PM, Jiri Slaby <jirislaby@xxxxxxxxx> wrote:> On 09/11/2009 08:16 AM, Nick Kossifidis wrote:>>> static inline u32 ath5k_hw_reg_read(struct ath5k_hw *ah, u16 reg)>>> {>>> - return ioread32(ah->ah_iobase + reg);>>> + return ath5k_hw_common(ah)->ops->read(ah, reg);>>> }>>>>>> -/*>>> - * Write to a register>>> - */>>> static inline void ath5k_hw_reg_write(struct ath5k_hw *ah, u32 val, u16 reg)>>> {>>> - iowrite32(val, ah->ah_iobase + reg);>>> + ath5k_hw_common(ah)->ops->write(ah, reg, val);> ...>> Since each driver will use it's own reg read/write functions (ath5k hw>> code still uses ath5k_hw_reg_read/write), why do we need to have>> common reg read/write functions like that used in the driver code ?>> This makes the code more complex that it needs to be and i don't see a>> reason why we need it. I understand why we need a way to handle>> read/write functions from common ath code but i don't see a reason to>> use these functions on ath5k, we can just fill ath_ops struct so that>> common code can use them and leave ath5k_hw_read/write untouched.>> I definitely agree with Nick here. Althought whole ath_ops will be hot> cache after the first operation, there is no need to prolong hot paths> by computing the op address and a call. Ok, read/write on PCI is pretty> slow, but still...
That is the way I had it originally before submission, and Icompletely agree its reasonable to not incur additional cost at theexpense of having two separate read/write paths, and perhaps we shouldonly incur the extra cost on routines shared betweenath9k/ath9k/ath9k_htc. But -- is there really is a measurable costpenalty?
This is why I asked if someone can test and give measurabledifferences over this. If there really isn't then that's not strongpoint against it.
For example, long ago I had argued over the cost incurred over theunnecessary branching on ioread()/iowrite() when you know you haveMMIO devices [1] -- the defense then, and IMHO reasonable now, wasthat the benefits of allowing cleaner drivers through the newinterfaces outweigh the theoretical penalties imposed by them.
Granted you can argue these new interfaces betweenath5k/ath9k/ath9k_htc would make things a little more complex, but Iwould expect sharing the code will help in the end. And if theseinterfaces are not acceptable I'm completely open to better suggestedalternatives.
[1] http://lkml.indiana.edu/hypermail/linux/kernel/0709.2/1068.html
Luis_______________________________________________devel mailing listdevel@xxxxxxxxxxxxxxxxxxxxxxxxxx://driverdev.linuxdriverproject.org/mailman/listinfo/devel
