Netdev Archive on lore.kernel.org help / color / mirror / Atom feed
* [PATCH 1/3] net: mdio-ipq4019: Add mdio reset function @ 2021-07-29 12:53 Luo Jie 2021-07-29 12:53 ` [PATCH 2/3] net: mdio-ipq4019: rename mdio_ipq4019 to mdio_ipq Luo Jie ` (2 more replies) 0 siblings, 3 replies; 14+ messages in thread From: Luo Jie @ 2021-07-29 12:53 UTC (permalink / raw) To: andrew, hkallweit1, davem, kuba, p.zabel, agross, bjorn.andersson, robh+dt, robert.marko Cc: netdev, linux-kernel, linux-arm-msm, devicetree, sricharan, Luo Jie Support PHY reset function and MDIO clock frequency configuration. Signed-off-by: Luo Jie <luoj@codeaurora.org> --- drivers/net/mdio/Kconfig | 1 + drivers/net/mdio/mdio-ipq4019.c | 71 +++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/drivers/net/mdio/Kconfig b/drivers/net/mdio/Kconfig index 99a6c13a11af..06a605ffb950 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 GPIOLIB && COMMON_CLK && RESET_CONTROLLER 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..01f5b9393537 100644 --- a/drivers/net/mdio/mdio-ipq4019.c +++ b/drivers/net/mdio/mdio-ipq4019.c @@ -11,6 +11,9 @@ #include <linux/of_mdio.h> #include <linux/phy.h> #include <linux/platform_device.h> +#include <linux/gpio/consumer.h> +#include <linux/reset.h> +#include <linux/clk.h> #define MDIO_MODE_REG 0x40 #define MDIO_ADDR_REG 0x44 @@ -31,8 +34,16 @@ #define IPQ4019_MDIO_TIMEOUT 10000 #define IPQ4019_MDIO_SLEEP 10 +/* MDIO clock source frequency is fixed to 100M */ +#define QCA_MDIO_CLK_RATE 100000000 + +#define QCA_PHY_SET_DELAY_US 100000 + struct ipq4019_mdio_data { void __iomem *membase; + void __iomem *eth_ldo_rdy; + struct reset_control *reset_ctrl; + struct clk *mdio_clk; }; static int ipq4019_mdio_wait_busy(struct mii_bus *bus) @@ -171,10 +182,61 @@ 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; + 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); + } + + /* 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); + } + + 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 +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"); + 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); -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/3] net: mdio-ipq4019: rename mdio_ipq4019 to mdio_ipq 2021-07-29 12:53 [PATCH 1/3] net: mdio-ipq4019: Add mdio reset function Luo Jie @ 2021-07-29 12:53 ` Luo Jie 2021-07-29 13:15 ` Andrew Lunn 2021-07-29 12:53 ` [PATCH 3/3] dt-bindings: net: rename Qualcomm IPQ MDIO bindings Luo Jie 2021-07-29 13:26 ` [PATCH 1/3] net: mdio-ipq4019: Add mdio reset function Andrew Lunn 2 siblings, 1 reply; 14+ messages in thread From: Luo Jie @ 2021-07-29 12:53 UTC (permalink / raw) To: andrew, hkallweit1, davem, kuba, p.zabel, agross, bjorn.andersson, robh+dt, robert.marko Cc: netdev, linux-kernel, linux-arm-msm, devicetree, sricharan, Luo Jie mdio_ipq driver supports more SOCs such as ipq40xx, ipq807x, ipq60xx and ipq50xx. Signed-off-by: Luo Jie <luoj@codeaurora.org> --- drivers/net/mdio/Kconfig | 6 +- drivers/net/mdio/Makefile | 2 +- .../net/mdio/{mdio-ipq4019.c => mdio-ipq.c} | 66 +++++++++---------- 3 files changed, 37 insertions(+), 37 deletions(-) rename drivers/net/mdio/{mdio-ipq4019.c => mdio-ipq.c} (81%) diff --git a/drivers/net/mdio/Kconfig b/drivers/net/mdio/Kconfig index 06a605ffb950..133c3d9cb083 100644 --- a/drivers/net/mdio/Kconfig +++ b/drivers/net/mdio/Kconfig @@ -166,13 +166,13 @@ config MDIO_OCTEON buses. It is required by the Octeon and ThunderX ethernet device drivers on some systems. -config MDIO_IPQ4019 - tristate "Qualcomm IPQ4019 MDIO interface support" +config MDIO_IPQ + tristate "Qualcomm IPQ MDIO interface support" depends on HAS_IOMEM && OF_MDIO depends on GPIOLIB && COMMON_CLK && RESET_CONTROLLER help This driver supports the MDIO interface found in Qualcomm - IPQ40xx series Soc-s. + IPQ40xx, IPQ60XX, IPQ807X and IPQ50XX series Soc-s. config MDIO_IPQ8064 tristate "Qualcomm IPQ8064 MDIO interface support" diff --git a/drivers/net/mdio/Makefile b/drivers/net/mdio/Makefile index 15f8dc4042ce..df7afc8462de 100644 --- a/drivers/net/mdio/Makefile +++ b/drivers/net/mdio/Makefile @@ -13,7 +13,7 @@ obj-$(CONFIG_MDIO_CAVIUM) += mdio-cavium.o obj-$(CONFIG_MDIO_GPIO) += mdio-gpio.o obj-$(CONFIG_MDIO_HISI_FEMAC) += mdio-hisi-femac.o obj-$(CONFIG_MDIO_I2C) += mdio-i2c.o -obj-$(CONFIG_MDIO_IPQ4019) += mdio-ipq4019.o +obj-$(CONFIG_MDIO_IPQ) += mdio-ipq.o obj-$(CONFIG_MDIO_IPQ8064) += mdio-ipq8064.o obj-$(CONFIG_MDIO_MOXART) += mdio-moxart.o obj-$(CONFIG_MDIO_MSCC_MIIM) += mdio-mscc-miim.o diff --git a/drivers/net/mdio/mdio-ipq4019.c b/drivers/net/mdio/mdio-ipq.c similarity index 81% rename from drivers/net/mdio/mdio-ipq4019.c rename to drivers/net/mdio/mdio-ipq.c index 01f5b9393537..70e1ae05a64f 100644 --- a/drivers/net/mdio/mdio-ipq4019.c +++ b/drivers/net/mdio/mdio-ipq.c @@ -31,38 +31,38 @@ /* 0 = Clause 22, 1 = Clause 45 */ #define MDIO_MODE_C45 BIT(8) -#define IPQ4019_MDIO_TIMEOUT 10000 -#define IPQ4019_MDIO_SLEEP 10 +#define IPQ_MDIO_TIMEOUT 10000 +#define IPQ_MDIO_SLEEP 10 /* MDIO clock source frequency is fixed to 100M */ #define QCA_MDIO_CLK_RATE 100000000 #define QCA_PHY_SET_DELAY_US 100000 -struct ipq4019_mdio_data { +struct ipq_mdio_data { void __iomem *membase; void __iomem *eth_ldo_rdy; struct reset_control *reset_ctrl; struct clk *mdio_clk; }; -static int ipq4019_mdio_wait_busy(struct mii_bus *bus) +static int ipq_mdio_wait_busy(struct mii_bus *bus) { - struct ipq4019_mdio_data *priv = bus->priv; + struct ipq_mdio_data *priv = bus->priv; unsigned int busy; return readl_poll_timeout(priv->membase + MDIO_CMD_REG, busy, (busy & MDIO_CMD_ACCESS_BUSY) == 0, - IPQ4019_MDIO_SLEEP, IPQ4019_MDIO_TIMEOUT); + IPQ_MDIO_SLEEP, IPQ_MDIO_TIMEOUT); } -static int ipq4019_mdio_read(struct mii_bus *bus, int mii_id, int regnum) +static int ipq_mdio_read(struct mii_bus *bus, int mii_id, int regnum) { - struct ipq4019_mdio_data *priv = bus->priv; + struct ipq_mdio_data *priv = bus->priv; unsigned int data; unsigned int cmd; - if (ipq4019_mdio_wait_busy(bus)) + if (ipq_mdio_wait_busy(bus)) return -ETIMEDOUT; /* Clause 45 support */ @@ -102,7 +102,7 @@ static int ipq4019_mdio_read(struct mii_bus *bus, int mii_id, int regnum) writel(cmd, priv->membase + MDIO_CMD_REG); /* Wait read complete */ - if (ipq4019_mdio_wait_busy(bus)) + if (ipq_mdio_wait_busy(bus)) return -ETIMEDOUT; if (regnum & MII_ADDR_C45) { @@ -110,7 +110,7 @@ static int ipq4019_mdio_read(struct mii_bus *bus, int mii_id, int regnum) writel(cmd, priv->membase + MDIO_CMD_REG); - if (ipq4019_mdio_wait_busy(bus)) + if (ipq_mdio_wait_busy(bus)) return -ETIMEDOUT; } @@ -118,14 +118,13 @@ static int ipq4019_mdio_read(struct mii_bus *bus, int mii_id, int regnum) return readl(priv->membase + MDIO_DATA_READ_REG); } -static int ipq4019_mdio_write(struct mii_bus *bus, int mii_id, int regnum, - u16 value) +static int ipq_mdio_write(struct mii_bus *bus, int mii_id, int regnum, u16 value) { - struct ipq4019_mdio_data *priv = bus->priv; + struct ipq_mdio_data *priv = bus->priv; unsigned int data; unsigned int cmd; - if (ipq4019_mdio_wait_busy(bus)) + if (ipq_mdio_wait_busy(bus)) return -ETIMEDOUT; /* Clause 45 support */ @@ -150,7 +149,7 @@ static int ipq4019_mdio_write(struct mii_bus *bus, int mii_id, int regnum, writel(cmd, priv->membase + MDIO_CMD_REG); - if (ipq4019_mdio_wait_busy(bus)) + if (ipq_mdio_wait_busy(bus)) return -ETIMEDOUT; } else { /* Enter Clause 22 mode */ @@ -176,7 +175,7 @@ static int ipq4019_mdio_write(struct mii_bus *bus, int mii_id, int regnum, writel(cmd, priv->membase + MDIO_CMD_REG); /* Wait write complete */ - if (ipq4019_mdio_wait_busy(bus)) + if (ipq_mdio_wait_busy(bus)) return -ETIMEDOUT; return 0; @@ -184,7 +183,7 @@ static int ipq4019_mdio_write(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; + struct ipq_mdio_data *priv = bus->priv; struct device *dev = bus->parent; struct gpio_desc *reset_gpio; u32 val; @@ -232,9 +231,9 @@ static int ipq_mdio_reset(struct mii_bus *bus) return 0; } -static int ipq4019_mdio_probe(struct platform_device *pdev) +static int ipq_mdio_probe(struct platform_device *pdev) { - struct ipq4019_mdio_data *priv; + struct ipq_mdio_data *priv; struct mii_bus *bus; struct resource *res; int ret; @@ -257,9 +256,9 @@ static int ipq4019_mdio_probe(struct platform_device *pdev) 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"); - bus->name = "ipq4019_mdio"; - bus->read = ipq4019_mdio_read; - bus->write = ipq4019_mdio_write; + bus->name = "ipq_mdio"; + bus->read = ipq_mdio_read; + bus->write = ipq_mdio_write; bus->reset = ipq_mdio_reset; bus->parent = &pdev->dev; snprintf(bus->id, MII_BUS_ID_SIZE, "%s%d", pdev->name, pdev->id); @@ -275,7 +274,7 @@ static int ipq4019_mdio_probe(struct platform_device *pdev) return 0; } -static int ipq4019_mdio_remove(struct platform_device *pdev) +static int ipq_mdio_remove(struct platform_device *pdev) { struct mii_bus *bus = platform_get_drvdata(pdev); @@ -284,23 +283,24 @@ static int ipq4019_mdio_remove(struct platform_device *pdev) return 0; } -static const struct of_device_id ipq4019_mdio_dt_ids[] = { +static const struct of_device_id ipq_mdio_dt_ids[] = { { .compatible = "qcom,ipq4019-mdio" }, + { .compatible = "qcom,ipq-mdio" }, { } }; -MODULE_DEVICE_TABLE(of, ipq4019_mdio_dt_ids); +MODULE_DEVICE_TABLE(of, ipq_mdio_dt_ids); -static struct platform_driver ipq4019_mdio_driver = { - .probe = ipq4019_mdio_probe, - .remove = ipq4019_mdio_remove, +static struct platform_driver ipq_mdio_driver = { + .probe = ipq_mdio_probe, + .remove = ipq_mdio_remove, .driver = { - .name = "ipq4019-mdio", - .of_match_table = ipq4019_mdio_dt_ids, + .name = "ipq-mdio", + .of_match_table = ipq_mdio_dt_ids, }, }; -module_platform_driver(ipq4019_mdio_driver); +module_platform_driver(ipq_mdio_driver); -MODULE_DESCRIPTION("ipq4019 MDIO interface driver"); +MODULE_DESCRIPTION("ipq MDIO interface driver"); MODULE_AUTHOR("Qualcomm Atheros"); MODULE_LICENSE("Dual BSD/GPL"); -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] net: mdio-ipq4019: rename mdio_ipq4019 to mdio_ipq 2021-07-29 12:53 ` [PATCH 2/3] net: mdio-ipq4019: rename mdio_ipq4019 to mdio_ipq Luo Jie @ 2021-07-29 13:15 ` Andrew Lunn 2021-08-02 5:56 ` luoj 0 siblings, 1 reply; 14+ messages in thread From: Andrew Lunn @ 2021-07-29 13:15 UTC (permalink / raw) To: Luo Jie Cc: hkallweit1, davem, kuba, p.zabel, agross, bjorn.andersson, robh+dt, robert.marko, netdev, linux-kernel, linux-arm-msm, devicetree, sricharan On Thu, Jul 29, 2021 at 08:53:57PM +0800, Luo Jie wrote: > mdio_ipq driver supports more SOCs such as ipq40xx, ipq807x, > ipq60xx and ipq50xx. > > Signed-off-by: Luo Jie <luoj@codeaurora.org> > --- > drivers/net/mdio/Kconfig | 6 +- > drivers/net/mdio/Makefile | 2 +- > .../net/mdio/{mdio-ipq4019.c => mdio-ipq.c} | 66 +++++++++---------- > 3 files changed, 37 insertions(+), 37 deletions(-) > rename drivers/net/mdio/{mdio-ipq4019.c => mdio-ipq.c} (81%) Hi Luo We don't rename files unless there is a very good reason. It makes back porting of fixes harder in stable. There are plenty of examples of files with device specific names, but supporting a broad range of devices. Take for example lm75, at24. > -config MDIO_IPQ4019 > - tristate "Qualcomm IPQ4019 MDIO interface support" > +config MDIO_IPQ > + tristate "Qualcomm IPQ MDIO interface support" > depends on HAS_IOMEM && OF_MDIO > depends on GPIOLIB && COMMON_CLK && RESET_CONTROLLER > help > This driver supports the MDIO interface found in Qualcomm > - IPQ40xx series Soc-s. > + IPQ40xx, IPQ60XX, IPQ807X and IPQ50XX series Soc-s. Please leave the MDIO_IPQ4019 unchanged, so we don't break backwards compatibility, but the changes to the text are O.K. > @@ -31,38 +31,38 @@ > /* 0 = Clause 22, 1 = Clause 45 */ > #define MDIO_MODE_C45 BIT(8) > > -#define IPQ4019_MDIO_TIMEOUT 10000 > -#define IPQ4019_MDIO_SLEEP 10 > +#define IPQ_MDIO_TIMEOUT 10000 > +#define IPQ_MDIO_SLEEP 10 This sort of mass rename will also make back porting fixes harder. Please don't do it. > -static const struct of_device_id ipq4019_mdio_dt_ids[] = { > +static const struct of_device_id ipq_mdio_dt_ids[] = { > { .compatible = "qcom,ipq4019-mdio" }, > + { .compatible = "qcom,ipq-mdio" }, > { } > }; Such a generic name is not a good idea. It appears this driver is not compatible with the IPQ8064? It is O.K. to add more specific compatibles. So you could add qcom,ipq40xx, qcom,ipq60xx, qcom,ipq807x and qcom,ipq50xx. But really, there is no need. Take for example snps,dwmac-mdio, which is used in all sorts of devices. Andrew ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] net: mdio-ipq4019: rename mdio_ipq4019 to mdio_ipq 2021-07-29 13:15 ` Andrew Lunn @ 2021-08-02 5:56 ` luoj 0 siblings, 0 replies; 14+ messages in thread From: luoj @ 2021-08-02 5:56 UTC (permalink / raw) On 2021-07-29 21:15, Andrew Lunn wrote: > On Thu, Jul 29, 2021 at 08:53:57PM +0800, Luo Jie wrote: >> mdio_ipq driver supports more SOCs such as ipq40xx, ipq807x, >> ipq60xx and ipq50xx. >> >> Signed-off-by: Luo Jie <luoj@codeaurora.org> >> --- >> drivers/net/mdio/Kconfig | 6 +- >> drivers/net/mdio/Makefile | 2 +- >> .../net/mdio/{mdio-ipq4019.c => mdio-ipq.c} | 66 >> +++++++++---------- >> 3 files changed, 37 insertions(+), 37 deletions(-) >> rename drivers/net/mdio/{mdio-ipq4019.c => mdio-ipq.c} (81%) > > Hi Luo > > We don't rename files unless there is a very good reason. It makes > back porting of fixes harder in stable. There are plenty of examples > of files with device specific names, but supporting a broad range of > devices. Take for example lm75, at24. > > Hi Andrew > Thanks for the comments, will update the patch set to keep the name > unchanged. > >> -config MDIO_IPQ4019 >> - tristate "Qualcomm IPQ4019 MDIO interface support" >> +config MDIO_IPQ >> + tristate "Qualcomm IPQ MDIO interface support" >> depends on HAS_IOMEM && OF_MDIO >> depends on GPIOLIB && COMMON_CLK && RESET_CONTROLLER >> help >> This driver supports the MDIO interface found in Qualcomm >> - IPQ40xx series Soc-s. >> + IPQ40xx, IPQ60XX, IPQ807X and IPQ50XX series Soc-s. > > Please leave the MDIO_IPQ4019 unchanged, so we don't break backwards > compatibility, but the changes to the text are O.K. > > will correct it in the next patch set. > >> @@ -31,38 +31,38 @@ >> /* 0 = Clause 22, 1 = Clause 45 */ >> #define MDIO_MODE_C45 BIT(8) >> >> -#define IPQ4019_MDIO_TIMEOUT 10000 >> -#define IPQ4019_MDIO_SLEEP 10 >> +#define IPQ_MDIO_TIMEOUT 10000 >> +#define IPQ_MDIO_SLEEP 10 > > This sort of mass rename will also make back porting fixes > harder. Please don't do it. > > will keep it unchanged in the next patch set. > >> -static const struct of_device_id ipq4019_mdio_dt_ids[] = { >> +static const struct of_device_id ipq_mdio_dt_ids[] = { >> { .compatible = "qcom,ipq4019-mdio" }, >> + { .compatible = "qcom,ipq-mdio" }, >> { } >> }; > > Such a generic name is not a good idea. It appears this driver is not > compatible with the IPQ8064? It is O.K. to add more specific > compatibles. So you could add > > qcom,ipq40xx, qcom,ipq60xx, qcom,ipq807x and qcom,ipq50xx. > > But really, there is no need. Take for example snps,dwmac-mdio, which > is used in all sorts of devices. > > Andrew > Hi Andrew, yes, this driver is not compatible with IPQ8064, but it is > compatible with > the new chipset such as ipq807x, ipq60xx and ipq50xx, will take your > suggestion in > the next patch set, thanks for the comments. > ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] dt-bindings: net: rename Qualcomm IPQ MDIO bindings 2021-07-29 12:53 [PATCH 1/3] net: mdio-ipq4019: Add mdio reset function Luo Jie 2021-07-29 12:53 ` [PATCH 2/3] net: mdio-ipq4019: rename mdio_ipq4019 to mdio_ipq Luo Jie @ 2021-07-29 12:53 ` Luo Jie 2021-07-29 13:17 ` Andrew Lunn ` (2 more replies) 2021-07-29 13:26 ` [PATCH 1/3] net: mdio-ipq4019: Add mdio reset function Andrew Lunn 2 siblings, 3 replies; 14+ messages in thread From: Luo Jie @ 2021-07-29 12:53 UTC (permalink / raw) To: andrew, hkallweit1, davem, kuba, p.zabel, agross, bjorn.andersson, robh+dt, robert.marko Cc: netdev, linux-kernel, linux-arm-msm, devicetree, sricharan, Luo Jie rename ipq4019-mdio.yaml to ipq-mdio.yaml for supporting more ipq boards such as ipq40xx, ipq807x, ipq60xx and ipq50xx. Signed-off-by: Luo Jie <luoj@codeaurora.org> --- ...m,ipq4019-mdio.yaml => qcom,ipq-mdio.yaml} | 32 ++++++++++++++++--- 1 file changed, 28 insertions(+), 4 deletions(-) rename Documentation/devicetree/bindings/net/{qcom,ipq4019-mdio.yaml => qcom,ipq-mdio.yaml} (58%) diff --git a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml b/Documentation/devicetree/bindings/net/qcom,ipq-mdio.yaml similarity index 58% rename from Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml rename to Documentation/devicetree/bindings/net/qcom,ipq-mdio.yaml index 0c973310ada0..5bdeb461523b 100644 --- a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml +++ b/Documentation/devicetree/bindings/net/qcom,ipq-mdio.yaml @@ -1,10 +1,10 @@ # SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause %YAML 1.2 --- -$id: http://devicetree.org/schemas/net/qcom,ipq4019-mdio.yaml# +$id: http://devicetree.org/schemas/net/qcom,ipq-mdio.yaml# $schema: http://devicetree.org/meta-schemas/core.yaml# -title: Qualcomm IPQ40xx MDIO Controller Device Tree Bindings +title: Qualcomm IPQ MDIO Controller Device Tree Bindings maintainers: - Robert Marko <robert.marko@sartura.hr> @@ -14,7 +14,9 @@ allOf: properties: compatible: - const: qcom,ipq4019-mdio + oneOf: + - const: qcom,ipq4019-mdio + - const: qcom,ipq-mdio "#address-cells": const: 1 @@ -23,7 +25,29 @@ properties: const: 0 reg: - maxItems: 1 + maxItems: 2 + + clocks: + items: + - description: MDIO clock + + clock-names: + items: + - const: gcc_mdio_ahb_clk + + resets: + items: + - description: MDIO reset & GEPHY hardware reset + + reset-names: + items: + - const: gephy_mdc_rst + + phy-reset-gpios: + maxItems: 3 + description: + The phandle and specifier for the GPIO that controls the RESET + lines of PHY devices on that MDIO bus. required: - compatible -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] dt-bindings: net: rename Qualcomm IPQ MDIO bindings 2021-07-29 12:53 ` [PATCH 3/3] dt-bindings: net: rename Qualcomm IPQ MDIO bindings Luo Jie @ 2021-07-29 13:17 ` Andrew Lunn 2021-08-02 6:02 ` luoj 2021-07-29 13:57 ` Rob Herring 2021-07-29 17:29 ` Rob Herring 2 siblings, 1 reply; 14+ messages in thread From: Andrew Lunn @ 2021-07-29 13:17 UTC (permalink / raw) To: Luo Jie Cc: hkallweit1, davem, kuba, p.zabel, agross, bjorn.andersson, robh+dt, robert.marko, netdev, linux-kernel, linux-arm-msm, devicetree, sricharan > @@ -23,7 +25,29 @@ properties: > const: 0 > > reg: > - maxItems: 1 > + maxItems: 2 > + > + clocks: > + items: > + - description: MDIO clock > + > + clock-names: > + items: > + - const: gcc_mdio_ahb_clk > + > + resets: > + items: > + - description: MDIO reset & GEPHY hardware reset > + > + reset-names: > + items: > + - const: gephy_mdc_rst > + > + phy-reset-gpios: > + maxItems: 3 > + description: > + The phandle and specifier for the GPIO that controls the RESET > + lines of PHY devices on that MDIO bus. This is clearly not a rename. It is great you are adding missing properties, but please do it as a separate patch. Andrew ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] dt-bindings: net: rename Qualcomm IPQ MDIO bindings 2021-07-29 13:17 ` Andrew Lunn @ 2021-08-02 6:02 ` luoj 0 siblings, 0 replies; 14+ messages in thread From: luoj @ 2021-08-02 6:02 UTC (permalink / raw) To: Andrew Lunn Cc: hkallweit1, davem, kuba, p.zabel, agross, bjorn.andersson, robh+dt, robert.marko, netdev, linux-kernel, linux-arm-msm, devicetree, sricharan On 2021-07-29 21:17, Andrew Lunn wrote: >> @@ -23,7 +25,29 @@ properties: >> const: 0 >> >> reg: >> - maxItems: 1 >> + maxItems: 2 >> + >> + clocks: >> + items: >> + - description: MDIO clock >> + >> + clock-names: >> + items: >> + - const: gcc_mdio_ahb_clk >> + >> + resets: >> + items: >> + - description: MDIO reset & GEPHY hardware reset >> + >> + reset-names: >> + items: >> + - const: gephy_mdc_rst >> + >> + phy-reset-gpios: >> + maxItems: 3 >> + description: >> + The phandle and specifier for the GPIO that controls the RESET >> + lines of PHY devices on that MDIO bus. > > This is clearly not a rename. It is great you are adding missing > properties, but please do it as a separate patch. > > Andrew > Hi Andrew, thanks for the comments, will separate it out in the next > patch set. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] dt-bindings: net: rename Qualcomm IPQ MDIO bindings 2021-07-29 12:53 ` [PATCH 3/3] dt-bindings: net: rename Qualcomm IPQ MDIO bindings Luo Jie 2021-07-29 13:17 ` Andrew Lunn @ 2021-07-29 13:57 ` Rob Herring 2021-07-29 17:29 ` Rob Herring 2 siblings, 0 replies; 14+ messages in thread From: Rob Herring @ 2021-07-29 13:57 UTC (permalink / raw) To: Luo Jie Cc: linux-arm-msm, kuba, p.zabel, hkallweit1, agross, andrew, netdev, davem, robert.marko, linux-kernel, sricharan, bjorn.andersson, devicetree, robh+dt On Thu, 29 Jul 2021 20:53:58 +0800, Luo Jie wrote: > rename ipq4019-mdio.yaml to ipq-mdio.yaml for supporting more > ipq boards such as ipq40xx, ipq807x, ipq60xx and ipq50xx. > > Signed-off-by: Luo Jie <luoj@codeaurora.org> > --- > ...m,ipq4019-mdio.yaml => qcom,ipq-mdio.yaml} | 32 ++++++++++++++++--- > 1 file changed, 28 insertions(+), 4 deletions(-) > rename Documentation/devicetree/bindings/net/{qcom,ipq4019-mdio.yaml => qcom,ipq-mdio.yaml} (58%) > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/qcom,ipq-mdio.example.dt.yaml: mdio@90000: reg: [[589824, 100]] is too short From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/qcom,ipq-mdio.yaml \ndoc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/1511253 This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] dt-bindings: net: rename Qualcomm IPQ MDIO bindings 2021-07-29 12:53 ` [PATCH 3/3] dt-bindings: net: rename Qualcomm IPQ MDIO bindings Luo Jie 2021-07-29 13:17 ` Andrew Lunn 2021-07-29 13:57 ` Rob Herring @ 2021-07-29 17:29 ` Rob Herring 2021-08-02 7:19 ` luoj 2 siblings, 1 reply; 14+ messages in thread From: Rob Herring @ 2021-07-29 17:29 UTC (permalink / raw) To: Luo Jie Cc: Andrew Lunn, Heiner Kallweit, David Miller, Jakub Kicinski, Philipp Zabel, Gross, Andy, Bjorn Andersson, Robert Marko, netdev, linux-kernel, linux-arm-msm, devicetree, Sricharan On Thu, Jul 29, 2021 at 6:54 AM Luo Jie <luoj@codeaurora.org> wrote: > > rename ipq4019-mdio.yaml to ipq-mdio.yaml for supporting more > ipq boards such as ipq40xx, ipq807x, ipq60xx and ipq50xx. > > Signed-off-by: Luo Jie <luoj@codeaurora.org> > --- > ...m,ipq4019-mdio.yaml => qcom,ipq-mdio.yaml} | 32 ++++++++++++++++--- > 1 file changed, 28 insertions(+), 4 deletions(-) > rename Documentation/devicetree/bindings/net/{qcom,ipq4019-mdio.yaml => qcom,ipq-mdio.yaml} (58%) > > diff --git a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml b/Documentation/devicetree/bindings/net/qcom,ipq-mdio.yaml > similarity index 58% > rename from Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml > rename to Documentation/devicetree/bindings/net/qcom,ipq-mdio.yaml > index 0c973310ada0..5bdeb461523b 100644 > --- a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml > +++ b/Documentation/devicetree/bindings/net/qcom,ipq-mdio.yaml > @@ -1,10 +1,10 @@ > # SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > %YAML 1.2 > --- > -$id: http://devicetree.org/schemas/net/qcom,ipq4019-mdio.yaml# > +$id: http://devicetree.org/schemas/net/qcom,ipq-mdio.yaml# > $schema: http://devicetree.org/meta-schemas/core.yaml# > > -title: Qualcomm IPQ40xx MDIO Controller Device Tree Bindings > +title: Qualcomm IPQ MDIO Controller Device Tree Bindings > > maintainers: > - Robert Marko <robert.marko@sartura.hr> > @@ -14,7 +14,9 @@ allOf: > > properties: > compatible: > - const: qcom,ipq4019-mdio > + oneOf: > + - const: qcom,ipq4019-mdio > + - const: qcom,ipq-mdio This is more than the commit log suggests. A generic compatible by itself is not sufficient. If other chips have the same block, just use 'qcom,ipq4019-mdio'. They should also have a compatible for the new SoC in case it's not 'the same'. Also, use 'enum' rather than oneOf plus const. > > "#address-cells": > const: 1 > @@ -23,7 +25,29 @@ properties: > const: 0 > > reg: > - maxItems: 1 > + maxItems: 2 This breaks compatibility because now 1 entry is not valid. > + > + clocks: > + items: > + - description: MDIO clock > + > + clock-names: > + items: > + - const: gcc_mdio_ahb_clk > + > + resets: > + items: > + - description: MDIO reset & GEPHY hardware reset > + > + reset-names: > + items: > + - const: gephy_mdc_rst These all now apply to 'qcom,ipq4019-mdio'. The h/w had no clocks or resets and now does? You don't need *-names when there is only 1. > + phy-reset-gpios: > + maxItems: 3 > + description: > + The phandle and specifier for the GPIO that controls the RESET > + lines of PHY devices on that MDIO bus. This belongs in the phy node since the reset is connected to the phy. > > required: > - compatible > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] dt-bindings: net: rename Qualcomm IPQ MDIO bindings 2021-07-29 17:29 ` Rob Herring @ 2021-08-02 7:19 ` luoj 2021-08-02 13:39 ` Andrew Lunn 0 siblings, 1 reply; 14+ messages in thread From: luoj @ 2021-08-02 7:19 UTC (permalink / raw) To: Rob Herring Cc: Andrew Lunn, Heiner Kallweit, David Miller, Jakub Kicinski, Philipp Zabel, Gross, Andy, Bjorn Andersson, Robert Marko, netdev, linux-kernel, linux-arm-msm, devicetree, Sricharan On 2021-07-30 01:29, Rob Herring wrote: > On Thu, Jul 29, 2021 at 6:54 AM Luo Jie <luoj@codeaurora.org> wrote: >> >> rename ipq4019-mdio.yaml to ipq-mdio.yaml for supporting more >> ipq boards such as ipq40xx, ipq807x, ipq60xx and ipq50xx. >> >> Signed-off-by: Luo Jie <luoj@codeaurora.org> >> --- >> ...m,ipq4019-mdio.yaml => qcom,ipq-mdio.yaml} | 32 >> ++++++++++++++++--- >> 1 file changed, 28 insertions(+), 4 deletions(-) >> rename Documentation/devicetree/bindings/net/{qcom,ipq4019-mdio.yaml >> => qcom,ipq-mdio.yaml} (58%) >> >> diff --git >> a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml >> b/Documentation/devicetree/bindings/net/qcom,ipq-mdio.yaml >> similarity index 58% >> rename from >> Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml >> rename to Documentation/devicetree/bindings/net/qcom,ipq-mdio.yaml >> index 0c973310ada0..5bdeb461523b 100644 >> --- a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml >> +++ b/Documentation/devicetree/bindings/net/qcom,ipq-mdio.yaml >> @@ -1,10 +1,10 @@ >> # SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause >> %YAML 1.2 >> --- >> -$id: http://devicetree.org/schemas/net/qcom,ipq4019-mdio.yaml# >> +$id: http://devicetree.org/schemas/net/qcom,ipq-mdio.yaml# >> $schema: http://devicetree.org/meta-schemas/core.yaml# >> >> -title: Qualcomm IPQ40xx MDIO Controller Device Tree Bindings >> +title: Qualcomm IPQ MDIO Controller Device Tree Bindings >> >> maintainers: >> - Robert Marko <robert.marko@sartura.hr> >> @@ -14,7 +14,9 @@ allOf: >> >> properties: >> compatible: >> - const: qcom,ipq4019-mdio >> + oneOf: >> + - const: qcom,ipq4019-mdio >> + - const: qcom,ipq-mdio > > This is more than the commit log suggests. A generic compatible by > itself is not sufficient. If other chips have the same block, just use > 'qcom,ipq4019-mdio'. They should also have a compatible for the new > SoC in case it's not 'the same'. > > Also, use 'enum' rather than oneOf plus const. > > Hi Rob > Thanks for the comments, will keep the compatible "qcom,ipq4019-mdio" > unchanged, > and add the new compatible name by using 'enum' in the next patch set. > the commit log will be updated in the next patch set. >> >> "#address-cells": >> const: 1 >> @@ -23,7 +25,29 @@ properties: >> const: 0 >> >> reg: >> - maxItems: 1 >> + maxItems: 2 > > This breaks compatibility because now 1 entry is not valid. > > will update it by using "minItems: 1, maxItems: 2" in the next patch > set. > >> + >> + clocks: >> + items: >> + - description: MDIO clock >> + >> + clock-names: >> + items: >> + - const: gcc_mdio_ahb_clk >> + >> + resets: >> + items: >> + - description: MDIO reset & GEPHY hardware reset >> + >> + reset-names: >> + items: >> + - const: gephy_mdc_rst > > These all now apply to 'qcom,ipq4019-mdio'. The h/w had no clocks or > resets and now does? > > You don't need *-names when there is only 1. > > Hi Rob > thanks for the comment, clocks is for configuring the source clock of > MDIO bus, > which is apply to ipq4019 and the new chip set such as ipq807x, ipq50xx > and ipq60xx, > which is not configured because of uboot configuring it, kernel should > not depends on > the configuration of uboot, so i add it. > will remove the *-name in the next patch set. > >> + phy-reset-gpios: >> + maxItems: 3 >> + description: >> + The phandle and specifier for the GPIO that controls the RESET >> + lines of PHY devices on that MDIO bus. > > This belongs in the phy node since the reset is connected to the phy. > > since the phylib code can't satisfy resetting PHY in IPQ chipset, > phylib resets phy by > configuring GPIO output value to 1, then to 0. however the PHY reset in > IPQ chipset need > to configuring GPIO output value to 0, then to 1 for the PHY reset, so > i put the phy-reset-gpios here. > >> >> required: >> - compatible >> -- >> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora >> Forum, >> a Linux Foundation Collaborative Project >> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] dt-bindings: net: rename Qualcomm IPQ MDIO bindings 2021-08-02 7:19 ` luoj @ 2021-08-02 13:39 ` Andrew Lunn 2021-08-04 2:37 ` Jie Luo 0 siblings, 1 reply; 14+ messages in thread From: Andrew Lunn @ 2021-08-02 13:39 UTC (permalink / raw) To: luoj Cc: Rob Herring, Heiner Kallweit, David Miller, Jakub Kicinski, Philipp Zabel, Gross, Andy, Bjorn Andersson, Robert Marko, netdev, linux-kernel, linux-arm-msm, devicetree, Sricharan > > since the phylib code can't satisfy resetting PHY in IPQ chipset, phylib > > resets phy by > > configuring GPIO output value to 1, then to 0. however the PHY reset in > > IPQ chipset need > > to configuring GPIO output value to 0, then to 1 for the PHY reset, so i > > put the phy-reset-gpios here. Look at the active low DT property of a GPIO. Andrew ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] dt-bindings: net: rename Qualcomm IPQ MDIO bindings 2021-08-02 13:39 ` Andrew Lunn @ 2021-08-04 2:37 ` Jie Luo 0 siblings, 0 replies; 14+ messages in thread From: Jie Luo @ 2021-08-04 2:37 UTC (permalink / raw) To: Andrew Lunn Cc: Rob Herring, Heiner Kallweit, David Miller, Jakub Kicinski, Philipp Zabel, Gross, Andy, Bjorn Andersson, Robert Marko, netdev, linux-kernel, linux-arm-msm, devicetree, Sricharan On 8/2/2021 9:39 PM, Andrew Lunn wrote: >>> since the phylib code can't satisfy resetting PHY in IPQ chipset, phylib >>> resets phy by >>> configuring GPIO output value to 1, then to 0. however the PHY reset in >>> IPQ chipset need >>> to configuring GPIO output value to 0, then to 1 for the PHY reset, so i >>> put the phy-reset-gpios here. > Look at the active low DT property of a GPIO. > > Andrew > thanks Andrew, will update it in the next patch set. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] net: mdio-ipq4019: Add mdio reset function 2021-07-29 12:53 [PATCH 1/3] net: mdio-ipq4019: Add mdio reset function Luo Jie 2021-07-29 12:53 ` [PATCH 2/3] net: mdio-ipq4019: rename mdio_ipq4019 to mdio_ipq Luo Jie 2021-07-29 12:53 ` [PATCH 3/3] dt-bindings: net: rename Qualcomm IPQ MDIO bindings Luo Jie @ 2021-07-29 13:26 ` Andrew Lunn 2021-08-02 6:46 ` luoj 2 siblings, 1 reply; 14+ messages in thread From: Andrew Lunn @ 2021-07-29 13:26 UTC (permalink / raw) To: Luo Jie Cc: hkallweit1, davem, kuba, p.zabel, agross, bjorn.andersson, robh+dt, robert.marko, netdev, linux-kernel, linux-arm-msm, devicetree, sricharan 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] net: mdio-ipq4019: Add mdio reset function 2021-07-29 13:26 ` [PATCH 1/3] net: mdio-ipq4019: Add mdio reset function Andrew Lunn @ 2021-08-02 6:46 ` luoj 0 siblings, 0 replies; 14+ messages in thread From: luoj @ 2021-08-02 6:46 UTC (permalink / raw) To: Andrew Lunn Cc: hkallweit1, davem, kuba, p.zabel, agross, bjorn.andersson, robh+dt, robert.marko, netdev, linux-kernel, linux-arm-msm, devicetree, sricharan On 2021-07-29 21:26, Andrew Lunn wrote: > 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. > > Hi Andrew, > Thanks for reminder, will provide it in the next patch set. > >> +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. > > The GEPHY is the embedded Giga Ethernet PHY in the chipset IPQ50xx, > there is a dedicated MDIO bus for this internal PHY. > what the reset function does here is for resetting this dedicated MDIO > bus and this embedded PHY DSP hardware. > because this dedicated MDIO bus is only connected with this internal > PHY on chip set IPQ50xx, so i put this > code in the MDIO reset function. > >> + >> + /* 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. > > Hi Andrew, > The common code in phylib for resetting PHY by GPIO pin is high active, > which is not suitable for the PHY reset here. > for resetting the PHY, calling gpiod_set_value_cansleep(reset_gpio, 1), > then gpiod_set_value_cansleep(reset_gpio, 0). > but as for resetting the PHY by GPIO pin in IPQ chipset, this is the > opposite process(low active) from the phylib code, > which needs to set the GPIO output value to 0, then to 1 for reset as > the code above. > >> 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 > > thanks for the comment, will update it in the next patch set. ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-08-04 2:37 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-07-29 12:53 [PATCH 1/3] net: mdio-ipq4019: Add mdio reset function Luo Jie 2021-07-29 12:53 ` [PATCH 2/3] net: mdio-ipq4019: rename mdio_ipq4019 to mdio_ipq Luo Jie 2021-07-29 13:15 ` Andrew Lunn 2021-08-02 5:56 ` luoj 2021-07-29 12:53 ` [PATCH 3/3] dt-bindings: net: rename Qualcomm IPQ MDIO bindings Luo Jie 2021-07-29 13:17 ` Andrew Lunn 2021-08-02 6:02 ` luoj 2021-07-29 13:57 ` Rob Herring 2021-07-29 17:29 ` Rob Herring 2021-08-02 7:19 ` luoj 2021-08-02 13:39 ` Andrew Lunn 2021-08-04 2:37 ` Jie Luo 2021-07-29 13:26 ` [PATCH 1/3] net: mdio-ipq4019: Add mdio reset function Andrew Lunn 2021-08-02 6:46 ` luoj
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).