[rt2x00-users] [PATCH 4/9] rt2x00: Remove rt2800 version constants.

[Gertjan van Wingerde]

On 04/09/10 09:00, Julian Calaby wrote:
> On Fri, Apr 9, 2010 at 16:54, Ivo Van Doorn <ivdoorn at gmail.com> wrote:
>> On Fri, Apr 9, 2010 at 1:53 AM, Julian Calaby <julian.calaby at gmail.com> wrote:
>>> On Fri, Apr 9, 2010 at 08:32, Ivo van Doorn <ivdoorn at gmail.com> wrote:
>>>> On Thursday 08 April 2010, Gertjan van Wingerde wrote:
>>>>> The rt2800 version constants are inconsistent, and the version number don't
>>>>> mean a lot of things anyway. Use the literal values in the code instead of
>>>>> some sort of fabricated version name macro.
>>>>>
>>>>> Signed-off-by: Gertjan van Wingerde <gwingerde at gmail.com>
>>>>
>>>> Perhaps a more elegant way of using and defining needs to be found.
>>>> But at least the defined show what the purpose for the values is
>>>> rather then having magical values spread around the code.
>>>
>>> Maybe something like:
>>>
>>> #define RTDEV_IS_RT2883_R1(dev) (rt2x00_rt(dev, RT2883) && \
>>>                                                   rt2x00_rev(dev) < 0x0300)
>>
>> I considered this as well, but we have many checks which either do
>>    rt2x00_rev() < 0xffff
>> but also
>>    rt2x00_ref() == 0xffff
> 
> I assume that there are certain ranges of revisions that correspond to
> certain chips with certain ... features. So, you could have:
> 
> #define RTDEV_IS_RT2883_R1(dev) (rt2x00_rt(dev, RT2883) && \
>                                                   rt2x00_rev(dev) < 0x0300)
> 
> for the original chip, then
> 
> #define RTDEV_IS_RT2883_R2(dev) (rt2x00_rt(dev, RT2883) && \
>                                                   rt2x00_rev(dev) >= 0x0300 && \
>                                                   rt2x00_rev(dev) < 0x1000)
> 
> for the troubled second version and
> 
> #define RTDEV_IS_RT2883_R3(dev) (rt2x00_rt(dev, RT2883) && \
>                                                   rt2x00_rev(dev) >= 0x1000)
> 
> as a catch all for newer chips.

OK. After spending the entire morning trying to understand the Ralink numbering scheme,
I think I have come up with some sensible constant naming scheme and a set of helpers
to clarify the code. As you can see below there is no constant numbering scheme for all
chipsets, and the revision codes are chipset specific.

First of all, we would define the following chipset revision constants:

#define REV_RT2860C	0x0100
#define REV_RT2860D	0x0101
#define REV_RT2870D	0x0101
#define REV_RT2872E	0x0200
#define REV_RT3070E	0x0200
#define REV_RT3070F	0x0201
#define REV_RT3071E	0x0211
#define REV_RT3090E	0x0211
#define REV_RT3390E	0x0211

Next, we would create three helper functions:

bool rt2x00_rt_rev(struct rt2x00_dev *rt2x00dev, u16 rt, u16 rev);
bool rt2x00_less_than_rt_rev(struct rt2x00_dev *rt2x00dev, u16 rt, u16 rev);
bool rt2x00_at_least_rt_rev(struct rt2x00_dev *rt2x00dev, u16 rt, u16 rev);

This would cover all the usages that we currently have inside the code.

Obviously more can be added once needed.

Are you guys OK with this proposal?
If yes, I'll update the series to change the version constants removal
patch into a patch converting to the above scheme.

---
Gertjan.