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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Thu, 17 Dec 2020 10:45:39 -0800
From:   Richard Cochran <richardcochran@...il.com>
To:     Divya Koppera <Divya.Koppera@...rochip.com>
Cc:     andrew@...n.ch, hkallweit1@...il.com, linux@...linux.org.uk,
        davem@...emloft.net, kuba@...nel.org, marex@...x.de,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        UNGLinuxDriver@...rochip.com
Subject: Re: [PATCH v4 net-next 2/2] net: phy: mchp: Add 1588 support for
 LAN8814 Quad PHY

On Thu, Dec 17, 2020 at 06:11:50PM +0530, Divya Koppera wrote:

> +struct lan8814_ptphdr {
> +	u8 tsmt; /* transportSpecific | messageType */
> +	u8 ver;  /* reserved0 | versionPTP */
> +	__be16 msglen;
> +	u8 domain;
> +	u8 rsrvd1;
> +	__be16 flags;
> +	__be64 correction;
> +	__be32 rsrvd2;
> +	__be64 clk_identity;
> +	__be16 src_port_id;
> +	__be16 seq_id;
> +	u8 ctrl;
> +	u8 log_interval;
> +} __attribute__((__packed__));

Please use the new 'struct ptp_header' from ptp_classify.h instead.

> +struct kszphy_ptp_priv {
> +	struct mii_timestamper mii_ts;
> +	struct phy_device *phydev;
> +	struct lan8814_ptp ptp;
> +	int hwts_tx_en;
> +	int hwts_rx_en;
> +	int layer;
> +	int version;
> +};
> +
>  struct kszphy_priv {
> +	struct kszphy_ptp_priv ptp_priv;
>  	const struct kszphy_type *type;
>  	int led_mode;
>  	bool rmii_ref_clk_sel;
> @@ -171,6 +313,38 @@ static const struct kszphy_type ksz9021_type = {
>  	.interrupt_level_mask	= BIT(14),
>  };
>  
> +static void lan8814_ptp_clock_get(struct phy_device *phydev,
> +				  u32 *seconds, u32 *nano_seconds,
> +				  u32 *sub_nano_seconds);
> +
> +static int lan8814_read_page_reg(struct phy_device *phydev,
> +				 int page, u32 addr)
> +{
> +	u32 data;
> +
> +	phy_write(phydev, KSZ_EXT_PAGE_ACCESS_CONTROL, page);
> +	phy_write(phydev, KSZ_EXT_PAGE_ACCESS_ADDRESS_DATA, addr);
> +	phy_write(phydev, KSZ_EXT_PAGE_ACCESS_CONTROL, (page | 0x4000));

What prevents concurrent reads from happening?  Nothing, AFAICT.

> +	data = phy_read(phydev, KSZ_EXT_PAGE_ACCESS_ADDRESS_DATA);
> +
> +	return data;
> +}
> +
> +static int lan8814_write_page_reg(struct phy_device *phydev,
> +				  int page, u16 addr, u16 val)
> +{
> +	phy_write(phydev, KSZ_EXT_PAGE_ACCESS_CONTROL, page);

Consider caching the page value in order to make a sequence of
reads/writes more efficient.  (example in dp83640.c)

> +	phy_write(phydev, KSZ_EXT_PAGE_ACCESS_ADDRESS_DATA, addr);
> +	phy_write(phydev, KSZ_EXT_PAGE_ACCESS_CONTROL, (page | 0x4000));

Again, this needs some kind of serialization.  Maybe not here but
rather in the callers... you need to sort that out.

> +	val = phy_write(phydev, KSZ_EXT_PAGE_ACCESS_ADDRESS_DATA, val);
> +	if (val != 0) {
> +		phydev_err(phydev, "Error: phy_write has returned error %d\n",
> +			   val);
> +		return val;
> +	}
> +	return 0;
> +}
> +
>  static int kszphy_extended_write(struct phy_device *phydev,
>  				u32 regnum, u16 val)
>  {
> @@ -185,10 +359,156 @@ static int kszphy_extended_read(struct phy_device *phydev,
>  	return phy_read(phydev, MII_KSZPHY_EXTREG_READ);
>  }
>  
> +static void lan8814_ptp_tx_ts_get(struct phy_device *phydev,
> +				  u32 *seconds, u32 *nano_seconds,
> +				  u32 *seq_id);
> +
> +static struct lan8814_ptphdr *get_ptp_header_l4(struct sk_buff *skb,
> +						struct iphdr *iphdr,
> +						struct udphdr *udphdr)
> +{
> +	if (iphdr->version != 4 || iphdr->protocol != IPPROTO_UDP)
> +		return NULL;
> +
> +	return (struct lan8814_ptphdr *)(((unsigned char *)udphdr) + UDP_HLEN);
> +}
> +
> +static struct lan8814_ptphdr *get_ptp_header_tx(struct sk_buff *skb)
> +{
> +	struct ethhdr *ethhdr = eth_hdr(skb);
> +	struct udphdr *udphdr;
> +	struct iphdr *iphdr;
> +
> +	if (ethhdr->h_proto == htons(ETH_P_1588))
> +		return (struct lan8814_ptphdr *)(((unsigned char *)ethhdr) +
> +				skb_mac_header_len(skb));
> +
> +	if (ethhdr->h_proto != htons(ETH_P_IP))
> +		return NULL;
> +
> +	iphdr = ip_hdr(skb);
> +	udphdr = udp_hdr(skb);
> +
> +	return get_ptp_header_l4(skb, iphdr, udphdr);

Use ptp_parse_header() from ptp_classify.h

> +}
> +
> +static int get_sig(struct sk_buff *skb, u8 *sig)
> +{
> +	struct lan8814_ptphdr *ptphdr = get_ptp_header_tx(skb);
> +	struct ethhdr *ethhdr = eth_hdr(skb);
> +	unsigned int i;
> +
> +	if (!ptphdr)
> +		return -EOPNOTSUPP;
> +
> +	sig[0] = (__force u16)ptphdr->seq_id >> 8;
> +	sig[1] = (__force u16)ptphdr->seq_id & GENMASK(7, 0);
> +	sig[2] = ptphdr->domain;
> +	sig[3] = ptphdr->tsmt & GENMASK(3, 0);
> +
> +	memcpy(&sig[4], ethhdr->h_dest, ETH_ALEN);
> +
> +	/* Fill the last bytes of the signature to reach a 16B signature */
> +	for (i = 10; i < 16; i++)
> +		sig[i] = ptphdr->tsmt & GENMASK(3, 0);
> +
> +	return 0;
> +}
> +
> +static void lan8814_dequeue_skb(struct lan8814_ptp *ptp)
> +{
> +	struct kszphy_ptp_priv *priv = container_of(ptp, struct kszphy_ptp_priv, ptp);
> +	struct phy_device *phydev = priv->phydev;
> +	struct skb_shared_hwtstamps shhwtstamps;
> +	struct sk_buff *skb;
> +	u8 skb_sig[16];
> +	int len;
> +	u32 reg, cnt;
> +	u32 seconds, nsec, seq_id;
> +
> +	reg = lan8814_read_page_reg(phydev, 5, PTP_CAP_INFO);
> +	cnt = PTP_CAP_INFO_TX_TS_CNT_GET_(reg);
> +
> +	/* FIFO is Empty*/
> +	if (cnt == 0)
> +		return;
> +
> +	len = skb_queue_len(&ptp->tx_queue);
> +	if (len < 1)
> +		return;
> +
> +	while (len--) {
> +		skb = __skb_dequeue(&ptp->tx_queue);
> +		if (!skb)
> +			return;
> +
> +		/* Can't get the signature of the packet, won't ever
> +		 * be able to have one so let's dequeue the packet.
> +		 */
> +		if (get_sig(skb, skb_sig) < 0) {
> +			kfree_skb(skb);
> +			continue;
> +		}
> +
> +		lan8814_ptp_tx_ts_get(phydev, &seconds, &nsec, &seq_id);
> +
> +		/* Check if we found the signature we were looking for. */
> +		if (memcmp(skb_sig, &seq_id, sizeof(seq_id))) {
> +			memset(&shhwtstamps, 0, sizeof(shhwtstamps));
> +			shhwtstamps.hwtstamp = ktime_set(seconds, nsec);
> +			skb_complete_tx_timestamp(skb, &shhwtstamps);
> +
> +			return;
> +		}
> +
> +		/* Valid signature but does not match the one of the
> +		 * packet in the FIFO right now, reschedule it for later
> +		 * packets.
> +		 */
> +		__skb_queue_tail(&ptp->tx_queue, skb);
> +	}
> +}
> +
> +static void lan8814_get_tx_ts(struct lan8814_ptp *ptp)
> +{
> +	u32 reg;
> +	struct kszphy_ptp_priv *priv = container_of(ptp, struct kszphy_ptp_priv, ptp);
> +	struct phy_device *phydev = priv->phydev;
> +
> +	do {
> +		lan8814_dequeue_skb(ptp);
> +
> +		/* If other timestamps are available in the FIFO, process them. */
> +		reg = lan8814_read_page_reg(phydev, 5, PTP_CAP_INFO);
> +	} while (PTP_CAP_INFO_TX_TS_CNT_GET_(reg) > 1);
> +}
> +
> +static irqreturn_t lan8814_handle_ptp_interrupt(struct phy_device *phydev)
> +{
> +	struct kszphy_ptp_priv *lan8814 = phydev->priv;
> +	int rc;
> +
> +	rc = lan8814_read_page_reg(phydev, 5, PTP_TSU_INT_STS);
> +
> +	if (!(rc & PTP_TSU_INT_STS_PTP_TX_TS_EN))
> +		return IRQ_NONE;
> +
> +	lan8814_get_tx_ts(&lan8814->ptp);
> +
> +	return IRQ_HANDLED;
> +}
> +
>  static irqreturn_t lan8814_handle_interrupt(struct phy_device *phydev)
>  {
>  	int irq_status;
>  
> +	irq_status = lan8814_read_page_reg(phydev, 4, LAN8814_INTR_STS_REG);
> +	if (irq_status < 0)
> +		return IRQ_NONE;
> +
> +	if (irq_status & LAN8814_INTR_STS_REG_1588_TSU0)
> +		return lan8814_handle_ptp_interrupt(phydev);
> +
>  	irq_status = phy_read(phydev, LAN8814_INTS);
>  	if (irq_status < 0)
>  		return IRQ_NONE;
> @@ -199,10 +519,20 @@ static irqreturn_t lan8814_handle_interrupt(struct phy_device *phydev)
>  	return IRQ_HANDLED;
>  }
>  
> +static int lan8814_config_ts_intr(struct phy_device *phydev)
> +{
> +	return lan8814_write_page_reg(phydev, 5, PTP_TSU_INT_EN, PTP_TSU_INT_EN_PTP_TX_TS_EN |
> +					  PTP_TSU_INT_EN_PTP_TX_TS_OVRFL_EN);
> +}
> +
>  static int lan8814_config_intr(struct phy_device *phydev)
>  {
>  	int temp;
>  
> +	lan8814_write_page_reg(phydev, 4, LAN8814_INTR_CTRL_REG,
> +			       LAN8814_INTR_CTRL_REG_POLARITY |
> +			       LAN8814_INTR_CTRL_REG_INTR_ENABLE);
> +
>  	/* enable / disable interrupts */
>  	if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
>  		temp = LAN8814_INTC_ALL;
> @@ -1187,6 +1517,534 @@ static int kszphy_resume(struct phy_device *phydev)
>  	return 0;
>  }
>  
> +static bool lan8814_ptp_is_enable(struct phy_device *phydev)
> +{
> +	if (phy_read(phydev, PTP_CMD_CTL) & PTP_CMD_CTL_PTP_ENABLE_)
> +		return true;
> +
> +	return false;
> +}
> +
> +static void lan8814_ptp_rx_ts_get(struct phy_device *phydev,
> +				  u32 *seconds, u32 *nano_seconds, u32 *seq_id)
> +{
> +	if (seconds) {

No need to check this.  The one and only caller provides a pointer.

> +		(*seconds) = lan8814_read_page_reg(phydev, 5, PTP_RX_INGRESS_SEC_HI);
> +		(*seconds) = ((*seconds) << 16) |
> +			     lan8814_read_page_reg(phydev, 5, PTP_RX_INGRESS_SEC_LO);
> +	}
> +
> +	if (nano_seconds) {

same here.

> +		(*nano_seconds) = lan8814_read_page_reg(phydev, 5, PTP_RX_INGRESS_NS_HI);
> +		(*nano_seconds) = (((*nano_seconds) & 0x3fff) << 16)
> +				   | lan8814_read_page_reg(phydev, 5, PTP_RX_INGRESS_NS_LO);
> +	}
> +
> +	if (seq_id)

and here.

> +		(*seq_id) = lan8814_read_page_reg(phydev, 5, PTP_RX_MSG_HEADER2);
> +}
> +
> +static void lan8814_ptp_tx_ts_get(struct phy_device *phydev,
> +				  u32 *seconds, u32 *nano_seconds, u32 *seq_id)
> +{
> +	if (seconds) {

ditto.

> +		(*seconds) = lan8814_read_page_reg(phydev, 5, PTP_TX_EGRESS_SEC_HI);
> +		(*seconds) = ((*seconds) << 16) |
> +			     lan8814_read_page_reg(phydev, 5, PTP_TX_EGRESS_SEC_LO);
> +	}
> +
> +	if (nano_seconds) {
> +		(*nano_seconds) = lan8814_read_page_reg(phydev, 5, PTP_TX_EGRESS_NS_HI);
> +		(*nano_seconds) = (((*nano_seconds) & 0x3fff) << 16) |
> +			lan8814_read_page_reg(phydev, 5, PTP_TX_EGRESS_NS_LO);
> +	}
> +
> +	if (seq_id)
> +		(*seq_id) = lan8814_read_page_reg(phydev, 5, PTP_TX_MSG_HEADER2);
> +}
> +
> +static int lan8814_ts_info(struct mii_timestamper *mii_ts, struct ethtool_ts_info *info)
> +{
> +	struct kszphy_ptp_priv *lan8814 = container_of(mii_ts, struct kszphy_ptp_priv, mii_ts);
> +
> +	info->so_timestamping =
> +		SOF_TIMESTAMPING_TX_HARDWARE |
> +		SOF_TIMESTAMPING_RX_HARDWARE |
> +		SOF_TIMESTAMPING_RAW_HARDWARE;
> +
> +	info->phc_index = ptp_clock_index(lan8814->ptp.ptp_clock);
> +
> +	info->tx_types =
> +		(1 << HWTSTAMP_TX_OFF) |
> +		(1 << HWTSTAMP_TX_ON) |
> +		(1 << HWTSTAMP_TX_ONESTEP_SYNC);
> +
> +	info->rx_filters =
> +		(1 << HWTSTAMP_FILTER_NONE) |
> +		(1 << HWTSTAMP_FILTER_PTP_V1_L4_EVENT) |
> +		(1 << HWTSTAMP_FILTER_PTP_V2_L4_EVENT) |
> +		(1 << HWTSTAMP_FILTER_PTP_V2_L2_EVENT) |
> +		(1 << HWTSTAMP_FILTER_PTP_V2_EVENT);
> +
> +	return 0;
> +}
> +
> +static int lan8814_hwtstamp(struct mii_timestamper *mii_ts, struct ifreq *ifr)
> +{
> +	struct kszphy_ptp_priv *lan8814 = container_of(mii_ts, struct kszphy_ptp_priv, mii_ts);
> +	struct hwtstamp_config config;
> +	int txcfg, rxcfg;
> +
> +	if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
> +		return -EFAULT;
> +
> +	lan8814->hwts_tx_en = config.tx_type;
> +
> +	lan8814->ptp.rx_filter = config.rx_filter;
> +	lan8814->ptp.tx_type = config.tx_type;
> +
> +	switch (config.rx_filter) {
> +	case HWTSTAMP_FILTER_NONE:
> +		lan8814->hwts_rx_en = 0;
> +		lan8814->layer = 0;
> +		lan8814->version = 0;
> +		break;
> +	case HWTSTAMP_FILTER_PTP_V2_L4_EVENT:
> +	case HWTSTAMP_FILTER_PTP_V2_L4_SYNC:
> +	case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ:
> +		lan8814->hwts_rx_en = 1;
> +		lan8814->layer = PTP_CLASS_L4;
> +		lan8814->version = PTP_CLASS_V2;
> +		break;
> +	case HWTSTAMP_FILTER_PTP_V2_L2_EVENT:
> +	case HWTSTAMP_FILTER_PTP_V2_L2_SYNC:
> +	case HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ:
> +		lan8814->hwts_rx_en = 1;
> +		lan8814->layer = PTP_CLASS_L2;
> +		lan8814->version = PTP_CLASS_V2;
> +		break;
> +	case HWTSTAMP_FILTER_PTP_V2_EVENT:
> +	case HWTSTAMP_FILTER_PTP_V2_SYNC:
> +	case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ:
> +		lan8814->hwts_rx_en = 1;
> +		lan8814->layer = PTP_CLASS_L4 | PTP_CLASS_L2;
> +		lan8814->version = PTP_CLASS_V2;
> +		break;
> +	default:
> +		return -ERANGE;
> +	}
> +
> +	lan8814_write_page_reg(lan8814->phydev, 5, PTP_RX_PARSE_CONFIG, 0);
> +	lan8814_write_page_reg(lan8814->phydev, 5, PTP_TX_PARSE_CONFIG, 0);
> +
> +	if (lan8814->hwts_rx_en && (lan8814->layer & PTP_CLASS_L2))
> +		rxcfg = PTP_RX_PARSE_CONFIG_LAYER2_EN_;
> +
> +	if (lan8814->hwts_tx_en && (lan8814->layer & PTP_CLASS_L2))
> +		txcfg = PTP_TX_PARSE_CONFIG_LAYER2_EN_;
> +
> +	if (lan8814->hwts_rx_en && (lan8814->layer & PTP_CLASS_L4))
> +		rxcfg |= PTP_RX_PARSE_CONFIG_IPV4_EN_;
> +
> +	if (lan8814->hwts_tx_en && (lan8814->layer & PTP_CLASS_L4))
> +		txcfg |= PTP_TX_PARSE_CONFIG_IPV4_EN_;
> +
> +	if (lan8814_ptp_is_enable(lan8814->phydev))
> +		lan8814_write_page_reg(lan8814->phydev, 4, PTP_CMD_CTL,
> +				       PTP_CMD_CTL_PTP_DISABLE_);

Huh?  If the PTP mode is enabled, you now disable it?

> +	lan8814_write_page_reg(lan8814->phydev, 5, PTP_RX_PARSE_CONFIG, rxcfg);
> +	lan8814_write_page_reg(lan8814->phydev, 5, PTP_TX_PARSE_CONFIG, txcfg);
> +	lan8814_write_page_reg(lan8814->phydev, 5, PTP_RX_TIMESTAMP_EN, 0x3);
> +	lan8814_write_page_reg(lan8814->phydev, 5, PTP_TX_TIMESTAMP_EN, 0x3);
> +	lan8814_write_page_reg(lan8814->phydev, 5, PTP_TX_PARSE_L2_ADDR_EN, 0);
> +	lan8814_write_page_reg(lan8814->phydev, 5, PTP_RX_PARSE_L2_ADDR_EN, 0);
> +
> +	lan8814_write_page_reg(lan8814->phydev, 4, PTP_CMD_CTL, PTP_CMD_CTL_PTP_ENABLE_);
> +	if (lan8814->hwts_tx_en == HWTSTAMP_TX_ONESTEP_SYNC) {
> +		lan8814_write_page_reg(lan8814->phydev, 5, PTP_TX_MOD,
> +				       PTP_TX_MOD_TX_PTP_SYNC_TS_INSERT_);
> +	} else if (lan8814->hwts_tx_en == HWTSTAMP_TX_ON) {
> +		/* Enabling 2 step offloading and all option for TS insertion/correction fields */
> +		lan8814_write_page_reg(lan8814->phydev, 5, PTP_TX_MOD, 0x800);
> +		lan8814_config_ts_intr(lan8814->phydev);
> +	}

Again, there is no protection here against concurrent changes from
user space.

> +
> +	return copy_to_user(ifr->ifr_data, &config, sizeof(config)) ? -EFAULT : 0;
> +}
> +
> +static bool lan8814_rxtstamp(struct mii_timestamper *mii_ts, struct sk_buff *skb, int type)
> +{
> +	struct kszphy_ptp_priv *lan8814 = container_of(mii_ts, struct kszphy_ptp_priv, mii_ts);
> +	struct skb_shared_hwtstamps *shhwtstamps = NULL;
> +	u32 seconds, nsec, seq_id;
> +
> +	if (!lan8814->hwts_rx_en)
> +		return false;
> +
> +	if (!skb)

Remove the useless check.  skb is guaranteed to be non-null.

> +		return false;
> +
> +	if ((type & lan8814->version) == 0 || (type & lan8814->layer) == 0)
> +		return false;
> +
> +	if (lan8814->ptp.rx_filter == HWTSTAMP_FILTER_NONE ||
> +	    type == PTP_CLASS_NONE)

Isn't this test redundant?  You just checked (type & lan8814->version),
and lan8814->version depends on lan8814->ptp.rx_filter.

> +		return false;
> +
> +	lan8814_ptp_rx_ts_get(lan8814->phydev, &seconds, &nsec, &seq_id);

This read the time stamp unconditionally.  Can't that fail?
What happens when the time stamp doesn't match the skb?

> +	/* Saving timestamp and sending through skbuff */
> +	shhwtstamps = skb_hwtstamps(skb);
> +	memset(shhwtstamps, 0, sizeof(*shhwtstamps));
> +	shhwtstamps->hwtstamp = ktime_set(seconds, nsec);
> +
> +	netif_rx_ni(skb);
> +
> +	return true;
> +}
> +
> +static int is_sync(struct sk_buff *skb, int type)
> +{
> +	u8 *data = skb->data, *msgtype;
> +	unsigned int offset = 0;

Please use ptp_parse_header() and ptp_get_msgtype();

> +	if (type & PTP_CLASS_VLAN)
> +		offset += VLAN_HLEN;
> +
> +	switch (type & PTP_CLASS_PMASK) {
> +	case PTP_CLASS_IPV4:
> +		offset += ETH_HLEN + IPV4_HLEN(data + offset) + UDP_HLEN;
> +		break;
> +	case PTP_CLASS_IPV6:
> +		offset += ETH_HLEN + IP6_HLEN + UDP_HLEN;
> +		break;
> +	case PTP_CLASS_L2:
> +		offset += ETH_HLEN;
> +		break;
> +	default:
> +		return 0;
> +	}
> +
> +	if (type & PTP_CLASS_V1)
> +		offset += OFF_PTP_CONTROL;
> +
> +	if (skb->len < offset + 1)
> +		return 0;
> +
> +	msgtype = data + offset;
> +
> +	return (*msgtype & 0xf) == 0;
> +}
> +
> +static void lan8814_txtstamp(struct mii_timestamper *mii_ts,
> +			     struct sk_buff *skb, int type)
> +{
> +	struct kszphy_ptp_priv *lan8814 = container_of(mii_ts, struct kszphy_ptp_priv, mii_ts);
> +
> +	switch (lan8814->hwts_tx_en) {
> +	case HWTSTAMP_TX_ONESTEP_SYNC:
> +		if (is_sync(skb, type)) {
> +			kfree_skb(skb);
> +			return;
> +		}
> +		break;
> +
> +	case HWTSTAMP_TX_ON:
> +		skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
> +		skb_queue_tail(&lan8814->ptp.tx_queue, skb);
> +		break;
> +
> +	case HWTSTAMP_TX_OFF:
> +	default:
> +		kfree_skb(skb);
> +		break;
> +	}
> +}
> +
> +static void lan8814_ptp_clock_set(struct phy_device *phydev,
> +				  u32 seconds, u32 nano_seconds,
> +				 u32 sub_nano_seconds)
> +{
> +	u32 sec_low, sec_high, nsec_low, nsec_high, snsec_low, snsec_high;
> +
> +	sec_low = seconds & 0xffff;
> +	sec_high = ((seconds >> 16) & 0xffff);
> +	nsec_low = nano_seconds & 0xffff;
> +	nsec_high = ((nano_seconds >> 16) & 0x3fff);
> +	snsec_low = sub_nano_seconds & 0xffff;
> +	snsec_high = ((sub_nano_seconds >> 16) & 0xffff);
> +
> +	lan8814_write_page_reg(phydev, 4, PTP_CLOCK_SET_SEC_LO, sec_low);
> +	lan8814_write_page_reg(phydev, 4, PTP_CLOCK_SET_SEC_MID, sec_high);
> +	lan8814_write_page_reg(phydev, 4, PTP_CLOCK_SET_NS_LO, nsec_low);
> +	lan8814_write_page_reg(phydev, 4, PTP_CLOCK_SET_NS_HI, nsec_high);
> +	lan8814_write_page_reg(phydev, 4, PTP_CLOCK_SET_SUBNS_LO, snsec_low);
> +	lan8814_write_page_reg(phydev, 4, PTP_CLOCK_SET_SUBNS_HI, snsec_high);
> +
> +	lan8814_write_page_reg(phydev, 4, PTP_CMD_CTL, PTP_CMD_CTL_PTP_CLOCK_LOAD_);

Needs protection against concurrent calls to clock_settime();

> +}
> +
> +static void lan8814_ptp_clock_get(struct phy_device *phydev,
> +				  u32 *seconds, u32 *nano_seconds,
> +				  u32 *sub_nano_seconds)
> +{
> +	lan8814_write_page_reg(phydev, 4, PTP_CMD_CTL, PTP_CMD_CTL_PTP_CLOCK_READ_);
> +
> +	if (seconds) {
> +		(*seconds) = lan8814_read_page_reg(phydev, 4, PTP_CLOCK_READ_SEC_MID);
> +		(*seconds) = ((*seconds) << 16) |
> +			     lan8814_read_page_reg(phydev, 4, PTP_CLOCK_READ_SEC_LO);
> +	}
> +
> +	if (nano_seconds) {
> +		(*nano_seconds) = lan8814_read_page_reg(phydev, 4, PTP_CLOCK_READ_NS_HI);
> +		(*nano_seconds) = (((*nano_seconds) & 0x3fff) << 16) |
> +				  lan8814_read_page_reg(phydev, 4, PTP_CLOCK_READ_NS_LO);
> +	}
> +
> +	if (sub_nano_seconds) {
> +		(*sub_nano_seconds) = lan8814_read_page_reg(phydev, 4,
> +								PTP_CLOCK_READ_SUBNS_HI);
> +		(*sub_nano_seconds) = ((*sub_nano_seconds) << 16) |
> +				       lan8814_read_page_reg(phydev, 4,
> +							     PTP_CLOCK_READ_SUBNS_LO);
> +	}
> +}
> +
> +static int lan8814_ptpci_gettime64(struct ptp_clock_info *ptpci,
> +				   struct timespec64 *ts)
> +{
> +	struct lan8814_ptp *ptp =
> +		container_of(ptpci, struct lan8814_ptp, ptp_clock_info);
> +	struct kszphy_ptp_priv *priv =
> +		container_of(ptp, struct kszphy_ptp_priv, ptp);
> +	struct phy_device *phydev = priv->phydev;
> +	u32 nano_seconds = 0;
> +	u32 seconds = 0;
> +
> +	lan8814_ptp_clock_get(phydev, &seconds, &nano_seconds, NULL);
> +	ts->tv_sec = seconds;
> +	ts->tv_nsec = nano_seconds;
> +
> +	return 0;
> +}
> +
> +static void lan8814_ptp_clock_step(struct phy_device *phydev,
> +				   s64 time_step_ns)
> +{
> +	u32 nano_seconds_step	= 0;
> +	u64 abs_time_step_ns	= 0;
> +	u32 unsigned_seconds	= 0;
> +	u32 nano_seconds	= 0;
> +	u32 remainder		= 0;
> +	s32 seconds		= 0;
> +
> +	if (time_step_ns >  15000000000LL) {
> +		/* convert to clock set */
> +		lan8814_ptp_clock_get(phydev, &unsigned_seconds,	&nano_seconds, NULL);
> +		unsigned_seconds += div_u64_rem(time_step_ns, 1000000000LL,
> +				&remainder);
> +		nano_seconds += remainder;
> +		if (nano_seconds >= 1000000000) {
> +			unsigned_seconds++;
> +			nano_seconds -= 1000000000;
> +		}
> +		lan8814_ptp_clock_set(phydev, unsigned_seconds,
> +				      nano_seconds, 0);
> +		return;
> +	} else if (time_step_ns < -15000000000LL) {
> +		/* convert to clock set */
> +		time_step_ns = -time_step_ns;
> +
> +		lan8814_ptp_clock_get(phydev, &unsigned_seconds,
> +				      &nano_seconds, NULL);
> +		unsigned_seconds -= div_u64_rem(time_step_ns, 1000000000LL,
> +				&remainder);
> +		nano_seconds_step = remainder;
> +		if (nano_seconds < nano_seconds_step) {
> +			unsigned_seconds--;
> +			nano_seconds += 1000000000;
> +		}
> +		nano_seconds -= nano_seconds_step;
> +		lan8814_ptp_clock_set(phydev, unsigned_seconds,
> +				      nano_seconds, 0);
> +		return;
> +	}
> +
> +	/* do clock step */
> +	if (time_step_ns >= 0) {
> +		abs_time_step_ns = (u64)(time_step_ns);
> +		seconds = (s32)div_u64_rem(abs_time_step_ns, 1000000000,
> +				&remainder);
> +		nano_seconds = (u32)remainder;
> +	} else {
> +		abs_time_step_ns = (u64)(-time_step_ns);
> +		seconds = -((s32)div_u64_rem(abs_time_step_ns, 1000000000,
> +					&remainder));
> +		nano_seconds = (u32)remainder;
> +		if (nano_seconds > 0) {
> +			/* subtracting nano seconds is not allowed
> +			 * convert to subtracting from seconds,
> +			 * and adding to nanoseconds
> +			 */
> +			seconds--;
> +			nano_seconds = (1000000000 - nano_seconds);
> +		}
> +	}
> +
> +	if (nano_seconds > 0) {
> +		/* add 8 ns to cover the likely normal increment */
> +		nano_seconds += 8;
> +	}
> +
> +	if (nano_seconds >= 1000000000) {
> +		/* carry into seconds */
> +		seconds++;
> +		nano_seconds -= 1000000000;
> +	}
> +
> +	while (seconds) {
> +		if (seconds > 0) {
> +			u32 adjustment_value = (u32)seconds;
> +			u16 adjustment_value_lo, adjustment_value_hi;
> +
> +			adjustment_value_lo = adjustment_value & 0xffff;
> +			adjustment_value_hi = (adjustment_value >> 16) & 0x3fff;
> +
> +			if (adjustment_value > 0xF)
> +				adjustment_value = 0xF;
> +			lan8814_write_page_reg(phydev, 4, PTP_LTC_STEP_ADJ_LO,
> +					       adjustment_value_lo);
> +			lan8814_write_page_reg(phydev, 4, PTP_LTC_STEP_ADJ_HI,
> +					       PTP_LTC_STEP_ADJ_DIR_ |
> +					       adjustment_value_hi);
> +			seconds -= ((s32)adjustment_value);
> +		} else {
> +			u32 adjustment_value = (u32)(-seconds);
> +			u16 adjustment_value_lo, adjustment_value_hi;
> +
> +			adjustment_value_lo = adjustment_value & 0xffff;
> +			adjustment_value_hi = (adjustment_value >> 16) & 0x3fff;
> +			if (adjustment_value > 0xF)
> +				adjustment_value = 0xF;
> +			lan8814_write_page_reg(phydev, 4, PTP_LTC_STEP_ADJ_LO,
> +					       adjustment_value_lo);
> +			lan8814_write_page_reg(phydev, 4, PTP_LTC_STEP_ADJ_HI,
> +					       PTP_LTC_STEP_ADJ_DIR_ |
> +					       adjustment_value_hi);
> +			seconds += ((s32)adjustment_value);
> +		}
> +		lan8814_write_page_reg(phydev, 4, PTP_CMD_CTL,
> +				       PTP_CMD_CTL_PTP_LTC_STEP_SEC_);
> +	}
> +	if (nano_seconds) {
> +		u16 nano_seconds_lo;
> +		u16 nano_seconds_hi;
> +
> +		nano_seconds_lo = nano_seconds & 0xffff;
> +		nano_seconds_hi = (nano_seconds >> 16) & 0x3fff;
> +
> +		lan8814_write_page_reg(phydev, 4, PTP_LTC_STEP_ADJ_LO,
> +				       nano_seconds_lo);
> +		lan8814_write_page_reg(phydev, 4, PTP_LTC_STEP_ADJ_HI,
> +				       PTP_LTC_STEP_ADJ_DIR_ |
> +				       nano_seconds_hi);
> +		lan8814_write_page_reg(phydev, 4, PTP_CMD_CTL,
> +				       PTP_CMD_CTL_PTP_LTC_STEP_NSEC_);
> +	}
> +}
> +
> +static int lan8814_ptpci_adjtime(struct ptp_clock_info *ptpci, s64 delta)
> +{
> +	struct lan8814_ptp *ptp =
> +		container_of(ptpci, struct lan8814_ptp, ptp_clock_info);
> +	struct kszphy_ptp_priv *priv =
> +		container_of(ptp, struct kszphy_ptp_priv, ptp);
> +	struct phy_device *phydev = priv->phydev;
> +
> +	lan8814_ptp_clock_step(phydev, delta);
> +
> +	return 0;
> +}
> +
> +static int lan8814_ptpci_adjfine(struct ptp_clock_info *ptpci, long scaled_ppm)
> +{
> +	struct lan8814_ptp *ptp =
> +		container_of(ptpci, struct lan8814_ptp, ptp_clock_info);
> +	struct kszphy_ptp_priv *priv = container_of(ptp, struct kszphy_ptp_priv, ptp);
> +	struct phy_device *phydev = priv->phydev;
> +	u32 kszphy_rate_adj = 0;
> +	u16 kszphy_rate_adj_lo = 0, kszphy_rate_adj_hi = 0;
> +	bool positive = true;
> +	u64 u64_delta = 0;
> +
> +	if ((scaled_ppm < (-LAN8814_PTP_MAX_FINE_ADJ_IN_SCALED_PPM)) ||
> +	    scaled_ppm > LAN8814_PTP_MAX_FINE_ADJ_IN_SCALED_PPM) {
> +		return -EINVAL;
> +	}
> +	if (scaled_ppm > 0) {
> +		u64_delta = (u64)scaled_ppm;
> +		positive = true;
> +	} else {
> +		u64_delta = (u64)(-scaled_ppm);
> +		positive = false;
> +	}
> +	u64_delta = (u64_delta << 13);
> +	kszphy_rate_adj = div_u64(u64_delta, 15625);
> +
> +	kszphy_rate_adj_lo = kszphy_rate_adj & 0xffff;
> +	kszphy_rate_adj_hi = (kszphy_rate_adj >> 16) & 0x3fff;
> +
> +	if (positive)
> +		kszphy_rate_adj_hi |= PTP_CLOCK_RATE_ADJ_DIR_;
> +
> +	lan8814_write_page_reg(phydev, 4, PTP_CLOCK_RATE_ADJ_HI_, kszphy_rate_adj_hi);
> +	lan8814_write_page_reg(phydev, 4, PTP_CLOCK_RATE_ADJ_LO_, kszphy_rate_adj_lo);
> +
> +	return 0;
> +}
> +
> +static void lan8814_ptp_reset(struct phy_device *phydev)
> +{
> +	if (lan8814_ptp_is_enable(phydev))
> +		lan8814_write_page_reg(phydev, 4, PTP_CMD_CTL, PTP_CMD_CTL_PTP_DISABLE_);
> +
> +	lan8814_write_page_reg(phydev, 4, LTC_HARD_RESET, LTC_HARD_RESET_);
> +	lan8814_write_page_reg(phydev, 5, TSU_HARD_RESET, TSU_HARD_RESET_);
> +}
> +
> +static void lan8814_ptp_enable(struct phy_device *phydev)
> +{
> +	if (lan8814_ptp_is_enable(phydev))
> +		return;
> +
> +	lan8814_write_page_reg(phydev, 4, PTP_CMD_CTL, PTP_CMD_CTL_PTP_ENABLE_);
> +}
> +
> +static void lan8814_ptp_sync_to_system_clock(struct phy_device *phydev)
> +{
> +	struct timespec64 ts;
> +
> +	ktime_get_clocktai_ts64(&ts);
> +
> +	lan8814_ptp_clock_set(phydev, ts.tv_sec, ts.tv_nsec, 0);
> +}
> +
> +static void lan8814_ptp_init(struct phy_device *phydev)
> +{
> +	u32 temp;
> +
> +	lan8814_ptp_reset(phydev);
> +	lan8814_ptp_sync_to_system_clock(phydev);
> +	temp = lan8814_read_page_reg(phydev, 5, PTP_TX_MOD);
> +	temp |= PTP_TX_MOD_BAD_UDPV4_CHKSUM_FORCE_FCS_DIS_;
> +	lan8814_write_page_reg(phydev, 5, PTP_TX_MOD, temp);
> +	lan8814_write_page_reg(phydev, 4, PTP_OPERATING_MODE,
> +			       PTP_OPERATING_MODE_STANDALONE);
> +	lan8814_ptp_enable(phydev);
> +}
> +
>  static int kszphy_probe(struct phy_device *phydev)
>  {
>  	const struct kszphy_type *type = phydev->drv->driver_data;
> @@ -1248,6 +2106,88 @@ static int kszphy_probe(struct phy_device *phydev)
>  	return 0;
>  }
>  
> +static int lan8814_probe(struct phy_device *phydev)
> +{
> +	struct kszphy_priv *priv;
> +	struct clk *clk;
> +	const struct kszphy_type *type = phydev->drv->driver_data;
> +	const struct device_node *np = phydev->mdio.dev.of_node;
> +
> +	priv = devm_kzalloc(&phydev->mdio.dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	skb_queue_head_init(&priv->ptp_priv.ptp.tx_queue);
> +
> +	priv->ptp_priv.phydev = phydev;
> +
> +	priv->type = type;
> +
> +	priv->ptp_priv.mii_ts.rxtstamp = lan8814_rxtstamp;
> +	priv->ptp_priv.mii_ts.txtstamp = lan8814_txtstamp;
> +	priv->ptp_priv.mii_ts.hwtstamp = lan8814_hwtstamp;
> +	priv->ptp_priv.mii_ts.ts_info  = lan8814_ts_info;
> +
> +	phydev->mii_ts = &priv->ptp_priv.mii_ts;
> +
> +	phydev->priv = priv;
> +
> +	lan8814_ptp_init(phydev);
> +
> +	priv->ptp_priv.ptp.flags |= PTP_FLAG_ISR_ENABLED;
> +
> +	if (!IS_ENABLED(CONFIG_PTP_1588_CLOCK))
> +		return 0;
> +
> +	clk = devm_clk_get(&phydev->mdio.dev, "rmii-ref");
> +	/* NOTE: clk may be NULL if building without CONFIG_HAVE_CLK */
> +	if (!IS_ERR_OR_NULL(clk)) {
> +		unsigned long rate = clk_get_rate(clk);
> +		bool rmii_ref_clk_sel_25_mhz;
> +
> +		priv->rmii_ref_clk_sel = type->has_rmii_ref_clk_sel;
> +		rmii_ref_clk_sel_25_mhz = of_property_read_bool(np,
> +				"micrel,rmii-reference-clock-select-25-mhz");
> +
> +		if (rate > 24500000 && rate < 25500000) {
> +			priv->rmii_ref_clk_sel_val = rmii_ref_clk_sel_25_mhz;
> +		} else if (rate > 49500000 && rate < 50500000) {
> +			priv->rmii_ref_clk_sel_val = !rmii_ref_clk_sel_25_mhz;
> +		} else {
> +			phydev_err(phydev, "Clock rate out of range: %ld\n",
> +				   rate);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	priv->ptp_priv.ptp.ptp_clock_info.owner = THIS_MODULE;
> +	snprintf(priv->ptp_priv.ptp.ptp_clock_info.name, 30, "%s",
> +		 phydev->drv->name);
> +	priv->ptp_priv.ptp.ptp_clock_info.max_adj = LAN8814_PTP_MAX_FREQ_ADJ_IN_PPB;
> +	priv->ptp_priv.ptp.ptp_clock_info.n_alarm = 0;
> +	priv->ptp_priv.ptp.ptp_clock_info.n_ext_ts = 0;
> +	priv->ptp_priv.ptp.ptp_clock_info.n_per_out = 1;
> +	priv->ptp_priv.ptp.ptp_clock_info.n_pins = 0;
> +	priv->ptp_priv.ptp.ptp_clock_info.pps = 0;
> +	priv->ptp_priv.ptp.ptp_clock_info.pin_config = NULL;
> +	priv->ptp_priv.ptp.ptp_clock_info.adjfine = lan8814_ptpci_adjfine;
> +	priv->ptp_priv.ptp.ptp_clock_info.adjtime = lan8814_ptpci_adjtime;
> +	priv->ptp_priv.ptp.ptp_clock_info.gettime64 = lan8814_ptpci_gettime64;

Don't forget about settime64()!

Thanks,
Richard


> +	priv->ptp_priv.ptp.ptp_clock_info.getcrosststamp = NULL;
> +
> +	priv->ptp_priv.ptp.ptp_clock = ptp_clock_register(&priv->ptp_priv.ptp.ptp_clock_info,
> +							  &phydev->mdio.dev);
> +
> +	if (IS_ERR_OR_NULL(priv->ptp_priv.ptp.ptp_clock))
> +		phydev_err(phydev, "ptp_clock_register failed %lu\n",
> +			   PTR_ERR(priv->ptp_priv.ptp.ptp_clock));
> +
> +	priv->ptp_priv.ptp.flags |= PTP_FLAG_PTP_CLOCK_REGISTERED;
> +	phydev_info(phydev, "successfully registered ptp clock\n");
> +
> +	return 0;
> +}
> +
>  static struct phy_driver ksphy_driver[] = {
>  {
>  	.phy_id		= PHY_ID_KS8737,
> @@ -1415,7 +2355,7 @@ static struct phy_driver ksphy_driver[] = {
>  	.phy_id_mask	= MICREL_PHY_ID_MASK,
>  	.name		= "Microchip INDY Gigabit Quad PHY",
>  	.driver_data	= &ksz9021_type,
> -	.probe		= kszphy_probe,
> +	.probe		= lan8814_probe,
>  	.soft_reset	= genphy_soft_reset,
>  	.read_status	= ksz9031_read_status,
>  	.get_sset_count	= kszphy_get_sset_count,
> -- 
> 2.17.1
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