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

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

[Anatoly Burakov]

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(-)

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

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

[Bruce Richardson]

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
> 

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

[Bruce Richardson]

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