Re: [PATCH] staging: media: tegra-vde: Align line break to match with the open parenthesis in file trace.h
On Tue, 2021-04-13 at 21:35 +0530, Dwaipayan Ray wrote:
> On Tue, Apr 13, 2021 at 8:59 PM Thierry Reding
> 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
> > > ---
> > > 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
Re: [PATCH] staging: media: tegra-vde: Align line break to match with the open parenthesis in file trace.h
On Tue, Apr 13, 2021 at 8:59 PM Thierry Reding 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
> > ---
> > 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)
> > );
> >
Re: [PATCH] staging: media: tegra-vde: Align line break to match with the open parenthesis in file trace.h
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
> ---
> 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 & 0x7F, __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 }",
> +
Re: [PATCH] staging: media: tegra-vde: Align line break to match with the open parenthesis in file trace.h
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
> ---
> 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 & 0x7F, __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 =
[PATCH] staging: media: tegra-vde: Align line break to match with the open parenthesis in file trace.h
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
---
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 & 0x7F, __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 & 0x7F, __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 =
