[PATCH] drivers/cros_ec: Make log polling period a parameter

[PATCH] drivers/cros_ec: Make log polling period a parameter

[Rob Barnes]

Make EC log polling period a module parameter. This allows the polling
period to be set via the kernel command line. Keeping the default at
the current 10 second period. The optimal polling period will differ by
board.

Signed-off-by: Rob Barnes <robbarnes@google.com>
---
 drivers/platform/chrome/cros_ec_debugfs.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_debugfs.c b/drivers/platform/chrome/cros_ec_debugfs.c
index 6bf6f0e7b597..95101b0d5227 100644
--- a/drivers/platform/chrome/cros_ec_debugfs.c
+++ b/drivers/platform/chrome/cros_ec_debugfs.c
@@ -25,6 +25,10 @@
 
 #define CIRC_ADD(idx, size, value)	(((idx) + (value)) & ((size) - 1))
 
+static unsigned int log_poll_period_ms = LOG_POLL_SEC * MSEC_PER_SEC;
+module_param(log_poll_period_ms, uint, 0644);
+MODULE_PARM_DESC(log_poll_period_ms, "EC log polling period(ms)");
+
 /* waitqueue for log readers */
 static DECLARE_WAIT_QUEUE_HEAD(cros_ec_debugfs_log_wq);
 
@@ -56,7 +60,7 @@ struct cros_ec_debugfs {
 
 /*
  * We need to make sure that the EC log buffer on the UART is large enough,
- * so that it is unlikely enough to overlow within LOG_POLL_SEC.
+ * so that it is unlikely enough to overlow within log_poll_period_ms.
  */
 static void cros_ec_console_log_work(struct work_struct *__work)
 {
@@ -118,7 +122,7 @@ static void cros_ec_console_log_work(struct work_struct *__work)
 
 resched:
 	schedule_delayed_work(&debug_info->log_poll_work,
-			      msecs_to_jiffies(LOG_POLL_SEC * 1000));
+			      msecs_to_jiffies(log_poll_period_ms));
 }
 
 static int cros_ec_console_log_open(struct inode *inode, struct file *file)
-- 
2.45.0.rc1.225.g2a3ae87e7f-goog

Re: [PATCH] drivers/cros_ec: Make log polling period a parameter

[Tzung-Bi Shih]

On Tue, May 07, 2024 at 03:58:09PM +0000, Rob Barnes wrote:
> Make EC log polling period a module parameter. This allows the polling
> period to be set via the kernel command line. Keeping the default at
> the current 10 second period. The optimal polling period will differ by
> board.

If you get chance for sending next version, please use title prefix
"platform/chrome: cros_ec_debugfs:".

> @@ -118,7 +122,7 @@ static void cros_ec_console_log_work(struct work_struct *__work)
>  
>  resched:
>  	schedule_delayed_work(&debug_info->log_poll_work,
> -			      msecs_to_jiffies(LOG_POLL_SEC * 1000));
> +			      msecs_to_jiffies(log_poll_period_ms));

If the value is set to 0, it works like an infinite loop on
cros_ec_console_log_work().  Is it expected?

Re: [PATCH] drivers/cros_ec: Make log polling period a parameter

[Rob Barnes]

On Wed, May 22, 2024 at 7:30 PM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> On Tue, May 07, 2024 at 03:58:09PM +0000, Rob Barnes wrote:
> > Make EC log polling period a module parameter. This allows the polling
> > period to be set via the kernel command line. Keeping the default at
> > the current 10 second period. The optimal polling period will differ by
> > board.
>
> If you get chance for sending next version, please use title prefix
> "platform/chrome: cros_ec_debugfs:".
>
> > @@ -118,7 +122,7 @@ static void cros_ec_console_log_work(struct work_struct *__work)
> >
> >  resched:
> >       schedule_delayed_work(&debug_info->log_poll_work,
> > -                           msecs_to_jiffies(LOG_POLL_SEC * 1000));
> > +                           msecs_to_jiffies(log_poll_period_ms));
>
> If the value is set to 0, it works like an infinite loop on
> cros_ec_console_log_work().  Is it expected?

Yes. I think this result is clear from the parameter name,
"log_poll_period_ms", i.e. a developer setting this parameter to 0
would expect the EC log to be polled as fast as possible. This may be
done for testing, but it wouldn't make sense in production. We could
enforce an absolute minimum, but the value would be mostly a guess.
Let me know if you still want a minimum enforced and if you have a
recommendation on the value?

Re: [PATCH] drivers/cros_ec: Make log polling period a parameter

[patchwork-bot+chrome-platform]

Hello:

This patch was applied to chrome-platform/linux.git (for-kernelci)
by Tzung-Bi Shih <tzungbi@kernel.org>:

On Tue,  7 May 2024 15:58:09 +0000 you wrote:
> Make EC log polling period a module parameter. This allows the polling
> period to be set via the kernel command line. Keeping the default at
> the current 10 second period. The optimal polling period will differ by
> board.
> 
> Signed-off-by: Rob Barnes <robbarnes@google.com>
> 
> [...]

Here is the summary with links:
  - drivers/cros_ec: Make log polling period a parameter
    https://git.kernel.org/chrome-platform/c/7b44d5381e54

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH] drivers/cros_ec: Make log polling period a parameter

[patchwork-bot+chrome-platform]

Hello:

This patch was applied to chrome-platform/linux.git (for-next)
by Tzung-Bi Shih <tzungbi@kernel.org>:

On Tue,  7 May 2024 15:58:09 +0000 you wrote:
> Make EC log polling period a module parameter. This allows the polling
> period to be set via the kernel command line. Keeping the default at
> the current 10 second period. The optimal polling period will differ by
> board.
> 
> Signed-off-by: Rob Barnes <robbarnes@google.com>
> 
> [...]

Here is the summary with links:
  - drivers/cros_ec: Make log polling period a parameter
    https://git.kernel.org/chrome-platform/c/7b44d5381e54

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html