Introduce the Counter subsystem
Subsystems (8)
v6 cover Introduce the Counter subsystem
This revision is primarily a code refactor and formatting cleanup -- the interface has remained essentially the same as revision 5, with some minor additions.
Here's a brief summary of the Generic Counter interface and related code changes in this revision:
* checkpatch.pl formatting suggestions implemented (parameters line up with opening round bracket, etc.)
* Implement struct counter_ops to enable const structure for driver callbacks
* Provide const string array for count_direction and count_mode options to help maintain a consistent interface
* Reduce local variables by inlining where appropriate
* Bring memory cleanup code closer to respective allocations for clarity and easier debugging
* Implement counter_name_attribute_create function to reduce repeated code for various "name" attributes
* Implement counter_global_attr_register function to organize global counter attribute registration together
* Implement prepare_counter_device_groups_list and prepare_counter_device_groups functions to organize sysfs attribute configuration into logical groups (this makes the counter_register function logic much easier to follow)
Furthermore, the following additions have been made:
* The "ceiling" and "floor" attributes are defined to represent the upper limit and lower limit respectively for a Count
* The following attributes are now part of the main Generic Counter
interface: count_mode, direction, enable, preset, preset_enable, and error_noise
* The "Quadrature x1" and "Quadrature x2" function modes are now known as "Quadrature x1 A" and "Quadrature x2 A" respectively
* The following new function modes are defined in order to support the STM32 LP Timer quadrature modes: Quadrature x1 B, Quadrature x2 B, Quadrature x2 Rising, and Quadrature x2 Falling
* The "num_counts" and "num_signals" attributes are defined to represent the total number of Counts and total number of Signals respectively
The 104-QUAD-8 counter driver has been updated to make use of the Generic Counter API changes ("ceiling" and "floor" attributes, constant counter_ops structure, count_mode and direction string arrays, etc.).
The STM32 Timer counter driver has been updated to make use of the Generic Counter API changes ("ceiling" attribute, constant counter_ops structure, etc.). In addition, Fabrice Gasnier added support for a missing encoder mode (see STM32_COUNT_ENCODER_MODE_2).
Proper Generic Counter interface support for the STM32 LP Timer has been added to its respective counter driver. This is the last in-tree Linux kernel driver using the deprecated IIO Counter interface, so we can safely remove that interface in the future now.
I've added deprecation notices to the IIO Counter attributes to encourage the switch to the Generic Counter interface. I didn't mention a removal date for the relevant code, but perhaps before the next longterm kernel release would be a good idea -- this should prevent the IIO Counter API from persisting longer than necessary.
William Breathitt Gray
Benjamin Gaignard (2):
dt-bindings: counter: Document stm32 quadrature encoder
counter: Add STM32 Timer quadrature encoder
Fabrice Gasnier (1):
counter: stm32-lptimer: add counter device
William Breathitt Gray (6):
counter: Introduce the Generic Counter interface
counter: Documentation: Add Generic Counter sysfs documentation
docs: Add Generic Counter interface documentation
counter: 104-quad-8: Add Generic Counter interface support
counter: 104-quad-8: Documentation: Add Generic Counter sysfs
documentation
iio: counter: Remove IIO counter subdirectory
Documentation/ABI/testing/sysfs-bus-counter | 241 +++
.../ABI/testing/sysfs-bus-counter-104-quad-8 | 36 +
Documentation/ABI/testing/sysfs-bus-iio | 8 +
.../testing/sysfs-bus-iio-counter-104-quad-8 | 16 +
.../{iio => }/counter/stm32-lptimer-cnt.txt | 0
.../bindings/counter/stm32-timer-cnt.txt | 26 +
.../devicetree/bindings/mfd/stm32-lptimer.txt | 2 +-
.../devicetree/bindings/mfd/stm32-timers.txt | 7 +
Documentation/driver-api/generic-counter.rst | 336 ++++
Documentation/driver-api/index.rst | 1 +
MAINTAINERS | 14 +-
drivers/Kconfig | 2 +
drivers/Makefile | 1 +
drivers/counter/104-quad-8.c | 1335 ++++++++++++++
drivers/counter/Kconfig | 59 +
drivers/{iio => }/counter/Makefile | 6 +-
drivers/counter/generic-counter.c | 1541 +++++++++++++++++
drivers/counter/stm32-lptimer-cnt.c | 722 ++++++++
drivers/counter/stm32-timer-cnt.c | 390 +++++
drivers/iio/Kconfig | 1 -
drivers/iio/Makefile | 1 -
drivers/iio/counter/104-quad-8.c | 596 -------
drivers/iio/counter/Kconfig | 34 -
drivers/iio/counter/stm32-lptimer-cnt.c | 382 ----
include/linux/counter.h | 554 ++++++
25 files changed, 5293 insertions(+), 1018 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-bus-counter
create mode 100644 Documentation/ABI/testing/sysfs-bus-counter-104-quad-8
rename Documentation/devicetree/bindings/{iio => }/counter/stm32-lptimer-cnt.txt (100%)
create mode 100644 Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt
create mode 100644 Documentation/driver-api/generic-counter.rst
create mode 100644 drivers/counter/104-quad-8.c
create mode 100644 drivers/counter/Kconfig
rename drivers/{iio => }/counter/Makefile (52%)
create mode 100644 drivers/counter/generic-counter.c
create mode 100644 drivers/counter/stm32-lptimer-cnt.c
create mode 100644 drivers/counter/stm32-timer-cnt.c
delete mode 100644 drivers/iio/counter/104-quad-8.c
delete mode 100644 drivers/iio/counter/Kconfig
delete mode 100644 drivers/iio/counter/stm32-lptimer-cnt.c
create mode 100644 include/linux/counter.hSignature · 1 line
2.17.0
PATCH v6 1/9 counter: Introduce the Generic Counter interface
This patch introduces the Generic Counter interface for supporting counter devices.
In the context of the Generic Counter interface, a counter is defined as a device that reports one or more "counts" based on the state changes of one or more "signals" as evaluated by a defined "count function."
Driver callbacks should be provided to communicate with the device: to read and write various Signals and Counts, and to set and get the "action mode" and "count function" for various Synapses and Counts respectively.
To support a counter device, a driver must first allocate the available Counter Signals via counter_signal structures. These Signals should be stored as an array and set to the signals array member of an allocated counter_device structure before the Counter is registered to the system.
Counter Counts may be allocated via counter_count structures, and respective Counter Signal associations (Synapses) made via counter_synapse structures. Associated counter_synapse structures are stored as an array and set to the the synapses array member of the respective counter_count structure. These counter_count structures are set to the counts array member of an allocated counter_device structure before the Counter is registered to the system.
A counter device is registered to the system by passing the respective initialized counter_device structure to the counter_register function; similarly, the counter_unregister function unregisters the respective Counter. The devm_counter_register and devm_counter_unregister functions serve as device memory-managed versions of the counter_register and counter_unregister functions respectively.
--- MAINTAINERS | 7 + drivers/Kconfig | 2 + drivers/Makefile | 1 + drivers/counter/Kconfig | 18 + drivers/counter/Makefile | 8 + drivers/counter/generic-counter.c | 1541 +++++++++++++++++++++++++++++ include/linux/counter.h | 554 +++++++++++ 7 files changed, 2131 insertions(+) create mode 100644 drivers/counter/Kconfig create mode 100644 drivers/counter/Makefile create mode 100644 drivers/counter/generic-counter.c create mode 100644 include/linux/counter.h
MAINTAINERS +7 -0
@@ -3669,6 +3669,13 @@ W: http://www.fi.muni.cz/~kas/cosa/ S: Maintained F: drivers/net/wan/cosa* +COUNTER SUBSYSTEM +M: William Breathitt Gray <[email protected]> +L: linux-iio at vger.kernel.org +S: Maintained +F: drivers/counter/ +F: include/linux/counter.h + CPMAC ETHERNET DRIVER M: Florian Fainelli <[email protected]> L: netdev at vger.kernel.org
drivers/Kconfig +2 -0
@@ -165,6 +165,8 @@ source "drivers/memory/Kconfig" source "drivers/iio/Kconfig" +source "drivers/counter/Kconfig" + source "drivers/ntb/Kconfig" source "drivers/vme/Kconfig"
drivers/Makefile +1 -0
@@ -165,6 +165,7 @@ obj-$(CONFIG_PM_DEVFREQ) += devfreq/ obj-$(CONFIG_EXTCON) += extcon/ obj-$(CONFIG_MEMORY) += memory/ obj-$(CONFIG_IIO) += iio/ +obj-$(CONFIG_COUNTER) += counter/ obj-$(CONFIG_VME_BUS) += vme/ obj-$(CONFIG_IPACK_BUS) += ipack/ obj-$(CONFIG_NTB) += ntb/
drivers/counter/Kconfig new file +18 -0
@@ -0,0 +1,18 @@ +# +# Counter devices +# +# When adding new entries keep the list in alphabetical order + +menuconfig COUNTER + tristate "Counter support" + help + Provides Generic Counter interface support for counter devices. + + Counter devices are prevalent within a diverse spectrum of industries. + The ubiquitous presence of these devices necessitates a common + interface and standard of interaction and exposure. This driver API + attempts to resolve the issue of duplicate code found among existing + counter device drivers by providing a generic counter interface for + consumption. The Generic Counter interface enables drivers to support + and expose a common set of components and functionality present in + counter devices.
drivers/counter/Makefile new file +8 -0
@@ -0,0 +1,8 @@ +# +# Makefile for Counter devices +# + +# When adding new entries keep the list in alphabetical order + +obj-$(CONFIG_COUNTER) += counter.o +counter-y := generic-counter.o
drivers/counter/generic-counter.c new file +1541 -0
1,541 lines not on this page · raw message
include/linux/counter.h new file +554 -0
554 lines not on this page · raw message
Signature · 1 line
2.17.0
Re: [PATCH v6 1/9] counter: Introduce the Generic Counter interface
Quoted 26 lines · On Wed, 16 May 2018 13:50:43 -0400 William Breathitt Gray…
This patch introduces the Generic Counter interface for supporting counter devices.
In the context of the Generic Counter interface, a counter is defined as a device that reports one or more "counts" based on the state changes of one or more "signals" as evaluated by a defined "count function."
Driver callbacks should be provided to communicate with the device: to read and write various Signals and Counts, and to set and get the "action mode" and "count function" for various Synapses and Counts respectively.
To support a counter device, a driver must first allocate the available Counter Signals via
counter_signalstructures. These Signals should be stored as an array and set to the signals array member of an allocatedcounter_devicestructure before the Counter is registered to the system.Counter Counts may be allocated via
counter_countstructures, and respective Counter Signal associations (Synapses) made viacounter_synapsestructures. Associatedcounter_synapsestructures are stored as an array and set to the the synapses array member of the respectivecounter_countstructure. Thesecounter_countstructures are set to the counts array member of an allocatedcounter_devicestructure before the Counter is registered to the system.
Quoted 8 lines
A counter device is registered to the system by passing the respective initialized
counter_devicestructure to thecounter_registerfunction; similarly, thecounter_unregisterfunction unregisters the respective Counter. Thedevm_counter_registeranddevm_counter_unregisterfunctions serve as device memory-managed versions of thecounter_registerandcounter_unregisterfunctions respectively.Signed-off-by William Breathitt Gray <[email protected]>
A few minor comments inline. I do somewhat wonder if we can cut back on the huge amount of 'similar' code in here, but there tend to be just enough small differences to make that really tricky...
Nothing major enough in here that I really plan on reading it again (though you never know if you change lots ;)
Quoted 37 lines
--- MAINTAINERS | 7 + drivers/Kconfig | 2 + drivers/Makefile | 1 + drivers/counter/Kconfig | 18 + drivers/counter/Makefile | 8 + drivers/counter/generic-counter.c | 1541 +++++++++++++++++++++++++++++ include/linux/counter.h | 554 +++++++++++ 7 files changed, 2131 insertions(+) create mode 100644 drivers/counter/Kconfig create mode 100644 drivers/counter/Makefile create mode 100644 drivers/counter/generic-counter.c create mode 100644 include/linux/counter.hMAINTAINERS +7
-0@@ -3669,6 +3669,13 @@ W: http://www.fi.muni.cz/~kas/cosa/ S: Maintained F: drivers/net/wan/cosa* +COUNTER SUBSYSTEM +M: William Breathitt Gray <[email protected]> +L: linux-iio at vger.kernel.org +S: Maintained +F: drivers/counter/ +F: include/linux/counter.h + CPMAC ETHERNET DRIVER M: Florian Fainelli <[email protected]> L: netdev at vger.kernel.orgdrivers/Kconfig +0
-0
drivers/Kconfig source "drivers/memory/Kconfig" · line 169
Quoted 5 lines
drivers/Kconfig +2
-0@@ -165,6 +165,8 @@ source "drivers/memory/Kconfig" source "drivers/iio/Kconfig" +source "drivers/counter/Kconfig" +
Same comment as below.
drivers/Makefile obj-$(CONFIG_PM_DEVFREQ) += devfreq/ · line 168
Quoted 12 lines
drivers/Kconfig +0
-0source "drivers/ntb/Kconfig" source "drivers/vme/Kconfig"drivers/Makefile +1
-0@@ -165,6 +165,7 @@ obj-$(CONFIG_PM_DEVFREQ) += devfreq/ obj-$(CONFIG_EXTCON) += extcon/ obj-$(CONFIG_MEMORY) += memory/ obj-$(CONFIG_IIO) += iio/ +obj-$(CONFIG_COUNTER) += counter/
I can see your logic in putting this here, but I think the convention is to go at the end rather than grouping.
Quoted 50 lines
drivers/Makefile +0
-0obj-$(CONFIG_VME_BUS) += vme/ obj-$(CONFIG_IPACK_BUS) += ipack/ obj-$(CONFIG_NTB) += ntb/drivers/counter/Kconfig new file +18
-0@@ -0,0 +1,18 @@ +# +# Counter devices +# +# When adding new entries keep the list in alphabetical order + +menuconfig COUNTER + tristate "Counter support" + help + Provides Generic Counter interface support for counter devices. + + Counter devices are prevalent within a diverse spectrum of industries. + The ubiquitous presence of these devices necessitates a common + interface and standard of interaction and exposure. This driver API + attempts to resolve the issue of duplicate code found among existing + counter device drivers by providing a generic counter interface for + consumption. The Generic Counter interface enables drivers to support + and expose a common set of components and functionality present in + counter devices.drivers/counter/Makefile new file +8
-0@@ -0,0 +1,8 @@ +# +# Makefile for Counter devices +# + +# When adding new entries keep the list in alphabetical order + +obj-$(CONFIG_COUNTER) += counter.o +counter-y := generic-counter.odrivers/counter/generic-counter.c new file +3
-0@@ -0,0 +1,1541 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* +* Generic Counter interface
drivers/counter/generic-counter.c line 13
Quoted 10 lines
drivers/counter/generic-counter.c new file +10
-0+* Copyright (C) 2017 William Breathitt Gray +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License, version 2, as +* published by the Free Software Foundation. +* +* This program is distributed in the hope that it will be useful, but +* WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +* General Public License for more details.
As below, SPDX and license seems silly. Unless you are feeling paranoid just drop the license text.
Quoted 661 lines
drivers/counter/generic-counter.c new file +661
-0+*/ +#include <linux/device.h> +#include <linux/err.h> +#include <linux/export.h> +#include <linux/fs.h> +#include <linux/gfp.h> +#include <linux/idr.h> +#include <linux/init.h> +#include <linux/kernel.h> +#include <linux/list.h> +#include <linux/module.h> +#include <linux/printk.h> +#include <linux/slab.h> +#include <linux/string.h> +#include <linux/sysfs.h> +#include <linux/types.h> + +#include <linux/counter.h> + +const char *const count_direction_str[2 ] = { + [COUNT_DIRECTION_FORWARD ] ="forward" , + [COUNT_DIRECTION_BACKWARD ] ="backward" +}; +EXPORT_SYMBOL (count_direction_str); + +const char *const count_mode_str[4 ] = { + [COUNT_MODE_NORMAL ] ="normal" , + [COUNT_MODE_RANGE_LIMIT ] ="range limit" , + [COUNT_MODE_NON_RECYCLE ] ="non-recycle" , + [COUNT_MODE_MODULO_N ] ="modulo-n" +}; +EXPORT_SYMBOL (count_mode_str); + +ssize_t counter_signal_enum_read (struct counter_device *counter, +struct counter_signal *signal,void *priv, +char *buf) +{ +const struct counter_signal_enum_ext *const e = priv; +int err; +size_t index; + +if (!e->get) +return -EINVAL ; + + err = e->get (counter, signal, &index); +if (err) +return err; + +if (index >= e->num_items) +return -EINVAL ; + +return scnprintf (buf,PAGE_SIZE ,"%s\n" , e->items[index]); +} +EXPORT_SYMBOL (counter_signal_enum_read); + +ssize_t counter_signal_enum_write (struct counter_device *counter, +struct counter_signal *signal,void *priv, +const char *buf,size_t len) +{ +const struct counter_signal_enum_ext *const e = priv; +ssize_t index; +int err; + +if (!e->set) +return -EINVAL ; + + index =__sysfs_match_string (e->items, e->num_items, buf); +if (index <0 ) +return index; + + err = e->set (counter, signal, index); +if (err) +return err; + +return len; +} +EXPORT_SYMBOL (counter_signal_enum_write); + +ssize_t counter_signal_enum_available_read (struct counter_device *counter, +struct counter_signal *signal, +void *priv,char *buf) +{ +const struct counter_signal_enum_ext *const e = priv; +size_t i; +size_t len =0 ; + +if (!e->num_items) +return 0 ; + +for (i =0 ; i < e->num_items; i++) + len +=scnprintf (buf + len,PAGE_SIZE - len,"%s\n" , + e->items[i]); + +return len; +} +EXPORT_SYMBOL (counter_signal_enum_available_read); + +ssize_t counter_count_enum_read (struct counter_device *counter, +struct counter_count *count,void *priv, +char *buf) +{ +const struct counter_count_enum_ext *const e = priv; +int err; +size_t index; + +if (!e->get) +return -EINVAL ; + + err = e->get (counter, count, &index); +if (err) +return err; + +if (index >= e->num_items) +return -EINVAL ; + +return scnprintf (buf,PAGE_SIZE ,"%s\n" , e->items[index]); +} +EXPORT_SYMBOL (counter_count_enum_read); + +ssize_t counter_count_enum_write (struct counter_device *counter, +struct counter_count *count,void *priv, +const char *buf,size_t len) +{ +const struct counter_count_enum_ext *const e = priv; +ssize_t index; +int err; + +if (!e->set) +return -EINVAL ; + + index =__sysfs_match_string (e->items, e->num_items, buf); +if (index <0 ) +return index; + + err = e->set (counter, count, index); +if (err) +return err; + +return len; +} +EXPORT_SYMBOL (counter_count_enum_write); + +ssize_t counter_count_enum_available_read (struct counter_device *counter, +struct counter_count *count, +void *priv,char *buf) +{ +const struct counter_count_enum_ext *const e = priv; +size_t i; +size_t len =0 ; + +if (!e->num_items) +return 0 ; + +for (i =0 ; i < e->num_items; i++) + len +=scnprintf (buf + len,PAGE_SIZE - len,"%s\n" , + e->items[i]); + +return len; +} +EXPORT_SYMBOL (counter_count_enum_available_read); + +ssize_t counter_device_enum_read (struct counter_device *counter,void *priv, +char *buf) +{ +const struct counter_device_enum_ext *const e = priv; +int err; +size_t index; + +if (!e->get) +return -EINVAL ; + + err = e->get (counter, &index); +if (err) +return err; + +if (index >= e->num_items) +return -EINVAL ; + +return scnprintf (buf,PAGE_SIZE ,"%s\n" , e->items[index]); +} +EXPORT_SYMBOL (counter_device_enum_read); + +ssize_t counter_device_enum_write (struct counter_device *counter,void *priv, +const char *buf,size_t len) +{ +const struct counter_device_enum_ext *const e = priv; +ssize_t index; +int err; + +if (!e->set) +return -EINVAL ; + + index =__sysfs_match_string (e->items, e->num_items, buf); +if (index <0 ) +return index; + + err = e->set (counter, index); +if (err) +return err; + +return len; +} +EXPORT_SYMBOL (counter_device_enum_write); + +ssize_t counter_device_enum_available_read (struct counter_device *counter, +void *priv,char *buf) +{ +const struct counter_device_enum_ext *const e = priv; +size_t i; +size_t len =0 ; + +if (!e->num_items) +return 0 ; + +for (i =0 ; i < e->num_items; i++) + len +=scnprintf (buf + len,PAGE_SIZE - len,"%s\n" , + e->items[i]); + +return len; +} +EXPORT_SYMBOL (counter_device_enum_available_read); + +static const char *const signal_level_str[] = { + [SIGNAL_LEVEL_LOW ] ="low" , + [SIGNAL_LEVEL_HIGH ] ="high" +}; + +/** +* set_signal_read_value - set signal_read_value data +* @val: signal_read_value structure to set +* @type: property Signal data represents +* @data: Signal data +* +* This function sets an opaque signal_read_value structure with the provided +* Signal data. +*/ +void set_signal_read_value (struct signal_read_value *const val, +const enum signal_value_type type,void *const data) +{ +if (type ==SIGNAL_LEVEL ) + val->len =scnprintf (val->buf,PAGE_SIZE ,"%s\n" , + signal_level_str[*(enum signal_level *)data]); +else + val->len =0 ; +} +EXPORT_SYMBOL (set_signal_read_value); + +/** +* set_count_read_value - set count_read_value data +* @val: count_read_value structure to set +* @type: property Count data represents +* @data: Count data +* +* This function sets an opaque count_read_value structure with the provided +* Count data. +*/ +void set_count_read_value (struct count_read_value *const val, +const enum count_value_type type,void *const data) +{ +switch (type) { +case COUNT_POSITION_UNSIGNED : + val->len =scnprintf (val->buf,PAGE_SIZE ,"%lu\n" , + *(unsigned long *)data); +break ; +case COUNT_POSITION_SIGNED : + val->len =scnprintf (val->buf,PAGE_SIZE ,"%ld\n" , + *(long *)data); +break ; +default : + val->len =0 ; + } +} +EXPORT_SYMBOL (set_count_read_value); + +/** +* get_count_write_value - get count_write_value data +* @data: Count data +* @type: property Count data represents +* @val: count_write_value structure containing data +* +* This function extracts Count data from the provided opaque count_write_value +* structure and stores it at the address provided by @data. +* +* RETURNS: +* 0 on success, negative error number on failure. +*/ +int get_count_write_value (void *const data,const enum count_value_type type, +const struct count_write_value *const val) +{ +int err; + +switch (type) { +case COUNT_POSITION_UNSIGNED : + err =kstrtoul (val->buf,0 , data); +if (err) +return err; +break ; +case COUNT_POSITION_SIGNED : + err =kstrtol (val->buf,0 , data); +if (err) +return err; +break ; + } + +return 0 ; +} +EXPORT_SYMBOL (get_count_write_value); + +struct counter_device_attr { +struct device_attribute dev_attr; +struct list_head l; +void *component; +}; + +static int counter_attribute_create ( +struct counter_device_attr_group *const group, +const char *const prefix, +const char *const name, +ssize_t (*show)(struct device *dev,struct device_attribute *attr, +char *buf), +ssize_t (*store)(struct device *dev,struct device_attribute *attr, +const char *buf,size_t len), +void *const component) +{ +struct counter_device_attr *counter_attr; +struct device_attribute *dev_attr; +int err; +struct list_head *const attr_list = &group->attr_list; + +/* Allocate a Counter device attribute */ + counter_attr =kzalloc (sizeof (*counter_attr),GFP_KERNEL ); +if (!counter_attr) +return -ENOMEM ; + dev_attr = &counter_attr->dev_attr; + +sysfs_attr_init (&dev_attr->attr); + +/* Configure device attribute */ + dev_attr->attr.name =kasprintf (GFP_KERNEL ,"%s%s" , prefix, name); +if (!dev_attr->attr.name) { + err = -ENOMEM ; + goto err_free_counter_attr; + } +if (show) { + dev_attr->attr.mode |=0444 ; + dev_attr->show = show; + } +if (store) { + dev_attr->attr.mode |=0200 ; + dev_attr->store = store; + } + +/* Store associated Counter component with attribute */ + counter_attr->component = component; + +/* Keep track of the attribute for later cleanup */ +list_add (&counter_attr->l, attr_list); + group->num_attr++; + +return 0 ; + +err_free_counter_attr: +kfree (counter_attr); +return err; +} + +#define to_counter_attr (_dev_attr) \ + container_of(_dev_attr, struct counter_device_attr, dev_attr) + +struct signal_comp_t { +struct counter_signal *signal; +}; + +static ssize_t counter_signal_show (struct device *dev, +struct device_attribute *attr,char *buf) +{ +struct counter_device *const counter =dev_get_drvdata (dev); +const struct counter_device_attr *const devattr =to_counter_attr (attr); +const struct signal_comp_t *const component = devattr->component; +struct counter_signal *const signal = component->signal; +int err; +struct signal_read_value val = { .buf = buf }; + + err = counter->ops->signal_read (counter, signal, &val); +if (err) +return err; + +return val.len; +} + +struct name_comp_t { +const char *name; +}; + +static ssize_t counter_device_attr_name_show (struct device *dev, +struct device_attribute *attr, +char *buf) +{ +const struct name_comp_t *const comp =to_counter_attr (attr)->component; + +return scnprintf (buf,PAGE_SIZE ,"%s\n" , comp->name); +} + +static int counter_name_attribute_create ( +struct counter_device_attr_group *const group, +const char *const name) +{ +struct name_comp_t *name_comp; +int err; + +/* Skip if no name */ +if (!name) +return 0 ; + +/* Allocate name attribute component */ + name_comp =kmalloc (sizeof (*name_comp),GFP_KERNEL ); +if (!name_comp) +return -ENOMEM ; + name_comp->name = name; + +/* Allocate Signal name attribute */ + err =counter_attribute_create (group,"" ,"name" , + counter_device_attr_name_show,NULL , + name_comp); +if (err) + goto err_free_name_comp; + +return 0 ; + +err_free_name_comp: +kfree (name_comp); +return err; +} + +struct signal_ext_comp_t { +struct counter_signal *signal; +const struct counter_signal_ext *ext; +}; + +static ssize_t counter_signal_ext_show (struct device *dev, +struct device_attribute *attr,char *buf) +{ +const struct counter_device_attr *const devattr =to_counter_attr (attr); +const struct signal_ext_comp_t *const comp = devattr->component; +const struct counter_signal_ext *const ext = comp->ext; + +return ext->read (dev_get_drvdata (dev), comp->signal, ext->priv, buf); +} + +static ssize_t counter_signal_ext_store (struct device *dev, +struct device_attribute *attr, +const char *buf,size_t len) +{ +const struct counter_device_attr *const devattr =to_counter_attr (attr); +const struct signal_ext_comp_t *const comp = devattr->component; +const struct counter_signal_ext *const ext = comp->ext; + +return ext->write (dev_get_drvdata (dev), comp->signal, ext->priv, buf, + len); +} + +static void free_counter_device_attr_list (struct list_head *attr_list) +{ +struct counter_device_attr *p, *n; + +list_for_each_entry_safe (p, n, attr_list, l) { +kfree (p->dev_attr.attr.name); +kfree (p->component); +list_del (&p->l); +kfree (p); + } +} + +static int counter_signal_ext_register ( +struct counter_device_attr_group *const group, +struct counter_signal *const signal) +{ +const size_t num_ext = signal->num_ext; +size_t i; +const struct counter_signal_ext *ext; +struct signal_ext_comp_t *signal_ext_comp; +int err; + +/* Create an attribute for each extension */ +for (i =0 ; i < num_ext; i++) { + ext = signal->ext + i; + +/* Allocate signal_ext attribute component */ + signal_ext_comp =kmalloc (sizeof (*signal_ext_comp),GFP_KERNEL ); +if (!signal_ext_comp) { + err = -ENOMEM ; + goto err_free_attr_list; + } + signal_ext_comp->signal = signal; + signal_ext_comp->ext = ext; + +/* Allocate a Counter device attribute */ + err =counter_attribute_create (group,"" , ext->name, + (ext->read) ? counter_signal_ext_show :NULL , + (ext->write) ? counter_signal_ext_store :NULL , + signal_ext_comp); +if (err) { +kfree (signal_ext_comp); + goto err_free_attr_list; + } + } + +return 0 ; + +err_free_attr_list: +free_counter_device_attr_list (&group->attr_list); +return err; +} + +static int counter_signal_attributes_create ( +struct counter_device_attr_group *const group, +const struct counter_device *const counter, +struct counter_signal *const signal) +{ +struct signal_comp_t *signal_comp; +int err; + +/* Allocate Signal attribute component */ + signal_comp =kmalloc (sizeof (*signal_comp),GFP_KERNEL ); +if (!signal_comp) +return -ENOMEM ; + signal_comp->signal = signal; + +/* Create main Signal attribute */ + err =counter_attribute_create (group,"" ,"signal" , + (counter->ops->signal_read) ? counter_signal_show :NULL ,NULL , + signal_comp); +if (err) { +kfree (signal_comp); +return err; + } + +/* Create Signal name attribute */ + err =counter_name_attribute_create (group, signal->name); +if (err) + goto err_free_attr_list; + +/* Register Signal extension attributes */ + err =counter_signal_ext_register (group, signal); +if (err) + goto err_free_attr_list; + +return 0 ; + +err_free_attr_list: +free_counter_device_attr_list (&group->attr_list); +return err; +} + +static int counter_signals_register ( +struct counter_device_attr_group *const groups_list, +const struct counter_device *const counter) +{ +const size_t num_signals = counter->num_signals; +size_t i; +struct counter_signal *signal; +const char *name; +int err; + +/* Register each Signal */ +for (i =0 ; i < num_signals; i++) { + signal = counter->signals + i; + +/* Generate Signal attribute directory name */ + name =kasprintf (GFP_KERNEL ,"signal%d" , signal->id); +if (!name) { + err = -ENOMEM ; + goto err_free_attr_groups; + } + groups_list[i].attr_group.name = name; + +/* Create all attributes associated with Signal */ + err =counter_signal_attributes_create (groups_list + i, counter, + signal); +if (err) + goto err_free_attr_groups; + } + +return 0 ; + +err_free_attr_groups: +do { +kfree (groups_list[i].attr_group.name); +free_counter_device_attr_list (&groups_list[i].attr_list); + }while (i--); +return err; +} + +static const char *const synapse_action_str[] = { + [SYNAPSE_ACTION_NONE ] ="none" , + [SYNAPSE_ACTION_RISING_EDGE ] ="rising edge" , + [SYNAPSE_ACTION_FALLING_EDGE ] ="falling edge" , + [SYNAPSE_ACTION_BOTH_EDGES ] ="both edges" +}; + +struct action_comp_t { +struct counter_synapse *synapse; +struct counter_count *count; +}; + +static ssize_t counter_action_show (struct device *dev, +struct device_attribute *attr,char *buf) +{ +const struct counter_device_attr *const devattr =to_counter_attr (attr); +int err; +struct counter_device *const counter =dev_get_drvdata (dev); +const struct action_comp_t *const component = devattr->component; +struct counter_count *const count = component->count; +struct counter_synapse *const synapse = component->synapse; +size_t action_index; +enum synapse_action action; + + err = counter->ops->action_get (counter, count, synapse, &action_index); +if (err) +return err; + + synapse->action = action_index; + + action = synapse->actions_list[action_index]; +return scnprintf (buf,PAGE_SIZE ,"%s\n" , synapse_action_str[action]); +} + +static ssize_t counter_action_store (struct device *dev, +struct device_attribute *attr, +const char *buf,size_t len) +{ +const struct counter_device_attr *const devattr =to_counter_attr (attr); +const struct action_comp_t *const component = devattr->component; +struct counter_synapse *const synapse = component->synapse; +size_t action_index; +const size_t num_actions = synapse->num_actions; +enum synapse_action action; +int err; +struct counter_device *const counter =dev_get_drvdata (dev); +struct counter_count *const count = component->count; + +/* Find requested action mode */ +for (action_index =0 ; action_index < num_actions; action_index++) { + action = synapse->actions_list[action_index]; +if (sysfs_streq (buf, synapse_action_str[action])) +break ; + } +/* If requested action mode not found */ +if (action_index >= num_actions) +return -EINVAL ; + + err = counter->ops->action_set (counter, count, synapse, action_index); +if (err) +return err; + + synapse->action = action_index; + +return len; +} + +struct action_avail_comp_t {
drivers/counter/generic-counter.c line 684
Quoted 10 lines
drivers/counter/generic-counter.c new file +10
-0+const enum synapse_action *actions_list; +size_t num_actions; +}; + +static ssize_t counter_synapse_action_available_show (struct device *dev, +struct device_attribute *attr,char *buf) +{ +const struct counter_device_attr *const devattr =to_counter_attr (attr); +const struct action_avail_comp_t *const component = devattr->component; +const enum synapse_action *const actions_list = component->actions_list;
I'm not sure this local variable helps much either...
drivers/counter/generic-counter.c line 685
Quoted 1 line
drivers/counter/generic-counter.c new file +1
-0+const size_t num_actions = component->num_actions;
Local variable adds nothing as far as I can see.. Just use it inline.
Quoted 9 lines
drivers/counter/generic-counter.c new file +9
-0+size_t i; +enum synapse_action action; +ssize_t len =0 ; + +for (i =0 ; i < num_actions; i++) { + action = actions_list[i]; + len +=scnprintf (buf + len,PAGE_SIZE - len,"%s\n" , + synapse_action_str[action]); + }
drivers/counter/generic-counter.c line 704
Quoted 10 lines
drivers/counter/generic-counter.c new file +10
-0+ +return len; +} + +static int counter_synapses_register ( +struct counter_device_attr_group *const group, +const struct counter_device *const counter, +struct counter_count *const count,const char *const count_attr_name) +{ +const size_t num_synapses = count->num_synapses;
Local variable doesn't add anything. Only used once.
Quoted 197 lines
drivers/counter/generic-counter.c new file +197
-0+size_t i; +struct counter_synapse *synapse; +const char *prefix; +struct action_comp_t *action_comp; +int err; +struct action_avail_comp_t *avail_comp; + +/* Register each Synapse */ +for (i =0 ; i < num_synapses; i++) { + synapse = count->synapses + i; + +/* Generate attribute prefix */ + prefix =kasprintf (GFP_KERNEL ,"signal%d_" , + synapse->signal->id); +if (!prefix) { + err = -ENOMEM ; + goto err_free_attr_list; + } + +/* Allocate action attribute component */ + action_comp =kmalloc (sizeof (*action_comp),GFP_KERNEL ); +if (!action_comp) { + err = -ENOMEM ; + goto err_free_prefix; + } + action_comp->synapse = synapse; + action_comp->count = count; + +/* Create action attribute */ + err =counter_attribute_create (group, prefix,"action" , + (counter->ops->action_get) ? counter_action_show :NULL , + (counter->ops->action_set) ? counter_action_store :NULL , + action_comp); +if (err) { +kfree (action_comp); + goto err_free_prefix; + } + +/* Allocate action available attribute component */ + avail_comp =kmalloc (sizeof (*avail_comp),GFP_KERNEL ); +if (!avail_comp) { + err = -ENOMEM ; + goto err_free_prefix; + } + avail_comp->actions_list = synapse->actions_list; + avail_comp->num_actions = synapse->num_actions; + +/* Create action_available attribute */ + err =counter_attribute_create (group, prefix, +"action_available" , + counter_synapse_action_available_show,NULL , + avail_comp); +if (err) { +kfree (avail_comp); + goto err_free_prefix; + } + +kfree (prefix); + } + +return 0 ; + +err_free_prefix: +kfree (prefix); +err_free_attr_list: +free_counter_device_attr_list (&group->attr_list); +return err; +} + +struct count_comp_t { +struct counter_count *count; +}; + +static ssize_t counter_count_show (struct device *dev, +struct device_attribute *attr, +char *buf) +{ +struct counter_device *const counter =dev_get_drvdata (dev); +const struct counter_device_attr *const devattr =to_counter_attr (attr); +const struct count_comp_t *const component = devattr->component; +struct counter_count *const count = component->count; +int err; +struct count_read_value val = { .buf = buf }; + + err = counter->ops->count_read (counter, count, &val); +if (err) +return err; + +return val.len; +} + +static ssize_t counter_count_store (struct device *dev, +struct device_attribute *attr, +const char *buf,size_t len) +{ +struct counter_device *const counter =dev_get_drvdata (dev); +const struct counter_device_attr *const devattr =to_counter_attr (attr); +const struct count_comp_t *const component = devattr->component; +struct counter_count *const count = component->count; +int err; +struct count_write_value val = { .buf = buf }; + + err = counter->ops->count_write (counter, count, &val); +if (err) +return err; + +return len; +} + +static const char *const count_function_str[] = { + [COUNT_FUNCTION_INCREASE ] ="increase" , + [COUNT_FUNCTION_DECREASE ] ="decrease" , + [COUNT_FUNCTION_PULSE_DIRECTION ] ="pulse-direction" , + [COUNT_FUNCTION_QUADRATURE_X1_A ] ="quadrature x1 a" , + [COUNT_FUNCTION_QUADRATURE_X1_B ] ="quadrature x1 b" , + [COUNT_FUNCTION_QUADRATURE_X2_A ] ="quadrature x2 a" , + [COUNT_FUNCTION_QUADRATURE_X2_B ] ="quadrature x2 b" , + [COUNT_FUNCTION_QUADRATURE_X2_RISING ] ="quadrature x2 rising" , + [COUNT_FUNCTION_QUADRATURE_X2_FALLING ] ="quadrature x2 falling" , + [COUNT_FUNCTION_QUADRATURE_X4 ] ="quadrature x4" +}; + +static ssize_t counter_function_show (struct device *dev, +struct device_attribute *attr,char *buf) +{ +int err; +struct counter_device *const counter =dev_get_drvdata (dev); +const struct counter_device_attr *const devattr =to_counter_attr (attr); +const struct count_comp_t *const component = devattr->component; +struct counter_count *const count = component->count; +size_t func_index; +enum count_function function; + + err = counter->ops->function_get (counter, count, &func_index); +if (err) +return err; + + count->function = func_index; + + function = count->functions_list[func_index]; +return scnprintf (buf,PAGE_SIZE ,"%s\n" , count_function_str[function]); +} + +static ssize_t counter_function_store (struct device *dev, +struct device_attribute *attr, +const char *buf,size_t len) +{ +const struct counter_device_attr *const devattr =to_counter_attr (attr); +const struct count_comp_t *const component = devattr->component; +struct counter_count *const count = component->count; +const size_t num_functions = count->num_functions; +size_t func_index; +enum count_function function; +int err; +struct counter_device *const counter =dev_get_drvdata (dev); + +/* Find requested Count function mode */ +for (func_index =0 ; func_index < num_functions; func_index++) { + function = count->functions_list[func_index]; +if (sysfs_streq (buf, count_function_str[function])) +break ; + } +/* Return error if requested Count function mode not found */ +if (func_index >= num_functions) +return -EINVAL ; + + err = counter->ops->function_set (counter, count, func_index); +if (err) +return err; + + count->function = func_index; + +return len; +} + +struct count_ext_comp_t { +struct counter_count *count; +const struct counter_count_ext *ext; +}; + +static ssize_t counter_count_ext_show (struct device *dev, +struct device_attribute *attr,char *buf) +{ +const struct counter_device_attr *const devattr =to_counter_attr (attr); +const struct count_ext_comp_t *const comp = devattr->component; +const struct counter_count_ext *const ext = comp->ext; + +return ext->read (dev_get_drvdata (dev), comp->count, ext->priv, buf); +} + +static ssize_t counter_count_ext_store (struct device *dev, +struct device_attribute *attr, +const char *buf,size_t len) +{ +const struct counter_device_attr *const devattr =to_counter_attr (attr); +const struct count_ext_comp_t *const comp = devattr->component; +const struct counter_count_ext *const ext = comp->ext;
drivers/counter/generic-counter.c line 911
Quoted 10 lines
drivers/counter/generic-counter.c new file +10
-0+ +return ext->write (dev_get_drvdata (dev), comp->count, ext->priv, buf, + len); +} + +static int counter_count_ext_register ( +struct counter_device_attr_group *const group, +struct counter_count *const count) +{ +const size_t num_ext = count->num_ext;
Used in one place, just put it inline?
Quoted 135 lines
drivers/counter/generic-counter.c new file +135
-0+size_t i; +const struct counter_count_ext *ext; +struct count_ext_comp_t *count_ext_comp; +int err; + +/* Create an attribute for each extension */ +for (i =0 ; i < num_ext; i++) { + ext = count->ext + i; + +/* Allocate count_ext attribute component */ + count_ext_comp =kmalloc (sizeof (*count_ext_comp),GFP_KERNEL ); +if (!count_ext_comp) { + err = -ENOMEM ; + goto err_free_attr_list; + } + count_ext_comp->count = count; + count_ext_comp->ext = ext; + +/* Allocate count_ext attribute */ + err =counter_attribute_create (group,"" , ext->name, + (ext->read) ? counter_count_ext_show :NULL , + (ext->write) ? counter_count_ext_store :NULL , + count_ext_comp); +if (err) { +kfree (count_ext_comp); + goto err_free_attr_list; + } + } + +return 0 ; + +err_free_attr_list: +free_counter_device_attr_list (&group->attr_list); +return err; +} + +struct func_avail_comp_t { +const enum count_function *functions_list; +size_t num_functions; +}; + +static ssize_t counter_count_function_available_show (struct device *dev, +struct device_attribute *attr,char *buf) +{ +const struct counter_device_attr *const devattr =to_counter_attr (attr); +const struct func_avail_comp_t *const component = devattr->component; +const enum count_function *const func_list = component->functions_list; +const size_t num_functions = component->num_functions; +size_t i; +enum count_function function; +ssize_t len =0 ; + +for (i =0 ; i < num_functions; i++) { + function = func_list[i]; + len +=scnprintf (buf + len,PAGE_SIZE - len,"%s\n" , + count_function_str[function]); + } + +return len; +} + +static int counter_count_attributes_create ( +struct counter_device_attr_group *const group, +const struct counter_device *const counter, +struct counter_count *const count) +{ +struct count_comp_t *count_comp; +int err; +struct count_comp_t *func_comp; +struct func_avail_comp_t *avail_comp; + +/* Allocate count attribute component */ + count_comp =kmalloc (sizeof (*count_comp),GFP_KERNEL ); +if (!count_comp) +return -ENOMEM ; + count_comp->count = count; + +/* Create main Count attribute */ + err =counter_attribute_create (group,"" ,"count" , + (counter->ops->count_read) ? counter_count_show :NULL , + (counter->ops->count_write) ? counter_count_store :NULL , + count_comp); +if (err) { +kfree (count_comp); +return err; + } + +/* Allocate function attribute component */ + func_comp =kmalloc (sizeof (*func_comp),GFP_KERNEL ); +if (!func_comp) { + err = -ENOMEM ; + goto err_free_attr_list; + } + func_comp->count = count; + +/* Create Count function attribute */ + err =counter_attribute_create (group,"" ,"function" , + (counter->ops->function_get) ? counter_function_show :NULL , + (counter->ops->function_set) ? counter_function_store :NULL , + func_comp); +if (err) { +kfree (func_comp); + goto err_free_attr_list; + } + +/* Allocate function available attribute component */ + avail_comp =kmalloc (sizeof (*avail_comp),GFP_KERNEL ); +if (!avail_comp) { + err = -ENOMEM ; + goto err_free_attr_list; + } + avail_comp->functions_list = count->functions_list; + avail_comp->num_functions = count->num_functions; + +/* Create Count function_available attribute */ + err =counter_attribute_create (group,"" ,"function_available" , + counter_count_function_available_show, +NULL , avail_comp); +if (err) { +kfree (avail_comp); + goto err_free_attr_list; + } + +/* Create Count name attribute */ + err =counter_name_attribute_create (group, count->name); +if (err) + goto err_free_attr_list; + +/* Register Count extension attributes */ + err =counter_count_ext_register (group, count); +if (err) + goto err_free_attr_list; + +return 0 ; +
drivers/counter/generic-counter.c line 1056
Quoted 10 lines
drivers/counter/generic-counter.c new file +10
-0+err_free_attr_list: +free_counter_device_attr_list (&group->attr_list); +return err; +} + +static int counter_counts_register ( +struct counter_device_attr_group *const groups_list, +const struct counter_device *const counter) +{ +const size_t num_counts = counter->num_counts;
I think this is only used on one place. Not sure having a local variable is worthwhile.
Quoted 99 lines
drivers/counter/generic-counter.c new file +99
-0+size_t i; +struct counter_count *count; +const char *name; +int err; + +/* Register each Count */ +for (i =0 ; i < num_counts; i++) { + count = counter->counts + i; + +/* Generate Count attribute directory name */ + name =kasprintf (GFP_KERNEL ,"count%d" , count->id); +if (!name) { + err = -ENOMEM ; + goto err_free_attr_groups; + } + groups_list[i].attr_group.name = name; + +/* Register the Synapses associated with each Count */ + err =counter_synapses_register (groups_list + i, counter, count, + name); +if (err) + goto err_free_attr_groups; + +/* Create all attributes associated with Count */ + err =counter_count_attributes_create (groups_list + i, counter, + count); +if (err) + goto err_free_attr_groups; + } + +return 0 ; + +err_free_attr_groups: +do { +kfree (groups_list[i].attr_group.name); +free_counter_device_attr_list (&groups_list[i].attr_list); + }while (i--); +return err; +} + +struct size_comp_t { +size_t size; +}; + +static ssize_t counter_device_attr_size_show (struct device *dev, +struct device_attribute *attr, +char *buf) +{ +const struct size_comp_t *const comp =to_counter_attr (attr)->component; + +return scnprintf (buf,PAGE_SIZE ,"%zu\n" , comp->size); +} + +static int counter_size_attribute_create ( +struct counter_device_attr_group *const group, +const size_t size,const char *const name) +{ +struct size_comp_t *size_comp; +int err; + +/* Allocate size attribute component */ + size_comp =kmalloc (sizeof (*size_comp),GFP_KERNEL ); +if (!size_comp) +return -ENOMEM ; + size_comp->size = size; + + err =counter_attribute_create (group,"" , name, + counter_device_attr_size_show,NULL , + size_comp); +if (err) + goto err_free_size_comp; + +return 0 ; + +err_free_size_comp: +kfree (size_comp); +return err; +} + +struct ext_comp_t { +const struct counter_device_ext *ext; +}; + +static ssize_t counter_device_ext_show (struct device *dev, +struct device_attribute *attr,char *buf) +{ +const struct counter_device_attr *const devattr =to_counter_attr (attr); +const struct ext_comp_t *const component = devattr->component; +const struct counter_device_ext *const ext = component->ext; + +return ext->read (dev_get_drvdata (dev), ext->priv, buf); +} + +static ssize_t counter_device_ext_store (struct device *dev, +struct device_attribute *attr, +const char *buf,size_t len) +{ +const struct counter_device_attr *const devattr =to_counter_attr (attr); +const struct ext_comp_t *const component = devattr->component;
drivers/counter/generic-counter.c line 1165
Quoted 10 lines
drivers/counter/generic-counter.c new file +10
-0+const struct counter_device_ext *const ext = component->ext; + +return ext->write (dev_get_drvdata (dev), ext->priv, buf, len); +} + +static int counter_device_ext_register ( +struct counter_device_attr_group *const group, +struct counter_device *const counter) +{ +const size_t num_ext = counter->num_ext;
num_ext only used in one place so if anything the local variable is hurting readability.
drivers/counter/generic-counter.c line 1173
Quoted 8 lines
drivers/counter/generic-counter.c new file +8
-0+struct ext_comp_t *ext_comp; +size_t i; +const struct counter_device_ext *ext; +int err; + +/* Create an attribute for each extension */ +for (i =0 ; i < num_ext; i++) { + ext = counter->ext + i;
This local variable isn't gaining us much that I can see. Just use the value directly.
Quoted 63 lines
drivers/counter/generic-counter.c new file +63
-0+ +/* Allocate extension attribute component */ + ext_comp =kmalloc (sizeof (*ext_comp),GFP_KERNEL ); +if (!ext_comp) { + err = -ENOMEM ; + goto err_free_attr_list; + } + + ext_comp->ext = ext; + +/* Allocate extension attribute */ + err =counter_attribute_create (group,"" , ext->name, + (ext->read) ? counter_device_ext_show :NULL , + (ext->write) ? counter_device_ext_store :NULL , + ext_comp); +if (err) { +kfree (ext_comp); + goto err_free_attr_list; + } + } + +return 0 ; + +err_free_attr_list: +free_counter_device_attr_list (&group->attr_list); +return err; +} + +static int counter_global_attr_register ( +struct counter_device_attr_group *const group, +struct counter_device *const counter) +{ +int err; + +/* Create name attribute */ + err =counter_name_attribute_create (group, counter->name); +if (err) +return err; + +/* Create num_counts attribute */ + err =counter_size_attribute_create (group, counter->num_counts, +"num_counts" ); +if (err) + goto err_free_attr_list; + +/* Create num_signals attribute */ + err =counter_size_attribute_create (group, counter->num_signals, +"num_signals" ); +if (err) + goto err_free_attr_list; + +/* Register Counter device extension attributes */ + err =counter_device_ext_register (group, counter); +if (err) + goto err_free_attr_list; + +return 0 ; + +err_free_attr_list: +free_counter_device_attr_list (&group->attr_list); +return err; +} +
drivers/counter/generic-counter.c line 1246
Quoted 10 lines
drivers/counter/generic-counter.c new file +10
-0+static void free_counter_device_groups_list ( +struct counter_device_attr_group *const groups_list, +const size_t num_groups) +{ +struct counter_device_attr_group *group; +size_t i; + +for (i =0 ; i < num_groups; i++) { + group = groups_list + i; +
I'd like to see a comment somewhere here on the fact this cleans up both the registered per counter stuff and the global attributes (that took a bit of chasing to check it did...)
Quoted 44 lines
drivers/counter/generic-counter.c new file +44
-0+kfree (group->attr_group.name); +kfree (group->attr_group.attrs); +free_counter_device_attr_list (&group->attr_list); + } + +kfree (groups_list); +} + +static int prepare_counter_device_groups_list ( +struct counter_device *const counter) +{ +const size_t total_num_groups = + counter->num_signals + counter->num_counts +1 ; +struct counter_device_attr_group *groups_list; +size_t i; +int err; +size_t num_groups =0 ; + +/* Allocate space for attribute groups (signals. counts, and ext) */ + groups_list =kcalloc (total_num_groups,sizeof (*groups_list), +GFP_KERNEL ); +if (!groups_list) +return -ENOMEM ; + +/* Initialize attribute lists */ +for (i =0 ; i < total_num_groups; i++) +INIT_LIST_HEAD (&groups_list[i].attr_list); + +/* Register Signals */ + err =counter_signals_register (groups_list, counter); +if (err) + goto err_free_groups_list; + num_groups += counter->num_signals; + +/* Register Counts and respective Synapses */ + err =counter_counts_register (groups_list + num_groups, counter); +if (err) + goto err_free_groups_list; + num_groups += counter->num_counts; + +/* Register Counter global attributes */ + err =counter_global_attr_register (groups_list + num_groups, counter); +if (err) + goto err_free_groups_list;
drivers/counter/generic-counter.c line 1300
Quoted 10 lines
drivers/counter/generic-counter.c new file +10
-0+ num_groups++; + +/* Store groups_list in device_state */ + counter->device_state->groups_list = groups_list; + counter->device_state->num_groups = num_groups; + +return 0 ; + +err_free_groups_list: +free_counter_device_groups_list (groups_list, num_groups);
Consistent naming would be good.
counter_device_groups_list_free.
I would tidy this up throughout. I know from experience that you'll probably end up doing so eventually and it is easier whilst there isn't too much code.
drivers/counter/generic-counter.c line 1310
Quoted 10 lines
drivers/counter/generic-counter.c new file +10
-0+return err; +} + +static int prepare_counter_device_groups ( +struct counter_device_state *const device_state) +{ +size_t i; +struct counter_device_attr_group *group; +int err; +size_t j;
Tidy this up a little,size_t i, j;
Quoted 25 lines
drivers/counter/generic-counter.c new file +25
-0+struct counter_device_attr *p; + +/* Allocate attribute groups for association with device */ + device_state->groups =kcalloc (device_state->num_groups +1 , +sizeof (*device_state->groups), +GFP_KERNEL ); +if (!device_state->groups) +return -ENOMEM ; + +/* Prepare each group of attributes for association */ +for (i =0 ; i < device_state->num_groups; i++) { + group = device_state->groups_list + i; + +/* Allocate space for attribute pointers in attribute group */ + group->attr_group.attrs =kcalloc (group->num_attr +1 , +sizeof (*group->attr_group.attrs),GFP_KERNEL ); +if (!group->attr_group.attrs) { + err = -ENOMEM ; + goto err_free_groups; + } + +/* Add attribute pointers to attribute group */ + j =0 ; +list_for_each_entry (p, &group->attr_list, l) + group->attr_group.attrs[j++] = &p->dev_attr.attr;
drivers/counter/generic-counter.c line 1345
Quoted 10 lines
drivers/counter/generic-counter.c new file +10
-0+ +/* Group attributes in attribute group */ + device_state->groups[i] = &group->attr_group; + } +/* Associate attributes with device */ + device_state->dev.groups = device_state->groups; + +return 0 ; + +err_free_groups:
I'm not convinced this is cleaning up properly. What about the kcalloc of group->attr_group.attrs for previous loops?
Quoted 205 lines
drivers/counter/generic-counter.c new file +196
-0+kfree (device_state->groups); +return err; +} + +/* Provides a unique ID for each counter device */ +static DEFINE_IDA (counter_ida); + +static void counter_device_release (struct device *dev) +{ +struct counter_device *const counter =dev_get_drvdata (dev); +struct counter_device_state *const device_state = counter->device_state; + +kfree (device_state->groups); +free_counter_device_groups_list (device_state->groups_list, + device_state->num_groups); +ida_simple_remove (&counter_ida, device_state->id); +kfree (device_state); +} + +static struct device_type counter_device_type = { + .name ="counter_device" , + .release = counter_device_release +}; + +static struct bus_type counter_bus_type = { + .name ="counter" +}; + +/** +* counter_register - register Counter to the system +* @counter: pointer to Counter to register +* +* This function registers a Counter to the system. A sysfs "counter" directory +* will be created and populated with sysfs attributes correlating with the +* Counter Signals, Synapses, and Counts respectively. +*/ +int counter_register (struct counter_device *const counter) +{ +struct counter_device_state *device_state; +int err; + +/* Allocate internal state container for Counter device */ + device_state =kzalloc (sizeof (*device_state),GFP_KERNEL ); +if (!device_state) +return -ENOMEM ; + counter->device_state = device_state; + +/* Acquire unique ID */ + device_state->id =ida_simple_get (&counter_ida,0 ,0 ,GFP_KERNEL ); +if (device_state->id <0 ) { + err = device_state->id; + goto err_free_device_state; + } + +/* Configure device structure for Counter */ + device_state->dev.type = &counter_device_type; + device_state->dev.bus = &counter_bus_type; +if (counter->parent) { + device_state->dev.parent = counter->parent; + device_state->dev.of_node = counter->parent->of_node; + } +dev_set_name (&device_state->dev,"counter%d" , device_state->id); +device_initialize (&device_state->dev); +dev_set_drvdata (&device_state->dev, counter); + +/* Prepare device attributes */ + err =prepare_counter_device_groups_list (counter); +if (err) + goto err_free_id; + +/* Organize device attributes to groups and match to device */ + err =prepare_counter_device_groups (device_state); +if (err) + goto err_free_groups_list; + +/* Add device to system */ + err =device_add (&device_state->dev); +if (err) + goto err_free_groups; + +return 0 ; + +err_free_groups: +kfree (device_state->groups); +err_free_groups_list: +free_counter_device_groups_list (device_state->groups_list, + device_state->num_groups); +err_free_id: +ida_simple_remove (&counter_ida, device_state->id); +err_free_device_state: +kfree (device_state); +return err; +} +EXPORT_SYMBOL (counter_register); + +/** +* counter_unregister - unregister Counter from the system +* @counter: pointer to Counter to unregister +* +* The Counter is unregistered from the system; all allocated memory is freed. +*/ +void counter_unregister (struct counter_device *const counter) +{ +if (counter) +device_del (&counter->device_state->dev); +} +EXPORT_SYMBOL (counter_unregister); + +static void devm_counter_unreg (struct device *dev,void *res) +{ +counter_unregister (*(struct counter_device **)res); +} + +/** +* devm_counter_register - Resource-managed counter_register +* @dev: device to allocate counter_device for +* @counter: pointer to Counter to register +* +* Managed counter_register. The Counter registered with this function is +* automatically unregistered on driver detach. This function calls +* counter_register internally. Refer to that function for more information. +* +* If an Counter registered with this function needs to be unregistered +* separately, devm_counter_unregister must be used. +* +* RETURNS: +* 0 on success, negative error number on failure. +*/ +int devm_counter_register (struct device *dev, +struct counter_device *const counter) +{ +struct counter_device **ptr; +int ret; + + ptr =devres_alloc (devm_counter_unreg,sizeof (*ptr),GFP_KERNEL ); +if (!ptr) +return -ENOMEM ; + + ret =counter_register (counter); +if (!ret) { + *ptr = counter; +devres_add (dev, ptr); + }else { +devres_free (ptr); + } + +return ret; +} +EXPORT_SYMBOL (devm_counter_register); + +static int devm_counter_match (struct device *dev,void *res,void *data) +{ +struct counter_device **r = res; + +if (!r || !*r) { +WARN_ON (!r || !*r); +return 0 ; + } + +return *r == data; +} + +/** +* devm_counter_unregister - Resource-managed counter_unregister +* @dev: device this counter_device belongs to +* @counter: pointer to Counter associated with the device +* +* Unregister Counter registered with devm_counter_register. +*/ +void devm_counter_unregister (struct device *dev, +struct counter_device *const counter) +{ +int rc; + + rc =devres_release (dev, devm_counter_unreg, devm_counter_match, + counter); +WARN_ON (rc); +} +EXPORT_SYMBOL (devm_counter_unregister); + +static int __init counter_init (void ) +{ +return bus_register (&counter_bus_type); +} + +static void __exit counter_exit(void) +{ +bus_unregister (&counter_bus_type); +} + +subsys_initcall (counter_init); +module_exit (counter_exit); + +MODULE_AUTHOR ("William Breathitt Gray <[email protected]>" ); +MODULE_DESCRIPTION ("Generic Counter interface" ); +MODULE_LICENSE ("GPL v2" );include/linux/counter.h new file +3
-0@@ -0,0 +1,554 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* +* Counter interface
include/linux/counter.h line 13
Quoted 10 lines
include/linux/counter.h new file +10
-0+* Copyright (C) 2017 William Breathitt Gray +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License, version 2, as +* published by the Free Software Foundation. +* +* This program is distributed in the hope that it will be useful, but +* WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +* General Public License for more details.
It's a bit controversial, but most people consider SPDX headers equivalent of the license statement. As such you normally don't have both and just go for the SPDK header.
I thought we were also standardising on
// SPDX-...
Quoted 198 lines
include/linux/counter.h new file +198
-0+*/ +#ifndef _COUNTER_H_ +#define _COUNTER_H_ + +#include <linux/device.h> +#include <linux/types.h> + +enum count_direction { +COUNT_DIRECTION_FORWARD =0 , +COUNT_DIRECTION_BACKWARD +}; +extern const char *const count_direction_str[2 ]; + +enum count_mode { +COUNT_MODE_NORMAL =0 , +COUNT_MODE_RANGE_LIMIT , +COUNT_MODE_NON_RECYCLE , +COUNT_MODE_MODULO_N +}; +extern const char *const count_mode_str[4 ]; + +struct counter_device ; +struct counter_signal ; + +/** +* struct counter_signal_ext - Counter Signal extensions +* @name: attribute name +* @read: read callback for this attribute; may be NULL +* @write: write callback for this attribute; may be NULL +* @priv: data private to the driver +*/ +struct counter_signal_ext { +const char *name; +ssize_t (*read)(struct counter_device *counter, +struct counter_signal *signal,void *priv, +char *buf); +ssize_t (*write)(struct counter_device *counter, +struct counter_signal *signal,void *priv, +const char *buf,size_t len); +void *priv; +}; + +/** +* struct counter_signal - Counter Signal node +* @id: unique ID used to identify signal +* @name: device-specific Signal name; ideally, this should match the name +* as it appears in the datasheet documentation +* @ext: optional array of Counter Signal extensions +* @num_ext: number of Counter Signal extensions specified in @ext +* @priv: optional private data supplied by driver +*/ +struct counter_signal { +int id; +const char *name; + +const struct counter_signal_ext *ext; +size_t num_ext; + +void *priv; +}; + +/** +* struct counter_signal_enum_ext - Signal enum extension attribute +* @items: Array of strings +* @num_items: Number of items specified in @items +* @set: Set callback function; may be NULL +* @get: Get callback function; may be NULL +* +* The counter_signal_enum_ext structure can be used to implement enum style +* Signal extension attributes. Enum style attributes are those which have a set +* of strings that map to unsigned integer values. The Generic Counter Signal +* enum extension helper code takes care of mapping between value and string, as +* well as generating a "_available" file which contains a list of all available +* items. The get callback is used to query the currently active item; the index +* of the item within the respective items array is returned via the 'item' +* parameter. The set callback is called when the attribute is updated; the +* 'item' parameter contains the index of the newly activated item within the +* respective items array. +*/ +struct counter_signal_enum_ext { +const char *const *items; +size_t num_items; +int (*get)(struct counter_device *counter, +struct counter_signal *signal, +size_t *item); +int (*set)(struct counter_device *counter, +struct counter_signal *signal, +size_t item); +}; + +ssize_t counter_signal_enum_read (struct counter_device *counter, +struct counter_signal *signal,void *priv, +char *buf); +ssize_t counter_signal_enum_write (struct counter_device *counter, +struct counter_signal *signal,void *priv, +const char *buf,size_t len); + +/** +* COUNTER_SIGNAL_ENUM() - Initialize Signal enum extension +* @_name: Attribute name +* @_e: Pointer to a counter_count_enum structure +* +* This should usually be used together with COUNTER_SIGNAL_ENUM_AVAILABLE() +*/ +#define COUNTER_SIGNAL_ENUM (_name, _e) \ +{ \ + .name = (_name), \ + .read = counter_signal_enum_read, \ + .write = counter_signal_enum_write, \ + .priv = (_e) \ +} + +ssize_t counter_signal_enum_available_read (struct counter_device *counter, +struct counter_signal *signal, +void *priv,char *buf); + +/** +* COUNTER_SIGNAL_ENUM_AVAILABLE() - Initialize Signal enum available extension +* @_name: Attribute name ("_available" will be appended to the name) +* @_e: Pointer to a counter_signal_enum structure +* +* Creates a read only attribute that lists all the available enum items in a +* newline separated list. This should usually be used together with +* COUNTER_SIGNAL_ENUM() +*/ +#define COUNTER_SIGNAL_ENUM_AVAILABLE (_name, _e) \ +{ \ + .name = (_name "_available"), \ + .read = counter_signal_enum_available_read, \ + .priv = (_e) \ +} + +enum synapse_action { +SYNAPSE_ACTION_NONE =0 , +SYNAPSE_ACTION_RISING_EDGE , +SYNAPSE_ACTION_FALLING_EDGE , +SYNAPSE_ACTION_BOTH_EDGES +}; + +/** +* struct counter_synapse - Counter Synapse node +* @action: index of current action mode +* @actions_list: array of available action modes +* @num_actions: number of action modes specified in @actions_list +* @signal: pointer to associated signal +*/ +struct counter_synapse { +size_t action; +const enum synapse_action *actions_list; +size_t num_actions; + +struct counter_signal *signal; +}; + +struct counter_count ; + +/** +* struct counter_count_ext - Counter Count extension +* @name: attribute name +* @read: read callback for this attribute; may be NULL +* @write: write callback for this attribute; may be NULL +* @priv: data private to the driver +*/ +struct counter_count_ext { +const char *name; +ssize_t (*read)(struct counter_device *counter, +struct counter_count *count,void *priv, +char *buf); +ssize_t (*write)(struct counter_device *counter, +struct counter_count *count,void *priv, +const char *buf,size_t len); +void *priv; +}; + +enum count_function { +COUNT_FUNCTION_INCREASE =0 , +COUNT_FUNCTION_DECREASE , +COUNT_FUNCTION_PULSE_DIRECTION , +COUNT_FUNCTION_QUADRATURE_X1_A , +COUNT_FUNCTION_QUADRATURE_X1_B , +COUNT_FUNCTION_QUADRATURE_X2_A , +COUNT_FUNCTION_QUADRATURE_X2_B , +COUNT_FUNCTION_QUADRATURE_X2_RISING , +COUNT_FUNCTION_QUADRATURE_X2_FALLING , +COUNT_FUNCTION_QUADRATURE_X4 +}; + +/** +* struct counter_count - Counter Count node +* @id: unique ID used to identify Count +* @name: device-specific Count name; ideally, this should match +* the name as it appears in the datasheet documentation +* @function: index of current function mode +* @functions_list: array available function modes +* @num_functions: number of function modes specified in @functions_list +* @synapses: array of synapses for initialization +* @num_synapses: number of synapses specified in @synapses +* @ext: optional array of Counter Count extensions
include/linux/counter.h line 221
Quoted 10 lines
include/linux/counter.h new file +10
-0+* @num_ext: number of Counter Count extensions specified in @ext +* @priv: optional private data supplied by driver +*/ +struct counter_count { +int id; +const char *name; + +size_t function; +const enum count_function *functions_list; +size_t num_functions;
There is a very good illustration here of the issues with using tabs to pretty print structure elements. I would drop them entirely as personally I'm not sure they help readability and you will forever be having more noise in patches because of the need to change the number of tabs due to name changes etc.
Quoted 88 lines
include/linux/counter.h new file +88
-0+ +struct counter_synapse *synapses; +size_t num_synapses; + +const struct counter_count_ext *ext; +size_t num_ext; + +void *priv; +}; + +/** +* struct counter_count_enum_ext - Count enum extension attribute +* @items: Array of strings +* @num_items: Number of items specified in @items +* @set: Set callback function; may be NULL +* @get: Get callback function; may be NULL +* +* The counter_count_enum_ext structure can be used to implement enum style +* Count extension attributes. Enum style attributes are those which have a set +* of strings that map to unsigned integer values. The Generic Counter Count +* enum extension helper code takes care of mapping between value and string, as +* well as generating a "_available" file which contains a list of all available +* items. The get callback is used to query the currently active item; the index +* of the item within the respective items array is returned via the 'item' +* parameter. The set callback is called when the attribute is updated; the +* 'item' parameter contains the index of the newly activated item within the +* respective items array. +*/ +struct counter_count_enum_ext { +const char *const *items; +size_t num_items; +int (*get)(struct counter_device *counter, +struct counter_count *count, +size_t *item); +int (*set)(struct counter_device *counter, +struct counter_count *count, +size_t item); +}; + +ssize_t counter_count_enum_read (struct counter_device *counter, +struct counter_count *count,void *priv, +char *buf); +ssize_t counter_count_enum_write (struct counter_device *counter, +struct counter_count *count,void *priv, +const char *buf,size_t len); + +/** +* COUNTER_COUNT_ENUM() - Initialize Count enum extension +* @_name: Attribute name +* @_e: Pointer to a counter_count_enum structure +* +* This should usually be used together with COUNTER_COUNT_ENUM_AVAILABLE() +*/ +#define COUNTER_COUNT_ENUM (_name, _e) \ +{ \ + .name = (_name), \ + .read = counter_count_enum_read, \ + .write = counter_count_enum_write, \ + .priv = (_e) \ +} + +ssize_t counter_count_enum_available_read (struct counter_device *counter, +struct counter_count *count, +void *priv,char *buf); + +/** +* COUNTER_COUNT_ENUM_AVAILABLE() - Initialize Count enum available extension +* @_name: Attribute name ("_available" will be appended to the name) +* @_e: Pointer to a counter_count_enum structure +* +* Creates a read only attribute that lists all the available enum items in a +* newline separated list. This should usually be used together with +* COUNTER_COUNT_ENUM() +*/ +#define COUNTER_COUNT_ENUM_AVAILABLE (_name, _e) \ +{ \ + .name = (_name "_available"), \ + .read = counter_count_enum_available_read, \ + .priv = (_e) \ +} + +/** +* struct counter_device_attr_group - internal container for attribute group +* @attr_group: Counter sysfs attributes group +* @attr_list: list to keep track of created Counter sysfs attributes +* @num_attr: number of Counter sysfs attributes +*/ +struct counter_device_attr_group {
include/linux/counter.h line 319
Quoted 10 lines
include/linux/counter.h new file +10
-0+struct attribute_group attr_group; +struct list_head attr_list; +size_t num_attr; +}; + +/** +* struct counter_device_state - internal state container for a Counter device +* @id: unique ID used to identify the Counter +* @dev: internal device structure +* @groups_list attribute groups list (groups for Signals, Counts, and ext)
Run kernel-doc script over these files. You are missing some :s and it would have told you that.
Quoted 36 lines
include/linux/counter.h new file +36
-0+* @num_groups number of attribute groups containers +* @groups: Counter sysfs attribute groups (used to populate @dev.groups) +*/ +struct counter_device_state { +int id; +struct device dev; +struct counter_device_attr_group *groups_list; +size_t num_groups; +const struct attribute_group **groups; +}; + +/** +* struct signal_read_value - Opaque Signal read value +* @buf: string representation of Signal read value +* @len: length of string in @buf +*/ +struct signal_read_value { +char *buf; +size_t len; +}; + +/** +* struct count_read_value - Opaque Count read value +* @buf: string representation of Count read value +* @len: length of string in @buf +*/ +struct count_read_value { +char *buf; +size_t len; +}; + +/** +* struct count_write_value - Opaque Count write value +* @buf: string representation of Count write value +*/ +struct count_write_value {
include/linux/counter.h line 365
Quoted 10 lines
include/linux/counter.h new file +10
-0+const char *buf; +}; + +/** +* struct counter_ops - Callbacks from driver +* @signal_read: optional read callback for Signal attribute. The read +* value of the respective Signal should be passed back via +* the val parameter. val points to an opaque type which +* should be set only via the set_signal_read_value +* function.
This last part had me a little confused. I would make it clear that this set_signal_read_value function should be called to set the value within this signal_read callback rather than elsewhere...
Quoted 170 lines
include/linux/counter.h new file +170
-0+* @count_read: optional read callback for Count attribute. The read +* value of the respective Count should be passed back via +* the val parameter. val points to an opaque type which +* should be set only via the set_count_read_value +* function. +* @count_write: optional write callback for Count attribute. The write +* value for the respective Count is passed in via the val +* parameter. val points to an opaque type which should be +* access only via the get_count_write_value function. +* @function_get: function to get the current count function mode. Returns +* 0 on success and negative error code on error. The index +* of the respective Count's returned function mode should +* be passed back via the function parameter. +* @function_set: function to set the count function mode. function is the +* index of the requested function mode from the respective +* Count's functions_list array. +* @action_get: function to get the current action mode. Returns 0 on +* success and negative error code on error. The index of +* the respective Signal's returned action mode should be +* passed back via the action parameter. +* @action_set: function to set the action mode. action is the index of +* the requested action mode from the respective Synapse's +* actions_list array. +*/ +struct counter_ops { +int (*signal_read)(struct counter_device *counter, +struct counter_signal *signal, +struct signal_read_value *val); +int (*count_read)(struct counter_device *counter, +struct counter_count *count, +struct count_read_value *val); +int (*count_write)(struct counter_device *counter, +struct counter_count *count, +struct count_write_value *val); +int (*function_get)(struct counter_device *counter, +struct counter_count *count,size_t *function); +int (*function_set)(struct counter_device *counter, +struct counter_count *count,size_t function); +int (*action_get)(struct counter_device *counter, +struct counter_count *count, +struct counter_synapse *synapse,size_t *action); +int (*action_set)(struct counter_device *counter, +struct counter_count *count, +struct counter_synapse *synapse,size_t action); +}; + +/** +* struct counter_device_ext - Counter device extension +* @name: attribute name +* @read: read callback for this attribute; may be NULL +* @write: write callback for this attribute; may be NULL +* @priv: data private to the driver +*/ +struct counter_device_ext { +const char *name; +ssize_t (*read)(struct counter_device *counter,void *priv, +char *buf); +ssize_t (*write)(struct counter_device *counter,void *priv, +const char *buf,size_t len); +void *priv; +}; + +/** +* struct counter_device_enum_ext - Counter enum extension attribute +* @items: Array of strings +* @num_items: Number of items specified in @items +* @set: Set callback function; may be NULL +* @get: Get callback function; may be NULL +* +* The counter_device_enum_ext structure can be used to implement enum style +* Counter extension attributes. Enum style attributes are those which have a +* set of strings that map to unsigned integer values. The Generic Counter enum +* extension helper code takes care of mapping between value and string, as well +* as generating a "_available" file which contains a list of all available +* items. The get callback is used to query the currently active item; the index +* of the item within the respective items array is returned via the 'item' +* parameter. The set callback is called when the attribute is updated; the +* 'item' parameter contains the index of the newly activated item within the +* respective items array. +*/ +struct counter_device_enum_ext { +const char *const *items; +size_t num_items; +int (*get)(struct counter_device *counter, +size_t *item); +int (*set)(struct counter_device *counter, +size_t item); +}; + +ssize_t counter_device_enum_read (struct counter_device *counter,void *priv, +char *buf); +ssize_t counter_device_enum_write (struct counter_device *counter,void *priv, +const char *buf,size_t len); + +/** +* COUNTER_DEVICE_ENUM() - Initialize Counter enum extension +* @_name: Attribute name +* @_e: Pointer to a counter_device_enum structure +* +* This should usually be used together with COUNTER_DEVICE_ENUM_AVAILABLE() +*/ +#define COUNTER_DEVICE_ENUM (_name, _e) \ +{ \ + .name = (_name), \ + .read = counter_device_enum_read, \ + .write = counter_device_enum_write, \ + .priv = (_e) \ +} + +ssize_t counter_device_enum_available_read (struct counter_device *counter, +void *priv,char *buf); + +/** +* COUNTER_DEVICE_ENUM_AVAILABLE() - Initialize Counter enum available extension +* @_name: Attribute name ("_available" will be appended to the name) +* @_e: Pointer to a counter_device_enum structure +* +* Creates a read only attribute that lists all the available enum items in a +* newline separated list. This should usually be used together with +* COUNTER_DEVICE_ENUM() +*/ +#define COUNTER_DEVICE_ENUM_AVAILABLE (_name, _e) \ +{ \ + .name = (_name "_available"), \ + .read = counter_device_enum_available_read, \ + .priv = (_e) \ +} + +/** +* struct counter_device - Counter data structure +* @name: name of the device as it appears in the datasheet +* @parent: optional parent device providing the counters +* @device_state: internal device state container +* @ops: callbacks from driver +* @signals: array of Signals +* @num_signals: number of Signals specified in @signals +* @counts: array of Counts +* @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 +*/ +struct counter_device { +const char *name; +struct device *parent; +struct counter_device_state *device_state; + +const struct counter_ops *ops; + +struct counter_signal *signals; +size_t num_signals; +struct counter_count *counts; +size_t num_counts; + +const struct counter_device_ext *ext; +size_t num_ext; + +void *priv; +}; + +enum signal_level { +SIGNAL_LEVEL_LOW =0 , +SIGNAL_LEVEL_HIGH +}; + +enum signal_value_type { +SIGNAL_LEVEL =0 +}; + +enum count_value_type {
include/linux/counter.h line 545
Quoted 10 lines
include/linux/counter.h new file +10
-0+COUNT_POSITION_UNSIGNED =0 , + COUNT_POSITION_SIGNED +}; + +void set_signal_read_value (struct signal_read_value *const val, +const enum signal_value_type type,void *const data); +void set_count_read_value (struct count_read_value *const val, +const enum count_value_type type,void *const data); +int get_count_write_value (void *const data,const enum count_value_type type, +const struct count_write_value *const val);
I wonder if naming wise, we would be better sticking to the noun_verb naming format.
signal_read_value_setcount_read_value_setcount_write_value_get
for example?
Quoted 9 lines
include/linux/counter.h new file +9
-0+ +int counter_register (struct counter_device *const counter); +void counter_unregister (struct counter_device *const counter); +int devm_counter_register (struct device *dev, +struct counter_device *const counter); +void devm_counter_unregister (struct device *dev, +struct counter_device *const counter); + +#endif /* _COUNTER_H_ */
Re: [PATCH v6 1/9] counter: Introduce the Generic Counter interface
Quoted 38 lines · Jonathan Cameron
Quoted 26 lines · On Wed, 16 May 2018 13:50:43 -0400 William Breathitt Gray…
This patch introduces the Generic Counter interface for supporting counter devices.
In the context of the Generic Counter interface, a counter is defined as a device that reports one or more "counts" based on the state changes of one or more "signals" as evaluated by a defined "count function."
Driver callbacks should be provided to communicate with the device: to read and write various Signals and Counts, and to set and get the "action mode" and "count function" for various Synapses and Counts respectively.
To support a counter device, a driver must first allocate the available Counter Signals via
counter_signalstructures. These Signals should be stored as an array and set to the signals array member of an allocatedcounter_devicestructure before the Counter is registered to the system.Counter Counts may be allocated via
counter_countstructures, and respective Counter Signal associations (Synapses) made viacounter_synapsestructures. Associatedcounter_synapsestructures are stored as an array and set to the the synapses array member of the respectivecounter_countstructure. Thesecounter_countstructures are set to the counts array member of an allocatedcounter_devicestructure before the Counter is registered to the system.Quoted 8 lines
A counter device is registered to the system by passing the respective initialized
counter_devicestructure to thecounter_registerfunction; similarly, thecounter_unregisterfunction unregisters the respective Counter. Thedevm_counter_registeranddevm_counter_unregisterfunctions serve as device memory-managed versions of thecounter_registerandcounter_unregisterfunctions respectively.Signed-off-by William Breathitt Gray <[email protected]>
Quoted 8 lines
A few minor comments inline. I do somewhat wonder if we can cut back on the huge amount of 'similar' code in here, but there tend to be just enough small differences to make that really tricky...
Nothing major enough in here that I really plan on reading it again (though you never know if you change lots ;)
Reviewed-by Jonathan Cameron <[email protected]>
These look like quick tidy-up suggestions so I expect to incorporate them in the next revision without trouble. I'll also look around to organize the repetitive code into functions for reuse where possible, in order to help clean up the code and make the logic clearer. In short though, I don't expect any major changes in the next revision, just minor formatting and cleanup.
William Breathitt Gray
Quoted 2,295 lines
Quoted 37 lines
--- MAINTAINERS | 7 + drivers/Kconfig | 2 + drivers/Makefile | 1 + drivers/counter/Kconfig | 18 + drivers/counter/Makefile | 8 + drivers/counter/generic-counter.c | 1541 +++++++++++++++++++++++++++++ include/linux/counter.h | 554 +++++++++++ 7 files changed, 2131 insertions(+) create mode 100644 drivers/counter/Kconfig create mode 100644 drivers/counter/Makefile create mode 100644 drivers/counter/generic-counter.c create mode 100644 include/linux/counter.hMAINTAINERS +7
-0@@ -3669,6 +3669,13 @@ W: http://www.fi.muni.cz/~kas/cosa/ S: Maintained F: drivers/net/wan/cosa* +COUNTER SUBSYSTEM +M: William Breathitt Gray <[email protected]> +L: linux-iio at vger.kernel.org +S: Maintained +F: drivers/counter/ +F: include/linux/counter.h + CPMAC ETHERNET DRIVER M: Florian Fainelli <[email protected]> L: netdev at vger.kernel.orgdrivers/Kconfig +0
-0drivers/Kconfig source "drivers/memory/Kconfig" · line 169
Quoted 5 lines
drivers/Kconfig +2
-0@@ -165,6 +165,8 @@ source "drivers/memory/Kconfig" source "drivers/iio/Kconfig" +source "drivers/counter/Kconfig" +Same comment as below.
drivers/Makefile obj-$(CONFIG_PM_DEVFREQ) += devfreq/ · line 168
Quoted 12 lines
drivers/Kconfig +0
-0source "drivers/ntb/Kconfig" source "drivers/vme/Kconfig"drivers/Makefile +1
-0@@ -165,6 +165,7 @@ obj-$(CONFIG_PM_DEVFREQ) += devfreq/ obj-$(CONFIG_EXTCON) += extcon/ obj-$(CONFIG_MEMORY) += memory/ obj-$(CONFIG_IIO) += iio/ +obj-$(CONFIG_COUNTER) += counter/I can see your logic in putting this here, but I think the convention is to go at the end rather than grouping.
Quoted 50 lines
drivers/Makefile +0
-0obj-$(CONFIG_VME_BUS) += vme/ obj-$(CONFIG_IPACK_BUS) += ipack/ obj-$(CONFIG_NTB) += ntb/drivers/counter/Kconfig new file +18
-0@@ -0,0 +1,18 @@ +# +# Counter devices +# +# When adding new entries keep the list in alphabetical order + +menuconfig COUNTER + tristate "Counter support" + help + Provides Generic Counter interface support for counter devices. + + Counter devices are prevalent within a diverse spectrum of industries. + The ubiquitous presence of these devices necessitates a common + interface and standard of interaction and exposure. This driver API + attempts to resolve the issue of duplicate code found among existing + counter device drivers by providing a generic counter interface for + consumption. The Generic Counter interface enables drivers to support + and expose a common set of components and functionality present in + counter devices.drivers/counter/Makefile new file +8
-0@@ -0,0 +1,8 @@ +# +# Makefile for Counter devices +# + +# When adding new entries keep the list in alphabetical order + +obj-$(CONFIG_COUNTER) += counter.o +counter-y := generic-counter.odrivers/counter/generic-counter.c new file +3
-0@@ -0,0 +1,1541 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* +* Generic Counter interface drivers/counter/generic-counter.c line 13
Quoted 10 lines
drivers/counter/generic-counter.c new file +10
-0+* Copyright (C) 2017 William Breathitt Gray +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License, version 2, as +* published by the Free Software Foundation. +* +* This program is distributed in the hope that it will be useful, but +* WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +* General Public License for more details. As below, SPDX and license seems silly. Unless you are feeling paranoid just drop the license text.
Quoted 661 lines
drivers/counter/generic-counter.c new file +661
-0+*/ +#include <linux/device.h> +#include <linux/err.h> +#include <linux/export.h> +#include <linux/fs.h> +#include <linux/gfp.h> +#include <linux/idr.h> +#include <linux/init.h> +#include <linux/kernel.h> +#include <linux/list.h> +#include <linux/module.h> +#include <linux/printk.h> +#include <linux/slab.h> +#include <linux/string.h> +#include <linux/sysfs.h> +#include <linux/types.h> + +#include <linux/counter.h> + +const char *const count_direction_str[2 ] = { + [COUNT_DIRECTION_FORWARD ] ="forward" , + [COUNT_DIRECTION_BACKWARD ] ="backward" +}; +EXPORT_SYMBOL (count_direction_str); + +const char *const count_mode_str[4 ] = { + [COUNT_MODE_NORMAL ] ="normal" , + [COUNT_MODE_RANGE_LIMIT ] ="range limit" , + [COUNT_MODE_NON_RECYCLE ] ="non-recycle" , + [COUNT_MODE_MODULO_N ] ="modulo-n" +}; +EXPORT_SYMBOL (count_mode_str); + +ssize_t counter_signal_enum_read (struct counter_device *counter, +struct counter_signal *signal,void *priv, +char *buf) +{ +const struct counter_signal_enum_ext *const e = priv; +int err; +size_t index; + +if (!e->get) +return -EINVAL ; + + err = e->get (counter, signal, &index); +if (err) +return err; + +if (index >= e->num_items) +return -EINVAL ; + +return scnprintf (buf,PAGE_SIZE ,"%s\n" , e->items[index]); +} +EXPORT_SYMBOL (counter_signal_enum_read); + +ssize_t counter_signal_enum_write (struct counter_device *counter, +struct counter_signal *signal,void *priv, +const char *buf,size_t len) +{ +const struct counter_signal_enum_ext *const e = priv; +ssize_t index; +int err; + +if (!e->set) +return -EINVAL ; + + index =__sysfs_match_string (e->items, e->num_items, buf); +if (index <0 ) +return index; + + err = e->set (counter, signal, index); +if (err) +return err; + +return len; +} +EXPORT_SYMBOL (counter_signal_enum_write); + +ssize_t counter_signal_enum_available_read (struct counter_device *counter, +struct counter_signal *signal, +void *priv,char *buf) +{ +const struct counter_signal_enum_ext *const e = priv; +size_t i; +size_t len =0 ; + +if (!e->num_items) +return 0 ; + +for (i =0 ; i < e->num_items; i++) + len +=scnprintf (buf + len,PAGE_SIZE - len,"%s\n" , + e->items[i]); + +return len; +} +EXPORT_SYMBOL (counter_signal_enum_available_read); + +ssize_t counter_count_enum_read (struct counter_device *counter, +struct counter_count *count,void *priv, +char *buf) +{ +const struct counter_count_enum_ext *const e = priv; +int err; +size_t index; + +if (!e->get) +return -EINVAL ; + + err = e->get (counter, count, &index); +if (err) +return err; + +if (index >= e->num_items) +return -EINVAL ; + +return scnprintf (buf,PAGE_SIZE ,"%s\n" , e->items[index]); +} +EXPORT_SYMBOL (counter_count_enum_read); + +ssize_t counter_count_enum_write (struct counter_device *counter, +struct counter_count *count,void *priv, +const char *buf,size_t len) +{ +const struct counter_count_enum_ext *const e = priv; +ssize_t index; +int err; + +if (!e->set) +return -EINVAL ; + + index =__sysfs_match_string (e->items, e->num_items, buf); +if (index <0 ) +return index; + + err = e->set (counter, count, index); +if (err) +return err; + +return len; +} +EXPORT_SYMBOL (counter_count_enum_write); + +ssize_t counter_count_enum_available_read (struct counter_device *counter, +struct counter_count *count, +void *priv,char *buf) +{ +const struct counter_count_enum_ext *const e = priv; +size_t i; +size_t len =0 ; + +if (!e->num_items) +return 0 ; + +for (i =0 ; i < e->num_items; i++) + len +=scnprintf (buf + len,PAGE_SIZE - len,"%s\n" , + e->items[i]); + +return len; +} +EXPORT_SYMBOL (counter_count_enum_available_read); + +ssize_t counter_device_enum_read (struct counter_device *counter,void *priv, +char *buf) +{ +const struct counter_device_enum_ext *const e = priv; +int err; +size_t index; + +if (!e->get) +return -EINVAL ; + + err = e->get (counter, &index); +if (err) +return err; + +if (index >= e->num_items) +return -EINVAL ; + +return scnprintf (buf,PAGE_SIZE ,"%s\n" , e->items[index]); +} +EXPORT_SYMBOL (counter_device_enum_read); + +ssize_t counter_device_enum_write (struct counter_device *counter,void *priv, +const char *buf,size_t len) +{ +const struct counter_device_enum_ext *const e = priv; +ssize_t index; +int err; + +if (!e->set) +return -EINVAL ; + + index =__sysfs_match_string (e->items, e->num_items, buf); +if (index <0 ) +return index; + + err = e->set (counter, index); +if (err) +return err; + +return len; +} +EXPORT_SYMBOL (counter_device_enum_write); + +ssize_t counter_device_enum_available_read (struct counter_device *counter, +void *priv,char *buf) +{ +const struct counter_device_enum_ext *const e = priv; +size_t i; +size_t len =0 ; + +if (!e->num_items) +return 0 ; + +for (i =0 ; i < e->num_items; i++) + len +=scnprintf (buf + len,PAGE_SIZE - len,"%s\n" , + e->items[i]); + +return len; +} +EXPORT_SYMBOL (counter_device_enum_available_read); + +static const char *const signal_level_str[] = { + [SIGNAL_LEVEL_LOW ] ="low" , + [SIGNAL_LEVEL_HIGH ] ="high" +}; + +/** +* set_signal_read_value - set signal_read_value data +* @val: signal_read_value structure to set +* @type: property Signal data represents +* @data: Signal data +* +* This function sets an opaque signal_read_value structure with the provided +* Signal data. +*/ +void set_signal_read_value (struct signal_read_value *const val, +const enum signal_value_type type,void *const data) +{ +if (type ==SIGNAL_LEVEL ) + val->len =scnprintf (val->buf,PAGE_SIZE ,"%s\n" , + signal_level_str[*(enum signal_level *)data]); +else + val->len =0 ; +} +EXPORT_SYMBOL (set_signal_read_value); + +/** +* set_count_read_value - set count_read_value data +* @val: count_read_value structure to set +* @type: property Count data represents +* @data: Count data +* +* This function sets an opaque count_read_value structure with the provided +* Count data. +*/ +void set_count_read_value (struct count_read_value *const val, +const enum count_value_type type,void *const data) +{ +switch (type) { +case COUNT_POSITION_UNSIGNED : + val->len =scnprintf (val->buf,PAGE_SIZE ,"%lu\n" , + *(unsigned long *)data); +break ; +case COUNT_POSITION_SIGNED : + val->len =scnprintf (val->buf,PAGE_SIZE ,"%ld\n" , + *(long *)data); +break ; +default : + val->len =0 ; + } +} +EXPORT_SYMBOL (set_count_read_value); + +/** +* get_count_write_value - get count_write_value data +* @data: Count data +* @type: property Count data represents +* @val: count_write_value structure containing data +* +* This function extracts Count data from the provided opaque count_write_value +* structure and stores it at the address provided by @data. +* +* RETURNS: +* 0 on success, negative error number on failure. +*/ +int get_count_write_value (void *const data,const enum count_value_type type, +const struct count_write_value *const val) +{ +int err; + +switch (type) { +case COUNT_POSITION_UNSIGNED : + err =kstrtoul (val->buf,0 , data); +if (err) +return err; +break ; +case COUNT_POSITION_SIGNED : + err =kstrtol (val->buf,0 , data); +if (err) +return err; +break ; + } + +return 0 ; +} +EXPORT_SYMBOL (get_count_write_value); + +struct counter_device_attr { +struct device_attribute dev_attr; +struct list_head l; +void *component; +}; + +static int counter_attribute_create ( +struct counter_device_attr_group *const group, +const char *const prefix, +const char *const name, +ssize_t (*show)(struct device *dev,struct device_attribute *attr, +char *buf), +ssize_t (*store)(struct device *dev,struct device_attribute *attr, +const char *buf,size_t len), +void *const component) +{ +struct counter_device_attr *counter_attr; +struct device_attribute *dev_attr; +int err; +struct list_head *const attr_list = &group->attr_list; + +/* Allocate a Counter device attribute */ + counter_attr =kzalloc (sizeof (*counter_attr),GFP_KERNEL ); +if (!counter_attr) +return -ENOMEM ; + dev_attr = &counter_attr->dev_attr; + +sysfs_attr_init (&dev_attr->attr); + +/* Configure device attribute */ + dev_attr->attr.name =kasprintf (GFP_KERNEL ,"%s%s" , prefix, name); +if (!dev_attr->attr.name) { + err = -ENOMEM ; + goto err_free_counter_attr; + } +if (show) { + dev_attr->attr.mode |=0444 ; + dev_attr->show = show; + } +if (store) { + dev_attr->attr.mode |=0200 ; + dev_attr->store = store; + } + +/* Store associated Counter component with attribute */ + counter_attr->component = component; + +/* Keep track of the attribute for later cleanup */ +list_add (&counter_attr->l, attr_list); + group->num_attr++; + +return 0 ; + +err_free_counter_attr: +kfree (counter_attr); +return err; +} + +#define to_counter_attr (_dev_attr) \ + container_of(_dev_attr, struct counter_device_attr, dev_attr) + +struct signal_comp_t { +struct counter_signal *signal; +}; + +static ssize_t counter_signal_show (struct device *dev, +struct device_attribute *attr,char *buf) +{ +struct counter_device *const counter =dev_get_drvdata (dev); +const struct counter_device_attr *const devattr =to_counter_attr (attr); +const struct signal_comp_t *const component = devattr->component; +struct counter_signal *const signal = component->signal; +int err; +struct signal_read_value val = { .buf = buf }; + + err = counter->ops->signal_read (counter, signal, &val); +if (err) +return err; + +return val.len; +} + +struct name_comp_t { +const char *name; +}; + +static ssize_t counter_device_attr_name_show (struct device *dev, +struct device_attribute *attr, +char *buf) +{ +const struct name_comp_t *const comp =to_counter_attr (attr)->component; + +return scnprintf (buf,PAGE_SIZE ,"%s\n" , comp->name); +} + +static int counter_name_attribute_create ( +struct counter_device_attr_group *const group, +const char *const name) +{ +struct name_comp_t *name_comp; +int err; + +/* Skip if no name */ +if (!name) +return 0 ; + +/* Allocate name attribute component */ + name_comp =kmalloc (sizeof (*name_comp),GFP_KERNEL ); +if (!name_comp) +return -ENOMEM ; + name_comp->name = name; + +/* Allocate Signal name attribute */ + err =counter_attribute_create (group,"" ,"name" , + counter_device_attr_name_show,NULL , + name_comp); +if (err) + goto err_free_name_comp; + +return 0 ; + +err_free_name_comp: +kfree (name_comp); +return err; +} + +struct signal_ext_comp_t { +struct counter_signal *signal; +const struct counter_signal_ext *ext; +}; + +static ssize_t counter_signal_ext_show (struct device *dev, +struct device_attribute *attr,char *buf) +{ +const struct counter_device_attr *const devattr =to_counter_attr (attr); +const struct signal_ext_comp_t *const comp = devattr->component; +const struct counter_signal_ext *const ext = comp->ext; + +return ext->read (dev_get_drvdata (dev), comp->signal, ext->priv, buf); +} + +static ssize_t counter_signal_ext_store (struct device *dev, +struct device_attribute *attr, +const char *buf,size_t len) +{ +const struct counter_device_attr *const devattr =to_counter_attr (attr); +const struct signal_ext_comp_t *const comp = devattr->component; +const struct counter_signal_ext *const ext = comp->ext; + +return ext->write (dev_get_drvdata (dev), comp->signal, ext->priv, buf, + len); +} + +static void free_counter_device_attr_list (struct list_head *attr_list) +{ +struct counter_device_attr *p, *n; + +list_for_each_entry_safe (p, n, attr_list, l) { +kfree (p->dev_attr.attr.name); +kfree (p->component); +list_del (&p->l); +kfree (p); + } +} + +static int counter_signal_ext_register ( +struct counter_device_attr_group *const group, +struct counter_signal *const signal) +{ +const size_t num_ext = signal->num_ext; +size_t i; +const struct counter_signal_ext *ext; +struct signal_ext_comp_t *signal_ext_comp; +int err; + +/* Create an attribute for each extension */ +for (i =0 ; i < num_ext; i++) { + ext = signal->ext + i; + +/* Allocate signal_ext attribute component */ + signal_ext_comp =kmalloc (sizeof (*signal_ext_comp),GFP_KERNEL ); +if (!signal_ext_comp) { + err = -ENOMEM ; + goto err_free_attr_list; + } + signal_ext_comp->signal = signal; + signal_ext_comp->ext = ext; + +/* Allocate a Counter device attribute */ + err =counter_attribute_create (group,"" , ext->name, + (ext->read) ? counter_signal_ext_show :NULL , + (ext->write) ? counter_signal_ext_store :NULL , + signal_ext_comp); +if (err) { +kfree (signal_ext_comp); + goto err_free_attr_list; + } + } + +return 0 ; + +err_free_attr_list: +free_counter_device_attr_list (&group->attr_list); +return err; +} + +static int counter_signal_attributes_create ( +struct counter_device_attr_group *const group, +const struct counter_device *const counter, +struct counter_signal *const signal) +{ +struct signal_comp_t *signal_comp; +int err; + +/* Allocate Signal attribute component */ + signal_comp =kmalloc (sizeof (*signal_comp),GFP_KERNEL ); +if (!signal_comp) +return -ENOMEM ; + signal_comp->signal = signal; + +/* Create main Signal attribute */ + err =counter_attribute_create (group,"" ,"signal" , + (counter->ops->signal_read) ? counter_signal_show :NULL ,NULL , + signal_comp); +if (err) { +kfree (signal_comp); +return err; + } + +/* Create Signal name attribute */ + err =counter_name_attribute_create (group, signal->name); +if (err) + goto err_free_attr_list; + +/* Register Signal extension attributes */ + err =counter_signal_ext_register (group, signal); +if (err) + goto err_free_attr_list; + +return 0 ; + +err_free_attr_list: +free_counter_device_attr_list (&group->attr_list); +return err; +} + +static int counter_signals_register ( +struct counter_device_attr_group *const groups_list, +const struct counter_device *const counter) +{ +const size_t num_signals = counter->num_signals; +size_t i; +struct counter_signal *signal; +const char *name; +int err; + +/* Register each Signal */ +for (i =0 ; i < num_signals; i++) { + signal = counter->signals + i; + +/* Generate Signal attribute directory name */ + name =kasprintf (GFP_KERNEL ,"signal%d" , signal->id); +if (!name) { + err = -ENOMEM ; + goto err_free_attr_groups; + } + groups_list[i].attr_group.name = name; + +/* Create all attributes associated with Signal */ + err =counter_signal_attributes_create (groups_list + i, counter, + signal); +if (err) + goto err_free_attr_groups; + } + +return 0 ; + +err_free_attr_groups: +do { +kfree (groups_list[i].attr_group.name); +free_counter_device_attr_list (&groups_list[i].attr_list); + }while (i--); +return err; +} + +static const char *const synapse_action_str[] = { + [SYNAPSE_ACTION_NONE ] ="none" , + [SYNAPSE_ACTION_RISING_EDGE ] ="rising edge" , + [SYNAPSE_ACTION_FALLING_EDGE ] ="falling edge" , + [SYNAPSE_ACTION_BOTH_EDGES ] ="both edges" +}; + +struct action_comp_t { +struct counter_synapse *synapse; +struct counter_count *count; +}; + +static ssize_t counter_action_show (struct device *dev, +struct device_attribute *attr,char *buf) +{ +const struct counter_device_attr *const devattr =to_counter_attr (attr); +int err; +struct counter_device *const counter =dev_get_drvdata (dev); +const struct action_comp_t *const component = devattr->component; +struct counter_count *const count = component->count; +struct counter_synapse *const synapse = component->synapse; +size_t action_index; +enum synapse_action action; + + err = counter->ops->action_get (counter, count, synapse, &action_index); +if (err) +return err; + + synapse->action = action_index; + + action = synapse->actions_list[action_index]; +return scnprintf (buf,PAGE_SIZE ,"%s\n" , synapse_action_str[action]); +} + +static ssize_t counter_action_store (struct device *dev, +struct device_attribute *attr, +const char *buf,size_t len) +{ +const struct counter_device_attr *const devattr =to_counter_attr (attr); +const struct action_comp_t *const component = devattr->component; +struct counter_synapse *const synapse = component->synapse; +size_t action_index; +const size_t num_actions = synapse->num_actions; +enum synapse_action action; +int err; +struct counter_device *const counter =dev_get_drvdata (dev); +struct counter_count *const count = component->count; + +/* Find requested action mode */ +for (action_index =0 ; action_index < num_actions; action_index++) { + action = synapse->actions_list[action_index]; +if (sysfs_streq (buf, synapse_action_str[action])) +break ; + } +/* If requested action mode not found */ +if (action_index >= num_actions) +return -EINVAL ; + + err = counter->ops->action_set (counter, count, synapse, action_index); +if (err) +return err; + + synapse->action = action_index; + +return len; +} + +struct action_avail_comp_t {drivers/counter/generic-counter.c line 684
Quoted 10 lines
drivers/counter/generic-counter.c new file +10
-0+const enum synapse_action *actions_list; +size_t num_actions; +}; + +static ssize_t counter_synapse_action_available_show (struct device *dev, +struct device_attribute *attr,char *buf) +{ +const struct counter_device_attr *const devattr =to_counter_attr (attr); +const struct action_avail_comp_t *const component = devattr->component; +const enum synapse_action *const actions_list = component->actions_list;I'm not sure this local variable helps much either...
drivers/counter/generic-counter.c line 685
Quoted 1 line
drivers/counter/generic-counter.c new file +1
-0+const size_t num_actions = component->num_actions;Local variable adds nothing as far as I can see.. Just use it inline.
Quoted 9 lines
drivers/counter/generic-counter.c new file +9
-0+size_t i; +enum synapse_action action; +ssize_t len =0 ; + +for (i =0 ; i < num_actions; i++) { + action = actions_list[i]; + len +=scnprintf (buf + len,PAGE_SIZE - len,"%s\n" , + synapse_action_str[action]); + }drivers/counter/generic-counter.c line 704
Quoted 10 lines
drivers/counter/generic-counter.c new file +10
-0+ +return len; +} + +static int counter_synapses_register ( +struct counter_device_attr_group *const group, +const struct counter_device *const counter, +struct counter_count *const count,const char *const count_attr_name) +{ +const size_t num_synapses = count->num_synapses;Local variable doesn't add anything. Only used once.
Quoted 197 lines
drivers/counter/generic-counter.c new file +197
-0+size_t i; +struct counter_synapse *synapse; +const char *prefix; +struct action_comp_t *action_comp; +int err; +struct action_avail_comp_t *avail_comp; + +/* Register each Synapse */ +for (i =0 ; i < num_synapses; i++) { + synapse = count->synapses + i; + +/* Generate attribute prefix */ + prefix =kasprintf (GFP_KERNEL ,"signal%d_" , + synapse->signal->id); +if (!prefix) { + err = -ENOMEM ; + goto err_free_attr_list; + } + +/* Allocate action attribute component */ + action_comp =kmalloc (sizeof (*action_comp),GFP_KERNEL ); +if (!action_comp) { + err = -ENOMEM ; + goto err_free_prefix; + } + action_comp->synapse = synapse; + action_comp->count = count; + +/* Create action attribute */ + err =counter_attribute_create (group, prefix,"action" , + (counter->ops->action_get) ? counter_action_show :NULL , + (counter->ops->action_set) ? counter_action_store :NULL , + action_comp); +if (err) { +kfree (action_comp); + goto err_free_prefix; + } + +/* Allocate action available attribute component */ + avail_comp =kmalloc (sizeof (*avail_comp),GFP_KERNEL ); +if (!avail_comp) { + err = -ENOMEM ; + goto err_free_prefix; + } + avail_comp->actions_list = synapse->actions_list; + avail_comp->num_actions = synapse->num_actions; + +/* Create action_available attribute */ + err =counter_attribute_create (group, prefix, +"action_available" , + counter_synapse_action_available_show,NULL , + avail_comp); +if (err) { +kfree (avail_comp); + goto err_free_prefix; + } + +kfree (prefix); + } + +return 0 ; + +err_free_prefix: +kfree (prefix); +err_free_attr_list: +free_counter_device_attr_list (&group->attr_list); +return err; +} + +struct count_comp_t { +struct counter_count *count; +}; + +static ssize_t counter_count_show (struct device *dev, +struct device_attribute *attr, +char *buf) +{ +struct counter_device *const counter =dev_get_drvdata (dev); +const struct counter_device_attr *const devattr =to_counter_attr (attr); +const struct count_comp_t *const component = devattr->component; +struct counter_count *const count = component->count; +int err; +struct count_read_value val = { .buf = buf }; + + err = counter->ops->count_read (counter, count, &val); +if (err) +return err; + +return val.len; +} + +static ssize_t counter_count_store (struct device *dev, +struct device_attribute *attr, +const char *buf,size_t len) +{ +struct counter_device *const counter =dev_get_drvdata (dev); +const struct counter_device_attr *const devattr =to_counter_attr (attr); +const struct count_comp_t *const component = devattr->component; +struct counter_count *const count = component->count; +int err; +struct count_write_value val = { .buf = buf }; + + err = counter->ops->count_write (counter, count, &val); +if (err) +return err; + +return len; +} + +static const char *const count_function_str[] = { + [COUNT_FUNCTION_INCREASE ] ="increase" , + [COUNT_FUNCTION_DECREASE ] ="decrease" , + [COUNT_FUNCTION_PULSE_DIRECTION ] ="pulse-direction" , + [COUNT_FUNCTION_QUADRATURE_X1_A ] ="quadrature x1 a" , + [COUNT_FUNCTION_QUADRATURE_X1_B ] ="quadrature x1 b" , + [COUNT_FUNCTION_QUADRATURE_X2_A ] ="quadrature x2 a" , + [COUNT_FUNCTION_QUADRATURE_X2_B ] ="quadrature x2 b" , + [COUNT_FUNCTION_QUADRATURE_X2_RISING ] ="quadrature x2 rising" , + [COUNT_FUNCTION_QUADRATURE_X2_FALLING ] ="quadrature x2 falling" , + [COUNT_FUNCTION_QUADRATURE_X4 ] ="quadrature x4" +}; + +static ssize_t counter_function_show (struct device *dev, +struct device_attribute *attr,char *buf) +{ +int err; +struct counter_device *const counter =dev_get_drvdata (dev); +const struct counter_device_attr *const devattr =to_counter_attr (attr); +const struct count_comp_t *const component = devattr->component; +struct counter_count *const count = component->count; +size_t func_index; +enum count_function function; + + err = counter->ops->function_get (counter, count, &func_index); +if (err) +return err; + + count->function = func_index; + + function = count->functions_list[func_index]; +return scnprintf (buf,PAGE_SIZE ,"%s\n" , count_function_str[function]); +} + +static ssize_t counter_function_store (struct device *dev, +struct device_attribute *attr, +const char *buf,size_t len) +{ +const struct counter_device_attr *const devattr =to_counter_attr (attr); +const struct count_comp_t *const component = devattr->component; +struct counter_count *const count = component->count; +const size_t num_functions = count->num_functions; +size_t func_index; +enum count_function function; +int err; +struct counter_device *const counter =dev_get_drvdata (dev); + +/* Find requested Count function mode */ +for (func_index =0 ; func_index < num_functions; func_index++) { + function = count->functions_list[func_index]; +if (sysfs_streq (buf, count_function_str[function])) +break ; + } +/* Return error if requested Count function mode not found */ +if (func_index >= num_functions) +return -EINVAL ; + + err = counter->ops->function_set (counter, count, func_index); +if (err) +return err; + + count->function = func_index; + +return len; +} + +struct count_ext_comp_t { +struct counter_count *count; +const struct counter_count_ext *ext; +}; + +static ssize_t counter_count_ext_show (struct device *dev, +struct device_attribute *attr,char *buf) +{ +const struct counter_device_attr *const devattr =to_counter_attr (attr); +const struct count_ext_comp_t *const comp = devattr->component; +const struct counter_count_ext *const ext = comp->ext; + +return ext->read (dev_get_drvdata (dev), comp->count, ext->priv, buf); +} + +static ssize_t counter_count_ext_store (struct device *dev, +struct device_attribute *attr, +const char *buf,size_t len) +{ +const struct counter_device_attr *const devattr =to_counter_attr (attr); +const struct count_ext_comp_t *const comp = devattr->component; +const struct counter_count_ext *const ext = comp->ext;drivers/counter/generic-counter.c line 911
Quoted 10 lines
drivers/counter/generic-counter.c new file +10
-0+ +return ext->write (dev_get_drvdata (dev), comp->count, ext->priv, buf, + len); +} + +static int counter_count_ext_register ( +struct counter_device_attr_group *const group, +struct counter_count *const count) +{ +const size_t num_ext = count->num_ext;Used in one place, just put it inline?
Quoted 135 lines
drivers/counter/generic-counter.c new file +135
-0+size_t i; +const struct counter_count_ext *ext; +struct count_ext_comp_t *count_ext_comp; +int err; + +/* Create an attribute for each extension */ +for (i =0 ; i < num_ext; i++) { + ext = count->ext + i; + +/* Allocate count_ext attribute component */ + count_ext_comp =kmalloc (sizeof (*count_ext_comp),GFP_KERNEL ); +if (!count_ext_comp) { + err = -ENOMEM ; + goto err_free_attr_list; + } + count_ext_comp->count = count; + count_ext_comp->ext = ext; + +/* Allocate count_ext attribute */ + err =counter_attribute_create (group,"" , ext->name, + (ext->read) ? counter_count_ext_show :NULL , + (ext->write) ? counter_count_ext_store :NULL , + count_ext_comp); +if (err) { +kfree (count_ext_comp); + goto err_free_attr_list; + } + } + +return 0 ; + +err_free_attr_list: +free_counter_device_attr_list (&group->attr_list); +return err; +} + +struct func_avail_comp_t { +const enum count_function *functions_list; +size_t num_functions; +}; + +static ssize_t counter_count_function_available_show (struct device *dev, +struct device_attribute *attr,char *buf) +{ +const struct counter_device_attr *const devattr =to_counter_attr (attr); +const struct func_avail_comp_t *const component = devattr->component; +const enum count_function *const func_list = component->functions_list; +const size_t num_functions = component->num_functions; +size_t i; +enum count_function function; +ssize_t len =0 ; + +for (i =0 ; i < num_functions; i++) { + function = func_list[i]; + len +=scnprintf (buf + len,PAGE_SIZE - len,"%s\n" , + count_function_str[function]); + } + +return len; +} + +static int counter_count_attributes_create ( +struct counter_device_attr_group *const group, +const struct counter_device *const counter, +struct counter_count *const count) +{ +struct count_comp_t *count_comp; +int err; +struct count_comp_t *func_comp; +struct func_avail_comp_t *avail_comp; + +/* Allocate count attribute component */ + count_comp =kmalloc (sizeof (*count_comp),GFP_KERNEL ); +if (!count_comp) +return -ENOMEM ; + count_comp->count = count; + +/* Create main Count attribute */ + err =counter_attribute_create (group,"" ,"count" , + (counter->ops->count_read) ? counter_count_show :NULL , + (counter->ops->count_write) ? counter_count_store :NULL , + count_comp); +if (err) { +kfree (count_comp); +return err; + } + +/* Allocate function attribute component */ + func_comp =kmalloc (sizeof (*func_comp),GFP_KERNEL ); +if (!func_comp) { + err = -ENOMEM ; + goto err_free_attr_list; + } + func_comp->count = count; + +/* Create Count function attribute */ + err =counter_attribute_create (group,"" ,"function" , + (counter->ops->function_get) ? counter_function_show :NULL , + (counter->ops->function_set) ? counter_function_store :NULL , + func_comp); +if (err) { +kfree (func_comp); + goto err_free_attr_list; + } + +/* Allocate function available attribute component */ + avail_comp =kmalloc (sizeof (*avail_comp),GFP_KERNEL ); +if (!avail_comp) { + err = -ENOMEM ; + goto err_free_attr_list; + } + avail_comp->functions_list = count->functions_list; + avail_comp->num_functions = count->num_functions; + +/* Create Count function_available attribute */ + err =counter_attribute_create (group,"" ,"function_available" , + counter_count_function_available_show, +NULL , avail_comp); +if (err) { +kfree (avail_comp); + goto err_free_attr_list; + } + +/* Create Count name attribute */ + err =counter_name_attribute_create (group, count->name); +if (err) + goto err_free_attr_list; + +/* Register Count extension attributes */ + err =counter_count_ext_register (group, count); +if (err) + goto err_free_attr_list; + +return 0 ; +drivers/counter/generic-counter.c line 1056
Quoted 10 lines
drivers/counter/generic-counter.c new file +10
-0+err_free_attr_list: +free_counter_device_attr_list (&group->attr_list); +return err; +} + +static int counter_counts_register ( +struct counter_device_attr_group *const groups_list, +const struct counter_device *const counter) +{ +const size_t num_counts = counter->num_counts;I think this is only used on one place. Not sure having a local variable is worthwhile.
Quoted 99 lines
drivers/counter/generic-counter.c new file +99
-0+size_t i; +struct counter_count *count; +const char *name; +int err; + +/* Register each Count */ +for (i =0 ; i < num_counts; i++) { + count = counter->counts + i; + +/* Generate Count attribute directory name */ + name =kasprintf (GFP_KERNEL ,"count%d" , count->id); +if (!name) { + err = -ENOMEM ; + goto err_free_attr_groups; + } + groups_list[i].attr_group.name = name; + +/* Register the Synapses associated with each Count */ + err =counter_synapses_register (groups_list + i, counter, count, + name); +if (err) + goto err_free_attr_groups; + +/* Create all attributes associated with Count */ + err =counter_count_attributes_create (groups_list + i, counter, + count); +if (err) + goto err_free_attr_groups; + } + +return 0 ; + +err_free_attr_groups: +do { +kfree (groups_list[i].attr_group.name); +free_counter_device_attr_list (&groups_list[i].attr_list); + }while (i--); +return err; +} + +struct size_comp_t { +size_t size; +}; + +static ssize_t counter_device_attr_size_show (struct device *dev, +struct device_attribute *attr, +char *buf) +{ +const struct size_comp_t *const comp =to_counter_attr (attr)->component; + +return scnprintf (buf,PAGE_SIZE ,"%zu\n" , comp->size); +} + +static int counter_size_attribute_create ( +struct counter_device_attr_group *const group, +const size_t size,const char *const name) +{ +struct size_comp_t *size_comp; +int err; + +/* Allocate size attribute component */ + size_comp =kmalloc (sizeof (*size_comp),GFP_KERNEL ); +if (!size_comp) +return -ENOMEM ; + size_comp->size = size; + + err =counter_attribute_create (group,"" , name, + counter_device_attr_size_show,NULL , + size_comp); +if (err) + goto err_free_size_comp; + +return 0 ; + +err_free_size_comp: +kfree (size_comp); +return err; +} + +struct ext_comp_t { +const struct counter_device_ext *ext; +}; + +static ssize_t counter_device_ext_show (struct device *dev, +struct device_attribute *attr,char *buf) +{ +const struct counter_device_attr *const devattr =to_counter_attr (attr); +const struct ext_comp_t *const component = devattr->component; +const struct counter_device_ext *const ext = component->ext; + +return ext->read (dev_get_drvdata (dev), ext->priv, buf); +} + +static ssize_t counter_device_ext_store (struct device *dev, +struct device_attribute *attr, +const char *buf,size_t len) +{ +const struct counter_device_attr *const devattr =to_counter_attr (attr); +const struct ext_comp_t *const component = devattr->component;drivers/counter/generic-counter.c line 1165
Quoted 10 lines
drivers/counter/generic-counter.c new file +10
-0+const struct counter_device_ext *const ext = component->ext; + +return ext->write (dev_get_drvdata (dev), ext->priv, buf, len); +} + +static int counter_device_ext_register ( +struct counter_device_attr_group *const group, +struct counter_device *const counter) +{ +const size_t num_ext = counter->num_ext;
num_extonly used in one place so if anything the local variable is hurting readability.drivers/counter/generic-counter.c line 1173
Quoted 8 lines
drivers/counter/generic-counter.c new file +8
-0+struct ext_comp_t *ext_comp; +size_t i; +const struct counter_device_ext *ext; +int err; + +/* Create an attribute for each extension */ +for (i =0 ; i < num_ext; i++) { + ext = counter->ext + i;This local variable isn't gaining us much that I can see. Just use the value directly.
Quoted 63 lines
drivers/counter/generic-counter.c new file +63
-0+ +/* Allocate extension attribute component */ + ext_comp =kmalloc (sizeof (*ext_comp),GFP_KERNEL ); +if (!ext_comp) { + err = -ENOMEM ; + goto err_free_attr_list; + } + + ext_comp->ext = ext; + +/* Allocate extension attribute */ + err =counter_attribute_create (group,"" , ext->name, + (ext->read) ? counter_device_ext_show :NULL , + (ext->write) ? counter_device_ext_store :NULL , + ext_comp); +if (err) { +kfree (ext_comp); + goto err_free_attr_list; + } + } + +return 0 ; + +err_free_attr_list: +free_counter_device_attr_list (&group->attr_list); +return err; +} + +static int counter_global_attr_register ( +struct counter_device_attr_group *const group, +struct counter_device *const counter) +{ +int err; + +/* Create name attribute */ + err =counter_name_attribute_create (group, counter->name); +if (err) +return err; + +/* Create num_counts attribute */ + err =counter_size_attribute_create (group, counter->num_counts, +"num_counts" ); +if (err) + goto err_free_attr_list; + +/* Create num_signals attribute */ + err =counter_size_attribute_create (group, counter->num_signals, +"num_signals" ); +if (err) + goto err_free_attr_list; + +/* Register Counter device extension attributes */ + err =counter_device_ext_register (group, counter); +if (err) + goto err_free_attr_list; + +return 0 ; + +err_free_attr_list: +free_counter_device_attr_list (&group->attr_list); +return err; +} +drivers/counter/generic-counter.c line 1246
Quoted 10 lines
drivers/counter/generic-counter.c new file +10
-0+static void free_counter_device_groups_list ( +struct counter_device_attr_group *const groups_list, +const size_t num_groups) +{ +struct counter_device_attr_group *group; +size_t i; + +for (i =0 ; i < num_groups; i++) { + group = groups_list + i; +I'd like to see a comment somewhere here on the fact this cleans up both the registered per counter stuff and the global attributes (that took a bit of chasing to check it did...)
Quoted 44 lines
drivers/counter/generic-counter.c new file +44
-0+kfree (group->attr_group.name); +kfree (group->attr_group.attrs); +free_counter_device_attr_list (&group->attr_list); + } + +kfree (groups_list); +} + +static int prepare_counter_device_groups_list ( +struct counter_device *const counter) +{ +const size_t total_num_groups = + counter->num_signals + counter->num_counts +1 ; +struct counter_device_attr_group *groups_list; +size_t i; +int err; +size_t num_groups =0 ; + +/* Allocate space for attribute groups (signals. counts, and ext) */ + groups_list =kcalloc (total_num_groups,sizeof (*groups_list), +GFP_KERNEL ); +if (!groups_list) +return -ENOMEM ; + +/* Initialize attribute lists */ +for (i =0 ; i < total_num_groups; i++) +INIT_LIST_HEAD (&groups_list[i].attr_list); + +/* Register Signals */ + err =counter_signals_register (groups_list, counter); +if (err) + goto err_free_groups_list; + num_groups += counter->num_signals; + +/* Register Counts and respective Synapses */ + err =counter_counts_register (groups_list + num_groups, counter); +if (err) + goto err_free_groups_list; + num_groups += counter->num_counts; + +/* Register Counter global attributes */ + err =counter_global_attr_register (groups_list + num_groups, counter); +if (err) + goto err_free_groups_list;drivers/counter/generic-counter.c line 1300
Quoted 10 lines
drivers/counter/generic-counter.c new file +10
-0+ num_groups++; + +/* Store groups_list in device_state */ + counter->device_state->groups_list = groups_list; + counter->device_state->num_groups = num_groups; + +return 0 ; + +err_free_groups_list: +free_counter_device_groups_list (groups_list, num_groups);Consistent naming would be good.
counter_device_groups_list_free.I would tidy this up throughout. I know from experience that you'll probably end up doing so eventually and it is easier whilst there isn't too much code.
drivers/counter/generic-counter.c line 1310
Quoted 10 lines
drivers/counter/generic-counter.c new file +10
-0+return err; +} + +static int prepare_counter_device_groups ( +struct counter_device_state *const device_state) +{ +size_t i; +struct counter_device_attr_group *group; +int err; +size_t j;Tidy this up a little,
size_ti, j;Quoted 25 lines
drivers/counter/generic-counter.c new file +25
-0+struct counter_device_attr *p; + +/* Allocate attribute groups for association with device */ + device_state->groups =kcalloc (device_state->num_groups +1 , +sizeof (*device_state->groups), +GFP_KERNEL ); +if (!device_state->groups) +return -ENOMEM ; + +/* Prepare each group of attributes for association */ +for (i =0 ; i < device_state->num_groups; i++) { + group = device_state->groups_list + i; + +/* Allocate space for attribute pointers in attribute group */ + group->attr_group.attrs =kcalloc (group->num_attr +1 , +sizeof (*group->attr_group.attrs),GFP_KERNEL ); +if (!group->attr_group.attrs) { + err = -ENOMEM ; + goto err_free_groups; + } + +/* Add attribute pointers to attribute group */ + j =0 ; +list_for_each_entry (p, &group->attr_list, l) + group->attr_group.attrs[j++] = &p->dev_attr.attr;drivers/counter/generic-counter.c line 1345
Quoted 10 lines
drivers/counter/generic-counter.c new file +10
-0+ +/* Group attributes in attribute group */ + device_state->groups[i] = &group->attr_group; + } +/* Associate attributes with device */ + device_state->dev.groups = device_state->groups; + +return 0 ; + +err_free_groups:I'm not convinced this is cleaning up properly. What about the kcalloc of
group->attr_group.attrsfor previous loops?Quoted 205 lines
drivers/counter/generic-counter.c new file +196
-0+kfree (device_state->groups); +return err; +} + +/* Provides a unique ID for each counter device */ +static DEFINE_IDA (counter_ida); + +static void counter_device_release (struct device *dev) +{ +struct counter_device *const counter =dev_get_drvdata (dev); +struct counter_device_state *const device_state = counter->device_state; + +kfree (device_state->groups); +free_counter_device_groups_list (device_state->groups_list, + device_state->num_groups); +ida_simple_remove (&counter_ida, device_state->id); +kfree (device_state); +} + +static struct device_type counter_device_type = { + .name ="counter_device" , + .release = counter_device_release +}; + +static struct bus_type counter_bus_type = { + .name ="counter" +}; + +/** +* counter_register - register Counter to the system +* @counter: pointer to Counter to register +* +* This function registers a Counter to the system. A sysfs "counter" directory +* will be created and populated with sysfs attributes correlating with the +* Counter Signals, Synapses, and Counts respectively. +*/ +int counter_register (struct counter_device *const counter) +{ +struct counter_device_state *device_state; +int err; + +/* Allocate internal state container for Counter device */ + device_state =kzalloc (sizeof (*device_state),GFP_KERNEL ); +if (!device_state) +return -ENOMEM ; + counter->device_state = device_state; + +/* Acquire unique ID */ + device_state->id =ida_simple_get (&counter_ida,0 ,0 ,GFP_KERNEL ); +if (device_state->id <0 ) { + err = device_state->id; + goto err_free_device_state; + } + +/* Configure device structure for Counter */ + device_state->dev.type = &counter_device_type; + device_state->dev.bus = &counter_bus_type; +if (counter->parent) { + device_state->dev.parent = counter->parent; + device_state->dev.of_node = counter->parent->of_node; + } +dev_set_name (&device_state->dev,"counter%d" , device_state->id); +device_initialize (&device_state->dev); +dev_set_drvdata (&device_state->dev, counter); + +/* Prepare device attributes */ + err =prepare_counter_device_groups_list (counter); +if (err) + goto err_free_id; + +/* Organize device attributes to groups and match to device */ + err =prepare_counter_device_groups (device_state); +if (err) + goto err_free_groups_list; + +/* Add device to system */ + err =device_add (&device_state->dev); +if (err) + goto err_free_groups; + +return 0 ; + +err_free_groups: +kfree (device_state->groups); +err_free_groups_list: +free_counter_device_groups_list (device_state->groups_list, + device_state->num_groups); +err_free_id: +ida_simple_remove (&counter_ida, device_state->id); +err_free_device_state: +kfree (device_state); +return err; +} +EXPORT_SYMBOL (counter_register); + +/** +* counter_unregister - unregister Counter from the system +* @counter: pointer to Counter to unregister +* +* The Counter is unregistered from the system; all allocated memory is freed. +*/ +void counter_unregister (struct counter_device *const counter) +{ +if (counter) +device_del (&counter->device_state->dev); +} +EXPORT_SYMBOL (counter_unregister); + +static void devm_counter_unreg (struct device *dev,void *res) +{ +counter_unregister (*(struct counter_device **)res); +} + +/** +* devm_counter_register - Resource-managed counter_register +* @dev: device to allocate counter_device for +* @counter: pointer to Counter to register +* +* Managed counter_register. The Counter registered with this function is +* automatically unregistered on driver detach. This function calls +* counter_register internally. Refer to that function for more information. +* +* If an Counter registered with this function needs to be unregistered +* separately, devm_counter_unregister must be used. +* +* RETURNS: +* 0 on success, negative error number on failure. +*/ +int devm_counter_register (struct device *dev, +struct counter_device *const counter) +{ +struct counter_device **ptr; +int ret; + + ptr =devres_alloc (devm_counter_unreg,sizeof (*ptr),GFP_KERNEL ); +if (!ptr) +return -ENOMEM ; + + ret =counter_register (counter); +if (!ret) { + *ptr = counter; +devres_add (dev, ptr); + }else { +devres_free (ptr); + } + +return ret; +} +EXPORT_SYMBOL (devm_counter_register); + +static int devm_counter_match (struct device *dev,void *res,void *data) +{ +struct counter_device **r = res; + +if (!r || !*r) { +WARN_ON (!r || !*r); +return 0 ; + } + +return *r == data; +} + +/** +* devm_counter_unregister - Resource-managed counter_unregister +* @dev: device this counter_device belongs to +* @counter: pointer to Counter associated with the device +* +* Unregister Counter registered with devm_counter_register. +*/ +void devm_counter_unregister (struct device *dev, +struct counter_device *const counter) +{ +int rc; + + rc =devres_release (dev, devm_counter_unreg, devm_counter_match, + counter); +WARN_ON (rc); +} +EXPORT_SYMBOL (devm_counter_unregister); + +static int __init counter_init (void ) +{ +return bus_register (&counter_bus_type); +} + +static void __exit counter_exit(void) +{ +bus_unregister (&counter_bus_type); +} + +subsys_initcall (counter_init); +module_exit (counter_exit); + +MODULE_AUTHOR ("William Breathitt Gray <[email protected]>" ); +MODULE_DESCRIPTION ("Generic Counter interface" ); +MODULE_LICENSE ("GPL v2" );include/linux/counter.h new file +3
-0@@ -0,0 +1,554 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* +* Counter interface include/linux/counter.h line 13
Quoted 10 lines
include/linux/counter.h new file +10
-0+* Copyright (C) 2017 William Breathitt Gray +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License, version 2, as +* published by the Free Software Foundation. +* +* This program is distributed in the hope that it will be useful, but +* WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +* General Public License for more details. It's a bit controversial, but most people consider SPDX headers equivalent of the license statement. As such you normally don't have both and just go for the SPDK header.
I thought we were also standardising on
// SPDX-...Quoted 198 lines
include/linux/counter.h new file +198
-0+*/ +#ifndef _COUNTER_H_ +#define _COUNTER_H_ + +#include <linux/device.h> +#include <linux/types.h> + +enum count_direction { +COUNT_DIRECTION_FORWARD =0 , +COUNT_DIRECTION_BACKWARD +}; +extern const char *const count_direction_str[2 ]; + +enum count_mode { +COUNT_MODE_NORMAL =0 , +COUNT_MODE_RANGE_LIMIT , +COUNT_MODE_NON_RECYCLE , +COUNT_MODE_MODULO_N +}; +extern const char *const count_mode_str[4 ]; + +struct counter_device ; +struct counter_signal ; + +/** +* struct counter_signal_ext - Counter Signal extensions +* @name: attribute name +* @read: read callback for this attribute; may be NULL +* @write: write callback for this attribute; may be NULL +* @priv: data private to the driver +*/ +struct counter_signal_ext { +const char *name; +ssize_t (*read)(struct counter_device *counter, +struct counter_signal *signal,void *priv, +char *buf); +ssize_t (*write)(struct counter_device *counter, +struct counter_signal *signal,void *priv, +const char *buf,size_t len); +void *priv; +}; + +/** +* struct counter_signal - Counter Signal node +* @id: unique ID used to identify signal +* @name: device-specific Signal name; ideally, this should match the name +* as it appears in the datasheet documentation +* @ext: optional array of Counter Signal extensions +* @num_ext: number of Counter Signal extensions specified in @ext +* @priv: optional private data supplied by driver +*/ +struct counter_signal { +int id; +const char *name; + +const struct counter_signal_ext *ext; +size_t num_ext; + +void *priv; +}; + +/** +* struct counter_signal_enum_ext - Signal enum extension attribute +* @items: Array of strings +* @num_items: Number of items specified in @items +* @set: Set callback function; may be NULL +* @get: Get callback function; may be NULL +* +* The counter_signal_enum_ext structure can be used to implement enum style +* Signal extension attributes. Enum style attributes are those which have a set +* of strings that map to unsigned integer values. The Generic Counter Signal +* enum extension helper code takes care of mapping between value and string, as +* well as generating a "_available" file which contains a list of all available +* items. The get callback is used to query the currently active item; the index +* of the item within the respective items array is returned via the 'item' +* parameter. The set callback is called when the attribute is updated; the +* 'item' parameter contains the index of the newly activated item within the +* respective items array. +*/ +struct counter_signal_enum_ext { +const char *const *items; +size_t num_items; +int (*get)(struct counter_device *counter, +struct counter_signal *signal, +size_t *item); +int (*set)(struct counter_device *counter, +struct counter_signal *signal, +size_t item); +}; + +ssize_t counter_signal_enum_read (struct counter_device *counter, +struct counter_signal *signal,void *priv, +char *buf); +ssize_t counter_signal_enum_write (struct counter_device *counter, +struct counter_signal *signal,void *priv, +const char *buf,size_t len); + +/** +* COUNTER_SIGNAL_ENUM() - Initialize Signal enum extension +* @_name: Attribute name +* @_e: Pointer to a counter_count_enum structure +* +* This should usually be used together with COUNTER_SIGNAL_ENUM_AVAILABLE() +*/ +#define COUNTER_SIGNAL_ENUM (_name, _e) \ +{ \ + .name = (_name), \ + .read = counter_signal_enum_read, \ + .write = counter_signal_enum_write, \ + .priv = (_e) \ +} + +ssize_t counter_signal_enum_available_read (struct counter_device *counter, +struct counter_signal *signal, +void *priv,char *buf); + +/** +* COUNTER_SIGNAL_ENUM_AVAILABLE() - Initialize Signal enum available extension +* @_name: Attribute name ("_available" will be appended to the name) +* @_e: Pointer to a counter_signal_enum structure +* +* Creates a read only attribute that lists all the available enum items in a +* newline separated list. This should usually be used together with +* COUNTER_SIGNAL_ENUM() +*/ +#define COUNTER_SIGNAL_ENUM_AVAILABLE (_name, _e) \ +{ \ + .name = (_name "_available"), \ + .read = counter_signal_enum_available_read, \ + .priv = (_e) \ +} + +enum synapse_action { +SYNAPSE_ACTION_NONE =0 , +SYNAPSE_ACTION_RISING_EDGE , +SYNAPSE_ACTION_FALLING_EDGE , +SYNAPSE_ACTION_BOTH_EDGES +}; + +/** +* struct counter_synapse - Counter Synapse node +* @action: index of current action mode +* @actions_list: array of available action modes +* @num_actions: number of action modes specified in @actions_list +* @signal: pointer to associated signal +*/ +struct counter_synapse { +size_t action; +const enum synapse_action *actions_list; +size_t num_actions; + +struct counter_signal *signal; +}; + +struct counter_count ; + +/** +* struct counter_count_ext - Counter Count extension +* @name: attribute name +* @read: read callback for this attribute; may be NULL +* @write: write callback for this attribute; may be NULL +* @priv: data private to the driver +*/ +struct counter_count_ext { +const char *name; +ssize_t (*read)(struct counter_device *counter, +struct counter_count *count,void *priv, +char *buf); +ssize_t (*write)(struct counter_device *counter, +struct counter_count *count,void *priv, +const char *buf,size_t len); +void *priv; +}; + +enum count_function { +COUNT_FUNCTION_INCREASE =0 , +COUNT_FUNCTION_DECREASE , +COUNT_FUNCTION_PULSE_DIRECTION , +COUNT_FUNCTION_QUADRATURE_X1_A , +COUNT_FUNCTION_QUADRATURE_X1_B , +COUNT_FUNCTION_QUADRATURE_X2_A , +COUNT_FUNCTION_QUADRATURE_X2_B , +COUNT_FUNCTION_QUADRATURE_X2_RISING , +COUNT_FUNCTION_QUADRATURE_X2_FALLING , +COUNT_FUNCTION_QUADRATURE_X4 +}; + +/** +* struct counter_count - Counter Count node +* @id: unique ID used to identify Count +* @name: device-specific Count name; ideally, this should match +* the name as it appears in the datasheet documentation +* @function: index of current function mode +* @functions_list: array available function modes +* @num_functions: number of function modes specified in @functions_list +* @synapses: array of synapses for initialization +* @num_synapses: number of synapses specified in @synapses +* @ext: optional array of Counter Count extensions include/linux/counter.h line 221
Quoted 10 lines
include/linux/counter.h new file +10
-0+* @num_ext: number of Counter Count extensions specified in @ext +* @priv: optional private data supplied by driver +*/ +struct counter_count { +int id; +const char *name; + +size_t function; +const enum count_function *functions_list; +size_t num_functions;There is a very good illustration here of the issues with using tabs to pretty print structure elements. I would drop them entirely as personally I'm not sure they help readability and you will forever be having more noise in patches because of the need to change the number of tabs due to name changes etc.
Quoted 88 lines
include/linux/counter.h new file +88
-0+ +struct counter_synapse *synapses; +size_t num_synapses; + +const struct counter_count_ext *ext; +size_t num_ext; + +void *priv; +}; + +/** +* struct counter_count_enum_ext - Count enum extension attribute +* @items: Array of strings +* @num_items: Number of items specified in @items +* @set: Set callback function; may be NULL +* @get: Get callback function; may be NULL +* +* The counter_count_enum_ext structure can be used to implement enum style +* Count extension attributes. Enum style attributes are those which have a set +* of strings that map to unsigned integer values. The Generic Counter Count +* enum extension helper code takes care of mapping between value and string, as +* well as generating a "_available" file which contains a list of all available +* items. The get callback is used to query the currently active item; the index +* of the item within the respective items array is returned via the 'item' +* parameter. The set callback is called when the attribute is updated; the +* 'item' parameter contains the index of the newly activated item within the +* respective items array. +*/ +struct counter_count_enum_ext { +const char *const *items; +size_t num_items; +int (*get)(struct counter_device *counter, +struct counter_count *count, +size_t *item); +int (*set)(struct counter_device *counter, +struct counter_count *count, +size_t item); +}; + +ssize_t counter_count_enum_read (struct counter_device *counter, +struct counter_count *count,void *priv, +char *buf); +ssize_t counter_count_enum_write (struct counter_device *counter, +struct counter_count *count,void *priv, +const char *buf,size_t len); + +/** +* COUNTER_COUNT_ENUM() - Initialize Count enum extension +* @_name: Attribute name +* @_e: Pointer to a counter_count_enum structure +* +* This should usually be used together with COUNTER_COUNT_ENUM_AVAILABLE() +*/ +#define COUNTER_COUNT_ENUM (_name, _e) \ +{ \ + .name = (_name), \ + .read = counter_count_enum_read, \ + .write = counter_count_enum_write, \ + .priv = (_e) \ +} + +ssize_t counter_count_enum_available_read (struct counter_device *counter, +struct counter_count *count, +void *priv,char *buf); + +/** +* COUNTER_COUNT_ENUM_AVAILABLE() - Initialize Count enum available extension +* @_name: Attribute name ("_available" will be appended to the name) +* @_e: Pointer to a counter_count_enum structure +* +* Creates a read only attribute that lists all the available enum items in a +* newline separated list. This should usually be used together with +* COUNTER_COUNT_ENUM() +*/ +#define COUNTER_COUNT_ENUM_AVAILABLE (_name, _e) \ +{ \ + .name = (_name "_available"), \ + .read = counter_count_enum_available_read, \ + .priv = (_e) \ +} + +/** +* struct counter_device_attr_group - internal container for attribute group +* @attr_group: Counter sysfs attributes group +* @attr_list: list to keep track of created Counter sysfs attributes +* @num_attr: number of Counter sysfs attributes +*/ +struct counter_device_attr_group {include/linux/counter.h line 319
Quoted 10 lines
include/linux/counter.h new file +10
-0+struct attribute_group attr_group; +struct list_head attr_list; +size_t num_attr; +}; + +/** +* struct counter_device_state - internal state container for a Counter device +* @id: unique ID used to identify the Counter +* @dev: internal device structure +* @groups_list attribute groups list (groups for Signals, Counts, and ext) Run kernel-doc script over these files. You are missing some :s and it would have told you that.
Quoted 36 lines
include/linux/counter.h new file +36
-0+* @num_groups number of attribute groups containers +* @groups: Counter sysfs attribute groups (used to populate @dev.groups) +*/ +struct counter_device_state { +int id; +struct device dev; +struct counter_device_attr_group *groups_list; +size_t num_groups; +const struct attribute_group **groups; +}; + +/** +* struct signal_read_value - Opaque Signal read value +* @buf: string representation of Signal read value +* @len: length of string in @buf +*/ +struct signal_read_value { +char *buf; +size_t len; +}; + +/** +* struct count_read_value - Opaque Count read value +* @buf: string representation of Count read value +* @len: length of string in @buf +*/ +struct count_read_value { +char *buf; +size_t len; +}; + +/** +* struct count_write_value - Opaque Count write value +* @buf: string representation of Count write value +*/ +struct count_write_value {include/linux/counter.h line 365
Quoted 10 lines
include/linux/counter.h new file +10
-0+const char *buf; +}; + +/** +* struct counter_ops - Callbacks from driver +* @signal_read: optional read callback for Signal attribute. The read +* value of the respective Signal should be passed back via +* the val parameter. val points to an opaque type which +* should be set only via the set_signal_read_value +* function. This last part had me a little confused. I would make it clear that this
set_signal_read_valuefunction should be called to set the value within thissignal_readcallback rather than elsewhere...Quoted 170 lines
include/linux/counter.h new file +170
-0+* @count_read: optional read callback for Count attribute. The read +* value of the respective Count should be passed back via +* the val parameter. val points to an opaque type which +* should be set only via the set_count_read_value +* function. +* @count_write: optional write callback for Count attribute. The write +* value for the respective Count is passed in via the val +* parameter. val points to an opaque type which should be +* access only via the get_count_write_value function. +* @function_get: function to get the current count function mode. Returns +* 0 on success and negative error code on error. The index +* of the respective Count's returned function mode should +* be passed back via the function parameter. +* @function_set: function to set the count function mode. function is the +* index of the requested function mode from the respective +* Count's functions_list array. +* @action_get: function to get the current action mode. Returns 0 on +* success and negative error code on error. The index of +* the respective Signal's returned action mode should be +* passed back via the action parameter. +* @action_set: function to set the action mode. action is the index of +* the requested action mode from the respective Synapse's +* actions_list array. +*/ +struct counter_ops { +int (*signal_read)(struct counter_device *counter, +struct counter_signal *signal, +struct signal_read_value *val); +int (*count_read)(struct counter_device *counter, +struct counter_count *count, +struct count_read_value *val); +int (*count_write)(struct counter_device *counter, +struct counter_count *count, +struct count_write_value *val); +int (*function_get)(struct counter_device *counter, +struct counter_count *count,size_t *function); +int (*function_set)(struct counter_device *counter, +struct counter_count *count,size_t function); +int (*action_get)(struct counter_device *counter, +struct counter_count *count, +struct counter_synapse *synapse,size_t *action); +int (*action_set)(struct counter_device *counter, +struct counter_count *count, +struct counter_synapse *synapse,size_t action); +}; + +/** +* struct counter_device_ext - Counter device extension +* @name: attribute name +* @read: read callback for this attribute; may be NULL +* @write: write callback for this attribute; may be NULL +* @priv: data private to the driver +*/ +struct counter_device_ext { +const char *name; +ssize_t (*read)(struct counter_device *counter,void *priv, +char *buf); +ssize_t (*write)(struct counter_device *counter,void *priv, +const char *buf,size_t len); +void *priv; +}; + +/** +* struct counter_device_enum_ext - Counter enum extension attribute +* @items: Array of strings +* @num_items: Number of items specified in @items +* @set: Set callback function; may be NULL +* @get: Get callback function; may be NULL +* +* The counter_device_enum_ext structure can be used to implement enum style +* Counter extension attributes. Enum style attributes are those which have a +* set of strings that map to unsigned integer values. The Generic Counter enum +* extension helper code takes care of mapping between value and string, as well +* as generating a "_available" file which contains a list of all available +* items. The get callback is used to query the currently active item; the index +* of the item within the respective items array is returned via the 'item' +* parameter. The set callback is called when the attribute is updated; the +* 'item' parameter contains the index of the newly activated item within the +* respective items array. +*/ +struct counter_device_enum_ext { +const char *const *items; +size_t num_items; +int (*get)(struct counter_device *counter, +size_t *item); +int (*set)(struct counter_device *counter, +size_t item); +}; + +ssize_t counter_device_enum_read (struct counter_device *counter,void *priv, +char *buf); +ssize_t counter_device_enum_write (struct counter_device *counter,void *priv, +const char *buf,size_t len); + +/** +* COUNTER_DEVICE_ENUM() - Initialize Counter enum extension +* @_name: Attribute name +* @_e: Pointer to a counter_device_enum structure +* +* This should usually be used together with COUNTER_DEVICE_ENUM_AVAILABLE() +*/ +#define COUNTER_DEVICE_ENUM (_name, _e) \ +{ \ + .name = (_name), \ + .read = counter_device_enum_read, \ + .write = counter_device_enum_write, \ + .priv = (_e) \ +} + +ssize_t counter_device_enum_available_read (struct counter_device *counter, +void *priv,char *buf); + +/** +* COUNTER_DEVICE_ENUM_AVAILABLE() - Initialize Counter enum available extension +* @_name: Attribute name ("_available" will be appended to the name) +* @_e: Pointer to a counter_device_enum structure +* +* Creates a read only attribute that lists all the available enum items in a +* newline separated list. This should usually be used together with +* COUNTER_DEVICE_ENUM() +*/ +#define COUNTER_DEVICE_ENUM_AVAILABLE (_name, _e) \ +{ \ + .name = (_name "_available"), \ + .read = counter_device_enum_available_read, \ + .priv = (_e) \ +} + +/** +* struct counter_device - Counter data structure +* @name: name of the device as it appears in the datasheet +* @parent: optional parent device providing the counters +* @device_state: internal device state container +* @ops: callbacks from driver +* @signals: array of Signals +* @num_signals: number of Signals specified in @signals +* @counts: array of Counts +* @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 +*/ +struct counter_device { +const char *name; +struct device *parent; +struct counter_device_state *device_state; + +const struct counter_ops *ops; + +struct counter_signal *signals; +size_t num_signals; +struct counter_count *counts; +size_t num_counts; + +const struct counter_device_ext *ext; +size_t num_ext; + +void *priv; +}; + +enum signal_level { +SIGNAL_LEVEL_LOW =0 , +SIGNAL_LEVEL_HIGH +}; + +enum signal_value_type { +SIGNAL_LEVEL =0 +}; + +enum count_value_type {include/linux/counter.h line 545
Quoted 10 lines
include/linux/counter.h new file +10
-0+COUNT_POSITION_UNSIGNED =0 , + COUNT_POSITION_SIGNED +}; + +void set_signal_read_value (struct signal_read_value *const val, +const enum signal_value_type type,void *const data); +void set_count_read_value (struct count_read_value *const val, +const enum count_value_type type,void *const data); +int get_count_write_value (void *const data,const enum count_value_type type, +const struct count_write_value *const val);I wonder if naming wise, we would be better sticking to the
noun_verbnaming format.
signal_read_value_setcount_read_value_setcount_write_value_getfor example?
Quoted 9 lines
include/linux/counter.h new file +9
-0+ +int counter_register (struct counter_device *const counter); +void counter_unregister (struct counter_device *const counter); +int devm_counter_register (struct device *dev, +struct counter_device *const counter); +void devm_counter_unregister (struct device *dev, +struct counter_device *const counter); + +#endif /* _COUNTER_H_ */
PATCH v6 2/9 counter: Documentation: Add Generic Counter sysfs documentation
In reply to William Breathitt GrayThis patch adds standard documentation for the userspace sysfs attributes of the Generic Counter interface.
--- Documentation/ABI/testing/sysfs-bus-counter | 241 ++++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 242 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-bus-counter
Documentation/ABI/testing/sysfs-bus-counter new file +241 -0
@@ -0,0 +1,241 @@ +What: /sys/bus/counter/devices/counterX/countY/count +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Count data of Count Y represented as a string. + +What: /sys/bus/counter/devices/counterX/countY/ceiling +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Count value ceiling for Count Y. This is the upper limit for the + respective counter. + +What: /sys/bus/counter/devices/counterX/countY/floor +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Count value floor for Count Y. This is the lower limit for the + respective counter. + +What: /sys/bus/counter/devices/counterX/countY/count_mode +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Count mode for channel Y. The ceiling and floor values for + Count Y are used by the count mode where required. The following + count modes are available: + + Normal: + Counting is continuous in either direction. + + Range Limit: + An upper or lower limit is set, mimicking limit switches + in the mechanical counterpart. The upper limit is set to + the Count Y ceiling value, while the lower limit is set + to the Count Y floor value. The counter freezes at + count = ceiling when counting up, and at count = floor + when counting down. At either of these limits, the + counting is resumed only when the count direction is + reversed. + + Non-Recycle: + The counter is disabled whenever a counter overflow or + underflow takes place. The counter is re-enabled when a + new count value is loaded to the counter via a preset + operation or direct write. + + Modulo-N: + A count value boundary is set between the Count Y floor + value and the Count Y ceiling value. The counter is + reset to the Cunt Y floor value at count = ceiling when + counting up, while the counter is set to the Count Y + ceiling value at count = floor when counting down; the + counter does not freeze at the boundary points, but + counts continuously throughout. + +What: /sys/bus/counter/devices/counterX/countY/count_mode_available +What: /sys/bus/counter/devices/counterX/countY/error_noise_available +What: /sys/bus/counter/devices/counterX/countY/function_available +What: /sys/bus/counter/devices/counterX/countY/signalZ_action_available +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Discrete set of available values for the respective Count Y + configuration are listed in this file. Values are delineated by + newline characters. + +What: /sys/bus/counter/devices/counterX/countY/direction +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Read-only attribute that indicates the count direction of Count + Y. Two count directions are available: forward and backward. + + Some counter devices are able to determine the direction of + their counting. For example, quadrature encoding counters can + determine the direction of movement by evaluating the leading + phase of the respective A and B quadrature encoding signals. + This attribute exposes such count directions. + +What: /sys/bus/counter/devices/counterX/countY/enable +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Whether channel Y counter is enabled. Valid attribute values are + boolean. + + This attribute is intended to serve as a pause/unpause mechanism + for Count Y. Suppose a counter device is used to count the total + movement of a conveyor belt: this attribute allows an operator + to temporarily pause the counter, service the conveyor belt, + and then finally unpause the counter to continue where it had + left off. + +What: /sys/bus/counter/devices/counterX/countY/error_noise +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Read-only attribute that indicates whether excessive noise is + present at the channel Y counter inputs. + +What: /sys/bus/counter/devices/counterX/countY/function +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Count function mode of Count Y; count function evaluation is + triggered by conditions specified by the Count Y signalZ_action + attributes. The following count functions are available: + + Increase: + Accumulated count is incremented. + + Decrease: + Accumulated count is decremented. + + Pulse-Direction: + Rising edges on quadrature pair signal A updates the + respective count. The input level of quadrature pair + signal B determines direction. + + Quadrature x1 A: + If direction is forward, rising edges on quadrature pair + signal A updates the respective count; if the direction + is backward, falling edges on quadrature pair signal A + updates the respective count. Quadrature encoding + determines the direction. + + Quadrature x1 B: + If direction is forward, rising edges on quadrature pair + signal B updates the respective count; if the direction + is backward, falling edges on quadrature pair signal B + updates the respective count. Quadrature encoding + determines the direction. + + Quadrature x2 A: + Any state transition on quadrature pair signal A updates + the respective count. Quadrature encoding determines the + direction. + + Quadrature x2 B: + Any state transition on quadrature pair signal B updates + the respective count. Quadrature encoding determines the + direction. + + Quadrature x2 Rising: + Rising edges on either quadrature pair signals updates + the respective count. Quadrature encoding determines the + direction. + + Quadrature x2 Falling: + Falling edges on either quadrature pair signals updates + the respective count. Quadrature encoding determines the + direction. + + Quadrature x4: + Any state transition on either quadrature pair signals + updates the respective count. Quadrature encoding + determines the direction. + +What: /sys/bus/counter/devices/counterX/countY/name +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Read-only attribute that indicates the device-specific name of + Count Y. If possible, this should match the name of the + respective channel as it appears in the device datasheet + documentation text. + +What: /sys/bus/counter/devices/counterX/countY/preset +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + If the counter device supports preset registers, the preset + count for channel Y is provided by this attribute. + +What: /sys/bus/counter/devices/counterX/countY/preset_enable +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Whether channel Y counter preset operation is enabled. Valid + attribute values are boolean. + +What: /sys/bus/counter/devices/counterX/countY/signalZ_action +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Action mode of Count Y for Signal Z. This attribute indicates + the condition of Signal Z that triggers the count function + evaluation for Count Y. The following action modes are + available: + + None: + Signal does not trigger the count function. In + Pulse-Direction count function mode, this Signal is + evaluated as Direction. + + Rising Edge: + Low state transitions to high state. + + Falling Edge: + High state transitions to low state. + + Both Edges: + Any state transition. + +What: /sys/bus/counter/devices/counterX/name +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Read-only attribute that indicates the device-specific name of + the Counter. This should match the name of the device as it + appears in its respective datasheet documentation text. + +What: /sys/bus/counter/devices/counterX/num_counts +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Read-only attribute that indicates the total number of Counts + belonging to the Counter. + +What: /sys/bus/counter/devices/counterX/num_signals +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Read-only attribute that indicates the total number of Signals + belonging to the Counter. + +What: /sys/bus/counter/devices/counterX/signalY/signal +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Signal data of Signal Y represented as a string. + +What: /sys/bus/counter/devices/counterX/signalY/name +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Read-only attribute that indicates the device-specific name of + Signal Y. If possible, this should match the name of the + respective signal as it appears in the device datasheet + documentation text.
MAINTAINERS +1 -0
@@ -3673,6 +3673,7 @@ COUNTER SUBSYSTEM M: William Breathitt Gray <[email protected]> L: linux-iio at vger.kernel.org S: Maintained +F: Documentation/ABI/testing/sysfs-bus-counter* F: drivers/counter/ F: include/linux/counter.h
Signature · 1 line
2.17.0
Re: [PATCH v6 2/9] counter: Documentation: Add Generic Counter sysfs documentation
Quoted 4 lines · On Wed, 16 May 2018 13:50:55 -0400 William Breathitt Gray…
This patch adds standard documentation for the userspace sysfs attributes of the Generic Counter interface.
Signed-off-by William Breathitt Gray <[email protected]>
Some really minor stuff inline. No functional changes.
Quoted 176 lines
--- Documentation/ABI/testing/sysfs-bus-counter | 241 ++++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 242 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-bus-counterDocumentation/ABI/testing/sysfs-bus-counter new file +164
-0@@ -0,0 +1,241 @@ +What: /sys/bus/counter/devices/counterX/countY/count +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Count data of Count Y represented as a string. + +What: /sys/bus/counter/devices/counterX/countY/ceiling +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Count value ceiling for Count Y. This is the upper limit for the + respective counter. + +What: /sys/bus/counter/devices/counterX/countY/floor +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Count value floor for Count Y. This is the lower limit for the + respective counter. + +What: /sys/bus/counter/devices/counterX/countY/count_mode +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Count mode for channel Y. The ceiling and floor values for + Count Y are used by the count mode where required. The following + count modes are available: + + Normal: + Counting is continuous in either direction. + + Range Limit: + An upper or lower limit is set, mimicking limit switches + in the mechanical counterpart. The upper limit is set to + the Count Y ceiling value, while the lower limit is set + to the Count Y floor value. The counter freezes at + count = ceiling when counting up, and at count = floor + when counting down. At either of these limits, the + counting is resumed only when the count direction is + reversed. + + Non-Recycle: + The counter is disabled whenever a counter overflow or + underflow takes place. The counter is re-enabled when a + new count value is loaded to the counter via a preset + operation or direct write. + + Modulo-N: + A count value boundary is set between the Count Y floor + value and the Count Y ceiling value. The counter is + reset to the Cunt Y floor value at count = ceiling when + counting up, while the counter is set to the Count Y + ceiling value at count = floor when counting down; the + counter does not freeze at the boundary points, but + counts continuously throughout. + +What: /sys/bus/counter/devices/counterX/countY/count_mode_available +What: /sys/bus/counter/devices/counterX/countY/error_noise_available +What: /sys/bus/counter/devices/counterX/countY/function_available +What: /sys/bus/counter/devices/counterX/countY/signalZ_action_available +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Discrete set of available values for the respective Count Y + configuration are listed in this file. Values are delineated by + newline characters. + +What: /sys/bus/counter/devices/counterX/countY/direction +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Read-only attribute that indicates the count direction of Count + Y. Two count directions are available: forward and backward. + + Some counter devices are able to determine the direction of + their counting. For example, quadrature encoding counters can + determine the direction of movement by evaluating the leading + phase of the respective A and B quadrature encoding signals. + This attribute exposes such count directions. + +What: /sys/bus/counter/devices/counterX/countY/enable +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Whether channel Y counter is enabled. Valid attribute values are + boolean. + + This attribute is intended to serve as a pause/unpause mechanism + for Count Y. Suppose a counter device is used to count the total + movement of a conveyor belt: this attribute allows an operator + to temporarily pause the counter, service the conveyor belt, + and then finally unpause the counter to continue where it had + left off. + +What: /sys/bus/counter/devices/counterX/countY/error_noise +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Read-only attribute that indicates whether excessive noise is + present at the channel Y counter inputs. + +What: /sys/bus/counter/devices/counterX/countY/function +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Count function mode of Count Y; count function evaluation is + triggered by conditions specified by the Count Y signalZ_action + attributes. The following count functions are available: + + Increase: + Accumulated count is incremented. + + Decrease: + Accumulated count is decremented. + + Pulse-Direction: + Rising edges on quadrature pair signal A updates the + respective count. The input level of quadrature pair + signal B determines direction. + + Quadrature x1 A: + If direction is forward, rising edges on quadrature pair + signal A updates the respective count; if the direction + is backward, falling edges on quadrature pair signal A + updates the respective count. Quadrature encoding + determines the direction. + + Quadrature x1 B: + If direction is forward, rising edges on quadrature pair + signal B updates the respective count; if the direction + is backward, falling edges on quadrature pair signal B + updates the respective count. Quadrature encoding + determines the direction. + + Quadrature x2 A: + Any state transition on quadrature pair signal A updates + the respective count. Quadrature encoding determines the + direction. + + Quadrature x2 B: + Any state transition on quadrature pair signal B updates + the respective count. Quadrature encoding determines the + direction. + + Quadrature x2 Rising: + Rising edges on either quadrature pair signals updates + the respective count. Quadrature encoding determines the + direction. + + Quadrature x2 Falling: + Falling edges on either quadrature pair signals updates + the respective count. Quadrature encoding determines the + direction. + + Quadrature x4: + Any state transition on either quadrature pair signals + updates the respective count. Quadrature encoding + determines the direction. + +What: /sys/bus/counter/devices/counterX/countY/name +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Read-only attribute that indicates the device-specific name of
Documentation/ABI/testing/sysfs-bus-counter line 174
Quoted 10 lines
Documentation/ABI/testing/sysfs-bus-counter new file +10
-0+ Count Y. If possible, this should match the name of the + respective channel as it appears in the device datasheet + documentation text. + +What: /sys/bus/counter/devices/counterX/countY/preset +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + If the counter device supports preset registers, the preset + count for channel Y is provided by this attribute.
I would add a small block of text here saying what a "preset" typically is. It is a term heavily used in encoders etc, but perhaps not some other types of counter.
Quoted 28 lines
Documentation/ABI/testing/sysfs-bus-counter new file +28
-0+ +What: /sys/bus/counter/devices/counterX/countY/preset_enable +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Whether channel Y counter preset operation is enabled. Valid + attribute values are boolean. + +What: /sys/bus/counter/devices/counterX/countY/signalZ_action +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Action mode of Count Y for Signal Z. This attribute indicates + the condition of Signal Z that triggers the count function + evaluation for Count Y. The following action modes are + available: + + None: + Signal does not trigger the count function. In + Pulse-Direction count function mode, this Signal is + evaluated as Direction. + + Rising Edge: + Low state transitions to high state. + + Falling Edge: + High state transitions to low state. +
Documentation/ABI/testing/sysfs-bus-counter line 212
Quoted 10 lines
Documentation/ABI/testing/sysfs-bus-counter new file +10
-0+ Both Edges: + Any state transition. + +What: /sys/bus/counter/devices/counterX/name +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Read-only attribute that indicates the device-specific name of + the Counter. This should match the name of the device as it + appears in its respective datasheet documentation text.
As below, I'm not sure if "documentation text" makes it clearer or less clear..
Quoted 19 lines
Documentation/ABI/testing/sysfs-bus-counter new file +19
-0+ +What: /sys/bus/counter/devices/counterX/num_counts +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Read-only attribute that indicates the total number of Counts + belonging to the Counter. + +What: /sys/bus/counter/devices/counterX/num_signals +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Read-only attribute that indicates the total number of Signals + belonging to the Counter. + +What: /sys/bus/counter/devices/counterX/signalY/signal +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description:
Documentation/ABI/testing/sysfs-bus-counter line 241
Quoted 10 lines
Documentation/ABI/testing/sysfs-bus-counter new file +10
-0+ Signal data of Signal Y represented as a string. + +What: /sys/bus/counter/devices/counterX/signalY/name +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Read-only attribute that indicates the device-specific name of + Signal Y. If possible, this should match the name of the + respective signal as it appears in the device datasheet + documentation text.
Not sure "documentation text" adds any clarity over the simply "device datasheet"
Quoted 12 lines
MAINTAINERS +1
-0@@ -3673,6 +3673,7 @@ COUNTER SUBSYSTEM M: William Breathitt Gray <[email protected]> L: linux-iio at vger.kernel.org S: Maintained +F: Documentation/ABI/testing/sysfs-bus-counter* F: drivers/counter/ F: include/linux/counter.h
PATCH v6 3/9 docs: Add Generic Counter interface documentation
In reply to William Breathitt GrayThis patch adds high-level documentation about the Generic Counter interface.
--- Documentation/driver-api/generic-counter.rst | 336 +++++++++++++++++++ Documentation/driver-api/index.rst | 1 + MAINTAINERS | 1 + 3 files changed, 338 insertions(+) create mode 100644 Documentation/driver-api/generic-counter.rst
Documentation/driver-api/generic-counter.rst new file +336 -0
@@ -0,0 +1,336 @@ +.. SPDX-License-Identifier: GPL-2.0 + +========================= +Generic Counter Interface +========================= + +Introduction +============ + +Counter devices are prevalent within a diverse spectrum of industries. +The ubiquitous presence of these devices necessitates a common interface +and standard of interaction and exposure. This driver API attempts to +resolve the issue of duplicate code found among existing counter device +drivers by introducing a generic counter interface for consumption. The +Generic Counter interface enables drivers to support and expose a common +set of components and functionality present in counter devices. + +Theory +====== + +Counter devices can vary greatly in design, but regardless of whether +some devices are quadrature encoder counters or tally counters, all +counter devices consist of a core set of components. This core set of +components, shared by all counter devices, is what forms the essence of +the Generic Counter interface. + +There are three core components to a counter: + +COUNT +----- +A Count represents the count data for a set of Signals. The Generic +Counter interface provides the following available count data types: + +* COUNT_POSITION_UNSIGNED: + Unsigned integer value representing position. + +* COUNT_POSITION_SIGNED: + Signed integer value representing position. + +A Count has a count function mode which represents the update behavior +for the count data. The Generic Counter interface provides the following +available count function modes: + +* Increase: + Accumulated count is incremented. + +* Decrease: + Accumulated count is decremented. + +* Pulse-Direction: + Rising edges on quadrature pair signal A updates the respective count. + The input level of quadrature pair signal B determines direction. + +* Quadrature x1 A: + If direction is forward, rising edges on quadrature pair signal A + updates the respective count; if the direction is backward, falling + edges on quadrature pair signal A updates the respective count. + Quadrature encoding determines the direction. + +* Quadrature x1 B: + If direction is forward, rising edges on quadrature pair signal B + updates the respective count; if the direction is backward, falling + edges on quadrature pair signal B updates the respective count. + Quadrature encoding determines the direction. + +* Quadrature x2 A: + Any state transition on quadrature pair signal A updates the + respective count. Quadrature encoding determines the direction. + +* Quadrature x2 B: + Any state transition on quadrature pair signal B updates the + respective count. Quadrature encoding determines the direction. + +* Quadrature x2 Rising: + Rising edges on either quadrature pair signals updates the respective + count. Quadrature encoding determines the direction. + +* Quadrature x2 Falling: + Falling edges on either quadrature pair signals updates the respective + count. Quadrature encoding determines the direction. + +* Quadrature x4: + Any state transition on either quadrature pair signals updates the + respective count. Quadrature encoding determines the direction. + +A Count has a set of one or more associated Signals. + +SIGNAL +------ +A Signal represents a counter input data; this is the input data that is +analyzed by the counter to determine the count data; e.g. a quadrature +signal output line of a rotary encoder. Not all counter devices provide +user access to the Signal data. + +The Generic Counter interface provides the following available signal +data types for when the Signal data is available for user access: + +* SIGNAL_LEVEL_LOW: + Signal line is in a low state. + +* SIGNAL_LEVEL_HIGH: + Signal line is in a high state. + +A Signal may be associated with one or more Counts. + +SYNAPSE +------- +A Synapse represents the association of a Signal with a respective +Count. Signal data affects respective Count data, and the Synapse +represents this relationship. + +The Synapse action mode specifies the Signal data condition which +triggers the respective Count's count function evaluation to update the +count data. The Generic Counter interface provides the following +available action modes: + +* None: + Signal does not trigger the count function. In Pulse-Direction count + function mode, this Signal is evaluated as Direction. + +* Rising Edge: + Low state transitions to high state. + +* Falling Edge: + High state transitions to low state. + +* Both Edges: + Any state transition. + +A counter is defined as a set of input signals associated with count +data that are generated by the evaluation of the state of the associated +input signals as defined by the respective count functions. Within the +context of the Generic Counter interface, a counter consists of Counts +each associated with a set of Signals, whose respective Synapse +instances represent the count function update conditions for the +associated Counts. + +Paradigm +======== + +The most basic counter device may be expressed as a single Count +associated with a single Signal via a single Synapse. Take for example +a counter device which simply accumulates a count of rising edges on a +source input line:: + + Count Synapse Signal + ----- ------- ------ + +---------------------+ + | Data: Count | Rising Edge ________ + | Function: Increase | <------------- / Source \ + | | ____________ + +---------------------+ + +In this example, the Signal is a source input line with a pulsing +voltage, while the Count is a persistent count value which is repeatedly +incremented. The Signal is associated with the respective Count via a +Synapse. The increase function is triggered by the Signal data condition +specified by the Synapse -- in this case a rising edge condition on the +voltage input line. In summary, the counter device existence and +behavior is aptly represented by respective Count, Signal, and Synapse +components: a rising edge condition triggers an increase function on an +accumulating count datum. + +A counter device is not limited to a single Signal; in fact, in theory +many Signals may be associated with even a single Count. For example, a +quadrature encoder counter device can keep track of position based on +the states of two input lines:: + + Count Synapse Signal + ----- ------- ------ + +-------------------------+ + | Data: Position | Both Edges ___ + | Function: Quadrature x4 | <------------ / A \ + | | _______ + | | + | | Both Edges ___ + | | <------------ / B \ + | | _______ + +-------------------------+ + +In this example, two Signals (quadrature encoder lines A and B) are +associated with a single Count: a rising or falling edge on either A or +B triggers the "Quadrature x4" function which determines the direction +of movement and updates the respective position data. The "Quadrature +x4" function is likely implemented in the hardware of the quadrature +encoder counter device; the Count, Signals, and Synapses simply +represent this hardware behavior and functionality. + +Signals associated with the same Count can have differing Synapse action +mode conditions. For example, a quadrature encoder counter device +operating in a non-quadrature Pulse-Direction mode could have one input +line dedicated for movement and a second input line dedicated for +direction:: + + Count Synapse Signal + ----- ------- ------ + +---------------------------+ + | Data: Position | Rising Edge ___ + | Function: Pulse-Direction | <------------- / A \ (Movement) + | | _______ + | | + | | None ___ + | | <------------- / B \ (Direction) + | | _______ + +---------------------------+ + +Only Signal A triggers the "Pulse-Direction" update function, but the +instantaneous state of Signal B is still required in order to know the +direction so that the position data may be properly updated. Ultimately, +both Signals are associated with the same Count via two respective +Synapses, but only one Synapse has an active action mode condition which +triggers the respective count function while the other is left with a +"None" condition action mode to indicate its respective Signal's +availability for state evaluation despite its non-triggering mode. + +Keep in mind that the Signal, Synapse, and Count are abstract +representations which do not need to be closely married to their +respective physical sources. This allows the user of a counter to +divorce themselves from the nuances of physical components (such as +whether an input line is differential or single-ended) and instead focus +on the core idea of what the data and process represent (e.g. position +as interpreted from quadrature encoding data). + +Userspace Interface +=================== + +Several sysfs attributes are generated by the Generic Counter interface, +and reside under the /sys/bus/counter/devices/counterX directory, where +counterX refers to the respective counter device. Please see +Documentation/ABI/testing/sys-bus-counter-generic-sysfs for detailed +information on each Generic Counter interface sysfs attribute. + +Through these sysfs attributes, programs and scripts may interact with +the Generic Counter paradigm Counts, Signals, and Synapses of respective +counter devices. + +Driver API +========== + +Driver authors may utilize the Generic Counter interface in their code +by including the include/linux/iio/counter.h header file. This header +file provides several core data structures, function prototypes, and +macros for defining a counter device. + +.. kernel-doc:: include/linux/counter.h + :internal: + +.. kernel-doc:: drivers/counter/generic-counter.c + :export: + +Implementation +============== + +To support a counter device, a driver must first allocate the available +Counter Signals via counter_signal structures. These Signals should +be stored as an array and set to the signals array member of an +allocated counter_device structure before the Counter is registered to +the system. + +Counter Counts may be allocated via counter_count structures, and +respective Counter Signal associations (Synapses) made via +counter_synapse structures. Associated counter_synapse structures are +stored as an array and set to the the synapses array member of the +respective counter_count structure. These counter_count structures are +set to the counts array member of an allocated counter_device structure +before the Counter is registered to the system. + +Driver callbacks should be provided to the counter_device structure via +a constant counter_ops structure in order to communicate with the +device: to read and write various Signals and Counts, and to set and get +the "action mode" and "function mode" for various Synapses and Counts +respectively. + +A defined counter_device structure may be registered to the system by +passing it to the counter_register function, and unregistered by passing +it to the counter_unregister function. Similarly, the +devm_counter_register and devm_counter_unregister functions may be used +if device memory-managed registration is desired. + +Extension sysfs attributes can be created for auxiliary functionality +and data by passing in defined counter_device_ext, counter_count_ext, +and counter_signal_ext structures. In these cases, the +counter_device_ext structure is used for global configuration of the +respective Counter device, while the counter_count_ext and +counter_signal_ext structures allow for auxiliary exposure and +configuration of a specific Count or Signal respectively. + +Architecture +============ + +When the Generic Counter interface counter module is loaded, the +counter_init function is called which registers a bus_type named +"counter" to the system. Subsequently, when the module is unloaded, the +counter_exit function is called which unregisters the bus_type named +"counter" from the system. + +Counter devices are registered to the system via the counter_register +function, and later removed via the counter_unregister function. The +counter_register function establishes a unique ID for the Counter +device and creates a respective sysfs directory, where X is the +mentioned unique ID: + + /sys/bus/counter/devices/counterX + +Sysfs attributes are created within the counterX directory to expose +functionality, configurations, and data relating to the Counts, Signals, +and Synapses of the Counter device, as well as options and information +for the Counter device itself. + +Each Signal has a directory created to house its relevant sysfs +attributes, where Y is the unique ID of the respective Signal: + + /sys/bus/counter/devices/counterX/signalY + +Similarly, each Count has a directory created to house its relevant +sysfs attributes, where Y is the unique ID of the respective Count: + + /sys/bus/counter/devices/counterX/countY + +For a more detailed breakdown of the available Generic Counter interface +sysfs attributes, please refer to the +Documentation/ABI/testing/sys-bus-counter file. + +The Signals and Counts associated with the Counter device are registered +to the system as well by the counter_register function. The +signal_read/signal_write driver callbacks are associated with their +respective Signal attributes, while the count_read/count_write and +function_get/function_set driver callbacks are associated with their +respective Count attributes; similarly, the same is true for the +action_get/action_set driver callbacks and their respective Synapse +attributes. If a driver callback is left undefined, then the respective +read/write permission is left disabled for the relevant attributes. + +Similarly, extension sysfs attributes are created for the defined +counter_device_ext, counter_count_ext, and counter_signal_ext +structures that are passed in.
Documentation/driver-api/index.rst +1 -0
@@ -25,6 +25,7 @@ available subsections can be seen below. frame-buffer regulator iio/index + generic-counter input usb/index pci
MAINTAINERS +1 -0
@@ -3674,6 +3674,7 @@ M: William Breathitt Gray <[email protected]> L: linux-iio at vger.kernel.org S: Maintained F: Documentation/ABI/testing/sysfs-bus-counter* +F: Documentation/driver-api/generic-counter.rst F: drivers/counter/ F: include/linux/counter.h
Signature · 1 line
2.17.0
kbuild test robot
build botRe: [PATCH v6 3/9] docs: Add Generic Counter interface documentation
I love your patch! Perhaps something to improve:
Hi William,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v4.17-rc5 next-20180517]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/William-Breathitt-Gray/Introduce-the-Counter-subsystem/20180519-151353
reproduce: make htmldocs
All warnings (new ones prefixed by >>):
WARNING: convert(1) not found, for SVG to PDF conversion install ImageMagick (https://www.imagemagick.org)
include/net/cfg80211.h:4216: warning: Function parameter or member 'wext.ibss' not described in 'wireless_dev' include/net/cfg80211.h:4216: warning: Function parameter or member 'wext.connect' not described in 'wireless_dev' include/net/cfg80211.h:4216: warning: Function parameter or member 'wext.keys' not described in 'wireless_dev' include/net/cfg80211.h:4216: warning: Function parameter or member 'wext.ie' not described in 'wireless_dev' include/net/cfg80211.h:4216: warning: Function parameter or member 'wext.ie_len' not described in 'wireless_dev' include/net/cfg80211.h:4216: warning: Function parameter or member 'wext.bssid' not described in 'wireless_dev' include/net/cfg80211.h:4216: warning: Function parameter or member 'wext.ssid' not described in 'wireless_dev' include/net/cfg80211.h:4216: warning: Function parameter or member 'wext.default_key' not described in 'wireless_dev' include/net/cfg80211.h:4216: warning: Function parameter or member 'wext.default_mgmt_key' not described in 'wireless_dev' include/net/cfg80211.h:4216: warning: Function parameter or member 'wext.prev_bssid_valid' not described in 'wireless_dev' include/net/mac80211.h:2282: warning: Function parameter or member 'radiotap_timestamp.units_pos' not described in 'ieee80211_hw' include/net/mac80211.h:2282: warning: Function parameter or member 'radiotap_timestamp.accuracy' not described in 'ieee80211_hw' include/net/mac80211.h:955: warning: Function parameter or member 'control.rates' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.rts_cts_rate_idx' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.use_rts' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.use_cts_prot' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.short_preamble' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.skip_table' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.jiffies' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.vif' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.hw_key' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.flags' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'control.enqueue_time' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'ack' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'ack.cookie' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.rates' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.ack_signal' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.ampdu_ack_len' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.ampdu_len' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.antenna' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.tx_time' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.is_valid_ack_signal' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'status.status_driver_data' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'driver_rates' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'pad' not described in 'ieee80211_tx_info' include/net/mac80211.h:955: warning: Function parameter or member 'rate_driver_data' not described in 'ieee80211_tx_info' net/mac80211/sta_info.h:586: warning: Function parameter or member 'rx_stats_avg' not described in 'sta_info' net/mac80211/sta_info.h:586: warning: Function parameter or member 'rx_stats_avg.signal' not described in 'sta_info' net/mac80211/sta_info.h:586: warning: Function parameter or member 'rx_stats_avg.chain_signal' not described in 'sta_info' net/mac80211/sta_info.h:586: warning: Function parameter or member 'status_stats.filtered' not described in 'sta_info' net/mac80211/sta_info.h:586: warning: Function parameter or member 'status_stats.retry_failed' not described in 'sta_info' net/mac80211/sta_info.h:586: warning: Function parameter or member 'status_stats.retry_count' not described in 'sta_info' net/mac80211/sta_info.h:586: warning: Function parameter or member 'status_stats.lost_packets' not described in 'sta_info' net/mac80211/sta_info.h:586: warning: Function parameter or member 'status_stats.last_tdls_pkt_time' not described in 'sta_info' net/mac80211/sta_info.h:586: warning: Function parameter or member 'status_stats.msdu_retries' not described in 'sta_info' net/mac80211/sta_info.h:586: warning: Function parameter or member 'status_stats.msdu_failed' not described in 'sta_info' net/mac80211/sta_info.h:586: warning: Function parameter or member 'status_stats.last_ack' not described in 'sta_info' net/mac80211/sta_info.h:586: warning: Function parameter or member 'status_stats.last_ack_signal' not described in 'sta_info' net/mac80211/sta_info.h:586: warning: Function parameter or member 'status_stats.ack_signal_filled' not described in 'sta_info' net/mac80211/sta_info.h:586: warning: Function parameter or member 'tx_stats.packets' not described in 'sta_info' net/mac80211/sta_info.h:586: warning: Function parameter or member 'tx_stats.bytes' not described in 'sta_info' net/mac80211/sta_info.h:586: warning: Function parameter or member 'tx_stats.last_rate' not described in 'sta_info' net/mac80211/sta_info.h:586: warning: Function parameter or member 'tx_stats.msdu' not described in 'sta_info' kernel/sched/fair.c:3719: warning: Function parameter or member 'flags' not described in 'attach_entity_load_avg' include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_excl.cb' not described in 'dma_buf' include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_excl.poll' not described in 'dma_buf' include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_excl.active' not described in 'dma_buf' include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_shared.cb' not described in 'dma_buf' include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_shared.poll' not described in 'dma_buf' include/linux/dma-buf.h:307: warning: Function parameter or member 'cb_shared.active' not described in 'dma_buf' include/linux/dma-fence-array.h:54: warning: Function parameter or member 'work' not described in 'dma_fence_array'
>> include/linux/counter.h:330: warning: Function parameter or member 'groups_list' not described in 'counter_device_state' >> include/linux/counter.h:330: warning: Function parameter or member 'num_groups' not described in 'counter_device_state'
include/linux/gpio/driver.h:142: warning: Function parameter or member 'request_key' not described in 'gpio_irq_chip' include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.sign' not described in 'iio_chan_spec' include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.realbits' not described in 'iio_chan_spec' include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.storagebits' not described in 'iio_chan_spec' include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.shift' not described in 'iio_chan_spec' include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.repeat' not described in 'iio_chan_spec' include/linux/iio/iio.h:270: warning: Function parameter or member 'scan_type.endianness' not described in 'iio_chan_spec' include/linux/iio/hw-consumer.h:1: warning: no structured comments found include/linux/input/sparse-keymap.h:46: warning: Function parameter or member 'sw' not described in 'key_entry' include/linux/mtd/rawnand.h:752: warning: Function parameter or member 'timings.sdr' not described in 'nand_data_interface' include/linux/mtd/rawnand.h:817: warning: Function parameter or member 'buf' not described in 'nand_op_data_instr' include/linux/mtd/rawnand.h:817: warning: Function parameter or member 'buf.in' not described in 'nand_op_data_instr' include/linux/mtd/rawnand.h:817: warning: Function parameter or member 'buf.out' not described in 'nand_op_data_instr' include/linux/mtd/rawnand.h:863: warning: Function parameter or member 'ctx' not described in 'nand_op_instr' include/linux/mtd/rawnand.h:863: warning: Function parameter or member 'ctx.cmd' not described in 'nand_op_instr' include/linux/mtd/rawnand.h:863: warning: Function parameter or member 'ctx.addr' not described in 'nand_op_instr' include/linux/mtd/rawnand.h:863: warning: Function parameter or member 'ctx.data' not described in 'nand_op_instr' include/linux/mtd/rawnand.h:863: warning: Function parameter or member 'ctx.waitrdy' not described in 'nand_op_instr' include/linux/mtd/rawnand.h:1010: warning: Function parameter or member 'ctx' not described in 'nand_op_parser_pattern_elem' include/linux/mtd/rawnand.h:1010: warning: Function parameter or member 'ctx.addr' not described in 'nand_op_parser_pattern_elem' include/linux/mtd/rawnand.h:1010: warning: Function parameter or member 'ctx.data' not described in 'nand_op_parser_pattern_elem' include/linux/mtd/rawnand.h:1313: warning: Function parameter or member 'manufacturer.desc' not described in 'nand_chip' include/linux/mtd/rawnand.h:1313: warning: Function parameter or member 'manufacturer.priv' not described in 'nand_chip' include/linux/regulator/driver.h:222: warning: Function parameter or member 'resume_early' not described in 'regulator_ops' drivers/regulator/core.c:4306: warning: Excess function parameter 'state' description in 'regulator_suspend_late' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw0' not described in 'irb' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw1' not described in 'irb' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw2' not described in 'irb' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.esw3' not described in 'irb' arch/s390/include/asm/cio.h:245: warning: Function parameter or member 'esw.eadm' not described in 'irb' drivers/usb/typec/mux.c:186: warning: Function parameter or member 'mux' not described in 'typec_mux_unregister' drivers/usb/typec/mux.c:186: warning: Excess function parameter 'sw' description in 'typec_mux_unregister' include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_pin' not described in 'drm_driver' include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_unpin' not described in 'drm_driver' include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_res_obj' not described in 'drm_driver' include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_get_sg_table' not described in 'drm_driver' include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_import_sg_table' not described in 'drm_driver' include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_vmap' not described in 'drm_driver' include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_vunmap' not described in 'drm_driver' include/drm/drm_drv.h:610: warning: Function parameter or member 'gem_prime_mmap' not described in 'drm_driver' drivers/gpu/drm/drm_prime.c:342: warning: Function parameter or member 'attach' not described in 'drm_gem_unmap_dma_buf' drivers/gpu/drm/drm_prime.c:342: warning: Function parameter or member 'sgt' not described in 'drm_gem_unmap_dma_buf' drivers/gpu/drm/drm_prime.c:342: warning: Function parameter or member 'dir' not described in 'drm_gem_unmap_dma_buf' drivers/gpu/drm/drm_prime.c:438: warning: Function parameter or member 'dma_buf' not described in 'drm_gem_dmabuf_kmap_atomic' drivers/gpu/drm/drm_prime.c:438: warning: Function parameter or member 'page_num' not described in 'drm_gem_dmabuf_kmap_atomic' drivers/gpu/drm/drm_prime.c:450: warning: Function parameter or member 'dma_buf' not described in 'drm_gem_dmabuf_kunmap_atomic' drivers/gpu/drm/drm_prime.c:450: warning: Function parameter or member 'page_num' not described in 'drm_gem_dmabuf_kunmap_atomic' drivers/gpu/drm/drm_prime.c:450: warning: Function parameter or member 'addr' not described in 'drm_gem_dmabuf_kunmap_atomic' drivers/gpu/drm/drm_prime.c:461: warning: Function parameter or member 'dma_buf' not described in 'drm_gem_dmabuf_kmap' drivers/gpu/drm/drm_prime.c:461: warning: Function parameter or member 'page_num' not described in 'drm_gem_dmabuf_kmap' drivers/gpu/drm/drm_prime.c:473: warning: Function parameter or member 'dma_buf' not described in 'drm_gem_dmabuf_kunmap' drivers/gpu/drm/drm_prime.c:473: warning: Function parameter or member 'page_num' not described in 'drm_gem_dmabuf_kunmap' drivers/gpu/drm/drm_prime.c:473: warning: Function parameter or member 'addr' not described in 'drm_gem_dmabuf_kunmap' include/media/v4l2-dev.h:42: warning: Enum value 'VFL_TYPE_MAX' not described in enum 'vfl_devnode_type' include/linux/skbuff.h:850: warning: Function parameter or member 'dev_scratch' not described in 'sk_buff' include/linux/skbuff.h:850: warning: Function parameter or member 'ip_defrag_offset' not described in 'sk_buff' include/linux/skbuff.h:850: warning: Function parameter or member 'skb_mstamp' not described in 'sk_buff' include/linux/skbuff.h:850: warning: Function parameter or member '__cloned_offset' not described in 'sk_buff' include/linux/skbuff.h:850: warning: Function parameter or member 'head_frag' not described in 'sk_buff' include/linux/skbuff.h:850: warning: Function parameter or member '__unused' not described in 'sk_buff' include/linux/skbuff.h:850: warning: Function parameter or member '__pkt_type_offset' not described in 'sk_buff' include/linux/skbuff.h:850: warning: Function parameter or member 'pfmemalloc' not described in 'sk_buff' include/linux/skbuff.h:850: warning: Function parameter or member 'encapsulation' not described in 'sk_buff' include/linux/skbuff.h:850: warning: Function parameter or member 'encap_hdr_csum' not described in 'sk_buff' include/linux/skbuff.h:850: warning: Function parameter or member 'csum_valid' not described in 'sk_buff' include/linux/skbuff.h:850: warning: Function parameter or member 'csum_complete_sw' not described in 'sk_buff' include/linux/skbuff.h:850: warning: Function parameter or member 'csum_level' not described in 'sk_buff' include/linux/skbuff.h:850: warning: Function parameter or member 'inner_protocol_type' not described in 'sk_buff' include/linux/skbuff.h:850: warning: Function parameter or member 'remcsum_offload' not described in 'sk_buff' include/linux/skbuff.h:850: warning: Function parameter or member 'offload_fwd_mark' not described in 'sk_buff' include/linux/skbuff.h:850: warning: Function parameter or member 'offload_mr_fwd_mark' not described in 'sk_buff' include/linux/skbuff.h:850: warning: Function parameter or member 'sender_cpu' not described in 'sk_buff' include/linux/skbuff.h:850: warning: Function parameter or member 'reserved_tailroom' not described in 'sk_buff' include/linux/skbuff.h:850: warning: Function parameter or member 'inner_ipproto' not described in 'sk_buff' include/net/sock.h:234: warning: Function parameter or member 'skc_addrpair' not described in 'sock_common' include/net/sock.h:234: warning: Function parameter or member 'skc_portpair' not described in 'sock_common' include/net/sock.h:234: warning: Function parameter or member 'skc_ipv6only' not described in 'sock_common' include/net/sock.h:234: warning: Function parameter or member 'skc_net_refcnt' not described in 'sock_common' include/net/sock.h:234: warning: Function parameter or member 'skc_v6_daddr' not described in 'sock_common' include/net/sock.h:234: warning: Function parameter or member 'skc_v6_rcv_saddr' not described in 'sock_common' include/net/sock.h:234: warning: Function parameter or member 'skc_cookie' not described in 'sock_common' include/net/sock.h:234: warning: Function parameter or member 'skc_listener' not described in 'sock_common' include/net/sock.h:234: warning: Function parameter or member 'skc_tw_dr' not described in 'sock_common' include/net/sock.h:234: warning: Function parameter or member 'skc_rcv_wnd' not described in 'sock_common' include/net/sock.h:234: warning: Function parameter or member 'skc_tw_rcv_nxt' not described in 'sock_common' include/net/sock.h:488: warning: Function parameter or member 'sk_backlog.rmem_alloc' not described in 'sock' include/net/sock.h:488: warning: Function parameter or member 'sk_backlog.len' not described in 'sock' include/net/sock.h:488: warning: Function parameter or member 'sk_backlog.head' not described in 'sock' include/net/sock.h:488: warning: Function parameter or member 'sk_backlog.tail' not described in 'sock' include/net/sock.h:488: warning: Function parameter or member 'sk_wq_raw' not described in 'sock' include/net/sock.h:488: warning: Function parameter or member 'tcp_rtx_queue' not described in 'sock' include/net/sock.h:488: warning: Function parameter or member 'sk_route_forced_caps' not described in 'sock' include/linux/netdevice.h:1955: warning: Function parameter or member 'adj_list.upper' not described in 'net_device' include/linux/netdevice.h:1955: warning: Function parameter or member 'adj_list.lower' not described in 'net_device' include/linux/netdevice.h:1955: warning: Function parameter or member 'gso_partial_features' not described in 'net_device' include/linux/netdevice.h:1955: warning: Function parameter or member 'switchdev_ops' not described in 'net_device' include/linux/netdevice.h:1955: warning: Function parameter or member 'l3mdev_ops' not described in 'net_device' include/linux/netdevice.h:1955: warning: Function parameter or member 'xfrmdev_ops' not described in 'net_device' include/linux/netdevice.h:1955: warning: Function parameter or member 'name_assign_type' not described in 'net_device' include/linux/netdevice.h:1955: warning: Function parameter or member 'ieee802154_ptr' not described in 'net_device'
vim +330 include/linux/counter.h
624995ae William Breathitt Gray 2018-05-16 @330
:::::: The code at line 330 was first introduced by commit
:::::: 624995aed3646d19eb4ce4cd7f527fe95a165a92 counter: Introduce the Generic Counter interface
:::::: TO: William Breathitt Gray <[email protected]> :::::: CC: 0day robot <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 6365 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180519/0eca97a3/attachment-0001.gz>
Jonathan Cameron
maintainerRe: [PATCH v6 3/9] docs: Add Generic Counter interface documentation
In reply to William Breathitt GrayQuoted 4 lines · On Wed, 16 May 2018 13:51:06 -0400 William Breathitt Gray…
This patch adds high-level documentation about the Generic Counter interface.
Signed-off-by William Breathitt Gray <[email protected]>
Various comments inline. I've been doing a lot long reviews recently (outside of the kernel world) and keep discovering the old rule that everytime you read a document you'll find something else to improve :(
Jonathan
Quoted 30 lines
--- Documentation/driver-api/generic-counter.rst | 336 +++++++++++++++++++ Documentation/driver-api/index.rst | 1 + MAINTAINERS | 1 + 3 files changed, 338 insertions(+) create mode 100644 Documentation/driver-api/generic-counter.rstDocumentation/driver-api/generic-counter.rst new file +17
-0@@ -0,0 +1,336 @@ +.. SPDX-License-Identifier: GPL-2.0 + +========================= +Generic Counter Interface +========================= + +Introduction +============ + +Counter devices are prevalent within a diverse spectrum of industries. +The ubiquitous presence of these devices necessitates a common interface +and standard of interaction and exposure. This driver API attempts to +resolve the issue of duplicate code found among existing counter device +drivers by introducing a generic counter interface for consumption. The +Generic Counter interface enables drivers to support and expose a common +set of components and functionality present in counter devices. +
Documentation/driver-api/generic-counter.rst line 27
Quoted 10 lines
Documentation/driver-api/generic-counter.rst new file +10
-0+Theory +====== + +Counter devices can vary greatly in design, but regardless of whether +some devices are quadrature encoder counters or tally counters, all +counter devices consist of a core set of components. This core set of +components, shared by all counter devices, is what forms the essence of +the Generic Counter interface. + +There are three core components to a counter:
Enumerate them here. If people are reading this as a paged document (pdf etc) then the list of 3 as titles of next few sections may not be clear.
* Count
* Signal
* Synapse
Documentation/driver-api/generic-counter.rst line 38
Quoted 11 lines
Documentation/driver-api/generic-counter.rst new file +11
-0+ +COUNT +----- +A Count represents the count data for a set of Signals. The Generic +Counter interface provides the following available count data types: + +* COUNT_POSITION_UNSIGNED: + Unsigned integer value representing position. + +* COUNT_POSITION_SIGNED: + Signed integer value representing position.
Just a thought: As the '0' position is effectively arbitrary is there any actual difference between signed and unsigned? If we defined all counters to be unsigned and just offset any signed ones so the range still fitted would we end up with a simpler interface - there would be no real loss of meaning that I can see.. I suppose it might not be what people expect though when they see their counters start at a large positive value...
Quoted 5 lines
Documentation/driver-api/generic-counter.rst new file +5
-0+ +A Count has a count function mode which represents the update behavior +for the count data. The Generic Counter interface provides the following +available count function modes: +
Documentation/driver-api/generic-counter.rst line 53
Quoted 10 lines
Documentation/driver-api/generic-counter.rst new file +10
-0+* Increase: + Accumulated count is incremented. + +* Decrease: + Accumulated count is decremented. + +* Pulse-Direction: + Rising edges on quadrature pair signal A updates the respective count. + The input level of quadrature pair signal B determines direction. +
Perhaps group the quadrature modes for the point of view of this document? Might be slightly easier to read?
* Quadrature Modes
- x1 A: etc.
Quoted 13 lines
Documentation/driver-api/generic-counter.rst new file +13
-0+* Quadrature x1 A: + If direction is forward, rising edges on quadrature pair signal A + updates the respective count; if the direction is backward, falling + edges on quadrature pair signal A updates the respective count. + Quadrature encoding determines the direction. + +* Quadrature x1 B: + If direction is forward, rising edges on quadrature pair signal B + updates the respective count; if the direction is backward, falling + edges on quadrature pair signal B updates the respective count. + Quadrature encoding determines the direction. + +* Quadrature x2 A:
Documentation/driver-api/generic-counter.rst line 76
Quoted 10 lines
Documentation/driver-api/generic-counter.rst new file +10
-0+ Any state transition on quadrature pair signal A updates the + respective count. Quadrature encoding determines the direction. + +* Quadrature x2 B: + Any state transition on quadrature pair signal B updates the + respective count. Quadrature encoding determines the direction. + +* Quadrature x2 Rising: + Rising edges on either quadrature pair signals updates the respective + count. Quadrature encoding determines the direction.
This one I've never met. Really? There are devices who do this form of crazy? It gives really uneven counting and I'm failing to see when it would ever make sense... References for these odd corner cases would be good.
__|---|____|-----|____
____|----|____|-----|____
001122222223334444444
Quoted 155 lines
Documentation/driver-api/generic-counter.rst new file +155
-0+ +* Quadrature x2 Falling: + Falling edges on either quadrature pair signals updates the respective + count. Quadrature encoding determines the direction. + +* Quadrature x4: + Any state transition on either quadrature pair signals updates the + respective count. Quadrature encoding determines the direction. + +A Count has a set of one or more associated Signals. + +SIGNAL +------ +A Signal represents a counter input data; this is the input data that is +analyzed by the counter to determine the count data; e.g. a quadrature +signal output line of a rotary encoder. Not all counter devices provide +user access to the Signal data. + +The Generic Counter interface provides the following available signal +data types for when the Signal data is available for user access: + +* SIGNAL_LEVEL_LOW: + Signal line is in a low state. + +* SIGNAL_LEVEL_HIGH: + Signal line is in a high state. + +A Signal may be associated with one or more Counts. + +SYNAPSE +------- +A Synapse represents the association of a Signal with a respective +Count. Signal data affects respective Count data, and the Synapse +represents this relationship. + +The Synapse action mode specifies the Signal data condition which +triggers the respective Count's count function evaluation to update the +count data. The Generic Counter interface provides the following +available action modes: + +* None: + Signal does not trigger the count function. In Pulse-Direction count + function mode, this Signal is evaluated as Direction. + +* Rising Edge: + Low state transitions to high state. + +* Falling Edge: + High state transitions to low state. + +* Both Edges: + Any state transition. + +A counter is defined as a set of input signals associated with count +data that are generated by the evaluation of the state of the associated +input signals as defined by the respective count functions. Within the +context of the Generic Counter interface, a counter consists of Counts +each associated with a set of Signals, whose respective Synapse +instances represent the count function update conditions for the +associated Counts. + +Paradigm +======== + +The most basic counter device may be expressed as a single Count +associated with a single Signal via a single Synapse. Take for example +a counter device which simply accumulates a count of rising edges on a +source input line:: + + Count Synapse Signal + ----- ------- ------ + +---------------------+ + | Data: Count | Rising Edge ________ + | Function: Increase | <------------- / Source \ + | | ____________ + +---------------------+ + +In this example, the Signal is a source input line with a pulsing +voltage, while the Count is a persistent count value which is repeatedly +incremented. The Signal is associated with the respective Count via a +Synapse. The increase function is triggered by the Signal data condition +specified by the Synapse -- in this case a rising edge condition on the +voltage input line. In summary, the counter device existence and +behavior is aptly represented by respective Count, Signal, and Synapse +components: a rising edge condition triggers an increase function on an +accumulating count datum. + +A counter device is not limited to a single Signal; in fact, in theory +many Signals may be associated with even a single Count. For example, a +quadrature encoder counter device can keep track of position based on +the states of two input lines:: + + Count Synapse Signal + ----- ------- ------ + +-------------------------+ + | Data: Position | Both Edges ___ + | Function: Quadrature x4 | <------------ / A \ + | | _______ + | | + | | Both Edges ___ + | | <------------ / B \ + | | _______ + +-------------------------+ + +In this example, two Signals (quadrature encoder lines A and B) are +associated with a single Count: a rising or falling edge on either A or +B triggers the "Quadrature x4" function which determines the direction +of movement and updates the respective position data. The "Quadrature +x4" function is likely implemented in the hardware of the quadrature +encoder counter device; the Count, Signals, and Synapses simply +represent this hardware behavior and functionality. + +Signals associated with the same Count can have differing Synapse action +mode conditions. For example, a quadrature encoder counter device +operating in a non-quadrature Pulse-Direction mode could have one input +line dedicated for movement and a second input line dedicated for +direction:: + + Count Synapse Signal + ----- ------- ------ + +---------------------------+ + | Data: Position | Rising Edge ___ + | Function: Pulse-Direction | <------------- / A \ (Movement) + | | _______ + | | + | | None ___ + | | <------------- / B \ (Direction) + | | _______ + +---------------------------+ + +Only Signal A triggers the "Pulse-Direction" update function, but the +instantaneous state of Signal B is still required in order to know the +direction so that the position data may be properly updated. Ultimately, +both Signals are associated with the same Count via two respective +Synapses, but only one Synapse has an active action mode condition which +triggers the respective count function while the other is left with a +"None" condition action mode to indicate its respective Signal's +availability for state evaluation despite its non-triggering mode. + +Keep in mind that the Signal, Synapse, and Count are abstract +representations which do not need to be closely married to their +respective physical sources. This allows the user of a counter to +divorce themselves from the nuances of physical components (such as +whether an input line is differential or single-ended) and instead focus +on the core idea of what the data and process represent (e.g. position +as interpreted from quadrature encoding data). + +Userspace Interface +=================== + +Several sysfs attributes are generated by the Generic Counter interface, +and reside under the /sys/bus/counter/devices/counterX directory, where +counterX refers to the respective counter device. Please see +Documentation/ABI/testing/sys-bus-counter-generic-sysfs for detailed +information on each Generic Counter interface sysfs attribute.
Documentation/driver-api/generic-counter.rst line 241
Quoted 10 lines
Documentation/driver-api/generic-counter.rst new file +10
-0+ +Through these sysfs attributes, programs and scripts may interact with +the Generic Counter paradigm Counts, Signals, and Synapses of respective +counter devices. + +Driver API +========== + +Driver authors may utilize the Generic Counter interface in their code +by including the include/linux/iio/counter.h header file. This header
Didn't this move?
Quoted 119 lines
Documentation/driver-api/generic-counter.rst new file +95
-0+file provides several core data structures, function prototypes, and +macros for defining a counter device. + +.. kernel-doc:: include/linux/counter.h + :internal: + +.. kernel-doc:: drivers/counter/generic-counter.c + :export: + +Implementation +============== + +To support a counter device, a driver must first allocate the available +Counter Signals via counter_signal structures. These Signals should +be stored as an array and set to the signals array member of an +allocated counter_device structure before the Counter is registered to +the system. + +Counter Counts may be allocated via counter_count structures, and +respective Counter Signal associations (Synapses) made via +counter_synapse structures. Associated counter_synapse structures are +stored as an array and set to the the synapses array member of the +respective counter_count structure. These counter_count structures are +set to the counts array member of an allocated counter_device structure +before the Counter is registered to the system. + +Driver callbacks should be provided to the counter_device structure via +a constant counter_ops structure in order to communicate with the +device: to read and write various Signals and Counts, and to set and get +the "action mode" and "function mode" for various Synapses and Counts +respectively. + +A defined counter_device structure may be registered to the system by +passing it to the counter_register function, and unregistered by passing +it to the counter_unregister function. Similarly, the +devm_counter_register and devm_counter_unregister functions may be used +if device memory-managed registration is desired. + +Extension sysfs attributes can be created for auxiliary functionality +and data by passing in defined counter_device_ext, counter_count_ext, +and counter_signal_ext structures. In these cases, the +counter_device_ext structure is used for global configuration of the +respective Counter device, while the counter_count_ext and +counter_signal_ext structures allow for auxiliary exposure and +configuration of a specific Count or Signal respectively. + +Architecture +============ + +When the Generic Counter interface counter module is loaded, the +counter_init function is called which registers a bus_type named +"counter" to the system. Subsequently, when the module is unloaded, the +counter_exit function is called which unregisters the bus_type named +"counter" from the system. + +Counter devices are registered to the system via the counter_register +function, and later removed via the counter_unregister function. The +counter_register function establishes a unique ID for the Counter +device and creates a respective sysfs directory, where X is the +mentioned unique ID: + + /sys/bus/counter/devices/counterX + +Sysfs attributes are created within the counterX directory to expose +functionality, configurations, and data relating to the Counts, Signals, +and Synapses of the Counter device, as well as options and information +for the Counter device itself. + +Each Signal has a directory created to house its relevant sysfs +attributes, where Y is the unique ID of the respective Signal: + + /sys/bus/counter/devices/counterX/signalY + +Similarly, each Count has a directory created to house its relevant +sysfs attributes, where Y is the unique ID of the respective Count: + + /sys/bus/counter/devices/counterX/countY + +For a more detailed breakdown of the available Generic Counter interface +sysfs attributes, please refer to the +Documentation/ABI/testing/sys-bus-counter file. + +The Signals and Counts associated with the Counter device are registered +to the system as well by the counter_register function. The +signal_read/signal_write driver callbacks are associated with their +respective Signal attributes, while the count_read/count_write and +function_get/function_set driver callbacks are associated with their +respective Count attributes; similarly, the same is true for the +action_get/action_set driver callbacks and their respective Synapse +attributes. If a driver callback is left undefined, then the respective +read/write permission is left disabled for the relevant attributes. + +Similarly, extension sysfs attributes are created for the defined +counter_device_ext, counter_count_ext, and counter_signal_ext +structures that are passed in.Documentation/driver-api/index.rst +1
-0@@ -25,6 +25,7 @@ available subsections can be seen below. frame-buffer regulator iio/index + generic-counter input usb/index pciMAINTAINERS +1
-0@@ -3674,6 +3674,7 @@ M: William Breathitt Gray <[email protected]> L: linux-iio at vger.kernel.org S: Maintained F: Documentation/ABI/testing/sysfs-bus-counter* +F: Documentation/driver-api/generic-counter.rst F: drivers/counter/ F: include/linux/counter.h
Re: [PATCH v6 3/9] docs: Add Generic Counter interface documentation
Quoted 6 lines · Jonathan Cameron
Quoted 3 lines · On Wed, 16 May 2018 13:51:06 -0400 William Breathitt Gray…
This patch adds high-level documentation about the Generic Counter interface.
Quoted 8 lines
Quoted 1 line
Signed-off-by William Breathitt Gray <[email protected]>Various comments inline. I've been doing a lot long reviews recently (outside of the kernel world) and keep discovering the old rule that everytime you read a document you'll find something else to improve :(
Jonathan
But it is good too to have multiple eyes on this -- I have found as an author my brain tends to skip over minor errors while rereading passages, so having persons reading it for both the first and subsequent times helps catch those mistakes I may have overlooked in my mind. :)
Quoted 60 lines
Quoted 30 lines
--- Documentation/driver-api/generic-counter.rst | 336 +++++++++++++++++++ Documentation/driver-api/index.rst | 1 + MAINTAINERS | 1 + 3 files changed, 338 insertions(+) create mode 100644 Documentation/driver-api/generic-counter.rstDocumentation/driver-api/generic-counter.rst new file +17
-0@@ -0,0 +1,336 @@ +.. SPDX-License-Identifier: GPL-2.0 + +========================= +Generic Counter Interface +========================= + +Introduction +============ + +Counter devices are prevalent within a diverse spectrum of industries. +The ubiquitous presence of these devices necessitates a common interface +and standard of interaction and exposure. This driver API attempts to +resolve the issue of duplicate code found among existing counter device +drivers by introducing a generic counter interface for consumption. The +Generic Counter interface enables drivers to support and expose a common +set of components and functionality present in counter devices. +Documentation/driver-api/generic-counter.rst line 27
Quoted 10 lines
Documentation/driver-api/generic-counter.rst new file +10
-0+Theory +====== + +Counter devices can vary greatly in design, but regardless of whether +some devices are quadrature encoder counters or tally counters, all +counter devices consist of a core set of components. This core set of +components, shared by all counter devices, is what forms the essence of +the Generic Counter interface. + +There are three core components to a counter:Enumerate them here. If people are reading this as a paged document (pdf etc) then the list of 3 as titles of next few sections may not be clear.
* Count
* Signal
* Synapse
Quoted 10 lines
Documentation/driver-api/generic-counter.rst new file +10
-0+ +COUNT +----- +A Count represents the count data for a set of Signals. The Generic +Counter interface provides the following available count data types: + +* COUNT_POSITION_UNSIGNED: + Unsigned integer value representing position. + +* COUNT_POSITION_SIGNED:
Quoted 8 lines
Documentation/driver-api/generic-counter.rst line 38
Quoted 1 line
Documentation/driver-api/generic-counter.rst new file +1
-0+ Signed integer value representing position.Just a thought: As the '0' position is effectively arbitrary is there any actual difference between signed and unsigned? If we defined all counters to be unsigned and just offset any signed ones so the range still fitted would we end up with a simpler interface - there would be no real loss of meaning that I can see.. I suppose it might not be what people expect though when they see their counters start at a large positive value...
This is something I've been on the fence about for a while. I would actually prefer the interface to have simply a COUNT_POSITION data type to represent position and leave it as unsigned; distinguishing between signed and unsigned position seems ultimately arbitrary given that it's mathematically just an offset as you have pointed out.
If we were to go down this route, then we'd have a count value that may always be represented using an unsigned data type, with an offset value that may always be represented using a signed data type -- the relationship being such: position = count + offset. Is that correct?
My reason for giving the option for either signed or unsigned position was to help minimize the work userspace would have to do in order to get the value in which they're actually interested. I suppose it was a question of how abstract I want to make the interface -- although, making it simpler for userspace put more of a burden on the kernel side.
However, the "offset" value route may actually be more robust in the end because userspace would have to know whether they want a signed or unsigned position regardless in order to parse, so with count and offet defined we know they will always be unsigned and signed respectively.
Quoted 56 lines
Quoted 9 lines
Documentation/driver-api/generic-counter.rst new file +5
-0+ +A Count has a count function mode which represents the update behavior +for the count data. The Generic Counter interface provides the following +available count function modes: +Documentation/driver-api/generic-counter.rst line 53
Quoted 10 lines
Documentation/driver-api/generic-counter.rst new file +10
-0+* Increase: + Accumulated count is incremented. + +* Decrease: + Accumulated count is decremented. + +* Pulse-Direction: + Rising edges on quadrature pair signal A updates the respective count. + The input level of quadrature pair signal B determines direction. +Perhaps group the quadrature modes for the point of view of this document? Might be slightly easier to read?
* Quadrature Modes
- x1 A: etc.
Quoted 13 lines
Documentation/driver-api/generic-counter.rst new file +13
-0+* Quadrature x1 A: + If direction is forward, rising edges on quadrature pair signal A + updates the respective count; if the direction is backward, falling + edges on quadrature pair signal A updates the respective count. + Quadrature encoding determines the direction. + +* Quadrature x1 B: + If direction is forward, rising edges on quadrature pair signal B + updates the respective count; if the direction is backward, falling + edges on quadrature pair signal B updates the respective count. + Quadrature encoding determines the direction. + +* Quadrature x2 A:Documentation/driver-api/generic-counter.rst line 76
Quoted 10 lines
Documentation/driver-api/generic-counter.rst new file +10
-0+ Any state transition on quadrature pair signal A updates the + respective count. Quadrature encoding determines the direction. + +* Quadrature x2 B: + Any state transition on quadrature pair signal B updates the + respective count. Quadrature encoding determines the direction. + +* Quadrature x2 Rising: + Rising edges on either quadrature pair signals updates the respective + count. Quadrature encoding determines the direction.This one I've never met. Really? There are devices who do this form of crazy? It gives really uneven counting and I'm failing to see when it would ever make sense... References for these odd corner cases would be good.
Quoted 4 lines
__|---|____|-----|____
____|----|____|-----|____001122222223334444444
That's the same reaction I had when I discovered this -- in fact the STM32 LP Timer is the first time I've come across such a quadrature mode. I'm not sure of the use case for this mode, because positioning wouldn't be precise as you've pointed out. Perhaps Fabrice or Benjamin can probe the ST guys responsible for this design choice to figure out the rationale.
I'm leaving in these modes for now, as they do exist in the STM32 LP Timer, but it does make me curious what the intentions for them were (perhaps use cases outside of traditional quadrature encoder positioning).
Quoted 161 lines
Quoted 161 lines
Documentation/driver-api/generic-counter.rst new file +159
-0+ +* Quadrature x2 Falling: + Falling edges on either quadrature pair signals updates the respective + count. Quadrature encoding determines the direction. + +* Quadrature x4: + Any state transition on either quadrature pair signals updates the + respective count. Quadrature encoding determines the direction. + +A Count has a set of one or more associated Signals. + +SIGNAL +------ +A Signal represents a counter input data; this is the input data that is +analyzed by the counter to determine the count data; e.g. a quadrature +signal output line of a rotary encoder. Not all counter devices provide +user access to the Signal data. + +The Generic Counter interface provides the following available signal +data types for when the Signal data is available for user access: + +* SIGNAL_LEVEL_LOW: + Signal line is in a low state. + +* SIGNAL_LEVEL_HIGH: + Signal line is in a high state. + +A Signal may be associated with one or more Counts. + +SYNAPSE +------- +A Synapse represents the association of a Signal with a respective +Count. Signal data affects respective Count data, and the Synapse +represents this relationship. + +The Synapse action mode specifies the Signal data condition which +triggers the respective Count's count function evaluation to update the +count data. The Generic Counter interface provides the following +available action modes: + +* None: + Signal does not trigger the count function. In Pulse-Direction count + function mode, this Signal is evaluated as Direction. + +* Rising Edge: + Low state transitions to high state. + +* Falling Edge: + High state transitions to low state. + +* Both Edges: + Any state transition. + +A counter is defined as a set of input signals associated with count +data that are generated by the evaluation of the state of the associated +input signals as defined by the respective count functions. Within the +context of the Generic Counter interface, a counter consists of Counts +each associated with a set of Signals, whose respective Synapse +instances represent the count function update conditions for the +associated Counts. + +Paradigm +======== + +The most basic counter device may be expressed as a single Count +associated with a single Signal via a single Synapse. Take for example +a counter device which simply accumulates a count of rising edges on a +source input line:: + + Count Synapse Signal + ----- ------- ------ + +---------------------+ + | Data: Count | Rising Edge ________ + | Function: Increase | <------------- / Source \ + | | ____________ + +---------------------+ + +In this example, the Signal is a source input line with a pulsing +voltage, while the Count is a persistent count value which is repeatedly +incremented. The Signal is associated with the respective Count via a +Synapse. The increase function is triggered by the Signal data condition +specified by the Synapse -- in this case a rising edge condition on the +voltage input line. In summary, the counter device existence and +behavior is aptly represented by respective Count, Signal, and Synapse +components: a rising edge condition triggers an increase function on an +accumulating count datum. + +A counter device is not limited to a single Signal; in fact, in theory +many Signals may be associated with even a single Count. For example, a +quadrature encoder counter device can keep track of position based on +the states of two input lines:: + + Count Synapse Signal + ----- ------- ------ + +-------------------------+ + | Data: Position | Both Edges ___ + | Function: Quadrature x4 | <------------ / A \ + | | _______ + | | + | | Both Edges ___ + | | <------------ / B \ + | | _______ + +-------------------------+ + +In this example, two Signals (quadrature encoder lines A and B) are +associated with a single Count: a rising or falling edge on either A or +B triggers the "Quadrature x4" function which determines the direction +of movement and updates the respective position data. The "Quadrature +x4" function is likely implemented in the hardware of the quadrature +encoder counter device; the Count, Signals, and Synapses simply +represent this hardware behavior and functionality. + +Signals associated with the same Count can have differing Synapse action +mode conditions. For example, a quadrature encoder counter device +operating in a non-quadrature Pulse-Direction mode could have one input +line dedicated for movement and a second input line dedicated for +direction:: + + Count Synapse Signal + ----- ------- ------ + +---------------------------+ + | Data: Position | Rising Edge ___ + | Function: Pulse-Direction | <------------- / A \ (Movement) + | | _______ + | | + | | None ___ + | | <------------- / B \ (Direction) + | | _______ + +---------------------------+ + +Only Signal A triggers the "Pulse-Direction" update function, but the +instantaneous state of Signal B is still required in order to know the +direction so that the position data may be properly updated. Ultimately, +both Signals are associated with the same Count via two respective +Synapses, but only one Synapse has an active action mode condition which +triggers the respective count function while the other is left with a +"None" condition action mode to indicate its respective Signal's +availability for state evaluation despite its non-triggering mode. + +Keep in mind that the Signal, Synapse, and Count are abstract +representations which do not need to be closely married to their +respective physical sources. This allows the user of a counter to +divorce themselves from the nuances of physical components (such as +whether an input line is differential or single-ended) and instead focus +on the core idea of what the data and process represent (e.g. position +as interpreted from quadrature encoding data). + +Userspace Interface +=================== + +Several sysfs attributes are generated by the Generic Counter interface, +and reside under the /sys/bus/counter/devices/counterX directory, where +counterX refers to the respective counter device. Please see +Documentation/ABI/testing/sys-bus-counter-generic-sysfs for detailed +information on each Generic Counter interface sysfs attribute. + +Through these sysfs attributes, programs and scripts may interact with +the Generic Counter paradigm Counts, Signals, and Synapses of respective +counter devices.
Quoted 8 lines
Documentation/driver-api/generic-counter.rst line 241
Quoted 6 lines
Documentation/driver-api/generic-counter.rst new file +6
-0+ +Driver API +========== + +Driver authors may utilize the Generic Counter interface in their code +by including the include/linux/iio/counter.h header file. This headerDidn't this move?
Yes you are correct, looks like an oversight I made. I'll cleanup this and the rest with the next revision then.
William Breathitt Gray
Quoted 121 lines
Quoted 120 lines
Documentation/driver-api/generic-counter.rst new file +95
-0+file provides several core data structures, function prototypes, and +macros for defining a counter device. + +.. kernel-doc:: include/linux/counter.h + :internal: + +.. kernel-doc:: drivers/counter/generic-counter.c + :export: + +Implementation +============== + +To support a counter device, a driver must first allocate the available +Counter Signals via counter_signal structures. These Signals should +be stored as an array and set to the signals array member of an +allocated counter_device structure before the Counter is registered to +the system. + +Counter Counts may be allocated via counter_count structures, and +respective Counter Signal associations (Synapses) made via +counter_synapse structures. Associated counter_synapse structures are +stored as an array and set to the the synapses array member of the +respective counter_count structure. These counter_count structures are +set to the counts array member of an allocated counter_device structure +before the Counter is registered to the system. + +Driver callbacks should be provided to the counter_device structure via +a constant counter_ops structure in order to communicate with the +device: to read and write various Signals and Counts, and to set and get +the "action mode" and "function mode" for various Synapses and Counts +respectively. + +A defined counter_device structure may be registered to the system by +passing it to the counter_register function, and unregistered by passing +it to the counter_unregister function. Similarly, the +devm_counter_register and devm_counter_unregister functions may be used +if device memory-managed registration is desired. + +Extension sysfs attributes can be created for auxiliary functionality +and data by passing in defined counter_device_ext, counter_count_ext, +and counter_signal_ext structures. In these cases, the +counter_device_ext structure is used for global configuration of the +respective Counter device, while the counter_count_ext and +counter_signal_ext structures allow for auxiliary exposure and +configuration of a specific Count or Signal respectively. + +Architecture +============ + +When the Generic Counter interface counter module is loaded, the +counter_init function is called which registers a bus_type named +"counter" to the system. Subsequently, when the module is unloaded, the +counter_exit function is called which unregisters the bus_type named +"counter" from the system. + +Counter devices are registered to the system via the counter_register +function, and later removed via the counter_unregister function. The +counter_register function establishes a unique ID for the Counter +device and creates a respective sysfs directory, where X is the +mentioned unique ID: + + /sys/bus/counter/devices/counterX + +Sysfs attributes are created within the counterX directory to expose +functionality, configurations, and data relating to the Counts, Signals, +and Synapses of the Counter device, as well as options and information +for the Counter device itself. + +Each Signal has a directory created to house its relevant sysfs +attributes, where Y is the unique ID of the respective Signal: + + /sys/bus/counter/devices/counterX/signalY + +Similarly, each Count has a directory created to house its relevant +sysfs attributes, where Y is the unique ID of the respective Count: + + /sys/bus/counter/devices/counterX/countY + +For a more detailed breakdown of the available Generic Counter interface +sysfs attributes, please refer to the +Documentation/ABI/testing/sys-bus-counter file. + +The Signals and Counts associated with the Counter device are registered +to the system as well by the counter_register function. The +signal_read/signal_write driver callbacks are associated with their +respective Signal attributes, while the count_read/count_write and +function_get/function_set driver callbacks are associated with their +respective Count attributes; similarly, the same is true for the +action_get/action_set driver callbacks and their respective Synapse +attributes. If a driver callback is left undefined, then the respective +read/write permission is left disabled for the relevant attributes. + +Similarly, extension sysfs attributes are created for the defined +counter_device_ext, counter_count_ext, and counter_signal_ext +structures that are passed in.Documentation/driver-api/index.rst +1
-0@@ -25,6 +25,7 @@ available subsections can be seen below. frame-buffer regulator iio/index + generic-counter input usb/index pciMAINTAINERS +1
-0@@ -3674,6 +3674,7 @@ M: William Breathitt Gray <[email protected]> L: linux-iio at vger.kernel.org S: Maintained F: Documentation/ABI/testing/sysfs-bus-counter* +F: Documentation/driver-api/generic-counter.rst F: drivers/counter/ F: include/linux/counter.h
Jonathan Cameron
maintainerRe: [PATCH v6 3/9] docs: Add Generic Counter interface documentation
...
Quoted 25 lines
Quoted 10 lines
Quoted 10 lines
Documentation/driver-api/generic-counter.rst new file +10
-0+ +COUNT +----- +A Count represents the count data for a set of Signals. The Generic +Counter interface provides the following available count data types: + +* COUNT_POSITION_UNSIGNED: + Unsigned integer value representing position. + +* COUNT_POSITION_SIGNED:Quoted 8 lines
Quoted 1 line
+ Signed integer value representing position.
Just a thought: As the '0' position is effectively arbitrary is there any actual difference between signed and unsigned? If we defined all counters to be unsigned and just offset any signed ones so the range still fitted would we end up with a simpler interface - there would be no real loss of meaning that I can see.. I suppose it might not be what people expect though when they see their counters start at a large positive value...
This is something I've been on the fence about for a while. I would actually prefer the interface to have simply a
COUNT_POSITIONdata type to represent position and leave it as unsigned; distinguishing between signed and unsigned position seems ultimately arbitrary given that it's mathematically just an offset as you have pointed out.
Quoted 4 lines
If we were to go down this route, then we'd have a count value that may always be represented using an unsigned data type, with an offset value that may always be represented using a signed data type -- the relationship being such: position = count + offset. Is that correct?
Given the positions are all arbitrary (such as limits etc) there is no real need to have 0 as in anyway special. So you could just apply an offset to everything then don't make them visible to userspace at all.
Quoted 11 lines
My reason for giving the option for either signed or unsigned position was to help minimize the work userspace would have to do in order to get the value in which they're actually interested. I suppose it was a question of how abstract I want to make the interface -- although, making it simpler for userspace put more of a burden on the kernel side.
However, the "offset" value route may actually be more robust in the end because userspace would have to know whether they want a signed or unsigned position regardless in order to parse, so with count and offet defined we know they will always be unsigned and signed respectively.
Hmm. It's not obvious to me which is the best option.
Quoted 60 lines
Quoted 60 lines
Quoted 10 lines
Documentation/driver-api/generic-counter.rst new file +5
-0+ +A Count has a count function mode which represents the update behavior +for the count data. The Generic Counter interface provides the following +available count function modes: +Documentation/driver-api/generic-counter.rst line 53
Quoted 10 lines
Documentation/driver-api/generic-counter.rst new file +10
-0+* Increase: + Accumulated count is incremented. + +* Decrease: + Accumulated count is decremented. + +* Pulse-Direction: + Rising edges on quadrature pair signal A updates the respective count. + The input level of quadrature pair signal B determines direction. +Perhaps group the quadrature modes for the point of view of this document? Might be slightly easier to read?
* Quadrature Modes
- x1 A: etc.
Quoted 13 lines
Documentation/driver-api/generic-counter.rst new file +13
-0+* Quadrature x1 A: + If direction is forward, rising edges on quadrature pair signal A + updates the respective count; if the direction is backward, falling + edges on quadrature pair signal A updates the respective count. + Quadrature encoding determines the direction. + +* Quadrature x1 B: + If direction is forward, rising edges on quadrature pair signal B + updates the respective count; if the direction is backward, falling + edges on quadrature pair signal B updates the respective count. + Quadrature encoding determines the direction. + +* Quadrature x2 A:Documentation/driver-api/generic-counter.rst line 76
Quoted 10 lines
Documentation/driver-api/generic-counter.rst new file +10
-0+ Any state transition on quadrature pair signal A updates the + respective count. Quadrature encoding determines the direction. + +* Quadrature x2 B: + Any state transition on quadrature pair signal B updates the + respective count. Quadrature encoding determines the direction. + +* Quadrature x2 Rising: + Rising edges on either quadrature pair signals updates the respective + count. Quadrature encoding determines the direction.This one I've never met. Really? There are devices who do this form of crazy? It gives really uneven counting and I'm failing to see when it would ever make sense... References for these odd corner cases would be good.
__|---|____|-----|____
____|----|____|-----|____
Quoted 8 lines
Quoted 1 line
001122222223334444444
That's the same reaction I had when I discovered this -- in fact the STM32 LP Timer is the first time I've come across such a quadrature mode. I'm not sure of the use case for this mode, because positioning wouldn't be precise as you've pointed out. Perhaps Fabrice or Benjamin can probe the ST guys responsible for this design choice to figure out the rationale.
Hmm. My inclination would be to not support it unless someone can up with a meaningful use. We are adding ABI (be it not much) for a case that to us makes no sense.
Looks rather like the sort of thing that is a side effect of the implementation rather than deliberate.
Quoted 6 lines
I'm leaving in these modes for now, as they do exist in the STM32 LP Timer, but it does make me curious what the intentions for them were (perhaps use cases outside of traditional quadrature encoder positioning).
Sure if there is a usecase then fair enough.
Jonathan
Re: [PATCH v6 3/9] docs: Add Generic Counter interface documentation
Quoted 19 lines · Jonathan Cameron
Quoted 12 lines
Quoted 12 lines
Documentation/driver-api/generic-counter.rst line 76
Quoted 3 lines
Documentation/driver-api/generic-counter.rst new file +3
-0+* Quadrature x2 Rising: + Rising edges on either quadrature pair signals updates the respective + count. Quadrature encoding determines the direction.This one I've never met. Really? There are devices who do this form of crazy? It gives really uneven counting and I'm failing to see when it would ever make sense... References for these odd corner cases would be good.
__|---|____|-----|____
____|----|____|-----|____Quoted 7 lines
Quoted 1 line
001122222223334444444
That's the same reaction I had when I discovered this -- in fact the STM32 LP Timer is the first time I've come across such a quadrature mode. I'm not sure of the use case for this mode, because positioning wouldn't be precise as you've pointed out. Perhaps Fabrice or Benjamin can probe the ST guys responsible for this design choice to figure out the rationale.
Quoted 3 lines
Hmm. My inclination would be to not support it unless someone can up with a meaningful use. We are adding ABI (be it not much) for a case that to us makes no sense.
Hi Jonathan, William,
Sorry for the late reply. To follow your advise, we can probably drop this for now. I think simple counter, or quadrature x4 will be mostly used for now. As you pointed out, there's not much ABI for x2 rising/falling cases. It will not be a big deal to add it later if needed.
I can help to update (remove & test) this in LP-Timer counter driver if you wish.
Please let me know,
Thanks,
Fabrice
Quoted 14 lines
Looks rather like the sort of thing that is a side effect of the implementation rather than deliberate.
Quoted 5 lines
I'm leaving in these modes for now, as they do exist in the STM32 LP Timer, but it does make me curious what the intentions for them were (perhaps use cases outside of traditional quadrature encoder positioning).
Sure if there is a usecase then fair enough.
Jonathan
Re: [PATCH v6 3/9] docs: Add Generic Counter interface documentation
Quoted 31 lines · Fabrice Gasnier
Quoted 19 lines · Jonathan Cameron
Quoted 12 lines
Quoted 12 lines
Documentation/driver-api/generic-counter.rst line 76
Quoted 3 lines
Documentation/driver-api/generic-counter.rst new file +3
-0+* Quadrature x2 Rising: + Rising edges on either quadrature pair signals updates the respective + count. Quadrature encoding determines the direction.This one I've never met. Really? There are devices who do this form of crazy? It gives really uneven counting and I'm failing to see when it would ever make sense... References for these odd corner cases would be good.
__|---|____|-----|____
____|----|____|-----|____Quoted 7 lines
Quoted 1 line
001122222223334444444
That's the same reaction I had when I discovered this -- in fact the STM32 LP Timer is the first time I've come across such a quadrature mode. I'm not sure of the use case for this mode, because positioning wouldn't be precise as you've pointed out. Perhaps Fabrice or Benjamin can probe the ST guys responsible for this design choice to figure out the rationale.
Quoted 3 lines
Hmm. My inclination would be to not support it unless someone can up with a meaningful use. We are adding ABI (be it not much) for a case that to us makes no sense.
Hi Jonathan, William,
Sorry for the late reply. To follow your advise, we can probably drop this for now. I think simple counter, or quadrature x4 will be mostly used for now. As you pointed out, there's not much ABI for x2 rising/falling cases. It will not be a big deal to add it later if needed.
Quoted 7 lines
I can help to update (remove & test) this in LP-Timer counter driver if you wish.
Please let me know,
Thanks,
Fabrice
All right, let's postpone the COUNT_FUNCTION_QUADRATURE_X2_RISING and COUNT_FUNCTION_QUADRATURE_X2_FALLING modes for now. Fabrice, send me over an update patch removing these modes from the LP-Timer counter driver and I'll squash it in with the next patchset revision.
I'll keep the rest of the quadrature modes the same as they are used in the other counter drivers as well (with the remaining "Quadrature x1 B" staying to complete the pattern) and I've seen real world use cases for each:
* COUNT_FUNCTION_QUADRATURE_X1_A
* COUNT_FUNCTION_QUADRATURE_X1_B
* COUNT_FUNCTION_QUADRATURE_X2_A
* COUNT_FUNCTION_QUADRATURE_X2_B
Adding support in the future for "Quadrature x2 Rising" and "Quadrature x2 Falling" will be trivial, so really the main requirement in order to bring these modes back is to provide reasonable use cases for them. My suspicion is that there was some rationale for these Quadrature x2 modes in the STM32 LP-Timer -- afterall, why else would the engineers go through the trouble of designing and implementing it -- but until that use case is clear, it's best to wait on changing the Generic Counter ABI lest we end up with an interface that is never used in the real world.
William Breathitt Gray
Quoted 15 lines
Quoted 15 lines
Looks rather like the sort of thing that is a side effect of the implementation rather than deliberate.
Quoted 5 lines
I'm leaving in these modes for now, as they do exist in the STM32 LP Timer, but it does make me curious what the intentions for them were (perhaps use cases outside of traditional quadrature encoder positioning).
Sure if there is a usecase then fair enough.
Jonathan
PATCH v6 4/9 counter: 104-quad-8: Add Generic Counter interface support
In reply to William Breathitt GrayThis patch adds support for the Generic Counter interface to the 104-QUAD-8 driver. The existing 104-QUAD-8 device interface should not be affected by this patch; all changes are intended as supplemental additions as perceived by the user.
Generic Counter Counts are created for the eight quadrature channel counts, as well as their respective quadrature A and B Signals (which are associated via respective Synapse structures) and respective index Signals.
The new Generic Counter interface sysfs attributes are intended to expose the same functionality and data available via the existing 104-QUAD-8 IIO device interface; the Generic Counter interface serves to provide the respective functionality and data in a standard way expected of counter devices.
--- MAINTAINERS | 4 +- drivers/counter/104-quad-8.c | 1335 ++++++++++++++++++++++++++++++ drivers/counter/Kconfig | 21 + drivers/counter/Makefile | 2 + drivers/iio/counter/104-quad-8.c | 596 ------------- drivers/iio/counter/Kconfig | 17 - drivers/iio/counter/Makefile | 1 - 7 files changed, 1360 insertions(+), 616 deletions(-) create mode 100644 drivers/counter/104-quad-8.c delete mode 100644 drivers/iio/counter/104-quad-8.c
MAINTAINERS +2 -2
@@ -266,12 +266,12 @@ L: linux-gpio at vger.kernel.org S: Maintained F: drivers/gpio/gpio-104-idio-16.c -ACCES 104-QUAD-8 IIO DRIVER +ACCES 104-QUAD-8 DRIVER M: William Breathitt Gray <[email protected]> L: linux-iio at vger.kernel.org S: Maintained F: Documentation/ABI/testing/sysfs-bus-iio-counter-104-quad-8 -F: drivers/iio/counter/104-quad-8.c +F: drivers/counter/104-quad-8.c ACCES PCI-IDIO-16 GPIO DRIVER M: William Breathitt Gray <[email protected]>
drivers/counter/104-quad-8.c new file +1335 -0
1,335 lines not on this page · raw message
drivers/counter/Kconfig +21 -0
@@ -16,3 +16,24 @@ menuconfig COUNTER consumption. The Generic Counter interface enables drivers to support and expose a common set of components and functionality present in counter devices. + +if COUNTER + +config 104_QUAD_8 + tristate "ACCES 104-QUAD-8 driver" + depends on PC104 && X86 && IIO + select ISA_BUS_API + help + Say yes here to build support for the ACCES 104-QUAD-8 quadrature + encoder counter/interface device family (104-QUAD-8, 104-QUAD-4). + + Performing a write to a counter's IIO_CHAN_INFO_RAW sets the counter and + also clears the counter's respective error flag. Although the counters + have a 25-bit range, only the lower 24 bits may be set, either directly + or via a counter's preset attribute. Interrupts are not supported by + this driver. + + The base port addresses for the devices may be configured via the base + array module parameter. + +endif # COUNTER
drivers/counter/Makefile +2 -0
@@ -6,3 +6,5 @@ obj-$(CONFIG_COUNTER) += counter.o counter-y := generic-counter.o + +obj-$(CONFIG_104_QUAD_8) += 104-quad-8.o
drivers/iio/counter/104-quad-8.c deleted +0 -596
596 lines not on this page · raw message
drivers/iio/counter/Kconfig +0 -17
@@ -5,23 +5,6 @@ menu "Counters" -config 104_QUAD_8 - tristate "ACCES 104-QUAD-8 driver" - depends on PC104 && X86 - select ISA_BUS_API - help - Say yes here to build support for the ACCES 104-QUAD-8 quadrature - encoder counter/interface device family (104-QUAD-8, 104-QUAD-4). - - Performing a write to a counter's IIO_CHAN_INFO_RAW sets the counter and - also clears the counter's respective error flag. Although the counters - have a 25-bit range, only the lower 24 bits may be set, either directly - or via a counter's preset attribute. Interrupts are not supported by - this driver. - - The base port addresses for the devices may be configured via the base - array module parameter. - config STM32_LPTIMER_CNT tristate "STM32 LP Timer encoder counter driver" depends on MFD_STM32_LPTIMER || COMPILE_TEST
drivers/iio/counter/Makefile +0 -1
@@ -4,5 +4,4 @@ # When adding new entries keep the list in alphabetical order -obj-$(CONFIG_104_QUAD_8) += 104-quad-8.o obj-$(CONFIG_STM32_LPTIMER_CNT) += stm32-lptimer-cnt.o
Signature · 1 line
2.17.0
kbuild test robot
build bot[RFC PATCH] counter: 104-quad-8: quad8_ops can be static
Fixes: b3822048aa0b ("counter: 104-quad-8: Add Generic Counter interface support")
--- 104-quad-8.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/counter/104-quad-8.c +1 -1
@@ -800,7 +800,7 @@ static int quad8_action_get(struct counter_device *counter,return 0 ; } -const struct counter_ops quad8_ops = { +static const struct counter_ops quad8_ops = { .signal_read = quad8_signal_read, .count_read = quad8_count_read, .count_write = quad8_count_write,
kbuild test robot
build botRe: [PATCH v6 4/9] counter: 104-quad-8: Add Generic Counter interface support
In reply to William Breathitt GrayI love your patch! Perhaps something to improve:
Hi William,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v4.17-rc5 next-20180517]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/William-Breathitt-Gray/Introduce-the-Counter-subsystem/20180519-151353
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__sparse warnings: (new ones prefixed by >>)
>> drivers/counter/104-quad-8.c:803:26: sparse: symbol 'quad8_ops' was not declared. Should it be static?
Please review and possibly fold the followup patch.
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Re: [PATCH v6 4/9] counter: 104-quad-8: Add Generic Counter interface support
Quoted 15 lines · kbuild test robot
Hi William,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v4.17-rc5 next-20180517]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]url: https://github.com/0day-ci/linux/commits/William-Breathitt-Gray/Introduce-the-Counter-subsystem/20180519-151353
reproduce:# apt-get install sparse make ARCH=x86_64 allmodconfig make C=1 CF=-D__CHECK_ENDIAN__
Quoted 5 lines
sparse warnings: (new ones prefixed by >>)
Quoted 1 line
Quoted 1 line
drivers/counter/104-quad-8.c:803:26: sparse: symbol 'quad8_ops' was not declared. Should it be static?Please review and possibly fold the followup patch.
Ah, yes it should be static -- looks like I made a minor mistake. I'll squash the fix into the next revision of this patchset.
Thank you,
William Breathitt Gray
Quoted 4 lines
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Jonathan Cameron
maintainerRe: [PATCH v6 4/9] counter: 104-quad-8: Add Generic Counter interface support
In reply to William Breathitt GrayQuoted 10 lines · On Wed, 16 May 2018 13:51:25 -0400 William Breathitt Gray…
This patch adds support for the Generic Counter interface to the 104-QUAD-8 driver. The existing 104-QUAD-8 device interface should not be affected by this patch; all changes are intended as supplemental additions as perceived by the user.
Generic Counter Counts are created for the eight quadrature channel counts, as well as their respective quadrature A and B Signals (which are associated via respective Synapse structures) and respective index Signals.
Quoted 7 lines
The new Generic Counter interface sysfs attributes are intended to expose the same functionality and data available via the existing 104-QUAD-8 IIO device interface; the Generic Counter interface serves to provide the respective functionality and data in a standard way expected of counter devices.
Signed-off-by William Breathitt Gray <[email protected]>
A few general comments that applied just as well to the original driver as they do to the modified version.
I wonder if this would be easier to review as two patches. Move the driver then add the counter interfaces?
Right now people kind of have to review the old IIO driver and all the new stuff which is a big job..
Jonathan
Quoted 37 lines
--- MAINTAINERS | 4 +- drivers/counter/104-quad-8.c | 1335 ++++++++++++++++++++++++++++++ drivers/counter/Kconfig | 21 + drivers/counter/Makefile | 2 + drivers/iio/counter/104-quad-8.c | 596 ------------- drivers/iio/counter/Kconfig | 17 - drivers/iio/counter/Makefile | 1 - 7 files changed, 1360 insertions(+), 616 deletions(-) create mode 100644 drivers/counter/104-quad-8.c delete mode 100644 drivers/iio/counter/104-quad-8.cMAINTAINERS +2
-2@@ -266,12 +266,12 @@ L: linux-gpio at vger.kernel.org S: Maintained F: drivers/gpio/gpio-104-idio-16.c -ACCES 104-QUAD-8 IIO DRIVER +ACCES 104-QUAD-8 DRIVER M: William Breathitt Gray <[email protected]> L: linux-iio at vger.kernel.org S: Maintained F: Documentation/ABI/testing/sysfs-bus-iio-counter-104-quad-8 -F: drivers/iio/counter/104-quad-8.c +F: drivers/counter/104-quad-8.c ACCES PCI-IDIO-16 GPIO DRIVER M: William Breathitt Gray <[email protected]>drivers/counter/104-quad-8.c new file +0
-0
drivers/counter/104-quad-8.c line 1
Quoted 1 line
drivers/counter/104-quad-8.c new file +1
-0@@ -0,0 +1,1335 @@ +// SPDX-License-Identifier: GPL-2.0-only
If you are happy with SPDX drop the GPL text below to keep things short.
Quoted 16 lines
drivers/counter/104-quad-8.c new file +16
-0+/* +* IIO driver for the ACCES 104-QUAD-8 +* Copyright (C) 2016 William Breathitt Gray +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License, version 2, as +* published by the Free Software Foundation. +* +* This program is distributed in the hope that it will be useful, but +* WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +* General Public License for more details. +* +* This driver supports the ACCES 104-QUAD-8 and ACCES 104-QUAD-4. +*/ +#include <linux/bitops.h>
...
Quoted 36 lines
drivers/counter/104-quad-8.c new file +36
-0+static int quad8_probe (struct device *dev,unsigned int id) +{ +struct iio_dev *indio_dev; +struct quad8_iio *quad8iio; +int i, j; +unsigned int base_offset; +int err; + +if (!devm_request_region (dev, base[id],QUAD8_EXTENT ,dev_name (dev))) { +dev_err (dev,"Unable to lock port addresses (0x%X-0x%X)\n" , + base[id], base[id] +QUAD8_EXTENT ); +return -EBUSY ; + } + +/* Allocate IIO device; this also allocates driver data structure */ + indio_dev =devm_iio_device_alloc (dev,sizeof (*quad8iio)); +if (!indio_dev) +return -ENOMEM ; + +/* Initialize IIO device */ + indio_dev->info = &quad8_info; + indio_dev->modes =INDIO_DIRECT_MODE ; + indio_dev->num_channels =ARRAY_SIZE (quad8_channels); + indio_dev->channels = quad8_channels; + indio_dev->name =dev_name (dev); + indio_dev->dev.parent = dev; + +/* Initialize Counter device and driver data */ + quad8iio =iio_priv (indio_dev); + quad8iio->counter.name =dev_name (dev); + quad8iio->counter.parent = dev; + quad8iio->counter.ops = &quad8_ops; + quad8iio->counter.counts = quad8_counts; + quad8iio->counter.num_counts =ARRAY_SIZE (quad8_counts); + quad8iio->counter.signals = quad8_signals; + quad8iio->counter.num_signals =ARRAY_SIZE (quad8_signals);
drivers/counter/104-quad-8.c line 1297
Quoted 10 lines
drivers/counter/104-quad-8.c new file +10
-0+ quad8iio->counter.priv = quad8iio; + quad8iio->base = base[id]; + +/* Reset all counters and disable interrupt function */ +outb (0x01 , base[id] +0x11 ); +/* Set initial configuration for all counters */ +for (i =0 ; i <QUAD8_NUM_COUNTERS ; i++) { + base_offset = base[id] +2 * i; +/* Reset Byte Pointer */ +outb (0x01 , base_offset +1 );
I'm going to be fussy. There are lots of values
in here that look like register bits and you could exchange much of this documentation for a some good defines...
Taking base_offset + 1 bits 5 and 6 look to select the actual register and the rest of them do the control.
Anyhow, not critical but the readability of this code could be improved somewhat.
Quoted 52 lines
drivers/counter/104-quad-8.c new file +38
-0+/* Reset Preset Register */ +for (j =0 ; j <3 ; j++) +outb (0x00 , base_offset); +/* Reset Borrow, Carry, Compare, and Sign flags */ +outb (0x04 , base_offset +1 ); +/* Reset Error flag */ +outb (0x06 , base_offset +1 ); +/* Binary encoding; Normal count; non-quadrature mode */ +outb (0x20 , base_offset +1 ); +/* Disable A and B inputs; preset on index; FLG1 as Carry */ +outb (0x40 , base_offset +1 ); +/* Disable index function; negative index polarity */ +outb (0x60 , base_offset +1 ); + } +/* Enable all counters */ +outb (0x00 , base[id] +0x11 ); + +/* Register IIO device */ + err =devm_iio_device_register (dev, indio_dev); +if (err) +return err; + +/* Register Counter device */ +return devm_counter_register (dev, &quad8iio->counter); +} + +static struct isa_driver quad8_driver = { + .probe = quad8_probe, + .driver = { + .name ="104-quad-8" + } +}; + +module_isa_driver (quad8_driver, num_quad8); + +MODULE_AUTHOR ("William Breathitt Gray <[email protected]>" ); +MODULE_DESCRIPTION ("ACCES 104-QUAD-8 IIO driver" ); +MODULE_LICENSE ("GPL v2" );drivers/counter/Kconfig +6
-0@@ -16,3 +16,24 @@ menuconfig COUNTER consumption. The Generic Counter interface enables drivers to support and expose a common set of components and functionality present in counter devices. + +if COUNTER + +config 104_QUAD_8 + tristate "ACCES 104-QUAD-8 driver" + depends on PC104 && X86 && IIO
drivers/counter/Kconfig menuconfig COUNTER · line 34
Quoted 10 lines
drivers/counter/Kconfig +10
-0+ select ISA_BUS_API + help + Say yes here to build support for the ACCES 104-QUAD-8 quadrature + encoder counter/interface device family (104-QUAD-8, 104-QUAD-4). + + Performing a write to a counter's IIO_CHAN_INFO_RAW sets the counter and + also clears the counter's respective error flag. Although the counters + have a 25-bit range, only the lower 24 bits may be set, either directly + or via a counter's preset attribute. Interrupts are not supported by + this driver.
This text probably wants to be updated to reflect the new counter subsystem support..
Quoted 16 lines
drivers/counter/Kconfig +5
-0+ + The base port addresses for the devices may be configured via the base + array module parameter. + +endif # COUNTERdrivers/counter/Makefile +2
-0@@ -6,3 +6,5 @@ obj-$(CONFIG_COUNTER) += counter.o counter-y := generic-counter.o + +obj-$(CONFIG_104_QUAD_8) += 104-quad-8.odrivers/iio/counter/104-quad-8.c +0
-0
...
Re: [PATCH v6 4/9] counter: 104-quad-8: Add Generic Counter interface support
Quoted 24 lines · Jonathan Cameron
Quoted 10 lines · On Wed, 16 May 2018 13:51:25 -0400 William Breathitt Gray…
This patch adds support for the Generic Counter interface to the 104-QUAD-8 driver. The existing 104-QUAD-8 device interface should not be affected by this patch; all changes are intended as supplemental additions as perceived by the user.
Generic Counter Counts are created for the eight quadrature channel counts, as well as their respective quadrature A and B Signals (which are associated via respective Synapse structures) and respective index Signals.
Quoted 7 lines
The new Generic Counter interface sysfs attributes are intended to expose the same functionality and data available via the existing 104-QUAD-8 IIO device interface; the Generic Counter interface serves to provide the respective functionality and data in a standard way expected of counter devices.
Signed-off-by William Breathitt Gray <[email protected]>A few general comments that applied just as well to the original driver as they do to the modified version.
Quoted 7 lines
I wonder if this would be easier to review as two patches. Move the driver then add the counter interfaces?
Right now people kind of have to review the old IIO driver and all the new stuff which is a big job..
Jonathan
This looks like more simple cleanup, so I expect to incorporate your suggestions without problem here as well. I'll also try to make the code easier to read by writing some defines for the magic numbers throughout.
William Breathitt Gray
Quoted 198 lines
Quoted 37 lines
--- MAINTAINERS | 4 +- drivers/counter/104-quad-8.c | 1335 ++++++++++++++++++++++++++++++ drivers/counter/Kconfig | 21 + drivers/counter/Makefile | 2 + drivers/iio/counter/104-quad-8.c | 596 ------------- drivers/iio/counter/Kconfig | 17 - drivers/iio/counter/Makefile | 1 - 7 files changed, 1360 insertions(+), 616 deletions(-) create mode 100644 drivers/counter/104-quad-8.c delete mode 100644 drivers/iio/counter/104-quad-8.cMAINTAINERS +2
-2@@ -266,12 +266,12 @@ L: linux-gpio at vger.kernel.org S: Maintained F: drivers/gpio/gpio-104-idio-16.c -ACCES 104-QUAD-8 IIO DRIVER +ACCES 104-QUAD-8 DRIVER M: William Breathitt Gray <[email protected]> L: linux-iio at vger.kernel.org S: Maintained F: Documentation/ABI/testing/sysfs-bus-iio-counter-104-quad-8 -F: drivers/iio/counter/104-quad-8.c +F: drivers/counter/104-quad-8.c ACCES PCI-IDIO-16 GPIO DRIVER M: William Breathitt Gray <[email protected]>drivers/counter/104-quad-8.c new file +0
-0drivers/counter/104-quad-8.c line 1
Quoted 1 line
drivers/counter/104-quad-8.c new file +1
-0@@ -0,0 +1,1335 @@ +// SPDX-License-Identifier: GPL-2.0-only If you are happy with SPDX drop the GPL text below to keep things short.
Quoted 16 lines
drivers/counter/104-quad-8.c new file +16
-0+/* +* IIO driver for the ACCES 104-QUAD-8 +* Copyright (C) 2016 William Breathitt Gray +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License, version 2, as +* published by the Free Software Foundation. +* +* This program is distributed in the hope that it will be useful, but +* WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +* General Public License for more details. +* +* This driver supports the ACCES 104-QUAD-8 and ACCES 104-QUAD-4. +*/ +#include <linux/bitops.h> ...
Quoted 36 lines
drivers/counter/104-quad-8.c new file +36
-0+static int quad8_probe (struct device *dev,unsigned int id) +{ +struct iio_dev *indio_dev; +struct quad8_iio *quad8iio; +int i, j; +unsigned int base_offset; +int err; + +if (!devm_request_region (dev, base[id],QUAD8_EXTENT ,dev_name (dev))) { +dev_err (dev,"Unable to lock port addresses (0x%X-0x%X)\n" , + base[id], base[id] +QUAD8_EXTENT ); +return -EBUSY ; + } + +/* Allocate IIO device; this also allocates driver data structure */ + indio_dev =devm_iio_device_alloc (dev,sizeof (*quad8iio)); +if (!indio_dev) +return -ENOMEM ; + +/* Initialize IIO device */ + indio_dev->info = &quad8_info; + indio_dev->modes =INDIO_DIRECT_MODE ; + indio_dev->num_channels =ARRAY_SIZE (quad8_channels); + indio_dev->channels = quad8_channels; + indio_dev->name =dev_name (dev); + indio_dev->dev.parent = dev; + +/* Initialize Counter device and driver data */ + quad8iio =iio_priv (indio_dev); + quad8iio->counter.name =dev_name (dev); + quad8iio->counter.parent = dev; + quad8iio->counter.ops = &quad8_ops; + quad8iio->counter.counts = quad8_counts; + quad8iio->counter.num_counts =ARRAY_SIZE (quad8_counts); + quad8iio->counter.signals = quad8_signals; + quad8iio->counter.num_signals =ARRAY_SIZE (quad8_signals);drivers/counter/104-quad-8.c line 1297
Quoted 10 lines
drivers/counter/104-quad-8.c new file +10
-0+ quad8iio->counter.priv = quad8iio; + quad8iio->base = base[id]; + +/* Reset all counters and disable interrupt function */ +outb (0x01 , base[id] +0x11 ); +/* Set initial configuration for all counters */ +for (i =0 ; i <QUAD8_NUM_COUNTERS ; i++) { + base_offset = base[id] +2 * i; +/* Reset Byte Pointer */ +outb (0x01 , base_offset +1 );I'm going to be fussy. There are lots of values
in here that look like register bits and you could exchange much of this documentation for a some good defines...Taking
base_offset+ 1 bits 5 and 6 look to select the actual register and the rest of them do the control.Anyhow, not critical but the readability of this code could be improved somewhat.
Quoted 52 lines
drivers/counter/104-quad-8.c new file +38
-0+/* Reset Preset Register */ +for (j =0 ; j <3 ; j++) +outb (0x00 , base_offset); +/* Reset Borrow, Carry, Compare, and Sign flags */ +outb (0x04 , base_offset +1 ); +/* Reset Error flag */ +outb (0x06 , base_offset +1 ); +/* Binary encoding; Normal count; non-quadrature mode */ +outb (0x20 , base_offset +1 ); +/* Disable A and B inputs; preset on index; FLG1 as Carry */ +outb (0x40 , base_offset +1 ); +/* Disable index function; negative index polarity */ +outb (0x60 , base_offset +1 ); + } +/* Enable all counters */ +outb (0x00 , base[id] +0x11 ); + +/* Register IIO device */ + err =devm_iio_device_register (dev, indio_dev); +if (err) +return err; + +/* Register Counter device */ +return devm_counter_register (dev, &quad8iio->counter); +} + +static struct isa_driver quad8_driver = { + .probe = quad8_probe, + .driver = { + .name ="104-quad-8" + } +}; + +module_isa_driver (quad8_driver, num_quad8); + +MODULE_AUTHOR ("William Breathitt Gray <[email protected]>" ); +MODULE_DESCRIPTION ("ACCES 104-QUAD-8 IIO driver" ); +MODULE_LICENSE ("GPL v2" );drivers/counter/Kconfig +6
-0@@ -16,3 +16,24 @@ menuconfig COUNTER consumption. The Generic Counter interface enables drivers to support and expose a common set of components and functionality present in counter devices. + +if COUNTER + +config 104_QUAD_8 + tristate "ACCES 104-QUAD-8 driver" + depends on PC104 && X86 && IIOdrivers/counter/Kconfig menuconfig COUNTER · line 34
Quoted 10 lines
drivers/counter/Kconfig +10
-0+ select ISA_BUS_API + help + Say yes here to build support for the ACCES 104-QUAD-8 quadrature + encoder counter/interface device family (104-QUAD-8, 104-QUAD-4). + + Performing a write to a counter's IIO_CHAN_INFO_RAW sets the counter and + also clears the counter's respective error flag. Although the counters + have a 25-bit range, only the lower 24 bits may be set, either directly + or via a counter's preset attribute. Interrupts are not supported by + this driver.This text probably wants to be updated to reflect the new counter subsystem support..
Quoted 16 lines
drivers/counter/Kconfig +5
-0+ + The base port addresses for the devices may be configured via the base + array module parameter. + +endif # COUNTERdrivers/counter/Makefile +2
-0@@ -6,3 +6,5 @@ obj-$(CONFIG_COUNTER) += counter.o counter-y := generic-counter.o + +obj-$(CONFIG_104_QUAD_8) += 104-quad-8.odrivers/iio/counter/104-quad-8.c +0
-0...
PATCH v6 5/9 counter: 104-quad-8: Documentation: Add Generic Counter sysfs documentation
In reply to William Breathitt GrayThis patch adds standard documentation for the Generic Counter interface userspace sysfs attributes of the 104-QUAD-8 driver.
--- .../ABI/testing/sysfs-bus-counter-104-quad-8 | 36 +++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 37 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-bus-counter-104-quad-8
Documentation/ABI/testing/sysfs-bus-counter-104-quad-8 new file +36 -0
@@ -0,0 +1,36 @@ +What: /sys/bus/counter/devices/counterX/signalY/index_polarity +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Active level of index input Signal Y; irrelevant in + non-synchronous load mode. + +What: /sys/bus/counter/devices/counterX/signalY/index_polarity_available +What: /sys/bus/counter/devices/counterX/signalY/synchronous_mode_available +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Discrete set of available values for the respective Signal Y + configuration are listed in this file. + +What: /sys/bus/counter/devices/counterX/signalY/synchronous_mode +KernelVersion: 4.18 +Contact: linux-iio at vger.kernel.org +Description: + Configure the counter associated with Signal Y for + non-synchronous or synchronous load mode. Synchronous load mode + cannot be selected in non-quadrature (Pulse-Direction) clock + mode. + + Non-synchronous: + A logic low level is the active level at this index + input. The index function (as enabled via preset_enable) + is performed directly on the active level of the index + input. + + Synchronous: + Intended for interfacing with encoder Index output in + quadrature clock mode. The active level is configured + via index_polarity. The index function (as enabled via + preset_enable) is performed synchronously with the + quadrature clock on the active level of the index input.
MAINTAINERS +1 -0
@@ -270,6 +270,7 @@ ACCES 104-QUAD-8 DRIVER M: William Breathitt Gray <[email protected]> L: linux-iio at vger.kernel.org S: Maintained +F: Documentation/ABI/testing/sysfs-bus-counter-104-quad-8 F: Documentation/ABI/testing/sysfs-bus-iio-counter-104-quad-8 F: drivers/counter/104-quad-8.c
Signature · 1 line
2.17.0
PATCH v6 6/9 dt-bindings: counter: Document stm32 quadrature encoder
In reply to William Breathitt GrayFrom: Benjamin Gaignard <[email protected]>
Add bindings for STM32 Timer quadrature encoder. It is a sub-node of STM32 Timer which implement the counter part of the hardware.
--- .../bindings/counter/stm32-timer-cnt.txt | 26 +++++++++++++++++++ .../devicetree/bindings/mfd/stm32-timers.txt | 7 +++++ 2 files changed, 33 insertions(+) create mode 100644 Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt
Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt new file +26 -0
@@ -0,0 +1,26 @@ +STMicroelectronics STM32 Timer quadrature encoder + +STM32 Timer provides quadrature encoder counter mode to detect +angular position and direction of rotary elements, +from IN1 and IN2 input signals. + +Must be a sub-node of an STM32 Timer device tree node. +See ../mfd/stm32-timers.txt for details about the parent node. + +Required properties: +- compatible: Must be "st,stm32-timer-counter". +- pinctrl-names: Set to "default". +- pinctrl-0: List of phandles pointing to pin configuration nodes, + to set IN1/IN2 pins in mode of operation for Low-Power + Timer input on external pin. + +Example: + timers at 40010000 { + compatible = "st,stm32-timers"; + ... + counter { + compatible = "st,stm32-timer-counter"; + pinctrl-names = "default"; + pinctrl-0 = <&tim1_in_pins>; + }; + };
Documentation/devicetree/bindings/mfd/stm32-timers.txt +7 -0
@@ -23,6 +23,7 @@ Optional parameters: Optional subnodes: - pwm: See ../pwm/pwm-stm32.txt - timer: See ../iio/timer/stm32-timer-trigger.txt +- counter: See ../counter/stm32-timer-cnt.txt Example: timers at 40010000 {
@@ -43,4 +44,10 @@ Example: compatible = "st,stm32-timer-trigger"; reg = <0>; }; + + counter { + compatible = "st,stm32-timer-counter"; + pinctrl-names = "default"; + pinctrl-0 = <&tim1_in_pins>; + }; };
Signature · 1 line
2.17.0
Re: [PATCH v6 6/9] dt-bindings: counter: Document stm32 quadrature encoder
Quoted 1 line · William Breathitt Gray
From: Benjamin Gaignard <[email protected]>
v6? Where's v1-v5?
Quoted 20 lines
Add bindings for STM32 Timer quadrature encoder. It is a sub-node of STM32 Timer which implement the counter part of the hardware.
Cc Rob Herring <[email protected]>Cc Mark Rutland <[email protected]>Signed-off-by Benjamin Gaignard <[email protected]>Signed-off-by William Breathitt Gray <[email protected]>--- .../bindings/counter/stm32-timer-cnt.txt | 26 +++++++++++++++++++ .../devicetree/bindings/mfd/stm32-timers.txt | 7 +++++ 2 files changed, 33 insertions(+) create mode 100644 Documentation/devicetree/bindings/counter/stm32-timer-cnt.txtDocumentation/devicetree/bindings/counter/stm32-timer-cnt.txt new file +0
-0
Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt line 3
Quoted 3 lines
Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt new file +3
-0@@ -0,0 +1,26 @@ +STMicroelectronics STM32 Timer quadrature encoder + +STM32 Timer provides quadrature encoder counter mode to detect
'mode' does not sound like a sub-block of the timers block.
Quoted 9 lines
Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt new file +9
-0+angular position and direction of rotary elements, +from IN1 and IN2 input signals. + +Must be a sub-node of an STM32 Timer device tree node. +See ../mfd/stm32-timers.txt for details about the parent node. + +Required properties: +- compatible: Must be "st,stm32-timer-counter". +- pinctrl-names: Set to "default".
Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt line 22
Quoted 10 lines
Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt new file +10
-0+- pinctrl-0: List of phandles pointing to pin configuration nodes, + to set IN1/IN2 pins in mode of operation for Low-Power + Timer input on external pin. + +Example: + timers at 40010000 { + compatible = "st,stm32-timers"; + ... + counter { + compatible = "st,stm32-timer-counter";
Is there only 1? How is the counter addressed?
Quoted 4 lines
Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt new file +4
-0+ pinctrl-names = "default"; + pinctrl-0 = <&tim1_in_pins>; + }; + };
Re: [PATCH v6 6/9] dt-bindings: counter: Document stm32 quadrature encoder
Quoted 5 lines · Rob Herring
Quoted 1 line · William Breathitt Gray
From: Benjamin Gaignard <[email protected]>
v6? Where's v1-v5?
Hi Rob,
I apologize, I should have CC you on the rest of the patchset to give you a better idea of the context of this particular patch. Benjamin Gaignard authored this particular patch, so I'll leave it up to him to respond to your inline comments, but I can at least provide a brief history of the revisions of this patchset as a whole.
This patchset introduces a "Generic Counter" interface for drivers to support various counter devices (tally counters, rotary encoders, etc.). The v1 revision was submitted on 31 July 2017
(https://lkml.org/lkml/2017/7/31/514) as a wrapper over existing IIO functionality. This implementation design was unsuitable for the needs of Generic Counter API, so the v4 revision submitted on 14 December 2017 reimplemented the Generic Counter API as its own Counter subsystem (https://lkml.org/lkml/2017/12/14/778).
The v5 revision was submitted on 9 March 2018
(https://lkml.org/lkml/2018/3/9/728) and introduced the STM32 Timer quadrature encoder driver with Generic Counter interface support. I should have CC you in that revision to comment on the dt-bindings patch, but I overlooked it, so I made sure to CC you on this v6 revision.
William Breathitt Gray
Quoted 53 lines
Quoted 21 lines
Add bindings for STM32 Timer quadrature encoder. It is a sub-node of STM32 Timer which implement the counter part of the hardware.
Cc Rob Herring <[email protected]>Cc Mark Rutland <[email protected]>Signed-off-by Benjamin Gaignard <[email protected]>Signed-off-by William Breathitt Gray <[email protected]>--- .../bindings/counter/stm32-timer-cnt.txt | 26 +++++++++++++++++++ .../devicetree/bindings/mfd/stm32-timers.txt | 7 +++++ 2 files changed, 33 insertions(+) create mode 100644 Documentation/devicetree/bindings/counter/stm32-timer-cnt.txtDocumentation/devicetree/bindings/counter/stm32-timer-cnt.txt new file +0
-0Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt line 3
Quoted 3 lines
Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt new file +3
-0@@ -0,0 +1,26 @@ +STMicroelectronics STM32 Timer quadrature encoder + +STM32 Timer provides quadrature encoder counter mode to detect'mode' does not sound like a sub-block of the timers block.
Quoted 9 lines
Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt new file +9
-0+angular position and direction of rotary elements, +from IN1 and IN2 input signals. + +Must be a sub-node of an STM32 Timer device tree node. +See ../mfd/stm32-timers.txt for details about the parent node. + +Required properties: +- compatible: Must be "st,stm32-timer-counter". +- pinctrl-names: Set to "default".Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt line 22
Quoted 10 lines
Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt new file +10
-0+- pinctrl-0: List of phandles pointing to pin configuration nodes, + to set IN1/IN2 pins in mode of operation for Low-Power + Timer input on external pin. + +Example: + timers at 40010000 { + compatible = "st,stm32-timers"; + ... + counter { + compatible = "st,stm32-timer-counter";Is there only 1? How is the counter addressed?
Quoted 4 lines
Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt new file +4
-0+ pinctrl-names = "default"; + pinctrl-0 = <&tim1_in_pins>; + }; + };
Re: [PATCH v6 6/9] dt-bindings: counter: Document stm32 quadrature encoder
In reply to Rob Herring2018-05-17 18:23 GMT+02:00 Rob Herring <[email protected]>:
Quoted 20 lines
Quoted 1 line · William Breathitt Gray
From: Benjamin Gaignard <[email protected]>
v6? Where's v1-v5?
Quoted 14 lines
Add bindings for STM32 Timer quadrature encoder. It is a sub-node of STM32 Timer which implement the counter part of the hardware.
Cc Rob Herring <[email protected]>Cc Mark Rutland <[email protected]>Signed-off-by Benjamin Gaignard <[email protected]>Signed-off-by William Breathitt Gray <[email protected]>--- .../bindings/counter/stm32-timer-cnt.txt | 26 +++++++++++++++++++ .../devicetree/bindings/mfd/stm32-timers.txt | 7 +++++ 2 files changed, 33 insertions(+) create mode 100644 Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt
Quoted 11 lines
Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt line 3
Quoted 9 lines
Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt new file +3
-0@@ -0,0 +1,26 @@ +STMicroelectronics STM32 Timer quadrature encoder + +STM32 Timer provides quadrature encoder counter mode to detect'mode' does not sound like a sub-block of the timers block.
quadrature encoding is one of the counting modes of this hardware block which is enable to count on other signals/triggers
Quoted 14 lines
Quoted 14 lines
Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt new file +13
-0+angular position and direction of rotary elements, +from IN1 and IN2 input signals. + +Must be a sub-node of an STM32 Timer device tree node. +See ../mfd/stm32-timers.txt for details about the parent node. + +Required properties: +- compatible: Must be "st,stm32-timer-counter". +- pinctrl-names: Set to "default". +- pinctrl-0: List of phandles pointing to pin configuration nodes, + to set IN1/IN2 pins in mode of operation for Low-Power + Timer input on external pin. +
Quoted 8 lines
Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt line 22
Quoted 6 lines
Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt new file +6
-0+Example: + timers at 40010000 { + compatible = "st,stm32-timers"; + ... + counter { + compatible = "st,stm32-timer-counter";Is there only 1? How is the counter addressed?
Yes there is only one counter per hardware block. Counter is addressed like the two others sub-nodes and the details about parent mode are describe in stm32-timers.txt Should I add them here too ? so example will be like that:
timers at 40010000 {
#address-cells = <1>;
#size-cells = <0>;
compatible = "st,stm32-timers";
reg = <0x40010000 0x400>;
clocks = <&rcc 0 160>;
clock-names = "int";
counter {
compatible = "st,stm32-timer-counter";
pinctrl-names = "default";
pinctrl-0 = <&tim1_in_pins>;
};};
Benjamin
Quoted 5 lines
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Rob Herring
maintainerRe: [PATCH v6 6/9] dt-bindings: counter: Document stm32 quadrature encoder
Quoted 33 lines · Benjamin Gaignard
2018-05-17 18:23 GMT+02:00 Rob Herring <[email protected]>:
Quoted 20 lines
Quoted 1 line · William Breathitt Gray
From: Benjamin Gaignard <[email protected]>
v6? Where's v1-v5?
Quoted 14 lines
Add bindings for STM32 Timer quadrature encoder. It is a sub-node of STM32 Timer which implement the counter part of the hardware.
Cc Rob Herring <[email protected]>Cc Mark Rutland <[email protected]>Signed-off-by Benjamin Gaignard <[email protected]>Signed-off-by William Breathitt Gray <[email protected]>--- .../bindings/counter/stm32-timer-cnt.txt | 26 +++++++++++++++++++ .../devicetree/bindings/mfd/stm32-timers.txt | 7 +++++ 2 files changed, 33 insertions(+) create mode 100644 Documentation/devicetree/bindings/counter/stm32-timer-cnt.txtQuoted 11 lines
Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt line 3
Quoted 9 lines
Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt new file +3
-0@@ -0,0 +1,26 @@ +STMicroelectronics STM32 Timer quadrature encoder + +STM32 Timer provides quadrature encoder counter mode to detect'mode' does not sound like a sub-block of the timers block.
Quoted 2 lines
quadrature encoding is one of the counting modes of this hardware block which is enable to count on other signals/triggers
You don't need a child node and compatible to set a mode.
Quoted 22 lines
Quoted 13 lines
Quoted 13 lines
Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt new file +13
-0+angular position and direction of rotary elements, +from IN1 and IN2 input signals. + +Must be a sub-node of an STM32 Timer device tree node. +See ../mfd/stm32-timers.txt for details about the parent node. + +Required properties: +- compatible: Must be "st,stm32-timer-counter". +- pinctrl-names: Set to "default". +- pinctrl-0: List of phandles pointing to pin configuration nodes, + to set IN1/IN2 pins in mode of operation for Low-Power + Timer input on external pin. +Quoted 8 lines
Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt line 22
Quoted 6 lines
Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt new file +6
-0+Example: + timers at 40010000 { + compatible = "st,stm32-timers"; + ... + counter { + compatible = "st,stm32-timer-counter";Is there only 1? How is the counter addressed?
Quoted 4 lines
Yes there is only one counter per hardware block. Counter is addressed like the two others sub-nodes and the details about parent mode are describe in stm32-timers.txt Should I add them here too ? so example will be like that:
No, you should drop the child node and add pinctrl to the parent.
Any other functions this block has that you plan on adding? Please make bindings as complete as possible, not what you currently have drivers for.
Quoted 20 lines
timers at 40010000 {
#address-cells = <1>;
#size-cells = <0>;
compatible = "st,stm32-timers";
reg = <0x400100000x400>;
clocks = <&rcc 0 160>;
clock-names = "int";counter { compatible = "st,stm32-timer-counter"; pinctrl-names = "default"; pinctrl-0 = <&tim1_in_pins>; };};
Benjamin
Quoted 5 lines
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Jonathan Cameron
maintainerRe: [PATCH v6 6/9] dt-bindings: counter: Document stm32 quadrature encoder
Quoted 34 lines · On Fri, 18 May 2018 11:28:15 -0500 Rob Herring <[email protected]> wrote:
Quoted 33 lines · Benjamin Gaignard
2018-05-17 18:23 GMT+02:00 Rob Herring <[email protected]>:
Quoted 31 lines
Quoted 1 line · William Breathitt Gray
From: Benjamin Gaignard <[email protected]>
v6? Where's v1-v5?
Quoted 20 lines
Add bindings for STM32 Timer quadrature encoder. It is a sub-node of STM32 Timer which implement the counter part of the hardware.
Cc Rob Herring <[email protected]>Cc Mark Rutland <[email protected]>Signed-off-by Benjamin Gaignard <[email protected]>Signed-off-by William Breathitt Gray <[email protected]>--- .../bindings/counter/stm32-timer-cnt.txt | 26 +++++++++++++++++++ .../devicetree/bindings/mfd/stm32-timers.txt | 7 +++++ 2 files changed, 33 insertions(+) create mode 100644 Documentation/devicetree/bindings/counter/stm32-timer-cnt.txtDocumentation/devicetree/bindings/counter/stm32-timer-cnt.txt new file +0
-0Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt line 3
Quoted 3 lines
Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt new file +3
-0@@ -0,0 +1,26 @@ +STMicroelectronics STM32 Timer quadrature encoder + +STM32 Timer provides quadrature encoder counter mode to detect'mode' does not sound like a sub-block of the timers block.
Quoted 4 lines
Quoted 2 lines
quadrature encoding is one of the counting modes of this hardware block which is enable to count on other signals/triggers
You don't need a child node and compatible to set a mode.
A pile of extra hardware becomes relevant and you only want to be in this state if you have appropriate external device wired up. In this case there is admittedly not a lot here but some devices will look a bit more like a touchscreen controller.
They are often build on top of an ADC module, but have a load of touch screen only signals and electrical elements that warrant being represented as a separate node in the DT.
Quoted 54 lines
Quoted 23 lines
Quoted 14 lines
Quoted 14 lines
Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt new file +13
-0+angular position and direction of rotary elements, +from IN1 and IN2 input signals. + +Must be a sub-node of an STM32 Timer device tree node. +See ../mfd/stm32-timers.txt for details about the parent node. + +Required properties: +- compatible: Must be "st,stm32-timer-counter". +- pinctrl-names: Set to "default". +- pinctrl-0: List of phandles pointing to pin configuration nodes, + to set IN1/IN2 pins in mode of operation for Low-Power + Timer input on external pin. +Quoted 8 lines
Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt line 22
Quoted 6 lines
Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt new file +6
-0+Example: + timers at 40010000 { + compatible = "st,stm32-timers"; + ... + counter { + compatible = "st,stm32-timer-counter";Is there only 1? How is the counter addressed?
Quoted 4 lines
Yes there is only one counter per hardware block. Counter is addressed like the two others sub-nodes and the details about parent mode are describe in stm32-timers.txt Should I add them here too ? so example will be like that:
No, you should drop the child node and add pinctrl to the parent.
Any other functions this block has that you plan on adding? Please make bindings as complete as possible, not what you currently have drivers for.
Quoted 20 lines
timers at 40010000 {
#address-cells = <1>;
#size-cells = <0>;
compatible = "st,stm32-timers";
reg = <0x400100000x400>;
clocks = <&rcc 0 160>;
clock-names = "int";counter { compatible = "st,stm32-timer-counter"; pinctrl-names = "default"; pinctrl-0 = <&tim1_in_pins>; };};
Benjamin
Quoted 5 lines
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH v6 6/9] dt-bindings: counter: Document stm32 quadrature encoder
In reply to Rob Herring2018-05-18 18:28 GMT+02:00 Rob Herring <[email protected]>:
Quoted 34 lines · Benjamin Gaignard
Quoted 33 lines
2018-05-17 18:23 GMT+02:00 Rob Herring <[email protected]>:
Quoted 31 lines
Quoted 1 line · William Breathitt Gray
From: Benjamin Gaignard <[email protected]>
v6? Where's v1-v5?
Quoted 20 lines
Add bindings for STM32 Timer quadrature encoder. It is a sub-node of STM32 Timer which implement the counter part of the hardware.
Cc Rob Herring <[email protected]>Cc Mark Rutland <[email protected]>Signed-off-by Benjamin Gaignard <[email protected]>Signed-off-by William Breathitt Gray <[email protected]>--- .../bindings/counter/stm32-timer-cnt.txt | 26 +++++++++++++++++++ .../devicetree/bindings/mfd/stm32-timers.txt | 7 +++++ 2 files changed, 33 insertions(+) create mode 100644 Documentation/devicetree/bindings/counter/stm32-timer-cnt.txtDocumentation/devicetree/bindings/counter/stm32-timer-cnt.txt new file +0
-0Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt line 3
Quoted 3 lines
Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt new file +3
-0@@ -0,0 +1,26 @@ +STMicroelectronics STM32 Timer quadrature encoder + +STM32 Timer provides quadrature encoder counter mode to detect'mode' does not sound like a sub-block of the timers block.
Quoted 4 lines
Quoted 2 lines
quadrature encoding is one of the counting modes of this hardware block which is enable to count on other signals/triggers
You don't need a child node and compatible to set a mode.
"mode" isn't the good word here because quadratic encoder enable a sub-block of this hardware.
Timer internal counter input could be internal or external clocks, some IIO triggers
or the output of the quadratic encoder sub-block.
It is a child like pwm or IIO trigger.
Quoted 28 lines
Quoted 23 lines
Quoted 14 lines
Quoted 14 lines
Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt new file +13
-0+angular position and direction of rotary elements, +from IN1 and IN2 input signals. + +Must be a sub-node of an STM32 Timer device tree node. +See ../mfd/stm32-timers.txt for details about the parent node. + +Required properties: +- compatible: Must be "st,stm32-timer-counter". +- pinctrl-names: Set to "default". +- pinctrl-0: List of phandles pointing to pin configuration nodes, + to set IN1/IN2 pins in mode of operation for Low-Power + Timer input on external pin. +Quoted 8 lines
Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt line 22
Quoted 6 lines
Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt new file +6
-0+Example: + timers at 40010000 { + compatible = "st,stm32-timers"; + ... + counter { + compatible = "st,stm32-timer-counter";Is there only 1? How is the counter addressed?
Quoted 4 lines
Yes there is only one counter per hardware block. Counter is addressed like the two others sub-nodes and the details about parent mode are describe in stm32-timers.txt Should I add them here too ? so example will be like that:
Quoted 5 lines
No, you should drop the child node and add pinctrl to the parent.
Any other functions this block has that you plan on adding? Please make bindings as complete as possible, not what you currently have drivers for.
Counter framework didn't exist when I pushed timer node but thanks to William's effort
it will allow us to use this kindf of hardware
Benjamin
Quoted 21 lines
Quoted 21 lines
timers at 40010000 {
#address-cells = <1>;
#size-cells = <0>;
compatible = "st,stm32-timers";
reg = <0x400100000x400>;
clocks = <&rcc 0 160>;
clock-names = "int";counter { compatible = "st,stm32-timer-counter"; pinctrl-names = "default"; pinctrl-0 = <&tim1_in_pins>; };};
Benjamin
Quoted 5 lines
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Signature · 7 lines
Benjamin Gaignard Graphic Study Group Linaro.org ? Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
PATCH v6 7/9 counter: Add STM32 Timer quadrature encoder
In reply to William Breathitt GrayFrom: Benjamin Gaignard <[email protected]>
Implement counter part of the STM32 timer hardware block by using counter API. Hardware only supports X2 and X4 quadrature modes. A ceiling value can be set to define the maximum value reachable by the counter.
--- drivers/counter/Kconfig | 10 + drivers/counter/Makefile | 1 + drivers/counter/stm32-timer-cnt.c | 390 ++++++++++++++++++++++++++++++ 3 files changed, 401 insertions(+) create mode 100644 drivers/counter/stm32-timer-cnt.c
drivers/counter/Kconfig +10 -0
@@ -36,4 +36,14 @@ config 104_QUAD_8 The base port addresses for the devices may be configured via the base array module parameter. +config STM32_TIMER_CNT + tristate "STM32 Timer encoder counter driver" + depends on MFD_STM32_TIMERS || COMPILE_TEST + help + Select this option to enable STM32 Timer quadrature encoder + and counter driver. + + To compile this driver as a module, choose M here: the + module will be called stm32-timer-cnt. + endif # COUNTER
drivers/counter/Makefile +1 -0
@@ -8,3 +8,4 @@ obj-$(CONFIG_COUNTER) += counter.o counter-y := generic-counter.o obj-$(CONFIG_104_QUAD_8) += 104-quad-8.o +obj-$(CONFIG_STM32_TIMER_CNT) += stm32-timer-cnt.o
drivers/counter/stm32-timer-cnt.c new file +390 -0
390 lines not on this page · raw message
Signature · 1 line
2.17.0
PATCH v6 8/9 counter: stm32-lptimer: add counter device
In reply to William Breathitt GrayFrom: Fabrice Gasnier <[email protected]>
Add support for new counter device to stm32-lptimer.
---
.../{iio => }/counter/stm32-lptimer-cnt.txt | 0
.../devicetree/bindings/mfd/stm32-lptimer.txt | 2 +-
drivers/counter/Kconfig | 10 +
drivers/counter/Makefile | 1 +
drivers/counter/stm32-lptimer-cnt.c | 722 ++++++++++++++++++
drivers/iio/counter/Kconfig | 9 -
drivers/iio/counter/Makefile | 2 -
drivers/iio/counter/stm32-lptimer-cnt.c | 382 ---------
8 files changed, 734 insertions(+), 394 deletions(-)
rename Documentation/devicetree/bindings/{iio => }/counter/stm32-lptimer-cnt.txt (100%)
create mode 100644 drivers/counter/stm32-lptimer-cnt.c
delete mode 100644 drivers/iio/counter/stm32-lptimer-cnt.cDocumentation/devicetree/bindings/counter/stm32-lptimer-cnt.txt renamed from Documentation/devicetree/bindings/iio/counter/stm32-lptimer-cnt.txt +0 -0
Documentation/devicetree/bindings/mfd/stm32-lptimer.txt +1 -1
@@ -16,7 +16,7 @@ Required properties: Optional subnodes: - pwm: See ../pwm/pwm-stm32-lp.txt -- counter: See ../iio/timer/stm32-lptimer-cnt.txt +- counter: See ../counter/stm32-lptimer-cnt.txt - trigger: See ../iio/timer/stm32-lptimer-trigger.txt Example:
drivers/counter/Kconfig +10 -0
@@ -46,4 +46,14 @@ config STM32_TIMER_CNT To compile this driver as a module, choose M here: the module will be called stm32-timer-cnt. +config STM32_LPTIMER_CNT + tristate "STM32 LP Timer encoder counter driver" + depends on (MFD_STM32_LPTIMER || COMPILE_TEST) && IIO + help + Select this option to enable STM32 Low-Power Timer quadrature encoder + and counter driver. + + To compile this driver as a module, choose M here: the + module will be called stm32-lptimer-cnt. + endif # COUNTER
drivers/counter/Makefile +1 -0
@@ -9,3 +9,4 @@ counter-y := generic-counter.o obj-$(CONFIG_104_QUAD_8) += 104-quad-8.o obj-$(CONFIG_STM32_TIMER_CNT) += stm32-timer-cnt.o +obj-$(CONFIG_STM32_LPTIMER_CNT) += stm32-lptimer-cnt.o
drivers/counter/stm32-lptimer-cnt.c new file +722 -0
722 lines not on this page · raw message
drivers/iio/counter/Kconfig +0 -9
@@ -5,13 +5,4 @@ menu "Counters" -config STM32_LPTIMER_CNT - tristate "STM32 LP Timer encoder counter driver" - depends on MFD_STM32_LPTIMER || COMPILE_TEST - help - Select this option to enable STM32 Low-Power Timer quadrature encoder - and counter driver. - - To compile this driver as a module, choose M here: the - module will be called stm32-lptimer-cnt. endmenu
drivers/iio/counter/Makefile +0 -2
@@ -3,5 +3,3 @@ # # When adding new entries keep the list in alphabetical order - -obj-$(CONFIG_STM32_LPTIMER_CNT) += stm32-lptimer-cnt.o
drivers/iio/counter/stm32-lptimer-cnt.c deleted +0 -382
382 lines not on this page · raw message
Signature · 1 line
2.17.0
Rob Herring
maintainerRe: [PATCH v6 8/9] counter: stm32-lptimer: add counter device
Quoted 9 lines · William Breathitt Gray
From: Fabrice Gasnier <[email protected]>
Add support for new counter device to stm32-lptimer.
Signed-off-by Fabrice Gasnier <[email protected]>Signed-off-by William Breathitt Gray <[email protected]>--- .../{iio => }/counter/stm32-lptimer-cnt.txt | 0 .../devicetree/bindings/mfd/stm32-lptimer.txt | 2 +-
Please split bindings to separate patch.
Quoted 10 lines
drivers/counter/Kconfig | 10 +
drivers/counter/Makefile | 1 +
drivers/counter/stm32-lptimer-cnt.c | 722 ++++++++++++++++++
drivers/iio/counter/Kconfig | 9 -
drivers/iio/counter/Makefile | 2 -
drivers/iio/counter/stm32-lptimer-cnt.c | 382 ---------
8 files changed, 734 insertions(+), 394 deletions(-)
rename Documentation/devicetree/bindings/{iio => }/counter/stm32-lptimer-cnt.txt (100%)
create mode 100644 drivers/counter/stm32-lptimer-cnt.c
delete mode 100644 drivers/iio/counter/stm32-lptimer-cnt.cPATCH v6 9/9 iio: counter: Remove IIO counter subdirectory
In reply to William Breathitt GrayThis patch removes the IIO counter subdirectory which is now superceded by the Counter subsystem. Deprecation warnings are added to the documentation of the relevant IIO counter sysfs attributes.
--- Documentation/ABI/testing/sysfs-bus-iio | 8 ++++++++ .../ABI/testing/sysfs-bus-iio-counter-104-quad-8 | 16 ++++++++++++++++ drivers/iio/Kconfig | 1 - drivers/iio/Makefile | 1 - drivers/iio/counter/Kconfig | 8 -------- drivers/iio/counter/Makefile | 5 ----- 6 files changed, 24 insertions(+), 15 deletions(-) delete mode 100644 drivers/iio/counter/Kconfig delete mode 100644 drivers/iio/counter/Makefile
Documentation/ABI/testing/sysfs-bus-iio +8 -0
@@ -1637,6 +1637,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_raw KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Raw counter device counts from channel Y. For quadrature counters, multiplication by an available [Y]_scale results in the counts of a single quadrature signal phase from channel Y.
@@ -1645,6 +1647,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_indexY_raw KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Raw counter device index value from channel Y. This attribute provides an absolute positional reference (e.g. a pulse once per revolution) which may be used to home positional systems as
@@ -1654,6 +1658,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_count_count_direction_available KernelVersion: 4.12 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + A list of possible counting directions which are: - "up" : counter device is increasing. - "down": counter device is decreasing.
@@ -1662,4 +1668,6 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_count_direction KernelVersion: 4.12 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Raw counter device counters direction for channel Y.
Documentation/ABI/testing/sysfs-bus-iio-counter-104-quad-8 +16 -0
@@ -6,6 +6,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_index_synchronous_mode_available KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Discrete set of available values for the respective counter configuration are listed in this file.
@@ -13,6 +15,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_count_mode KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Count mode for channel Y. Four count modes are available: normal, range limit, non-recycle, and modulo-n. The preset value for channel Y is used by the count mode where required.
@@ -47,6 +51,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_noise_error KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Read-only attribute that indicates whether excessive noise is present at the channel Y count inputs in quadrature clock mode; irrelevant in non-quadrature clock mode.
@@ -55,6 +61,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_preset KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + If the counter device supports preset registers, the preset count for channel Y is provided by this attribute.
@@ -62,6 +70,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_quadrature_mode KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Configure channel Y counter for non-quadrature or quadrature clock mode. Selecting non-quadrature clock mode will disable synchronous load mode. In quadrature clock mode, the channel Y
@@ -83,6 +93,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_set_to_preset_on_index KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Whether to set channel Y counter with channel Y preset value when channel Y index input is active, or continuously count. Valid attribute values are boolean.
@@ -91,6 +103,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_indexY_index_polarity KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Active level of channel Y index input; irrelevant in non-synchronous load mode.
@@ -98,6 +112,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_indexY_synchronous_mode KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Configure channel Y counter for non-synchronous or synchronous load mode. Synchronous load mode cannot be selected in non-quadrature clock mode.
drivers/iio/Kconfig +0 -1
@@ -74,7 +74,6 @@ source "drivers/iio/afe/Kconfig" source "drivers/iio/amplifiers/Kconfig" source "drivers/iio/chemical/Kconfig" source "drivers/iio/common/Kconfig" -source "drivers/iio/counter/Kconfig" source "drivers/iio/dac/Kconfig" source "drivers/iio/dummy/Kconfig" source "drivers/iio/frequency/Kconfig"
drivers/iio/Makefile +0 -1
@@ -20,7 +20,6 @@ obj-y += amplifiers/ obj-y += buffer/ obj-y += chemical/ obj-y += common/ -obj-y += counter/ obj-y += dac/ obj-y += dummy/ obj-y += gyro/
drivers/iio/counter/Kconfig deleted +0 -8
@@ -1,8 +0,0 @@ -# -# Counter devices -# -# When adding new entries keep the list in alphabetical order - -menu "Counters" - -endmenu
drivers/iio/counter/Makefile deleted +0 -5
@@ -1,5 +0,0 @@ -# -# Makefile for IIO counter devices -# - -# When adding new entries keep the list in alphabetical order
Signature · 1 line
2.17.0
Re: [PATCH v6 9/9] iio: counter: Remove IIO counter subdirectory
Quoted 5 lines · On Wed, 16 May 2018 13:52:39 -0400 William Breathitt Gray…
This patch removes the IIO counter subdirectory which is now superceded by the Counter subsystem. Deprecation warnings are added to the documentation of the relevant IIO counter sysfs attributes.
Signed-off-by William Breathitt Gray <[email protected]>
Please drop the directory when it becomes empty rather than in a later patch. IIRC there are some issues with empty Makefiles that will make building inbetween tricky.
For the deprecated markings.
Quoted 174 lines
--- Documentation/ABI/testing/sysfs-bus-iio | 8 ++++++++ .../ABI/testing/sysfs-bus-iio-counter-104-quad-8 | 16 ++++++++++++++++ drivers/iio/Kconfig | 1 - drivers/iio/Makefile | 1 - drivers/iio/counter/Kconfig | 8 -------- drivers/iio/counter/Makefile | 5 ----- 6 files changed, 24 insertions(+), 15 deletions(-) delete mode 100644 drivers/iio/counter/Kconfig delete mode 100644 drivers/iio/counter/MakefileDocumentation/ABI/testing/sysfs-bus-iio +8
-0@@ -1637,6 +1637,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_raw KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Raw counter device counts from channel Y. For quadrature counters, multiplication by an available [Y]_scale results in the counts of a single quadrature signal phase from channel Y.@@ -1645,6 +1647,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_indexY_raw KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Raw counter device index value from channel Y. This attribute provides an absolute positional reference (e.g. a pulse once per revolution) which may be used to home positional systems as@@ -1654,6 +1658,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_count_count_direction_available KernelVersion: 4.12 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + A list of possible counting directions which are: - "up" : counter device is increasing. - "down": counter device is decreasing.@@ -1662,4 +1668,6 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_count_direction KernelVersion: 4.12 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Raw counter device counters direction for channel Y.Documentation/ABI/testing/sysfs-bus-iio-counter-104-quad-8 +16
-0@@ -6,6 +6,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_index_synchronous_mode_available KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Discrete set of available values for the respective counter configuration are listed in this file.@@ -13,6 +15,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_count_mode KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Count mode for channel Y. Four count modes are available: normal, range limit, non-recycle, and modulo-n. The preset value for channel Y is used by the count mode where required.@@ -47,6 +51,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_noise_error KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Read-only attribute that indicates whether excessive noise is present at the channel Y count inputs in quadrature clock mode; irrelevant in non-quadrature clock mode.@@ -55,6 +61,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_preset KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + If the counter device supports preset registers, the preset count for channel Y is provided by this attribute.@@ -62,6 +70,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_quadrature_mode KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Configure channel Y counter for non-quadrature or quadrature clock mode. Selecting non-quadrature clock mode will disable synchronous load mode. In quadrature clock mode, the channel Y@@ -83,6 +93,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_set_to_preset_on_index KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Whether to set channel Y counter with channel Y preset value when channel Y index input is active, or continuously count. Valid attribute values are boolean.@@ -91,6 +103,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_indexY_index_polarity KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Active level of channel Y index input; irrelevant in non-synchronous load mode.@@ -98,6 +112,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_indexY_synchronous_mode KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Configure channel Y counter for non-synchronous or synchronous load mode. Synchronous load mode cannot be selected in non-quadrature clock mode.drivers/iio/Kconfig +0
-1@@ -74,7 +74,6 @@ source "drivers/iio/afe/Kconfig" source "drivers/iio/amplifiers/Kconfig" source "drivers/iio/chemical/Kconfig" source "drivers/iio/common/Kconfig" -source "drivers/iio/counter/Kconfig" source "drivers/iio/dac/Kconfig" source "drivers/iio/dummy/Kconfig" source "drivers/iio/frequency/Kconfig"drivers/iio/Makefile +0
-1@@ -20,7 +20,6 @@ obj-y += amplifiers/ obj-y += buffer/ obj-y += chemical/ obj-y += common/ -obj-y += counter/ obj-y += dac/ obj-y += dummy/ obj-y += gyro/drivers/iio/counter/Kconfig deleted +0
-8@@ -1,8 +0,0 @@ -# -# Counter devices -# -# When adding new entries keep the list in alphabetical order - -menu "Counters" - -endmenudrivers/iio/counter/Makefile deleted +0
-5@@ -1,5 +0,0 @@ -# -# Makefile for IIO counter devices -# - -# When adding new entries keep the list in alphabetical order
Re: [PATCH v6 9/9] iio: counter: Remove IIO counter subdirectory
Quoted 9 lines · Jonathan Cameron
Quoted 5 lines · On Wed, 16 May 2018 13:52:39 -0400 William Breathitt Gray…
This patch removes the IIO counter subdirectory which is now superceded by the Counter subsystem. Deprecation warnings are added to the documentation of the relevant IIO counter sysfs attributes.
Signed-off-by William Breathitt Gray <[email protected]>
Quoted 7 lines
Please drop the directory when it becomes empty rather than in a later patch. IIRC there are some issues with empty Makefiles that will make building inbetween tricky.
For the deprecated markings.
Acked-by Jonathan Cameron <[email protected]>
I'll have the directory removal occur with the removal of the last module then when the directory becomes empty.
Regarding the deprecation markings, should I select a specific kernel version to date the removal of these attributes, or leave the future date open as this patch is now?
William Breathitt Gray
Quoted 176 lines
Quoted 175 lines
--- Documentation/ABI/testing/sysfs-bus-iio | 8 ++++++++ .../ABI/testing/sysfs-bus-iio-counter-104-quad-8 | 16 ++++++++++++++++ drivers/iio/Kconfig | 1 - drivers/iio/Makefile | 1 - drivers/iio/counter/Kconfig | 8 -------- drivers/iio/counter/Makefile | 5 ----- 6 files changed, 24 insertions(+), 15 deletions(-) delete mode 100644 drivers/iio/counter/Kconfig delete mode 100644 drivers/iio/counter/MakefileDocumentation/ABI/testing/sysfs-bus-iio +8
-0@@ -1637,6 +1637,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_raw KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Raw counter device counts from channel Y. For quadrature counters, multiplication by an available [Y]_scale results in the counts of a single quadrature signal phase from channel Y.@@ -1645,6 +1647,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_indexY_raw KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Raw counter device index value from channel Y. This attribute provides an absolute positional reference (e.g. a pulse once per revolution) which may be used to home positional systems as@@ -1654,6 +1658,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_count_count_direction_available KernelVersion: 4.12 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + A list of possible counting directions which are: - "up" : counter device is increasing. - "down": counter device is decreasing.@@ -1662,4 +1668,6 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_count_direction KernelVersion: 4.12 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Raw counter device counters direction for channel Y.Documentation/ABI/testing/sysfs-bus-iio-counter-104-quad-8 +16
-0@@ -6,6 +6,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_index_synchronous_mode_available KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Discrete set of available values for the respective counter configuration are listed in this file.@@ -13,6 +15,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_count_mode KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Count mode for channel Y. Four count modes are available: normal, range limit, non-recycle, and modulo-n. The preset value for channel Y is used by the count mode where required.@@ -47,6 +51,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_noise_error KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Read-only attribute that indicates whether excessive noise is present at the channel Y count inputs in quadrature clock mode; irrelevant in non-quadrature clock mode.@@ -55,6 +61,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_preset KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + If the counter device supports preset registers, the preset count for channel Y is provided by this attribute.@@ -62,6 +70,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_quadrature_mode KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Configure channel Y counter for non-quadrature or quadrature clock mode. Selecting non-quadrature clock mode will disable synchronous load mode. In quadrature clock mode, the channel Y@@ -83,6 +93,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_set_to_preset_on_index KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Whether to set channel Y counter with channel Y preset value when channel Y index input is active, or continuously count. Valid attribute values are boolean.@@ -91,6 +103,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_indexY_index_polarity KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Active level of channel Y index input; irrelevant in non-synchronous load mode.@@ -98,6 +112,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_indexY_synchronous_mode KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Configure channel Y counter for non-synchronous or synchronous load mode. Synchronous load mode cannot be selected in non-quadrature clock mode.drivers/iio/Kconfig +0
-1@@ -74,7 +74,6 @@ source "drivers/iio/afe/Kconfig" source "drivers/iio/amplifiers/Kconfig" source "drivers/iio/chemical/Kconfig" source "drivers/iio/common/Kconfig" -source "drivers/iio/counter/Kconfig" source "drivers/iio/dac/Kconfig" source "drivers/iio/dummy/Kconfig" source "drivers/iio/frequency/Kconfig"drivers/iio/Makefile +0
-1@@ -20,7 +20,6 @@ obj-y += amplifiers/ obj-y += buffer/ obj-y += chemical/ obj-y += common/ -obj-y += counter/ obj-y += dac/ obj-y += dummy/ obj-y += gyro/drivers/iio/counter/Kconfig deleted +0
-8@@ -1,8 +0,0 @@ -# -# Counter devices -# -# When adding new entries keep the list in alphabetical order - -menu "Counters" - -endmenudrivers/iio/counter/Makefile deleted +0
-5@@ -1,5 +0,0 @@ -# -# Makefile for IIO counter devices -# - -# When adding new entries keep the list in alphabetical order
Re: [PATCH v6 9/9] iio: counter: Remove IIO counter subdirectory
Quoted 16 lines · On Mon, 21 May 2018 09:58:28 -0400 William Breathitt Gray…
Quoted 15 lines · Jonathan Cameron
Quoted 5 lines · On Wed, 16 May 2018 13:52:39 -0400 William Breathitt Gray…
This patch removes the IIO counter subdirectory which is now superceded by the Counter subsystem. Deprecation warnings are added to the documentation of the relevant IIO counter sysfs attributes.
Signed-off-by William Breathitt Gray <[email protected]>Please drop the directory when it becomes empty rather than in a later patch. IIRC there are some issues with empty Makefiles that will make building inbetween tricky.
For the deprecated markings.
Quoted 8 lines
Quoted 1 line
Acked-by Jonathan Cameron <[email protected]>I'll have the directory removal occur with the removal of the last module then when the directory becomes empty.
Regarding the deprecation markings, should I select a specific kernel version to date the removal of these attributes, or leave the future date open as this patch is now?
Leave it open. The ultimate test is whether it is actually enough of a burden to make us want to clean up the deprecated interface.
This burden may be that it is a pain to implement something new, or to test or to maintain the code (or something I haven't thought of).
If it's not it may stay there for ever.
Jonathan
Quoted 183 lines
William Breathitt Gray
Quoted 167 lines
Quoted 167 lines
--- Documentation/ABI/testing/sysfs-bus-iio | 8 ++++++++ .../ABI/testing/sysfs-bus-iio-counter-104-quad-8 | 16 ++++++++++++++++ drivers/iio/Kconfig | 1 - drivers/iio/Makefile | 1 - drivers/iio/counter/Kconfig | 8 -------- drivers/iio/counter/Makefile | 5 ----- 6 files changed, 24 insertions(+), 15 deletions(-) delete mode 100644 drivers/iio/counter/Kconfig delete mode 100644 drivers/iio/counter/MakefileDocumentation/ABI/testing/sysfs-bus-iio +8
-0@@ -1637,6 +1637,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_raw KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Raw counter device counts from channel Y. For quadrature counters, multiplication by an available [Y]_scale results in the counts of a single quadrature signal phase from channel Y.@@ -1645,6 +1647,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_indexY_raw KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Raw counter device index value from channel Y. This attribute provides an absolute positional reference (e.g. a pulse once per revolution) which may be used to home positional systems as@@ -1654,6 +1658,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_count_count_direction_available KernelVersion: 4.12 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + A list of possible counting directions which are: - "up" : counter device is increasing. - "down": counter device is decreasing.@@ -1662,4 +1668,6 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_count_direction KernelVersion: 4.12 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Raw counter device counters direction for channel Y.Documentation/ABI/testing/sysfs-bus-iio-counter-104-quad-8 +16
-0@@ -6,6 +6,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_index_synchronous_mode_available KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Discrete set of available values for the respective counter configuration are listed in this file.@@ -13,6 +15,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_count_mode KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Count mode for channel Y. Four count modes are available: normal, range limit, non-recycle, and modulo-n. The preset value for channel Y is used by the count mode where required.@@ -47,6 +51,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_noise_error KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Read-only attribute that indicates whether excessive noise is present at the channel Y count inputs in quadrature clock mode; irrelevant in non-quadrature clock mode.@@ -55,6 +61,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_preset KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + If the counter device supports preset registers, the preset count for channel Y is provided by this attribute.@@ -62,6 +70,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_quadrature_mode KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Configure channel Y counter for non-quadrature or quadrature clock mode. Selecting non-quadrature clock mode will disable synchronous load mode. In quadrature clock mode, the channel Y@@ -83,6 +93,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_countY_set_to_preset_on_index KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Whether to set channel Y counter with channel Y preset value when channel Y index input is active, or continuously count. Valid attribute values are boolean.@@ -91,6 +103,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_indexY_index_polarity KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Active level of channel Y index input; irrelevant in non-synchronous load mode.@@ -98,6 +112,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_indexY_synchronous_mode KernelVersion: 4.10 Contact: linux-iio at vger.kernel.org Description: + This interface is deprecated; please use the Counter subsystem. + Configure channel Y counter for non-synchronous or synchronous load mode. Synchronous load mode cannot be selected in non-quadrature clock mode.drivers/iio/Kconfig +0
-1@@ -74,7 +74,6 @@ source "drivers/iio/afe/Kconfig" source "drivers/iio/amplifiers/Kconfig" source "drivers/iio/chemical/Kconfig" source "drivers/iio/common/Kconfig" -source "drivers/iio/counter/Kconfig" source "drivers/iio/dac/Kconfig" source "drivers/iio/dummy/Kconfig" source "drivers/iio/frequency/Kconfig"drivers/iio/Makefile +0
-1@@ -20,7 +20,6 @@ obj-y += amplifiers/ obj-y += buffer/ obj-y += chemical/ obj-y += common/ -obj-y += counter/ obj-y += dac/ obj-y += dummy/ obj-y += gyro/drivers/iio/counter/Kconfig deleted +0
-8@@ -1,8 +0,0 @@ -# -# Counter devices -# -# When adding new entries keep the list in alphabetical order - -menu "Counters" - -endmenudrivers/iio/counter/Makefile deleted +0
-0Quoted 9 lines
drivers/iio/counter/Makefile line 5
Quoted 8 lines
drivers/iio/counter/Makefile deleted +0
-5@@ -1,5 +0,0 @@ -# -# Makefile for IIO counter devices -# - -# When adding new entries keep the list in alphabetical order--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html