| Message ID | 1672996895-22762-3-git-send-email-quic_linyyuan@quicinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] usb: dwc3: simplify operation in dwc3_readl() and dwc3_writel() | expand |
On Fri, Jan 06, 2023, Linyu Yuan wrote: > when trace register read/write operation, it is not necessary to show > virtual address cacaulate from base and offset. > > remove the base register value, it will save trace buffer. > it is enough only save and show the offset. > > Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com> > --- > drivers/usb/dwc3/io.h | 4 ++-- > drivers/usb/dwc3/trace.h | 17 +++++++---------- > 2 files changed, 9 insertions(+), 12 deletions(-) > > diff --git a/drivers/usb/dwc3/io.h b/drivers/usb/dwc3/io.h > index d24513e..fcb5726 100644 > --- a/drivers/usb/dwc3/io.h > +++ b/drivers/usb/dwc3/io.h > @@ -32,7 +32,7 @@ static inline u32 dwc3_readl(void __iomem *base, u32 offset) > * documentation, so we revert it back to the proper addresses, the > * same way they are described on SNPS documentation > */ > - trace_dwc3_readl(base, offset, value); > + trace_dwc3_readl(offset, value); > > return value; > } > @@ -61,7 +61,7 @@ static inline void dwc3_writel(void __iomem *base, u32 offset, u32 value) > * documentation, so we revert it back to the proper addresses, the > * same way they are described on SNPS documentation > */ > - trace_dwc3_writel(base, offset, value); > + trace_dwc3_writel(offset, value); > } > > #endif /* __DRIVERS_USB_DWC3_IO_H */ > diff --git a/drivers/usb/dwc3/trace.h b/drivers/usb/dwc3/trace.h > index 1975aec..536b9a1 100644 > --- a/drivers/usb/dwc3/trace.h > +++ b/drivers/usb/dwc3/trace.h > @@ -20,32 +20,29 @@ > #include "debug.h" > > DECLARE_EVENT_CLASS(dwc3_log_io, > - TP_PROTO(void *base, u32 offset, u32 value), > - TP_ARGS(base, offset, value), > + TP_PROTO(u32 offset, u32 value), > + TP_ARGS(offset, value), > TP_STRUCT__entry( > - __field(void *, base) > __field(u32, offset) > __field(u32, value) > ), > TP_fast_assign( > - __entry->base = base; > __entry->offset = offset; > __entry->value = value; > ), > - TP_printk("addr %p offset %04x value %08x", > - __entry->base + __entry->offset, Please don't remove this. Sometime we need to check the base address for different register offset. Currently some offsets need this to be able to differientiate between different registers (e.g. different endpoint registers DEPCMP and parameters) Thanks, Thinh > + TP_printk("offset %04x value %08x", > __entry->offset, > __entry->value) > ); > > DEFINE_EVENT(dwc3_log_io, dwc3_readl, > - TP_PROTO(void __iomem *base, u32 offset, u32 value), > - TP_ARGS(base, offset, value) > + TP_PROTO(u32 offset, u32 value), > + TP_ARGS(offset, value) > ); > > DEFINE_EVENT(dwc3_log_io, dwc3_writel, > - TP_PROTO(void __iomem *base, u32 offset, u32 value), > - TP_ARGS(base, offset, value) > + TP_PROTO(u32 offset, u32 value), > + TP_ARGS(offset, value) > ); > > DECLARE_EVENT_CLASS(dwc3_log_event, > -- > 2.7.4 >
On 1/10/2023 2:47 AM, Thinh Nguyen wrote: > On Fri, Jan 06, 2023, Linyu Yuan wrote: >> when trace register read/write operation, it is not necessary to show >> virtual address cacaulate from base and offset. >> >> remove the base register value, it will save trace buffer. >> it is enough only save and show the offset. >> >> Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com> >> --- >> drivers/usb/dwc3/io.h | 4 ++-- >> drivers/usb/dwc3/trace.h | 17 +++++++---------- >> 2 files changed, 9 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/usb/dwc3/io.h b/drivers/usb/dwc3/io.h >> index d24513e..fcb5726 100644 >> --- a/drivers/usb/dwc3/io.h >> +++ b/drivers/usb/dwc3/io.h >> @@ -32,7 +32,7 @@ static inline u32 dwc3_readl(void __iomem *base, u32 offset) >> * documentation, so we revert it back to the proper addresses, the >> * same way they are described on SNPS documentation >> */ >> - trace_dwc3_readl(base, offset, value); >> + trace_dwc3_readl(offset, value); >> >> return value; >> } >> @@ -61,7 +61,7 @@ static inline void dwc3_writel(void __iomem *base, u32 offset, u32 value) >> * documentation, so we revert it back to the proper addresses, the >> * same way they are described on SNPS documentation >> */ >> - trace_dwc3_writel(base, offset, value); >> + trace_dwc3_writel(offset, value); >> } >> >> #endif /* __DRIVERS_USB_DWC3_IO_H */ >> diff --git a/drivers/usb/dwc3/trace.h b/drivers/usb/dwc3/trace.h >> index 1975aec..536b9a1 100644 >> --- a/drivers/usb/dwc3/trace.h >> +++ b/drivers/usb/dwc3/trace.h >> @@ -20,32 +20,29 @@ >> #include "debug.h" >> >> DECLARE_EVENT_CLASS(dwc3_log_io, >> - TP_PROTO(void *base, u32 offset, u32 value), >> - TP_ARGS(base, offset, value), >> + TP_PROTO(u32 offset, u32 value), >> + TP_ARGS(offset, value), >> TP_STRUCT__entry( >> - __field(void *, base) >> __field(u32, offset) >> __field(u32, value) >> ), >> TP_fast_assign( >> - __entry->base = base; >> __entry->offset = offset; >> __entry->value = value; >> ), >> - TP_printk("addr %p offset %04x value %08x", >> - __entry->base + __entry->offset, > Please don't remove this. Sometime we need to check the base address for > different register offset. Currently some offsets need this to be able > to differientiate between different registers (e.g. different endpoint > registers DEPCMP and parameters) thanks, will drop this patch. > > Thanks, > Thinh > >> + TP_printk("offset %04x value %08x", >> __entry->offset, >> __entry->value) >> ); >> >> DEFINE_EVENT(dwc3_log_io, dwc3_readl, >> - TP_PROTO(void __iomem *base, u32 offset, u32 value), >> - TP_ARGS(base, offset, value) >> + TP_PROTO(u32 offset, u32 value), >> + TP_ARGS(offset, value) >> ); >> >> DEFINE_EVENT(dwc3_log_io, dwc3_writel, >> - TP_PROTO(void __iomem *base, u32 offset, u32 value), >> - TP_ARGS(base, offset, value) >> + TP_PROTO(u32 offset, u32 value), >> + TP_ARGS(offset, value) >> ); >> >> DECLARE_EVENT_CLASS(dwc3_log_event, >> -- >> 2.7.4
On Tue, Jan 10, 2023, Linyu Yuan wrote: > > On 1/10/2023 2:47 AM, Thinh Nguyen wrote: > > On Fri, Jan 06, 2023, Linyu Yuan wrote: > > > when trace register read/write operation, it is not necessary to show > > > virtual address cacaulate from base and offset. > > > > > > remove the base register value, it will save trace buffer. > > > it is enough only save and show the offset. > > > > > > Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com> > > > --- > > > drivers/usb/dwc3/io.h | 4 ++-- > > > drivers/usb/dwc3/trace.h | 17 +++++++---------- > > > 2 files changed, 9 insertions(+), 12 deletions(-) > > > > > > diff --git a/drivers/usb/dwc3/io.h b/drivers/usb/dwc3/io.h > > > index d24513e..fcb5726 100644 > > > --- a/drivers/usb/dwc3/io.h > > > +++ b/drivers/usb/dwc3/io.h > > > @@ -32,7 +32,7 @@ static inline u32 dwc3_readl(void __iomem *base, u32 offset) > > > * documentation, so we revert it back to the proper addresses, the > > > * same way they are described on SNPS documentation > > > */ > > > - trace_dwc3_readl(base, offset, value); > > > + trace_dwc3_readl(offset, value); > > > return value; > > > } > > > @@ -61,7 +61,7 @@ static inline void dwc3_writel(void __iomem *base, u32 offset, u32 value) > > > * documentation, so we revert it back to the proper addresses, the > > > * same way they are described on SNPS documentation > > > */ > > > - trace_dwc3_writel(base, offset, value); > > > + trace_dwc3_writel(offset, value); > > > } > > > #endif /* __DRIVERS_USB_DWC3_IO_H */ > > > diff --git a/drivers/usb/dwc3/trace.h b/drivers/usb/dwc3/trace.h > > > index 1975aec..536b9a1 100644 > > > --- a/drivers/usb/dwc3/trace.h > > > +++ b/drivers/usb/dwc3/trace.h > > > @@ -20,32 +20,29 @@ > > > #include "debug.h" > > > DECLARE_EVENT_CLASS(dwc3_log_io, > > > - TP_PROTO(void *base, u32 offset, u32 value), > > > - TP_ARGS(base, offset, value), > > > + TP_PROTO(u32 offset, u32 value), > > > + TP_ARGS(offset, value), > > > TP_STRUCT__entry( > > > - __field(void *, base) > > > __field(u32, offset) > > > __field(u32, value) > > > ), > > > TP_fast_assign( > > > - __entry->base = base; > > > __entry->offset = offset; > > > __entry->value = value; > > > ), > > > - TP_printk("addr %p offset %04x value %08x", > > > - __entry->base + __entry->offset, > > Please don't remove this. Sometime we need to check the base address for > > different register offset. Currently some offsets need this to be able > > to differientiate between different registers (e.g. different endpoint > > registers DEPCMP and parameters) DEPCMP -> DEPCMD* > > thanks, will drop this patch. > If we fix it so that all the offset prints represent the correct offset of all the registers, then we can drop this base address print. That would be great. Thanks, Thinh
On 1/10/2023 10:57 AM, Thinh Nguyen wrote: > On Tue, Jan 10, 2023, Linyu Yuan wrote: >> On 1/10/2023 2:47 AM, Thinh Nguyen wrote: >>> On Fri, Jan 06, 2023, Linyu Yuan wrote: >>>> when trace register read/write operation, it is not necessary to show >>>> virtual address cacaulate from base and offset. >>>> >>>> remove the base register value, it will save trace buffer. >>>> it is enough only save and show the offset. >>>> >>>> Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com> >>>> --- >>>> drivers/usb/dwc3/io.h | 4 ++-- >>>> drivers/usb/dwc3/trace.h | 17 +++++++---------- >>>> 2 files changed, 9 insertions(+), 12 deletions(-) >>>> >>>> diff --git a/drivers/usb/dwc3/io.h b/drivers/usb/dwc3/io.h >>>> index d24513e..fcb5726 100644 >>>> --- a/drivers/usb/dwc3/io.h >>>> +++ b/drivers/usb/dwc3/io.h >>>> @@ -32,7 +32,7 @@ static inline u32 dwc3_readl(void __iomem *base, u32 offset) >>>> * documentation, so we revert it back to the proper addresses, the >>>> * same way they are described on SNPS documentation >>>> */ >>>> - trace_dwc3_readl(base, offset, value); >>>> + trace_dwc3_readl(offset, value); >>>> return value; >>>> } >>>> @@ -61,7 +61,7 @@ static inline void dwc3_writel(void __iomem *base, u32 offset, u32 value) >>>> * documentation, so we revert it back to the proper addresses, the >>>> * same way they are described on SNPS documentation >>>> */ >>>> - trace_dwc3_writel(base, offset, value); >>>> + trace_dwc3_writel(offset, value); >>>> } >>>> #endif /* __DRIVERS_USB_DWC3_IO_H */ >>>> diff --git a/drivers/usb/dwc3/trace.h b/drivers/usb/dwc3/trace.h >>>> index 1975aec..536b9a1 100644 >>>> --- a/drivers/usb/dwc3/trace.h >>>> +++ b/drivers/usb/dwc3/trace.h >>>> @@ -20,32 +20,29 @@ >>>> #include "debug.h" >>>> DECLARE_EVENT_CLASS(dwc3_log_io, >>>> - TP_PROTO(void *base, u32 offset, u32 value), >>>> - TP_ARGS(base, offset, value), >>>> + TP_PROTO(u32 offset, u32 value), >>>> + TP_ARGS(offset, value), >>>> TP_STRUCT__entry( >>>> - __field(void *, base) >>>> __field(u32, offset) >>>> __field(u32, value) >>>> ), >>>> TP_fast_assign( >>>> - __entry->base = base; >>>> __entry->offset = offset; >>>> __entry->value = value; >>>> ), >>>> - TP_printk("addr %p offset %04x value %08x", >>>> - __entry->base + __entry->offset, >>> Please don't remove this. Sometime we need to check the base address for >>> different register offset. Currently some offsets need this to be able >>> to differientiate between different registers (e.g. different endpoint >>> registers DEPCMP and parameters) > DEPCMP -> DEPCMD* I can't understand your comment, I think every DEPCMD* register has it own offset. > >> thanks, will drop this patch. >> > If we fix it so that all the offset prints represent the correct offset > of all the registers, then we can drop this base address print. That > would be great. I think only if multiple gadgets exist in real world, then base address is needed. > > Thanks, > Thinh
diff --git a/drivers/usb/dwc3/io.h b/drivers/usb/dwc3/io.h index d24513e..fcb5726 100644 --- a/drivers/usb/dwc3/io.h +++ b/drivers/usb/dwc3/io.h @@ -32,7 +32,7 @@ static inline u32 dwc3_readl(void __iomem *base, u32 offset) * documentation, so we revert it back to the proper addresses, the * same way they are described on SNPS documentation */ - trace_dwc3_readl(base, offset, value); + trace_dwc3_readl(offset, value); return value; } @@ -61,7 +61,7 @@ static inline void dwc3_writel(void __iomem *base, u32 offset, u32 value) * documentation, so we revert it back to the proper addresses, the * same way they are described on SNPS documentation */ - trace_dwc3_writel(base, offset, value); + trace_dwc3_writel(offset, value); } #endif /* __DRIVERS_USB_DWC3_IO_H */ diff --git a/drivers/usb/dwc3/trace.h b/drivers/usb/dwc3/trace.h index 1975aec..536b9a1 100644 --- a/drivers/usb/dwc3/trace.h +++ b/drivers/usb/dwc3/trace.h @@ -20,32 +20,29 @@ #include "debug.h" DECLARE_EVENT_CLASS(dwc3_log_io, - TP_PROTO(void *base, u32 offset, u32 value), - TP_ARGS(base, offset, value), + TP_PROTO(u32 offset, u32 value), + TP_ARGS(offset, value), TP_STRUCT__entry( - __field(void *, base) __field(u32, offset) __field(u32, value) ), TP_fast_assign( - __entry->base = base; __entry->offset = offset; __entry->value = value; ), - TP_printk("addr %p offset %04x value %08x", - __entry->base + __entry->offset, + TP_printk("offset %04x value %08x", __entry->offset, __entry->value) ); DEFINE_EVENT(dwc3_log_io, dwc3_readl, - TP_PROTO(void __iomem *base, u32 offset, u32 value), - TP_ARGS(base, offset, value) + TP_PROTO(u32 offset, u32 value), + TP_ARGS(offset, value) ); DEFINE_EVENT(dwc3_log_io, dwc3_writel, - TP_PROTO(void __iomem *base, u32 offset, u32 value), - TP_ARGS(base, offset, value) + TP_PROTO(u32 offset, u32 value), + TP_ARGS(offset, value) ); DECLARE_EVENT_CLASS(dwc3_log_event,
when trace register read/write operation, it is not necessary to show virtual address cacaulate from base and offset. remove the base register value, it will save trace buffer. it is enough only save and show the offset. Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com> --- drivers/usb/dwc3/io.h | 4 ++-- drivers/usb/dwc3/trace.h | 17 +++++++---------- 2 files changed, 9 insertions(+), 12 deletions(-)