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
| ||
|
Date: Wed, 13 Oct 2021 17:24:48 -0700 From: Jakub Kicinski <kuba@...nel.org> To: Vladimir Oltean <vladimir.oltean@....com> Cc: "David S . Miller" <davem@...emloft.net>, netdev@...r.kernel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, Rob Herring <robh+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>, Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>, Russell King <linux@...linux.org.uk>, Vivien Didelot <vivien.didelot@...il.com>, Vladimir Oltean <olteanv@...il.com>, Florian Fainelli <f.fainelli@...il.com>, Prasanna Vengateshan <prasanna.vengateshan@...rochip.com>, Ansuel Smith <ansuelsmth@...il.com>, Alvin Šipraga <alsi@...g-olufsen.dk> Subject: Re: [PATCH net-next 6/6] net: dsa: sja1105: parse {rx,tx}-internal-delay-ps properties for RGMII delays Some take it or leave it comments, checkpatch pointed out some extra brackets so I had a look at the patch. On Thu, 14 Oct 2021 01:23:13 +0300 Vladimir Oltean wrote: > + int rx_delay = -1, tx_delay = -1; > > + if (!phy_interface_mode_is_rgmii(phy_mode)) > + return 0; > > + of_property_read_u32(port_dn, "rx-internal-delay-ps", &rx_delay); > + of_property_read_u32(port_dn, "tx-internal-delay-ps", &tx_delay); If I'm reading this right you're depending on delays being left as -1 in case the property reads fail. Is this commonly accepted practice? Why not code it up as: u32 rx_delay; if (of_property_read_u32(...)) rx_delay = 0; else if (rx_delay != clamp(rx_delay, ...MIN, ...MAX) goto err; or some such? > + if ((rx_delay && rx_delay < SJA1105_RGMII_DELAY_MIN_PS) || > + (tx_delay && tx_delay < SJA1105_RGMII_DELAY_MIN_PS) || > + (rx_delay > SJA1105_RGMII_DELAY_MAX_PS) || > + (tx_delay > SJA1105_RGMII_DELAY_MAX_PS)) { nit: checkpatch says the brackets around the latter two are unnecessary, just in case it's not for symmetry / on purpose
Powered by blists - more mailing lists