From 1f8fef7b3388b5a976e80839679b5bae581a1091 Mon Sep 17 00:00:00 2001 From: Clemens Ladisch Date: Thu, 24 Dec 2009 11:59:57 +0100 Subject: firewire: add fw_csr_string() helper function The core (sysfs attributes), the firedtv driver, and possible future drivers all read strings from some configuration ROM directory. Factor out the generic code from show_text_leaf() into a new helper function, modified slightly to handle arbitrary buffer sizes. Signed-off-by: Clemens Ladisch Signed-off-by: Stefan Richter --- drivers/firewire/core-device.c | 110 ++++++++++++++++++++++---------- drivers/media/dvb/firewire/firedtv-fw.c | 39 ++--------- 2 files changed, 82 insertions(+), 67 deletions(-) (limited to 'drivers') diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c index 9d0dfcbe2c1..a39e4344cd5 100644 --- a/drivers/firewire/core-device.c +++ b/drivers/firewire/core-device.c @@ -59,6 +59,67 @@ int fw_csr_iterator_next(struct fw_csr_iterator *ci, int *key, int *value) } EXPORT_SYMBOL(fw_csr_iterator_next); +static u32 *search_leaf(u32 *directory, int search_key) +{ + struct fw_csr_iterator ci; + int last_key = 0, key, value; + + fw_csr_iterator_init(&ci, directory); + while (fw_csr_iterator_next(&ci, &key, &value)) { + if (last_key == search_key && + key == (CSR_DESCRIPTOR | CSR_LEAF)) + return ci.p - 1 + value; + last_key = key; + } + return NULL; +} + +static int textual_leaf_to_string(u32 *block, char *buf, size_t size) +{ + unsigned int quadlets, length; + + if (!size || !buf) + return -EINVAL; + + quadlets = min(block[0] >> 16, 256u); + if (quadlets < 2) + return -ENODATA; + + if (block[1] != 0 || block[2] != 0) + /* unknown language/character set */ + return -ENODATA; + + block += 3; + quadlets -= 2; + for (length = 0; length < quadlets * 4 && length + 1 < size; length++) { + char c = block[length / 4] >> (24 - 8 * (length % 4)); + if (c == '\0') + break; + buf[length] = c; + } + buf[length] = '\0'; + return length; +} + +/** + * fw_csr_string - reads a string from the configuration ROM + * @directory: device or unit directory; + * fw_device->config_rom+5 or fw_unit->directory + * @key: the key of the preceding directory entry + * @buf: where to put the string + * @size: size of @buf, in bytes + * + * Returns string length (>= 0) or error code (< 0). + */ +int fw_csr_string(u32 *directory, int key, char *buf, size_t size) +{ + u32 *leaf = search_leaf(directory, key); + if (!leaf) + return -ENOENT; + return textual_leaf_to_string(leaf, buf, size); +} +EXPORT_SYMBOL(fw_csr_string); + static bool is_fw_unit(struct device *dev); static int match_unit_directory(u32 *directory, u32 match_flags, @@ -226,10 +287,10 @@ static ssize_t show_text_leaf(struct device *dev, { struct config_rom_attribute *attr = container_of(dattr, struct config_rom_attribute, attr); - struct fw_csr_iterator ci; - u32 *dir, *block = NULL, *p, *end; - int length, key, value, last_key = 0, ret = -ENOENT; - char *b; + u32 *dir; + size_t bufsize; + char dummy_buf[2]; + int ret; down_read(&fw_device_rwsem); @@ -238,40 +299,23 @@ static ssize_t show_text_leaf(struct device *dev, else dir = fw_device(dev)->config_rom + 5; - fw_csr_iterator_init(&ci, dir); - while (fw_csr_iterator_next(&ci, &key, &value)) { - if (attr->key == last_key && - key == (CSR_DESCRIPTOR | CSR_LEAF)) - block = ci.p - 1 + value; - last_key = key; + if (buf) { + bufsize = PAGE_SIZE - 1; + } else { + buf = dummy_buf; + bufsize = 1; } - if (block == NULL) - goto out; - - length = min(block[0] >> 16, 256U); - if (length < 3) - goto out; - - if (block[1] != 0 || block[2] != 0) - /* Unknown encoding. */ - goto out; + ret = fw_csr_string(dir, attr->key, buf, bufsize); - if (buf == NULL) { - ret = length * 4; - goto out; + if (ret >= 0) { + /* Strip trailing whitespace and add newline. */ + while (ret > 0 && isspace(buf[ret - 1])) + ret--; + strcpy(buf + ret, "\n"); + ret++; } - b = buf; - end = &block[length + 1]; - for (p = &block[3]; p < end; p++, b += 4) - * (u32 *) b = (__force u32) __cpu_to_be32(*p); - - /* Strip trailing whitespace and add newline. */ - while (b--, (isspace(*b) || *b == '\0') && b > buf); - strcpy(b + 1, "\n"); - ret = b + 2 - buf; - out: up_read(&fw_device_rwsem); return ret; diff --git a/drivers/media/dvb/firewire/firedtv-fw.c b/drivers/media/dvb/firewire/firedtv-fw.c index 6223bf01efe..4253b7ab009 100644 --- a/drivers/media/dvb/firewire/firedtv-fw.c +++ b/drivers/media/dvb/firewire/firedtv-fw.c @@ -239,47 +239,18 @@ static const struct fw_address_region fcp_region = { }; /* Adjust the template string if models with longer names appear. */ -#define MAX_MODEL_NAME_LEN ((int)DIV_ROUND_UP(sizeof("FireDTV ????"), 4)) - -static size_t model_name(u32 *directory, __be32 *buffer) -{ - struct fw_csr_iterator ci; - int i, length, key, value, last_key = 0; - u32 *block = NULL; - - fw_csr_iterator_init(&ci, directory); - while (fw_csr_iterator_next(&ci, &key, &value)) { - if (last_key == CSR_MODEL && - key == (CSR_DESCRIPTOR | CSR_LEAF)) - block = ci.p - 1 + value; - last_key = key; - } - - if (block == NULL) - return 0; - - length = min((int)(block[0] >> 16) - 2, MAX_MODEL_NAME_LEN); - if (length <= 0) - return 0; - - /* fast-forward to text string */ - block += 3; - - for (i = 0; i < length; i++) - buffer[i] = cpu_to_be32(block[i]); - - return length * 4; -} +#define MAX_MODEL_NAME_LEN sizeof("FireDTV ????") static int node_probe(struct device *dev) { struct firedtv *fdtv; - __be32 name[MAX_MODEL_NAME_LEN]; + char name[MAX_MODEL_NAME_LEN]; int name_len, err; - name_len = model_name(fw_unit(dev)->directory, name); + name_len = fw_csr_string(fw_unit(dev)->directory, CSR_MODEL, + name, sizeof(name)); - fdtv = fdtv_alloc(dev, &backend, (char *)name, name_len); + fdtv = fdtv_alloc(dev, &backend, name, name_len >= 0 ? name_len : 0); if (!fdtv) return -ENOMEM; -- cgit v1.2.3-18-g5258 From 3c2c58cb33b3b15a2c4871babeec8fe1456e1db6 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Sat, 26 Dec 2009 01:43:21 +0100 Subject: firewire: core: fw_csr_string addendum Witespace and comment changes, and a different way to say i + 1 < end. Signed-off-by: Stefan Richter --- drivers/firewire/core-device.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) (limited to 'drivers') diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c index a39e4344cd5..5d5c6a68983 100644 --- a/drivers/firewire/core-device.c +++ b/drivers/firewire/core-device.c @@ -69,19 +69,22 @@ static u32 *search_leaf(u32 *directory, int search_key) if (last_key == search_key && key == (CSR_DESCRIPTOR | CSR_LEAF)) return ci.p - 1 + value; + last_key = key; } + return NULL; } static int textual_leaf_to_string(u32 *block, char *buf, size_t size) { - unsigned int quadlets, length; + unsigned int quadlets, i; + char c; if (!size || !buf) return -EINVAL; - quadlets = min(block[0] >> 16, 256u); + quadlets = min(block[0] >> 16, 256U); if (quadlets < 2) return -ENODATA; @@ -91,31 +94,34 @@ static int textual_leaf_to_string(u32 *block, char *buf, size_t size) block += 3; quadlets -= 2; - for (length = 0; length < quadlets * 4 && length + 1 < size; length++) { - char c = block[length / 4] >> (24 - 8 * (length % 4)); + for (i = 0; i < quadlets * 4 && i < size - 1; i++) { + c = block[i / 4] >> (24 - 8 * (i % 4)); if (c == '\0') break; - buf[length] = c; + buf[i] = c; } - buf[length] = '\0'; - return length; + buf[i] = '\0'; + + return i; } /** * fw_csr_string - reads a string from the configuration ROM - * @directory: device or unit directory; - * fw_device->config_rom+5 or fw_unit->directory + * @directory: e.g. root directory or unit directory * @key: the key of the preceding directory entry * @buf: where to put the string * @size: size of @buf, in bytes * - * Returns string length (>= 0) or error code (< 0). + * The string is taken from a minimal ASCII text descriptor leaf after + * the immediate entry with @key. The string is zero-terminated. + * Returns strlen(buf) or a negative error code. */ int fw_csr_string(u32 *directory, int key, char *buf, size_t size) { u32 *leaf = search_leaf(directory, key); if (!leaf) return -ENOENT; + return textual_leaf_to_string(leaf, buf, size); } EXPORT_SYMBOL(fw_csr_string); -- cgit v1.2.3-18-g5258 From 13b302d0a217580c0129b0641b0ca8b592e437b0 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Sat, 26 Dec 2009 01:44:10 +0100 Subject: firewire: qualify config ROM cache pointers as const pointers Several config ROM related functions only peek at the ROM cache; mark their arguments as const pointers. Ditto fw_device.config_rom and fw_unit.directory, as the memory behind them is meant to be write-once. Signed-off-by: Stefan Richter --- drivers/firewire/core-device.c | 21 +++++++++++---------- drivers/firewire/sbp2.c | 5 +++-- 2 files changed, 14 insertions(+), 12 deletions(-) (limited to 'drivers') diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c index 5d5c6a68983..eecd52dc8e9 100644 --- a/drivers/firewire/core-device.c +++ b/drivers/firewire/core-device.c @@ -43,7 +43,7 @@ #include "core.h" -void fw_csr_iterator_init(struct fw_csr_iterator *ci, u32 * p) +void fw_csr_iterator_init(struct fw_csr_iterator *ci, const u32 *p) { ci->p = p + 1; ci->end = ci->p + (p[0] >> 16); @@ -59,7 +59,7 @@ int fw_csr_iterator_next(struct fw_csr_iterator *ci, int *key, int *value) } EXPORT_SYMBOL(fw_csr_iterator_next); -static u32 *search_leaf(u32 *directory, int search_key) +static const u32 *search_leaf(const u32 *directory, int search_key) { struct fw_csr_iterator ci; int last_key = 0, key, value; @@ -76,7 +76,7 @@ static u32 *search_leaf(u32 *directory, int search_key) return NULL; } -static int textual_leaf_to_string(u32 *block, char *buf, size_t size) +static int textual_leaf_to_string(const u32 *block, char *buf, size_t size) { unsigned int quadlets, i; char c; @@ -116,9 +116,9 @@ static int textual_leaf_to_string(u32 *block, char *buf, size_t size) * the immediate entry with @key. The string is zero-terminated. * Returns strlen(buf) or a negative error code. */ -int fw_csr_string(u32 *directory, int key, char *buf, size_t size) +int fw_csr_string(const u32 *directory, int key, char *buf, size_t size) { - u32 *leaf = search_leaf(directory, key); + const u32 *leaf = search_leaf(directory, key); if (!leaf) return -ENOENT; @@ -128,7 +128,7 @@ EXPORT_SYMBOL(fw_csr_string); static bool is_fw_unit(struct device *dev); -static int match_unit_directory(u32 *directory, u32 match_flags, +static int match_unit_directory(const u32 *directory, u32 match_flags, const struct ieee1394_device_id *id) { struct fw_csr_iterator ci; @@ -262,7 +262,7 @@ static ssize_t show_immediate(struct device *dev, struct config_rom_attribute *attr = container_of(dattr, struct config_rom_attribute, attr); struct fw_csr_iterator ci; - u32 *dir; + const u32 *dir; int key, value, ret = -ENOENT; down_read(&fw_device_rwsem); @@ -293,7 +293,7 @@ static ssize_t show_text_leaf(struct device *dev, { struct config_rom_attribute *attr = container_of(dattr, struct config_rom_attribute, attr); - u32 *dir; + const u32 *dir; size_t bufsize; char dummy_buf[2]; int ret; @@ -421,7 +421,7 @@ static ssize_t guid_show(struct device *dev, return ret; } -static int units_sprintf(char *buf, u32 *directory) +static int units_sprintf(char *buf, const u32 *directory) { struct fw_csr_iterator ci; int key, value; @@ -503,7 +503,8 @@ static int read_rom(struct fw_device *device, */ static int read_bus_info_block(struct fw_device *device, int generation) { - u32 *rom, *stack, *old_rom, *new_rom; + const u32 *old_rom, *new_rom; + u32 *rom, *stack; u32 sp, key; int i, end, length, ret = -1; diff --git a/drivers/firewire/sbp2.c b/drivers/firewire/sbp2.c index d485cdd8cba..7e33b0b1704 100644 --- a/drivers/firewire/sbp2.c +++ b/drivers/firewire/sbp2.c @@ -1014,7 +1014,8 @@ static int sbp2_add_logical_unit(struct sbp2_target *tgt, int lun_entry) return 0; } -static int sbp2_scan_logical_unit_dir(struct sbp2_target *tgt, u32 *directory) +static int sbp2_scan_logical_unit_dir(struct sbp2_target *tgt, + const u32 *directory) { struct fw_csr_iterator ci; int key, value; @@ -1027,7 +1028,7 @@ static int sbp2_scan_logical_unit_dir(struct sbp2_target *tgt, u32 *directory) return 0; } -static int sbp2_scan_unit_dir(struct sbp2_target *tgt, u32 *directory, +static int sbp2_scan_unit_dir(struct sbp2_target *tgt, const u32 *directory, u32 *model, u32 *firmware_revision) { struct fw_csr_iterator ci; -- cgit v1.2.3-18-g5258 From a67483d2be12dfc5563c09e6169bec9a88f434b0 Mon Sep 17 00:00:00 2001 From: Németh Márton Date: Sun, 10 Jan 2010 13:14:26 +0100 Subject: firewire: make PCI device id constant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The id_table field of the struct pci_driver is constant in so it is worth to make pci_table also constant. Found with Coccinelle. Signed-off-by: Márton Németh Cc: Julia Lawall Cc: cocci@diku.dk Signed-off-by: Stefan Richter stefanr@s5r6.in-berlin.de> (changelog) --- drivers/firewire/ohci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index a61571c63c5..6610d2d3880 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -2657,7 +2657,7 @@ static int pci_resume(struct pci_dev *dev) } #endif -static struct pci_device_id pci_table[] = { +static const struct pci_device_id pci_table[] = { { PCI_DEVICE_CLASS(PCI_CLASS_SERIAL_FIREWIRE_OHCI, ~0) }, { } }; -- cgit v1.2.3-18-g5258 From b677532b971276f48e82578b4d829fb4382e7b41 Mon Sep 17 00:00:00 2001 From: Clemens Ladisch Date: Wed, 20 Jan 2010 09:58:02 +0100 Subject: firewire: ohci: work around cycle timer bugs on VIA controllers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit VIA controllers sometimes return an inconsistent value when reading the isochronous cycle timer register. To work around this, read the register multiple times and add consistency checks. Signed-off-by: Clemens Ladisch Reported-by: Pieter Palmers Reported-by: Håkan Johansson Signed-off-by: Stefan Richter --- drivers/firewire/ohci.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 49 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index 6610d2d3880..d6ba897b219 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -192,6 +192,7 @@ struct fw_ohci { bool use_dualbuffer; bool old_uninorth; bool bus_reset_packet_quirk; + bool iso_cycle_timer_quirk; /* * Spinlock for accessing fw_ohci data. Never call out of @@ -1794,14 +1795,57 @@ static int ohci_enable_phys_dma(struct fw_card *card, #endif /* CONFIG_FIREWIRE_OHCI_REMOTE_DMA */ } +static inline u32 cycle_timer_ticks(u32 cycle_timer) +{ + u32 ticks; + + ticks = cycle_timer & 0xfff; + ticks += 3072 * ((cycle_timer >> 12) & 0x1fff); + ticks += (3072 * 8000) * (cycle_timer >> 25); + return ticks; +} + static u64 ohci_get_bus_time(struct fw_card *card) { struct fw_ohci *ohci = fw_ohci(card); - u32 cycle_time; + u32 c0, c1, c2; + u32 t0, t1, t2; + s32 diff01, diff12; u64 bus_time; - cycle_time = reg_read(ohci, OHCI1394_IsochronousCycleTimer); - bus_time = ((u64)atomic_read(&ohci->bus_seconds) << 32) | cycle_time; + if (!ohci->iso_cycle_timer_quirk) { + c2 = reg_read(ohci, OHCI1394_IsochronousCycleTimer); + } else { + /* + * VIA controllers have two bugs when updating the iso cycle + * timer register: + * 1) When the lowest six bits are wrapping around to zero, + * a read that happens at the same time will return garbage + * in the lowest ten bits. + * 2) When the cycleOffset field wraps around to zero, the + * cycleCount field is not incremented for about 60 ns. + * + * To catch these, we read the register three times and ensure + * that the difference between each two consecutive reads is + * approximately the same, i.e., less than twice the other. + * Furthermore, any negative difference indicates an error. + * (A PCI read should take at least 20 ticks of the 24.576 MHz + * timer to execute, so we have enough precision to compute the + * ratio of the differences.) + */ + do { + c0 = reg_read(ohci, OHCI1394_IsochronousCycleTimer); + c1 = reg_read(ohci, OHCI1394_IsochronousCycleTimer); + c2 = reg_read(ohci, OHCI1394_IsochronousCycleTimer); + t0 = cycle_timer_ticks(c0); + t1 = cycle_timer_ticks(c1); + t2 = cycle_timer_ticks(c2); + diff01 = t1 - t0; + diff12 = t2 - t1; + } while (diff01 <= 0 || diff12 <= 0 || + diff01 / diff12 >= 2 || diff12 / diff01 >= 2); + } + bus_time = ((u64)atomic_read(&ohci->bus_seconds) << 32) | c2; return bus_time; } @@ -2498,6 +2542,8 @@ static int __devinit pci_probe(struct pci_dev *dev, #endif ohci->bus_reset_packet_quirk = dev->vendor == PCI_VENDOR_ID_TI; + ohci->iso_cycle_timer_quirk = dev->vendor == PCI_VENDOR_ID_VIA; + ar_context_init(&ohci->ar_request_ctx, ohci, OHCI1394_AsReqRcvContextControlSet); -- cgit v1.2.3-18-g5258 From 1c1517efe173599ca2f1526ce7a04521cd424a9f Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Sun, 14 Feb 2010 18:47:07 +0100 Subject: firewire: ohci: enable cycle timer fix on ALi and NEC controllers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Discussed in "read_cycle_timer backwards for sub-cycle 0000, 0001", http://thread.gmane.org/gmane.linux.kernel.firewire.devel/13704 Known bad controllers: ALi M5271, listed by lspci as M5253 [10b9:5253] NEC OrangeLink [1033:00cd] (rev 03) NEC uPD72874 [1033:00f2] (rev 01) VIA VT6306 [1106:3044] (rev 46) VIA VT6308P, listed by lspci as rev c0 Reported-by: Pieter Palmers Reported-by: Håkan Johansson Reported-by: Clemens Ladisch Signed-off-by: Stefan Richter --- drivers/firewire/ohci.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) (limited to 'drivers') diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index d6ba897b219..bf5e1128442 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -1817,13 +1817,14 @@ static u64 ohci_get_bus_time(struct fw_card *card) c2 = reg_read(ohci, OHCI1394_IsochronousCycleTimer); } else { /* - * VIA controllers have two bugs when updating the iso cycle - * timer register: - * 1) When the lowest six bits are wrapping around to zero, + * Some controllers exhibit one or more of the following bugs + * when updating the iso cycle timer register: + * - When the lowest six bits are wrapping around to zero, * a read that happens at the same time will return garbage * in the lowest ten bits. - * 2) When the cycleOffset field wraps around to zero, the + * - When the cycleOffset field wraps around to zero, the * cycleCount field is not incremented for about 60 ns. + * - Occasionally, the entire register reads zero. * * To catch these, we read the register three times and ensure * that the difference between each two consecutive reads is @@ -2542,7 +2543,9 @@ static int __devinit pci_probe(struct pci_dev *dev, #endif ohci->bus_reset_packet_quirk = dev->vendor == PCI_VENDOR_ID_TI; - ohci->iso_cycle_timer_quirk = dev->vendor == PCI_VENDOR_ID_VIA; + ohci->iso_cycle_timer_quirk = dev->vendor == PCI_VENDOR_ID_AL || + dev->vendor == PCI_VENDOR_ID_NEC || + dev->vendor == PCI_VENDOR_ID_VIA; ar_context_init(&ohci->ar_request_ctx, ohci, OHCI1394_AsReqRcvContextControlSet); -- cgit v1.2.3-18-g5258 From 4a9bde9b8ab55a2bb51b57cad215a97bcf80bae2 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Sat, 20 Feb 2010 22:24:43 +0100 Subject: firewire: get_cycle_timer optimization and cleanup ohci: Break out of the retry loop if too many attempts were necessary. This may theoretically happen if the chip is fatally defective or if the get_cycle_timer ioctl was performed after a CardBus controller was ejected. Also micro-optimize the loop by re-using the last two register reads in the next iteration, remove a questionable inline keyword, and shuffle a comment around. core: ioctl_get_cycle_timer() is always called with interrupts on, therefore local_irq_save() can be replaced by local_irq_disable(). Disabled local IRQs imply disabled preemption, hence preempt_disable() can be removed. Signed-off-by: Stefan Richter --- drivers/firewire/core-cdev.c | 16 ++++++------- drivers/firewire/ohci.c | 57 ++++++++++++++++++++++---------------------- 2 files changed, 36 insertions(+), 37 deletions(-) (limited to 'drivers') diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c index e6d63849e78..ecd0a4d81ab 100644 --- a/drivers/firewire/core-cdev.c +++ b/drivers/firewire/core-cdev.c @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -32,7 +33,6 @@ #include #include #include -#include #include #include #include @@ -1013,21 +1013,19 @@ static int ioctl_get_cycle_timer(struct client *client, void *buffer) { struct fw_cdev_get_cycle_timer *request = buffer; struct fw_card *card = client->device->card; - unsigned long long bus_time; struct timeval tv; - unsigned long flags; + u32 cycle_time; - preempt_disable(); - local_irq_save(flags); + local_irq_disable(); - bus_time = card->driver->get_bus_time(card); + cycle_time = card->driver->get_bus_time(card); do_gettimeofday(&tv); - local_irq_restore(flags); - preempt_enable(); + local_irq_enable(); request->local_time = tv.tv_sec * 1000000ULL + tv.tv_usec; - request->cycle_timer = bus_time & 0xffffffff; + request->cycle_timer = cycle_time; + return 0; } diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index bf5e1128442..c3eb471d22f 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -1795,60 +1795,61 @@ static int ohci_enable_phys_dma(struct fw_card *card, #endif /* CONFIG_FIREWIRE_OHCI_REMOTE_DMA */ } -static inline u32 cycle_timer_ticks(u32 cycle_timer) +static u32 cycle_timer_ticks(u32 cycle_timer) { u32 ticks; ticks = cycle_timer & 0xfff; ticks += 3072 * ((cycle_timer >> 12) & 0x1fff); ticks += (3072 * 8000) * (cycle_timer >> 25); + return ticks; } +/* + * Some controllers exhibit one or more of the following bugs when updating the + * iso cycle timer register: + * - When the lowest six bits are wrapping around to zero, a read that happens + * at the same time will return garbage in the lowest ten bits. + * - When the cycleOffset field wraps around to zero, the cycleCount field is + * not incremented for about 60 ns. + * - Occasionally, the entire register reads zero. + * + * To catch these, we read the register three times and ensure that the + * difference between each two consecutive reads is approximately the same, i.e. + * less than twice the other. Furthermore, any negative difference indicates an + * error. (A PCI read should take at least 20 ticks of the 24.576 MHz timer to + * execute, so we have enough precision to compute the ratio of the differences.) + */ static u64 ohci_get_bus_time(struct fw_card *card) { struct fw_ohci *ohci = fw_ohci(card); u32 c0, c1, c2; u32 t0, t1, t2; s32 diff01, diff12; - u64 bus_time; + int i; - if (!ohci->iso_cycle_timer_quirk) { + c2 = reg_read(ohci, OHCI1394_IsochronousCycleTimer); + + if (ohci->iso_cycle_timer_quirk) { + i = 0; + c1 = c2; c2 = reg_read(ohci, OHCI1394_IsochronousCycleTimer); - } else { - /* - * Some controllers exhibit one or more of the following bugs - * when updating the iso cycle timer register: - * - When the lowest six bits are wrapping around to zero, - * a read that happens at the same time will return garbage - * in the lowest ten bits. - * - When the cycleOffset field wraps around to zero, the - * cycleCount field is not incremented for about 60 ns. - * - Occasionally, the entire register reads zero. - * - * To catch these, we read the register three times and ensure - * that the difference between each two consecutive reads is - * approximately the same, i.e., less than twice the other. - * Furthermore, any negative difference indicates an error. - * (A PCI read should take at least 20 ticks of the 24.576 MHz - * timer to execute, so we have enough precision to compute the - * ratio of the differences.) - */ do { - c0 = reg_read(ohci, OHCI1394_IsochronousCycleTimer); - c1 = reg_read(ohci, OHCI1394_IsochronousCycleTimer); + c0 = c1; + c1 = c2; c2 = reg_read(ohci, OHCI1394_IsochronousCycleTimer); t0 = cycle_timer_ticks(c0); t1 = cycle_timer_ticks(c1); t2 = cycle_timer_ticks(c2); diff01 = t1 - t0; diff12 = t2 - t1; - } while (diff01 <= 0 || diff12 <= 0 || - diff01 / diff12 >= 2 || diff12 / diff01 >= 2); + } while ((diff01 <= 0 || diff12 <= 0 || + diff01 / diff12 >= 2 || diff12 / diff01 >= 2) + && i++ < 20); } - bus_time = ((u64)atomic_read(&ohci->bus_seconds) << 32) | c2; - return bus_time; + return ((u64)atomic_read(&ohci->bus_seconds) << 32) | c2; } static void copy_iso_headers(struct iso_context *ctx, void *p) -- cgit v1.2.3-18-g5258 From 168cf9af699e87d5a6f44b684583714ecabb8e71 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Sun, 14 Feb 2010 18:49:18 +0100 Subject: firewire: remove incomplete Bus_Time CSR support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The current implementation of Bus_Time read access was buggy since it did not ensure that Bus_Time.second_count_hi and second_count_lo came from the same 128 seconds period. Reported-by: Håkan Johansson Instead of a fix, remove Bus_Time register support altogether. The spec requires all cycle master capable nodes to implement this (all Linux nodes are cycle master capable) while it also says that it "may" be initialized by the bus manager or by the IRM standing in for a bus manager. (Neither Linux' firewire-core nor ieee1394 nodemgr implement this.) Since we cannot rely on Bus_Time having been initialized by a bus manager, it is better to return an error instead of a nonsensical value on a read request to Bus_Time. Alternatively, we could fix the Bus_Time read integrity bug _and_ implement (a) cycle master's write support of the register as well as (b) bus manager's Bus_Time initialization service, i.e. preservation of the Bus_Time when the cycle master node of a bus changes. However, that would be quite some code for a feature that is unreliable to begin with and very likely unused in practice. Signed-off-by: Stefan Richter --- drivers/firewire/core-cdev.c | 2 +- drivers/firewire/core-transaction.c | 17 ++++++----------- drivers/firewire/core.h | 2 +- drivers/firewire/ohci.c | 25 +++++++------------------ 4 files changed, 15 insertions(+), 31 deletions(-) (limited to 'drivers') diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c index ecd0a4d81ab..5538d5130f7 100644 --- a/drivers/firewire/core-cdev.c +++ b/drivers/firewire/core-cdev.c @@ -1018,7 +1018,7 @@ static int ioctl_get_cycle_timer(struct client *client, void *buffer) local_irq_disable(); - cycle_time = card->driver->get_bus_time(card); + cycle_time = card->driver->get_cycle_time(card); do_gettimeofday(&tv); local_irq_enable(); diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c index 495849eb13c..673b03f8b4e 100644 --- a/drivers/firewire/core-transaction.c +++ b/drivers/firewire/core-transaction.c @@ -921,23 +921,15 @@ static void handle_registers(struct fw_card *card, struct fw_request *request, void *payload, size_t length, void *callback_data) { int reg = offset & ~CSR_REGISTER_BASE; - unsigned long long bus_time; __be32 *data = payload; int rcode = RCODE_COMPLETE; switch (reg) { case CSR_CYCLE_TIME: - case CSR_BUS_TIME: - if (!TCODE_IS_READ_REQUEST(tcode) || length != 4) { - rcode = RCODE_TYPE_ERROR; - break; - } - - bus_time = card->driver->get_bus_time(card); - if (reg == CSR_CYCLE_TIME) - *data = cpu_to_be32(bus_time); + if (TCODE_IS_READ_REQUEST(tcode) && length == 4) + *data = cpu_to_be32(card->driver->get_cycle_time(card)); else - *data = cpu_to_be32(bus_time >> 25); + rcode = RCODE_TYPE_ERROR; break; case CSR_BROADCAST_CHANNEL: @@ -968,6 +960,9 @@ static void handle_registers(struct fw_card *card, struct fw_request *request, case CSR_BUSY_TIMEOUT: /* FIXME: Implement this. */ + case CSR_BUS_TIME: + /* Useless without initialization by the bus manager. */ + default: rcode = RCODE_ADDRESS_ERROR; break; diff --git a/drivers/firewire/core.h b/drivers/firewire/core.h index ed3b1a765c0..fb0321300cc 100644 --- a/drivers/firewire/core.h +++ b/drivers/firewire/core.h @@ -70,7 +70,7 @@ struct fw_card_driver { int (*enable_phys_dma)(struct fw_card *card, int node_id, int generation); - u64 (*get_bus_time)(struct fw_card *card); + u32 (*get_cycle_time)(struct fw_card *card); struct fw_iso_context * (*allocate_iso_context)(struct fw_card *card, diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c index c3eb471d22f..f8a71397cf6 100644 --- a/drivers/firewire/ohci.c +++ b/drivers/firewire/ohci.c @@ -38,7 +38,6 @@ #include #include -#include #include #include #include @@ -187,7 +186,6 @@ struct fw_ohci { int node_id; int generation; int request_generation; /* for timestamping incoming requests */ - atomic_t bus_seconds; bool use_dualbuffer; bool old_uninorth; @@ -276,7 +274,7 @@ static void log_irqs(u32 evt) !(evt & OHCI1394_busReset)) return; - fw_notify("IRQ %08x%s%s%s%s%s%s%s%s%s%s%s%s%s%s\n", evt, + fw_notify("IRQ %08x%s%s%s%s%s%s%s%s%s%s%s%s%s\n", evt, evt & OHCI1394_selfIDComplete ? " selfID" : "", evt & OHCI1394_RQPkt ? " AR_req" : "", evt & OHCI1394_RSPkt ? " AR_resp" : "", @@ -286,7 +284,6 @@ static void log_irqs(u32 evt) evt & OHCI1394_isochTx ? " IT" : "", evt & OHCI1394_postedWriteErr ? " postedWriteErr" : "", evt & OHCI1394_cycleTooLong ? " cycleTooLong" : "", - evt & OHCI1394_cycle64Seconds ? " cycle64Seconds" : "", evt & OHCI1394_cycleInconsistent ? " cycleInconsistent" : "", evt & OHCI1394_regAccessFail ? " regAccessFail" : "", evt & OHCI1394_busReset ? " busReset" : "", @@ -294,8 +291,7 @@ static void log_irqs(u32 evt) OHCI1394_RSPkt | OHCI1394_reqTxComplete | OHCI1394_respTxComplete | OHCI1394_isochRx | OHCI1394_isochTx | OHCI1394_postedWriteErr | - OHCI1394_cycleTooLong | OHCI1394_cycle64Seconds | - OHCI1394_cycleInconsistent | + OHCI1394_cycleTooLong | OHCI1394_cycleInconsistent | OHCI1394_regAccessFail | OHCI1394_busReset) ? " ?" : ""); } @@ -1385,7 +1381,7 @@ static void bus_reset_tasklet(unsigned long data) static irqreturn_t irq_handler(int irq, void *data) { struct fw_ohci *ohci = data; - u32 event, iso_event, cycle_time; + u32 event, iso_event; int i; event = reg_read(ohci, OHCI1394_IntEventClear); @@ -1455,12 +1451,6 @@ static irqreturn_t irq_handler(int irq, void *data) fw_notify("isochronous cycle inconsistent\n"); } - if (event & OHCI1394_cycle64Seconds) { - cycle_time = reg_read(ohci, OHCI1394_IsochronousCycleTimer); - if ((cycle_time & 0x80000000) == 0) - atomic_inc(&ohci->bus_seconds); - } - return IRQ_HANDLED; } @@ -1554,8 +1544,7 @@ static int ohci_enable(struct fw_card *card, OHCI1394_reqTxComplete | OHCI1394_respTxComplete | OHCI1394_isochRx | OHCI1394_isochTx | OHCI1394_postedWriteErr | OHCI1394_cycleTooLong | - OHCI1394_cycleInconsistent | - OHCI1394_cycle64Seconds | OHCI1394_regAccessFail | + OHCI1394_cycleInconsistent | OHCI1394_regAccessFail | OHCI1394_masterIntEnable); if (param_debug & OHCI_PARAM_DEBUG_BUSRESETS) reg_write(ohci, OHCI1394_IntMaskSet, OHCI1394_busReset); @@ -1821,7 +1810,7 @@ static u32 cycle_timer_ticks(u32 cycle_timer) * error. (A PCI read should take at least 20 ticks of the 24.576 MHz timer to * execute, so we have enough precision to compute the ratio of the differences.) */ -static u64 ohci_get_bus_time(struct fw_card *card) +static u32 ohci_get_cycle_time(struct fw_card *card) { struct fw_ohci *ohci = fw_ohci(card); u32 c0, c1, c2; @@ -1849,7 +1838,7 @@ static u64 ohci_get_bus_time(struct fw_card *card) && i++ < 20); } - return ((u64)atomic_read(&ohci->bus_seconds) << 32) | c2; + return c2; } static void copy_iso_headers(struct iso_context *ctx, void *p) @@ -2426,7 +2415,7 @@ static const struct fw_card_driver ohci_driver = { .send_response = ohci_send_response, .cancel_packet = ohci_cancel_packet, .enable_phys_dma = ohci_enable_phys_dma, - .get_bus_time = ohci_get_bus_time, + .get_cycle_time = ohci_get_cycle_time, .allocate_iso_context = ohci_allocate_iso_context, .free_iso_context = ohci_free_iso_context, -- cgit v1.2.3-18-g5258 From d54423c62c2f687919d4e5bdd4bb064234ff2d44 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Thu, 18 Feb 2010 01:50:31 +0100 Subject: firewire: core: fix "giving up on config rom" with Panasonic AG-DV2500 The Panasonic AG-DV2500 tape deck contains an invalid entry in its configuration ROM root directory: A leaf pointer with the undefined key ID 0 and an offset that points way out of the standard config ROM area. This caused firewire-core to dismiss the device with the generic log message "giving up on config rom for node id...", after which it was of course impossible to access the tape deck with dvgrab or any other program. https://bugzilla.redhat.com/show_bug.cgi?id=449252#c29 The fix is to simply ignore this invalid ROM entry and proceed to read the valid rest of the ROM. There is a catch though: When the kernel later iterates over the ROM, it would be nasty having to check again for such too large ROM offsets. Therefore we manipulate the defective or unsupported ROM entry to become a harmless immediate entry that won't have any side effects later (an entry with the value 0x00000000). Reported-by: George Chriss Signed-off-by: Stefan Richter --- drivers/firewire/core-device.c | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) (limited to 'drivers') diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c index eecd52dc8e9..e02bf2dff84 100644 --- a/drivers/firewire/core-device.c +++ b/drivers/firewire/core-device.c @@ -18,6 +18,7 @@ * Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. */ +#include #include #include #include @@ -580,11 +581,7 @@ static int read_bus_info_block(struct fw_device *device, int generation) */ key = stack[--sp]; i = key & 0xffffff; - if (i >= READ_BIB_ROM_SIZE) - /* - * The reference points outside the standard - * config rom area, something's fishy. - */ + if (WARN_ON(i >= READ_BIB_ROM_SIZE)) goto out; /* Read header quadlet for the block to get the length. */ @@ -606,14 +603,29 @@ static int read_bus_info_block(struct fw_device *device, int generation) * block, check the entries as we read them to see if * it references another block, and push it in that case. */ - while (i < end) { + for (; i < end; i++) { if (read_rom(device, generation, i, &rom[i]) != RCODE_COMPLETE) goto out; - if ((key >> 30) == 3 && (rom[i] >> 30) > 1 && - sp < READ_BIB_STACK_SIZE) - stack[sp++] = i + rom[i]; - i++; + + if ((key >> 30) != 3 || (rom[i] >> 30) < 2 || + sp >= READ_BIB_STACK_SIZE) + continue; + /* + * Offset points outside the ROM. May be a firmware + * bug or an Extended ROM entry (IEEE 1212-2001 clause + * 7.7.18). Simply overwrite this pointer here by a + * fake immediate entry so that later iterators over + * the ROM don't have to check offsets all the time. + */ + if (i + (rom[i] & 0xffffff) >= READ_BIB_ROM_SIZE) { + fw_error("skipped unsupported ROM entry %x at %llx\n", + rom[i], + i * 4 | CSR_REGISTER_BASE | CSR_CONFIG_ROM); + rom[i] = 0; + continue; + } + stack[sp++] = i + rom[i]; } if (length < i) length = i; -- cgit v1.2.3-18-g5258 From 2799d5c5f9d2064c6d1f50ec82e28e3eac5f6954 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Thu, 18 Feb 2010 01:52:45 +0100 Subject: firewire: core: don't fail device creation in case of too large config ROM blocks It never happened yet, but better safe than sorry: If a device's config ROM contains a block which overlaps the boundary at 0xfffff00007ff, just ignore that one block instead of refusing to add the device representation. That way, upper layers (kernelspace or userspace drivers) might still be able to use the device to some degree. That's better than total inaccessibility of the device. Worse, the core would have logged only a generic "giving up on config rom" message which could only be debugged by feeding a firewire-ohci debug logging session through a config ROM interpreter, IOW would likely remain undiagnosed. Signed-off-by: Stefan Richter --- drivers/firewire/core-device.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) (limited to 'drivers') diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c index e02bf2dff84..01cb6a327e2 100644 --- a/drivers/firewire/core-device.c +++ b/drivers/firewire/core-device.c @@ -588,15 +588,19 @@ static int read_bus_info_block(struct fw_device *device, int generation) if (read_rom(device, generation, i, &rom[i]) != RCODE_COMPLETE) goto out; end = i + (rom[i] >> 16) + 1; - i++; - if (end > READ_BIB_ROM_SIZE) + if (end > READ_BIB_ROM_SIZE) { /* - * This block extends outside standard config - * area (and the array we're reading it - * into). That's broken, so ignore this - * device. + * This block extends outside the config ROM which is + * a firmware bug. Ignore this whole block, i.e. + * simply set a fake block length of 0. */ - goto out; + fw_error("skipped invalid ROM block %x at %llx\n", + rom[i], + i * 4 | CSR_REGISTER_BASE | CSR_CONFIG_ROM); + rom[i] = 0; + end = i; + } + i++; /* * Now read in the block. If this is a directory -- cgit v1.2.3-18-g5258 From 58aaa5427663b680030aa58aaaf1e2738564b8dc Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Thu, 18 Feb 2010 01:54:00 +0100 Subject: firewire: core: increase stack size of config ROM reader The stack size of 16 was artificially chosen and may be too small in extreme cases. A device won't be accessible then. Since it doesn't really matter to the slab allocator whether we ask for 1088 bytes or 2048 bytes of scratch memory, just allocate 2048 bytes for the sum of temporary config ROM image and stack, and we will never ever overflow the stack (because there simply can't be more stack items than ROM entries). Signed-off-by: Stefan Richter --- drivers/firewire/core-device.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c index 01cb6a327e2..150a8ba9748 100644 --- a/drivers/firewire/core-device.c +++ b/drivers/firewire/core-device.c @@ -493,7 +493,6 @@ static int read_rom(struct fw_device *device, } #define READ_BIB_ROM_SIZE 256 -#define READ_BIB_STACK_SIZE 16 /* * Read the bus info block, perform a speed probe, and read all of the rest of @@ -510,7 +509,7 @@ static int read_bus_info_block(struct fw_device *device, int generation) int i, end, length, ret = -1; rom = kmalloc(sizeof(*rom) * READ_BIB_ROM_SIZE + - sizeof(*stack) * READ_BIB_STACK_SIZE, GFP_KERNEL); + sizeof(*stack) * READ_BIB_ROM_SIZE, GFP_KERNEL); if (rom == NULL) return -ENOMEM; @@ -612,8 +611,7 @@ static int read_bus_info_block(struct fw_device *device, int generation) RCODE_COMPLETE) goto out; - if ((key >> 30) != 3 || (rom[i] >> 30) < 2 || - sp >= READ_BIB_STACK_SIZE) + if ((key >> 30) != 3 || (rom[i] >> 30) < 2) continue; /* * Offset points outside the ROM. May be a firmware -- cgit v1.2.3-18-g5258 From 137d9ebfdbaa45c01f9f0f6d5121ae6f1eb942bd Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Fri, 19 Feb 2010 21:00:02 +0100 Subject: firewire: core: fix an information leak If a device exposes a sparsely populated configuration ROM, firewire-core's sysfs interface and character device file interface showed random data in the gaps between config ROM blocks. Fix this by zero-initialization of the config ROM reader's scratch buffer. Signed-off-by: Stefan Richter --- drivers/firewire/core-device.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers') diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c index 150a8ba9748..f61211977d3 100644 --- a/drivers/firewire/core-device.c +++ b/drivers/firewire/core-device.c @@ -514,6 +514,7 @@ static int read_bus_info_block(struct fw_device *device, int generation) return -ENOMEM; stack = &rom[READ_BIB_ROM_SIZE]; + memset(rom, 0, sizeof(*rom) * READ_BIB_ROM_SIZE); device->max_speed = SCODE_100; -- cgit v1.2.3-18-g5258 From fd6e0c518121d22b50060d26c8aec2b701c6aab7 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Fri, 19 Feb 2010 21:00:31 +0100 Subject: firewire: core: rename an internal function according to what it really does. Signed-off-by: Stefan Richter --- drivers/firewire/core-device.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) (limited to 'drivers') diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c index f61211977d3..014cabd3afd 100644 --- a/drivers/firewire/core-device.c +++ b/drivers/firewire/core-device.c @@ -492,29 +492,29 @@ static int read_rom(struct fw_device *device, return rcode; } -#define READ_BIB_ROM_SIZE 256 +#define MAX_CONFIG_ROM_SIZE 256 /* * Read the bus info block, perform a speed probe, and read all of the rest of * the config ROM. We do all this with a cached bus generation. If the bus - * generation changes under us, read_bus_info_block will fail and get retried. + * generation changes under us, read_config_rom will fail and get retried. * It's better to start all over in this case because the node from which we * are reading the ROM may have changed the ROM during the reset. */ -static int read_bus_info_block(struct fw_device *device, int generation) +static int read_config_rom(struct fw_device *device, int generation) { const u32 *old_rom, *new_rom; u32 *rom, *stack; u32 sp, key; int i, end, length, ret = -1; - rom = kmalloc(sizeof(*rom) * READ_BIB_ROM_SIZE + - sizeof(*stack) * READ_BIB_ROM_SIZE, GFP_KERNEL); + rom = kmalloc(sizeof(*rom) * MAX_CONFIG_ROM_SIZE + + sizeof(*stack) * MAX_CONFIG_ROM_SIZE, GFP_KERNEL); if (rom == NULL) return -ENOMEM; - stack = &rom[READ_BIB_ROM_SIZE]; - memset(rom, 0, sizeof(*rom) * READ_BIB_ROM_SIZE); + stack = &rom[MAX_CONFIG_ROM_SIZE]; + memset(rom, 0, sizeof(*rom) * MAX_CONFIG_ROM_SIZE); device->max_speed = SCODE_100; @@ -581,14 +581,14 @@ static int read_bus_info_block(struct fw_device *device, int generation) */ key = stack[--sp]; i = key & 0xffffff; - if (WARN_ON(i >= READ_BIB_ROM_SIZE)) + if (WARN_ON(i >= MAX_CONFIG_ROM_SIZE)) goto out; /* Read header quadlet for the block to get the length. */ if (read_rom(device, generation, i, &rom[i]) != RCODE_COMPLETE) goto out; end = i + (rom[i] >> 16) + 1; - if (end > READ_BIB_ROM_SIZE) { + if (end > MAX_CONFIG_ROM_SIZE) { /* * This block extends outside the config ROM which is * a firmware bug. Ignore this whole block, i.e. @@ -621,7 +621,7 @@ static int read_bus_info_block(struct fw_device *device, int generation) * fake immediate entry so that later iterators over * the ROM don't have to check offsets all the time. */ - if (i + (rom[i] & 0xffffff) >= READ_BIB_ROM_SIZE) { + if (i + (rom[i] & 0xffffff) >= MAX_CONFIG_ROM_SIZE) { fw_error("skipped unsupported ROM entry %x at %llx\n", rom[i], i * 4 | CSR_REGISTER_BASE | CSR_CONFIG_ROM); @@ -971,7 +971,7 @@ static void fw_device_init(struct work_struct *work) * device. */ - if (read_bus_info_block(device, device->generation) < 0) { + if (read_config_rom(device, device->generation) < 0) { if (device->config_rom_retries < MAX_RETRIES && atomic_read(&device->state) == FW_DEVICE_INITIALIZING) { device->config_rom_retries++; @@ -1088,7 +1088,7 @@ enum { }; /* Reread and compare bus info block and header of root directory */ -static int reread_bus_info_block(struct fw_device *device, int generation) +static int reread_config_rom(struct fw_device *device, int generation) { u32 q; int i; @@ -1114,7 +1114,7 @@ static void fw_device_refresh(struct work_struct *work) struct fw_card *card = device->card; int node_id = device->node_id; - switch (reread_bus_info_block(device, device->generation)) { + switch (reread_config_rom(device, device->generation)) { case REREAD_BIB_ERROR: if (device->config_rom_retries < MAX_RETRIES / 2 && atomic_read(&device->state) == FW_DEVICE_INITIALIZING) { @@ -1148,7 +1148,7 @@ static void fw_device_refresh(struct work_struct *work) */ device_for_each_child(&device->device, NULL, shutdown_unit); - if (read_bus_info_block(device, device->generation) < 0) { + if (read_config_rom(device, device->generation) < 0) { if (device->config_rom_retries < MAX_RETRIES && atomic_read(&device->state) == FW_DEVICE_INITIALIZING) { device->config_rom_retries++; -- cgit v1.2.3-18-g5258 From abfe5a01ef1e463cbafdae461b693db34e308c02 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Sat, 20 Feb 2010 12:13:49 +0100 Subject: firewire: cdev: add more flexible cycle timer ioctl The system time from CLOCK_REALTIME is not monotonic, hence problematic for the main user of the FW_CDEV_IOC_GET_CYCLE_TIMER ioctl. This issue exists in its successor ABI, i.e. raw1394, too. http://subversion.ffado.org/ticket/242 We now offer an alternative ioctl which lets the caller choose between CLOCK_REALTIME, CLOCK_MONOTONIC, and CLOCK_MONOTONIC_RAW as source of the local time, very similar to the clock_gettime libc function. The format of the local time return value matches that of clock_gettime (seconds and nanoseconds, instead of a single microseconds value from the existing ioctl). Signed-off-by: Stefan Richter --- drivers/firewire/core-cdev.c | 38 ++++++++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 6 deletions(-) (limited to 'drivers') diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c index 3c1ac0933d2..a4aa477b9b2 100644 --- a/drivers/firewire/core-cdev.c +++ b/drivers/firewire/core-cdev.c @@ -1031,22 +1031,46 @@ static int ioctl_stop_iso(struct client *client, void *buffer) return fw_iso_context_stop(client->iso_context); } -static int ioctl_get_cycle_timer(struct client *client, void *buffer) +static int ioctl_get_cycle_timer2(struct client *client, void *buffer) { - struct fw_cdev_get_cycle_timer *request = buffer; + struct fw_cdev_get_cycle_timer2 *request = buffer; struct fw_card *card = client->device->card; - struct timeval tv; + struct timespec ts = {0, 0}; u32 cycle_time; + int ret = 0; local_irq_disable(); cycle_time = card->driver->get_cycle_time(card); - do_gettimeofday(&tv); + + switch (request->clk_id) { + case CLOCK_REALTIME: getnstimeofday(&ts); break; + case CLOCK_MONOTONIC: do_posix_clock_monotonic_gettime(&ts); break; + case CLOCK_MONOTONIC_RAW: getrawmonotonic(&ts); break; + default: + ret = -EINVAL; + } local_irq_enable(); - request->local_time = tv.tv_sec * 1000000ULL + tv.tv_usec; - request->cycle_timer = cycle_time; + request->tv_sec = ts.tv_sec; + request->tv_nsec = ts.tv_nsec; + request->cycle_timer = cycle_time; + + return ret; +} + +static int ioctl_get_cycle_timer(struct client *client, void *buffer) +{ + struct fw_cdev_get_cycle_timer *request = buffer; + struct fw_cdev_get_cycle_timer2 ct2; + + ct2.clk_id = CLOCK_REALTIME; + ioctl_get_cycle_timer2(client, &ct2); + + request->local_time = ct2.tv_sec * USEC_PER_SEC + + ct2.tv_nsec / NSEC_PER_USEC; + request->cycle_timer = ct2.cycle_timer; return 0; } @@ -1320,6 +1344,7 @@ static int (* const ioctl_handlers[])(struct client *client, void *buffer) = { ioctl_get_speed, ioctl_send_broadcast_request, ioctl_send_stream_packet, + ioctl_get_cycle_timer2, }; static int dispatch_ioctl(struct client *client, @@ -1341,6 +1366,7 @@ static int dispatch_ioctl(struct client *client, struct fw_cdev_get_cycle_timer _0c; struct fw_cdev_allocate_iso_resource _0d; struct fw_cdev_send_stream_packet _13; + struct fw_cdev_get_cycle_timer2 _14; })]; int ret; -- cgit v1.2.3-18-g5258 From 6e95dea728f4af36c033fcf2318529bd46dae540 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Sun, 21 Feb 2010 17:56:21 +0100 Subject: firewire: core: change type of a data buffer from array of char to union of structs. I already used a union to size the buffer which holds ioctl arguments; more consequent is to define it as an instance of this union in the first place. Also rename several local variables from "request" to "a"(rgument) since the term request can be mistaken to mean a transaction subaction, e.g. an instance of struct fw_request. Signed-off-by: Stefan Richter --- drivers/firewire/core-cdev.c | 325 ++++++++++++++++++++----------------------- 1 file changed, 152 insertions(+), 173 deletions(-) (limited to 'drivers') diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c index a4aa477b9b2..d7de17a0f25 100644 --- a/drivers/firewire/core-cdev.c +++ b/drivers/firewire/core-cdev.c @@ -368,39 +368,56 @@ void fw_device_cdev_remove(struct fw_device *device) for_each_client(device, wake_up_client); } -static int ioctl_get_info(struct client *client, void *buffer) +union ioctl_arg { + struct fw_cdev_get_info get_info; + struct fw_cdev_send_request send_request; + struct fw_cdev_allocate allocate; + struct fw_cdev_deallocate deallocate; + struct fw_cdev_send_response send_response; + struct fw_cdev_initiate_bus_reset initiate_bus_reset; + struct fw_cdev_add_descriptor add_descriptor; + struct fw_cdev_remove_descriptor remove_descriptor; + struct fw_cdev_create_iso_context create_iso_context; + struct fw_cdev_queue_iso queue_iso; + struct fw_cdev_start_iso start_iso; + struct fw_cdev_stop_iso stop_iso; + struct fw_cdev_get_cycle_timer get_cycle_timer; + struct fw_cdev_allocate_iso_resource allocate_iso_resource; + struct fw_cdev_send_stream_packet send_stream_packet; + struct fw_cdev_get_cycle_timer2 get_cycle_timer2; +}; + +static int ioctl_get_info(struct client *client, union ioctl_arg *arg) { - struct fw_cdev_get_info *get_info = buffer; + struct fw_cdev_get_info *a = &arg->get_info; struct fw_cdev_event_bus_reset bus_reset; unsigned long ret = 0; - client->version = get_info->version; - get_info->version = FW_CDEV_VERSION; - get_info->card = client->device->card->index; + client->version = a->version; + a->version = FW_CDEV_VERSION; + a->card = client->device->card->index; down_read(&fw_device_rwsem); - if (get_info->rom != 0) { - void __user *uptr = u64_to_uptr(get_info->rom); - size_t want = get_info->rom_length; + if (a->rom != 0) { + size_t want = a->rom_length; size_t have = client->device->config_rom_length * 4; - ret = copy_to_user(uptr, client->device->config_rom, - min(want, have)); + ret = copy_to_user(u64_to_uptr(a->rom), + client->device->config_rom, min(want, have)); } - get_info->rom_length = client->device->config_rom_length * 4; + a->rom_length = client->device->config_rom_length * 4; up_read(&fw_device_rwsem); if (ret != 0) return -EFAULT; - client->bus_reset_closure = get_info->bus_reset_closure; - if (get_info->bus_reset != 0) { - void __user *uptr = u64_to_uptr(get_info->bus_reset); - + client->bus_reset_closure = a->bus_reset_closure; + if (a->bus_reset != 0) { fill_bus_reset_event(&bus_reset, client); - if (copy_to_user(uptr, &bus_reset, sizeof(bus_reset))) + if (copy_to_user(u64_to_uptr(a->bus_reset), + &bus_reset, sizeof(bus_reset))) return -EFAULT; } @@ -571,11 +588,9 @@ static int init_request(struct client *client, return ret; } -static int ioctl_send_request(struct client *client, void *buffer) +static int ioctl_send_request(struct client *client, union ioctl_arg *arg) { - struct fw_cdev_send_request *request = buffer; - - switch (request->tcode) { + switch (arg->send_request.tcode) { case TCODE_WRITE_QUADLET_REQUEST: case TCODE_WRITE_BLOCK_REQUEST: case TCODE_READ_QUADLET_REQUEST: @@ -592,7 +607,7 @@ static int ioctl_send_request(struct client *client, void *buffer) return -EINVAL; } - return init_request(client, request, client->device->node_id, + return init_request(client, &arg->send_request, client->device->node_id, client->device->max_speed); } @@ -683,9 +698,9 @@ static void release_address_handler(struct client *client, kfree(r); } -static int ioctl_allocate(struct client *client, void *buffer) +static int ioctl_allocate(struct client *client, union ioctl_arg *arg) { - struct fw_cdev_allocate *request = buffer; + struct fw_cdev_allocate *a = &arg->allocate; struct address_handler_resource *r; struct fw_address_region region; int ret; @@ -694,13 +709,13 @@ static int ioctl_allocate(struct client *client, void *buffer) if (r == NULL) return -ENOMEM; - region.start = request->offset; - region.end = request->offset + request->length; - r->handler.length = request->length; + region.start = a->offset; + region.end = a->offset + a->length; + r->handler.length = a->length; r->handler.address_callback = handle_request; - r->handler.callback_data = r; - r->closure = request->closure; - r->client = client; + r->handler.callback_data = r; + r->closure = a->closure; + r->client = client; ret = fw_core_add_address_handler(&r->handler, ®ion); if (ret < 0) { @@ -714,27 +729,25 @@ static int ioctl_allocate(struct client *client, void *buffer) release_address_handler(client, &r->resource); return ret; } - request->handle = r->resource.handle; + a->handle = r->resource.handle; return 0; } -static int ioctl_deallocate(struct client *client, void *buffer) +static int ioctl_deallocate(struct client *client, union ioctl_arg *arg) { - struct fw_cdev_deallocate *request = buffer; - - return release_client_resource(client, request->handle, + return release_client_resource(client, arg->deallocate.handle, release_address_handler, NULL); } -static int ioctl_send_response(struct client *client, void *buffer) +static int ioctl_send_response(struct client *client, union ioctl_arg *arg) { - struct fw_cdev_send_response *request = buffer; + struct fw_cdev_send_response *a = &arg->send_response; struct client_resource *resource; struct inbound_transaction_resource *r; int ret = 0; - if (release_client_resource(client, request->handle, + if (release_client_resource(client, a->handle, release_request, &resource) < 0) return -EINVAL; @@ -743,28 +756,24 @@ static int ioctl_send_response(struct client *client, void *buffer) if (is_fcp_request(r->request)) goto out; - if (request->length < r->length) - r->length = request->length; - if (copy_from_user(r->data, u64_to_uptr(request->data), r->length)) { + if (a->length < r->length) + r->length = a->length; + if (copy_from_user(r->data, u64_to_uptr(a->data), r->length)) { ret = -EFAULT; kfree(r->request); goto out; } - fw_send_response(client->device->card, r->request, request->rcode); + fw_send_response(client->device->card, r->request, a->rcode); out: kfree(r); return ret; } -static int ioctl_initiate_bus_reset(struct client *client, void *buffer) +static int ioctl_initiate_bus_reset(struct client *client, union ioctl_arg *arg) { - struct fw_cdev_initiate_bus_reset *request = buffer; - int short_reset; - - short_reset = (request->type == FW_CDEV_SHORT_RESET); - - return fw_core_initiate_bus_reset(client->device->card, short_reset); + return fw_core_initiate_bus_reset(client->device->card, + arg->initiate_bus_reset.type == FW_CDEV_SHORT_RESET); } static void release_descriptor(struct client *client, @@ -777,9 +786,9 @@ static void release_descriptor(struct client *client, kfree(r); } -static int ioctl_add_descriptor(struct client *client, void *buffer) +static int ioctl_add_descriptor(struct client *client, union ioctl_arg *arg) { - struct fw_cdev_add_descriptor *request = buffer; + struct fw_cdev_add_descriptor *a = &arg->add_descriptor; struct descriptor_resource *r; int ret; @@ -787,22 +796,21 @@ static int ioctl_add_descriptor(struct client *client, void *buffer) if (!client->device->is_local) return -ENOSYS; - if (request->length > 256) + if (a->length > 256) return -EINVAL; - r = kmalloc(sizeof(*r) + request->length * 4, GFP_KERNEL); + r = kmalloc(sizeof(*r) + a->length * 4, GFP_KERNEL); if (r == NULL) return -ENOMEM; - if (copy_from_user(r->data, - u64_to_uptr(request->data), request->length * 4)) { + if (copy_from_user(r->data, u64_to_uptr(a->data), a->length * 4)) { ret = -EFAULT; goto failed; } - r->descriptor.length = request->length; - r->descriptor.immediate = request->immediate; - r->descriptor.key = request->key; + r->descriptor.length = a->length; + r->descriptor.immediate = a->immediate; + r->descriptor.key = a->key; r->descriptor.data = r->data; ret = fw_core_add_descriptor(&r->descriptor); @@ -815,7 +823,7 @@ static int ioctl_add_descriptor(struct client *client, void *buffer) fw_core_remove_descriptor(&r->descriptor); goto failed; } - request->handle = r->resource.handle; + a->handle = r->resource.handle; return 0; failed: @@ -824,11 +832,9 @@ static int ioctl_add_descriptor(struct client *client, void *buffer) return ret; } -static int ioctl_remove_descriptor(struct client *client, void *buffer) +static int ioctl_remove_descriptor(struct client *client, union ioctl_arg *arg) { - struct fw_cdev_remove_descriptor *request = buffer; - - return release_client_resource(client, request->handle, + return release_client_resource(client, arg->remove_descriptor.handle, release_descriptor, NULL); } @@ -851,49 +857,44 @@ static void iso_callback(struct fw_iso_context *context, u32 cycle, sizeof(e->interrupt) + header_length, NULL, 0); } -static int ioctl_create_iso_context(struct client *client, void *buffer) +static int ioctl_create_iso_context(struct client *client, union ioctl_arg *arg) { - struct fw_cdev_create_iso_context *request = buffer; + struct fw_cdev_create_iso_context *a = &arg->create_iso_context; struct fw_iso_context *context; /* We only support one context at this time. */ if (client->iso_context != NULL) return -EBUSY; - if (request->channel > 63) + if (a->channel > 63) return -EINVAL; - switch (request->type) { + switch (a->type) { case FW_ISO_CONTEXT_RECEIVE: - if (request->header_size < 4 || (request->header_size & 3)) + if (a->header_size < 4 || (a->header_size & 3)) return -EINVAL; - break; case FW_ISO_CONTEXT_TRANSMIT: - if (request->speed > SCODE_3200) + if (a->speed > SCODE_3200) return -EINVAL; - break; default: return -EINVAL; } - context = fw_iso_context_create(client->device->card, - request->type, - request->channel, - request->speed, - request->header_size, - iso_callback, client); + context = fw_iso_context_create(client->device->card, a->type, + a->channel, a->speed, a->header_size, + iso_callback, client); if (IS_ERR(context)) return PTR_ERR(context); - client->iso_closure = request->closure; + client->iso_closure = a->closure; client->iso_context = context; /* We only support one context at this time. */ - request->handle = 0; + a->handle = 0; return 0; } @@ -906,9 +907,9 @@ static int ioctl_create_iso_context(struct client *client, void *buffer) #define GET_SY(v) (((v) >> 20) & 0x0f) #define GET_HEADER_LENGTH(v) (((v) >> 24) & 0xff) -static int ioctl_queue_iso(struct client *client, void *buffer) +static int ioctl_queue_iso(struct client *client, union ioctl_arg *arg) { - struct fw_cdev_queue_iso *request = buffer; + struct fw_cdev_queue_iso *a = &arg->queue_iso; struct fw_cdev_iso_packet __user *p, *end, *next; struct fw_iso_context *ctx = client->iso_context; unsigned long payload, buffer_end, header_length; @@ -919,7 +920,7 @@ static int ioctl_queue_iso(struct client *client, void *buffer) u8 header[256]; } u; - if (ctx == NULL || request->handle != 0) + if (ctx == NULL || a->handle != 0) return -EINVAL; /* @@ -929,23 +930,23 @@ static int ioctl_queue_iso(struct client *client, void *buffer) * set them both to 0, which will still let packets with * payload_length == 0 through. In other words, if no packets * use the indirect payload, the iso buffer need not be mapped - * and the request->data pointer is ignored. + * and the a->data pointer is ignored. */ - payload = (unsigned long)request->data - client->vm_start; + payload = (unsigned long)a->data - client->vm_start; buffer_end = client->buffer.page_count << PAGE_SHIFT; - if (request->data == 0 || client->buffer.pages == NULL || + if (a->data == 0 || client->buffer.pages == NULL || payload >= buffer_end) { payload = 0; buffer_end = 0; } - p = (struct fw_cdev_iso_packet __user *)u64_to_uptr(request->packets); + p = (struct fw_cdev_iso_packet __user *)u64_to_uptr(a->packets); - if (!access_ok(VERIFY_READ, p, request->size)) + if (!access_ok(VERIFY_READ, p, a->size)) return -EFAULT; - end = (void __user *)p + request->size; + end = (void __user *)p + a->size; count = 0; while (p < end) { if (get_user(control, &p->control)) @@ -995,45 +996,41 @@ static int ioctl_queue_iso(struct client *client, void *buffer) count++; } - request->size -= uptr_to_u64(p) - request->packets; - request->packets = uptr_to_u64(p); - request->data = client->vm_start + payload; + a->size -= uptr_to_u64(p) - a->packets; + a->packets = uptr_to_u64(p); + a->data = client->vm_start + payload; return count; } -static int ioctl_start_iso(struct client *client, void *buffer) +static int ioctl_start_iso(struct client *client, union ioctl_arg *arg) { - struct fw_cdev_start_iso *request = buffer; + struct fw_cdev_start_iso *a = &arg->start_iso; - if (client->iso_context == NULL || request->handle != 0) + if (client->iso_context == NULL || a->handle != 0) return -EINVAL; - if (client->iso_context->type == FW_ISO_CONTEXT_RECEIVE) { - if (request->tags == 0 || request->tags > 15) - return -EINVAL; - - if (request->sync > 15) - return -EINVAL; - } + if (client->iso_context->type == FW_ISO_CONTEXT_RECEIVE && + (a->tags == 0 || a->tags > 15 || a->sync > 15)) + return -EINVAL; - return fw_iso_context_start(client->i