| Message ID | 1616501538-120724-1-git-send-email-dj0227@163.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | regulator: Use IRQF_ONESHOT | expand |
On 23/03/2021 13:12, Jian Dong wrote: > From: Jian Dong <dongjian@yulong.com> > > Fixes coccicheck error: > > drivers/regulator/mt6360-regulator.c:388:8-33: ERROR: > drivers/regulator/pca9450-regulator.c:781:7-32: ERROR: > drivers/regulator/slg51000-regulator.c:480:8-33: ERROR: > drivers/regulator/qcom-labibb-regulator.c:364:8-33: ERROR: > Threaded IRQ with no primary handler requested without IRQF_ONESHOT > > Signed-off-by: Jian Dong <dongjian@yulong.com> > --- > drivers/regulator/mt6360-regulator.c | 4 ++-- > drivers/regulator/pca9450-regulator.c | 2 +- > drivers/regulator/qcom-labibb-regulator.c | 3 ++- > drivers/regulator/slg51000-regulator.c | 4 ++-- > 4 files changed, 7 insertions(+), 6 deletions(-) > > diff --git a/drivers/regulator/mt6360-regulator.c b/drivers/regulator/mt6360-regulator.c > index 15308ee..947350d 100644 > --- a/drivers/regulator/mt6360-regulator.c > +++ b/drivers/regulator/mt6360-regulator.c > @@ -385,8 +385,8 @@ static int mt6360_regulator_irq_register(struct platform_device *pdev, > return irq; > } > > - ret = devm_request_threaded_irq(&pdev->dev, irq, NULL, irq_desc->handler, 0, > - irq_desc->name, rdev); > + ret = devm_request_threaded_irq(&pdev->dev, irq, NULL, irq_desc->handler, > + IRQF_ONESHOT, irq_desc->name, rdev); This does not look like trivial rename/replace fix. This should be tested but it looks that you just did what coccinelle asked for, without testing. > if (ret) { > dev_err(&pdev->dev, "Fail to request %s irq\n", irq_desc->name); > return ret; > diff --git a/drivers/regulator/pca9450-regulator.c b/drivers/regulator/pca9450-regulator.c > index 2f7ee21..d4bc1c3 100644 > --- a/drivers/regulator/pca9450-regulator.c > +++ b/drivers/regulator/pca9450-regulator.c > @@ -780,7 +780,7 @@ static int pca9450_i2c_probe(struct i2c_client *i2c, > > ret = devm_request_threaded_irq(pca9450->dev, pca9450->irq, NULL, > pca9450_irq_handler, > - (IRQF_TRIGGER_FALLING | IRQF_ONESHOT), > + IRQF_TRIGGER_FALLING | IRQF_ONESHOT, How this is related to the missing IRQF_ONESHOT? > "pca9450-irq", pca9450); > if (ret != 0) { > dev_err(pca9450->dev, "Failed to request IRQ: %d\n", > diff --git a/drivers/regulator/qcom-labibb-regulator.c b/drivers/regulator/qcom-labibb-regulator.c > index de25e32..cac5fd5 100644 > --- a/drivers/regulator/qcom-labibb-regulator.c > +++ b/drivers/regulator/qcom-labibb-regulator.c > @@ -362,7 +362,8 @@ static int qcom_labibb_set_ocp(struct regulator_dev *rdev) > return ret; > > return devm_request_threaded_irq(vreg->dev, vreg->ocp_irq, NULL, > - qcom_labibb_ocp_isr, irq_flags, > + qcom_labibb_ocp_isr, > + irq_flags | IRQF_ONESHOT, > ocp_irq_name, vreg); > } > > diff --git a/drivers/regulator/slg51000-regulator.c b/drivers/regulator/slg51000-regulator.c > index 75a941f..3f310ab 100644 > --- a/drivers/regulator/slg51000-regulator.c > +++ b/drivers/regulator/slg51000-regulator.c > @@ -479,8 +479,8 @@ static int slg51000_i2c_probe(struct i2c_client *client) > if (chip->chip_irq) { > ret = devm_request_threaded_irq(dev, chip->chip_irq, NULL, > slg51000_irq_handler, > - (IRQF_TRIGGER_HIGH | > - IRQF_ONESHOT), > + IRQF_TRIGGER_HIGH | > + IRQF_ONESHOT, > "slg51000-irq", chip); How this is related to the missing IRQF_ONESHOT? Best regards, Krzysztof
Hi, Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> writes: > On 23/03/2021 13:12, Jian Dong wrote: >> From: Jian Dong <dongjian@yulong.com> >> >> Fixes coccicheck error: >> >> drivers/regulator/mt6360-regulator.c:388:8-33: ERROR: >> drivers/regulator/pca9450-regulator.c:781:7-32: ERROR: >> drivers/regulator/slg51000-regulator.c:480:8-33: ERROR: >> drivers/regulator/qcom-labibb-regulator.c:364:8-33: ERROR: >> Threaded IRQ with no primary handler requested without IRQF_ONESHOT >> >> Signed-off-by: Jian Dong <dongjian@yulong.com> >> --- >> drivers/regulator/mt6360-regulator.c | 4 ++-- >> drivers/regulator/pca9450-regulator.c | 2 +- >> drivers/regulator/qcom-labibb-regulator.c | 3 ++- >> drivers/regulator/slg51000-regulator.c | 4 ++-- >> 4 files changed, 7 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/regulator/mt6360-regulator.c b/drivers/regulator/mt6360-regulator.c >> index 15308ee..947350d 100644 >> --- a/drivers/regulator/mt6360-regulator.c >> +++ b/drivers/regulator/mt6360-regulator.c >> @@ -385,8 +385,8 @@ static int mt6360_regulator_irq_register(struct platform_device *pdev, >> return irq; >> } >> >> - ret = devm_request_threaded_irq(&pdev->dev, irq, NULL, irq_desc->handler, 0, >> - irq_desc->name, rdev); >> + ret = devm_request_threaded_irq(&pdev->dev, irq, NULL, irq_desc->handler, >> + IRQF_ONESHOT, irq_desc->name, rdev); > > This does not look like trivial rename/replace fix. This should be > tested but it looks that you just did what coccinelle asked for, without > testing. Right, but it must be done. If things work today, they work out of sheer luck. Also, which evidence is there that $subject wasn't tested? >> diff --git a/drivers/regulator/pca9450-regulator.c b/drivers/regulator/pca9450-regulator.c >> index 2f7ee21..d4bc1c3 100644 >> --- a/drivers/regulator/pca9450-regulator.c >> +++ b/drivers/regulator/pca9450-regulator.c >> @@ -780,7 +780,7 @@ static int pca9450_i2c_probe(struct i2c_client *i2c, >> >> ret = devm_request_threaded_irq(pca9450->dev, pca9450->irq, NULL, >> pca9450_irq_handler, >> - (IRQF_TRIGGER_FALLING | IRQF_ONESHOT), >> + IRQF_TRIGGER_FALLING | IRQF_ONESHOT, > > How this is related to the missing IRQF_ONESHOT? agreed. >> diff --git a/drivers/regulator/slg51000-regulator.c b/drivers/regulator/slg51000-regulator.c >> index 75a941f..3f310ab 100644 >> --- a/drivers/regulator/slg51000-regulator.c >> +++ b/drivers/regulator/slg51000-regulator.c >> @@ -479,8 +479,8 @@ static int slg51000_i2c_probe(struct i2c_client *client) >> if (chip->chip_irq) { >> ret = devm_request_threaded_irq(dev, chip->chip_irq, NULL, >> slg51000_irq_handler, >> - (IRQF_TRIGGER_HIGH | >> - IRQF_ONESHOT), >> + IRQF_TRIGGER_HIGH | >> + IRQF_ONESHOT, >> "slg51000-irq", chip); > > How this is related to the missing IRQF_ONESHOT? agreed.
On Thu, 25 Mar 2021 14:29:02 +0200 Felipe Balbi <balbi@kernel.org> wrote: > Hi, > > Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> writes: > > On 23/03/2021 13:12, Jian Dong wrote: > >> From: Jian Dong <dongjian@yulong.com> > >> > >> Fixes coccicheck error: > >> > >> drivers/regulator/mt6360-regulator.c:388:8-33: ERROR: > >> drivers/regulator/pca9450-regulator.c:781:7-32: ERROR: > >> drivers/regulator/slg51000-regulator.c:480:8-33: ERROR: > >> drivers/regulator/qcom-labibb-regulator.c:364:8-33: ERROR: > >> Threaded IRQ with no primary handler requested without IRQF_ONESHOT > >> > >> Signed-off-by: Jian Dong <dongjian@yulong.com> > >> --- > >> drivers/regulator/mt6360-regulator.c | 4 ++-- > >> drivers/regulator/pca9450-regulator.c | 2 +- > >> drivers/regulator/qcom-labibb-regulator.c | 3 ++- > >> drivers/regulator/slg51000-regulator.c | 4 ++-- > >> 4 files changed, 7 insertions(+), 6 deletions(-) > >> > >> diff --git a/drivers/regulator/mt6360-regulator.c > >> b/drivers/regulator/mt6360-regulator.c index 15308ee..947350d > >> 100644 --- a/drivers/regulator/mt6360-regulator.c > >> +++ b/drivers/regulator/mt6360-regulator.c > >> @@ -385,8 +385,8 @@ static int > >> mt6360_regulator_irq_register(struct platform_device *pdev, return > >> irq; } > >> > >> - ret = devm_request_threaded_irq(&pdev->dev, irq, > >> NULL, irq_desc->handler, 0, > >> - irq_desc->name, > >> rdev); > >> + ret = devm_request_threaded_irq(&pdev->dev, irq, > >> NULL, irq_desc->handler, > >> + IRQF_ONESHOT, > >> irq_desc->name, rdev); > > > > This does not look like trivial rename/replace fix. This should be > > tested but it looks that you just did what coccinelle asked for, > > without testing. > > Right, but it must be done. If things work today, they work out of > sheer luck. Also, which evidence is there that $subject wasn't tested? > it just use coccinelle to check again, the warning didn't rise after modify > >> diff --git a/drivers/regulator/pca9450-regulator.c > >> b/drivers/regulator/pca9450-regulator.c index 2f7ee21..d4bc1c3 > >> 100644 --- a/drivers/regulator/pca9450-regulator.c > >> +++ b/drivers/regulator/pca9450-regulator.c > >> @@ -780,7 +780,7 @@ static int pca9450_i2c_probe(struct i2c_client > >> *i2c, > >> ret = devm_request_threaded_irq(pca9450->dev, > >> pca9450->irq, NULL, pca9450_irq_handler, > >> - (IRQF_TRIGGER_FALLING | > >> IRQF_ONESHOT), > >> + IRQF_TRIGGER_FALLING | > >> IRQF_ONESHOT, > > > > How this is related to the missing IRQF_ONESHOT? > > agreed. > > >> diff --git a/drivers/regulator/slg51000-regulator.c > >> b/drivers/regulator/slg51000-regulator.c index 75a941f..3f310ab > >> 100644 --- a/drivers/regulator/slg51000-regulator.c > >> +++ b/drivers/regulator/slg51000-regulator.c > >> @@ -479,8 +479,8 @@ static int slg51000_i2c_probe(struct > >> i2c_client *client) if (chip->chip_irq) { > >> ret = devm_request_threaded_irq(dev, > >> chip->chip_irq, NULL, slg51000_irq_handler, > >> - > >> (IRQF_TRIGGER_HIGH | > >> - IRQF_ONESHOT), > >> + IRQF_TRIGGER_HIGH > >> | > >> + IRQF_ONESHOT, > >> "slg51000-irq", > >> chip); > > > > How this is related to the missing IRQF_ONESHOT? > > agreed. > I thnik it maybe the result of the coccinelle rule. it need to check value explicit. another wanring like this: define irq_flag = IRQF_ONESHOT, then reference irq_flag in fuction, it also will trigger coccinelle warning. it seems not the code error but coccicheck bug.
diff --git a/drivers/regulator/mt6360-regulator.c b/drivers/regulator/mt6360-regulator.c index 15308ee..947350d 100644 --- a/drivers/regulator/mt6360-regulator.c +++ b/drivers/regulator/mt6360-regulator.c @@ -385,8 +385,8 @@ static int mt6360_regulator_irq_register(struct platform_device *pdev, return irq; } - ret = devm_request_threaded_irq(&pdev->dev, irq, NULL, irq_desc->handler, 0, - irq_desc->name, rdev); + ret = devm_request_threaded_irq(&pdev->dev, irq, NULL, irq_desc->handler, + IRQF_ONESHOT, irq_desc->name, rdev); if (ret) { dev_err(&pdev->dev, "Fail to request %s irq\n", irq_desc->name); return ret; diff --git a/drivers/regulator/pca9450-regulator.c b/drivers/regulator/pca9450-regulator.c index 2f7ee21..d4bc1c3 100644 --- a/drivers/regulator/pca9450-regulator.c +++ b/drivers/regulator/pca9450-regulator.c @@ -780,7 +780,7 @@ static int pca9450_i2c_probe(struct i2c_client *i2c, ret = devm_request_threaded_irq(pca9450->dev, pca9450->irq, NULL, pca9450_irq_handler, - (IRQF_TRIGGER_FALLING | IRQF_ONESHOT), + IRQF_TRIGGER_FALLING | IRQF_ONESHOT, "pca9450-irq", pca9450); if (ret != 0) { dev_err(pca9450->dev, "Failed to request IRQ: %d\n", diff --git a/drivers/regulator/qcom-labibb-regulator.c b/drivers/regulator/qcom-labibb-regulator.c index de25e32..cac5fd5 100644 --- a/drivers/regulator/qcom-labibb-regulator.c +++ b/drivers/regulator/qcom-labibb-regulator.c @@ -362,7 +362,8 @@ static int qcom_labibb_set_ocp(struct regulator_dev *rdev) return ret; return devm_request_threaded_irq(vreg->dev, vreg->ocp_irq, NULL, - qcom_labibb_ocp_isr, irq_flags, + qcom_labibb_ocp_isr, + irq_flags | IRQF_ONESHOT, ocp_irq_name, vreg); } diff --git a/drivers/regulator/slg51000-regulator.c b/drivers/regulator/slg51000-regulator.c index 75a941f..3f310ab 100644 --- a/drivers/regulator/slg51000-regulator.c +++ b/drivers/regulator/slg51000-regulator.c @@ -479,8 +479,8 @@ static int slg51000_i2c_probe(struct i2c_client *client) if (chip->chip_irq) { ret = devm_request_threaded_irq(dev, chip->chip_irq, NULL, slg51000_irq_handler, - (IRQF_TRIGGER_HIGH | - IRQF_ONESHOT), + IRQF_TRIGGER_HIGH | + IRQF_ONESHOT, "slg51000-irq", chip); if (ret != 0) { dev_err(dev, "Failed to request IRQ: %d\n",