RE: [PATCH v5 02/46] usb: gadget: add endpoint capabilities flags

2015-07-31 Thread David Laight
From: Robert Baldyga
 Sent: 31 July 2015 15:00
 Introduce struct usb_ep_caps which contains information about capabilities
 of usb endpoints - supported transfer types and directions. This structure
 should be filled by UDC driver for each of its endpoints, and will be
 used in epautoconf in new ep matching mechanism which will replace ugly
 guessing of endpoint capabilities basing on its name.
 
 Signed-off-by: Robert Baldyga r.bald...@samsung.com
 ---
  include/linux/usb/gadget.h | 21 +
  1 file changed, 21 insertions(+)
 
 diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
 index 68fb5e8..a9a4959 100644
 --- a/include/linux/usb/gadget.h
 +++ b/include/linux/usb/gadget.h
 @@ -141,10 +141,29 @@ struct usb_ep_ops {
  };
...
 +struct usb_ep_caps {
 + unsigned type_control:1;
 + unsigned type_iso:1;
 + unsigned type_bulk:1;
 + unsigned type_int:1;
 + unsigned dir_in:1;
 + unsigned dir_out:1;
 +};

With the way this is used (eg below from 13/46)

+
+   if (i == 0) {
+   ep-ep.caps.type_control = true;
+   } else {
+   ep-ep.caps.type_iso = true;
+   ep-ep.caps.type_bulk = true;
+   ep-ep.caps.type_int = true;
+   }
+
+   ep-ep.caps.dir_in = true;
+   ep-ep.caps.dir_out = true;

I think it would be more obvious if you used a u8 and explicit bitmasks.
The initialisation (as above) would the be explicitly assigning 'not supported'
to the other fields.
The compiler will also generate much better code...

David



RE: [PATCH] ARM: Blacklist GCC 4.8.0 to GCC 4.8.2 - PR58854

2014-10-16 Thread David Laight
From: Russell King
 These stock GCC versions miscompile the kernel by incorrectly optimising
 the function epilogue code - by first increasing the stack pointer, and
 then loading entries from below the stack.  This means that an opportune
 interrupt or exception will corrupt the current function's saved state,
 which may result in the parent function seeing different register
 values.
 
 As this bug has been known to result in corrupted filesystems, and these
 buggy compiler versions seem to be frequently used, we have little
 option but to blacklist these compiler versions.
 
 Distributions may have fixed PR58854, but as their compilers are totally
 indistinguishable from the buggy versions, it is unfortunate that this
 also results in those also being blacklisted.  Given the filesystem
 corruption potential of the original, this is the lesser evil.  People
 who want to build with their fixed compiler versions will need to adjust
 the kernel source.  (Distros need to think about the implications of
 fixing such a compiler bug, and consider how to ensure that their fixed
 compiler versions can be detected if they wish to avoid this.)
 
 Signed-off-by: Russell King rmk+ker...@arm.linux.org.uk
 ---
 
 This is what I came up with - this places the build check right at the
 beginning of the kernel build, rather than at some point where
 linux/compiler.h gets included.  Note that this is where we have
 previous ARM specific GCC version blacklisting.  I'm blacklisting
 GCC 4.8.0 to GCC 4.8.2 inclussive, which seems to be the right versions
 for stock GCC.
 
 I was in two minds whether to include 4.8.3 as Linaro released a buggy
 toolchain which identifies itself as 4.8.3, but I decided that's also
 a distro problem.  IMHO Linaro should really think about taking that
 compiler down given the seriousness of this bug and it being
 indistinguishable from the fixed stock version.
 
  arch/arm/kernel/asm-offsets.c | 12 +++-
  1 file changed, 11 insertions(+), 1 deletion(-)
 
 diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c
 index 713e807621d2..e14c1a12b414 100644
 --- a/arch/arm/kernel/asm-offsets.c
 +++ b/arch/arm/kernel/asm-offsets.c
 @@ -10,6 +10,7 @@
   * it under the terms of the GNU General Public License version 2 as
   * published by the Free Software Foundation.
   */
 +#include linux/compiler.h
  #include linux/sched.h
  #include linux/mm.h
  #include linux/dma-mapping.h
 @@ -39,10 +40,19 @@
   * GCC 3.2.x: miscompiles NEW_AUX_ENT in fs/binfmt_elf.c
   *(http://gcc.gnu.org/PR8896) and incorrect structure
   * initialisation in fs/jffs2/erase.c
 + * GCC 4.8.0-4.8.2: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58854
 + * miscompiles find_get_entry(), and can result in EXT3 and EXT4
 + * filesystem corruption (possibly other FS too).
   */
 +#ifdef __GNUC__
  #if (__GNUC__ == 3  __GNUC_MINOR__  3)
  #error Your compiler is too buggy; it is known to miscompile kernels.
 -#errorKnown good compilers: 3.3
 +#errorKnown good compilers: 3.3, 4.x

Except that isn't true since 4.8.0 isn't a good compiler.

 +#endif
 +#if GCC_VERSION = 40800 || GCC_VERSION  40803
 +#error Your compiler is too buggy; it is known to miscompile kernels
 +#error and result in filesystem corruption and oopses.
 +#endif
  #endif

You are mixing the style of the version check.
Why not the single test:
#if GCC_VERSION  30300 || (GCC_VERSION = 40800  GCC_VERSION  40803)
#error Your compiler is too buggy; it is known to miscompile code.
#error Known good compilers: 3.3 onwards excluding 4.8.0 through 4.8.2
#endif

David

 
  int main(void)
 --
 1.8.3.1



RE: RCU bug with v3.17-rc3 ?

2014-10-13 Thread David Laight
From: Nathan Lynch
 On 10/10/2014 11:25 AM, Russell King - ARM Linux wrote:
 
  Right, so GCC 4.8.{1,2} are totally unsuitable for kernel building (and
  it seems that this has been known about for some time.)
 
 Looking at http://gcc.gnu.org/PR58854 it seems that all 4.8.x for x  3
 are affected, as well as 4.9.0.
 
  We can blacklist these GCC versions quite easily.  We already have GCC
  3.3 blacklisted, and it's trivial to add others.  I would want to include
  some proper details about the bug, just like the other existing entries
  we already have in asm-offsets.c, where we name the functions that the
  compiler is known to break where appropriate.
 
 Before blacklisting anything, it's worth considering that simple version
 checks would break existing pre-4.8.3 compilers that have been patched
 for PR58854.  It looks like Yocto and Buildroot issued releases with
 patched 4.8.2 compilers well before the (fixed) 4.8.3 release.  I think
 the most we can reasonably do without breaking some correctly-behaving
 toolchains is to emit a warning.

Is it possible to compile a small code fragment and check the generated
code for the bug?
Possibly predicated on the broken version number to avoid false positives.

David



--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 1/2] usb: rename 'phy' field of 'struct usb_hcd' to 'transceiver'

2014-04-10 Thread David Laight
From: Sergei Shtylyov
 It doesn't do any pin muxing. It switches SoC internal USB signals between
 USB controllers. The pins remain the same.

Doesn't something like that already happen for the companion USB1
controllers for USB2 ports?

That also doesn't sound like you are changing the PHY.
I'd have thought that would happen if you had a single controller
that select between multiply PHY. 

David



--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 1/2] usb: rename 'phy' field of 'struct usb_hcd' to 'transceiver'

2014-04-10 Thread David Laight
From: Ben Dooks
 On 10/04/14 11:49, Sergei Shtylyov wrote:
  On 10-04-2014 13:20, David Laight wrote:
 
   It doesn't do any pin muxing. It switches SoC internal USB
  signals between
  USB controllers. The pins remain the same.
 
  Doesn't something like that already happen for the companion USB1
  controllers for USB2 ports?
 
  Did you mean USB 1.1 and USB 2.0 controllers by USB1 and USB2?

Yes.

Why do you care which USB controller is driving the pins?

  That also doesn't sound like you are changing the PHY.
 
  I am changing one of the PHY registers that controls USB port
  (Renesas calls it channel) multiplexing.
 
  I'd have thought that would happen if you had a single controller
  that select between multiply PHY.
 
  No, it's not the case.

I realised that wasn't what you were doing, but at first it did seem
to be what you were doing.
 
 There is an interesting case, the USB3 shares a PHY with a SATA
 and the PCIE and SATA also share a PHY on the R8A7790.

