[PATCH 0/5] phy: ralink: mt7621-pci-phy: some improvements

[PATCH 0/5] phy: ralink: mt7621-pci-phy: some improvements

[Sergio Paracuellos]

Hi all,

This series contains some improvements in the pci phy driver
for MT7621 SoCs.

MT7621 SoC clock driver has already mainlined in
'commit 48df7a26f470 ("clk: ralink: add clock driver for mt7621 SoC")'

Because of this we can update schema documentation and device tree
to add related clock entries and avoid custom architecture code
in favour of using the clock kernel framework to retrieve clock 
frequency needed to properly configure the PCIe related Phys.

After this changes there is no problem to properly enable this
driver for COMPILE_TEST.

Configuration has also modified from 'tristate' to 'bool' depending
on PCI_MT7621 which seems to have more sense.

Thanks in advance for your time.

Best regards,
    Sergio Paracuellos


Sergio Paracuellos (5):
  staging: mt7621-dts: use clock in pci phy nodes
  dt-bindings: phy: mediatek,mt7621-pci-phy: add clock entries
  phy: ralink: phy-mt7621-pci: use kernel clock APIS
  phy: ralink: Kconfig: enable COMPILE_TEST on mt7621-pci-phy driver
  phy: ralink: Kconfig: convert mt7621-pci-phy into 'bool'

 .../bindings/phy/mediatek,mt7621-pci-phy.yaml | 12 +++++++
 drivers/phy/ralink/Kconfig                    |  4 +--
 drivers/phy/ralink/phy-mt7621-pci.c           | 33 +++++++++++--------
 drivers/staging/mt7621-dts/mt7621.dtsi        |  4 +++
 4 files changed, 38 insertions(+), 15 deletions(-)

-- 
2.25.1

[PATCH 1/5] staging: mt7621-dts: use clock in pci phy nodes

[Sergio Paracuellos]

MT7621 SoC clock driver has already mainlined in
'commit 48df7a26f470 ("clk: ralink: add clock driver for mt7621 SoC")'
Hence we can use the clock in pcie phy nodes to
be able to get it from there in driver code.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/mt7621-dts/mt7621.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/staging/mt7621-dts/mt7621.dtsi b/drivers/staging/mt7621-dts/mt7621.dtsi
index 5623d542bcf2..001ff8f51033 100644
--- a/drivers/staging/mt7621-dts/mt7621.dtsi
+++ b/drivers/staging/mt7621-dts/mt7621.dtsi
@@ -549,12 +549,16 @@ pcie@2,0 {
 	pcie0_phy: pcie-phy@1e149000 {
 		compatible = "mediatek,mt7621-pci-phy";
 		reg = <0x1e149000 0x0700>;
+		clocks = <&sysc MT7621_CLK_XTAL>;
+		clock-names = "sys_clk";
 		#phy-cells = <1>;
 	};
 
 	pcie2_phy: pcie-phy@1e14a000 {
 		compatible = "mediatek,mt7621-pci-phy";
 		reg = <0x1e14a000 0x0700>;
+		clocks = <&sysc MT7621_CLK_XTAL>;
+		clock-names = "sys_clk";
 		#phy-cells = <1>;
 	};
 };
-- 
2.25.1

[PATCH 2/5] dt-bindings: phy: mediatek,mt7621-pci-phy: add clock entries

[Sergio Paracuellos]

MT7621 SoC clock driver has already mainlined in
'commit 48df7a26f470 ("clk: ralink: add clock driver for mt7621 SoC")'
Hence update schema with the add of the entries related to
clock.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 .../bindings/phy/mediatek,mt7621-pci-phy.yaml        | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.yaml b/Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.yaml
index 0ccaded3f245..d8614ef8995c 100644
--- a/Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.yaml
@@ -16,6 +16,14 @@ properties:
   reg:
     maxItems: 1
 
+  clocks:
+    maxItems: 1
+    description:
+      PHY reference clock. Must contain an entry in clock-names.
+
+  clock-names:
+    const: sys_clk
+
   "#phy-cells":
     const: 1
     description: selects if the phy is dual-ported
@@ -23,6 +31,8 @@ properties:
 required:
   - compatible
   - reg
