Re: [PATCH net v2] net: dpaa2-mac: Get serdes only for backplane links

[Sean Anderson]

Hi all,

On 3/6/23 11:13, 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.
> 
> --Sean

I see this is marked as "changes requested" on patchwork. However, as explained
above, I do not think that the requested changes are necessary. Please review the
patch status.

--Sean