[v1,1/1] net/ice: fix E830 PTP phy model

Message ID 1f6ca9415c22415fc73d7bd72f220e09b5f79025.1721740015.git.anatoly.burakov@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Bruce Richardson
Headers
Series [v1,1/1] net/ice: fix E830 PTP phy model |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-marvell-Functional success Functional Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-compile-amd64-testing pending Testing pending
ci/iol-sample-apps-testing success Testing PASS

Commit Message

Burakov, Anatoly July 23, 2024, 1:10 p.m. UTC
Currently, we manually set PHY model in `ice_dev_init`, however we missed
adding case for E830, so for E830 the initialization ends up calling E822
code instead. This results in incorrect phy model being set and having
several downstream consequences for E830 as a result, ranging from a
stray error message from attempting to start PHY timer, and up to
inability to enable timesync on E830 devices.

We could've fixed it by adding a case for E830, however there are several
other missing bits of initialization (such as `phy_ports` field). All of
this can be fixed by replacing manual setting of `phy_model` with a call
to `ice_ptp_init_phy_model()`, which calls into base code and initializes
the fields appropriately for all device types, including another option
that is missing from current implementation - ETH56G.

Fixes: c3bedb7114f2 ("net/ice/base: add E830 PTP initialization")

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 drivers/net/ice/ice_ethdev.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
  

Comments

Bruce Richardson July 23, 2024, 1:17 p.m. UTC | #1
On Tue, Jul 23, 2024 at 02:10:56PM +0100, Anatoly Burakov wrote:
> Currently, we manually set PHY model in `ice_dev_init`, however we missed
> adding case for E830, so for E830 the initialization ends up calling E822
> code instead. This results in incorrect phy model being set and having
> several downstream consequences for E830 as a result, ranging from a
> stray error message from attempting to start PHY timer, and up to
> inability to enable timesync on E830 devices.
> 
> We could've fixed it by adding a case for E830, however there are several
> other missing bits of initialization (such as `phy_ports` field). All of
> this can be fixed by replacing manual setting of `phy_model` with a call
> to `ice_ptp_init_phy_model()`, which calls into base code and initializes
> the fields appropriately for all device types, including another option
> that is missing from current implementation - ETH56G.
> 
> Fixes: c3bedb7114f2 ("net/ice/base: add E830 PTP initialization")
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>

Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>

> ---
>  drivers/net/ice/ice_ethdev.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
> index 29509b4329..304f959b7e 100644
> --- a/drivers/net/ice/ice_ethdev.c
> +++ b/drivers/net/ice/ice_ethdev.c
> @@ -2496,10 +2496,8 @@ ice_dev_init(struct rte_eth_dev *dev)
>  	/* Initialize TM configuration */
>  	ice_tm_conf_init(dev);
>  
> -	if (ice_is_e810(hw))
> -		hw->phy_model = ICE_PHY_E810;
> -	else
> -		hw->phy_model = ICE_PHY_E822;
> +	/* Initialize PHY model */
> +	ice_ptp_init_phy_model(hw);
>  
>  	if (hw->phy_model == ICE_PHY_E822) {
>  		ret = ice_start_phy_timer_e822(hw, hw->pf_id);
> -- 
> 2.43.5
>
  
Bruce Richardson July 23, 2024, 1:57 p.m. UTC | #2
On Tue, Jul 23, 2024 at 02:17:22PM +0100, Bruce Richardson wrote:
> On Tue, Jul 23, 2024 at 02:10:56PM +0100, Anatoly Burakov wrote:
> > Currently, we manually set PHY model in `ice_dev_init`, however we missed
> > adding case for E830, so for E830 the initialization ends up calling E822
> > code instead. This results in incorrect phy model being set and having
> > several downstream consequences for E830 as a result, ranging from a
> > stray error message from attempting to start PHY timer, and up to
> > inability to enable timesync on E830 devices.
> > 
> > We could've fixed it by adding a case for E830, however there are several
> > other missing bits of initialization (such as `phy_ports` field). All of
> > this can be fixed by replacing manual setting of `phy_model` with a call
> > to `ice_ptp_init_phy_model()`, which calls into base code and initializes
> > the fields appropriately for all device types, including another option
> > that is missing from current implementation - ETH56G.
> > 
> > Fixes: c3bedb7114f2 ("net/ice/base: add E830 PTP initialization")
> > 
> > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> 
> Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>
> 
Applied to dpdk-next-net-intel

Thanks,
/Bruce
  

Patch

diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index 29509b4329..304f959b7e 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -2496,10 +2496,8 @@  ice_dev_init(struct rte_eth_dev *dev)
 	/* Initialize TM configuration */
 	ice_tm_conf_init(dev);
 
-	if (ice_is_e810(hw))
-		hw->phy_model = ICE_PHY_E810;
-	else
-		hw->phy_model = ICE_PHY_E822;
+	/* Initialize PHY model */
+	ice_ptp_init_phy_model(hw);
 
 	if (hw->phy_model == ICE_PHY_E822) {
 		ret = ice_start_phy_timer_e822(hw, hw->pf_id);