RE: [PATCH 2/3 v2] usb: chipidea: Fix Internal error: : 808 [#1] ARM related to STS flag
{
u32 portsc, lpm, sts = 0;
switch (ci-platdata-phy_mode) {
case USBPHY_INTERFACE_MODE_UTMI:
portsc = PORTSC_PTS(PTS_UTMI);
@@ -273,10 +275,12 @@ static void hw_phymode_configure(struct ci_hdrc
*ci)
if (ci-hw_bank.lpm) {
hw_write(ci, OP_DEVLC, DEVLC_PTS(7) | DEVLC_PTW, lpm);
- hw_write(ci, OP_DEVLC, DEVLC_STS, sts);
+ if (sts)
+ hw_write(ci, OP_DEVLC, DEVLC_STS, sts);
} else {
hw_write(ci, OP_PORTSC, PORTSC_PTS(7) | PORTSC_PTW, portsc);
- hw_write(ci, OP_PORTSC, PORTSC_STS, sts);
+ if ( sts )
+ hw_write(ci, OP_PORTSC, PORTSC_STS, sts);
The conditions coding style is broken.
}
}
Still don't get why a system with ehci compliant PORTSC register
should not want to have the sts bit _explicitly_ set to 0 if
we don't use serial phy mode. So NACK!
At current code the PORTSC_STS is set to unknown value due to sts is
uninitialized, and it causes ARM internal error at Chris's imx27 platform.
This patch just gives sts an initialized value, and only set PORTSC_STS
for serial phy, it follows the code original logic mostly.
Until now, no one reports problem due to the wrong value of PORTSC_STS, it means
both 1 and 0 are working for most of platforms. So, just keep its current logic
until someone blames.
The chipidea datasheet said This bit has no effect unless Parallel Transceiver
Select is set to UTMI+.,
my imx6 uses UTIM+, but the controller works fine whether PORTSC_STS is 0 or 1.
My original idea is just delete the code about PORTSC_STS, but chris
mentions it is used by serial phy at current logic, then, just keep current
logic.
Peter
Michael
--
Pengutronix e.K. |
|
Industrial Linux Solutions | http://www.pengutronix.de/
|
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0
|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-
|
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/3 v2] usb: chipidea: Fix Internal error: : 808 [#1] ARM related to STS flag
Best regards,
Peter Chen
-Original Message-
From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-
ow...@vger.kernel.org] On Behalf Of Chris Ruehl
Sent: Friday, November 29, 2013 3:20 PM
To: alexander.shish...@linux.intel.com; gre...@linuxfoundation.org
Cc: linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org; Chris Ruehl
Subject: [PATCH 2/3 v2] usb: chipidea: Fix Internal error: : 808 [#1] ARM
related to STS flag
usb: chipidea: Fix Internal error: : 808 [#1] ARM related to STS flag
* init the sts flag to 0 (missed)
* set the sts flag only if not 0
Signed-off-by: Chris Ruehl chris.ru...@gtsys.com.hk
---
drivers/usb/chipidea/core.c |8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 5075407..1a6010e 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -245,6 +245,8 @@ static void hw_phymode_configure(struct ci_hdrc *ci)
{
u32 portsc, lpm, sts = 0;
switch (ci-platdata-phy_mode) {
case USBPHY_INTERFACE_MODE_UTMI:
portsc = PORTSC_PTS(PTS_UTMI);
@@ -273,10 +275,12 @@ static void hw_phymode_configure(struct ci_hdrc *ci)
if (ci-hw_bank.lpm) {
hw_write(ci, OP_DEVLC, DEVLC_PTS(7) | DEVLC_PTW, lpm);
- hw_write(ci, OP_DEVLC, DEVLC_STS, sts);
+ if (sts)
+ hw_write(ci, OP_DEVLC, DEVLC_STS, sts);
} else {
hw_write(ci, OP_PORTSC, PORTSC_PTS(7) | PORTSC_PTW, portsc);
- hw_write(ci, OP_PORTSC, PORTSC_STS, sts);
+ if ( sts )
+ hw_write(ci, OP_PORTSC, PORTSC_STS, sts);
}
}
--
Acked-by: Peter Chen peter.c...@freescale.com
Peter
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3 v2] usb: chipidea: Fix Internal error: : 808 [#1] ARM related to STS flag
On Fri, Nov 29, 2013 at 03:19:45PM +0800, Chris Ruehl wrote:
usb: chipidea: Fix Internal error: : 808 [#1] ARM related to STS flag
* init the sts flag to 0 (missed)
* set the sts flag only if not 0
Signed-off-by: Chris Ruehl chris.ru...@gtsys.com.hk
---
drivers/usb/chipidea/core.c |8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 5075407..1a6010e 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -245,6 +245,8 @@ static void hw_phymode_configure(struct ci_hdrc *ci)
{
u32 portsc, lpm, sts = 0;
switch (ci-platdata-phy_mode) {
case USBPHY_INTERFACE_MODE_UTMI:
portsc = PORTSC_PTS(PTS_UTMI);
@@ -273,10 +275,12 @@ static void hw_phymode_configure(struct ci_hdrc *ci)
if (ci-hw_bank.lpm) {
hw_write(ci, OP_DEVLC, DEVLC_PTS(7) | DEVLC_PTW, lpm);
- hw_write(ci, OP_DEVLC, DEVLC_STS, sts);
+ if (sts)
+ hw_write(ci, OP_DEVLC, DEVLC_STS, sts);
} else {
hw_write(ci, OP_PORTSC, PORTSC_PTS(7) | PORTSC_PTW, portsc);
- hw_write(ci, OP_PORTSC, PORTSC_STS, sts);
+ if ( sts )
+ hw_write(ci, OP_PORTSC, PORTSC_STS, sts);
The conditions coding style is broken.
}
}
Still don't get why a system with ehci compliant PORTSC register
should not want to have the sts bit _explicitly_ set to 0 if
we don't use serial phy mode. So NACK!
Michael
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3 v2] usb: chipidea: Fix Internal error: : 808 [#1] ARM related to STS flag
On Saturday, November 30, 2013 02:28 AM, Michael Grzeschik wrote:
On Fri, Nov 29, 2013 at 03:19:45PM +0800, Chris Ruehl wrote:
usb: chipidea: Fix Internal error: : 808 [#1] ARM related to STS flag
* init the sts flag to 0 (missed)
* set the sts flag only if not 0
Signed-off-by: Chris Ruehlchris.ru...@gtsys.com.hk
---
drivers/usb/chipidea/core.c |8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 5075407..1a6010e 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -245,6 +245,8 @@ static void hw_phymode_configure(struct ci_hdrc *ci)
{
u32 portsc, lpm, sts = 0;
switch (ci-platdata-phy_mode) {
case USBPHY_INTERFACE_MODE_UTMI:
portsc = PORTSC_PTS(PTS_UTMI);
@@ -273,10 +275,12 @@ static void hw_phymode_configure(struct ci_hdrc *ci)
if (ci-hw_bank.lpm) {
hw_write(ci, OP_DEVLC, DEVLC_PTS(7) | DEVLC_PTW, lpm);
- hw_write(ci, OP_DEVLC, DEVLC_STS, sts);
+ if (sts)
+ hw_write(ci, OP_DEVLC, DEVLC_STS, sts);
} else {
hw_write(ci, OP_PORTSC, PORTSC_PTS(7) | PORTSC_PTW, portsc);
- hw_write(ci, OP_PORTSC, PORTSC_STS, sts);
+ if ( sts )
+ hw_write(ci, OP_PORTSC, PORTSC_STS, sts);
The conditions coding style is broken.
}
}
Still don't get why a system with ehci compliant PORTSC register
should not want to have the sts bit _explicitly_ set to 0 if
we don't use serial phy mode. So NACK!
Michael
Michael,
I agree that this patch is not sufficient. I had a look in the reference manual
for the imx27 and here:
Serial Transceiver Select—Read/Write. This register bit is used in conjunction
with the configuration constant
VUSB_HS_PHY_SERIAL to control whether the parallel or serial transceiver
interface is selected for FS and LS
operation. The Serial Interface Engine can be used in combination with the UTMI+
or ULPI physical interface to
provide FS/LS signaling instead of the parallel interface. If VUSB_HS_PHY_SERIAL
is set for 0 or 1 then this
bit is read only. If VUSB_HS_PHY_SERIAL is 3 or 4 then this bit is read/write.
This bit has no effect unless Parallel Transceiver Select is set to UTMI+ or
ULPI. The Serial/1.1 physical interface
will use the Serial Interface Engine for FS/LS signaling regardless of this bit
value.
Note: This bit is reserved for future operation and is a placeholder adding
dynamic use of the serial engine in
accord with UMTI+ and ULPI characterization logic.
This bit is not defined in the EHCI specification.
To make it short. In some VUSB_HS_PHY_SERIAL configurations the STS is a READ
ONLY and should not be written. The current code (without my wrong patch) always
write to the STS!
With the patch there was no write and no oops with Internal 808 ARM exception..
Let me rework it.
Regards
Chris
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3 v2] usb: chipidea: Fix Internal error: : 808 [#1] ARM related to STS flag
On Saturday, November 30, 2013 02:28 AM, Michael Grzeschik wrote:
On Fri, Nov 29, 2013 at 03:19:45PM +0800, Chris Ruehl wrote:
usb: chipidea: Fix Internal error: : 808 [#1] ARM related to STS flag
* init the sts flag to 0 (missed)
* set the sts flag only if not 0
Signed-off-by: Chris Ruehlchris.ru...@gtsys.com.hk
---
drivers/usb/chipidea/core.c |8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 5075407..1a6010e 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -245,6 +245,8 @@ static void hw_phymode_configure(struct ci_hdrc *ci)
{
u32 portsc, lpm, sts = 0;
switch (ci-platdata-phy_mode) {
case USBPHY_INTERFACE_MODE_UTMI:
portsc = PORTSC_PTS(PTS_UTMI);
@@ -273,10 +275,12 @@ static void hw_phymode_configure(struct ci_hdrc *ci)
if (ci-hw_bank.lpm) {
hw_write(ci, OP_DEVLC, DEVLC_PTS(7) | DEVLC_PTW, lpm);
- hw_write(ci, OP_DEVLC, DEVLC_STS, sts);
+ if (sts)
+ hw_write(ci, OP_DEVLC, DEVLC_STS, sts);
} else {
hw_write(ci, OP_PORTSC, PORTSC_PTS(7) | PORTSC_PTW, portsc);
- hw_write(ci, OP_PORTSC, PORTSC_STS, sts);
+ if ( sts )
+ hw_write(ci, OP_PORTSC, PORTSC_STS, sts);
The conditions coding style is broken.
}
}
Still don't get why a system with ehci compliant PORTSC register
should not want to have the sts bit _explicitly_ set to 0 if
we don't use serial phy mode. So NACK!
Michael
That's happen when I remove the if() statements and unconditional write the sts
flag
[1.128482] ehci-mxc: Freescale On-Chip EHCI Host driver
[1.136702] usbcore: registered new interface driver usb-storage
[1.147594] imx_usb 10024000.usb: dummy supplies not allowed
[1.154803] Unhandled fault: external abort on non-linefetch (0x808) at
0xf4424184
[1.162424] Internal error: : 808 [#1] ARM
[1.166548] Modules linked in:
[1.169670] CPU: 0 PID: 1 Comm: swapper Not tainted
3.13.0-rc1-next-20131125-dirty #44
[1.177629] task: cf85 ti: cf852000 task.ti: cf852000
[1.183086] PC is at ci_hdrc_probe+0x250/0x630
[1.187582] LR is at console_unlock+0x2d4/0x30c
[1.192165] pc : [c025f7a8]lr : [c003ec50]psr: 6013
regards
Chris
--
GTSYS Limited RFID Technology
A01 24/F Gold King Industrial Bld
35-41 Tai Lin Pai Road, Kwai Chung, Hong Kong
Fax (852) 8167 4060 - Tel (852) 3598 9488
Disclaimer: http://www.gtsys.com.hk/email/classified.html
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3 v2] usb: chipidea: Fix Internal error: : 808 [#1] ARM related to STS flag
On Saturday, November 30, 2013 02:28 AM, Michael Grzeschik wrote:
On Fri, Nov 29, 2013 at 03:19:45PM +0800, Chris Ruehl wrote:
usb: chipidea: Fix Internal error: : 808 [#1] ARM related to STS flag
* init the sts flag to 0 (missed)
* set the sts flag only if not 0
Signed-off-by: Chris Ruehlchris.ru...@gtsys.com.hk
---
drivers/usb/chipidea/core.c |8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 5075407..1a6010e 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -245,6 +245,8 @@ static void hw_phymode_configure(struct ci_hdrc *ci)
{
u32 portsc, lpm, sts = 0;
switch (ci-platdata-phy_mode) {
case USBPHY_INTERFACE_MODE_UTMI:
portsc = PORTSC_PTS(PTS_UTMI);
@@ -273,10 +275,12 @@ static void hw_phymode_configure(struct ci_hdrc *ci)
if (ci-hw_bank.lpm) {
hw_write(ci, OP_DEVLC, DEVLC_PTS(7) | DEVLC_PTW, lpm);
- hw_write(ci, OP_DEVLC, DEVLC_STS, sts);
+ if (sts)
+ hw_write(ci, OP_DEVLC, DEVLC_STS, sts);
} else {
hw_write(ci, OP_PORTSC, PORTSC_PTS(7) | PORTSC_PTW, portsc);
- hw_write(ci, OP_PORTSC, PORTSC_STS, sts);
+ if ( sts )
+ hw_write(ci, OP_PORTSC, PORTSC_STS, sts);
The conditions coding style is broken.
}
}
Still don't get why a system with ehci compliant PORTSC register
should not want to have the sts bit _explicitly_ set to 0 if
we don't use serial phy mode. So NACK!
Michael
Guys,,, I must be blind
hw_write(ci, OP_PORTSC, PORTSC_STS, sts);
sts has a wrong value, its should be (sts 29) when sts = 1
same for lpm value (lpm 28)
Still needs to check the value of PTS, if PTS 1 or sts flag is read only.
if (ci-hw_bank.lpm) {
hw_write(ci, OP_DEVLC, DEVLC_PTS(7) | DEVLC_PTW, lpm);
if ( sts )
hw_write(ci, OP_DEVLC, DEVLC_STS, BIT(28));
else
hw_write(ci, OP_DEVLC, DEVLC_STS, ~BIT(28));
} else {
hw_write(ci, OP_PORTSC, PORTSC_PTS(7) | PORTSC_PTW, portsc);
if (((portsc 30) 0x3) 1) { /* check if STS is read
only */
if (sts) {
hw_write(ci, OP_PORTSC, PORTSC_STS, BIT(29));
}
else {
portsc =
(ioread32(ci-hw_bank.regmap[OP_PORTSC])
BIT(29));
if (portsc) /* sts needs reset */
hw_write(ci, OP_PORTSC, PORTSC_STS,
~BIT(29));
}
}
}
With a write to an already sts = 0 my kernel oops 808. so check if the sts is 0
and if 0 then no write.
Can someone have a look for the lpm stuff ?
Michael, thanks for the NOACK ;-) otherwise
Regards
Chris
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3 v2] usb: chipidea: Fix Internal error: : 808 [#1] ARM related to STS flag
usb: chipidea: Fix Internal error: : 808 [#1] ARM related to STS flag
* init the sts flag to 0 (missed)
* set the sts flag only if not 0
Signed-off-by: Chris Ruehl chris.ru...@gtsys.com.hk
---
drivers/usb/chipidea/core.c |8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 5075407..1a6010e 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -245,6 +245,8 @@ static void hw_phymode_configure(struct ci_hdrc *ci)
{
u32 portsc, lpm, sts = 0;
switch (ci-platdata-phy_mode) {
case USBPHY_INTERFACE_MODE_UTMI:
portsc = PORTSC_PTS(PTS_UTMI);
@@ -273,10 +275,12 @@ static void hw_phymode_configure(struct ci_hdrc *ci)
if (ci-hw_bank.lpm) {
hw_write(ci, OP_DEVLC, DEVLC_PTS(7) | DEVLC_PTW, lpm);
- hw_write(ci, OP_DEVLC, DEVLC_STS, sts);
+ if (sts)
+ hw_write(ci, OP_DEVLC, DEVLC_STS, sts);
} else {
hw_write(ci, OP_PORTSC, PORTSC_PTS(7) | PORTSC_PTW, portsc);
- hw_write(ci, OP_PORTSC, PORTSC_STS, sts);
+ if ( sts )
+ hw_write(ci, OP_PORTSC, PORTSC_STS, sts);
}
}
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
