We want the rcvPhyPkt bit in LinkControl off before we start using the
chip. However, the spec says that the reset value of it is undefined.
Hence switch it explicitly off.
https://bugzilla.redhat.com/show_bug.cgi?id=244576#c48 shows that for
example the nForce2 integrated FireWire controller seems to have it on
by default.
Signed-off-by: Stefan Richter <[email protected]>
---
Supersedes patch "fw-ohci: clear LinkControl register when enabling the
chip". Changed from ~0 to OHCI1394_LinkControl_rcvPhyPkt bit only.
drivers/firewire/fw-ohci.c | 2 ++
1 file changed, 2 insertions(+)
Index: linux/drivers/firewire/fw-ohci.c
===================================================================
--- linux.orig/drivers/firewire/fw-ohci.c
+++ linux/drivers/firewire/fw-ohci.c
@@ -1473,6 +1473,8 @@ static int ohci_enable(struct fw_card *c
reg_write(ohci, OHCI1394_HCControlClear,
OHCI1394_HCControl_noByteSwapData);
+ reg_write(ohci, OHCI1394_LinkControlClear,
+ OHCI1394_LinkControl_rcvPhyPkt);
reg_write(ohci, OHCI1394_LinkControlSet,
OHCI1394_LinkControl_rcvSelfID |
OHCI1394_LinkControl_cycleTimerEnable |
--
Stefan Richter
-=====-==--- -==- --=-=
http://arcgraph.de/sr/
OHCI 1.1 clause 5.10 requires that selfIDBufferPtr is valid when a 1 is
written into LinkControl.rcvSelfID.
This driver bug has so far not been known to cause harm because most
chips obviously accept a later selfIDBufferPtr write.
Signed-off-by: Stefan Richter <[email protected]>
---
drivers/firewire/fw-ohci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux/drivers/firewire/fw-ohci.c
===================================================================
--- linux.orig/drivers/firewire/fw-ohci.c
+++ linux/drivers/firewire/fw-ohci.c
@@ -1473,6 +1473,7 @@ static int ohci_enable(struct fw_card *c
reg_write(ohci, OHCI1394_HCControlClear,
OHCI1394_HCControl_noByteSwapData);
+ reg_write(ohci, OHCI1394_SelfIDBuffer, ohci->self_id_bus);
reg_write(ohci, OHCI1394_LinkControlClear,
OHCI1394_LinkControl_rcvPhyPkt);
reg_write(ohci, OHCI1394_LinkControlSet,
@@ -1488,7 +1489,6 @@ static int ohci_enable(struct fw_card *c
ar_context_run(&ohci->ar_request_ctx);
ar_context_run(&ohci->ar_response_ctx);
- reg_write(ohci, OHCI1394_SelfIDBuffer, ohci->self_id_bus);
reg_write(ohci, OHCI1394_PhyUpperBound, 0x00010000);
reg_write(ohci, OHCI1394_IntEventClear, ~0);
reg_write(ohci, OHCI1394_IntMaskClear, ~0);
--
Stefan Richter
-=====-==--- -==- --=-=
http://arcgraph.de/sr/
On Thursday 05 June 2008 02:49:38 pm Stefan Richter wrote:
> We want the rcvPhyPkt bit in LinkControl off before we start using the
> chip. However, the spec says that the reset value of it is undefined.
> Hence switch it explicitly off.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=244576#c48 shows that for
> example the nForce2 integrated FireWire controller seems to have it on
> by default.
>
> Signed-off-by: Stefan Richter <[email protected]>
Good catch.
Signed-off-by: Jarod Wilson <[email protected]>
--
Jarod Wilson
[email protected]
On Thursday 05 June 2008 02:50:53 pm Stefan Richter wrote:
> OHCI 1.1 clause 5.10 requires that selfIDBufferPtr is valid when a 1 is
> written into LinkControl.rcvSelfID.
>
> This driver bug has so far not been known to cause harm because most
> chips obviously accept a later selfIDBufferPtr write.
>
> Signed-off-by: Stefan Richter <[email protected]>
I vaguely recall discussing this one before. Doesn't seem to have caused any
problems in practice, but yeah, should go ahead and be more spec compliant
here.
Signed-off-by: Jarod Wilson <[email protected]>
--
Jarod Wilson
[email protected]
On Thu, 2008-06-05 at 20:50 +0200, Stefan Richter wrote:
> OHCI 1.1 clause 5.10 requires that selfIDBufferPtr is valid when a 1 is
> written into LinkControl.rcvSelfID.
>
> This driver bug has so far not been known to cause harm because most
> chips obviously accept a later selfIDBufferPtr write.
>
> Signed-off-by: Stefan Richter <[email protected]>
Yeah, good point. I think it also caused no harm because we set it up
before enabling the link. But your change definitely looks good.
Signed-off-by: Kristian Høgsberg <[email protected]>