Re: [net-next 1/6] net: marvell: prestera: Add driver for Prestera family ASIC devices
From: Vadym Kochan
Date: Mon Jun 01 2020 - 06:50:27 EST
Hi Ido,
On Sat, May 30, 2020 at 06:48:01PM +0300, Ido Schimmel wrote:
> On Thu, May 28, 2020 at 06:12:40PM +0300, Vadym Kochan wrote:
>
[...]
> Nit: "From" ?
>
> > + PRESTERA_DSA_CMD_FROM_CPU,
> > +};
> > +
> > +struct prestera_dsa_vlan {
> > + u16 vid;
> > + u8 vpt;
> > + u8 cfi_bit;
> > + bool is_tagged;
> > +};
> > +
> > +struct prestera_dsa {
> > + struct prestera_dsa_vlan vlan;
> > + u32 hw_dev_num;
> > + u32 port_num;
> > +};
> > +
> > +int prestera_dsa_parse(struct prestera_dsa *dsa, const u8 *dsa_buf);
> > +int prestera_dsa_build(const struct prestera_dsa *dsa, u8 *dsa_buf);
> > +
> > +#endif /* _PRESTERA_DSA_H_ */
> > diff --git a/drivers/net/ethernet/marvell/prestera/prestera_hw.c b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
> > new file mode 100644
> > index 000000000000..3aa3974f957a
> > --- /dev/null
> > +++ b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
> > @@ -0,0 +1,610 @@
> > +// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
> > +/* Copyright (c) 2019-2020 Marvell International Ltd. All rights reserved */
> > +
> > +#include <linux/etherdevice.h>
> > +#include <linux/ethtool.h>
> > +#include <linux/netdevice.h>
> > +#include <linux/list.h>
> > +
> > +#include "prestera.h"
> > +#include "prestera_hw.h"
> > +
> > +#define PRESTERA_SWITCH_INIT_TIMEOUT 30000000 /* 30sec */
>
> Out of curiosity, how long does it actually take you to initialize the
> hardware?
>
> Also, I find it useful to note the units in the name, so:
>
> #define PRESTERA_SWITCH_INIT_TIMEOUT_US (30 * 1000 * 1000)
>
> BTW, it says 30 seconds in comment, but the call chain where it is used
> is:
>
> prestera_cmd_ret_wait(, PRESTERA_SWITCH_INIT_TIMEOUT)
> __prestera_cmd_ret(..., wait)
> prestera_fw_send_req(..., waitms)
> prestera_fw_cmd_send(..., waitms)
> prestera_fw_wait_reg32(..., waitms)
> readl_poll_timeout(..., waitms * 1000)
>
> So I think you should actually define it as:
>
> #define PRESTERA_SWITCH_INIT_TIMEOUT_MS (30 * 1000)
>
> And rename all these 'wait' arguments to 'waitms' so it's clearer which
> unit they expect.
>
[...]
> > +static int __prestera_cmd_ret(struct prestera_switch *sw,
> > + enum prestera_cmd_type_t type,
> > + struct prestera_msg_cmd *cmd, size_t clen,
> > + struct prestera_msg_ret *ret, size_t rlen,
> > + int wait)
> > +{
> > + struct prestera_device *dev = sw->dev;
> > + int err;
> > +
> > + cmd->type = type;
> > +
> > + err = dev->send_req(dev, (u8 *)cmd, clen, (u8 *)ret, rlen, wait);
> > + if (err)
> > + return err;
> > +
> > + if (ret->cmd.type != PRESTERA_CMD_TYPE_ACK)
> > + return -EBADE;
> > + if (ret->status != PRESTERA_CMD_ACK_OK)
>
> You don't have more states here other than OK / FAIL ? It might help you
> in debugging if you include them. You might find trace_devlink_hwerr()
> useful.
Thanks, I will consider this.
>
> > + return -EINVAL;
> > +
> > + return 0;
> > +}
> > +
> > +static int prestera_cmd_ret(struct prestera_switch *sw,
> > + enum prestera_cmd_type_t type,
> > + struct prestera_msg_cmd *cmd, size_t clen,
> > + struct prestera_msg_ret *ret, size_t rlen)
> > +{
> > + return __prestera_cmd_ret(sw, type, cmd, clen, ret, rlen, 0);
> > +}
> > +
> > +static int prestera_cmd_ret_wait(struct prestera_switch *sw,
> > + enum prestera_cmd_type_t type,
> > + struct prestera_msg_cmd *cmd, size_t clen,
> > + struct prestera_msg_ret *ret, size_t rlen,
> > + int wait)
> > +{
> > + return __prestera_cmd_ret(sw, type, cmd, clen, ret, rlen, wait);
> > +}
> > +
> > +static int prestera_cmd(struct prestera_switch *sw,
> > + enum prestera_cmd_type_t type,
> > + struct prestera_msg_cmd *cmd, size_t clen)
> > +{
> > + struct prestera_msg_common_resp resp;
> > +
> > + return prestera_cmd_ret(sw, type, cmd, clen, &resp.ret, sizeof(resp));
> > +}
> > +
> > +static int prestera_fw_parse_port_evt(u8 *msg, struct prestera_event *evt)
> > +{
> > + struct prestera_msg_event_port *hw_evt;
> > +
> > + hw_evt = (struct prestera_msg_event_port *)msg;
> > +
> > + evt->port_evt.port_id = hw_evt->port_id;
> > +
> > + if (evt->id == PRESTERA_PORT_EVENT_STATE_CHANGED)
> > + evt->port_evt.data.oper_state = hw_evt->param.oper_state;
> > + else
> > + return -EINVAL;
> > +
> > + return 0;
> > +}
> > +
> > +static struct prestera_fw_evt_parser {
> > + int (*func)(u8 *msg, struct prestera_event *evt);
> > +} fw_event_parsers[PRESTERA_EVENT_TYPE_MAX] = {
> > + [PRESTERA_EVENT_TYPE_PORT] = {.func = prestera_fw_parse_port_evt},
> > +};
> > +
> > +static struct prestera_fw_event_handler *
> > +__find_event_handler(const struct prestera_switch *sw,
> > + enum prestera_event_type type)
> > +{
> > + struct prestera_fw_event_handler *eh;
> > +
> > + list_for_each_entry_rcu(eh, &sw->event_handlers, list) {
>
> It does not look that this is always called under RCU which will result
> in various splats. For example in the following call path:
>
> prestera_device_register()
> prestera_switch_init()
> prestera_event_handlers_register()
> prestera_hw_event_handler_register()
> __find_event_handler()
>
> You want to make sure that you are testing with various debug options.
> For example:
So, right. Currently this prestera_hw_event_handler_register is called
synchronously and as I understand does not require additional locking
when use list rcu API. I will check with these options which you
suggested.
>
> # Debug options
> ## General debug options
> config_enable CONFIG_PREEMPT
> config_enable CONFIG_DEBUG_PREEMPT
> config_enable CONFIG_DEBUG_INFO
> config_enable CONFIG_UNWINDER_ORC
> config_enable CONFIG_DYNAMIC_DEBUG
> config_enable CONFIG_DEBUG_NOTIFIERS
> ## Lock debugging
> config_enable CONFIG_LOCKDEP
> config_enable CONFIG_PROVE_LOCKING
> config_enable CONFIG_DEBUG_ATOMIC_SLEEP
> config_enable CONFIG_PROVE_RCU
> config_enable CONFIG_DEBUG_MUTEXES
> config_enable CONFIG_DEBUG_SPINLOCK
> config_enable CONFIG_LOCK_STAT
> ## Memory debugging
> config_enable CONFIG_DEBUG_VM
> config_enable CONFIG_FORTIFY_SOURCE
> config_enable CONFIG_KASAN
> config_enable CONFIG_KASAN_EXTRA
> config_enable CONFIG_KASAN_INLINE
> ## Reference counting debugging
> config_enable CONFIG_REFCOUNT_FULL
> ## Lockups debugging
> config_enable CONFIG_LOCKUP_DETECTOR
> config_enable CONFIG_SOFTLOCKUP_DETECTOR
> config_enable CONFIG_HARDLOCKUP_DETECTOR
> config_enable CONFIG_DETECT_HUNG_TASK
> config_enable CONFIG_WQ_WATCHDOG
> config_enable CONFIG_DETECT_HUNG_TASK
> config_set_val CONFIG_DEFAULT_HUNG_TASK_TIMEOUT 120
> ## Undefined behavior debugging
> config_enable CONFIG_UBSAN
> config_enable CONFIG_UBSAN_SANITIZE_ALL
> config_disable CONFIG_UBSAN_ALIGNMENT
> config_disable CONFIG_UBSAN_NULL
> ## Memory debugging
> config_enable CONFIG_SLUB_DEBUG
> config_enable CONFIG_SLUB_DEBUG_ON
> config_enable CONFIG_DEBUG_PAGEALLOC
> config_enable CONFIG_DEBUG_KMEMLEAK
> config_disable CONFIG_DEBUG_KMEMLEAK_DEFAULT_OFF
> config_set_val CONFIG_DEBUG_KMEMLEAK_EARLY_LOG_SIZE 8192
> config_enable CONFIG_DEBUG_STACKOVERFLOW
> config_enable CONFIG_DEBUG_LIST
> config_enable CONFIG_DEBUG_PER_CPU_MAPS
> config_set_val CONFIG_DEBUG_OBJECTS_ENABLE_DEFAULT 1
> config_enable CONFIG_DEBUG_OBJECTS
> config_enable CONFIG_DEBUG_OBJECTS_FREE
> config_enable CONFIG_DEBUG_OBJECTS_TIMERS
> config_enable CONFIG_DEBUG_OBJECTS_WORK
> config_enable CONFIG_DEBUG_OBJECTS_PERCPU_COUNTER
> config_enable CONFIG_DMA_API_DEBUG
> ## Lock debugging
> config_enable CONFIG_DEBUG_LOCK_ALLOC
> config_enable CONFIG_PROVE_LOCKING
> config_enable CONFIG_LOCK_STAT
> config_enable CONFIG_DEBUG_OBJECTS_RCU_HEAD
> config_enable CONFIG_SPARSE_RCU_POINTER
>
> > + if (eh->type == type)
> > + return eh;
> > + }
> > +
> > + return NULL;
> > +}
> > +
> > +static int prestera_find_event_handler(const struct prestera_switch *sw,
> > + enum prestera_event_type type,
> > + struct prestera_fw_event_handler *eh)
> > +{
> > + struct prestera_fw_event_handler *tmp;
> > + int err = 0;
> > +
> > + rcu_read_lock();
> > + tmp = __find_event_handler(sw, type);
> > + if (tmp)
> > + *eh = *tmp;
> > + else
> > + err = -EEXIST;
> > + rcu_read_unlock();
> > +
> > + return err;
> > +}
> > +
> > +static int prestera_evt_recv(struct prestera_device *dev, u8 *buf, size_t size)
> > +{
> > + struct prestera_msg_event *msg = (struct prestera_msg_event *)buf;
> > + struct prestera_switch *sw = dev->priv;
> > + struct prestera_fw_event_handler eh;
> > + struct prestera_event evt;
> > + int err;
> > +
> > + if (msg->type >= PRESTERA_EVENT_TYPE_MAX)
> > + return -EINVAL;
> > +
> > + err = prestera_find_event_handler(sw, msg->type, &eh);
> > +
> > + if (err || !fw_event_parsers[msg->type].func)
> > + return 0;
> > +
> > + evt.id = msg->id;
> > +
> > + err = fw_event_parsers[msg->type].func(buf, &evt);
> > + if (!err)
> > + eh.func(sw, &evt, eh.arg);
> > +
> > + return err;
> > +}
> > +
> > +static void prestera_pkt_recv(struct prestera_device *dev)
> > +{
> > + struct prestera_switch *sw = dev->priv;
> > + struct prestera_fw_event_handler eh;
> > + struct prestera_event ev;
> > + int err;
> > +
> > + ev.id = PRESTERA_RXTX_EVENT_RCV_PKT;
> > +
> > + err = prestera_find_event_handler(sw, PRESTERA_EVENT_TYPE_RXTX, &eh);
> > + if (err)
> > + return;
> > +
> > + eh.func(sw, &ev, eh.arg);
> > +}
> > +
> > +int prestera_hw_port_info_get(const struct prestera_port *port,
> > + u16 *fp_id, u32 *hw_id, u32 *dev_id)
> > +{
> > + struct prestera_msg_port_info_resp resp;
> > + struct prestera_msg_port_info_req req = {
> > + .port = port->id
> > + };
> > + int err;
> > +
> > + err = prestera_cmd_ret(port->sw, PRESTERA_CMD_TYPE_PORT_INFO_GET,
> > + &req.cmd, sizeof(req), &resp.ret, sizeof(resp));
> > + if (err)
> > + return err;
> > +
> > + *hw_id = resp.hw_id;
> > + *dev_id = resp.dev_id;
> > + *fp_id = resp.fp_id;
> > +
> > + return 0;
> > +}
> > +
> > +int prestera_hw_switch_mac_set(struct prestera_switch *sw, char *mac)
> > +{
> > + struct prestera_msg_switch_attr_req req = {
> > + .attr = PRESTERA_CMD_SWITCH_ATTR_MAC,
> > + };
> > +
> > + memcpy(req.param.mac, mac, sizeof(req.param.mac));
> > +
> > + return prestera_cmd(sw, PRESTERA_CMD_TYPE_SWITCH_ATTR_SET,
> > + &req.cmd, sizeof(req));
> > +}
> > +
> > +int prestera_hw_switch_init(struct prestera_switch *sw)
> > +{
> > + struct prestera_msg_switch_init_resp resp;
> > + struct prestera_msg_common_req req;
> > + int err;
> > +
> > + INIT_LIST_HEAD(&sw->event_handlers);
> > +
> > + err = prestera_cmd_ret_wait(sw, PRESTERA_CMD_TYPE_SWITCH_INIT,
> > + &req.cmd, sizeof(req),
> > + &resp.ret, sizeof(resp),
> > + PRESTERA_SWITCH_INIT_TIMEOUT);
> > + if (err)
> > + return err;
> > +
> > + sw->id = resp.switch_id;
> > + sw->port_count = resp.port_count;
> > + sw->mtu_min = PRESTERA_MIN_MTU;
> > + sw->mtu_max = resp.mtu_max;
> > + sw->dev->recv_msg = prestera_evt_recv;
> > + sw->dev->recv_pkt = prestera_pkt_recv;
> > +
> > + return 0;
> > +}
>
> Consider adding prestera_hw_switch_fini() that verifies that
> '&sw->event_handlers' is empty.
>
As I see it can just warn on if list is not empty, right ?
> > +
> > +int prestera_hw_port_state_set(const struct prestera_port *port,
> > + bool admin_state)
> > +{
> > + struct prestera_msg_port_attr_req req = {
> > + .attr = PRESTERA_CMD_PORT_ATTR_ADMIN_STATE,
> > + .port = port->hw_id,
> > + .dev = port->dev_id,
> > + .param = {.admin_state = admin_state}
> > + };
> > +
> > + return prestera_cmd(port->sw, PRESTERA_CMD_TYPE_PORT_ATTR_SET,
> > + &req.cmd, sizeof(req));
> > +}
> > +
[...]
> > +
> > +struct prestera_port *prestera_port_find_by_hwid(struct prestera_switch *sw,
> > + u32 dev_id, u32 hw_id)
> > +{
> > + struct prestera_port *port;
> > +
> > + rcu_read_lock();
> > +
> > + list_for_each_entry_rcu(port, &sw->port_list, list) {
> > + if (port->dev_id == dev_id && port->hw_id == hw_id) {
> > + rcu_read_unlock();
> > + return port;
>
> This does not look correct. You call rcu_read_unlock(), but do not take
> a reference on the object, so nothing prevents it from being freed.
>
Currently there is no logic which can dynamically create/delete the
port, so how do you think may I just use synchronize_rcu() when freeing
the port instance on module unlolad ?
> > + }
> > + }
> > +
> > + rcu_read_unlock();
> > +
> > + return NULL;
> > +}
> > +
> > +static struct prestera_port *prestera_find_port(struct prestera_switch *sw,
> > + u32 port_id)
> > +{
> > + struct prestera_port *port;
> > +
> > + rcu_read_lock();
> > +
> > + list_for_each_entry_rcu(port, &sw->port_list, list) {
> > + if (port->id == port_id)
> > + break;
> > + }
> > +
> > + rcu_read_unlock();
> > +
> > + return port;
>
> Same here.
>
> > +}
> > +
> > +static int prestera_port_state_set(struct net_device *dev, bool is_up)
> > +{
> > + struct prestera_port *port = netdev_priv(dev);
> > + int err;
> > +
> > + if (!is_up)
> > + netif_stop_queue(dev);
> > +
> > + err = prestera_hw_port_state_set(port, is_up);
> > +
> > + if (is_up && !err)
> > + netif_start_queue(dev);
> > +
> > + return err;
> > +}
> > +
[...]
> > +static void prestera_port_destroy(struct prestera_port *port)
> > +{
> > + struct net_device *dev = port->dev;
> > +
> > + cancel_delayed_work_sync(&port->cached_hw_stats.caching_dw);
> > + unregister_netdev(dev);
> > +
> > + list_del_rcu(&port->list);
> > +
>
> I'm not sure what is the point of these blank lines. Best to remove
> them.
Will fix it.
>
> > + free_netdev(dev);
> > +}
> > +
> > +static void prestera_destroy_ports(struct prestera_switch *sw)
> > +{
> > + struct prestera_port *port, *tmp;
> > + struct list_head remove_list;
> > +
> > + INIT_LIST_HEAD(&remove_list);
> > +
> > + list_splice_init(&sw->port_list, &remove_list);
> > +
> > + list_for_each_entry_safe(port, tmp, &remove_list, list)
> > + prestera_port_destroy(port);
> > +}
> > +
> > +static int prestera_create_ports(struct prestera_switch *sw)
> > +{
> > + u32 port;
> > + int err;
> > +
> > + for (port = 0; port < sw->port_count; port++) {
> > + err = prestera_port_create(sw, port);
> > + if (err)
> > + goto err_ports_init;
>
> err_port_create ?
>
> > + }
> > +
> > + return 0;
> > +
> > +err_ports_init:
> > + prestera_destroy_ports(sw);
>
> I'm not a fan of this construct. I find it best to always do proper
> rollback in the error path. Then you can always maintain init() being
> followed by fini() which is much easier to review.
As I understand you meant to move this destroy_ports() recovery to the
error path in xxx_switch_init() ?
>
> > + return err;
> > +}
> > +
> > +static void prestera_port_handle_event(struct prestera_switch *sw,
> > + struct prestera_event *evt, void *arg)
> > +{
> > + struct delayed_work *caching_dw;
> > + struct prestera_port *port;
> > +
> > + port = prestera_find_port(sw, evt->port_evt.port_id);
> > + if (!port)
> > + return;
> > +
> > + caching_dw = &port->cached_hw_stats.caching_dw;
> > +
> > + if (evt->id == PRESTERA_PORT_EVENT_STATE_CHANGED) {
> > + if (evt->port_evt.data.oper_state) {
> > + netif_carrier_on(port->dev);
> > + if (!delayed_work_pending(caching_dw))
> > + queue_delayed_work(prestera_wq, caching_dw, 0);
> > + } else {
> > + netif_carrier_off(port->dev);
> > + if (delayed_work_pending(caching_dw))
> > + cancel_delayed_work(caching_dw);
> > + }
> > + }
> > +}
> > +
> > +static void prestera_event_handlers_unregister(struct prestera_switch *sw)
> > +{
> > + prestera_hw_event_handler_unregister(sw, PRESTERA_EVENT_TYPE_PORT,
> > + prestera_port_handle_event);
> > +}
>
> Please reverse the order so that register() is first.
>
> > +
> > +static int prestera_event_handlers_register(struct prestera_switch *sw)
> > +{
> > + return prestera_hw_event_handler_register(sw, PRESTERA_EVENT_TYPE_PORT,
> > + prestera_port_handle_event,
> > + NULL);
> > +}
> > +
> > +static int prestera_switch_set_base_mac_addr(struct prestera_switch *sw)
> > +{
> > + struct device_node *base_mac_np;
> > + struct device_node *np;
> > +
> > + np = of_find_compatible_node(NULL, NULL, "marvell,prestera");
> > + if (np) {
> > + base_mac_np = of_parse_phandle(np, "base-mac-provider", 0);
> > + if (base_mac_np) {
> > + const char *base_mac;
> > +
> > + base_mac = of_get_mac_address(base_mac_np);
> > + of_node_put(base_mac_np);
> > + if (!IS_ERR(base_mac))
> > + ether_addr_copy(sw->base_mac, base_mac);
> > + }
> > + }
> > +
> > + if (!is_valid_ether_addr(sw->base_mac)) {
> > + eth_random_addr(sw->base_mac);
> > + dev_info(sw->dev->dev, "using random base mac address\n");
> > + }
> > +
> > + return prestera_hw_switch_mac_set(sw, sw->base_mac);
> > +}
> > +
> > +static int prestera_switch_init(struct prestera_switch *sw)
> > +{
> > + int err;
> > +
> > + err = prestera_hw_switch_init(sw);
> > + if (err) {
> > + dev_err(prestera_dev(sw), "Failed to init Switch device\n");
> > + return err;
> > + }
> > +
> > + INIT_LIST_HEAD(&sw->port_list);
> > +
> > + err = prestera_switch_set_base_mac_addr(sw);
> > + if (err)
> > + return err;
> > +
> > + err = prestera_rxtx_switch_init(sw);
> > + if (err)
> > + return err;
> > +
> > + err = prestera_event_handlers_register(sw);
> > + if (err)
> > + return err;
> > +
> > + err = prestera_create_ports(sw);
> > + if (err)
> > + goto err_ports_create;
> > +
> > + return 0;
> > +
> > +err_ports_create:
> > + prestera_event_handlers_unregister(sw);
> > +
>
> You are missing prestera_rxtx_switch_fini() here... With init() always
> followed by fini() you can easily tell that the error path is not
> symmetric with fini().
Yeah, for some reason I mixed this fix with Switchdev patch, I will fix
this.
>
> > + return err;
> > +}
> > +
> > +static void prestera_switch_fini(struct prestera_switch *sw)
> > +{
> > + prestera_destroy_ports(sw);
> > + prestera_event_handlers_unregister(sw);
> > + prestera_rxtx_switch_fini(sw);
> > +}
> > +
> > +int prestera_device_register(struct prestera_device *dev)
> > +{
> > + struct prestera_switch *sw;
> > + int err;
> > +
> > + sw = kzalloc(sizeof(*sw), GFP_KERNEL);
> > + if (!sw)
> > + return -ENOMEM;
> > +
> > + dev->priv = sw;
> > + sw->dev = dev;
> > +
> > + err = prestera_switch_init(sw);
> > + if (err) {
> > + kfree(sw);
> > + return err;
> > + }
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(prestera_device_register);
> > +
> > +void prestera_device_unregister(struct prestera_device *dev)
> > +{
> > + struct prestera_switch *sw = dev->priv;
> > +
> > + prestera_switch_fini(sw);
> > + kfree(sw);
> > +}
[...]
> > +int prestera_rxtx_port_init(struct prestera_port *port)
> > +{
> > + int err;
> > +
> > + err = prestera_hw_rxtx_port_init(port);
> > + if (err)
> > + return err;
> > +
> > + port->dev->needed_headroom = PRESTERA_DSA_HLEN + ETH_FCS_LEN;
>
> Why do you need headroom for FCS?
>
I had issue when the SKB did not have additional bytes for the FCS, so I
thought to added this to the needed_headroom to be sure.
> > + return 0;
> > +}
> > +
> > +netdev_tx_t prestera_rxtx_xmit(struct prestera_port *port, struct sk_buff *skb)
> > +{
> > + struct prestera_dsa dsa;
> > +
> > + dsa.hw_dev_num = port->dev_id;
> > + dsa.port_num = port->hw_id;
> > +
> > + if (skb_cow_head(skb, PRESTERA_DSA_HLEN) < 0)
> > + return NET_XMIT_DROP;
> > +
> > + skb_push(skb, PRESTERA_DSA_HLEN);
> > + memmove(skb->data, skb->data + PRESTERA_DSA_HLEN, 2 * ETH_ALEN);
> > +
> > + if (prestera_dsa_build(&dsa, skb->data + 2 * ETH_ALEN) != 0)
> > + return NET_XMIT_DROP;
> > +
> > + return prestera_sdma_xmit(&port->sw->rxtx->sdma, skb);
> > +}
> > diff --git a/drivers/net/ethernet/marvell/prestera/prestera_rxtx.h b/drivers/net/ethernet/marvell/prestera/prestera_rxtx.h
> > new file mode 100644
> > index 000000000000..bbbadfa5accf
> > --- /dev/null
> > +++ b/drivers/net/ethernet/marvell/prestera/prestera_rxtx.h
> > @@ -0,0 +1,21 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
> > + *
> > + * Copyright (c) 2019-2020 Marvell International Ltd. All rights reserved.
> > + *
> > + */
> > +
> > +#ifndef _PRESTERA_RXTX_H_
> > +#define _PRESTERA_RXTX_H_
> > +
> > +#include <linux/netdevice.h>
> > +
> > +#include "prestera.h"
> > +
> > +int prestera_rxtx_switch_init(struct prestera_switch *sw);
> > +void prestera_rxtx_switch_fini(struct prestera_switch *sw);
> > +
> > +int prestera_rxtx_port_init(struct prestera_port *port);
> > +
> > +netdev_tx_t prestera_rxtx_xmit(struct prestera_port *port, struct sk_buff *skb);
> > +
> > +#endif /* _PRESTERA_RXTX_H_ */
> > --
> > 2.17.1
> >
Thank you for review.