+  - clocks
+  - clock-names
   - "#phy-cells"
 
 additionalProperties: false
@@ -32,5 +42,7 @@ examples:
     pcie0_phy: pcie-phy@1e149000 {
       compatible = "mediatek,mt7621-pci-phy";
       reg = <0x1e149000 0x0700>;
+      clocks = <&sysc 0>;
+      clock-names = "sys_clk";
       #phy-cells = <1>;
     };
-- 
2.25.1

[PATCH 3/5] phy: ralink: phy-mt7621-pci: use kernel clock APIS

[Sergio Paracuellos]

MT7621 SoC clock driver has already mainlined in
'commit 48df7a26f470 ("clk: ralink: add clock driver for mt7621 SoC")'
This allow us to properly use kernel clock apis to get
the clock frequency needed for the phy configuration
instead of use custom architecture code to do the same.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/phy/ralink/phy-mt7621-pci.c | 33 +++++++++++++++++------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/drivers/phy/ralink/phy-mt7621-pci.c b/drivers/phy/ralink/phy-mt7621-pci.c
index 753cb5bab930..5222edc7be10 100644
--- a/drivers/phy/ralink/phy-mt7621-pci.c
+++ b/drivers/phy/ralink/phy-mt7621-pci.c
@@ -5,6 +5,7 @@
  */
 
 #include <dt-bindings/phy/phy.h>
+#include <linux/clk.h>
 #include <linux/bitfield.h>
 #include <linux/bitops.h>
 #include <linux/module.h>
@@ -14,8 +15,6 @@
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include <linux/sys_soc.h>
-#include <mt7621.h>
-#include <ralink_regs.h>
 
 #define RG_PE1_PIPE_REG				0x02c
 #define RG_PE1_PIPE_RST				BIT(12)
@@ -62,8 +61,6 @@
 
 #define RG_PE1_FRC_MSTCKDIV			BIT(5)
 
-#define XTAL_MASK				GENMASK(8, 6)
-
 #define MAX_PHYS	2
 
 /**
@@ -71,6 +68,7 @@
  * @dev: pointer to device
  * @regmap: kernel regmap pointer
  * @phy: pointer to the kernel PHY device
+ * @sys_clk: pointer to the system XTAL clock
  * @port_base: base register
  * @has_dual_port: if the phy has dual ports.
  * @bypass_pipe_rst: mark if 'mt7621_bypass_pipe_rst'
@@ -80,6 +78,7 @@ struct mt7621_pci_phy {
 	struct device *dev;
 	struct regmap *regmap;
 	struct phy *phy;
+	struct clk *sys_clk;
 	void __iomem *port_base;
 	bool has_dual_port;
 	bool bypass_pipe_rst;
@@ -116,12 +115,14 @@ static void mt7621_bypass_pipe_rst(struct mt7621_pci_phy *phy)
 	}
 }
 
-static void mt7621_set_phy_for_ssc(struct mt7621_pci_phy *phy)
+static int mt7621_set_phy_for_ssc(struct mt7621_pci_phy *phy)
 {
 	struct device *dev = phy->dev;
-	u32 xtal_mode;
+	unsigned long clk_rate;
 
-	xtal_mode = FIELD_GET(XTAL_MASK, rt_sysc_r32(SYSC_REG_SYSTEM_CONFIG0));
+	clk_rate = clk_get_rate(phy->sys_clk);
+	if (!clk_rate)
+		return -EINVAL;
 
 	/* Set PCIe Port PHY to disable SSC */
 	/* Debug Xtal Type */
@@ -139,13 +140,13 @@ static void mt7621_set_phy_for_ssc(struct mt7621_pci_phy *phy)
 			       RG_PE1_PHY_EN, RG_PE1_FRC_PHY_EN);
 	}
 
-	if (xtal_mode <= 5 && xtal_mode >= 3) { /* 40MHz Xtal */
+	if (clk_rate == 40000000) { /* 40MHz Xtal */
 		/* Set Pre-divider ratio (for host mode) */
 		mt7621_phy_rmw(phy, RG_PE1_H_PLL_REG, RG_PE1_H_PLL_PREDIV,
 			       FIELD_PREP(RG_PE1_H_PLL_PREDIV, 0x01));
 
 		dev_dbg(dev, "Xtal is 40MHz\n");
-	} else if (xtal_mode >= 6) { /* 25MHz Xal */
+	} else if (clk_rate == 25000000) { /* 25MHz Xal */
 		mt7621_phy_rmw(phy, RG_PE1_H_PLL_REG, RG_PE1_H_PLL_PREDIV,
 			       FIELD_PREP(RG_PE1_H_PLL_PREDIV, 0x00));
 
@@ -196,13 +197,15 @@ static void mt7621_set_phy_for_ssc(struct mt7621_pci_phy *phy)
 	mt7621_phy_rmw(phy, RG_PE1_H_PLL_BR_REG, RG_PE1_H_PLL_BR,
 		       FIELD_PREP(RG_PE1_H_PLL_BR, 0x00));
 
-	if (xtal_mode <= 5 && xtal_mode >= 3) { /* 40MHz Xtal */
+	if (clk_rate == 40000000) { /* 40MHz Xtal */
 		/* set force mode enable of da_pe1_mstckdiv */
 		mt7621_phy_rmw(phy, RG_PE1_MSTCKDIV_REG,
 			       RG_PE1_MSTCKDIV | RG_PE1_FRC_MSTCKDIV,
 			       FIELD_PREP(RG_PE1_MSTCKDIV, 0x01) |
 			       RG_PE1_FRC_MSTCKDIV);
 	}
+
+	return 0;
 }
 
 static int mt7621_pci_phy_init(struct phy *phy)
@@ -212,9 +215,7 @@ static int mt7621_pci_phy_init(struct phy *phy)
 	if (mphy->bypass_pipe_rst)
 		mt7621_bypass_pipe_rst(mphy);
 
-	mt7621_set_phy_for_ssc(mphy);
-
-	return 0;
+	return mt7621_set_phy_for_ssc(mphy);
 }
 
 static int mt7621_pci_phy_power_on(struct phy *phy)
@@ -324,6 +325,12 @@ static int mt7621_pci_phy_probe(struct platform_device *pdev)
 		return PTR_ERR(phy->phy);
 	}
 
+	phy->sys_clk = devm_clk_get(dev, "sys_clk");
+	if (IS_ERR(phy->sys_clk)) {
+		dev_err(dev, "failed to get phy clock\n");
+		return PTR_ERR(phy->sys_clk);
+	}
+
 	phy_set_drvdata(phy->phy, phy);
 
 	provider = devm_of_phy_provider_register(dev, mt7621_pcie_phy_of_xlate);
-- 
2.25.1

[PATCH 4/5] phy: ralink: Kconfig: enable COMPILE_TEST on mt7621-pci-phy driver

[Sergio Paracuellos]

After use the clock apis and avoid custom architecture
code this driver can properly be enabled for COMPILE_TEST.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/phy/ralink/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/phy/ralink/Kconfig b/drivers/phy/ralink/Kconfig
index ecc309ba9fee..c2373b30b8a6 100644
--- a/drivers/phy/ralink/Kconfig
+++ b/drivers/phy/ralink/Kconfig
@@ -4,7 +4,7 @@
 #
 config PHY_MT7621_PCI
 	tristate "MediaTek MT7621 PCI PHY Driver"
-	depends on RALINK && OF
+	depends on (RALINK && OF) || COMPILE_TEST
 	select GENERIC_PHY
 	select REGMAP_MMIO
 	help
-- 
2.25.1

[PATCH 5/5] phy: ralink: Kconfig: convert mt7621-pci-phy into 'bool'

[Sergio Paracuellos]

Make dependant on PCI_MT7621 configuration option and mark
this pci phy configuration as bool which has more sense.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/phy/ralink/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/ralink/Kconfig b/drivers/phy/ralink/Kconfig
index c2373b30b8a6..ed0c71eff2c4 100644
--- a/drivers/phy/ralink/Kconfig
+++ b/drivers/phy/ralink/Kconfig
@@ -3,8 +3,8 @@
 # PHY drivers for Ralink platforms.
 #
 config PHY_MT7621_PCI
