Messages in this thread Patch in this message | | | From | <> | Subject | Re: [RFC Patch net-next 0/6] net: dsa: microchip: add gPTP support for LAN937x switch | Date | Fri, 4 Nov 2022 10:36:32 +0000 |
| |
Hi Christian, I updated the n_per_out code and tested with lan9370 board, it is working in LED_0 pin.
Is there any testing pending from your side. If no can I generate next version of patch with your changes which you shared in this thread.
Attached the code for reference
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h index 3a640ca4f7e1..f6a1a7acda07 100644 --- a/drivers/net/dsa/microchip/ksz_common.h +++ b/drivers/net/dsa/microchip/ksz_common.h @@ -472,6 +472,19 @@ static inline int ksz_rmw16(struct ksz_device *dev, u32 reg, u16 mask, return ret; } +static inline int ksz_rmw32(struct ksz_device *dev, u32 reg, u32 mask, + u32 value) +{ + int ret; + + ret = regmap_update_bits(dev->regmap[2], reg, mask, value); + if (ret) + dev_err(dev->dev, "can't rmw 32bit reg: 0x%x %pe\n", reg, + ERR_PTR(ret)); + + return ret; +} + static inline int ksz_write64(struct ksz_device *dev, u32 reg, u64 value) { u32 val[2]; diff --git a/drivers/net/dsa/microchip/ksz_ptp.c b/drivers/net/dsa/microchip/ksz_ptp.c index d11a490a6c87..54527754a50f 100644 --- a/drivers/net/dsa/microchip/ksz_ptp.c +++ b/drivers/net/dsa/microchip/ksz_ptp.c @@ -26,6 +26,147 @@ #define KSZ_PTP_INT_START 13 +static int ksz_ptp_restart_perout(struct ksz_device *dev); + +static int ksz_ptp_tou_gpio(struct ksz_device *dev) +{ + int ret; + + /* Set the Led Override register */ + ret = ksz_rmw32(dev, REG_SW_GLOBAL_LED_OVR__4, LED_OVR_1, LED_OVR_1); + if (ret) + return ret; + + /* Set the Led Source register */ + return ksz_rmw32(dev, REG_SW_GLOBAL_LED_SRC__4, LED_SRC_PTP_GPIO_1, + LED_SRC_PTP_GPIO_1); +} + +/* Shared register access routines (Trigger Output Unit) */ +static int ksz_ptp_tou_reset(struct ksz_device *dev, unsigned int unit) +{ + u32 data; + int ret; + + /* Reset trigger unit (clears TRIGGER_EN, but not GPIOSTATx) */ + ret = ksz_rmw32(dev, REG_PTP_CTRL_STAT__4, TRIG_RESET, TRIG_RESET); + + /* Clear DONE */ + data = 1 << (unit + TRIG_DONE_S); + ret = ksz_write32(dev, REG_PTP_TRIG_STATUS__4, data); + if (ret) + return ret; + + /* Clear IRQ */ + data = 1 << (unit + TRIG_INT_S); + ret = ksz_write32(dev, REG_PTP_INT_STATUS__4, data); + if (ret) + return ret; + + /* Clear reset and set GPIO direction */ + return ksz_rmw32(dev, REG_PTP_CTRL_STAT__4, (TRIG_RESET | TRIG_ENABLE), + 0); +} + +static int ksz_ptp_tou_cycle_width_set(struct ksz_device *dev, u32 width_ns) +{ + int ret; + + ret = ksz_write32(dev, REG_TRIG_CYCLE_WIDTH, width_ns); + if (ret) + return ret; + + return 0; +} + +static int ksz_ptp_tou_cycle_count_set(struct ksz_device *dev, u16 count) +{ + u32 data; + int ret; + + ret = ksz_read32(dev, REG_TRIG_CYCLE_CNT, &data); + if (ret) + return ret; + + data &= ~(TRIG_CYCLE_CNT_M << TRIG_CYCLE_CNT_S); + data |= (count & TRIG_CYCLE_CNT_M) << TRIG_CYCLE_CNT_S; + + ret = ksz_write32(dev, REG_TRIG_CYCLE_CNT, data); + if (ret) + return ret; + + return 0; +} + +static int ksz_ptp_tou_pulse_verify(u64 pulse_ns) +{ + u32 data; + + if (pulse_ns & 0x3) + return -EINVAL; + + data = (pulse_ns / 8); + if (data != (data & TRIG_PULSE_WIDTH_M)) + return -ERANGE; + + return 0; +} + +static int ksz_ptp_tou_pulse_set(struct ksz_device *dev, u32 pulse_ns) +{ + u32 data; + + data = (pulse_ns / 8); + + return ksz_write32(dev, REG_TRIG_PULSE_WIDTH__4, data); +} + +static int ksz_ptp_tou_target_time_set(struct ksz_device *dev, + struct timespec64 const *ts) +{ + int ret; + + /* Hardware has only 32 bit */ + if ((ts->tv_sec & 0xffffffff) != ts->tv_sec) + return -EINVAL; + + ret = ksz_write32(dev, REG_TRIG_TARGET_NANOSEC, ts->tv_nsec); + if (ret) + return ret; + + ret = ksz_write32(dev, REG_TRIG_TARGET_SEC, ts->tv_sec); + if (ret) + return ret; + + return 0; +} + +static int ksz_ptp_tou_start(struct ksz_device *dev, u32 *ctrl_stat_) +{ + u32 ctrl_stat; + int ret; + + /* Configure GPIO pins */ + ret = ksz_ptp_tou_gpio(dev); + if (ret) + return ret; + + ret = ksz_read32(dev, REG_PTP_CTRL_STAT__4, &ctrl_stat); + if (ret) + return ret; + + ctrl_stat |= (TRIG_ENABLE | GPIO_OUT); + + ret = ksz_write32(dev, REG_PTP_CTRL_STAT__4, ctrl_stat); + if (ret) + return ret; + + if (ctrl_stat_) + *ctrl_stat_ = ctrl_stat; + + return 0; +} + static int ksz_ptp_enable_mode(struct ksz_device *dev, bool enable) { struct ksz_ptp_data *ptp_data = &dev->ptp_data; @@ -277,6 +418,20 @@ static int ksz_ptp_settime(struct ptp_clock_info *ptp, /* Load PTP clock from shadow registers */ ret = ksz_rmw16(dev, REG_PTP_CLK_CTRL, PTP_LOAD_TIME, PTP_LOAD_TIME); + switch (ptp_data->tou_mode) { + case KSZ_PTP_TOU_IDLE: + break; + + case KSZ_PTP_TOU_PEROUT: + dev_info(dev->dev, "Restarting periodic output signal\n"); + + ret = ksz_ptp_restart_perout(dev); + if (ret) + goto error_return; + + break; + } + spin_lock_bh(&ptp_data->clock_lock); ptp_data->clock_time = *ts; spin_unlock_bh(&ptp_data->clock_lock); @@ -368,6 +523,20 @@ static int ksz_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta) ret = ksz_write16(dev, REG_PTP_CLK_CTRL, data16); + switch (ptp_data->tou_mode) { + case KSZ_PTP_TOU_IDLE: + break; + + case KSZ_PTP_TOU_PEROUT: + dev_info(dev->dev, "Restarting periodic output signal\n"); + + ret = ksz_ptp_restart_perout(dev); + if (ret) + goto error_return; + + break; + } + spin_lock_bh(&ptp_data->clock_lock); ptp_data->clock_time = timespec64_add(ptp_data->clock_time, delta64); spin_unlock_bh(&ptp_data->clock_lock); @@ -377,6 +546,201 @@ static int ksz_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta) return ret; } +static int ksz_ptp_configure_perout(struct ksz_device *dev, + u32 cycle_width_ns, u16 cycle_count, + u32 pulse_width_ns, + struct timespec64 const *target_time) +{ + u32 trig_ctrl; + int ret; + + /* Enable notify, set rising edge, set periodic pattern */ + trig_ctrl = TRIG_NOTIFY | (TRIG_POS_PERIOD << TRIG_PATTERN_S); + ret = ksz_write32(dev, REG_TRIG_CTRL__4, trig_ctrl); + if (ret) + return ret; + + ret = ksz_ptp_tou_cycle_width_set(dev, cycle_width_ns); + if (ret) + return ret; + + ksz_ptp_tou_cycle_count_set(dev, cycle_count); + if (ret) + return ret; + + ret = ksz_ptp_tou_pulse_set(dev, pulse_width_ns); + if (ret) + return ret; + + ret = ksz_ptp_tou_target_time_set(dev, target_time); + if (ret) + return ret; + + return 0; +} + +#define KSZ_PEROUT_VALID_FLAGS ( \ + PTP_PEROUT_DUTY_CYCLE \ + ) + +static int ksz_ptp_enable_perout(struct ksz_device *dev, + struct ptp_perout_request const *perout_request, + int on) +{ + struct ksz_ptp_data *ptp_data = &dev->ptp_data; + u64 cycle_width_ns; + u64 pulse_width_ns; + u32 gpio_stat0; + int ret; + + if (perout_request->flags & ~KSZ_PEROUT_VALID_FLAGS) + return -EINVAL; + + if (ptp_data->tou_mode != KSZ_PTP_TOU_PEROUT && + ptp_data->tou_mode != KSZ_PTP_TOU_IDLE) + return -EBUSY; + + ret = ksz_ptp_tou_reset(dev, 0); + if (ret) + return ret; + + if (!on) { + ptp_data->tou_mode = KSZ_PTP_TOU_IDLE; + return 0; /* success */ + } + + ptp_data->perout_target_time_first.tv_sec = perout_request- >start.sec; + ptp_data->perout_target_time_first.tv_nsec = perout_request- >start.nsec; + + ptp_data->perout_period.tv_sec = perout_request->period.sec; + ptp_data->perout_period.tv_nsec = perout_request->period.nsec; + + cycle_width_ns = timespec64_to_ns(&ptp_data->perout_period); + if ((cycle_width_ns & GENMASK(31, 0)) != cycle_width_ns) + return -EINVAL; + + if (perout_request->flags & PTP_PEROUT_DUTY_CYCLE) + pulse_width_ns = perout_request->on.sec * NSEC_PER_SEC + + perout_request->on.nsec; + + else + /* Use a duty cycle of 50%. Maximum pulse width supported by the + * hardware is a little bit more than 125 ms. + */ + pulse_width_ns = min_t(u64, + (perout_request->period.sec * NSEC_PER_SEC + + perout_request->period.nsec) / 2 + / 8 * 8, + 125000000LL); + + ret = ksz_ptp_tou_pulse_verify(pulse_width_ns); + if (ret) + return ret; + + ret = ksz_ptp_configure_perout(dev, cycle_width_ns, + 0, + pulse_width_ns, + &ptp_data- >perout_target_time_first); + if (ret) + return ret; + + /* Activate trigger unit */ + ret = ksz_ptp_tou_start(dev, NULL); + if (ret) + return ret; + + /* Check error flag: + * - the ACTIVE flag is NOT cleared an error! + */ + ret = ksz_read32(dev, REG_PTP_TRIG_STATUS__4, &gpio_stat0); + if (ret) + return ret; + + if (gpio_stat0 & (1 << (0 + TRIG_ERROR_S))) { + dev_err(dev->dev, "%s: Trigger unit0 error!\n", __func__); + ret = -EIO; + /* Unit will be reset on next access */ + return ret; + } + + ptp_data->tou_mode = KSZ_PTP_TOU_PEROUT; + + return 0; +} + +static int ksz_ptp_restart_perout(struct ksz_device *dev) +{ + struct ksz_ptp_data *ptp_data = &dev->ptp_data; + s64 now_ns, first_ns, period_ns, next_ns; + struct timespec64 now; + unsigned int count; + int ret; + + ret = _ksz_ptp_gettime(dev, &now); + if (ret) + return ret; + + now_ns = timespec64_to_ns(&now); + first_ns = timespec64_to_ns(&ptp_data- >perout_target_time_first); + + /* Calculate next perout event based on start time and period */ + period_ns = timespec64_to_ns(&ptp_data->perout_period); + + if (first_ns < now_ns) { + count = div_u64(now_ns - first_ns, period_ns); + next_ns = first_ns + count * period_ns; + } else { + next_ns = first_ns; + } + + /* Ensure 100 ms guard time prior next event */ + while (next_ns < now_ns + 100000000) + next_ns += period_ns; + + /* Restart periodic output signal */ + { + struct timespec64 next = ns_to_timespec64(next_ns); + struct ptp_perout_request perout_request = { + .start = { + .sec = next.tv_sec, + .nsec = next.tv_nsec + }, + .period = { + .sec = ptp_data->perout_period.tv_sec, + .nsec = ptp_data->perout_period.tv_nsec + }, + .index = 0, + .flags = 0, /* keep current values */ + }; + ret = ksz_ptp_enable_perout(dev, &perout_request, 1); + if (ret) + return ret; + } + + return 0; +} + +static int ksz_ptp_enable(struct ptp_clock_info *ptp, + struct ptp_clock_request *req, int on) +{ + struct ksz_ptp_data *ptp_data = ptp_caps_to_data(ptp); + struct ksz_device *dev = ptp_data_to_ksz_dev(ptp_data); + struct ptp_perout_request *perout_request = &req->perout; + int ret; + + switch (req->type) { + case PTP_CLK_REQ_PEROUT: + mutex_lock(&ptp_data->lock); + ret = ksz_ptp_enable_perout(dev, perout_request, on); + mutex_unlock(&ptp_data->lock); + break; + default: + return -EINVAL; + } + + return ret; +} + /* Function is pointer to the do_aux_work in the ptp_clock capability */ static long ksz_ptp_do_aux_work(struct ptp_clock_info *ptp) { @@ -489,6 +853,8 @@ static const struct ptp_clock_info ksz_ptp_caps = { .adjfine = ksz_ptp_adjfine, .adjtime = ksz_ptp_adjtime, .do_aux_work = ksz_ptp_do_aux_work, + .enable = ksz_ptp_enable, + .n_per_out = 1, }; int ksz_ptp_clock_register(struct dsa_switch *ds) diff --git a/drivers/net/dsa/microchip/ksz_ptp.h b/drivers/net/dsa/microchip/ksz_ptp.h index b2035a0bcbb2..160c05e573bd 100644 --- a/drivers/net/dsa/microchip/ksz_ptp.h +++ b/drivers/net/dsa/microchip/ksz_ptp.h @@ -8,6 +8,11 @@ #if IS_ENABLED(CONFIG_NET_DSA_MICROCHIP_KSZ_PTP) +enum ksz_ptp_tou_mode { + KSZ_PTP_TOU_IDLE, + KSZ_PTP_TOU_PEROUT, +}; + struct ksz_ptp_data { struct ptp_clock_info caps; struct ptp_clock *clock; @@ -18,6 +23,9 @@ struct ksz_ptp_data { /* lock for accessing the clock_time */ spinlock_t clock_lock; struct timespec64 clock_time; + enum ksz_ptp_tou_mode tou_mode; + struct timespec64 perout_target_time_first; /* start of first perout pulse */ + struct timespec64 perout_period; }; int ksz_ptp_clock_register(struct dsa_switch *ds); diff --git a/drivers/net/dsa/microchip/ksz_ptp_reg.h b/drivers/net/dsa/microchip/ksz_ptp_reg.h index 2ae6c8b01b00..0f391a01a7f7 100644 --- a/drivers/net/dsa/microchip/ksz_ptp_reg.h +++ b/drivers/net/dsa/microchip/ksz_ptp_reg.h @@ -3,6 +3,14 @@ * Copyright (C) 2019-2021 Microchip Technology Inc. */ +#define REG_SW_GLOBAL_LED_OVR__4 0x0120 +#define LED_OVR_2 BIT(1) +#define LED_OVR_1 BIT(0) + +#define REG_SW_GLOBAL_LED_SRC__4 0x0128 +#define LED_SRC_PTP_GPIO_1 BIT(3) +#define LED_SRC_PTP_GPIO_2 BIT(2) + /* 5 - PTP Clock */ #define REG_PTP_CLK_CTRL 0x0500 @@ -51,6 +59,77 @@ #define PTP_MASTER BIT(1) #define PTP_1STEP BIT(0) +#define REG_PTP_UNIT_INDEX__4 0x0520 + +#define PTP_UNIT_M 0xF + +#define PTP_GPIO_INDEX_S 16 +#define PTP_TSI_INDEX_S 8 +#define PTP_TOU_INDEX_S 0 + +#define REG_PTP_TRIG_STATUS__4 0x0524 + +#define TRIG_ERROR_S 16 +#define TRIG_DONE_S 0 + +#define REG_PTP_INT_STATUS__4 0x0528 + +#define TRIG_INT_S 16 +#define TS_INT_S 0 + +#define TRIG_UNIT_M 0x7 +#define TS_UNIT_M 0x3 + +#define REG_PTP_CTRL_STAT__4 0x052C + +#define GPIO_IN BIT(7) +#define GPIO_OUT BIT(6) +#define TS_INT_ENABLE BIT(5) +#define TRIG_ACTIVE BIT(4) +#define TRIG_ENABLE BIT(3) +#define TRIG_RESET BIT(2) +#define TS_ENABLE BIT(1) +#define TS_RESET BIT(0) + +#define REG_TRIG_TARGET_NANOSEC 0x0530 +#define REG_TRIG_TARGET_SEC 0x0534 + +#define REG_TRIG_CTRL__4 0x0538 + +#define TRIG_CASCADE_ENABLE BIT(31) +#define TRIG_CASCADE_TAIL BIT(30) +#define TRIG_CASCADE_UPS_M 0xF +#define TRIG_CASCADE_UPS_S 26 +#define TRIG_NOW BIT(25) +#define TRIG_NOTIFY BIT(24) +#define TRIG_EDGE BIT(23) +#define TRIG_PATTERN_S 20 +#define TRIG_PATTERN_M 0x7 +#define TRIG_NEG_EDGE 0 +#define TRIG_POS_EDGE 1 +#define TRIG_NEG_PULSE 2 +#define TRIG_POS_PULSE 3 +#define TRIG_NEG_PERIOD 4 +#define TRIG_POS_PERIOD 5 +#define TRIG_REG_OUTPUT 6 +#define TRIG_GPO_S 16 +#define TRIG_GPO_M 0xF +#define TRIG_CASCADE_ITERATE_CNT_M 0xFFFF + +#define REG_TRIG_CYCLE_WIDTH 0x053C + +#define REG_TRIG_CYCLE_CNT 0x0540 + +#define TRIG_CYCLE_CNT_M 0xFFFF +#define TRIG_CYCLE_CNT_S 16 +#define TRIG_BIT_PATTERN_M 0xFFFF + +#define REG_TRIG_ITERATE_TIME 0x0544 + +#define REG_TRIG_PULSE_WIDTH__4 0x0548 + +#define TRIG_PULSE_WIDTH_M 0x00FFFFFF + /* Port PTP Register */ #define REG_PTP_PORT_RX_DELAY__2 0x0C00 #define REG_PTP_PORT_TX_DELAY__2 0x0C02 On Wed, 2022-11-02 at 14:11 +0100, Christian Eggers wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you > know the content is safe > Hi Vladimir, > > On Wednesday, 26 October 2022, 23:44:55 CEST, Vladimir Oltean wrote: > > Arun didn't share the PPS output patch publicly, so I don't know > why > > we're discussing this here. Anyway, in it, Arun (incorrectly) > > implemented support for PTP_CLK_REQ_PPS rather than > PTP_CLK_REQ_PEROUT, > > so there will not be any n_periodic_outputs visible in sysfs. For > now, > > try via pps_available and pps_enable. > I can continue testing this week. I can either try with the > (incorrect) > PTP_CLK_REQ_PPS or I can try to forward port my earlier patches. > > > > BTW: Which is the preferred delay measurement which I shall test > (E2E/P2P)? > > > > As this time around there is somebody from Microchip finally on the > > line, I will not interfere in this part. I tried once, and failed > to > > understand the KSZ PTP philosophy. I hope you get some answers from > > Arun. Just one question below. > > > > > I started with E2E is this was configured in the hardware and > needs no 1-step > > > time stamping, but I had to add PTP_MSGTYPE_DELAY_REQ in > ksz_port_txtstamp(). > > > > Hm? So if E2E "doesn't need" 1-step TX timestamping and KSZ9563 > doesn't > > support 2-step TX timestamping, then what kind of TX timestamping > is > > used here for Delay_Req messages? > > Perhaps you mean that E2E doesn't need moving the RX timestamp of > the > > Pdelay_Req (t2) into the KSZ TX timestamp trailer of the > Pdelay_Resp (t3)? > I think that Delay_Req is not related to 1-step / 2-step time > stamping. As far > as I understand, this is only relevant for SYNC and PDelay_Resp. > > > > > May be this is due to kconfig of config_ksz_ptp defined bool > instead > > > > of tristate. Do I need to change the config_ksz_ptp to tristate > in > > > > order to compile as modules? > > > > > > I'm not an expert for kbuild and cannot tell whether it's allowed > to use > > > bool options which depend on tristate options. At least ksz_ptp.c > is compiled > > > by kbuild if tristate is used. But I needed to add additional > EXPORT_SYMBOL() > > > statements for all non-static functions (see below) for > successful linking. > > > > If ksz_ptp.o gets linked into ksz_ptp.ko, then yes. But this > probably > > doesn't make sense, as you point out. So EXPORT_SYMBOL() should not > be > > needed. > > > > > I'm unsure whether it makes sense to build ksz_ptp as a separate > module. > > > Perhaps it should be (conditionally) added to ksz_switch.ko. > > So let's conditionally add ksz_ptp.o to ksz_switch.ko. > > > > static int ksz_set_hwtstamp_config(struct ksz_device *dev, int > port, > > > struct hwtstamp_config *config) > > > @@ -106,7 +108,7 @@ static int ksz_set_hwtstamp_config(struct > ksz_device *dev, int port, > > > case HWTSTAMP_TX_OFF: > > > prt->hwts_tx_en = false; > > > break; > > > - case HWTSTAMP_TX_ON: > > > + case HWTSTAMP_TX_ONESTEP_P2P: > > > > One shouldn't replace the other; this implementation is simplistic, > of course. > > > > Also, why did you choose HWTSTAMP_TX_ONESTEP_P2P and not > HWTSTAMP_TX_ONESTEP_SYNC? > Because my (old) ptp4l.conf was configured for P2P+p2p1step. When I > started working on > KSZ9563 PTP two years ago, you suggested doing P2P first, because E2E > is affected by nasty > packet filters on the switch hardware... But for the first tests now > I switched to E2E > for the reasons you mentioned above. > > Probably HWTSTAMP_TX_ON needs to be rejected for KSZ9563 and only > HWTSTAMP_TX_ONESTEP_SYNC >
| |