[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220408232557.b62l3lksotq5vuvm@skbuf>
Date: Sat, 9 Apr 2022 02:25:57 +0300
From: Vladimir Oltean <olteanv@...il.com>
To: Prasanna Vengateshan <prasanna.vengateshan@...rochip.com>
Cc: andrew@...n.ch, netdev@...r.kernel.org, robh+dt@...nel.org,
UNGLinuxDriver@...rochip.com, woojung.huh@...rochip.com,
hkallweit1@...il.com, linux@...linux.org.uk, davem@...emloft.net,
kuba@...nel.org, linux-kernel@...r.kernel.org,
vivien.didelot@...il.com, f.fainelli@...il.com,
devicetree@...r.kernel.org, pabeni@...hat.com
Subject: Re: [RFC PATCH v11 net-next 05/10] net: dsa: microchip: add DSA
support for microchip lan937x
On Fri, Mar 25, 2022 at 10:23:36PM +0530, Prasanna Vengateshan wrote:
> +static void lan937x_r_mib_stats64(struct ksz_device *dev, int port)
> +{
> + struct ksz_port_mib *mib = &dev->ports[port].mib;
> + struct rtnl_link_stats64 *s;
> + u64 *ctr = mib->counters;
> +
> + s = &mib->stats64;
> + spin_lock(&mib->stats64_lock);
I haven't looked at further patches yet to see if the situation improves
or not, but right now, this spin lock is useless, as you do not
implement .get_stats64.
> +
> + s->rx_packets = ctr[lan937x_mib_rx_mcast] +
> + ctr[lan937x_mib_rx_bcast] +
> + ctr[lan937x_mib_rx_ucast] +
> + ctr[lan937x_mib_rx_pause];
> +
> + s->tx_packets = ctr[lan937x_mib_tx_mcast] +
> + ctr[lan937x_mib_tx_bcast] +
> + ctr[lan937x_mib_tx_ucast] +
> + ctr[lan937x_mib_tx_pause];
> +
> + s->rx_bytes = ctr[lan937x_mib_rx_total];
> + s->tx_bytes = ctr[lan937x_mib_tx_total];
> +
> + s->rx_errors = ctr[lan937x_mib_rx_fragments] +
> + ctr[lan937x_mib_rx_jabbers] +
> + ctr[lan937x_mib_rx_sym_err] +
> + ctr[lan937x_mib_rx_align_err] +
> + ctr[lan937x_mib_rx_crc_err];
> +
> + s->tx_errors = ctr[lan937x_mib_tx_exc_col] +
> + ctr[lan937x_mib_tx_late_col];
> +
> + s->rx_dropped = ctr[lan937x_mib_rx_discard];
> + s->tx_dropped = ctr[lan937x_mib_tx_discard];
> + s->multicast = ctr[lan937x_mib_rx_mcast];
> +
> + s->collisions = ctr[lan937x_mib_tx_late_col] +
> + ctr[lan937x_mib_tx_single_col] +
> + ctr[lan937x_mib_tx_mult_col];
> +
> + s->rx_length_errors = ctr[lan937x_mib_rx_fragments] +
> + ctr[lan937x_mib_rx_jabbers];
> +
> + s->rx_crc_errors = ctr[lan937x_mib_rx_crc_err];
> + s->rx_frame_errors = ctr[lan937x_mib_rx_align_err];
> + s->tx_aborted_errors = ctr[lan937x_mib_tx_exc_col];
> + s->tx_window_errors = ctr[lan937x_mib_tx_late_col];
> +
> + spin_unlock(&mib->stats64_lock);
> +}
> +static int lan937x_init(struct ksz_device *dev)
> +{
> + int ret;
> +
> + ret = lan937x_switch_init(dev);
> + if (ret < 0) {
> + dev_err(dev->dev, "failed to initialize the switch");
> + return ret;
> + }
> +
> + /* enable Indirect Access from SPI to the VPHY registers */
> + ret = lan937x_enable_spi_indirect_access(dev);
Do you need to call this both from lan937x_init() and from lan937x_setup()?
> + if (ret < 0) {
> + dev_err(dev->dev, "failed to enable spi indirect access");
> + return ret;
> + }
> +
> + ret = lan937x_mdio_register(dev);
> + if (ret < 0) {
> + dev_err(dev->dev, "failed to register the mdio");
> + return ret;
> + }
> +
> + return 0;
> +}
> +static void lan937x_port_stp_state_set(struct dsa_switch *ds, int port,
> + u8 state)
> +{
> + struct ksz_device *dev = ds->priv;
> + struct ksz_port *p;
> + u8 data;
> +
> + lan937x_pread8(dev, port, P_STP_CTRL, &data);
This is a copy-paste of ksz8_port_stp_state_set() except for the use of
lan937x_pread8() instead of ksz_pread8(). But ksz_pread8() should work
too, since it calls dev->dev_ops->get_port_addr(port, offset) which you
translate into PORT_CTRL_ADDR(port, offset) which is exactly what
lan937x_pread8() does.
> + data &= ~(PORT_TX_ENABLE | PORT_RX_ENABLE | PORT_LEARN_DISABLE);
> +
> + switch (state) {
> + case BR_STATE_DISABLED:
> + data |= PORT_LEARN_DISABLE;
> + break;
> + case BR_STATE_LISTENING:
> + data |= (PORT_RX_ENABLE | PORT_LEARN_DISABLE);
> + break;
> + case BR_STATE_LEARNING:
> + data |= PORT_RX_ENABLE;
> + break;
> + case BR_STATE_FORWARDING:
> + data |= (PORT_TX_ENABLE | PORT_RX_ENABLE);
> + break;
> + case BR_STATE_BLOCKING:
> + data |= PORT_LEARN_DISABLE;
> + break;
> + default:
> + dev_err(ds->dev, "invalid STP state: %d\n", state);
> + return;
> + }
> +
> + lan937x_pwrite8(dev, port, P_STP_CTRL, data);
> +
> + p = &dev->ports[port];
> + p->stp_state = state;
> +
> + ksz_update_port_member(dev, port);
> +}
Powered by blists - more mailing lists