[PATCH v2 0/5] net: phy: bcm63xx: Enable internal GPHY on BCM63268

[PATCH v2 0/5] net: phy: bcm63xx: Enable internal GPHY on BCM63268

[Kyle Hendry]

Some BCM63268 bootloaders do not enable the internal PHYs by default.
This patch series adds a phy driver to set the registers required 
for the gigabit PHY to work. 

Currently the PHY can't be detected until the b53 switch is initialized,
but this should be solvable through the device tree. I'm currently 
investigating whether the the PHY needs the whole switch to be set up
or just specific clocks, etc. 

v2 changes:
- Remove changes to b53 dsa code and rework fix as a PHY driver
- Use a regmap for accessing GPHY control register
- Add documentaion for device tree changes

v1: https://lore.kernel.org/netdev/20250206043055.177004-1-kylehendrydev@gmail.com/

Signed-off-by: Kyle Hendry <kylehendrydev@gmail.com>

Kyle Hendry (5):
  net: phy: bcm63xx: add support for BCM63268 GPHY
  net: phy: enable bcm63xx on bmips
  dt-bindings: net: bcm6368-mdio-mux: add gphy-ctrl property
  dt-bindings: mfd: brcm: add brcm,bcm63268-gphy-ctrl compatible
  dt-bindings: mfd: brcm: add gphy controller to BCM63268 sysctl

 .../mfd/brcm,bcm63268-gpio-sysctl.yaml        | 13 +++
 .../devicetree/bindings/mfd/syscon.yaml       |  2 +
 .../bindings/net/brcm,bcm6368-mdio-mux.yaml   |  7 ++
 drivers/net/phy/Kconfig                       |  4 +-
 drivers/net/phy/bcm63xx.c                     | 96 +++++++++++++++++++
 5 files changed, 120 insertions(+), 2 deletions(-)

-- 
2.43.0

[PATCH v2 1/5] net: phy: bcm63xx: add support for BCM63268 GPHY

[Kyle Hendry]

This patch adds support for the internal gigabit PHY on the
BCM63268 SoC. The PHY has a low power mode that has can be
enabled/disabled through the GPHY control register. The
register is passed in through the device tree, and the
relevant bits are set in the suspend and resume functions.

Signed-off-by: Kyle Hendry <kylehendrydev@gmail.com>
---
 drivers/net/phy/bcm63xx.c | 96 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 96 insertions(+)

diff --git a/drivers/net/phy/bcm63xx.c b/drivers/net/phy/bcm63xx.c
index b46a736a3130..613c3da315ac 100644
--- a/drivers/net/phy/bcm63xx.c
+++ b/drivers/net/phy/bcm63xx.c
@@ -3,8 +3,11 @@
  *	Driver for Broadcom 63xx SOCs integrated PHYs
  */
 #include "bcm-phy-lib.h"
+#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/phy.h>
+#include <linux/regmap.h>
+
 
 #define MII_BCM63XX_IR		0x1a	/* interrupt register */
 #define MII_BCM63XX_IR_EN	0x4000	/* global interrupt enable */
@@ -13,10 +16,19 @@
 #define MII_BCM63XX_IR_LINK	0x0200	/* link changed */
 #define MII_BCM63XX_IR_GMASK	0x0100	/* global interrupt mask */
 
+#define PHY_ID_BCM63268_GPHY	0x03625f50
+
+#define GPHY_CTRL_IDDQ_BIAS	BIT(0)
+#define GPHY_CTRL_LOW_PWR	BIT(3)
+
 MODULE_DESCRIPTION("Broadcom 63xx internal PHY driver");
 MODULE_AUTHOR("Maxime Bizon <mbizon@freebox.fr>");
 MODULE_LICENSE("GPL");
 
+struct bcm_gphy_priv {
+		struct regmap *gphy_ctrl;
+};
+
 static int bcm63xx_config_intr(struct phy_device *phydev)
 {
 	int reg, err;
@@ -69,6 +81,80 @@ static int bcm63xx_config_init(struct phy_device *phydev)
 	return phy_write(phydev, MII_BCM63XX_IR, reg);
 }
 
+int bcm63268_gphy_set(struct phy_device *phydev, bool enable)
+{
+	struct bcm_gphy_priv *priv = phydev->priv;
+	u32 pwr_bits;
+	int ret;
+
+	pwr_bits = GPHY_CTRL_IDDQ_BIAS | GPHY_CTRL_LOW_PWR;
+
+	if (enable)
+		ret = regmap_update_bits(priv->gphy_ctrl, 0, pwr_bits, 0);
+	else
+		ret = regmap_update_bits(priv->gphy_ctrl, 0, pwr_bits, pwr_bits);
+
+	return ret;
+}
+
+int bcm63268_gphy_resume(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = bcm63268_gphy_set(phydev, true);
+	if (ret)
+		return ret;
+
+	ret = genphy_resume(phydev);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+int bcm63268_gphy_suspend(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = genphy_suspend(phydev);
+	if (ret)
+		return ret;
+
+	ret = bcm63268_gphy_set(phydev, false);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int bcm63268_gphy_probe(struct phy_device *phydev)
+{
+	struct device_node *np = dev_of_node(&phydev->mdio.bus->dev);
+	struct mdio_device *mdio = &phydev->mdio;
+	struct device *dev = &mdio->dev;
+	struct bcm_gphy_priv *priv;
+	struct regmap *regmap;
+	int err;
+
+	err = devm_phy_package_join(dev, phydev, 0, 0);
+	if (err)
+		return err;
+
+	priv = devm_kzalloc(dev, sizeof(struct bcm_gphy_priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	phydev->priv = priv;
+
+	regmap = syscon_regmap_lookup_by_phandle(np, "brcm,gphy-ctrl");
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	priv->gphy_ctrl = regmap;
+
+	return 0;
+}
+
 static struct phy_driver bcm63xx_driver[] = {
 {
 	.phy_id		= 0x00406000,
@@ -89,6 +175,15 @@ static struct phy_driver bcm63xx_driver[] = {
 	.config_init	= bcm63xx_config_init,
 	.config_intr	= bcm63xx_config_intr,
 	.handle_interrupt = bcm_phy_handle_interrupt,
+}, {
+	.phy_id         = PHY_ID_BCM63268_GPHY,
+	.phy_id_mask    = 0xfffffff0,
+	.name           = "Broadcom BCM63268 GPHY",
+	/* PHY_BASIC_FEATURES */
+	.flags          = PHY_IS_INTERNAL,
+	.probe          = bcm63268_gphy_probe,
+	.resume			= bcm63268_gphy_resume,
+	.suspend		= bcm63268_gphy_suspend,
 } };
 
 module_phy_driver(bcm63xx_driver);
@@ -96,6 +191,7 @@ module_phy_driver(bcm63xx_driver);
 static const struct mdio_device_id __maybe_unused bcm63xx_tbl[] = {
 	{ 0x00406000, 0xfffffc00 },
 	{ 0x002bdc00, 0xfffffc00 },
+	{ PHY_ID_MATCH_EXACT(PHY_ID_BCM63268_GPHY) },
 	{ }
 };
 
-- 
2.43.0

Re: [PATCH v2 1/5] net: phy: bcm63xx: add support for BCM63268 GPHY

[Krzysztof Kozlowski]

On Mon, Feb 17, 2025 at 05:36:40PM -0800, Kyle Hendry wrote:
> This patch adds support for the internal gigabit PHY on the

Please do not use "This commit/patch/change", but imperative mood. See
longer explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

> BCM63268 SoC. The PHY has a low power mode that has can be
> enabled/disabled through the GPHY control register. The
> register is passed in through the device tree, and the
> relevant bits are set in the suspend and resume functions.
> 

...

> +int bcm63268_gphy_resume(struct phy_device *phydev)
> +{
> +	int ret;
> +
> +	ret = bcm63268_gphy_set(phydev, true);
> +	if (ret)
> +		return ret;
> +
> +	ret = genphy_resume(phydev);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +int bcm63268_gphy_suspend(struct phy_device *phydev)

Why these are not static? Where is EXPORT_SYMBOL and kerneldoc?

> +{
> +	int ret;
> +
> +	ret = genphy_suspend(phydev);
> +	if (ret)
> +		return ret;
> +
> +	ret = bcm63268_gphy_set(phydev, false);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int bcm63268_gphy_probe(struct phy_device *phydev)
> +{
> +	struct device_node *np = dev_of_node(&phydev->mdio.bus->dev);
> +	struct mdio_device *mdio = &phydev->mdio;
> +	struct device *dev = &mdio->dev;
> +	struct bcm_gphy_priv *priv;
> +	struct regmap *regmap;
> +	int err;
> +
> +	err = devm_phy_package_join(dev, phydev, 0, 0);
> +	if (err)
> +		return err;
> +
> +	priv = devm_kzalloc(dev, sizeof(struct bcm_gphy_priv), GFP_KERNEL);

sizeof(*)

> +	if (!priv)
> +		return -ENOMEM;
> +
> +	phydev->priv = priv;
> +
> +	regmap = syscon_regmap_lookup_by_phandle(np, "brcm,gphy-ctrl");

No. ABI break without any explanation in commit msg.

Best regards,
Krzysztof

Re: [PATCH v2 1/5] net: phy: bcm63xx: add support for BCM63268 GPHY

[Krzysztof Kozlowski]

On 18/02/2025 02:36, Kyle Hendry wrote:
> This patch adds support for the internal gigabit PHY on the
> BCM63268 SoC. The PHY has a low power mode that has can be
> enabled/disabled through the GPHY control register. The
> register is passed in through the device tree, and the
> relevant bits are set in the suspend and resume functions.
> 
> Signed-off-by: Kyle Hendry <kylehendrydev@gmail.com>
> ---
>  drivers/net/phy/bcm63xx.c | 96 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 96 insertions(+)


Also two more nits:

> 
> diff --git a/drivers/net/phy/bcm63xx.c b/drivers/net/phy/bcm63xx.c
> index b46a736a3130..613c3da315ac 100644
> --- a/drivers/net/phy/bcm63xx.c
> +++ b/drivers/net/phy/bcm63xx.c
> @@ -3,8 +3,11 @@
>   *	Driver for Broadcom 63xx SOCs integrated PHYs
>   */
>  #include "bcm-phy-lib.h"
> +#include <linux/mfd/syscon.h>
>  #include <linux/module.h>
>  #include <linux/phy.h>
> +#include <linux/regmap.h>
> +
>  


No need for new line.

>  #define MII_BCM63XX_IR		0x1a	/* interrupt register */
>  #define MII_BCM63XX_IR_EN	0x4000	/* global interrupt enable */
> @@ -13,10 +16,19 @@
>  #define MII_BCM63XX_IR_LINK	0x0200	/* link changed */
>  #define MII_BCM63XX_IR_GMASK	0x0100	/* global interrupt mask */
>  
> +#define PHY_ID_BCM63268_GPHY	0x03625f50
> +
> +#define GPHY_CTRL_IDDQ_BIAS	BIT(0)
> +#define GPHY_CTRL_LOW_PWR	BIT(3)
> +
>  MODULE_DESCRIPTION("Broadcom 63xx internal PHY driver");
>  MODULE_AUTHOR("Maxime Bizon <mbizon@freebox.fr>");
>  MODULE_LICENSE("GPL");
>  
> +struct bcm_gphy_priv {
> +		struct regmap *gphy_ctrl;


Messed indentation.

> +};
> +
>  static int bcm63xx_config_intr(struct phy_device *phydev)
>  {
>  	int reg, err;
> @@ -69,6 +81,80 @@ static int bcm63xx_config_init(struct phy_device *phydev)
>  	return phy_write(phydev, MII_BCM63XX_IR, reg);
>  }



Best regards,
Krzysztof

Re: [PATCH v2 1/5] net: phy: bcm63xx: add support for BCM63268 GPHY

[kernel test robot]

Hi Kyle,

kernel test robot noticed the following build warnings:

[auto build test WARNING on lee-mfd/for-mfd-next]
[also build test WARNING on robh/for-next lee-leds/for-leds-next linus/master lee-mfd/for-mfd-fixes v6.14-rc3 next-20250218]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Kyle-Hendry/net-phy-bcm63xx-add-support-for-BCM63268-GPHY/20250218-094117
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
patch link:    https://lore.kernel.org/r/20250218013653.229234-2-kylehendrydev%40gmail.com
patch subject: [PATCH v2 1/5] net: phy: bcm63xx: add support for BCM63268 GPHY
config: x86_64-buildonly-randconfig-004-20250219 (https://download.01.org/0day-ci/archive/20250219/202502191212.s0NqhQ3T-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250219/202502191212.s0NqhQ3T-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202502191212.s0NqhQ3T-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/net/phy/bcm63xx.c:84:5: warning: no previous prototype for function 'bcm63268_gphy_set' [-Wmissing-prototypes]
      84 | int bcm63268_gphy_set(struct phy_device *phydev, bool enable)
         |     ^
   drivers/net/phy/bcm63xx.c:84:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
      84 | int bcm63268_gphy_set(struct phy_device *phydev, bool enable)
         | ^
         | static 
>> drivers/net/phy/bcm63xx.c:100:5: warning: no previous prototype for function 'bcm63268_gphy_resume' [-Wmissing-prototypes]
     100 | int bcm63268_gphy_resume(struct phy_device *phydev)
         |     ^
   drivers/net/phy/bcm63xx.c:100:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
     100 | int bcm63268_gphy_resume(struct phy_device *phydev)
         | ^
         | static 
>> drivers/net/phy/bcm63xx.c:115:5: warning: no previous prototype for function 'bcm63268_gphy_suspend' [-Wmissing-prototypes]
     115 | int bcm63268_gphy_suspend(struct phy_device *phydev)
         |     ^
   drivers/net/phy/bcm63xx.c:115:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
     115 | int bcm63268_gphy_suspend(struct phy_device *phydev)
         | ^
         | static 
   3 warnings generated.


vim +/bcm63268_gphy_set +84 drivers/net/phy/bcm63xx.c

    83	
  > 84	int bcm63268_gphy_set(struct phy_device *phydev, bool enable)
    85	{
    86		struct bcm_gphy_priv *priv = phydev->priv;
    87		u32 pwr_bits;
    88		int ret;
    89	
    90		pwr_bits = GPHY_CTRL_IDDQ_BIAS | GPHY_CTRL_LOW_PWR;
    91	
    92		if (enable)
    93			ret = regmap_update_bits(priv->gphy_ctrl, 0, pwr_bits, 0);
    94		else
    95			ret = regmap_update_bits(priv->gphy_ctrl, 0, pwr_bits, pwr_bits);
    96	
    97		return ret;
    98	}
    99	
 > 100	int bcm63268_gphy_resume(struct phy_device *phydev)
   101	{
   102		int ret;
   103	
   104		ret = bcm63268_gphy_set(phydev, true);
   105		if (ret)
   106			return ret;
   107	
   108		ret = genphy_resume(phydev);
   109		if (ret)
   110			return ret;
   111	
   112		return 0;
   113	}
   114	
 > 115	int bcm63268_gphy_suspend(struct phy_device *phydev)
   116	{
   117		int ret;
   118	
   119		ret = genphy_suspend(phydev);
   120		if (ret)
   121			return ret;
   122	
   123		ret = bcm63268_gphy_set(phydev, false);
   124		if (ret)
   125			return ret;
   126	
   127		return 0;
   128	}
   129	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 1/5] net: phy: bcm63xx: add support for BCM63268 GPHY

[kernel test robot]

Hi Kyle,

kernel test robot noticed the following build warnings:

[auto build test WARNING on lee-mfd/for-mfd-next]
[also build test WARNING on robh/for-next lee-leds/for-leds-next linus/master lee-mfd/for-mfd-fixes v6.14-rc3 next-20250218]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Kyle-Hendry/net-phy-bcm63xx-add-support-for-BCM63268-GPHY/20250218-094117
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
patch link:    https://lore.kernel.org/r/20250218013653.229234-2-kylehendrydev%40gmail.com
patch subject: [PATCH v2 1/5] net: phy: bcm63xx: add support for BCM63268 GPHY
config: loongarch-randconfig-001-20250219 (https://download.01.org/0day-ci/archive/20250219/202502191246.zER47JXl-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250219/202502191246.zER47JXl-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202502191246.zER47JXl-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/net/phy/bcm63xx.c:84:5: warning: no previous prototype for 'bcm63268_gphy_set' [-Wmissing-prototypes]
      84 | int bcm63268_gphy_set(struct phy_device *phydev, bool enable)
         |     ^~~~~~~~~~~~~~~~~
>> drivers/net/phy/bcm63xx.c:100:5: warning: no previous prototype for 'bcm63268_gphy_resume' [-Wmissing-prototypes]
     100 | int bcm63268_gphy_resume(struct phy_device *phydev)
         |     ^~~~~~~~~~~~~~~~~~~~
>> drivers/net/phy/bcm63xx.c:115:5: warning: no previous prototype for 'bcm63268_gphy_suspend' [-Wmissing-prototypes]
     115 | int bcm63268_gphy_suspend(struct phy_device *phydev)
         |     ^~~~~~~~~~~~~~~~~~~~~


vim +/bcm63268_gphy_set +84 drivers/net/phy/bcm63xx.c

    83	
  > 84	int bcm63268_gphy_set(struct phy_device *phydev, bool enable)
    85	{
    86		struct bcm_gphy_priv *priv = phydev->priv;
    87		u32 pwr_bits;
    88		int ret;
    89	
    90		pwr_bits = GPHY_CTRL_IDDQ_BIAS | GPHY_CTRL_LOW_PWR;
    91	
    92		if (enable)
    93			ret = regmap_update_bits(priv->gphy_ctrl, 0, pwr_bits, 0);
    94		else
    95			ret = regmap_update_bits(priv->gphy_ctrl, 0, pwr_bits, pwr_bits);
    96	
    97		return ret;
    98	}
    99	
 > 100	int bcm63268_gphy_resume(struct phy_device *phydev)
   101	{
   102		int ret;
   103	
   104		ret = bcm63268_gphy_set(phydev, true);
   105		if (ret)
   106			return ret;
   107	
   108		ret = genphy_resume(phydev);
   109		if (ret)
   110			return ret;
   111	
   112		return 0;
   113	}
   114	
 > 115	int bcm63268_gphy_suspend(struct phy_device *phydev)
   116	{
   117		int ret;
   118	
   119		ret = genphy_suspend(phydev);
   120		if (ret)
   121			return ret;
   122	
   123		ret = bcm63268_gphy_set(phydev, false);
   124		if (ret)
   125			return ret;
   126	
   127		return 0;
   128	}
   129	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v2 2/5] net: phy: enable bcm63xx on bmips

[Kyle Hendry]

Allow the bcm63xx PHY driver to be built on bmips machines

Signed-off-by: Kyle Hendry <kylehendrydev@gmail.com>
---
 drivers/net/phy/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 41c15a2c2037..0f2956ba472d 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -156,10 +156,10 @@ config BCM54140_PHY
 
 config BCM63XX_PHY
 	tristate "Broadcom 63xx SOCs internal PHY"
-	depends on BCM63XX || COMPILE_TEST
+	depends on BCM63XX || BMIPS_GENERIC || COMPILE_TEST
 	select BCM_NET_PHYLIB
 	help
-	  Currently supports the 6348 and 6358 PHYs.
+	  Currently supports the 6348, 6358 and 63268 PHYs.
 
 config BCM7XXX_PHY
 	tristate "Broadcom 7xxx SOCs internal PHYs"
-- 
2.43.0

Re: [PATCH v2 2/5] net: phy: enable bcm63xx on bmips

[Florian Fainelli]

On 2/17/2025 5:36 PM, Kyle Hendry wrote:
> Allow the bcm63xx PHY driver to be built on bmips machines
> 
> Signed-off-by: Kyle Hendry <kylehendrydev@gmail.com>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

[PATCH v2 3/5] dt-bindings: net: bcm6368-mdio-mux: add gphy-ctrl property

[Kyle Hendry]

Add documentation for the brcm,gphy-ctrl phandle to the
register for controlling ghpy modes.

Signed-off-by: Kyle Hendry <kylehendrydev@gmail.com>
---
 .../devicetree/bindings/net/brcm,bcm6368-mdio-mux.yaml     | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/brcm,bcm6368-mdio-mux.yaml b/Documentation/devicetree/bindings/net/brcm,bcm6368-mdio-mux.yaml
index 9ef28c2a0afc..9630b87b0473 100644
--- a/Documentation/devicetree/bindings/net/brcm,bcm6368-mdio-mux.yaml
+++ b/Documentation/devicetree/bindings/net/brcm,bcm6368-mdio-mux.yaml
@@ -24,6 +24,11 @@ properties:
   reg:
     maxItems: 1
 
+properties:
+  brcm,gphy-ctrl:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description: A phandle to the gphy control register
+
 required:
   - compatible
   - reg
@@ -42,6 +47,8 @@ examples:
         #address-cells = <1>;
         #size-cells = <0>;
         reg = <0>;
+
+        brcm,gphy-ctrl = <&gphy_ctrl>;
       };
 
       mdio_ext: mdio@1 {
-- 
2.43.0

Re: [PATCH v2 3/5] dt-bindings: net: bcm6368-mdio-mux: add gphy-ctrl property

[Krzysztof Kozlowski]

On Mon, Feb 17, 2025 at 05:36:42PM -0800, Kyle Hendry wrote:
> Add documentation for the brcm,gphy-ctrl phandle to the
> register for controlling ghpy modes.
> 
> Signed-off-by: Kyle Hendry <kylehendrydev@gmail.com>
> ---
>  .../devicetree/bindings/net/brcm,bcm6368-mdio-mux.yaml     | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/brcm,bcm6368-mdio-mux.yaml b/Documentation/devicetree/bindings/net/brcm,bcm6368-mdio-mux.yaml
> index 9ef28c2a0afc..9630b87b0473 100644
> --- a/Documentation/devicetree/bindings/net/brcm,bcm6368-mdio-mux.yaml
> +++ b/Documentation/devicetree/bindings/net/brcm,bcm6368-mdio-mux.yaml
> @@ -24,6 +24,11 @@ properties:
>    reg:
>      maxItems: 1
>  

Looks like whitespace error

> +properties:
> +  brcm,gphy-ctrl:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description: A phandle to the gphy control register

Missing items. Why is this array?

Anyway, explain the purpose of this phandle - how it is used by the
*device*.

Best regards,
Krzysztof

[PATCH v2 4/5] dt-bindings: mfd: brcm: add brcm,bcm63268-gphy-ctrl compatible

[Kyle Hendry]

Add BCM63268 GPHY control register compatible.

Signed-off-by: Kyle Hendry <kylehendrydev@gmail.com>
---
 Documentation/devicetree/bindings/mfd/syscon.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/syscon.yaml b/Documentation/devicetree/bindings/mfd/syscon.yaml
index 4d67ff26d445..1d4c66014340 100644
--- a/Documentation/devicetree/bindings/mfd/syscon.yaml
+++ b/Documentation/devicetree/bindings/mfd/syscon.yaml
@@ -54,6 +54,7 @@ select:
           - atmel,sama5d4-sfrbu
           - axis,artpec6-syscon
           - brcm,cru-clkset
+          - brcm,bcm63268-gphy-ctrl
           - brcm,sr-cdru
           - brcm,sr-mhb
           - cirrus,ep7209-syscon1
@@ -153,6 +154,7 @@ properties:
           - atmel,sama5d4-sfrbu
           - axis,artpec6-syscon
           - brcm,cru-clkset
+          - brcm,bcm63268-gphy-ctrl
           - brcm,sr-cdru
           - brcm,sr-mhb
           - cirrus,ep7209-syscon1
-- 
2.43.0

Re: [PATCH v2 4/5] dt-bindings: mfd: brcm: add brcm,bcm63268-gphy-ctrl compatible

[Krzysztof Kozlowski]

On Mon, Feb 17, 2025 at 05:36:43PM -0800, Kyle Hendry wrote:
> Add BCM63268 GPHY control register compatible.
> 
> Signed-off-by: Kyle Hendry <kylehendrydev@gmail.com>
> ---
>  Documentation/devicetree/bindings/mfd/syscon.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/syscon.yaml b/Documentation/devicetree/bindings/mfd/syscon.yaml
> index 4d67ff26d445..1d4c66014340 100644
> --- a/Documentation/devicetree/bindings/mfd/syscon.yaml
> +++ b/Documentation/devicetree/bindings/mfd/syscon.yaml
> @@ -54,6 +54,7 @@ select:
>            - atmel,sama5d4-sfrbu
>            - axis,artpec6-syscon
>            - brcm,cru-clkset
> +          - brcm,bcm63268-gphy-ctrl

Why random order? No, keep exiting, alphabetical order.

Best regards,
Krzysztof

[PATCH v2 5/5] dt-bindings: mfd: brcm: add gphy controller to BCM63268 sysctl

[Kyle Hendry]

Add documentation for BCM63268 gphy controller in the
bcm63268-gpio-sysctl register range.

Signed-off-by: Kyle Hendry <kylehendrydev@gmail.com>
---
 .../bindings/mfd/brcm,bcm63268-gpio-sysctl.yaml     | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/brcm,bcm63268-gpio-sysctl.yaml b/Documentation/devicetree/bindings/mfd/brcm,bcm63268-gpio-sysctl.yaml
index 9c2a04829da5..99610a5f2912 100644
--- a/Documentation/devicetree/bindings/mfd/brcm,bcm63268-gpio-sysctl.yaml
+++ b/Documentation/devicetree/bindings/mfd/brcm,bcm63268-gpio-sysctl.yaml
@@ -50,6 +50,15 @@ patternProperties:
       should follow the bindings specified in
       Documentation/devicetree/bindings/pinctrl/brcm,bcm63268-pinctrl.yaml.
 
+  "^gphy_ctrl@[0-9a-f]+$":
+    # Child node
+    type: object
+    $ref: /schemas/mfd/syscon.yaml
+    description:
+      Control register for GPHY modes. This child node definition
+      should follow the bindings specified in
+      Documentation/devicetree/bindings/mfd/syscon.yaml
+
 required:
   - "#address-cells"
   - compatible
@@ -191,4 +200,8 @@ examples:
           pins = "dsl_gpio9";
         };
       };
+      gphy_ctrl: gphy_ctrl@54 {
+      compatible = "brcm,bcm63268-gphy-ctrl", "syscon";
+      reg = <0x54 0x4>;
+      };
     };
-- 
2.43.0

Re: [PATCH v2 5/5] dt-bindings: mfd: brcm: add gphy controller to BCM63268 sysctl

[Krzysztof Kozlowski]

On Mon, Feb 17, 2025 at 05:36:44PM -0800, Kyle Hendry wrote:
> Add documentation for BCM63268 gphy controller in the
> bcm63268-gpio-sysctl register range.
> 
> Signed-off-by: Kyle Hendry <kylehendrydev@gmail.com>
> ---
>  .../bindings/mfd/brcm,bcm63268-gpio-sysctl.yaml     | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/brcm,bcm63268-gpio-sysctl.yaml b/Documentation/devicetree/bindings/mfd/brcm,bcm63268-gpio-sysctl.yaml
> index 9c2a04829da5..99610a5f2912 100644
> --- a/Documentation/devicetree/bindings/mfd/brcm,bcm63268-gpio-sysctl.yaml
> +++ b/Documentation/devicetree/bindings/mfd/brcm,bcm63268-gpio-sysctl.yaml
> @@ -50,6 +50,15 @@ patternProperties:
>        should follow the bindings specified in
>        Documentation/devicetree/bindings/pinctrl/brcm,bcm63268-pinctrl.yaml.
>  
> +  "^gphy_ctrl@[0-9a-f]+$":

Read DTS coding style.

> +    # Child node
> +    type: object
> +    $ref: /schemas/mfd/syscon.yaml

No, not really... how is syscon a child of other syscon? Isn't the other
device the syscon?

This looks really fake hardware description, like recent bootlin claim that
"one register in syscon is device".

> +    description:
> +      Control register for GPHY modes. This child node definition
> +      should follow the bindings specified in
> +      Documentation/devicetree/bindings/mfd/syscon.yaml
> +
>  required:
>    - "#address-cells"
>    - compatible
> @@ -191,4 +200,8 @@ examples:
>            pins = "dsl_gpio9";
>          };
>        };
> +      gphy_ctrl: gphy_ctrl@54 {
> +      compatible = "brcm,bcm63268-gphy-ctrl", "syscon";

Messed indentation.

Best regards,
Krzysztof

Re: [PATCH v2 5/5] dt-bindings: mfd: brcm: add gphy controller to BCM63268 sysctl

[Kyle Hendry]

On 2025-02-17 23:35, Krzysztof Kozlowski wrote:
>> +    # Child node
>> +    type: object
>> +    $ref: /schemas/mfd/syscon.yaml
> No, not really... how is syscon a child of other syscon? Isn't the other
> device the syscon?
>
> This looks really fake hardware description, like recent bootlin claim that
> "one register in syscon is device".

I will change the driver to access the register through the main syscon.

Best regards,
Kyle

Re: [PATCH v2 0/5] net: phy: bcm63xx: Enable internal GPHY on BCM63268

[Florian Fainelli]

On 2/17/2025 5:36 PM, Kyle Hendry wrote:
> Some BCM63268 bootloaders do not enable the internal PHYs by default.
> This patch series adds a phy driver to set the registers required
> for the gigabit PHY to work.
> 
> Currently the PHY can't be detected until the b53 switch is initialized,
> but this should be solvable through the device tree. I'm currently
> investigating whether the the PHY needs the whole switch to be set up
> or just specific clocks, etc.
> 
> v2 changes:
> - Remove changes to b53 dsa code and rework fix as a PHY driver
> - Use a regmap for accessing GPHY control register
> - Add documentaion for device tree changes

I really preferred v1 to v2 which conveyed the special intent better 
than going through layers and layers of abstraction here with limited 
re-usability.

At least with v2, the logic to toggle the IDDQ enable/disable remains 
within the PHY driver which is a better location.
-- 
Florian

Re: [PATCH v2 0/5] net: phy: bcm63xx: Enable internal GPHY on BCM63268

[Kyle Hendry]

On 2025-02-21 12:09, Florian Fainelli wrote:
>
>
> On 2/17/2025 5:36 PM, Kyle Hendry wrote:
>> Some BCM63268 bootloaders do not enable the internal PHYs by default.
>> This patch series adds a phy driver to set the registers required
>> for the gigabit PHY to work.
>>
>> Currently the PHY can't be detected until the b53 switch is initialized,
>> but this should be solvable through the device tree. I'm currently
>> investigating whether the the PHY needs the whole switch to be set up
>> or just specific clocks, etc.
>>
>> v2 changes:
>> - Remove changes to b53 dsa code and rework fix as a PHY driver
>> - Use a regmap for accessing GPHY control register
>> - Add documentaion for device tree changes
>
> I really preferred v1 to v2 which conveyed the special intent better 
> than going through layers and layers of abstraction here with limited 
> re-usability.
>
> At least with v2, the logic to toggle the IDDQ enable/disable remains 
> within the PHY driver which is a better location.


The next version should be much more simplified. I'm going to move
the syscon phandle to the actual phy node, so I think the device tree
documentation is going to need a new schema file for the phy. Who
should I list as the maintainer for the new binding?

Best regards,
Kyle