Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210412222040.GA13659@focaruja> <YHW5Fwl55c7NJAeo@orome.fritz.box>
In-Reply-To: <YHW5Fwl55c7NJAeo@orome.fritz.box>
From:   Dwaipayan Ray <dwaipayanray1@gmail.com>
Date:   Tue, 13 Apr 2021 21:35:16 +0530
Message-ID: <CABJPP5AEtg+615Q2d-d9aZf=CE7EFPKEbc7tr=gKo_H-XL+M5Q@mail.gmail.com>
Subject: Re: [PATCH] staging: media: tegra-vde: Align line break to match with
 the open parenthesis in file trace.h
To:     Thierry Reding <thierry.reding@gmail.com>
Cc:     Aline Santana Cordeiro <alinesantanacordeiro@gmail.com>,
        Andy Whitcroft <apw@canonical.com>,
        Joe Perches <joe@perches.com>,
        Lukas Bulwahn <lukas.bulwahn@gmail.com>,
        Dmitry Osipenko <digetx@gmail.com>,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Jonathan Hunter <jonathanh@nvidia.com>,
        linux-media@vger.kernel.org, linux-staging@lists.linux.dev,
        linux-kernel <linux-kernel@vger.kernel.org>,
        outreachy-kernel@googlegroups.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Apr 13, 2021 at 8:59 PM Thierry Reding <thierry.reding@gmail.com> wrote:
>
> On Mon, Apr 12, 2021 at 07:20:40PM -0300, Aline Santana Cordeiro wrote:
> > Align line break to match with the open parenthesis.
> > Issue detected by checkpatch.pl.
> > It consequently solved a few end lines with a '(',
> > issue also detected by checkpatch.pl
> >
> > Signed-off-by: Aline Santana Cordeiro <alinesantanacordeiro@gmail.com>
> > ---
> >  drivers/staging/media/tegra-vde/trace.h | 111 ++++++++++++++------------------
> >  1 file changed, 50 insertions(+), 61 deletions(-)
>
> Ugh... I'd say this is one case where checkpatch.pl really shouldn't be
> complaining since this isn't a function call or declaration. It's more
> like a code snippet written with macros, so I don't think the same rules
> should apply.
>
> Adding checkpatch folks (hence quoting in full): is there anything we
> can do about this without too much effort? Or should we just accept this
> the way it is?
>

While it may be possible to add exceptions for trace headers on the
alignment rules, I don't know how many more such exceptions we will
end up adding. Such fine grained checks might be unnecessarily complex.
So I think we should just accept the way it is for now.

Joe might have a different opinion?

Thanks,
Dwaipayan.

> Thierry
>
> > diff --git a/drivers/staging/media/tegra-vde/trace.h b/drivers/staging/media/tegra-vde/trace.h
> > index e571410..1fcc573 100644
> > --- a/drivers/staging/media/tegra-vde/trace.h
> > +++ b/drivers/staging/media/tegra-vde/trace.h
> > @@ -11,79 +11,68 @@
> >  #include "vde.h"
> >
> >  DECLARE_EVENT_CLASS(register_access,
> > -     TP_PROTO(struct tegra_vde *vde, void __iomem *base,
> > -              u32 offset, u32 value),
> > -     TP_ARGS(vde, base, offset, value),
> > -     TP_STRUCT__entry(
> > -             __string(hw_name, tegra_vde_reg_base_name(vde, base))
> > -             __field(u32, offset)
> > -             __field(u32, value)
> > -     ),
> > -     TP_fast_assign(
> > -             __assign_str(hw_name, tegra_vde_reg_base_name(vde, base));
> > -             __entry->offset = offset;
> > -             __entry->value = value;
> > -     ),
> > -     TP_printk("%s:0x%03x 0x%08x", __get_str(hw_name), __entry->offset,
> > -               __entry->value)
> > +                 TP_PROTO(struct tegra_vde *vde, void __iomem *base,
> > +                          u32 offset, u32 value),
> > +                 TP_ARGS(vde, base, offset, value),
> > +                         TP_STRUCT__entry(__string(hw_name, tegra_vde_reg_base_name(vde, base))
> > +                                          __field(u32, offset)
> > +                                          __field(u32, value)
> > +                         ),
> > +                         TP_fast_assign(__assign_str(hw_name,
> > +                                                     tegra_vde_reg_base_name(vde, base));
> > +                                        __entry->offset = offset;
> > +                                        __entry->value = value;
> > +                         ),
> > +                         TP_printk("%s:0x%03x 0x%08x", __get_str(hw_name), __entry->offset,
> > +                                   __entry->value)
> >  );
> >