| Message ID | 20240830084620.396417-1-yanzhen@vivo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] irqchip: madera: Simplify with dev_err_probe() | expand |
On 30/08/2024 10:46, Yan Zhen wrote: > Switch to use dev_err_probe() to simplify the error path and > unify a message template. > > Using this helper is totally fine even if err is known to never > be -EPROBE_DEFER. > > The benefit compared to a normal dev_err() is the standardized format > of the error code, it being emitted symbolically and the fact that > the error code is returned which allows more compact error paths. > > Signed-off-by: Yan Zhen <yanzhen@vivo.com> > --- > > Changes in v3: > -Rewrite the subject as 'irqchip: madera:'. > > drivers/irqchip/irq-madera.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/irqchip/irq-madera.c b/drivers/irqchip/irq-madera.c > index acceb6e7fa95..d5ad4466a140 100644 > --- a/drivers/irqchip/irq-madera.c > +++ b/drivers/irqchip/irq-madera.c > @@ -199,9 +199,8 @@ static int madera_irq_probe(struct platform_device *pdev) > ret = regmap_update_bits(madera->regmap, MADERA_IRQ1_CTRL, > MADERA_IRQ_POL_MASK, 0); > if (ret) { > - dev_err(&pdev->dev, > - "Failed to set IRQ polarity: %d\n", ret); > - return ret; > + return dev_err_probe(&pdev->dev, ret, > + "Failed to set IRQ polarity"); Another example, one of many from @vivo.com, where you touch one line and leave everything else. Are you going to send 5 different patches - one per each line? It's nonsense. Best regards, Krzysztof
On Fri, Aug 30 2024 at 16:46, Yan Zhen wrote: > Switch to use dev_err_probe() to simplify the error path and > unify a message template. > > Using this helper is totally fine even if err is known to never > be -EPROBE_DEFER. Is fine? We don't care about fine. The question is whether it is correct or not. And if so, then there wants to be an explanation why it is correct. > Changes in v3: > -Rewrite the subject as 'irqchip: madera:'. Which is still wrong. Care to read Documentation/process/ ? > drivers/irqchip/irq-madera.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/irqchip/irq-madera.c b/drivers/irqchip/irq-madera.c > index acceb6e7fa95..d5ad4466a140 100644 > --- a/drivers/irqchip/irq-madera.c > +++ b/drivers/irqchip/irq-madera.c > @@ -199,9 +199,8 @@ static int madera_irq_probe(struct platform_device *pdev) > ret = regmap_update_bits(madera->regmap, MADERA_IRQ1_CTRL, > MADERA_IRQ_POL_MASK, 0); > if (ret) { > - dev_err(&pdev->dev, > - "Failed to set IRQ polarity: %d\n", ret); > - return ret; > + return dev_err_probe(&pdev->dev, ret, > + "Failed to set IRQ polarity"); https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#line-breaks But that's moot as Krzysztof pointed out that this patch just picks a random instance of the code for no reason. Thanks, tglx
diff --git a/drivers/irqchip/irq-madera.c b/drivers/irqchip/irq-madera.c index acceb6e7fa95..d5ad4466a140 100644 --- a/drivers/irqchip/irq-madera.c +++ b/drivers/irqchip/irq-madera.c @@ -199,9 +199,8 @@ static int madera_irq_probe(struct platform_device *pdev) ret = regmap_update_bits(madera->regmap, MADERA_IRQ1_CTRL, MADERA_IRQ_POL_MASK, 0); if (ret) { - dev_err(&pdev->dev, - "Failed to set IRQ polarity: %d\n", ret); - return ret; + return dev_err_probe(&pdev->dev, ret, + "Failed to set IRQ polarity"); } }
Switch to use dev_err_probe() to simplify the error path and unify a message template. Using this helper is totally fine even if err is known to never be -EPROBE_DEFER. The benefit compared to a normal dev_err() is the standardized format of the error code, it being emitted symbolically and the fact that the error code is returned which allows more compact error paths. Signed-off-by: Yan Zhen <yanzhen@vivo.com> --- Changes in v3: -Rewrite the subject as 'irqchip: madera:'. drivers/irqchip/irq-madera.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)