This moves tsi148 error handling into VME subsystem so it can be shared with
the other bridge driver. Then there is a change to close a fixme on separating
errors by address space. And finally a fix for memory leak problem that was
introduced with support of mmap's.
The next logical step in this direction would be to add error handling support
to ca91cx42 and make it unconditional for tsi148. It also makes much sense to
add synchronization to error-related list operations (spinlocks, rcu).
Dmitry Kalinkin (3):
vme: move tsi148 error handling into VME subsystem
vme: include address space in error filtering
vme: change bus error handling scheme
drivers/vme/bridges/vme_ca91cx42.c | 3 +-
drivers/vme/bridges/vme_tsi148.c | 170 ++++++++++---------------------------
drivers/vme/vme.c | 83 ++++++++++++++++++
drivers/vme/vme_bridge.h | 21 +++--
4 files changed, 147 insertions(+), 130 deletions(-)
--
1.8.3.1
Error handling code found in tsi148 is not device specific. In fact it
already relies on shared vme_bus_error struct and vme_bridge.vme_errors
field. The other bridge driver could reuse this code if it is shared.
This introduces a slight behavior change: vme error message won't be
triggered in a rare case when err_chk=1 and kmalloc fails.
Signed-off-by: Dmitry Kalinkin <[email protected]>
Cc: Igor Alekseev <[email protected]>
---
drivers/vme/bridges/vme_tsi148.c | 93 +++-------------------------------------
drivers/vme/vme.c | 86 +++++++++++++++++++++++++++++++++++++
drivers/vme/vme_bridge.h | 6 +++
3 files changed, 99 insertions(+), 86 deletions(-)
diff --git a/drivers/vme/bridges/vme_tsi148.c b/drivers/vme/bridges/vme_tsi148.c
index fb1e7ad..c1f9385 100644
--- a/drivers/vme/bridges/vme_tsi148.c
+++ b/drivers/vme/bridges/vme_tsi148.c
@@ -169,7 +169,6 @@ static u32 tsi148_VERR_irqhandler(struct vme_bridge *tsi148_bridge)
unsigned int error_addr_high, error_addr_low;
unsigned long long error_addr;
u32 error_attrib;
- struct vme_bus_error *error = NULL;
struct tsi148_driver *bridge;
bridge = tsi148_bridge->driver_priv;
@@ -186,23 +185,12 @@ static u32 tsi148_VERR_irqhandler(struct vme_bridge *tsi148_bridge)
"Occurred\n");
}
- if (err_chk) {
- error = kmalloc(sizeof(struct vme_bus_error), GFP_ATOMIC);
- if (error) {
- error->address = error_addr;
- error->attributes = error_attrib;
- list_add_tail(&error->list, &tsi148_bridge->vme_errors);
- } else {
- dev_err(tsi148_bridge->parent,
- "Unable to alloc memory for VMEbus Error reporting\n");
- }
- }
-
- if (!error) {
+ if (err_chk)
+ vme_bus_error_handler(tsi148_bridge, error_addr, error_attrib);
+ else
dev_err(tsi148_bridge->parent,
"VME Bus Error at address: 0x%llx, attributes: %08x\n",
error_addr, error_attrib);
- }
/* Clear Status */
iowrite32be(TSI148_LCSR_VEAT_VESCL, bridge->base + TSI148_LCSR_VEAT);
@@ -483,73 +471,6 @@ static int tsi148_irq_generate(struct vme_bridge *tsi148_bridge, int level,
}
/*
- * Find the first error in this address range
- */
-static struct vme_bus_error *tsi148_find_error(struct vme_bridge *tsi148_bridge,
- u32 aspace, unsigned long long address, size_t count)
-{
- struct list_head *err_pos;
- struct vme_bus_error *vme_err, *valid = NULL;
- unsigned long long bound;
-
- bound = address + count;
-
- /*
- * XXX We are currently not looking at the address space when parsing
- * for errors. This is because parsing the Address Modifier Codes
- * is going to be quite resource intensive to do properly. We
- * should be OK just looking at the addresses and this is certainly
- * much better than what we had before.
- */
- err_pos = NULL;
- /* Iterate through errors */
- list_for_each(err_pos, &tsi148_bridge->vme_errors) {
- vme_err = list_entry(err_pos, struct vme_bus_error, list);
- if ((vme_err->address >= address) &&
- (vme_err->address < bound)) {
-
- valid = vme_err;
- break;
- }
- }
-
- return valid;
-}
-
-/*
- * Clear errors in the provided address range.
- */
-static void tsi148_clear_errors(struct vme_bridge *tsi148_bridge,
- u32 aspace, unsigned long long address, size_t count)
-{
- struct list_head *err_pos, *temp;
- struct vme_bus_error *vme_err;
- unsigned long long bound;
-
- bound = address + count;
-
- /*
- * XXX We are currently not looking at the address space when parsing
- * for errors. This is because parsing the Address Modifier Codes
- * is going to be quite resource intensive to do properly. We
- * should be OK just looking at the addresses and this is certainly
- * much better than what we had before.
- */
- err_pos = NULL;
- /* Iterate through errors */
- list_for_each_safe(err_pos, temp, &tsi148_bridge->vme_errors) {
- vme_err = list_entry(err_pos, struct vme_bus_error, list);
-
- if ((vme_err->address >= address) &&
- (vme_err->address < bound)) {
-
- list_del(err_pos);
- kfree(vme_err);
- }
- }
-}
-
-/*
* Initialize a slave window with the requested attributes.
*/
static int tsi148_slave_set(struct vme_slave_resource *image, int enabled,
@@ -1323,14 +1244,14 @@ out:
__tsi148_master_get(image, &enabled, &vme_base, &size, &aspace, &cycle,
&dwidth);
- vme_err = tsi148_find_error(tsi148_bridge, aspace, vme_base + offset,
+ vme_err = vme_find_error(tsi148_bridge, aspace, vme_base + offset,
count);
if (vme_err != NULL) {
dev_err(image->parent->parent, "First VME read error detected "
"an at address 0x%llx\n", vme_err->address);
retval = vme_err->address - (vme_base + offset);
/* Clear down save errors in this address range */
- tsi148_clear_errors(tsi148_bridge, aspace, vme_base + offset,
+ vme_clear_errors(tsi148_bridge, aspace, vme_base + offset,
count);
}
@@ -1422,14 +1343,14 @@ out:
ioread16(bridge->flush_image->kern_base + 0x7F000);
- vme_err = tsi148_find_error(tsi148_bridge, aspace, vme_base + offset,
+ vme_err = vme_find_error(tsi148_bridge, aspace, vme_base + offset,
count);
if (vme_err != NULL) {
dev_warn(tsi148_bridge->parent, "First VME write error detected"
" an at address 0x%llx\n", vme_err->address);
retval = vme_err->address - (vme_base + offset);
/* Clear down save errors in this address range */
- tsi148_clear_errors(tsi148_bridge, aspace, vme_base + offset,
+ vme_clear_errors(tsi148_bridge, aspace, vme_base + offset,
count);
}
diff --git a/drivers/vme/vme.c b/drivers/vme/vme.c
index 5670891..6803744 100644
--- a/drivers/vme/vme.c
+++ b/drivers/vme/vme.c
@@ -990,6 +990,92 @@ int vme_dma_free(struct vme_resource *resource)
}
EXPORT_SYMBOL(vme_dma_free);
+void vme_bus_error_handler(struct vme_bridge *bridge,
+ unsigned long long address, u32 attributes)
+{
+ struct vme_bus_error *error;
+
+ error = kmalloc(sizeof(struct vme_bus_error), GFP_ATOMIC);
+ if (error) {
+ error->address = address;
+ error->attributes = attributes;
+ list_add_tail(&error->list, &bridge->vme_errors);
+ } else {
+ dev_err(bridge->parent,
+ "Unable to alloc memory for VMEbus Error reporting\n");
+ }
+}
+EXPORT_SYMBOL(vme_bus_error_handler);
+
+/*
+ * Find the first error in this address range
+ */
+struct vme_bus_error *vme_find_error(struct vme_bridge *bridge, u32 aspace,
+ unsigned long long address, size_t count)
+{
+ struct list_head *err_pos;
+ struct vme_bus_error *vme_err, *valid = NULL;
+ unsigned long long bound;
+
+ bound = address + count;
+
+ /*
+ * XXX We are currently not looking at the address space when parsing
+ * for errors. This is because parsing the Address Modifier Codes
+ * is going to be quite resource intensive to do properly. We
+ * should be OK just looking at the addresses and this is certainly
+ * much better than what we had before.
+ */
+ err_pos = NULL;
+ /* Iterate through errors */
+ list_for_each(err_pos, &bridge->vme_errors) {
+ vme_err = list_entry(err_pos, struct vme_bus_error, list);
+ if ((vme_err->address >= address) &&
+ (vme_err->address < bound)) {
+
+ valid = vme_err;
+ break;
+ }
+ }
+
+ return valid;
+}
+EXPORT_SYMBOL(vme_find_error);
+
+/*
+ * Clear errors in the provided address range.
+ */
+void vme_clear_errors(struct vme_bridge *bridge, u32 aspace,
+ unsigned long long address, size_t count)
+{
+ struct list_head *err_pos, *temp;
+ struct vme_bus_error *vme_err;
+ unsigned long long bound;
+
+ bound = address + count;
+
+ /*
+ * XXX We are currently not looking at the address space when parsing
+ * for errors. This is because parsing the Address Modifier Codes
+ * is going to be quite resource intensive to do properly. We
+ * should be OK just looking at the addresses and this is certainly
+ * much better than what we had before.
+ */
+ err_pos = NULL;
+ /* Iterate through errors */
+ list_for_each_safe(err_pos, temp, &bridge->vme_errors) {
+ vme_err = list_entry(err_pos, struct vme_bus_error, list);
+
+ if ((vme_err->address >= address) &&
+ (vme_err->address < bound)) {
+
+ list_del(err_pos);
+ kfree(vme_err);
+ }
+ }
+}
+EXPORT_SYMBOL(vme_clear_errors);
+
void vme_irq_handler(struct vme_bridge *bridge, int level, int statid)
{
void (*call)(int, int, void *);
diff --git a/drivers/vme/vme_bridge.h b/drivers/vme/vme_bridge.h
index 934949a..d8d6b14 100644
--- a/drivers/vme/vme_bridge.h
+++ b/drivers/vme/vme_bridge.h
@@ -166,6 +166,12 @@ struct vme_bridge {
void *vaddr, dma_addr_t dma);
};
+void vme_bus_error_handler(struct vme_bridge *bridge,
+ unsigned long long address, u32 attributes);
+struct vme_bus_error *vme_find_error(struct vme_bridge *bridge, u32 aspace,
+ unsigned long long address, size_t count);
+void vme_clear_errors(struct vme_bridge *bridge, u32 aspace,
+ unsigned long long address, size_t count);
void vme_irq_handler(struct vme_bridge *, int, int);
int vme_register_bridge(struct vme_bridge *);
--
1.8.3.1
Also changes vme_bus_error_handler to take generic address modifier code
instead of raw contents of a device-specific attribute register.
Signed-off-by: Dmitry Kalinkin <[email protected]>
Cc: Igor Alekseev <[email protected]>
---
drivers/vme/bridges/vme_tsi148.c | 4 ++-
drivers/vme/vme.c | 61 +++++++++++++++++++++++++++-------------
drivers/vme/vme_bridge.h | 4 +--
3 files changed, 46 insertions(+), 23 deletions(-)
diff --git a/drivers/vme/bridges/vme_tsi148.c b/drivers/vme/bridges/vme_tsi148.c
index c1f9385..76ccfae 100644
--- a/drivers/vme/bridges/vme_tsi148.c
+++ b/drivers/vme/bridges/vme_tsi148.c
@@ -169,6 +169,7 @@ static u32 tsi148_VERR_irqhandler(struct vme_bridge *tsi148_bridge)
unsigned int error_addr_high, error_addr_low;
unsigned long long error_addr;
u32 error_attrib;
+ int error_am;
struct tsi148_driver *bridge;
bridge = tsi148_bridge->driver_priv;
@@ -176,6 +177,7 @@ static u32 tsi148_VERR_irqhandler(struct vme_bridge *tsi148_bridge)
error_addr_high = ioread32be(bridge->base + TSI148_LCSR_VEAU);
error_addr_low = ioread32be(bridge->base + TSI148_LCSR_VEAL);
error_attrib = ioread32be(bridge->base + TSI148_LCSR_VEAT);
+ error_am = (error_attrib & TSI148_LCSR_VEAT_AM_M) >> 8;
reg_join(error_addr_high, error_addr_low, &error_addr);
@@ -186,7 +188,7 @@ static u32 tsi148_VERR_irqhandler(struct vme_bridge *tsi148_bridge)
}
if (err_chk)
- vme_bus_error_handler(tsi148_bridge, error_addr, error_attrib);
+ vme_bus_error_handler(tsi148_bridge, error_addr, error_am);
else
dev_err(tsi148_bridge->parent,
"VME Bus Error at address: 0x%llx, attributes: %08x\n",
diff --git a/drivers/vme/vme.c b/drivers/vme/vme.c
index 6803744..2b79cd2 100644
--- a/drivers/vme/vme.c
+++ b/drivers/vme/vme.c
@@ -223,6 +223,39 @@ int vme_check_window(u32 aspace, unsigned long long vme_base,
}
EXPORT_SYMBOL(vme_check_window);
+static u32 vme_get_aspace(int am)
+{
+ switch (am) {
+ case 0x29:
+ case 0x2D:
+ return VME_A16;
+ case 0x38:
+ case 0x39:
+ case 0x3A:
+ case 0x3B:
+ case 0x3C:
+ case 0x3D:
+ case 0x3E:
+ case 0x3F:
+ return VME_A24;
+ case 0x8:
+ case 0x9:
+ case 0xA:
+ case 0xB:
+ case 0xC:
+ case 0xD:
+ case 0xE:
+ case 0xF:
+ return VME_A32;
+ case 0x0:
+ case 0x1:
+ case 0x3:
+ return VME_A64;
+ }
+
+ return 0;
+}
+
/*
* Request a slave image with specific attributes, return some unique
* identifier.
@@ -991,14 +1024,14 @@ int vme_dma_free(struct vme_resource *resource)
EXPORT_SYMBOL(vme_dma_free);
void vme_bus_error_handler(struct vme_bridge *bridge,
- unsigned long long address, u32 attributes)
+ unsigned long long address, int am)
{
struct vme_bus_error *error;
error = kmalloc(sizeof(struct vme_bus_error), GFP_ATOMIC);
if (error) {
+ error->aspace = vme_get_aspace(am);
error->address = address;
- error->attributes = attributes;
list_add_tail(&error->list, &bridge->vme_errors);
} else {
dev_err(bridge->parent,
@@ -1019,19 +1052,13 @@ struct vme_bus_error *vme_find_error(struct vme_bridge *bridge, u32 aspace,
bound = address + count;
- /*
- * XXX We are currently not looking at the address space when parsing
- * for errors. This is because parsing the Address Modifier Codes
- * is going to be quite resource intensive to do properly. We
- * should be OK just looking at the addresses and this is certainly
- * much better than what we had before.
- */
err_pos = NULL;
/* Iterate through errors */
list_for_each(err_pos, &bridge->vme_errors) {
vme_err = list_entry(err_pos, struct vme_bus_error, list);
- if ((vme_err->address >= address) &&
- (vme_err->address < bound)) {
+ if ((vme_err->aspace == aspace) &&
+ (vme_err->address >= address) &&
+ (vme_err->address < bound)) {
valid = vme_err;
break;
@@ -1054,20 +1081,14 @@ void vme_clear_errors(struct vme_bridge *bridge, u32 aspace,
bound = address + count;
- /*
- * XXX We are currently not looking at the address space when parsing
- * for errors. This is because parsing the Address Modifier Codes
- * is going to be quite resource intensive to do properly. We
- * should be OK just looking at the addresses and this is certainly
- * much better than what we had before.
- */
err_pos = NULL;
/* Iterate through errors */
list_for_each_safe(err_pos, temp, &bridge->vme_errors) {
vme_err = list_entry(err_pos, struct vme_bus_error, list);
- if ((vme_err->address >= address) &&
- (vme_err->address < bound)) {
+ if ((vme_err->aspace == aspace) &&
+ (vme_err->address >= address) &&
+ (vme_err->address < bound)) {
list_del(err_pos);
kfree(vme_err);
diff --git a/drivers/vme/vme_bridge.h b/drivers/vme/vme_bridge.h
index d8d6b14..92fbe18 100644
--- a/drivers/vme/vme_bridge.h
+++ b/drivers/vme/vme_bridge.h
@@ -77,8 +77,8 @@ struct vme_lm_resource {
struct vme_bus_error {
struct list_head list;
+ u32 aspace;
unsigned long long address;
- u32 attributes;
};
struct vme_callback {
@@ -167,7 +167,7 @@ struct vme_bridge {
};
void vme_bus_error_handler(struct vme_bridge *bridge,
- unsigned long long address, u32 attributes);
+ unsigned long long address, int am);
struct vme_bus_error *vme_find_error(struct vme_bridge *bridge, u32 aspace,
unsigned long long address, size_t count);
void vme_clear_errors(struct vme_bridge *bridge, u32 aspace,
--
1.8.3.1
The current VME bus error handler adds errors to the bridge error list.
vme_master_{read,write} then traverses that list to look for relevant
errors.
Such scheme didn't work well for accesses going through vme_master_mmap
because they would also allocate a vme_bus_error, but have no way to do
vme_clear_errors call to free that memory.
This changes the error handling process to be other way around: now
vme_master_{read,write} defines a window in VME address space that will
catch possible errors. VME bus error interrupt only traverses these
windows and marks those that had errors in them.
Signed-off-by: Dmitry Kalinkin <[email protected]>
Cc: Igor Alekseev <[email protected]>
---
drivers/vme/bridges/vme_ca91cx42.c | 3 +-
drivers/vme/bridges/vme_tsi148.c | 83 +++++++++++++++++-----------------
drivers/vme/vme.c | 92 ++++++++++++++------------------------
drivers/vme/vme_bridge.h | 23 ++++++----
4 files changed, 91 insertions(+), 110 deletions(-)
diff --git a/drivers/vme/bridges/vme_ca91cx42.c b/drivers/vme/bridges/vme_ca91cx42.c
index f692efc..834883d 100644
--- a/drivers/vme/bridges/vme_ca91cx42.c
+++ b/drivers/vme/bridges/vme_ca91cx42.c
@@ -204,8 +204,7 @@ static int ca91cx42_irq_init(struct vme_bridge *ca91cx42_bridge)
/* Need pdev */
pdev = container_of(ca91cx42_bridge->parent, struct pci_dev, dev);
- /* Initialise list for VME bus errors */
- INIT_LIST_HEAD(&ca91cx42_bridge->vme_errors);
+ INIT_LIST_HEAD(&ca91cx42_bridge->vme_error_handlers);
mutex_init(&ca91cx42_bridge->irq_mtx);
diff --git a/drivers/vme/bridges/vme_tsi148.c b/drivers/vme/bridges/vme_tsi148.c
index 76ccfae..b0132e0 100644
--- a/drivers/vme/bridges/vme_tsi148.c
+++ b/drivers/vme/bridges/vme_tsi148.c
@@ -314,8 +314,7 @@ static int tsi148_irq_init(struct vme_bridge *tsi148_bridge)
bridge = tsi148_bridge->driver_priv;
- /* Initialise list for VME bus errors */
- INIT_LIST_HEAD(&tsi148_bridge->vme_errors);
+ INIT_LIST_HEAD(&tsi148_bridge->vme_error_handlers);
mutex_init(&tsi148_bridge->irq_mtx);
@@ -1187,7 +1186,7 @@ static ssize_t tsi148_master_read(struct vme_master_resource *image, void *buf,
int retval, enabled;
unsigned long long vme_base, size;
u32 aspace, cycle, dwidth;
- struct vme_bus_error *vme_err = NULL;
+ struct vme_error_handler *handler;
struct vme_bridge *tsi148_bridge;
void __iomem *addr = image->kern_base + offset;
unsigned int done = 0;
@@ -1197,6 +1196,17 @@ static ssize_t tsi148_master_read(struct vme_master_resource *image, void *buf,
spin_lock(&image->lock);
+ if (err_chk) {
+ __tsi148_master_get(image, &enabled, &vme_base, &size, &aspace,
+ &cycle, &dwidth);
+ handler = vme_register_error_handler(tsi148_bridge, aspace,
+ vme_base + offset, count);
+ if (!handler) {
+ spin_unlock(&image->lock);
+ return -ENOMEM;
+ }
+ }
+
/* The following code handles VME address alignment. We cannot use
* memcpy_xxx here because it may cut data transfers in to 8-bit
* cycles when D16 or D32 cycles are required on the VME bus.
@@ -1240,24 +1250,16 @@ static ssize_t tsi148_master_read(struct vme_master_resource *image, void *buf,
out:
retval = count;
- if (!err_chk)
- goto skip_chk;
-
- __tsi148_master_get(image, &enabled, &vme_base, &size, &aspace, &cycle,
- &dwidth);
-
- vme_err = vme_find_error(tsi148_bridge, aspace, vme_base + offset,
- count);
- if (vme_err != NULL) {
- dev_err(image->parent->parent, "First VME read error detected "
- "an at address 0x%llx\n", vme_err->address);
- retval = vme_err->address - (vme_base + offset);
- /* Clear down save errors in this address range */
- vme_clear_errors(tsi148_bridge, aspace, vme_base + offset,
- count);
+ if (err_chk) {
+ if (handler->num_errors) {
+ dev_err(image->parent->parent,
+ "First VME read error detected an at address 0x%llx\n",
+ handler->first_error);
+ retval = handler->first_error - (vme_base + offset);
+ }
+ vme_unregister_error_handler(handler);
}
-skip_chk:
spin_unlock(&image->lock);
return retval;
@@ -1274,7 +1276,7 @@ static ssize_t tsi148_master_write(struct vme_master_resource *image, void *buf,
unsigned int done = 0;
unsigned int count32;
- struct vme_bus_error *vme_err = NULL;
+ struct vme_error_handler *handler;
struct vme_bridge *tsi148_bridge;
struct tsi148_driver *bridge;
@@ -1284,6 +1286,17 @@ static ssize_t tsi148_master_write(struct vme_master_resource *image, void *buf,
spin_lock(&image->lock);
+ if (err_chk) {
+ __tsi148_master_get(image, &enabled, &vme_base, &size, &aspace,
+ &cycle, &dwidth);
+ handler = vme_register_error_handler(tsi148_bridge, aspace,
+ vme_base + offset, count);
+ if (!handler) {
+ spin_unlock(&image->lock);
+ return -ENOMEM;
+ }
+ }
+
/* Here we apply for the same strategy we do in master_read
* function in order to assure the correct cycles.
*/
@@ -1333,30 +1346,18 @@ out:
* We check for saved errors in the written address range/space.
*/
- if (!err_chk)
- goto skip_chk;
-
- /*
- * Get window info first, to maximise the time that the buffers may
- * fluch on their own
- */
- __tsi148_master_get(image, &enabled, &vme_base, &size, &aspace, &cycle,
- &dwidth);
-
- ioread16(bridge->flush_image->kern_base + 0x7F000);
+ if (err_chk) {
+ ioread16(bridge->flush_image->kern_base + 0x7F000);
- vme_err = vme_find_error(tsi148_bridge, aspace, vme_base + offset,
- count);
- if (vme_err != NULL) {
- dev_warn(tsi148_bridge->parent, "First VME write error detected"
- " an at address 0x%llx\n", vme_err->address);
- retval = vme_err->address - (vme_base + offset);
- /* Clear down save errors in this address range */
- vme_clear_errors(tsi148_bridge, aspace, vme_base + offset,
- count);
+ if (handler->num_errors) {
+ dev_warn(tsi148_bridge->parent,
+ "First VME write error detected an at address 0x%llx\n",
+ handler->first_error);
+ retval = handler->first_error - (vme_base + offset);
+ }
+ vme_unregister_error_handler(handler);
}
-skip_chk:
spin_unlock(&image->lock);
return retval;
diff --git a/drivers/vme/vme.c b/drivers/vme/vme.c
index 2b79cd2..7a10d92 100644
--- a/drivers/vme/vme.c
+++ b/drivers/vme/vme.c
@@ -1026,76 +1026,52 @@ EXPORT_SYMBOL(vme_dma_free);
void vme_bus_error_handler(struct vme_bridge *bridge,
unsigned long long address, int am)
{
- struct vme_bus_error *error;
-
- error = kmalloc(sizeof(struct vme_bus_error), GFP_ATOMIC);
- if (error) {
- error->aspace = vme_get_aspace(am);
- error->address = address;
- list_add_tail(&error->list, &bridge->vme_errors);
- } else {
- dev_err(bridge->parent,
- "Unable to alloc memory for VMEbus Error reporting\n");
+ struct list_head *handler_pos = NULL;
+ struct vme_error_handler *handler;
+ u32 aspace = vme_get_aspace(am);
+
+ list_for_each(handler_pos, &bridge->vme_error_handlers) {
+ handler = list_entry(handler_pos, struct vme_error_handler,
+ list);
+ if ((aspace == handler->aspace) &&
+ (address >= handler->start) &&
+ (address < handler->end)) {
+ if (!handler->num_errors)
+ handler->first_error = address;
+ if (handler->num_errors != UINT_MAX)
+ handler->num_errors++;
+ }
}
}
EXPORT_SYMBOL(vme_bus_error_handler);
-/*
- * Find the first error in this address range
- */
-struct vme_bus_error *vme_find_error(struct vme_bridge *bridge, u32 aspace,
- unsigned long long address, size_t count)
+struct vme_error_handler *vme_register_error_handler(
+ struct vme_bridge *bridge, u32 aspace,
+ unsigned long long address, size_t len)
{
- struct list_head *err_pos;
- struct vme_bus_error *vme_err, *valid = NULL;
- unsigned long long bound;
-
- bound = address + count;
+ struct vme_error_handler *handler;
- err_pos = NULL;
- /* Iterate through errors */
- list_for_each(err_pos, &bridge->vme_errors) {
- vme_err = list_entry(err_pos, struct vme_bus_error, list);
- if ((vme_err->aspace == aspace) &&
- (vme_err->address >= address) &&
- (vme_err->address < bound)) {
+ handler = kmalloc(sizeof(*handler), GFP_KERNEL);
+ if (!handler)
+ return NULL;
- valid = vme_err;
- break;
- }
- }
+ handler->aspace = aspace;
+ handler->start = address;
+ handler->end = address + len;
+ handler->num_errors = 0;
+ handler->first_error = 0;
+ list_add_tail(&handler->list, &bridge->vme_error_handlers);
- return valid;
+ return handler;
}
-EXPORT_SYMBOL(vme_find_error);
+EXPORT_SYMBOL(vme_register_error_handler);
-/*
- * Clear errors in the provided address range.
- */
-void vme_clear_errors(struct vme_bridge *bridge, u32 aspace,
- unsigned long long address, size_t count)
+void vme_unregister_error_handler(struct vme_error_handler *handler)
{
- struct list_head *err_pos, *temp;
- struct vme_bus_error *vme_err;
- unsigned long long bound;
-
- bound = address + count;
-
- err_pos = NULL;
- /* Iterate through errors */
- list_for_each_safe(err_pos, temp, &bridge->vme_errors) {
- vme_err = list_entry(err_pos, struct vme_bus_error, list);
-
- if ((vme_err->aspace == aspace) &&
- (vme_err->address >= address) &&
- (vme_err->address < bound)) {
-
- list_del(err_pos);
- kfree(vme_err);
- }
- }
+ list_del(&handler->list);
+ kfree(handler);
}
-EXPORT_SYMBOL(vme_clear_errors);
+EXPORT_SYMBOL(vme_unregister_error_handler);
void vme_irq_handler(struct vme_bridge *bridge, int level, int statid)
{
diff --git a/drivers/vme/vme_bridge.h b/drivers/vme/vme_bridge.h
index 92fbe18..397578a 100644
--- a/drivers/vme/vme_bridge.h
+++ b/drivers/vme/vme_bridge.h
@@ -75,10 +75,13 @@ struct vme_lm_resource {
int monitors;
};
-struct vme_bus_error {
+struct vme_error_handler {
struct list_head list;
- u32 aspace;
- unsigned long long address;
+ unsigned long long start; /* Beginning of error window */
+ unsigned long long end; /* End of error window */
+ unsigned long long first_error; /* Address of the first error */
+ u32 aspace; /* Address space of error window*/
+ unsigned num_errors; /* Number of errors */
};
struct vme_callback {
@@ -106,8 +109,10 @@ struct vme_bridge {
struct list_head dma_resources;
struct list_head lm_resources;
- struct list_head vme_errors; /* List for errors generated on VME */
- struct list_head devices; /* List of devices on this bridge */
+ /* List for registered errors handlers */
+ struct list_head vme_error_handlers;
+ /* List of devices on this bridge */
+ struct list_head devices;
/* Bridge Info - XXX Move to private structure? */
struct device *parent; /* Parent device (eg. pdev->dev for PCI) */
@@ -168,13 +173,13 @@ struct vme_bridge {
void vme_bus_error_handler(struct vme_bridge *bridge,
unsigned long long address, int am);
-struct vme_bus_error *vme_find_error(struct vme_bridge *bridge, u32 aspace,
- unsigned long long address, size_t count);
-void vme_clear_errors(struct vme_bridge *bridge, u32 aspace,
- unsigned long long address, size_t count);
void vme_irq_handler(struct vme_bridge *, int, int);
int vme_register_bridge(struct vme_bridge *);
void vme_unregister_bridge(struct vme_bridge *);
+struct vme_error_handler *vme_register_error_handler(
+ struct vme_bridge *bridge, u32 aspace,
+ unsigned long long address, size_t len);
+void vme_unregister_error_handler(struct vme_error_handler *handler);
#endif /* _VME_BRIDGE_H_ */
--
1.8.3.1
Hi Dmitry,
These are looking good to me.
You mention error handling in relation to vme_master_mmap, am I right in
thinking patch 3 avoids errors being recorded when triggered by an mmap
access (so as not to appear as a spurious error on a later access)?
I think it would be worth at least logging errors to the kernel log
should they be generated and not be handled by a error handler, so
someone using mmap gets at least some form of notification that their
accesses are resulting on bus errors. What do you think?
Martyn
On 02/07/15 15:11, Dmitry Kalinkin wrote:
> This moves tsi148 error handling into VME subsystem so it can be shared with
> the other bridge driver. Then there is a change to close a fixme on separating
> errors by address space. And finally a fix for memory leak problem that was
> introduced with support of mmap's.
>
> The next logical step in this direction would be to add error handling support
> to ca91cx42 and make it unconditional for tsi148. It also makes much sense to
> add synchronization to error-related list operations (spinlocks, rcu).
>
> Dmitry Kalinkin (3):
> vme: move tsi148 error handling into VME subsystem
> vme: include address space in error filtering
> vme: change bus error handling scheme
>
> drivers/vme/bridges/vme_ca91cx42.c | 3 +-
> drivers/vme/bridges/vme_tsi148.c | 170 ++++++++++---------------------------
> drivers/vme/vme.c | 83 ++++++++++++++++++
> drivers/vme/vme_bridge.h | 21 +++--
> 4 files changed, 147 insertions(+), 130 deletions(-)
>
--
Martyn Welch (Lead Software Engineer) | Registered in England and Wales
GE Intelligent Platforms | (3828642) at 100 Barbirolli Square
T +44(0)1327322748 | Manchester, M2 3AB
E [email protected] | VAT:GB 927559189
Hi Martyn,
> On 06 Jul 2015, at 15:31, Martyn Welch <[email protected]> wrote:
>
> These are looking good to me.
Thank you for finding time to look at this and the others.
>
> You mention error handling in relation to vme_master_mmap, am I right in thinking patch 3 avoids errors being recorded when triggered by an mmap access (so as not to appear as a spurious error on a later access)?
Yes, this is covered as well.
>
> I think it would be worth at least logging errors to the kernel log should they be generated and not be handled by a error handler, so someone using mmap gets at least some form of notification that their accesses are resulting on bus errors. What do you think?
Yes. I agree with you. Bus error handler could print a message when an error doesn?t trigger any of the registered handlers.
What would be really useful, is if we could dispatch such errors to the userspace in a more direct way. Not sure how, though.
Cheers,
Dmitry
>
>
> On 02/07/15 15:11, Dmitry Kalinkin wrote:
>> This moves tsi148 error handling into VME subsystem so it can be shared with
>> the other bridge driver. Then there is a change to close a fixme on separating
>> errors by address space. And finally a fix for memory leak problem that was
>> introduced with support of mmap's.
>>
>> The next logical step in this direction would be to add error handling support
>> to ca91cx42 and make it unconditional for tsi148. It also makes much sense to
>> add synchronization to error-related list operations (spinlocks, rcu).
>>
>> Dmitry Kalinkin (3):
>> vme: move tsi148 error handling into VME subsystem
>> vme: include address space in error filtering
>> vme: change bus error handling scheme
>>
>> drivers/vme/bridges/vme_ca91cx42.c | 3 +-
>> drivers/vme/bridges/vme_tsi148.c | 170 ++++++++++---------------------------
>> drivers/vme/vme.c | 83 ++++++++++++++++++
>> drivers/vme/vme_bridge.h | 21 +++--
>> 4 files changed, 147 insertions(+), 130 deletions(-)
>>
>
> --
> Martyn Welch (Lead Software Engineer) | Registered in England and Wales
> GE Intelligent Platforms | (3828642) at 100 Barbirolli Square
> T +44(0)1327322748 | Manchester, M2 3AB
> E [email protected] | VAT:GB 927559189
This will enable error messages for accesses done through mmap.
Signed-off-by: Dmitry Kalinkin <[email protected]>
---
This depends on '[PATCH 0/3] VME bus error handling overhaul' patchset.
---
drivers/vme/vme.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/vme/vme.c b/drivers/vme/vme.c
index 7a10d92..72924b0 100644
--- a/drivers/vme/vme.c
+++ b/drivers/vme/vme.c
@@ -1028,6 +1028,7 @@ void vme_bus_error_handler(struct vme_bridge *bridge,
{
struct list_head *handler_pos = NULL;
struct vme_error_handler *handler;
+ int handler_triggered = 0;
u32 aspace = vme_get_aspace(am);
list_for_each(handler_pos, &bridge->vme_error_handlers) {
@@ -1040,8 +1041,14 @@ void vme_bus_error_handler(struct vme_bridge *bridge,
handler->first_error = address;
if (handler->num_errors != UINT_MAX)
handler->num_errors++;
+ handler_triggered = 1;
}
}
+
+ if (!handler_triggered)
+ dev_err(bridge->parent,
+ "Unhandled VME access error at address 0x%llx\n",
+ address);
}
EXPORT_SYMBOL(vme_bus_error_handler);
--
1.8.3.1
That's the ticket :-)
On 06/07/15 15:43, Dmitry Kalinkin wrote:
> This will enable error messages for accesses done through mmap.
>
> Signed-off-by: Dmitry Kalinkin <[email protected]>
> ---
> This depends on '[PATCH 0/3] VME bus error handling overhaul' patchset.
> ---
> drivers/vme/vme.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/vme/vme.c b/drivers/vme/vme.c
> index 7a10d92..72924b0 100644
> --- a/drivers/vme/vme.c
> +++ b/drivers/vme/vme.c
> @@ -1028,6 +1028,7 @@ void vme_bus_error_handler(struct vme_bridge *bridge,
> {
> struct list_head *handler_pos = NULL;
> struct vme_error_handler *handler;
> + int handler_triggered = 0;
> u32 aspace = vme_get_aspace(am);
>
> list_for_each(handler_pos, &bridge->vme_error_handlers) {
> @@ -1040,8 +1041,14 @@ void vme_bus_error_handler(struct vme_bridge *bridge,
> handler->first_error = address;
> if (handler->num_errors != UINT_MAX)
> handler->num_errors++;
> + handler_triggered = 1;
> }
> }
> +
> + if (!handler_triggered)
> + dev_err(bridge->parent,
> + "Unhandled VME access error at address 0x%llx\n",
> + address);
> }
> EXPORT_SYMBOL(vme_bus_error_handler);
>
>
--
Martyn Welch (Lead Software Engineer) | Registered in England and Wales
GE Intelligent Platforms | (3828642) at 100 Barbirolli Square
T +44(0)1327322748 | Manchester, M2 3AB
E [email protected] | VAT:GB 927559189
On Mon, Jul 06, 2015 at 01:31:47PM +0100, Martyn Welch wrote:
> Hi Dmitry,
>
> These are looking good to me.
Can I get an "Acked-by:" or something so that I know it's ok to apply
these?
thanks,
greg k-h-