On Sat, Apr 10, 2010 at 18:57, Gertjan van Wingerde <gwingerde at gmail.com> wrote:
> 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?
I have no real say, but for what it's worth, that seems like a good idea.
Thanks,
--
Julian Calaby
Email: julian.calaby at gmail.com
.Plan: http://sites.google.com/site/juliancalaby/