| Message ID | 20191226095851.24325-1-yangbo.lu@nxp.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | net: mscc: ocelot: support PPS signal generation | expand |
Hi Yangbo, On Thu, 26 Dec 2019 at 12:00, Yangbo Lu <yangbo.lu@nxp.com> wrote: > > This patch is to support PPS signal generation for Ocelot family > switches, including VSC9959 switch. > > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> > --- Shouldn't this be integrated with the .enable callback of the PTP core? > drivers/net/dsa/ocelot/felix_vsc9959.c | 2 ++ > drivers/net/ethernet/mscc/ocelot.c | 25 +++++++++++++++++++++++++ > drivers/net/ethernet/mscc/ocelot_ptp.h | 2 ++ > drivers/net/ethernet/mscc/ocelot_regs.c | 2 ++ > include/soc/mscc/ocelot.h | 2 ++ > 5 files changed, 33 insertions(+) > > diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c > index b9758b0..ee0ce7c 100644 > --- a/drivers/net/dsa/ocelot/felix_vsc9959.c > +++ b/drivers/net/dsa/ocelot/felix_vsc9959.c > @@ -287,6 +287,8 @@ static const u32 vsc9959_ptp_regmap[] = { > REG(PTP_PIN_TOD_SEC_MSB, 0x000004), > REG(PTP_PIN_TOD_SEC_LSB, 0x000008), > REG(PTP_PIN_TOD_NSEC, 0x00000c), > + REG(PTP_PIN_WF_HIGH_PERIOD, 0x000014), > + REG(PTP_PIN_WF_LOW_PERIOD, 0x000018), > REG(PTP_CFG_MISC, 0x0000a0), > REG(PTP_CLK_CFG_ADJ_CFG, 0x0000a4), > REG(PTP_CLK_CFG_ADJ_FREQ, 0x0000a8), > diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c > index 985b46d..c0f8a9e 100644 > --- a/drivers/net/ethernet/mscc/ocelot.c > +++ b/drivers/net/ethernet/mscc/ocelot.c > @@ -2147,6 +2147,29 @@ static struct ptp_clock_info ocelot_ptp_clock_info = { > .adjfine = ocelot_ptp_adjfine, > }; > > +static void ocelot_ptp_init_pps(struct ocelot *ocelot) > +{ > + u32 val; > + > + /* PPS signal generation uses CLOCK action. Together with SYNC option, > + * a single pulse will be generated after <WAFEFORM_LOW> nanoseconds > + * after the time of day has increased the seconds. The pulse will > + * get a width of <WAFEFORM_HIGH> nanoseconds. Also, I think what you have implemented here is periodic output (PTP_CLK_REQ_PEROUT) not PPS [input] (PTP_CLK_REQ_PPS). I have found the PTP documentation to be rather confusing on what PTP_CLK_REQ_PPS means, so I'm adding Richard in the hope that he may clarify (also what's different between PTP_CLK_REQ_PPS and PTP_CLK_REQ_PPS). > + * > + * In default, > + * WAFEFORM_LOW = 0 > + * WAFEFORM_HIGH = 1us > + */ > + ocelot_write_rix(ocelot, 0, PTP_PIN_WF_LOW_PERIOD, ALT_PPS_PIN); > + ocelot_write_rix(ocelot, 1000, PTP_PIN_WF_HIGH_PERIOD, ALT_PPS_PIN); > + > + val = ocelot_read_rix(ocelot, PTP_PIN_CFG, ALT_PPS_PIN); > + val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK | PTP_PIN_CFG_DOM); > + val |= (PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_CLOCK)); > + I suppose the reason why you didn't use ocelot_rmw_rix here is that it doesn't fit in 80 characters? Do you even need to read-modify-write? The only other field is the polarity bit which is by default 0 (active high) and non-configurable via the current API (struct ptp_pin_desc) as far as I can see. > + ocelot_write_rix(ocelot, val, PTP_PIN_CFG, ALT_PPS_PIN); > +} > + > static int ocelot_init_timestamp(struct ocelot *ocelot) > { > struct ptp_clock *ptp_clock; > @@ -2478,6 +2501,8 @@ int ocelot_init(struct ocelot *ocelot) > "Timestamp initialization failed\n"); > return ret; > } > + > + ocelot_ptp_init_pps(ocelot); > } > > return 0; > diff --git a/drivers/net/ethernet/mscc/ocelot_ptp.h b/drivers/net/ethernet/mscc/ocelot_ptp.h > index 9ede14a..21bc744 100644 > --- a/drivers/net/ethernet/mscc/ocelot_ptp.h > +++ b/drivers/net/ethernet/mscc/ocelot_ptp.h > @@ -13,6 +13,8 @@ > #define PTP_PIN_TOD_SEC_MSB_RSZ PTP_PIN_CFG_RSZ > #define PTP_PIN_TOD_SEC_LSB_RSZ PTP_PIN_CFG_RSZ > #define PTP_PIN_TOD_NSEC_RSZ PTP_PIN_CFG_RSZ > +#define PTP_PIN_WF_HIGH_PERIOD_RSZ PTP_PIN_CFG_RSZ > +#define PTP_PIN_WF_LOW_PERIOD_RSZ PTP_PIN_CFG_RSZ > > #define PTP_PIN_CFG_DOM BIT(0) > #define PTP_PIN_CFG_SYNC BIT(2) > diff --git a/drivers/net/ethernet/mscc/ocelot_regs.c b/drivers/net/ethernet/mscc/ocelot_regs.c > index b88b589..ed4dd01 100644 > --- a/drivers/net/ethernet/mscc/ocelot_regs.c > +++ b/drivers/net/ethernet/mscc/ocelot_regs.c > @@ -239,6 +239,8 @@ static const u32 ocelot_ptp_regmap[] = { > REG(PTP_PIN_TOD_SEC_MSB, 0x000004), > REG(PTP_PIN_TOD_SEC_LSB, 0x000008), > REG(PTP_PIN_TOD_NSEC, 0x00000c), > + REG(PTP_PIN_WF_HIGH_PERIOD, 0x000014), > + REG(PTP_PIN_WF_LOW_PERIOD, 0x000018), > REG(PTP_CFG_MISC, 0x0000a0), > REG(PTP_CLK_CFG_ADJ_CFG, 0x0000a4), > REG(PTP_CLK_CFG_ADJ_FREQ, 0x0000a8), > diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h > index 64cbbbe..c2ab20d 100644 > --- a/include/soc/mscc/ocelot.h > +++ b/include/soc/mscc/ocelot.h > @@ -325,6 +325,8 @@ enum ocelot_reg { > PTP_PIN_TOD_SEC_MSB, > PTP_PIN_TOD_SEC_LSB, > PTP_PIN_TOD_NSEC, > + PTP_PIN_WF_HIGH_PERIOD, > + PTP_PIN_WF_LOW_PERIOD, > PTP_CFG_MISC, > PTP_CLK_CFG_ADJ_CFG, > PTP_CLK_CFG_ADJ_FREQ, > -- > 2.7.4 > Thanks, -Vladimir
On Thu, 26 Dec 2019 at 12:49, Vladimir Oltean <olteanv@gmail.com> wrote: > > Hi Yangbo, > > On Thu, 26 Dec 2019 at 12:00, Yangbo Lu <yangbo.lu@nxp.com> wrote: > > > > This patch is to support PPS signal generation for Ocelot family > > switches, including VSC9959 switch. > > > > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> > > --- > > Shouldn't this be integrated with the .enable callback of the PTP core? > > > drivers/net/dsa/ocelot/felix_vsc9959.c | 2 ++ > > drivers/net/ethernet/mscc/ocelot.c | 25 +++++++++++++++++++++++++ > > drivers/net/ethernet/mscc/ocelot_ptp.h | 2 ++ > > drivers/net/ethernet/mscc/ocelot_regs.c | 2 ++ > > include/soc/mscc/ocelot.h | 2 ++ > > 5 files changed, 33 insertions(+) > > > > diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c > > index b9758b0..ee0ce7c 100644 > > --- a/drivers/net/dsa/ocelot/felix_vsc9959.c > > +++ b/drivers/net/dsa/ocelot/felix_vsc9959.c > > @@ -287,6 +287,8 @@ static const u32 vsc9959_ptp_regmap[] = { > > REG(PTP_PIN_TOD_SEC_MSB, 0x000004), > > REG(PTP_PIN_TOD_SEC_LSB, 0x000008), > > REG(PTP_PIN_TOD_NSEC, 0x00000c), > > + REG(PTP_PIN_WF_HIGH_PERIOD, 0x000014), > > + REG(PTP_PIN_WF_LOW_PERIOD, 0x000018), > > REG(PTP_CFG_MISC, 0x0000a0), > > REG(PTP_CLK_CFG_ADJ_CFG, 0x0000a4), > > REG(PTP_CLK_CFG_ADJ_FREQ, 0x0000a8), > > diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c > > index 985b46d..c0f8a9e 100644 > > --- a/drivers/net/ethernet/mscc/ocelot.c > > +++ b/drivers/net/ethernet/mscc/ocelot.c > > @@ -2147,6 +2147,29 @@ static struct ptp_clock_info ocelot_ptp_clock_info = { > > .adjfine = ocelot_ptp_adjfine, > > }; > > > > +static void ocelot_ptp_init_pps(struct ocelot *ocelot) > > +{ > > + u32 val; > > + > > + /* PPS signal generation uses CLOCK action. Together with SYNC option, > > + * a single pulse will be generated after <WAFEFORM_LOW> nanoseconds > > + * after the time of day has increased the seconds. The pulse will > > + * get a width of <WAFEFORM_HIGH> nanoseconds. > > Also, I think what you have implemented here is periodic output > (PTP_CLK_REQ_PEROUT) not PPS [input] (PTP_CLK_REQ_PPS). I have found > the PTP documentation to be rather confusing on what PTP_CLK_REQ_PPS > means, so I'm adding Richard in the hope that he may clarify (also > what's different between PTP_CLK_REQ_PPS and PTP_CLK_REQ_PPS). EXTTS, sorry, obviously nothing is different between X and itself. > > > + * > > + * In default, > > + * WAFEFORM_LOW = 0 > > + * WAFEFORM_HIGH = 1us > > + */ > > + ocelot_write_rix(ocelot, 0, PTP_PIN_WF_LOW_PERIOD, ALT_PPS_PIN); > > + ocelot_write_rix(ocelot, 1000, PTP_PIN_WF_HIGH_PERIOD, ALT_PPS_PIN); > > + > > + val = ocelot_read_rix(ocelot, PTP_PIN_CFG, ALT_PPS_PIN); > > + val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK | PTP_PIN_CFG_DOM); > > + val |= (PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_CLOCK)); > > + > > I suppose the reason why you didn't use ocelot_rmw_rix here is that it > doesn't fit in 80 characters? > Do you even need to read-modify-write? The only other field is the > polarity bit which is by default 0 (active high) and non-configurable > via the current API (struct ptp_pin_desc) as far as I can see. > > > + ocelot_write_rix(ocelot, val, PTP_PIN_CFG, ALT_PPS_PIN); > > +} > > + > > static int ocelot_init_timestamp(struct ocelot *ocelot) > > { > > struct ptp_clock *ptp_clock; > > @@ -2478,6 +2501,8 @@ int ocelot_init(struct ocelot *ocelot) > > "Timestamp initialization failed\n"); > > return ret; > > } > > + > > + ocelot_ptp_init_pps(ocelot); > > } > > > > return 0; > > diff --git a/drivers/net/ethernet/mscc/ocelot_ptp.h b/drivers/net/ethernet/mscc/ocelot_ptp.h > > index 9ede14a..21bc744 100644 > > --- a/drivers/net/ethernet/mscc/ocelot_ptp.h > > +++ b/drivers/net/ethernet/mscc/ocelot_ptp.h > > @@ -13,6 +13,8 @@ > > #define PTP_PIN_TOD_SEC_MSB_RSZ PTP_PIN_CFG_RSZ > > #define PTP_PIN_TOD_SEC_LSB_RSZ PTP_PIN_CFG_RSZ > > #define PTP_PIN_TOD_NSEC_RSZ PTP_PIN_CFG_RSZ > > +#define PTP_PIN_WF_HIGH_PERIOD_RSZ PTP_PIN_CFG_RSZ > > +#define PTP_PIN_WF_LOW_PERIOD_RSZ PTP_PIN_CFG_RSZ > > > > #define PTP_PIN_CFG_DOM BIT(0) > > #define PTP_PIN_CFG_SYNC BIT(2) > > diff --git a/drivers/net/ethernet/mscc/ocelot_regs.c b/drivers/net/ethernet/mscc/ocelot_regs.c > > index b88b589..ed4dd01 100644 > > --- a/drivers/net/ethernet/mscc/ocelot_regs.c > > +++ b/drivers/net/ethernet/mscc/ocelot_regs.c > > @@ -239,6 +239,8 @@ static const u32 ocelot_ptp_regmap[] = { > > REG(PTP_PIN_TOD_SEC_MSB, 0x000004), > > REG(PTP_PIN_TOD_SEC_LSB, 0x000008), > > REG(PTP_PIN_TOD_NSEC, 0x00000c), > > + REG(PTP_PIN_WF_HIGH_PERIOD, 0x000014), > > + REG(PTP_PIN_WF_LOW_PERIOD, 0x000018), > > REG(PTP_CFG_MISC, 0x0000a0), > > REG(PTP_CLK_CFG_ADJ_CFG, 0x0000a4), > > REG(PTP_CLK_CFG_ADJ_FREQ, 0x0000a8), > > diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h > > index 64cbbbe..c2ab20d 100644 > > --- a/include/soc/mscc/ocelot.h > > +++ b/include/soc/mscc/ocelot.h > > @@ -325,6 +325,8 @@ enum ocelot_reg { > > PTP_PIN_TOD_SEC_MSB, > > PTP_PIN_TOD_SEC_LSB, > > PTP_PIN_TOD_NSEC, > > + PTP_PIN_WF_HIGH_PERIOD, > > + PTP_PIN_WF_LOW_PERIOD, > > PTP_CFG_MISC, > > PTP_CLK_CFG_ADJ_CFG, > > PTP_CLK_CFG_ADJ_FREQ, > > -- > > 2.7.4 > > > > Thanks, > -Vladimir
On Thu, Dec 26, 2019 at 05:58:51PM +0800, Yangbo Lu wrote: > This patch is to support PPS signal generation for Ocelot family > switches, including VSC9959 switch. Hi Yangbo Please always Cc: Richard Cochran <richardcochran@gmail.com> for ptp patches. Andrew > > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> > --- > drivers/net/dsa/ocelot/felix_vsc9959.c | 2 ++ > drivers/net/ethernet/mscc/ocelot.c | 25 +++++++++++++++++++++++++ > drivers/net/ethernet/mscc/ocelot_ptp.h | 2 ++ > drivers/net/ethernet/mscc/ocelot_regs.c | 2 ++ > include/soc/mscc/ocelot.h | 2 ++ > 5 files changed, 33 insertions(+) > > diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c > index b9758b0..ee0ce7c 100644 > --- a/drivers/net/dsa/ocelot/felix_vsc9959.c > +++ b/drivers/net/dsa/ocelot/felix_vsc9959.c > @@ -287,6 +287,8 @@ static const u32 vsc9959_ptp_regmap[] = { > REG(PTP_PIN_TOD_SEC_MSB, 0x000004), > REG(PTP_PIN_TOD_SEC_LSB, 0x000008), > REG(PTP_PIN_TOD_NSEC, 0x00000c), > + REG(PTP_PIN_WF_HIGH_PERIOD, 0x000014), > + REG(PTP_PIN_WF_LOW_PERIOD, 0x000018), > REG(PTP_CFG_MISC, 0x0000a0), > REG(PTP_CLK_CFG_ADJ_CFG, 0x0000a4), > REG(PTP_CLK_CFG_ADJ_FREQ, 0x0000a8), > diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c > index 985b46d..c0f8a9e 100644 > --- a/drivers/net/ethernet/mscc/ocelot.c > +++ b/drivers/net/ethernet/mscc/ocelot.c > @@ -2147,6 +2147,29 @@ static struct ptp_clock_info ocelot_ptp_clock_info = { > .adjfine = ocelot_ptp_adjfine, > }; > > +static void ocelot_ptp_init_pps(struct ocelot *ocelot) > +{ > + u32 val; > + > + /* PPS signal generation uses CLOCK action. Together with SYNC option, > + * a single pulse will be generated after <WAFEFORM_LOW> nanoseconds > + * after the time of day has increased the seconds. The pulse will > + * get a width of <WAFEFORM_HIGH> nanoseconds. > + * > + * In default, > + * WAFEFORM_LOW = 0 > + * WAFEFORM_HIGH = 1us > + */ > + ocelot_write_rix(ocelot, 0, PTP_PIN_WF_LOW_PERIOD, ALT_PPS_PIN); > + ocelot_write_rix(ocelot, 1000, PTP_PIN_WF_HIGH_PERIOD, ALT_PPS_PIN); > + > + val = ocelot_read_rix(ocelot, PTP_PIN_CFG, ALT_PPS_PIN); > + val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK | PTP_PIN_CFG_DOM); > + val |= (PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_CLOCK)); > + > + ocelot_write_rix(ocelot, val, PTP_PIN_CFG, ALT_PPS_PIN); > +} > + > static int ocelot_init_timestamp(struct ocelot *ocelot) > { > struct ptp_clock *ptp_clock; > @@ -2478,6 +2501,8 @@ int ocelot_init(struct ocelot *ocelot) > "Timestamp initialization failed\n"); > return ret; > } > + > + ocelot_ptp_init_pps(ocelot); > } > > return 0; > diff --git a/drivers/net/ethernet/mscc/ocelot_ptp.h b/drivers/net/ethernet/mscc/ocelot_ptp.h > index 9ede14a..21bc744 100644 > --- a/drivers/net/ethernet/mscc/ocelot_ptp.h > +++ b/drivers/net/ethernet/mscc/ocelot_ptp.h > @@ -13,6 +13,8 @@ > #define PTP_PIN_TOD_SEC_MSB_RSZ PTP_PIN_CFG_RSZ > #define PTP_PIN_TOD_SEC_LSB_RSZ PTP_PIN_CFG_RSZ > #define PTP_PIN_TOD_NSEC_RSZ PTP_PIN_CFG_RSZ > +#define PTP_PIN_WF_HIGH_PERIOD_RSZ PTP_PIN_CFG_RSZ > +#define PTP_PIN_WF_LOW_PERIOD_RSZ PTP_PIN_CFG_RSZ > > #define PTP_PIN_CFG_DOM BIT(0) > #define PTP_PIN_CFG_SYNC BIT(2) > diff --git a/drivers/net/ethernet/mscc/ocelot_regs.c b/drivers/net/ethernet/mscc/ocelot_regs.c > index b88b589..ed4dd01 100644 > --- a/drivers/net/ethernet/mscc/ocelot_regs.c > +++ b/drivers/net/ethernet/mscc/ocelot_regs.c > @@ -239,6 +239,8 @@ static const u32 ocelot_ptp_regmap[] = { > REG(PTP_PIN_TOD_SEC_MSB, 0x000004), > REG(PTP_PIN_TOD_SEC_LSB, 0x000008), > REG(PTP_PIN_TOD_NSEC, 0x00000c), > + REG(PTP_PIN_WF_HIGH_PERIOD, 0x000014), > + REG(PTP_PIN_WF_LOW_PERIOD, 0x000018), > REG(PTP_CFG_MISC, 0x0000a0), > REG(PTP_CLK_CFG_ADJ_CFG, 0x0000a4), > REG(PTP_CLK_CFG_ADJ_FREQ, 0x0000a8), > diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h > index 64cbbbe..c2ab20d 100644 > --- a/include/soc/mscc/ocelot.h > +++ b/include/soc/mscc/ocelot.h > @@ -325,6 +325,8 @@ enum ocelot_reg { > PTP_PIN_TOD_SEC_MSB, > PTP_PIN_TOD_SEC_LSB, > PTP_PIN_TOD_NSEC, > + PTP_PIN_WF_HIGH_PERIOD, > + PTP_PIN_WF_LOW_PERIOD, > PTP_CFG_MISC, > PTP_CLK_CFG_ADJ_CFG, > PTP_CLK_CFG_ADJ_FREQ, > -- > 2.7.4 >
> -----Original Message----- > From: Vladimir Oltean <olteanv@gmail.com> > Sent: Thursday, December 26, 2019 6:49 PM > To: Y.b. Lu <yangbo.lu@nxp.com> > Cc: netdev <netdev@vger.kernel.org>; David S . Miller > <davem@davemloft.net>; Claudiu Manoil <claudiu.manoil@nxp.com>; > Vladimir Oltean <vladimir.oltean@nxp.com>; Alexandre Belloni > <alexandre.belloni@bootlin.com>; Microchip Linux Driver Support > <UNGLinuxDriver@microchip.com>; Richard Cochran > <richardcochran@gmail.com> > Subject: Re: [PATCH] net: mscc: ocelot: support PPS signal generation > > Hi Yangbo, > > On Thu, 26 Dec 2019 at 12:00, Yangbo Lu <yangbo.lu@nxp.com> wrote: > > > > This patch is to support PPS signal generation for Ocelot family > > switches, including VSC9959 switch. > > > > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> > > --- > > Shouldn't this be integrated with the .enable callback of the PTP core? The .enable callback is to enable PPS interrupt and to handle the event. Current patch is just to initialize PPS signal, so that we can observe the signal on board. > > > drivers/net/dsa/ocelot/felix_vsc9959.c | 2 ++ > > drivers/net/ethernet/mscc/ocelot.c | 25 > +++++++++++++++++++++++++ > > drivers/net/ethernet/mscc/ocelot_ptp.h | 2 ++ > > drivers/net/ethernet/mscc/ocelot_regs.c | 2 ++ > > include/soc/mscc/ocelot.h | 2 ++ > > 5 files changed, 33 insertions(+) > > > > diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c > b/drivers/net/dsa/ocelot/felix_vsc9959.c > > index b9758b0..ee0ce7c 100644 > > --- a/drivers/net/dsa/ocelot/felix_vsc9959.c > > +++ b/drivers/net/dsa/ocelot/felix_vsc9959.c > > @@ -287,6 +287,8 @@ static const u32 vsc9959_ptp_regmap[] = { > > REG(PTP_PIN_TOD_SEC_MSB, 0x000004), > > REG(PTP_PIN_TOD_SEC_LSB, 0x000008), > > REG(PTP_PIN_TOD_NSEC, 0x00000c), > > + REG(PTP_PIN_WF_HIGH_PERIOD, 0x000014), > > + REG(PTP_PIN_WF_LOW_PERIOD, 0x000018), > > REG(PTP_CFG_MISC, 0x0000a0), > > REG(PTP_CLK_CFG_ADJ_CFG, 0x0000a4), > > REG(PTP_CLK_CFG_ADJ_FREQ, 0x0000a8), > > diff --git a/drivers/net/ethernet/mscc/ocelot.c > b/drivers/net/ethernet/mscc/ocelot.c > > index 985b46d..c0f8a9e 100644 > > --- a/drivers/net/ethernet/mscc/ocelot.c > > +++ b/drivers/net/ethernet/mscc/ocelot.c > > @@ -2147,6 +2147,29 @@ static struct ptp_clock_info > ocelot_ptp_clock_info = { > > .adjfine = ocelot_ptp_adjfine, > > }; > > > > +static void ocelot_ptp_init_pps(struct ocelot *ocelot) > > +{ > > + u32 val; > > + > > + /* PPS signal generation uses CLOCK action. Together with SYNC > option, > > + * a single pulse will be generated after <WAFEFORM_LOW> > nanoseconds > > + * after the time of day has increased the seconds. The pulse will > > + * get a width of <WAFEFORM_HIGH> nanoseconds. > > Also, I think what you have implemented here is periodic output > (PTP_CLK_REQ_PEROUT) not PPS [input] (PTP_CLK_REQ_PPS). I have found > the PTP documentation to be rather confusing on what PTP_CLK_REQ_PPS > means, so I'm adding Richard in the hope that he may clarify (also > what's different between PTP_CLK_REQ_PPS and PTP_CLK_REQ_PPS). My understand is PTP_CLK_REQ_PEROUT is for periodical output, and PTP_CLK_REQ_PPS is for PPS event handling. This patch is to initialize PPS signal. You may check reference manual for how to generate PPS. "For the CLOCK action, the sync option makes the pin generate a single pulse, <WAFEFORM_LOW> nanoseconds after the time of day has increased the seconds. The pulse will get a width of <WAVEFORM_HIGH> nanoseconds. " If the sync option is not used, it is for periodical clock generation that you mentioned. > > > + * > > + * In default, > > + * WAFEFORM_LOW = 0 > > + * WAFEFORM_HIGH = 1us > > + */ > > + ocelot_write_rix(ocelot, 0, PTP_PIN_WF_LOW_PERIOD, > ALT_PPS_PIN); > > + ocelot_write_rix(ocelot, 1000, PTP_PIN_WF_HIGH_PERIOD, > ALT_PPS_PIN); > > + > > + val = ocelot_read_rix(ocelot, PTP_PIN_CFG, ALT_PPS_PIN); > > + val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK | > PTP_PIN_CFG_DOM); > > + val |= (PTP_PIN_CFG_SYNC | > PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_CLOCK)); > > + > > I suppose the reason why you didn't use ocelot_rmw_rix here is that it > doesn't fit in 80 characters? > Do you even need to read-modify-write? The only other field is the > polarity bit which is by default 0 (active high) and non-configurable > via the current API (struct ptp_pin_desc) as far as I can see. Some improvement is needed here. This is initialization. So I should only write an initial value into register, without reading. I will improve in new version patch. > > > + ocelot_write_rix(ocelot, val, PTP_PIN_CFG, ALT_PPS_PIN); > > +} > > + > > static int ocelot_init_timestamp(struct ocelot *ocelot) > > { > > struct ptp_clock *ptp_clock; > > @@ -2478,6 +2501,8 @@ int ocelot_init(struct ocelot *ocelot) > > "Timestamp initialization failed\n"); > > return ret; > > } > > + > > + ocelot_ptp_init_pps(ocelot); > > } > > > > return 0; > > diff --git a/drivers/net/ethernet/mscc/ocelot_ptp.h > b/drivers/net/ethernet/mscc/ocelot_ptp.h > > index 9ede14a..21bc744 100644 > > --- a/drivers/net/ethernet/mscc/ocelot_ptp.h > > +++ b/drivers/net/ethernet/mscc/ocelot_ptp.h > > @@ -13,6 +13,8 @@ > > #define PTP_PIN_TOD_SEC_MSB_RSZ PTP_PIN_CFG_RSZ > > #define PTP_PIN_TOD_SEC_LSB_RSZ PTP_PIN_CFG_RSZ > > #define PTP_PIN_TOD_NSEC_RSZ PTP_PIN_CFG_RSZ > > +#define PTP_PIN_WF_HIGH_PERIOD_RSZ PTP_PIN_CFG_RSZ > > +#define PTP_PIN_WF_LOW_PERIOD_RSZ PTP_PIN_CFG_RSZ > > > > #define PTP_PIN_CFG_DOM BIT(0) > > #define PTP_PIN_CFG_SYNC BIT(2) > > diff --git a/drivers/net/ethernet/mscc/ocelot_regs.c > b/drivers/net/ethernet/mscc/ocelot_regs.c > > index b88b589..ed4dd01 100644 > > --- a/drivers/net/ethernet/mscc/ocelot_regs.c > > +++ b/drivers/net/ethernet/mscc/ocelot_regs.c > > @@ -239,6 +239,8 @@ static const u32 ocelot_ptp_regmap[] = { > > REG(PTP_PIN_TOD_SEC_MSB, 0x000004), > > REG(PTP_PIN_TOD_SEC_LSB, 0x000008), > > REG(PTP_PIN_TOD_NSEC, 0x00000c), > > + REG(PTP_PIN_WF_HIGH_PERIOD, 0x000014), > > + REG(PTP_PIN_WF_LOW_PERIOD, 0x000018), > > REG(PTP_CFG_MISC, 0x0000a0), > > REG(PTP_CLK_CFG_ADJ_CFG, 0x0000a4), > > REG(PTP_CLK_CFG_ADJ_FREQ, 0x0000a8), > > diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h > > index 64cbbbe..c2ab20d 100644 > > --- a/include/soc/mscc/ocelot.h > > +++ b/include/soc/mscc/ocelot.h > > @@ -325,6 +325,8 @@ enum ocelot_reg { > > PTP_PIN_TOD_SEC_MSB, > > PTP_PIN_TOD_SEC_LSB, > > PTP_PIN_TOD_NSEC, > > + PTP_PIN_WF_HIGH_PERIOD, > > + PTP_PIN_WF_LOW_PERIOD, > > PTP_CFG_MISC, > > PTP_CLK_CFG_ADJ_CFG, > > PTP_CLK_CFG_ADJ_FREQ, > > -- > > 2.7.4 > > > > Thanks, > -Vladimir
> -----Original Message----- > From: Vladimir Oltean <olteanv@gmail.com> > Sent: Thursday, December 26, 2019 6:50 PM > To: Y.b. Lu <yangbo.lu@nxp.com> > Cc: netdev <netdev@vger.kernel.org>; David S . Miller > <davem@davemloft.net>; Claudiu Manoil <claudiu.manoil@nxp.com>; > Vladimir Oltean <vladimir.oltean@nxp.com>; Alexandre Belloni > <alexandre.belloni@bootlin.com>; Microchip Linux Driver Support > <UNGLinuxDriver@microchip.com>; Richard Cochran > <richardcochran@gmail.com> > Subject: Re: [PATCH] net: mscc: ocelot: support PPS signal generation > > On Thu, 26 Dec 2019 at 12:49, Vladimir Oltean <olteanv@gmail.com> wrote: > > > > Hi Yangbo, > > > > On Thu, 26 Dec 2019 at 12:00, Yangbo Lu <yangbo.lu@nxp.com> wrote: > > > > > > This patch is to support PPS signal generation for Ocelot family > > > switches, including VSC9959 switch. > > > > > > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> > > > --- > > > > Shouldn't this be integrated with the .enable callback of the PTP core? > > > > > drivers/net/dsa/ocelot/felix_vsc9959.c | 2 ++ > > > drivers/net/ethernet/mscc/ocelot.c | 25 > +++++++++++++++++++++++++ > > > drivers/net/ethernet/mscc/ocelot_ptp.h | 2 ++ > > > drivers/net/ethernet/mscc/ocelot_regs.c | 2 ++ > > > include/soc/mscc/ocelot.h | 2 ++ > > > 5 files changed, 33 insertions(+) > > > > > > diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c > b/drivers/net/dsa/ocelot/felix_vsc9959.c > > > index b9758b0..ee0ce7c 100644 > > > --- a/drivers/net/dsa/ocelot/felix_vsc9959.c > > > +++ b/drivers/net/dsa/ocelot/felix_vsc9959.c > > > @@ -287,6 +287,8 @@ static const u32 vsc9959_ptp_regmap[] = { > > > REG(PTP_PIN_TOD_SEC_MSB, 0x000004), > > > REG(PTP_PIN_TOD_SEC_LSB, 0x000008), > > > REG(PTP_PIN_TOD_NSEC, 0x00000c), > > > + REG(PTP_PIN_WF_HIGH_PERIOD, 0x000014), > > > + REG(PTP_PIN_WF_LOW_PERIOD, 0x000018), > > > REG(PTP_CFG_MISC, 0x0000a0), > > > REG(PTP_CLK_CFG_ADJ_CFG, 0x0000a4), > > > REG(PTP_CLK_CFG_ADJ_FREQ, 0x0000a8), > > > diff --git a/drivers/net/ethernet/mscc/ocelot.c > b/drivers/net/ethernet/mscc/ocelot.c > > > index 985b46d..c0f8a9e 100644 > > > --- a/drivers/net/ethernet/mscc/ocelot.c > > > +++ b/drivers/net/ethernet/mscc/ocelot.c > > > @@ -2147,6 +2147,29 @@ static struct ptp_clock_info > ocelot_ptp_clock_info = { > > > .adjfine = ocelot_ptp_adjfine, > > > }; > > > > > > +static void ocelot_ptp_init_pps(struct ocelot *ocelot) > > > +{ > > > + u32 val; > > > + > > > + /* PPS signal generation uses CLOCK action. Together with SYNC > option, > > > + * a single pulse will be generated after <WAFEFORM_LOW> > nanoseconds > > > + * after the time of day has increased the seconds. The pulse will > > > + * get a width of <WAFEFORM_HIGH> nanoseconds. > > > > Also, I think what you have implemented here is periodic output > > (PTP_CLK_REQ_PEROUT) not PPS [input] (PTP_CLK_REQ_PPS). I have found > > the PTP documentation to be rather confusing on what PTP_CLK_REQ_PPS > > means, so I'm adding Richard in the hope that he may clarify (also > > what's different between PTP_CLK_REQ_PPS and PTP_CLK_REQ_PPS). > > EXTTS, sorry, obviously nothing is different between X and itself. > EXTTS is for capturing timestamps for external trigger pin which is input signal. > > > > > + * > > > + * In default, > > > + * WAFEFORM_LOW = 0 > > > + * WAFEFORM_HIGH = 1us > > > + */ > > > + ocelot_write_rix(ocelot, 0, PTP_PIN_WF_LOW_PERIOD, > ALT_PPS_PIN); > > > + ocelot_write_rix(ocelot, 1000, PTP_PIN_WF_HIGH_PERIOD, > ALT_PPS_PIN); > > > + > > > + val = ocelot_read_rix(ocelot, PTP_PIN_CFG, ALT_PPS_PIN); > > > + val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK | > PTP_PIN_CFG_DOM); > > > + val |= (PTP_PIN_CFG_SYNC | > PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_CLOCK)); > > > + > > > > I suppose the reason why you didn't use ocelot_rmw_rix here is that it > > doesn't fit in 80 characters? > > Do you even need to read-modify-write? The only other field is the > > polarity bit which is by default 0 (active high) and non-configurable > > via the current API (struct ptp_pin_desc) as far as I can see. > > > > > + ocelot_write_rix(ocelot, val, PTP_PIN_CFG, ALT_PPS_PIN); > > > +} > > > + > > > static int ocelot_init_timestamp(struct ocelot *ocelot) > > > { > > > struct ptp_clock *ptp_clock; > > > @@ -2478,6 +2501,8 @@ int ocelot_init(struct ocelot *ocelot) > > > "Timestamp initialization failed\n"); > > > return ret; > > > } > > > + > > > + ocelot_ptp_init_pps(ocelot); > > > } > > > > > > return 0; > > > diff --git a/drivers/net/ethernet/mscc/ocelot_ptp.h > b/drivers/net/ethernet/mscc/ocelot_ptp.h > > > index 9ede14a..21bc744 100644 > > > --- a/drivers/net/ethernet/mscc/ocelot_ptp.h > > > +++ b/drivers/net/ethernet/mscc/ocelot_ptp.h > > > @@ -13,6 +13,8 @@ > > > #define PTP_PIN_TOD_SEC_MSB_RSZ > PTP_PIN_CFG_RSZ > > > #define PTP_PIN_TOD_SEC_LSB_RSZ PTP_PIN_CFG_RSZ > > > #define PTP_PIN_TOD_NSEC_RSZ PTP_PIN_CFG_RSZ > > > +#define PTP_PIN_WF_HIGH_PERIOD_RSZ PTP_PIN_CFG_RSZ > > > +#define PTP_PIN_WF_LOW_PERIOD_RSZ PTP_PIN_CFG_RSZ > > > > > > #define PTP_PIN_CFG_DOM BIT(0) > > > #define PTP_PIN_CFG_SYNC BIT(2) > > > diff --git a/drivers/net/ethernet/mscc/ocelot_regs.c > b/drivers/net/ethernet/mscc/ocelot_regs.c > > > index b88b589..ed4dd01 100644 > > > --- a/drivers/net/ethernet/mscc/ocelot_regs.c > > > +++ b/drivers/net/ethernet/mscc/ocelot_regs.c > > > @@ -239,6 +239,8 @@ static const u32 ocelot_ptp_regmap[] = { > > > REG(PTP_PIN_TOD_SEC_MSB, 0x000004), > > > REG(PTP_PIN_TOD_SEC_LSB, 0x000008), > > > REG(PTP_PIN_TOD_NSEC, 0x00000c), > > > + REG(PTP_PIN_WF_HIGH_PERIOD, 0x000014), > > > + REG(PTP_PIN_WF_LOW_PERIOD, 0x000018), > > > REG(PTP_CFG_MISC, 0x0000a0), > > > REG(PTP_CLK_CFG_ADJ_CFG, 0x0000a4), > > > REG(PTP_CLK_CFG_ADJ_FREQ, 0x0000a8), > > > diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h > > > index 64cbbbe..c2ab20d 100644 > > > --- a/include/soc/mscc/ocelot.h > > > +++ b/include/soc/mscc/ocelot.h > > > @@ -325,6 +325,8 @@ enum ocelot_reg { > > > PTP_PIN_TOD_SEC_MSB, > > > PTP_PIN_TOD_SEC_LSB, > > > PTP_PIN_TOD_NSEC, > > > + PTP_PIN_WF_HIGH_PERIOD, > > > + PTP_PIN_WF_LOW_PERIOD, > > > PTP_CFG_MISC, > > > PTP_CLK_CFG_ADJ_CFG, > > > PTP_CLK_CFG_ADJ_FREQ, > > > -- > > > 2.7.4 > > > > > > > Thanks, > > -Vladimir
> -----Original Message----- > From: Andrew Lunn <andrew@lunn.ch> > Sent: Thursday, December 26, 2019 6:59 PM > To: Y.b. Lu <yangbo.lu@nxp.com> > Cc: netdev@vger.kernel.org; David S . Miller <davem@davemloft.net>; > Claudiu Manoil <claudiu.manoil@nxp.com>; Vladimir Oltean > <vladimir.oltean@nxp.com>; Alexandre Belloni > <alexandre.belloni@bootlin.com>; Microchip Linux Driver Support > <UNGLinuxDriver@microchip.com>; Richard Cochran > <richardcochran@gmail.com> > Subject: Re: [PATCH] net: mscc: ocelot: support PPS signal generation > > On Thu, Dec 26, 2019 at 05:58:51PM +0800, Yangbo Lu wrote: > > This patch is to support PPS signal generation for Ocelot family > > switches, including VSC9959 switch. > > Hi Yangbo > > Please always Cc: Richard Cochran <richardcochran@gmail.com> for ptp > patches. > Sorry for missing... > Andrew > > > > > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> > > --- > > drivers/net/dsa/ocelot/felix_vsc9959.c | 2 ++ > > drivers/net/ethernet/mscc/ocelot.c | 25 > +++++++++++++++++++++++++ > > drivers/net/ethernet/mscc/ocelot_ptp.h | 2 ++ > > drivers/net/ethernet/mscc/ocelot_regs.c | 2 ++ > > include/soc/mscc/ocelot.h | 2 ++ > > 5 files changed, 33 insertions(+) > > > > diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c > b/drivers/net/dsa/ocelot/felix_vsc9959.c > > index b9758b0..ee0ce7c 100644 > > --- a/drivers/net/dsa/ocelot/felix_vsc9959.c > > +++ b/drivers/net/dsa/ocelot/felix_vsc9959.c > > @@ -287,6 +287,8 @@ static const u32 vsc9959_ptp_regmap[] = { > > REG(PTP_PIN_TOD_SEC_MSB, 0x000004), > > REG(PTP_PIN_TOD_SEC_LSB, 0x000008), > > REG(PTP_PIN_TOD_NSEC, 0x00000c), > > + REG(PTP_PIN_WF_HIGH_PERIOD, 0x000014), > > + REG(PTP_PIN_WF_LOW_PERIOD, 0x000018), > > REG(PTP_CFG_MISC, 0x0000a0), > > REG(PTP_CLK_CFG_ADJ_CFG, 0x0000a4), > > REG(PTP_CLK_CFG_ADJ_FREQ, 0x0000a8), > > diff --git a/drivers/net/ethernet/mscc/ocelot.c > b/drivers/net/ethernet/mscc/ocelot.c > > index 985b46d..c0f8a9e 100644 > > --- a/drivers/net/ethernet/mscc/ocelot.c > > +++ b/drivers/net/ethernet/mscc/ocelot.c > > @@ -2147,6 +2147,29 @@ static struct ptp_clock_info > ocelot_ptp_clock_info = { > > .adjfine = ocelot_ptp_adjfine, > > }; > > > > +static void ocelot_ptp_init_pps(struct ocelot *ocelot) > > +{ > > + u32 val; > > + > > + /* PPS signal generation uses CLOCK action. Together with SYNC option, > > + * a single pulse will be generated after <WAFEFORM_LOW> > nanoseconds > > + * after the time of day has increased the seconds. The pulse will > > + * get a width of <WAFEFORM_HIGH> nanoseconds. > > + * > > + * In default, > > + * WAFEFORM_LOW = 0 > > + * WAFEFORM_HIGH = 1us > > + */ > > + ocelot_write_rix(ocelot, 0, PTP_PIN_WF_LOW_PERIOD, ALT_PPS_PIN); > > + ocelot_write_rix(ocelot, 1000, PTP_PIN_WF_HIGH_PERIOD, > ALT_PPS_PIN); > > + > > + val = ocelot_read_rix(ocelot, PTP_PIN_CFG, ALT_PPS_PIN); > > + val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK | > PTP_PIN_CFG_DOM); > > + val |= (PTP_PIN_CFG_SYNC | > PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_CLOCK)); > > + > > + ocelot_write_rix(ocelot, val, PTP_PIN_CFG, ALT_PPS_PIN); > > +} > > + > > static int ocelot_init_timestamp(struct ocelot *ocelot) > > { > > struct ptp_clock *ptp_clock; > > @@ -2478,6 +2501,8 @@ int ocelot_init(struct ocelot *ocelot) > > "Timestamp initialization failed\n"); > > return ret; > > } > > + > > + ocelot_ptp_init_pps(ocelot); > > } > > > > return 0; > > diff --git a/drivers/net/ethernet/mscc/ocelot_ptp.h > b/drivers/net/ethernet/mscc/ocelot_ptp.h > > index 9ede14a..21bc744 100644 > > --- a/drivers/net/ethernet/mscc/ocelot_ptp.h > > +++ b/drivers/net/ethernet/mscc/ocelot_ptp.h > > @@ -13,6 +13,8 @@ > > #define PTP_PIN_TOD_SEC_MSB_RSZ PTP_PIN_CFG_RSZ > > #define PTP_PIN_TOD_SEC_LSB_RSZ PTP_PIN_CFG_RSZ > > #define PTP_PIN_TOD_NSEC_RSZ PTP_PIN_CFG_RSZ > > +#define PTP_PIN_WF_HIGH_PERIOD_RSZ PTP_PIN_CFG_RSZ > > +#define PTP_PIN_WF_LOW_PERIOD_RSZ PTP_PIN_CFG_RSZ > > > > #define PTP_PIN_CFG_DOM BIT(0) > > #define PTP_PIN_CFG_SYNC BIT(2) > > diff --git a/drivers/net/ethernet/mscc/ocelot_regs.c > b/drivers/net/ethernet/mscc/ocelot_regs.c > > index b88b589..ed4dd01 100644 > > --- a/drivers/net/ethernet/mscc/ocelot_regs.c > > +++ b/drivers/net/ethernet/mscc/ocelot_regs.c > > @@ -239,6 +239,8 @@ static const u32 ocelot_ptp_regmap[] = { > > REG(PTP_PIN_TOD_SEC_MSB, 0x000004), > > REG(PTP_PIN_TOD_SEC_LSB, 0x000008), > > REG(PTP_PIN_TOD_NSEC, 0x00000c), > > + REG(PTP_PIN_WF_HIGH_PERIOD, 0x000014), > > + REG(PTP_PIN_WF_LOW_PERIOD, 0x000018), > > REG(PTP_CFG_MISC, 0x0000a0), > > REG(PTP_CLK_CFG_ADJ_CFG, 0x0000a4), > > REG(PTP_CLK_CFG_ADJ_FREQ, 0x0000a8), > > diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h > > index 64cbbbe..c2ab20d 100644 > > --- a/include/soc/mscc/ocelot.h > > +++ b/include/soc/mscc/ocelot.h > > @@ -325,6 +325,8 @@ enum ocelot_reg { > > PTP_PIN_TOD_SEC_MSB, > > PTP_PIN_TOD_SEC_LSB, > > PTP_PIN_TOD_NSEC, > > + PTP_PIN_WF_HIGH_PERIOD, > > + PTP_PIN_WF_LOW_PERIOD, > > PTP_CFG_MISC, > > PTP_CLK_CFG_ADJ_CFG, > > PTP_CLK_CFG_ADJ_FREQ, > > -- > > 2.7.4 > >
On Thu, 26 Dec 2019 at 13:17, Y.b. Lu <yangbo.lu@nxp.com> wrote: > > Also, I think what you have implemented here is periodic output > > (PTP_CLK_REQ_PEROUT) not PPS [input] (PTP_CLK_REQ_PPS). I have found > > the PTP documentation to be rather confusing on what PTP_CLK_REQ_PPS > > means, so I'm adding Richard in the hope that he may clarify (also > > what's different between PTP_CLK_REQ_PPS and PTP_CLK_REQ_PPS). > > My understand is PTP_CLK_REQ_PEROUT is for periodical output, and PTP_CLK_REQ_PPS is for PPS event handling. > This patch is to initialize PPS signal. You may check reference manual for how to generate PPS. > > "For the CLOCK action, the sync option makes the pin generate a single pulse, <WAFEFORM_LOW> > nanoseconds after the time of day has increased the seconds. The pulse will get a width of > <WAVEFORM_HIGH> nanoseconds. " > > If the sync option is not used, it is for periodical clock generation that you mentioned. > So basically this hardware emits a periodic signal with the frequency f equal to: - NSEC_PER_SEC / (WAFEFORM_LOW + WAFEFORM_HIGH) if PTP_PIN_SYNC is not set - 1 Hz if PTP_PIN_SYNC is set So the hardware hardcodes the frequency, basically, and makes WAFEFORM_LOW be the phase adjustment. So all in all, it's still PTP_CLK_REQ_PEROUT that needs to be treated for this. Maybe you have to special-case the value of rq->perout.period.sec and rq->perout.period.nsec. What is the phase adjustment (pin start time) if the PTP_PIN_SYNC option is not set? Anyway, it's good that you figured it out, but it's not really ok to hardcode it like this. On some boards there may be electrical issues if the PTP pins just emit pulses by default. > EXTTS is for capturing timestamps for external trigger pin which is input signal. And isn't PTP_CLK_REQ_PPS the same, just that the input signal is expected to have a particular waveform? Some drivers, like ptp_qoriq, seem like they misinterpret the PPS request as meaning "generate PPS output". I find this strange because it's exactly Richard who added the code for it. Thanks, -Vladimir
On Thu, Dec 26, 2019 at 11:17:26AM +0000, Y.b. Lu wrote: > > -----Original Message----- > > From: Vladimir Oltean <olteanv@gmail.com> > > Also, I think what you have implemented here is periodic output > > (PTP_CLK_REQ_PEROUT) not PPS [input] (PTP_CLK_REQ_PPS). I have found > > the PTP documentation to be rather confusing on what PTP_CLK_REQ_PPS > > means, so I'm adding Richard in the hope that he may clarify (also > > what's different between PTP_CLK_REQ_PPS and PTP_CLK_REQ_PPS). The PTP_CLK_REQ_PPS is for generating events for the kernel's PPS subsystem. (See drivers/pps). This has nothing to do with actual PPS signals. > My understand is PTP_CLK_REQ_PEROUT is for periodical output, Yes. > and PTP_CLK_REQ_PPS is for PPS event handling. No. Some cards generate an interrupt at the full second roll over. The interrupt service routine can feed a system time stamp into the kernel's pps subsystem for use by NTP. If your device is generating an actual PPS output signal, then you should implement the PTP_CLK_REQ_PEROUT method. Bonus points for making the signal fully programmable! Thanks, Richard
> -----Original Message----- > From: Richard Cochran <richardcochran@gmail.com> > Sent: Friday, December 27, 2019 10:08 AM > To: Y.b. Lu <yangbo.lu@nxp.com> > Cc: Vladimir Oltean <olteanv@gmail.com>; netdev <netdev@vger.kernel.org>; > David S . Miller <davem@davemloft.net>; Claudiu Manoil > <claudiu.manoil@nxp.com>; Vladimir Oltean <vladimir.oltean@nxp.com>; > Alexandre Belloni <alexandre.belloni@bootlin.com>; Microchip Linux Driver > Support <UNGLinuxDriver@microchip.com> > Subject: Re: [PATCH] net: mscc: ocelot: support PPS signal generation > > On Thu, Dec 26, 2019 at 11:17:26AM +0000, Y.b. Lu wrote: > > > -----Original Message----- > > > From: Vladimir Oltean <olteanv@gmail.com> > > > Also, I think what you have implemented here is periodic output > > > (PTP_CLK_REQ_PEROUT) not PPS [input] (PTP_CLK_REQ_PPS). I have found > > > the PTP documentation to be rather confusing on what PTP_CLK_REQ_PPS > > > means, so I'm adding Richard in the hope that he may clarify (also > > > what's different between PTP_CLK_REQ_PPS and PTP_CLK_REQ_PPS). > > The PTP_CLK_REQ_PPS is for generating events for the kernel's PPS > subsystem. (See drivers/pps). This has nothing to do with actual PPS > signals. > > > My understand is PTP_CLK_REQ_PEROUT is for periodical output, > > Yes. > > > and PTP_CLK_REQ_PPS is for PPS event handling. > > No. > > Some cards generate an interrupt at the full second roll over. The > interrupt service routine can feed a system time stamp into the > kernel's pps subsystem for use by NTP. > > If your device is generating an actual PPS output signal, then you > should implement the PTP_CLK_REQ_PEROUT method. > I'm a little confused. It seems PTP_CLK_REQ_PEROUT method needs req.perout.start and req.perout.period to generate periodical *clock* signal, while PPS is *pulse* signal every second. For the two cases (1Hz clock signal and 1 pulse very second), how to configure with PTP_CLK_REQ_PEROUT method? > Bonus points for making the signal fully programmable! For some hardware, each pin has fixed function. And some hardware, each pin could be programable for function. The Ocelot PTP pin is programable, but initially the software author may plan to set fixed function for each pin. Do you suggest we make all pins function programable? In include/soc/mscc/ocelot.h, enum ocelot_clk_pins { ALT_PPS_PIN = 1, EXT_CLK_PIN, ALT_LDST_PIN, TOD_ACC_PIN }; BTW, current ptp clock code is embedded in ocelot.c. More and more functions will be added in the future, and the interrupt implementation in SoC is different between Ocelot and VSC9959. Do you think it's proper to separate common code as a single PTP driver? Thanks. > > Thanks, > Richard
On Fri, Dec 27, 2019 at 03:51:08AM +0000, Y.b. Lu wrote: > I'm a little confused. > It seems PTP_CLK_REQ_PEROUT method needs req.perout.start and req.perout.period to generate periodical *clock* signal, while PPS is *pulse* signal every second. > For the two cases (1Hz clock signal and 1 pulse very second), how to configure with PTP_CLK_REQ_PEROUT method? If your HW can generate other periods, then implement them! If your HW can only do exactly one pps, then you can check that the nanoseconds fields are zero, returning -ERANGE when they are non-zero. > For some hardware, each pin has fixed function. And some hardware, each pin could be programable for function. > The Ocelot PTP pin is programable, but initially the software author may plan to set fixed function for each pin. > Do you suggest we make all pins function programable? Yes. You should implement the ptp_clock_info.verify method: * @verify: Confirm that a pin can perform a given function. The PTP * Hardware Clock subsystem maintains the 'pin_config' * array on behalf of the drivers, but the PHC subsystem * assumes that every pin can perform every function. This * hook gives drivers a way of telling the core about * limitations on specific pins. This function must return * zero if the function can be assigned to this pin, and * nonzero otherwise. If the pin cannot be changed on a particular SoC, then the .verify should simply make sure user space chose the correct setting. > BTW, current ptp clock code is embedded in ocelot.c. > More and more functions will be added in the future, and the interrupt implementation in SoC is different between Ocelot and VSC9959. > Do you think it's proper to separate common code as a single PTP driver? If you get a lot of code re-use, then sure. Thanks, Richard
diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c index b9758b0..ee0ce7c 100644 --- a/drivers/net/dsa/ocelot/felix_vsc9959.c +++ b/drivers/net/dsa/ocelot/felix_vsc9959.c @@ -287,6 +287,8 @@ static const u32 vsc9959_ptp_regmap[] = { REG(PTP_PIN_TOD_SEC_MSB, 0x000004), REG(PTP_PIN_TOD_SEC_LSB, 0x000008), REG(PTP_PIN_TOD_NSEC, 0x00000c), + REG(PTP_PIN_WF_HIGH_PERIOD, 0x000014), + REG(PTP_PIN_WF_LOW_PERIOD, 0x000018), REG(PTP_CFG_MISC, 0x0000a0), REG(PTP_CLK_CFG_ADJ_CFG, 0x0000a4), REG(PTP_CLK_CFG_ADJ_FREQ, 0x0000a8), diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c index 985b46d..c0f8a9e 100644 --- a/drivers/net/ethernet/mscc/ocelot.c +++ b/drivers/net/ethernet/mscc/ocelot.c @@ -2147,6 +2147,29 @@ static struct ptp_clock_info ocelot_ptp_clock_info = { .adjfine = ocelot_ptp_adjfine, }; +static void ocelot_ptp_init_pps(struct ocelot *ocelot) +{ + u32 val; + + /* PPS signal generation uses CLOCK action. Together with SYNC option, + * a single pulse will be generated after <WAFEFORM_LOW> nanoseconds + * after the time of day has increased the seconds. The pulse will + * get a width of <WAFEFORM_HIGH> nanoseconds. + * + * In default, + * WAFEFORM_LOW = 0 + * WAFEFORM_HIGH = 1us + */ + ocelot_write_rix(ocelot, 0, PTP_PIN_WF_LOW_PERIOD, ALT_PPS_PIN); + ocelot_write_rix(ocelot, 1000, PTP_PIN_WF_HIGH_PERIOD, ALT_PPS_PIN); + + val = ocelot_read_rix(ocelot, PTP_PIN_CFG, ALT_PPS_PIN); + val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK | PTP_PIN_CFG_DOM); + val |= (PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_CLOCK)); + + ocelot_write_rix(ocelot, val, PTP_PIN_CFG, ALT_PPS_PIN); +} + static int ocelot_init_timestamp(struct ocelot *ocelot) { struct ptp_clock *ptp_clock; @@ -2478,6 +2501,8 @@ int ocelot_init(struct ocelot *ocelot) "Timestamp initialization failed\n"); return ret; } + + ocelot_ptp_init_pps(ocelot); } return 0; diff --git a/drivers/net/ethernet/mscc/ocelot_ptp.h b/drivers/net/ethernet/mscc/ocelot_ptp.h index 9ede14a..21bc744 100644 --- a/drivers/net/ethernet/mscc/ocelot_ptp.h +++ b/drivers/net/ethernet/mscc/ocelot_ptp.h @@ -13,6 +13,8 @@ #define PTP_PIN_TOD_SEC_MSB_RSZ PTP_PIN_CFG_RSZ #define PTP_PIN_TOD_SEC_LSB_RSZ PTP_PIN_CFG_RSZ #define PTP_PIN_TOD_NSEC_RSZ PTP_PIN_CFG_RSZ +#define PTP_PIN_WF_HIGH_PERIOD_RSZ PTP_PIN_CFG_RSZ +#define PTP_PIN_WF_LOW_PERIOD_RSZ PTP_PIN_CFG_RSZ #define PTP_PIN_CFG_DOM BIT(0) #define PTP_PIN_CFG_SYNC BIT(2) diff --git a/drivers/net/ethernet/mscc/ocelot_regs.c b/drivers/net/ethernet/mscc/ocelot_regs.c index b88b589..ed4dd01 100644 --- a/drivers/net/ethernet/mscc/ocelot_regs.c +++ b/drivers/net/ethernet/mscc/ocelot_regs.c @@ -239,6 +239,8 @@ static const u32 ocelot_ptp_regmap[] = { REG(PTP_PIN_TOD_SEC_MSB, 0x000004), REG(PTP_PIN_TOD_SEC_LSB, 0x000008), REG(PTP_PIN_TOD_NSEC, 0x00000c), + REG(PTP_PIN_WF_HIGH_PERIOD, 0x000014), + REG(PTP_PIN_WF_LOW_PERIOD, 0x000018), REG(PTP_CFG_MISC, 0x0000a0), REG(PTP_CLK_CFG_ADJ_CFG, 0x0000a4), REG(PTP_CLK_CFG_ADJ_FREQ, 0x0000a8), diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h index 64cbbbe..c2ab20d 100644 --- a/include/soc/mscc/ocelot.h +++ b/include/soc/mscc/ocelot.h @@ -325,6 +325,8 @@ enum ocelot_reg { PTP_PIN_TOD_SEC_MSB, PTP_PIN_TOD_SEC_LSB, PTP_PIN_TOD_NSEC, + PTP_PIN_WF_HIGH_PERIOD, + PTP_PIN_WF_LOW_PERIOD, PTP_CFG_MISC, PTP_CLK_CFG_ADJ_CFG, PTP_CLK_CFG_ADJ_FREQ,
This patch is to support PPS signal generation for Ocelot family switches, including VSC9959 switch. Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com> --- drivers/net/dsa/ocelot/felix_vsc9959.c | 2 ++ drivers/net/ethernet/mscc/ocelot.c | 25 +++++++++++++++++++++++++ drivers/net/ethernet/mscc/ocelot_ptp.h | 2 ++ drivers/net/ethernet/mscc/ocelot_regs.c | 2 ++ include/soc/mscc/ocelot.h | 2 ++ 5 files changed, 33 insertions(+)