diff mbox series

[net-next,v4,1/6] net: dsa: mv88e6xxx: Add RMU enable for select switches.

Message ID 20220906063450.3698671-2-mattias.forsblad@gmail.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series net: dsa: mv88e6xxx: qca8k: rmon: Add RMU support | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 1 maintainers not CCed: linux@armlinux.org.uk
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 154 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Mattias Forsblad Sept. 6, 2022, 6:34 a.m. UTC 
Add RMU enable functionality for some Marvell SOHO switches.

Signed-off-by: Mattias Forsblad <mattias.forsblad@gmail.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c    |  6 +++
 drivers/net/dsa/mv88e6xxx/chip.h    |  1 +
 drivers/net/dsa/mv88e6xxx/global1.c | 76 +++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/global1.h |  3 ++
 4 files changed, 86 insertions(+)

Comments

Andrew Lunn Sept. 6, 2022, 12:29 p.m. UTC | #1 
> +int mv88e6085_g1_rmu_enable(struct mv88e6xxx_chip *chip, int upstream_port)
> +{
> +	int val = MV88E6352_G1_CTL2_RMU_MODE_DISABLED;
> +
> +	dev_dbg(chip->dev, "RMU: Enabling on port %d", upstream_port);
> +
> +	switch (upstream_port) {

>  
> +int mv88e6352_g1_rmu_enable(struct mv88e6xxx_chip *chip, int port)
> +{
> +	int val = MV88E6352_G1_CTL2_RMU_MODE_DISABLED;
> +	int upstream_port;
> +
> +	upstream_port = dsa_switch_upstream_port(chip->ds);
> +	dev_dbg(chip->dev, "RMU: Enabling on port %d", upstream_port);
> +	if (upstream_port < 0)
> +		return -EOPNOTSUPP;
> +
> +	switch (upstream_port) {

> +int mv88e6390_g1_rmu_enable(struct mv88e6xxx_chip *chip, int upstream_port)
> +{
> +	int val = MV88E6390_G1_CTL2_RMU_MODE_DISABLED;
> +
> +	dev_dbg(chip->dev, "RMU: Enabling on port %d", upstream_port);
> +
> +	switch (upstream_port) {


Why is 6352 different to 6085 and 6390? This is the sort of thing
which should be explained in the commit message. The commit message is
about the 'Why?' of the change. You could explain why there is this
difference, so a reviewer does not need to ask.

	    Andrew
Florian Fainelli Sept. 6, 2022, 9:46 p.m. UTC | #2 
On 9/5/2022 11:34 PM, Mattias Forsblad wrote:
> Add RMU enable functionality for some Marvell SOHO switches.
> 
> Signed-off-by: Mattias Forsblad <mattias.forsblad@gmail.com>
> ---

[snip]

> +int mv88e6085_g1_rmu_enable(struct mv88e6xxx_chip *chip, int upstream_port)
> +{
> +	int val = MV88E6352_G1_CTL2_RMU_MODE_DISABLED;
> +
> +	dev_dbg(chip->dev, "RMU: Enabling on port %d", upstream_port);

This debug print is in every chip-specific function, so maybe you can 
consider moving it to mv88e6xxx_master_change()?

> +
> +	switch (upstream_port) {
> +	case 9:
> +		val = MV88E6085_G1_CTL2_RM_ENABLE;
> +		break;
> +	case 10:
> +		val = MV88E6085_G1_CTL2_RM_ENABLE | MV88E6085_G1_CTL2_P10RM;
> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	return mv88e6xxx_g1_ctl2_mask(chip, MV88E6085_G1_CTL2_P10RM |
> +				      MV88E6085_G1_CTL2_RM_ENABLE, val);
> +}
> +
>   int mv88e6352_g1_rmu_disable(struct mv88e6xxx_chip *chip)
>   {
>   	return mv88e6xxx_g1_ctl2_mask(chip, MV88E6352_G1_CTL2_RMU_MODE_MASK,
>   				      MV88E6352_G1_CTL2_RMU_MODE_DISABLED);
>   }
>   
> +int mv88e6352_g1_rmu_enable(struct mv88e6xxx_chip *chip, int port)

Can we name this argument upstream_port and pass it a 
dsa_switch_upstream_port() port already?
Mattias Forsblad Sept. 7, 2022, 5:55 a.m. UTC | #3 
On 2022-09-06 14:29, Andrew Lunn wrote:
>> +int mv88e6085_g1_rmu_enable(struct mv88e6xxx_chip *chip, int upstream_port)
>> +{
>> +	int val = MV88E6352_G1_CTL2_RMU_MODE_DISABLED;
>> +
>> +	dev_dbg(chip->dev, "RMU: Enabling on port %d", upstream_port);
>> +
>> +	switch (upstream_port) {
> 
>>  
>> +int mv88e6352_g1_rmu_enable(struct mv88e6xxx_chip *chip, int port)
>> +{
>> +	int val = MV88E6352_G1_CTL2_RMU_MODE_DISABLED;
>> +	int upstream_port;
>> +
>> +	upstream_port = dsa_switch_upstream_port(chip->ds);
>> +	dev_dbg(chip->dev, "RMU: Enabling on port %d", upstream_port);
>> +	if (upstream_port < 0)
>> +		return -EOPNOTSUPP;
>> +
>> +	switch (upstream_port) {
> 
>> +int mv88e6390_g1_rmu_enable(struct mv88e6xxx_chip *chip, int upstream_port)
>> +{
>> +	int val = MV88E6390_G1_CTL2_RMU_MODE_DISABLED;
>> +
>> +	dev_dbg(chip->dev, "RMU: Enabling on port %d", upstream_port);
>> +
>> +	switch (upstream_port) {
> 
> 
> Why is 6352 different to 6085 and 6390? This is the sort of thing
> which should be explained in the commit message. The commit message is
> about the 'Why?' of the change. You could explain why there is this
> difference, so a reviewer does not need to ask.
> 
> 	    Andrew

I'm sorry, I must have slipped on the keyboard :/ It should be similar for
all functions. I'll fix that.

	Mattias
Mattias Forsblad Sept. 7, 2022, 6:29 a.m. UTC | #4 
On 2022-09-06 23:46, Florian Fainelli wrote:
> 
> 
> On 9/5/2022 11:34 PM, Mattias Forsblad wrote:
>> Add RMU enable functionality for some Marvell SOHO switches.
>>
>> Signed-off-by: Mattias Forsblad <mattias.forsblad@gmail.com>
>> ---
> 
> [snip]
> 
>> +int mv88e6085_g1_rmu_enable(struct mv88e6xxx_chip *chip, int upstream_port)
>> +{
>> +    int val = MV88E6352_G1_CTL2_RMU_MODE_DISABLED;
>> +
>> +    dev_dbg(chip->dev, "RMU: Enabling on port %d", upstream_port);
> 
> This debug print is in every chip-specific function, so maybe you can consider moving it to mv88e6xxx_master_change()?
> 

Ofc, will fix.
>> +
>> +    switch (upstream_port) {
>> +    case 9:
>> +        val = MV88E6085_G1_CTL2_RM_ENABLE;
>> +        break;
>> +    case 10:
>> +        val = MV88E6085_G1_CTL2_RM_ENABLE | MV88E6085_G1_CTL2_P10RM;
>> +        break;
>> +    default:
>> +        return -EOPNOTSUPP;
>> +    }
>> +
>> +    return mv88e6xxx_g1_ctl2_mask(chip, MV88E6085_G1_CTL2_P10RM |
>> +                      MV88E6085_G1_CTL2_RM_ENABLE, val);
>> +}
>> +
>>   int mv88e6352_g1_rmu_disable(struct mv88e6xxx_chip *chip)
>>   {
>>       return mv88e6xxx_g1_ctl2_mask(chip, MV88E6352_G1_CTL2_RMU_MODE_MASK,
>>                         MV88E6352_G1_CTL2_RMU_MODE_DISABLED);
>>   }
>>   +int mv88e6352_g1_rmu_enable(struct mv88e6xxx_chip *chip, int port)
> 
> Can we name this argument upstream_port and pass it a dsa_switch_upstream_port() port already?

Will fix.

	Mattias
diff mbox series

Patch

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 6f4ea39ab466..46e12b53a9e4 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -4098,6 +4098,7 @@  static const struct mv88e6xxx_ops mv88e6085_ops = {
 	.ppu_disable = mv88e6185_g1_ppu_disable,
 	.reset = mv88e6185_g1_reset,
 	.rmu_disable = mv88e6085_g1_rmu_disable,
+	.rmu_enable = mv88e6085_g1_rmu_enable,
 	.vtu_getnext = mv88e6352_g1_vtu_getnext,
 	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
 	.stu_getnext = mv88e6352_g1_stu_getnext,
@@ -4181,6 +4182,7 @@  static const struct mv88e6xxx_ops mv88e6097_ops = {
 	.pot_clear = mv88e6xxx_g2_pot_clear,
 	.reset = mv88e6352_g1_reset,
 	.rmu_disable = mv88e6085_g1_rmu_disable,
+	.rmu_enable = mv88e6085_g1_rmu_enable,
 	.vtu_getnext = mv88e6352_g1_vtu_getnext,
 	.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
 	.phylink_get_caps = mv88e6095_phylink_get_caps,
@@ -5300,6 +5302,7 @@  static const struct mv88e6xxx_ops mv88e6352_ops = {
 	.pot_clear = mv88e6xxx_g2_pot_clear,
 	.reset = mv88e6352_g1_reset,
 	.rmu_disable = mv88e6352_g1_rmu_disable,
+	.rmu_enable = mv88e6352_g1_rmu_enable,
 	.atu_get_hash = mv88e6165_g1_atu_get_hash,
 	.atu_set_hash = mv88e6165_g1_atu_set_hash,
 	.vtu_getnext = mv88e6352_g1_vtu_getnext,
@@ -5367,6 +5370,7 @@  static const struct mv88e6xxx_ops mv88e6390_ops = {
 	.pot_clear = mv88e6xxx_g2_pot_clear,
 	.reset = mv88e6352_g1_reset,
 	.rmu_disable = mv88e6390_g1_rmu_disable,
+	.rmu_enable = mv88e6390_g1_rmu_enable,
 	.atu_get_hash = mv88e6165_g1_atu_get_hash,
 	.atu_set_hash = mv88e6165_g1_atu_set_hash,
 	.vtu_getnext = mv88e6390_g1_vtu_getnext,
@@ -5434,6 +5438,7 @@  static const struct mv88e6xxx_ops mv88e6390x_ops = {
 	.pot_clear = mv88e6xxx_g2_pot_clear,
 	.reset = mv88e6352_g1_reset,
 	.rmu_disable = mv88e6390_g1_rmu_disable,
+	.rmu_enable = mv88e6390_g1_rmu_enable,
 	.atu_get_hash = mv88e6165_g1_atu_get_hash,
 	.atu_set_hash = mv88e6165_g1_atu_set_hash,
 	.vtu_getnext = mv88e6390_g1_vtu_getnext,
@@ -5504,6 +5509,7 @@  static const struct mv88e6xxx_ops mv88e6393x_ops = {
 	.pot_clear = mv88e6xxx_g2_pot_clear,
 	.reset = mv88e6352_g1_reset,
 	.rmu_disable = mv88e6390_g1_rmu_disable,
+	.rmu_enable = mv88e6390_g1_rmu_enable,
 	.atu_get_hash = mv88e6165_g1_atu_get_hash,
 	.atu_set_hash = mv88e6165_g1_atu_set_hash,
 	.vtu_getnext = mv88e6390_g1_vtu_getnext,
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index e693154cf803..7ce3c41f6caf 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -637,6 +637,7 @@  struct mv88e6xxx_ops {
 
 	/* Remote Management Unit operations */
 	int (*rmu_disable)(struct mv88e6xxx_chip *chip);
+	int (*rmu_enable)(struct mv88e6xxx_chip *chip, int port);
 
 	/* Precision Time Protocol operations */
 	const struct mv88e6xxx_ptp_ops *ptp_ops;
diff --git a/drivers/net/dsa/mv88e6xxx/global1.c b/drivers/net/dsa/mv88e6xxx/global1.c
index 5848112036b0..f6c288ece0ba 100644
--- a/drivers/net/dsa/mv88e6xxx/global1.c
+++ b/drivers/net/dsa/mv88e6xxx/global1.c
@@ -466,18 +466,94 @@  int mv88e6085_g1_rmu_disable(struct mv88e6xxx_chip *chip)
 				      MV88E6085_G1_CTL2_RM_ENABLE, 0);
 }
 
+int mv88e6085_g1_rmu_enable(struct mv88e6xxx_chip *chip, int upstream_port)
+{
+	int val = MV88E6352_G1_CTL2_RMU_MODE_DISABLED;
+
+	dev_dbg(chip->dev, "RMU: Enabling on port %d", upstream_port);
+
+	switch (upstream_port) {
+	case 9:
+		val = MV88E6085_G1_CTL2_RM_ENABLE;
+		break;
+	case 10:
+		val = MV88E6085_G1_CTL2_RM_ENABLE | MV88E6085_G1_CTL2_P10RM;
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return mv88e6xxx_g1_ctl2_mask(chip, MV88E6085_G1_CTL2_P10RM |
+				      MV88E6085_G1_CTL2_RM_ENABLE, val);
+}
+
 int mv88e6352_g1_rmu_disable(struct mv88e6xxx_chip *chip)
 {
 	return mv88e6xxx_g1_ctl2_mask(chip, MV88E6352_G1_CTL2_RMU_MODE_MASK,
 				      MV88E6352_G1_CTL2_RMU_MODE_DISABLED);
 }
 
+int mv88e6352_g1_rmu_enable(struct mv88e6xxx_chip *chip, int port)
+{
+	int val = MV88E6352_G1_CTL2_RMU_MODE_DISABLED;
+	int upstream_port;
+
+	upstream_port = dsa_switch_upstream_port(chip->ds);
+	dev_dbg(chip->dev, "RMU: Enabling on port %d", upstream_port);
+	if (upstream_port < 0)
+		return -EOPNOTSUPP;
+
+	switch (upstream_port) {
+	case 4:
+		val = MV88E6352_G1_CTL2_RMU_MODE_PORT_4;
+		break;
+	case 5:
+		val = MV88E6352_G1_CTL2_RMU_MODE_PORT_5;
+		break;
+	case 6:
+		val = MV88E6352_G1_CTL2_RMU_MODE_PORT_6;
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return mv88e6xxx_g1_ctl2_mask(chip, MV88E6352_G1_CTL2_RMU_MODE_MASK,
+			val);
+}
+
 int mv88e6390_g1_rmu_disable(struct mv88e6xxx_chip *chip)
 {
 	return mv88e6xxx_g1_ctl2_mask(chip, MV88E6390_G1_CTL2_RMU_MODE_MASK,
 				      MV88E6390_G1_CTL2_RMU_MODE_DISABLED);
 }
 
+int mv88e6390_g1_rmu_enable(struct mv88e6xxx_chip *chip, int upstream_port)
+{
+	int val = MV88E6390_G1_CTL2_RMU_MODE_DISABLED;
+
+	dev_dbg(chip->dev, "RMU: Enabling on port %d", upstream_port);
+
+	switch (upstream_port) {
+	case 0:
+		val = MV88E6390_G1_CTL2_RMU_MODE_PORT_0;
+		break;
+	case 1:
+		val = MV88E6390_G1_CTL2_RMU_MODE_PORT_1;
+		break;
+	case 9:
+		val = MV88E6390_G1_CTL2_RMU_MODE_PORT_9;
+		break;
+	case 10:
+		val = MV88E6390_G1_CTL2_RMU_MODE_PORT_10;
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return mv88e6xxx_g1_ctl2_mask(chip, MV88E6390_G1_CTL2_RMU_MODE_MASK,
+			val);
+}
+
 int mv88e6390_g1_stats_set_histogram(struct mv88e6xxx_chip *chip)
 {
 	return mv88e6xxx_g1_ctl2_mask(chip, MV88E6390_G1_CTL2_HIST_MODE_MASK,
diff --git a/drivers/net/dsa/mv88e6xxx/global1.h b/drivers/net/dsa/mv88e6xxx/global1.h
index 65958b2a0d3a..29c0c8acb583 100644
--- a/drivers/net/dsa/mv88e6xxx/global1.h
+++ b/drivers/net/dsa/mv88e6xxx/global1.h
@@ -313,8 +313,11 @@  int mv88e6250_g1_ieee_pri_map(struct mv88e6xxx_chip *chip);
 int mv88e6185_g1_set_cascade_port(struct mv88e6xxx_chip *chip, int port);
 
 int mv88e6085_g1_rmu_disable(struct mv88e6xxx_chip *chip);
+int mv88e6085_g1_rmu_enable(struct mv88e6xxx_chip *chip, int upstream_port);
 int mv88e6352_g1_rmu_disable(struct mv88e6xxx_chip *chip);
+int mv88e6352_g1_rmu_enable(struct mv88e6xxx_chip *chip, int upstream_port);
 int mv88e6390_g1_rmu_disable(struct mv88e6xxx_chip *chip);
+int mv88e6390_g1_rmu_enable(struct mv88e6xxx_chip *chip, int upstream_port);
 
 int mv88e6xxx_g1_set_device_number(struct mv88e6xxx_chip *chip, int index);