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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 25 Oct 2023 11:16:16 +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 1/9] net: ethernet: implement OPEN Alliance
 control transaction interface

Hi Andrew,

Thanks for reviewing my patches.

On 24/10/23 2:58 am, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
>> +static void oa_tc6_prepare_ctrl_buf(struct oa_tc6 *tc6, u32 addr, u32 val[],
>> +                                 u8 len, bool wnr, u8 *buf, bool prote)
> 
> One of the comments i made last time was that wnr is not obvious. I
> assume it means write-not-read. So why not just write? Since it a
> boolean, i assume war is never needed, so !wnr cal always be
> considered rnw.
> 
> And prote could well be protect, the two extra characters make it a
> lot more obvious. Or better still.
Just to be aligned with the naming in the OPEN Alliance specification 
document, used the same name as it is. wnr and prote are taken from the 
specification document. Please refer the screenshots attached here from 
the specification document.

Still if you feel like using "write" instead of "wnr" and "protect" 
instead of "prote", I will change them in the next revision.
> 
>> +{
>> +     u32 hdr;
>> +
>> +     /* Prepare the control header with the required details */
>> +     hdr = FIELD_PREP(CTRL_HDR_DNC, 0) |
>> +           FIELD_PREP(CTRL_HDR_WNR, wnr) |
>> +           FIELD_PREP(CTRL_HDR_AID, 0) |
>> +           FIELD_PREP(CTRL_HDR_MMS, addr >> 16) |
>> +           FIELD_PREP(CTRL_HDR_ADDR, addr) |
>> +           FIELD_PREP(CTRL_HDR_LEN, len - 1);
>> +     hdr |= FIELD_PREP(CTRL_HDR_P, oa_tc6_get_parity(hdr));
>> +     *(__be32 *)buf = cpu_to_be32(hdr);
>> +
>> +     if (wnr) {
>> +             for (u8 i = 0; i < len; i++) {
>> +                     u16 pos;
>> +
>> +                     if (!prote) {
> 
> nitpick, but please use positive logic. Do the protected case first.
Sure, will do it in the next revision.

Best Regards,
Parthiban V
> 
>           Andrew
> 

Download attachment "wnr.png" of type "image/png" (108929 bytes)

Download attachment "prote.png" of type "image/png" (163542 bytes)

Powered by blists - more mailing lists