Re: [RFC][USB] powerpc: Workaround for the PPC440EPX USBH_23 errata [take 2]

2008-10-14 Thread Alan Stern
On Tue, 14 Oct 2008, Vitaly Bordug wrote:

 A published errata for ppc440epx states, that when running Linux with both
 EHCI and OHCI modules loaded, the EHCI module experiences a fatal error
 when a high-speed device is connected to the USB2.0, and functions normally
 if OHCI module is not loaded.
 
 There used to be recommendation to use only hi-speed or full-speed
 devices with specific conditions, when respective module was unloaded.
 Later, it was observed that ohci suspend is enough to keep things
 going, and it was turned into workaround, as explained below.

Have you tried building this on a non-PPC440EPx system?  It looks like 
this patch would cause a compile error.

Alan Stern

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [RFC][USB] powerpc: Workaround for the PPC440EPX USBH_23 errata [take 2]

2008-10-14 Thread Vitaly Bordug

 Have you tried building this on a non-PPC440EPx system?  It looks
 like this patch would cause a compile error.
yes, correct, last minute fix didn't make it into the patch, sorry
about that.

Will refresh, test and resubmit.

-Vitaly
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [RFC][USB] powerpc: Workaround for the PPC440EPX USBH_23 errata

2008-09-06 Thread Vitaly Bordug
В Fri, 5 Sep 2008 11:17:03 -0400 (EDT)
Alan Stern [EMAIL PROTECTED] пишет:

 On Fri, 5 Sep 2008, Vitaly Bordug wrote:
 
Not every hub will work (none of available did so far), and it
is often not an option for embedded device without rewiring.
   
   It's odd that your hubs don't work.  What's wrong with them?
   
  
  well, they do not have transaction translators then. Nothing really
  wrong
 
 No.  If the hubs run at high speed then they must have transaction 
 translators; it's required by the USB 2.0 specification.
 
 Did you try using them with ohci-hcd not loaded?

yes, not with every unit though. That seems to be usb hub issue then.


Anyway, the patch is targeted more to
the embedded platforms, that cannot afford work the issue around with
extra devices.

 
 Alan Stern
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [RFC][USB] powerpc: Workaround for the PPC440EPX USBH_23 errata

2008-09-05 Thread Vitaly Bordug
В Thu, 4 Sep 2008 17:40:49 -0400 (EDT)
Alan Stern [EMAIL PROTECTED] пишет:

 On Fri, 5 Sep 2008, Vitaly Bordug wrote:
 
  I've started looking this way. Sorry, but this approach is a little
  bit fragile, and unreliable.
  
  First, it just did not work out in case of usb2 hub with memory
  stick and keybord plugged - OHCI did not suspend, ehci is hosed and
  we're happily using hi-speed device on full-speed:
 
  Secondly, we may *not* rely on the fact, that OHCI will always have
  the same suspend policy. Even kicking the code up to the shape when
  it will automagically suspend in proper timing to get the HW issue
  around, we cannot be sure that it will persist along kernel
  lifecycle, and won't require concerned people to kick suspend
  timings back to the working state subsequently each rc release.
  
  Thirdly, PM is disabled by Kconfig explicitly in case of 44x.
  Reasoning is not clear at the moment, but I believe that isn't
  there just in case.
 
 I assume that's the reason the suggested approach failed.

Oh geez. I may miss some small point, but dunno what made me appear
*that* dense. The kernel was properly rebuilt with Kconfig fixed and
_PM set.
 
   What to do when CONFIG_PM is off is a separate matter.  Let's not
   worry about it for now -- especially since, as Matthias suggested,
   you can use a USB 2.0 hub.
  
  Not every hub will work (none of available did so far), and it is
  often not an option for embedded device without rewiring.
 
 It's odd that your hubs don't work.  What's wrong with them?
 

well, they do not have transaction translators then. Nothing really
wrong
  As this touches powerpc stuff only, are there any objections to let
  powerpc peolple consider if approach suggested earlier is
  applicable or not?
 
 I don't mind doing that, provided the changes are cleaned up so that 
 they don't affect people who aren't building kernels for 44x systems.
OK, thanks for review and comments...

--
Sincerely,

Vitaly
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [RFC][USB] powerpc: Workaround for the PPC440EPX USBH_23 errata

2008-09-05 Thread Alan Stern
On Fri, 5 Sep 2008, Vitaly Bordug wrote:

   Not every hub will work (none of available did so far), and it is
   often not an option for embedded device without rewiring.
  
  It's odd that your hubs don't work.  What's wrong with them?
  
 
 well, they do not have transaction translators then. Nothing really
 wrong

