[PATCH 0/8] counter: Remove struct counter_device::priv

[PATCH 0/8] counter: Remove struct counter_device::priv

[Uwe Kleine-König]

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.

This series was compile tested using ARCH=arm allmodconfig with the
following change:

 config 104_QUAD_8
        tristate "ACCES 104-QUAD-8 driver"
-       depends on PC104 && X86
+       depends on PC104 && X86 || COMPILE_TEST
        select ISA_BUS_API

in drivers/counter/Kconfig.

Best regards
Uwe

Uwe Kleine-König (8):
  counter: 104-quad-8: Use container_of instead of struct
    counter_device::priv
  counter: ftm-quaddec: Use container_of instead of struct
    counter_device::priv
  counter: intel-qeb: Use container_of instead of struct
    counter_device::priv
  counter: interrupt-cnt: Use container_of instead of struct
    counter_device::priv
  counter: microchip-tcp-capture: Use container_of instead of struct
    counter_device::priv
  counter: stm32-lptimer-cnt: Use container_of instead of struct
    counter_device::priv
  counter: stm32-timer-cnt: Use container_of instead of struct
    counter_device::priv
  counter: Remove unused member from struct counter_device

 drivers/counter/104-quad-8.c            | 64 +++++++++++++------------
 drivers/counter/ftm-quaddec.c           | 14 ++++--
 drivers/counter/intel-qep.c             | 24 ++++++----
 drivers/counter/interrupt-cnt.c         | 16 ++++---
 drivers/counter/microchip-tcb-capture.c | 18 ++++---
 drivers/counter/stm32-lptimer-cnt.c     | 24 ++++++----
 drivers/counter/stm32-timer-cnt.c       | 24 ++++++----
 drivers/counter/ti-eqep.c               |  1 -
 include/linux/counter.h                 |  3 --
 9 files changed, 106 insertions(+), 82 deletions(-)


base-commit: fa55b7dcdc43c1aa1ba12bca9d2dd4318c2a0dbf
prerequisite-patch-id: 9459ad8bc78190558df9123f8bebe28ca1c396ea
-- 
2.33.0

[PATCH 1/8] counter: 104-quad-8: Use container_of instead of struct counter_device::priv

[Uwe Kleine-König]

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

[PATCH 2/8] counter: ftm-quaddec: Use container_of instead of struct counter_device::priv

[Uwe Kleine-König]

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

[PATCH 3/8] counter: intel-qeb: Use container_of instead of struct counter_device::priv

[Uwe Kleine-König]

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

[PATCH 4/8] counter: interrupt-cnt: Use container_of instead of struct counter_device::priv

[Uwe Kleine-König]

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

[PATCH 5/8] counter: microchip-tcp-capture: Use container_of instead of struct counter_device::priv

[Uwe Kleine-König]

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

[PATCH 6/8] counter: stm32-lptimer-cnt: Use container_of instead of struct counter_device::priv

[Uwe Kleine-König]

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

[PATCH 7/8] counter: stm32-timer-cnt: Use container_of instead of struct counter_device::priv

[Uwe Kleine-König]

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

[PATCH 8/8] counter: Remove unused member from struct counter_device

[Uwe Kleine-König]

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

Re: [PATCH 0/8] counter: Remove struct counter_device::priv

[Lars-Peter Clausen]

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

Re: [PATCH 0/8] counter: Remove struct counter_device::priv

[Uwe Kleine-König]

[-- 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 --]

Re: [PATCH 0/8] counter: Remove struct counter_device::priv

[Lars-Peter Clausen]

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 :-\

Re: [PATCH 0/8] counter: Remove struct counter_device::priv

[Uwe Kleine-König]

[-- 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 --]

Re: [PATCH 0/8] counter: Remove struct counter_device::priv

[Lars-Peter Clausen]

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.

Re: [PATCH 0/8] counter: Remove struct counter_device::priv

[Uwe Kleine-König]

[-- 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 --]