Counter subsystem changes for the 5.17 cycle
Subsystems (4)
cover Counter subsystem changes for the 5.17 cycle
There are only a few changes for the 5.17 cycle: a fix for missing colons in the comments of struct counter_comp, a minor improvement in the access of private data in the ti-eqep driver, and bug fix for the interrupt enablement code of the 104-quad-8 driver. Due to the small number of patches, the respective changes are submitted here as a patch series.
Uwe Kleine-König (1):
counter: ti-eqep: Use container_of instead of struct
counter_device::priv
William Breathitt Gray (1):
counter: 104-quad-8: Fix persistent enabled events bug
Yanteng Si (1):
counter: Add the necessary colons and indents to the comments of
counter_compi
drivers/counter/104-quad-8.c | 82 +++++++++++++++++------------------- drivers/counter/ti-eqep.c | 23 ++++++---- include/linux/counter.h | 40 +++++++++--------- 3 files changed, 73 insertions(+), 72 deletions(-)
base-commit: 1b18af40c1db195619e611faaeae624d6319b1f1
Signature · 1 line
2.33.1
PATCH 1/3 counter: Add the necessary colons and indents to the comments of counter_compi
From: Yanteng Si <[email protected]>
Since commit aaec1a0f76ec ("counter: Internalize sysfs interface code") introduce a warning as:
linux-next/Documentation/driver-api/generic-counter:234: ./include/linux/counter.h:43: WARNING: Unexpected indentation. linux-next/Documentation/driver-api/generic-counter:234: ./include/linux/counter.h:45: WARNING: Block quote ends without a blank line; unexpected unindent.
Add the necessary colons and indents.
--- include/linux/counter.h | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-)
include/linux/counter.h +20 -20
@@ -38,64 +38,64 @@ enum counter_comp_type {* @type: Counter component data type * @name: device-specific component name * @priv: component-relevant data -* @action_read Synapse action mode read callback. The read value of the +* @action_read: Synapse action mode read callback. The read value of the * respective Synapse action mode should be passed back via * the action parameter. -* @device_u8_read Device u8 component read callback. The read value of the +* @device_u8_read: Device u8 component read callback. The read value of the * respective Device u8 component should be passed back via * the val parameter. -* @count_u8_read Count u8 component read callback. The read value of the +* @count_u8_read: Count u8 component read callback. The read value of the * respective Count u8 component should be passed back via * the val parameter. -* @signal_u8_read Signal u8 component read callback. The read value of the +* @signal_u8_read: Signal u8 component read callback. The read value of the * respective Signal u8 component should be passed back via * the val parameter. -* @device_u32_read Device u32 component read callback. The read value of +* @device_u32_read: Device u32 component read callback. The read value of * the respective Device u32 component should be passed * back via the val parameter. -* @count_u32_read Count u32 component read callback. The read value of the +* @count_u32_read: Count u32 component read callback. The read value of the * respective Count u32 component should be passed back via * the val parameter. -* @signal_u32_read Signal u32 component read callback. The read value of +* @signal_u32_read: Signal u32 component read callback. The read value of * the respective Signal u32 component should be passed * back via the val parameter. -* @device_u64_read Device u64 component read callback. The read value of +* @device_u64_read: Device u64 component read callback. The read value of * the respective Device u64 component should be passed * back via the val parameter. -* @count_u64_read Count u64 component read callback. The read value of the +* @count_u64_read: Count u64 component read callback. The read value of the * respective Count u64 component should be passed back via * the val parameter. -* @signal_u64_read Signal u64 component read callback. The read value of +* @signal_u64_read: Signal u64 component read callback. The read value of * the respective Signal u64 component should be passed * back via the val parameter. -* @action_write Synapse action mode write callback. The write value of +* @action_write: Synapse action mode write callback. The write value of * the respective Synapse action mode is passed via the * action parameter. -* @device_u8_write Device u8 component write callback. The write value of +* @device_u8_write: Device u8 component write callback. The write value of * the respective Device u8 component is passed via the val * parameter. -* @count_u8_write Count u8 component write callback. The write value of +* @count_u8_write: Count u8 component write callback. The write value of * the respective Count u8 component is passed via the val * parameter. -* @signal_u8_write Signal u8 component write callback. The write value of +* @signal_u8_write: Signal u8 component write callback. The write value of * the respective Signal u8 component is passed via the val * parameter. -* @device_u32_write Device u32 component write callback. The write value of +* @device_u32_write: Device u32 component write callback. The write value of * the respective Device u32 component is passed via the * val parameter. -* @count_u32_write Count u32 component write callback. The write value of +* @count_u32_write: Count u32 component write callback. The write value of * the respective Count u32 component is passed via the val * parameter. -* @signal_u32_write Signal u32 component write callback. The write value of +* @signal_u32_write: Signal u32 component write callback. The write value of * the respective Signal u32 component is passed via the * val parameter. -* @device_u64_write Device u64 component write callback. The write value of +* @device_u64_write: Device u64 component write callback. The write value of * the respective Device u64 component is passed via the * val parameter. -* @count_u64_write Count u64 component write callback. The write value of +* @count_u64_write: Count u64 component write callback. The write value of * the respective Count u64 component is passed via the val * parameter. -* @signal_u64_write Signal u64 component write callback. The write value of +* @signal_u64_write: Signal u64 component write callback. The write value of * the respective Signal u64 component is passed via the * val parameter. */
Signature · 1 line
2.33.1
PATCH 2/3 counter: ti-eqep: Use container_of instead of struct counter_device::priv
In reply to William Breathitt GrayFrom: Uwe Kleine-König <[email protected]>
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):
$ source/scripts/bloat-o-meter drivers/counter/ti-eqep.o-pre drivers/counter/ti-eqep.o add/remove: 0/0 grow/shrink: 0/9 up/down: 0/-108 (-108) Function old new delta ti_eqep_position_enable_write 132 120 -12 ti_eqep_position_enable_read 260 248 -12 ti_eqep_position_ceiling_write 132 120 -12 ti_eqep_position_ceiling_read 236 224 -12 ti_eqep_function_write 220 208 -12 ti_eqep_function_read 372 360 -12 ti_eqep_count_write 312 300 -12 ti_eqep_count_read 236 224 -12 ti_eqep_action_read 664 652 -12 Total: Before=4598, After=4490, chg -2.35%
--- drivers/counter/ti-eqep.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-)
drivers/counter/ti-eqep.c +14 -9
@@ -87,10 +87,15 @@ struct ti_eqep_cnt {struct regmap *regmap16; }; +static struct ti_eqep_cnt *ti_eqep_count_from_counter (struct counter_device *counter) +{ + return container_of(counter, struct ti_eqep_cnt, counter); +} +static int ti_eqep_count_read (struct counter_device *counter,struct counter_count *count,u64 *val) { -struct ti_eqep_cnt *priv = counter->priv; +struct ti_eqep_cnt *priv =ti_eqep_count_from_counter (counter);u32 cnt;regmap_read (priv->regmap32,QPOSCNT , &cnt);
@@ -102,7 +107,7 @@ static int ti_eqep_count_read(struct counter_device *counter,static int ti_eqep_count_write (struct counter_device *counter,struct counter_count *count,u64 val) { -struct ti_eqep_cnt *priv = counter->priv; +struct ti_eqep_cnt *priv =ti_eqep_count_from_counter (counter);u32 max;regmap_read (priv->regmap32,QPOSMAX , &max);
@@ -116,7 +121,7 @@ static int ti_eqep_function_read(struct counter_device *counter,struct counter_count *count, enum counter_function *function) { - struct ti_eqep_cnt *priv = counter->priv; + struct ti_eqep_cnt *priv = ti_eqep_count_from_counter(counter);u32 qdecctl;regmap_read (priv->regmap16,QDECCTL , &qdecctl);
@@ -143,7 +148,7 @@ static int ti_eqep_function_write(struct counter_device *counter,struct counter_count *count, enum counter_function function) { - struct ti_eqep_cnt *priv = counter->priv; + struct ti_eqep_cnt *priv = ti_eqep_count_from_counter(counter);enum ti_eqep_count_func qsrc;switch (function) {
@@ -173,7 +178,7 @@ static int ti_eqep_action_read(struct counter_device *counter,struct counter_synapse *synapse, enum counter_synapse_action *action) { - struct ti_eqep_cnt *priv = counter->priv; + struct ti_eqep_cnt *priv = ti_eqep_count_from_counter(counter);enum counter_function function;u32 qdecctl;int err;
@@ -245,7 +250,7 @@ static int ti_eqep_position_ceiling_read(struct counter_device *counter,struct counter_count *count, u64 *ceiling) { - struct ti_eqep_cnt *priv = counter->priv; + struct ti_eqep_cnt *priv = ti_eqep_count_from_counter(counter);u32 qposmax;regmap_read (priv->regmap32,QPOSMAX , &qposmax);
@@ -259,7 +264,7 @@ static int ti_eqep_position_ceiling_write(struct counter_device *counter, struct counter_count *count, u64 ceiling) { -struct ti_eqep_cnt *priv = counter->priv; +struct ti_eqep_cnt *priv =ti_eqep_count_from_counter (counter);if (ceiling != (u32 )ceiling)return -ERANGE ;
@@ -272,7 +277,7 @@ static int ti_eqep_position_ceiling_write(struct counter_device *counter,static int ti_eqep_position_enable_read (struct counter_device *counter,struct counter_count *count,u8 *enable) { -struct ti_eqep_cnt *priv = counter->priv; +struct ti_eqep_cnt *priv =ti_eqep_count_from_counter (counter);u32 qepctl;regmap_read (priv->regmap16,QEPCTL , &qepctl);
@@ -285,7 +290,7 @@ static int ti_eqep_position_enable_read(struct counter_device *counter,static int ti_eqep_position_enable_write (struct counter_device *counter,struct counter_count *count,u8 enable) { -struct ti_eqep_cnt *priv = counter->priv; +struct ti_eqep_cnt *priv =ti_eqep_count_from_counter (counter);regmap_write_bits (priv->regmap16,QEPCTL ,QEPCTL_PHEN , enable ?-1 :0 );
Signature · 1 line
2.33.1
PATCH 3/3 counter: 104-quad-8: Fix persistent enabled events bug
In reply to William Breathitt GrayA bug exists if the user executes a COUNTER_ADD_WATCH_IOCTL ioctl call, and then executes a COUNTER_DISABLE_EVENTS_IOCTL ioctl call. Disabling the events should disable the 104-QUAD-8 interrupts, but because of this bug the interrupts are not disabling.
The reason this bug is occurring is because quad8_events_configure() is called when COUNTER_DISABLE_EVENTS_IOCTL is handled, but the next_irq_trigger[] array has not been cleared before it is checked in the loop.
This patch fixes the bug by removing the next_irq_trigger array and instead utilizing a different algorithm of walking the events_list list for the current requested events. When a COUNTER_DISABLE_EVENTS_IOCTL is handled, events_list will be empty and thus all device channels end up with interrupts disabled.
--- drivers/counter/104-quad-8.c | 82 +++++++++++++++++------------------- 1 file changed, 39 insertions(+), 43 deletions(-)
drivers/counter/104-quad-8.c +39 -43
@@ -14,6 +14,7 @@#include <linux/interrupt.h> #include <linux/isa.h> #include <linux/kernel.h> +#include <linux/list.h> #include <linux/module.h> #include <linux/moduleparam.h> #include <linux/types.h>
@@ -44,7 +45,6 @@ MODULE_PARM_DESC(irq, "ACCES 104-QUAD-8 interrupt line numbers");* @ab_enable: array of A and B inputs enable configurations * @preset_enable: array of set_to_preset_on_index attribute configurations * @irq_trigger: array of current IRQ trigger function configurations -* @next_irq_trigger: array of next IRQ trigger function configurations * @synchronous_mode: array of index function synchronous mode configurations * @index_polarity: array of index function polarity configurations * @cable_fault_enable: differential encoder cable status enable configurations
@@ -61,7 +61,6 @@ struct quad8 {unsigned int ab_enable[QUAD8_NUM_COUNTERS ];unsigned int preset_enable[QUAD8_NUM_COUNTERS ];unsigned int irq_trigger[QUAD8_NUM_COUNTERS ]; -unsigned int next_irq_trigger[QUAD8_NUM_COUNTERS ];unsigned int synchronous_mode[QUAD8_NUM_COUNTERS ];unsigned int index_polarity[QUAD8_NUM_COUNTERS ];unsigned int cable_fault_enable;
@@ -390,7 +389,6 @@ static int quad8_action_read(struct counter_device *counter, }enum { -QUAD8_EVENT_NONE =-1 ,QUAD8_EVENT_CARRY =0 ,QUAD8_EVENT_COMPARE =1 ,QUAD8_EVENT_CARRY_BORROW =2 ,
@@ -402,34 +400,49 @@ static int quad8_events_configure(struct counter_device *counter)struct quad8 *const priv = counter->priv;unsigned long irq_enabled =0 ;unsigned long irqflags; -size_t channel; +struct counter_event_node *event_node; +unsigned int next_irq_trigger;unsigned long ior_cfg;unsigned long base_offset;spin_lock_irqsave (&priv->lock, irqflags); -/* Enable interrupts for the requested channels, disable for the rest */ -for (channel =0 ; channel <QUAD8_NUM_COUNTERS ; channel++) { -if (priv->next_irq_trigger[channel] ==QUAD8_EVENT_NONE ) -continue ; +list_for_each_entry (event_node, &counter->events_list, l) { +switch (event_node->event) { +case COUNTER_EVENT_OVERFLOW : + next_irq_trigger =QUAD8_EVENT_CARRY ; +break ; +case COUNTER_EVENT_THRESHOLD : + next_irq_trigger =QUAD8_EVENT_COMPARE ; +break ; +case COUNTER_EVENT_OVERFLOW_UNDERFLOW : + next_irq_trigger =QUAD8_EVENT_CARRY_BORROW ; +break ; +case COUNTER_EVENT_INDEX : + next_irq_trigger =QUAD8_EVENT_INDEX ; +break ; +default : +/* should never reach this path */ +spin_unlock_irqrestore (&priv->lock, irqflags); +return -EINVAL ; + } -if (priv->irq_trigger[channel] != priv->next_irq_trigger[channel]) { -/* Save new IRQ function configuration */ - priv->irq_trigger[channel] = priv->next_irq_trigger[channel]; +/* Skip configuration if it is the same as previously set */ +if (priv->irq_trigger[event_node->channel] == next_irq_trigger) +continue ; -/* Load configuration to I/O Control Register */ - ior_cfg = priv->ab_enable[channel] | - priv->preset_enable[channel] <<1 | - priv->irq_trigger[channel] <<3 ; - base_offset = priv->base +2 * channel +1 ; -outb (QUAD8_CTR_IOR | ior_cfg, base_offset); - } +/* Save new IRQ function configuration */ + priv->irq_trigger[event_node->channel] = next_irq_trigger; -/* Reset next IRQ trigger function configuration */ - priv->next_irq_trigger[channel] =QUAD8_EVENT_NONE ; +/* Load configuration to I/O Control Register */ + ior_cfg = priv->ab_enable[event_node->channel] | + priv->preset_enable[event_node->channel] <<1 | + priv->irq_trigger[event_node->channel] <<3 ; + base_offset = priv->base +2 * event_node->channel +1 ; +outb (QUAD8_CTR_IOR | ior_cfg, base_offset);/* Enable IRQ line */ - irq_enabled |=BIT (channel); + irq_enabled |=BIT (event_node->channel); }outb (irq_enabled, priv->base +QUAD8_REG_INDEX_INTERRUPT );
@@ -442,35 +455,20 @@ 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 counter_event_node *event_node;if (watch->channel >QUAD8_NUM_COUNTERS -1 )return -EINVAL ;switch (watch->event) {case COUNTER_EVENT_OVERFLOW : -if (priv->next_irq_trigger[watch->channel] ==QUAD8_EVENT_NONE ) - priv->next_irq_trigger[watch->channel] =QUAD8_EVENT_CARRY ; -else if (priv->next_irq_trigger[watch->channel] !=QUAD8_EVENT_CARRY ) -return -EINVAL ; -return 0 ;case COUNTER_EVENT_THRESHOLD : -if (priv->next_irq_trigger[watch->channel] ==QUAD8_EVENT_NONE ) - priv->next_irq_trigger[watch->channel] =QUAD8_EVENT_COMPARE ; -else if (priv->next_irq_trigger[watch->channel] !=QUAD8_EVENT_COMPARE ) -return -EINVAL ; -return 0 ;case COUNTER_EVENT_OVERFLOW_UNDERFLOW : -if (priv->next_irq_trigger[watch->channel] ==QUAD8_EVENT_NONE ) - priv->next_irq_trigger[watch->channel] =QUAD8_EVENT_CARRY_BORROW ; -else if (priv->next_irq_trigger[watch->channel] !=QUAD8_EVENT_CARRY_BORROW ) -return -EINVAL ; -return 0 ;case COUNTER_EVENT_INDEX : -if (priv->next_irq_trigger[watch->channel] ==QUAD8_EVENT_NONE ) - priv->next_irq_trigger[watch->channel] =QUAD8_EVENT_INDEX ; -else if (priv->next_irq_trigger[watch->channel] !=QUAD8_EVENT_INDEX ) -return -EINVAL ; +list_for_each_entry (event_node, &counter->next_events_list, l) +if (watch->channel == event_node->channel && + watch->event != event_node->event) +return -EINVAL ;return 0 ;default :return -EINVAL ;
@@ -1183,8 +1181,6 @@ static int quad8_probe(struct device *dev, unsigned int id)outb (QUAD8_CTR_IOR , base_offset +1 );/* Disable index function; negative index polarity */ outb (QUAD8_CTR_IDR , base_offset +1 ); -/* Initialize next IRQ trigger function configuration */ - priv->next_irq_trigger[i] =QUAD8_EVENT_NONE ; }/* Disable Differential Encoder Cable Status for all channels */ outb (0xFF , base[id] +QUAD8_DIFF_ENCODER_CABLE_STATUS );
Signature · 1 line
2.33.1