| Message ID | 20250331075838.3444332-1-christianshewitt@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: si2168: increase cmd execution timeout value | expand |
> if (cmd->rlen) { > /* wait cmd execution terminate */ > - #define TIMEOUT 70 > + #define TIMEOUT 200 While we are here, can we rename it to CMD_TIMEOUT and put it next to the #includes?
> On 31 Mar 2025, at 12:06 pm, Wolfram Sang <wsa+renesas@sang-engineering.com> wrote: > > >> if (cmd->rlen) { >> /* wait cmd execution terminate */ >> - #define TIMEOUT 70 >> + #define TIMEOUT 200 > > While we are here, can we rename it to CMD_TIMEOUT and put it next to > the #includes? I’m at the novice end of kernel contributors, so like this? --- a/drivers/media/dvb-frontends/si2168.c +++ b/drivers/media/dvb-frontends/si2168.c @@ -9,6 +9,8 @@ #include "si2168_priv.h" +#define CMD_TIMEOUT 200 + static const struct dvb_frontend_ops si2168_ops; static void cmd_init(struct si2168_cmd *cmd, const u8 *buf, int wlen, int rlen) @@ -40,8 +42,7 @@ static int si2168_cmd_execute(struct i2c_client *client, struct si2168_cmd *cmd) if (cmd->rlen) { /* wait cmd execution terminate */ - #define TIMEOUT 200 - timeout = jiffies + msecs_to_jiffies(TIMEOUT); + timeout = jiffies + msecs_to_jiffies(CMD_TIMEOUT); while (!time_after(jiffies, timeout)) { ret = i2c_master_recv(client, cmd->args, cmd->rlen); if (ret < 0) { @@ -58,7 +59,7 @@ static int si2168_cmd_execute(struct i2c_client *client, struct si2168_cmd *cmd) dev_dbg(&client->dev, "cmd execution took %d ms\n", jiffies_to_msecs(jiffies) - - (jiffies_to_msecs(timeout) - TIMEOUT)); + (jiffies_to_msecs(timeout) - CMD_TIMEOUT)); /* error bit set? */ if ((cmd->args[0] >> 6) & 0x01) { Christian
On Mon, Mar 31, 2025 at 12:31:48PM +0400, Christian Hewitt wrote: > > On 31 Mar 2025, at 12:06 pm, Wolfram Sang <wsa+renesas@sang-engineering.com> wrote: > > > > > >> if (cmd->rlen) { > >> /* wait cmd execution terminate */ > >> - #define TIMEOUT 70 > >> + #define TIMEOUT 200 > > > > While we are here, can we rename it to CMD_TIMEOUT and put it next to > > the #includes? > > I’m at the novice end of kernel contributors, so like this? Exactly! Thank you. If you send v2 of this patch, you can already add: Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c index d6b6b8bc7d4e..557e6228ffe8 100644 --- a/drivers/media/dvb-frontends/si2168.c +++ b/drivers/media/dvb-frontends/si2168.c @@ -40,7 +40,7 @@ static int si2168_cmd_execute(struct i2c_client *client, struct si2168_cmd *cmd) if (cmd->rlen) { /* wait cmd execution terminate */ - #define TIMEOUT 70 + #define TIMEOUT 200 timeout = jiffies + msecs_to_jiffies(TIMEOUT); while (!time_after(jiffies, timeout)) { ret = i2c_master_recv(client, cmd->args, cmd->rlen);
Testing with a MyGica T230C v2 USB device (0572:c68a) shows occasional cmd timeouts that cause Tvheadend services to fail: Jan 28 12:23:46.788180 LibreELEC kernel: si2168 1-0060: cmd execution took 0 ms Jan 28 12:23:46.790799 LibreELEC kernel: si2168 1-0060: cmd execution took 0 ms Jan 28 12:23:46.878158 LibreELEC kernel: si2168 1-0060: cmd execution took 80 ms Jan 28 12:23:46.879158 LibreELEC kernel: si2168 1-0060: failed=-110 Jan 28 12:23:46.879908 LibreELEC kernel: si2168 1-0060: failed=-110 Jan 28 12:23:46.948234 LibreELEC kernel: si2168 1-0060: cmd execution took 60 ms Jan 28 12:23:46.949121 LibreELEC kernel: si2168 1-0060: cmd execution took 0 ms Jan 28 12:23:46.949940 LibreELEC kernel: si2168 1-0060: cmd execution took 10 ms .. Jan 28 12:23:57.457216 LibreELEC tvheadend[3126]: subscription: 009B: service instance is bad, reason: No input detected Jan 28 12:23:57.457392 LibreELEC tvheadend[3126]: linuxdvb: Silicon Labs Si2168 #0 : DVB-T #0 - stopping 778MHz in DVB-T Network .. Jan 28 12:23:57.457584 LibreELEC tvheadend[3126]: subscription: 009B: No input source available for subscription "127.0.0.1 [ | Kodi Media Center ]" to channel "XXXXXXX" The original timeout of 50ms was extended to 70ms in commit 551c33e729f6 ("[media] Si2168: increase timeout to fix firmware loading") but testing shows there are other demux commands that take longer. The largest value observed from user reports/logs is 150ms so increase timeout to 200ms. Signed-off-by: Christian Hewitt <christianshewitt@gmail.com> --- drivers/media/dvb-frontends/si2168.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)