From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from pidgin.makrotopia.org (pidgin.makrotopia.org [185.142.180.65]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 689471FD4; Sun, 8 Feb 2026 00:06:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.142.180.65 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770509218; cv=none; b=Nek9jznZ4TqlBjVVNedrfLDQ1ycr6o2J6KzGNPZscvwkwUFYtMcIY6uhnbFIo9jgH4jWmNccR0E6UwIn9kRCi5x+soWOGn9btsrWXGmvkOdX4MoRLa/2B44CI6WqDIU23iZrZTqRuNK4/6ajhkX5glaVv56zcG2pgEz21tpbtww= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770509218; c=relaxed/simple; bh=HEuRv9yTtRl6sHMC2RKh4CDTkHLLVecluzq6wjSj2sQ=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=QhlgmrSawdjauQnWRnC15ycNxcfenC8iRyToPls4ruD6i4ZOA+p5HMje1EQO8MhHMcD+Y7quIlF+q4Qm87j2sP6Q/AFom76dZuOQOL+p3EGqvkiwx1TqiCZKqOebj08UGD81ShqK7Kx2YKy+SU0Dmb7gIDuccvROrmk/Zh8iWGU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=makrotopia.org; spf=pass smtp.mailfrom=makrotopia.org; arc=none smtp.client-ip=185.142.180.65 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=makrotopia.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=makrotopia.org Received: from local by pidgin.makrotopia.org with esmtpsa (TLS1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.99) (envelope-from ) id 1vosK4-000000007Cx-2JQD; Sun, 08 Feb 2026 00:06:44 +0000 Date: Sun, 8 Feb 2026 00:06:40 +0000 From: Daniel Golle To: Vladimir Oltean Cc: Andrew Lunn , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Heiner Kallweit , Russell King , Simon Horman , netdev@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Frank Wunderlich , Chad Monroe , Cezary Wilmanski , Liang Xu , John Crispin Subject: Re: [PATCH net-next v14 4/4] net: dsa: add basic initial driver for MxL862xx switches Message-ID: References: <20260207215902.mtsg43zeoadqqfz5@skbuf> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260207215902.mtsg43zeoadqqfz5@skbuf> On Sat, Feb 07, 2026 at 11:59:02PM +0200, Vladimir Oltean wrote: > On Sat, Feb 07, 2026 at 03:07:27AM +0000, Daniel Golle wrote: > > +/* PHY access via firmware relay */ > > +static int mxl862xx_phy_read_mmd(struct mxl862xx_priv *priv, int port, > > + int devadd, int reg) > > +{ > > + struct mdio_relay_data param = { > > + .phy = port, > > + .mmd = devadd, > > + .reg = cpu_to_le16(reg), > > + }; > > + int ret; > > + > > + ret = MXL862XX_API_READ(priv, INT_GPHY_READ, param); > > + if (ret) > > + return ret; > > + > > + return le16_to_cpu(param.data); > > +} > > + > > +static int mxl862xx_phy_write_mmd(struct mxl862xx_priv *priv, int port, > > + int devadd, int reg, u16 data) > > +{ > > + struct mdio_relay_data param = { > > + .phy = port, > > + .mmd = devadd, > > + .reg = cpu_to_le16(reg), > > + .data = cpu_to_le16(data), > > + }; > > + > > + return MXL862XX_API_WRITE(priv, INT_GPHY_WRITE, param); > > +} > > + > > +static int mxl862xx_phy_read_mii_bus(struct mii_bus *bus, int port, int regnum) > > +{ > > + return mxl862xx_phy_read_mmd(bus->priv, port, 0, regnum); > > +} > > + > > +static int mxl862xx_phy_write_mii_bus(struct mii_bus *bus, int port, > > + int regnum, u16 val) > > +{ > > + return mxl862xx_phy_write_mmd(bus->priv, port, 0, regnum, val); > > +} > > + > > +static int mxl862xx_phy_read_c45_mii_bus(struct mii_bus *bus, int port, > > + int devadd, int regnum) > > +{ > > + return mxl862xx_phy_read_mmd(bus->priv, port, devadd, regnum); > > +} > > + > > +static int mxl862xx_phy_write_c45_mii_bus(struct mii_bus *bus, int port, > > + int devadd, int regnum, u16 val) > > +{ > > + return mxl862xx_phy_write_mmd(bus->priv, port, devadd, regnum, val); > > +} > > You took inspiration from the wrong place with the mii_bus ops prototypes, > specifically with the "int port" argument. > > The second argument does not hold the port, it holds the PHY address. > I.e. in this case: > port@6 { > reg = <6>; > phy-handle = <&phy5>; > phy-mode = "internal"; > }; > phy5: ethernet-phy@5 { > reg = <5>; > }; > > "int port" is 5, not 6. > > Your source of inspiration are the prototypes of an mii_bus used as > ds->user_mii_bus. We have a different set of requirements there, because > ds->user_mii_bus exists for the case where the PHY is not described in > the device tree, so the port index is given as argument and the > user_mii_bus is responsible for internally translating the port index to > a PHY address. > > So while the use of "int port" as argument name for these operations is > justifiable in some cases, it is not applicable to this driver, and will > be a pitfall for anyone who has to modify or debug this code. Ack. While not completely correct from the beginning I should have addressed that and changed the parameter to 'int addr' when I started to count physical ports from 0 and no longer hide the microcontroller -- from that moment on the PHY address and the port address are no longer equal. I will post a follow-up series to address this and also the removal of the 'label' property from the DT binding example once this has been merged. Both cases don't justify a "Fixes:"-tag though, but I'll just try to be fast, so both can still be applied before net-next closes.