| Message ID | 20240328020522.50995-12-herdler@nurfuerspam.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: ttpci: make checkpatch happy | expand |
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@nurfuerspam.de> > --- > > 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 > >
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) {
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. 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@nurfuerspam.de> --- 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(-) -- 2.34.0