Messages in this thread | | | Date | Thu, 23 Mar 2023 12:30:34 -0400 | Subject | Re: [PATCH net v2] net: dpaa2-mac: Get serdes only for backplane links | From | Sean Anderson <> |
| |
On 3/23/23 11:27, Ioana Ciornei wrote: > On Mon, Mar 06, 2023 at 11:13:17AM -0500, Sean Anderson wrote: >> On 3/6/23 03:09, Ioana Ciornei wrote: >> > On Fri, Mar 03, 2023 at 07:31:59PM -0500, Sean Anderson wrote: >> >> When commenting on what would become commit 085f1776fa03 ("net: dpaa2-mac: >> >> add backplane link mode support"), Ioana Ciornei said [1]: >> >> >> >> > ...DPMACs in TYPE_BACKPLANE can have both their PCS and SerDes managed >> >> > by Linux (since the firmware is not touching these). That being said, >> >> > DPMACs in TYPE_PHY (the type that is already supported in dpaa2-mac) can >> >> > also have their PCS managed by Linux (no interraction from the >> >> > firmware's part with the PCS, just the SerDes). >> >> >> >> This implies that Linux only manages the SerDes when the link type is >> >> backplane. Modify the condition in dpaa2_mac_connect to reflect this, >> >> moving the existing conditions to more appropriate places. >> > >> > I am not sure I understand why are you moving the conditions to >> > different places. Could you please explain? >> >> This is not (just) a movement of conditions, but a changing of what they >> apply to. >> >> There are two things which this patch changes: whether we manage the phy >> and whether we say we support alternate interfaces. According to your >> comment above (and roughly in-line with my testing), Linux manages the >> phy *exactly* when the link type is BACKPLANE. In all other cases, the >> firmware manages the phy. Similarly, alternate interfaces are supported >> *exactly* when the firmware supports PROTOCOL_CHANGE. However, currently >> the conditions do not match this. >> >> > Why not just append the existing condition from dpaa2_mac_connect() with >> > "mac->attr.link_type == DPMAC_LINK_TYPE_BACKPLANE"? >> > >> > This way, the serdes_phy is populated only if all the conditions pass >> > and you don't have to scatter them all around the driver. >> >> If we have link type BACKPLANE, Linux manages the phy, even if the >> firmware doesn't support changing the interface. Therefore, we need to >> grab the phy, but not fill in alternate interfaces. >> >> This does not scatter the conditions, but instead moves them to exactly >> where they are needed. Currently, they are in the wrong places. > > Sorry for not making my position clear from the first time which is: > there is no point in having a SerDes driver or a reference to the > SerDes PHY if the firmware does not actually support changing of > interfaces. > > Why I think that is because the SerDes is configured at boot time > anyway for the interface type defined in the RCW (Reset Configuration > Word). If the firmware does not support any protocol change then why > trouble the dpaa2-eth driver with anything SerDes related?
It's actually the other way around. If the firmware is managing the phy, why try to probe it? Consider a situation where the firmware supports protocol change, but the link type is PHY. Then we will probe the serdes, but we may confuse the firmware (or vice versa).
> This is why I am ok with only extending the condition from > dpaa2_mac_connect() with an additional check but not the exact patch > that you sent.
AIUI the condition there is correct because Linux controls the PCS in both PHY and BACKPLANE modes (although the RGMII check is a bit strange).
--Sean
| |