Re: [PATCH] usb: musb: Kconfig: Depend on some machines under blackfin
On 4/5/15 16:29, Greg Kroah-Hartman wrote: On Sun, Apr 05, 2015 at 06:33:44AM +0800, Chen Gang wrote: On 4/4/15 17:54, Greg Kroah-Hartman wrote: On Sat, Apr 04, 2015 at 05:51:21AM +0800, Chen Gang wrote: Under blackfin, only bf527, bf548 and bf609 may use musb. The related error with allmodconfig: CC [M] drivers/usb/misc/trancevibrator.o In file included from drivers/usb/musb/musb_core.h:70:0, from drivers/usb/musb/musb_core.c:103: drivers/usb/musb/musb_core.c: In function 'musb_indexed_ep_select': drivers/usb/musb/musb_regs.h:458:32: error: 'USB_INDEX' undeclared (first use in this function) #define MUSB_INDEX USB_OFFSET(USB_INDEX) /* 8 bit */ ^ Signed-off-by: Chen Gang gang.chen.5...@gmail.com Why not fix the root cause and provide this symbol on blackfin? There's no specific reason why it shouldn't be able to be built on this platform. It's better to make platforms properly build everything, patches like this just make things messier and harder to maintain. I am not quite sure all machines of blackfin can support musb (according to current code, at present, we can only sure bf527, bf548 and bf609 can support it). What do you mean by can support? What is missing? Why doesn't the code build there? Only some of machines of blackfin define USB_INDEX, but machine bf533 ( which is in current building configuration) does not define USB_INDEX, so cause building break. The related information: [root@localhost arch]# grep -rn \USB_INDEX\ * blackfin/kernel/debug-mmrs.c:1587:D16(USB_INDEX); blackfin/mach-bf527/include/mach/cdefBF525.h:33:#define bfin_read_USB_INDEX() bfin_read16(USB_INDEX) blackfin/mach-bf527/include/mach/cdefBF525.h:34:#define bfin_write_USB_INDEX(val) bfin_write16(USB_INDEX, val) blackfin/mach-bf527/include/mach/defBF525.h:24:#define USB_INDEX 0xffc03824 /* Index register for selecting the indexed endpoint registers */ blackfin/mach-bf527/include/mach/defBF525.h:394:/* Bit masks for USB_INDEX */ blackfin/mach-bf548/include/mach/cdefBF542.h:153:#define bfin_read_USB_INDEX()bfin_read16(USB_INDEX) blackfin/mach-bf548/include/mach/cdefBF542.h:154:#define bfin_write_USB_INDEX(val)bfin_write16(USB_INDEX, val) blackfin/mach-bf548/include/mach/cdefBF547.h:320:#define bfin_read_USB_INDEX()bfin_read16(USB_INDEX) blackfin/mach-bf548/include/mach/cdefBF547.h:321:#define bfin_write_USB_INDEX(val)bfin_write16(USB_INDEX, val) blackfin/mach-bf548/include/mach/defBF542.h:88:#define USB_INDEX 0xffc03c24 /* Index register for selecting the indexed endpoint registers */ blackfin/mach-bf548/include/mach/defBF542.h:571:/* Bit masks for USB_INDEX */ blackfin/mach-bf548/include/mach/defBF547.h:202:#define USB_INDEX 0xffc03c24 /* Index register for selecting the indexed endpoint registers */ blackfin/mach-bf548/include/mach/defBF547.h:848:/* Bit masks for USB_INDEX */ blackfin/mach-bf609/include/mach/defBF60x_base.h:3262:#define USB_INDEX 0xFFCC100E /* USB Index */ USB_INDEX is one of core macro for musb, so I guess, bf533 and other blackfin machines which do not define USB_INDEX can not support musb. So, I suggest to add new macro HAVE_MUSB for BLACKFIN in patch v2. And welcome any other members' idea. I recommend fixing blackfin, what makes this one specific platform so broken compared to the 20+ other platforms that don't need this special treatment? Originally, I try to remove CONFIG_BLACKFIN in source code, but it seems not quite easy, and the original related git commit: commit c6cf8b003e5a37f8193c2883876c5942adcd7284 Author: Bryan Wu coolo...@kernel.org Date: Tue Dec 2 21:33:48 2008 +0200 USB: musb: add Blackfin specific configuration to MUSB Some config registers are not avaiable in Blackfin, we have to comment them out. v1-v2: - remove Blackfin specific header file - add Blackfin register version to musb_regs.h header file Signed-off-by: Bryan Wu coolo...@kernel.org Signed-off-by: Felipe Balbi felipe.ba...@nokia.com Signed-off-by: Greg Kroah-Hartman gre...@suse.de So for me, only CONFIG_BLACKFIN can not be sure it must support musb, we have to define a new config macro HAVE_MUSB (or SUPPORT_MUSB) for it (at least, within blackfin). Welcome other members ideas (especially blackfin related members). Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: musb: Kconfig: Depend on some machines under blackfin
On 4/4/15 17:54, Greg Kroah-Hartman wrote: On Sat, Apr 04, 2015 at 05:51:21AM +0800, Chen Gang wrote: Under blackfin, only bf527, bf548 and bf609 may use musb. The related error with allmodconfig: CC [M] drivers/usb/misc/trancevibrator.o In file included from drivers/usb/musb/musb_core.h:70:0, from drivers/usb/musb/musb_core.c:103: drivers/usb/musb/musb_core.c: In function 'musb_indexed_ep_select': drivers/usb/musb/musb_regs.h:458:32: error: 'USB_INDEX' undeclared (first use in this function) #define MUSB_INDEX USB_OFFSET(USB_INDEX) /* 8 bit */ ^ Signed-off-by: Chen Gang gang.chen.5...@gmail.com Why not fix the root cause and provide this symbol on blackfin? There's no specific reason why it shouldn't be able to be built on this platform. It's better to make platforms properly build everything, patches like this just make things messier and harder to maintain. I am not quite sure all machines of blackfin can support musb (according to current code, at present, we can only sure bf527, bf548 and bf609 can support it). So, I suggest to add new macro HAVE_MUSB for BLACKFIN in patch v2. And welcome any other members' idea. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: musb: Kconfig: Depend on some machines under blackfin
On 4/4/15 06:03, Richard Weinberger wrote: On Fri, Apr 3, 2015 at 11:51 PM, Chen Gang xili_gchen_5...@hotmail.com wrote: Under blackfin, only bf527, bf548 and bf609 may use musb. The related error with allmodconfig: CC [M] drivers/usb/misc/trancevibrator.o In file included from drivers/usb/musb/musb_core.h:70:0, from drivers/usb/musb/musb_core.c:103: drivers/usb/musb/musb_core.c: In function 'musb_indexed_ep_select': drivers/usb/musb/musb_regs.h:458:32: error: 'USB_INDEX' undeclared (first use in this function) #define MUSB_INDEX USB_OFFSET(USB_INDEX) /* 8 bit */ ^ Signed-off-by: Chen Gang gang.chen.5...@gmail.com --- drivers/usb/musb/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig index 39db8b6..5fca898 100644 --- a/drivers/usb/musb/Kconfig +++ b/drivers/usb/musb/Kconfig @@ -6,7 +6,7 @@ # (M)HDRC = (Multipoint) Highspeed Dual-Role Controller config USB_MUSB_HDRC tristate 'Inventra Highspeed Dual Role Controller (TI, ADI, ...)' - depends on (USB || USB_GADGET) + depends on (USB || USB_GADGET) (!BLACKFIN || (BLACKFIN (BF527 || BF548 || BF609))) This is ugly. Can't you add a new Kconfig symbol in arch/blackfin and change the #ifndef CONFIG_BLACKFIN in drivers/usb/musb/musb_regs.h to it? For me, for configuration, we need to try to finish them in Kconfig and keep the .c or .h source code no touch. But it is really not quite well to let machine details (e.g. BF527, BF548, BF609) in the outside of arch/blackfin. So, I guess, we need to add new config value HAVE_MUSB for it in patch v2. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: musb: Kconfig: Depend on some machines under blackfin
Under blackfin, only bf527, bf548 and bf609 may use musb. The related error with allmodconfig: CC [M] drivers/usb/misc/trancevibrator.o In file included from drivers/usb/musb/musb_core.h:70:0, from drivers/usb/musb/musb_core.c:103: drivers/usb/musb/musb_core.c: In function 'musb_indexed_ep_select': drivers/usb/musb/musb_regs.h:458:32: error: 'USB_INDEX' undeclared (first use in this function) #define MUSB_INDEX USB_OFFSET(USB_INDEX) /* 8 bit */ ^ Signed-off-by: Chen Gang gang.chen.5...@gmail.com --- drivers/usb/musb/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig index 39db8b6..5fca898 100644 --- a/drivers/usb/musb/Kconfig +++ b/drivers/usb/musb/Kconfig @@ -6,7 +6,7 @@ # (M)HDRC = (Multipoint) Highspeed Dual-Role Controller config USB_MUSB_HDRC tristate 'Inventra Highspeed Dual Role Controller (TI, ADI, ...)' - depends on (USB || USB_GADGET) + depends on (USB || USB_GADGET) (!BLACKFIN || (BLACKFIN (BF527 || BF548 || BF609))) help Say Y here if your system has a dual role high speed USB controller based on the Mentor Graphics silicon IP. Then -- 1.9.3 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] net: usb: sr9700: Use 'SR_' prefix for the common register macros
The commone register macors (e.g. RSR) is too commont to drivers, it may
be conflict with the architectures (e.g. xtensa, sh).
The related warnings (with allmodconfig under xtensa):
CC [M] drivers/net/usb/sr9700.o
In file included from drivers/net/usb/sr9700.c:24:0:
drivers/net/usb/sr9700.h:65:0: warning: RSR redefined
#define RSR 0x06
^
In file included from ./arch/xtensa/include/asm/bitops.h:22:0,
from include/linux/bitops.h:36,
from include/linux/kernel.h:10,
from include/linux/list.h:8,
from include/linux/module.h:9,
from drivers/net/usb/sr9700.c:13:
./arch/xtensa/include/asm/processor.h:190:0: note: this is the location of
the previous definition
#define RSR(v,sr) __asm__ __volatile__ (rsr %0,__stringify(sr) : =a(v));
^
Signed-off-by: Chen Gang gang.chen.5...@gmail.com
---
drivers/net/usb/sr9700.c | 36 +-
drivers/net/usb/sr9700.h | 66
2 files changed, 51 insertions(+), 51 deletions(-)
diff --git a/drivers/net/usb/sr9700.c b/drivers/net/usb/sr9700.c
index 99b69af..4a1e9c4 100644
--- a/drivers/net/usb/sr9700.c
+++ b/drivers/net/usb/sr9700.c
@@ -77,7 +77,7 @@ static int wait_phy_eeprom_ready(struct usbnet *dev, int phy)
int ret;
udelay(1);
- ret = sr_read_reg(dev, EPCR, tmp);
+ ret = sr_read_reg(dev, SR_EPCR, tmp);
if (ret 0)
return ret;
@@ -98,15 +98,15 @@ static int sr_share_read_word(struct usbnet *dev, int phy,
u8 reg,
mutex_lock(dev-phy_mutex);
- sr_write_reg(dev, EPAR, phy ? (reg | EPAR_PHY_ADR) : reg);
- sr_write_reg(dev, EPCR, phy ? (EPCR_EPOS | EPCR_ERPRR) : EPCR_ERPRR);
+ sr_write_reg(dev, SR_EPAR, phy ? (reg | EPAR_PHY_ADR) : reg);
+ sr_write_reg(dev, SR_EPCR, phy ? (EPCR_EPOS | EPCR_ERPRR) : EPCR_ERPRR);
ret = wait_phy_eeprom_ready(dev, phy);
if (ret 0)
goto out_unlock;
- sr_write_reg(dev, EPCR, 0x0);
- ret = sr_read(dev, EPDR, 2, value);
+ sr_write_reg(dev, SR_EPCR, 0x0);
+ ret = sr_read(dev, SR_EPDR, 2, value);
netdev_dbg(dev-net, read shared %d 0x%02x returned 0x%04x, %d\n,
phy, reg, *value, ret);
@@ -123,19 +123,19 @@ static int sr_share_write_word(struct usbnet *dev, int
phy, u8 reg,
mutex_lock(dev-phy_mutex);
- ret = sr_write(dev, EPDR, 2, value);
+ ret = sr_write(dev, SR_EPDR, 2, value);
if (ret 0)
goto out_unlock;
- sr_write_reg(dev, EPAR, phy ? (reg | EPAR_PHY_ADR) : reg);
- sr_write_reg(dev, EPCR, phy ? (EPCR_WEP | EPCR_EPOS | EPCR_ERPRW) :
+ sr_write_reg(dev, SR_EPAR, phy ? (reg | EPAR_PHY_ADR) : reg);
+ sr_write_reg(dev, SR_EPCR, phy ? (EPCR_WEP | EPCR_EPOS | EPCR_ERPRW) :
(EPCR_WEP | EPCR_ERPRW));
ret = wait_phy_eeprom_ready(dev, phy);
if (ret 0)
goto out_unlock;
- sr_write_reg(dev, EPCR, 0x0);
+ sr_write_reg(dev, SR_EPCR, 0x0);
out_unlock:
mutex_unlock(dev-phy_mutex);
@@ -188,7 +188,7 @@ static int sr_mdio_read(struct net_device *netdev, int
phy_id, int loc)
if (loc == MII_BMSR) {
u8 value;
- sr_read_reg(dev, NSR, value);
+ sr_read_reg(dev, SR_NSR, value);
if (value NSR_LINKST)
rc = 1;
}
@@ -228,7 +228,7 @@ static u32 sr9700_get_link(struct net_device *netdev)
int rc = 0;
/* Get the Link Status directly */
- sr_read_reg(dev, NSR, value);
+ sr_read_reg(dev, SR_NSR, value);
if (value NSR_LINKST)
rc = 1;
@@ -281,8 +281,8 @@ static void sr9700_set_multicast(struct net_device *netdev)
}
}
- sr_write_async(dev, MAR, SR_MCAST_SIZE, hashes);
- sr_write_reg_async(dev, RCR, rx_ctl);
+ sr_write_async(dev, SR_MAR, SR_MCAST_SIZE, hashes);
+ sr_write_reg_async(dev, SR_RCR, rx_ctl);
}
static int sr9700_set_mac_address(struct net_device *netdev, void *p)
@@ -297,7 +297,7 @@ static int sr9700_set_mac_address(struct net_device
*netdev, void *p)
}
memcpy(netdev-dev_addr, addr-sa_data, netdev-addr_len);
- sr_write_async(dev, PAR, 6, netdev-dev_addr);
+ sr_write_async(dev, SR_PAR, 6, netdev-dev_addr);
return 0;
}
@@ -340,7 +340,7 @@ static int sr9700_bind(struct usbnet *dev, struct
usb_interface *intf)
mii-phy_id_mask = 0x1f;
mii-reg_num_mask = 0x1f;
- sr_write_reg(dev, NCR, NCR_RST);
+ sr_write_reg(dev, SR_NCR, NCR_RST);
udelay(20);
/* read MAC
@@ -348,17 +348,17 @@ static int sr9700_bind(struct usbnet *dev, struct
usb_interface *intf)
* EEPROM automatically to PAR. In case there is no EEPROM externally
Re: [PATCH] drivers/usb/host/ehci-xilinx-of.c: Include linux/of_irq.h to avoid compiling error
On 9/29/14 9:52, Greg Kroah-Hartman wrote: On Wed, Sep 24, 2014 at 11:41:55AM -0400, Alan Stern wrote: On Mon, 22 Sep 2014, Michal Simek wrote: Alan: Can you please add this patch to your queue? Greg: If Alan is not maintaining this part of kernel, is this patch in your queue? I have also not a problem to add this patch through my microblaze tree but I think it will be much better to use any USB tree. Greg should be able to accept a trivial patch like this one directly. I can, if someone will resend it so I can apply it :) This patch you have already applied (applied this patch in 2014-09-24), so I need not resend it. Thank all of you for your work. Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed -- 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] drivers/usb/host/ehci-xilinx-of.c: Include linux/of_irq.h to avoid compiling error
Need include it for irq_of_parse_and_map(), the related error with allmodconfig under microblaze: drivers/usb/host/ehci-xilinx-of.c: In function ‘ehci_hcd_xilinx_of_probe’: drivers/usb/host/ehci-xilinx-of.c:156:2: error: implicit declaration of function ‘irq_of_parse_and_map’ [-Werror=implicit-function-declaration] irq = irq_of_parse_and_map(dn, 0); ^ Signed-off-by: Chen Gang gang.chen.5...@gmail.com --- drivers/usb/host/ehci-xilinx-of.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/host/ehci-xilinx-of.c b/drivers/usb/host/ehci-xilinx-of.c index fe57710..a232836 100644 --- a/drivers/usb/host/ehci-xilinx-of.c +++ b/drivers/usb/host/ehci-xilinx-of.c @@ -31,6 +31,7 @@ #include linux/of.h #include linux/of_platform.h #include linux/of_address.h +#include linux/of_irq.h /** * ehci_xilinx_port_handed_over - hand the port out if failed to enable it -- 1.9.3 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] include/linux/usb/usb_phy_gen_xceiv.h: check built-in or module for swithing usb_nop_xceiv_register() implementation
On 11/15/2013 02:37 AM, Tony Lindgren wrote:
* Chen Gang gang.c...@asianux.com [131023 02:57]:
When CONFIG_NOP_USB_XCEIV is as 'm', usb_nop_xceiv_register() will be
exported when the related module is loaded. So for built-in source
code, still need use the empty one.
Or it will can not pass compiling, the related error (for arm, with
allmodconfig):
arch/arm/mach-omap2/built-in.o: In function `omap3_evm_init':
arch/arm/mach-omap2/board-omap3evm.c:703: undefined reference to
`usb_nop_xceiv_register'
Signed-off-by: Chen Gang gang.c...@asianux.com
Added Felipe to cc, he should probably deal with this one.
OK, thank you very much.
And welcome Felipe to help checking, when he has time.
:-)
Regards,
Tony
---
include/linux/usb/usb_phy_gen_xceiv.h |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/include/linux/usb/usb_phy_gen_xceiv.h
b/include/linux/usb/usb_phy_gen_xceiv.h
index f9a7e7b..8515958 100644
--- a/include/linux/usb/usb_phy_gen_xceiv.h
+++ b/include/linux/usb/usb_phy_gen_xceiv.h
@@ -12,7 +12,8 @@ struct usb_phy_gen_xceiv_platform_data {
unsigned int needs_reset:1;
};
-#if IS_ENABLED(CONFIG_NOP_USB_XCEIV)
+#if IS_BUILTIN(CONFIG_NOP_USB_XCEIV) || \
+(IS_MODULE(CONFIG_NOP_USB_XCEIV) defined(MODULE))
/* sometimes transceivers are accessed only through e.g. ULPI */
extern void usb_nop_xceiv_register(void);
extern void usb_nop_xceiv_unregister(void);
--
1.7.7.6
--
Chen Gang
--
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] include/linux/usb/usb_phy_gen_xceiv.h: check built-in or module for swithing usb_nop_xceiv_register() implementation
When CONFIG_NOP_USB_XCEIV is as 'm', usb_nop_xceiv_register() will be
exported when the related module is loaded. So for built-in source
code, still need use the empty one.
Or it will can not pass compiling, the related error (for arm, with
allmodconfig):
arch/arm/mach-omap2/built-in.o: In function `omap3_evm_init':
arch/arm/mach-omap2/board-omap3evm.c:703: undefined reference to
`usb_nop_xceiv_register'
Signed-off-by: Chen Gang gang.c...@asianux.com
---
include/linux/usb/usb_phy_gen_xceiv.h |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/include/linux/usb/usb_phy_gen_xceiv.h
b/include/linux/usb/usb_phy_gen_xceiv.h
index f9a7e7b..8515958 100644
--- a/include/linux/usb/usb_phy_gen_xceiv.h
+++ b/include/linux/usb/usb_phy_gen_xceiv.h
@@ -12,7 +12,8 @@ struct usb_phy_gen_xceiv_platform_data {
unsigned int needs_reset:1;
};
-#if IS_ENABLED(CONFIG_NOP_USB_XCEIV)
+#if IS_BUILTIN(CONFIG_NOP_USB_XCEIV) || \
+ (IS_MODULE(CONFIG_NOP_USB_XCEIV) defined(MODULE))
/* sometimes transceivers are accessed only through e.g. ULPI */
extern void usb_nop_xceiv_register(void);
extern void usb_nop_xceiv_unregister(void);
--
1.7.7.6
--
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] drivers: usb: gadget: add '__ref' for rndis_config_register() and cdc_config_register()
They are only called by '__ref' function multi_bind(), and they will
call '__init' functions, so recommend to let them '__ref' too.
The related warnings:
WARNING: drivers/usb/gadget/g_multi.o(.text+0xded6): Section mismatch in
reference from the variable .LM2921 to the variable .init.text:_rndis_do_config
The function .LM2921() references
the variable __init _rndis_do_config.
This is often because .LM2921 lacks a __init
annotation or the annotation of _rndis_do_config is wrong.
WARNING: drivers/usb/gadget/g_multi.o(.text+0xdf16): Section mismatch in
reference from the variable .LM2953 to the variable .init.text:_cdc_do_config
The function .LM2953() references
the variable __init _cdc_do_config.
This is often because .LM2953 lacks a __init
annotation or the annotation of _cdc_do_config is wrong.
Signed-off-by: Chen Gang gang.c...@asianux.com
---
drivers/usb/gadget/multi.c |8
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/multi.c b/drivers/usb/gadget/multi.c
index 2a1ebef..2339325 100644
--- a/drivers/usb/gadget/multi.c
+++ b/drivers/usb/gadget/multi.c
@@ -179,7 +179,7 @@ err_conf:
return ret;
}
-static int rndis_config_register(struct usb_composite_dev *cdev)
+static __ref int rndis_config_register(struct usb_composite_dev *cdev)
{
static struct usb_configuration config = {
.bConfigurationValue= MULTI_RNDIS_CONFIG_NUM,
@@ -194,7 +194,7 @@ static int rndis_config_register(struct usb_composite_dev
*cdev)
#else
-static int rndis_config_register(struct usb_composite_dev *cdev)
+static __ref int rndis_config_register(struct usb_composite_dev *cdev)
{
return 0;
}
@@ -241,7 +241,7 @@ err_conf:
return ret;
}
-static int cdc_config_register(struct usb_composite_dev *cdev)
+static __ref int cdc_config_register(struct usb_composite_dev *cdev)
{
static struct usb_configuration config = {
.bConfigurationValue= MULTI_CDC_CONFIG_NUM,
@@ -256,7 +256,7 @@ static int cdc_config_register(struct usb_composite_dev
*cdev)
#else
-static int cdc_config_register(struct usb_composite_dev *cdev)
+static __ref int cdc_config_register(struct usb_composite_dev *cdev)
{
return 0;
}
--
1.7.7.6
--
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/2] usb: class: move checking 'actual' code block into checking 'buffer[1]' code block
Hello Maintainers:
Please help check this patch when you have time.
Thanks.
On 07/11/2013 09:08 AM, Chen Gang wrote:
Hello Maintainers:
Please help check this patch when you have time, thanks.
BTW: this uninitialized variable warning may not be found by gcc
compiler (which a gcc bug exists almost 10 years).
Thanks.
On 07/02/2013 12:06 PM, Chen Gang wrote:
The variable 'actual' is only used in checking 'buffer[1]' code
block, so need move it into, or it may not be initialized.
Signed-off-by: Chen Gang gang.c...@asianux.com
---
drivers/usb/class/usbtmc.c | 14 --
1 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 609dbc2..42d62c9 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -786,7 +786,7 @@ usbtmc_clear_check_status:
goto exit;
}
-if (buffer[1] == 1)
+if (buffer[1] == 1) {
do {
dev_dbg(dev, Reading from bulk in EP\n);
@@ -805,11 +805,13 @@ usbtmc_clear_check_status:
} while ((actual == max_size)
(n USBTMC_MAX_READS_TO_CLEAR_BULK_IN));
-if (actual == max_size) {
-dev_err(dev, Couldn't clear device buffer within %d cycles\n,
-USBTMC_MAX_READS_TO_CLEAR_BULK_IN);
-rv = -EPERM;
-goto exit;
+if (actual == max_size) {
+dev_err(dev,
+Couldn't clear device buffer within %d
cycles\n,
+USBTMC_MAX_READS_TO_CLEAR_BULK_IN);
+rv = -EPERM;
+goto exit;
+}
}
goto usbtmc_clear_check_status;
--
Chen Gang
--
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/2] usb: class: move checking 'actual' code block into checking 'buffer[1]' code block
On 07/19/2013 12:43 PM, Ming Lei wrote: Hi Chen Gang, On Fri, Jul 19, 2013 at 12:21 PM, Chen Gang gang.c...@asianux.com wrote: Hello Maintainers: Please help check this patch when you have time. Looks your patch is correct, and I think Greg will handle your patch when usb-next tree is open. Thank you very much for your information. Thanks, -- Ming Lei -- 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 -- Chen Gang -- 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/2] usb: class: move checking 'actual' code block into checking 'buffer[1]' code block
Hello Maintainers:
Please help check this patch when you have time, thanks.
BTW: this uninitialized variable warning may not be found by gcc
compiler (which a gcc bug exists almost 10 years).
Thanks.
On 07/02/2013 12:06 PM, Chen Gang wrote:
The variable 'actual' is only used in checking 'buffer[1]' code
block, so need move it into, or it may not be initialized.
Signed-off-by: Chen Gang gang.c...@asianux.com
---
drivers/usb/class/usbtmc.c | 14 --
1 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 609dbc2..42d62c9 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -786,7 +786,7 @@ usbtmc_clear_check_status:
goto exit;
}
- if (buffer[1] == 1)
+ if (buffer[1] == 1) {
do {
dev_dbg(dev, Reading from bulk in EP\n);
@@ -805,11 +805,13 @@ usbtmc_clear_check_status:
} while ((actual == max_size)
(n USBTMC_MAX_READS_TO_CLEAR_BULK_IN));
- if (actual == max_size) {
- dev_err(dev, Couldn't clear device buffer within %d cycles\n,
- USBTMC_MAX_READS_TO_CLEAR_BULK_IN);
- rv = -EPERM;
- goto exit;
+ if (actual == max_size) {
+ dev_err(dev,
+ Couldn't clear device buffer within %d
cycles\n,
+ USBTMC_MAX_READS_TO_CLEAR_BULK_IN);
+ rv = -EPERM;
+ goto exit;
+ }
}
goto usbtmc_clear_check_status;
--
Chen Gang
--
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: class: 2 modifications for usbtmc.c
Hello Maintainers: Patch 1/2: move checking 'actual' code block into checking 'buffer[1]' code block. Patch 2/2: check the looping count for USBTMC_MAX_READS_TO_CLEAR_BULK_IN There are several code blocks which are almost same as the current modified code block. If the 2 patches can pass checking, we also need check the other related code blocks. Diff stat: --- drivers/usb/class/usbtmc.c | 22 -- 1 files changed, 16 insertions(+), 6 deletions(-) Thanks. -- Chen Gang -- 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: class: move checking 'actual' code block into checking 'buffer[1]' code block
The variable 'actual' is only used in checking 'buffer[1]' code
block, so need move it into, or it may not be initialized.
Signed-off-by: Chen Gang gang.c...@asianux.com
---
drivers/usb/class/usbtmc.c | 14 --
1 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 609dbc2..42d62c9 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -786,7 +786,7 @@ usbtmc_clear_check_status:
goto exit;
}
- if (buffer[1] == 1)
+ if (buffer[1] == 1) {
do {
dev_dbg(dev, Reading from bulk in EP\n);
@@ -805,11 +805,13 @@ usbtmc_clear_check_status:
} while ((actual == max_size)
(n USBTMC_MAX_READS_TO_CLEAR_BULK_IN));
- if (actual == max_size) {
- dev_err(dev, Couldn't clear device buffer within %d cycles\n,
- USBTMC_MAX_READS_TO_CLEAR_BULK_IN);
- rv = -EPERM;
- goto exit;
+ if (actual == max_size) {
+ dev_err(dev,
+ Couldn't clear device buffer within %d
cycles\n,
+ USBTMC_MAX_READS_TO_CLEAR_BULK_IN);
+ rv = -EPERM;
+ goto exit;
+ }
}
goto usbtmc_clear_check_status;
--
1.7.7.6
--
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: class: check the looping count for USBTMC_MAX_READS_TO_CLEAR_BULK_IN.
The variable 'n' is initialized before goto usbtmc_clear_check_status
looping, and used in inside do ... while looping,
So it may be not less than 'USBTMC_MAX_READS_TO_CLEAR_BULK_IN', need
add related checking for it before the looping do ... while.
Signed-off-by: Chen Gang gang.c...@asianux.com
---
drivers/usb/class/usbtmc.c |8
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 42d62c9..cc137c6 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -787,6 +787,14 @@ usbtmc_clear_check_status:
}
if (buffer[1] == 1) {
+ if (n = USBTMC_MAX_READS_TO_CLEAR_BULK_IN) {
+ dev_err(dev,
+ Couldn't clear device buffer within %d
cycles\n,
+ USBTMC_MAX_READS_TO_CLEAR_BULK_IN);
+ rv = -EPERM;
+ goto exit;
+ }
+
do {
dev_dbg(dev, Reading from bulk in EP\n);
--
1.7.7.6
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] USB: mos7840: looping issue: avoid the return value overriden by looping
On 2013年04月03日 17:25, Chen Gang wrote: inside the 'for' looping: the return value 'rv' may override if not have a check in time. the next checking (outside the 'for' looping): can not find failure which generated during the 'for' looping the fix is for: let outside know about the failure, and not stop servicing the other ports just because there is an error in another port. Signed-off-by: Chen Gang gang.c...@asianux.com Signed-off-by: Oliver Neukum oneu...@suse.de --- Hello Greg KH: according to what you said for another patches of mine. I need test it, firstly. and then resend it without Signed-off-by: Oliver Neukum oneu...@suse.de. is it correct ? thanks. -- Chen Gang Asianux Corporation -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] USB: mos7840: looping issue: avoid the return value overriden by looping
On 2013年04月06日 15:34, Chen Gang wrote: On 2013年04月03日 17:25, Chen Gang wrote: inside the 'for' looping: the return value 'rv' may override if not have a check in time. the next checking (outside the 'for' looping): can not find failure which generated during the 'for' looping the fix is for: let outside know about the failure, and not stop servicing the other ports just because there is an error in another port. Signed-off-by: Chen Gang gang.c...@asianux.com Signed-off-by: Oliver Neukum oneu...@suse.de --- Hello Greg KH: according to what you said for another patches of mine. I need test it, firstly. and then resend it without Signed-off-by: Oliver Neukum oneu...@suse.de. is it correct ? thanks. excuse me, I have no related hard ware, and seems not easy to virtualizing. so I think of an idea to test it, please help check whether it is OK. our demands: our test goal is to check what we modified whether can work correctly. it only has effect within the function mos7840_interrupt_callback. it only has effect with the work flow procedure. so what we should do is test mos7840_interrupt_callback work flow procedure with our modification. plan to do: will make a driver which will call function mos7840_interrupt_callback. rewrite all sub functions which mos7840_interrupt_callback will call. such as mos7840_get_port_private, mos7840_get_reg, and usb_submit_urb. provide related data to cover all related work flow procedures. a. let all things succeed (normal work flow). b. let a failure occures at the first of the looping. c. let a failure occures in the middle of the looping. d. let a failure occures at the last of the looping. e. let 2 failures occure during the looping. f. let 3 failures occure during the looping (one at the first). g. let 3 failures occure during the looping (one at the end). welcome any suggestions or completions. thanks. By the way: in this patch, I think Signed-off-by: Oliver Neukum oneu...@suse.de is ok. the reason is he gives necessary design information to direct implementaion. -- Chen Gang Asianux Corporation -- 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] USB: mos7840: looping issue: avoid the return value overriden by looping
inside the 'for' looping:
the return value 'rv' may override if not have a check in time.
the next checking (outside the 'for' looping):
can not find failure which generated during the 'for' looping
so need let outside know about the failure.
the original related commit is:
commit 0de9a7024e7ae62512d080c7e2beb59d82958cd5
Author: Oliver Neukum oneu...@suse.de
Date: Fri Mar 16 20:28:28 2007 +0100
Signed-off-by: Chen Gang gang.c...@asianux.com
---
drivers/usb/serial/mos7840.c | 15 ++-
1 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c
index a0d5ea5..91e6c2c 100644
--- a/drivers/usb/serial/mos7840.c
+++ b/drivers/usb/serial/mos7840.c
@@ -585,7 +585,7 @@ static void mos7840_interrupt_callback(struct urb *urb)
__u16 Data;
unsigned char *data;
__u8 sp[5], st;
- int i, rv = 0;
+ int i;
__u16 wval, wreg = 0;
int status = urb-status;
@@ -651,13 +651,18 @@ static void mos7840_interrupt_callback(struct urb *urb)
wreg = MODEM_STATUS_REGISTER;
break;
}
- rv = mos7840_get_reg(mos7840_port, wval, wreg,
Data);
+ if (mos7840_get_reg(mos7840_port, wval, wreg,
+ Data) 0)
+ goto exit;
}
}
}
- if (!(rv 0))
- /* the completion handler for the control urb will resubmit */
- return;
+
+ /*
+* if all things ok, the completion handler for the control
+* urb will resubmit
+*/
+ return;
exit:
result = usb_submit_urb(urb, GFP_ATOMIC);
if (result) {
--
1.7.7.6
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] USB: mos7840: looping issue: avoid the return value overriden by looping
On 2013年04月03日 16:12, Oliver Neukum wrote: Good catch, but I am afraid the fix is wrong. You cannot stop servicing the other ports just because there is an error in another port. Regards Oliver ok, thanks. and how about my original fix for it ? (the related patch is below) :-) On 2013年04月01日 11:50, Chen Gang wrote: inside the 'for' looping: the return value 'rv' may override if not have a check in time. next checking, outside the 'for' looping: can not find failure which generated during the 'for' looping so need let outside know about the failure. Signed-off-by: Chen Gang gang.c...@asianux.com --- drivers/usb/serial/mos7840.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c index a0d5ea5..13aae1e 100644 --- a/drivers/usb/serial/mos7840.c +++ b/drivers/usb/serial/mos7840.c @@ -651,7 +651,9 @@ static void mos7840_interrupt_callback(struct urb *urb) wreg = MODEM_STATUS_REGISTER; break; } - rv = mos7840_get_reg(mos7840_port, wval, wreg, Data); + if (mos7840_get_reg(mos7840_port, wval, + wreg, Data) 0) + rv = -1; } } } Asianux Corporation -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] USB: mos7840: looping issue: avoid the return value overriden by looping
On 2013年04月03日 16:31, Oliver Neukum wrote: correct but dirty. A boolean and a comment would be nice. ok, thanks. I will send patch v3. -- Chen Gang Asianux Corporation -- 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] USB: mos7840: looping issue: avoid the return value overriden by looping
inside the 'for' looping: the return value 'rv' may override if not have a check in time. the next checking (outside the 'for' looping): can not find failure which generated during the 'for' looping the fix is for: let outside know about the failure, and not stop servicing the other ports just because there is an error in another port. Signed-off-by: Chen Gang gang.c...@asianux.com Signed-off-by: Oliver Neukum oneu...@suse.de --- drivers/usb/serial/mos7840.c | 12 +--- 1 files changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/usb/serial/mos7840.c b/drivers/usb/serial/mos7840.c index a0d5ea5..20cab64 100644 --- a/drivers/usb/serial/mos7840.c +++ b/drivers/usb/serial/mos7840.c @@ -585,7 +585,8 @@ static void mos7840_interrupt_callback(struct urb *urb) __u16 Data; unsigned char *data; __u8 sp[5], st; - int i, rv = 0; + int i; + bool rv = true; __u16 wval, wreg = 0; int status = urb-status; @@ -651,11 +652,16 @@ static void mos7840_interrupt_callback(struct urb *urb) wreg = MODEM_STATUS_REGISTER; break; } - rv = mos7840_get_reg(mos7840_port, wval, wreg, Data); + if (mos7840_get_reg(mos7840_port, wval, wreg, + Data) 0) + /* +* mark failure, but still need continue +*/ + rv = false; } } } - if (!(rv 0)) + if (rv) /* the completion handler for the control urb will resubmit */ return; exit: -- 1.7.7.6 -- 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] drivers/usb/core: processing failure, maching resume condition with suspend condition
On 2013年04月01日 22:58, Alan Stern wrote: Thanks for spotting this. Acked-by: Alan Stern st...@rowland.harvard.edu thank you too. -- Chen Gang Asianux Corporation -- 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] drivers/usb/core: processing failure, maching resume condition with suspend condition
when suspend, it need check 'udev-actconfig'.
so when process failure, also need check it.
Signed-off-by: Chen Gang gang.c...@asianux.com
---
drivers/usb/core/driver.c | 10 ++
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index eb1d00a..1a50003 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -1253,10 +1253,12 @@ static int usb_suspend_both(struct usb_device *udev,
pm_message_t msg)
/* If the suspend failed, resume interfaces that did get suspended */
if (status != 0) {
- msg.event ^= (PM_EVENT_SUSPEND | PM_EVENT_RESUME);
- while (++i n) {
- intf = udev-actconfig-interface[i];
- usb_resume_interface(udev, intf, msg, 0);
+ if (udev-actconfig) {
+ msg.event ^= (PM_EVENT_SUSPEND | PM_EVENT_RESUME);
+ while (++i n) {
+ intf = udev-actconfig-interface[i];
+ usb_resume_interface(udev, intf, msg, 0);
+ }
}
/* If the suspend succeeded then prevent any more URB submissions
--
1.7.7.6
--
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] drivers/usb/gadget: beautify code, delete unused code
于 2013年03月14日 23:07, Alan Stern 写道: I have a net2280. Which patch do you want me to test? The two of you have come up with two different versions. thank Alan very much, thank Felipe, too. :-) -- Chen Gang Asianux Corporation -- 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] vers/usb/gadget: beautify code, delete useless comments
since parameter driver has been deleted, also need delete relative comments. relative commit number is d93e2600d80fc41ccf339b4a2843a3007d479907 Signed-off-by: Chen Gang gang.c...@asianux.com --- drivers/usb/gadget/s3c-hsudc.c |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/drivers/usb/gadget/s3c-hsudc.c b/drivers/usb/gadget/s3c-hsudc.c index 458965a..7da26cf 100644 --- a/drivers/usb/gadget/s3c-hsudc.c +++ b/drivers/usb/gadget/s3c-hsudc.c @@ -283,7 +283,6 @@ static void s3c_hsudc_nuke_ep(struct s3c_hsudc_ep *hsep, int status) /** * s3c_hsudc_stop_activity - Stop activity on all endpoints. * @hsudc: Device controller for which EP activity is to be stopped. - * @driver: Reference to the gadget driver which is currently active. * * All the endpoints are stopped and any pending transfer requests if any on * the endpoint are terminated. -- 1.7.7.6 -- 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] drivers/usb/gadget: beautify code, delete unused code
于 2013年03月04日 22:35, Felipe Balbi 写道: since stop_activity() also gets called from RESET interrupt, and in that case we need to call driver-disconnect(). Can you make a simple test that would take current code and issue a device reset to see if that would break, then apply my suggestion above and run the same test again? after reference the commit: d93e2600d80fc41ccf339b4a2843a3007d479907 it seems udc_start and udc_stop will have effect, so not need call driver-disconnect() is it correct ? thanks. -- Chen Gang Asianux Corporation -- 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] drivers/usb/gadget: beautify code, delete unused code
于 2013年03月11日 18:17, Chen Gang 写道: 于 2013年03月04日 22:35, Felipe Balbi 写道: since stop_activity() also gets called from RESET interrupt, and in that case we need to call driver-disconnect(). Can you make a simple test that would take current code and issue a device reset to see if that would break, then apply my suggestion above and run the same test again? after reference the commit: d93e2600d80fc41ccf339b4a2843a3007d479907 it seems udc_start and udc_stop will have effect, so not need call driver-disconnect() is it correct ? thanks. another reference commit: 6166c24669678662547bb4e5dbd6a810268b8b7b also seems udc_start and udc_stop will have effect. -- Chen Gang Asianux Corporation -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: s3c-hsudc: delete outdated comment
since commit d93e260 (usb: gadget: s3c-hsudc: use udc_start and udc_stop functions) the 'driver' parameter has been deleted from s3c_hsudc_stop_activity() but its documentation was left outdated. This patch deletes the comment since it makes no sense anymore. Signed-off-by: Chen Gang gang.c...@asianux.com --- drivers/usb/gadget/s3c-hsudc.c |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/drivers/usb/gadget/s3c-hsudc.c b/drivers/usb/gadget/s3c-hsudc.c index 458965a..7da26cf 100644 --- a/drivers/usb/gadget/s3c-hsudc.c +++ b/drivers/usb/gadget/s3c-hsudc.c @@ -283,7 +283,6 @@ static void s3c_hsudc_nuke_ep(struct s3c_hsudc_ep *hsep, int status) /** * s3c_hsudc_stop_activity - Stop activity on all endpoints. * @hsudc: Device controller for which EP activity is to be stopped. - * @driver: Reference to the gadget driver which is currently active. * * All the endpoints are stopped and any pending transfer requests if any on * the endpoint are terminated. -- 1.7.7.6 -- 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] drivers/usb/gadget: beautify code, delete unused code
于 2013年03月11日 18:50, Felipe Balbi 写道: On Mon, Mar 11, 2013 at 06:42:25PM +0800, Chen Gang wrote: if I can not find other members to help us, I will try to find another ways. code inspection works most of the time. excuse me, my English is not quite well, could you describe it with more details ? thanks. -- Chen Gang Asianux Corporation -- 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] drivers/usb/gadget: beautify code, delete unused code
于 2013年03月11日 19:07, Felipe Balbi 写道: I mean just reading the code and understanding what it does. When you don't have HW to test, it's the only way to patch the driver. ok, thanks. and I still try to find another ways, at least within this week (before 2013-03-15). thanks again. -- Chen Gang Flying Transformer -- 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] vers/usb/gadget: beautify code, delete useless comments
Felipe Balbi also pointed my subject and comments issue. and I have sent the related new patch for it. the new patch subject is [PATCH] usb: gadget: s3c-hsudc: delete outdated comment please help checking the new patch, thanks. :-) 于 2013年03月11日 19:28, Sergei Shtylyov 写道: Hello. On 11-03-2013 14:14, Chen Gang wrote: since parameter driver has been deleted, also need delete relative comments. relative You probably meant related in both cases? ok, thanks, I need notice, next time. commit number is d93e2600d80fc41ccf339b4a2843a3007d479907 Please also specify that commit's summary line in parens (or however you like). ok, thanks. I need notice, next time. Signed-off-by: Chen Gang gang.c...@asianux.com WBR, Sergei -- Chen Gang Asianux Corporation -- 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] drivers/usb/gadget: beautify code, delete unused code
于 2013年03月05日 19:10, Felipe Balbi 写道: no problem, take your time. Family is more important than a bugfix. I don't have that HW anyway and can't test fixes to it. I do not have either, but I should try (I need think ways to perform, such as qemu kvm, or another ways) welcome additional suggestions or completions. thanks :-) -- Chen Gang Asianux Corporation -- 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] drivers/usb/gadget: using strlcpy instead of strncpy
于 2013年03月02日 03:47, Laurent Pinchart 写道: I've taken the patch in my tree. I've just sent a consolidated series of most pending UVC gadget patches to the list, and I will send you a pull request as soon as I receive a Tested-by. thanks -- Chen Gang Asianux Corporation -- 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] drivers/usb/gadget: beautify code, delete unused code
于 2013年03月04日 22:35, Felipe Balbi 写道:
this is the wrong fix, I believe. Looks like when I wrote the commits
you mention, I deleted more code then I should. Looks like the real fix
would be to add back:
/* report disconnect; the driver is already quiesced */
if (driver) {
spin_unlock(dev-lock);
driver-disconnect(dev-gadget);
spin_lock(dev-lock);
}
ok, thanks.
since stop_activity() also gets called from RESET interrupt, and in that
case we need to call driver-disconnect(). Can you make a simple test
that would take current code and issue a device reset to see if that
would break, then apply my suggestion above and run the same test again?
ok, thanks, that is what I should do.
but excuse me for my time limitation:
in these days, my father had a serious heart disease in hospital.
within this week end, most of my time has to be in hospital.
(God bless, and thank Jesus Christ, my father is safe now).
within my company (Asianux), also has something to do.
so I try to finish it in next week end (Mar-15-2013).
if we can not bear the time point, please reply (or prefer another
member to finish it)
thanks.
:-)
If you want, I have a simple libusb-1.0-based little app which can issue
resets to any device you ask. Currently it will reset a device 10K
times but you can remove the for loop if you want:
thank you for your information, they are valuable to me.
--
Chen Gang
Asianux Corporation
--
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] drivers/usb/gadget: using strlcpy instead of strncpy
于 2013年03月04日 01:35, Laurent Pinchart 写道: On Sunday 03 March 2013 01:23:46 Felipe Balbi wrote: On Fri, Mar 01, 2013 at 08:47:34PM +0100, Laurent Pinchart wrote: On Wednesday 27 February 2013 10:26:23 Felipe Balbi wrote: On Sat, Feb 02, 2013 at 03:48:54PM +0800, Chen Gang wrote: for NUL terminated string, better notice '\0' in the end. Signed-off-by: Chen Gang gang.c...@asianux.com Laurent, are you taking this patch or should I ? I've taken the patch in my tree. I've just sent a consolidated series of most pending UVC gadget patches to the list, and I will send you a pull request as soon as I receive a Tested-by. I'll take them as patches. No problem. Bhupesh wrote that he would test the patches this week. Hopefully there will be no further issue. thank you for your work. :-) -- Chen Gang Asianux Corporation -- 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] drivers/usb/gadget: beautify code, delete unused code
originally, when deleted the relative code, left some 'another'.
need delete 'another', too.
the relative patches are:
commit 96f8db6a77e3490608e5b5b3f57e7201f8c85496
Author: Felipe Balbi ba...@ti.com
Date: Mon Oct 10 10:33:47 2011 +0300
usb: gadget: net2272: convert to new style
commit 4cf5e00b055ba5e4f3852e477a2a4346730ea283
Author: Felipe Balbi ba...@ti.com
Date: Mon Oct 10 10:37:17 2011 +0300
usb: gadget: net2280: convert to new style
Signed-off-by: Chen Gang gang.c...@asianux.com
---
drivers/usb/gadget/net2272.c |4
drivers/usb/gadget/net2280.c |4
2 files changed, 0 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/gadget/net2272.c b/drivers/usb/gadget/net2272.c
index d226058..22ee57c 100644
--- a/drivers/usb/gadget/net2272.c
+++ b/drivers/usb/gadget/net2272.c
@@ -1484,10 +1484,6 @@ stop_activity(struct net2272 *dev, struct
usb_gadget_driver *driver)
{
int i;
- /* don't disconnect if it's not connected */
- if (dev-gadget.speed == USB_SPEED_UNKNOWN)
- driver = NULL;
-
/* stop hardware; prevent new request submissions;
* and kill any outstanding requests.
*/
diff --git a/drivers/usb/gadget/net2280.c b/drivers/usb/gadget/net2280.c
index a1b650e..4af3a4e 100644
--- a/drivers/usb/gadget/net2280.c
+++ b/drivers/usb/gadget/net2280.c
@@ -1935,10 +1935,6 @@ stop_activity (struct net2280 *dev, struct
usb_gadget_driver *driver)
{
int i;
- /* don't disconnect if it's not connected */
- if (dev-gadget.speed == USB_SPEED_UNKNOWN)
- driver = NULL;
-
/* stop hardware; prevent new request submissions;
* and kill any outstanding requests.
*/
--
1.7.7.6
--
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] drivers/usb/gadget: using strlcpy instead of strncpy
for NUL terminated string, better notice '\0' in the end. Signed-off-by: Chen Gang gang.c...@asianux.com --- drivers/usb/gadget/f_uvc.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/drivers/usb/gadget/f_uvc.c b/drivers/usb/gadget/f_uvc.c index 5b62987..92efd6e 100644 --- a/drivers/usb/gadget/f_uvc.c +++ b/drivers/usb/gadget/f_uvc.c @@ -16,6 +16,7 @@ #include linux/fs.h #include linux/list.h #include linux/mutex.h +#include linux/string.h #include linux/usb/ch9.h #include linux/usb/gadget.h #include linux/usb/video.h @@ -419,7 +420,7 @@ uvc_register_video(struct uvc_device *uvc) video-parent = cdev-gadget-dev; video-fops = uvc_v4l2_fops; video-release = video_device_release; - strncpy(video-name, cdev-gadget-name, sizeof(video-name)); + strlcpy(video-name, cdev-gadget-name, sizeof(video-name)); uvc-vdev = video; video_set_drvdata(video, uvc); -- 1.7.7.6 -- 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] drivers/usb/core: using strlcpy instead of strncpy
for NUL terminated string, better notice '\0' in the end.
Signed-off-by: Chen Gang gang.c...@asianux.com
---
drivers/usb/core/devio.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index b78fbe2..4a863fd 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -40,6 +40,7 @@
#include linux/signal.h
#include linux/poll.h
#include linux/module.h
+#include linux/string.h
#include linux/usb.h
#include linux/usbdevice_fs.h
#include linux/usb/hcd.h /* for usbcore internals */
@@ -1077,7 +1078,7 @@ static int proc_getdriver(struct dev_state *ps, void
__user *arg)
if (!intf || !intf-dev.driver)
ret = -ENODATA;
else {
- strncpy(gd.driver, intf-dev.driver-name,
+ strlcpy(gd.driver, intf-dev.driver-name,
sizeof(gd.driver));
ret = (copy_to_user(arg, gd, sizeof(gd)) ? -EFAULT : 0);
}
--
1.7.7.6
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 v2] USB: uhci: beautify source code
于 2013年01月25日 05:59, Greg KH 写道: This patch was line-wrapped and trailing spaces dropped, making it impossible to apply. I edited it by hand to get it to work, but please be more careful in the future. thank you very much. I should notice, next time. -- Chen Gang Asianux Corporation -- 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] drivers/usb/host/uhci-*: fix memory flow bug and beautify source code
PATCH 1/2: check buffer length to avoid memory overflow PATCH 2/2: beautify source code PATCH 2/2 is made based on PATCH 1/2 please apply PATCH 1/2 firstly, and then apply PATCH 2/2 total stat (2 patches together): --- drivers/usb/host/uhci-debug.c | 178 +++-- drivers/usb/host/uhci-hcd.c | 31 --- drivers/usb/host/uhci-hub.c |6 +- drivers/usb/host/uhci-q.c |2 +- 4 files changed, 140 insertions(+), 77 deletions(-) -- 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] drivers/usb/host/uhci-* : check buffer length to avoid memory overflow
for function uhci_sprint_schedule:
the buffer len is MAX_OUTPUT: 64 * 1024, which may not be enough:
may loop UHCI_NUMFRAMES times (UHCI_NUMFRAMES is 1024)
each time of loop may get more than 64 bytes
so need check the buffer length to avoid memory overflow
this patch fix it like this:
at first, make enough room for buffering the exceeding contents
judge the contents which written whether bigger than buffer length
if bigger (the exceeding contents will be in the exceeding buffer)
break current work flow, and return.
Signed-off-by: Chen Gang gang.c...@asianux.com
---
drivers/usb/host/uhci-debug.c | 150 -
drivers/usb/host/uhci-hcd.c |4 +-
drivers/usb/host/uhci-q.c |2 +-
3 files changed, 107 insertions(+), 49 deletions(-)
diff --git a/drivers/usb/host/uhci-debug.c b/drivers/usb/host/uhci-debug.c
index fc0b0da..8a55bb2 100644
--- a/drivers/usb/host/uhci-debug.c
+++ b/drivers/usb/host/uhci-debug.c
@@ -16,6 +16,8 @@
#include uhci-hcd.h
+#define EXTRA_SPACE1024
+
static struct dentry *uhci_debugfs_root;
#ifdef DEBUG
@@ -44,10 +46,6 @@ static int uhci_show_td(struct uhci_hcd *uhci, struct
uhci_td *td, char *buf,
char *spid;
u32 status, token;
- /* Try to make sure there's enough memory */
- if (len 160)
- return 0;
-
status = td_status(uhci, td);
out += sprintf(out, %*s[%p] link (%08x) , space, , td,
hc32_to_cpu(uhci, td-link));
@@ -64,6 +62,8 @@ static int uhci_show_td(struct uhci_hcd *uhci, struct uhci_td
*td, char *buf,
(status TD_CTRL_CRCTIMEO) ? CRC/Timeo : ,
(status TD_CTRL_BITSTUFF) ? BitStuff : ,
status 0x7ff);
+ if (out - buf len)
+ goto done;
token = td_token(uhci, td);
switch (uhci_packetid(token)) {
@@ -90,6 +90,9 @@ static int uhci_show_td(struct uhci_hcd *uhci, struct uhci_td
*td, char *buf,
spid);
out += sprintf(out, (buf=%08x)\n, hc32_to_cpu(uhci, td-buffer));
+done:
+ if (out - buf len)
+ out += sprintf(out, ...\n);
return out - buf;
}
@@ -101,8 +104,6 @@ static int uhci_show_urbp(struct uhci_hcd *uhci, struct
urb_priv *urbp,
int i, nactive, ninactive;
char *ptype;
- if (len 200)
- return 0;
out += sprintf(out, urb_priv [%p] , urbp);
out += sprintf(out, urb [%p] , urbp-urb);
@@ -110,6 +111,8 @@ static int uhci_show_urbp(struct uhci_hcd *uhci, struct
urb_priv *urbp,
out += sprintf(out, Dev=%d , usb_pipedevice(urbp-urb-pipe));
out += sprintf(out, EP=%x(%s) , usb_pipeendpoint(urbp-urb-pipe),
(usb_pipein(urbp-urb-pipe) ? IN : OUT));
+ if (out - buf len)
+ goto done;
switch (usb_pipetype(urbp-urb-pipe)) {
case PIPE_ISOCHRONOUS: ptype = ISO; break;
@@ -128,6 +131,9 @@ static int uhci_show_urbp(struct uhci_hcd *uhci, struct
urb_priv *urbp,
out += sprintf(out, Unlinked=%d, urbp-urb-unlinked);
out += sprintf(out, \n);
+ if (out - buf len)
+ goto done;
+
i = nactive = ninactive = 0;
list_for_each_entry(td, urbp-td_list, list) {
if (urbp-qh-type != USB_ENDPOINT_XFER_ISOC
@@ -135,6 +141,8 @@ static int uhci_show_urbp(struct uhci_hcd *uhci, struct
urb_priv *urbp,
out += sprintf(out, %*s%d: , space + 2, , i);
out += uhci_show_td(uhci, td, out,
len - (out - buf), 0);
+ if (out - buf len)
+ goto tail;
} else {
if (td_status(uhci, td) TD_CTRL_ACTIVE)
++nactive;
@@ -146,7 +154,10 @@ static int uhci_show_urbp(struct uhci_hcd *uhci, struct
urb_priv *urbp,
out += sprintf(out, %*s[skipped %d inactive and %d active
TDs]\n,
space, , ninactive, nactive);
-
+done:
+ if (out - buf len)
+ out += sprintf(out, ...\n);
+tail:
return out - buf;
}
@@ -158,10 +169,6 @@ static int uhci_show_qh(struct uhci_hcd *uhci,
__hc32 element = qh_element(qh);
char *qtype;
- /* Try to make sure there's enough memory */
- if (len 80 * 7)
- return 0;
-
switch (qh-type) {
case USB_ENDPOINT_XFER_ISOC: qtype = ISO; break;
case USB_ENDPOINT_XFER_INT: qtype = INT; break;
@@ -182,6 +189,8 @@ static int uhci_show_qh(struct uhci_hcd *uhci,
else if (qh-type == USB_ENDPOINT_XFER_INT)
out += sprintf(out, %*speriod %d phase %d load %d us\n,
space, , qh-period, qh-phase, qh-load);
+ if (out - buf len)
+ goto done
[PATCH 2/2] drivers/usb/host/uhci-*: beautify source code
get rid of the line breaks in string constants.
let comments within 80 with limitation.
delete ' \' at the end of a statement.
Signed-off-by: Chen Gang gang.c...@asianux.com
---
drivers/usb/host/uhci-debug.c | 28
drivers/usb/host/uhci-hcd.c | 27 +--
drivers/usb/host/uhci-hub.c |6 --
3 files changed, 33 insertions(+), 28 deletions(-)
diff --git a/drivers/usb/host/uhci-debug.c b/drivers/usb/host/uhci-debug.c
index 8a55bb2..4557375 100644
--- a/drivers/usb/host/uhci-debug.c
+++ b/drivers/usb/host/uhci-debug.c
@@ -151,8 +151,8 @@ static int uhci_show_urbp(struct uhci_hcd *uhci, struct
urb_priv *urbp,
}
}
if (nactive + ninactive 0)
- out += sprintf(out, %*s[skipped %d inactive and %d active
- TDs]\n,
+ out += sprintf(out,
+ %*s[skipped %d inactive and %d active TDs]\n,
space, , ninactive, nactive);
done:
if (out - buf len)
@@ -182,8 +182,8 @@ static int uhci_show_qh(struct uhci_hcd *uhci,
hc32_to_cpu(uhci, qh-link),
hc32_to_cpu(uhci, element));
if (qh-type == USB_ENDPOINT_XFER_ISOC)
- out += sprintf(out, %*speriod %d phase %d load %d us,
- frame %x desc [%p]\n,
+ out += sprintf(out,
+ %*speriod %d phase %d load %d us, frame %x
desc [%p]\n,
space, , qh-period, qh-phase, qh-load,
qh-iso_frame, qh-iso_packet_desc);
else if (qh-type == USB_ENDPOINT_XFER_INT)
@@ -434,8 +434,8 @@ static int uhci_sprint_schedule(struct uhci_hcd *uhci, char
*buf, int len)
tmp = tmp-next;
if (link != LINK_TO_TD(uhci, td)) {
if (nframes 0) {
- out += sprintf(out, link does
- not match list entry!\n);
+ out += sprintf(out,
+ link does not match list
entry!\n);
if (out - buf len)
goto done;
} else
@@ -460,8 +460,8 @@ check_link:
i, hc32_to_cpu(uhci, link));
j = 1;
}
- out += sprintf(out,link does not match
- QH (%08x)!\n,
+ out += sprintf(out,
+ link does not match QH (%08x)!\n,
hc32_to_cpu(uhci, qh_dma));
if (out - buf len)
goto done;
@@ -483,7 +483,7 @@ check_link:
int cnt = 0;
qh = uhci-skelqh[i];
- out += sprintf(out, - skel_%s_qh\n, qh_names[i]); \
+ out += sprintf(out, - skel_%s_qh\n, qh_names[i]);
out += uhci_show_qh(uhci, qh, out, len - (out - buf), 4);
if (out - buf len)
goto tail;
@@ -491,7 +491,8 @@ check_link:
/* Last QH is the Terminating QH, it's different */
if (i == SKEL_TERM) {
if (qh_element(qh) != LINK_TO_TD(uhci, uhci-term_td)) {
- out += sprintf(out, skel_term_qh element
is not set to term_td!\n);
+ out += sprintf(out,
+ skel_term_qh element is not set to
term_td!\n);
if (out - buf len)
goto done;
}
@@ -530,7 +531,8 @@ check_link:
link = LINK_TO_QH(uhci, uhci-skel_term_qh);
check_qh_link:
if (qh-link != link)
- out += sprintf(out, last QH not linked to next
skeleton!\n);
+ out += sprintf(out,
+ last QH not linked to next skeleton!\n);
if (out - buf len)
goto done;
@@ -587,7 +589,9 @@ static loff_t uhci_debug_lseek(struct file *file, loff_t
off, int whence)
up = file-private_data;
- /* XXX: atomic 64bit seek access, but that needs to be fixed in the VFS
*/
+ /*
+* XXX: atomic 64bit seek access, but that needs to be fixed in the VFS
+*/
switch (whence) {
case 0:
new = off;
diff --git a/drivers/usb/host/uhci-hcd.c b/drivers/usb/host/uhci-hcd.c
index 7c12b26..01628e3 100644
--- a/drivers/usb/host/uhci
Re: [PATCH 2/2] drivers/usb/host/uhci-*: beautify source code
于 2013年01月24日 00:34, Alan Stern 写道: Here you could just get rid of the second *** on each line. Or put the 7 ports max into parentheses and get rid of all the ***'s. Alan Stern thank you, I will send [PATCH 2/2 v2]. -- Chen Gang Asianux Corporation -- 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 v2] drivers/usb/host/uhci-*: beautify source code
get rid of the line breaks in string constants.
let comments within 80 with limitation.
delete ' \' at the end of a statement.
Signed-off-by: Chen Gang gang.c...@asianux.com
---
drivers/usb/host/uhci-debug.c | 28
drivers/usb/host/uhci-hcd.c | 27 +--
drivers/usb/host/uhci-hub.c |4 ++--
3 files changed, 31 insertions(+), 28 deletions(-)
diff --git a/drivers/usb/host/uhci-debug.c b/drivers/usb/host/uhci-debug.c
index 8a55bb2..4557375 100644
--- a/drivers/usb/host/uhci-debug.c
+++ b/drivers/usb/host/uhci-debug.c
@@ -151,8 +151,8 @@ static int uhci_show_urbp(struct uhci_hcd *uhci,
struct urb_priv *urbp,
}
}
if (nactive + ninactive 0)
- out += sprintf(out, %*s[skipped %d inactive and %d active
- TDs]\n,
+ out += sprintf(out,
+ %*s[skipped %d inactive and %d active TDs]\n,
space, , ninactive, nactive);
done:
if (out - buf len)
@@ -182,8 +182,8 @@ static int uhci_show_qh(struct uhci_hcd *uhci,
hc32_to_cpu(uhci, qh-link),
hc32_to_cpu(uhci, element));
if (qh-type == USB_ENDPOINT_XFER_ISOC)
- out += sprintf(out, %*speriod %d phase %d load %d us,
- frame %x desc [%p]\n,
+ out += sprintf(out,
+ %*speriod %d phase %d load %d us, frame %x
desc [%p]\n,
space, , qh-period, qh-phase, qh-load,
qh-iso_frame, qh-iso_packet_desc);
else if (qh-type == USB_ENDPOINT_XFER_INT)
@@ -434,8 +434,8 @@ static int uhci_sprint_schedule(struct uhci_hcd
*uhci, char *buf, int len)
tmp = tmp-next;
if (link != LINK_TO_TD(uhci, td)) {
if (nframes 0) {
- out += sprintf(out, link does
- not match list entry!\n);
+ out += sprintf(out,
+ link does not match list
entry!\n);
if (out - buf len)
goto done;
} else
@@ -460,8 +460,8 @@ check_link:
i, hc32_to_cpu(uhci, link));
j = 1;
}
- out += sprintf(out,link does not match
- QH (%08x)!\n,
+ out += sprintf(out,
+ link does not match QH (%08x)!\n,
hc32_to_cpu(uhci, qh_dma));
if (out - buf len)
goto done;
@@ -483,7 +483,7 @@ check_link:
int cnt = 0;
qh = uhci-skelqh[i];
- out += sprintf(out, - skel_%s_qh\n, qh_names[i]); \
+ out += sprintf(out, - skel_%s_qh\n, qh_names[i]);
out += uhci_show_qh(uhci, qh, out, len - (out - buf), 4);
if (out - buf len)
goto tail;
@@ -491,7 +491,8 @@ check_link:
/* Last QH is the Terminating QH, it's different */
if (i == SKEL_TERM) {
if (qh_element(qh) != LINK_TO_TD(uhci, uhci-term_td)) {
- out += sprintf(out, skel_term_qh element
is not set to
term_td!\n);
+ out += sprintf(out,
+ skel_term_qh element is not set to
term_td!\n);
if (out - buf len)
goto done;
}
@@ -530,7 +531,8 @@ check_link:
link = LINK_TO_QH(uhci, uhci-skel_term_qh);
check_qh_link:
if (qh-link != link)
- out += sprintf(out, last QH not linked to next
skeleton!\n);
+ out += sprintf(out,
+ last QH not linked to next skeleton!\n);
if (out - buf len)
goto done;
@@ -587,7 +589,9 @@ static loff_t uhci_debug_lseek(struct file *file,
loff_t off, int whence)
up = file-private_data;
- /* XXX: atomic 64bit seek access, but that needs to be fixed in the VFS
*/
+ /*
+* XXX: atomic 64bit seek access, but that needs to be fixed in the VFS
+*/
switch (whence) {
case 0:
new = off;
diff --git a/drivers/usb/host/uhci-hcd.c b/drivers/usb/host/uhci-hcd.c
index 7c12b26..01628e3 100644
--- a/drivers/usb/host/uhci-hcd.c
+++ b
Re: [PATCH] drivers/usb/gadget: using strlcpy instead of strncpy
于 2013年01月23日 09:00, Laurent Pinchart 写道: Thank you for the patch. I've applied it to my tree. thanks. -- Chen Gang Asianux Corporation -- 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] drivers/usb/gadget: using strlcpy instead of strncpy
for NUL terminated string, better notice '\0' in the end. Signed-off-by: Chen Gang gang.c...@asianux.com --- drivers/usb/gadget/uvc_v4l2.c |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/usb/gadget/uvc_v4l2.c b/drivers/usb/gadget/uvc_v4l2.c index 2ca9386..a9934c7 100644 --- a/drivers/usb/gadget/uvc_v4l2.c +++ b/drivers/usb/gadget/uvc_v4l2.c @@ -178,9 +178,9 @@ uvc_v4l2_do_ioctl(struct file *file, unsigned int cmd, void *arg) struct v4l2_capability *cap = arg; memset(cap, 0, sizeof *cap); - strncpy(cap-driver, g_uvc, sizeof(cap-driver)); - strncpy(cap-card, cdev-gadget-name, sizeof(cap-card)); - strncpy(cap-bus_info, dev_name(cdev-gadget-dev), + strlcpy(cap-driver, g_uvc, sizeof(cap-driver)); + strlcpy(cap-card, cdev-gadget-name, sizeof(cap-card)); + strlcpy(cap-bus_info, dev_name(cdev-gadget-dev), sizeof cap-bus_info); cap-version = DRIVER_VERSION_NUMBER; cap-capabilities = V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_STREAMING; -- 1.7.10.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 v2] drivers/usb/host/uhci-* : check buffer length to avoid memory overflow
于 2013年01月22日 00:17, Alan Stern 写道: This needs to be broken up into two distinct patches: One to fix the buffer-overflow problem; One to get rid of the line breaks in string constants. Two totally separate goals like these should not be combined into a single patch... ok, thank you, I will send relative patches, please wait within 2 days. ...Also, why did you change the comment in uhci_debug_lseek? the original comments exceeds 80 bondary (including '*/'). but exculding '*/', the comments are within 80 bondary. for me, I think it is better to seperate it into multiple lines. if you do not think so, please tell me, I will restore, thanks. and also thank you for your details checking. -- Chen Gang Asianux Corporation -- 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] drivers/usb/host/uhci-* : check buffer length to avoid memory overflow
Hello Alan Stern When you have time, could you help checking this patch ? welcome any reasonable rejections, too. thanks. gchen. 于 2013年01月19日 07:37, Greg KH 写道: On Sat, Jan 12, 2013 at 11:18:03PM +0800, Chen Gang wrote: for function uhci_sprint_schedule: the buffer len is MAX_OUTPUT: 64 * 1024, which may not be enough: may loop UHCI_NUMFRAMES times (UHCI_NUMFRAMES is 1024) each time of loop may get more than 64 bytes so need check the buffer length to avoid memory overflow this patch fix it like this: at first, make enough room for buffering the exceeding contents judge the contents which written whether bigger than buffer length if bigger (the exceeding contents will be in the exceeding buffer) break current work flow, and return. also let the const string contents not seperated in second line. Signed-off-by: Chen Gang gang.c...@asianux.com --- drivers/usb/host/uhci-debug.c | 178 +++-- drivers/usb/host/uhci-hcd.c | 31 --- drivers/usb/host/uhci-q.c |2 +- 3 files changed, 136 insertions(+), 75 deletions(-) Alan, any objections to me taking this patch? thanks, greg k-h -- Chen Gang Asianux Corporation -- 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] drivers/usb/host/uhci-* : check buffer length to avoid memory overflow
for function uhci_sprint_schedule:
the buffer len is MAX_OUTPUT: 64 * 1024, which may not be enough:
may loop UHCI_NUMFRAMES times (UHCI_NUMFRAMES is 1024)
each time of loop may get more than 64 bytes
so need check the buffer length to avoid memory overflow
this patch fix it like this:
at first, make enough room for buffering the exceeding contents
judge the contents which written whether bigger than buffer length
if bigger (the exceeding contents will be in the exceeding buffer)
break current work flow, and return.
also let the const string contents not seperated in second line.
Signed-off-by: Chen Gang gang.c...@asianux.com
---
drivers/usb/host/uhci-debug.c | 178 +++--
drivers/usb/host/uhci-hcd.c | 31 ---
drivers/usb/host/uhci-q.c |2 +-
3 files changed, 136 insertions(+), 75 deletions(-)
diff --git a/drivers/usb/host/uhci-debug.c b/drivers/usb/host/uhci-debug.c
index fc0b0da..4557375 100644
--- a/drivers/usb/host/uhci-debug.c
+++ b/drivers/usb/host/uhci-debug.c
@@ -16,6 +16,8 @@
#include uhci-hcd.h
+#define EXTRA_SPACE1024
+
static struct dentry *uhci_debugfs_root;
#ifdef DEBUG
@@ -44,10 +46,6 @@ static int uhci_show_td(struct uhci_hcd *uhci, struct
uhci_td *td, char *buf,
char *spid;
u32 status, token;
- /* Try to make sure there's enough memory */
- if (len 160)
- return 0;
-
status = td_status(uhci, td);
out += sprintf(out, %*s[%p] link (%08x) , space, , td,
hc32_to_cpu(uhci, td-link));
@@ -64,6 +62,8 @@ static int uhci_show_td(struct uhci_hcd *uhci, struct uhci_td
*td, char *buf,
(status TD_CTRL_CRCTIMEO) ? CRC/Timeo : ,
(status TD_CTRL_BITSTUFF) ? BitStuff : ,
status 0x7ff);
+ if (out - buf len)
+ goto done;
token = td_token(uhci, td);
switch (uhci_packetid(token)) {
@@ -90,6 +90,9 @@ static int uhci_show_td(struct uhci_hcd *uhci, struct uhci_td
*td, char *buf,
spid);
out += sprintf(out, (buf=%08x)\n, hc32_to_cpu(uhci, td-buffer));
+done:
+ if (out - buf len)
+ out += sprintf(out, ...\n);
return out - buf;
}
@@ -101,8 +104,6 @@ static int uhci_show_urbp(struct uhci_hcd *uhci, struct
urb_priv *urbp,
int i, nactive, ninactive;
char *ptype;
- if (len 200)
- return 0;
out += sprintf(out, urb_priv [%p] , urbp);
out += sprintf(out, urb [%p] , urbp-urb);
@@ -110,6 +111,8 @@ static int uhci_show_urbp(struct uhci_hcd *uhci, struct
urb_priv *urbp,
out += sprintf(out, Dev=%d , usb_pipedevice(urbp-urb-pipe));
out += sprintf(out, EP=%x(%s) , usb_pipeendpoint(urbp-urb-pipe),
(usb_pipein(urbp-urb-pipe) ? IN : OUT));
+ if (out - buf len)
+ goto done;
switch (usb_pipetype(urbp-urb-pipe)) {
case PIPE_ISOCHRONOUS: ptype = ISO; break;
@@ -128,6 +131,9 @@ static int uhci_show_urbp(struct uhci_hcd *uhci, struct
urb_priv *urbp,
out += sprintf(out, Unlinked=%d, urbp-urb-unlinked);
out += sprintf(out, \n);
+ if (out - buf len)
+ goto done;
+
i = nactive = ninactive = 0;
list_for_each_entry(td, urbp-td_list, list) {
if (urbp-qh-type != USB_ENDPOINT_XFER_ISOC
@@ -135,6 +141,8 @@ static int uhci_show_urbp(struct uhci_hcd *uhci, struct
urb_priv *urbp,
out += sprintf(out, %*s%d: , space + 2, , i);
out += uhci_show_td(uhci, td, out,
len - (out - buf), 0);
+ if (out - buf len)
+ goto tail;
} else {
if (td_status(uhci, td) TD_CTRL_ACTIVE)
++nactive;
@@ -143,10 +151,13 @@ static int uhci_show_urbp(struct uhci_hcd *uhci, struct
urb_priv *urbp,
}
}
if (nactive + ninactive 0)
- out += sprintf(out, %*s[skipped %d inactive and %d active
- TDs]\n,
+ out += sprintf(out,
+ %*s[skipped %d inactive and %d active TDs]\n,
space, , ninactive, nactive);
-
+done:
+ if (out - buf len)
+ out += sprintf(out, ...\n);
+tail:
return out - buf;
}
@@ -158,10 +169,6 @@ static int uhci_show_qh(struct uhci_hcd *uhci,
__hc32 element = qh_element(qh);
char *qtype;
- /* Try to make sure there's enough memory */
- if (len 80 * 7)
- return 0;
-
switch (qh-type) {
case USB_ENDPOINT_XFER_ISOC: qtype = ISO; break;
case USB_ENDPOINT_XFER_INT: qtype = INT; break;
@@ -175,13 +182,15 @@ static int uhci_show_qh(struct uhci_hcd *uhci
Re: [PATCH] drivers/usb/host/uhci-* : check buffer length to avoid memory overflow
于 2013年01月09日 04:53, Alan Stern 写道: Gang: I apologize for taking so long to respond to your patch. I didn't get much work done during the holidays... I understand. it is necessary to have a rest, so can keep our contributes (work), with efficiency and sustainable. :-) Can you rewrite this using ordinary English sentences? Describe what you changed and why you changed it. You don't have to include the details of your tests. ok, thanks. +#define LEFT_OUTPUT (1024) A better name would be EXTRA_SPACE. ok, thanks. @@ -90,7 +90,11 @@ static int uhci_show_td(struct uhci_hcd *uhci, struct uhci_td *td, char *buf, spid); out += sprintf(out, (buf=%08x)\n, hc32_to_cpu(uhci, td-buffer)); +tail: return out - buf; +truncate: +out += sprintf(out, (%s truncated)\n, __FUNCTION__); +goto tail; } Here and in all the other functions, it would be better to do this: + done: + if (out - buf len) + out += sprintf(out, ...\n); return out - buf; } ok, thanks. You don't need to write the word truncated all the time. A simple ... will let people know what happened. Have all the goto statements jump to done. The reason for doing it this way is... ok, thanks. @@ -135,6 +142,8 @@ static int uhci_show_urbp(struct uhci_hcd *uhci, struct urb_priv *urbp, out += sprintf(out, %*s%d: , space + 2, , i); out += uhci_show_td(uhci, td, out, len - (out - buf), 0); +if (out - buf len) +goto truncate; .. here you can jump directly to the return statement and avoid printing out a duplicate ... line. ok, thanks. I think your meaning is: use goto tail; instead of goto truncate; and the return area like this: + done: + if (out - buf len) + out += sprintf(out, ...\n); + tail: return out - buf; } if I misunderstanding, please reply. (no reply means I am not misunderstanding) @@ -375,14 +402,19 @@ static int uhci_sprint_schedule(struct uhci_hcd *uhci, char *buf, int len) uhci_to_hcd(uhci)-self.bandwidth_int_reqs, uhci_to_hcd(uhci)-self.bandwidth_isoc_reqs); if (debug = 1) -return out - buf; +goto tail; out += sprintf(out, Frame List\n); +if (out - buf len) +goto truncate; This test isn't needed because there's another test inside the loop below. ok, thanks. I will send patch v2 within 4 days (execuse me, today and tommorrow, I have to do another things) -- Chen Gang Asianux Corporation -- 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] drivers/usb/host/uhci-* : check buffer length to avoid memory overflow
reason (why):
for function uhci_sprint_schedule:
the buffer len is MAX_OUTPUT: 64 * 1024
the buffer may not be enough:
may loop UHCI_NUMFRAMES times (UHCI_NUMFRAMES is 1024)
each time of loop may get more than 64 bytes
so need check the buffer length to avoid memory overflow
goal (what):
this patch fixes this correctness issue.
design (and why):
at first, we make enough room for buffering the exceeding contents
judge the contents which we have written whether bigger than buffer length
if bigger (the exceeding contents will be in the exceeding buffer)
break current work flow, and return
test:
plan:
let MAX_OUTPUT as various values:
some values which are enough for use, then can get full contents.
some values which small enough, so can truncate in various locations.
check the result:
cat the contents from the /sys/kernel/debug/usb/uhci/*
use wc -c to get the count of output contents (match the MAX_OUTPUT)
result:
make the buffer size in different size: (1024 is the room for exceeding)
63 * 1024 + 1024
1st (debug == 3) pass
2nd (debug == 3) pass
3rd (debug == 1) pass
4rd (not define DEBUG) pass
1689 + 1024 (debug == 3) pass (1689 is full contents length of my test)
1688 + 1024 (debug == 3) pass
1024 + 1024 (debug == 3) pass
512 + 1024 (debug == 3) pass
30 + 1024 (debug == 3) pass
left:
not test it by calling from uhci-q.c and uhci-hcd.c
not test the dump objects which already have rich data contents
if testing them are necessary:
please tell me, I will try.
and it is better to tell me how to test them.
Signed-off-by: Chen Gang gang.c...@asianux.com
---
drivers/usb/host/uhci-debug.c | 140 +
drivers/usb/host/uhci-hcd.c |4 +-
drivers/usb/host/uhci-q.c |2 +-
3 files changed, 102 insertions(+), 44 deletions(-)
diff --git a/drivers/usb/host/uhci-debug.c b/drivers/usb/host/uhci-debug.c
index fc0b0da..4951a1c 100644
--- a/drivers/usb/host/uhci-debug.c
+++ b/drivers/usb/host/uhci-debug.c
@@ -16,6 +16,8 @@
#include uhci-hcd.h
+#define LEFT_OUTPUT(1024)
+
static struct dentry *uhci_debugfs_root;
#ifdef DEBUG
@@ -44,10 +46,6 @@ static int uhci_show_td(struct uhci_hcd *uhci, struct
uhci_td *td, char *buf,
char *spid;
u32 status, token;
- /* Try to make sure there's enough memory */
- if (len 160)
- return 0;
-
status = td_status(uhci, td);
out += sprintf(out, %*s[%p] link (%08x) , space, , td,
hc32_to_cpu(uhci, td-link));
@@ -64,6 +62,8 @@ static int uhci_show_td(struct uhci_hcd *uhci, struct uhci_td
*td, char *buf,
(status TD_CTRL_CRCTIMEO) ? CRC/Timeo : ,
(status TD_CTRL_BITSTUFF) ? BitStuff : ,
status 0x7ff);
+ if (out - buf len)
+ goto truncate;
token = td_token(uhci, td);
switch (uhci_packetid(token)) {
@@ -90,7 +90,11 @@ static int uhci_show_td(struct uhci_hcd *uhci, struct
uhci_td *td, char *buf,
spid);
out += sprintf(out, (buf=%08x)\n, hc32_to_cpu(uhci, td-buffer));
+tail:
return out - buf;
+truncate:
+ out += sprintf(out, (%s truncated)\n, __FUNCTION__);
+ goto tail;
}
static int uhci_show_urbp(struct uhci_hcd *uhci, struct urb_priv *urbp,
@@ -101,8 +105,6 @@ static int uhci_show_urbp(struct uhci_hcd *uhci, struct
urb_priv *urbp,
int i, nactive, ninactive;
char *ptype;
- if (len 200)
- return 0;
out += sprintf(out, urb_priv [%p] , urbp);
out += sprintf(out, urb [%p] , urbp-urb);
@@ -110,6 +112,8 @@ static int uhci_show_urbp(struct uhci_hcd *uhci, struct
urb_priv *urbp,
out += sprintf(out, Dev=%d , usb_pipedevice(urbp-urb-pipe));
out += sprintf(out, EP=%x(%s) , usb_pipeendpoint(urbp-urb-pipe),
(usb_pipein(urbp-urb-pipe) ? IN : OUT));
+ if (out - buf len)
+ goto truncate;
switch (usb_pipetype(urbp-urb-pipe)) {
case PIPE_ISOCHRONOUS: ptype = ISO; break;
@@ -128,6 +132,9 @@ static int uhci_show_urbp(struct uhci_hcd *uhci, struct
urb_priv *urbp,
out += sprintf(out, Unlinked=%d, urbp-urb-unlinked);
out += sprintf(out, \n);
+ if (out - buf len)
+ goto truncate;
+
i = nactive = ninactive = 0;
list_for_each_entry(td, urbp-td_list, list) {
if (urbp-qh-type != USB_ENDPOINT_XFER_ISOC
@@ -135,6 +142,8 @@ static int uhci_show_urbp(struct uhci_hcd *uhci, struct
urb_priv *urbp,
out += sprintf(out, %*s%d: , space + 2, , i);
out += uhci_show_td(uhci, td, out,
len - (out - buf), 0
[PATCH] drivers/usb/host/ohci* : set urb-hcpriv = NULL immediately, after free it
although we can not say it is surely a bug.
it is better to set urb-hcpriv = NULL, after finish calling urb_free_priv.
before kfree urb_priv, better to judge whether urb_priv == NULL, firstly.
Signed-off-by: Chen Gang gang.c...@asianux.com
---
drivers/usb/host/ohci-q.c |7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/ohci-q.c b/drivers/usb/host/ohci-q.c
index 7482cfb..6671107 100644
--- a/drivers/usb/host/ohci-q.c
+++ b/drivers/usb/host/ohci-q.c
@@ -12,8 +12,12 @@
static void urb_free_priv (struct ohci_hcd *hc, urb_priv_t *urb_priv)
{
- int last = urb_priv-length - 1;
+ int last;
+ if (!urb_priv)
+ return;
+
+ last = urb_priv-length - 1;
if (last = 0) {
int i;
struct td *td;
@@ -44,6 +48,7 @@ __acquires(ohci-lock)
// ASSERT (urb-hcpriv != 0);
urb_free_priv (ohci, urb-hcpriv);
+ urb-hcpriv = NULL;
if (likely(status == -EINPROGRESS))
status = 0;
--
1.7.10.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] drivers/usb/host/ohci* : set urb-hcpriv = NULL immediately, after free it
于 2012年12月18日 23:17, Alan Stern 写道:
static void urb_free_priv (struct ohci_hcd *hc, urb_priv_t *urb_priv)
{
- int last = urb_priv-length - 1;
+ int last;
+ if (!urb_priv)
+ return;
+
+ last = urb_priv-length - 1;
Please don't do this. If urb_priv is NULL, that's a bug. We want it
to cause a visible error, not silently fail.
good idea ! (for private use, it is a good idea)
I'll send patch v2
--
Chen Gang
Asianux Corporation
--
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] drivers/usb/host/ohci* : set urb-hcpriv = NULL immediately, after free it
although we can not say it is surely a bug. it is better to set urb-hcpriv = NULL, after finish calling urb_free_priv. Signed-off-by: Chen Gang gang.c...@asianux.com --- drivers/usb/host/ohci-q.c |1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/host/ohci-q.c b/drivers/usb/host/ohci-q.c index 7482cfb..88731b7 100644 --- a/drivers/usb/host/ohci-q.c +++ b/drivers/usb/host/ohci-q.c @@ -44,6 +44,7 @@ __acquires(ohci-lock) // ASSERT (urb-hcpriv != 0); urb_free_priv (ohci, urb-hcpriv); + urb-hcpriv = NULL; if (likely(status == -EINPROGRESS)) status = 0; -- 1.7.10.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: [Suggestion] drivers/usb/host/uhci* : sprintf, need check len when use buf
Hello Alan Stern I finished constructing envrionments. let uhci-debug.c has effect (#define DEBUG, debug = 3 in drivers/usb/host/uhci-hcd.c) build kernel and install, and restart machine. can cat /sys/kernel/debug/usb/uhci/* to get full display contents for fixing this issue of uhci-debug.c, design: A) I will let all are according to usb_device_dump in drivers/usb/core/devices.c it has effect for all sub call functions (uhci_show_td, uhci_show_urbp...). for originally, length judging of uhci_show_urbp seems need improvement. it needs line = 200 (line 104) it call uhci_show_td in a loop (line 132..144) uhci_show_td nees line = 160 ! so I prefer to touch all sub functions. if you have another suggestions, please tell me. B) I will not touch original looping within 10 times (if 10 will be skipped) in function uhci_show_urbp line 132..148 in function uhci_show_qh line 213..222 in function uhci_sprint_schedule line 460..470, for nframes line 381..433. if you think it is necessary to remove them, please tell me. test: A) let MAX_OUTPUT as various values: one value is enough for use, then can get full contents. test 3 various values which small enough to let output truncate in various location. B) check the result: I will cat the contents from the /sys/kernel/debug/usb/uhci/* use wc -c to get the count of output contents, then judge whether match the MAX_OUTPUT. if you have additional completions, please tell me. I will start the implementation when get your reply. Regards. -- Chen Gang Asianux Corporation -- 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: [Suggestion] drivers/usb/host/ohci* : not setting urb-hcpriv = NULL after kfree it.
于 2012年12月17日 23:27, Alan Stern 写道: On Mon, 17 Dec 2012, Chen Gang wrote: 于 2012年12月17日 11:08, Alan Stern 写道: It is pretty much as I explained in my previous email. finish_urb calls usb_free_priv while holding the lock. Then while still holding the lock, it calls usb_hcd_unlink_urb_from_ep. In addition, ohci_urb_dequeue calls usb_hcd_check_unlink_urb while holding the lock, and does nothing if the return value is nonzero. So all you need to do is verify that after usb_hcd_unlink_urb_from_ep runs, usb_hcd_check_unlink_urb will always return a nonzero value. In fact, it will return -EIDRM -- until the next time the URB is submitted and usb_hcd_link_urb_to_ep is called. it is true for ohci_urb_dequeue. how about finish_unlinks and takeback_td in drives/usb/host/ohci-q.c ? (they also can call finish_urb). Those routines, like almost all the rest of the driver, look at TDs that haven't been processed yet and URBs that haven't been completed yet. Once a TD is processed, it is freed. Since finish_urb isn't called until all the TDs in an URB have been processed, urb-hcpriv will be a valid pointer for any URB encountered. The only places where this isn't true is while an URB is being submitted. The submission routines are careful to free all the private data structures if the URB is not accepted. Alan Stern thank you for your reply in details. :-) next: I will write the 2 patches one for current suggestion. the other for sprintf suggestion. time region: tomorrow, I begin constructing relative environments. I should try to finish 2 patches within this week (before Mon Dec 24 2012) excuse me: I can not give a full test, but I need provide enough unit test, at least. after finish 2 patches, I will turn back for the details of your reply, again. welcome any additional suggestions thanks. -- Chen Gang Asianux Corporation -- 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: [Suggestion] drivers/usb/host/ohci* : not setting urb-hcpriv = NULL after kfree it.
于 2012年12月17日 23:27, Alan Stern 写道: On Mon, 17 Dec 2012, Chen Gang wrote: 于 2012年12月17日 11:08, Alan Stern 写道: It is pretty much as I explained in my previous email. finish_urb calls usb_free_priv while holding the lock. Then while still holding the lock, it calls usb_hcd_unlink_urb_from_ep. In addition, ohci_urb_dequeue calls usb_hcd_check_unlink_urb while holding the lock, and does nothing if the return value is nonzero. So all you need to do is verify that after usb_hcd_unlink_urb_from_ep runs, usb_hcd_check_unlink_urb will always return a nonzero value. In fact, it will return -EIDRM -- until the next time the URB is submitted and usb_hcd_link_urb_to_ep is called. it is true for ohci_urb_dequeue. how about finish_unlinks and takeback_td in drives/usb/host/ohci-q.c ? (they also can call finish_urb). Those routines, like almost all the rest of the driver, look at TDs that haven't been processed yet and URBs that haven't been completed yet. Once a TD is processed, it is freed. Since finish_urb isn't called until all the TDs in an URB have been processed, urb-hcpriv will be a valid pointer for any URB encountered. The only places where this isn't true is while an URB is being submitted. The submission routines are careful to free all the private data structures if the URB is not accepted. Alan Stern thank you for your reply in details. :-) next: I will write the 2 patches one for current suggestion. the other for sprintf suggestion. time region: tomorrow, I begin constructing relative environments. I should try to finish 2 patches within this week (before Mon Dec 24 2012) excuse me: I can not give a full test, but I need provide enough unit test, at least. after finish 2 patches, I will turn back for the details of your reply, again. welcome any additional suggestions thanks. -- Chen Gang Asianux Corporation -- 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: [Suggestion] drivers/usb/host/uhci* : sprintf, need check len when use buf
于 2012年12月16日 00:58, Alan Stern 写道: In my opinion this problem is not serious enough to spend much time on, and I have lots of other, more important things, to do. If you want to fix it, go ahead and send in a patch. Alan Stern ok, I will send the relative patch (although I do not think it is a good idea). excuse me, please wait some days (at least, I need construct the relative environments for testing). thanks. -- Chen Gang Asianux Corporation -- 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: [Suggestion] drivers/usb/host/ohci* : not setting urb-hcpriv = NULL after kfree it.
于 2012年12月16日 01:04, Alan Stern 写道: On Sat, 15 Dec 2012, Chen Gang wrote: Hello Alan Stern: in drivers/usb/host/ohci-q.c, function finish_urb: when we finish call urb_free_priv, we not set urb-hcpriv = NULL (line 46) urb_free_priv call kfree for urb_priv (which is urb-hcpriv, line 29) within finish_urb, we not set urb-hcpriv = NULL (line 43..81) in drivers/usb/host/ohci-hdc.c, function ohci_urb_dequeue: it judges urb-hcpriv == NULL to decide whether call finish_urb (line 317..322) within ohci_urb_dequeue, we not set urb-hcpriv = NULL (line 321..326) another functions (finish_unlinks, takeback_td...), can call finish_urb, too. they do not set urb-hcpriv = NULL, either. they do not judge urb-hcpriv == NULL before calling finish_urb. although I can not say it is surely a bug. It isn't. ohci_urb_dequeue calls usb_hcd_check_unlink_urb before looking at urb-hcpriv. If urb_free_priv has been called already then usb_hcd_check_unlink_urb will return a non-zero value. This is because finish_urb calls usb_hcd_unlink_urb_from_ep. for ohci_urb_dequeue, it is surely as what you said above. can you be sure that another (finish_unlinks, takeback_td...) also consider it ? it seems we have already considered, but I am not quite sure (since we already agree to sending a patch, it is not necessary to reply). Also, don't forget that the first think usb_hcd_giveback_urb does is set urb-hcpriv to NULL. in finish_urb: when call usb_hcd_giveback_urb, need unlock firstly (lock again, after finish). kfree urb-hcpriv in lock protected, but set it NULL out of lock protected. (at least, it seems not a good idea) (since we already agree to sending a patch, it is not necessary to reply). I still suggest: in finish_urb, set urb-hcpriv = NULL, after finish calling urb_free_priv. in urb_free_priv, better to judge whether urb_priv == NULL, firstly. If you want to send in a patch to do this, go ahead. ok, I will send (although it may be not a bug, better to do it) excuse me, please wait for some days (I should construct environments for test it). thanks. Alan Stern -- Chen Gang Asianux Corporation -- 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: [Suggestion] drivers/usb/host/ohci* : not setting urb-hcpriv = NULL after kfree it.
于 2012年12月17日 09:37, Chen Gang 写道: 于 2012年12月16日 01:04, Alan Stern 写道: Also, don't forget that the first think usb_hcd_giveback_urb does is set urb-hcpriv to NULL. in finish_urb: when call usb_hcd_giveback_urb, need unlock firstly (lock again, after finish). kfree urb-hcpriv in lock protected, but set it NULL out of lock protected. (at least, it seems not a good idea) (since we already agree to sending a patch, it is not necessary to reply). oh, sorry, I forgot 2 things: I originally said not find set urb-hcpriv to NULL within finish_urb. it is incorrect (as your reply: usb_hcd_giveback_urb does in finish_urb) it is my fault. :-) are all kfree for urb-hcpriv in lock protected ? (set urb-hcpriv = NULL in usb_hcd_giveback_urb, not in lock protected). can you be sure that it is no synchronization issue ? if you can be sure would you please explain to me (I am learning). else I will check it in details to make sure whether it can cause issue, or not. thanks. -- Chen Gang Asianux Corporation -- 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: [Suggestion] drivers/usb/host/ohci* : not setting urb-hcpriv = NULL after kfree it.
于 2012年12月17日 11:08, Alan Stern 写道: It is pretty much as I explained in my previous email. finish_urb calls usb_free_priv while holding the lock. Then while still holding the lock, it calls usb_hcd_unlink_urb_from_ep. In addition, ohci_urb_dequeue calls usb_hcd_check_unlink_urb while holding the lock, and does nothing if the return value is nonzero. So all you need to do is verify that after usb_hcd_unlink_urb_from_ep runs, usb_hcd_check_unlink_urb will always return a nonzero value. In fact, it will return -EIDRM -- until the next time the URB is submitted and usb_hcd_link_urb_to_ep is called. it is true for ohci_urb_dequeue. how about finish_unlinks and takeback_td in drives/usb/host/ohci-q.c ? (they also can call finish_urb). -- Chen Gang Asianux Corporation -- 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
[Suggestion] drivers/usb/host/ohci* : not setting urb-hcpriv = NULL after kfree it.
Hello Alan Stern:
in drivers/usb/host/ohci-q.c, function finish_urb:
when we finish call urb_free_priv, we not set urb-hcpriv = NULL (line 46)
urb_free_priv call kfree for urb_priv (which is urb-hcpriv, line 29)
within finish_urb, we not set urb-hcpriv = NULL (line 43..81)
in drivers/usb/host/ohci-hdc.c, function ohci_urb_dequeue:
it judges urb-hcpriv == NULL to decide whether call finish_urb (line
317..322)
within ohci_urb_dequeue, we not set urb-hcpriv = NULL (line 321..326)
another functions (finish_unlinks, takeback_td...), can call finish_urb,
too.
they do not set urb-hcpriv = NULL, either.
they do not judge urb-hcpriv == NULL before calling finish_urb.
although I can not say it is surely a bug.
I still suggest:
in finish_urb, set urb-hcpriv = NULL, after finish calling urb_free_priv.
in urb_free_priv, better to judge whether urb_priv == NULL, firstly.
thanks.
gchen.
drivers/usb/host/ohci-q.c:
13 static void urb_free_priv (struct ohci_hcd *hc, urb_priv_t *urb_priv)
14 {
15 int last = urb_priv-length - 1;
16
17 if (last = 0) {
18 int i;
19 struct td *td;
20
21 for (i = 0; i = last; i++) {
22 td = urb_priv-td [i];
23 if (td)
24 td_free (hc, td);
25 }
26 }
27
28 list_del (urb_priv-pending);
29 kfree (urb_priv);
30 }
31
32
/*-*/
33
34 /*
35 * URB goes back to driver, and isn't reissued.
36 * It's completely gone from HC data structures.
37 * PRECONDITION: ohci lock held, irqs blocked.
38 */
39 static void
40 finish_urb(struct ohci_hcd *ohci, struct urb *urb, int status)
41 __releases(ohci-lock)
42 __acquires(ohci-lock)
43 {
44 // ASSERT (urb-hcpriv != 0);
45
46 urb_free_priv (ohci, urb-hcpriv);
47 if (likely(status == -EINPROGRESS))
48 status = 0;
49
50 switch (usb_pipetype (urb-pipe)) {
51 case PIPE_ISOCHRONOUS:
52 ohci_to_hcd(ohci)-self.bandwidth_isoc_reqs--;
53 if (ohci_to_hcd(ohci)-self.bandwidth_isoc_reqs == 0) {
54 if (quirk_amdiso(ohci))
55 usb_amd_quirk_pll_enable();
56 if (quirk_amdprefetch(ohci))
57 sb800_prefetch(ohci, 0);
58 }
59 break;
60 case PIPE_INTERRUPT:
61 ohci_to_hcd(ohci)-self.bandwidth_int_reqs--;
62 break;
63 }
64
65 #ifdef OHCI_VERBOSE_DEBUG
66 urb_print(urb, RET, usb_pipeout (urb-pipe), status);
67 #endif
68
69 /* urb-complete() can reenter this HCD */
70 usb_hcd_unlink_urb_from_ep(ohci_to_hcd(ohci), urb);
71 spin_unlock (ohci-lock);
72 usb_hcd_giveback_urb(ohci_to_hcd(ohci), urb, status);
73 spin_lock (ohci-lock);
74
75 /* stop periodic dma if it's not needed */
76 if (ohci_to_hcd(ohci)-self.bandwidth_isoc_reqs == 0
77 ohci_to_hcd(ohci)-self.bandwidth_int_reqs ==
0) {
78 ohci-hc_control = ~(OHCI_CTRL_PLE|OHCI_CTRL_IE);
79 ohci_writel (ohci, ohci-hc_control, ohci-regs-control);
80 }
81 }
...
in drivers/usb/host/ohci-hcd.c
290 static int ohci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int
status)
291 {
292 struct ohci_hcd *ohci = hcd_to_ohci (hcd);
293 unsigned long flags;
294 int rc;
295
296 #ifdef OHCI_VERBOSE_DEBUG
297 urb_print(urb, UNLINK, 1, status);
298 #endif
299
300 spin_lock_irqsave (ohci-lock, flags);
301 rc = usb_hcd_check_unlink_urb(hcd, urb, status);
302 if (rc) {
303 ; /* Do nothing */
304 } else if (ohci-rh_state == OHCI_RH_RUNNING) {
305 urb_priv_t *urb_priv;
306
307 /* Unless an IRQ completed the unlink while it was being
308 * handed to us, flag it for unlink and giveback, and force
309 * some upcoming INTR_SF to call finish_unlinks()
310 */
311 urb_priv = urb-hcpriv;
312 if (urb_priv) {
313 if (urb_priv-ed-state == ED_OPER)
314 start_ed_unlink (ohci, urb_priv-ed);
315 }
316 } else {
317 /*
318 * with HC dead, we won't respect hc queue pointers
319 * any more ... just clean up every urb's memory.
320 */
321 if (urb-hcpriv)
[Suggestion] drivers/usb/host/uhci* : sprintf, need check len when use buf
Hello Alan Stern:
in drivers/usb/host/uhci-debug.c, for function uhci_sprint_schedule:
we are not check the len of buf (not like another static functions in this
file).
the buffer len is MAX_OUTPUT: 64 * 1024 (line 491, line 517)
the buffer may not be enough:
we may loop UHCI_NUMFRAMES times (line 383..433)
UHCI_NUMFRAMES is 1024 (drivers/usb/host/uhci-hcd.h:87)
each time of loop may get more than 64 bytes (line 383..433)
it seems we have noticed len checking when call uhci_show_td (line 411)
but we not check it outside of uhci_show_td (line 400..414)
so I suggest to add checking len when use buf (although it seems a little
complex)
I find it by code review, please help checking this suggestion, thanks.
if this suggestion is valid:
please help sending patch.
better to mark me as Reported-by.
not need cc to me (I am not reviewer)
Regards.
gchen.
drivers/usb/host/uhci-hcd.h:87:#define UHCI_NUMFRAMES 1024/* in
the frame list [array] */
drivers/usb/host/uhci-debug.c
347 static int uhci_sprint_schedule(struct uhci_hcd *uhci, char *buf, int len)
348 {
349 char *out = buf;
350 int i, j;
351 struct uhci_qh *qh;
352 struct uhci_td *td;
353 struct list_head *tmp, *head;
354 int nframes, nerrs;
355 __hc32 link;
356 __hc32 fsbr_link;
357
358 static const char * const qh_names[] = {
359 unlink, iso, int128, int64, int32, int16,
360 int8, int4, int2, async, term
361 };
362
363 out += uhci_show_root_hub_state(uhci, out, len - (out - buf));
364 out += sprintf(out, HC status\n);
365 out += uhci_show_status(uhci, out, len - (out - buf));
366
367 out += sprintf(out, Periodic load table\n);
368 for (i = 0; i MAX_PHASE; ++i) {
369 out += sprintf(out, \t%d, uhci-load[i]);
370 if (i % 8 == 7)
371 *out++ = '\n';
372 }
373 out += sprintf(out, Total: %d, #INT: %d, #ISO: %d\n,
374 uhci-total_load,
375 uhci_to_hcd(uhci)-self.bandwidth_int_reqs,
376 uhci_to_hcd(uhci)-self.bandwidth_isoc_reqs);
377 if (debug = 1)
378 return out - buf;
379
380 out += sprintf(out, Frame List\n);
381 nframes = 10;
382 nerrs = 0;
383 for (i = 0; i UHCI_NUMFRAMES; ++i) {
384 __hc32 qh_dma;
385
386 j = 0;
387 td = uhci-frame_cpu[i];
388 link = uhci-frame[i];
389 if (!td)
390 goto check_link;
391
392 if (nframes 0) {
393 out += sprintf(out, - Frame %d - (%08x)\n,
394 i, hc32_to_cpu(uhci, link));
395 j = 1;
396 }
397
398 head = td-fl_list;
399 tmp = head;
400 do {
401 td = list_entry(tmp, struct uhci_td, fl_list);
402 tmp = tmp-next;
403 if (link != LINK_TO_TD(uhci, td)) {
404 if (nframes 0)
405 out += sprintf(out, link does
406 not match list entry!\n);
407 else
408 ++nerrs;
409 }
410 if (nframes 0)
411 out += uhci_show_td(uhci, td, out,
412 len - (out - buf), 4);
413 link = td-link;
414 } while (tmp != head);
415
416 check_link:
417 qh_dma = uhci_frame_skel_link(uhci, i);
418 if (link != qh_dma) {
419 if (nframes 0) {
420 if (!j) {
421 out += sprintf(out,
422 - Frame %d - (%08x)\n,
423 i, hc32_to_cpu(uhci, link));
424 j = 1;
425 }
426 out += sprintf(out,link does not match
427 QH (%08x)!\n,
428 hc32_to_cpu(uhci, qh_dma));
429 } else
430 ++nerrs;
431 }
432 nframes -= j;
433 }
434 if (nerrs 0)
435 out += sprintf(out, Skipped %d bad links\n, nerrs);
436
437 out += sprintf(out, Skeleton QHs\n);
438
439 fsbr_link = 0;
440 for (i = 0; i UHCI_NUM_SKELQH; ++i) {
441
Re: [Suggestion] drivers/usb/host/uhci* : sprintf, need check len when use buf
于 2012年12月14日 23:46, Alan Stern 写道: The easiest fix is to increase MAX_OUTPUT. Can you figure out how large it needs to be? for me, it seems only increasing MAX_OUTPUT is not a good idea. maybe we can reference usb_device_dump: it processes the same thing in a simple way. it is in drivers/usb/core/devices.c function usb_device_dump calls function usb_dump_desc and function usb_dump_desc calls another dump functions. usb_device_dump also has to recuse call itself to process a tree. I'm not concerned with making this code absolutely 100% reliable. It is used only for debugging; in almost all kernel builds it will not be compiled. if it is useless: I suggest to delete it (which is better that fix it) else we need be sure that it is correct (or it can cause issue). I feel: we provide 20% can finish 80%. but the left 20%, need us to provide 80%. if we are focus on quality, we have to provide 80% for the left 20%. gchen 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 -- Chen Gang Asianux Corporation -- 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: [Suggestion] drivers/usb/renesas_usbhs: pkt is still in use, after it was already free.
于 2012年12月10日 08:21, Kuninori Morimoto 写道: Hi Chen Hello Greg Kroah-Hartman: in drivers/usb/renesas_usbhs/mod_host.c, in function usbhsh_queue_done: get ureq from pkt, by using the macro usbhsh_pkt_to_ureq (at line 637) pkt is the sub-object of ureq (line 73..76, line 157..158) free ureq, by calling function usbhsh_ureq_free (at line 655) use kfree to free ureq in function usbhsh_ureq_free (line 184..191) originally ureq is call kzalloc in function usbhsh_ureq_alloc (line 171..179) so pkt also free, too. still use pkt, by calling function usbhsh_endpoint_sequence_save (at line 657) use pkt-zero in funcion usbhsh_endpoint_sequence_save (at line 243) I find it through code review, please help to check this suggestion whether valid. if it was valid: I prefer the relative member to provide relative patch. if can not find relative member, I should try (although it seems not a good idea) Thank you for your review, and nice catch ! I think this is bug, and I didn't notice it. thank you for your reply. is it suitable to help me send patch ? I am not quite expert about it, and also no releative environments. if you will: please mark me as Reported-by in your patch. need not cc to me: I am not reviewer, so I am not suitable to review another maintainers patch else it is my duty to send the relative patch (although it seems not a good idea) thanks. gchen. Best regards --- Kuninori Morimoto -- Chen Gang Asianux Corporation -- 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: [Suggestion] drivers/usb/core: dev-config and dev-rawdescriptors, when processing failure.
ok, thanks. :-) gchen. 于 2012年12月07日 23:18, Alan Stern 写道: On Fri, 7 Dec 2012, Chen Gang wrote: but I still not quite be sure, please help checking (total 3 steps, below). thanks. -- Step 1: in drivers/usb/core/sysfs.c: for the same device, can usb_dev_authorized_store be called multi-times ? according to the function comments, it seems can be called multi-times. Yes, it can. Step 2: in drivers/usb/core/hub.c: usb_authorize_device may call usb_enumerate_device at line 2355 Yes. Step 3: also in drivers/usb/core/hub.c: if udev-config != NULL: we will assume that it has already configured, and will not call usb_get_configuration again. if first call for usb_get_confiuration failed, may udev-config will not be NULL. and next call usb_enumerate_device, we will misunderstand that it has already configured (all things are 0). If the first call to usb_get_configuration fails, there won't be any more calls to usb_enumerate_device. The device will be rejected by usb_new_device. Alan Stern -- Chen Gang Asianux Corporation -- 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: [Suggestion] drivers/usb/renesas_usbhs: pkt is still in use, after it was already free.
于 2012年12月07日 23:03, Greg KH 写道: On Fri, Dec 07, 2012 at 08:42:25PM +0800, Chen Gang wrote: Hello Greg Kroah-Hartman: in drivers/usb/renesas_usbhs/mod_host.c, in function usbhsh_queue_done: get ureq from pkt, by using the macro usbhsh_pkt_to_ureq (at line 637) pkt is the sub-object of ureq (line 73..76, line 157..158) free ureq, by calling function usbhsh_ureq_free (at line 655) use kfree to free ureq in function usbhsh_ureq_free (line 184..191) originally ureq is call kzalloc in function usbhsh_ureq_alloc (line 171..179) so pkt also free, too. still use pkt, by calling function usbhsh_endpoint_sequence_save (at line 657) use pkt-zero in funcion usbhsh_endpoint_sequence_save (at line 243) I find it through code review, please help to check this suggestion whether valid. I don't know, run the code to see if this is correct or not. Since no relative member reply, I should try (although I have no environments about renesas_usbhs), it is my duty. please wait for some days. thanks greg k-h -- Chen Gang Asianux Corporation -- 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
[Suggestion] drivers/usb/renesas_usbhs: pkt is still in use, after it was already free.
Hello Greg Kroah-Hartman:
in drivers/usb/renesas_usbhs/mod_host.c, in function usbhsh_queue_done:
get ureq from pkt, by using the macro usbhsh_pkt_to_ureq (at line 637)
pkt is the sub-object of ureq (line 73..76, line 157..158)
free ureq, by calling function usbhsh_ureq_free (at line 655)
use kfree to free ureq in function usbhsh_ureq_free (line 184..191)
originally ureq is call kzalloc in function usbhsh_ureq_alloc (line
171..179)
so pkt also free, too.
still use pkt, by calling function usbhsh_endpoint_sequence_save (at line
657)
use pkt-zero in funcion usbhsh_endpoint_sequence_save (at line 243)
I find it through code review, please help to check this suggestion whether
valid.
if it was valid:
I prefer the relative member to provide relative patch.
if can not find relative member, I should try (although it seems not a good
idea)
Regards
gchen.
73 struct usbhsh_request {
74 struct urb *urb;
75 struct usbhs_pktpkt;
76 };
77
...
157 #define usbhsh_pkt_to_ureq(p) \
158 container_of((void *)p, struct usbhsh_request, pkt)
...
163 static struct usbhsh_request *usbhsh_ureq_alloc(struct usbhsh_hpriv *hpriv,
164struct urb *urb,
165gfp_t mem_flags)
166 {
167 struct usbhsh_request *ureq;
168 struct usbhs_priv *priv = usbhsh_hpriv_to_priv(hpriv);
169 struct device *dev = usbhs_priv_to_dev(priv);
170
171 ureq = kzalloc(sizeof(struct usbhsh_request), mem_flags);
172 if (!ureq) {
173 dev_err(dev, ureq alloc fail\n);
174 return NULL;
175 }
176
177 usbhs_pkt_init(ureq-pkt);
178 ureq-urb = urb;
179 usbhsh_urb_to_ureq(urb) = ureq;
180
181 return ureq;
182 }
183
184 static void usbhsh_ureq_free(struct usbhsh_hpriv *hpriv,
185 struct usbhsh_request *ureq)
186 {
187 usbhsh_urb_to_ureq(ureq-urb) = NULL;
188 ureq-urb = NULL;
189
190 kfree(ureq);
191 }
...
211 static void usbhsh_endpoint_sequence_save(struct usbhsh_hpriv *hpriv,
212 struct urb *urb,
213 struct usbhs_pkt *pkt)
214 {
215 int len = urb-actual_length;
216 int maxp = usb_endpoint_maxp(urb-ep-desc);
217 int t = 0;
218
219 /* DCP is out of sequence control */
220 if (usb_pipecontrol(urb-pipe))
221 return;
222
223 /*
224 * renesas_usbhs pipe has a limitation in a number.
225 * So, driver should re-use the limited pipe for each
device/endpoint.
226 * DATA0/1 sequence should be saved for it.
227 * see [image of mod_host]
228 * [HARDWARE LIMITATION]
229 */
230
231 /*
232 * next sequence depends on actual_length
233 *
234 * ex) actual_length = 1147, maxp = 512
235 * data0 : 512
236 * data1 : 512
237 * data0 : 123
238 * data1 is the next sequence
239 */
240 t = len / maxp;
241 if (len % maxp)
242 t++;
243 if (pkt-zero)
244 t++;
245 t %= 2;
246
247 if (t)
248 usb_dotoggle(urb-dev,
249 usb_pipeendpoint(urb-pipe),
250 usb_pipeout(urb-pipe));
251 }
...
635 static void usbhsh_queue_done(struct usbhs_priv *priv, struct usbhs_pkt
*pkt)
636 {
637 struct usbhsh_request *ureq = usbhsh_pkt_to_ureq(pkt);
638 struct usbhsh_hpriv *hpriv = usbhsh_priv_to_hpriv(priv);
639 struct usb_hcd *hcd = usbhsh_hpriv_to_hcd(hpriv);
640 struct urb *urb = ureq-urb;
641 struct device *dev = usbhs_priv_to_dev(priv);
642 int status = 0;
643
644 dev_dbg(dev, %s\n, __func__);
645
646 if (!urb) {
647 dev_warn(dev, pkt doesn't have urb\n);
648 return;
649 }
650
651 if (!usbhsh_is_running(hpriv))
652 status = -ESHUTDOWN;
653
654 urb-actual_length = pkt-actual;
655 usbhsh_ureq_free(hpriv, ureq);
656
657 usbhsh_endpoint_sequence_save(hpriv, urb, pkt);
658 usbhsh_pipe_detach(hpriv, usbhsh_ep_to_uep(urb-ep));
659
660 usb_hcd_unlink_urb_from_ep(hcd, urb);
661 usb_hcd_giveback_urb(hcd, urb, status);
662 }
--
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
[Suggestion] drivers/usb/core: for u16, use stack allocation instead of kmalloc
Hello Greg Kroah-Hartman:
in drivers/usb/core/message.c:
at line 943, status is kmalloc ( sizeof u16 )
at line 952, assign the value of status to data
at line 953, free status.
it is better to let u16 status instead of u16 *status = kmalloc
thanks.
gchen.
940 int usb_get_status(struct usb_device *dev, int type, int target, void
*data)
941 {
942 int ret;
943 u16 *status = kmalloc(sizeof(*status), GFP_KERNEL);
944
945 if (!status)
946 return -ENOMEM;
947
948 ret = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
949 USB_REQ_GET_STATUS, USB_DIR_IN | type, 0, target, status,
950 sizeof(*status), USB_CTRL_GET_TIMEOUT);
951
952 *(u16 *)data = *status;
953 kfree(status);
954 return ret;
955 }
956 EXPORT_SYMBOL_GPL(usb_get_status);
--
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: [Suggestion] drivers/usb/core: for u16, use stack allocation instead of kmalloc
于 2012年12月06日 19:01, Peter Stuge 写道:
Chen Gang wrote:
it is better to let u16 status instead of u16 *status = kmalloc
.
940 int usb_get_status(struct usb_device *dev, int type, int target, void
*data)
941 {
942 int ret;
943 u16 *status = kmalloc(sizeof(*status), GFP_KERNEL);
944
945 if (!status)
946 return -ENOMEM;
947
948 ret = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
949 USB_REQ_GET_STATUS, USB_DIR_IN | type, 0, target,
status,
950 sizeof(*status), USB_CTRL_GET_TIMEOUT);
951
952 *(u16 *)data = *status;
953 kfree(status);
954 return ret;
955 }
956 EXPORT_SYMBOL_GPL(usb_get_status);
Maybe you can send a patch with a proposed improvement?
ok, thank you, I will do.
are the title of this kind of patch different with normal patch ?
for example:
normal patch is [PATCH] drivers/usb/core: ...
and our proposed improvement patch is same the normal patch ?
if not same, please help describing it (I am just learning)
thank you.
:-)
gchen.
Best regards
//Peter
--
Chen Gang
Asianux Corporation
--
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: [Suggestion] drivers/usb/core: for u16, use stack allocation instead of kmalloc
于 2012年12月06日 19:32, Oliver Neukum 写道: On Thursday 06 December 2012 18:51:25 Chen Gang wrote: Hello Greg Kroah-Hartman: in drivers/usb/core/message.c: at line 943, status is kmalloc ( sizeof u16 ) at line 952, assign the value of status to data at line 953, free status. it is better to let u16 status instead of u16 *status = kmalloc No, that would be a bug, because then we would do DMA on the stack. Doing DMA on the stack violates the DMA rules. Anything passed to usb_control_msg() as a buffer must be allocated from the heap, regardless of size. thank you. (learned) :-) gchen. Regards Oliver -- Chen Gang Asianux Corporation -- 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
[Suggestion] drivers/usb/core: dev-config and dev-rawdescriptors, when processing failure.
Hello Oliver Neukum:
in drivers/usb/core/config.c, for function usb_get_configuration:
if processing failed at line 694, we will goto err2 (line 755..759)
in this condition, we do not free dev-config and dev-rawdescriptors.
after checking another relative source code, it seems not an issue (I am not
quite sure).
but I still suggest to:
free them, when failure occurs
or check them whether be NULL before new allocation, (if not NULL, free
them firstly).
if this suggestion is valid, please help to send relative patch.
thanks.
:-)
gchen.
658 int usb_get_configuration(struct usb_device *dev)
659 {
660 struct device *ddev = dev-dev;
661 int ncfg = dev-descriptor.bNumConfigurations;
662 int result = 0;
663 unsigned int cfgno, length;
664 unsigned char *bigbuffer;
665 struct usb_config_descriptor *desc;
666
667 cfgno = 0;
668 if (dev-authorized == 0) /* Not really an error */
669 goto out_not_authorized;
670 result = -ENOMEM;
671 if (ncfg USB_MAXCONFIG) {
672 dev_warn(ddev, too many configurations: %d,
673 using maximum allowed: %d\n, ncfg, USB_MAXCONFIG);
674 dev-descriptor.bNumConfigurations = ncfg = USB_MAXCONFIG;
675 }
676
677 if (ncfg 1) {
678 dev_err(ddev, no configurations\n);
679 return -EINVAL;
680 }
681
682 length = ncfg * sizeof(struct usb_host_config);
683 dev-config = kzalloc(length, GFP_KERNEL);
684 if (!dev-config)
685 goto err2;
686
687 length = ncfg * sizeof(char *);
688 dev-rawdescriptors = kzalloc(length, GFP_KERNEL);
689 if (!dev-rawdescriptors)
690 goto err2;
691
692 desc = kmalloc(USB_DT_CONFIG_SIZE, GFP_KERNEL);
693 if (!desc)
694 goto err2;
695
696 result = 0;
697 for (; cfgno ncfg; cfgno++) {
698 /* We grab just the first descriptor so we know how long
699 * the whole configuration is */
700 result = usb_get_descriptor(dev, USB_DT_CONFIG, cfgno,
701 desc, USB_DT_CONFIG_SIZE);
702 if (result 0) {
703 dev_err(ddev, unable to read config index %d
704 descriptor/%s: %d\n, cfgno, start, result);
705 if (result != -EPIPE)
706 goto err;
707 dev_err(ddev, chopping to %d config(s)\n, cfgno);
708 dev-descriptor.bNumConfigurations = cfgno;
709 break;
710 } else if (result 4) {
711 dev_err(ddev, config index %d descriptor too short
712 (expected %i, got %i)\n, cfgno,
713 USB_DT_CONFIG_SIZE, result);
714 result = -EINVAL;
715 goto err;
716 }
717 length = max((int) le16_to_cpu(desc-wTotalLength),
718 USB_DT_CONFIG_SIZE);
719
720 /* Now that we know the length, get the whole thing */
721 bigbuffer = kmalloc(length, GFP_KERNEL);
722 if (!bigbuffer) {
723 result = -ENOMEM;
724 goto err;
725 }
726 result = usb_get_descriptor(dev, USB_DT_CONFIG, cfgno,
727 bigbuffer, length);
728 if (result 0) {
729 dev_err(ddev, unable to read config index %d
730 descriptor/%s\n, cfgno, all);
731 kfree(bigbuffer);
732 goto err;
733 }
734 if (result length) {
735 dev_warn(ddev, config index %d descriptor too
short
736 (expected %i, got %i)\n, cfgno, length,
result);
737 length = result;
738 }
739
740 dev-rawdescriptors[cfgno] = bigbuffer;
741
742 result = usb_parse_configuration(dev, cfgno,
743 dev-config[cfgno], bigbuffer, length);
744 if (result 0) {
745 ++cfgno;
746 goto err;
747 }
748 }
749 result = 0;
750
751 err:
752 kfree(desc);
753 out_not_authorized:
754 dev-descriptor.bNumConfigurations = cfgno;
755 err2:
756 if (result == -ENOMEM)
757 dev_err(ddev, out of memory\n);
758 return result;
759 }
--
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: [Suggestion] drivers/usb/core: dev-config and dev-rawdescriptors, when processing failure.
于 2012年12月06日 23:50, Alan Stern 写道:
On Thu, 6 Dec 2012, Chen Gang wrote:
in drivers/usb/core/config.c, for function usb_get_configuration:
if processing failed at line 694, we will goto err2 (line 755..759)
in this condition, we do not free dev-config and dev-rawdescriptors.
This isn't necessary. They are freed in usb_destroy_configuration().
after checking another relative source code, it seems not an issue (I am
not quite sure).
but I still suggest to:
free them, when failure occurs
or check them whether be NULL before new allocation, (if not NULL, free
them firstly).
There never is any new allocation. usb_get_configuration() is called
only once for any device. (For an example of this sort of check, see
usb_enumerate_device() in hub.c.)
but I still not quite be sure, please help checking (total 3 steps, below).
thanks.
--
Step 1:
in drivers/usb/core/sysfs.c:
for the same device, can usb_dev_authorized_store be called multi-times ?
according to the function comments, it seems can be called multi-times.
and usb_dev_authorized_store may call usb_authorize_device at line 600..603
585 /*
586 * Authorize a device to be used in the system
587 *
588 * Writing a 0 deauthorizes the device, writing a 1 authorizes it.
589 */
590 static ssize_t usb_dev_authorized_store(struct device *dev,
591 struct device_attribute *attr,
592 const char *buf, size_t size)
593 {
594 ssize_t result;
595 struct usb_device *usb_dev = to_usb_device(dev);
596 unsigned val;
597 result = sscanf(buf, %u\n, val);
598 if (result != 1)
599 result = -EINVAL;
600 else if (val == 0)
601 result = usb_deauthorize_device(usb_dev);
602 else
603 result = usb_authorize_device(usb_dev);
604 return result 0? result : size;
605 }
606
607 static DEVICE_ATTR_IGNORE_LOCKDEP(authorized, 0644,
608 usb_dev_authorized_show, usb_dev_authorized_store);
609
--
Step 2:
in drivers/usb/core/hub.c:
usb_authorize_device may call usb_enumerate_device at line 2355
2326 int usb_authorize_device(struct usb_device *usb_dev)
2327 {
2328 int result = 0, c;
2329
2330 usb_lock_device(usb_dev);
2331 if (usb_dev-authorized == 1)
2332 goto out_authorized;
2333
2334 result = usb_autoresume_device(usb_dev);
2335 if (result 0) {
2336 dev_err(usb_dev-dev,
2337 can't autoresume for authorization: %d\n,
result);
2338 goto error_autoresume;
2339 }
2340 result = usb_get_device_descriptor(usb_dev,
sizeof(usb_dev-descriptor));
2341 if (result 0) {
2342 dev_err(usb_dev-dev, can't re-read device descriptor
for
2343 authorization: %d\n, result);
2344 goto error_device_descriptor;
2345 }
2346
2347 kfree(usb_dev-product);
2348 usb_dev-product = NULL;
2349 kfree(usb_dev-manufacturer);
2350 usb_dev-manufacturer = NULL;
2351 kfree(usb_dev-serial);
2352 usb_dev-serial = NULL;
2353
2354 usb_dev-authorized = 1;
2355 result = usb_enumerate_device(usb_dev);
2356 if (result 0)
2357 goto error_enumerate;
2358 /* Choose and set the configuration. This registers the interfaces
2359 * with the driver core and lets interface drivers bind to them.
2360 */
2361 c = usb_choose_configuration(usb_dev);
2362 if (c = 0) {
2363 result = usb_set_configuration(usb_dev, c);
2364 if (result) {
2365 dev_err(usb_dev-dev,
2366 can't set config #%d, error %d\n, c,
result);
2367 /* This need not be fatal. The user can try to
2368 * set other configurations. */
2369 }
2370 }
2371 dev_info(usb_dev-dev, authorized to connect\n);
2372
2373 error_enumerate:
2374 error_device_descriptor:
2375 usb_autosuspend_device(usb_dev);
2376 error_autoresume:
2377 out_authorized:
2378 usb_unlock_device(usb_dev); // complements locktree
2379 return result;
2380 }
--
Step 3:
also in drivers/usb/core/hub.c:
if udev-config != NULL:
we will assume that it has already configured,
and will not call usb_get_configuration again.
if first call for usb_get_confiuration failed, may udev-config will not be
NULL.
and next call usb_enumerate_device,
we will misunderstand that it has already
