* [PATCH] staging: media: tegra-vde: Align line break to match with the open parenthesis in file trace.h
@ 2021-04-12 22:20 Aline Santana Cordeiro
2021-04-13 15:30 ` Thierry Reding
2021-04-13 15:38 ` Hans Verkuil
0 siblings, 2 replies; 8+ messages in thread
From: Aline Santana Cordeiro @ 2021-04-12 22:20 UTC (permalink / raw)
To: Dmitry Osipenko, Mauro Carvalho Chehab, Greg Kroah-Hartman,
Thierry Reding, Jonathan Hunter
Cc: linux-media, linux-staging, linux-kernel, outreachy-kernel
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(-)
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)
);
DEFINE_EVENT(register_access, vde_writel,
- TP_PROTO(struct tegra_vde *vde, void __iomem *base,
- u32 offset, u32 value),
- TP_ARGS(vde, base, offset, value));
+ TP_PROTO(struct tegra_vde *vde, void __iomem *base,
+ u32 offset, u32 value),
+ TP_ARGS(vde, base, offset, value));
DEFINE_EVENT(register_access, vde_readl,
- TP_PROTO(struct tegra_vde *vde, void __iomem *base,
- u32 offset, u32 value),
- TP_ARGS(vde, base, offset, value));
+ TP_PROTO(struct tegra_vde *vde, void __iomem *base,
+ u32 offset, u32 value),
+ TP_ARGS(vde, base, offset, value));
TRACE_EVENT(vde_setup_iram_entry,
- TP_PROTO(unsigned int table, unsigned int row, u32 value, u32 aux_addr),
- TP_ARGS(table, row, value, aux_addr),
- TP_STRUCT__entry(
- __field(unsigned int, table)
- __field(unsigned int, row)
- __field(u32, value)
- __field(u32, aux_addr)
- ),
- TP_fast_assign(
- __entry->table = table;
- __entry->row = row;
- __entry->value = value;
- __entry->aux_addr = aux_addr;
- ),
- TP_printk("[%u][%u] = { 0x%08x (flags = \"%s\", frame_num = %u); 0x%08x }",
- __entry->table, __entry->row, __entry->value,
- __print_flags(__entry->value, " ", { (1 << 25), "B" }),
- __entry->value & 0x7FFFFF, __entry->aux_addr)
+ TP_PROTO(unsigned int table, unsigned int row, u32 value, u32 aux_addr),
+ TP_ARGS(table, row, value, aux_addr),
+ TP_STRUCT__entry(__field(unsigned int, table)
+ __field(unsigned int, row)
+ __field(u32, value)
+ __field(u32, aux_addr)
+ ),
+ TP_fast_assign(__entry->table = table;
+ __entry->row = row;
+ __entry->value = value;
+ __entry->aux_addr = aux_addr;
+ ),
+ TP_printk("[%u][%u] = { 0x%08x (flags = \"%s\", frame_num = %u); 0x%08x }",
+ __entry->table, __entry->row, __entry->value,
+ __print_flags(__entry->value, " ", { (1 << 25), "B" }),
+ __entry->value & 0x7FFFFF, __entry->aux_addr)
);
TRACE_EVENT(vde_ref_l0,
- TP_PROTO(unsigned int frame_num),
- TP_ARGS(frame_num),
- TP_STRUCT__entry(
- __field(unsigned int, frame_num)
- ),
- TP_fast_assign(
- __entry->frame_num = frame_num;
- ),
- TP_printk("REF L0: DPB: Frame 0: frame_num = %u", __entry->frame_num)
+ TP_PROTO(unsigned int frame_num),
+ TP_ARGS(frame_num),
+ TP_STRUCT__entry(__field(unsigned int, frame_num)),
+ TP_fast_assign(__entry->frame_num = frame_num;),
+ TP_printk("REF L0: DPB: Frame 0: frame_num = %u", __entry->frame_num)
);
TRACE_EVENT(vde_ref_l1,
- TP_PROTO(unsigned int with_later_poc_nb,
- unsigned int with_earlier_poc_nb),
- TP_ARGS(with_later_poc_nb, with_earlier_poc_nb),
- TP_STRUCT__entry(
- __field(unsigned int, with_later_poc_nb)
- __field(unsigned int, with_earlier_poc_nb)
- ),
- TP_fast_assign(
- __entry->with_later_poc_nb = with_later_poc_nb;
- __entry->with_earlier_poc_nb = with_earlier_poc_nb;
- ),
- TP_printk("REF L1: with_later_poc_nb %u, with_earlier_poc_nb %u",
- __entry->with_later_poc_nb, __entry->with_earlier_poc_nb)
+ TP_PROTO(unsigned int with_later_poc_nb,
+ unsigned int with_earlier_poc_nb),
+ TP_ARGS(with_later_poc_nb, with_earlier_poc_nb),
+ TP_STRUCT__entry(__field(unsigned int, with_later_poc_nb)
+ __field(unsigned int, with_earlier_poc_nb)),
+ TP_fast_assign(__entry->with_later_poc_nb = with_later_poc_nb;
+ __entry->with_earlier_poc_nb = with_earlier_poc_nb;),
+ TP_printk("REF L1: with_later_poc_nb %u, with_earlier_poc_nb %u",
+ __entry->with_later_poc_nb, __entry->with_earlier_poc_nb)
);
#endif /* TEGRA_VDE_TRACE_H */
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] staging: media: tegra-vde: Align line break to match with the open parenthesis in file trace.h
2021-04-12 22:20 [PATCH] staging: media: tegra-vde: Align line break to match with the open parenthesis in file trace.h Aline Santana Cordeiro
@ 2021-04-13 15:30 ` Thierry Reding
2021-04-13 16:05 ` Dwaipayan Ray
2021-04-13 15:38 ` Hans Verkuil
1 sibling, 1 reply; 8+ messages in thread
From: Thierry Reding @ 2021-04-13 15:30 UTC (permalink / raw)
To: Aline Santana Cordeiro, Andy Whitcroft, Joe Perches,
Dwaipayan Ray, Lukas Bulwahn
Cc: Dmitry Osipenko, Mauro Carvalho Chehab, Greg Kroah-Hartman,
Jonathan Hunter, linux-media, linux-staging, linux-kernel,
outreachy-kernel
[-- Attachment #1: Type: text/plain, Size: 6261 bytes --]
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?
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)
> );
>
> DEFINE_EVENT(register_access, vde_writel,
> - TP_PROTO(struct tegra_vde *vde, void __iomem *base,
> - u32 offset, u32 value),
> - TP_ARGS(vde, base, offset, value));
> + TP_PROTO(struct tegra_vde *vde, void __iomem *base,
> + u32 offset, u32 value),
> + TP_ARGS(vde, base, offset, value));
> DEFINE_EVENT(register_access, vde_readl,
> - TP_PROTO(struct tegra_vde *vde, void __iomem *base,
> - u32 offset, u32 value),
> - TP_ARGS(vde, base, offset, value));
> + TP_PROTO(struct tegra_vde *vde, void __iomem *base,
> + u32 offset, u32 value),
> + TP_ARGS(vde, base, offset, value));
>
> TRACE_EVENT(vde_setup_iram_entry,
> - TP_PROTO(unsigned int table, unsigned int row, u32 value, u32 aux_addr),
> - TP_ARGS(table, row, value, aux_addr),
> - TP_STRUCT__entry(
> - __field(unsigned int, table)
> - __field(unsigned int, row)
> - __field(u32, value)
> - __field(u32, aux_addr)
> - ),
> - TP_fast_assign(
> - __entry->table = table;
> - __entry->row = row;
> - __entry->value = value;
> - __entry->aux_addr = aux_addr;
> - ),
> - TP_printk("[%u][%u] = { 0x%08x (flags = \"%s\", frame_num = %u); 0x%08x }",
> - __entry->table, __entry->row, __entry->value,
> - __print_flags(__entry->value, " ", { (1 << 25), "B" }),
> - __entry->value & 0x7FFFFF, __entry->aux_addr)
> + TP_PROTO(unsigned int table, unsigned int row, u32 value, u32 aux_addr),
> + TP_ARGS(table, row, value, aux_addr),
> + TP_STRUCT__entry(__field(unsigned int, table)
> + __field(unsigned int, row)
> + __field(u32, value)
> + __field(u32, aux_addr)
> + ),
> + TP_fast_assign(__entry->table = table;
> + __entry->row = row;
> + __entry->value = value;
> + __entry->aux_addr = aux_addr;
> + ),
> + TP_printk("[%u][%u] = { 0x%08x (flags = \"%s\", frame_num = %u); 0x%08x }",
> + __entry->table, __entry->row, __entry->value,
> + __print_flags(__entry->value, " ", { (1 << 25), "B" }),
> + __entry->value & 0x7FFFFF, __entry->aux_addr)
> );
>
> TRACE_EVENT(vde_ref_l0,
> - TP_PROTO(unsigned int frame_num),
> - TP_ARGS(frame_num),
> - TP_STRUCT__entry(
> - __field(unsigned int, frame_num)
> - ),
> - TP_fast_assign(
> - __entry->frame_num = frame_num;
> - ),
> - TP_printk("REF L0: DPB: Frame 0: frame_num = %u", __entry->frame_num)
> + TP_PROTO(unsigned int frame_num),
> + TP_ARGS(frame_num),
> + TP_STRUCT__entry(__field(unsigned int, frame_num)),
> + TP_fast_assign(__entry->frame_num = frame_num;),
> + TP_printk("REF L0: DPB: Frame 0: frame_num = %u", __entry->frame_num)
> );
>
> TRACE_EVENT(vde_ref_l1,
> - TP_PROTO(unsigned int with_later_poc_nb,
> - unsigned int with_earlier_poc_nb),
> - TP_ARGS(with_later_poc_nb, with_earlier_poc_nb),
> - TP_STRUCT__entry(
> - __field(unsigned int, with_later_poc_nb)
> - __field(unsigned int, with_earlier_poc_nb)
> - ),
> - TP_fast_assign(
> - __entry->with_later_poc_nb = with_later_poc_nb;
> - __entry->with_earlier_poc_nb = with_earlier_poc_nb;
> - ),
> - TP_printk("REF L1: with_later_poc_nb %u, with_earlier_poc_nb %u",
> - __entry->with_later_poc_nb, __entry->with_earlier_poc_nb)
> + TP_PROTO(unsigned int with_later_poc_nb,
> + unsigned int with_earlier_poc_nb),
> + TP_ARGS(with_later_poc_nb, with_earlier_poc_nb),
> + TP_STRUCT__entry(__field(unsigned int, with_later_poc_nb)
> + __field(unsigned int, with_earlier_poc_nb)),
> + TP_fast_assign(__entry->with_later_poc_nb = with_later_poc_nb;
> + __entry->with_earlier_poc_nb = with_earlier_poc_nb;),
> + TP_printk("REF L1: with_later_poc_nb %u, with_earlier_poc_nb %u",
> + __entry->with_later_poc_nb, __entry->with_earlier_poc_nb)
> );
>
> #endif /* TEGRA_VDE_TRACE_H */
> --
> 2.7.4
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] staging: media: tegra-vde: Align line break to match with the open parenthesis in file trace.h
2021-04-13 15:30 ` Thierry Reding
@ 2021-04-13 16:05 ` Dwaipayan Ray
0 siblings, 0 replies; 8+ messages in thread
From: Dwaipayan Ray @ 2021-04-13 16:05 UTC (permalink / raw)
To: Thierry Reding
Cc: Aline Santana Cordeiro, Andy Whitcroft, Joe Perches,
Lukas Bulwahn, Dmitry Osipenko, Mauro Carvalho Chehab,
Greg Kroah-Hartman, Jonathan Hunter, linux-media, linux-staging,
linux-kernel, outreachy-kernel
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)
> > );
> >
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] staging: media: tegra-vde: Align line break to match with the open parenthesis in file trace.h
@ 2021-04-13 16:05 ` Dwaipayan Ray
0 siblings, 0 replies; 8+ messages in thread
From: Dwaipayan Ray @ 2021-04-13 16:05 UTC (permalink / raw)
To: Thierry Reding
Cc: Aline Santana Cordeiro, Andy Whitcroft, Joe Perches,
Lukas Bulwahn, Dmitry Osipenko, Mauro Carvalho Chehab,
Greg Kroah-Hartman, Jonathan Hunter, linux-media, linux-staging,
linux-kernel, outreachy-kernel
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)
> > );
> >
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] staging: media: tegra-vde: Align line break to match with the open parenthesis in file trace.h
2021-04-13 16:05 ` Dwaipayan Ray
(?)
@ 2021-04-13 17:43 ` Joe Perches
-1 siblings, 0 replies; 8+ messages in thread
From: Joe Perches @ 2021-04-13 17:43 UTC (permalink / raw)
To: Dwaipayan Ray, Thierry Reding
Cc: Aline Santana Cordeiro, Andy Whitcroft, Lukas Bulwahn,
Dmitry Osipenko, Mauro Carvalho Chehab, Greg Kroah-Hartman,
Jonathan Hunter, linux-media, linux-staging, linux-kernel,
outreachy-kernel
On Tue, 2021-04-13 at 21:35 +0530, Dwaipayan Ray wrote:
> 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?
Tracing uses a different style.
Maybe just suppress various messages for complete code blocks
of DECLARE_EVENT_CLASS, DEFINE_EVENT, and TRACE_EVENT
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] staging: media: tegra-vde: Align line break to match with the open parenthesis in file trace.h
@ 2021-04-13 17:43 ` Joe Perches
0 siblings, 0 replies; 8+ messages in thread
From: Joe Perches @ 2021-04-13 17:43 UTC (permalink / raw)
To: Dwaipayan Ray, Thierry Reding
Cc: Aline Santana Cordeiro, Andy Whitcroft, Lukas Bulwahn,
Dmitry Osipenko, Mauro Carvalho Chehab, Greg Kroah-Hartman,
Jonathan Hunter, linux-media, linux-staging, linux-kernel,
outreachy-kernel
On Tue, 2021-04-13 at 21:35 +0530, Dwaipayan Ray wrote:
> 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?
Tracing uses a different style.
Maybe just suppress various messages for complete code blocks
of DECLARE_EVENT_CLASS, DEFINE_EVENT, and TRACE_EVENT
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] staging: media: tegra-vde: Align line break to match with the open parenthesis in file trace.h
@ 2021-04-13 17:43 ` Joe Perches
0 siblings, 0 replies; 8+ messages in thread
From: Joe Perches @ 2021-04-13 17:43 UTC (permalink / raw)
To: Dwaipayan Ray, Thierry Reding
Cc: Aline Santana Cordeiro, Andy Whitcroft, Lukas Bulwahn,
Dmitry Osipenko, Mauro Carvalho Chehab, Greg Kroah-Hartman,
Jonathan Hunter, linux-media, linux-staging, linux-kernel,
outreachy-kernel
On Tue, 2021-04-13 at 21:35 +0530, Dwaipayan Ray wrote:
> 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?
Tracing uses a different style.
Maybe just suppress various messages for complete code blocks
of DECLARE_EVENT_CLASS, DEFINE_EVENT, and TRACE_EVENT
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] staging: media: tegra-vde: Align line break to match with the open parenthesis in file trace.h
2021-04-12 22:20 [PATCH] staging: media: tegra-vde: Align line break to match with the open parenthesis in file trace.h Aline Santana Cordeiro
2021-04-13 15:30 ` Thierry Reding
@ 2021-04-13 15:38 ` Hans Verkuil
1 sibling, 0 replies; 8+ messages in thread
From: Hans Verkuil @ 2021-04-13 15:38 UTC (permalink / raw)
To: Aline Santana Cordeiro, Dmitry Osipenko, Mauro Carvalho Chehab,
Greg Kroah-Hartman, Thierry Reding, Jonathan Hunter
Cc: linux-media, linux-staging, linux-kernel, outreachy-kernel
Hi Aline,
On 13/04/2021 00:20, 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
These trace headers are almost a language by themselves, and the
usual alignment rules do not apply. Look at other existing trace.h files
in the kernel and you'll see that they have their own coding style.
So I prefer not to apply this patch.
You couldn't have known that, so it's not your fault.
Regards,
Hans
>
> Signed-off-by: Aline Santana Cordeiro <alinesantanacordeiro@gmail.com>
> ---
> drivers/staging/media/tegra-vde/trace.h | 111 ++++++++++++++------------------
> 1 file changed, 50 insertions(+), 61 deletions(-)
>
> 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)
> );
>
> DEFINE_EVENT(register_access, vde_writel,
> - TP_PROTO(struct tegra_vde *vde, void __iomem *base,
> - u32 offset, u32 value),
> - TP_ARGS(vde, base, offset, value));
> + TP_PROTO(struct tegra_vde *vde, void __iomem *base,
> + u32 offset, u32 value),
> + TP_ARGS(vde, base, offset, value));
> DEFINE_EVENT(register_access, vde_readl,
> - TP_PROTO(struct tegra_vde *vde, void __iomem *base,
> - u32 offset, u32 value),
> - TP_ARGS(vde, base, offset, value));
> + TP_PROTO(struct tegra_vde *vde, void __iomem *base,
> + u32 offset, u32 value),
> + TP_ARGS(vde, base, offset, value));
>
> TRACE_EVENT(vde_setup_iram_entry,
> - TP_PROTO(unsigned int table, unsigned int row, u32 value, u32 aux_addr),
> - TP_ARGS(table, row, value, aux_addr),
> - TP_STRUCT__entry(
> - __field(unsigned int, table)
> - __field(unsigned int, row)
> - __field(u32, value)
> - __field(u32, aux_addr)
> - ),
> - TP_fast_assign(
> - __entry->table = table;
> - __entry->row = row;
> - __entry->value = value;
> - __entry->aux_addr = aux_addr;
> - ),
> - TP_printk("[%u][%u] = { 0x%08x (flags = \"%s\", frame_num = %u); 0x%08x }",
> - __entry->table, __entry->row, __entry->value,
> - __print_flags(__entry->value, " ", { (1 << 25), "B" }),
> - __entry->value & 0x7FFFFF, __entry->aux_addr)
> + TP_PROTO(unsigned int table, unsigned int row, u32 value, u32 aux_addr),
> + TP_ARGS(table, row, value, aux_addr),
> + TP_STRUCT__entry(__field(unsigned int, table)
> + __field(unsigned int, row)
> + __field(u32, value)
> + __field(u32, aux_addr)
> + ),
> + TP_fast_assign(__entry->table = table;
> + __entry->row = row;
> + __entry->value = value;
> + __entry->aux_addr = aux_addr;
> + ),
> + TP_printk("[%u][%u] = { 0x%08x (flags = \"%s\", frame_num = %u); 0x%08x }",
> + __entry->table, __entry->row, __entry->value,
> + __print_flags(__entry->value, " ", { (1 << 25), "B" }),
> + __entry->value & 0x7FFFFF, __entry->aux_addr)
> );
>
> TRACE_EVENT(vde_ref_l0,
> - TP_PROTO(unsigned int frame_num),
> - TP_ARGS(frame_num),
> - TP_STRUCT__entry(
> - __field(unsigned int, frame_num)
> - ),
> - TP_fast_assign(
> - __entry->frame_num = frame_num;
> - ),
> - TP_printk("REF L0: DPB: Frame 0: frame_num = %u", __entry->frame_num)
> + TP_PROTO(unsigned int frame_num),
> + TP_ARGS(frame_num),
> + TP_STRUCT__entry(__field(unsigned int, frame_num)),
> + TP_fast_assign(__entry->frame_num = frame_num;),
> + TP_printk("REF L0: DPB: Frame 0: frame_num = %u", __entry->frame_num)
> );
>
> TRACE_EVENT(vde_ref_l1,
> - TP_PROTO(unsigned int with_later_poc_nb,
> - unsigned int with_earlier_poc_nb),
> - TP_ARGS(with_later_poc_nb, with_earlier_poc_nb),
> - TP_STRUCT__entry(
> - __field(unsigned int, with_later_poc_nb)
> - __field(unsigned int, with_earlier_poc_nb)
> - ),
> - TP_fast_assign(
> - __entry->with_later_poc_nb = with_later_poc_nb;
> - __entry->with_earlier_poc_nb = with_earlier_poc_nb;
> - ),
> - TP_printk("REF L1: with_later_poc_nb %u, with_earlier_poc_nb %u",
> - __entry->with_later_poc_nb, __entry->with_earlier_poc_nb)
> + TP_PROTO(unsigned int with_later_poc_nb,
> + unsigned int with_earlier_poc_nb),
> + TP_ARGS(with_later_poc_nb, with_earlier_poc_nb),
> + TP_STRUCT__entry(__field(unsigned int, with_later_poc_nb)
> + __field(unsigned int, with_earlier_poc_nb)),
> + TP_fast_assign(__entry->with_later_poc_nb = with_later_poc_nb;
> + __entry->with_earlier_poc_nb = with_earlier_poc_nb;),
> + TP_printk("REF L1: with_later_poc_nb %u, with_earlier_poc_nb %u",
> + __entry->with_later_poc_nb, __entry->with_earlier_poc_nb)
> );
>
> #endif /* TEGRA_VDE_TRACE_H */
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-04-14 10:58 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-12 22:20 [PATCH] staging: media: tegra-vde: Align line break to match with the open parenthesis in file trace.h Aline Santana Cordeiro
2021-04-13 15:30 ` Thierry Reding
2021-04-13 16:05 ` Dwaipayan Ray
2021-04-13 16:05 ` Dwaipayan Ray
2021-04-13 17:43 ` Joe Perches
2021-04-13 17:43 ` Joe Perches
2021-04-13 17:43 ` Joe Perches
2021-04-13 15:38 ` Hans Verkuil
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.