linux-kernel - Re: [PATCH 06/25] USB: ehci: allow need_io_watchdog to be passed to ehci-platform driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Wed, 03 Oct 2012 18:12:29 +0200
From:	Florian Fainelli <florian@...nwrt.org>
To:	Alan Stern <stern@...land.harvard.edu>
Cc:	linux-usb@...r.kernel.org, Ralf Baechle <ralf@...ux-mips.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Gabor Juhos <juhosg@...nwrt.org>,
	Hauke Mehrtens <hauke@...ke-m.de>,
	Kelvin Cheung <keguang.zhang@...il.com>,
	Jayachandran C <jayachandranc@...logicmicro.com>,
	Dan Carpenter <dan.carpenter@...cle.com>,
	Geert Uytterhoeven <geert@...ux-m68k.org>,
	linux-mips@...ux-mips.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 06/25] USB: ehci: allow need_io_watchdog to be passed to ehci-platform driver

On Wednesday 03 October 2012 12:01:22 Alan Stern wrote:
> On Wed, 3 Oct 2012, Florian Fainelli wrote:
> 
> > And convert all the existing users of ehci-platform to specify a correct
> > need_io_watchdog value.
> 
> IMO (and I realize that not everybody agrees), the patch description 
> should not be considered an extension of the patch title, as though the 
> title were the first sentence and the description the remainder of the 
> same paragraph.  Descriptions should stand on their own and be 
> comprehensible even to somebody who hasn't read the title.
> 
> > Signed-off-by: Florian Fainelli <florian@...nwrt.org>
> > ---
> >  arch/mips/ath79/dev-usb.c             |    2 ++
> >  arch/mips/loongson1/common/platform.c |    1 +
> >  arch/mips/netlogic/xlr/platform.c     |    1 +
> >  drivers/usb/host/bcma-hcd.c           |    1 +
> >  drivers/usb/host/ehci-platform.c      |    1 +
> >  drivers/usb/host/ssb-hcd.c            |    1 +
> >  include/linux/usb/ehci_pdriver.h      |    3 +++
> >  7 files changed, 10 insertions(+)
> 
> More importantly...  Nearly every driver will have need_io_watchdog
> set.  Only a few of them explicitly turn it off.  The approach you're 
> using puts an extra burden on most of the drivers.
> 
> > diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-
platform.c
> > index 764e010..08d5dec 100644
> > --- a/drivers/usb/host/ehci-platform.c
> > +++ b/drivers/usb/host/ehci-platform.c
> > @@ -32,6 +32,7 @@ static int ehci_platform_reset(struct usb_hcd *hcd)
> >  	ehci->has_synopsys_hc_bug = pdata->has_synopsys_hc_bug;
> >  	ehci->big_endian_desc = pdata->big_endian_desc;
> >  	ehci->big_endian_mmio = pdata->big_endian_mmio;
> > +	ehci->need_io_watchdog = pdata->need_io_watchdog;
> 
> > --- a/include/linux/usb/ehci_pdriver.h
> > +++ b/include/linux/usb/ehci_pdriver.h
> > @@ -29,6 +29,8 @@
> >   *			initialization.
> >   * @port_power_off:	set to 1 if the controller needs to be powered down
> >   *			after initialization.
> > + * @need_io_watchdog:	set to 1 if the controller needs the I/O 
watchdog to
> > + *			run.
> 
> Instead, how about adding a no_io_watchdog flag?  Then after 
> ehci-platform.c calls ehci_setup(), it can see whether to turn off 
> ehci->need_io_watchdog.

Sounds good, you are right,  this also greatly reduces the chances to miss one 
user of this "feature".

> 
> This way, most of this patch would become unnecessary.
> 
> Alan Stern
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/