2017-08-05 11:06:58

by Egil Hjelmeland

[permalink] [raw]
Subject: [PATCH v4 net-next 0/5] Refactor lan9303_xxx_packet_processing

This series is purely non functional.

It changes the lan9303_enable_packet_processing,
lan9303_disable_packet_processing() to pass port number (0,1,2) as
parameter instead of port offset. This aligns them with
other functions in the module, and makes it possible to simplify the code.

The lan9303_enable_packet_processing, lan9303_disable_packet_processing
functions operate on port. Therefore rename the functions to reflect that
as well.

Reviewer pointed out lan9303_get_ethtool_stats would be better off with
the use of a lan9303_read_switch_port(). So that was added to the series.


Changes v3 -> v4:
- Whitespace adjustments.

Changes v2 -> v3:
- Patch 1: Removed the change in lan9303_get_ethtool_stats
- Added patch 4: rename lan9303_xxx_packet_processing
- Added patch 5: refactor lan9303_get_ethtool_stats

Changes v1 -> v2:
- introduced lan9303_write_switch_port() in first patch
- inserted LAN9303_NUM_PORTS patch
- Use LAN9303_NUM_PORTS in last patch. Plus whitespace change.

Egil Hjelmeland (5):
net: dsa: lan9303: Change lan9303_xxx_packet_processing() port param.
net: dsa: lan9303: define LAN9303_NUM_PORTS 3
net: dsa: lan9303: Simplify lan9303_xxx_packet_processing() usage
net: dsa: lan9303: Rename lan9303_xxx_packet_processing()
net: dsa: lan9303: refactor lan9303_get_ethtool_stats

drivers/net/dsa/lan9303-core.c | 96 ++++++++++++++++++++++--------------------
1 file changed, 50 insertions(+), 46 deletions(-)

--
2.11.0


2017-08-05 11:06:59

by Egil Hjelmeland

[permalink] [raw]
Subject: [PATCH v4 net-next 1/5] net: dsa: lan9303: Change lan9303_xxx_packet_processing() port param.

lan9303_enable_packet_processing, lan9303_disable_packet_processing()
Pass port number (0,1,2) as parameter instead of port offset.
Because other functions in the module pass port numbers.
And to enable simplifications in following patch.

Introduce lan9303_write_switch_port().

Signed-off-by: Egil Hjelmeland <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
---
drivers/net/dsa/lan9303-core.c | 46 +++++++++++++++++++++---------------------
1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index 8e430d1ee297..77f3cee3af0d 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -159,9 +159,7 @@
# define LAN9303_BM_EGRSS_PORT_TYPE_SPECIAL_TAG_PORT1 (BIT(9) | BIT(8))
# define LAN9303_BM_EGRSS_PORT_TYPE_SPECIAL_TAG_PORT0 (BIT(1) | BIT(0))

-#define LAN9303_PORT_0_OFFSET 0x400
-#define LAN9303_PORT_1_OFFSET 0x800
-#define LAN9303_PORT_2_OFFSET 0xc00
+#define LAN9303_SWITCH_PORT_REG(port, reg0) (0x400 * (port) + (reg0))

/* the built-in PHYs are of type LAN911X */
#define MII_LAN911X_SPECIAL_MODES 0x12
@@ -428,6 +426,13 @@ static int lan9303_read_switch_reg(struct lan9303 *chip, u16 regnum, u32 *val)
return ret;
}

+static int lan9303_write_switch_port(struct lan9303 *chip, int port,
+ u16 regnum, u32 val)
+{
+ return lan9303_write_switch_reg(
+ chip, LAN9303_SWITCH_PORT_REG(port, regnum), val);
+}
+
static int lan9303_detect_phy_setup(struct lan9303 *chip)
{
int reg;
@@ -458,22 +463,19 @@ static int lan9303_detect_phy_setup(struct lan9303 *chip)
return 0;
}

