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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAK+owojiySqLBtHq-=OpotxR3_Z0uoZzukVrF9Ak=fiUHtPm5Q@mail.gmail.com>
Date: Mon, 19 May 2025 23:46:34 +0200
From: Stefano Radaelli <stefano.radaelli21@...il.com>
To: Simon Horman <horms@...nel.org>
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>, 
	Russell King <linux@...linux.org.uk>, "David S. Miller" <davem@...emloft.net>, 
	Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, 
	Xu Liang <lxu@...linear.com>
Subject: Re: [PATCH net-next v2] net: phy: add driver for MaxLinear MxL86110 PHY

Hi Simon,

Thank you for your feedback!
v4 will be ready soon.

Best Regards,

Stefano

Il giorno lun 19 mag 2025 alle ore 18:40 Simon Horman
<horms@...nel.org> ha scritto:
>
> 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@...il.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.
>
> ...