On Tue, Jul 25, 2023 at 01:28:21PM +0530, Md Danish Anwar wrote: > On 25/07/23 1:14 pm, Simon Horman wrote: > > 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. > > For now I think I will leave it as it is. Let reviewers review all other > patches. Let's see if there are any other comments on all the patches in this > series. If there are any more comments on other patches, then while re-spinning > next revision I will keep this in mind and try to add __maybe_unused tags in > all APIs that are used later. Sure, that sounds reasonable. > The idea behind splitting the patches was to get them reviewed individually as > it is quite difficult to get one big patch reviewed as explained by Jakub. And > these warnings were expected. If there are any other comments on this series, I > will try to address all of them together in next revision. Yes, I understand. Thanks for splitting things up into multiple patches. I know that is a lot of work. But it is very helpful. > Meanwhile, Please let me know if you have any comments on other patches > in this series. Will do, but I nothing to add at this time.
