* [PATCH 1/8] counter: 104-quad-8: Use container_of instead of struct counter_device::priv
2021-12-21 10:45 [PATCH 0/8] counter: Remove struct counter_device::priv Uwe Kleine-König
@ 2021-12-21 10:45 ` Uwe Kleine-König
2021-12-21 10:45 ` [PATCH 2/8] counter: ftm-quaddec: " Uwe Kleine-König
` (7 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Uwe Kleine-König @ 2021-12-21 10:45 UTC (permalink / raw)
To: William Breathitt Gray, Syed Nayyar Waris; +Cc: linux-iio, linux-kernel
Using counter->priv is a memory read and so more expensive than
container_of which is only an addition.
So container_of is expected to be a tad faster, it's type-safe, and
produces smaller code (ARCH=arm allmodconfig):
add/remove: 0/0 grow/shrink: 5/17 up/down: 76/-172 (-96)
Function old new delta
quad8_function_write 612 648 +36
quad8_count_mode_write 296 312 +16
quad8_count_enable_write 232 248 +16
quad8_events_configure 384 388 +4
quad8_count_preset_enable_write 264 268 +4
quad8_signal_fck_prescaler_read 108 104 -4
quad8_count_enable_read 100 96 -4
quad8_synchronous_mode_set 304 296 -8
quad8_signal_cable_fault_read 240 232 -8
quad8_signal_cable_fault_enable_write 244 236 -8
quad8_index_polarity_set 200 192 -8
quad8_function_read 292 284 -8
quad8_count_read 304 296 -8
quad8_count_preset_write 160 152 -8
quad8_count_ceiling_read 216 208 -8
quad8_signal_read 212 200 -12
quad8_signal_cable_fault_enable_read 108 96 -12
quad8_probe 1116 1104 -12
quad8_error_noise_get 156 144 -12
quad8_direction_read 156 144 -12
quad8_action_read 836 824 -12
quad8_watch_validate 528 500 -28
Total: Before=11802, After=11706, chg -0.81%
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/counter/104-quad-8.c | 64 +++++++++++++++++++-----------------
1 file changed, 34 insertions(+), 30 deletions(-)
diff --git a/drivers/counter/104-quad-8.c b/drivers/counter/104-quad-8.c
index 1cbd60aaed69..a9e75c70ad30 100644
--- a/drivers/counter/104-quad-8.c
+++ b/drivers/counter/104-quad-8.c
@@ -109,11 +109,16 @@ struct quad8 {
#define QUAD8_CMR_QUADRATURE_X2 0x10
#define QUAD8_CMR_QUADRATURE_X4 0x18
+static struct quad8 *quad8_from_counter(struct counter_device *counter)
+{
+ return container_of(counter, struct quad8, counter);
+}
+
static int quad8_signal_read(struct counter_device *counter,
struct counter_signal *signal,
enum counter_signal_level *level)
{
- const struct quad8 *const priv = counter->priv;
+ const struct quad8 *const priv = quad8_from_counter(counter);
unsigned int state;
/* Only Index signal levels can be read */
@@ -131,7 +136,7 @@ static int quad8_signal_read(struct counter_device *counter,
static int quad8_count_read(struct counter_device *counter,
struct counter_count *count, u64 *val)
{
- struct quad8 *const priv = counter->priv;
+ struct quad8 *const priv = quad8_from_counter(counter);
const int base_offset = priv->base + 2 * count->id;
unsigned int flags;
unsigned int borrow;
@@ -163,7 +168,7 @@ static int quad8_count_read(struct counter_device *counter,
static int quad8_count_write(struct counter_device *counter,
struct counter_count *count, u64 val)
{
- struct quad8 *const priv = counter->priv;
+ struct quad8 *const priv = quad8_from_counter(counter);
const int base_offset = priv->base + 2 * count->id;
unsigned long irqflags;
int i;
@@ -213,7 +218,7 @@ static int quad8_function_read(struct counter_device *counter,
struct counter_count *count,
enum counter_function *function)
{
- struct quad8 *const priv = counter->priv;
+ struct quad8 *const priv = quad8_from_counter(counter);
const int id = count->id;
unsigned long irqflags;
@@ -243,7 +248,7 @@ static int quad8_function_write(struct counter_device *counter,
struct counter_count *count,
enum counter_function function)
{
- struct quad8 *const priv = counter->priv;
+ struct quad8 *const priv = quad8_from_counter(counter);
const int id = count->id;
unsigned int *const quadrature_mode = priv->quadrature_mode + id;
unsigned int *const scale = priv->quadrature_scale + id;
@@ -305,7 +310,7 @@ static int quad8_direction_read(struct counter_device *counter,
struct counter_count *count,
enum counter_count_direction *direction)
{
- const struct quad8 *const priv = counter->priv;
+ const struct quad8 *const priv = quad8_from_counter(counter);
unsigned int ud_flag;
const unsigned int flag_addr = priv->base + 2 * count->id + 1;
@@ -335,7 +340,7 @@ static int quad8_action_read(struct counter_device *counter,
struct counter_synapse *synapse,
enum counter_synapse_action *action)
{
- struct quad8 *const priv = counter->priv;
+ struct quad8 *const priv = quad8_from_counter(counter);
int err;
enum counter_function function;
const size_t signal_a_id = count->synapses[0].signal->id;
@@ -399,7 +404,7 @@ enum {
static int quad8_events_configure(struct counter_device *counter)
{
- struct quad8 *const priv = counter->priv;
+ struct quad8 *const priv = quad8_from_counter(counter);
unsigned long irq_enabled = 0;
unsigned long irqflags;
size_t channel;
@@ -442,7 +447,7 @@ static int quad8_events_configure(struct counter_device *counter)
static int quad8_watch_validate(struct counter_device *counter,
const struct counter_watch *watch)
{
- struct quad8 *const priv = counter->priv;
+ struct quad8 *const priv = quad8_from_counter(counter);
if (watch->channel > QUAD8_NUM_COUNTERS - 1)
return -EINVAL;
@@ -497,7 +502,7 @@ static int quad8_index_polarity_get(struct counter_device *counter,
struct counter_signal *signal,
u32 *index_polarity)
{
- const struct quad8 *const priv = counter->priv;
+ const struct quad8 *const priv = quad8_from_counter(counter);
const size_t channel_id = signal->id - 16;
*index_polarity = priv->index_polarity[channel_id];
@@ -509,7 +514,7 @@ static int quad8_index_polarity_set(struct counter_device *counter,
struct counter_signal *signal,
u32 index_polarity)
{
- struct quad8 *const priv = counter->priv;
+ struct quad8 *const priv = quad8_from_counter(counter);
const size_t channel_id = signal->id - 16;
const int base_offset = priv->base + 2 * channel_id + 1;
unsigned long irqflags;
@@ -538,7 +543,7 @@ static int quad8_synchronous_mode_get(struct counter_device *counter,
struct counter_signal *signal,
u32 *synchronous_mode)
{
- const struct quad8 *const priv = counter->priv;
+ const struct quad8 *const priv = quad8_from_counter(counter);
const size_t channel_id = signal->id - 16;
*synchronous_mode = priv->synchronous_mode[channel_id];
@@ -550,7 +555,7 @@ static int quad8_synchronous_mode_set(struct counter_device *counter,
struct counter_signal *signal,
u32 synchronous_mode)
{
- struct quad8 *const priv = counter->priv;
+ struct quad8 *const priv = quad8_from_counter(counter);
const size_t channel_id = signal->id - 16;
const int base_offset = priv->base + 2 * channel_id + 1;
unsigned long irqflags;
@@ -589,7 +594,7 @@ static int quad8_count_mode_read(struct counter_device *counter,
struct counter_count *count,
enum counter_count_mode *cnt_mode)
{
- const struct quad8 *const priv = counter->priv;
+ const struct quad8 *const priv = quad8_from_counter(counter);
/* Map 104-QUAD-8 count mode to Generic Counter count mode */
switch (priv->count_mode[count->id]) {
@@ -614,7 +619,7 @@ static int quad8_count_mode_write(struct counter_device *counter,
struct counter_count *count,
enum counter_count_mode cnt_mode)
{
- struct quad8 *const priv = counter->priv;
+ struct quad8 *const priv = quad8_from_counter(counter);
unsigned int count_mode;
unsigned int mode_cfg;
const int base_offset = priv->base + 2 * count->id + 1;
@@ -661,7 +666,7 @@ static int quad8_count_mode_write(struct counter_device *counter,
static int quad8_count_enable_read(struct counter_device *counter,
struct counter_count *count, u8 *enable)
{
- const struct quad8 *const priv = counter->priv;
+ const struct quad8 *const priv = quad8_from_counter(counter);
*enable = priv->ab_enable[count->id];
@@ -671,7 +676,7 @@ static int quad8_count_enable_read(struct counter_device *counter,
static int quad8_count_enable_write(struct counter_device *counter,
struct counter_count *count, u8 enable)
{
- struct quad8 *const priv = counter->priv;
+ struct quad8 *const priv = quad8_from_counter(counter);
const int base_offset = priv->base + 2 * count->id;
unsigned long irqflags;
unsigned int ior_cfg;
@@ -699,7 +704,7 @@ static const char *const quad8_noise_error_states[] = {
static int quad8_error_noise_get(struct counter_device *counter,
struct counter_count *count, u32 *noise_error)
{
- const struct quad8 *const priv = counter->priv;
+ const struct quad8 *const priv = quad8_from_counter(counter);
const int base_offset = priv->base + 2 * count->id + 1;
*noise_error = !!(inb(base_offset) & QUAD8_FLAG_E);
@@ -710,7 +715,7 @@ static int quad8_error_noise_get(struct counter_device *counter,
static int quad8_count_preset_read(struct counter_device *counter,
struct counter_count *count, u64 *preset)
{
- const struct quad8 *const priv = counter->priv;
+ const struct quad8 *const priv = quad8_from_counter(counter);
*preset = priv->preset[count->id];
@@ -736,7 +741,7 @@ static void quad8_preset_register_set(struct quad8 *const priv, const int id,
static int quad8_count_preset_write(struct counter_device *counter,
struct counter_count *count, u64 preset)
{
- struct quad8 *const priv = counter->priv;
+ struct quad8 *const priv = quad8_from_counter(counter);
unsigned long irqflags;
/* Only 24-bit values are supported */
@@ -755,7 +760,7 @@ static int quad8_count_preset_write(struct counter_device *counter,
static int quad8_count_ceiling_read(struct counter_device *counter,
struct counter_count *count, u64 *ceiling)
{
- struct quad8 *const priv = counter->priv;
+ struct quad8 *const priv = quad8_from_counter(counter);
unsigned long irqflags;
spin_lock_irqsave(&priv->lock, irqflags);
@@ -780,7 +785,7 @@ static int quad8_count_ceiling_read(struct counter_device *counter,
static int quad8_count_ceiling_write(struct counter_device *counter,
struct counter_count *count, u64 ceiling)
{
- struct quad8 *const priv = counter->priv;
+ struct quad8 *const priv = quad8_from_counter(counter);
unsigned long irqflags;
/* Only 24-bit values are supported */
@@ -807,7 +812,7 @@ static int quad8_count_preset_enable_read(struct counter_device *counter,
struct counter_count *count,
u8 *preset_enable)
{
- const struct quad8 *const priv = counter->priv;
+ const struct quad8 *const priv = quad8_from_counter(counter);
*preset_enable = !priv->preset_enable[count->id];
@@ -818,7 +823,7 @@ static int quad8_count_preset_enable_write(struct counter_device *counter,
struct counter_count *count,
u8 preset_enable)
{
- struct quad8 *const priv = counter->priv;
+ struct quad8 *const priv = quad8_from_counter(counter);
const int base_offset = priv->base + 2 * count->id + 1;
unsigned long irqflags;
unsigned int ior_cfg;
@@ -845,7 +850,7 @@ static int quad8_signal_cable_fault_read(struct counter_device *counter,
struct counter_signal *signal,
u8 *cable_fault)
{
- struct quad8 *const priv = counter->priv;
+ struct quad8 *const priv = quad8_from_counter(counter);
const size_t channel_id = signal->id / 2;
unsigned long irqflags;
bool disabled;
@@ -875,7 +880,7 @@ static int quad8_signal_cable_fault_enable_read(struct counter_device *counter,
struct counter_signal *signal,
u8 *enable)
{
- const struct quad8 *const priv = counter->priv;
+ const struct quad8 *const priv = quad8_from_counter(counter);
const size_t channel_id = signal->id / 2;
*enable = !!(priv->cable_fault_enable & BIT(channel_id));
@@ -887,7 +892,7 @@ static int quad8_signal_cable_fault_enable_write(struct counter_device *counter,
struct counter_signal *signal,
u8 enable)
{
- struct quad8 *const priv = counter->priv;
+ struct quad8 *const priv = quad8_from_counter(counter);
const size_t channel_id = signal->id / 2;
unsigned long irqflags;
unsigned int cable_fault_enable;
@@ -913,7 +918,7 @@ static int quad8_signal_fck_prescaler_read(struct counter_device *counter,
struct counter_signal *signal,
u8 *prescaler)
{
- const struct quad8 *const priv = counter->priv;
+ const struct quad8 *const priv = quad8_from_counter(counter);
*prescaler = priv->fck_prescaler[signal->id / 2];
@@ -924,7 +929,7 @@ static int quad8_signal_fck_prescaler_write(struct counter_device *counter,
struct counter_signal *signal,
u8 prescaler)
{
- struct quad8 *const priv = counter->priv;
+ struct quad8 *const priv = quad8_from_counter(counter);
const size_t channel_id = signal->id / 2;
const int base_offset = priv->base + 2 * channel_id;
unsigned long irqflags;
@@ -1150,7 +1155,6 @@ static int quad8_probe(struct device *dev, unsigned int id)
priv->counter.num_counts = ARRAY_SIZE(quad8_counts);
priv->counter.signals = quad8_signals;
priv->counter.num_signals = ARRAY_SIZE(quad8_signals);
- priv->counter.priv = priv;
priv->base = base[id];
spin_lock_init(&priv->lock);
--
2.33.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 2/8] counter: ftm-quaddec: Use container_of instead of struct counter_device::priv
2021-12-21 10:45 [PATCH 0/8] counter: Remove struct counter_device::priv Uwe Kleine-König
2021-12-21 10:45 ` [PATCH 1/8] counter: 104-quad-8: Use container_of instead of " Uwe Kleine-König
@ 2021-12-21 10:45 ` Uwe Kleine-König
2021-12-21 10:45 ` [PATCH 3/8] counter: intel-qeb: " Uwe Kleine-König
` (6 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Uwe Kleine-König @ 2021-12-21 10:45 UTC (permalink / raw)
To: Patrick Havelange, William Breathitt Gray; +Cc: linux-iio, linux-kernel
Using counter->priv is a memory read and so more expensive than
container_of which is only an addition. (In this case even a noop
because the offset is 0.)
So container_of is expected to be a tad faster, it's type-safe, and
produces smaller code (ARCH=arm allmodconfig):
add/remove: 0/0 grow/shrink: 0/5 up/down: 0/-60 (-60)
Function old new delta
ftm_quaddec_set_prescaler 612 600 -12
ftm_quaddec_probe 596 584 -12
ftm_quaddec_get_prescaler 156 144 -12
ftm_quaddec_count_write 232 220 -12
ftm_quaddec_count_read 152 140 -12
Total: Before=5096, After=5036, chg -1.18%
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/counter/ftm-quaddec.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/counter/ftm-quaddec.c b/drivers/counter/ftm-quaddec.c
index 5ef0478709cd..b58a4f54e97b 100644
--- a/drivers/counter/ftm-quaddec.c
+++ b/drivers/counter/ftm-quaddec.c
@@ -33,6 +33,11 @@ struct ftm_quaddec {
struct mutex ftm_quaddec_mutex;
};
+static inline struct ftm_quaddec *ftm_from_counter(struct counter_device *counter)
+{
+ return container_of(counter, struct ftm_quaddec, counter);
+}
+
static void ftm_read(struct ftm_quaddec *ftm, uint32_t offset, uint32_t *data)
{
if (ftm->big_endian)
@@ -118,7 +123,7 @@ static void ftm_quaddec_disable(void *ftm)
static int ftm_quaddec_get_prescaler(struct counter_device *counter,
struct counter_count *count, u32 *cnt_mode)
{
- struct ftm_quaddec *ftm = counter->priv;
+ struct ftm_quaddec *ftm = ftm_from_counter(counter);
uint32_t scflags;
ftm_read(ftm, FTM_SC, &scflags);
@@ -131,7 +136,7 @@ static int ftm_quaddec_get_prescaler(struct counter_device *counter,
static int ftm_quaddec_set_prescaler(struct counter_device *counter,
struct counter_count *count, u32 cnt_mode)
{
- struct ftm_quaddec *ftm = counter->priv;
+ struct ftm_quaddec *ftm = ftm_from_counter(counter);
mutex_lock(&ftm->ftm_quaddec_mutex);
@@ -162,7 +167,7 @@ static int ftm_quaddec_count_read(struct counter_device *counter,
struct counter_count *count,
u64 *val)
{
- struct ftm_quaddec *const ftm = counter->priv;
+ struct ftm_quaddec *const ftm = ftm_from_counter(counter);
uint32_t cntval;
ftm_read(ftm, FTM_CNT, &cntval);
@@ -176,7 +181,7 @@ static int ftm_quaddec_count_write(struct counter_device *counter,
struct counter_count *count,
const u64 val)
{
- struct ftm_quaddec *const ftm = counter->priv;
+ struct ftm_quaddec *const ftm = ftm_from_counter(counter);
if (val != 0) {
dev_warn(&ftm->pdev->dev, "Can only accept '0' as new counter value\n");
@@ -292,7 +297,6 @@ static int ftm_quaddec_probe(struct platform_device *pdev)
ftm->counter.num_counts = 1;
ftm->counter.signals = ftm_quaddec_signals;
ftm->counter.num_signals = ARRAY_SIZE(ftm_quaddec_signals);
- ftm->counter.priv = ftm;
mutex_init(&ftm->ftm_quaddec_mutex);
--
2.33.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 3/8] counter: intel-qeb: Use container_of instead of struct counter_device::priv
2021-12-21 10:45 [PATCH 0/8] counter: Remove struct counter_device::priv Uwe Kleine-König
2021-12-21 10:45 ` [PATCH 1/8] counter: 104-quad-8: Use container_of instead of " Uwe Kleine-König
2021-12-21 10:45 ` [PATCH 2/8] counter: ftm-quaddec: " Uwe Kleine-König
@ 2021-12-21 10:45 ` Uwe Kleine-König
2021-12-21 10:45 ` [PATCH 4/8] counter: interrupt-cnt: " Uwe Kleine-König
` (5 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Uwe Kleine-König @ 2021-12-21 10:45 UTC (permalink / raw)
To: Jarkko Nikula, William Breathitt Gray; +Cc: linux-iio, linux-kernel
Using counter->priv is a memory read and so more expensive than
container_of which is only an addition. (In this case even a noop
because the offset is 0.)
So container_of is expected to be a tad faster, it's type-safe, and
produces smaller code (ARCH=arm allmodconfig):
add/remove: 0/0 grow/shrink: 0/10 up/down: 0/-116 (-116)
Function old new delta
intel_qep_spike_filter_ns_write 552 544 -8
intel_qep_spike_filter_ns_read 252 240 -12
intel_qep_probe 692 680 -12
intel_qep_preset_enable_write 276 264 -12
intel_qep_preset_enable_read 164 152 -12
intel_qep_enable_write 500 488 -12
intel_qep_enable_read 80 68 -12
intel_qep_count_read 140 128 -12
intel_qep_ceiling_write 260 248 -12
intel_qep_ceiling_read 140 128 -12
Total: Before=4867, After=4751, chg -2.38%
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/counter/intel-qep.c | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/counter/intel-qep.c b/drivers/counter/intel-qep.c
index 0924d16de6e2..168f8f5357cf 100644
--- a/drivers/counter/intel-qep.c
+++ b/drivers/counter/intel-qep.c
@@ -74,6 +74,11 @@ struct intel_qep {
u32 qepmax;
};
+static inline struct intel_qep *intel_qep_from_counter(struct counter_device *counter)
+{
+ return container_of(counter, struct intel_qep, counter);
+}
+
static inline u32 intel_qep_readl(struct intel_qep *qep, u32 offset)
{
return readl(qep->regs + offset);
@@ -109,7 +114,7 @@ static void intel_qep_init(struct intel_qep *qep)
static int intel_qep_count_read(struct counter_device *counter,
struct counter_count *count, u64 *val)
{
- struct intel_qep *const qep = counter->priv;
+ struct intel_qep *const qep = intel_qep_from_counter(counter);
pm_runtime_get_sync(qep->dev);
*val = intel_qep_readl(qep, INTEL_QEPCOUNT);
@@ -176,7 +181,7 @@ static struct counter_synapse intel_qep_count_synapses[] = {
static int intel_qep_ceiling_read(struct counter_device *counter,
struct counter_count *count, u64 *ceiling)
{
- struct intel_qep *qep = counter->priv;
+ struct intel_qep *qep = intel_qep_from_counter(counter);
pm_runtime_get_sync(qep->dev);
*ceiling = intel_qep_readl(qep, INTEL_QEPMAX);
@@ -188,7 +193,7 @@ static int intel_qep_ceiling_read(struct counter_device *counter,
static int intel_qep_ceiling_write(struct counter_device *counter,
struct counter_count *count, u64 max)
{
- struct intel_qep *qep = counter->priv;
+ struct intel_qep *qep = intel_qep_from_counter(counter);
int ret = 0;
/* Intel QEP ceiling configuration only supports 32-bit values */
@@ -213,7 +218,7 @@ static int intel_qep_ceiling_write(struct counter_device *counter,
static int intel_qep_enable_read(struct counter_device *counter,
struct counter_count *count, u8 *enable)
{
- struct intel_qep *qep = counter->priv;
+ struct intel_qep *qep = intel_qep_from_counter(counter);
*enable = qep->enabled;
@@ -223,7 +228,7 @@ static int intel_qep_enable_read(struct counter_device *counter,
static int intel_qep_enable_write(struct counter_device *counter,
struct counter_count *count, u8 val)
{
- struct intel_qep *qep = counter->priv;
+ struct intel_qep *qep = intel_qep_from_counter(counter);
u32 reg;
bool changed;
@@ -256,7 +261,7 @@ static int intel_qep_spike_filter_ns_read(struct counter_device *counter,
struct counter_count *count,
u64 *length)
{
- struct intel_qep *qep = counter->priv;
+ struct intel_qep *qep = intel_qep_from_counter(counter);
u32 reg;
pm_runtime_get_sync(qep->dev);
@@ -277,7 +282,7 @@ static int intel_qep_spike_filter_ns_write(struct counter_device *counter,
struct counter_count *count,
u64 length)
{
- struct intel_qep *qep = counter->priv;
+ struct intel_qep *qep = intel_qep_from_counter(counter);
u32 reg;
bool enable;
int ret = 0;
@@ -326,7 +331,7 @@ static int intel_qep_preset_enable_read(struct counter_device *counter,
struct counter_count *count,
u8 *preset_enable)
{
- struct intel_qep *qep = counter->priv;
+ struct intel_qep *qep = intel_qep_from_counter(counter);
u32 reg;
pm_runtime_get_sync(qep->dev);
@@ -341,7 +346,7 @@ static int intel_qep_preset_enable_read(struct counter_device *counter,
static int intel_qep_preset_enable_write(struct counter_device *counter,
struct counter_count *count, u8 val)
{
- struct intel_qep *qep = counter->priv;
+ struct intel_qep *qep = intel_qep_from_counter(counter);
u32 reg;
int ret = 0;
@@ -429,7 +434,6 @@ static int intel_qep_probe(struct pci_dev *pci, const struct pci_device_id *id)
qep->counter.num_counts = ARRAY_SIZE(intel_qep_counter_count);
qep->counter.signals = intel_qep_signals;
qep->counter.num_signals = ARRAY_SIZE(intel_qep_signals);
- qep->counter.priv = qep;
qep->enabled = false;
pm_runtime_put(dev);
--
2.33.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 4/8] counter: interrupt-cnt: Use container_of instead of struct counter_device::priv
2021-12-21 10:45 [PATCH 0/8] counter: Remove struct counter_device::priv Uwe Kleine-König
` (2 preceding siblings ...)
2021-12-21 10:45 ` [PATCH 3/8] counter: intel-qeb: " Uwe Kleine-König
@ 2021-12-21 10:45 ` Uwe Kleine-König
2021-12-21 10:45 ` [PATCH 5/8] counter: microchip-tcp-capture: " Uwe Kleine-König
` (4 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Uwe Kleine-König @ 2021-12-21 10:45 UTC (permalink / raw)
To: Oleksij Rempel, William Breathitt Gray; +Cc: linux-iio, linux-kernel
Using counter->priv is a memory read and so more expensive than
container_of which is only an addition.
So container_of is expected to be a tad faster, it's type-safe, and
produces smaller code (ARCH=arm allmodconfig):
add/remove: 0/0 grow/shrink: 0/6 up/down: 0/-60 (-60)
Function old new delta
interrupt_cnt_write 136 128 -8
interrupt_cnt_read 92 84 -8
interrupt_cnt_enable_write 180 172 -8
interrupt_cnt_enable_read 76 68 -8
interrupt_cnt_probe 1048 1036 -12
interrupt_cnt_signal_read 164 148 -16
Total: Before=2841, After=2781, chg -2.11%
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/counter/interrupt-cnt.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/counter/interrupt-cnt.c b/drivers/counter/interrupt-cnt.c
index 8514a87fcbee..c61628aa5c32 100644
--- a/drivers/counter/interrupt-cnt.c
+++ b/drivers/counter/interrupt-cnt.c
@@ -25,6 +25,11 @@ struct interrupt_cnt_priv {
struct counter_count cnts;
};
+static inline struct interrupt_cnt_priv *interrupt_cnt_from_counter(struct counter_device *counter)
+{
+ return container_of(counter, struct interrupt_cnt_priv, counter);
+}
+
static irqreturn_t interrupt_cnt_isr(int irq, void *dev_id)
{
struct interrupt_cnt_priv *priv = dev_id;
@@ -37,7 +42,7 @@ static irqreturn_t interrupt_cnt_isr(int irq, void *dev_id)
static int interrupt_cnt_enable_read(struct counter_device *counter,
struct counter_count *count, u8 *enable)
{
- struct interrupt_cnt_priv *priv = counter->priv;
+ struct interrupt_cnt_priv *priv = interrupt_cnt_from_counter(counter);
*enable = priv->enabled;
@@ -47,7 +52,7 @@ static int interrupt_cnt_enable_read(struct counter_device *counter,
static int interrupt_cnt_enable_write(struct counter_device *counter,
struct counter_count *count, u8 enable)
{
- struct interrupt_cnt_priv *priv = counter->priv;
+ struct interrupt_cnt_priv *priv = interrupt_cnt_from_counter(counter);
if (priv->enabled == enable)
return 0;
@@ -85,7 +90,7 @@ static int interrupt_cnt_action_read(struct counter_device *counter,
static int interrupt_cnt_read(struct counter_device *counter,
struct counter_count *count, u64 *val)
{
- struct interrupt_cnt_priv *priv = counter->priv;
+ struct interrupt_cnt_priv *priv = interrupt_cnt_from_counter(counter);
*val = atomic_read(&priv->count);
@@ -95,7 +100,7 @@ static int interrupt_cnt_read(struct counter_device *counter,
static int interrupt_cnt_write(struct counter_device *counter,
struct counter_count *count, const u64 val)
{
- struct interrupt_cnt_priv *priv = counter->priv;
+ struct interrupt_cnt_priv *priv = interrupt_cnt_from_counter(counter);
if (val != (typeof(priv->count.counter))val)
return -ERANGE;
@@ -122,7 +127,7 @@ static int interrupt_cnt_signal_read(struct counter_device *counter,
struct counter_signal *signal,
enum counter_signal_level *level)
{
- struct interrupt_cnt_priv *priv = counter->priv;
+ struct interrupt_cnt_priv *priv = interrupt_cnt_from_counter(counter);
int ret;
if (!priv->gpio)
@@ -199,7 +204,6 @@ static int interrupt_cnt_probe(struct platform_device *pdev)
priv->cnts.ext = interrupt_cnt_ext;
priv->cnts.num_ext = ARRAY_SIZE(interrupt_cnt_ext);
- priv->counter.priv = priv;
priv->counter.name = dev_name(dev);
priv->counter.parent = dev;
priv->counter.ops = &interrupt_cnt_ops;
--
2.33.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 5/8] counter: microchip-tcp-capture: Use container_of instead of struct counter_device::priv
2021-12-21 10:45 [PATCH 0/8] counter: Remove struct counter_device::priv Uwe Kleine-König
` (3 preceding siblings ...)
2021-12-21 10:45 ` [PATCH 4/8] counter: interrupt-cnt: " Uwe Kleine-König
@ 2021-12-21 10:45 ` Uwe Kleine-König
2021-12-21 10:45 ` [PATCH 6/8] counter: stm32-lptimer-cnt: " Uwe Kleine-König
` (3 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Uwe Kleine-König @ 2021-12-21 10:45 UTC (permalink / raw)
To: Kamel Bouhara, William Breathitt Gray
Cc: linux-arm-kernel, linux-iio, linux-kernel
Using counter->priv is a memory read and so more expensive than
container_of which is only an addition.
So container_of is expected to be a tad faster, it's type-safe, and
produces smaller code (ARCH=arm allmodconfig):
add/remove: 0/0 grow/shrink: 1/6 up/down: 12/-68 (-56)
Function old new delta
mchp_tc_count_function_write 1016 1028 +12
mchp_tc_count_action_write 204 196 -8
mchp_tc_probe 1376 1364 -12
mchp_tc_count_signal_read 360 348 -12
mchp_tc_count_read 264 252 -12
mchp_tc_count_function_read 108 96 -12
mchp_tc_count_action_read 392 380 -12
Total: Before=5920, After=5864, chg -0.95%
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/counter/microchip-tcb-capture.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/counter/microchip-tcb-capture.c b/drivers/counter/microchip-tcb-capture.c
index 0ab1b2716784..031e79f5f06a 100644
--- a/drivers/counter/microchip-tcb-capture.c
+++ b/drivers/counter/microchip-tcb-capture.c
@@ -32,6 +32,11 @@ struct mchp_tc_data {
bool trig_inverted;
};
+static inline struct mchp_tc_data *mchp_tc_from_counter(struct counter_device *counter)
+{
+ return container_of(counter, struct mchp_tc_data, counter);
+}
+
static const enum counter_function mchp_tc_count_functions[] = {
COUNTER_FUNCTION_INCREASE,
COUNTER_FUNCTION_QUADRATURE_X4,
@@ -72,7 +77,7 @@ static int mchp_tc_count_function_read(struct counter_device *counter,
struct counter_count *count,
enum counter_function *function)
{
- struct mchp_tc_data *const priv = counter->priv;
+ struct mchp_tc_data *const priv = mchp_tc_from_counter(counter);
if (priv->qdec_mode)
*function = COUNTER_FUNCTION_QUADRATURE_X4;
@@ -86,7 +91,7 @@ static int mchp_tc_count_function_write(struct counter_device *counter,
struct counter_count *count,
enum counter_function function)
{
- struct mchp_tc_data *const priv = counter->priv;
+ struct mchp_tc_data *const priv = mchp_tc_from_counter(counter);
u32 bmr, cmr;
regmap_read(priv->regmap, ATMEL_TC_BMR, &bmr);
@@ -148,7 +153,7 @@ static int mchp_tc_count_signal_read(struct counter_device *counter,
struct counter_signal *signal,
enum counter_signal_level *lvl)
{
- struct mchp_tc_data *const priv = counter->priv;
+ struct mchp_tc_data *const priv = mchp_tc_from_counter(counter);
bool sigstatus;
u32 sr;
@@ -169,7 +174,7 @@ static int mchp_tc_count_action_read(struct counter_device *counter,
struct counter_synapse *synapse,
enum counter_synapse_action *action)
{
- struct mchp_tc_data *const priv = counter->priv;
+ struct mchp_tc_data *const priv = mchp_tc_from_counter(counter);
u32 cmr;
regmap_read(priv->regmap, ATMEL_TC_REG(priv->channel[0], CMR), &cmr);
@@ -197,7 +202,7 @@ static int mchp_tc_count_action_write(struct counter_device *counter,
struct counter_synapse *synapse,
enum counter_synapse_action action)
{
- struct mchp_tc_data *const priv = counter->priv;
+ struct mchp_tc_data *const priv = mchp_tc_from_counter(counter);
u32 edge = ATMEL_TC_ETRGEDG_NONE;
/* QDEC mode is rising edge only */
@@ -230,7 +235,7 @@ static int mchp_tc_count_action_write(struct counter_device *counter,
static int mchp_tc_count_read(struct counter_device *counter,
struct counter_count *count, u64 *val)
{
- struct mchp_tc_data *const priv = counter->priv;
+ struct mchp_tc_data *const priv = mchp_tc_from_counter(counter);
u32 cnt;
regmap_read(priv->regmap, ATMEL_TC_REG(priv->channel[0], CV), &cnt);
@@ -369,7 +374,6 @@ static int mchp_tc_probe(struct platform_device *pdev)
priv->counter.counts = mchp_tc_counts;
priv->counter.num_signals = ARRAY_SIZE(mchp_tc_count_signals);
priv->counter.signals = mchp_tc_count_signals;
- priv->counter.priv = priv;
return devm_counter_register(&pdev->dev, &priv->counter);
}
--
2.33.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 6/8] counter: stm32-lptimer-cnt: Use container_of instead of struct counter_device::priv
2021-12-21 10:45 [PATCH 0/8] counter: Remove struct counter_device::priv Uwe Kleine-König
` (4 preceding siblings ...)
2021-12-21 10:45 ` [PATCH 5/8] counter: microchip-tcp-capture: " Uwe Kleine-König
@ 2021-12-21 10:45 ` Uwe Kleine-König
2021-12-21 10:45 ` [PATCH 7/8] counter: stm32-timer-cnt: " Uwe Kleine-König
` (2 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Uwe Kleine-König @ 2021-12-21 10:45 UTC (permalink / raw)
To: Fabrice Gasnier, William Breathitt Gray, Maxime Coquelin,
Alexandre Torgue
Cc: linux-iio, linux-stm32, linux-arm-kernel, linux-kernel
Using counter->priv is a memory read and so more expensive than
container_of which is only an addition. (In this case even a noop
because the offset is 0.)
So container_of is expected to be a tad faster, it's type-safe, and
produces smaller code (ARCH=arm allmodconfig):
add/remove: 0/0 grow/shrink: 0/10 up/down: 0/-140 (-140)
Function old new delta
stm32_lptim_cnt_read 272 260 -12
stm32_lptim_cnt_probe 528 516 -12
stm32_lptim_cnt_function_write 420 408 -12
stm32_lptim_cnt_function_read 184 172 -12
stm32_lptim_cnt_enable_write 436 424 -12
stm32_lptim_cnt_enable_read 312 300 -12
stm32_lptim_cnt_ceiling_write 368 356 -12
stm32_lptim_cnt_ceiling_read 84 72 -12
stm32_lptim_cnt_action_read 388 376 -12
stm32_lptim_cnt_action_write 576 544 -32
Total: Before=6458, After=6318, chg -2.17%
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/counter/stm32-lptimer-cnt.c | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/counter/stm32-lptimer-cnt.c b/drivers/counter/stm32-lptimer-cnt.c
index 5168833b1fdf..c6eb3071571f 100644
--- a/drivers/counter/stm32-lptimer-cnt.c
+++ b/drivers/counter/stm32-lptimer-cnt.c
@@ -30,6 +30,11 @@ struct stm32_lptim_cnt {
bool enabled;
};
+static inline struct stm32_lptim_cnt *stm32_lptim_from_counter(struct counter_device *counter)
+{
+ return container_of(counter, struct stm32_lptim_cnt, counter);
+}
+
static int stm32_lptim_is_enabled(struct stm32_lptim_cnt *priv)
{
u32 val;
@@ -141,7 +146,7 @@ static const enum counter_synapse_action stm32_lptim_cnt_synapse_actions[] = {
static int stm32_lptim_cnt_read(struct counter_device *counter,
struct counter_count *count, u64 *val)
{
- struct stm32_lptim_cnt *const priv = counter->priv;
+ struct stm32_lptim_cnt *const priv = stm32_lptim_from_counter(counter);
u32 cnt;
int ret;
@@ -158,7 +163,7 @@ static int stm32_lptim_cnt_function_read(struct counter_device *counter,
struct counter_count *count,
enum counter_function *function)
{
- struct stm32_lptim_cnt *const priv = counter->priv;
+ struct stm32_lptim_cnt *const priv = stm32_lptim_from_counter(counter);
if (!priv->quadrature_mode) {
*function = COUNTER_FUNCTION_INCREASE;
@@ -177,7 +182,7 @@ static int stm32_lptim_cnt_function_write(struct counter_device *counter,
struct counter_count *count,
enum counter_function function)
{
- struct stm32_lptim_cnt *const priv = counter->priv;
+ struct stm32_lptim_cnt *const priv = stm32_lptim_from_counter(counter);
if (stm32_lptim_is_enabled(priv))
return -EBUSY;
@@ -200,7 +205,7 @@ static int stm32_lptim_cnt_enable_read(struct counter_device *counter,
struct counter_count *count,
u8 *enable)
{
- struct stm32_lptim_cnt *const priv = counter->priv;
+ struct stm32_lptim_cnt *const priv = stm32_lptim_from_counter(counter);
int ret;
ret = stm32_lptim_is_enabled(priv);
@@ -216,7 +221,7 @@ static int stm32_lptim_cnt_enable_write(struct counter_device *counter,
struct counter_count *count,
u8 enable)
{
- struct stm32_lptim_cnt *const priv = counter->priv;
+ struct stm32_lptim_cnt *const priv = stm32_lptim_from_counter(counter);
int ret;
/* Check nobody uses the timer, or already disabled/enabled */
@@ -241,7 +246,7 @@ static int stm32_lptim_cnt_ceiling_read(struct counter_device *counter,
struct counter_count *count,
u64 *ceiling)
{
- struct stm32_lptim_cnt *const priv = counter->priv;
+ struct stm32_lptim_cnt *const priv = stm32_lptim_from_counter(counter);
*ceiling = priv->ceiling;
@@ -252,7 +257,7 @@ static int stm32_lptim_cnt_ceiling_write(struct counter_device *counter,
struct counter_count *count,
u64 ceiling)
{
- struct stm32_lptim_cnt *const priv = counter->priv;
+ struct stm32_lptim_cnt *const priv = stm32_lptim_from_counter(counter);
if (stm32_lptim_is_enabled(priv))
return -EBUSY;
@@ -277,7 +282,7 @@ static int stm32_lptim_cnt_action_read(struct counter_device *counter,
struct counter_synapse *synapse,
enum counter_synapse_action *action)
{
- struct stm32_lptim_cnt *const priv = counter->priv;
+ struct stm32_lptim_cnt *const priv = stm32_lptim_from_counter(counter);
enum counter_function function;
int err;
@@ -321,7 +326,7 @@ static int stm32_lptim_cnt_action_write(struct counter_device *counter,
struct counter_synapse *synapse,
enum counter_synapse_action action)
{
- struct stm32_lptim_cnt *const priv = counter->priv;
+ struct stm32_lptim_cnt *const priv = stm32_lptim_from_counter(counter);
enum counter_function function;
int err;
@@ -438,7 +443,6 @@ static int stm32_lptim_cnt_probe(struct platform_device *pdev)
}
priv->counter.num_counts = 1;
priv->counter.signals = stm32_lptim_cnt_signals;
- priv->counter.priv = priv;
platform_set_drvdata(pdev, priv);
--
2.33.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 7/8] counter: stm32-timer-cnt: Use container_of instead of struct counter_device::priv
2021-12-21 10:45 [PATCH 0/8] counter: Remove struct counter_device::priv Uwe Kleine-König
` (5 preceding siblings ...)
2021-12-21 10:45 ` [PATCH 6/8] counter: stm32-lptimer-cnt: " Uwe Kleine-König
@ 2021-12-21 10:45 ` Uwe Kleine-König
2021-12-21 10:45 ` [PATCH 8/8] counter: Remove unused member from struct counter_device Uwe Kleine-König
2021-12-21 11:12 ` [PATCH 0/8] counter: Remove struct counter_device::priv Lars-Peter Clausen
8 siblings, 0 replies; 15+ messages in thread
From: Uwe Kleine-König @ 2021-12-21 10:45 UTC (permalink / raw)
To: Fabrice Gasnier, William Breathitt Gray, Maxime Coquelin,
Alexandre Torgue
Cc: linux-iio, linux-stm32, linux-arm-kernel, linux-kernel
Using counter->priv is a memory read and so more expensive than
container_of which is only an addition. (In this case even a noop
because the offset is 0.)
So container_of is expected to be a tad faster, it's type-safe, and
produces smaller code (ARCH=arm allmodconfig):
add/remove: 0/0 grow/shrink: 0/11 up/down: 0/-132 (-132)
Function old new delta
stm32_timer_cnt_probe 436 424 -12
stm32_count_write 312 300 -12
stm32_count_read 236 224 -12
stm32_count_function_write 492 480 -12
stm32_count_function_read 396 384 -12
stm32_count_enable_write 488 476 -12
stm32_count_enable_read 236 224 -12
stm32_count_direction_read 240 228 -12
stm32_count_ceiling_write 200 188 -12
stm32_count_ceiling_read 236 224 -12
stm32_action_read 492 480 -12
Total: Before=5504, After=5372, chg -2.40%
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/counter/stm32-timer-cnt.c | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/counter/stm32-timer-cnt.c b/drivers/counter/stm32-timer-cnt.c
index 0546e932db0c..ac0bea6ce690 100644
--- a/drivers/counter/stm32-timer-cnt.c
+++ b/drivers/counter/stm32-timer-cnt.c
@@ -37,6 +37,11 @@ struct stm32_timer_cnt {
struct stm32_timer_regs bak;
};
+static inline struct stm32_timer_cnt *stm32_count_from_counter(struct counter_device *counter)
+{
+ return container_of(counter, struct stm32_timer_cnt, counter);
+}
+
static const enum counter_function stm32_count_functions[] = {
COUNTER_FUNCTION_INCREASE,
COUNTER_FUNCTION_QUADRATURE_X2_A,
@@ -47,7 +52,7 @@ static const enum counter_function stm32_count_functions[] = {
static int stm32_count_read(struct counter_device *counter,
struct counter_count *count, u64 *val)
{
- struct stm32_timer_cnt *const priv = counter->priv;
+ struct stm32_timer_cnt *const priv = stm32_count_from_counter(counter);
u32 cnt;
regmap_read(priv->regmap, TIM_CNT, &cnt);
@@ -59,7 +64,7 @@ static int stm32_count_read(struct counter_device *counter,
static int stm32_count_write(struct counter_device *counter,
struct counter_count *count, const u64 val)
{
- struct stm32_timer_cnt *const priv = counter->priv;
+ struct stm32_timer_cnt *const priv = stm32_count_from_counter(counter);
u32 ceiling;
regmap_read(priv->regmap, TIM_ARR, &ceiling);
@@ -73,7 +78,7 @@ static int stm32_count_function_read(struct counter_device *counter,
struct counter_count *count,
enum counter_function *function)
{
- struct stm32_timer_cnt *const priv = counter->priv;
+ struct stm32_timer_cnt *const priv = stm32_count_from_counter(counter);
u32 smcr;
regmap_read(priv->regmap, TIM_SMCR, &smcr);
@@ -100,7 +105,7 @@ static int stm32_count_function_write(struct counter_device *counter,
struct counter_count *count,
enum counter_function function)
{
- struct stm32_timer_cnt *const priv = counter->priv;
+ struct stm32_timer_cnt *const priv = stm32_count_from_counter(counter);
u32 cr1, sms;
switch (function) {
@@ -140,7 +145,7 @@ static int stm32_count_direction_read(struct counter_device *counter,
struct counter_count *count,
enum counter_count_direction *direction)
{
- struct stm32_timer_cnt *const priv = counter->priv;
+ struct stm32_timer_cnt *const priv = stm32_count_from_counter(counter);
u32 cr1;
regmap_read(priv->regmap, TIM_CR1, &cr1);
@@ -153,7 +158,7 @@ static int stm32_count_direction_read(struct counter_device *counter,
static int stm32_count_ceiling_read(struct counter_device *counter,
struct counter_count *count, u64 *ceiling)
{
- struct stm32_timer_cnt *const priv = counter->priv;
+ struct stm32_timer_cnt *const priv = stm32_count_from_counter(counter);
u32 arr;
regmap_read(priv->regmap, TIM_ARR, &arr);
@@ -166,7 +171,7 @@ static int stm32_count_ceiling_read(struct counter_device *counter,
static int stm32_count_ceiling_write(struct counter_device *counter,
struct counter_count *count, u64 ceiling)
{
- struct stm32_timer_cnt *const priv = counter->priv;
+ struct stm32_timer_cnt *const priv = stm32_count_from_counter(counter);
if (ceiling > priv->max_arr)
return -ERANGE;
@@ -181,7 +186,7 @@ static int stm32_count_ceiling_write(struct counter_device *counter,
static int stm32_count_enable_read(struct counter_device *counter,
struct counter_count *count, u8 *enable)
{
- struct stm32_timer_cnt *const priv = counter->priv;
+ struct stm32_timer_cnt *const priv = stm32_count_from_counter(counter);
u32 cr1;
regmap_read(priv->regmap, TIM_CR1, &cr1);
@@ -194,7 +199,7 @@ static int stm32_count_enable_read(struct counter_device *counter,
static int stm32_count_enable_write(struct counter_device *counter,
struct counter_count *count, u8 enable)
{
- struct stm32_timer_cnt *const priv = counter->priv;
+ struct stm32_timer_cnt *const priv = stm32_count_from_counter(counter);
u32 cr1;
if (enable) {
@@ -336,7 +341,6 @@ static int stm32_timer_cnt_probe(struct platform_device *pdev)
priv->counter.num_counts = 1;
priv->counter.signals = stm32_signals;
priv->counter.num_signals = ARRAY_SIZE(stm32_signals);
- priv->counter.priv = priv;
platform_set_drvdata(pdev, priv);
--
2.33.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 8/8] counter: Remove unused member from struct counter_device
2021-12-21 10:45 [PATCH 0/8] counter: Remove struct counter_device::priv Uwe Kleine-König
` (6 preceding siblings ...)
2021-12-21 10:45 ` [PATCH 7/8] counter: stm32-timer-cnt: " Uwe Kleine-König
@ 2021-12-21 10:45 ` Uwe Kleine-König
2021-12-21 11:12 ` [PATCH 0/8] counter: Remove struct counter_device::priv Lars-Peter Clausen
8 siblings, 0 replies; 15+ messages in thread
From: Uwe Kleine-König @ 2021-12-21 10:45 UTC (permalink / raw)
To: David Lechner, William Breathitt Gray; +Cc: linux-iio, linux-kernel
The functionality priv was designed for can better be accomplished using
container_of. All drivers have been converted, so drop this now unused
member.
Just one assignment was missed in the conversion of the ti-eqep driver.
This is unused and so can be safely dropped, too.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/counter/ti-eqep.c | 1 -
include/linux/counter.h | 3 ---
2 files changed, 4 deletions(-)
diff --git a/drivers/counter/ti-eqep.c b/drivers/counter/ti-eqep.c
index 9e0e46bca4c2..8cdc9ab98859 100644
--- a/drivers/counter/ti-eqep.c
+++ b/drivers/counter/ti-eqep.c
@@ -402,7 +402,6 @@ static int ti_eqep_probe(struct platform_device *pdev)
priv->counter.num_counts = ARRAY_SIZE(ti_eqep_counts);
priv->counter.signals = ti_eqep_signals;
priv->counter.num_signals = ARRAY_SIZE(ti_eqep_signals);
- priv->counter.priv = priv;
platform_set_drvdata(pdev, priv);
diff --git a/include/linux/counter.h b/include/linux/counter.h
index b7d0a00a61cf..fd58f36ea2f7 100644
--- a/include/linux/counter.h
+++ b/include/linux/counter.h
@@ -287,7 +287,6 @@ struct counter_ops {
* @num_counts: number of Counts specified in @counts
* @ext: optional array of Counter device extensions
* @num_ext: number of Counter device extensions specified in @ext
- * @priv: optional private data supplied by driver
* @dev: internal device structure
* @chrdev: internal character device structure
* @events_list: list of current watching Counter events
@@ -314,8 +313,6 @@ struct counter_device {
struct counter_comp *ext;
size_t num_ext;
- void *priv;
-
struct device dev;
struct cdev chrdev;
struct list_head events_list;
--
2.33.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 0/8] counter: Remove struct counter_device::priv
2021-12-21 10:45 [PATCH 0/8] counter: Remove struct counter_device::priv Uwe Kleine-König
` (7 preceding siblings ...)
2021-12-21 10:45 ` [PATCH 8/8] counter: Remove unused member from struct counter_device Uwe Kleine-König
@ 2021-12-21 11:12 ` Lars-Peter Clausen
2021-12-21 11:35 ` Uwe Kleine-König
8 siblings, 1 reply; 15+ messages in thread
From: Lars-Peter Clausen @ 2021-12-21 11:12 UTC (permalink / raw)
To: Uwe Kleine-König, Alexandre Torgue, David Lechner,
Fabrice Gasnier, Jarkko Nikula, Kamel Bouhara, Maxime Coquelin,
Oleksij Rempel, Patrick Havelange, Syed Nayyar Waris,
William Breathitt Gray
Cc: linux-arm-kernel, linux-iio, linux-kernel, linux-stm32
On 12/21/21 11:45 AM, Uwe Kleine-König wrote:
> Hello,
>
> similar to patch
> https://lore.kernel.org/r/4bde7cbd9e43a5909208102094444219d3154466.1640072891.git.vilhelm.gray@gmail.com
> the usage of struct counter_device::priv can be replaced by
> container_of which improves type safety and code size.
>
> This series depends on above patch, converts the remaining drivers and
> finally drops struct counter_device::priv.
Not sure if this is such a good idea. struct counter_device should not
be embedded in the drivers state struct in the first place.
struct counter_device contains a struct device, which is a reference
counted object. But by embedding it in the driver state struct the life
time of both the struct counter_device and and struct device are bound
to the life time of the driver state struct.
Which means the struct device memory can get freed before the last
reference is dropped, which leads to a use-after-free and undefined
behavior.
The framework should be changed to rather then embedding the struct
counter_device in the state struct to just have a pointer to it. With
the struct counter_device having its own allocation that will be freed
when the last reference to the struct device is dropped.
- Lars
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 0/8] counter: Remove struct counter_device::priv
2021-12-21 11:12 ` [PATCH 0/8] counter: Remove struct counter_device::priv Lars-Peter Clausen
@ 2021-12-21 11:35 ` Uwe Kleine-König
2021-12-21 12:04 ` Lars-Peter Clausen
0 siblings, 1 reply; 15+ messages in thread
From: Uwe Kleine-König @ 2021-12-21 11:35 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Alexandre Torgue, David Lechner, Fabrice Gasnier, Jarkko Nikula,
Kamel Bouhara, Maxime Coquelin, Oleksij Rempel,
Patrick Havelange, Syed Nayyar Waris, William Breathitt Gray,
linux-arm-kernel, linux-iio, linux-kernel, linux-stm32
[-- Attachment #1: Type: text/plain, Size: 2012 bytes --]
Hello Lars,
On Tue, Dec 21, 2021 at 12:12:12PM +0100, Lars-Peter Clausen wrote:
> On 12/21/21 11:45 AM, Uwe Kleine-König wrote:
> > similar to patch
> > https://lore.kernel.org/r/4bde7cbd9e43a5909208102094444219d3154466.1640072891.git.vilhelm.gray@gmail.com
> > the usage of struct counter_device::priv can be replaced by
> > container_of which improves type safety and code size.
> >
> > This series depends on above patch, converts the remaining drivers and
> > finally drops struct counter_device::priv.
>
> Not sure if this is such a good idea. struct counter_device should not be
> embedded in the drivers state struct in the first place.
Just to mention it: My patch series didn't change this, this was already
broken before.
> struct counter_device contains a struct device, which is a reference counted
> object. But by embedding it in the driver state struct the life time of both
> the struct counter_device and and struct device are bound to the life time
> of the driver state struct.
>
> Which means the struct device memory can get freed before the last reference
> is dropped, which leads to a use-after-free and undefined behavior.
Well, the driver struct is allocated using devm_kzalloc for all drivers.
So I think it's not *very* urgent to fix. Still you're right, this
should be addressed.
> The framework should be changed to rather then embedding the struct
> counter_device in the state struct to just have a pointer to it. With the
> struct counter_device having its own allocation that will be freed when the
> last reference to the struct device is dropped.
My favourite would be to implement a counter_device_alloc /
counter_device_add approach, similar to what spi_alloc_controller and
alloc_etherdev do. The downside is that this isn't typesafe either :-\
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/8] counter: Remove struct counter_device::priv
2021-12-21 11:35 ` Uwe Kleine-König
@ 2021-12-21 12:04 ` Lars-Peter Clausen
2021-12-21 13:22 ` Uwe Kleine-König
2021-12-22 17:51 ` Uwe Kleine-König
0 siblings, 2 replies; 15+ messages in thread
From: Lars-Peter Clausen @ 2021-12-21 12:04 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Alexandre Torgue, David Lechner, Fabrice Gasnier, Jarkko Nikula,
Kamel Bouhara, Maxime Coquelin, Oleksij Rempel,
Patrick Havelange, Syed Nayyar Waris, William Breathitt Gray,
linux-arm-kernel, linux-iio, linux-kernel, linux-stm32
On 12/21/21 12:35 PM, Uwe Kleine-König wrote:
> Hello Lars,
>
> On Tue, Dec 21, 2021 at 12:12:12PM +0100, Lars-Peter Clausen wrote:
>> On 12/21/21 11:45 AM, Uwe Kleine-König wrote:
>>> similar to patch
>>> https://lore.kernel.org/r/4bde7cbd9e43a5909208102094444219d3154466.1640072891.git.vilhelm.gray@gmail.com
>>> the usage of struct counter_device::priv can be replaced by
>>> container_of which improves type safety and code size.
>>>
>>> This series depends on above patch, converts the remaining drivers and
>>> finally drops struct counter_device::priv.
>> Not sure if this is such a good idea. struct counter_device should not be
>> embedded in the drivers state struct in the first place.
> Just to mention it: My patch series didn't change this, this was already
> broken before.
I know, but this series has to be reverted when the framework is fixed.
>
>> struct counter_device contains a struct device, which is a reference counted
>> object. But by embedding it in the driver state struct the life time of both
>> the struct counter_device and and struct device are bound to the life time
>> of the driver state struct.
>>
>> Which means the struct device memory can get freed before the last reference
>> is dropped, which leads to a use-after-free and undefined behavior.
> Well, the driver struct is allocated using devm_kzalloc for all drivers.
devm_kzalloc() doesn't make a difference. The managed memory is freed
when the parent device is unbound/removed. There may very well be
reference to the counter_device at this point.
> So I think it's not *very* urgent to fix. Still you're right, this
> should be addressed.
Yes and no, this can trivially be used for privilege escalation, but
then again on systems with a counter_device probably everything runs as
root anyway.
>
>> The framework should be changed to rather then embedding the struct
>> counter_device in the state struct to just have a pointer to it. With the
>> struct counter_device having its own allocation that will be freed when the
>> last reference to the struct device is dropped.
> My favourite would be to implement a counter_device_alloc /
> counter_device_add approach, similar to what spi_alloc_controller and
> alloc_etherdev do. The downside is that this isn't typesafe either :-\
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/8] counter: Remove struct counter_device::priv
2021-12-21 12:04 ` Lars-Peter Clausen
@ 2021-12-21 13:22 ` Uwe Kleine-König
2021-12-21 14:12 ` Lars-Peter Clausen
2021-12-22 17:51 ` Uwe Kleine-König
1 sibling, 1 reply; 15+ messages in thread
From: Uwe Kleine-König @ 2021-12-21 13:22 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Alexandre Torgue, David Lechner, Fabrice Gasnier, Jarkko Nikula,
Kamel Bouhara, Maxime Coquelin, Oleksij Rempel,
Patrick Havelange, Syed Nayyar Waris, William Breathitt Gray,
linux-arm-kernel, linux-iio, linux-kernel, linux-stm32
[-- Attachment #1: Type: text/plain, Size: 1333 bytes --]
On Tue, Dec 21, 2021 at 01:04:50PM +0100, Lars-Peter Clausen wrote:
> On 12/21/21 12:35 PM, Uwe Kleine-König wrote:
> > On Tue, Dec 21, 2021 at 12:12:12PM +0100, Lars-Peter Clausen wrote:
> > > On 12/21/21 11:45 AM, Uwe Kleine-König wrote:
> > > > similar to patch
> > > > https://lore.kernel.org/r/4bde7cbd9e43a5909208102094444219d3154466.1640072891.git.vilhelm.gray@gmail.com
> > > > the usage of struct counter_device::priv can be replaced by
> > > > container_of which improves type safety and code size.
> > > >
> > > > This series depends on above patch, converts the remaining drivers and
> > > > finally drops struct counter_device::priv.
> > > Not sure if this is such a good idea. struct counter_device should not be
> > > embedded in the drivers state struct in the first place.
> > Just to mention it: My patch series didn't change this, this was already
> > broken before.
>
> I know, but this series has to be reverted when the framework is fixed.
All drivers have to be touched. With my patch series you have to modify
one function in each driver, without my patch you have touch nearly
every function.
*shrug*
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/8] counter: Remove struct counter_device::priv
2021-12-21 13:22 ` Uwe Kleine-König
@ 2021-12-21 14:12 ` Lars-Peter Clausen
0 siblings, 0 replies; 15+ messages in thread
From: Lars-Peter Clausen @ 2021-12-21 14:12 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Alexandre Torgue, David Lechner, Fabrice Gasnier, Jarkko Nikula,
Kamel Bouhara, Maxime Coquelin, Oleksij Rempel,
Patrick Havelange, Syed Nayyar Waris, William Breathitt Gray,
linux-arm-kernel, linux-iio, linux-kernel, linux-stm32
On 12/21/21 2:22 PM, Uwe Kleine-König wrote:
> On Tue, Dec 21, 2021 at 01:04:50PM +0100, Lars-Peter Clausen wrote:
>> On 12/21/21 12:35 PM, Uwe Kleine-König wrote:
>>> On Tue, Dec 21, 2021 at 12:12:12PM +0100, Lars-Peter Clausen wrote:
>>>> On 12/21/21 11:45 AM, Uwe Kleine-König wrote:
>>>>> similar to patch
>>>>> https://lore.kernel.org/r/4bde7cbd9e43a5909208102094444219d3154466.1640072891.git.vilhelm.gray@gmail.com
>>>>> the usage of struct counter_device::priv can be replaced by
>>>>> container_of which improves type safety and code size.
>>>>>
>>>>> This series depends on above patch, converts the remaining drivers and
>>>>> finally drops struct counter_device::priv.
>>>> Not sure if this is such a good idea. struct counter_device should not be
>>>> embedded in the drivers state struct in the first place.
>>> Just to mention it: My patch series didn't change this, this was already
>>> broken before.
>> I know, but this series has to be reverted when the framework is fixed.
> All drivers have to be touched. With my patch series you have to modify
> one function in each driver, without my patch you have touch nearly
> every function.
>
I'm not so sure. I don't see how you have to modify every function.
You'd keep using priv to get a pointer to the state struct.
That said having a centralized function in each driver to get the state
struct from the counter device doesn't hurt either.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/8] counter: Remove struct counter_device::priv
2021-12-21 12:04 ` Lars-Peter Clausen
2021-12-21 13:22 ` Uwe Kleine-König
@ 2021-12-22 17:51 ` Uwe Kleine-König
1 sibling, 0 replies; 15+ messages in thread
From: Uwe Kleine-König @ 2021-12-22 17:51 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Alexandre Torgue, David Lechner, Fabrice Gasnier, Jarkko Nikula,
Kamel Bouhara, Maxime Coquelin, Oleksij Rempel,
Patrick Havelange, Syed Nayyar Waris, William Breathitt Gray,
linux-arm-kernel, linux-iio, linux-kernel, linux-stm32
[-- Attachment #1: Type: text/plain, Size: 2985 bytes --]
Hello,
On Tue, Dec 21, 2021 at 01:04:50PM +0100, Lars-Peter Clausen wrote:
> On 12/21/21 12:35 PM, Uwe Kleine-König wrote:
> > On Tue, Dec 21, 2021 at 12:12:12PM +0100, Lars-Peter Clausen wrote:
> > > On 12/21/21 11:45 AM, Uwe Kleine-König wrote:
> > > > similar to patch
> > > > https://lore.kernel.org/r/4bde7cbd9e43a5909208102094444219d3154466.1640072891.git.vilhelm.gray@gmail.com
> > > > the usage of struct counter_device::priv can be replaced by
> > > > container_of which improves type safety and code size.
> > > >
> > > > This series depends on above patch, converts the remaining drivers and
> > > > finally drops struct counter_device::priv.
> > > Not sure if this is such a good idea. struct counter_device should not be
> > > embedded in the drivers state struct in the first place.
> > Just to mention it: My patch series didn't change this, this was already
> > broken before.
> I know, but this series has to be reverted when the framework is fixed.
> >
> > > struct counter_device contains a struct device, which is a reference counted
> > > object. But by embedding it in the driver state struct the life time of both
> > > the struct counter_device and and struct device are bound to the life time
> > > of the driver state struct.
> > >
> > > Which means the struct device memory can get freed before the last reference
> > > is dropped, which leads to a use-after-free and undefined behavior.
> > Well, the driver struct is allocated using devm_kzalloc for all drivers.
>
> devm_kzalloc() doesn't make a difference. The managed memory is freed when
> the parent device is unbound/removed. There may very well be reference to
> the counter_device at this point.
>
> > So I think it's not *very* urgent to fix. Still you're right, this
> > should be addressed.
>
> Yes and no, this can trivially be used for privilege escalation, but then
> again on systems with a counter_device probably everything runs as root
> anyway.
I could provoke an oops with the following shell command:
{ sleep 5; echo bang; } > /dev/counter0 & sleep 1; echo 40000000.timer:counter > /sys/bus/platform/drivers/stm32-timer-counter/unbind
I have a protype here to split counter_register() into counter_alloc() +
counter_add(), but I didn't convert a driver to it yet. If you want to
take a look, it's currently available from
https://git.pengutronix.de/git/ukl/linux counter-dev-livetime
(or if you prefer a webif:
https://git.pengutronix.de/cgit/ukl/linux/log/?h=counter-dev-livetime
). I planned to just invest a two or so hours to fix this. But the plan
failed (one reason is that v5.16-rc6 failed to boot on the stm32mp1 I
work on and I bisected that first.)
Maybe I find some time between the years to get this forward a bit.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread