> +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
