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

[Jiri Slaby <jirislaby@gmail.com>]

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.

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