* Re: [PATCH] bpf: return EOPNOTSUPP when JIT is needed and not possible
From: Daniel Borkmann @ 2021-12-09 23:03 UTC (permalink / raw)
To: Ido Schimmel, John Fastabend
Cc: Thadeu Lima de Souza Cascardo, bpf, netdev, ast, linux-kernel,
kuba
In-Reply-To: <YbJZoK+qBEiLAxxM@shredder>
On 12/9/21 8:31 PM, Ido Schimmel wrote:
> On Thu, Dec 09, 2021 at 11:05:18AM -0800, John Fastabend wrote:
>> Thadeu Lima de Souza Cascardo wrote:
>>> When a CBPF program is JITed and CONFIG_BPF_JIT_ALWAYS_ON is enabled, and
>>> the JIT fails, it would return ENOTSUPP, which is not a valid userspace
>>> error code. Instead, EOPNOTSUPP should be returned.
>>>
>>> Fixes: 290af86629b2 ("bpf: introduce BPF_JIT_ALWAYS_ON config")
>>> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
>>> ---
>>> kernel/bpf/core.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
>>> index de3e5bc6781f..5c89bae0d6f9 100644
>>> --- a/kernel/bpf/core.c
>>> +++ b/kernel/bpf/core.c
>>> @@ -1931,7 +1931,7 @@ struct bpf_prog *bpf_prog_select_runtime(struct bpf_prog *fp, int *err)
>>> fp = bpf_int_jit_compile(fp);
>>> bpf_prog_jit_attempt_done(fp);
>>> if (!fp->jited && jit_needed) {
>>> - *err = -ENOTSUPP;
>>> + *err = -EOPNOTSUPP;
>>> return fp;
>>> }
>>> } else {
>>
>> It seems BPF subsys returns ENOTSUPP in multiple places. This fixes one
>> paticular case and is user facing. Not sure we want to one-off fix them
>> here creating user facing changes over multiple kernel versions. On the
>> fence with this one curious to see what others think. Haven't apps
>> already adapted to the current convention or they don't care?
>
> Similar issue was discussed in the past. See:
> https://lore.kernel.org/netdev/20191204.125135.750458923752225025.davem@davemloft.net/
With regards to ENOTSUPP exposure, if the consensus is that we should fix all
occurences over to EOPNOTSUPP even if they've been exposed for quite some time
(Jakub?), we could give this patch a try maybe via bpf-next and see if anyone
complains.
Thadeu, I think you also need to fix up BPF selftests as test_verifier, to mention
one example (there are also bunch of others under tools/testing/selftests/), is
checking for ENOTSUPP specifically..
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH] Add Multiple TX/RX Queues Support for WWAN Network Device
From: Sergey Ryazanov @ 2021-12-09 23:11 UTC (permalink / raw)
To: xiayu.zhang
Cc: Loic Poulain, David Miller, Jakub Kicinski, Johannes Berg, netdev,
open list, haijun.liu, zhaoping.shu, hw.he, srv_heupstream
In-Reply-To: <20211208040414.151960-1-xiayu.zhang@mediatek.com>
On Wed, Dec 8, 2021 at 7:04 AM <xiayu.zhang@mediatek.com> wrote:
> This patch adds 2 callback functions get_num_tx_queues() and
> get_num_rx_queues() to let WWAN network device driver customize its own
> TX and RX queue numbers. It gives WWAN driver a chance to implement its
> own software strategies, such as TX Qos.
>
> Currently, if WWAN device driver creates default bearer interface when
> calling wwan_register_ops(), there will be only 1 TX queue and 1 RX queue
> for the WWAN network device. In this case, driver is not able to enlarge
> the queue numbers by calling netif_set_real_num_tx_queues() or
> netif_set_real_num_rx_queues() to take advantage of the network device's
> capability of supporting multiple TX/RX queues.
>
> As for additional interfaces of secondary bearers, if userspace service
> doesn't specify the num_tx_queues or num_rx_queues in netlink message or
> iproute2 command, there also will be only 1 TX queue and 1 RX queue for
> each additional interface. If userspace service specifies the num_tx_queues
> and num_rx_queues, however, these numbers could be not able to match the
> capabilities of network device.
>
> Besides, userspace service is hard to learn every WWAN network device's
> TX/RX queue numbers.
>
> In order to let WWAN driver determine the queue numbers, this patch adds
> below callback functions in wwan_ops:
> struct wwan_ops {
> unsigned int priv_size;
> ...
> unsigned int (*get_num_tx_queues)(unsigned int hint_num);
> unsigned int (*get_num_rx_queues)(unsigned int hint_num);
> };
>
> WWAN subsystem uses the input parameters num_tx_queues and num_rx_queues of
> wwan_rtnl_alloc() as hint values, and passes the 2 values to the two
> callback functions. WWAN device driver should determine the actual numbers
> of network device's TX and RX queues according to the hint value and
> device's capabilities.
As already stated by Jakub, it is hard to understand a new API
suitability without an API user. I will try to describe possible
issues with the proposed API as far as I understand its usage and
possible solutions. Correct me if I am wrong.
There are actually two tasks related to the queues number selection:
1) default queues number selection if the userspace provides no
information about a wishful number of queues;
2) rejecting the new netdev (bearer) creation if a requested number of
queues seems to be invalid.
Your proposal tries to solve both of these tasks with a single hook
that silently increases or decreases the requested number of queues.
This is creative, but seems contradictory to regular RTNL behavior.
RTNL usually selects a correct default value if no value was
requested, or performs what is requested, or explicitly rejects
requested configuration.
You could handle an invalid queues configuration in the .newlink
callback. This callback is even able to return a string error
representation via the extack argument.
As for the default queues number selection it seems better to
implement the RTNL .get_num_rx_queues callback in the WWAN core and
call optional driver specific callback through it. Something like
this:
static unsigned int wwan_rtnl_get_num_tx_queues(struct nlattr *tb[])
{
const char *devname = nla_data(tb[IFLA_PARENT_DEV_NAME]);
struct wwan_device *wwandev = wwan_dev_get_by_name(devname);
return wwandev && wwandev->ops && wwandev->ops->get_num_tx_queues ?
wwandev->ops->get_num_tx_queues() : 1;
}
static struct rtnl_link_ops wwan_rtnl_link_ops __read_mostly = {
...
.get_num_tx_queues = wwan_rtnl_get_num_tx_queues,
};
This way the default queues number selection will be implemented in a
less surprising way.
But to be able to implement this we need to modify the RTNL ops
.get_num_tx_queues/.get_num_rx_queues callback definitions to make
them able to accept the RTM_NEWLINK message attributes. This is not
difficult since the callbacks are implemented only by a few virtual
devices, but can be assumed too intrusive to implement a one feature
for a single subsystem.
> Signed-off-by: Xiayu Zhang <Xiayu.Zhang@mediatek.com>
> ---
> drivers/net/wwan/wwan_core.c | 25 ++++++++++++++++++++++++-
> include/linux/wwan.h | 6 ++++++
> 2 files changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wwan/wwan_core.c b/drivers/net/wwan/wwan_core.c
> index d293ab688044..00095c6987be 100644
> --- a/drivers/net/wwan/wwan_core.c
> +++ b/drivers/net/wwan/wwan_core.c
> @@ -823,6 +823,7 @@ static struct net_device *wwan_rtnl_alloc(struct nlattr *tb[],
> struct wwan_device *wwandev = wwan_dev_get_by_name(devname);
> struct net_device *dev;
> unsigned int priv_size;
> + unsigned int num_txqs, num_rxqs;
>
> if (IS_ERR(wwandev))
> return ERR_CAST(wwandev);
> @@ -833,9 +834,31 @@ static struct net_device *wwan_rtnl_alloc(struct nlattr *tb[],
> goto out;
> }
>
> + /* let wwan device driver determine TX queue number if it wants */
> + if (wwandev->ops->get_num_tx_queues) {
> + num_txqs = wwandev->ops->get_num_tx_queues(num_tx_queues);
> + if (num_txqs < 1 || num_txqs > 4096) {
> + dev = ERR_PTR(-EINVAL);
> + goto out;
> + }
> + } else {
> + num_txqs = num_tx_queues;
> + }
> +
> + /* let wwan device driver determine RX queue number if it wants */
> + if (wwandev->ops->get_num_rx_queues) {
> + num_rxqs = wwandev->ops->get_num_rx_queues(num_rx_queues);
> + if (num_rxqs < 1 || num_rxqs > 4096) {
> + dev = ERR_PTR(-EINVAL);
> + goto out;
> + }
> + } else {
> + num_rxqs = num_rx_queues;
> + }
> +
> priv_size = sizeof(struct wwan_netdev_priv) + wwandev->ops->priv_size;
> dev = alloc_netdev_mqs(priv_size, ifname, name_assign_type,
> - wwandev->ops->setup, num_tx_queues, num_rx_queues);
> + wwandev->ops->setup, num_txqs, num_rxqs);
>
> if (dev) {
> SET_NETDEV_DEV(dev, &wwandev->dev);
> diff --git a/include/linux/wwan.h b/include/linux/wwan.h
> index 9fac819f92e3..69c0af7ab6af 100644
> --- a/include/linux/wwan.h
> +++ b/include/linux/wwan.h
> @@ -156,6 +156,10 @@ static inline void *wwan_netdev_drvpriv(struct net_device *dev)
> * @setup: set up a new netdev
> * @newlink: register the new netdev
> * @dellink: remove the given netdev
> + * @get_num_tx_queues: determine number of transmit queues
> + * to create when creating a new device.
> + * @get_num_rx_queues: determine number of receive queues
> + * to create when creating a new device.
> */
> struct wwan_ops {
> unsigned int priv_size;
> @@ -164,6 +168,8 @@ struct wwan_ops {
> u32 if_id, struct netlink_ext_ack *extack);
> void (*dellink)(void *ctxt, struct net_device *dev,
> struct list_head *head);
> + unsigned int (*get_num_tx_queues)(unsigned int hint_num);
> + unsigned int (*get_num_rx_queues)(unsigned int hint_num);
> };
>
> int wwan_register_ops(struct device *parent, const struct wwan_ops *ops,
--
Sergey
^ permalink raw reply
* Re: [PATCH v5 0/5] fix statistics and payload issues for error
From: Stefan Mätje @ 2021-12-09 23:21 UTC (permalink / raw)
To: linux-can@vger.kernel.org, mailhol.vincent@wanadoo.fr,
mkl@pengutronix.de
Cc: linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-sunxi@lists.linux.dev,
socketcan@hartkopp.net, extja@kvaser.com
In-Reply-To: <20211207121531.42941-1-mailhol.vincent@wanadoo.fr>
Hello Vincent,
I now had some time to test the patches [1/5], [4/5] and [5/5] for esd_usb2.c
and they work as expected. You may add
For esd_usb2.c
Acked-by: Stefan Mätje <stefan.maetje@esd.eu>
Tested-by: Stefan Mätje <stefan.maetje@esd.eu>
Thanks for the work changing all the drivers.
Best regards,
Stefan Mätje
Am Dienstag, den 07.12.2021, 21:15 +0900 schrieb Vincent Mailhol:
> Important: this patch series depends on below patch:
> https://lore.kernel.org/linux-can/20211123111654.621610-1-mailhol.vincent@wanadoo.fr/T/#u
>
> There are some common errors which are made when updating the network
> statistics or processing the CAN payload:
>
> 1. Incrementing the "normal" stats when generating or sending a CAN
> error message frame. Error message frames are an abstraction of
> Socket CAN and do not exist on the wire. The first patch of this
> series fixes the RX stats for 22 different drivers, the second one
> fixes the TX stasts for the kvaser driver (N.B. only this driver is
> capable of sending error on the bus).
>
> 2. Copying the payload of RTR frames: RTR frames have no payload and
> the data buffer only contains garbage. The DLC/length should not be
> used to do a memory copy. The third patch of this series address
> this issue for 3 different drivers.
>
> 3. Counting the length of the Remote Transmission Frames (RTR). The
> length of an RTR frame is the length of the requested frame not the
> actual payload. In reality the payload of an RTR frame is always 0
> bytes long. The fourth patch of this series fixes the RX stats for
> 27 different drivers and the fifth one fixes the TX stats for 25
> different ones.
>
>
> * Changelog *
>
> v4 -> v5:
>
> * at91_can: netdev_tx_t at91_start_xmit: replace
> | dev->stats.tx_bytes = ...
> by
> | dev->stats.tx_bytes += ...
>
> v3 -> v4:
>
> * patch 2/5: kvaser: include the suggestion from Jimmy Assarsson so
> that we don't need to get the original CAN frame anymore to
> determine whether or not it was an error frame. patch 5/5 is
> rebased accordingly.
>
> * patch 5/5: kvaser: kvaser_usb_hydra_frame_to_cmd_std: remove
> unrelated change.
>
> * patch 5/5: slcan: better factorize code for the tx RTR frames
> (reorder the line instead of adding a new "if" branch).
>
>
> v2 -> v3:
>
> * Fix an issue in the fourth patch ("do not increase rx_bytes
> statistics for RTR frames"). In ucan_rx_can_msg() of the ucan
> driver, the changes in v2 made no sense. Reverted it to v1.
>
>
> v1 -> v2:
>
> * can_rx_offload_napi_poll: v1 used CAN_ERR_MASK instead of
> CAN_ERR_FLAG. Fixed the issue.
>
> * use correct vocabulary. The correct term to designate the Socket
> CAN specific error skb is "error message frames" not "error
> frames". "error frames" is used in the standard and has a
> different meaning.
>
> * better factorize code for the rx RTR frames. Most of the drivers
> already have a switch to check if the frame is a RTR. Moved the
> instruction to increase net_device_stats:rx_bytes inside the else
> branch of those switches whenever possible (for some drivers with
> some complex logic, putting and additional RTR check was easier).
>
> * add a patch which prevent drivers to copy the payload of RTR
> frames.
>
> * add a patch to cover the tx RTR frames (the fifth patch of
> v2). The tx RTR frames issue was supposedly covered by the
> can_get_echo_skb() function which returns the correct length for
> drivers to increase their stats. However, the reality is that most
> of the drivers do not check this value and instead use a local
> copy of the length/dlc.
>
> Vincent Mailhol (5):
> can: do not increase rx statistics when generating a CAN rx error
> message frame
> can: kvaser_usb: do not increase tx statistics when sending error
> message frames
> can: do not copy the payload of RTR frames
> can: do not increase rx_bytes statistics for RTR frames
> can: do not increase tx_bytes statistics for RTR frames
>
> drivers/net/can/at91_can.c | 18 ++---
> drivers/net/can/c_can/c_can.h | 1 -
> drivers/net/can/c_can/c_can_main.c | 16 ++---
> drivers/net/can/cc770/cc770.c | 16 ++---
> drivers/net/can/dev/dev.c | 4 --
> drivers/net/can/dev/rx-offload.c | 7 +-
> drivers/net/can/grcan.c | 6 +-
> drivers/net/can/ifi_canfd/ifi_canfd.c | 11 +--
> drivers/net/can/janz-ican3.c | 6 +-
> drivers/net/can/kvaser_pciefd.c | 16 ++---
> drivers/net/can/m_can/m_can.c | 13 +---
> drivers/net/can/mscan/mscan.c | 14 ++--
> drivers/net/can/pch_can.c | 33 ++++-----
> drivers/net/can/peak_canfd/peak_canfd.c | 14 ++--
> drivers/net/can/rcar/rcar_can.c | 22 +++---
> drivers/net/can/rcar/rcar_canfd.c | 13 +---
> drivers/net/can/sja1000/sja1000.c | 11 ++-
> drivers/net/can/slcan.c | 7 +-
> drivers/net/can/softing/softing_main.c | 8 +--
> drivers/net/can/spi/hi311x.c | 31 ++++----
> drivers/net/can/spi/mcp251x.c | 31 ++++----
> drivers/net/can/sun4i_can.c | 22 +++---
> drivers/net/can/usb/ems_usb.c | 14 ++--
> drivers/net/can/usb/esd_usb2.c | 13 ++--
> drivers/net/can/usb/etas_es58x/es58x_core.c | 7 --
> drivers/net/can/usb/gs_usb.c | 7 +-
> drivers/net/can/usb/kvaser_usb/kvaser_usb.h | 5 +-
> .../net/can/usb/kvaser_usb/kvaser_usb_core.c | 4 +-
> .../net/can/usb/kvaser_usb/kvaser_usb_hydra.c | 71 +++++++++----------
> .../net/can/usb/kvaser_usb/kvaser_usb_leaf.c | 20 ++----
> drivers/net/can/usb/mcba_usb.c | 23 +++---
> drivers/net/can/usb/peak_usb/pcan_usb.c | 9 ++-
> drivers/net/can/usb/peak_usb/pcan_usb_core.c | 20 +++---
> drivers/net/can/usb/peak_usb/pcan_usb_core.h | 1 -
> drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 11 ++-
> drivers/net/can/usb/peak_usb/pcan_usb_pro.c | 12 ++--
> drivers/net/can/usb/ucan.c | 17 +++--
> drivers/net/can/usb/usb_8dev.c | 17 ++---
> drivers/net/can/vcan.c | 7 +-
> drivers/net/can/vxcan.c | 2 +-
> drivers/net/can/xilinx_can.c | 19 +++--
> include/linux/can/skb.h | 5 +-
> 42 files changed, 254 insertions(+), 350 deletions(-)
>
>
> base-commit: 11f2c3c57f37befb1af6ccac0defacb813411d9d
^ permalink raw reply
* [PATCH v2 net-next 00/11] Replace DSA dp->priv with tagger-owned storage
From: Vladimir Oltean @ 2021-12-09 23:34 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Martin Kaistra, Kurt Kanzenbach, Ansuel Smith,
Tobias Waldekranz
Ansuel's recent work on qca8k register access over Ethernet:
https://patchwork.kernel.org/project/netdevbpf/cover/20211207145942.7444-1-ansuelsmth@gmail.com/
has triggered me to do something which I should've done for a longer
time:
https://patchwork.kernel.org/project/netdevbpf/patch/20211109095013.27829-7-martin.kaistra@linutronix.de/#24585521
which is to replace dp->priv with something that has less caveats.
The dp->priv was introduced when sja1105 needed to hold stateful
information in the tagging protocol driver. In that design, dp->priv
held memory allocated by the switch driver, because the tagging protocol
driver design was 100% stateless.
Some years have passed and others have started to feel the need for
stateful information kept by the tagger, as well as passing data back
and forth between the tagging protocol driver and the switch driver.
This isn't possible cleanly in DSA due to a circular dependency which
leads to broken module autoloading:
https://lore.kernel.org/netdev/20210908220834.d7gmtnwrorhharna@skbuf/
This patchset introduces a framework that resembles something normal,
which allows data to be passed from the tagging protocol driver (things
like switch management packets, which aren't intended for the network
stack) to the switch driver, while the tagging protocol still remains
more or less stateless. The overall design of the framework was
discussed with Ansuel too and it appears to be flexible enough to cover
the "register access over Ethernet" use case. Additionally, the existing
uses of dp->priv, which have mainly to do with PTP timestamping, have
also been migrated.
Changes in v2:
Fix transient build breakage in patch 5/11 due to a missing parenthesis,
https://patchwork.hopto.org/static/nipa/592567/12665213/build_clang/
and another transient build warning in patch 4/11 that for some reason
doesn't appear in my W=1 C=1 build.
https://patchwork.hopto.org/static/nipa/592567/12665209/build_clang/stderr
Vladimir Oltean (11):
net: dsa: introduce tagger-owned storage for private and shared data
net: dsa: tag_ocelot: convert to tagger-owned data
net: dsa: sja1105: let deferred packets time out when sent to ports
going down
net: dsa: sja1105: bring deferred xmit implementation in line with
ocelot-8021q
net: dsa: sja1105: remove hwts_tx_en from tagger data
net: dsa: sja1105: make dp->priv point directly to sja1105_tagger_data
net: dsa: sja1105: move ts_id from sja1105_tagger_data
net: dsa: tag_sja1105: convert to tagger-owned data
Revert "net: dsa: move sja1110_process_meta_tstamp inside the tagging
protocol driver"
net: dsa: tag_sja1105: split sja1105_tagger_data into private and
public sections
net: dsa: remove dp->priv
drivers/net/dsa/ocelot/felix.c | 64 ++-----
drivers/net/dsa/sja1105/sja1105.h | 6 +-
drivers/net/dsa/sja1105/sja1105_main.c | 121 ++++---------
drivers/net/dsa/sja1105/sja1105_ptp.c | 86 ++++++----
drivers/net/dsa/sja1105/sja1105_ptp.h | 24 +++
include/linux/dsa/ocelot.h | 12 +-
include/linux/dsa/sja1105.h | 61 +++----
include/net/dsa.h | 18 +-
net/dsa/dsa2.c | 73 +++++++-
net/dsa/dsa_priv.h | 1 +
net/dsa/switch.c | 14 ++
net/dsa/tag_ocelot_8021q.c | 73 +++++++-
net/dsa/tag_sja1105.c | 224 +++++++++++++++++--------
13 files changed, 483 insertions(+), 294 deletions(-)
--
2.25.1
^ permalink raw reply
* [PATCH v2 net-next 01/11] net: dsa: introduce tagger-owned storage for private and shared data
From: Vladimir Oltean @ 2021-12-09 23:34 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Martin Kaistra, Kurt Kanzenbach, Ansuel Smith,
Tobias Waldekranz
In-Reply-To: <20211209233447.336331-1-vladimir.oltean@nxp.com>
Ansuel is working on register access over Ethernet for the qca8k switch
family. This requires the qca8k tagging protocol driver to receive
frames which aren't intended for the network stack, but instead for the
qca8k switch driver itself.
The dp->priv is currently the prevailing method for passing data back
and forth between the tagging protocol driver and the switch driver.
However, this method is riddled with caveats.
The DSA design allows in principle for any switch driver to return any
protocol it desires in ->get_tag_protocol(). The dsa_loop driver can be
modified to do just that. But in the current design, the memory behind
dp->priv has to be allocated by the switch driver, so if the tagging
protocol is paired to an unexpected switch driver, we may end up in NULL
pointer dereferences inside the kernel, or worse (a switch driver may
allocate dp->priv according to the expectations of a different tagger).
The latter possibility is even more plausible considering that DSA
switches can dynamically change tagging protocols in certain cases
(dsa <-> edsa, ocelot <-> ocelot-8021q), and the current design lends
itself to mistakes that are all too easy to make.
This patch proposes that the tagging protocol driver should manage its
own memory, instead of relying on the switch driver to do so.
After analyzing the different in-tree needs, it can be observed that the
required tagger storage is per switch, therefore a ds->tagger_data
pointer is introduced. In principle, per-port storage could also be
introduced, although there is no need for it at the moment. Future
changes will replace the current usage of dp->priv with ds->tagger_data.
We define a "binding" event between the DSA switch tree and the tagging
protocol. During this binding event, the tagging protocol's ->connect()
method is called first, and this may allocate some memory for each
switch of the tree. Then a cross-chip notifier is emitted for the
switches within that tree, and they are given the opportunity to fix up
the tagger's memory (for example, they might set up some function
pointers that represent virtual methods for consuming packets).
Because the memory is owned by the tagger, there exists a ->disconnect()
method for the tagger (which is the place to free the resources), but
there doesn't exist a ->disconnect() method for the switch driver.
This is part of the design. The switch driver should make minimal use of
the public part of the tagger data, and only after type-checking it
using the supplied "proto" argument.
In the code there are in fact two binding events, one is the initial
event in dsa_switch_setup_tag_protocol(). At this stage, the cross chip
notifier chains aren't initialized, so we call each switch's connect()
method by hand. Then there is dsa_tree_bind_tag_proto() during
dsa_tree_change_tag_proto(), and here we have an old protocol and a new
one. We first connect to the new one before disconnecting from the old
one, to simplify error handling a bit and to ensure we remain in a valid
state at all times.
Co-developed-by: Ansuel Smith <ansuelsmth@gmail.com>
Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
include/net/dsa.h | 12 ++++++++
net/dsa/dsa2.c | 73 +++++++++++++++++++++++++++++++++++++++++++---
net/dsa/dsa_priv.h | 1 +
net/dsa/switch.c | 14 +++++++++
4 files changed, 96 insertions(+), 4 deletions(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index bdf308a5c55e..8b496c7e62ef 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -82,12 +82,15 @@ enum dsa_tag_protocol {
};
struct dsa_switch;
+struct dsa_switch_tree;
struct dsa_device_ops {
struct sk_buff *(*xmit)(struct sk_buff *skb, struct net_device *dev);
struct sk_buff *(*rcv)(struct sk_buff *skb, struct net_device *dev);
void (*flow_dissect)(const struct sk_buff *skb, __be16 *proto,
int *offset);
+ int (*connect)(struct dsa_switch_tree *dst);
+ void (*disconnect)(struct dsa_switch_tree *dst);
unsigned int needed_headroom;
unsigned int needed_tailroom;
const char *name;
@@ -337,6 +340,8 @@ struct dsa_switch {
*/
void *priv;
+ void *tagger_data;
+
/*
* Configuration data for this switch.
*/
@@ -689,6 +694,13 @@ struct dsa_switch_ops {
enum dsa_tag_protocol mprot);
int (*change_tag_protocol)(struct dsa_switch *ds, int port,
enum dsa_tag_protocol proto);
+ /*
+ * Method for switch drivers to connect to the tagging protocol driver
+ * in current use. The switch driver can provide handlers for certain
+ * types of packets for switch management.
+ */
+ int (*connect_tag_protocol)(struct dsa_switch *ds,
+ enum dsa_tag_protocol proto);
/* Optional switch-wide initialization and destruction methods */
int (*setup)(struct dsa_switch *ds);
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 8814fa0e44c8..cf6566168620 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -248,8 +248,12 @@ static struct dsa_switch_tree *dsa_tree_alloc(int index)
static void dsa_tree_free(struct dsa_switch_tree *dst)
{
- if (dst->tag_ops)
+ if (dst->tag_ops) {
+ if (dst->tag_ops->disconnect)
+ dst->tag_ops->disconnect(dst);
+
dsa_tag_driver_put(dst->tag_ops);
+ }
list_del(&dst->list);
kfree(dst);
}
@@ -822,7 +826,7 @@ static int dsa_switch_setup_tag_protocol(struct dsa_switch *ds)
int err;
if (tag_ops->proto == dst->default_proto)
- return 0;
+ goto connect;
dsa_switch_for_each_cpu_port(cpu_dp, ds) {
rtnl_lock();
@@ -836,6 +840,17 @@ static int dsa_switch_setup_tag_protocol(struct dsa_switch *ds)
}
}
+connect:
+ if (ds->ops->connect_tag_protocol) {
+ err = ds->ops->connect_tag_protocol(ds, tag_ops->proto);
+ if (err) {
+ dev_err(ds->dev,
+ "Unable to connect to tag protocol \"%s\": %pe\n",
+ tag_ops->name, ERR_PTR(err));
+ return err;
+ }
+ }
+
return 0;
}
@@ -1136,6 +1151,46 @@ static void dsa_tree_teardown(struct dsa_switch_tree *dst)
dst->setup = false;
}
+static int dsa_tree_bind_tag_proto(struct dsa_switch_tree *dst,
+ const struct dsa_device_ops *tag_ops)
+{
+ const struct dsa_device_ops *old_tag_ops = dst->tag_ops;
+ struct dsa_notifier_tag_proto_info info;
+ int err;
+
+ dst->tag_ops = tag_ops;
+
+ /* Notify the new tagger about the connection to this tree */
+ if (tag_ops->connect) {
+ err = tag_ops->connect(dst);
+ if (err)
+ goto out_revert;
+ }
+
+ /* Notify the switches from this tree about the connection
+ * to the new tagger
+ */
+ info.tag_ops = tag_ops;
+ err = dsa_tree_notify(dst, DSA_NOTIFIER_TAG_PROTO_CONNECT, &info);
+ if (err && err != -EOPNOTSUPP)
+ goto out_disconnect;
+
+ /* Notify the old tagger about the disconnection from this tree */
+ if (old_tag_ops->disconnect)
+ old_tag_ops->disconnect(dst);
+
+ return 0;
+
+out_disconnect:
+ /* Revert the new tagger's connection to this tree */
+ if (tag_ops->disconnect)
+ tag_ops->disconnect(dst);
+out_revert:
+ dst->tag_ops = old_tag_ops;
+
+ return err;
+}
+
/* Since the dsa/tagging sysfs device attribute is per master, the assumption
* is that all DSA switches within a tree share the same tagger, otherwise
* they would have formed disjoint trees (different "dsa,member" values).
@@ -1168,12 +1223,15 @@ int dsa_tree_change_tag_proto(struct dsa_switch_tree *dst,
goto out_unlock;
}
+ /* Notify the tag protocol change */
info.tag_ops = tag_ops;
err = dsa_tree_notify(dst, DSA_NOTIFIER_TAG_PROTO, &info);
if (err)
- goto out_unwind_tagger;
+ return err;
- dst->tag_ops = tag_ops;
+ err = dsa_tree_bind_tag_proto(dst, tag_ops);
+ if (err)
+ goto out_unwind_tagger;
rtnl_unlock();
@@ -1260,6 +1318,7 @@ static int dsa_port_parse_cpu(struct dsa_port *dp, struct net_device *master,
struct dsa_switch_tree *dst = ds->dst;
const struct dsa_device_ops *tag_ops;
enum dsa_tag_protocol default_proto;
+ int err;
/* Find out which protocol the switch would prefer. */
default_proto = dsa_get_tag_protocol(dp, master);
@@ -1307,6 +1366,12 @@ static int dsa_port_parse_cpu(struct dsa_port *dp, struct net_device *master,
*/
dsa_tag_driver_put(tag_ops);
} else {
+ if (tag_ops->connect) {
+ err = tag_ops->connect(dst);
+ if (err)
+ return err;
+ }
+
dst->tag_ops = tag_ops;
}
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 38ce5129a33d..0db2b26b0c83 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -37,6 +37,7 @@ enum {
DSA_NOTIFIER_VLAN_DEL,
DSA_NOTIFIER_MTU,
DSA_NOTIFIER_TAG_PROTO,
+ DSA_NOTIFIER_TAG_PROTO_CONNECT,
DSA_NOTIFIER_MRP_ADD,
DSA_NOTIFIER_MRP_DEL,
DSA_NOTIFIER_MRP_ADD_RING_ROLE,
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index 9c92edd96961..06948f536829 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -647,6 +647,17 @@ static int dsa_switch_change_tag_proto(struct dsa_switch *ds,
return 0;
}
+static int dsa_switch_connect_tag_proto(struct dsa_switch *ds,
+ struct dsa_notifier_tag_proto_info *info)
+{
+ const struct dsa_device_ops *tag_ops = info->tag_ops;
+
+ if (!ds->ops->connect_tag_protocol)
+ return -EOPNOTSUPP;
+
+ return ds->ops->connect_tag_protocol(ds, tag_ops->proto);
+}
+
static int dsa_switch_mrp_add(struct dsa_switch *ds,
struct dsa_notifier_mrp_info *info)
{
@@ -766,6 +777,9 @@ static int dsa_switch_event(struct notifier_block *nb,
case DSA_NOTIFIER_TAG_PROTO:
err = dsa_switch_change_tag_proto(ds, info);
break;
+ case DSA_NOTIFIER_TAG_PROTO_CONNECT:
+ err = dsa_switch_connect_tag_proto(ds, info);
+ break;
case DSA_NOTIFIER_MRP_ADD:
err = dsa_switch_mrp_add(ds, info);
break;
--
2.25.1
^ permalink raw reply related
* [PATCH v2 net-next 02/11] net: dsa: tag_ocelot: convert to tagger-owned data
From: Vladimir Oltean @ 2021-12-09 23:34 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Martin Kaistra, Kurt Kanzenbach, Ansuel Smith,
Tobias Waldekranz
In-Reply-To: <20211209233447.336331-1-vladimir.oltean@nxp.com>
The felix driver makes very light use of dp->priv, and the tagger is
effectively stateless. dp->priv is practically only needed to set up a
callback to perform deferred xmit of PTP and STP packets using the
ocelot-8021q tagging protocol (the main ocelot tagging protocol makes no
use of dp->priv, although this driver sets up dp->priv irrespective of
actual tagging protocol in use).
struct felix_port (what used to be pointed to by dp->priv) is removed
and replaced with a two-sided structure. The public side of this
structure, visible to the switch driver, is ocelot_8021q_tagger_data.
The private side is ocelot_8021q_tagger_private, and the latter
structure physically encapsulates the former. The public half of the
tagger data structure can be accessed through a helper of the same name
(ocelot_8021q_tagger_data) which also sanity-checks the protocol
currently in use by the switch. The public/private split was requested
by Andrew Lunn.
Suggested-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/dsa/ocelot/felix.c | 64 +++++++----------------------
include/linux/dsa/ocelot.h | 12 +++++-
net/dsa/tag_ocelot_8021q.c | 73 ++++++++++++++++++++++++++++++++--
3 files changed, 94 insertions(+), 55 deletions(-)
diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index a02fec33552d..f4fc403fbc1e 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -1155,38 +1155,22 @@ static void felix_port_deferred_xmit(struct kthread_work *work)
kfree(xmit_work);
}
-static int felix_port_setup_tagger_data(struct dsa_switch *ds, int port)
+static int felix_connect_tag_protocol(struct dsa_switch *ds,
+ enum dsa_tag_protocol proto)
{
- struct dsa_port *dp = dsa_to_port(ds, port);
- struct ocelot *ocelot = ds->priv;
- struct felix *felix = ocelot_to_felix(ocelot);
- struct felix_port *felix_port;
+ struct ocelot_8021q_tagger_data *tagger_data;
- if (!dsa_port_is_user(dp))
+ switch (proto) {
+ case DSA_TAG_PROTO_OCELOT_8021Q:
+ tagger_data = ocelot_8021q_tagger_data(ds);
+ tagger_data->xmit_work_fn = felix_port_deferred_xmit;
return 0;
-
- felix_port = kzalloc(sizeof(*felix_port), GFP_KERNEL);
- if (!felix_port)
- return -ENOMEM;
-
- felix_port->xmit_worker = felix->xmit_worker;
- felix_port->xmit_work_fn = felix_port_deferred_xmit;
-
- dp->priv = felix_port;
-
- return 0;
-}
-
-static void felix_port_teardown_tagger_data(struct dsa_switch *ds, int port)
-{
- struct dsa_port *dp = dsa_to_port(ds, port);
- struct felix_port *felix_port = dp->priv;
-
- if (!felix_port)
- return;
-
- dp->priv = NULL;
- kfree(felix_port);
+ case DSA_TAG_PROTO_OCELOT:
+ case DSA_TAG_PROTO_SEVILLE:
+ return 0;
+ default:
+ return -EPROTONOSUPPORT;
+ }
}
/* Hardware initialization done here so that we can allocate structures with
@@ -1217,12 +1201,6 @@ static int felix_setup(struct dsa_switch *ds)
}
}
- felix->xmit_worker = kthread_create_worker(0, "felix_xmit");
- if (IS_ERR(felix->xmit_worker)) {
- err = PTR_ERR(felix->xmit_worker);
- goto out_deinit_timestamp;
- }
-
for (port = 0; port < ds->num_ports; port++) {
if (dsa_is_unused_port(ds, port))
continue;
@@ -1233,14 +1211,6 @@ static int felix_setup(struct dsa_switch *ds)
* bits of vlan tag.
*/
felix_port_qos_map_init(ocelot, port);
-
- err = felix_port_setup_tagger_data(ds, port);
- if (err) {
- dev_err(ds->dev,
- "port %d failed to set up tagger data: %pe\n",
- port, ERR_PTR(err));
- goto out_deinit_ports;
- }
}
err = ocelot_devlink_sb_register(ocelot);
@@ -1268,13 +1238,9 @@ static int felix_setup(struct dsa_switch *ds)
if (dsa_is_unused_port(ds, port))
continue;
- felix_port_teardown_tagger_data(ds, port);
ocelot_deinit_port(ocelot, port);
}
- kthread_destroy_worker(felix->xmit_worker);
-
-out_deinit_timestamp:
ocelot_deinit_timestamp(ocelot);
ocelot_deinit(ocelot);
@@ -1303,12 +1269,9 @@ static void felix_teardown(struct dsa_switch *ds)
if (dsa_is_unused_port(ds, port))
continue;
- felix_port_teardown_tagger_data(ds, port);
ocelot_deinit_port(ocelot, port);
}
- kthread_destroy_worker(felix->xmit_worker);
-
ocelot_devlink_sb_unregister(ocelot);
ocelot_deinit_timestamp(ocelot);
ocelot_deinit(ocelot);
@@ -1648,6 +1611,7 @@ felix_mrp_del_ring_role(struct dsa_switch *ds, int port,
const struct dsa_switch_ops felix_switch_ops = {
.get_tag_protocol = felix_get_tag_protocol,
.change_tag_protocol = felix_change_tag_protocol,
+ .connect_tag_protocol = felix_connect_tag_protocol,
.setup = felix_setup,
.teardown = felix_teardown,
.set_ageing_time = felix_set_ageing_time,
diff --git a/include/linux/dsa/ocelot.h b/include/linux/dsa/ocelot.h
index 7ee708ad7df2..dca2969015d8 100644
--- a/include/linux/dsa/ocelot.h
+++ b/include/linux/dsa/ocelot.h
@@ -8,6 +8,7 @@
#include <linux/kthread.h>
#include <linux/packing.h>
#include <linux/skbuff.h>
+#include <net/dsa.h>
struct ocelot_skb_cb {
struct sk_buff *clone;
@@ -168,11 +169,18 @@ struct felix_deferred_xmit_work {
struct kthread_work work;
};
-struct felix_port {
+struct ocelot_8021q_tagger_data {
void (*xmit_work_fn)(struct kthread_work *work);
- struct kthread_worker *xmit_worker;
};
+static inline struct ocelot_8021q_tagger_data *
+ocelot_8021q_tagger_data(struct dsa_switch *ds)
+{
+ BUG_ON(ds->dst->tag_ops->proto != DSA_TAG_PROTO_OCELOT_8021Q);
+
+ return ds->tagger_data;
+}
+
static inline void ocelot_xfh_get_rew_val(void *extraction, u64 *rew_val)
{
packing(extraction, rew_val, 116, 85, OCELOT_TAG_LEN, UNPACK, 0);
diff --git a/net/dsa/tag_ocelot_8021q.c b/net/dsa/tag_ocelot_8021q.c
index a1919ea5e828..fe451f4de7ba 100644
--- a/net/dsa/tag_ocelot_8021q.c
+++ b/net/dsa/tag_ocelot_8021q.c
@@ -12,25 +12,39 @@
#include <linux/dsa/ocelot.h>
#include "dsa_priv.h"
+struct ocelot_8021q_tagger_private {
+ struct ocelot_8021q_tagger_data data; /* Must be first */
+ struct kthread_worker *xmit_worker;
+};
+
static struct sk_buff *ocelot_defer_xmit(struct dsa_port *dp,
struct sk_buff *skb)
{
+ struct ocelot_8021q_tagger_private *priv = dp->ds->tagger_data;
+ struct ocelot_8021q_tagger_data *data = &priv->data;
+ void (*xmit_work_fn)(struct kthread_work *work);
struct felix_deferred_xmit_work *xmit_work;
- struct felix_port *felix_port = dp->priv;
+ struct kthread_worker *xmit_worker;
+
+ xmit_work_fn = data->xmit_work_fn;
+ xmit_worker = priv->xmit_worker;
+
+ if (!xmit_work_fn || !xmit_worker)
+ return NULL;
xmit_work = kzalloc(sizeof(*xmit_work), GFP_ATOMIC);
if (!xmit_work)
return NULL;
/* Calls felix_port_deferred_xmit in felix.c */
- kthread_init_work(&xmit_work->work, felix_port->xmit_work_fn);
+ kthread_init_work(&xmit_work->work, xmit_work_fn);
/* Increase refcount so the kfree_skb in dsa_slave_xmit
* won't really free the packet.
*/
xmit_work->dp = dp;
xmit_work->skb = skb_get(skb);
- kthread_queue_work(felix_port->xmit_worker, &xmit_work->work);
+ kthread_queue_work(xmit_worker, &xmit_work->work);
return NULL;
}
@@ -67,11 +81,64 @@ static struct sk_buff *ocelot_rcv(struct sk_buff *skb,
return skb;
}
+static void ocelot_disconnect(struct dsa_switch_tree *dst)
+{
+ struct ocelot_8021q_tagger_private *priv;
+ struct dsa_port *dp;
+
+ list_for_each_entry(dp, &dst->ports, list) {
+ priv = dp->ds->tagger_data;
+
+ if (!priv)
+ continue;
+
+ if (priv->xmit_worker)
+ kthread_destroy_worker(priv->xmit_worker);
+
+ kfree(priv);
+ dp->ds->tagger_data = NULL;
+ }
+}
+
+static int ocelot_connect(struct dsa_switch_tree *dst)
+{
+ struct ocelot_8021q_tagger_private *priv;
+ struct dsa_port *dp;
+ int err;
+
+ list_for_each_entry(dp, &dst->ports, list) {
+ if (dp->ds->tagger_data)
+ continue;
+
+ priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+ if (!priv) {
+ err = -ENOMEM;
+ goto out;
+ }
+
+ priv->xmit_worker = kthread_create_worker(0, "felix_xmit");
+ if (IS_ERR(priv->xmit_worker)) {
+ err = PTR_ERR(priv->xmit_worker);
+ goto out;
+ }
+
+ dp->ds->tagger_data = priv;
+ }
+
+ return 0;
+
+out:
+ ocelot_disconnect(dst);
+ return err;
+}
+
static const struct dsa_device_ops ocelot_8021q_netdev_ops = {
.name = "ocelot-8021q",
.proto = DSA_TAG_PROTO_OCELOT_8021Q,
.xmit = ocelot_xmit,
.rcv = ocelot_rcv,
+ .connect = ocelot_connect,
+ .disconnect = ocelot_disconnect,
.needed_headroom = VLAN_HLEN,
.promisc_on_master = true,
};
--
2.25.1
^ permalink raw reply related
* [PATCH v2 net-next 03/11] net: dsa: sja1105: let deferred packets time out when sent to ports going down
From: Vladimir Oltean @ 2021-12-09 23:34 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Martin Kaistra, Kurt Kanzenbach, Ansuel Smith,
Tobias Waldekranz
In-Reply-To: <20211209233447.336331-1-vladimir.oltean@nxp.com>
This code is not necessary and complicates the conversion of this driver
to tagger-owned memory. If there is a PTP packet that is sent
concurrently with the port getting disabled, the deferred xmit mechanism
is robust enough to time out when it sees that it hasn't been delivered,
and recovers.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/dsa/sja1105/sja1105_main.c | 13 -------------
1 file changed, 13 deletions(-)
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index cefde41ce8d6..f7c88da377e4 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -2617,18 +2617,6 @@ static int sja1105_prechangeupper(struct dsa_switch *ds, int port,
return 0;
}
-static void sja1105_port_disable(struct dsa_switch *ds, int port)
-{
- struct sja1105_private *priv = ds->priv;
- struct sja1105_port *sp = &priv->ports[port];
-
- if (!dsa_is_user_port(ds, port))
- return;
-
- kthread_cancel_work_sync(&sp->xmit_work);
- skb_queue_purge(&sp->xmit_queue);
-}
-
static int sja1105_mgmt_xmit(struct dsa_switch *ds, int port, int slot,
struct sk_buff *skb, bool takets)
{
@@ -3215,7 +3203,6 @@ static const struct dsa_switch_ops sja1105_switch_ops = {
.get_ethtool_stats = sja1105_get_ethtool_stats,
.get_sset_count = sja1105_get_sset_count,
.get_ts_info = sja1105_get_ts_info,
- .port_disable = sja1105_port_disable,
.port_fdb_dump = sja1105_fdb_dump,
.port_fdb_add = sja1105_fdb_add,
.port_fdb_del = sja1105_fdb_del,
--
2.25.1
^ permalink raw reply related
* [PATCH v2 net-next 04/11] net: dsa: sja1105: bring deferred xmit implementation in line with ocelot-8021q
From: Vladimir Oltean @ 2021-12-09 23:34 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Martin Kaistra, Kurt Kanzenbach, Ansuel Smith,
Tobias Waldekranz
In-Reply-To: <20211209233447.336331-1-vladimir.oltean@nxp.com>
When the ocelot-8021q driver was converted to deferred xmit as part of
commit 8d5f7954b7c8 ("net: dsa: felix: break at first CPU port during
init and teardown"), the deferred implementation was deliberately made
subtly different from what sja1105 has.
The implementation differences lied on the following observations:
- There might be a race between these two lines in tag_sja1105.c:
skb_queue_tail(&sp->xmit_queue, skb_get(skb));
kthread_queue_work(sp->xmit_worker, &sp->xmit_work);
and the skb dequeue logic in sja1105_port_deferred_xmit(). For
example, the xmit_work might be already queued, however the work item
has just finished walking through the skb queue. Because we don't
check the return code from kthread_queue_work, we don't do anything if
the work item is already queued.
However, nobody will take that skb and send it, at least until the
next timestampable skb is sent. This creates additional (and
avoidable) TX timestamping latency.
To close that race, what the ocelot-8021q driver does is it doesn't
keep a single work item per port, and a skb timestamping queue, but
rather dynamically allocates a work item per packet.
- It is also unnecessary to have more than one kthread that does the
work. So delete the per-port kthread allocations and replace them with
a single kthread which is global to the switch.
This change brings the two implementations in line by applying those
observations to the sja1105 driver as well.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/dsa/sja1105/sja1105_main.c | 77 +++++++++++---------------
include/linux/dsa/sja1105.h | 11 +++-
net/dsa/tag_sja1105.c | 21 +++++--
3 files changed, 57 insertions(+), 52 deletions(-)
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index f7c88da377e4..c21822bb6834 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -2675,10 +2675,8 @@ static int sja1105_mgmt_xmit(struct dsa_switch *ds, int port, int slot,
return NETDEV_TX_OK;
}
-#define work_to_port(work) \
- container_of((work), struct sja1105_port, xmit_work)
-#define tagger_to_sja1105(t) \
- container_of((t), struct sja1105_private, tagger_data)
+#define work_to_xmit_work(w) \
+ container_of((w), struct sja1105_deferred_xmit_work, work)
/* Deferred work is unfortunately necessary because setting up the management
* route cannot be done from atomit context (SPI transfer takes a sleepable
@@ -2686,25 +2684,25 @@ static int sja1105_mgmt_xmit(struct dsa_switch *ds, int port, int slot,
*/
static void sja1105_port_deferred_xmit(struct kthread_work *work)
{
- struct sja1105_port *sp = work_to_port(work);
- struct sja1105_tagger_data *tagger_data = sp->data;
- struct sja1105_private *priv = tagger_to_sja1105(tagger_data);
- int port = sp - priv->ports;
- struct sk_buff *skb;
+ struct sja1105_deferred_xmit_work *xmit_work = work_to_xmit_work(work);
+ struct sk_buff *clone, *skb = xmit_work->skb;
+ struct dsa_switch *ds = xmit_work->dp->ds;
+ struct sja1105_private *priv = ds->priv;
+ int port = xmit_work->dp->index;
- while ((skb = skb_dequeue(&sp->xmit_queue)) != NULL) {
- struct sk_buff *clone = SJA1105_SKB_CB(skb)->clone;
+ clone = SJA1105_SKB_CB(skb)->clone;
- mutex_lock(&priv->mgmt_lock);
+ mutex_lock(&priv->mgmt_lock);
- sja1105_mgmt_xmit(priv->ds, port, 0, skb, !!clone);
+ sja1105_mgmt_xmit(ds, port, 0, skb, !!clone);
- /* The clone, if there, was made by dsa_skb_tx_timestamp */
- if (clone)
- sja1105_ptp_txtstamp_skb(priv->ds, port, clone);
+ /* The clone, if there, was made by dsa_skb_tx_timestamp */
+ if (clone)
+ sja1105_ptp_txtstamp_skb(ds, port, clone);
- mutex_unlock(&priv->mgmt_lock);
- }
+ mutex_unlock(&priv->mgmt_lock);
+
+ kfree(xmit_work);
}
/* The MAXAGE setting belongs to the L2 Forwarding Parameters table,
@@ -3009,54 +3007,43 @@ static int sja1105_port_bridge_flags(struct dsa_switch *ds, int port,
static void sja1105_teardown_ports(struct sja1105_private *priv)
{
- struct dsa_switch *ds = priv->ds;
- int port;
-
- for (port = 0; port < ds->num_ports; port++) {
- struct sja1105_port *sp = &priv->ports[port];
+ struct sja1105_tagger_data *tagger_data = &priv->tagger_data;
- if (sp->xmit_worker)
- kthread_destroy_worker(sp->xmit_worker);
- }
+ kthread_destroy_worker(tagger_data->xmit_worker);
}
static int sja1105_setup_ports(struct sja1105_private *priv)
{
struct sja1105_tagger_data *tagger_data = &priv->tagger_data;
struct dsa_switch *ds = priv->ds;
- int port, rc;
+ struct kthread_worker *worker;
+ int port;
+
+ worker = kthread_create_worker(0, "dsa%d:%d_xmit", ds->dst->index,
+ ds->index);
+ if (IS_ERR(worker)) {
+ dev_err(ds->dev,
+ "failed to create deferred xmit thread: %pe\n",
+ worker);
+ return PTR_ERR(worker);
+ }
+
+ tagger_data->xmit_worker = worker;
+ tagger_data->xmit_work_fn = sja1105_port_deferred_xmit;
/* Connections between dsa_port and sja1105_port */
for (port = 0; port < ds->num_ports; port++) {
struct sja1105_port *sp = &priv->ports[port];
struct dsa_port *dp = dsa_to_port(ds, port);
- struct kthread_worker *worker;
- struct net_device *slave;
if (!dsa_port_is_user(dp))
continue;
dp->priv = sp;
sp->data = tagger_data;
- slave = dp->slave;
- kthread_init_work(&sp->xmit_work, sja1105_port_deferred_xmit);
- worker = kthread_create_worker(0, "%s_xmit", slave->name);
- if (IS_ERR(worker)) {
- rc = PTR_ERR(worker);
- dev_err(ds->dev,
- "failed to create deferred xmit thread: %d\n",
- rc);
- goto out_destroy_workers;
- }
- sp->xmit_worker = worker;
- skb_queue_head_init(&sp->xmit_queue);
}
return 0;
-
-out_destroy_workers:
- sja1105_teardown_ports(priv);
- return rc;
}
/* The programming model for the SJA1105 switch is "all-at-once" via static
diff --git a/include/linux/dsa/sja1105.h b/include/linux/dsa/sja1105.h
index e6c78be40bde..acd9d2afccab 100644
--- a/include/linux/dsa/sja1105.h
+++ b/include/linux/dsa/sja1105.h
@@ -37,6 +37,12 @@
#define SJA1105_HWTS_RX_EN 0
+struct sja1105_deferred_xmit_work {
+ struct dsa_port *dp;
+ struct sk_buff *skb;
+ struct kthread_work work;
+};
+
/* Global tagger data: each struct sja1105_port has a reference to
* the structure defined in struct sja1105_private.
*/
@@ -52,6 +58,8 @@ struct sja1105_tagger_data {
* 2-step TX timestamps
*/
struct sk_buff_head skb_txtstamp_queue;
+ struct kthread_worker *xmit_worker;
+ void (*xmit_work_fn)(struct kthread_work *work);
};
struct sja1105_skb_cb {
@@ -65,9 +73,6 @@ struct sja1105_skb_cb {
((struct sja1105_skb_cb *)((skb)->cb))
struct sja1105_port {
- struct kthread_worker *xmit_worker;
- struct kthread_work xmit_work;
- struct sk_buff_head xmit_queue;
struct sja1105_tagger_data *data;
bool hwts_tx_en;
};
diff --git a/net/dsa/tag_sja1105.c b/net/dsa/tag_sja1105.c
index 6c293c2a3008..7008952b6c1d 100644
--- a/net/dsa/tag_sja1105.c
+++ b/net/dsa/tag_sja1105.c
@@ -125,16 +125,29 @@ static inline bool sja1105_is_meta_frame(const struct sk_buff *skb)
static struct sk_buff *sja1105_defer_xmit(struct dsa_port *dp,
struct sk_buff *skb)
{
+ void (*xmit_work_fn)(struct kthread_work *work);
+ struct sja1105_deferred_xmit_work *xmit_work;
struct sja1105_port *sp = dp->priv;
+ struct kthread_worker *xmit_worker;
- if (!dsa_port_is_sja1105(dp))
- return skb;
+ xmit_work_fn = sp->data->xmit_work_fn;
+ xmit_worker = sp->data->xmit_worker;
+
+ if (!xmit_work_fn || !xmit_worker)
+ return NULL;
+ xmit_work = kzalloc(sizeof(*xmit_work), GFP_ATOMIC);
+ if (!xmit_work)
+ return NULL;
+
+ kthread_init_work(&xmit_work->work, xmit_work_fn);
/* Increase refcount so the kfree_skb in dsa_slave_xmit
* won't really free the packet.
*/
- skb_queue_tail(&sp->xmit_queue, skb_get(skb));
- kthread_queue_work(sp->xmit_worker, &sp->xmit_work);
+ xmit_work->dp = dp;
+ xmit_work->skb = skb_get(skb);
+
+ kthread_queue_work(xmit_worker, &xmit_work->work);
return NULL;
}
--
2.25.1
^ permalink raw reply related
* [PATCH v2 net-next 05/11] net: dsa: sja1105: remove hwts_tx_en from tagger data
From: Vladimir Oltean @ 2021-12-09 23:34 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Martin Kaistra, Kurt Kanzenbach, Ansuel Smith,
Tobias Waldekranz
In-Reply-To: <20211209233447.336331-1-vladimir.oltean@nxp.com>
This tagger property is in fact not used at all by the tagger, only by
the switch driver. Therefore it makes sense to be moved to
sja1105_private.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/dsa/sja1105/sja1105.h | 1 +
drivers/net/dsa/sja1105/sja1105_ptp.c | 9 ++++-----
include/linux/dsa/sja1105.h | 1 -
3 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/net/dsa/sja1105/sja1105.h b/drivers/net/dsa/sja1105/sja1105.h
index 21dba16af097..b0612c763ec0 100644
--- a/drivers/net/dsa/sja1105/sja1105.h
+++ b/drivers/net/dsa/sja1105/sja1105.h
@@ -249,6 +249,7 @@ struct sja1105_private {
bool fixed_link[SJA1105_MAX_NUM_PORTS];
unsigned long ucast_egress_floods;
unsigned long bcast_egress_floods;
+ unsigned long hwts_tx_en;
const struct sja1105_info *info;
size_t max_xfer_len;
struct spi_device *spidev;
diff --git a/drivers/net/dsa/sja1105/sja1105_ptp.c b/drivers/net/dsa/sja1105/sja1105_ptp.c
index 54396992a919..b97bd4d948f5 100644
--- a/drivers/net/dsa/sja1105/sja1105_ptp.c
+++ b/drivers/net/dsa/sja1105/sja1105_ptp.c
@@ -98,10 +98,10 @@ int sja1105_hwtstamp_set(struct dsa_switch *ds, int port, struct ifreq *ifr)
switch (config.tx_type) {
case HWTSTAMP_TX_OFF:
- priv->ports[port].hwts_tx_en = false;
+ priv->hwts_tx_en &= ~BIT(port);
break;
case HWTSTAMP_TX_ON:
- priv->ports[port].hwts_tx_en = true;
+ priv->hwts_tx_en |= BIT(port);
break;
default:
return -ERANGE;
@@ -140,7 +140,7 @@ int sja1105_hwtstamp_get(struct dsa_switch *ds, int port, struct ifreq *ifr)
struct hwtstamp_config config;
config.flags = 0;
- if (priv->ports[port].hwts_tx_en)
+ if (priv->hwts_tx_en & BIT(port))
config.tx_type = HWTSTAMP_TX_ON;
else
config.tx_type = HWTSTAMP_TX_OFF;
@@ -486,10 +486,9 @@ void sja1110_txtstamp(struct dsa_switch *ds, int port, struct sk_buff *skb)
void sja1105_port_txtstamp(struct dsa_switch *ds, int port, struct sk_buff *skb)
{
struct sja1105_private *priv = ds->priv;
- struct sja1105_port *sp = &priv->ports[port];
struct sk_buff *clone;
- if (!sp->hwts_tx_en)
+ if (!(priv->hwts_tx_en & BIT(port)))
return;
clone = skb_clone_sk(skb);
diff --git a/include/linux/dsa/sja1105.h b/include/linux/dsa/sja1105.h
index acd9d2afccab..32a8a1344cf6 100644
--- a/include/linux/dsa/sja1105.h
+++ b/include/linux/dsa/sja1105.h
@@ -74,7 +74,6 @@ struct sja1105_skb_cb {
struct sja1105_port {
struct sja1105_tagger_data *data;
- bool hwts_tx_en;
};
/* Timestamps are in units of 8 ns clock ticks (equivalent to
--
2.25.1
^ permalink raw reply related
* [PATCH v2 net-next 06/11] net: dsa: sja1105: make dp->priv point directly to sja1105_tagger_data
From: Vladimir Oltean @ 2021-12-09 23:34 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Martin Kaistra, Kurt Kanzenbach, Ansuel Smith,
Tobias Waldekranz
In-Reply-To: <20211209233447.336331-1-vladimir.oltean@nxp.com>
The design of the sja1105 tagger dp->priv is that each port has a
separate struct sja1105_port, and the sp->data pointer points to a
common struct sja1105_tagger_data.
We have removed all per-port members accessible by the tagger, and now
only struct sja1105_tagger_data remains. Make dp->priv point directly to
this.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/dsa/sja1105/sja1105.h | 1 -
drivers/net/dsa/sja1105/sja1105_main.c | 15 ++------
drivers/net/dsa/sja1105/sja1105_ptp.c | 14 ++++----
include/linux/dsa/sja1105.h | 8 +----
net/dsa/tag_sja1105.c | 48 ++++++++++++++------------
5 files changed, 37 insertions(+), 49 deletions(-)
diff --git a/drivers/net/dsa/sja1105/sja1105.h b/drivers/net/dsa/sja1105/sja1105.h
index b0612c763ec0..6ef6fb4f30e6 100644
--- a/drivers/net/dsa/sja1105/sja1105.h
+++ b/drivers/net/dsa/sja1105/sja1105.h
@@ -257,7 +257,6 @@ struct sja1105_private {
u16 bridge_pvid[SJA1105_MAX_NUM_PORTS];
u16 tag_8021q_pvid[SJA1105_MAX_NUM_PORTS];
struct sja1105_flow_block flow_block;
- struct sja1105_port ports[SJA1105_MAX_NUM_PORTS];
/* Serializes transmission of management frames so that
* the switch doesn't confuse them with one another.
*/
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index c21822bb6834..e7a6cc7f681c 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -3017,7 +3017,7 @@ static int sja1105_setup_ports(struct sja1105_private *priv)
struct sja1105_tagger_data *tagger_data = &priv->tagger_data;
struct dsa_switch *ds = priv->ds;
struct kthread_worker *worker;
- int port;
+ struct dsa_port *dp;
worker = kthread_create_worker(0, "dsa%d:%d_xmit", ds->dst->index,
ds->index);
@@ -3031,17 +3031,8 @@ static int sja1105_setup_ports(struct sja1105_private *priv)
tagger_data->xmit_worker = worker;
tagger_data->xmit_work_fn = sja1105_port_deferred_xmit;
- /* Connections between dsa_port and sja1105_port */
- for (port = 0; port < ds->num_ports; port++) {
- struct sja1105_port *sp = &priv->ports[port];
- struct dsa_port *dp = dsa_to_port(ds, port);
-
- if (!dsa_port_is_user(dp))
- continue;
-
- dp->priv = sp;
- sp->data = tagger_data;
- }
+ dsa_switch_for_each_user_port(dp, ds)
+ dp->priv = tagger_data;
return 0;
}
diff --git a/drivers/net/dsa/sja1105/sja1105_ptp.c b/drivers/net/dsa/sja1105/sja1105_ptp.c
index b97bd4d948f5..9077067328c2 100644
--- a/drivers/net/dsa/sja1105/sja1105_ptp.c
+++ b/drivers/net/dsa/sja1105/sja1105_ptp.c
@@ -461,22 +461,24 @@ void sja1110_txtstamp(struct dsa_switch *ds, int port, struct sk_buff *skb)
{
struct sk_buff *clone = SJA1105_SKB_CB(skb)->clone;
struct sja1105_private *priv = ds->priv;
- struct sja1105_port *sp = &priv->ports[port];
+ struct sja1105_tagger_data *tagger_data;
u8 ts_id;
+ tagger_data = &priv->tagger_data;
+
skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
- spin_lock(&sp->data->meta_lock);
+ spin_lock(&tagger_data->meta_lock);
- ts_id = sp->data->ts_id;
+ ts_id = tagger_data->ts_id;
/* Deal automatically with 8-bit wraparound */
- sp->data->ts_id++;
+ tagger_data->ts_id++;
SJA1105_SKB_CB(clone)->ts_id = ts_id;
- spin_unlock(&sp->data->meta_lock);
+ spin_unlock(&tagger_data->meta_lock);
- skb_queue_tail(&sp->data->skb_txtstamp_queue, clone);
+ skb_queue_tail(&tagger_data->skb_txtstamp_queue, clone);
}
/* Called from dsa_skb_tx_timestamp. This callback is just to clone
diff --git a/include/linux/dsa/sja1105.h b/include/linux/dsa/sja1105.h
index 32a8a1344cf6..1dda9cce85d9 100644
--- a/include/linux/dsa/sja1105.h
+++ b/include/linux/dsa/sja1105.h
@@ -43,9 +43,7 @@ struct sja1105_deferred_xmit_work {
struct kthread_work work;
};
-/* Global tagger data: each struct sja1105_port has a reference to
- * the structure defined in struct sja1105_private.
- */
+/* Global tagger data */
struct sja1105_tagger_data {
struct sk_buff *stampable_skb;
/* Protects concurrent access to the meta state machine
@@ -72,10 +70,6 @@ struct sja1105_skb_cb {
#define SJA1105_SKB_CB(skb) \
((struct sja1105_skb_cb *)((skb)->cb))
-struct sja1105_port {
- struct sja1105_tagger_data *data;
-};
-
/* Timestamps are in units of 8 ns clock ticks (equivalent to
* a fixed 125 MHz clock).
*/
diff --git a/net/dsa/tag_sja1105.c b/net/dsa/tag_sja1105.c
index 7008952b6c1d..fc2af71b965c 100644
--- a/net/dsa/tag_sja1105.c
+++ b/net/dsa/tag_sja1105.c
@@ -125,13 +125,13 @@ static inline bool sja1105_is_meta_frame(const struct sk_buff *skb)
static struct sk_buff *sja1105_defer_xmit(struct dsa_port *dp,
struct sk_buff *skb)
{
+ struct sja1105_tagger_data *tagger_data = dp->priv;
void (*xmit_work_fn)(struct kthread_work *work);
struct sja1105_deferred_xmit_work *xmit_work;
- struct sja1105_port *sp = dp->priv;
struct kthread_worker *xmit_worker;
- xmit_work_fn = sp->data->xmit_work_fn;
- xmit_worker = sp->data->xmit_worker;
+ xmit_work_fn = tagger_data->xmit_work_fn;
+ xmit_worker = tagger_data->xmit_worker;
if (!xmit_work_fn || !xmit_worker)
return NULL;
@@ -368,32 +368,32 @@ static struct sk_buff
*/
if (is_link_local) {
struct dsa_port *dp = dsa_slave_to_port(skb->dev);
- struct sja1105_port *sp = dp->priv;
+ struct sja1105_tagger_data *tagger_data = dp->priv;
if (unlikely(!dsa_port_is_sja1105(dp)))
return skb;
- if (!test_bit(SJA1105_HWTS_RX_EN, &sp->data->state))
+ if (!test_bit(SJA1105_HWTS_RX_EN, &tagger_data->state))
/* Do normal processing. */
return skb;
- spin_lock(&sp->data->meta_lock);
+ spin_lock(&tagger_data->meta_lock);
/* Was this a link-local frame instead of the meta
* that we were expecting?
*/
- if (sp->data->stampable_skb) {
+ if (tagger_data->stampable_skb) {
dev_err_ratelimited(dp->ds->dev,
"Expected meta frame, is %12llx "
"in the DSA master multicast filter?\n",
SJA1105_META_DMAC);
- kfree_skb(sp->data->stampable_skb);
+ kfree_skb(tagger_data->stampable_skb);
}
/* Hold a reference to avoid dsa_switch_rcv
* from freeing the skb.
*/
- sp->data->stampable_skb = skb_get(skb);
- spin_unlock(&sp->data->meta_lock);
+ tagger_data->stampable_skb = skb_get(skb);
+ spin_unlock(&tagger_data->meta_lock);
/* Tell DSA we got nothing */
return NULL;
@@ -406,7 +406,7 @@ static struct sk_buff
*/
} else if (is_meta) {
struct dsa_port *dp = dsa_slave_to_port(skb->dev);
- struct sja1105_port *sp = dp->priv;
+ struct sja1105_tagger_data *tagger_data = dp->priv;
struct sk_buff *stampable_skb;
if (unlikely(!dsa_port_is_sja1105(dp)))
@@ -415,13 +415,13 @@ static struct sk_buff
/* Drop the meta frame if we're not in the right state
* to process it.
*/
- if (!test_bit(SJA1105_HWTS_RX_EN, &sp->data->state))
+ if (!test_bit(SJA1105_HWTS_RX_EN, &tagger_data->state))
return NULL;
- spin_lock(&sp->data->meta_lock);
+ spin_lock(&tagger_data->meta_lock);
- stampable_skb = sp->data->stampable_skb;
- sp->data->stampable_skb = NULL;
+ stampable_skb = tagger_data->stampable_skb;
+ tagger_data->stampable_skb = NULL;
/* Was this a meta frame instead of the link-local
* that we were expecting?
@@ -429,14 +429,14 @@ static struct sk_buff
if (!stampable_skb) {
dev_err_ratelimited(dp->ds->dev,
"Unexpected meta frame\n");
- spin_unlock(&sp->data->meta_lock);
+ spin_unlock(&tagger_data->meta_lock);
return NULL;
}
if (stampable_skb->dev != skb->dev) {
dev_err_ratelimited(dp->ds->dev,
"Meta frame on wrong port\n");
- spin_unlock(&sp->data->meta_lock);
+ spin_unlock(&tagger_data->meta_lock);
return NULL;
}
@@ -447,7 +447,7 @@ static struct sk_buff
skb = stampable_skb;
sja1105_transfer_meta(skb, meta);
- spin_unlock(&sp->data->meta_lock);
+ spin_unlock(&tagger_data->meta_lock);
}
return skb;
@@ -545,8 +545,8 @@ static void sja1110_process_meta_tstamp(struct dsa_switch *ds, int port,
{
struct sk_buff *skb, *skb_tmp, *skb_match = NULL;
struct dsa_port *dp = dsa_to_port(ds, port);
+ struct sja1105_tagger_data *tagger_data;
struct skb_shared_hwtstamps shwt = {0};
- struct sja1105_port *sp = dp->priv;
if (!dsa_port_is_sja1105(dp))
return;
@@ -555,19 +555,21 @@ static void sja1110_process_meta_tstamp(struct dsa_switch *ds, int port,
if (dir == SJA1110_META_TSTAMP_RX)
return;
- spin_lock(&sp->data->skb_txtstamp_queue.lock);
+ tagger_data = dp->priv;
- skb_queue_walk_safe(&sp->data->skb_txtstamp_queue, skb, skb_tmp) {
+ spin_lock(&tagger_data->skb_txtstamp_queue.lock);
+
+ skb_queue_walk_safe(&tagger_data->skb_txtstamp_queue, skb, skb_tmp) {
if (SJA1105_SKB_CB(skb)->ts_id != ts_id)
continue;
- __skb_unlink(skb, &sp->data->skb_txtstamp_queue);
+ __skb_unlink(skb, &tagger_data->skb_txtstamp_queue);
skb_match = skb;
break;
}
- spin_unlock(&sp->data->skb_txtstamp_queue.lock);
+ spin_unlock(&tagger_data->skb_txtstamp_queue.lock);
if (WARN_ON(!skb_match))
return;
--
2.25.1
^ permalink raw reply related
* [PATCH v2 net-next 07/11] net: dsa: sja1105: move ts_id from sja1105_tagger_data
From: Vladimir Oltean @ 2021-12-09 23:34 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Martin Kaistra, Kurt Kanzenbach, Ansuel Smith,
Tobias Waldekranz
In-Reply-To: <20211209233447.336331-1-vladimir.oltean@nxp.com>
The TX timestamp ID is incremented by the SJA1110 PTP timestamping
callback (->port_tx_timestamp) for every packet, when cloning it.
It isn't used by the tagger at all, even though it sits inside the
struct sja1105_tagger_data.
Also, serialization to this structure is currently done through
tagger_data->meta_lock, which is a cheap hack because the meta_lock
isn't used for anything else on SJA1110 (sja1105_rcv_meta_state_machine
isn't called).
This change moves ts_id from sja1105_tagger_data to sja1105_private and
introduces a dedicated spinlock for it, also in sja1105_private.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/dsa/sja1105/sja1105.h | 3 +++
drivers/net/dsa/sja1105/sja1105_main.c | 1 +
drivers/net/dsa/sja1105/sja1105_ptp.c | 8 ++++----
include/linux/dsa/sja1105.h | 1 -
4 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/net/dsa/sja1105/sja1105.h b/drivers/net/dsa/sja1105/sja1105.h
index 6ef6fb4f30e6..850a7d3e69bb 100644
--- a/drivers/net/dsa/sja1105/sja1105.h
+++ b/drivers/net/dsa/sja1105/sja1105.h
@@ -261,6 +261,9 @@ struct sja1105_private {
* the switch doesn't confuse them with one another.
*/
struct mutex mgmt_lock;
+ /* PTP two-step TX timestamp ID, and its serialization lock */
+ spinlock_t ts_id_lock;
+ u8 ts_id;
/* Serializes access to the dynamic config interface */
struct mutex dynamic_config_lock;
struct devlink_region **regions;
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index e7a6cc7f681c..880f28ea184f 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -3348,6 +3348,7 @@ static int sja1105_probe(struct spi_device *spi)
mutex_init(&priv->ptp_data.lock);
mutex_init(&priv->dynamic_config_lock);
mutex_init(&priv->mgmt_lock);
+ spin_lock_init(&priv->ts_id_lock);
rc = sja1105_parse_dt(priv);
if (rc < 0) {
diff --git a/drivers/net/dsa/sja1105/sja1105_ptp.c b/drivers/net/dsa/sja1105/sja1105_ptp.c
index 9077067328c2..0904ab10bd2f 100644
--- a/drivers/net/dsa/sja1105/sja1105_ptp.c
+++ b/drivers/net/dsa/sja1105/sja1105_ptp.c
@@ -468,15 +468,15 @@ void sja1110_txtstamp(struct dsa_switch *ds, int port, struct sk_buff *skb)
skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
- spin_lock(&tagger_data->meta_lock);
+ spin_lock(&priv->ts_id_lock);
- ts_id = tagger_data->ts_id;
+ ts_id = priv->ts_id;
/* Deal automatically with 8-bit wraparound */
- tagger_data->ts_id++;
+ priv->ts_id++;
SJA1105_SKB_CB(clone)->ts_id = ts_id;
- spin_unlock(&tagger_data->meta_lock);
+ spin_unlock(&priv->ts_id_lock);
skb_queue_tail(&tagger_data->skb_txtstamp_queue, clone);
}
diff --git a/include/linux/dsa/sja1105.h b/include/linux/dsa/sja1105.h
index 1dda9cce85d9..d8ee53085c09 100644
--- a/include/linux/dsa/sja1105.h
+++ b/include/linux/dsa/sja1105.h
@@ -51,7 +51,6 @@ struct sja1105_tagger_data {
*/
spinlock_t meta_lock;
unsigned long state;
- u8 ts_id;
/* Used on SJA1110 where meta frames are generated only for
* 2-step TX timestamps
*/
--
2.25.1
^ permalink raw reply related
* [PATCH v2 net-next 08/11] net: dsa: tag_sja1105: convert to tagger-owned data
From: Vladimir Oltean @ 2021-12-09 23:34 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Martin Kaistra, Kurt Kanzenbach, Ansuel Smith,
Tobias Waldekranz
In-Reply-To: <20211209233447.336331-1-vladimir.oltean@nxp.com>
Currently, struct sja1105_tagger_data is a part of struct
sja1105_private, and is used by the sja1105 driver to populate dp->priv.
With the movement towards tagger-owned storage, the sja1105 driver
should not be the owner of this memory.
This change implements the connection between the sja1105 switch driver
and its tagging protocol, which means that sja1105_tagger_data no longer
stays in dp->priv but in ds->tagger_data, and that the sja1105 driver
now only populates the sja1105_port_deferred_xmit callback pointer.
The kthread worker is now the responsibility of the tagger.
The sja1105 driver also alters the tagger's state some more, especially
with regard to the PTP RX timestamping state. This will be fixed up a
bit in further changes.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/dsa/sja1105/sja1105.h | 1 -
drivers/net/dsa/sja1105/sja1105_main.c | 54 +++++-----------
drivers/net/dsa/sja1105/sja1105_ptp.c | 35 +++++-----
include/linux/dsa/sja1105.h | 7 +-
net/dsa/tag_sja1105.c | 90 +++++++++++++++++++++-----
5 files changed, 110 insertions(+), 77 deletions(-)
diff --git a/drivers/net/dsa/sja1105/sja1105.h b/drivers/net/dsa/sja1105/sja1105.h
index 850a7d3e69bb..9ba2ec2b966d 100644
--- a/drivers/net/dsa/sja1105/sja1105.h
+++ b/drivers/net/dsa/sja1105/sja1105.h
@@ -272,7 +272,6 @@ struct sja1105_private {
struct mii_bus *mdio_base_tx;
struct mii_bus *mdio_pcs;
struct dw_xpcs *xpcs[SJA1105_MAX_NUM_PORTS];
- struct sja1105_tagger_data tagger_data;
struct sja1105_ptp_data ptp_data;
struct sja1105_tas_data tas_data;
};
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 880f28ea184f..4f5ea5d6a623 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -2705,6 +2705,21 @@ static void sja1105_port_deferred_xmit(struct kthread_work *work)
kfree(xmit_work);
}
+static int sja1105_connect_tag_protocol(struct dsa_switch *ds,
+ enum dsa_tag_protocol proto)
+{
+ struct sja1105_tagger_data *tagger_data;
+
+ switch (proto) {
+ case DSA_TAG_PROTO_SJA1105:
+ tagger_data = sja1105_tagger_data(ds);
+ tagger_data->xmit_work_fn = sja1105_port_deferred_xmit;
+ return 0;
+ default:
+ return -EPROTONOSUPPORT;
+ }
+}
+
/* The MAXAGE setting belongs to the L2 Forwarding Parameters table,
* which cannot be reconfigured at runtime. So a switch reset is required.
*/
@@ -3005,38 +3020,6 @@ static int sja1105_port_bridge_flags(struct dsa_switch *ds, int port,
return 0;
}
-static void sja1105_teardown_ports(struct sja1105_private *priv)
-{
- struct sja1105_tagger_data *tagger_data = &priv->tagger_data;
-
- kthread_destroy_worker(tagger_data->xmit_worker);
-}
-
-static int sja1105_setup_ports(struct sja1105_private *priv)
-{
- struct sja1105_tagger_data *tagger_data = &priv->tagger_data;
- struct dsa_switch *ds = priv->ds;
- struct kthread_worker *worker;
- struct dsa_port *dp;
-
- worker = kthread_create_worker(0, "dsa%d:%d_xmit", ds->dst->index,
- ds->index);
- if (IS_ERR(worker)) {
- dev_err(ds->dev,
- "failed to create deferred xmit thread: %pe\n",
- worker);
- return PTR_ERR(worker);
- }
-
- tagger_data->xmit_worker = worker;
- tagger_data->xmit_work_fn = sja1105_port_deferred_xmit;
-
- dsa_switch_for_each_user_port(dp, ds)
- dp->priv = tagger_data;
-
- return 0;
-}
-
/* The programming model for the SJA1105 switch is "all-at-once" via static
* configuration tables. Some of these can be dynamically modified at runtime,
* but not the xMII mode parameters table.
@@ -3082,10 +3065,6 @@ static int sja1105_setup(struct dsa_switch *ds)
}
}
- rc = sja1105_setup_ports(priv);
- if (rc)
- goto out_static_config_free;
-
sja1105_tas_setup(ds);
sja1105_flower_setup(ds);
@@ -3142,7 +3121,6 @@ static int sja1105_setup(struct dsa_switch *ds)
out_flower_teardown:
sja1105_flower_teardown(ds);
sja1105_tas_teardown(ds);
- sja1105_teardown_ports(priv);
out_static_config_free:
sja1105_static_config_free(&priv->static_config);
@@ -3162,12 +3140,12 @@ static void sja1105_teardown(struct dsa_switch *ds)
sja1105_ptp_clock_unregister(ds);
sja1105_flower_teardown(ds);
sja1105_tas_teardown(ds);
- sja1105_teardown_ports(priv);
sja1105_static_config_free(&priv->static_config);
}
static const struct dsa_switch_ops sja1105_switch_ops = {
.get_tag_protocol = sja1105_get_tag_protocol,
+ .connect_tag_protocol = sja1105_connect_tag_protocol,
.setup = sja1105_setup,
.teardown = sja1105_teardown,
.set_ageing_time = sja1105_set_ageing_time,
diff --git a/drivers/net/dsa/sja1105/sja1105_ptp.c b/drivers/net/dsa/sja1105/sja1105_ptp.c
index 0904ab10bd2f..b34e4674e217 100644
--- a/drivers/net/dsa/sja1105/sja1105_ptp.c
+++ b/drivers/net/dsa/sja1105/sja1105_ptp.c
@@ -58,13 +58,13 @@ enum sja1105_ptp_clk_mode {
#define ptp_data_to_sja1105(d) \
container_of((d), struct sja1105_private, ptp_data)
-/* Must be called only with priv->tagger_data.state bit
+/* Must be called only with the tagger_data->state bit
* SJA1105_HWTS_RX_EN cleared
*/
static int sja1105_change_rxtstamping(struct sja1105_private *priv,
+ struct sja1105_tagger_data *tagger_data,
bool on)
{
- struct sja1105_tagger_data *tagger_data = &priv->tagger_data;
struct sja1105_ptp_data *ptp_data = &priv->ptp_data;
struct sja1105_general_params_entry *general_params;
struct sja1105_table *table;
@@ -75,9 +75,9 @@ static int sja1105_change_rxtstamping(struct sja1105_private *priv,
general_params->send_meta0 = on;
/* Initialize the meta state machine to a known state */
- if (priv->tagger_data.stampable_skb) {
- kfree_skb(priv->tagger_data.stampable_skb);
- priv->tagger_data.stampable_skb = NULL;
+ if (tagger_data->stampable_skb) {
+ kfree_skb(tagger_data->stampable_skb);
+ tagger_data->stampable_skb = NULL;
}
ptp_cancel_worker_sync(ptp_data->clock);
skb_queue_purge(&tagger_data->skb_txtstamp_queue);
@@ -88,6 +88,7 @@ static int sja1105_change_rxtstamping(struct sja1105_private *priv,
int sja1105_hwtstamp_set(struct dsa_switch *ds, int port, struct ifreq *ifr)
{
+ struct sja1105_tagger_data *tagger_data = sja1105_tagger_data(ds);
struct sja1105_private *priv = ds->priv;
struct hwtstamp_config config;
bool rx_on;
@@ -116,17 +117,17 @@ int sja1105_hwtstamp_set(struct dsa_switch *ds, int port, struct ifreq *ifr)
break;
}
- if (rx_on != test_bit(SJA1105_HWTS_RX_EN, &priv->tagger_data.state)) {
- clear_bit(SJA1105_HWTS_RX_EN, &priv->tagger_data.state);
+ if (rx_on != test_bit(SJA1105_HWTS_RX_EN, &tagger_data->state)) {
+ clear_bit(SJA1105_HWTS_RX_EN, &tagger_data->state);
- rc = sja1105_change_rxtstamping(priv, rx_on);
+ rc = sja1105_change_rxtstamping(priv, tagger_data, rx_on);
if (rc < 0) {
dev_err(ds->dev,
"Failed to change RX timestamping: %d\n", rc);
return rc;
}
if (rx_on)
- set_bit(SJA1105_HWTS_RX_EN, &priv->tagger_data.state);
+ set_bit(SJA1105_HWTS_RX_EN, &tagger_data->state);
}
if (copy_to_user(ifr->ifr_data, &config, sizeof(config)))
@@ -136,6 +137,7 @@ int sja1105_hwtstamp_set(struct dsa_switch *ds, int port, struct ifreq *ifr)
int sja1105_hwtstamp_get(struct dsa_switch *ds, int port, struct ifreq *ifr)
{
+ struct sja1105_tagger_data *tagger_data = sja1105_tagger_data(ds);
struct sja1105_private *priv = ds->priv;
struct hwtstamp_config config;
@@ -144,7 +146,7 @@ int sja1105_hwtstamp_get(struct dsa_switch *ds, int port, struct ifreq *ifr)
config.tx_type = HWTSTAMP_TX_ON;
else
config.tx_type = HWTSTAMP_TX_OFF;
- if (test_bit(SJA1105_HWTS_RX_EN, &priv->tagger_data.state))
+ if (test_bit(SJA1105_HWTS_RX_EN, &tagger_data->state))
config.rx_filter = HWTSTAMP_FILTER_PTP_V2_L2_EVENT;
else
config.rx_filter = HWTSTAMP_FILTER_NONE;
@@ -417,10 +419,11 @@ static long sja1105_rxtstamp_work(struct ptp_clock_info *ptp)
bool sja1105_rxtstamp(struct dsa_switch *ds, int port, struct sk_buff *skb)
{
+ struct sja1105_tagger_data *tagger_data = sja1105_tagger_data(ds);
struct sja1105_private *priv = ds->priv;
struct sja1105_ptp_data *ptp_data = &priv->ptp_data;
- if (!test_bit(SJA1105_HWTS_RX_EN, &priv->tagger_data.state))
+ if (!test_bit(SJA1105_HWTS_RX_EN, &tagger_data->state))
return false;
/* We need to read the full PTP clock to reconstruct the Rx
@@ -459,13 +462,11 @@ bool sja1105_port_rxtstamp(struct dsa_switch *ds, int port,
*/
void sja1110_txtstamp(struct dsa_switch *ds, int port, struct sk_buff *skb)
{
+ struct sja1105_tagger_data *tagger_data = sja1105_tagger_data(ds);
struct sk_buff *clone = SJA1105_SKB_CB(skb)->clone;
struct sja1105_private *priv = ds->priv;
- struct sja1105_tagger_data *tagger_data;
u8 ts_id;
- tagger_data = &priv->tagger_data;
-
skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
spin_lock(&priv->ts_id_lock);
@@ -897,7 +898,6 @@ static struct ptp_pin_desc sja1105_ptp_pin = {
int sja1105_ptp_clock_register(struct dsa_switch *ds)
{
struct sja1105_private *priv = ds->priv;
- struct sja1105_tagger_data *tagger_data = &priv->tagger_data;
struct sja1105_ptp_data *ptp_data = &priv->ptp_data;
ptp_data->caps = (struct ptp_clock_info) {
@@ -919,9 +919,6 @@ int sja1105_ptp_clock_register(struct dsa_switch *ds)
/* Only used on SJA1105 */
skb_queue_head_init(&ptp_data->skb_rxtstamp_queue);
- /* Only used on SJA1110 */
- skb_queue_head_init(&tagger_data->skb_txtstamp_queue);
- spin_lock_init(&tagger_data->meta_lock);
ptp_data->clock = ptp_clock_register(&ptp_data->caps, ds->dev);
if (IS_ERR_OR_NULL(ptp_data->clock))
@@ -937,8 +934,8 @@ int sja1105_ptp_clock_register(struct dsa_switch *ds)
void sja1105_ptp_clock_unregister(struct dsa_switch *ds)
{
+ struct sja1105_tagger_data *tagger_data = sja1105_tagger_data(ds);
struct sja1105_private *priv = ds->priv;
- struct sja1105_tagger_data *tagger_data = &priv->tagger_data;
struct sja1105_ptp_data *ptp_data = &priv->ptp_data;
if (IS_ERR_OR_NULL(ptp_data->clock))
diff --git a/include/linux/dsa/sja1105.h b/include/linux/dsa/sja1105.h
index d8ee53085c09..9f7d42cbbc08 100644
--- a/include/linux/dsa/sja1105.h
+++ b/include/linux/dsa/sja1105.h
@@ -84,9 +84,12 @@ static inline s64 sja1105_ticks_to_ns(s64 ticks)
return ticks * SJA1105_TICK_NS;
}
-static inline bool dsa_port_is_sja1105(struct dsa_port *dp)
+static inline struct sja1105_tagger_data *
+sja1105_tagger_data(struct dsa_switch *ds)
{
- return true;
+ BUG_ON(ds->dst->tag_ops->proto != DSA_TAG_PROTO_SJA1105);
+
+ return ds->tagger_data;
}
#endif /* _NET_DSA_SJA1105_H */
diff --git a/net/dsa/tag_sja1105.c b/net/dsa/tag_sja1105.c
index fc2af71b965c..f3c1b31645f5 100644
--- a/net/dsa/tag_sja1105.c
+++ b/net/dsa/tag_sja1105.c
@@ -125,7 +125,7 @@ static inline bool sja1105_is_meta_frame(const struct sk_buff *skb)
static struct sk_buff *sja1105_defer_xmit(struct dsa_port *dp,
struct sk_buff *skb)
{
- struct sja1105_tagger_data *tagger_data = dp->priv;
+ struct sja1105_tagger_data *tagger_data = sja1105_tagger_data(dp->ds);
void (*xmit_work_fn)(struct kthread_work *work);
struct sja1105_deferred_xmit_work *xmit_work;
struct kthread_worker *xmit_worker;
@@ -368,10 +368,10 @@ static struct sk_buff
*/
if (is_link_local) {
struct dsa_port *dp = dsa_slave_to_port(skb->dev);
- struct sja1105_tagger_data *tagger_data = dp->priv;
+ struct sja1105_tagger_data *tagger_data;
+ struct dsa_switch *ds = dp->ds;
- if (unlikely(!dsa_port_is_sja1105(dp)))
- return skb;
+ tagger_data = sja1105_tagger_data(ds);
if (!test_bit(SJA1105_HWTS_RX_EN, &tagger_data->state))
/* Do normal processing. */
@@ -382,7 +382,7 @@ static struct sk_buff
* that we were expecting?
*/
if (tagger_data->stampable_skb) {
- dev_err_ratelimited(dp->ds->dev,
+ dev_err_ratelimited(ds->dev,
"Expected meta frame, is %12llx "
"in the DSA master multicast filter?\n",
SJA1105_META_DMAC);
@@ -406,11 +406,11 @@ static struct sk_buff
*/
} else if (is_meta) {
struct dsa_port *dp = dsa_slave_to_port(skb->dev);
- struct sja1105_tagger_data *tagger_data = dp->priv;
+ struct sja1105_tagger_data *tagger_data;
+ struct dsa_switch *ds = dp->ds;
struct sk_buff *stampable_skb;
- if (unlikely(!dsa_port_is_sja1105(dp)))
- return skb;
+ tagger_data = sja1105_tagger_data(ds);
/* Drop the meta frame if we're not in the right state
* to process it.
@@ -427,14 +427,14 @@ static struct sk_buff
* that we were expecting?
*/
if (!stampable_skb) {
- dev_err_ratelimited(dp->ds->dev,
+ dev_err_ratelimited(ds->dev,
"Unexpected meta frame\n");
spin_unlock(&tagger_data->meta_lock);
return NULL;
}
if (stampable_skb->dev != skb->dev) {
- dev_err_ratelimited(dp->ds->dev,
+ dev_err_ratelimited(ds->dev,
"Meta frame on wrong port\n");
spin_unlock(&tagger_data->meta_lock);
return NULL;
@@ -543,20 +543,14 @@ static void sja1110_process_meta_tstamp(struct dsa_switch *ds, int port,
u8 ts_id, enum sja1110_meta_tstamp dir,
u64 tstamp)
{
+ struct sja1105_tagger_data *tagger_data = sja1105_tagger_data(ds);
struct sk_buff *skb, *skb_tmp, *skb_match = NULL;
- struct dsa_port *dp = dsa_to_port(ds, port);
- struct sja1105_tagger_data *tagger_data;
struct skb_shared_hwtstamps shwt = {0};
- if (!dsa_port_is_sja1105(dp))
- return;
-
/* We don't care about RX timestamps on the CPU port */
if (dir == SJA1110_META_TSTAMP_RX)
return;
- tagger_data = dp->priv;
-
spin_lock(&tagger_data->skb_txtstamp_queue.lock);
skb_queue_walk_safe(&tagger_data->skb_txtstamp_queue, skb, skb_tmp) {
@@ -737,11 +731,71 @@ static void sja1110_flow_dissect(const struct sk_buff *skb, __be16 *proto,
*proto = ((__be16 *)skb->data)[(VLAN_HLEN / 2) - 1];
}
+static void sja1105_disconnect(struct dsa_switch_tree *dst)
+{
+ struct sja1105_tagger_data *tagger_data;
+ struct dsa_port *dp;
+
+ list_for_each_entry(dp, &dst->ports, list) {
+ tagger_data = dp->ds->tagger_data;
+
+ if (!tagger_data)
+ continue;
+
+ if (tagger_data->xmit_worker)
+ kthread_destroy_worker(tagger_data->xmit_worker);
+
+ kfree(tagger_data);
+ dp->ds->tagger_data = NULL;
+ }
+}
+
+static int sja1105_connect(struct dsa_switch_tree *dst)
+{
+ struct sja1105_tagger_data *tagger_data;
+ struct kthread_worker *xmit_worker;
+ struct dsa_port *dp;
+ int err;
+
+ list_for_each_entry(dp, &dst->ports, list) {
+ if (dp->ds->tagger_data)
+ continue;
+
+ tagger_data = kzalloc(sizeof(*tagger_data), GFP_KERNEL);
+ if (!tagger_data) {
+ err = -ENOMEM;
+ goto out;
+ }
+
+ /* Only used on SJA1110 */
+ skb_queue_head_init(&tagger_data->skb_txtstamp_queue);
+ spin_lock_init(&tagger_data->meta_lock);
+
+ xmit_worker = kthread_create_worker(0, "dsa%d:%d_xmit",
+ dst->index, dp->ds->index);
+ if (IS_ERR(xmit_worker)) {
+ err = PTR_ERR(xmit_worker);
+ goto out;
+ }
+
+ tagger_data->xmit_worker = xmit_worker;
+ dp->ds->tagger_data = tagger_data;
+ }
+
+ return 0;
+
+out:
+ sja1105_disconnect(dst);
+ return err;
+}
+
static const struct dsa_device_ops sja1105_netdev_ops = {
.name = "sja1105",
.proto = DSA_TAG_PROTO_SJA1105,
.xmit = sja1105_xmit,
.rcv = sja1105_rcv,
+ .connect = sja1105_connect,
+ .disconnect = sja1105_disconnect,
.needed_headroom = VLAN_HLEN,
.flow_dissect = sja1105_flow_dissect,
.promisc_on_master = true,
@@ -755,6 +809,8 @@ static const struct dsa_device_ops sja1110_netdev_ops = {
.proto = DSA_TAG_PROTO_SJA1110,
.xmit = sja1110_xmit,
.rcv = sja1110_rcv,
+ .connect = sja1105_connect,
+ .disconnect = sja1105_disconnect,
.flow_dissect = sja1110_flow_dissect,
.needed_headroom = SJA1110_HEADER_LEN + VLAN_HLEN,
.needed_tailroom = SJA1110_RX_TRAILER_LEN + SJA1110_MAX_PADDING_LEN,
--
2.25.1
^ permalink raw reply related
* [PATCH v2 net-next 09/11] Revert "net: dsa: move sja1110_process_meta_tstamp inside the tagging protocol driver"
From: Vladimir Oltean @ 2021-12-09 23:34 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Martin Kaistra, Kurt Kanzenbach, Ansuel Smith,
Tobias Waldekranz
In-Reply-To: <20211209233447.336331-1-vladimir.oltean@nxp.com>
This reverts commit 6d709cadfde68dbd12bef12fcced6222226dcb06.
The above change was done to avoid calling symbols exported by the
switch driver from the tagging protocol driver.
With the tagger-owned storage model, we have a new option on our hands,
and that is for the switch driver to provide a data consumer handler in
the form of a function pointer inside the ->connect_tag_protocol()
method. Having a function pointer avoids the problems of the exported
symbols approach.
By creating a handler for metadata frames holding TX timestamps on
SJA1110, we are able to eliminate an skb queue from the tagger data, and
replace it with a simple, and stateless, function pointer. This skb
queue is now handled exclusively by sja1105_ptp.c, which makes the code
easier to follow, as it used to be before the reverted patch.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/dsa/sja1105/sja1105_main.c | 1 +
drivers/net/dsa/sja1105/sja1105_ptp.c | 44 ++++++++++++++++++++---
drivers/net/dsa/sja1105/sja1105_ptp.h | 24 +++++++++++++
include/linux/dsa/sja1105.h | 26 ++++----------
net/dsa/tag_sja1105.c | 50 ++++----------------------
5 files changed, 78 insertions(+), 67 deletions(-)
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index 4f5ea5d6a623..9171fbea588c 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -2714,6 +2714,7 @@ static int sja1105_connect_tag_protocol(struct dsa_switch *ds,
case DSA_TAG_PROTO_SJA1105:
tagger_data = sja1105_tagger_data(ds);
tagger_data->xmit_work_fn = sja1105_port_deferred_xmit;
+ tagger_data->meta_tstamp_handler = sja1110_process_meta_tstamp;
return 0;
default:
return -EPROTONOSUPPORT;
diff --git a/drivers/net/dsa/sja1105/sja1105_ptp.c b/drivers/net/dsa/sja1105/sja1105_ptp.c
index b34e4674e217..a9f7e4ae0bb2 100644
--- a/drivers/net/dsa/sja1105/sja1105_ptp.c
+++ b/drivers/net/dsa/sja1105/sja1105_ptp.c
@@ -80,7 +80,7 @@ static int sja1105_change_rxtstamping(struct sja1105_private *priv,
tagger_data->stampable_skb = NULL;
}
ptp_cancel_worker_sync(ptp_data->clock);
- skb_queue_purge(&tagger_data->skb_txtstamp_queue);
+ skb_queue_purge(&ptp_data->skb_txtstamp_queue);
skb_queue_purge(&ptp_data->skb_rxtstamp_queue);
return sja1105_static_config_reload(priv, SJA1105_RX_HWTSTAMPING);
@@ -456,15 +456,48 @@ bool sja1105_port_rxtstamp(struct dsa_switch *ds, int port,
return priv->info->rxtstamp(ds, port, skb);
}
+void sja1110_process_meta_tstamp(struct dsa_switch *ds, int port, u8 ts_id,
+ enum sja1110_meta_tstamp dir, u64 tstamp)
+{
+ struct sja1105_private *priv = ds->priv;
+ struct sja1105_ptp_data *ptp_data = &priv->ptp_data;
+ struct sk_buff *skb, *skb_tmp, *skb_match = NULL;
+ struct skb_shared_hwtstamps shwt = {0};
+
+ /* We don't care about RX timestamps on the CPU port */
+ if (dir == SJA1110_META_TSTAMP_RX)
+ return;
+
+ spin_lock(&ptp_data->skb_txtstamp_queue.lock);
+
+ skb_queue_walk_safe(&ptp_data->skb_txtstamp_queue, skb, skb_tmp) {
+ if (SJA1105_SKB_CB(skb)->ts_id != ts_id)
+ continue;
+
+ __skb_unlink(skb, &ptp_data->skb_txtstamp_queue);
+ skb_match = skb;
+
+ break;
+ }
+
+ spin_unlock(&ptp_data->skb_txtstamp_queue.lock);
+
+ if (WARN_ON(!skb_match))
+ return;
+
+ shwt.hwtstamp = ns_to_ktime(sja1105_ticks_to_ns(tstamp));
+ skb_complete_tx_timestamp(skb_match, &shwt);
+}
+
/* In addition to cloning the skb which is done by the common
* sja1105_port_txtstamp, we need to generate a timestamp ID and save the
* packet to the TX timestamping queue.
*/
void sja1110_txtstamp(struct dsa_switch *ds, int port, struct sk_buff *skb)
{
- struct sja1105_tagger_data *tagger_data = sja1105_tagger_data(ds);
struct sk_buff *clone = SJA1105_SKB_CB(skb)->clone;
struct sja1105_private *priv = ds->priv;
+ struct sja1105_ptp_data *ptp_data = &priv->ptp_data;
u8 ts_id;
skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
@@ -479,7 +512,7 @@ void sja1110_txtstamp(struct dsa_switch *ds, int port, struct sk_buff *skb)
spin_unlock(&priv->ts_id_lock);
- skb_queue_tail(&tagger_data->skb_txtstamp_queue, clone);
+ skb_queue_tail(&ptp_data->skb_txtstamp_queue, clone);
}
/* Called from dsa_skb_tx_timestamp. This callback is just to clone
@@ -919,6 +952,8 @@ int sja1105_ptp_clock_register(struct dsa_switch *ds)
/* Only used on SJA1105 */
skb_queue_head_init(&ptp_data->skb_rxtstamp_queue);
+ /* Only used on SJA1110 */
+ skb_queue_head_init(&ptp_data->skb_txtstamp_queue);
ptp_data->clock = ptp_clock_register(&ptp_data->caps, ds->dev);
if (IS_ERR_OR_NULL(ptp_data->clock))
@@ -934,7 +969,6 @@ int sja1105_ptp_clock_register(struct dsa_switch *ds)
void sja1105_ptp_clock_unregister(struct dsa_switch *ds)
{
- struct sja1105_tagger_data *tagger_data = sja1105_tagger_data(ds);
struct sja1105_private *priv = ds->priv;
struct sja1105_ptp_data *ptp_data = &priv->ptp_data;
@@ -943,7 +977,7 @@ void sja1105_ptp_clock_unregister(struct dsa_switch *ds)
del_timer_sync(&ptp_data->extts_timer);
ptp_cancel_worker_sync(ptp_data->clock);
- skb_queue_purge(&tagger_data->skb_txtstamp_queue);
+ skb_queue_purge(&ptp_data->skb_txtstamp_queue);
skb_queue_purge(&ptp_data->skb_rxtstamp_queue);
ptp_clock_unregister(ptp_data->clock);
ptp_data->clock = NULL;
diff --git a/drivers/net/dsa/sja1105/sja1105_ptp.h b/drivers/net/dsa/sja1105/sja1105_ptp.h
index 3ae6b9fdd492..416461ee95d2 100644
--- a/drivers/net/dsa/sja1105/sja1105_ptp.h
+++ b/drivers/net/dsa/sja1105/sja1105_ptp.h
@@ -8,6 +8,21 @@
#if IS_ENABLED(CONFIG_NET_DSA_SJA1105_PTP)
+/* Timestamps are in units of 8 ns clock ticks (equivalent to
+ * a fixed 125 MHz clock).
+ */
+#define SJA1105_TICK_NS 8
+
+static inline s64 ns_to_sja1105_ticks(s64 ns)
+{
+ return ns / SJA1105_TICK_NS;
+}
+
+static inline s64 sja1105_ticks_to_ns(s64 ticks)
+{
+ return ticks * SJA1105_TICK_NS;
+}
+
/* Calculate the first base_time in the future that satisfies this
* relationship:
*
@@ -62,6 +77,10 @@ struct sja1105_ptp_data {
struct timer_list extts_timer;
/* Used only on SJA1105 to reconstruct partial timestamps */
struct sk_buff_head skb_rxtstamp_queue;
+ /* Used on SJA1110 where meta frames are generated only for
+ * 2-step TX timestamps
+ */
+ struct sk_buff_head skb_txtstamp_queue;
struct ptp_clock_info caps;
struct ptp_clock *clock;
struct sja1105_ptp_cmd cmd;
@@ -112,6 +131,9 @@ bool sja1105_rxtstamp(struct dsa_switch *ds, int port, struct sk_buff *skb);
bool sja1110_rxtstamp(struct dsa_switch *ds, int port, struct sk_buff *skb);
void sja1110_txtstamp(struct dsa_switch *ds, int port, struct sk_buff *skb);
+void sja1110_process_meta_tstamp(struct dsa_switch *ds, int port, u8 ts_id,
+ enum sja1110_meta_tstamp dir, u64 tstamp);
+
#else
struct sja1105_ptp_cmd;
@@ -178,6 +200,8 @@ static inline int sja1105_ptp_commit(struct dsa_switch *ds,
#define sja1110_rxtstamp NULL
#define sja1110_txtstamp NULL
+#define sja1110_process_meta_tstamp NULL
+
#endif /* IS_ENABLED(CONFIG_NET_DSA_SJA1105_PTP) */
#endif /* _SJA1105_PTP_H */
diff --git a/include/linux/dsa/sja1105.h b/include/linux/dsa/sja1105.h
index 9f7d42cbbc08..d216211b64f8 100644
--- a/include/linux/dsa/sja1105.h
+++ b/include/linux/dsa/sja1105.h
@@ -37,6 +37,11 @@
#define SJA1105_HWTS_RX_EN 0
+enum sja1110_meta_tstamp {
+ SJA1110_META_TSTAMP_TX = 0,
+ SJA1110_META_TSTAMP_RX = 1,
+};
+
struct sja1105_deferred_xmit_work {
struct dsa_port *dp;
struct sk_buff *skb;
@@ -51,12 +56,10 @@ struct sja1105_tagger_data {
*/
spinlock_t meta_lock;
unsigned long state;
- /* Used on SJA1110 where meta frames are generated only for
- * 2-step TX timestamps
- */
- struct sk_buff_head skb_txtstamp_queue;
struct kthread_worker *xmit_worker;
void (*xmit_work_fn)(struct kthread_work *work);
+ void (*meta_tstamp_handler)(struct dsa_switch *ds, int port, u8 ts_id,
+ enum sja1110_meta_tstamp dir, u64 tstamp);
};
struct sja1105_skb_cb {
@@ -69,21 +72,6 @@ struct sja1105_skb_cb {
#define SJA1105_SKB_CB(skb) \
((struct sja1105_skb_cb *)((skb)->cb))
-/* Timestamps are in units of 8 ns clock ticks (equivalent to
- * a fixed 125 MHz clock).
- */
-#define SJA1105_TICK_NS 8
-
-static inline s64 ns_to_sja1105_ticks(s64 ns)
-{
- return ns / SJA1105_TICK_NS;
-}
-
-static inline s64 sja1105_ticks_to_ns(s64 ticks)
-{
- return ticks * SJA1105_TICK_NS;
-}
-
static inline struct sja1105_tagger_data *
sja1105_tagger_data(struct dsa_switch *ds)
{
diff --git a/net/dsa/tag_sja1105.c b/net/dsa/tag_sja1105.c
index f3c1b31645f5..fe6a6d95bb26 100644
--- a/net/dsa/tag_sja1105.c
+++ b/net/dsa/tag_sja1105.c
@@ -4,7 +4,6 @@
#include <linux/if_vlan.h>
#include <linux/dsa/sja1105.h>
#include <linux/dsa/8021q.h>
-#include <linux/skbuff.h>
#include <linux/packing.h>
#include "dsa_priv.h"
@@ -54,11 +53,6 @@
#define SJA1110_TX_TRAILER_LEN 4
#define SJA1110_MAX_PADDING_LEN 15
-enum sja1110_meta_tstamp {
- SJA1110_META_TSTAMP_TX = 0,
- SJA1110_META_TSTAMP_RX = 1,
-};
-
/* Similar to is_link_local_ether_addr(hdr->h_dest) but also covers PTP */
static inline bool sja1105_is_link_local(const struct sk_buff *skb)
{
@@ -539,44 +533,12 @@ static struct sk_buff *sja1105_rcv(struct sk_buff *skb,
is_meta);
}
-static void sja1110_process_meta_tstamp(struct dsa_switch *ds, int port,
- u8 ts_id, enum sja1110_meta_tstamp dir,
- u64 tstamp)
-{
- struct sja1105_tagger_data *tagger_data = sja1105_tagger_data(ds);
- struct sk_buff *skb, *skb_tmp, *skb_match = NULL;
- struct skb_shared_hwtstamps shwt = {0};
-
- /* We don't care about RX timestamps on the CPU port */
- if (dir == SJA1110_META_TSTAMP_RX)
- return;
-
- spin_lock(&tagger_data->skb_txtstamp_queue.lock);
-
- skb_queue_walk_safe(&tagger_data->skb_txtstamp_queue, skb, skb_tmp) {
- if (SJA1105_SKB_CB(skb)->ts_id != ts_id)
- continue;
-
- __skb_unlink(skb, &tagger_data->skb_txtstamp_queue);
- skb_match = skb;
-
- break;
- }
-
- spin_unlock(&tagger_data->skb_txtstamp_queue.lock);
-
- if (WARN_ON(!skb_match))
- return;
-
- shwt.hwtstamp = ns_to_ktime(sja1105_ticks_to_ns(tstamp));
- skb_complete_tx_timestamp(skb_match, &shwt);
-}
-
static struct sk_buff *sja1110_rcv_meta(struct sk_buff *skb, u16 rx_header)
{
u8 *buf = dsa_etype_header_pos_rx(skb) + SJA1110_HEADER_LEN;
int switch_id = SJA1110_RX_HEADER_SWITCH_ID(rx_header);
int n_ts = SJA1110_RX_HEADER_N_TS(rx_header);
+ struct sja1105_tagger_data *tagger_data;
struct net_device *master = skb->dev;
struct dsa_port *cpu_dp;
struct dsa_switch *ds;
@@ -590,6 +552,10 @@ static struct sk_buff *sja1110_rcv_meta(struct sk_buff *skb, u16 rx_header)
return NULL;
}
+ tagger_data = sja1105_tagger_data(ds);
+ if (!tagger_data->meta_tstamp_handler)
+ return NULL;
+
for (i = 0; i <= n_ts; i++) {
u8 ts_id, source_port, dir;
u64 tstamp;
@@ -599,8 +565,8 @@ static struct sk_buff *sja1110_rcv_meta(struct sk_buff *skb, u16 rx_header)
dir = (buf[1] & BIT(3)) >> 3;
tstamp = be64_to_cpu(*(__be64 *)(buf + 2));
- sja1110_process_meta_tstamp(ds, source_port, ts_id, dir,
- tstamp);
+ tagger_data->meta_tstamp_handler(ds, source_port, ts_id, dir,
+ tstamp);
buf += SJA1110_META_TSTAMP_SIZE;
}
@@ -767,8 +733,6 @@ static int sja1105_connect(struct dsa_switch_tree *dst)
goto out;
}
- /* Only used on SJA1110 */
- skb_queue_head_init(&tagger_data->skb_txtstamp_queue);
spin_lock_init(&tagger_data->meta_lock);
xmit_worker = kthread_create_worker(0, "dsa%d:%d_xmit",
--
2.25.1
^ permalink raw reply related
* [PATCH v2 net-next 10/11] net: dsa: tag_sja1105: split sja1105_tagger_data into private and public sections
From: Vladimir Oltean @ 2021-12-09 23:34 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Martin Kaistra, Kurt Kanzenbach, Ansuel Smith,
Tobias Waldekranz
In-Reply-To: <20211209233447.336331-1-vladimir.oltean@nxp.com>
The sja1105 driver messes with the tagging protocol's state when PTP RX
timestamping is enabled/disabled. This is fundamentally necessary
because the tagger needs to know what to do when it receives a PTP
packet. If RX timestamping is enabled, then a metadata follow-up frame
is expected, and this holds the (partial) timestamp. So the tagger plays
hide-and-seek with the network stack until it also gets the metadata
frame, and then presents a single packet, the timestamped PTP packet.
But when RX timestamping isn't enabled, there is no metadata frame
expected, so the hide-and-seek game must be turned off and the packet
must be delivered right away to the network stack.
Considering this, we create a pseudo isolation by devising two tagger
methods callable by the switch: one to get the RX timestamping state,
and one to set it. Since we can't export symbols between the tagger and
the switch driver, these methods are exposed through function pointers.
After this change, the public portion of the sja1105_tagger_data
contains only function pointers.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
drivers/net/dsa/sja1105/sja1105_ptp.c | 22 ++----
include/linux/dsa/sja1105.h | 13 +--
net/dsa/tag_sja1105.c | 109 +++++++++++++++++++-------
3 files changed, 91 insertions(+), 53 deletions(-)
diff --git a/drivers/net/dsa/sja1105/sja1105_ptp.c b/drivers/net/dsa/sja1105/sja1105_ptp.c
index a9f7e4ae0bb2..be3068a935af 100644
--- a/drivers/net/dsa/sja1105/sja1105_ptp.c
+++ b/drivers/net/dsa/sja1105/sja1105_ptp.c
@@ -58,11 +58,10 @@ enum sja1105_ptp_clk_mode {
#define ptp_data_to_sja1105(d) \
container_of((d), struct sja1105_private, ptp_data)
-/* Must be called only with the tagger_data->state bit
- * SJA1105_HWTS_RX_EN cleared
+/* Must be called only while the RX timestamping state of the tagger
+ * is turned off
*/
static int sja1105_change_rxtstamping(struct sja1105_private *priv,
- struct sja1105_tagger_data *tagger_data,
bool on)
{
struct sja1105_ptp_data *ptp_data = &priv->ptp_data;
@@ -74,11 +73,6 @@ static int sja1105_change_rxtstamping(struct sja1105_private *priv,
general_params->send_meta1 = on;
general_params->send_meta0 = on;
- /* Initialize the meta state machine to a known state */
- if (tagger_data->stampable_skb) {
- kfree_skb(tagger_data->stampable_skb);
- tagger_data->stampable_skb = NULL;
- }
ptp_cancel_worker_sync(ptp_data->clock);
skb_queue_purge(&ptp_data->skb_txtstamp_queue);
skb_queue_purge(&ptp_data->skb_rxtstamp_queue);
@@ -117,17 +111,17 @@ int sja1105_hwtstamp_set(struct dsa_switch *ds, int port, struct ifreq *ifr)
break;
}
- if (rx_on != test_bit(SJA1105_HWTS_RX_EN, &tagger_data->state)) {
- clear_bit(SJA1105_HWTS_RX_EN, &tagger_data->state);
+ if (rx_on != tagger_data->rxtstamp_get_state(ds)) {
+ tagger_data->rxtstamp_set_state(ds, false);
- rc = sja1105_change_rxtstamping(priv, tagger_data, rx_on);
+ rc = sja1105_change_rxtstamping(priv, rx_on);
if (rc < 0) {
dev_err(ds->dev,
"Failed to change RX timestamping: %d\n", rc);
return rc;
}
if (rx_on)
- set_bit(SJA1105_HWTS_RX_EN, &tagger_data->state);
+ tagger_data->rxtstamp_set_state(ds, true);
}
if (copy_to_user(ifr->ifr_data, &config, sizeof(config)))
@@ -146,7 +140,7 @@ int sja1105_hwtstamp_get(struct dsa_switch *ds, int port, struct ifreq *ifr)
config.tx_type = HWTSTAMP_TX_ON;
else
config.tx_type = HWTSTAMP_TX_OFF;
- if (test_bit(SJA1105_HWTS_RX_EN, &tagger_data->state))
+ if (tagger_data->rxtstamp_get_state(ds))
config.rx_filter = HWTSTAMP_FILTER_PTP_V2_L2_EVENT;
else
config.rx_filter = HWTSTAMP_FILTER_NONE;
@@ -423,7 +417,7 @@ bool sja1105_rxtstamp(struct dsa_switch *ds, int port, struct sk_buff *skb)
struct sja1105_private *priv = ds->priv;
struct sja1105_ptp_data *ptp_data = &priv->ptp_data;
- if (!test_bit(SJA1105_HWTS_RX_EN, &tagger_data->state))
+ if (!tagger_data->rxtstamp_get_state(ds))
return false;
/* We need to read the full PTP clock to reconstruct the Rx
diff --git a/include/linux/dsa/sja1105.h b/include/linux/dsa/sja1105.h
index d216211b64f8..e9cb1ae6d742 100644
--- a/include/linux/dsa/sja1105.h
+++ b/include/linux/dsa/sja1105.h
@@ -35,8 +35,6 @@
#define SJA1105_META_SMAC 0x222222222222ull
#define SJA1105_META_DMAC 0x0180C200000Eull
-#define SJA1105_HWTS_RX_EN 0
-
enum sja1110_meta_tstamp {
SJA1110_META_TSTAMP_TX = 0,
SJA1110_META_TSTAMP_RX = 1,
@@ -50,16 +48,13 @@ struct sja1105_deferred_xmit_work {
/* Global tagger data */
struct sja1105_tagger_data {
- struct sk_buff *stampable_skb;
- /* Protects concurrent access to the meta state machine
- * from taggers running on multiple ports on SMP systems
- */
- spinlock_t meta_lock;
- unsigned long state;
- struct kthread_worker *xmit_worker;
+ /* Tagger to switch */
void (*xmit_work_fn)(struct kthread_work *work);
void (*meta_tstamp_handler)(struct dsa_switch *ds, int port, u8 ts_id,
enum sja1110_meta_tstamp dir, u64 tstamp);
+ /* Switch to tagger */
+ bool (*rxtstamp_get_state)(struct dsa_switch *ds);
+ void (*rxtstamp_set_state)(struct dsa_switch *ds, bool on);
};
struct sja1105_skb_cb {
diff --git a/net/dsa/tag_sja1105.c b/net/dsa/tag_sja1105.c
index fe6a6d95bb26..93d2484b2480 100644
--- a/net/dsa/tag_sja1105.c
+++ b/net/dsa/tag_sja1105.c
@@ -53,6 +53,25 @@
#define SJA1110_TX_TRAILER_LEN 4
#define SJA1110_MAX_PADDING_LEN 15
+#define SJA1105_HWTS_RX_EN 0
+
+struct sja1105_tagger_private {
+ struct sja1105_tagger_data data; /* Must be first */
+ unsigned long state;
+ /* Protects concurrent access to the meta state machine
+ * from taggers running on multiple ports on SMP systems
+ */
+ spinlock_t meta_lock;
+ struct sk_buff *stampable_skb;
+ struct kthread_worker *xmit_worker;
+};
+
+static struct sja1105_tagger_private *
+sja1105_tagger_private(struct dsa_switch *ds)
+{
+ return ds->tagger_data;
+}
+
/* Similar to is_link_local_ether_addr(hdr->h_dest) but also covers PTP */
static inline bool sja1105_is_link_local(const struct sk_buff *skb)
{
@@ -120,12 +139,13 @@ static struct sk_buff *sja1105_defer_xmit(struct dsa_port *dp,
struct sk_buff *skb)
{
struct sja1105_tagger_data *tagger_data = sja1105_tagger_data(dp->ds);
+ struct sja1105_tagger_private *priv = sja1105_tagger_private(dp->ds);
void (*xmit_work_fn)(struct kthread_work *work);
struct sja1105_deferred_xmit_work *xmit_work;
struct kthread_worker *xmit_worker;
xmit_work_fn = tagger_data->xmit_work_fn;
- xmit_worker = tagger_data->xmit_worker;
+ xmit_worker = priv->xmit_worker;
if (!xmit_work_fn || !xmit_worker)
return NULL;
@@ -362,32 +382,32 @@ static struct sk_buff
*/
if (is_link_local) {
struct dsa_port *dp = dsa_slave_to_port(skb->dev);
- struct sja1105_tagger_data *tagger_data;
+ struct sja1105_tagger_private *priv;
struct dsa_switch *ds = dp->ds;
- tagger_data = sja1105_tagger_data(ds);
+ priv = sja1105_tagger_private(ds);
- if (!test_bit(SJA1105_HWTS_RX_EN, &tagger_data->state))
+ if (!test_bit(SJA1105_HWTS_RX_EN, &priv->state))
/* Do normal processing. */
return skb;
- spin_lock(&tagger_data->meta_lock);
+ spin_lock(&priv->meta_lock);
/* Was this a link-local frame instead of the meta
* that we were expecting?
*/
- if (tagger_data->stampable_skb) {
+ if (priv->stampable_skb) {
dev_err_ratelimited(ds->dev,
"Expected meta frame, is %12llx "
"in the DSA master multicast filter?\n",
SJA1105_META_DMAC);
- kfree_skb(tagger_data->stampable_skb);
+ kfree_skb(priv->stampable_skb);
}
/* Hold a reference to avoid dsa_switch_rcv
* from freeing the skb.
*/
- tagger_data->stampable_skb = skb_get(skb);
- spin_unlock(&tagger_data->meta_lock);
+ priv->stampable_skb = skb_get(skb);
+ spin_unlock(&priv->meta_lock);
/* Tell DSA we got nothing */
return NULL;
@@ -400,22 +420,22 @@ static struct sk_buff
*/
} else if (is_meta) {
struct dsa_port *dp = dsa_slave_to_port(skb->dev);
- struct sja1105_tagger_data *tagger_data;
+ struct sja1105_tagger_private *priv;
struct dsa_switch *ds = dp->ds;
struct sk_buff *stampable_skb;
- tagger_data = sja1105_tagger_data(ds);
+ priv = sja1105_tagger_private(ds);
/* Drop the meta frame if we're not in the right state
* to process it.
*/
- if (!test_bit(SJA1105_HWTS_RX_EN, &tagger_data->state))
+ if (!test_bit(SJA1105_HWTS_RX_EN, &priv->state))
return NULL;
- spin_lock(&tagger_data->meta_lock);
+ spin_lock(&priv->meta_lock);
- stampable_skb = tagger_data->stampable_skb;
- tagger_data->stampable_skb = NULL;
+ stampable_skb = priv->stampable_skb;
+ priv->stampable_skb = NULL;
/* Was this a meta frame instead of the link-local
* that we were expecting?
@@ -423,14 +443,14 @@ static struct sk_buff
if (!stampable_skb) {
dev_err_ratelimited(ds->dev,
"Unexpected meta frame\n");
- spin_unlock(&tagger_data->meta_lock);
+ spin_unlock(&priv->meta_lock);
return NULL;
}
if (stampable_skb->dev != skb->dev) {
dev_err_ratelimited(ds->dev,
"Meta frame on wrong port\n");
- spin_unlock(&tagger_data->meta_lock);
+ spin_unlock(&priv->meta_lock);
return NULL;
}
@@ -441,12 +461,36 @@ static struct sk_buff
skb = stampable_skb;
sja1105_transfer_meta(skb, meta);
- spin_unlock(&tagger_data->meta_lock);
+ spin_unlock(&priv->meta_lock);
}
return skb;
}
+static bool sja1105_rxtstamp_get_state(struct dsa_switch *ds)
+{
+ struct sja1105_tagger_private *priv = sja1105_tagger_private(ds);
+
+ return test_bit(SJA1105_HWTS_RX_EN, &priv->state);
+}
+
+static void sja1105_rxtstamp_set_state(struct dsa_switch *ds, bool on)
+{
+ struct sja1105_tagger_private *priv = sja1105_tagger_private(ds);
+
+ if (on)
+ set_bit(SJA1105_HWTS_RX_EN, &priv->state);
+ else
+ clear_bit(SJA1105_HWTS_RX_EN, &priv->state);
+
+ /* Initialize the meta state machine to a known state */
+ if (!priv->stampable_skb)
+ return;
+
+ kfree_skb(priv->stampable_skb);
+ priv->stampable_skb = NULL;
+}
+
static bool sja1105_skb_has_tag_8021q(const struct sk_buff *skb)
{
u16 tpid = ntohs(eth_hdr(skb)->h_proto);
@@ -699,26 +743,27 @@ static void sja1110_flow_dissect(const struct sk_buff *skb, __be16 *proto,
static void sja1105_disconnect(struct dsa_switch_tree *dst)
{
- struct sja1105_tagger_data *tagger_data;
+ struct sja1105_tagger_private *priv;
struct dsa_port *dp;
list_for_each_entry(dp, &dst->ports, list) {
- tagger_data = dp->ds->tagger_data;
+ priv = dp->ds->tagger_data;
- if (!tagger_data)
+ if (!priv)
continue;
- if (tagger_data->xmit_worker)
- kthread_destroy_worker(tagger_data->xmit_worker);
+ if (priv->xmit_worker)
+ kthread_destroy_worker(priv->xmit_worker);
- kfree(tagger_data);
- dp->ds->tagger_data = NULL;
+ kfree(priv);
+ dp->ds->priv = NULL;
}
}
static int sja1105_connect(struct dsa_switch_tree *dst)
{
struct sja1105_tagger_data *tagger_data;
+ struct sja1105_tagger_private *priv;
struct kthread_worker *xmit_worker;
struct dsa_port *dp;
int err;
@@ -727,13 +772,13 @@ static int sja1105_connect(struct dsa_switch_tree *dst)
if (dp->ds->tagger_data)
continue;
- tagger_data = kzalloc(sizeof(*tagger_data), GFP_KERNEL);
- if (!tagger_data) {
+ priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+ if (!priv) {
err = -ENOMEM;
goto out;
}
- spin_lock_init(&tagger_data->meta_lock);
+ spin_lock_init(&priv->meta_lock);
xmit_worker = kthread_create_worker(0, "dsa%d:%d_xmit",
dst->index, dp->ds->index);
@@ -742,8 +787,12 @@ static int sja1105_connect(struct dsa_switch_tree *dst)
goto out;
}
- tagger_data->xmit_worker = xmit_worker;
- dp->ds->tagger_data = tagger_data;
+ priv->xmit_worker = xmit_worker;
+ /* Export functions for switch driver use */
+ tagger_data = &priv->data;
+ tagger_data->rxtstamp_get_state = sja1105_rxtstamp_get_state;
+ tagger_data->rxtstamp_set_state = sja1105_rxtstamp_set_state;
+ dp->ds->tagger_data = priv;
}
return 0;
--
2.25.1
^ permalink raw reply related
* [PATCH v2 net-next 11/11] net: dsa: remove dp->priv
From: Vladimir Oltean @ 2021-12-09 23:34 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Andrew Lunn, Vivien Didelot,
Florian Fainelli, Martin Kaistra, Kurt Kanzenbach, Ansuel Smith,
Tobias Waldekranz
In-Reply-To: <20211209233447.336331-1-vladimir.oltean@nxp.com>
All current in-tree uses of dp->priv have been replaced with
ds->tagger_data, which provides for a safer API especially when the
connection isn't the regular 1:1 link between one switch driver and one
tagging protocol driver, but could be either one switch to many taggers,
or many switches to one tagger.
Therefore, we can remove this unused pointer.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
include/net/dsa.h | 6 ------
1 file changed, 6 deletions(-)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 8b496c7e62ef..64d71968aa91 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -276,12 +276,6 @@ struct dsa_port {
struct list_head list;
- /*
- * Give the switch driver somewhere to hang its per-port private data
- * structures (accessible from the tagger).
- */
- void *priv;
-
/*
* Original copy of the master netdev ethtool_ops
*/
--
2.25.1
^ permalink raw reply related
* Re: [PATCH net-next v1] net: dsa: mv88e6xxx: Trap PTP traffic
From: Tobias Waldekranz @ 2021-12-10 0:07 UTC (permalink / raw)
To: Kurt Kanzenbach, Andrew Lunn, Vivien Didelot
Cc: Florian Fainelli, Vladimir Oltean, David S. Miller,
Jakub Kicinski, netdev, Richard Cochran, Kurt Kanzenbach
In-Reply-To: <20211209173337.24521-1-kurt@kmk-computers.de>
On Thu, Dec 09, 2021 at 18:33, Kurt Kanzenbach <kurt@kmk-computers.de> wrote:
> A time aware switch should trap PTP traffic to the management CPU. User space
> daemons such as ptp4l will process these messages to implement Boundary (or
> Transparent) Clocks.
>
> At the moment the mv88e6xxx driver for mv88e6341 doesn't trap these messages
> which leads to confusion when multiple end devices are connected to the
> switch. Therefore, setup PTP traps. Leverage the already implemented policy
> mechanism based on destination addresses. Configure the traps only if
> timestamping is enabled so that non time aware use case is still functioning.
Do we know how PTP is supposed to work in relation to things like STP?
I.e should you be able to run PTP over a link that is currently in
blocking? It seems like being able to sync your clock before a topology
change occurs would be nice. In that case, these addresses should be
added to the ATU as MGMT instead of policy entries.
> Introduce an additional PTP operation in case other devices need special
> handling in regards to trapping as well.
>
> Tested on Marvell Topaz (mv88e6341) switch with multiple end devices connected
> like this:
>
> |# DSA setup
> |$ ip link set eth0 up
> |$ ip link set lan0 up
> |$ ip link set lan1 up
> |$ ip link set lan2 up
> |$ ip link add name br0 type bridge
> |$ ip link set dev lan0 master br0
> |$ ip link set dev lan1 master br0
> |$ ip link set dev lan2 master br0
> |$ ip link set lan0 up
> |$ ip link set lan1 up
> |$ ip link set lan2 up
> |$ ip link set br0 up
> |$ dhclient br0
> |# Configure bridge routing
> |$ ebtables --table broute --append BROUTING --protocol 0x88F7 --jump DROP
> |# Start linuxptp
> |$ ptp4l -H -2 -i lan0 -i lan1 -i lan2 --tx_timestamp_timeout=40 -m
>
> Verified added policies with mv88e6xxx_dump.
>
> Signed-off-by: Kurt Kanzenbach <kurt@kmk-computers.de>
> ---
> drivers/net/dsa/mv88e6xxx/chip.c | 12 +++---
> drivers/net/dsa/mv88e6xxx/chip.h | 5 +++
> drivers/net/dsa/mv88e6xxx/hwtstamp.c | 7 ++++
> drivers/net/dsa/mv88e6xxx/ptp.c | 59 ++++++++++++++++++++++++++++
> drivers/net/dsa/mv88e6xxx/ptp.h | 2 +
> 5 files changed, 80 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index 7fadbf987b23..ab50ebd85f1f 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -1816,8 +1816,8 @@ static int mv88e6xxx_port_db_load_purge(struct mv88e6xxx_chip *chip, int port,
> return mv88e6xxx_g1_atu_loadpurge(chip, fid, &entry);
> }
>
> -static int mv88e6xxx_policy_apply(struct mv88e6xxx_chip *chip, int port,
> - const struct mv88e6xxx_policy *policy)
> +int mv88e6xxx_policy_apply(struct mv88e6xxx_chip *chip, int port,
> + const struct mv88e6xxx_policy *policy)
> {
> enum mv88e6xxx_policy_mapping mapping = policy->mapping;
> enum mv88e6xxx_policy_action action = policy->action;
> @@ -1835,10 +1835,12 @@ static int mv88e6xxx_policy_apply(struct mv88e6xxx_chip *chip, int port,
> case MV88E6XXX_POLICY_MAPPING_SA:
> if (action == MV88E6XXX_POLICY_ACTION_NORMAL)
> state = 0; /* Dissociate the port and address */
> - else if (action == MV88E6XXX_POLICY_ACTION_DISCARD &&
> + else if ((action == MV88E6XXX_POLICY_ACTION_DISCARD ||
> + action == MV88E6XXX_POLICY_ACTION_TRAP) &&
> is_multicast_ether_addr(addr))
> state = MV88E6XXX_G1_ATU_DATA_STATE_MC_STATIC_POLICY;
> - else if (action == MV88E6XXX_POLICY_ACTION_DISCARD &&
> + else if ((action == MV88E6XXX_POLICY_ACTION_DISCARD ||
> + action == MV88E6XXX_POLICY_ACTION_TRAP) &&
> is_unicast_ether_addr(addr))
> state = MV88E6XXX_G1_ATU_DATA_STATE_UC_STATIC_POLICY;
> else
> @@ -4589,7 +4591,7 @@ static const struct mv88e6xxx_ops mv88e6341_ops = {
> .serdes_irq_status = mv88e6390_serdes_irq_status,
> .gpio_ops = &mv88e6352_gpio_ops,
> .avb_ops = &mv88e6390_avb_ops,
> - .ptp_ops = &mv88e6352_ptp_ops,
> + .ptp_ops = &mv88e6341_ptp_ops,
> .serdes_get_sset_count = mv88e6390_serdes_get_sset_count,
> .serdes_get_strings = mv88e6390_serdes_get_strings,
> .serdes_get_stats = mv88e6390_serdes_get_stats,
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
> index 8271b8aa7b71..795ae5a56834 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.h
> +++ b/drivers/net/dsa/mv88e6xxx/chip.h
> @@ -673,6 +673,8 @@ struct mv88e6xxx_ptp_ops {
> int (*port_disable)(struct mv88e6xxx_chip *chip, int port);
> int (*global_enable)(struct mv88e6xxx_chip *chip);
> int (*global_disable)(struct mv88e6xxx_chip *chip);
> + int (*setup_ptp_traps)(struct mv88e6xxx_chip *chip, int port,
> + bool enable);
> int n_ext_ts;
> int arr0_sts_reg;
> int arr1_sts_reg;
> @@ -760,4 +762,7 @@ static inline void mv88e6xxx_reg_unlock(struct mv88e6xxx_chip *chip)
>
> int mv88e6xxx_fid_map(struct mv88e6xxx_chip *chip, unsigned long *bitmap);
>
> +int mv88e6xxx_policy_apply(struct mv88e6xxx_chip *chip, int port,
> + const struct mv88e6xxx_policy *policy);
> +
> #endif /* _MV88E6XXX_CHIP_H */
> diff --git a/drivers/net/dsa/mv88e6xxx/hwtstamp.c b/drivers/net/dsa/mv88e6xxx/hwtstamp.c
> index 8f74ffc7a279..617aeb6cbaac 100644
> --- a/drivers/net/dsa/mv88e6xxx/hwtstamp.c
> +++ b/drivers/net/dsa/mv88e6xxx/hwtstamp.c
> @@ -94,6 +94,7 @@ static int mv88e6xxx_set_hwtstamp_config(struct mv88e6xxx_chip *chip, int port,
> const struct mv88e6xxx_ptp_ops *ptp_ops = chip->info->ops->ptp_ops;
> struct mv88e6xxx_port_hwtstamp *ps = &chip->port_hwtstamp[port];
> bool tstamp_enable = false;
> + int ret;
>
> /* Prevent the TX/RX paths from trying to interact with the
> * timestamp hardware while we reconfigure it.
> @@ -161,6 +162,12 @@ static int mv88e6xxx_set_hwtstamp_config(struct mv88e6xxx_chip *chip, int port,
> if (chip->enable_count == 0 && ptp_ops->global_disable)
> ptp_ops->global_disable(chip);
> }
> +
> + if (ptp_ops->setup_ptp_traps) {
> + ret = ptp_ops->setup_ptp_traps(chip, port, tstamp_enable);
> + if (tstamp_enable && ret)
> + dev_warn(chip->dev, "Failed to setup PTP traps. PTP might not work as desired!\n");
> + }
> mv88e6xxx_reg_unlock(chip);
>
> /* Once hardware has been configured, enable timestamp checks
> diff --git a/drivers/net/dsa/mv88e6xxx/ptp.c b/drivers/net/dsa/mv88e6xxx/ptp.c
> index d838c174dc0d..8d6ff03d37c8 100644
> --- a/drivers/net/dsa/mv88e6xxx/ptp.c
> +++ b/drivers/net/dsa/mv88e6xxx/ptp.c
> @@ -345,6 +345,37 @@ static int mv88e6352_ptp_verify(struct ptp_clock_info *ptp, unsigned int pin,
> return 0;
> }
>
> +static int mv88e6341_setup_ptp_traps(struct mv88e6xxx_chip *chip, int port,
> + bool enable)
> +{
> + static const u8 ptp_destinations[][ETH_ALEN] = {
> + { 0x01, 0x1b, 0x19, 0x00, 0x00, 0x00 }, /* L2 PTP */
> + { 0x01, 0x80, 0xc2, 0x00, 0x00, 0x0e }, /* L2 P2P */
> + { 0x01, 0x00, 0x5e, 0x00, 0x01, 0x81 }, /* IPv4 PTP */
> + { 0x01, 0x00, 0x5e, 0x00, 0x00, 0x6b }, /* IPv4 P2P */
> + { 0x33, 0x33, 0x00, 0x00, 0x01, 0x81 }, /* IPv6 PTP */
> + { 0x33, 0x33, 0x00, 0x00, 0x00, 0x6b }, /* IPv6 P2P */
How does the L3 entries above play together with IGMP/MLD? I.e. what
happens if, after launching ptp4l, an IGMP report comes in on lanX,
requesting that same group? Would the policy entry not be overwritten by
mv88e6xxx_port_mdb_add?
Eventually I think we will have many interfaces to configure static
entries in the ATU:
1. MDB entries from a bridge (already in place)
2. User-configured entries through ethtool's rxnfc (already in place)
3. Driver-internal consumers (this patch, MRP, etc.)
4. User-configured entries through TC.
Seems to me like we need to start tracking the owners for these to stop
them from stomping on one another.
> + };
> + int ret, i;
> +
> + for (i = 0; i < ARRAY_SIZE(ptp_destinations); ++i) {
> + struct mv88e6xxx_policy policy = { };
> +
> + policy.mapping = MV88E6XXX_POLICY_MAPPING_DA;
> + policy.action = enable ? MV88E6XXX_POLICY_ACTION_TRAP :
> + MV88E6XXX_POLICY_ACTION_NORMAL;
> + policy.port = port;
> + policy.vid = 0;
> + ether_addr_copy(policy.addr, ptp_destinations[i]);
> +
> + ret = mv88e6xxx_policy_apply(chip, port, &policy);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> const struct mv88e6xxx_ptp_ops mv88e6165_ptp_ops = {
> .clock_read = mv88e6165_ptp_clock_read,
> .global_enable = mv88e6165_global_enable,
> @@ -419,6 +450,34 @@ const struct mv88e6xxx_ptp_ops mv88e6352_ptp_ops = {
> .cc_mult_dem = MV88E6XXX_CC_MULT_DEM,
> };
>
> +const struct mv88e6xxx_ptp_ops mv88e6341_ptp_ops = {
> + .clock_read = mv88e6352_ptp_clock_read,
> + .ptp_enable = mv88e6352_ptp_enable,
> + .ptp_verify = mv88e6352_ptp_verify,
> + .event_work = mv88e6352_tai_event_work,
> + .port_enable = mv88e6352_hwtstamp_port_enable,
> + .port_disable = mv88e6352_hwtstamp_port_disable,
> + .setup_ptp_traps = mv88e6341_setup_ptp_traps,
Is there any reason why this could not be added to the existing ops for
6352 instead? Their ATU's are compatible, IIRC.
> + .n_ext_ts = 1,
> + .arr0_sts_reg = MV88E6XXX_PORT_PTP_ARR0_STS,
> + .arr1_sts_reg = MV88E6XXX_PORT_PTP_ARR1_STS,
> + .dep_sts_reg = MV88E6XXX_PORT_PTP_DEP_STS,
> + .rx_filters = (1 << HWTSTAMP_FILTER_NONE) |
> + (1 << HWTSTAMP_FILTER_PTP_V2_L4_EVENT) |
> + (1 << HWTSTAMP_FILTER_PTP_V2_L4_SYNC) |
> + (1 << HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ) |
> + (1 << HWTSTAMP_FILTER_PTP_V2_L2_EVENT) |
> + (1 << HWTSTAMP_FILTER_PTP_V2_L2_SYNC) |
> + (1 << HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ) |
> + (1 << HWTSTAMP_FILTER_PTP_V2_EVENT) |
> + (1 << HWTSTAMP_FILTER_PTP_V2_SYNC) |
> + (1 << HWTSTAMP_FILTER_PTP_V2_DELAY_REQ),
> + .cc_shift = MV88E6XXX_CC_SHIFT,
> + .cc_mult = MV88E6XXX_CC_MULT,
> + .cc_mult_num = MV88E6XXX_CC_MULT_NUM,
> + .cc_mult_dem = MV88E6XXX_CC_MULT_DEM,
> +};
> +
> static u64 mv88e6xxx_ptp_clock_read(const struct cyclecounter *cc)
> {
> struct mv88e6xxx_chip *chip = cc_to_chip(cc);
> diff --git a/drivers/net/dsa/mv88e6xxx/ptp.h b/drivers/net/dsa/mv88e6xxx/ptp.h
> index 269d5d16a466..badcb72d10a6 100644
> --- a/drivers/net/dsa/mv88e6xxx/ptp.h
> +++ b/drivers/net/dsa/mv88e6xxx/ptp.h
> @@ -151,6 +151,7 @@ void mv88e6xxx_ptp_free(struct mv88e6xxx_chip *chip);
> extern const struct mv88e6xxx_ptp_ops mv88e6165_ptp_ops;
> extern const struct mv88e6xxx_ptp_ops mv88e6250_ptp_ops;
> extern const struct mv88e6xxx_ptp_ops mv88e6352_ptp_ops;
> +extern const struct mv88e6xxx_ptp_ops mv88e6341_ptp_ops;
>
> #else /* !CONFIG_NET_DSA_MV88E6XXX_PTP */
>
> @@ -171,6 +172,7 @@ static inline void mv88e6xxx_ptp_free(struct mv88e6xxx_chip *chip)
> static const struct mv88e6xxx_ptp_ops mv88e6165_ptp_ops = {};
> static const struct mv88e6xxx_ptp_ops mv88e6250_ptp_ops = {};
> static const struct mv88e6xxx_ptp_ops mv88e6352_ptp_ops = {};
> +static const struct mv88e6xxx_ptp_ops mv88e6341_ptp_ops = {};
>
> #endif /* CONFIG_NET_DSA_MV88E6XXX_PTP */
>
> --
> 2.34.1
^ permalink raw reply
* Re: [RFC net-next 2/2] ipv6: ioam: Support for Buffer occupancy data field
From: Jakub Kicinski @ 2021-12-10 0:38 UTC (permalink / raw)
To: Justin Iurman
Cc: netdev, davem, dsahern, yoshfuji, linux-mm, cl, penberg, rientjes,
iamjoonsoo kim, akpm, vbabka, Roopa Prabhu, Nikolay Aleksandrov,
Andrew Lunn, Stephen Hemminger, Vladimir Oltean, Florian Fainelli,
Florian Westphal, Paolo Abeni
In-Reply-To: <1067680364.223350225.1639059024535.JavaMail.zimbra@uliege.be>
On Thu, 9 Dec 2021 15:10:24 +0100 (CET) Justin Iurman wrote:
> > because Linux routers can run a full telemetry stack and all sort
> > of advanced SW instrumentation. The use case for reporting kernel
> > memory use via IOAM's constrained interface does not seem particularly
> > practical since it's not providing a very strong signal on what's
> > going on.
>
> I agree and disagree. I disagree because this value definitely tells you
> that something (potentially bad) is going on, when it increases
> significantly enough to reach a critical threshold. Basically, we need
> more skb's, but oh, the pool is exhausted. OK, not a problem, expand the
> pool. Oh wait, no memory left. Why? Is it only due to too much
> (temporary?) load? Should I put the blame on the NIC? Is it a memory
> issue? Is it something else? Or maybe several issues combined? Well, you
> might not know exactly why (though you know there is a problem), which is
> also why I agree with you. But, this is also why you have other data
> fields available (i.e., detecting a problem might require 2+ symptoms
> instead of just one).
>
> > For switches running Linux the switch ASIC buffer occupancy can be read
> > via devlink-sb that'd seem like a better fit for me, but unfortunately
> > the devlink calls can sleep so we can't read such device info from the
> > datapath.
>
> Indeed, would be a better fit. I didn't know about this one, thanks for
> that. It's a shame it can't be used in this context, though. But, at the
> end of the day, we're left with nothing regarding buffer occupancy. So
> I'm wondering if "something" is not better than "nothing" in this case.
> And, for that, we're back to my previous answer on why I agree and
> disagree with what you said about its utility.
I think we're on the same page, the main problem is I've not seen
anyone use the skbuff_head_cache occupancy as a signal in practice.
I'm adding a bunch of people to the CC list, hopefully someone has
an opinion one way or the other.
Lore link to the full thread, FWIW:
https://lore.kernel.org/all/20211206211758.19057-1-justin.iurman@uliege.be/
^ permalink raw reply
* Re: [PATCH net-next v3] net: phylink: Add helpers for c22 registers without MDIO
From: Russell King (Oracle) @ 2021-12-10 1:05 UTC (permalink / raw)
To: Sean Anderson
Cc: netdev, linux-kernel, Horatiu Vultur, Jakub Kicinski, Andrew Lunn,
David S . Miller, Heiner Kallweit
In-Reply-To: <b0b80264-0a1d-f67b-b1ca-204857352b31@seco.com>
On Thu, Dec 09, 2021 at 05:26:16PM -0500, Sean Anderson wrote:
> ping?
Hi Sean,
There is a version of the patch that is in net-next:
291dcae39bc4 net: phylink: Add helpers for c22 registers without MDIO
Looking at the date in that commit, it ties up with the date in your
v3 patch, but patchwork has been set to "Not applicable" - I think
someone didn't update patchwork correctly.
I think we can assume it has been merged, but incorrectly marked in
patchwork.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* Re: [PATCH net-next v7 1/6] stmmac: dwmac-mediatek: add platform level clocks management
From: Biao Huang @ 2021-12-10 1:19 UTC (permalink / raw)
To: AngeloGioacchino Del Regno, davem, Jakub Kicinski, Rob Herring
Cc: Matthias Brugger, Giuseppe Cavallaro, Alexandre Torgue,
Jose Abreu, Maxime Coquelin, netdev, devicetree, linux-kernel,
linux-arm-kernel, linux-mediatek, linux-stm32, srv_heupstream,
macpaul.lin, dkirjanov
In-Reply-To: <2e8ccd43-bba0-9695-8d6d-d37e0b71fa7d@collabora.com>
Dear Angelo,
Thanks for your comments~
On Thu, 2021-12-09 at 11:51 +0100, AngeloGioacchino Del Regno wrote:
> Il 08/12/21 06:47, Biao Huang ha scritto:
> > This patch implements clks_config callback for dwmac-mediatek
> > platform,
> > which could support platform level clocks management.
> >
> > Signed-off-by: Biao Huang <biao.huang@mediatek.com>
>
> Sorry, I've sent my ack on v6. Sending it on v7.
>
> Acked-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
I'll add "acked-by" in next send.
^ permalink raw reply
* [PATCH net-next v8 1/6] stmmac: dwmac-mediatek: add platform level clocks management
From: Biao Huang @ 2021-12-10 1:31 UTC (permalink / raw)
To: davem, Jakub Kicinski, Rob Herring
Cc: Matthias Brugger, Giuseppe Cavallaro, Alexandre Torgue,
Jose Abreu, Maxime Coquelin, Biao Huang, netdev, devicetree,
linux-kernel, linux-arm-kernel, linux-mediatek, linux-stm32,
srv_heupstream, macpaul.lin, angelogioacchino.delregno, dkirjanov
In-Reply-To: <20211210013129.811-1-biao.huang@mediatek.com>
This patch implements clks_config callback for dwmac-mediatek platform,
which could support platform level clocks management.
Signed-off-by: Biao Huang <biao.huang@mediatek.com>
Acked-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
.../ethernet/stmicro/stmmac/dwmac-mediatek.c | 25 +++++++++++++------
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
index 58c0feaa8131..0ff57c268dca 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
@@ -9,7 +9,6 @@
#include <linux/of.h>
#include <linux/of_device.h>
#include <linux/of_net.h>
-#include <linux/pm_runtime.h>
#include <linux/regmap.h>
#include <linux/stmmac.h>
@@ -359,9 +358,6 @@ static int mediatek_dwmac_init(struct platform_device *pdev, void *priv)
return ret;
}
- pm_runtime_enable(&pdev->dev);
- pm_runtime_get_sync(&pdev->dev);
-
return 0;
}
@@ -370,11 +366,25 @@ static void mediatek_dwmac_exit(struct platform_device *pdev, void *priv)
struct mediatek_dwmac_plat_data *plat = priv;
clk_bulk_disable_unprepare(plat->num_clks_to_config, plat->clks);
-
- pm_runtime_put_sync(&pdev->dev);
- pm_runtime_disable(&pdev->dev);
}
+static int mediatek_dwmac_clks_config(void *priv, bool enabled)
+{
+ struct mediatek_dwmac_plat_data *plat = priv;
+ int ret = 0;
+
+ if (enabled) {
+ ret = clk_bulk_prepare_enable(plat->num_clks_to_config, plat->clks);
+ if (ret) {
+ dev_err(plat->dev, "failed to enable clks, err = %d\n", ret);
+ return ret;
+ }
+ } else {
+ clk_bulk_disable_unprepare(plat->num_clks_to_config, plat->clks);
+ }
+
+ return ret;
+}
static int mediatek_dwmac_probe(struct platform_device *pdev)
{
struct mediatek_dwmac_plat_data *priv_plat;
@@ -420,6 +430,7 @@ static int mediatek_dwmac_probe(struct platform_device *pdev)
plat_dat->bsp_priv = priv_plat;
plat_dat->init = mediatek_dwmac_init;
plat_dat->exit = mediatek_dwmac_exit;
+ plat_dat->clks_config = mediatek_dwmac_clks_config;
mediatek_dwmac_init(pdev, priv_plat);
ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
--
2.25.1
^ permalink raw reply related
* [PATCH net-next v8 0/6] MediaTek Ethernet Patches on MT8195
From: Biao Huang @ 2021-12-10 1:31 UTC (permalink / raw)
To: davem, Jakub Kicinski, Rob Herring
Cc: Matthias Brugger, Giuseppe Cavallaro, Alexandre Torgue,
Jose Abreu, Maxime Coquelin, Biao Huang, netdev, devicetree,
linux-kernel, linux-arm-kernel, linux-mediatek, linux-stm32,
srv_heupstream, macpaul.lin, angelogioacchino.delregno, dkirjanov
Changes in v8:
1. add acked-by in "stmmac: dwmac-mediatek: add platform level clocks
management" patch
Changes in v7:
1. fix uninitialized warning as Jakub's comments.
Changes in v6:
1. update commit message as Jakub's comments.
2. split mt8195 eth dts patch("arm64: dts: mt8195: add ethernet device
node") from this series, since mt8195 dtsi/dts basic patches is still
under reviewing.
https://patchwork.kernel.org/project/linux-mediatek/list/?series=579071
we'll resend mt8195 eth dts patch once all the dependent patches are
accepted.
Changes in v5:
1. remove useless inclusion in dwmac-mediatek.c as Angelo's comments.
2. add acked-by in "net-next: stmmac: dwmac-mediatek: add support for
mt8195" patch
Changes in v4:
1. add changes in commit message in "net-next: dt-bindings: dwmac:
Convert mediatek-dwmac to DT schema" patch.
2. remove ethernet-controller.yaml since snps,dwmac.yaml already include it.
Changes in v3:
1. Add prefix "net-next" to support new IC as Denis's suggestion.
2. Split dt-bindings to two patches, one for conversion, and the other for
new IC.
3. add a new patch to update device node in mt2712-evb.dts to accommodate to
changes in driver.
4. remove unnecessary wrapper as Angelo's suggestion.
5. Add acked-by in "net-next: stmmac: dwmac-mediatek: Reuse more common
features" patch.
Changes in v2:
1. fix errors/warnings in mediatek-dwmac.yaml with upgraded dtschema tools
This series include 5 patches:
1. add platform level clocks management for dwmac-mediatek
2. resue more common features defined in stmmac_platform.c
3. add ethernet entry for mt8195
4. convert mediatek-dwmac.txt to mediatek-dwmac.yaml
Biao Huang (6):
stmmac: dwmac-mediatek: add platform level clocks management
stmmac: dwmac-mediatek: Reuse more common features
arm64: dts: mt2712: update ethernet device node
net: dt-bindings: dwmac: Convert mediatek-dwmac to DT schema
stmmac: dwmac-mediatek: add support for mt8195
net: dt-bindings: dwmac: add support for mt8195
.../bindings/net/mediatek-dwmac.txt | 91 ------
.../bindings/net/mediatek-dwmac.yaml | 210 ++++++++++++
arch/arm64/boot/dts/mediatek/mt2712-evb.dts | 1 +
arch/arm64/boot/dts/mediatek/mt2712e.dtsi | 14 +-
.../ethernet/stmicro/stmmac/dwmac-mediatek.c | 306 ++++++++++++++++--
5 files changed, 503 insertions(+), 119 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/net/mediatek-dwmac.txt
create mode 100644 Documentation/devicetree/bindings/net/mediatek-dwmac.yaml
--
2.18.0
^ permalink raw reply
* [PATCH net-next v8 3/6] arm64: dts: mt2712: update ethernet device node
From: Biao Huang @ 2021-12-10 1:31 UTC (permalink / raw)
To: davem, Jakub Kicinski, Rob Herring
Cc: Matthias Brugger, Giuseppe Cavallaro, Alexandre Torgue,
Jose Abreu, Maxime Coquelin, Biao Huang, netdev, devicetree,
linux-kernel, linux-arm-kernel, linux-mediatek, linux-stm32,
srv_heupstream, macpaul.lin, angelogioacchino.delregno, dkirjanov
In-Reply-To: <20211210013129.811-1-biao.huang@mediatek.com>
Since there are some changes in ethernet driver,
update ethernet device node in dts to accommodate to it.
Signed-off-by: Biao Huang <biao.huang@mediatek.com>
---
arch/arm64/boot/dts/mediatek/mt2712-evb.dts | 1 +
arch/arm64/boot/dts/mediatek/mt2712e.dtsi | 14 +++++++++-----
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/boot/dts/mediatek/mt2712-evb.dts b/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
index 7d369fdd3117..11aa135aa0f3 100644
--- a/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
+++ b/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
@@ -110,6 +110,7 @@ ð {
phy-handle = <ðernet_phy0>;
mediatek,tx-delay-ps = <1530>;
snps,reset-gpio = <&pio 87 GPIO_ACTIVE_LOW>;
+ snps,reset-delays-us = <0 10000 10000>;
pinctrl-names = "default", "sleep";
pinctrl-0 = <ð_default>;
pinctrl-1 = <ð_sleep>;
diff --git a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
index a9cca9c146fd..9e850e04fffb 100644
--- a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
@@ -726,7 +726,7 @@ queue2 {
};
eth: ethernet@1101c000 {
- compatible = "mediatek,mt2712-gmac";
+ compatible = "mediatek,mt2712-gmac", "snps,dwmac-4.20a";
reg = <0 0x1101c000 0 0x1300>;
interrupts = <GIC_SPI 237 IRQ_TYPE_LEVEL_LOW>;
interrupt-names = "macirq";
@@ -734,15 +734,19 @@ eth: ethernet@1101c000 {
clock-names = "axi",
"apb",
"mac_main",
- "ptp_ref";
+ "ptp_ref",
+ "rmii_internal";
clocks = <&pericfg CLK_PERI_GMAC>,
<&pericfg CLK_PERI_GMAC_PCLK>,
<&topckgen CLK_TOP_ETHER_125M_SEL>,
- <&topckgen CLK_TOP_ETHER_50M_SEL>;
+ <&topckgen CLK_TOP_ETHER_50M_SEL>,
+ <&topckgen CLK_TOP_ETHER_50M_RMII_SEL>;
assigned-clocks = <&topckgen CLK_TOP_ETHER_125M_SEL>,
- <&topckgen CLK_TOP_ETHER_50M_SEL>;
+ <&topckgen CLK_TOP_ETHER_50M_SEL>,
+ <&topckgen CLK_TOP_ETHER_50M_RMII_SEL>;
assigned-clock-parents = <&topckgen CLK_TOP_ETHERPLL_125M>,
- <&topckgen CLK_TOP_APLL1_D3>;
+ <&topckgen CLK_TOP_APLL1_D3>,
+ <&topckgen CLK_TOP_ETHERPLL_50M>;
power-domains = <&scpsys MT2712_POWER_DOMAIN_AUDIO>;
mediatek,pericfg = <&pericfg>;
snps,axi-config = <&stmmac_axi_setup>;
--
2.25.1
^ permalink raw reply related
* [PATCH net-next v8 2/6] stmmac: dwmac-mediatek: Reuse more common features
From: Biao Huang @ 2021-12-10 1:31 UTC (permalink / raw)
To: davem, Jakub Kicinski, Rob Herring
Cc: Matthias Brugger, Giuseppe Cavallaro, Alexandre Torgue,
Jose Abreu, Maxime Coquelin, Biao Huang, netdev, devicetree,
linux-kernel, linux-arm-kernel, linux-mediatek, linux-stm32,
srv_heupstream, macpaul.lin, angelogioacchino.delregno, dkirjanov
In-Reply-To: <20211210013129.811-1-biao.huang@mediatek.com>
This patch makes dwmac-mediatek reuse more features
supported by stmmac_platform.c.
Signed-off-by: Biao Huang <biao.huang@mediatek.com>
Acked-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
.../ethernet/stmicro/stmmac/dwmac-mediatek.c | 32 +++++++++----------
1 file changed, 15 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
index 0ff57c268dca..8747aa4403e8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
@@ -334,22 +334,20 @@ static int mediatek_dwmac_init(struct platform_device *pdev, void *priv)
const struct mediatek_dwmac_variant *variant = plat->variant;
int ret;
- ret = dma_set_mask_and_coherent(plat->dev, DMA_BIT_MASK(variant->dma_bit_mask));
- if (ret) {
- dev_err(plat->dev, "No suitable DMA available, err = %d\n", ret);
- return ret;
- }
-
- ret = variant->dwmac_set_phy_interface(plat);
- if (ret) {
- dev_err(plat->dev, "failed to set phy interface, err = %d\n", ret);
- return ret;
+ if (variant->dwmac_set_phy_interface) {
+ ret = variant->dwmac_set_phy_interface(plat);
+ if (ret) {
+ dev_err(plat->dev, "failed to set phy interface, err = %d\n", ret);
+ return ret;
+ }
}
- ret = variant->dwmac_set_delay(plat);
- if (ret) {
- dev_err(plat->dev, "failed to set delay value, err = %d\n", ret);
- return ret;
+ if (variant->dwmac_set_delay) {
+ ret = variant->dwmac_set_delay(plat);
+ if (ret) {
+ dev_err(plat->dev, "failed to set delay value, err = %d\n", ret);
+ return ret;
+ }
}
ret = clk_bulk_prepare_enable(plat->num_clks_to_config, plat->clks);
@@ -422,15 +420,15 @@ static int mediatek_dwmac_probe(struct platform_device *pdev)
return PTR_ERR(plat_dat);
plat_dat->interface = priv_plat->phy_mode;
- plat_dat->has_gmac4 = 1;
- plat_dat->has_gmac = 0;
- plat_dat->pmt = 0;
+ plat_dat->use_phy_wol = 1;
plat_dat->riwt_off = 1;
plat_dat->maxmtu = ETH_DATA_LEN;
+ plat_dat->addr64 = priv_plat->variant->dma_bit_mask;
plat_dat->bsp_priv = priv_plat;
plat_dat->init = mediatek_dwmac_init;
plat_dat->exit = mediatek_dwmac_exit;
plat_dat->clks_config = mediatek_dwmac_clks_config;
+
mediatek_dwmac_init(pdev, priv_plat);
ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
--
2.25.1
^ permalink raw reply related
* [PATCH net-next v8 6/6] net: dt-bindings: dwmac: add support for mt8195
From: Biao Huang @ 2021-12-10 1:31 UTC (permalink / raw)
To: davem, Jakub Kicinski, Rob Herring
Cc: Matthias Brugger, Giuseppe Cavallaro, Alexandre Torgue,
Jose Abreu, Maxime Coquelin, Biao Huang, netdev, devicetree,
linux-kernel, linux-arm-kernel, linux-mediatek, linux-stm32,
srv_heupstream, macpaul.lin, angelogioacchino.delregno, dkirjanov
In-Reply-To: <20211210013129.811-1-biao.huang@mediatek.com>
Add binding document for the ethernet on mt8195.
Signed-off-by: Biao Huang <biao.huang@mediatek.com>
---
.../bindings/net/mediatek-dwmac.yaml | 86 +++++++++++++++----
1 file changed, 70 insertions(+), 16 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/mediatek-dwmac.yaml b/Documentation/devicetree/bindings/net/mediatek-dwmac.yaml
index 9207266a6e69..fb04166404d8 100644
--- a/Documentation/devicetree/bindings/net/mediatek-dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/mediatek-dwmac.yaml
@@ -19,11 +19,67 @@ select:
contains:
enum:
- mediatek,mt2712-gmac
+ - mediatek,mt8195-gmac
required:
- compatible
allOf:
- $ref: "snps,dwmac.yaml#"
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - mediatek,mt2712-gmac
+
+ then:
+ properties:
+ clocks:
+ minItems: 5
+ items:
+ - description: AXI clock
+ - description: APB clock
+ - description: MAC Main clock
+ - description: PTP clock
+ - description: RMII reference clock provided by MAC
+
+ clock-names:
+ minItems: 5
+ items:
+ - const: axi
+ - const: apb
+ - const: mac_main
+ - const: ptp_ref
+ - const: rmii_internal
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - mediatek,mt8195-gmac
+
+ then:
+ properties:
+ clocks:
+ minItems: 6
+ items:
+ - description: AXI clock
+ - description: APB clock
+ - description: MAC clock gate
+ - description: MAC Main clock
+ - description: PTP clock
+ - description: RMII reference clock provided by MAC
+
+ clock-names:
+ minItems: 6
+ items:
+ - const: axi
+ - const: apb
+ - const: mac_cg
+ - const: mac_main
+ - const: ptp_ref
+ - const: rmii_internal
properties:
compatible:
@@ -32,22 +88,10 @@ properties:
- enum:
- mediatek,mt2712-gmac
- const: snps,dwmac-4.20a
-
- clocks:
- items:
- - description: AXI clock
- - description: APB clock
- - description: MAC Main clock
- - description: PTP clock
- - description: RMII reference clock provided by MAC
-
- clock-names:
- items:
- - const: axi
- - const: apb
- - const: mac_main
- - const: ptp_ref
- - const: rmii_internal
+ - items:
+ - enum:
+ - mediatek,mt8195-gmac
+ - const: snps,dwmac-5.10a
mediatek,pericfg:
$ref: /schemas/types.yaml#/definitions/phandle
@@ -62,6 +106,8 @@ properties:
or will round down. Range 0~31*170.
For MT2712 RMII/MII interface, Allowed value need to be a multiple of 550,
or will round down. Range 0~31*550.
+ For MT8195 RGMII/RMII/MII interface, Allowed value need to be a multiple of 290,
+ or will round down. Range 0~31*290.
mediatek,rx-delay-ps:
description:
@@ -70,6 +116,8 @@ properties:
or will round down. Range 0~31*170.
For MT2712 RMII/MII interface, Allowed value need to be a multiple of 550,
or will round down. Range 0~31*550.
+ For MT8195 RGMII/RMII/MII interface, Allowed value need to be a multiple
+ of 290, or will round down. Range 0~31*290.
mediatek,rmii-rxc:
type: boolean
@@ -103,6 +151,12 @@ properties:
3. the inside clock, which be sent to MAC, will be inversed in RMII case when
the reference clock is from MAC.
+ mediatek,mac-wol:
+ type: boolean
+ description:
+ If present, indicates that MAC supports WOL(Wake-On-LAN), and MAC WOL will be enabled.
+ Otherwise, PHY WOL is perferred.
+
required:
- compatible
- reg
--
2.25.1
^ permalink raw reply related
* [PATCH net-next v8 4/6] net: dt-bindings: dwmac: Convert mediatek-dwmac to DT schema
From: Biao Huang @ 2021-12-10 1:31 UTC (permalink / raw)
To: davem, Jakub Kicinski, Rob Herring
Cc: Matthias Brugger, Giuseppe Cavallaro, Alexandre Torgue,
Jose Abreu, Maxime Coquelin, Biao Huang, netdev, devicetree,
linux-kernel, linux-arm-kernel, linux-mediatek, linux-stm32,
srv_heupstream, macpaul.lin, angelogioacchino.delregno, dkirjanov
In-Reply-To: <20211210013129.811-1-biao.huang@mediatek.com>
Convert mediatek-dwmac to DT schema, and delete old mediatek-dwmac.txt.
And there are some changes in .yaml than .txt, others almost keep the same:
1. compatible "const: snps,dwmac-4.20".
2. delete "snps,reset-active-low;" in example, since driver remove this
property long ago.
3. add "snps,reset-delay-us = <0 10000 10000>" in example.
4. the example is for rgmii interface, keep related properties only.
Signed-off-by: Biao Huang <biao.huang@mediatek.com>
---
.../bindings/net/mediatek-dwmac.txt | 91 ----------
.../bindings/net/mediatek-dwmac.yaml | 156 ++++++++++++++++++
2 files changed, 156 insertions(+), 91 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/net/mediatek-dwmac.txt
create mode 100644 Documentation/devicetree/bindings/net/mediatek-dwmac.yaml
diff --git a/Documentation/devicetree/bindings/net/mediatek-dwmac.txt b/Documentation/devicetree/bindings/net/mediatek-dwmac.txt
deleted file mode 100644
index afbcaebf062e..000000000000
--- a/Documentation/devicetree/bindings/net/mediatek-dwmac.txt
+++ /dev/null
@@ -1,91 +0,0 @@
-MediaTek DWMAC glue layer controller
-
-This file documents platform glue layer for stmmac.
-Please see stmmac.txt for the other unchanged properties.
-
-The device node has following properties.
-
-Required properties:
-- compatible: Should be "mediatek,mt2712-gmac" for MT2712 SoC
-- reg: Address and length of the register set for the device
-- interrupts: Should contain the MAC interrupts
-- interrupt-names: Should contain a list of interrupt names corresponding to
- the interrupts in the interrupts property, if available.
- Should be "macirq" for the main MAC IRQ
-- clocks: Must contain a phandle for each entry in clock-names.
-- clock-names: The name of the clock listed in the clocks property. These are
- "axi", "apb", "mac_main", "ptp_ref", "rmii_internal" for MT2712 SoC.
-- mac-address: See ethernet.txt in the same directory
-- phy-mode: See ethernet.txt in the same directory
-- mediatek,pericfg: A phandle to the syscon node that control ethernet
- interface and timing delay.
-
-Optional properties:
-- mediatek,tx-delay-ps: TX clock delay macro value. Default is 0.
- It should be defined for RGMII/MII interface.
- It should be defined for RMII interface when the reference clock is from MT2712 SoC.
-- mediatek,rx-delay-ps: RX clock delay macro value. Default is 0.
- It should be defined for RGMII/MII interface.
- It should be defined for RMII interface.
-Both delay properties need to be a multiple of 170 for RGMII interface,
-or will round down. Range 0~31*170.
-Both delay properties need to be a multiple of 550 for MII/RMII interface,
-or will round down. Range 0~31*550.
-
-- mediatek,rmii-rxc: boolean property, if present indicates that the RMII
- reference clock, which is from external PHYs, is connected to RXC pin
- on MT2712 SoC.
- Otherwise, is connected to TXC pin.
-- mediatek,rmii-clk-from-mac: boolean property, if present indicates that
- MT2712 SoC provides the RMII reference clock, which outputs to TXC pin only.
-- mediatek,txc-inverse: boolean property, if present indicates that
- 1. tx clock will be inversed in MII/RGMII case,
- 2. tx clock inside MAC will be inversed relative to reference clock
- which is from external PHYs in RMII case, and it rarely happen.
- 3. the reference clock, which outputs to TXC pin will be inversed in RMII case
- when the reference clock is from MT2712 SoC.
-- mediatek,rxc-inverse: boolean property, if present indicates that
- 1. rx clock will be inversed in MII/RGMII case.
- 2. reference clock will be inversed when arrived at MAC in RMII case, when
- the reference clock is from external PHYs.
- 3. the inside clock, which be sent to MAC, will be inversed in RMII case when
- the reference clock is from MT2712 SoC.
-- assigned-clocks: mac_main and ptp_ref clocks
-- assigned-clock-parents: parent clocks of the assigned clocks
-
-Example:
- eth: ethernet@1101c000 {
- compatible = "mediatek,mt2712-gmac";
- reg = <0 0x1101c000 0 0x1300>;
- interrupts = <GIC_SPI 237 IRQ_TYPE_LEVEL_LOW>;
- interrupt-names = "macirq";
- phy-mode ="rgmii-rxid";
- mac-address = [00 55 7b b5 7d f7];
- clock-names = "axi",
- "apb",
- "mac_main",
- "ptp_ref",
- "rmii_internal";
- clocks = <&pericfg CLK_PERI_GMAC>,
- <&pericfg CLK_PERI_GMAC_PCLK>,
- <&topckgen CLK_TOP_ETHER_125M_SEL>,
- <&topckgen CLK_TOP_ETHER_50M_SEL>,
- <&topckgen CLK_TOP_ETHER_50M_RMII_SEL>;
- assigned-clocks = <&topckgen CLK_TOP_ETHER_125M_SEL>,
- <&topckgen CLK_TOP_ETHER_50M_SEL>,
- <&topckgen CLK_TOP_ETHER_50M_RMII_SEL>;
- assigned-clock-parents = <&topckgen CLK_TOP_ETHERPLL_125M>,
- <&topckgen CLK_TOP_APLL1_D3>,
- <&topckgen CLK_TOP_ETHERPLL_50M>;
- power-domains = <&scpsys MT2712_POWER_DOMAIN_AUDIO>;
- mediatek,pericfg = <&pericfg>;
- mediatek,tx-delay-ps = <1530>;
- mediatek,rx-delay-ps = <1530>;
- mediatek,rmii-rxc;
- mediatek,txc-inverse;
- mediatek,rxc-inverse;
- snps,txpbl = <1>;
- snps,rxpbl = <1>;
- snps,reset-gpio = <&pio 87 GPIO_ACTIVE_LOW>;
- snps,reset-active-low;
- };
diff --git a/Documentation/devicetree/bindings/net/mediatek-dwmac.yaml b/Documentation/devicetree/bindings/net/mediatek-dwmac.yaml
new file mode 100644
index 000000000000..9207266a6e69
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/mediatek-dwmac.yaml
@@ -0,0 +1,156 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/mediatek-dwmac.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek DWMAC glue layer controller
+
+maintainers:
+ - Biao Huang <biao.huang@mediatek.com>
+
+description:
+ This file documents platform glue layer for stmmac.
+
+# We need a select here so we don't match all nodes with 'snps,dwmac'
+select:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - mediatek,mt2712-gmac
+ required:
+ - compatible
+
+allOf:
+ - $ref: "snps,dwmac.yaml#"
+
+properties:
+ compatible:
+ oneOf:
+ - items:
+ - enum:
+ - mediatek,mt2712-gmac
+ - const: snps,dwmac-4.20a
+
+ clocks:
+ items:
+ - description: AXI clock
+ - description: APB clock
+ - description: MAC Main clock
+ - description: PTP clock
+ - description: RMII reference clock provided by MAC
+
+ clock-names:
+ items:
+ - const: axi
+ - const: apb
+ - const: mac_main
+ - const: ptp_ref
+ - const: rmii_internal
+
+ mediatek,pericfg:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description:
+ The phandle to the syscon node that control ethernet
+ interface and timing delay.
+
+ mediatek,tx-delay-ps:
+ description:
+ The internal TX clock delay (provided by this driver) in nanoseconds.
+ For MT2712 RGMII interface, Allowed value need to be a multiple of 170,
+ or will round down. Range 0~31*170.
+ For MT2712 RMII/MII interface, Allowed value need to be a multiple of 550,
+ or will round down. Range 0~31*550.
+
+ mediatek,rx-delay-ps:
+ description:
+ The internal RX clock delay (provided by this driver) in nanoseconds.
+ For MT2712 RGMII interface, Allowed value need to be a multiple of 170,
+ or will round down. Range 0~31*170.
+ For MT2712 RMII/MII interface, Allowed value need to be a multiple of 550,
+ or will round down. Range 0~31*550.
+
+ mediatek,rmii-rxc:
+ type: boolean
+ description:
+ If present, indicates that the RMII reference clock, which is from external
+ PHYs, is connected to RXC pin. Otherwise, is connected to TXC pin.
+
+ mediatek,rmii-clk-from-mac:
+ type: boolean
+ description:
+ If present, indicates that MAC provides the RMII reference clock, which
+ outputs to TXC pin only.
+
+ mediatek,txc-inverse:
+ type: boolean
+ description:
+ If present, indicates that
+ 1. tx clock will be inversed in MII/RGMII case,
+ 2. tx clock inside MAC will be inversed relative to reference clock
+ which is from external PHYs in RMII case, and it rarely happen.
+ 3. the reference clock, which outputs to TXC pin will be inversed in RMII case
+ when the reference clock is from MAC.
+
+ mediatek,rxc-inverse:
+ type: boolean
+ description:
+ If present, indicates that
+ 1. rx clock will be inversed in MII/RGMII case.
+ 2. reference clock will be inversed when arrived at MAC in RMII case, when
+ the reference clock is from external PHYs.
+ 3. the inside clock, which be sent to MAC, will be inversed in RMII case when
+ the reference clock is from MAC.
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - interrupt-names
+ - clocks
+ - clock-names
+ - phy-mode
+ - mediatek,pericfg
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/mt2712-clk.h>
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/power/mt2712-power.h>
+
+ eth: ethernet@1101c000 {
+ compatible = "mediatek,mt2712-gmac", "snps,dwmac-4.20a";
+ reg = <0x1101c000 0x1300>;
+ interrupts = <GIC_SPI 237 IRQ_TYPE_LEVEL_LOW>;
+ interrupt-names = "macirq";
+ phy-mode ="rgmii-rxid";
+ mac-address = [00 55 7b b5 7d f7];
+ clock-names = "axi",
+ "apb",
+ "mac_main",
+ "ptp_ref",
+ "rmii_internal";
+ clocks = <&pericfg CLK_PERI_GMAC>,
+ <&pericfg CLK_PERI_GMAC_PCLK>,
+ <&topckgen CLK_TOP_ETHER_125M_SEL>,
+ <&topckgen CLK_TOP_ETHER_50M_SEL>,
+ <&topckgen CLK_TOP_ETHER_50M_RMII_SEL>;
+ assigned-clocks = <&topckgen CLK_TOP_ETHER_125M_SEL>,
+ <&topckgen CLK_TOP_ETHER_50M_SEL>,
+ <&topckgen CLK_TOP_ETHER_50M_RMII_SEL>;
+ assigned-clock-parents = <&topckgen CLK_TOP_ETHERPLL_125M>,
+ <&topckgen CLK_TOP_APLL1_D3>,
+ <&topckgen CLK_TOP_ETHERPLL_50M>;
+ power-domains = <&scpsys MT2712_POWER_DOMAIN_AUDIO>;
+ mediatek,pericfg = <&pericfg>;
+ mediatek,tx-delay-ps = <1530>;
+ snps,txpbl = <1>;
+ snps,rxpbl = <1>;
+ snps,reset-gpio = <&pio 87 GPIO_ACTIVE_LOW>;
+ snps,reset-delays-us = <0 10000 10000>;
+ };
--
2.25.1
^ permalink raw reply related
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