* Re: [PATCH net-next 00/18] net: mvpp2: add RSS support
From: David Miller @ 2018-07-13 0:31 UTC (permalink / raw)
To: maxime.chevallier
Cc: netdev, linux-kernel, antoine.tenart, thomas.petazzoni,
gregory.clement, miquel.raynal, nadavh, stefanc, ymarkman, mw
In-Reply-To: <20180712115427.27375-1-maxime.chevallier@bootlin.com>
From: Maxime Chevallier <maxime.chevallier@bootlin.com>
Date: Thu, 12 Jul 2018 13:54:09 +0200
> This series adds support for RSS on PPv2. There already was some code to
> handle the RSS tables, but the driver was missing all the classification
> steps required to actually use these tables.
>
> RSS is used through the classifier, using at least 2 lookups :
> - One using the C2 engine, a TCAM engine that match the packet based on
> some header extracted fields, assigns the default rx queue for that
> packet and tag it for RSS
> - One using the C3Hx engine, which computes the hash that's used to perform
> the lookup in the RSS table.
>
> Since RSS spreads the load across CPUs, we need to make sure that packets
> from the same flow are always assigned the same rx queue, to prevent
> re-ordering.
>
> This series therefore adds a classification step based on the Header Parser,
> that separate ingress traffic into 52 flows, based on some L2, L3 and L4
> parameters.
>
> Patches 1 and 2 fix some header issues, from the driver splitting
>
> Patches 3 to 7 make sure the correct receive queue setup is used for RSS
>
> Patches 8 to 14 deal with the way we handle the RSS tables
>
> Patch 15 implement basic classifier configuration, by using it to assign the
> default receive queue
>
> Patch 16 implement the ingress traffic splitting into multiple flows
>
> Patch 17 adds RSS support, by using the needed classification steps
>
> Patch 18 adds the required ethtool ops to configure the flow hash parameters
>
> This was tested on MacchiatoBin, giving some nice performance improvements
> using ip forwarding (going from 5Gbps to 9.6Gbps total throughput).
>
> RSS is disabled by default.
Series applied, thank you.
^ permalink raw reply
* linux-next: manual merge of the net-next tree with the net tree
From: Stephen Rothwell @ 2018-07-13 0:47 UTC (permalink / raw)
To: David Miller, Networking
Cc: Linux-Next Mailing List, Linux Kernel Mailing List,
Stefan Baranoff, Arnd Bergmann, Eric Dumazet
[-- Attachment #1: Type: text/plain, Size: 2299 bytes --]
Hi all,
Today's linux-next merge of the net-next tree got a conflict in:
net/ipv4/tcp_ipv4.c
between commit:
21684dc46c59 ("tcp: fix sequence numbers for repaired sockets re-using TIME-WAIT sockets")
from the net tree and commit:
cca9bab1b72c ("tcp: use monotonic timestamps for PAWS")
from the net-next tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc net/ipv4/tcp_ipv4.c
index 3b2711e33e4c,dc415c66a33a..000000000000
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@@ -155,25 -155,13 +155,26 @@@ int tcp_twsk_unique(struct sock *sk, st
and use initial timestamp retrieved from peer table.
*/
if (tcptw->tw_ts_recent_stamp &&
- (!twp || (reuse && get_seconds() - tcptw->tw_ts_recent_stamp > 1))) {
+ (!twp || (reuse && time_after32(ktime_get_seconds(),
+ tcptw->tw_ts_recent_stamp)))) {
- tp->write_seq = tcptw->tw_snd_nxt + 65535 + 2;
- if (tp->write_seq == 0)
- tp->write_seq = 1;
- tp->rx_opt.ts_recent = tcptw->tw_ts_recent;
- tp->rx_opt.ts_recent_stamp = tcptw->tw_ts_recent_stamp;
+ /* In case of repair and re-using TIME-WAIT sockets we still
+ * want to be sure that it is safe as above but honor the
+ * sequence numbers and time stamps set as part of the repair
+ * process.
+ *
+ * Without this check re-using a TIME-WAIT socket with TCP
+ * repair would accumulate a -1 on the repair assigned
+ * sequence number. The first time it is reused the sequence
+ * is -1, the second time -2, etc. This fixes that issue
+ * without appearing to create any others.
+ */
+ if (likely(!tp->repair)) {
+ tp->write_seq = tcptw->tw_snd_nxt + 65535 + 2;
+ if (tp->write_seq == 0)
+ tp->write_seq = 1;
+ tp->rx_opt.ts_recent = tcptw->tw_ts_recent;
+ tp->rx_opt.ts_recent_stamp = tcptw->tw_ts_recent_stamp;
+ }
sock_hold(sktw);
return 1;
}
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [net-next, 3/3] tcp: convert icsk_user_timeout from jiffies to msecs
From: Jon Maxwell @ 2018-07-13 0:49 UTC (permalink / raw)
To: davem
Cc: edumazet, ncardwell, David.Laight, kuznet, yoshfuji, netdev,
linux-kernel, jmaxwell
The final patch in this series. Create the tcp_clamp_rto_to_user_timeout()
helper routine. To calculate the correct rto, so that the TCP_USER_TIMEOUT
socket option is more accurate. Taking suggestions and feedback into account
from Eric Dumazet, Neal Cardwell and David Laight. Due to the 1st commit we
can avoid the msecs_to_jiffies() and jiffies_to_msecs() dance.
Signed-off-by: Jon Maxwell <jmaxwell37@gmail.com>
---
net/ipv4/tcp_timer.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 8ab8c9645294..d4d4b30a6bec 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -36,6 +36,22 @@ u32 tcp_retransmit_stamp(const struct sock *sk)
return start_ts;
}
+static u32 tcp_clamp_rto_to_user_timeout(const struct sock *sk)
+{
+ struct inet_connection_sock *icsk = inet_csk(sk);
+ u32 elapsed, start_ts;
+
+ start_ts = tcp_retransmit_stamp(sk);
+ if (!icsk->icsk_user_timeout || !start_ts)
+ return icsk->icsk_rto;
+ elapsed = tcp_time_stamp(tcp_sk(sk)) - start_ts;
+ if (elapsed >= icsk->icsk_user_timeout)
+ return 1; /* user timeout has passed; fire ASAP */
+ else
+ return min_t(u32, icsk->icsk_rto, msecs_to_jiffies(icsk->icsk_user_timeout -
+ elapsed));
+}
+
/**
* tcp_write_err() - close socket and save error info
* @sk: The socket the error has appeared on.
@@ -544,7 +560,8 @@ void tcp_retransmit_timer(struct sock *sk)
/* Use normal (exponential) backoff */
icsk->icsk_rto = min(icsk->icsk_rto << 1, TCP_RTO_MAX);
}
- inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS, icsk->icsk_rto, TCP_RTO_MAX);
+ inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS,
+ tcp_clamp_rto_to_user_timeout(sk), TCP_RTO_MAX);
if (retransmits_timed_out(sk, net->ipv4.sysctl_tcp_retries1 + 1, 0))
__sk_dst_reset(sk);
--
2.13.6
^ permalink raw reply related
* Re: [PATCH net-next v3 00/11] devlink: Add support for region access
From: David Miller @ 2018-07-13 0:37 UTC (permalink / raw)
To: valex; +Cc: netdev, jiri, dsahern, andrew, rahul.lakkireddy, jakub.kicinski
In-Reply-To: <1531397598-11207-1-git-send-email-valex@mellanox.com>
From: Alex Vesker <valex@mellanox.com>
Date: Thu, 12 Jul 2018 15:13:07 +0300
> This is a proposal which will allow access to driver defined address
> regions using devlink. Each device can create its supported address
> regions and register them. A device which exposes a region will allow
> access to it using devlink.
>
> The suggested implementation will allow exposing regions to the user,
> reading and dumping snapshots taken from different regions.
> A snapshot represents a memory image of a region taken by the driver.
>
> If a device collects a snapshot of an address region it can be later
> exposed using devlink region read or dump commands.
> This functionality allows for future analyses on the snapshots to be
> done.
>
> The major benefit of this support is not only to provide access to
> internal address regions which were inaccessible to the user but also
> to provide an additional way to debug complex error states using the
> region snapshots.
...
Series applied, thanks!
^ permalink raw reply
* Re: [PATCH net-next 1/5 v3] net: gemini: Look up L3 maxlen from table
From: David Miller @ 2018-07-13 0:39 UTC (permalink / raw)
To: linus.walleij; +Cc: netdev, ulli.kroll, f.fainelli, mirq-linux, andrew
In-Reply-To: <20180711193245.21980-1-linus.walleij@linaro.org>
From: Linus Walleij <linus.walleij@linaro.org>
Date: Wed, 11 Jul 2018 21:32:41 +0200
> The code to calculate the hardware register enumerator
> for the maximum L3 length isn't entirely simple to read.
> Use the existing defines and rewrite the function into a
> table look-up.
>
> Acked-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 2/5 v3] net: gemini: Improve connection prints
From: David Miller @ 2018-07-13 0:39 UTC (permalink / raw)
To: linus.walleij; +Cc: netdev, ulli.kroll, f.fainelli, mirq-linux, andrew
In-Reply-To: <20180711193245.21980-2-linus.walleij@linaro.org>
From: Linus Walleij <linus.walleij@linaro.org>
Date: Wed, 11 Jul 2018 21:32:42 +0200
> Switch over to using a module parameter and debug prints
> that can be controlled by this or ethtool like everyone
> else. Depromote all other prints to debug messages.
>
> The phy_print_status() was already in place, albeit never
> really used because the debuglevel hiding it had to be
> set up using ethtool.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 3/5 v3] net: gemini: Allow multiple ports to instantiate
From: David Miller @ 2018-07-13 0:39 UTC (permalink / raw)
To: linus.walleij; +Cc: netdev, ulli.kroll, f.fainelli, mirq-linux, andrew
In-Reply-To: <20180711193245.21980-3-linus.walleij@linaro.org>
From: Linus Walleij <linus.walleij@linaro.org>
Date: Wed, 11 Jul 2018 21:32:43 +0200
> The code was not tested with two ports actually in use at
> the same time. (I blame this on lack of actual hardware using
> that feature.) Now after locating a system using both ports,
> add necessary fix to make both ports come up.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 4/5 v3] net: gemini: Move main init to port
From: David Miller @ 2018-07-13 0:39 UTC (permalink / raw)
To: linus.walleij; +Cc: netdev, ulli.kroll, f.fainelli, mirq-linux, andrew
In-Reply-To: <20180711193245.21980-4-linus.walleij@linaro.org>
From: Linus Walleij <linus.walleij@linaro.org>
Date: Wed, 11 Jul 2018 21:32:44 +0200
> The initialization sequence for the ethernet, setting up
> interrupt routing and such things, need to be done after
> both the ports are clocked and reset. Before this the
> config will not "take". Move the initialization to the
> port probe function and keep track of init status in
> the state.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 5/5 v3] net: gemini: Indicate that we can handle jumboframes
From: David Miller @ 2018-07-13 0:39 UTC (permalink / raw)
To: linus.walleij; +Cc: netdev, ulli.kroll, f.fainelli, mirq-linux, andrew
In-Reply-To: <20180711193245.21980-5-linus.walleij@linaro.org>
From: Linus Walleij <linus.walleij@linaro.org>
Date: Wed, 11 Jul 2018 21:32:45 +0200
> The hardware supposedly handles frames up to 10236 bytes and
> implements .ndo_change_mtu() so accept 10236 minus the ethernet
> header for a VLAN tagged frame on the netdevices. Use
> ETH_MIN_MTU as minimum MTU.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Applied.
^ permalink raw reply
* Re: [PATCH net-next v2 0/5] virtio: support packed ring
From: Jason Wang @ 2018-07-13 0:52 UTC (permalink / raw)
To: David Miller, tiwei.bie
Cc: mst, virtualization, linux-kernel, netdev, virtio-dev, wexu,
jfreimann
In-Reply-To: <20180712.144458.2076041018423659380.davem@davemloft.net>
On 2018年07月13日 05:44, David Miller wrote:
> From: Tiwei Bie <tiwei.bie@intel.com>
> Date: Wed, 11 Jul 2018 10:27:06 +0800
>
>> Hello everyone,
>>
>> This patch set implements packed ring support in virtio driver.
>>
>> Some functional tests have been done with Jason's
>> packed ring implementation in vhost:
>>
>> https://lkml.org/lkml/2018/7/3/33
>>
>> Both of ping and netperf worked as expected.
> Michael and Jason, where are we with this series?
For the series:
Acked-by: Jason Wang <jasowang@redhat.com>
^ permalink raw reply
* Re: [PATCH v4 net-next 19/19] net/mlx5e: Kconfig, mutually exclude compilation of TLS and IPsec accel
From: David Miller @ 2018-07-13 0:44 UTC (permalink / raw)
To: borisp; +Cc: netdev, davejwatson, aviadye, saeedm
In-Reply-To: <1531423557-30926-20-git-send-email-borisp@mellanox.com>
From: Boris Pismenny <borisp@mellanox.com>
Date: Thu, 12 Jul 2018 22:25:57 +0300
> We currently have no devices that support both TLS and IPsec using the
> accel framework, and the current code does not support both IPsec and
> TLS. This patch prevents such combinations.
>
> Signed-off-by: Boris Pismenny <borisp@mellanox.com>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
> index 2545296..d3e8c70 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
> @@ -93,6 +93,7 @@ config MLX5_EN_TLS
> depends on TLS_DEVICE
> depends on TLS=y || MLX5_CORE=m
> depends on MLX5_ACCEL
> + depends on !MLX5_EN_IPSEC
> default n
You absolutely cannot do this.
You are forcing a distribution to pick one offload or the other at
build time, that's insane.
Please find a way to support both offloads in the driver. It is
absolutely valid for a distribution to ship the driver in a state that
supports both offloads and you must therefore support this properly.
Thank you.
^ permalink raw reply
* [PATCH net-next] TCP: make seq # error messages more readable
From: Randy Dunlap @ 2018-07-13 0:48 UTC (permalink / raw)
To: netdev@vger.kernel.org, David Miller, Eric Dumazet
Cc: 積丹尼 Dan Jacobson
From: Randy Dunlap <rdunlap@infradead.org>
Attempt to make cryptic TCP seq number error messages clearer by
(1) adding the function name, (2) identifying the errors as "seq # bug",
and (3) grouping the field identifiers and values by separating them
with commas.
E.g., the following message is changed from:
recvmsg bug 2: copied 73BCB6CD seq 70F17CBE rcvnxt 73BCB9AA fl 0
WARNING: CPU: 2 PID: 1501 at /linux/net/ipv4/tcp.c:1881 tcp_recvmsg+0x649/0xb90
to:
tcp_recvmsg: TCP recvmsg seq # bug 2: copied 73BCB6CD, seq 70F17CBE, rcvnxt 73BCB9AA, fl 0
WARNING: CPU: 2 PID: 1501 at /linux/net/ipv4/tcp.c:2011 tcp_recvmsg+0x694/0xba0
Suggested-by: 積丹尼 Dan Jacobson <jidanni@jidanni.org>
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
---
net/ipv4/tcp.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
--- linux-next-20180712.orig/net/ipv4/tcp.c
+++ linux-next-20180712/net/ipv4/tcp.c
@@ -1994,9 +1994,9 @@ int tcp_recvmsg(struct sock *sk, struct
* shouldn't happen.
*/
if (WARN(before(*seq, TCP_SKB_CB(skb)->seq),
- "recvmsg bug: copied %X seq %X rcvnxt %X fl %X\n",
- *seq, TCP_SKB_CB(skb)->seq, tp->rcv_nxt,
- flags))
+ "%s: TCP recvmsg seq # bug: copied %X, seq %X, rcvnxt %X, fl %X\n",
+ __func__, *seq,
+ TCP_SKB_CB(skb)->seq, tp->rcv_nxt, flags))
break;
offset = *seq - TCP_SKB_CB(skb)->seq;
@@ -2009,8 +2009,9 @@ int tcp_recvmsg(struct sock *sk, struct
if (TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN)
goto found_fin_ok;
WARN(!(flags & MSG_PEEK),
- "recvmsg bug 2: copied %X seq %X rcvnxt %X fl %X\n",
- *seq, TCP_SKB_CB(skb)->seq, tp->rcv_nxt, flags);
+ "%s: TCP recvmsg seq # bug 2: copied %X, seq %X, rcvnxt %X, fl %X\n",
+ __func__, *seq,
+ TCP_SKB_CB(skb)->seq, tp->rcv_nxt, flags);
}
/* Well, if we have backlog, try to process it now yet. */
^ permalink raw reply
* Re: [PATCH net-next v3 02/11] devlink: Add callback to query for snapshot id before snapshot create
From: Jakub Kicinski @ 2018-07-13 0:51 UTC (permalink / raw)
To: Alex Vesker; +Cc: netdev, jiri, dsahern, andrew, rahul.lakkireddy
In-Reply-To: <1531397598-11207-3-git-send-email-valex@mellanox.com>
On Thu, 12 Jul 2018 15:13:09 +0300, Alex Vesker wrote:
> To restrict the driver with the snapshot ID selection a new callback
> is introduced for the driver to get the snapshot ID before creating
> a new snapshot. This will also allow giving the same ID for multiple
> snapshots taken of different regions on the same time.
I'm not in position to criticize other people's commit messages :), but
I find this one hard to parse. I think what you meant to say is that
you add a helper for numbering the snapshot per-devlink instance.
There is no callback to be seen here. You *prevent* from giving the
same ID to multiple snapshot even if they are from different regions.
> diff --git a/net/core/devlink.c b/net/core/devlink.c
> index cac8561..6c92ddd 100644
> --- a/net/core/devlink.c
> +++ b/net/core/devlink.c
> @@ -4193,6 +4193,27 @@ void devlink_region_destroy(struct devlink_region *region)
> }
> EXPORT_SYMBOL_GPL(devlink_region_destroy);
>
> +/**
> + * devlink_region_shapshot_id_get - get snapshot ID
> + *
> + * This callback should be called when adding a new snapshot,
> + * Driver should use the same id for multiple snapshots taken
> + * on multiple regions at the same time/by the same trigger.
> + *
> + * @devlink: devlink
> + */
> +u32 devlink_region_shapshot_id_get(struct devlink *devlink)
> +{
> + u32 id;
> +
> + mutex_lock(&devlink->lock);
> + id = ++devlink->snapshot_id;
Any reason not to use an IDA? The reuse may seem unlikely, OTOH IDA
isn't going to cost much, so why risk it...
> + mutex_unlock(&devlink->lock);
> +
> + return id;
> +}
> +EXPORT_SYMBOL_GPL(devlink_region_shapshot_id_get);
Sorry for only spotting this now.
^ permalink raw reply
* Re: Re: [Qemu-devel] [PATCH v3 0/3] Use of unique identifier for pairing virtio and passthrough devices...
From: Samudrala, Sridhar @ 2018-07-13 1:20 UTC (permalink / raw)
To: Siwei Liu, Michael S. Tsirkin
Cc: Cornelia Huck, si-wei liu, Roman Kagan, Venu Busireddy,
Marcel Apfelbaum, virtio-dev, qemu-devel, Alexander Duyck, Netdev
In-Reply-To: <CADGSJ23c4hHAoAwG61xmrRd-OO6EpsHgH9Fd-9R69XQAVkduCA@mail.gmail.com>
On 7/12/2018 6:19 PM, Siwei Liu wrote:
> On Thu, Jul 12, 2018 at 2:00 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> On Thu, Jul 12, 2018 at 01:52:53PM -0700, Siwei Liu wrote:
>>> The definition is incomplete due to lack of spec. There's no "host"
>>> part defined yet in the host-guest interface. If match by MAC is an
>>> interface, the same must be done on the host(device) side as well,
>>> which has been agreed not the way to go. However, I don't think that's
>>> what the author intends to do by interpreting his QEMU patch - it
>>> missed the other parts as well, such as the feature negotiation and
>>> how it interacts with the paired device.
>>>
>>> What I said is that match by MAC is just a guest implementation that
>>> one can change at any time. We now have the group ID on QEMU, why
>>> still sticking to matching by MAC? It shoulnd't be a host-guest
>>> interface in the first place anyway.
>> I think that match by MAC is a simple portable way to match devices.
>> E.g. it will work seamlessly with niche things like zPCI. However
> That's a good point. I'm not sure if it's a valid assumption that zPCI
> should always use the same MAC address as that of virtio. Someone
> who's more familiar with the use case may decide and work on that. It
> means VFIO device has to take in the MAC address as an identifier to
> the "-device vfio-pci,.." QEMU option. I think there's no point to
> match device using group ID in QEMU while using MAC in the guest.
> Based on that assumption, I'd go with making VIRTIO_NET_F_STANDBY to
> match device based on group ID, while someone may come up with another
> feature bit later, say VIRTIO_NET_F_STANDBY_BY_MAC when its QEMU
> support is available. Would it make sense?
VIRTIO_NET_F_STANDBY as defined in the guest virtio_net driver supports match
by MAC address. I think we should add support for this feature bit in QEMU.
If submitting a patch to update the spec is a pre-requisite to add this
feature bit to QEMU, i can do that.
As far as i understand, group id patches to QEMU are still under review.
Matching by group ID can be another feature bit that could support matching
by group id as well as MAC.
> -Siwei
>
>> there are other niche use-cases that aren't addressed by match by MAC
>> such as PF pass-through as a primary, and the pci bridge trick addresses
>> that at cost of some portability.
>>
>> So I see no issues supporting both mechanisms, but others on the TC
>> might feel differently.
>>
>> --
>> MST
^ permalink raw reply
* [PATCH 07/18] hisilicon: change strncpy+truncation to strlcpy
From: Dominique Martinet @ 2018-07-13 1:25 UTC (permalink / raw)
Cc: Dominique Martinet, Yisen Zhuang, Salil Mehta, David S. Miller,
Matthias Brugger, Lin Yun Sheng, stephen hemminger, netdev,
linux-kernel
In-Reply-To: <1531444483-17338-1-git-send-email-asmadeus@codewreck.org>
Generated by scripts/coccinelle/misc/strncpy_truncation.cocci
Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
---
Please see https://marc.info/?l=linux-kernel&m=153144450722324&w=2 (the
first patch of the serie) for the motivation behind this patch
drivers/net/ethernet/hisilicon/hns/hns_ethtool.c | 7 ++-----
drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c | 7 ++-----
2 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c b/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
index 2e14a3ae1d8b..35b4d72d1997 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
@@ -638,12 +638,9 @@ static void hns_nic_get_drvinfo(struct net_device *net_dev,
{
struct hns_nic_priv *priv = netdev_priv(net_dev);
- strncpy(drvinfo->version, HNAE_DRIVER_VERSION,
+ strlcpy(drvinfo->version, HNAE_DRIVER_VERSION,
sizeof(drvinfo->version));
- drvinfo->version[sizeof(drvinfo->version) - 1] = '\0';
-
- strncpy(drvinfo->driver, HNAE_DRIVER_NAME, sizeof(drvinfo->driver));
- drvinfo->driver[sizeof(drvinfo->driver) - 1] = '\0';
+ strlcpy(drvinfo->driver, HNAE_DRIVER_NAME, sizeof(drvinfo->driver));
strncpy(drvinfo->bus_info, priv->dev->bus->name,
sizeof(drvinfo->bus_info));
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
index 40c0425b4023..630c8d186707 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
@@ -493,13 +493,10 @@ static void hns3_get_drvinfo(struct net_device *netdev,
struct hns3_nic_priv *priv = netdev_priv(netdev);
struct hnae3_handle *h = priv->ae_handle;
- strncpy(drvinfo->version, hns3_driver_version,
+ strlcpy(drvinfo->version, hns3_driver_version,
sizeof(drvinfo->version));
- drvinfo->version[sizeof(drvinfo->version) - 1] = '\0';
-
- strncpy(drvinfo->driver, h->pdev->driver->name,
+ strlcpy(drvinfo->driver, h->pdev->driver->name,
sizeof(drvinfo->driver));
- drvinfo->driver[sizeof(drvinfo->driver) - 1] = '\0';
strncpy(drvinfo->bus_info, pci_name(h->pdev),
sizeof(drvinfo->bus_info));
--
2.17.1
^ permalink raw reply related
* [PATCH 08/18] myricom: change strncpy+truncation to strlcpy
From: Dominique Martinet @ 2018-07-13 1:25 UTC (permalink / raw)
Cc: Dominique Martinet, Chris Lee, David S. Miller, netdev,
linux-kernel
In-Reply-To: <1531444483-17338-1-git-send-email-asmadeus@codewreck.org>
Generated by scripts/coccinelle/misc/strncpy_truncation.cocci
Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
---
Please see https://marc.info/?l=linux-kernel&m=153144450722324&w=2 (the
first patch of the serie) for the motivation behind this patch
drivers/net/ethernet/myricom/myri10ge/myri10ge.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
index b2d2ec8c11e2..f7178cdb6bd8 100644
--- a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
+++ b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
@@ -553,8 +553,7 @@ myri10ge_validate_firmware(struct myri10ge_priv *mgp,
}
/* save firmware version for ethtool */
- strncpy(mgp->fw_version, hdr->version, sizeof(mgp->fw_version));
- mgp->fw_version[sizeof(mgp->fw_version) - 1] = '\0';
+ strlcpy(mgp->fw_version, hdr->version, sizeof(mgp->fw_version));
sscanf(mgp->fw_version, "%d.%d.%d", &mgp->fw_ver_major,
&mgp->fw_ver_minor, &mgp->fw_ver_tiny);
--
2.17.1
^ permalink raw reply related
* [PATCH 09/18] qlogic/qed: change strncpy+truncation to strlcpy
From: Dominique Martinet @ 2018-07-13 1:25 UTC (permalink / raw)
Cc: Dominique Martinet, Ariel Elior, everest-linux-l2,
David S. Miller, netdev, linux-kernel
In-Reply-To: <1531444483-17338-1-git-send-email-asmadeus@codewreck.org>
Generated by scripts/coccinelle/misc/strncpy_truncation.cocci
Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
---
Please see https://marc.info/?l=linux-kernel&m=153144450722324&w=2 (the
first patch of the serie) for the motivation behind this patch
drivers/net/ethernet/qlogic/qed/qed_debug.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_debug.c b/drivers/net/ethernet/qlogic/qed/qed_debug.c
index a14e48489029..4fe0a72230e8 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_debug.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_debug.c
@@ -6212,8 +6212,7 @@ static void qed_read_str_from_buf(void *buf, u32 *offset, u32 size, char *dest)
{
const char *source_str = &((const char *)buf)[*offset];
- strncpy(dest, source_str, size);
- dest[size - 1] = '\0';
+ strlcpy(dest, source_str, size);
*offset += size;
}
--
2.17.1
^ permalink raw reply related
* [PATCH 10/18] brcmsmac: change strncpy+truncation to strlcpy
From: Dominique Martinet @ 2018-07-13 1:25 UTC (permalink / raw)
Cc: Dominique Martinet, Arend van Spriel, Franky Lin, Hante Meuleman,
Chi-Hsien Lin, Wright Feng, Kalle Valo, David S. Miller,
linux-wireless, brcm80211-dev-list.pdl, brcm80211-dev-list,
netdev, linux-kernel
In-Reply-To: <1531444483-17338-1-git-send-email-asmadeus@codewreck.org>
Generated by scripts/coccinelle/misc/strncpy_truncation.cocci
Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
---
Please see https://marc.info/?l=linux-kernel&m=153144450722324&w=2 (the
first patch of the serie) for the motivation behind this patch
drivers/net/wireless/broadcom/brcm80211/brcmsmac/dma.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/dma.c b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/dma.c
index b7df576bb84d..58ccd72d672c 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmsmac/dma.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmsmac/dma.c
@@ -584,8 +584,7 @@ struct dma_pub *dma_attach(char *name, struct brcms_c_info *wlc,
rxextheadroom, nrxpost, rxoffset, txregbase, rxregbase);
/* make a private copy of our callers name */
- strncpy(di->name, name, MAXNAMEL);
- di->name[MAXNAMEL - 1] = '\0';
+ strlcpy(di->name, name, MAXNAMEL);
di->dmadev = core->dma_dev;
--
2.17.1
^ permalink raw reply related
* [PATCH 11/18] wireless/ti: change strncpy+truncation to strlcpy
From: Dominique Martinet @ 2018-07-13 1:25 UTC (permalink / raw)
Cc: Dominique Martinet, Kalle Valo, David S. Miller, Thomas Gleixner,
Greg Kroah-Hartman, Kate Stewart, Philippe Ombredanne,
Joe Perches, linux-wireless, netdev, linux-kernel
In-Reply-To: <1531444483-17338-1-git-send-email-asmadeus@codewreck.org>
Generated by scripts/coccinelle/misc/strncpy_truncation.cocci
Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
---
Please see https://marc.info/?l=linux-kernel&m=153144450722324&w=2 (the
first patch of the serie) for the motivation behind this patch
drivers/net/wireless/ti/wl1251/acx.c | 9 +--------
drivers/net/wireless/ti/wl18xx/main.c | 5 +----
drivers/net/wireless/ti/wlcore/boot.c | 5 +----
3 files changed, 3 insertions(+), 16 deletions(-)
diff --git a/drivers/net/wireless/ti/wl1251/acx.c b/drivers/net/wireless/ti/wl1251/acx.c
index f78fc3880423..c4f1a63300bb 100644
--- a/drivers/net/wireless/ti/wl1251/acx.c
+++ b/drivers/net/wireless/ti/wl1251/acx.c
@@ -150,14 +150,7 @@ int wl1251_acx_fw_version(struct wl1251 *wl, char *buf, size_t len)
}
/* be careful with the buffer sizes */
- strncpy(buf, rev->fw_version, min(len, sizeof(rev->fw_version)));
-
- /*
- * if the firmware version string is exactly
- * sizeof(rev->fw_version) long or fw_len is less than
- * sizeof(rev->fw_version) it won't be null terminated
- */
- buf[min(len, sizeof(rev->fw_version)) - 1] = '\0';
+ strlcpy(buf, rev->fw_version, min(len, sizeof(rev->fw_version)));
out:
kfree(rev);
diff --git a/drivers/net/wireless/ti/wl18xx/main.c b/drivers/net/wireless/ti/wl18xx/main.c
index ca0f936fc119..8595e9bf1cfa 100644
--- a/drivers/net/wireless/ti/wl18xx/main.c
+++ b/drivers/net/wireless/ti/wl18xx/main.c
@@ -1529,12 +1529,9 @@ static int wl18xx_handle_static_data(struct wl1271 *wl,
struct wl18xx_static_data_priv *static_data_priv =
(struct wl18xx_static_data_priv *) static_data->priv;
- strncpy(wl->chip.phy_fw_ver_str, static_data_priv->phy_version,
+ strlcpy(wl->chip.phy_fw_ver_str, static_data_priv->phy_version,
sizeof(wl->chip.phy_fw_ver_str));
- /* make sure the string is NULL-terminated */
- wl->chip.phy_fw_ver_str[sizeof(wl->chip.phy_fw_ver_str) - 1] = '\0';
-
wl1271_info("PHY firmware version: %s", static_data_priv->phy_version);
return 0;
diff --git a/drivers/net/wireless/ti/wlcore/boot.c b/drivers/net/wireless/ti/wlcore/boot.c
index f00509ea8aca..6b33951d5b34 100644
--- a/drivers/net/wireless/ti/wlcore/boot.c
+++ b/drivers/net/wireless/ti/wlcore/boot.c
@@ -55,12 +55,9 @@ static int wlcore_boot_parse_fw_ver(struct wl1271 *wl,
{
int ret;
- strncpy(wl->chip.fw_ver_str, static_data->fw_version,
+ strlcpy(wl->chip.fw_ver_str, static_data->fw_version,
sizeof(wl->chip.fw_ver_str));
- /* make sure the string is NULL-terminated */
- wl->chip.fw_ver_str[sizeof(wl->chip.fw_ver_str) - 1] = '\0';
-
ret = sscanf(wl->chip.fw_ver_str + 4, "%u.%u.%u.%u.%u",
&wl->chip.fw_ver[0], &wl->chip.fw_ver[1],
&wl->chip.fw_ver[2], &wl->chip.fw_ver[3],
--
2.17.1
^ permalink raw reply related
* [PATCH net-next] bnxt_en: remove redundant debug register dma mem allocation
From: YueHaibing @ 2018-07-13 3:02 UTC (permalink / raw)
To: michael.chan; +Cc: linux-kernel, netdev, davem, YueHaibing
hwrm_dbg_resp_addr and hwrm_dbg_resp_dma_addr are never used
and can be removed.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 13 -------------
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 3 ---
2 files changed, 16 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 4394c11..4db3180 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -3012,13 +3012,6 @@ static void bnxt_free_hwrm_resources(struct bnxt *bp)
bp->hwrm_cmd_resp_dma_addr);
bp->hwrm_cmd_resp_addr = NULL;
- if (bp->hwrm_dbg_resp_addr) {
- dma_free_coherent(&pdev->dev, HWRM_DBG_REG_BUF_SIZE,
- bp->hwrm_dbg_resp_addr,
- bp->hwrm_dbg_resp_dma_addr);
-
- bp->hwrm_dbg_resp_addr = NULL;
- }
}
static int bnxt_alloc_hwrm_resources(struct bnxt *bp)
@@ -3030,12 +3023,6 @@ static int bnxt_alloc_hwrm_resources(struct bnxt *bp)
GFP_KERNEL);
if (!bp->hwrm_cmd_resp_addr)
return -ENOMEM;
- bp->hwrm_dbg_resp_addr = dma_alloc_coherent(&pdev->dev,
- HWRM_DBG_REG_BUF_SIZE,
- &bp->hwrm_dbg_resp_dma_addr,
- GFP_KERNEL);
- if (!bp->hwrm_dbg_resp_addr)
- netdev_warn(bp->dev, "fail to alloc debug register dma mem\n");
return 0;
}
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 91575ef..934aa11 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1287,9 +1287,6 @@ struct bnxt {
dma_addr_t hwrm_short_cmd_req_dma_addr;
void *hwrm_cmd_resp_addr;
dma_addr_t hwrm_cmd_resp_dma_addr;
- void *hwrm_dbg_resp_addr;
- dma_addr_t hwrm_dbg_resp_dma_addr;
-#define HWRM_DBG_REG_BUF_SIZE 128
struct rx_port_stats *hw_rx_port_stats;
struct tx_port_stats *hw_tx_port_stats;
--
2.7.0
^ permalink raw reply related
* Re: [PATCH net-next v2 0/5] virtio: support packed ring
From: Michael S. Tsirkin @ 2018-07-13 3:26 UTC (permalink / raw)
To: David Miller
Cc: tiwei.bie, jasowang, virtualization, linux-kernel, netdev,
virtio-dev, wexu, jfreimann
In-Reply-To: <20180712.144458.2076041018423659380.davem@davemloft.net>
On Thu, Jul 12, 2018 at 02:44:58PM -0700, David Miller wrote:
> From: Tiwei Bie <tiwei.bie@intel.com>
> Date: Wed, 11 Jul 2018 10:27:06 +0800
>
> > Hello everyone,
> >
> > This patch set implements packed ring support in virtio driver.
> >
> > Some functional tests have been done with Jason's
> > packed ring implementation in vhost:
> >
> > https://lkml.org/lkml/2018/7/3/33
> >
> > Both of ping and netperf worked as expected.
>
> Michael and Jason, where are we with this series?
I'm at netdev, won't be able to review before Monday.
--
MST
^ permalink raw reply
* Re: Re: [Qemu-devel] [PATCH v3 0/3] Use of unique identifier for pairing virtio and passthrough devices...
From: Michael S. Tsirkin @ 2018-07-13 3:28 UTC (permalink / raw)
To: Samudrala, Sridhar
Cc: Siwei Liu, Cornelia Huck, si-wei liu, Roman Kagan, Venu Busireddy,
Marcel Apfelbaum, virtio-dev, qemu-devel, Alexander Duyck, Netdev
In-Reply-To: <77823ff9-a02a-808b-e919-b4950a64ae3c@intel.com>
On Thu, Jul 12, 2018 at 09:20:41PM -0400, Samudrala, Sridhar wrote:
> On 7/12/2018 6:19 PM, Siwei Liu wrote:
> > On Thu, Jul 12, 2018 at 2:00 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > > On Thu, Jul 12, 2018 at 01:52:53PM -0700, Siwei Liu wrote:
> > > > The definition is incomplete due to lack of spec. There's no "host"
> > > > part defined yet in the host-guest interface. If match by MAC is an
> > > > interface, the same must be done on the host(device) side as well,
> > > > which has been agreed not the way to go. However, I don't think that's
> > > > what the author intends to do by interpreting his QEMU patch - it
> > > > missed the other parts as well, such as the feature negotiation and
> > > > how it interacts with the paired device.
> > > >
> > > > What I said is that match by MAC is just a guest implementation that
> > > > one can change at any time. We now have the group ID on QEMU, why
> > > > still sticking to matching by MAC? It shoulnd't be a host-guest
> > > > interface in the first place anyway.
> > > I think that match by MAC is a simple portable way to match devices.
> > > E.g. it will work seamlessly with niche things like zPCI. However
> > That's a good point. I'm not sure if it's a valid assumption that zPCI
> > should always use the same MAC address as that of virtio. Someone
> > who's more familiar with the use case may decide and work on that. It
> > means VFIO device has to take in the MAC address as an identifier to
> > the "-device vfio-pci,.." QEMU option. I think there's no point to
> > match device using group ID in QEMU while using MAC in the guest.
> > Based on that assumption, I'd go with making VIRTIO_NET_F_STANDBY to
> > match device based on group ID, while someone may come up with another
> > feature bit later, say VIRTIO_NET_F_STANDBY_BY_MAC when its QEMU
> > support is available. Would it make sense?
>
> VIRTIO_NET_F_STANDBY as defined in the guest virtio_net driver supports match
> by MAC address. I think we should add support for this feature bit in QEMU.
> If submitting a patch to update the spec is a pre-requisite to add this
> feature bit to QEMU, i can do that.
It's not strictly a prerequisite but we need it in spec all the same, so
pls do that.
> As far as i understand, group id patches to QEMU are still under review.
> Matching by group ID can be another feature bit that could support matching
> by group id as well as MAC.
>
>
> > -Siwei
> >
> > > there are other niche use-cases that aren't addressed by match by MAC
> > > such as PF pass-through as a primary, and the pci bridge trick addresses
> > > that at cost of some portability.
> > >
> > > So I see no issues supporting both mechanisms, but others on the TC
> > > might feel differently.
> > >
> > > --
> > > MST
^ permalink raw reply
* Re: [PATCH v2 net-next 9/9] lan743x: Add PTP support
From: Richard Cochran @ 2018-07-13 3:31 UTC (permalink / raw)
To: Bryan Whitehead; +Cc: davem, netdev, UNGLinuxDriver
In-Reply-To: <1531422306-6547-10-git-send-email-Bryan.Whitehead@microchip.com>
On Thu, Jul 12, 2018 at 03:05:06PM -0400, Bryan Whitehead wrote:
> +static int lan743x_ethtool_get_ts_info(struct net_device *netdev,
> + struct ethtool_ts_info *ts_info)
> +{
> + struct lan743x_adapter *adapter = netdev_priv(netdev);
> +
> + ts_info->so_timestamping = SOF_TIMESTAMPING_TX_SOFTWARE |
> + SOF_TIMESTAMPING_RX_SOFTWARE |
> + SOF_TIMESTAMPING_SOFTWARE |
> + SOF_TIMESTAMPING_TX_HARDWARE |
> + SOF_TIMESTAMPING_RX_HARDWARE |
> + SOF_TIMESTAMPING_RAW_HARDWARE;
> +#ifdef CONFIG_PTP_1588_CLOCK
No need for this ifdeferry - ptp_clock_index() already returns -1 in
that case.
> + if (adapter->ptp.ptp_clock)
> + ts_info->phc_index = ptp_clock_index(adapter->ptp.ptp_clock);
> + else
> + ts_info->phc_index = -1;
> +#else
> + ts_info->phc_index = -1;
> +#endif
> + ts_info->tx_types = BIT(HWTSTAMP_TX_OFF) |
> + BIT(HWTSTAMP_TX_ON);
> + ts_info->rx_filters = BIT(HWTSTAMP_FILTER_NONE) |
> + BIT(HWTSTAMP_FILTER_ALL);
> + return 0;
> +}
> +
> @@ -690,6 +717,7 @@ const struct ethtool_ops lan743x_ethtool_ops = {
> .get_rxfh_indir_size = lan743x_ethtool_get_rxfh_indir_size,
> .get_rxfh = lan743x_ethtool_get_rxfh,
> .set_rxfh = lan743x_ethtool_set_rxfh,
> + .get_ts_info = lan743x_ethtool_get_ts_info,
> .get_eee = lan743x_ethtool_get_eee,
> .set_eee = lan743x_ethtool_set_eee,
> .get_link_ksettings = phy_ethtool_get_link_ksettings,
> diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
> index 953b581..ca9ae49 100644
> --- a/drivers/net/ethernet/microchip/lan743x_main.c
> +++ b/drivers/net/ethernet/microchip/lan743x_main.c
> @@ -267,6 +267,10 @@ static void lan743x_intr_shared_isr(void *context, u32 int_sts, u32 flags)
> lan743x_intr_software_isr(adapter);
> int_sts &= ~INT_BIT_SW_GP_;
> }
> + if (int_sts & INT_BIT_1588_) {
> + lan743x_ptp_isr(adapter);
> + int_sts &= ~INT_BIT_1588_;
> + }
> }
> if (int_sts)
> lan743x_csr_write(adapter, INT_EN_CLR, int_sts);
> @@ -976,6 +980,7 @@ static void lan743x_phy_link_status_change(struct net_device *netdev)
> ksettings.base.duplex,
> local_advertisement,
> remote_advertisement);
> + lan743x_ptp_update_latency(adapter, ksettings.base.speed);
> }
> }
>
> @@ -1256,11 +1261,29 @@ static void lan743x_tx_release_desc(struct lan743x_tx *tx,
> buffer_info->dma_ptr = 0;
> buffer_info->buffer_length = 0;
> }
> - if (buffer_info->skb) {
> + if (!buffer_info->skb)
> + goto clear_active;
> +
> + if (!(buffer_info->flags &
> + TX_BUFFER_INFO_FLAG_TIMESTAMP_REQUESTED)) {
Bad line break.
> dev_kfree_skb(buffer_info->skb);
> - buffer_info->skb = NULL;
> + goto clear_skb;
> }
>
> + if (cleanup) {
> + lan743x_ptp_unrequest_tx_timestamp(tx->adapter);
> + dev_kfree_skb(buffer_info->skb);
> + } else {
> + lan743x_ptp_tx_timestamp_skb(tx->adapter,
> + buffer_info->skb,
> + (buffer_info->flags &
> + TX_BUFFER_INFO_FLAG_IGNORE_SYNC)
> + != 0);
This is poor coding style. Please find a better way.
> + }
> +
> +clear_skb:
> + buffer_info->skb = NULL;
> +
> clear_active:
> buffer_info->flags &= ~TX_BUFFER_INFO_FLAG_ACTIVE;
>
> @@ -1321,10 +1344,25 @@ static int lan743x_tx_get_avail_desc(struct lan743x_tx *tx)
> return last_head - last_tail - 1;
> }
>
> +void lan743x_tx_set_timestamping_mode(struct lan743x_tx *tx,
> + bool enable_timestamping,
> + bool enable_onestep_sync)
> +{
> + if (enable_timestamping)
> + tx->ts_flags |= TX_TS_FLAG_TIMESTAMPING_ENABLED;
> + else
> + tx->ts_flags &= ~TX_TS_FLAG_TIMESTAMPING_ENABLED;
> + if (enable_onestep_sync)
> + tx->ts_flags |= TX_TS_FLAG_ONE_STEP_SYNC;
> + else
> + tx->ts_flags &= ~TX_TS_FLAG_ONE_STEP_SYNC;
> +}
> +
> static int lan743x_tx_frame_start(struct lan743x_tx *tx,
> unsigned char *first_buffer,
> unsigned int first_buffer_length,
> unsigned int frame_length,
> + bool time_stamp,
> bool check_sum)
> {
> /* called only from within lan743x_tx_xmit_frame.
> @@ -1362,6 +1400,8 @@ static int lan743x_tx_frame_start(struct lan743x_tx *tx,
> TX_DESC_DATA0_DTYPE_DATA_ |
> TX_DESC_DATA0_FS_ |
> TX_DESC_DATA0_FCS_;
> + if (time_stamp)
> + tx->frame_data0 |= TX_DESC_DATA0_TSE_;
>
> if (check_sum)
> tx->frame_data0 |= TX_DESC_DATA0_ICE_ |
> @@ -1475,6 +1515,7 @@ static int lan743x_tx_frame_add_fragment(struct lan743x_tx *tx,
>
> static void lan743x_tx_frame_end(struct lan743x_tx *tx,
> struct sk_buff *skb,
> + bool time_stamp,
> bool ignore_sync)
> {
> /* called only from within lan743x_tx_xmit_frame
> @@ -1492,6 +1533,8 @@ static void lan743x_tx_frame_end(struct lan743x_tx *tx,
> tx_descriptor = &tx->ring_cpu_ptr[tx->frame_tail];
> buffer_info = &tx->buffer_info[tx->frame_tail];
> buffer_info->skb = skb;
> + if (time_stamp)
> + buffer_info->flags |= TX_BUFFER_INFO_FLAG_TIMESTAMP_REQUESTED;
> if (ignore_sync)
> buffer_info->flags |= TX_BUFFER_INFO_FLAG_IGNORE_SYNC;
>
> @@ -1520,6 +1563,7 @@ static netdev_tx_t lan743x_tx_xmit_frame(struct lan743x_tx *tx,
> unsigned int frame_length = 0;
> unsigned int head_length = 0;
> unsigned long irq_flags = 0;
> + bool do_timestamp = false;
> bool ignore_sync = false;
> int nr_frags = 0;
> bool gso = false;
> @@ -1541,6 +1585,16 @@ static netdev_tx_t lan743x_tx_xmit_frame(struct lan743x_tx *tx,
> }
>
> /* space available, transmit skb */
> + if (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) {
> + if (tx->ts_flags & TX_TS_FLAG_TIMESTAMPING_ENABLED) {
> + if (lan743x_ptp_request_tx_timestamp(tx->adapter)) {
Why not use && instead of three nested tests?
> + skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
> + do_timestamp = true;
> + if (tx->ts_flags & TX_TS_FLAG_ONE_STEP_SYNC)
> + ignore_sync = true;
> + }
> + }
> + }
> head_length = skb_headlen(skb);
> frame_length = skb_pagelen(skb);
> nr_frags = skb_shinfo(skb)->nr_frags;
> @@ -1554,6 +1608,7 @@ static netdev_tx_t lan743x_tx_xmit_frame(struct lan743x_tx *tx,
> if (lan743x_tx_frame_start(tx,
> skb->data, head_length,
> start_frame_length,
> + do_timestamp,
> skb->ip_summed == CHECKSUM_PARTIAL)) {
> dev_kfree_skb(skb);
> goto unlock;
> @@ -1581,7 +1636,7 @@ static netdev_tx_t lan743x_tx_xmit_frame(struct lan743x_tx *tx,
> }
>
> finish:
> - lan743x_tx_frame_end(tx, skb, ignore_sync);
> + lan743x_tx_frame_end(tx, skb, do_timestamp, ignore_sync);
>
> unlock:
> spin_unlock_irqrestore(&tx->ring_lock, irq_flags);
> @@ -2410,6 +2465,8 @@ static int lan743x_netdev_close(struct net_device *netdev)
> for (index = 0; index < LAN743X_USED_RX_CHANNELS; index++)
> lan743x_rx_close(&adapter->rx[index]);
>
> + lan743x_ptp_close(adapter);
> +
> lan743x_phy_close(adapter);
>
> lan743x_mac_close(adapter);
> @@ -2437,6 +2494,10 @@ static int lan743x_netdev_open(struct net_device *netdev)
> if (ret)
> goto close_mac;
>
> + ret = lan743x_ptp_open(adapter);
> + if (ret)
> + goto close_phy;
> +
> lan743x_rfe_open(adapter);
>
> for (index = 0; index < LAN743X_USED_RX_CHANNELS; index++) {
> @@ -2456,6 +2517,9 @@ static int lan743x_netdev_open(struct net_device *netdev)
> if (adapter->rx[index].ring_cpu_ptr)
> lan743x_rx_close(&adapter->rx[index]);
> }
> + lan743x_ptp_close(adapter);
> +
> +close_phy:
> lan743x_phy_close(adapter);
>
> close_mac:
> @@ -2483,6 +2547,8 @@ static int lan743x_netdev_ioctl(struct net_device *netdev,
> {
> if (!netif_running(netdev))
> return -EINVAL;
> + if (cmd == SIOCSHWTSTAMP)
> + return lan743x_ptp_ioctl(netdev, ifr, cmd);
> return phy_mii_ioctl(netdev->phydev, ifr, cmd);
> }
>
> @@ -2607,6 +2673,11 @@ static int lan743x_hardware_init(struct lan743x_adapter *adapter,
> adapter->intr.irq = adapter->pdev->irq;
> lan743x_csr_write(adapter, INT_EN_CLR, 0xFFFFFFFF);
> mutex_init(&adapter->dp_lock);
> +
> + ret = lan743x_gpio_init(adapter);
> + if (ret)
> + return ret;
> +
> ret = lan743x_mac_init(adapter);
> if (ret)
> return ret;
> @@ -2615,6 +2686,10 @@ static int lan743x_hardware_init(struct lan743x_adapter *adapter,
> if (ret)
> return ret;
>
> + ret = lan743x_ptp_init(adapter);
> + if (ret)
> + return ret;
> +
> lan743x_rfe_update_mac_address(adapter);
>
> ret = lan743x_dmac_init(adapter);
...
> diff --git a/drivers/net/ethernet/microchip/lan743x_ptp.c b/drivers/net/ethernet/microchip/lan743x_ptp.c
> new file mode 100644
> index 0000000..f14565b
> --- /dev/null
> +++ b/drivers/net/ethernet/microchip/lan743x_ptp.c
> @@ -0,0 +1,1194 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/* Copyright (C) 2018 Microchip Technology Inc. */
> +
> +#include <linux/netdevice.h>
> +#include "lan743x_main.h"
> +
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/netdevice.h>
> +#include <linux/net_tstamp.h>
> +
> +#include "lan743x_ptp.h"
> +
> +/* GPIO */
> +#define LAN743X_NUMBER_OF_GPIO (12)
> +
> +int lan743x_gpio_init(struct lan743x_adapter *adapter)
> +{
> + struct lan743x_gpio *gpio = &adapter->gpio;
> +
> + spin_lock_init(&gpio->gpio_lock);
> +
> + gpio->gpio_cfg0 = 0; /* set all direction to input, data = 0 */
> + gpio->gpio_cfg1 = 0x0FFF0000;/* disable all gpio, set to open drain */
> + gpio->gpio_cfg2 = 0;/* set all to 1588 low polarity level */
> + gpio->gpio_cfg3 = 0;/* disable all 1588 output */
> + lan743x_csr_write(adapter, GPIO_CFG0, gpio->gpio_cfg0);
> + lan743x_csr_write(adapter, GPIO_CFG1, gpio->gpio_cfg1);
> + lan743x_csr_write(adapter, GPIO_CFG2, gpio->gpio_cfg2);
> + lan743x_csr_write(adapter, GPIO_CFG3, gpio->gpio_cfg3);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static int lan743x_gpio_reserve_ptp_output(struct lan743x_adapter *adapter,
> + int bit, int ptp_channel)
> +{
> + struct lan743x_gpio *gpio = &adapter->gpio;
> + unsigned long irq_flags = 0;
> + int bit_mask = BIT(bit);
> + int ret = -EBUSY;
> +
> + spin_lock_irqsave(&gpio->gpio_lock, irq_flags);
> +
> + if (!(gpio->used_bits & bit_mask)) {
> + gpio->used_bits |= bit_mask;
> + gpio->output_bits |= bit_mask;
> + gpio->ptp_bits |= bit_mask;
> +
> + /* set as output, and zero initial value */
> + gpio->gpio_cfg0 |= GPIO_CFG0_GPIO_DIR_BIT_(bit);
> + gpio->gpio_cfg0 &= ~GPIO_CFG0_GPIO_DATA_BIT_(bit);
> + lan743x_csr_write(adapter, GPIO_CFG0, gpio->gpio_cfg0);
> +
> + /* enable gpio , and set buffer type to push pull */
> + gpio->gpio_cfg1 &= ~GPIO_CFG1_GPIOEN_BIT_(bit);
> + gpio->gpio_cfg1 |= GPIO_CFG1_GPIOBUF_BIT_(bit);
> + lan743x_csr_write(adapter, GPIO_CFG1, gpio->gpio_cfg1);
> +
> + /* set 1588 polarity to high */
> + gpio->gpio_cfg2 |= GPIO_CFG2_1588_POL_BIT_(bit);
> + lan743x_csr_write(adapter, GPIO_CFG2, gpio->gpio_cfg2);
> +
> + if (!ptp_channel) {
> + /* use channel A */
> + gpio->gpio_cfg3 &= ~GPIO_CFG3_1588_CH_SEL_BIT_(bit);
> + } else {
> + /* use channel B */
> + gpio->gpio_cfg3 |= GPIO_CFG3_1588_CH_SEL_BIT_(bit);
> + }
> + gpio->gpio_cfg3 |= GPIO_CFG3_1588_OE_BIT_(bit);
> + lan743x_csr_write(adapter, GPIO_CFG3, gpio->gpio_cfg3);
> +
> + ret = bit;
> + }
> + spin_unlock_irqrestore(&gpio->gpio_lock, irq_flags);
> + return ret;
> +}
> +#endif /* CONFIG_PTP_1588_CLOCK */
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static void lan743x_gpio_release(struct lan743x_adapter *adapter, int bit)
> +{
> + struct lan743x_gpio *gpio = &adapter->gpio;
> + unsigned long irq_flags = 0;
> + int bit_mask = BIT(bit);
> +
> + spin_lock_irqsave(&gpio->gpio_lock, irq_flags);
> + if (gpio->used_bits & bit_mask) {
> + gpio->used_bits &= ~bit_mask;
> + if (gpio->output_bits & bit_mask) {
> + gpio->output_bits &= ~bit_mask;
> +
> + if (gpio->ptp_bits & bit_mask) {
> + gpio->ptp_bits &= ~bit_mask;
> + /* disable ptp output */
> + gpio->gpio_cfg3 &= ~GPIO_CFG3_1588_OE_BIT_(bit);
> + lan743x_csr_write(adapter, GPIO_CFG3,
> + gpio->gpio_cfg3);
> + }
> + /* release gpio output */
> +
> + /* disable gpio */
> + gpio->gpio_cfg1 |= GPIO_CFG1_GPIOEN_BIT_(bit);
> + gpio->gpio_cfg1 &= ~GPIO_CFG1_GPIOBUF_BIT_(bit);
> + lan743x_csr_write(adapter, GPIO_CFG1, gpio->gpio_cfg1);
> +
> + /* reset back to input */
> + gpio->gpio_cfg0 &= ~GPIO_CFG0_GPIO_DIR_BIT_(bit);
> + gpio->gpio_cfg0 &= ~GPIO_CFG0_GPIO_DATA_BIT_(bit);
> + lan743x_csr_write(adapter, GPIO_CFG0, gpio->gpio_cfg0);
> + }
> + }
> + spin_unlock_irqrestore(&gpio->gpio_lock, irq_flags);
> +}
> +#endif /* CONFIG_PTP_1588_CLOCK */
> +
> +/* PTP */
> +#define LAN743X_PTP_MAX_FREQ_ADJ_IN_PPB (31249999)
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static int lan743x_ptp_reserve_event_ch(struct lan743x_adapter *adapter);
> +static void lan743x_ptp_release_event_ch(struct lan743x_adapter *adapter,
> + int event_channel);
> +#endif
> +
> +static bool lan743x_ptp_is_enabled(struct lan743x_adapter *adapter);
> +static void lan743x_ptp_enable(struct lan743x_adapter *adapter);
> +static void lan743x_ptp_disable(struct lan743x_adapter *adapter);
> +static void lan743x_ptp_reset(struct lan743x_adapter *adapter);
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static void lan743x_ptp_clock_get(struct lan743x_adapter *adapter,
> + u32 *seconds, u32 *nano_seconds,
> + u32 *sub_nano_seconds);
> +static int lan743x_ptp_enable_pps(struct lan743x_adapter *adapter);
> +static void lan743x_ptp_disable_pps(struct lan743x_adapter *adapter);
> +#endif /* CONFIG_PTP_1588_CLOCK */
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static void lan743x_ptp_clock_step(struct lan743x_adapter *adapter,
> + s64 time_step_ns);
> +#endif /* CONFIG_PTP_1588_CLOCK */
The constant ifdef CONFIG_PTP_1588_CLOCK is poor style and
unnecessary. Just group this code together under one ifdef, or better
yet put it into its own file.
> +static void lan743x_ptp_clock_set(struct lan743x_adapter *adapter,
> + u32 seconds, u32 nano_seconds,
> + u32 sub_nano_seconds);
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static int lan743x_ptpci_adjfreq(struct ptp_clock_info *ptpci, s32 delta_ppb)
Please implement adjfine().
* @adjfine: Adjusts the frequency of the hardware clock.
* parameter scaled_ppm: Desired frequency offset from
* nominal frequency in parts per million, but with a
* 16 bit binary fractional field.
*
* @adjfreq: Adjusts the frequency of the hardware clock.
* This method is deprecated. New drivers should implement
* the @adjfine method instead.
* parameter delta: Desired frequency offset from nominal frequency
* in parts per billion
> +{
> + struct lan743x_ptp *ptp = container_of(ptpci, struct lan743x_ptp,
> + ptp_clock_info);
> + struct lan743x_adapter *adapter = container_of(ptp,
> + struct lan743x_adapter,
> + ptp);
Coding style:
struct lan743x_adapter *adapter =
container_of(ptp, struct lan743x_adapter, ptp);
> + u32 lan743x_rate_adj = 0;
> + bool positive = true;
> + u32 u32_delta = 0;
> + u64 u64_delta = 0;
> +
> + if ((delta_ppb < (-LAN743X_PTP_MAX_FREQ_ADJ_IN_PPB)) ||
> + delta_ppb > LAN743X_PTP_MAX_FREQ_ADJ_IN_PPB) {
> + return -EINVAL;
> + }
> + if (delta_ppb > 0) {
> + u32_delta = (u32)delta_ppb;
> + positive = true;
> + } else {
> + u32_delta = (u32)(-delta_ppb);
> + positive = false;
> + }
> + u64_delta = (((u64)u32_delta) * 0x800000000ULL);
> + lan743x_rate_adj = (u32)(u64_delta / 1000000000);
You need to use the div_u64() macro here.
> +
> + if (positive)
> + lan743x_rate_adj |= PTP_CLOCK_RATE_ADJ_DIR_;
> +
> + lan743x_csr_write(adapter, PTP_CLOCK_RATE_ADJ,
> + lan743x_rate_adj);
> +
> + netif_info(adapter, drv, adapter->netdev,
> + "adjfreq, delta_ppb = %d, lan743x_rate_adj = 0x%08X\n",
> + delta_ppb, lan743x_rate_adj);
This definitely should be at the debug level, or just delete it altogether.
> + return 0;
> +}
> +#endif /*CONFIG_PTP_1588_CLOCK */
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static int lan743x_ptpci_adjtime(struct ptp_clock_info *ptpci, s64 delta)
> +{
> + struct lan743x_ptp *ptp = container_of(ptpci, struct lan743x_ptp,
> + ptp_clock_info);
> + struct lan743x_adapter *adapter = container_of(ptp,
> + struct lan743x_adapter,
> + ptp);
Coding style.
> + bool enable_pps = false;
> +
> + if (ptp->pps_event_ch >= 0) {
> + lan743x_ptp_disable_pps(adapter);
> + enable_pps = true;
> + }
> +
> + lan743x_ptp_clock_step(adapter, delta);
> + netif_info(adapter, drv, adapter->netdev,
> + "adjtime, delta = %lld\n", delta);
Again, debug or delete.
> +
> + if (enable_pps)
> + lan743x_ptp_enable_pps(adapter);
> +
> + return 0;
> +}
> +#endif /*CONFIG_PTP_1588_CLOCK */
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static int lan743x_ptpci_gettime64(struct ptp_clock_info *ptpci,
> + struct timespec64 *ts)
> +{
> + struct lan743x_ptp *ptp = container_of(ptpci, struct lan743x_ptp,
> + ptp_clock_info);
> + struct lan743x_adapter *adapter = container_of(ptp,
> + struct lan743x_adapter,
> + ptp);
Style.
> +
> + if (ts) {
> + u32 seconds = 0;
> + u32 nano_seconds = 0;
Please declare stack variables at the top of the function.
> +
> + lan743x_ptp_clock_get(adapter, &seconds, &nano_seconds, NULL);
> + ts->tv_sec = seconds;
> + ts->tv_nsec = nano_seconds;
> + netif_info(adapter, drv, adapter->netdev,
> + "gettime = %u.%09u\n", seconds, nano_seconds);
Debug/delete
> + } else {
> + netif_warn(adapter, drv, adapter->netdev, "ts == NULL\n");
> + return -EINVAL;
No need to test for 'ts'. The caller must supply a valid pointer.
> + }
> + return 0;
> +}
> +#endif /*CONFIG_PTP_1588_CLOCK */
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static int lan743x_ptpci_settime64(struct ptp_clock_info *ptpci,
> + const struct timespec64 *ts)
> +{
> + struct lan743x_ptp *ptp = container_of(ptpci, struct lan743x_ptp,
> + ptp_clock_info);
> + struct lan743x_adapter *adapter = container_of(ptp,
> + struct lan743x_adapter,
> + ptp);
> + bool enable_pps = false;
> +
> + if (ptp->pps_event_ch >= 0) {
> + lan743x_ptp_disable_pps(adapter);
> + enable_pps = true;
> + }
> +
> + if (ts) {
> + u32 seconds = 0;
> + u32 nano_seconds = 0;
> +
> + if (ts->tv_sec > 0xFFFFFFFFLL ||
> + ts->tv_sec < 0) {
Actually seconds will exceed four bytes sooner than you think. If
your HW has a restriction, then simply keep the seconds offset in SW
in the driver, adding it in where needed (like when reading the clock
or providing time stamps).
> + netif_warn(adapter, drv, adapter->netdev,
> + "ts->tv_sec out of range, %lld\n",
> + ts->tv_sec);
> + return -EINVAL;
> + }
> + if (ts->tv_nsec >= 1000000000L ||
> + ts->tv_nsec < 0) {
> + netif_warn(adapter, drv, adapter->netdev,
> + "ts->tv_nsec out of range, %ld\n",
> + ts->tv_nsec);
> + return -EINVAL;
> + }
> + seconds = ts->tv_sec;
> + nano_seconds = ts->tv_nsec;
> + netif_info(adapter, drv, adapter->netdev,
> + "settime = %u.%09u\n", seconds, nano_seconds);
> + lan743x_ptp_clock_set(adapter, seconds, nano_seconds, 0);
> + } else {
> + netif_warn(adapter, drv, adapter->netdev, "ts == NULL\n");
> + return -EINVAL;
> + }
> +
> + if (enable_pps)
> + lan743x_ptp_enable_pps(adapter);
> +
> + return 0;
> +}
> +#endif /*CONFIG_PTP_1588_CLOCK */
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static int lan743x_ptp_enable_pps(struct lan743x_adapter *adapter)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> + u32 current_seconds = 0;
> + u32 target_seconds = 0;
> + u32 general_config = 0;
> + int result = -ENODEV;
> + int pps_bit = 0;
So this function is really *not* implementing the PTP_CLK_REQ_PPS
feature but rather the PTP_CLK_REQ_PEROUT with a period of once per
second.
PTP_CLK_REQ_PPS means placing a PPS event into the kernel's "hardpps"
subsystem by calling ptp_clock_event().
I'm sorry this isn't really documented. I should fix that.
If you HW can output arbitrary signals, then you should implement
PTP_CLK_REQ_PEROUT. In any case, you shouldn't advertise the
ptp_clock_info.pps capability.
> + if (ptp->pps_event_ch >= 0) {
> + result = 0;
> + goto done;
> + }
> +
> + ptp->pps_event_ch = lan743x_ptp_reserve_event_ch(adapter);
> + if (ptp->pps_event_ch < 0) {
> + netif_warn(adapter, drv, adapter->netdev,
> + "Failed to reserve event channel for PPS\n");
> + goto done;
> + }
> +
> + switch(adapter->csr.id_rev & ID_REV_ID_MASK_) {
> + case ID_REV_ID_LAN7430_:
> + pps_bit = 2;/* GPIO 2 is preferred on EVB LAN7430 */
> + break;
> + case ID_REV_ID_LAN7431_:
> + pps_bit = 4;/* GPIO 4 is preferred on EVB LAN7431 */
> + break;
> + }
> +
> + ptp->pps_gpio_bit = lan743x_gpio_reserve_ptp_output(adapter, pps_bit,
> + ptp->pps_event_ch);
> +
> + if (ptp->pps_gpio_bit < 0) {
> + netif_warn(adapter, drv, adapter->netdev,
> + "Failed to reserve gpio 0 for PPS\n");
> + goto done;
> + }
> +
> + lan743x_ptp_clock_get(adapter, ¤t_seconds, NULL, NULL);
> +
> + /* set the first target ahead by 2 seconds
> + * to make sure its not missed
> + */
> + target_seconds = current_seconds + 2;
> +
> + /* set the new target */
> + lan743x_csr_write(adapter,
> + PTP_CLOCK_TARGET_SEC_X(ptp->pps_event_ch),
> + 0xFFFF0000);
> + lan743x_csr_write(adapter,
> + PTP_CLOCK_TARGET_NS_X(ptp->pps_event_ch), 0);
> +
> + general_config = lan743x_csr_read(adapter, PTP_GENERAL_CONFIG);
> +
> + general_config &= ~(PTP_GENERAL_CONFIG_CLOCK_EVENT_X_MASK_
> + (ptp->pps_event_ch));
> + general_config |= PTP_GENERAL_CONFIG_CLOCK_EVENT_X_SET_
> + (ptp->pps_event_ch,
> + PTP_GENERAL_CONFIG_CLOCK_EVENT_100US_);
> + general_config &= ~PTP_GENERAL_CONFIG_RELOAD_ADD_X_
> + (ptp->pps_event_ch);
> + lan743x_csr_write(adapter, PTP_GENERAL_CONFIG, general_config);
> +
> + /* set the reload to one second steps */
> + lan743x_csr_write(adapter,
> + PTP_CLOCK_TARGET_RELOAD_SEC_X(ptp->pps_event_ch),
> + 1);
> + lan743x_csr_write(adapter,
> + PTP_CLOCK_TARGET_RELOAD_NS_X(ptp->pps_event_ch),
> + 0);
> +
> + /* set the new target */
> + lan743x_csr_write(adapter,
> + PTP_CLOCK_TARGET_SEC_X(ptp->pps_event_ch),
> + target_seconds);
> + lan743x_csr_write(adapter,
> + PTP_CLOCK_TARGET_NS_X(ptp->pps_event_ch),
> + 0);
> + return 0;
> +
> +done:
> + if (ptp->pps_gpio_bit >= 0) {
> + lan743x_gpio_release(adapter, ptp->pps_gpio_bit);
> + ptp->pps_gpio_bit = -1;
> + }
> + if (ptp->pps_event_ch >= 0) {
> + lan743x_ptp_release_event_ch(adapter,
> + ptp->pps_event_ch);
> + ptp->pps_event_ch = -1;
> + }
> + return result;
> +}
> +#endif /* CONFIG_PTP_1588_CLOCK */
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static void lan743x_ptp_disable_pps(struct lan743x_adapter *adapter)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> +
> + if (ptp->pps_gpio_bit >= 0) {
> + lan743x_gpio_release(adapter, ptp->pps_gpio_bit);
> + ptp->pps_gpio_bit = -1;
> + }
> +
> + if (ptp->pps_event_ch >= 0) {
> + u32 general_config = 0;
> +
> + /* set target to far in the future, effectively disabling it */
> + lan743x_csr_write(adapter,
> + PTP_CLOCK_TARGET_SEC_X(ptp->pps_event_ch),
> + 0xFFFF0000);
> + lan743x_csr_write(adapter,
> + PTP_CLOCK_TARGET_NS_X(ptp->pps_event_ch), 0);
> +
> + general_config = lan743x_csr_read(adapter, PTP_GENERAL_CONFIG);
> + general_config |= PTP_GENERAL_CONFIG_RELOAD_ADD_X_
> + (ptp->pps_event_ch);
> + lan743x_csr_write(adapter, PTP_GENERAL_CONFIG, general_config);
> + lan743x_ptp_release_event_ch(adapter, ptp->pps_event_ch);
> + ptp->pps_event_ch = -1;
> + }
> +}
> +#endif /* CONFIG_PTP_1588_CLOCK */
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static int lan743x_ptpci_enable(struct ptp_clock_info *ptpci,
> + struct ptp_clock_request *request, int on)
> +{
> + struct lan743x_ptp *ptp = container_of(ptpci, struct lan743x_ptp,
> + ptp_clock_info);
> + struct lan743x_adapter *adapter = container_of(ptp,
> + struct lan743x_adapter,
> + ptp);
> +
> + if (request) {
> + switch (request->type) {
> + case PTP_CLK_REQ_EXTTS:
> + return -EINVAL;
> + case PTP_CLK_REQ_PEROUT:
> + return -EINVAL;
> + case PTP_CLK_REQ_PPS:
> + if (on) {
> + if (lan743x_ptp_enable_pps(adapter) >= 0)
> + netif_info(adapter, drv,
> + adapter->netdev,
> + "PPS is ON\n");
> + else
> + netif_warn(adapter, drv,
> + adapter->netdev,
> + "Error starting PPS\n");
> + } else {
> + lan743x_ptp_disable_pps(adapter);
> + netif_info(adapter, drv, adapter->netdev,
> + "PPS is OFF\n");
> + }
> + break;
> + default:
> + netif_err(adapter, drv, adapter->netdev,
> + "request->type == %d, Unknown\n",
> + request->type);
> + break;
> + }
> + } else {
> + netif_err(adapter, drv, adapter->netdev, "request == NULL\n");
> + }
> + return 0;
> +}
> +#endif /* CONFIG_PTP_1588_CLOCK */
> +
> +void lan743x_ptp_isr(void *context)
> +{
> + struct lan743x_adapter *adapter = (struct lan743x_adapter *)context;
> + struct lan743x_ptp *ptp = NULL;
> + int enable_flag = 1;
> + u32 ptp_int_sts = 0;
> +
> + ptp = &adapter->ptp;
> +
> + lan743x_csr_write(adapter, INT_EN_CLR, INT_BIT_1588_);
> +
> + ptp_int_sts = lan743x_csr_read(adapter, PTP_INT_STS);
> + ptp_int_sts &= lan743x_csr_read(adapter, PTP_INT_EN_SET);
> +
> + if (ptp_int_sts & PTP_INT_BIT_TX_TS_) {
> + tasklet_schedule(&ptp->ptp_isr_bottom_half);
Please no new tasklets. Instead use a work queue. If you need lower
latency, consider using ptp_schedule_worker().
> + enable_flag = 0;/* tasklet will re-enable later */
> + }
> + if (ptp_int_sts & PTP_INT_BIT_TX_SWTS_ERR_) {
> + netif_err(adapter, drv, adapter->netdev,
> + "PTP TX Software Timestamp Error\n");
> + /* clear int status bit */
> + lan743x_csr_write(adapter, PTP_INT_STS,
> + PTP_INT_BIT_TX_SWTS_ERR_);
> + }
> + if (ptp_int_sts & PTP_INT_BIT_TIMER_B_) {
> + netif_info(adapter, drv, adapter->netdev,
> + "PTP TIMER B Interrupt\n");
Don't print like this from an ISR. Or is this an error, since you
don't enable this bit?
> + /* clear int status bit */
> + lan743x_csr_write(adapter, PTP_INT_STS,
> + PTP_INT_BIT_TIMER_B_);
> + }
> + if (ptp_int_sts & PTP_INT_BIT_TIMER_A_) {
> + netif_info(adapter, drv, adapter->netdev,
> + "PTP TIMER A Interrupt\n");
> + /* clear int status bit */
> + lan743x_csr_write(adapter, PTP_INT_STS,
> + PTP_INT_BIT_TIMER_A_);
> + }
> +
> + if (enable_flag) {
> + /* re-enable isr */
> + lan743x_csr_write(adapter, INT_EN_SET, INT_BIT_1588_);
> + }
> +}
> +
> +static void lan743x_ptp_tx_ts_complete(struct lan743x_adapter *adapter)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> + int i;
> + int c;
Put same types on one line:
int c, i;
> +
> + spin_lock_bh(&ptp->tx_ts_lock);
> + c = ptp->tx_ts_skb_queue_size;
> +
> + if (c > ptp->tx_ts_queue_size)
> + c = ptp->tx_ts_queue_size;
> + if (c <= 0)
> + goto done;
> +
> + for (i = 0; i < c; i++) {
> + bool ignore_sync = ((ptp->tx_ts_ignore_sync_queue &
> + BIT(i)) != 0);
> + struct sk_buff *skb = ptp->tx_ts_skb_queue[i];
> + u32 nseconds = ptp->tx_ts_nseconds_queue[i];
> + u32 seconds = ptp->tx_ts_seconds_queue[i];
> + u32 header = ptp->tx_ts_header_queue[i];
> + struct skb_shared_hwtstamps tstamps;
Locals to top of function please.
> + memset(&tstamps, 0, sizeof(tstamps));
> + tstamps.hwtstamp = ktime_set(seconds, nseconds);
> + if (!ignore_sync ||
> + ((header & PTP_TX_MSG_HEADER_MSG_TYPE_) !=
> + PTP_TX_MSG_HEADER_MSG_TYPE_SYNC_))
> + skb_tstamp_tx(skb, &tstamps);
> +
> + dev_kfree_skb(skb);
> +
> + ptp->tx_ts_skb_queue[i] = NULL;
> + ptp->tx_ts_seconds_queue[i] = 0;
> + ptp->tx_ts_nseconds_queue[i] = 0;
> + ptp->tx_ts_header_queue[i] = 0;
> + }
> +
> + /* shift queue */
> + ptp->tx_ts_ignore_sync_queue >>= c;
> + for (i = c; i < LAN743X_PTP_NUMBER_OF_TX_TIMESTAMPS; i++) {
> + ptp->tx_ts_skb_queue[i - c] = ptp->tx_ts_skb_queue[i];
> + ptp->tx_ts_seconds_queue[i - c] = ptp->tx_ts_seconds_queue[i];
> + ptp->tx_ts_nseconds_queue[i - c] = ptp->tx_ts_nseconds_queue[i];
> + ptp->tx_ts_header_queue[i - c] = ptp->tx_ts_header_queue[i];
> +
> + ptp->tx_ts_skb_queue[i] = NULL;
> + ptp->tx_ts_seconds_queue[i] = 0;
> + ptp->tx_ts_nseconds_queue[i] = 0;
> + ptp->tx_ts_header_queue[i] = 0;
> + }
> + ptp->tx_ts_skb_queue_size -= c;
> + ptp->tx_ts_queue_size -= c;
> +done:
> + ptp->pending_tx_timestamps -= c;
> + spin_unlock_bh(&ptp->tx_ts_lock);
> +}
> +
> +static void lan743x_ptp_tx_ts_enqueue_skb(struct lan743x_adapter *adapter,
> + struct sk_buff *skb, bool ignore_sync)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> +
> + spin_lock_bh(&ptp->tx_ts_lock);
> + if (ptp->tx_ts_skb_queue_size < LAN743X_PTP_NUMBER_OF_TX_TIMESTAMPS) {
> + ptp->tx_ts_skb_queue[ptp->tx_ts_skb_queue_size] = skb;
> + if (ignore_sync)
> + ptp->tx_ts_ignore_sync_queue |=
> + BIT(ptp->tx_ts_skb_queue_size);
> + ptp->tx_ts_skb_queue_size++;
> + } else {
> + /* this should never happen, so long as the tx channel
> + * calls and honors the result from
> + * lan743x_ptp_request_tx_timestamp
> + */
> + netif_err(adapter, drv, adapter->netdev,
> + "tx ts skb queue overflow\n");
> + dev_kfree_skb(skb);
> + }
> + spin_unlock_bh(&ptp->tx_ts_lock);
> +}
> +
> +static void lan743x_ptp_tx_ts_enqueue_ts(struct lan743x_adapter *adapter,
> + u32 seconds, u32 nano_seconds,
> + u32 header)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> +
> + spin_lock_bh(&ptp->tx_ts_lock);
> + if (ptp->tx_ts_queue_size < LAN743X_PTP_NUMBER_OF_TX_TIMESTAMPS) {
> + ptp->tx_ts_seconds_queue[ptp->tx_ts_queue_size] = seconds;
> + ptp->tx_ts_nseconds_queue[ptp->tx_ts_queue_size] = nano_seconds;
> + ptp->tx_ts_header_queue[ptp->tx_ts_queue_size] = header;
> + ptp->tx_ts_queue_size++;
> + } else {
> + netif_err(adapter, drv, adapter->netdev,
> + "tx ts queue overflow\n");
> + }
> + spin_unlock_bh(&ptp->tx_ts_lock);
> +}
> +
> +static void lan743x_ptp_isr_bottom_half(unsigned long param)
> +{
> + struct lan743x_adapter *adapter = (struct lan743x_adapter *)param;
> + bool new_timestamp_available = false;
> +
> + while (lan743x_csr_read(adapter, PTP_INT_STS) & PTP_INT_BIT_TX_TS_) {
As a sanity check, you should break this loop using a counter.
> + u32 cap_info = lan743x_csr_read(adapter, PTP_CAP_INFO);
> +
> + if (PTP_CAP_INFO_TX_TS_CNT_GET_(cap_info) > 0) {
> + u32 seconds = lan743x_csr_read(adapter,
> + PTP_TX_EGRESS_SEC);
> + u32 nsec = lan743x_csr_read(adapter, PTP_TX_EGRESS_NS);
> + u32 cause = (nsec &
> + PTP_TX_EGRESS_NS_CAPTURE_CAUSE_MASK_);
> + u32 header = lan743x_csr_read(adapter,
> + PTP_TX_MSG_HEADER);
> +
> + if (cause == PTP_TX_EGRESS_NS_CAPTURE_CAUSE_SW_) {
> + nsec &= PTP_TX_EGRESS_NS_TS_NS_MASK_;
> + lan743x_ptp_tx_ts_enqueue_ts(adapter,
> + seconds, nsec,
> + header);
> + new_timestamp_available = true;
> + } else if (cause ==
> + PTP_TX_EGRESS_NS_CAPTURE_CAUSE_AUTO_) {
> + netif_err(adapter, drv, adapter->netdev,
> + "Auto capture cause not supported\n");
> + } else {
> + netif_warn(adapter, drv, adapter->netdev,
> + "unknown tx timestamp capture cause\n");
> + }
> + } else {
> + netif_warn(adapter, drv, adapter->netdev,
> + "TX TS INT but no TX TS CNT\n");
> + }
> + lan743x_csr_write(adapter, PTP_INT_STS, PTP_INT_BIT_TX_TS_);
> + }
> +
> + if (new_timestamp_available)
> + lan743x_ptp_tx_ts_complete(adapter);
> +
> + lan743x_csr_write(adapter, INT_EN_SET, INT_BIT_1588_);
> +}
> +
> +static void lan743x_ptp_sync_to_system_clock(struct lan743x_adapter *adapter)
> +{
> + struct timeval tv;
> +
> + memset(&tv, 0, sizeof(tv));
> + do_gettimeofday(&tv);
Use the TAI clock instead.
> + lan743x_ptp_clock_set(adapter, tv.tv_sec, tv.tv_usec * 1000, 0);
> +}
> +
> +void lan743x_ptp_update_latency(struct lan743x_adapter *adapter,
> + u32 link_speed)
> +{
> + switch (link_speed) {
> + case 10:
> + lan743x_csr_write(adapter, PTP_LATENCY,
> + PTP_LATENCY_TX_SET_(0) |
> + PTP_LATENCY_RX_SET_(0));
> + break;
> + case 100:
> + lan743x_csr_write(adapter, PTP_LATENCY,
> + PTP_LATENCY_TX_SET_(181) |
> + PTP_LATENCY_RX_SET_(594));
> + break;
> + case 1000:
> + lan743x_csr_write(adapter, PTP_LATENCY,
> + PTP_LATENCY_TX_SET_(30) |
> + PTP_LATENCY_RX_SET_(525));
> + break;
> + }
> +}
> +
> +int lan743x_ptp_init(struct lan743x_adapter *adapter)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> +
> + mutex_init(&ptp->command_lock);
> + spin_lock_init(&ptp->tx_ts_lock);
> + tasklet_init(&ptp->ptp_isr_bottom_half,
> + lan743x_ptp_isr_bottom_half, (unsigned long)adapter);
> + tasklet_disable(&ptp->ptp_isr_bottom_half);
> + ptp->used_event_ch = 0;
> + ptp->pps_event_ch = -1;
> + ptp->pps_gpio_bit = -1;
> + return 0;
> +}
> +
> +int lan743x_ptp_open(struct lan743x_adapter *adapter)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> + int ret = -ENODEV;
> + u32 temp;
> +
> + lan743x_ptp_reset(adapter);
> + lan743x_ptp_sync_to_system_clock(adapter);
> + temp = lan743x_csr_read(adapter, PTP_TX_MOD2);
> + temp |= PTP_TX_MOD2_TX_PTP_CLR_UDPV4_CHKSUM_;
> + lan743x_csr_write(adapter, PTP_TX_MOD2, temp);
> + lan743x_ptp_enable(adapter);
> + tasklet_enable(&ptp->ptp_isr_bottom_half);
> + lan743x_csr_write(adapter, INT_EN_SET, INT_BIT_1588_);
> + lan743x_csr_write(adapter, PTP_INT_EN_SET,
> + PTP_INT_BIT_TX_SWTS_ERR_ | PTP_INT_BIT_TX_TS_);
> + ptp->flags |= PTP_FLAG_ISR_ENABLED;
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> + snprintf(ptp->pin_config[0].name, 32, "lan743x_ptp_pin_0");
> + ptp->pin_config[0].index = 0;
> + ptp->pin_config[0].func = PTP_PF_PEROUT;
> + ptp->pin_config[0].chan = 0;
> +
> + ptp->ptp_clock_info.owner = THIS_MODULE;
> + snprintf(ptp->ptp_clock_info.name, 16, "%pm",
> + adapter->netdev->dev_addr);
> + ptp->ptp_clock_info.max_adj = LAN743X_PTP_MAX_FREQ_ADJ_IN_PPB;
> + ptp->ptp_clock_info.n_alarm = 0;
> + ptp->ptp_clock_info.n_ext_ts = 0;
> + ptp->ptp_clock_info.n_per_out = 0;
> + ptp->ptp_clock_info.n_pins = 0;
> + ptp->ptp_clock_info.pps = 1;
> + ptp->ptp_clock_info.pin_config = NULL;
> + ptp->ptp_clock_info.adjfreq = lan743x_ptpci_adjfreq;
> + ptp->ptp_clock_info.adjtime = lan743x_ptpci_adjtime;
> + ptp->ptp_clock_info.gettime64 = lan743x_ptpci_gettime64;
> + ptp->ptp_clock_info.getcrosststamp = NULL;
> + ptp->ptp_clock_info.settime64 = lan743x_ptpci_settime64;
> + ptp->ptp_clock_info.enable = lan743x_ptpci_enable;
> + ptp->ptp_clock_info.verify = NULL;
> +
> + ptp->ptp_clock = ptp_clock_register(&ptp->ptp_clock_info,
> + &adapter->pdev->dev);
> +
> + if (IS_ERR(ptp->ptp_clock)) {
> + netif_err(adapter, ifup, adapter->netdev,
> + "ptp_clock_register failed\n");
> + goto done;
> + }
> + ptp->flags |= PTP_FLAG_PTP_CLOCK_REGISTERED;
> + netif_info(adapter, ifup, adapter->netdev,
> + "successfully registered ptp clock\n");
> +#endif
> +
> + return 0;
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +done:
> + lan743x_ptp_close(adapter);
> + return ret;
> +#endif
> +}
> +
> +void lan743x_ptp_close(struct lan743x_adapter *adapter)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> + int index;
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> + if (ptp->flags & PTP_FLAG_PTP_CLOCK_REGISTERED) {
> + ptp_clock_unregister(ptp->ptp_clock);
> + ptp->ptp_clock = NULL;
> + ptp->flags &= ~PTP_FLAG_PTP_CLOCK_REGISTERED;
> + netif_info(adapter, drv, adapter->netdev,
> + "ptp clock unregister\n");
> + }
> +#endif
> +
> + if (ptp->flags & PTP_FLAG_ISR_ENABLED) {
> + lan743x_csr_write(adapter, PTP_INT_EN_CLR,
> + PTP_INT_BIT_TX_SWTS_ERR_ |
> + PTP_INT_BIT_TX_TS_);
> + lan743x_csr_write(adapter, INT_EN_CLR, INT_BIT_1588_);
> + tasklet_disable(&ptp->ptp_isr_bottom_half);
> + ptp->flags &= ~PTP_FLAG_ISR_ENABLED;
> + }
> +
> + /* clean up pending timestamp requests */
> + lan743x_ptp_tx_ts_complete(adapter);
> + spin_lock_bh(&ptp->tx_ts_lock);
> + for (index = 0;
> + index < LAN743X_PTP_NUMBER_OF_TX_TIMESTAMPS;
> + index++) {
> + struct sk_buff *skb = ptp->tx_ts_skb_queue[index];
> +
> + if (skb)
> + dev_kfree_skb(skb);
> + ptp->tx_ts_skb_queue[index] = NULL;
> + ptp->tx_ts_seconds_queue[index] = 0;
> + ptp->tx_ts_nseconds_queue[index] = 0;
> + }
> + ptp->tx_ts_skb_queue_size = 0;
> + ptp->tx_ts_queue_size = 0;
> + ptp->pending_tx_timestamps = 0;
> + spin_unlock_bh(&ptp->tx_ts_lock);
> +
> + lan743x_ptp_disable(adapter);
> +}
> +
> +void lan743x_ptp_set_sync_ts_insert(struct lan743x_adapter *adapter,
> + bool ts_insert_enable)
> +{
> + u32 ptp_tx_mod = lan743x_csr_read(adapter, PTP_TX_MOD);
> +
> + if (ts_insert_enable)
> + ptp_tx_mod |= PTP_TX_MOD_TX_PTP_SYNC_TS_INSERT_;
> + else
> + ptp_tx_mod &= ~PTP_TX_MOD_TX_PTP_SYNC_TS_INSERT_;
> +
> + lan743x_csr_write(adapter, PTP_TX_MOD, ptp_tx_mod);
> +}
> +
> +static bool lan743x_ptp_is_enabled(struct lan743x_adapter *adapter)
> +{
> + if (lan743x_csr_read(adapter, PTP_CMD_CTL) & PTP_CMD_CTL_PTP_ENABLE_)
> + return true;
> + return false;
> +}
> +
> +static void lan743x_ptp_wait_till_cmd_done(struct lan743x_adapter *adapter,
> + u32 bit_mask)
> +{
> + int timeout = 1000;
> + u32 data = 0;
> +
> + while (timeout &&
> + (data = (lan743x_csr_read(adapter, PTP_CMD_CTL) &
> + bit_mask))) {
> + usleep_range(1000, 20000);
> + timeout--;
> + }
> + if (data) {
> + netif_err(adapter, drv, adapter->netdev,
> + "timeout waiting for cmd to be done, cmd = 0x%08X\n",
> + bit_mask);
> + }
> +}
> +
> +static void lan743x_ptp_enable(struct lan743x_adapter *adapter)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> +
> + mutex_lock(&ptp->command_lock);
> +
> + if (lan743x_ptp_is_enabled(adapter)) {
> + netif_warn(adapter, drv, adapter->netdev,
> + "PTP already enabled\n");
> + goto done;
> + }
> + lan743x_csr_write(adapter, PTP_CMD_CTL, PTP_CMD_CTL_PTP_ENABLE_);
> +done:
> + mutex_unlock(&ptp->command_lock);
> +}
> +
> +static void lan743x_ptp_disable(struct lan743x_adapter *adapter)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> +
> + mutex_lock(&ptp->command_lock);
> + if (!lan743x_ptp_is_enabled(adapter)) {
> + netif_warn(adapter, drv, adapter->netdev,
> + "PTP already disabled\n");
> + goto done;
> + }
> + lan743x_csr_write(adapter, PTP_CMD_CTL, PTP_CMD_CTL_PTP_DISABLE_);
> + lan743x_ptp_wait_till_cmd_done(adapter, PTP_CMD_CTL_PTP_ENABLE_);
> +done:
> + mutex_unlock(&ptp->command_lock);
> +}
> +
> +static void lan743x_ptp_reset(struct lan743x_adapter *adapter)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> +
> + mutex_lock(&ptp->command_lock);
> +
> + if (lan743x_ptp_is_enabled(adapter)) {
> + netif_err(adapter, drv, adapter->netdev,
> + "Attempting reset while enabled\n");
> + goto done;
> + }
> +
> + lan743x_csr_write(adapter, PTP_CMD_CTL, PTP_CMD_CTL_PTP_RESET_);
> + lan743x_ptp_wait_till_cmd_done(adapter, PTP_CMD_CTL_PTP_RESET_);
> +done:
> + mutex_unlock(&ptp->command_lock);
> +}
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static int lan743x_ptp_reserve_event_ch(struct lan743x_adapter *adapter)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> + int result = -ENODEV;
> + int index = 0;
> +
> + mutex_lock(&ptp->command_lock);
> + for (index = 0; index < LAN743X_PTP_NUMBER_OF_EVENT_CHANNELS; index++) {
> + if (!(test_bit(index, &ptp->used_event_ch))) {
> + ptp->used_event_ch |= BIT(index);
> + result = index;
> + break;
> + }
> + }
> + mutex_unlock(&ptp->command_lock);
> + return result;
> +}
> +#endif /* CONFIG_PTP_1588_CLOCK */
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static void lan743x_ptp_release_event_ch(struct lan743x_adapter *adapter,
> + int event_channel)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> +
> + mutex_lock(&ptp->command_lock);
> + if (test_bit(event_channel, &ptp->used_event_ch)) {
> + ptp->used_event_ch &= ~BIT(event_channel);
> + } else {
> + netif_warn(adapter, drv, adapter->netdev,
> + "attempted release on a not used event_channel = %d\n",
> + event_channel);
> + }
> + mutex_unlock(&ptp->command_lock);
> +}
> +#endif /* CONFIG_PTP_1588_CLOCK */
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static void lan743x_ptp_clock_get(struct lan743x_adapter *adapter,
> + u32 *seconds, u32 *nano_seconds,
> + u32 *sub_nano_seconds)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> +
> + mutex_lock(&ptp->command_lock);
> +
> + lan743x_csr_write(adapter, PTP_CMD_CTL, PTP_CMD_CTL_PTP_CLOCK_READ_);
> + lan743x_ptp_wait_till_cmd_done(adapter, PTP_CMD_CTL_PTP_CLOCK_READ_);
> +
> + if (seconds)
> + (*seconds) = lan743x_csr_read(adapter, PTP_CLOCK_SEC);
> +
> + if (nano_seconds)
> + (*nano_seconds) = lan743x_csr_read(adapter, PTP_CLOCK_NS);
> +
> + if (sub_nano_seconds)
> + (*sub_nano_seconds) =
> + lan743x_csr_read(adapter, PTP_CLOCK_SUBNS);
> +
> + mutex_unlock(&ptp->command_lock);
> +}
> +#endif /* CONFIG_PTP_1588_CLOCK */
> +
> +static void lan743x_ptp_clock_set(struct lan743x_adapter *adapter,
> + u32 seconds, u32 nano_seconds,
> + u32 sub_nano_seconds)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> +
> + mutex_lock(&ptp->command_lock);
> +
> + lan743x_csr_write(adapter, PTP_CLOCK_SEC, seconds);
> + lan743x_csr_write(adapter, PTP_CLOCK_NS, nano_seconds);
> + lan743x_csr_write(adapter, PTP_CLOCK_SUBNS, sub_nano_seconds);
> +
> + lan743x_csr_write(adapter, PTP_CMD_CTL, PTP_CMD_CTL_PTP_CLOCK_LOAD_);
> + lan743x_ptp_wait_till_cmd_done(adapter, PTP_CMD_CTL_PTP_CLOCK_LOAD_);
> + mutex_unlock(&ptp->command_lock);
> +}
> +
> +#ifdef CONFIG_PTP_1588_CLOCK
> +static void lan743x_ptp_clock_step(struct lan743x_adapter *adapter,
> + s64 time_step_ns)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> + u64 abs_time_step_ns = 0;
> + u32 nano_seconds = 0;
> + s32 seconds = 0;
> +
> + if (time_step_ns > 15000000000LL) {
> + /* convert to clock set */
> + u32 nano_seconds = 0;
> + u32 seconds = 0;
> +
> + lan743x_ptp_clock_get(adapter, &seconds, &nano_seconds, NULL);
> + seconds += (time_step_ns / 1000000000LL);
Use the macro.
> + nano_seconds += (time_step_ns % 1000000000LL);
Actually, use div_u64_rem() to avoid the % operator.
> + if (nano_seconds >= 1000000000) {
How can this test be true?
> + seconds++;
> + nano_seconds -= 1000000000;
> + }
> + lan743x_ptp_clock_set(adapter, seconds, nano_seconds, 0);
> + return;
> + } else if (time_step_ns < -15000000000LL) {
> + /* convert to clock set */
> + u32 nano_seconds_step = 0;
> + u32 nano_seconds = 0;
> + u32 seconds = 0;
Ugh. Now you have these defined twice. Move them to the top, please.
> + time_step_ns = -time_step_ns;
> +
> + lan743x_ptp_clock_get(adapter, &seconds, &nano_seconds, NULL);
> + seconds -= (time_step_ns / 1000000000LL);
> + nano_seconds_step = (time_step_ns % 1000000000LL);
Use division macro for 64 bit.
> + if (nano_seconds < nano_seconds_step) {
> + seconds--;
> + nano_seconds += 1000000000;
> + }
> + nano_seconds -= nano_seconds_step;
> + lan743x_ptp_clock_set(adapter, seconds, nano_seconds, 0);
> + return;
> + }
> +
> + /* do clock step */
> +
> + if (time_step_ns >= 0) {
> + abs_time_step_ns = (u64)(time_step_ns);
> + seconds = (s32)(abs_time_step_ns / 1000000000);
> + nano_seconds = (u32)(abs_time_step_ns % 1000000000);
> + } else {
> + abs_time_step_ns = (u64)(-time_step_ns);
> + seconds = -((s32)(abs_time_step_ns / 1000000000));
> + nano_seconds = (u32)(abs_time_step_ns % 1000000000);
> + 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) {
> + mutex_lock(&ptp->command_lock);
> + if (seconds > 0) {
> + u32 adjustment_value = (u32)seconds;
> +
> + if (adjustment_value > 0xF)
> + adjustment_value = 0xF;
> + lan743x_csr_write(adapter, PTP_CLOCK_STEP_ADJ,
> + PTP_CLOCK_STEP_ADJ_DIR_ |
> + adjustment_value);
> + seconds -= ((s32)adjustment_value);
> + } else {
> + u32 adjustment_value = (u32)(-seconds);
> +
> + if (adjustment_value > 0xF)
> + adjustment_value = 0xF;
> + lan743x_csr_write(adapter, PTP_CLOCK_STEP_ADJ,
> + adjustment_value);
> + seconds += ((s32)adjustment_value);
> + }
> + lan743x_csr_write(adapter, PTP_CMD_CTL,
> + PTP_CMD_CTL_PTP_CLOCK_STEP_SEC_);
> + lan743x_ptp_wait_till_cmd_done(adapter,
> + PTP_CMD_CTL_PTP_CLOCK_STEP_SEC_);
> + mutex_unlock(&ptp->command_lock);
> + }
> + if (nano_seconds) {
> + mutex_lock(&ptp->command_lock);
> + lan743x_csr_write(adapter, PTP_CLOCK_STEP_ADJ,
> + PTP_CLOCK_STEP_ADJ_DIR_ |
> + (nano_seconds &
> + PTP_CLOCK_STEP_ADJ_VALUE_MASK_));
> + lan743x_csr_write(adapter, PTP_CMD_CTL,
> + PTP_CMD_CTL_PTP_CLK_STP_NSEC_);
> + lan743x_ptp_wait_till_cmd_done(adapter,
> + PTP_CMD_CTL_PTP_CLK_STP_NSEC_);
> + mutex_unlock(&ptp->command_lock);
> + }
> +}
> +#endif /* CONFIG_PTP_1588_CLOCK */
> +
> +bool lan743x_ptp_request_tx_timestamp(struct lan743x_adapter *adapter)
> +{
> + struct lan743x_ptp *ptp = &adapter->ptp;
> + bool result = false;
> +
> + spin_lock_bh(&ptp->tx_ts_lock);
> + if (ptp->pending_tx_timestamps < LAN743X_PTP_NUMBER_OF_TX_TIMESTAMPS) {
> + ptp->pending_tx_timestamps++;
> + result = true;/* request granted */
Avoid tail comments please.
> + }
> + spin_unlock_bh(&ptp->tx_ts_lock);
> + return result;
> +}
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH net-next v6 01/11] net: sched: use rcu for action cookie update
From: Cong Wang @ 2018-07-13 3:52 UTC (permalink / raw)
To: Vlad Buslov
Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
Jiri Pirko, Alexei Starovoitov, Daniel Borkmann,
Yevgeny Kliteynik, Jiri Pirko
In-Reply-To: <1530800673-12280-2-git-send-email-vladbu@mellanox.com>
On Thu, Jul 5, 2018 at 7:24 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>
> Implement functions to atomically update and free action cookie
> using rcu mechanism.
Without stating any reason..... Is this even a changelog?
>
> Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Dear Marcelo, how did it pass your review? See below:
> +static void tcf_set_action_cookie(struct tc_cookie __rcu **old_cookie,
> + struct tc_cookie *new_cookie)
> +{
> + struct tc_cookie *old;
> +
> + old = xchg(old_cookie, new_cookie);
This is an incorrect use of RCU, obviously should be rcu_assign_pointer()
here.
> @@ -65,10 +83,7 @@ static void free_tcf(struct tc_action *p)
> free_percpu(p->cpu_bstats);
> free_percpu(p->cpu_qstats);
>
> - if (p->act_cookie) {
> - kfree(p->act_cookie->data);
> - kfree(p->act_cookie);
> - }
> + tcf_set_action_cookie(&p->act_cookie, NULL);
So, this is called in free_tcf(), where the action is already
invisible from readers so it is ready to be freed.
The question is:
If the action itself is already ready to be freed, why do you
need RCU here? What could still read 'act->act_cookie'
while 'act' is already invisible?
Its last refcnt is already gone, the fast path RCU readers
are gone too given filters use rcu work already.
Standalone action dump? Again, the last refcnt is already
gone.
Marcelo, Vlad, Jiri, please explain.
Thanks!
^ permalink raw reply
* Re: [PATCH net-next v6 00/11] Modify action API for implementing lockless actions
From: Cong Wang @ 2018-07-13 3:54 UTC (permalink / raw)
To: David Miller
Cc: Vlad Buslov, Linux Kernel Network Developers, Jamal Hadi Salim,
Jiri Pirko, Alexei Starovoitov, Daniel Borkmann,
Yevgeny Kliteynik
In-Reply-To: <20180708.124325.344679298289898945.davem@davemloft.net>
On Sat, Jul 7, 2018 at 8:43 PM David Miller <davem@davemloft.net> wrote:
>
> From: Vlad Buslov <vladbu@mellanox.com>
> Date: Thu, 5 Jul 2018 17:24:22 +0300
>
> > Currently, all netlink protocol handlers for updating rules, actions and
> > qdiscs are protected with single global rtnl lock which removes any
> > possibility for parallelism. This patch set is a first step to remove
> > rtnl lock dependency from TC rules update path.
> ...
>
> I'll apply this for now, I reviewed it a few more times and I see
> where you are going with this.
Dear David,
I don't understand why you even believe the claim of lockless
updaters here, it at least should raise a red flag when you see any
kinda of this claim.
I know you don't trust me, how about thinking it in this way:
Why does RCU still require a lock for RCU writers? (Or at least
RCU recommends a lock, if anyone really wants to point out some
lockless algorithm here.)
or:
If writers could really go lockless as easily as Vlad claims, how could
even Paul E. McKenney never bring it into RCU?
Maybe Vlad is much cleverer than any of us here, and maybe he really
discovers a very brilliant algorithm to allow TC actions to be updated
locklessly, why not wait until he shows a proof (either code or a paper)?
Is there a rush? I don't see it.
In fact, I discussed this with Vlad a little bit at netdev TC workshop.
I never see any brilliant algorithm from him from his slides, and I was
told by him he used "copy and replace" to archive parallel updaters, I
told him that is basically how RCU works and RCU writers have to be
sync'ed with a lock (or at least recommended).
Also, to confirm my judgement, I checked this with Paul privately too.
Paul said you have to be extremely careful to go lockless, it is very hard
to be bug free for lockless, although he _never_ says it is impossible.
My _personal_ bet is that, lockless updates for TC filters or actions
are impossible unless there are more things hiding behind "copy and
replace", for example, some brilliant lockless algorithm. If lockless is
really impossible in this circumstance, then many of your efforts in
this patchset are vain, by the way.
I _do_ believe you can break RTNL down to per device, per filter or per
action, but no matter how small the locking scope is, there is still a lock.
With a lock, there is no need to make things friendly to lockless, like
making an integer increment inside an action to be atomic (your patch
02/11).
Please _do_ prove my personal judgement is wrong, by showing your
final code or a formal paper/article. I am very *happy* to be proved
to be wrong here, I am very open to change my mind here.
Vlad, we need your proof. Please prove I am wrong, seriously!!! :)
Thanks to anyone for proving me I am wrong just in case!!! :)
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox