From: Andrew Lunn <andrew@lunn.ch>
To: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
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 v3 2/8] net: dsa: mv88e6xxx: drop revision probing
Date: Sun, 17 Apr 2016 17:24:30 +0200 [thread overview]
Message-ID: <20160417152430.GA907@lunn.ch> (raw)
In-Reply-To: <1460846505-20305-3-git-send-email-vivien.didelot@savoirfairelinux.com>
On Sat, Apr 16, 2016 at 06:41:39PM -0400, Vivien Didelot wrote:
> There is no point in having a special case for the revision when probing
> a switch model. The code gets cluttered with unnecessary defines, and
> leads to errors when code such as mv88e6131_setup compares
> PORT_SWITCH_ID_6131_B2 to ps->id which masks the revision.
>
> Drop every revision definition, and lookup only the product number.
>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Thanks
Andrew
> ---
> drivers/net/dsa/mv88e6123.c | 6 ------
> drivers/net/dsa/mv88e6131.c | 2 --
> drivers/net/dsa/mv88e6352.c | 6 ------
> drivers/net/dsa/mv88e6xxx.c | 14 +-------------
> drivers/net/dsa/mv88e6xxx.h | 15 ---------------
> 5 files changed, 1 insertion(+), 42 deletions(-)
>
> diff --git a/drivers/net/dsa/mv88e6123.c b/drivers/net/dsa/mv88e6123.c
> index 88a812d..00c1121 100644
> --- a/drivers/net/dsa/mv88e6123.c
> +++ b/drivers/net/dsa/mv88e6123.c
> @@ -19,14 +19,8 @@
>
> static const struct mv88e6xxx_switch_id mv88e6123_table[] = {
> { PORT_SWITCH_ID_6123, "Marvell 88E6123" },
> - { PORT_SWITCH_ID_6123_A1, "Marvell 88E6123 (A1)" },
> - { PORT_SWITCH_ID_6123_A2, "Marvell 88E6123 (A2)" },
> { PORT_SWITCH_ID_6161, "Marvell 88E6161" },
> - { PORT_SWITCH_ID_6161_A1, "Marvell 88E6161 (A1)" },
> - { PORT_SWITCH_ID_6161_A2, "Marvell 88E6161 (A2)" },
> { PORT_SWITCH_ID_6165, "Marvell 88E6165" },
> - { PORT_SWITCH_ID_6165_A1, "Marvell 88E6165 (A1)" },
> - { PORT_SWITCH_ID_6165_A2, "Marvell 88e6165 (A2)" },
> };
>
> static char *mv88e6123_drv_probe(struct device *dsa_dev,
> diff --git a/drivers/net/dsa/mv88e6131.c b/drivers/net/dsa/mv88e6131.c
> index 6b2bcb0..df534da 100644
> --- a/drivers/net/dsa/mv88e6131.c
> +++ b/drivers/net/dsa/mv88e6131.c
> @@ -21,7 +21,6 @@ static const struct mv88e6xxx_switch_id mv88e6131_table[] = {
> { PORT_SWITCH_ID_6085, "Marvell 88E6085" },
> { PORT_SWITCH_ID_6095, "Marvell 88E6095/88E6095F" },
> { PORT_SWITCH_ID_6131, "Marvell 88E6131" },
> - { PORT_SWITCH_ID_6131_B2, "Marvell 88E6131 (B2)" },
> { PORT_SWITCH_ID_6185, "Marvell 88E6185" },
> };
>
> @@ -109,7 +108,6 @@ static int mv88e6131_setup(struct dsa_switch *ds)
> ps->num_ports = 11;
> break;
> case PORT_SWITCH_ID_6131:
> - case PORT_SWITCH_ID_6131_B2:
> ps->num_ports = 8;
> break;
> default:
> diff --git a/drivers/net/dsa/mv88e6352.c b/drivers/net/dsa/mv88e6352.c
> index dbd920e..30fc5f6 100644
> --- a/drivers/net/dsa/mv88e6352.c
> +++ b/drivers/net/dsa/mv88e6352.c
> @@ -27,14 +27,8 @@ static const struct mv88e6xxx_switch_id mv88e6352_table[] = {
> { PORT_SWITCH_ID_6176, "Marvell 88E6176" },
> { PORT_SWITCH_ID_6240, "Marvell 88E6240" },
> { PORT_SWITCH_ID_6320, "Marvell 88E6320" },
> - { PORT_SWITCH_ID_6320_A1, "Marvell 88E6320 (A1)" },
> - { PORT_SWITCH_ID_6320_A2, "Marvell 88e6320 (A2)" },
> { PORT_SWITCH_ID_6321, "Marvell 88E6321" },
> - { PORT_SWITCH_ID_6321_A1, "Marvell 88E6321 (A1)" },
> - { PORT_SWITCH_ID_6321_A2, "Marvell 88e6321 (A2)" },
> { PORT_SWITCH_ID_6352, "Marvell 88E6352" },
> - { PORT_SWITCH_ID_6352_A0, "Marvell 88E6352 (A0)" },
> - { PORT_SWITCH_ID_6352_A1, "Marvell 88E6352 (A1)" },
> };
>
> static char *mv88e6352_drv_probe(struct device *dsa_dev,
> diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
> index 9985a0c..b63e985 100644
> --- a/drivers/net/dsa/mv88e6xxx.c
> +++ b/drivers/net/dsa/mv88e6xxx.c
> @@ -3081,22 +3081,10 @@ static char *mv88e6xxx_lookup_name(struct mii_bus *bus, int sw_addr,
> if (ret < 0)
> return NULL;
>
> - /* Look up the exact switch ID */
> for (i = 0; i < num; ++i)
> - if (table[i].id == ret)
> + if (table[i].id == (ret & 0xfff0))
> return table[i].name;
>
> - /* Look up only the product number */
> - for (i = 0; i < num; ++i) {
> - if (table[i].id == (ret & PORT_SWITCH_ID_PROD_NUM_MASK)) {
> - dev_warn(&bus->dev,
> - "unknown revision %d, using base switch 0x%x\n",
> - ret & PORT_SWITCH_ID_REV_MASK,
> - ret & PORT_SWITCH_ID_PROD_NUM_MASK);
> - return table[i].name;
> - }
> - }
> -
> return NULL;
> }
>
> diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
> index 5d27dec..9242aea 100644
> --- a/drivers/net/dsa/mv88e6xxx.h
> +++ b/drivers/net/dsa/mv88e6xxx.h
> @@ -68,8 +68,6 @@
> #define PORT_PCS_CTRL_UNFORCED 0x03
> #define PORT_PAUSE_CTRL 0x02
> #define PORT_SWITCH_ID 0x03
> -#define PORT_SWITCH_ID_PROD_NUM_MASK 0xfff0
> -#define PORT_SWITCH_ID_REV_MASK 0x000f
> #define PORT_SWITCH_ID_6031 0x0310
> #define PORT_SWITCH_ID_6035 0x0350
> #define PORT_SWITCH_ID_6046 0x0480
> @@ -84,18 +82,11 @@
> #define PORT_SWITCH_ID_6121 0x1040
> #define PORT_SWITCH_ID_6122 0x1050
> #define PORT_SWITCH_ID_6123 0x1210
> -#define PORT_SWITCH_ID_6123_A1 0x1212
> -#define PORT_SWITCH_ID_6123_A2 0x1213
> #define PORT_SWITCH_ID_6131 0x1060
> -#define PORT_SWITCH_ID_6131_B2 0x1066
> #define PORT_SWITCH_ID_6152 0x1a40
> #define PORT_SWITCH_ID_6155 0x1a50
> #define PORT_SWITCH_ID_6161 0x1610
> -#define PORT_SWITCH_ID_6161_A1 0x1612
> -#define PORT_SWITCH_ID_6161_A2 0x1613
> #define PORT_SWITCH_ID_6165 0x1650
> -#define PORT_SWITCH_ID_6165_A1 0x1652
> -#define PORT_SWITCH_ID_6165_A2 0x1653
> #define PORT_SWITCH_ID_6171 0x1710
> #define PORT_SWITCH_ID_6172 0x1720
> #define PORT_SWITCH_ID_6175 0x1750
> @@ -104,16 +95,10 @@
> #define PORT_SWITCH_ID_6185 0x1a70
> #define PORT_SWITCH_ID_6240 0x2400
> #define PORT_SWITCH_ID_6320 0x1150
> -#define PORT_SWITCH_ID_6320_A1 0x1151
> -#define PORT_SWITCH_ID_6320_A2 0x1152
> #define PORT_SWITCH_ID_6321 0x3100
> -#define PORT_SWITCH_ID_6321_A1 0x3101
> -#define PORT_SWITCH_ID_6321_A2 0x3102
> #define PORT_SWITCH_ID_6350 0x3710
> #define PORT_SWITCH_ID_6351 0x3750
> #define PORT_SWITCH_ID_6352 0x3520
> -#define PORT_SWITCH_ID_6352_A0 0x3521
> -#define PORT_SWITCH_ID_6352_A1 0x3522
> #define PORT_CONTROL 0x04
> #define PORT_CONTROL_USE_CORE_TAG BIT(15)
> #define PORT_CONTROL_DROP_ON_LOCK BIT(14)
> --
> 2.8.0
>
next prev parent reply other threads:[~2016-04-17 15:24 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-16 22:41 [PATCH net-next v3 0/8] net: dsa: mv88e6xxx: factorize switch info Vivien Didelot
2016-04-16 22:41 ` [PATCH net-next v3 1/8] net: dsa: mv88e6xxx: drop double ds assignment Vivien Didelot
2016-04-16 22:41 ` [PATCH net-next v3 2/8] net: dsa: mv88e6xxx: drop revision probing Vivien Didelot
2016-04-17 15:24 ` Andrew Lunn [this message]
2016-04-16 22:41 ` [PATCH net-next v3 3/8] net: dsa: mv88e6xxx: read switch ID in probe Vivien Didelot
2016-04-17 15:26 ` Andrew Lunn
2016-04-16 22:41 ` [PATCH net-next v3 4/8] net: dsa: mv88e6xxx: add switch info Vivien Didelot
2016-04-17 15:35 ` Andrew Lunn
2016-04-17 16:37 ` Vivien Didelot
2016-04-16 22:41 ` [PATCH net-next v3 5/8] net: dsa: mv88e6xxx: add family to info Vivien Didelot
2016-04-16 22:41 ` [PATCH net-next v3 6/8] net: dsa: mv88e6xxx: add number of ports " Vivien Didelot
2016-04-16 22:41 ` [PATCH net-next v3 7/8] net: dsa: mv88e6xxx: add number of db " Vivien Didelot
2016-04-16 22:41 ` [PATCH net-next v3 8/8] net: dsa: mv88e6xxx: remove switch ID from ps Vivien Didelot
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=20160417152430.GA907@lunn.ch \
--to=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=f.fainelli@gmail.com \
--cc=kernel@savoirfairelinux.com \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=vivien.didelot@savoirfairelinux.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).