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:   Tue, 25 Jul 2023 09:44:25 +0200
From:   Simon Horman <simon.horman@corigine.com>
To:     Md Danish Anwar <a0501179@ti.com>
Cc:     MD Danish Anwar <danishanwar@ti.com>,
        Randy Dunlap <rdunlap@infradead.org>,
        Roger Quadros <rogerq@kernel.org>,
        Vignesh Raghavendra <vigneshr@ti.com>,
        Andrew Lunn <andrew@lunn.ch>,
        Richard Cochran <richardcochran@gmail.com>,
        Conor Dooley <conor+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Eric Dumazet <edumazet@google.com>,
        "David S. Miller" <davem@davemloft.net>, nm@ti.com, srk@ti.com,
        linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
        netdev@vger.kernel.org, linux-omap@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org
Subject: Re: [EXTERNAL] Re: [PATCH v11 03/10] net: ti: icssg-prueth: Add
 Firmware config and classification APIs.
Message-ID: <ZL99WfF7iuzeMP78@corigine.com>
References: <20230724112934.2637802-1-danishanwar@ti.com>
 <20230724112934.2637802-4-danishanwar@ti.com>
 <ZL94/L1RMlU5TiAb@corigine.com>
 <b2016718-b8e4-a1f8-92ed-f0d9e3cb9c17@ti.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <b2016718-b8e4-a1f8-92ed-f0d9e3cb9c17@ti.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?6mw9eqSHXtY3QJybpgmghZ+FGYRL6hsYO0XGo3LAL2hyb6FFIl+SnrjAgENf?=
 =?us-ascii?Q?Pg6q1lkvwnej1Vr2qrBfhUhNUrv+pW7oXLLlnLSkcVGcM1kVtgNQ/JYAFCpx?=
 =?us-ascii?Q?tvesOkgIzxq88VwgSodUh8XFbLxj/N/ngXRZPexR37kgtWe0Fx2G9hEPCegf?=
 =?us-ascii?Q?fu/qHl7PbnpvsdyJLFsK/hhYvACYgmoiOU+VbDy7fq4AuaWmtR5gXtOoOOGW?=
 =?us-ascii?Q?oknOnoTmyyTiw33hEeYPMh3UpuCS0btCdPOS6TQJ7Uho76zLpm7HitHa7zl/?=
 =?us-ascii?Q?uABhoEIXqQe3STrVHuaLGi5rwj1iyxogE08VGYdeuqnoHIy2sZfcC9FGqhJo?=
 =?us-ascii?Q?6QZOEBHcYUG85fBbDgt+1zlgQTfBS2l/T1ed2Nq1qYwAMTe9dhOG4Jvyc64P?=
 =?us-ascii?Q?QBUwaLFBxBSlfGg8zffh0Q0eKDBfYCH24j2PmWl9u/+u2Aolrwe5TCO9Uzoo?=
 =?us-ascii?Q?LTnXaQMSyOwYb7KrPyBMl3lLF/sb5GdIDFAfra7mWpeSIB3cpRPoI+Ud5J5a?=
 =?us-ascii?Q?cAtUggvPDQC1us7/rxsjBv0TRR+XCMp2sDv1Okb/nX/Uh0o4jX+BE7DdaH9F?=
 =?us-ascii?Q?Ob9oThKsK7KEYgtibD/UVtvPNX4f0r/Q/D93XRAT+esFK4f5s2f2VFMARWuW?=
 =?us-ascii?Q?K0lLi+MdDg+Tbq07s7GA/yMBd3eR2fioFKX9C4XNeEGgtIQ4+06KhDa516aM?=
 =?us-ascii?Q?AGSced+WP03P+xsjSXLYdrIh/SPFLeT57XiUQXUZkGMat9/W7BtCQ8Lfiz7n?=
 =?us-ascii?Q?u4X6x01lfQRuPgfCPzdy3e62yDJr0XrvA6T/ld2R0kko5VHNccsyzeJhpIUp?=
 =?us-ascii?Q?yqjzGJ83FfsyIBJ3ZD4DJ/N2V6T01oJh1FSyoKcFVf6vajxOuTsmqWxZFJG+?=
 =?us-ascii?Q?b2pEtW7yUvx1Xm21TnJDHadjPpbLeDqWOqfPJrVX02IzCNIOgg3VeH6dBahx?=
 =?us-ascii?Q?0Kvzwx57VW6m70tZGLLchrXeCVRqYTuEEDDuYUI9vWBBs6g5moevLOqLWADM?=
 =?us-ascii?Q?Ps7RRK0Ylm2ze7R+Y2gHJCwKznXlaRWd2JZrkQmhNmjAux/KGqP06UaMKUoK?=
 =?us-ascii?Q?bE3ScNY96CSyzMcZlgcRFklvGXM20O63CGFH4GFkUfnrjWD3VGFKSZlhptX7?=
 =?us-ascii?Q?0YeeFTOTuzp29IG2iZkvBR+VLo/bl/Nz3vJOTOBcddgM5iIoJg/M0pvQ93+C?=
 =?us-ascii?Q?9amotxT7ofWBxv8fGLGBgt7RLet5M7pro9TLl4NyrDXc8jM1auCltMnctycq?=
 =?us-ascii?Q?n/UKx6vGIWVlvz9sPG7ycPijVxSgToEsEeQP1qcPEDZc3U2B8DftAshlOwsO?=
 =?us-ascii?Q?bRx823y77cSJnjUKbKhmfrMiS/3ZACbzq4t9nfOGSgXXKZBV3d5fSU4Z92pe?=
 =?us-ascii?Q?7Y3ChiOXpu1lGFhCPFu27iTdpSSA/GHYvgLeLQmZOX8pN7HixLfutHT3zDwX?=
 =?us-ascii?Q?GyS34EoP5Wk+DSvpj2G186B9teiFFC00XOYJPOuKDB9r+JysP1BuhD3per6m?=
 =?us-ascii?Q?BR8xp6kfZ+Sz/Kd6aagnZ/2vSm+F7mELGGlfcJ4P5nlX9TP5EgsNNC1HxEZ/?=
 =?us-ascii?Q?CP0gVrsIzsLmuVJG2FXmn7uII2SVg6JyFSxcReqFHGITnyZGjI6mdRauEcQv?=
 =?us-ascii?Q?4Z5+CchCgzblCoYC74uAuAYRXcgcN4GRYTvBlqawV0jE7pX65Qg0o8hh4I8m?=
 =?us-ascii?Q?QdQPkQ=3D=3D?=
