Received-SPF: pass (google.com: domain of linux-kernel+bounces-22161-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Wed, 10 Jan 2024 12:18:34 +0000
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Luo Jie <quic_luoj@quicinc.com>
Cc: agross@kernel.org, andersson@kernel.org, konrad.dybcio@linaro.org,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	corbet@lwn.net, catalin.marinas@arm.com, will@kernel.org,
	p.zabel@pengutronix.de, shannon.nelson@amd.com,
	anthony.l.nguyen@intel.com, jasowang@redhat.com,
	brett.creeley@amd.com, rrameshbabu@nvidia.com,
	joshua.a.hay@intel.com, arnd@arndb.de, geert+renesas@glider.be,
	neil.armstrong@linaro.org, dmitry.baryshkov@linaro.org,
	nfraprado@collabora.com, m.szyprowski@samsung.com, u-kumar1@ti.com,
	jacob.e.keller@intel.com, andrew@lunn.ch, netdev@vger.kernel.org,
	linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, ryazanov.s.a@gmail.com,
	ansuelsmth@gmail.com, quic_kkumarcs@quicinc.com,
	quic_suruchia@quicinc.com, quic_soni@quicinc.com,
	quic_pavir@quicinc.com, quic_souravp@quicinc.com,
	quic_linchen@quicinc.com, quic_leiwei@quicinc.com
Subject: Re: [PATCH net-next 18/20] net: ethernet: qualcomm: Add PPE MAC
 support for phylink
Message-ID: <ZZ6LGiSde4hHM+6j@shell.armlinux.org.uk>
References: <20240110114033.32575-1-quic_luoj@quicinc.com>
 <20240110114033.32575-19-quic_luoj@quicinc.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240110114033.32575-19-quic_luoj@quicinc.com>
Sender: Russell King (Oracle) <linux@armlinux.org.uk>

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...

> @@ -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?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!