netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vadim Fedorenko <vadim.fedorenko@linux.dev>
To: Kamil Zaripov <zaripov-kamil@avride.ai>
Cc: Michael Chan <michael.chan@broadcom.com>,
	Jacob Keller <jacob.e.keller@intel.com>,
	Pavan Chebbi <pavan.chebbi@broadcom.com>,
	Linux Netdev List <netdev@vger.kernel.org>
Subject: Re: bnxt_en: Incorrect tx timestamp report
Date: Tue, 25 Mar 2025 10:41:53 +0000	[thread overview]
Message-ID: <0316a190-6022-4656-bd5e-1a1f544efa2d@linux.dev> (raw)
In-Reply-To: <9200948E-51F9-45A4-A04C-8AD0C749AD7B@avride.ai>

On 25/03/2025 10:13, Kamil Zaripov wrote:
> 
>> On 24 Mar 2025, at 17:04, Pavan Chebbi <pavan.chebbi@broadcom.com> wrote:
>>
>>> On Fri, 21 Mar, 2025, 11:03 pm Michael Chan, <michael.chan@broadcom.com> wrote:
>>>
>>>> On Fri, Mar 21, 2025 at 8:17 AM Kamil Zaripov <zaripov-kamil@avride.ai> wrote:
>>>>
>>>>> That depends. If it has only one underlying clock, but each PF has its
>>>>> own register space, it may functionally be independent clocks in
>>>>> practice. I don't know the bnxt_en driver or hardware well enough to
>>>>> know if that is the case.
>>>>
>>>>> If it really is one clock with one set of registers to control it, then
>>>>> it should only expose one PHC. This may be tricky depending on the
>>>>> driver design. (See ice as an example where we've had a lot of
>>>>> challenges in this space because of the multiple PFs)
>>>>
>>>> I can only guess, from looking at the __bnxt_hwrm_ptp_qcfg function,
>>>> that it depends on hardware and/or firmware (see
>>>> https://elixir.bootlin.com/linux/v6.13.7/source/drivers/net/ethernet/broadcom/bnxt/bnxt.c#L9427-L9431).
>>>> I hope that broadcom folks can clarify this.
>>>>
>>>
>>> It is one physical PHC per chip.  Each function has access to the
>>> shared PHC.   It won't work properly when multiple functions try to
>>> adjust the PHC independently.  That's why we use the non-RTC mode when
>>> the PHC is shared in multi-function mode.  Pavan can add more details
>>> on this.
>> Yes, that's correct. It's one PHC shared across functions. The way we handle multiple
>> functions accessing the shared PHC is by firmware allowing only one function to adjust
>> the frequency. All the other functions' adjustments are ignored. ...
> 
> I guess I don’t understand how does it work. Am I right that if userspace program changes frequency of PHC devices 0,1,2,3 (one for each port present in NIC) driver will send PHC frequency change 4 times but firmware will drop 3 of these frequency change commands and will pick up only one? How can I understand which PHC will actually represent adjustable clock and which one is phony?

It can be any of PHC devices, mostly the first to try to adjust will be 
used.

> 
> Another thing that I cannot understand is so-called RTC and non-RTC mode. Is there any documentation that describes it? Or specific parts of the driver that change its behavior on for RTC and non-RTC mode?

Generally, non-RTC means free-running HW PHC clock with timecounter
adjustment on top of it. With RTC mode every adjfine() call tries to
adjust HW configuration to change the slope of PHC.

> 
>> … However, needless to say,
>> they all still receive the latest timestamps. As I recall, this event design was an earlier
>> version of our multi host support implementation where the rollover was being tracked in
>> the firmware.
> 
>  From which version the bnxt_en driver starts to track rollover on the driver side rather than firmware side?

It was done a couple of years ago, in 5.x era.

> 
>> The latest driver handles the rollover on its own and we don't need the firmware to tell us.
>> I checked with the firmware team and I gather that the version you are using is very old.
>> Firmware version 230.x onwards, you should not receive this event for rollovers.
>> Is it possible for you to update the firmware? Do you have access to a more recent (230+) firmware?
> 
> Yes, I can update firmware if you can tell where can I find the latest firmware and the update instructions?
> 

Broadcom's web site has pretty easy support portal with NIC firmware
publicly available. Current version is 232 and it has all the
improvements Pavan mentioned.


  reply	other threads:[~2025-03-25 10:41 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-20 14:35 bnxt_en: Incorrect tx timestamp report Kamil Zaripov
2025-03-20 14:48 ` Andrew Lunn
     [not found]   ` <CAGtf3ibFAidzpFKm1o5zmZF3Neu8MgdXp_n_Wt+mv8M9YZhhug@mail.gmail.com>
2025-03-20 15:14     ` Kamil Zaripov
2025-03-20 16:21   ` Vadim Fedorenko
2025-03-20 15:56 ` Pavan Chebbi
2025-03-20 16:21   ` Kamil Zaripov
2025-03-20 16:26   ` Vadim Fedorenko
2025-03-20 17:11 ` Jacob Keller
2025-03-21 15:17   ` Kamil Zaripov
2025-03-21 17:33     ` Michael Chan
2025-03-24 15:04       ` Pavan Chebbi
2025-03-25 10:13         ` Kamil Zaripov
2025-03-25 10:41           ` Vadim Fedorenko [this message]
2025-03-25 12:24             ` Pavan Chebbi
2025-03-26 13:50             ` Kamil Zaripov
2025-03-26 20:31               ` Jacob Keller
2025-03-27 13:16               ` Pavan Chebbi
2025-04-01 20:17                 ` Keller, Jacob E

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0316a190-6022-4656-bd5e-1a1f544efa2d@linux.dev \
    --to=vadim.fedorenko@linux.dev \
    --cc=jacob.e.keller@intel.com \
    --cc=michael.chan@broadcom.com \
    --cc=netdev@vger.kernel.org \
    --cc=pavan.chebbi@broadcom.com \
    --cc=zaripov-kamil@avride.ai \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).