X-OriginatorOrg: corigine.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4149bf13-8c6a-403a-e4bc-08db8ce2fd46
X-MS-Exchange-CrossTenant-AuthSource: BY3PR13MB4834.namprd13.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Jul 2023 07:44:34.4953
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: fe128f2c-073b-4c20-818e-7246a585940c
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: h7amhPFhVALn4BjnSqvfxFV2O/Cxpx3rsrDlfJ4mueoLCJzDNq5CFizjRjhyRQiN621buHkPbAsU7eGt6pTjiUewi3F6DCb7dKLTDTMO6f0=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR13MB3843
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,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 Tue, Jul 25, 2023 at 01:10:30PM +0530, Md Danish Anwar wrote:
> Hi Simon,
> 
> On 25/07/23 12:55 pm, Simon Horman wrote:
> > On Mon, Jul 24, 2023 at 04:59:27PM +0530, MD Danish Anwar wrote:
> >> Add icssg_config.h / .c and icssg_classifier.c files. These are firmware
> >> configuration and classification related files. These will be used by
> >> ICSSG ethernet driver.
> >>
> >> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
> >> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> > 
> > Hi Danish,
> > 
> > some feedback from my side.
> > 
> 
> Thanks for the feedback.
> 
> > ...
> > 
> >> diff --git a/drivers/net/ethernet/ti/icssg_classifier.c b/drivers/net/ethernet/ti/icssg_classifier.c
> > 
> > ...
> > 
> >> +void icssg_class_set_mac_addr(struct regmap *miig_rt, int slice, u8 *mac)
> > 
> > This function appears to be unused.
> > Perhaps it would be better placed in a later patch?
> > 
> > Or perhaps not, if it makes it hard to split up the patches nicely.
> > In which case, perhaps the __maybe_unused annotation could be added,
> > temporarily.
> > 
> 
> Due to splitting the patch into 8-9 patches, I had to introduce these helper
> APIs earlier. All these APIs are helper APIs, they will be used in patch 6
> (Introduce ICSSG Prueth driver).
> 
> I had this concern that some APIs which will be used later but introduced
> earlier can create some warnings, before splitting the patches.
> 
> I had raised this concern in [1] and asked Jakub if it would be OK to introduce
> these APIs earlier. Jakub said it would be fine [2], so I went ahead with this
> approach.
> 
> It will make very hard to break patches if these APIs are introduced and used
> in same patch.

Thanks, I understand.

In that case my suggestion is to, temporarily, add __maybe_unused,
which will allow static analysis tools to work more cleanly over the
series. It is just a suggestion, not a hard requirement.

Probably something along those lines applies to all the
review I provided in my previous email. Please use your discretion here.