Date:   Tue, 14 Feb 2023 14:20:27 +0100
From:   Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To:     Jon Hunter <jonathanh@nvidia.com>
Cc:     Andrzej Pietrasiewicz <andrzej.p@collabora.com>,
        linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-tegra@vger.kernel.org
Subject: Re: [PATCH V3 1/2] usb: gadget: u_ether: Convert prints to device
 prints
Message-ID: <Y+uKmxD+QLNSMx8e@kroah.com>
References: <20230209125319.18589-1-jonathanh@nvidia.com>
 <bcbf9d22-c90a-a4d1-2931-0da43bc7371e@collabora.com>
 <3dc8575e-050c-5e04-ae7c-af380801a227@nvidia.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <3dc8575e-050c-5e04-ae7c-af380801a227@nvidia.com>
Precedence: bulk

On Tue, Feb 14, 2023 at 11:55:56AM +0000, Jon Hunter wrote:
> 
> On 13/02/2023 13:49, Andrzej Pietrasiewicz wrote:
> > Hi Jon,
> > 
> > W dniu 9.02.2023 o?13:53, Jon Hunter pisze:
> > > The USB ethernet gadget driver implements its own print macros which
> > > call printk. Device drivers should use the device prints that print the
> > > device name. Fortunately, the same macro names are defined in the header
> > > file 'linux/usb/composite.h' and these use the device prints. Therefore,
> > > remove the local definitions in the USB ethernet gadget driver and use
> > > those in 'linux/usb/composite.h'. The only difference is that now the
> > > device name is printed instead of the ethernet interface name.
> > > 
> > > Tested using ethernet gadget on Jetson AGX Orin.
> > > 
> > > Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> > > Tested-by: Jon Hunter <jonathanh@nvidia.com>
> > > ---
> > > V3: Added this patch and dropped the patch in V2 that improved some of
> > > ???? the prints.
> > > 
> > > ? drivers/usb/gadget/function/u_ether.c | 36 +--------------------------
> > > ? 1 file changed, 1 insertion(+), 35 deletions(-)
> > > 
> > > diff --git a/drivers/usb/gadget/function/u_ether.c
> > > b/drivers/usb/gadget/function/u_ether.c
> > > index 8f12f3f8f6ee..740331882e8d 100644
> > > --- a/drivers/usb/gadget/function/u_ether.c
> > > +++ b/drivers/usb/gadget/function/u_ether.c
> > > @@ -17,6 +17,7 @@
> > > ? #include <linux/etherdevice.h>
> > > ? #include <linux/ethtool.h>
> > > ? #include <linux/if_vlan.h>
> > > +#include <linux/usb/composite.h>
> > > ? #include "u_ether.h"
> > > @@ -103,41 +104,6 @@ static inline int qlen(struct usb_gadget
> > > *gadget, unsigned qmult)
> > > /*-------------------------------------------------------------------------*/
> > > -/* REVISIT there must be a better way than having two sets
> > > - * of debug calls ...
> > > - */
> > > -
> > > -#undef DBG
> > > -#undef VDBG
> > > -#undef ERROR
> > > -#undef INFO
> > > -
> > > -#define xprintk(d, level, fmt, args...) \
> > > -??? printk(level "%s: " fmt , (d)->net->name , ## args)
> > > -
> > > -#ifdef DEBUG
> > > -#undef DEBUG
> > > -#define DBG(dev, fmt, args...) \
> > > -??? xprintk(dev , KERN_DEBUG , fmt , ## args)
> > > -#else
> > > -#define DBG(dev, fmt, args...) \
> > > -??? do { } while (0)
> > > -#endif /* DEBUG */
> > 
> > Actually there are more (at least hypothetical) changes than the declared
> > change of printed device name.
> > 
> > If DEBUG is not set there can be _more_ messages printed
> > when your patch is applied as-is (i.e. all DBG() invocations will
> > expand into some dev_dbg(), whereas before the patch is applied
> > they compile into nothing).
> 
> So dev_dbg() messages will only be printed if DEBUG is defined or if dynamic
> debug is enabled and the prints are enabled.
> 
> Greg, let me know what your thoughts on this version are?

Looks good to me, let me queue it up now.

thanks,

greg k-h