-	tristate "MediaTek MT7621 PCI PHY Driver"
-	depends on (RALINK && OF) || COMPILE_TEST
+	bool "MediaTek MT7621 PCI PHY Driver"
+	depends on (RALINK && OF && PCI_MT7621) || COMPILE_TEST
 	select GENERIC_PHY
 	select REGMAP_MMIO
 	help
-- 
2.25.1

Re: [PATCH 4/5] phy: ralink: Kconfig: enable COMPILE_TEST on mt7621-pci-phy driver

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 3757 bytes --]

Hi Sergio,

I love your patch! Perhaps something to improve:

[auto build test WARNING on staging/staging-testing]
[also build test WARNING on linus/master next-20210506]
[cannot apply to robh/for-next v5.12]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Sergio-Paracuellos/phy-ralink-mt7621-pci-phy-some-improvements/20210506-191714
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git e120332923e13d8d9386594a83dc7cbf327e3edf
config: sparc-allyesconfig (attached as .config)
compiler: sparc64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/44e8bb824a0a4af6fc41d67b70ec3a678bd8125f
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Sergio-Paracuellos/phy-ralink-mt7621-pci-phy-some-improvements/20210506-191714
        git checkout 44e8bb824a0a4af6fc41d67b70ec3a678bd8125f
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross W=1 ARCH=sparc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from include/linux/device.h:15,
                    from include/linux/node.h:18,
                    from include/linux/cpu.h:17,
                    from include/linux/of_device.h:5,
                    from drivers/phy/ralink/phy-mt7621-pci.c:13:
   drivers/phy/ralink/phy-mt7621-pci.c: In function 'mt7621_pcie_phy_of_xlate':
>> drivers/phy/ralink/phy-mt7621-pci.c:277:4: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
     277 |    (unsigned int)mt7621_phy->port_base, mt7621_phy->has_dual_port);
         |    ^
   include/linux/dev_printk.h:118:33: note: in definition of macro 'dev_info'
     118 |  _dev_info(dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                 ^~~~~~~~~~~


vim +277 drivers/phy/ralink/phy-mt7621-pci.c

d87da32372a03c Sergio Paracuellos 2020-11-21  265  
d87da32372a03c Sergio Paracuellos 2020-11-21  266  static struct phy *mt7621_pcie_phy_of_xlate(struct device *dev,
d87da32372a03c Sergio Paracuellos 2020-11-21  267  					    struct of_phandle_args *args)
d87da32372a03c Sergio Paracuellos 2020-11-21  268  {
d87da32372a03c Sergio Paracuellos 2020-11-21  269  	struct mt7621_pci_phy *mt7621_phy = dev_get_drvdata(dev);
d87da32372a03c Sergio Paracuellos 2020-11-21  270  
d87da32372a03c Sergio Paracuellos 2020-11-21  271  	if (WARN_ON(args->args[0] >= MAX_PHYS))
d87da32372a03c Sergio Paracuellos 2020-11-21  272  		return ERR_PTR(-ENODEV);
d87da32372a03c Sergio Paracuellos 2020-11-21  273  
d87da32372a03c Sergio Paracuellos 2020-11-21  274  	mt7621_phy->has_dual_port = args->args[0];
d87da32372a03c Sergio Paracuellos 2020-11-21  275  
d87da32372a03c Sergio Paracuellos 2020-11-21  276  	dev_info(dev, "PHY for 0x%08x (dual port = %d)\n",
d87da32372a03c Sergio Paracuellos 2020-11-21 @277  		 (unsigned int)mt7621_phy->port_base, mt7621_phy->has_dual_port);
d87da32372a03c Sergio Paracuellos 2020-11-21  278  
d87da32372a03c Sergio Paracuellos 2020-11-21  279  	return mt7621_phy->phy;
d87da32372a03c Sergio Paracuellos 2020-11-21  280  }
d87da32372a03c Sergio Paracuellos 2020-11-21  281  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 69607 bytes --]

Re: [PATCH 4/5] phy: ralink: Kconfig: enable COMPILE_TEST on mt7621-pci-phy driver

[Sergio Paracuellos]

Hi,

