Re: [PATCH net-next v3 01/10] net: dsa: mv88e6xxx: add mv88e6250_g1_ieee_pri_map

[Rasmus Villemoes]

On 03/06/2019 17.37, Vivien Didelot wrote:
> Hi Rasmus,
> 
> On Mon, 3 Jun 2019 14:42:12 +0000, Rasmus Villemoes <rasmus.villemoes@xxxxxxxxx> wrote:
>> Quite a few of the existing supported chips that use
>> mv88e6085_g1_ieee_pri_map as ->ieee_pri_map (including, incidentally,
>> mv88e6085 itself) actually have a reset value of 0xfa50 in the
>> G1_IEEE_PRI register.
>>
>> The data sheet for the mv88e6095, however, does describe a reset value
>> of 0xfa41.
>>
>> So rather than changing the value in the existing callback, introduce
>> a new variant with the 0xfa50 value. That will be used by the upcoming
>> mv88e6250, and existing chips can be switched over one by one,
>> preferably double-checking both the data sheet and actual hardware in
>> each case - if anybody actually feels this is important enough to
>> care.
> 
> Given your previous thread on this topic, I'd prefer that you include
> a first patch which implements mv88e6095_g1_ieee_pri_map() using 0xfa41
> and update mv88e{6092,6095}_ops to use it, then a second one which fixes
> mv88e6085_g1_ieee_pri_map to use 0xfa50. Then mv88e6250_ops can use it.

Well, the thing is, that would of course fix the value for 6240 and
6085, keeping the right value for 6092 and 6095, but I'd also be
changing the value for a whole lot of other chips for which I don't know
which one would be the right one.

Originally I thought that 0xfa50 was the right value for all chips
(simply because x -> floor(x/2) is a sane default mapping), but since I
now know that 0xfa41 (i.e., 3 and 0 are mapped to 1, 2 and 1 are mapped
to 0) is the reset value for at least some chips, I'd rather not make a
blanket change that might fix some and "break" other chips.

Rasmus