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

[Parthiban.Veerasooran]

On 26/05/23 12:40 pm, Ramón Nordin Rodriguez wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Fri, May 26, 2023 at 05:48:25AM +0000, Parthiban.Veerasooran@xxxxxxxxxxxxx wrote:
>> Hi Ramon,
>>> Nitpick, I think this block comment can be reduced to:
>>> /* The following block deviates from AN1699 which states that a values
>>>    * should be written back, even if unmodified.
>>>    * Which is not necessary, so it's safe to use phy_modify_mmd here.*/
>>>
>>>    The comment I added was intended to describe why I was doing weird
>>>    things, but now I think it's more interesting to describe why we're
>>>    deviating from the AN.
>>>
>>>    Or the block comment could be dropped all togheter, I'm guessing no one
>>>    is going to consult the AN if things 'just work'
>>>
>> By consolidating all your comments in the other emails as well on this
>> 2nd patch, do you agree for my below proposal?
>>
>> We will remove all block comments and simply put AN1699 reference as we
>> did for lan865x_revb0_config_init with a small addition on top of
>> phy_modify_mmd for loop? so the comment will look like below,
>>
>> /* Reference to AN1699
>>    *
>> https://ww1.microchip.com/downloads/aemDocuments/documents/AIS/ProductDocuments/SupportingCollateral/AN-LAN8670-1-2-config-60001699.pdf
>>    * AN1699 says Read, Modify, Write, but the Write is not required if
>> the  register already has the required value. So it is safe to use
>> phy_modify_mmd here.
>>    */
>>
>> So in future, if someone wants to know about this configuration they can
>> simply refer the AN1699.
>>
>> What do you think?
>>
> 
> I'm not sure about the link, resources have a tendency to move.
Yes, I agree with you but somehow there is no way for giving the 
reference to this document. May be we will keep this link for the 
reference, later if someone is not able to access the link then they can 
request Microchip to get the document.

What do you think about this proposal? If you agree then I will proceed 
for preparing the next version with your comments.
> Otherwise LGTM
> 
>> Best Regards,
>> Parthiban V