Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Parshuram Raju Thombare <pthombar@cadence.com>
To:     Russell King - ARM Linux admin <linux@armlinux.org.uk>
CC:     "andrew@lunn.ch" <andrew@lunn.ch>,
        "nicolas.ferre@microchip.com" <nicolas.ferre@microchip.com>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Milind Parab <mparab@cadence.com>
Subject: RE: [PATCH v3] net: macb: add support for high speed interface
Thread-Topic: [PATCH v3] net: macb: add support for high speed interface
Thread-Index: AQHWp9HS4UctazNdVUWuQbMULgmNwKmiZpMAgAEAG1CAAY/28A==
Date:   Fri, 23 Oct 2020 10:59:42 +0000
Message-ID: <DM5PR07MB3196723723F236F6113DDF9EC11A0@DM5PR07MB3196.namprd07.prod.outlook.com>
References: <1603302245-30654-1-git-send-email-pthombar@cadence.com>
 <20201021185056.GN1551@shell.armlinux.org.uk>
 <DM5PR07MB31961F14DD8A38B7FFA8DA24C11D0@DM5PR07MB3196.namprd07.prod.outlook.com>
In-Reply-To: <DM5PR07MB31961F14DD8A38B7FFA8DA24C11D0@DM5PR07MB3196.namprd07.prod.outlook.com>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 450d27ca-ebcf-4bd2-3821-08d87742bed4
X-MS-Exchange-CrossTenant-originalarrivaltime: 23 Oct 2020 10:59:42.4781
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: d36035c5-6ce6-4662-a3dc-e762e61ae4c9
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: qtoR4RCA/jbUdwFIUeZiM3U1HGMoSF+3kld6r7KKyiPBHqnxMo37zY3mqAbyOjBXrXx1CChfAU51u/TGzjm8Ge+eeQAiIVOXAaNXpL6trX0=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR07MB5564
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.235,18.0.737
 definitions=2020-10-23_04:2020-10-23,2020-10-23 signatures=0
X-Proofpoint-Spam-Details: rule=outbound_check_notspam policy=outbound_check score=0 clxscore=1015
 mlxlogscore=945 phishscore=0 suspectscore=0 malwarescore=0 impostorscore=0
 adultscore=0 lowpriorityscore=0 mlxscore=0 priorityscore=1501 bulkscore=0
 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2009150000 definitions=main-2010230077
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi,

I was trying to find out any ethernet driver where this issue of selecting =
appropriate
pcs_ops due to phylink changing interface mode dynamically is handled.=20
But, apparently, so far only mvpp2 has adapted pcs_ops. And even in mvpp2, =
it is
not obvious how this case is handled.=20

Also, apart from interface mode changed due to SFPs with different types of=
 PHY
being used, it is not clear when phylink selects interface mode different t=
han it
initially requested to the ethernet driver.

>pcs_config and pcs_link_up passes "interface" as an argument, and in
>pcs_get_state call "state->interface" appeared to be populated just before
>calling it and hence should be valid.

It seems state->interface in pcs_get_state is not always valid when SFPs wi=
th
different types of PHY are used. =20
There is a chance of SFP with different type of PHY is inserted, eventually=
 invoking
phylink_resolve for interface mode different than phylink initially request=
ed,
and causing major reconfiguration.

However, pcs_get_state is called before major reconfiguration, where select=
ing
which pcs_ops and PCS to be used is difficult without correct interface mod=
e. =20

As struct phylink and hence phy_state is private to phylink layer, IMO this=
 need to be
handled at phylink level by passing appropriate interface mode to all neces=
sary
methods registered by drivers.

Something like

523 static void phylink_mac_pcs_get_state(struct phylink *pl,
 524                                       struct phylink_link_state *state=
)
 525 {
 526         linkmode_copy(state->advertising, pl->link_config.advertising)=
;
 527         linkmode_zero(state->lp_advertising);
 528         if (pl->phydev)
 529                 state->interface =3D pl->phy_state.interface;
 530         else
 531                 state->interface =3D pl->link_config.interface;
 532         state->an_enabled =3D pl->link_config.an_enabled;
 533         state->speed =3D SPEED_UNKNOWN;
 534         state->duplex =3D DUPLEX_UNKNOWN;
 535         state->pause =3D MLO_PAUSE_NONE;
 536         state->an_complete =3D 0;
 537         state->link =3D 1;
 538
 539         if (pl->pcs_ops)
 540                 pl->pcs_ops->pcs_get_state(pl->pcs, state);
 541         else
 542                 pl->mac_ops->mac_pcs_get_state(pl->config, state);


Regards,
Parshuram Thombare