netdev - Re: [PATCH net-next v3 2/6] net: phy: microchip_t1s: replace read-modify-write code with phy_modify

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]

Date: Thu, 25 May 2023 17:50:21 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Ramón Nordin Rodriguez <ramon.nordin.rodriguez@...roamp.se>
Cc: Parthiban Veerasooran <Parthiban.Veerasooran@...rochip.com>,
	hkallweit1@...il.com, linux@...linux.org.uk, davem@...emloft.net,
	edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
	horatiu.vultur@...rochip.com, Woojung.Huh@...rochip.com,
	Nicolas.Ferre@...rochip.com, Thorsten.Kummermehr@...rochip.com
Subject: Re: [PATCH net-next v3 2/6] net: phy: microchip_t1s: replace
 read-modify-write code with phy_modify_mmd

> This change also invalidates most of the comment. I think this should be
> reduced to something along the lines of:
> 	/* HW quirk: Microchip states in the application note (AN1699) for the phy
> 	 * that a set of read-modify-write (rmw) operations has to be performed
> 	 * on a set of seemingly magic registers.
> 	 * The result of these operations is just described as 'optimal performance'
> 	 * Microchip gives no explanation as to what these mmd regs do,
> 	 * in fact they are marked as reserved in the datasheet.*/

I agree the comments should be reviewed in light of these changes.

> 
> Additionally I don't mind it if you change the tone of the comment. This was brought
> up in the sitdown we had, where it was explained from Microchip that
> documenting what the reg operations actually does would expose to much
> of the internal workings of the chip.

They cannot care too much, or the firmware in the PHY would do this
where it is all hidden away.

      Andrew