From dd6254e5c0efe01ad255188898cb3dadf98cb56d Mon Sep 17 00:00:00 2001 From: Clemens Ladisch Date: Mon, 16 May 2011 08:10:10 +0200 Subject: firewire: ohci: remove superfluous posted write flushes The call to flush_writes() in context_stop() is superfluous because another register read is done immediately afterwards. The call to flush_writes() in ar_context_run() does not need to be done individually for each AR context, so move it to ohci_enable(). This also makes ohci_enable() clearer because it no longer depends on a side effect of ar_context_run() to flush its own register writes. Finally, the setting of a context's wake bit does not need to be flushed because neither the driver logic nor the API require the CPU to wait for this action. This removes the last MMIO reads from the packet queueing code paths. Signed-off-by: Clemens Ladisch Signed-off-by: Stefan Richter --- drivers/firewire/ohci.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) (limited to 'drivers/firewire/ohci.c') diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 438e6c831170..e291edaa5eef 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -629,7 +629,6 @@ static void ar_context_link_page(struct ar_context *ctx, unsigned int index) ctx->last_buffer_index = index; reg_write(ctx->ohci, CONTROL_SET(ctx->regs), CONTEXT_WAKE); - flush_writes(ctx->ohci); } static void ar_context_release(struct ar_context *ctx) @@ -1001,7 +1000,6 @@ static void ar_context_run(struct ar_context *ctx) reg_write(ctx->ohci, COMMAND_PTR(ctx->regs), ctx->descriptors_bus | 1); reg_write(ctx->ohci, CONTROL_SET(ctx->regs), CONTEXT_RUN); - flush_writes(ctx->ohci); } static struct descriptor *find_branch_descriptor(struct descriptor *d, int z) @@ -1201,7 +1199,6 @@ static void context_stop(struct context *ctx) reg_write(ctx->ohci, CONTROL_CLEAR(ctx->regs), CONTEXT_RUN); ctx->running = false; - flush_writes(ctx->ohci); for (i = 0; i < 10; i++) { reg = reg_read(ctx->ohci, CONTROL_SET(ctx->regs)); @@ -1345,12 +1342,10 @@ static int at_context_queue_packet(struct context *ctx, context_append(ctx, d, z, 4 - z); - if (ctx->running) { + if (ctx->running) reg_write(ohci, CONTROL_SET(ctx->regs), CONTEXT_WAKE); - flush_writes(ohci); - } else { + else context_run(ctx, 0); - } return 0; } @@ -2196,7 +2191,9 @@ static int ohci_enable(struct fw_card *card, OHCI1394_LinkControl_rcvPhyPkt); ar_context_run(&ohci->ar_request_ctx); - ar_context_run(&ohci->ar_response_ctx); /* also flushes writes */ + ar_context_run(&ohci->ar_response_ctx); + + flush_writes(ohci); /* We are ready to go, reset bus to finish initialization. */ fw_schedule_bus_reset(&ohci->card, false, true); @@ -3128,7 +3125,6 @@ static void ohci_flush_queue_iso(struct fw_iso_context *base) &container_of(base, struct iso_context, base)->context; reg_write(ctx->ohci, CONTROL_SET(ctx->regs), CONTEXT_WAKE); - flush_writes(ctx->ohci); } static const struct fw_card_driver ohci_driver = { -- cgit v1.2.1 From 9ef28ccd59a23d219c4660f55a11ac06ca91f632 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Sun, 12 Jun 2011 14:30:57 +0200 Subject: firewire: ohci: reduce potential context_stop latency Stopping an isochronous reception DMA context takes two loop iterations in context_stop on several controllers (JMicron, NEC, VIA). But there is no extra delay necessary between these two reg_read trials; the MMIO reads themselves are slow enough. Hence bring back the behavior from before commit dd6254e5c0efe01ad255188898cb3dadf98cb56d "firewire: ohci: remove superfluous posted write flushes" on these controllers by means of an "if (i)" condition. Isochronous context stop is performed in preemptible contexts (and only rarely), hence this change is of little impact. (Besides, Agere and TI controllers always, or almost always, have the context stopped already at the first ContextControl read.) More important is asynchronous transmit context stop, which is performed while local interrupts are disabled (on the two AT DMAs in bus_reset_tasklet, i.e. after a self-ID-complete event). In my experience with several controllers, tested with a usermode AT-request transmitter as well as with FTP transmission over firewire-net, the AT contexts were luckily already stopped at the first ContextControl read, i.e. never required another MMIO read let alone mdelay. A possible explanation for this is that the controllers which I tested perhaps stop AT DMA before they perform the self-ID reception DMA. But we cannot be sure about that and should keep the interrupts-disabled busy loop as short as possible. Hence, query the ContextControl register in 1000 udelay(10) intervals instead of 10 udelay(1000) intervals. I understand from an estimation by Clemens Ladisch that stopping a busy DMA context should take microseconds or at worst tens of microseconds, not milliseconds. Signed-off-by: Stefan Richter --- drivers/firewire/ohci.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'drivers/firewire/ohci.c') diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index e291edaa5eef..3b6f3429fb4a 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -1200,12 +1200,13 @@ static void context_stop(struct context *ctx) reg_write(ctx->ohci, CONTROL_CLEAR(ctx->regs), CONTEXT_RUN); ctx->running = false; - for (i = 0; i < 10; i++) { + for (i = 0; i < 1000; i++) { reg = reg_read(ctx->ohci, CONTROL_SET(ctx->regs)); if ((reg & CONTEXT_ACTIVE) == 0) return; - mdelay(1); + if (i) + udelay(10); } fw_error("Error: DMA context still active (0x%08x)\n", reg); } -- cgit v1.2.1 From b14c369d87d7fbf120ad21919d34a8f1290290f1 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Tue, 21 Jun 2011 15:24:26 +0200 Subject: firewire: ohci: add a comment on PHY reg access serialization Signed-off-by: Stefan Richter --- drivers/firewire/ohci.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'drivers/firewire/ohci.c') diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 3b6f3429fb4a..a818dc834690 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -513,6 +513,12 @@ static inline void flush_writes(const struct fw_ohci *ohci) reg_read(ohci, OHCI1394_Version); } +/* + * Beware! read_phy_reg(), write_phy_reg(), update_phy_reg(), and + * read_paged_phy_reg() require the caller to hold ohci->phy_reg_mutex. + * In other words, only use ohci_read_phy_reg() and ohci_update_phy_reg() + * directly. Exceptions are intrinsically serialized contexts like pci_probe. + */ static int read_phy_reg(struct fw_ohci *ohci, int addr) { u32 val; -- cgit v1.2.1 From 215fa444c2a6d571f1f915cf3dc7a8b01cc51a0a Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Wed, 22 Jun 2011 21:05:08 +0200 Subject: firewire: ohci: fix PHY reg access after card ejection Detect and handle ejection of FireWire CardBus cards in PHY register accesses: - The last attempt of firewire-core to reset the bus during shutdown caused a spurious "firewire_ohci: failed to write phy reg" error message in the log. Skip this message as well as the prior retry loop that needlessly took 100 milliseconds. - In the unlikely case that a PHY register was read right after card ejection, a bogus value was obtained and possibly acted upon. Instead, fail the read attempt. Signed-off-by: Stefan Richter --- drivers/firewire/ohci.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'drivers/firewire/ohci.c') diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index a818dc834690..448598876278 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -527,6 +527,9 @@ static int read_phy_reg(struct fw_ohci *ohci, int addr) reg_write(ohci, OHCI1394_PhyControl, OHCI1394_PhyControl_Read(addr)); for (i = 0; i < 3 + 100; i++) { val = reg_read(ohci, OHCI1394_PhyControl); + if (!~val) + return -ENODEV; /* Card was ejected. */ + if (val & OHCI1394_PhyControl_ReadDone) return OHCI1394_PhyControl_ReadData(val); @@ -550,6 +553,9 @@ static int write_phy_reg(const struct fw_ohci *ohci, int addr, u32 val) OHCI1394_PhyControl_Write(addr, val)); for (i = 0; i < 3 + 100; i++) { val = reg_read(ohci, OHCI1394_PhyControl); + if (!~val) + return -ENODEV; /* Card was ejected. */ + if (!(val & OHCI1394_PhyControl_WritePending)) return 0; -- cgit v1.2.1 From 9f426173e54a4f0882f9516c226f3165a3bd5474 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Sun, 3 Jul 2011 17:39:26 +0200 Subject: firewire: ohci: skip soft reset retries after card ejection The software reset in firewire-ohci's pci_remove does not have a great prospect of success if the card was already physically removed at this point. So let's skip the 500 ms that were spent in retries here. Also, replace a defined constant by its open-coded value. This is not a constant from a specification but an arbitrarily chosen retry limit. It was only used in this single place. Signed-off-by: Stefan Richter --- drivers/firewire/ohci.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'drivers/firewire/ohci.c') diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 448598876278..4f6d72f87f6f 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -253,7 +253,6 @@ static inline struct fw_ohci *fw_ohci(struct fw_card *card) #define OHCI1394_MAX_PHYS_RESP_RETRIES 0x8 #define OHCI1394_REGISTER_SIZE 0x800 -#define OHCI_LOOP_COUNT 500 #define OHCI1394_PCI_HCI_Control 0x40 #define SELF_ID_BUF_SIZE 0x800 #define OHCI_TCODE_PHY_PACKET 0x0e @@ -1967,14 +1966,18 @@ static irqreturn_t irq_handler(int irq, void *data) static int software_reset(struct fw_ohci *ohci) { + u32 val; int i; reg_write(ohci, OHCI1394_HCControlSet, OHCI1394_HCControl_softReset); + for (i = 0; i < 500; i++) { + val = reg_read(ohci, OHCI1394_HCControlSet); + if (!~val) + return -ENODEV; /* Card was ejected. */ - for (i = 0; i < OHCI_LOOP_COUNT; i++) { - if ((reg_read(ohci, OHCI1394_HCControlSet) & - OHCI1394_HCControl_softReset) == 0) + if (!(val & OHCI1394_HCControl_softReset)) return 0; + msleep(1); } -- cgit v1.2.1