From:   "Sit, Michael Wei Hong" <michael.wei.hong.sit@intel.com>
To:     Russell King <linux@armlinux.org.uk>, Andrew Lunn <andrew@lunn.ch>
CC:     Giuseppe Cavallaro <peppe.cavallaro@st.com>,
        Alexandre Torgue <alexandre.torgue@foss.st.com>,
        Jose Abreu <joabreu@synopsys.com>,
        "David S . Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        "Jakub Kicinski" <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Maxime Coquelin <mcoquelin.stm32@gmail.com>,
        "Ong, Boon Leong" <boon.leong.ong@intel.com>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-stm32@st-md-mailman.stormreply.com" 
        <linux-stm32@st-md-mailman.stormreply.com>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "Looi, Hong Aun" <hong.aun.looi@intel.com>,
        "Voon, Weifeng" <weifeng.voon@intel.com>,
        "Lai, Peter Jun Ann" <peter.jun.ann.lai@intel.com>
Subject: RE: [PATCH net v2 1/2] net: stmmac: fix PHY handle parsing
Thread-Topic: [PATCH net v2 1/2] net: stmmac: fix PHY handle parsing
Thread-Index: AQHZWQqKiptgMBBkxEe3QmwqS1rIqK7/dC0AgAV1bFA=
Date:   Tue, 21 Mar 2023 08:34:49 +0000
Message-ID: <PH0PR11MB7587DC1E7B2947BDB0126F979D819@PH0PR11MB7587.namprd11.prod.outlook.com>
References: <20230314070208.3703963-1-michael.wei.hong.sit@intel.com>
 <20230314070208.3703963-2-michael.wei.hong.sit@intel.com>
 <10aff941-e18a-4d77-974b-1760529988a6@lunn.ch>
 <ZBTUTD6RL22pdlmq@shell.armlinux.org.uk>
In-Reply-To: <ZBTUTD6RL22pdlmq@shell.armlinux.org.uk>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?djaemcAHhFirhKj5q5IGB0qhHpwflvQE4TSrqNBRtTnVBiXHaZ4U3uivv6dc?=
 =?us-ascii?Q?Avv1hetbdfw09I8xzXLzzPffW3avjnd+072ZsrdjUoJQ21HoUKMuLDfiIW2+?=
 =?us-ascii?Q?D5eDG+avLPzCsXD9glhGmR/DKP57zJ0jj3ZT2fLammGFNV1XUmkCIQ8poe+H?=
 =?us-ascii?Q?WqFf9khs4DP3PzNxI9qVaHiGOt8TOm+wPSbhbzAxL0U9VaXEGp030g89FS9F?=
 =?us-ascii?Q?piEwm7KpElbzhFCRbCjzXssuiF7u8BOq9+wVpa/hB/+G1xRIdx5oLxw0Dm75?=
 =?us-ascii?Q?3PeuAURFBZexuttibdWhMDdzjGstqOj1kDtUMMl7BF0eR2M2x7u+WAoVA2aG?=
 =?us-ascii?Q?vk5o7jS1I6U1aOjHJojP7UDF2Ro2gTsaRw11QY6zCuGHx9OmjeTmIT1alEL5?=
 =?us-ascii?Q?u86iLK3Dav+eMtHE+OlOCSZn5OTxRlpFjTUnA/ZiKsanHebacHz527OeP2LM?=
 =?us-ascii?Q?RWzeC7HGS1E3q1mDVsfhqhUqenhd3iRZWFj6SfMMKz6QDyOiBj6LkEw8Hevs?=
 =?us-ascii?Q?LIHkJMQLJbWVPn0zz91o5S9ASfEwcD6IbCSYmEFualXigw5AjBzK50rbLOTk?=
 =?us-ascii?Q?rfrBpuTIF80UDGX6w4kIQUeSkipCKGxzmmedlvHW0f9qSPU2GhdZWjRBSvSw?=
 =?us-ascii?Q?G94zmrkCN1J+YMrrAn0p+8pUWaB29Xn8+ro78eIRLdu43MgvLyR9MhLMocNa?=
 =?us-ascii?Q?rxJTlhKvalEAnW+af+hLlvTQ3htLpe/ww+kKcJwsBdh4Zfkqe4iPXnhJeT94?=
 =?us-ascii?Q?8RL5C/qrCD4I8dJSiT7F7AKy26B/ofIAixAYzYvVJLFLZwhSvi2cp0mjRozv?=
 =?us-ascii?Q?ALA7c7yNl5HMOjyN6oYryaV/Zw42liFXHsR1C2jojWraejFQwc1zCTTgtjbk?=
 =?us-ascii?Q?09OQTi2l5tCNaQizQQ8N0KffvAbTdDImqQ/WaZXNHylPABI9M/cPt+MUXijM?=
 =?us-ascii?Q?pv/Qlb5DEID+OHm3ZR1sScSTwNaobc43i1GLi/uhnwO27Ip19QogW34Nj4Qx?=
 =?us-ascii?Q?mQs4K6d1qJkqn3FymHYPHkntgvVTq90/8bMAuN26joj56j9uTl7Ka7vW3Znw?=
 =?us-ascii?Q?Pk/3tkVFENQLMCUh7gjrk/OK18c00AxAnDolBUtLHMNknf5JSiVbjjV/n5Jy?=
 =?us-ascii?Q?sVjlppcNytR2GrDb7r+kQgIVrKUV0XYKBWEzjQ5Pu27YZ1blLgnK3YexO5Tg?=
 =?us-ascii?Q?0DXhijGbKwLEyCQg2MmWSouVd/6tmwdX5n/HlhgJtlZkGmWaYZXmRHIAa83c?=
 =?us-ascii?Q?TUsg1w1XaWfXGLAGP3uOzylYg8S20y1gXQOyXFDb50YlfmrHhTG3LWgdAYo/?=
 =?us-ascii?Q?cXvqJ3pG30J+2+h2q/PhIVXJgEI6wfBbZ+7jHtwTcWZ8prV2DNsP1dUBDSl9?=
 =?us-ascii?Q?htTD3WepeGRXzzFq+6eI1GZKdGCLLLY98TAzVWFbwdUxlLo2EebicUupMltK?=
 =?us-ascii?Q?SkRuErxfrKYfk8BMxKmFtibndNf6hqCTYu787Iupw5T0zcYFnyYa4iN6NZHC?=
 =?us-ascii?Q?rl3C7o6JApCt9rVTKQPZ1wP834CtT/JSz6FCM2sM7WL9/Ms2S7PmGCv23ooF?=
 =?us-ascii?Q?isoyX6qHJUeZ2OI8ZCpIgchxPTk8Lkrtl/+ttcpC+Ast2R27YIJx8j7tsSOq?=
 =?us-ascii?Q?2g=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB7587.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 8fae234b-0786-475b-c061-08db29e7227e
X-MS-Exchange-CrossTenant-originalarrivaltime: 21 Mar 2023 08:34:49.5661
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 1ItbaCklIH4rCd4Y7G35Xlhv54educ4AhWfM8Qnsc1GdzRw9XN/eI5cbVbYp63F247tijRyFhGAsztE2m3m/uUEfnMNQo365GPimtt9/tqc=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR11MB8259
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> -----Original Message-----
> From: Russell King <linux@armlinux.org.uk>
> Sent: Saturday, March 18, 2023 4:58 AM
> To: Andrew Lunn <andrew@lunn.ch>
> Cc: Sit, Michael Wei Hong <michael.wei.hong.sit@intel.com>;
> Giuseppe Cavallaro <peppe.cavallaro@st.com>; Alexandre
> Torgue <alexandre.torgue@foss.st.com>; Jose Abreu
> <joabreu@synopsys.com>; David S . Miller
> <davem@davemloft.net>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>;
> Paolo Abeni <pabeni@redhat.com>; Maxime Coquelin
> <mcoquelin.stm32@gmail.com>; Ong, Boon Leong
> <boon.leong.ong@intel.com>; netdev@vger.kernel.org; linux-
> stm32@st-md-mailman.stormreply.com; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Looi,
> Hong Aun <hong.aun.looi@intel.com>; Voon, Weifeng
> <weifeng.voon@intel.com>; Lai, Peter Jun Ann
> <peter.jun.ann.lai@intel.com>
> Subject: Re: [PATCH net v2 1/2] net: stmmac: fix PHY handle
> parsing
>=20
> On Fri, Mar 17, 2023 at 08:56:19PM +0100, Andrew Lunn wrote:
> > On Tue, Mar 14, 2023 at 03:02:07PM +0800, Michael Sit Wei
> Hong wrote:
> > > phylink_fwnode_phy_connect returns 0 when set to
> MLO_AN_INBAND.
> > > This causes the PHY handle parsing to skip and the PHY will not
> be
> > > attached to the MAC.
> >
> > Please could you expand the commit message because i'm
> having trouble
> > following this.
> >
> > phylink_fwnode_phy_connect() says:
> >
> > 	/* Fixed links and 802.3z are handled without needing a
> PHY */
> > 	if (pl->cfg_link_an_mode =3D=3D MLO_AN_FIXED ||
> > 	    (pl->cfg_link_an_mode =3D=3D MLO_AN_INBAND &&
> > 	     phy_interface_mode_is_8023z(pl->link_interface)))
> > 		return 0;
> >
> > So your first statement is not true. It should be
> MLO_AN_INBAND and
> > phy_interface_mode_is_8023z.
> >
> > > Add additional check for PHY handle parsing when set to
> MLO_AN_INBAND.
> >
> > Looking at the patch, there is no reference to
> MLO_AN_INBAND, or
> > managed =3D "in-band-status";
>=20
> That's the pesky "xpcs_an_inband" which ends up as phylink's
> "ovr_an_inband"... I'm sure these are random renames of stuff
> to make sure that people struggle to follow the code.
>=20
It is as mentioned above, the "xpcs_an_inband" will end up as
"ovr_an_inband" which will then
set pl->cfg_link_an_mode =3D MLO_AN_INBAND in the
phylink_parse_mode() in phylink.c

The phylink_fwnode_phy_connect() checks if both
MLO_AN_INBAND && phy_interface_mode_is_8023z() true
before returning 0.

But in our case, we only have MLO_AN_INBAND is true, which
then goes to the next part of the code.

	phy_fwnode =3D fwnode_get_phy_node(fwnode);
	if (IS_ERR(phy_fwnode)) {
		if (pl->cfg_link_an_mode =3D=3D MLO_AN_PHY)
			return -ENODEV;
		return 0;
	}

Where here the IS_ERR(phy_fwnode) returns true, then it
Checks for MLO_AN_PHY, which in our case is not, so it returns
a 0.

When returned 0, our driver will then skip the manual phy parsing
due to if (!fwnode || ret)
> --
> RMK's Patch system:
> https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at
> last!