> -----Original Message----- > From: Intel-wired-lan <[email protected]> On Behalf Of > Jacob Keller > Sent: Thursday, October 17, 2024 1:25 AM > To: Kolacinski, Karol <[email protected]>; intel-wired- > [email protected] > Cc: [email protected]; Kubalewski, Arkadiusz > <[email protected]>; Nguyen, Anthony L > <[email protected]>; Kitszel, Przemyslaw > <[email protected]> > Subject: Re: [Intel-wired-lan] [PATCH v2 iwl-net 1/4] ice: Fix E825 > initialization > > > > On 10/10/2024 7:21 AM, Karol Kolacinski wrote: > > @@ -2686,16 +2687,9 @@ static void ice_ptp_init_phy_e825c(struct > ice_hw *hw) > > ptp->num_lports = params->num_phys * ptp->ports_per_phy; > > > > ice_sb_access_ena_eth56g(hw, true); > > - for (phy = 0; phy < params->num_phys; phy++) { > > - u32 phy_rev; > > - int err; > > - > > - err = ice_read_phy_eth56g(hw, phy, PHY_REG_REVISION, > &phy_rev); > > - if (err || phy_rev != PHY_REVISION_ETH56G) { > > - ptp->phy_model = ICE_PHY_UNSUP; > > - return; > > - } > > - } > > + err = ice_read_phy_eth56g(hw, hw->pf_id, PHY_REG_REVISION, > &phy_rev); > > + if (err || phy_rev != PHY_REVISION_ETH56G) > > + ptp->phy_model = ICE_PHY_UNSUP; > > > > Shouldn't this return like it did above? >
You're correct. Also, as you noticed in patch 4/4 of this series, in current Implementation, where PHY type is derived from mac_type, above checker seems to be redundant. To be fixed in v3 > > ptp->is_2x50g_muxed_topo = ice_is_muxed_topo(hw); > > } > > > diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h > b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h > > index 0852a34ade91..35141198f261 100644 > > --- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h > > +++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h > > @@ -326,7 +326,8 @@ extern const struct ice_vernier_info_e82x > e822_vernier[NUM_ICE_PTP_LNK_SPD]; > > */ > > #define ICE_E810_PLL_FREQ 812500000 > > #define ICE_PTP_NOMINAL_INCVAL_E810 0x13b13b13bULL > > -#define E810_OUT_PROP_DELAY_NS 1 > > +#define ICE_E810_OUT_PROP_DELAY_NS 1 > > +#define ICE_E825_SYNC_DELAY 6 > > > > Why is this hunk in this patch? Looks like some leftover after rebase. To be removed in next version
