| Message ID | 1549598829-25970-3-git-send-email-Tristram.Ha@microchip.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | net: dsa: microchip: add MIB counters support | expand |
On Thu, Feb 07, 2019 at 08:07:07PM -0800, Tristram.Ha@microchip.com wrote: > From: Tristram Ha <Tristram.Ha@microchip.com> > > Add MIB counter reading support. > > Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com> > --- > drivers/net/dsa/microchip/ksz9477.c | 139 +++++++++++++++++++++++---------- > drivers/net/dsa/microchip/ksz_common.c | 96 +++++++++++++++++++++++ > drivers/net/dsa/microchip/ksz_common.h | 2 + > drivers/net/dsa/microchip/ksz_priv.h | 7 +- > 4 files changed, 198 insertions(+), 46 deletions(-) > > diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c > index 0fdb22d..4502e13 100644 > --- a/drivers/net/dsa/microchip/ksz9477.c > +++ b/drivers/net/dsa/microchip/ksz9477.c > @@ -10,6 +10,7 @@ > #include <linux/gpio.h> > #include <linux/kernel.h> > #include <linux/module.h> > +#include <linux/iopoll.h> > #include <linux/platform_data/microchip-ksz.h> > #include <linux/phy.h> > #include <linux/etherdevice.h> > @@ -18,8 +19,8 @@ > #include <net/switchdev.h> > > #include "ksz_priv.h" > -#include "ksz_common.h" > #include "ksz9477_reg.h" > +#include "ksz_common.h" > > static const struct { > int index; > @@ -92,6 +93,27 @@ static void ksz9477_port_cfg32(struct ksz_device *dev, int port, int offset, > ksz_write32(dev, addr, data); > } > > +#define read8_op(addr) \ > +({ \ > + u8 data; \ > + ksz_read8(dev, addr, &data); \ > + data; \ > +}) > + > +#define read32_op(addr) \ > +({ \ > + u32 data; \ > + ksz_read32(dev, addr, &data); \ > + data; \ > +}) These two are not used. Please remove them. > + > +#define pread32_op(addr) \ > +({ \ > + u32 data; \ > + ksz_pread32(dev, port, addr, &data); \ > + data; \ > +}) It works, but it is not nice, and it makes assumptions about how readx_poll_timeout is implemented. > + ret = readx_poll_timeout(pread32_op, REG_PORT_MIB_CTRL_STAT__4, data, > + !(data & MIB_COUNTER_READ), 10, 1000); The macro is only used one, and addr is fixed, REG_PORT_MIB_CTRL_STAT__4. So you can at least replace addr with port, and rename the macro pread32_stat(port). > + /* failed to read MIB. get out of loop */ > + if (ret < 0) { > + dev_dbg(dev->dev, "Failed to get MIB\n"); > + return; > + } > + > + /* count resets upon read */ > + ksz_pread32(dev, port, REG_PORT_MIB_DATA, &data); > + *cnt += data; > +} > + > +static void ksz9477_r_mib_pkt(struct ksz_device *dev, int port, u16 addr, > + u64 *dropped, u64 *cnt) > +{ > + addr = ksz9477_mib_names[addr].index; > + ksz9477_r_mib_cnt(dev, port, addr, cnt); > +} > + > +static void ksz9477_freeze_mib(struct ksz_device *dev, int port, bool freeze) > +{ > + struct ksz_port *p = &dev->ports[port]; > + u32 val = freeze ? MIB_COUNTER_FLUSH_FREEZE : 0; Reverse Christmas tree. > + > + /* enable/disable the port for flush/freeze function */ > + mutex_lock(&p->mib.cnt_mutex); > + ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4, val); > + > + /* used by MIB counter reading code to know freeze is enabled */ > + p->freeze = freeze; > + mutex_unlock(&p->mib.cnt_mutex); > +} > +static void ksz_mib_read_work(struct work_struct *work) > +{ > + struct ksz_device *dev = > + container_of(work, struct ksz_device, mib_read); > + struct ksz_port *p; > + struct ksz_port_mib *mib; > + int i; > + > + for (i = 0; i < dev->mib_port_cnt; i++) { > + p = &dev->ports[i]; > + if (!p->on) > + continue; > + mib = &p->mib; > + mutex_lock(&mib->cnt_mutex); > + > + /* read only dropped counters when link is not up */ > + if (p->link_just_down) > + p->link_just_down = 0; > + else if (!p->phydev.link) > + mib->cnt_ptr = dev->reg_mib_cnt; This link_just_down stuff is not clear at all. Why can the drop counters not be read when the link is up? > + port_r_cnt(dev, i); > + mutex_unlock(&mib->cnt_mutex); > + } > +}
> > +static void ksz9477_freeze_mib(struct ksz_device *dev, int port, bool > freeze) > > +{ > > + struct ksz_port *p = &dev->ports[port]; > > + u32 val = freeze ? MIB_COUNTER_FLUSH_FREEZE : 0; > > Reverse Christmas tree. There was a checkpatch.pl patch in 2016 that tried to check this, but it was never accepted? > > + /* read only dropped counters when link is not up */ > > + if (p->link_just_down) > > + p->link_just_down = 0; > > + else if (!p->phydev.link) > > + mib->cnt_ptr = dev->reg_mib_cnt; > > This link_just_down stuff is not clear at all. Why can the drop > counters not be read when the link is up? All of the MIB counters, except some that may be marked by driver, do not get updated when the link is down, so it is a waste of time to read them. My intention is the driver eventually reads the MIB counters at least every second or faster so that the ethtool API called to show MIB counters gets them from memory rather than starting a read operation. For now that API is called from user space with the ethtool utility, so it may not be called too often and too fast. But theoretically that API can be called from a program continually. For simple switches that do not need to do anything special the MIB read operation does not cause any issue except CPU load, for more complicate switches that need to do some background operations too many read operation can affect some critical functions.
> All of the MIB counters, except some that may be marked by driver, > do not get updated when the link is down, so it is a waste of time > to read them. Hi Tristram O.K, so make this clear in the code. Maybe rather than having this link_just_down, have the adjust link callback update the cached values for all counters? > My intention is the driver eventually reads the MIB counters at > least every second or faster so that the ethtool API called to show > MIB counters gets them from memory rather than starting a read > operation. The user expects to see the current counters, not some cached values. For me it is O.K. to frequently read the counters to prevent wrap around, but each ethtool call should update the counters before returning them to user space. > For simple switches that do not need to do anything special the MIB > read operation does not cause any issue except CPU load, for more > complicate switches that need to do some background operations too > many read operation can affect some critical functions. Sounds like a bad design of the switch, if reading statistics from it can upset its operation. You might want to consider rate limiting the ethtool call. Andrew
On February 12, 2019 6:39:49 PM PST, Tristram.Ha@microchip.com wrote: >> > +static void ksz9477_freeze_mib(struct ksz_device *dev, int port, >bool >> freeze) >> > +{ >> > + struct ksz_port *p = &dev->ports[port]; >> > + u32 val = freeze ? MIB_COUNTER_FLUSH_FREEZE : 0; >> >> Reverse Christmas tree. > >There was a checkpatch.pl patch in 2016 that tried to check this, but >it was never accepted? > >> > + /* read only dropped counters when link is not up */ >> > + if (p->link_just_down) >> > + p->link_just_down = 0; >> > + else if (!p->phydev.link) >> > + mib->cnt_ptr = dev->reg_mib_cnt; >> >> This link_just_down stuff is not clear at all. Why can the drop >> counters not be read when the link is up? > >All of the MIB counters, except some that may be marked by driver, do >not get updated when the link is down, so it is a waste of time to read >them. Can you use netif_running() to determine that condition? Maintaining your own set of variables when the PHY state machine should already determine the link state sounds redundant if not error prone. > >My intention is the driver eventually reads the MIB counters at least >every second or faster so that the ethtool API called to show MIB >counters gets them from memory rather than starting a read operation. >For now that API is called from user space with the ethtool utility, so >it may not be called too often and too fast. But theoretically that >API can be called from a program continually. > >For simple switches that do not need to do anything special the MIB >read operation does not cause any issue except CPU load, for more >complicate switches that need to do some background operations too many >read operation can affect some critical functions. Some switches have a MIB autocast feature taking a snapshot which AFAIR is internally implemented as a fast read register with no contention on other registers internally, do you have something similar?
> >> > + /* read only dropped counters when link is not up */ > >> > + if (p->link_just_down) > >> > + p->link_just_down = 0; > >> > + else if (!p->phydev.link) > >> > + mib->cnt_ptr = dev->reg_mib_cnt; > >> > >> This link_just_down stuff is not clear at all. Why can the drop > >> counters not be read when the link is up? > > > >All of the MIB counters, except some that may be marked by driver, do > >not get updated when the link is down, so it is a waste of time to read > >them. > > Can you use netif_running() to determine that condition? Maintaining your > own set of variables when the PHY state machine should already determine > the link state sounds redundant if not error prone. > The driver can store the PHY device pointer passed to it when the port is enabled. But I am a little worried that pointer can be changed or completely gone as it is out of control of the driver. > >My intention is the driver eventually reads the MIB counters at least > >every second or faster so that the ethtool API called to show MIB > >counters gets them from memory rather than starting a read operation. > >For now that API is called from user space with the ethtool utility, so > >it may not be called too often and too fast. But theoretically that > >API can be called from a program continually. > > > >For simple switches that do not need to do anything special the MIB > >read operation does not cause any issue except CPU load, for more > >complicate switches that need to do some background operations too many > >read operation can affect some critical functions. > > Some switches have a MIB autocast feature taking a snapshot which AFAIR is > internally implemented as a fast read register with no contention on other > registers internally, do you have something similar? There is no such function in the switch. Every MIB counter read has to go through a single SPI transfer using indirect access. There are no table-like stored values that a single SPI transfer can retrieve all. For i2C the access is even slower, but then I do not expect this access mechanism is used when the switch can do more complex things.
On 2/14/19 11:26 AM, Tristram.Ha@microchip.com wrote: >>>>> + /* read only dropped counters when link is not up */ >>>>> + if (p->link_just_down) >>>>> + p->link_just_down = 0; >>>>> + else if (!p->phydev.link) >>>>> + mib->cnt_ptr = dev->reg_mib_cnt; >>>> >>>> This link_just_down stuff is not clear at all. Why can the drop >>>> counters not be read when the link is up? >>> >>> All of the MIB counters, except some that may be marked by driver, do >>> not get updated when the link is down, so it is a waste of time to read >>> them. >> >> Can you use netif_running() to determine that condition? Maintaining your >> own set of variables when the PHY state machine should already determine >> the link state sounds redundant if not error prone. >> > > The driver can store the PHY device pointer passed to it when the port is enabled. But I am a little worried that pointer can be changed or completely gone as it is out of control of the driver. The per-port network device is accessible from dp->slave so you can do netif_running(dp->slave) from your driver, what are you talking about here? > >>> My intention is the driver eventually reads the MIB counters at least >>> every second or faster so that the ethtool API called to show MIB >>> counters gets them from memory rather than starting a read operation. >>> For now that API is called from user space with the ethtool utility, so >>> it may not be called too often and too fast. But theoretically that >>> API can be called from a program continually. >>> >>> For simple switches that do not need to do anything special the MIB >>> read operation does not cause any issue except CPU load, for more >>> complicate switches that need to do some background operations too many >>> read operation can affect some critical functions. >> >> Some switches have a MIB autocast feature taking a snapshot which AFAIR is >> internally implemented as a fast read register with no contention on other >> registers internally, do you have something similar? > > There is no such function in the switch. Every MIB counter read has to go through a single SPI transfer using indirect access. There are no table-like stored values that a single SPI transfer can retrieve all. > > For i2C the access is even slower, but then I do not expect this access mechanism is used when the switch can do more complex things. > Is that something you are considering to change for future designs?
On Tue, 2019-02-12 at 19:51 -0800, Florian Fainelli wrote: > On February 12, 2019 6:39:49 PM PST, Tristram.Ha@microchip.com wrote: > > > > +static void ksz9477_freeze_mib(struct ksz_device *dev, int port, > > > > + bool freeze) > > > > +{ > > > > + struct ksz_port *p = &dev->ports[port]; > > > > + u32 val = freeze ? MIB_COUNTER_FLUSH_FREEZE : 0; > > > > > > Reverse Christmas tree. > > > > There was a checkpatch.pl patch in 2016 that tried to check this, but > > it was never accepted? https://lore.kernel.org/patchwork/patch/732076/ While I still think use of reverse christmas tree is misguided, there were some disagreements about what form it really is: Is this reverse christmas tree? void foo(void) { int aa; int a = 1; or is void foo(void) { int a = 1; int aa; IMHO: neither really helps to visually find or scan for automatics so the whole concept isn't particularly useful.
On Wed 2019-02-20 15:49:25, Joe Perches wrote: > On Tue, 2019-02-12 at 19:51 -0800, Florian Fainelli wrote: > > On February 12, 2019 6:39:49 PM PST, Tristram.Ha@microchip.com wrote: > > > > > +static void ksz9477_freeze_mib(struct ksz_device *dev, int port, > > > > > + bool freeze) > > > > > +{ > > > > > + struct ksz_port *p = &dev->ports[port]; > > > > > + u32 val = freeze ? MIB_COUNTER_FLUSH_FREEZE : 0; > > > > > > > > Reverse Christmas tree. > > > > > > There was a checkpatch.pl patch in 2016 that tried to check this, but > > > it was never accepted? > > https://lore.kernel.org/patchwork/patch/732076/ > > While I still think use of reverse christmas tree is misguided, > there were some disagreements about what form it really is: Agreed about "misguided". Often there's more logical order, like "self" pointer first... which this patch follows. Pavel
diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c index 0fdb22d..4502e13 100644 --- a/drivers/net/dsa/microchip/ksz9477.c +++ b/drivers/net/dsa/microchip/ksz9477.c @@ -10,6 +10,7 @@ #include <linux/gpio.h> #include <linux/kernel.h> #include <linux/module.h> +#include <linux/iopoll.h> #include <linux/platform_data/microchip-ksz.h> #include <linux/phy.h> #include <linux/etherdevice.h> @@ -18,8 +19,8 @@ #include <net/switchdev.h> #include "ksz_priv.h" -#include "ksz_common.h" #include "ksz9477_reg.h" +#include "ksz_common.h" static const struct { int index; @@ -92,6 +93,27 @@ static void ksz9477_port_cfg32(struct ksz_device *dev, int port, int offset, ksz_write32(dev, addr, data); } +#define read8_op(addr) \ +({ \ + u8 data; \ + ksz_read8(dev, addr, &data); \ + data; \ +}) + +#define read32_op(addr) \ +({ \ + u32 data; \ + ksz_read32(dev, addr, &data); \ + data; \ +}) + +#define pread32_op(addr) \ +({ \ + u32 data; \ + ksz_pread32(dev, port, addr, &data); \ + data; \ +}) + static int ksz9477_wait_vlan_ctrl_ready(struct ksz_device *dev, u32 waiton, int timeout) { @@ -259,6 +281,70 @@ static int ksz9477_reset_switch(struct ksz_device *dev) return 0; } +static void ksz9477_r_mib_cnt(struct ksz_device *dev, int port, u16 addr, + u64 *cnt) +{ + u32 data; + int ret; + struct ksz_port *p = &dev->ports[port]; + + /* retain the flush/freeze bit */ + data = p->freeze ? MIB_COUNTER_FLUSH_FREEZE : 0; + data |= MIB_COUNTER_READ; + data |= (addr << MIB_COUNTER_INDEX_S); + ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4, data); + + ret = readx_poll_timeout(pread32_op, REG_PORT_MIB_CTRL_STAT__4, data, + !(data & MIB_COUNTER_READ), 10, 1000); + + /* failed to read MIB. get out of loop */ + if (ret < 0) { + dev_dbg(dev->dev, "Failed to get MIB\n"); + return; + } + + /* count resets upon read */ + ksz_pread32(dev, port, REG_PORT_MIB_DATA, &data); + *cnt += data; +} + +static void ksz9477_r_mib_pkt(struct ksz_device *dev, int port, u16 addr, + u64 *dropped, u64 *cnt) +{ + addr = ksz9477_mib_names[addr].index; + ksz9477_r_mib_cnt(dev, port, addr, cnt); +} + +static void ksz9477_freeze_mib(struct ksz_device *dev, int port, bool freeze) +{ + struct ksz_port *p = &dev->ports[port]; + u32 val = freeze ? MIB_COUNTER_FLUSH_FREEZE : 0; + + /* enable/disable the port for flush/freeze function */ + mutex_lock(&p->mib.cnt_mutex); + ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4, val); + + /* used by MIB counter reading code to know freeze is enabled */ + p->freeze = freeze; + mutex_unlock(&p->mib.cnt_mutex); +} + +static void ksz9477_port_init_cnt(struct ksz_device *dev, int port) +{ + struct ksz_port_mib *mib = &dev->ports[port].mib; + + /* flush all enabled port MIB counters */ + mutex_lock(&mib->cnt_mutex); + ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4, + MIB_COUNTER_FLUSH_FREEZE); + ksz_write8(dev, REG_SW_MAC_CTRL_6, SW_MIB_COUNTER_FLUSH); + ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4, 0); + mutex_unlock(&mib->cnt_mutex); + + mib->cnt_ptr = 0; + memset(mib->counters, 0, dev->mib_cnt * sizeof(u64)); +} + static enum dsa_tag_protocol ksz9477_get_tag_protocol(struct dsa_switch *ds, int port) { @@ -342,47 +428,6 @@ static void ksz9477_get_strings(struct dsa_switch *ds, int port, } } -static void ksz_get_ethtool_stats(struct dsa_switch *ds, int port, - uint64_t *buf) -{ - struct ksz_device *dev = ds->priv; - int i; - u32 data; - int timeout; - - mutex_lock(&dev->stats_mutex); - - for (i = 0; i < TOTAL_SWITCH_COUNTER_NUM; i++) { - data = MIB_COUNTER_READ; - data |= ((ksz9477_mib_names[i].index & 0xFF) << - MIB_COUNTER_INDEX_S); - ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4, data); - - timeout = 1000; - do { - ksz_pread32(dev, port, REG_PORT_MIB_CTRL_STAT__4, - &data); - usleep_range(1, 10); - if (!(data & MIB_COUNTER_READ)) - break; - } while (timeout-- > 0); - - /* failed to read MIB. get out of loop */ - if (!timeout) { - dev_dbg(dev->dev, "Failed to get MIB\n"); - break; - } - - /* count resets upon read */ - ksz_pread32(dev, port, REG_PORT_MIB_DATA, &data); - - dev->mib_value[i] += (uint64_t)data; - buf[i] = dev->mib_value[i]; - } - - mutex_unlock(&dev->stats_mutex); -} - static void ksz9477_cfg_port_member(struct ksz_device *dev, int port, u8 member) { @@ -1153,9 +1198,14 @@ static int ksz9477_setup(struct dsa_switch *ds) /* queue based egress rate limit */ ksz_cfg(dev, REG_SW_MAC_CTRL_5, SW_OUT_RATE_LIMIT_QUEUE_BASED, true); + /* enable global MIB counter freeze function */ + ksz_cfg(dev, REG_SW_MAC_CTRL_6, SW_MIB_COUNTER_FREEZE, true); + /* start switch */ ksz_cfg(dev, REG_SW_OPERATION, SW_START, true); + ksz_init_mib_timer(dev); + return 0; } @@ -1290,6 +1340,7 @@ static int ksz9477_switch_init(struct ksz_device *dev) if (!dev->ports) return -ENOMEM; for (i = 0; i < dev->mib_port_cnt; i++) { + mutex_init(&dev->ports[i].mib.cnt_mutex); dev->ports[i].mib.counters = devm_kzalloc(dev->dev, sizeof(u64) * @@ -1314,6 +1365,10 @@ static void ksz9477_switch_exit(struct ksz_device *dev) .flush_dyn_mac_table = ksz9477_flush_dyn_mac_table, .phy_setup = ksz9477_phy_setup, .port_setup = ksz9477_port_setup, + .r_mib_cnt = ksz9477_r_mib_cnt, + .r_mib_pkt = ksz9477_r_mib_pkt, + .freeze_mib = ksz9477_freeze_mib, + .port_init_cnt = ksz9477_port_init_cnt, .shutdown = ksz9477_reset_switch, .detect = ksz9477_switch_detect, .init = ksz9477_switch_init, diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c index a57bda7..62d4344 100644 --- a/drivers/net/dsa/microchip/ksz_common.c +++ b/drivers/net/dsa/microchip/ksz_common.c @@ -40,6 +40,82 @@ void ksz_update_port_member(struct ksz_device *dev, int port) } EXPORT_SYMBOL_GPL(ksz_update_port_member); +static void port_r_cnt(struct ksz_device *dev, int port) +{ + struct ksz_port_mib *mib = &dev->ports[port].mib; + u64 *dropped; + + /* Some ports may not have MIB counters before SWITCH_COUNTER_NUM. */ + while (mib->cnt_ptr < dev->reg_mib_cnt) { + dev->dev_ops->r_mib_cnt(dev, port, mib->cnt_ptr, + &mib->counters[mib->cnt_ptr]); + ++mib->cnt_ptr; + } + + /* last one in storage */ + dropped = &mib->counters[dev->mib_cnt]; + + /* Some ports may not have MIB counters after SWITCH_COUNTER_NUM. */ + while (mib->cnt_ptr < dev->mib_cnt) { + dev->dev_ops->r_mib_pkt(dev, port, mib->cnt_ptr, + dropped, &mib->counters[mib->cnt_ptr]); + ++mib->cnt_ptr; + } + mib->cnt_ptr = 0; +} + +static void ksz_mib_read_work(struct work_struct *work) +{ + struct ksz_device *dev = + container_of(work, struct ksz_device, mib_read); + struct ksz_port *p; + struct ksz_port_mib *mib; + int i; + + for (i = 0; i < dev->mib_port_cnt; i++) { + p = &dev->ports[i]; + if (!p->on) + continue; + mib = &p->mib; + mutex_lock(&mib->cnt_mutex); + + /* read only dropped counters when link is not up */ + if (p->link_just_down) + p->link_just_down = 0; + else if (!p->phydev.link) + mib->cnt_ptr = dev->reg_mib_cnt; + port_r_cnt(dev, i); + mutex_unlock(&mib->cnt_mutex); + } +} + +static void mib_monitor(struct timer_list *t) +{ + struct ksz_device *dev = from_timer(dev, t, mib_read_timer); + + mod_timer(&dev->mib_read_timer, jiffies + dev->mib_read_interval); + schedule_work(&dev->mib_read); +} + +void ksz_init_mib_timer(struct ksz_device *dev) +{ + int i; + + /* Read MIB counters every 30 seconds to avoid overflow. */ + dev->mib_read_interval = msecs_to_jiffies(30000); + + INIT_WORK(&dev->mib_read, ksz_mib_read_work); + timer_setup(&dev->mib_read_timer, mib_monitor, 0); + + for (i = 0; i < dev->mib_port_cnt; i++) + dev->dev_ops->port_init_cnt(dev, i); + + /* Start the timer 2 seconds later. */ + dev->mib_read_timer.expires = jiffies + msecs_to_jiffies(2000); + add_timer(&dev->mib_read_timer); +} +EXPORT_SYMBOL_GPL(ksz_init_mib_timer); + int ksz_phy_read16(struct dsa_switch *ds, int addr, int reg) { struct ksz_device *dev = ds->priv; @@ -88,6 +164,20 @@ int ksz_sset_count(struct dsa_switch *ds, int port, int sset) } EXPORT_SYMBOL_GPL(ksz_sset_count); +void ksz_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *buf) +{ + struct ksz_device *dev = ds->priv; + struct ksz_port_mib *mib; + + mib = &dev->ports[port].mib; + + mutex_lock(&mib->cnt_mutex); + port_r_cnt(dev, port); + memcpy(buf, mib->counters, dev->mib_cnt * sizeof(u64)); + mutex_unlock(&mib->cnt_mutex); +} +EXPORT_SYMBOL_GPL(ksz_get_ethtool_stats); + int ksz_port_bridge_join(struct dsa_switch *ds, int port, struct net_device *br) { @@ -355,6 +445,12 @@ int ksz_switch_register(struct ksz_device *dev, void ksz_switch_remove(struct ksz_device *dev) { + /* timer started */ + if (dev->mib_read_timer.expires) { + del_timer_sync(&dev->mib_read_timer); + flush_work(&dev->mib_read); + } + dev->dev_ops->exit(dev); dsa_unregister_switch(dev->ds); diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h index 6d49319..f5f5fd8 100644 --- a/drivers/net/dsa/microchip/ksz_common.h +++ b/drivers/net/dsa/microchip/ksz_common.h @@ -8,6 +8,7 @@ #define __KSZ_COMMON_H void ksz_update_port_member(struct ksz_device *dev, int port); +void ksz_init_mib_timer(struct ksz_device *dev); /* Common DSA access functions */ @@ -16,6 +17,7 @@ void ksz_adjust_link(struct dsa_switch *ds, int port, struct phy_device *phydev); int ksz_sset_count(struct dsa_switch *ds, int port, int sset); +void ksz_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *buf); int ksz_port_bridge_join(struct dsa_switch *ds, int port, struct net_device *br); void ksz_port_bridge_leave(struct dsa_switch *ds, int port, diff --git a/drivers/net/dsa/microchip/ksz_priv.h b/drivers/net/dsa/microchip/ksz_priv.h index 0fdc58b..a35bb34 100644 --- a/drivers/net/dsa/microchip/ksz_priv.h +++ b/drivers/net/dsa/microchip/ksz_priv.h @@ -14,8 +14,6 @@ #include <linux/etherdevice.h> #include <net/dsa.h> -#include "ksz9477_reg.h" - struct ksz_io_ops; struct vlan_table { @@ -23,6 +21,7 @@ struct vlan_table { }; struct ksz_port_mib { + struct mutex cnt_mutex; /* structure access */ u8 cnt_ptr; u64 *counters; }; @@ -39,6 +38,7 @@ struct ksz_port { u32 sgmii:1; /* port is SGMII */ u32 force:1; u32 link_just_down:1; /* link just goes down */ + u32 freeze:1; /* MIB counter freeze is enabled */ struct ksz_port_mib mib; }; @@ -79,8 +79,6 @@ struct ksz_device { struct vlan_table *vlan_cache; - u64 mib_value[TOTAL_SWITCH_COUNTER_NUM]; - u8 *txbuf; struct ksz_port *ports; @@ -153,6 +151,7 @@ struct ksz_dev_ops { u64 *cnt); void (*r_mib_pkt)(struct ksz_device *dev, int port, u16 addr, u64 *dropped, u64 *cnt); + void (*freeze_mib)(struct ksz_device *dev, int port, bool freeze); void (*port_init_cnt)(struct ksz_device *dev, int port); int (*shutdown)(struct ksz_device *dev); int (*detect)(struct ksz_device *dev);