Hi Luo
For a patchset, netdev wants to see a patch 0/X which describes the
big picture. What is the patchset as a whole doing.
> +static int ipq_mdio_reset(struct mii_bus *bus)
> +{
> + struct ipq4019_mdio_data *priv = bus->priv;
> + struct device *dev = bus->parent;
> + struct gpio_desc *reset_gpio;
> + u32 val;
> + int i, ret;
> +
> + /* To indicate CMN_PLL that ethernet_ldo has been ready if needed */
> + if (!IS_ERR(priv->eth_ldo_rdy)) {
> + val = readl(priv->eth_ldo_rdy);
> + val |= BIT(0);
> + writel(val, priv->eth_ldo_rdy);
> + fsleep(QCA_PHY_SET_DELAY_US);
> + }
> +
> + /* Reset GEPHY if need */
> + if (!IS_ERR(priv->reset_ctrl)) {
> + reset_control_assert(priv->reset_ctrl);
> + fsleep(QCA_PHY_SET_DELAY_US);
> + reset_control_deassert(priv->reset_ctrl);
> + fsleep(QCA_PHY_SET_DELAY_US);
> + }
What exactly is being reset here? Which is GEPHY?
The MDIO bus master driver should not be touching any Ethernet
PHYs. All it provides is a bus, nothing more.
> +
> + /* Configure MDIO clock frequency */
> + if (!IS_ERR(priv->mdio_clk)) {
> + ret = clk_set_rate(priv->mdio_clk, QCA_MDIO_CLK_RATE);
> + if (ret)
> + return ret;
> +
> + ret = clk_prepare_enable(priv->mdio_clk);
> + if (ret)
> + return ret;
> + }
> +
> + /* Reset PHYs by gpio pins */
> + for (i = 0; i < gpiod_count(dev, "phy-reset"); i++) {
> + reset_gpio = gpiod_get_index_optional(dev, "phy-reset", i, GPIOD_OUT_HIGH);
> + if (IS_ERR(reset_gpio))
> + continue;
> + gpiod_set_value_cansleep(reset_gpio, 0);
> + fsleep(QCA_PHY_SET_DELAY_US);
> + gpiod_set_value_cansleep(reset_gpio, 1);
> + fsleep(QCA_PHY_SET_DELAY_US);
> + gpiod_put(reset_gpio);
> + }
No, there is common code in phylib to do that.
> 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 +244,23 @@ 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);
>
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + if (res)
> + priv->eth_ldo_rdy = devm_ioremap_resource(&pdev->dev, res);
> +
> + priv->reset_ctrl = devm_reset_control_get_exclusive(&pdev->dev, "gephy_mdc_rst");
> + priv->mdio_clk = devm_clk_get(&pdev->dev, "gcc_mdio_ahb_clk");
You probably want to use devm_clk_get_optional().
Andrew
