diff mbox series

[net-next,v12,3/7] stmmac: dwmac-mediatek: re-arrange clock setting

Message ID 20220117070706.17853-4-biao.huang@mediatek.com (mailing list archive)
State New, archived
Headers show
Series MediaTek Ethernet Patches on MT8195 | expand

Commit Message

Biao Huang (黄彪) Jan. 17, 2022, 7:07 a.m. UTC 
The rmii_internal clock is needed only when PHY
interface is RMII, and reference clock is from MAC.

Re-arrange the clock setting as following:
1. the optional "rmii_internal" is controlled by devm_clk_get(),
2. other clocks still be configured by devm_clk_bulk_get().

Signed-off-by: Biao Huang <biao.huang@mediatek.com>
---
 .../ethernet/stmicro/stmmac/dwmac-mediatek.c  | 72 +++++++++++++------
 1 file changed, 49 insertions(+), 23 deletions(-)

Comments

AngeloGioacchino Del Regno Jan. 17, 2022, 10:38 a.m. UTC | #1 
Il 17/01/22 08:07, Biao Huang ha scritto:
> The rmii_internal clock is needed only when PHY
> interface is RMII, and reference clock is from MAC.
> 
> Re-arrange the clock setting as following:
> 1. the optional "rmii_internal" is controlled by devm_clk_get(),
> 2. other clocks still be configured by devm_clk_bulk_get().
> 
> Signed-off-by: Biao Huang <biao.huang@mediatek.com>
> ---
>   .../ethernet/stmicro/stmmac/dwmac-mediatek.c  | 72 +++++++++++++------
>   1 file changed, 49 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> index 8747aa4403e8..2678d2deb26a 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> @@ -49,14 +49,15 @@ struct mac_delay_struct {
>   struct mediatek_dwmac_plat_data {
>   	const struct mediatek_dwmac_variant *variant;
>   	struct mac_delay_struct mac_delay;
> +	struct clk *rmii_internal_clk;
>   	struct clk_bulk_data *clks;
> -	struct device_node *np;
>   	struct regmap *peri_regmap;
> +	struct device_node *np;
>   	struct device *dev;
>   	phy_interface_t phy_mode;
> -	int num_clks_to_config;
>   	bool rmii_clk_from_mac;
>   	bool rmii_rxc;
> +	int num_clks;

I don't see any need to get a num_clks here, at this point: since all functions
reading this are getting passed a pointer to this entire structure, you can
simply always access plat->variant->num_clks.

Please, drop the addition of num_clks in this struct.

Regards,
Angelo
Biao Huang (黄彪) Jan. 18, 2022, 1:51 a.m. UTC | #2 
Dear Angelo,
	Thanks for your comments.
On Mon, 2022-01-17 at 11:38 +0100, AngeloGioacchino Del Regno wrote:
> Il 17/01/22 08:07, Biao Huang ha scritto:
> > The rmii_internal clock is needed only when PHY
> > interface is RMII, and reference clock is from MAC.
> > 
> > Re-arrange the clock setting as following:
> > 1. the optional "rmii_internal" is controlled by devm_clk_get(),
> > 2. other clocks still be configured by devm_clk_bulk_get().
> > 
> > Signed-off-by: Biao Huang <biao.huang@mediatek.com>
> > ---
> >   .../ethernet/stmicro/stmmac/dwmac-mediatek.c  | 72 +++++++++++++-
> > -----
> >   1 file changed, 49 insertions(+), 23 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> > b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> > index 8747aa4403e8..2678d2deb26a 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
> > @@ -49,14 +49,15 @@ struct mac_delay_struct {
> >   struct mediatek_dwmac_plat_data {
> >   	const struct mediatek_dwmac_variant *variant;
> >   	struct mac_delay_struct mac_delay;
> > +	struct clk *rmii_internal_clk;
> >   	struct clk_bulk_data *clks;
> > -	struct device_node *np;
> >   	struct regmap *peri_regmap;
> > +	struct device_node *np;
> >   	struct device *dev;
> >   	phy_interface_t phy_mode;
> > -	int num_clks_to_config;
> >   	bool rmii_clk_from_mac;
> >   	bool rmii_rxc;
> > +	int num_clks;
> 
> I don't see any need to get a num_clks here, at this point: since all
> functions
> reading this are getting passed a pointer to this entire structure,
> you can
> simply always access plat->variant->num_clks.
> 
> Please, drop the addition of num_clks in this struct.
> 
> Regards,
> Angelo
OK, will remove it in next send.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
index 8747aa4403e8..2678d2deb26a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-mediatek.c
@@ -49,14 +49,15 @@  struct mac_delay_struct {
 struct mediatek_dwmac_plat_data {
 	const struct mediatek_dwmac_variant *variant;
 	struct mac_delay_struct mac_delay;
+	struct clk *rmii_internal_clk;
 	struct clk_bulk_data *clks;
-	struct device_node *np;
 	struct regmap *peri_regmap;
+	struct device_node *np;
 	struct device *dev;
 	phy_interface_t phy_mode;
-	int num_clks_to_config;
 	bool rmii_clk_from_mac;
 	bool rmii_rxc;
+	int num_clks;
 };
 
 struct mediatek_dwmac_variant {
@@ -74,7 +75,7 @@  struct mediatek_dwmac_variant {
 
 /* list of clocks required for mac */
 static const char * const mt2712_dwmac_clk_l[] = {
-	"axi", "apb", "mac_main", "ptp_ref", "rmii_internal"
+	"axi", "apb", "mac_main", "ptp_ref"
 };
 
 static int mt2712_set_interface(struct mediatek_dwmac_plat_data *plat)
@@ -83,23 +84,12 @@  static int mt2712_set_interface(struct mediatek_dwmac_plat_data *plat)
 	int rmii_rxc = plat->rmii_rxc ? RMII_CLK_SRC_RXC : 0;
 	u32 intf_val = 0;
 
-	/* The clock labeled as "rmii_internal" in mt2712_dwmac_clk_l is needed
-	 * only in RMII(when MAC provides the reference clock), and useless for
-	 * RGMII/MII/RMII(when PHY provides the reference clock).
-	 * num_clks_to_config indicates the real number of clocks should be
-	 * configured, equals to (plat->variant->num_clks - 1) in default for all the case,
-	 * then +1 for rmii_clk_from_mac case.
-	 */
-	plat->num_clks_to_config = plat->variant->num_clks - 1;
-
 	/* select phy interface in top control domain */
 	switch (plat->phy_mode) {
 	case PHY_INTERFACE_MODE_MII:
 		intf_val |= PHY_INTF_MII;
 		break;
 	case PHY_INTERFACE_MODE_RMII:
-		if (plat->rmii_clk_from_mac)
-			plat->num_clks_to_config++;
 		intf_val |= (PHY_INTF_RMII | rmii_rxc | rmii_clk_from_mac);
 		break;
 	case PHY_INTERFACE_MODE_RGMII:
@@ -314,18 +304,36 @@  static int mediatek_dwmac_config_dt(struct mediatek_dwmac_plat_data *plat)
 static int mediatek_dwmac_clk_init(struct mediatek_dwmac_plat_data *plat)
 {
 	const struct mediatek_dwmac_variant *variant = plat->variant;
-	int i, num = variant->num_clks;
+	int i, ret;
+
+	plat->num_clks = variant->num_clks;
 
-	plat->clks = devm_kcalloc(plat->dev, num, sizeof(*plat->clks), GFP_KERNEL);
+	plat->clks = devm_kcalloc(plat->dev, plat->num_clks, sizeof(*plat->clks), GFP_KERNEL);
 	if (!plat->clks)
 		return -ENOMEM;
 
-	for (i = 0; i < num; i++)
+	for (i = 0; i < plat->num_clks; i++)
 		plat->clks[i].id = variant->clk_list[i];
 
-	plat->num_clks_to_config = variant->num_clks;
+	ret = devm_clk_bulk_get(plat->dev, plat->num_clks, plat->clks);
+	if (ret)
+		return ret;
+
+	/* The clock labeled as "rmii_internal" is needed only in RMII(when
+	 * MAC provides the reference clock), and useless for RGMII/MII or
+	 * RMII(when PHY provides the reference clock).
+	 * So, "rmii_internal" clock is got and configured only when
+	 * reference clock of RMII is from MAC.
+	 */
+	if (plat->rmii_clk_from_mac) {
+		plat->rmii_internal_clk = devm_clk_get(plat->dev, "rmii_internal");
+		if (IS_ERR(plat->rmii_internal_clk))
+			ret = PTR_ERR(plat->rmii_internal_clk);
+	} else {
+		plat->rmii_internal_clk = NULL;
+	}
 
-	return devm_clk_bulk_get(plat->dev, num, plat->clks);
+	return ret;
 }
 
 static int mediatek_dwmac_init(struct platform_device *pdev, void *priv)
@@ -350,20 +358,31 @@  static int mediatek_dwmac_init(struct platform_device *pdev, void *priv)
 		}
 	}
 
-	ret = clk_bulk_prepare_enable(plat->num_clks_to_config, plat->clks);
+	ret = clk_bulk_prepare_enable(plat->num_clks, plat->clks);
 	if (ret) {
 		dev_err(plat->dev, "failed to enable clks, err = %d\n", ret);
 		return ret;
 	}
 
+	ret = clk_prepare_enable(plat->rmii_internal_clk);
+	if (ret) {
+		dev_err(plat->dev, "failed to enable rmii internal clk, err = %d\n", ret);
+		goto err_clk;
+	}
+
 	return 0;
+
+err_clk:
+	clk_bulk_disable_unprepare(plat->num_clks, plat->clks);
+	return ret;
 }
 
 static void mediatek_dwmac_exit(struct platform_device *pdev, void *priv)
 {
 	struct mediatek_dwmac_plat_data *plat = priv;
 
-	clk_bulk_disable_unprepare(plat->num_clks_to_config, plat->clks);
+	clk_disable_unprepare(plat->rmii_internal_clk);
+	clk_bulk_disable_unprepare(plat->num_clks, plat->clks);
 }
 
 static int mediatek_dwmac_clks_config(void *priv, bool enabled)
@@ -372,13 +391,20 @@  static int mediatek_dwmac_clks_config(void *priv, bool enabled)
 	int ret = 0;
 
 	if (enabled) {
-		ret = clk_bulk_prepare_enable(plat->num_clks_to_config, plat->clks);
+		ret = clk_bulk_prepare_enable(plat->num_clks, plat->clks);
 		if (ret) {
 			dev_err(plat->dev, "failed to enable clks, err = %d\n", ret);
 			return ret;
 		}
+
+		ret = clk_prepare_enable(plat->rmii_internal_clk);
+		if (ret) {
+			dev_err(plat->dev, "failed to enable rmii internal clk, err = %d\n", ret);
+			return ret;
+		}
 	} else {
-		clk_bulk_disable_unprepare(plat->num_clks_to_config, plat->clks);
+		clk_disable_unprepare(plat->rmii_internal_clk);
+		clk_bulk_disable_unprepare(plat->num_clks, plat->clks);
 	}
 
 	return ret;