Re: [PATCH] usb: core: hub: Removed some warnings generated by checkpatch.pl
On 2015-08-15 11:41, Chase Metzger wrote:
Removed some checkpatch.pl warnings saying there was an unwanted space between
function names and their arguments.
Signed-off-by: Chase Metzger
---
drivers/usb/core/hub.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 38cb6d3..b9cab51 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4218,7 +4218,7 @@ static int hub_enable_device(struct usb_device *udev)
* but it is still necessary to lock the port.
*/
static int
-hub_port_init (struct usb_hub *hub, struct usb_device *udev, int port1,
+hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
int retry_counter)
{
struct usb_device *hdev = hub->hdev;
@@ -4522,7 +4522,7 @@ fail:
}
static void
-check_highspeed (struct usb_hub *hub, struct usb_device *udev, int port1)
+check_highspeed(struct usb_hub *hub, struct usb_device *udev, int port1)
{
struct usb_qualifier_descriptor *qual;
int status;
@@ -4530,11 +4530,11 @@ check_highspeed (struct usb_hub *hub, struct usb_device
*udev, int port1)
if (udev->quirks & USB_QUIRK_DEVICE_QUALIFIER)
return;
- qual = kmalloc (sizeof *qual, GFP_KERNEL);
+ qual = kmalloc(sizeof *qual, GFP_KERNEL);
if (qual == NULL)
return;
- status = usb_get_descriptor (udev, USB_DT_DEVICE_QUALIFIER, 0,
+ status = usb_get_descriptor udev, USB_DT_DEVICE_QUALIFIER, 0,
qual, sizeof *qual);
If you had test compiled you would have got an unbalanced parenthesis
error here. So you didn't test your patch..
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: core: hub: Removed some warnings generated by checkpatch.pl
Removed some checkpatch.pl warnings saying there was an unwanted space between
function names and their arguments.
Signed-off-by: Chase Metzger
---
drivers/usb/core/hub.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 38cb6d3..b9cab51 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4218,7 +4218,7 @@ static int hub_enable_device(struct usb_device *udev)
* but it is still necessary to lock the port.
*/
static int
-hub_port_init (struct usb_hub *hub, struct usb_device *udev, int port1,
+hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
int retry_counter)
{
struct usb_device *hdev = hub->hdev;
@@ -4522,7 +4522,7 @@ fail:
}
static void
-check_highspeed (struct usb_hub *hub, struct usb_device *udev, int port1)
+check_highspeed(struct usb_hub *hub, struct usb_device *udev, int port1)
{
struct usb_qualifier_descriptor *qual;
int status;
@@ -4530,11 +4530,11 @@ check_highspeed (struct usb_hub *hub, struct usb_device
*udev, int port1)
if (udev->quirks & USB_QUIRK_DEVICE_QUALIFIER)
return;
- qual = kmalloc (sizeof *qual, GFP_KERNEL);
+ qual = kmalloc(sizeof *qual, GFP_KERNEL);
if (qual == NULL)
return;
- status = usb_get_descriptor (udev, USB_DT_DEVICE_QUALIFIER, 0,
+ status = usb_get_descriptor udev, USB_DT_DEVICE_QUALIFIER, 0,
qual, sizeof *qual);
if (status == sizeof *qual) {
dev_info(&udev->dev, "not running at top speed; "
@@ -4550,7 +4550,7 @@ check_highspeed (struct usb_hub *hub, struct usb_device
*udev, int port1)
}
static unsigned
-hub_power_remaining (struct usb_hub *hub)
+hub_power_remaining(struct usb_hub *hub)
{
struct usb_device *hdev = hub->hdev;
int remaining;
@@ -4737,7 +4737,7 @@ static void hub_port_connect(struct usb_hub *hub, int
port1, u16 portstatus,
if (le16_to_cpu(udev->descriptor.bcdUSB) >= 0x0200
&& udev->speed == USB_SPEED_FULL
&& highspeed_hubs != 0)
- check_highspeed (hub, udev, port1);
+ check_highspeed(hub, udev, port1);
/* Store the parent's children[] pointer. At this point
* udev becomes globally accessible, although presumably
@@ -5111,7 +5111,7 @@ static const struct usb_device_id hub_id_table[] = {
{ }/* Terminating entry */
};
-MODULE_DEVICE_TABLE (usb, hub_id_table);
+MODULE_DEVICE_TABLE(usb, hub_id_table);
static struct usb_driver hub_driver = {
.name = "hub",
@@ -5223,7 +5223,7 @@ static int descriptors_changed(struct usb_device *udev,
changed = 1;
break;
}
- if (memcmp (buf, udev->rawdescriptors[index], old_length)
+ if (memcmp(buf, udev->rawdescriptors[index], old_length)
!= 0) {
dev_dbg(&udev->dev, "config index %d changed (#%d)\n",
index,
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[usb:usb-testing 49/60] DockBook: Warning(include/linux/usb.h:187): No description found for parameter 'authorized'
tree: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing head: 17248569499eae54b314fb05c4ff19fd47c9e99b commit: 484ebaedecc5ddf778a30ee1efab367cbee27030 [49/60] usb: interface authorization: Declare authorized attribute reproduce: make htmldocs All warnings (new ones prefixed by >>): >> Warning(include/linux/usb.h:187): No description found for parameter >> 'authorized' --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation # # Automatically generated file; DO NOT EDIT. # Linux/i386 4.2.0-rc6 Kernel Configuration # # CONFIG_64BIT is not set CONFIG_X86_32=y CONFIG_X86=y CONFIG_INSTRUCTION_DECODER=y CONFIG_OUTPUT_FORMAT="elf32-i386" CONFIG_ARCH_DEFCONFIG="arch/x86/configs/i386_defconfig" CONFIG_LOCKDEP_SUPPORT=y CONFIG_STACKTRACE_SUPPORT=y CONFIG_HAVE_LATENCYTOP_SUPPORT=y CONFIG_MMU=y CONFIG_NEED_SG_DMA_LENGTH=y CONFIG_GENERIC_ISA_DMA=y CONFIG_GENERIC_HWEIGHT=y CONFIG_ARCH_MAY_HAVE_PC_FDC=y CONFIG_RWSEM_XCHGADD_ALGORITHM=y CONFIG_GENERIC_CALIBRATE_DELAY=y CONFIG_ARCH_HAS_CPU_RELAX=y CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y CONFIG_HAVE_SETUP_PER_CPU_AREA=y CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y CONFIG_ARCH_HIBERNATION_POSSIBLE=y CONFIG_ARCH_SUSPEND_POSSIBLE=y CONFIG_ARCH_WANT_HUGE_PMD_SHARE=y CONFIG_ARCH_WANT_GENERAL_HUGETLB=y CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y CONFIG_X86_32_LAZY_GS=y CONFIG_ARCH_HWEIGHT_CFLAGS="-fcall-saved-ecx -fcall-saved-edx" CONFIG_ARCH_SUPPORTS_UPROBES=y CONFIG_FIX_EARLYCON_MEM=y CONFIG_PGTABLE_LEVELS=2 CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config" CONFIG_IRQ_WORK=y CONFIG_BUILDTIME_EXTABLE_SORT=y # # General setup # CONFIG_BROKEN_ON_SMP=y CONFIG_INIT_ENV_ARG_LIMIT=32 CONFIG_CROSS_COMPILE="" # CONFIG_COMPILE_TEST is not set CONFIG_LOCALVERSION="" # CONFIG_LOCALVERSION_AUTO is not set CONFIG_HAVE_KERNEL_GZIP=y CONFIG_HAVE_KERNEL_BZIP2=y CONFIG_HAVE_KERNEL_LZMA=y CONFIG_HAVE_KERNEL_XZ=y CONFIG_HAVE_KERNEL_LZO=y CONFIG_HAVE_KERNEL_LZ4=y CONFIG_KERNEL_GZIP=y # CONFIG_KERNEL_BZIP2 is not set # CONFIG_KERNEL_LZMA is not set # CONFIG_KERNEL_XZ is not set # CONFIG_KERNEL_LZO is not set # CONFIG_KERNEL_LZ4 is not set CONFIG_DEFAULT_HOSTNAME="(none)" # CONFIG_SYSVIPC is not set # CONFIG_CROSS_MEMORY_ATTACH is not set # CONFIG_FHANDLE is not set # CONFIG_USELIB is not set CONFIG_HAVE_ARCH_AUDITSYSCALL=y # # IRQ subsystem # CONFIG_GENERIC_IRQ_PROBE=y CONFIG_GENERIC_IRQ_SHOW=y CONFIG_IRQ_FORCED_THREADING=y CONFIG_SPARSE_IRQ=y CONFIG_CLOCKSOURCE_WATCHDOG=y CONFIG_ARCH_CLOCKSOURCE_DATA=y CONFIG_CLOCKSOURCE_VALIDATE_LAST_CYCLE=y CONFIG_GENERIC_TIME_VSYSCALL=y CONFIG_GENERIC_CLOCKEVENTS=y CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=y CONFIG_GENERIC_CMOS_UPDATE=y # # Timers subsystem # CONFIG_HZ_PERIODIC=y # CONFIG_NO_HZ_IDLE is not set # CONFIG_NO_HZ is not set # CONFIG_HIGH_RES_TIMERS is not set # # CPU/Task time and stats accounting # CONFIG_TICK_CPU_ACCOUNTING=y # CONFIG_IRQ_TIME_ACCOUNTING is not set # # RCU Subsystem # CONFIG_TINY_RCU=y # CONFIG_RCU_EXPERT is not set CONFIG_SRCU=y # CONFIG_TASKS_RCU is not set # CONFIG_RCU_STALL_COMMON is not set # CONFIG_TREE_RCU_TRACE is not set # CONFIG_RCU_EXPEDITE_BOOT is not set # CONFIG_BUILD_BIN2C is not set # CONFIG_IKCONFIG is not set CONFIG_HAVE_UNSTABLE_SCHED_CLOCK=y # CONFIG_CGROUPS is not set # CONFIG_CHECKPOINT_RESTORE is not set # CONFIG_SCHED_AUTOGROUP is not set # CONFIG_RELAY is not set # CONFIG_BLK_DEV_INITRD is not set # CONFIG_CC_OPTIMIZE_FOR_SIZE is not set CONFIG_ANON_INODES=y CONFIG_HAVE_UID16=y CONFIG_SYSCTL_EXCEPTION_TRACE=y CONFIG_HAVE_PCSPKR_PLATFORM=y CONFIG_EXPERT=y # CONFIG_MULTIUSER is not set # CONFIG_SGETMASK_SYSCALL is not set # CONFIG_SYSFS_SYSCALL is not set # CONFIG_KALLSYMS is not set # CONFIG_PRINTK is not set # CONFIG_BUG is not set # CONFIG_PCSPKR_PLATFORM is not set # CONFIG_BASE_FULL is not set # CONFIG_FUTEX is not set # CONFIG_EPOLL is not set # CONFIG_SIGNALFD is not set # CONFIG_TIMERFD is not set # CONFIG_EVENTFD is not set # CONFIG_BPF_SYSCALL is not set # CONFIG_SHMEM is not set # CONFIG_AIO is not set # CONFIG_ADVISE_SYSCALLS is not set CONFIG_EMBEDDED=y CONFIG_HAVE_PERF_EVENTS=y # # Kernel Performance Events And Counters # CONFIG_PERF_EVENTS=y # CONFIG_DEBUG_PERF_USE_VMALLOC is not set # CONFIG_VM_EVENT_COUNTERS is not set # CONFIG_COMPAT_BRK is not set # CONFIG_SLAB is not set CONFIG_SLUB=y # CONFIG_SLOB is not set # CONFIG_PROFILING is not set CONFIG_HAVE_OPROFILE=y CONFIG_OPROFILE_NMI_TIMER=y # CONFIG_JUMP_LABEL is not set # CONFIG_UPROBES is not set # CONFIG_HAVE_64BIT_ALIGNED_ACCESS is not set CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS=y CONFIG_ARCH_USE_BUILTIN_BSWAP=y CONFIG_HAVE_IOREMAP_PROT=y CONFIG_HAVE_KPROBES=y CONFIG_HAVE_KRETPROBES=y CONFIG_HAVE_OPTPROBES=y CONFIG_HAVE_KPROBES_ON_FTRACE=y CONFIG_HAVE_ARCH_TRACEHOOK=y CONFIG_HAVE_DMA_ATTRS=y CONFIG_HAVE_DMA_CONTIGUOUS=y CONFIG_GE
Re: 4.2 kernel trace when hot unplug a mounted USB/SATA/MMC devices with ext2/ext3/ext4 file system
On Fri, Aug 14, 2015 at 1:09 AM, Jan Kara wrote:
> On Thu 13-08-15 09:01:28, Darrick J. Wong wrote:
>> On Wed, Aug 12, 2015 at 08:17:28PM -0700, Duc Dang wrote:
>> > Hi Theodore, Andreas, Jan, Andrew and All,
>> >
>> > We are seeing kernel trace when we disconnect an USB/SATA/MMC devices
>> > that has its ext2/ext3/ext4 partition currently mounted. These traces
>> > are seen with kernel 4.2-rc5 on APM Arm64 X-Gene platforms. Similar
>> > issue also happens on an x86 machine (Lenovo T420s running Fedora Core
>> > 22 (Linux 4.1.x))
>>
>> Sounds like this issue:
>> https://bugzilla.kernel.org/show_bug.cgi?id=101011
>
> Hum, the above bug mentions that at least for ext4 this is a regression
> caused by commit 08439fec26 "ext4: remove block_device_ejected" from
> Christoph, let's add him to CC. The assumption of the commit is that
> bdi->dev never goes away so there's no point in checking it but these guys
> are apparently able to make bdi go away before fs is done with it... Any
> idea Christoph?
The commit 08439fec26 "ext4: remove block_device_ejected" only causes
issue with ext4 and
trying reverting it helps our test passes with ext4.
But how about the same issue with ext2/ext3?
>
> Honza
>
>> > We also tested with xfs kernel and kernel complains with some error
>> > message but there is no trace:
>> >
>> > usb 1-1: USB disconnect, device number 3
>> > XFS (sda1): Unmounting Filesystem
>> > XFS (sda1): metadata I/O error: block 0x1ddc12 ("xlog_iodone") error 19
>> > numblks
>> > 64
>> > XFS (sda1): xfs_do_force_shutdown(0x2) called from line 1180 of file
>> > /projects/svdc/P4wsIPCSW/buildsw_shadowcat_2_03/shadowcat/linux/fs/xfs/xfs_log.c.
>> > Return address = 0xffc0003aa8d8
>> > XFS (sda1): Log I/O Error Detected. Shutting down filesystem
>> > XFS (sda1): Unable to update superblock counters. Freespace may not be
>> > correct
>> > on next mount.
>> > XFS (sda1): xfs_log_force: error -5 returned.
>> > XFS (sda1): Please umount the filesystem and rectify the problem(s)
>> >
>> > Below are traces for each case. Do you aware of this issue and is
>> > there a fix for it?
>> >
>> > CASE 1: SATA with ext2 partition:
>> >
>> > EXT2-fs (sda1): previous I/O error to superblock detected
>> >
>> > Unable to handle kernel paging request at virtual address 1ff256000
>> > pgd = ffc0e784b000
>> > [1ff256000] *pgd=, *pud=
>> > Internal error: Oops: 9605 [#1] PREEMPT SMP
>> > Modules linked in:
>> > CPU: 4 PID: 1484 Comm: umount Not tainted
>> > 4.1.0-xgene_sw_2.03.05-beta_rc_pm #1
>> > Hardware name: APM X-Gene Mustang board (DT)
>> > task: ffc1ed5d3840 ti: ffc0e79dc000 task.ti: ffc0e79dc000
>> > PC is at __percpu_counter_add+0x2c/0x10c
>> > LR is at account_page_dirtied+0x78/0x12c
>> > pc : [] lr : [] pstate: 81c5
>> > sp : ffc0e79dfc40
>> > x29: ffc0e79dfc40 x28: ffc0e79dc000
>> > x27: ffc000927000 x26: 0027
>> > x25: 011a x24: 0015
>> > x23: 8000 x22: 0140
>> > x21: ffc000d33000 x20: ffc1f6440fb8
>> > x19: ffbec7b3e500 x18:
>> > x17: 007f8d95bfd0 x16: ffc0001dfad0
>> > x15: 003b9aca x14: 0ffe
>> > x13: 0020 x12: 0101010101010101
>> > x11: 0174 x10: 0006
>> > x9 : ffc1fff1e35c x8 : 746564206b636f6c
>> > x7 : ffc000d33618 x6 : 0001ff256000
>> > x5 : ffc0e79dfc40 x4 : ffc0e79dc000
>> > x3 : x2 : 0020
>> > x1 : 0001 x0 : 0001ff256000
>> >
>> > Process umount (pid: 1484, stack limit = 0xffc0e79dc020)
>> > Stack: (0xffc0e79dfc40 to 0xffc0e79e)
>> > fc40: e79dfc70 ffc0 00147180 ffc0 c7b3e500 ffbe f6440f78
>> > ffc1
>> > fc60: e79dfc70 ffc0 0014716c ffc0 e79dfcb0 ffc0 001ccd7c
>> > ffc0
>> > fc80: c7b3e500 ffbe f6b28f48 ffc1 f6b28f60 ffc1 e79dfca0
>> > ffc0
>> > fca0: f6b28f48 ffc1 e79dfd00 ffc0 001cdfd4
>> > ffc0
>> > fcc0: c7b3e500 ffbe ecf94400 ffc1 ee6022d8 ffc0
>> >
>> > fce0: 8000 00226674 ffc0 e79dfd20 ffc0
>> >
>> > fd00: e79dfd20 ffc0 0022ac84 ffc0 f6ff7000 ffc1 ecf94400
>> > ffc1
>> > fd20: e79dfd50 ffc0 0022ad0c ffc0 f677e380 ffc1 ecf94400
>> > ffc1
>> > fd40: f6ff7000 ffc1 ed5d3840 ffc1 e79dfd90 ffc0 001cb0c4
>> > ffc0
>> > fd60: f6ff7000 ffc1 f6ff70a8 ffc1 00946080 ffc0 001b5ff0
>> > ffc0
>> > fd80: f6ff7000 ffc1 e79dfdb0 ffc0 001a05e0
>> > ffc0
>> > fda0: f6ff7000 ffc1 0083 e79dfde0 ffc0 001a0958
>> > ffc0
>> > fdc0: f6b28d00 ffc1 0083 00db9000 ffc0 0015
>> >
>> > fde0: e79dfe10 ffc0 001a0c68 ffc0 f6ff7000 ffc1 00d511e8
Re: [PATCH] usbtest: Use boottime
On Tue, Aug 11, 2015 at 03:24:04PM -0400, Abhilash Jindal wrote: > Wall time obtained from do_gettimeofday is susceptible to sudden jumps due to > user setting the time or due to NTP. Boot time is constantly increasing time > better suited for comparing two timestamps. > > Signed-off-by: Abhilash Jindal > --- > drivers/usb/misc/usbtest.c | 14 -- > 1 file changed, 4 insertions(+), 10 deletions(-) What changed from the first one? Always properly version your patches, and say what changed. Please do this one over. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: USB 3.0 LPM Certification issues
On Tue, Aug 11, 2015 at 04:10:47PM +0100, Joao Pinto wrote: > Hi! > > When testing our USB 3.0 Device IP solution we typically run the Compliance > Tests for USB 3.0 and also for USB 2.0. We find an issue regarding the USB 2.0 > CV Chapter 9 test: LPM L1 Suspend Resume Test. > For it to be successful we have to edit the gadget driver in order to > configure > the baseline and deep besl parameters: > > --- b/drivers/usb/gadget/composite.c 2015-08-05 07:21:03.0 +0100 > +++ a/drivers/usb/gadget/composite.c 2015-08-07 11:20:52.037425955 +0100 > @@ -560,7 +560,17 @@ > usb_ext->bLength = USB_DT_USB_EXT_CAP_SIZE; > usb_ext->bDescriptorType = USB_DT_DEVICE_CAPABILITY; > usb_ext->bDevCapabilityType = USB_CAP_TYPE_EXT; > - usb_ext->bmAttributes = cpu_to_le32(USB_LPM_SUPPORT | USB_BESL_SUPPORT); > + usb_ext->bmAttributes = cpu_to_le32(USB_LPM_SUPPORT | > + USB_BESL_SUPPORT | > + USB_BESL_BASELINE_VALID | > + USB_BESL_DEEP_VALID); > + > + //baseline besl bits (0x00f00) and deep besl bits (0x0f000) > + usb_ext->bmAttributes &= cpu_to_le32(~(0x00f00 | 0x0f000)); > + //setting baseline besl = 4 > + usb_ext->bmAttributes |= cpu_to_le32(4 << 8); > + //setting deep besl = 5 > + usb_ext->bmAttributes |= cpu_to_le32(5 << 12); > > /* >* The Superspeed USB Capability descriptor shall be implemented by all > > Is there any plans to put this parameters configurable in the gadget driver > avoiding the need of patching it like shown previously? Care to turn this into a "real" patch that could be applied so we can handle it that way? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4]USB:OHCI: BugFix:Proper handling of ed_rm_list to handle race condition between usb_kill_urb() and finish_unlinks()
On Mon, Aug 10, 2015 at 11:05:11AM +, AMAN DEEP wrote:
> There is a race condition between
> finish_unlinks->finish_urb() function and
> usb_kill_urb() in ohci controller case. The finish_urb
> calls spin_unlock(&ohci->lock) before
> usb_hcd_giveback_urb() function call, then if during
> this time, usb_kill_urb is called for another endpoint,
>then new ed will be added to ed_rm_list at beginning
> for unlink. and ed_rm_list will point to newly added
> ed.
>
> When finish_urb() is completed in finish_unlinks() and
> ed->td_list becomes empty as in below code (in finish_unlinks() function)
> if (list_empty(&ed->td_list)) {
> *last = ed->ed_next;
> ed->ed_next = NULL;
> } else if (ohci->rh_state == OHCI_RH_RUNNING) {
> *last = ed->ed_next;
> ed->ed_next = NULL;
> ed_schedule(ohci, ed);
> }
> *last = ed->ed_next will make ed_rm_list to point to ed->ed_next and
> previously added ed by usb_kill_urb will be left unreferenced by
> ed_rm_list. This causes usb_kill_urb() hang forever waiting for
> finish_unlink to remove added ed from ed_rm_list.
>
> The main reason for hang in this race condtion is addition and removal
> of ed from ed_rm_list in the beginning during usb_kill_urb and later last*
> is modified in finish_unlinks().
> As suggested by Alan Stern, the solution for proper handling of
> ohci->ed_rm_list is to remove ed from the ed_rm_list before finishing
> any URBs. Then at the end, we can add ed back to the list if necessary.
> This properly handle the updated ohci->ed_rm_list in
> usb_kill_urb().
>
> Fixes:977dcfdc6031("USB:OHCI:don't lose track of EDs when a
> controller dies")
>
> Acked-by: Alan Stern
>
> Signed-off-by: Aman Deep
> CC:
> ---
> drivers/usb/host/ohci-q.c | 17 ++---
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/usb/host/ohci-q.c b/drivers/usb/host/ohci-q.c
> index d029bbe..da878f3 100644
> --- a/drivers/usb/host/ohci-q.c
> +++ b/drivers/usb/host/ohci-q.c
> @@ -1017,6 +1017,8 @@ skip_ed:
>* have modified this list. normally it's just prepending
>* entries (which we'd ignore), but paranoia won't hurt.
>*/
> + *last = ed->ed_next;
> + ed->ed_next = NULL;
> modified = 0;
>
> /* unlink urbs as requested, but rescan the list after
> @@ -1075,21 +1077,22 @@ rescan_this:
> goto rescan_this;
>
> /*
> - * If no TDs are queued, take ED off the ed_rm_list.
> + * If no TDs are queued, ED is now idle.
>* Otherwise, if the HC is running, reschedule.
> - * If not, leave it on the list for further dequeues.
> + * If the HC isn't running, add ED back to the
> + * start of the list for later processing.
>*/
> if (list_empty(&ed->td_list)) {
> - *last = ed->ed_next;
> - ed->ed_next = NULL;
> ed->state = ED_IDLE;
> list_del(&ed->in_use_list);
> } else if (ohci->rh_state == OHCI_RH_RUNNING) {
> - *last = ed->ed_next;
> - ed->ed_next = NULL;
> ed_schedule(ohci, ed);
> } else {
> - last = &ed->ed_next;
> + ed->ed_next = ohci->ed_rm_list;
> + ohci->ed_rm_list = ed;
> + /* Don't loop on the same ED */
> + if (last == &ohci->ed_rm_list)
> + last = &ed->ed_next;
> }
>
> if (modified)
> --
> 1.7.9.5
This still does not apply properly :(
What tree are you making it against?
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL] USB: chipidea updates for v4.3-rc3
On Fri, Aug 14, 2015 at 10:18:39AM +0800, Peter Chen wrote: > The following changes since commit e6bbe1d05353a29628a4ca72d88bac0bdcec5f38: > > Merge 4.2-rc6 into usb-next (2015-08-10 10:01:33 -0700) > > are available in the git repository at: > > > git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git/ > tags/usb-ci-v4.3-rc1 Pulled and pushed out, thanks. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL] USB patches for v4.3 merge window
On Thu, Aug 13, 2015 at 09:26:50AM -0500, Felipe Balbi wrote: > Hi Greg, > > here's the big Gadget pull request. It contains 146 non-merge commits this > time around. > > All patches have been soaking in next for quite a while and have also been > tested with the platforms I have around. > > Let me know if you want anything to be changed. > > cheers > > The following changes since commit cbfe8fa6cd672011c755c3cd85c9ffd4e2d10a6f: > > Linux 4.2-rc4 (2015-07-26 12:26:21 -0700) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git > tags/usb-for-v4.3 Pulled and pushed out, thanks. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] Display a DMA error message
On Friday 14 August 2015 16:44:12 Jeremy Linton wrote:
> On 08/14/2015 04:19 PM, Arnd Bergmann wrote:
> > On Wednesday 12 August 2015 16:51:29 Jeremy Linton wrote:
> >> diff --git a/drivers/usb/host/ehci-platform.c
> >> b/drivers/usb/host/ehci-platform.c
> >> index 2593def..82e396f 100644
> >> --- a/drivers/usb/host/ehci-platform.c
> >> +++ b/drivers/usb/host/ehci-platform.c
> >> @@ -162,8 +162,10 @@ static int ehci_platform_probe(struct platform_device
> >> *dev)
> >>
> >> err = dma_coerce_mask_and_coherent(&dev->dev,
> >> pdata->dma_mask_64 ? DMA_BIT_MASK(64) : DMA_BIT_MASK(32));
> >> - if (err)
> >> + if (err) {
> >> + dev_err(&dev->dev, "Error: DMA setup failed\n");
> >> return err;
> >> + }
> >
> > We should really stop doing this: the platform should provide the
> > correct dma mask when creating the device, instead of setting a
> > bogus pdata field. Do not duplicate this bug for ACPI.
>
>
> I'm not sure I understand, I'm not changing the mask anywhere, all I'm
> doing is printing a message if the mask cannot be set.
The problem is that dma_coerce_mask_and_coherent() is not a proper
interface for a driver to use, it's an annotation that tells us
that the driver is trying to do something horrible by overriding
the dma mask that was set by the platform.
> AKA if CCA is missing or set to 0 then, this line is triggered because
> the dma_coerce_mask_and_coherent() routine returns with a failure
> indicating there aren't any DMA ops to set a mask on. Right now what
> happens is the device detection displays that there is an EHCI
> controller then silently disappears with telling the user why it didn't
> configure.
Adding a warning is fine, but the text is not right here (the driver is
not setting up the dma mask, but trying to override the one that has been
set up by the platform), and the solution should really be to not
call dma_coerce_mask_and_coherent() at all for ACPI. The main reason
that we have not removed this call here yet is that there is still a
MIPS platform that relies on it. We should probably remove it for DT
already and only do it if platform data was provided, but that requires
careful testing.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] Display a DMA error message
Arnd,
On 08/14/2015 04:19 PM, Arnd Bergmann wrote:
On Wednesday 12 August 2015 16:51:29 Jeremy Linton wrote:
diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
index 2593def..82e396f 100644
--- a/drivers/usb/host/ehci-platform.c
+++ b/drivers/usb/host/ehci-platform.c
@@ -162,8 +162,10 @@ static int ehci_platform_probe(struct platform_device *dev)
err = dma_coerce_mask_and_coherent(&dev->dev,
pdata->dma_mask_64 ? DMA_BIT_MASK(64) : DMA_BIT_MASK(32));
- if (err)
+ if (err) {
+ dev_err(&dev->dev, "Error: DMA setup failed\n");
return err;
+ }
We should really stop doing this: the platform should provide the
correct dma mask when creating the device, instead of setting a
bogus pdata field. Do not duplicate this bug for ACPI.
I'm not sure I understand, I'm not changing the mask anywhere, all I'm
doing is printing a message if the mask cannot be set.
AKA if CCA is missing or set to 0 then, this line is triggered because
the dma_coerce_mask_and_coherent() routine returns with a failure
indicating there aren't any DMA ops to set a mask on. Right now what
happens is the device detection displays that there is an EHCI
controller then silently disappears with telling the user why it didn't
configure.
Thanks,
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] Display a DMA error message
On Wednesday 12 August 2015 16:51:29 Jeremy Linton wrote:
> diff --git a/drivers/usb/host/ehci-platform.c
> b/drivers/usb/host/ehci-platform.c
> index 2593def..82e396f 100644
> --- a/drivers/usb/host/ehci-platform.c
> +++ b/drivers/usb/host/ehci-platform.c
> @@ -162,8 +162,10 @@ static int ehci_platform_probe(struct platform_device
> *dev)
>
> err = dma_coerce_mask_and_coherent(&dev->dev,
> pdata->dma_mask_64 ? DMA_BIT_MASK(64) : DMA_BIT_MASK(32));
> - if (err)
> + if (err) {
> + dev_err(&dev->dev, "Error: DMA setup failed\n");
> return err;
> + }
We should really stop doing this: the platform should provide the
correct dma mask when creating the device, instead of setting a
bogus pdata field. Do not duplicate this bug for ACPI.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 1/3] usb: dwc2: reset AHB hclk domain before init
On 8/13/2015 8:29 PM, Yunzhi Li wrote: > > > 在 2015/8/14 8:09, John Youn 写道: >> On 8/11/2015 12:57 AM, Yunzhi Li wrote: >>> We initiate dwc2 usb controller in BIOS, when kernel driver >>> start-up we should reset AHB hclk domain to reset all AHB >>> interface registers to default. Without this the FIFO value >>> setting might be incorrect because calculating FIFO size need the >>> power-on value of GRXFSIZ/GNPTXFSIZ/HPTXFSIZ registers. >>> >>> This patch could avoid warnning massage like in rk3288 platform: >>> [2.074764] dwc2 ff58.usb: 256 invalid for >>> host_perio_tx_fifo_size. Check HW configuration. >>> >>> .. >> I didn't receive the other two patches in this series so I was >> confused about where the "ahb_reset" was coming from when I >> replied to your other patch. >> >> I see you changed the name and documented the DT so never mind. >> >> Another thing is that there probably shouldn't be a debug >> message on the IS_ERR condition since that is the common case >> and of no interest to other platforms. >> >> The other two resets you added aren't used by the driver >> anywhere right? Maybe those should be left out until they are. >> >> John >> > Hi John , > >Here is the other two patches : > https://patchwork.kernel.org/patch/6989541/ > https://patchwork.kernel.org/patch/6989531/ > >ahb_reset is hreset_n signal of dwc2 IP. Our rk3288 SoC implement > connect this signal to a special > register in clock ang reset unit (CRU) module, set this register will > reset dwc2 control and status registers(CSR) > to default value. You could find more info in < Hi Speed On-TheGo (OTG) Databook 3.10a>> > 4.4.1 System Clock and Reset Signals. > >Our problem is that dwc2_get_hwparams() reads fifo size registers and > reguards it as the power-on reset value, > then dwc2_set_param_host_perio_tx_fifo_size() will check this value and > make sure the new fifo size value is no bigger > than the power-on reset value. But we init and set these fifo registers > in BIOS, so here hw->xxx_fifo_size is not the > real power-on reset vaule. So we hope to reset CSR before > dwc2_get_hwparams(). > > I have another ideal: we might use GRSTCTL.CSftRst instead of hreset_n > to reset dwc2 CSR. Yes, please try doing that before calling dwc2_get_hwparams(). Maybe by calling dwc2_core_reset(). If that works for you, I think it would be better. John -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usbnet: Fix two races between usbnet_stop() and the BH
Both races may happen when a device (e.g. YOTA 4G LTE Modem) is
unplugged while the system is downloading a large file from the Net.
Hardware breakpoints and Kprobes with delays were used to confirm that
the races do actually happen.
1. The first race is on skb_queue ('next' pointer) between usbnet_stop()
and rx_complete(), which, in turn, calls usbnet_bh().
Here is a part of the call stack with the code where the changes to the
queue happen. The line numbers are for the kernel 4.1.0:
*0 __skb_unlink (skbuff.h:1517)
prev->next = next;
*1 defer_bh (usbnet.c:430)
spin_lock_irqsave(&list->lock, flags);
old_state = entry->state;
entry->state = state;
__skb_unlink(skb, list);
spin_unlock(&list->lock);
spin_lock(&dev->done.lock);
__skb_queue_tail(&dev->done, skb);
if (dev->done.qlen == 1)
tasklet_schedule(&dev->bh);
spin_unlock_irqrestore(&dev->done.lock, flags);
*2 rx_complete (usbnet.c:640)
state = defer_bh(dev, skb, &dev->rxq, state);
At the same time, the following code repeatedly checks if the queue is
empty and reads these values concurrently with the above changes:
*0 usbnet_terminate_urbs (usbnet.c:765)
/* maybe wait for deletions to finish. */
while (!skb_queue_empty(&dev->rxq)
&& !skb_queue_empty(&dev->txq)
&& !skb_queue_empty(&dev->done)) {
schedule_timeout(msecs_to_jiffies(UNLINK_TIMEOUT_MS));
set_current_state(TASK_UNINTERRUPTIBLE);
netif_dbg(dev, ifdown, dev->net,
"waited for %d urb completions\n", temp);
}
*1 usbnet_stop (usbnet.c:806)
if (!(info->flags & FLAG_AVOID_UNLINK_URBS))
usbnet_terminate_urbs(dev);
As a result, it is possible, for example, that the skb is removed from
dev->rxq by __skb_unlink() before the check
"!skb_queue_empty(&dev->rxq)" in usbnet_terminate_urbs() is made. It is
also possible in this case that the skb is added to dev->done queue
after "!skb_queue_empty(&dev->done)" is checked. So
usbnet_terminate_urbs() may stop waiting and return while dev->done
queue still has an item.
Locking in defer_bh() and usbnet_terminate_urbs() was revisited to avoid
this race.
2. The second race is on dev->flags.
dev->flags is set to 0 here:
*0 usbnet_stop (usbnet.c:816)
/* deferred work (task, timer, softirq) must also stop.
* can't flush_scheduled_work() until we drop rtnl (later),
* else workers could deadlock; so make workers a NOP.
*/
dev->flags = 0;
del_timer_sync (&dev->delay);
tasklet_kill (&dev->bh);
And here, the code clears EVENT_RX_KILL bit in dev->flags, which may
execute concurrently with the above operation:
*0 clear_bit (bitops.h:113, inlined)
*1 usbnet_bh (usbnet.c:1475)
/* restart RX again after disabling due to high error rate */
clear_bit(EVENT_RX_KILL, &dev->flags);
It seems, setting dev->flags to 0 is not necessarily atomic w.r.t.
clear_bit() and other bit operations with dev->flags. It is safer to
make it atomic and this way, make the race harmless.
While at it, the checking of EVENT_NO_RUNTIME_PM bit of dev->flags in
usbnet_stop() was fixed too: the bit should be checked before dev->flags
is cleared.
Signed-off-by: Eugene Shatokhin
---
drivers/net/usb/usbnet.c | 49 --
include/linux/usb/usbnet.h | 33 +++
2 files changed, 54 insertions(+), 28 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 3c86b10..a53124c 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -428,12 +428,18 @@ static enum skb_state defer_bh(struct usbnet *dev, struct
sk_buff *skb,
old_state = entry->state;
entry->state = state;
__skb_unlink(skb, list);
- spin_unlock(&list->lock);
- spin_lock(&dev->done.lock);
+
+ /* defer_bh() is never called with list == &dev->done.
+* spin_lock_nested() tells lockdep that it is OK to take
+* dev->done.lock here with list->lock held. *
+*/
+ spin_lock_nested(&dev->done.lock, SINGLE_DEPTH_NESTING);
+
__skb_queue_tail(&dev->done, skb);
if (dev->done.qlen == 1)
tasklet_schedule(&dev->bh);
- spin_unlock_irqrestore(&dev->done.lock, flags);
+ spin_unlock(&dev->done.lock);
+ spin_unlock_irqrestore(&list->lock, flags);
return old_state;
}
@@ -749,6 +755,20 @@ EXPORT_SYMBOL_GPL(usbnet_unlink_rx_urbs);
/*-*/
+static void wait_skb_queue_empty(struct sk_buff_head *q)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&q->lock, flags);
+ while (!skb_queue_empty(q)) {
+ spin_unlock_irqrestore(&q->lock, flags);
+ schedule_timeout(msecs_to_jiffies(UNLINK_TIMEOUT_MS));
+ set_current_state(TASK_UNINTERRUPTIBLE);
+ spin_lock_irqsave(&q->lock, flags);
+ }
+ spin_unlock_ir
Re: Several races in "usbnet" module (kernel 4.1.x)
Hi,
21.07.2015 17:22, Oliver Neukum пишет:
On Mon, 2015-07-20 at 21:13 +0300, Eugene Shatokhin wrote:
And here, the code clears EVENT_RX_KILL bit in dev->flags, which may
execute concurrently with the above operation:
#0 clear_bit (bitops.h:113, inlined)
#1 usbnet_bh (usbnet.c:1475)
/* restart RX again after disabling due to high error rate */
clear_bit(EVENT_RX_KILL, &dev->flags);
If clear_bit() is atomic w.r.t. setting dev->flags to 0, this race is
not a problem, I guess. Otherwise, it may be.
clear_bit is atomic with respect to other atomic operations.
So how about this:
Regards
Oliver
From 1c4e685b3a9c183e04c46b661830e5c7ed35b513 Mon Sep 17 00:00:00 2001
From: Oliver Neukum
Date: Tue, 21 Jul 2015 16:19:40 +0200
Subject: [PATCH] usbnet: fix race between usbnet_stop() and the BH
Does this do the job?
Signed-off-by: Oliver Neukum
---
drivers/net/usb/usbnet.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 3c86b10..77a9a86 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -778,7 +778,7 @@ int usbnet_stop (struct net_device *net)
{
struct usbnet *dev = netdev_priv(net);
struct driver_info *info = dev->driver_info;
- int retval, pm;
+ int retval, pm, mpn;
clear_bit(EVENT_DEV_OPEN, &dev->flags);
netif_stop_queue (net);
@@ -813,14 +813,17 @@ int usbnet_stop (struct net_device *net)
* can't flush_scheduled_work() until we drop rtnl (later),
* else workers could deadlock; so make workers a NOP.
*/
+ mpn = !test_and_clear_bit(EVENT_NO_RUNTIME_PM, &dev->flags);
dev->flags = 0;
del_timer_sync (&dev->delay);
tasklet_kill (&dev->bh);
+ mpn |= !test_and_clear_bit(EVENT_NO_RUNTIME_PM, &dev->flags);
+ /* in case the bh reset a flag */
+ dev->flags = 0;
if (!pm)
usb_autopm_put_interface(dev->intf);
- if (info->manage_power &&
- !test_and_clear_bit(EVENT_NO_RUNTIME_PM, &dev->flags))
+ if (info->manage_power && mpn)
info->manage_power(dev, 0);
else
usb_autopm_put_interface(dev->intf);
From what we have discussed here, I have combined a patch that fixes
the race #1 in usbnet_stop() and makes #4 harmless by using atomics. I
will send it shortly.
I had to make some adjustments (e.g. using spin_lock_nested in one place
for lockdep to see it is OK to take dev->done.lock there).
I have tested the patch on the mainline kernel 4.2-rc6 built for x86-64,
with the same USB modem. So far, lockdep, Kmemleak (just in case) and my
tools have not detected problems in the relevant parts of the code. The
device and the driver seem to work well.
So, what is your opinion?
Regards,
Eugene
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] gadget: Introduce the usb charger framework
On Fri, Aug 14, 2015 at 05:47:45PM +0800, Baolin Wang wrote:
> This patch introduces the usb charger driver based on usb gadget that
> makes an enhancement to a power driver. It works well in practice but
> that requires a system with suitable hardware.
>
> The basic conception of the usb charger is that, when one usb charger
> is added or removed by reporting from the usb gadget state change or
> the extcon device state change, the usb charger will report to power
> user to set the current limitation.
>
> The usb charger will register notifiees on the usb gadget or the extcon
> device to get notified the usb charger state.
>
> Power user will register a notifiee on the usb charger to get notified
> by status changes from the usb charger. It will report to power user
> to set the current limitation when detecting the usb charger is added
> or removed from extcon device state or usb gadget state.
>
> Signed-off-by: Baolin Wang
> ---
> drivers/usb/gadget/Kconfig |7 +
> drivers/usb/gadget/Makefile |1 +
> drivers/usb/gadget/charger.c| 561
> +++
> include/linux/usb/usb_charger.h | 145 ++
> 4 files changed, 714 insertions(+)
> create mode 100644 drivers/usb/gadget/charger.c
> create mode 100644 include/linux/usb/usb_charger.h
>
> diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
> index bcf83c0..3d2b959 100644
> --- a/drivers/usb/gadget/Kconfig
> +++ b/drivers/usb/gadget/Kconfig
> @@ -127,6 +127,13 @@ config USB_GADGET_STORAGE_NUM_BUFFERS
> a module parameter as well.
> If unsure, say 2.
>
> +config USB_CHARGER
> + bool "USB charger support"
> + help
> + The usb charger driver based on the usb gadget that makes an
> + enhancement to a power driver which can set the current limitation
> + when the usb charger is added or removed.
> +
> source "drivers/usb/gadget/udc/Kconfig"
>
> #
> diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
> index 598a67d..1e421c1 100644
> --- a/drivers/usb/gadget/Makefile
> +++ b/drivers/usb/gadget/Makefile
> @@ -10,3 +10,4 @@ libcomposite-y := usbstring.o config.o
> epautoconf.o
> libcomposite-y += composite.o functions.o configfs.o
> u_f.o
>
> obj-$(CONFIG_USB_GADGET) += udc/ function/ legacy/
> +obj-$(CONFIG_USB_CHARGER)+= charger.o
> diff --git a/drivers/usb/gadget/charger.c b/drivers/usb/gadget/charger.c
> new file mode 100644
> index 000..f24f7b7
> --- /dev/null
> +++ b/drivers/usb/gadget/charger.c
> @@ -0,0 +1,561 @@
> +/*
> + * usb charger.c -- USB charger driver
I think your company also wants a copyright line here. Not that it
means anything at all, but some lawyers love cargo-cult text blobs.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
I have to ask, do you really mean "any later version"?
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define DEFAULT_CUR_PROTECT (50)
> +#define DEFAULT_SDP_CUR_LIMIT(500 - DEFAULT_CUR_PROTECT)
> +#define DEFAULT_DCP_CUR_LIMIT(1500 - DEFAULT_CUR_PROTECT)
> +#define DEFAULT_CDP_CUR_LIMIT(1500 - DEFAULT_CUR_PROTECT)
> +#define DEFAULT_ACA_CUR_LIMIT(1500 - DEFAULT_CUR_PROTECT)
> +
> +static DEFINE_IDA(usb_charger_ida);
> +static struct bus_type usb_charger_subsys = {
> + .name = "usb-charger",
> + .dev_name = "usb-charger",
> +};
> +
> +static struct usb_charger *dev_to_uchger(struct device *udev)
> +{
> + return container_of(udev, struct usb_charger, dev);
> +}
> +
> +static ssize_t usb_charger_cur_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct usb_charger *uchger = dev_to_uchger(dev);
> +
> + return scnprintf(buf, PAGE_SIZE, "%d %d %d %d\n",
> + uchger->cur_limit.sdp_cur_limit,
> + uchger->cur_limit.dcp_cur_limit,
> + uchger->cur_limit.cdp_cur_limit,
> + uchger->cur_limit.aca_cur_limit);
> +}
> +
> +static ssize_t usb_charger_cur_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct usb_charger *uchger = dev_to_uchger(dev);
> + struct usb_charger_cur_limit cur;
> + int ret;
> +
> + ret = sscanf(buf, "%d %d %d %d",
> + &cur.sdp_cur_limit, &cur.dcp_cur_limit,
> + &cur.cdp_cur_limit, &cur.aca_cur_limit)
Re: [PATCH] usb: chipidea: reuse the platform_data to store the ci info
On Fri, 14 Aug 2015, Felipe Balbi wrote:
> > That's not quite what I had in mind. I was thinking of something more
> > like this:
> >
> > Platform device drvdata > struct usb_pointers
> > |
> > |
> > |---|---|
> > | | |
> > gadget hcd otg
> >
> > where struct usb_pointers (maybe you can think of a better name) looks
> > like this:
> >
> > struct usb_pointers {
> > struct usb_hcd *hcd;
> > struct usb_gadget *gadget;
> > struct usb_otg *otg;
> > };
> >
> > If you want, you can use your IP Core device in place of my platform
> > device. All that matters is that the parent device of the root hub
> > (for hosts) or gadget (for peripherals) should store the address of a
> > usb_pointers structure in its drvdata.
>
> that looks pretty clean to me, Alan. Wonder how that looks after
> actually implementing it.
Implementing it will take a fair amount of work, because all of the
existing UDC and HC drivers will have to be changed. Probably a bunch
of OTG drivers too, but I'm not familiar with them.
The concept is simple, though. A little new code to allocate and
deallocate these usb_pointer structures can be added to
common/common.c. (I'd _really_ like to find a better name!)
Or maybe we could use a simpler approach: Always allocate a usb_otg
structure (even if the hardware doesn't support OTG) and make the
platform driver's drvdata point to it. Since struct usb_otg already
contains pointers to a usb_bus (should that really be usb_hcd?) and a
usb_gadget, we'd be all set.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: chipidea: reuse the platform_data to store the ci info
On Fri, Aug 14, 2015 at 10:26:35AM -0400, Alan Stern wrote:
> On Fri, 14 Aug 2015, Peter Chen wrote:
>
> > > In the old days, a single device could be a USB host controller and
> > > nothing else. Then later, a single device could be either a host
> > > controller or a device controller. Now a single device can be both.
> > >
> > > Obviously this causes problems for our original design, where the
> > > device's drvdata stored a pointer to the hcd structure or the udc
> > > structure.
> > >
> > > In principle, the drvdata should point to something that contains
> > > pointers to both the hcd and udc structures. Maybe also an otg
> > > structure. That would require a lot of changes to existing code but it
> > > might be worthwhile.
> > >
> >
> > Yes, the core device drvdata should contain standard hcd, gadget,
> > otg structure together. Currently, the gadget is child device
> > for core device, it is reasonable the same for hcd, and we can
> > create device for chipidea host to achieve it. The dwc3 driver
> > does the similar things.
> >
> > Below is the reference device hierarchy for current IP driver:
> >
> > Glue Layer Device
> >|
> >|
> >|
> > IP Core Device
> > |
> >|||
> >|||
> > Gadget hcd otg
> >
> > Alan/Felipe, what do you think?
>
> That's not quite what I had in mind. I was thinking of something more
> like this:
>
> Platform device drvdata > struct usb_pointers
> |
> |
> |---|---|
> | | |
> gadget hcd otg
>
> where struct usb_pointers (maybe you can think of a better name) looks
> like this:
>
> struct usb_pointers {
> struct usb_hcd *hcd;
> struct usb_gadget *gadget;
> struct usb_otg *otg;
> };
>
> If you want, you can use your IP Core device in place of my platform
> device. All that matters is that the parent device of the root hub
> (for hosts) or gadget (for peripherals) should store the address of a
> usb_pointers structure in its drvdata.
that looks pretty clean to me, Alan. Wonder how that looks after
actually implementing it.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH] usb: chipidea: reuse the platform_data to store the ci info
On Fri, 14 Aug 2015, Peter Chen wrote:
> > In the old days, a single device could be a USB host controller and
> > nothing else. Then later, a single device could be either a host
> > controller or a device controller. Now a single device can be both.
> >
> > Obviously this causes problems for our original design, where the
> > device's drvdata stored a pointer to the hcd structure or the udc
> > structure.
> >
> > In principle, the drvdata should point to something that contains
> > pointers to both the hcd and udc structures. Maybe also an otg
> > structure. That would require a lot of changes to existing code but it
> > might be worthwhile.
> >
>
> Yes, the core device drvdata should contain standard hcd, gadget,
> otg structure together. Currently, the gadget is child device
> for core device, it is reasonable the same for hcd, and we can
> create device for chipidea host to achieve it. The dwc3 driver
> does the similar things.
>
> Below is the reference device hierarchy for current IP driver:
>
> Glue Layer Device
> |
> |
> |
>IP Core Device
> |
> |||
> |||
> Gadget hcd otg
>
> Alan/Felipe, what do you think?
That's not quite what I had in mind. I was thinking of something more
like this:
Platform device drvdata > struct usb_pointers
|
|
|---|---|
| | |
gadget hcd otg
where struct usb_pointers (maybe you can think of a better name) looks
like this:
struct usb_pointers {
struct usb_hcd *hcd;
struct usb_gadget *gadget;
struct usb_otg *otg;
};
If you want, you can use your IP Core device in place of my platform
device. All that matters is that the parent device of the root hub
(for hosts) or gadget (for peripherals) should store the address of a
usb_pointers structure in its drvdata.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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/3] Honor ACPI _CCA attribute setting
Hello Suravee, On 08/13/2015 08:45 PM, Suravee Suthikulpanit wrote: On 8/13/15 04:51, Jeremy Linton wrote: ACPI configurations can now mark devices as noncoherent, support that choice. This change was in my earlier revisions for the original patch series to add ACPI CCA support. At the time, this was pushed back since we were not sure whether this would be a useful case, and whether such hardware exists. Yes, I was made aware of that conversation during the review here. Basically, the ARM Juno development platform has a mix of coherent and noncoherent devices. The EHCI being one of the primary noncoherent peripherals. Without which, the usefulness of the platform for developing with an ACPI kernel is limited. Thanks, -- To unsubscribe from this list: send the line "unsubscribe linux-usb" 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/3] Honor ACPI _CCA attribute setting
On Fri, Aug 14, 2015 at 08:45:22AM +0700, Suravee Suthikulpanit wrote:
> On 8/13/15 04:51, Jeremy Linton wrote:
> >ACPI configurations can now mark devices as noncoherent,
> >support that choice.
> >
> >Signed-off-by: Jeremy Linton
> >---
> > include/acpi/acpi_bus.h | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> >diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> >index 83061ca..7ecb8e4 100644
> >--- a/include/acpi/acpi_bus.h
> >+++ b/include/acpi/acpi_bus.h
> >@@ -399,7 +399,7 @@ static inline bool acpi_check_dma(struct acpi_device
> >*adev, bool *coherent)
> > * case 1. Do not support and disable DMA.
> > * case 2. Support but rely on arch-specific cache maintenance for
> > * non-coherence DMA operations.
> >- * Currently, we implement case 1 above.
> >+ * Currently, we implement case 2 above.
> > *
> > * For the case when _CCA is missing (i.e. cca_seen=0) and
> > * platform specifies ACPI_CCA_REQUIRED, we do not support DMA,
> >@@ -407,7 +407,8 @@ static inline bool acpi_check_dma(struct acpi_device
> >*adev, bool *coherent)
> > *
> > * See acpi_init_coherency() for more info.
> > */
> >-if (adev->flags.coherent_dma) {
> >+if (adev->flags.coherent_dma ||
> >+(adev->flags.cca_seen && IS_ENABLED(CONFIG_ARM64))) {
> > ret = true;
> > if (coherent)
> > *coherent = adev->flags.coherent_dma;
> >
>
> This change was in my earlier revisions for the original patch series to add
> ACPI CCA support. At the time, this was pushed back since we were not sure
> whether this would be a useful case, and whether such hardware exists.
>
> Would it be useful to document somewhere (may be in the GIT commit message)
> about which hardware might need this?
So far, it's the ARM Juno development board (the emphasis here is on
being able to use it for development, not a production system).
I think the commit log should also give you credit for the original
implementation.
> Arnd/Catalin, any feedback on this?
That's where it was left in the previous thread:
https://lkml.org/lkml/2015/5/21/376
(and I'll refrain from further comments ;))
--
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/3] Introduce usb charger framework to deal with the usb gadget power negotation
On 14 August 2015 at 16:55, Li Jun wrote: > Hi Baolin, > > On Fri, Aug 14, 2015 at 05:47:43PM +0800, Baolin Wang wrote: >> Currently the Linux kernel does not provide any standard integration of this >> feature that integrates the USB subsystem with the system power regulation >> provided by PMICs meaning that either vendors must add this in their kernels >> or USB gadget devices based on Linux (such as mobile phones) may not behave >> as they should. >> >> Providing a standard framework for doing this in the kernel. >> > > Why not add power supply class support into this? > Hi Jun, We don't need the power supply class support into the usb charger, just introduce the notify mechanism for power to set the current limit when notifying some events from usb charger. Maybe I misunderstand your meanings, please describe it detailedly. Thanks for your comments. > Li Jun > -- Baolin.wang Best Regards -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/3] power: wm831x_power: Support USB charger current limit management
On Fri, 14 Aug 2015, Baolin Wang wrote:
> Integrate with the newly added USB charger interface to limit the current
> we draw from the USB input based on the input device configuration
> identified by the USB stack, allowing us to charge more quickly from high
> current inputs without drawing more current than specified from others.
>
> Signed-off-by: Mark Brown
> Signed-off-by: Baolin Wang
> ---
> drivers/power/wm831x_power.c | 69
> ++
> include/linux/mfd/wm831x/pdata.h |3 ++
Acked-by: Lee Jones
> 2 files changed, 72 insertions(+)
>
> diff --git a/drivers/power/wm831x_power.c b/drivers/power/wm831x_power.c
> index db11ae6..72c661f 100644
> --- a/drivers/power/wm831x_power.c
> +++ b/drivers/power/wm831x_power.c
> @@ -13,6 +13,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -31,6 +32,8 @@ struct wm831x_power {
> char usb_name[20];
> char battery_name[20];
> bool have_battery;
> + struct usb_charger *usb_charger;
> + struct notifier_block usb_notify;
> };
>
> static int wm831x_power_check_online(struct wm831x *wm831x, int supply,
> @@ -125,6 +128,43 @@ static enum power_supply_property wm831x_usb_props[] = {
> POWER_SUPPLY_PROP_VOLTAGE_NOW,
> };
>
> +/* In miliamps */
> +static unsigned int wm831x_usb_limits[] = {
> + 0,
> + 2,
> + 100,
> + 500,
> + 900,
> + 1500,
> + 1800,
> + 550,
> +};
> +
> +static int wm831x_usb_limit_change(struct notifier_block *nb,
> +unsigned long limit, void *data)
> +{
> + struct wm831x_power *wm831x_power = container_of(nb,
> + struct wm831x_power,
> + usb_notify);
> + int i, best;
> +
> + /* Find the highest supported limit */
> + best = 0;
> + for (i = 0; i < ARRAY_SIZE(wm831x_usb_limits); i++) {
> + if (limit < wm831x_usb_limits[i] &&
> + wm831x_usb_limits[best] < wm831x_usb_limits[i])
> + best = i;
> + }
> +
> + dev_dbg(wm831x_power->wm831x->dev,
> + "Limiting USB current to %dmA", wm831x_usb_limits[best]);
> +
> + wm831x_set_bits(wm831x_power->wm831x, WM831X_POWER_STATE,
> + WM831X_USB_ILIM_MASK, best);
> +
> + return 0;
> +}
> +
> /*
> * Battery properties
> */
> @@ -606,8 +646,31 @@ static int wm831x_power_probe(struct platform_device
> *pdev)
> }
> }
>
> + if (wm831x_pdata && wm831x_pdata->usb_gadget) {
> + power->usb_charger =
> + usb_charger_find_by_name(wm831x_pdata->usb_gadget);
> + if (IS_ERR(power->usb_charger)) {
> + ret = PTR_ERR(power->usb_charger);
> + dev_err(&pdev->dev,
> + "Failed to find USB gadget: %d\n", ret);
> + goto err_bat_irq;
> + }
> +
> + power->usb_notify.notifier_call = wm831x_usb_limit_change;
> +
> + ret = usb_charger_register_notify(power->usb_charger,
> + &power->usb_notify);
> + if (ret != 0) {
> + dev_err(&pdev->dev,
> + "Failed to register notifier: %d\n", ret);
> + goto err_usb_charger;
> + }
> + }
> +
> return ret;
>
> +err_usb_charger:
> + usb_charger_put(power->usb_charger);
> err_bat_irq:
> --i;
> for (; i >= 0; i--) {
> @@ -637,6 +700,12 @@ static int wm831x_power_remove(struct platform_device
> *pdev)
> struct wm831x *wm831x = wm831x_power->wm831x;
> int irq, i;
>
> + if (wm831x_power->usb_charger) {
> + usb_charger_unregister_notify(wm831x_power->usb_charger,
> + &wm831x_power->usb_notify);
> + usb_charger_put(wm831x_power->usb_charger);
> + }
> +
> for (i = 0; i < ARRAY_SIZE(wm831x_bat_irqs); i++) {
> irq = wm831x_irq(wm831x,
>platform_get_irq_byname(pdev,
> diff --git a/include/linux/mfd/wm831x/pdata.h
> b/include/linux/mfd/wm831x/pdata.h
> index dcc9631..5af8399 100644
> --- a/include/linux/mfd/wm831x/pdata.h
> +++ b/include/linux/mfd/wm831x/pdata.h
> @@ -126,6 +126,9 @@ struct wm831x_pdata {
> /** The driver should initiate a power off sequence during shutdown */
> bool soft_shutdown;
>
> + /** dev_name of USB charger gadget to integrate with */
> + const char *usb_gadget;
> +
> int irq_base;
> int gpio_base;
> int gpio_defaults[WM831X_GPIO_NUM];
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open sourc
Re: [PATCH v2 0/3] Introduce usb charger framework to deal with the usb gadget power negotation
Hi Baolin, On Fri, Aug 14, 2015 at 05:47:43PM +0800, Baolin Wang wrote: > Currently the Linux kernel does not provide any standard integration of this > feature that integrates the USB subsystem with the system power regulation > provided by PMICs meaning that either vendors must add this in their kernels > or USB gadget devices based on Linux (such as mobile phones) may not behave > as they should. > > Providing a standard framework for doing this in the kernel. > Why not add power supply class support into this? Li Jun -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 08/10] Doc: usb: ci-hdrc-usb2: add tx(rx)-burst-config-dword for binding doc
Am Freitag, den 14.08.2015, 16:40 +0800 schrieb Peter Chen: > On Fri, Aug 14, 2015 at 10:37:30AM +0200, Lucas Stach wrote: > > Am Freitag, den 07.08.2015, 15:15 +0800 schrieb Peter Chen: > > > It is used to override the default setting for burst size, changing > > > burst size takes effect only when the SBUSCFG.AHBBRST = 0. > > > > > > Signed-off-by: Peter Chen > > > --- > > > Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 10 ++ > > > 1 file changed, 10 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > > > b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > > > index d52a747..d71ef07 100644 > > > --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > > > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > > > @@ -37,6 +37,14 @@ Optional properties: > > >property is used to change AHB burst configuration, check the chipidea > > >spec for meaning of each value. If this property is not existed, it > > >will use the reset value. > > > +- tx-burst-size-dword: it is vendor dependent, the tx burst size in dword > > > + (4 bytes), This register represents the maximum length of a the burst > > > + in 32-bit words while moving data from system memory to the USB > > > + bus, changing this value takes effect only the SBUSCFG.AHBBRST is 0. > > > > The last bits about SBUSCFG.AHBBRST don't make any sense in the DT > > binding. The DT writer doesn't know about the SBUSCFG.AHBBRST value and > > should not care either. > > If someone sets the burst size to something > > non-standard in the DT, the driver should make sure to enable to > > necessary bits to make this setting take effect. > > The SBUSCFG.AHBBRST property description is just above this one, > and the user should read the spec for changing these system > configuration parameters. > This is not clear from the description. This should read something like "The value of this property will only take effect if property ahb-burst-config is set to 0." The DT binding should be coherent and understandable without reading the DW USB spec and/or the code. Please take some care about this. > This requirement is from the spec, and the code logic makes sure > the burst size changes only when SBUSCFG.AHBBRST is 0, since the > hardware will only change burst size if SBUSCFG.AHBBRST is 0. > > > > > Also both those descriptions are missing a description to what value the > > burst sizes will be set if the DT properties are not found. If it's > > implementation defined spell this out in the doc. > > It is optional property, if without this property, it will use the > default value. > Then specify this in the binding. Again, the binding is a spec that one should be able to understand without reading the code. "If this property is missing the reset defaults of the hardware implementation will be used." > > > > Are there really cases where it makes sense to set RX and TX burst sizes > > to different values? > > Yes, the default burst size is the same for all SoCs, but the bus > burst size may be larger for some SoCs. > This question wasn't meant to dispute the RX/TX burst size configuration, but I'm questioning if it really makes sense to set them to different values. Do we really need 2 properties for this, or is 1 enough? Do you see any use case where someone would like to do something like: tx-burst-size-dword = <0x8>; rx-burst-size-dword = <0x10>; Regards, Lucas -- Pengutronix e.K. | Lucas Stach | Industrial Linux Solutions | http://www.pengutronix.de/ | -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 07/11] usb: otg: add OTG core
On Fri, Aug 14, 2015 at 12:42:38PM +0300, Roger Quadros wrote: > Hi Peter, > > On 13/07/15 13:20, Roger Quadros wrote: > > On 13/07/15 05:14, Peter Chen wrote: > >> On Wed, Jul 08, 2015 at 01:19:33PM +0300, Roger Quadros wrote: > >>> The OTG core instantiates the OTG Finite State Machine > >>> per OTG controller and manages starting/stopping the > >>> host and gadget controllers based on the bus state. > >>> > >>> It provides APIs for the following tasks > >>> > >>> - Registering an OTG capable controller > >>> - Registering Host and Gadget controllers to OTG core > >>> - Providing inputs to and kicking the OTG state machine > >>> > >>> Signed-off-by: Roger Quadros > >>> --- > >>> MAINTAINERS | 4 +- > >>> drivers/usb/Kconfig | 2 +- > >>> drivers/usb/Makefile | 1 + > >>> drivers/usb/common/Makefile | 3 +- > >>> drivers/usb/common/usb-otg.c | 768 > >>> +++ > >>> drivers/usb/common/usb-otg.h | 71 > >>> drivers/usb/core/Kconfig | 11 +- > >>> include/linux/usb/otg.h | 91 - > >>> 8 files changed, 930 insertions(+), 21 deletions(-) > >>> create mode 100644 drivers/usb/common/usb-otg.c > >>> create mode 100644 drivers/usb/common/usb-otg.h > >>> > >>> diff --git a/MAINTAINERS b/MAINTAINERS > >>> index 8133cef..b21278e 100644 > >>> --- a/MAINTAINERS > >>> +++ b/MAINTAINERS > >>> @@ -10640,12 +10640,14 @@ S: Maintained > >>> F: Documentation/usb/ohci.txt > >>> F: drivers/usb/host/ohci* > >>> > >>> -USB OTG FSM (Finite State Machine) > >>> +USB OTG/DRD core and FSM (Finite State Machine) > >>> M: Peter Chen > >>> +M: Roger Quadros > >>> T: git > >>> git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git > >>> L: linux-usb@vger.kernel.org > >>> S: Maintained > >>> F: drivers/usb/common/usb-otg-fsm.c > >>> +F: drivers/usb/common/usb-otg.c > >>> > >>> USB OVER IP DRIVER > >>> M: Valentina Manea > >>> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig > >>> index 8ed451d..5b625e2 100644 > >>> --- a/drivers/usb/Kconfig > >>> +++ b/drivers/usb/Kconfig > >>> @@ -32,7 +32,7 @@ if USB_SUPPORT > >>> config USB_COMMON > >>> tristate > >>> default y > >>> - depends on USB || USB_GADGET > >>> + depends on USB || USB_GADGET || USB_OTG > >>> > >> > >> USB_OTG can depends on USB || UB_GADGET? > > > > I didn't understand. The above is for USB_COMMON. > > > >> > >>> config USB_ARCH_HAS_HCD > >>> def_bool y > >>> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile > >>> index d8926c6..769d13b 100644 > >>> --- a/drivers/usb/Makefile > >>> +++ b/drivers/usb/Makefile > >>> @@ -60,5 +60,6 @@ obj-$(CONFIG_USB_RENESAS_USBHS) += renesas_usbhs/ > >>> obj-$(CONFIG_USB_GADGET) += gadget/ > >>> > >>> obj-$(CONFIG_USB_COMMON) += common/ > >>> +obj-$(CONFIG_USB_OTG)+= common/ > >>> > >> > >> The comment like above. > > > > What should it look like? > > > > Can you please clarify what you meant at the above two > comments? Thanks. > Forget them, I had thought the USB_OTG could be module. -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][PATCH 1/2] usb: gadget: configfs: add MTP function
On 14 August 2015 at 02:11, Krzysztof Opasiak wrote: > > > On 08/13/2015 09:57 PM, Greg Kroah-Hartman wrote: >> >> On Thu, Aug 13, 2015 at 09:34:46PM +0200, Krzysztof Opasiak wrote: >>> >>> Hello, >>> >>> On 08/13/2015 09:12 PM, Amit Pundir wrote: his MTP function is based on years of work originally done in the Android kernel tree by: Mike Lockwood Benoit Goby Colin Cross Arve Hjønnevåg Peter Oh Greg Hackmann Badhri Jagan Sridharan I've folded the series up to make it easier to review, and to provide a coherent patch description. Post Gingerbread (Android v2.3), Android dropped USB Mass Storage in favor of Media Transfer Protocal (MTP), which is widely used for transferring media files to digital music players and similar applications. This USB gadget function implements MTP functionalty. Historically this function has been a part of Android composite gadget driver. Android composite driver was Android's solution for dynamic gadget function switching prior to the ConfigFS gadget being merged. There were failed few attempts in past http://marc.info/?l=linux-usb&m=132451695808552 to upstream Android composite driver as well. Now this Android MTP gadget function has been re-implemented so as to be used as a generic ConfigFS function instead. Again, many thanks to Mike, Benoit, Colin, Arve, Peter, Greg and Badhri, as they are the real authors of this work. However, I've folded their patches together and modified it enough that I don't want them to be blamed for any mistakes I've made condensing their patches down. Cc: Mike Lockwood Cc: Benoit Goby Cc: Colin Cross Cc: Arve Hjønnevåg Cc: Peter Oh Cc: Greg Hackmann Cc: Badhri Jagan Sridharan Cc: Android Kernel Team Cc: Greg Kroah-Hartman Cc: Jonathan Corbet Cc: Felipe Balbi Cc: Andrzej Pietrasiewicz Cc: Laurent Pinchart Cc: Yegor Yefremov Cc: Philippe Reynes Cc: John Stultz Cc: Sumit Semwal Signed-off-by: Amit Pundir >>> >>> >>> In my humble opinion adding such function to Linux kernel doesn't make >>> any >>> sense. By design, MTP is a protocol which requires access to userspace >>> features esp. file system. It is very important to run MTP daemon with >>> suitable user and LSM label and many many other issues which should be >>> handled by userspace access policy. >>> >>> Moreover this is not a fully functional USB function but only some >>> interface >>> which can be used by mtp-responder (mtp-daemon - call it as you like) to >>> communicate with host. As we have FunctionFS which allows to implement >>> any >>> USB function in as a userspace service. As MTP nature is more related to >>> userspace I think that porting MTP daemon to use this is a right way to >>> go. >>> This should be much more reasonable than adding new function which also >>> requires daemon for proper working. So why add another interface while we >>> can use a generic one? Fairly valid point. I did see MTP mentioned in FunctionFS context in Documentation/usb/functionfs.txt but I could not find an open userpsace MTP daemon implementation based on F_FS to try. >> >> >> Isn't there already a userspace MTP daemon that uses the existing >> functionfs for usb gadgets? I thought I remember seeing that >> somewhere... >> > > I've found some interesting link[2] which may mean that Sailfish OS guys has > some mtp implementation with functionfs backend: > > < cite > > - /dev/mtp > mtp functionfs rw,relatime > >> cite > > Started digging and got it! > > This looks like mtp with ffs backend: > > https://github.com/nemomobile/buteo-mtp Thanks for the pointer. This "buteo-mtp" looks interesting. Regards, Amit Pundir > > Didn't tested, even didn't try to compile, no guarantee;) > > Footnotes: > 1 - > http://reviewjolla.blogspot.com/2014/06/techspecs-android-on-jolla-phone.html > > > Best regards, > > -- > Krzysztof Opasiak > Samsung R&D Institute Poland > Samsung Electronics -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/3] power: wm831x_power: Support USB charger current limit management
Integrate with the newly added USB charger interface to limit the current
we draw from the USB input based on the input device configuration
identified by the USB stack, allowing us to charge more quickly from high
current inputs without drawing more current than specified from others.
Signed-off-by: Mark Brown
Signed-off-by: Baolin Wang
---
drivers/power/wm831x_power.c | 69 ++
include/linux/mfd/wm831x/pdata.h |3 ++
2 files changed, 72 insertions(+)
diff --git a/drivers/power/wm831x_power.c b/drivers/power/wm831x_power.c
index db11ae6..72c661f 100644
--- a/drivers/power/wm831x_power.c
+++ b/drivers/power/wm831x_power.c
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#include
#include
@@ -31,6 +32,8 @@ struct wm831x_power {
char usb_name[20];
char battery_name[20];
bool have_battery;
+ struct usb_charger *usb_charger;
+ struct notifier_block usb_notify;
};
static int wm831x_power_check_online(struct wm831x *wm831x, int supply,
@@ -125,6 +128,43 @@ static enum power_supply_property wm831x_usb_props[] = {
POWER_SUPPLY_PROP_VOLTAGE_NOW,
};
+/* In miliamps */
+static unsigned int wm831x_usb_limits[] = {
+ 0,
+ 2,
+ 100,
+ 500,
+ 900,
+ 1500,
+ 1800,
+ 550,
+};
+
+static int wm831x_usb_limit_change(struct notifier_block *nb,
+ unsigned long limit, void *data)
+{
+ struct wm831x_power *wm831x_power = container_of(nb,
+struct wm831x_power,
+usb_notify);
+ int i, best;
+
+ /* Find the highest supported limit */
+ best = 0;
+ for (i = 0; i < ARRAY_SIZE(wm831x_usb_limits); i++) {
+ if (limit < wm831x_usb_limits[i] &&
+ wm831x_usb_limits[best] < wm831x_usb_limits[i])
+ best = i;
+ }
+
+ dev_dbg(wm831x_power->wm831x->dev,
+ "Limiting USB current to %dmA", wm831x_usb_limits[best]);
+
+ wm831x_set_bits(wm831x_power->wm831x, WM831X_POWER_STATE,
+ WM831X_USB_ILIM_MASK, best);
+
+ return 0;
+}
+
/*
* Battery properties
*/
@@ -606,8 +646,31 @@ static int wm831x_power_probe(struct platform_device *pdev)
}
}
+ if (wm831x_pdata && wm831x_pdata->usb_gadget) {
+ power->usb_charger =
+ usb_charger_find_by_name(wm831x_pdata->usb_gadget);
+ if (IS_ERR(power->usb_charger)) {
+ ret = PTR_ERR(power->usb_charger);
+ dev_err(&pdev->dev,
+ "Failed to find USB gadget: %d\n", ret);
+ goto err_bat_irq;
+ }
+
+ power->usb_notify.notifier_call = wm831x_usb_limit_change;
+
+ ret = usb_charger_register_notify(power->usb_charger,
+ &power->usb_notify);
+ if (ret != 0) {
+ dev_err(&pdev->dev,
+ "Failed to register notifier: %d\n", ret);
+ goto err_usb_charger;
+ }
+ }
+
return ret;
+err_usb_charger:
+ usb_charger_put(power->usb_charger);
err_bat_irq:
--i;
for (; i >= 0; i--) {
@@ -637,6 +700,12 @@ static int wm831x_power_remove(struct platform_device
*pdev)
struct wm831x *wm831x = wm831x_power->wm831x;
int irq, i;
+ if (wm831x_power->usb_charger) {
+ usb_charger_unregister_notify(wm831x_power->usb_charger,
+ &wm831x_power->usb_notify);
+ usb_charger_put(wm831x_power->usb_charger);
+ }
+
for (i = 0; i < ARRAY_SIZE(wm831x_bat_irqs); i++) {
irq = wm831x_irq(wm831x,
platform_get_irq_byname(pdev,
diff --git a/include/linux/mfd/wm831x/pdata.h b/include/linux/mfd/wm831x/pdata.h
index dcc9631..5af8399 100644
--- a/include/linux/mfd/wm831x/pdata.h
+++ b/include/linux/mfd/wm831x/pdata.h
@@ -126,6 +126,9 @@ struct wm831x_pdata {
/** The driver should initiate a power off sequence during shutdown */
bool soft_shutdown;
+ /** dev_name of USB charger gadget to integrate with */
+ const char *usb_gadget;
+
int irq_base;
int gpio_base;
int gpio_defaults[WM831X_GPIO_NUM];
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 08/10] Doc: usb: ci-hdrc-usb2: add tx(rx)-burst-config-dword for binding doc
On Fri, Aug 14, 2015 at 10:37:30AM +0200, Lucas Stach wrote: > Am Freitag, den 07.08.2015, 15:15 +0800 schrieb Peter Chen: > > It is used to override the default setting for burst size, changing > > burst size takes effect only when the SBUSCFG.AHBBRST = 0. > > > > Signed-off-by: Peter Chen > > --- > > Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 10 ++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > > b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > > index d52a747..d71ef07 100644 > > --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > > @@ -37,6 +37,14 @@ Optional properties: > >property is used to change AHB burst configuration, check the chipidea > >spec for meaning of each value. If this property is not existed, it > >will use the reset value. > > +- tx-burst-size-dword: it is vendor dependent, the tx burst size in dword > > + (4 bytes), This register represents the maximum length of a the burst > > + in 32-bit words while moving data from system memory to the USB > > + bus, changing this value takes effect only the SBUSCFG.AHBBRST is 0. > > The last bits about SBUSCFG.AHBBRST don't make any sense in the DT > binding. The DT writer doesn't know about the SBUSCFG.AHBBRST value and > should not care either. > If someone sets the burst size to something > non-standard in the DT, the driver should make sure to enable to > necessary bits to make this setting take effect. The SBUSCFG.AHBBRST property description is just above this one, and the user should read the spec for changing these system configuration parameters. This requirement is from the spec, and the code logic makes sure the burst size changes only when SBUSCFG.AHBBRST is 0, since the hardware will only change burst size if SBUSCFG.AHBBRST is 0. > > Also both those descriptions are missing a description to what value the > burst sizes will be set if the DT properties are not found. If it's > implementation defined spell this out in the doc. It is optional property, if without this property, it will use the default value. > > Are there really cases where it makes sense to set RX and TX burst sizes > to different values? Yes, the default burst size is the same for all SoCs, but the bus burst size may be larger for some SoCs. -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/3] gadget: Introduce the usb charger framework
This patch introduces the usb charger driver based on usb gadget that
makes an enhancement to a power driver. It works well in practice but
that requires a system with suitable hardware.
The basic conception of the usb charger is that, when one usb charger
is added or removed by reporting from the usb gadget state change or
the extcon device state change, the usb charger will report to power
user to set the current limitation.
The usb charger will register notifiees on the usb gadget or the extcon
device to get notified the usb charger state.
Power user will register a notifiee on the usb charger to get notified
by status changes from the usb charger. It will report to power user
to set the current limitation when detecting the usb charger is added
or removed from extcon device state or usb gadget state.
Signed-off-by: Baolin Wang
---
drivers/usb/gadget/Kconfig |7 +
drivers/usb/gadget/Makefile |1 +
drivers/usb/gadget/charger.c| 561 +++
include/linux/usb/usb_charger.h | 145 ++
4 files changed, 714 insertions(+)
create mode 100644 drivers/usb/gadget/charger.c
create mode 100644 include/linux/usb/usb_charger.h
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index bcf83c0..3d2b959 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -127,6 +127,13 @@ config USB_GADGET_STORAGE_NUM_BUFFERS
a module parameter as well.
If unsure, say 2.
+config USB_CHARGER
+ bool "USB charger support"
+ help
+ The usb charger driver based on the usb gadget that makes an
+ enhancement to a power driver which can set the current limitation
+ when the usb charger is added or removed.
+
source "drivers/usb/gadget/udc/Kconfig"
#
diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
index 598a67d..1e421c1 100644
--- a/drivers/usb/gadget/Makefile
+++ b/drivers/usb/gadget/Makefile
@@ -10,3 +10,4 @@ libcomposite-y:= usbstring.o config.o
epautoconf.o
libcomposite-y += composite.o functions.o configfs.o u_f.o
obj-$(CONFIG_USB_GADGET) += udc/ function/ legacy/
+obj-$(CONFIG_USB_CHARGER) += charger.o
diff --git a/drivers/usb/gadget/charger.c b/drivers/usb/gadget/charger.c
new file mode 100644
index 000..f24f7b7
--- /dev/null
+++ b/drivers/usb/gadget/charger.c
@@ -0,0 +1,561 @@
+/*
+ * usb charger.c -- USB charger driver
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DEFAULT_CUR_PROTECT(50)
+#define DEFAULT_SDP_CUR_LIMIT (500 - DEFAULT_CUR_PROTECT)
+#define DEFAULT_DCP_CUR_LIMIT (1500 - DEFAULT_CUR_PROTECT)
+#define DEFAULT_CDP_CUR_LIMIT (1500 - DEFAULT_CUR_PROTECT)
+#define DEFAULT_ACA_CUR_LIMIT (1500 - DEFAULT_CUR_PROTECT)
+
+static DEFINE_IDA(usb_charger_ida);
+static struct bus_type usb_charger_subsys = {
+ .name = "usb-charger",
+ .dev_name = "usb-charger",
+};
+
+static struct usb_charger *dev_to_uchger(struct device *udev)
+{
+ return container_of(udev, struct usb_charger, dev);
+}
+
+static ssize_t usb_charger_cur_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct usb_charger *uchger = dev_to_uchger(dev);
+
+ return scnprintf(buf, PAGE_SIZE, "%d %d %d %d\n",
+uchger->cur_limit.sdp_cur_limit,
+uchger->cur_limit.dcp_cur_limit,
+uchger->cur_limit.cdp_cur_limit,
+uchger->cur_limit.aca_cur_limit);
+}
+
+static ssize_t usb_charger_cur_store(struct device *dev,
+struct device_attribute *attr,
+const char *buf, size_t count)
+{
+ struct usb_charger *uchger = dev_to_uchger(dev);
+ struct usb_charger_cur_limit cur;
+ int ret;
+
+ ret = sscanf(buf, "%d %d %d %d",
+&cur.sdp_cur_limit, &cur.dcp_cur_limit,
+&cur.cdp_cur_limit, &cur.aca_cur_limit);
+ if (ret == 0)
+ return -EINVAL;
+
+ ret = usb_charger_set_cur_limit(uchger, &cur);
+ if (ret < 0)
+ return ret;
+
+ return count;
+}
+static DEVICE_ATTR(cur_limit, 0644, usb_charger_cur_show,
usb_charger_cur_store);
+
+static struct attribute *uchger_attributes[] = {
+ &dev_attr_cur_limit.attr,
+ NULL
+};
+
+static const struct attribute_group uchger_attr_group = {
+ .attrs = uchger_attributes,
+};
+
+/*
+ *
[PATCH v2 1/3] gadget: Support for the usb charger framework
The usb charger framework is based on usb gadget, and each usb gadget
can be one usb charger to set the current limitation.
This patch adds a notifier mechanism for usb charger to report to usb
charger when the usb gadget state is changed.
Also we introduce a callback 'get_charger_type' which will implemented
by user for usb gadget operations to get the usb charger type.
Signed-off-by: Baolin Wang
---
drivers/usb/gadget/udc/udc-core.c | 38 +
include/linux/usb/gadget.h| 20 +++
2 files changed, 58 insertions(+)
diff --git a/drivers/usb/gadget/udc/udc-core.c
b/drivers/usb/gadget/udc/udc-core.c
index f660afb..47b231c 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -28,6 +28,7 @@
#include
#include
#include
+#include
/**
* struct usb_udc - describes one usb device controller
@@ -129,6 +130,32 @@ void usb_gadget_giveback_request(struct usb_ep *ep,
}
EXPORT_SYMBOL_GPL(usb_gadget_giveback_request);
+int usb_gadget_register_notify(struct usb_gadget *gadget,
+ struct notifier_block *nb)
+{
+ int ret;
+
+ mutex_lock(&gadget->lock);
+ ret = raw_notifier_chain_register(&gadget->nh, nb);
+ mutex_unlock(&gadget->lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(usb_gadget_register_notify);
+
+int usb_gadget_unregister_notify(struct usb_gadget *gadget,
+struct notifier_block *nb)
+{
+ int ret;
+
+ mutex_lock(&gadget->lock);
+ ret = raw_notifier_chain_unregister(&gadget->nh, nb);
+ mutex_unlock(&gadget->lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(usb_gadget_unregister_notify);
+
/* - */
/**
@@ -226,6 +253,10 @@ static void usb_gadget_state_work(struct work_struct *work)
struct usb_gadget *gadget = work_to_gadget(work);
struct usb_udc *udc = gadget->udc;
+ mutex_lock(&gadget->lock);
+ raw_notifier_call_chain(&gadget->nh, gadget->state, gadget);
+ mutex_unlock(&gadget->lock);
+
if (udc)
sysfs_notify(&udc->dev.kobj, NULL, "state");
}
@@ -364,6 +395,8 @@ int usb_add_gadget_udc_release(struct device *parent,
struct usb_gadget *gadget,
dev_set_name(&gadget->dev, "gadget");
INIT_WORK(&gadget->work, usb_gadget_state_work);
+ RAW_INIT_NOTIFIER_HEAD(&gadget->nh);
+ mutex_init(&gadget->lock);
gadget->dev.parent = parent;
#ifdef CONFIG_HAS_DMA
@@ -405,6 +438,10 @@ int usb_add_gadget_udc_release(struct device *parent,
struct usb_gadget *gadget,
mutex_unlock(&udc_lock);
+ ret = usb_charger_init(gadget);
+ if (ret)
+ goto err4;
+
return 0;
err4:
@@ -481,6 +518,7 @@ void usb_del_gadget_udc(struct usb_gadget *gadget)
kobject_uevent(&udc->dev.kobj, KOBJ_REMOVE);
flush_work(&gadget->work);
device_unregister(&udc->dev);
+ usb_charger_exit(gadget);
device_unregister(&gadget->dev);
}
EXPORT_SYMBOL_GPL(usb_del_gadget_udc);
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index c14a69b..78cc862 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -537,6 +537,7 @@ struct usb_gadget_ops {
struct usb_ep *(*match_ep)(struct usb_gadget *,
struct usb_endpoint_descriptor *,
struct usb_ss_ep_comp_descriptor *);
+ enum usb_charger_type (*get_charger_type)(struct usb_gadget *);
};
/**
@@ -609,6 +610,9 @@ struct usb_gadget {
unsignedout_epnum;
unsignedin_epnum;
struct usb_otg_caps *otg_caps;
+ struct raw_notifier_headnh;
+ struct usb_charger *uchger;
+ struct mutexlock;
unsignedsg_supported:1;
unsignedis_otg:1;
@@ -1183,6 +1187,22 @@ extern void usb_gadget_unmap_request(struct usb_gadget
*gadget,
/*-*/
+/**
+ * Register a notifiee to get notified by any attach status changes from
+ * the usb gadget
+ */
+int usb_gadget_register_notify(struct usb_gadget *gadget,
+ struct notifier_block *nb);
+
+/*-*/
+
+
+/* Unregister a notifiee from the usb gadget */
+int usb_gadget_unregister_notify(struct usb_gadget *gadget,
+struct notifier_block *nb);
+
+/*-*/
+
/* utility to set gadget state properly */
extern void usb_gadget_set_state(struct usb_gadget *gadget,
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a mess
[PATCH v2 0/3] Introduce usb charger framework to deal with the usb gadget power negotation
Currently the Linux kernel does not provide any standard integration of this feature that integrates the USB subsystem with the system power regulation provided by PMICs meaning that either vendors must add this in their kernels or USB gadget devices based on Linux (such as mobile phones) may not behave as they should. Providing a standard framework for doing this in the kernel. Now introduce one user with wm831x_power to support and test the usb charger, which is pending testing. Moreover there may be other potential users will use it in future. Changes since v1: - Rebase my branch and re-create the patch series. - Remove the extcon wrappers and gadget wrappers for registering a notifier. - Remove the user count in usb charger. - Use idr instead of my own logic. - Replace the spinlock with mutex. - Use the list of driver core to manage the usb charger device. - Add the Kconfig and Makefile changes. - Add some attribute files to show or store the current limitation. - Introduce one user of usb charger by Mark Brown. - Other modifications. Baolin Wang (3): gadget: Support for the usb charger framework gadget: Introduce the usb charger framework power: wm831x_power: Support USB charger current limit management drivers/power/wm831x_power.c | 69 + drivers/usb/gadget/Kconfig|7 + drivers/usb/gadget/Makefile |1 + drivers/usb/gadget/charger.c | 561 + drivers/usb/gadget/udc/udc-core.c | 38 +++ include/linux/mfd/wm831x/pdata.h |3 + include/linux/usb/gadget.h| 20 ++ include/linux/usb/usb_charger.h | 145 ++ 8 files changed, 844 insertions(+) create mode 100644 drivers/usb/gadget/charger.c create mode 100644 include/linux/usb/usb_charger.h -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 07/11] usb: otg: add OTG core
Hi Peter, On 13/07/15 13:20, Roger Quadros wrote: > On 13/07/15 05:14, Peter Chen wrote: >> On Wed, Jul 08, 2015 at 01:19:33PM +0300, Roger Quadros wrote: >>> The OTG core instantiates the OTG Finite State Machine >>> per OTG controller and manages starting/stopping the >>> host and gadget controllers based on the bus state. >>> >>> It provides APIs for the following tasks >>> >>> - Registering an OTG capable controller >>> - Registering Host and Gadget controllers to OTG core >>> - Providing inputs to and kicking the OTG state machine >>> >>> Signed-off-by: Roger Quadros >>> --- >>> MAINTAINERS | 4 +- >>> drivers/usb/Kconfig | 2 +- >>> drivers/usb/Makefile | 1 + >>> drivers/usb/common/Makefile | 3 +- >>> drivers/usb/common/usb-otg.c | 768 >>> +++ >>> drivers/usb/common/usb-otg.h | 71 >>> drivers/usb/core/Kconfig | 11 +- >>> include/linux/usb/otg.h | 91 - >>> 8 files changed, 930 insertions(+), 21 deletions(-) >>> create mode 100644 drivers/usb/common/usb-otg.c >>> create mode 100644 drivers/usb/common/usb-otg.h >>> >>> diff --git a/MAINTAINERS b/MAINTAINERS >>> index 8133cef..b21278e 100644 >>> --- a/MAINTAINERS >>> +++ b/MAINTAINERS >>> @@ -10640,12 +10640,14 @@ S:Maintained >>> F: Documentation/usb/ohci.txt >>> F: drivers/usb/host/ohci* >>> >>> -USB OTG FSM (Finite State Machine) >>> +USB OTG/DRD core and FSM (Finite State Machine) >>> M: Peter Chen >>> +M: Roger Quadros >>> T: git git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git >>> L: linux-usb@vger.kernel.org >>> S: Maintained >>> F: drivers/usb/common/usb-otg-fsm.c >>> +F: drivers/usb/common/usb-otg.c >>> >>> USB OVER IP DRIVER >>> M: Valentina Manea >>> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig >>> index 8ed451d..5b625e2 100644 >>> --- a/drivers/usb/Kconfig >>> +++ b/drivers/usb/Kconfig >>> @@ -32,7 +32,7 @@ if USB_SUPPORT >>> config USB_COMMON >>> tristate >>> default y >>> - depends on USB || USB_GADGET >>> + depends on USB || USB_GADGET || USB_OTG >>> >> >> USB_OTG can depends on USB || UB_GADGET? > > I didn't understand. The above is for USB_COMMON. > >> >>> config USB_ARCH_HAS_HCD >>> def_bool y >>> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile >>> index d8926c6..769d13b 100644 >>> --- a/drivers/usb/Makefile >>> +++ b/drivers/usb/Makefile >>> @@ -60,5 +60,6 @@ obj-$(CONFIG_USB_RENESAS_USBHS) += renesas_usbhs/ >>> obj-$(CONFIG_USB_GADGET) += gadget/ >>> >>> obj-$(CONFIG_USB_COMMON) += common/ >>> +obj-$(CONFIG_USB_OTG) += common/ >>> >> >> The comment like above. > > What should it look like? > Can you please clarify what you meant at the above two comments? Thanks. cheers, -roger -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb/gadget/function/u_ether.c: Allow jumbo frames
ping...
More than a week has passed, haven't seen any response though. Any comments?
On 05-08-15 08:54, Mike Looijmans wrote:
USB network adapters support Jumbo frames. The only thing blocking
that feature is the code in the gadget driver that disposes of
packets larger than 1518 bytes, and the limit on the ioctl to set
the mtu.
This patch relaxes these limits, and allows up to 15k frames sizes.
The 15k value was chosen because 16k does not work on all platforms,
and usingclose to 16k will result in allocating 5 or 8 4k pages to
store the skb, wasting pages at no measurable performance gain.
On a topic-miami board (Zynq-7000), iperf3 performance reports:
MTU= 1500, PC-to-gadget: 139 Mbps, Gadget-to-PC: 116 Mbps
MTU=15000, PC-to-gadget: 239 Mbps, Gadget-to-PC: 361 Mbps
On boards with slower CPUs the performance improvement will be
relatively much larger, e.g. an OMAP-L138 increased from 40 to
220 Mbps using a similar patch on an 2.6.37 kernel.
Signed-off-by: Mike Looijmans
---
drivers/usb/gadget/function/u_ether.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/function/u_ether.c
b/drivers/usb/gadget/function/u_ether.c
index f1fd777..6828ea2 100644
--- a/drivers/usb/gadget/function/u_ether.c
+++ b/drivers/usb/gadget/function/u_ether.c
@@ -48,6 +48,11 @@
#define UETH__VERSION "29-May-2008"
+/* Experiments show that both Linux and Windows hosts allow up to 16k
+ * frame sizes. Set the max size to 15k+52 to prevent allocating 32k
+ * blocks and still have efficient handling. */
+#define GETHER_MAX_ETH_FRAME_LEN 15412
+
struct eth_dev {
/* lock is held while accessing port_usb
*/
@@ -146,7 +151,7 @@ static int ueth_change_mtu(struct net_device *net, int
new_mtu)
spin_lock_irqsave(&dev->lock, flags);
if (dev->port_usb)
status = -EBUSY;
- else if (new_mtu <= ETH_HLEN || new_mtu > ETH_FRAME_LEN)
+ else if (new_mtu <= ETH_HLEN || new_mtu > GETHER_MAX_ETH_FRAME_LEN)
status = -ERANGE;
else
net->mtu = new_mtu;
@@ -294,7 +299,7 @@ static void rx_complete(struct usb_ep *ep, struct
usb_request *req)
while (skb2) {
if (status < 0
|| ETH_HLEN > skb2->len
- || skb2->len > VLAN_ETH_FRAME_LEN) {
+ || skb2->len >
GETHER_MAX_ETH_FRAME_LEN) {
dev->net->stats.rx_errors++;
dev->net->stats.rx_length_errors++;
DBG(dev, "rx length %d\n", skb2->len);
Kind regards,
Mike Looijmans
System Expert
TOPIC Embedded Products
Eindhovenseweg 32-C, NL-5683 KH Best
Postbus 440, NL-5680 AK Best
Telefoon: +31 (0) 499 33 69 79
Telefax: +31 (0) 499 33 69 70
E-mail: mike.looijm...@topicproducts.com
Website: www.topicproducts.com
Please consider the environment before printing this e-mail
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: chipidea: reuse the platform_data to store the ci info
On Thu, Aug 13, 2015 at 11:01:18AM -0400, Alan Stern wrote: > On Thu, 13 Aug 2015, Peter Chen wrote: > > > > > Alan, do you have any suggestions? Currently, IP core driver and ehci > > > > core both takes its internal structure as driver data. Thanks. > > > > > > It's not just ehci-hcd: The USB core stores the hcd address as driver > > > data. usb_create_shared_hcd() does: > > > > > > dev_set_drvdata(dev, hcd); > > > > > > (The core uses this value only in usb_hcd_platform_shutdown(), but > > > other platform-glue drivers also store the hcd address there.) > > > > > > The standard answer is to put the Chipidea private data in the > > > ehci_ci_priv structure. That's what it's meant for. > > > > > > Is that okay for you? > > > > Thanks, Alan. It is ok for ehci glue layer as a separate driver. > > But it is not ok for the drivers like having all hcd/gadget/otg > > part together, the IP core driver's suspend/resume/remove, > > sysfs will use driver's private data, unless the ehci platform > > device as child for IP core device, but it is not for gadget. > > > > If there is no better solution, I will use Rong's solution. > > In the old days, a single device could be a USB host controller and > nothing else. Then later, a single device could be either a host > controller or a device controller. Now a single device can be both. > > Obviously this causes problems for our original design, where the > device's drvdata stored a pointer to the hcd structure or the udc > structure. > > In principle, the drvdata should point to something that contains > pointers to both the hcd and udc structures. Maybe also an otg > structure. That would require a lot of changes to existing code but it > might be worthwhile. > Yes, the core device drvdata should contain standard hcd, gadget, otg structure together. Currently, the gadget is child device for core device, it is reasonable the same for hcd, and we can create device for chipidea host to achieve it. The dwc3 driver does the similar things. Below is the reference device hierarchy for current IP driver: Glue Layer Device | | | IP Core Device | ||| ||| Gadget hcd otg Alan/Felipe, what do you think? > Felipe, what do you think? > > Alan Stern > -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 08/10] Doc: usb: ci-hdrc-usb2: add tx(rx)-burst-config-dword for binding doc
Am Freitag, den 07.08.2015, 15:15 +0800 schrieb Peter Chen: > It is used to override the default setting for burst size, changing > burst size takes effect only when the SBUSCFG.AHBBRST = 0. > > Signed-off-by: Peter Chen > --- > Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt | 10 ++ > 1 file changed, 10 insertions(+) > > diff --git a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > index d52a747..d71ef07 100644 > --- a/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > +++ b/Documentation/devicetree/bindings/usb/ci-hdrc-usb2.txt > @@ -37,6 +37,14 @@ Optional properties: >property is used to change AHB burst configuration, check the chipidea >spec for meaning of each value. If this property is not existed, it >will use the reset value. > +- tx-burst-size-dword: it is vendor dependent, the tx burst size in dword > + (4 bytes), This register represents the maximum length of a the burst > + in 32-bit words while moving data from system memory to the USB > + bus, changing this value takes effect only the SBUSCFG.AHBBRST is 0. The last bits about SBUSCFG.AHBBRST don't make any sense in the DT binding. The DT writer doesn't know about the SBUSCFG.AHBBRST value and should not care either. If someone sets the burst size to something non-standard in the DT, the driver should make sure to enable to necessary bits to make this setting take effect. Also both those descriptions are missing a description to what value the burst sizes will be set if the DT properties are not found. If it's implementation defined spell this out in the doc. Are there really cases where it makes sense to set RX and TX burst sizes to different values? Regards, Lucas > +- rx-burst-size-dword: it is vendor dependent, the rx burst size in dword > + (4 bytes), This register represents the maximum length of a the burst > + in 32-bit words while moving data from the USB bus to system memory, > + changing this value takes effect only the SBUSCFG.AHBBRST is 0. > > Example: > > @@ -51,4 +59,6 @@ Example: > gadget-itc-setting = <0x4>; /* 4 micro-frames */ >/* Incremental burst of unspecified length */ > ahb-burst-config = <0x0>; > + tx-burst-size-dword = <0x10>; /* 64 bytes */ > + rx-burst-size-dword = <0x10>; > }; -- Pengutronix e.K. | Lucas Stach | Industrial Linux Solutions | http://www.pengutronix.de/ | -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 4.2 kernel trace when hot unplug a mounted USB/SATA/MMC devices with ext2/ext3/ext4 file system
On Thu 13-08-15 09:01:28, Darrick J. Wong wrote:
> On Wed, Aug 12, 2015 at 08:17:28PM -0700, Duc Dang wrote:
> > Hi Theodore, Andreas, Jan, Andrew and All,
> >
> > We are seeing kernel trace when we disconnect an USB/SATA/MMC devices
> > that has its ext2/ext3/ext4 partition currently mounted. These traces
> > are seen with kernel 4.2-rc5 on APM Arm64 X-Gene platforms. Similar
> > issue also happens on an x86 machine (Lenovo T420s running Fedora Core
> > 22 (Linux 4.1.x))
>
> Sounds like this issue:
> https://bugzilla.kernel.org/show_bug.cgi?id=101011
Hum, the above bug mentions that at least for ext4 this is a regression
caused by commit 08439fec26 "ext4: remove block_device_ejected" from
Christoph, let's add him to CC. The assumption of the commit is that
bdi->dev never goes away so there's no point in checking it but these guys
are apparently able to make bdi go away before fs is done with it... Any
idea Christoph?
Honza
> > We also tested with xfs kernel and kernel complains with some error
> > message but there is no trace:
> >
> > usb 1-1: USB disconnect, device number 3
> > XFS (sda1): Unmounting Filesystem
> > XFS (sda1): metadata I/O error: block 0x1ddc12 ("xlog_iodone") error 19
> > numblks
> > 64
> > XFS (sda1): xfs_do_force_shutdown(0x2) called from line 1180 of file
> > /projects/svdc/P4wsIPCSW/buildsw_shadowcat_2_03/shadowcat/linux/fs/xfs/xfs_log.c.
> > Return address = 0xffc0003aa8d8
> > XFS (sda1): Log I/O Error Detected. Shutting down filesystem
> > XFS (sda1): Unable to update superblock counters. Freespace may not be
> > correct
> > on next mount.
> > XFS (sda1): xfs_log_force: error -5 returned.
> > XFS (sda1): Please umount the filesystem and rectify the problem(s)
> >
> > Below are traces for each case. Do you aware of this issue and is
> > there a fix for it?
> >
> > CASE 1: SATA with ext2 partition:
> >
> > EXT2-fs (sda1): previous I/O error to superblock detected
> >
> > Unable to handle kernel paging request at virtual address 1ff256000
> > pgd = ffc0e784b000
> > [1ff256000] *pgd=, *pud=
> > Internal error: Oops: 9605 [#1] PREEMPT SMP
> > Modules linked in:
> > CPU: 4 PID: 1484 Comm: umount Not tainted 4.1.0-xgene_sw_2.03.05-beta_rc_pm
> > #1
> > Hardware name: APM X-Gene Mustang board (DT)
> > task: ffc1ed5d3840 ti: ffc0e79dc000 task.ti: ffc0e79dc000
> > PC is at __percpu_counter_add+0x2c/0x10c
> > LR is at account_page_dirtied+0x78/0x12c
> > pc : [] lr : [] pstate: 81c5
> > sp : ffc0e79dfc40
> > x29: ffc0e79dfc40 x28: ffc0e79dc000
> > x27: ffc000927000 x26: 0027
> > x25: 011a x24: 0015
> > x23: 8000 x22: 0140
> > x21: ffc000d33000 x20: ffc1f6440fb8
> > x19: ffbec7b3e500 x18:
> > x17: 007f8d95bfd0 x16: ffc0001dfad0
> > x15: 003b9aca x14: 0ffe
> > x13: 0020 x12: 0101010101010101
> > x11: 0174 x10: 0006
> > x9 : ffc1fff1e35c x8 : 746564206b636f6c
> > x7 : ffc000d33618 x6 : 0001ff256000
> > x5 : ffc0e79dfc40 x4 : ffc0e79dc000
> > x3 : x2 : 0020
> > x1 : 0001 x0 : 0001ff256000
> >
> > Process umount (pid: 1484, stack limit = 0xffc0e79dc020)
> > Stack: (0xffc0e79dfc40 to 0xffc0e79e)
> > fc40: e79dfc70 ffc0 00147180 ffc0 c7b3e500 ffbe f6440f78
> > ffc1
> > fc60: e79dfc70 ffc0 0014716c ffc0 e79dfcb0 ffc0 001ccd7c
> > ffc0
> > fc80: c7b3e500 ffbe f6b28f48 ffc1 f6b28f60 ffc1 e79dfca0
> > ffc0
> > fca0: f6b28f48 ffc1 e79dfd00 ffc0 001cdfd4
> > ffc0
> > fcc0: c7b3e500 ffbe ecf94400 ffc1 ee6022d8 ffc0
> >
> > fce0: 8000 00226674 ffc0 e79dfd20 ffc0
> >
> > fd00: e79dfd20 ffc0 0022ac84 ffc0 f6ff7000 ffc1 ecf94400
> > ffc1
> > fd20: e79dfd50 ffc0 0022ad0c ffc0 f677e380 ffc1 ecf94400
> > ffc1
> > fd40: f6ff7000 ffc1 ed5d3840 ffc1 e79dfd90 ffc0 001cb0c4
> > ffc0
> > fd60: f6ff7000 ffc1 f6ff70a8 ffc1 00946080 ffc0 001b5ff0
> > ffc0
> > fd80: f6ff7000 ffc1 e79dfdb0 ffc0 001a05e0
> > ffc0
> > fda0: f6ff7000 ffc1 0083 e79dfde0 ffc0 001a0958
> > ffc0
> > fdc0: f6b28d00 ffc1 0083 00db9000 ffc0 0015
> >
> > fde0: e79dfe10 ffc0 001a0c68 ffc0 f6ff7000 ffc1 00d511e8
> > ffc0
> > fe00: 00db9000 ffc0 00d511e8 ffc0 e79dfe30 ffc0 001a1168
> > ffc0
> > fe20: f6ff7000 ffc1 e79dfe50 ffc0 001bcef0
> > ffc0
> > fe40: f6c8b080 ffc1 0015 e79dfe70 ffc0 001bcf94
> > ffc0
> > fe60: ed5d4190 ffc1 90a791c8 007f e79dfe80 ffc0 000ce4
Re: [GIT PULL] USB: chipidea updates for v4.3-rc3
On Fri, Aug 14, 2015 at 10:18:39AM +0800, Peter Chen wrote: > The following changes since commit e6bbe1d05353a29628a4ca72d88bac0bdcec5f38: > > Merge 4.2-rc6 into usb-next (2015-08-10 10:01:33 -0700) > > are available in the git repository at: > > > git://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git/ > tags/usb-ci-v4.3-rc1 > > for you to fetch changes up to 96625eadca1bb8832fb502f0899a543695f1ba35: > > usb: chipidea: add tx/rx burst size configuration interface (2015-08-14 > 10:03:58 +0800) > > > USB: chipidea updates for v4.3-rc1 > > The main changes are adding several system interfaces for > tuning performance, and each vendors can adjust them according > to their design configurations. > > Others are tiny improvements, like more well siTD supports, > USB_DEVICE_A_HNP_SUPPORT supports, etc. > > > Li Jun (1): > usb: chipidea: debug: add runtime pm for register access > > Nicholas Krause (1): > usb:chipidea:Make the function hw_alloc_repmap have a return type of > void > > Peter Chen (17): > usb: chipidea: add ttctrl.ttha control interface > usb: chipidea: imx: enable CI_HDRC_SET_NON_ZERO_TTHA > usb: chipidea: udc: add USB_DEVICE_A_HNP_SUPPORT request support > usb: chipidea: otg_fsm: delete the duplicated reset controller operation > usb: chipidea: introduce ci_platform_configure > Doc: usb: ci-hdrc-usb2: add itc-setting at binding doc > usb: chipidea: introduce ITC tuning interface > usb: chipidea: host: delete the redundancy ci_hdrc struct > usb: chipidea: host: override hcd reset API > usb: chipidea: udc: zero-length packet is only needed for TX > usb: chipidea: define stream mode disable for both roles > usb: chipidea: imx: add stream mode enable for device mode at > imx6sl/imx6sx > Doc: usb: ci-hdrc-usb2: add ahb-burst-config for binding doc > usb: chipidea: add ahb burst configuration interface > usb: chipidea: usbmisc_imx: add non-burst setting for imx6 > Doc: usb: ci-hdrc-usb2: add tx(rx)-burst-config-dword for binding doc > usb: chipidea: add tx/rx burst size configuration interface > > Tomeu Vizoso (1): > usb: chipidea: imx: properly check for usbmisc > > .../devicetree/bindings/usb/ci-hdrc-usb2.txt | 20 > drivers/usb/chipidea/bits.h| 12 +++ > drivers/usb/chipidea/ci.h | 4 + > drivers/usb/chipidea/ci_hdrc_imx.c | 17 +-- > drivers/usb/chipidea/core.c| 115 > ++--- > drivers/usb/chipidea/debug.c | 6 ++ > drivers/usb/chipidea/host.c| 26 +++-- > drivers/usb/chipidea/otg_fsm.c | 1 - > drivers/usb/chipidea/udc.c | 9 +- > drivers/usb/chipidea/usbmisc_imx.c | 12 ++- > include/linux/usb/chipidea.h | 14 ++- > 11 files changed, 204 insertions(+), 32 deletions(-) > > -- > Typo for headline, it is for v4.3-rc1 (v4.3 merge window). -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
