Re: [PATCH net-next v2 0/4] sfc: support unicast PTP

["Íñigo Huguet" <ihuguet@redhat.com>]

On Wed, Feb 1, 2023 at 8:05 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Wed,  1 Feb 2023 09:08:45 +0100 Íñigo Huguet wrote:
> > v2: fixed missing IS_ERR
> >     added doc of missing fields in efx_ptp_rxfilter
>
> 1. don't repost within 24h, *especially* if you're reposting
> because of compilation problems
>
> https://www.kernel.org/doc/html/next/process/maintainer-netdev.html

Sorry, I wasn't aware of this.

> 2. please don't repost in a thread, it makes it harder for me
> to maintain a review queue

What do you mean? I sent it with `git send-email --in-reply-to`, I
thought this was the canonical way to send a v2 and superseed the
previous version.

> 3. drop the pointless inline in the source file in patch 4
>
> +static inline void efx_ptp_remove_one_filter(struct efx_nic *efx,
> +                                            struct efx_ptp_rxfilter *rxfilter)

This is the second time I get pushback because of this. Could you
please explain the rationale of not allowing it?

I understand that the compiler probably will do the right thing with
or without the `inline`, and more being in the same translation unit.
Actually, I checked the style guide [1] and I thought it was fine like
this: it says that `inline` should not be abused, but it's fine in
cases like this one. Quotes from the guide:
  "Generally, inline functions are preferable to macros resembling functions"
  "A reasonable rule of thumb is to not put inline at functions that
have more than 3 lines of code in them"

I have the feeling that if I had made it as a macro it had been
accepted, but inline not, despite the "prefer inline over macro".

I don't mind changing it, but I'd like to understand it so I can
remember the next time. And if it's such a hard rule that it's
considered a "fail" in the patchwork checks, maybe it should be
documented somewhere.

Thanks!

[1] https://www.kernel.org/doc/html/latest/process/coding-style.html


-- 
Íñigo Huguet