* [PATCH net-next 3/5] net: dsa: mv88e6xxx: Allow the SERDES interfaces to have statistics
From: Andrew Lunn @ 2018-01-03 14:09 UTC (permalink / raw)
To: David Miller
Cc: Vivien Didelot, Florian Fainelli, netdev, Russell King,
Andrew Lunn
In-Reply-To: <1514988562-20079-1-git-send-email-andrew@lunn.ch>
When gettting the number of statistics, the strings and the actual
statistics, call the SERDES ops if implemented. This means the stats
code needs to return the number of strings/stats they have placed into
the data, so that the SERDES strings/stats can follow on.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Tested-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/mv88e6xxx/chip.c | 72 +++++++++++++++++++++++++++-------------
drivers/net/dsa/mv88e6xxx/chip.h | 13 ++++++--
2 files changed, 59 insertions(+), 26 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 12e274a3ff24..5274e8292451 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -627,8 +627,8 @@ static uint64_t _mv88e6xxx_get_ethtool_stat(struct mv88e6xxx_chip *chip,
return value;
}
-static void mv88e6xxx_stats_get_strings(struct mv88e6xxx_chip *chip,
- uint8_t *data, int types)
+static int mv88e6xxx_stats_get_strings(struct mv88e6xxx_chip *chip,
+ uint8_t *data, int types)
{
struct mv88e6xxx_hw_stat *stat;
int i, j;
@@ -641,31 +641,39 @@ static void mv88e6xxx_stats_get_strings(struct mv88e6xxx_chip *chip,
j++;
}
}
+
+ return j;
}
-static void mv88e6095_stats_get_strings(struct mv88e6xxx_chip *chip,
- uint8_t *data)
+static int mv88e6095_stats_get_strings(struct mv88e6xxx_chip *chip,
+ uint8_t *data)
{
- mv88e6xxx_stats_get_strings(chip, data,
- STATS_TYPE_BANK0 | STATS_TYPE_PORT);
+ return mv88e6xxx_stats_get_strings(chip, data,
+ STATS_TYPE_BANK0 | STATS_TYPE_PORT);
}
-static void mv88e6320_stats_get_strings(struct mv88e6xxx_chip *chip,
- uint8_t *data)
+static int mv88e6320_stats_get_strings(struct mv88e6xxx_chip *chip,
+ uint8_t *data)
{
- mv88e6xxx_stats_get_strings(chip, data,
- STATS_TYPE_BANK0 | STATS_TYPE_BANK1);
+ return mv88e6xxx_stats_get_strings(chip, data,
+ STATS_TYPE_BANK0 | STATS_TYPE_BANK1);
}
static void mv88e6xxx_get_strings(struct dsa_switch *ds, int port,
uint8_t *data)
{
struct mv88e6xxx_chip *chip = ds->priv;
+ int count = 0;
mutex_lock(&chip->reg_lock);
if (chip->info->ops->stats_get_strings)
- chip->info->ops->stats_get_strings(chip, data);
+ count = chip->info->ops->stats_get_strings(chip, data);
+
+ if (chip->info->ops->serdes_get_strings) {
+ data += count * ETH_GSTRING_LEN;
+ chip->info->ops->serdes_get_strings(chip, port, data);
+ }
mutex_unlock(&chip->reg_lock);
}
@@ -699,11 +707,21 @@ static int mv88e6320_stats_get_sset_count(struct mv88e6xxx_chip *chip)
static int _mv88e6xxx_get_sset_count(struct dsa_switch *ds, int port)
{
struct mv88e6xxx_chip *chip = ds->priv;
+ int serdes_count = 0;
+ int count = 0;
if (chip->info->ops->stats_get_sset_count)
- return chip->info->ops->stats_get_sset_count(chip);
+ count = chip->info->ops->stats_get_sset_count(chip);
+ if (count < 0)
+ return count;
- return 0;
+ if (chip->info->ops->serdes_get_sset_count)
+ serdes_count = chip->info->ops->serdes_get_sset_count(chip,
+ port);
+ if (serdes_count < 0)
+ return serdes_count;
+
+ return count + serdes_count;
}
static int mv88e6xxx_get_sset_count(struct dsa_switch *ds, int port)
@@ -719,9 +737,9 @@ static int mv88e6xxx_get_sset_count(struct dsa_switch *ds, int port)
}
-static void mv88e6xxx_stats_get_stats(struct mv88e6xxx_chip *chip, int port,
- uint64_t *data, int types,
- u16 bank1_select, u16 histogram)
+static int mv88e6xxx_stats_get_stats(struct mv88e6xxx_chip *chip, int port,
+ uint64_t *data, int types,
+ u16 bank1_select, u16 histogram)
{
struct mv88e6xxx_hw_stat *stat;
int i, j;
@@ -735,18 +753,19 @@ static void mv88e6xxx_stats_get_stats(struct mv88e6xxx_chip *chip, int port,
j++;
}
}
+ return j;
}
-static void mv88e6095_stats_get_stats(struct mv88e6xxx_chip *chip, int port,
- uint64_t *data)
+static int mv88e6095_stats_get_stats(struct mv88e6xxx_chip *chip, int port,
+ uint64_t *data)
{
return mv88e6xxx_stats_get_stats(chip, port, data,
STATS_TYPE_BANK0 | STATS_TYPE_PORT,
0, MV88E6XXX_G1_STATS_OP_HIST_RX_TX);
}
-static void mv88e6320_stats_get_stats(struct mv88e6xxx_chip *chip, int port,
- uint64_t *data)
+static int mv88e6320_stats_get_stats(struct mv88e6xxx_chip *chip, int port,
+ uint64_t *data)
{
return mv88e6xxx_stats_get_stats(chip, port, data,
STATS_TYPE_BANK0 | STATS_TYPE_BANK1,
@@ -754,8 +773,8 @@ static void mv88e6320_stats_get_stats(struct mv88e6xxx_chip *chip, int port,
MV88E6XXX_G1_STATS_OP_HIST_RX_TX);
}
-static void mv88e6390_stats_get_stats(struct mv88e6xxx_chip *chip, int port,
- uint64_t *data)
+static int mv88e6390_stats_get_stats(struct mv88e6xxx_chip *chip, int port,
+ uint64_t *data)
{
return mv88e6xxx_stats_get_stats(chip, port, data,
STATS_TYPE_BANK0 | STATS_TYPE_BANK1,
@@ -766,8 +785,15 @@ static void mv88e6390_stats_get_stats(struct mv88e6xxx_chip *chip, int port,
static void mv88e6xxx_get_stats(struct mv88e6xxx_chip *chip, int port,
uint64_t *data)
{
+ int count = 0;
+
if (chip->info->ops->stats_get_stats)
- chip->info->ops->stats_get_stats(chip, port, data);
+ count = chip->info->ops->stats_get_stats(chip, port, data);
+
+ if (chip->info->ops->serdes_get_stats) {
+ data += count;
+ chip->info->ops->serdes_get_stats(chip, port, data);
+ }
}
static void mv88e6xxx_get_ethtool_stats(struct dsa_switch *ds, int port,
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index 334f6f7544ba..1787fc43167d 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -325,9 +325,9 @@ struct mv88e6xxx_ops {
/* Return the number of strings describing statistics */
int (*stats_get_sset_count)(struct mv88e6xxx_chip *chip);
- void (*stats_get_strings)(struct mv88e6xxx_chip *chip, uint8_t *data);
- void (*stats_get_stats)(struct mv88e6xxx_chip *chip, int port,
- uint64_t *data);
+ int (*stats_get_strings)(struct mv88e6xxx_chip *chip, uint8_t *data);
+ int (*stats_get_stats)(struct mv88e6xxx_chip *chip, int port,
+ uint64_t *data);
int (*set_cpu_port)(struct mv88e6xxx_chip *chip, int port);
int (*set_egress_port)(struct mv88e6xxx_chip *chip, int port);
const struct mv88e6xxx_irq_ops *watchdog_ops;
@@ -337,6 +337,13 @@ struct mv88e6xxx_ops {
/* Power on/off a SERDES interface */
int (*serdes_power)(struct mv88e6xxx_chip *chip, int port, bool on);
+ /* Statistics from the SERDES interface */
+ int (*serdes_get_sset_count)(struct mv88e6xxx_chip *chip, int port);
+ void (*serdes_get_strings)(struct mv88e6xxx_chip *chip, int port,
+ uint8_t *data);
+ void (*serdes_get_stats)(struct mv88e6xxx_chip *chip, int port,
+ uint64_t *data);
+
/* VLAN Translation Unit operations */
int (*vtu_getnext)(struct mv88e6xxx_chip *chip,
struct mv88e6xxx_vtu_entry *entry);
--
2.15.1
^ permalink raw reply related
* [PATCH net-next 4/5] net: dsa: mv88e6xxx: Add helper to determining if port has SERDES
From: Andrew Lunn @ 2018-01-03 14:09 UTC (permalink / raw)
To: David Miller
Cc: Vivien Didelot, Florian Fainelli, netdev, Russell King,
Andrew Lunn
In-Reply-To: <1514988562-20079-1-git-send-email-andrew@lunn.ch>
Refactor the existing code. This helper will be used for SERDES
statistics.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Tested-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/mv88e6xxx/serdes.c | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c
index f3c01119b3d1..d32522276fea 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.c
+++ b/drivers/net/dsa/mv88e6xxx/serdes.c
@@ -55,18 +55,30 @@ static int mv88e6352_serdes_power_set(struct mv88e6xxx_chip *chip, bool on)
return err;
}
-int mv88e6352_serdes_power(struct mv88e6xxx_chip *chip, int port, bool on)
+static int mv88e6352_port_has_serdes(struct mv88e6xxx_chip *chip, int port)
{
- int err;
u8 cmode;
+ int err;
err = mv88e6xxx_port_get_cmode(chip, port, &cmode);
- if (err)
- return err;
+ if (err) {
+ dev_err(chip->dev, "failed to read cmode\n");
+ return 0;
+ }
if ((cmode == MV88E6XXX_PORT_STS_CMODE_100BASE_X) ||
(cmode == MV88E6XXX_PORT_STS_CMODE_1000BASE_X) ||
- (cmode == MV88E6XXX_PORT_STS_CMODE_SGMII)) {
+ (cmode == MV88E6XXX_PORT_STS_CMODE_SGMII))
+ return 1;
+
+ return 0;
+}
+
+int mv88e6352_serdes_power(struct mv88e6xxx_chip *chip, int port, bool on)
+{
+ int err;
+
+ if (mv88e6352_port_has_serdes(chip, port)) {
err = mv88e6352_serdes_power_set(chip, on);
if (err < 0)
return err;
--
2.15.1
^ permalink raw reply related
* [PATCH net-next 1/5] dsa: Pass the port to get_sset_count()
From: Andrew Lunn @ 2018-01-03 14:09 UTC (permalink / raw)
To: David Miller
Cc: Vivien Didelot, Florian Fainelli, netdev, Russell King,
Andrew Lunn
In-Reply-To: <1514988562-20079-1-git-send-email-andrew@lunn.ch>
By passing the port, we allow different ports to have different
statistics. This is useful since some ports have SERDES interfaces
with their own statistic counters.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Tested-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/b53/b53_common.c | 2 +-
drivers/net/dsa/b53/b53_priv.h | 2 +-
drivers/net/dsa/dsa_loop.c | 2 +-
drivers/net/dsa/lan9303-core.c | 2 +-
drivers/net/dsa/microchip/ksz_common.c | 2 +-
drivers/net/dsa/mt7530.c | 2 +-
drivers/net/dsa/mv88e6xxx/chip.c | 2 +-
drivers/net/dsa/qca8k.c | 2 +-
include/net/dsa.h | 2 +-
net/dsa/master.c | 4 ++--
net/dsa/slave.c | 2 +-
11 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 561b05089cb6..0c1d814573b3 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -852,7 +852,7 @@ void b53_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *data)
}
EXPORT_SYMBOL(b53_get_ethtool_stats);
-int b53_get_sset_count(struct dsa_switch *ds)
+int b53_get_sset_count(struct dsa_switch *ds, int port)
{
struct b53_device *dev = ds->priv;
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index d954cf36ecd8..1187ebd79287 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -288,7 +288,7 @@ void b53_imp_vlan_setup(struct dsa_switch *ds, int cpu_port);
int b53_configure_vlan(struct dsa_switch *ds);
void b53_get_strings(struct dsa_switch *ds, int port, uint8_t *data);
void b53_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *data);
-int b53_get_sset_count(struct dsa_switch *ds);
+int b53_get_sset_count(struct dsa_switch *ds, int port);
int b53_br_join(struct dsa_switch *ds, int port, struct net_device *bridge);
void b53_br_leave(struct dsa_switch *ds, int port, struct net_device *bridge);
void b53_br_set_stp_state(struct dsa_switch *ds, int port, u8 state);
diff --git a/drivers/net/dsa/dsa_loop.c b/drivers/net/dsa/dsa_loop.c
index 7aa84ee4e771..f77be9f85cb3 100644
--- a/drivers/net/dsa/dsa_loop.c
+++ b/drivers/net/dsa/dsa_loop.c
@@ -86,7 +86,7 @@ static int dsa_loop_setup(struct dsa_switch *ds)
return 0;
}
-static int dsa_loop_get_sset_count(struct dsa_switch *ds)
+static int dsa_loop_get_sset_count(struct dsa_switch *ds, int port)
{
return __DSA_LOOP_CNT_MAX;
}
diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index 944901f03f8b..ba46dd319b10 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -1011,7 +1011,7 @@ static void lan9303_get_ethtool_stats(struct dsa_switch *ds, int port,
}
}
-static int lan9303_get_sset_count(struct dsa_switch *ds)
+static int lan9303_get_sset_count(struct dsa_switch *ds, int port)
{
return ARRAY_SIZE(lan9303_mib);
}
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 663b0d5b982b..bcb3e6c734f2 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -439,7 +439,7 @@ static void ksz_disable_port(struct dsa_switch *ds, int port,
ksz_port_cfg(dev, port, REG_PORT_CTRL_0, PORT_MAC_LOOPBACK, true);
}
-static int ksz_sset_count(struct dsa_switch *ds)
+static int ksz_sset_count(struct dsa_switch *ds, int port)
{
return TOTAL_SWITCH_COUNTER_NUM;
}
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 8a0bb000d056..511ca134f13f 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -604,7 +604,7 @@ mt7530_get_ethtool_stats(struct dsa_switch *ds, int port,
}
static int
-mt7530_get_sset_count(struct dsa_switch *ds)
+mt7530_get_sset_count(struct dsa_switch *ds, int port)
{
return ARRAY_SIZE(mt7530_mib);
}
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index fc512c98f2f8..504407adc7aa 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -692,7 +692,7 @@ static int mv88e6320_stats_get_sset_count(struct mv88e6xxx_chip *chip)
STATS_TYPE_BANK1);
}
-static int mv88e6xxx_get_sset_count(struct dsa_switch *ds)
+static int mv88e6xxx_get_sset_count(struct dsa_switch *ds, int port)
{
struct mv88e6xxx_chip *chip = ds->priv;
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 9df22ebee822..600d5ad1fbde 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -631,7 +631,7 @@ qca8k_get_ethtool_stats(struct dsa_switch *ds, int port,
}
static int
-qca8k_get_sset_count(struct dsa_switch *ds)
+qca8k_get_sset_count(struct dsa_switch *ds, int port)
{
return ARRAY_SIZE(ar8327_mib);
}
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 6cb602dd970c..35433386c314 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -357,7 +357,7 @@ struct dsa_switch_ops {
void (*get_strings)(struct dsa_switch *ds, int port, uint8_t *data);
void (*get_ethtool_stats)(struct dsa_switch *ds,
int port, uint64_t *data);
- int (*get_sset_count)(struct dsa_switch *ds);
+ int (*get_sset_count)(struct dsa_switch *ds, int port);
/*
* ethtool Wake-on-LAN
diff --git a/net/dsa/master.c b/net/dsa/master.c
index 00589147f042..f20a9600318f 100644
--- a/net/dsa/master.c
+++ b/net/dsa/master.c
@@ -42,7 +42,7 @@ static int dsa_master_get_sset_count(struct net_device *dev, int sset)
count += ops->get_sset_count(dev, sset);
if (sset == ETH_SS_STATS && ds->ops->get_sset_count)
- count += ds->ops->get_sset_count(ds);
+ count += ds->ops->get_sset_count(ds, cpu_dp->index);
return count;
}
@@ -76,7 +76,7 @@ static void dsa_master_get_strings(struct net_device *dev, uint32_t stringset,
* constructed earlier
*/
ds->ops->get_strings(ds, port, ndata);
- count = ds->ops->get_sset_count(ds);
+ count = ds->ops->get_sset_count(ds, cpu_dp->index);
for (i = 0; i < count; i++) {
memmove(ndata + (i * len + sizeof(pfx)),
ndata + i * len, len - sizeof(pfx));
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index f52307296de4..0d07004d59d4 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -559,7 +559,7 @@ static int dsa_slave_get_sset_count(struct net_device *dev, int sset)
count = 4;
if (ds->ops->get_sset_count)
- count += ds->ops->get_sset_count(ds);
+ count += ds->ops->get_sset_count(ds, dp->index);
return count;
}
--
2.15.1
^ permalink raw reply related
* [PATCH net-next 5/5] net: dsa: mv88e6xxx: Get mv88e6352 SERDES statistics
From: Andrew Lunn @ 2018-01-03 14:09 UTC (permalink / raw)
To: David Miller
Cc: Vivien Didelot, Florian Fainelli, netdev, Russell King,
Andrew Lunn
In-Reply-To: <1514988562-20079-1-git-send-email-andrew@lunn.ch>
Add support for reading the SERDES statistics of the mv88e8352, using
the standard ethtool -S option. The SERDES interface can be mapped to
either port 4 or 5, so only return statistics on those ports, if the
SERDES interface is in use.
The counters are reset on read, so need to be accumulated. Add a per
port structure to hold the stats counters. The 6352 only has a single
SERDES interface and so only one port will using the newly added
array. However the 6390 family has as many SERDES interfaces as ports,
each with statistics counters. Also, PTP has a number of counters per
port which will also need accumulating.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Tested-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/mv88e6xxx/chip.c | 3 ++
drivers/net/dsa/mv88e6xxx/chip.h | 7 ++++
drivers/net/dsa/mv88e6xxx/serdes.c | 84 ++++++++++++++++++++++++++++++++++++++
drivers/net/dsa/mv88e6xxx/serdes.h | 6 ++-
4 files changed, 99 insertions(+), 1 deletion(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 5274e8292451..a335ef2f1087 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -3129,6 +3129,9 @@ static const struct mv88e6xxx_ops mv88e6352_ops = {
.vtu_getnext = mv88e6352_g1_vtu_getnext,
.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
.serdes_power = mv88e6352_serdes_power,
+ .serdes_get_sset_count = mv88e6352_serdes_get_sset_count,
+ .serdes_get_strings = mv88e6352_serdes_get_strings,
+ .serdes_get_stats = mv88e6352_serdes_get_stats,
};
static const struct mv88e6xxx_ops mv88e6390_ops = {
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index 1787fc43167d..c5accbb84eea 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -154,6 +154,10 @@ struct mv88e6xxx_irq {
unsigned int nirqs;
};
+struct mv88e6xxx_port {
+ u64 serdes_stats[2];
+};
+
struct mv88e6xxx_chip {
const struct mv88e6xxx_info *info;
@@ -207,6 +211,9 @@ struct mv88e6xxx_chip {
int irq;
int device_irq;
int watchdog_irq;
+
+ /* Array of port structures. */
+ struct mv88e6xxx_port ports[DSA_MAX_PORTS];
};
struct mv88e6xxx_bus_ops {
diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c
index d32522276fea..3b911b80da33 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.c
+++ b/drivers/net/dsa/mv88e6xxx/serdes.c
@@ -87,6 +87,90 @@ int mv88e6352_serdes_power(struct mv88e6xxx_chip *chip, int port, bool on)
return 0;
}
+struct mv88e6352_serdes_hw_stat {
+ char string[ETH_GSTRING_LEN];
+ int sizeof_stat;
+ int reg;
+};
+
+static struct mv88e6352_serdes_hw_stat mv88e6352_serdes_hw_stats[] = {
+ { "serdes_fibre_rx_error", 16, 21 },
+ { "serdes_PRBS_error", 32, 24 },
+};
+
+int mv88e6352_serdes_get_sset_count(struct mv88e6xxx_chip *chip, int port)
+{
+ if (mv88e6352_port_has_serdes(chip, port))
+ return ARRAY_SIZE(mv88e6352_serdes_hw_stats);
+
+ return 0;
+}
+
+void mv88e6352_serdes_get_strings(struct mv88e6xxx_chip *chip,
+ int port, uint8_t *data)
+{
+ struct mv88e6352_serdes_hw_stat *stat;
+ int i;
+
+ if (!mv88e6352_port_has_serdes(chip, port))
+ return;
+
+ for (i = 0; i < ARRAY_SIZE(mv88e6352_serdes_hw_stats); i++) {
+ stat = &mv88e6352_serdes_hw_stats[i];
+ memcpy(data + i * ETH_GSTRING_LEN, stat->string,
+ ETH_GSTRING_LEN);
+ }
+}
+
+static uint64_t mv88e6352_serdes_get_stat(struct mv88e6xxx_chip *chip,
+ struct mv88e6352_serdes_hw_stat *stat)
+{
+ u64 val = 0;
+ u16 reg;
+ int err;
+
+ err = mv88e6352_serdes_read(chip, stat->reg, ®);
+ if (err) {
+ dev_err(chip->dev, "failed to read statistic\n");
+ return 0;
+ }
+
+ val = reg;
+
+ if (stat->sizeof_stat == 32) {
+ err = mv88e6352_serdes_read(chip, stat->reg + 1, ®);
+ if (err) {
+ dev_err(chip->dev, "failed to read statistic\n");
+ return 0;
+ }
+ val = val << 16 | reg;
+ }
+
+ return val;
+}
+
+void mv88e6352_serdes_get_stats(struct mv88e6xxx_chip *chip, int port,
+ uint64_t *data)
+{
+ struct mv88e6xxx_port *mv88e6xxx_port = &chip->ports[port];
+ struct mv88e6352_serdes_hw_stat *stat;
+ u64 value;
+ int i;
+
+ if (!mv88e6352_port_has_serdes(chip, port))
+ return;
+
+ BUILD_BUG_ON(ARRAY_SIZE(mv88e6352_serdes_hw_stats) >
+ ARRAY_SIZE(mv88e6xxx_port->serdes_stats));
+
+ for (i = 0; i < ARRAY_SIZE(mv88e6352_serdes_hw_stats); i++) {
+ stat = &mv88e6352_serdes_hw_stats[i];
+ value = mv88e6352_serdes_get_stat(chip, stat);
+ mv88e6xxx_port->serdes_stats[i] += value;
+ data[i] = mv88e6xxx_port->serdes_stats[i];
+ }
+}
+
/* Set the power on/off for 10GBASE-R and 10GBASE-X4/X2 */
static int mv88e6390_serdes_10g(struct mv88e6xxx_chip *chip, int addr, bool on)
{
diff --git a/drivers/net/dsa/mv88e6xxx/serdes.h b/drivers/net/dsa/mv88e6xxx/serdes.h
index 5c1cd6d8e9a5..641baa75f910 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.h
+++ b/drivers/net/dsa/mv88e6xxx/serdes.h
@@ -44,5 +44,9 @@
int mv88e6352_serdes_power(struct mv88e6xxx_chip *chip, int port, bool on);
int mv88e6390_serdes_power(struct mv88e6xxx_chip *chip, int port, bool on);
-
+int mv88e6352_serdes_get_sset_count(struct mv88e6xxx_chip *chip, int port);
+void mv88e6352_serdes_get_strings(struct mv88e6xxx_chip *chip,
+ int port, uint8_t *data);
+void mv88e6352_serdes_get_stats(struct mv88e6xxx_chip *chip, int port,
+ uint64_t *data);
#endif
--
2.15.1
^ permalink raw reply related
* [PATCH net-next 0/5] Export SERDES stats via ethtool -S
From: Andrew Lunn @ 2018-01-03 14:09 UTC (permalink / raw)
To: David Miller
Cc: Vivien Didelot, Florian Fainelli, netdev, Russell King,
Andrew Lunn
The mv88e6352 family has a SERDES interface which can be used for
example to connect to SFF/SFP modules. This interface has a couple of
statistics counters. Add support for including these counters in the
output of ethtool -S.
Andrew Lunn (5):
dsa: Pass the port to get_sset_count()
net: dsa: mv88e6xxx: Hold mutex while doing stats operations
net: dsa: mv88e6xxx: Allow the SERDES interfaces to have statistics
net: dsa: mv88e6xxx: Add helper to determining if port has SERDES
net: dsa: mv88e6xxx: Get mv88e6352 SERDES statistics
drivers/net/dsa/b53/b53_common.c | 2 +-
drivers/net/dsa/b53/b53_priv.h | 2 +-
drivers/net/dsa/dsa_loop.c | 2 +-
drivers/net/dsa/lan9303-core.c | 2 +-
drivers/net/dsa/microchip/ksz_common.c | 2 +-
drivers/net/dsa/mt7530.c | 2 +-
drivers/net/dsa/mv88e6xxx/chip.c | 94 +++++++++++++++++++++--------
drivers/net/dsa/mv88e6xxx/chip.h | 20 ++++++-
drivers/net/dsa/mv88e6xxx/serdes.c | 106 +++++++++++++++++++++++++++++++--
drivers/net/dsa/mv88e6xxx/serdes.h | 6 +-
drivers/net/dsa/qca8k.c | 2 +-
include/net/dsa.h | 2 +-
net/dsa/master.c | 4 +-
net/dsa/slave.c | 2 +-
14 files changed, 204 insertions(+), 44 deletions(-)
--
2.15.1
^ permalink raw reply
* Re: [PATCH net-next 2/2] l2tp: add peer_offset parameter
From: Guillaume Nault @ 2018-01-03 14:16 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: James Chapman, David S. Miller, netdev, Hangbin Liu
In-Reply-To: <CAJ0CqmWhhzvi8HCKjf-xUjkszuNKgnuoD_MXmDNtUtWyEExdNQ@mail.gmail.com>
On Tue, Jan 02, 2018 at 08:28:03PM +0100, Lorenzo Bianconi wrote:
> Perhaps I am little bit polarized on UABI issue, but I was rethinking
> about it and maybe removing offset parameter would lead to an
> interoperability issue for device running L2TPv3 since offset
> parameter is there and it is not a nope.
> Please consider this setup:
> - 2 endpoint running L2TPv3, the first running net-next and the second
> running 4.14
> - both endpoint are configured using iproute2 in this way:
>
> - ip l2tp add tunnel local <ip0> remote <ip1> tunnel_id <id0>
> peer_tunnel_id <id1> udp_sport <p0> udp_dport <p1>
> - ip l2tp add tunnel local <ip1> remote <ip0> tunnel_id <id1>
> peer_tunnel_id <id0> udp_sport <p1> udp_dport <p0>
> - ip l2tp add session name l2tp0 tunnel_id <id0> session_id <s0>
> peer_session_id <s1> offset 8
> - ip l2tp add session name l2tp0 tunnel_id <id1> session_id <s1>
> peer_session_id <s0> offset 8
>
> Can we assume offset is never used for L2TPv3?
>
That's what I think. You're right worrying about ABI issues. And I
wouldn't dare proposing such a removal if I had doubts about breaking a
user setup.
Considering the lack of use cases and the absence of interoperability
of this feature, I hardly can imagine it being used.
But it's not only that: the feature has been buggy for years without
anyone noticing. And this bug wasn't difficult to spot (one just needs
to look at an L2TPv3 header in a network packet dump).
It's really the combination of these three issues (buggy, no use case
and not producing valid L2TPv3 frames) that makes me propose a removal.
^ permalink raw reply
* Re: [PATCH] ethernet: mlx4: Delete an error message for a failed memory allocation in five functions
From: Leon Romanovsky @ 2018-01-03 14:17 UTC (permalink / raw)
To: Tariq Toukan
Cc: Julia Lawall, SF Markus Elfring, linux-rdma, netdev, LKML,
kernel-janitors
In-Reply-To: <adfc3229-d3f9-afb9-00e6-9f765af0cbe8@mellanox.com>
[-- Attachment #1: Type: text/plain, Size: 2529 bytes --]
On Wed, Jan 03, 2018 at 01:24:59PM +0200, Tariq Toukan wrote:
>
>
> On 03/01/2018 10:06 AM, Julia Lawall wrote:
> >
> >
> > On Wed, 3 Jan 2018, Tariq Toukan wrote:
> >
> > >
> > >
> > > On 01/01/2018 10:46 PM, SF Markus Elfring wrote:
> > > > From: Markus Elfring <elfring@users.sourceforge.net>
> > > > Date: Mon, 1 Jan 2018 21:42:27 +0100
> > > >
> > > > Omit an extra message for a memory allocation failure in these functions.
> > > >
> > > > This issue was detected by using the Coccinelle software.
> > > >
> > > > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> > > > ---
> > >
> > > Is this an issue? Why? What is your motivation?
> > > These are error messages, very informative, appear only upon errors, and in
> > > control flow.
> >
> > Strings take up space. Since there is a backtrace on an out of memory
> > problem, if the string does not provide any more information than the
> > position of the call, then there is not much added value. I don't know
> > what was the string in this case. If it provides some additional
> > information, then it would be reasonable to keep it.
>
> I don't really accept this claim...
>
> Short informative strings worth the tiny space they consume. It helps the
> users of our driver understand what went wrong in simple words, without the
> need to understand the role of the functions/callstack or being familiar
> with different parts of the driver code.
>
> In addition, some out-of-memory errors are recoverable, even though their
> backtrace is also printed. For example, in function mlx4_en_create_cq
> (appears in patch) we have a first allocation attempt (kzalloc_node) and a
> fallback (kzalloc). I'd prefer to state a clear error message only when both
> have failed, because otherwise the user might be confused whether the
> backtrace should indicate a malfunctioning interface, or not.
Tariq,
There is standard way to handle fallback in allocation and it is to
use __GFP_NOWARN flag in first allocation. So actually you pointed to the
"better-to-be-improved" function call.
Thanks
>
> Tariq
>
> >
> > julia
> >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: ethernet: mlx4: Delete an error message for a failed memory allocation in five functions
From: SF Markus Elfring @ 2018-01-03 14:22 UTC (permalink / raw)
To: Tariq Toukan, linux-rdma, netdev; +Cc: Julia Lawall, LKML, kernel-janitors
In-Reply-To: <adfc3229-d3f9-afb9-00e6-9f765af0cbe8@mellanox.com>
> I don't really accept this claim...
> Short informative strings worth the tiny space they consume.
There can be different opinions for their usefulness.
> In addition, some out-of-memory errors are recoverable, even though their backtrace is also printed.
How do you think about to suppress the backtrace generation for them?
> For example, in function mlx4_en_create_cq (appears in patch) we have a first allocation attempt (kzalloc_node)
Would it be helpful to pass the option “__GFP_NOWARN” there?
> and a fallback (kzalloc). I'd prefer to state a clear error message only when both have failed,
> because otherwise the user might be confused whether the backtrace should indicate a malfunctioning interface, or not.
Can the distinction become easier by any other means?
Regards,
Markus
^ permalink raw reply
* Re: [PATCH v6 3/6] can: m_can: Add PM Runtime
From: Marc Kleine-Budde @ 2018-01-03 14:25 UTC (permalink / raw)
To: Faiz Abbas, wg-5Yr1BZd7O62+XT7JhA+gdA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8
Cc: linux-can-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, nsekhar-l0cyMroinI0,
fcooper-l0cyMroinI0, robh-DgEjT+Ai2ygdnm+yROfE0A,
Wenyou.Yang-UWL1GkI3JZL3oGB3hsPCZA,
sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8
In-Reply-To: <1308ee81-a1e9-ac5a-2d97-334fa825ef8d-l0cyMroinI0@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 3878 bytes --]
On 01/03/2018 01:39 PM, Faiz Abbas wrote:
> On Tuesday 02 January 2018 09:37 PM, Marc Kleine-Budde wrote:
>> On 12/22/2017 02:31 PM, Faiz Abbas wrote:
>>> From: Franklin S Cooper Jr <fcooper-l0cyMroinI0@public.gmane.org>
>>>
>>> Add support for PM Runtime which is the new way to handle managing clocks.
>>> However, to avoid breaking SoCs not using PM_RUNTIME leave the old clk
>>> management approach in place.
>>
>> There is no PM_RUNTIME anymore since 464ed18ebdb6 ("PM: Eliminate
>> CONFIG_PM_RUNTIME")
>
> Ok. Will change the commit message.
>
>>
>> Have a look at the discussion: https://patchwork.kernel.org/patch/9436507/ :
>>
>>>> Well, I admit it would be nicer if drivers didn't have to worry about
>>>> whether or not CONFIG_PM was enabled. A slightly cleaner approach
>>>> from the one outlined above would have the probe routine do this:
>>>>
>>>> my_power_up(dev);
>>>> pm_runtime_set_active(dev);
>>>> pm_runtime_get_noresume(dev);
>>>> pm_runtime_enable(dev);
>
> This discussion seems to be about cases in which CONFIG_PM is not
> enabled. CONFIG_PM is always selected in the case of omap devices.
Yes, but in the commit message you state that you need to support
systems that don't have PM_RUNTIME enabled. The only mainline SoCs I see
is "arch/arm/boot/dts/sama5d2.dtsi" so far. Please check if they select
CONFIG_PM, then we can make the driver much simpler.
>>> PM_RUNTIME is required by OMAP based devices to handle clock management.
>>> Therefore, this allows future Texas Instruments SoCs that have the MCAN IP
>>> to work with this driver.
>>
>> Who will set the SET_RUNTIME_PM_OPS in this case?
>
> It is set with a common SET_RUNTIME_PM_OPS in the case of omap at
> arch/arm/mach-omap2/omap_device.c:632
>
> struct dev_pm_domain omap_device_pm_domain = {
> .ops = {
> SET_RUNTIME_PM_OPS(_od_runtime_suspend, _od_runtime_resume,
> NULL)
> USE_PLATFORM_PM_SLEEP_OPS
> SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(_od_suspend_noirq,
> _od_resume_noirq)
> }
> };
>
>
>>
>>> Signed-off-by: Franklin S Cooper Jr <fcooper-l0cyMroinI0@public.gmane.org>
>>> [nsekhar-l0cyMroinI0@public.gmane.org: handle pm_runtime_get_sync() failure, fix some bugs]
>>> Signed-off-by: Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>
>>> Signed-off-by: Faiz Abbas <faiz_abbas-l0cyMroinI0@public.gmane.org>
>>> ---
>>> drivers/net/can/m_can/m_can.c | 38 ++++++++++++++++++++++++++++++++++----
>>> 1 file changed, 34 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
>>> index f72116e..53e764f 100644
>>> --- a/drivers/net/can/m_can/m_can.c
>>> +++ b/drivers/net/can/m_can/m_can.c
>>> @@ -23,6 +23,7 @@
>>> #include <linux/of.h>
>>> #include <linux/of_device.h>
>>> #include <linux/platform_device.h>
>>> +#include <linux/pm_runtime.h>
>>> #include <linux/iopoll.h>
>>> #include <linux/can/dev.h>
>>>
>>> @@ -625,19 +626,33 @@ static int m_can_clk_start(struct m_can_priv *priv)
>>> {
>>> int err;
>>>
>>> + err = pm_runtime_get_sync(priv->device);
>>> + if (err) {
>>> + pm_runtime_put_noidle(priv->device);
>>
>> Why do you call this in case of an error?
>
> pm_runtime_get_sync() increments the usage count of the device before
> any error is returned. This needs to be decremented using
> pm_runtime_put_noidle().
Oh, I'm curious how many drivers don't get this right.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 2/2] l2tp: add peer_offset parameter
From: Guillaume Nault @ 2018-01-03 14:27 UTC (permalink / raw)
To: James Chapman; +Cc: Lorenzo Bianconi, David S. Miller, netdev, Hangbin Liu
In-Reply-To: <d6570c0f-2e60-6ea9-d277-27a848a49dd2@katalix.com>
On Tue, Jan 02, 2018 at 08:59:44PM +0000, James Chapman wrote:
> I just realised the peer_offset attribute changes are already applied in
> net-next. (I missed these when they were submitted just before Christmas.)
> Should these commits be reverted? We probably don't want v4.15 to get an
> additional l2tp peer_offset attribute if we are going to remove it and the
> rest of the code supporting configurable offset attributes in the next
> release.
>
Yes, I agree for a revert. I'm sorry for Lorenzo's work but I'd rather
not expand the user API in this direction.
^ permalink raw reply
* Re: [PATCH net-next 2/5] net: dsa: mv88e6xxx: Hold mutex while doing stats operations
From: Vivien Didelot @ 2018-01-03 14:32 UTC (permalink / raw)
To: Andrew Lunn, David Miller
Cc: Florian Fainelli, netdev, Russell King, Andrew Lunn
In-Reply-To: <1514988562-20079-3-git-send-email-andrew@lunn.ch>
Hi Andrew,
Andrew Lunn <andrew@lunn.ch> writes:
> -static int mv88e6xxx_get_sset_count(struct dsa_switch *ds, int port)
> +static int _mv88e6xxx_get_sset_count(struct dsa_switch *ds, int port)
> {
> struct mv88e6xxx_chip *chip = ds->priv;
>
> @@ -702,6 +706,19 @@ static int mv88e6xxx_get_sset_count(struct dsa_switch *ds, int port)
> return 0;
> }
We worked to remove the old underscore prefix convention. Please don't
add it back... Simply rework the return statements of
mv88e6xxx_get_sset_count to lock/unlock there.
>
> +static int mv88e6xxx_get_sset_count(struct dsa_switch *ds, int port)
> +{
> + struct mv88e6xxx_chip *chip = ds->priv;
> + int ret;
> +
> + mutex_lock(&chip->reg_lock);
> + ret = _mv88e6xxx_get_sset_count(ds, port);
> + mutex_unlock(&chip->reg_lock);
> +
> + return ret;
> +}
> +
> +
Extra newline.
> static void mv88e6xxx_stats_get_stats(struct mv88e6xxx_chip *chip, int port,
> uint64_t *data, int types,
> u16 bank1_select, u16 histogram)
Thanks,
Vivien
^ permalink raw reply
* Re: [PATCH net-next 1/6] phy: add 2.5G SGMII mode to the phy_mode enum
From: Antoine Tenart @ 2018-01-03 14:35 UTC (permalink / raw)
To: Florian Fainelli
Cc: Antoine Tenart, Andrew Lunn, thomas.petazzoni, ymarkman, jason,
netdev, linux-kernel, linux, kishon, nadavh, miquel.raynal,
gregory.clement, stefanc, mw, davem, linux-arm-kernel,
sebastian.hesselbarth
In-Reply-To: <91838ce5-a1a8-c41a-36e8-bef7adaf82fd@gmail.com>
Hi Florian,
On Thu, Dec 28, 2017 at 06:16:51AM -0800, Florian Fainelli wrote:
> On 12/28/2017 02:06 AM, Antoine Tenart wrote:
> > On Thu, Dec 28, 2017 at 08:20:53AM +0100, Andrew Lunn wrote:
> >> On Wed, Dec 27, 2017 at 11:14:41PM +0100, Antoine Tenart wrote:
> >>> This patch adds one more generic PHY mode to the phy_mode enum, to allow
> >>> configuring generic PHYs to the 2.5G SGMII mode by using the set_mode
> >>> callback.
> >>>
> >>> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> >>> ---
> >>> include/linux/phy/phy.h | 1 +
> >>> 1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> >>> index 4f8423a948d5..70459a28f3a1 100644
> >>> --- a/include/linux/phy/phy.h
> >>> +++ b/include/linux/phy/phy.h
> >>> @@ -28,6 +28,7 @@ enum phy_mode {
> >>> PHY_MODE_USB_DEVICE,
> >>> PHY_MODE_USB_OTG,
> >>> PHY_MODE_SGMII,
> >>> + PHY_MODE_SGMII_2_5G,
> >>> PHY_MODE_10GKR,
> >>> PHY_MODE_UFS_HS_A,
> >>> PHY_MODE_UFS_HS_B,
> >>
> >> There was a discussion maybe last month about adding 2.5G SGMII. I
> >> would prefer 2500SGMII. Putting the number first makes it uniform with
> >> the other defines, 1000BASEX, 25000BASEX, 10GKR.
> >
> > Good to know. I wasn't completely sure how to name this mode properly,
> > but I'm fine with PHY_MODE_2500SGMII. I'll update the patches and send a
> > v2 (without the dt part).
>
> And since you are respinning, please make sure you update phy_modes() in
> the same header file as well as
> Documentation/devicetree/bindings/net/ethernet.txt with the newly added
> PHY interface mode.
Actually it's a generic PHY mode I'm adding, not a network PHY mode.
There's no phy_modes() function for generic PHYs (and this 2500BaseX
mode already is supported in the network PHY modes).
Thanks!
Antoine
--
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v5 02/39] openrisc: add ioremap_nocache declaration before include asm-generic/io.h and sync ioremap prototype with it.
From: Stafford Horne @ 2018-01-03 14:38 UTC (permalink / raw)
To: Greentime Hu
Cc: greentime, linux-kernel, arnd, linux-arch, tglx, jason,
marc.zyngier, robh+dt, netdev, deanbo422, devicetree, viro,
dhowells, will.deacon, daniel.lezcano, linux-serial,
geert.uytterhoeven, linus.walleij, mark.rutland, greg, ren_guo,
rdunlap, davem, jonas, stefan.kristiansson
In-Reply-To: <3e5ba33674a883b56e20b35ea9ae34990ea838c8.1514874857.git.green.hu@gmail.com>
Hello,
On Tue, Jan 02, 2018 at 04:24:34PM +0800, Greentime Hu wrote:
> From: Greentime Hu <greentime@andestech.com>
>
> It will be built failed if commit id: d25ea659 is selected. This patch can fix this
> build error.
Ideally you would mention the commit description since the id is not yet
usptream. I found it here (its 1 in this series):
https://github.com/andestech/linux/commit/d25ea659
asm-generic/io.h: move ioremap_nocache/ioremap_uc/ioremap_wc/ioremap_...
> Signed-off-by: Greentime Hu <greentime@andestech.com>
> ---
> arch/openrisc/include/asm/io.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/openrisc/include/asm/io.h b/arch/openrisc/include/asm/io.h
> index 7c69139..6709b28 100644
> --- a/arch/openrisc/include/asm/io.h
> +++ b/arch/openrisc/include/asm/io.h
> @@ -29,13 +29,14 @@
> #define PIO_OFFSET 0
> #define PIO_MASK 0
>
> +#define ioremap_nocache ioremap_nocache
> #include <asm-generic/io.h>
Ideally we could move <asm-generic/io.h> include down to the bottom of the file
and not have to do the defines like like this, it seems clumsy to me. In
'cris', 'nios2' and other architectures I can see they have the generic include
at the bottom of the file and not need for #define's.
I tried that but I get a lot of errors. Does your patch to asm-generic/io.h
cause build issues for those architectures as well?
-Stafford
> #include <asm/pgtable.h>
>
> extern void __iomem *__ioremap(phys_addr_t offset, unsigned long size,
> pgprot_t prot);
>
> -static inline void __iomem *ioremap(phys_addr_t offset, unsigned long size)
> +static inline void __iomem *ioremap(phys_addr_t offset, size_t size)
> {
> return __ioremap(offset, size, PAGE_KERNEL);
> }
> --
> 1.7.9.5
>
^ permalink raw reply
* [PATCH] ps3_gelic_net: Delete an error message for a failed memory allocation in gelic_descr_prepare_rx()
From: SF Markus Elfring @ 2018-01-03 14:40 UTC (permalink / raw)
To: netdev, linuxppc-dev, Benjamin Herrenschmidt, Geoff Levand,
Michael Ellerman, Paul Mackerras
Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 3 Jan 2018 14:50:59 +0100
Omit an extra message for a memory allocation failure in this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/ethernet/toshiba/ps3_gelic_net.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
index 88d74aef218a..644b875e324c 100644
--- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
+++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
@@ -395,8 +395,6 @@ static int gelic_descr_prepare_rx(struct gelic_card *card,
descr->skb = dev_alloc_skb(bufsize + GELIC_NET_RXBUF_ALIGN - 1);
if (!descr->skb) {
descr->buf_addr = 0; /* tell DMAC don't touch memory */
- dev_info(ctodev(card),
- "%s:allocate skb failed !!\n", __func__);
return -ENOMEM;
}
descr->buf_size = cpu_to_be32(bufsize);
--
2.15.1
^ permalink raw reply related
* Re: [PATCH net-next 1/5] dsa: Pass the port to get_sset_count()
From: Vivien Didelot @ 2018-01-03 14:41 UTC (permalink / raw)
To: Andrew Lunn, David Miller
Cc: Florian Fainelli, netdev, Russell King, Andrew Lunn
In-Reply-To: <1514988562-20079-2-git-send-email-andrew@lunn.ch>
Hi Andrew,
Andrew Lunn <andrew@lunn.ch> writes:
> --- a/net/dsa/master.c
> +++ b/net/dsa/master.c
> @@ -42,7 +42,7 @@ static int dsa_master_get_sset_count(struct net_device *dev, int sset)
> count += ops->get_sset_count(dev, sset);
>
> if (sset == ETH_SS_STATS && ds->ops->get_sset_count)
> - count += ds->ops->get_sset_count(ds);
> + count += ds->ops->get_sset_count(ds, cpu_dp->index);
>
> return count;
> }
> @@ -76,7 +76,7 @@ static void dsa_master_get_strings(struct net_device *dev, uint32_t stringset,
> * constructed earlier
> */
> ds->ops->get_strings(ds, port, ndata);
> - count = ds->ops->get_sset_count(ds);
> + count = ds->ops->get_sset_count(ds, cpu_dp->index);
You could reuse the 'port' variable already assigned to cpu_dp->index.
> for (i = 0; i < count; i++) {
> memmove(ndata + (i * len + sizeof(pfx)),
> ndata + i * len, len - sizeof(pfx));
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index f52307296de4..0d07004d59d4 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -559,7 +559,7 @@ static int dsa_slave_get_sset_count(struct net_device *dev, int sset)
>
> count = 4;
> if (ds->ops->get_sset_count)
> - count += ds->ops->get_sset_count(ds);
> + count += ds->ops->get_sset_count(ds, dp->index);
>
> return count;
> }
Otherwise:
Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Thanks,
Vivien
^ permalink raw reply
* [PATCHv2] 3c59x: fix missing dma_mapping_error check and bad ring refill logic
From: Neil Horman @ 2018-01-03 14:44 UTC (permalink / raw)
To: netdev
Cc: tedheadster, Neil Horman, Neil Horman, Steffen Klassert,
David S. Miller
In-Reply-To: <CAP8WD_aJSCCCYWFdNV3xH_NqfrgB9XtzBGp-nnua8YSaZfFf3w@mail.gmail.com>
A few spots in 3c59x missed calls to dma_mapping_error checks, casuing
WARN_ONS to trigger. Clean those up. While we're at it, refactor the
refill code a bit so that if skb allocation or dma mapping fails, we
recycle the existing buffer. This prevents holes in the rx ring, and
makes for much simpler logic
Note: This is compile only tested. Ted, if you could run this and
confirm that it continues to work properly, I would appreciate it, as I
currently don't have access to this hardware
Signed-off-by: Neil Horman <nhorman@redhat.com>
CC: Steffen Klassert <klassert@mathematik.tu-chemnitz.de>
CC: "David S. Miller" <davem@davemloft.net>
Reported-by: tedheadster@gmail.com
---
Change notes:
v2)
* Fixed tx path to free skb on mapping error
* Refactored rx path to recycle skbs on allocation/mapping error
* Used refactoring to remove oom timer and dirty_rx index
---
drivers/net/ethernet/3com/3c59x.c | 91 +++++++++++++++++----------------------
1 file changed, 39 insertions(+), 52 deletions(-)
diff --git a/drivers/net/ethernet/3com/3c59x.c b/drivers/net/ethernet/3com/3c59x.c
index f4e13a7014bd..2928c9a4b477 100644
--- a/drivers/net/ethernet/3com/3c59x.c
+++ b/drivers/net/ethernet/3com/3c59x.c
@@ -602,7 +602,7 @@ struct vortex_private {
struct sk_buff* rx_skbuff[RX_RING_SIZE];
struct sk_buff* tx_skbuff[TX_RING_SIZE];
unsigned int cur_rx, cur_tx; /* The next free ring entry */
- unsigned int dirty_rx, dirty_tx; /* The ring entries to be free()ed. */
+ unsigned int dirty_tx; /* The ring entries to be free()ed. */
struct vortex_extra_stats xstats; /* NIC-specific extra stats */
struct sk_buff *tx_skb; /* Packet being eaten by bus master ctrl. */
dma_addr_t tx_skb_dma; /* Allocated DMA address for bus master ctrl DMA. */
@@ -618,7 +618,6 @@ struct vortex_private {
/* The remainder are related to chip state, mostly media selection. */
struct timer_list timer; /* Media selection timer. */
- struct timer_list rx_oom_timer; /* Rx skb allocation retry timer */
int options; /* User-settable misc. driver options. */
unsigned int media_override:4, /* Passed-in media type. */
default_media:4, /* Read from the EEPROM/Wn3_Config. */
@@ -760,7 +759,6 @@ static void mdio_sync(struct vortex_private *vp, int bits);
static int mdio_read(struct net_device *dev, int phy_id, int location);
static void mdio_write(struct net_device *vp, int phy_id, int location, int value);
static void vortex_timer(struct timer_list *t);
-static void rx_oom_timer(struct timer_list *t);
static netdev_tx_t vortex_start_xmit(struct sk_buff *skb,
struct net_device *dev);
static netdev_tx_t boomerang_start_xmit(struct sk_buff *skb,
@@ -1601,7 +1599,6 @@ vortex_up(struct net_device *dev)
timer_setup(&vp->timer, vortex_timer, 0);
mod_timer(&vp->timer, RUN_AT(media_tbl[dev->if_port].wait));
- timer_setup(&vp->rx_oom_timer, rx_oom_timer, 0);
if (vortex_debug > 1)
pr_debug("%s: Initial media type %s.\n",
@@ -1676,7 +1673,7 @@ vortex_up(struct net_device *dev)
window_write16(vp, 0x0040, 4, Wn4_NetDiag);
if (vp->full_bus_master_rx) { /* Boomerang bus master. */
- vp->cur_rx = vp->dirty_rx = 0;
+ vp->cur_rx = 0;
/* Initialize the RxEarly register as recommended. */
iowrite16(SetRxThreshold + (1536>>2), ioaddr + EL3_CMD);
iowrite32(0x0020, ioaddr + PktStatus);
@@ -1729,6 +1726,7 @@ vortex_open(struct net_device *dev)
struct vortex_private *vp = netdev_priv(dev);
int i;
int retval;
+ dma_addr_t dma;
/* Use the now-standard shared IRQ implementation. */
if ((retval = request_irq(dev->irq, vp->full_bus_master_rx ?
@@ -1753,7 +1751,11 @@ vortex_open(struct net_device *dev)
break; /* Bad news! */
skb_reserve(skb, NET_IP_ALIGN); /* Align IP on 16 byte boundaries */
- vp->rx_ring[i].addr = cpu_to_le32(pci_map_single(VORTEX_PCI(vp), skb->data, PKT_BUF_SZ, PCI_DMA_FROMDEVICE));
+ dma = pci_map_single(VORTEX_PCI(vp), skb->data,
+ PKT_BUF_SZ, PCI_DMA_FROMDEVICE);
+ if (dma_mapping_error(&VORTEX_PCI(vp)->dev, dma))
+ break;
+ vp->rx_ring[i].addr = cpu_to_le32(dma);
}
if (i != RX_RING_SIZE) {
pr_emerg("%s: no memory for rx ring\n", dev->name);
@@ -2067,6 +2069,12 @@ vortex_start_xmit(struct sk_buff *skb, struct net_device *dev)
int len = (skb->len + 3) & ~3;
vp->tx_skb_dma = pci_map_single(VORTEX_PCI(vp), skb->data, len,
PCI_DMA_TODEVICE);
+ if (dma_mapping_error(&VORTEX_PCI(vp)->dev, vp->tx_skb_dma)) {
+ dev_kfree_skb_any(skb);
+ dev->stats.tx_dropped++;
+ return NETDEV_TX_OK;
+ }
+
spin_lock_irq(&vp->window_lock);
window_set(vp, 7);
iowrite32(vp->tx_skb_dma, ioaddr + Wn7_MasterAddr);
@@ -2593,7 +2601,8 @@ boomerang_rx(struct net_device *dev)
int entry = vp->cur_rx % RX_RING_SIZE;
void __iomem *ioaddr = vp->ioaddr;
int rx_status;
- int rx_work_limit = vp->dirty_rx + RX_RING_SIZE - vp->cur_rx;
+ int rx_work_limit = RX_RING_SIZE;
+ dma_addr_t dma;
if (vortex_debug > 5)
pr_debug("boomerang_rx(): status %4.4x\n", ioread16(ioaddr+EL3_STATUS));
@@ -2614,7 +2623,8 @@ boomerang_rx(struct net_device *dev)
} else {
/* The packet length: up to 4.5K!. */
int pkt_len = rx_status & 0x1fff;
- struct sk_buff *skb;
+ struct sk_buff *skb, *newskb;
+ dma_addr_t newdma;
dma_addr_t dma = le32_to_cpu(vp->rx_ring[entry].addr);
if (vortex_debug > 4)
@@ -2633,9 +2643,27 @@ boomerang_rx(struct net_device *dev)
pci_dma_sync_single_for_device(VORTEX_PCI(vp), dma, PKT_BUF_SZ, PCI_DMA_FROMDEVICE);
vp->rx_copy++;
} else {
+ /* Pre-allocate the replacement skb. If it or its
+ * mapping fails then recycle the buffer thats already
+ * in place
+ */
+ newskb = netdev_alloc_skb_ip_align(dev, PKT_BUF_SZ);
+ if (!newskb) {
+ dev->stats.rx_dropped++;
+ goto clear_complete;
+ }
+ newdma = pci_map_single(VORTEX_PCI(vp), newskb->data,
+ PKT_BUF_SZ, PCI_DMA_FROMDEVICE);
+ if (dma_mapping_error(&VORTEX_PCI(vp)->dev, newdma)) {
+ dev->stats.rx_dropped++;
+ consume_skb(newskb);
+ goto clear_complete;
+ }
+
/* Pass up the skbuff already on the Rx ring. */
skb = vp->rx_skbuff[entry];
- vp->rx_skbuff[entry] = NULL;
+ vp->rx_skbuff[entry] = newskb;
+ vp->rx_ring[entry].addr = cpu_to_le32(newdma);
skb_put(skb, pkt_len);
pci_unmap_single(VORTEX_PCI(vp), dma, PKT_BUF_SZ, PCI_DMA_FROMDEVICE);
vp->rx_nocopy++;
@@ -2653,55 +2681,15 @@ boomerang_rx(struct net_device *dev)
netif_rx(skb);
dev->stats.rx_packets++;
}
- entry = (++vp->cur_rx) % RX_RING_SIZE;
- }
- /* Refill the Rx ring buffers. */
- for (; vp->cur_rx - vp->dirty_rx > 0; vp->dirty_rx++) {
- struct sk_buff *skb;
- entry = vp->dirty_rx % RX_RING_SIZE;
- if (vp->rx_skbuff[entry] == NULL) {
- skb = netdev_alloc_skb_ip_align(dev, PKT_BUF_SZ);
- if (skb == NULL) {
- static unsigned long last_jif;
- if (time_after(jiffies, last_jif + 10 * HZ)) {
- pr_warn("%s: memory shortage\n",
- dev->name);
- last_jif = jiffies;
- }
- if ((vp->cur_rx - vp->dirty_rx) == RX_RING_SIZE)
- mod_timer(&vp->rx_oom_timer, RUN_AT(HZ * 1));
- break; /* Bad news! */
- }
- vp->rx_ring[entry].addr = cpu_to_le32(pci_map_single(VORTEX_PCI(vp), skb->data, PKT_BUF_SZ, PCI_DMA_FROMDEVICE));
- vp->rx_skbuff[entry] = skb;
- }
+clear_complete:
vp->rx_ring[entry].status = 0; /* Clear complete bit. */
iowrite16(UpUnstall, ioaddr + EL3_CMD);
+ entry = (++vp->cur_rx) % RX_RING_SIZE;
}
return 0;
}
-/*
- * If we've hit a total OOM refilling the Rx ring we poll once a second
- * for some memory. Otherwise there is no way to restart the rx process.
- */
-static void
-rx_oom_timer(struct timer_list *t)
-{
- struct vortex_private *vp = from_timer(vp, t, rx_oom_timer);
- struct net_device *dev = vp->mii.dev;
-
- spin_lock_irq(&vp->lock);
- if ((vp->cur_rx - vp->dirty_rx) == RX_RING_SIZE) /* This test is redundant, but makes me feel good */
- boomerang_rx(dev);
- if (vortex_debug > 1) {
- pr_debug("%s: rx_oom_timer %s\n", dev->name,
- ((vp->cur_rx - vp->dirty_rx) != RX_RING_SIZE) ? "succeeded" : "retrying");
- }
- spin_unlock_irq(&vp->lock);
-}
-
static void
vortex_down(struct net_device *dev, int final_down)
{
@@ -2711,7 +2699,6 @@ vortex_down(struct net_device *dev, int final_down)
netdev_reset_queue(dev);
netif_stop_queue(dev);
- del_timer_sync(&vp->rx_oom_timer);
del_timer_sync(&vp->timer);
/* Turn off statistics ASAP. We update dev->stats below. */
--
2.14.3
^ permalink raw reply related
* Re: [PATCHv1 3/6] ARM: dts: imx6q-bx50v3: Add internal switch
From: Sergei Shtylyov @ 2018-01-03 14:48 UTC (permalink / raw)
To: Sebastian Reichel, Andrew Lunn, Vivien Didelot, Florian Fainelli,
Shawn Guo, Sascha Hauer, Fabio Estevam
Cc: Ian Ray, Nandor Han, Rob Herring, David S. Miller,
netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20180103122609.5482-4-sebastian.reichel-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
Hello!
On 01/03/2018 03:26 PM, Sebastian Reichel wrote:
> B850v3, B650v3 and B450v3 all have a GPIO bit banged MDIO bus to
> communicate with a Marvell switch. On all devices the switch is
> connected to a PCI based network card, which needs to be referenced
> by DT, so this also adds the common PCI root node.
>
> Signed-off-by: Sebastian Reichel <sebastian.reichel-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
> ---
> arch/arm/boot/dts/imx6q-bx50v3.dtsi | 37 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 37 insertions(+)
>
> diff --git a/arch/arm/boot/dts/imx6q-bx50v3.dtsi b/arch/arm/boot/dts/imx6q-bx50v3.dtsi
> index b915837bbb5f..689981e90e68 100644
> --- a/arch/arm/boot/dts/imx6q-bx50v3.dtsi
> +++ b/arch/arm/boot/dts/imx6q-bx50v3.dtsi
> @@ -92,6 +92,31 @@
> mux-int-port = <1>;
> mux-ext-port = <4>;
> };
> +
> + aliases {
> + mdio-gpio0 = &mdio0;
> + };
> +
> + mdio0: mdio-gpio {
> + compatible = "virtual,mdio-gpio";
> + gpios = <&gpio2 5 GPIO_ACTIVE_HIGH>, /* mdc */
> + <&gpio2 7 GPIO_ACTIVE_HIGH>; /* mdio */
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + switch@0 {
> + compatible = "marvell,mv88e6240";
Why suddenly 2 extra tabs instead of 1?
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0>;
> +
> + switch_ports: ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + };
> + };
> };
>
> &ecspi5 {
[...]
MBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] 3c59x: fix missing dma_mapping_error check
From: David Miller @ 2018-01-03 14:53 UTC (permalink / raw)
To: nhorman; +Cc: netdev, nhorman, klassert
In-Reply-To: <20180103104204.GB18309@hmswarspite.think-freely.org>
From: Neil Horman <nhorman@tuxdriver.com>
Date: Wed, 3 Jan 2018 05:42:04 -0500
> On Tue, Jan 02, 2018 at 09:48:27PM -0500, David Miller wrote:
>> And for the RX cases, it allows the RX ring to deplete to empty which
>> tends to hang most chips. You need to make the DMA failure detection
>> early and recycle the RX buffer back to the chip instead of passing
>> it up to the stack.
>>
> Strictly speaking, I think we're ok here, because the dirty_rx counter creates a
> contiguous area to refill, and we will just pick up where we left off on the
> next napi poll.
If you continually fail the mappings, even NAPI poll, eventually the
RX ring will empty.
I don't think we're ok here.
^ permalink raw reply
* Re: [PATCH net-next 3/5] net: dsa: mv88e6xxx: Allow the SERDES interfaces to have statistics
From: Vivien Didelot @ 2018-01-03 14:54 UTC (permalink / raw)
To: Andrew Lunn, David Miller
Cc: Florian Fainelli, netdev, Russell King, Andrew Lunn
In-Reply-To: <1514988562-20079-4-git-send-email-andrew@lunn.ch>
Hi Andrew,
Andrew Lunn <andrew@lunn.ch> writes:
> When gettting the number of statistics, the strings and the actual
getting
> statistics, call the SERDES ops if implemented. This means the stats
> code needs to return the number of strings/stats they have placed into
> the data, so that the SERDES strings/stats can follow on.
> /* Return the number of strings describing statistics */
> int (*stats_get_sset_count)(struct mv88e6xxx_chip *chip);
> - void (*stats_get_strings)(struct mv88e6xxx_chip *chip, uint8_t *data);
> - void (*stats_get_stats)(struct mv88e6xxx_chip *chip, int port,
> - uint64_t *data);
> + int (*stats_get_strings)(struct mv88e6xxx_chip *chip, uint8_t *data);
> + int (*stats_get_stats)(struct mv88e6xxx_chip *chip, int port,
> + uint64_t *data);
> int (*set_cpu_port)(struct mv88e6xxx_chip *chip, int port);
> int (*set_egress_port)(struct mv88e6xxx_chip *chip, int port);
> const struct mv88e6xxx_irq_ops *watchdog_ops;
> @@ -337,6 +337,13 @@ struct mv88e6xxx_ops {
> /* Power on/off a SERDES interface */
> int (*serdes_power)(struct mv88e6xxx_chip *chip, int port, bool on);
>
> + /* Statistics from the SERDES interface */
> + int (*serdes_get_sset_count)(struct mv88e6xxx_chip *chip, int port);
> + void (*serdes_get_strings)(struct mv88e6xxx_chip *chip, int port,
> + uint8_t *data);
> + void (*serdes_get_stats)(struct mv88e6xxx_chip *chip, int port,
> + uint64_t *data);
> +
Shouldn't serdes_get_{strings,stats} be symmetrical with
stats_get_{strings,stats} and return the count as well?
Thanks,
Vivien
^ permalink raw reply
* Re: [PATCHv2] 3c59x: fix missing dma_mapping_error check and bad ring refill logic
From: David Miller @ 2018-01-03 14:58 UTC (permalink / raw)
To: nhorman; +Cc: netdev, tedheadster, nhorman, klassert
In-Reply-To: <20180103144415.13446-1-nhorman@tuxdriver.com>
From: Neil Horman <nhorman@tuxdriver.com>
Date: Wed, 3 Jan 2018 09:44:15 -0500
> A few spots in 3c59x missed calls to dma_mapping_error checks, casuing
> WARN_ONS to trigger. Clean those up. While we're at it, refactor the
> refill code a bit so that if skb allocation or dma mapping fails, we
> recycle the existing buffer. This prevents holes in the rx ring, and
> makes for much simpler logic
>
> Note: This is compile only tested. Ted, if you could run this and
> confirm that it continues to work properly, I would appreciate it, as I
> currently don't have access to this hardware
>
> Signed-off-by: Neil Horman <nhorman@redhat.com>
> CC: Steffen Klassert <klassert@mathematik.tu-chemnitz.de>
> CC: "David S. Miller" <davem@davemloft.net>
> Reported-by: tedheadster@gmail.com
See my other reply.
Your RX handling must become more sophisticated.
This is exactly what we tell driver authors to do. If you cannot allocate
or DMA map a replacement RX buffer, you _MUST_ recycle the existing buffer
back to the chip rather than pass it up to the stack.
Thank you.
^ permalink raw reply
* Re: [PATCH net-next 4/5] net: dsa: mv88e6xxx: Add helper to determining if port has SERDES
From: Vivien Didelot @ 2018-01-03 15:00 UTC (permalink / raw)
To: Andrew Lunn, David Miller
Cc: Florian Fainelli, netdev, Russell King, Andrew Lunn
In-Reply-To: <1514988562-20079-5-git-send-email-andrew@lunn.ch>
Hi Andrew,
Andrew Lunn <andrew@lunn.ch> writes:
> -int mv88e6352_serdes_power(struct mv88e6xxx_chip *chip, int port, bool on)
> +static int mv88e6352_port_has_serdes(struct mv88e6xxx_chip *chip, int port)
> {
> - int err;
> u8 cmode;
> + int err;
>
> err = mv88e6xxx_port_get_cmode(chip, port, &cmode);
> - if (err)
> - return err;
> + if (err) {
> + dev_err(chip->dev, "failed to read cmode\n");
> + return 0;
> + }
>
> if ((cmode == MV88E6XXX_PORT_STS_CMODE_100BASE_X) ||
> (cmode == MV88E6XXX_PORT_STS_CMODE_1000BASE_X) ||
> - (cmode == MV88E6XXX_PORT_STS_CMODE_SGMII)) {
> + (cmode == MV88E6XXX_PORT_STS_CMODE_SGMII))
> + return 1;
> +
> + return 0;
> +}
Please use a bool for such helpers.
Thanks,
Vivien
^ permalink raw reply
* Re: [PATCH net-next 2/5] net: dsa: mv88e6xxx: Hold mutex while doing stats operations
From: Andrew Lunn @ 2018-01-03 15:02 UTC (permalink / raw)
To: Vivien Didelot; +Cc: David Miller, Florian Fainelli, netdev, Russell King
In-Reply-To: <878tdfdncl.fsf@weeman.i-did-not-set--mail-host-address--so-tickle-me>
On Wed, Jan 03, 2018 at 09:32:42AM -0500, Vivien Didelot wrote:
> Hi Andrew,
>
> Andrew Lunn <andrew@lunn.ch> writes:
>
> > -static int mv88e6xxx_get_sset_count(struct dsa_switch *ds, int port)
> > +static int _mv88e6xxx_get_sset_count(struct dsa_switch *ds, int port)
> > {
> > struct mv88e6xxx_chip *chip = ds->priv;
> >
> > @@ -702,6 +706,19 @@ static int mv88e6xxx_get_sset_count(struct dsa_switch *ds, int port)
> > return 0;
> > }
>
> We worked to remove the old underscore prefix convention. Please don't
> add it back... Simply rework the return statements of
> mv88e6xxx_get_sset_count to lock/unlock there.
Hi Vivien
That makes mv88e6xxx_get_sset_count quite complex, making it error
prone. Doing the locking in a separate function makes is very clear
the lock is held and then correctly released. So i will just rename
_mv88e6xxx_get_sset_count() to mv88e6xxx_get_sset_count_locked()
Andrew
^ permalink raw reply
* Re: [PATCH net-next 2/2] l2tp: add peer_offset parameter
From: Lorenzo Bianconi @ 2018-01-03 15:06 UTC (permalink / raw)
To: Guillaume Nault; +Cc: James Chapman, David S. Miller, netdev, Hangbin Liu
In-Reply-To: <20180103141635.GD1402@alphalink.fr>
> On Tue, Jan 02, 2018 at 08:28:03PM +0100, Lorenzo Bianconi wrote:
>> Perhaps I am little bit polarized on UABI issue, but I was rethinking
>> about it and maybe removing offset parameter would lead to an
>> interoperability issue for device running L2TPv3 since offset
>> parameter is there and it is not a nope.
>> Please consider this setup:
>> - 2 endpoint running L2TPv3, the first running net-next and the second
>> running 4.14
>> - both endpoint are configured using iproute2 in this way:
>>
>> - ip l2tp add tunnel local <ip0> remote <ip1> tunnel_id <id0>
>> peer_tunnel_id <id1> udp_sport <p0> udp_dport <p1>
>> - ip l2tp add tunnel local <ip1> remote <ip0> tunnel_id <id1>
>> peer_tunnel_id <id0> udp_sport <p1> udp_dport <p0>
>> - ip l2tp add session name l2tp0 tunnel_id <id0> session_id <s0>
>> peer_session_id <s1> offset 8
>> - ip l2tp add session name l2tp0 tunnel_id <id1> session_id <s1>
>> peer_session_id <s0> offset 8
>>
>> Can we assume offset is never used for L2TPv3?
>>
> That's what I think. You're right worrying about ABI issues. And I
> wouldn't dare proposing such a removal if I had doubts about breaking a
> user setup.
>
> Considering the lack of use cases and the absence of interoperability
> of this feature, I hardly can imagine it being used.
> But it's not only that: the feature has been buggy for years without
> anyone noticing. And this bug wasn't difficult to spot (one just needs
> to look at an L2TPv3 header in a network packet dump).
>
> It's really the combination of these three issues (buggy, no use case
> and not producing valid L2TPv3 frames) that makes me propose a removal.
Hi Guillaume, James,
I agree to remove offset parameter in this case. What about (as
already suggested by James) to take into account possible alignment
issues with previous version of L2TPv3 protocol using 'L2 specific
sublayer'?
I guess, on the kernel side (we will need to patch iproute2 on
userspace side), we need just to properly initialized the 'l2specific'
field to 0 since otherwise we will have the same memleak issue there
if assume we can have l2specific_len != {0,4}.
Moreover does it worth to add some sanity checks in netlink code to
enforce the relation between l2specific_len and l2specific_type? At
the moment there are no guarantee that if l2specific_type is set to
L2TP_L2SPECTYPE_DEFAULT, l2specific_len will be grater or equal than
4.
Regards,
Lorenzo
^ permalink raw reply
* Re: [PATCH v6 3/6] can: m_can: Add PM Runtime
From: Faiz Abbas @ 2018-01-03 15:06 UTC (permalink / raw)
To: Marc Kleine-Budde, wg, robh+dt, mark.rutland
Cc: linux-can, netdev, devicetree, linux-kernel, nsekhar, fcooper,
robh, Wenyou.Yang, sergei.shtylyov
In-Reply-To: <7d857263-14a7-6001-8f13-42d80f757573@pengutronix.de>
Hi,
On Wednesday 03 January 2018 07:55 PM, Marc Kleine-Budde wrote:
> On 01/03/2018 01:39 PM, Faiz Abbas wrote:
>> On Tuesday 02 January 2018 09:37 PM, Marc Kleine-Budde wrote:
>>> On 12/22/2017 02:31 PM, Faiz Abbas wrote:
>>>> From: Franklin S Cooper Jr <fcooper@ti.com>
>>>>
>>>> Add support for PM Runtime which is the new way to handle managing clocks.
>>>> However, to avoid breaking SoCs not using PM_RUNTIME leave the old clk
>>>> management approach in place.
>>>
>>> There is no PM_RUNTIME anymore since 464ed18ebdb6 ("PM: Eliminate
>>> CONFIG_PM_RUNTIME")
>>
>> Ok. Will change the commit message.
>>
>>>
>>> Have a look at the discussion: https://patchwork.kernel.org/patch/9436507/ :
>>>
>>>>> Well, I admit it would be nicer if drivers didn't have to worry about
>>>>> whether or not CONFIG_PM was enabled. A slightly cleaner approach
>>>>> from the one outlined above would have the probe routine do this:
>>>>>
>>>>> my_power_up(dev);
>>>>> pm_runtime_set_active(dev);
>>>>> pm_runtime_get_noresume(dev);
>>>>> pm_runtime_enable(dev);
>>
>> This discussion seems to be about cases in which CONFIG_PM is not
>> enabled. CONFIG_PM is always selected in the case of omap devices.
>
> Yes, but in the commit message you state that you need to support
> systems that don't have PM_RUNTIME enabled. The only mainline SoCs I see
> is "arch/arm/boot/dts/sama5d2.dtsi" so far. Please check if they select
> CONFIG_PM, then we can make the driver much simpler.
Actually the old clock management (for hclk which is the interface
clock) is still required as mentioned in the cover letter. Will change
the rather misleading description.
Thanks,
Faiz
>
>>>> PM_RUNTIME is required by OMAP based devices to handle clock management.
>>>> Therefore, this allows future Texas Instruments SoCs that have the MCAN IP
>>>> to work with this driver.
>>>
>>> Who will set the SET_RUNTIME_PM_OPS in this case?
>>
>> It is set with a common SET_RUNTIME_PM_OPS in the case of omap at
>> arch/arm/mach-omap2/omap_device.c:632
>>
>> struct dev_pm_domain omap_device_pm_domain = {
>> .ops = {
>> SET_RUNTIME_PM_OPS(_od_runtime_suspend, _od_runtime_resume,
>> NULL)
>> USE_PLATFORM_PM_SLEEP_OPS
>> SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(_od_suspend_noirq,
>> _od_resume_noirq)
>> }
>> };
>>
>>
>>>
>>>> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
>>>> [nsekhar@ti.com: handle pm_runtime_get_sync() failure, fix some bugs]
>>>> Signed-off-by: Sekhar Nori <nsekhar@ti.com>
>>>> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
>>>> ---
>>>> drivers/net/can/m_can/m_can.c | 38 ++++++++++++++++++++++++++++++++++----
>>>> 1 file changed, 34 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
>>>> index f72116e..53e764f 100644
>>>> --- a/drivers/net/can/m_can/m_can.c
>>>> +++ b/drivers/net/can/m_can/m_can.c
>>>> @@ -23,6 +23,7 @@
>>>> #include <linux/of.h>
>>>> #include <linux/of_device.h>
>>>> #include <linux/platform_device.h>
>>>> +#include <linux/pm_runtime.h>
>>>> #include <linux/iopoll.h>
>>>> #include <linux/can/dev.h>
>>>>
>>>> @@ -625,19 +626,33 @@ static int m_can_clk_start(struct m_can_priv *priv)
>>>> {
>>>> int err;
>>>>
>>>> + err = pm_runtime_get_sync(priv->device);
>>>> + if (err) {
>>>> + pm_runtime_put_noidle(priv->device);
>>>
>>> Why do you call this in case of an error?
>>
>> pm_runtime_get_sync() increments the usage count of the device before
>> any error is returned. This needs to be decremented using
>> pm_runtime_put_noidle().
>
> Oh, I'm curious how many drivers don't get this right.
>
> Marc
>
^ permalink raw reply
* Re: [PATCHv1 1/6] net: dsa: Support internal phy on 'cpu' port
From: Sebastian Reichel @ 2018-01-03 15:07 UTC (permalink / raw)
To: Andrew Lunn
Cc: Vivien Didelot, Florian Fainelli, Shawn Guo, Sascha Hauer,
Fabio Estevam, Ian Ray, Nandor Han, Rob Herring, David S. Miller,
netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20180103132128.GI15036-g2DYL2Zd6BY@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 3868 bytes --]
Hi Andrew,
On Wed, Jan 03, 2018 at 02:21:28PM +0100, Andrew Lunn wrote:
> On Wed, Jan 03, 2018 at 01:26:04PM +0100, Sebastian Reichel wrote:
> > This adds support for enabling the internal phy for a 'cpu' port.
> > It has been tested on GE B850v3 and B650v3, which have a built-in
> > MV88E6240 switch connected to a PCIe based network card. Without
> > this patch the link does not come up and no traffic can be routed
> > through the switch.
> >
> > Signed-off-by: Sebastian Reichel <sebastian.reichel-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
> > ---
> > net/dsa/port.c | 26 ++++++++++++++++++++++----
> > 1 file changed, 22 insertions(+), 4 deletions(-)
> >
> > diff --git a/net/dsa/port.c b/net/dsa/port.c
> > index bb4be2679904..f99c1d34416c 100644
> > --- a/net/dsa/port.c
> > +++ b/net/dsa/port.c
> > @@ -282,6 +282,10 @@ int dsa_port_fixed_link_register_of(struct dsa_port *dp)
> > int mode;
> > int err;
> >
> > + mode = of_get_phy_mode(dn);
> > + if (mode < 0)
> > + mode = PHY_INTERFACE_MODE_NA;
> > +
> > if (of_phy_is_fixed_link(dn)) {
> > err = of_phy_register_fixed_link(dn);
> > if (err) {
> > @@ -292,10 +296,6 @@ int dsa_port_fixed_link_register_of(struct dsa_port *dp)
> > }
> >
> > phydev = of_phy_find_device(dn);
> > -
> > - mode = of_get_phy_mode(dn);
> > - if (mode < 0)
> > - mode = PHY_INTERFACE_MODE_NA;
> > phydev->interface = mode;
> >
> > genphy_config_init(phydev);
> > @@ -305,6 +305,24 @@ int dsa_port_fixed_link_register_of(struct dsa_port *dp)
> > ds->ops->adjust_link(ds, port, phydev);
> >
> > put_device(&phydev->mdio.dev);
> > + } else if (mode == PHY_INTERFACE_MODE_INTERNAL ||
> > + mode == PHY_INTERFACE_MODE_NA) {
>
> Hi Sebastian
>
> I understand what you are trying to do, i've got boards which also
> have back-to-back PHYs for the CPU port. These boards however have the
> strapping correct, so nothing needs doing in software.
What I have is a PCIe intel network card with phy, that is wired to a
mv88e6240 switch. The network card is exposed as normal network device,
so phy is enabled when the interface is brought up. The 'cpu' port
for mv88e6240 has an integrated phy, that needs to be enabled.
Your boards must be different, since mv88e6xxx is being reset during
probe(). So even if the 'cpu' phy was enabled before driver probe(),
it would be disabled afterwards.
> But the way you are doing it is wrong. PHY_INTERFACE_MODE_NA means
> something else has already setup the interface mode, leave it alone.
Ok, I assumed, that PHY_INTERFACE_MODE_NA means "no explicit
configuration found, use implicit configuration". E.g. for
mv88e6xxx the downstream ports are not configured in DT, but
their PHY is enabled.
> PHY_INTERFACE_MODE_INTERNAL means there is some other sort of bus
> between the MAC and the PHY than the normal MII.
>
> What you want to say is that there is a PHY on this port, and that you
> want to configure it to a given fixed configuration, probably 1000
> Full, with auto-neg turned off. This is something completely different
> to a fixed phy, which is used when there is no PHY at all.
That's why I put the new code into
if (of_phy_is_fixed_link(...)) {
<<< old code >>>
} else {
<<< new code >>>
}
I agree, that the function name dsa_port_fixed_link_register_of() is
a bit confusing with the added code. I actually added this to
dsa_cpu_dsa_setup() and with the rebase to current master it ended
up there.
> What state is the PHY in, if you don't have this patch? Is it powered
> down?
The phy is part of mv88e6240, which is being reset during probe.
So the phy is powered down and DSA is not functional except for
phy information of downstream ports. The PCIe network interface
does not detect a carrier.
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox