Use a dedicated spinlock, rather than the chip register mutex, to protect read access to the hardware timestamp cycle counter. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Luke Howard --- drivers/net/dsa/mv88e6xxx/chip.c | 1 + drivers/net/dsa/mv88e6xxx/chip.h | 7 ++-- drivers/net/dsa/mv88e6xxx/hwtstamp.c | 8 ++--- drivers/net/dsa/mv88e6xxx/ptp.c | 65 +++++++++++++++++++++++++++++++----- drivers/net/dsa/mv88e6xxx/ptp.h | 1 + 5 files changed, 65 insertions(+), 17 deletions(-) diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c index 80b877c74513d..b24ebbb275b34 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c +++ b/drivers/net/dsa/mv88e6xxx/chip.c @@ -6635,6 +6635,7 @@ static struct mv88e6xxx_chip *mv88e6xxx_alloc_chip(struct device *dev) chip->dev = dev; mutex_init(&chip->reg_lock); + spin_lock_init(&chip->ptp_clock_lock); INIT_LIST_HEAD(&chip->mdios); idr_init(&chip->policies); INIT_LIST_HEAD(&chip->msts); diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h index e966e7c4cc5de..7272b7dc4955c 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.h +++ b/drivers/net/dsa/mv88e6xxx/chip.h @@ -419,9 +419,12 @@ struct mv88e6xxx_chip { /* GPIO resources */ u8 gpio_data[2]; - /* This cyclecounter abstracts the switch PTP time. - * reg_lock must be held for any operation that read()s. + /* This cyclecounter abstracts the switch PTP time. ptp_clock_lock + * protects tstamp_cc and tstamp_tc. tstamp_cycles caches the + * result most recently returned by mv88e6xxx_ptp_read_cycles(). */ + spinlock_t ptp_clock_lock; + u64 tstamp_cycles; struct cyclecounter tstamp_cc; struct timecounter tstamp_tc; struct delayed_work overflow_work; diff --git a/drivers/net/dsa/mv88e6xxx/hwtstamp.c b/drivers/net/dsa/mv88e6xxx/hwtstamp.c index d468d988b258c..a5b362a999e64 100644 --- a/drivers/net/dsa/mv88e6xxx/hwtstamp.c +++ b/drivers/net/dsa/mv88e6xxx/hwtstamp.c @@ -293,9 +293,7 @@ static void mv88e6xxx_get_rxts(struct mv88e6xxx_chip *chip, if (mv88e6xxx_ts_valid(status) && seq_match(skb, seq_id)) { ns = timehi << 16 | timelo; - mv88e6xxx_reg_lock(chip); - ns = timecounter_cyc2time(&chip->tstamp_tc, ns); - mv88e6xxx_reg_unlock(chip); + ns = mv88e6xxx_timecounter_cyc2time(chip, ns); shwt = skb_hwtstamps(skb); memset(shwt, 0, sizeof(*shwt)); shwt->hwtstamp = ns_to_ktime(ns); @@ -420,9 +418,7 @@ static int mv88e6xxx_txtstamp_work(struct mv88e6xxx_chip *chip, memset(&shhwtstamps, 0, sizeof(shhwtstamps)); time_raw = ((u32)departure_block[2] << 16) | departure_block[1]; - mv88e6xxx_reg_lock(chip); - ns = timecounter_cyc2time(&chip->tstamp_tc, time_raw); - mv88e6xxx_reg_unlock(chip); + ns = mv88e6xxx_timecounter_cyc2time(chip, time_raw); shhwtstamps.hwtstamp = ns_to_ktime(ns); dev_dbg(chip->dev, diff --git a/drivers/net/dsa/mv88e6xxx/ptp.c b/drivers/net/dsa/mv88e6xxx/ptp.c index f7603573d3a98..2a7f97625e375 100644 --- a/drivers/net/dsa/mv88e6xxx/ptp.c +++ b/drivers/net/dsa/mv88e6xxx/ptp.c @@ -231,15 +231,37 @@ static void mv88e6352_tai_event_work(struct work_struct *ugly) /* We only have one timestamping channel. */ ev.index = 0; - mv88e6xxx_reg_lock(chip); - ev.timestamp = timecounter_cyc2time(&chip->tstamp_tc, raw_ts); - mv88e6xxx_reg_unlock(chip); + ev.timestamp = mv88e6xxx_timecounter_cyc2time(chip, raw_ts); ptp_clock_event(chip->ptp_clock, &ev); out: schedule_delayed_work(&chip->tai_event_work, TAI_EVENT_WORK_INTERVAL); } +/* Refresh the cached counter value that the read() callback returns. The + * read cannot acquire the register lock whilst holding ptp_clock_lock + * because MDIO reads can sleep. The caller must hold reg_lock, which + * serializes against the other timecounter writers. + */ +static void mv88e6xxx_ptp_read_cycles(struct mv88e6xxx_chip *chip) +{ + const struct mv88e6xxx_ptp_ops *ptp_ops = chip->info->ops->ptp_ops; + + if (ptp_ops->clock_read) + chip->tstamp_cycles = ptp_ops->clock_read(&chip->tstamp_cc); +} + +u64 mv88e6xxx_timecounter_cyc2time(struct mv88e6xxx_chip *chip, u64 cycles) +{ + u64 ns; + + spin_lock_bh(&chip->ptp_clock_lock); + ns = timecounter_cyc2time(&chip->tstamp_tc, cycles); + spin_unlock_bh(&chip->ptp_clock_lock); + + return ns; +} + static int mv88e6xxx_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm) { struct mv88e6xxx_chip *chip = ptp_to_chip(ptp); @@ -258,9 +280,12 @@ static int mv88e6xxx_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm) diff = div_u64(adj, chip->cc_coeffs->cc_mult_dem); mv88e6xxx_reg_lock(chip); + mv88e6xxx_ptp_read_cycles(chip); + spin_lock_bh(&chip->ptp_clock_lock); timecounter_read(&chip->tstamp_tc); chip->tstamp_cc.mult = neg_adj ? mult - diff : mult + diff; + spin_unlock_bh(&chip->ptp_clock_lock); mv88e6xxx_reg_unlock(chip); @@ -271,8 +296,16 @@ static int mv88e6xxx_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta) { struct mv88e6xxx_chip *chip = ptp_to_chip(ptp); + /* No register access is needed here, but reg_lock still serialises + * this against the other timecounter writers, which drop it only + * after their hardware read has completed. + */ mv88e6xxx_reg_lock(chip); + + spin_lock_bh(&chip->ptp_clock_lock); timecounter_adjtime(&chip->tstamp_tc, delta); + spin_unlock_bh(&chip->ptp_clock_lock); + mv88e6xxx_reg_unlock(chip); return 0; @@ -285,7 +318,12 @@ static int mv88e6xxx_ptp_gettime(struct ptp_clock_info *ptp, u64 ns; mv88e6xxx_reg_lock(chip); + mv88e6xxx_ptp_read_cycles(chip); + + spin_lock_bh(&chip->ptp_clock_lock); ns = timecounter_read(&chip->tstamp_tc); + spin_unlock_bh(&chip->ptp_clock_lock); + mv88e6xxx_reg_unlock(chip); *ts = ns_to_timespec64(ns); @@ -302,7 +340,12 @@ static int mv88e6xxx_ptp_settime(struct ptp_clock_info *ptp, ns = timespec64_to_ns(ts); mv88e6xxx_reg_lock(chip); + mv88e6xxx_ptp_read_cycles(chip); + + spin_lock_bh(&chip->ptp_clock_lock); timecounter_init(&chip->tstamp_tc, &chip->tstamp_cc, ns); + spin_unlock_bh(&chip->ptp_clock_lock); + mv88e6xxx_reg_unlock(chip); return 0; @@ -444,14 +487,12 @@ const struct mv88e6xxx_ptp_ops mv88e6390_ptp_ops = { (1 << HWTSTAMP_FILTER_PTP_V2_DELAY_REQ), }; +/* Return the value most recently fetched by mv88e6xxx_ptp_read_cycles() + * rather than reading the hardware over MDIO. + */ static u64 mv88e6xxx_ptp_clock_read(struct cyclecounter *cc) { - struct mv88e6xxx_chip *chip = cc_to_chip(cc); - - if (chip->info->ops->ptp_ops->clock_read) - return chip->info->ops->ptp_ops->clock_read(cc); - - return 0; + return cc_to_chip(cc)->tstamp_cycles; } /* With a 250MHz input clock, the 32-bit timestamp counter overflows in ~17.2 @@ -486,6 +527,12 @@ int mv88e6xxx_ptp_setup(struct mv88e6xxx_chip *chip) chip->tstamp_cc.mult = chip->cc_coeffs->cc_mult; chip->tstamp_cc.shift = chip->cc_coeffs->cc_shift; + /* Prime the cycle counter cache for the timecounter_init() below. + * The caller holds reg_lock, and nothing can reach the PTP clock + * until ptp_clock_register() below, so no locking is needed here. + */ + mv88e6xxx_ptp_read_cycles(chip); + timecounter_init(&chip->tstamp_tc, &chip->tstamp_cc, ktime_to_ns(ktime_get_real())); diff --git a/drivers/net/dsa/mv88e6xxx/ptp.h b/drivers/net/dsa/mv88e6xxx/ptp.h index 95bdddb0bf39f..44718e120de20 100644 --- a/drivers/net/dsa/mv88e6xxx/ptp.h +++ b/drivers/net/dsa/mv88e6xxx/ptp.h @@ -68,6 +68,7 @@ int mv88e6xxx_ptp_setup(struct mv88e6xxx_chip *chip); void mv88e6xxx_ptp_free(struct mv88e6xxx_chip *chip); +u64 mv88e6xxx_timecounter_cyc2time(struct mv88e6xxx_chip *chip, u64 cycles); #define ptp_to_chip(ptp) container_of(ptp, struct mv88e6xxx_chip, \ ptp_clock_info) -- 2.43.0