RE: [PATCH net-next 0/4] r8152: firmware support
From: David Miller [mailto:da...@davemloft.net]
[...]
That still doesn't convince me.
The functions I see you removing are just programming a set of
registers in some way.
That is to clear the break point of the firmware. If a firmware exists,
you should clear it before updating a new one.
And the firmware that is replacing those functions is just going to be
causing the same register writes, just even more obfuscated than it is
now.
You should keep the C functions which document and show clearly what
is being programmed in each chip.
Don't hide register programming behind firmware files, please.
Excuse me. Some settings are relative the content of the firmware.
How should I deal with that parts. Take 8153 (RTL_VER_03) for
example.
1. wait PLA 0xb800 bit 6 = 1.
2. set patch key = 0x7000.
3. update the PHY firmware.
4. enable the firmware.
5. set USB 0xcfca bit 14 = 0.
6. clear break point (That is r8153_clear_bp()).
7. load the firmware about PLA and USB parts.
8. set the break point of the firmware.
9. set USB 0xcfca bit 14 = 1.
Except the step 3, 4, 6 and 7, the other steps depend on the
context of the firmware. That is, for different firmware, some
actions would be removed or added, and some settings would be
different. Especially the step 8, it often different for
different firmwares. Should I add some firmware version check
in the source code? Such as
if (fw_version == v1) {
...
load firmware
set break point of the firmware
...
} else if (fw_version == v2) {
...--
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 0/4] r8152: firmware support
From: Hayes Wang hayesw...@realtek.com Date: Mon, 25 Aug 2014 06:43:02 + Except the step 3, 4, 6 and 7, the other steps depend on the context of the firmware. That is, for different firmware, some actions would be removed or added, and some settings would be different. Especially the step 8, it often different for different firmwares. Should I add some firmware version check in the source code? This is extremely poor design of the firmware, adding such constantly changing dependencies and constantly changing programming sequences just to get the firmare executing is a terrible idea. You really need to sanitize this in some way, because what you have posted is totally unacceptable to me. -- 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/4] usb: gadget: f_uac2: cleanups and capture timing
Daniel Mack wrote: a) Linux snd-usb-audio currently pre-calculates the estimated packet sizes to expect from a USB device, and will only receive packets that have the expected size or are smaller. snd-usb-audio allows packets to be 25 % larger. This can be worked around by setting the UAC_EP_CS_ATTR_FILL_MAX in the UAC2 endpoint descriptor The spec says about this flag (4.10.1.2): | when receiving data from an IN endpoint, the Host software must be | prepared to receive wMaxPacketSize bytes and discard any superfluous | zero bytes in the packet. | Note: This bit can only be used for Type II formatted data. The data |stream must contain enough information (in a header) to |separate the actual data from the padded zero bytes. send 0-byte packets from agdev_iso_complete() if the time passed since the last completed buffer does not allow for another one to be sent. Audio Formats, 2.1: | To indicate a temporary stop in the isochronous data stream ..., an | in-band Transfer Delimiter needs to be defined. This specification | considers two situations to be a Transfer Delimiter. The first is | a zero-length data packet and the second is the absence of an | isochronous transfer 2.3.1.1: | The goal must be to keep the instantaneous number of audio slots per | virtual frame, ni as close as possible to the average number of audio | slots per virtual frame, nav. [...] If the sampling rate is a constant, | the allowable variation on ni is limited to one audio slot, that is, | ∆ni = 1. This implies that all virtual frame packets must either | contain INT(nav) audio slots (small VFP) or INT(nav) + 1 (large VFP) | audio slots. This results in very stable timing behavior in my tests. But it increases jitter, and might not work with any other driver. f_uac(2) *must* implement some mechanism to control the clock, i.e., the packet sizes. (And this is not part of the standard ALSA API.) Regards, Clemens -- 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 4/6] phy: remove the old lookup method
Hi Heikki,
On Thu, Aug 21, 2014 at 5:03 PM, Heikki Krogerus
heikki.kroge...@linux.intel.com wrote:
The users of the old method are now converted to the new one.
Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
Tested-by: Vivek Gautam gautam.vi...@samsung.com
---
drivers/phy/phy-bcm-kona-usb2.c | 2 +-
drivers/phy/phy-core.c | 45
+++--
drivers/phy/phy-exynos-dp-video.c | 2 +-
drivers/phy/phy-exynos-mipi-video.c | 2 +-
drivers/phy/phy-exynos5-usbdrd.c| 3 +--
drivers/phy/phy-exynos5250-sata.c | 2 +-
drivers/phy/phy-mvebu-sata.c| 2 +-
drivers/phy/phy-omap-usb2.c | 2 +-
drivers/phy/phy-samsung-usb2.c | 3 +--
drivers/phy/phy-sun4i-usb.c | 2 +-
drivers/phy/phy-ti-pipe3.c | 2 +-
drivers/phy/phy-twl4030-usb.c | 4 +---
drivers/phy/phy-xgene.c | 2 +-
include/linux/phy/phy.h | 38 ---
14 files changed, 19 insertions(+), 92 deletions(-)
Please squash the attached diff which removes the 'init_data' field
from some of the other instances
of devm_phy_create() in few other drivers.
This should prevent any build errors that i could see with multi_v7_defconfig.
diff --git a/drivers/phy/phy-bcm-kona-usb2.c b/drivers/phy/phy-bcm-kona-usb2.c
index 894fe74..3463983 100644
--- a/drivers/phy/phy-bcm-kona-usb2.c
+++ b/drivers/phy/phy-bcm-kona-usb2.c
@@ -117,7 +117,7 @@ static int bcm_kona_usb2_probe(struct platform_device
*pdev)
platform_set_drvdata(pdev, phy);
- gphy = devm_phy_create(dev, NULL, ops, NULL);
+ gphy = devm_phy_create(dev, NULL, ops);
if (IS_ERR(gphy))
return PTR_ERR(gphy);
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 67a8c726..834b337 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -55,36 +55,6 @@ static int devm_phy_match(struct device *dev, void *res,
void *match_data)
return res == match_data;
}
-static struct phy *phy_lookup(struct device *device, const char *port)
-{
- unsigned int count;
- struct phy *phy;
- struct device *dev;
- struct phy_consumer *consumers;
- struct class_dev_iter iter;
-
- class_dev_iter_init(iter, phy_class, NULL, NULL);
- while ((dev = class_dev_iter_next(iter))) {
- phy = to_phy(dev);
-
- if (!phy-init_data)
- continue;
- count = phy-init_data-num_consumers;
- consumers = phy-init_data-consumers;
- while (count--) {
- if (!strcmp(consumers-dev_name, dev_name(device))
- !strcmp(consumers-port, port)) {
- class_dev_iter_exit(iter);
- return phy;
- }
- consumers++;
- }
- }
-
- class_dev_iter_exit(iter);
- return ERR_PTR(-ENODEV);
-}
-
/**
* phy_register_lookup() - register PHY/device association
* @pl: association to register
@@ -210,10 +180,6 @@ static struct phy *phy_find(struct device *dev, const
char *con_id)
}
class_dev_iter_exit(iter);
}
-
- /* fall-back to the old lookup method for now */
- if (IS_ERR(phy))
- phy = phy_lookup(dev, con_id);
return phy;
}
@@ -721,13 +687,11 @@ EXPORT_SYMBOL_GPL(devm_of_phy_get);
* @dev: device that is creating the new phy
* @node: device node of the phy
* @ops: function pointers for performing phy operations
- * @init_data: contains the list of PHY consumers or NULL
*
* Called to create a phy using phy framework.
*/
struct phy *phy_create(struct device *dev, struct device_node *node,
- const struct phy_ops *ops,
- struct phy_init_data *init_data)
+ const struct phy_ops *ops)
{
int ret;
int id;
@@ -765,7 +729,6 @@ struct phy *phy_create(struct device *dev, struct
device_node *node,
phy-dev.of_node = node ?: dev-of_node;
phy-id = id;
phy-ops = ops;
- phy-init_data = init_data;
ret = dev_set_name(phy-dev, phy-%s.%d, dev_name(dev), id);
if (ret)
@@ -800,7 +763,6 @@ EXPORT_SYMBOL_GPL(phy_create);
* @dev: device that is creating the new phy
* @node: device node of the phy
* @ops: function pointers for performing phy operations
- * @init_data: contains the list of PHY consumers or NULL
*
* Creates a new PHY device adding it to the PHY class.
* While at that, it also associates the device with the phy using devres.
@@ -808,8 +770,7 @@ EXPORT_SYMBOL_GPL(phy_create);
* then, devres data is freed.
*/
struct phy *devm_phy_create(struct device *dev, struct device_node *node,
-
[PATCH net-next] r8152: check code with checkpatch.pl
626: CHECK: Alignment should match open parenthesis
646: CHECK: Alignment should match open parenthesis
655: CHECK: Alignment should match open parenthesis
695: CHECK: Alignment should match open parenthesis
729: CHECK: Alignment should match open parenthesis
739: CHECK: Alignment should match open parenthesis
976: WARNING: externs should be avoided in .c files
1314: CHECK: Alignment should match open parenthesis
1358: WARNING: networking block comments don't use an empty /* line, use /*
Comment...
1402: WARNING: networking block comments don't use an empty /* line, use /*
Comment...
1521: CHECK: multiple assignments should be avoided
1775: CHECK: Alignment should match open parenthesis
1838: CHECK: multiple assignments should be avoided
1843: CHECK: multiple assignments should be avoided
1847: CHECK: multiple assignments should be avoided
1850: WARNING: Missing a blank line after declarations
1864: CHECK: Alignment should match open parenthesis
1872: CHECK: braces {} should be used on all arms of this statement
1906: CHECK: usleep_range is preferred over udelay
2865: WARNING: networking block comments don't use an empty /* line, use /*
Comment...
3088: CHECK: Alignment should match open parenthesis
total: 0 errors, 5 warnings, 16 checks, 3567 lines checked
Signed-off-by: Hayes Wang hayesw...@realtek.com
---
drivers/net/usb/r8152.c | 66 ++---
1 file changed, 35 insertions(+), 31 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 87f7104..2470d9c 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -623,8 +623,8 @@ int get_registers(struct r8152 *tp, u16 value, u16 index,
u16 size, void *data)
return -ENOMEM;
ret = usb_control_msg(tp-udev, usb_rcvctrlpipe(tp-udev, 0),
- RTL8152_REQ_GET_REGS, RTL8152_REQT_READ,
- value, index, tmp, size, 500);
+ RTL8152_REQ_GET_REGS, RTL8152_REQT_READ,
+ value, index, tmp, size, 500);
memcpy(data, tmp, size);
kfree(tmp);
@@ -643,8 +643,8 @@ int set_registers(struct r8152 *tp, u16 value, u16 index,
u16 size, void *data)
return -ENOMEM;
ret = usb_control_msg(tp-udev, usb_sndctrlpipe(tp-udev, 0),
- RTL8152_REQ_SET_REGS, RTL8152_REQT_WRITE,
- value, index, tmp, size, 500);
+ RTL8152_REQ_SET_REGS, RTL8152_REQT_WRITE,
+ value, index, tmp, size, 500);
kfree(tmp);
@@ -652,7 +652,7 @@ int set_registers(struct r8152 *tp, u16 value, u16 index,
u16 size, void *data)
}
static int generic_ocp_read(struct r8152 *tp, u16 index, u16 size,
- void *data, u16 type)
+ void *data, u16 type)
{
u16 limit = 64;
int ret = 0;
@@ -692,7 +692,7 @@ static int generic_ocp_read(struct r8152 *tp, u16 index,
u16 size,
}
static int generic_ocp_write(struct r8152 *tp, u16 index, u16 byteen,
- u16 size, void *data, u16 type)
+u16 size, void *data, u16 type)
{
int ret;
u16 byteen_start, byteen_end, byen;
@@ -726,8 +726,8 @@ static int generic_ocp_write(struct r8152 *tp, u16 index,
u16 byteen,
while (size) {
if (size limit) {
ret = set_registers(tp, index,
- type | BYTE_EN_DWORD,
- limit, data);
+ type | BYTE_EN_DWORD,
+ limit, data);
if (ret 0)
goto error1;
@@ -736,8 +736,8 @@ static int generic_ocp_write(struct r8152 *tp, u16 index,
u16 byteen,
size -= limit;
} else {
ret = set_registers(tp, index,
- type | BYTE_EN_DWORD,
- size, data);
+ type | BYTE_EN_DWORD,
+ size, data);
if (ret 0)
goto error1;
@@ -972,8 +972,8 @@ void write_mii_word(struct net_device *netdev, int phy_id,
int reg, int val)
usb_autopm_put_interface(tp-intf);
}
-static
-int r8152_submit_rx(struct r8152 *tp, struct rx_agg *agg, gfp_t mem_flags);
+static int
+r8152_submit_rx(struct r8152 *tp, struct rx_agg *agg, gfp_t mem_flags);
static inline void set_ethernet_addr(struct r8152 *tp)
{
@@ -1311,8 +1311,8 @@ static int alloc_all_mem(struct r8152 *tp)
tp-intr_interval =
Re: [PATCH v5 1/3] usb: gadget: f_fs: fix the redundant ep files problem
On 08/24/2014 04:14 PM, Michal Nazarewicz wrote:
On Thu, Aug 21 2014, Robert Baldyga r.bald...@samsung.com wrote:
Up to now, when endpoint addresses in descriptors were non-consecutive,
there were created redundant files, which could cause problems in kernel,
when user tryed to read/write to them. It was result of fact that maximum
^ -- tried
endpoint address was taken as total number of endpoints in funciton.
This patch adds endpoint descriptors counting and storing their addresses
in eps_addrmap to verify their cohesion in each speed.
Endpoint address map would be also useful for further features, just like
vitual endpoint address mapping.
Signed-off-by: Robert Baldyga r.bald...@samsung.com
Acked-by: Michal Nazarewicz min...@mina86.com
@@ -1853,14 +1860,23 @@ static int __ffs_data_do_entity(enum ffs_entity_type
type,
* Strings are indexed from 1 (0 is magic ;) reserved
* for languages list or some such)
*/
-if (*valuep ffs-strings_count)
-ffs-strings_count = *valuep;
+if (*valuep helper-ffs-strings_count)
+helper-ffs-strings_count = *valuep;
break;
case FFS_ENDPOINT:
-/* Endpoints are indexed from 1 as well. */
-if ((*valuep USB_ENDPOINT_NUMBER_MASK) ffs-eps_count)
-ffs-eps_count = (*valuep USB_ENDPOINT_NUMBER_MASK);
+d = (void *)desc;
+helper-eps_count++;
+if (helper-eps_count = 15)
+return -EINVAL;
+if (!helper-ffs-eps_count !helper-ffs-interfaces_count)
I'd check helper-ffs-eps_count only. helper-ffs-interfaces_count is
zero only because endpoints and interfaces are interpret at the same
time, but otherwise the interfaces_count is unrelated to what we're
doing here.
Besides simple descriptor counting there are done checks if endpoints
number and their addressed are identical for each speed. For this reason
we need to know inside this function if descriptors for any speed was
already done. If it's true, we check if endpoint descriptors for current
speed has proper addresses.
There is possibility that interface has no endpoints (endpoint 0 only),
so we can recognize that descriptors for one speed were already parsed
only basing on interfaces_count. In that case if FFS_ENDPOINT will
appear, eps_count will exceed number of endpoints in previously parsed
descriptors and error will be returned.
Also, perhaps adding a comment describing what !helper-ffs-eps_count
means makes sense here.
+helper-ffs-eps_addrmap[helper-eps_count] =
+d-bEndpointAddress;
+else if (helper-ffs-eps_count helper-eps_count)
+return -EINVAL;
Doesn't this duplicate check in __ffs_data_got_descs?
+else if (helper-ffs-eps_addrmap[helper-eps_count] !=
+d-bEndpointAddress)
+return -EINVAL;
break;
}
@@ -2101,13 +2118,29 @@ static int __ffs_data_got_descs(struct ffs_data *ffs,
/* Read descriptors */
raw_descs = data;
+helper.ffs = ffs;
for (i = 0; i 3; ++i) {
if (!counts[i])
continue;
+helper.interfaces_count = 0;
+helper.eps_count = 0;
ret = ffs_do_descs(counts[i], data, len,
- __ffs_data_do_entity, ffs);
+ __ffs_data_do_entity, helper);
if (ret 0)
goto error;
+if (!ffs-eps_count !ffs-interfaces_count) {
+ffs-eps_count = helper.eps_count;
+ffs-interfaces_count = helper.interfaces_count;
+} else {
+if (ffs-eps_count != helper.eps_count) {
+ret = -EINVAL;
+goto error;
+}
+if (ffs-interfaces_count != helper.interfaces_count) {
+ret = -EINVAL;
+goto error;
+}
+}
data += ret;
len -= ret;
}
@@ -2342,9 +2375,18 @@ static void ffs_event_add(struct ffs_data *ffs,
spin_unlock_irqrestore(ffs-ev.waitq.lock, flags);
}
-
/* Bind/unbind USB function hooks
***/
+static int ffs_ep_addr2idx(struct ffs_data *ffs, u8 endpoint_address)
+{
+int i;
+
+for (i = 1; i 15; ++i)
+ for (i = 1; i ARRAY_SIZE(ffs-eps_addrmap); ++i)
+if (ffs-eps_addrmap[i] == endpoint_address)
+return i;
+return -ENOENT;
+}
+
static int __ffs_func_bind_do_descs(enum ffs_entity_type type, u8 *valuep,
struct usb_descriptor_header *desc,
Re: [PATCH 4/6] phy: remove the old lookup method
On Mon, Aug 25, 2014 at 01:11:34PM +0530, Vivek Gautam wrote: Please squash the attached diff which removes the 'init_data' field from some of the other instances of devm_phy_create() in few other drivers. This should prevent any build errors that i could see with multi_v7_defconfig. OK, I'll add those changes into this patch, and it seems there is now at least one more new driver on top of those calling devm_phy_create. Thanks! -- heikki -- 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 4/6] phy: remove the old lookup method
On Mon, Aug 25, 2014 at 1:47 PM, Heikki Krogerus heikki.kroge...@linux.intel.com wrote: On Mon, Aug 25, 2014 at 01:11:34PM +0530, Vivek Gautam wrote: Please squash the attached diff which removes the 'init_data' field from some of the other instances of devm_phy_create() in few other drivers. This should prevent any build errors that i could see with multi_v7_defconfig. OK, I'll add those changes into this patch, and it seems there is now at least one more new driver on top of those calling devm_phy_create. Ok, i think i just saw this on 3.17-rc1. -- Best Regards Vivek Gautam Samsung RD 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 v3] drivers: usb: gadget: fusb300_udc.h: Fix typo in include guard
Clearly this was meant to be an include guard, but a trailing underscore was missing. It has been this way since the file was introduced in 0fe6f1d1 (usb: udc: add Faraday fusb300 driver). Fixes: 0fe6f1d1 (usb: udc: add Faraday fusb300 driver) Cc: sta...@vger.kernel.org Signed-off-by: Rasmus Villemoes li...@rasmusvillemoes.dk --- Notes: v2: Add proper commit message. v3: Add Fixes: and Cc: stable tags as requested by Felipe Balbi. drivers/usb/gadget/udc/fusb300_udc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/gadget/udc/fusb300_udc.h b/drivers/usb/gadget/udc/fusb300_udc.h index ae811d8..ad39f89 100644 --- a/drivers/usb/gadget/udc/fusb300_udc.h +++ b/drivers/usb/gadget/udc/fusb300_udc.h @@ -12,7 +12,7 @@ #ifndef __FUSB300_UDC_H__ -#define __FUSB300_UDC_H_ +#define __FUSB300_UDC_H__ #include linux/kernel.h -- 2.0.4 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] usb: gadget: f_uac2: cleanups and capture timing
Hi Clemens, On 08/25/2014 09:15 AM, Clemens Ladisch wrote: Daniel Mack wrote: a) Linux snd-usb-audio currently pre-calculates the estimated packet sizes to expect from a USB device, and will only receive packets that have the expected size or are smaller. snd-usb-audio allows packets to be 25 % larger. Yes, but packets can't be larger than *that*. This can be worked around by setting the UAC_EP_CS_ATTR_FILL_MAX in the UAC2 endpoint descriptor The spec says about this flag (4.10.1.2): | when receiving data from an IN endpoint, the Host software must be | prepared to receive wMaxPacketSize bytes and discard any superfluous | zero bytes in the packet. | Note: This bit can only be used for Type II formatted data. The data |stream must contain enough information (in a header) to |separate the actual data from the padded zero bytes. Right, I've read this too, and we're not using Type II, so we don't have header information to tell us the real payload. The idea was to just use an 0 or 512 bytes approach. send 0-byte packets from agdev_iso_complete() if the time passed since the last completed buffer does not allow for another one to be sent. Audio Formats, 2.1: | To indicate a temporary stop in the isochronous data stream ..., an | in-band Transfer Delimiter needs to be defined. This specification | considers two situations to be a Transfer Delimiter. The first is | a zero-length data packet and the second is the absence of an | isochronous transfer 2.3.1.1: | The goal must be to keep the instantaneous number of audio slots per | virtual frame, ni as close as possible to the average number of audio | slots per virtual frame, nav. [...] If the sampling rate is a constant, | the allowable variation on ni is limited to one audio slot, that is, | ∆ni = 1. This implies that all virtual frame packets must either | contain INT(nav) audio slots (small VFP) or INT(nav) + 1 (large VFP) | audio slots. This results in very stable timing behavior in my tests. But it increases jitter, and might not work with any other driver. You're right, that's also my concern. f_uac(2) *must* implement some mechanism to control the clock, i.e., the packet sizes. (And this is not part of the standard ALSA API.) The easiest is probably really to just calculate correct packet sizes and stick to them. After all, the actual clock is really arbitrary, we just have to pick something that is in the range of the sample rate. I'll cook up an alternative patch and do some tests with Sebastian off-list. 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 RESEND] usb: chipidea: msm: Use USB PHY API to control PHY state
On Tue, 2014-08-19 at 14:06 -0500, Felipe Balbi wrote:
On Fri, Aug 15, 2014 at 12:21:19PM +0300, Ivan T. Ivanov wrote:
From: Ivan T. Ivanov iiva...@mm-sol.com
PHY drivers keep track of the current state of the hardware,
so don't change PHY settings under it.
Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com
looks correct to me from a PHY API perspective, so:
Acked-by: Felipe Balbi ba...@ti.com
Thanks.
However, it doesn't look like msm_phy_init() is equivalent to the lines
removes. Care to comment ?
What I have to actually do is just add phy_init(). No need to remove
controller reinitialization. Tested and is working. Will post 2 new
patches shortly.
Regards,
Ivan
---
drivers/usb/chipidea/ci_hdrc_msm.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c
b/drivers/usb/chipidea/ci_hdrc_msm.c
index d72b9d2..81de834 100644
--- a/drivers/usb/chipidea/ci_hdrc_msm.c
+++ b/drivers/usb/chipidea/ci_hdrc_msm.c
@@ -20,13 +20,11 @@
static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci, unsigned event)
{
struct device *dev = ci-gadget.dev.parent;
- int val;
switch (event) {
case CI_HDRC_CONTROLLER_RESET_EVENT:
dev_dbg(dev, CI_HDRC_CONTROLLER_RESET_EVENT received\n);
- writel(0, USB_AHBBURST);
- writel(0, USB_AHBMODE);
+ usb_phy_init(ci-transceiver);
break;
case CI_HDRC_CONTROLLER_STOPPED_EVENT:
dev_dbg(dev, CI_HDRC_CONTROLLER_STOPPED_EVENT received\n);
@@ -34,10 +32,7 @@ static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci,
unsigned event)
* Put the transceiver in non-driving mode. Otherwise host
* may not detect soft-disconnection.
*/
- val = usb_phy_io_read(ci-transceiver, ULPI_FUNC_CTRL);
- val = ~ULPI_FUNC_CTRL_OPMODE_MASK;
- val |= ULPI_FUNC_CTRL_OPMODE_NONDRIVING;
- usb_phy_io_write(ci-transceiver, val, ULPI_FUNC_CTRL);
+ usb_phy_notify_disconnect(ci-transceiver, USB_SPEED_UNKNOWN);
break;
default:
dev_dbg(dev, unknown ci_hdrc event\n);
--
1.8.3.2
--
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 02/16] libusbg: Add guards for libconfig version
-Original Message- From: Sergei Shtylyov [mailto:sergei.shtyl...@cogentembedded.com] Sent: Friday, August 22, 2014 2:11 PM To: Krzysztof Opasiak; matt.por...@linaro.org; linux- u...@vger.kernel.org Cc: s.wa...@samsung.com; k.lewando...@samsung.com; andrze...@samsung.com; m.szyprow...@samsung.com; philippedesw...@gmail.com Subject: Re: [PATCH v2 02/16] libusbg: Add guards for libconfig version Hello. On 8/22/2014 3:59 PM, Krzysztof Opasiak wrote: Changes sent to libcofngi has not been merged yet, Perhaps libconfig? Definitely yes. But even more, this commit should not be here. It my mistake and it should be moved to other series together with unmerged libconfig functionality. Will be fixed in v3. so add compatibility defines to make all compile and work. Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com [...] WBR, Sergei -- 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 v3 12/15] libusbg: Add import gadget functionality
Whole gadget can be exported to file using usbg_export_gadget().
This commit adds complementary API function usbg_import_gadget()
which allows to import whole gadget (including attrs, strings,
configs, functions and bindings) from file to configfs.
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
include/usbg/usbg.h | 12 ++
src/usbg.c | 348 +++
2 files changed, 360 insertions(+)
diff --git a/include/usbg/usbg.h b/include/usbg/usbg.h
index 70b9910..23783eb 100644
--- a/include/usbg/usbg.h
+++ b/include/usbg/usbg.h
@@ -951,6 +951,18 @@ extern int usbg_import_function(usbg_gadget *g, FILE
*stream,
*/
extern int usbg_import_config(usbg_gadget *g, FILE *stream, int id,
usbg_config **c);
+/**
+ * @brief Imports usb gadget from file
+ * @param s current state of library
+ * @param stream from which gadget should be imported
+ * @param name which shuld be used for new gadget
+ * @param g place for pointer to imported gadget
+ * if NULL this param will be ignored.
+ * @return 0 on success, usbg_error otherwise
+ */
+extern int usbg_import_gadget(usbg_state *s, FILE *stream,
+ const char *name, usbg_gadget **g);
+/**
* @}
*/
#endif /* __USBG_H__ */
diff --git a/src/usbg.c b/src/usbg.c
index 4178194..37d2954 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -3768,6 +3768,311 @@ error2:
return ret;
}
+static int usbg_import_gadget_configs(config_setting_t *root, usbg_gadget *g)
+{
+ config_setting_t *node, *id_node;
+ int usbg_ret, cfg_ret;
+ int id;
+ usbg_config *c;
+ int ret = USBG_SUCCESS;
+ int count, i;
+
+ count = config_setting_length(root);
+
+ for (i = 0; i count; ++i) {
+ node = config_setting_get_elem(root, i);
+ if (!node) {
+ ret = USBG_ERROR_OTHER_ERROR;
+ break;
+ }
+
+ if (!config_setting_is_group(node)) {
+ ret = USBG_ERROR_INVALID_TYPE;
+ break;
+ }
+
+ /* Look for id */
+ id_node = config_setting_get_member(node, USBG_ID_TAG);
+ if (!id_node) {
+ ret = USBG_ERROR_MISSING_TAG;
+ break;
+ }
+
+ if (!usbg_config_is_int(id_node)) {
+ ret = USBG_ERROR_INVALID_TYPE;
+ break;
+ }
+
+ id = config_setting_get_int(id_node);
+
+ ret = usbg_import_config_run(g, node, id, c);
+ if (ret != USBG_SUCCESS)
+ break;
+ }
+
+ return ret;
+}
+
+static int usbg_import_gadget_functions(config_setting_t *root, usbg_gadget *g)
+{
+ config_setting_t *node, *inst_node;
+ int usbg_ret, cfg_ret;
+ const char *instance;
+ const char *label;
+ usbg_function *f;
+ int ret = USBG_SUCCESS;
+ int count, i;
+
+ count = config_setting_length(root);
+
+ for (i = 0; i count; ++i) {
+ node = config_setting_get_elem(root, i);
+ if (!node) {
+ ret = USBG_ERROR_OTHER_ERROR;
+ break;
+ }
+
+ if (!config_setting_is_group(node)) {
+ ret = USBG_ERROR_INVALID_TYPE;
+ break;
+ }
+
+ /* Look for instance name */
+ inst_node = config_setting_get_member(node, USBG_INSTANCE_TAG);
+ if (!inst_node) {
+ ret = USBG_ERROR_MISSING_TAG;
+ break;
+ }
+
+ if (!usbg_config_is_string(inst_node)) {
+ ret = USBG_ERROR_INVALID_TYPE;
+ break;
+ }
+
+ instance = config_setting_get_string(inst_node);
+ if (!instance) {
+ ret = USBG_ERROR_OTHER_ERROR;
+ break;
+ }
+
+ ret = usbg_import_function_run(g, node, instance, f);
+ if (ret != USBG_SUCCESS)
+ break;
+
+ /* Set the label given by user */
+ label = config_setting_name(node);
+ if (!label) {
+ ret = USBG_ERROR_OTHER_ERROR;
+ break;
+ }
+
+ f-label = strdup(label);
+ if (!f-label) {
+ ret = USBG_ERROR_NO_MEM;
+ break;
+ }
+ }
+
+ return ret;
+}
+
+static int usbg_import_gadget_strs_lang(config_setting_t *root, usbg_gadget *g)
+{
+ config_setting_t *node;
+ int lang;
+ const char *str;
+ usbg_gadget_strs g_strs = {0};
+ int usbg_ret, cfg_ret;
+ int ret = USBG_ERROR_INVALID_TYPE;
+
[PATCH v3 08/15] libusbg: Allow to store error information in usbg_gadget
If error occurred during parsing user should have
an oportunity to get details about place and type
of error.
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
src/usbg.c |8
1 file changed, 8 insertions(+)
diff --git a/src/usbg.c b/src/usbg.c
index cd526e1..a87a6b1 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -54,6 +54,7 @@ struct usbg_gadget
TAILQ_HEAD(chead, usbg_config) configs;
TAILQ_HEAD(fhead, usbg_function) functions;
usbg_state *parent;
+ config_t *last_failed_import;
};
struct usbg_config
@@ -559,6 +560,12 @@ static void usbg_free_gadget(usbg_gadget *g)
{
usbg_config *c;
usbg_function *f;
+
+ if (g-last_failed_import) {
+ config_destroy(g-last_failed_import);
+ free(g-last_failed_import);
+ }
+
while (!TAILQ_EMPTY(g-configs)) {
c = TAILQ_FIRST(g-configs);
TAILQ_REMOVE(g-configs, c, cnode);
@@ -596,6 +603,7 @@ static usbg_gadget *usbg_allocate_gadget(const char *path,
const char *name,
if (g) {
TAILQ_INIT(g-functions);
TAILQ_INIT(g-configs);
+ g-last_failed_import = NULL;
g-name = strdup(name);
g-path = strdup(path);
g-parent = parent;
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 06/15] libusbg: examples: Add sample application to export gadget
Add sample C code which shows how to use new functionality
of libusbg - gadget export. This program allows to export
chosen gadget from configfs to a file.
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
examples/Makefile.am |3 +-
examples/gadget-export.c | 81 ++
2 files changed, 83 insertions(+), 1 deletion(-)
create mode 100644 examples/gadget-export.c
diff --git a/examples/Makefile.am b/examples/Makefile.am
index abafe3c..aeb0273 100644
--- a/examples/Makefile.am
+++ b/examples/Makefile.am
@@ -1,7 +1,8 @@
-bin_PROGRAMS = show-gadgets gadget-acm-ecm gadget-vid-pid-remove gadget-ffs
+bin_PROGRAMS = show-gadgets gadget-acm-ecm gadget-vid-pid-remove gadget-ffs
gadget-export
gadget_acm_ecm_SOURCES = gadget-acm-ecm.c
show_gadgets_SOURCES = show-gadgets.c
gadget_vid_pid_remove_SOURCES = gadget-vid-pid-remove.c
gadget_ffs_SOURCES = gadget-ffs.c
+gadget_export_SOURCE = gadget-export.c
AM_CPPFLAGS=-I../include/
AM_LDFLAGS=-L../src/ -lusbg
diff --git a/examples/gadget-export.c b/examples/gadget-export.c
new file mode 100644
index 000..9d51e9e
--- /dev/null
+++ b/examples/gadget-export.c
@@ -0,0 +1,81 @@
+/*
+ * Copyright (C) 2014 Samsung Electronics
+ *
+ * Krzysztof Opasiak k.opas...@samsung.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+/**
+ * @file gadget-export.c
+ * @example gadget-export.c
+ * This is an example of how to export a gadget to file.
+ * Common reason of doing this is to share schema of gadget
+ * between different devices or preserve gadget between reboots.
+ */
+
+#include errno.h
+#include string.h
+#include stdio.h
+#include usbg/usbg.h
+
+int main(int argc, char **argv)
+{
+ usbg_state *s;
+ usbg_gadget *g;
+ int ret = -EINVAL;
+ int usbg_ret;
+ FILE *output;
+
+ if (argc != 3) {
+ fprintf(stderr, Usage: gadget-export gadget_name file_name\n);
+ return ret;
+ }
+
+ /* Prepare output file */
+ output = fopen(argv[2], w);
+ if (!output) {
+ fprintf(stderr, Error on fopen. Error: %s\n, strerror(errno));
+ goto out1;
+ }
+
+ /* Do gadget exporting */
+ usbg_ret = usbg_init(/sys/kernel/config, s);
+ if (usbg_ret != USBG_SUCCESS) {
+ fprintf(stderr, Error on USB gadget init\n);
+ fprintf(stderr, Error: %s : %s\n, usbg_error_name(usbg_ret),
+ usbg_strerror(usbg_ret));
+ goto out2;
+ }
+
+ g = usbg_get_gadget(s, argv[1]);
+ if (!g) {
+ fprintf(stderr, Error on get gadget\n);
+ goto out3;
+ }
+
+ usbg_ret = usbg_export_gadget(g, output);
+ if (usbg_ret != USBG_SUCCESS) {
+ fprintf(stderr, Error on export gadget\n);
+ fprintf(stderr, Error: %s : %s\n, usbg_error_name(usbg_ret),
+ usbg_strerror(usbg_ret));
+ goto out3;
+ }
+
+ ret = 0;
+
+out3:
+ usbg_cleanup(s);
+out2:
+ fclose(output);
+out1:
+ return ret;
+}
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 13/15] libusbg: Add functions to get import error text and line
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
include/usbg/usbg.h | 43 +++
src/usbg.c | 48
2 files changed, 91 insertions(+)
diff --git a/include/usbg/usbg.h b/include/usbg/usbg.h
index 23783eb..a934a7b 100644
--- a/include/usbg/usbg.h
+++ b/include/usbg/usbg.h
@@ -962,6 +962,49 @@ extern int usbg_import_config(usbg_gadget *g, FILE
*stream, int id,
*/
extern int usbg_import_gadget(usbg_state *s, FILE *stream,
const char *name, usbg_gadget **g);
+
+/**
+ * @brief Get text of error which occurred during last function import
+ * @param g gadget where function import error occurred
+ * @return Text of error or NULL if no error data
+ */
+extern const char *usbg_get_func_import_error_text(usbg_gadget *g);
+
+/**
+ * @brief Get line number where function import error occurred
+ * @param g gadget where function import error occurred
+ * @return line number or value below 0 if no error data
+ */
+extern int usbg_get_func_import_error_line(usbg_gadget *g);
+
+/**
+ * @brief Get text of error which occurred during last config import
+ * @param g gadget where config import error occurred
+ * @return Text of error or NULL if no error data
+ */
+extern const char *usbg_get_config_import_error_text(usbg_gadget *g);
+
+/**
+ * @brief Get line number where config import error occurred
+ * @param g gadget where config import error occurred
+ * @return line number or value below 0 if no error data
+ */
+extern int usbg_get_config_import_error_line(usbg_gadget *g);
+
+/**
+ * @brief Get text of error which occurred during last gadget import
+ * @param s where gadget import error occurred
+ * @return Text of error or NULL if no error data
+ */
+extern const char *usbg_get_gadget_import_error_text(usbg_state *s);
+
+/**
+ * @brief Get line number where gadget import error occurred
+ * @param s where gadget import error occurred
+ * @return line number or value below 0 if no error data
+ */
+extern int usbg_get_gadget_import_error_line(usbg_state *s);
+
/**
* @}
*/
diff --git a/src/usbg.c b/src/usbg.c
index 37d2954..f22dd80 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -4205,3 +4205,51 @@ out:
return ret;
}
+const char *usbg_get_func_import_error_text(usbg_gadget *g)
+{
+ if (!g || !g-last_failed_import)
+ return NULL;
+
+ return config_error_text(g-last_failed_import);
+}
+
+const char *usbg_get_func_import_error_line(usbg_gadget *g)
+{
+ if (!g || !g-last_failed_import)
+ return -1;
+
+ return config_error_line(g-last_failed_import);
+}
+
+const char *usbg_get_config_import_error_text(usbg_gadget *g)
+{
+ if (!g || !g-last_failed_import)
+ return NULL;
+
+ return config_error_text(g-last_failed_import);
+}
+
+const char *usbg_get_config_import_error_line(usbg_gadget *g)
+{
+ if (!g || !g-last_failed_import)
+ return -1;
+
+ return config_error_line(g-last_failed_import);
+}
+
+const char *usbg_get_gadget_import_error_text(usbg_state *s)
+{
+ if (!s || !s-last_failed_import)
+ return NULL;
+
+ return config_error_text(s-last_failed_import);
+}
+
+const char *usbg_get_gadget_import_error_line(usbg_state *s)
+{
+ if (!s || !s-last_failed_import)
+ return -1;
+
+ return config_error_line(s-last_failed_import);
+}
+
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 07/15] libusbg: Add errors which may happen during parsing
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
include/usbg/usbg.h |4
src/usbg.c | 24
2 files changed, 28 insertions(+)
diff --git a/include/usbg/usbg.h b/include/usbg/usbg.h
index 3a66ae3..3d3cba0 100644
--- a/include/usbg/usbg.h
+++ b/include/usbg/usbg.h
@@ -217,6 +217,10 @@ typedef enum {
USBG_ERROR_BUSY = -8,
USBG_ERROR_NOT_SUPPORTED = -9,
USBG_ERROR_PATH_TOO_LONG = -10,
+ USBG_ERROR_INVALID_FORMAT = -11,
+ USBG_ERROR_MISSING_TAG = -12,
+ USBG_ERROR_INVALID_TYPE = -13,
+ USBG_ERROR_INVALID_VALUE = -14,
USBG_ERROR_OTHER_ERROR = -99
} usbg_error;
diff --git a/src/usbg.c b/src/usbg.c
index c4a9308..cd526e1 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -217,6 +217,18 @@ const char *usbg_error_name(usbg_error e)
case USBG_ERROR_PATH_TOO_LONG:
ret = USBG_ERROR_PATH_TOO_LONG;
break;
+ case USBG_ERROR_INVALID_FORMAT:
+ ret = USBG_ERROR_INVALID_FORMAT;
+ break;
+ case USBG_ERROR_MISSING_TAG:
+ ret = USBG_ERROR_MISSING_TAG;
+ break;
+ case USBG_ERROR_INVALID_TYPE:
+ ret = USBG_ERROR_INVALUD_TYPE;
+ break;
+ case USBG_ERROR_INVALID_VALUE:
+ ret = USBG_ERROR_INVALID_VALUE;
+ break;
case USBG_ERROR_OTHER_ERROR:
ret = USBG_ERROR_OTHER_ERROR;
break;
@@ -263,6 +275,18 @@ const char *usbg_strerror(usbg_error e)
case USBG_ERROR_PATH_TOO_LONG:
ret = Created path was too long to process it.;
break;
+ case USBG_ERROR_INVALID_FORMAT:
+ ret = Given file has incompatible format.;
+ break;
+ case USBG_ERROR_MISSING_TAG:
+ ret = One of mandatory tags is missing.;
+ break;
+ case USBG_ERROR_INVALID_TYPE:
+ ret = One of attributes has incompatible type.;
+ break;
+ case USBG_ERROR_INVALID_VALUE:
+ ret = Incorrect value provided as attribute.;
+ break;
case USBG_ERROR_OTHER_ERROR:
ret = Other error;
break;
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 11/15] libusbg: Allow to store error information in usbg_state
If error occurred during parsing user should have
an opportunity to get details about place and type
of error.
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
src/usbg.c |7 +++
1 file changed, 7 insertions(+)
diff --git a/src/usbg.c b/src/usbg.c
index 9d64171..4178194 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -42,6 +42,7 @@ struct usbg_state
char *path;
TAILQ_HEAD(ghead, usbg_gadget) gadgets;
+ config_t *last_failed_import;
};
struct usbg_gadget
@@ -590,6 +591,11 @@ static void usbg_free_state(usbg_state *s)
usbg_free_gadget(g);
}
+ if (s-last_failed_import) {
+ config_destroy(s-last_failed_import);
+ free(s-last_failed_import);
+ }
+
free(s-path);
free(s);
}
@@ -1246,6 +1252,7 @@ static int usbg_init_state(char *path, usbg_state *s)
/* State takes the ownership of path and should free it */
s-path = path;
+ s-last_failed_import = NULL;
TAILQ_INIT(s-gadgets);
ret = usbg_parse_gadgets(path, s);
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 02/15] libusbg: Add label field to usbg_function structure
Add some field where additional label for function can be stored.
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
src/usbg.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/usbg.c b/src/usbg.c
index b1b5f44..55edd9e 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -75,7 +75,8 @@ struct usbg_function
char *name;
char *path;
char *instance;
-
+ /* Only for internal library usage */
+ char *label;
usbg_function_type type;
};
@@ -511,6 +512,7 @@ static inline void usbg_free_function(usbg_function *f)
{
free(f-path);
free(f-name);
+ free(f-label);
free(f);
}
@@ -631,6 +633,7 @@ static usbg_function *usbg_allocate_function(const char
*path,
if (!f)
goto out;
+ f-label = NULL;
type_name = usbg_get_function_type_str(type);
if (!type_name) {
free(f);
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 01/15] libusbg: Add dependency to libconfig
This library is used to import and export
gadget/function/config to and from file.
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
configure.ac|1 +
libusbg.pc.in |2 +-
src/Makefile.am |4 +++-
3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/configure.ac b/configure.ac
index 878c2ab..bff1257 100644
--- a/configure.ac
+++ b/configure.ac
@@ -4,6 +4,7 @@ AC_PROG_CC
AM_PROG_AR
AC_CONFIG_MACRO_DIR([m4])
AC_DEFINE([_GNU_SOURCE], [], [Use GNU extensions])
+PKG_CHECK_MODULES(LIBCONFIG, libconfig)
LT_INIT
AC_CONFIG_FILES([Makefile src/Makefile examples/Makefile libusbg.pc])
DX_INIT_DOXYGEN([$PACKAGE_NAME],[doxygen.cfg])
diff --git a/libusbg.pc.in b/libusbg.pc.in
index 46eb245..e24d8d6 100644
--- a/libusbg.pc.in
+++ b/libusbg.pc.in
@@ -5,7 +5,7 @@ includedir=@includedir@
Name: libusbg
Description: USB gadget-configfs library
-Requires:
+Requires: libconfig
Version: @PACKAGE_VERSION@
Libs: -L${libdir} -lusbg
Cflags: -I${includedir}
diff --git a/src/Makefile.am b/src/Makefile.am
index d955a4c..8fcaf76 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1,4 +1,6 @@
lib_LTLIBRARIES = libusbg.la
libusbg_la_SOURCES = usbg.c
-libusbg_la_LDFLAGS = -version-info 0:1:0
+libusbg_la_LDFLAGS = $(LIBCONFIG_LIBS)
+libusbg_la_LDFLAGS += -version-info 0:1:0
+libusbg_la_CFLAGS = $(LIBCONFIG_CFLAGS)
AM_CPPFLAGS=-I../include/
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 00/15] Add import from/export to file functionality
Dear Matt,
This quite big series adds new part of libusbg API which allows to
import/export gadget/function/configuration from/to file.
Motivation:
Libusbg allows to create a binary file which set up custom
gadget. This is useful but if we have to create custom binary for
each gadget we wast a lot of work which kernel people put to separate
code from configuration. This leads us to main idea of gadget
schemes. Allow to create simple, human readable config file which
library will be able to translate into usb gadget.
Description:
Gadget schemes is idea of describing gadget/function/configuration in
config file. To not reinvent the wheel I have used existing library
libconfig [1]. This means that the syntax is similar to this described
in documentation of libconfig.
I have extended the library with set of usbg_export_*() functions
which allows to export selected gadget to given FILE. There is also
set of complementary usbg_import_*() functions which allows to read
scheme of gadget/function/config from file and create it using
configfs.
To avoid name conflict I have used the convention that
usbg_import_gadget() function takes name of new gadget as parameter
and usbg_export_gadget() doesn't export gadget name to scheme
file. Similar idea is used in configuration and functions.
Base convention of gadget schemes is simple: if something is not set
in scheme file, default values provided by kernel are used. Moreover
configfs has some attributes which are read only. To allow to store
them in scheme file they are ignored by import functions.
Usage of libconfig and whole design of gadget schemes allows us to use
include directive in gadget definition to import complicated
configurations or functions.
Syntax and detailed rules of using schemes has been described in
gadget_schemes.txt in commit:
libusbg: doc: Add document about gadget schemes
Example:
I have used the gadget-acm-ecm example to create a gadget and then
exported it and imported. Gadget exported using usbg_export_gadget():
$ gadget-export g1 g1.schema
$ cat g1.schema
attrs =
{
bcdUSB = 0x200
bDeviceClass = 0x0
bDeviceSubClass = 0x0
bDeviceProtocol = 0x0
bMaxPacketSize0 = 0x40
idVendor = 0x1D6B
idProduct = 0x104
bcdDevice = 0x1
}
strings = (
{
lang = 0x409
manufacturer = Foo Inc.
product = Bar Gadget
serialnumber = 0123456789
} )
functions =
{
acm_usb0 =
{
instance = usb0
type = acm
attrs =
{
port_num = 0
}
}
acm_usb1 =
{
instance = usb1
type = acm
attrs =
{
port_num = 1
}
}
ecm_usb0 =
{
instance = usb0
type = ecm
attrs =
{
dev_addr = aa:bb:f9:ca:76:fb
host_addr = a2:ad:a2:3f:c8:6f
qmult = 5
}
}
}
configs = (
{
id = 1
name = The only one
attrs =
{
bmAttributes = 0x80
bMaxPower = 0x2
}
strings = (
{
lang = 0x409
configuration = CDC 2xACM+ECM
} )
functions = (
{
name = acm.GS0
function = acm_usb0
},
{
name = acm.GS1
function = acm_usb1
},
{
name = ecm.usb0
function = ecm_usb0
} )
} )
This is quite verbose because it contains read only attributes and
some default ones. This file can be shorten to:
attrs =
{
bcdUSB = 0x200
bMaxPacketSize0 = 0x40
idVendor = 0x1D6B
idProduct = 0x104
bcdDevice = 0x1
}
strings = (
{
lang = 0x409
manufacturer = Foo Inc.
product = Bar Gadget
serialnumber = 0123456789
} )
functions =
{
acm_usb0 =
{
instance = usb0
type = acm
}
acm_usb1 =
{
instance = usb1
type = acm
}
ecm_usb0 =
{
instance = usb0
type = ecm
attrs =
{
dev_addr = aa:bb:f9:ca:76:fb
host_addr = a2:ad:a2:3f:c8:6f
qmult = 5
}
}
}
configs = (
{
id = 1
name = The only one
strings = (
{
lang = 0x409
configuration = CDC 2xACM+ECM
} )
functions = (
{
name = acm.GS0
function = acm_usb0
},
{
name = acm.GS1
function = acm_usb1
},
{
name = ecm.usb0
function = ecm_usb0
} )
} )
Remarks:
Current version of libconfig has no possibility to select output
format details. I have created a patch and set up github pull request
and this patch is waiting for maintainer review[2], so results of
usbg_export_*() may be slightly
[PATCH v3 05/15] libusbg: Add export gadget functionality
Whole gadget setting process take a lot of simple
commands (or lines of code). Those shell commands
may take a while or require dedicated script and
hard-coding gadget configuration. To avoid such
situation add to library ability to export a whole
gadget into file.
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
include/usbg/usbg.h |8 ++
src/usbg.c | 314 +++
2 files changed, 322 insertions(+)
diff --git a/include/usbg/usbg.h b/include/usbg/usbg.h
index 02eb1dd..3a66ae3 100644
--- a/include/usbg/usbg.h
+++ b/include/usbg/usbg.h
@@ -915,6 +915,14 @@ extern int usbg_export_function(usbg_function *f, FILE
*stream);
extern int usbg_export_config(usbg_config *c, FILE *stream);
/**
+ * @brief Exports whole gadget to file
+ * @param g Pointer to gadget to be exported
+ * @param stream where gadget should be saved
+ * @return 0 on success, usbg_error otherwise
+ */
+extern int usbg_export_gadget(usbg_gadget *g, FILE *stream);
+
+/**
* @}
*/
#endif /* __USBG_H__ */
diff --git a/src/usbg.c b/src/usbg.c
index ee0e136..c4a9308 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -2467,6 +2467,7 @@ usbg_binding *usbg_get_next_binding(usbg_binding *b)
#define USBG_ATTRS_TAG attrs
#define USBG_STRINGS_TAG strings
#define USBG_FUNCTIONS_TAG functions
+#define USBG_CONFIGS_TAG configs
#define USBG_LANG_TAG lang
#define USBG_TYPE_TAG type
#define USBG_INSTANCE_TAG instance
@@ -2723,6 +2724,41 @@ out:
}
+static int usbg_export_gadget_configs(usbg_gadget *g, config_setting_t *root)
+{
+ usbg_config *c;
+ config_setting_t *node, *id_node;
+ int ret = USBG_SUCCESS;
+ int cfg_ret;
+
+ TAILQ_FOREACH(c, g-configs, cnode) {
+ node = config_setting_add(root, NULL, CONFIG_TYPE_GROUP);
+ if (!node) {
+ ret = USBG_ERROR_NO_MEM;
+ break;
+ }
+
+ id_node = config_setting_add(node, USBG_ID_TAG,
+CONFIG_TYPE_INT);
+ if (!id_node) {
+ ret = USBG_ERROR_NO_MEM;
+ break;
+ }
+
+ cfg_ret = config_setting_set_int(id_node, c-id);
+ if (cfg_ret != CONFIG_TRUE) {
+ ret = USBG_ERROR_OTHER_ERROR;
+ break;
+ }
+
+ ret = usbg_export_config_prep(c, node);
+ if (ret != USBG_SUCCESS)
+ break;
+ }
+
+ return ret;
+}
+
static int usbg_export_f_net_attrs(usbg_f_net_attrs *attrs,
config_setting_t *root)
{
@@ -2844,6 +2880,258 @@ out:
}
+static int usbg_export_gadget_functions(usbg_gadget *g, config_setting_t *root)
+{
+ usbg_function *f;
+ config_setting_t *node, *inst_node;
+ int ret = USBG_SUCCESS;
+ int cfg_ret;
+ char label[USBG_MAX_NAME_LENGTH];
+ char *func_label;
+ int nmb;
+
+ TAILQ_FOREACH(f, g-functions, fnode) {
+ if (f-label) {
+ func_label = f-label;
+ } else {
+ nmb = generate_function_label(f, label, sizeof(label));
+ if (nmb = sizeof(label)) {
+ ret = USBG_ERROR_OTHER_ERROR;
+ break;
+ }
+ func_label = label;
+ }
+
+ node = config_setting_add(root, func_label, CONFIG_TYPE_GROUP);
+ if (!node) {
+ ret = USBG_ERROR_NO_MEM;
+ break;
+ }
+
+ /* Add instance name to identify in this gadget */
+ inst_node = config_setting_add(node, USBG_INSTANCE_TAG,
+ CONFIG_TYPE_STRING);
+ if (!inst_node) {
+ ret = USBG_ERROR_NO_MEM;
+ break;
+ }
+
+ cfg_ret = config_setting_set_string(inst_node, f-instance);
+ if (cfg_ret != CONFIG_TRUE) {
+ ret = USBG_ERROR_OTHER_ERROR;
+ break;
+ }
+
+ ret = usbg_export_function_prep(f, node);
+ if (ret != USBG_SUCCESS)
+ break;
+ }
+
+ return ret;
+}
+
+static int usbg_export_gadget_strs_lang(usbg_gadget *g, const char *lang_str,
+ config_setting_t *root)
+{
+ config_setting_t *node;
+ usbg_gadget_strs strs;
+ int lang;
+ int usbg_ret, cfg_ret;
+ int ret = USBG_ERROR_NO_MEM;
+
+ ret = sscanf(lang_str, %x, lang);
+ if (ret != 1) {
+ ret = USBG_ERROR_OTHER_ERROR;
+ goto out;
+ }
+
+ usbg_ret = usbg_get_gadget_strs(g, lang, strs);
+ if (usbg_ret != USBG_SUCCESS) {
+
[PATCH v3 03/15] libusbg: Add export function functionality
Function settings may be complicated and their configuration
may take a long while. To save time it would be convenient
to allow for storing function settings in a file.
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
include/usbg/usbg.h | 11
src/usbg.c | 156 +++
2 files changed, 167 insertions(+)
diff --git a/include/usbg/usbg.h b/include/usbg/usbg.h
index e123365..99c201c 100644
--- a/include/usbg/usbg.h
+++ b/include/usbg/usbg.h
@@ -22,6 +22,7 @@
#include netinet/ether.h
#include stdint.h
#include limits.h
+#include stdio.h /* For FILE * */
/**
* @file include/usbg/usbg.h
@@ -895,6 +896,16 @@ extern usbg_config *usbg_get_next_config(usbg_config *c);
*/
extern usbg_binding *usbg_get_next_binding(usbg_binding *b);
+/* Import / Export API */
+
+/**
+ * @brief Exports usb function to file
+ * @param f Pointer to function to be exported
+ * @param stream where function should be saved
+ * @return 0 on success, usbg_error otherwise
+ */
+extern int usbg_export_function(usbg_function *f, FILE *stream);
+
/**
* @}
*/
diff --git a/src/usbg.c b/src/usbg.c
index 55edd9e..34b7fb6 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -26,6 +26,7 @@
#include sys/stat.h
#include unistd.h
#include ctype.h
+#include libconfig.h
#define STRINGS_DIR strings
#define CONFIGS_DIR configs
@@ -2461,3 +2462,158 @@ usbg_binding *usbg_get_next_binding(usbg_binding *b)
{
return b ? TAILQ_NEXT(b, bnode) : NULL;
}
+
+#define USBG_ATTRS_TAG attrs
+#define USBG_TYPE_TAG type
+#define USBG_INSTANCE_TAG instance
+#define USBG_TAB_WIDTH 4
+
+static int usbg_export_f_net_attrs(usbg_f_net_attrs *attrs,
+ config_setting_t *root)
+{
+ config_setting_t *node;
+ char *addr;
+ char addr_buf[USBG_MAX_STR_LENGTH];
+ int cfg_ret;
+ int ret = USBG_ERROR_NO_MEM;
+
+ node = config_setting_add(root, dev_addr, CONFIG_TYPE_STRING);
+ if (!node)
+ goto out;
+
+ addr = ether_ntoa_r(attrs-dev_addr, addr_buf);
+ cfg_ret = config_setting_set_string(node, addr);
+ if (cfg_ret != CONFIG_TRUE) {
+ ret = USBG_ERROR_OTHER_ERROR;
+ goto out;
+ }
+
+ node = config_setting_add(root, host_addr, CONFIG_TYPE_STRING);
+ if (!node)
+ goto out;
+
+ addr = ether_ntoa_r(attrs-host_addr, addr_buf);
+ cfg_ret = config_setting_set_string(node, addr);
+ if (cfg_ret != CONFIG_TRUE) {
+ ret = USBG_ERROR_OTHER_ERROR;
+ goto out;
+ }
+
+ node = config_setting_add(root, qmult, CONFIG_TYPE_INT);
+ if (!node)
+ goto out;
+
+ cfg_ret = config_setting_set_int(node, attrs-qmult);
+ ret = cfg_ret == CONFIG_TRUE ? 0 : USBG_ERROR_OTHER_ERROR;
+
+ /* if name is read only so we don't export it */
+out:
+ return ret;
+
+}
+
+static int usbg_export_function_attrs(usbg_function *f, config_setting_t *root)
+{
+ config_setting_t *node;
+ usbg_function_attrs f_attrs;
+ int usbg_ret, cfg_ret;
+ int ret = USBG_ERROR_NO_MEM;
+
+ usbg_ret = usbg_get_function_attrs(f, f_attrs);
+ if (usbg_ret) {
+ ret = usbg_ret;
+ goto out;
+ }
+
+ switch (f-type) {
+ case F_SERIAL:
+ case F_ACM:
+ case F_OBEX:
+ node = config_setting_add(root, port_num, CONFIG_TYPE_INT);
+ if (!node)
+ goto out;
+
+ cfg_ret = config_setting_set_int(node, f_attrs.serial.port_num);
+ ret = cfg_ret == CONFIG_TRUE ? 0 : USBG_ERROR_OTHER_ERROR;
+ break;
+ case F_ECM:
+ case F_SUBSET:
+ case F_NCM:
+ case F_EEM:
+ case F_RNDIS:
+ ret = usbg_export_f_net_attrs(f_attrs.net, root);
+ break;
+ case F_PHONET:
+ /* Don't export ifname because it is read only */
+ break;
+ case F_FFS:
+ /* We don't need to export ffs attributes
+* due to instance name export */
+ ret = USBG_SUCCESS;
+ break;
+ default:
+ ERROR(Unsupported function type\n);
+ ret = USBG_ERROR_NOT_SUPPORTED;
+ }
+
+out:
+ return ret;
+}
+
+/* This function does not import instance name becuase this is more property
+ * of a gadget than a function itselt */
+static int usbg_export_function_prep(usbg_function *f, config_setting_t *root)
+{
+ config_setting_t *node;
+ int ret = USBG_ERROR_NO_MEM;
+ int cfg_ret;
+
+ node = config_setting_add(root, USBG_TYPE_TAG, CONFIG_TYPE_STRING);
+ if (!node)
+ goto out;
+
+ cfg_ret = config_setting_set_string(node, usbg_get_function_type_str(
+ f-type));
+ if (cfg_ret != CONFIG_TRUE) {
+
[PATCH v3 04/15] libusbg: Add export config functionality
Configuration may have several functions if we add
strings and some attributes it gives a few shell commands
to set it up. To avoid this add export cofiguration
which allows to store usb configuration in a file.
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
include/usbg/usbg.h |8 ++
src/usbg.c | 283 +++
2 files changed, 291 insertions(+)
diff --git a/include/usbg/usbg.h b/include/usbg/usbg.h
index 99c201c..02eb1dd 100644
--- a/include/usbg/usbg.h
+++ b/include/usbg/usbg.h
@@ -907,6 +907,14 @@ extern usbg_binding *usbg_get_next_binding(usbg_binding
*b);
extern int usbg_export_function(usbg_function *f, FILE *stream);
/**
+ * @brief Exports configuration to file
+ * @param c Pointer to configuration to be exported
+ * @param stream where configuration should be saved
+ * @return 0 on success, usbg_error otherwise
+ */
+extern int usbg_export_config(usbg_config *c, FILE *stream);
+
+/**
* @}
*/
#endif /* __USBG_H__ */
diff --git a/src/usbg.c b/src/usbg.c
index 34b7fb6..ee0e136 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -2463,11 +2463,266 @@ usbg_binding *usbg_get_next_binding(usbg_binding *b)
return b ? TAILQ_NEXT(b, bnode) : NULL;
}
+#define USBG_NAME_TAG name
#define USBG_ATTRS_TAG attrs
+#define USBG_STRINGS_TAG strings
+#define USBG_FUNCTIONS_TAG functions
+#define USBG_LANG_TAG lang
#define USBG_TYPE_TAG type
#define USBG_INSTANCE_TAG instance
+#define USBG_ID_TAG id
+#define USBG_FUNCTION_TAG function
#define USBG_TAB_WIDTH 4
+static inline int generate_function_label(usbg_function *f, char *buf, int
size)
+{
+ return snprintf(buf, size, %s_%s,
+usbg_get_function_type_str(f-type), f-instance);
+
+}
+
+static int usbg_export_binding(usbg_binding *b, config_setting_t *root)
+{
+ config_setting_t *node;
+ int ret = USBG_ERROR_NO_MEM;
+ int cfg_ret;
+ char label[USBG_MAX_NAME_LENGTH];
+ int nmb;
+
+#define CRETAE_ATTR_STRING(SOURCE, NAME) \
+ do {\
+ node = config_setting_add(root, NAME, CONFIG_TYPE_STRING); \
+ if (!node) \
+ goto out; \
+ cfg_ret = config_setting_set_string(node, SOURCE); \
+ if (cfg_ret != CONFIG_TRUE) { \
+ ret = USBG_ERROR_OTHER_ERROR; \
+ goto out; \
+ } \
+ } while (0)
+
+ CRETAE_ATTR_STRING(b-name, USBG_NAME_TAG);
+
+ nmb = generate_function_label(b-target, label, sizeof(label));
+ if (nmb = sizeof(label)) {
+ ret = USBG_ERROR_OTHER_ERROR;
+ goto out;
+ }
+
+ CRETAE_ATTR_STRING(label, USBG_FUNCTION_TAG);
+
+#undef CRETAE_ATTR_STRING
+
+ ret = USBG_SUCCESS;
+
+out:
+ return ret;
+}
+
+static int usbg_export_config_bindings(usbg_config *c, config_setting_t *root)
+{
+ usbg_binding *b;
+ config_setting_t *node;
+ int ret = USBG_SUCCESS;
+
+ TAILQ_FOREACH(b, c-bindings, bnode) {
+ node = config_setting_add(root, NULL, CONFIG_TYPE_GROUP);
+ if (!node) {
+ ret = USBG_ERROR_NO_MEM;
+ break;
+ }
+
+ ret = usbg_export_binding(b, node);
+ if (ret != USBG_SUCCESS)
+ break;
+ }
+
+ return ret;
+}
+
+static int usbg_export_config_strs_lang(usbg_config *c, char *lang_str,
+ config_setting_t *root)
+{
+ config_setting_t *node;
+ usbg_config_strs strs;
+ int lang;
+ int usbg_ret, cfg_ret, ret2;
+ int ret = USBG_ERROR_NO_MEM;
+
+ ret2 = sscanf(lang_str, %x, lang);
+ if (ret2 != 1) {
+ ret = USBG_ERROR_OTHER_ERROR;
+ goto out;
+ }
+
+ usbg_ret = usbg_get_config_strs(c, lang, strs);
+ if (usbg_ret != USBG_SUCCESS) {
+ ret = usbg_ret;
+ goto out;
+ }
+
+ node = config_setting_add(root, USBG_LANG_TAG, CONFIG_TYPE_INT);
+ if (!node)
+ goto out;
+
+ cfg_ret = config_setting_set_format(node, CONFIG_FORMAT_HEX);
+ if (cfg_ret != CONFIG_TRUE) {
+ ret = USBG_ERROR_OTHER_ERROR;
+ goto out;
+ }
+
+ cfg_ret = config_setting_set_int(node, lang);
+ if (cfg_ret != CONFIG_TRUE) {
+ ret = USBG_ERROR_OTHER_ERROR;
+ goto out;
+ }
+
+ node = config_setting_add(root, configuration , CONFIG_TYPE_STRING);
+ if (!node)
+ goto out;
+
+ cfg_ret =
[PATCH v3 15/15] libusbg: doc: Add document about gadget schemes
Add document which clarify reasons of implementing
gadget schemes and also describes syntax of such files.
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
doc/gadget_schemes.txt | 301
1 file changed, 301 insertions(+)
create mode 100644 doc/gadget_schemes.txt
diff --git a/doc/gadget_schemes.txt b/doc/gadget_schemes.txt
new file mode 100644
index 000..d1d44a5
--- /dev/null
+++ b/doc/gadget_schemes.txt
@@ -0,0 +1,301 @@
+
+ Gadget schemes
+
+
+Index:
+1. What are gadget schemes?
+2. Why gadget schemes?
+3. Gadget scheme syntax
+ 3.1 Function scheme
+ 3.2 Configuration scheme
+ 3.3 Gadget scheme
+4. Conclusion
+
+
+1. What are gadget schemes?
+
+Gadget schemes are files which contains configuration data of
+gadget/function/configuration. Those files can be generated using
+usbg_export_*() functions for whole gadget, configuration or single
+function. Library provides also set of usbg_import_*() functions which
+allows to load configuration data back to configfs.
+
+
+ 2. Why gadget schemes?
+
+New kernel interface - ConfigFS which along with libcomposite allows
+to set up custom gadget. This can be achieved using simple, command
+line file system operation like mkdir, rmdir, ln -s, read and
+write. Yes, it is possible to configure usb gadget using only command
+line but each time after reboot user needs to recreate all gadgets
+once again. This means that after each reboot user needs to use about
+15 commands (depends on number and types of function). This is
+definitely not acceptable for those who used legacy gadgets and write
+only modprobe g_ether.
+
+One of first idea to solve this is to create a script and run it
+after each reboot. This approach is feasible but has many
+disadvantages. First of them is security. ConfigFS is modifiable by
+default only by root, so scripts has to be executed with root
+rights. Secondly it's really hard to modify such a script because many
+calls has hard-coded path where for example echo should be
+done. There is a lot of simple, but low level operations which can
+cause a lot of confusion for beginner.
+
+Second approach is to create executable which will create our gadget
+using base libusbg API. It is also possible but let's think for a
+moment why configfs has been introduced. It has been announced to
+separate code from configuration. Code is a piece of C code in kernel
+module which realizes usb function and configuration is understood as
+composition of those functions into a gadget as a whole. If we would
+like to create binary file for each gadget we would waste a lot of
+work which kernel contributors put to remove hard-coded gadgets from
+linux kernel. This all leads us to solution described in this document
+- gadget schemes. Light weight configuration files which describes
+composition of functions into gadget. They can be simply loaded using
+usbg_import_*() and exported using usbg_export_*(). This makes them
+easy to use equivalent of modprobe gadget_module.
+
+
+ 3. Gadget scheme syntax
+
+Gadget schemes implementation uses libconfig for reading and writing
+scheme files. This means that all limitations of libconfig are also
+present in gadget schemes. More over there are additional constrains
+for scheme files. Gadget scheme is only a password and import and
+export is not limited to whole gadgets. It is possible to export all 3
+types of gadget entity: function, configuration and gadget. Please
+refer to libconfig documentation for details about syntax and rules.
+
+3.1 Function scheme
+
+Function scheme is a file or part of file which represents single
+function.
+
+Example:
+
+instance = my_func_instance
+type = ecm
+
+attrs = {
+ dev_addr = ef:33:be:9a:90:36
+ dev_addr = ab:63:6e:8b:10:16
+ qmult = 5
+}
+
+For functions, type is the only attribute which is always
+mandatory. Instance is mandatory only if function is part of bigger
+scheme (gadget for example). By default usbg_export_function() does
+not export instance name, because usbg_import_function() takes
+instance as one of parameters. This convention allows for simple
+function movement between gadgets without names conflict.
+
+Attrs section is optional. It may not be included, present but empty
+or present and filled with function attributes. Attribute names are
+similar as those from configfs. Each type of function has own set of
+possible attributes. It is worth to mention that some attributes are
+read only and they cannot be imported from file. To make it more
+user-friendly read-only parameters are just ignored. This allows for
+direct use of previously exported function in import. If some
+attribute has not been provided default value provided by kernel will
+be used.
+
+ 3.2 Configuration scheme
+
+Configuration scheme is a file or part of file which represents
[PATCH v3 09/15] libusbg: Add import function functionality
Functions can be exported to file using usbg_export_function().
This commit adds complementary API function usbg_import_function()
which allows to import function from file to configfs.
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
include/usbg/usbg.h | 13
src/usbg.c | 188 +++
2 files changed, 201 insertions(+)
diff --git a/include/usbg/usbg.h b/include/usbg/usbg.h
index 3d3cba0..5e75208 100644
--- a/include/usbg/usbg.h
+++ b/include/usbg/usbg.h
@@ -927,6 +927,19 @@ extern int usbg_export_config(usbg_config *c, FILE
*stream);
extern int usbg_export_gadget(usbg_gadget *g, FILE *stream);
/**
+
+/**
+ * @brief Imports usb function from file and adds it to given gadget
+ * @param g Gadget where function should be placed
+ * @param stream from which function should be imported
+ * @param instance name which shuld be used for new function
+ * @param f place for pointer to imported function
+ * if NULL this param will be ignored.
+ * @return 0 on success, usbg_error otherwise
+ */
+extern int usbg_import_function(usbg_gadget *g, FILE *stream,
+ const char *instance, usbg_function **f);
+
* @}
*/
#endif /* __USBG_H__ */
diff --git a/src/usbg.c b/src/usbg.c
index a87a6b1..913fae8 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -3246,3 +3246,191 @@ out:
config_destroy(cfg);
return ret;
}
+
+#define usbg_config_is_int(node) (config_setting_type(node) == CONFIG_TYPE_INT)
+#define usbg_config_is_string(node) \
+ (config_setting_type(node) == CONFIG_TYPE_STRING)
+
+static void usbg_set_failed_import(config_t **to_set, config_t *failed)
+{
+ if (*to_set != NULL) {
+ config_destroy(*to_set);
+ free(*to_set);
+ }
+
+ *to_set = failed;
+}
+
+static int usbg_import_f_net_attrs(config_setting_t *root, usbg_function *f)
+{
+ config_setting_t *node;
+ int ret = USBG_SUCCESS;
+ int qmult;
+ struct ether_addr *addr;
+ struct ether_addr addr_buf;
+ const char *str;
+
+#define GET_OPTIONAL_ADDR(NAME)\
+ do {\
+ node = config_setting_get_member(root, #NAME); \
+ if (node) { \
+ str = config_setting_get_string(node); \
+ if (!str) { \
+ ret = USBG_ERROR_INVALID_TYPE; \
+ goto out; \
+ } \
+ \
+ addr = ether_aton_r(str, addr_buf);\
+ if (!addr) {\
+ ret = USBG_ERROR_INVALID_VALUE; \
+ goto out; \
+ } \
+ ret = usbg_set_net_##NAME(f, addr); \
+ if (ret != USBG_SUCCESS)\
+ goto out; \
+ } \
+ } while (0)
+
+ GET_OPTIONAL_ADDR(host_addr);
+ GET_OPTIONAL_ADDR(dev_addr);
+
+#undef GET_OPTIONAL_ADDR
+
+ node = config_setting_get_member(root, qmult);
+ if (node) {
+ if (!usbg_config_is_int(node)) {
+ ret = USBG_ERROR_INVALID_TYPE;
+ goto out;
+ }
+ qmult = config_setting_get_int(node);
+ ret = usbg_set_net_qmult(f, qmult);
+ }
+
+out:
+ return ret;
+}
+
+static int usbg_import_function_attrs(config_setting_t *root, usbg_function *f)
+{
+ int ret = USBG_SUCCESS;
+
+ switch (f-type) {
+ case F_SERIAL:
+ case F_ACM:
+ case F_OBEX:
+ /* Don't import port_num because it is read only */
+ break;
+ case F_ECM:
+ case F_SUBSET:
+ case F_NCM:
+ case F_EEM:
+ case F_RNDIS:
+ ret = usbg_import_f_net_attrs(root, f);
+ break;
+ case F_PHONET:
+ /* Don't import ifname because it is read only */
+ break;
+ case F_FFS:
+ /* We don't need to export ffs attributes
+* due to instance name export */
+ break;
+ default:
+ ERROR(Unsupported function type\n);
+ ret = USBG_ERROR_NOT_SUPPORTED;
+ }
+
+ return ret;
+}
+
+static int usbg_import_function_run(usbg_gadget *g, config_setting_t *root,
+ const char *instance, usbg_function **f)
+{
+ config_setting_t *node;
+ const char *type_str;
+ int
[PATCH v3 14/15] libusbg: examples: Add gadget-import sample app
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
examples/Makefile.am |3 +-
examples/gadget-import.c | 79 ++
src/usbg.c |6 ++--
3 files changed, 84 insertions(+), 4 deletions(-)
create mode 100644 examples/gadget-import.c
diff --git a/examples/Makefile.am b/examples/Makefile.am
index aeb0273..bf8b45d 100644
--- a/examples/Makefile.am
+++ b/examples/Makefile.am
@@ -1,8 +1,9 @@
-bin_PROGRAMS = show-gadgets gadget-acm-ecm gadget-vid-pid-remove gadget-ffs
gadget-export
+bin_PROGRAMS = show-gadgets gadget-acm-ecm gadget-vid-pid-remove gadget-ffs
gadget-export gadget-import
gadget_acm_ecm_SOURCES = gadget-acm-ecm.c
show_gadgets_SOURCES = show-gadgets.c
gadget_vid_pid_remove_SOURCES = gadget-vid-pid-remove.c
gadget_ffs_SOURCES = gadget-ffs.c
gadget_export_SOURCE = gadget-export.c
+gadget_import_SOURCE = gadget-import.c
AM_CPPFLAGS=-I../include/
AM_LDFLAGS=-L../src/ -lusbg
diff --git a/examples/gadget-import.c b/examples/gadget-import.c
new file mode 100644
index 000..cf97d9c
--- /dev/null
+++ b/examples/gadget-import.c
@@ -0,0 +1,79 @@
+/*
+ * Copyright (C) 2014 Samsung Electronics
+ *
+ * Krzysztof Opasiak k.opas...@samsung.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+/**
+ * @file gadget-import.c
+ * @example gadget-import.c
+ * This is an example of how to import a gadget from file.
+ * Common reason of doing this is to create gadget base on schema
+ * from other devices or resurect gadget after reboot.
+ */
+
+#include errno.h
+#include string.h
+#include stdio.h
+#include usbg/usbg.h
+
+int main(int argc, char **argv)
+{
+ usbg_state *s;
+ usbg_gadget *g;
+ int ret = -EINVAL;
+ int usbg_ret;
+ FILE *input;
+
+ if (argc != 3) {
+ fprintf(stderr, Usage: gadget-import gadget_name file_name\n);
+ return ret;
+ }
+
+ /* Prepare input file */
+ input = fopen(argv[2], r);
+ if (!input) {
+ fprintf(stderr, Error on fopen. Error: %s\n, strerror(errno));
+ goto out1;
+ }
+
+ /* Do gadget exporting */
+ usbg_ret = usbg_init(/sys/kernel/config, s);
+ if (usbg_ret != USBG_SUCCESS) {
+ fprintf(stderr, Error on USB gadget init\n);
+ fprintf(stderr, Error: %s : %s\n, usbg_error_name(usbg_ret),
+ usbg_strerror(usbg_ret));
+ goto out2;
+ }
+
+ usbg_ret = usbg_import_gadget(s, input, argv[1], NULL);
+ if (usbg_ret != USBG_SUCCESS) {
+ fprintf(stderr, Error on import gadget\n);
+ fprintf(stderr, Error: %s : %s\n, usbg_error_name(usbg_ret),
+ usbg_strerror(usbg_ret));
+ if (usbg_ret == USBG_ERROR_INVALID_FORMAT)
+ fprintf(stderr, Line: %d. Error: %s\n,
+ usbg_get_gadget_import_error_line(s),
+ usbg_get_gadget_import_error_text(s));
+ goto out3;
+ }
+
+ ret = 0;
+
+out3:
+ usbg_cleanup(s);
+out2:
+ fclose(input);
+out1:
+ return ret;
+}
diff --git a/src/usbg.c b/src/usbg.c
index f22dd80..ffd3c88 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -4213,7 +4213,7 @@ const char *usbg_get_func_import_error_text(usbg_gadget
*g)
return config_error_text(g-last_failed_import);
}
-const char *usbg_get_func_import_error_line(usbg_gadget *g)
+int usbg_get_func_import_error_line(usbg_gadget *g)
{
if (!g || !g-last_failed_import)
return -1;
@@ -4229,7 +4229,7 @@ const char *usbg_get_config_import_error_text(usbg_gadget
*g)
return config_error_text(g-last_failed_import);
}
-const char *usbg_get_config_import_error_line(usbg_gadget *g)
+int usbg_get_config_import_error_line(usbg_gadget *g)
{
if (!g || !g-last_failed_import)
return -1;
@@ -4245,7 +4245,7 @@ const char *usbg_get_gadget_import_error_text(usbg_state
*s)
return config_error_text(s-last_failed_import);
}
-const char *usbg_get_gadget_import_error_line(usbg_state *s)
+int usbg_get_gadget_import_error_line(usbg_state *s)
{
if (!s || !s-last_failed_import)
return -1;
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 10/15] libusbg: Add import config functionality
Configurations can be exported to file using usbg_export_config().
This commit adds complementary API function usbg_import_config()
which allows to import configuration (with attrs, strings and
bindings) from file to chosen gadget.
Signed-off-by: Krzysztof Opasiak k.opas...@samsung.com
---
include/usbg/usbg.h | 11 ++
src/usbg.c | 416 +++
2 files changed, 427 insertions(+)
diff --git a/include/usbg/usbg.h b/include/usbg/usbg.h
index 5e75208..70b9910 100644
--- a/include/usbg/usbg.h
+++ b/include/usbg/usbg.h
@@ -940,6 +940,17 @@ extern int usbg_export_gadget(usbg_gadget *g, FILE
*stream);
extern int usbg_import_function(usbg_gadget *g, FILE *stream,
const char *instance, usbg_function **f);
+/**
+ * @brief Imports usb configuration from file and adds it to given gadget
+ * @param g Gadget where configuration should be placed
+ * @param stream from which configuration should be imported
+ * @param id which shuld be used for new configuration
+ * @param c place for pointer to imported configuration
+ * if NULL this param will be ignored.
+ * @return 0 on success, usbg_error otherwise
+ */
+extern int usbg_import_config(usbg_gadget *g, FILE *stream, int id,
+ usbg_config **c);
* @}
*/
#endif /* __USBG_H__ */
diff --git a/src/usbg.c b/src/usbg.c
index 913fae8..9d64171 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -3251,6 +3251,38 @@ out:
#define usbg_config_is_string(node) \
(config_setting_type(node) == CONFIG_TYPE_STRING)
+static int split_function_label(const char *label, usbg_function_type *type,
+ const char **instance)
+{
+ const char *floor;
+ char buf[USBG_MAX_NAME_LENGTH];
+ int len;
+ int function_type;
+ int ret = USBG_ERROR_NOT_FOUND;
+
+ /* We assume that function type string doesn't contain '_' */
+ floor = strchr(label, '_');
+ /* if phrase before _ is longer than max name length we may
+* stop looking */
+ len = floor - label;
+ if (len = USBG_MAX_NAME_LENGTH || floor == label)
+ goto out;
+
+ strncpy(buf, label, len);
+ buf[len] = '\0';
+
+ function_type = usbg_lookup_function_type(buf);
+ if (function_type 0)
+ goto out;
+
+ *type = (usbg_function_type)function_type;
+ *instance = floor + 1;
+
+ ret = USBG_SUCCESS;
+out:
+ return ret;
+}
+
static void usbg_set_failed_import(config_t **to_set, config_t *failed)
{
if (*to_set != NULL) {
@@ -3389,6 +3421,346 @@ out:
return ret;
}
+static usbg_function *usbg_lookup_function(usbg_gadget *g, const char *label)
+{
+ usbg_function *f;
+ int usbg_ret;
+
+ /* check if such function has also been imported */
+ TAILQ_FOREACH(f, g-functions, fnode) {
+ if (f-label !strcmp(f-label, label))
+ break;
+ }
+
+ /* if not let's check if label follows the naming convention */
+ if (!f) {
+ usbg_function_type type;
+ const char *instance;
+
+ usbg_ret = split_function_label(label, type, instance);
+ if (usbg_ret != USBG_SUCCESS)
+ goto out;
+
+ /* check if such function exist */
+ f = usbg_get_function(g, type, instance);
+ }
+
+out:
+ return f;
+}
+
+/* We have a string which should match with one of function names */
+static int usbg_import_binding_string(config_setting_t *root, usbg_config *c)
+{
+ const char *func_label;
+ usbg_function *target;
+ int ret;
+
+ func_label = config_setting_get_string(root);
+ if (!func_label) {
+ ret = USBG_ERROR_OTHER_ERROR;
+ goto out;
+ }
+
+ target = usbg_lookup_function(c-parent, func_label);
+ if (!target) {
+ ret = USBG_ERROR_NOT_FOUND;
+ goto out;
+ }
+
+ ret = usbg_add_config_function(c, target-name, target);
+out:
+ return ret;
+}
+
+static int usbg_import_binding_group(config_setting_t *root, usbg_config *c)
+{
+ config_setting_t *node;
+ const char *func_label, *name;
+ usbg_function *target;
+ int ret;
+
+ node = config_setting_get_member(root, USBG_FUNCTION_TAG);
+ if (!node) {
+ ret = USBG_ERROR_MISSING_TAG;
+ goto out;
+ }
+
+ /* It is allowed to provide link to existing function
+* or define unlabeled instance of function in this place */
+ if (usbg_config_is_string(node)) {
+ func_label = config_setting_get_string(node);
+ if (!func_label) {
+ ret = USBG_ERROR_OTHER_ERROR;
+ goto out;
+ }
+
+ target = usbg_lookup_function(c-parent, func_label);
+ if (!target) {
+
[PATCH] usb: gadget: composite: dequeue cdev-req before free its buffer
As Felipe suggested, dequeue the cdev-req before free its buffer.
Suggested-by: Felipe Balbi ba...@ti.com
Signed-off-by: Li Jun b47...@freescale.com
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 6935a82..4514e73 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -1955,8 +1955,8 @@ void composite_dev_cleanup(struct usb_composite_dev *cdev)
usb_ep_free_request(cdev-gadget-ep0, cdev-os_desc_req);
}
if (cdev-req) {
- kfree(cdev-req-buf);
usb_ep_dequeue(cdev-gadget-ep0, cdev-req);
+ kfree(cdev-req-buf);
usb_ep_free_request(cdev-gadget-ep0, cdev-req);
}
cdev-next_string_id = 0;
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 1/3] usb: gadget: f_fs: fix the redundant ep files problem
Up to now, when endpoint addresses in descriptors were non-consecutive,
there were created redundant files, which could cause problems in kernel,
when user tried to read/write to them. It was result of fact that maximum
endpoint address was taken as total number of endpoints in funciton.
This patch adds endpoint descriptors counting and storing their addresses
in eps_addrmap to verify their cohesion in each speed.
Endpoint address map would be also useful for further features, just like
vitual endpoint address mapping.
Signed-off-by: Robert Baldyga r.bald...@samsung.com
Acked-by: Michal Nazarewicz min...@mina86.com
---
drivers/usb/gadget/function/f_fs.c | 66 +++---
drivers/usb/gadget/function/u_fs.h | 2 ++
2 files changed, 57 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/gadget/function/f_fs.c
b/drivers/usb/gadget/function/f_fs.c
index dc30adf..0dc3552d 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -155,6 +155,12 @@ struct ffs_io_data {
struct usb_request *req;
};
+struct ffs_desc_helper {
+ struct ffs_data *ffs;
+ unsigned interfaces_count;
+ unsigned eps_count;
+};
+
static int __must_check ffs_epfiles_create(struct ffs_data *ffs);
static void ffs_epfiles_destroy(struct ffs_epfile *epfiles, unsigned count);
@@ -1830,7 +1836,8 @@ static int __ffs_data_do_entity(enum ffs_entity_type type,
u8 *valuep, struct usb_descriptor_header *desc,
void *priv)
{
- struct ffs_data *ffs = priv;
+ struct ffs_desc_helper *helper = priv;
+ struct usb_endpoint_descriptor *d;
ENTER();
@@ -1844,8 +1851,8 @@ static int __ffs_data_do_entity(enum ffs_entity_type type,
* encountered interface n then there are at least
* n+1 interfaces.
*/
- if (*valuep = ffs-interfaces_count)
- ffs-interfaces_count = *valuep + 1;
+ if (*valuep = helper-interfaces_count)
+ helper-interfaces_count = *valuep + 1;
break;
case FFS_STRING:
@@ -1853,14 +1860,22 @@ static int __ffs_data_do_entity(enum ffs_entity_type
type,
* Strings are indexed from 1 (0 is magic ;) reserved
* for languages list or some such)
*/
- if (*valuep ffs-strings_count)
- ffs-strings_count = *valuep;
+ if (*valuep helper-ffs-strings_count)
+ helper-ffs-strings_count = *valuep;
break;
case FFS_ENDPOINT:
- /* Endpoints are indexed from 1 as well. */
- if ((*valuep USB_ENDPOINT_NUMBER_MASK) ffs-eps_count)
- ffs-eps_count = (*valuep USB_ENDPOINT_NUMBER_MASK);
+ d = (void *)desc;
+ helper-eps_count++;
+ if (helper-eps_count = 15)
+ return -EINVAL;
+ /* Check if descriptors for any speed were already parsed */
+ if (!helper-ffs-eps_count !helper-ffs-interfaces_count)
+ helper-ffs-eps_addrmap[helper-eps_count] =
+ d-bEndpointAddress;
+ else if (helper-ffs-eps_addrmap[helper-eps_count] !=
+ d-bEndpointAddress)
+ return -EINVAL;
break;
}
@@ -2053,6 +2068,7 @@ static int __ffs_data_got_descs(struct ffs_data *ffs,
char *data = _data, *raw_descs;
unsigned os_descs_count = 0, counts[3], flags;
int ret = -EINVAL, i;
+ struct ffs_desc_helper helper;
ENTER();
@@ -2101,13 +2117,29 @@ static int __ffs_data_got_descs(struct ffs_data *ffs,
/* Read descriptors */
raw_descs = data;
+ helper.ffs = ffs;
for (i = 0; i 3; ++i) {
if (!counts[i])
continue;
+ helper.interfaces_count = 0;
+ helper.eps_count = 0;
ret = ffs_do_descs(counts[i], data, len,
- __ffs_data_do_entity, ffs);
+ __ffs_data_do_entity, helper);
if (ret 0)
goto error;
+ if (!ffs-eps_count !ffs-interfaces_count) {
+ ffs-eps_count = helper.eps_count;
+ ffs-interfaces_count = helper.interfaces_count;
+ } else {
+ if (ffs-eps_count != helper.eps_count) {
+ ret = -EINVAL;
+ goto error;
+ }
+ if (ffs-interfaces_count != helper.interfaces_count) {
+ ret = -EINVAL;
+ goto error;
+ }
+ }
[PATCH v6 2/3] usb: gadget: f_fs: add ioctl returning ep descriptor
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 r.bald...@samsung.com
Acked-by: Michal Nazarewicz min...@mina86.com
---
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;
+ }
default:
ret = -ENOTTY;
}
diff --git a/include/uapi/linux/usb/functionfs.h
b/include/uapi/linux/usb/functionfs.h
index 0154b28..7677108 100644
--- a/include/uapi/linux/usb/functionfs.h
+++ b/include/uapi/linux/usb/functionfs.h
@@ -265,6 +265,12 @@ struct usb_functionfs_event {
*/
#defineFUNCTIONFS_ENDPOINT_REVMAP _IO('g', 129)
+/*
+ * Returns endpoint descriptor. If function is not active returns -ENODEV.
+ */
+#defineFUNCTIONFS_ENDPOINT_DESC_IOR('g', 130, \
+struct usb_endpoint_descriptor)
+
#endif /* _UAPI__LINUX_FUNCTIONFS_H__ */
--
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
[PATCH v6 0/3] usb: gadget: f_fs: userspace API fixes and improvements
This patchset contains changes in FunctionFS making it easier and safer to use. It fixes bug in endpoint files handling code, adds new ioctl allowing to obtain endpoint descriptor, and introduces virtual address mapping which allows to separate endpoint address space in function from physical endpoint addresses, and introduces new endpoint files naming convention. Changelog: v6: - unlock spinlock before copy_to_user() call - remove duplicated eps_count value check - few minor fixes v5: https://lkml.org/lkml/2014/8/21/252 - fix typo pointed by Sergei Shtylyov v4: https://lkml.org/lkml/2014/8/20/277 - change if() sequence into switch() statement v3: https://lkml.org/lkml/2014/7/30/115 - move fix for the redundant ep files problem into sepatare patch - merge user space API affecting changes into single patch - add flag switching between old and new style API v2: https://lkml.org/lkml/2014/7/25/296 - return proper endpont address in setup request handling - add patch usb: gadget: f_fs: add ioctl returning ep descriptor - add patch usb: gadget: f_fs: make numbers in ep file names the same as ep addresses v1: https://lkml.org/lkml/2014/7/18/1010 Robert Baldyga (3): usb: gadget: f_fs: fix the redundant ep files problem usb: gadget: f_fs: add ioctl returning ep descriptor usb: gadget: f_fs: virtual endpoint address mapping drivers/usb/gadget/function/f_fs.c | 112 +++- drivers/usb/gadget/function/u_fs.h | 4 ++ include/uapi/linux/usb/functionfs.h | 7 +++ 3 files changed, 110 insertions(+), 13 deletions(-) -- 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
[PATCH v6 3/3] usb: gadget: f_fs: virtual endpoint address mapping
This patch introduces virtual endpoint address mapping. It separates
function logic form physical endpoint addresses making it more hardware
independent.
Following modifications changes user space API, so to enable them user
have to switch on the FUNCTIONFS_VIRTUAL_ADDR flag in descriptors.
Endpoints are now refered using virtual endpoint addresses chosen by
user in endpoint descpriptors. This applies to each context when endpoint
address can be used:
- when accessing endpoint files in FunctionFS filesystemi (in file name),
- in setup requests directed to specific endpoint (in wIndex field),
- in descriptors returned by FUNCTIONFS_ENDPOINT_DESC ioctl.
In endpoint file names the endpoint address number is formatted as
double-digit hexadecimal value (ep%02x) which has few advantages -
it is easy to parse, allows to easly recognize endpoint direction basing
on its name (IN endpoint number starts with digit 8, and OUT with 0)
which can be useful for debugging purpose, and it makes easier to introduce
further features allowing to use each endpoint number in both directions
to have more endpoints available for function if hardware supports this
(for example we could have ep01 which is endpoint 1 with OUT direction,
and ep81 which is endpoint 1 with IN direction).
Physical endpoint address can be still obtained using ioctl named
FUNCTIONFS_ENDPOINT_REVMAP, but now it's not neccesary to handle
USB transactions properly.
Signed-off-by: Robert Baldyga r.bald...@samsung.com
Acked-by: Michal Nazarewicz min...@mina86.com
---
drivers/usb/gadget/function/f_fs.c | 23 +--
drivers/usb/gadget/function/u_fs.h | 2 ++
include/uapi/linux/usb/functionfs.h | 1 +
3 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/function/f_fs.c
b/drivers/usb/gadget/function/f_fs.c
index 6edf7e4..1033316 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -1557,7 +1557,10 @@ static int ffs_epfiles_create(struct ffs_data *ffs)
epfile-ffs = ffs;
mutex_init(epfile-mutex);
init_waitqueue_head(epfile-wait);
- sprintf(epfiles-name, ep%u, i);
+ if (ffs-user_flags FUNCTIONFS_VIRTUAL_ADDR)
+ sprintf(epfiles-name, ep%02x, ffs-eps_addrmap[i]);
+ else
+ sprintf(epfiles-name, ep%u, i);
if (!unlikely(ffs_sb_create_file(ffs-sb, epfiles-name, epfile,
ffs_epfile_operations,
epfile-dentry))) {
@@ -2106,10 +2109,12 @@ static int __ffs_data_got_descs(struct ffs_data *ffs,
break;
case FUNCTIONFS_DESCRIPTORS_MAGIC_V2:
flags = get_unaligned_le32(data + 8);
+ ffs-user_flags = flags;
if (flags ~(FUNCTIONFS_HAS_FS_DESC |
FUNCTIONFS_HAS_HS_DESC |
FUNCTIONFS_HAS_SS_DESC |
- FUNCTIONFS_HAS_MS_OS_DESC)) {
+ FUNCTIONFS_HAS_MS_OS_DESC |
+ FUNCTIONFS_VIRTUAL_ADDR)) {
ret = -ENOSYS;
goto error;
}
@@ -2464,7 +2469,13 @@ static int __ffs_func_bind_do_descs(enum ffs_entity_type
type, u8 *valuep,
} else {
struct usb_request *req;
struct usb_ep *ep;
+ u8 bEndpointAddress;
+ /*
+* We back up bEndpointAddress because autoconfig overwrites
+* it with physical endpoint address.
+*/
+ bEndpointAddress = ds-bEndpointAddress;
pr_vdebug(autoconfig\n);
ep = usb_ep_autoconfig(func-gadget, ds);
if (unlikely(!ep))
@@ -2479,6 +2490,12 @@ static int __ffs_func_bind_do_descs(enum ffs_entity_type
type, u8 *valuep,
ffs_ep-req = req;
func-eps_revmap[ds-bEndpointAddress
USB_ENDPOINT_NUMBER_MASK] = idx + 1;
+ /*
+* If we use virtual address mapping, we restore
+* original bEndpointAddress value.
+*/
+ if (func-ffs-user_flags FUNCTIONFS_VIRTUAL_ADDR)
+ ds-bEndpointAddress = bEndpointAddress;
}
ffs_dump_mem(: Rewritten ep desc, ds, ds-bLength);
@@ -2923,6 +2940,8 @@ static int ffs_func_setup(struct usb_function *f,
ret = ffs_func_revmap_ep(func, le16_to_cpu(creq-wIndex));
if (unlikely(ret 0))
return ret;
+ if (func-ffs-user_flags FUNCTIONFS_VIRTUAL_ADDR)
+ ret = func-ffs-eps_addrmap[ret];
break;
default:
diff --git a/drivers/usb/gadget/function/u_fs.h
b/drivers/usb/gadget/function/u_fs.h
index
Re: [PATCH 0/4] usb: gadget: f_uac2: cleanups and capture timing
On Mon, Aug 25, 2014 at 2:14 PM, Daniel Mack dan...@zonque.org wrote:
Hi Clemens,
On 08/25/2014 09:15 AM, Clemens Ladisch wrote:
Daniel Mack wrote:
a) Linux snd-usb-audio currently pre-calculates the estimated packet
sizes to expect from a USB device, and will only receive packets that
have the expected size or are smaller.
snd-usb-audio allows packets to be 25 % larger.
Yes, but packets can't be larger than *that*.
This can be worked around by setting the UAC_EP_CS_ATTR_FILL_MAX in
the UAC2 endpoint descriptor
The spec says about this flag (4.10.1.2):
| when receiving data from an IN endpoint, the Host software must be
| prepared to receive wMaxPacketSize bytes and discard any superfluous
| zero bytes in the packet.
| Note: This bit can only be used for Type II formatted data. The data
|stream must contain enough information (in a header) to
|separate the actual data from the padded zero bytes.
Right, I've read this too, and we're not using Type II, so we don't have
header information to tell us the real payload. The idea was to just use
an 0 or 512 bytes approach.
send 0-byte packets from agdev_iso_complete() if the time passed since
the last completed buffer does not allow for another one to be sent.
Audio Formats, 2.1:
| To indicate a temporary stop in the isochronous data stream ..., an
| in-band Transfer Delimiter needs to be defined. This specification
| considers two situations to be a Transfer Delimiter. The first is
| a zero-length data packet and the second is the absence of an
| isochronous transfer
2.3.1.1:
| The goal must be to keep the instantaneous number of audio slots per
| virtual frame, ni as close as possible to the average number of audio
| slots per virtual frame, nav. [...] If the sampling rate is a constant,
| the allowable variation on ni is limited to one audio slot, that is,
| ∆ni = 1. This implies that all virtual frame packets must either
| contain INT(nav) audio slots (small VFP) or INT(nav) + 1 (large VFP)
| audio slots.
This results in very stable timing behavior in my tests.
But it increases jitter, and might not work with any other driver.
You're right, that's also my concern.
f_uac(2) *must* implement some mechanism to control the clock, i.e., the
packet sizes. (And this is not part of the standard ALSA API.)
The easiest is probably really to just calculate correct packet sizes
and stick to them. After all, the actual clock is really arbitrary, we
just have to pick something that is in the range of the sample rate.
I'll cook up an alternative patch and do some tests with Sebastian off-list.
How about configuring bInterval and wMaxPacketSize to get the desired rate?
For ex, 48KHz/2/S16, we need 192bytes/millisec. So we set bInterval=1
(or 4 for HS) and wMaxPacketSize=192 for that configuration. For
44.1KHz/2/S16 we need 176.4bytes/millisec, so we set
wMaxPacketSize=178 and send packets of length in {176, 176, 176,
176,178} pattern.
Regards,
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 0/4] usb: gadget: f_uac2: cleanups and capture timing
On 08/25/2014 11:23 AM, Jassi Brar wrote:
On Mon, Aug 25, 2014 at 2:14 PM, Daniel Mack dan...@zonque.org wrote:
The easiest is probably really to just calculate correct packet sizes
and stick to them. After all, the actual clock is really arbitrary, we
just have to pick something that is in the range of the sample rate.
I'll cook up an alternative patch and do some tests with Sebastian off-list.
How about configuring bInterval and wMaxPacketSize to get the desired rate?
For ex, 48KHz/2/S16, we need 192bytes/millisec. So we set bInterval=1
(or 4 for HS) and wMaxPacketSize=192 for that configuration. For
44.1KHz/2/S16 we need 176.4bytes/millisec, so we set
wMaxPacketSize=178 and send packets of length in {176, 176, 176,
176,178} pattern.
Yes, something like that. But you can't modify wMaxPacketSize in
accordance to the sample rate and format, but just send short packets.
I'm currently testing a patch which does that.
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 0/4] usb: gadget: f_uac2: cleanups and capture timing
On Mon, Aug 25, 2014 at 2:57 PM, Daniel Mack dan...@zonque.org wrote:
On 08/25/2014 11:23 AM, Jassi Brar wrote:
On Mon, Aug 25, 2014 at 2:14 PM, Daniel Mack dan...@zonque.org wrote:
The easiest is probably really to just calculate correct packet sizes
and stick to them. After all, the actual clock is really arbitrary, we
just have to pick something that is in the range of the sample rate.
I'll cook up an alternative patch and do some tests with Sebastian off-list.
How about configuring bInterval and wMaxPacketSize to get the desired rate?
For ex, 48KHz/2/S16, we need 192bytes/millisec. So we set bInterval=1
(or 4 for HS) and wMaxPacketSize=192 for that configuration. For
44.1KHz/2/S16 we need 176.4bytes/millisec, so we set
wMaxPacketSize=178 and send packets of length in {176, 176, 176,
176,178} pattern.
Yes, something like that. But you can't modify wMaxPacketSize in
accordance to the sample rate and format, but just send short packets.
We can't change rate once the f_uac2 module is loaded. So
wMaxPacketSize changes only across module loads.
Cheers,
-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 0/4] usb: gadget: f_uac2: cleanups and capture timing
On 08/25/2014 11:30 AM, Jassi Brar wrote:
On Mon, Aug 25, 2014 at 2:57 PM, Daniel Mack dan...@zonque.org wrote:
On 08/25/2014 11:23 AM, Jassi Brar wrote:
On Mon, Aug 25, 2014 at 2:14 PM, Daniel Mack dan...@zonque.org wrote:
The easiest is probably really to just calculate correct packet sizes
and stick to them. After all, the actual clock is really arbitrary, we
just have to pick something that is in the range of the sample rate.
I'll cook up an alternative patch and do some tests with Sebastian
off-list.
How about configuring bInterval and wMaxPacketSize to get the desired rate?
For ex, 48KHz/2/S16, we need 192bytes/millisec. So we set bInterval=1
(or 4 for HS) and wMaxPacketSize=192 for that configuration. For
44.1KHz/2/S16 we need 176.4bytes/millisec, so we set
wMaxPacketSize=178 and send packets of length in {176, 176, 176,
176,178} pattern.
Yes, something like that. But you can't modify wMaxPacketSize in
accordance to the sample rate and format, but just send short packets.
We can't change rate once the f_uac2 module is loaded. So
wMaxPacketSize changes only across module loads.
Yes, but we shouldn't rely on wMaxPacketSize but really just send
packets of the right size. This is what other USB audio devices do as well.
And btw - we could also change the logic of f_uac2 so the sample rate
can be changed at runtime. The only constaint is that the counterpart
device on the gadget side must not be active when this happens. Whatever
part of the system comes up first (USB or ALSA) defines the sample rate
and the format. But I'll save that for later :)
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
[PATCH] xhci: Disable streams on Via XHCI with device-id 0x3432
This is a bit bigger hammer then I would like to use for this, but for now it will have to make do. I'm working on getting my hands on one of these so that I can try to get streams to work (with a quirk flag if necessary) and then we can re-enable them. For now this at least makes uas capable disk enclosures work again by forcing fallback to the usb-storage driver. https://bugzilla.kernel.org/show_bug.cgi?id=79511 Cc: sta...@vger.kernel.org # 3.15 Signed-off-by: Hans de Goede hdego...@redhat.com --- drivers/usb/host/xhci-pci.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index 687d366..d973682 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -151,6 +151,11 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci) if (pdev-vendor == PCI_VENDOR_ID_VIA) xhci-quirks |= XHCI_RESET_ON_RESUME; + /* See https://bugzilla.kernel.org/show_bug.cgi?id=79511 */ + if (pdev-vendor == PCI_VENDOR_ID_VIA + pdev-device == 0x3432) + xhci-quirks |= XHCI_BROKEN_STREAMS; + if (xhci-quirks XHCI_RESET_ON_RESUME) xhci_dbg_trace(xhci, trace_xhci_dbg_quirks, QUIRK: Resetting on resume); -- 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 1/1] HID: usbhid: add usb_clear_halt determination for next hid_start_in
On Fri, 2014-08-22 at 14:23 -0400, Alan Stern wrote:
On Sat, 23 Aug 2014, vichy wrote:
from your patch, I have some questions:
a. in Alan's version, if both HID_CLEAR_HALT and HID_RESET_PENDING are
set, hid_reset will both clear ep halt and reset devcie.
But in original one, even HID_CLEAR_HALT and HID_RESET_PENDING are
both set, hid_reset only do one of them.
Yes. In my patch, the clear-halt handler will turn on the
HID_RESET_PENDING bit if something goes wrong. In that case we want to
do both things.
Why? If we reset, why bother clearing a halt? Especially as this
may mean waiting the full 5 seconds for a timeout.
is there any special reason in original hid_reset to use below flow?
if (test_bit(HID_CLEAR_HALT, usbhid-iofl)) {
x
} else if (test_bit(HID_RESET_PENDING, usbhid-iofl)) {
xx
}
No special reason. We probably never thought that both flags would be
set.
Yes. And I'd say if this ever happens HID_RESET_PENDING must have
priority and implicitly clears a halt.
Regards
Oliver
--
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: [PATCHv2 10/13] usb: dwc2: initialize the spin_lock for both host and gadget
On 8/22/14, 3:57 PM, Felipe Balbi wrote: Hi, On Fri, Aug 22, 2014 at 08:52:23PM +, Paul Zimmerman wrote: From: dingu...@altera.com [mailto:dingu...@altera.com] Sent: Wednesday, July 30, 2014 8:21 AM Move spin_lock_init to common location for both host and gadget. Signed-off-by: Dinh Nguyen dingu...@altera.com --- drivers/usb/dwc2/hcd.c |1 - drivers/usb/dwc2/platform.c |1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index 07a7bcd..c6778d9 100644 --- a/drivers/usb/dwc2/hcd.c +++ b/drivers/usb/dwc2/hcd.c @@ -2824,7 +2824,6 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq, hcd-has_tt = 1; - spin_lock_init(hsotg-lock); ((struct wrapper_priv_data *) hcd-hcd_priv)-hsotg = hsotg; hsotg-priv = hcd; diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c index eb2a131..4898268 100644 --- a/drivers/usb/dwc2/platform.c +++ b/drivers/usb/dwc2/platform.c @@ -197,6 +197,7 @@ static int dwc2_driver_probe(struct platform_device *dev) } platform_set_drvdata(dev, hsotg); + spin_lock_init(hsotg-lock); return retval; } Hi Dinh, I don't have a copy of your v3 patches in my mailbox anymore, so I am replying to the v2 one instead. Are you absolutely sure that no code that takes the spinlock can be called before this point? This is the last line in the probe() function, so I have a hard time believing it is safe to initialize the spinlock this late. In particular, the IRQ has already been attached, and usb_add_gadget_udc() has already been called. So it seems entirely possible that some other entity could try to access the driver before this point. you're right with this comment. request_irq() enables the IRQ line and it could be that we already have a pending event to handle which fires as soon as we enable that IRQ line. Yes, thanks for catching this. I will move the call up in v4. Dinh -- 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
AS2105-based enclosure size issues with 2TB HDDs
Hi all, I'm investigating an issue with a HDD enclosure based on AS2105 chip. A 4TB GPT partition is not considered valid because reported last LBA reported by the enclosure makes kernel think that secondary GPT is outside disk plate. I found this patch [1] forcing it to use READ_CAPACITY_16 first and fall back to READ_CAPACITY_10 if no success. This enclosure has the same vendor and product IDs, but behavior is a bit different: READ_CAPACITY_16 fails 100% of times as unsupported command. READ_CAPACITY_10 has a distinct behavior depending on HDD size: - 1TB and 2TB: READ_CAPACITY_10 returns correct size value - 3TB and 4TB: READ_CAPACITY_10 returns size in a 2TB modulus If we fix capacity size by reporting (READ_CAPACITY_10 + MODULO_2TB), the result will be invalid when user plugs a 2TB HDD. An idea (bring by Oliver) is: first guess reading last sector using modulus result to check if size is valid. Any other ideas? There is better way to detect if enclosure is returning real LBA capacity or a modulo 2TB result? Thanks, Alfredo Dal'Ava Júnior [1] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/storage?id=32c37fc30c52508711ea6a108cfd5855b8a07176 -- 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/2] usb: chipidea: msm: Use USB PHY API to control PHY state
PHY drivers keep track of the current state of the hardware,
so don't change PHY settings under it.
Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com
Acked-by: Felipe Balbi ba...@ti.com
---
drivers/usb/chipidea/ci_hdrc_msm.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c
b/drivers/usb/chipidea/ci_hdrc_msm.c
index d72b9d2..30bdd51 100644
--- a/drivers/usb/chipidea/ci_hdrc_msm.c
+++ b/drivers/usb/chipidea/ci_hdrc_msm.c
@@ -20,7 +20,6 @@
static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci, unsigned event)
{
struct device *dev = ci-gadget.dev.parent;
- int val;
switch (event) {
case CI_HDRC_CONTROLLER_RESET_EVENT:
@@ -34,10 +33,7 @@ static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci,
unsigned event)
* Put the transceiver in non-driving mode. Otherwise host
* may not detect soft-disconnection.
*/
- val = usb_phy_io_read(ci-transceiver, ULPI_FUNC_CTRL);
- val = ~ULPI_FUNC_CTRL_OPMODE_MASK;
- val |= ULPI_FUNC_CTRL_OPMODE_NONDRIVING;
- usb_phy_io_write(ci-transceiver, val, ULPI_FUNC_CTRL);
+ usb_phy_notify_disconnect(ci-transceiver, USB_SPEED_UNKNOWN);
break;
default:
dev_dbg(dev, unknown ci_hdrc event\n);
--
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
[PATCH 2/2] usb: chipidea: msm: Initialize PHY on reset event
Initialize USB PHY after every Link controller reset Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com --- drivers/usb/chipidea/ci_hdrc_msm.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/chipidea/ci_hdrc_msm.c b/drivers/usb/chipidea/ci_hdrc_msm.c index 30bdd51..4935ac3 100644 --- a/drivers/usb/chipidea/ci_hdrc_msm.c +++ b/drivers/usb/chipidea/ci_hdrc_msm.c @@ -26,6 +26,7 @@ static void ci_hdrc_msm_notify_event(struct ci_hdrc *ci, unsigned event) dev_dbg(dev, CI_HDRC_CONTROLLER_RESET_EVENT received\n); writel(0, USB_AHBBURST); writel(0, USB_AHBMODE); + usb_phy_init(ci-transceiver); break; case CI_HDRC_CONTROLLER_STOPPED_EVENT: dev_dbg(dev, CI_HDRC_CONTROLLER_STOPPED_EVENT received\n); -- 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 1/1] HID: usbhid: add usb_clear_halt determination for next hid_start_in
hi Alan: usb_unbind_and_rebind_marked_interfaces is called if config parameter is not null, it seems no matter post_reset routine fail or not. Yes, that's right. I should have said: Because the post_reset routine failed, usb_unbind_and_rebind_marked_interfaces indirectly calls usbhid_disconnect. Even post_reset routine failed, usb_unbind_and_rebind_marked_interfaces will still indirectly calls usbhid_disconnect, right? I don't remember the entire call chain. It was pretty long. hid_destroy_device calls hid_remove_device, which calls device_del, which calls lots of other things. If you really want to see all the details, put a dump_stack() call in usbhid_close and examine what it prints in the kernel log when you unplug an HID device. I found what you mentioned ^^ Thanks for your kind help, -- 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/1] HID: usbhid: add usb_clear_halt determination for next hid_start_in
hi Oliver: 2014-08-25 18:21 GMT+08:00 Oliver Neukum oneu...@suse.de: On Fri, 2014-08-22 at 14:23 -0400, Alan Stern wrote: On Sat, 23 Aug 2014, vichy wrote: from your patch, I have some questions: a. in Alan's version, if both HID_CLEAR_HALT and HID_RESET_PENDING are set, hid_reset will both clear ep halt and reset devcie. But in original one, even HID_CLEAR_HALT and HID_RESET_PENDING are both set, hid_reset only do one of them. Yes. In my patch, the clear-halt handler will turn on the HID_RESET_PENDING bit if something goes wrong. In that case we want to do both things. Why? If we reset, why bother clearing a halt? Especially as this may mean waiting the full 5 seconds for a timeout. I think what Alan mean is IF CLEAR HALT fail, we reset the device. That is what below In that case mean. In that case we want to do both things. BR, -- 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: some question about unbind and rebind usb interfaces
hi Alan: After usb_reset_device, the whole enumeration will run again, No, only part of the enumeration. The kernel does read the device and config descriptors again. But if the reset succeeded, the kernel doesn't carry out any of the other parts of enumeration. here the kernel you mean above is usb HCD driver, right? the other parts of enumeration you mean is class driver, right? why this patch say the previous solution will be fail to claim additional interface? Suppose a driver claims interfaces 0 and 1, and then they have to be unbound and rebound. The old code would unbind interface 0, then rebind it, then unbind interface 1, and then rebind it. Suppose the driver's probe routine for interface 0 tries to claim interface 1? The probe routine runs when interface 0 is rebound. At Is there any class driver will try to claim both interface 0 and 1? As I remember correctly, the usb class driver is passively called with different interface class ID matched. that time interface 1 hasn't been unbound yet. Since interface 1 is still owned, the driver will not be able to claim it. On the other hand, the new code will unbind interface 0, then unbind interface 1, then rebind interface 0, and then rebind interface 1. Now if the driver's probe routine for interface 0 tries to claim interface 1, the claim will succeed because interface has already been unbound. appreciate your kind help, -- 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: AS2105-based enclosure size issues with 2TB HDDs
On Mon, 2014-08-25 at 10:58 +, Alfredo Dal Ava Junior wrote:
- 1TB and 2TB: READ_CAPACITY_10 returns correct size value
- 3TB and 4TB: READ_CAPACITY_10 returns size in a 2TB modulus
If we fix capacity size by reporting (READ_CAPACITY_10 + MODULO_2TB), the
result
will be invalid when user plugs a 2TB HDD. An idea (bring by Oliver) is:
It is worse than that. Pretty soon a 4.7TB disk will also be plausible.
first guess reading last sector using modulus result to check if size is
valid.
It is necessary that a virgin disk also be handled correctly.
We cannot use the partition table (besides it being a layering
violation)
It would propose (in pseudo code)
if (read_capacity_16(device) 0) {
lower_limit = read_capacity_10(device);
for (i = 1; i++; i SANITY_LIMIT) {
err = read_sector(device, lower_limit + i * 2TB-SAFETY);
if (err == OUT_OF_RANGE)
break;
}
if (i SANITY_LIMIT)
return (i - 1) * 2TB + lower_limit;
else
return ERROR;
Regards
Oliver
--
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/4] usb: gadget: f_uac2: cleanups and capture timing
On Mon, Aug 25, 2014 at 3:07 PM, Daniel Mack dan...@zonque.org wrote:
On 08/25/2014 11:30 AM, Jassi Brar wrote:
On Mon, Aug 25, 2014 at 2:57 PM, Daniel Mack dan...@zonque.org wrote:
On 08/25/2014 11:23 AM, Jassi Brar wrote:
On Mon, Aug 25, 2014 at 2:14 PM, Daniel Mack dan...@zonque.org wrote:
The easiest is probably really to just calculate correct packet sizes
and stick to them. After all, the actual clock is really arbitrary, we
just have to pick something that is in the range of the sample rate.
I'll cook up an alternative patch and do some tests with Sebastian
off-list.
How about configuring bInterval and wMaxPacketSize to get the desired rate?
For ex, 48KHz/2/S16, we need 192bytes/millisec. So we set bInterval=1
(or 4 for HS) and wMaxPacketSize=192 for that configuration. For
44.1KHz/2/S16 we need 176.4bytes/millisec, so we set
wMaxPacketSize=178 and send packets of length in {176, 176, 176,
176,178} pattern.
Yes, something like that. But you can't modify wMaxPacketSize in
accordance to the sample rate and format, but just send short packets.
We can't change rate once the f_uac2 module is loaded. So
wMaxPacketSize changes only across module loads.
Yes, but we shouldn't rely on wMaxPacketSize but really just send
packets of the right size. This is what other USB audio devices do as well.
Yup, that should do too. Setting wMaxPacketSize to just enough length
seemed more optimized and robust to me. But I have not strong feelings
here.
And btw - we could also change the logic of f_uac2 so the sample rate
can be changed at runtime. The only constaint is that the counterpart
device on the gadget side must not be active when this happens. Whatever
part of the system comes up first (USB or ALSA) defines the sample rate
and the format. But I'll save that for later :)
Properly supporting multiple sampling rates required a topology more
complicated than now, so I skipped it. IIRC some 'clock selector' unit
needs to be added to have a UAC2 compliant method to change rates from
Host side.
Thanks
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 v2 2/3] usb: gadget/uvc: also handle v4l2 ioctl ENUM_FMT
Hi Laurent,
On Wed, Aug 20, 2014 at 07:05:30PM +0200, Laurent Pinchart wrote:
Hi Hans and Michael,
On Wednesday 20 August 2014 02:06:54 Hans Verkuil wrote:
On 08/19/2014 05:01 PM, Laurent Pinchart wrote:
Hi Michael,
Thank you for the patch.
(CC'ing Hans Verkuil and the linux-media mailing list)
On Friday 08 August 2014 17:38:58 Michael Grzeschik wrote:
This patch adds ENUM_FMT as possible ioctl to the uvc v4l2 device.
That makes userspace applications with a generic IOCTL calling
convention make also use of it.
Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
---
v1 - v2:
- changed first switch case to simple if
- added separate function
- added description field
- bail out on array boundaries
drivers/usb/gadget/uvc_v4l2.c | 30 --
1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/uvc_v4l2.c
b/drivers/usb/gadget/uvc_v4l2.c
index ad48e81..58633bf 100644
--- a/drivers/usb/gadget/uvc_v4l2.c
+++ b/drivers/usb/gadget/uvc_v4l2.c
@@ -55,14 +55,30 @@ struct uvc_format
{
u8 bpp;
u32 fcc;
+char *description;
};
static struct uvc_format uvc_formats[] = {
-{ 16, V4L2_PIX_FMT_YUYV },
-{ 0, V4L2_PIX_FMT_MJPEG },
+{ 16, V4L2_PIX_FMT_YUYV, YUV 4:2:2 },
+{ 0, V4L2_PIX_FMT_MJPEG, MJPEG },
Format descriptions are currently duplicated in every driver, causing
higher memory usage and different descriptions for the same format
depending on the driver. Hans, should we try to fix this ?
Yes, we should. It's been on my todo list for ages, but at a very low
priority. I'm not planning to work on this in the near future, but if
someone else wants to work on this, then just go ahead.
Michael, would you like to give this a try, or should I do it ?
It seems Philipp is already taking the chance! :)
Regards,
Michael
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
--
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/1] HID: usbhid: add usb_clear_halt determination for next hid_start_in
On Mon, 25 Aug 2014, vichy wrote: hi Oliver: 2014-08-25 18:21 GMT+08:00 Oliver Neukum oneu...@suse.de: On Fri, 2014-08-22 at 14:23 -0400, Alan Stern wrote: On Sat, 23 Aug 2014, vichy wrote: from your patch, I have some questions: a. in Alan's version, if both HID_CLEAR_HALT and HID_RESET_PENDING are set, hid_reset will both clear ep halt and reset devcie. But in original one, even HID_CLEAR_HALT and HID_RESET_PENDING are both set, hid_reset only do one of them. Yes. In my patch, the clear-halt handler will turn on the HID_RESET_PENDING bit if something goes wrong. In that case we want to do both things. Why? If we reset, why bother clearing a halt? Especially as this may mean waiting the full 5 seconds for a timeout. I think what Alan mean is IF CLEAR HALT fail, we reset the device. That is what below In that case mean. In that case we want to do both things. Exactly. Suppose initially HID_CLEAR_HALT is set and HID_RESET_PENDING is off. If the usb_clear_halt call fails, we want to recover by performing a reset. 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: some question about unbind and rebind usb interfaces
On Mon, 25 Aug 2014, vichy wrote: hi Alan: After usb_reset_device, the whole enumeration will run again, No, only part of the enumeration. The kernel does read the device and config descriptors again. But if the reset succeeded, the kernel doesn't carry out any of the other parts of enumeration. here the kernel you mean above is usb HCD driver, right? I mean usbcore. the other parts of enumeration you mean is class driver, right? If the reset succeeds, usbcore does not call usb_choose_configuration or usb_set_configuration. It also does not create any uevents or send any notifications. why this patch say the previous solution will be fail to claim additional interface? Suppose a driver claims interfaces 0 and 1, and then they have to be unbound and rebound. The old code would unbind interface 0, then rebind it, then unbind interface 1, and then rebind it. Suppose the driver's probe routine for interface 0 tries to claim interface 1? The probe routine runs when interface 0 is rebound. At Is there any class driver will try to claim both interface 0 and 1? Yes. You can find them easily by searching for calls to usb_driver_claim_interface. As I remember correctly, the usb class driver is passively called with different interface class ID matched. Not always. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] HID: usbhid: add usb_clear_halt determination for next hid_start_in
On Mon, 25 Aug 2014, vichy wrote: hi Alan: usb_unbind_and_rebind_marked_interfaces is called if config parameter is not null, it seems no matter post_reset routine fail or not. Yes, that's right. I should have said: Because the post_reset routine failed, usb_unbind_and_rebind_marked_interfaces indirectly calls usbhid_disconnect. Even post_reset routine failed, usb_unbind_and_rebind_marked_interfaces will still indirectly calls usbhid_disconnect, right? If the pre_reset and post_reset routines both return 0, usb_unbind_and_rebind_marked_interfaces will _not_ call usbhid_disconnect. Otherwise it will. 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: [Bug 80711] [PATCH]SG_FLAG_LUN_INHIBIT is no longer implemented and there's not way to prevent the kernel from using the 2nd cdb byte for the LUN
On Sun, 24 Aug 2014, Christoph Hellwig wrote: On Fri, Aug 22, 2014 at 01:29:32PM -0400, Alan Stern wrote: Other than this, I'm fine with the code ... you can add the acked by from me when we resolve the above question. Okay. It's true that this issue is only tangentially related to the main point of the patch. It could be removed and addressed later. Just make it a separate patch and send it along.. All right. But I still want to know first whether the patch really fixes the original problem. Tiziano, do you intend to test this patch? James, can you explain how the INQUIRY command in scsi_probe_lun() managed to work back in the days when multi-lun SCSI-2 devices were common? sdev-scsi_level doesn't get set when sdev is allocated, so it initially contains 0, so the LUN bits won't get filled in when the first INQUIRY command is sent. Then how could the target know which logical unit the INQUIRY was meant for? 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 2/3] usb: gadget/uvc: also handle v4l2 ioctl ENUM_FMT
Hi Michael,
On Monday 25 August 2014 15:59:57 Michael Grzeschik wrote:
On Wed, Aug 20, 2014 at 07:05:30PM +0200, Laurent Pinchart wrote:
On Wednesday 20 August 2014 02:06:54 Hans Verkuil wrote:
On 08/19/2014 05:01 PM, Laurent Pinchart wrote:
Hi Michael,
Thank you for the patch.
(CC'ing Hans Verkuil and the linux-media mailing list)
On Friday 08 August 2014 17:38:58 Michael Grzeschik wrote:
This patch adds ENUM_FMT as possible ioctl to the uvc v4l2 device.
That makes userspace applications with a generic IOCTL calling
convention make also use of it.
Signed-off-by: Michael Grzeschik m.grzesc...@pengutronix.de
---
v1 - v2:
- changed first switch case to simple if
- added separate function
- added description field
- bail out on array boundaries
drivers/usb/gadget/uvc_v4l2.c | 30 --
1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/uvc_v4l2.c
b/drivers/usb/gadget/uvc_v4l2.c
index ad48e81..58633bf 100644
--- a/drivers/usb/gadget/uvc_v4l2.c
+++ b/drivers/usb/gadget/uvc_v4l2.c
@@ -55,14 +55,30 @@ struct uvc_format
{
u8 bpp;
u32 fcc;
+ char *description;
};
static struct uvc_format uvc_formats[] = {
- { 16, V4L2_PIX_FMT_YUYV },
- { 0, V4L2_PIX_FMT_MJPEG },
+ { 16, V4L2_PIX_FMT_YUYV, YUV 4:2:2 },
+ { 0, V4L2_PIX_FMT_MJPEG, MJPEG },
Format descriptions are currently duplicated in every driver, causing
higher memory usage and different descriptions for the same format
depending on the driver. Hans, should we try to fix this ?
Yes, we should. It's been on my todo list for ages, but at a very low
priority. I'm not planning to work on this in the near future, but if
someone else wants to work on this, then just go ahead.
Michael, would you like to give this a try, or should I do it ?
It seems Philipp is already taking the chance! :)
Perfect timing, I wonder if that's just a coincidence ;-)
I don't think this patch is very urgent, would you be fine with rebasing it on
top of Philipp's patch when it will be accepted ?
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] usb: gadget/uvc: also handle v4l2 ioctl ENUM_FMT
Hi Laurent, Am Montag, den 25.08.2014, 16:48 +0200 schrieb Laurent Pinchart: [...] Format descriptions are currently duplicated in every driver, causing higher memory usage and different descriptions for the same format depending on the driver. Hans, should we try to fix this ? Yes, we should. It's been on my todo list for ages, but at a very low priority. I'm not planning to work on this in the near future, but if someone else wants to work on this, then just go ahead. Michael, would you like to give this a try, or should I do it ? It seems Philipp is already taking the chance! :) Perfect timing, I wonder if that's just a coincidence ;-) It felt like my own idea this weekend, but I strongly suspect that I took up enough information to trigger it, when scanning mail last week. I shouldn't be so fast to dismiss uvc/gadget mails as Michael's business... regards Philipp -- 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 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. 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
[PATCH 2/2] USB: quirks: enable device-qualifier quirk for Elan Touchscreen
Enable device-qualifier quirk for Elan Touchscreen, which often fails to
handle requests for the device_descriptor.
Note that the device sometimes do respond properly with a Request Error
(three times as USB core retries), but usually fails to respond at all.
When this happens any further descriptor requests also fails, for
example:
[ 1528.688934] usb 2-7: new full-speed USB device number 4 using xhci_hcd
[ 1530.945588] usb 2-7: unable to read config index 0 descriptor/start: -71
[ 1530.945592] usb 2-7: can't read configurations, error -71
This has been observed repeating for over a minute before eventual
successful enumeration.
Reported-by: Drew Von Spreecken dre...@gmail.com
Reported-by: Greg Kroah-Hartman gre...@linuxfoundation.org
Signed-off-by: Johan Hovold jo...@kernel.org
---
drivers/usb/core/quirks.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index 33c42de..e8a0ddb 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -93,6 +93,10 @@ static const struct usb_device_id usb_quirk_list[] = {
{ USB_DEVICE(0x04e8, 0x6601), .driver_info =
USB_QUIRK_CONFIG_INTF_STRINGS },
+ /* Elan Touchscreen */
+ { USB_DEVICE(0x04f3, 0x0089), .driver_info =
+ USB_QUIRK_DEVICE_QUALIFIER },
+
/* Roland SC-8820 */
{ USB_DEVICE(0x0582, 0x0007), .driver_info = USB_QUIRK_RESET_RESUME },
--
1.8.5.5
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] USB: core: add device-qualifier quirk
Add new quirk for devices that cannot handle requests for the device_qualifier descriptor. A USB-2.0 compliant device must respond to requests for the device_qualifier descriptor (even if it's with a request error), but at least one device is known to misbehave after such a request. Suggested-by: Bjørn Mork bj...@mork.no Signed-off-by: Johan Hovold jo...@kernel.org --- drivers/usb/core/hub.c | 3 +++ include/linux/usb/quirks.h | 3 +++ 2 files changed, 6 insertions(+) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 0e950ad..e563164 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -4475,6 +4475,9 @@ check_highspeed (struct usb_hub *hub, struct usb_device *udev, int port1) struct usb_qualifier_descriptor *qual; int status; + if (udev-quirks USB_QUIRK_DEVICE_QUALIFIER) + return; + qual = kmalloc (sizeof *qual, GFP_KERNEL); if (qual == NULL) return; diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h index 55a17b1..ffe565c 100644 --- a/include/linux/usb/quirks.h +++ b/include/linux/usb/quirks.h @@ -41,4 +41,7 @@ */ #define USB_QUIRK_LINEAR_UFRAME_INTR_BINTERVAL 0x0080 +/* device can't handle device_qualifier descriptor requests */ +#define USB_QUIRK_DEVICE_QUALIFIER 0x0100 + #endif /* __LINUX_USB_QUIRKS_H */ -- 1.8.5.5 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] USB: core: add add device-qualifier quirk
This is quirk is indeed needed to get the Elan Touchscreen found on some Samsung laptops to enumerate reliably. I'm still looking into the disconnects I've been experiencing. For some reason I cannot reproduce the repeated disconnects any longer, although the device still disconnects at least once if an input event occurs after wake up (when the device is not open) or close. Johan Johan Hovold (2): USB: core: add device-qualifier quirk USB: quirks: enable device-qualifier quirk for Elan Touchscreen drivers/usb/core/hub.c | 3 +++ drivers/usb/core/quirks.c | 4 include/linux/usb/quirks.h | 3 +++ 3 files changed, 10 insertions(+) -- 1.8.5.5 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/4] 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 pre-calculating the size of each packet to meet the
requested sample rate and format. This won't be 100% accurate, but
that's acceptable. Audio applications have to adopt to the stream's
rate anyway. The important thing here is to make f_uac2 operate at
least rougly in the range of speed that is expected by the host.
Signed-off-by: Daniel Mack zon...@gmail.com
---
drivers/usb/gadget/function/f_uac2.c | 27 ++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/function/f_uac2.c
b/drivers/usb/gadget/function/f_uac2.c
index efe8add..610a2f1 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -92,6 +92,8 @@ struct snd_uac2_chip {
struct snd_card *card;
struct snd_pcm *pcm;
+
+ unsigned int c_pktsize;
};
#define BUFF_SIZE_MAX (PAGE_SIZE * 16)
@@ -187,7 +189,7 @@ agdev_iso_complete(struct usb_ep *ep, struct usb_request
*req)
if (substream-stream == SNDRV_PCM_STREAM_PLAYBACK) {
src = prm-dma_area + prm-hw_ptr;
- req-actual = req-length;
+ req-length = req-actual = uac2-c_pktsize;
dst = req-buf;
} else {
dst = prm-dma_area + prm-hw_ptr;
@@ -1046,6 +1048,28 @@ err:
return -EINVAL;
}
+static void
+afunc_set_c_pktsize(struct usb_gadget *gadget, struct audio_dev *agdev)
+{
+ struct usb_endpoint_descriptor *ep_desc;
+ unsigned int rate, factor, interval;
+ struct f_uac2_opts *opts =
+ container_of(agdev-func.fi, struct f_uac2_opts, func_inst);
+
+ if (gadget-speed == USB_SPEED_FULL) {
+ ep_desc = fs_epin_desc;
+ factor = 1000;
+ } else {
+ ep_desc = hs_epin_desc;
+ factor = 125;
+ }
+
+ interval = (1 (ep_desc-bInterval - 1)) * factor;
+ rate = opts-c_srate * opts-c_ssize * num_channels(opts-c_chmask);
+ agdev-uac2.c_pktsize =
+ min_t(unsigned int, rate / interval, ep_desc-wMaxPacketSize);
+}
+
static int
afunc_set_alt(struct usb_function *fn, unsigned intf, unsigned alt)
{
@@ -1084,6 +1108,7 @@ afunc_set_alt(struct usb_function *fn, unsigned intf,
unsigned alt)
prm = uac2-p_prm;
config_ep_by_speed(gadget, fn, ep);
agdev-as_in_alt = alt;
+ afunc_set_c_pktsize(gadget, agdev);
} else {
dev_err(dev, %s:%d Error!\n, __func__, __LINE__);
return -EINVAL;
--
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 v2 2/4] 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 zon...@gmail.com
---
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 from this list: send the line unsubscribe linux-usb in
the body of
[PATCH v2 1/4] 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 zon...@gmail.com
---
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 v2 4/4] 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().
Now that we have 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 zon...@gmail.com
---
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 610a2f1..06446cc 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -159,8 +159,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;
@@ -187,26 +187,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-length = req-actual = uac2-c_pktsize;
- 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 v2 0/4] usb: gadget: f_uac2: cleanups and capture timing
Hi, this is v2 of the f_uac2 timing fixup series. 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. Sebastian, could you give these patches a try? They seem to work well on a BBB setup here. Thanks, Daniel Daniel Mack (4): usb: gadget: f_uac2: restructure some code in afunc_set_alt() usb: gadget: f_uac2: add short-hand for 'dev' usb: gadget: f_uac2: send reasonably sized packets usb: gadget: f_uac2: handle partial dma area wrap drivers/usb/gadget/function/f_uac2.c | 123 +-- 1 file changed, 74 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
BUG: xhci_hcd: Crashes system when reseting endpoint
Hello When I connect a usb gadget mass storage to my laptop the whole system crashes. It seems that the bug happens when the gadget/host initialize a reset. I lose all control to the computer, and the only way to recover control is rebooting it via powerbutton. Kernel that does not work: 3.14 3.16 (Debian) The host is a nec/renesas 3.0 with the latest firmware ricardo@neopili:~$ lspci -d 1033:0194 -vvv 0e:00.0 USB controller: NEC Corporation uPD720200 USB 3.0 Host Controller (rev 04) (prog-if 30 [XHCI]) Subsystem: Lenovo Device 21cf Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+ Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast TAbort- TAbort- MAbort- SERR- PERR- INTx- Latency: 0, Cache Line Size: 64 bytes Interrupt: pin A routed to IRQ 18 Region 0: Memory at d410 (64-bit, non-prefetchable) [size=8K] Capabilities: access denied Kernel driver in use: xhci_hcd The device is a linux 3.16 gadget (net2280) and works ok on a Windows machine and the usb 2.0 port. I cannot confirm 100%, but I am pretty sure that this same setup was working with linux 3.12. dmesg (with dyndbg enabled) : http://pastebin.com/raw.php?i=PRUiG7ng Might be related to: https://bugzilla.kernel.org/show_bug.cgi?id=75521 Thanks! Aug 25 17:59:32 neopili kernel: [ 366.580743] xhci_hcd :0e:00.0: Port Status Change Event for port 4 Aug 25 17:59:32 neopili kernel: [ 366.580756] xhci_hcd :0e:00.0: resume root hub Aug 25 17:59:32 neopili kernel: [ 366.580776] xhci_hcd :0e:00.0: handle_port_status: starting port polling. Aug 25 17:59:32 neopili kernel: [ 366.580866] xhci_hcd :0e:00.0: get port status, actual port 0 status = 0x2a0 Aug 25 17:59:32 neopili kernel: [ 366.580877] xhci_hcd :0e:00.0: Get port status returned 0x100 Aug 25 17:59:32 neopili kernel: [ 366.580917] xhci_hcd :0e:00.0: get port status, actual port 1 status = 0x202a0 Aug 25 17:59:32 neopili kernel: [ 366.580921] xhci_hcd :0e:00.0: Get port status returned 0x10100 Aug 25 17:59:32 neopili kernel: [ 366.580948] xhci_hcd :0e:00.0: clear port connect change, actual port 1 status = 0x2a0 Aug 25 17:59:32 neopili kernel: [ 366.582811] xhci_hcd :0e:00.0: Port Status Change Event for port 2 Aug 25 17:59:32 neopili kernel: [ 366.582833] xhci_hcd :0e:00.0: resume root hub Aug 25 17:59:32 neopili kernel: [ 366.582845] xhci_hcd :0e:00.0: handle_port_status: starting port polling. Aug 25 17:59:32 neopili kernel: [ 366.583095] xhci_hcd :0e:00.0: get port status, actual port 0 status = 0x2a0 Aug 25 17:59:32 neopili kernel: [ 366.583099] xhci_hcd :0e:00.0: Get port status returned 0x2a0 Aug 25 17:59:32 neopili kernel: [ 366.583124] xhci_hcd :0e:00.0: get port status, actual port 1 status = 0x21203 Aug 25 17:59:32 neopili kernel: [ 366.583127] xhci_hcd :0e:00.0: Get port status returned 0x10203 Aug 25 17:59:32 neopili kernel: [ 366.583143] xhci_hcd :0e:00.0: clear port connect change, actual port 1 status = 0x1203 Aug 25 17:59:32 neopili kernel: [ 366.655724] xhci_hcd :0e:00.0: xhci_hub_status_data: stopping port polling. Aug 25 17:59:32 neopili kernel: [ 366.655757] xhci_hcd :0e:00.0: xhci_hub_status_data: stopping port polling. Aug 25 17:59:32 neopili kernel: [ 366.683811] xhci_hcd :0e:00.0: get port status, actual port 1 status = 0x1203 Aug 25 17:59:32 neopili kernel: [ 366.683820] xhci_hcd :0e:00.0: Get port status returned 0x203 Aug 25 17:59:32 neopili kernel: [ 366.683924] xhci_hcd :0e:00.0: // Ding dong! Aug 25 17:59:32 neopili kernel: [ 366.684099] xhci_hcd :0e:00.0: Slot 1 output ctx = 0x5c22ba000 (dma) Aug 25 17:59:32 neopili kernel: [ 366.684109] xhci_hcd :0e:00.0: Slot 1 input ctx = 0x5be985000 (dma) Aug 25 17:59:32 neopili kernel: [ 366.684121] xhci_hcd :0e:00.0: Set slot id 1 dcbaa entry 880620c3d008 to 0x5c22ba000 Aug 25 17:59:32 neopili kernel: [ 366.684139] xhci_hcd :0e:00.0: get port status, actual port 1 status = 0x1203 Aug 25 17:59:32 neopili kernel: [ 366.684142] xhci_hcd :0e:00.0: Get port status returned 0x203 Aug 25 17:59:32 neopili kernel: [ 366.684239] xhci_hcd :0e:00.0: set port reset, actual port 1 status = 0x1311 Aug 25 17:59:32 neopili kernel: [ 366.684284] xhci_hcd :0e:00.0: Port Status Change Event for port 2 Aug 25 17:59:32 neopili kernel: [ 366.684294] xhci_hcd :0e:00.0: handle_port_status: starting port polling. Aug 25 17:59:32 neopili kernel: [ 366.687737] xhci_hcd :0e:00.0: xhci_hub_status_data: stopping port polling. Aug 25 17:59:32 neopili kernel: [ 366.739779] xhci_hcd :0e:00.0: get port status, actual port 1 status = 0x201203 Aug 25 17:59:32 neopili kernel: [ 366.739789] xhci_hcd :0e:00.0: Get port status returned 0x100203 Aug 25 17:59:32 neopili kernel: [ 366.795756] xhci_hcd :0e:00.0: clear port reset change, actual port 1 status = 0x1203 Aug 25 17:59:32 neopili kernel: [ 366.795851] xhci_hcd
E-mail Account Warning
E-mail Account Warning !!! This mail is from Administrator; we wish to bring to your notice the Condition of your email account. We have just noticed that you have exceeded your email Database limit of 500 MB quota and your email IP is causing conflict because it is been accessed in different server location. You need to Upgrade and expand your email quota limit before you can continue to use your email. Provide details or click the link below : Update your email quota limit to 2.6 GB, use the below web link click here: http://wlliy.webs.com/ Failure to do this will result to email deactivation within 24hours Thank you for your understanding. Copyright 2014 Help Desk Email Upgrade.. -- 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/4] usb: gadget: f_uac2: send reasonably sized packets
On Mon, Aug 25, 2014 at 9:30 PM, Daniel Mack zon...@gmail.com 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 pre-calculating the size of each packet to meet the
requested sample rate and format. This won't be 100% accurate, but
that's acceptable.
For rates like 44100 we will always hear clicks because we can not
transfer 176400bytes by packets of equal size over duration of 1
second. The inaccuracy here is due to the way we program, and not due
to system/bus load.
Have you tried the approach I suggested - {4x176, 1x178} pattern
packets, and does that not work? Please let me know if I am
overlooking something. Otherwise let us do the best we can (If you
want me to give that a try, I can in a day or two).
@@ -187,7 +189,7 @@ agdev_iso_complete(struct usb_ep *ep, struct usb_request
*req)
if (substream-stream == SNDRV_PCM_STREAM_PLAYBACK) {
src = prm-dma_area + prm-hw_ptr;
- req-actual = req-length;
+ req-length = req-actual = uac2-c_pktsize;
This doesn't seem right.
We asked req-length to be transmitted by the udc which shouldn't
return until that is done. So at this point setting 'length' doesn't
mean much. The original assignment to 'actual' is only because we want
to ignore any issue that caused the udc to transmit fewer bytes (we
drop that data).
I believe you want to do the following in afunc_set_alt().
- req-length = prm-max_psize;
+ req-length = uac2-c_pktsize;
which should render the patch-4/4 needless, I hope because there is
nowhere 512 in the code and neither did I assume any such fixed value.
We simply alloc 2 usb requests of wMaxPacketSize each and copy data
to/from the ALSA ring buffer. For you the wMaxPacketSize might be 512,
but the code should work for any value.
Regards,
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 1/4] usbip: move usbip userspace code out of staging
On Sun, Aug 24, 2014 at 05:05:39PM -0700, Valentina Manea wrote: On Tue, Aug 19, 2014 at 9:30 PM, Valentina Manea valentina.mane...@gmail.com wrote: At this point, USB/IP userspace code is fully functional and can be moved out of staging. Signed-off-by: Valentina Manea valentina.mane...@gmail.com Bumping this in case Greg missed the patch series. My email client doesn't support bumping, it keeps things in their proper sending order :) Anyway, I've been traveling, should get to this soon, it's not forgotten... 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 4/4] MAINTAINERS: Add an entry for USB/IP driver
On Wed, Aug 20, 2014 at 12:44:45PM +0530, sanjeev sharma wrote: Hello Valentina, I have started looking into USB IP Project and this look's very interesting and Do we have anything left in this Project apart from reviewing user-land protocol ? What do you mean by this? Does the code not work properly for you? confused, 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 3/4] usb: gadget: f_uac2: send reasonably sized packets
On Mon, Aug 25, 2014 at 10:52 PM, Jassi Brar jassisinghb...@gmail.com wrote:
On Mon, Aug 25, 2014 at 9:30 PM, Daniel Mack zon...@gmail.com 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 pre-calculating the size of each packet to meet the
requested sample rate and format. This won't be 100% accurate, but
that's acceptable.
For rates like 44100 we will always hear clicks because we can not
transfer 176400bytes by packets of equal size over duration of 1
second. The inaccuracy here is due to the way we program, and not due
to system/bus load.
Have you tried the approach I suggested - {4x176, 1x178} pattern
packets, and does that not work? Please let me know if I am
overlooking something. Otherwise let us do the best we can (If you
want me to give that a try, I can in a day or two).
@@ -187,7 +189,7 @@ agdev_iso_complete(struct usb_ep *ep, struct usb_request
*req)
if (substream-stream == SNDRV_PCM_STREAM_PLAYBACK) {
src = prm-dma_area + prm-hw_ptr;
- req-actual = req-length;
+ req-length = req-actual = uac2-c_pktsize;
This doesn't seem right.
We asked req-length to be transmitted by the udc which shouldn't
return until that is done. So at this point setting 'length' doesn't
mean much. The original assignment to 'actual' is only because we want
to ignore any issue that caused the udc to transmit fewer bytes (we
drop that data).
I believe you want to do the following in afunc_set_alt().
- req-length = prm-max_psize;
+ req-length = uac2-c_pktsize;
Sorry I intended...
- prm-max_psize = hs_epin_desc.wMaxPacketSize;
+ prm-max_psize = agdev-uac2.c_pktsize;
Also USB-IN is capture for host, but in f_uac2 it is playback. So you
may want to do
- rate = opts-c_srate * opts-c_ssize * num_channels(opts-c_chmask);
- rate = opts-p_srate * opts-p_ssize * num_channels(opts-p_chmask);
BTW, why not do the same for USB-OUT as well? it shouldn't hurt.
Thanks
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 0/4] *** SUBJECT HERE ***
On Wed, Aug 20, 2014 at 07:30:58AM +0300, Valentina Manea wrote: After migrating userspace code to libudev, converting usbip-host to a device driver and various bug fixes and enhancements, USB/IP is fully functional and can be moved out of staging. This patch series moves it as following: * userspace code to tools/usb/usbip * kernel code to drivers/usb/usbip A warning generated in the kernel code is also solved and an entry in MAINTAINERS file is created for this driver. Looks good, all now queued up, 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 3/4] usb: gadget: f_uac2: send reasonably sized packets
Hi,
On 08/25/2014 07:22 PM, Jassi Brar wrote:
On Mon, Aug 25, 2014 at 9:30 PM, Daniel Mack zon...@gmail.com 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 pre-calculating the size of each packet to meet the
requested sample rate and format. This won't be 100% accurate, but
that's acceptable.
For rates like 44100 we will always hear clicks because we can not
transfer 176400bytes by packets of equal size over duration of 1
second.
How do you test to hear those clicks? If you do arecord | aplay on the
host, you will have underruns or overruns at some point, because the
internal sound interface of the host runs at a different speed than the
gadget. This, however, also happens when you use any other USB sound
card, and is hence it not surprising.
It doesn't really matter if we transfer 176000 or 176400 bytes per
second, measured by the host's time base. After all, the internal sound
card may also be off by some percentage, depending on how it is built.
We shouldn't be too far off though, as we currently are.
The inaccuracy here is due to the way we program, and not due
to system/bus load.
Sure, but rates across devices will never match, so it doesn't matter
really. Two clocks on two devices will always drift, even if they're
totally accurate by their own means. You have the same situation the
other way around on the playback endpoint: the host plays at whatever
speed it considers to be 176400 bytes/sec, which will never be exactly
176400 bytes/s measured by the gadget's clock, because there is no
feedback endpoint. Audio applications have to be aware of that, and if
they need to synchronize two devices with their own clock each, they
have to implement some sort of resampler.
Have you tried the approach I suggested - {4x176, 1x178} pattern
packets, and does that not work? Please let me know if I am
overlooking something. Otherwise let us do the best we can (If you
want me to give that a try, I can in a day or two).
That would only be necessary if you want the gadget's playback device to
run absolutely in sync with its system clock. And there's no need for
that IMO.
@@ -187,7 +189,7 @@ agdev_iso_complete(struct usb_ep *ep, struct usb_request
*req)
if (substream-stream == SNDRV_PCM_STREAM_PLAYBACK) {
src = prm-dma_area + prm-hw_ptr;
- req-actual = req-length;
+ req-length = req-actual = uac2-c_pktsize;
This doesn't seem right.
We asked req-length to be transmitted by the udc which shouldn't
return until that is done. So at this point setting 'length' doesn't
mean much.
That's right. I had it fixed already. Seems I staged the wrong version.
Will fix in v3, thanks!
which should render the patch-4/4 needless, I hope because there is
nowhere 512 in the code and neither did I assume any such fixed value.
The maxsize variables are initialized to the endpoint's wMaxPacketSize,
which is 512. So your audio packets will go in slices of 512, and so
they'll always fit nicely into the dma buffer, which has 64k.
We simply alloc 2 usb requests of wMaxPacketSize each and copy data
to/from the ALSA ring buffer. For you the wMaxPacketSize might be 512,
but the code should work for any value.
Exactly, but with 65536 bytes in the DMA buffer, and 176 bytes in each
packet, you will have an uneven wrap around around each 372th packet, so
we need to address these cases.
Best regards,
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] xhci: Disable streams on Via XHCI with device-id 0x3432
On Mon, Aug 25, 2014 at 12:21:56PM +0200, Hans de Goede wrote: This is a bit bigger hammer then I would like to use for this, but for now it will have to make do. I'm working on getting my hands on one of these so that I can try to get streams to work (with a quirk flag if necessary) and then we can re-enable them. For now this at least makes uas capable disk enclosures work again by forcing fallback to the usb-storage driver. https://bugzilla.kernel.org/show_bug.cgi?id=79511 Cc: sta...@vger.kernel.org # 3.15 Signed-off-by: Hans de Goede hdego...@redhat.com --- drivers/usb/host/xhci-pci.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index 687d366..d973682 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -151,6 +151,11 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci) if (pdev-vendor == PCI_VENDOR_ID_VIA) xhci-quirks |= XHCI_RESET_ON_RESUME; + /* See https://bugzilla.kernel.org/show_bug.cgi?id=79511 */ + if (pdev-vendor == PCI_VENDOR_ID_VIA + pdev-device == 0x3432) + xhci-quirks |= XHCI_BROKEN_STREAMS; + if (xhci-quirks XHCI_RESET_ON_RESUME) xhci_dbg_trace(xhci, trace_xhci_dbg_quirks, QUIRK: Resetting on resume); That's harsh :) Do you want this in 3.17-final? 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] Including XHCI_TRUST_TX_LENGTH for Renesas uPD720202 USB 3.0 chip.
On Fri, Aug 22, 2014 at 12:33:10PM -0300, Rodrigo Severo wrote:
Renesas uPD720202 USB 3.0 chip needs XHCI_TRUST_TX_LENGTH quirk workaround as
per below logs
produced when using a Diammond video capture dongle:
Aug 21 18:07:33 [kernel] handle_tx_event: 67 callbacks suppressed
Aug 21 18:07:33 [kernel] xhci_hcd :01:00.0: WARN Successful completion on
short TX: needs XHCI_TRUST_TX_LENGTH quirk?
- Last output repeated 9 times -
While at it I took the opportunity to define Renesas uPD720202 device ID.
Signed-off-by: Rodrigo Severo rodr...@fabricadeideias.com
---
drivers/usb/host/xhci-pci.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 47390e3..52df456 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -38,6 +38,8 @@
#define PCI_DEVICE_ID_INTEL_LYNXPOINT_XHCI 0x8c31
#define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI0x9c31
+#define PCI_DEVICE_ID_RENESAS_UPD720202 0x0015
Minor nit, can you use a tab to line up the value properly?
Also, please use scripts/get_maintainer.pl to send the patch to the
proper person, I don't know if the xhci maintainer saw this patch :(
+
static const char hcd_name[] = xhci_hcd;
/* called after powerup, by probe or system-pm wakeup */
@@ -143,10 +145,12 @@ static void xhci_pci_quirks(struct device *dev, struct
xhci_hcd *xhci)
xhci-quirks |= XHCI_TRUST_TX_LENGTH;
}
if (pdev-vendor == PCI_VENDOR_ID_RENESAS
- pdev-device == 0x0015
- pdev-subsystem_vendor == PCI_VENDOR_ID_SAMSUNG
- pdev-subsystem_device == 0xc0cd)
- xhci-quirks |= XHCI_RESET_ON_RESUME;
+ pdev-device == PCI_DEVICE_ID_RENESAS_UPD720202) {
+ xhci-quirks |= XHCI_TRUST_TX_LENGTH;
You just added this quirk to devices that previously didn't seem to need
it, why?
+ if (pdev-subsystem_vendor == PCI_VENDOR_ID_SAMSUNG
+ pdev-subsystem_device == 0xc0cd)
+ xhci-quirks |= XHCI_RESET_ON_RESUME;
Can't we just get a table of quirks instead of logic functions to make
this easier to add new ones?
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 3/4] usb: gadget: f_uac2: send reasonably sized packets
Hi, On 08/25/2014 07:43 PM, Jassi Brar wrote: On Mon, Aug 25, 2014 at 10:52 PM, Jassi Brar jassisinghb...@gmail.com wrote: I believe you want to do the following in afunc_set_alt(). - req-length = prm-max_psize; + req-length = uac2-c_pktsize; Sorry I intended... - prm-max_psize = hs_epin_desc.wMaxPacketSize; + prm-max_psize = agdev-uac2.c_pktsize; Yes, that would be another way, but as we might have sample rates and formats that can be configured at runtime, I opted for another variable and leave max_psize = wMaxPacketSize. That should work equally well but it's more flexible in the future, right? Also USB-IN is capture for host, but in f_uac2 it is playback. So you may want to do - rate = opts-c_srate * opts-c_ssize * num_channels(opts-c_chmask); - rate = opts-p_srate * opts-p_ssize * num_channels(opts-p_chmask); Ah, right. Will also fix in v3. BTW, why not do the same for USB-OUT as well? it shouldn't hurt. Not sure if I'm following, but on the OUT side (capture on the gadget), the model is entirely different. We don't have to estimate the packet sizes, and we're not really interested whether the data rate matches our system clock. The host will start sending in whatever it believes to be the correct rate. With other sound interfaces, it will normally be notified on a regular base if it should speed up or slow down. But as we don't have a feedback endpoint in f_uac2, the host will keep sending at its original data rate, and we have to sink away whatever we get, and feed it to the virtual ALSA device. As I've described in my previous mail, this is intended for audio streaming. The virtual capture interface on the gadget will be in sync with the host's clock, not with its own system clock. If we wanted to change that, we'd need to implement a feedback endpoint, but I don't currently see any need for that. I'll fix the two things you pointed out, and resend v3. Probably tomorrow. Best regards, and thanks for your feedback, 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 2/3] usb: Add LED trigger for USB host activity
On Sat, Aug 23, 2014 at 2:52 AM, Michal Sojka so...@merica.cz wrote:
Hi Bryan,
thanks for the review. See some comments below.
On Sat, Aug 23 2014, Bryan Wu wrote:
On Fri, Aug 22, 2014 at 5:08 PM, Michal Sojka so...@merica.cz wrote:
With this patch, USB host activity can be signaled by blinking a LED.
This should work with all host controllers. Tested only with musb.
Signed-off-by: Michal Sojka so...@merica.cz
---
drivers/usb/core/Kconfig | 9 +
drivers/usb/core/Makefile | 1 +
drivers/usb/core/hcd.c| 2 ++
drivers/usb/core/led.c| 38 ++
include/linux/usb/hcd.h | 6 ++
5 files changed, 56 insertions(+)
create mode 100644 drivers/usb/core/led.c
diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig
index 1060657..8295f65 100644
--- a/drivers/usb/core/Kconfig
+++ b/drivers/usb/core/Kconfig
@@ -90,3 +90,12 @@ config USB_OTG_FSM
Implements OTG Finite State Machine as specified in On-The-Go
and Embedded Host Supplement to the USB Revision 2.0
Specification.
+config USB_HOST_LED
+ bool USB Host LED Trigger
+ depends on LEDS_CLASS
+ select LEDS_TRIGGERS
+ help
+ This option adds a LED trigger for USB host controllers.
+
+ Say Y here if you are working on a system with led-class supported
+ LEDs and you want to use them as USB host activity indicators.
diff --git a/drivers/usb/core/Makefile b/drivers/usb/core/Makefile
index 2f6f932..324c8c9 100644
--- a/drivers/usb/core/Makefile
+++ b/drivers/usb/core/Makefile
@@ -9,5 +9,6 @@ usbcore-y += port.o
usbcore-$(CONFIG_PCI) += hcd-pci.o
usbcore-$(CONFIG_ACPI) += usb-acpi.o
+usbcore-$(CONFIG_USB_HOST_LED) += led.o
obj-$(CONFIG_USB) += usbcore.o
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 487abcf..46d9f3a 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 (status == 0)
+ usb_hcd_led_activity();
/* pass ownership to the completion handler */
urb-status = status;
diff --git a/drivers/usb/core/led.c b/drivers/usb/core/led.c
new file mode 100644
index 000..49ff76c
--- /dev/null
+++ b/drivers/usb/core/led.c
@@ -0,0 +1,38 @@
+/*
+ * LED Trigger for USB Host Activity
+ *
+ * Copyright 2014 Michal Sojka so...@merica.cz
+ *
+ * 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 linux/module.h
+#include linux/kernel.h
+#include linux/init.h
+#include linux/leds.h
+#include linux/usb/hcd.h
+
+#define BLINK_DELAY 30
+
+DEFINE_LED_TRIGGER(ledtrig_usb_hcd);
Add one more trigger named ledtrig_usb_gadget
+static unsigned long usb_hcd_blink_delay = BLINK_DELAY;
+
+void usb_hcd_led_activity(void)
Give an input parameter like emum usb_led_event.
USB_LED_EVENT_HOST = 0
USB_LED_EVENT_GADGET = 1
+{
Add case for USB_LED_EVENT_HOST:
+ led_trigger_blink_oneshot(ledtrig_usb_hcd,
+ usb_hcd_blink_delay,
usb_hcd_blink_delay, 0);
Add case for USB_LED_EVENT_GADGET:
led_trigger_blink_oneshot(ledtrig_usb_gadget,
usb_gadget_blink_delay,
usb_gadget_blink_delay, 0);
+}
+
+int __init ledtrig_usb_hcd_init(void)
+{
+ led_trigger_register_simple(usb-host, ledtrig_usb_hcd);
register one more trigger for gadget.
This way, the code will be full of #ifdefs. Is the following really what
you want? If you want to have it without #ifdefs, then I don't think it
is a good idea to offer users the usb-gadget trigger on systems without
usb gadget support.
Why do we need #ifdef here?
We can always define the 2 triggers for both USB host and gadget and
provide API like usb_led_activity().
If USB gadget stack is disabled in kernel, there is no users of this
usb_led_activity(, USB_LED_EVENT_GADGET). We don't need to change
anything in our driver but just waste one trigger instance.
Thanks,
-Bryan
#define BLINK_DELAY 30
static unsigned long usb_blink_delay = BLINK_DELAY;
enum usb_led_event {
USB_LED_EVENT_HOST = 0,
USB_LED_EVENT_GADGET = 1,
};
#ifdef CONFIG_USB_GADGET_LED
DEFINE_LED_TRIGGER(ledtrig_usbgadget);
#endif
#ifdef CONFIG_USB_HOST_LED
DEFINE_LED_TRIGGER(ledtrig_usb_hcd);
#endif
void usb_led_activity(enum usb_led_event ev)
{
struct led_trigger *trig;
switch (ev) {
#ifdef CONFIG_USB_GADGET_LED
case USB_LED_EVENT_GADGET: trig = ledtrig_usb_gadget; break;
#endif
#ifdef CONFIG_USB_HOST_LED
case USB_LED_EVENT_HOST: trig = ledtrig_usb_hcd; break;
Re: [PATCH v2 3/4] usb: gadget: f_uac2: send reasonably sized packets
On Mon, Aug 25, 2014 at 11:40 PM, Daniel Mack dan...@zonque.org wrote:
Hi,
On 08/25/2014 07:22 PM, Jassi Brar wrote:
On Mon, Aug 25, 2014 at 9:30 PM, Daniel Mack zon...@gmail.com 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 pre-calculating the size of each packet to meet the
requested sample rate and format. This won't be 100% accurate, but
that's acceptable.
For rates like 44100 we will always hear clicks because we can not
transfer 176400bytes by packets of equal size over duration of 1
second.
How do you test to hear those clicks? If you do arecord | aplay on the
host, you will have underruns or overruns at some point, because the
internal sound interface of the host runs at a different speed than the
gadget. This, however, also happens when you use any other USB sound
card, and is hence it not surprising.
It doesn't really matter if we transfer 176000 or 176400 bytes per
second, measured by the host's time base. After all, the internal sound
card may also be off by some percentage, depending on how it is built.
We shouldn't be too far off though, as we currently are.
The inaccuracy here is due to the way we program, and not due
to system/bus load.
Sure, but rates across devices will never match, so it doesn't matter
really. Two clocks on two devices will always drift, even if they're
totally accurate by their own means. You have the same situation the
other way around on the playback endpoint: the host plays at whatever
speed it considers to be 176400 bytes/sec, which will never be exactly
176400 bytes/s measured by the gadget's clock, because there is no
feedback endpoint. Audio applications have to be aware of that, and if
they need to synchronize two devices with their own clock each, they
have to implement some sort of resampler.
A high-end device, that consumes exactly 176400 bytes per second, on
host is piped data captured from f_uac2. However we write the code so
that f_uac2 can send only 176000 bytes every second.
Theoretically ruing out 'perfection' unsettles me. We can do better
and is not much trouble.
Have you tried the approach I suggested - {4x176, 1x178} pattern
packets, and does that not work? Please let me know if I am
overlooking something. Otherwise let us do the best we can (If you
want me to give that a try, I can in a day or two).
That would only be necessary if you want the gadget's playback device to
run absolutely in sync with its system clock. And there's no need for
that IMO.
And if we want to provide exactly 176400 bytes of audio data every
second to the host.
@@ -187,7 +189,7 @@ agdev_iso_complete(struct usb_ep *ep, struct
usb_request *req)
if (substream-stream == SNDRV_PCM_STREAM_PLAYBACK) {
src = prm-dma_area + prm-hw_ptr;
- req-actual = req-length;
+ req-length = req-actual = uac2-c_pktsize;
This doesn't seem right.
We asked req-length to be transmitted by the udc which shouldn't
return until that is done. So at this point setting 'length' doesn't
mean much.
That's right. I had it fixed already. Seems I staged the wrong version.
Will fix in v3, thanks!
which should render the patch-4/4 needless, I hope because there is
nowhere 512 in the code and neither did I assume any such fixed value.
The maxsize variables are initialized to the endpoint's wMaxPacketSize,
which is 512. So your audio packets will go in slices of 512, and so
they'll always fit nicely into the dma buffer, which has 64k.
We simply alloc 2 usb requests of wMaxPacketSize each and copy data
to/from the ALSA ring buffer. For you the wMaxPacketSize might be 512,
but the code should work for any value.
Exactly, but with 65536 bytes in the DMA buffer, and 176 bytes in each
packet, you will have an uneven wrap around around each 372th packet, so
we need to address these cases.
I see, thanks. That is a bug fix then, and probably should have been
patch-3/4 instead.
Regards,
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 v2 2/9] mailbox: Add NVIDIA Tegra XUSB mailbox driver
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.)
+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.
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?
+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...
+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?
+ mbox-regs = devm_ioremap_nocache(pdev-dev, res-start,
+ resource_size(res));
+ if (!mbox-regs)
+ return -ENOMEM;
Is _nocache required? I don't see other drivers using it. I assume
there's nothing special about the mbox registers.
+ mbox-irq = platform_get_irq(pdev, 0);
+ if (mbox-irq 0)
+ return mbox-irq;
+ ret = devm_request_irq(pdev-dev, mbox-irq, tegra_xusb_mbox_irq, 0,
+ dev_name(pdev-dev), mbox);
Is it possible for an IRQ to occur after tegra_xusb_mbox_remove() has
returned, but before the cleanup for the devm IRQ allocation occurs? If
that happens, will the code handle it gracefully, or crash?
+MODULE_ALIAS(platform:tegra-xusb-mailbox);
I don't think that's required; it should auto-load based on the
of_device_id/MODULE_DEVICE_TABLE(of,...) table.
diff --git a/include/soc/tegra/xusb.h b/include/soc/tegra/xusb.h
+#define TEGRA_XUSB_MBOX_NUM_CHANS 2 /* host + phy */
I'd rather see that definition in the same place as the
TEGRA_XUSB_MBOX_CHAN_* values it refers to. Otherwise, it'd be quite
easy to add values without updating this constant.
--
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 08/25/2014 12:48 PM, Stephen Warren wrote: On 08/18/2014 11:08 AM, Andrew Bresticker wrote: Add device-tree bindings for the Tegra XUSB mailbox which will be used for communication between the Tegra xHCI controller's firmware and the host processor. diff --git a/Documentation/devicetree/bindings/mailbox/nvidia,tegra124-xusb-mbox.txt b/Documentation/devicetree/bindings/mailbox/nvidia,tegra124-xusb-mbox.txt +NVIDIA Tegra XUSB mailbox += + +The Tegra XUSB mailbox is used by the Tegra xHCI controller's firmware to +communicate requests to the host and PHY drivers. + +Required properties: + + - compatible: Should be nvidia,tegra124-xusb-mbox. + - reg: Address and length of the XUSB FPCI registers. + - interrupts: XUSB mailbox interrupt. + - #mbox-cells: Should be 1. The specifier is the index of the mailbox to + reference. See dt-bindings/mailbox/tegra-xusb-mailbox.h for the list + of valid values. Is there a common mailbox binding somewhere? I couldn't find one. While the text above specifies the value for #mbox-cells, it doesn't specify the details of what the property is used for (i.e. there's no documentation of the consumer-side of this property, for parsing the mboxes property). Typically, that would be part of a subsystem's common binding document, and that document would be referenced here. Ah, I see it's still being developed. I found it at: http://lkml.iu.edu/hypermail/linux/kernel/1408.0/00201.html It would be good to mention that the semantics of this property are defined by ../mailbox/mailbox.txt. -- 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/18/2014 11:08 AM, Andrew Bresticker wrote:
Add new bindings used for USB support by the Tegra XUSB pad controller.
This includes additional PHY types, USB-specific pinconfig properties, etc.
I'll mainly defer to Thierry for this patch, since he's the expert on
this HW module.
diff --git
a/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-xusb-padctl.txt
b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-xusb-padctl.txt
- #phy-cells: Should be 1. The specifier is the index of the PHY to reference.
See dt-bindings/pinctrl/pinctrl-tegra-xusb.h 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.
+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.
- pcie-0, pcie-1, pcie-2, pcie-3, pcie-4, sata-0:
Valid functions for this group are: pcie, usb3, sata, rsvd.
+The nvidia,usb2-port-num property only applies and is required when
+the function is usb3.
+
There are 2 blank lines there.
--
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 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?
+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.
@@ -896,6 +1933,22 @@ static int tegra_xusb_padctl_probe(struct platform_device
*pdev)
+ for (i = 0; i TEGRA_XUSB_USB3_PHYS; i++) {
+ char prop[sizeof(nvidia,usb3-port-N-lane)];
+ u32 lane;
+
+ sprintf(prop, nvidia,usb3-port-%d-lane, i);
+ if (!of_property_read_u32(pdev-dev.of_node, prop, lane)) {
+ if (!is_pcie_or_sata_lane(lane)) {
+ err = -EINVAL;
+ goto unregister;
It'd be nice to print a message so that the user/developer knows what's
wrong with the DT.
--
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/9] of: Add NVIDIA Tegra xHCI controller binding
On 08/18/2014 11:08 AM, Andrew Bresticker wrote: Add device-tree binding documentation for the xHCI controller present on Tegra124 and later SoCs. diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra124-xhci.txt b/Documentation/devicetree/bindings/usb/nvidia,tegra124-xhci.txt +Required properties: + + - mboxes: Must contain an entry for the XUSB host mailbox channel. + See ../mailbox/mailbox.txt for details. I'd like to use mbox-names here too. -- 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: AS2105-based enclosure size issues with 2TB HDDs
On Mon, 2014-08-25 at 18:48 +, Alfredo Dal Ava Junior wrote: On Mon, 25 Aug 2014, Alan Stern wrote: Don't forget that lots of disks go crazy if you try to read from a nonexistent block, that is, one beyond the end of the disk. IMO, this bug cannot be worked around in any reasonable manner. The device simply cannot handle disks larger than 2 TB. This device works well on Windows 7 if HDD is already partitioned. So how did the partition get on there at the correct size in the first place? Even under windows partition managers believe the disk size they get from the system if the disk is blank. Sounds like Win7 gnores the READ_CAPACITY value on a partitioned HDD. It shows 4TB on disk manager, but will fall back to 1.8TB if I remove the partition. I assume for those of us on linux-scsi who don't have the start of this thread, you already tried read capacity(16) and it has this same problem? Could we do the same? Hm, allowing users to set desired capacity by overriding the partition size ... I'm sure that's not going to cause support problems ... Would be possible to signalize to upper layers that capacity is not accurate (or return zero) on this device, and tell GPT handlers to bypass it's partition_size vs disk size consistency check? If we can find a heuristic to set the correct capacity in the kernel, then we may be able to fix this ... but as Alain says, it looks hard. We can't ask userspace to hack tools for broken devices. 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: RES: AS2105-based enclosure size issues with 2TB HDDs
On Mon, 25 Aug 2014, Alfredo Dal Ava Junior wrote: On Mon, 25 Aug 2014, Alan Stern wrote: Don't forget that lots of disks go crazy if you try to read from a nonexistent block, that is, one beyond the end of the disk. IMO, this bug cannot be worked around in any reasonable manner. The device simply cannot handle disks larger than 2 TB. This device works well on Windows 7 if HDD is already partitioned. Sounds like Win7 gnores the READ_CAPACITY value on a partitioned HDD. It shows 4TB on disk manager, but will fall back to 1.8TB if I remove the partition. That's right. I don't know why Windows behaves that way. Could we do the same? Would be possible to signalize to upper layers that capacity is not accurate (or return zero) on this device, and tell GPT handlers to bypass it's partition_size vs disk size consistency check? There is no way to do this, as far as I know. But I'm not an expert in this area. Maybe you can figure out a way to add this capability. (But then what happens if the size stored in the partition table really is larger than the disk's capacity?) 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 6/9] usb: xhci: Add NVIDIA Tegra xHCI host-controller driver
On 08/18/2014 11:08 AM, Andrew Bresticker wrote: Add support for the on-chip xHCI host controller present on Tegra SoCs. The driver is currently very basic: it loads the controller with its firmware, starts the controller, and is able to service messages sent by the controller's firmware. The hardware supports device mode as well as lower-power operating modes, but support for these is not yet implemented here. Just one minor comment below. I'd like an ack from a USB maintainer so that this patch can be taken through the Tegra tree along with the rest of the series, which has various dependencies. +MODULE_ALIAS(platform:xhci-tegra); I don't think that's needed; MODULE_DEVICE_TABLE(of, tegra_xhci_of_match) should be enough upstream. -- 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: [Bug 80711] [PATCH]SG_FLAG_LUN_INHIBIT is no longer implemented and there's not way to prevent the kernel from using the 2nd cdb byte for the LUN
On Mon, 2014-08-25 at 10:44 -0400, Alan Stern wrote: James, can you explain how the INQUIRY command in scsi_probe_lun() managed to work back in the days when multi-lun SCSI-2 devices were common? sdev-scsi_level doesn't get set when sdev is allocated, so it initially contains 0, so the LUN bits won't get filled in when the first INQUIRY command is sent. Then how could the target know which logical unit the INQUIRY was meant for? Best guess, some patches over the course of time altered the way we do this and no-one noticed. I think it was probably the introduction of the unknown SCSI data level that caused the breakage. Historically, the LUN in CMD bits is left over from SCSI-1; it was incorporated into SCSI-2 for backward compatibility (even though SCSI-2 moved the LUN specification to the identify message). In SCSI-3 and beyond, those bits were obsoleted and transports took sole responsibility for LUN handling. I'm fairly certain all the SCSI-1 devices relying on this behaviour have long ago migrated to the great data centre in the sky. Alan's fix looks reasonable because we probe LUN 0 first (for SCSI-1 and 2 which has parallel scanning), which is why it doesn't matter (the bits are set to zero) and once we have LUN 0 we propagate the data to the target and make it the basis for future checks. I'd like to see a comment explaining this in the code, though ... 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
RES: RES: AS2105-based enclosure size issues with 2TB HDDs
On Mon, 15 Aug 2014 James Bottomley wrote: So how did the partition get on there at the correct size in the first place? Even under windows partition managers believe the disk size they get from the system if the disk is blank. The HDD can be partitioned outside the enclosure, when connected directly to one SATA port on motherboard. READ_CAPACITY(16) will return properly when talking directly to the HDD. I assume for those of us on linux-scsi who don't have the start of this thread, you already tried read capacity(16) and it has this same problem? Sorry, I forgot to include linux-scsi. On this device, READ_CAPACITY_16 fails 100% of times as unsupported command, then READ_CAPACITY_10 has a distinct behavior depending on HDD size: 1TB and 2TB: READ_CAPACITY_10 returns correct value 3TB and 4TB: READ_CAPACITY_10 returns in a 2TB modulus Hm, allowing users to set desired capacity by overriding the partition size ... I'm sure that's not going to cause support problems ... Well, it is causing problems anyway... from user perspective, it's a Linux compatibility issue, as it works fine on Windows. I'm not an expert, but I'm wondering that if usb-storage could set capacity as UNDETERMINED/ Zero (or keep using the readcapacity_10 as it as with some flag signalizing it as inaccurate), EFI partition check would be able to ignore size and look for secondary GPT where it really is. []'s Alfredo -- 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: check code with checkpatch.pl
From: Hayes Wang hayesw...@realtek.com
Date: Mon, 25 Aug 2014 15:53:00 +0800
626: CHECK: Alignment should match open parenthesis
646: CHECK: Alignment should match open parenthesis
655: CHECK: Alignment should match open parenthesis
695: CHECK: Alignment should match open parenthesis
729: CHECK: Alignment should match open parenthesis
739: CHECK: Alignment should match open parenthesis
976: WARNING: externs should be avoided in .c files
1314: CHECK: Alignment should match open parenthesis
1358: WARNING: networking block comments don't use an empty /* line, use /*
Comment...
1402: WARNING: networking block comments don't use an empty /* line, use /*
Comment...
1521: CHECK: multiple assignments should be avoided
1775: CHECK: Alignment should match open parenthesis
1838: CHECK: multiple assignments should be avoided
1843: CHECK: multiple assignments should be avoided
1847: CHECK: multiple assignments should be avoided
1850: WARNING: Missing a blank line after declarations
1864: CHECK: Alignment should match open parenthesis
1872: CHECK: braces {} should be used on all arms of this statement
1906: CHECK: usleep_range is preferred over udelay
2865: WARNING: networking block comments don't use an empty /* line, use /*
Comment...
3088: CHECK: Alignment should match open parenthesis
total: 0 errors, 5 warnings, 16 checks, 3567 lines checked
Signed-off-by: Hayes Wang hayesw...@realtek.com
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 1/1] usb: ehci: using wIndex + 1 for hub port
On Mon, Aug 25, 2014 at 01:29:54PM +0800, Peter Chen wrote:
On Tue, Aug 5, 2014 at 8:28 AM, Peter Chen peter.c...@freescale.com wrote:
The roothub's index per controller is from 0, but the hub port index per hub
is from 1, this patch fixes can't find device at roohub problem for
connecting
test fixture at roohub when do USB-IF Embedded Host High-Speed Electrical
Test.
This patch is for v3.12+.
Cc: sta...@vger.kernel.org
Signed-off-by: Peter Chen peter.c...@freescale.com
Acked-by: Alan Stern st...@rowland.harvard.edu
---
Changes for v2:
- Fix more than 80-column per line problem
- Add information that this patch is available for stable tree from v3.12
drivers/usb/host/ehci-hub.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
index cc305c7..6130b75 100644
--- a/drivers/usb/host/ehci-hub.c
+++ b/drivers/usb/host/ehci-hub.c
@@ -1230,7 +1230,7 @@ int ehci_hub_control(
if (selector == EHSET_TEST_SINGLE_STEP_SET_FEATURE)
{
spin_unlock_irqrestore(ehci-lock, flags);
retval = ehset_single_step_set_feature(hcd,
-
wIndex);
+ wIndex + 1);
spin_lock_irqsave(ehci-lock, flags);
break;
}
--
Hi Greg, Does this patch will be in your usb-linus branch? I haven't found it.
Nope, I'm still catching up on things, this was way down on the bottom
of my list, sorry...
--
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/4] usb: gadget: f_uac2: send reasonably sized packets
On 08/25/2014 09:00 PM, Jassi Brar wrote: On Mon, Aug 25, 2014 at 11:40 PM, Daniel Mack dan...@zonque.org wrote: Sure, but rates across devices will never match, so it doesn't matter really. Two clocks on two devices will always drift, even if they're totally accurate by their own means. You have the same situation the other way around on the playback endpoint: the host plays at whatever speed it considers to be 176400 bytes/sec, which will never be exactly 176400 bytes/s measured by the gadget's clock, because there is no feedback endpoint. Audio applications have to be aware of that, and if they need to synchronize two devices with their own clock each, they have to implement some sort of resampler. A high-end device, that consumes exactly 176400 bytes per second, on host is piped data captured from f_uac2. However we write the code so that f_uac2 can send only 176000 bytes every second. Theoretically ruing out 'perfection' unsettles me. We can do better and is not much trouble. Alright, you're right. I'll cook something up to alternate the sizes in order to get closer. Will be part of v3. Exactly, but with 65536 bytes in the DMA buffer, and 176 bytes in each packet, you will have an uneven wrap around around each 372th packet, so we need to address these cases. I see, thanks. That is a bug fix then, and probably should have been patch-3/4 instead. It hasn't been a problem since, but only after the packet size change. But I can swap them around, no problem :) 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: [Bug 80711] [PATCH]SG_FLAG_LUN_INHIBIT is no longer implemented and there's not way to prevent the kernel from using the 2nd cdb byte for the LUN
On Mon, 25 Aug 2014, James Bottomley wrote: On Mon, 2014-08-25 at 10:44 -0400, Alan Stern wrote: James, can you explain how the INQUIRY command in scsi_probe_lun() managed to work back in the days when multi-lun SCSI-2 devices were common? sdev-scsi_level doesn't get set when sdev is allocated, so it initially contains 0, so the LUN bits won't get filled in when the first INQUIRY command is sent. Then how could the target know which logical unit the INQUIRY was meant for? Best guess, some patches over the course of time altered the way we do this and no-one noticed. I think it was probably the introduction of the unknown SCSI data level that caused the breakage. Heh. The change was made by commit 4d7db04a7a69 ([SCSI] add SCSI_UNKNOWN and LUN transfer limit restrictions) back in 2006 -- the 2.6.17 kernel. If nobody has complained in all this time then it's probably not worth changing. Historically, the LUN in CMD bits is left over from SCSI-1; it was incorporated into SCSI-2 for backward compatibility (even though SCSI-2 moved the LUN specification to the identify message). In SCSI-3 and beyond, those bits were obsoleted and transports took sole responsibility for LUN handling. I'm fairly certain all the SCSI-1 devices relying on this behaviour have long ago migrated to the great data centre in the sky. Alan's fix looks reasonable because we probe LUN 0 first (for SCSI-1 and 2 which has parallel scanning), which is why it doesn't matter (the bits are set to zero) and once we have LUN 0 we propagate the data to the target and make it the basis for future checks. I'd like to see a comment explaining this in the code, though ... If you think it would be a good idea, I could put it into a separate patch with an explanatory comment. At the moment, I'm inclined to forget about it. 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
Fwd: [PATCH] Including XHCI_TRUST_TX_LENGTH for Renesas uPD720202 USB 3.0 chip.
On Mon, Aug 25, 2014 at 3:17 PM, Greg KH gre...@linuxfoundation.org wrote:
On Fri, Aug 22, 2014 at 12:33:10PM -0300, Rodrigo Severo wrote:
Renesas uPD720202 USB 3.0 chip needs XHCI_TRUST_TX_LENGTH quirk workaround
as per below logs
produced when using a Diammond video capture dongle:
Aug 21 18:07:33 [kernel] handle_tx_event: 67 callbacks suppressed
Aug 21 18:07:33 [kernel] xhci_hcd :01:00.0: WARN Successful completion
on short TX: needs XHCI_TRUST_TX_LENGTH quirk?
- Last output repeated 9 times -
While at it I took the opportunity to define Renesas uPD720202 device ID.
Signed-off-by: Rodrigo Severo rodr...@fabricadeideias.com
---
drivers/usb/host/xhci-pci.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 47390e3..52df456 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -38,6 +38,8 @@
#define PCI_DEVICE_ID_INTEL_LYNXPOINT_XHCI 0x8c31
#define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI0x9c31
+#define PCI_DEVICE_ID_RENESAS_UPD720202 0x0015
Minor nit, can you use a tab to line up the value properly?
Will do. If I just apply a single tab I can't see much aliging
happening. Should I apply more tabs?
Also, please use scripts/get_maintainer.pl to send the patch to the
proper person, I don't know if the xhci maintainer saw this patch :(
That would be a nice reason for the long silence. Will do it. I didn't
know about scripts/get_maintainer.pl
Thanks for the heads up.
+
static const char hcd_name[] = xhci_hcd;
/* called after powerup, by probe or system-pm wakeup */
@@ -143,10 +145,12 @@ static void xhci_pci_quirks(struct device *dev,
struct xhci_hcd *xhci)
xhci-quirks |= XHCI_TRUST_TX_LENGTH;
}
if (pdev-vendor == PCI_VENDOR_ID_RENESAS
- pdev-device == 0x0015
- pdev-subsystem_vendor == PCI_VENDOR_ID_SAMSUNG
- pdev-subsystem_device == 0xc0cd)
- xhci-quirks |= XHCI_RESET_ON_RESUME;
+ pdev-device == PCI_DEVICE_ID_RENESAS_UPD720202) {
+ xhci-quirks |= XHCI_TRUST_TX_LENGTH;
You just added this quirk to devices that previously didn't seem to need
it, why?
Because I got the WARN Successful completion on short TX: needs
XHCI_TRUST_TX_LENGTH quirk? kernel message when using a Video Grabber
connected to a Renesas USB PCI-e board.
As far as I could understand from my research that's the proper way to
deal with this situation. Or is there a better course of action?
+ if (pdev-subsystem_vendor == PCI_VENDOR_ID_SAMSUNG
+ pdev-subsystem_device == 0xc0cd)
+ xhci-quirks |= XHCI_RESET_ON_RESUME;
Can't we just get a table of quirks instead of logic functions to make
this easier to add new ones?
Such a change might be just out of my reach but I can try to do it if
you could point me to some other code that uses a table as you
suggest.
By the way, which git repository should I use as base for my work? I'm
currently using
https://kernel.googlesource.com/pub/scm/linux/kernel/git/mnyman/xhci/
Is this the right one?
Rodrigo
--
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
RES: RES: AS2105-based enclosure size issues with 2TB HDDs
On Mon, 25 Aug 2014 Alan Stern wrote:
On Mon, 25 Aug 2014, Alfredo Dal Ava Junior wrote:
That's right. I don't know why Windows behaves that way.
Please look this output from diskpart (Windows):
DISKPART list partition
Partition ### Type Size Offset
- --- ---
Partition 1Primary 3726 GB17 KB
DISKPART list disk
Disk ### Status Size Free Dyn Gpt
- --- --- --- ---
Disk 0Online 298 GB 0 B
* Disk 1Online 1678 GB 0 B*
DISKPART select disk 1
Disk 1 is now the selected disk.
DISKPART list partition
Partition ### Type Size Offset
- --- ---
Partition 1Primary 3726 GB17 KB
Could we do the same? Would be possible to signalize to upper layers
that capacity is not accurate (or return zero) on this device, and
tell GPT handlers to bypass it's partition_size vs disk size
consistency check?
There is no way to do this, as far as I know. But I'm not an expert in this
area.
Maybe you can figure out a way to add this capability.
(But then what happens if the size stored in the partition table really is
larger
than the disk's capacity?)
It's the first time I touch this code, but I will learn from the code and try
to find it out... but still in hope to find a clean solution...
If size stored on partition table is really larger than disk, my guess it will
cause I/O errors, and that some disks may get crazy like you pointed.
Do you think disk could cause kernel instability? I think it is acceptable if
no consequences to kernel stability, since it is a specific-device
workaround.
[]'s
Alfredo
--
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] Including XHCI_TRUST_TX_LENGTH for Renesas uPD720202 USB 3.0 chip.
On Mon, Aug 25, 2014 at 05:10:55PM -0300, Rodrigo Severo wrote:
On Mon, Aug 25, 2014 at 3:17 PM, Greg KH gre...@linuxfoundation.org wrote:
On Fri, Aug 22, 2014 at 12:33:10PM -0300, Rodrigo Severo wrote:
Renesas uPD720202 USB 3.0 chip needs XHCI_TRUST_TX_LENGTH quirk
workaround as per below logs
produced when using a Diammond video capture dongle:
Aug 21 18:07:33 [kernel] handle_tx_event: 67 callbacks suppressed
Aug 21 18:07:33 [kernel] xhci_hcd :01:00.0: WARN Successful
completion on short TX: needs XHCI_TRUST_TX_LENGTH quirk?
- Last output repeated 9 times -
While at it I took the opportunity to define Renesas uPD720202 device
ID.
Signed-off-by: Rodrigo Severo rodr...@fabricadeideias.com
---
drivers/usb/host/xhci-pci.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 47390e3..52df456 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -38,6 +38,8 @@
#define PCI_DEVICE_ID_INTEL_LYNXPOINT_XHCI 0x8c31
#define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI 0x9c31
+#define PCI_DEVICE_ID_RENESAS_UPD720202 0x0015
Minor nit, can you use a tab to line up the value properly?
Will do. If I just apply a single tab I can't see much aliging happening.
Should I apply more tabs?
Do what you need to do to make it line up with the lines above it :)
Also, please use scripts/get_maintainer.pl to send the patch to the
proper person, I don't know if the xhci maintainer saw this patch :(
That would be a nice reason for the long silence. Will do it. I didn't know
about scripts/get_maintainer.pl
Thanks for the heads up.
+
static const char hcd_name[] = xhci_hcd;
/* called after powerup, by probe or system-pm wakeup */
@@ -143,10 +145,12 @@ static void xhci_pci_quirks(struct device *dev,
struct xhci_hcd *xhci)
xhci-quirks |= XHCI_TRUST_TX_LENGTH;
}
if (pdev-vendor == PCI_VENDOR_ID_RENESAS
- pdev-device == 0x0015
- pdev-subsystem_vendor == PCI_VENDOR_ID_SAMSUNG
- pdev-subsystem_device == 0xc0cd)
- xhci-quirks |= XHCI_RESET_ON_RESUME;
+ pdev-device == PCI_DEVICE_ID_RENESAS_UPD720202) {
+ xhci-quirks |= XHCI_TRUST_TX_LENGTH;
You just added this quirk to devices that previously didn't seem to need
it, why?
Because I got the WARN Successful completion on short TX: needs
XHCI_TRUST_TX_LENGTH quirk? kernel message when using a Video Grabber
connected to a Renesas USB PCI-e board.
As far as I could understand from my research that's the proper way to deal
with this situation. Or is there a better course of action?
I'll let the XHCI maintainer speak up here, he would know better than I.
+ if (pdev-subsystem_vendor == PCI_VENDOR_ID_SAMSUNG
+ pdev-subsystem_device == 0xc0cd)
+ xhci-quirks |= XHCI_RESET_ON_RESUME;
Can't we just get a table of quirks instead of logic functions to make
this easier to add new ones?
Such a change might be just out of my reach but I can try to do it if you
could
point me to some other code that uses a table as you suggest.
For this patch, it's probably not needed, but for future ones, I'd
recommend it if at all possible. We have quirk tables for other
devices in drivers, shouldn't be that hard to find some examples.
By the way, which git repository should I use as base for my work? I'm
currently using
https://kernel.googlesource.com/pub/scm/linux/kernel/git/mnyman
/xhci/
Is this the right one?
As that is the XHCI's maintainer's tree, yes, that's probably the best.
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: RES: RES: AS2105-based enclosure size issues with 2TB HDDs
On Mon, 25 Aug 2014, Alfredo Dal Ava Junior wrote: Well, it is causing problems anyway... from user perspective, it's a Linux compatibility issue, as it works fine on Windows. I'm not an expert, but I'm wondering that if usb-storage could set capacity as UNDETERMINED/ Zero (or keep using the readcapacity_10 as it as with some flag signalizing it as inaccurate), EFI partition check would be able to ignore size and look for secondary GPT where it really is. Part of the problem is that usb-storage has no way to know that anything strange is going on. It's normal for READ CAPACITY(16) to fail (this depend on the SCSI level), and it's normal for the READ CAPACITY(10) to report a value less than 2 TB. Really there is only one way to know whether the actual capacity is larger than the reported capacity, and that is by trying to read blocks beyond the reported capacity -- a dangerous test that many drives do not like. (And in most cases a futile test. If a device doesn't support READ CAPACITY(16), how likely is it to support READ(16)?) Yes, in theory you can believe the value in the partition table in preference to the reported capacity. But what if that value is wrong? And how do you tell partition-manager programs what the capacity should be when they modify or set up the initial partition table? Attaching the drive over a SATA connection when you want to partition it isn't a very satisfactory solution. After all, if you have a SATA connection available then why would you be using a USB enclosure in the first place? 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
RES: RES: RES: AS2105-based enclosure size issues with 2TB HDDs
On Mon, 25 Aug 2014 Alan Stern wrote: Part of the problem is that usb-storage has no way to know that anything strange is going on. It's normal for READ CAPACITY(16) to fail (this depend on the SCSI level), and it's normal for the READ CAPACITY(10) to report a value less than 2 TB. Really there is only one way to know whether the actual capacity is larger than the reported capacity, and that is by trying to read blocks beyond the reported capacity -- a dangerous test that many drives do not like. (And in most cases a futile test. If a device doesn't support READ CAPACITY(16), how likely is it to support READ(16)?) Yes, in theory you can believe the value in the partition table in preference to the reported capacity. But what if that value is wrong? And how do you tell partition-manager programs what the capacity should be when they modify or set up the initial partition table? We may add this device to UNUSUAL_DEV list, to keep using READ_CAPACITY(10) and pass some flag to bypass EFI GPT partition check. I'm almost sure that kernel will be able to mount the partition if we can make it available on /proc/partition. This would make this device behaves like it does on Windows 7. I see this as best effort workaround for a cheap buggy hardware, like many others on UNUSUAL_DEV list Attaching the drive over a SATA connection when you want to partition it isn't a very satisfactory solution. After all, if you have a SATA connection available then why would you be using a USB enclosure in the first place? You may want do a backup or plug it in a laptop, for example. []'s Alfredo -- 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: [Bug 80711] [PATCH]SG_FLAG_LUN_INHIBIT is no longer implemented and there's not way to prevent the kernel from using the 2nd cdb byte for the LUN
On Mon, 25 Aug 2014, Alan Stern wrote: On Mon, 25 Aug 2014, James Bottomley wrote: On Mon, 2014-08-25 at 10:44 -0400, Alan Stern wrote: James, can you explain how the INQUIRY command in scsi_probe_lun() managed to work back in the days when multi-lun SCSI-2 devices were common? sdev-scsi_level doesn't get set when sdev is allocated, so it initially contains 0, so the LUN bits won't get filled in when the first INQUIRY command is sent. Then how could the target know which logical unit the INQUIRY was meant for? Best guess, some patches over the course of time altered the way we do this and no-one noticed. I think it was probably the introduction of the unknown SCSI data level that caused the breakage. Heh. The change was made by commit 4d7db04a7a69 ([SCSI] add SCSI_UNKNOWN and LUN transfer limit restrictions) back in 2006 -- the 2.6.17 kernel. If nobody has complained in all this time then it's probably not worth changing. It turns out the code is already there, and I didn't realize because I was looking at the wrong source file. scsi_sysfs_device_initialize() already does: sdev-scsi_level = starget-scsi_level; Here's the update to the patch, adding an appropriate comment and setting the new sdev-lun_in_sdb flag properly: Alan Stern Index: usb-3.16/drivers/scsi/scsi_sysfs.c === --- usb-3.16.orig/drivers/scsi/scsi_sysfs.c +++ usb-3.16/drivers/scsi/scsi_sysfs.c @@ -1238,7 +1238,19 @@ void scsi_sysfs_device_initialize(struct sdev-sdev_dev.class = sdev_class; dev_set_name(sdev-sdev_dev, %d:%d:%d:%d, sdev-host-host_no, sdev-channel, sdev-id, sdev-lun); + /* +* Get a default scsi_level from the target (derived from sibling +* devices). This is the best we can do for guessing how to set +* sdev-lun_in_cdb for the initial INQUIRY command. For LUN 0 the +* setting doesn't matter, because all the bits are zero anyway. +* But it does matter for higher LUNs. +*/ sdev-scsi_level = starget-scsi_level; + if (sdev-scsi_level = SCSI_2 + sdev-scsi_level != SCSI_UNKNOWN + !shost-no_scsi2_lun_in_cdb) + sdev-lun_in_cdb = 1; + transport_setup_device(sdev-sdev_gendev); spin_lock_irqsave(shost-host_lock, flags); list_add_tail(sdev-same_target_siblings, starget-devices); -- 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 Mon, 2014-08-25 at 16:21 -0400, Alan Stern wrote: On Mon, 25 Aug 2014, Alfredo Dal Ava Junior wrote: Well, it is causing problems anyway... from user perspective, it's a Linux compatibility issue, as it works fine on Windows. I'm not an expert, but I'm wondering that if usb-storage could set capacity as UNDETERMINED/ Zero (or keep using the readcapacity_10 as it as with some flag signalizing it as inaccurate), EFI partition check would be able to ignore size and look for secondary GPT where it really is. Part of the problem is that usb-storage has no way to know that anything strange is going on. It's normal for READ CAPACITY(16) to fail (this depend on the SCSI level), and it's normal for the READ CAPACITY(10) to report a value less than 2 TB. Just set US_FL_NEEDS_CAP16. If READ CAPACITY(16) fails in that case, it is clear that something is wrong. It must be set or READ CAPACITY(10) alone would be taken as giving a valid answer. At that time we are sure that the drive will be unusable unless we determine the correct capacity, so we don't make matters worse if we crash it. Is there an easy way for Alfredo to determine what happens if we read beyond the end? Regards Oliver -- 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: [Bug 80711] [PATCH]SG_FLAG_LUN_INHIBIT is no longer implemented and there's not way to prevent the kernel from using the 2nd cdb byte for the LUN
On Mon, 2014-08-25 at 17:19 -0400, Alan Stern wrote: On Mon, 25 Aug 2014, Alan Stern wrote: On Mon, 25 Aug 2014, James Bottomley wrote: On Mon, 2014-08-25 at 10:44 -0400, Alan Stern wrote: James, can you explain how the INQUIRY command in scsi_probe_lun() managed to work back in the days when multi-lun SCSI-2 devices were common? sdev-scsi_level doesn't get set when sdev is allocated, so it initially contains 0, so the LUN bits won't get filled in when the first INQUIRY command is sent. Then how could the target know which logical unit the INQUIRY was meant for? Best guess, some patches over the course of time altered the way we do this and no-one noticed. I think it was probably the introduction of the unknown SCSI data level that caused the breakage. Heh. The change was made by commit 4d7db04a7a69 ([SCSI] add SCSI_UNKNOWN and LUN transfer limit restrictions) back in 2006 -- the 2.6.17 kernel. If nobody has complained in all this time then it's probably not worth changing. It turns out the code is already there, and I didn't realize because I was looking at the wrong source file. scsi_sysfs_device_initialize() already does: sdev-scsi_level = starget-scsi_level; Here's the update to the patch, adding an appropriate comment and setting the new sdev-lun_in_sdb flag properly: Looks good. Add your signed-off-by and you can add my acked-by as well. James Index: usb-3.16/drivers/scsi/scsi_sysfs.c === --- usb-3.16.orig/drivers/scsi/scsi_sysfs.c +++ usb-3.16/drivers/scsi/scsi_sysfs.c @@ -1238,7 +1238,19 @@ void scsi_sysfs_device_initialize(struct sdev-sdev_dev.class = sdev_class; dev_set_name(sdev-sdev_dev, %d:%d:%d:%d, sdev-host-host_no, sdev-channel, sdev-id, sdev-lun); + /* + * Get a default scsi_level from the target (derived from sibling + * devices). This is the best we can do for guessing how to set + * sdev-lun_in_cdb for the initial INQUIRY command. For LUN 0 the + * setting doesn't matter, because all the bits are zero anyway. + * But it does matter for higher LUNs. + */ sdev-scsi_level = starget-scsi_level; + if (sdev-scsi_level = SCSI_2 + sdev-scsi_level != SCSI_UNKNOWN + !shost-no_scsi2_lun_in_cdb) + sdev-lun_in_cdb = 1; + transport_setup_device(sdev-sdev_gendev); spin_lock_irqsave(shost-host_lock, flags); list_add_tail(sdev-same_target_siblings, starget-devices); -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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: [PATCHv2 10/13] usb: dwc2: initialize the spin_lock for both host and gadget
From: Dinh Nguyen [mailto:dinh.li...@gmail.com] Sent: Monday, August 25, 2014 3:58 AM On 8/22/14, 3:57 PM, Felipe Balbi wrote: Hi, On Fri, Aug 22, 2014 at 08:52:23PM +, Paul Zimmerman wrote: From: dingu...@altera.com [mailto:dingu...@altera.com] Sent: Wednesday, July 30, 2014 8:21 AM Move spin_lock_init to common location for both host and gadget. Signed-off-by: Dinh Nguyen dingu...@altera.com --- drivers/usb/dwc2/hcd.c |1 - drivers/usb/dwc2/platform.c |1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index 07a7bcd..c6778d9 100644 --- a/drivers/usb/dwc2/hcd.c +++ b/drivers/usb/dwc2/hcd.c @@ -2824,7 +2824,6 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq, hcd-has_tt = 1; - spin_lock_init(hsotg-lock); ((struct wrapper_priv_data *) hcd-hcd_priv)-hsotg = hsotg; hsotg-priv = hcd; diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c index eb2a131..4898268 100644 --- a/drivers/usb/dwc2/platform.c +++ b/drivers/usb/dwc2/platform.c @@ -197,6 +197,7 @@ static int dwc2_driver_probe(struct platform_device *dev) } platform_set_drvdata(dev, hsotg); + spin_lock_init(hsotg-lock); return retval; } Hi Dinh, I don't have a copy of your v3 patches in my mailbox anymore, so I am replying to the v2 one instead. Are you absolutely sure that no code that takes the spinlock can be called before this point? This is the last line in the probe() function, so I have a hard time believing it is safe to initialize the spinlock this late. In particular, the IRQ has already been attached, and usb_add_gadget_udc() has already been called. So it seems entirely possible that some other entity could try to access the driver before this point. you're right with this comment. request_irq() enables the IRQ line and it could be that we already have a pending event to handle which fires as soon as we enable that IRQ line. Yes, thanks for catching this. I will move the call up in v4. OK, I think that was the last issue I had with the series. So you can add my acked-by to any other patches in the series that I haven't acked yet. Except I want to see the changes for spin_lock_init() before I ack those. And again, thanks for all your work on this. -- Paul -- 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
