[PATCH net v2 1/1] net: fec: fix the PTP periodic output sysfs interface

[PATCH net v2 1/1] net: fec: fix the PTP periodic output sysfs interface

[Buday Csaba]

When the PPS channel configuration was implemented, the number of
supported periodic outputs (`n_per_out`) was left at 1.

This prohibits using channels 1..3 from the sysfs interface, since
period_store() rejects channel numbers greater than `n_per_out`.

Fix by increasing `n_per_out` to the number of channels supported
by the hardware.

Fixes: 566c2d83887f ("net: fec: make PPS channel configurable")
Signed-off-by: Buday Csaba <buday.csaba@prolan.hu>
---
 drivers/net/ethernet/freescale/fec_ptp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
index 4b7bad9a485d..1a7aa280e7f6 100644
--- a/drivers/net/ethernet/freescale/fec_ptp.c
+++ b/drivers/net/ethernet/freescale/fec_ptp.c
@@ -756,7 +756,7 @@ void fec_ptp_init(struct platform_device *pdev, int irq_idx)
 	fep->ptp_caps.max_adj = 250000000;
 	fep->ptp_caps.n_alarm = 0;
 	fep->ptp_caps.n_ext_ts = 0;
-	fep->ptp_caps.n_per_out = 1;
+	fep->ptp_caps.n_per_out = 4;
 	fep->ptp_caps.n_pins = 0;
 	fep->ptp_caps.pps = 1;
 	fep->ptp_caps.adjfine = fec_ptp_adjfine;

base-commit: 8a63baadf08453f66eb582fdb6dd234f72024723
-- 
2.39.5

RE: [PATCH net v2 1/1] net: fec: fix the PTP periodic output sysfs interface

[Wei Fang]

> When the PPS channel configuration was implemented, the number of
> supported periodic outputs (`n_per_out`) was left at 1.
> 
> This prohibits using channels 1..3 from the sysfs interface, since
> period_store() rejects channel numbers greater than `n_per_out`.
> 
> Fix by increasing `n_per_out` to the number of channels supported
> by the hardware.
> 
> Fixes: 566c2d83887f ("net: fec: make PPS channel configurable")
> Signed-off-by: Buday Csaba <buday.csaba@prolan.hu>
> ---
>  drivers/net/ethernet/freescale/fec_ptp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec_ptp.c
> b/drivers/net/ethernet/freescale/fec_ptp.c
> index 4b7bad9a485d..1a7aa280e7f6 100644
> --- a/drivers/net/ethernet/freescale/fec_ptp.c
> +++ b/drivers/net/ethernet/freescale/fec_ptp.c
> @@ -756,7 +756,7 @@ void fec_ptp_init(struct platform_device *pdev, int
> irq_idx)
>  	fep->ptp_caps.max_adj = 250000000;
>  	fep->ptp_caps.n_alarm = 0;
>  	fep->ptp_caps.n_ext_ts = 0;
> -	fep->ptp_caps.n_per_out = 1;
> +	fep->ptp_caps.n_per_out = 4;
>  	fep->ptp_caps.n_pins = 0;
>  	fep->ptp_caps.pps = 1;
>  	fep->ptp_caps.adjfine = fec_ptp_adjfine;
> 
> base-commit: 8a63baadf08453f66eb582fdb6dd234f72024723
> --
> 2.39.5
> 

Reviewed-by: Wei Fang <wei.fang@nxp.com>

Re: [PATCH net v2 1/1] net: fec: fix the PTP periodic output sysfs interface

[Jakub Kicinski]

