| Message ID | 20240426125707.585269-6-christophe.roullier@foss.st.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Series to deliver Ethernets for STM32MP13 | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
On 4/26/24 2:57 PM, Christophe Roullier wrote: > Some cleaning because some Ethernet PHY configs do not need to add > st,ext-phyclk property. > Change print info message "No phy clock provided" only when debug. > > Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com> > --- > .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 27 ++++++++++--------- > 1 file changed, 14 insertions(+), 13 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c > index 7529a8d15492..e648c4e790a7 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c > @@ -55,17 +55,17 @@ > *| | | 25MHz | 50MHz | | > * --------------------------------------------------------------------------- > *| MII | - | eth-ck | n/a | n/a | > - *| | | st,ext-phyclk | | | > + *| | | | | | > * --------------------------------------------------------------------------- > *| GMII | - | eth-ck | n/a | n/a | > - *| | | st,ext-phyclk | | | > + *| | | | | | > * --------------------------------------------------------------------------- > *| RGMII | - | eth-ck | n/a | eth-ck | > - *| | | st,ext-phyclk | | st,eth-clk-sel or| > + *| | | | | st,eth-clk-sel or| > *| | | | | st,ext-phyclk | > * --------------------------------------------------------------------------- > *| RMII | - | eth-ck | eth-ck | n/a | > - *| | | st,ext-phyclk | st,eth-ref-clk-sel | | > + *| | | | st,eth-ref-clk-sel | | > *| | | | or st,ext-phyclk | | > * --------------------------------------------------------------------------- > * > @@ -174,23 +174,22 @@ static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat) > 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 (clk_rate == ETH_CK_F_25M) I see two problems here. First, according to the table above, in MII mode, clk_rate cannot be anything else but 25 MHz, so the (clk_rate == ETH_CK_F_25M) condition is always true. Why not drop that condition ? The "dwmac->ext_phyclk" means "Ethernet PHY have no crystal", which means the clock are provided by the STM32 RCC clock IP instead, which means if the dwmac->ext_phyclk is true, dwmac->enable_eth_ck should be set to true, because dwmac->enable_eth_ck controls the enablement of these STM32 clock IP generated clock. Second, as far as I understand it, there is no way to operate this IP with external clock in MII mode, so this section should always be only: dwmac->enable_eth_ck = true; > dwmac->enable_eth_ck = true; > val = dwmac->ops->pmcsetr.eth1_selmii; > 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 (clk_rate == ETH_CK_F_25M) > dwmac->enable_eth_ck = true; > - val |= dwmac->ops->pmcsetr.eth1_clk_sel; > - } > pr_debug("SYSCFG init : PHY_INTERFACE_MODE_GMII\n"); > break; > case PHY_INTERFACE_MODE_RMII: > val = dwmac->ops->pmcsetr.eth1_sel_rmii | dwmac->ops->pmcsetr.eth2_sel_rmii; > - if ((clk_rate == ETH_CK_F_25M || clk_rate == ETH_CK_F_50M) && > + if (clk_rate == ETH_CK_F_25M) > + dwmac->enable_eth_ck = true; > + if (clk_rate == ETH_CK_F_50M && > (dwmac->eth_ref_clk_sel_reg || dwmac->ext_phyclk)) { This doesn't seem to be equivalent change to the previous code . Here, if the clock frequency is 25 MHz, the clock are unconditionally enabled. Before, the code enabled the clock only if clock frequency was 25 MHz AND one of the "dwmac->eth_ref_clk_sel_reg" or "dwmac->ext_phyclk" was set (i.e. clock provided by SoC RCC clock IP). I think it might make this code easier if you drop all of the frequency test conditionals, which aren't really all that useful, and only enable the clock if either dwmac->ext_phyclk / dwmac->eth_clk_sel_reg / dwmac->eth_ref_clk_sel_reg is set , because effectively what this entire convoluted code is implementing is "if (clock supplied by clock IP i.e. RCC) enable the clock()" *, right ? * And it is also toggling the right clock mux bit in PMCSETR. So, for MII this would be plain: dwmac->enable_eth_ck = true; For GMII/RGMII this would be: if (dwmac->ext_phyclk || dwmac->eth_clk_sel_reg) dwmac->enable_eth_ck = true; For RMII this would be: if (dwmac->ext_phyclk || dwmac->eth_ref_clk_sel_reg) dwmac->enable_eth_ck = true; Maybe the clock frequency validation can be retained, but done separately? > dwmac->enable_eth_ck = true; > val |= dwmac->ops->pmcsetr.eth1_ref_clk_sel; > @@ -203,7 +202,9 @@ static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat) > case PHY_INTERFACE_MODE_RGMII_RXID: > case PHY_INTERFACE_MODE_RGMII_TXID: > val = dwmac->ops->pmcsetr.eth1_sel_rgmii | dwmac->ops->pmcsetr.eth2_sel_rgmii; > - if ((clk_rate == ETH_CK_F_25M || clk_rate == ETH_CK_F_125M) && > + if (clk_rate == ETH_CK_F_25M) > + dwmac->enable_eth_ck = true; > + if (clk_rate == ETH_CK_F_125M && > (dwmac->eth_clk_sel_reg || dwmac->ext_phyclk)) { > dwmac->enable_eth_ck = true; > val |= dwmac->ops->pmcsetr.eth1_clk_sel; > @@ -219,7 +220,7 @@ static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat) > } > > /* Need to update PMCCLRR (clear register) */ > - regmap_write(dwmac->regmap, reg + dwmac->ops->syscfg_clr_off, > + regmap_write(dwmac->regmap, dwmac->ops->syscfg_clr_off, > dwmac->mode_mask); > > /* Update PMCSETR (set register) */ > @@ -328,7 +329,7 @@ static int stm32mp1_parse_data(struct stm32_dwmac *dwmac, > /* Get ETH_CLK clocks */ > dwmac->clk_eth_ck = devm_clk_get(dev, "eth-ck"); > if (IS_ERR(dwmac->clk_eth_ck)) { > - dev_info(dev, "No phy clock provided...\n"); > + dev_dbg(dev, "No phy clock provided...\n"); > dwmac->clk_eth_ck = NULL; > } >
On 4/26/24 17:37, Marek Vasut wrote: > On 4/26/24 2:57 PM, Christophe Roullier wrote: >> Some cleaning because some Ethernet PHY configs do not need to add >> st,ext-phyclk property. >> Change print info message "No phy clock provided" only when debug. >> >> Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com> >> --- >> .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 27 ++++++++++--------- >> 1 file changed, 14 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c >> b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c >> index 7529a8d15492..e648c4e790a7 100644 >> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c >> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c >> @@ -55,17 +55,17 @@ >> *| | | 25MHz | 50MHz >> | | >> * >> --------------------------------------------------------------------------- >> *| MII | - | eth-ck | n/a | >> n/a | >> - *| | | st,ext-phyclk | | | >> + *| | | | | | >> * >> --------------------------------------------------------------------------- >> *| GMII | - | eth-ck | n/a | >> n/a | >> - *| | | st,ext-phyclk | | | >> + *| | | | | | >> * >> --------------------------------------------------------------------------- >> *| RGMII | - | eth-ck | n/a | >> eth-ck | >> - *| | | st,ext-phyclk | | >> st,eth-clk-sel or| >> + *| | | | | >> st,eth-clk-sel or| >> *| | | | | >> st,ext-phyclk | >> * >> --------------------------------------------------------------------------- >> *| RMII | - | eth-ck | eth-ck | >> n/a | >> - *| | | st,ext-phyclk | st,eth-ref-clk-sel >> | | >> + *| | | | st,eth-ref-clk-sel >> | | >> *| | | | or st,ext-phyclk >> | | >> * >> --------------------------------------------------------------------------- >> * >> @@ -174,23 +174,22 @@ static int stm32mp1_set_mode(struct >> plat_stmmacenet_data *plat_dat) >> 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 (clk_rate == ETH_CK_F_25M) > > I see two problems here. > > First, according to the table above, in MII mode, clk_rate cannot be > anything else but 25 MHz, so the (clk_rate == ETH_CK_F_25M) condition > is always true. Why not drop that condition ? Not agree, there is also "Normal" case MII (MII with quartz/cristal) (first column in the table above), so need to keep this test to check clk_rate 25MHz. > > The "dwmac->ext_phyclk" means "Ethernet PHY have no crystal", which > means the clock are provided by the STM32 RCC clock IP instead, which > means if the dwmac->ext_phyclk is true, dwmac->enable_eth_ck should be > set to true, because dwmac->enable_eth_ck controls the enablement of > these STM32 clock IP generated clock. Right > > Second, as far as I understand it, there is no way to operate this IP > with external clock in MII mode, so this section should always be only: > > dwmac->enable_eth_ck = true; Not for case "Normal" MII :-) > >> dwmac->enable_eth_ck = true; >> val = dwmac->ops->pmcsetr.eth1_selmii; >> 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 (clk_rate == ETH_CK_F_25M) >> dwmac->enable_eth_ck = true; >> - val |= dwmac->ops->pmcsetr.eth1_clk_sel; >> - } >> pr_debug("SYSCFG init : PHY_INTERFACE_MODE_GMII\n"); >> break; >> case PHY_INTERFACE_MODE_RMII: >> val = dwmac->ops->pmcsetr.eth1_sel_rmii | >> dwmac->ops->pmcsetr.eth2_sel_rmii; >> - if ((clk_rate == ETH_CK_F_25M || clk_rate == ETH_CK_F_50M) && >> + if (clk_rate == ETH_CK_F_25M) >> + dwmac->enable_eth_ck = true; >> + if (clk_rate == ETH_CK_F_50M && >> (dwmac->eth_ref_clk_sel_reg || dwmac->ext_phyclk)) { > > This doesn't seem to be equivalent change to the previous code . Here, > if the clock frequency is 25 MHz, the clock are unconditionally > enabled. Before, the code enabled the clock only if clock frequency > was 25 MHz AND one of the "dwmac->eth_ref_clk_sel_reg" or > "dwmac->ext_phyclk" was set (i.e. clock provided by SoC RCC clock IP). You are right, but in STM32MP15/MP13 reference manual it is write that we need to update SYSCFG (SYSCFG_PMCSETR) register only in "Ethernet 50MHz RMII clock selection": Bit 17 ETH_REF_CLK_SEL: Ethernet 50MHz RMII clock selection. Set by software. 0: Writing '0' has no effect, reading '0' means External clock is used. Need selection of AFMux. Could be used with all PHY 1: Writing '1' set this bit, reading '1' means Internal clock ETH_CLK1 from RCC is used regardless AFMux. Could be used only with RMII PHY > > I think it might make this code easier if you drop all of the > frequency test conditionals, which aren't really all that useful, and > only enable the clock if either dwmac->ext_phyclk / > dwmac->eth_clk_sel_reg / dwmac->eth_ref_clk_sel_reg is set , because > effectively what this entire convoluted code is implementing is "if > (clock supplied by clock IP i.e. RCC) enable the clock()" *, right ? > > * And it is also toggling the right clock mux bit in PMCSETR. > > So, for MII this would be plain: > dwmac->enable_eth_ck = true; > > For GMII/RGMII this would be: > if (dwmac->ext_phyclk || dwmac->eth_clk_sel_reg) > dwmac->enable_eth_ck = true; > > For RMII this would be: > if (dwmac->ext_phyclk || dwmac->eth_ref_clk_sel_reg) > dwmac->enable_eth_ck = true; > > Maybe the clock frequency validation can be retained, but done > separately? As explained previously, need to keep check of clock frequency in this test. > >> dwmac->enable_eth_ck = true; >> val |= dwmac->ops->pmcsetr.eth1_ref_clk_sel; >> @@ -203,7 +202,9 @@ static int stm32mp1_set_mode(struct >> plat_stmmacenet_data *plat_dat) >> case PHY_INTERFACE_MODE_RGMII_RXID: >> case PHY_INTERFACE_MODE_RGMII_TXID: >> val = dwmac->ops->pmcsetr.eth1_sel_rgmii | >> dwmac->ops->pmcsetr.eth2_sel_rgmii; >> - if ((clk_rate == ETH_CK_F_25M || clk_rate == ETH_CK_F_125M) && >> + if (clk_rate == ETH_CK_F_25M) >> + dwmac->enable_eth_ck = true; >> + if (clk_rate == ETH_CK_F_125M && >> (dwmac->eth_clk_sel_reg || dwmac->ext_phyclk)) { >> dwmac->enable_eth_ck = true; >> val |= dwmac->ops->pmcsetr.eth1_clk_sel; >> @@ -219,7 +220,7 @@ static int stm32mp1_set_mode(struct >> plat_stmmacenet_data *plat_dat) >> } >> /* Need to update PMCCLRR (clear register) */ >> - regmap_write(dwmac->regmap, reg + dwmac->ops->syscfg_clr_off, >> + regmap_write(dwmac->regmap, dwmac->ops->syscfg_clr_off, >> dwmac->mode_mask); >> /* Update PMCSETR (set register) */ >> @@ -328,7 +329,7 @@ static int stm32mp1_parse_data(struct stm32_dwmac >> *dwmac, >> /* Get ETH_CLK clocks */ >> dwmac->clk_eth_ck = devm_clk_get(dev, "eth-ck"); >> if (IS_ERR(dwmac->clk_eth_ck)) { >> - dev_info(dev, "No phy clock provided...\n"); >> + dev_dbg(dev, "No phy clock provided...\n"); >> dwmac->clk_eth_ck = NULL; >> }
On 5/13/24 5:11 PM, Christophe ROULLIER wrote: > > On 4/26/24 17:37, Marek Vasut wrote: >> On 4/26/24 2:57 PM, Christophe Roullier wrote: >>> Some cleaning because some Ethernet PHY configs do not need to add >>> st,ext-phyclk property. >>> Change print info message "No phy clock provided" only when debug. >>> >>> Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com> >>> --- >>> .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 27 ++++++++++--------- >>> 1 file changed, 14 insertions(+), 13 deletions(-) >>> >>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c >>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c >>> index 7529a8d15492..e648c4e790a7 100644 >>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c >>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c >>> @@ -55,17 +55,17 @@ >>> *| | | 25MHz | 50MHz >>> | | >>> * >>> --------------------------------------------------------------------------- >>> *| MII | - | eth-ck | n/a | n/a | >>> - *| | | st,ext-phyclk | | | >>> + *| | | | | | >>> * >>> --------------------------------------------------------------------------- >>> *| GMII | - | eth-ck | n/a | n/a | >>> - *| | | st,ext-phyclk | | | >>> + *| | | | | | >>> * >>> --------------------------------------------------------------------------- >>> *| RGMII | - | eth-ck | n/a | eth-ck | >>> - *| | | st,ext-phyclk | | >>> st,eth-clk-sel or| >>> + *| | | | | >>> st,eth-clk-sel or| >>> *| | | | | >>> st,ext-phyclk | >>> * >>> --------------------------------------------------------------------------- >>> *| RMII | - | eth-ck | eth-ck | n/a | >>> - *| | | st,ext-phyclk | st,eth-ref-clk-sel >>> | | >>> + *| | | | st,eth-ref-clk-sel >>> | | >>> *| | | | or st,ext-phyclk >>> | | >>> * >>> --------------------------------------------------------------------------- >>> * >>> @@ -174,23 +174,22 @@ static int stm32mp1_set_mode(struct >>> plat_stmmacenet_data *plat_dat) >>> 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 (clk_rate == ETH_CK_F_25M) >> >> I see two problems here. >> >> First, according to the table above, in MII mode, clk_rate cannot be >> anything else but 25 MHz, so the (clk_rate == ETH_CK_F_25M) condition >> is always true. Why not drop that condition ? > Not agree, there is also "Normal" case MII (MII with quartz/cristal) > (first column in the table above), so need to keep this test to check > clk_rate 25MHz. What other rate is supported in the MII mode ? Isn't the rate always 25 MHz for MII , no matter whether it is generated by RCC or external Xtal ? >> The "dwmac->ext_phyclk" means "Ethernet PHY have no crystal", which >> means the clock are provided by the STM32 RCC clock IP instead, which >> means if the dwmac->ext_phyclk is true, dwmac->enable_eth_ck should be >> set to true, because dwmac->enable_eth_ck controls the enablement of >> these STM32 clock IP generated clock. > Right >> >> Second, as far as I understand it, there is no way to operate this IP >> with external clock in MII mode, so this section should always be only: >> >> dwmac->enable_eth_ck = true; > Not for case "Normal" MII :-) What happens if that external clock source is not an xtal, but an oscillator with its own driver described in DT, and enabled e.g. using GPIO (that's compatible "gpio-gate-clock" for that oscillator) . Then you do need to enable those clock even in the "normal" (with external clock source instead of RCC clock source) MII case. >>> dwmac->enable_eth_ck = true; >>> val = dwmac->ops->pmcsetr.eth1_selmii; >>> 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 (clk_rate == ETH_CK_F_25M) >>> dwmac->enable_eth_ck = true; >>> - val |= dwmac->ops->pmcsetr.eth1_clk_sel; >>> - } >>> pr_debug("SYSCFG init : PHY_INTERFACE_MODE_GMII\n"); >>> break; >>> case PHY_INTERFACE_MODE_RMII: >>> val = dwmac->ops->pmcsetr.eth1_sel_rmii | >>> dwmac->ops->pmcsetr.eth2_sel_rmii; >>> - if ((clk_rate == ETH_CK_F_25M || clk_rate == ETH_CK_F_50M) && >>> + if (clk_rate == ETH_CK_F_25M) >>> + dwmac->enable_eth_ck = true; >>> + if (clk_rate == ETH_CK_F_50M && >>> (dwmac->eth_ref_clk_sel_reg || dwmac->ext_phyclk)) { >> >> This doesn't seem to be equivalent change to the previous code . Here, >> if the clock frequency is 25 MHz, the clock are unconditionally >> enabled. Before, the code enabled the clock only if clock frequency >> was 25 MHz AND one of the "dwmac->eth_ref_clk_sel_reg" or >> "dwmac->ext_phyclk" was set (i.e. clock provided by SoC RCC clock IP). > > You are right, but in STM32MP15/MP13 reference manual it is write that > we need to update SYSCFG (SYSCFG_PMCSETR) register only in "Ethernet > 50MHz RMII clock selection": > > Bit 17 ETH_REF_CLK_SEL: Ethernet 50MHz RMII clock selection. > > Set by software. > > 0: Writing '0' has no effect, reading '0' means External clock is > used. Need selection of AFMux. Could be used with all PHY > > 1: Writing '1' set this bit, reading '1' means Internal clock > ETH_CLK1 from RCC is used regardless AFMux. Could be used only with RMII > PHY Look at this: " RM0436 Rev 6 Reset and clock control (RCC) Clock distribution for Ethernet (ETH) Figure 83. Peripheral clock distribution for Ethernet Page 575 " See the mux at bottom left side in the PKCS group. I suspect that no matter whether the clock are 25 MHz or 50 MHz, they enter this mux, the mux selects the clock source from either RCC or from external oscillator, and therefore the original code was correct. >> I think it might make this code easier if you drop all of the >> frequency test conditionals, which aren't really all that useful, and >> only enable the clock if either dwmac->ext_phyclk / >> dwmac->eth_clk_sel_reg / dwmac->eth_ref_clk_sel_reg is set , because >> effectively what this entire convoluted code is implementing is "if >> (clock supplied by clock IP i.e. RCC) enable the clock()" *, right ? >> >> * And it is also toggling the right clock mux bit in PMCSETR. >> >> So, for MII this would be plain: >> dwmac->enable_eth_ck = true; >> >> For GMII/RGMII this would be: >> if (dwmac->ext_phyclk || dwmac->eth_clk_sel_reg) >> dwmac->enable_eth_ck = true; >> >> For RMII this would be: >> if (dwmac->ext_phyclk || dwmac->eth_ref_clk_sel_reg) >> dwmac->enable_eth_ck = true; >> >> Maybe the clock frequency validation can be retained, but done >> separately? > As explained previously, need to keep check of clock frequency in this > test. I sent 5 patches which split this code up, you were on CC: [net-next,RFC,PATCH 1/5] net: stmmac: dwmac-stm32: Separate out external clock rate validation Maybe you can apply those, fix them up as needed, and then add this series on top ? I think it would simplify this code a lot, since that series splits up the clock rate validation / PMCR configuration / external-internal clock selection into separate steps. What do you think ?
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c index 7529a8d15492..e648c4e790a7 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c @@ -55,17 +55,17 @@ *| | | 25MHz | 50MHz | | * --------------------------------------------------------------------------- *| MII | - | eth-ck | n/a | n/a | - *| | | st,ext-phyclk | | | + *| | | | | | * --------------------------------------------------------------------------- *| GMII | - | eth-ck | n/a | n/a | - *| | | st,ext-phyclk | | | + *| | | | | | * --------------------------------------------------------------------------- *| RGMII | - | eth-ck | n/a | eth-ck | - *| | | st,ext-phyclk | | st,eth-clk-sel or| + *| | | | | st,eth-clk-sel or| *| | | | | st,ext-phyclk | * --------------------------------------------------------------------------- *| RMII | - | eth-ck | eth-ck | n/a | - *| | | st,ext-phyclk | st,eth-ref-clk-sel | | + *| | | | st,eth-ref-clk-sel | | *| | | | or st,ext-phyclk | | * --------------------------------------------------------------------------- * @@ -174,23 +174,22 @@ static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat) 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 (clk_rate == ETH_CK_F_25M) dwmac->enable_eth_ck = true; val = dwmac->ops->pmcsetr.eth1_selmii; 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 (clk_rate == ETH_CK_F_25M) dwmac->enable_eth_ck = true; - val |= dwmac->ops->pmcsetr.eth1_clk_sel; - } pr_debug("SYSCFG init : PHY_INTERFACE_MODE_GMII\n"); break; case PHY_INTERFACE_MODE_RMII: val = dwmac->ops->pmcsetr.eth1_sel_rmii | dwmac->ops->pmcsetr.eth2_sel_rmii; - if ((clk_rate == ETH_CK_F_25M || clk_rate == ETH_CK_F_50M) && + if (clk_rate == ETH_CK_F_25M) + dwmac->enable_eth_ck = true; + if (clk_rate == ETH_CK_F_50M && (dwmac->eth_ref_clk_sel_reg || dwmac->ext_phyclk)) { dwmac->enable_eth_ck = true; val |= dwmac->ops->pmcsetr.eth1_ref_clk_sel; @@ -203,7 +202,9 @@ static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat) case PHY_INTERFACE_MODE_RGMII_RXID: case PHY_INTERFACE_MODE_RGMII_TXID: val = dwmac->ops->pmcsetr.eth1_sel_rgmii | dwmac->ops->pmcsetr.eth2_sel_rgmii; - if ((clk_rate == ETH_CK_F_25M || clk_rate == ETH_CK_F_125M) && + if (clk_rate == ETH_CK_F_25M) + dwmac->enable_eth_ck = true; + if (clk_rate == ETH_CK_F_125M && (dwmac->eth_clk_sel_reg || dwmac->ext_phyclk)) { dwmac->enable_eth_ck = true; val |= dwmac->ops->pmcsetr.eth1_clk_sel; @@ -219,7 +220,7 @@ static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat) } /* Need to update PMCCLRR (clear register) */ - regmap_write(dwmac->regmap, reg + dwmac->ops->syscfg_clr_off, + regmap_write(dwmac->regmap, dwmac->ops->syscfg_clr_off, dwmac->mode_mask); /* Update PMCSETR (set register) */ @@ -328,7 +329,7 @@ static int stm32mp1_parse_data(struct stm32_dwmac *dwmac, /* Get ETH_CLK clocks */ dwmac->clk_eth_ck = devm_clk_get(dev, "eth-ck"); if (IS_ERR(dwmac->clk_eth_ck)) { - dev_info(dev, "No phy clock provided...\n"); + dev_dbg(dev, "No phy clock provided...\n"); dwmac->clk_eth_ck = NULL; }
Some cleaning because some Ethernet PHY configs do not need to add st,ext-phyclk property. Change print info message "No phy clock provided" only when debug. Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com> --- .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-)