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@xxxxxxxxx>
> > > ---
> > >  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@xxxxxxx>
> 
> 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