From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Andrew Lunn <andrew@lunn.ch>
Cc: "Daniel Golle" <daniel@makrotopia.org>,
"Christian Marangi" <ansuelsmth@gmail.com>,
"Pavel Machek" <pavel@ucw.cz>, "Lee Jones" <lee@kernel.org>,
"Rob Herring" <robh+dt@kernel.org>,
"Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
"Conor Dooley" <conor+dt@kernel.org>,
"William Zhang" <william.zhang@broadcom.com>,
"Anand Gore" <anand.gore@broadcom.com>,
"Kursad Oney" <kursad.oney@broadcom.com>,
"Florian Fainelli" <florian.fainelli@broadcom.com>,
"Rafał Miłecki" <rafal@milecki.pl>,
"Broadcom internal kernel review list"
<bcm-kernel-feedback-list@broadcom.com>,
"David S. Miller" <davem@davemloft.net>,
"Eric Dumazet" <edumazet@google.com>,
"Jakub Kicinski" <kuba@kernel.org>,
"Paolo Abeni" <pabeni@redhat.com>,
"Heiner Kallweit" <hkallweit1@gmail.com>,
"Jacek Anaszewski" <jacek.anaszewski@gmail.com>,
"Fernández Rojas" <noltari@gmail.com>,
"Sven Schwermer" <sven.schwermer@disruptive-technologies.com>,
linux-leds@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org
Subject: Re: [net-next PATCH v10 3/5] net: phy: add support for PHY LEDs polarity modes
Date: Sat, 11 May 2024 10:35:13 +0100 [thread overview]
Message-ID: <Zj870XBJFEkXZM1z@shell.armlinux.org.uk> (raw)
In-Reply-To: <bb146832-30fc-4c76-a083-51a1bc087e61@lunn.ch>
On Sat, May 11, 2024 at 01:58:13AM +0200, Andrew Lunn wrote:
> > Wanting to make use of this I noticed that polarity settings are only
> > applied once in of_phy_led(), which is not sufficient for my use-case:
> >
> > I'm writing a LED driver for Aquantia PHYs and those PHYs reset the
> > polarity mode every time a PHY reset is triggered.
What sort of reset? There are hard resets that set the registers back
to default, and soft resets that don't. I think you are referring to
a hard reset, but please be clear.
> > I ended up writing the patch below, but I'm not sure if phy_init_hw
> > should take care of this or if the polarity modes should be stored in
> > memory allocated by the PHY driver and re-applied by the driver after
> > reset (eg. in .config_init). Kinda depends on taste and on how common
> > this behavior is in practise, so I thought the best is to reach out to
> > discuss.
>
> There was a similar discussion recently about WoL settings getting
> lost. The conclusion about that was the PHY should keep track of WoL
> setting. So i would say the same applies there. Please store it in a
> local priv structure.
Agreed. If it turns out that it's something that many PHYs need to do,
that would be the tiem to move it into the core phylib code. If it only
affects a minority of drivers, then it's something drivers should do.
The reasoning here is: if its only a problem for a small amount of PHY
drivers, then we don't need to penalise everyone with additional
overhead. If it's the majority of drivers, then it makes sense to
remove this burden from drivers.
Also note that this is one of the reasons I don't particularly like
the kernel's approach to PHY hardware resets - if a platform firmware
decides to describe the PHy hardware reset to the kernel, then we end
up with the hardware reset being used at various points which will
clear all the registers back to the reset defaults including clearing
WoL settings and the like. That's fine for AN state which will get
reloaded, but other state does not, so describing the hardware reset
can make things much more complicated and introduce differing
behaviours compared to platforms that don't describe it.
A lot of platforms choose not to describe the PHY hardware reset, but
some people see that there's a way to describe it, so they do whether
or not there's a reason for the kernel to be manipulating that reset
signal.
What I'm saying is there's several issues here that all interact...
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
next prev parent reply other threads:[~2024-05-11 9:35 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-25 20:36 [net-next PATCH v10 0/5] net: phy: generic polarity + LED support for qca808x Christian Marangi
2024-01-25 20:36 ` [net-next PATCH v10 1/5] dt-bindings: net: phy: Make LED active-low property common Christian Marangi
2024-01-25 20:36 ` [net-next PATCH v10 2/5] dt-bindings: net: phy: Document LED inactive high impedance mode Christian Marangi
2024-01-25 20:36 ` [net-next PATCH v10 3/5] net: phy: add support for PHY LEDs polarity modes Christian Marangi
2024-05-10 23:14 ` Daniel Golle
2024-05-10 23:58 ` Andrew Lunn
2024-05-11 9:35 ` Russell King (Oracle) [this message]
2024-01-25 20:37 ` [net-next PATCH v10 4/5] dt-bindings: net: Document QCA808x PHYs Christian Marangi
2024-01-25 20:37 ` [net-next PATCH v10 5/5] net: phy: at803x: add LED support for qca808x Christian Marangi
2024-01-27 5:10 ` [net-next PATCH v10 0/5] net: phy: generic polarity + " patchwork-bot+netdevbpf
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=Zj870XBJFEkXZM1z@shell.armlinux.org.uk \
--to=linux@armlinux.org.uk \
--cc=anand.gore@broadcom.com \
--cc=andrew@lunn.ch \
--cc=ansuelsmth@gmail.com \
--cc=bcm-kernel-feedback-list@broadcom.com \
--cc=conor+dt@kernel.org \
--cc=daniel@makrotopia.org \
--cc=davem@davemloft.net \
--cc=devicetree@vger.kernel.org \
--cc=edumazet@google.com \
--cc=florian.fainelli@broadcom.com \
--cc=hkallweit1@gmail.com \
--cc=jacek.anaszewski@gmail.com \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=kuba@kernel.org \
--cc=kursad.oney@broadcom.com \
--cc=lee@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-leds@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=noltari@gmail.com \
--cc=pabeni@redhat.com \
--cc=pavel@ucw.cz \
--cc=rafal@milecki.pl \
--cc=robh+dt@kernel.org \
--cc=sven.schwermer@disruptive-technologies.com \
--cc=william.zhang@broadcom.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).