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

[Krzysztof Kozlowski <krzk@kernel.org>]

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