-#define LAN9303_MAC_RX_CFG_OFFS (LAN9303_MAC_RX_CFG_0 - LAN9303_PORT_0_OFFSET)
-#define LAN9303_MAC_TX_CFG_OFFS (LAN9303_MAC_TX_CFG_0 - LAN9303_PORT_0_OFFSET)
-
static int lan9303_disable_packet_processing(struct lan9303 *chip,
unsigned int port)
{
int ret;

/* disable RX, but keep register reset default values else */
- ret = lan9303_write_switch_reg(chip, LAN9303_MAC_RX_CFG_OFFS + port,
- LAN9303_MAC_RX_CFG_X_REJECT_MAC_TYPES);
+ ret = lan9303_write_switch_port(chip, port, LAN9303_MAC_RX_CFG_0,
+ LAN9303_MAC_RX_CFG_X_REJECT_MAC_TYPES);
if (ret)
return ret;

/* disable TX, but keep register reset default values else */
- return lan9303_write_switch_reg(chip, LAN9303_MAC_TX_CFG_OFFS + port,
+ return lan9303_write_switch_port(chip, port, LAN9303_MAC_TX_CFG_0,
LAN9303_MAC_TX_CFG_X_TX_IFG_CONFIG_DEFAULT |
LAN9303_MAC_TX_CFG_X_TX_PAD_ENABLE);
}
@@ -484,14 +486,14 @@ static int lan9303_enable_packet_processing(struct lan9303 *chip,
int ret;

/* enable RX and keep register reset default values else */
- ret = lan9303_write_switch_reg(chip, LAN9303_MAC_RX_CFG_OFFS + port,
- LAN9303_MAC_RX_CFG_X_REJECT_MAC_TYPES |
- LAN9303_MAC_RX_CFG_X_RX_ENABLE);
+ ret = lan9303_write_switch_port(chip, port, LAN9303_MAC_RX_CFG_0,
+ LAN9303_MAC_RX_CFG_X_REJECT_MAC_TYPES |
+ LAN9303_MAC_RX_CFG_X_RX_ENABLE);
if (ret)
return ret;

/* enable TX and keep register reset default values else */
- return lan9303_write_switch_reg(chip, LAN9303_MAC_TX_CFG_OFFS + port,
+ return lan9303_write_switch_port(chip, port, LAN9303_MAC_TX_CFG_0,
LAN9303_MAC_TX_CFG_X_TX_IFG_CONFIG_DEFAULT |
LAN9303_MAC_TX_CFG_X_TX_PAD_ENABLE |
LAN9303_MAC_TX_CFG_X_TX_ENABLE);
@@ -558,13 +560,13 @@ static int lan9303_disable_processing(struct lan9303 *chip)
{
int ret;

- ret = lan9303_disable_packet_processing(chip, LAN9303_PORT_0_OFFSET);
+ ret = lan9303_disable_packet_processing(chip, 0);
if (ret)
return ret;
- ret = lan9303_disable_packet_processing(chip, LAN9303_PORT_1_OFFSET);
+ ret = lan9303_disable_packet_processing(chip, 1);
if (ret)
return ret;
- return lan9303_disable_packet_processing(chip, LAN9303_PORT_2_OFFSET);
+ return lan9303_disable_packet_processing(chip, 2);
}

static int lan9303_check_device(struct lan9303 *chip)
@@ -634,7 +636,7 @@ static int lan9303_setup(struct dsa_switch *ds)
if (ret)
dev_err(chip->dev, "failed to separate ports %d\n", ret);

- ret = lan9303_enable_packet_processing(chip, LAN9303_PORT_0_OFFSET);
+ ret = lan9303_enable_packet_processing(chip, 0);
if (ret)
dev_err(chip->dev, "failed to re-enable switching %d\n", ret);

@@ -757,11 +759,9 @@ static int lan9303_port_enable(struct dsa_switch *ds, int port,
/* enable internal packet processing */
switch (port) {
case 1:
- return lan9303_enable_packet_processing(chip,
- LAN9303_PORT_1_OFFSET);
+ return lan9303_enable_packet_processing(chip, port);
case 2:
- return lan9303_enable_packet_processing(chip,
- LAN9303_PORT_2_OFFSET);
+ return lan9303_enable_packet_processing(chip, port);
default:
dev_dbg(chip->dev,
"Error: request to power up invalid port %d\n", port);
@@ -778,12 +778,12 @@ static void lan9303_port_disable(struct dsa_switch *ds, int port,
/* disable internal packet processing */
switch (port) {
case 1:
- lan9303_disable_packet_processing(chip, LAN9303_PORT_1_OFFSET);
+ lan9303_disable_packet_processing(chip, port);
lan9303_phy_write(ds, chip->phy_addr_sel_strap + 1,
MII_BMCR, BMCR_PDOWN);
break;
case 2:
- lan9303_disable_packet_processing(chip, LAN9303_PORT_2_OFFSET);
+ lan9303_disable_packet_processing(chip, port);
lan9303_phy_write(ds, chip->phy_addr_sel_strap + 2,
MII_BMCR, BMCR_PDOWN);
break;
--
2.11.0

2017-08-05 11:07:02

by Egil Hjelmeland

[permalink] [raw]
Subject: [PATCH v4 net-next 5/5] net: dsa: lan9303: refactor lan9303_get_ethtool_stats

In lan9303_get_ethtool_stats: Get rid of 0x400 constant magic
by using new lan9303_read_switch_reg() inside loop.
Reduced scope of two variables.

Signed-off-by: Egil Hjelmeland <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
---
drivers/net/dsa/lan9303-core.c | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index 444958bb19f6..15befd155251 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -435,6 +435,13 @@ static int lan9303_write_switch_port(struct lan9303 *chip, int port,
chip, LAN9303_SWITCH_PORT_REG(port, regnum), val);
}

+static int lan9303_read_switch_port(struct lan9303 *chip, int port,
+ u16 regnum, u32 *val)
+{
+ return lan9303_read_switch_reg(
+ chip, LAN9303_SWITCH_PORT_REG(port, regnum), val);
+}
+
static int lan9303_detect_phy_setup(struct lan9303 *chip)
{
int reg;
@@ -705,19 +712,18 @@ static void lan9303_get_ethtool_stats(struct dsa_switch *ds, int port,
uint64_t *data)
{
struct lan9303 *chip = ds->priv;
- u32 reg;
- unsigned int u, poff;
- int ret;
-
- poff = port * 0x400;
+ unsigned int u;

for (u = 0; u < ARRAY_SIZE(lan9303_mib); u++) {
- ret = lan9303_read_switch_reg(chip,
- lan9303_mib[u].offset + poff,
- &reg);
+ u32 reg;
+ int ret;
+
+ ret = lan9303_read_switch_port(
+ chip, port, lan9303_mib[u].offset, &reg);
+
if (ret)
- dev_warn(chip->dev, "Reading status reg %u failed\n",
- lan9303_mib[u].offset + poff);
+ dev_warn(chip->dev, "Reading status port %d reg %u failed\n",
+ port, lan9303_mib[u].offset);
data[u] = reg;
}
}
--
2.11.0

2017-08-05 11:07:18

by Egil Hjelmeland

[permalink] [raw]
Subject: [PATCH v4 net-next 4/5] net: dsa: lan9303: Rename lan9303_xxx_packet_processing()

The lan9303_enable_packet_processing, lan9303_disable_packet_processing
functions operate on port, so the names should reflect that.
And to align with lan9303_disable_processing(), rename:

lan9303_enable_packet_processing -> lan9303_enable_processing_port
lan9303_disable_packet_processing -> lan9303_disable_processing_port

Signed-off-by: Egil Hjelmeland <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
---
drivers/net/dsa/lan9303-core.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index daae267f6b05..444958bb19f6 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -465,8 +465,8 @@ static int lan9303_detect_phy_setup(struct lan9303 *chip)
return 0;
}

-static int lan9303_disable_packet_processing(struct lan9303 *chip,
- unsigned int port)
+static int lan9303_disable_processing_port(struct lan9303 *chip,
+ unsigned int port)
{
int ret;

@@ -482,8 +482,8 @@ static int lan9303_disable_packet_processing(struct lan9303 *chip,
LAN9303_MAC_TX_CFG_X_TX_PAD_ENABLE);
}

-static int lan9303_enable_packet_processing(struct lan9303 *chip,
- unsigned int port)
+static int lan9303_enable_processing_port(struct lan9303 *chip,
+ unsigned int port)
{
int ret;

@@ -563,7 +563,7 @@ static int lan9303_disable_processing(struct lan9303 *chip)
int p;

for (p = 0; p < LAN9303_NUM_PORTS; p++) {
- int ret = lan9303_disable_packet_processing(chip, p);
+ int ret = lan9303_disable_processing_port(chip, p);

if (ret)
return ret;
@@ -639,7 +639,7 @@ static int lan9303_setup(struct dsa_switch *ds)
if (ret)
dev_err(chip->dev, "failed to separate ports %d\n", ret);

- ret = lan9303_enable_packet_processing(chip, 0);
+ ret = lan9303_enable_processing_port(chip, 0);
if (ret)
dev_err(chip->dev, "failed to re-enable switching %d\n", ret);

@@ -763,7 +763,7 @@ static int lan9303_port_enable(struct dsa_switch *ds, int port,
switch (port) {
case 1:
case 2:
- return lan9303_enable_packet_processing(chip, port);
+ return lan9303_enable_processing_port(chip, port);
default:
dev_dbg(chip->dev,
"Error: request to power up invalid port %d\n", port);
@@ -781,7 +781,7 @@ static void lan9303_port_disable(struct dsa_switch *ds, int port,
switch (port) {
case 1:
case 2:
- lan9303_disable_packet_processing(chip, port);
+ lan9303_disable_processing_port(chip, port);
lan9303_phy_write(ds, chip->phy_addr_sel_strap + port,
MII_BMCR, BMCR_PDOWN);
break;
--
2.11.0

2017-08-05 11:07:24

by Egil Hjelmeland

[permalink] [raw]
Subject: [PATCH v4 net-next 3/5] net: dsa: lan9303: Simplify lan9303_xxx_packet_processing() usage

Simplify usage of lan9303_enable_packet_processing,
lan9303_disable_packet_processing()

Signed-off-by: Egil Hjelmeland <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
---
drivers/net/dsa/lan9303-core.c | 24 ++++++++++--------------
1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index 49e3a0cb1b5b..daae267f6b05 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -560,15 +560,16 @@ static int lan9303_handle_reset(struct lan9303 *chip)
/* stop processing packets for all ports */
static int lan9303_disable_processing(struct lan9303 *chip)
{
- int ret;
+ int p;

- ret = lan9303_disable_packet_processing(chip, 0);
- if (ret)
- return ret;
- ret = lan9303_disable_packet_processing(chip, 1);
- if (ret)
- return ret;
- return lan9303_disable_packet_processing(chip, 2);
+ for (p = 0; p < LAN9303_NUM_PORTS; p++) {
+ int ret = lan9303_disable_packet_processing(chip, p);
+
+ if (ret)
+ return ret;
+ }
+
+ return 0;
}

static int lan9303_check_device(struct lan9303 *chip)
@@ -761,7 +762,6 @@ static int lan9303_port_enable(struct dsa_switch *ds, int port,
/* enable internal packet processing */
switch (port) {
case 1:
- return lan9303_enable_packet_processing(chip, port);
case 2:
return lan9303_enable_packet_processing(chip, port);
default:
@@ -780,13 +780,9 @@ static void lan9303_port_disable(struct dsa_switch *ds, int port,
/* disable internal packet processing */
switch (port) {
case 1:
- lan9303_disable_packet_processing(chip, port);
- lan9303_phy_write(ds, chip->phy_addr_sel_strap + 1,
- MII_BMCR, BMCR_PDOWN);
- break;
case 2:
lan9303_disable_packet_processing(chip, port);
- lan9303_phy_write(ds, chip->phy_addr_sel_strap + 2,
+ lan9303_phy_write(ds, chip->phy_addr_sel_strap + port,
MII_BMCR, BMCR_PDOWN);
break;
default:
--
2.11.0

2017-08-05 11:08:24

by Egil Hjelmeland

[permalink] [raw]
Subject: [PATCH v4 net-next 2/5] net: dsa: lan9303: define LAN9303_NUM_PORTS 3

Will be used instead of '3' in upcomming patches.

Signed-off-by: Egil Hjelmeland <[email protected]>
Signed-off-by: Andrew Lunn <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
---
drivers/net/dsa/lan9303-core.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index 77f3cee3af0d..49e3a0cb1b5b 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -20,6 +20,8 @@

#include "lan9303.h"

+#define LAN9303_NUM_PORTS 3
+
/* 13.2 System Control and Status Registers
* Multiply register number by 4 to get address offset.
*/
--
2.11.0

2017-08-07 21:18:41

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v4 net-next 0/5] Refactor lan9303_xxx_packet_processing

From: Egil Hjelmeland <[email protected]>
Date: Sat, 5 Aug 2017 13:05:45 +0200

> This series is purely non functional.
>
> It changes the lan9303_enable_packet_processing,
> lan9303_disable_packet_processing() to pass port number (0,1,2) as
> parameter instead of port offset. This aligns them with
> other functions in the module, and makes it possible to simplify the code.
>
> The lan9303_enable_packet_processing, lan9303_disable_packet_processing
> functions operate on port. Therefore rename the functions to reflect that
> as well.
>
> Reviewer pointed out lan9303_get_ethtool_stats would be better off with
> the use of a lan9303_read_switch_port(). So that was added to the series.
...

Series applied, thank you.