Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 20 Apr 2023 20:03:54 +0300
From:   Vladimir Oltean <vladimir.oltean@nxp.com>
To:     Simon Horman <simon.horman@corigine.com>
Cc:     netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
        Michal Kubecek <mkubecek@suse.cz>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Claudiu Manoil <claudiu.manoil@nxp.com>,
        Xiaoliang Yang <xiaoliang.yang_1@nxp.com>,
        Petr Machata <petrm@nvidia.com>,
        Danielle Ratson <danieller@nvidia.com>,
        Pranavi Somisetty <pranavi.somisetty@amd.com>,
        Harini Katakam <harini.katakam@amd.com>,
        Vinicius Costa Gomes <vinicius.gomes@intel.com>,
        Kurt Kanzenbach <kurt@linutronix.de>,
        Gerhard Engleder <gerhard@engleder-embedded.com>,
        Ferenc Fejes <ferenc.fejes@ericsson.com>,
        Aaron Conole <aconole@redhat.com>,
        linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 net-next 1/9] net: enetc: fix MAC Merge layer
 remaining enabled until a link down event
Message-ID: <20230420170354.n76b53ws6bitcoj2@skbuf>
References: <20230418111459.811553-1-vladimir.oltean@nxp.com>
 <20230418111459.811553-2-vladimir.oltean@nxp.com>
 <ZEFKjPR/VL6llxDm@corigine.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZEFKjPR/VL6llxDm@corigine.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?gEVGuW7+/YDYAjgDaCz0DyHqo56umaSLgfqrIWqg8k9LeK1TLpwcQ2PFEcxr?=
 =?us-ascii?Q?ngmHhsrNsxN1VP1MJoGSW5xSOhqDlwgFarWaaGsEEm87xVp3AXyzFjEK9KGe?=
 =?us-ascii?Q?EeD9LxKvDnzujVlbRSj4UwZvYcFb9Ak7iIoCxQdQDyas7WZf9pf+iKUI+Ddt?=
 =?us-ascii?Q?WLqFykHFwynwnotEkO+DL0Pj3vdOwxOaXiGG0vRmQCOA/+WGLfe4h1aj69v/?=
 =?us-ascii?Q?EeKQfOKUy2q9qunrv8/d1w3OIWw0ivLiXLoqtqR7zf+Qg/S6+hvkBG4Ruz2v?=
 =?us-ascii?Q?8ztibRff4P5zYPJ2YI8wxaLq6E2BY3B3XKZO+f3b1Pvi4StA5tB7sXSPVUus?=
 =?us-ascii?Q?GS4QN3jH4g6zVLvZwi+UVBk68U36vi1jNEAGXD9ccJeKUlbadDwq0pFSC/xF?=
 =?us-ascii?Q?XZAmA7IHAJDu5yGtIivR2FVqb4n+2PbA3VoGZNgXsTlW7R33H2RhlUHMCw0J?=
 =?us-ascii?Q?hRjFImWWWX4AYdCXjtw6bT/CO6mTVb8N/pgt9rdinf0tuBhipp8J+fZkX7J1?=
 =?us-ascii?Q?3KeEoP1mumKh+wQahnCOaR3lKNz20FFbdopmUzxL5+g6DgepiTsuTnpdpEKM?=
 =?us-ascii?Q?n9brlU39hvvqhENFuGLjYy62F4StHso9EpD1o8Dj2xLz8RYOuIS6lTHbsdqo?=
 =?us-ascii?Q?U1Qg5sz1x1glaIzf+0P6R5IyEpx2Kw8A3BNPumlThcptjDA0Rz4HRd4oSoqN?=
 =?us-ascii?Q?6jtkSwI9S8lAGPNRGyVlcvn0HYRFKOGvrAZOTwlqvO2LQQSto8SvF8dAJ+Hz?=
 =?us-ascii?Q?5K0QHzsQgVTHDiFetTDyksCGqnxH93ESTmJGqWQV/7IylINs5Mawb+I0FXh1?=
 =?us-ascii?Q?VPaCgsrr81cKQfKun3+CV1xrGYY3QNNNf2HsOSRjXq51SU1Rr2PCSjdfhF/G?=
 =?us-ascii?Q?SX3Y+FkN22pOZR8860a5ZAun3chEYIvIGcJkWQeFJkUYTnvQ1Ond7mh2v5jd?=
 =?us-ascii?Q?5JclZrF6NH+KkQ7uqh3umoggnAwfZdDv7nHVVsJBdYePwOF+Y3Cxr/C9D8mp?=
 =?us-ascii?Q?L4uL69YxcHa9IkKK/Kz7JZdcwDdhsvEU4goBlddqgPOgD1Pwm9oL7vn7+7MB?=
 =?us-ascii?Q?SdkRGMk7asBjfh9I6l7UbDW2epnA0P/u0Q6APcLT0W6wFoyAKaS0tcmlOLDH?=
 =?us-ascii?Q?uzaIqM/didumAp4DNQFkjKgV3uCvu9Tr81EZjx/vSMy30Mheb3fE1GmNeD1P?=
 =?us-ascii?Q?L+IxwTVvGmn0DxvO7UQTdbHPYwTxDgKXXzpszwMlTwRAUGV4mD5X328ehvqc?=
 =?us-ascii?Q?Hi7VjBqudiTpc73r1+S9KhhGsl3EcvImKNWLVf9Q4WlqokogkyPrcnjooo32?=
 =?us-ascii?Q?6EfQg2HvRNpj9YUshZrCmiIVHNsntNl7l/iH8zY1GO2V1axnLgQTAqFVsJpv?=
 =?us-ascii?Q?8/UaR/fMtfdfh+Zt8i4T8hUwOys3IFnmanlUYsbzcZDkDBur7XPsenLwrCTE?=
 =?us-ascii?Q?ApjYvFbp1X+h5ad8Uc2TKssds0ByZS3omTFfRH3zDSmeARixt5CfnaO6g2FB?=
 =?us-ascii?Q?CkCQ2HrOpodQWiXBLLDguntIjZgSYsQ67NEqRSwR7DKfZIk5+ByvNMIiaQxc?=
 =?us-ascii?Q?A9b07bvhtQH0QT0cmAWNG+fCNFfoE1AcftAq0MjIpWhsjcvA+YjcaM3c8ZjM?=
 =?us-ascii?Q?Eg=3D=3D?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: f17fcae5-91a7-4aa0-1da5-08db41c13b2e
X-MS-Exchange-CrossTenant-AuthSource: AM0PR04MB6452.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Apr 2023 17:03:58.1858
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: nVAA/sqaBzzxmu7qYnsCskFjIE4DoCAdxxc7Ft8snQ24WOcBA04qecnDJcumHlSyhEJxVhTMcF0mSSPXGNRrDg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PAXPR04MB8557
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, Apr 20, 2023 at 04:22:04PM +0200, Simon Horman wrote:
> > -	/* If link is up, enable MAC Merge right away */
> > -	if (!!(priv->active_offloads & ENETC_F_QBU) &&
> > -	    !(val & ENETC_MMCSR_LINK_FAIL))
> > -		val |= ENETC_MMCSR_ME;
> > +	/* If link is up, enable/disable MAC Merge right away */
> > +	if (!(val & ENETC_MMCSR_LINK_FAIL)) {
> > +		if (!!(priv->active_offloads & ENETC_F_QBU))
> 
> nit: The !!() seems unnecessary,
>      I wonder if it can be written in a simpler way as:
> 
> 		if (priv->active_offloads & ENETC_F_QBU)

I agree. Normally I omit the double negation in simple statements like this.
Here I didn't, because the expression was split into 2 "if" conditions,
and I kept the individual terms as-is for some reason.

Since the generated object code is absolutely the same either way, I would not
resend just for minor style comments such as this one, if you don't mind.
However, I do appreciate the review and I'll pay more attention to this
detail in the future.