| 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
| ||
|
Date: Mon, 29 Apr 2024 07:19:11 +0000
From: Sai Krishna Gajula <saikrishnag@...vell.com>
To: Marek Vasut <marex@...x.de>,
"netdev@...r.kernel.org"
<netdev@...r.kernel.org>
CC: "David S. Miller" <davem@...emloft.net>,
Alexandre Torgue
<alexandre.torgue@...s.st.com>,
Christophe Roullier
<christophe.roullier@...s.st.com>,
Eric Dumazet <edumazet@...gle.com>, Jakub
Kicinski <kuba@...nel.org>,
Jose Abreu <joabreu@...opsys.com>,
Maxime
Coquelin <mcoquelin.stm32@...il.com>,
Paolo Abeni <pabeni@...hat.com>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-stm32@...md-mailman.stormreply.com"
<linux-stm32@...md-mailman.stormreply.com>
Subject: RE: [net-next,RFC,PATCH 1/5] net: stmmac: dwmac-stm32: Separate out
external clock rate validation
> -----Original Message-----
> From: Marek Vasut <marex@...x.de>
> Sent: Sunday, April 28, 2024 3:21 AM
> To: netdev@...r.kernel.org
> Cc: Marek Vasut <marex@...x.de>; David S. Miller <davem@...emloft.net>;
> Alexandre Torgue <alexandre.torgue@...s.st.com>; Christophe Roullier
> <christophe.roullier@...s.st.com>; Eric Dumazet <edumazet@...gle.com>;
> Jakub Kicinski <kuba@...nel.org>; Jose Abreu <joabreu@...opsys.com>;
> Maxime Coquelin <mcoquelin.stm32@...il.com>; Paolo Abeni
> <pabeni@...hat.com>; linux-arm-kernel@...ts.infradead.org; linux-
> stm32@...md-mailman.stormreply.com
> Subject: [net-next,RFC,PATCH 1/5] net: stmmac: dwmac-stm32:
> Separate out external clock rate validation
>
> Pull the external clock frequency validation into a separate function, to avoid
> conflating it with external clock DT property decoding and clock mux register
> configuration. This should make the code easier to read and understand.
>
> This does change the code behavior slightly. The clock mux PMCR register
> setting now depends solely on the DT properties which configure the clock
> mux between external clock and internal RCC generated clock. The mux
> PMCR register settings no longer depend on the supplied clock frequency, that
> supplied clock frequency is now only validated, and if the clock frequency is
> invalid for a mode, it is rejected.
>
> Previously, the code would switch the PMCR register clock mux to internal RCC
> generated clock if external clock couldn't provide suitable frequency, without
> checking whether the RCC generated clock frequency is correct. Such behavior
> is risky at best, user should have configured their clock correctly in the first
> place, so this behavior is removed here.
>
> Signed-off-by: Marek Vasut <marex@...x.de>
> ---
> Cc: "David S. Miller" <davem@...emloft.net>
> Cc: Alexandre Torgue <alexandre.torgue@...s.st.com>
> Cc: Christophe Roullier <christophe.roullier@...s.st.com>
> Cc: Eric Dumazet <edumazet@...gle.com>
> Cc: Jakub Kicinski <kuba@...nel.org>
> Cc: Jose Abreu <joabreu@...opsys.com>
> Cc: Maxime Coquelin <mcoquelin.stm32@...il.com>
> Cc: Paolo Abeni <pabeni@...hat.com>
> Cc: linux-arm-kernel@...ts.infradead.org
> Cc: linux-stm32@...md-mailman.stormreply.com
> Cc: netdev@...r.kernel.org
> ---
> .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 54 +++++++++++++++----
> 1 file changed, 44 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
> b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
> index c92dfc4ecf570..43340a5573c64 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
> @@ -157,25 +157,57 @@ static int stm32_dwmac_init(struct
> plat_stmmacenet_data *plat_dat, bool resume)
> return stm32_dwmac_clk_enable(dwmac, resume); }
>
> +static int stm32mp1_validate_ethck_rate(struct plat_stmmacenet_data
> +*plat_dat) {
> + struct stm32_dwmac *dwmac = plat_dat->bsp_priv;
> + const u32 clk_rate = clk_get_rate(dwmac->clk_eth_ck);
Please check reverse x-mass tree is followed for these variables, if possible.
> +
> + switch (plat_dat->mac_interface) {
> + case PHY_INTERFACE_MODE_MII:
> + if (clk_rate == ETH_CK_F_25M)
> + return 0;
> + break;
> + case PHY_INTERFACE_MODE_GMII:
> + if (clk_rate == ETH_CK_F_25M)
> + return 0;
> + break;
Please check, whether we can combine the two cases..
> + case PHY_INTERFACE_MODE_RMII:
> + if (clk_rate == ETH_CK_F_25M || clk_rate == ETH_CK_F_50M)
> + return 0;
> + break;
> + case PHY_INTERFACE_MODE_RGMII:
> + case PHY_INTERFACE_MODE_RGMII_ID:
> + case PHY_INTERFACE_MODE_RGMII_RXID:
> + case PHY_INTERFACE_MODE_RGMII_TXID:
> + if (clk_rate == ETH_CK_F_25M || clk_rate ==
> ETH_CK_F_125M)
> + return 0;
> + break;
> + default:
> + break;
> + }
> +
> + dev_err(dwmac->dev, "Mode %s does not match eth-ck frequency %d
> Hz",
> + phy_modes(plat_dat->mac_interface), clk_rate);
> + return -EINVAL;
> +}
> +
> static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat) {
> struct stm32_dwmac *dwmac = plat_dat->bsp_priv;
> - u32 reg = dwmac->mode_reg, clk_rate;
> - int val;
> + u32 reg = dwmac->mode_reg;
> + int val, ret;
>
> - clk_rate = clk_get_rate(dwmac->clk_eth_ck);
> dwmac->enable_eth_ck = false;
> switch (plat_dat->mac_interface) {
> case PHY_INTERFACE_MODE_MII:
> - if (clk_rate == ETH_CK_F_25M && dwmac->ext_phyclk)
> + if (dwmac->ext_phyclk)
> dwmac->enable_eth_ck = true;
> val = SYSCFG_PMCR_ETH_SEL_MII;
> pr_debug("SYSCFG init : PHY_INTERFACE_MODE_MII\n");
> break;
> case PHY_INTERFACE_MODE_GMII:
> val = SYSCFG_PMCR_ETH_SEL_GMII;
> - if (clk_rate == ETH_CK_F_25M &&
> - (dwmac->eth_clk_sel_reg || dwmac->ext_phyclk)) {
> + if (dwmac->eth_clk_sel_reg || dwmac->ext_phyclk) {
> dwmac->enable_eth_ck = true;
> val |= SYSCFG_PMCR_ETH_CLK_SEL;
> }
> @@ -183,8 +215,7 @@ static int stm32mp1_set_mode(struct
> plat_stmmacenet_data *plat_dat)
> break;
> case PHY_INTERFACE_MODE_RMII:
> val = SYSCFG_PMCR_ETH_SEL_RMII;
> - if ((clk_rate == ETH_CK_F_25M || clk_rate == ETH_CK_F_50M)
> &&
> - (dwmac->eth_ref_clk_sel_reg || dwmac->ext_phyclk)) {
> + if (dwmac->eth_ref_clk_sel_reg || dwmac->ext_phyclk) {
> dwmac->enable_eth_ck = true;
> val |= SYSCFG_PMCR_ETH_REF_CLK_SEL;
> }
> @@ -195,8 +226,7 @@ static int stm32mp1_set_mode(struct
> plat_stmmacenet_data *plat_dat)
> case PHY_INTERFACE_MODE_RGMII_RXID:
> case PHY_INTERFACE_MODE_RGMII_TXID:
> val = SYSCFG_PMCR_ETH_SEL_RGMII;
> - if ((clk_rate == ETH_CK_F_25M || clk_rate ==
> ETH_CK_F_125M) &&
> - (dwmac->eth_clk_sel_reg || dwmac->ext_phyclk)) {
> + if (dwmac->eth_clk_sel_reg || dwmac->ext_phyclk) {
> dwmac->enable_eth_ck = true;
> val |= SYSCFG_PMCR_ETH_CLK_SEL;
> }
> @@ -209,6 +239,10 @@ static int stm32mp1_set_mode(struct
> plat_stmmacenet_data *plat_dat)
> return -EINVAL;
> }
>
> + ret = stm32mp1_validate_ethck_rate(plat_dat);
> + if (ret)
> + return ret;
> +
> /* Need to update PMCCLRR (clear register) */
> regmap_write(dwmac->regmap, reg + SYSCFG_PMCCLRR_OFFSET,
> dwmac->ops->syscfg_eth_mask);
> --
> 2.43.0
>
Powered by blists - more mailing lists