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); > +}