From: Andrew Lunn <andrew@lunn•ch>
To: Tristram.Ha@microchip•com
Cc: Sergio Paracuellos <sergio.paracuellos@gmail•com>,
Florian Fainelli <f.fainelli@gmail•com>,
Pavel Machek <pavel@ucw•cz>,
UNGLinuxDriver@microchip•com, netdev@vger•kernel.org
Subject: Re: [PATCH v1 net-next 2/4] net: dsa: microchip: add MIB counter reading support
Date: Sat, 9 Feb 2019 18:22:49 +0100 [thread overview]
Message-ID: <20190209172249.GG30856@lunn.ch> (raw)
In-Reply-To: <1549598829-25970-3-git-send-email-Tristram.Ha@microchip.com>
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);
> + }
> +}
next prev parent reply other threads:[~2019-02-09 17:22 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-08 4:07 [PATCH v1 net-next 0/4] net: dsa: microchip: add MIB counters support Tristram.Ha
2019-02-08 4:07 ` [PATCH v1 net-next 1/4] net: dsa: microchip: prepare PHY for proper advertisement Tristram.Ha
2019-02-09 16:54 ` Andrew Lunn
2019-02-08 4:07 ` [PATCH v1 net-next 2/4] net: dsa: microchip: add MIB counter reading support Tristram.Ha
2019-02-09 17:22 ` Andrew Lunn [this message]
2019-02-13 2:39 ` Tristram.Ha
2019-02-13 3:28 ` Andrew Lunn
2019-02-13 3:51 ` Florian Fainelli
2019-02-14 19:26 ` Tristram.Ha
2019-02-14 19:33 ` Florian Fainelli
2019-02-20 23:49 ` Joe Perches
2019-02-25 18:34 ` Pavel Machek
2019-02-08 4:07 ` [PATCH v1 net-next 3/4] net: dsa: microchip: use readx_poll_time for polling Tristram.Ha
2019-02-09 17:01 ` Andrew Lunn
2019-02-13 2:53 ` Tristram.Ha
2019-02-08 4:07 ` [PATCH v1 net-next 4/4] net: dsa: microchip: remove unnecessary include headers Tristram.Ha
2019-02-09 17:01 ` Andrew Lunn
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190209172249.GG30856@lunn.ch \
--to=andrew@lunn$(echo .)ch \
--cc=Tristram.Ha@microchip$(echo .)com \
--cc=UNGLinuxDriver@microchip$(echo .)com \
--cc=f.fainelli@gmail$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=pavel@ucw$(echo .)cz \
--cc=sergio.paracuellos@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox