Re: [PATCH 3/4] ath5k: define ath_common ops

[Jiri Slaby <jirislaby@gmail.com>]

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...