netdev - Re: [PATCH net-next] net: dsa: mv88e6xxx: keep CMODE writable code private

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190828184909.300fdae1@dellmb.labs.office.nic.cz>
Date:   Wed, 28 Aug 2019 18:49:09 +0200
From:   Marek Behún <marek.behun@....cz>
To:     Vivien Didelot <vivien.didelot@...il.com>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, f.fainelli@...il.com,
        andrew@...n.ch
Subject: Re: [PATCH net-next] net: dsa: mv88e6xxx: keep CMODE writable code
 private

On Wed, 28 Aug 2019 12:26:59 -0400
Vivien Didelot <vivien.didelot@...il.com> wrote:

> This is a follow-up patch for commit 7a3007d22e8d ("net: dsa:
> mv88e6xxx: fully support SERDES on Topaz family").
> 
> Since .port_set_cmode is only called from mv88e6xxx_port_setup_mac and
> mv88e6xxx_phylink_mac_config, it is fine to keep this "make writable"
> code private to the mv88e6341_port_set_cmode implementation, instead
> of adding yet another operation to the switch info structure.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@...il.com>
> ---
>  drivers/net/dsa/mv88e6xxx/chip.c | 8 --------
>  drivers/net/dsa/mv88e6xxx/chip.h | 1 -
>  drivers/net/dsa/mv88e6xxx/port.c | 9 ++++++++-
>  drivers/net/dsa/mv88e6xxx/port.h | 1 -
>  4 files changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c
> b/drivers/net/dsa/mv88e6xxx/chip.c index 54e88aafba2f..6525075f6bd3
> 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -454,12 +454,6 @@ int mv88e6xxx_port_setup_mac(struct
> mv88e6xxx_chip *chip, int port, int link, goto restore_link;
>  	}
>  
> -	if (chip->info->ops->port_set_cmode_writable) {
> -		err = chip->info->ops->port_set_cmode_writable(chip,
> port);
> -		if (err && err != -EOPNOTSUPP)
> -			goto restore_link;
> -	}
> -
>  	if (chip->info->ops->port_set_cmode) {
>  		err = chip->info->ops->port_set_cmode(chip, port,
> mode); if (err && err != -EOPNOTSUPP)
> @@ -2919,7 +2913,6 @@ static const struct mv88e6xxx_ops mv88e6141_ops
> = { .port_disable_pri_override = mv88e6xxx_port_disable_pri_override,
>  	.port_link_state = mv88e6352_port_link_state,
>  	.port_get_cmode = mv88e6352_port_get_cmode,
> -	.port_set_cmode_writable = mv88e6341_port_set_cmode_writable,
>  	.port_set_cmode = mv88e6341_port_set_cmode,
>  	.port_setup_message_port = mv88e6xxx_setup_message_port,
>  	.stats_snapshot = mv88e6390_g1_stats_snapshot,
> @@ -3618,7 +3611,6 @@ static const struct mv88e6xxx_ops mv88e6341_ops
> = { .port_disable_pri_override = mv88e6xxx_port_disable_pri_override,
>  	.port_link_state = mv88e6352_port_link_state,
>  	.port_get_cmode = mv88e6352_port_get_cmode,
> -	.port_set_cmode_writable = mv88e6341_port_set_cmode_writable,
>  	.port_set_cmode = mv88e6341_port_set_cmode,
>  	.port_setup_message_port = mv88e6xxx_setup_message_port,
>  	.stats_snapshot = mv88e6390_g1_stats_snapshot,
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.h
> b/drivers/net/dsa/mv88e6xxx/chip.h index d6b1aa35aa1a..421e8b84bec3
> 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.h
> +++ b/drivers/net/dsa/mv88e6xxx/chip.h
> @@ -400,7 +400,6 @@ struct mv88e6xxx_ops {
>  	/* CMODE control what PHY mode the MAC will use, eg. SGMII,
> RGMII, etc.
>  	 * Some chips allow this to be configured on specific ports.
>  	 */
> -	int (*port_set_cmode_writable)(struct mv88e6xxx_chip *chip,
> int port); int (*port_set_cmode)(struct mv88e6xxx_chip *chip, int
> port, phy_interface_t mode);
>  	int (*port_get_cmode)(struct mv88e6xxx_chip *chip, int port,
> u8 *cmode); diff --git a/drivers/net/dsa/mv88e6xxx/port.c
> b/drivers/net/dsa/mv88e6xxx/port.c index 542201214c36..4f841335ea32
> 100644 --- a/drivers/net/dsa/mv88e6xxx/port.c
> +++ b/drivers/net/dsa/mv88e6xxx/port.c
> @@ -510,7 +510,8 @@ int mv88e6390_port_set_cmode(struct
> mv88e6xxx_chip *chip, int port, return mv88e6xxx_port_set_cmode(chip,
> port, mode); }
>  
> -int mv88e6341_port_set_cmode_writable(struct mv88e6xxx_chip *chip,
> int port) +static int mv88e6341_port_set_cmode_writable(struct
> mv88e6xxx_chip *chip,
> +					     int port)
>  {
>  	int err, addr;
>  	u16 reg, bits;
> @@ -537,6 +538,8 @@ int mv88e6341_port_set_cmode_writable(struct
> mv88e6xxx_chip *chip, int port) int mv88e6341_port_set_cmode(struct
> mv88e6xxx_chip *chip, int port, phy_interface_t mode)
>  {
> +	int err;
> +
>  	if (port != 5)
>  		return -EOPNOTSUPP;
>  
> @@ -551,6 +554,10 @@ int mv88e6341_port_set_cmode(struct
> mv88e6xxx_chip *chip, int port, break;
>  	}
>  
> +	err = mv88e6341_port_set_cmode_writable(chip, port);
> +	if (err)
> +		return err;
> +
>  	return mv88e6xxx_port_set_cmode(chip, port, mode);
>  }
>  
> diff --git a/drivers/net/dsa/mv88e6xxx/port.h
> b/drivers/net/dsa/mv88e6xxx/port.h index e78d68c3e671..d4e9bea6e82f
> 100644 --- a/drivers/net/dsa/mv88e6xxx/port.h
> +++ b/drivers/net/dsa/mv88e6xxx/port.h
> @@ -336,7 +336,6 @@ int mv88e6097_port_pause_limit(struct
> mv88e6xxx_chip *chip, int port, u8 in, u8 out);
>  int mv88e6390_port_pause_limit(struct mv88e6xxx_chip *chip, int
> port, u8 in, u8 out);
> -int mv88e6341_port_set_cmode_writable(struct mv88e6xxx_chip *chip,
> int port); int mv88e6341_port_set_cmode(struct mv88e6xxx_chip *chip,
> int port, phy_interface_t mode);
>  int mv88e6390_port_set_cmode(struct mv88e6xxx_chip *chip, int port,

Reviewed-by: Marek Behún <marek.behun@....cz>