Re: [PATCH net-next v6 6/6] net: dsa: motorcomm: Add support for Motorcomm YT922x

[David Yang]

On Tue, Sep 8, 2026 at 4:36 PM Kyle Switch <kyle.switch@xxxxxxxxxxxxxx> wrote:

>  config NET_DSA_YT921X
>         tristate "Motorcomm YT9215 ethernet switch chip support"
>         select NET_DSA_TAG_YT921X
> +       select NET_DSA_TAG_YT922X
>         select NET_IEEE8021Q_HELPERS if DCB
>         help
>           This enables support for the Motorcomm YT9215 ethernet switch

Not sure, but might be better with NET_DSA_TAG_YT921X ||
NET_DSA_TAG_YT922X if that does not cause an error.

> +/**
> + * Initialize serdes configuration based on interface mode.
> + */
> +static int yt922x_sds_init(struct yt921x_priv *priv, int port,
> +                          phy_interface_t interface)
> +{
> +       int addr;
> +       u16 data;
> +       int res;
> +
> +       addr = yt922x_sds_phyaddr_get
> +               (port, YT922X_PHY_REG_TYPE_SDS_COMMON_EXT,
> +                YT922X_PHY_REG_SPACE_SGMII);
> +       if (addr < 0)
> +               return -EINVAL;
> +       /* write protect */
> +       res = yt921x_intif_ext_write(priv, addr, 0x4be, 0xd);
> +       if (res)
> +               return res;
> +       /* CDR */
> +       if (interface == PHY_INTERFACE_MODE_100BASEX) {
> +               res = yt921x_intif_ext_write(priv, addr, 0x406, 0x0);
> +               if (res)
> +                       return res;
> +               res = yt921x_intif_ext_write(priv, addr, 0x416, 0x3458);
> +               if (res)
> +                       return res;
> +       } else {
> +               res = yt921x_intif_ext_write(priv, addr, 0x406, 0x800);
> +               if (res)
> +                       return res;
> +               res = yt921x_intif_ext_write(priv, addr, 0x416, 0x4558);
> +               if (res)
> +                       return res;
> +       }
> +       /* PLL */
> +       if (interface == PHY_INTERFACE_MODE_USXGMII) {
> +               res = yt921x_intif_ext_write(priv, addr, 0x43a, 0x1006);
> +               if (res)
> +                       return res;
> +               res = yt921x_intif_ext_write(priv, addr, 0x43f, 0x3029);
> +               if (res)
> +                       return res;
> +               res = yt921x_intif_ext_write(priv, addr, 0x42a, 0xf070);
> +               if (res)
> +                       return res;
> +       } else {
> +               res = yt921x_intif_ext_write(priv, addr, 0x43d, 0x207d);
> +               if (res)
> +                       return res;
> +               res = yt921x_intif_ext_write(priv, addr, 0x43c, 0x207d);
> +               if (res)
> +                       return res;
> +               res = yt921x_intif_ext_write(priv, addr, 0x43f, 0x3032);
> +               if (res)
> +                       return res;
> +               res = yt921x_intif_ext_write(priv, addr, 0x43a, 0x6);
> +               if (res)
> +                       return res;
> +               res = yt921x_intif_ext_write(priv, addr, 0x42a, 0xf070);
> +               if (res)
> +                       return res;
> +       }
> +       /* VCO */
> +       res = yt921x_intif_ext_write(priv, addr, 0x439, 0xC0);
> +       if (res)
> +               return res;
> +       /* Vdac */
> +       res = yt921x_intif_ext_write(priv, addr, 0x492, 0x7f7f);
> +       if (res)
> +               return res;
> +       res = yt921x_intif_ext_write(priv, addr, 0x491, 0x7f);
> +       if (res)
> +               return res;
> +       /* Eye */
> +       res = yt921x_intif_ext_write(priv, addr, 0x454, 0xf14);
> +       if (res)
> +               return res;
> +       res = yt921x_intif_ext_write(priv, addr, 0x497, 0xa44);
> +       if (res)
> +               return res;
> +       res = yt921x_intif_ext_write(priv, addr, 0x4cd, 0x0);
> +       if (res)
> +               return res;
> +
> +       res = yt921x_intif_ext_write(priv, addr, 0x4af, 0x45e3);
> +       if (res)
> +               return res;
> +       res = yt921x_intif_ext_write(priv, addr, 0x48a, 0xfff);
> +       if (res)
> +               return res;
> +       res = yt921x_intif_ext_write(priv, addr, 0x408, 0x7c00);
> +       if (res)
> +               return res;
> +       res = yt921x_intif_ext_write(priv, addr, 0x4d6, 0x7f);
> +       if (res)
> +               return res;
> +       res = yt921x_intif_ext_write(priv, addr, 0x44f, 0xff08);
> +       if (res)
> +               return res;
> +       /* FFE */
> +       res = yt921x_intif_ext_write(priv, addr, 0x48e, 0x7d00);
> +       if (res)
> +               return res;
> +       res = yt921x_intif_ext_write(priv, addr, 0xd, 0x60f);
> +       if (res)
> +               return res;
> +       /* CTLE */
> +       res = yt921x_intif_ext_write(priv, addr, 0x4b0, 0x804);
> +       if (res)
> +               return res;
> +       res = yt921x_intif_ext_write(priv, addr, 0x4b1, 0x7774);
> +       if (res)
> +               return res;
> +       res = yt921x_intif_ext_write(priv, addr, 0x4af, 0x45e7);
> +       if (res)
> +               return res;
> +       res = yt921x_intif_ext_write(priv, addr, 0x3, 0x5603);
> +       if (res)
> +               return res;
> +
> +       msleep(20);
> +       res = yt921x_intif_ext_write(priv, addr, 0x492, 0x7fff);
> +       if (res)
> +               return res;
> +       res = yt921x_intif_ext_write(priv, addr, 0x492, 0x7f7f);
> +       if (res)
> +               return res;
> +       /* CTLE */
> +       res = yt921x_intif_ext_write(priv, addr, 0x2000, 0x40);
> +       if (res)
> +               return res;
> +       res = yt921x_intif_ext_write(priv, addr, 0x2000, 0x0);
> +       if (res)
> +               return res;
> +
> +       if (interface == PHY_INTERFACE_MODE_SGMII) {
> +               res = yt921x_intif_ext_write(priv, addr, 0x1042, 0x48c);
> +               if (res)
> +                       return res;
> +       }
> +       /* soft reset */
> +       addr = yt922x_sds_phyaddr_get(port, YT922X_PHY_REG_TYPE_MII,
> +                                     YT922X_PHY_REG_SPACE_SGMII);
> +       if (addr < 0)
> +               return res;
> +       res = yt921x_intif_read(priv, addr, 0x0, &data);
> +       if (res)
> +               return res;
> +       data &= ~(1 << 15);
> +       res = yt921x_intif_write(priv, addr, 0x0, data);
> +       if (res)
> +               return res;
> +       addr = yt922x_sds_phyaddr_get(port, YT922X_PHY_REG_TYPE_MII,
> +                                     YT922X_PHY_REG_SPACE_USXGMII);
> +       if (addr < 0)
> +               return res;
> +       res = yt921x_intif_read(priv, addr, 0x0, &data);
> +       if (res)
> +               return res;
> +       data |= 1 << 15;
> +       res = yt921x_intif_write(priv, addr, 0x0, data);
> +       if (res)
> +               return res;
> +
> +       return 0;
> +}

No one else, except you, can maintain this stuff. Drop it for now and
discuss it later.

> +static void
> +yt922x_dsa_phylink_get_caps(struct dsa_switch *ds, int port,
> +                           struct phylink_config *config)
> +{
> +       struct yt921x_priv *priv = to_yt921x_priv(ds);
> +       const struct yt921x_info *info = priv->info;
> +
> +       config->mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
> +                                  MAC_10 | MAC_100 | MAC_1000;
> +
> +       if (info->internal_mask & BIT(port)) {
> +               if (port >= 4 && port <= 7) {
> +                       /* port 4 to port 7, internal utp */
> +                       __set_bit(PHY_INTERFACE_MODE_INTERNAL,
> +                                 config->supported_interfaces);
> +                       config->mac_capabilities |= MAC_2500FD;
> +               } else {
> +                       __set_bit(PHY_INTERFACE_MODE_SGMII,
> +                                 config->supported_interfaces);
> +                       __set_bit(PHY_INTERFACE_MODE_100BASEX,
> +                                 config->supported_interfaces);
> +                       __set_bit(PHY_INTERFACE_MODE_1000BASEX,
> +                                 config->supported_interfaces);
> +                       __set_bit(PHY_INTERFACE_MODE_2500BASEX,
> +                                 config->supported_interfaces);
> +                       config->mac_capabilities |= MAC_2500FD;
> +                       __set_bit(PHY_INTERFACE_MODE_USXGMII,
> +                                 config->supported_interfaces);

By definition internal_mask is PHY_INTERFACE_MODE_INTERNAL only. Move
other modes into external_mask.

> +                       config->mac_capabilities |= MAC_2500FD;

Duplicated config->mac_capabilities |= MAC_2500FD;

> +static int yt922x_chip_detect(struct yt921x_priv *priv)
> +{
> +       struct device *dev = to_device(priv);
> +       const struct yt921x_info *info;
> +       u32 chipid;
> +       u32 major;
> +       int res;
> +
> +       res = yt921x_reg_read(priv, YT921X_CHIP_ID, &chipid);
> +       if (res)
> +               return res;
> +       major = FIELD_GET(YT921X_CHIP_ID_MAJOR, chipid);
> +       for (info = yt921x_infos; info->name; info++)
> +               if (info->major == major)
> +                       break;
> +       if (!info->name) {
> +               dev_err(dev, "Unexpected chipid 0x%x\n", chipid);
> +               return -ENODEV;
> +       }
> +       priv->info = info;
> +
> +       return 0;
> +}
> +
> +static int yt922x_chip_reset(struct yt921x_priv *priv)
> +{
> +       struct device *dev = to_device(priv);
> +       u16 eth_p_tag;
> +       u32 val;
> +       int res;
> +
> +       res = yt922x_chip_detect(priv);
> +       if (res)
> +               return res;
> +
> +       /* Reset */
> +       res = yt921x_reg_write(priv, YT921X_RST, YT921X_RST_HW);
> +       if (res)
> +               return res;
> +
> +       fsleep(YT921X_RST_DELAY_US);
> +
> +       val = 0;
> +       res = yt921x_reg_wait(priv, YT921X_RST, ~0, &val);
> +       if (res)
> +               return res;
> +
> +       /* TPID check */
> +       res = yt921x_reg_read(priv, YT921X_CPU_TAG_TPID, &val);
> +       if (res)
> +               return res;
> +       eth_p_tag = FIELD_GET(YT921X_CPU_TAG_TPID_TPID_M, val);
> +       if (eth_p_tag != ETH_P_YT921X) {
> +               dev_err(dev, "Tag type 0x%x != 0x%x\n", eth_p_tag,
> +                       ETH_P_YT921X);
> +               return -EINVAL;
> +       }
> +
> +       return 0;
> +}

Isn't it just yt921x_chip_reset? Please reuse existing functions.

> +static int yt922x_cpu_port_set(struct yt921x_priv *priv)
> +{
> +       struct dsa_switch *ds = &priv->ds;
> +       u32 ctrl;
> +       int res;
> +
> +       /* cpu tag mode */
> +       res = yt922x_cpu_tag_mode_set_8b(priv);
> +       if (res)
> +               return res;
> +
> +       /* Enable DSA */
> +       priv->cpu_ports_mask = dsa_cpu_ports(ds);
> +       ctrl = YT921X_EXT_CPU_PORT_TAG_EN | YT921X_EXT_CPU_PORT_PORT_EN |
> +              YT921X_EXT_CPU_PORT_PORT(__ffs(priv->cpu_ports_mask));
> +       res = yt921x_reg_write(priv, YT921X_EXT_CPU_PORT, ctrl);
> +       if (res)
> +               return res;
> +
> +       /* Setup software switch */
> +       ctrl = YT922X_CPU_COPY_TO_EXT_CPU;
> +       res = yt921x_reg_write(priv, YT922X_CPU_COPY, ctrl);
> +       if (res)
> +               return res;
> +
> +       return res;
> +}

Move this into chip_setup_dsa() to align with existing yt921x functions.

> @@ -1007,6 +1108,8 @@ struct yt921x_priv {
>
>         u8 acl_masks[YT921X_ACL_BLK_NUM];
>         struct yt921x_acl_blk *acl_blks[YT921X_ACL_BLK_NUM];
> +       struct yt922x_pcs pcs_port_0;
> +       struct yt922x_pcs pcs_port_8;
>  };

More reasonable if pre-port "struct phylink_pcs yt921x_port::pcs".