| Message ID | 20241125230220.9994-1-rafael.v.volkmer@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | counter: add COUNTER_FUNCTION_DISABLE for energy saving | expand |
On 11/25/24 5:02 PM, Rafael V. Volkmer wrote: > Add `COUNTER_FUNCTION_DISABLE` to the `counter_function` enum in the > counter API. This allows file operations to signal other drivers to > disable hardware resources, reducing energy consumption in > power-sensitive scenarios. > > Previously, tests with Texas Instruments' eQEP modules revealed that > hardware resources remained active unless the driver was removed, > offering no user command to stop the count. This approach exposed the > fragility of these resources. > > To address this, introduce a new enum option in the counter API to > receive commands for disabling the hardware. This ensures the hardware > enters an idle, power-saving state when not in use. How does this work without an additional patch to modify the TI eQEP counter driver to handle this new enum value? For example, I would expect that this enum value would be added to ti_eqep_position_functions and case statements added in ti_eqep_function_read(), ti_eqep_function_write() and ti_eqep_action_read() to handle the new option. > > Signed-off-by: Rafael V. Volkmer <rafael.v.volkmer@gmail.com> > --- > include/uapi/linux/counter.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/include/uapi/linux/counter.h b/include/uapi/linux/counter.h > index 008a691c254b..6f9a6ef878cf 100644 > --- a/include/uapi/linux/counter.h > +++ b/include/uapi/linux/counter.h > @@ -145,6 +145,7 @@ enum counter_function { > COUNTER_FUNCTION_QUADRATURE_X2_A, > COUNTER_FUNCTION_QUADRATURE_X2_B, > COUNTER_FUNCTION_QUADRATURE_X4, > + COUNTER_FUNCTION_DISABLE, > }; > > /* Signal values */
On Mon, Nov 25, 2024 at 08:02:20PM -0300, Rafael V. Volkmer wrote: > Add `COUNTER_FUNCTION_DISABLE` to the `counter_function` enum in the > counter API. This allows file operations to signal other drivers to > disable hardware resources, reducing energy consumption in > power-sensitive scenarios. > > Previously, tests with Texas Instruments' eQEP modules revealed that > hardware resources remained active unless the driver was removed, > offering no user command to stop the count. This approach exposed the > fragility of these resources. > > To address this, introduce a new enum option in the counter API to > receive commands for disabling the hardware. This ensures the hardware > enters an idle, power-saving state when not in use. > > Signed-off-by: Rafael V. Volkmer <rafael.v.volkmer@gmail.com> Hello Rafael, I wonder whether a new enum option is actually needed in this case. Wouldn't the Count "enable" extension already provide a way for users to stop the counting? I imagine the driver would determine implicitly that the device can enter a power-saving state in that scenario when counting has been disabled by the user. William Breathitt Gray
On Mon, Nov 25, 2024 at 05:26 PM, William Breathitt Gray wrote: > Hello Rafael, > > I wonder whether a new enum option is actually needed in this case. > Wouldn't the Count "enable" extension already provide a way for users to > stop the counting? I imagine the driver would determine implicitly that > the device can enter a power-saving state in that scenario when counting > has been disabled by the user. > > William Breathitt Gray Hello, William! Thanks a lot for the feedback! You raised an important point. I would like to explain my reasoning behind the implementation so you can see if it makes sense or if I misunderstood. Basically, Texas drivers are described as user-operated through the sysfs file system, echoing some directories in /dev/. The requirement of my application was to communicate at the kernel space level with another driver, and to do so, I retrieved the pdev with platform_get_drvdata(), which was allocated with platform_set_drvdata in the texas driver probe. To apply the required settings, I used the pdev I retrieved pointing to the fops, such as ti_eqep_function_write(). I made this small change in the module for this enum value and a patch in the ti eqep driver to handle this in order to establish a safe shutdown. In my application it worked perfectly, and so I had the idea of applying a patch to kernel.org to see if my changes could contribute something to the Texas driver infrastructure. Best regards
On Mon, Nov 25, 2024 at 05:26 PM, William Breathitt Gray wrote: > Hello Rafael, > > I wonder whether a new enum option is actually needed in this case. > Wouldn't the Count "enable" extension already provide a way for users to > stop the counting? I imagine the driver would determine implicitly that > the device can enter a power-saving state in that scenario when counting > has been disabled by the user. > > William Breathitt Gray Hello, William! Thanks a lot for the feedback! You raised an important point. I would like to explain my reasoning behind the implementation so you can see if it makes sense or if I misunderstood. Basically, Texas drivers are described as user-operated through the sysfs file system, echoing some directories in /dev/. The requirement of my application was to communicate at the kernel space level with another driver, and to do so, I retrieved the pdev with platform_get_drvdata(), which was allocated with platform_set_drvdata in the texas driver probe. To apply the required settings, I used the pdev I retrieved pointing to the fops, such as ti_eqep_function_write(). I made this small change in the module for this enum value and a patch in the ti eqep driver to handle this in order to establish a safe shutdown. In my application it worked perfectly, and so I had the idea of applying a patch to kernel.org to see if my changes could contribute something to the Texas driver infrastructure. Best regards
On 11/25/24 11:36 AM, David Lechner wrote: > How does this work without an additional patch to modify the TI eQEP > counter driver to handle this new enum value? For example, I would > expect that this enum value would be added to ti_eqep_position_functions > and case statements added in ti_eqep_function_read(), > ti_eqep_function_write() and ti_eqep_action_read() to handle the new > option. Hi, David! Yes, the intention is to have a second path where the eQEP driver handles this within these file operations functions. Best regards
On 11/27/24 3:54 PM, Rafael V. Volkmer wrote: > On 11/25/24 11:36 AM, David Lechner wrote: >> How does this work without an additional patch to modify the TI eQEP >> counter driver to handle this new enum value? For example, I would >> expect that this enum value would be added to ti_eqep_position_functions >> and case statements added in ti_eqep_function_read(), >> ti_eqep_function_write() and ti_eqep_action_read() to handle the new >> option. > > Hi, David! > > Yes, the intention is to have a second path where the eQEP driver handles > this within these file operations functions. > > Best regards OK, so please send those patches too so that we can see the whole picture. Based on your discussion with William, it sounds like there are 2 things that need to be resolved. 1. Should a power saving mode actually be a counter function or should it be controlled by the counter enable attribute or something else, like more general Linux power management stuff? 2. If there are going to be in-kernel users calling these functions, then we will likely want to introduce some new APIs in the kernel for this. Using platform_get_drvdata() from one driver to another is a bit fragile. Likely we will want some devicetree bindings for counter consumers and providers and some kernel APIs like a counter_get() that takes an index or string ID to get the counter provider assigned to a counter consumer. Then we will probably want some wrappers around the counter ops pointers so that consumer drivers don't have to depend on the internal implementation of the counter subsystem.
diff --git a/include/uapi/linux/counter.h b/include/uapi/linux/counter.h index 008a691c254b..6f9a6ef878cf 100644 --- a/include/uapi/linux/counter.h +++ b/include/uapi/linux/counter.h @@ -145,6 +145,7 @@ enum counter_function { COUNTER_FUNCTION_QUADRATURE_X2_A, COUNTER_FUNCTION_QUADRATURE_X2_B, COUNTER_FUNCTION_QUADRATURE_X4, + COUNTER_FUNCTION_DISABLE, }; /* Signal values */
Add `COUNTER_FUNCTION_DISABLE` to the `counter_function` enum in the counter API. This allows file operations to signal other drivers to disable hardware resources, reducing energy consumption in power-sensitive scenarios. Previously, tests with Texas Instruments' eQEP modules revealed that hardware resources remained active unless the driver was removed, offering no user command to stop the count. This approach exposed the fragility of these resources. To address this, introduce a new enum option in the counter API to receive commands for disabling the hardware. This ensures the hardware enters an idle, power-saving state when not in use. Signed-off-by: Rafael V. Volkmer <rafael.v.volkmer@gmail.com> --- include/uapi/linux/counter.h | 1 + 1 file changed, 1 insertion(+)