netdev - Re: [PATCH] [88q2xxx] Add support for handling master/slave in forced mode

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: <20250820173528.68a5c33d@kernel.org>
Date: Wed, 20 Aug 2025 17:35:28 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: "Ilya A. Evenbach" <ievenbach@...ora.tech>
Cc: netdev@...r.kernel.org, dima.fedrau@...il.com
Subject: Re: [PATCH] [88q2xxx] Add support for handling master/slave in
 forced mode

A few drive-by comments...

On Wed, 20 Aug 2025 11:11:43 -0700 Ilya A. Evenbach wrote:
> In-Reply-To: <20250820181143.2288755-1-ievenbach@...ora.tech>

Please start a new thread for each new revision.
Change subject to [PATCH vN], where vN is the revision.
Add

v1: https://lore.kernel.org/20250820181143.2288755-2-ievenbach@aurora.tech

under the --- for reference to older versions

> Cc: dima.fedrau@...il.com, "Ilya A. Evenbach" <ievenbach@...ora.tech>

The cc list is still pretty sparse

> Subject: [PATCH] [88q2xxx] Add support for handling master/slave in forced mode

the [88q2xxx] will be stripped by git when applying. Please look thru
the history of the file and find the correct way to prefix.

> 88q2xxx PHYs have non-standard way of setting master/slave in
> forced mode.
> This change adds support for changing and reporting this setting
> correctly through ethtool.

imperative mood:

 Add support for changing...

> +/* Marvell vendor PMA/PMD control for forced master/slave when AN is disabled */
> +#define PMAPMD_MVL_PMAPMD_CTL				0x0834
> +#define MASTER_MODE					BIT(14)
> +#define MODE_MASK					BIT(14)

Other defines in context seem to be prefixed with  MV88Q2XXX_
please prefer prefixing local defines.

>  struct mv88q2xxx_priv {
>  	bool enable_led0;
>  };
> @@ -377,13 +382,57 @@ static int mv88q2xxx_read_link(struct phy_device *phydev)
>  static int mv88q2xxx_read_master_slave_state(struct phy_device *phydev)
>  {
>  	int ret;
> +	int adv_l, adv_m, stat, stat2;

nit: in networking we like variable declaration lines to be sorted longest to shortest.
-- 
pw-bot: cr