For IPQ5332 platform, there are two MAC PCSs, and qca8084 is
connected with one of them.
1. The Ethernet LDO needs to be enabled to make the PHY GPIO
reset taking effect, which uses the MDIO bus level reset.
2. The SoC GCC uniphy AHB and SYS clocks need to be enabled
to make the ethernet PHY device accessible.
3. To provide the clock to the ethernet, the CMN clock needs
to be initialized for selecting reference clock and enabling
the output clock.
4. Support optional MDIO clock frequency config.
5. Update dt-bindings doc for the new added properties.
Changes in v2:
* remove the PHY related features such as PHY address
program and clock initialization.
* leverage the MDIO level GPIO reset for qca8084 PHY.
Changes in v3:
* fix the christmas-tree format issue.
* improve the dt-binding changes.
Luo Jie (5):
net: mdio: ipq4019: move eth_ldo_rdy before MDIO bus register
net: mdio: ipq4019: enable the SoC uniphy clocks for ipq5332 platform
net: mdio: ipq4019: configure CMN PLL clock for ipq5332
net: mdio: ipq4019: support MDIO clock frequency divider
dt-bindings: net: ipq4019-mdio: Document ipq5332 platform
.../bindings/net/qcom,ipq4019-mdio.yaml | 143 ++++++++-
drivers/net/mdio/mdio-ipq4019.c | 296 ++++++++++++++++--
2 files changed, 410 insertions(+), 29 deletions(-)
base-commit: 11651f8cb2e88372d4ed523d909514dc9a613ea3
--
2.42.0
The ethernet LDO provides the clock for the ethernet PHY that
is connected with PCS, each LDO enables the clock output to
each PCS, after the clock output enablement, the PHY GPIO reset
can take effect.
For the PHY taking the MDIO bus level GPIO reset, the ethernet
LDO should be enabled before the MDIO bus register.
For example, the qca8084 PHY takes the MDIO bus level GPIO
reset for quad PHYs, there is another reason for qca8084 PHY
using MDIO bus level GPIO reset instead of PHY level GPIO
reset as below.
The work sequence of qca8084:
1. enable ethernet LDO.
2. GPIO reset on quad PHYs.
3. register clock provider based on MDIO device of qca8084.
4. PHY probe function called for initializing common clocks.
5. PHY capabilities acquirement.
If qca8084 takes PHY level GPIO reset in the step 4, the clock
provider of qca8084 can't be registered correctly, since the
clock parent(reading the current qca8084 hardware registers in
step 3) of the registered clocks is deserted after GPIO reset.
There are two PCS(UNIPHY) supported in SOC side on ipq5332,
and three PCS(UNIPHY) supported on ipq9574.
Signed-off-by: Luo Jie <[email protected]>
---
drivers/net/mdio/mdio-ipq4019.c | 51 +++++++++++++++++++++------------
1 file changed, 32 insertions(+), 19 deletions(-)
diff --git a/drivers/net/mdio/mdio-ipq4019.c b/drivers/net/mdio/mdio-ipq4019.c
index abd8b508ec16..5273864fabb3 100644
--- a/drivers/net/mdio/mdio-ipq4019.c
+++ b/drivers/net/mdio/mdio-ipq4019.c
@@ -37,9 +37,12 @@
#define IPQ_PHY_SET_DELAY_US 100000
+/* Maximum SOC PCS(uniphy) number on IPQ platform */
+#define ETH_LDO_RDY_CNT 3
+
struct ipq4019_mdio_data {
- void __iomem *membase;
- void __iomem *eth_ldo_rdy;
+ void __iomem *membase;
+ void __iomem *eth_ldo_rdy[ETH_LDO_RDY_CNT];
struct clk *mdio_clk;
};
@@ -206,19 +209,8 @@ 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;
- 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 (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 */
ret = clk_set_rate(priv->mdio_clk, IPQ_MDIO_CLK_RATE);
if (ret)
@@ -236,7 +228,7 @@ static int ipq4019_mdio_probe(struct platform_device *pdev)
struct ipq4019_mdio_data *priv;
struct mii_bus *bus;
struct resource *res;
- int ret;
+ int ret, index;
bus = devm_mdiobus_alloc_size(&pdev->dev, sizeof(*priv));
if (!bus)
@@ -252,11 +244,32 @@ static int ipq4019_mdio_probe(struct platform_device *pdev)
if (IS_ERR(priv->mdio_clk))
return PTR_ERR(priv->mdio_clk);
- /* The platform resource is provided on the chipset IPQ5018 */
- /* This resource is optional */
- res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
- if (res)
- priv->eth_ldo_rdy = devm_ioremap_resource(&pdev->dev, res);
+ /* These platform resources are provided on the chipset IPQ5018 or
+ * IPQ5332.
+ */
+ /* This resource are optional */
+ for (index = 0; index < ETH_LDO_RDY_CNT; index++) {
+ res = platform_get_resource(pdev, IORESOURCE_MEM, index + 1);
+ if (res) {
+ priv->eth_ldo_rdy[index] = devm_ioremap(&pdev->dev,
+ res->start,
+ resource_size(res));
+
+ /* The ethernet LDO enable is necessary to reset PHY
+ * by GPIO, some PHY(such as qca8084) GPIO reset uses
+ * the MDIO level reset, so this function should be
+ * called before the MDIO bus register.
+ */
+ if (priv->eth_ldo_rdy[index]) {
+ u32 val;
+
+ val = readl(priv->eth_ldo_rdy[index]);
+ val |= BIT(0);
+ writel(val, priv->eth_ldo_rdy[index]);
+ fsleep(IPQ_PHY_SET_DELAY_US);
+ }
+ }
+ }
bus->name = "ipq4019_mdio";
bus->read = ipq4019_mdio_read_c22;
--
2.42.0
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 <[email protected]>
---
drivers/net/mdio/mdio-ipq4019.c | 75 ++++++++++++++++++++++++++++-----
1 file changed, 64 insertions(+), 11 deletions(-)
diff --git a/drivers/net/mdio/mdio-ipq4019.c b/drivers/net/mdio/mdio-ipq4019.c
index 5273864fabb3..e24b0e688b10 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",
+ "uniphy0_ahb",
+ "uniphy0_sys",
+ "uniphy1_ahb",
+ "uniphy1_sys"
};
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;
+ unsigned long rate;
+ int ret, index;
- /* 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;
+ /* 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;
+ }
+
+ 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;
--
2.42.0
The reference clock of CMN PLL block is selectable, the internal
48MHZ is used by default.
The output clock of CMN PLL block is for providing the clock
source of ethernet device(such as qca8084), there are 1 * 25MHZ
and 3 * 50MHZ output clocks available for the ethernet devices.
Signed-off-by: Luo Jie <[email protected]>
---
drivers/net/mdio/mdio-ipq4019.c | 137 +++++++++++++++++++++++++++++++-
1 file changed, 136 insertions(+), 1 deletion(-)
diff --git a/drivers/net/mdio/mdio-ipq4019.c b/drivers/net/mdio/mdio-ipq4019.c
index e24b0e688b10..3568ce7f48c6 100644
--- a/drivers/net/mdio/mdio-ipq4019.c
+++ b/drivers/net/mdio/mdio-ipq4019.c
@@ -44,6 +44,25 @@
/* Maximum SOC PCS(uniphy) number on IPQ platform */
#define ETH_LDO_RDY_CNT 3
+#define CMN_PLL_REFERENCE_SOURCE_SEL 0x28
+#define CMN_PLL_REFCLK_SOURCE_DIV GENMASK(9, 8)
+
+#define CMN_PLL_REFERENCE_CLOCK 0x784
+#define CMN_PLL_REFCLK_EXTERNAL BIT(9)
+#define CMN_PLL_REFCLK_DIV GENMASK(8, 4)
+#define CMN_PLL_REFCLK_INDEX GENMASK(3, 0)
+
+#define CMN_PLL_POWER_ON_AND_RESET 0x780
+#define CMN_ANA_EN_SW_RSTN BIT(6)
+
+#define CMN_REFCLK_INTERNAL_48MHZ 0
+#define CMN_REFCLK_EXTERNAL_25MHZ 1
+#define CMN_REFCLK_EXTERNAL_31250KHZ 2
+#define CMN_REFCLK_EXTERNAL_40MHZ 3
+#define CMN_REFCLK_EXTERNAL_48MHZ 4
+#define CMN_REFCLK_EXTERNAL_50MHZ 5
+#define CMN_REFCLK_INTERNAL_96MHZ 6
+
enum mdio_clk_id {
MDIO_CLK_MDIO_AHB,
MDIO_CLK_UNIPHY0_AHB,
@@ -55,6 +74,7 @@ enum mdio_clk_id {
struct ipq4019_mdio_data {
void __iomem *membase;
+ void __iomem *cmn_membase;
void __iomem *eth_ldo_rdy[ETH_LDO_RDY_CNT];
struct clk *clk[MDIO_CLK_CNT];
};
@@ -227,12 +247,116 @@ static int ipq4019_mdio_write_c22(struct mii_bus *bus, int mii_id, int regnum,
return 0;
}
+/* For the CMN PLL block, the reference clock can be configured according to
+ * the device tree property "cmn-reference-clock", the internal 48MHZ is used
+ * by default on the ipq533 platform.
+ *
+ * The output clock of CMN PLL block is provided to the ethernet devices,
+ * threre are 4 CMN PLL output clocks (1*25MHZ + 3*50MHZ) enabled by default.
+ *
+ * Such as the output 50M clock for the qca8084 ethernet PHY.
+ */
+static int ipq_cmn_clock_config(struct mii_bus *bus)
+{
+ struct ipq4019_mdio_data *priv;
+ u32 reg_val, src_sel, ref_clk;
+ int ret;
+
+ priv = bus->priv;
+ if (priv->cmn_membase) {
+ reg_val = readl(priv->cmn_membase + CMN_PLL_REFERENCE_CLOCK);
+
+ /* Select reference clock source */
+ ret = of_property_read_u32(bus->parent->of_node,
+ "cmn-reference-clock",
+ &ref_clk);
+ if (!ret) {
+ switch (ref_clk) {
+ case CMN_REFCLK_INTERNAL_48MHZ:
+ reg_val &= ~(CMN_PLL_REFCLK_EXTERNAL |
+ CMN_PLL_REFCLK_INDEX);
+ reg_val |= FIELD_PREP(CMN_PLL_REFCLK_INDEX, 7);
+ break;
+ case CMN_REFCLK_EXTERNAL_25MHZ:
+ reg_val &= ~(CMN_PLL_REFCLK_EXTERNAL |
+ CMN_PLL_REFCLK_INDEX);
+ reg_val |= (CMN_PLL_REFCLK_EXTERNAL |
+ FIELD_PREP(CMN_PLL_REFCLK_INDEX, 3));
+ break;
+ case CMN_REFCLK_EXTERNAL_31250KHZ:
+ reg_val &= ~(CMN_PLL_REFCLK_EXTERNAL |
+ CMN_PLL_REFCLK_INDEX);
+ reg_val |= (CMN_PLL_REFCLK_EXTERNAL |
+ FIELD_PREP(CMN_PLL_REFCLK_INDEX, 4));
+ break;
+ case CMN_REFCLK_EXTERNAL_40MHZ:
+ reg_val &= ~(CMN_PLL_REFCLK_EXTERNAL |
+ CMN_PLL_REFCLK_INDEX);
+ reg_val |= (CMN_PLL_REFCLK_EXTERNAL |
+ FIELD_PREP(CMN_PLL_REFCLK_INDEX, 6));
+ break;
+ case CMN_REFCLK_EXTERNAL_48MHZ:
+ reg_val &= ~(CMN_PLL_REFCLK_EXTERNAL |
+ CMN_PLL_REFCLK_INDEX);
+ reg_val |= (CMN_PLL_REFCLK_EXTERNAL |
+ FIELD_PREP(CMN_PLL_REFCLK_INDEX, 7));
+ break;
+ case CMN_REFCLK_EXTERNAL_50MHZ:
+ reg_val &= ~(CMN_PLL_REFCLK_EXTERNAL |
+ CMN_PLL_REFCLK_INDEX);
+ reg_val |= (CMN_PLL_REFCLK_EXTERNAL |
+ FIELD_PREP(CMN_PLL_REFCLK_INDEX, 8));
+ break;
+ case CMN_REFCLK_INTERNAL_96MHZ:
+ src_sel = readl(priv->cmn_membase +
+ CMN_PLL_REFERENCE_SOURCE_SEL);
+ src_sel &= ~CMN_PLL_REFCLK_SOURCE_DIV;
+ src_sel |= FIELD_PREP(CMN_PLL_REFCLK_SOURCE_DIV, 0);
+ writel(src_sel, priv->cmn_membase +
+ CMN_PLL_REFERENCE_SOURCE_SEL);
+
+ reg_val &= ~CMN_PLL_REFCLK_DIV;
+ reg_val |= FIELD_PREP(CMN_PLL_REFCLK_DIV, 2);
+ break;
+ default:
+ return -EINVAL;
+ }
+ } else if (ret == -EINVAL) {
+ /* If the cmn-reference-clock is not specified,
+ * the internal 48MHZ is selected by default.
+ */
+ reg_val |= FIELD_PREP(CMN_PLL_REFCLK_INDEX, 7);
+ } else {
+ return ret;
+ }
+
+ writel(reg_val, priv->cmn_membase + CMN_PLL_REFERENCE_CLOCK);
+
+ /* assert CMN PLL */
+ reg_val = readl(priv->cmn_membase + CMN_PLL_POWER_ON_AND_RESET);
+ reg_val &= ~CMN_ANA_EN_SW_RSTN;
+ writel(reg_val, priv->cmn_membase);
+ fsleep(IPQ_PHY_SET_DELAY_US);
+
+ /* deassert CMN PLL */
+ reg_val |= CMN_ANA_EN_SW_RSTN;
+ writel(reg_val, priv->cmn_membase + CMN_PLL_POWER_ON_AND_RESET);
+ fsleep(IPQ_PHY_SET_DELAY_US);
+ }
+
+ return 0;
+}
+
static int ipq_mdio_reset(struct mii_bus *bus)
{
struct ipq4019_mdio_data *priv = bus->priv;
unsigned long rate;
int ret, index;
+ ret = ipq_cmn_clock_config(bus);
+ if (ret)
+ return ret;
+
/* 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
@@ -296,7 +420,7 @@ static int ipq4019_mdio_probe(struct platform_device *pdev)
/* This resource are optional */
for (index = 0; index < ETH_LDO_RDY_CNT; index++) {
res = platform_get_resource(pdev, IORESOURCE_MEM, index + 1);
- if (res) {
+ if (res && strcmp(res->name, "cmn_blk")) {
priv->eth_ldo_rdy[index] = devm_ioremap(&pdev->dev,
res->start,
resource_size(res));
@@ -317,6 +441,17 @@ static int ipq4019_mdio_probe(struct platform_device *pdev)
}
}
+ /* The CMN block resource is for providing clock source to ethernet,
+ * which can be optionally configured on the platform ipq9574 and
+ * ipq5332.
+ */
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cmn_blk");
+ if (res) {
+ priv->cmn_membase = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(priv->cmn_membase))
+ return PTR_ERR(priv->cmn_membase);
+ }
+
for (index = 0; index < MDIO_CLK_CNT; index++) {
priv->clk[index] = devm_clk_get_optional(&pdev->dev,
mdio_clk_name[index]);
--
2.42.0
The MDIO clock frequency can be divided according to the
MDIO control register value.
The MDIO system clock is fixed to 100MHZ, the working
frequency is 100MHZ/(divider + 1), the divider value
is from the bit[7:0] of control register 0x40.
Signed-off-by: Luo Jie <[email protected]>
---
drivers/net/mdio/mdio-ipq4019.c | 45 +++++++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/drivers/net/mdio/mdio-ipq4019.c b/drivers/net/mdio/mdio-ipq4019.c
index 3568ce7f48c6..330963026475 100644
--- a/drivers/net/mdio/mdio-ipq4019.c
+++ b/drivers/net/mdio/mdio-ipq4019.c
@@ -29,6 +29,9 @@
/* 0 = Clause 22, 1 = Clause 45 */
#define MDIO_MODE_C45 BIT(8)
+/* MDC frequency is SYS_CLK/(MDIO_CLK_DIV + 1), SYS_CLK is 100MHz */
+#define MDIO_CLK_DIV_MASK GENMASK(7, 0)
+
#define IPQ4019_MDIO_TIMEOUT 10000
#define IPQ4019_MDIO_SLEEP 10
@@ -77,6 +80,7 @@ struct ipq4019_mdio_data {
void __iomem *cmn_membase;
void __iomem *eth_ldo_rdy[ETH_LDO_RDY_CNT];
struct clk *clk[MDIO_CLK_CNT];
+ int clk_div;
};
static const char *const mdio_clk_name[] = {
@@ -110,6 +114,7 @@ static int ipq4019_mdio_read_c45(struct mii_bus *bus, int mii_id, int mmd,
data = readl(priv->membase + MDIO_MODE_REG);
data |= MDIO_MODE_C45;
+ data |= FIELD_PREP(MDIO_CLK_DIV_MASK, priv->clk_div);
writel(data, priv->membase + MDIO_MODE_REG);
@@ -151,6 +156,7 @@ static int ipq4019_mdio_read_c22(struct mii_bus *bus, int mii_id, int regnum)
data = readl(priv->membase + MDIO_MODE_REG);
data &= ~MDIO_MODE_C45;
+ data |= FIELD_PREP(MDIO_CLK_DIV_MASK, priv->clk_div);
writel(data, priv->membase + MDIO_MODE_REG);
@@ -183,6 +189,7 @@ static int ipq4019_mdio_write_c45(struct mii_bus *bus, int mii_id, int mmd,
data = readl(priv->membase + MDIO_MODE_REG);
data |= MDIO_MODE_C45;
+ data |= FIELD_PREP(MDIO_CLK_DIV_MASK, priv->clk_div);
writel(data, priv->membase + MDIO_MODE_REG);
@@ -226,6 +233,7 @@ static int ipq4019_mdio_write_c22(struct mii_bus *bus, int mii_id, int regnum,
data = readl(priv->membase + MDIO_MODE_REG);
data &= ~MDIO_MODE_C45;
+ data |= FIELD_PREP(MDIO_CLK_DIV_MASK, priv->clk_div);
writel(data, priv->membase + MDIO_MODE_REG);
@@ -397,6 +405,39 @@ static int ipq_mdio_reset(struct mii_bus *bus)
return ret;
}
+static int ipq_mdio_clk_set(struct platform_device *pdev, int *clk_div)
+{
+ int freq;
+
+ /* Keep the MDIO clock divider as the hardware default value 0xff if
+ * the MDIO property "clock-frequency" is not specified.
+ */
+ if (of_property_read_u32(pdev->dev.of_node, "clock-frequency", &freq)) {
+ *clk_div = 0xff;
+ return 0;
+ }
+
+ /* MDC frequency is SYS_CLK/(MDIO_CLK_DIV + 1), SYS_CLK is fixed
+ * to 100MHz, the MDIO_CLK_DIV can be only configured the valid
+ * values, other values cause malfunction.
+ */
+ switch (freq) {
+ case 12500000:
+ case 6250000:
+ case 3125000:
+ case 1562500:
+ case 781250:
+ case 390625:
+ *clk_div = DIV_ROUND_UP(IPQ_MDIO_CLK_RATE, freq) - 1;
+ break;
+ default:
+ dev_err(&pdev->dev, "Invalid clock frequency %dHZ\n", freq);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static int ipq4019_mdio_probe(struct platform_device *pdev)
{
struct ipq4019_mdio_data *priv;
@@ -459,6 +500,10 @@ static int ipq4019_mdio_probe(struct platform_device *pdev)
return PTR_ERR(priv->clk[index]);
}
+ ret = ipq_mdio_clk_set(pdev, &priv->clk_div);
+ if (ret)
+ return ret;
+
bus->name = "ipq4019_mdio";
bus->read = ipq4019_mdio_read_c22;
bus->write = ipq4019_mdio_write_c22;
--
2.42.0
Update the yaml file for the new DTS properties.
1. cmn-reference-clock for the CMN PLL source clock select.
2. clock-frequency for MDIO clock frequency config.
3. add uniphy AHB & SYS GCC clocks.
4. add reset-gpios for MDIO bus level reset.
Signed-off-by: Luo Jie <[email protected]>
---
.../bindings/net/qcom,ipq4019-mdio.yaml | 143 +++++++++++++++++-
1 file changed, 139 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml b/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
index 3407e909e8a7..79f8513739e7 100644
--- a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
+++ b/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
@@ -20,6 +20,8 @@ properties:
- enum:
- qcom,ipq6018-mdio
- qcom,ipq8074-mdio
+ - qcom,ipq9574-mdio
+ - qcom,ipq5332-mdio
- const: qcom,ipq4019-mdio
"#address-cells":
@@ -30,19 +32,77 @@ properties:
reg:
minItems: 1
- maxItems: 2
+ maxItems: 5
description:
- the first Address and length of the register set for the MDIO controller.
- the second Address and length of the register for ethernet LDO, this second
- address range is only required by the platform IPQ50xx.
+ the first Address and length of the register set for the MDIO controller,
+ the optional second, third and fourth address and length of the register
+ for ethernet LDO, these three address range are required by the platform
+ IPQ50xx/IPQ5332, the last address and length is for the CMN clock to
+ select the reference clock.
+
+ reg-names:
+ minItems: 1
+ maxItems: 5
clocks:
+ minItems: 1
items:
- description: MDIO clock source frequency fixed to 100MHZ
+ - description: UNIPHY0 AHB clock source frequency fixed to 100MHZ
+ - description: UNIPHY1 AHB clock source frequency fixed to 100MHZ
+ - description: UNIPHY0 SYS clock source frequency fixed to 24MHZ
+ - description: UNIPHY1 SYS clock source frequency fixed to 24MHZ
clock-names:
+ minItems: 1
items:
- const: gcc_mdio_ahb_clk
+ - const: uniphy0_ahb
+ - const: uniphy1_ahb
+ - const: uniphy0_sys
+ - const: uniphy1_sys
+
+ cmn-reference-clock:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ oneOf:
+ - items:
+ - enum:
+ - 0 # CMN PLL reference internal 48MHZ
+ - 1 # CMN PLL reference external 25MHZ
+ - 2 # CMN PLL reference external 31250KHZ
+ - 3 # CMN PLL reference external 40MHZ
+ - 4 # CMN PLL reference external 48MHZ
+ - 5 # CMN PLL reference external 50MHZ
+ - 6 # CMN PLL reference internal 96MHZ
+
+ clock-frequency:
+ oneOf:
+ - items:
+ - enum:
+ - 12500000
+ - 6250000
+ - 3125000
+ - 1562500
+ - 781250
+ - 390625
+ description:
+ The MDIO bus clock that must be output by the MDIO bus hardware,
+ only the listed frequencies above can be supported, other frequency
+ will cause malfunction. If absent, the default hardware value 0xff
+ is used, which means the default MDIO clock frequency 390625HZ, The
+ MDIO clock frequency is MDIO_SYS_CLK/(MDIO_CLK_DIV + 1), the SoC
+ MDIO_SYS_CLK is fixed to 100MHZ, the MDIO_CLK_DIV is from MDIO control
+ register, there is higher clock frequency requirement on the normal
+ working case where the MDIO slave devices support high clock frequency.
+
+ reset-gpios:
+ maxItems: 1
+
+ reset-assert-us:
+ maxItems: 1
+
+ reset-deassert-us:
+ maxItems: 1
required:
- compatible
@@ -61,6 +121,8 @@ allOf:
- qcom,ipq5018-mdio
- qcom,ipq6018-mdio
- qcom,ipq8074-mdio
+ - qcom,ipq5332-mdio
+ - qcom,ipq9574-mdio
then:
required:
- clocks
@@ -70,6 +132,20 @@ allOf:
clocks: false
clock-names: false
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,ipq5332-mdio
+ then:
+ properties:
+ clocks:
+ minItems: 5
+ maxItems: 5
+ reg-names:
+ minItems: 4
+
unevaluatedProperties: false
examples:
@@ -100,3 +176,62 @@ examples:
reg = <4>;
};
};
+
+ - |
+ #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
+ #include <dt-bindings/gpio/gpio.h>
+
+ mdio@90000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "qcom,ipq5332-mdio",
+ "qcom,ipq4019-mdio";
+ cmn-reference-clock = <0>;
+ clock-frequency = <6250000>;
+
+ reset-gpios = <&tlmm 51 GPIO_ACTIVE_LOW>;
+ reset-assert-us = <100000>;
+ reset-deassert-us = <100000>;
+
+ reg = <0x90000 0x64>,
+ <0x9B000 0x800>,
+ <0x7A00610 0x4>,
+ <0x7A10610 0x4>;
+
+ reg-names = "mdio",
+ "cmn_blk",
+ "eth_ldo1",
+ "eth_ldo2";
+
+ clocks = <&gcc GCC_MDIO_AHB_CLK>,
+ <&gcc GCC_UNIPHY0_AHB_CLK>,
+ <&gcc GCC_UNIPHY1_AHB_CLK>,
+ <&gcc GCC_UNIPHY0_SYS_CLK>,
+ <&gcc GCC_UNIPHY1_SYS_CLK>;
+
+ clock-names = "gcc_mdio_ahb_clk",
+ "uniphy0_ahb",
+ "uniphy1_ahb",
+ "uniphy0_sys",
+ "uniphy1_sys";
+
+ qca8kphy0: ethernet-phy@1 {
+ compatible = "ethernet-phy-id004d.d180";
+ reg = <1>;
+ };
+
+ qca8kphy1: ethernet-phy@2 {
+ compatible = "ethernet-phy-id004d.d180";
+ reg = <2>;
+ };
+
+ qca8kphy2: ethernet-phy@3 {
+ compatible = "ethernet-phy-id004d.d180";
+ reg = <3>;
+ };
+
+ qca8kphy3: ethernet-phy@4 {
+ compatible = "ethernet-phy-id004d.d180";
+ reg = <4>;
+ };
+ };
--
2.42.0
On Thu, Dec 14, 2023 at 05:03:04PM +0800, Luo Jie wrote:
> Update the yaml file for the new DTS properties.
>
> 1. cmn-reference-clock for the CMN PLL source clock select.
> 2. clock-frequency for MDIO clock frequency config.
> 3. add uniphy AHB & SYS GCC clocks.
> 4. add reset-gpios for MDIO bus level reset.
>
> Signed-off-by: Luo Jie <[email protected]>
Can you please wait until discussion has finished on a patchset before
sending another version? I had not yet got a chance to look at the
reply you sent to my comments on v2.
Thanks,
Conor.
> ---
> .../bindings/net/qcom,ipq4019-mdio.yaml | 143 +++++++++++++++++-
> 1 file changed, 139 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml b/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
> index 3407e909e8a7..79f8513739e7 100644
> --- a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
> +++ b/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
> @@ -20,6 +20,8 @@ properties:
> - enum:
> - qcom,ipq6018-mdio
> - qcom,ipq8074-mdio
> + - qcom,ipq9574-mdio
> + - qcom,ipq5332-mdio
> - const: qcom,ipq4019-mdio
>
> "#address-cells":
> @@ -30,19 +32,77 @@ properties:
>
> reg:
> minItems: 1
> - maxItems: 2
> + maxItems: 5
> description:
> - the first Address and length of the register set for the MDIO controller.
> - the second Address and length of the register for ethernet LDO, this second
> - address range is only required by the platform IPQ50xx.
> + the first Address and length of the register set for the MDIO controller,
> + the optional second, third and fourth address and length of the register
> + for ethernet LDO, these three address range are required by the platform
> + IPQ50xx/IPQ5332, the last address and length is for the CMN clock to
> + select the reference clock.
> +
> + reg-names:
> + minItems: 1
> + maxItems: 5
>
> clocks:
> + minItems: 1
> items:
> - description: MDIO clock source frequency fixed to 100MHZ
> + - description: UNIPHY0 AHB clock source frequency fixed to 100MHZ
> + - description: UNIPHY1 AHB clock source frequency fixed to 100MHZ
> + - description: UNIPHY0 SYS clock source frequency fixed to 24MHZ
> + - description: UNIPHY1 SYS clock source frequency fixed to 24MHZ
>
> clock-names:
> + minItems: 1
> items:
> - const: gcc_mdio_ahb_clk
> + - const: uniphy0_ahb
> + - const: uniphy1_ahb
> + - const: uniphy0_sys
> + - const: uniphy1_sys
> +
> + cmn-reference-clock:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + oneOf:
> + - items:
> + - enum:
> + - 0 # CMN PLL reference internal 48MHZ
> + - 1 # CMN PLL reference external 25MHZ
> + - 2 # CMN PLL reference external 31250KHZ
> + - 3 # CMN PLL reference external 40MHZ
> + - 4 # CMN PLL reference external 48MHZ
> + - 5 # CMN PLL reference external 50MHZ
> + - 6 # CMN PLL reference internal 96MHZ
> +
> + clock-frequency:
> + oneOf:
> + - items:
> + - enum:
> + - 12500000
> + - 6250000
> + - 3125000
> + - 1562500
> + - 781250
> + - 390625
> + description:
> + The MDIO bus clock that must be output by the MDIO bus hardware,
> + only the listed frequencies above can be supported, other frequency
> + will cause malfunction. If absent, the default hardware value 0xff
> + is used, which means the default MDIO clock frequency 390625HZ, The
> + MDIO clock frequency is MDIO_SYS_CLK/(MDIO_CLK_DIV + 1), the SoC
> + MDIO_SYS_CLK is fixed to 100MHZ, the MDIO_CLK_DIV is from MDIO control
> + register, there is higher clock frequency requirement on the normal
> + working case where the MDIO slave devices support high clock frequency.
> +
> + reset-gpios:
> + maxItems: 1
> +
> + reset-assert-us:
> + maxItems: 1
> +
> + reset-deassert-us:
> + maxItems: 1
>
> required:
> - compatible
> @@ -61,6 +121,8 @@ allOf:
> - qcom,ipq5018-mdio
> - qcom,ipq6018-mdio
> - qcom,ipq8074-mdio
> + - qcom,ipq5332-mdio
> + - qcom,ipq9574-mdio
> then:
> required:
> - clocks
> @@ -70,6 +132,20 @@ allOf:
> clocks: false
> clock-names: false
>
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - qcom,ipq5332-mdio
> + then:
> + properties:
> + clocks:
> + minItems: 5
> + maxItems: 5
> + reg-names:
> + minItems: 4
> +
> unevaluatedProperties: false
>
> examples:
> @@ -100,3 +176,62 @@ examples:
> reg = <4>;
> };
> };
> +
> + - |
> + #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
> + #include <dt-bindings/gpio/gpio.h>
> +
> + mdio@90000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "qcom,ipq5332-mdio",
> + "qcom,ipq4019-mdio";
> + cmn-reference-clock = <0>;
> + clock-frequency = <6250000>;
> +
> + reset-gpios = <&tlmm 51 GPIO_ACTIVE_LOW>;
> + reset-assert-us = <100000>;
> + reset-deassert-us = <100000>;
> +
> + reg = <0x90000 0x64>,
> + <0x9B000 0x800>,
> + <0x7A00610 0x4>,
> + <0x7A10610 0x4>;
> +
> + reg-names = "mdio",
> + "cmn_blk",
> + "eth_ldo1",
> + "eth_ldo2";
> +
> + clocks = <&gcc GCC_MDIO_AHB_CLK>,
> + <&gcc GCC_UNIPHY0_AHB_CLK>,
> + <&gcc GCC_UNIPHY1_AHB_CLK>,
> + <&gcc GCC_UNIPHY0_SYS_CLK>,
> + <&gcc GCC_UNIPHY1_SYS_CLK>;
> +
> + clock-names = "gcc_mdio_ahb_clk",
> + "uniphy0_ahb",
> + "uniphy1_ahb",
> + "uniphy0_sys",
> + "uniphy1_sys";
> +
> + qca8kphy0: ethernet-phy@1 {
> + compatible = "ethernet-phy-id004d.d180";
> + reg = <1>;
> + };
> +
> + qca8kphy1: ethernet-phy@2 {
> + compatible = "ethernet-phy-id004d.d180";
> + reg = <2>;
> + };
> +
> + qca8kphy2: ethernet-phy@3 {
> + compatible = "ethernet-phy-id004d.d180";
> + reg = <3>;
> + };
> +
> + qca8kphy3: ethernet-phy@4 {
> + compatible = "ethernet-phy-id004d.d180";
> + reg = <4>;
> + };
> + };
> --
> 2.42.0
>
On 12/14/2023 11:58 PM, Conor Dooley wrote:
>> Update the yaml file for the new DTS properties.
>>
>> 1. cmn-reference-clock for the CMN PLL source clock select.
>> 2. clock-frequency for MDIO clock frequency config.
>> 3. add uniphy AHB & SYS GCC clocks.
>> 4. add reset-gpios for MDIO bus level reset.
>>
>> Signed-off-by: Luo Jie<[email protected]>
> Can you please wait until discussion has finished on a patchset before
> sending another version? I had not yet got a chance to look at the
> reply you sent to my comments on v2.
>
> Thanks,
> Conor.
got it, will keep this in mind, thanks for the suggestion.
On 14/12/2023 10:03, Luo Jie wrote:
> Update the yaml file for the new DTS properties.
>
> 1. cmn-reference-clock for the CMN PLL source clock select.
> 2. clock-frequency for MDIO clock frequency config.
> 3. add uniphy AHB & SYS GCC clocks.
> 4. add reset-gpios for MDIO bus level reset.
>
> Signed-off-by: Luo Jie <[email protected]>
> ---
> .../bindings/net/qcom,ipq4019-mdio.yaml | 143 +++++++++++++++++-
> 1 file changed, 139 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml b/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
> index 3407e909e8a7..79f8513739e7 100644
> --- a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
> +++ b/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
> @@ -20,6 +20,8 @@ properties:
> - enum:
> - qcom,ipq6018-mdio
> - qcom,ipq8074-mdio
> + - qcom,ipq9574-mdio
> + - qcom,ipq5332-mdio
> - const: qcom,ipq4019-mdio
>
> "#address-cells":
> @@ -30,19 +32,77 @@ properties:
>
> reg:
> minItems: 1
> - maxItems: 2
> + maxItems: 5
> description:
> - the first Address and length of the register set for the MDIO controller.
> - the second Address and length of the register for ethernet LDO, this second
> - address range is only required by the platform IPQ50xx.
> + the first Address and length of the register set for the MDIO controller,
> + the optional second, third and fourth address and length of the register
> + for ethernet LDO, these three address range are required by the platform
> + IPQ50xx/IPQ5332, the last address and length is for the CMN clock to
> + select the reference clock.
> +
> + reg-names:
> + minItems: 1
> + maxItems: 5
>
> clocks:
> + minItems: 1
> items:
> - description: MDIO clock source frequency fixed to 100MHZ
> + - description: UNIPHY0 AHB clock source frequency fixed to 100MHZ
> + - description: UNIPHY1 AHB clock source frequency fixed to 100MHZ
> + - description: UNIPHY0 SYS clock source frequency fixed to 24MHZ
> + - description: UNIPHY1 SYS clock source frequency fixed to 24MHZ
>
> clock-names:
> + minItems: 1
> items:
> - const: gcc_mdio_ahb_clk
> + - const: uniphy0_ahb
> + - const: uniphy1_ahb
> + - const: uniphy0_sys
> + - const: uniphy1_sys
> +
> + cmn-reference-clock:
> + $ref: /schemas/types.yaml#/definitions/uint32
Nothing improved here
> + oneOf:
> + - items:
So it is enum or not?
> + - enum:
> + - 0 # CMN PLL reference internal 48MHZ
> + - 1 # CMN PLL reference external 25MHZ
> + - 2 # CMN PLL reference external 31250KHZ
> + - 3 # CMN PLL reference external 40MHZ
> + - 4 # CMN PLL reference external 48MHZ
> + - 5 # CMN PLL reference external 50MHZ
> + - 6 # CMN PLL reference internal 96MHZ
> +
> + clock-frequency:
> + oneOf:
> + - items:
Same questions
> + - enum:
> + - 12500000
> + - 6250000
> + - 3125000
> + - 1562500
> + - 781250
> + - 390625
> + description:
> + The MDIO bus clock that must be output by the MDIO bus hardware,
> + only the listed frequencies above can be supported, other frequency
> + will cause malfunction. If absent, the default hardware value 0xff
> + is used, which means the default MDIO clock frequency 390625HZ, The
> + MDIO clock frequency is MDIO_SYS_CLK/(MDIO_CLK_DIV + 1), the SoC
> + MDIO_SYS_CLK is fixed to 100MHZ, the MDIO_CLK_DIV is from MDIO control
> + register, there is higher clock frequency requirement on the normal
> + working case where the MDIO slave devices support high clock frequency.
> +
> + reset-gpios:
> + maxItems: 1
> +
> + reset-assert-us:
> + maxItems: 1
This does not look related to ipq5332.
> +
> + reset-deassert-us:
> + maxItems: 1
Neither this.
>
> required:
> - compatible
> @@ -61,6 +121,8 @@ allOf:
> - qcom,ipq5018-mdio
> - qcom,ipq6018-mdio
> - qcom,ipq8074-mdio
> + - qcom,ipq5332-mdio
> + - qcom,ipq9574-mdio
> then:
> required:
> - clocks
> @@ -70,6 +132,20 @@ allOf:
> clocks: false
> clock-names: false
>
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - qcom,ipq5332-mdio
> + then:
> + properties:
> + clocks:
> + minItems: 5
> + maxItems: 5
> + reg-names:
> + minItems: 4
> +
> unevaluatedProperties: false
>
> examples:
> @@ -100,3 +176,62 @@ examples:
> reg = <4>;
> };
> };
> +
> + - |
> + #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
> + #include <dt-bindings/gpio/gpio.h>
> +
> + mdio@90000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "qcom,ipq5332-mdio",
> + "qcom,ipq4019-mdio";
> + cmn-reference-clock = <0>;
> + clock-frequency = <6250000>;
> +
> + reset-gpios = <&tlmm 51 GPIO_ACTIVE_LOW>;
> + reset-assert-us = <100000>;
> + reset-deassert-us = <100000>;
> +
> + reg = <0x90000 0x64>,
> + <0x9B000 0x800>,
> + <0x7A00610 0x4>,
> + <0x7A10610 0x4>;
Lowercase hex, wrong order of properties. Align it with coding style.
Best regards,
Krzysztof
On 12/15/2023 3:27 PM, Krzysztof Kozlowski wrote:
> On 14/12/2023 10:03, Luo Jie wrote:
>> Update the yaml file for the new DTS properties.
>>
>> 1. cmn-reference-clock for the CMN PLL source clock select.
>> 2. clock-frequency for MDIO clock frequency config.
>> 3. add uniphy AHB & SYS GCC clocks.
>> 4. add reset-gpios for MDIO bus level reset.
>>
>> Signed-off-by: Luo Jie <[email protected]>
>> ---
>> .../bindings/net/qcom,ipq4019-mdio.yaml | 143 +++++++++++++++++-
>> 1 file changed, 139 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml b/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
>> index 3407e909e8a7..79f8513739e7 100644
>> --- a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
>> +++ b/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
>> @@ -20,6 +20,8 @@ properties:
>> - enum:
>> - qcom,ipq6018-mdio
>> - qcom,ipq8074-mdio
>> + - qcom,ipq9574-mdio
>> + - qcom,ipq5332-mdio
>> - const: qcom,ipq4019-mdio
>>
>> "#address-cells":
>> @@ -30,19 +32,77 @@ properties:
>>
>> reg:
>> minItems: 1
>> - maxItems: 2
>> + maxItems: 5
>> description:
>> - the first Address and length of the register set for the MDIO controller.
>> - the second Address and length of the register for ethernet LDO, this second
>> - address range is only required by the platform IPQ50xx.
>> + the first Address and length of the register set for the MDIO controller,
>> + the optional second, third and fourth address and length of the register
>> + for ethernet LDO, these three address range are required by the platform
>> + IPQ50xx/IPQ5332, the last address and length is for the CMN clock to
>> + select the reference clock.
>> +
>> + reg-names:
>> + minItems: 1
>> + maxItems: 5
>>
>> clocks:
>> + minItems: 1
>> items:
>> - description: MDIO clock source frequency fixed to 100MHZ
>> + - description: UNIPHY0 AHB clock source frequency fixed to 100MHZ
>> + - description: UNIPHY1 AHB clock source frequency fixed to 100MHZ
>> + - description: UNIPHY0 SYS clock source frequency fixed to 24MHZ
>> + - description: UNIPHY1 SYS clock source frequency fixed to 24MHZ
>>
>> clock-names:
>> + minItems: 1
>> items:
>> - const: gcc_mdio_ahb_clk
>> + - const: uniphy0_ahb
>> + - const: uniphy1_ahb
>> + - const: uniphy0_sys
>> + - const: uniphy1_sys
>> +
>> + cmn-reference-clock:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>
> Nothing improved here
With this change, the warning is not reported when i run
dt_binding_check, looks the new added property needs
the type ref to avoid the warning reported.
>
>> + oneOf:
>> + - items:
>
> So it is enum or not?
it's enum, i will remove the "oneOf: - items:" in the next patch set.
>
>> + - enum:
>> + - 0 # CMN PLL reference internal 48MHZ
>> + - 1 # CMN PLL reference external 25MHZ
>> + - 2 # CMN PLL reference external 31250KHZ
>> + - 3 # CMN PLL reference external 40MHZ
>> + - 4 # CMN PLL reference external 48MHZ
>> + - 5 # CMN PLL reference external 50MHZ
>> + - 6 # CMN PLL reference internal 96MHZ
>> +
>> + clock-frequency:
>> + oneOf:
>> + - items:
>
> Same questions
it's enum.
>
>> + - enum:
>> + - 12500000
>> + - 6250000
>> + - 3125000
>> + - 1562500
>> + - 781250
>> + - 390625
>> + description:
>> + The MDIO bus clock that must be output by the MDIO bus hardware,
>> + only the listed frequencies above can be supported, other frequency
>> + will cause malfunction. If absent, the default hardware value 0xff
>> + is used, which means the default MDIO clock frequency 390625HZ, The
>> + MDIO clock frequency is MDIO_SYS_CLK/(MDIO_CLK_DIV + 1), the SoC
>> + MDIO_SYS_CLK is fixed to 100MHZ, the MDIO_CLK_DIV is from MDIO control
>> + register, there is higher clock frequency requirement on the normal
>> + working case where the MDIO slave devices support high clock frequency.
>> +
>> + reset-gpios:
>> + maxItems: 1
>> +
>> + reset-assert-us:
>> + maxItems: 1
>
> This does not look related to ipq5332.
The reset gpio properties are needed on ipq5332, since qca8084 phy is
connected, which uses the MDIO bus level gpio reset.
Without declaring these gpio properties, the warning will be reported
by dt_binding_check.
>
>> +
>> + reset-deassert-us:
>> + maxItems: 1
>
> Neither this.
same as above.
>
>>
>> required:
>> - compatible
>> @@ -61,6 +121,8 @@ allOf:
>> - qcom,ipq5018-mdio
>> - qcom,ipq6018-mdio
>> - qcom,ipq8074-mdio
>> + - qcom,ipq5332-mdio
>> + - qcom,ipq9574-mdio
>> then:
>> required:
>> - clocks
>> @@ -70,6 +132,20 @@ allOf:
>> clocks: false
>> clock-names: false
>>
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + enum:
>> + - qcom,ipq5332-mdio
>> + then:
>> + properties:
>> + clocks:
>> + minItems: 5
>> + maxItems: 5
>> + reg-names:
>> + minItems: 4
>> +
>> unevaluatedProperties: false
>>
>> examples:
>> @@ -100,3 +176,62 @@ examples:
>> reg = <4>;
>> };
>> };
>> +
>> + - |
>> + #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
>> + #include <dt-bindings/gpio/gpio.h>
>> +
>> + mdio@90000 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + compatible = "qcom,ipq5332-mdio",
>> + "qcom,ipq4019-mdio";
>> + cmn-reference-clock = <0>;
>> + clock-frequency = <6250000>;
>> +
>> + reset-gpios = <&tlmm 51 GPIO_ACTIVE_LOW>;
>> + reset-assert-us = <100000>;
>> + reset-deassert-us = <100000>;
>> +
>> + reg = <0x90000 0x64>,
>> + <0x9B000 0x800>,
>> + <0x7A00610 0x4>,
>> + <0x7A10610 0x4>;
>
> Lowercase hex, wrong order of properties. Align it with coding style.
will correct it in the next patch set, thanks.
>
>
>
> Best regards,
> Krzysztof
>
On 15/12/2023 09:28, Jie Luo wrote:
>
>
> On 12/15/2023 3:27 PM, Krzysztof Kozlowski wrote:
>> On 14/12/2023 10:03, Luo Jie wrote:
>>> Update the yaml file for the new DTS properties.
>>>
>>> 1. cmn-reference-clock for the CMN PLL source clock select.
>>> 2. clock-frequency for MDIO clock frequency config.
>>> 3. add uniphy AHB & SYS GCC clocks.
>>> 4. add reset-gpios for MDIO bus level reset.
>>>
>>> Signed-off-by: Luo Jie <[email protected]>
>>> ---
>>> .../bindings/net/qcom,ipq4019-mdio.yaml | 143 +++++++++++++++++-
>>> 1 file changed, 139 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml b/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
>>> index 3407e909e8a7..79f8513739e7 100644
>>> --- a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
>>> +++ b/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
>>> @@ -20,6 +20,8 @@ properties:
>>> - enum:
>>> - qcom,ipq6018-mdio
>>> - qcom,ipq8074-mdio
>>> + - qcom,ipq9574-mdio
>>> + - qcom,ipq5332-mdio
Why do you add entries to the end of the list? In reversed order?
>>> - const: qcom,ipq4019-mdio
>>>
>>> "#address-cells":
>>> @@ -30,19 +32,77 @@ properties:
>>>
>>> reg:
>>> minItems: 1
>>> - maxItems: 2
>>> + maxItems: 5
>>> description:
>>> - the first Address and length of the register set for the MDIO controller.
>>> - the second Address and length of the register for ethernet LDO, this second
>>> - address range is only required by the platform IPQ50xx.
>>> + the first Address and length of the register set for the MDIO controller,
>>> + the optional second, third and fourth address and length of the register
>>> + for ethernet LDO, these three address range are required by the platform
>>> + IPQ50xx/IPQ5332, the last address and length is for the CMN clock to
>>> + select the reference clock.
>>> +
>>> + reg-names:
>>> + minItems: 1
>>> + maxItems: 5
>>>
>>> clocks:
>>> + minItems: 1
>>> items:
>>> - description: MDIO clock source frequency fixed to 100MHZ
>>> + - description: UNIPHY0 AHB clock source frequency fixed to 100MHZ
>>> + - description: UNIPHY1 AHB clock source frequency fixed to 100MHZ
>>> + - description: UNIPHY0 SYS clock source frequency fixed to 24MHZ
>>> + - description: UNIPHY1 SYS clock source frequency fixed to 24MHZ
>>>
>>> clock-names:
>>> + minItems: 1
>>> items:
>>> - const: gcc_mdio_ahb_clk
>>> + - const: uniphy0_ahb
>>> + - const: uniphy1_ahb
>>> + - const: uniphy0_sys
>>> + - const: uniphy1_sys
>>> +
>>> + cmn-reference-clock:
>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>
>> Nothing improved here
>
> With this change, the warning is not reported when i run
> dt_binding_check, looks the new added property needs
> the type ref to avoid the warning reported.
Nothing improved in the property name, nor its style, nor in the actual
contents/values.
...
>>> + reset-gpios:
>>> + maxItems: 1
>>> +
>>> + reset-assert-us:
>>> + maxItems: 1
>>
>> This does not look related to ipq5332.
>
> The reset gpio properties are needed on ipq5332, since qca8084 phy is
> connected, which uses the MDIO bus level gpio reset.
I am talking about this property, not these properties.
>
> Without declaring these gpio properties, the warning will be reported
> by dt_binding_check.
How is it even possible to have warnings if there is no such node in
DTS? We do not care about warnings in your downstream code.
Best regards,
Krzysztof
On 12/15/2023 4:39 PM, Krzysztof Kozlowski wrote:
> On 15/12/2023 09:28, Jie Luo wrote:
>>
>>
>> On 12/15/2023 3:27 PM, Krzysztof Kozlowski wrote:
>>> On 14/12/2023 10:03, Luo Jie wrote:
>>>> Update the yaml file for the new DTS properties.
>>>>
>>>> 1. cmn-reference-clock for the CMN PLL source clock select.
>>>> 2. clock-frequency for MDIO clock frequency config.
>>>> 3. add uniphy AHB & SYS GCC clocks.
>>>> 4. add reset-gpios for MDIO bus level reset.
>>>>
>>>> Signed-off-by: Luo Jie <[email protected]>
>>>> ---
>>>> .../bindings/net/qcom,ipq4019-mdio.yaml | 143 +++++++++++++++++-
>>>> 1 file changed, 139 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml b/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
>>>> index 3407e909e8a7..79f8513739e7 100644
>>>> --- a/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
>>>> +++ b/Documentation/devicetree/bindings/net/qcom,ipq4019-mdio.yaml
>>>> @@ -20,6 +20,8 @@ properties:
>>>> - enum:
>>>> - qcom,ipq6018-mdio
>>>> - qcom,ipq8074-mdio
>>>> + - qcom,ipq9574-mdio
>>>> + - qcom,ipq5332-mdio
>
> Why do you add entries to the end of the list? In reversed order?
Thanks for pointing it out, i will move "- qcom,ipq5332-mdio" before
"- qcom,ipq6018-mdio".
>
>>>> - const: qcom,ipq4019-mdio
>>>>
>>>> "#address-cells":
>>>> @@ -30,19 +32,77 @@ properties:
>>>>
>>>> reg:
>>>> minItems: 1
>>>> - maxItems: 2
>>>> + maxItems: 5
>>>> description:
>>>> - the first Address and length of the register set for the MDIO controller.
>>>> - the second Address and length of the register for ethernet LDO, this second
>>>> - address range is only required by the platform IPQ50xx.
>>>> + the first Address and length of the register set for the MDIO controller,
>>>> + the optional second, third and fourth address and length of the register
>>>> + for ethernet LDO, these three address range are required by the platform
>>>> + IPQ50xx/IPQ5332, the last address and length is for the CMN clock to
>>>> + select the reference clock.
>>>> +
>>>> + reg-names:
>>>> + minItems: 1
>>>> + maxItems: 5
>>>>
>>>> clocks:
>>>> + minItems: 1
>>>> items:
>>>> - description: MDIO clock source frequency fixed to 100MHZ
>>>> + - description: UNIPHY0 AHB clock source frequency fixed to 100MHZ
>>>> + - description: UNIPHY1 AHB clock source frequency fixed to 100MHZ
>>>> + - description: UNIPHY0 SYS clock source frequency fixed to 24MHZ
>>>> + - description: UNIPHY1 SYS clock source frequency fixed to 24MHZ
>>>>
>>>> clock-names:
>>>> + minItems: 1
>>>> items:
>>>> - const: gcc_mdio_ahb_clk
>>>> + - const: uniphy0_ahb
>>>> + - const: uniphy1_ahb
>>>> + - const: uniphy0_sys
>>>> + - const: uniphy1_sys
>>>> +
>>>> + cmn-reference-clock:
>>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>>
>>> Nothing improved here
>>
>> With this change, the warning is not reported when i run
>> dt_binding_check, looks the new added property needs
>> the type ref to avoid the warning reported.
>
> Nothing improved in the property name, nor its style, nor in the actual
> contents/values.
This property is for CMN PLL block reference clock configuration,
so i use this property name.
it will be appreciated if you can suggest a suitable name, thanks.
>
> ...
>
>>>> + reset-gpios:
>>>> + maxItems: 1
>>>> +
>>>> + reset-assert-us:
>>>> + maxItems: 1
>>>
>>> This does not look related to ipq5332.
>>
>> The reset gpio properties are needed on ipq5332, since qca8084 phy is
>> connected, which uses the MDIO bus level gpio reset.
>
> I am talking about this property, not these properties.
ok.
>
>>
>> Without declaring these gpio properties, the warning will be reported
>> by dt_binding_check.
>
> How is it even possible to have warnings if there is no such node in
> DTS? We do not care about warnings in your downstream code.
>
>
> Best regards,
> Krzysztof
>
If i do not declare the property "reset-assert-us" and
"reset-deassert-us", the warning will be reported by "make
dt_binding_check" since i
add a example in this file.
On 15/12/2023 11:03, Jie Luo wrote:
>>>>> + cmn-reference-clock:
>>>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>>>
>>>> Nothing improved here
>>>
>>> With this change, the warning is not reported when i run
>>> dt_binding_check, looks the new added property needs
>>> the type ref to avoid the warning reported.
>>
>> Nothing improved in the property name, nor its style, nor in the actual
>> contents/values.
>
> This property is for CMN PLL block reference clock configuration,
> so i use this property name.
>
> it will be appreciated if you can suggest a suitable name, thanks.
See example-schema about naming. Read writing-bindings. You need vendor
prefix for custom properties.
>
>>
>> ...
>>
>>>>> + reset-gpios:
>>>>> + maxItems: 1
>>>>> +
>>>>> + reset-assert-us:
>>>>> + maxItems: 1
>>>>
>>>> This does not look related to ipq5332.
>>>
>>> The reset gpio properties are needed on ipq5332, since qca8084 phy is
>>> connected, which uses the MDIO bus level gpio reset.
>>
>> I am talking about this property, not these properties.
>
> ok.
>
>>
>>>
>>> Without declaring these gpio properties, the warning will be reported
>>> by dt_binding_check.
>>
>> How is it even possible to have warnings if there is no such node in
>> DTS? We do not care about warnings in your downstream code.
>>
>>
>> Best regards,
>> Krzysztof
>>
>
> If i do not declare the property "reset-assert-us" and
> "reset-deassert-us", the warning will be reported by "make
> dt_binding_check" since i
> add a example in this file.
This argument does not make sense, sorry. Obviously if property is not
allowed, it should be removed.
Provide rationale, in terms of hardware, why this property must be added
and why it cannot be deduced from the compatible.
Best regards,
Krzysztof
On 12/15/2023 6:21 PM, Krzysztof Kozlowski wrote:
> On 15/12/2023 11:03, Jie Luo wrote:
>>>>>> + cmn-reference-clock:
>>>>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>>>>
>>>>> Nothing improved here
>>>>
>>>> With this change, the warning is not reported when i run
>>>> dt_binding_check, looks the new added property needs
>>>> the type ref to avoid the warning reported.
>>>
>>> Nothing improved in the property name, nor its style, nor in the actual
>>> contents/values.
>>
>> This property is for CMN PLL block reference clock configuration,
>> so i use this property name.
>>
>> it will be appreciated if you can suggest a suitable name, thanks.
>
> See example-schema about naming. Read writing-bindings. You need vendor
> prefix for custom properties.
Ok, thanks.
>
>>
>>>
>>> ...
>>>
>>>>>> + reset-gpios:
>>>>>> + maxItems: 1
>>>>>> +
>>>>>> + reset-assert-us:
>>>>>> + maxItems: 1
>>>>>
>>>>> This does not look related to ipq5332.
>>>>
>>>> The reset gpio properties are needed on ipq5332, since qca8084 phy is
>>>> connected, which uses the MDIO bus level gpio reset.
>>>
>>> I am talking about this property, not these properties.
>>
>> ok.
>>
>>>
>>>>
>>>> Without declaring these gpio properties, the warning will be reported
>>>> by dt_binding_check.
>>>
>>> How is it even possible to have warnings if there is no such node in
>>> DTS? We do not care about warnings in your downstream code.
>>>
>>>
>>> Best regards,
>>> Krzysztof
>>>
>>
>> If i do not declare the property "reset-assert-us" and
>> "reset-deassert-us", the warning will be reported by "make
>> dt_binding_check" since i
>> add a example in this file.
>
> This argument does not make sense, sorry. Obviously if property is not
> allowed, it should be removed.
>
> Provide rationale, in terms of hardware, why this property must be added
> and why it cannot be deduced from the compatible.
>
> Best regards,
> Krzysztof
>
So i can remove "reset-assert-us" and "reset-deassert-us" from the added
example to avoid the dt check warning? even these two properties are
needed to be defined in the device tree to make this driver working
correctly.
On 15/12/2023 13:03, Jie Luo wrote:
>>> If i do not declare the property "reset-assert-us" and
>>> "reset-deassert-us", the warning will be reported by "make
>>> dt_binding_check" since i
>>> add a example in this file.
>>
>> This argument does not make sense, sorry. Obviously if property is not
>> allowed, it should be removed.
>>
>> Provide rationale, in terms of hardware, why this property must be added
>> and why it cannot be deduced from the compatible.
>>
>> Best regards,
>> Krzysztof
>>
>
> So i can remove "reset-assert-us" and "reset-deassert-us" from the added
> example to avoid the dt check warning? even these two properties are
> needed to be defined in the device tree to make this driver working
> correctly.
Sorry, that does not answer my question at all. First, "Driver" is not
hardware. My second question was simply ignored. In the v2 thread you as
well respond with some short, unrelated sentences not answering to the
real questions. It's a waste of my time. Please reach internally in
Qualcomm for guidance how to upstream patches and how to write bindings.
Best regards,
Krzysztof
On 12/15/2023 8:14 PM, Krzysztof Kozlowski wrote:
> On 15/12/2023 13:03, Jie Luo wrote:
>>>> If i do not declare the property "reset-assert-us" and
>>>> "reset-deassert-us", the warning will be reported by "make
>>>> dt_binding_check" since i
>>>> add a example in this file.
>>>
>>> This argument does not make sense, sorry. Obviously if property is not
>>> allowed, it should be removed.
>>>
>>> Provide rationale, in terms of hardware, why this property must be added
>>> and why it cannot be deduced from the compatible.
>>>
>>> Best regards,
>>> Krzysztof
>>>
>>
>> So i can remove "reset-assert-us" and "reset-deassert-us" from the added
>> example to avoid the dt check warning? even these two properties are
>> needed to be defined in the device tree to make this driver working
>> correctly.
>
> Sorry, that does not answer my question at all. First, "Driver" is not
> hardware. My second question was simply ignored. In the v2 thread you as
> well respond with some short, unrelated sentences not answering to the
> real questions. It's a waste of my time. Please reach internally in
> Qualcomm for guidance how to upstream patches and how to write bindings.
>
> Best regards,
> Krzysztof
>
These properties "reset-assert-us" and "reset-deassert-us" are the
general properties from mdio.yaml, which are used when the MDIO
bus driver is registered by the MDIO framework.
The general DT property already supports to do the correct config,
then compatible string is not needed to be checked for doing the
configs.
i will check the binding examples to avoid this kind of problems.
> These properties "reset-assert-us" and "reset-deassert-us" are the
> general properties from mdio.yaml, which are used when the MDIO
> bus driver is registered by the MDIO framework.
> The general DT property already supports to do the correct config,
> then compatible string is not needed to be checked for doing the
> configs.
Given the complexity of your device, i doubt you can make it work
without using a compatible containing the ID register values. That
will get your driver loaded, and the probe method called which can
then deal with all the clocks and resets in whatever way it wants.
Andrew
On 12/15/2023 9:34 PM, Andrew Lunn wrote:
>> These properties "reset-assert-us" and "reset-deassert-us" are the
>> general properties from mdio.yaml, which are used when the MDIO
>> bus driver is registered by the MDIO framework.
>> The general DT property already supports to do the correct config,
>> then compatible string is not needed to be checked for doing the
>> configs.
>
> Given the complexity of your device, i doubt you can make it work
> without using a compatible containing the ID register values. That
> will get your driver loaded, and the probe method called which can
> then deal with all the clocks and resets in whatever way it wants.
>
> Andrew
>
I misunderstood Krzysztof's suggestion in the previous message, i
thought the reset properties configuration is checked according
to the compatible string in the drive code.
Yes, these properties can be deduced from the compatible string in
the DT doc, i will update this in the next patch set.
Thanks for the comments and suggestion.