From: Vivien Didelot <vivien.didelot@savoirfairelinux•com>
To: Andrew Lunn <andrew@lunn•ch>
Cc: netdev@vger•kernel.org, linux-kernel@vger•kernel.org,
kernel@savoirfairelinux•com,
"David S. Miller" <davem@davemloft•net>,
Florian Fainelli <f.fainelli@gmail•com>
Subject: Re: [PATCH net-next 7/7] net: dsa: mv88e6xxx: drop switch id
Date: Fri, 15 Apr 2016 17:00:50 -0400 [thread overview]
Message-ID: <87k2jya8ot.fsf@ketchup.mtl.sfl> (raw)
In-Reply-To: <20160415193818.GE18523@lunn.ch>
Hi Andrew,
Andrew Lunn <andrew@lunn•ch> writes:
<snip>
>> -#define PORT_SWITCH_ID_6350 0x3710
>> -#define PORT_SWITCH_ID_6351 0x3750
>> -#define PORT_SWITCH_ID_6352 0x3520
>
> NACK
>
> These numbers are not obvious. PORT_SWITCH_ID_6320 i can
> understand. 0x1150 i have no idea what it is.
0x1150 is not even correct. That's the product number (bits 4:15) masked
with an assumed revision 0 (bits 0:3).
That leads to confusion and error, as seen in the patch 2/7.
These values are now only used in a device description table, where they
seem pretty understandable to me.
This header file is full of inconsistencies. We have masks, offsets,
shifts, shifted and unshifted values, just for the sake of hidding said
magic numbers, while an explicit comment in a function could do the job.
But OK if we really want them defined, I'll introduce 12-bit
PORT_SWITCH_ID_PROD_NUM_* before dropping the 16-bit PORT_SWITCH_ID_*.
Thanks,
Vivien
next prev parent reply other threads:[~2016-04-15 21:00 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-15 18:25 [PATCH net-next 0/7] net: dsa: mv88e6xxx: factorize switch info Vivien Didelot
2016-04-15 18:25 ` [PATCH net-next 1/7] net: dsa: mv88e6xxx: drop double ds assignment Vivien Didelot
2016-04-15 18:25 ` [PATCH net-next 2/7] net: dsa: mv88e6xxx: drop revision probing Vivien Didelot
2016-04-15 19:16 ` Andrew Lunn
2016-04-15 18:25 ` [PATCH net-next 3/7] net: dsa: mv88e6xxx: add switch info Vivien Didelot
2016-04-15 19:13 ` Andrew Lunn
2016-04-15 18:25 ` [PATCH net-next 4/7] net: dsa: mv88e6xxx: add family to info Vivien Didelot
2016-04-15 19:11 ` Andrew Lunn
[not found] ` <notmuch-sha1-5b8efc36765bacf253ff62234056042d5c4c5c37>
2016-04-15 21:06 ` Vivien Didelot
2016-04-15 18:25 ` [PATCH net-next 5/7] net: dsa: mv88e6xxx: add number of ports " Vivien Didelot
2016-04-15 18:25 ` [PATCH net-next 6/7] net: dsa: mv88e6xxx: add number of database " Vivien Didelot
2016-04-15 19:35 ` Andrew Lunn
2016-04-15 20:24 ` Vivien Didelot
2016-04-15 20:27 ` Andrew Lunn
2016-04-15 18:25 ` [PATCH net-next 7/7] net: dsa: mv88e6xxx: drop switch id Vivien Didelot
2016-04-15 19:38 ` Andrew Lunn
2016-04-15 21:00 ` Vivien Didelot [this message]
2016-04-15 21:51 ` Andrew Lunn
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87k2jya8ot.fsf@ketchup.mtl.sfl \
--to=vivien.didelot@savoirfairelinux$(echo .)com \
--cc=andrew@lunn$(echo .)ch \
--cc=davem@davemloft$(echo .)net \
--cc=f.fainelli@gmail$(echo .)com \
--cc=kernel@savoirfairelinux$(echo .)com \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox