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] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 3 Oct 2012 12:01:22 -0400 (EDT)
From:	Alan Stern <stern@...land.harvard.edu>
To:	Florian Fainelli <florian@...nwrt.org>
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 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.

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/