Re: [PATCH net-next v12 7/7] net: stmmac: qcom-ethqos: add support for sa8255p
| Newsgroups | org.infradead.lists.linux-amlogic,dev.linux.lists.imx,dev.linux.lists.linux-sunxi,org.kernel.vger.linux-devicetree |
|---|---|
| Message-ID | <[email protected]> |
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Missing rejection of unsupported RGMII PHY modes in ethqos_scmi_setup() -- commit c343cb8d359e16483f354f89b27f2cd9e3505850 Author: Bartosz Golaszewski <[email protected]> net: stmmac: qcom-ethqos: add support for sa8255p This commit extends the qcom-ethqos driver to support the sa8255p model, which handles power management in firmware over SCMI. It adds a new emac data variant and a separate setup callback. > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c > index f379570f80680..47175670a3263 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c [ ... ] > @@ -749,6 +775,62 @@ static int ethqos_hlos_setup(struct qcom_ethqos *ethqos, > return 0; > } > > +static const char *const ethqos_scmi_pd_names[] = { "core", "mdio" }; > + > +static int ethqos_scmi_setup(struct qcom_ethqos *ethqos, > + struct plat_stmmacenet_data *plat_dat) > +{ [ ... ] > + switch (ethqos->phy_mode) { > + case PHY_INTERFACE_MODE_2500BASEX: > + case PHY_INTERFACE_MODE_SGMII: > + plat_dat->fix_mac_speed = ethqos_fix_mac_speed_sgmii; > + break; > + default: > + break; [Severity: Medium] Will this silently ignore RGMII and other unsupported PHY modes without returning an error? If a misconfigured device tree specifies an unsupported mode like RGMII for an SCMI-based variant, qcom_ethqos_probe() allows it globally, but this setup function in dwmac-qcom-ethqos.c will silently fall through the default case without configuring plat_dat->fix_mac_speed or returning -EINVAL. Could this leave the interface in a broken state that is difficult to debug, rather than failing probe explicitly? > + } > + > + return 0; > +} -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7 _______________________________________________ linux-amlogic mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-amlogic