netdev - Re: [PATCH v2 2/5] net: mdio: ipq4019: enable the SoC uniphy clocks for ipq5332 platform

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Tue, 12 Dec 2023 13:46:21 +0100
From: Maxime Chevallier <maxime.chevallier@...tlin.com>
To: Luo Jie <quic_luoj@...cinc.com>
Cc: <agross@...nel.org>, <andersson@...nel.org>, <konrad.dybcio@...aro.org>,
 <davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
 <pabeni@...hat.com>, <robh+dt@...nel.org>,
 <krzysztof.kozlowski+dt@...aro.org>, <conor+dt@...nel.org>,
 <andrew@...n.ch>, <hkallweit1@...il.com>, <linux@...linux.org.uk>,
 <robert.marko@...tura.hr>, <linux-arm-msm@...r.kernel.org>,
 <netdev@...r.kernel.org>, <devicetree@...r.kernel.org>,
 <linux-kernel@...r.kernel.org>, <quic_srichara@...cinc.com>
Subject: Re: [PATCH v2 2/5] net: mdio: ipq4019: enable the SoC uniphy clocks
 for ipq5332 platform

Hello,

On Tue, 12 Dec 2023 19:51:47 +0800
Luo Jie <quic_luoj@...cinc.com> 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@...cinc.com>

[...]

>  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;
> +	int ret, index;
> +	unsigned long rate;

Please remember to use reverse christmas-tree ordering, meaning longer
declaration lines go first :

	struct ipq4019_mdio_data *priv = bus->priv;
	unsigned long rate;
	int ret, index;

> +
> +	/* 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;
> +		}
>  
> -	/* 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;
> +		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]);
> +	}

You should be able to use devm_clk_bulk_get_optional(), to avoid that
loop.

Thanks,

Maxime