Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 25 May 2023 17:50:21 +0200
From:   Andrew Lunn <andrew@lunn.ch>
To:     =?iso-8859-1?Q?Ram=F3n?= Nordin Rodriguez 
        <ramon.nordin.rodriguez@ferroamp.se>
Cc:     Parthiban Veerasooran <Parthiban.Veerasooran@microchip.com>,
        hkallweit1@gmail.com, linux@armlinux.org.uk, davem@davemloft.net,
        edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        horatiu.vultur@microchip.com, Woojung.Huh@microchip.com,
        Nicolas.Ferre@microchip.com, Thorsten.Kummermehr@microchip.com
Subject: Re: [PATCH net-next v3 2/6] net: phy: microchip_t1s: replace
 read-modify-write code with phy_modify_mmd
Message-ID: <99ccdedb-c2c7-4187-9fb4-b2047480e097@lunn.ch>
References: <20230524144539.62618-1-Parthiban.Veerasooran@microchip.com>
 <20230524144539.62618-3-Parthiban.Veerasooran@microchip.com>
 <ZG9/E8Am2ICEHIbr@debian>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZG9/E8Am2ICEHIbr@debian>
Precedence: bulk

> 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