Re: [PATCH v7 net-next 2/2] net: dsa: qca: ar9331: export stats64
On Sat, Jan 09, 2021 at 02:21:43AM +0200, Vladimir Oltean wrote:
> On Fri, Jan 08, 2021 at 06:32:28AM +0100, Oleksij Rempel wrote:
> > May be the "net: dsa: add optional stats64 support" can already be
> > taken?
>
> I'm not sure that I see the point. David and Jakub won't cherry-pick
> partial series, and if you resend just patch 1/2, they won't accept new
> code that doesn't have any callers.
>
> You don't have to wait for my series if you don't want to. If you're
> going to conflict with it anyway (it changes the prototype of
> ndo_get_stats64), you might as well not wait. I don't know when, or if,
> it's going to be over with it. It is going to take at least one more
> respin since it now conflicts with net.git commit 9f9d41f03bb0 ("docs:
> net: fix documentation on .ndo_get_stats") merged a few hours ago into
> net-next. So just say which way it is that you prefer.
Ok, thx. If no one is against it, i'll prefer to push this patches now.
Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [PATCH v7 net-next 2/2] net: dsa: qca: ar9331: export stats64
On Thu, 7 Jan 2021 15:36:45 +0100 Andrew Lunn wrote:
> > +static void ar9331_get_stats64(struct dsa_switch *ds, int port,
> > + struct rtnl_link_stats64 *s)
> > +{
> > + struct ar9331_sw_priv *priv = (struct ar9331_sw_priv *)ds->priv;
> > + struct ar9331_sw_port *p = >port[port];
> > +
> > + spin_lock(>stats_lock);
> > + memcpy(s, >stats, sizeof(*s));
> > + spin_unlock(>stats_lock);
> > +}
>
> This should probably wait until Vladimir's changes for stat64 are
> merged, so this call can sleep. You can then return up to date
> statistics.
Plus rx_nohandler is still updated from HW stats here :|
+ stats->rx_nohandler += raw.filtered;
Re: [PATCH v7 net-next 2/2] net: dsa: qca: ar9331: export stats64
On Fri, Jan 08, 2021 at 06:32:28AM +0100, Oleksij Rempel wrote:
> May be the "net: dsa: add optional stats64 support" can already be
> taken?
I'm not sure that I see the point. David and Jakub won't cherry-pick
partial series, and if you resend just patch 1/2, they won't accept new
code that doesn't have any callers.
You don't have to wait for my series if you don't want to. If you're
going to conflict with it anyway (it changes the prototype of
ndo_get_stats64), you might as well not wait. I don't know when, or if,
it's going to be over with it. It is going to take at least one more
respin since it now conflicts with net.git commit 9f9d41f03bb0 ("docs:
net: fix documentation on .ndo_get_stats") merged a few hours ago into
net-next. So just say which way it is that you prefer.
Re: [PATCH v7 net-next 2/2] net: dsa: qca: ar9331: export stats64
On Thu, Jan 07, 2021 at 03:36:45PM +0100, Andrew Lunn wrote:
> > +static void ar9331_get_stats64(struct dsa_switch *ds, int port,
> > + struct rtnl_link_stats64 *s)
> > +{
> > + struct ar9331_sw_priv *priv = (struct ar9331_sw_priv *)ds->priv;
> > + struct ar9331_sw_port *p = >port[port];
> > +
> > + spin_lock(>stats_lock);
> > + memcpy(s, >stats, sizeof(*s));
> > + spin_unlock(>stats_lock);
> > +}
>
> This should probably wait until Vladimir's changes for stat64 are
> merged, so this call can sleep. You can then return up to date
> statistics.
Ack, no problem. Beside, i forgot to collect all the Reviewed-by tags.
Will resend all needed changes after Vladimirs patches are accepted.
May be the "net: dsa: add optional stats64 support" can already be
taken?
Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [PATCH v7 net-next 2/2] net: dsa: qca: ar9331: export stats64
> +static void ar9331_get_stats64(struct dsa_switch *ds, int port,
> +struct rtnl_link_stats64 *s)
> +{
> + struct ar9331_sw_priv *priv = (struct ar9331_sw_priv *)ds->priv;
> + struct ar9331_sw_port *p = >port[port];
> +
> + spin_lock(>stats_lock);
> + memcpy(s, >stats, sizeof(*s));
> + spin_unlock(>stats_lock);
> +}
This should probably wait until Vladimir's changes for stat64 are
merged, so this call can sleep. You can then return up to date
statistics.
Andrew
[PATCH v7 net-next 2/2] net: dsa: qca: ar9331: export stats64
Add stats support for the ar9331 switch.
Signed-off-by: Oleksij Rempel
---
drivers/net/dsa/qca/ar9331.c | 164 ++-
1 file changed, 163 insertions(+), 1 deletion(-)
diff --git a/drivers/net/dsa/qca/ar9331.c b/drivers/net/dsa/qca/ar9331.c
index 4d49c5f2b790..1a15845ceedd 100644
--- a/drivers/net/dsa/qca/ar9331.c
+++ b/drivers/net/dsa/qca/ar9331.c
@@ -101,6 +101,9 @@
AR9331_SW_PORT_STATUS_RX_FLOW_EN | AR9331_SW_PORT_STATUS_TX_FLOW_EN | \
AR9331_SW_PORT_STATUS_SPEED_M)
+/* MIB registers */
+#define AR9331_MIB_COUNTER(x) (0x2 + ((x) * 0x100))
+
/* Phy bypass mode
*
* Bit: | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 |10 |11 |12 |13 |14 |15 |
@@ -154,6 +157,66 @@
#define AR9331_SW_MDIO_POLL_SLEEP_US 1
#define AR9331_SW_MDIO_POLL_TIMEOUT_US 20
+/* The interval should be small enough to avoid overflow of 32bit MIBs */
+/*
+ * FIXME: until we can read MIBs from stats64 call directly (i.e. sleep
+ * there), we have to poll stats more frequently then it is actually needed.
+ * For overflow protection, normally, 100 sec interval should have been OK.
+ */
+#define STATS_INTERVAL_JIFFIES (3 * HZ)
+
+struct ar9331_sw_stats_raw {
+ u32 rxbroad;/* 0x00 */
+ u32 rxpause;/* 0x04 */
+ u32 rxmulti;/* 0x08 */
+ u32 rxfcserr; /* 0x0c */
+ u32 rxalignerr; /* 0x10 */
+ u32 rxrunt; /* 0x14 */
+ u32 rxfragment; /* 0x18 */
+ u32 rx64byte; /* 0x1c */
+ u32 rx128byte; /* 0x20 */
+ u32 rx256byte; /* 0x24 */
+ u32 rx512byte; /* 0x28 */
+ u32 rx1024byte; /* 0x2c */
+ u32 rx1518byte; /* 0x30 */
+ u32 rxmaxbyte; /* 0x34 */
+ u32 rxtoolong; /* 0x38 */
+ u32 rxgoodbyte; /* 0x3c */
+ u32 rxgoodbyte_hi;
+ u32 rxbadbyte; /* 0x44 */
+ u32 rxbadbyte_hi;
+ u32 rxoverflow; /* 0x4c */
+ u32 filtered; /* 0x50 */
+ u32 txbroad;/* 0x54 */
+ u32 txpause;/* 0x58 */
+ u32 txmulti;/* 0x5c */
+ u32 txunderrun; /* 0x60 */
+ u32 tx64byte; /* 0x64 */
+ u32 tx128byte; /* 0x68 */
+ u32 tx256byte; /* 0x6c */
+ u32 tx512byte; /* 0x70 */
+ u32 tx1024byte; /* 0x74 */
+ u32 tx1518byte; /* 0x78 */
+ u32 txmaxbyte; /* 0x7c */
+ u32 txoversize; /* 0x80 */
+ u32 txbyte; /* 0x84 */
+ u32 txbyte_hi;
+ u32 txcollision;/* 0x8c */
+ u32 txabortcol; /* 0x90 */
+ u32 txmulticol; /* 0x94 */
+ u32 txsinglecol;/* 0x98 */
+ u32 txexcdefer; /* 0x9c */
+ u32 txdefer;/* 0xa0 */
+ u32 txlatecol; /* 0xa4 */
+};
+
+struct ar9331_sw_port {
+ int idx;
+ struct delayed_work mib_read;
+ struct rtnl_link_stats64 stats;
+ struct spinlock stats_lock;
+};
+
struct ar9331_sw_priv {
struct device *dev;
struct dsa_switch ds;
@@ -165,8 +228,17 @@ struct ar9331_sw_priv {
struct mii_bus *sbus; /* mdio slave */
struct regmap *regmap;
struct reset_control *sw_reset;
+ struct ar9331_sw_port port[AR9331_SW_PORTS];
};
+static struct ar9331_sw_priv *ar9331_sw_port_to_priv(struct ar9331_sw_port
*port)
+{
+ struct ar9331_sw_port *p = port - port->idx;
+
+ return (struct ar9331_sw_priv *)((void *)p -
+offsetof(struct ar9331_sw_priv, port));
+}
+
/* Warning: switch reset will reset last AR9331_SW_MDIO_PHY_MODE_PAGE request
* If some kind of optimization is used, the request should be repeated.
*/
@@ -424,6 +496,7 @@ static void ar9331_sw_phylink_mac_link_down(struct
dsa_switch *ds, int port,
phy_interface_t interface)
{
struct ar9331_sw_priv *priv = (struct ar9331_sw_priv *)ds->priv;
+ struct ar9331_sw_port *p = >port[port];
struct regmap *regmap = priv->regmap;
int ret;
@@ -431,6 +504,8 @@ static void ar9331_sw_phylink_mac_link_down(struct
dsa_switch *ds, int port,
AR9331_SW_PORT_STATUS_MAC_MASK, 0);
if (ret)
dev_err_ratelimited(priv->dev, "%s: %i\n", __func__, ret);
+
+ cancel_delayed_work_sync(>mib_read);
}
static void