No.  If the hubs run at high speed then they must have transaction 
translators; it's required by the USB 2.0 specification.

Did you try using them with ohci-hcd not loaded?

Alan Stern

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [RFC][USB] powerpc: Workaround for the PPC440EPX USBH_23 errata

2008-09-04 Thread Vitaly Bordug
В Fri, 29 Aug 2008 17:30:57 -0400 (EDT)
Alan Stern [EMAIL PROTECTED] пишет:

 On Fri, 29 Aug 2008, Vitaly Bordug wrote:
 
  But even assuming PM set, common use-case of
  embedded systems to have stuff on USB bus that is never plugged off;
  and in case of compiled-in ehci and ohci there is just 
  
  Initializing USB Mass Storage driver...
  usb 1-1: new high speed USB device using ppc-of-ehci and address 2
  usb 1-1: device descriptor read/all, error -110
  hub 1-0:1.0: unable to enumerate USB device on port 1
  
  (not touching PM here to be clear)
  
  even 1-second delay would be enough for root hub to be hosed... So,
  is the patch (with all the issues addressed) acceptable for
  mainline?
 
 Try doing this instead.  First, make sure CONFIG_PM is set!  :-)
 Then replace your patch with something that adds a 2-second delay to 
 the end of ehci_start() when running on a 440EPx system.  That should 
 give the OHCI controller time to suspend before the EHCI root hub is 
 registered.
 
I've started looking this way. Sorry, but this approach is a little bit
fragile, and unreliable.

First, it just did not work out in case of usb2 hub with memory stick
and keybord plugged - OHCI did not suspend, ehci is hosed and we're
happily using hi-speed device on full-speed:

ppc-of-ehci e300.ehci: OF EHCI
ppc-of-ehci e300.ehci: new USB bus registered, assigned bus number 1
ppc-of-ehci e300.ehci: irq 32, io mem 0xe300
ppc-of-ehci e300.ehci: USB 2.0 started, EHCI 1.00, driver 10 Dec
2004 usb usb1: configuration #1 chosen from 1 choice
hub 1-0:1.0: USB hub found
hub 1-0:1.0: 1 port detected
usb usb1: New USB device found, idVendor=1d6b, idProduct=0002
usb usb1: New USB device strings: Mfr=3, Product=2, SerialNumber=1
usb usb1: Product: OF EHCI
usb usb1: Manufacturer: Linux 2.6.26-00016-g8914f6a-dirty ehci_hcd
usb usb1: SerialNumber: PPC-OF USB
ppc-of-ohci e400.usb: OF OHCI
ppc-of-ohci e400.usb: new USB bus registered, assigned bus number 2
ppc-of-ohci e400.usb: irq 33, io mem 0xe400
usb usb2: configuration #1 chosen from 1 choice
hub 2-0:1.0: USB hub found
hub 2-0:1.0: 1 port detected
usb usb2: New USB device found, idVendor=1d6b, idProduct=0001
usb usb2: New USB device strings: Mfr=3, Product=2, SerialNumber=1
usb usb2: Product: OF OHCI
usb usb2: Manufacturer: Linux 2.6.26-00016-g8914f6a-dirty ohci_hcd
usb usb2: SerialNumber: PPC-OF USB
Initializing USB Mass Storage driver...
usb 1-1: new high speed USB device using ppc-of-ehci and address 2
usb 1-1: device not accepting address 2, error -110
usb 1-1: new high speed USB device using ppc-of-ehci and address 3
usb 1-1: device not accepting address 3, error -110
usb 1-1: new high speed USB device using ppc-of-ehci and address 4
usb 1-1: device not accepting address 4, error -110
usb 1-1: new high speed USB device using ppc-of-ehci and address 5
usb 1-1: device not accepting address 5, error -110
hub 1-0:1.0: unable to enumerate USB device on port 1
usb 2-1: new full speed USB device using ppc-of-ohci and address 2

Secondly, we may *not* rely on the fact, that OHCI will always have the
same suspend policy. Even kicking the code up to the shape when it will
automagically suspend in proper timing to get the HW issue around, we
cannot be sure that it will persist along kernel lifecycle, and won't
require concerned people to kick suspend timings back to the working
state subsequently each rc release.

Thirdly, PM is disabled by Kconfig explicitly in case of 44x. Reasoning
is not clear at the moment, but I believe that isn't there just in case.


 A little more tweaking will be needed to handle system sleeps.  But 
 this should be a good start.

