[PATCH 1/1] usb: gadget: Remove use of PWD in Makefiles
Using PWD breaks out-of-tree builds in certain circumstances [1], and other kernel Makefiles use relative paths just fine. [1]: https://bugzilla.kernel.org/show_bug.cgi?id=83251 Signed-off-by: Shea Levy --- drivers/usb/gadget/Makefile | 2 +- drivers/usb/gadget/function/Makefile | 4 ++-- drivers/usb/gadget/legacy/Makefile | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile index a186afe..9add915 100644 --- a/drivers/usb/gadget/Makefile +++ b/drivers/usb/gadget/Makefile @@ -3,7 +3,7 @@ # subdir-ccflags-$(CONFIG_USB_GADGET_DEBUG) := -DDEBUG subdir-ccflags-$(CONFIG_USB_GADGET_VERBOSE)+= -DVERBOSE_DEBUG -ccflags-y += -I$(PWD)/drivers/usb/gadget/udc +ccflags-y += -Idrivers/usb/gadget/udc obj-$(CONFIG_USB_LIBCOMPOSITE) += libcomposite.o libcomposite-y := usbstring.o config.o epautoconf.o diff --git a/drivers/usb/gadget/function/Makefile b/drivers/usb/gadget/function/Makefile index 6d91f21..83ae106 100644 --- a/drivers/usb/gadget/function/Makefile +++ b/drivers/usb/gadget/function/Makefile @@ -2,8 +2,8 @@ # USB peripheral controller drivers # -ccflags-y := -I$(PWD)/drivers/usb/gadget/ -ccflags-y += -I$(PWD)/drivers/usb/gadget/udc/ +ccflags-y := -Idrivers/usb/gadget/ +ccflags-y += -Idrivers/usb/gadget/udc/ # USB Functions usb_f_acm-y:= f_acm.o diff --git a/drivers/usb/gadget/legacy/Makefile b/drivers/usb/gadget/legacy/Makefile index a11aad5..edba2d1 100644 --- a/drivers/usb/gadget/legacy/Makefile +++ b/drivers/usb/gadget/legacy/Makefile @@ -2,9 +2,9 @@ # USB gadget drivers # -ccflags-y := -I$(PWD)/drivers/usb/gadget/ -ccflags-y += -I$(PWD)/drivers/usb/gadget/udc/ -ccflags-y += -I$(PWD)/drivers/usb/gadget/function/ +ccflags-y := -Idrivers/usb/gadget/ +ccflags-y += -Idrivers/usb/gadget/udc/ +ccflags-y += -Idrivers/usb/gadget/function/ g_zero-y := zero.o g_audio-y := audio.o -- 2.1.0 -- 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 v5 4/4] phy: exynos5-usbdrd: Calibrate LOS levels for exynos5420/5800
Hi Jingoo,
On Wed, Aug 27, 2014 at 7:28 AM, Jingoo Han wrote:
> On Thursday, August 21, 2014 11:55 PM, Vivek Gautam wrote:
>>
>> Adding phy calibrate callback, which facilitates setting certain
>> PHY settings post initialization of the PHY controller.
>> Exynos5420 and Exynos5800 have 28nm USB 3.0 DRD PHY for which
>> the Loss-of-Signal (LOS) Detector Threshold Level as well as
>> Tx-Vboost-Level should be controlled for Super-Speed operations.
>>
>> Additionally set proper time to wait for RxDetect measurement,
>> for desired PHY reference clock, so as to solve issue with enumeration
>> of few USB 3.0 devices, like Samsung SUM-TSB16S 3.0 USB drive
>> on the controller.
>> We are using CR_port for this purpose to send required data
>> to override the LOS values.
>>
>> On testing with USB 3.0 devices on USB 3.0 port present on
>> SMDK5420, and peach-pit boards should see following message:
>> usb 2-1: new SuperSpeed USB device number 2 using xhci-hcd
>>
>> and without this patch, should see below shown message:
>> usb 1-1: new high-speed USB device number 2 using xhci-hcd
>>
>> Signed-off-by: Vivek Gautam
>> ---
>> drivers/phy/phy-exynos5-usbdrd.c | 169
>> ++
>> 1 file changed, 169 insertions(+)
>>
>> diff --git a/drivers/phy/phy-exynos5-usbdrd.c
>> b/drivers/phy/phy-exynos5-usbdrd.c
>> index 47f47fe..fa13784 100644
>> --- a/drivers/phy/phy-exynos5-usbdrd.c
>> +++ b/drivers/phy/phy-exynos5-usbdrd.c
>> @@ -89,8 +89,20 @@
>> #define PHYCLKRST_COMMONONN BIT(0)
>>
>> #define EXYNOS5_DRD_PHYREG0 0x14
>> +
>> +#define EXYNOS5_DRD_PHYREG0_SSC_REF_CLK_SEL BIT(21)
>> +#define EXYNOS5_DRD_PHYREG0_SSC_RANGEBIT(20)
>> +#define EXYNOS5_DRD_PHYREG0_CR_WRITE BIT(19)
>> +#define EXYNOS5_DRD_PHYREG0_CR_READ BIT(18)
>> +#define EXYNOS5_DRD_PHYREG0_CR_DATA_IN(_x) ((_x) << 2)
>> +#define EXYNOS5_DRD_PHYREG0_CR_CAP_DATA BIT(1)
>> +#define EXYNOS5_DRD_PHYREG0_CR_CAP_ADDR BIT(0)
>> +
>> #define EXYNOS5_DRD_PHYREG1 0x18
>>
>> +#define EXYNOS5_DRD_PHYREG1_CR_DATA_OUT(_x) ((_x) << 1)
>> +#define EXYNOS5_DRD_PHYREG1_CR_ACK BIT(0)
>> +
>> #define EXYNOS5_DRD_PHYPARAM00x1c
>>
>> #define PHYPARAM0_REF_USE_PADBIT(31)
>> @@ -118,6 +130,26 @@
>> #define EXYNOS5_DRD_PHYRESUME0x34
>> #define EXYNOS5_DRD_LINKPORT 0x44
>>
>> +/* USB 3.0 DRD PHY SS Function Control Reg; accessed by CR_PORT */
>> +#define EXYNOS5_DRD_PHYSS_LOSLEVEL_OVRD_IN (0x15)
>> +
>
> Please remove unnecessary line.
Sure will remove extra lines.
[snip]
>> +static void crport_ctrl_write(struct exynos5_usbdrd_phy *phy_drd,
>> + u32 addr, u32 data)
>> +{
>> + /* Write Address */
>> + crport_handshake(phy_drd, EXYNOS5_DRD_PHYREG0_CR_DATA_IN(addr),
>> + EXYNOS5_DRD_PHYREG0_CR_CAP_ADDR);
>
> According to the guidance from H/W team, before calling crport_handshake(),
> write access for EXYNOS5_DRD_PHYREG0 register is necessary.
>
> Please, add the write access as follows.
>
> + /* Write Address */
> + writel(EXYNOS5_DRD_PHYREG0_CR_DATA_IN(addr),
> + phy_drd->reg_phy + EXYNOS5_DRD_PHYREG0);
> + crport_handshake(phy_drd, EXYNOS5_DRD_PHYREG0_CR_DATA_IN(addr),
> +EXYNOS5_DRD_PHYREG0_CR_CAP_ADDR);
Sure will add this, thanks for getting the information from H/W team
and suggesting.
>
> Best regards,
> Jingoo Han
>
>> +
>> + /* Write Data */
>> + crport_handshake(phy_drd, EXYNOS5_DRD_PHYREG0_CR_DATA_IN(data),
>> + EXYNOS5_DRD_PHYREG0_CR_CAP_DATA);
>> + crport_handshake(phy_drd, EXYNOS5_DRD_PHYREG0_CR_DATA_IN(data),
>> + EXYNOS5_DRD_PHYREG0_CR_WRITE);
>> +}
>> +
>> +/*
>> + * Override PHY paramaeters using CR_PORT register to calibrate settings
>> + * to meet meet SuperSpeed requirements, on Exynos5420 and Exynos5800
>> systems,
>> + * which have 28nm USB 3.0 DRD PHY.
>> + */
>> +static void exynos5420_usbdrd_phy_calibrate(struct exynos5_usbdrd_phy
>> *phy_drd)
>> +{
>> + u32 temp;
>> +
>> + /*
>> + * Change los_bias to (0x5) for 28nm PHY from a
>> + * default value (0x0); los_level is set as default
>> + * (0x9) as also reflected in los_level[30:26] bits
>> + * of PHYPARAM0 register.
>> + */
>> + temp = LOSLEVEL_OVRD_IN_LOS_BIAS_5420 |
>> + LOSLEVEL_OVRD_IN_EN |
>> + LOSLEVEL_OVRD_IN_LOS_LEVEL_DEFAULT;
>> + crport_ctrl_write(phy_drd,
>> + EXYNOS5_DRD_PHYSS_LOSLEVEL_OVRD_IN,
>> + temp);
>> +
>> + /*
>> + * Set tx_vboost_lvl to (0x5) for 28nm PHY Tuning,
>> + * to raise Tx signal level from its default value of (0x4)
>> + */
>> + temp = TX_VBOOSTLEVEL_OVRD_IN_VBOOST_5420;
>> + crport_ctrl_write(phy_dr
[xhci] kernel BUG at arch/x86/mm/physaddr.c:26!
c
[4.360029] RSP
[4.392028] ---[ end trace 68ef4c4340f54dcf ]---
[4.392651] Kernel panic - not syncing: Fatal exception
git bisect start 87e45e9aee6e16808da24d42620c30e62ef78f72
52addcf9d6669fa439387610bc65c92fa0980cef --
git bisect good 21e7954c90927f091ef81611f7f186d8f2f068a7 # 02:54 20+
0 Merge 'ipvs/master' into devel-hourly-2014082801
git bisect good db76ed1b63ae17ebfbbcac90a25e9b15d7c81593 # 03:02 20+
0 Merge 'robclark/msm-fixes-3.17' into devel-hourly-2014082801
git bisect bad d742d6a6a9ba64655ff7c57b5995f5e88e417c4d # 03:06 0-
20 Merge 'xen-tip/devel/for-linus-3.18' into devel-hourly-2014082801
git bisect good b590845ddf136fab351e0fbed14e3d1b1e655d56 # 03:13 20+
1 Merge 'regulator/for-next' into devel-hourly-2014082801
git bisect bad a768419190a3acf1db12e2e27d3589035d2ca713 # 03:24 0-
20 Merge 'djbw-usb/td-fragments-v1' into devel-hourly-2014082801
git bisect good e705f1f3500d34055a829cec1178012108c2b5aa # 03:28 20+
0 Merge 'stericsson/tcm' into devel-hourly-2014082801
git bisect good a75ef911cf100b8cf7d25baf6dac8052328a96e7 # 03:39 20+
0 xhci: clarify "ring valid" checks
git bisect good 652b7ee36207f186f3d701675483df43b4845c5c # 03:45 20+
0 xhci: kill ->num_trbs_free_temp in struct xhci_ring
git bisect good 1c11eb8545a3321e7ca27fc7ba8c56b6e6df2b57 # 03:51 20+
0 xhci: add xhci_ring_reap_td() helper
git bisect bad e65e21a542cab81d794db4e5fe919c4e1d624ea7 # 03:56 0-
20 xhci: unit test ring enqueue/dequeue routines
git bisect good fb6fa3e625e1e453aea9eeb97d58bee30e1c0781 # 04:07 20+
0 xhci: v1.0 scatterlist enqueue support (td-fragment rework)
# first bad commit: [e65e21a542cab81d794db4e5fe919c4e1d624ea7] xhci: unit test
ring enqueue/dequeue routines
git bisect good fb6fa3e625e1e453aea9eeb97d58bee30e1c0781 # 04:10 60+
0 xhci: v1.0 scatterlist enqueue support (td-fragment rework)
git bisect bad 87e45e9aee6e16808da24d42620c30e62ef78f72 # 04:10 0-
11 0day head guard for 'devel-hourly-2014082801'
git bisect good ff0c57ac70434bc936cb0110eaf033a0a1a62e52 # 04:19 60+
5 Merge branch 'for-linus' of
git://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid
git bisect good d05446ae2128064a4bb8f74c84f6901ffb5c94bc # 04:22 60+
0 Add linux-next specific files for 20140827
This script may reproduce the error.
#!/bin/bash
kernel=$1
initrd=yocto-minimal-x86_64.cgz
wget --no-clobber
https://github.com/fengguang/reproduce-kernel-bug/raw/master/initrd/$initrd
kvm=(
qemu-system-x86_64
-cpu kvm64
-enable-kvm
-kernel $kernel
-initrd $initrd
-m 320
-smp 1
-net nic,vlan=1,model=e1000
-net user,vlan=1
-boot order=nc
-no-reboot
-watchdog i6300esb
-rtc base=localtime
-serial stdio
-display none
-monitor null
)
append=(
hung_task_panic=1
earlyprintk=ttyS0,115200
debug
apic=debug
sysrq_always_enabled
rcupdate.rcu_cpu_stall_timeout=100
panic=-1
softlockup_panic=1
nmi_watchdog=panic
oops=panic
load_ramdisk=2
prompt_ramdisk=0
console=ttyS0,115200
console=tty0
vga=normal
root=/dev/ram0
rw
drbd.minor_count=8
)
"${kvm[@]}" --append "${append[*]}"
Thanks,
Fengguang
early console in setup code
[0.00] Initializing cgroup subsys cpuset
[0.00] Initializing cgroup subsys cpu
[0.00] Linux version 3.16.0-rc5-00225-ge65e21a (kbuild@lkp-hsx01) (gcc
version 4.8.2 (Debian 4.8.2-18) ) #6 Thu Aug 28 11:54:57 CST 2014
[0.00] Command line: hung_task_panic=1 earlyprintk=ttyS0,115200 debug
apic=debug sysrq_always_enabled rcupdate.rcu_cpu_stall_timeout=100 panic=-1
softlockup_panic=1 nmi_watchdog=panic oops=panic load_ramdisk=2
prompt_ramdisk=0 console=ttyS0,115200 console=tty0 vga=normal root=/dev/ram0
rw
link=/kbuild-tests/run-queue/kvm/x86_64-randconfig-hsxa2-08281025/linux-devel:devel-hourly-2014082801:e65e21a542cab81d794db4e5fe919c4e1d624ea7:bisect-linux-5/.vmlinuz-e65e21a542cab81d794db4e5fe919c4e1d624ea7-20140828115559-4-vp
branch=linux-devel/devel-hourly-2014082801
BOOT_IMAGE=/kernel/x86_64-randconfig-hsxa2-08281025/e65e21a542cab81d794db4e5fe919c4e1d624ea7/vmlinuz-3.16.0-rc5-00225-ge65e21a
drbd.minor_count=8
[0.00] KERNEL supported cpus:
[0.00] AMD AuthenticAMD
[0.00] Centaur CentaurHauls
[0.00] CPU: vendor_id 'GenuineIntel' unknown, using generic init.
[0.00] CPU: Your system may be unstable.
[0.00] e
Re: [PATCH v5 3/4] usb: hcd: Caibrate PHY post hcd reset
Hi, On Fri, Aug 22, 2014 at 4:19 PM, Andreas Färber wrote: > Hi, > > s/Caibrate/Calibrate/ Sure, will rectify the title, thanks for pointing it out. -- Best Regards Vivek Gautam Samsung R&D Institute, Bangalore India -- 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 net-next v2] r8152: reduce the number of Tx
Because the Tx has the features of stopping queue and aggregation,
We don't need many tx buffers. Change the tx number from 10 to 4
to reduce the usage of the memory. This could save 16K * 6 bytes
memory.
Signed-off-by: Hayes Wang
---
drivers/net/usb/r8152.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 33dcc97..cc64dc0 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -424,7 +424,7 @@ enum rtl_register_content {
FULL_DUP= 0x01,
};
-#define RTL8152_MAX_TX 10
+#define RTL8152_MAX_TX 4
#define RTL8152_MAX_RX 10
#define INTBUFSIZE 2
#define CRC_SIZE 4
--
1.9.3
--
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/5] usb: phy: twl4030-usb: Remove unused irq_enabled
Hi,
On Wed, Aug 27, 2014 at 04:28:07PM -0700, Tony Lindgren wrote:
> It's not being used any longer.
>
> Signed-off-by: Tony Lindgren
> ---
> drivers/phy/phy-twl4030-usb.c | 2 --
> drivers/usb/phy/phy-twl6030-usb.c | 2 --
> 2 files changed, 4 deletions(-)
>
> diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
> index 9cd33a4..bc28ecc 100644
> --- a/drivers/phy/phy-twl4030-usb.c
> +++ b/drivers/phy/phy-twl4030-usb.c
> @@ -164,7 +164,6 @@ struct twl4030_usb {
> enum omap_musb_vbus_id_status linkstat;
> boolvbus_supplied;
> u8 asleep;
> - boolirq_enabled;
>
> struct delayed_work id_workaround_work;
> };
> @@ -755,7 +754,6 @@ static int twl4030_usb_probe(struct platform_device *pdev)
>* set_host() and/or set_peripheral() ... OTG_capable boards
>* need both handles, otherwise just one suffices.
>*/
> - twl->irq_enabled = true;
> status = devm_request_threaded_irq(twl->dev, twl->irq, NULL,
> twl4030_usb_irq, IRQF_TRIGGER_FALLING |
> IRQF_TRIGGER_RISING | IRQF_ONESHOT, "twl4030_usb", twl);
can you split this into two patches ? drivers/phy will be taken by
Kishon and drivers/usb/phy by me. another possibility is that I get an
Acked-by from Kishon and I can take $subject as is.
--
balbi
signature.asc
Description: Digital signature
[PATCH net-next] r8152: reduce the number of Tx
Because the Tx has the features of stopping queue and aggregation,
We don't need many tx buffers. Change the tx number from 10 to 4
to reduce the usage of the memory. This could save 16K * 10 bytes
memory.
Signed-off-by: Hayes Wang
---
drivers/net/usb/r8152.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 33dcc97..cc64dc0 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -424,7 +424,7 @@ enum rtl_register_content {
FULL_DUP= 0x01,
};
-#define RTL8152_MAX_TX 10
+#define RTL8152_MAX_TX 4
#define RTL8152_MAX_RX 10
#define INTBUFSIZE 2
#define CRC_SIZE 4
--
1.9.3
--
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: net: Add ops->ndo_xmit_flush()
From: Dan Carpenter
Date: Tue, 26 Aug 2014 15:21:37 +0300
> Hello David S. Miller,
>
> The patch 4798248e4e02: "net: Add ops->ndo_xmit_flush()" from Aug 22,
> 2014, leads to the following static checker warning:
>
> drivers/usb/gadget/function/f_ncm.c:1104 ncm_tx_tasklet()
> error: NULL dereference inside function.
>
> drivers/usb/gadget/function/f_ncm.c
> 1094 static void ncm_tx_tasklet(unsigned long data)
> 1095 {
> 1096 struct f_ncm*ncm = (void *)data;
> 1097
> 1098 if (ncm->timer_stopping)
> 1099 return;
> 1100
> 1101 /* Only send if data is available. */
> 1102 if (ncm->skb_tx_data) {
> 1103 ncm->timer_force_tx = true;
> 1104 netdev_start_xmit(NULL, ncm->netdev);
>
> You can't pass a NULL skb to netdev_start_xmit() or it leads to a NULL
> dereference when we set "skb->xmit_more = 0;" in __netdev_start_xmit().
>
> 1105 ncm->timer_force_tx = false;
> 1106 }
> 1107 }
Sigh, this code.
The u_ether stuff has an ndo_start_xmit function which is invoked
sometimes with a NULL skb argument, just to trigger the calls to the
dev->wrap() methods which occurs in this eth_start_xmit() routine.
This is really outside of the acceptable call pattern for this method,
and the gadget folks really have to redesign this so that
ndo_start_xmit() behaves and is invoked with proper, sane, arguments.
For now I'll put back the direct invocation of ops->ndo_start_xmit()
but with a huge comment.
--
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: net2280: Remove pci_class from PCI_TABLE
On Wed, Aug 27, 2014 at 11:09:08PM +0200, Bjørn Mork wrote: > Greg Kroah-Hartman writes: > > On Wed, Aug 27, 2014 at 09:39:43PM +0200, Ricardo Ribalda Delgado wrote: > >> > >> return sprintf(buf, "pci:v%08Xd%08Xsv%08Xsd%08Xbc%02Xsc%02Xi%02x\n", > > That final 'x' does look like a typo, doesn't it? We are otherwise > consistently using upper-case hex digits for field values and lower case > letter for field names. But it looks like it has been like that since > the beginning, so it might be difficult to fix... Yes, it should be fixed, sorry, my later email said that, no one has hit it in 9+ years, pretty impressive. > > No, the root cause of the problem is a userspace tool looking at a hex > > value as a string and not a number. It doesn't matter if we print it in > > upper or lower case, it's a digit, not a string. Do the numeric > > compare, not a string compare. > > Now I don't really know much about the history here, but the format of > module aliases, using wildcards, seem to suggest a string match to me. > Do you really mean that these strings should be parsed into field names > + values before matching? If that was the intention then surely we > would have exported the fields one-by-one as separate sysfs attributes? > Ref the "one value per file" policy. No, this is a bit field, so you can't do a string compare. kmod should know how do handle this, it does so for other types of "class" fields in module device ids. And no, we didn't export these as a set of files, it's one unique value that you can use to match up a device to a driver. > >> Not many drivers define the pci interface and there is no other driver > >> that has the same vendor and product id. Therefore I see no hurt in > >> adding both patches, one to make the driver broader, and another to > >> fix pci-sysfs. > >> > >> Also, the change on pci-sysfs might affect more stuff and therefore > >> take longer to be applied. > > > > As we have been printing the value to userspace in this way for well > > over a decade now, and nothing has changed, I say it's a userspace bug > > that you should fix instead. Don't work around broken user programs in > > the kernel by changing something that has been stable for 10+ years. > > > > Ok, sorry, not 10+ years, the commit was written May of 2005, so 9 > > years. > > well, just looking at a few common PCI devices on my PCs I wonder if the > reason this hasn't been a problem is because there are _very_ few PCI > programming interfaces using anything by 0-9 digits. One? Looking at the > modules built by Debian I can only find one udc module matching on any > hex value: > > bjorn@nemi:~$ grep pci: /lib/modules/3.16-trunk-amd64/modules.alias|egrep > "i[A-F]" > alias pci:v10DBd8808sv*sd*bc0Csc03iFE* pch_udc > alias pci:v10DBd801Dsv*sd*bc0Csc03iFE* pch_udc > alias pci:v8086d8808sv*sd*bc0Csc03iFE* pch_udc > > This makes me wonder if this is exclusively a problem for PCI UDCs, > which tend to be pretty rare devices? Yes, this is a rare thing, as the age of this line proves :) 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 v7] usb:serial:pl2303: add GPIOs interface on PL2303
On Mon, Aug 18, 2014 at 12:07:20PM +0200, Johan Hovold wrote: > On Sun, Aug 17, 2014 at 10:05:36AM +0800, Wang YanQing wrote: > > Hi Johan Hovold. > > > > Another two questions. > > > > On Tue, Aug 12, 2014 at 04:46:25PM +0200, Johan Hovold wrote: > > > > > > > + int (*gpio_startup)(struct usb_serial *serial); > > > > + void (*gpio_release)(struct usb_serial *serial); > > > > > > This isn't the right place for this abstraction. Most of the setup code > > > would be common for any device type with GPIOs. > > > > I assume you mean any pl2303 variant, not any device type, because > > no device in drivers/gpio has common setup code except many of them > > use struct gpio_chip. > > Yes, pl2303 type/variant. Specifically, much of the setup code will be > identical even if say the number of gpio differ (2 or 4) depending on > type. Yes, indeed unless I know how to make kernel could distinguish those two types, this patch willn't be fit for mergence. And I don't have free time recent, I need more time to prepare this patch, or could you figure out how to distinguish them? 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: Resend Re: [PATCH v6] usb:serial:pl2303: add GPIOs interface on PL2303
On Tue, Aug 12, 2014 at 04:02:59PM +0200, Johan Hovold wrote: > On Sat, Aug 09, 2014 at 02:46:56AM +0800, Wang YanQing wrote: > > On Fri, Aug 08, 2014 at 09:54:42AM +0200, Johan Hovold wrote: > > > On Fri, Aug 08, 2014 at 03:10:34AM +0800, Wang YanQing wrote: > > > > On Tue, Aug 05, 2014 at 03:54:08PM +0200, Johan Hovold wrote: > > > > > > > I noticed that setting direction to output and setting the gpio > > > > > > > high has > > > > > > > no effect on the read-back value (i.e. I still read back 0) for my > > > > > > > pl2303hx (note that my device has no easily accessible gpios so I > > > > > > > haven't verified the actual state of the output pin). > > > > > > > > > > > > > > What happens on your system? Is the read-back value still 0, even > > > > > > > when > > > > > > > the GPIO output is actually high? Should we return the cached > > > > > > > value in > > > > > > > this case? > > > > > > > > > > > > If i set direction to output, then I could control gpio high and low > > > > > > by set 1 or 0, and the read-back value is 1 or 0 according to high > > > > > > and > > > > > > low(I test high and low by oscillscope) > > > > > > > > > > > > I test it with my pl2303hx with only two gpios. > > > > > > > > > > > > Could you use usbmon to see whether the traffic is right according > > > > > > to comment in struct pl2303_gpio? > > > > > > > > > > The traffic appears correct judging from the debug output (which I > > > > > trust). Output-enable is reflected in register 0x81, but the value > > > > > isn't. > > > > > > > > > > What is the lsusb -v output for your device? > > > > > > > > Bus 001 Device 005: ID 067b:2303 Prolific Technology, Inc. PL2303 > > > > Serial Port. > > > > > > You forgot the verbose flag (-v). > > Sorry, below is output with -v: > > Bus 002 Device 004: ID 067b:2303 Prolific Technology, Inc. PL2303 Serial > > Port > > Device Descriptor: > > bLength18 > > bDescriptorType 1 > > bcdUSB 1.10 > > bDeviceClass0 (Defined at Interface level) > > bDeviceSubClass 0 > > bDeviceProtocol 0 > > bMaxPacketSize064 > > idVendor 0x067b Prolific Technology, Inc. > > idProduct 0x2303 PL2303 Serial Port > > bcdDevice3.00 > > You seem to have an HX device, whereas mine is an HXD (rev D) with > bcdDevice 4.00. That could account for the different behaviour of the > GPIO state/value register. > > How did you figure out which registers to use? Were you sniffing the > traffic of some driver for some other OS? And does your device only have > two GPIOs and not four like the HX rev D? After I found I need to use GPIOs in pl2303, I found below patch in Internet firstly: http://comments.gmane.org/gmane.linux.usb.general/65066 Then I verified the protocol by sniffing the traffic of some driver for some other OS running in virtualbox, and host OS is linux:) Prolific has pl2303 gpio test program (.exe) for windows, maybe you could find it from Internet. It support HXA and HXD, I use it to test two gpios in my pl2303HXA, and analyze output of usbmon. Yes, my device only have two GPIOs. > > > > > > It is strange your device doesn't work, I verify the control method by > > > > analyze usbmon output from linux host which has VirtualBox running > > > > gpio test program, but I don't have right to distribute the gpio test > > > > program I think, so I can't help you to figure out why it doesn't work > > > > for your device. > > > > > > What do you use the gpio test program for? I thought you verified the > > > gpios with a scope? > > > > Yes, I verified gpios with a scope. > > > > " > > You must allocate the buffer dynamically as some platforms cannot do > > DMA to the stack. > > " > > Thanks very much for point out it, could you clarify it? > > I want to know the reason. > > The memory where the stack resides might not be available for DMA, and > even if it is, there could still be problems with cache coherency. It is still vague: stack memory maybe resident higher place than normal memory, but I don't think kmalloc could be immune from this problem, unless we use GFP_DMA? -- 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 2/5] usb: phy: twl4030-usb: Simplify phy init to use runtime PM
We can now let the interrupt and delayed work do all that's
needed with runtime PM.
Signed-off-by: Tony Lindgren
---
drivers/phy/phy-twl4030-usb.c | 20 +++-
1 file changed, 3 insertions(+), 17 deletions(-)
diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
index bc28ecc..a292db0 100644
--- a/drivers/phy/phy-twl4030-usb.c
+++ b/drivers/phy/phy-twl4030-usb.c
@@ -471,16 +471,8 @@ static int twl4030_phy_power_on(struct phy *phy)
twl4030_usb_set_mode(twl, twl->usb_mode);
if (twl->usb_mode == T2_USB_MODE_ULPI)
twl4030_i2c_access(twl, 0);
+ schedule_delayed_work(&twl->id_workaround_work, 0);
- /*
-* XXX When VBUS gets driven after musb goes to A mode,
-* ID_PRES related interrupts no longer arrive, why?
-* Register itself is updated fine though, so we must poll.
-*/
- if (twl->linkstat == OMAP_MUSB_ID_GROUND) {
- cancel_delayed_work(&twl->id_workaround_work);
- schedule_delayed_work(&twl->id_workaround_work, HZ);
- }
return 0;
}
@@ -612,16 +604,9 @@ static void twl4030_id_workaround_work(struct work_struct
*work)
static int twl4030_phy_init(struct phy *phy)
{
struct twl4030_usb *twl = phy_get_drvdata(phy);
- enum omap_musb_vbus_id_status status;
pm_runtime_get_sync(twl->dev);
- status = twl4030_usb_linkstat(twl);
- twl->linkstat = status;
-
- if (status == OMAP_MUSB_ID_GROUND || status == OMAP_MUSB_VBUS_VALID)
- omap_musb_mailbox(twl->linkstat);
-
- sysfs_notify(&twl->dev->kobj, NULL, "vbus");
+ schedule_delayed_work(&twl->id_workaround_work, 0);
pm_runtime_mark_last_busy(twl->dev);
pm_runtime_put_autosuspend(twl->dev);
@@ -698,6 +683,7 @@ static int twl4030_usb_probe(struct platform_device *pdev)
twl->dev= &pdev->dev;
twl->irq= platform_get_irq(pdev, 0);
twl->vbus_supplied = false;
+ twl->linkstat = -EINVAL;
twl->asleep = 1;
twl->linkstat = OMAP_MUSB_UNKNOWN;
--
1.8.1.1
--
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 0/5] Clean-up for twl4030-usb
Hi, Here are few more patches for v3.18 to clean up twl4030-usb. These are based on the two regression fixes I sent earlier: [PATCH] usb: phy: twl4030-usb: Fix regressions to runtime PM on omaps [PATCH] usb: phy: twl4030-usb: Fix lost interrupts after ID pin goes down For testing with v3.17-rc2, you probably also want to revert commit a9232076374334ca2bc2a448dfde96d38a54349a until that hits the mainline tree. And you may also want to apply the following patch if testing with PM: [PATCH] mfd: twl4030-power: Fix PM idle pin configuration to not conflict with regulators I've also pushed these patches with the optional patches into a temporary testing branch at [1]. Regards, Tony [1] git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git omap-for-v3.17/testing-pending-usb Tony Lindgren (5): usb: phy: twl4030-usb: Remove unused irq_enabled usb: phy: twl4030-usb: Simplify phy init to use runtime PM usb: phy: twl4030-usb: Move code from twl4030_phy_power to the runtime PM calls usb: phy: twl4030-usb: Remove asleep and rely on runtime PM usb: phy: twl4030-usb: Use mutex instead of spinlock for protecting the data drivers/phy/phy-twl4030-usb.c | 124 ++ drivers/usb/phy/phy-twl6030-usb.c | 2 - 2 files changed, 46 insertions(+), 80 deletions(-) -- 1.8.1.1 -- 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 1/5] usb: phy: twl4030-usb: Remove unused irq_enabled
It's not being used any longer.
Signed-off-by: Tony Lindgren
---
drivers/phy/phy-twl4030-usb.c | 2 --
drivers/usb/phy/phy-twl6030-usb.c | 2 --
2 files changed, 4 deletions(-)
diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
index 9cd33a4..bc28ecc 100644
--- a/drivers/phy/phy-twl4030-usb.c
+++ b/drivers/phy/phy-twl4030-usb.c
@@ -164,7 +164,6 @@ struct twl4030_usb {
enum omap_musb_vbus_id_status linkstat;
boolvbus_supplied;
u8 asleep;
- boolirq_enabled;
struct delayed_work id_workaround_work;
};
@@ -755,7 +754,6 @@ static int twl4030_usb_probe(struct platform_device *pdev)
* set_host() and/or set_peripheral() ... OTG_capable boards
* need both handles, otherwise just one suffices.
*/
- twl->irq_enabled = true;
status = devm_request_threaded_irq(twl->dev, twl->irq, NULL,
twl4030_usb_irq, IRQF_TRIGGER_FALLING |
IRQF_TRIGGER_RISING | IRQF_ONESHOT, "twl4030_usb", twl);
diff --git a/drivers/usb/phy/phy-twl6030-usb.c
b/drivers/usb/phy/phy-twl6030-usb.c
index 04778cf..44ea082 100644
--- a/drivers/usb/phy/phy-twl6030-usb.c
+++ b/drivers/usb/phy/phy-twl6030-usb.c
@@ -104,7 +104,6 @@ struct twl6030_usb {
int irq2;
enum omap_musb_vbus_id_status linkstat;
u8 asleep;
- boolirq_enabled;
boolvbus_enable;
const char *regulator;
};
@@ -373,7 +372,6 @@ static int twl6030_usb_probe(struct platform_device *pdev)
INIT_WORK(&twl->set_vbus_work, otg_set_vbus_work);
- twl->irq_enabled = true;
status = request_threaded_irq(twl->irq1, NULL, twl6030_usbotg_irq,
IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING |
IRQF_ONESHOT,
"twl6030_usb", twl);
--
1.8.1.1
--
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 4/5] usb: phy: twl4030-usb: Remove asleep and rely on runtime PM
There's no longer need for tracking the phy state in the driver
with asleep, we can now rely on runtime PM.
Signed-off-by: Tony Lindgren
---
drivers/phy/phy-twl4030-usb.c | 14 +-
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
index 519cc90..24ff3c6 100644
--- a/drivers/phy/phy-twl4030-usb.c
+++ b/drivers/phy/phy-twl4030-usb.c
@@ -163,7 +163,6 @@ struct twl4030_usb {
int irq;
enum omap_musb_vbus_id_status linkstat;
boolvbus_supplied;
- u8 asleep;
struct delayed_work id_workaround_work;
};
@@ -388,14 +387,13 @@ static int twl4030_usb_runtime_suspend(struct device *dev)
struct twl4030_usb *twl = dev_get_drvdata(dev);
dev_dbg(twl->dev, "%s\n", __func__);
- if (twl->asleep)
+ if (pm_runtime_suspended(dev))
return 0;
__twl4030_phy_power(twl, 0);
regulator_disable(twl->usb1v5);
regulator_disable(twl->usb1v8);
regulator_disable(twl->usb3v1);
- twl->asleep = 1;
return 0;
}
@@ -406,7 +404,7 @@ static int twl4030_usb_runtime_resume(struct device *dev)
int res;
dev_dbg(twl->dev, "%s\n", __func__);
- if (!twl->asleep)
+ if (pm_runtime_active(dev))
return 0;
res = regulator_enable(twl->usb3v1);
@@ -435,7 +433,6 @@ static int twl4030_usb_runtime_resume(struct device *dev)
twl4030_usb_read(twl, PHY_CLK_CTRL) |
(PHY_CLK_CTRL_CLOCKGATING_EN |
PHY_CLK_CTRL_CLK32K_EN));
- twl->asleep = 0;
return 0;
}
@@ -560,10 +557,10 @@ static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
*/
if ((status == OMAP_MUSB_VBUS_VALID) ||
(status == OMAP_MUSB_ID_GROUND)) {
- if (twl->asleep)
+ if (pm_runtime_suspended(twl->dev))
pm_runtime_get_sync(twl->dev);
} else {
- if (!twl->asleep) {
+ if (pm_runtime_active(twl->dev)) {
pm_runtime_mark_last_busy(twl->dev);
pm_runtime_put_autosuspend(twl->dev);
}
@@ -572,7 +569,7 @@ static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
}
/* don't schedule during sleep - irq works right then */
- if (status == OMAP_MUSB_ID_GROUND && !twl->asleep) {
+ if (status == OMAP_MUSB_ID_GROUND && pm_runtime_active(twl->dev)) {
cancel_delayed_work(&twl->id_workaround_work);
schedule_delayed_work(&twl->id_workaround_work, HZ);
}
@@ -674,7 +671,6 @@ static int twl4030_usb_probe(struct platform_device *pdev)
twl->irq= platform_get_irq(pdev, 0);
twl->vbus_supplied = false;
twl->linkstat = -EINVAL;
- twl->asleep = 1;
twl->linkstat = OMAP_MUSB_UNKNOWN;
twl->phy.dev= twl->dev;
--
1.8.1.1
--
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 3/5] usb: phy: twl4030-usb: Move code from twl4030_phy_power to the runtime PM calls
We don't need twl4030_phy_power() any longer now that we have
the runtime PM calls. Let's get rid of it as it's confusing.
No functional changes, just move the code and use res instead
of ret as we are not returning that value.
Signed-off-by: Tony Lindgren
---
drivers/phy/phy-twl4030-usb.c | 72 +++
1 file changed, 31 insertions(+), 41 deletions(-)
diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
index a292db0..519cc90 100644
--- a/drivers/phy/phy-twl4030-usb.c
+++ b/drivers/phy/phy-twl4030-usb.c
@@ -383,45 +383,6 @@ static void __twl4030_phy_power(struct twl4030_usb *twl,
int on)
WARN_ON(twl4030_usb_write_verify(twl, PHY_PWR_CTRL, pwr) < 0);
}
-static void twl4030_phy_power(struct twl4030_usb *twl, int on)
-{
- int ret;
-
- if (on) {
- ret = regulator_enable(twl->usb3v1);
- if (ret)
- dev_err(twl->dev, "Failed to enable usb3v1\n");
-
- ret = regulator_enable(twl->usb1v8);
- if (ret)
- dev_err(twl->dev, "Failed to enable usb1v8\n");
-
- /*
-* Disabling usb3v1 regulator (= writing 0 to VUSB3V1_DEV_GRP
-* in twl4030) resets the VUSB_DEDICATED2 register. This reset
-* enables VUSB3V1_SLEEP bit that remaps usb3v1 ACTIVE state to
-* SLEEP. We work around this by clearing the bit after usv3v1
-* is re-activated. This ensures that VUSB3V1 is really active.
-*/
- twl_i2c_write_u8(TWL_MODULE_PM_RECEIVER, 0, VUSB_DEDICATED2);
-
- ret = regulator_enable(twl->usb1v5);
- if (ret)
- dev_err(twl->dev, "Failed to enable usb1v5\n");
-
- __twl4030_phy_power(twl, 1);
- twl4030_usb_write(twl, PHY_CLK_CTRL,
- twl4030_usb_read(twl, PHY_CLK_CTRL) |
- (PHY_CLK_CTRL_CLOCKGATING_EN |
- PHY_CLK_CTRL_CLK32K_EN));
- } else {
- __twl4030_phy_power(twl, 0);
- regulator_disable(twl->usb1v5);
- regulator_disable(twl->usb1v8);
- regulator_disable(twl->usb3v1);
- }
-}
-
static int twl4030_usb_runtime_suspend(struct device *dev)
{
struct twl4030_usb *twl = dev_get_drvdata(dev);
@@ -430,7 +391,10 @@ static int twl4030_usb_runtime_suspend(struct device *dev)
if (twl->asleep)
return 0;
- twl4030_phy_power(twl, 0);
+ __twl4030_phy_power(twl, 0);
+ regulator_disable(twl->usb1v5);
+ regulator_disable(twl->usb1v8);
+ regulator_disable(twl->usb3v1);
twl->asleep = 1;
return 0;
@@ -439,12 +403,38 @@ static int twl4030_usb_runtime_suspend(struct device *dev)
static int twl4030_usb_runtime_resume(struct device *dev)
{
struct twl4030_usb *twl = dev_get_drvdata(dev);
+ int res;
dev_dbg(twl->dev, "%s\n", __func__);
if (!twl->asleep)
return 0;
- twl4030_phy_power(twl, 1);
+ res = regulator_enable(twl->usb3v1);
+ if (res)
+ dev_err(twl->dev, "Failed to enable usb3v1\n");
+
+ res = regulator_enable(twl->usb1v8);
+ if (res)
+ dev_err(twl->dev, "Failed to enable usb1v8\n");
+
+ /*
+* Disabling usb3v1 regulator (= writing 0 to VUSB3V1_DEV_GRP
+* in twl4030) resets the VUSB_DEDICATED2 register. This reset
+* enables VUSB3V1_SLEEP bit that remaps usb3v1 ACTIVE state to
+* SLEEP. We work around this by clearing the bit after usv3v1
+* is re-activated. This ensures that VUSB3V1 is really active.
+*/
+ twl_i2c_write_u8(TWL_MODULE_PM_RECEIVER, 0, VUSB_DEDICATED2);
+
+ res = regulator_enable(twl->usb1v5);
+ if (res)
+ dev_err(twl->dev, "Failed to enable usb1v5\n");
+
+ __twl4030_phy_power(twl, 1);
+ twl4030_usb_write(twl, PHY_CLK_CTRL,
+ twl4030_usb_read(twl, PHY_CLK_CTRL) |
+ (PHY_CLK_CTRL_CLOCKGATING_EN |
+ PHY_CLK_CTRL_CLK32K_EN));
twl->asleep = 0;
return 0;
--
1.8.1.1
--
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 5/5] usb: phy: twl4030-usb: Use mutex instead of spinlock for protecting the data
We're using threaded irq on a I2C bus and we're sleeping in
twl4030_usb_irq() as it calls twl4030_usb_linkstat() which
calls the i2c functions. If we ever need to lock for longer
I2C transaction sequences a mutex will allow us to do that
easily.
Signed-off-by: Tony Lindgren
---
drivers/phy/phy-twl4030-usb.c | 16 +++-
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
index 24ff3c6..1e0e2d1 100644
--- a/drivers/phy/phy-twl4030-usb.c
+++ b/drivers/phy/phy-twl4030-usb.c
@@ -28,7 +28,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -155,7 +154,7 @@ struct twl4030_usb {
struct regulator*usb3v1;
/* for vbus reporting with irqs disabled */
- spinlock_t lock;
+ struct mutexlock;
/* pin configuration */
enum twl4030_usb_mode usb_mode;
@@ -516,13 +515,12 @@ static ssize_t twl4030_usb_vbus_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct twl4030_usb *twl = dev_get_drvdata(dev);
- unsigned long flags;
int ret = -EINVAL;
- spin_lock_irqsave(&twl->lock, flags);
+ mutex_lock(&twl->lock);
ret = sprintf(buf, "%s\n",
twl->vbus_supplied ? "on" : "off");
- spin_unlock_irqrestore(&twl->lock, flags);
+ mutex_unlock(&twl->lock);
return ret;
}
@@ -536,12 +534,12 @@ static irqreturn_t twl4030_usb_irq(int irq, void *_twl)
status = twl4030_usb_linkstat(twl);
- spin_lock_irq(&twl->lock);
+ mutex_lock(&twl->lock);
if (status >= 0 && status != twl->linkstat) {
twl->linkstat = status;
status_changed = true;
}
- spin_unlock_irq(&twl->lock);
+ mutex_unlock(&twl->lock);
if (status_changed) {
/* FIXME add a set_power() method so that B-devices can
@@ -695,8 +693,8 @@ static int twl4030_usb_probe(struct platform_device *pdev)
if (IS_ERR(phy_provider))
return PTR_ERR(phy_provider);
- /* init spinlock for workqueue */
- spin_lock_init(&twl->lock);
+ /* init mutex for workqueue */
+ mutex_init(&twl->lock);
INIT_DELAYED_WORK(&twl->id_workaround_work, twl4030_id_workaround_work);
--
1.8.1.1
--
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 net-next] r8152: replace strncpy with strlcpy
From: Hayes Wang Date: Tue, 26 Aug 2014 10:08:23 +0800 > Replace the strncpy with strlcpy, and use sizeof to determine the > length. > > Signed-off-by: Hayes Wang Applied, 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 v2 2/9] mailbox: Add NVIDIA Tegra XUSB mailbox driver
On 08/27/2014 03:56 PM, Andrew Bresticker wrote:
On Wed, Aug 27, 2014 at 11:19 AM, Stephen Warren wrote:
On 08/27/2014 12:13 PM, Andrew Bresticker wrote:
On Wed, Aug 27, 2014 at 10:50 AM, Stephen Warren
wrote:
On 08/27/2014 11:38 AM, Andrew Bresticker wrote:
On Mon, Aug 25, 2014 at 12:01 PM, Stephen Warren
wrote:
On 08/18/2014 11:08 AM, Andrew Bresticker wrote:
+static int tegra_xusb_mbox_probe(struct platform_device *pdev)
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res)
+ return -ENODEV;
Should devm_request_mem_region() be called here to claim the region?
No, the xHCI host driver also needs to map these registers, so they
cannot be mapped exclusively here.
That's unfortunate. Having multiple drivers with overlapping register
regions is not a good idea. Can we instead have a top-level driver map
all
the IO regions, then instantiate the various different sub-components
internally, and divide up the address space. Probably via MFD or similar.
That would prevent multiple drivers from touching the same register
region.
Perhaps I'm misunderstanding, but I don't see how MFD would prevent us
from having to map this register space in two different locations -
the XUSB FPCI address space cannot be divided cleanly between host and
mailbox registers. Or are you saying that there should be a separate
device driver that exposes an API for accessing this register space,
like the Tegra fuse or PMC drivers?
With MFD, there's typically a top-level driver for the HW module (or
register space) that gets instantiated by the DT node. This driver then
instantiates all the different sub-drivers that use that register space, and
provides APIs for the sub-drivers to access the registers (either custom
APIs or more recently by passing a regmap object down to the sub-drivers).
This top-level driver is the only driver that maps the space, and can manage
sharing the space between the various sub-drivers.
So if I'm understanding correctly, we end up with something like this:
usb@7009 {
compatible = "nvidia,tegra124-xusb";
reg = <0x0 0x7009 0x0 0x8000>, // xHCI host registers
<0x0 0x70098000 0x0 0x1000>, // FPCI registers
<0x0 0x70099000 0x0 0x1000>; // IPFS registers
interrupts = , // host interrupt
Something like that, yes.
Given that the "xusb" driver knows that its HW module contains both an
XHCI and XUSB mailbox chunk, those might not need to appear inside the
main XUSB module, but could be hard-coded into the driver. They might
serve as convenient containers for sub-device-specific properties though.
Other alternatives might be:
a) If the registers that are shared between drivers are distinct, then
divide the reg values into non-overlapping lists. We have taken this
approach for the MC/SMMU drivers on Tegra, although it's a horrible mess
and Thierry is actively thinking about reverting that and doing it
through MFD or something MFD-like.
b) Allow the same IO region to be claimed by multiple devices, perhaps
with a new API so that it doesn't accidentally happen when not desired.
c) Ignore the issue and deal with the fact that not all driver usage
shows up in /proc/iomem. This is what we have for the Tegra USB2 and
USB2 PHY drivers, and this is (I think) what your current patch does.
To be honest, none of the options are that good; some end up with the
correct result but are a pain to implement, but others are nice and
simple yet /proc/iomem isn't complete. Given that, I'm personally not
going to try and mandate one option or the other, so the current patch
is fine. Food for thought though:-)
--
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: host: xhci: fix compliance mode workaround
On Wed, Aug 27, 2014 at 04:38:04PM -0500, Felipe Balbi wrote:
> Commit 71c731a (usb: host: xhci: Fix Compliance Mode
> on SN65LVP3502CP Hardware) implemented a workaround
> for a known issue with Texas Instruments' USB 3.0
> redriver IC but it left a condition where any xHCI
> host would be taken out of reset if port was placed
> in compliance mode and there was no device connected
> to the port.
>
> That condition would trigger a fake connection to a
> non-existent device so that usbcore would trigger a
> warm reset of the port, thus taking the link out of
> reset.
>
> This has the side-effect of preventing any xHCI host
> connected to a Linux machine from starting and running
> the USB 3.0 Electrical Compliance Suite because the
> port will mysteriously taken out of compliance mode
> and, thus, xHCI won't step through the necessary
> compliance patterns for link validation.
>
> This patch fixes the issue by just adding a missing
> check for XHCI_COMP_MODE_QUIRK inside
> xhci_hub_report_usb3_link_state() when PORT_CAS isn't
> set.
>
> This patch should be backported to all kernels containing
> commit 71c731a.
>
> Fixes: 71c731a (usb: host: xhci: Fix Compliance Mode on SN65LVP3502CP
> Hardware)
> Cc: Alexis R. Cortes
> Cc: # v3.2+
> Signed-off-by: Felipe Balbi
> ---
>
> This has been tested on a certification setup with LeCroy Voyager M3i
> and a really expensive oscilloscope :-)
>
> Without this patch we cannot keep the host in compliance.
>
> drivers/usb/host/xhci-hub.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
> index aa79e87..69aece3 100644
> --- a/drivers/usb/host/xhci-hub.c
> +++ b/drivers/usb/host/xhci-hub.c
> @@ -468,7 +468,8 @@ static void xhci_hub_report_usb2_link_state(u32 *status,
> u32 status_reg)
> }
>
> /* Updates Link Status for super Speed port */
> -static void xhci_hub_report_usb3_link_state(u32 *status, u32 status_reg)
> +static void xhci_hub_report_usb3_link_state(struct xhci_hcd *xhci,
> + u32 *status, u32 status_reg)
> {
> u32 pls = status_reg & PORT_PLS_MASK;
>
> @@ -507,7 +508,8 @@ static void xhci_hub_report_usb3_link_state(u32 *status,
> u32 status_reg)
>* in which sometimes the port enters compliance mode
>* caused by a delay on the host-device negotiation.
>*/
> - if (pls == USB_SS_PORT_LS_COMP_MOD)
> + if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) &&
> + (pls == USB_SS_PORT_LS_COMP_MOD))
> pls |= USB_PORT_STAT_CONNECTION;
> }
>
> @@ -666,7 +668,7 @@ static u32 xhci_get_port_status(struct usb_hcd *hcd,
> }
> /* Update Port Link State */
> if (hcd->speed == HCD_USB3) {
> - xhci_hub_report_usb3_link_state(&status, raw_port_status);
> + xhci_hub_report_usb3_link_state(xhci, &status, raw_port_status);
> /*
>* Verify if all USB3 Ports Have entered U0 already.
>* Delete Compliance Mode Timer if so.
Looks good, Mathias, can I get an ACK so I can queue this up now?
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: [RFC 1/2] USB: OTG: Hold wakeupsource when VBUS present
On Fri, Aug 22, 2014 at 10:12 AM, Felipe Balbi wrote: ... > you never explain why this is needed and you have also added some > information to commit log which shouldn't be here. Android uses this to prevent suspend from interfering with USB peripheral traffic, notably adb. The wakeup source is held only when USB is connected and enumerated for a host session (I might be using wrong terminology here). It may not be necessary on a platform that implements wakeup on incoming USB traffic, although it is likely adb and other protocols would need to hold wakeup sources at certain times. ... >> +static struct otgws_lock vbus_lock; > > should be per-PHY One of the reasons this was done as a separate driver and via notifiers was to keep the (original Android wakelock) logic out of the USB code. If the general idea is something that finds favor with the USB and PM folks then perhaps adding a wakeup source per PHY in the PHY driver would be better. Todd -- 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/9] mailbox: Add NVIDIA Tegra XUSB mailbox driver
On Wed, Aug 27, 2014 at 11:19 AM, Stephen Warren wrote:
> On 08/27/2014 12:13 PM, Andrew Bresticker wrote:
>>
>> On Wed, Aug 27, 2014 at 10:50 AM, Stephen Warren
>> wrote:
>>>
>>> On 08/27/2014 11:38 AM, Andrew Bresticker wrote:
On Mon, Aug 25, 2014 at 12:01 PM, Stephen Warren
wrote:
>
>
> On 08/18/2014 11:08 AM, Andrew Bresticker wrote:
>>
>>
>> +static int tegra_xusb_mbox_probe(struct platform_device *pdev)
>
>
>
>
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>
>> + if (!res)
>> + return -ENODEV;
>
>
>
>
> Should devm_request_mem_region() be called here to claim the region?
No, the xHCI host driver also needs to map these registers, so they
cannot be mapped exclusively here.
>>>
>>>
>>>
>>> That's unfortunate. Having multiple drivers with overlapping register
>>> regions is not a good idea. Can we instead have a top-level driver map
>>> all
>>> the IO regions, then instantiate the various different sub-components
>>> internally, and divide up the address space. Probably via MFD or similar.
>>> That would prevent multiple drivers from touching the same register
>>> region.
>>
>>
>> Perhaps I'm misunderstanding, but I don't see how MFD would prevent us
>> from having to map this register space in two different locations -
>> the XUSB FPCI address space cannot be divided cleanly between host and
>> mailbox registers. Or are you saying that there should be a separate
>> device driver that exposes an API for accessing this register space,
>> like the Tegra fuse or PMC drivers?
>
>
> With MFD, there's typically a top-level driver for the HW module (or
> register space) that gets instantiated by the DT node. This driver then
> instantiates all the different sub-drivers that use that register space, and
> provides APIs for the sub-drivers to access the registers (either custom
> APIs or more recently by passing a regmap object down to the sub-drivers).
>
> This top-level driver is the only driver that maps the space, and can manage
> sharing the space between the various sub-drivers.
So if I'm understanding correctly, we end up with something like this:
usb@7009 {
compatible = "nvidia,tegra124-xusb";
reg = <0x0 0x7009 0x0 0x8000>, // xHCI host registers
<0x0 0x70098000 0x0 0x1000>, // FPCI registers
<0x0 0x70099000 0x0 0x1000>; // IPFS registers
interrupts = , // host interrupt
http://vger.kernel.org/majordomo-info.html
[PATCH] usb: host: xhci: fix compliance mode workaround
Commit 71c731a (usb: host: xhci: Fix Compliance Mode
on SN65LVP3502CP Hardware) implemented a workaround
for a known issue with Texas Instruments' USB 3.0
redriver IC but it left a condition where any xHCI
host would be taken out of reset if port was placed
in compliance mode and there was no device connected
to the port.
That condition would trigger a fake connection to a
non-existent device so that usbcore would trigger a
warm reset of the port, thus taking the link out of
reset.
This has the side-effect of preventing any xHCI host
connected to a Linux machine from starting and running
the USB 3.0 Electrical Compliance Suite because the
port will mysteriously taken out of compliance mode
and, thus, xHCI won't step through the necessary
compliance patterns for link validation.
This patch fixes the issue by just adding a missing
check for XHCI_COMP_MODE_QUIRK inside
xhci_hub_report_usb3_link_state() when PORT_CAS isn't
set.
This patch should be backported to all kernels containing
commit 71c731a.
Fixes: 71c731a (usb: host: xhci: Fix Compliance Mode on SN65LVP3502CP Hardware)
Cc: Alexis R. Cortes
Cc: # v3.2+
Signed-off-by: Felipe Balbi
---
This has been tested on a certification setup with LeCroy Voyager M3i
and a really expensive oscilloscope :-)
Without this patch we cannot keep the host in compliance.
drivers/usb/host/xhci-hub.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index aa79e87..69aece3 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -468,7 +468,8 @@ static void xhci_hub_report_usb2_link_state(u32 *status,
u32 status_reg)
}
/* Updates Link Status for super Speed port */
-static void xhci_hub_report_usb3_link_state(u32 *status, u32 status_reg)
+static void xhci_hub_report_usb3_link_state(struct xhci_hcd *xhci,
+ u32 *status, u32 status_reg)
{
u32 pls = status_reg & PORT_PLS_MASK;
@@ -507,7 +508,8 @@ static void xhci_hub_report_usb3_link_state(u32 *status,
u32 status_reg)
* in which sometimes the port enters compliance mode
* caused by a delay on the host-device negotiation.
*/
- if (pls == USB_SS_PORT_LS_COMP_MOD)
+ if ((xhci->quirks & XHCI_COMP_MODE_QUIRK) &&
+ (pls == USB_SS_PORT_LS_COMP_MOD))
pls |= USB_PORT_STAT_CONNECTION;
}
@@ -666,7 +668,7 @@ static u32 xhci_get_port_status(struct usb_hcd *hcd,
}
/* Update Port Link State */
if (hcd->speed == HCD_USB3) {
- xhci_hub_report_usb3_link_state(&status, raw_port_status);
+ xhci_hub_report_usb3_link_state(xhci, &status, raw_port_status);
/*
* Verify if all USB3 Ports Have entered U0 already.
* Delete Compliance Mode Timer if so.
--
2.0.1.563.g66f467c
--
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: net2280: Remove pci_class from PCI_TABLE
On Wed, Aug 27, 2014 at 11:03:00PM +0200, Ricardo Ribalda Delgado wrote: > Hello Greg > > >> > >> Not many drivers define the pci interface and there is no other driver > >> that has the same vendor and product id. Therefore I see no hurt in > >> adding both patches, one to make the driver broader, and another to > >> fix pci-sysfs. > >> > >> Also, the change on pci-sysfs might affect more stuff and therefore > >> take longer to be applied. > > > > As we have been printing the value to userspace in this way for well > > over a decade now, and nothing has changed, I say it's a userspace bug > > that you should fix instead. Don't work around broken user programs in > > the kernel by changing something that has been stable for 10+ years. > > > > Ok, sorry, not 10+ years, the commit was written May of 2005, so 9 > > years. > > > > Fix your module loading code please. > > On the other thread ( https://lkml.org/lkml/2014/8/27/242 ) we have > agreed about fixing this thing on pci-sysfs.c . > > Still I think that there is no good reason to add the pci interface to > the pci_table on this driver. Therefore I consider that this patch is > still valid. > > What do you think. This patch is NACK? Yeah, I don't think this patch is needed as it properly sets the class of the device to be matched against, so it should not be necessary at all. 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] usb: gadget: net2280: Remove pci_class from PCI_TABLE
Greg Kroah-Hartman writes: > On Wed, Aug 27, 2014 at 09:39:43PM +0200, Ricardo Ribalda Delgado wrote: >> >> return sprintf(buf, "pci:v%08Xd%08Xsv%08Xsd%08Xbc%02Xsc%02Xi%02x\n", That final 'x' does look like a typo, doesn't it? We are otherwise consistently using upper-case hex digits for field values and lower case letter for field names. But it looks like it has been like that since the beginning, so it might be difficult to fix... > No, the root cause of the problem is a userspace tool looking at a hex > value as a string and not a number. It doesn't matter if we print it in > upper or lower case, it's a digit, not a string. Do the numeric > compare, not a string compare. Now I don't really know much about the history here, but the format of module aliases, using wildcards, seem to suggest a string match to me. Do you really mean that these strings should be parsed into field names + values before matching? If that was the intention then surely we would have exported the fields one-by-one as separate sysfs attributes? Ref the "one value per file" policy. >> Not many drivers define the pci interface and there is no other driver >> that has the same vendor and product id. Therefore I see no hurt in >> adding both patches, one to make the driver broader, and another to >> fix pci-sysfs. >> >> Also, the change on pci-sysfs might affect more stuff and therefore >> take longer to be applied. > > As we have been printing the value to userspace in this way for well > over a decade now, and nothing has changed, I say it's a userspace bug > that you should fix instead. Don't work around broken user programs in > the kernel by changing something that has been stable for 10+ years. > > Ok, sorry, not 10+ years, the commit was written May of 2005, so 9 > years. well, just looking at a few common PCI devices on my PCs I wonder if the reason this hasn't been a problem is because there are _very_ few PCI programming interfaces using anything by 0-9 digits. One? Looking at the modules built by Debian I can only find one udc module matching on any hex value: bjorn@nemi:~$ grep pci: /lib/modules/3.16-trunk-amd64/modules.alias|egrep "i[A-F]" alias pci:v10DBd8808sv*sd*bc0Csc03iFE* pch_udc alias pci:v10DBd801Dsv*sd*bc0Csc03iFE* pch_udc alias pci:v8086d8808sv*sd*bc0Csc03iFE* pch_udc This makes me wonder if this is exclusively a problem for PCI UDCs, which tend to be pretty rare devices? Bjørn -- 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 1/3] usb: gadget: Refactor request completion
On Wed, Aug 27, 2014 at 10:57:58PM +0200, Michal Sojka wrote:
> All USB peripheral controller drivers called completion routines
> directly. This patch moves the completion call from drivers to
> usb_gadget_giveback_request(), in order to have a place where common
> functionality can be added.
>
> All places in drivers/usb/ matching "[-.]complete(" were replaced with a
> call to usb_gadget_giveback_request(). This was compile-tested with all
> ARM drivers enabled and runtime-tested for musb.
>
> Signed-off-by: Michal Sojka
> ---
> drivers/usb/chipidea/udc.c | 6 +++---
> drivers/usb/dwc2/gadget.c | 6 +++---
> drivers/usb/dwc3/gadget.c | 2 +-
> drivers/usb/gadget/udc/amd5536udc.c | 2 +-
> drivers/usb/gadget/udc/at91_udc.c | 2 +-
> drivers/usb/gadget/udc/atmel_usba_udc.c | 4 ++--
> drivers/usb/gadget/udc/bcm63xx_udc.c| 2 +-
> drivers/usb/gadget/udc/dummy_hcd.c | 10 +-
> drivers/usb/gadget/udc/fotg210-udc.c| 2 +-
> drivers/usb/gadget/udc/fsl_qe_udc.c | 6 +-
> drivers/usb/gadget/udc/fsl_udc_core.c | 6 ++
> drivers/usb/gadget/udc/fusb300_udc.c| 2 +-
> drivers/usb/gadget/udc/goku_udc.c | 2 +-
> drivers/usb/gadget/udc/gr_udc.c | 2 +-
> drivers/usb/gadget/udc/lpc32xx_udc.c| 2 +-
> drivers/usb/gadget/udc/m66592-udc.c | 2 +-
> drivers/usb/gadget/udc/mv_u3d_core.c| 8 ++--
> drivers/usb/gadget/udc/mv_udc_core.c| 8 ++--
> drivers/usb/gadget/udc/net2272.c| 2 +-
> drivers/usb/gadget/udc/net2280.c| 2 +-
> drivers/usb/gadget/udc/omap_udc.c | 2 +-
> drivers/usb/gadget/udc/pch_udc.c| 2 +-
> drivers/usb/gadget/udc/pxa25x_udc.c | 2 +-
> drivers/usb/gadget/udc/pxa27x_udc.c | 2 +-
> drivers/usb/gadget/udc/r8a66597-udc.c | 2 +-
> drivers/usb/gadget/udc/s3c-hsudc.c | 3 +--
> drivers/usb/gadget/udc/s3c2410_udc.c| 2 +-
> drivers/usb/gadget/udc/udc-core.c | 17 +
> drivers/usb/musb/musb_gadget.c | 2 +-
> drivers/usb/renesas_usbhs/mod_gadget.c | 2 +-
> include/linux/usb/gadget.h | 8
> 31 files changed, 66 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index b8125aa..0444d3f 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -627,7 +627,7 @@ __acquires(hwep->lock)
>
> if (hwreq->req.complete != NULL) {
> spin_unlock(hwep->lock);
> - hwreq->req.complete(&hwep->ep, &hwreq->req);
> + usb_gadget_giveback_request(&hwep->ep, &hwreq->req);
> spin_lock(hwep->lock);
> }
> }
> @@ -922,7 +922,7 @@ __acquires(hwep->lock)
> if ((hwep->type == USB_ENDPOINT_XFER_CONTROL) &&
> hwreq->req.length)
> hweptemp = hwep->ci->ep0in;
> - hwreq->req.complete(&hweptemp->ep, &hwreq->req);
> + usb_gadget_giveback_request(&hweptemp->ep, &hwreq->req);
> spin_lock(hwep->lock);
> }
> }
> @@ -1347,7 +1347,7 @@ static int ep_dequeue(struct usb_ep *ep, struct
> usb_request *req)
>
> if (hwreq->req.complete != NULL) {
> spin_unlock(hwep->lock);
> - hwreq->req.complete(&hwep->ep, &hwreq->req);
> + usb_gadget_giveback_request(&hwep->ep, &hwreq->req);
> spin_lock(hwep->lock);
> }
>
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 0ba9c33..5a524a6 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -987,8 +987,8 @@ static int s3c_hsotg_process_req_feature(struct s3c_hsotg
> *hsotg,
> hs_req = ep->req;
> ep->req = NULL;
> list_del_init(&hs_req->queue);
> - hs_req->req.complete(&ep->ep,
> - &hs_req->req);
> + usb_gadget_giveback_request(&ep->ep,
> +
> &hs_req->req);
> }
>
> /* If we have pending request, then start it */
> @@ -1245,7 +1245,7 @@ static void s3c_hsotg_complete_request(struct s3c_hsotg
> *hsotg,
>
> if (hs_req->req.complete) {
> spin_unlock(&hsotg->lock);
> - hs_req->req.complete(&hs_ep->ep, &hs_req->req);
> + usb_gadget_giveback_request(&hs_ep->ep, &hs_req->req);
> spin_lock(&hsotg->lock);
> }
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 349cacc..b4b7a6b 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/us
Re: [PATCH v4 3/3] usb: Add LED triggers for USB activity
On Wed, Aug 27, 2014 at 10:58:00PM +0200, Michal Sojka wrote:
> With this patch, USB activity can be signaled by blinking a LED. There
> are two triggers, one for activity on USB host and one for USB gadget.
>
> Both trigger should work with all host/device controllers. Tested only
> with musb.
>
> Signed-off-by: Michal Sojka
> ---
> drivers/usb/Kconfig | 11
> drivers/usb/common/Makefile | 1 +
> drivers/usb/common/led.c | 56
> +++
> drivers/usb/core/hcd.c| 2 ++
> drivers/usb/gadget/udc/udc-core.c | 4 +++
> include/linux/usb.h | 12 +
> 6 files changed, 86 insertions(+)
> create mode 100644 drivers/usb/common/led.c
>
> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
> index e0cad441..fc90cda 100644
> --- a/drivers/usb/Kconfig
> +++ b/drivers/usb/Kconfig
> @@ -147,4 +147,15 @@ source "drivers/usb/phy/Kconfig"
>
> source "drivers/usb/gadget/Kconfig"
>
> +config USB_LED_TRIG
> + bool "USB LED Triggers"
> + depends on LEDS_CLASS && USB_COMMON
> + select LEDS_TRIGGERS
I hate select lines, can't we just depend on LEDS_TRIGGERS as well as
LEDS_CLASS?
> + help
> + This option adds LED triggers for USB host and/or gadget activity.
> +
> + Say Y here if you are working on a system with led-class supported
> + LEDs and you want to use them as activity indicators for USB host or
> + gadget.
> +
> endif # USB_SUPPORT
> diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
> index 052c120..ca2f8bd 100644
> --- a/drivers/usb/common/Makefile
> +++ b/drivers/usb/common/Makefile
> @@ -4,5 +4,6 @@
>
> obj-$(CONFIG_USB_COMMON) += usb-common.o
> usb-common-y += common.o
> +usb-common-$(CONFIG_USB_LED_TRIG) += led.o
>
> obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
> diff --git a/drivers/usb/common/led.c b/drivers/usb/common/led.c
> new file mode 100644
> index 000..af3ce2c
> --- /dev/null
> +++ b/drivers/usb/common/led.c
> @@ -0,0 +1,56 @@
> +/*
> + * LED Triggers for USB Activity
> + *
> + * Copyright 2014 Michal Sojka
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define BLINK_DELAY 30
> +
> +static unsigned long usb_blink_delay = BLINK_DELAY;
> +
> +DEFINE_LED_TRIGGER(ledtrig_usb_gadget);
> +DEFINE_LED_TRIGGER(ledtrig_usb_host);
> +
> +void usb_led_activity(enum usb_led_event ev)
> +{
> + struct led_trigger *trig = NULL;
> +
> + switch (ev) {
> + case USB_LED_EVENT_GADGET: trig = ledtrig_usb_gadget; break;
> + case USB_LED_EVENT_HOST: trig = ledtrig_usb_host;break;
> + }
Very odd formatting, please use the correct one and don't try to put
case expressions all on one line.
> + led_trigger_blink_oneshot(trig, &usb_blink_delay, &usb_blink_delay, 0);
What happens if trig is NULL?
> +}
> +EXPORT_SYMBOL(usb_led_activity);
EXPORT_SYMBOL_GPL() please.
> +static int __init ledtrig_usb_init(void)
> +{
> +#ifdef CONFIG_USB_GADGET
> + led_trigger_register_simple("usb-gadget", &ledtrig_usb_gadget);
> +#endif
> +#ifdef CONFIG_USB
> + led_trigger_register_simple("usb-host", &ledtrig_usb_host);
> +#endif
Why not just always register both?
> + return 0;
> +}
> +
> +static void __exit ledtrig_usb_exit(void)
> +{
> + led_trigger_unregister_simple(ledtrig_usb_gadget);
> + led_trigger_unregister_simple(ledtrig_usb_host);
So you can unregister things that you never registered with no issues?
> +}
> +
> +module_init(ledtrig_usb_init);
> +module_exit(ledtrig_usb_exit);
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 487abcf..409cb95 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -1664,6 +1664,8 @@ static void __usb_hcd_giveback_urb(struct urb *urb)
> usbmon_urb_complete(&hcd->self, urb, status);
> usb_anchor_suspend_wakeups(anchor);
> usb_unanchor_urb(urb);
> + if (likely(status == 0))
> + usb_led_activity(USB_LED_EVENT_HOST);
That's a _really_ hot code path, how much is this going to cause in
overhead?
> /* pass ownership to the completion handler */
> urb->status = status;
> diff --git a/drivers/usb/gadget/udc/udc-core.c
> b/drivers/usb/gadget/udc/udc-core.c
> index c1df19b..9fc4a36 100644
> --- a/drivers/usb/gadget/udc/udc-core.c
> +++ b/drivers/usb/gadget/udc/udc-core.c
> @@ -27,6 +27,7 @@
>
> #include
> #include
> +#include
>
> /**
> * struct usb_udc - describes one usb device controller
> @@ -116,6 +117,9 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
> void usb_gadget_giveback_request(struct usb_ep *ep,
> struct usb_request *req)
> {
> + if (likely(req->status == 0))
> + usb_
[PATCH 3/5] tools: ffs-test: add compatibility code for old kernels
If ffs-test is used with a kernel prior to 3.14, which do not
support the new descriptors format, it will fail when trying to
write the descriptors. Add a function that converts the new
descriptors to the legacy ones and use it to retry writing the
descriptors using the legacy format.
Also add “-l” flag to ffs-test which will cause the tool to
never try the new format and instead immediatelly try the
legacy one. This should be useful to test whether parsing
of the old format still works on given 3.14+ kernel.
Signed-off-by: Michal Nazarewicz
---
tools/usb/ffs-test.c | 112 ---
1 file changed, 107 insertions(+), 5 deletions(-)
diff --git a/tools/usb/ffs-test.c b/tools/usb/ffs-test.c
index 708d317..88d5e71 100644
--- a/tools/usb/ffs-test.c
+++ b/tools/usb/ffs-test.c
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -172,6 +173,89 @@ static const struct {
},
};
+static size_t descs_to_legacy(void **legacy, const void *descriptors_v2)
+{
+ const unsigned char *descs_end, *descs_start;
+ __u32 length, fs_count = 0, hs_count = 0, count;
+
+ /* Read v2 header */
+ {
+ const struct {
+ const struct usb_functionfs_descs_head_v2 header;
+ const __le32 counts[];
+ } __attribute__((packed)) *const in = descriptors_v2;
+ const __le32 *counts = in->counts;
+ __u32 flags;
+
+ if (le32_to_cpu(in->header.magic) !=
+ FUNCTIONFS_DESCRIPTORS_MAGIC_V2)
+ return 0;
+ length = le32_to_cpu(in->header.length);
+ if (length <= sizeof in->header)
+ return 0;
+ length -= sizeof in->header;
+ flags = le32_to_cpu(in->header.flags);
+ if (flags & ~(FUNCTIONFS_HAS_FS_DESC | FUNCTIONFS_HAS_HS_DESC |
+ FUNCTIONFS_HAS_SS_DESC))
+ return 0;
+
+#define GET_NEXT_COUNT_IF_FLAG(ret, flg) do { \
+ if (!(flags & (flg))) \
+ break; \
+ if (length < 4) \
+ return 0; \
+ ret = le32_to_cpu(*counts); \
+ length -= 4;\
+ ++counts; \
+ } while (0)
+
+ GET_NEXT_COUNT_IF_FLAG(fs_count, FUNCTIONFS_HAS_FS_DESC);
+ GET_NEXT_COUNT_IF_FLAG(hs_count, FUNCTIONFS_HAS_HS_DESC);
+ GET_NEXT_COUNT_IF_FLAG(count, FUNCTIONFS_HAS_SS_DESC);
+
+ count = fs_count + hs_count;
+ if (!count)
+ return 0;
+ descs_start = (const void *)counts;
+
+#undef GET_NEXT_COUNT_IF_FLAG
+ }
+
+ /*
+* Find the end of FS and HS USB descriptors. SS descriptors
+* are ignored since legacy format does not support them.
+*/
+ descs_end = descs_start;
+ do {
+ if (length < *descs_end)
+ return 0;
+ length -= *descs_end;
+ descs_end += *descs_end;
+ } while (--count);
+
+ /* Allocate legacy descriptors and copy the data. */
+ {
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+ struct {
+ struct usb_functionfs_descs_head header;
+ __u8 descriptors[];
+ } __attribute__((packed)) *out;
+#pragma GCC diagnostic pop
+
+ length = sizeof out->header + (descs_end - descs_start);
+ out = malloc(length);
+ out->header.magic = cpu_to_le32(FUNCTIONFS_DESCRIPTORS_MAGIC);
+ out->header.length = cpu_to_le32(length);
+ out->header.fs_count = cpu_to_le32(fs_count);
+ out->header.hs_count = cpu_to_le32(hs_count);
+ memcpy(out->descriptors, descs_start, descs_end - descs_start);
+ *legacy = out;
+ }
+
+ return length;
+}
+
#define STR_INTERFACE_ "Source/Sink"
@@ -491,12 +575,29 @@ ep0_consume(struct thread *ignore, const void *buf,
size_t nbytes)
return nbytes;
}
-static void ep0_init(struct thread *t)
+static void ep0_init(struct thread *t, bool legacy_descriptors)
{
+ void *legacy;
ssize_t ret;
+ size_t len;
+
+ if (legacy_descriptors) {
+ info("%s: writing descriptors\n", t->filename);
+ goto legacy;
+ }
- info("%s: writing descriptors\n", t->filename);
+ info("%s: writing descriptors (in v2 format)\n", t->filename);
ret = write(t->fd, &descriptors, sizeof descriptors);
+
+ if (ret < 0 && errno == EINVAL) {
+ warn("%
[PATCH 0/5] Small USB fixes/improvements
Apparently I've slept over 3.17 merge window, so I guess this is for 3.18. Rebased on top of balbi/usb.git next. Michal Nazarewicz (4): usb: gadget: f_fs: add usb_functionfs_descs_head_v2 structure tools: ffs-test: convert to new descriptor format tools: ffs-test: add compatibility code for old kernels usb: gadget: f_mass_storage: simplify start_transfer slightly Yang Wei (1): USB:gadget: Fix a warning while loading g_mass_storage drivers/usb/gadget/function/f_mass_storage.c | 33 +++ include/uapi/linux/usb/functionfs.h | 12 ++- tools/usb/ffs-test.c | 126 --- 3 files changed, 144 insertions(+), 27 deletions(-) -- 2.1.0.rc2.206.gedb03e5 -- 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 4/5] usb: gadget: f_mass_storage: simplify start_transfer slightly
Flatten the start_transfer function by reversing the if condition and
returning early out of the function if everything went fine. It makes
the function look less complicated, at least to me, and easier to
understand.
Signed-off-by: Michal Nazarewicz
---
drivers/usb/gadget/function/f_mass_storage.c | 27 +--
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/gadget/function/f_mass_storage.c
b/drivers/usb/gadget/function/f_mass_storage.c
index b963939..811929c 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -566,22 +566,22 @@ static void start_transfer(struct fsg_dev *fsg, struct
usb_ep *ep,
*pbusy = 1;
*state = BUF_STATE_BUSY;
spin_unlock_irq(&fsg->common->lock);
+
rc = usb_ep_queue(ep, req, GFP_KERNEL);
- if (rc != 0) {
- *pbusy = 0;
- *state = BUF_STATE_EMPTY;
+ if (rc == 0)
+ return; /* All good, we're done */
- /* We can't do much more than wait for a reset */
+ *pbusy = 0;
+ *state = BUF_STATE_EMPTY;
- /*
-* Note: currently the net2280 driver fails zero-length
-* submissions if DMA is enabled.
-*/
- if (rc != -ESHUTDOWN &&
- !(rc == -EOPNOTSUPP && req->length == 0))
- WARNING(fsg, "error in submission: %s --> %d\n",
- ep->name, rc);
- }
+ /* We can't do much more than wait for a reset */
+
+ /*
+* Note: currently the net2280 driver fails zero-length
+* submissions if DMA is enabled.
+*/
+ if (rc != -ESHUTDOWN && !(rc == -EOPNOTSUPP && req->length == 0))
+ WARNING(fsg, "error in submission: %s --> %d\n", ep->name, rc);
}
static bool start_in_transfer(struct fsg_common *common, struct fsg_buffhd *bh)
@@ -3665,4 +3665,3 @@ void fsg_config_from_params(struct fsg_config *cfg,
cfg->fsg_num_buffers = fsg_num_buffers;
}
EXPORT_SYMBOL_GPL(fsg_config_from_params);
-
--
2.1.0.rc2.206.gedb03e5
--
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 1/5] usb: gadget: f_fs: add usb_functionfs_descs_head_v2 structure
The structure can be used with user space tools that use the new
functionfs description format, for example as follows:
static const struct {
struct usb_functionfs_descs_head_v2 header;
__le32 fs_count;
__le32 hs_count;
struct {
…
} fs_desc;
struct {
…
} hs_desc;
} descriptors = {
.header = {
.magic = cpu_to_le32(FUNCTIONFS_DESCRIPTORS_MAGIC_V2),
.length = cpu_to_le32(sizeof(descriptors)),
.flags = cpu_to_le32(FUNCTIONFS_HAS_FS_DESC |
FUNCTIONFS_HAS_HS_DESC)
},
.fs_count = cpu_to_le32(X),
.fs_desc = {
…
},
.hs_count = cpu_to_le32(Y),
.hs_desc = {
…
}
};
Signed-off-by: Michal Nazarewicz
---
include/uapi/linux/usb/functionfs.h | 10 ++
1 file changed, 10 insertions(+)
diff --git a/include/uapi/linux/usb/functionfs.h
b/include/uapi/linux/usb/functionfs.h
index 0154b28..3ca03de 100644
--- a/include/uapi/linux/usb/functionfs.h
+++ b/include/uapi/linux/usb/functionfs.h
@@ -32,6 +32,16 @@ struct usb_endpoint_descriptor_no_audio {
__u8 bInterval;
} __attribute__((packed));
+struct usb_functionfs_descs_head_v2 {
+ __le32 magic;
+ __le32 length;
+ __le32 flags;
+ /*
+* __le32 fs_count, hs_count, fs_count; must be included manually in
+* the structure taking flags into consideration.
+*/
+} __attribute__((packed));
+
/* Legacy format, deprecated as of 3.14. */
struct usb_functionfs_descs_head {
__le32 magic;
--
2.1.0.rc2.206.gedb03e5
--
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 5/5] USB:gadget: Fix a warning while loading g_mass_storage
From: Yang Wei While loading g_mass_storage module, the following warning is triggered. WARNING: at drivers/usb/gadget/composite.c: usb_composite_setup_continue: Unexpected call Modules linked in: fat vfat minix nls_cp437 nls_iso8859_1 g_mass_storage [<800179cc>] (unwind_backtrace+0x0/0x104) from [<80619608>] (dump_stack+0x20/0x24) [<80619608>] (dump_stack+0x20/0x24) from [<80025100>] (warn_slowpath_common+0x64/0x74) [<80025100>] (warn_slowpath_common+0x64/0x74) from [<800251cc>] (warn_slowpath_fmt+0x40/0x48) [<800251cc>] (warn_slowpath_fmt+0x40/0x48) from [<7f047774>] (usb_composite_setup_continue+0xb4/0xbc [g_mass_storage]) [<7f047774>] (usb_composite_setup_continue+0xb4/0xbc [g_mass_storage]) from [<7f047ad4>] (handle_exception+0x358/0x3e4 [g_mass_storage]) [<7f047ad4>] (handle_exception+0x358/0x3e4 [g_mass_storage]) from [<7f048080>] (fsg_main_thread+0x520/0x157c [g_mass_storage]) [<7f048080>] (fsg_main_thread+0x520/0x157c [g_mass_storage]) from [<8004bc90>] (kthread+0x98/0x9c) [<8004bc90>] (kthread+0x98/0x9c) from [<8000faec>] (kernel_thread_exit+0x0/0x8) The root cause is that the existing code fails to take into account the possibility that common->new_fsg can change while do_set_interface() is running, so the value of common->new_fsg that gets tested after do_set_interface returns needs to be the same as the value used by do_set_interface. Signed-off-by: Yang Wei Signed-off-by: Michal Nazarewicz Acked-by: Alan Stern --- drivers/usb/gadget/function/f_mass_storage.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c index 811929c..cf4b202 100644 --- a/drivers/usb/gadget/function/f_mass_storage.c +++ b/drivers/usb/gadget/function/f_mass_storage.c @@ -2342,6 +2342,7 @@ static void handle_exception(struct fsg_common *common) struct fsg_buffhd *bh; enum fsg_state old_state; struct fsg_lun *curlun; + struct fsg_dev *new_fsg; unsigned intexception_req_tag; /* @@ -2460,8 +2461,9 @@ static void handle_exception(struct fsg_common *common) break; case FSG_STATE_CONFIG_CHANGE: - do_set_interface(common, common->new_fsg); - if (common->new_fsg) + new_fsg = common->new_fsg; + do_set_interface(common, new_fsg); + if (new_fsg) usb_composite_setup_continue(common->cdev); break; -- 2.1.0.rc2.206.gedb03e5 -- 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 2/5] tools: ffs-test: convert to new descriptor format
Since commit [ac8dde11: “Add flags to descriptors block”] functionfs
supports a new, more powerful and extensible, descriptor format.
Since ffs-test is probably the first thing users of the functionfs
interface see when they start writing functionfs user space daemons,
convert it to use the new format thus promoting it.
Signed-off-by: Michal Nazarewicz
---
include/uapi/linux/usb/functionfs.h | 2 +-
tools/usb/ffs-test.c| 14 +-
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/include/uapi/linux/usb/functionfs.h
b/include/uapi/linux/usb/functionfs.h
index 3ca03de..6d2a16b 100644
--- a/include/uapi/linux/usb/functionfs.h
+++ b/include/uapi/linux/usb/functionfs.h
@@ -102,7 +102,7 @@ struct usb_ext_prop_desc {
* structure. Any flags that are not recognised cause the whole block to be
* rejected with -ENOSYS.
*
- * Legacy descriptors format:
+ * Legacy descriptors format (deprecated as of 3.14):
*
* | off | name | type | description |
* |-+---+--+--|
diff --git a/tools/usb/ffs-test.c b/tools/usb/ffs-test.c
index a87e99f..708d317 100644
--- a/tools/usb/ffs-test.c
+++ b/tools/usb/ffs-test.c
@@ -1,5 +1,5 @@
/*
- * ffs-test.c.c -- user mode filesystem api for usb composite function
+ * ffs-test.c -- user mode filesystem api for usb composite function
*
* Copyright (C) 2010 Samsung Electronics
*Author: Michal Nazarewicz
@@ -106,7 +106,9 @@ static void _msg(unsigned level, const char *fmt, ...)
/ Descriptors and Strings ***/
static const struct {
- struct usb_functionfs_descs_head header;
+ struct usb_functionfs_descs_head_v2 header;
+ __le32 fs_count;
+ __le32 hs_count;
struct {
struct usb_interface_descriptor intf;
struct usb_endpoint_descriptor_no_audio sink;
@@ -114,11 +116,12 @@ static const struct {
} __attribute__((packed)) fs_descs, hs_descs;
} __attribute__((packed)) descriptors = {
.header = {
- .magic = cpu_to_le32(FUNCTIONFS_DESCRIPTORS_MAGIC),
+ .magic = cpu_to_le32(FUNCTIONFS_DESCRIPTORS_MAGIC_V2),
+ .flags = cpu_to_le32(FUNCTIONFS_HAS_FS_DESC |
+FUNCTIONFS_HAS_HS_DESC),
.length = cpu_to_le32(sizeof descriptors),
- .fs_count = cpu_to_le32(3),
- .hs_count = cpu_to_le32(3),
},
+ .fs_count = cpu_to_le32(3),
.fs_descs = {
.intf = {
.bLength = sizeof descriptors.fs_descs.intf,
@@ -142,6 +145,7 @@ static const struct {
/* .wMaxPacketSize = autoconfiguration (kernel) */
},
},
+ .hs_count = cpu_to_le32(3),
.hs_descs = {
.intf = {
.bLength = sizeof descriptors.fs_descs.intf,
--
2.1.0.rc2.206.gedb03e5
--
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: net2280: Remove pci_class from PCI_TABLE
Hello Greg >> >> Not many drivers define the pci interface and there is no other driver >> that has the same vendor and product id. Therefore I see no hurt in >> adding both patches, one to make the driver broader, and another to >> fix pci-sysfs. >> >> Also, the change on pci-sysfs might affect more stuff and therefore >> take longer to be applied. > > As we have been printing the value to userspace in this way for well > over a decade now, and nothing has changed, I say it's a userspace bug > that you should fix instead. Don't work around broken user programs in > the kernel by changing something that has been stable for 10+ years. > > Ok, sorry, not 10+ years, the commit was written May of 2005, so 9 > years. > > Fix your module loading code please. On the other thread ( https://lkml.org/lkml/2014/8/27/242 ) we have agreed about fixing this thing on pci-sysfs.c . Still I think that there is no good reason to add the pci interface to the pci_table on this driver. Therefore I consider that this patch is still valid. What do you think. This patch is NACK? Thanks! > > thanks, > > greg k-h -- Ricardo Ribalda -- 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 1/3] usb: gadget: Refactor request completion
On Wed, 27 Aug 2014, Michal Sojka wrote:
> +/**
> + * usb_gadget_giveback_request - give the request back to the gadget layer
> + * Context: in_interrupt()
> + *
> + * This is called by device controller drivers in order to return the
> + * completed request back to the gadget layer.
> + */
> +void usb_gadget_giveback_request(struct usb_ep *ep,
> + struct usb_request *req)
> +{
> + BUG_ON(req->complete == NULL);
> + req->complete(ep, req);
> +}
I guess it doesn't hurt to have the BUG_ON, but it doesn't help either.
Think about what would happen if req->complete was NULL and the BUG_ON
wasn't present.
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 v2] usb: hub: Prevent hub autosuspend if usbcore.autosuspend is -1
On Wed, Aug 27, 2014 at 12:23:39PM -0700, Greg KH wrote: > On Wed, Aug 27, 2014 at 03:11:10PM +0300, Roger Quadros wrote: > > If user specifies that USB autosuspend must be disabled by module > > parameter "usbcore.autosuspend=-1" then we must prevent > > autosuspend of USB hub devices as well. > > > > commit 596d789a211d introduced in v3.8 changed the original behaivour > > and stopped respecting the usbcore.autosuspend parameter for hubs. > > > > Fixes: 596d789a211d "USB: set hub's default autosuspend delay as 0" > > > > Cc: [3.8+] > > Signed-off-by: Roger Quadros > > --- > > drivers/usb/core/hub.c | 8 +++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > > index 8a4dcbc..59b599d 100644 > > --- a/drivers/usb/core/hub.c > > +++ b/drivers/usb/core/hub.c > > @@ -1728,8 +1728,14 @@ static int hub_probe(struct usb_interface *intf, > > const struct usb_device_id *id) > > * - Change autosuspend delay of hub can avoid unnecessary auto > > * suspend timer for hub, also may decrease power consumption > > * of USB bus. > > +* > > +* - If user has indicated to prevent autosuspend by passing > > +* usbcore.autosuspend = -1 then keep autosuspend disabled. > > */ > > - pm_runtime_set_autosuspend_delay(&hdev->dev, 0); > > +#ifdef CONFIG_PM_RUNTIME > > + if (hdev->dev.power.autosuspend_delay >= 0) > > + pm_runtime_set_autosuspend_delay(&hdev->dev, 0); > > +#endif > > > > /* > > * Hubs have proper suspend/resume support, except for root hubs > > Sorry, but can I have a patch that just adds the #ifdef as I already > have your original one in my tree. This should be all that is needed, right? - diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 003cb6b1a6bf..46f5161c7891 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -1732,8 +1732,10 @@ static int hub_probe(struct usb_interface *intf, const struct usb_device_id *id) * - If user has indicated to prevent autosuspend by passing * usbcore.autosuspend = -1 then keep autosuspend disabled. */ +#ifdef CONFIG_PM_RUNTIME if (hdev->dev.power.autosuspend_delay >= 0) pm_runtime_set_autosuspend_delay(&hdev->dev, 0); +#endif /* * Hubs have proper suspend/resume support, except for root hubs -- 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 v4 0/3] LED triggers for USB host and device
This adds LED triggers for USB host and device. First patch refactors UDC drivers as requested by Felipe Balbi, second is a preparation for the third, which adds the LED triggers. Changes from v3: - usb_gadget_giveback_request() moved outside of CONFIG_HAS_DMA conditioned block. - Added kernel-doc for usb_gadget_giveback_request() (Felipe Balbi). - Removed outdated comment (Alan Stern). - req->complete == NULL is now a bug. Previously, this was ignored (Alan Stern). - File rename moved to a separate commit (greg k-h). Changes from v2: - Host/gadget triggers merged to a single file in usb/common/ (Felipe Balbi). - UDC drivers refactored so that LED trigger works for all of them. Changes from v1: - Moved from drivers/leds/ to drivers/usb/. - Improved Kconfig help. - Linked with other modules rather than being standalone modules. Michal Sojka (2): usb: gadget: Refactor request completion usb: Add LED triggers for USB activity drivers/usb/Kconfig | 11 +++ drivers/usb/chipidea/udc.c | 6 +- drivers/usb/common/Makefile | 5 +- drivers/usb/common/common.c | 144 drivers/usb/common/led.c| 59 + drivers/usb/common/usb-common.c | 144 drivers/usb/core/hcd.c | 2 + drivers/usb/dwc2/gadget.c | 6 +- drivers/usb/dwc3/gadget.c | 2 +- drivers/usb/gadget/udc/amd5536udc.c | 2 +- drivers/usb/gadget/udc/at91_udc.c | 2 +- drivers/usb/gadget/udc/atmel_usba_udc.c | 4 +- drivers/usb/gadget/udc/bcm63xx_udc.c| 2 +- drivers/usb/gadget/udc/dummy_hcd.c | 10 +-- drivers/usb/gadget/udc/fotg210-udc.c| 2 +- drivers/usb/gadget/udc/fsl_qe_udc.c | 6 +- drivers/usb/gadget/udc/fsl_udc_core.c | 6 +- drivers/usb/gadget/udc/fusb300_udc.c| 2 +- drivers/usb/gadget/udc/goku_udc.c | 2 +- drivers/usb/gadget/udc/gr_udc.c | 2 +- drivers/usb/gadget/udc/lpc32xx_udc.c| 2 +- drivers/usb/gadget/udc/m66592-udc.c | 2 +- drivers/usb/gadget/udc/mv_u3d_core.c| 8 +- drivers/usb/gadget/udc/mv_udc_core.c| 8 +- drivers/usb/gadget/udc/net2272.c| 2 +- drivers/usb/gadget/udc/net2280.c| 2 +- drivers/usb/gadget/udc/omap_udc.c | 2 +- drivers/usb/gadget/udc/pch_udc.c| 2 +- drivers/usb/gadget/udc/pxa25x_udc.c | 2 +- drivers/usb/gadget/udc/pxa27x_udc.c | 2 +- drivers/usb/gadget/udc/r8a66597-udc.c | 2 +- drivers/usb/gadget/udc/s3c-hsudc.c | 3 +- drivers/usb/gadget/udc/s3c2410_udc.c| 2 +- drivers/usb/gadget/udc/udc-core.c | 13 +++ drivers/usb/musb/musb_gadget.c | 2 +- drivers/usb/renesas_usbhs/mod_gadget.c | 2 +- include/linux/usb.h | 12 +++ include/linux/usb/gadget.h | 8 ++ 38 files changed, 294 insertions(+), 201 deletions(-) create mode 100644 drivers/usb/common/common.c create mode 100644 drivers/usb/common/led.c delete mode 100644 drivers/usb/common/usb-common.c -- 2.1.0 -- 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 v4 3/3] usb: Add LED triggers for USB activity
With this patch, USB activity can be signaled by blinking a LED. There
are two triggers, one for activity on USB host and one for USB gadget.
Both trigger should work with all host/device controllers. Tested only
with musb.
Signed-off-by: Michal Sojka
---
drivers/usb/Kconfig | 11
drivers/usb/common/Makefile | 1 +
drivers/usb/common/led.c | 56 +++
drivers/usb/core/hcd.c| 2 ++
drivers/usb/gadget/udc/udc-core.c | 4 +++
include/linux/usb.h | 12 +
6 files changed, 86 insertions(+)
create mode 100644 drivers/usb/common/led.c
diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
index e0cad441..fc90cda 100644
--- a/drivers/usb/Kconfig
+++ b/drivers/usb/Kconfig
@@ -147,4 +147,15 @@ source "drivers/usb/phy/Kconfig"
source "drivers/usb/gadget/Kconfig"
+config USB_LED_TRIG
+ bool "USB LED Triggers"
+ depends on LEDS_CLASS && USB_COMMON
+ select LEDS_TRIGGERS
+ help
+ This option adds LED triggers for USB host and/or gadget activity.
+
+ Say Y here if you are working on a system with led-class supported
+ LEDs and you want to use them as activity indicators for USB host or
+ gadget.
+
endif # USB_SUPPORT
diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
index 052c120..ca2f8bd 100644
--- a/drivers/usb/common/Makefile
+++ b/drivers/usb/common/Makefile
@@ -4,5 +4,6 @@
obj-$(CONFIG_USB_COMMON) += usb-common.o
usb-common-y += common.o
+usb-common-$(CONFIG_USB_LED_TRIG) += led.o
obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
diff --git a/drivers/usb/common/led.c b/drivers/usb/common/led.c
new file mode 100644
index 000..af3ce2c
--- /dev/null
+++ b/drivers/usb/common/led.c
@@ -0,0 +1,56 @@
+/*
+ * LED Triggers for USB Activity
+ *
+ * Copyright 2014 Michal Sojka
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+#define BLINK_DELAY 30
+
+static unsigned long usb_blink_delay = BLINK_DELAY;
+
+DEFINE_LED_TRIGGER(ledtrig_usb_gadget);
+DEFINE_LED_TRIGGER(ledtrig_usb_host);
+
+void usb_led_activity(enum usb_led_event ev)
+{
+ struct led_trigger *trig = NULL;
+
+ switch (ev) {
+ case USB_LED_EVENT_GADGET: trig = ledtrig_usb_gadget; break;
+ case USB_LED_EVENT_HOST: trig = ledtrig_usb_host;break;
+ }
+ led_trigger_blink_oneshot(trig, &usb_blink_delay, &usb_blink_delay, 0);
+}
+EXPORT_SYMBOL(usb_led_activity);
+
+
+static int __init ledtrig_usb_init(void)
+{
+#ifdef CONFIG_USB_GADGET
+ led_trigger_register_simple("usb-gadget", &ledtrig_usb_gadget);
+#endif
+#ifdef CONFIG_USB
+ led_trigger_register_simple("usb-host", &ledtrig_usb_host);
+#endif
+ return 0;
+}
+
+static void __exit ledtrig_usb_exit(void)
+{
+ led_trigger_unregister_simple(ledtrig_usb_gadget);
+ led_trigger_unregister_simple(ledtrig_usb_host);
+}
+
+module_init(ledtrig_usb_init);
+module_exit(ledtrig_usb_exit);
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 487abcf..409cb95 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1664,6 +1664,8 @@ static void __usb_hcd_giveback_urb(struct urb *urb)
usbmon_urb_complete(&hcd->self, urb, status);
usb_anchor_suspend_wakeups(anchor);
usb_unanchor_urb(urb);
+ if (likely(status == 0))
+ usb_led_activity(USB_LED_EVENT_HOST);
/* pass ownership to the completion handler */
urb->status = status;
diff --git a/drivers/usb/gadget/udc/udc-core.c
b/drivers/usb/gadget/udc/udc-core.c
index c1df19b..9fc4a36 100644
--- a/drivers/usb/gadget/udc/udc-core.c
+++ b/drivers/usb/gadget/udc/udc-core.c
@@ -27,6 +27,7 @@
#include
#include
+#include
/**
* struct usb_udc - describes one usb device controller
@@ -116,6 +117,9 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
void usb_gadget_giveback_request(struct usb_ep *ep,
struct usb_request *req)
{
+ if (likely(req->status == 0))
+ usb_led_activity(USB_LED_EVENT_GADGET);
+
BUG_ON(req->complete == NULL);
req->complete(ep, req);
}
diff --git a/include/linux/usb.h b/include/linux/usb.h
index d2465bc..447a7e2 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1862,6 +1862,18 @@ extern void usb_unregister_notify(struct notifier_block
*nb);
/* debugfs stuff */
extern struct dentry *usb_debug_root;
+/* LED triggers */
+enum usb_led_event {
+ USB_LED_EVENT_HOST = 0,
+ USB_LED_EVENT_GADGET = 1,
+};
+
+#ifdef CONFIG_USB_LED_TRIG
+extern void usb_led_activity(enum usb_led_event ev);
+#else
+static inline void usb_led_activity(enum usb_led_event ev) {}
+#endif
+
#endif /* __KERNEL__ */
#endif
--
2.1.0
[PATCH v4 1/3] usb: gadget: Refactor request completion
All USB peripheral controller drivers called completion routines
directly. This patch moves the completion call from drivers to
usb_gadget_giveback_request(), in order to have a place where common
functionality can be added.
All places in drivers/usb/ matching "[-.]complete(" were replaced with a
call to usb_gadget_giveback_request(). This was compile-tested with all
ARM drivers enabled and runtime-tested for musb.
Signed-off-by: Michal Sojka
---
drivers/usb/chipidea/udc.c | 6 +++---
drivers/usb/dwc2/gadget.c | 6 +++---
drivers/usb/dwc3/gadget.c | 2 +-
drivers/usb/gadget/udc/amd5536udc.c | 2 +-
drivers/usb/gadget/udc/at91_udc.c | 2 +-
drivers/usb/gadget/udc/atmel_usba_udc.c | 4 ++--
drivers/usb/gadget/udc/bcm63xx_udc.c| 2 +-
drivers/usb/gadget/udc/dummy_hcd.c | 10 +-
drivers/usb/gadget/udc/fotg210-udc.c| 2 +-
drivers/usb/gadget/udc/fsl_qe_udc.c | 6 +-
drivers/usb/gadget/udc/fsl_udc_core.c | 6 ++
drivers/usb/gadget/udc/fusb300_udc.c| 2 +-
drivers/usb/gadget/udc/goku_udc.c | 2 +-
drivers/usb/gadget/udc/gr_udc.c | 2 +-
drivers/usb/gadget/udc/lpc32xx_udc.c| 2 +-
drivers/usb/gadget/udc/m66592-udc.c | 2 +-
drivers/usb/gadget/udc/mv_u3d_core.c| 8 ++--
drivers/usb/gadget/udc/mv_udc_core.c| 8 ++--
drivers/usb/gadget/udc/net2272.c| 2 +-
drivers/usb/gadget/udc/net2280.c| 2 +-
drivers/usb/gadget/udc/omap_udc.c | 2 +-
drivers/usb/gadget/udc/pch_udc.c| 2 +-
drivers/usb/gadget/udc/pxa25x_udc.c | 2 +-
drivers/usb/gadget/udc/pxa27x_udc.c | 2 +-
drivers/usb/gadget/udc/r8a66597-udc.c | 2 +-
drivers/usb/gadget/udc/s3c-hsudc.c | 3 +--
drivers/usb/gadget/udc/s3c2410_udc.c| 2 +-
drivers/usb/gadget/udc/udc-core.c | 17 +
drivers/usb/musb/musb_gadget.c | 2 +-
drivers/usb/renesas_usbhs/mod_gadget.c | 2 +-
include/linux/usb/gadget.h | 8
31 files changed, 66 insertions(+), 56 deletions(-)
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index b8125aa..0444d3f 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -627,7 +627,7 @@ __acquires(hwep->lock)
if (hwreq->req.complete != NULL) {
spin_unlock(hwep->lock);
- hwreq->req.complete(&hwep->ep, &hwreq->req);
+ usb_gadget_giveback_request(&hwep->ep, &hwreq->req);
spin_lock(hwep->lock);
}
}
@@ -922,7 +922,7 @@ __acquires(hwep->lock)
if ((hwep->type == USB_ENDPOINT_XFER_CONTROL) &&
hwreq->req.length)
hweptemp = hwep->ci->ep0in;
- hwreq->req.complete(&hweptemp->ep, &hwreq->req);
+ usb_gadget_giveback_request(&hweptemp->ep, &hwreq->req);
spin_lock(hwep->lock);
}
}
@@ -1347,7 +1347,7 @@ static int ep_dequeue(struct usb_ep *ep, struct
usb_request *req)
if (hwreq->req.complete != NULL) {
spin_unlock(hwep->lock);
- hwreq->req.complete(&hwep->ep, &hwreq->req);
+ usb_gadget_giveback_request(&hwep->ep, &hwreq->req);
spin_lock(hwep->lock);
}
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 0ba9c33..5a524a6 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -987,8 +987,8 @@ static int s3c_hsotg_process_req_feature(struct s3c_hsotg
*hsotg,
hs_req = ep->req;
ep->req = NULL;
list_del_init(&hs_req->queue);
- hs_req->req.complete(&ep->ep,
-&hs_req->req);
+ usb_gadget_giveback_request(&ep->ep,
+
&hs_req->req);
}
/* If we have pending request, then start it */
@@ -1245,7 +1245,7 @@ static void s3c_hsotg_complete_request(struct s3c_hsotg
*hsotg,
if (hs_req->req.complete) {
spin_unlock(&hsotg->lock);
- hs_req->req.complete(&hs_ep->ep, &hs_req->req);
+ usb_gadget_giveback_request(&hs_ep->ep, &hs_req->req);
spin_lock(&hsotg->lock);
}
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 349cacc..b4b7a6b 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -268,7 +268,7 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct
dwc3_request *req,
req->request.length, status);
spin_unlock(&dwc->lock);
-
[PATCH v4 2/3] usb: Rename usb-common.c
In the next commit, we will want the usb-common module to be composed of
two object files. Since Kbuild cannot "append" another object to an
existing one, we need to rename usb-common.c to something
else (common.c) and create usb-common.o by linking the wanted objects
together. Currently, usb-common.o comprises only common.o.
Signed-off-by: Michal Sojka
---
drivers/usb/common/Makefile | 4 +++-
drivers/usb/common/{usb-common.c => common.c} | 0
2 files changed, 3 insertions(+), 1 deletion(-)
rename drivers/usb/common/{usb-common.c => common.c} (100%)
diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
index 7526461..052c120 100644
--- a/drivers/usb/common/Makefile
+++ b/drivers/usb/common/Makefile
@@ -2,5 +2,7 @@
# Makefile for the usb common parts.
#
-obj-$(CONFIG_USB_COMMON) += usb-common.o
+obj-$(CONFIG_USB_COMMON) += usb-common.o
+usb-common-y += common.o
+
obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
diff --git a/drivers/usb/common/usb-common.c b/drivers/usb/common/common.c
similarity index 100%
rename from drivers/usb/common/usb-common.c
rename to drivers/usb/common/common.c
--
2.1.0
--
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 0/7] usb: gadget: add reset API at usb_gadget_driver
On Wed, Aug 27, 2014 at 04:20:25PM -0400, Alan Stern wrote: > On Wed, 27 Aug 2014, Felipe Balbi wrote: > > > > Hi Alan & Felipe, > > > > > > During the changing UDC drivers process, I find we can't simply move > > > usb_gadget_disconnect to usb_gadget_driver.disconnect, some UDC > > > drivers (like dwc3) will be no chance to set software pullup bit > > > (dp control always means software dp pullup bit if no specific at below) > > > again between disconnection and re-connection. > > > > sorry, can you rephrase this a bit, I really can't get what you mean. > > > > DWC3 doesn't really have a pullup bit. It's got a RUN/STOP bit. Sure, it > > controls the pullups but it also kills the internal DMA and quite a bit > > of other internal blocks. > > > > The way the code is today, we will have a pullup connected when a gadget > > driver is loaded and not before that. > > > > > There are two kinds of UDCs, dp is always pullup(UDC-A), pullup dp relies > > > on vbus (UDC-B), so we may need to introduce a flag like pullup_on_vbus > > > at struct usb_gadget, UDC-B needs to set this flag at .udc_start. > > > > > > For the whole gadget life cycle, the dp change for the two kinds of > > > UDC like below: > > > Process dp at UDC-A dp at UDC-B > > > insmod1 0 > > > connect 1 1 > > > bus_reset 1 1 > > > disconnect1 0 > > > rmmod 0 0 > > > > > > For insmod/rmmod gadget driver, we can control dp at udc-core relies > > > on flag pullup_on_vbus. > > > > > > For other three stages (no need to control at bus_reset), we can control > > > dp > > > at two gadget driver's API relies on flag pullup_on_vbus. > > > > I also can't get what you mean here. > > I think Peter is saying that some UDC hardware (which he calls UDC-A) > automatically turns off the pullup when Vbus is absent, whereas other > hardware (UDC-B) relies on software to do this. > > I'm not sure why this matters, however. Does anything go wrong if the > driver tells UDC-A hardware to turn off the pullup when the cable is > unplugged and Vbus goes away? nope, nothing bad should happen. > > > Do you agree above? > > > > > > If you agree, the first patchset for adding reset API at > > > usb_gadget_driver may > > > do less thing, and the reset API implementation at each gadget driver is > > > the > > > same with disconnect. > > > > that's why we never had a ->reset() callback so far. From a gadget > > driver perspective, it's the same as disconnect followed by a connect. > > > > > At the second patchset, we introduce the flag pullup_on_vbus, connect API > > > at usb_gadget_driver, change the disconnect implementation at > > > each gadget driver, and add control dp through function driver. > > > > IIRC, only mass storage gadget was showing a need for a ->reset() > > callback, why would you need to modify every gadget driver ? > > The mass-storage gadget is a function driver, not a gadget driver. > Even g_mass_storage.c is simply a single-function wrapper; it still > relies on the composite layer. > > There are only four gadget drivers in the tree: composite, configfs, > gadgetfs, and dbgp. right, but those are the only ones which should be modified. For all gadget drivers which are composed of function drivers (even single function drivers) they should rely on the function knowing what to do, otherwise we might still mistakenly connect to the host when some userspace daemon isn't ready yet. cheers -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb: gadget: net2280: Remove pci_class from PCI_TABLE
On Wed, Aug 27, 2014 at 09:39:43PM +0200, Ricardo Ribalda Delgado wrote:
> Hello Greg
>
>
>
>
>
> On Wed, Aug 27, 2014 at 9:25 PM, Greg Kroah-Hartman
> wrote:
> > On Wed, Aug 27, 2014 at 03:00:29PM +0200, Ricardo Ribalda Delgado wrote:
> >> Defining the vendor and the product id should be enough to discriminate
> >> the device.
> >>
> >> The reason for this patch is that there is a missmatch betweed the
> >> modalias showed by sysfs and the modalias generated by file2alias.
> >>
> >> One expects the programming interface in uppercase and the other
> >> generates it in lowercase.
> >
> > I don't understand, what is wrong here? Who does it in uppercase and
> > who in lower? And does it matter? It's just a numeric value that
> > should not be used as a string compare.
> >
>
> In pci-sysfs:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/pci-sysfs.c#n175
>
> static ssize_t modalias_show(struct device *dev, struct device_attribute
> *attr,
> char *buf)
> {
> struct pci_dev *pci_dev = to_pci_dev(dev);
>
> return sprintf(buf, "pci:v%08Xd%08Xsv%08Xsd%08Xbc%02Xsc%02Xi%02x\n",
> pci_dev->vendor, pci_dev->device,
> pci_dev->subsystem_vendor, pci_dev->subsystem_device,
> (u8)(pci_dev->class >> 16), (u8)(pci_dev->class >> 8),
> (u8)(pci_dev->class));
> }
>
>
> In file2alias:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/scripts/mod/file2alias.c
>
> #define ADD(str, sep, cond, field) \
> do {\
> strcat(str, sep); \
> if (cond) \
> sprintf(str + strlen(str), \
> sizeof(field) == 1 ? "%02X" : \
> sizeof(field) == 2 ? "%04X" : \
> sizeof(field) == 4 ? "%08X" : "", \
> field); \
> else\
> sprintf(str + strlen(str), "*");\
> } while(0)
>
>
> ADD(alias, "bc", baseclass_mask == 0xFF, baseclass);
> ADD(alias, "sc", subclass_mask == 0xFF, subclass);
> ADD(alias, "i", interface_mask == 0xFF, interface);
>
>
>
> >> This means that some implementations modprobe will fail to load the
> >> driver.
> >
> > What implementations fail to work? Shouldn't we fix the root of the
> > problem and not just patch up all drivers to display incorrect data?
>
> At least the implementation of kmod in yocproject does a case sensitive match.
>
>
> I have already sent a patch to fix what I consider the root of the problem
>
> https://lkml.org/lkml/2014/8/27/242
No, the root cause of the problem is a userspace tool looking at a hex
value as a string and not a number. It doesn't matter if we print it in
upper or lower case, it's a digit, not a string. Do the numeric
compare, not a string compare.
> > And I mean incorrect, as you are changing the values here from being
> > very specific, to being much broader.
> >
>
> Not many drivers define the pci interface and there is no other driver
> that has the same vendor and product id. Therefore I see no hurt in
> adding both patches, one to make the driver broader, and another to
> fix pci-sysfs.
>
> Also, the change on pci-sysfs might affect more stuff and therefore
> take longer to be applied.
As we have been printing the value to userspace in this way for well
over a decade now, and nothing has changed, I say it's a userspace bug
that you should fix instead. Don't work around broken user programs in
the kernel by changing something that has been stable for 10+ years.
Ok, sorry, not 10+ years, the commit was written May of 2005, so 9
years.
Fix your module loading code please.
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: [RFC PATCH 0/7] usb: gadget: add reset API at usb_gadget_driver
On Wed, 27 Aug 2014, Felipe Balbi wrote: > > Hi Alan & Felipe, > > > > During the changing UDC drivers process, I find we can't simply move > > usb_gadget_disconnect to usb_gadget_driver.disconnect, some UDC > > drivers (like dwc3) will be no chance to set software pullup bit > > (dp control always means software dp pullup bit if no specific at below) > > again between disconnection and re-connection. > > sorry, can you rephrase this a bit, I really can't get what you mean. > > DWC3 doesn't really have a pullup bit. It's got a RUN/STOP bit. Sure, it > controls the pullups but it also kills the internal DMA and quite a bit > of other internal blocks. > > The way the code is today, we will have a pullup connected when a gadget > driver is loaded and not before that. > > > There are two kinds of UDCs, dp is always pullup(UDC-A), pullup dp relies > > on vbus (UDC-B), so we may need to introduce a flag like pullup_on_vbus > > at struct usb_gadget, UDC-B needs to set this flag at .udc_start. > > > > For the whole gadget life cycle, the dp change for the two kinds of > > UDC like below: > > Process dp at UDC-A dp at UDC-B > > insmod 1 0 > > connect 1 1 > > bus_reset 1 1 > > disconnect 1 0 > > rmmod 0 0 > > > > For insmod/rmmod gadget driver, we can control dp at udc-core relies > > on flag pullup_on_vbus. > > > > For other three stages (no need to control at bus_reset), we can control dp > > at two gadget driver's API relies on flag pullup_on_vbus. > > I also can't get what you mean here. I think Peter is saying that some UDC hardware (which he calls UDC-A) automatically turns off the pullup when Vbus is absent, whereas other hardware (UDC-B) relies on software to do this. I'm not sure why this matters, however. Does anything go wrong if the driver tells UDC-A hardware to turn off the pullup when the cable is unplugged and Vbus goes away? > > Do you agree above? > > > > If you agree, the first patchset for adding reset API at usb_gadget_driver > > may > > do less thing, and the reset API implementation at each gadget driver is the > > same with disconnect. > > that's why we never had a ->reset() callback so far. From a gadget > driver perspective, it's the same as disconnect followed by a connect. > > > At the second patchset, we introduce the flag pullup_on_vbus, connect API > > at usb_gadget_driver, change the disconnect implementation at > > each gadget driver, and add control dp through function driver. > > IIRC, only mass storage gadget was showing a need for a ->reset() > callback, why would you need to modify every gadget driver ? The mass-storage gadget is a function driver, not a gadget driver. Even g_mass_storage.c is simply a single-function wrapper; it still relies on the composite layer. There are only four gadget drivers in the tree: composite, configfs, gadgetfs, and dbgp. 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 v3 1/2] usb: gadget: Refactor request completion
On Wed, Aug 27, 2014 at 03:03:44PM +0200, Michal Sojka wrote:
> All USB peripheral controller drivers called completion routines
> directly. This patch moves the completion call from drivers to
> usb_gadget_giveback_request(), in order to have a place where common
> functionality can be added.
>
> All places in drivers/usb/ matching "[-.]complete(" were replaced with a
> call to usb_gadget_giveback_request(). This was compile-tested with all
> ARM drivers enabled and runtime-tested for musb.
>
> Signed-off-by: Michal Sojka
> ---
> drivers/usb/chipidea/udc.c | 6 +++---
> drivers/usb/dwc2/gadget.c | 6 +++---
> drivers/usb/dwc3/gadget.c | 2 +-
> drivers/usb/gadget/udc/amd5536udc.c | 2 +-
> drivers/usb/gadget/udc/at91_udc.c | 2 +-
> drivers/usb/gadget/udc/atmel_usba_udc.c | 4 ++--
> drivers/usb/gadget/udc/bcm63xx_udc.c| 2 +-
> drivers/usb/gadget/udc/dummy_hcd.c | 10 +-
> drivers/usb/gadget/udc/fotg210-udc.c| 2 +-
> drivers/usb/gadget/udc/fsl_qe_udc.c | 6 +-
> drivers/usb/gadget/udc/fsl_udc_core.c | 6 ++
> drivers/usb/gadget/udc/fusb300_udc.c| 2 +-
> drivers/usb/gadget/udc/goku_udc.c | 2 +-
> drivers/usb/gadget/udc/gr_udc.c | 2 +-
> drivers/usb/gadget/udc/lpc32xx_udc.c| 2 +-
> drivers/usb/gadget/udc/m66592-udc.c | 2 +-
> drivers/usb/gadget/udc/mv_u3d_core.c| 8 ++--
> drivers/usb/gadget/udc/mv_udc_core.c| 8 ++--
> drivers/usb/gadget/udc/net2272.c| 2 +-
> drivers/usb/gadget/udc/net2280.c| 2 +-
> drivers/usb/gadget/udc/omap_udc.c | 2 +-
> drivers/usb/gadget/udc/pch_udc.c| 2 +-
> drivers/usb/gadget/udc/pxa25x_udc.c | 2 +-
> drivers/usb/gadget/udc/pxa27x_udc.c | 2 +-
> drivers/usb/gadget/udc/r8a66597-udc.c | 2 +-
> drivers/usb/gadget/udc/s3c-hsudc.c | 3 +--
> drivers/usb/gadget/udc/s3c2410_udc.c| 2 +-
> drivers/usb/gadget/udc/udc-core.c | 9 +
> drivers/usb/musb/musb_gadget.c | 2 +-
> drivers/usb/renesas_usbhs/mod_gadget.c | 2 +-
> include/linux/usb/gadget.h | 8
> 31 files changed, 58 insertions(+), 56 deletions(-)
>
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index b8125aa..0444d3f 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -627,7 +627,7 @@ __acquires(hwep->lock)
>
> if (hwreq->req.complete != NULL) {
> spin_unlock(hwep->lock);
> - hwreq->req.complete(&hwep->ep, &hwreq->req);
> + usb_gadget_giveback_request(&hwep->ep, &hwreq->req);
> spin_lock(hwep->lock);
> }
> }
> @@ -922,7 +922,7 @@ __acquires(hwep->lock)
> if ((hwep->type == USB_ENDPOINT_XFER_CONTROL) &&
> hwreq->req.length)
> hweptemp = hwep->ci->ep0in;
> - hwreq->req.complete(&hweptemp->ep, &hwreq->req);
> + usb_gadget_giveback_request(&hweptemp->ep, &hwreq->req);
> spin_lock(hwep->lock);
> }
> }
> @@ -1347,7 +1347,7 @@ static int ep_dequeue(struct usb_ep *ep, struct
> usb_request *req)
>
> if (hwreq->req.complete != NULL) {
> spin_unlock(hwep->lock);
> - hwreq->req.complete(&hwep->ep, &hwreq->req);
> + usb_gadget_giveback_request(&hwep->ep, &hwreq->req);
> spin_lock(hwep->lock);
> }
>
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 0ba9c33..5a524a6 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -987,8 +987,8 @@ static int s3c_hsotg_process_req_feature(struct s3c_hsotg
> *hsotg,
> hs_req = ep->req;
> ep->req = NULL;
> list_del_init(&hs_req->queue);
> - hs_req->req.complete(&ep->ep,
> - &hs_req->req);
> + usb_gadget_giveback_request(&ep->ep,
> +
> &hs_req->req);
> }
>
> /* If we have pending request, then start it */
> @@ -1245,7 +1245,7 @@ static void s3c_hsotg_complete_request(struct s3c_hsotg
> *hsotg,
>
> if (hs_req->req.complete) {
> spin_unlock(&hsotg->lock);
> - hs_req->req.complete(&hs_ep->ep, &hs_req->req);
> + usb_gadget_giveback_request(&hs_ep->ep, &hs_req->req);
> spin_lock(&hsotg->lock);
> }
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 349cacc..b4b7a6b 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/g
Re: [PATCH v3 1/2] usb: gadget: Refactor request completion
On Wed, Aug 27 2014, Michal Sojka wrote:
> All USB peripheral controller drivers called completion routines
> directly. This patch moves the completion call from drivers to
> usb_gadget_giveback_request(), in order to have a place where common
> functionality can be added.
>
> All places in drivers/usb/ matching "[-.]complete(" were replaced with a
> call to usb_gadget_giveback_request(). This was compile-tested with all
> ARM drivers enabled and runtime-tested for musb.
[...]
> diff --git a/drivers/usb/gadget/udc/udc-core.c
> b/drivers/usb/gadget/udc/udc-core.c
> index b0d9817..2eb0ae4 100644
> --- a/drivers/usb/gadget/udc/udc-core.c
> +++ b/drivers/usb/gadget/udc/udc-core.c
> @@ -102,6 +102,15 @@ void usb_gadget_unmap_request(struct usb_gadget *gadget,
> }
> EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
>
> +void usb_gadget_giveback_request(struct usb_ep *ep,
> + struct usb_request *req)
> +{
> + /* complete() is from gadget layer,
> + * eg fsg->bulk_in_complete() */
> + if (req->complete)
> + req->complete(ep, req);
> +}
> +EXPORT_SYMBOL_GPL(usb_gadget_giveback_request);
> #endif /* CONFIG_HAS_DMA */
#endif should be before usb_gadget_giveback_request(). I'll fix this in
v4.
-Michal
--
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: Problem with USB-to-SATA adapters (was: AS2105-based enclosure size issues with >2TB HDDs)
On Wed, 27 Aug 2014, Dale R. Worley wrote: > What I find interesting is that Windows (at least, Windows 7 > Professional) seems to be able to handle the deficient adapter. So does Linux. The difference is that Windows believes the values in the partition table in preference to what the hardware says, whereas Linux believes the hardware in preference to the partition table. Thus, if the hardware says the disk contains 0.8 TB and the partition table says the first partition contains 2.8 TB, Windows will try to access all 2.8 TB but Linux will complain that the partition entry is invalid (because the partition extends beyond the end of the disk). If you try to repartition the drive under Windows using the deficient adapter, you'll see that the problem still exists. It just doesn't show up during normal use. > What > I'd like to do is log the disk commands during the mounting sequence, > preferably at both the SCSI and USB layers. Then at least we'll know > exactly what the driver is doing. Are there kernel options to do > this? You can record the USB transfers by using usbmon (see Documentation/usb/usbmon.txt). The trace will include all the important SCSI data, so you don't need to record anything at the SCSI layer. This isn't really necessary, though. We already know what the driver is doing. > Unfortunately, I don't know any way to log what Windows is doing with > the drive. My experiments with Windows show that it does essentially the same things as Linux does. The important difference is not what commands and data get sent, but whether the OS pays attention to the result. 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: Problem with USB-to-SATA adapters (was: AS2105-based enclosure size issues with >2TB HDDs)
On Wed, 2014-08-27 at 15:21 -0400, Dale R. Worley wrote: > > From: James Bottomley > > > Did you try read capacity 16 on it? What happened? (the AS2105 rejects > > read capacity 16, so there's no reliable way to deduce the capacity of > > drives over 2TB). > > OK, I had to track down which package contains sg_readcap. > > The adapter that fails gives this output: > > # sg_readcap --16 /dev/sdb > bad field in READ CAPACITY (16) cdb including unsupported service action OK, so that's definitely the tame taxonomy. James -- 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: net2280: Remove pci_class from PCI_TABLE
Hello Greg
On Wed, Aug 27, 2014 at 9:25 PM, Greg Kroah-Hartman
wrote:
> On Wed, Aug 27, 2014 at 03:00:29PM +0200, Ricardo Ribalda Delgado wrote:
>> Defining the vendor and the product id should be enough to discriminate
>> the device.
>>
>> The reason for this patch is that there is a missmatch betweed the
>> modalias showed by sysfs and the modalias generated by file2alias.
>>
>> One expects the programming interface in uppercase and the other
>> generates it in lowercase.
>
> I don't understand, what is wrong here? Who does it in uppercase and
> who in lower? And does it matter? It's just a numeric value that
> should not be used as a string compare.
>
In pci-sysfs:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/pci-sysfs.c#n175
static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
return sprintf(buf, "pci:v%08Xd%08Xsv%08Xsd%08Xbc%02Xsc%02Xi%02x\n",
pci_dev->vendor, pci_dev->device,
pci_dev->subsystem_vendor, pci_dev->subsystem_device,
(u8)(pci_dev->class >> 16), (u8)(pci_dev->class >> 8),
(u8)(pci_dev->class));
}
In file2alias:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/scripts/mod/file2alias.c
#define ADD(str, sep, cond, field) \
do {\
strcat(str, sep); \
if (cond) \
sprintf(str + strlen(str), \
sizeof(field) == 1 ? "%02X" : \
sizeof(field) == 2 ? "%04X" : \
sizeof(field) == 4 ? "%08X" : "", \
field); \
else\
sprintf(str + strlen(str), "*");\
} while(0)
ADD(alias, "bc", baseclass_mask == 0xFF, baseclass);
ADD(alias, "sc", subclass_mask == 0xFF, subclass);
ADD(alias, "i", interface_mask == 0xFF, interface);
>> This means that some implementations modprobe will fail to load the
>> driver.
>
> What implementations fail to work? Shouldn't we fix the root of the
> problem and not just patch up all drivers to display incorrect data?
At least the implementation of kmod in yocproject does a case sensitive match.
I have already sent a patch to fix what I consider the root of the problem
https://lkml.org/lkml/2014/8/27/242
>
> And I mean incorrect, as you are changing the values here from being
> very specific, to being much broader.
>
Not many drivers define the pci interface and there is no other driver
that has the same vendor and product id. Therefore I see no hurt in
adding both patches, one to make the driver broader, and another to
fix pci-sysfs.
Also, the change on pci-sysfs might affect more stuff and therefore
take longer to be applied.
> thanks,
Thank you :)
>
> greg k-h
--
Ricardo Ribalda
--
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 0/7] usb: gadget: add reset API at usb_gadget_driver
Hi, On Tue, Aug 26, 2014 at 03:30:32PM +0800, Peter Chen wrote: > On Mon, Aug 25, 2014 at 11:27:47AM -0400, Alan Stern wrote: > > On Mon, 25 Aug 2014, Peter Chen wrote: > > > > > Hi Felipe & Alan, > > > > > > It is the follow-up for: > > > http://www.spinics.net/lists/linux-usb/msg112193.html > > > > > > This patchset adds reset API at usb_gadget_driver, the UDC driver > > > can call it at bus_reset handler instead of calling disconnect. > > > The benefits of this patchset are: > > > - We can let the gadget driver do different things for bus reset. > > > and host is disconnected, eg, whether pull down dp or not. > > > - Match kernel doc for disconnect API, it says it is invoked > > > "when the host is disconnected". > > > - Will be more match for the real things the gadget driver > > > does for connect and disconnect, when we introduce connect API later. > > > > > > The reason for I mark RFC is I don't add the modification for mass > > > UDC driver changes, if you consider this patchset is ok, I will > > > add them without RFC later. > > > > This looks good. > > > > In patches 2, 4, and 5, shouldn't you call usb_gadget_disconnect() > > _before_ the gadget driver's original disconnect handler, instead of > > _after_? That's how we do it now. > > > > Hi Alan & Felipe, > > During the changing UDC drivers process, I find we can't simply move > usb_gadget_disconnect to usb_gadget_driver.disconnect, some UDC > drivers (like dwc3) will be no chance to set software pullup bit > (dp control always means software dp pullup bit if no specific at below) > again between disconnection and re-connection. sorry, can you rephrase this a bit, I really can't get what you mean. DWC3 doesn't really have a pullup bit. It's got a RUN/STOP bit. Sure, it controls the pullups but it also kills the internal DMA and quite a bit of other internal blocks. The way the code is today, we will have a pullup connected when a gadget driver is loaded and not before that. > There are two kinds of UDCs, dp is always pullup(UDC-A), pullup dp relies > on vbus (UDC-B), so we may need to introduce a flag like pullup_on_vbus > at struct usb_gadget, UDC-B needs to set this flag at .udc_start. > > For the whole gadget life cycle, the dp change for the two kinds of > UDC like below: > Process dp at UDC-A dp at UDC-B > insmod1 0 > connect 1 1 > bus_reset 1 1 > disconnect1 0 > rmmod 0 0 > > For insmod/rmmod gadget driver, we can control dp at udc-core relies > on flag pullup_on_vbus. > > For other three stages (no need to control at bus_reset), we can control dp > at two gadget driver's API relies on flag pullup_on_vbus. I also can't get what you mean here. > Do you agree above? > > If you agree, the first patchset for adding reset API at usb_gadget_driver may > do less thing, and the reset API implementation at each gadget driver is the > same with disconnect. that's why we never had a ->reset() callback so far. From a gadget driver perspective, it's the same as disconnect followed by a connect. > At the second patchset, we introduce the flag pullup_on_vbus, connect API > at usb_gadget_driver, change the disconnect implementation at > each gadget driver, and add control dp through function driver. IIRC, only mass storage gadget was showing a need for a ->reset() callback, why would you need to modify every gadget driver ? -- balbi signature.asc Description: Digital signature
Re: [PATCH v3 2/2] usb: Add LED triggers for USB activity
On Wed, Aug 27, 2014 at 12:27:51PM -0700, Greg Kroah-Hartman wrote:
> On Wed, Aug 27, 2014 at 03:03:45PM +0200, Michal Sojka wrote:
> > With this patch, USB activity can be signaled by blinking a LED. There
> > are two triggers, one for activity on USB host and one for USB gadget.
> >
> > Both trigger should work with all host/device controllers. Tested only
> > with musb.
> >
> > Signed-off-by: Michal Sojka
> > ---
> > drivers/usb/Kconfig | 11 ++
> > drivers/usb/common/Makefile | 5 ++-
> > drivers/usb/common/{usb-common.c => common.c} | 0
>
> I don't mind renaming files, but don't "hide" it in this patch, do it in
> a separate patch please, and then base your next patch on this one.
+1
yeah, this prevents us from reviewing the actual changes
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH] usb: dwc3: exynos: remove usb_phy_generic support
Hi,
On Wed, Aug 27, 2014 at 11:42:25PM +0530, Vivek Gautam wrote:
> On Wed, Aug 27, 2014 at 1:22 PM, Bartlomiej Zolnierkiewicz
> wrote:
> > dwc3 driver is using the new Exynos5 SoC series USB DRD PHY driver
> > (PHY_EXYNOS5_USBDRD which selects GENERIC_PHY) as can be seen by
> > looking at the following commits:
> >
> > 7a4cf0fde054 ("ARM: dts: Update DWC3 usb controller to use new
> > phy driver for exynos5250")
> >
> > f070267b5fc1 ("ARM: dts: Enable support for DWC3 controller for
> > exynos5420")
> >
> > Thus remove unused usb_phy_generic support from dwc3 Exynos glue
> > layer.
> >
> > [ The code that is being removed is harmful in the context of
> > multi_v7_defconfig and enabling "EHCI support for Samsung
> > S5P/EXYNOS SoC Series" (USB_EHCI_EXYNOS) + "OHCI support for
> > Samsung S5P/EXYNOS SoC Series" (USB_OHCI_EXYNOS) because "EHCI
> > support for OMAP3 and later chips" (USB_EHCI_HCD_OMAP) selects
> > "NOP USB Transceiver Driver" (NOP_USB_XCEIV). NOP USB driver
> > attaches itself to usb_phy_generic platform devices created by
> > dwc3 Exynos glue layer and later causes Exynos EHCI driver to
> > fail probe and Exynos OHCI driver to hang on probe (as observed
> > on Exynos5250 Arndale board). ]
>
> The issue with EHCI and OHCI on exynos platforms is that until now
> they also request
> usb-phy and only later if they don't find one, they go ahead and get a
> 'phy' generic.
>
> Fortunately we missed this issue with exynos_defconfig, and as you rightly
> mentioned with multi_v7_defconfig, the NOP_USB_XCEIV gets enabled and
> EHCI and OHCI exynos get no-op usb-phy, which actually blocks EHCI/OHCI from
> initializing the real PHYs.
right, when I added PHY support to dwc3 I expected people to remote NOP
transceivers from their glue layers after they had proper PHY drivers
available.
> This issue is resolved with patches:
> [PATCH v2 1/2] usb: host: ehci-exynos: Remove unnecessary usb-phy support
> [PATCH v2 2/2] usb: host: ohci-exynos: Remove unnecessary usb-phy support
> wherein we are completely removing the usb-phy support from ehci/ohci-exynos.
> So with these patches we should not see the issue mentioned by you.
>
> Now for the DWC3 part, the usb-phy part was added to use register two
> separate no-op-xceivers of USB2_PHY type and USB3_PHY type,
> so that platforms with no separate PHY can still be able to use dwc3.
correct.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v3 2/2] usb: Add LED triggers for USB activity
On Wed, Aug 27, 2014 at 03:03:45PM +0200, Michal Sojka wrote:
> With this patch, USB activity can be signaled by blinking a LED. There
> are two triggers, one for activity on USB host and one for USB gadget.
>
> Both trigger should work with all host/device controllers. Tested only
> with musb.
>
> Signed-off-by: Michal Sojka
> ---
> drivers/usb/Kconfig | 11 ++
> drivers/usb/common/Makefile | 5 ++-
> drivers/usb/common/{usb-common.c => common.c} | 0
I don't mind renaming files, but don't "hide" it in this patch, do it in
a separate patch please, and then base your next patch on this one.
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 v2 2/9] mailbox: Add NVIDIA Tegra XUSB mailbox driver
On 27 August 2014 23:08, Andrew Bresticker wrote: > On Mon, Aug 25, 2014 at 12:01 PM, Stephen Warren > wrote: >> I'm not even sure if it's appropriate for the low-level mailbox driver to >> know about the semantics of the message, rather than simply sending them on >> to the client driver? Perhaps when drivers register(?) for callbacks(?) for >> messages, they should state which types of messages they want to listen to? > > So there's not really a way for the client driver to tell the mailbox > driver which types of messages it wants to listen to on a particular > channel with the mailbox framework - it simply provides a way for > clients to bind with channels. I think there are a couple of options > here, either: a) have a channel per message (as you mentioned in the > previous patch), which allows the client to only register for messages > (channels) it wants to handle, or b) extend the mailbox framework to > allow shared channels so that both clients can receive messages on the > single channel and handle messages appropriately. The disadvantage > of (a) is that the commands are firmware defined and could > theoretically change between releases of the firmware, though I'm not > sure how common that is in practice. So that leaves (b) - Jassi, what > do you think about having shared (non-exclusive) channels? > n++ ... 'mailbox' is one such device that imbibes properties of local controller hardware and the remote firmware. Change in remote f/w's behavior might require the controller driver to change besides our client driver. A typical example is format of payloads (if involved) a client and mailbox controller driver have direct understanding of the packet format ... which is likely to change with remote f/w. So if the original concern "why are we doing s/w protocol stuff in controller driver?" won't go away by providing for shared channels (which would have its own tradeoffs). BTW, on DMAEngine we are moving towards virtual channels backed by limited physical ones ... your setup looks quite similar. So your demuxer doesn't hurt my eyes at all. -jassi -- 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: net2280: Remove pci_class from PCI_TABLE
On Wed, Aug 27, 2014 at 03:00:29PM +0200, Ricardo Ribalda Delgado wrote: > Defining the vendor and the product id should be enough to discriminate > the device. > > The reason for this patch is that there is a missmatch betweed the > modalias showed by sysfs and the modalias generated by file2alias. > > One expects the programming interface in uppercase and the other > generates it in lowercase. I don't understand, what is wrong here? Who does it in uppercase and who in lower? And does it matter? It's just a numeric value that should not be used as a string compare. > This means that some implementations modprobe will fail to load the > driver. What implementations fail to work? Shouldn't we fix the root of the problem and not just patch up all drivers to display incorrect data? And I mean incorrect, as you are changing the values here from being very specific, to being much broader. 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 v2 5/5] usb: phy: samsung: remove old common USB PHY code
Hi, On Fri, Aug 22, 2014 at 06:59:05PM +0200, Bartlomiej Zolnierkiewicz wrote: > drivers/usb/phy/phy-samsung-usb[2,3] drivers got replaced by > drivers/phy/phy-samsung-usb[2,3] ones and the old common Samsung > USB PHY code is no longer used. > > Signed-off-by: Bartlomiej Zolnierkiewicz > Acked-by: Kyungmin Park > Reviewed-by: Vivek Gautam > Reviewed-by: Jingoo Han > Acked-by: Kishon Vijay Abraham I > Cc: Kamil Debski this patch doesn't apply. Please rebase on my testing/next -- balbi signature.asc Description: Digital signature
Re: [PATCH v2] usb: hub: Prevent hub autosuspend if usbcore.autosuspend is -1
On Wed, Aug 27, 2014 at 03:11:10PM +0300, Roger Quadros wrote: > If user specifies that USB autosuspend must be disabled by module > parameter "usbcore.autosuspend=-1" then we must prevent > autosuspend of USB hub devices as well. > > commit 596d789a211d introduced in v3.8 changed the original behaivour > and stopped respecting the usbcore.autosuspend parameter for hubs. > > Fixes: 596d789a211d "USB: set hub's default autosuspend delay as 0" > > Cc: [3.8+] > Signed-off-by: Roger Quadros > --- > drivers/usb/core/hub.c | 8 +++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > index 8a4dcbc..59b599d 100644 > --- a/drivers/usb/core/hub.c > +++ b/drivers/usb/core/hub.c > @@ -1728,8 +1728,14 @@ static int hub_probe(struct usb_interface *intf, const > struct usb_device_id *id) >* - Change autosuspend delay of hub can avoid unnecessary auto >* suspend timer for hub, also may decrease power consumption >* of USB bus. > + * > + * - If user has indicated to prevent autosuspend by passing > + * usbcore.autosuspend = -1 then keep autosuspend disabled. >*/ > - pm_runtime_set_autosuspend_delay(&hdev->dev, 0); > +#ifdef CONFIG_PM_RUNTIME > + if (hdev->dev.power.autosuspend_delay >= 0) > + pm_runtime_set_autosuspend_delay(&hdev->dev, 0); > +#endif > > /* >* Hubs have proper suspend/resume support, except for root hubs Sorry, but can I have a patch that just adds the #ifdef as I already have your original one in my tree. 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: Problem with USB-to-SATA adapters (was: AS2105-based enclosure size issues with >2TB HDDs)
What I find interesting is that Windows (at least, Windows 7 Professional) seems to be able to handle the deficient adapter. What I'd like to do is log the disk commands during the mounting sequence, preferably at both the SCSI and USB layers. Then at least we'll know exactly what the driver is doing. Are there kernel options to do this? Unfortunately, I don't know any way to log what Windows is doing with the drive. Dale -- 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/5] usb: phy: samsung: remove old USB PHY code
Hi, On Fri, Aug 22, 2014 at 06:59:00PM +0200, Bartlomiej Zolnierkiewicz wrote: > Hi, > > This patch series removes the old Samsung USB PHY drivers that > got replaced by the new ones using the generic PHY layer. > > Depends on: > - v3.17-rc1 branch of Linus' kernel > > Changes since v1 (https://lkml.org/lkml/2014/8/14/241): > - rebased on v3.17-rc1 kernel > - added Acked-by-s from Kishon Vijay Abraham I > - added Reviewed-by-s from Vivek Gautam and Jingoo Han So, everybody agrees that there will be no regressions, right ? I'll merge this on my testing/next. -- balbi signature.asc Description: Digital signature
Re: [PATCH 02/13] usb: udc: set the udc is ready to pullup dp when it needs
On Wed, Aug 20, 2014 at 01:30:40PM +0800, Peter Chen wrote: > On Tue, Aug 19, 2014 at 09:02:54PM +, Paul Zimmerman wrote: > > > From: linux-usb-ow...@vger.kernel.org > > > [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Peter Chen > > > Sent: Tuesday, August 19, 2014 7:26 AM > > > > > > On Tue, Aug 19, 2014 at 01:46:17AM +, Paul Zimmerman wrote: > > > > > From: Peter Chen [mailto:peter.c...@freescale.com] > > > > > Sent: Sunday, August 17, 2014 9:14 PM > > > > > > > > > > Except for chipidea driver, all other udc drivers will tell the > > > > > gadget driver that they are ready to pullup dp at udc_start, it > > > > > is the default behaviour. > > > > > > > > > > The chipidea driver is ready to pullup dp when the vbus is there, > > > > > and isn't ready to pullup dp when the vbus is not there. Other > > > > > udc drivers which should not pull up when vbus is not there should > > > > > change like chipidea does to pass the Back-Voltage Testing at > > > > > www.usb.org/developers/docs/USB-IFTestProc1_3.pdf. > > > > > > > > > > Signed-off-by: Peter Chen > > > > > --- > > > > > drivers/usb/chipidea/udc.c |9 - > > > > > drivers/usb/dwc2/gadget.c |2 ++ > > > > > drivers/usb/dwc3/gadget.c |2 ++ > > > > > drivers/usb/gadget/udc/amd5536udc.c |1 + > > > > > drivers/usb/gadget/udc/atmel_usba_udc.c |2 ++ > > > > > drivers/usb/gadget/udc/bcm63xx_udc.c|2 ++ > > > > > drivers/usb/gadget/udc/dummy_hcd.c |1 + > > > > > drivers/usb/gadget/udc/fotg210-udc.c|1 + > > > > > drivers/usb/gadget/udc/fsl_qe_udc.c |1 + > > > > > drivers/usb/gadget/udc/fsl_udc_core.c |2 ++ > > > > > drivers/usb/gadget/udc/fusb300_udc.c|1 + > > > > > drivers/usb/gadget/udc/gr_udc.c |2 ++ > > > > > drivers/usb/gadget/udc/lpc32xx_udc.c|2 ++ > > > > > drivers/usb/gadget/udc/m66592-udc.c |2 ++ > > > > > drivers/usb/gadget/udc/mv_u3d_core.c|1 + > > > > > drivers/usb/gadget/udc/mv_udc_core.c|2 ++ > > > > > drivers/usb/gadget/udc/net2272.c|1 + > > > > > drivers/usb/gadget/udc/net2280.c|1 + > > > > > drivers/usb/gadget/udc/omap_udc.c |1 + > > > > > drivers/usb/gadget/udc/pch_udc.c|1 + > > > > > drivers/usb/gadget/udc/pxa25x_udc.c |1 + > > > > > drivers/usb/gadget/udc/pxa27x_udc.c |1 + > > > > > drivers/usb/gadget/udc/r8a66597-udc.c |1 + > > > > > drivers/usb/gadget/udc/s3c-hsudc.c |1 + > > > > > drivers/usb/gadget/udc/s3c2410_udc.c|1 + > > > > > drivers/usb/musb/musb_gadget.c |2 ++ > > > > > drivers/usb/renesas_usbhs/mod_gadget.c |7 ++- > > > > > 27 files changed, 45 insertions(+), 6 deletions(-) > > > > > > > > Instead of modifying all of the UDC drivers, how about adding a flag to > > > > 'struct usb_gadget' named 'needs_ready' or similar? If the UDC doesn't > > > > set the flag, udc-core would call usb_udc_ready_to_connect() on behalf > > > > of the UDC. If the UDC sets the flag (chipidea only?) then the UDC would > > > > be responsible for calling usb_udc_ready_to_connect(). > > > > > > > > > > USB spec requires dp is not pulled up when the vbus is not there, the > > > dwc3 is > > > the newest core, I don't think other older udc cores all has similar > > > capability > > > that does don't draw back voltage if software pullup bit is set and vbus > > > is > > > not there. > > > > > > This patchset will delete the unconditional pullup dp operation at > > > udc-core, > > > and we need to pullup dp at the end of hardware initialization (not > > > consider > > > vbus case), then the end of .udc_start at udc driver is the old place. > > > > I think you misunderstood my suggestion. Since you are adding a call > > to usb_udc_ready_to_connect() at the end of almost every .udc_start > > function, why not have udc-core do it instead, immediately after the > > call to .udc_start? Unless the 'needs_ready' flag is set, which would > > only be set by the udc drivers for those controllers that need it. > > > > Thanks. > > Yes, we can do that, my original plan is > usb_gadget_connect/usb_gadget_disconnect > are only called by gadget driver. If Felipe has no comment for it, I will not directly though. At least not for those using the composite layer. All functions using composite layer should use activate/deactivate. -- balbi signature.asc Description: Digital signature
Re: Problem with USB-to-SATA adapters (was: AS2105-based enclosure size issues with >2TB HDDs)
> From: James Bottomley > Did you try read capacity 16 on it? What happened? (the AS2105 rejects > read capacity 16, so there's no reliable way to deduce the capacity of > drives over 2TB). OK, I had to track down which package contains sg_readcap. The adapter that fails gives this output: # sg_readcap --16 /dev/sdb bad field in READ CAPACITY (16) cdb including unsupported service action The adapter that succeeds gives this output: # sg_readcap --16 /dev/sdc Read Capacity results: Protection: prot_en=0, p_type=0, p_i_exponent=0 Logical block provisioning: lbpme=0, lbprz=0 Last logical block address=5860533167 (0x15d50a3af), Number of logical blocks=5860533168 Logical block length=512 bytes Logical blocks per physical block exponent=0 Lowest aligned logical block address=0 Hence: Device size: 3000592982016 bytes, 2861588.5 MiB, 3000.59 GB Dale -- 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: hub: Prevent hub autosuspend if usbcore.autosuspend is -1
On Wed, Aug 27, 2014 at 01:31:24PM +0300, Roger Quadros wrote: > On 08/04/2014 05:07 PM, Alan Stern wrote: > > On Mon, 4 Aug 2014, Roger Quadros wrote: > > > >> If user specifies that USB autosuspend must be disabled by module > >> parameter "usbcore.autosuspend=-1" then we must prevent > >> autosuspend of USB hub devices as well. > >> > >> commit 596d789a211d introduced in v3.8 changed the original behaivour > >> and stopped respecting the usbcore.autosuspend parameter for hubs. > >> > >> Fixes: 596d789a211d "USB: set hub's default autosuspend delay as 0" > >> > >> Cc: [3.8+] > >> Signed-off-by: Roger Quadros > > > > Acked-by: Alan Stern > > > > Just noticed the below error by the Intel's build script > > > tree: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git > > usb-linus > > head: 039368901ad0a6476c7ecf0cfe4f84d735e30135 > > commit: bdd405d2a5287bdb9b04670ea255e1f122138e66 [19/20] usb: hub: Prevent > > hub autosuspend if usbcore.autosuspend is -1 > > config: make ARCH=sparc64 defconfig > > > > All error/warnings: > > > >drivers/usb/core/hub.c: In function 'hub_probe': > >>> drivers/usb/core/hub.c:1735:21: error: 'struct dev_pm_info' has no member > >>> named 'autosuspend_delay' > > if (hdev->dev.power.autosuspend_delay >= 0) > > ^ > > Seems like the "if" has to be placed within a #ifdef CONFIG_PM_RUNTIME. > > Greg, > > Should I send a v2 of this patch or a fix on top of your usb-linus branch? Please send a fix-on-top, as I already have your patch applied, and can't remove it. 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 v6 2/3] usb: gadget: f_fs: add ioctl returning ep descriptor
Hi,
On Mon, Aug 25, 2014 at 11:16:28AM +0200, Robert Baldyga wrote:
> This patch introduces ioctl named FUNCTIONFS_ENDPOINT_DESC, which
> returns endpoint descriptor to userspace. It works only if function
> is active.
>
> Signed-off-by: Robert Baldyga
> Acked-by: Michal Nazarewicz
> ---
> drivers/usb/gadget/function/f_fs.c | 23 +++
> include/uapi/linux/usb/functionfs.h | 6 ++
> 2 files changed, 29 insertions(+)
>
> diff --git a/drivers/usb/gadget/function/f_fs.c
> b/drivers/usb/gadget/function/f_fs.c
> index 0dc3552d..6edf7e4 100644
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -1032,6 +1032,29 @@ static long ffs_epfile_ioctl(struct file *file,
> unsigned code,
> case FUNCTIONFS_ENDPOINT_REVMAP:
> ret = epfile->ep->num;
> break;
> + case FUNCTIONFS_ENDPOINT_DESC:
> + {
> + int desc_idx;
> + struct usb_endpoint_descriptor *desc;
> +
> + switch (epfile->ffs->gadget->speed) {
> + case USB_SPEED_SUPER:
> + desc_idx = 2;
> + break;
> + case USB_SPEED_HIGH:
> + desc_idx = 1;
> + break;
> + default:
> + desc_idx = 0;
> + }
> + desc = epfile->ep->descs[desc_idx];
> +
> + spin_unlock_irq(&epfile->ffs->eps_lock);
> + ret = copy_to_user((void *)value, desc, sizeof(*desc));
> + if (ret)
> + ret = -EFAULT;
> + return;
should return a value here. Have you tested this at all ?
--
balbi
signature.asc
Description: Digital signature
Re: [RFC 2/2] usb: otg: Temporarily hold wakeupsource on charger and disconnect events
On Fri, Aug 22, 2014 at 03:20:00PM +0530, Kiran Kumar Raparthy wrote: > From: Todd Poynor > > usb: otg: Temporarily hold wakeupsource on charger and disconnect events > > Allow other parts of the system to react to the charger connect/disconnect > event without allowing the system to suspend before the other parts can > process > the event. This wakeup_source times out after 2 seconds; if nobody else holds > a > wakeup_source by that time then the device can sleep. > > This patch also refactoras the logic of wakeupsource_init,otg_notifications > and > handle event funtions refactoring should be in another patch. > This is one of the number of patches from the Android AOSP tegra.git tree, > which is used on Android devices. so I wanted to submit it for > review to see if it should go upstream. this paragraph doesn't need to be in commit log. -- balbi signature.asc Description: Digital signature
Re: [PATCH 2/2] doc: dt: mxs-phy: add compatible string for imx6sx-usbphy
On Tue, Aug 26, 2014 at 10:55:18AM +0800, Peter Chen wrote: > Add compatible string for imx6sx-usbphy. > > Signed-off-by: Peter Chen do I need to take this one as well or will DT folks take care of patch 2 ? -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb: gadget: composite: dequeue cdev->req before free its buffer
Hi, On Mon, Aug 25, 2014 at 04:49:12PM +0800, Li Jun wrote: > As Felipe suggested, dequeue the cdev->req before free its buffer. > > Suggested-by: Felipe Balbi > Signed-off-by: Li Jun please read Documentation/SubmittingPatches. Your commit log does not make sense. You need to explain why the change is necessary and what is it fixing. -- balbi signature.asc Description: Digital signature
Re: [PATCH v2 2/9] mailbox: Add NVIDIA Tegra XUSB mailbox driver
On 08/27/2014 12:13 PM, Andrew Bresticker wrote: On Wed, Aug 27, 2014 at 10:50 AM, Stephen Warren wrote: On 08/27/2014 11:38 AM, Andrew Bresticker wrote: On Mon, Aug 25, 2014 at 12:01 PM, Stephen Warren wrote: On 08/18/2014 11:08 AM, Andrew Bresticker wrote: +static int tegra_xusb_mbox_probe(struct platform_device *pdev) + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) + return -ENODEV; Should devm_request_mem_region() be called here to claim the region? No, the xHCI host driver also needs to map these registers, so they cannot be mapped exclusively here. That's unfortunate. Having multiple drivers with overlapping register regions is not a good idea. Can we instead have a top-level driver map all the IO regions, then instantiate the various different sub-components internally, and divide up the address space. Probably via MFD or similar. That would prevent multiple drivers from touching the same register region. Perhaps I'm misunderstanding, but I don't see how MFD would prevent us from having to map this register space in two different locations - the XUSB FPCI address space cannot be divided cleanly between host and mailbox registers. Or are you saying that there should be a separate device driver that exposes an API for accessing this register space, like the Tegra fuse or PMC drivers? With MFD, there's typically a top-level driver for the HW module (or register space) that gets instantiated by the DT node. This driver then instantiates all the different sub-drivers that use that register space, and provides APIs for the sub-drivers to access the registers (either custom APIs or more recently by passing a regmap object down to the sub-drivers). This top-level driver is the only driver that maps the space, and can manage sharing the space between the various sub-drivers. That said, I haven't noticed many MFD drivers for MMIO devices. I certainly have seen multiple different drivers just re-mapping shared registers for themselves. It is simpler and does work. However, people usually mutter about it when it happens, since it's not clear which drivers are using what from the IO mapping registry. Using MFD or similar would allow the sharing to work in a clean fashion. -- 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: dwc3: exynos: remove usb_phy_generic support
Hi Baltlomiej,
On Wed, Aug 27, 2014 at 1:22 PM, Bartlomiej Zolnierkiewicz
wrote:
> dwc3 driver is using the new Exynos5 SoC series USB DRD PHY driver
> (PHY_EXYNOS5_USBDRD which selects GENERIC_PHY) as can be seen by
> looking at the following commits:
>
> 7a4cf0fde054 ("ARM: dts: Update DWC3 usb controller to use new
> phy driver for exynos5250")
>
> f070267b5fc1 ("ARM: dts: Enable support for DWC3 controller for
> exynos5420")
>
> Thus remove unused usb_phy_generic support from dwc3 Exynos glue
> layer.
>
> [ The code that is being removed is harmful in the context of
> multi_v7_defconfig and enabling "EHCI support for Samsung
> S5P/EXYNOS SoC Series" (USB_EHCI_EXYNOS) + "OHCI support for
> Samsung S5P/EXYNOS SoC Series" (USB_OHCI_EXYNOS) because "EHCI
> support for OMAP3 and later chips" (USB_EHCI_HCD_OMAP) selects
> "NOP USB Transceiver Driver" (NOP_USB_XCEIV). NOP USB driver
> attaches itself to usb_phy_generic platform devices created by
> dwc3 Exynos glue layer and later causes Exynos EHCI driver to
> fail probe and Exynos OHCI driver to hang on probe (as observed
> on Exynos5250 Arndale board). ]
The issue with EHCI and OHCI on exynos platforms is that until now
they also request
usb-phy and only later if they don't find one, they go ahead and get a
'phy' generic.
Fortunately we missed this issue with exynos_defconfig, and as you rightly
mentioned with multi_v7_defconfig, the NOP_USB_XCEIV gets enabled and
EHCI and OHCI exynos get no-op usb-phy, which actually blocks EHCI/OHCI from
initializing the real PHYs.
This issue is resolved with patches:
[PATCH v2 1/2] usb: host: ehci-exynos: Remove unnecessary usb-phy support
[PATCH v2 2/2] usb: host: ohci-exynos: Remove unnecessary usb-phy support
wherein we are completely removing the usb-phy support from ehci/ohci-exynos.
So with these patches we should not see the issue mentioned by you.
Now for the DWC3 part, the usb-phy part was added to use register two
separate no-op-xceivers of USB2_PHY type and USB3_PHY type,
so that platforms with no separate PHY can still be able to use dwc3.
>
> Cc: Olof Johansson
> Cc: Kukjin Kim
> Cc: Vivek Gautam
> Acked-by: Kyungmin Park
> Signed-off-by: Bartlomiej Zolnierkiewicz
> ---
> drivers/usb/dwc3/dwc3-exynos.c | 68
> -
> 1 file changed, 1 insertion(+), 67 deletions(-)
>
> Index: b/drivers/usb/dwc3/dwc3-exynos.c
> ===
> --- a/drivers/usb/dwc3/dwc3-exynos.c2014-08-25 14:57:04.991781925 +0200
> +++ b/drivers/usb/dwc3/dwc3-exynos.c2014-08-27 09:16:38.312617727 +0200
> @@ -23,15 +23,12 @@
> #include
> #include
> #include
> -#include
> -#include
> +#include
> #include
> #include
> #include
>
> struct dwc3_exynos {
> - struct platform_device *usb2_phy;
> - struct platform_device *usb3_phy;
> struct device *dev;
>
> struct clk *clk;
> @@ -39,61 +36,6 @@ struct dwc3_exynos {
> struct regulator*vdd10;
> };
>
> -static int dwc3_exynos_register_phys(struct dwc3_exynos *exynos)
> -{
> - struct usb_phy_generic_platform_data pdata;
> - struct platform_device *pdev;
> - int ret;
> -
> - memset(&pdata, 0x00, sizeof(pdata));
> -
> - pdev = platform_device_alloc("usb_phy_generic", PLATFORM_DEVID_AUTO);
> - if (!pdev)
> - return -ENOMEM;
> -
> - exynos->usb2_phy = pdev;
> - pdata.type = USB_PHY_TYPE_USB2;
> - pdata.gpio_reset = -1;
> -
> - ret = platform_device_add_data(exynos->usb2_phy, &pdata,
> sizeof(pdata));
> - if (ret)
> - goto err1;
> -
> - pdev = platform_device_alloc("usb_phy_generic", PLATFORM_DEVID_AUTO);
> - if (!pdev) {
> - ret = -ENOMEM;
> - goto err1;
> - }
> -
> - exynos->usb3_phy = pdev;
> - pdata.type = USB_PHY_TYPE_USB3;
> -
> - ret = platform_device_add_data(exynos->usb3_phy, &pdata,
> sizeof(pdata));
> - if (ret)
> - goto err2;
> -
> - ret = platform_device_add(exynos->usb2_phy);
> - if (ret)
> - goto err2;
> -
> - ret = platform_device_add(exynos->usb3_phy);
> - if (ret)
> - goto err3;
> -
> - return 0;
> -
> -err3:
> - platform_device_del(exynos->usb2_phy);
> -
> -err2:
> - platform_device_put(exynos->usb3_phy);
> -
> -err1:
> - platform_device_put(exynos->usb2_phy);
> -
> - return ret;
> -}
> -
> static int dwc3_exynos_remove_child(struct device *dev, void *unused)
> {
> struct platform_device *pdev = to_platform_device(dev);
> @@ -127,12 +69,6 @@ static int dwc3_exynos_probe(struct plat
>
> platform_set_drvdata(pdev, exynos);
>
> - ret = dwc3_exynos_register_phys(exynos);
> - if (ret) {
> - dev_err(dev, "couldn't register PHYs\n");
> -
Re: [PATCH v2 2/9] mailbox: Add NVIDIA Tegra XUSB mailbox driver
On Wed, Aug 27, 2014 at 10:50 AM, Stephen Warren wrote: > On 08/27/2014 11:38 AM, Andrew Bresticker wrote: >> >> On Mon, Aug 25, 2014 at 12:01 PM, Stephen Warren >> wrote: >>> >>> On 08/18/2014 11:08 AM, Andrew Bresticker wrote: +static int tegra_xusb_mbox_probe(struct platform_device *pdev) >>> >>> >>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) + return -ENODEV; >>> >>> >>> >>> Should devm_request_mem_region() be called here to claim the region? >> >> >> No, the xHCI host driver also needs to map these registers, so they >> cannot be mapped exclusively here. > > > That's unfortunate. Having multiple drivers with overlapping register > regions is not a good idea. Can we instead have a top-level driver map all > the IO regions, then instantiate the various different sub-components > internally, and divide up the address space. Probably via MFD or similar. > That would prevent multiple drivers from touching the same register region. Perhaps I'm misunderstanding, but I don't see how MFD would prevent us from having to map this register space in two different locations - the XUSB FPCI address space cannot be divided cleanly between host and mailbox registers. Or are you saying that there should be a separate device driver that exposes an API for accessing this register space, like the Tegra fuse or PMC drivers? -- 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/9] mailbox: Add NVIDIA Tegra XUSB mailbox driver
On 08/27/2014 11:38 AM, Andrew Bresticker wrote:
On Mon, Aug 25, 2014 at 12:01 PM, Stephen Warren wrote:
On 08/18/2014 11:08 AM, Andrew Bresticker wrote:
The Tegra xHCI controller's firmware communicates requests to the host
processor through a mailbox interface. While there is only a single
communication channel, messages sent by the controller can be divided
into two groups: those intended for the PHY driver and those intended
for the host-controller driver. This mailbox driver exposes the two
channels and routes incoming messages to the appropriate channel based
on the command encoded in the message.
diff --git a/drivers/mailbox/tegra-xusb-mailbox.c
b/drivers/mailbox/tegra-xusb-mailbox.c
+static struct mbox_chan *mbox_cmd_to_chan(struct tegra_xusb_mbox *mbox,
u32 cmd)
+{
+ switch (cmd) {
+ case MBOX_CMD_INC_FALC_CLOCK:
+ case MBOX_CMD_DEC_FALC_CLOCK:
+ case MBOX_CMD_INC_SSPI_CLOCK:
+ case MBOX_CMD_DEC_SSPI_CLOCK:
+ case MBOX_CMD_SET_BW:
+ return &mbox->mbox.chans[TEGRA_XUSB_MBOX_CHAN_HOST];
+ case MBOX_CMD_SAVE_DFE_CTLE_CTX:
+ case MBOX_CMD_START_HSIC_IDLE:
+ case MBOX_CMD_STOP_HSIC_IDLE:
+ return &mbox->mbox.chans[TEGRA_XUSB_MBOX_CHAN_PHY];
+ default:
+ return NULL;
+ }
+}
This makes me think that the CHAN_HOST/CHAN_PHY values are purely a facet of
the Linux driver's message de-multiplexing, rather than anything to do with
the HW.
Yup, they are...
I'm not even sure if it's appropriate for the low-level mailbox driver to
know about the semantics of the message, rather than simply sending them on
to the client driver? Perhaps when drivers register(?) for callbacks(?) for
messages, they should state which types of messages they want to listen to?
So there's not really a way for the client driver to tell the mailbox
driver which types of messages it wants to listen to on a particular
channel with the mailbox framework - it simply provides a way for
clients to bind with channels. I think there are a couple of options
here, either: a) have a channel per message (as you mentioned in the
previous patch), which allows the client to only register for messages
(channels) it wants to handle, or b) extend the mailbox framework to
allow shared channels so that both clients can receive messages on the
single channel and handle messages appropriately. The disadvantage
of (a) is that the commands are firmware defined and could
theoretically change between releases of the firmware, though I'm not
sure how common that is in practice. So that leaves (b) - Jassi, what
do you think about having shared (non-exclusive) channels?
Another alternative might be for each client driver to hard-code a
unique dummy channel ID so that each client still gets a separate
exclusive channel, but then have the mbox driver broadcast each message
to each of those channels. I'm not sure that would be any better though;
adding (b) as an explicit option to the mbox subsystem would almost
certainly be cleaner.
+static int tegra_xusb_mbox_probe(struct platform_device *pdev)
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res)
+ return -ENODEV;
Should devm_request_mem_region() be called here to claim the region?
No, the xHCI host driver also needs to map these registers, so they
cannot be mapped exclusively here.
That's unfortunate. Having multiple drivers with overlapping register
regions is not a good idea. Can we instead have a top-level driver map
all the IO regions, then instantiate the various different
sub-components internally, and divide up the address space. Probably via
MFD or similar. That would prevent multiple drivers from touching the
same register region.
--
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/9] mailbox: Add NVIDIA Tegra XUSB mailbox driver
On Mon, Aug 25, 2014 at 12:01 PM, Stephen Warren wrote:
> On 08/18/2014 11:08 AM, Andrew Bresticker wrote:
>>
>> The Tegra xHCI controller's firmware communicates requests to the host
>> processor through a mailbox interface. While there is only a single
>> communication channel, messages sent by the controller can be divided
>> into two groups: those intended for the PHY driver and those intended
>> for the host-controller driver. This mailbox driver exposes the two
>> channels and routes incoming messages to the appropriate channel based
>> on the command encoded in the message.
>
>
>> diff --git a/drivers/mailbox/tegra-xusb-mailbox.c
>> b/drivers/mailbox/tegra-xusb-mailbox.c
>
>
>> +#define XUSB_CFG_ARU_MBOX_CMD 0xe4
>> +#define MBOX_FALC_INT_EN BIT(27)
>> +#define MBOX_PME_INT_EN BIT(28)
>> +#define MBOX_SMI_INT_EN BIT(29)
>> +#define MBOX_XHCI_INT_EN BIT(30)
>> +#define MBOX_INT_EN BIT(31)
>
>
> Those field names don't match the documentation in the TRM; they're called
> DEST_xxx rather than xxx_INT_EN. I'm not sure what that disconnect means
> (i.e. whether it's just a different naming choice, or there's some practical
> disconnect that will cause issues.)
Hmm... interestingly *_INT_EN is the convention the downstream kernels
used. DEST_* is definitely more accurate as I'm pretty sure these
bits select the destination for the interrupt.
>> +static struct mbox_chan *mbox_cmd_to_chan(struct tegra_xusb_mbox *mbox,
>> u32 cmd)
>> +{
>> + switch (cmd) {
>> + case MBOX_CMD_INC_FALC_CLOCK:
>> + case MBOX_CMD_DEC_FALC_CLOCK:
>> + case MBOX_CMD_INC_SSPI_CLOCK:
>> + case MBOX_CMD_DEC_SSPI_CLOCK:
>> + case MBOX_CMD_SET_BW:
>> + return &mbox->mbox.chans[TEGRA_XUSB_MBOX_CHAN_HOST];
>> + case MBOX_CMD_SAVE_DFE_CTLE_CTX:
>> + case MBOX_CMD_START_HSIC_IDLE:
>> + case MBOX_CMD_STOP_HSIC_IDLE:
>> + return &mbox->mbox.chans[TEGRA_XUSB_MBOX_CHAN_PHY];
>> + default:
>> + return NULL;
>> + }
>> +}
>
>
> This makes me think that the CHAN_HOST/CHAN_PHY values are purely a facet of
> the Linux driver's message de-multiplexing, rather than anything to do with
> the HW.
Yup, they are...
> I'm not even sure if it's appropriate for the low-level mailbox driver to
> know about the semantics of the message, rather than simply sending them on
> to the client driver? Perhaps when drivers register(?) for callbacks(?) for
> messages, they should state which types of messages they want to listen to?
So there's not really a way for the client driver to tell the mailbox
driver which types of messages it wants to listen to on a particular
channel with the mailbox framework - it simply provides a way for
clients to bind with channels. I think there are a couple of options
here, either: a) have a channel per message (as you mentioned in the
previous patch), which allows the client to only register for messages
(channels) it wants to handle, or b) extend the mailbox framework to
allow shared channels so that both clients can receive messages on the
single channel and handle messages appropriately. The disadvantage
of (a) is that the commands are firmware defined and could
theoretically change between releases of the firmware, though I'm not
sure how common that is in practice. So that leaves (b) - Jassi, what
do you think about having shared (non-exclusive) channels?
>> +static irqreturn_t tegra_xusb_mbox_irq(int irq, void *p)
>
>
>> + /* Clear mbox interrupts */
>>
>> + reg = mbox_readl(mbox, XUSB_CFG_ARU_SMI_INTR);
>> + if (reg & MBOX_SMI_INTR_FW_HANG)
>> + dev_err(mbox->mbox.dev, "Controller firmware hang\n");
>> + mbox_writel(mbox, reg, XUSB_CFG_ARU_SMI_INTR);
>
>
>> + /*
>>
>> +* Set the mailbox back to idle. The recipient of the message is
>> +* responsible for sending an ACK/NAK, if necessary.
>> +*/
>> + reg = mbox_readl(mbox, XUSB_CFG_ARU_MBOX_CMD);
>> + reg &= ~MBOX_SMI_INT_EN;
>> + mbox_writel(mbox, reg, XUSB_CFG_ARU_MBOX_CMD);
>
>
> Does the protocol not allow the remote firmware to send another message
> until the host has ack'd/nak'd the message; the code above turns off the IRQ
> that indicated to the host that a message was sent to it...
While the firmware generally will not send another message until the
previous one is ACK'd/NAK'd (with the exception of the SET_BW
command), the above does not prevent it from doing so. I believe the
controller sets up the DEST_* bits properly before sending another
message.
>> +static int tegra_xusb_mbox_probe(struct platform_device *pdev)
>
>
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>
>> + if (!res)
>> + return -ENODEV;
>
>
> Should devm_request_mem_region() be called here to claim the region?
No, the xHCI
Re: [xhci] BUG: unable to handle kernel NULL pointer dereference at (null)
> [ 12.410950] [] ? setup_test_wrap64+0x320/0x320
> [ 12.410950] [] ? setup_test_dont_trim+0x2f0/0x2f0
> [ 12.410950] [] ? xhci_ring_free+0x1d0/0x1d0
> [ 12.410950] [] ? ohci_platform_init+0x60/0x60
> [ 12.410950] [] do_one_initcall+0x143/0x24d
> [ 12.410950] [] ? parse_args+0x2fb/0x530
> [ 12.410950] [] kernel_init_freeable+0x1dc/0x2aa
> [ 12.410950] [] ? do_early_param+0xc3/0xc3
> [ 12.410950] [] ? rest_init+0xd0/0xd0
> [ 12.410950] [] kernel_init+0xe/0x160
> [ 12.410950] [] ret_from_fork+0x7c/0xb0
> [ 12.410950] [] ? rest_init+0xd0/0xd0
> [ 12.410950] Code: 48 85 ff 40 0f 94 c6 44 0f b6 ce 49 83 c1 02 4a 83 04 cd
> a0 e9 b3 82 01 45 31 c9 40 84 f6 75 0b 45 0f b6 ca 49 c1 e1 04 49 01 f9 <49>
> 8b 39 48 8b 30 48 c1 e1 06 4c 89 78 10 44 89 40 08 01 d3 89
> [ 12.410950] RIP [] setup_test_skip64+0x183/0x270
> [ 12.410950] RSP
> [ 12.410950] CR2:
> [ 12.410950] ---[ end trace 3157077290b0c2c1 ]---
> [ 12.410950] Kernel panic - not syncing: Fatal exception
>
> git bisect start 66e8dfa4e0d9600dedc08adcaac83c378b65351b
> 52addcf9d6669fa439387610bc65c92fa0980cef --
> git bisect good 511b6daa3a596ab5c54bee5dab56ed4f77337a40 # 22:39 20+
> 0 Merge 'ipvs-next/master' into devel-hourly-2014082722
> git bisect bad 73e9ac542728ea03b8796cf9818950dc9e05d534 # 22:49 0-
> 20 Merge 'hid/for-3.18/upstream' into devel-hourly-2014082722
> git bisect good 513dd18bd1b397935660c01daa14e53e819b9270 # 23:00 20+
> 0 Merge 'netdev-next/master' into devel-hourly-2014082722
> git bisect good a617416625136eec767df79308544cbb46fe0311 # 23:03 20+
> 0 Merge 'kvm-ppc/kvm-ppc-queue' into devel-hourly-2014082722
> git bisect good 858bf88bf6175c80920daa8c9210b0209443b7e1 # 23:06 20+
> 0 Merge 'spi/for-next' into devel-hourly-2014082722
> git bisect good cdb03bc488490bb364fa29ec292ecd3291ca5770 # 23:10 20+
> 0 Merge 'regulator/for-next' into devel-hourly-2014082722
> git bisect bad 8f5a71eb299401d62562e7ab634665ff98850e8f # 23:13 0-
> 20 Merge 'djbw-usb/td-fragments-v1' into devel-hourly-2014082722
> git bisect good a75ef911cf100b8cf7d25baf6dac8052328a96e7 # 23:22 20+
> 0 xhci: clarify "ring valid" checks
> git bisect good 652b7ee36207f186f3d701675483df43b4845c5c # 23:26 20+
> 0 xhci: kill ->num_trbs_free_temp in struct xhci_ring
> git bisect good 1c11eb8545a3321e7ca27fc7ba8c56b6e6df2b57 # 23:31 20+
> 0 xhci: add xhci_ring_reap_td() helper
> git bisect bad e65e21a542cab81d794db4e5fe919c4e1d624ea7 # 23:54 0-
> 20 xhci: unit test ring enqueue/dequeue routines
> git bisect good fb6fa3e625e1e453aea9eeb97d58bee30e1c0781 # 23:58 20+
> 0 xhci: v1.0 scatterlist enqueue support (td-fragment rework)
> # first bad commit: [e65e21a542cab81d794db4e5fe919c4e1d624ea7] xhci: unit
> test ring enqueue/dequeue routines
> git bisect good fb6fa3e625e1e453aea9eeb97d58bee30e1c0781 # 00:00 60+
> 0 xhci: v1.0 scatterlist enqueue support (td-fragment rework)
> git bisect bad 66e8dfa4e0d9600dedc08adcaac83c378b65351b # 00:00 0-
> 11 0day head guard for 'devel-hourly-2014082722'
> git bisect good 68e370289c29e3beac99d59c6d840d470af9dfcf # 00:19 60+
> 2 Merge branch 'for-linus' of
> git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux
> git bisect good d05446ae2128064a4bb8f74c84f6901ffb5c94bc # 00:33 60+
> 1 Add linux-next specific files for 20140827
>
>
> This script may reproduce the error.
>
>
> #!/bin/bash
>
> kernel=$1
> initrd=quantal-core-x86_64.cgz
>
> wget --no-clobber
> https://github.com/fengguang/reproduce-kernel-bug/raw/master/initrd/$initrd
>
> kvm=(
> qemu-system-x86_64
> -cpu kvm64
> -enable-kvm
> -kernel $kernel
> -initrd $initrd
> -m 320
> -smp 2
> -net nic,vlan=1,model=e1000
> -net user,vlan=1
> -boot order=nc
> -no-reboot
> -watchdog i6300esb
> -rtc base=localtime
> -serial stdio
> -display none
> -monitor null
> )
>
> append=(
> hung_task_panic=1
> earlyprintk=ttyS0,115200
> debug
> apic=debug
> sysrq_always_enabled
> rcupdate.rcu_cpu_stall_timeout=100
> panic=-1
> softlockup_panic=1
> nmi_watchdog=panic
> oops=panic
> load_ramdisk=2
> prompt_ramdisk=0
> console=ttyS0,115200
> console=tty0
> vga=normal
> root=/dev/ram0
> rw
> drbd.minor_count=8
> )
>
> "${kvm[@]}" --append "${append[*]}"
>
>
> Thanks,
> Fengguang
--
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 v6 1/5] usb: gadget: f_uac2: restructure some code in afunc_set_alt()
Restructure some code to make it easier to read.
While at it, return -ENOMEM instead of -EINVAL if
usb_ep_alloc_request() fails, and omit the logging in such cases
(the mm core will complain loud enough).
Signed-off-by: Daniel Mack
---
drivers/usb/gadget/function/f_uac2.c | 39 +++-
1 file changed, 16 insertions(+), 23 deletions(-)
diff --git a/drivers/usb/gadget/function/f_uac2.c
b/drivers/usb/gadget/function/f_uac2.c
index 0d65e7c..ab4652e 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -1104,31 +1104,24 @@ afunc_set_alt(struct usb_function *fn, unsigned intf,
unsigned alt)
usb_ep_enable(ep);
for (i = 0; i < USB_XFERS; i++) {
- if (prm->ureq[i].req) {
- if (usb_ep_queue(ep, prm->ureq[i].req, GFP_ATOMIC))
- dev_err(&uac2->pdev.dev, "%d Error!\n",
- __LINE__);
- continue;
- }
-
- req = usb_ep_alloc_request(ep, GFP_ATOMIC);
- if (req == NULL) {
- dev_err(&uac2->pdev.dev,
- "%s:%d Error!\n", __func__, __LINE__);
- return -EINVAL;
+ if (!prm->ureq[i].req) {
+ req = usb_ep_alloc_request(ep, GFP_ATOMIC);
+ if (req == NULL)
+ return -ENOMEM;
+
+ prm->ureq[i].req = req;
+ prm->ureq[i].pp = prm;
+
+ req->zero = 0;
+ req->context = &prm->ureq[i];
+ req->length = prm->max_psize;
+ req->complete = agdev_iso_complete;
+ req->buf = prm->rbuf + i * req->length;
}
- prm->ureq[i].req = req;
- prm->ureq[i].pp = prm;
-
- req->zero = 0;
- req->context = &prm->ureq[i];
- req->length = prm->max_psize;
- req->complete = agdev_iso_complete;
- req->buf = prm->rbuf + i * req->length;
-
- if (usb_ep_queue(ep, req, GFP_ATOMIC))
- dev_err(&uac2->pdev.dev, "%d Error!\n", __LINE__);
+ if (usb_ep_queue(ep, prm->ureq[i].req, GFP_ATOMIC))
+ dev_err(&uac2->pdev.dev, "%s:%d Error!\n",
+ __func__, __LINE__);
}
return 0;
--
2.1.0
--
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 v6 4/5] usb: gadget: f_uac2: handle partial dma area wrap
With packet sizes other than 512, payloads in the packets may wrap
around the ALSA dma buffer partially, which leads to memory corruption
and audible clicks and pops in the audio stream at the moment, because
there is no boundary check before the memcpy().
In preparation to an implementation for smaller and dynamically sized
packets, we have to address such cases, and copy the payload in two
steps conditionally.
The 'src' and 'dst' approach doesn't work here anymore, as different
behavior is necessary in playback and capture cases. Thus, this patch
open-codes the routine now.
Signed-off-by: Daniel Mack
---
drivers/usb/gadget/function/f_uac2.c | 32 +++-
1 file changed, 23 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/gadget/function/f_uac2.c
b/drivers/usb/gadget/function/f_uac2.c
index 9c8831d..246a778 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -163,8 +163,8 @@ agdev_iso_complete(struct usb_ep *ep, struct usb_request
*req)
{
unsigned pending;
unsigned long flags;
+ unsigned int hw_ptr;
bool update_alsa = false;
- unsigned char *src, *dst;
int status = req->status;
struct uac2_req *ur = req->context;
struct snd_pcm_substream *substream;
@@ -191,26 +191,40 @@ agdev_iso_complete(struct usb_ep *ep, struct usb_request
*req)
spin_lock_irqsave(&prm->lock, flags);
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
- src = prm->dma_area + prm->hw_ptr;
+ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
req->actual = req->length;
- dst = req->buf;
- } else {
- dst = prm->dma_area + prm->hw_ptr;
- src = req->buf;
- }
pending = prm->hw_ptr % prm->period_size;
pending += req->actual;
if (pending >= prm->period_size)
update_alsa = true;
+ hw_ptr = prm->hw_ptr;
prm->hw_ptr = (prm->hw_ptr + req->actual) % prm->dma_bytes;
spin_unlock_irqrestore(&prm->lock, flags);
/* Pack USB load in ALSA ring buffer */
- memcpy(dst, src, req->actual);
+ pending = prm->dma_bytes - hw_ptr;
+
+ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+ if (unlikely(pending < req->actual)) {
+ memcpy(req->buf, prm->dma_area + hw_ptr, pending);
+ memcpy(req->buf + pending, prm->dma_area,
+ req->actual - pending);
+ } else {
+ memcpy(req->buf, prm->dma_area + hw_ptr, req->actual);
+ }
+ } else {
+ if (unlikely(pending < req->actual)) {
+ memcpy(prm->dma_area + hw_ptr, req->buf, pending);
+ memcpy(prm->dma_area, req->buf + pending,
+ req->actual - pending);
+ } else {
+ memcpy(prm->dma_area + hw_ptr, req->buf, req->actual);
+ }
+ }
+
exit:
if (usb_ep_queue(ep, req, GFP_ATOMIC))
dev_err(&uac2->pdev.dev, "%d Error!\n", __LINE__);
--
2.1.0
--
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 v6 0/5] usb: gadget: f_uac2: cleanups and capture timing
Hi, this is v6 of the f_uac2 timing fixup series. Changes from v5: * Pre-calculate some more variables so we have to do their computation at runtime for each frame Changes from v4: * Fix buffer setup in afunc_set_alt() Changes from v3: * add another patch (3/5) to introduce agdev_to_uac2_opts() which is also needed in 5/5 * patch 5/5 only: move from a pre-calculated sequence of packet lengths to an accumulator that sums up the residual values from the packet size calculation and adds an extra sample frame now and then when the accumulator has gathered enough bytes. Changes from v2: * swap path 3 and 4, so that the ALSA buffer wrap around fix comes in first. It's not actually a bug fix for the current code, but more a preparation to allow for smaller packets. * use the p_ssize, p_chmask and p_srate for the length calculations * prepare a sequence of packet sizes to send, and alternate over them when sending the the IN packets. The actual commit log and the comments yield some more details on that. Changes from v1: * drop UAC_EP_CS_ATTR_FILL_MAX approach and rather size the packets correctly * add a patch to fix buffer wrap problems in the ALSA buffer logic, which wasn't needed before The first two patches are just cleanups. Thanks to Alan Stern and Jassi Brar for the feedback! Daniel Subject: [PATCH v6 0/5] *** SUBJECT HERE *** *** BLURB HERE *** Daniel Mack (5): usb: gadget: f_uac2: restructure some code in afunc_set_alt() usb: gadget: f_uac2: add short-hand for 'dev' usb: gadget: f_uac2: introduce agdev_to_uac2_opts usb: gadget: f_uac2: handle partial dma area wrap usb: gadget: f_uac2: send reasonably sized packets drivers/usb/gadget/function/f_uac2.c | 167 +-- 1 file changed, 118 insertions(+), 49 deletions(-) -- 2.1.0 -- 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 v6 5/5] usb: gadget: f_uac2: send reasonably sized packets
The UAC2 function driver currently responds to all packets at all times
with wMaxPacketSize packets. That results in way too fast audio
playback as the function driver (which is in fact supposed to define
the audio stream pace) delivers as fast as it can.
Fix this by sizing each packet correctly with the following steps:
a) Set the packet's size by dividing the nominal data rate by the
playback endpoint's interval.
b) If there is a residual value from the calculation in a), add
it to a accumulator to keep track of it across packets.
c) If the accumulator has gathered at least the number of bytes
that are needed for one sample frame, increase the packet size.
This way, the packet size calculation will get rid of any kind of
imprecision that would otherwise occur with a simple division over
time.
Some of the variables that are needed while processing each packet
are pre-computed for performance reasons.
Signed-off-by: Daniel Mack
---
drivers/usb/gadget/function/f_uac2.c | 69 +---
1 file changed, 65 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/function/f_uac2.c
b/drivers/usb/gadget/function/f_uac2.c
index 246a778..a5a27a5 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -92,6 +92,15 @@ struct snd_uac2_chip {
struct snd_card *card;
struct snd_pcm *pcm;
+
+ /* timekeeping for the playback endpoint */
+ unsigned int p_interval;
+ unsigned int p_residue;
+
+ /* pre-calculated values for playback iso completion */
+ unsigned int p_pktsize;
+ unsigned int p_pktsize_residue;
+ unsigned int p_framesize;
};
#define BUFF_SIZE_MAX (PAGE_SIZE * 16)
@@ -191,8 +200,29 @@ agdev_iso_complete(struct usb_ep *ep, struct usb_request
*req)
spin_lock_irqsave(&prm->lock, flags);
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+ /*
+* For each IN packet, take the quotient of the current data
+* rate and the endpoint's interval as the base packet size.
+* If there is a residue from this division, add it to the
+* residue accumulator.
+*/
+ req->length = uac2->p_pktsize;
+ uac2->p_residue += uac2->p_pktsize_residue;
+
+ /*
+* Whenever there are more bytes in the accumulator than we
+* need to add one more sample frame, increase this packet's
+* size and decrease the accumulator.
+*/
+ if (uac2->p_residue / uac2->p_interval >= uac2->p_framesize) {
+ req->length += uac2->p_framesize;
+ uac2->p_residue -= uac2->p_framesize *
+ uac2->p_interval;
+ }
+
req->actual = req->length;
+ }
pending = prm->hw_ptr % prm->period_size;
pending += req->actual;
@@ -346,6 +376,7 @@ static int uac2_pcm_open(struct snd_pcm_substream
*substream)
c_srate = opts->c_srate;
p_chmask = opts->p_chmask;
c_chmask = opts->c_chmask;
+ uac2->p_residue = 0;
runtime->hw = uac2_pcm_hardware;
@@ -1077,7 +1108,7 @@ afunc_set_alt(struct usb_function *fn, unsigned intf,
unsigned alt)
struct usb_request *req;
struct usb_ep *ep;
struct uac2_rtd_params *prm;
- int i;
+ int req_len, i;
/* No i/f has more than 2 alt settings */
if (alt > 1) {
@@ -1099,11 +1130,41 @@ afunc_set_alt(struct usb_function *fn, unsigned intf,
unsigned alt)
prm = &uac2->c_prm;
config_ep_by_speed(gadget, fn, ep);
agdev->as_out_alt = alt;
+ req_len = prm->max_psize;
} else if (intf == agdev->as_in_intf) {
+ struct f_uac2_opts *opts = agdev_to_uac2_opts(agdev);
+ unsigned int factor, rate;
+ struct usb_endpoint_descriptor *ep_desc;
+
ep = agdev->in_ep;
prm = &uac2->p_prm;
config_ep_by_speed(gadget, fn, ep);
agdev->as_in_alt = alt;
+
+ /* pre-calculate the playback endpoint's interval */
+ if (gadget->speed == USB_SPEED_FULL) {
+ ep_desc = &fs_epin_desc;
+ factor = 1000;
+ } else {
+ ep_desc = &hs_epin_desc;
+ factor = 125;
+ }
+
+ /* pre-compute some values for iso_complete() */
+ uac2->p_framesize = opts->p_ssize *
+ num_channels(opts->p_chmask);
+ rate = opts->p_srate * uac2->p_framesize;
+ uac2->p_interval = (1 << (ep_desc->bInterval - 1)) * factor;
+ uac2->p_pktsize = min_t(
[PATCH v6 2/5] usb: gadget: f_uac2: add short-hand for 'dev'
In afunc_bind() and afunc_set_alt(), &uac2->pdev.dev are used multiple
times. Adding a short-hand for them makes lines shorter so we can
remove some line wraps.
No functional change.
Signed-off-by: Daniel Mack
---
drivers/usb/gadget/function/f_uac2.c | 29 +++--
1 file changed, 11 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/gadget/function/f_uac2.c
b/drivers/usb/gadget/function/f_uac2.c
index ab4652e..efe8add 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -918,6 +918,7 @@ afunc_bind(struct usb_configuration *cfg, struct
usb_function *fn)
struct snd_uac2_chip *uac2 = &agdev->uac2;
struct usb_composite_dev *cdev = cfg->cdev;
struct usb_gadget *gadget = cdev->gadget;
+ struct device *dev = &uac2->pdev.dev;
struct uac2_rtd_params *prm;
struct f_uac2_opts *uac2_opts;
struct usb_string *us;
@@ -961,8 +962,7 @@ afunc_bind(struct usb_configuration *cfg, struct
usb_function *fn)
ret = usb_interface_id(cfg, fn);
if (ret < 0) {
- dev_err(&uac2->pdev.dev,
- "%s:%d Error!\n", __func__, __LINE__);
+ dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
return ret;
}
std_ac_if_desc.bInterfaceNumber = ret;
@@ -971,8 +971,7 @@ afunc_bind(struct usb_configuration *cfg, struct
usb_function *fn)
ret = usb_interface_id(cfg, fn);
if (ret < 0) {
- dev_err(&uac2->pdev.dev,
- "%s:%d Error!\n", __func__, __LINE__);
+ dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
return ret;
}
std_as_out_if0_desc.bInterfaceNumber = ret;
@@ -982,8 +981,7 @@ afunc_bind(struct usb_configuration *cfg, struct
usb_function *fn)
ret = usb_interface_id(cfg, fn);
if (ret < 0) {
- dev_err(&uac2->pdev.dev,
- "%s:%d Error!\n", __func__, __LINE__);
+ dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
return ret;
}
std_as_in_if0_desc.bInterfaceNumber = ret;
@@ -993,16 +991,14 @@ afunc_bind(struct usb_configuration *cfg, struct
usb_function *fn)
agdev->out_ep = usb_ep_autoconfig(gadget, &fs_epout_desc);
if (!agdev->out_ep) {
- dev_err(&uac2->pdev.dev,
- "%s:%d Error!\n", __func__, __LINE__);
+ dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
goto err;
}
agdev->out_ep->driver_data = agdev;
agdev->in_ep = usb_ep_autoconfig(gadget, &fs_epin_desc);
if (!agdev->in_ep) {
- dev_err(&uac2->pdev.dev,
- "%s:%d Error!\n", __func__, __LINE__);
+ dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
goto err;
}
agdev->in_ep->driver_data = agdev;
@@ -1057,6 +1053,7 @@ afunc_set_alt(struct usb_function *fn, unsigned intf,
unsigned alt)
struct audio_dev *agdev = func_to_agdev(fn);
struct snd_uac2_chip *uac2 = &agdev->uac2;
struct usb_gadget *gadget = cdev->gadget;
+ struct device *dev = &uac2->pdev.dev;
struct usb_request *req;
struct usb_ep *ep;
struct uac2_rtd_params *prm;
@@ -1064,16 +1061,14 @@ afunc_set_alt(struct usb_function *fn, unsigned intf,
unsigned alt)
/* No i/f has more than 2 alt settings */
if (alt > 1) {
- dev_err(&uac2->pdev.dev,
- "%s:%d Error!\n", __func__, __LINE__);
+ dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
return -EINVAL;
}
if (intf == agdev->ac_intf) {
/* Control I/f has only 1 AltSetting - 0 */
if (alt) {
- dev_err(&uac2->pdev.dev,
- "%s:%d Error!\n", __func__, __LINE__);
+ dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
return -EINVAL;
}
return 0;
@@ -1090,8 +1085,7 @@ afunc_set_alt(struct usb_function *fn, unsigned intf,
unsigned alt)
config_ep_by_speed(gadget, fn, ep);
agdev->as_in_alt = alt;
} else {
- dev_err(&uac2->pdev.dev,
- "%s:%d Error!\n", __func__, __LINE__);
+ dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
return -EINVAL;
}
@@ -1120,8 +1114,7 @@ afunc_set_alt(struct usb_function *fn, unsigned intf,
unsigned alt)
}
if (usb_ep_queue(ep, prm->ureq[i].req, GFP_ATOMIC))
- dev_err(&uac2->pdev.dev, "%s:%d Error!\n",
- __func__, __LINE__);
+ dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
}
return 0;
--
2.1.0
--
To unsubscribe
[PATCH v6 3/5] usb: gadget: f_uac2: introduce agdev_to_uac2_opts
Add a simple container_of() wrapper to get a struct f_uac2_opts from a
struct struct audio_dev. Use it in two places where it is currently
open-coded.
Signed-off-by: Daniel Mack
---
drivers/usb/gadget/function/f_uac2.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/function/f_uac2.c
b/drivers/usb/gadget/function/f_uac2.c
index efe8add..9c8831d 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -140,6 +140,12 @@ struct snd_uac2_chip *pdev_to_uac2(struct platform_device
*p)
}
static inline
+struct f_uac2_opts *agdev_to_uac2_opts(struct audio_dev *agdev)
+{
+ return container_of(agdev->func.fi, struct f_uac2_opts, func_inst);
+}
+
+static inline
uint num_channels(uint chanmask)
{
uint num = 0;
@@ -1168,7 +1174,7 @@ in_rq_cur(struct usb_function *fn, const struct
usb_ctrlrequest *cr)
int value = -EOPNOTSUPP;
int p_srate, c_srate;
- opts = container_of(agdev->func.fi, struct f_uac2_opts, func_inst);
+ opts = agdev_to_uac2_opts(agdev);
p_srate = opts->p_srate;
c_srate = opts->c_srate;
@@ -1210,7 +1216,7 @@ in_rq_range(struct usb_function *fn, const struct
usb_ctrlrequest *cr)
int value = -EOPNOTSUPP;
int p_srate, c_srate;
- opts = container_of(agdev->func.fi, struct f_uac2_opts, func_inst);
+ opts = agdev_to_uac2_opts(agdev);
p_srate = opts->p_srate;
c_srate = opts->c_srate;
--
2.1.0
--
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: About the reboot hang issue with EHCI driver on the Baytrail platform
Hi Alan, On Wed, Aug 27, 2014 at 10:38 PM, Alan Stern wrote: > On Wed, 27 Aug 2014, Gavin Guo wrote: > >> > Still, why not use the BIOS setting to disable EHCI? >> >> It's hardware enablement policy. By default, we can't change the BIOS >> setting to make the bug disappear until we find out the bug is due to >> BIOS's bug. Add kernel parameters is the temporary solution for the >> platform can ship. And if the bug is from kernel, we need to send >> patches to upstream and pull back to Ubuntu kernel or send to Ubuntu >> Kernel directly if it's a trivial solution like adding device id. >> However, as you said, in this case about the bug the root may be from >> BIOS code. So, we can coordinate with the OEM BIOS team to modify the >> default BIOS setting. > > There is no doubt that the BIOS is buggy. Your log contains this line: > > [2.804205] pci :00:1d.0: EHCI: BIOS handoff failed (BIOS bug?) > 01010001 > > The only way that can happen is if the BIOS isn't working right. > However, this may be a different bug from the crashes you've been > seeing. Thanks for your comments. I really appreciate your support. Gavin Guo > > Alan Stern > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- 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 v5 5/5] usb: gadget: f_uac2: send reasonably sized packets
On 08/27/2014 04:18 PM, Alan Stern wrote:
> On Wed, 27 Aug 2014, Daniel Mack wrote:
>> +/*
>> + * Whenever there are more bytes in the accumulator than we
>> + * need to add one more sample frame, increase this packet's
>> + * size and decrease the accumulator.
>> + */
>> +if (uac2->p_residue / uac2->p_interval >= fsz) {
>> +req->length += fsz;
>> +uac2->p_residue -= fsz * uac2->p_interval;
>> +}
>
> One thing I didn't mention about the algorithm I posted earlier: The
> individual packet calculations don't contain any multiplications or
> divisions.
>
> The code here is in the hottest part of the driver, so you might want
> to optimize it a little. For instance, several of the operations could
> be precomputed and stored for later use.
Yeah, well. I did tests with that, and at least the difference in load
is not measureable on BBB. But you're right of course, and it even leads
to nicer code. Will send that as v6.
Thanks,
Daniel
--
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/9] of: Update Tegra XUSB pad controller binding for USB
On Mon, Aug 25, 2014 at 12:12 PM, Stephen Warren wrote:
> On 08/18/2014 11:08 AM, Andrew Bresticker wrote:
>> - #phy-cells: Should be 1. The specifier is the index of the PHY to
>> reference.
>> See for the list of valid
>> values.
>> +- mboxes: Must contain an entry for the XUSB PHY mailbox channel.
>> + See ../mailbox/mailbox.txt for details.
>
>
> Can we require the mbox-names property here, so that everything is looked up
> by names. I know that the proposed mbox binding states that using indexes is
> preferred over names, but that's just silly considering that names are
> widely used in most other similar bindings, and are much easier to extend in
> a backwards compatible fashion in the face of optional entries. As such, I'd
> prefer that all Tegra bindings use foo-names properties where they exist.
Sure, will do.
>> +Optional properties:
>> +---
>> +- vbus-otg-{0,1,2}-supply: VBUS regulator for the corresponding UTMI pad.
>
>
> Why "-otg"? It's quite possible to have a regulator for VBUS even on systems
> that don't support OTG, but rather simply have the ability to turn VBUS off.
Because they're the VBUS supplies for the OTG 'lanes'. It doesn't
really add anything, so I'll omit the "-otg".
--
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 4/9] pinctrl: tegra-xusb: Add USB PHY support
On Mon, Aug 25, 2014 at 12:22 PM, Stephen Warren wrote:
> On 08/18/2014 11:08 AM, Andrew Bresticker wrote:
>>
>> In addition to the PCIe and SATA PHYs, the XUSB pad controller also
>> supports 3 UTMI, 2 HSIC, and 2 USB3 PHYs. Each USB3 PHY uses a single
>> PCIe or SATA lane and is mapped to one of the three UTMI ports.
>>
>> The xHCI controller will also send messages intended for the PHY driver,
>> so request and listen for messages on the mailbox's PHY channel.
>
>
> I'd like a review from Thierry here as the HW expert.
>
> I need an ack from LinusW in order to take this pinctrl patch through the
> Tegra tree.
>
>> diff --git a/drivers/pinctrl/pinctrl-tegra-xusb.c
>> b/drivers/pinctrl/pinctrl-tegra-xusb.c
>
>
>> +static int usb3_phy_power_on(struct phy *phy)
>> +{
>> + struct tegra_xusb_padctl *padctl = phy_get_drvdata(phy);
>> + int port = usb3_phy_to_port(phy);
>> + int lane = padctl->usb3_ports[port].lane;
>> + u32 value, offset;
>> +
>> + if (!is_pcie_or_sata_lane(lane)) {
>> + dev_err(padctl->dev, "USB3 PHY %d mapped to invalid lane:
>> %d\n",
>> + port, lane);
>> + return -EINVAL;
>> + }
>
>
> An aside: This implies that the SATA driver should be talking to this
> pinctrl driver and explicitly powering on the XUSB pins. However, the SATA
> driver doesn't depend on this series. I'm a bit confused how that works.
> Perhaps it's just by accident? Mikko, can you comment?
As Mikko mentioned, the enabling of the SATA lane in
usb3_phy_power_on() is for when the SATA lane is being used for USB3.
>> +static int utmi_phy_to_port(struct phy *phy)
>> +{
>> + struct tegra_xusb_padctl *padctl = phy_get_drvdata(phy);
>> + int i;
>> +
>> + for (i = 0; i < TEGRA_XUSB_UTMI_PHYS; i++) {
>> + if (phy == padctl->phys[TEGRA_XUSB_PADCTL_UTMI_P0 + i])
>> + break;
>> + }
>> + BUG_ON(i == TEGRA_XUSB_UTMI_PHYS);
>
>
> Can this be triggered by e.g. bad DT content? If so, returning an error
> would be nicer. The comment applies to other xxx_to_port() functions.
No, it cannot. The struct phy that's passed in here comes from the
PHY core and must be a PHY that we registered earlier in probe().
--
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/9] of: Update Tegra XUSB pad controller binding for USB
On 08/27/2014 10:36 AM, Andrew Bresticker wrote:
On Mon, Aug 25, 2014 at 12:12 PM, Stephen Warren wrote:
On 08/18/2014 11:08 AM, Andrew Bresticker wrote:
- #phy-cells: Should be 1. The specifier is the index of the PHY to
reference.
See for the list of valid
values.
+- mboxes: Must contain an entry for the XUSB PHY mailbox channel.
+ See ../mailbox/mailbox.txt for details.
Can we require the mbox-names property here, so that everything is looked up
by names. I know that the proposed mbox binding states that using indexes is
preferred over names, but that's just silly considering that names are
widely used in most other similar bindings, and are much easier to extend in
a backwards compatible fashion in the face of optional entries. As such, I'd
prefer that all Tegra bindings use foo-names properties where they exist.
Sure, will do.
+Optional properties:
+---
+- vbus-otg-{0,1,2}-supply: VBUS regulator for the corresponding UTMI pad.
Why "-otg"? It's quite possible to have a regulator for VBUS even on systems
that don't support OTG, but rather simply have the ability to turn VBUS off.
Because they're the VBUS supplies for the OTG 'lanes'. It doesn't
really add anything, so I'll omit the "-otg".
Ah right. In that case, if the lanes are named "OTG" lanes in the HW
docs, I'm happy either way. If you did decide to keep the "-otg",
rewording as "VBUS regulator for the corresponding OTG UTMI pad" would
make the meaning clearer.
--
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 1/9] of: Add NVIDIA Tegra XUSB mailbox binding
On Mon, Aug 25, 2014 at 11:48 AM, Stephen Warren wrote: > On 08/18/2014 11:08 AM, Andrew Bresticker wrote: > >> diff --git a/include/dt-bindings/mailbox/tegra-xusb-mailbox.h >> b/include/dt-bindings/mailbox/tegra-xusb-mailbox.h > > >> +#define TEGRA_XUSB_MBOX_CHAN_HOST 0 >> +#define TEGRA_XUSB_MBOX_CHAN_PHY 1 > > > I can't work out how these values relate to hardware at all. Are they in > fact properties of the particular firmware that's loaded into the XUSB > module? If so, I don't think the DT should contain these values at all. Yes this is rather ugly... they're used by software for the demuxing of messages and don't correspond to actual hardware. > I wonder if the individual MBOX_CMD_* values from patch 2 are any > better, although I think those are also defined by the firmware, not the > hardware? They are indeed defined by the firmware, so this would become an issue if the firmware API ever changed. -- 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] xhci-ring: Fix Null pointer dereference
Perhaps we could apply both patches to current tree and backport mine to older kernels? On Wed, Aug 27, 2014 at 5:27 PM, Mathias Nyman wrote: > On 08/27/2014 05:14 PM, Ricardo Ribalda Delgado wrote: >> At least I have seen the issue on Debian 3.14 and 3.16. Is your patch >> going to be backported to linux-stable? The computer crashes very very >> badly >> > > Yes, it is, but it might need some additional work as it won't apply cleanly > on older versions > > http://marc.info/?l=linux-usb&m=140913688327011&w=2 > > -Mathias -- Ricardo Ribalda -- 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
Status of g_webcam uvc-gadget
Hello Is somebody using/supporting g_webcam? The only reference userland server is uvc-gadget from http://git.ideasonboard.org/?p=uvc-gadget.git;a=summary ? I have an industrial fpga camera that speaks v4l2, my plan is to export it as an uvc camera via usb3380 as a debug interface. Thanks! -- Ricardo Ribalda -- 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 0/7] g_webcam: Preparation for configfs
W dniu 18.08.2014 o 21:45, Laurent Pinchart pisze: Hi Andrzej, Thank you for the patches. The series looks good, I only had a few comments. I have rebased the patches on top of my UVC gadget branch, addressed my comments (the modified patches are marked as such in the commit message) and pushed the result to git://linuxtv.org/pinchartl/media.git uvc/gadget Would you be able to test the result and hopefully ack it ? If everything is fine with you there's no need to submit a new version. It seems that the f_uvc transition to video_ioctl2 is missing vfl_dir flag; I posted a patch fixing it. After the fix is applied I was able to test on real hardware, so as far as your changes to my patches are concerned, this is Acked-by: Andrzej Pietrasiewicz AP -- 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 0/4] usb: gadget: f_uac2: cleanups and capture timing
Hi Xuebing, > Would you mind to share how did you run the test for both > Playback/Capture? What host version are you using, as different Host > enumerates (sends sequences of USB_REQ_SET_INTERFACE request differently)? sure, on client side (BBB) I have a jackd on top alsa running and connected capture and playback ports together. On host side I use a Linux Kernel 3.16.1 and forward the gadget usb with alsa_in and alsa_out to a jackd pipe with another usb interface (focusrite UAC2). regards, Sebastian signature.asc Description: This is a digitally signed message part
[PATCH] usb: gadget: f_uvc fix transition to video_ioctl2
UVC video node is a TX device from the point of view of the gadget, so we cannot rely on the video struct being filled with zeros, because VFL_DIR_TX is actually 1. Suggested-by: Sylwester Nawrocki Signed-off-by: Andrzej Pietrasiewicz --- drivers/usb/gadget/function/f_uvc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c index 5209105..95dc1c6 100644 --- a/drivers/usb/gadget/function/f_uvc.c +++ b/drivers/usb/gadget/function/f_uvc.c @@ -411,6 +411,7 @@ uvc_register_video(struct uvc_device *uvc) video->fops = &uvc_v4l2_fops; video->ioctl_ops = &uvc_v4l2_ioctl_ops; video->release = video_device_release; + video->vfl_dir = VFL_DIR_TX; strlcpy(video->name, cdev->gadget->name, sizeof(video->name)); uvc->vdev = video; -- 1.9.1 -- 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] xhci-ring: Fix Null pointer dereference
On 08/27/2014 05:14 PM, Ricardo Ribalda Delgado wrote: > At least I have seen the issue on Debian 3.14 and 3.16. Is your patch > going to be backported to linux-stable? The computer crashes very very > badly > Yes, it is, but it might need some additional work as it won't apply cleanly on older versions http://marc.info/?l=linux-usb&m=140913688327011&w=2 -Mathias -- 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 1/2] usb: gadget: Refactor request completion
On Wed, 27 Aug 2014, Michal Sojka wrote:
> All USB peripheral controller drivers called completion routines
> directly. This patch moves the completion call from drivers to
> usb_gadget_giveback_request(), in order to have a place where common
> functionality can be added.
>
> All places in drivers/usb/ matching "[-.]complete(" were replaced with a
> call to usb_gadget_giveback_request(). This was compile-tested with all
> ARM drivers enabled and runtime-tested for musb.
> --- a/drivers/usb/gadget/udc/udc-core.c
> +++ b/drivers/usb/gadget/udc/udc-core.c
> @@ -102,6 +102,15 @@ void usb_gadget_unmap_request(struct usb_gadget *gadget,
> }
> EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
>
> +void usb_gadget_giveback_request(struct usb_ep *ep,
> + struct usb_request *req)
> +{
> + /* complete() is from gadget layer,
> + * eg fsg->bulk_in_complete() */
Wrong format for a multi-line comment. It should look like this:
/*
* Blah, blah, blah
* blah, blah, blah
*/
Also, the reference to fsg is obsolete; the File-backed Storage Gadget
is no longer in the kernel. You might as well leave the comment out
entirely.
> + if (req->complete)
This test shouldn't be here. The UDC drivers don't do it, but they
probably do check the req->complete isn't NULL when the request is
submitted. In any case, if req->complete is NULL then we want to know
about it because it is a bug. It should not be covered up silently.
> + req->complete(ep, req);
> +}
> +EXPORT_SYMBOL_GPL(usb_gadget_giveback_request);
> #endif /* CONFIG_HAS_DMA */
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: Buffer I/O error after s2ram with usb storage
On Wed, 27 Aug 2014, Douglas Gilbert wrote: > The unit attention doesn't look like a problem, it > looks correct. If the system is unable to detect > removable media being changed while the system is > suspended, then > > If the media has a unique identifier, then this unit > attention at wakeup should trigger sd to make sure > that unique identifier has not changed. Does sd have any code to do this? I'm not aware of any, but there ought to be some. Otherwise there's no way to tell when a so-called media change was just the old media being taken out and put back in. Or maybe this functionality belongs in the block layer rather than in sd. 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: Buffer I/O error after s2ram with usb storage
On Wed, 27 Aug 2014, Matthieu CASTET wrote:
> Ping
>
> I have got also a problem with a usb sdcard reader (without power cut
> during suspend)
> > > The usb storage driver call scsi_report_bus_reset after device reset,
> > > but because of commit dfcf7775 <4>, we don't ignore unit attention if
> > > "sshdr.asc == 0x28 && sshdr.ascq == 0x00" ("Not-ready to ready").
> > >
> > > If dfcf7775 is reverted there is no more Buffer I/O error.
> > >
> > > Is that possible to find a way to restore the behavior before dfcf7775
> > > commit (no Buffer I/O error after device reset) after a suspend to ram ?
Since that commit was written to fix a problem with certain cdrom
drives, maybe we would work around the issue by modifying the commit.
Have it go back to the original behavior if the device isn't a cdrom
drive.
That's not a complete fix (it won't help when a CD drive is attached
via USB), but maybe it's better than nothing.
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: RES: RES: AS2105-based enclosure size issues with >2TB HDDs
On Wed, 27 Aug 2014, Oliver Neukum wrote: > > I don't think we want to add another SCSI flag to say that READ > > CAPACITY(10) is unreliable. > > Why not? It would only be friendly to tell the upper layer > of a malfunction if we know about it. To what end? What will the upper layer do with this information? > > Given the difficulty of determining the true capacity, I see two > > alternatives. We could set the capacity to a ridiculously large value > > (like 1 billion TB), or we could leave the capacity set to the low > > value and disable the "block within bounds" checks. Neither of these > > is attractive and they both have issues of their own -- although the > > second is close to what Windows does. > > That seems to be the most attractive solution to me. I'm skeptical that you can convince the SCSI and block-layer developers about this. Maybe they'll accept it if it is applied only to USB transports... > > (For example, udev often tries to read the last sectors of a new drive, > > looking for a GPT or RAID signature. That won't work if the capacity > > is set to some random value.) > > Yes, but clipping has its own dangers. Suppose you use the medium > without a partition table. What would Windows do? In the absence of a partition table, it would believe the value from READ CAPACITY, right? Isn't that the same as clipping? 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: About the reboot hang issue with EHCI driver on the Baytrail platform
On Wed, 27 Aug 2014, Gavin Guo wrote: > > Still, why not use the BIOS setting to disable EHCI? > > It's hardware enablement policy. By default, we can't change the BIOS > setting to make the bug disappear until we find out the bug is due to > BIOS's bug. Add kernel parameters is the temporary solution for the > platform can ship. And if the bug is from kernel, we need to send > patches to upstream and pull back to Ubuntu kernel or send to Ubuntu > Kernel directly if it's a trivial solution like adding device id. > However, as you said, in this case about the bug the root may be from > BIOS code. So, we can coordinate with the OEM BIOS team to modify the > default BIOS setting. There is no doubt that the BIOS is buggy. Your log contains this line: [2.804205] pci :00:1d.0: EHCI: BIOS handoff failed (BIOS bug?) 01010001 The only way that can happen is if the BIOS isn't working right. However, this may be a different bug from the crashes you've been seeing. 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 v5 5/5] usb: gadget: f_uac2: send reasonably sized packets
On Wed, 27 Aug 2014, Daniel Mack wrote:
> The UAC2 function driver currently responds to all packets at all times
> with wMaxPacketSize packets. That results in way too fast audio
> playback as the function driver (which is in fact supposed to define
> the audio stream pace) delivers as fast as it can.
>
> Fix this by sizing each packet correctly with the following steps:
>
> a) Set the packet's size by dividing the nominal data rate by the
> playback endpoint's interval.q
>
> b) If there is a residual value from the calculation in a), add
> it to a accumulator to keep track of it across packets.
>
> c) If the accumulator has gathered at least the number of bytes
> that are needed for one sample frame, increase the packet size.
>
> This way, the packet size calculation will get rid of any kind of
> imprecision that would otherwise occur with a simple division over
> time.
>
> Signed-off-by: Daniel Mack
> @@ -191,8 +195,43 @@ agdev_iso_complete(struct usb_ep *ep, struct usb_request
> *req)
>
> spin_lock_irqsave(&prm->lock, flags);
>
> - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> + struct audio_dev *agdev = uac2_to_agdev(uac2);
> + struct f_uac2_opts *opts = agdev_to_uac2_opts(agdev);
> + unsigned int fsz = opts->p_ssize * num_channels(opts->p_chmask);
> + unsigned int rate = opts->p_srate * fsz;
> +
> + /*
> + * For each IN packet, calculate the minimum packet size by
> + * dividing the nominal bytes per second as required by the
> + * current audio format by the endpoint's interval.
> + */
> + req->length = min_t(unsigned int,
> + rate / uac2->p_interval, prm->max_psize);
> +
> + /*
> + * If there is a residual value from the division, add it to
> + * the residue accumulator.
> + */
> + uac2->p_residue += rate % uac2->p_interval;
> +
> + /*
> + * Whenever there are more bytes in the accumulator than we
> + * need to add one more sample frame, increase this packet's
> + * size and decrease the accumulator.
> + */
> + if (uac2->p_residue / uac2->p_interval >= fsz) {
> + req->length += fsz;
> + uac2->p_residue -= fsz * uac2->p_interval;
> + }
One thing I didn't mention about the algorithm I posted earlier: The
individual packet calculations don't contain any multiplications or
divisions.
The code here is in the hottest part of the driver, so you might want
to optimize it a little. For instance, several of the operations could
be precomputed and stored for later use.
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 3/3] xhci: rework cycle bit checking for new dequeue pointers
On 08/27/2014 07:07 AM, Mathias Nyman wrote:
> On 08/21/2014 01:06 AM, Joseph Salisbury wrote:
>> On 08/19/2014 08:17 AM, Mathias Nyman wrote:
>>> When we manually need to move the TR dequeue pointer we need to set the
>>> correct cycle bit as well. Previously we used the trb pointer from the
>>> last event received as a base, but this was changed in
>>> commit 1f81b6d22a59 ("usb: xhci: Prefer endpoint context dequeue pointer")
>>> to use the dequeue pointer from the endpoint context instead
>>>
>>> It turns out some Asmedia controllers advance the dequeue pointer
>>> stored in the endpoint context past the event triggering TRB, and
>>> this messed up the way the cycle bit was calculated.
>>>
>>> Instead of adding a quirk or complicating the already hard to follow cycle
>>> bit
>>> code, the whole cycle bit calculation is now simplified and adapted to
>>> handle
>>> event and endpoint context dequeue pointer differences.
>>>
>>> Fixes: 1f81b6d22a59 ("usb: xhci: Prefer endpoint context dequeue pointer")
>>> Reported-by: Maciej Puzio
>>> Reported-by: Evan Langlois
>>> Reviewed-by: Julius Werner
>>> Tested-by: Maciej Puzio
>>> Tested-by: Evan Langlois
>>> Signed-off-by: Mathias Nyman
>>> Cc: sta...@vger.kernel.org
>>> ---
>>> drivers/usb/host/xhci-ring.c | 101
>>> +--
>>> drivers/usb/host/xhci.c | 3 ++
>>> 2 files changed, 42 insertions(+), 62 deletions(-)
>>>
>>> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
>>> index ac8cf23..abed30b 100644
>>> --- a/drivers/usb/host/xhci-ring.c
>>> +++ b/drivers/usb/host/xhci-ring.c
>>> @@ -364,32 +364,6 @@ static void ring_doorbell_for_active_rings(struct
>>> xhci_hcd *xhci,
>>> }
>>> }
>>>
>>> -/*
>>> - * Find the segment that trb is in. Start searching in start_seg.
>>> - * If we must move past a segment that has a link TRB with a toggle cycle
>>> state
>>> - * bit set, then we will toggle the value pointed at by cycle_state.
>>> - */
>>> -static struct xhci_segment *find_trb_seg(
>>> - struct xhci_segment *start_seg,
>>> - union xhci_trb *trb, int *cycle_state)
>>> -{
>>> - struct xhci_segment *cur_seg = start_seg;
>>> - struct xhci_generic_trb *generic_trb;
>>> -
>>> - while (cur_seg->trbs > trb ||
>>> - &cur_seg->trbs[TRBS_PER_SEGMENT - 1] < trb) {
>>> - generic_trb = &cur_seg->trbs[TRBS_PER_SEGMENT - 1].generic;
>>> - if (generic_trb->field[3] & cpu_to_le32(LINK_TOGGLE))
>>> - *cycle_state ^= 0x1;
>>> - cur_seg = cur_seg->next;
>>> - if (cur_seg == start_seg)
>>> - /* Looped over the entire list. Oops! */
>>> - return NULL;
>>> - }
>>> - return cur_seg;
>>> -}
>>> -
>>> -
>>> static struct xhci_ring *xhci_triad_to_transfer_ring(struct xhci_hcd *xhci,
>>> unsigned int slot_id, unsigned int ep_index,
>>> unsigned int stream_id)
>>> @@ -459,9 +433,12 @@ void xhci_find_new_dequeue_state(struct xhci_hcd *xhci,
>>> struct xhci_virt_device *dev = xhci->devs[slot_id];
>>> struct xhci_virt_ep *ep = &dev->eps[ep_index];
>>> struct xhci_ring *ep_ring;
>>> - struct xhci_generic_trb *trb;
>>> + struct xhci_segment *new_seg;
>>> + union xhci_trb *new_deq;
>>> dma_addr_t addr;
>>> u64 hw_dequeue;
>>> + bool cycle_found = false;
>>> + bool td_last_trb_found = false;
>>>
>>> ep_ring = xhci_triad_to_transfer_ring(xhci, slot_id,
>>> ep_index, stream_id);
>>> @@ -486,45 +463,45 @@ void xhci_find_new_dequeue_state(struct xhci_hcd
>>> *xhci,
>>> hw_dequeue = le64_to_cpu(ep_ctx->deq);
>>> }
>>>
>>> - /* Find virtual address and segment of hardware dequeue pointer */
>>> - state->new_deq_seg = ep_ring->deq_seg;
>>> - state->new_deq_ptr = ep_ring->dequeue;
>>> - while (xhci_trb_virt_to_dma(state->new_deq_seg, state->new_deq_ptr)
>>> - != (dma_addr_t)(hw_dequeue & ~0xf)) {
>>> - next_trb(xhci, ep_ring, &state->new_deq_seg,
>>> - &state->new_deq_ptr);
>>> - if (state->new_deq_ptr == ep_ring->dequeue) {
>>> - WARN_ON(1);
>>> - return;
>>> - }
>>> - }
>>> + new_seg = ep_ring->deq_seg;
>>> + new_deq = ep_ring->dequeue;
>>> + state->new_cycle_state = hw_dequeue & 0x1;
>>> +
>>> /*
>>> -* Find cycle state for last_trb, starting at old cycle state of
>>> -* hw_dequeue. If there is only one segment ring, find_trb_seg() will
>>> -* return immediately and cannot toggle the cycle state if this search
>>> -* wraps around, so add one more toggle manually in that case.
>>> +* We want to find the pointer, segment and cycle state of the new trb
>>> +* (the one after current TD's last_trb). We know the cycle state at
>>> +* hw_dequeue, so walk the ring until both hw_dequeue and last_trb are
>>> +* found.
>>>
Re: [PATCH] xhci-ring: Fix Null pointer dereference
At least I have seen the issue on Debian 3.14 and 3.16. Is your patch going to be backported to linux-stable? The computer crashes very very badly On Wed, Aug 27, 2014 at 4:25 PM, Mathias Nyman wrote: > On 08/26/2014 06:47 PM, Ricardo Ribalda Delgado wrote: >> While testing a usb gadget I managed to crash completely the host >> computer. This was due to a NULL pointer derefence. >> >> This patch avoids the crash although the kernel still outputs some >> warnings. >> >> Without this patch, kernels from (at least) 3.14 can be crashed with >> mass storage gadgets. >> >> Affected host: NEC Corporation uPD720200 USB 3.0 >> > > > This should not be necessary anymore after > commit 365038d83313951d6ace15342eb24624bbef1666 > xhci: rework cycle bit checking for new dequeue pointers > > http://marc.info/?l=linux-usb&m=140844993115671&w=2 > > Which was just added to Greg's usb-linus branch. > It checks that the new_deq_ptr and new_deq_seg are valid before calling > xhci_queue_new_dequeue_state() > > -Mathias > > > > > -- Ricardo Ribalda -- 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] xhci-ring: Fix Null pointer dereference
On 08/26/2014 06:47 PM, Ricardo Ribalda Delgado wrote: > While testing a usb gadget I managed to crash completely the host > computer. This was due to a NULL pointer derefence. > > This patch avoids the crash although the kernel still outputs some > warnings. > > Without this patch, kernels from (at least) 3.14 can be crashed with > mass storage gadgets. > > Affected host: NEC Corporation uPD720200 USB 3.0 > This should not be necessary anymore after commit 365038d83313951d6ace15342eb24624bbef1666 xhci: rework cycle bit checking for new dequeue pointers http://marc.info/?l=linux-usb&m=140844993115671&w=2 Which was just added to Greg's usb-linus branch. It checks that the new_deq_ptr and new_deq_seg are valid before calling xhci_queue_new_dequeue_state() -Mathias -- 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: Buffer I/O error after s2ram with usb storage
On 14-08-27 04:40 AM, Matthieu CASTET wrote:
Ping
I have got also a problem with a usb sdcard reader (without power cut
during suspend)
[ 1073.606668] PM: Entering mem sleep
[ 1073.606742] Suspending console(s) (use no_console_suspend to debug)
[ 1073.607146] sd 1:0:0:0: [sda] Synchronizing SCSI cache
[ 1073.639076] sd 1:0:0:0: [sda] Stopping disk
[ 1074.186688] PM: suspend of devices complete after 580.127 msecs
[...]
[ 1075.265183] PM: resume of devices complete after 615.990 msecs
[ 1075.265627] PM: Finishing wakeup.
[ 1075.265630] Restarting tasks ...
[...]
[ 1203.404593] EXT4-fs error (device sdb6): ext4_mb_generate_buddy:756: group
6, 29065 clusters in bitmap, 32768 in gd; block bitmap corrupt.
[ 1203.404628] EXT4-fs error (device sdb6): ext4_mb_generate_buddy:756: group
5, 1601 clusters in bitmap, 32321 in gd; block bitmap corrupt.
[ 1203.404648] EXT4-fs error (device sdb6): ext4_mb_generate_buddy:756: group
4, 0 clusters in bitmap, 32768 in gd; block bitmap corrupt.
[ 1203.404667] EXT4-fs error (device sdb6): ext4_mb_generate_buddy:756: group
3, 1601 clusters in bitmap, 32321 in gd; block bitmap corrupt.
[ 1203.404686] EXT4-fs error (device sdb6): ext4_mb_generate_buddy:756: group
2, 0 clusters in bitmap, 32768 in gd; block bitmap corrupt.
[ 1203.404705] EXT4-fs error (device sdb6): ext4_mb_generate_buddy:756: group
1, 1600 clusters in bitmap, 32321 in gd; block bitmap corrupt.
[ 1203.404726] JBD2: Spotted dirty metadata buffer (dev = sdb6, blocknr = 0).
There's a risk of filesystem corruption in case of system crash.
[ 1204.141482] sd 8:0:0:0: [sdb] Media Changed
[ 1204.141490] sd 8:0:0:0: [sdb]
[ 1204.141494] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
[ 1204.141497] sd 8:0:0:0: [sdb]
[ 1204.141499] Sense Key : Unit Attention [current]
[ 1204.141504] Info fld=0x0
[ 1204.141506] sd 8:0:0:0: [sdb]
[ 1204.141510] Add. Sense: Not ready to ready change, medium may have changed
The unit attention doesn't look like a problem, it
looks correct. If the system is unable to detect
removable media being changed while the system is
suspended, then
If the media has a unique identifier, then this unit
attention at wakeup should trigger sd to make sure
that unique identifier has not changed.
Not sure why ext4 starts looking at sdb6 _before_ the
sd driver processes that unit attention. Perhaps a
TEST UNIT READY should be done earlier in the wake-up
sequence to flush out (and process) unit attentions.
There is also the case in which the removable media is
no longer present; and that should change EXT4-fs
processing to a surprise removal.
Doug Gilbert
[ 1204.141514] sd 8:0:0:0: [sdb] CDB:
[ 1204.141516] Read(10): 28 00 00 0a 75 f8 00 00 08 00
[ 1204.141526] end_request: I/O error, dev sdb, sector 685560
Le Mon, 28 Apr 2014 15:01:39 +0200,
Matthieu CASTET a écrit :
Hi,
any news on this.
Matthieu CASTET
Le Tue, 22 Apr 2014 16:01:15 +0200,
Matthieu CASTET a écrit :
Hi,
while playing with suspend to ram I found a strange behavior with usb
key.
This can be easily reproduced by doing :
- plug a usb key
- start to read the usb key : "cat /dev/sdx > /dev/null"
- go to suspend : "echo mem > /sys/power/state"
- while in suspend, unplug and replug the usb key (simulate usb power
loss for computer that keep power)
- exit suspend
- there is read error on the usb key
Because the power was cut during s2ram, the usb stack reset the device
<1>.
When new data transfer are done, we got a UNIT_ATTENTION from the
device <2> and IO error are returned to user space application.
After some investigation it seems some (all on the 3 I tested) usb key
report as removable, and scsi layer abort the transfer in case of
UNIT_ATTENTION <3>.
The usb storage driver call scsi_report_bus_reset after device reset,
but because of commit dfcf7775 <4>, we don't ignore unit attention if
"sshdr.asc == 0x28 && sshdr.ascq == 0x00" ("Not-ready to ready").
If dfcf7775 is reverted there is no more Buffer I/O error.
Is that possible to find a way to restore the behavior before dfcf7775
commit (no Buffer I/O error after device reset) after a suspend to ram ?
Matthieu CASTET
PS : the same error happen if sg_reset -b /dev/sdx is used on the
device.
<1>
[ 117.070255] usb 2-1.4: reset high-speed USB device number 3 using
ehci-pci [...]
[ 117.543922] Restarting tasks ... done.
[ 117.548390] video LNXVIDEO:01: Restoring backlight state
<2>
[ 117.549179] sd 6:0:0:0: [sdb] Media Changed
[ 117.549184] sd 6:0:0:0: [sdb]
[ 117.549187] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
[ 117.549189] sd 6:0:0:0: [sdb]
[ 117.549191] Sense Key : Unit Attention [current]
[ 117.549195] Info fld=0x0
[ 117.549197] sd 6:0:0:0: [sdb]
[ 117.549201] Add. Sense: Not ready to ready change, medium may have
changed [ 117.549203] sd 6:0:0:0: [sdb] CDB:
[ 117.549205] Read(10): 28 00 00 02 c9 00 00 00 f0 00
[ 117.549212] end_request: I/O error, dev sdb, sector 182528
[ 117.549218] Buffer I/O error on device sdb1, logical block 22
[PATCH v3 0/2] LED triggers for USB host and device
This adds LED triggers for USB host and device. First patch refactors UDC drivers as requested by Felipe Balbi, second patch adds the LED triggers. Changes from v2: - host/gadget triggers merged to a single file in usb/common/ - udc drivers refactored so that LED trigger works for all of them Changes from v1: - Moved from drivers/leds/ to drivers/usb/ - Improved Kconfig help - Linked with other modules rather than being standalone modules Michal Sojka (2): usb: gadget: Refactor request completion usb: Add LED triggers for USB activity drivers/usb/Kconfig | 11 +++ drivers/usb/chipidea/udc.c | 6 +- drivers/usb/common/Makefile | 5 +- drivers/usb/common/common.c | 144 drivers/usb/common/led.c| 59 + drivers/usb/common/usb-common.c | 144 drivers/usb/core/hcd.c | 2 + drivers/usb/dwc2/gadget.c | 6 +- drivers/usb/dwc3/gadget.c | 2 +- drivers/usb/gadget/udc/amd5536udc.c | 2 +- drivers/usb/gadget/udc/at91_udc.c | 2 +- drivers/usb/gadget/udc/atmel_usba_udc.c | 4 +- drivers/usb/gadget/udc/bcm63xx_udc.c| 2 +- drivers/usb/gadget/udc/dummy_hcd.c | 10 +-- drivers/usb/gadget/udc/fotg210-udc.c| 2 +- drivers/usb/gadget/udc/fsl_qe_udc.c | 6 +- drivers/usb/gadget/udc/fsl_udc_core.c | 6 +- drivers/usb/gadget/udc/fusb300_udc.c| 2 +- drivers/usb/gadget/udc/goku_udc.c | 2 +- drivers/usb/gadget/udc/gr_udc.c | 2 +- drivers/usb/gadget/udc/lpc32xx_udc.c| 2 +- drivers/usb/gadget/udc/m66592-udc.c | 2 +- drivers/usb/gadget/udc/mv_u3d_core.c| 8 +- drivers/usb/gadget/udc/mv_udc_core.c| 8 +- drivers/usb/gadget/udc/net2272.c| 2 +- drivers/usb/gadget/udc/net2280.c| 2 +- drivers/usb/gadget/udc/omap_udc.c | 2 +- drivers/usb/gadget/udc/pch_udc.c| 2 +- drivers/usb/gadget/udc/pxa25x_udc.c | 2 +- drivers/usb/gadget/udc/pxa27x_udc.c | 2 +- drivers/usb/gadget/udc/r8a66597-udc.c | 2 +- drivers/usb/gadget/udc/s3c-hsudc.c | 3 +- drivers/usb/gadget/udc/s3c2410_udc.c| 2 +- drivers/usb/gadget/udc/udc-core.c | 13 +++ drivers/usb/musb/musb_gadget.c | 2 +- drivers/usb/renesas_usbhs/mod_gadget.c | 2 +- include/linux/usb.h | 12 +++ include/linux/usb/gadget.h | 8 ++ 38 files changed, 294 insertions(+), 201 deletions(-) create mode 100644 drivers/usb/common/common.c create mode 100644 drivers/usb/common/led.c delete mode 100644 drivers/usb/common/usb-common.c -- 2.1.0 -- 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
