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: <20230522151104.clf3lmsqdndihsvo@skbuf>
Date: Mon, 22 May 2023 18:11:04 +0300
From: Vladimir Oltean <olteanv@...il.com>
To: David Epping <david.epping@...singlinkelectronics.com>
Cc: Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>,
	Russell King <linux@...linux.org.uk>,
	"David S . Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
	UNGLinuxDriver@...rochip.com
Subject: Re: [PATCH net 3/3] net: phy: mscc: enable VSC8501/2 RGMII RX clock

On Mon, May 22, 2023 at 04:00:57PM +0200, David Epping wrote:
> On Mon, May 22, 2023 at 12:58:33PM +0300, Vladimir Oltean wrote:
> > If you still prefer to write twice in a row to the same paged register
> > instead of combining the changes, then fine by me, it's not a huge deal.
> 
> Since the clock enablement now happens in all modes the existing rgmii
> function name seems misleading to me.

To be fair, it's only as misleading as the datasheet name for the register
that holds this field, "RGMII CONTROL". Anyway, the function could be
renamed as necessary to be less confusing: vsc85xx_update_rgmii_ctrl()
or something along those lines.

MDIO reads and writes are not exactly the quickest I/O in the world, and
having 2 read-modify-write consecutive accesses to the same paged
register (which in turn implies indirect access) just because readability
seems like the type of thing that can play its part in deteriorating
boot time latency. Maybe we can deal with the readability some other way.

> Also we don't want to enable for
> all PHY types, and the differentiation is already available at the
> caller. I would thus opt for a separate function and fewer conditional
> statements.

I don't understand this. We don't? For what PHY types don't we want to
enable the RX_CLK?

> Its my first patch re-submission, so sorry for the noob question:
> Should I include your "pw-bot: changes-requested" tag with the third
> patch? Probably not.

Nope.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