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