On Thu, Feb 10, 2022 at 01:33:04PM -0800, Veerasenareddy Burru wrote:
> Add support for ndo ops to set MAC address, change MTU, get stats.
> Add control path support to set MAC address, change MTU, get stats,
> set speed, get and set link mode.
>
> Signed-off-by: Veerasenareddy Burru <vburru@xxxxxxxxxxx>
> Signed-off-by: Abhijit Ayarekar <aayarekar@xxxxxxxxxxx>
> Signed-off-by: Satananda Burla <sburla@xxxxxxxxxxx>
> ---
> .../marvell/octeon_ep/octep_ctrl_net.c | 105 ++++++++++++++++++
> .../ethernet/marvell/octeon_ep/octep_main.c | 67 +++++++++++
> 2 files changed, 172 insertions(+)
Please don't put "." in end of patch title.
>
> diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_net.c b/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_net.c
> index 1f0d8ba3c8ee..be9b0f31c754 100644
> --- a/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_net.c
> +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_ctrl_net.c
> @@ -87,3 +87,108 @@ int octep_get_mac_addr(struct octep_device *oct, u8 *addr)
>
> return 0;
> }
> +
> +int octep_set_mac_addr(struct octep_device *oct, u8 *addr)
> +{
> + struct octep_ctrl_mbox_msg msg = { 0 };
> + struct octep_ctrl_net_h2f_req req = { 0 };
It is enough to write {} without 0.
> +
> + req.hdr.cmd = OCTEP_CTRL_NET_H2F_CMD_MAC;
> + req.mac.cmd = OCTEP_CTRL_NET_CMD_SET;
> + memcpy(&req.mac.addr, addr, ETH_ALEN);
> +
> + msg.hdr.flags = OCTEP_CTRL_MBOX_MSG_HDR_FLAG_REQ;
> + msg.hdr.sizew = OCTEP_CTRL_NET_H2F_MAC_REQ_SZW;
> + msg.msg = &req;
> + return octep_ctrl_mbox_send(&oct->ctrl_mbox, &msg);
> +}
> +
> +int octep_set_mtu(struct octep_device *oct, int mtu)
> +{
> + struct octep_ctrl_mbox_msg msg = { 0 };
> + struct octep_ctrl_net_h2f_req req = { 0 };
> +
> + req.hdr.cmd = OCTEP_CTRL_NET_H2F_CMD_MTU;
> + req.mtu.cmd = OCTEP_CTRL_NET_CMD_SET;
> + req.mtu.val = mtu;
> +
> + msg.hdr.flags = OCTEP_CTRL_MBOX_MSG_HDR_FLAG_REQ;
> + msg.hdr.sizew = OCTEP_CTRL_NET_H2F_MTU_REQ_SZW;
> + msg.msg = &req;
> + return octep_ctrl_mbox_send(&oct->ctrl_mbox, &msg);
> +}
> +
> +int octep_get_if_stats(struct octep_device *oct)
> +{
> + struct octep_ctrl_mbox_msg msg = { 0 };
> + struct octep_ctrl_net_h2f_req req = { 0 };
> + struct octep_iface_rx_stats *iface_rx_stats;
> + struct octep_iface_tx_stats *iface_tx_stats;
> + int err;
Reversed Christmas tree, in all functions.
> +
> + req.hdr.cmd = OCTEP_CTRL_NET_H2F_CMD_GET_IF_STATS;
> + req.mac.cmd = OCTEP_CTRL_NET_CMD_GET;
> + req.get_stats.offset = oct->ctrl_mbox_ifstats_offset;
> +
> + msg.hdr.flags = OCTEP_CTRL_MBOX_MSG_HDR_FLAG_REQ;
> + msg.hdr.sizew = OCTEP_CTRL_NET_H2F_GET_STATS_REQ_SZW;
> + msg.msg = &req;
> + err = octep_ctrl_mbox_send(&oct->ctrl_mbox, &msg);
> + if (!err) {
Please use success oriented approach, in all places.
if (err)
return err;
....
> + iface_rx_stats = (struct octep_iface_rx_stats *)(oct->ctrl_mbox.barmem +
> + oct->ctrl_mbox_ifstats_offset);
> + iface_tx_stats = (struct octep_iface_tx_stats *)(oct->ctrl_mbox.barmem +
> + oct->ctrl_mbox_ifstats_offset +
> + sizeof(struct octep_iface_rx_stats)
> + );
> + memcpy(&oct->iface_rx_stats, iface_rx_stats, sizeof(struct octep_iface_rx_stats));
> + memcpy(&oct->iface_tx_stats, iface_tx_stats, sizeof(struct octep_iface_tx_stats));
> + }
> +
> + return 0;
> +}
> +
Thanks
