| Message ID | 20190911061622.774006-2-felipe.balbi@linux.intel.com |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | [v4,1/2] PTP: introduce new versions of IOCTLs | expand |
On Wed, Sep 11, 2019 at 09:16:22AM +0300, Felipe Balbi wrote: > Some controllers allow for a one-shot output pulse, in contrast to > periodic output. Now that we have extensible versions of our IOCTLs, we > can finally make use of the 'flags' field to pass a bit telling driver > that if we want one-shot pulse output. > > Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> > --- > > Changes since v3: > - Remove bogus bitwise negation > > Changes since v2: > - Add _PEROUT_ to bit macro > > Changes since v1: > - remove comment from .flags field Reviewed-by: Richard Cochran <richardcochran@gmail.com> @davem, these two are good to go! Thanks, Richard
From: Richard Cochran <richardcochran@gmail.com> Date: Thu, 12 Sep 2019 09:56:09 -0700 > On Wed, Sep 11, 2019 at 09:16:22AM +0300, Felipe Balbi wrote: >> Some controllers allow for a one-shot output pulse, in contrast to >> periodic output. Now that we have extensible versions of our IOCTLs, we >> can finally make use of the 'flags' field to pass a bit telling driver >> that if we want one-shot pulse output. >> >> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> >> --- >> >> Changes since v3: >> - Remove bogus bitwise negation >> >> Changes since v2: >> - Add _PEROUT_ to bit macro >> >> Changes since v1: >> - remove comment from .flags field > > Reviewed-by: Richard Cochran <richardcochran@gmail.com> > > @davem, these two are good to go! Ok, thanks for reviewing.
From: Felipe Balbi <felipe.balbi@linux.intel.com> Date: Wed, 11 Sep 2019 09:16:22 +0300 > Some controllers allow for a one-shot output pulse, in contrast to > periodic output. Now that we have extensible versions of our IOCTLs, we > can finally make use of the 'flags' field to pass a bit telling driver > that if we want one-shot pulse output. > > Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> Applied.
> -----Original Message----- > From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On > Behalf Of Felipe Balbi > Sent: Tuesday, September 10, 2019 11:16 PM > To: Richard Cochran <richardcochran@gmail.com> > Cc: Hall, Christopher S <christopher.s.hall@intel.com>; netdev@vger.kernel.org; > linux-kernel@vger.kernel.org; Felipe Balbi <felipe.balbi@linux.intel.com> > Subject: [PATCH v4 2/2] PTP: add support for one-shot output > > Some controllers allow for a one-shot output pulse, in contrast to > periodic output. Now that we have extensible versions of our IOCTLs, we > can finally make use of the 'flags' field to pass a bit telling driver > that if we want one-shot pulse output. > > Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> > --- > > Changes since v3: > - Remove bogus bitwise negation > > Changes since v2: > - Add _PEROUT_ to bit macro > > Changes since v1: > - remove comment from .flags field > > include/uapi/linux/ptp_clock.h | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/include/uapi/linux/ptp_clock.h b/include/uapi/linux/ptp_clock.h > index 9a0af3511b68..f16301015949 100644 > --- a/include/uapi/linux/ptp_clock.h > +++ b/include/uapi/linux/ptp_clock.h > @@ -38,8 +38,8 @@ > /* > * Bits of the ptp_perout_request.flags field: > */ > -#define PTP_PEROUT_VALID_FLAGS (0) > - > +#define PTP_PEROUT_ONE_SHOT (1<<0) > +#define PTP_PEROUT_VALID_FLAGS (PTP_PEROUT_ONE_SHOT) > /* > * struct ptp_clock_time - represents a time value > * > @@ -77,7 +77,7 @@ struct ptp_perout_request { > struct ptp_clock_time start; /* Absolute start time. */ > struct ptp_clock_time period; /* Desired period, zero means disable. */ > unsigned int index; /* Which channel to configure. */ > - unsigned int flags; /* Reserved for future use. */ > + unsigned int flags; > unsigned int rsv[4]; /* Reserved for future use. */ > }; > > -- > 2.23.0 Hi Felipe, Do you have any examples for how you envision using this? I don't see any drivers or other code on the list for doing so. Additionally, it seems weird because we do not have support for specifying the pulse width. I guess you leave that up to driver choice? Thanks, Jake
> -----Original Message----- > From: Keller, Jacob E > Sent: Tuesday, September 24, 2019 12:23 PM > To: Felipe Balbi <felipe.balbi@linux.intel.com>; Richard Cochran > <richardcochran@gmail.com> > Cc: Hall, Christopher S <christopher.s.hall@intel.com>; > netdev@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: RE: [PATCH v4 2/2] PTP: add support for one-shot output > > > > > -----Original Message----- > > From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] > On > > Behalf Of Felipe Balbi > > Sent: Tuesday, September 10, 2019 11:16 PM > > To: Richard Cochran <richardcochran@gmail.com> > > Cc: Hall, Christopher S <christopher.s.hall@intel.com>; > netdev@vger.kernel.org; > > linux-kernel@vger.kernel.org; Felipe Balbi > <felipe.balbi@linux.intel.com> > > Subject: [PATCH v4 2/2] PTP: add support for one-shot output > > > > Some controllers allow for a one-shot output pulse, in contrast to > > periodic output. Now that we have extensible versions of our IOCTLs, we > > can finally make use of the 'flags' field to pass a bit telling driver > > that if we want one-shot pulse output. > > > > Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> > > --- > > > > Changes since v3: > > - Remove bogus bitwise negation > > > > Changes since v2: > > - Add _PEROUT_ to bit macro > > > > Changes since v1: > > - remove comment from .flags field > > > > include/uapi/linux/ptp_clock.h | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/include/uapi/linux/ptp_clock.h > b/include/uapi/linux/ptp_clock.h > > index 9a0af3511b68..f16301015949 100644 > > --- a/include/uapi/linux/ptp_clock.h > > +++ b/include/uapi/linux/ptp_clock.h > > @@ -38,8 +38,8 @@ > > /* > > * Bits of the ptp_perout_request.flags field: > > */ > > -#define PTP_PEROUT_VALID_FLAGS (0) > > - > > +#define PTP_PEROUT_ONE_SHOT (1<<0) > > +#define PTP_PEROUT_VALID_FLAGS (PTP_PEROUT_ONE_SHOT) > > /* > > * struct ptp_clock_time - represents a time value > > * > > @@ -77,7 +77,7 @@ struct ptp_perout_request { > > struct ptp_clock_time start; /* Absolute start time. */ > > struct ptp_clock_time period; /* Desired period, zero means disable. > */ > > unsigned int index; /* Which channel to configure. */ > > - unsigned int flags; /* Reserved for future use. */ > > + unsigned int flags; > > unsigned int rsv[4]; /* Reserved for future use. */ > > }; > > > > -- > > 2.23.0 > > Hi Felipe, > > Do you have any examples for how you envision using this? I don't see any > drivers or other code on the list for doing so. > > Additionally, it seems weird because we do not have support for specifying > the pulse width. I guess you leave that up to driver choice? > > Thanks, > Jake Jake, Good catch on the terminology. This is an API that produces edges not pulses. This flag causes the PEROUT ioctl to ignore the period argument and produce a single edge. Currently, the igb driver implements the same function, but uses a "magic" invalid period specification to signal that the period argument should be ignored (use_freq == 0): if (on && ((ns <= 70000000LL) || (ns == 125000000LL) || (ns == 250000000LL) || (ns == 500000000LL))) { if (ns < 8LL) return -EINVAL; use_freq = 1; } The proposal is to support this function without magic period specifications using an explicit flag instead. An example use case is pulse-per-second output. While PPS is periodic, time-aware GPIO is driven by (an unadjustable) Always Running Timer (ART). It's necessary to schedule each edge in software to produce PPS synced with system time. Chris
> -----Original Message----- > From: Hall, Christopher S > Sent: Tuesday, September 24, 2019 1:24 PM > To: Keller, Jacob E <jacob.e.keller@intel.com>; Felipe Balbi > <felipe.balbi@linux.intel.com>; Richard Cochran <richardcochran@gmail.com> > Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: RE: [PATCH v4 2/2] PTP: add support for one-shot output > > Good catch on the terminology. This is an API that produces edges not pulses. > This flag causes the PEROUT ioctl to ignore the period argument and produce a > single edge. Currently, the igb driver implements the same function, but uses > a "magic" invalid period specification to signal that the period argument > should be ignored (use_freq == 0): > > if (on && ((ns <= 70000000LL) || (ns == 125000000LL) || > (ns == 250000000LL) || (ns == 500000000LL))) { > if (ns < 8LL) > return -EINVAL; > use_freq = 1; > } From my understanding, the use_freq = 0 is intended to perform a clock using the target time registers with an interrupt to re-trigger the next toggle. If you use a frequency not supported by freqout, it will result in an interrupt that re-toggles the target time, not a single edge. > > The proposal is to support this function without magic period specifications > using an explicit flag instead. An example use case is pulse-per-second > output. While PPS is periodic, time-aware GPIO is driven by (an > unadjustable) Always Running Timer (ART). It's necessary to schedule each > edge in software to produce PPS synced with system time. > > Chris Oh, so "one shot" will simply toggle the clock output once. I see. So this won't really work for generating a pulse per second, and we would possibly still want an API for that? Thanks, Jake
> -----Original Message----- > From: Hall, Christopher S > Sent: Tuesday, September 24, 2019 1:24 PM > To: Keller, Jacob E <jacob.e.keller@intel.com>; Felipe Balbi > <felipe.balbi@linux.intel.com>; Richard Cochran <richardcochran@gmail.com> > Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: RE: [PATCH v4 2/2] PTP: add support for one-shot output > > > -----Original Message----- > > From: Keller, Jacob E > > Sent: Tuesday, September 24, 2019 12:23 PM > > To: Felipe Balbi <felipe.balbi@linux.intel.com>; Richard Cochran > > <richardcochran@gmail.com> > > Cc: Hall, Christopher S <christopher.s.hall@intel.com>; > > netdev@vger.kernel.org; linux-kernel@vger.kernel.org > > Subject: RE: [PATCH v4 2/2] PTP: add support for one-shot output > > > > > > > > > -----Original Message----- > > > From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] > > On > > > Behalf Of Felipe Balbi > > > Sent: Tuesday, September 10, 2019 11:16 PM > > > To: Richard Cochran <richardcochran@gmail.com> > > > Cc: Hall, Christopher S <christopher.s.hall@intel.com>; > > netdev@vger.kernel.org; > > > linux-kernel@vger.kernel.org; Felipe Balbi > > <felipe.balbi@linux.intel.com> > > > Subject: [PATCH v4 2/2] PTP: add support for one-shot output > > > > > > Some controllers allow for a one-shot output pulse, in contrast to > > > periodic output. Now that we have extensible versions of our IOCTLs, we > > > can finally make use of the 'flags' field to pass a bit telling driver > > > that if we want one-shot pulse output. > > > > > > Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> > > > --- > > > > > > Changes since v3: > > > - Remove bogus bitwise negation > > > > > > Changes since v2: > > > - Add _PEROUT_ to bit macro > > > > > > Changes since v1: > > > - remove comment from .flags field > > > > > > include/uapi/linux/ptp_clock.h | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/include/uapi/linux/ptp_clock.h > > b/include/uapi/linux/ptp_clock.h > > > index 9a0af3511b68..f16301015949 100644 > > > --- a/include/uapi/linux/ptp_clock.h > > > +++ b/include/uapi/linux/ptp_clock.h > > > @@ -38,8 +38,8 @@ > > > /* > > > * Bits of the ptp_perout_request.flags field: > > > */ > > > -#define PTP_PEROUT_VALID_FLAGS (0) > > > - > > > +#define PTP_PEROUT_ONE_SHOT (1<<0) > > > +#define PTP_PEROUT_VALID_FLAGS (PTP_PEROUT_ONE_SHOT) > > > /* > > > * struct ptp_clock_time - represents a time value > > > * > > > @@ -77,7 +77,7 @@ struct ptp_perout_request { > > > struct ptp_clock_time start; /* Absolute start time. */ > > > struct ptp_clock_time period; /* Desired period, zero means disable. > > */ > > > unsigned int index; /* Which channel to configure. */ > > > - unsigned int flags; /* Reserved for future use. */ > > > + unsigned int flags; > > > unsigned int rsv[4]; /* Reserved for future use. */ > > > }; > > > > > > -- > > > 2.23.0 > > > > Hi Felipe, > > > > Do you have any examples for how you envision using this? I don't see any > > drivers or other code on the list for doing so. > > > > Additionally, it seems weird because we do not have support for specifying > > the pulse width. I guess you leave that up to driver choice? > > > > Thanks, > > Jake > Also a quick note/question: Is there a spot where flags are explicitly checked and rejected? I don't see any driver which would reject this as "not an acceptable configuration". I.e. if a function calls the PEROUT_REQUEST2 ioctl, they will pass the flag through, and drivers today don't seem to bother checking flags at all. I think we also need a patch so that all drivers are updated to reject non-zero flags, ensuring that they do not attempt to configure a request incorrectly. Thanks, Jake
diff --git a/include/uapi/linux/ptp_clock.h b/include/uapi/linux/ptp_clock.h index 9a0af3511b68..f16301015949 100644 --- a/include/uapi/linux/ptp_clock.h +++ b/include/uapi/linux/ptp_clock.h @@ -38,8 +38,8 @@ /* * Bits of the ptp_perout_request.flags field: */ -#define PTP_PEROUT_VALID_FLAGS (0) - +#define PTP_PEROUT_ONE_SHOT (1<<0) +#define PTP_PEROUT_VALID_FLAGS (PTP_PEROUT_ONE_SHOT) /* * struct ptp_clock_time - represents a time value * @@ -77,7 +77,7 @@ struct ptp_perout_request { struct ptp_clock_time start; /* Absolute start time. */ struct ptp_clock_time period; /* Desired period, zero means disable. */ unsigned int index; /* Which channel to configure. */ - unsigned int flags; /* Reserved for future use. */ + unsigned int flags; unsigned int rsv[4]; /* Reserved for future use. */ };
Some controllers allow for a one-shot output pulse, in contrast to periodic output. Now that we have extensible versions of our IOCTLs, we can finally make use of the 'flags' field to pass a bit telling driver that if we want one-shot pulse output. Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> --- Changes since v3: - Remove bogus bitwise negation Changes since v2: - Add _PEROUT_ to bit macro Changes since v1: - remove comment from .flags field include/uapi/linux/ptp_clock.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)