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@xxxxxxxxxxx> > > --- > > 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 >
