Re: [PATCH 01/28] usb: common: change usb_debug_root as static variable

From: Chunfeng Yun <chunfeng.yun@mediatek.com>
To: Guenter Roeck <linux@roeck-us.net>
Cc: Peter Chen <Peter.Chen@nxp.com>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	Heikki Krogerus <heikki.krogerus@linux.intel.com>,
	Wolfram Sang <wsa@the-dreams.de>,
	Yangtao Li <tiny.windzz@gmail.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	linux-kernel@vger.kernel.org,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Mans Rullgard <mans@mansr.com>,
	Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Fabio Estevam <festevam@gmail.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Robert Jarzmik <robert.jarzmik@free.fr>,
	Kevin Cernekee <cernekee@gmail.com>,
	"Gustavo A. R. Silva" <gustavo@embeddedor.com>,
	Ludovic Desroches <ludovic.desroches@microchip.com>,
	Alan Stern <stern@rowland.harvard.edu>,
	NXP Linux Team <linux-imx@nxp.com>,
	bcm-kernel-feedback-list@broadcom.com,
	David Kershner <david.kershner@unisys.com>,
	Hans Ulli Kroll <ulli.kroll@googlemail.com>,
	Cristian Birsan <cristian.birsan@microchip.com>,
	linux-media@vger.kernel.org,
	Fabrizio Castro <fabrizio.castro@bp.renesas.com>,
	Mathias Payer <mathias.payer@nebelwelt.net>,
	Mathias Nyman <mathias.nyman@intel.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Haojian Zhuang <haojian.zhuang@gmail.com>,
	Stephen Boyd <swboyd@chromium.org>,
	linux-mediatek@lists.infradead.org,
	Minas Harutyunyan <hminas@synopsys.com>,
	Simon Horman <horms+renesas@verge.net.au>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Bin Liu <b-liu@ti.com>,
	linux-arm-kernel@lists.infradead.org,
	Biju Das <biju.das@bp.renesas.com>,
	Felipe Balbi <balbi@kernel.org>,
	Laurentiu Tudor <laurentiu.tudor@nxp.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>,
	linux-usb@vger.kernel.org,
	Nicolas Ferre <nicolas.ferre@microchip.com>,
	Dmitry Torokhov <dtor@chromium.org>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Colin Ian King <colin.king@canonical.com>,
	Shawn Guo <shawnguo@kernel.org>, Daniel Mack <daniel@zonque.org>
Subject: Re: [PATCH 01/28] usb: common: change usb_debug_root as static variable
Date: Wed, 6 Nov 2019 17:11:31 +0800	[thread overview]
Message-ID: <1573031491.18464.70.camel@mhfsdcap03> (raw)
In-Reply-To: <79fd2ebd-b8c6-ee00-c71e-782475a20de6@roeck-us.net>

On Tue, 2019-11-05 at 20:03 -0800, Guenter Roeck wrote:
> On 11/5/19 7:15 PM, Chunfeng Yun wrote:
> > Try to avoid using extern global variable, and provide two
> > functions for the usage cases
> > 
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> >   drivers/usb/common/common.c | 16 ++++++++++++++--
> >   include/linux/usb.h         |  5 ++++-
> >   2 files changed, 18 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
> > index 1433260d99b4..639ee6d243a2 100644
> > --- a/drivers/usb/common/common.c
> > +++ b/drivers/usb/common/common.c
> > @@ -293,8 +293,20 @@ struct device *usb_of_get_companion_dev(struct device *dev)
> >   EXPORT_SYMBOL_GPL(usb_of_get_companion_dev);
> >   #endif
> >   
> > -struct dentry *usb_debug_root;
> > -EXPORT_SYMBOL_GPL(usb_debug_root);
> > +static struct dentry *usb_debug_root;
> > +
> 
> I don't think it is a good idea to declare this variable static
> before all its uses are removed. This patch will leave the system
> in a non-compileable state until all patches of the series have
> been applied.
Yes, you are right, I'll fix it, may be split into two patch

> 
> I am curious. What is the rationale for this change ?
> Did exported variables run out of favor ?
Personally, I think it's good practice to avoid global varibale,
it may not a good reason. 
another one is that want to create all debug directory/file in usb root
for host/device controller drivers.

>  Sorry if there was a
> patch 0 of the series explaining the rationale and I missed it.
Sorry, I do not prepare cover latter, will add it if send out next
version.


> 
> Guenter


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek