[PATCH] firewire: fw-ohci: clear LinkControl register when enabling the chip

[PATCH] firewire: fw-ohci: clear LinkControl register when enabling the chip

[Stefan Richter]

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 (and all other bits while we are at 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 <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/fw-ohci.c |    1 +
 1 file changed, 1 insertion(+)

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_LinkControlClear, ~0);
 	reg_write(ohci, OHCI1394_LinkControlSet,
 		  OHCI1394_LinkControl_rcvSelfID |
 		  OHCI1394_LinkControl_cycleTimerEnable |

-- 
Stefan Richter
-=====-==--- -==- ----=
http://arcgraph.de/sr/

[PATCH] firewire: fw-ohci: disable PHY packet reception into AR context

[Stefan Richter]

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 <stefanr@s5r6.in-berlin.de>
---

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/

[PATCH] firewire: fw-ohci: write selfIDBufferPtr before LinkControl.rcvSelfID

[Stefan Richter]

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 <stefanr@s5r6.in-berlin.de>
---
 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/

Re: [PATCH] firewire: fw-ohci: disable PHY packet reception into AR context

[Jarod Wilson]

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 <stefanr@s5r6.in-berlin.de>

Good catch.

Signed-off-by: Jarod Wilson <jwilson@redhat.com>

-- 
Jarod Wilson
jwilson@redhat.com

Re: [PATCH] firewire: fw-ohci: write selfIDBufferPtr before LinkControl.rcvSelfID

[Jarod Wilson]

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 <stefanr@s5r6.in-berlin.de>

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 <jwilson@redhat.com>

-- 
Jarod Wilson
jwilson@redhat.com

Re: [PATCH] firewire: fw-ohci: write selfIDBufferPtr before LinkControl.rcvSelfID

[Kristian Høgsberg]

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 <stefanr@s5r6.in-berlin.de>

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 <krh@redhat.com>