On Mon, 23 Mar 2026 12:54:46 +0000 Wei Fang wrote:
> > When the PPS channel configuration was implemented, the number of
> > supported periodic outputs (`n_per_out`) was left at 1.
> > 
> > This prohibits using channels 1..3 from the sysfs interface, since
> > period_store() rejects channel numbers greater than `n_per_out`.
> > 
> > Fix by increasing `n_per_out` to the number of channels supported
> > by the hardware.
> > 
> > Fixes: 566c2d83887f ("net: fec: make PPS channel configurable")
> > Signed-off-by: Buday Csaba <buday.csaba@prolan.hu>
> > ---
> >  drivers/net/ethernet/freescale/fec_ptp.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/ethernet/freescale/fec_ptp.c
> > b/drivers/net/ethernet/freescale/fec_ptp.c
> > index 4b7bad9a485d..1a7aa280e7f6 100644
> > --- a/drivers/net/ethernet/freescale/fec_ptp.c
> > +++ b/drivers/net/ethernet/freescale/fec_ptp.c
> > @@ -756,7 +756,7 @@ void fec_ptp_init(struct platform_device *pdev, int
> > irq_idx)
> >  	fep->ptp_caps.max_adj = 250000000;
> >  	fep->ptp_caps.n_alarm = 0;
> >  	fep->ptp_caps.n_ext_ts = 0;
> > -	fep->ptp_caps.n_per_out = 1;
> > +	fep->ptp_caps.n_per_out = 4;
> >  	fep->ptp_caps.n_pins = 0;
> >  	fep->ptp_caps.pps = 1;
> >  	fep->ptp_caps.adjfine = fec_ptp_adjfine;
> > 
> Reviewed-by: Wei Fang <wei.fang@nxp.com>

I don't understand why you think that we should be exposing 4 channels
to the user when they can only use one. And have the user guess which
one should be programmed. Please explain (and Buday will have to update
the commit message), to me v1 looked like a much better fix.
-- 
pw-bot: cr

RE: [PATCH net v2 1/1] net: fec: fix the PTP periodic output sysfs interface

[Wei Fang]

> > > When the PPS channel configuration was implemented, the number of
> > > supported periodic outputs (`n_per_out`) was left at 1.
> > >
> > > This prohibits using channels 1..3 from the sysfs interface, since
> > > period_store() rejects channel numbers greater than `n_per_out`.
> > >
> > > Fix by increasing `n_per_out` to the number of channels supported
> > > by the hardware.
> > >
> > > Fixes: 566c2d83887f ("net: fec: make PPS channel configurable")
> > > Signed-off-by: Buday Csaba <buday.csaba@prolan.hu>
> > > ---
> > >  drivers/net/ethernet/freescale/fec_ptp.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/ethernet/freescale/fec_ptp.c
> > > b/drivers/net/ethernet/freescale/fec_ptp.c
> > > index 4b7bad9a485d..1a7aa280e7f6 100644
> > > --- a/drivers/net/ethernet/freescale/fec_ptp.c
> > > +++ b/drivers/net/ethernet/freescale/fec_ptp.c
> > > @@ -756,7 +756,7 @@ void fec_ptp_init(struct platform_device *pdev, int
> > > irq_idx)
> > >  	fep->ptp_caps.max_adj = 250000000;
> > >  	fep->ptp_caps.n_alarm = 0;
> > >  	fep->ptp_caps.n_ext_ts = 0;
> > > -	fep->ptp_caps.n_per_out = 1;
> > > +	fep->ptp_caps.n_per_out = 4;
> > >  	fep->ptp_caps.n_pins = 0;
> > >  	fep->ptp_caps.pps = 1;
> > >  	fep->ptp_caps.adjfine = fec_ptp_adjfine;
> > >
> > Reviewed-by: Wei Fang <wei.fang@nxp.com>
> 
> I don't understand why you think that we should be exposing 4 channels
> to the user when they can only use one. And have the user guess which
> one should be programmed. Please explain (and Buday will have to update
> the commit message), to me v1 looked like a much better fix.

As I explained in v1, I think the "channel index" parameter of
"/sys/class/ptp/ptp<N>/period" refers to the hardware channel number,
rather than the software mapping to the hardware channel. Although the
current driver only supports one channel for output, the channel index
should specify the correct hardware channel instead of a fixed 0. Perhaps
Richard could share his thoughts on the channel index parameter.

> --
> pw-bot: cr