On Thu, May 6, 2021 at 5:59 PM kernel test robot <lkp@intel.com> wrote:
>
> Hi Sergio,
>
> I love your patch! Perhaps something to improve:
>
> [auto build test WARNING on staging/staging-testing]
> [also build test WARNING on linus/master next-20210506]
> [cannot apply to robh/for-next v5.12]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url:    https://github.com/0day-ci/linux/commits/Sergio-Paracuellos/phy-ralink-mt7621-pci-phy-some-improvements/20210506-191714
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git e120332923e13d8d9386594a83dc7cbf327e3edf
> config: sparc-allyesconfig (attached as .config)
> compiler: sparc64-linux-gcc (GCC) 9.3.0
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # https://github.com/0day-ci/linux/commit/44e8bb824a0a4af6fc41d67b70ec3a678bd8125f
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Sergio-Paracuellos/phy-ralink-mt7621-pci-phy-some-improvements/20210506-191714
>         git checkout 44e8bb824a0a4af6fc41d67b70ec3a678bd8125f
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross W=1 ARCH=sparc
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
>    In file included from include/linux/device.h:15,
>                     from include/linux/node.h:18,
>                     from include/linux/cpu.h:17,
>                     from include/linux/of_device.h:5,
>                     from drivers/phy/ralink/phy-mt7621-pci.c:13:
>    drivers/phy/ralink/phy-mt7621-pci.c: In function 'mt7621_pcie_phy_of_xlate':
> >> drivers/phy/ralink/phy-mt7621-pci.c:277:4: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
>      277 |    (unsigned int)mt7621_phy->port_base, mt7621_phy->has_dual_port);
>          |    ^
>    include/linux/dev_printk.h:118:33: note: in definition of macro 'dev_info'
>      118 |  _dev_info(dev, dev_fmt(fmt), ##__VA_ARGS__)
>          |                                 ^~~~~~~~~~~
>

Thanks!, COMPILE_TEST is doing a good job there :). Ok, so I have just
sent a patch for this:
See: https://lkml.org/lkml/2021/5/7/28

Best regards,
    Sergio Paracuellos

>
> vim +277 drivers/phy/ralink/phy-mt7621-pci.c
>
> d87da32372a03c Sergio Paracuellos 2020-11-21  265
> d87da32372a03c Sergio Paracuellos 2020-11-21  266  static struct phy *mt7621_pcie_phy_of_xlate(struct device *dev,
> d87da32372a03c Sergio Paracuellos 2020-11-21  267                                           struct of_phandle_args *args)
> d87da32372a03c Sergio Paracuellos 2020-11-21  268  {
> d87da32372a03c Sergio Paracuellos 2020-11-21  269       struct mt7621_pci_phy *mt7621_phy = dev_get_drvdata(dev);
> d87da32372a03c Sergio Paracuellos 2020-11-21  270
> d87da32372a03c Sergio Paracuellos 2020-11-21  271       if (WARN_ON(args->args[0] >= MAX_PHYS))
> d87da32372a03c Sergio Paracuellos 2020-11-21  272               return ERR_PTR(-ENODEV);
> d87da32372a03c Sergio Paracuellos 2020-11-21  273
> d87da32372a03c Sergio Paracuellos 2020-11-21  274       mt7621_phy->has_dual_port = args->args[0];
> d87da32372a03c Sergio Paracuellos 2020-11-21  275
> d87da32372a03c Sergio Paracuellos 2020-11-21  276       dev_info(dev, "PHY for 0x%08x (dual port = %d)\n",
> d87da32372a03c Sergio Paracuellos 2020-11-21 @277                (unsigned int)mt7621_phy->port_base, mt7621_phy->has_dual_port);
> d87da32372a03c Sergio Paracuellos 2020-11-21  278
> d87da32372a03c Sergio Paracuellos 2020-11-21  279       return mt7621_phy->phy;
> d87da32372a03c Sergio Paracuellos 2020-11-21  280  }
> d87da32372a03c Sergio Paracuellos 2020-11-21  281
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Re: [PATCH 2/5] dt-bindings: phy: mediatek,mt7621-pci-phy: add clock entries

[Rob Herring]

