| Message ID | 20180820103001.6453-3-felipe.balbi@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] usb: dwc3: gadget: return errors from __dwc3_gadget_start_isoc() | expand |
Hi Felipe, Thank you for the patch. On Monday, 20 August 2018 13:30:00 EEST Felipe Balbi wrote: > They are much more useful in hexadecimal than in decimal. Moreover, > generic commands are already logged in hex. > > Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> > --- > drivers/usb/dwc3/trace.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/dwc3/trace.h b/drivers/usb/dwc3/trace.h > index f22714cce070..50fb6f2d92dd 100644 > --- a/drivers/usb/dwc3/trace.h > +++ b/drivers/usb/dwc3/trace.h > @@ -199,7 +199,7 @@ DECLARE_EVENT_CLASS(dwc3_log_gadget_ep_cmd, > __entry->param2 = params->param2; > __entry->cmd_status = cmd_status; > ), > - TP_printk("%s: cmd '%s' [%d] params %08x %08x %08x --> status: %s", > + TP_printk("%s: cmd '%s' [%x] params %08x %08x %08x --> status: %s", How about 0x%x ? Otherwise one could get confused when the command representation in hex doesn't contain a letter (especially given that this patch transitions from decimal to hexadecimal). > __get_str(name), dwc3_gadget_ep_cmd_string(__entry->cmd), > __entry->cmd, __entry->param0, > __entry->param1, __entry->param2,
On Mon, Aug 20, 2018 at 2:25 PM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: >> - TP_printk("%s: cmd '%s' [%d] params %08x %08x %08x --> status: %s", >> + TP_printk("%s: cmd '%s' [%x] params %08x %08x %08x --> status: %s", > > How about 0x%x ? Side note: # is one character less for the same.
Hi Andy, On Monday, 20 August 2018 15:06:31 EEST Andy Shevchenko wrote: > On Mon, Aug 20, 2018 at 2:25 PM, Laurent Pinchart wrote: > >> - TP_printk("%s: cmd '%s' [%d] params %08x %08x %08x --> status: %s", > >> + TP_printk("%s: cmd '%s' [%x] params %08x %08x %08x --> status: %s", > > > > How about 0x%x ? > > Side note: # is one character less for the same. Doesn't that print 0 instead of 0x0 ? There's no ambiguity with 0, but I find that always printing the 0x is more consistent. I'll leave that up to Felipe, I'm OK with both options.
On Mon, Aug 20, 2018 at 3:21 PM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Andy, > > On Monday, 20 August 2018 15:06:31 EEST Andy Shevchenko wrote: > > On Mon, Aug 20, 2018 at 2:25 PM, Laurent Pinchart wrote: > > >> - TP_printk("%s: cmd '%s' [%d] params %08x %08x %08x --> status: %s", > > >> + TP_printk("%s: cmd '%s' [%x] params %08x %08x %08x --> status: %s", > > > > > > How about 0x%x ? > > > > Side note: # is one character less for the same. > > Doesn't that print 0 instead of 0x0 ? There's no ambiguity with 0, but I find > that always printing the 0x is more consistent. I'll leave that up to Felipe, > I'm OK with both options. # The value should be converted to an "alternate form". For o conversions, the first character of the output string is made zero (by prefixing a 0 if it was not zero already). For x and X con‐ versions, a nonzero result has the string "0x" (or "0X" for X conversions) prepended to it.
Hi Andy, On Thursday, 23 August 2018 12:57:57 EEST Andy Shevchenko wrote: > On Mon, Aug 20, 2018 at 3:21 PM Laurent Pinchart wrote: > > On Monday, 20 August 2018 15:06:31 EEST Andy Shevchenko wrote: > >> On Mon, Aug 20, 2018 at 2:25 PM, Laurent Pinchart wrote: > >>>> - TP_printk("%s: cmd '%s' [%d] params %08x %08x %08x --> status: > >>>> %s", > >>>> + TP_printk("%s: cmd '%s' [%x] params %08x %08x %08x --> status: > >>>> %s", > >>> > >>> How about 0x%x ? > >> > >> Side note: # is one character less for the same. > > > > Doesn't that print 0 instead of 0x0 ? There's no ambiguity with 0, but I > > find that always printing the 0x is more consistent. I'll leave that up > > to Felipe, I'm OK with both options. > > # The value should be converted to an "alternate form". > For o conversions, the first character of > the output string is made zero (by prefixing a 0 if it > was not zero already). For x and X con‐ > versions, a nonzero result has the string "0x" (or "0X" > for X conversions) prepended to it. That's exactly my point, it will only prepend 0x when the value is not zero. Small inconsistency, but I don't mind too much.
diff --git a/drivers/usb/dwc3/trace.h b/drivers/usb/dwc3/trace.h index f22714cce070..50fb6f2d92dd 100644 --- a/drivers/usb/dwc3/trace.h +++ b/drivers/usb/dwc3/trace.h @@ -199,7 +199,7 @@ DECLARE_EVENT_CLASS(dwc3_log_gadget_ep_cmd, __entry->param2 = params->param2; __entry->cmd_status = cmd_status; ), - TP_printk("%s: cmd '%s' [%d] params %08x %08x %08x --> status: %s", + TP_printk("%s: cmd '%s' [%x] params %08x %08x %08x --> status: %s", __get_str(name), dwc3_gadget_ep_cmd_string(__entry->cmd), __entry->cmd, __entry->param0, __entry->param1, __entry->param2,
They are much more useful in hexadecimal than in decimal. Moreover, generic commands are already logged in hex. Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> --- drivers/usb/dwc3/trace.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)