linux-kernel - Re: [PATCH net-next v4 13/12] net: lan865x: optional hardware reset

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, 2 May 2024 10:10:09 +0000
From: <Parthiban.Veerasooran@...rochip.com>
To: <andrew@...n.ch>
CC: <davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
	<pabeni@...hat.com>, <horms@...nel.org>, <saeedm@...dia.com>,
	<anthony.l.nguyen@...el.com>, <netdev@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <corbet@....net>,
	<linux-doc@...r.kernel.org>, <robh+dt@...nel.org>,
	<krzysztof.kozlowski+dt@...aro.org>, <conor+dt@...nel.org>,
	<devicetree@...r.kernel.org>, <Horatiu.Vultur@...rochip.com>,
	<ruanjinjie@...wei.com>, <Steen.Hegelund@...rochip.com>,
	<vladimir.oltean@....com>, <UNGLinuxDriver@...rochip.com>,
	<Thorsten.Kummermehr@...rochip.com>, <Pier.Beruto@...emi.com>,
	<Selvamani.Rajagopal@...emi.com>, <Nicolas.Ferre@...rochip.com>,
	<benjamin.bigler@...nformulastudent.ch>, <ramon.nordin.rodriguez@...roamp.se>
Subject: Re: [PATCH net-next v4 13/12] net: lan865x: optional hardware reset

Hi Andrew,

On 30/04/24 7:44 pm, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
>> In the spec, external RESET pin is mentioned as "if implemented", in my
>> understanding it is MAC-PHY vendors choice of implementing it where
>> Microchip is implemented it. Using this reset, can be a application
>> requirement/decision. It can be controlled from an external application
>> where it is not needed SPI to operate.
> 
> Since it is optional, controlling the reset pin is clearly not
> something for the TC6 core.
> 
> However, i doubt having an external application controlling the reset
> is a good idea. You don't want to reset during operation. So to me,
> this reset should be controlled by the driver. I tend to agree, that
> actually performing a reset is optional, but i would expect the driver
> to ensure the device is taken out of reset during probe, if the power
> on default of the board is to hold it in reset.
POR - Power ON Reset.

If I understand you correctly, we need to ensure that whether the 
MAC-PHY is out of POR before doing any SPI operation right? so you 
expect oa_tc6_sw_reset_macphy() also may fail as the SWRESET bit write 
may fail if the device is still in POR.

Basically oa_tc6_sw_reset_macphy() does the following,
  - Writes 1 to SWRESET bit in the Reset Control and Status Register to 
trigger software reset.
  - Polls for software reset complete for every 1ms until 1s timeout.
  - Clears reset status complete status.

Can we rename the function like oa_tc6_check_power_on_reset_complete()
and the definition like below?
  - Poll for software reset complete for every 1ms until 1s timeout. The 
return value of oa_tc6_read_register() may not be needed to check for 
error as it might fail if the POR is taking place right?
  - Clear reset complete status.

But this case will work fine with initial power ON but after that if we 
are only reloading the driver for next time without powering OFF and ON 
then it will fail.

I am just thinking from TC6 framework point of view. Controlling from 
ext reset pin will not be an option as it is mentioned like an optional 
implementation in the OPEN Alliance. So some MAC-PHY vendors might not 
implement this.

Just an open question, after powering ON the Linux system, there are 
different stages are involved in the booting sequence of a system before 
kernel driver loads. A device like MAC-PHY which takes hardly around 5 
to 10us will complete the POR before that? OR is my understanding here 
is wrong?

Best regards,
Parthiban V
> 
>     Andrew