Getting 44x into proper PM land is good direction, but right now there
is a situation for certain platform when USB trimmed down to full-speed
mode in evaluation design and many inheritances. The only way to have
all the benefits, was maintaining internally some version of AMCC
workaorund
 
 What to do when CONFIG_PM is off is a separate matter.  Let's not
 worry about it for now -- especially since, as Matthias suggested,
 you can use a USB 2.0 hub.

Not every hub will work (none of available did so far), and it is often
not an option for embedded device without rewiring.

As this touches powerpc stuff only, are there any objections to let
powerpc peolple consider if approach suggested earlier is applicable or
not?

Thanks,

-Vitaly
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [RFC][USB] powerpc: Workaround for the PPC440EPX USBH_23 errata

2008-09-04 Thread Alan Stern
On Fri, 5 Sep 2008, Vitaly Bordug wrote:

 I've started looking this way. Sorry, but this approach is a little bit
 fragile, and unreliable.
 
 First, it just did not work out in case of usb2 hub with memory stick
 and keybord plugged - OHCI did not suspend, ehci is hosed and we're
 happily using hi-speed device on full-speed:

 Secondly, we may *not* rely on the fact, that OHCI will always have the
 same suspend policy. Even kicking the code up to the shape when it will
 automagically suspend in proper timing to get the HW issue around, we
 cannot be sure that it will persist along kernel lifecycle, and won't
 require concerned people to kick suspend timings back to the working
 state subsequently each rc release.
 
 Thirdly, PM is disabled by Kconfig explicitly in case of 44x. Reasoning
 is not clear at the moment, but I believe that isn't there just in case.

I assume that's the reason the suggested approach failed.

  What to do when CONFIG_PM is off is a separate matter.  Let's not
  worry about it for now -- especially since, as Matthias suggested,
  you can use a USB 2.0 hub.
 
 Not every hub will work (none of available did so far), and it is often
 not an option for embedded device without rewiring.

It's odd that your hubs don't work.  What's wrong with them?

 As this touches powerpc stuff only, are there any objections to let
 powerpc peolple consider if approach suggested earlier is applicable or
 not?

I don't mind doing that, provided the changes are cleaned up so that 
they don't affect people who aren't building kernels for 44x systems.

Alan Stern

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [RFC][USB] powerpc: Workaround for the PPC440EPX USBH_23 errata

2008-09-04 Thread Benjamin Herrenschmidt

 I assume that's the reason the suggested approach failed.

I think (hope) that Vitaly actually fixed that before trying it :-)

 I don't mind doing that, provided the changes are cleaned up so that 
 they don't affect people who aren't building kernels for 44x systems.

I also find the CONFIG_PM approach not terribly reliable... too timing
sensitive, and it forces boards to add CONFIG_PM which is not
necessarily the nicest thing to do in embedded space.

Sounds better to me to clean up the initial patch to ensure it's not
going to introduce bloat for other platforms and use that, ie just
consider it as yet another HW errata.

Cheers,
Ben.

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [RFC][USB] powerpc: Workaround for the PPC440EPX USBH_23 errata

2008-08-29 Thread Steven A. Falco
Alan Stern wrote:
 On Thu, 28 Aug 2008, Steven A. Falco wrote:
 
 Alan Stern wrote:
 Your original post mentioned that the 440EPx has only one USB 2.0 host
 port.  Then how can you use a keyboard and memory stick at the same
 time?  You'd have to plug them into a hub -- in which case only one
 controller would be needed, the one driving the hub.  The patch would 
 be unnecessary.
 I have one of these processors on a Sequoia board.  What happens is that
 if you build the kernel with both EHCI and OHCI support, then plug in
 a modern USB memory stick, it initially tries EHCI, the driver fails, and
 the whole thing falls back to OHCI.  So you wind up running at 12 Mbps.
 The only way to make high speed work is to turn off the OHCI driver, and
 then you cannot support slow devices with that kernel.

 So, hile you cannot plug two devices in at one time, you can plug in
 different speed devices at different times, and my understanding is that
 this patch will let that work seamlessly.
 
 Is there some reason why it doesn't work already?  All the patch does
 is suspend the OHCI root hub when you plug in the memory stick -- but
 the root hub should already be suspended.
 
 Unless the memory stick is already plugged in when the kernel boots.  
 In which case the root hub won't be suspended -- it won't suspend until 
 1 second after ohci-hcd initializes the controller.  Is that the 
 scenario you're worried about?
Not sure about other users, but in my case, yes.  I was using a USB stick
as the root filesystem.  When the kernel first came up, it would try the
EHCI driver, which would fail.  Then the drive would be recognized by the
OHCI driver, but would only run at full (12 Mbps) speed rather than high
speed.

I've since gone to a directly attached CF card on the processor external
bus, and I'm not using USB anymore.  But I think the patch is still
desirable to work around the processor bug.

Steve

 
 Alan Stern
 
 

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [RFC][USB] powerpc: Workaround for the PPC440EPX USBH_23 errata

2008-08-29 Thread Alan Stern
On Fri, 29 Aug 2008, Benjamin Herrenschmidt wrote:

 I suppose some embedded platforms don't use CONFIG_PM, is this still a
 requirement for autosuspend ? Or do that happen always on an empty port
 nowadays ?

ohci-hcd doesn't automatically suspend the root hub if CONFIG_PM isn't
set.  However it isn't necessary to set CONFIG_SLEEP,
CONFIG_HIBERNATION, or CONFIG_USB_SUSPEND.

Alan Stern

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [RFC][USB] powerpc: Workaround for the PPC440EPX USBH_23 errata

2008-08-29 Thread Matthias Fuchs
Hi,

please see my comment below. It might be helpful for some of you.

On Thursday 28 August 2008 23:11:51 Steven A. Falco wrote:
 Alan Stern wrote:
  Your original post mentioned that the 440EPx has only one USB 2.0 host
  port.  Then how can you use a keyboard and memory stick at the same
  time?  You'd have to plug them into a hub -- in which case only one
  controller would be needed, the one driving the hub.  The patch would
  be unnecessary.

 I have one of these processors on a Sequoia board.  What happens is that
 if you build the kernel with both EHCI and OHCI support, then plug in
 a modern USB memory stick, it initially tries EHCI, the driver fails, and
 the whole thing falls back to OHCI.  So you wind up running at 12 Mbps.
 The only way to make high speed work is to turn off the OHCI driver, and
 then you cannot support slow devices with that kernel.
There is a simple trick to get all types (full, low ang high speed) of  
devices work: use a USB hub with transaction translators. We are doing so on 
a custom design based on the 440EPx. So we only build the kernel with EHCI 
support and there was no need for this ugly hack.

 So, hile you cannot plug two devices in at one time, you can plug in
 different speed devices at different times, and my understanding is that
 this patch will let that work seamlessly.
That's true.

Matthias
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [RFC][USB] powerpc: Workaround for the PPC440EPX USBH_23 errata

2008-08-29 Thread Vitaly Bordug
В Fri, 29 Aug 2008 11:26:07 -0400 (EDT)
Alan Stern [EMAIL PROTECTED] пишет:

 On Fri, 29 Aug 2008, Benjamin Herrenschmidt wrote:
 
  I suppose some embedded platforms don't use CONFIG_PM, is this
  still a requirement for autosuspend ? Or do that happen always on
  an empty port nowadays ?
 
 ohci-hcd doesn't automatically suspend the root hub if CONFIG_PM isn't
 set.  However it isn't necessary to set CONFIG_SLEEP,
 CONFIG_HIBERNATION, or CONFIG_USB_SUSPEND.

That is good to know. 

But even assuming PM set, common use-case of
embedded systems to have stuff on USB bus that is never plugged off;
and in case of compiled-in ehci and ohci there is just 

Initializing USB Mass Storage driver...
usb 1-1: new high speed USB device using ppc-of-ehci and address 2
usb 1-1: device descriptor read/all, error -110
hub 1-0:1.0: unable to enumerate USB device on port 1

(not touching PM here to be clear)

even 1-second delay would be enough for root hub to be hosed... So, is
the patch (with all the issues addressed) acceptable for mainline?

Thanks, 
-Vitaly
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [RFC][USB] powerpc: Workaround for the PPC440EPX USBH_23 errata

2008-08-29 Thread Alan Stern
On Fri, 29 Aug 2008, Vitaly Bordug wrote:

 But even assuming PM set, common use-case of
 embedded systems to have stuff on USB bus that is never plugged off;
 and in case of compiled-in ehci and ohci there is just 
 
 Initializing USB Mass Storage driver...
 usb 1-1: new high speed USB device using ppc-of-ehci and address 2
 usb 1-1: device descriptor read/all, error -110
 hub 1-0:1.0: unable to enumerate USB device on port 1
 
 (not touching PM here to be clear)
 
 even 1-second delay would be enough for root hub to be hosed... So, is
 the patch (with all the issues addressed) acceptable for mainline?

