lkml.org 
[lkml]   [2023]   [Mar]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH net v2] net: dpaa2-mac: Get serdes only for backplane links
    From
    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

    \
     
     \ /
      Last update: 2023-03-27 01:15    [W:3.771 / U:0.288 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site