| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20190118022343.qmorsbg6mjtaq3gi@localhost>
Date: Thu, 17 Jan 2019 18:23:43 -0800
From: Richard Cochran <richardcochran@...il.com>
To: Antoine Tenart <antoine.tenart@...tlin.com>
Cc: davem@...emloft.net, alexandre.belloni@...tlin.com,
UNGLinuxDriver@...rochip.com, ralf@...ux-mips.org,
paul.burton@...s.com, jhogan@...nel.org, netdev@...r.kernel.org,
linux-mips@...r.kernel.org, thomas.petazzoni@...tlin.com,
quentin.schulz@...tlin.com, allan.nielsen@...rochip.com
Subject: Re: [PATCH net-next 8/8] net: mscc: PTP offloading support
On Thu, Jan 17, 2019 at 11:02:12AM +0100, Antoine Tenart wrote:
> This patch adds support for offloading PTP timestamping to the Ocelot
> switch for both 1-step and 2-step modes.
For PTP Hardware Clock drivers, please add the PTP maintainer onto CC.
> +int ocelot_ptp_gettime64(struct ptp_clock_info *ptp, struct timespec64 *ts)
> +{
> + struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
> + u32 val;
> + time64_t s;
> + s64 ns;
> +
> + val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
> + val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK | PTP_PIN_CFG_DOM);
> + val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_SAVE);
> + ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
> +
> + s = ocelot_read_rix(ocelot, PTP_PIN_TOD_SEC_MSB, TOD_ACC_PIN);
> + s += ocelot_read_rix(ocelot, PTP_PIN_TOD_SEC_LSB, TOD_ACC_PIN);
> + ns = ocelot_read_rix(ocelot, PTP_PIN_TOD_NSEC, TOD_ACC_PIN);
> +
> + /* Deal with negative values */
> + if (ns >= 0x3ffffff0 && ns <= 0x3fffffff) {
> + s--;
> + ns &= 0xf;
> + ns += 999999984;
> + }
> +
> + set_normalized_timespec64(ts, s, ns);
> + return 0;
> +}
> +
> +static int ocelot_ptp_settime64(struct ptp_clock_info *ptp,
> + const struct timespec64 *ts)
> +{
> + struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
> + u32 val;
> +
> + val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
> + val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK | PTP_PIN_CFG_DOM);
> + val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_IDLE);
> +
> + ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
> +
> + ocelot_write_rix(ocelot, lower_32_bits(ts->tv_sec), PTP_PIN_TOD_SEC_LSB,
> + TOD_ACC_PIN);
> + ocelot_write_rix(ocelot, upper_32_bits(ts->tv_sec), PTP_PIN_TOD_SEC_MSB,
> + TOD_ACC_PIN);
> + ocelot_write_rix(ocelot, ts->tv_nsec, PTP_PIN_TOD_NSEC, TOD_ACC_PIN);
> +
> + val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
> + val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK | PTP_PIN_CFG_DOM);
> + val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_LOAD);
> +
> + ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
You are writing multiple registers. This code is not safe when called
concurrently.
Ditto for gettime, adjtime, and adjfreq.
> + return 0;
> +}
> +
> +static int ocelot_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta)
> +{
> + if (delta > -(NSEC_PER_SEC / 2) && delta < (NSEC_PER_SEC / 2)) {
> + struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
> + u32 val;
> +
> + val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
> + val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK | PTP_PIN_CFG_DOM);
> + val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_IDLE);
> +
> + ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
> +
> + ocelot_write_rix(ocelot, 0, PTP_PIN_TOD_SEC_LSB, TOD_ACC_PIN);
> + ocelot_write_rix(ocelot, 0, PTP_PIN_TOD_SEC_MSB, TOD_ACC_PIN);
> + ocelot_write_rix(ocelot, delta, PTP_PIN_TOD_NSEC, TOD_ACC_PIN);
> +
> + val = ocelot_read_rix(ocelot, PTP_PIN_CFG, TOD_ACC_PIN);
> + val &= ~(PTP_PIN_CFG_SYNC | PTP_PIN_CFG_ACTION_MASK | PTP_PIN_CFG_DOM);
> + val |= PTP_PIN_CFG_ACTION(PTP_PIN_ACTION_DELTA);
> +
> + ocelot_write_rix(ocelot, val, PTP_PIN_CFG, TOD_ACC_PIN);
> + } else {
> + /* Fall back using ocelot_ptp_settime64 which is not exact. */
> + struct timespec64 ts;
> + u64 now;
> +
> + ocelot_ptp_gettime64(ptp, &ts);
> +
> + now = ktime_to_ns(timespec64_to_ktime(ts));
> + ts = ns_to_timespec64(now + delta);
> +
> + ocelot_ptp_settime64(ptp, &ts);
> + }
> + return 0;
> +}
> +
> +static int ocelot_ptp_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
> +{
Please implement adjfine instead.
> + struct ocelot *ocelot = container_of(ptp, struct ocelot, ptp_info);
> + u64 adj = 0;
> + u32 unit = 0, direction = 0;
> +
> + if (!ppb)
> + goto disable_adj;
> +
> + if (ppb < 0) {
> + direction = PTP_CFG_CLK_ADJ_CFG_DIR;
> + ppb = -ppb;
> + }
> +
> + adj = PSEC_PER_SEC;
> + do_div(adj, ppb);
> +
> + /* If the adjustment value is too large, use ns instead */
> + if (adj >= (1L << 30)) {
> + unit = PTP_CFG_CLK_ADJ_FREQ_NS;
> + do_div(adj, 1000);
> + }
> +
> + /* Still too big */
> + if (adj >= (1L << 30))
> + goto disable_adj;
> +
> + ocelot_write(ocelot, unit | adj, PTP_CLK_CFG_ADJ_FREQ);
> + ocelot_write(ocelot, PTP_CFG_CLK_ADJ_CFG_ENA | direction,
> + PTP_CLK_CFG_ADJ_CFG);
> + return 0;
> +
> +disable_adj:
> + ocelot_write(ocelot, 0, PTP_CLK_CFG_ADJ_CFG);
> + return 0;
> +}
> +
> +static struct ptp_clock_info ocelot_ptp_clock_info = {
> + .owner = THIS_MODULE,
> + .name = "ocelot ptp",
> + .max_adj = 0x7fffffff,
> + .n_alarm = 0,
> + .n_ext_ts = 0,
> + .n_per_out = 0,
> + .n_pins = 0,
> + .pps = 0,
> + .gettime64 = ocelot_ptp_gettime64,
> + .settime64 = ocelot_ptp_settime64,
> + .adjtime = ocelot_ptp_adjtime,
> + .adjfreq = ocelot_ptp_adjfreq,
> +};
> +
> +static int ocelot_init_timestamp(struct ocelot *ocelot)
> +{
> + ocelot->ptp_info = ocelot_ptp_clock_info;
> +
> + ocelot->ptp_clock = ptp_clock_register(&ocelot->ptp_info, ocelot->dev);
> + if (IS_ERR(ocelot->ptp_clock))
> + return PTR_ERR(ocelot->ptp_clock);
> +
> + ocelot_write(ocelot, SYS_PTP_CFG_PTP_STAMP_WID(30), SYS_PTP_CFG);
> + ocelot_write(ocelot, 0xffffffff, ANA_TABLES_PTP_ID_LOW);
> + ocelot_write(ocelot, 0xffffffff, ANA_TABLES_PTP_ID_HIGH);
> +
> + ocelot_write(ocelot, PTP_CFG_MISC_PTP_EN, PTP_CFG_MISC);
> +
> + return 0;
> +}
> +
> int ocelot_probe_port(struct ocelot *ocelot, u8 port,
> void __iomem *regs,
> struct phy_device *phy)
> @@ -1649,6 +2138,8 @@ int ocelot_probe_port(struct ocelot *ocelot, u8 port,
> ocelot_mact_learn(ocelot, PGID_CPU, dev->dev_addr, ocelot_port->pvid,
> ENTRYTYPE_LOCKED);
>
> + INIT_LIST_HEAD(&ocelot_port->skbs);
> +
> err = register_netdev(dev);
> if (err) {
> dev_err(ocelot->dev, "register_netdev failed\n");
> @@ -1669,7 +2160,7 @@ EXPORT_SYMBOL(ocelot_probe_port);
> int ocelot_init(struct ocelot *ocelot)
> {
> u32 port;
> - int i, cpu = ocelot->num_phys_ports;
> + int i, ret, cpu = ocelot->num_phys_ports;
> char queue_name[32];
>
> ocelot->lags = devm_kcalloc(ocelot->dev, ocelot->num_phys_ports,
> @@ -1796,6 +2287,18 @@ int ocelot_init(struct ocelot *ocelot)
> INIT_DELAYED_WORK(&ocelot->stats_work, ocelot_check_stats);
> queue_delayed_work(ocelot->stats_queue, &ocelot->stats_work,
> OCELOT_STATS_CHECK_DELAY);
> +
> + if (ocelot->ptp) {
> + ret = ocelot_init_timestamp(ocelot);
> + if (ret) {
> + dev_err(ocelot->dev,
> + "Timestamp initialization failed\n");
> + return ret;
> + }
> +
> + ocelot_vcap_is2_init(ocelot);
> + }
> +
> return 0;
> }
> EXPORT_SYMBOL(ocelot_init);
> diff --git a/drivers/net/ethernet/mscc/ocelot.h b/drivers/net/ethernet/mscc/ocelot.h
> index 994ba953d60e..ed1583037dc2 100644
> --- a/drivers/net/ethernet/mscc/ocelot.h
> +++ b/drivers/net/ethernet/mscc/ocelot.h
> @@ -11,9 +11,11 @@
> #include <linux/bitops.h>
> #include <linux/etherdevice.h>
> #include <linux/if_vlan.h>
> +#include <linux/net_tstamp.h>
> #include <linux/phy.h>
> #include <linux/phy/phy.h>
> #include <linux/platform_device.h>
> +#include <linux/ptp_clock_kernel.h>
> #include <linux/regmap.h>
>
> #include "ocelot_ana.h"
> @@ -46,6 +48,8 @@ struct frame_info {
> u16 port;
> u16 vid;
> u8 tag_type;
> + u16 rew_op;
> + u32 timestamp; /* rew_val */
> };
>
> #define IFH_INJ_BYPASS BIT(31)
> @@ -54,6 +58,12 @@ struct frame_info {
> #define IFH_TAG_TYPE_C 0
> #define IFH_TAG_TYPE_S 1
>
> +#define IFH_REW_OP_NOOP 0x0
> +#define IFH_REW_OP_DSCP 0x1
> +#define IFH_REW_OP_ONE_STEP_PTP 0x2
> +#define IFH_REW_OP_TWO_STEP_PTP 0x3
> +#define IFH_REW_OP_ORIGIN_PTP 0x5
> +
> #define OCELOT_SPEED_2500 0
> #define OCELOT_SPEED_1000 1
> #define OCELOT_SPEED_100 2
> @@ -401,6 +411,13 @@ enum ocelot_regfield {
> REGFIELD_MAX
> };
>
> +enum ocelot_clk_pins {
> + ALT_PPS_PIN = 1,
> + EXT_CLK_PIN,
> + ALT_LDST_PIN,
> + TOD_ACC_PIN
> +};
> +
> struct ocelot_multicast {
> struct list_head list;
> unsigned char addr[ETH_ALEN];
> @@ -450,6 +467,12 @@ struct ocelot {
> u64 *stats;
> struct delayed_work stats_work;
> struct workqueue_struct *stats_queue;
> +
> + u8 ptp:1;
> + struct ptp_clock *ptp_clock;
> + struct ptp_clock_info ptp_info;
> + struct hwtstamp_config hwtstamp_config;
> + struct mutex ptp_lock;
Just what does this mutex protect? Please add a comment.
Thanks,
Richard
Powered by blists - more mailing lists