| Message ID | 20200826181706.11098-1-krzk@kernel.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [01/24] Input: bcm-keypad - Simplify with dev_err_probe() | expand |
On Wed, Aug 26, 2020 at 08:16:43PM +0200, Krzysztof Kozlowski wrote: > Common pattern of handling deferred probe can be simplified with > dev_err_probe(). Less code and also it prints the error value. > +++ b/drivers/input/keyboard/bcm-keypad.c > @@ -379,11 +379,9 @@ static int bcm_kp_probe(struct platform_device *pdev) > kp->clk = devm_clk_get(&pdev->dev, "peri_clk"); > if (IS_ERR(kp->clk)) { > error = PTR_ERR(kp->clk); > - if (error != -ENOENT) { > - if (error != -EPROBE_DEFER) > - dev_err(&pdev->dev, "Failed to get clock\n"); > - return error; > - } > + if (error != -ENOENT) > + return dev_err_probe(&pdev->dev, error, "Failed to get clock\n"); > + > dev_dbg(&pdev->dev, > "No clock specified. Assuming it's enabled\n"); Shouldn't be this rather switch to devm_clk_get_optional() + dev_err_probe? > kp->clk = NULL;
On Wed, Aug 26, 2020 at 10:12:17PM +0300, Andy Shevchenko wrote: > On Wed, Aug 26, 2020 at 08:16:43PM +0200, Krzysztof Kozlowski wrote: > > Common pattern of handling deferred probe can be simplified with > > dev_err_probe(). Less code and also it prints the error value. > > > +++ b/drivers/input/keyboard/bcm-keypad.c > > @@ -379,11 +379,9 @@ static int bcm_kp_probe(struct platform_device *pdev) > > kp->clk = devm_clk_get(&pdev->dev, "peri_clk"); > > if (IS_ERR(kp->clk)) { > > error = PTR_ERR(kp->clk); > > - if (error != -ENOENT) { > > - if (error != -EPROBE_DEFER) > > - dev_err(&pdev->dev, "Failed to get clock\n"); > > - return error; > > - } > > + if (error != -ENOENT) > > + return dev_err_probe(&pdev->dev, error, "Failed to get clock\n"); > > + > > dev_dbg(&pdev->dev, > > "No clock specified. Assuming it's enabled\n"); > > Shouldn't be this rather switch to devm_clk_get_optional() + dev_err_probe? Indeed, it could be simplified even more. I'll send v2. Best regards, Krzysztof
Hi, As someone who has added the if (error != -EPROBE_DEFER) dev_err() pattern in way too many places myself, I'm quite happy to see this new (I presume?) helper and this nice cleanup series. The entire series looks good to me, and you can add my: Reviewed-by: Hans de Goede <hdegoede@redhat.com> To the entire series. Regards, Hans On 8/26/20 8:16 PM, Krzysztof Kozlowski wrote: > Common pattern of handling deferred probe can be simplified with > dev_err_probe(). Less code and also it prints the error value. > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > --- > drivers/input/keyboard/bcm-keypad.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/input/keyboard/bcm-keypad.c b/drivers/input/keyboard/bcm-keypad.c > index 2b771c3a5578..1bf71e7c9e0d 100644 > --- a/drivers/input/keyboard/bcm-keypad.c > +++ b/drivers/input/keyboard/bcm-keypad.c > @@ -379,11 +379,9 @@ static int bcm_kp_probe(struct platform_device *pdev) > kp->clk = devm_clk_get(&pdev->dev, "peri_clk"); > if (IS_ERR(kp->clk)) { > error = PTR_ERR(kp->clk); > - if (error != -ENOENT) { > - if (error != -EPROBE_DEFER) > - dev_err(&pdev->dev, "Failed to get clock\n"); > - return error; > - } > + if (error != -ENOENT) > + return dev_err_probe(&pdev->dev, error, "Failed to get clock\n"); > + > dev_dbg(&pdev->dev, > "No clock specified. Assuming it's enabled\n"); > kp->clk = NULL; >
diff --git a/drivers/input/keyboard/bcm-keypad.c b/drivers/input/keyboard/bcm-keypad.c index 2b771c3a5578..1bf71e7c9e0d 100644 --- a/drivers/input/keyboard/bcm-keypad.c +++ b/drivers/input/keyboard/bcm-keypad.c @@ -379,11 +379,9 @@ static int bcm_kp_probe(struct platform_device *pdev) kp->clk = devm_clk_get(&pdev->dev, "peri_clk"); if (IS_ERR(kp->clk)) { error = PTR_ERR(kp->clk); - if (error != -ENOENT) { - if (error != -EPROBE_DEFER) - dev_err(&pdev->dev, "Failed to get clock\n"); - return error; - } + if (error != -ENOENT) + return dev_err_probe(&pdev->dev, error, "Failed to get clock\n"); + dev_dbg(&pdev->dev, "No clock specified. Assuming it's enabled\n"); kp->clk = NULL;
Common pattern of handling deferred probe can be simplified with dev_err_probe(). Less code and also it prints the error value. Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> --- drivers/input/keyboard/bcm-keypad.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)