Re: [PATCH net-next] bnxt_en: add timestamping statistics support

[Vadim Fedorenko <vadim.fedorenko@linux.dev>]

On 29/05/2024 18:48, Michael Chan wrote:
> On Wed, May 29, 2024 at 10:19 AM Vadim Fedorenko <vadfed@meta.com> wrote:
>>
>> The ethtool_ts_stats structure was introduced earlier this year. Now
>> it's time to support this group of counters in more drivers.
>> This patch adds support to bnxt driver.
>>
>> Signed-off-by: Vadim Fedorenko <vadfed@meta.com>
>> ---
>>   drivers/net/ethernet/broadcom/bnxt/bnxt.c      | 18 +++++++++++++-----
>>   .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c  | 18 ++++++++++++++++++
>>   drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c  | 18 ++++++++++++++++++
>>   drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h  |  8 ++++++++
>>   4 files changed, 57 insertions(+), 5 deletions(-)
>>
> 
>> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h
>> index 2c3415c8fc03..589e093b1608 100644
>> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h
>> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h
>> @@ -79,6 +79,12 @@ struct bnxt_pps {
>>          struct pps_pin pins[BNXT_MAX_TSIO_PINS];
>>   };
>>
>> +struct bnxt_ptp_stats {
>> +       u64             ts_pkts;
>> +       u64             ts_lost;
>> +       atomic64_t      ts_err;
>> +};
>> +
>>   struct bnxt_ptp_cfg {
>>          struct ptp_clock_info   ptp_info;
>>          struct ptp_clock        *ptp_clock;
>> @@ -125,6 +131,8 @@ struct bnxt_ptp_cfg {
>>          u32                     refclk_mapped_regs[2];
>>          u32                     txts_tmo;
>>          unsigned long           abs_txts_tmo;
>> +
>> +       struct bnxt_ptp_stats   *stats;
> 
> I think there is no need to allocate this small stats structure
> separately.  It can just be:
> 
> struct bnxt_ptp_stats    stats;
> 
> The struct bnxt_ptp_cfg will only be allocated if the device supports
> PTP.  So the stats can always be a part of struct bnxt_ptp_cfg.

Yeah, I was thinking about embedding the struct into bnxt_ptp_cfg.
Ok, I'll make v2 in 24hr then.

Thanks!

> Other than that, it looks good to me.  Thanks.
> 
>>   };
>>