2021-02-12 05:11:04

by Vincent Cheng

[permalink] [raw]
Subject: [PATCH net-next 0/2] ptp: ptp_clockmatrix: Fix output 1 PPS alignment.

From: Vincent Cheng <[email protected]>

This series fixes a race condition that may result in the output clock
not aligned to internal 1 PPS clock.

Part of device initialization is to align the rising edge of output
clocks to the internal rising edge of the 1 PPS clock. If the system
APLL and DPLL are not locked when this alignment occurs, the alignment
fails and a fixed offset between the internal 1 PPS clock and the
output clock occurs.

If a clock is dynamically enabled after power-up, the output clock
also needs to be aligned to the internal 1 PPS clock.

Vincent Cheng (2):
ptp: ptp_clockmatrix: Add wait_for_sys_apll_dpll_lock.
ptp: ptp_clockmatrix: Add alignment of 1 PPS to idtcm_perout_enable.

drivers/ptp/idt8a340_reg.h | 10 +++++
drivers/ptp/ptp_clockmatrix.c | 92 ++++++++++++++++++++++++++++++++++++++++---
drivers/ptp/ptp_clockmatrix.h | 17 +++++++-
3 files changed, 112 insertions(+), 7 deletions(-)

--
2.7.4


2021-02-12 05:11:37

by Vincent Cheng

[permalink] [raw]
Subject: [PATCH net-next 1/2] ptp: ptp_clockmatrix: Add wait_for_sys_apll_dpll_lock.

From: Vincent Cheng <[email protected]>

Part of the device initialization aligns the rising edge of the output
clock to the internal 1 PPS clock. If the system APLL and DPLL is not
locked, then the alignment will fail and there will be a fixed offset
between the internal 1 PPS clock and the output clock.

After loading the device firmware, poll the system APLL and DPLL for
locked state prior to initialization, timing out after 2 seconds.

Signed-off-by: Vincent Cheng <[email protected]>
---
drivers/ptp/idt8a340_reg.h | 10 ++++++
drivers/ptp/ptp_clockmatrix.c | 76 +++++++++++++++++++++++++++++++++++++++++--
drivers/ptp/ptp_clockmatrix.h | 17 ++++++++--
3 files changed, 99 insertions(+), 4 deletions(-)

diff --git a/drivers/ptp/idt8a340_reg.h b/drivers/ptp/idt8a340_reg.h
index a664dfe..ac524cf 100644
--- a/drivers/ptp/idt8a340_reg.h
+++ b/drivers/ptp/idt8a340_reg.h
@@ -122,6 +122,8 @@
#define OTP_SCSR_CONFIG_SELECT 0x0022

#define STATUS 0xc03c
+#define DPLL_SYS_STATUS 0x0020
+#define DPLL_SYS_APLL_STATUS 0x0021
#define USER_GPIO0_TO_7_STATUS 0x008a
#define USER_GPIO8_TO_15_STATUS 0x008b

@@ -707,4 +709,12 @@
/* Bit definitions for the DPLL_CTRL_COMBO_MASTER_CFG register */
#define COMBO_MASTER_HOLD BIT(0)

+/* Bit definitions for DPLL_SYS_STATUS register */
+#define DPLL_SYS_STATE_MASK (0xf)
+
+/* Bit definitions for SYS_APLL_STATUS register */
+#define SYS_APLL_LOSS_LOCK_LIVE_MASK BIT(0)
+#define SYS_APLL_LOSS_LOCK_LIVE_LOCKED 0
+#define SYS_APLL_LOSS_LOCK_LIVE_UNLOCKED 1
+
#endif
diff --git a/drivers/ptp/ptp_clockmatrix.c b/drivers/ptp/ptp_clockmatrix.c
index 051511f..1918de5 100644
--- a/drivers/ptp/ptp_clockmatrix.c
+++ b/drivers/ptp/ptp_clockmatrix.c
@@ -335,6 +335,79 @@ static int wait_for_boot_status_ready(struct idtcm *idtcm)
return -EBUSY;
}

