netdev - Re: [EXTERNAL] Re: [PATCH v11 03/10] net: ti: icssg-prueth: Add Firmware config and classification APIs.

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: Tue, 25 Jul 2023 09:44:25 +0200
From: Simon Horman <simon.horman@...igine.com>
To: Md Danish Anwar <a0501179@...com>
Cc: MD Danish Anwar <danishanwar@...com>,
	Randy Dunlap <rdunlap@...radead.org>,
	Roger Quadros <rogerq@...nel.org>,
	Vignesh Raghavendra <vigneshr@...com>, Andrew Lunn <andrew@...n.ch>,
	Richard Cochran <richardcochran@...il.com>,
	Conor Dooley <conor+dt@...nel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
	Rob Herring <robh+dt@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Jakub Kicinski <kuba@...nel.org>,
	Eric Dumazet <edumazet@...gle.com>,
	"David S. Miller" <davem@...emloft.net>, nm@...com, srk@...com,
	linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
	netdev@...r.kernel.org, linux-omap@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [EXTERNAL] Re: [PATCH v11 03/10] net: ti: icssg-prueth: Add
 Firmware config and classification APIs.

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@...com>
> >> Reviewed-by: Andrew Lunn <andrew@...n.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.