| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aGZxc-9C0rPVMsGH@shell.armlinux.org.uk>
Date: Thu, 3 Jul 2025 13:02:59 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: weishangjuan@...incomputing.com
Cc: andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com,
kuba@...nel.org, robh@...nel.org, krzk+dt@...nel.org,
conor+dt@...nel.org, netdev@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
mcoquelin.stm32@...il.com, alexandre.torgue@...s.st.com,
yong.liang.choong@...ux.intel.com, vladimir.oltean@....com,
jszhang@...nel.org, jan.petrous@....nxp.com,
prabhakar.mahadev-lad.rj@...renesas.com, inochiama@...il.com,
boon.khai.ng@...era.com, dfustini@...storrent.com, 0x1207@...il.com,
linux-stm32@...md-mailman.stormreply.com,
linux-arm-kernel@...ts.infradead.org, ningyu@...incomputing.com,
linmin@...incomputing.com, lizhi2@...incomputing.com
Subject: Re: [PATCH v3 2/2] ethernet: eswin: Add eic7700 ethernet driver
On Thu, Jul 03, 2025 at 05:20:15PM +0800, weishangjuan@...incomputing.com wrote:
> +static void eic7700_qos_fix_speed(void *priv, int speed, u32 mode)
> +{
> + struct eic7700_qos_priv *dwc_priv = priv;
> + int i;
> +
> + switch (speed) {
> + case SPEED_1000:
> + for (i = 0; i < 3; i++)
> + regmap_write(dwc_priv->hsp_regmap,
> + dwc_priv->dly_hsp_reg[i],
> + dwc_priv->dly_param_1000m[i]);
> + break;
> + case SPEED_100:
> + for (i = 0; i < 3; i++) {
> + regmap_write(dwc_priv->hsp_regmap,
> + dwc_priv->dly_hsp_reg[i],
> + dwc_priv->dly_param_100m[i]);
> + }
The other two instances don't have the curley braces, why does this need
it?
> + break;
> + case SPEED_10:
> + for (i = 0; i < 3; i++) {
> + regmap_write(dwc_priv->hsp_regmap,
> + dwc_priv->dly_hsp_reg[i],
> + dwc_priv->dly_param_10m[i]);
> + }
> + break;
> + default:
> + dev_err(dwc_priv->dev, "invalid speed %u\n", speed);
> + break;
> + }
Overall, wouldn't:
const u32 *dly_param;
switch (speed) {
case SPEED_1000:
dly_param = dwc_priv->dly_param_1000m;
break;
... etc ...
default:
dly_param = NULL;
dev_err(dwc_priv->dev, "invalid speed %u\n", speed);
break;
}
if (dly_param)
for (i = 0; i < 3; i++)
regmap_write(dwc_priv->hsp_regmap,
dwc_priv->dly_hsp_reg[i],
dly_param[i]);
be more concise and easier to read?
> +}
> +
> +static int eic7700_dwmac_probe(struct platform_device *pdev)
> +{
> + struct plat_stmmacenet_data *plat_dat;
> + struct stmmac_resources stmmac_res;
> + struct eic7700_qos_priv *dwc_priv;
> + u32 hsp_aclk_ctrl_offset;
> + u32 hsp_aclk_ctrl_regset;
> + u32 hsp_cfg_ctrl_offset;
> + u32 eth_axi_lp_ctrl_offset;
> + u32 eth_phy_ctrl_offset;
> + u32 eth_phy_ctrl_regset;
> + bool has_rx_dly = false;
> + bool has_tx_dly = false;
> + int ret;
> +
> + ret = stmmac_get_platform_resources(pdev, &stmmac_res);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "failed to get resources\n");
> +
> + plat_dat = devm_stmmac_probe_config_dt(pdev, stmmac_res.mac);
> + if (IS_ERR(plat_dat))
> + return dev_err_probe(&pdev->dev, PTR_ERR(plat_dat),
> + "dt configuration failed\n");
> +
> + dwc_priv = devm_kzalloc(&pdev->dev, sizeof(*dwc_priv), GFP_KERNEL);
> + if (!dwc_priv)
> + return -ENOMEM;
> +
> + dwc_priv->dev = &pdev->dev;
> + dwc_priv->dly_param_1000m[0] = EIC7700_DELAY_VALUE0;
> + dwc_priv->dly_param_1000m[1] = EIC7700_DELAY_VALUE1;
> + dwc_priv->dly_param_1000m[2] = EIC7700_DELAY_VALUE0;
> + dwc_priv->dly_param_100m[0] = EIC7700_DELAY_VALUE0;
> + dwc_priv->dly_param_100m[1] = EIC7700_DELAY_VALUE1;
> + dwc_priv->dly_param_100m[2] = EIC7700_DELAY_VALUE0;
> + dwc_priv->dly_param_10m[0] = 0x0;
> + dwc_priv->dly_param_10m[1] = 0x0;
> + dwc_priv->dly_param_10m[2] = 0x0;
> +
> + ret = of_property_read_u32(pdev->dev.of_node, "rx-internal-delay-ps",
> + &dwc_priv->rx_delay_ps);
> + if (ret)
> + dev_dbg(&pdev->dev, "can't get rx-internal-delay-ps, ret(%d).", ret);
Consider using %pe and ERR_PTR(ret) so that error codes can be
translated to human readable strings. Ditto elsewhere.
> + else
> + has_rx_dly = true;
> +
> + ret = of_property_read_u32(pdev->dev.of_node, "tx-internal-delay-ps",
> + &dwc_priv->tx_delay_ps);
> + if (ret)
> + dev_dbg(&pdev->dev, "can't get tx-internal-delay-ps, ret(%d).", ret);
> + else
> + has_tx_dly = true;
> + if (has_rx_dly && has_tx_dly) {
> + eic7700_set_delay(dwc_priv->rx_delay_ps, dwc_priv->tx_delay_ps,
> + &dwc_priv->dly_param_1000m[1]);
> + eic7700_set_delay(dwc_priv->rx_delay_ps, dwc_priv->tx_delay_ps,
> + &dwc_priv->dly_param_100m[1]);
> + eic7700_set_delay(dwc_priv->rx_delay_ps, dwc_priv->tx_delay_ps,
> + &dwc_priv->dly_param_10m[1]);
> + } else {
> + dev_dbg(&pdev->dev, " use default dly\n");
> + }
> +
> + ret = of_property_read_variable_u32_array(pdev->dev.of_node, "eswin,dly_hsp_reg",
> + &dwc_priv->dly_hsp_reg[0], 3, 0);
> + if (ret != 3) {
> + dev_err(&pdev->dev, "can't get delay hsp reg.ret(%d)\n", ret);
> + return ret;
> + }
> +
> + dwc_priv->crg_regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
> + "eswin,syscrg_csr");
> + if (IS_ERR(dwc_priv->crg_regmap))
> + return dev_err_probe(&pdev->dev, PTR_ERR(dwc_priv->crg_regmap),
> + "Failed to get syscrg_csr regmap\n");
> +
> + ret = of_property_read_u32_index(pdev->dev.of_node, "eswin,syscrg_csr", 1,
> + &hsp_aclk_ctrl_offset);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret, "can't get hsp_aclk_ctrl_offset\n");
> +
> + regmap_read(dwc_priv->crg_regmap, hsp_aclk_ctrl_offset, &hsp_aclk_ctrl_regset);
> + hsp_aclk_ctrl_regset |= (EIC7700_HSP_ACLK_CLKEN | EIC7700_HSP_ACLK_DIVSOR);
> + regmap_write(dwc_priv->crg_regmap, hsp_aclk_ctrl_offset, hsp_aclk_ctrl_regset);
> +
> + ret = of_property_read_u32_index(pdev->dev.of_node, "eswin,syscrg_csr", 2,
> + &hsp_cfg_ctrl_offset);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret, "can't get hsp_cfg_ctrl_offset\n");
> +
> + regmap_write(dwc_priv->crg_regmap, hsp_cfg_ctrl_offset, EIC7700_HSP_CFG_CTRL_REGSET);
> +
> + dwc_priv->hsp_regmap = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
> + "eswin,hsp_sp_csr");
> + if (IS_ERR(dwc_priv->hsp_regmap))
> + return dev_err_probe(&pdev->dev, PTR_ERR(dwc_priv->hsp_regmap),
> + "Failed to get hsp_sp_csr regmap\n");
> +
> + ret = of_property_read_u32_index(pdev->dev.of_node, "eswin,hsp_sp_csr", 2,
> + ð_phy_ctrl_offset);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret, "can't get eth_phy_ctrl_offset\n");
> +
> + regmap_read(dwc_priv->hsp_regmap, eth_phy_ctrl_offset, ð_phy_ctrl_regset);
> + eth_phy_ctrl_regset |= (EIC7700_ETH_TX_CLK_SEL | EIC7700_ETH_PHY_INTF_SELI);
> + regmap_write(dwc_priv->hsp_regmap, eth_phy_ctrl_offset, eth_phy_ctrl_regset);
> +
> + ret = of_property_read_u32_index(pdev->dev.of_node, "eswin,hsp_sp_csr", 3,
> + ð_axi_lp_ctrl_offset);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret, "can't get eth_axi_lp_ctrl_offset\n");
> +
> + regmap_write(dwc_priv->hsp_regmap, eth_axi_lp_ctrl_offset, EIC7700_ETH_CSYSREQ_VAL);
> +
Consider more sensible wrapping of this (netdev frowns at >80
characters per line, except for message strings that should remain
greppable.)
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Powered by blists - more mailing lists