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: <14219af7-5074-4aef-a26c-b68412451f5a@microchip.com> Date: Fri, 27 Oct 2023 07:00:57 +0000 From: <Parthiban.Veerasooran@...rochip.com> To: <andrew@...n.ch> CC: <davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com>, <robh+dt@...nel.org>, <krzysztof.kozlowski+dt@...aro.org>, <conor+dt@...nel.org>, <corbet@....net>, <Steen.Hegelund@...rochip.com>, <rdunlap@...radead.org>, <horms@...nel.org>, <casper.casan@...il.com>, <netdev@...r.kernel.org>, <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>, <linux-doc@...r.kernel.org>, <Horatiu.Vultur@...rochip.com>, <Woojung.Huh@...rochip.com>, <Nicolas.Ferre@...rochip.com>, <UNGLinuxDriver@...rochip.com>, <Thorsten.Kummermehr@...rochip.com> Subject: Re: [PATCH net-next v2 2/9] net: ethernet: oa_tc6: implement mac-phy software reset Hi Andrew, On 27/10/23 1:31 am, Andrew Lunn wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > >>>> + ret = oa_tc6_perform_ctrl(tc6, STATUS0, ®val, 1, false, false); >>>> + if (ret) >>>> + return ret; >>>> + >>>> + /* Check for reset complete interrupt status */ >>>> + if (regval & RESETC) { >>>> + regval = RESETC; >>> >>> People don't always agree, but i found STATUS0_RESETC easier to see >>> you have the correct bit for the register you just read. >> Do you want me to define STATUS0_RESETC instead of RESETC or is my >> understanding wrong? > > Correct, STATUS0_RESETC. It avoids silly typos like: > > ret = oa_tc6_perform_ctrl(tc6, STATUS0, ®val, 1, false, false); > if (ret) > return ret; > > /* Check for reset complete interrupt status */ > if (regval & RESET) { > regval = RESETC; > > where RESET is a valid register name, but not a bit. Or say: > > ret = oa_tc6_perform_ctrl(tc6, STATUS0, ®val, 1, false, false); > if (ret) > return ret; > > /* Check for reset complete interrupt status */ > if (regval & SWRESET) { > regval = STATUS0_; > > where SWRESET is a valid bit, but not for STATUS0. > > I've made silly mistakes like this, and learnt that good naming helps > to avoid it. Ok, thanks for the detailed explanation. Best Regards, Parthiban V > > Andrew
Powered by blists - more mailing lists