Re: [PATCH net-next v8 2/6] net: dsa: Add convenience functions for frame handling
From: Vladimir Oltean <olteanv@gmail.com>
Date: 2022-09-11 13:27:26
On Fri, Sep 09, 2022 at 10:51:34AM +0200, Mattias Forsblad wrote:
Add common control functions for drivers that need to send and wait for control frames. Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Please no new lines between tags.
quoted hunk ↗ jump to hunk
Signed-off-by: Mattias Forsblad <redacted> --- include/net/dsa.h | 14 ++++++++++++++ net/dsa/dsa.c | 17 +++++++++++++++++ net/dsa/dsa2.c | 2 ++ 3 files changed, 33 insertions(+)diff --git a/include/net/dsa.h b/include/net/dsa.h index f2ce12860546..0e8a7ef17490 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h@@ -495,6 +495,8 @@ struct dsa_switch { unsigned int max_num_bridges; unsigned int num_ports; + + struct completion inband_done; }; static inline struct dsa_port *dsa_to_port(struct dsa_switch *ds, int p)@@ -1390,6 +1392,18 @@ void dsa_tag_drivers_register(struct dsa_tag_driver *dsa_tag_driver_array[], void dsa_tag_drivers_unregister(struct dsa_tag_driver *dsa_tag_driver_array[], unsigned int count); +int dsa_switch_inband_tx(struct dsa_switch *ds, struct sk_buff *skb, + struct completion *completion, unsigned long timeout); + +static inline void dsa_switch_inband_complete(struct dsa_switch *ds, struct completion *completion) +{ + /* Custom completion? */ + if (completion) + complete(completion); + else + complete(&ds->inband_done);
Could you please do the same thing here as what Paolo suggested: complete(completion ?: &ds->inband_done);
quoted hunk ↗ jump to hunk
+} + #define dsa_tag_driver_module_drivers(__dsa_tag_drivers_array, __count) \ static int __init dsa_tag_driver_module_init(void) \ { \diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c index be7b320cda76..ad870494d68b 100644 --- a/net/dsa/dsa.c +++ b/net/dsa/dsa.c@@ -324,6 +324,23 @@ int dsa_switch_resume(struct dsa_switch *ds) EXPORT_SYMBOL_GPL(dsa_switch_resume); #endif +int dsa_switch_inband_tx(struct dsa_switch *ds, struct sk_buff *skb, + struct completion *completion, unsigned long timeout) +{ + struct completion *com; + + /* Custom completion? */ + com = completion ? : &ds->inband_done; + + reinit_completion(com); + + if (skb) + dev_queue_xmit(skb); + + return wait_for_completion_timeout(com, msecs_to_jiffies(timeout)); +} +EXPORT_SYMBOL_GPL(dsa_switch_inband_tx); + static struct packet_type dsa_pack_type __read_mostly = { .type = cpu_to_be16(ETH_P_XDSA), .func = dsa_switch_rcv,diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c index ed56c7a554b8..a1b3ecfdffb8 100644 --- a/net/dsa/dsa2.c +++ b/net/dsa/dsa2.c@@ -1746,6 +1746,8 @@ static int dsa_switch_probe(struct dsa_switch *ds) dsa_tree_put(dst); } + init_completion(&ds->inband_done); +
This is actually done relatively late here (later than ds->ops->setup() is called), considering that we could use Ethernet based management even for general purpose register access. I would consider putting it right at the beginning of dsa_switch_setup(), after the "if (ds->setup)" check.
return err; } -- 2.25.1