[PATCH net-next] bnxt_en: support PPS in/out on all pins

[PATCH net-next] bnxt_en: support PPS in/out on all pins

[Vadim Fedorenko]

n_ext_ts and n_per_out from ptp_clock_caps are checked as a max number
of pins rather than max number of active pins.

supported_extts_flags and supported_perout_flags have to be added as
well to make the driver complaint with the latest API.

Signed-off-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
index db81cf6d5289..c9b7df669415 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
@@ -965,10 +965,12 @@ static int bnxt_ptp_pps_init(struct bnxt *bp)
 	hwrm_req_drop(bp, req);
 
 	/* Only 1 each of ext_ts and per_out pins is available in HW */
-	ptp_info->n_ext_ts = 1;
-	ptp_info->n_per_out = 1;
+	ptp_info->n_ext_ts = pps_info->num_pins;
+	ptp_info->n_per_out = pps_info->num_pins;
 	ptp_info->pps = 1;
 	ptp_info->verify = bnxt_ptp_verify;
+	ptp_info->supported_extts_flags = PTP_RISING_EDGE | PTP_STRICT_FLAGS;
+	ptp_info->supported_perout_flags = PTP_PEROUT_DUTY_CYCLE;
 
 	return 0;
 }
-- 
2.47.3

Re: [PATCH net-next] bnxt_en: support PPS in/out on all pins

[Pavan Chebbi]

[-- Attachment #1: Type: text/plain, Size: 1586 bytes --]

On Fri, Oct 17, 2025 at 3:54 AM Vadim Fedorenko
<vadim.fedorenko@linux.dev> wrote:
>
> n_ext_ts and n_per_out from ptp_clock_caps are checked as a max number
> of pins rather than max number of active pins.

I am not 100pc sure. How is n_pins going to be different then?
https://elixir.bootlin.com/linux/v6.17/source/include/linux/ptp_clock_kernel.h#L69

>
> supported_extts_flags and supported_perout_flags have to be added as
> well to make the driver complaint with the latest API.
>
> Signed-off-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
> ---
>  drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
> index db81cf6d5289..c9b7df669415 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
> @@ -965,10 +965,12 @@ static int bnxt_ptp_pps_init(struct bnxt *bp)
>         hwrm_req_drop(bp, req);
>
>         /* Only 1 each of ext_ts and per_out pins is available in HW */
> -       ptp_info->n_ext_ts = 1;
> -       ptp_info->n_per_out = 1;
> +       ptp_info->n_ext_ts = pps_info->num_pins;
> +       ptp_info->n_per_out = pps_info->num_pins;
>         ptp_info->pps = 1;
>         ptp_info->verify = bnxt_ptp_verify;
> +       ptp_info->supported_extts_flags = PTP_RISING_EDGE | PTP_STRICT_FLAGS;
> +       ptp_info->supported_perout_flags = PTP_PEROUT_DUTY_CYCLE;
>
>         return 0;
>  }
> --
> 2.47.3
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5469 bytes --]

Re: [PATCH net-next] bnxt_en: support PPS in/out on all pins

[Vadim Fedorenko]

On 17.10.2025 04:45, Pavan Chebbi wrote:
> On Fri, Oct 17, 2025 at 3:54 AM Vadim Fedorenko
> <vadim.fedorenko@linux.dev> wrote:
>>
>> n_ext_ts and n_per_out from ptp_clock_caps are checked as a max number
>> of pins rather than max number of active pins.
> 
> I am not 100pc sure. How is n_pins going to be different then?
> https://elixir.bootlin.com/linux/v6.17/source/include/linux/ptp_clock_kernel.h#L69

So in general it's more for the case where HW has pins connected through mux to
the DPLL channels. According to the bnxt_ptp_cfg_pin() bnxt HW has pins
hardwired to channels and NVM has pre-defined configuration of pins' functions.

[host ~]# ./testptp -d /dev/ptp2 -l
name bnxt_pps0 index 0 func 0 chan 0
name bnxt_pps1 index 1 func 0 chan 1
name bnxt_pps2 index 2 func 0 chan 2
name bnxt_pps3 index 3 func 0 chan 3

without the change user cannot configure EXTTS or PEROUT function on pins
1-3 preserving channels 1-3 on them.

