| Message ID | 20210412222040.GA13659@focaruja |
|---|---|
| State | New |
| Headers | show |
| Series | staging: media: tegra-vde: Align line break to match with the open parenthesis in file trace.h | expand |
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 >
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 */ >
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) > > ); > >
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
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 */
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(-)