Re: [PATCH net 1/1] igc: return an error if the mac type is unknown in igc_ptp_systim_to_hwtstamp()

[Jiri Pirko <jiri@resnulli.us>]

Tue, Jan 31, 2023 at 10:54:37PM CET, anthony.l.nguyen@intel.com wrote:
>From: Tom Rix <trix@redhat.com>
>
>clang static analysis reports
>drivers/net/ethernet/intel/igc/igc_ptp.c:673:3: warning: The left operand of
>  '+' is a garbage value [core.UndefinedBinaryOperatorResult]
>   ktime_add_ns(shhwtstamps.hwtstamp, adjust);
>   ^            ~~~~~~~~~~~~~~~~~~~~
>
>igc_ptp_systim_to_hwtstamp() silently returns without setting the hwtstamp
>if the mac type is unknown.  This should be treated as an error.
>
>Fixes: 81b055205e8b ("igc: Add support for RX timestamping")
>Signed-off-by: Tom Rix <trix@redhat.com>
>Reviewed-by: Simon Horman <simon.horman@corigine.com>
>Acked-by: Sasha Neftin <sasha.neftin@intel.com>
>Tested-by: Naama Meir <naamax.meir@linux.intel.com>
>Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
>---
> drivers/net/ethernet/intel/igc/igc_ptp.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
>diff --git a/drivers/net/ethernet/intel/igc/igc_ptp.c b/drivers/net/ethernet/intel/igc/igc_ptp.c
>index c34734d432e0..4e10ced736db 100644
>--- a/drivers/net/ethernet/intel/igc/igc_ptp.c
>+++ b/drivers/net/ethernet/intel/igc/igc_ptp.c
>@@ -417,10 +417,12 @@ static int igc_ptp_verify_pin(struct ptp_clock_info *ptp, unsigned int pin,
>  *
>  * We need to convert the system time value stored in the RX/TXSTMP registers
>  * into a hwtstamp which can be used by the upper level timestamping functions.
>+ *
>+ * Returns 0 on success.
>  **/
>-static void igc_ptp_systim_to_hwtstamp(struct igc_adapter *adapter,
>-				       struct skb_shared_hwtstamps *hwtstamps,
>-				       u64 systim)
>+static int igc_ptp_systim_to_hwtstamp(struct igc_adapter *adapter,
>+				      struct skb_shared_hwtstamps *hwtstamps,
>+				      u64 systim)
> {
> 	switch (adapter->hw.mac.type) {
> 	case igc_i225:
>@@ -430,8 +432,9 @@ static void igc_ptp_systim_to_hwtstamp(struct igc_adapter *adapter,
> 						systim & 0xFFFFFFFF);
> 		break;
> 	default:
>-		break;
>+		return -EINVAL;
> 	}
>+	return 0;
> }
> 
> /**
>@@ -652,7 +655,8 @@ static void igc_ptp_tx_hwtstamp(struct igc_adapter *adapter)
> 
> 	regval = rd32(IGC_TXSTMPL);
> 	regval |= (u64)rd32(IGC_TXSTMPH) << 32;
>-	igc_ptp_systim_to_hwtstamp(adapter, &shhwtstamps, regval);
>+	if (igc_ptp_systim_to_hwtstamp(adapter, &shhwtstamps, regval))

Use variable to store the return value.


>+		return;
> 
> 	switch (adapter->link_speed) {
> 	case SPEED_10:
>-- 
>2.38.1
>