From: Alexander Lobakin <aleksander.lobakin@intel.com>
To: "Lucero Palau, Alejandro" <alejandro.lucero-palau@amd.com>
Cc: Leon Romanovsky <leon@kernel.org>,
Edward Cree <ecree.xilinx@gmail.com>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"linux-net-drivers (AMD-Xilinx)" <linux-net-drivers@amd.com>,
"davem@davemloft.net" <davem@davemloft.net>,
"kuba@kernel.org" <kuba@kernel.org>,
"pabeni@redhat.com" <pabeni@redhat.com>,
"edumazet@google.com" <edumazet@google.com>,
"habetsm.xilinx@gmail.com" <habetsm.xilinx@gmail.com>,
"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
"corbet@lwn.net" <corbet@lwn.net>,
"jiri@nvidia.com" <jiri@nvidia.com>
Subject: Re: [PATCH v7 net-next 2/8] sfc: add devlink info support for ef100
Date: Wed, 15 Feb 2023 18:07:01 +0100 [thread overview]
Message-ID: <cf18fcc9-d56d-bfe0-d993-128ef27c60e5@intel.com> (raw)
In-Reply-To: <Y+zOeGYK0EctinF1@unreal>
From: Leon Romanovsky <leon@kernel.org>
Date: Wed, 15 Feb 2023 14:22:16 +0200
> On Wed, Feb 15, 2023 at 08:43:21AM +0000, Lucero Palau, Alejandro wrote:
>>
>> On 2/14/23 16:56, Alexander Lobakin wrote:
>>> From: Edward Cree <ecree.xilinx@gmail.com>
>>> Date: Tue, 14 Feb 2023 15:28:24 +0000
>>>
>>>> On 14/02/2023 07:39, Leon Romanovsky wrote:
>>>>> On Mon, Feb 13, 2023 at 06:34:22PM +0000, alejandro.lucero-palau@amd.com wrote:
>>>>>> +#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.
Correct. What I meant is that I always prefer to send stuff already done
right and not continue adding more todo-stuff to the kernel just because
there are tons of similar todo-stuff in the kernel already :D
>
> 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
>>
>>> Thanks,
>>> Olek
Thanks,
Olek
next prev parent reply other threads:[~2023-02-15 17:11 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-13 18:34 [PATCH v7 net-next 0/8] sfc: devlink support for ef100 alejandro.lucero-palau
2023-02-13 18:34 ` [PATCH v7 net-next 1/8] sfc: add " alejandro.lucero-palau
2023-02-13 18:34 ` [PATCH v7 net-next 2/8] sfc: add devlink info " alejandro.lucero-palau
2023-02-14 7:39 ` Leon Romanovsky
2023-02-14 15:28 ` Edward Cree
2023-02-14 16:56 ` Alexander Lobakin
2023-02-15 8:43 ` Lucero Palau, Alejandro
2023-02-15 12:22 ` Leon Romanovsky
2023-02-15 17:07 ` Alexander Lobakin [this message]
2023-02-14 8:51 ` Martin Habets
2023-02-13 18:34 ` [PATCH v7 net-next 3/8] sfc: enumerate mports in ef100 alejandro.lucero-palau
2023-02-13 18:34 ` [PATCH v7 net-next 4/8] sfc: add mport lookup based on driver's mport data alejandro.lucero-palau
2023-02-13 18:34 ` [PATCH v7 net-next 5/8] sfc: add devlink port support for ef100 alejandro.lucero-palau
2023-02-13 18:34 ` [PATCH v7 net-next 6/8] sfc: obtain device mac address based on firmware handle " alejandro.lucero-palau
2023-02-13 18:34 ` [PATCH v7 net-next 7/8] sfc: add support for devlink port_function_hw_addr_get in ef100 alejandro.lucero-palau
2023-02-13 18:34 ` [PATCH v7 net-next 8/8] sfc: add support for devlink port_function_hw_addr_set " alejandro.lucero-palau
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cf18fcc9-d56d-bfe0-d993-128ef27c60e5@intel.com \
--to=aleksander.lobakin@intel.com \
--cc=alejandro.lucero-palau@amd.com \
--cc=corbet@lwn.net \
--cc=davem@davemloft.net \
--cc=ecree.xilinx@gmail.com \
--cc=edumazet@google.com \
--cc=habetsm.xilinx@gmail.com \
--cc=jiri@nvidia.com \
--cc=kuba@kernel.org \
--cc=leon@kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-net-drivers@amd.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).