* [PATCH 0/3] usb: chipidea: add one errata for revision 2.40a
@ 2014-12-19 9:55 Sanchayan Maity
2014-12-19 9:55 ` [PATCH 1/3] usb: chipidea: Add identification registers access APIs Sanchayan Maity
` (3 more replies)
0 siblings, 4 replies; 20+ messages in thread
From: Sanchayan Maity @ 2014-12-19 9:55 UTC (permalink / raw)
To: Peter.Chen; +Cc: stefan, gregkh, linux-usb, linux-kernel, 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.
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(-)
--
1.7.9.5
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/3] usb: chipidea: Add identification registers access APIs
2014-12-19 9:55 [PATCH 0/3] usb: chipidea: add one errata for revision 2.40a Sanchayan Maity
@ 2014-12-19 9:55 ` Sanchayan Maity
2014-12-24 16:00 ` Stefan Agner
2014-12-19 9:55 ` [PATCH 2/3] usb: chipidea: Add chipidea revision information Sanchayan Maity
` (2 subsequent siblings)
3 siblings, 1 reply; 20+ messages in thread
From: Sanchayan Maity @ 2014-12-19 9:55 UTC (permalink / raw)
To: Peter.Chen; +Cc: stefan, gregkh, linux-usb, linux-kernel, Sanchayan Maity
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.
Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
---
drivers/usb/chipidea/ci.h | 53 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index ea40626..94db636 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -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
--
1.7.9.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/3] usb: chipidea: Add chipidea revision information
2014-12-19 9:55 [PATCH 0/3] usb: chipidea: add one errata for revision 2.40a Sanchayan Maity
2014-12-19 9:55 ` [PATCH 1/3] usb: chipidea: Add identification registers access APIs Sanchayan Maity
@ 2014-12-19 9:55 ` Sanchayan Maity
2014-12-24 16:22 ` Stefan Agner
2014-12-19 9:55 ` [PATCH 3/3] usb: chipidea: Add errata for revision 2.40a Sanchayan Maity
2014-12-22 1:18 ` [PATCH 0/3] usb: chipidea: add one " Peter Chen
3 siblings, 1 reply; 20+ messages in thread
From: Sanchayan Maity @ 2014-12-19 9:55 UTC (permalink / raw)
To: Peter.Chen; +Cc: stefan, gregkh, linux-usb, linux-kernel, Sanchayan Maity
Define 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.
Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
---
drivers/usb/chipidea/bits.h | 10 ++++++++++
drivers/usb/chipidea/core.c | 23 +++++++++++++++++++++--
2 files changed, 31 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/chipidea/bits.h b/drivers/usb/chipidea/bits.h
index ca57e3d..e935ccc 100644
--- a/drivers/usb/chipidea/bits.h
+++ b/drivers/usb/chipidea/bits.h
@@ -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)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 9bdc6bd..33a8c4a 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -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 ? */
--
1.7.9.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/3] usb: chipidea: Add errata for revision 2.40a
2014-12-19 9:55 [PATCH 0/3] usb: chipidea: add one errata for revision 2.40a Sanchayan Maity
2014-12-19 9:55 ` [PATCH 1/3] usb: chipidea: Add identification registers access APIs Sanchayan Maity
2014-12-19 9:55 ` [PATCH 2/3] usb: chipidea: Add chipidea revision information Sanchayan Maity
@ 2014-12-19 9:55 ` Sanchayan Maity
2014-12-22 1:18 ` [PATCH 0/3] usb: chipidea: add one " Peter Chen
3 siblings, 0 replies; 20+ messages in thread
From: Sanchayan Maity @ 2014-12-19 9:55 UTC (permalink / raw)
To: Peter.Chen; +Cc: stefan, gregkh, linux-usb, linux-kernel, Sanchayan Maity
At 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.
Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
---
drivers/usb/chipidea/udc.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 0444d3f..551ab37 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -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;
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 0/3] usb: chipidea: add one errata for revision 2.40a
2014-12-19 9:55 [PATCH 0/3] usb: chipidea: add one errata for revision 2.40a Sanchayan Maity
` (2 preceding siblings ...)
2014-12-19 9:55 ` [PATCH 3/3] usb: chipidea: Add errata for revision 2.40a Sanchayan Maity
@ 2014-12-22 1:18 ` Peter Chen
2014-12-22 13:09 ` Sanchayan Maity
2014-12-24 15:42 ` Stefan Agner
3 siblings, 2 replies; 20+ messages in thread
From: Peter Chen @ 2014-12-22 1:18 UTC (permalink / raw)
To: Sanchayan Maity; +Cc: stefan, gregkh, linux-usb, linux-kernel
On Fri, Dec 19, 2014 at 03:25:26PM +0530, Sanchayan Maity wrote:
> 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
>
> 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(-)
>
> --
> 1.7.9.5
>
--
Best Regards,
Peter Chen
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/3] usb: chipidea: add one errata for revision 2.40a
2014-12-22 1:18 ` [PATCH 0/3] usb: chipidea: add one " Peter Chen
@ 2014-12-22 13:09 ` Sanchayan Maity
2014-12-23 0:09 ` Peter Chen
2014-12-24 15:42 ` Stefan Agner
1 sibling, 1 reply; 20+ messages in thread
From: Sanchayan Maity @ 2014-12-22 13:09 UTC (permalink / raw)
To: Peter Chen; +Cc: stefan, gregkh, linux-usb, linux-kernel
On 12/22/2014 06:48 AM, Peter Chen wrote:
> On Fri, Dec 19, 2014 at 03:25:26PM +0530, Sanchayan Maity wrote:
>> 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.
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.
>
> Peter
>>
>> 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(-)
>>
>> --
>> 1.7.9.5
>>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/3] usb: chipidea: add one errata for revision 2.40a
2014-12-22 13:09 ` Sanchayan Maity
@ 2014-12-23 0:09 ` Peter Chen
2014-12-23 4:11 ` Sanchayan Maity
2014-12-24 7:50 ` Sanchayan Maity
0 siblings, 2 replies; 20+ messages in thread
From: Peter Chen @ 2014-12-23 0:09 UTC (permalink / raw)
To: Sanchayan Maity; +Cc: stefan, gregkh, linux-usb, linux-kernel
On Mon, Dec 22, 2014 at 06:39:42PM +0530, Sanchayan Maity wrote:
> On 12/22/2014 06:48 AM, Peter Chen wrote:
> > On Fri, Dec 19, 2014 at 03:25:26PM +0530, Sanchayan Maity wrote:
> >> 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.
>
> 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
> -Regards,
> Sanchayan.
> >
> > Peter
> >>
> >> 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(-)
> >>
> >> --
> >> 1.7.9.5
> >>
> >
--
Best Regards,
Peter Chen
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/3] usb: chipidea: add one errata for revision 2.40a
2014-12-23 0:09 ` Peter Chen
@ 2014-12-23 4:11 ` Sanchayan Maity
2014-12-24 7:50 ` Sanchayan Maity
1 sibling, 0 replies; 20+ messages in thread
From: Sanchayan Maity @ 2014-12-23 4:11 UTC (permalink / raw)
To: Peter Chen; +Cc: stefan, gregkh, linux-usb, linux-kernel
On 12/23/2014 05:39 AM, Peter Chen wrote:
> On Mon, Dec 22, 2014 at 06:39:42PM +0530, Sanchayan Maity wrote:
>> On 12/22/2014 06:48 AM, Peter Chen wrote:
>>> On Fri, Dec 19, 2014 at 03:25:26PM +0530, Sanchayan Maity wrote:
>>>> 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.
>>
>> 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.
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.
>
> Peter
>
>> -Regards,
>> Sanchayan.
>>>
>>> Peter
>>>>
>>>> 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(-)
>>>>
>>>> --
>>>> 1.7.9.5
>>>>
>>>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/3] usb: chipidea: add one errata for revision 2.40a
2014-12-23 0:09 ` Peter Chen
2014-12-23 4:11 ` Sanchayan Maity
@ 2014-12-24 7:50 ` Sanchayan Maity
2014-12-24 9:00 ` Peter Chen
1 sibling, 1 reply; 20+ messages in thread
From: Sanchayan Maity @ 2014-12-24 7:50 UTC (permalink / raw)
To: Peter Chen; +Cc: stefan, gregkh, linux-usb, linux-kernel
On 12/23/2014 05:39 AM, Peter Chen wrote:
> On Mon, Dec 22, 2014 at 06:39:42PM +0530, Sanchayan Maity wrote:
>> On 12/22/2014 06:48 AM, Peter Chen wrote:
>>> On Fri, Dec 19, 2014 at 03:25:26PM +0530, Sanchayan Maity wrote:
>>>> 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.
>>
>> 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.
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.
>
> Peter
>
>> -Regards,
>> Sanchayan.
>>>
>>> Peter
>>>>
>>>> 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(-)
>>>>
>>>> --
>>>> 1.7.9.5
>>>>
>>>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH 0/3] usb: chipidea: add one errata for revision 2.40a
2014-12-24 7:50 ` Sanchayan Maity
@ 2014-12-24 9:00 ` Peter Chen
2014-12-24 9:20 ` Sanchayan Maity
0 siblings, 1 reply; 20+ messages in thread
From: Peter Chen @ 2014-12-24 9:00 UTC (permalink / raw)
To: Sanchayan Maity; +Cc: stefan, gregkh, linux-usb, linux-kernel
>
> On 12/23/2014 05:39 AM, Peter Chen wrote:
> > On Mon, Dec 22, 2014 at 06:39:42PM +0530, Sanchayan Maity wrote:
> >> On 12/22/2014 06:48 AM, Peter Chen wrote:
> >>> On Fri, Dec 19, 2014 at 03:25:26PM +0530, Sanchayan Maity wrote:
> >>>> 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.
> >>
> >> 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.
>
> 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.
>
What's your backfile for mass storage, mmc?
You mean without this errata, running bonnie++ will meet error?
> [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,,,,,,
>
> 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
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/3] usb: chipidea: add one errata for revision 2.40a
2014-12-24 9:00 ` Peter Chen
@ 2014-12-24 9:20 ` Sanchayan Maity
0 siblings, 0 replies; 20+ messages in thread
From: Sanchayan Maity @ 2014-12-24 9:20 UTC (permalink / raw)
To: Peter Chen; +Cc: stefan, gregkh, linux-usb, linux-kernel
On 12/24/2014 02:30 PM, Peter Chen wrote:
>
>>
>> On 12/23/2014 05:39 AM, Peter Chen wrote:
>>> On Mon, Dec 22, 2014 at 06:39:42PM +0530, Sanchayan Maity wrote:
>>>> On 12/22/2014 06:48 AM, Peter Chen wrote:
>>>>> On Fri, Dec 19, 2014 at 03:25:26PM +0530, Sanchayan Maity wrote:
>>>>>> 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.
>>>>
>>>> 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.
>>
>> 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.
>>
>
> 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
> 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.
>
>> [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,,,,,,
>>
>> 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.
> Peter
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/3] usb: chipidea: add one errata for revision 2.40a
2014-12-22 1:18 ` [PATCH 0/3] usb: chipidea: add one " Peter Chen
2014-12-22 13:09 ` Sanchayan Maity
@ 2014-12-24 15:42 ` Stefan Agner
1 sibling, 0 replies; 20+ messages in thread
From: Stefan Agner @ 2014-12-24 15:42 UTC (permalink / raw)
To: Peter Chen; +Cc: Sanchayan Maity, gregkh, linux-usb, linux-kernel
On 2014-12-22 02:18, Peter Chen wrote:
> On Fri, Dec 19, 2014 at 03:25:26PM +0530, Sanchayan Maity wrote:
>> 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.
>
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:
Tested-by: Stefan Agner <stefan@agner.ch>
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
> Peter
>>
>> 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(-)
>>
>> --
>> 1.7.9.5
>>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] usb: chipidea: Add identification registers access APIs
2014-12-19 9:55 ` [PATCH 1/3] usb: chipidea: Add identification registers access APIs Sanchayan Maity
@ 2014-12-24 16:00 ` Stefan Agner
2014-12-24 16:15 ` Sanchayan Maity
2014-12-25 2:03 ` Peter Chen
0 siblings, 2 replies; 20+ messages in thread
From: Stefan Agner @ 2014-12-24 16:00 UTC (permalink / raw)
To: Sanchayan Maity; +Cc: Peter.Chen, gregkh, linux-usb, linux-kernel
On 2014-12-19 10:55, Sanchayan Maity wrote:
> 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.
Checkpatch is complaining about DOS line endings and some trailing
whitespace. This applies to all three patches.
>
> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
> ---
> drivers/usb/chipidea/ci.h | 53 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 53 insertions(+)
>
> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> index ea40626..94db636 100644
> --- a/drivers/usb/chipidea/ci.h
> +++ b/drivers/usb/chipidea/ci.h
> @@ -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 {
Usually the enum names are small caps, only the labels are capitalized.
I would suggest use ci_revision here (similar to the enum above).
> + 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);
> +}
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
> +
> +/**
> * hw_read: reads from a hw register
> * @ci: the controller
> * @reg: register index
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] usb: chipidea: Add identification registers access APIs
2014-12-24 16:00 ` Stefan Agner
@ 2014-12-24 16:15 ` Sanchayan Maity
2014-12-24 16:41 ` Stefan Agner
2014-12-25 2:13 ` Peter Chen
2014-12-25 2:03 ` Peter Chen
1 sibling, 2 replies; 20+ messages in thread
From: Sanchayan Maity @ 2014-12-24 16:15 UTC (permalink / raw)
To: Stefan Agner; +Cc: Peter.Chen, gregkh, linux-usb, linux-kernel
On 12/24/2014 09:30 PM, Stefan Agner wrote:
> On 2014-12-19 10:55, Sanchayan Maity wrote:
>> 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.
>
> 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
>
>>
>> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
>> ---
>> drivers/usb/chipidea/ci.h | 53 +++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 53 insertions(+)
>>
>> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
>> index ea40626..94db636 100644
>> --- a/drivers/usb/chipidea/ci.h
>> +++ b/drivers/usb/chipidea/ci.h
>> @@ -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 {
>
> Usually the enum names are small caps, only the labels are capitalized.
> I would suggest use ci_revision here (similar to the enum above).
>
>> + 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);
>> +}
>
> 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
>
>> +
>> +/**
>> * hw_read: reads from a hw register
>> * @ci: the controller
>> * @reg: register index
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] usb: chipidea: Add chipidea revision information
2014-12-19 9:55 ` [PATCH 2/3] usb: chipidea: Add chipidea revision information Sanchayan Maity
@ 2014-12-24 16:22 ` Stefan Agner
2014-12-25 2:30 ` Peter Chen
0 siblings, 1 reply; 20+ messages in thread
From: Stefan Agner @ 2014-12-24 16:22 UTC (permalink / raw)
To: Sanchayan Maity; +Cc: Peter.Chen, gregkh, linux-usb, linux-kernel
On 2014-12-19 10:55, Sanchayan Maity wrote:
> Define 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.
>
> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
> ---
> drivers/usb/chipidea/bits.h | 10 ++++++++++
> drivers/usb/chipidea/core.c | 23 +++++++++++++++++++++--
> 2 files changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/chipidea/bits.h b/drivers/usb/chipidea/bits.h
> index ca57e3d..e935ccc 100644
> --- a/drivers/usb/chipidea/bits.h
> +++ b/drivers/usb/chipidea/bits.h
> @@ -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)
> +
Hm, the other defines use spaces in this file, I like tabs more too, but
I guess uniformity wins here.. Peter?
> /* HCCPARAMS */
> #define HCCPARAMS_LEN BIT(17)
>
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 9bdc6bd..33a8c4a 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -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);
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);
> + 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 ? */
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] usb: chipidea: Add identification registers access APIs
2014-12-24 16:15 ` Sanchayan Maity
@ 2014-12-24 16:41 ` Stefan Agner
2014-12-25 2:13 ` Peter Chen
1 sibling, 0 replies; 20+ messages in thread
From: Stefan Agner @ 2014-12-24 16:41 UTC (permalink / raw)
To: Sanchayan Maity; +Cc: Peter.Chen, gregkh, linux-usb, linux-kernel
On 2014-12-24 17:15, Sanchayan Maity wrote:
> On 12/24/2014 09:30 PM, Stefan Agner wrote:
>> On 2014-12-19 10:55, Sanchayan Maity wrote:
>>> 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.
>>
>> 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
> I will resend if somehow errors crept in with my earlier patches.
>
> -Sanchayan
>
>>
>>>
>>> Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
>>> ---
>>> drivers/usb/chipidea/ci.h | 53 +++++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 53 insertions(+)
>>>
>>> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
>>> index ea40626..94db636 100644
>>> --- a/drivers/usb/chipidea/ci.h
>>> +++ b/drivers/usb/chipidea/ci.h
>>> @@ -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 {
>>
>> Usually the enum names are small caps, only the labels are capitalized.
>> I would suggest use ci_revision here (similar to the enum above).
>>
>>> + 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);
>>> +}
>>
>> 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
>>
>>> +
>>> +/**
>>> * hw_read: reads from a hw register
>>> * @ci: the controller
>>> * @reg: register index
>>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] usb: chipidea: Add identification registers access APIs
2014-12-24 16:00 ` Stefan Agner
2014-12-24 16:15 ` Sanchayan Maity
@ 2014-12-25 2:03 ` Peter Chen
1 sibling, 0 replies; 20+ messages in thread
From: Peter Chen @ 2014-12-25 2:03 UTC (permalink / raw)
To: Stefan Agner; +Cc: Sanchayan Maity, gregkh, linux-usb, linux-kernel
On Wed, Dec 24, 2014 at 05:00:05PM +0100, Stefan Agner wrote:
> On 2014-12-19 10:55, Sanchayan Maity wrote:
> > 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.
>
> Checkpatch is complaining about DOS line endings and some trailing
> whitespace. This applies to all three patches.
>
> >
> > Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
> > ---
> > drivers/usb/chipidea/ci.h | 53 +++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 53 insertions(+)
> >
> > diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> > index ea40626..94db636 100644
> > --- a/drivers/usb/chipidea/ci.h
> > +++ b/drivers/usb/chipidea/ci.h
> > @@ -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 {
>
> Usually the enum names are small caps, only the labels are capitalized.
> I would suggest use ci_revision here (similar to the enum above).
I will change it.
>
> > + 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);
> > +}
>
> 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.
--
Best Regards,
Peter Chen
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] usb: chipidea: Add identification registers access APIs
2014-12-24 16:15 ` Sanchayan Maity
2014-12-24 16:41 ` Stefan Agner
@ 2014-12-25 2:13 ` Peter Chen
2014-12-25 3:33 ` Sanchayan Maity
1 sibling, 1 reply; 20+ messages in thread
From: Peter Chen @ 2014-12-25 2:13 UTC (permalink / raw)
To: Sanchayan Maity; +Cc: Stefan Agner, gregkh, linux-usb, linux-kernel
On Wed, Dec 24, 2014 at 09:45:31PM +0530, Sanchayan Maity wrote:
> On 12/24/2014 09:30 PM, Stefan Agner wrote:
> > On 2014-12-19 10:55, Sanchayan Maity wrote:
> >> 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.
> >
> > 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
>
Hi Sanchayan,
I will take care the first two, you are the third if there is any
comments, thanks.
--
Best Regards,
Peter Chen
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] usb: chipidea: Add chipidea revision information
2014-12-24 16:22 ` Stefan Agner
@ 2014-12-25 2:30 ` Peter Chen
0 siblings, 0 replies; 20+ messages in thread
From: Peter Chen @ 2014-12-25 2:30 UTC (permalink / raw)
To: Stefan Agner; +Cc: Sanchayan Maity, gregkh, linux-usb, linux-kernel
On Wed, Dec 24, 2014 at 05:22:44PM +0100, Stefan Agner wrote:
> On 2014-12-19 10:55, Sanchayan Maity wrote:
> > Define 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.
> >
> > Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
> > ---
> > drivers/usb/chipidea/bits.h | 10 ++++++++++
> > drivers/usb/chipidea/core.c | 23 +++++++++++++++++++++--
> > 2 files changed, 31 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/chipidea/bits.h b/drivers/usb/chipidea/bits.h
> > index ca57e3d..e935ccc 100644
> > --- a/drivers/usb/chipidea/bits.h
> > +++ b/drivers/usb/chipidea/bits.h
> > @@ -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)
> > +
>
> 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.
>
> > /* HCCPARAMS */
> > #define HCCPARAMS_LEN BIT(17)
> >
> > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> > index 9bdc6bd..33a8c4a 100644
> > --- a/drivers/usb/chipidea/core.c
> > +++ b/drivers/usb/chipidea/core.c
> > @@ -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);
>
> 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);
> 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.
--
Best Regards,
Peter Chen
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] usb: chipidea: Add identification registers access APIs
2014-12-25 2:13 ` Peter Chen
@ 2014-12-25 3:33 ` Sanchayan Maity
0 siblings, 0 replies; 20+ messages in thread
From: Sanchayan Maity @ 2014-12-25 3:33 UTC (permalink / raw)
To: Peter Chen; +Cc: Stefan Agner, gregkh, linux-usb, linux-kernel
On 12/25/2014 07:43 AM, Peter Chen wrote:
> On Wed, Dec 24, 2014 at 09:45:31PM +0530, Sanchayan Maity wrote:
>> On 12/24/2014 09:30 PM, Stefan Agner wrote:
>>> On 2014-12-19 10:55, Sanchayan Maity wrote:
>>>> 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.
>>>
>>> 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
>>
>
> 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.
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2014-12-25 3:39 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-19 9:55 [PATCH 0/3] usb: chipidea: add one errata for revision 2.40a Sanchayan Maity
2014-12-19 9:55 ` [PATCH 1/3] usb: chipidea: Add identification registers access APIs Sanchayan Maity
2014-12-24 16:00 ` Stefan Agner
2014-12-24 16:15 ` Sanchayan Maity
2014-12-24 16:41 ` Stefan Agner
2014-12-25 2:13 ` Peter Chen
2014-12-25 3:33 ` Sanchayan Maity
2014-12-25 2:03 ` Peter Chen
2014-12-19 9:55 ` [PATCH 2/3] usb: chipidea: Add chipidea revision information Sanchayan Maity
2014-12-24 16:22 ` Stefan Agner
2014-12-25 2:30 ` Peter Chen
2014-12-19 9:55 ` [PATCH 3/3] usb: chipidea: Add errata for revision 2.40a Sanchayan Maity
2014-12-22 1:18 ` [PATCH 0/3] usb: chipidea: add one " Peter Chen
2014-12-22 13:09 ` Sanchayan Maity
2014-12-23 0:09 ` Peter Chen
2014-12-23 4:11 ` Sanchayan Maity
2014-12-24 7:50 ` Sanchayan Maity
2014-12-24 9:00 ` Peter Chen
2014-12-24 9:20 ` Sanchayan Maity
2014-12-24 15:42 ` Stefan Agner
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.