Re: [PATCH 1/4] usb: cdns3: gadget: Replace trace_printk by dev_dbg
Nicolas Boichat writes:
> On Thu, Jul 23, 2020 at 9:17 PM Felipe Balbi wrote:
>>
>> Nicolas Boichat writes:
>>
>> > trace_printk should not be used in production code, replace it
>> > call with dev_dbg.
>> >
>> > Signed-off-by: Nicolas Boichat
>> >
>> > ---
>> >
>> > Unclear why a trace_printk was used in the first place, it's
>> > possible that some rate-limiting is necessary here.
>> >
>> > drivers/usb/cdns3/gadget.c | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
>> > index 5e24c2e57c0d8c8..c303ab7c62d1651 100644
>> > --- a/drivers/usb/cdns3/gadget.c
>> > +++ b/drivers/usb/cdns3/gadget.c
>> > @@ -421,7 +421,7 @@ static int cdns3_start_all_request(struct cdns3_device
>> > *priv_dev,
>> > if ((priv_req->flags & REQUEST_INTERNAL) ||
>> > (priv_ep->flags & EP_TDLCHK_EN) ||
>> > priv_ep->use_streams) {
>> > - trace_printk("Blocking external request\n");
>> > + dev_dbg(priv_dev->dev, "Blocking external
>> > request\n");
>>
>> Instead, I would suggest adding a proper trace event here; one that
>> includes "priv_ep->flags" in the output.
>
> The patch was already merged by Greg
> (https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/usb/cdns3/gadget.c?id=b3a5ce874c2619c9b8a6c5bbcfefdb95e0227600),
> but feel free to do that as a follow-up CL.
>
> Looks like Peter -- the main author, is ok with dev_dbg (also,
> apologies for missing the R-b tag when I sent a v2 -- which is the one
> that was merged by Greg).
That's okay, we can get a proper trace event for v5.10. Maybe Pawel or
Roger would like to take the effort?
--
balbi
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/4] usb: cdns3: gadget: Replace trace_printk by dev_dbg
On Thu, Jul 23, 2020 at 9:17 PM Felipe Balbi wrote:
>
> Nicolas Boichat writes:
>
> > trace_printk should not be used in production code, replace it
> > call with dev_dbg.
> >
> > Signed-off-by: Nicolas Boichat
> >
> > ---
> >
> > Unclear why a trace_printk was used in the first place, it's
> > possible that some rate-limiting is necessary here.
> >
> > drivers/usb/cdns3/gadget.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
> > index 5e24c2e57c0d8c8..c303ab7c62d1651 100644
> > --- a/drivers/usb/cdns3/gadget.c
> > +++ b/drivers/usb/cdns3/gadget.c
> > @@ -421,7 +421,7 @@ static int cdns3_start_all_request(struct cdns3_device
> > *priv_dev,
> > if ((priv_req->flags & REQUEST_INTERNAL) ||
> > (priv_ep->flags & EP_TDLCHK_EN) ||
> > priv_ep->use_streams) {
> > - trace_printk("Blocking external request\n");
> > + dev_dbg(priv_dev->dev, "Blocking external request\n");
>
> Instead, I would suggest adding a proper trace event here; one that
> includes "priv_ep->flags" in the output.
The patch was already merged by Greg
(https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/usb/cdns3/gadget.c?id=b3a5ce874c2619c9b8a6c5bbcfefdb95e0227600),
but feel free to do that as a follow-up CL.
Looks like Peter -- the main author, is ok with dev_dbg (also,
apologies for missing the R-b tag when I sent a v2 -- which is the one
that was merged by Greg).
Thanks,
>
> --
> balbi
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/4] usb: cdns3: gadget: Replace trace_printk by dev_dbg
Nicolas Boichat writes:
> trace_printk should not be used in production code, replace it
> call with dev_dbg.
>
> Signed-off-by: Nicolas Boichat
>
> ---
>
> Unclear why a trace_printk was used in the first place, it's
> possible that some rate-limiting is necessary here.
>
> drivers/usb/cdns3/gadget.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
> index 5e24c2e57c0d8c8..c303ab7c62d1651 100644
> --- a/drivers/usb/cdns3/gadget.c
> +++ b/drivers/usb/cdns3/gadget.c
> @@ -421,7 +421,7 @@ static int cdns3_start_all_request(struct cdns3_device
> *priv_dev,
> if ((priv_req->flags & REQUEST_INTERNAL) ||
> (priv_ep->flags & EP_TDLCHK_EN) ||
> priv_ep->use_streams) {
> - trace_printk("Blocking external request\n");
> + dev_dbg(priv_dev->dev, "Blocking external request\n");
Instead, I would suggest adding a proper trace event here; one that
includes "priv_ep->flags" in the output.
--
balbi
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/4] usb: cdns3: gadget: Replace trace_printk by dev_dbg
On 20-06-27 15:03:04, Nicolas Boichat wrote:
> trace_printk should not be used in production code, replace it
> call with dev_dbg.
>
> Signed-off-by: Nicolas Boichat
>
> ---
>
> Unclear why a trace_printk was used in the first place, it's
> possible that some rate-limiting is necessary here.
>
> drivers/usb/cdns3/gadget.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
> index 5e24c2e57c0d8c8..c303ab7c62d1651 100644
> --- a/drivers/usb/cdns3/gadget.c
> +++ b/drivers/usb/cdns3/gadget.c
> @@ -421,7 +421,7 @@ static int cdns3_start_all_request(struct cdns3_device
> *priv_dev,
> if ((priv_req->flags & REQUEST_INTERNAL) ||
> (priv_ep->flags & EP_TDLCHK_EN) ||
> priv_ep->use_streams) {
> - trace_printk("Blocking external request\n");
> + dev_dbg(priv_dev->dev, "Blocking external request\n");
> return ret;
> }
> }
> --
Reviewed-by: Peter Chen
--
Thanks,
Peter Chen
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/4] usb: cdns3: gadget: Replace trace_printk by dev_dbg
trace_printk should not be used in production code, replace it
call with dev_dbg.
Signed-off-by: Nicolas Boichat
---
Unclear why a trace_printk was used in the first place, it's
possible that some rate-limiting is necessary here.
drivers/usb/cdns3/gadget.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
index 5e24c2e57c0d8c8..c303ab7c62d1651 100644
--- a/drivers/usb/cdns3/gadget.c
+++ b/drivers/usb/cdns3/gadget.c
@@ -421,7 +421,7 @@ static int cdns3_start_all_request(struct cdns3_device
*priv_dev,
if ((priv_req->flags & REQUEST_INTERNAL) ||
(priv_ep->flags & EP_TDLCHK_EN) ||
priv_ep->use_streams) {
- trace_printk("Blocking external request\n");
+ dev_dbg(priv_dev->dev, "Blocking external request\n");
return ret;
}
}
--
2.27.0.212.ge8ba1cc988-goog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
