Message-ID: <d278ad9a-5d23-4cb8-9de7-5a51d838ba5d@quicinc.com>
Date: Tue, 24 Dec 2024 12:29:56 +0530
From: Manikanta Mylavarapu <quic_mmanikan@...cinc.com>
To: Lei Wei <quic_leiwei@...cinc.com>, Andrew Lunn <andrew+netdev@...n.ch>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, "Rob
 Herring" <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        "Conor
 Dooley" <conor+dt@...nel.org>, Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit
	<hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>
CC: <netdev@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>,
        <quic_kkumarcs@...cinc.com>, <quic_suruchia@...cinc.com>,
        <quic_pavir@...cinc.com>, <quic_linchen@...cinc.com>,
        <quic_luoj@...cinc.com>, <srinivas.kandagatla@...aro.org>,
        <bartosz.golaszewski@...aro.org>, <vsmuthu@....qualcomm.com>,
        <john@...ozen.org>
Subject: Re: [PATCH net-next v3 3/5] net: pcs: qcom-ipq9574: Add PCS
 instantiation and phylink operations



On 12/16/2024 7:10 PM, Lei Wei wrote:
> This patch adds the following PCS functionality for the PCS driver
> for IPQ9574 SoC:
> 
> a.) Parses PCS MII DT nodes and instantiate each MII PCS instance.
> b.) Exports PCS instance get and put APIs. The network driver calls
> the PCS get API to get and associate the PCS instance with the port
> MAC.
> c.) PCS phylink operations for SGMII/QSGMII interface modes.
> 
> Signed-off-by: Lei Wei <quic_leiwei@...cinc.com>
> ---
>  drivers/net/pcs/pcs-qcom-ipq9574.c   | 463 +++++++++++++++++++++++++++++++++++
>  include/linux/pcs/pcs-qcom-ipq9574.h |  16 ++
>  2 files changed, 479 insertions(+)
> 
> diff --git a/drivers/net/pcs/pcs-qcom-ipq9574.c b/drivers/net/pcs/pcs-qcom-ipq9574.c
> index ea90c1902b61..54acb1c8c67f 100644
> --- a/drivers/net/pcs/pcs-qcom-ipq9574.c
> +++ b/drivers/net/pcs/pcs-qcom-ipq9574.c
> @@ -6,12 +6,46 @@
>  #include <linux/clk.h>
>  #include <linux/clk-provider.h>
>  #include <linux/device.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/pcs/pcs-qcom-ipq9574.h>
>  #include <linux/phy.h>
> +#include <linux/phylink.h>
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>
>  
>  #include <dt-bindings/net/qcom,ipq9574-pcs.h>
>  
> +/* Maximum number of MIIs per PCS instance. There are 5 MIIs for PSGMII. */
> +#define PCS_MAX_MII_NRS			5
> +
> +#define PCS_CALIBRATION			0x1e0
> +#define PCS_CALIBRATION_DONE		BIT(7)
> +
> +#define PCS_MODE_CTRL			0x46c
> +#define PCS_MODE_SEL_MASK		GENMASK(12, 8)
> +#define PCS_MODE_SGMII			FIELD_PREP(PCS_MODE_SEL_MASK, 0x4)
> +#define PCS_MODE_QSGMII			FIELD_PREP(PCS_MODE_SEL_MASK, 0x1)
> +
> +#define PCS_MII_CTRL(x)			(0x480 + 0x18 * (x))
> +#define PCS_MII_ADPT_RESET		BIT(11)
> +#define PCS_MII_FORCE_MODE		BIT(3)
> +#define PCS_MII_SPEED_MASK		GENMASK(2, 1)
> +#define PCS_MII_SPEED_1000		FIELD_PREP(PCS_MII_SPEED_MASK, 0x2)
> +#define PCS_MII_SPEED_100		FIELD_PREP(PCS_MII_SPEED_MASK, 0x1)
> +#define PCS_MII_SPEED_10		FIELD_PREP(PCS_MII_SPEED_MASK, 0x0)
> +
> +#define PCS_MII_STS(x)			(0x488 + 0x18 * (x))
> +#define PCS_MII_LINK_STS		BIT(7)
> +#define PCS_MII_STS_DUPLEX_FULL		BIT(6)
> +#define PCS_MII_STS_SPEED_MASK		GENMASK(5, 4)
> +#define PCS_MII_STS_SPEED_10		0
> +#define PCS_MII_STS_SPEED_100		1
> +#define PCS_MII_STS_SPEED_1000		2
> +
> +#define PCS_PLL_RESET			0x780
> +#define PCS_ANA_SW_RESET		BIT(6)
> +
>  #define XPCS_INDIRECT_ADDR		0x8000
>  #define XPCS_INDIRECT_AHB_ADDR		0x83fc
>  #define XPCS_INDIRECT_ADDR_H		GENMASK(20, 8)
> @@ -20,6 +54,18 @@
>  					 FIELD_PREP(GENMASK(9, 2), \
>  					 FIELD_GET(XPCS_INDIRECT_ADDR_L, reg)))
>  
> +/* Per PCS MII private data */
> +struct ipq_pcs_mii {
> +	struct ipq_pcs *qpcs;
> +	struct phylink_pcs pcs;
> +	int index;
> +
> +	/* RX clock from NSSCC to PCS MII */
> +	struct clk *rx_clk;
> +	/* TX clock from NSSCC to PCS MII */
> +	struct clk *tx_clk;
> +};
> +
>  /* PCS private data */
>  struct ipq_pcs {
>  	struct device *dev;
> @@ -27,12 +73,423 @@ struct ipq_pcs {
>  	struct regmap *regmap;
>  	phy_interface_t interface;
>  
> +	/* Lock to protect PCS configurations shared by multiple MII ports */
> +	struct mutex config_lock;
> +
>  	/* RX clock supplied to NSSCC */
>  	struct clk_hw rx_hw;
>  	/* TX clock supplied to NSSCC */
>  	struct clk_hw tx_hw;
> +
> +	struct ipq_pcs_mii *qpcs_mii[PCS_MAX_MII_NRS];
>  };
>  
> +#define phylink_pcs_to_qpcs_mii(_pcs)	\
> +	container_of(_pcs, struct ipq_pcs_mii, pcs)
> +
> +static void ipq_pcs_get_state_sgmii(struct ipq_pcs *qpcs,
> +				    int index,
> +				    struct phylink_link_state *state)
> +{
> +	unsigned int val;
> +	int ret;
> +
> +	ret = regmap_read(qpcs->regmap, PCS_MII_STS(index), &val);
> +	if (ret) {
> +		state->link = 0;
> +		return;
> +	}
> +
> +	state->link = !!(val & PCS_MII_LINK_STS);
> +
> +	if (!state->link)
> +		return;
> +
> +	switch (FIELD_GET(PCS_MII_STS_SPEED_MASK, val)) {
> +	case PCS_MII_STS_SPEED_1000:
> +		state->speed = SPEED_1000;
> +		break;
> +	case PCS_MII_STS_SPEED_100:
> +		state->speed = SPEED_100;
> +		break;
> +	case PCS_MII_STS_SPEED_10:
> +		state->speed = SPEED_10;
> +		break;
> +	default:
> +		state->link = false;
> +		return;
> +	}
> +
> +	if (val & PCS_MII_STS_DUPLEX_FULL)
> +		state->duplex = DUPLEX_FULL;
> +	else
> +		state->duplex = DUPLEX_HALF;
> +}
> +
> +static int ipq_pcs_config_mode(struct ipq_pcs *qpcs,
> +			       phy_interface_t interface)
> +{
> +	unsigned int val;
> +	int ret;
> +
> +	/* Configure PCS interface mode */
> +	switch (interface) {
> +	case PHY_INTERFACE_MODE_SGMII:
> +		val = PCS_MODE_SGMII;
> +		break;
> +	case PHY_INTERFACE_MODE_QSGMII:
> +		val = PCS_MODE_QSGMII;
> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	ret = regmap_update_bits(qpcs->regmap, PCS_MODE_CTRL,
> +				 PCS_MODE_SEL_MASK, val);
> +	if (ret)
> +		return ret;
> +
> +	/* PCS PLL reset */
> +	ret = regmap_clear_bits(qpcs->regmap, PCS_PLL_RESET, PCS_ANA_SW_RESET);
> +	if (ret)
> +		return ret;
> +
> +	fsleep(1000);
> +	ret = regmap_set_bits(qpcs->regmap, PCS_PLL_RESET, PCS_ANA_SW_RESET);
> +	if (ret)
> +		return ret;
> +
> +	/* Wait for calibration completion */
> +	ret = regmap_read_poll_timeout(qpcs->regmap, PCS_CALIBRATION,
> +				       val, val & PCS_CALIBRATION_DONE,
> +				       1000, 100000);
> +	if (ret) {
> +		dev_err(qpcs->dev, "PCS calibration timed-out\n");
> +		return ret;
> +	}
> +
> +	qpcs->interface = interface;
> +
> +	return 0;
> +}
> +
> +static int ipq_pcs_config_sgmii(struct ipq_pcs *qpcs,
> +				int index,
> +				unsigned int neg_mode,
> +				phy_interface_t interface)
> +{
> +	int ret;
> +
> +	/* Access to PCS registers such as PCS_MODE_CTRL which are
> +	 * common to all MIIs, is lock protected and configured
> +	 * only once.
> +	 */
> +	mutex_lock(&qpcs->config_lock);
> +
> +	if (qpcs->interface != interface) {
> +		ret = ipq_pcs_config_mode(qpcs, interface);
> +		if (ret) {
> +			mutex_unlock(&qpcs->config_lock);
> +			return ret;
> +		}
> +	}
> +
> +	mutex_unlock(&qpcs->config_lock);
> +
> +	/* Nothing to do here as in-band autoneg mode is enabled
> +	 * by default for each PCS MII port.
> +	 */
> +	if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED)
> +		return 0;
> +
> +	/* Set force speed mode */
> +	return regmap_set_bits(qpcs->regmap,
> +			       PCS_MII_CTRL(index), PCS_MII_FORCE_MODE);
> +}
> +
> +static int ipq_pcs_link_up_config_sgmii(struct ipq_pcs *qpcs,
> +					int index,
> +					unsigned int neg_mode,
> +					int speed)
> +{
> +	unsigned int val;
> +	int ret;
> +
> +	/* PCS speed need not be configured if in-band autoneg is enabled */
> +	if (neg_mode != PHYLINK_PCS_NEG_INBAND_ENABLED) {
> +		/* PCS speed set for force mode */
> +		switch (speed) {
> +		case SPEED_1000:
> +			val = PCS_MII_SPEED_1000;
> +			break;
> +		case SPEED_100:
> +			val = PCS_MII_SPEED_100;
> +			break;
> +		case SPEED_10:
> +			val = PCS_MII_SPEED_10;
> +			break;
> +		default:
> +			dev_err(qpcs->dev, "Invalid SGMII speed %d\n", speed);
> +			return -EINVAL;
> +		}
> +
> +		ret = regmap_update_bits(qpcs->regmap, PCS_MII_CTRL(index),
> +					 PCS_MII_SPEED_MASK, val);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	/* PCS adapter reset */
> +	ret = regmap_clear_bits(qpcs->regmap,
> +				PCS_MII_CTRL(index), PCS_MII_ADPT_RESET);
> +	if (ret)
> +		return ret;
> +
> +	return regmap_set_bits(qpcs->regmap,
> +			       PCS_MII_CTRL(index), PCS_MII_ADPT_RESET);
> +}
> +
> +static int ipq_pcs_validate(struct phylink_pcs *pcs, unsigned long *supported,
> +			    const struct phylink_link_state *state)
> +{
> +	switch (state->interface) {
> +	case PHY_INTERFACE_MODE_SGMII:
> +	case PHY_INTERFACE_MODE_QSGMII:
> +		return 0;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int ipq_pcs_enable(struct phylink_pcs *pcs)
> +{
> +	struct ipq_pcs_mii *qpcs_mii = phylink_pcs_to_qpcs_mii(pcs);
> +	struct ipq_pcs *qpcs = qpcs_mii->qpcs;
> +	int index = qpcs_mii->index;
> +	int ret;
> +
> +	ret = clk_prepare_enable(qpcs_mii->rx_clk);
> +	if (ret) {
> +		dev_err(qpcs->dev, "Failed to enable MII %d RX clock\n", index);
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(qpcs_mii->tx_clk);
> +	if (ret) {
> +		dev_err(qpcs->dev, "Failed to enable MII %d TX clock\n", index);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static void ipq_pcs_disable(struct phylink_pcs *pcs)
> +{
> +	struct ipq_pcs_mii *qpcs_mii = phylink_pcs_to_qpcs_mii(pcs);
> +
> +	clk_disable_unprepare(qpcs_mii->rx_clk);
> +	clk_disable_unprepare(qpcs_mii->tx_clk);
> +}
> +
> +static void ipq_pcs_get_state(struct phylink_pcs *pcs,
> +			      struct phylink_link_state *state)
> +{
> +	struct ipq_pcs_mii *qpcs_mii = phylink_pcs_to_qpcs_mii(pcs);
> +	struct ipq_pcs *qpcs = qpcs_mii->qpcs;
> +	int index = qpcs_mii->index;
> +
> +	switch (state->interface) {
> +	case PHY_INTERFACE_MODE_SGMII:
> +	case PHY_INTERFACE_MODE_QSGMII:
> +		ipq_pcs_get_state_sgmii(qpcs, index, state);
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	dev_dbg_ratelimited(qpcs->dev,
> +			    "mode=%s/%s/%s link=%u\n",
> +			    phy_modes(state->interface),
> +			    phy_speed_to_str(state->speed),
> +			    phy_duplex_to_str(state->duplex),
> +			    state->link);
> +}
> +
> +static int ipq_pcs_config(struct phylink_pcs *pcs,
> +			  unsigned int neg_mode,
> +			  phy_interface_t interface,
> +			  const unsigned long *advertising,
> +			  bool permit)
> +{
> +	struct ipq_pcs_mii *qpcs_mii = phylink_pcs_to_qpcs_mii(pcs);
> +	struct ipq_pcs *qpcs = qpcs_mii->qpcs;
> +	int index = qpcs_mii->index;
> +
> +	switch (interface) {
> +	case PHY_INTERFACE_MODE_SGMII:
> +	case PHY_INTERFACE_MODE_QSGMII:
> +		return ipq_pcs_config_sgmii(qpcs, index, neg_mode, interface);
> +	default:
> +		return -EOPNOTSUPP;
> +	};
> +}
> +
> +static void ipq_pcs_link_up(struct phylink_pcs *pcs,
> +			    unsigned int neg_mode,
> +			    phy_interface_t interface,
> +			    int speed, int duplex)
> +{
> +	struct ipq_pcs_mii *qpcs_mii = phylink_pcs_to_qpcs_mii(pcs);
> +	struct ipq_pcs *qpcs = qpcs_mii->qpcs;
> +	int index = qpcs_mii->index;
> +	int ret;
> +
> +	switch (interface) {
> +	case PHY_INTERFACE_MODE_SGMII:
> +	case PHY_INTERFACE_MODE_QSGMII:
> +		ret = ipq_pcs_link_up_config_sgmii(qpcs, index,
> +						   neg_mode, speed);
> +		break;
> +	default:
> +		return;
> +	}
> +
> +	if (ret)
> +		dev_err(qpcs->dev, "PCS link up fail for interface %s\n",
> +			phy_modes(interface));
> +}
> +
> +static const struct phylink_pcs_ops ipq_pcs_phylink_ops = {
> +	.pcs_validate = ipq_pcs_validate,
> +	.pcs_enable = ipq_pcs_enable,
> +	.pcs_disable = ipq_pcs_disable,
> +	.pcs_get_state = ipq_pcs_get_state,
> +	.pcs_config = ipq_pcs_config,
> +	.pcs_link_up = ipq_pcs_link_up,
> +};
> +
> +/**
> + * ipq_pcs_get() - Get the IPQ PCS MII instance
> + * @np: Device tree node to the PCS MII
> + *
> + * Description: Get the phylink PCS instance for the given PCS MII node @np.
> + * This instance is associated with the specific MII of the PCS and the
> + * corresponding Ethernet netdevice.
> + *
> + * Return: A pointer to the phylink PCS instance or an error-pointer value.
> + */
> +struct phylink_pcs *ipq_pcs_get(struct device_node *np)
> +{
> +	struct platform_device *pdev;
> +	struct ipq_pcs_mii *qpcs_mii;
> +	struct ipq_pcs *qpcs;
> +	u32 index;
> +
> +	if (of_property_read_u32(np, "reg", &index))
> +		return ERR_PTR(-EINVAL);
> +
> +	if (index >= PCS_MAX_MII_NRS)
> +		return ERR_PTR(-EINVAL);
> +
> +	/* Get the parent device */
> +	pdev = of_find_device_by_node(np->parent);
> +	if (!pdev)
> +		return ERR_PTR(-ENODEV);
> +
> +	qpcs = platform_get_drvdata(pdev);
> +	if (!qpcs) {
> +		put_device(&pdev->dev);
> +
> +		/* If probe is not yet completed, return DEFER to
> +		 * the dependent driver.
> +		 */
> +		return ERR_PTR(-EPROBE_DEFER);
> +	}
> +
> +	qpcs_mii = qpcs->qpcs_mii[index];
> +	if (!qpcs_mii) {
> +		put_device(&pdev->dev);
> +		return ERR_PTR(-ENOENT);
> +	}
> +
> +	return &qpcs_mii->pcs;
> +}
> +EXPORT_SYMBOL(ipq_pcs_get);
> +
> +/**
> + * ipq_pcs_put() - Release the IPQ PCS MII instance
> + * @pcs: PCS instance
> + *
> + * Description: Release a phylink PCS instance.
> + */
> +void ipq_pcs_put(struct phylink_pcs *pcs)
> +{
> +	struct ipq_pcs_mii *qpcs_mii = phylink_pcs_to_qpcs_mii(pcs);
> +
> +	/* Put reference taken by of_find_device_by_node() in
> +	 * ipq_pcs_get().
> +	 */
> +	put_device(qpcs_mii->qpcs->dev);
> +}
> +EXPORT_SYMBOL(ipq_pcs_put);
> +
> +/* Parse the PCS MII DT nodes which are child nodes of the PCS node,
> + * and instantiate each MII PCS instance.
> + */
> +static int ipq_pcs_create_miis(struct ipq_pcs *qpcs)
> +{
> +	struct device *dev = qpcs->dev;
> +	struct ipq_pcs_mii *qpcs_mii;
> +	struct device_node *mii_np;
> +	u32 index;
> +	int ret;
> +
> +	for_each_available_child_of_node(dev->of_node, mii_np) {
> +		ret = of_property_read_u32(mii_np, "reg", &index);
> +		if (ret) {
> +			dev_err(dev, "Failed to read MII index\n");
> +			of_node_put(mii_np);

Assume, the second child node failed here.
Returning without calling the first child node of_node_put().

Please clear the previous child nodes resources before return.

Thanks & Regards,
Manikanta.

> +			return ret;
> +		}
> +
> +		if (index >= PCS_MAX_MII_NRS) {
> +			dev_err(dev, "Invalid MII index\n");
> +			of_node_put(mii_np);
> +			return -EINVAL;
> +		}
> +
> +		qpcs_mii = devm_kzalloc(dev, sizeof(*qpcs_mii), GFP_KERNEL);
> +		if (!qpcs_mii) {
> +			of_node_put(mii_np);
> +			return -ENOMEM;
> +		}
> +
> +		qpcs_mii->qpcs = qpcs;
> +		qpcs_mii->index = index;
> +		qpcs_mii->pcs.ops = &ipq_pcs_phylink_ops;
> +		qpcs_mii->pcs.neg_mode = true;
> +		qpcs_mii->pcs.poll = true;
> +
> +		qpcs_mii->rx_clk = devm_get_clk_from_child(dev, mii_np, "rx");
> +		if (IS_ERR(qpcs_mii->rx_clk)) {
> +			of_node_put(mii_np);
> +			return dev_err_probe(dev, PTR_ERR(qpcs_mii->rx_clk),
> +					     "Failed to get MII %d RX clock\n", index);
> +		}
> +
> +		qpcs_mii->tx_clk = devm_get_clk_from_child(dev, mii_np, "tx");
> +		if (IS_ERR(qpcs_mii->tx_clk)) {
> +			of_node_put(mii_np);
> +			return dev_err_probe(dev, PTR_ERR(qpcs_mii->tx_clk),
> +					     "Failed to get MII %d TX clock\n", index);
> +		}
> +
> +		qpcs->qpcs_mii[index] = qpcs_mii;
> +	}
> +
> +	return 0;
> +}
> +
>  static unsigned long ipq_pcs_clk_rate_get(struct ipq_pcs *qpcs)
>  {
>  	switch (qpcs->interface) {
> @@ -219,6 +676,12 @@ static int ipq9574_pcs_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	ret = ipq_pcs_create_miis(qpcs);
> +	if (ret)
> +		return ret;
> +
> +	mutex_init(&qpcs->config_lock);
> +
>  	platform_set_drvdata(pdev, qpcs);
>  
>  	return 0;
> diff --git a/include/linux/pcs/pcs-qcom-ipq9574.h b/include/linux/pcs/pcs-qcom-ipq9574.h
> new file mode 100644
> index 000000000000..5469a81b4482
> --- /dev/null
> +++ b/include/linux/pcs/pcs-qcom-ipq9574.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
> + *
> + */
> +
> +#ifndef __LINUX_PCS_QCOM_IPQ9574_H
> +#define __LINUX_PCS_QCOM_IPQ9574_H
> +
> +struct device_node;
> +struct phylink_pcs;
> +
> +struct phylink_pcs *ipq_pcs_get(struct device_node *np);
> +void ipq_pcs_put(struct phylink_pcs *pcs);
> +
> +#endif /* __LINUX_PCS_QCOM_IPQ9574_H */
> 

Powered by blists - more mailing lists