usb: chipidea: add one errata for revision 2.40a
Subsystems (2)
cover usb: chipidea: add one errata for revision 2.40a
The first two patches add identification register API's. These can be used to get controller's revision.
The third patch implements an errata for revision 2.40a. Not sure which other SOCs implement this version of the Chipidea core but this fixes the usb client issue observed on Vybrids. The patch was tested on a Toradex Colibri VF61 module with the 3.18 kernel. iperf tests ran for three hours plus, with these patches applied have found the USB client connection to be now reliable.
This patchset is based off on Shawn Guo's for-next branch
https://git.kernel.org/cgit/linux/kernel/git/shawnguo/linux.git/tree/?h=for-next
The credit for the patches and fix goes to Matthieu Castet and Peter Chen. First two patches are by Peter Chen and the third patch which fixed the bug we observed was reported by Matthieu Castet.
The discussion of the problem and the relevant testing details can be found at this link: http://www.spinics.net/lists/linux-usb/msg118544.html
The first version of this patchset originally send by Peter Chen can be found at this link: http://www.spinics.net/lists/linux-usb/msg118753.html
Comments for review are welcome :).
Note: I am going on a vacation so will not be able to reply or do any further tests till Monday. Will attend and take care of any comments/requests for further changes/testing from Tuesday. Apologize for the delay in advance.
Sanchayan Maity (3):
usb: chipidea: Add identification registers access APIs
usb: chipidea: Add chipidea revision information
usb: chipidea: Add errata for revision 2.40a
drivers/usb/chipidea/bits.h | 10 ++++++++ drivers/usb/chipidea/ci.h | 53 +++++++++++++++++++++++++++++++++++++++++++ drivers/usb/chipidea/core.c | 23 +++++++++++++++++-- drivers/usb/chipidea/udc.c | 20 ++++++++++++++++ 4 files changed, 104 insertions(+), 2 deletions(-)
Signature · 1 line
1.7.9.5
PATCH 1/3 usb: chipidea: Add identification registers access APIs
Using hw_write_id_reg and hw_read_id_reg to write and read identification registers contents. This can be used to get controller information, change some system configurations and so on.
--- drivers/usb/chipidea/ci.h | 53 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+)
drivers/usb/chipidea/ci.h +53 -0
@@ -29,6 +29,15 @@/****************************************************************************** * REGISTERS *****************************************************************************/ +/* Identification Registers */ +#define ID_ID 0x0 +#define ID_HWGENERAL 0x4 +#define ID_HWHOST 0x8 +#define ID_HWDEVICE 0xc +#define ID_HWTXBUF 0x10 +#define ID_HWRXBUF 0x14 +#define ID_SBUSCFG 0x90 +/* register indices */ enum ci_hw_regs {CAP_CAPLENGTH ,
@@ -97,6 +106,18 @@ enum ci_role { CI_ROLE_END, }; +enum CI_REVISION { +CI_REVISION_1X =10 ,/* Revision 1.x */ +CI_REVISION_20 =20 ,/* Revision 2.0 */ +CI_REVISION_21 ,/* Revision 2.1 */ +CI_REVISION_22 ,/* Revision 2.2 */ +CI_REVISION_23 ,/* Revision 2.3 */ +CI_REVISION_24 ,/* Revision 2.4 */ +CI_REVISION_25 ,/* Revision 2.5 */ +CI_REVISION_25_PLUS ,/* Revision above than 2.5 */ +CI_REVISION_UNKNOWN =99 ,/* Unknown Revision */ +}; +/** * struct ci_role_driver - host/gadget role driver * @start: start this role
@@ -168,6 +189,7 @@ struct hw_bank {* @b_sess_valid_event: indicates there is a vbus event, and handled * at ci_otg_work * @imx28_write_fix: Freescale imx28 needs swp instruction for writing +* @rev: The revision number for controller */ struct ci_hdrc {struct device *dev;
@@ -207,6 +229,7 @@ struct ci_hdrc {bool id_event;bool b_sess_valid_event;bool imx28_write_fix; +enum CI_REVISION rev; };static inline struct ci_role_driver *ci_role (struct ci_hdrc *ci)
@@ -244,6 +267,36 @@ static inline void ci_role_stop(struct ci_hdrc *ci) }/** +* hw_read_id_reg: reads from a identification register +* @ci: the controller +* @offset: offset from the beginning of identification registers region +* @mask: bitfield mask +* +* This function returns register contents +*/ +static inline u32 hw_read_id_reg (struct ci_hdrc *ci,u32 offset,u32 mask) +{ +return ioread32 (ci->hw_bank.abs + offset) & mask; +} + +/** +* hw_write_id_reg: writes to a identification register +* @ci: the controller +* @offset: offset from the beginning of identification registers region +* @mask: bitfield mask +* @data: new value +*/ +static inline void hw_write_id_reg (struct ci_hdrc *ci,u32 offset, +u32 mask,u32 data) +{ +if (~mask) + data = (ioread32 (ci->hw_bank.abs + offset) & ~mask) + | (data & mask); + +iowrite32 (data, ci->hw_bank.abs + offset); +} + +/** * hw_read: reads from a hw register * @ci: the controller * @reg: register index
Signature · 1 line
1.7.9.5
Re: [PATCH 1/3] usb: chipidea: Add identification registers access APIs
Quoted 4 lines · Sanchayan Maity
Using
hw_write_id_regandhw_read_id_regto write and read identification registers contents. This can be used to get controller information, change some system configurations and so on.
Checkpatch is complaining about DOS line endings and some trailing whitespace. This applies to all three patches.
Quoted 27 lines
Signed-off-by Sanchayan Maity <[email protected]>--- drivers/usb/chipidea/ci.h | 53 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+)drivers/usb/chipidea/ci.h +9
-0@@ -29,6 +29,15 @@/****************************************************************************** * REGISTERS *****************************************************************************/ +/* Identification Registers */ +#define ID_ID 0x0 +#define ID_HWGENERAL 0x4 +#define ID_HWHOST 0x8 +#define ID_HWDEVICE 0xc +#define ID_HWTXBUF 0x10 +#define ID_HWRXBUF 0x14 +#define ID_SBUSCFG 0x90 +/* register indices */ enum ci_hw_regs {CAP_CAPLENGTH ,
drivers/usb/chipidea/ci.h enum ci_role { · line 109
Quoted 4 lines
drivers/usb/chipidea/ci.h +1
-0@@ -97,6 +106,18 @@ enum ci_role { CI_ROLE_END, }; +enum CI_REVISION {
Usually the enum names are small caps, only the labels are capitalized. I would suggest use ci_revision here (similar to the enum above).
Quoted 52 lines
drivers/usb/chipidea/ci.h +31
-0+CI_REVISION_1X =10 ,/* Revision 1.x */ +CI_REVISION_20 =20 ,/* Revision 2.0 */ +CI_REVISION_21 ,/* Revision 2.1 */ +CI_REVISION_22 ,/* Revision 2.2 */ +CI_REVISION_23 ,/* Revision 2.3 */ +CI_REVISION_24 ,/* Revision 2.4 */ +CI_REVISION_25 ,/* Revision 2.5 */ +CI_REVISION_25_PLUS ,/* Revision above than 2.5 */ + CI_REVISION_UNKNOWN = 99, /* Unknown Revision */ +}; +/** * struct ci_role_driver - host/gadget role driver * @start: start this role @@ -168,6 +189,7 @@ struct hw_bank {* @b_sess_valid_event: indicates there is a vbus event, and handled * at ci_otg_work * @imx28_write_fix: Freescale imx28 needs swp instruction for writing +* @rev: The revision number for controller */ struct ci_hdrc {struct device *dev;@@ -207,6 +229,7 @@ struct ci_hdrc {bool id_event;bool b_sess_valid_event;bool imx28_write_fix; +enum CI_REVISION rev; };static inline struct ci_role_driver *ci_role (struct ci_hdrc *ci)@@ -244,6 +267,36 @@ static inline void ci_role_stop(struct ci_hdrc *ci) }/** +* hw_read_id_reg: reads from a identification register +* @ci: the controller +* @offset: offset from the beginning of identification registers region +* @mask: bitfield mask +* +* This function returns register contents +*/ +static inline u32 hw_read_id_reg (struct ci_hdrc *ci,u32 offset,u32 mask) +{ +return ioread32 (ci->hw_bank.abs + offset) & mask; +} + +/** +* hw_write_id_reg: writes to a identification register +* @ci: the controller +* @offset: offset from the beginning of identification registers region +* @mask: bitfield mask +* @data: new value
drivers/usb/chipidea/ci.h static inline void ci_role_stop(struct ci_hdrc *ci) · line 297
Quoted 10 lines
drivers/usb/chipidea/ci.h +10
-0+*/ +static inline void hw_write_id_reg (struct ci_hdrc *ci,u32 offset, +u32 mask,u32 data) +{ +if (~mask) + data = (ioread32 (ci->hw_bank.abs + offset) & ~mask) + | (data & mask); + +iowrite32 (data, ci->hw_bank.abs + offset); +}
This function isn't used anywhere, does it make sense to write an ID register? The ones I see in Vybrid's RM are read-only anyway..
--
Stefan
Quoted 5 lines
drivers/usb/chipidea/ci.h +2
-0+ +/** * hw_read: reads from a hw register * @ci: the controller * @reg: register index
Re: [PATCH 1/3] usb: chipidea: Add identification registers access APIs
Quoted 8 lines · Stefan Agner
Quoted 4 lines · Sanchayan Maity
Using
hw_write_id_regandhw_read_id_regto write and read identification registers contents. This can be used to get controller information, change some system configurations and so on.Checkpatch is complaining about DOS line endings and some trailing whitespace. This applies to all three patches.
Hmm... that's surprising. I did run checkpatch at my end before sending. I do not have the original patchset on my laptop which I have right now but downloading diffs back from lkml and running checkpatch on them back, only gives me signed off errors as it should.
I will resend if somehow errors crept in with my earlier patches.
-Sanchayan
Quoted 111 lines
Quoted 28 lines
Signed-off-by Sanchayan Maity <[email protected]>--- drivers/usb/chipidea/ci.h | 53 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+)drivers/usb/chipidea/ci.h +9
-0@@ -29,6 +29,15 @@/****************************************************************************** * REGISTERS *****************************************************************************/ +/* Identification Registers */ +#define ID_ID 0x0 +#define ID_HWGENERAL 0x4 +#define ID_HWHOST 0x8 +#define ID_HWDEVICE 0xc +#define ID_HWTXBUF 0x10 +#define ID_HWRXBUF 0x14 +#define ID_SBUSCFG 0x90 +/* register indices */ enum ci_hw_regs {CAP_CAPLENGTH ,drivers/usb/chipidea/ci.h enum ci_role { · line 109
Quoted 4 lines
drivers/usb/chipidea/ci.h +1
-0@@ -97,6 +106,18 @@ enum ci_role { CI_ROLE_END, }; +enum CI_REVISION {Usually the enum names are small caps, only the labels are capitalized. I would suggest use
ci_revisionhere (similar to the enum above).Quoted 52 lines
drivers/usb/chipidea/ci.h +31
-0+CI_REVISION_1X =10 ,/* Revision 1.x */ +CI_REVISION_20 =20 ,/* Revision 2.0 */ +CI_REVISION_21 ,/* Revision 2.1 */ +CI_REVISION_22 ,/* Revision 2.2 */ +CI_REVISION_23 ,/* Revision 2.3 */ +CI_REVISION_24 ,/* Revision 2.4 */ +CI_REVISION_25 ,/* Revision 2.5 */ +CI_REVISION_25_PLUS ,/* Revision above than 2.5 */ + CI_REVISION_UNKNOWN = 99, /* Unknown Revision */ +}; +/** * struct ci_role_driver - host/gadget role driver * @start: start this role @@ -168,6 +189,7 @@ struct hw_bank {* @b_sess_valid_event: indicates there is a vbus event, and handled * at ci_otg_work * @imx28_write_fix: Freescale imx28 needs swp instruction for writing +* @rev: The revision number for controller */ struct ci_hdrc {struct device *dev;@@ -207,6 +229,7 @@ struct ci_hdrc {bool id_event;bool b_sess_valid_event;bool imx28_write_fix; +enum CI_REVISION rev; };static inline struct ci_role_driver *ci_role (struct ci_hdrc *ci)@@ -244,6 +267,36 @@ static inline void ci_role_stop(struct ci_hdrc *ci) }/** +* hw_read_id_reg: reads from a identification register +* @ci: the controller +* @offset: offset from the beginning of identification registers region +* @mask: bitfield mask +* +* This function returns register contents +*/ +static inline u32 hw_read_id_reg (struct ci_hdrc *ci,u32 offset,u32 mask) +{ +return ioread32 (ci->hw_bank.abs + offset) & mask; +} + +/** +* hw_write_id_reg: writes to a identification register +* @ci: the controller +* @offset: offset from the beginning of identification registers region +* @mask: bitfield mask +* @data: new value drivers/usb/chipidea/ci.h static inline void ci_role_stop(struct ci_hdrc *ci) · line 297
Quoted 10 lines
drivers/usb/chipidea/ci.h +10
-0+*/ +static inline void hw_write_id_reg (struct ci_hdrc *ci,u32 offset, +u32 mask,u32 data) +{ +if (~mask) + data = (ioread32 (ci->hw_bank.abs + offset) & ~mask) + | (data & mask); + +iowrite32 (data, ci->hw_bank.abs + offset); +}This function isn't used anywhere, does it make sense to write an ID register? The ones I see in Vybrid's RM are read-only anyway..
--
StefanQuoted 5 lines
drivers/usb/chipidea/ci.h +2
-0+ +/** * hw_read: reads from a hw register * @ci: the controller * @reg: register index
Re: [PATCH 1/3] usb: chipidea: Add identification registers access APIs
Quoted 7 lines · Sanchayan Maity
Quoted 6 lines · Stefan Agner
Quoted 4 lines · Sanchayan Maity
Using
hw_write_id_regandhw_read_id_regto write and read identification registers contents. This can be used to get controller information, change some system configurations and so on.
Quoted 8 lines
Quoted 2 lines
Checkpatch is complaining about DOS line endings and some trailing whitespace. This applies to all three patches.
Hmm... that's surprising. I did run checkpatch at my end before sending. I do not have the original patchset on my laptop which I have right now but downloading diffs back from lkml and running checkpatch on them back, only gives me signed off errors as it should.
You are right, it's on my side. My webmailer (Roundcube) seem to introduce those line endings when using "Download (.eml)". Using "Show Source", and then save the e-mail makes checkpatch happy... Sorry about that.
--
Stefan
Quoted 115 lines
I will resend if somehow errors crept in with my earlier patches.
-Sanchayan
Quoted 111 lines
Quoted 28 lines
Signed-off-by Sanchayan Maity <[email protected]>--- drivers/usb/chipidea/ci.h | 53 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+)drivers/usb/chipidea/ci.h +9
-0@@ -29,6 +29,15 @@/****************************************************************************** * REGISTERS *****************************************************************************/ +/* Identification Registers */ +#define ID_ID 0x0 +#define ID_HWGENERAL 0x4 +#define ID_HWHOST 0x8 +#define ID_HWDEVICE 0xc +#define ID_HWTXBUF 0x10 +#define ID_HWRXBUF 0x14 +#define ID_SBUSCFG 0x90 +/* register indices */ enum ci_hw_regs {CAP_CAPLENGTH ,drivers/usb/chipidea/ci.h enum ci_role { · line 109
Quoted 4 lines
drivers/usb/chipidea/ci.h +1
-0@@ -97,6 +106,18 @@ enum ci_role { CI_ROLE_END, }; +enum CI_REVISION {Usually the enum names are small caps, only the labels are capitalized. I would suggest use
ci_revisionhere (similar to the enum above).Quoted 52 lines
drivers/usb/chipidea/ci.h +31
-0+CI_REVISION_1X =10 ,/* Revision 1.x */ +CI_REVISION_20 =20 ,/* Revision 2.0 */ +CI_REVISION_21 ,/* Revision 2.1 */ +CI_REVISION_22 ,/* Revision 2.2 */ +CI_REVISION_23 ,/* Revision 2.3 */ +CI_REVISION_24 ,/* Revision 2.4 */ +CI_REVISION_25 ,/* Revision 2.5 */ +CI_REVISION_25_PLUS ,/* Revision above than 2.5 */ + CI_REVISION_UNKNOWN = 99, /* Unknown Revision */ +}; +/** * struct ci_role_driver - host/gadget role driver * @start: start this role @@ -168,6 +189,7 @@ struct hw_bank {* @b_sess_valid_event: indicates there is a vbus event, and handled * at ci_otg_work * @imx28_write_fix: Freescale imx28 needs swp instruction for writing +* @rev: The revision number for controller */ struct ci_hdrc {struct device *dev;@@ -207,6 +229,7 @@ struct ci_hdrc {bool id_event;bool b_sess_valid_event;bool imx28_write_fix; +enum CI_REVISION rev; };static inline struct ci_role_driver *ci_role (struct ci_hdrc *ci)@@ -244,6 +267,36 @@ static inline void ci_role_stop(struct ci_hdrc *ci) }/** +* hw_read_id_reg: reads from a identification register +* @ci: the controller +* @offset: offset from the beginning of identification registers region +* @mask: bitfield mask +* +* This function returns register contents +*/ +static inline u32 hw_read_id_reg (struct ci_hdrc *ci,u32 offset,u32 mask) +{ +return ioread32 (ci->hw_bank.abs + offset) & mask; +} + +/** +* hw_write_id_reg: writes to a identification register +* @ci: the controller +* @offset: offset from the beginning of identification registers region +* @mask: bitfield mask +* @data: new value drivers/usb/chipidea/ci.h static inline void ci_role_stop(struct ci_hdrc *ci) · line 297
Quoted 10 lines
drivers/usb/chipidea/ci.h +10
-0+*/ +static inline void hw_write_id_reg (struct ci_hdrc *ci,u32 offset, +u32 mask,u32 data) +{ +if (~mask) + data = (ioread32 (ci->hw_bank.abs + offset) & ~mask) + | (data & mask); + +iowrite32 (data, ci->hw_bank.abs + offset); +}This function isn't used anywhere, does it make sense to write an ID register? The ones I see in Vybrid's RM are read-only anyway..
--
StefanQuoted 5 lines
drivers/usb/chipidea/ci.h +2
-0+ +/** * hw_read: reads from a hw register * @ci: the controller * @reg: register index
Re: [PATCH 1/3] usb: chipidea: Add identification registers access APIs
In reply to Sanchayan MaityQuoted 10 lines · Sanchayan Maity
Quoted 8 lines · Stefan Agner
Quoted 4 lines · Sanchayan Maity
Using
hw_write_id_regandhw_read_id_regto write and read identification registers contents. This can be used to get controller information, change some system configurations and so on.Checkpatch is complaining about DOS line endings and some trailing whitespace. This applies to all three patches.
Quoted 9 lines
Hmm... that's surprising. I did run checkpatch at my end before sending. I do not have the original patchset on my laptop which I have right now but downloading diffs back from lkml and running checkpatch on them back, only gives me signed off errors as it should.
I will resend if somehow errors crept in with my earlier patches.
-Sanchayan
Hi Sanchayan,
I will take care the first two, you are the third if there is any comments, thanks.
Signature · 2 lines
Best Regards, Peter Chen
Re: [PATCH 1/3] usb: chipidea: Add identification registers access APIs
Quoted 18 lines · Peter Chen
Quoted 17 lines · Sanchayan Maity
Quoted 8 lines · Stefan Agner
Quoted 4 lines · Sanchayan Maity
Using
hw_write_id_regandhw_read_id_regto write and read identification registers contents. This can be used to get controller information, change some system configurations and so on.Checkpatch is complaining about DOS line endings and some trailing whitespace. This applies to all three patches.
Hmm... that's surprising. I did run checkpatch at my end before sending. I do not have the original patchset on my laptop which I have right now but downloading diffs back from lkml and running checkpatch on them back, only gives me signed off errors as it should.
I will resend if somehow errors crept in with my earlier patches.
Quoted 8 lines
Quoted 2 lines
-Sanchayan
Hi Sanchayan,
I will take care the first two, you are the third if there is any comments, thanks.
Hi Peter,
OK with me. I can resend all the three patches with Stefan's recommended changes if you did like.
-Regards,
Sanchayan.
Re: [PATCH 1/3] usb: chipidea: Add identification registers access APIs
In reply to Stefan AgnerQuoted 39 lines · Stefan Agner
Quoted 4 lines · Sanchayan Maity
Using
hw_write_id_regandhw_read_id_regto write and read identification registers contents. This can be used to get controller information, change some system configurations and so on.Checkpatch is complaining about DOS line endings and some trailing whitespace. This applies to all three patches.
Quoted 30 lines
Signed-off-by Sanchayan Maity <[email protected]>--- drivers/usb/chipidea/ci.h | 53 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+)drivers/usb/chipidea/ci.h +9
-0@@ -29,6 +29,15 @@/****************************************************************************** * REGISTERS *****************************************************************************/ +/* Identification Registers */ +#define ID_ID 0x0 +#define ID_HWGENERAL 0x4 +#define ID_HWHOST 0x8 +#define ID_HWDEVICE 0xc +#define ID_HWTXBUF 0x10 +#define ID_HWRXBUF 0x14 +#define ID_SBUSCFG 0x90 +/* register indices */ enum ci_hw_regs {CAP_CAPLENGTH ,@@ -97,6 +106,18 @@ enum ci_role { CI_ROLE_END, };
Quoted 4 lines
drivers/usb/chipidea/ci.h enum ci_role { · line 109
Quoted 1 line
drivers/usb/chipidea/ci.h +1
-0+enum CI_REVISION {Usually the enum names are small caps, only the labels are capitalized. I would suggest use
ci_revisionhere (similar to the enum above).
I will change it.
Quoted 58 lines
Quoted 58 lines
drivers/usb/chipidea/ci.h +36
-0+CI_REVISION_1X =10 ,/* Revision 1.x */ +CI_REVISION_20 =20 ,/* Revision 2.0 */ +CI_REVISION_21 ,/* Revision 2.1 */ +CI_REVISION_22 ,/* Revision 2.2 */ +CI_REVISION_23 ,/* Revision 2.3 */ +CI_REVISION_24 ,/* Revision 2.4 */ +CI_REVISION_25 ,/* Revision 2.5 */ +CI_REVISION_25_PLUS ,/* Revision above than 2.5 */ + CI_REVISION_UNKNOWN = 99, /* Unknown Revision */ +}; +/** * struct ci_role_driver - host/gadget role driver * @start: start this role @@ -168,6 +189,7 @@ struct hw_bank {* @b_sess_valid_event: indicates there is a vbus event, and handled * at ci_otg_work * @imx28_write_fix: Freescale imx28 needs swp instruction for writing +* @rev: The revision number for controller */ struct ci_hdrc {struct device *dev;@@ -207,6 +229,7 @@ struct ci_hdrc {bool id_event;bool b_sess_valid_event;bool imx28_write_fix; +enum CI_REVISION rev; };static inline struct ci_role_driver *ci_role (struct ci_hdrc *ci)@@ -244,6 +267,36 @@ static inline void ci_role_stop(struct ci_hdrc *ci) }/** +* hw_read_id_reg: reads from a identification register +* @ci: the controller +* @offset: offset from the beginning of identification registers region +* @mask: bitfield mask +* +* This function returns register contents +*/ +static inline u32 hw_read_id_reg (struct ci_hdrc *ci,u32 offset,u32 mask) +{ +return ioread32 (ci->hw_bank.abs + offset) & mask; +} + +/** +* hw_write_id_reg: writes to a identification register +* @ci: the controller +* @offset: offset from the beginning of identification registers region +* @mask: bitfield mask +* @data: new value +*/ +static inline void hw_write_id_reg (struct ci_hdrc *ci,u32 offset, +u32 mask,u32 data) +{ +if (~mask)
Quoted 9 lines
drivers/usb/chipidea/ci.h static inline void ci_role_stop(struct ci_hdrc *ci) · line 297
Quoted 5 lines
drivers/usb/chipidea/ci.h +5
-0+ data = (ioread32 (ci->hw_bank.abs + offset) & ~mask) + | (data & mask); + +iowrite32 (data, ci->hw_bank.abs + offset); +}This function isn't used anywhere, does it make sense to write an ID register? The ones I see in Vybrid's RM are read-only anyway..
This API is used to write identification registers (offset, 0x00 - 0x90), not only the ID register, the SBUSCFG (offset 0x90) attribute is read/write.
Signature · 2 lines
Best Regards, Peter Chen
PATCH 2/3 usb: chipidea: Add chipidea revision information
In reply to Sanchayan MaityDefine ci_get_revision API to know the controller revision information according to chipidea 1.1a, 2.0a, 2.4 and 2.5a spec. Besides, add one entry in struct ci_hdrc to indicate revision information. This can be used for adding different code for revisions, implementing erratas.
--- drivers/usb/chipidea/bits.h | 10 ++++++++++ drivers/usb/chipidea/core.c | 23 +++++++++++++++++++++-- 2 files changed, 31 insertions(+), 2 deletions(-)
drivers/usb/chipidea/bits.h +10 -0
@@ -15,6 +15,16 @@#include <linux/usb/ehci_def.h> +/* +* ID +* For 1.x revision, bit24 - bit31 are reserved +* For 2.x revision, bit25 - bit28 are 0x2 +*/ +#define TAG (0x1F << 16) +#define REVISION (0xF << 21) +#define VERSION (0xF << 25) +#define CIVERSION (0x7 << 29) +/* HCCPARAMS */ #define HCCPARAMS_LEN BIT(17)
drivers/usb/chipidea/core.c +21 -2
@@ -136,6 +136,22 @@ static int hw_alloc_regmap(struct ci_hdrc *ci, bool is_lpm)return 0 ; } +static enum CI_REVISION ci_get_revision (struct ci_hdrc *ci) +{ +int ver =hw_read_id_reg (ci,ID_ID ,VERSION ) >>__ffs (VERSION ); +enum CI_REVISION rev =CI_REVISION_UNKNOWN ; + +if (ver ==0x2 ) { +int rev_reg =hw_read_id_reg + (ci,ID_ID ,REVISION ) >>__ffs (REVISION ); + rev = rev_reg +CI_REVISION_20 ; + }else if (ver ==0x0 ) { + rev =CI_REVISION_1X ; + } + +return rev; +} +/** * hw_read_intr_enable: returns interrupt enable register *
@@ -245,8 +261,11 @@ static int hw_device_init(struct ci_hdrc *ci, void __iomem *base)/* Clear all interrupts status bits*/ hw_write (ci,OP_USBSTS ,0xffffffff ,0xffffffff ); -dev_dbg (ci->dev,"ChipIdea HDRC found, lpm: %d; cap: %p op: %p\n" , - ci->hw_bank.lpm, ci->hw_bank.cap, ci->hw_bank.op); + ci->rev =ci_get_revision (ci); + +dev_dbg (ci->dev, +"ChipIdea HDRC found, revision: %d, lpm: %d; cap: %p op: %p\n" , + ci->rev, ci->hw_bank.lpm, ci->hw_bank.cap, ci->hw_bank.op);/* setup lock mode ? */
Signature · 1 line
1.7.9.5
Re: [PATCH 2/3] usb: chipidea: Add chipidea revision information
Quoted 20 lines · Sanchayan Maity
Define
ci_get_revisionAPI to know the controller revision information according to chipidea 1.1a, 2.0a, 2.4 and 2.5a spec. Besides, add one entry in structci_hdrcto indicate revision information. This can be used for adding different code for revisions, implementing erratas.Signed-off-by Sanchayan Maity <[email protected]>--- drivers/usb/chipidea/bits.h | 10 ++++++++++ drivers/usb/chipidea/core.c | 23 +++++++++++++++++++++-- 2 files changed, 31 insertions(+), 2 deletions(-)drivers/usb/chipidea/bits.h +0
-0@@ -15,6 +15,16 @@#include <linux/usb/ehci_def.h>
drivers/usb/chipidea/bits.h line 27
Quoted 10 lines
drivers/usb/chipidea/bits.h +10
-0+/* +* ID +* For 1.x revision, bit24 - bit31 are reserved +* For 2.x revision, bit25 - bit28 are 0x2 +*/ +#define TAG (0x1F << 16) +#define REVISION (0xF << 21) +#define VERSION (0xF << 25) +#define CIVERSION (0x7 << 29) +
Hm, the other defines use spaces in this file, I like tabs more too, but I guess uniformity wins here.. Peter?
Quoted 9 lines
drivers/usb/chipidea/bits.h +0
-0/* HCCPARAMS */ #define HCCPARAMS_LEN BIT(17)drivers/usb/chipidea/core.c +0
-0@@ -136,6 +136,22 @@ static int hw_alloc_regmap(struct ci_hdrc *ci, bool is_lpm)return 0 ;
drivers/usb/chipidea/core.c static int hw_alloc_regmap(struct ci_hdrc *ci, bool is_lpm) · line 146
Quoted 10 lines
drivers/usb/chipidea/core.c +8
-0} +static enum CI_REVISION ci_get_revision (struct ci_hdrc *ci) +{ +int ver =hw_read_id_reg (ci,ID_ID ,VERSION ) >>__ffs (VERSION ); +enum CI_REVISION rev =CI_REVISION_UNKNOWN ; + +if (ver ==0x2 ) { +int rev_reg =hw_read_id_reg + (ci,ID_ID ,REVISION ) >>__ffs (REVISION );
This line break is somewhat awkward, doesn't help for good readability.
Is the local integer variable necessary at all? Enums are arithmetic types, so afaik this should be fine too:
rev = CI_REVISION_20; rev += hw_read_id_reg(ci, ID_ID, REVISION) >> __ffs(REVISION);
Quoted 25 lines
drivers/usb/chipidea/core.c +13
-2+ rev = rev_reg +CI_REVISION_20 ; + }else if (ver ==0x0 ) { + rev =CI_REVISION_1X ; + } + +return rev; +} +/** * hw_read_intr_enable: returns interrupt enable register * @@ -245,8 +261,11 @@ static int hw_device_init(struct ci_hdrc *ci, void __iomem *base) /* Clear all interrupts status bits*/ hw_write(ci, OP_USBSTS, 0xffffffff, 0xffffffff); - dev_dbg(ci->dev, "ChipIdea HDRC found, lpm: %d; cap: %p op: %p\n", - ci->hw_bank.lpm, ci->hw_bank.cap, ci->hw_bank.op); + ci->rev = ci_get_revision(ci); + + dev_dbg(ci->dev, +"ChipIdea HDRC found, revision: %d, lpm: %d; cap: %p op: %p\n" , + ci->rev, ci->hw_bank.lpm, ci->hw_bank.cap, ci->hw_bank.op);/* setup lock mode ? */
Re: [PATCH 2/3] usb: chipidea: Add chipidea revision information
Quoted 21 lines · Stefan Agner
Quoted 20 lines · Sanchayan Maity
Define
ci_get_revisionAPI to know the controller revision information according to chipidea 1.1a, 2.0a, 2.4 and 2.5a spec. Besides, add one entry in structci_hdrcto indicate revision information. This can be used for adding different code for revisions, implementing erratas.Signed-off-by Sanchayan Maity <[email protected]>--- drivers/usb/chipidea/bits.h | 10 ++++++++++ drivers/usb/chipidea/core.c | 23 +++++++++++++++++++++-- 2 files changed, 31 insertions(+), 2 deletions(-)drivers/usb/chipidea/bits.h +0
-0@@ -15,6 +15,16 @@#include <linux/usb/ehci_def.h>
Quoted 13 lines
drivers/usb/chipidea/bits.h line 27
Quoted 10 lines
drivers/usb/chipidea/bits.h +10
-0+/* +* ID +* For 1.x revision, bit24 - bit31 are reserved +* For 2.x revision, bit25 - bit28 are 0x2 +*/ +#define TAG (0x1F << 16) +#define REVISION (0xF << 21) +#define VERSION (0xF << 25) +#define CIVERSION (0x7 << 29) +Hm, the other defines use spaces in this file, I like tabs more too, but I guess uniformity wins here.. Peter?
I will change to use spaces.
Quoted 12 lines
Quoted 12 lines
drivers/usb/chipidea/bits.h +0
-0/* HCCPARAMS */ #define HCCPARAMS_LEN BIT(17)drivers/usb/chipidea/core.c +0
-0@@ -136,6 +136,22 @@ static int hw_alloc_regmap(struct ci_hdrc *ci, bool is_lpm)return 0 ; }
Quoted 11 lines
drivers/usb/chipidea/core.c static int hw_alloc_regmap(struct ci_hdrc *ci, bool is_lpm) · line 146
Quoted 8 lines
drivers/usb/chipidea/core.c +8
-0+static enum CI_REVISION ci_get_revision (struct ci_hdrc *ci) +{ +int ver =hw_read_id_reg (ci,ID_ID ,VERSION ) >>__ffs (VERSION ); +enum CI_REVISION rev =CI_REVISION_UNKNOWN ; + +if (ver ==0x2 ) { +int rev_reg =hw_read_id_reg + (ci,ID_ID ,REVISION ) >>__ffs (REVISION );This line break is somewhat awkward, doesn't help for good readability.
But it exceeds 80 characters, maybe I change like below:
rev += hw_read_id_reg(ci, ID_ID, REVISION)
>> __ffs(REVISION);
Quoted 4 lines
Is the local integer variable necessary at all? Enums are arithmetic types, so afaik this should be fine too:
rev = CI_REVISION_20; rev += hw_read_id_reg(ci, ID_ID, REVISION) >> __ffs(REVISION);
Will change, thanks.
Signature · 2 lines
Best Regards, Peter Chen
PATCH 3/3 usb: chipidea: Add errata for revision 2.40a
In reply to Sanchayan MaityAt chipidea revision 2.40a, there is a below errata:
9000531823 B2-Medium Adding a dTD to a Primed Endpoint May Not Get Recognized
Title: Adding a dTD to a Primed Endpoint May Not Get Recognized
Impacted Configuration: All device mode configurations.
Description:
There is an issue with the add dTD tripwire semaphore (ATDTW bit in USBCMD register) that can cause the controller to ignore a dTD that is added to a primed endpoint. When this happens, the software can read the tripwire bit and the status bit at '1' even though the endpoint is unprimed.
After executing a dTD, the device controller endpoint state machine executes a final read of the dTD terminate bit to check if the application added a dTD to the linked list at the last moment. This read is done in the finpkt_read_latest_next_td (44) state. After the read is performed, if the terminate bit is still set, the state machine moves to unprime the endpoint. The decision to unprime the endpoint is done in the checkqh_decision (59) state, based on the value of the terminate bit. Before reaching the checkqh_decision state, the state machine traverses the writeqhtd_status (57), writeqh_status (56), and release_prime_mask (42) states. As shown in the waveform, the ep_addtd_tripwire_clr signal is not set to clear the tripwire bit in these states.
Workaround:
The software must implement a periodic poll cycle, and check for each dTD pending on execution (Active = 1), if the enpoint is primed. It can do this by reading the corresponding bits in the ENDPTPRIME and ENDPTSTAT registers. If these bits are read at 0, the software needs to re-prime the endpoint by writing 1 to the corresponding bit in the ENDPTPRIME register. This can be done for every microframe, every frame or with a larger interval, depending on the urgency of transfer execution for the application.
--- drivers/usb/chipidea/udc.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
drivers/usb/chipidea/udc.c +20 -0
@@ -522,6 +522,20 @@ static void free_pending_td(struct ci_hw_ep *hwep)kfree (pending); } +static int reprime_dtd (struct ci_hdrc *ci,struct ci_hw_ep *hwep, +struct td_node *node) +{ + hwep->qh.ptr->td.next = node->dma; + hwep->qh.ptr->td.token &= +cpu_to_le32 (~(TD_STATUS_HALTED |TD_STATUS_ACTIVE )); + +/* Synchronize before ep prime */ +wmb (); + +return hw_ep_prime (ci, hwep->num, hwep->dir, + hwep->type ==USB_ENDPOINT_XFER_CONTROL ); +} +/** * _hardware_dequeue: handles a request at hardware level * @gadget: gadget
@@ -535,6 +549,7 @@ static int _hardware_dequeue(struct ci_hw_ep *hwep, struct ci_hw_req *hwreq)struct td_node *node, *tmpnode;unsigned remaining_length;unsigned actual = hwreq->req.length; +struct ci_hdrc *ci = hwep->ci;if (hwreq->req.status != -EALREADY )return -EINVAL ;
@@ -544,6 +559,11 @@ static int _hardware_dequeue(struct ci_hw_ep *hwep, struct ci_hw_req *hwreq)list_for_each_entry_safe (node, tmpnode, &hwreq->tds, td) { tmptoken =le32_to_cpu (node->ptr->token);if ((TD_STATUS_ACTIVE & tmptoken) !=0 ) { +int n =hw_ep_bit (hwep->num, hwep->dir); + +if (ci->rev ==CI_REVISION_24 ) +if (!hw_read (ci,OP_ENDPTSTAT ,BIT (n))) +reprime_dtd (ci, hwep, node); hwreq->req.status = -EALREADY ;return -EBUSY ; }
Signature · 1 line
1.7.9.5
Quoted 9 lines · Sanchayan Maity
The first two patches add identification register API's. These can be used to get controller's revision.
The third patch implements an errata for revision 2.40a. Not sure which other SOCs implement this version of the Chipidea core but this fixes the usb client issue observed on Vybrids. The patch was tested on a Toradex Colibri VF61 module with the 3.18 kernel. iperf tests ran for three hours plus, with these patches applied have found the USB client connection to be now reliable.
Would you help do a overnight test? It is passed, I will queue them, thanks.
Peter
Quoted 33 lines
This patchset is based off on Shawn Guo's for-next branch
https://git.kernel.org/cgit/linux/kernel/git/shawnguo/linux.git/tree/?h=for-nextThe credit for the patches and fix goes to Matthieu Castet and Peter Chen. First two patches are by Peter Chen and the third patch which fixed the bug we observed was reported by Matthieu Castet.
The discussion of the problem and the relevant testing details can be found at this link: http://www.spinics.net/lists/linux-usb/msg118544.html
The first version of this patchset originally send by Peter Chen can be found at this link: http://www.spinics.net/lists/linux-usb/msg118753.html
Comments for review are welcome :).
Note: I am going on a vacation so will not be able to reply or do any further tests till Monday. Will attend and take care of any comments/requests for further changes/testing from Tuesday. Apologize for the delay in advance.Sanchayan Maity (3):
usb: chipidea: Add identification registers access APIs
usb: chipidea: Add chipidea revision information
usb: chipidea: Add errata for revision 2.40adrivers/usb/chipidea/bits.h | 10 ++++++++ drivers/usb/chipidea/ci.h | 53 +++++++++++++++++++++++++++++++++++++++++++ drivers/usb/chipidea/core.c | 23 +++++++++++++++++-- drivers/usb/chipidea/udc.c | 20 ++++++++++++++++ 4 files changed, 104 insertions(+), 2 deletions(-)Signature · 1 line
1.7.9.5
Signature · 2 lines
Best Regards, Peter Chen
Quoted 11 lines · Peter Chen
Quoted 9 lines · Sanchayan Maity
The first two patches add identification register API's. These can be used to get controller's revision.
The third patch implements an errata for revision 2.40a. Not sure which other SOCs implement this version of the Chipidea core but this fixes the usb client issue observed on Vybrids. The patch was tested on a Toradex Colibri VF61 module with the 3.18 kernel. iperf tests ran for three hours plus, with these patches applied have found the USB client connection to be now reliable.
Quoted 2 lines
Would you help do a overnight test? It is passed, I will queue them, thanks.
Yes definitely I can help with the testing. Are you looking for iperf tests only or something else? iperf tests running for 12 or 24 hours will do? I will need a bit of time to set things up here, as I am away from work, but, ya I will do them.
-Regards,
Sanchayan.
Quoted 36 lines
Peter
Quoted 33 lines
This patchset is based off on Shawn Guo's for-next branch
https://git.kernel.org/cgit/linux/kernel/git/shawnguo/linux.git/tree/?h=for-nextThe credit for the patches and fix goes to Matthieu Castet and Peter Chen. First two patches are by Peter Chen and the third patch which fixed the bug we observed was reported by Matthieu Castet.
The discussion of the problem and the relevant testing details can be found at this link: http://www.spinics.net/lists/linux-usb/msg118544.html
The first version of this patchset originally send by Peter Chen can be found at this link: http://www.spinics.net/lists/linux-usb/msg118753.html
Comments for review are welcome :).
Note: I am going on a vacation so will not be able to reply or do any further tests till Monday. Will attend and take care of any comments/requests for further changes/testing from Tuesday. Apologize for the delay in advance.Sanchayan Maity (3):
usb: chipidea: Add identification registers access APIs
usb: chipidea: Add chipidea revision information
usb: chipidea: Add errata for revision 2.40adrivers/usb/chipidea/bits.h | 10 ++++++++ drivers/usb/chipidea/ci.h | 53 +++++++++++++++++++++++++++++++++++++++++++ drivers/usb/chipidea/core.c | 23 +++++++++++++++++-- drivers/usb/chipidea/udc.c | 20 ++++++++++++++++ 4 files changed, 104 insertions(+), 2 deletions(-)Signature · 1 line
1.7.9.5
Quoted 12 lines · Sanchayan Maity
Quoted 11 lines · Peter Chen
Quoted 9 lines · Sanchayan Maity
The first two patches add identification register API's. These can be used to get controller's revision.
The third patch implements an errata for revision 2.40a. Not sure which other SOCs implement this version of the Chipidea core but this fixes the usb client issue observed on Vybrids. The patch was tested on a Toradex Colibri VF61 module with the 3.18 kernel. iperf tests ran for three hours plus, with these patches applied have found the USB client connection to be now reliable.
Quoted 8 lines
Quoted 2 lines
Would you help do a overnight test? It is passed, I will queue them, thanks.
Yes definitely I can help with the testing. Are you looking for iperf tests only or something else? iperf tests running for 12 or 24 hours will do? I will need a bit of time to set things up here, as I am away from work, but, ya I will do them.
iperf for g_ncm or g_ether and bonnie++ for g_mass_storage if you can run, thanks.
Peter
Quoted 38 lines
-Regards,
Sanchayan.Quoted 36 lines
Peter
Quoted 33 lines
This patchset is based off on Shawn Guo's for-next branch
https://git.kernel.org/cgit/linux/kernel/git/shawnguo/linux.git/tree/?h=for-nextThe credit for the patches and fix goes to Matthieu Castet and Peter Chen. First two patches are by Peter Chen and the third patch which fixed the bug we observed was reported by Matthieu Castet.
The discussion of the problem and the relevant testing details can be found at this link: http://www.spinics.net/lists/linux-usb/msg118544.html
The first version of this patchset originally send by Peter Chen can be found at this link: http://www.spinics.net/lists/linux-usb/msg118753.html
Comments for review are welcome :).
Note: I am going on a vacation so will not be able to reply or do any further tests till Monday. Will attend and take care of any comments/requests for further changes/testing from Tuesday. Apologize for the delay in advance.Sanchayan Maity (3):
usb: chipidea: Add identification registers access APIs
usb: chipidea: Add chipidea revision information
usb: chipidea: Add errata for revision 2.40adrivers/usb/chipidea/bits.h | 10 ++++++++ drivers/usb/chipidea/ci.h | 53 +++++++++++++++++++++++++++++++++++++++++++ drivers/usb/chipidea/core.c | 23 +++++++++++++++++-- drivers/usb/chipidea/udc.c | 20 ++++++++++++++++ 4 files changed, 104 insertions(+), 2 deletions(-)Signature · 1 line
1.7.9.5
Signature · 2 lines
Best Regards, Peter Chen
Quoted 16 lines · Peter Chen
Quoted 15 lines · Sanchayan Maity
Quoted 13 lines · Peter Chen
Quoted 9 lines · Sanchayan Maity
The first two patches add identification register API's. These can be used to get controller's revision.
The third patch implements an errata for revision 2.40a. Not sure which other SOCs implement this version of the Chipidea core but this fixes the usb client issue observed on Vybrids. The patch was tested on a Toradex Colibri VF61 module with the 3.18 kernel. iperf tests ran for three hours plus, with these patches applied have found the USB client connection to be now reliable.
Would you help do a overnight test? It is passed, I will queue them, thanks.
Quoted 8 lines
Quoted 5 lines
Yes definitely I can help with the testing. Are you looking for iperf tests only or something else? iperf tests running for 12 or 24 hours will do? I will need a bit of time to set things up here, as I am away from work, but, ya I will do them.
iperf for
g_ncmorg_etherand bonnie++ forg_mass_storageif you can run, thanks.
Will run 12 hour iperf test for CDC ECM. I am not aware of bonnie++, but, will look into it. Will get back once I have the results.
Thanks.
Regards,
Sanchayan.
Quoted 42 lines
Peter
Quoted 38 lines
-Regards,
Sanchayan.Quoted 36 lines
Peter
Quoted 33 lines
This patchset is based off on Shawn Guo's for-next branch
https://git.kernel.org/cgit/linux/kernel/git/shawnguo/linux.git/tree/?h=for-nextThe credit for the patches and fix goes to Matthieu Castet and Peter Chen. First two patches are by Peter Chen and the third patch which fixed the bug we observed was reported by Matthieu Castet.
The discussion of the problem and the relevant testing details can be found at this link: http://www.spinics.net/lists/linux-usb/msg118544.html
The first version of this patchset originally send by Peter Chen can be found at this link: http://www.spinics.net/lists/linux-usb/msg118753.html
Comments for review are welcome :).
Note: I am going on a vacation so will not be able to reply or do any further tests till Monday. Will attend and take care of any comments/requests for further changes/testing from Tuesday. Apologize for the delay in advance.Sanchayan Maity (3):
usb: chipidea: Add identification registers access APIs
usb: chipidea: Add chipidea revision information
usb: chipidea: Add errata for revision 2.40adrivers/usb/chipidea/bits.h | 10 ++++++++ drivers/usb/chipidea/ci.h | 53 +++++++++++++++++++++++++++++++++++++++++++ drivers/usb/chipidea/core.c | 23 +++++++++++++++++-- drivers/usb/chipidea/udc.c | 20 ++++++++++++++++ 4 files changed, 104 insertions(+), 2 deletions(-)Signature · 1 line
1.7.9.5
Quoted 16 lines · Peter Chen
Quoted 15 lines · Sanchayan Maity
Quoted 13 lines · Peter Chen
Quoted 9 lines · Sanchayan Maity
The first two patches add identification register API's. These can be used to get controller's revision.
The third patch implements an errata for revision 2.40a. Not sure which other SOCs implement this version of the Chipidea core but this fixes the usb client issue observed on Vybrids. The patch was tested on a Toradex Colibri VF61 module with the 3.18 kernel. iperf tests ran for three hours plus, with these patches applied have found the USB client connection to be now reliable.
Would you help do a overnight test? It is passed, I will queue them, thanks.
Quoted 8 lines
Quoted 5 lines
Yes definitely I can help with the testing. Are you looking for iperf tests only or something else? iperf tests running for 12 or 24 hours will do? I will need a bit of time to set things up here, as I am away from work, but, ya I will do them.
iperf for
g_ncmorg_etherand bonnie++ forg_mass_storageif you can run, thanks.
The tests were run on a Toradex Colibri Vybrid VF61 module having 256MB RAM with the 3.18 kernel.
The iperf tests ran for around 19 hours before I stopped it. A snip of the iperf tests is below. Used the Ethernet Gadget class for this.
[ 4] 70453.0-70453.5 sec 6.89 MBytes 116 Mbits/sec [ 4] 70453.5-70454.0 sec 6.83 MBytes 115 Mbits/sec [ 4] 70454.0-70454.5 sec 6.84 MBytes 115 Mbits/sec [ 4] 70454.5-70455.0 sec 6.89 MBytes 116 Mbits/sec [ 4] 70455.0-70455.5 sec 6.90 MBytes 116 Mbits/sec [ 4] 70455.5-70456.0 sec 6.90 MBytes 116 Mbits/sec [ 4] 70456.0-70456.5 sec 6.82 MBytes 114 Mbits/sec [ 4] 70456.5-70457.0 sec 6.80 MBytes 114 Mbits/sec [ 4] 70457.0-70457.5 sec 6.89 MBytes 116 Mbits/sec [ 4] 70457.5-70458.0 sec 6.85 MBytes 115 Mbits/sec [ 4] 70458.0-70458.5 sec 6.82 MBytes 114 Mbits/sec [ 4] 70458.5-70459.0 sec 6.82 MBytes 114 Mbits/sec [ 4] 0.0-70459.2 sec 946 GBytes 115 Mbits/sec
Ran bonnie++ on gadget mass storage. CPU usage around the time of running this test was mostly around the 90% mark with the minimum at 60% plus. The storage directory was formatted with ext4. bonnie++ version used is 1.97 and was installed from the Arch repositories with pacman.
The size of the file being specified for "lun" storage is 512MB. I have specified 128MB RAM in the below run with the size of file for IO performance as 256MB. Without this bonnie++ was giving me an error around the "Writing intelligently" point. I assume this has to do with the file size bonnie++ uses for testing.
[sanchayan@Sanchayan-Arch ~]$ sudo /usr/bin/bonnie++ -m Vybrid -r 128 -d /var/run/media/sanchayan/Vybrid/ -x 5 -u root -n 0 -s 256
Using uid:0, gid:0. format_version,bonnie_version,name,concurrency,seed,file_size,io_chunk_size,putc,putc_cpu,put_block,put_block_cpu,rewrite,rewrite_cpu,getc,getc_cpu,get_block,get_block_cpu,seeks,seeks_cpu,num_files,max_size,min_size,num_dirs,file_chunk_size,seq_create,seq_create_cpu,seq_stat,seq_stat_cpu,seq_del,seq_del_cpu,ran_create,ran_create_cpu,ran_stat,ran_stat_cpu,ran_del,ran_del_cpu,putc_latency,put_block_latency,rewrite_latency,getc_latency,get_block_latency,seeks_latency,seq_create_latency,seq_stat_latency,seq_del_latency,ran_create_latency,ran_stat_latency,ran_del_latency Writing a byte at a time...done
Writing intelligently...done
Rewriting...done
Reading a byte at a time...done
Reading intelligently...done
start 'em...done...done...done...done...done...
1.97,1.97,Vybrid,1,1419409300,256M,,659,87,8341,1,9401,0,4222,98,+++++,+++,3539,19,,,,,,,,,,,,,,,,,,23042us,66us,59us,4482us,79us,475us,,,,,,
Writing a byte at a time...done
Writing intelligently...done
Rewriting...done
Reading a byte at a time...done
Reading intelligently...done
start 'em...done...done...done...done...done...
1.97,1.97,Vybrid,1,1419409300,256M,,661,90,7689,1,9071,0,4011,99,+++++,+++,3426,20,,,,,,,,,,,,,,,,,,15406us,64us,62us,4667us,23us,10030us,,,,,,
Writing a byte at a time...done
Writing intelligently...done
Rewriting...done
Reading a byte at a time...done
Reading intelligently...done
start 'em...done...done...done...done...done...
1.97,1.97,Vybrid,1,1419409300,256M,,673,89,8117,1,9451,0,3879,98,+++++,+++,3355,22,,,,,,,,,,,,,,,,,,14210us,45us,69us,5069us,21us,10052us,,,,,,
Writing a byte at a time...done
Writing intelligently...done
Rewriting...done
Reading a byte at a time...done
Reading intelligently...done
start 'em...done...done...done...done...done...
1.97,1.97,Vybrid,1,1419409300,256M,,668,89,7801,1,9343,0,4099,98,+++++,+++,3336,16,,,,,,,,,,,,,,,,,,17019us,44us,75us,4920us,20us,10234us,,,,,,
Writing a byte at a time...done
Writing intelligently...done
Rewriting...done
Reading a byte at a time...done
Reading intelligently...done
start 'em...done...done...done...done...done...
1.97,1.97,Vybrid,1,1419409300,256M,,676,89,7953,1,9494,0,3878,98,+++++,+++,3396,22,,,,,,,,,,,,,,,,,,14080us,56us,42us,5177us,23us,12224us,,,,,,
Let me know if this is OK.
Thanks.
-Regards,
Sanchayan.
Quoted 42 lines
Peter
Quoted 38 lines
-Regards,
Sanchayan.Quoted 36 lines
Peter
Quoted 33 lines
This patchset is based off on Shawn Guo's for-next branch
https://git.kernel.org/cgit/linux/kernel/git/shawnguo/linux.git/tree/?h=for-nextThe credit for the patches and fix goes to Matthieu Castet and Peter Chen. First two patches are by Peter Chen and the third patch which fixed the bug we observed was reported by Matthieu Castet.
The discussion of the problem and the relevant testing details can be found at this link: http://www.spinics.net/lists/linux-usb/msg118544.html
The first version of this patchset originally send by Peter Chen can be found at this link: http://www.spinics.net/lists/linux-usb/msg118753.html
Comments for review are welcome :).
Note: I am going on a vacation so will not be able to reply or do any further tests till Monday. Will attend and take care of any comments/requests for further changes/testing from Tuesday. Apologize for the delay in advance.Sanchayan Maity (3):
usb: chipidea: Add identification registers access APIs
usb: chipidea: Add chipidea revision information
usb: chipidea: Add errata for revision 2.40adrivers/usb/chipidea/bits.h | 10 ++++++++ drivers/usb/chipidea/ci.h | 53 +++++++++++++++++++++++++++++++++++++++++++ drivers/usb/chipidea/core.c | 23 +++++++++++++++++-- drivers/usb/chipidea/udc.c | 20 ++++++++++++++++ 4 files changed, 104 insertions(+), 2 deletions(-)Signature · 1 line
1.7.9.5
Quoted 53 lines · Peter Chen
Quoted 16 lines · Sanchayan Maity
Quoted 15 lines · Peter Chen
Quoted 13 lines · Sanchayan Maity
Quoted 9 lines
The first two patches add identification register API's. These can be used to get controller's revision.
The third patch implements an errata for revision 2.40a. Not sure which other SOCs implement this version of the Chipidea core but this fixes the usb client issue observed on Vybrids. The patch was tested on a Toradex Colibri VF61 module with the 3.18 kernel. iperf tests ran for three hours plus, with these patches applied have found the USB client connection to be now reliable.
Would you help do a overnight test? It is passed, I will queue them, thanks.
Quoted 8 lines
Quoted 5 lines
Yes definitely I can help with the testing. Are you looking for iperf tests only or something else? iperf tests running for 12 or 24 hours will do? I will need a bit of time to set things up here, as I am away from work, but, ya I will do them.
iperf for
g_ncmorg_etherand bonnie++ forg_mass_storageif you can run, thanks.The tests were run on a Toradex Colibri Vybrid VF61 module having 256MB RAM with the 3.18 kernel.
The iperf tests ran for around 19 hours before I stopped it. A snip of the iperf tests is below. Used the Ethernet Gadget class for this.
[ 4] 70453.0-70453.5 sec 6.89 MBytes 116 Mbits/sec [ 4] 70453.5-70454.0 sec 6.83 MBytes 115 Mbits/sec [ 4] 70454.0-70454.5 sec 6.84 MBytes 115 Mbits/sec [ 4] 70454.5-70455.0 sec 6.89 MBytes 116 Mbits/sec [ 4] 70455.0-70455.5 sec 6.90 MBytes 116 Mbits/sec [ 4] 70455.5-70456.0 sec 6.90 MBytes 116 Mbits/sec [ 4] 70456.0-70456.5 sec 6.82 MBytes 114 Mbits/sec [ 4] 70456.5-70457.0 sec 6.80 MBytes 114 Mbits/sec [ 4] 70457.0-70457.5 sec 6.89 MBytes 116 Mbits/sec [ 4] 70457.5-70458.0 sec 6.85 MBytes 115 Mbits/sec [ 4] 70458.0-70458.5 sec 6.82 MBytes 114 Mbits/sec [ 4] 70458.5-70459.0 sec 6.82 MBytes 114 Mbits/sec [ 4] 0.0-70459.2 sec 946 GBytes 115 Mbits/secRan bonnie++ on gadget mass storage. CPU usage around the time of running this test was mostly around the 90% mark with the minimum at 60% plus. The storage directory was formatted with ext4. bonnie++ version used is 1.97 and was installed from the Arch repositories with pacman.
Quoted 5 lines
The size of the file being specified for "lun" storage is 512MB. I have specified 128MB RAM in the below run with the size of file for IO performance as 256MB. Without this bonnie++ was giving me an error around the "Writing intelligently" point. I assume this has to do with the file size bonnie++ uses for testing.
What's your backfile for mass storage, mmc?
You mean without this errata, running bonnie++ will meet error?
Quoted 51 lines
[sanchayan@Sanchayan-Arch ~]$ sudo
/usr/bin/bonnie++-m Vybrid -r 128 -d/var/run/media/sanchayan/Vybrid/-x 5 -u root -n 0 -s 256 Using uid:0, gid:0. format_version,bonnie_version,name,concurrency,seed,file_size,io_chunk_si ze,putc,putc_cpu,put_block,put_block_cpu,rewrite,rewrite_cpu,getc,getc_cp u,get_block,get_block_cpu,seeks,seeks_cpu,num_files,max_size,min_size,nu m_dirs,file_chunk_size,seq_create,seq_create_cpu,seq_stat,seq_stat_cpu,se q_del,seq_del_cpu,ran_create,ran_create_cpu,ran_stat,ran_stat_cpu,ran_del ,ran_del_cpu,putc_latency,put_block_latency,rewrite_latency,getc_latency,ge t_block_latency,seeks_latency,seq_create_latency,seq_stat_latency,seq_del_ latency,ran_create_latency,ran_stat_latency,ran_del_latency Writing a byte at a time...done
Writing intelligently...done
Rewriting...done
Reading a byte at a time...done
Reading intelligently...done
start 'em...done...done...done...done...done... 1.97,1.97,Vybrid,1,1419409300,256M,,659,87,8341,1,9401,0,4222,98,+++++,+++, 3539,19,,,,,,,,,,,,,,,,,,23042us,66us,59us,4482us,79us,475us,,,,,, Writing a byte at a time...done
Writing intelligently...done
Rewriting...done
Reading a byte at a time...done
Reading intelligently...done
start 'em...done...done...done...done...done... 1.97,1.97,Vybrid,1,1419409300,256M,,661,90,7689,1,9071,0,4011,99,+++++,+++, 3426,20,,,,,,,,,,,,,,,,,,15406us,64us,62us,4667us,23us,10030us,,,,,, Writing a byte at a time...done
Writing intelligently...done
Rewriting...done
Reading a byte at a time...done
Reading intelligently...done
start 'em...done...done...done...done...done... 1.97,1.97,Vybrid,1,1419409300,256M,,673,89,8117,1,9451,0,3879,98,+++++,+++, 3355,22,,,,,,,,,,,,,,,,,,14210us,45us,69us,5069us,21us,10052us,,,,,, Writing a byte at a time...done
Writing intelligently...done
Rewriting...done
Reading a byte at a time...done
Reading intelligently...done
start 'em...done...done...done...done...done... 1.97,1.97,Vybrid,1,1419409300,256M,,668,89,7801,1,9343,0,4099,98,+++++,+++, 3336,16,,,,,,,,,,,,,,,,,,17019us,44us,75us,4920us,20us,10234us,,,,,, Writing a byte at a time...done
Writing intelligently...done
Rewriting...done
Reading a byte at a time...done
Reading intelligently...done
start 'em...done...done...done...done...done... 1.97,1.97,Vybrid,1,1419409300,256M,,676,89,7953,1,9494,0,3878,98,+++++,+++, 3396,22,,,,,,,,,,,,,,,,,,14080us,56us,42us,5177us,23us,12224us,,,,,,
Quoted 1 line
Let me know if this is OK.
Thanks for making and testing the patch, it is really a good job. I will queue this patch in my chipidea tree.
Peter
Quoted 53 lines · Peter Chen
Quoted 53 lines · Peter Chen
Quoted 16 lines · Sanchayan Maity
Quoted 15 lines · Peter Chen
Quoted 13 lines · Sanchayan Maity
Quoted 9 lines
The first two patches add identification register API's. These can be used to get controller's revision.
The third patch implements an errata for revision 2.40a. Not sure which other SOCs implement this version of the Chipidea core but this fixes the usb client issue observed on Vybrids. The patch was tested on a Toradex Colibri VF61 module with the 3.18 kernel. iperf tests ran for three hours plus, with these patches applied have found the USB client connection to be now reliable.
Would you help do a overnight test? It is passed, I will queue them, thanks.
Quoted 8 lines
Quoted 5 lines
Yes definitely I can help with the testing. Are you looking for iperf tests only or something else? iperf tests running for 12 or 24 hours will do? I will need a bit of time to set things up here, as I am away from work, but, ya I will do them.
iperf for
g_ncmorg_etherand bonnie++ forg_mass_storageif you can run, thanks.The tests were run on a Toradex Colibri Vybrid VF61 module having 256MB RAM with the 3.18 kernel.
The iperf tests ran for around 19 hours before I stopped it. A snip of the iperf tests is below. Used the Ethernet Gadget class for this.
[ 4] 70453.0-70453.5 sec 6.89 MBytes 116 Mbits/sec [ 4] 70453.5-70454.0 sec 6.83 MBytes 115 Mbits/sec [ 4] 70454.0-70454.5 sec 6.84 MBytes 115 Mbits/sec [ 4] 70454.5-70455.0 sec 6.89 MBytes 116 Mbits/sec [ 4] 70455.0-70455.5 sec 6.90 MBytes 116 Mbits/sec [ 4] 70455.5-70456.0 sec 6.90 MBytes 116 Mbits/sec [ 4] 70456.0-70456.5 sec 6.82 MBytes 114 Mbits/sec [ 4] 70456.5-70457.0 sec 6.80 MBytes 114 Mbits/sec [ 4] 70457.0-70457.5 sec 6.89 MBytes 116 Mbits/sec [ 4] 70457.5-70458.0 sec 6.85 MBytes 115 Mbits/sec [ 4] 70458.0-70458.5 sec 6.82 MBytes 114 Mbits/sec [ 4] 70458.5-70459.0 sec 6.82 MBytes 114 Mbits/sec [ 4] 0.0-70459.2 sec 946 GBytes 115 Mbits/secRan bonnie++ on gadget mass storage. CPU usage around the time of running this test was mostly around the 90% mark with the minimum at 60% plus. The storage directory was formatted with ext4. bonnie++ version used is 1.97 and was installed from the Arch repositories with pacman.
Quoted 7 lines
Quoted 5 lines
The size of the file being specified for "lun" storage is 512MB. I have specified 128MB RAM in the below run with the size of file for IO performance as 256MB. Without this bonnie++ was giving me an error around the "Writing intelligently" point. I assume this has to do with the file size bonnie++ uses for testing.
What's your backfile for mass storage, mmc?
The backfile for this mass storage is a "file.img" in the /home/root directory created with the dd command as below
dd if=/dev/zero of=file.img bs=1024 count=0 seek=$[1024*512]
The script I use for activating mass storage is as below:
/bin/mount none /mnt -t configfs/bin/mkdir /mnt/usb_gadget/g1
cd /mnt/usb_gadget/g1/bin/mkdir configs/c.1/bin/mkdir functions/mass_storage.0
echo /home/root/file.img > functions/mass_storage.0/lun.0/file /bin/mkdir strings/0x409/bin/mkdir configs/c.1/strings/0x409
echo 0x006a > idProduct
echo 0x15a2 > idVendor
echo Freescale123 > strings/0x409/serialnumber
echo Freescale > strings/0x409/manufacturer
echo "Mass Storage Gadget" > strings/0x409/product echo "Conf 1" > configs/c.1/strings/0x409/configuration echo 200 > configs/c.1/MaxPower
ln -s functions/mass_storage.0 configs/c.1
echo ci_hdrc.0 > UDC
Quoted 1 line
You mean without this errata, running bonnie++ will meet error?
No, I didn't mean that. I meant if I do something like
sudo /usr/bin/bonnie++ -m Vybrid -r 256 -d /var/run/media/sanchayan/Vybrid/ -x 5 -u root -n 0
then this gives me an error. Not aware of the bonnie++ internals, but, I guess this would be because bonnie++ uses some higher multiple of the block size to write and the storage is much lesser than that. The exact output for this case is as below
[sanchayan@Sanchayan-Arch ~]$ sudo /usr/bin/bonnie++ -m Vybrid -r 256 -d /var/run/media/sanchayan/Vybrid/ -x 5 -u root -n 0
[sudo] password for sanchayan:
Using uid:0, gid:0. format_version,bonnie_version,name,concurrency,seed,file_size,io_chunk_size,putc,putc_cpu,put_block,put_block_cpu,rewrite,rewrite_cpu,getc,getc_cpu,get_block,get_block_cpu,seeks,seeks_cpu,num_files,max_size,min_size,num_dirs,file_chunk_size,seq_create,seq_create_cpu,seq_stat,seq_stat_cpu,seq_del,seq_del_cpu,ran_create,ran_create_cpu,ran_stat,ran_stat_cpu,ran_del,ran_del_cpu,putc_latency,put_block_latency,rewrite_latency,getc_latency,get_block_latency,seeks_latency,seq_create_latency,seq_stat_latency,seq_del_latency,ran_create_latency,ran_stat_latency,ran_del_latency Writing a byte at a time...done
Writing intelligently...Can't write block.: No such file or directory
Can't write block 61508.
Quoted 52 lines
Quoted 52 lines
[sanchayan@Sanchayan-Arch ~]$ sudo
/usr/bin/bonnie++-m Vybrid -r 128 -d/var/run/media/sanchayan/Vybrid/-x 5 -u root -n 0 -s 256 Using uid:0, gid:0. format_version,bonnie_version,name,concurrency,seed,file_size,io_chunk_si ze,putc,putc_cpu,put_block,put_block_cpu,rewrite,rewrite_cpu,getc,getc_cp u,get_block,get_block_cpu,seeks,seeks_cpu,num_files,max_size,min_size,nu m_dirs,file_chunk_size,seq_create,seq_create_cpu,seq_stat,seq_stat_cpu,se q_del,seq_del_cpu,ran_create,ran_create_cpu,ran_stat,ran_stat_cpu,ran_del ,ran_del_cpu,putc_latency,put_block_latency,rewrite_latency,getc_latency,ge t_block_latency,seeks_latency,seq_create_latency,seq_stat_latency,seq_del_ latency,ran_create_latency,ran_stat_latency,ran_del_latency Writing a byte at a time...done
Writing intelligently...done
Rewriting...done
Reading a byte at a time...done
Reading intelligently...done
start 'em...done...done...done...done...done... 1.97,1.97,Vybrid,1,1419409300,256M,,659,87,8341,1,9401,0,4222,98,+++++,+++, 3539,19,,,,,,,,,,,,,,,,,,23042us,66us,59us,4482us,79us,475us,,,,,, Writing a byte at a time...done
Writing intelligently...done
Rewriting...done
Reading a byte at a time...done
Reading intelligently...done
start 'em...done...done...done...done...done... 1.97,1.97,Vybrid,1,1419409300,256M,,661,90,7689,1,9071,0,4011,99,+++++,+++, 3426,20,,,,,,,,,,,,,,,,,,15406us,64us,62us,4667us,23us,10030us,,,,,, Writing a byte at a time...done
Writing intelligently...done
Rewriting...done
Reading a byte at a time...done
Reading intelligently...done
start 'em...done...done...done...done...done... 1.97,1.97,Vybrid,1,1419409300,256M,,673,89,8117,1,9451,0,3879,98,+++++,+++, 3355,22,,,,,,,,,,,,,,,,,,14210us,45us,69us,5069us,21us,10052us,,,,,, Writing a byte at a time...done
Writing intelligently...done
Rewriting...done
Reading a byte at a time...done
Reading intelligently...done
start 'em...done...done...done...done...done... 1.97,1.97,Vybrid,1,1419409300,256M,,668,89,7801,1,9343,0,4099,98,+++++,+++, 3336,16,,,,,,,,,,,,,,,,,,17019us,44us,75us,4920us,20us,10234us,,,,,, Writing a byte at a time...done
Writing intelligently...done
Rewriting...done
Reading a byte at a time...done
Reading intelligently...done
start 'em...done...done...done...done...done... 1.97,1.97,Vybrid,1,1419409300,256M,,676,89,7953,1,9494,0,3878,98,+++++,+++, 3396,22,,,,,,,,,,,,,,,,,,14080us,56us,42us,5177us,23us,12224us,,,,,,
Quoted 5 lines
Quoted 1 line
Let me know if this is OK.
Thanks for making and testing the patch, it is really a good job. I will queue this patch in my chipidea tree.
Thanks :). Happy to help.
-Sanchayan.
Quoted 2 lines
Peter
Stefan Agner
Tested-byQuoted 11 lines · Peter Chen
Quoted 9 lines · Sanchayan Maity
The first two patches add identification register API's. These can be used to get controller's revision.
The third patch implements an errata for revision 2.40a. Not sure which other SOCs implement this version of the Chipidea core but this fixes the usb client issue observed on Vybrids. The patch was tested on a Toradex Colibri VF61 module with the 3.18 kernel. iperf tests ran for three hours plus, with these patches applied have found the USB client connection to be now reliable.
Quoted 3 lines
Would you help do a overnight test? It is passed, I will queue them, thanks.
Hi Peter,
Also did a short test on 3.19-rc1 here:
# iperf -s ------------------------------------------------------------ Server listening on TCP port 5001 TCP window size: 85.3 KByte (default) ------------------------------------------------------------ [ 4] local 192.168.1.10 port 5001 connected with 192.168.1.1 port 53241 [ ID] Interval Transfer Bandwidth [ 4] 0.0-10.0 sec 152 MBytes 127 Mbits/sec [ 5] local 192.168.1.10 port 5001 connected with 192.168.1.1 port 53243 [ 8] local 192.168.1.10 port 5001 connected with 192.168.1.1 port 53247 [ 7] local 192.168.1.10 port 5001 connected with 192.168.1.1 port 53246 [ 6] local 192.168.1.10 port 5001 connected with 192.168.1.1 port 53245 [ 4] local 192.168.1.10 port 5001 connected with 192.168.1.1 port 53244 [ 6] 0.0-10.1 sec 30.9 MBytes 25.7 Mbits/sec [ 5] 0.0-10.1 sec 30.9 MBytes 25.7 Mbits/sec [ 7] 0.0-10.1 sec 30.9 MBytes 25.7 Mbits/sec [ 8] 0.0-10.1 sec 30.9 MBytes 25.6 Mbits/sec [ 4] 0.0-10.1 sec 30.9 MBytes 25.7 Mbits/sec [SUM] 0.0-10.1 sec 154 MBytes 128 Mbits/sec
Tried also UDP mode and scp copies as well as adding some kernel level latencies by pluging and unpluging a USB mass storage device. Looks good to me as well:
Btw, this was and solves now the error I was mentioning in my patchset enabling Vybrid support:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/274342.html
@Sanchayan/Peter:
Minor nits as reply to the actual patch emails.
--
Stefan
Quoted 34 lines
Peter
Quoted 33 lines
This patchset is based off on Shawn Guo's for-next branch
https://git.kernel.org/cgit/linux/kernel/git/shawnguo/linux.git/tree/?h=for-nextThe credit for the patches and fix goes to Matthieu Castet and Peter Chen. First two patches are by Peter Chen and the third patch which fixed the bug we observed was reported by Matthieu Castet.
The discussion of the problem and the relevant testing details can be found at this link: http://www.spinics.net/lists/linux-usb/msg118544.html
The first version of this patchset originally send by Peter Chen can be found at this link: http://www.spinics.net/lists/linux-usb/msg118753.html
Comments for review are welcome :).
Note: I am going on a vacation so will not be able to reply or do any further tests till Monday. Will attend and take care of any comments/requests for further changes/testing from Tuesday. Apologize for the delay in advance.Sanchayan Maity (3):
usb: chipidea: Add identification registers access APIs
usb: chipidea: Add chipidea revision information
usb: chipidea: Add errata for revision 2.40adrivers/usb/chipidea/bits.h | 10 ++++++++ drivers/usb/chipidea/ci.h | 53 +++++++++++++++++++++++++++++++++++++++++++ drivers/usb/chipidea/core.c | 23 +++++++++++++++++-- drivers/usb/chipidea/udc.c | 20 ++++++++++++++++ 4 files changed, 104 insertions(+), 2 deletions(-)--
1.7.9.5