linux-kernel - Re: [PATCH net-next v2 4/6] net: phy: microchip

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: <2523bd58-2b2c-723f-6261-aa44ca92e00a@microchip.com>
Date:   Wed, 24 May 2023 07:31:09 +0000
From:   <Parthiban.Veerasooran@...rochip.com>
To:     <andrew@...n.ch>
CC:     <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>,
        <ramon.nordin.rodriguez@...roamp.se>,
        <Horatiu.Vultur@...rochip.com>, <Woojung.Huh@...rochip.com>,
        <Nicolas.Ferre@...rochip.com>, <Thorsten.Kummermehr@...rochip.com>
Subject: Re: [PATCH net-next v2 4/6] net: phy: microchip_t1s: fix reset
 complete status handling

Hi Andrew,

Please ignore the previous reply for this comment and consider this one.

On 23/05/23 5:53 pm, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Tue, May 23, 2023 at 05:30:06AM +0000, Parthiban.Veerasooran@...rochip.com wrote:
>> Hi Andrew,
>>
>> On 22/05/23 6:13 pm, Andrew Lunn wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On Mon, May 22, 2023 at 05:03:29PM +0530, Parthiban Veerasooran wrote:
>>>> As per the datasheet DS-LAN8670-1-2-60001573C.pdf, the Reset Complete
>>>> status bit in the STS2 register to be checked before proceeding for the
>>>> initial configuration.
>>>
>>> Is this the unmaskable interrupt status bit which needs clearing?
>> Yes, it is non-maskable interrupt.
>>> There is no mention of interrupts here.
>> The device will assert the Reset Complete (RESETC) bit in the Status 2
>> (STS2) register to indicate that it has completed its internal
>> initialization and is ready for configuration. As the Reset Complete
>> status is non-maskable, the IRQ_N pin will always be asserted and driven
>> low following a device reset. Upon reading of the Status 2 register, the
>> pending Reset Complete status bit will be automatically cleared causing
>> the IRQ_N pin to be released and pulled high again.
>>
>> Do you think it makes sense to add these explanation regarding the reset
>> and interrupt behavior with the above comment for a better understanding?
> 
> Comments should explain 'Why?'. At the moment, it is not clear why you
> are reading the status. The discussion so far has been about clearing
> the interrupt, not about checking it has actually finished its
> internal reset. So i think you should be mentioning interrupts
> somewhere. Especially since this is a rather odd behaviour.
Shall I update the above commit message like below?

As per the datasheet DS-LAN8670-1-2-60001573C.pdf, during the Power ON 
Reset(POR)/Hard Reset/Soft Reset, the Reset Complete status bit in the 
STS2 register to be checked before proceeding for the initial 
configuration. Reading STS2 register will also clear the Reset Complete 
interrupt which is non-maskable.

Or still I have a misunderstanding here?

Best Regards,
Parthiban V
> 
>             Andrew