| Message ID | 20231212115151.20016-3-quic_luoj@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | support ipq5332 platform | expand |
Hello, On Tue, 12 Dec 2023 19:51:47 +0800 Luo Jie <quic_luoj@quicinc.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@quicinc.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
On 12/12/2023 8:46 PM, Maxime Chevallier wrote: > Hello, > > On Tue, 12 Dec 2023 19:51:47 +0800 > Luo Jie <quic_luoj@quicinc.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@quicinc.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; Thanks, i will update this. > >> + >> + /* 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 Thanks Maxime for the suggestion. These clocks need to be configured the different clock rate, MDIO system clock works on 100MHZ, but UNIPHY system clock works on 24MHZ. For the clock rate set, i still need the loop to configure the different clock rate on the different clock instance. So i use the devm_clk_get_optional to acquire the exact clock ID here.
diff --git a/drivers/net/mdio/mdio-ipq4019.c b/drivers/net/mdio/mdio-ipq4019.c index 5273864fabb3..582e41ab0990 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", + "gcc_uniphy0_ahb_clk", + "gcc_uniphy0_sys_clk", + "gcc_uniphy1_ahb_clk", + "gcc_uniphy1_sys_clk" }; 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; + + /* 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]); + } + bus->name = "ipq4019_mdio"; bus->read = ipq4019_mdio_read_c22; bus->write = ipq4019_mdio_write_c22;
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(-)