Re: [PATCH net v2 1/1] net: fec: fix the PTP periodic output sysfs interface

[Jakub Kicinski]

On Tue, 24 Mar 2026 02:36:55 +0000 Wei Fang wrote:
> > > Reviewed-by: Wei Fang <wei.fang@nxp.com>  
> > 
> > I don't understand why you think that we should be exposing 4 channels
> > to the user when they can only use one. And have the user guess which
> > one should be programmed. Please explain (and Buday will have to update
> > the commit message), to me v1 looked like a much better fix.  
> 
> As I explained in v1, I think the "channel index" parameter of
> "/sys/class/ptp/ptp<N>/period" refers to the hardware channel number,
> rather than the software mapping to the hardware channel.

Can you cite any documentation or examples?
What would be the benefit of exposing the ID within the MAC IP (not
even the board) to the user? 
As I said the benefit of having just 1 is that user doesn't have 
to poke around to find out which one works.

> Although the current driver only supports one channel for output,
> the channel index should specify the correct hardware channel instead
> of a fixed 0. Perhaps Richard could share his thoughts on the channel
> index parameter.

RE: [PATCH net v2 1/1] net: fec: fix the PTP periodic output sysfs interface

[Wei Fang]

> > > I don't understand why you think that we should be exposing 4 channels
> > > to the user when they can only use one. And have the user guess which
> > > one should be programmed. Please explain (and Buday will have to update
> > > the commit message), to me v1 looked like a much better fix.
> >
> > As I explained in v1, I think the "channel index" parameter of
> > "/sys/class/ptp/ptp<N>/period" refers to the hardware channel number,
> > rather than the software mapping to the hardware channel.
> 
> Can you cite any documentation or examples?

There is no documentation that explicitly specifies whether it must be
the hardware channel or the software-defined channel. So this leads
to different people having different understandings of it.

> What would be the benefit of exposing the ID within the MAC IP (not
> even the board) to the user?

From my perspective, I expect the "channel index" to be consistent with
"fsl,pps-channel" property, otherwise this may confuse some users.

Hi Buday,

If you want to revert to the first version, I suggest removing the check.
Because the check in period_store() already guarantees that
req.perout.index must be 0.

Re: RE: [PATCH net v2 1/1] net: fec: fix the PTP periodic output sysfs interface

[Buday Csaba]

>>>> I don't understand why you think that we should be exposing 4 channels
>>>> to the user when they can only use one. And have the user guess which
>>>> one should be programmed. Please explain (and Buday will have to update
>>>> the commit message), to me v1 looked like a much better fix.
>>>
>>> As I explained in v1, I think the "channel index" parameter of
>>> "/sys/class/ptp/ptp<N>/period" refers to the hardware channel number,
>>> rather than the software mapping to the hardware channel.
>>
>> Can you cite any documentation or examples?
> 
> There is no documentation that explicitly specifies whether it must be
> the hardware channel or the software-defined channel. So this leads
> to different people having different understandings of it.
> 

What about updating the documentation first?
Like:
--- a/Documentation/ABI/testing/sysfs-ptp
+++ b/Documentation/ABI/testing/sysfs-ptp
@@ -59,7 +59,8 @@ Date:         September 2010
  Contact:       Richard Cochran <richardcochran@gmail.com>
  Description:
                 This file contains the number of programmable periodic
-               output channels offered by the PTP hardware clock.
+               output channels offered by the PTP hardware clock, and
+               supported by the driver.

  What:          /sys/class/ptp/ptp<N>/n_pins
  Date:          March 2014

Same for the period.

>> What would be the benefit of exposing the ID within the MAC IP (not
>> even the board) to the user?
> 
>  From my perspective, I expect the "channel index" to be consistent with
> "fsl,pps-channel" property, otherwise this may confuse some users.
> 
> Hi Buday,
> 
> If you want to revert to the first version, I suggest removing the check.
> Because the check in period_store() already guarantees that
> req.perout.index must be 0.
> 
> 

Okay, I will do that.