RE: [PATCH v5 02/46] usb: gadget: add endpoint capabilities flags
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
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 ?
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'
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'
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
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
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