* [PATCH] regmap: introduce regmap_name to fix syscon regmap trace events
@ 2015-03-06 14:16 Philipp Zabel
2015-03-07 10:57 ` Mark Brown
0 siblings, 1 reply; 7+ messages in thread
From: Philipp Zabel @ 2015-03-06 14:16 UTC (permalink / raw)
To: Mark Brown
Cc: Pankaj Dubey, Steven Rostedt, Ingo Molnar, linux-kernel, kernel,
Philipp Zabel
This patch fixes a NULL pointer dereference when enabling regmap event
tracing in the presence of a syscon regmap, introduced by commit bdb0066df96e
("mfd: syscon: Decouple syscon interface from platform devices").
That patch introduced syscon regmaps that have their dev field set to NULL.
The regmap trace events expect it to point to a valid struct device and feed
it to dev_name():
$ echo 1 > /sys/kernel/debug/tracing/events/regmap/enable
Unable to handle kernel NULL pointer dereference at virtual address 0000002c
pgd = 80004000
[0000002c] *pgd=00000000
Internal error: Oops: 17 [#1] SMP ARM
Modules linked in: coda videobuf2_vmalloc
CPU: 0 PID: 304 Comm: kworker/0:2 Not tainted 4.0.0-rc2+ #9197
Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
Workqueue: events_freezable thermal_zone_device_check
task: 9f25a200 ti: 9f1ee000 task.ti: 9f1ee000
PC is at ftrace_raw_event_regmap_block+0x3c/0xe4
LR is at _regmap_raw_read+0x1bc/0x1cc
pc : [<803636e8>] lr : [<80365f2c>] psr: 600f0093
sp : 9f1efd78 ip : 9f1efdb8 fp : 9f1efdb4
r10: 00000004 r9 : 00000001 r8 : 00000001
r7 : 00000180 r6 : 00000000 r5 : 9f00e3c0 r4 : 00000003
r3 : 00000001 r2 : 00000180 r1 : 00000000 r0 : 9f00e3c0
Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment kernel
Control: 10c5387d Table: 2d91004a DAC: 00000015
Process kworker/0:2 (pid: 304, stack limit = 0x9f1ee210)
Stack: (0x9f1efd78 to 0x9f1f0000)
fd60: 9f1efda4 9f1efd88
fd80: 800708c0 805f9510 80927140 800f0013 9f1fc800 9eb2f490 00000000 00000180
fda0: 808e3840 00000001 9f1efdfc 9f1efdb8 80365f2c 803636b8 805f8958 800708e0
fdc0: a00f0013 803636ac 9f16de00 00000180 80927140 9f1fc800 9f1fc800 9f1efe6c
fde0: 9f1efe6c 9f732400 00000000 00000000 9f1efe1c 9f1efe00 80365f70 80365d7c
fe00: 80365f3c 9f1fc800 9f1fc800 00000180 9f1efe44 9f1efe20 803656a4 80365f48
fe20: 9f1fc800 00000180 9f1efe6c 9f1efe6c 9f732400 00000000 9f1efe64 9f1efe48
fe40: 803657bc 80365634 00000001 9e95f910 9f1fc800 9f1efeb4 9f1efe8c 9f1efe68
fe60: 80452ac0 80365778 9f1efe8c 9f1efe78 9e93d400 9e93d5e8 9f1efeb4 9f72ef40
fe80: 9f1efeac 9f1efe90 8044e11c 80452998 8045298c 9e93d608 9e93d400 808e1978
fea0: 9f1efecc 9f1efeb0 8044fd14 8044e0d0 ffffffff 9f25a200 9e93d608 9e481380
fec0: 9f1efedc 9f1efed0 8044fde8 8044fcec 9f1eff1c 9f1efee0 80038d50 8044fdd8
fee0: 9f1ee020 9f72ef40 9e481398 00000000 00000008 9f72ef54 9f1ee020 9f72ef40
ff00: 9e481398 9e481380 00000008 9f72ef40 9f1eff5c 9f1eff20 80039754 80038bfc
ff20: 00000000 9e481380 80894100 808e1662 00000000 9e4f2ec0 00000000 9e481380
ff40: 800396f8 00000000 00000000 00000000 9f1effac 9f1eff60 8003e020 80039704
ff60: ffffffff 00000000 ffffffff 9e481380 00000000 00000000 9f1eff78 9f1eff78
ff80: 00000000 00000000 9f1eff88 9f1eff88 9e4f2ec0 8003df30 00000000 00000000
ffa0: 00000000 9f1effb0 8000eb60 8003df3c 00000000 00000000 00000000 00000000
ffc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
ffe0: 00000000 00000000 00000000 00000000 00000013 00000000 ffffffff ffffffff
Backtrace:
[<803636ac>] (ftrace_raw_event_regmap_block) from [<80365f2c>] (_regmap_raw_read+0x1bc/0x1cc)
r9:00000001 r8:808e3840 r7:00000180 r6:00000000 r5:9eb2f490 r4:9f1fc800
[<80365d70>] (_regmap_raw_read) from [<80365f70>] (_regmap_bus_read+0x34/0x6c)
r10:00000000 r9:00000000 r8:9f732400 r7:9f1efe6c r6:9f1efe6c r5:9f1fc800
r4:9f1fc800
[<80365f3c>] (_regmap_bus_read) from [<803656a4>] (_regmap_read+0x7c/0x144)
r6:00000180 r5:9f1fc800 r4:9f1fc800 r3:80365f3c
[<80365628>] (_regmap_read) from [<803657bc>] (regmap_read+0x50/0x70)
r9:00000000 r8:9f732400 r7:9f1efe6c r6:9f1efe6c r5:00000180 r4:9f1fc800
[<8036576c>] (regmap_read) from [<80452ac0>] (imx_get_temp+0x134/0x1a4)
r6:9f1efeb4 r5:9f1fc800 r4:9e95f910 r3:00000001
[<8045298c>] (imx_get_temp) from [<8044e11c>] (thermal_zone_get_temp+0x58/0x74)
r7:9f72ef40 r6:9f1efeb4 r5:9e93d5e8 r4:9e93d400
[<8044e0c4>] (thermal_zone_get_temp) from [<8044fd14>] (thermal_zone_device_update+0x34/0xec)
r6:808e1978 r5:9e93d400 r4:9e93d608 r3:8045298c
[<8044fce0>] (thermal_zone_device_update) from [<8044fde8>] (thermal_zone_device_check+0x1c/0x20)
r5:9e481380 r4:9e93d608
[<8044fdcc>] (thermal_zone_device_check) from [<80038d50>] (process_one_work+0x160/0x3d4)
[<80038bf0>] (process_one_work) from [<80039754>] (worker_thread+0x5c/0x4f4)
r10:9f72ef40 r9:00000008 r8:9e481380 r7:9e481398 r6:9f72ef40 r5:9f1ee020
r4:9f72ef54
[<800396f8>] (worker_thread) from [<8003e020>] (kthread+0xf0/0x108)
r10:00000000 r9:00000000 r8:00000000 r7:800396f8 r6:9e481380 r5:00000000
r4:9e4f2ec0
[<8003df30>] (kthread) from [<8000eb60>] (ret_from_fork+0x14/0x34)
r7:00000000 r6:00000000 r5:8003df30 r4:9e4f2ec0
Code: e3140040 1a00001a e3140020 1a000016 (e596002c)
---[ end trace 193c15c2494ec960 ]---
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
drivers/base/regmap/internal.h | 8 +++
drivers/base/regmap/regcache.c | 19 ++++---
drivers/base/regmap/regmap.c | 36 ++++++------
include/trace/events/regmap.h | 121 ++++++++++++++++++++---------------------
4 files changed, 94 insertions(+), 90 deletions(-)
diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index beb8b27..a13587b 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -243,4 +243,12 @@ extern struct regcache_ops regcache_rbtree_ops;
extern struct regcache_ops regcache_lzo_ops;
extern struct regcache_ops regcache_flat_ops;
+static inline const char *regmap_name(const struct regmap *map)
+{
+ if (map->dev)
+ return dev_name(map->dev);
+
+ return map->name;
+}
+
#endif
diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index f373c35..5c502c8 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -15,10 +15,11 @@
#include <linux/export.h>
#include <linux/slab.h>
#include <linux/sort.h>
-#include <trace/events/regmap.h>
#include "internal.h"
+#include <trace/events/regmap.h>
+
static const struct regcache_ops *cache_types[] = {
®cache_rbtree_ops,
®cache_lzo_ops,
@@ -218,7 +219,7 @@ int regcache_read(struct regmap *map,
ret = map->cache_ops->read(map, reg, value);
if (ret == 0)
- trace_regmap_reg_read_cache(map->dev, reg, *value);
+ trace_regmap_reg_read_cache(map, reg, *value);
return ret;
}
@@ -311,7 +312,7 @@ int regcache_sync(struct regmap *map)
dev_dbg(map->dev, "Syncing %s cache\n",
map->cache_ops->name);
name = map->cache_ops->name;
- trace_regcache_sync(map->dev, name, "start");
+ trace_regcache_sync(map, name, "start");
if (!map->cache_dirty)
goto out;
@@ -346,7 +347,7 @@ out:
regmap_async_complete(map);
- trace_regcache_sync(map->dev, name, "stop");
+ trace_regcache_sync(map, name, "stop");
return ret;
}
@@ -381,7 +382,7 @@ int regcache_sync_region(struct regmap *map, unsigned int min,
name = map->cache_ops->name;
dev_dbg(map->dev, "Syncing %s cache from %d-%d\n", name, min, max);
- trace_regcache_sync(map->dev, name, "start region");
+ trace_regcache_sync(map, name, "start region");
if (!map->cache_dirty)
goto out;
@@ -401,7 +402,7 @@ out:
regmap_async_complete(map);
- trace_regcache_sync(map->dev, name, "stop region");
+ trace_regcache_sync(map, name, "stop region");
return ret;
}
@@ -428,7 +429,7 @@ int regcache_drop_region(struct regmap *map, unsigned int min,
map->lock(map->lock_arg);
- trace_regcache_drop_region(map->dev, min, max);
+ trace_regcache_drop_region(map, min, max);
ret = map->cache_ops->drop(map, min, max);
@@ -455,7 +456,7 @@ void regcache_cache_only(struct regmap *map, bool enable)
map->lock(map->lock_arg);
WARN_ON(map->cache_bypass && enable);
map->cache_only = enable;
- trace_regmap_cache_only(map->dev, enable);
+ trace_regmap_cache_only(map, enable);
map->unlock(map->lock_arg);
}
EXPORT_SYMBOL_GPL(regcache_cache_only);
@@ -493,7 +494,7 @@ void regcache_cache_bypass(struct regmap *map, bool enable)
map->lock(map->lock_arg);
WARN_ON(map->cache_only && enable);
map->cache_bypass = enable;
- trace_regmap_cache_bypass(map->dev, enable);
+ trace_regmap_cache_bypass(map, enable);
map->unlock(map->lock_arg);
}
EXPORT_SYMBOL_GPL(regcache_cache_bypass);
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index f99b098..f2bc255 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -19,11 +19,11 @@
#include <linux/rbtree.h>
#include <linux/sched.h>
+#include "internal.h"
+
#define CREATE_TRACE_POINTS
#include <trace/events/regmap.h>
-#include "internal.h"
-
/*
* Sometimes for failures during very early init the trace
* infrastructure isn't available early enough to be used. For this
@@ -1281,7 +1281,7 @@ int _regmap_raw_write(struct regmap *map, unsigned int reg,
if (map->async && map->bus->async_write) {
struct regmap_async *async;
- trace_regmap_async_write_start(map->dev, reg, val_len);
+ trace_regmap_async_write_start(map, reg, val_len);
spin_lock_irqsave(&map->async_lock, flags);
async = list_first_entry_or_null(&map->async_free,
@@ -1339,8 +1339,7 @@ int _regmap_raw_write(struct regmap *map, unsigned int reg,
return ret;
}
- trace_regmap_hw_write_start(map->dev, reg,
- val_len / map->format.val_bytes);
+ trace_regmap_hw_write_start(map, reg, val_len / map->format.val_bytes);
/* If we're doing a single register write we can probably just
* send the work_buf directly, otherwise try to do a gather
@@ -1372,8 +1371,7 @@ int _regmap_raw_write(struct regmap *map, unsigned int reg,
kfree(buf);
}
- trace_regmap_hw_write_done(map->dev, reg,
- val_len / map->format.val_bytes);
+ trace_regmap_hw_write_done(map, reg, val_len / map->format.val_bytes);
return ret;
}
@@ -1407,12 +1405,12 @@ static int _regmap_bus_formatted_write(void *context, unsigned int reg,
map->format.format_write(map, reg, val);
- trace_regmap_hw_write_start(map->dev, reg, 1);
+ trace_regmap_hw_write_start(map, reg, 1);
ret = map->bus->write(map->bus_context, map->work_buf,
map->format.buf_size);
- trace_regmap_hw_write_done(map->dev, reg, 1);
+ trace_regmap_hw_write_done(map, reg, 1);
return ret;
}
@@ -1470,7 +1468,7 @@ int _regmap_write(struct regmap *map, unsigned int reg,
dev_info(map->dev, "%x <= %x\n", reg, val);
#endif
- trace_regmap_reg_write(map->dev, reg, val);
+ trace_regmap_reg_write(map, reg, val);
return map->reg_write(context, reg, val);
}
@@ -1773,7 +1771,7 @@ static int _regmap_raw_multi_reg_write(struct regmap *map,
for (i = 0; i < num_regs; i++) {
int reg = regs[i].reg;
int val = regs[i].def;
- trace_regmap_hw_write_start(map->dev, reg, 1);
+ trace_regmap_hw_write_start(map, reg, 1);
map->format.format_reg(u8, reg, map->reg_shift);
u8 += reg_bytes + pad_bytes;
map->format.format_val(u8, val, 0);
@@ -1788,7 +1786,7 @@ static int _regmap_raw_multi_reg_write(struct regmap *map,
for (i = 0; i < num_regs; i++) {
int reg = regs[i].reg;
- trace_regmap_hw_write_done(map->dev, reg, 1);
+ trace_regmap_hw_write_done(map, reg, 1);
}
return ret;
}
@@ -2059,15 +2057,13 @@ static int _regmap_raw_read(struct regmap *map, unsigned int reg, void *val,
*/
u8[0] |= map->read_flag_mask;
- trace_regmap_hw_read_start(map->dev, reg,
- val_len / map->format.val_bytes);
+ trace_regmap_hw_read_start(map, reg, val_len / map->format.val_bytes);
ret = map->bus->read(map->bus_context, map->work_buf,
map->format.reg_bytes + map->format.pad_bytes,
val, val_len);
- trace_regmap_hw_read_done(map->dev, reg,
- val_len / map->format.val_bytes);
+ trace_regmap_hw_read_done(map, reg, val_len / map->format.val_bytes);
return ret;
}
@@ -2123,7 +2119,7 @@ static int _regmap_read(struct regmap *map, unsigned int reg,
dev_info(map->dev, "%x => %x\n", reg, *val);
#endif
- trace_regmap_reg_read(map->dev, reg, *val);
+ trace_regmap_reg_read(map, reg, *val);
if (!map->cache_bypass)
regcache_write(map, reg, *val);
@@ -2480,7 +2476,7 @@ void regmap_async_complete_cb(struct regmap_async *async, int ret)
struct regmap *map = async->map;
bool wake;
- trace_regmap_async_io_complete(map->dev);
+ trace_regmap_async_io_complete(map);
spin_lock(&map->async_lock);
list_move(&async->list, &map->async_free);
@@ -2525,7 +2521,7 @@ int regmap_async_complete(struct regmap *map)
if (!map->bus || !map->bus->async_write)
return 0;
- trace_regmap_async_complete_start(map->dev);
+ trace_regmap_async_complete_start(map);
wait_event(map->async_waitq, regmap_async_is_done(map));
@@ -2534,7 +2530,7 @@ int regmap_async_complete(struct regmap *map)
map->async_ret = 0;
spin_unlock_irqrestore(&map->async_lock, flags);
- trace_regmap_async_complete_done(map->dev);
+ trace_regmap_async_complete_done(map);
return ret;
}
diff --git a/include/trace/events/regmap.h b/include/trace/events/regmap.h
index 23d5615..7cb3245 100644
--- a/include/trace/events/regmap.h
+++ b/include/trace/events/regmap.h
@@ -7,7 +7,6 @@
#include <linux/ktime.h>
#include <linux/tracepoint.h>
-struct device;
struct regmap;
/*
@@ -15,19 +14,19 @@ struct regmap;
*/
DECLARE_EVENT_CLASS(regmap_reg,
- TP_PROTO(struct device *dev, unsigned int reg,
+ TP_PROTO(struct regmap *map, unsigned int reg,
unsigned int val),
- TP_ARGS(dev, reg, val),
+ TP_ARGS(map, reg, val),
TP_STRUCT__entry(
- __string( name, dev_name(dev) )
- __field( unsigned int, reg )
- __field( unsigned int, val )
+ __string( name, regmap_name(map) )
+ __field( unsigned int, reg )
+ __field( unsigned int, val )
),
TP_fast_assign(
- __assign_str(name, dev_name(dev));
+ __assign_str(name, regmap_name(map));
__entry->reg = reg;
__entry->val = val;
),
@@ -39,45 +38,45 @@ DECLARE_EVENT_CLASS(regmap_reg,
DEFINE_EVENT(regmap_reg, regmap_reg_write,
- TP_PROTO(struct device *dev, unsigned int reg,
+ TP_PROTO(struct regmap *map, unsigned int reg,
unsigned int val),
- TP_ARGS(dev, reg, val)
+ TP_ARGS(map, reg, val)
);
DEFINE_EVENT(regmap_reg, regmap_reg_read,
- TP_PROTO(struct device *dev, unsigned int reg,
+ TP_PROTO(struct regmap *map, unsigned int reg,
unsigned int val),
- TP_ARGS(dev, reg, val)
+ TP_ARGS(map, reg, val)
);
DEFINE_EVENT(regmap_reg, regmap_reg_read_cache,
- TP_PROTO(struct device *dev, unsigned int reg,
+ TP_PROTO(struct regmap *map, unsigned int reg,
unsigned int val),
- TP_ARGS(dev, reg, val)
+ TP_ARGS(map, reg, val)
);
DECLARE_EVENT_CLASS(regmap_block,
- TP_PROTO(struct device *dev, unsigned int reg, int count),
+ TP_PROTO(struct regmap *map, unsigned int reg, int count),
- TP_ARGS(dev, reg, count),
+ TP_ARGS(map, reg, count),
TP_STRUCT__entry(
- __string( name, dev_name(dev) )
- __field( unsigned int, reg )
- __field( int, count )
+ __string( name, regmap_name(map) )
+ __field( unsigned int, reg )
+ __field( int, count )
),
TP_fast_assign(
- __assign_str(name, dev_name(dev));
+ __assign_str(name, regmap_name(map));
__entry->reg = reg;
__entry->count = count;
),
@@ -89,48 +88,48 @@ DECLARE_EVENT_CLASS(regmap_block,
DEFINE_EVENT(regmap_block, regmap_hw_read_start,
- TP_PROTO(struct device *dev, unsigned int reg, int count),
+ TP_PROTO(struct regmap *map, unsigned int reg, int count),
- TP_ARGS(dev, reg, count)
+ TP_ARGS(map, reg, count)
);
DEFINE_EVENT(regmap_block, regmap_hw_read_done,
- TP_PROTO(struct device *dev, unsigned int reg, int count),
+ TP_PROTO(struct regmap *map, unsigned int reg, int count),
- TP_ARGS(dev, reg, count)
+ TP_ARGS(map, reg, count)
);
DEFINE_EVENT(regmap_block, regmap_hw_write_start,
- TP_PROTO(struct device *dev, unsigned int reg, int count),
+ TP_PROTO(struct regmap *map, unsigned int reg, int count),
- TP_ARGS(dev, reg, count)
+ TP_ARGS(map, reg, count)
);
DEFINE_EVENT(regmap_block, regmap_hw_write_done,
- TP_PROTO(struct device *dev, unsigned int reg, int count),
+ TP_PROTO(struct regmap *map, unsigned int reg, int count),
- TP_ARGS(dev, reg, count)
+ TP_ARGS(map, reg, count)
);
TRACE_EVENT(regcache_sync,
- TP_PROTO(struct device *dev, const char *type,
+ TP_PROTO(struct regmap *map, const char *type,
const char *status),
- TP_ARGS(dev, type, status),
+ TP_ARGS(map, type, status),
TP_STRUCT__entry(
- __string( name, dev_name(dev) )
- __string( status, status )
- __string( type, type )
- __field( int, type )
+ __string( name, regmap_name(map) )
+ __string( status, status )
+ __string( type, type )
+ __field( int, type )
),
TP_fast_assign(
- __assign_str(name, dev_name(dev));
+ __assign_str(name, regmap_name(map));
__assign_str(status, status);
__assign_str(type, type);
),
@@ -141,17 +140,17 @@ TRACE_EVENT(regcache_sync,
DECLARE_EVENT_CLASS(regmap_bool,
- TP_PROTO(struct device *dev, bool flag),
+ TP_PROTO(struct regmap *map, bool flag),
- TP_ARGS(dev, flag),
+ TP_ARGS(map, flag),
TP_STRUCT__entry(
- __string( name, dev_name(dev) )
- __field( int, flag )
+ __string( name, regmap_name(map) )
+ __field( int, flag )
),
TP_fast_assign(
- __assign_str(name, dev_name(dev));
+ __assign_str(name, regmap_name(map));
__entry->flag = flag;
),
@@ -161,32 +160,32 @@ DECLARE_EVENT_CLASS(regmap_bool,
DEFINE_EVENT(regmap_bool, regmap_cache_only,
- TP_PROTO(struct device *dev, bool flag),
+ TP_PROTO(struct regmap *map, bool flag),
- TP_ARGS(dev, flag)
+ TP_ARGS(map, flag)
);
DEFINE_EVENT(regmap_bool, regmap_cache_bypass,
- TP_PROTO(struct device *dev, bool flag),
+ TP_PROTO(struct regmap *map, bool flag),
- TP_ARGS(dev, flag)
+ TP_ARGS(map, flag)
);
DECLARE_EVENT_CLASS(regmap_async,
- TP_PROTO(struct device *dev),
+ TP_PROTO(struct regmap *map),
- TP_ARGS(dev),
+ TP_ARGS(map),
TP_STRUCT__entry(
- __string( name, dev_name(dev) )
+ __string( name, regmap_name(map) )
),
TP_fast_assign(
- __assign_str(name, dev_name(dev));
+ __assign_str(name, regmap_name(map));
),
TP_printk("%s", __get_str(name))
@@ -194,50 +193,50 @@ DECLARE_EVENT_CLASS(regmap_async,
DEFINE_EVENT(regmap_block, regmap_async_write_start,
- TP_PROTO(struct device *dev, unsigned int reg, int count),
+ TP_PROTO(struct regmap *map, unsigned int reg, int count),
- TP_ARGS(dev, reg, count)
+ TP_ARGS(map, reg, count)
);
DEFINE_EVENT(regmap_async, regmap_async_io_complete,
- TP_PROTO(struct device *dev),
+ TP_PROTO(struct regmap *map),
- TP_ARGS(dev)
+ TP_ARGS(map)
);
DEFINE_EVENT(regmap_async, regmap_async_complete_start,
- TP_PROTO(struct device *dev),
+ TP_PROTO(struct regmap *map),
- TP_ARGS(dev)
+ TP_ARGS(map)
);
DEFINE_EVENT(regmap_async, regmap_async_complete_done,
- TP_PROTO(struct device *dev),
+ TP_PROTO(struct regmap *map),
- TP_ARGS(dev)
+ TP_ARGS(map)
);
TRACE_EVENT(regcache_drop_region,
- TP_PROTO(struct device *dev, unsigned int from,
+ TP_PROTO(struct regmap *map, unsigned int from,
unsigned int to),
- TP_ARGS(dev, from, to),
+ TP_ARGS(map, from, to),
TP_STRUCT__entry(
- __string( name, dev_name(dev) )
- __field( unsigned int, from )
- __field( unsigned int, to )
+ __string( name, regmap_name(map) )
+ __field( unsigned int, from )
+ __field( unsigned int, to )
),
TP_fast_assign(
- __assign_str(name, dev_name(dev));
+ __assign_str(name, regmap_name(map));
__entry->from = from;
__entry->to = to;
),
--
2.1.4
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] regmap: introduce regmap_name to fix syscon regmap trace events
2015-03-06 14:16 [PATCH] regmap: introduce regmap_name to fix syscon regmap trace events Philipp Zabel
@ 2015-03-07 10:57 ` Mark Brown
2015-03-09 9:48 ` Philipp Zabel
0 siblings, 1 reply; 7+ messages in thread
From: Mark Brown @ 2015-03-07 10:57 UTC (permalink / raw)
To: Philipp Zabel
Cc: Pankaj Dubey, Steven Rostedt, Ingo Molnar, linux-kernel, kernel
[-- Attachment #1: Type: text/plain, Size: 473 bytes --]
On Fri, Mar 06, 2015 at 03:16:55PM +0100, Philipp Zabel wrote:
> #include <linux/sort.h>
> -#include <trace/events/regmap.h>
>
> #include "internal.h"
>
> +#include <trace/events/regmap.h>
> +
The change is basically OK but the above is a bit too funky for my
taste - it feels like it's asking for annoying compile breakage if
anyone changes anything. I'd be more inclined to move the header
somewhere where the events stuff can see it than do this.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] regmap: introduce regmap_name to fix syscon regmap trace events
2015-03-07 10:57 ` Mark Brown
@ 2015-03-09 9:48 ` Philipp Zabel
2015-03-09 10:42 ` Mark Brown
2015-03-19 19:52 ` Steven Rostedt
0 siblings, 2 replies; 7+ messages in thread
From: Philipp Zabel @ 2015-03-09 9:48 UTC (permalink / raw)
To: Mark Brown
Cc: kernel, Pankaj Dubey, Ingo Molnar, linux-kernel, Steven Rostedt
Am Samstag, den 07.03.2015, 10:57 +0000 schrieb Mark Brown:
> On Fri, Mar 06, 2015 at 03:16:55PM +0100, Philipp Zabel wrote:
>
> > #include <linux/sort.h>
> > -#include <trace/events/regmap.h>
> >
> > #include "internal.h"
> >
> > +#include <trace/events/regmap.h>
> > +
>
> The change is basically OK but the above is a bit too funky for my
> taste - it feels like it's asking for annoying compile breakage if
> anyone changes anything. I'd be more inclined to move the header
> somewhere where the events stuff can see it than do this.
Good point. As I see it I have three possiblities now:
a) Just #include "../../../drivers/base/regmap/internal.h" in
include/trace/events/regmap.h
b) Move drivers/base/regmap/internal.h to
include/linux/regmap-internal.h and
#include <linux/regmap-internal.h> in include/trace/events/regmap.h
c) Move include/trace/events/regmap.h to drivers/base/regmap/trace.h
and just #include "internal.h" in drivers/base/regmap/trace.h
Which, if any, of these options would you prefer? Here is an
unscientific survey:
a) $ git grep "#include \"\.\." include/
include/linux/libfdt.h:#include "../../scripts/dtc/libfdt/fdt.h"
include/linux/libfdt.h:#include "../../scripts/dtc/libfdt/libfdt.h"
include/xen/interface/io/vscsiif.h:#include "../grant_table.h"
b) $ find include/ -name "*internal.h"
include/linux/timekeeper_internal.h
c) $ find drivers/ -name trace.h
drivers/net/wireless/ath/ath10k/trace.h
drivers/net/wireless/ath/trace.h
drivers/net/wireless/ath/wil6210/trace.h
drivers/net/wireless/ath/ath5k/trace.h
drivers/net/wireless/ath/ath6kl/trace.h
drivers/usb/dwc3/trace.h
drivers/misc/cxl/trace.h
a) and b) aren't very common, and c) is mostly used by driver-level, not
subsystem-level trace events.
regards
Philipp
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] regmap: introduce regmap_name to fix syscon regmap trace events
2015-03-09 9:48 ` Philipp Zabel
@ 2015-03-09 10:42 ` Mark Brown
2015-03-19 19:23 ` Steven Rostedt
2015-03-19 19:52 ` Steven Rostedt
1 sibling, 1 reply; 7+ messages in thread
From: Mark Brown @ 2015-03-09 10:42 UTC (permalink / raw)
To: Philipp Zabel
Cc: kernel, Pankaj Dubey, Ingo Molnar, linux-kernel, Steven Rostedt
[-- Attachment #1: Type: text/plain, Size: 396 bytes --]
On Mon, Mar 09, 2015 at 10:48:22AM +0100, Philipp Zabel wrote:
> Good point. As I see it I have three possiblities now:
> a) Just #include "../../../drivers/base/regmap/internal.h" in
> include/trace/events/regmap.h
I think this is my preference - it looks ugly which means that it's not
likely to get abused by anything trying to peer inside the internals
without setting off alarm bells.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] regmap: introduce regmap_name to fix syscon regmap trace events
2015-03-09 10:42 ` Mark Brown
@ 2015-03-19 19:23 ` Steven Rostedt
0 siblings, 0 replies; 7+ messages in thread
From: Steven Rostedt @ 2015-03-19 19:23 UTC (permalink / raw)
To: Mark Brown; +Cc: Philipp Zabel, kernel, Pankaj Dubey, Ingo Molnar, linux-kernel
On Mon, 9 Mar 2015 10:42:40 +0000
Mark Brown <broonie@kernel.org> wrote:
> On Mon, Mar 09, 2015 at 10:48:22AM +0100, Philipp Zabel wrote:
>
> > Good point. As I see it I have three possiblities now:
>
> > a) Just #include "../../../drivers/base/regmap/internal.h" in
> > include/trace/events/regmap.h
>
> I think this is my preference - it looks ugly which means that it's not
> likely to get abused by anything trying to peer inside the internals
> without setting off alarm bells.
Grumble, reading my inbox backwards seems to make me reply to things
that has already been discussed :-p
-- Steve
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] regmap: introduce regmap_name to fix syscon regmap trace events
2015-03-09 9:48 ` Philipp Zabel
2015-03-09 10:42 ` Mark Brown
@ 2015-03-19 19:52 ` Steven Rostedt
2015-03-19 19:59 ` Mark Brown
1 sibling, 1 reply; 7+ messages in thread
From: Steven Rostedt @ 2015-03-19 19:52 UTC (permalink / raw)
To: Philipp Zabel; +Cc: Mark Brown, kernel, Pankaj Dubey, Ingo Molnar, linux-kernel
/me decides to continue the "reply backwards" method.
On Mon, 09 Mar 2015 10:48:22 +0100
Philipp Zabel <p.zabel@pengutronix.de> wrote:
> Am Samstag, den 07.03.2015, 10:57 +0000 schrieb Mark Brown:
> > On Fri, Mar 06, 2015 at 03:16:55PM +0100, Philipp Zabel wrote:
> >
> > > #include <linux/sort.h>
> > > -#include <trace/events/regmap.h>
> > >
> > > #include "internal.h"
> > >
> > > +#include <trace/events/regmap.h>
> > > +
> >
> > The change is basically OK but the above is a bit too funky for my
> > taste - it feels like it's asking for annoying compile breakage if
> > anyone changes anything. I'd be more inclined to move the header
> > somewhere where the events stuff can see it than do this.
>
> Good point. As I see it I have three possiblities now:
>
> a) Just #include "../../../drivers/base/regmap/internal.h" in
> include/trace/events/regmap.h
>
> b) Move drivers/base/regmap/internal.h to
> include/linux/regmap-internal.h and
> #include <linux/regmap-internal.h> in include/trace/events/regmap.h
>
> c) Move include/trace/events/regmap.h to drivers/base/regmap/trace.h
> and just #include "internal.h" in drivers/base/regmap/trace.h
>
> Which, if any, of these options would you prefer? Here is an
> unscientific survey:
>
> a) $ git grep "#include \"\.\." include/
> include/linux/libfdt.h:#include "../../scripts/dtc/libfdt/fdt.h"
> include/linux/libfdt.h:#include "../../scripts/dtc/libfdt/libfdt.h"
> include/xen/interface/io/vscsiif.h:#include "../grant_table.h"
>
> b) $ find include/ -name "*internal.h"
> include/linux/timekeeper_internal.h
>
> c) $ find drivers/ -name trace.h
> drivers/net/wireless/ath/ath10k/trace.h
> drivers/net/wireless/ath/trace.h
> drivers/net/wireless/ath/wil6210/trace.h
> drivers/net/wireless/ath/ath5k/trace.h
> drivers/net/wireless/ath/ath6kl/trace.h
> drivers/usb/dwc3/trace.h
> drivers/misc/cxl/trace.h
>
> a) and b) aren't very common, and c) is mostly used by driver-level, not
> subsystem-level trace events.
I'm not sure what you mean be driver-level vs subsystem-level. How is
this a subsystem level as regmap happens to be in the drivers code.
Also, if you want a more thorough list, try this:
$ find drivers/ -name "*.h" ! -type d | xargs grep -l "TRACE_SYSTEM"
drivers/misc/cxl/trace.h
drivers/usb/host/xhci-trace.h
drivers/usb/dwc3/trace.h
drivers/android/binder_trace.h
drivers/gpu/drm/drm_trace.h
drivers/gpu/drm/i915/i915_trace.h
drivers/gpu/drm/radeon/radeon_trace.h
drivers/staging/android/trace/sync.h
drivers/mtd/devices/docg3.h
drivers/net/wireless/brcm80211/brcmsmac/brcms_trace_events.h
drivers/net/wireless/brcm80211/brcmfmac/tracepoint.h
drivers/net/wireless/iwlwifi/iwl-devtrace.h
drivers/net/wireless/ath/trace.h
drivers/net/wireless/ath/ath6kl/trace.h
drivers/net/wireless/ath/ath5k/trace.h
drivers/net/wireless/ath/wil6210/trace.h
drivers/net/wireless/ath/ath10k/trace.h
And looking outside of drivers and trace:
$ find . -path ./include/trace -prune -o -name drivers -prune -o -name '*.h' -print | xargs grep -l TRACE_SYSTEM
./sound/core/pcm_trace.h
./sound/pci/hda/hda_intel_trace.h
./sound/pci/hda/hda_trace.h
./kernel/trace/trace_benchmark.h
./kernel/trace/trace_events_filter_test.h
./arch/arm64/kernel/trace-events-emulation.h
./arch/arm64/kvm/trace.h
./arch/x86/kernel/vsyscall_trace.h
./arch/x86/include/asm/trace/exceptions.h
./arch/x86/include/asm/trace/irq_vectors.h
./arch/x86/kvm/mmutrace.h
./arch/x86/kvm/trace.h
./arch/s390/kvm/trace.h
./arch/s390/kvm/trace-s390.h
./arch/powerpc/platforms/cell/spufs/sputrace.h
./arch/powerpc/include/asm/trace.h
./arch/powerpc/kvm/trace.h
./arch/powerpc/kvm/trace_hv.h
./arch/powerpc/kvm/trace_pr.h
./arch/powerpc/kvm/trace_booke.h
./arch/mips/kvm/trace.h
./arch/arm/kvm/trace.h
./include/ras/ras_event.h
./net/mac80211/trace.h
./net/wireless/trace.h
./samples/trace_events/trace-events-sample.h
./fs/ocfs2/ocfs2_trace.h
./fs/nfs/nfstrace.h
./fs/nfs/nfs4trace.h
./fs/nfsd/trace.h
./fs/gfs2/trace_gfs2.h
./fs/xfs/xfs_trace.h
-- Steve
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] regmap: introduce regmap_name to fix syscon regmap trace events
2015-03-19 19:52 ` Steven Rostedt
@ 2015-03-19 19:59 ` Mark Brown
0 siblings, 0 replies; 7+ messages in thread
From: Mark Brown @ 2015-03-19 19:59 UTC (permalink / raw)
To: Steven Rostedt
Cc: Philipp Zabel, kernel, Pankaj Dubey, Ingo Molnar, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 569 bytes --]
On Thu, Mar 19, 2015 at 03:52:28PM -0400, Steven Rostedt wrote:
> I'm not sure what you mean be driver-level vs subsystem-level. How is
> this a subsystem level as regmap happens to be in the drivers code.
Lots of generic, subsystem level code lives in drivers/ - most
directories in there have a bunch of drivers together with the
subsystem code that supports them. In the case of regmap the
percentage of generic code is very high as it's mostly there as
a helper subsystem hiding register map access details from drivers
that sit in other subsystems.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-03-19 19:59 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-06 14:16 [PATCH] regmap: introduce regmap_name to fix syscon regmap trace events Philipp Zabel
2015-03-07 10:57 ` Mark Brown
2015-03-09 9:48 ` Philipp Zabel
2015-03-09 10:42 ` Mark Brown
2015-03-19 19:23 ` Steven Rostedt
2015-03-19 19:52 ` Steven Rostedt
2015-03-19 19:59 ` Mark Brown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®