Re: [PATCH net-next v2] net: phy: add driver for MaxLinear MxL86110 PHY

[Simon Horman <horms@kernel.org>]

On Fri, May 16, 2025 at 06:41:23PM +0200, Stefano Radaelli wrote:
> Add support for the MaxLinear MxL86110 Gigabit Ethernet PHY, a low-power,
> cost-optimized transceiver supporting 10/100/1000 Mbps over twisted-pair
> copper, compliant with IEEE 802.3.
> 
> The driver implements basic features such as:
> - Device initialization
> - RGMII interface timing configuration
> - Wake-on-LAN support
> - LED initialization and control via /sys/class/leds
> 
> This driver has been tested on multiple Variscite boards, including:
> - VAR-SOM-MX93 (i.MX93)
> - VAR-SOM-MX8M-PLUS (i.MX8MP)
> 
> Example boot log showing driver probe:
> [    7.692101] imx-dwmac 428a0000.ethernet eth0:
>         PHY [stmmac-0:00] driver [MXL86110 Gigabit Ethernet] (irq=POLL)
> 
> Changes from v1:
> - Add net-next support
> - Improved locking management and tests using CONFIG_PROVE_LOCKING
> - General cleanup
> 
> Started a new thread
> 
> Signed-off-by: Stefano Radaelli <stefano.radaelli21@gmail.com>

Hi Stefano,

Some minor feedback from my side.

...

> diff --git a/drivers/net/phy/mxl-86110.c b/drivers/net/phy/mxl-86110.c
> new file mode 100644
> index 000000000000..63f32c49fcc1
> --- /dev/null
> +++ b/drivers/net/phy/mxl-86110.c
> @@ -0,0 +1,570 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * PHY driver for Maxlinear MXL86110
> + *
> + * Copyright 2023 MaxLinear Inc.
> + *
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/etherdevice.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/phy.h>
> +
> +/* PHY ID */
> +#define PHY_ID_MXL86110		0xc1335580
> +
> +/* required to access extended registers */
> +#define MXL86110_EXTD_REG_ADDR_OFFSET	0x1E
> +#define MXL86110_EXTD_REG_ADDR_DATA		0x1F
> +#define PHY_IRQ_ENABLE_REG				0x12
> +#define PHY_IRQ_ENABLE_REG_WOL			BIT(6)
> +
> +/* SyncE Configuration Register - COM_EXT SYNCE_CFG */
> +#define MXL86110_EXT_SYNCE_CFG_REG						0xA012

For Networking code, please restrict lines to no more than 80 columns
wide where you can do so without reducing readability (I'd say that is the
case here.

Likewise elsewhere in this patch.

checkpatch.pl --max-line-length=80 can be helpful here.

...

> +/**
> + * mxl86110_write_extended_reg() - write to a PHY's extended register
> + * @phydev: pointer to a &struct phy_device
> + * @regnum: register number to write
> + * @val: value to write to @regnum
> + *
> + * NOTE: This function assumes the caller already holds the MDIO bus lock
> + * or otherwise has exclusive access to the PHY.
> + *
> + * returns 0 or negative error code
> + */

Tooling expects 'Return:' or 'Returns: ' to document return values.

Likewise elsewhere in this patch.

Flagged by ./scripts/kernel-doc -Wall -none

...

> +static int mxl86110_led_hw_control_get(struct phy_device *phydev, u8 index,
> +				       unsigned long *rules)
> +{
> +	u16 val;
> +
> +	if (index >= MXL86110_MAX_LEDS)
> +		return -EINVAL;
> +
> +	phy_lock_mdio_bus(phydev);
> +	val = mxl86110_read_extended_reg(phydev, MXL86110_LED0_CFG_REG + index);
> +	phy_unlock_mdio_bus(phydev);
> +	if (val < 0)
> +		return val;

val is unsigned. It cannot be less than zero.

Likewise in mxl86110_broadcast_cfg() and mxl86110_enable_led_activity_blink().

Flagged by Smatch.

...