[PATCH 1/4] ASoC: codecs: wcd9335: Simplify with dev_err_probe
Code can be a bit simpler with dev_err_probe().
Signed-off-by: Krzysztof Kozlowski krzysztof.kozlowski@linaro.org --- sound/soc/codecs/wcd9335.c | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-)
diff --git a/sound/soc/codecs/wcd9335.c b/sound/soc/codecs/wcd9335.c index d2548fdf9ae5..8bf3510a3ea3 100644 --- a/sound/soc/codecs/wcd9335.c +++ b/sound/soc/codecs/wcd9335.c @@ -5138,20 +5138,17 @@ static int wcd9335_irq_init(struct wcd9335_codec *wcd) * INTR2 is a subset of first interrupt sources MAD, VBAT, and SVA */ wcd->intr1 = of_irq_get_byname(wcd->dev->of_node, "intr1"); - if (wcd->intr1 < 0) { - if (wcd->intr1 != -EPROBE_DEFER) - dev_err(wcd->dev, "Unable to configure IRQ\n"); - - return wcd->intr1; - } + if (wcd->intr1 < 0) + return dev_err_probe(wcd->dev, wcd->intr1, + "Unable to configure IRQ\n");
ret = devm_regmap_add_irq_chip(wcd->dev, wcd->regmap, wcd->intr1, IRQF_TRIGGER_HIGH, 0, &wcd9335_regmap_irq1_chip, &wcd->irq_data); if (ret) - dev_err(wcd->dev, "Failed to register IRQ chip: %d\n", ret); + return dev_err_probe(wcd->dev, ret, "Failed to register IRQ chip\n");
- return ret; + return 0; }
static int wcd9335_slim_probe(struct slim_device *slim) @@ -5207,17 +5204,15 @@ static int wcd9335_slim_status(struct slim_device *sdev, slim_get_logical_addr(wcd->slim_ifc_dev);
wcd->regmap = regmap_init_slimbus(sdev, &wcd9335_regmap_config); - if (IS_ERR(wcd->regmap)) { - dev_err(dev, "Failed to allocate slim register map\n"); - return PTR_ERR(wcd->regmap); - } + if (IS_ERR(wcd->regmap)) + return dev_err_probe(dev, PTR_ERR(wcd->regmap), + "Failed to allocate slim register map\n");
wcd->if_regmap = regmap_init_slimbus(wcd->slim_ifc_dev, &wcd9335_ifc_regmap_config); - if (IS_ERR(wcd->if_regmap)) { - dev_err(dev, "Failed to allocate ifc register map\n"); - return PTR_ERR(wcd->if_regmap); - } + if (IS_ERR(wcd->if_regmap)) + return dev_err_probe(dev, PTR_ERR(wcd->if_regmap), + "Failed to allocate ifc register map\n");
ret = wcd9335_bring_up(wcd); if (ret) {
Code can be a bit simpler with dev_err_probe().
Signed-off-by: Krzysztof Kozlowski krzysztof.kozlowski@linaro.org --- sound/soc/codecs/wcd934x.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/sound/soc/codecs/wcd934x.c b/sound/soc/codecs/wcd934x.c index 783479a4d535..56487ad2f048 100644 --- a/sound/soc/codecs/wcd934x.c +++ b/sound/soc/codecs/wcd934x.c @@ -5868,10 +5868,9 @@ static int wcd934x_codec_parse_data(struct wcd934x_codec *wcd) slim_get_logical_addr(wcd->sidev); wcd->if_regmap = regmap_init_slimbus(wcd->sidev, &wcd934x_ifc_regmap_config); - if (IS_ERR(wcd->if_regmap)) { - dev_err(dev, "Failed to allocate ifc register map\n"); - return PTR_ERR(wcd->if_regmap); - } + if (IS_ERR(wcd->if_regmap)) + return dev_err_probe(dev, PTR_ERR(wcd->if_regmap), + "Failed to allocate ifc register map\n");
of_property_read_u32(dev->parent->of_node, "qcom,dmic-sample-rate", &wcd->dmic_sample_rate); @@ -5923,19 +5922,15 @@ static int wcd934x_codec_probe(struct platform_device *pdev) memcpy(wcd->tx_chs, wcd934x_tx_chs, sizeof(wcd934x_tx_chs));
irq = regmap_irq_get_virq(data->irq_data, WCD934X_IRQ_SLIMBUS); - if (irq < 0) { - dev_err(wcd->dev, "Failed to get SLIM IRQ\n"); - return irq; - } + if (irq < 0) + return dev_err_probe(wcd->dev, irq, "Failed to get SLIM IRQ\n");
ret = devm_request_threaded_irq(dev, irq, NULL, wcd934x_slim_irq_handler, IRQF_TRIGGER_RISING | IRQF_ONESHOT, "slim", wcd); - if (ret) { - dev_err(dev, "Failed to request slimbus irq\n"); - return ret; - } + if (ret) + return dev_err_probe(dev, ret, "Failed to request slimbus irq\n");
wcd934x_register_mclk_output(wcd); platform_set_drvdata(pdev, wcd);
On Mon, Apr 17, 2023 at 04:14:51PM +0200, Krzysztof Kozlowski wrote:
Code can be a bit simpler with dev_err_probe().
- if (IS_ERR(wcd->if_regmap)) {
dev_err(dev, "Failed to allocate ifc register map\n");return PTR_ERR(wcd->if_regmap);- }
- if (IS_ERR(wcd->if_regmap))
return dev_err_probe(dev, PTR_ERR(wcd->if_regmap),"Failed to allocate ifc register map\n");
This is a functional change.
On 17/04/2023 17:33, Mark Brown wrote:
On Mon, Apr 17, 2023 at 04:14:51PM +0200, Krzysztof Kozlowski wrote:
Code can be a bit simpler with dev_err_probe().
- if (IS_ERR(wcd->if_regmap)) {
dev_err(dev, "Failed to allocate ifc register map\n");return PTR_ERR(wcd->if_regmap);- }
- if (IS_ERR(wcd->if_regmap))
return dev_err_probe(dev, PTR_ERR(wcd->if_regmap),"Failed to allocate ifc register map\n");This is a functional change.
Hmm... I don't see it. Return value is the same, same message is printed, same condition. Did I make some copy-paste error?
Best regards, Krzysztof
On Mon, Apr 17, 2023 at 05:43:03PM +0200, Krzysztof Kozlowski wrote:
On 17/04/2023 17:33, Mark Brown wrote:
On Mon, Apr 17, 2023 at 04:14:51PM +0200, Krzysztof Kozlowski wrote:
- if (IS_ERR(wcd->if_regmap)) {
dev_err(dev, "Failed to allocate ifc register map\n");return PTR_ERR(wcd->if_regmap);- }
- if (IS_ERR(wcd->if_regmap))
return dev_err_probe(dev, PTR_ERR(wcd->if_regmap),"Failed to allocate ifc register map\n");
This is a functional change.
Hmm... I don't see it. Return value is the same, same message is printed, same condition. Did I make some copy-paste error?
You've replaced an unconditional dev_err() with dev_err_probe().
On 17/04/2023 17:58, Mark Brown wrote:
On Mon, Apr 17, 2023 at 05:43:03PM +0200, Krzysztof Kozlowski wrote:
On 17/04/2023 17:33, Mark Brown wrote:
On Mon, Apr 17, 2023 at 04:14:51PM +0200, Krzysztof Kozlowski wrote:
- if (IS_ERR(wcd->if_regmap)) {
dev_err(dev, "Failed to allocate ifc register map\n");return PTR_ERR(wcd->if_regmap);- }
- if (IS_ERR(wcd->if_regmap))
return dev_err_probe(dev, PTR_ERR(wcd->if_regmap),"Failed to allocate ifc register map\n");This is a functional change.
Hmm... I don't see it. Return value is the same, same message is printed, same condition. Did I make some copy-paste error?
You've replaced an unconditional dev_err() with dev_err_probe().
Which is the core of this change... so what is here surprising? Yes, that's functional change and I never wrote that dev_err_probe is equal dev_err. It is similar but offers benefits and one difference - does not print DEFER. Which is in general exactly what we want.
Best regards, Krzysztof
On Mon, Apr 17, 2023 at 06:00:59PM +0200, Krzysztof Kozlowski wrote:
On 17/04/2023 17:58, Mark Brown wrote:
You've replaced an unconditional dev_err() with dev_err_probe().
Which is the core of this change... so what is here surprising? Yes, that's functional change and I never wrote that dev_err_probe is equal dev_err. It is similar but offers benefits and one difference - does not print DEFER. Which is in general exactly what we want.
This may well be what you intended to do but it's not what the commit message says that the change is doing, unlike patch 1 this isn't an open coded dev_err_probe() that's being replaced.
On 17/04/2023 18:27, Mark Brown wrote:
On Mon, Apr 17, 2023 at 06:00:59PM +0200, Krzysztof Kozlowski wrote:
On 17/04/2023 17:58, Mark Brown wrote:
You've replaced an unconditional dev_err() with dev_err_probe().
Which is the core of this change... so what is here surprising? Yes, that's functional change and I never wrote that dev_err_probe is equal dev_err. It is similar but offers benefits and one difference - does not print DEFER. Which is in general exactly what we want.
This may well be what you intended to do but it's not what the commit message says that the change is doing, unlike patch 1 this isn't an open coded dev_err_probe() that's being replaced.
But my patch 1 (and my other patches some time ago for wsa speakers) does exactly the same as this one here in few other places - introduces better message printing of EPROBE_DEFER. Only in one place it is equivalent.
If you prefer, I can mention the message/EPROBE_DEFER difference in commit msgs.
Best regards, Krzysztof
On Mon, Apr 17, 2023 at 06:32:07PM +0200, Krzysztof Kozlowski wrote:
If you prefer, I can mention the message/EPROBE_DEFER difference in commit msgs.
I know you prefer to maintain exacting standards in these areas.
On 17/04/2023 19:23, Mark Brown wrote:
On Mon, Apr 17, 2023 at 06:32:07PM +0200, Krzysztof Kozlowski wrote:
If you prefer, I can mention the message/EPROBE_DEFER difference in commit msgs.
I know you prefer to maintain exacting standards in these areas.
I don't understand what do you mean here. Do you wish me to re-phrase the commit msg or change something in the code?
Best regards, Krzysztof
On Mon, Apr 17, 2023 at 07:30:35PM +0200, Krzysztof Kozlowski wrote:
On 17/04/2023 19:23, Mark Brown wrote:
On Mon, Apr 17, 2023 at 06:32:07PM +0200, Krzysztof Kozlowski wrote:
If you prefer, I can mention the message/EPROBE_DEFER difference in commit msgs.
I know you prefer to maintain exacting standards in these areas.
I don't understand what do you mean here. Do you wish me to re-phrase the commit msg or change something in the code?
Your commit message does not accurately describe the change the patch makes, it should do so.
The probe already stores pointer to &pdev->dev, so use it to make the code a bit easier to read.
Signed-off-by: Krzysztof Kozlowski krzysztof.kozlowski@linaro.org --- sound/soc/codecs/wcd934x.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/soc/codecs/wcd934x.c b/sound/soc/codecs/wcd934x.c index 56487ad2f048..c0d1fa36d841 100644 --- a/sound/soc/codecs/wcd934x.c +++ b/sound/soc/codecs/wcd934x.c @@ -5892,12 +5892,12 @@ static int wcd934x_codec_parse_data(struct wcd934x_codec *wcd)
static int wcd934x_codec_probe(struct platform_device *pdev) { - struct wcd934x_ddata *data = dev_get_drvdata(pdev->dev.parent); - struct wcd934x_codec *wcd; struct device *dev = &pdev->dev; + struct wcd934x_ddata *data = dev_get_drvdata(dev->parent); + struct wcd934x_codec *wcd; int ret, irq;
- wcd = devm_kzalloc(&pdev->dev, sizeof(*wcd), GFP_KERNEL); + wcd = devm_kzalloc(dev, sizeof(*wcd), GFP_KERNEL); if (!wcd) return -ENOMEM;
Code can be a bit simpler with dev_err_probe().
Signed-off-by: Krzysztof Kozlowski krzysztof.kozlowski@linaro.org --- sound/soc/codecs/wcd938x.c | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-)
diff --git a/sound/soc/codecs/wcd938x.c b/sound/soc/codecs/wcd938x.c index f033f79ed238..11b264a63b04 100644 --- a/sound/soc/codecs/wcd938x.c +++ b/sound/soc/codecs/wcd938x.c @@ -4235,18 +4235,15 @@ static int wcd938x_populate_dt_data(struct wcd938x_priv *wcd938x, struct device int ret;
wcd938x->reset_gpio = of_get_named_gpio(dev->of_node, "reset-gpios", 0); - if (wcd938x->reset_gpio < 0) { - dev_err(dev, "Failed to get reset gpio: err = %d\n", - wcd938x->reset_gpio); - return wcd938x->reset_gpio; - } + if (wcd938x->reset_gpio < 0) + return dev_err_probe(dev, wcd938x->reset_gpio, + "Failed to get reset gpio\n");
wcd938x->us_euro_gpio = devm_gpiod_get_optional(dev, "us-euro", GPIOD_OUT_LOW); - if (IS_ERR(wcd938x->us_euro_gpio)) { - dev_err(dev, "us-euro swap Control GPIO not found\n"); - return PTR_ERR(wcd938x->us_euro_gpio); - } + if (IS_ERR(wcd938x->us_euro_gpio)) + return dev_err_probe(dev, PTR_ERR(wcd938x->us_euro_gpio), + "us-euro swap Control GPIO not found\n");
cfg->swap_gnd_mic = wcd938x_swap_gnd_mic;
@@ -4256,16 +4253,12 @@ static int wcd938x_populate_dt_data(struct wcd938x_priv *wcd938x, struct device wcd938x->supplies[3].supply = "vdd-mic-bias";
ret = regulator_bulk_get(dev, WCD938X_MAX_SUPPLY, wcd938x->supplies); - if (ret) { - dev_err(dev, "Failed to get supplies: err = %d\n", ret); - return ret; - } + if (ret) + return dev_err_probe(dev, ret, "Failed to get supplies\n");
ret = regulator_bulk_enable(WCD938X_MAX_SUPPLY, wcd938x->supplies); - if (ret) { - dev_err(dev, "Failed to enable supplies: err = %d\n", ret); - return ret; - } + if (ret) + return dev_err_probe(dev, ret, "Failed to enable supplies\n");
wcd938x_dt_parse_micbias_info(dev, wcd938x);
On Mon, 17 Apr 2023 16:14:50 +0200, Krzysztof Kozlowski wrote:
Code can be a bit simpler with dev_err_probe().
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/4] ASoC: codecs: wcd9335: Simplify with dev_err_probe commit: 67380533d450000699848e292d20ec18d0321f0e
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
participants (2)
-
Krzysztof Kozlowski -
Mark Brown