+static int read_sys_apll_status(struct idtcm *idtcm, u8 *status)
+{
+ int err;
+
+ err = idtcm_read(idtcm, STATUS, DPLL_SYS_APLL_STATUS, status,
+ sizeof(u8));
+
+ return err;
+}
+
+static int read_sys_dpll_status(struct idtcm *idtcm, u8 *status)
+{
+ int err;
+
+ err = idtcm_read(idtcm, STATUS, DPLL_SYS_STATUS, status, sizeof(u8));
+
+ return err;
+}
+
+static int wait_for_sys_apll_dpll_lock(struct idtcm *idtcm)
+{
+ char *fmt = "%d ms SYS lock timeout: APLL Loss Lock %d DPLL state %d";
+ u8 i = LOCK_TIMEOUT_MS / LOCK_POLL_INTERVAL_MS;
+ u8 apll = 0;
+ u8 dpll = 0;
+
+ int err;
+
+ do {
+ err = read_sys_apll_status(idtcm, &apll);
+
+ if (err)
+ return err;
+
+ err = read_sys_dpll_status(idtcm, &dpll);
+
+ if (err)
+ return err;
+
+ apll &= SYS_APLL_LOSS_LOCK_LIVE_MASK;
+ dpll &= DPLL_SYS_STATE_MASK;
+
+ if ((apll == SYS_APLL_LOSS_LOCK_LIVE_LOCKED)
+ && (dpll == DPLL_STATE_LOCKED)) {
+ return 0;
+ } else if ((dpll == DPLL_STATE_FREERUN) ||
+ (dpll == DPLL_STATE_HOLDOVER) ||
+ (dpll == DPLL_STATE_OPEN_LOOP)) {
+ dev_warn(&idtcm->client->dev,
+ "No wait state: DPLL_SYS_STATE %d", dpll);
+ return -EPERM;
+ }
+
+ msleep(LOCK_POLL_INTERVAL_MS);
+ i--;
+
+ } while (i);
+
+ dev_warn(&idtcm->client->dev, fmt, LOCK_TIMEOUT_MS, apll, dpll);
+
+ return -ETIME;
+}
+
+static void wait_for_chip_ready(struct idtcm *idtcm)
+{
+ if (wait_for_boot_status_ready(idtcm))
+ dev_warn(&idtcm->client->dev, "BOOT_STATUS != 0xA0");
+
+ if (wait_for_sys_apll_dpll_lock(idtcm))
+ dev_warn(&idtcm->client->dev,
+ "Continuing while SYS APLL/DPLL is not locked");
+}
+
static int _idtcm_gettime(struct idtcm_channel *channel,
struct timespec64 *ts)
{
@@ -2235,8 +2308,7 @@ static int idtcm_probe(struct i2c_client *client,
dev_warn(&idtcm->client->dev,
"loading firmware failed with %d\n", err);

- if (wait_for_boot_status_ready(idtcm))
- dev_warn(&idtcm->client->dev, "BOOT_STATUS != 0xA0\n");
+ wait_for_chip_ready(idtcm);

if (idtcm->tod_mask) {
for (i = 0; i < MAX_TOD; i++) {
diff --git a/drivers/ptp/ptp_clockmatrix.h b/drivers/ptp/ptp_clockmatrix.h
index 645de2c..fb32327 100644
--- a/drivers/ptp/ptp_clockmatrix.h
+++ b/drivers/ptp/ptp_clockmatrix.h
@@ -15,7 +15,6 @@
#define FW_FILENAME "idtcm.bin"
#define MAX_TOD (4)
#define MAX_PLL (8)
-#define MAX_OUTPUT (12)

#define MAX_ABS_WRITE_PHASE_PICOSECONDS (107374182350LL)

@@ -51,6 +50,9 @@
#define TOD_WRITE_OVERHEAD_COUNT_MAX (2)
#define TOD_BYTE_COUNT (11)

+#define LOCK_TIMEOUT_MS (2000)
+#define LOCK_POLL_INTERVAL_MS (10)
+
#define PEROUT_ENABLE_OUTPUT_MASK (0xdeadbeef)

#define IDTCM_MAX_WRITE_COUNT (512)
@@ -105,6 +107,18 @@ enum scsr_tod_write_type_sel {
SCSR_TOD_WR_TYPE_SEL_MAX = SCSR_TOD_WR_TYPE_SEL_DELTA_MINUS,
};

+/* Values STATUS.DPLL_SYS_STATUS.DPLL_SYS_STATE */
+enum dpll_state {
+ DPLL_STATE_MIN = 0,
+ DPLL_STATE_FREERUN = DPLL_STATE_MIN,
+ DPLL_STATE_LOCKACQ = 1,
+ DPLL_STATE_LOCKREC = 2,
+ DPLL_STATE_LOCKED = 3,
+ DPLL_STATE_HOLDOVER = 4,
+ DPLL_STATE_OPEN_LOOP = 5,
+ DPLL_STATE_MAX = DPLL_STATE_OPEN_LOOP,
+};
+
struct idtcm;

struct idtcm_channel {
@@ -123,7 +137,6 @@ struct idtcm_channel {
enum pll_mode pll_mode;
u8 pll;
u16 output_mask;
- u8 output_phase_adj[MAX_OUTPUT][4];
};

struct idtcm {
--
2.7.4

2021-02-12 05:12:28

by Vincent Cheng

[permalink] [raw]
Subject: [PATCH net-next 2/2] ptp: ptp_clockmatrix: Add alignment of 1 PPS to idtcm_perout_enable.

From: Vincent Cheng <[email protected]>

When enabling output using PTP_CLK_REQ_PEROUT, need to align the output
clock to the internal 1 PPS clock.

Signed-off-by: Vincent Cheng <[email protected]>
---
drivers/ptp/ptp_clockmatrix.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/ptp/ptp_clockmatrix.c b/drivers/ptp/ptp_clockmatrix.c
index 1918de5..ebe540e 100644
--- a/drivers/ptp/ptp_clockmatrix.c
+++ b/drivers/ptp/ptp_clockmatrix.c
@@ -1401,13 +1401,23 @@ static int idtcm_perout_enable(struct idtcm_channel *channel,
bool enable,
struct ptp_perout_request *perout)
{
+ struct idtcm *idtcm = channel->idtcm;
unsigned int flags = perout->flags;
+ struct timespec64 ts = {0, 0};
+ int err;

if (flags == PEROUT_ENABLE_OUTPUT_MASK)
- return idtcm_output_mask_enable(channel, enable);
+ err = idtcm_output_mask_enable(channel, enable);
+ else
+ err = idtcm_output_enable(channel, enable, perout->index);
+
+ if (err) {
+ dev_err(&idtcm->client->dev, "Unable to set output enable");
+ return err;
+ }

- /* Enable/disable individual output instead */
- return idtcm_output_enable(channel, enable, perout->index);
+ /* Align output to internal 1 PPS */
+ return _idtcm_settime(channel, &ts, SCSR_TOD_WR_TYPE_SEL_DELTA_PLUS);
}

static int idtcm_get_pll_mode(struct idtcm_channel *channel,
--
2.7.4

2021-02-12 15:34:15

by Richard Cochran

[permalink] [raw]
Subject: Re: [PATCH net-next 1/2] ptp: ptp_clockmatrix: Add wait_for_sys_apll_dpll_lock.

On Thu, Feb 11, 2021 at 11:38:44PM -0500, [email protected] wrote:

> +static int wait_for_sys_apll_dpll_lock(struct idtcm *idtcm)
> +{
> + char *fmt = "%d ms SYS lock timeout: APLL Loss Lock %d DPLL state %d";

Probably you want: const char *fmt

> diff --git a/drivers/ptp/ptp_clockmatrix.h b/drivers/ptp/ptp_clockmatrix.h
> index 645de2c..fb32327 100644
> --- a/drivers/ptp/ptp_clockmatrix.h
> +++ b/drivers/ptp/ptp_clockmatrix.h
> @@ -15,7 +15,6 @@
> #define FW_FILENAME "idtcm.bin"
> #define MAX_TOD (4)
> #define MAX_PLL (8)
> -#define MAX_OUTPUT (12)

...

> @@ -123,7 +137,6 @@ struct idtcm_channel {
> enum pll_mode pll_mode;
> u8 pll;
> u16 output_mask;
> - u8 output_phase_adj[MAX_OUTPUT][4];
> };

Looks like this removal is unrelated to the patch subject, and so it
deserves its own small patch.

Acked-by: Richard Cochran <[email protected]>

2021-02-12 15:35:51

by Richard Cochran

[permalink] [raw]
Subject: Re: [PATCH net-next 2/2] ptp: ptp_clockmatrix: Add alignment of 1 PPS to idtcm_perout_enable.

On Thu, Feb 11, 2021 at 11:38:45PM -0500, [email protected] wrote:
> From: Vincent Cheng <[email protected]>
>
> When enabling output using PTP_CLK_REQ_PEROUT, need to align the output
> clock to the internal 1 PPS clock.
>
> Signed-off-by: Vincent Cheng <[email protected]>

Acked-by: Richard Cochran <[email protected]>

2021-02-13 04:21:03

by Vincent Cheng

[permalink] [raw]
Subject: Re: [PATCH net-next 1/2] ptp: ptp_clockmatrix: Add wait_for_sys_apll_dpll_lock.

On Fri, Feb 12, 2021 at 10:31:40AM EST, Richard Cochran wrote:

>On Thu, Feb 11, 2021 at 11:38:44PM -0500, [email protected] wrote:
>
>> +static int wait_for_sys_apll_dpll_lock(struct idtcm *idtcm)
>> +{
>> + char *fmt = "%d ms SYS lock timeout: APLL Loss Lock %d DPLL state %d";
>
>Probably you want: const char *fmt

Good point, will change in V2 patch.

>
>> diff --git a/drivers/ptp/ptp_clockmatrix.h b/drivers/ptp/ptp_clockmatrix.h
>> index 645de2c..fb32327 100644
>> --- a/drivers/ptp/ptp_clockmatrix.h
>...
>
>> @@ -123,7 +137,6 @@ struct idtcm_channel {
>> enum pll_mode pll_mode;
>> u8 pll;
>> u16 output_mask;
>> - u8 output_phase_adj[MAX_OUTPUT][4];
>> };
>
>Looks like this removal is unrelated to the patch subject, and so it
>deserves its own small patch.

Ok, will separate into separate patch for V2.

Vincent