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

Luis R. Rodriguez lrodriguez at atheros.com
Mon Sep 14 02:01:40 PDT 2009


On Mon, Sep 14, 2009 at 1:46 AM, Jiri Slaby <jirislaby at gmail.com> wrote:
> On 09/14/2009 10:32 AM, Luis R. Rodriguez wrote:
>> --- a/drivers/net/wireless/ath/ath5k/base.c
>> +++ b/drivers/net/wireless/ath/ath5k/base.c
>> @@ -437,6 +437,22 @@ ath5k_chip_name(enum ath5k_srev_type type, u_int16_t val)
>>
>>       return name;
>>  }
>> +static unsigned int ath5k_ioread32(void *hw_priv, u32 reg_offset)
>> +{
>> +     struct ath5k_hw *ah = (struct ath5k_hw *) hw_priv;
>> +     return ioread32(ah->ah_iobase + reg_offset);
>
> Why not just
> return ath5k_hw_reg_read(hw_priv, reg_offset)
> as I suggested?
>
>> +static void ath5k_iowrite32(void *hw_priv, u32 val, u32 reg_offset)
>> +{
>> +     struct ath5k_hw *ah = (struct ath5k_hw *) hw_priv;
>> +     iowrite32(val, ah->ah_iobase + reg_offset);
>
> and ath5k_hw_reg_write(hw_priv, val, reg_offset)
>
> If we ever decide to not use ioread32/iowrite32, it will need a change
> on a single place.

Alright, will respin, sorry meant to have done that.

>> +}
>> +
>> +const static struct ath_ops ath5k_common_ops = {
>
> C99, 6.11.5 Storage-class specifiers:
> 1 The placement of a storage-class specifier other than at the
> beginning of the declaration specifiers in a declaration is
> an obsolescent feature.
>
> I.e. use static const, please.

Sure thanks for the review.

  Luis


More information about the devel mailing list