On Thu, May 06, 2021 at 01:15:28PM +0200, Sergio Paracuellos wrote:
> MT7621 SoC clock driver has already mainlined in
> 'commit 48df7a26f470 ("clk: ralink: add clock driver for mt7621 SoC")'
> Hence update schema with the add of the entries related to
> clock.
> 
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> ---
>  .../bindings/phy/mediatek,mt7621-pci-phy.yaml        | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.yaml b/Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.yaml
> index 0ccaded3f245..d8614ef8995c 100644
> --- a/Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.yaml
> @@ -16,6 +16,14 @@ properties:
>    reg:
>      maxItems: 1
>  
> +  clocks:
> +    maxItems: 1
> +    description:
> +      PHY reference clock. Must contain an entry in clock-names.
> +
> +  clock-names:
> +    const: sys_clk

You don't really need -names when there is only 1.

> +
>    "#phy-cells":
>      const: 1
>      description: selects if the phy is dual-ported
> @@ -23,6 +31,8 @@ properties:
>  required:
>    - compatible
>    - reg
> +  - clocks
> +  - clock-names

Technically, you can't add new properties and make them required as that 
breaks the ABI. If that's okay here, explain it in the commit message. 

>    - "#phy-cells"
>  
>  additionalProperties: false
> @@ -32,5 +42,7 @@ examples:
>      pcie0_phy: pcie-phy@1e149000 {
>        compatible = "mediatek,mt7621-pci-phy";
>        reg = <0x1e149000 0x0700>;
> +      clocks = <&sysc 0>;
> +      clock-names = "sys_clk";
>        #phy-cells = <1>;
>      };
> -- 
> 2.25.1
> 

Re: [PATCH 2/5] dt-bindings: phy: mediatek,mt7621-pci-phy: add clock entries

[Sergio Paracuellos]

Hi Rob,

On Sat, May 8, 2021 at 12:12 AM Rob Herring <robh@kernel.org> wrote:
>
> On Thu, May 06, 2021 at 01:15:28PM +0200, Sergio Paracuellos wrote:
> > MT7621 SoC clock driver has already mainlined in
> > 'commit 48df7a26f470 ("clk: ralink: add clock driver for mt7621 SoC")'
> > Hence update schema with the add of the entries related to
> > clock.
> >
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> > ---
> >  .../bindings/phy/mediatek,mt7621-pci-phy.yaml        | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.yaml b/Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.yaml
> > index 0ccaded3f245..d8614ef8995c 100644
> > --- a/Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.yaml
> > +++ b/Documentation/devicetree/bindings/phy/mediatek,mt7621-pci-phy.yaml
> > @@ -16,6 +16,14 @@ properties:
> >    reg:
> >      maxItems: 1
> >
> > +  clocks:
> > +    maxItems: 1
> > +    description:
> > +      PHY reference clock. Must contain an entry in clock-names.
> > +
> > +  clock-names:
> > +    const: sys_clk
>
> You don't really need -names when there is only 1.

Ok, I will drop this property then and
>
> > +
> >    "#phy-cells":
> >      const: 1
> >      description: selects if the phy is dual-ported
> > @@ -23,6 +31,8 @@ properties:
> >  required:
> >    - compatible
> >    - reg
> > +  - clocks
> > +  - clock-names
>
> Technically, you can't add new properties and make them required as that
> breaks the ABI. If that's okay here, explain it in the commit message.

So until now no clock driver existed and things were not properly
being done in driver code directly accessing registers to get the
clock frequency to properly configure the phy. Since the new clock
driver enters into the scene, make this mandatory force to update both
driver and dtb, which is something pretty normal when upgrading the
kind of devices using this SoC. So I think it should be finde to make
this a requirement.

>
> >    - "#phy-cells"
> >
> >  additionalProperties: false
> > @@ -32,5 +42,7 @@ examples:
> >      pcie0_phy: pcie-phy@1e149000 {
> >        compatible = "mediatek,mt7621-pci-phy";
> >        reg = <0x1e149000 0x0700>;
> > +      clocks = <&sysc 0>;
> > +      clock-names = "sys_clk";
> >        #phy-cells = <1>;
> >      };
> > --
> > 2.25.1
> >

Best regards,
    Sergio Paracuellos