The user can actually use channel 0 on every pin because bnxt driver doesn't
care about channels at all, but it's a bit confusing that it sets up different
channels during init phase.

>>
>> supported_extts_flags and supported_perout_flags have to be added as
>> well to make the driver complaint with the latest API.
>>
>> Signed-off-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
>> ---
>>   drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
>> index db81cf6d5289..c9b7df669415 100644
>> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
>> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
>> @@ -965,10 +965,12 @@ static int bnxt_ptp_pps_init(struct bnxt *bp)
>>          hwrm_req_drop(bp, req);
>>
>>          /* Only 1 each of ext_ts and per_out pins is available in HW */
>> -       ptp_info->n_ext_ts = 1;
>> -       ptp_info->n_per_out = 1;
>> +       ptp_info->n_ext_ts = pps_info->num_pins;
>> +       ptp_info->n_per_out = pps_info->num_pins;
>>          ptp_info->pps = 1;
>>          ptp_info->verify = bnxt_ptp_verify;
>> +       ptp_info->supported_extts_flags = PTP_RISING_EDGE | PTP_STRICT_FLAGS;
>> +       ptp_info->supported_perout_flags = PTP_PEROUT_DUTY_CYCLE;
>>
>>          return 0;
>>   }
>> --
>> 2.47.3
>>

Re: [PATCH net-next] bnxt_en: support PPS in/out on all pins

[Pavan Chebbi]

[-- Attachment #1: Type: text/plain, Size: 1736 bytes --]

On Fri, Oct 17, 2025 at 2:21 PM Vadim Fedorenko
<vadim.fedorenko@linux.dev> wrote:
>
> On 17.10.2025 04:45, Pavan Chebbi wrote:
> > On Fri, Oct 17, 2025 at 3:54 AM Vadim Fedorenko
> > <vadim.fedorenko@linux.dev> wrote:
> >>
> >> n_ext_ts and n_per_out from ptp_clock_caps are checked as a max number
> >> of pins rather than max number of active pins.
> >
> > I am not 100pc sure. How is n_pins going to be different then?
> > https://elixir.bootlin.com/linux/v6.17/source/include/linux/ptp_clock_kernel.h#L69
>
> So in general it's more for the case where HW has pins connected through mux to
> the DPLL channels. According to the bnxt_ptp_cfg_pin() bnxt HW has pins
> hardwired to channels and NVM has pre-defined configuration of pins' functions.
>
> [host ~]# ./testptp -d /dev/ptp2 -l
> name bnxt_pps0 index 0 func 0 chan 0
> name bnxt_pps1 index 1 func 0 chan 1
> name bnxt_pps2 index 2 func 0 chan 2
> name bnxt_pps3 index 3 func 0 chan 3
>
> without the change user cannot configure EXTTS or PEROUT function on pins
> 1-3 preserving channels 1-3 on them.
>
> The user can actually use channel 0 on every pin because bnxt driver doesn't
> care about channels at all, but it's a bit confusing that it sets up different
> channels during init phase.

You are right that we don't care about the channels. So I think
ideally it should have been set to 0 for all the pins.
Does that not make a better fix? Meaning to say, we don't care about
the channel but/therefore please use 0 for all pins.
What I am not sure about the proposed change in your patch is that it
may be overriding the definition of the n_ext_ts and n_per_out in
order to provide flexibility to users to change channels, no?

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5469 bytes --]

Re: [PATCH net-next] bnxt_en: support PPS in/out on all pins

[Vadim Fedorenko]