Try doing this instead.  First, make sure CONFIG_PM is set!  :-)
Then replace your patch with something that adds a 2-second delay to 
the end of ehci_start() when running on a 440EPx system.  That should 
give the OHCI controller time to suspend before the EHCI root hub is 
registered.

A little more tweaking will be needed to handle system sleeps.  But 
this should be a good start.

What to do when CONFIG_PM is off is a separate matter.  Let's not worry 
about it for now -- especially since, as Matthias suggested, you can 
use a USB 2.0 hub.

Alan Stern


___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [RFC][USB] powerpc: Workaround for the PPC440EPX USBH_23 errata

2008-08-28 Thread Vitaly Bordug
В Wed, 27 Aug 2008 10:36:20 -0400 (EDT)
Alan Stern [EMAIL PROTECTED] пишет:

 On Wed, 27 Aug 2008, Vitaly Bordug wrote:
 
  A published errata for ppc440epx states, that when running Linux
  with both EHCI and OHCI modules loaded, the EHCI module experiences
  a fatal error when a high-speed device is connected to the USB2.0,
  and functions normally if OHCI module is not loaded. 
  
  Quote from original descriprion:
  
  The 440EPx USB 2.0 Host controller is an EHCI compliant
  controller.  In USB 2.0 Host controllers, each EHCI controller has
  one or more companion controllers, which may be OHCI or UHCI.  An
  USB 2.0 Host controller will contain one or more ports.  For each
  port, only one of the controllers is connected at any one time. In
  the 440EPx, there is only one OHCI companion controller, and only
  one USB 2.0 Host port. All ports on an USB 2.0 controller default
  to the companion controller.  If you load only an ohci driver, it
  will have control of the ports and any deviceplugged in will
  operate, although high speed devices will be forced to operate at
  full speed.  When an ehci driver is loaded, it explicitly takes
  control of the ports.  If there is a device connected, and / or
  every time there is a new device connected, the ehci driver
  determines if the device is high speed or not.  If it is high
  speed, the driver retains control of the port.  If it is not, the
  driver explicitly gives the companion controller control of the
  port.
 
 This doesn't explain why the fatal error occurs.
 
On certain 44x set of SoCs, only one controller is able to function,
e.g. technically they are mutually exclusive.

There used to be recommendation to use only hi-speed or full-speed
devices with specific conditions, when respective module was unloaded.
Later, it was observed that ohci suspend is enough to keep things
going, and it was turned into workaround, as explained below.

  The is a software workaround that uses 
  Initial version of the software workaround was posted to
  linux-usb-devel:
  
  http://www.mail-archive.com/[EMAIL PROTECTED]/msg54019.html
  
  and later available from amcc.com:
  http://www.amcc.com/Embedded/Downloads/download.html?cat=1family=15ins=2
  
  The patch below is generally based on the latter, but reworked to
  powerpc/of_device USB drivers, and uses a few devicetree inquiries
  to get rid of all the hardcoded defines and CONFIG_* stuff, in
  favor to defining specific quirk. The latter required to add more
  accurate description into compatible field of USB node for
  'sequioia' board. 
 
 I have some doubts about parts of this patch.
What stands about noted gotchas, I agree and will fix them, thanks for
looking through the patch. I agree this doesn't look pleasant, but
unfortunately is the only way to say use USB keyboard, and hi-speed
memory stick at the same time.


 
  --- a/drivers/usb/host/ehci-ppc-of.c
  +++ b/drivers/usb/host/ehci-ppc-of.c
  @@ -107,16 +107,17 @@ ehci_hcd_ppc_of_probe(struct of_device *op,
  const struct of_device_id *match) {
  struct device_node *dn = op-node;
  struct usb_hcd *hcd;
  -   struct ehci_hcd *ehci;
  +   struct ehci_hcd *ehci = NULL;
  struct resource res;
  int irq;
  int rv;
   
  +   struct device_node *np;
  +
  if (usb_disabled())
  return -ENODEV;
   
  dev_dbg(op-dev, initializing PPC-OF USB Controller\n);
  -
  rv = of_address_to_resource(dn, 0, res);
  if (rv)
  return rv;
  @@ -125,6 +126,7 @@ ehci_hcd_ppc_of_probe(struct of_device *op,
  const struct of_device_id *match) if (!hcd)
  return -ENOMEM;
   
  +
 
 Is there any reason for these apparently gratuitous whitespace
 changes?
 
  hcd-rsrc_start = res.start;
  hcd-rsrc_len = res.end - res.start + 1;
   
  @@ -172,6 +174,21 @@ ehci_hcd_ppc_of_probe(struct of_device *op,
  const struct of_device_id *match) rv ? NOT : );
  }
   
  +   np = of_find_compatible_node(NULL, NULL,
  ibm,usb-ohci-440epx);
  +   if (np != NULL) {
  +   /* claim we really affected by usb23 erratum */
  +   ehci-has_amcc_usb23 = 1;
  +   if (!of_address_to_resource(np, 0, res))
  +   ehci-ohci_hcctrl_reg = ioremap(res.start +
  +   OHCI_HCCTRL_OFFSET,
  OHCI_HCCTRL_LEN);
  +   else
  +   pr_debug(__FILE__ : no ohci offset in
  fdt\n);
  +   if (!ehci-ohci_hcctrl_reg) {
  +   pr_debug(__FILE__ : ioremap for ohci
  hcctrl failed\n);
  +   return -ENOMEM;
 
 This is a memory leak.
 
  --- a/drivers/usb/host/ehci.h
  +++ b/drivers/usb/host/ehci.h
  @@ -809,6 +819,20 @@ static inline u32 hc32_to_cpup (const struct
  ehci_hcd *ehci, const __hc32 *x) le32_to_cpup((__force __le32 *)x);
   }
   
  +static inline void set_ohci_hcfs(struct ehci_hcd *ehci, int
  operational) +{
  +   u32 hc_control;
  +
  +   hc_control = (readl_be(ehci-ohci_hcctrl_reg) 
  ~OHCI_CTRL_HCFS);
  +   if 

Re: [RFC][USB] powerpc: Workaround for the PPC440EPX USBH_23 errata

2008-08-28 Thread Alan Stern
On Thu, 28 Aug 2008, Vitaly Bordug wrote:

  This doesn't explain why the fatal error occurs.
  
 On certain 44x set of SoCs, only one controller is able to function,
 e.g. technically they are mutually exclusive.
 
 There used to be recommendation to use only hi-speed or full-speed
 devices with specific conditions, when respective module was unloaded.
 Later, it was observed that ohci suspend is enough to keep things
 going, and it was turned into workaround, as explained below.

Okay, good.  _This_ is what you should have put in the patch 
description, instead of all that other stuff.

  I have some doubts about parts of this patch.
 What stands about noted gotchas, I agree and will fix them, thanks for
 looking through the patch. I agree this doesn't look pleasant, but
 unfortunately is the only way to say use USB keyboard, and hi-speed
 memory stick at the same time.

Your original post mentioned that the 440EPx has only one USB 2.0 host
port.  Then how can you use a keyboard and memory stick at the same
time?  You'd have to plug them into a hub -- in which case only one
controller would be needed, the one driving the hub.  The patch would 
be unnecessary.

  I doubt this will interact properly with usbcore and the rest of
  ohci-hcd.  They do not expect the root hub to be suspended unless they
  know about it.
 
 I need to reemphasize, that upper is not normal, but unfortunately
 the only way to have both full and hi-speed usb live in peace with
 44xEPX family. Quirks are not going to be executed on other chip
 anyway, and on chip in question, it was tested and works stable enough.
 
 I can add an explicit warning, that workaround is being utilised, to
 make things clear if it will be considered appropriate.

Do these systems support CONFIG_PM?  If they do then your patch
shouldn't be needed, because then ohci-hcd will automatically suspend
the OHCI root hub 1 second after the last full/low-speed device is
unplugged.  You could reduce that 1 second value if you wanted to 
decrease the probability of conflicts.

Alan Stern

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [RFC][USB] powerpc: Workaround for the PPC440EPX USBH_23 errata

2008-08-28 Thread Steven A. Falco
Alan Stern wrote:
 Your original post mentioned that the 440EPx has only one USB 2.0 host
 port.  Then how can you use a keyboard and memory stick at the same
 time?  You'd have to plug them into a hub -- in which case only one
 controller would be needed, the one driving the hub.  The patch would 
 be unnecessary.
I have one of these processors on a Sequoia board.  What happens is that
if you build the kernel with both EHCI and OHCI support, then plug in
a modern USB memory stick, it initially tries EHCI, the driver fails, and
the whole thing falls back to OHCI.  So you wind up running at 12 Mbps.
The only way to make high speed work is to turn off the OHCI driver, and
then you cannot support slow devices with that kernel.

So, hile you cannot plug two devices in at one time, you can plug in
different speed devices at different times, and my understanding is that
this patch will let that work seamlessly.
 
 I doubt this will interact properly with usbcore and the rest of
 ohci-hcd.  They do not expect the root hub to be suspended unless they
 know about it.
 I need to reemphasize, that upper is not normal, but unfortunately
 the only way to have both full and hi-speed usb live in peace with
 44xEPX family. Quirks are not going to be executed on other chip
 anyway, and on chip in question, it was tested and works stable enough.

 I can add an explicit warning, that workaround is being utilised, to
 make things clear if it will be considered appropriate.
 
 Do these systems support CONFIG_PM?  If they do then your patch
 shouldn't be needed, because then ohci-hcd will automatically suspend
 the OHCI root hub 1 second after the last full/low-speed device is
 unplugged.  You could reduce that 1 second value if you wanted to 
 decrease the probability of conflicts.
 
 Alan Stern
 
 ___
 Linuxppc-dev mailing list
 Linuxppc-dev@ozlabs.org
 https://ozlabs.org/mailman/listinfo/linuxppc-dev
 

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [RFC][USB] powerpc: Workaround for the PPC440EPX USBH_23 errata

2008-08-28 Thread Alan Stern
On Thu, 28 Aug 2008, Steven A. Falco wrote:

 Alan Stern wrote:
  Your original post mentioned that the 440EPx has only one USB 2.0 host
  port.  Then how can you use a keyboard and memory stick at the same
  time?  You'd have to plug them into a hub -- in which case only one
  controller would be needed, the one driving the hub.  The patch would 
  be unnecessary.
 I have one of these processors on a Sequoia board.  What happens is that
 if you build the kernel with both EHCI and OHCI support, then plug in
 a modern USB memory stick, it initially tries EHCI, the driver fails, and
 the whole thing falls back to OHCI.  So you wind up running at 12 Mbps.
 The only way to make high speed work is to turn off the OHCI driver, and
 then you cannot support slow devices with that kernel.
 
 So, hile you cannot plug two devices in at one time, you can plug in
 different speed devices at different times, and my understanding is that
 this patch will let that work seamlessly.

Is there some reason why it doesn't work already?  All the patch does
is suspend the OHCI root hub when you plug in the memory stick -- but
the root hub should already be suspended.

Unless the memory stick is already plugged in when the kernel boots.  
In which case the root hub won't be suspended -- it won't suspend until 
1 second after ohci-hcd initializes the controller.  Is that the 
scenario you're worried about?

Alan Stern

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [RFC][USB] powerpc: Workaround for the PPC440EPX USBH_23 errata

2008-08-28 Thread Benjamin Herrenschmidt
On Thu, 2008-08-28 at 17:33 -0400, Alan Stern wrote:
 Is there some reason why it doesn't work already?  All the patch does
 is suspend the OHCI root hub when you plug in the memory stick -- but
 the root hub should already be suspended.
 
 Unless the memory stick is already plugged in when the kernel boots.  
 In which case the root hub won't be suspended -- it won't suspend
 until 
 1 second after ohci-hcd initializes the controller.  Is that the 
 scenario you're worried about?

I suppose some embedded platforms don't use CONFIG_PM, is this still a
requirement for autosuspend ? Or do that happen always on an empty port
nowadays ?

Cheers,
Ben.


___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [RFC][USB] powerpc: Workaround for the PPC440EPX USBH_23 errata

2008-08-27 Thread Alan Stern
On Wed, 27 Aug 2008, Vitaly Bordug wrote:

 A published errata for ppc440epx states, that when running Linux with both
 EHCI and OHCI modules loaded, the EHCI module experiences a fatal error 
 when a high-speed device is connected to the USB2.0, and functions normally
 if OHCI module is not loaded. 
 
 Quote from original descriprion:
 
 The 440EPx USB 2.0 Host controller is an EHCI compliant controller.  In USB
 2.0 Host controllers, each EHCI controller has one or more companion
 controllers, which may be OHCI or UHCI.  An USB 2.0 Host controller will
 contain one or more ports.  For each port, only one of the controllers is
 connected at any one time. In the 440EPx, there is only one OHCI companion 
 controller, 
 and only one USB 2.0 Host port.
 All ports on an USB 2.0 controller default to the companion controller.  If
 you load only an ohci driver, it will have control of the ports and any
 deviceplugged in will operate, although high speed devices will be forced to
 operate at full speed.  When an ehci driver is loaded, it explicitly takes 
 control
 of the ports.  If there is a device connected, and / or every time there is a
 new device connected, the ehci driver determines if the device is high speed 
 or
 not.  If it is high speed, the driver retains control of the port.  If it
 is not, the driver explicitly gives the companion controller control of the
 port.

This doesn't explain why the fatal error occurs.

 The is a software workaround that uses 
 Initial version of the software workaround was posted to linux-usb-devel:
 
 http://www.mail-archive.com/[EMAIL PROTECTED]/msg54019.html
 
 and later available from amcc.com:
 http://www.amcc.com/Embedded/Downloads/download.html?cat=1family=15ins=2
 
 The patch below is generally based on the latter, but reworked to
 powerpc/of_device USB drivers, and uses a few devicetree inquiries to get
 rid of all the hardcoded defines and CONFIG_* stuff, in favor to
 defining specific quirk. The latter required to add more accurate
 description into compatible field of USB node for 'sequioia' board. 

I have some doubts about parts of this patch.

 --- a/drivers/usb/host/ehci-ppc-of.c
 +++ b/drivers/usb/host/ehci-ppc-of.c
 @@ -107,16 +107,17 @@ ehci_hcd_ppc_of_probe(struct of_device *op, const 
 struct of_device_id *match)
  {
   struct device_node *dn = op-node;
   struct usb_hcd *hcd;
 - struct ehci_hcd *ehci;
 + struct ehci_hcd *ehci = NULL;
   struct resource res;
   int irq;
   int rv;
  
 + struct device_node *np;
 +
   if (usb_disabled())
   return -ENODEV;
  
   dev_dbg(op-dev, initializing PPC-OF USB Controller\n);
 -
   rv = of_address_to_resource(dn, 0, res);
   if (rv)
   return rv;
 @@ -125,6 +126,7 @@ ehci_hcd_ppc_of_probe(struct of_device *op, const struct 
 of_device_id *match)
   if (!hcd)
   return -ENOMEM;
  
 +

Is there any reason for these apparently gratuitous whitespace changes?

   hcd-rsrc_start = res.start;
   hcd-rsrc_len = res.end - res.start + 1;
  
 @@ -172,6 +174,21 @@ ehci_hcd_ppc_of_probe(struct of_device *op, const struct 
 of_device_id *match)
   rv ? NOT : );
   }
  
 + np = of_find_compatible_node(NULL, NULL, ibm,usb-ohci-440epx);
 + if (np != NULL) {
 + /* claim we really affected by usb23 erratum */
 + ehci-has_amcc_usb23 = 1;
 + if (!of_address_to_resource(np, 0, res))
 + ehci-ohci_hcctrl_reg = ioremap(res.start +
 + OHCI_HCCTRL_OFFSET, OHCI_HCCTRL_LEN);
 + else
 + pr_debug(__FILE__ : no ohci offset in fdt\n);
 + if (!ehci-ohci_hcctrl_reg) {
 + pr_debug(__FILE__ : ioremap for ohci hcctrl failed\n);
 + return -ENOMEM;

This is a memory leak.

 --- a/drivers/usb/host/ehci.h
 +++ b/drivers/usb/host/ehci.h
 @@ -809,6 +819,20 @@ static inline u32 hc32_to_cpup (const struct ehci_hcd 
 *ehci, const __hc32 *x)
   le32_to_cpup((__force __le32 *)x);
  }
  
 +static inline void set_ohci_hcfs(struct ehci_hcd *ehci, int operational)
 +{
 + u32 hc_control;
 +
 + hc_control = (readl_be(ehci-ohci_hcctrl_reg)  ~OHCI_CTRL_HCFS);
 + if (operational)
 + hc_control |= OHCI_USB_OPER;
 + else
 + hc_control |= OHCI_USB_SUSPEND;
 +
 + writel_be(hc_control, ehci-ohci_hcctrl_reg);
 + (void) readl_be(ehci-ohci_hcctrl_reg);
 +}
 +

This should be protected by a preprocessor test so that the code 
doesn't get compiled on architectures that don't need it.

 --- a/drivers/usb/host/ohci-ppc-of.c
 +++ b/drivers/usb/host/ohci-ppc-of.c
 @@ -148,6 +148,31 @@ ohci_hcd_ppc_of_probe(struct of_device *op, const struct 
 of_device_id *match)
   if (rv == 0)
   return 0;
  
 + /* by now, 440epx is known to show usb_23 erratum */
 + np = of_find_compatible_node(NULL, NULL,