Messages in this thread | | | From | Shmuel Hazan <> | Subject | Re: [PATCH v2 2/3] net: mvpp2: tai: add extts support | Date | Tue, 18 Apr 2023 10:09:15 +0000 |
| |
On Tue, 2023-04-18 at 11:37 +0200, Horatiu Vultur wrote: > > The 04/17/2023 20:07, Shmuel Hazan wrote: > > > > This commit add support for capturing a timestamp in which the PTP_PULSE > > pin, received a signal. > > > > This feature is needed in order to synchronize multiple clocks in the > > same board, using tools like ts2phc from the linuxptp project. > > > > On the Armada 8040, this is the only way to do so as a result of > > multiple erattas with the PTP_PULSE_IN interface that was designed to > > synchronize the TAI on an external PPS signal (the errattas are > > FE-6856276, FE-7382160 from document MV-S501388-00). > > > > This patch introduces a pinctrl configuration "extts" that will be > > selected once the user had enabled extts, and then will be returned back > > to the "default" pinctrl config once it has been disabled. Additionally > > these configurations will be also used in any case that the user asks us > > to perform any action that involves "triggerering" the TAI subsystem, in > > order to avoid a case where the external trigger would trigger with the > > wrong action. > > > > This pinctrl mess is needed due to the fact that there is no way for us > > to distinguish betwee an external trigger (e.g. from the PTP_PULSE_IN > > pin) or an internal one, triggered by the registers. > > > > This feature has been tested on an Aramda > > 8040 based board, with linuxptp 3.1.1's ts2phc. > > It looks good to me, just one more questions bellow. > Reviewed-by: Horatiu Vultur <horatiu.vultur@microchip.com> > > > > > Signed-off-by: Shmuel Hazan <shmuel.h@siklu.com> > > --- > > .../net/ethernet/marvell/mvpp2/mvpp2_tai.c | 304 ++++++++++++++++-- > > 1 file changed, 273 insertions(+), 31 deletions(-) > > > > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_tai.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_tai.c > > index 2e3d43b1bac1..ff57075c6ebc 100644 > > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_tai.c > > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_tai.c > > @@ -3,8 +3,11 @@ > > * Marvell PP2.2 TAI support > > * > > * Note: > > - * Do NOT use the event capture support. > > - * Do Not even set the MPP muxes to allow PTP_EVENT_REQ to be used. > > + * In order to use the event capture support, please see the example > > + * in marvell,pp2.yaml. > > + * Do not manually (e.g. without pinctrl-1, as described in > > + * marvell,pp2.yaml) set the MPP muxes to allow PTP_EVENT_REQ to be > > + * used. > > * It will disrupt the operation of this driver, and there is nothing > > * that this driver can do to prevent that. Even using PTP_EVENT_REQ > > * as an output will be seen as a trigger input, which can't be masked. > > @@ -33,6 +36,8 @@ > > * Consequently, we support none of these. > > */ > > #include <linux/io.h> > > +#include <linux/pinctrl/consumer.h> > > +#include <linux/ptp_clock.h> > > #include <linux/ptp_clock_kernel.h> > > #include <linux/slab.h> > > > > @@ -53,6 +58,10 @@ > > #define TCSR_CAPTURE_1_VALID BIT(1) > > #define TCSR_CAPTURE_0_VALID BIT(0) > > > > +#define MVPP2_PINCTRL_EXTTS_STATE "extts" > > +#define MAX_PINS 1 > > +#define EXTTS_PERIOD_MS 95 > > How have you come with this 95 value?
Hi Horatiu,
Thanks for your review.
As for the 95 value, I just borrowed it from a similar driver (ptp_clockmatrix) that also employs a similar polling mechanism for extts support. It seems to work pretty well in testing so I had no reason to change it.
> > > <snip> > -- > /Horatiu
| |