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