* [PATCH] staging: octeon: refactor interface check logic in ethernet.c
@ 2021-02-21 14:55 Shreesh Adiga
2021-02-21 15:04 ` Greg Kroah-Hartman
0 siblings, 1 reply; 2+ messages in thread
From: Shreesh Adiga @ 2021-02-21 14:55 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: devel
The check for interface is duplicated in 3 places and has been refactored
into a function. Also the if condition was wrapping the whole body in all
three places, so it has been changed to return if the condition is false
to reduce the indentation levels.
Signed-off-by: Shreesh Adiga <16567adigashreesh@gmail.com>
---
drivers/staging/octeon/ethernet.c | 209 +++++++++++++++---------------
1 file changed, 102 insertions(+), 107 deletions(-)
diff --git a/drivers/staging/octeon/ethernet.c b/drivers/staging/octeon/ethernet.c
index 5dea6e96ec90..8357ce9bc11b 100644
--- a/drivers/staging/octeon/ethernet.c
+++ b/drivers/staging/octeon/ethernet.c
@@ -228,6 +228,12 @@ static struct net_device_stats *cvm_oct_common_get_stats(struct net_device *dev)
return &dev->stats;
}
+static int cvm_oct_validate_interface(int interface)
+{
+ return interface < 2 && (cvmx_helper_interface_get_mode(interface) !=
+ CVMX_HELPER_INTERFACE_MODE_SPI);
+}
+
/**
* cvm_oct_common_change_mtu - change the link MTU
* @dev: Device to change
@@ -245,42 +251,42 @@ static int cvm_oct_common_change_mtu(struct net_device *dev, int new_mtu)
int vlan_bytes = 0;
#endif
int mtu_overhead = ETH_HLEN + ETH_FCS_LEN + vlan_bytes;
+ int index;
+ /* Add ethernet header and FCS, and VLAN if configured. */
+ int max_packet = new_mtu + mtu_overhead;
dev->mtu = new_mtu;
- if ((interface < 2) &&
- (cvmx_helper_interface_get_mode(interface) !=
- CVMX_HELPER_INTERFACE_MODE_SPI)) {
- int index = INDEX(priv->port);
- /* Add ethernet header and FCS, and VLAN if configured. */
- int max_packet = new_mtu + mtu_overhead;
-
- if (OCTEON_IS_MODEL(OCTEON_CN3XXX) ||
- OCTEON_IS_MODEL(OCTEON_CN58XX)) {
- /* Signal errors on packets larger than the MTU */
- cvmx_write_csr(CVMX_GMXX_RXX_FRM_MAX(index, interface),
- max_packet);
- } else {
- /*
- * Set the hardware to truncate packets larger
- * than the MTU and smaller the 64 bytes.
- */
- union cvmx_pip_frm_len_chkx frm_len_chk;
-
- frm_len_chk.u64 = 0;
- frm_len_chk.s.minlen = VLAN_ETH_ZLEN;
- frm_len_chk.s.maxlen = max_packet;
- cvmx_write_csr(CVMX_PIP_FRM_LEN_CHKX(interface),
- frm_len_chk.u64);
- }
+ if (!cvm_oct_validate_interface(interface))
+ return 0;
+
+ index = INDEX(priv->port);
+
+ if (OCTEON_IS_MODEL(OCTEON_CN3XXX) || OCTEON_IS_MODEL(OCTEON_CN58XX)) {
+ /* Signal errors on packets larger than the MTU */
+ cvmx_write_csr(CVMX_GMXX_RXX_FRM_MAX(index, interface),
+ max_packet);
+ } else {
/*
- * Set the hardware to truncate packets larger than
- * the MTU. The jabber register must be set to a
- * multiple of 8 bytes, so round up.
+ * Set the hardware to truncate packets larger
+ * than the MTU and smaller the 64 bytes.
*/
- cvmx_write_csr(CVMX_GMXX_RXX_JABBER(index, interface),
- (max_packet + 7) & ~7u);
+ union cvmx_pip_frm_len_chkx frm_len_chk;
+
+ frm_len_chk.u64 = 0;
+ frm_len_chk.s.minlen = VLAN_ETH_ZLEN;
+ frm_len_chk.s.maxlen = max_packet;
+ cvmx_write_csr(CVMX_PIP_FRM_LEN_CHKX(interface),
+ frm_len_chk.u64);
}
+ /*
+ * Set the hardware to truncate packets larger than
+ * the MTU. The jabber register must be set to a
+ * multiple of 8 bytes, so round up.
+ */
+ cvmx_write_csr(CVMX_GMXX_RXX_JABBER(index, interface),
+ (max_packet + 7) & ~7u);
+
return 0;
}
@@ -293,51 +299,46 @@ static void cvm_oct_common_set_multicast_list(struct net_device *dev)
union cvmx_gmxx_prtx_cfg gmx_cfg;
struct octeon_ethernet *priv = netdev_priv(dev);
int interface = INTERFACE(priv->port);
+ union cvmx_gmxx_rxx_adr_ctl control;
+ int index;
- if ((interface < 2) &&
- (cvmx_helper_interface_get_mode(interface) !=
- CVMX_HELPER_INTERFACE_MODE_SPI)) {
- union cvmx_gmxx_rxx_adr_ctl control;
- int index = INDEX(priv->port);
-
- control.u64 = 0;
- control.s.bcst = 1; /* Allow broadcast MAC addresses */
-
- if (!netdev_mc_empty(dev) || (dev->flags & IFF_ALLMULTI) ||
- (dev->flags & IFF_PROMISC))
- /* Force accept multicast packets */
- control.s.mcst = 2;
- else
- /* Force reject multicast packets */
- control.s.mcst = 1;
-
- if (dev->flags & IFF_PROMISC)
- /*
- * Reject matches if promisc. Since CAM is
- * shut off, should accept everything.
- */
- control.s.cam_mode = 0;
- else
- /* Filter packets based on the CAM */
- control.s.cam_mode = 1;
-
- gmx_cfg.u64 =
- cvmx_read_csr(CVMX_GMXX_PRTX_CFG(index, interface));
- cvmx_write_csr(CVMX_GMXX_PRTX_CFG(index, interface),
- gmx_cfg.u64 & ~1ull);
-
- cvmx_write_csr(CVMX_GMXX_RXX_ADR_CTL(index, interface),
- control.u64);
- if (dev->flags & IFF_PROMISC)
- cvmx_write_csr(CVMX_GMXX_RXX_ADR_CAM_EN
- (index, interface), 0);
- else
- cvmx_write_csr(CVMX_GMXX_RXX_ADR_CAM_EN
- (index, interface), 1);
-
- cvmx_write_csr(CVMX_GMXX_PRTX_CFG(index, interface),
- gmx_cfg.u64);
- }
+ if (!cvm_oct_validate_interface(interface))
+ return;
+
+ index = INDEX(priv->port);
+
+ control.u64 = 0;
+ control.s.bcst = 1; /* Allow broadcast MAC addresses */
+
+ if (!netdev_mc_empty(dev) || (dev->flags & IFF_ALLMULTI) ||
+ (dev->flags & IFF_PROMISC))
+ /* Force accept multicast packets */
+ control.s.mcst = 2;
+ else
+ /* Force reject multicast packets */
+ control.s.mcst = 1;
+
+ if (dev->flags & IFF_PROMISC)
+ /*
+ * Reject matches if promisc. Since CAM is
+ * shut off, should accept everything.
+ */
+ control.s.cam_mode = 0;
+ else
+ /* Filter packets based on the CAM */
+ control.s.cam_mode = 1;
+
+ gmx_cfg.u64 = cvmx_read_csr(CVMX_GMXX_PRTX_CFG(index, interface));
+ cvmx_write_csr(CVMX_GMXX_PRTX_CFG(index, interface),
+ gmx_cfg.u64 & ~1ull);
+
+ cvmx_write_csr(CVMX_GMXX_RXX_ADR_CTL(index, interface), control.u64);
+ if (dev->flags & IFF_PROMISC)
+ cvmx_write_csr(CVMX_GMXX_RXX_ADR_CAM_EN(index, interface), 0);
+ else
+ cvmx_write_csr(CVMX_GMXX_RXX_ADR_CAM_EN(index, interface), 1);
+
+ cvmx_write_csr(CVMX_GMXX_PRTX_CFG(index, interface), gmx_cfg.u64);
}
static int cvm_oct_set_mac_filter(struct net_device *dev)
@@ -345,40 +346,34 @@ static int cvm_oct_set_mac_filter(struct net_device *dev)
struct octeon_ethernet *priv = netdev_priv(dev);
union cvmx_gmxx_prtx_cfg gmx_cfg;
int interface = INTERFACE(priv->port);
+ int i;
+ u64 mac = 0;
+ u8 *ptr;
+ int index;
+
+ if (!cvm_oct_validate_interface(interface))
+ return 0;
+
+ ptr = dev->dev_addr;
+ index = INDEX(priv->port);
+
+ for (i = 0; i < 6; i++)
+ mac = (mac << 8) | (u64)ptr[i];
+
+ gmx_cfg.u64 = cvmx_read_csr(CVMX_GMXX_PRTX_CFG(index, interface));
+ cvmx_write_csr(CVMX_GMXX_PRTX_CFG(index, interface),
+ gmx_cfg.u64 & ~1ull);
+
+ cvmx_write_csr(CVMX_GMXX_SMACX(index, interface), mac);
+ cvmx_write_csr(CVMX_GMXX_RXX_ADR_CAM0(index, interface), ptr[0]);
+ cvmx_write_csr(CVMX_GMXX_RXX_ADR_CAM1(index, interface), ptr[1]);
+ cvmx_write_csr(CVMX_GMXX_RXX_ADR_CAM2(index, interface), ptr[2]);
+ cvmx_write_csr(CVMX_GMXX_RXX_ADR_CAM3(index, interface), ptr[3]);
+ cvmx_write_csr(CVMX_GMXX_RXX_ADR_CAM4(index, interface), ptr[4]);
+ cvmx_write_csr(CVMX_GMXX_RXX_ADR_CAM5(index, interface), ptr[5]);
+ cvm_oct_common_set_multicast_list(dev);
+ cvmx_write_csr(CVMX_GMXX_PRTX_CFG(index, interface), gmx_cfg.u64);
- if ((interface < 2) &&
- (cvmx_helper_interface_get_mode(interface) !=
- CVMX_HELPER_INTERFACE_MODE_SPI)) {
- int i;
- u8 *ptr = dev->dev_addr;
- u64 mac = 0;
- int index = INDEX(priv->port);
-
- for (i = 0; i < 6; i++)
- mac = (mac << 8) | (u64)ptr[i];
-
- gmx_cfg.u64 =
- cvmx_read_csr(CVMX_GMXX_PRTX_CFG(index, interface));
- cvmx_write_csr(CVMX_GMXX_PRTX_CFG(index, interface),
- gmx_cfg.u64 & ~1ull);
-
- cvmx_write_csr(CVMX_GMXX_SMACX(index, interface), mac);
- cvmx_write_csr(CVMX_GMXX_RXX_ADR_CAM0(index, interface),
- ptr[0]);
- cvmx_write_csr(CVMX_GMXX_RXX_ADR_CAM1(index, interface),
- ptr[1]);
- cvmx_write_csr(CVMX_GMXX_RXX_ADR_CAM2(index, interface),
- ptr[2]);
- cvmx_write_csr(CVMX_GMXX_RXX_ADR_CAM3(index, interface),
- ptr[3]);
- cvmx_write_csr(CVMX_GMXX_RXX_ADR_CAM4(index, interface),
- ptr[4]);
- cvmx_write_csr(CVMX_GMXX_RXX_ADR_CAM5(index, interface),
- ptr[5]);
- cvm_oct_common_set_multicast_list(dev);
- cvmx_write_csr(CVMX_GMXX_PRTX_CFG(index, interface),
- gmx_cfg.u64);
- }
return 0;
}
--
2.30.0
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] staging: octeon: refactor interface check logic in ethernet.c
2021-02-21 14:55 [PATCH] staging: octeon: refactor interface check logic in ethernet.c Shreesh Adiga
@ 2021-02-21 15:04 ` Greg Kroah-Hartman
0 siblings, 0 replies; 2+ messages in thread
From: Greg Kroah-Hartman @ 2021-02-21 15:04 UTC (permalink / raw)
To: Shreesh Adiga; +Cc: devel
On Sun, Feb 21, 2021 at 08:25:05PM +0530, Shreesh Adiga wrote:
> The check for interface is duplicated in 3 places and has been refactored
> into a function. Also the if condition was wrapping the whole body in all
> three places, so it has been changed to return if the condition is false
> to reduce the indentation levels.
When you have an "also" in a changelog text, that's a huge hint this
needs to be broken up into multiple patches.
Please do that here and send a patch series.
thanks,
greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-02-21 15:04 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-21 14:55 [PATCH] staging: octeon: refactor interface check logic in ethernet.c Shreesh Adiga
2021-02-21 15:04 ` Greg Kroah-Hartman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).