Some of those look like pcb design decisions - so there is no dynamic
changing, just config time plumbing.
OTOH we are carrying PCIe using two SATA cables (the second carries the
clock) so I suspect some SoC system pcbs may be able to support SATA
or PCIe on the same connector).

David



--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 1/1] mfd: omap-usb-host: Fix USB device detection problems on OMAP4 Panda

2013-12-02 Thread David Laight
 From: Roger Quadros [mailto:rog...@ti.com]
 On 11/29/2013 03:17 PM, David Laight wrote:
...
  +  timeout = jiffies + msecs_to_jiffies(100);
  +  while (!(usbhs_read(omap-uhh_base, OMAP_UHH_SYSSTATUS)
  +   OMAP_UHH_SYSSTATUS_RESETDONE)) {
  +  cpu_relax();
 
 You mean use msleep(1) here instead of cpu_relax()?
 Shouldn't be a problem IMO, but can you please tell me why that is better
 as the reset seems to complete usually in the first iteration.

If it doesn't finish in the first iteration you don't want to
spin the cpu for 100ms.

If it hasn't finished in the first millisecond, you probably expect
it to actually time out - so you might as well look (say) every 10ms.

David



--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 1/1] mfd: omap-usb-host: Fix USB device detection problems on OMAP4 Panda

2013-11-29 Thread David Laight
 From: Of Roger Quadros
 With u-boot 2013.10, USB devices are sometimes not detected
 on OMAP4 Panda. To make us independent of what bootloader does
 with the USB Host module, we must RESET it to get it to a known
 good state. This patch Soft RESETs the USB Host module.
...
 +++ b/drivers/mfd/omap-usb-host.c
 @@ -43,14 +43,18 @@
  /* UHH Register Set */
  #define  OMAP_UHH_REVISION   (0x00)
  #define  OMAP_UHH_SYSCONFIG  (0x10)
 -#define  OMAP_UHH_SYSCONFIG_MIDLEMODE(1  12)
 +#define  OMAP_UHH_SYSCONFIG_MIDLEMASK(3  12)
 +#define OMAP_UHH_SYSCONFIG_MIDLESHIFT(12)

(tab/space issue)

Wouldn't it be clearer to define these in the opposite order with:
+#defineOMAP_UHH_SYSCONFIG_MIDLEMASK(3  
OMAP_UHH_SYSCONFIG_MIDLESHIFT)

... 
 +static void omap_usbhs_rev1_reset(struct device *dev)
 +{
 + struct usbhs_hcd_omap *omap = dev_get_drvdata(dev);
 + u32 reg;
 + unsigned long timeout;
 +
 + reg = usbhs_read(omap-uhh_base, OMAP_UHH_SYSCONFIG);
 +
 + /* Soft Reset */
 + usbhs_write(omap-uhh_base, OMAP_UHH_SYSCONFIG,
 + reg | OMAP_UHH_SYSCONFIG_SOFTRESET);
 +
 + timeout = jiffies + msecs_to_jiffies(100);
 + while (!(usbhs_read(omap-uhh_base, OMAP_UHH_SYSSTATUS)
 +  OMAP_UHH_SYSSTATUS_RESETDONE)) {
 + cpu_relax();
 +
 + if (time_after(jiffies, timeout)) {
 + dev_err(dev, Soft RESET operation timed out\n);
 + break;
 + }
 + }
 +
 + /* Set No-Standby */
 + reg = ~OMAP_UHH_SYSCONFIG_MIDLEMASK;
 + reg |= OMAP_UHH_SYSCONFIG_MIDLE_NOSTANDBY
 +  OMAP_UHH_SYSCONFIG_MIDLESHIFT;
 +
 + /* Set No-Idle */
 + reg = ~OMAP_UHH_SYSCONFIG_SIDLEMASK;
 + reg |= OMAP_UHH_SYSCONFIG_SIDLE_NOIDLE
 +  OMAP_UHH_SYSCONFIG_SIDLESHIFT;

Why not pass in the mask and value and avoid replicating the
entire function. I can't see any other significant differences,
the udelay(2) won't be significant.

I'm not sure of the context this code runs in, but if the reset
is likely to take a significant portion of the 100ms timeout
period, why not just sleep for a few ms between status polls.

David




--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html