| Message ID | 4223302f61b77b82b3927bd3280d0df791418d76.1625471640.git.vilhelm.gray@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Jonathan Cameron |
| Headers | show |
| Series | Introduce the Counter character device interface | expand |
On 7/5/21 3:18 AM, William Breathitt Gray wrote: > The Counter subsystem architecture and driver implementations have > changed in order to handle Counter sysfs interactions in a more > consistent way. This patch updates the Generic Counter interface > header file comments to reflect the changes. > > Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com> > --- > drivers/counter/counter-core.c | 3 +++ > include/linux/counter.h | 43 ++++++++++++++++------------------ > 2 files changed, 23 insertions(+), 23 deletions(-) > > diff --git a/drivers/counter/counter-core.c b/drivers/counter/counter-core.c > index 15f735ef296e..9442e3b91468 100644 > --- a/drivers/counter/counter-core.c > +++ b/drivers/counter/counter-core.c > @@ -41,6 +41,9 @@ static struct bus_type counter_bus_type = { > * 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. > + * > + * RETURNS: > + * 0 on success, negative error number on failure. > */ > int counter_register(struct counter_device *const counter) > { > diff --git a/include/linux/counter.h b/include/linux/counter.h > index b69277f5c4c5..e7fd6d81a929 100644 > --- a/include/linux/counter.h > +++ b/include/linux/counter.h > @@ -188,11 +188,10 @@ struct counter_comp { > > /** > * 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 > + * @id: unique ID used to identify the Signal > + * @name: device-specific Signal name > + * @ext: optional array of Signal extensions > + * @num_ext: number of Signal extensions specified in @ext > */ > struct counter_signal { > int id; > @@ -206,7 +205,7 @@ struct counter_signal { > * struct counter_synapse - Counter Synapse node > * @actions_list: array of available action modes > * @num_actions: number of action modes specified in @actions_list > - * @signal: pointer to associated signal > + * @signal: pointer to the associated Signal > */ > struct counter_synapse { > const enum counter_synapse_action *actions_list; > @@ -217,15 +216,14 @@ struct counter_synapse { > > /** > * 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 > - * @functions_list: array available function modes > + * @id: unique ID used to identify the Count > + * @name: device-specific Count name > + * @functions_list: array of 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 > - * @num_ext: number of Counter Count extensions specified in @ext > + * @synapses: array of Synapses for initialization > + * @num_synapses: number of Synapses specified in @synapses > + * @ext: optional array of Count extensions > + * @num_ext: number of Count extensions specified in @ext > */ > struct counter_count { > int id; > @@ -243,15 +241,14 @@ struct counter_count { > > /** > * struct counter_ops - Callbacks from driver > - * @signal_read: optional read callback for Signal attribute. The read > - * level of the respective Signal should be passed back via > - * the level parameter. > - * @count_read: optional read callback for Count attribute. The read > - * value of the respective Count should be passed back via > - * the val parameter. Are these no longer optional? If they really are optional, it would be nice to keep that information in the description. > - * @count_write: optional write callback for Count attribute. The write > - * value for the respective Count is passed in via the val > + * @signal_read: read callback for Signals. The read level of the > + * respective Signal should be passed back via the level > + * parameter. > + * @count_read: read callback for Counts. The read value of the > + * respective Count should be passed back via the value > * parameter. > + * @count_write: write callback for Counts. The write value for the > + * respective Count is passed in via the value parameter. > * @function_read: read callback the Count function modes. The read > * function mode of the respective Count should be passed > * back via the function parameter. > @@ -291,7 +288,7 @@ struct counter_ops { > > /** > * struct counter_device - Counter data structure > - * @name: name of the device as it appears in the datasheet > + * @name: name of the device Is there a recommended naming convention if using the datasheet is no longer recommended? > * @parent: optional parent device providing the counters > * @ops: callbacks from driver > * @signals: array of Signals >
On Fri, Jul 09, 2021 at 12:49:20PM -0500, David Lechner wrote: > On 7/5/21 3:18 AM, William Breathitt Gray wrote: > > The Counter subsystem architecture and driver implementations have > > changed in order to handle Counter sysfs interactions in a more > > consistent way. This patch updates the Generic Counter interface > > header file comments to reflect the changes. > > > > Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com> > > --- > > drivers/counter/counter-core.c | 3 +++ > > include/linux/counter.h | 43 ++++++++++++++++------------------ > > 2 files changed, 23 insertions(+), 23 deletions(-) > > > > diff --git a/drivers/counter/counter-core.c b/drivers/counter/counter-core.c > > index 15f735ef296e..9442e3b91468 100644 > > --- a/drivers/counter/counter-core.c > > +++ b/drivers/counter/counter-core.c > > @@ -41,6 +41,9 @@ static struct bus_type counter_bus_type = { > > * 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. > > + * > > + * RETURNS: > > + * 0 on success, negative error number on failure. > > */ > > int counter_register(struct counter_device *const counter) > > { > > diff --git a/include/linux/counter.h b/include/linux/counter.h > > index b69277f5c4c5..e7fd6d81a929 100644 > > --- a/include/linux/counter.h > > +++ b/include/linux/counter.h > > @@ -188,11 +188,10 @@ struct counter_comp { > > > > /** > > * 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 > > + * @id: unique ID used to identify the Signal > > + * @name: device-specific Signal name > > + * @ext: optional array of Signal extensions > > + * @num_ext: number of Signal extensions specified in @ext > > */ > > struct counter_signal { > > int id; > > @@ -206,7 +205,7 @@ struct counter_signal { > > * struct counter_synapse - Counter Synapse node > > * @actions_list: array of available action modes > > * @num_actions: number of action modes specified in @actions_list > > - * @signal: pointer to associated signal > > + * @signal: pointer to the associated Signal > > */ > > struct counter_synapse { > > const enum counter_synapse_action *actions_list; > > @@ -217,15 +216,14 @@ struct counter_synapse { > > > > /** > > * 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 > > - * @functions_list: array available function modes > > + * @id: unique ID used to identify the Count > > + * @name: device-specific Count name > > + * @functions_list: array of 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 > > - * @num_ext: number of Counter Count extensions specified in @ext > > + * @synapses: array of Synapses for initialization > > + * @num_synapses: number of Synapses specified in @synapses > > + * @ext: optional array of Count extensions > > + * @num_ext: number of Count extensions specified in @ext > > */ > > struct counter_count { > > int id; > > @@ -243,15 +241,14 @@ struct counter_count { > > > > /** > > * struct counter_ops - Callbacks from driver > > - * @signal_read: optional read callback for Signal attribute. The read > > - * level of the respective Signal should be passed back via > > - * the level parameter. > > - * @count_read: optional read callback for Count attribute. The read > > - * value of the respective Count should be passed back via > > - * the val parameter. > > Are these no longer optional? If they really are optional, it would be nice to > keep that information in the description. I'd expect drivers to have at least the count_read() and function_read() callbacks defined and set; otherwise such a counter driver would be rather useless to a user. I'll update the documentation here to make it clear the other callbacks are optional. > > - * @count_write: optional write callback for Count attribute. The write > > - * value for the respective Count is passed in via the val > > + * @signal_read: read callback for Signals. The read level of the > > + * respective Signal should be passed back via the level > > + * parameter. > > + * @count_read: read callback for Counts. The read value of the > > + * respective Count should be passed back via the value > > * parameter. > > + * @count_write: write callback for Counts. The write value for the > > + * respective Count is passed in via the value parameter. > > * @function_read: read callback the Count function modes. The read > > * function mode of the respective Count should be passed > > * back via the function parameter. > > @@ -291,7 +288,7 @@ struct counter_ops { > > > > /** > > * struct counter_device - Counter data structure > > - * @name: name of the device as it appears in the datasheet > > + * @name: name of the device > > Is there a recommended naming convention if using the datasheet is no longer > recommended? I decided to remove the "as it appears in the datasheet" phrase because there may be cases where the datasheet name isn't the best name to provide for the counter device. For example, with the 104-quad-8 driver there is ambiguity about whether it's more useful for the name to be "104-QUAD-8" to match the PC104 card, or "LS7266R1" to match the counter chip integrated on the 104-QUAD-8 card. Another example would be the recent interrupt-cnt driver which isn't for a particular device (it counts any interrupt source) so thus does not have a corresponding datasheet. I figure it's best to leave it up to the driver maintainer to choose an apt name that will make sense for the users. William Breathitt Gray
diff --git a/drivers/counter/counter-core.c b/drivers/counter/counter-core.c index 15f735ef296e..9442e3b91468 100644 --- a/drivers/counter/counter-core.c +++ b/drivers/counter/counter-core.c @@ -41,6 +41,9 @@ static struct bus_type counter_bus_type = { * 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. + * + * RETURNS: + * 0 on success, negative error number on failure. */ int counter_register(struct counter_device *const counter) { diff --git a/include/linux/counter.h b/include/linux/counter.h index b69277f5c4c5..e7fd6d81a929 100644 --- a/include/linux/counter.h +++ b/include/linux/counter.h @@ -188,11 +188,10 @@ struct counter_comp { /** * 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 + * @id: unique ID used to identify the Signal + * @name: device-specific Signal name + * @ext: optional array of Signal extensions + * @num_ext: number of Signal extensions specified in @ext */ struct counter_signal { int id; @@ -206,7 +205,7 @@ struct counter_signal { * struct counter_synapse - Counter Synapse node * @actions_list: array of available action modes * @num_actions: number of action modes specified in @actions_list - * @signal: pointer to associated signal + * @signal: pointer to the associated Signal */ struct counter_synapse { const enum counter_synapse_action *actions_list; @@ -217,15 +216,14 @@ struct counter_synapse { /** * 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 - * @functions_list: array available function modes + * @id: unique ID used to identify the Count + * @name: device-specific Count name + * @functions_list: array of 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 - * @num_ext: number of Counter Count extensions specified in @ext + * @synapses: array of Synapses for initialization + * @num_synapses: number of Synapses specified in @synapses + * @ext: optional array of Count extensions + * @num_ext: number of Count extensions specified in @ext */ struct counter_count { int id; @@ -243,15 +241,14 @@ struct counter_count { /** * struct counter_ops - Callbacks from driver - * @signal_read: optional read callback for Signal attribute. The read - * level of the respective Signal should be passed back via - * the level parameter. - * @count_read: optional read callback for Count attribute. The read - * value of the respective Count should be passed back via - * the val parameter. - * @count_write: optional write callback for Count attribute. The write - * value for the respective Count is passed in via the val + * @signal_read: read callback for Signals. The read level of the + * respective Signal should be passed back via the level + * parameter. + * @count_read: read callback for Counts. The read value of the + * respective Count should be passed back via the value * parameter. + * @count_write: write callback for Counts. The write value for the + * respective Count is passed in via the value parameter. * @function_read: read callback the Count function modes. The read * function mode of the respective Count should be passed * back via the function parameter. @@ -291,7 +288,7 @@ struct counter_ops { /** * struct counter_device - Counter data structure - * @name: name of the device as it appears in the datasheet + * @name: name of the device * @parent: optional parent device providing the counters * @ops: callbacks from driver * @signals: array of Signals
The Counter subsystem architecture and driver implementations have changed in order to handle Counter sysfs interactions in a more consistent way. This patch updates the Generic Counter interface header file comments to reflect the changes. Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com> --- drivers/counter/counter-core.c | 3 +++ include/linux/counter.h | 43 ++++++++++++++++------------------ 2 files changed, 23 insertions(+), 23 deletions(-)