From: "Ramón Nordin Rodriguez" <ramon.nordin.rodriguez@ferroamp.se>
To: Parthiban.Veerasooran@microchip.com
Cc: andrew@lunn.ch, 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 4/6] net: phy: microchip_t1s: fix reset complete status handling
Date: Fri, 26 May 2023 09:14:26 +0200 [thread overview]
Message-ID: <ZHBcUvSbX0taOED3@debian> (raw)
In-Reply-To: <8a46450d-7c6e-68a4-c09d-3b195a935907@microchip.com>
On Fri, May 26, 2023 at 06:00:08AM +0000, Parthiban.Veerasooran@microchip.com wrote:
> Hi Ramon,
> >> + /* Read STS2 register and check for the Reset Complete status to do the
> >> + * init configuration. If the Reset Complete is not set, wait for 5us
> >> + * and then read STS2 register again and check for Reset Complete status.
> >> + * Still if it is failed then declare PHY reset error or else proceed
> >> + * for the PHY initial register configuration.
> >> + */
> >
> > This comment explains exactly what the code does, which is also obvious
> > from reading the code. A meaningful comment would be explaining why the
> > state can change 5us later.
> >
> As per design, LAN867x reset to be completed by 3us. Just for a safer
> side it is recommended to use 5us. With the assumption of more than 3us
> completion, the first read checks for the Reset Complete. If the
> config_init is more faster, then once again checks for it after 5us.
>
> As you mentioned, can we remove the existing block comment as it
> explains the code and add the above comment to explain 5us delay.
> What is your opinion on this proposal?
>
> Best Regards,
> Parthiban V
>
I'd suggest the following
/*The chip completes a reset in 3us, we might get here earlier than that,
as an added margin we'll conditionally sleep 5us*/
next prev parent reply other threads:[~2023-05-26 7:14 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-24 14:45 [PATCH net-next v3 0/6] microchip_t1s: Update on Microchip 10BASE-T1S PHY driver Parthiban Veerasooran
2023-05-24 14:45 ` [PATCH net-next v3 1/6] net: phy: microchip_t1s: modify driver description to be more generic Parthiban Veerasooran
2023-05-25 18:15 ` Ramón Nordin Rodriguez
2023-05-24 14:45 ` [PATCH net-next v3 2/6] net: phy: microchip_t1s: replace read-modify-write code with phy_modify_mmd Parthiban Veerasooran
2023-05-25 1:04 ` Andrew Lunn
2023-05-25 15:08 ` Ramón Nordin Rodriguez
2023-05-26 5:48 ` Parthiban.Veerasooran
2023-05-26 7:10 ` Ramón Nordin Rodriguez
2023-05-26 13:22 ` Parthiban.Veerasooran
2023-05-26 14:02 ` Ramón Nordin Rodriguez
2023-05-26 14:15 ` Parthiban.Veerasooran
2023-05-25 15:30 ` Ramón Nordin Rodriguez
2023-05-25 15:50 ` Andrew Lunn
2023-05-24 14:45 ` [PATCH net-next v3 3/6] net: phy: microchip_t1s: update LAN867x PHY supported revision number Parthiban Veerasooran
2023-05-25 1:08 ` Andrew Lunn
2023-05-25 18:19 ` Ramón Nordin Rodriguez
2023-05-24 14:45 ` [PATCH net-next v3 4/6] net: phy: microchip_t1s: fix reset complete status handling Parthiban Veerasooran
2023-05-25 1:09 ` Andrew Lunn
2023-05-25 18:26 ` Ramón Nordin Rodriguez
2023-05-26 6:00 ` Parthiban.Veerasooran
2023-05-26 7:14 ` Ramón Nordin Rodriguez [this message]
2023-05-26 13:16 ` Parthiban.Veerasooran
2023-05-24 14:45 ` [PATCH net-next v3 5/6] net: phy: microchip_t1s: remove unnecessary interrupts disabling code Parthiban Veerasooran
2023-05-25 18:30 ` Ramón Nordin Rodriguez
2023-05-24 14:45 ` [PATCH net-next v3 6/6] net: phy: microchip_t1s: add support for Microchip LAN865x Rev.B0 PHYs Parthiban Veerasooran
2023-05-25 1:09 ` Andrew Lunn
2023-05-25 15:16 ` Ramón Nordin Rodriguez
2023-05-26 6:03 ` Parthiban.Veerasooran
2023-05-25 18:32 ` Ramón Nordin Rodriguez
2023-05-25 19:22 ` Ramón Nordin Rodriguez
2023-05-26 6:02 ` Parthiban.Veerasooran
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZHBcUvSbX0taOED3@debian \
--to=ramon.nordin.rodriguez@ferroamp.se \
--cc=Horatiu.Vultur@microchip.com \
--cc=Nicolas.Ferre@microchip.com \
--cc=Parthiban.Veerasooran@microchip.com \
--cc=Thorsten.Kummermehr@microchip.com \
--cc=Woojung.Huh@microchip.com \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=hkallweit1@gmail.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.