On 17/10/2025 10:15, Pavan Chebbi wrote:
> On Fri, Oct 17, 2025 at 2:21 PM Vadim Fedorenko
> <vadim.fedorenko@linux.dev> wrote:
>>
>> On 17.10.2025 04:45, Pavan Chebbi wrote:
>>> On Fri, Oct 17, 2025 at 3:54 AM Vadim Fedorenko
>>> <vadim.fedorenko@linux.dev> wrote:
>>>>
>>>> n_ext_ts and n_per_out from ptp_clock_caps are checked as a max number
>>>> of pins rather than max number of active pins.
>>>
>>> I am not 100pc sure. How is n_pins going to be different then?
>>> https://elixir.bootlin.com/linux/v6.17/source/include/linux/ptp_clock_kernel.h#L69
>>
>> So in general it's more for the case where HW has pins connected through mux to
>> the DPLL channels. According to the bnxt_ptp_cfg_pin() bnxt HW has pins
>> hardwired to channels and NVM has pre-defined configuration of pins' functions.
>>
>> [host ~]# ./testptp -d /dev/ptp2 -l
>> name bnxt_pps0 index 0 func 0 chan 0
>> name bnxt_pps1 index 1 func 0 chan 1
>> name bnxt_pps2 index 2 func 0 chan 2
>> name bnxt_pps3 index 3 func 0 chan 3
>>
>> without the change user cannot configure EXTTS or PEROUT function on pins
>> 1-3 preserving channels 1-3 on them.
>>
>> The user can actually use channel 0 on every pin because bnxt driver doesn't
>> care about channels at all, but it's a bit confusing that it sets up different
>> channels during init phase.
> 
> You are right that we don't care about the channels. So I think
> ideally it should have been set to 0 for all the pins.
> Does that not make a better fix? Meaning to say, we don't care about
> the channel but/therefore please use 0 for all pins.
> What I am not sure about the proposed change in your patch is that it
> may be overriding the definition of the n_ext_ts and n_per_out in
> order to provide flexibility to users to change channels, no?

Well, yeah, the overriding exists, but that's mostly the artifact of not
so flexible API. But I agree, we can improve init part to make it clear.
But one more thing has just come to my mind - is it
really possible to configure PPS-in/PPS-out on pins 0-1?
AFAIU, there are functions assigned to each pin, which can only be
enabled or disabled by the user-space app, and in this case
bnxt_ptp_verify() should be improved to validate function per pin,
right?

Re: [PATCH net-next] bnxt_en: support PPS in/out on all pins

[Pavan Chebbi]

[-- Attachment #1: Type: text/plain, Size: 2844 bytes --]

On Fri, Oct 17, 2025 at 4:10 PM Vadim Fedorenko
<vadim.fedorenko@linux.dev> wrote:
>
> On 17/10/2025 10:15, Pavan Chebbi wrote:
> > On Fri, Oct 17, 2025 at 2:21 PM Vadim Fedorenko
> > <vadim.fedorenko@linux.dev> wrote:
> >>
> >> On 17.10.2025 04:45, Pavan Chebbi wrote:
> >>> On Fri, Oct 17, 2025 at 3:54 AM Vadim Fedorenko
> >>> <vadim.fedorenko@linux.dev> wrote:
> >>>>
> >>>> n_ext_ts and n_per_out from ptp_clock_caps are checked as a max number
> >>>> of pins rather than max number of active pins.
> >>>
> >>> I am not 100pc sure. How is n_pins going to be different then?
> >>> https://elixir.bootlin.com/linux/v6.17/source/include/linux/ptp_clock_kernel.h#L69
> >>
> >> So in general it's more for the case where HW has pins connected through mux to
> >> the DPLL channels. According to the bnxt_ptp_cfg_pin() bnxt HW has pins
> >> hardwired to channels and NVM has pre-defined configuration of pins' functions.
> >>
> >> [host ~]# ./testptp -d /dev/ptp2 -l
> >> name bnxt_pps0 index 0 func 0 chan 0
> >> name bnxt_pps1 index 1 func 0 chan 1
> >> name bnxt_pps2 index 2 func 0 chan 2
> >> name bnxt_pps3 index 3 func 0 chan 3
> >>
> >> without the change user cannot configure EXTTS or PEROUT function on pins
> >> 1-3 preserving channels 1-3 on them.
> >>
> >> The user can actually use channel 0 on every pin because bnxt driver doesn't
> >> care about channels at all, but it's a bit confusing that it sets up different
> >> channels during init phase.
> >
> > You are right that we don't care about the channels. So I think
> > ideally it should have been set to 0 for all the pins.
> > Does that not make a better fix? Meaning to say, we don't care about
> > the channel but/therefore please use 0 for all pins.
> > What I am not sure about the proposed change in your patch is that it
> > may be overriding the definition of the n_ext_ts and n_per_out in
> > order to provide flexibility to users to change channels, no?
>
> Well, yeah, the overriding exists, but that's mostly the artifact of not
> so flexible API. But I agree, we can improve init part to make it clear.
> But one more thing has just come to my mind - is it
> really possible to configure PPS-in/PPS-out on pins 0-1?
> AFAIU, there are functions assigned to each pin, which can only be
> enabled or disabled by the user-space app, and in this case
> bnxt_ptp_verify() should be improved to validate function per pin,
> right?
>
The pin config was really flexible because we implemented it first for
575xx chipsets. We could remap the functions on Thor1.
With 576xx, what you are saying is true. The pin functions are fixed.
If the user is aware of the functions, then it's not a problem.
But yes, because there is verify() available, we can always validate.
So we can improve the bnxt_ptp_verify()

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5469 bytes --]

