Thread (26 messages) 26 messages, 4 authors, 2022-03-22

Re: [PATCH v9 net-next 06/11] net: dsa: microchip: add DSA support for microchip lan937x

From: Andrew Lunn <andrew@lunn.ch>
Date: 2022-03-20 01:17:50
Also in: linux-devicetree, lkml

+int lan937x_reset_switch(struct ksz_device *dev)
+{
+	u32 data32;
+	u8 data8;
+	int ret;
+
+	/* reset switch */
+	ret = lan937x_cfg(dev, REG_SW_OPERATION, SW_RESET, true);
+	if (ret < 0)
+		return ret;
+
+	ret = ksz_read8(dev, REG_SW_LUE_CTRL_1, &data8);
+	if (ret < 0)
+		return ret;
+
+	/* Enable Auto Aging */
+	ret = ksz_write8(dev, REG_SW_LUE_CTRL_1, data8 | SW_LINK_AUTO_AGING);
+	if (ret < 0)
+		return ret;
+
+	/* disable interrupts */
+	ret = ksz_write32(dev, REG_SW_INT_MASK__4, SWITCH_INT_MASK);
+	if (ret < 0)
+		return ret;
+
+	ret = ksz_write32(dev, REG_SW_PORT_INT_MASK__4, 0xFF);
+	if (ret < 0)
+		return ret;
+
+	return ksz_read32(dev, REG_SW_PORT_INT_STATUS__4, &data32);
I would probably enable auto aging in the setup routing, not the
reset.  Disabling interrupts is less clear, maybe it also belongs in
setup?
+static void lan937x_switch_exit(struct ksz_device *dev)
+{
+	lan937x_reset_switch(dev);
+}
Humm. Does a reset leave the switch in a dumb mode, or is it totally
disabled?
+int lan937x_internal_phy_write(struct ksz_device *dev, int addr, int reg,
+			       u16 val)
+{
+	u16 temp, addr_base;
+	unsigned int value;
+	int ret;
+
+	/* Check for internal phy port */
+	if (!lan937x_is_internal_phy_port(dev, addr))
+		return -EOPNOTSUPP;
+
+	if (lan937x_is_internal_base_tx_phy_port(dev, addr))
+		addr_base = REG_PORT_TX_PHY_CTRL_BASE;
+	else
+		addr_base = REG_PORT_T1_PHY_CTRL_BASE;
+
+	temp = PORT_CTRL_ADDR(addr, (addr_base + (reg << 2)));
+
+	ret = ksz_write16(dev, REG_VPHY_IND_ADDR__2, temp);
+	if (ret < 0)
+		return ret;
...
+}
+
+int lan937x_internal_phy_read(struct ksz_device *dev, int addr, int reg,
+			      u16 *val)
+{
+	u16 temp, addr_base;
+	unsigned int value;
+	int ret;
+
+	/* Check for internal phy port, return 0xffff for non-existent phy */
+	if (!lan937x_is_internal_phy_port(dev, addr))
+		return 0xffff;
+
+	if (lan937x_is_internal_base_tx_phy_port(dev, addr))
+		addr_base = REG_PORT_TX_PHY_CTRL_BASE;
+	else
+		addr_base = REG_PORT_T1_PHY_CTRL_BASE;
+
+	/* get register address based on the logical port */
+	temp = PORT_CTRL_ADDR(addr, (addr_base + (reg << 2)));
+
+	ret = ksz_write16(dev, REG_VPHY_IND_ADDR__2, temp);
+	if (ret < 0)
+		return ret;
Looks pretty similar to me. You should pull this out into a helper.

+void lan937x_port_setup(struct ksz_device *dev, int port, bool cpu_port)
+{
+	struct dsa_switch *ds = dev->ds;
+	u8 member;
+
+	/* enable tag tail for host port */
+	if (cpu_port) {
+		lan937x_port_cfg(dev, port, REG_PORT_CTRL_0,
+				 PORT_TAIL_TAG_ENABLE, true);
+	}
Checkpatch probably warns here that the {} are not needed.
+
+	/* disable frame check length field */
+	lan937x_port_cfg(dev, port, REG_PORT_MAC_CTRL_0, PORT_FR_CHK_LENGTH,
+			 false);
+
+	/* set back pressure for half duplex */
+	lan937x_port_cfg(dev, port, REG_PORT_MAC_CTRL_1, PORT_BACK_PRESSURE,
+			 true);
+
+	/* enable 802.1p priority */
+	lan937x_port_cfg(dev, port, P_PRIO_CTRL, PORT_802_1P_PRIO_ENABLE, true);
+
+	if (!lan937x_is_internal_phy_port(dev, port)) {
+		/* enable flow control */
+		lan937x_port_cfg(dev, port, REG_PORT_XMII_CTRL_0,
+				 PORT_TX_FLOW_CTRL | PORT_RX_FLOW_CTRL,
+				 true);
+	}
Here as well.
+struct lan_alu_struct {
+	/* entry 1 */
+	u8	is_static:1;
+	u8	is_src_filter:1;
+	u8	is_dst_filter:1;
+	u8	prio_age:3;
+	u32	_reserv_0_1:23;
+	u8	mstp:3;
I assume this works O.K, but bitfields are nornaly defined using one
type. I would of used u32 for them all. Is there some advantage of
missing u8 and u32?
+	/* entry 2 */
+	u8	is_override:1;
+	u8	is_use_fid:1;
+	u32	_reserv_1_1:22;
+	u8	port_forward:8;
+	/* entry 3 & 4*/
+	u32	_reserv_2_1:9;
+	u8	fid:7;
+	u8	mac[ETH_ALEN];
+};
  Andrew
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help