Re: [PATCH v7 net-next 2/8] sfc: add devlink info support for ef100

On Wed, Feb 15, 2023 at 08:43:21AM +0000, Lucero Palau, Alejandro wrote:

On 2/14/23 16:56, Alexander Lobakin wrote:

quoted

From: Edward Cree <ecree.xilinx@gmail.com>
Date: Tue, 14 Feb 2023 15:28:24 +0000

quoted

On 14/02/2023 07:39, Leon Romanovsky wrote:

quoted

On Mon, Feb 13, 2023 at 06:34:22PM +0000, alejandro.lucero-palau@amd.com wrote:

quoted

+#ifdef CONFIG_RTC_LIB
+	u64 tstamp;
+#endif

If you are going to resubmit the series.

Documentation/process/coding-style.rst
   1140 21) Conditional Compilation
   1141 ---------------------------
....
   1156 If you have a function or variable which may potentially go unused in a
   1157 particular configuration, and the compiler would warn about its definition
   1158 going unused, mark the definition as __maybe_unused rather than wrapping it in
   1159 a preprocessor conditional.  (However, if a function or variable *always* goes
   1160 unused, delete it.)

Thanks

FWIW, the existing code in sfc all uses the preprocessor
  conditional approach; maybe it's better to be consistent
  within the driver?

When it comes to "consistency vs start doing it right" thing, I always
go for the latter. This "we'll fix it all one day" moment often tends to
never happen and it's applicable to any vendor or subsys. Stop doing
things the discouraged way often is a good (and sometimes the only) start.

It is not clear to me what you prefer, if fixing this now or leaving it 
and fixing it later.

He asked to fix.

Thanks

The first sentence in your comment suggest the latter to me. The rest of 
the comment suggests the fix it now.

Anyway, patchwork says changes requested, so I'll send v8.

Thanks

quoted

Thanks,
Olek