netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Mattias Forsblad <mattias.forsblad@gmail.com>
Cc: Vladimir Oltean <olteanv@gmail.com>,
	netdev@vger.kernel.org, Vivien Didelot <vivien.didelot@gmail.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	"David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	linux@armlinux.org.uk, ansuelsmth@gmail.com
Subject: Re: [PATCH net-next v14 5/7] net: dsa: mv88e6xxx: rmu: Add functionality to get RMON
Date: Wed, 21 Sep 2022 17:50:15 +0200	[thread overview]
Message-ID: <Yysyt3e6iBN2qKRI@lunn.ch> (raw)
In-Reply-To: <5d50db8c-5504-f776-521b-eaae4d900e90@gmail.com>

> I understand want you want but I can see a lot of risks and pitfalls with moving
> ordinary read and writes to RMU, which I wanted to avoid by first doing
> RMON dump and then dump ATU and at a later stage with a better architecture
> for write/read combining doing that, instead of forcing through read/writes
> with all associated testing it would require. Can we please do this in
> steps?

If we are going to fall back to MDIO when RMU fails, we need a
different code structure for these operations. That different code
structure should also help solve the messy _ops structure stuff.

RMU affects us in two different locations:

ATU and MIB dump: Controlled by struct mv88e6xxx_ops

register read/write: Controlled by struct mv88e6xxx_bus_ops

We could add to struct mv88e6xxx_ops:

        int (*stats_rmu_get_sset_count)(struct mv88e6xxx_chip *chip);
        int (*stats_rmu_get_strings)(struct mv88e6xxx_chip *chip,  uint8_t *data);
        int (*stats_rmu_get_stats)(struct mv88e6xxx_chip *chip,  int port,
                                   uint64_t *data);

and then mv88e6xxx_get_stats() would become something like:

static void mv88e6xxx_get_stats(struct mv88e6xxx_chip *chip, int port,
                                uint64_t *data)
{
        int count = 0;
	int err;

	if (chip->info->ops->stats_rmu_get_stats && mv88e6xxx_rmu_enabled(chip)) {
		err = chip->info->ops->stats_rmu_get_stats(chip, port, data)
		if (!err)
			return;
	}
			
        if (chip->info->ops->stats_get_stats)
                count = chip->info->ops->stats_get_stats(chip, port, data);

We then get fall back to MDIO, and clean, separate implementations of
RMU and MDIO stats operations.

I hope the ATU/fdb dump can be done in a similar way.

register read/writes, we probably need to extend mv88e6xxx_smi_read()
and mv88e6xxx_smi_write(). Try RMU first, and then fall back to MDIO.

Please try something in this direction. But please, lots of small,
simple patches with good commit messages.

       Andrew



  reply	other threads:[~2022-09-21 15:57 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-19 11:08 [PATCH net-next v14 0/7] net: dsa: qca8k, mv88e6xxx: rmon: Add RMU support Mattias Forsblad
2022-09-19 11:08 ` [PATCH net-next v14 1/7] net: dsa: mv88e6xxx: Add RMU enable for select switches Mattias Forsblad
2022-09-19 11:08 ` [PATCH net-next v14 2/7] net: dsa: Add convenience functions for frame handling Mattias Forsblad
2022-09-19 22:14   ` Vladimir Oltean
2022-09-19 22:22     ` Andrew Lunn
2022-09-19 22:18   ` [PATCH rfc v0 0/9] DSA: Move parts of inband signalling into the DSA Andrew Lunn
2022-09-19 22:18     ` [PATCH rfc v0 1/9] net: dsa: qca8k: Fix inconsistent use of jiffies vs milliseconds Andrew Lunn
2022-09-19 22:18     ` [PATCH rfc v0 2/9] net: dsa: qca8k: Move completion into DSA core Andrew Lunn
2022-09-20 14:43       ` Vladimir Oltean
2022-09-21  0:19         ` Andrew Lunn
2022-09-21  0:22           ` Vladimir Oltean
2022-09-19 22:18     ` [PATCH rfc v0 3/9] net: dsa: qca8K: Move queuing for request frame into the core Andrew Lunn
2022-09-19 22:18     ` [PATCH rfc v0 4/9] net: dsa: qca8k: dsa_inband_request: More normal return values Andrew Lunn
2022-09-19 23:02       ` Vladimir Oltean
2022-09-19 23:21         ` Andrew Lunn
2022-09-19 23:16       ` Vladimir Oltean
2022-09-19 22:18     ` [PATCH rfc v0 5/9] net: dsa: qca8k: Move request sequence number handling into core Andrew Lunn
2022-09-19 22:18     ` [PATCH rfc v0 6/9] net: dsa: qca8k: Refactor sequence number mismatch to use error code Andrew Lunn
2022-09-19 23:30       ` Vladimir Oltean
2022-09-20  0:05         ` Andrew Lunn
2022-09-19 22:18     ` [PATCH rfc v0 7/9] net: dsa: qca8k: Pass error code from reply decoder to requester Andrew Lunn
2022-09-19 22:18     ` [PATCH rfc v0 8/9] net: dsa: qca8k: Pass response buffer via dsa_rmu_request Andrew Lunn
2022-09-20  0:27       ` Vladimir Oltean
2022-09-20 12:33         ` Andrew Lunn
2022-09-19 22:18     ` [PATCH rfc v0 9/9] net: dsa: qca8k: Move inband mutex into DSA core Andrew Lunn
2022-09-20  3:19       ` Christian Marangi
2022-09-20 15:48         ` Andrew Lunn
2022-09-19 11:08 ` [PATCH net-next v14 3/7] net: dsa: Introduce dsa tagger data operation Mattias Forsblad
2022-09-19 22:00   ` Vladimir Oltean
2022-09-20  6:41     ` Mattias Forsblad
2022-09-20 10:31       ` Vladimir Oltean
2022-09-20 11:10         ` Mattias Forsblad
2022-09-19 11:08 ` [PATCH net-next v14 4/7] net: dsa: mv88e6xxxx: Add RMU functionality Mattias Forsblad
2022-09-19 22:39   ` Vladimir Oltean
2022-09-20 11:53     ` Mattias Forsblad
2022-09-20 12:22       ` Vladimir Oltean
2022-09-19 11:08 ` [PATCH net-next v14 5/7] net: dsa: mv88e6xxx: rmu: Add functionality to get RMON Mattias Forsblad
2022-09-19 22:49   ` Vladimir Oltean
2022-09-20 12:26     ` Mattias Forsblad
2022-09-20 13:10       ` Vladimir Oltean
2022-09-20 13:40         ` Mattias Forsblad
2022-09-20 21:04         ` Andrew Lunn
2022-09-21  5:35           ` Mattias Forsblad
2022-09-21 15:50             ` Andrew Lunn [this message]
2022-09-22 11:48           ` Vladimir Oltean
2022-09-22 12:45             ` Andrew Lunn
2022-09-22 13:04               ` Vladimir Oltean
2022-09-22 17:27                 ` Andrew Lunn
2022-09-19 11:08 ` [PATCH net-next v14 6/7] net: dsa: mv88e6xxx: rmon: Use RMU for reading RMON data Mattias Forsblad
2022-09-19 11:08 ` [PATCH net-next v14 7/7] net: dsa: qca8k: Use new convenience functions Mattias Forsblad
2022-09-19 11:23   ` Christian Marangi

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=Yysyt3e6iBN2qKRI@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=ansuelsmth@gmail.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=f.fainelli@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=mattias.forsblad@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=pabeni@redhat.com \
    --cc=vivien.didelot@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).