This patch series adds support for PTP timestamping through the DSA
framework, as well as an implementation for mv8e6xxx switches.
This implementation was targeted at a National Instruments platform
that uses the Marvell 88E6341 (Topaz). I've tried to enable support on
other Marvell switches where the register interfaces seemed compatible,
but I don't have the hardware to verify their operation myself.
This series probably ties in well with Richard's comment last week
("Re: [RFC net-next 0/5] TSN: Add qdisc-based config interfaces for
traffic shapers") about figuring out proper interfaces for managing
switch-level PTP timestamps.
A couple patches that I expect may need further polishing:
- Patch #2: We expose the switch time as a PTP clock but don't support
adjustment (max_adj=0). Our platform adjusted a systemwide oscillator
from userspace, so we didn't need adjustment at this layer, but other
PTP clock drivers support this and we probably should too.
- Patch #3: The GPIO config support is handled in a very simple manner.
I suspect a longer term goal would be to use pinctrl here.
- Patch #6: the dsa_switch pointer and port index is plumbed from
dsa_device_ops::rcv so that we can call the correct port_rxtstamp
method. This involved instrumenting all of the *_tag_rcv functions in
a way that's kind of a kludge and that I'm not terribly happy with.
This applies to net-next as of 14a0d032f4ec.
Feedback is appreciated.
-- brandon
Brandon Streiff (9):
net: dsa: mv88e6xxx: add accessors for PTP/TAI registers
net: dsa: mv88e6xxx: expose switch time as a PTP hardware clock
net: dsa: mv88e6xxx: add support for GPIO configuration
net: dsa: mv88e6xxx: add support for event capture
net: dsa: forward hardware timestamping ioctls to switch driver
net: dsa: forward timestamping callbacks to switch drivers
ptp: add offset for reserved field to header
net: dsa: mv88e6xxx: add rx/tx timestamping support
net: dsa: mv88e6xxx: add workaround for 6341 timestamping
drivers/net/dsa/mv88e6xxx/Kconfig | 10 +
drivers/net/dsa/mv88e6xxx/Makefile | 2 +
drivers/net/dsa/mv88e6xxx/chip.c | 65 +++++
drivers/net/dsa/mv88e6xxx/chip.h | 71 +++++
drivers/net/dsa/mv88e6xxx/global2.c | 244 ++++++++++++++++
drivers/net/dsa/mv88e6xxx/global2.h | 59 +++-
drivers/net/dsa/mv88e6xxx/hwtstamp.c | 548 +++++++++++++++++++++++++++++++++++
drivers/net/dsa/mv88e6xxx/hwtstamp.h | 171 +++++++++++
drivers/net/dsa/mv88e6xxx/ptp.c | 493 +++++++++++++++++++++++++++++++
drivers/net/dsa/mv88e6xxx/ptp.h | 99 +++++++
include/linux/ptp_classify.h | 1 +
include/net/dsa.h | 28 +-
net/dsa/dsa.c | 39 ++-
net/dsa/slave.c | 67 ++++-
net/dsa/tag_brcm.c | 6 +-
net/dsa/tag_dsa.c | 6 +-
net/dsa/tag_edsa.c | 6 +-
net/dsa/tag_ksz.c | 6 +-
net/dsa/tag_lan9303.c | 6 +-
net/dsa/tag_mtk.c | 6 +-
net/dsa/tag_qca.c | 6 +-
net/dsa/tag_trailer.c | 6 +-
22 files changed, 1929 insertions(+), 16 deletions(-)
create mode 100644 drivers/net/dsa/mv88e6xxx/hwtstamp.c
create mode 100644 drivers/net/dsa/mv88e6xxx/hwtstamp.h
create mode 100644 drivers/net/dsa/mv88e6xxx/ptp.c
create mode 100644 drivers/net/dsa/mv88e6xxx/ptp.h
--
2.1.4
Forward the rx/tx timestamp machinery from the dsa infrastructure to the
switch driver.
On the rx side, defer delivery of skbs until we have an rx timestamp.
This mimicks the behavior of skb_defer_rx_timestamp. The implementation
does have to thread through the tagging protocol handlers because
it is where that we know which switch and port the skb goes to.
On the tx side, identify PTP packets, clone them, and pass them to the
underlying switch driver before we transmit. This mimicks the behavior
of skb_tx_timestamp.
Signed-off-by: Brandon Streiff <redacted>
---
include/net/dsa.h | 13 +++++++++++--
net/dsa/dsa.c | 39 ++++++++++++++++++++++++++++++++++++++-
net/dsa/slave.c | 25 +++++++++++++++++++++++++
net/dsa/tag_brcm.c | 6 +++++-
net/dsa/tag_dsa.c | 6 +++++-
net/dsa/tag_edsa.c | 6 +++++-
net/dsa/tag_ksz.c | 6 +++++-
net/dsa/tag_lan9303.c | 6 +++++-
net/dsa/tag_mtk.c | 6 +++++-
net/dsa/tag_qca.c | 6 +++++-
net/dsa/tag_trailer.c | 6 +++++-
11 files changed, 114 insertions(+), 11 deletions(-)
@@ -134,7 +137,9 @@ struct dsa_switch_tree {/* Copy of tag_ops->rcv for faster access in hot path */structsk_buff*(*rcv)(structsk_buff*skb,structnet_device*dev,-structpacket_type*pt);+structpacket_type*pt,+structdsa_switch**src_dev,+int*src_port);/**TheswitchporttowhichtheCPUisattached.
@@ -157,6 +158,37 @@ struct net_device *dsa_dev_to_net_device(struct device *dev)}EXPORT_SYMBOL_GPL(dsa_dev_to_net_device);+/* Determine if we should defer delivery of skb until we have a rx timestamp.+*+*Calledfromdsa_switch_rcv.Fornow,thiswillonlyworkiftaggingis+*enabledontheswitch.NormallytheMACdriverwouldretrievethehardware+*timestampwhenitreadsthepacketoutofthehardware.HoweverinaDSA+*switch,theDSAdriverowningtheinterfacetowhichthepacketis+*deliveredisnevernotifiedunlesswedosohere.+*/+staticbooldsa_skb_defer_rx_timestamp(structdsa_switch*ds,intport,+structsk_buff*skb)+{+unsignedinttype;++if(skb_headroom(skb)<ETH_HLEN)+returnfalse;++__skb_push(skb,ETH_HLEN);++type=ptp_classify_raw(skb);++__skb_pull(skb,ETH_HLEN);++if(type==PTP_CLASS_NONE)+returnfalse;++if(likely(ds->ops->port_rxtstamp))+returnds->ops->port_rxtstamp(ds,port,skb,type);++returnfalse;+}+staticintdsa_switch_rcv(structsk_buff*skb,structnet_device*dev,structpacket_type*pt,structnet_device*unused){
@@ -419,6 +439,11 @@ static netdev_tx_t dsa_slave_xmit(struct sk_buff *skb, struct net_device *dev)s->tx_bytes+=skb->len;u64_stats_update_end(&s->syncp);+/* Identify PTP protocol packets, clone them, and pass them to the+*switchdriver+*/+dsa_skb_tx_timestamp(p,skb);+/* Transmit function may have to reallocate the original SKB,*inwhichcaseitmusthavefreedit.Onlyfreeithereonerror.*/
This patch implements support for accessing PTP/TAI registers through
the AVB register interface in the Global 2 register.
The register interface differs slightly between different models; older
models use a 3-bit operations field, while newer models use a 2-bit
field. The operations values and the special "global port" values are
different between the two. This is a similar split to the differences
in the "Ingress Rate" register between models, so, like in that case,
we call the two variants "6352" and "6390" and create an ops structure
to abstract between the two.
Signed-off-by: Brandon Streiff <redacted>
---
drivers/net/dsa/mv88e6xxx/chip.c | 9 ++
drivers/net/dsa/mv88e6xxx/chip.h | 17 ++++
drivers/net/dsa/mv88e6xxx/global2.c | 173 ++++++++++++++++++++++++++++++++++++
drivers/net/dsa/mv88e6xxx/global2.h | 27 +++++-
4 files changed, 225 insertions(+), 1 deletion(-)
88E6341 devices default to timestamping at the PHY, but due to a
hardware issue, timestamps via this component are unreliable. For
this family, configure the PTP hardware to force the timestamping
to occur at the MAC.
Signed-off-by: Brandon Streiff <redacted>
---
drivers/net/dsa/mv88e6xxx/hwtstamp.c | 13 +++++++++++++
drivers/net/dsa/mv88e6xxx/hwtstamp.h | 9 +++++++++
2 files changed, 22 insertions(+)
@@ -523,6 +523,19 @@ int mv88e6xxx_hwtstamp_setup(struct mv88e6xxx_chip *chip)returnerr;}+/* 88E6341 devices default to timestamping at the PHY, but this has+*ahardwareissuethatresultsinunreliabletimestamps.Force+*thesedevicestotimestampattheMAC.+*/+if(chip->info->family==MV88E6XXX_FAMILY_6341){+u16val=MV88E6341_PTP_CFG_UPDATE|+MV88E6341_PTP_CFG_MODE_IDX|+MV88E6341_PTP_CFG_MODE_TS_AT_MAC;+err=mv88e6xxx_ptp_write(chip,MV88E6341_PTP_CFG,val);+if(err)+returnerr;+}+return0;}
This patch implements RX/TX timestamping support.
The Marvell PTP hardware supports RX timestamping individual message
types, but for simplicity we only support the EVENT receive filter since
few if any clients bother with the more specific filter types.
We also utilize a feature of the "generation 3" PTP hardware that lets
us to embed the timestamp value into one of the reserved fields in the
PTP header. This lets us extract the timestamp out of the header and
avoid an SMI access in the RX codepath. (This implementation does not
presently support the older generations.)
Signed-off-by: Brandon Streiff <redacted>
---
drivers/net/dsa/mv88e6xxx/Makefile | 1 +
drivers/net/dsa/mv88e6xxx/chip.c | 16 +-
drivers/net/dsa/mv88e6xxx/chip.h | 26 ++
drivers/net/dsa/mv88e6xxx/hwtstamp.c | 535 +++++++++++++++++++++++++++++++++++
drivers/net/dsa/mv88e6xxx/hwtstamp.h | 162 +++++++++++
5 files changed, 738 insertions(+), 2 deletions(-)
create mode 100644 drivers/net/dsa/mv88e6xxx/hwtstamp.c
create mode 100644 drivers/net/dsa/mv88e6xxx/hwtstamp.h
@@ -163,6 +163,29 @@ struct mv88e6xxx_irq {unsignedintnirqs;};+/* state flags for mv88e6xxx_port_hwtstamp::state */+enum{+MV88E6XXX_HWTSTAMP_ENABLED,+MV88E6XXX_HWTSTAMP_TX_IN_PROGRESS,+};++structmv88e6xxx_port_hwtstamp{+/* Port index */+intport_id;++/* Timestamping state */+unsignedlongstate;++/* Resources for transmit timestamping */+structwork_structtx_tstamp_work;+unsignedlongtx_tstamp_start;+structsk_buff*tx_skb;+u16tx_seq_id;++/* Current timestamp configuration */+structhwtstamp_configtstamp_config;+};+structmv88e6xxx_chip{conststructmv88e6xxx_info*info;
@@ -0,0 +1,535 @@+/*+*Marvell88E6xxxSwitchhardwaretimestampingsupport+*+*Copyright(c)2008MarvellSemiconductor+*+*Copyright(c)2017NationalInstruments+*ErikHons<erik.hons@ni.com>+*BrandonStreiff<brandon.streiff@ni.com>+*DaneWagner<dane.wagner@ni.com>+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodify+*itunderthetermsoftheGNUGeneralPublicLicenseaspublishedby+*theFreeSoftwareFoundation;eitherversion2oftheLicense,or+*(atyouroption)anylaterversion.+*/++#include"chip.h"+#include"global2.h"+#include"hwtstamp.h"+#include<linux/ptp_classify.h>++staticintmv88e6xxx_port_ptp_read(structmv88e6xxx_chip*chip,intport,+intaddr,u16*data,intlen)+{+if(!chip->info->ops->avb_ops->port_ptp_read)+return-EOPNOTSUPP;++returnchip->info->ops->avb_ops->port_ptp_read(chip,port,addr,+data,len);+}++staticintmv88e6xxx_port_ptp_write(structmv88e6xxx_chip*chip,intport,+intaddr,u16data)+{+if(!chip->info->ops->avb_ops->port_ptp_write)+return-EOPNOTSUPP;++returnchip->info->ops->avb_ops->port_ptp_write(chip,port,addr,+data);+}++staticintmv88e6xxx_ptp_write(structmv88e6xxx_chip*chip,intaddr,+u16data)+{+if(!chip->info->ops->avb_ops->ptp_write)+return-EOPNOTSUPP;++returnchip->info->ops->avb_ops->ptp_write(chip,addr,data);+}++/* TX_TSTAMP_TIMEOUT: This limits the time spent polling for a TX+*timestamp.Whenworkingproperly,hardwarewillproduceatimestamp+*within1ms.SoftwaremayenounterdelaysduetoMDIOcontention,so+*thetimeoutissetaccordingly.+*/+#define TX_TSTAMP_TIMEOUT msecs_to_jiffies(20)++intmv88e6xxx_get_ts_info(structdsa_switch*ds,intport,+structethtool_ts_info*info)+{+structmv88e6xxx_chip*chip=ds->priv;++if(!chip->info->ptp_support)+return-EOPNOTSUPP;++info->so_timestamping=+SOF_TIMESTAMPING_TX_HARDWARE|+SOF_TIMESTAMPING_RX_HARDWARE|+SOF_TIMESTAMPING_RAW_HARDWARE;+info->phc_index=ptp_clock_index(chip->ptp_clock);+info->tx_types=+(1<<HWTSTAMP_TX_OFF)|+(1<<HWTSTAMP_TX_ON);+info->rx_filters=+(1<<HWTSTAMP_FILTER_NONE)|+(1<<HWTSTAMP_FILTER_PTP_V2_L4_EVENT)|+(1<<HWTSTAMP_FILTER_PTP_V2_L4_SYNC)|+(1<<HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ)|+(1<<HWTSTAMP_FILTER_PTP_V2_L2_EVENT)|+(1<<HWTSTAMP_FILTER_PTP_V2_L2_SYNC)|+(1<<HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ)|+(1<<HWTSTAMP_FILTER_PTP_V2_EVENT)|+(1<<HWTSTAMP_FILTER_PTP_V2_SYNC)|+(1<<HWTSTAMP_FILTER_PTP_V2_DELAY_REQ);++return0;+}++staticintmv88e6xxx_set_hwtstamp_config(structmv88e6xxx_chip*chip,intport,+structhwtstamp_config*config)+{+structmv88e6xxx_port_hwtstamp*ps=&chip->port_hwtstamp[port];+booltstamp_enable=false;+u16port_config0;+interr;++/* Prevent the TX/RX paths from trying to interact with the+*timestamphardwarewhilewereconfigureit.+*/+clear_bit_unlock(MV88E6XXX_HWTSTAMP_ENABLED,&ps->state);++/* reserved for future extensions */+if(config->flags)+return-EINVAL;++switch(config->tx_type){+caseHWTSTAMP_TX_OFF:+tstamp_enable=false;+break;+caseHWTSTAMP_TX_ON:+tstamp_enable=true;+break;+default:+return-ERANGE;+}++/* The switch supports timestamping both L2 and L4; one cannot be+*disabledindependentlyoftheother.+*/+switch(config->rx_filter){+caseHWTSTAMP_FILTER_NONE:+tstamp_enable=false;+break;+caseHWTSTAMP_FILTER_PTP_V2_L4_EVENT:+caseHWTSTAMP_FILTER_PTP_V2_L4_SYNC:+caseHWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ:+caseHWTSTAMP_FILTER_PTP_V2_L2_EVENT:+caseHWTSTAMP_FILTER_PTP_V2_L2_SYNC:+caseHWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ:+caseHWTSTAMP_FILTER_PTP_V2_EVENT:+caseHWTSTAMP_FILTER_PTP_V2_SYNC:+caseHWTSTAMP_FILTER_PTP_V2_DELAY_REQ:+config->rx_filter=HWTSTAMP_FILTER_PTP_V2_EVENT;+break;+caseHWTSTAMP_FILTER_ALL:+default:+config->rx_filter=HWTSTAMP_FILTER_NONE;+return-ERANGE;+}++if(tstamp_enable){+/* Disable transportSpecific value matching, so that packets+*witheither1588(0)and802.1AS(1)willbetimestamped.+*/+port_config0=MV88E6XXX_PORT_PTP_CFG0_DISABLE_TSPEC_MATCH;+}else{+/* Disable PTP. This disables both RX and TX timestamping. */+port_config0=MV88E6XXX_PORT_PTP_CFG0_DISABLE_PTP;+}++mutex_lock(&chip->reg_lock);+err=mv88e6xxx_port_ptp_write(chip,port,MV88E6XXX_PORT_PTP_CFG0,+port_config0);+mutex_unlock(&chip->reg_lock);++if(err<0)+returnerr;++/* Once hardware has been configured, enable timestamp checks+*intheRX/TXpaths.+*/+if(tstamp_enable)+set_bit(MV88E6XXX_HWTSTAMP_ENABLED,&ps->state);++return0;+}++intmv88e6xxx_port_hwtstamp_set(structdsa_switch*ds,intport,+structifreq*ifr)+{+structmv88e6xxx_chip*chip=ds->priv;+structmv88e6xxx_port_hwtstamp*ps=&chip->port_hwtstamp[port];+structhwtstamp_configconfig;+interr;++if(!chip->info->ptp_support)+return-EOPNOTSUPP;++if(port<0||port>=mv88e6xxx_num_ports(chip))+return-EINVAL;++if(copy_from_user(&config,ifr->ifr_data,sizeof(config)))+return-EFAULT;++err=mv88e6xxx_set_hwtstamp_config(chip,port,&config);+if(err)+returnerr;++/* Save the chosen configuration to be returned later. */+memcpy(&ps->tstamp_config,&config,sizeof(config));++returncopy_to_user(ifr->ifr_data,&config,sizeof(config))?+-EFAULT:0;+}++intmv88e6xxx_port_hwtstamp_get(structdsa_switch*ds,intport,+structifreq*ifr)+{+structmv88e6xxx_chip*chip=ds->priv;+structmv88e6xxx_port_hwtstamp*ps=&chip->port_hwtstamp[port];+structhwtstamp_config*config=&ps->tstamp_config;++if(!chip->info->ptp_support)+return-EOPNOTSUPP;++if(port<0||port>=mv88e6xxx_num_ports(chip))+return-EINVAL;++returncopy_to_user(ifr->ifr_data,config,sizeof(*config))?+-EFAULT:0;+}++/* Get the start of the PTP header in this skb */+staticu8*_get_ptp_header(structsk_buff*skb,unsignedinttype)+{+unsignedintoffset=0;+u8*data=skb_mac_header(skb);++if(type&PTP_CLASS_VLAN)+offset+=VLAN_HLEN;++switch(type&PTP_CLASS_PMASK){+casePTP_CLASS_IPV4:+offset+=ETH_HLEN+IPV4_HLEN(data+offset)+UDP_HLEN;+break;+casePTP_CLASS_IPV6:+offset+=ETH_HLEN+IP6_HLEN+UDP_HLEN;+break;+casePTP_CLASS_L2:+offset+=ETH_HLEN;+break;+default:+returnERR_PTR(-EINVAL);+}++/* Ensure that the entire header is present in this packet. */+if(skb->len+ETH_HLEN<offset+34)+returnERR_PTR(-EINVAL);++returndata+offset;+}++staticboolmv88e6xxx_should_tstamp(structmv88e6xxx_chip*chip,intport,+structsk_buff*skb,unsignedinttype)+{+structmv88e6xxx_port_hwtstamp*ps=&chip->port_hwtstamp[port];+u8*ptp_hdr,*msgtype;+boolret;++if(port<0||port>=mv88e6xxx_num_ports(chip))+returnfalse;++ptp_hdr=_get_ptp_header(skb,type);+if(IS_ERR(ptp_hdr))+returnfalse;++if(unlikely(type&PTP_CLASS_V1))+msgtype=ptp_hdr+OFF_PTP_CONTROL;+else+msgtype=ptp_hdr;++ret=test_bit(MV88E6XXX_HWTSTAMP_ENABLED,&ps->state);++dev_dbg(chip->dev,+"p%d: PTP message classification 0x%x type 0x%x, tstamp? %d",+port,type,*msgtype,(int)ret);++returnret;+}++/* rxtstamp will be called in interrupt context so we don't to do+*anythinglikereadPTPregistersoverSMI.+*/+boolmv88e6xxx_port_rxtstamp(structdsa_switch*ds,intport,+structsk_buff*skb,unsignedinttype)+{+structmv88e6xxx_chip*chip=ds->priv;+structskb_shared_hwtstamps*shhwtstamps;+__be32*ptp_rx_ts;+u8*ptp_hdr;+u32raw_ts;+u64ns;++if(!chip->info->ptp_support)+returnfalse;++if(port<0||port>=mv88e6xxx_num_ports(chip))+returnfalse;++if(!mv88e6xxx_should_tstamp(chip,port,skb,type))+returnfalse;++shhwtstamps=skb_hwtstamps(skb);+memset(shhwtstamps,0,sizeof(*shhwtstamps));++/* Because we configured the arrival timestamper to put the counter+*intothe32-bit"reserved"fieldofthePTPheader,wecanretrieve+*thevaluefromthepacketdirectlyinsteadofhavingtoretrieveit+*viaSMI.+*/+ptp_hdr=_get_ptp_header(skb,type);+if(IS_ERR(ptp_hdr))+returnfalse;+ptp_rx_ts=(__be32*)(ptp_hdr+OFF_PTP_RESERVED);+raw_ts=__be32_to_cpu(*ptp_rx_ts);+ns=timecounter_cyc2time(&chip->tstamp_tc,raw_ts);+shhwtstamps->hwtstamp=ns_to_ktime(ns);++dev_dbg(chip->dev,"p%d: rxtstamp %llx\n",port,ns);++returnfalse;+}++staticvoidmv88e6xxx_txtstamp_work(structwork_struct*ugly)+{+structmv88e6xxx_port_hwtstamp*ps=container_of(+ugly,structmv88e6xxx_port_hwtstamp,tx_tstamp_work);+structmv88e6xxx_chip*chip=container_of(+ps,structmv88e6xxx_chip,port_hwtstamp[ps->port_id]);+structsk_buff*tmp_skb;+unsignedlongtmp_tstamp_start;+interr;+u16departure_block[4];+u16tmp_seq_id;++if(!test_bit(MV88E6XXX_HWTSTAMP_TX_IN_PROGRESS,&ps->state))+return;++tmp_skb=ps->tx_skb;+tmp_seq_id=ps->tx_seq_id;+tmp_tstamp_start=ps->tx_tstamp_start;++if(!tmp_skb)+return;++mutex_lock(&chip->reg_lock);+err=mv88e6xxx_port_ptp_read(chip,ps->port_id,+MV88E6XXX_PORT_PTP_DEP_STS,+departure_block,+ARRAY_SIZE(departure_block));+mutex_unlock(&chip->reg_lock);++if(err)+gotofree_and_clear_skb;++if(departure_block[0]&MV88E6XXX_PTP_TS_VALID){+structskb_shared_hwtstampsshhwtstamps;+u64ns;+u32time_raw;+u16status;++/* We have the timestamp; go ahead and clear valid now */+mutex_lock(&chip->reg_lock);+mv88e6xxx_port_ptp_write(chip,ps->port_id,+MV88E6XXX_PORT_PTP_DEP_STS,0);+mutex_unlock(&chip->reg_lock);++status=departure_block[0]&+MV88E6XXX_PTP_TS_STATUS_MASK;+if(status!=MV88E6XXX_PTP_TS_STATUS_NORMAL){+dev_warn(chip->dev,"p%d: tx timestamp overrun\n",+ps->port_id);+gotofree_and_clear_skb;+}++if(departure_block[3]!=tmp_seq_id){+dev_warn(chip->dev,"p%d: unexpected sequence id\n",+ps->port_id);+gotofree_and_clear_skb;+}++memset(&shhwtstamps,0,sizeof(shhwtstamps));+time_raw=((u32)departure_block[2]<<16)|+departure_block[1];+ns=timecounter_cyc2time(&chip->tstamp_tc,time_raw);+shhwtstamps.hwtstamp=ns_to_ktime(ns);++dev_dbg(chip->dev,+"p%d: txtstamp %llx status 0x%04x skb ID 0x%04x hw ID 0x%04x\n",+ps->port_id,ktime_to_ns(shhwtstamps.hwtstamp),+departure_block[0],tmp_seq_id,departure_block[3]);++/* skb_complete_tx_timestamp() will free up the client to make+*anothertimestamp-abletransmit.Wehavetobereadyforit+*--byclearingtheps->tx_skb"flag"--beforehand.+*/++ps->tx_skb=NULL;+clear_bit_unlock(MV88E6XXX_HWTSTAMP_TX_IN_PROGRESS,&ps->state);++skb_complete_tx_timestamp(tmp_skb,&shhwtstamps);++}else{+if(time_is_before_jiffies(+tmp_tstamp_start+TX_TSTAMP_TIMEOUT)){+dev_warn(chip->dev,"p%d: clearing tx timestamp hang\n",+ps->port_id);+gotofree_and_clear_skb;+}++/* The timestamp should be available quickly, while getting it+*ishighpriorityandtimeboundedtoonly10ms.Apollis+*warrantedandthisisthenicestwaytorealizeitinawork+*item.+*/++queue_work(system_highpri_wq,&ps->tx_tstamp_work);+}++return;++free_and_clear_skb:+ps->tx_skb=NULL;+clear_bit_unlock(MV88E6XXX_HWTSTAMP_TX_IN_PROGRESS,&ps->state);++dev_kfree_skb_any(tmp_skb);+}++voidmv88e6xxx_port_txtstamp(structdsa_switch*ds,intport,+structsk_buff*clone,unsignedinttype)+{+structmv88e6xxx_chip*chip=ds->priv;+structmv88e6xxx_port_hwtstamp*ps=&chip->port_hwtstamp[port];++if(!chip->info->ptp_support)+return;++if(port<0||port>=mv88e6xxx_num_ports(chip))+gotoout;++if(unlikely(skb_shinfo(clone)->tx_flags&SKBTX_HW_TSTAMP)&&+mv88e6xxx_should_tstamp(chip,port,clone,type)){+__be16*seq_ptr=(__be16*)(_get_ptp_header(clone,type)++OFF_PTP_SEQUENCE_ID);++if(!test_and_set_bit_lock(MV88E6XXX_HWTSTAMP_TX_IN_PROGRESS,+&ps->state)){+ps->tx_skb=clone;+ps->tx_tstamp_start=jiffies;+ps->tx_seq_id=be16_to_cpup(seq_ptr);++/* Fetching the timestamp is high-priority work because+*802.1ASboundsthetimeforaresponse.+*+*Noneedtocheckresultofqueue_work().ps->tx_skb+*checkensuresworkitemisnotpending(itmaybe+*waitingtoexit)+*/+queue_work(system_highpri_wq,&ps->tx_tstamp_work);+return;+}++/* Otherwise we're already in progress... */+dev_dbg(chip->dev,+"p%d: tx timestamp already in progress, discarding",+port);+}++out:+/* We don't need it after all. */+kfree_skb(clone);+}++staticintmv88e6xxx_hwtstamp_port_setup(structmv88e6xxx_chip*chip,intport)+{+structmv88e6xxx_port_hwtstamp*ps=&chip->port_hwtstamp[port];++ps->port_id=port;+INIT_WORK(&ps->tx_tstamp_work,mv88e6xxx_txtstamp_work);++returnmv88e6xxx_port_ptp_write(chip,port,MV88E6XXX_PORT_PTP_CFG0,+MV88E6XXX_PORT_PTP_CFG0_DISABLE_PTP);+}++staticvoidmv88e6xxx_hwtstamp_port_free(structmv88e6xxx_chip*chip,intport)+{+structmv88e6xxx_port_hwtstamp*ps=&chip->port_hwtstamp[port];++cancel_work_sync(&ps->tx_tstamp_work);+}++intmv88e6xxx_hwtstamp_setup(structmv88e6xxx_chip*chip)+{+inti;+interr;++/* Disable timestamping on all ports. */+for(i=0;i<mv88e6xxx_num_ports(chip);++i){+err=mv88e6xxx_hwtstamp_port_setup(chip,i);+if(err)+returnerr;+}++/* MV88E6XXX_PTP_MSG_TYPE is a mask of PTP message types to+*timestamp.Thisaffectsallportsthathavetimestampingenabled,+*butthetimestampconfigisper-port;thusweconfigureallevents+*hereandonlysupporttheHWTSTAMP_FILTER_*_EVENTfiltertypes.+*/+err=mv88e6xxx_ptp_write(chip,MV88E6XXX_PTP_MSGTYPE,+MV88E6XXX_PTP_MSGTYPE_ALL_EVENT);+if(err)+returnerr;++/* Each event type will be timestamped using ARRIVAL0. */+err=mv88e6xxx_ptp_write(chip,MV88E6XXX_PTP_TS_ARRIVAL_PTR,0x0);+if(err)+returnerr;++/* Configure the switch to embed the (32-bit) arrival timestamps in+*thepackets,inthe"reserved"fieldofthePTPheaderatoctet16+*(OFF_PTP_RESERVED),anddisableinterrupts.Whenwedotheper-port+*configurationlater,wewillalsoallowoverwrites(bynotsetting+*theDISABLE_OVERWRITEbit).Thiscombinationletsushandle+*back-to-backRXpacketseasily,becausewedon'thavetodoanSMI+*accesstoretrievethetimestamp.+*/+for(i=0;i<mv88e6xxx_num_ports(chip);++i){+u16val=MV88E6XXX_PORT_PTP_CFG2_EMBED_ARRIVAL;++err=mv88e6xxx_port_ptp_write(chip,i,+MV88E6XXX_PORT_PTP_CFG2,val);+if(err)+returnerr;+}++return0;+}++voidmv88e6xxx_hwtstamp_free(structmv88e6xxx_chip*chip)+{+inti;++for(i=0;i<mv88e6xxx_num_ports(chip);++i)+mv88e6xxx_hwtstamp_port_free(chip,i);+}
This patch adds support for configuring mv88e6xxx GPIO lines as PTP
pins, so that they may be used for time stamping external events or for
periodic output.
Signed-off-by: Brandon Streiff <redacted>
---
drivers/net/dsa/mv88e6xxx/chip.h | 4 +
drivers/net/dsa/mv88e6xxx/ptp.c | 317 ++++++++++++++++++++++++++++++++++++++-
drivers/net/dsa/mv88e6xxx/ptp.h | 16 ++
3 files changed, 335 insertions(+), 2 deletions(-)
@@ -27,6 +29,14 @@ static int mv88e6xxx_tai_read(struct mv88e6xxx_chip *chip, int addr,returnchip->info->ops->avb_ops->tai_read(chip,addr,data,len);}+staticintmv88e6xxx_tai_write(structmv88e6xxx_chip*chip,intaddr,u16data)+{+if(!chip->info->ops->avb_ops->tai_write)+return-EOPNOTSUPP;++returnchip->info->ops->avb_ops->tai_write(chip,addr,data);+}+staticu64mv88e6xxx_ptp_clock_read(conststructcyclecounter*cc){structmv88e6xxx_chip*chip=
@@ -42,6 +52,144 @@ static u64 mv88e6xxx_ptp_clock_read(const struct cyclecounter *cc)return((u32)phc_time[1]<<16)|phc_time[0];}+staticintmv88e6xxx_disable_trig(structmv88e6xxx_chip*chip)+{+interr;+u16global_config;++chip->trig_config=0;+global_config=(chip->evcap_config|chip->trig_config);+err=mv88e6xxx_tai_write(chip,MV88E6XXX_TAI_CFG,global_config);++returnerr;+}++staticintmv88e6xxx_config_periodic_trig(structmv88e6xxx_chip*chip,+u32ns,u16picos)+{+interr;+u16global_config;++if(picos>=1000)+return-ERANGE;++/* TRIG generation is in units of 8 ns clock periods. Convert ns+*andpsinto8nsclockperiodsandupto8000additionalps+*/+picos+=(ns&0x7)*1000;+ns=ns>>3;++err=mv88e6xxx_tai_write(chip,MV88E6XXX_TAI_TRIG_GEN_AMOUNT_LO,+ns&0xffff);+if(err)+returnerr;++err=mv88e6xxx_tai_write(chip,MV88E6XXX_TAI_TRIG_GEN_AMOUNT_HI,+ns>>16);+if(err)+returnerr;++err=mv88e6xxx_tai_write(chip,MV88E6XXX_TAI_TRIG_CLOCK_COMP,+picos);+if(err)+returnerr;++chip->trig_config=MV88E6XXX_TAI_CFG_TRIG_ENABLE;+global_config=(chip->evcap_config|chip->trig_config);+err=mv88e6xxx_tai_write(chip,MV88E6XXX_TAI_CFG,global_config);++returnerr;+}++/* mv88e6xxx_config_eventcap - configure TAI event capture+*@event:PTP_CLOCK_PPS(internal)orPTP_CLOCK_EXTTS(external)+*@rising:zeroforfalling-edgetrigger,elserising-edgetrigger+*+*Thiswillalsoresetthecapturesequencecounter.+*/+staticintmv88e6xxx_config_eventcap(structmv88e6xxx_chip*chip,intevent,+intrising)+{+u16global_config;+u16cap_config;+interr;++chip->evcap_config=MV88E6XXX_TAI_CFG_CAP_OVERWRITE|+MV88E6XXX_TAI_CFG_CAP_CTR_START;+if(!rising)+chip->evcap_config|=MV88E6XXX_TAI_CFG_EVREQ_FALLING;++global_config=(chip->evcap_config|chip->trig_config);+err=mv88e6xxx_tai_write(chip,MV88E6XXX_TAI_CFG,global_config);+if(err)+returnerr;++if(event==PTP_CLOCK_PPS){+cap_config=MV88E6XXX_TAI_EVENT_STATUS_CAP_TRIG;+}elseif(event==PTP_CLOCK_EXTTS){+/* if STATUS_CAP_TRIG is unset we capture PTP_EVREQ events */+cap_config=0;+}else{+return-EINVAL;+}++/* Write the capture config; this also clears the capture counter */+err=mv88e6xxx_tai_write(chip,MV88E6XXX_TAI_EVENT_STATUS,+cap_config);++returnerr;+}++staticvoidmv88e6xxx_tai_event_work(structwork_struct*ugly)+{+structdelayed_work*dw=to_delayed_work(ugly);+structmv88e6xxx_chip*chip=+container_of(dw,structmv88e6xxx_chip,tai_event_work);+u16ev_status[4];+interr;++mutex_lock(&chip->reg_lock);++err=mv88e6xxx_tai_read(chip,MV88E6XXX_TAI_EVENT_STATUS,+ev_status,ARRAY_SIZE(ev_status));+if(err){+mutex_unlock(&chip->reg_lock);+return;+}++if(ev_status[0]&MV88E6XXX_TAI_EVENT_STATUS_ERROR)+dev_warn(chip->dev,"missed event capture\n");++if(ev_status[0]&MV88E6XXX_TAI_EVENT_STATUS_VALID){+structptp_clock_eventev;+u32raw_ts=((u32)ev_status[2]<<16)|ev_status[1];++/* Clear the valid bit so the next timestamp can come in */+ev_status[0]&=~MV88E6XXX_TAI_EVENT_STATUS_VALID;+err=mv88e6xxx_tai_write(chip,MV88E6XXX_TAI_EVENT_STATUS,+ev_status[0]);++if(ev_status[0]&MV88E6XXX_TAI_EVENT_STATUS_CAP_TRIG){+/* TAI is configured to timestamp internal events.+*ThiswillbeaPPSevent.+*/+ev.type=PTP_CLOCK_PPS;+}else{+/* Otherwise this is an external timestamp */+ev.type=PTP_CLOCK_EXTTS;+}+/* We only have one timestamping channel. */+ev.index=0;+ev.timestamp=timecounter_cyc2time(&chip->tstamp_tc,raw_ts);++ptp_clock_event(chip->ptp_clock,&ev);+}++mutex_unlock(&chip->reg_lock);++schedule_delayed_work(&chip->tai_event_work,TAI_EVENT_WORK_INTERVAL);+}+staticintmv88e6xxx_ptp_adjfine(structptp_clock_info*ptp,longscaled_ppm){if(scaled_ppm==0)
@@ -95,16 +243,163 @@ static int mv88e6xxx_ptp_settime(struct ptp_clock_info *ptp,return0;}+staticintmv88e6xxx_ptp_enable_extts(structmv88e6xxx_chip*chip,+structptp_clock_request*rq,inton)+{+intrising=(rq->extts.flags&PTP_RISING_EDGE);+intpin;+interr;++pin=ptp_find_pin(chip->ptp_clock,PTP_PF_EXTTS,rq->extts.index);++if(pin<0)+return-EBUSY;++mutex_lock(&chip->reg_lock);++if(on){+err=mv88e6xxx_g2_set_gpio_config(+chip,pin,MV88E6XXX_G2_SCRATCH_GPIO_MODE_EVREQ,+MV88E6XXX_G2_SCRATCH_GPIO_DIR_IN);+if(err)+gotoout;++schedule_delayed_work(&chip->tai_event_work,+TAI_EVENT_WORK_INTERVAL);++err=mv88e6xxx_config_eventcap(chip,PTP_CLOCK_EXTTS,+rising);+}else{+err=mv88e6xxx_g2_set_gpio_config(+chip,pin,MV88E6XXX_G2_SCRATCH_GPIO_MODE_GPIO,+MV88E6XXX_G2_SCRATCH_GPIO_DIR_IN);++cancel_delayed_work_sync(&chip->tai_event_work);+}++out:+mutex_unlock(&chip->reg_lock);++returnerr;+}++staticintmv88e6xxx_ptp_enable_perout(structmv88e6xxx_chip*chip,+structptp_clock_request*rq,inton)+{+structtimespects;+u64ns;+intpin;+interr;++pin=ptp_find_pin(chip->ptp_clock,PTP_PF_PEROUT,rq->extts.index);++if(pin<0)+return-EBUSY;++ts.tv_sec=rq->perout.period.sec;+ts.tv_nsec=rq->perout.period.nsec;+ns=timespec_to_ns(&ts);++if(ns>U32_MAX)+return-ERANGE;++mutex_lock(&chip->reg_lock);++err=mv88e6xxx_config_periodic_trig(chip,(u32)ns,0);+if(err)+gotoout;++if(on){+err=mv88e6xxx_g2_set_gpio_config(+chip,pin,MV88E6XXX_G2_SCRATCH_GPIO_MODE_TRIG,+MV88E6XXX_G2_SCRATCH_GPIO_DIR_OUT);+}else{+err=mv88e6xxx_g2_set_gpio_config(+chip,pin,MV88E6XXX_G2_SCRATCH_GPIO_MODE_GPIO,+MV88E6XXX_G2_SCRATCH_GPIO_DIR_IN);+}++out:+mutex_unlock(&chip->reg_lock);++returnerr;+}++staticintmv88e6xxx_ptp_enable_pps(structmv88e6xxx_chip*chip,+structptp_clock_request*rq,inton)+{+intpin;+interr;++pin=ptp_find_pin(chip->ptp_clock,PTP_PF_PEROUT,rq->extts.index);++if(pin<0)+return-EBUSY;++mutex_lock(&chip->reg_lock);++if(on){+err=mv88e6xxx_g2_set_gpio_config(+chip,pin,MV88E6XXX_G2_SCRATCH_GPIO_MODE_TRIG,+MV88E6XXX_G2_SCRATCH_GPIO_DIR_OUT);+if(err)+gotoout;+err=mv88e6xxx_config_periodic_trig(chip,+NSEC_PER_SEC,0);+if(err)+gotoout;++schedule_delayed_work(&chip->tai_event_work,0);++err=mv88e6xxx_config_eventcap(chip,PTP_CLOCK_PPS,1);+}else{+err=mv88e6xxx_g2_set_gpio_config(+chip,pin,MV88E6XXX_G2_SCRATCH_GPIO_MODE_GPIO,+MV88E6XXX_G2_SCRATCH_GPIO_DIR_IN);+if(err)+gotoout;++err=mv88e6xxx_disable_trig(chip);++cancel_delayed_work_sync(&chip->tai_event_work);+}++out:+mutex_unlock(&chip->reg_lock);++returnerr;+}+staticintmv88e6xxx_ptp_enable(structptp_clock_info*ptp,structptp_clock_request*rq,inton){-return-EOPNOTSUPP;+structmv88e6xxx_chip*chip=+container_of(ptp,structmv88e6xxx_chip,ptp_clock_info);++switch(rq->type){+casePTP_CLK_REQ_EXTTS:+returnmv88e6xxx_ptp_enable_extts(chip,rq,on);+casePTP_CLK_REQ_PEROUT:+returnmv88e6xxx_ptp_enable_perout(chip,rq,on);+casePTP_CLK_REQ_PPS:+returnmv88e6xxx_ptp_enable_pps(chip,rq,on);+default:+return-EOPNOTSUPP;+}}staticintmv88e6xxx_ptp_verify(structptp_clock_info*ptp,unsignedintpin,enumptp_pin_functionfunc,unsignedintchan){-return-EOPNOTSUPP;+switch(func){+casePTP_PF_NONE:+casePTP_PF_EXTTS:+casePTP_PF_PEROUT:+break;+casePTP_PF_PHYSYNC:+return-EOPNOTSUPP;+}+return0;}/* The 32-bit timestamp counter overflows every ~34.3 seconds; this task
@@ -132,6 +427,8 @@ static void mv88e6xxx_ptp_overflow_check(struct work_struct *work)intmv88e6xxx_ptp_setup(structmv88e6xxx_chip*chip){+inti;+/* Set up the cycle counter */memset(&chip->tstamp_cc,0,sizeof(chip->tstamp_cc));chip->tstamp_cc.read=mv88e6xxx_ptp_clock_read;
@@ -146,12 +443,27 @@ int mv88e6xxx_ptp_setup(struct mv88e6xxx_chip *chip)chip->last_overflow_check=jiffies;INIT_DELAYED_WORK(&chip->overflow_work,mv88e6xxx_ptp_overflow_check);+INIT_DELAYED_WORK(&chip->tai_event_work,mv88e6xxx_tai_event_work);chip->ptp_clock_info.owner=THIS_MODULE;snprintf(chip->ptp_clock_info.name,sizeof(chip->ptp_clock_info.name),dev_name(chip->dev));chip->ptp_clock_info.max_adj=0;+chip->ptp_clock_info.n_ext_ts=1;+chip->ptp_clock_info.n_per_out=1;+chip->ptp_clock_info.n_pins=mv88e6xxx_num_gpio(chip);+chip->ptp_clock_info.pps=1;++for(i=0;i<chip->ptp_clock_info.n_pins;++i){+structptp_pin_desc*ppd=&chip->pin_config[i];++snprintf(ppd->name,sizeof(ppd->name),"mv88e6xxx_gpio%d",i);+ppd->index=i;+ppd->func=PTP_PF_NONE;+}+chip->ptp_clock_info.pin_config=chip->pin_config;+chip->ptp_clock_info.adjfine=mv88e6xxx_ptp_adjfine;chip->ptp_clock_info.adjtime=mv88e6xxx_ptp_adjtime;chip->ptp_clock_info.gettime64=mv88e6xxx_ptp_gettime;
There is a four-byte "reserved" field at octet 16 in PTPv2.
Signed-off-by: Brandon Streiff <redacted>
---
include/linux/ptp_classify.h | 1 +
1 file changed, 1 insertion(+)
@@ -47,6 +47,7 @@#define PTP_EV_PORT 319#define PTP_GEN_BIT 0x08 /* indicates general message, if set in message type */+#define OFF_PTP_RESERVED 16 /* PTPv2 only */#define OFF_PTP_SOURCE_UUID 22 /* PTPv1 only */#define OFF_PTP_SEQUENCE_ID 30#define OFF_PTP_CONTROL 32 /* PTPv1 only */
The Scratch/Misc register is a windowed interface that provides access
to the GPIO configuration. Provide a new method for configuration of
GPIO functions.
Signed-off-by: Brandon Streiff <redacted>
---
drivers/net/dsa/mv88e6xxx/chip.c | 13 +++++++
drivers/net/dsa/mv88e6xxx/chip.h | 8 +++++
drivers/net/dsa/mv88e6xxx/global2.c | 71 +++++++++++++++++++++++++++++++++++++
drivers/net/dsa/mv88e6xxx/global2.h | 32 +++++++++++++++++
4 files changed, 124 insertions(+)
@@ -971,6 +971,77 @@ int mv88e6xxx_g2_smi_phy_write(struct mv88e6xxx_chip *chip, struct mii_bus *bus,val);}+/* Offset 0x1A: Scratch and Misc. Register */+staticintmv88e6xxx_g2_scratch_reg_read(structmv88e6xxx_chip*chip,+intreg,u8*data)+{+interr;+u16value;++err=mv88e6xxx_g2_write(chip,MV88E6XXX_G2_SCRATCH_MISC_MISC,+reg<<8);+if(err)+returnerr;++err=mv88e6xxx_g2_read(chip,MV88E6XXX_G2_SCRATCH_MISC_MISC,&value);+if(err)+returnerr;++*data=(value&MV88E6XXX_G2_SCRATCH_MISC_DATA_MASK);++return0;+}++staticintmv88e6xxx_g2_scratch_reg_write(structmv88e6xxx_chip*chip,+intreg,u8data)+{+u16value=(reg<<8)|data;++returnmv88e6xxx_g2_update(chip,MV88E6XXX_G2_SCRATCH_MISC_MISC,value);+}++/* Configures the specified pin for the specified function. This function+*doesnotunsetotherpinsconfiguredforthesamefunction.Ifmultiple+*pinsareconfiguredforthesamefunction,thelower-indexpingets+*thatfunctionandthehigher-indexpingoesbacktobeingGPIO.+*/+intmv88e6xxx_g2_set_gpio_config(structmv88e6xxx_chip*chip,intpin,+intfunc,intdir)+{+intmode_reg=MV88E6XXX_G2_SCRATCH_GPIO_MODE(pin);+intdir_reg=MV88E6XXX_G2_SCRATCH_GPIO_DIR(pin);+interr;+u8val;++if(pin<0||pin>=mv88e6xxx_num_gpio(chip))+return-ERANGE;++/* Set function first */+err=mv88e6xxx_g2_scratch_reg_read(chip,mode_reg,&val);+if(err)+returnerr;++/* Zero bits in the field for this GPIO and OR in new config */+val&=~MV88E6XXX_G2_SCRATCH_GPIO_MODE_MASK(pin);+val|=(func<<MV88E6XXX_G2_SCRATCH_GPIO_MODE_OFFSET(pin));++err=mv88e6xxx_g2_scratch_reg_write(chip,mode_reg,val);+if(err)+returnerr;++/* Set direction */+err=mv88e6xxx_g2_scratch_reg_read(chip,dir_reg,&val);+if(err)+returnerr;++/* Zero bits in the field for this GPIO and OR in new config */+val&=~MV88E6XXX_G2_SCRATCH_GPIO_DIR_MASK(pin);+val|=(dir<<MV88E6XXX_G2_SCRATCH_GPIO_DIR_OFFSET(pin));++returnmv88e6xxx_g2_scratch_reg_write(chip,dir_reg,val);+}++/* Offset 0x1B: Watchdog Control */staticintmv88e6097_watchdog_action(structmv88e6xxx_chip*chip,intirq){u16reg;
This patch adds support to the dsa slave network device so that
switch drivers can implement the SIOC[GS]HWTSTAMP ioctls and the
ethtool timestamp-info interface.
Signed-off-by: Brandon Streiff <redacted>
---
include/net/dsa.h | 15 +++++++++++++++
net/dsa/slave.c | 39 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 53 insertions(+), 1 deletion(-)
@@ -0,0 +1,180 @@+/*+*Marvell88E6xxxSwitchPTPsupport+*+*Copyright(c)2008MarvellSemiconductor+*+*Copyright(c)2017NationalInstruments+*ErikHons<erik.hons@ni.com>+*BrandonStreiff<brandon.streiff@ni.com>+*DaneWagner<dane.wagner@ni.com>+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodify+*itunderthetermsoftheGNUGeneralPublicLicenseaspublishedby+*theFreeSoftwareFoundation;eitherversion2oftheLicense,or+*(atyouroption)anylaterversion.+*/++#include"chip.h"+#include"global2.h"+#include"ptp.h"++staticintmv88e6xxx_tai_read(structmv88e6xxx_chip*chip,intaddr,+u16*data,intlen)+{+if(!chip->info->ops->avb_ops->tai_read)+return-EOPNOTSUPP;++returnchip->info->ops->avb_ops->tai_read(chip,addr,data,len);+}++staticu64mv88e6xxx_ptp_clock_read(conststructcyclecounter*cc)+{+structmv88e6xxx_chip*chip=+container_of(cc,structmv88e6xxx_chip,tstamp_cc);+interr;+u16phc_time[2];++err=mv88e6xxx_tai_read(chip,MV88E6XXX_TAI_TIME_LO,phc_time,+ARRAY_SIZE(phc_time));+if(err)+return0;+else+return((u32)phc_time[1]<<16)|phc_time[0];+}++staticintmv88e6xxx_ptp_adjfine(structptp_clock_info*ptp,longscaled_ppm)+{+if(scaled_ppm==0)+return0;++return-EOPNOTSUPP;+}++staticintmv88e6xxx_ptp_adjtime(structptp_clock_info*ptp,s64delta)+{+structmv88e6xxx_chip*chip=+container_of(ptp,structmv88e6xxx_chip,ptp_clock_info);++mutex_lock(&chip->reg_lock);+timecounter_adjtime(&chip->tstamp_tc,delta);+mutex_unlock(&chip->reg_lock);++return0;+}++staticintmv88e6xxx_ptp_gettime(structptp_clock_info*ptp,+structtimespec64*ts)+{+structmv88e6xxx_chip*chip=+container_of(ptp,structmv88e6xxx_chip,ptp_clock_info);+u64ns;++mutex_lock(&chip->reg_lock);+ns=timecounter_read(&chip->tstamp_tc);+chip->last_overflow_check=jiffies;+mutex_unlock(&chip->reg_lock);++*ts=ns_to_timespec64(ns);++return0;+}++staticintmv88e6xxx_ptp_settime(structptp_clock_info*ptp,+conststructtimespec64*ts)+{+structmv88e6xxx_chip*chip=+container_of(ptp,structmv88e6xxx_chip,ptp_clock_info);+u64ns;++ns=timespec64_to_ns(ts);++mutex_lock(&chip->reg_lock);+timecounter_init(&chip->tstamp_tc,&chip->tstamp_cc,ns);+mutex_unlock(&chip->reg_lock);++return0;+}++staticintmv88e6xxx_ptp_enable(structptp_clock_info*ptp,+structptp_clock_request*rq,inton)+{+return-EOPNOTSUPP;+}++staticintmv88e6xxx_ptp_verify(structptp_clock_info*ptp,unsignedintpin,+enumptp_pin_functionfunc,unsignedintchan)+{+return-EOPNOTSUPP;+}++/* The 32-bit timestamp counter overflows every ~34.3 seconds; this task+*forcesperiodicreadssothatwedon'tmissanywraparounds.+*/+#define MV88E6XXX_TAI_OVERFLOW_PERIOD (34 * HZ / 2)+staticvoidmv88e6xxx_ptp_overflow_check(structwork_struct*work)+{+structdelayed_work*dw=to_delayed_work(work);+structmv88e6xxx_chip*chip=+container_of(dw,structmv88e6xxx_chip,overflow_work);+booltimeout=time_is_before_jiffies(chip->last_overflow_check++MV88E6XXX_TAI_OVERFLOW_PERIOD);++if(timeout){+mutex_lock(&chip->reg_lock);+timecounter_read(&chip->tstamp_tc);+chip->last_overflow_check=jiffies;+mutex_unlock(&chip->reg_lock);+}++schedule_delayed_work(&chip->overflow_work,+MV88E6XXX_TAI_OVERFLOW_PERIOD);+}++intmv88e6xxx_ptp_setup(structmv88e6xxx_chip*chip)+{+/* Set up the cycle counter */+memset(&chip->tstamp_cc,0,sizeof(chip->tstamp_cc));+chip->tstamp_cc.read=mv88e6xxx_ptp_clock_read;+chip->tstamp_cc.mask=CYCLECOUNTER_MASK(32);+/* Raw timestamps are in units of 8-ns clock periods. */+chip->tstamp_cc.mult=8;+chip->tstamp_cc.shift=0;++timecounter_init(&chip->tstamp_tc,&chip->tstamp_cc,+ktime_to_ns(ktime_get_real()));++chip->last_overflow_check=jiffies;++INIT_DELAYED_WORK(&chip->overflow_work,mv88e6xxx_ptp_overflow_check);++chip->ptp_clock_info.owner=THIS_MODULE;+snprintf(chip->ptp_clock_info.name,sizeof(chip->ptp_clock_info.name),+dev_name(chip->dev));+chip->ptp_clock_info.max_adj=0;++chip->ptp_clock_info.adjfine=mv88e6xxx_ptp_adjfine;+chip->ptp_clock_info.adjtime=mv88e6xxx_ptp_adjtime;+chip->ptp_clock_info.gettime64=mv88e6xxx_ptp_gettime;+chip->ptp_clock_info.settime64=mv88e6xxx_ptp_settime;+chip->ptp_clock_info.enable=mv88e6xxx_ptp_enable;+chip->ptp_clock_info.verify=mv88e6xxx_ptp_verify;++chip->ptp_clock=ptp_clock_register(&chip->ptp_clock_info,chip->dev);+if(IS_ERR(chip->ptp_clock))+returnPTR_ERR(chip->ptp_clock);++schedule_delayed_work(&chip->overflow_work,+MV88E6XXX_TAI_OVERFLOW_PERIOD);++return0;+}++voidmv88e6xxx_ptp_free(structmv88e6xxx_chip*chip)+{+if(chip->ptp_clock){+cancel_delayed_work_sync(&chip->overflow_work);++ptp_clock_unregister(chip->ptp_clock);+chip->ptp_clock=NULL;+}+}
Only nitpick: please keep the mv88e63{52,90}_g2_avb_ prefix here.
Otherwise thanks for respecting the code organization, very clear patch:
Reviewed-by: Vivien Didelot <redacted>
Vivien
@@ -18,3 +18,13 @@ config NET_DSA_MV88E6XXX_GLOBAL2Itisrequiredonmostchips.Ifthechipyoucompilethesupportfordoesn'thavesuchregistersset,sayNhere.Indoubt,sayY.++configNET_DSA_MV88E6XXX_PTP+bool"PTP support for Marvell 88E6xxx"+defaultn+depends onNET_DSA_MV88E6XXX_GLOBAL2+implyNETWORK_PHY_TIMESTAMPING
Hi Brandon
Cool to see this code.
One probably dumb question so far..
It is the MAC which is doing the time stamping, not they PHY?
So why NETWORK_PHY_TIMESTAMPING?
Andrew
From: Andrew Lunn <andrew@lunn.ch> Date: 2017-09-28 17:03:33
+/* The 32-bit timestamp counter overflows every ~34.3 seconds; this task
+ * forces periodic reads so that we don't miss any wraparounds.
+ */
+#define MV88E6XXX_TAI_OVERFLOW_PERIOD (34 * HZ / 2)
+static void mv88e6xxx_ptp_overflow_check(struct work_struct *work)
+{
+ struct delayed_work *dw = to_delayed_work(work);
+ struct mv88e6xxx_chip *chip =
+ container_of(dw, struct mv88e6xxx_chip, overflow_work);
+ bool timeout = time_is_before_jiffies(chip->last_overflow_check +
+ MV88E6XXX_TAI_OVERFLOW_PERIOD);
+
+ if (timeout) {
Why do you need this timeout? Do you think the kernel will call this
more often than required?
Also, if it did call this function early, you skip the read, and
reschedule. There is then a danger the next read is after the
wraparound.....
This patch adds support to the dsa slave network device so that
switch drivers can implement the SIOC[GS]HWTSTAMP ioctls and the
ethtool timestamp-info interface.
Signed-off-by: Brandon Streiff <redacted>
---
This echoes back to Andrew's comments in patch 2, but we may have to
prefer PHY timestamping over MAC timestamping if both are available?
Richard, is that usually how the preference should be made?
--
Florian
From: Andrew Lunn <andrew@lunn.ch> Date: 2017-09-28 17:36:33
- Patch #3: The GPIO config support is handled in a very simple manner.
I suspect a longer term goal would be to use pinctrl here.
I assume ptp already has the core code to use pinctrl and Linux
standard GPIOs? What does the device tree binding look like? How do
you specify the GPIOs to use?
What we want to avoid is defining an ABI now, otherwise it is going to
be hard to swap to pinctrl later.
- Patch #6: the dsa_switch pointer and port index is plumbed from
dsa_device_ops::rcv so that we can call the correct port_rxtstamp
method. This involved instrumenting all of the *_tag_rcv functions in
a way that's kind of a kludge and that I'm not terribly happy with.
Yes, this is ugly. I will see if i can find a better way to do
this.
Andrew
Forward the rx/tx timestamp machinery from the dsa infrastructure to the
switch driver.
On the rx side, defer delivery of skbs until we have an rx timestamp.
This mimicks the behavior of skb_defer_rx_timestamp. The implementation
does have to thread through the tagging protocol handlers because
it is where that we know which switch and port the skb goes to.
On the tx side, identify PTP packets, clone them, and pass them to the
underlying switch driver before we transmit. This mimicks the behavior
of skb_tx_timestamp.
Signed-off-by: Brandon Streiff <redacted>
---
include/net/dsa.h | 13 +++++++++++--
net/dsa/dsa.c | 39 ++++++++++++++++++++++++++++++++++++++-
net/dsa/slave.c | 25 +++++++++++++++++++++++++
net/dsa/tag_brcm.c | 6 +++++-
net/dsa/tag_dsa.c | 6 +++++-
net/dsa/tag_edsa.c | 6 +++++-
net/dsa/tag_ksz.c | 6 +++++-
net/dsa/tag_lan9303.c | 6 +++++-
net/dsa/tag_mtk.c | 6 +++++-
net/dsa/tag_qca.c | 6 +++++-
net/dsa/tag_trailer.c | 6 +++++-
11 files changed, 114 insertions(+), 11 deletions(-)
@@ -134,7 +137,9 @@ struct dsa_switch_tree {/* Copy of tag_ops->rcv for faster access in hot path */structsk_buff*(*rcv)(structsk_buff*skb,structnet_device*dev,-structpacket_type*pt);+structpacket_type*pt,+structdsa_switch**src_dev,+int*src_port);/**TheswitchporttowhichtheCPUisattached.
@@ -157,6 +158,37 @@ struct net_device *dsa_dev_to_net_device(struct device *dev)}EXPORT_SYMBOL_GPL(dsa_dev_to_net_device);+/* Determine if we should defer delivery of skb until we have a rx timestamp.+*+*Calledfromdsa_switch_rcv.Fornow,thiswillonlyworkiftaggingis+*enabledontheswitch.NormallytheMACdriverwouldretrievethehardware+*timestampwhenitreadsthepacketoutofthehardware.HoweverinaDSA+*switch,theDSAdriverowningtheinterfacetowhichthepacketis+*deliveredisnevernotifiedunlesswedosohere.+*/+staticbooldsa_skb_defer_rx_timestamp(structdsa_switch*ds,intport,+structsk_buff*skb)
You should not need the port information here because it's already
implied from skb->dev which points to the DSA slave network device, see
below.
+{
+ unsigned int type;
+
+ if (skb_headroom(skb) < ETH_HLEN)
+ return false;
Are you positive this is necessary? Because we called dst->rcv() we have
called eth_type_trans() which already made sure about that
Can we also have a fast-path bypass in case time stamping is not
supported by the switch so we don't have to even try to classify this
packet only to realize we don't have a port_rxtsamp() operation later?
You can either gate this with a compile-time option, or use e.g: a
static key or something like an early test?
I don't think this is necessary, what dst->rcv() does is actually
properly assign skb->dev to the correct dsa slave network device, which
has the information about the port number already in its private context.
The Scratch/Misc register is a windowed interface that provides access
to the GPIO configuration. Provide a new method for configuration of
GPIO functions.
Signed-off-by: Brandon Streiff <redacted>
---
With the write and read acquiring and then releasing the lock
immediately, is no there room for this sequence to be interrupted in the
middle and end-up returning inconsistent reads?
+
+static int mv88e6xxx_g2_scratch_reg_write(struct mv88e6xxx_chip *chip,
+ int reg, u8 data)
+{
+ u16 value = (reg << 8) | data;
+
+ return mv88e6xxx_g2_update(chip, MV88E6XXX_G2_SCRATCH_MISC_MISC, value);
+}
+
+/* Configures the specified pin for the specified function. This function
+ * does not unset other pins configured for the same function. If multiple
+ * pins are configured for the same function, the lower-index pin gets
+ * that function and the higher-index pin goes back to being GPIO.
+ */
+int mv88e6xxx_g2_set_gpio_config(struct mv88e6xxx_chip *chip, int pin,
+ int func, int dir)
+{
+ int mode_reg = MV88E6XXX_G2_SCRATCH_GPIO_MODE(pin);
+ int dir_reg = MV88E6XXX_G2_SCRATCH_GPIO_DIR(pin);
+ int err;
+ u8 val;
+
+ if (pin < 0 || pin >= mv88e6xxx_num_gpio(chip))
+ return -ERANGE;
+
+ /* Set function first */
+ err = mv88e6xxx_g2_scratch_reg_read(chip, mode_reg, &val);
+ if (err)
+ return err;
+
+ /* Zero bits in the field for this GPIO and OR in new config */
+ val &= ~MV88E6XXX_G2_SCRATCH_GPIO_MODE_MASK(pin);
+ val |= (func << MV88E6XXX_G2_SCRATCH_GPIO_MODE_OFFSET(pin));
+
+ err = mv88e6xxx_g2_scratch_reg_write(chip, mode_reg, val);
+ if (err)
+ return err;
+
+ /* Set direction */
+ err = mv88e6xxx_g2_scratch_reg_read(chip, dir_reg, &val);
+ if (err)
+ return err;
+
+ /* Zero bits in the field for this GPIO and OR in new config */
+ val &= ~MV88E6XXX_G2_SCRATCH_GPIO_DIR_MASK(pin);
+ val |= (dir << MV88E6XXX_G2_SCRATCH_GPIO_DIR_OFFSET(pin));
+
+ return mv88e6xxx_g2_scratch_reg_write(chip, dir_reg, val);
+}
Would there be any value in implementing a proper gpiochip structure
here such that other pieces of SW can see this GPIO controller as a
provider and you can reference it from e.g: Device Tree using GPIO
descriptors?
--
Florian
- Patch #3: The GPIO config support is handled in a very simple manner.
I suspect a longer term goal would be to use pinctrl here.
I assume ptp already has the core code to use pinctrl and Linux
standard GPIOs? What does the device tree binding look like? How do
you specify the GPIOs to use?
What we want to avoid is defining an ABI now, otherwise it is going to
be hard to swap to pinctrl later.
quoted
- Patch #6: the dsa_switch pointer and port index is plumbed from
dsa_device_ops::rcv so that we can call the correct port_rxtstamp
method. This involved instrumenting all of the *_tag_rcv functions in
a way that's kind of a kludge and that I'm not terribly happy with.
Yes, this is ugly. I will see if i can find a better way to do
this.
See my reply in patch 6, I may be missing something, but once
dst->rdcv() has been called, skb->dev points to the slave network device
which already contains the switch port and switch information in
dsa_slave_priv, so that should lift the need for asking the individual
taggers' rcv() callback to tell us about it.
--
Florian
From: Andrew Lunn <andrew@lunn.ch> Date: 2017-09-28 18:01:16
On Thu, Sep 28, 2017 at 10:45:03AM -0700, Florian Fainelli wrote:
On 09/28/2017 08:25 AM, Brandon Streiff wrote:
quoted
The Scratch/Misc register is a windowed interface that provides access
to the GPIO configuration. Provide a new method for configuration of
GPIO functions.
Signed-off-by: Brandon Streiff <redacted>
---
With the write and read acquiring and then releasing the lock
immediately, is no there room for this sequence to be interrupted in the
middle and end-up returning inconsistent reads?
Hi Florian
The general pattern in this code is that the lock chip->reg_lock is
taken at a higher level. That protects against other threads. The
driver tends to do that at the highest levels, at the entry points
into the driver. I've not yet checked this code follows the pattern
yet. However, we have a check in the low level to ensure the lock has
been taken. So it seems likely the lock is held.
Would there be any value in implementing a proper gpiochip structure
here such that other pieces of SW can see this GPIO controller as a
provider and you can reference it from e.g: Device Tree using GPIO
descriptors?
That would be my preference as well, or maybe a pinctrl driver.
Andrew
From: Vivien Didelot <hidden> Date: 2017-09-28 20:01:32
Hi Brandon,
quoted
Would there be any value in implementing a proper gpiochip structure
here such that other pieces of SW can see this GPIO controller as a
provider and you can reference it from e.g: Device Tree using GPIO
descriptors?
That would be my preference as well, or maybe a pinctrl driver.
Indeed seeing a gpio_chip or a pinctrl controller registered from a
gpio.c or pinctrl.c file in a separate patchset would be great.
Thanks,
Vivien
From: Richard Cochran <richardcochran@gmail.com> Date: 2017-09-29 09:43:29
Brandon,
On Thu, Sep 28, 2017 at 10:25:32AM -0500, Brandon Streiff wrote:
- Patch #2: We expose the switch time as a PTP clock but don't support
adjustment (max_adj=0).
The driver should implement a cyclecounter/timecounter.
Our platform adjusted a systemwide oscillator
from userspace, so we didn't need adjustment at this layer, but other
PTP clock drivers support this and we probably should too.
We don't currently have any way to support this kind of HW or anything
like an external VCO. I would like to find a way to do this, but that
is a different kettle of fish as it might require changes in the PHC
subsystem. For this driver, I think we should get it merged using the
cyclecounter/timecounter (as that will benefit lots of users) and
worry about the external oscillator later.
Feedback is appreciated.
I happy to see this series. I just finished porting an out-of-tree
PHC driver for the Marvell mv88e635x, and I want to mainline it, but I
also have a few uglies.
Unfortunately I am in the middle of a move right now, and so my review
of this series might have to wait a bit. However, I am looking
forward to comparing notes, and then getting this support in.
Thanks,
Richard
Why do you need this timeout? Do you think the kernel will call this
more often than required?
Also, if it did call this function early, you skip the read, and
reschedule. There is then a danger the next read is after the
wraparound.....
That was, conceptually, a copy-paste from ixgbe_ptp.c as I was looking for how to implement the overflow accounting; that driver has a similar time_is_before_jiffies check in ixgbe_ptp_overflow_check.
Although now that I'm looking it over again, I'm also not certain of the need. Even if we're called more frequently than we expect, that doesn't seem to be harmful with regard to timekeeping. Hmm.
-- brandon
From: Andrew Lunn [mailto:andrew@lunn.ch]
Sent: Thursday, September 28, 2017 1:01 PM
quoted
With the write and read acquiring and then releasing the lock
immediately, is no there room for this sequence to be interrupted in the
middle and end-up returning inconsistent reads?
The general pattern in this code is that the lock chip->reg_lock is
taken at a higher level. That protects against other threads. The
driver tends to do that at the highest levels, at the entry points
into the driver. I've not yet checked this code follows the pattern
yet. However, we have a check in the low level to ensure the lock has
been taken. So it seems likely the lock is held.
Yes, the expectation here is that an upper layer takes the reg_lock. All the functions in ptp.c that call this function do that. If they did not, then assert_reg_lock() gets very angry. :)
Perhaps using __must_hold() and similar annotations would also help document the requirements, but we don't seem to use those in this driver today.
-- brandon
From: Florian Fainelli [mailto:f.fainelli@gmail.com]
Sent: Thursday, September 28, 2017 12:40 PM
Can we also have a fast-path bypass in case time stamping is not
supported by the switch so we don't have to even try to classify this
packet only to realize we don't have a port_rxtsamp() operation later?
You can either gate this with a compile-time option, or use e.g: a
static key or something like an early test?
I was trying to follow the existing pattern for skb_defer_rx_timestamp, but that function be turned into a stub by not configuring NETWORK_PHY_TIMESTAMPING. Maybe a similar compile-time token is appropriate.
I don't think this is necessary, what dst->rcv() does is actually
properly assign skb->dev to the correct dsa slave network device, which
has the information about the port number already in its private context.
Yes, looking in that private context seems like it'd be a better approach (and avoids having to touch all the taggers). I'll look into that further.
quoted
+ type = ptp_classify_raw(skb);
+ if (type == PTP_CLASS_NONE)
+ return;
If we don't have a port_txtstamp option, is there even value in
classifying this packet?
There isn't. This could also use a bypass just like the RX case.
-- brandon
From: Andrew Lunn [mailto:andrew@lunn.ch]
Sent: Thursday, September 28, 2017 12:36 PM
I assume ptp already has the core code to use pinctrl and Linux
standard GPIOs? What does the device tree binding look like? How do
you specify the GPIOs to use?
What we want to avoid is defining an ABI now, otherwise it is going to
be hard to swap to pinctrl later.
A ptp_clock_info has an array of struct ptp_pin_desc which defines "pins" with a name ("Hardware specific human readable pin name"), an index, and a bitmask of valid functions. The ptp_pin_desc structure is shared with usermode for the PTP_PIN_GETFUNC and PTP_PIN_SETFUNC ioctls. The pins are also exposed in sysfs (see Documentation/ABI/testing/sysfs-ptp). The underlying implementation for configuring the hardware is left up to the PHC driver. I don't see any drivers today that use the PHC pin API as a layer over pinctrl/gpiochip, but there's no reason that that couldn't be the case.
For mv88e6xxx, we name the pins using the pattern "mv88e6xxx_gpio%d"; this appears to be in line with current practice (igb_ptp.c uses "SDP%d", mlx5 driver uses "mlx5_pps%d"). Usermode code appears to be expected to determine which pin it needs to use. (Our current userspace code, for instance, knows that it needs to find "mv88e6xxx_gpio8".)
For mv88e6xxx, Device Tree does feel like a better option here for declaring names, functions, and pin usages. Not all platforms that use the PTP API use Device Tree though.
-- brandon
From: Andrew Lunn [mailto:andrew@lunn.ch]
Sent: Thursday, September 28, 2017 11:57 AM
It is the MAC which is doing the time stamping, not they PHY?
So why NETWORK_PHY_TIMESTAMPING?
NETWORK_PHY_TIMESTAMPING implies NET_PTP_CLASSIFY (which I do use) and net/core/timestamping.c (which I didn't). It probably makes more sense to just depend on NET_PTP_CLASSIFY directly.
-- brandon
From: Richard Cochran <richardcochran@gmail.com> Date: 2017-10-08 11:59:18
On Fri, Sep 29, 2017 at 03:28:02PM +0000, Brandon Streiff wrote:
NETWORK_PHY_TIMESTAMPING implies NET_PTP_CLASSIFY (which I do use)
and net/core/timestamping.c (which I didn't). It probably makes more
sense to just depend on NET_PTP_CLASSIFY directly.
Yes, that makes sense to do, if you can make it work.
With my driver I tried depending on NET_PTP_CLASSIFY, but there was
some Kconfig issue, and rather than figuring it out I did the lazy
thing and used NETWORK_PHY_TIMESTAMPING.
Thanks,
Richard
From: Richard Cochran <richardcochran@gmail.com> Date: 2017-10-08 12:07:45
On Fri, Sep 29, 2017 at 03:17:02PM +0000, Brandon Streiff wrote:
Although now that I'm looking it over again, I'm also not certain of
the need. Even if we're called more frequently than we expect, that
doesn't seem to be harmful with regard to timekeeping. Hmm.
Just keep it simple and drop the extra logic. It doesn't hurt to
over-sample the clock. Here is what I did:
/* Covers both a 100 or a 125 MHz input clock. */
#define MV88E635X_OVERFLOW_PERIOD (HZ * 16)
static void mv88e635x_overflow_check(struct work_struct *ws)
{
struct timespec64 ts;
struct mv88e6xxx_chip *ps =
container_of(ws, struct mv88e6xxx_chip, oflow_work.work);
mv88e635x_ptp_gettime(&ps->ptp_info, &ts);
pr_debug("mv88e635x overflow check at %lld.%09lu\n",
ts.tv_sec, ts.tv_nsec);
schedule_delayed_work(&ps->oflow_work, MV88E635X_OVERFLOW_PERIOD);
}
Thanks,
Richard
From: Richard Cochran <richardcochran@gmail.com> Date: 2017-10-08 13:13:05
On Thu, Sep 28, 2017 at 10:25:34AM -0700, Florian Fainelli wrote:
This echoes back to Andrew's comments in patch 2, but we may have to
prefer PHY timestamping over MAC timestamping if both are available?
Richard, is that usually how the preference should be made?
No, if the MAC supports time stamping, then it will take precedence,
because the MAC driver doesn't know that the PHY also supports this.
In the case where a board design includes the PHYTER (the one and only
PHY PHC) and a MAC PHC, the user must de-select the MAC support in the
Kconfig in order to use the PHYTER.
So in general, we don't support PHC/timestamping simultaneously in the
MAC and PHY. It would be a lot of work to support this, and the user
timestamping API would have to be extended yet again, and so I think
it is not worth the effort.
Getting back to this patch, it should fall back to PHY timestamping
when the switch device doesn't support timestamping:
case SIOCGHWTSTAMP:
if (ds->ops->port_hwtstamp_get)
return ds->ops->port_hwtstamp_get(ds, port, ifr);
else
return phy_mii_ioctl(dev->phydev, ifr, cmd);
That way, if someone combines a PHYTER with a non-PTP capable switch,
it will just work.
Thanks,
Richard
+ __be16 *seq_ptr = (__be16 *)(_get_ptp_header(clone, type) +
+ OFF_PTP_SEQUENCE_ID);
+
+ if (!test_and_set_bit_lock(MV88E6XXX_HWTSTAMP_TX_IN_PROGRESS,
+ &ps->state)) {
+ ps->tx_skb = clone;
+ ps->tx_tstamp_start = jiffies;
+ ps->tx_seq_id = be16_to_cpup(seq_ptr);
+
+ /* Fetching the timestamp is high-priority work because
+ * 802.1AS bounds the time for a response.
+ *
+ * No need to check result of queue_work(). ps->tx_skb
+ * check ensures work item is not pending (it may be
+ * waiting to exit)
+ */
+ queue_work(system_highpri_wq, &ps->tx_tstamp_work);
+ return;
+ }
+
+ /* Otherwise we're already in progress... */
+ dev_dbg(chip->dev,
+ "p%d: tx timestamp already in progress, discarding",
+ port);
+ }
+
+out:
+ /* We don't need it after all. */
+ kfree_skb(clone);
How about moving this logic should into the caller, letting the tx
callback return a code that tells whether the clone was accepted or
not?
From: Richard Cochran <richardcochran@gmail.com> Date: 2017-10-08 14:52:44
On Thu, Sep 28, 2017 at 10:25:34AM -0500, Brandon Streiff wrote:
+static int mv88e6xxx_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
+{
+ if (scaled_ppm == 0)
+ return 0;
+
+ return -EOPNOTSUPP;
+}
We really want to have an adjustable clock here. More below.
+int mv88e6xxx_ptp_setup(struct mv88e6xxx_chip *chip)
+{
+ /* Set up the cycle counter */
+ memset(&chip->tstamp_cc, 0, sizeof(chip->tstamp_cc));
+ chip->tstamp_cc.read = mv88e6xxx_ptp_clock_read;
+ chip->tstamp_cc.mask = CYCLECOUNTER_MASK(32);
+ /* Raw timestamps are in units of 8-ns clock periods. */
+ chip->tstamp_cc.mult = 8;
+ chip->tstamp_cc.shift = 0;
First of all, the switch can use an external clock, and so at the very
least, the period should be a macro so that if and when we support the
external clock, the macro may be converted into a variable.
Secondly, the mult/shift should be chosen to allow the finest possible
frequency adjustment. Here is what I did:
---
#define N 28
#define CC_MULT (8 << N)
int mv88e635x_setup(struct dsa_switch *ds)
{
struct mv88e6xxx_chip *ps = ds->priv;
ps->cc.read = mv88e635x_global_time_read;
ps->cc.mask = CLOCKSOURCE_MASK(32);
ps->cc.mult = CC_MULT;
ps->cc.shift = N;
timecounter_init(&ps->tc, &ps->cc, ktime_to_ns(ktime_get_real()));
...
}
static int mv88e635x_ptp_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
{
u64 adj;
u32 diff, mult;
int neg_adj = 0;
struct mv88e6xxx_chip *ps =
container_of(ptp, struct mv88e6xxx_chip, ptp_info);
if (ppb < 0) {
neg_adj = 1;
ppb = -ppb;
}
mult = CC_MULT;
adj = mult;
adj *= ppb;
diff = div_u64(adj, 1000000000ULL);
mutex_lock(&ps->clock_mutex);
timecounter_read(&ps->tc);
ps->cc.mult = neg_adj ? mult - diff : mult + diff;
mutex_unlock(&ps->clock_mutex);
return 0;
}
---
(This is the legacy adjfreq method, but you can easily convert it into
the adjfine method.)
Of course, this means that you'll have to drop the periodic output
signal code.
Thanks,
Richard
Here you ignore the phase of the signal given in the trq->perout.start
field. That is not what the user expects. For periodic outputs where
the phase cannot be set, we really would need a new ioctl.
However, in this case, you should just drop this functionality. I
understand that this works with your adjustable external oscillator,
but we cannot support that in mainline (at least, not yet).
Thanks,
Richard
From: Richard Cochran <richardcochran@gmail.com> Date: 2017-10-08 15:12:34
On Thu, Sep 28, 2017 at 10:25:40AM -0500, Brandon Streiff wrote:
We also utilize a feature of the "generation 3" PTP hardware that lets
us to embed the timestamp value into one of the reserved fields in the
PTP header. This lets us extract the timestamp out of the header and
avoid an SMI access in the RX codepath. (This implementation does not
presently support the older generations.)
That is fine for the later models, but we really need the code to read
over MDIO as well. You added .ptp_support = true for those older
switches, and so the present series won't work.
If it helps, maybe I can adapt the relevant code from my driver to
your work.
Thanks,
Richard
From: Richard Cochran <richardcochran@gmail.com> Date: 2017-10-08 15:29:27
On Thu, Sep 28, 2017 at 10:25:40AM -0500, Brandon Streiff wrote:
+void mv88e6xxx_port_txtstamp(struct dsa_switch *ds, int port,
+ struct sk_buff *clone, unsigned int type)
+{
+ struct mv88e6xxx_chip *chip = ds->priv;
+ struct mv88e6xxx_port_hwtstamp *ps = &chip->port_hwtstamp[port];
+
+ if (!chip->info->ptp_support)
+ return;
+
+ if (port < 0 || port >= mv88e6xxx_num_ports(chip))
+ goto out;
+
+ if (unlikely(skb_shinfo(clone)->tx_flags & SKBTX_HW_TSTAMP) &&
+ mv88e6xxx_should_tstamp(chip, port, clone, type)) {
+ __be16 *seq_ptr = (__be16 *)(_get_ptp_header(clone, type) +
+ OFF_PTP_SEQUENCE_ID);
+
+ if (!test_and_set_bit_lock(MV88E6XXX_HWTSTAMP_TX_IN_PROGRESS,
+ &ps->state)) {
+ ps->tx_skb = clone;
+ ps->tx_tstamp_start = jiffies;
+ ps->tx_seq_id = be16_to_cpup(seq_ptr);
+
+ /* Fetching the timestamp is high-priority work because
+ * 802.1AS bounds the time for a response.
Can you please use this?
commit d9535cb7b7603aeb549c697ecdf92024e4d0a650
Author: Grygorii Strashko [off-list ref]
Date: Fri Jul 28 17:30:02 2017 -0500
ptp: introduce ptp auxiliary worker
Many PTP drivers required to perform some asynchronous or periodic work,
like periodically handling PHC counter overflow or handle delayed timestamp
for RX/TX network packets. In most of the cases, such work is implemented
using workqueues. Unfortunately, Kernel workqueues might introduce
significant delay in work scheduling under high system load and on -RT,
which could cause misbehavior of PTP drivers due to internal counter
overflow, for example, and there is no way to tune its execution policy and
priority manuallly.
Hence, The kthread_worker can be used insted of workqueues, as it create
separte named kthread for each worker and its its execution policy and
priority can be configured using chrt tool.
+ * No need to check result of queue_work(). ps->tx_skb
+ * check ensures work item is not pending (it may be
+ * waiting to exit)
+ */
+ queue_work(system_highpri_wq, &ps->tx_tstamp_work);
+ return;
+ }
+
+ /* Otherwise we're already in progress... */
+ dev_dbg(chip->dev,
+ "p%d: tx timestamp already in progress, discarding",
+ port);
+ }
+
+out:
+ /* We don't need it after all. */
+ kfree_skb(clone);
+}
From: Richard Cochran <richardcochran@gmail.com> Date: 2017-10-08 15:38:26
On Fri, Sep 29, 2017 at 05:43:23AM -0400, Richard Cochran wrote:
I happy to see this series. I just finished porting an out-of-tree
PHC driver for the Marvell mv88e635x, and I want to mainline it, but I
also have a few uglies.
This series looks really good. I won't even post my mine, as that
would now be too embarrassing.
I will try to get my hands on some HW, perhaps by the end of October,
in order to test and complete your driver...
Thanks,
Richard
Here you ignore the phase of the signal given in the trq->perout.start
field. That is not what the user expects. For periodic outputs where
the phase cannot be set, we really would need a new ioctl.
However, in this case, you should just drop this functionality. I
understand that this works with your adjustable external oscillator,
but we cannot support that in mainline (at least, not yet).
I've been working with this patchset and just came across this
limitation as well. The periodic timer output is the basis of the Qbv
t0 timer in devices that support Qbv, and setting this up with the
correct start time is pretty important in that context. The hardware
does support setting a start time, but it must be specified according
to the cycle count of the free-running timer rather than a nanosecond
value. I think this can be worked out from the values stored in the
timecounter struct and I'm writing some code for it now, but if you've
already written something I'd be happy to integrate that instead.
Another issue related to this is that while the free-running counter
in the hardware can't be easily adjusted, the periodic event generator
*can* be finely adjusted (via picosecond and sub-picosecond
accumulators) to correct for drift between the local clock and the PTP
grandmaster time. So to be semantically correct, this needs to be both
started at the right time *and* it needs to have the periodic
corrections made so that the fine correction parameters in the
hardware keep it adjusted to be synchronous with PTP grandmaster time.
So, taking this functionality out in the first pass seems like a good
move for Brandon to take, but I'm working on a complete implementation
for it. I think I've got a handle on how to do it, but if you have any
suggestions, I'm open to them.
Levi
From: Richard Cochran <richardcochran@gmail.com> Date: 2017-10-10 01:53:34
On Mon, Oct 09, 2017 at 04:08:50PM -0600, Levi Pearson wrote:
Another issue related to this is that while the free-running counter
in the hardware can't be easily adjusted, the periodic event generator
*can* be finely adjusted (via picosecond and sub-picosecond
accumulators) to correct for drift between the local clock and the PTP
grandmaster time. So to be semantically correct, this needs to be both
started at the right time *and* it needs to have the periodic
corrections made so that the fine correction parameters in the
hardware keep it adjusted to be synchronous with PTP grandmaster time.
So if the accumulators are safe to adjust on the fly, then the
adjfine() method will have to program them with every adjustment.
Thanks,
Richard
Only nitpick: please keep the mv88e63{52,90}_g2_avb_ prefix here.
Otherwise thanks for respecting the code organization, very clear patch:
Reviewed-by: Vivien Didelot <redacted>
Also feel free to move the mv88e6*_g2_avb_ functions into a
global2_avb.c file.
Thank you,
Vivien
From: Richard Cochran <richardcochran@gmail.com> Date: 2017-11-06 14:55:52
On Sun, Oct 08, 2017 at 11:38:21AM -0400, Richard Cochran wrote:
I will try to get my hands on some HW, perhaps by the end of October,
in order to test and complete your driver...
I now have a 88E6352 to test your series on. Unfortunately, it
doesn't really work. Here is what I did.
1. Gave one of the external switch ports an address (ifconfig ext0
192.168.1.111)
2. Ran ptp4l with option 'free_running 1'.
When I run with Layer2 transport and the switch as master, it seems to
work. Any other combination of role + transport fails.
| Switch Role | Tranport | Result |
|-------------+----------+------------------------------------------------|
| master | UDPv4 | no Delay_Resp appear at slave |
| master | UDPv6 | no Delay_Resp appear at slave |
| master | layer2 | seems okay |
| slave | layer2 | Announce messages not getting through to host? |
Have you tested any of this?
Do I need some special switch configuration first?
Thanks,
Richard
From: Andrew Lunn <andrew@lunn.ch> Date: 2017-11-06 15:04:30
On Mon, Nov 06, 2017 at 06:55:46AM -0800, Richard Cochran wrote:
On Sun, Oct 08, 2017 at 11:38:21AM -0400, Richard Cochran wrote:
quoted
I will try to get my hands on some HW, perhaps by the end of October,
in order to test and complete your driver...
I now have a 88E6352 to test your series on. Unfortunately, it
doesn't really work. Here is what I did.
1. Gave one of the external switch ports an address (ifconfig ext0
192.168.1.111)
Hi Richard
I assume you have tested basic networking? You can ping the other
machines in the network?
With DSA, users sometimes forget to set the DSA master interface up.
Then nothing works.
Andrew
Oops, I had "slaveOnly" set in my PC's configuration. So layer2 seems
to work as expected.
Have you tested UDPv4? It doesn't work.
I have not. Our usage has been focused on 802.1AS; the ptp4l settings we
use are the following:
transportSpecific 0x1
ptp_dst_mac 01:80:C2:00:00:0E
p2p_dst_mac 01:80:C2:00:00:0E
network_transport L2
delay_mechanism P2P
time_stamping hardware
One thing that we're not doing (and probably should be) is configuring
multicast frames to 01:1B:19:00:00:00 to be destined to the CPU port.
(01:80:C2:00:00:0E is used for management, so the *_mgmt_rsvd2cpu()
functions give us that "for free".) That might be necessary to make 1588
L2 work properly. I don't know if that would affect 1588 L4, or if
there's anything else missing to make L4 timestamping work from the HW
perspective.
-- brandon
From: Andrew Lunn <andrew@lunn.ch> Date: 2017-11-08 00:09:20
On Tue, Nov 07, 2017 at 08:56:05PM +0000, Brandon Streiff wrote:
quoted
Oops, I had "slaveOnly" set in my PC's configuration. So layer2 seems
to work as expected.
Have you tested UDPv4? It doesn't work.
I have not. Our usage has been focused on 802.1AS; the ptp4l settings we
use are the following:
transportSpecific 0x1
ptp_dst_mac 01:80:C2:00:00:0E
p2p_dst_mac 01:80:C2:00:00:0E
network_transport L2
delay_mechanism P2P
time_stamping hardware
One thing that we're not doing (and probably should be) is configuring
multicast frames to 01:1B:19:00:00:00 to be destined to the CPU port.
(01:80:C2:00:00:0E is used for management, so the *_mgmt_rsvd2cpu()
functions give us that "for free".) That might be necessary to make 1588
L2 work properly. I don't know if that would affect 1588 L4, or if
there's anything else missing to make L4 timestamping work from the HW
perspective.
Is the application performing a join on the group? If so, on which
interface?
I've not tested many multicast applications with DSA. It is possible
we have bugs.
Andrew
From: Andrew Lunn <andrew@lunn.ch> Date: 2017-11-08 03:02:32
One thing that we're not doing (and probably should be) is
configuring multicast frames to 01:1B:19:00:00:00 to be destined to
the CPU port.
So i did a quick test. If the application joins 224.0.1.129 on the
slave interface, the switch will pass the packets to the host and to
the application.
Andrew
From: Richard Cochran <richardcochran@gmail.com> Date: 2017-11-08 03:23:32
On Wed, Nov 08, 2017 at 04:02:26AM +0100, Andrew Lunn wrote:
So i did a quick test. If the application joins 224.0.1.129 on the
slave interface, the switch will pass the packets to the host and to
the application.
The application does join that group on the external (slave)
interface. I'll find out why the delay request mechanism isn't
working...
Thanks,
Richard
From: Richard Cochran <richardcochran@gmail.com> Date: 2017-12-04 01:13:25
On Tue, Nov 07, 2017 at 07:23:27PM -0800, Richard Cochran wrote:
The application does join that group on the external (slave)
interface. I'll find out why the delay request mechanism isn't
working...
Looking back, I now recall that the series lets the HW embed the time
stamps into the protocol buffers. In the case of UDP, this
invalidates the checksum unless the HW corrects it. I'll bet that is
what is happening...
Thanks,
Richard