Re: [PATCH net-next 18/20] net: ethernet: qualcomm: Add PPE MAC support for phylink

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 1/10/2024 8:18 PM, Russell King (Oracle) wrote:
On Wed, Jan 10, 2024 at 07:40:30PM +0800, Luo Jie wrote:
+static void ppe_phylink_mac_link_up(struct ppe_device *ppe_dev, int port,
+				    struct phy_device *phy,
+				    unsigned int mode, phy_interface_t interface,
+				    int speed, int duplex, bool tx_pause, bool rx_pause)
+{
+	struct phylink_pcs *pcs = ppe_phylink_mac_select_pcs(ppe_dev, port, interface);
+	struct ppe_uniphy *uniphy = pcs_to_ppe_uniphy(pcs);
+	struct ppe_port *ppe_port = ppe_port_get(ppe_dev, port);
+
+	/* Wait uniphy auto-negotiation completion */
+	ppe_uniphy_autoneg_complete_check(uniphy, port);

Way too late...



Yes agree, this will be removed. If inband autoneg is used, .pcs_get_state should report the link status. Then this function call should not be needed and should be removed.

@@ -352,6 +1230,12 @@ static int ppe_port_maxframe_set(struct ppe_device *ppe_dev,
  }
static struct ppe_device_ops qcom_ppe_ops = {
+	.phylink_setup = ppe_phylink_setup,
+	.phylink_destroy = ppe_phylink_destroy,
+	.phylink_mac_config = ppe_phylink_mac_config,
+	.phylink_mac_link_up = ppe_phylink_mac_link_up,
+	.phylink_mac_link_down = ppe_phylink_mac_link_down,
+	.phylink_mac_select_pcs = ppe_phylink_mac_select_pcs,
  	.set_maxframe = ppe_port_maxframe_set,
  };

Why this extra layer of abstraction? If you need separate phylink
operations, why not implement separate phylink_mac_ops structures?


This PPE driver will serve as the base driver for higher level drivers
such as the ethernet DMA (EDMA) driver and the DSA switch driver. The
ppe_device_ops is exported to these higher level drivers, to allow access to PPE operations. For example, the EDMA driver (ethernet netdevice driver to be pushed for review after the PPE driver) will use the phylink_setup/destroy ops for managing netdevice to PHY linkage. The set_maxframe op is also to be used by the EDMA driver during MTU change operation on the ethernet port.

I also mentioned it in the section "Exported PPE Device Operations" in PPE driver documentation:
https://lore.kernel.org/netdev/20240110114033.32575-2-quic_luoj@xxxxxxxxxxx/

Whereas the PPE DSA switch driver is expected to use the phylink_mac ops. However,we will remove the phylink_mac ops from this patch now since it is currently unused.





[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux