net: phy: bcm63xx: Enable internal GPHY on BCM63268

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

Posted by Kyle Hendry 9 months, 4 weeks ago

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

Posted by kernel test robot 9 months, 3 weeks ago

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

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

Posted by kernel test robot 9 months, 3 weeks ago

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

Posted by Krzysztof Kozlowski 9 months, 4 weeks ago

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

Posted by Krzysztof Kozlowski 9 months, 4 weeks ago

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

[PATCH v2 1/5] net: phy: bcm63xx: add support for BCM63268 GPHY
[PATCH v2 2/5] net: phy: enable bcm63xx on bmips
[PATCH v2 3/5] dt-bindings: net: bcm6368-mdio-mux: add gphy-ctrl property
[PATCH v2 4/5] dt-bindings: mfd: brcm: add brcm,bcm63268-gphy-ctrl compatible
[PATCH v2 5/5] dt-bindings: mfd: brcm: add gphy controller to BCM63268 sysctl