bout this.
Sadly, David Brownell died in 2011. I can tell you, though, that
commit 869aa98c does not eliminate the need to call ohci_usb_reset().
Alan Stern
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
On Fri, 21 Jun 2013, Manjunath Goudar wrote:
> On 20 June 2013 22:23, Alan Stern wrote:
>
> > On Thu, 20 Jun 2013, Manjunath Goudar wrote:
> >
> > > > > @@ -686,7 +631,7 @@ ohci_hcd_at91_drv_suspend(struct platform_device
> > > > *pdev, pm_message_t
ohci_resume() instead of ohci_restart().
Why? Don't you think the current code has a good reason for calling
ohci_usb_reset()?
Alan Stern
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
line.
> @@ -371,10 +388,9 @@ static int usb_hcd_s3c2410_probe(const struct hc_driver
> *driver,
> goto err_put;
> }
>
> + ohci_setup(hcd);
> s3c2410_start_hc(dev, hcd);
I thought this patch was supposed to get rid of the call to
ohci_setup()?
ch removes the call to
ohci_setup()?
> @@ -686,7 +631,7 @@ ohci_hcd_at91_drv_suspend(struct platform_device *pdev,
> pm_message_t mesg)
>* REVISIT: some boards will be able to turn VBUS off...
>*/
> if (at91_suspend_entering_slow
here. You can simply remove it.
> + dev_dbg(hcd->self.controller, "starting OHCI controller\n");
>
> ret = usb_add_hcd(hcd, irq, 0);
> if (ret) {
Alan Stern
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
.11.
>
> V2:
> -ohci_setup() removed because it is called in .reset member
> of the ohci_hc_driver structure.
> -debugging stuff isn't needed any more that's what removed.
Acked-by: Alan Stern
___
linaro-dev mailing list
linaro-dev@lis
ohci_to_hcd(ohci)->power_budget = 8;
> + ohci->hc_control = OHCI_CTRL_RWC;
> + writel(OHCI_CTRL_RWC, &ohci->regs->control);
This last line must not appear here, because ohci->regs doesn't get set
until ohci_setup() calls ohci_init(). Remo
des */
> need_transceiver = need_transceiver
> || machine_is_omap_h2() || machine_is_omap_h3();
> @@ -238,9 +245,6 @@ static int ohci_omap_init(struct usb_hcd *hcd)
> omap_1510_local_bus_init();
> }
>
> - if ((ret = ohci_init(o
> +#include
> +#include
> +#include
> +
> +#include "ohci.h"
> +
> +#define DRIVER_DESC "OHCI exynos driver"
I think the word "EXYNOS" is supposed to be in all capital letters.
Apart from that,
Acked-by: Alan Stern
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
, we need to wait until the
interrupt has been handled before checking whether there is a pending
wakeup. That's what synchronize_irq() does; it waits until all the
outstanding interrupt requests have been handled.
(Also, as Sachin pointed out, you ha
On Fri, 14 Jun 2013, Manjunath Goudar wrote:
> HI Alan,
>
> Can you review this patch series.
I will, but I won't get to it for a while. I have a lot of other stuff
to work on first.
Alan Stern
___
linaro-dev mailing l
rgh! You're right, of course.
I didn't see it, because the only existing place where this check is
made is in the PCI glue layer. Pushing it into the HCDs themselves is
obviously the right thing to do.
Manjanuth, let's do this. You can write a preliminary patch that puts
this ch
the spin_lock/unlock and
the call to s3c2410_stop_hc(). The comment isn't needed, nor is the
statement label. (In fact, I'm not even sure if the spin_lock/unlock
lines are needed. It depends on the hardware details.)
Alan Stern
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
r it.
> @@ -417,8 +428,8 @@ static int ohci_da8xx_suspend(struct platform_device
> *dev, pm_message_t message)
>
> ohci_da8xx_clock(0);
> hcd->state = HC_STATE_SUSPENDED;
> - dev->dev.power.power_state = PMSG_SUSPEND;
> - return 0;
>
;dev);
> + int ret;
Please use tab characters so that "do_wakeup" and "ret" are lined up
directly beneath "*hcd" and "*ohci". The rest of this patch is okay.
Acked-by: Alan Stern
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
EBUSY;
> > + }
Yes, 0 is the success case. You must not have seen the explanation for
this code; it is needed to handle a race. If the suspend succeeds but
a wakeup request has already arrived, we need to undo the suspend.
Obviously, if the suspend failed then there's no need to
On Wed, 12 Jun 2013, Alan Stern wrote:
> On Wed, 12 Jun 2013, Manjunath Goudar wrote:
>
> > Suspend scenario in case of ohci-at91 glue was not properly
> > handled as it was not suspending generic part of ohci controller.
> > Calling explicitly the
}
>
> - return 0;
> + return ret;
If ohci_suspend() fails, we should return right away. Don't execute
the enable_irq_wake() and all the other stuff.
Alan Stern
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
and we don't
> + * want to encourage others to override these functions by making it
> + * too easy.
> + */
> +
> + ohci_s3c2410_hc_driver.hub_status_data = ohci_s3c2410_hub_status_data;
> + ohci_s3c2410_hc_driver.hub_control = ohci_s3c2410_hub_control;
Like in ohci-at91
unctions by making it
> + * too easy.
> + */
> +
> + ohci_at91_hc_driver.hub_status_data = ohci_at91_hub_status_data;
> + ohci_at91_hc_driver.hub_control = ohci_at91_hub_control;
Since the hub_status_data and hub_control routines aren't going to be
ex
those two routines. You can copy the
approach used by Stephen Warren in the ehci-tegra conversion.
Alan Stern
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
->next_statechange))
> msleep(5);
> ohci->next_statechange = jiffies;
>
> - spear_start_ohci(ohci_p);
> + clk_prepare_enable(sohci_p->clk);
You got an extra space character after the tab.
Alan Stern
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
alling ohci_run. The reset value of RWC is 0.
> + */
> + ohci->hc_control = OHCI_CTRL_RWC;
> + writel(OHCI_CTRL_RWC, &ohci->regs->control);
> + dev_dbg(hcd->self.controller, "starting OHCI controller\n");
> +
> + ohci_setup(hcd);
Don&
_stop(hcd);
> - omap_ohci_clock_power(0);
This last line seems to have gotten lost. You need to add it to
usb_hcd_omap_remove().
> @@ -354,8 +362,7 @@ static int usb_hcd_omap_probe (const struct hc_driver
> *driver,
> goto err2;
> }
>
hci->otg, &hcd->self);
>
> - exynos_ohci_phy_enable(exynos_ohci);
> + exynos_ohci_phy_enable(pdev);
This call will not work, because you don't set pdev's platform_data
until later. The call to platform_set_drvdata() must be moved before
this line.
>
>
On Mon, 3 Jun 2013, Manjunath Goudar wrote:
> This patch adds an ohci->priv field for private use by OHCI
> platform drivers.
>
> Until now none of the platform drivers has used this private space,
> but that's about to change in the next patch of this series
On Sun, 2 Jun 2013, Manjunath Goudar wrote:
> On 31 May 2013 19:37, Alan Stern wrote:
>
> > > + return ohci_setup(ohci_to_hcd(ohci));
> >
> > You don't need to use ohci_to_hcd(), because the hcd value is already a
> > parameter in this function.
&
I tried last (based on yesterday's linux-next) also
> had other issues and was very slow, so it may have been something
> different.
My test ran successfully, no problems. I guess we can ignore this.
Alan Stern
___
linaro-dev mailing list
linaro
On Fri, 31 May 2013, Manjunath Goudar wrote:
> On 30 May 2013 22:56, Alan Stern wrote:
>
> > On Thu, 30 May 2013, Manjunath Goudar wrote:
> >
> > > This patch adds an ohci->priv field for private use by OHCI
> > > platform drivers.
> > >
>
cd(), because the hcd value is already a
parameter in this function.
Alan Stern
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
as I'm concerned, this doesn't need to be in a patch of its own.
It can be merged into the 1/3 patch of the previous series. After all,
that patch adds the extra_priv_size field in ohci_driver_overrides,
which doesn't make much sense unless there's a way to refer to the
private
rm_overrides = {
> + .product_desc = "Generic Platform EHCI controller",
> + .reset =ohci_platform_reset,
> };
Don't forget the __initconst annotation. After that is fixed, you can
add my Acked-by.
Alan Stern
__
On Wed, 29 May 2013, Arnd Bergmann wrote:
> On Wednesday 29 May 2013 12:21:02 Alan Stern wrote:
> >
> > Those error messages are annoying; they don't use dev_err(), so they
> > don't include the device and driver names. There's no way to know what
> >
I rather suspect they come from the usbaudio driver.
When you ran this test, did you have commit 815fa7b91761 applied?
Does the same problem occur without Manjunath's changes?
Alan Stern
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
2/3.
> -Most of the generic ohci pci changes moved in 2/3 patch,now this is
> complete ohci-pci separation patch.
>
> V7:
> -Unrelated include file has been removed from ohci.h file.
>
> V8:
> -USB_OHCI_HCD_PCI symbol does not dependence on STB03xxx, PPC_MPC52xx an
setup RWC may not be set for add-in PCI cards.
> + * This transfers PCI PM wakeup capabilities.
> + */
> + if (device_can_wakeup(&pdev->dev))
> + ohci->hc_control |= OHCI_CTRL_RWC;
> return ret;
> }
Alan Stern
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
vice" argment instead of "pci_dev", then we use to_pci_dev()
> to get the "pci_dev" structure.
Acked-by: Alan Stern
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
rgot about:
> +/* ohci_setup routine for generic controller initialization */
> +
> +int ohci_setup(struct usb_hcd *hcd)
> +{
> + struct ohci_hcd *ohci = hcd_to_ohci(hcd);
> +
> + return ohci_init(ohci);
> +}
> +EXPORT_SYMBOL_GPL(o
left out one thing that should still be here. What happened
to the part about changing
#if !defined(PCI_DRIVER) && \
to
#if !ENABLED(CONFIG_USB_OHCI_HCD_PCI) &&\
?
Alan Stern
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
On Thu, 23 May 2013, Arnd Bergmann wrote:
> On Thursday 23 May 2013, Alan Stern wrote:
> > On Thu, 23 May 2013, Manjunath Goudar wrote:
>
> > Also, you left out one thing that should still be here. What happened
> > to the part about changing
> >
to_hcd(ohci)->self.controller;
Then in sb800_prefetch(), where it makes sense, you can call
to_pci_dev().
> @@ -580,6 +581,7 @@ static void td_submit_urb (
> struct urb *urb
> ) {
> struct urb_priv *urb_priv = urb->hcpriv;
> + struct pci_dev *pdev = t
ntroller rather than each having its own idiosyncratic approach.
> This allow to clean duplicated code in most of SOC driver
This patch looks good.
Acked-by: Alan Stern
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
s, usecs + 1000);
> hcd->state = HC_STATE_RUNNING;
>
> if (quirk_zfmicro(ohci)) {
Why not simply use msleep()?
Alan Stern
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
> the number is added to Kconfig. If selecting USB_GADGET_DEBUG_FILES
> this value may be set by a module parameter as well.
>
> Signed-off-by: Per Forlin
Acked-by: Alan Stern
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
ule parameter we already know what module it will be applied
to. Also, the new module parameter should be documented in the
comments near the start of the source file.
Maybe USB_DEBUG isn't the best option for enabling the module
parameter. As far as I'm concerned, Per can change
#ifdef CONFIG_USB_DEBUG
to
#if 0 /* Change to #if 1 to enable testing of this parameter */
Editing the source file once and rebuilding it is not any harder than
changing a Kconfig option and rebuilding the entire USB stack.
Alan Stern
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
ers unless there's a very
good reason for them.
Since people won't want to experiment with changing this value unless
they're debugging anyway, we might make it a conditional module
parameter -- don't define the parameter unless CONFIG_USB_DEBUG is
enabled.
Alan Stern
___
gt; the number is added to Kconfig.
>
> Signed-off-by: Per Forlin
> Acked-by: Michal Nazarewicz
Acked-by: Alan Stern
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
In what sense do the bursts affect performance? It makes sense that
having all the buffers empty from time to time would reduce throughput.
But as long as each buffer gets filled and queued before the previous
buffer has finished transmitting, it shouldn't matter how "bursty" the
On Mon, 8 Aug 2011, Per Forlin wrote:
> On 8 August 2011 20:45, Alan Stern wrote:
> > On Mon, 8 Aug 2011, Per Forlin wrote:
> >
> >> > Okay, 6% is a worthwhile improvement, though not huge. �Did you try 6
> >> > or 8 buffers? �I bet going beyond 4 makes ve
pecific information.
>
> +config USB_GADGET_STORAGE_NUM_BUFFERS
> + int "Number of storage pipline buffers"
> + range 2 64
64 is definitely way into the overkill range. I'd be very tempted to
leave the maximum set at 4. Y
51 matches
Mail list logo
