| Message ID | 20210808072111.8365-2-luoj@codeaurora.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | net: mdio: Add IPQ MDIO reset related function | expand |
On 8/8/2021 11:27 PM, Andrew Lunn wrote: >> +static int ipq_mdio_reset(struct mii_bus *bus) >> +{ >> + struct ipq4019_mdio_data *priv = bus->priv; >> + u32 val; >> + int ret; >> + >> + /* To indicate CMN_PLL that ethernet_ldo has been ready if platform resource 1 >> + * is specified in the device tree. >> + * */ >> + if (!IS_ERR(priv->eth_ldo_rdy)) { >> + val = readl(priv->eth_ldo_rdy); >> + val |= BIT(0); >> + writel(val, priv->eth_ldo_rdy); >> + fsleep(IPQ_PHY_SET_DELAY_US); >> + } >> + >> + /* Configure MDIO clock source frequency if clock is specified in the device tree */ >> + if (!IS_ERR_OR_NULL(priv->mdio_clk)) { >> + ret = clk_set_rate(priv->mdio_clk, IPQ_MDIO_CLK_RATE); >> + if (ret) >> + return ret; >> + >> + ret = clk_prepare_enable(priv->mdio_clk); >> + if (ret) >> + return ret; >> + } > These !IS_ERR() are pretty ugly. So > >> @@ -182,14 +221,22 @@ static int ipq4019_mdio_probe(struct platform_device *pdev) >> return -ENOMEM; >> >> priv = bus->priv; >> + priv->eth_ldo_rdy = IOMEM_ERR_PTR(-EINVAL); >> >> priv->membase = devm_platform_ioremap_resource(pdev, 0); >> if (IS_ERR(priv->membase)) >> return PTR_ERR(priv->membase); >> >> + priv->mdio_clk = devm_clk_get_optional(&pdev->dev, "gcc_mdio_ahb_clk"); > If this returns an error, it is a real error. You should not ignore > it. Fail the probe returning the error. That then means when the reset > function is called priv->mdio_clk contains either a clock, or NULL, > which the clk API is happy to take. No need for an if. > > >> + >> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); >> + if (res) >> + priv->eth_ldo_rdy = devm_ioremap_resource(&pdev->dev, res); > platform_get_resource() returns a pointer or NULL. There is no error > code. So > >> + if (!IS_ERR(priv->eth_ldo_rdy)) { > is actually wrong, should simply become > >> + if (priv->eth_ldo_rdy) { > Andrew Hi Andrew, Thanks for the kindly review and the comments, will follow it in the next patch set.
On 8/8/2021 11:29 PM, Andrew Lunn wrote: >> + /* Configure MDIO clock source frequency if clock is specified in the device tree */ >> + if (!IS_ERR_OR_NULL(priv->mdio_clk)) { > Please document the clock in the binding. Make it clear which devices > require the clock, and which don't. > > Andrew sure, will document it in the next patch set, thanks Andrew.
diff --git a/drivers/net/mdio/Kconfig b/drivers/net/mdio/Kconfig index 99a6c13a11af..a94d34cc7dc1 100644 --- a/drivers/net/mdio/Kconfig +++ b/drivers/net/mdio/Kconfig @@ -169,6 +169,7 @@ config MDIO_OCTEON config MDIO_IPQ4019 tristate "Qualcomm IPQ4019 MDIO interface support" depends on HAS_IOMEM && OF_MDIO + depends on COMMON_CLK help This driver supports the MDIO interface found in Qualcomm IPQ40xx series Soc-s. diff --git a/drivers/net/mdio/mdio-ipq4019.c b/drivers/net/mdio/mdio-ipq4019.c index 9cd71d896963..b365f13c92ca 100644 --- a/drivers/net/mdio/mdio-ipq4019.c +++ b/drivers/net/mdio/mdio-ipq4019.c @@ -11,6 +11,7 @@ #include <linux/of_mdio.h> #include <linux/phy.h> #include <linux/platform_device.h> +#include <linux/clk.h> #define MDIO_MODE_REG 0x40 #define MDIO_ADDR_REG 0x44 @@ -31,8 +32,15 @@ #define IPQ4019_MDIO_TIMEOUT 10000 #define IPQ4019_MDIO_SLEEP 10 +/* MDIO clock source frequency is fixed to 100M */ +#define IPQ_MDIO_CLK_RATE 100000000 + +#define IPQ_PHY_SET_DELAY_US 100000 + struct ipq4019_mdio_data { void __iomem *membase; + void __iomem *eth_ldo_rdy; + struct clk *mdio_clk; }; static int ipq4019_mdio_wait_busy(struct mii_bus *bus) @@ -171,10 +179,41 @@ static int ipq4019_mdio_write(struct mii_bus *bus, int mii_id, int regnum, return 0; } +static int ipq_mdio_reset(struct mii_bus *bus) +{ + struct ipq4019_mdio_data *priv = bus->priv; + u32 val; + int ret; + + /* To indicate CMN_PLL that ethernet_ldo has been ready if platform resource 1 + * is specified in the device tree. + * */ + if (!IS_ERR(priv->eth_ldo_rdy)) { + val = readl(priv->eth_ldo_rdy); + val |= BIT(0); + writel(val, priv->eth_ldo_rdy); + fsleep(IPQ_PHY_SET_DELAY_US); + } + + /* Configure MDIO clock source frequency if clock is specified in the device tree */ + if (!IS_ERR_OR_NULL(priv->mdio_clk)) { + ret = clk_set_rate(priv->mdio_clk, IPQ_MDIO_CLK_RATE); + if (ret) + return ret; + + ret = clk_prepare_enable(priv->mdio_clk); + if (ret) + return ret; + } + + return 0; +} + static int ipq4019_mdio_probe(struct platform_device *pdev) { struct ipq4019_mdio_data *priv; struct mii_bus *bus; + struct resource *res; int ret; bus = devm_mdiobus_alloc_size(&pdev->dev, sizeof(*priv)); @@ -182,14 +221,22 @@ static int ipq4019_mdio_probe(struct platform_device *pdev) return -ENOMEM; priv = bus->priv; + priv->eth_ldo_rdy = IOMEM_ERR_PTR(-EINVAL); priv->membase = devm_platform_ioremap_resource(pdev, 0); if (IS_ERR(priv->membase)) return PTR_ERR(priv->membase); + priv->mdio_clk = devm_clk_get_optional(&pdev->dev, "gcc_mdio_ahb_clk"); + + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); + if (res) + priv->eth_ldo_rdy = devm_ioremap_resource(&pdev->dev, res); + bus->name = "ipq4019_mdio"; bus->read = ipq4019_mdio_read; bus->write = ipq4019_mdio_write; + bus->reset = ipq_mdio_reset; bus->parent = &pdev->dev; snprintf(bus->id, MII_BUS_ID_SIZE, "%s%d", pdev->name, pdev->id);
1. configure the MDIO clock source frequency. 2. the LDO resource is needed to indicate the ethernet LDO ready for CMN_PLL. Signed-off-by: Luo Jie <luoj@codeaurora.org> --- drivers/net/mdio/Kconfig | 1 + drivers/net/mdio/mdio-ipq4019.c | 47 +++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+)