Re: [PATCH net-next 18/20] net: ethernet: qualcomm: Add PPE MAC support for phylink
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
Date: 2024-01-22 17:37:34
Also in:
linux-arm-msm, linux-devicetree, linux-doc, lkml, netdev
On Mon, Jan 22, 2024 at 11:01:26PM +0800, Lei Wei wrote:
On 1/10/2024 8:18 PM, Russell King (Oracle) wrote:quoted
On Wed, Jan 10, 2024 at 07:40:30PM +0800, Luo Jie wrote:quoted
@@ -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.
Why not have the higher level drivers provide a pointer to the appropriate phylink_mac_ops structure? Having extra levels of indirection makes my future maintenance of phylink harder (I'm already bugged by DSA doing this, and it's a right pain.) For example, if one of your higher level drivers needs the mac_prepare or mac_finish functionality, you have to add a shim, extra function pointers and so on. If I need to add an extra parameter to a method, then I have to fix up your shim layer _as well_ as all the called methods - in other words, it adds extra maintenance burden. It also makes detecting whether an implementation provides something or not harder - see the problems when mac_select_pcs() was introduced and rather than testing to see whether the method is populated, we have to call the method with a dummy value to discover whether the sub-driver implements it or not. Honestly, I would really like to get rid of DSA's phylink_mac_ops shim layer. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last! _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel