On 28/03/2024 03:05, Stefan Herdler wrote:
> This patch fixes the following checkpatch warnings:
>
> WARNING:MSLEEP: msleep < 20ms can sleep for up to 20ms; see Documentation/timers/timers-howto.rst
>
>
> The driver is working with this behaviour since years. Just replacing
> all msleep < 20ms with msleep 20ms to silence the warning.
> Add a comment to indicate the time choice was not hardware related.
I would drop this patch. It is just a warning indicating that msleep with a
value < 20 can sleep up to 20 ms. In other words, that the code should be
checked that the msleep is actually appropriate.
An alternative to msleep is usleep_range: there you can be more precise.
In this case I would just leave it as-is.
Regards,
Hans
>
> There are only a few of these msleep in a row, the extra sleep time
> won't add up to much.
>
>
> Signed-off-by: Stefan Herdler <herdler@xxxxxxxxxxxxxx>
> ---
>
> Warning:
> I'm not able to test this changes properly. None of my cards are affected
> directly and I don't have a operational CAM.
>
> I think, we have 3 options to deal with this warnings:
> * Ignore them.
> This is save, but will keep the warnings.
> * Use usleep_range.
> Probably dangerous, it may break the driver.
> * Rise all shorter msleep to 20ms.
> This should be pretty save.
> Many of this msleep are in a row with much longer msleeps anyway and
> there are only less of 2 of these in a function.
> Most of the affected functions are init functions only.
> The most time critical msleeps I have spotted are tuning related. So
> tuning a new transponder may take up to 20ms longer on some frontends,
> in the worst case.
> Nevertheless, please consider this patch as a proposal and optional.
>
> drivers/media/pci/ttpci/budget-av.c | 8 ++++----
> drivers/media/pci/ttpci/budget-ci.c | 10 +++++-----
> drivers/media/pci/ttpci/budget.c | 4 ++--
> 3 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/media/pci/ttpci/budget-av.c b/drivers/media/pci/ttpci/budget-av.c
> index fd5a88e64..680e46cf1 100644
> --- a/drivers/media/pci/ttpci/budget-av.c
> +++ b/drivers/media/pci/ttpci/budget-av.c
> @@ -211,7 +211,7 @@ static int ciintf_slot_reset(struct dvb_ca_en50221 *ca, int slot)
> saa7146_setgpio(saa, 2, SAA7146_GPIO_OUTHI); /* disable card */
>
> saa7146_setgpio(saa, 0, SAA7146_GPIO_OUTHI); /* Vcc off */
> - msleep(2);
> + msleep(20); // Was 2, but msleep would have slept up to 20ms nevertheless.
> saa7146_setgpio(saa, 0, SAA7146_GPIO_OUTLO); /* Vcc on */
> msleep(20); /* 20 ms Vcc settling time */
>
> @@ -637,7 +637,7 @@ static int philips_cu1216_tuner_set_params(struct dvb_frontend *fe)
> fe->ops.i2c_gate_ctrl(fe, 1);
> if (i2c_transfer(&budget->i2c_adap, &msg, 1) == 1 && (buf[0] & 0x40))
> break;
> - msleep(10);
> + msleep(20); // Was 10, but msleep would have slept up to 20ms nevertheless.
> }
>
> /* switch the charge pump to the lower current */
> @@ -679,7 +679,7 @@ static int philips_tu1216_tuner_init(struct dvb_frontend *fe)
> fe->ops.i2c_gate_ctrl(fe, 1);
> if (i2c_transfer(&budget->i2c_adap, &tuner_msg, 1) != 1)
> return -EIO;
> - msleep(1);
> + msleep(20); // Was 1, but msleep would have slept up to 20ms nevertheless.
>
> return 0;
> }
> @@ -764,7 +764,7 @@ static int philips_tu1216_tuner_set_params(struct dvb_frontend *fe)
> if (i2c_transfer(&budget->i2c_adap, &tuner_msg, 1) != 1)
> return -EIO;
>
> - msleep(1);
> + msleep(20); // Was 1, but msleep would have slept up to 20ms nevertheless.
> return 0;
> }
>
> diff --git a/drivers/media/pci/ttpci/budget-ci.c b/drivers/media/pci/ttpci/budget-ci.c
> index d821710bb..066eba67f 100644
> --- a/drivers/media/pci/ttpci/budget-ci.c
> +++ b/drivers/media/pci/ttpci/budget-ci.c
> @@ -308,7 +308,7 @@ static int ciintf_slot_reset(struct dvb_ca_en50221 *ca, int slot)
> }
> budget_ci->slot_status = SLOTSTATUS_RESET;
> ttpci_budget_debiwrite(&budget_ci->budget, DEBICICTL, DEBIADDR_CICONTROL, 1, 0, 1, 0);
> - msleep(1);
> + msleep(20); // Was 1, but msleep would have slept up to 20ms nevertheless.
> ttpci_budget_debiwrite(&budget_ci->budget, DEBICICTL, DEBIADDR_CICONTROL, 1,
> CICONTROL_RESET, 1, 0);
>
> @@ -534,7 +534,7 @@ static void ciintf_deinit(struct budget_ci *budget_ci)
>
> // reset interface
> ttpci_budget_debiwrite(&budget_ci->budget, DEBICICTL, DEBIADDR_CICONTROL, 1, 0, 1, 0);
> - msleep(1);
> + msleep(20); // Was 1, but msleep would have slept up to 20ms nevertheless.
> ttpci_budget_debiwrite(&budget_ci->budget, DEBICICTL, DEBIADDR_CICONTROL, 1,
> CICONTROL_RESET, 1, 0);
>
> @@ -706,7 +706,7 @@ static int philips_tdm1316l_tuner_init(struct dvb_frontend *fe)
> fe->ops.i2c_gate_ctrl(fe, 1);
> if (i2c_transfer(&budget_ci->budget.i2c_adap, &tuner_msg, 1) != 1)
> return -EIO;
> - msleep(1);
> + msleep(20); // Was 1, but msleep would have slept up to 20ms nevertheless.
>
> // disable the mc44BC374c (do not check for errors)
> tuner_msg.addr = 0x65;
> @@ -805,7 +805,7 @@ static int philips_tdm1316l_tuner_set_params(struct dvb_frontend *fe)
> if (i2c_transfer(&budget_ci->budget.i2c_adap, &tuner_msg, 1) != 1)
> return -EIO;
>
> - msleep(1);
> + msleep(20); // Was 1, but msleep would have slept up to 20ms nevertheless.
> return 0;
> }
>
> @@ -910,7 +910,7 @@ static int dvbc_philips_tdm1316l_tuner_set_params(struct dvb_frontend *fe)
> if (i2c_transfer(&budget_ci->budget.i2c_adap, &tuner_msg, 1) != 1)
> return -EIO;
>
> - msleep(1);
> + msleep(20); // Was 1, but msleep would have slept up to 20ms nevertheless.
>
> return 0;
> }
> diff --git a/drivers/media/pci/ttpci/budget.c b/drivers/media/pci/ttpci/budget.c
> index 3842bad8d..10599037c 100644
> --- a/drivers/media/pci/ttpci/budget.c
> +++ b/drivers/media/pci/ttpci/budget.c
> @@ -630,9 +630,9 @@ static void frontend_init(struct budget *budget)
>
> // gpio2 is connected to CLB - reset it + leave it high
> saa7146_setgpio(budget->dev, 2, SAA7146_GPIO_OUTLO);
> - msleep(1);
> + msleep(20); // Was 1, but msleep would have slept up to 20ms nevertheless.
> saa7146_setgpio(budget->dev, 2, SAA7146_GPIO_OUTHI);
> - msleep(1);
> + msleep(20); // Was 1, but msleep would have slept up to 20ms nevertheless.
>
> fe = dvb_attach(tda10086_attach, &tda10086_config, &budget->i2c_adap);
> if (fe) {
> --
> 2.34.0
>
>
