On Thu, Mar 10, 2022 at 03:05:50PM +0000, Russell King (Oracle) wrote:

On Thu, Mar 10, 2022 at 04:51:59PM +0200, Ioana Ciornei wrote:

quoted

This patch integrates the dpaa2-eth driver with the generic PHY
infrastructure in order to search, find and reconfigure the SerDes lanes
in case of a protocol change.

On the .mac_config() callback, the phy_set_mode_ext() API is called so
that the Lynx 28G SerDes PHY driver can change the lane's configuration.
In the same phylink callback the MC firmware is called so that it
reconfigures the MAC side to run using the new protocol.

The consumer drivers - dpaa2-eth and dpaa2-switch - are updated to call
the dpaa2_mac_start/stop functions newly added which will
power_on/power_off the associated SerDes lane.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>

Looks better, there's a minor thing that I missed, sorry:

quoted

+	if (mac->features & DPAA2_MAC_FEATURE_PROTOCOL_CHANGE &&
+	    !phy_interface_mode_is_rgmii(mac->if_mode) &&
+	    is_of_node(dpmac_node)) {
+		serdes_phy = of_phy_get(to_of_node(dpmac_node), NULL);
+
+		if (IS_ERR(serdes_phy)) {
+			if (PTR_ERR(serdes_phy) == -ENODEV)
+				serdes_phy = NULL;
+			else
+				return PTR_ERR(serdes_phy);
+		} else {
+			phy_init(serdes_phy);
+		}

Would:
		if (PTR_ERR(serdes_phy) == -ENODEV)
			serdes_phy = NULL;
		else if (IS_ERR(serdes_phy))
			return PTR_ERR(serdes_phy);
		else
			phy_init(serdes_phy);

Yes, it wouldn't be an if inside another if statement.

be neater? There is no need to check IS_ERR() before testing PTR_ERR().
One may also prefer the pointer-comparison approach:

		if (serdes_phy == ERR_PTR(-ENODEV))

to remove any question about PTR_ERR(p) on a !IS_ERR(p) value too, but
it really doesn't make any difference.

I suspect this is just a code formatting issue, I'd think the compiler
would generate reasonable code either way, so as I said above, it's
quite minor.

As you said, since it's quite minor I am going to wait to see if more
comments will appear, if not I am going to fix this up in another patch.

Thanks!