diff mbox series

[v4,2/5] net: mdio: ipq4019: enable the SoC uniphy clocks for ipq5332 platform

Message ID 20231225084424.30986-3-quic_luoj@quicinc.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series support ipq5332 platform | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Guessed tree name to be net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1113 this patch: 1113
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 1140 this patch: 1140
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1140 this patch: 1140
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 109 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Luo Jie Dec. 25, 2023, 8:44 a.m. UTC
On the platform ipq5332, the related SoC uniphy GCC clocks need
to be enabled for making the MDIO slave devices accessible.

These UNIPHY clocks are from the SoC platform GCC clock provider,
which are enabled for the connected PHY devices working.

Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
 drivers/net/mdio/mdio-ipq4019.c | 75 ++++++++++++++++++++++++++++-----
 1 file changed, 64 insertions(+), 11 deletions(-)

Comments

Bryan O'Donoghue Jan. 3, 2024, 9:48 a.m. UTC | #1
On 25/12/2023 08:44, Luo Jie wrote:
> On the platform ipq5332, the related SoC uniphy GCC clocks need
> to be enabled for making the MDIO slave devices accessible.
> 
> These UNIPHY clocks are from the SoC platform GCC clock provider,
> which are enabled for the connected PHY devices working.
> 
> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
> ---
>   drivers/net/mdio/mdio-ipq4019.c | 75 ++++++++++++++++++++++++++++-----
>   1 file changed, 64 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/mdio/mdio-ipq4019.c b/drivers/net/mdio/mdio-ipq4019.c
> index 5273864fabb3..e24b0e688b10 100644
> --- a/drivers/net/mdio/mdio-ipq4019.c
> +++ b/drivers/net/mdio/mdio-ipq4019.c
> @@ -35,15 +35,36 @@
>   /* MDIO clock source frequency is fixed to 100M */
>   #define IPQ_MDIO_CLK_RATE	100000000
>   
> +/* SoC UNIPHY fixed clock */
> +#define IPQ_UNIPHY_AHB_CLK_RATE	100000000
> +#define IPQ_UNIPHY_SYS_CLK_RATE	24000000
> +
>   #define IPQ_PHY_SET_DELAY_US	100000
>   
>   /* Maximum SOC PCS(uniphy) number on IPQ platform */
>   #define ETH_LDO_RDY_CNT				3
>   
> +enum mdio_clk_id {
> +	MDIO_CLK_MDIO_AHB,
> +	MDIO_CLK_UNIPHY0_AHB,
> +	MDIO_CLK_UNIPHY0_SYS,
> +	MDIO_CLK_UNIPHY1_AHB,
> +	MDIO_CLK_UNIPHY1_SYS,
> +	MDIO_CLK_CNT
> +};
> +
>   struct ipq4019_mdio_data {
>   	void __iomem *membase;
>   	void __iomem *eth_ldo_rdy[ETH_LDO_RDY_CNT];
> -	struct clk *mdio_clk;
> +	struct clk *clk[MDIO_CLK_CNT];
> +};
> +
> +static const char *const mdio_clk_name[] = {
> +	"gcc_mdio_ahb_clk",
> +	"uniphy0_ahb",
> +	"uniphy0_sys",
> +	"uniphy1_ahb",
> +	"uniphy1_sys"
>   };
>   
>   static int ipq4019_mdio_wait_busy(struct mii_bus *bus)
> @@ -209,14 +230,43 @@ static int ipq4019_mdio_write_c22(struct mii_bus *bus, int mii_id, int regnum,
>   static int ipq_mdio_reset(struct mii_bus *bus)
>   {
>   	struct ipq4019_mdio_data *priv = bus->priv;
> -	int ret;
> +	unsigned long rate;
> +	int ret, index;
>   
> -	/* Configure MDIO clock source frequency if clock is specified in the device tree */
> -	ret = clk_set_rate(priv->mdio_clk, IPQ_MDIO_CLK_RATE);
> -	if (ret)
> -		return ret;
> +	/* For the platform ipq5332, there are two SoC uniphies available
> +	 * for connecting with ethernet PHY, the SoC uniphy gcc clock
> +	 * should be enabled for resetting the connected device such
> +	 * as qca8386 switch, qca8081 PHY or other PHYs effectively.
> +	 *
> +	 * Configure MDIO/UNIPHY clock source frequency if clock instance
> +	 * is specified in the device tree.
> +	 */
> +	for (index = MDIO_CLK_MDIO_AHB; index < MDIO_CLK_CNT; index++) {

you could do a

if (!priv->clk[index])
	continue;

here and save a few cycles executing code for absent clocks. ipq6018 has 
just 1/5 of the clocks you are checking for here.

Better still capture the number of clocks you find in probe() in a 
variable priv->num_clocks and only step through the array

for (i = 0; i < priv->num_clocks; i++) {}

---
bod
Luo Jie Jan. 3, 2024, 1:25 p.m. UTC | #2
On 1/3/2024 5:48 PM, Bryan O'Donoghue wrote:
> On 25/12/2023 08:44, Luo Jie wrote:
>> On the platform ipq5332, the related SoC uniphy GCC clocks need
>> to be enabled for making the MDIO slave devices accessible.
>>
>> These UNIPHY clocks are from the SoC platform GCC clock provider,
>> which are enabled for the connected PHY devices working.
>>
>> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
>> ---
>>   drivers/net/mdio/mdio-ipq4019.c | 75 ++++++++++++++++++++++++++++-----
>>   1 file changed, 64 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/net/mdio/mdio-ipq4019.c 
>> b/drivers/net/mdio/mdio-ipq4019.c
>> index 5273864fabb3..e24b0e688b10 100644
>> --- a/drivers/net/mdio/mdio-ipq4019.c
>> +++ b/drivers/net/mdio/mdio-ipq4019.c
>> @@ -35,15 +35,36 @@
>>   /* MDIO clock source frequency is fixed to 100M */
>>   #define IPQ_MDIO_CLK_RATE    100000000
>> +/* SoC UNIPHY fixed clock */
>> +#define IPQ_UNIPHY_AHB_CLK_RATE    100000000
>> +#define IPQ_UNIPHY_SYS_CLK_RATE    24000000
>> +
>>   #define IPQ_PHY_SET_DELAY_US    100000
>>   /* Maximum SOC PCS(uniphy) number on IPQ platform */
>>   #define ETH_LDO_RDY_CNT                3
>> +enum mdio_clk_id {
>> +    MDIO_CLK_MDIO_AHB,
>> +    MDIO_CLK_UNIPHY0_AHB,
>> +    MDIO_CLK_UNIPHY0_SYS,
>> +    MDIO_CLK_UNIPHY1_AHB,
>> +    MDIO_CLK_UNIPHY1_SYS,
>> +    MDIO_CLK_CNT
>> +};
>> +
>>   struct ipq4019_mdio_data {
>>       void __iomem *membase;
>>       void __iomem *eth_ldo_rdy[ETH_LDO_RDY_CNT];
>> -    struct clk *mdio_clk;
>> +    struct clk *clk[MDIO_CLK_CNT];
>> +};
>> +
>> +static const char *const mdio_clk_name[] = {
>> +    "gcc_mdio_ahb_clk",
>> +    "uniphy0_ahb",
>> +    "uniphy0_sys",
>> +    "uniphy1_ahb",
>> +    "uniphy1_sys"
>>   };
>>   static int ipq4019_mdio_wait_busy(struct mii_bus *bus)
>> @@ -209,14 +230,43 @@ static int ipq4019_mdio_write_c22(struct mii_bus 
>> *bus, int mii_id, int regnum,
>>   static int ipq_mdio_reset(struct mii_bus *bus)
>>   {
>>       struct ipq4019_mdio_data *priv = bus->priv;
>> -    int ret;
>> +    unsigned long rate;
>> +    int ret, index;
>> -    /* Configure MDIO clock source frequency if clock is specified in 
>> the device tree */
>> -    ret = clk_set_rate(priv->mdio_clk, IPQ_MDIO_CLK_RATE);
>> -    if (ret)
>> -        return ret;
>> +    /* For the platform ipq5332, there are two SoC uniphies available
>> +     * for connecting with ethernet PHY, the SoC uniphy gcc clock
>> +     * should be enabled for resetting the connected device such
>> +     * as qca8386 switch, qca8081 PHY or other PHYs effectively.
>> +     *
>> +     * Configure MDIO/UNIPHY clock source frequency if clock instance
>> +     * is specified in the device tree.
>> +     */
>> +    for (index = MDIO_CLK_MDIO_AHB; index < MDIO_CLK_CNT; index++) {
> 
> you could do a
> 
> if (!priv->clk[index])
>      continue;

Thanks for the comments, will update it.
> 
> here and save a few cycles executing code for absent clocks. ipq6018 has 
> just 1/5 of the clocks you are checking for here.
> 
> Better still capture the number of clocks you find in probe() in a 
> variable priv->num_clocks and only step through the array

Ok, thanks bod for the comments to improve the code.

> 
> for (i = 0; i < priv->num_clocks; i++) {}
> 
> ---
> bod
diff mbox series

Patch

diff --git a/drivers/net/mdio/mdio-ipq4019.c b/drivers/net/mdio/mdio-ipq4019.c
index 5273864fabb3..e24b0e688b10 100644
--- a/drivers/net/mdio/mdio-ipq4019.c
+++ b/drivers/net/mdio/mdio-ipq4019.c
@@ -35,15 +35,36 @@ 
 /* MDIO clock source frequency is fixed to 100M */
 #define IPQ_MDIO_CLK_RATE	100000000
 
+/* SoC UNIPHY fixed clock */
+#define IPQ_UNIPHY_AHB_CLK_RATE	100000000
+#define IPQ_UNIPHY_SYS_CLK_RATE	24000000
+
 #define IPQ_PHY_SET_DELAY_US	100000
 
 /* Maximum SOC PCS(uniphy) number on IPQ platform */
 #define ETH_LDO_RDY_CNT				3
 
+enum mdio_clk_id {
+	MDIO_CLK_MDIO_AHB,
+	MDIO_CLK_UNIPHY0_AHB,
+	MDIO_CLK_UNIPHY0_SYS,
+	MDIO_CLK_UNIPHY1_AHB,
+	MDIO_CLK_UNIPHY1_SYS,
+	MDIO_CLK_CNT
+};
+
 struct ipq4019_mdio_data {
 	void __iomem *membase;
 	void __iomem *eth_ldo_rdy[ETH_LDO_RDY_CNT];
-	struct clk *mdio_clk;
+	struct clk *clk[MDIO_CLK_CNT];
+};
+
+static const char *const mdio_clk_name[] = {
+	"gcc_mdio_ahb_clk",
+	"uniphy0_ahb",
+	"uniphy0_sys",
+	"uniphy1_ahb",
+	"uniphy1_sys"
 };
 
 static int ipq4019_mdio_wait_busy(struct mii_bus *bus)
@@ -209,14 +230,43 @@  static int ipq4019_mdio_write_c22(struct mii_bus *bus, int mii_id, int regnum,
 static int ipq_mdio_reset(struct mii_bus *bus)
 {
 	struct ipq4019_mdio_data *priv = bus->priv;
-	int ret;
+	unsigned long rate;
+	int ret, index;
 
-	/* Configure MDIO clock source frequency if clock is specified in the device tree */
-	ret = clk_set_rate(priv->mdio_clk, IPQ_MDIO_CLK_RATE);
-	if (ret)
-		return ret;
+	/* For the platform ipq5332, there are two SoC uniphies available
+	 * for connecting with ethernet PHY, the SoC uniphy gcc clock
+	 * should be enabled for resetting the connected device such
+	 * as qca8386 switch, qca8081 PHY or other PHYs effectively.
+	 *
+	 * Configure MDIO/UNIPHY clock source frequency if clock instance
+	 * is specified in the device tree.
+	 */
+	for (index = MDIO_CLK_MDIO_AHB; index < MDIO_CLK_CNT; index++) {
+		switch (index) {
+		case MDIO_CLK_MDIO_AHB:
+			rate = IPQ_MDIO_CLK_RATE;
+			break;
+		case MDIO_CLK_UNIPHY0_AHB:
+		case MDIO_CLK_UNIPHY1_AHB:
+			rate = IPQ_UNIPHY_AHB_CLK_RATE;
+			break;
+		case MDIO_CLK_UNIPHY0_SYS:
+		case MDIO_CLK_UNIPHY1_SYS:
+			rate = IPQ_UNIPHY_SYS_CLK_RATE;
+			break;
+		default:
+			break;
+		}
+
+		ret = clk_set_rate(priv->clk[index], rate);
+		if (ret)
+			return ret;
+
+		ret = clk_prepare_enable(priv->clk[index]);
+		if (ret)
+			return ret;
+	}
 
-	ret = clk_prepare_enable(priv->mdio_clk);
 	if (ret == 0)
 		mdelay(10);
 
@@ -240,10 +290,6 @@  static int ipq4019_mdio_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->membase))
 		return PTR_ERR(priv->membase);
 
-	priv->mdio_clk = devm_clk_get_optional(&pdev->dev, "gcc_mdio_ahb_clk");
-	if (IS_ERR(priv->mdio_clk))
-		return PTR_ERR(priv->mdio_clk);
-
 	/* These platform resources are provided on the chipset IPQ5018 or
 	 * IPQ5332.
 	 */
@@ -271,6 +317,13 @@  static int ipq4019_mdio_probe(struct platform_device *pdev)
 		}
 	}
 
+	for (index = 0; index < MDIO_CLK_CNT; index++) {
+		priv->clk[index] = devm_clk_get_optional(&pdev->dev,
+							 mdio_clk_name[index]);
+		if (IS_ERR(priv->clk[index]))
+			return PTR_ERR(priv->clk[index]);
+	}
+
 	bus->name = "ipq4019_mdio";
 	bus->read = ipq4019_mdio_read_c22;
 	bus->write = ipq4019_mdio_write_c22;