Re: [PATCH net-next] bnxt_en: support PPS in/out on all pins

[Vadim Fedorenko]

On 17/10/2025 15:08, Pavan Chebbi wrote:
> On Fri, Oct 17, 2025 at 4:10 PM Vadim Fedorenko
> <vadim.fedorenko@linux.dev> wrote:
>>
>> On 17/10/2025 10:15, Pavan Chebbi wrote:
>>> On Fri, Oct 17, 2025 at 2:21 PM Vadim Fedorenko
>>> <vadim.fedorenko@linux.dev> wrote:
>>>>
>>>> On 17.10.2025 04:45, Pavan Chebbi wrote:
>>>>> On Fri, Oct 17, 2025 at 3:54 AM Vadim Fedorenko
>>>>> <vadim.fedorenko@linux.dev> wrote:
>>>>>>
>>>>>> n_ext_ts and n_per_out from ptp_clock_caps are checked as a max number
>>>>>> of pins rather than max number of active pins.
>>>>>
>>>>> I am not 100pc sure. How is n_pins going to be different then?
>>>>> https://elixir.bootlin.com/linux/v6.17/source/include/linux/ptp_clock_kernel.h#L69
>>>>
>>>> So in general it's more for the case where HW has pins connected through mux to
>>>> the DPLL channels. According to the bnxt_ptp_cfg_pin() bnxt HW has pins
>>>> hardwired to channels and NVM has pre-defined configuration of pins' functions.
>>>>
>>>> [host ~]# ./testptp -d /dev/ptp2 -l
>>>> name bnxt_pps0 index 0 func 0 chan 0
>>>> name bnxt_pps1 index 1 func 0 chan 1
>>>> name bnxt_pps2 index 2 func 0 chan 2
>>>> name bnxt_pps3 index 3 func 0 chan 3
>>>>
>>>> without the change user cannot configure EXTTS or PEROUT function on pins
>>>> 1-3 preserving channels 1-3 on them.
>>>>
>>>> The user can actually use channel 0 on every pin because bnxt driver doesn't
>>>> care about channels at all, but it's a bit confusing that it sets up different
>>>> channels during init phase.
>>>
>>> You are right that we don't care about the channels. So I think
>>> ideally it should have been set to 0 for all the pins.
>>> Does that not make a better fix? Meaning to say, we don't care about
>>> the channel but/therefore please use 0 for all pins.
>>> What I am not sure about the proposed change in your patch is that it
>>> may be overriding the definition of the n_ext_ts and n_per_out in
>>> order to provide flexibility to users to change channels, no?
>>
>> Well, yeah, the overriding exists, but that's mostly the artifact of not
>> so flexible API. But I agree, we can improve init part to make it clear.
>> But one more thing has just come to my mind - is it
>> really possible to configure PPS-in/PPS-out on pins 0-1?
>> AFAIU, there are functions assigned to each pin, which can only be
>> enabled or disabled by the user-space app, and in this case
>> bnxt_ptp_verify() should be improved to validate function per pin,
>> right?
>>
> The pin config was really flexible because we implemented it first for
> 575xx chipsets. We could remap the functions on Thor1.
> With 576xx, what you are saying is true. The pin functions are fixed.
> If the user is aware of the functions, then it's not a problem.
> But yes, because there is verify() available, we can always validate.
> So we can improve the bnxt_ptp_verify()

Ok, I'm going to send v2 with supported_flags provided and changing
initialization to set channels to 0 based on CHIP generation.

I'll let you do the improvements to bnxt_ptp_verify() as you have more
insights on the HW itself.