| Message ID | 20240902071841.3519866-1-ruanjinjie@huawei.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: lan743x: Use NSEC_PER_SEC macro | expand |
On Mon, Sep 2, 2024 at 12:41 PM Jinjie Ruan <ruanjinjie@huawei.com> wrote: > > 1000000000L is number of ns per second, use NSEC_PER_SEC macro to replace > it to make it more readable. > > Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> > --- > drivers/net/ethernet/microchip/lan743x_ptp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/microchip/lan743x_ptp.c b/drivers/net/ethernet/microchip/lan743x_ptp.c > index dcea6652d56d..9c2ec293c163 100644 > --- a/drivers/net/ethernet/microchip/lan743x_ptp.c > +++ b/drivers/net/ethernet/microchip/lan743x_ptp.c > @@ -409,7 +409,7 @@ static int lan743x_ptpci_settime64(struct ptp_clock_info *ptpci, > ts->tv_sec); > return -ERANGE; > } > - if (ts->tv_nsec >= 1000000000L || > + if (ts->tv_nsec >= NSEC_PER_SEC || > ts->tv_nsec < 0) { > netif_warn(adapter, drv, adapter->netdev, > "ts->tv_nsec out of range, %ld\n", > -- > 2.34.1 Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
On Mon, Sep 02, 2024 at 03:18:41PM +0800, Jinjie Ruan wrote: > 1000000000L is number of ns per second, use NSEC_PER_SEC macro to replace > it to make it more readable. > > Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> > --- > drivers/net/ethernet/microchip/lan743x_ptp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/microchip/lan743x_ptp.c b/drivers/net/ethernet/microchip/lan743x_ptp.c > index dcea6652d56d..9c2ec293c163 100644 > --- a/drivers/net/ethernet/microchip/lan743x_ptp.c > +++ b/drivers/net/ethernet/microchip/lan743x_ptp.c > @@ -409,7 +409,7 @@ static int lan743x_ptpci_settime64(struct ptp_clock_info *ptpci, > ts->tv_sec); > return -ERANGE; > } > - if (ts->tv_nsec >= 1000000000L || > + if (ts->tv_nsec >= NSEC_PER_SEC || > ts->tv_nsec < 0) { > netif_warn(adapter, drv, adapter->netdev, > "ts->tv_nsec out of range, %ld\n", https://elixir.bootlin.com/linux/v6.10.7/source/include/linux/time64.h#L92 /* * Returns true if the timespec64 is norm, false if denorm: */ static inline bool timespec64_valid(const struct timespec64 *ts) { /* Dates before 1970 are bogus */ if (ts->tv_sec < 0) return false; /* Can't have more nanoseconds then a second */ if ((unsigned long)ts->tv_nsec >= NSEC_PER_SEC) return false; return true; } And the next question is, why is the driver checking this? It would make more sense that the PTP core checked this before calling ptp->info->settime64() Andrew
On 2024/9/3 0:26, Andrew Lunn wrote: > On Mon, Sep 02, 2024 at 03:18:41PM +0800, Jinjie Ruan wrote: >> 1000000000L is number of ns per second, use NSEC_PER_SEC macro to replace >> it to make it more readable. >> >> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> >> --- >> drivers/net/ethernet/microchip/lan743x_ptp.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/net/ethernet/microchip/lan743x_ptp.c b/drivers/net/ethernet/microchip/lan743x_ptp.c >> index dcea6652d56d..9c2ec293c163 100644 >> --- a/drivers/net/ethernet/microchip/lan743x_ptp.c >> +++ b/drivers/net/ethernet/microchip/lan743x_ptp.c >> @@ -409,7 +409,7 @@ static int lan743x_ptpci_settime64(struct ptp_clock_info *ptpci, >> ts->tv_sec); >> return -ERANGE; >> } >> - if (ts->tv_nsec >= 1000000000L || >> + if (ts->tv_nsec >= NSEC_PER_SEC || >> ts->tv_nsec < 0) { >> netif_warn(adapter, drv, adapter->netdev, >> "ts->tv_nsec out of range, %ld\n", > > https://elixir.bootlin.com/linux/v6.10.7/source/include/linux/time64.h#L92 > > /* > * Returns true if the timespec64 is norm, false if denorm: > */ > static inline bool timespec64_valid(const struct timespec64 *ts) > { > /* Dates before 1970 are bogus */ > if (ts->tv_sec < 0) > return false; > /* Can't have more nanoseconds then a second */ > if ((unsigned long)ts->tv_nsec >= NSEC_PER_SEC) > return false; > return true; > } > > And the next question is, why is the driver checking this? It would > make more sense that the PTP core checked this before calling > ptp->info->settime64() There are 2 places call ptp->info->settime64(), it may make more sense to check timespec64_valid() here and remove these check internal like lan743x_ptp.c? drivers/net/phy/micrel.c:4721: ptp->settime64(ptp, &ts); drivers/ptp/ptp_clock.c:103: return ptp->info->settime64(ptp->info, tp); > > Andrew
> > And the next question is, why is the driver checking this? It would > > make more sense that the PTP core checked this before calling > > ptp->info->settime64() > > There are 2 places call ptp->info->settime64(), it may make more sense > to check timespec64_valid() here and remove these check internal like > lan743x_ptp.c? > > drivers/net/phy/micrel.c:4721: ptp->settime64(ptp, &ts); > drivers/ptp/ptp_clock.c:103: return ptp->info->settime64(ptp->info, tp); Yes, please extend the core. Andrew
diff --git a/drivers/net/ethernet/microchip/lan743x_ptp.c b/drivers/net/ethernet/microchip/lan743x_ptp.c index dcea6652d56d..9c2ec293c163 100644 --- a/drivers/net/ethernet/microchip/lan743x_ptp.c +++ b/drivers/net/ethernet/microchip/lan743x_ptp.c @@ -409,7 +409,7 @@ static int lan743x_ptpci_settime64(struct ptp_clock_info *ptpci, ts->tv_sec); return -ERANGE; } - if (ts->tv_nsec >= 1000000000L || + if (ts->tv_nsec >= NSEC_PER_SEC || ts->tv_nsec < 0) { netif_warn(adapter, drv, adapter->netdev, "ts->tv_nsec out of range, %ld\n",
1000000000L is number of ns per second, use NSEC_PER_SEC macro to replace it to make it more readable. Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> --- drivers/net/ethernet/microchip/lan743x_ptp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)