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
| ||
|
Message-ID: <eeb48306-0564-46da-ae74-a9f779242399@lunn.ch> Date: Sat, 5 Jul 2025 10:23:23 +0200 From: Andrew Lunn <andrew@...n.ch> To: Tristram.Ha@...rochip.com Cc: Woojung Huh <woojung.huh@...rochip.com>, Vladimir Oltean <olteanv@...il.com>, Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Marek Vasut <marex@...x.de>, UNGLinuxDriver@...rochip.com, devicetree@...r.kernel.org, netdev@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH net-next 2/2] net: dsa: microchip: Add KSZ8463 switch support to KSZ DSA driver On Wed, Jul 02, 2025 at 07:01:55PM -0700, Tristram.Ha@...rochip.com wrote: > From: Tristram Ha <tristram.ha@...rochip.com> > > KSZ8463 switch is a 3-port switch based from KSZ8863. Its major > difference from other KSZ SPI switches is its register access is not a > simple continual 8-bit transfer with automatic address increase but uses > a byte-enable mechanism specifying 8-bit, 16-bit, or 32-bit access. Its > registers are also defined in 16-bit format because it shares a design > with a MAC controller using 16-bit access. As a result some common > register accesses need to be re-arranged. The 64-bit access used by > other switches needs to be broken into 2 32-bit accesses. This is a rather large patch, making it hard to review. Please could you break it up into smaller patches, with good commit messages. > static void ksz_cfg(struct ksz_device *dev, u32 addr, u8 bits, bool set) > { > - regmap_update_bits(ksz_regmap_8(dev), addr, bits, set ? bits : 0); > + regmap_update_bits(ksz_regmap_8(dev), reg8(dev, addr), bits, > + set ? bits : 0); > } This pattern of using reg8() appears a few times. That could be one patch. > > static void ksz_port_cfg(struct ksz_device *dev, int port, int offset, u8 bits, > bool set) > { > - regmap_update_bits(ksz_regmap_8(dev), PORT_CTRL_ADDR(port, offset), > + regmap_update_bits(ksz_regmap_8(dev), > + reg8(dev, dev->dev_ops->get_port_addr(port, offset)), > bits, set ? bits : 0); Adding this helper and using it could be a patch. > - if (ksz_is_ksz88x3(dev)) > + if (ksz_is_ksz88x3(dev) || ksz_is_ksz8463(dev)) > return -ENOTSUPP; All these ENOTSUPP could be a patch. etc. Andrew --- pw-bot: cr
Powered by blists - more mailing lists