[PATCH] ASoC: rt5645: Avoid upgrading static warnings to errors

[PATCH] ASoC: rt5645: Avoid upgrading static warnings to errors

[Mark Brown]

One of the fixes reverted as part of the UMN fallout was actually fine,
however rather than undoing the revert the process that handled all this
stuff resulted in a patch which attempted to add extra error checks
instead.  Unfortunately this new change wasn't really based on a good
understanding of the subsystem APIs and bypassed the usual patch flow
without ensuring it was reviewed by people with subsystem knowledge and
was merged as a fix rather than during the merge window.

The effect of the new fix is to upgrade what were previously warnings on
static data in the code to hard errors on that data.  If this actually
happens then it would break existing systems, if it doesn't happen then
the change has no effect so this was not a safe change to apply as a fix
to the release candidates.  Since the new code has not been tested and
doesn't in practice improve error handling revert it instead, and also
drop the original revert since the original fix was fine.  This takes
the driver back to what it was in -rc1.

Fixes: 5e70b8e22b64e ("ASoC: rt5645: add error checking to rt5645_probe function")
Fixes: 1e0ce84215dbf ("Revert "ASoC: rt5645: fix a NULL pointer dereference")
Signed-off-by: Mark Brown <broonie@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Phillip Potter <phil@philpotter.co.uk>
---
 sound/soc/codecs/rt5645.c | 49 +++++++++------------------------------
 1 file changed, 11 insertions(+), 38 deletions(-)

diff --git a/sound/soc/codecs/rt5645.c b/sound/soc/codecs/rt5645.c
index 438fa18bcb55..9408ee63cb26 100644
--- a/sound/soc/codecs/rt5645.c
+++ b/sound/soc/codecs/rt5645.c
@@ -3388,44 +3388,30 @@ static int rt5645_probe(struct snd_soc_component *component)
 {
 	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
 	struct rt5645_priv *rt5645 = snd_soc_component_get_drvdata(component);
-	int ret = 0;
 
 	rt5645->component = component;
 
 	switch (rt5645->codec_type) {
 	case CODEC_TYPE_RT5645:
-		ret = snd_soc_dapm_new_controls(dapm,
+		snd_soc_dapm_new_controls(dapm,
 			rt5645_specific_dapm_widgets,
 			ARRAY_SIZE(rt5645_specific_dapm_widgets));
-		if (ret < 0)
-			goto exit;
-
-		ret = snd_soc_dapm_add_routes(dapm,
+		snd_soc_dapm_add_routes(dapm,
 			rt5645_specific_dapm_routes,
 			ARRAY_SIZE(rt5645_specific_dapm_routes));
-		if (ret < 0)
-			goto exit;
-
 		if (rt5645->v_id < 3) {
-			ret = snd_soc_dapm_add_routes(dapm,
+			snd_soc_dapm_add_routes(dapm,
 				rt5645_old_dapm_routes,
 				ARRAY_SIZE(rt5645_old_dapm_routes));
-			if (ret < 0)
-				goto exit;
 		}
 		break;
 	case CODEC_TYPE_RT5650:
-		ret = snd_soc_dapm_new_controls(dapm,
+		snd_soc_dapm_new_controls(dapm,
 			rt5650_specific_dapm_widgets,
 			ARRAY_SIZE(rt5650_specific_dapm_widgets));
-		if (ret < 0)
-			goto exit;
-
-		ret = snd_soc_dapm_add_routes(dapm,
+		snd_soc_dapm_add_routes(dapm,
 			rt5650_specific_dapm_routes,
 			ARRAY_SIZE(rt5650_specific_dapm_routes));
-		if (ret < 0)
-			goto exit;
 		break;
 	}
 
@@ -3433,17 +3419,9 @@ static int rt5645_probe(struct snd_soc_component *component)
 
 	/* for JD function */
 	if (rt5645->pdata.jd_mode) {
-		ret = snd_soc_dapm_force_enable_pin(dapm, "JD Power");
-		if (ret < 0)
-			goto exit;
-
-		ret = snd_soc_dapm_force_enable_pin(dapm, "LDO2");
-		if (ret < 0)
-			goto exit;
-
-		ret = snd_soc_dapm_sync(dapm);
-		if (ret < 0)
-			goto exit;
+		snd_soc_dapm_force_enable_pin(dapm, "JD Power");
+		snd_soc_dapm_force_enable_pin(dapm, "LDO2");
+		snd_soc_dapm_sync(dapm);
 	}
 
 	if (rt5645->pdata.long_name)
@@ -3454,14 +3432,9 @@ static int rt5645_probe(struct snd_soc_component *component)
 		GFP_KERNEL);
 
 	if (!rt5645->eq_param)
-		ret = -ENOMEM;
-exit:
-	/*
-	 * If there was an error above, everything will be cleaned up by the
-	 * caller if we return an error here.  This will be done with a later
-	 * call to rt5645_remove().
-	 */
-	return ret;
+		return -ENOMEM;
+
+	return 0;
 }
 
 static void rt5645_remove(struct snd_soc_component *component)
-- 
2.20.1

Re: [PATCH] ASoC: rt5645: Avoid upgrading static warnings to errors

[Greg Kroah-Hartman]

On Tue, Jun 08, 2021 at 05:07:13PM +0100, Mark Brown wrote:
> One of the fixes reverted as part of the UMN fallout was actually fine,
> however rather than undoing the revert the process that handled all this
> stuff resulted in a patch which attempted to add extra error checks
> instead.  Unfortunately this new change wasn't really based on a good
> understanding of the subsystem APIs and bypassed the usual patch flow
> without ensuring it was reviewed by people with subsystem knowledge and
> was merged as a fix rather than during the merge window.
> 
> The effect of the new fix is to upgrade what were previously warnings on
> static data in the code to hard errors on that data.  If this actually
> happens then it would break existing systems, if it doesn't happen then
> the change has no effect so this was not a safe change to apply as a fix
> to the release candidates.  Since the new code has not been tested and
> doesn't in practice improve error handling revert it instead, and also
> drop the original revert since the original fix was fine.  This takes
> the driver back to what it was in -rc1.
> 
> Fixes: 5e70b8e22b64e ("ASoC: rt5645: add error checking to rt5645_probe function")
> Fixes: 1e0ce84215dbf ("Revert "ASoC: rt5645: fix a NULL pointer dereference")
> Signed-off-by: Mark Brown <broonie@kernel.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Phillip Potter <phil@philpotter.co.uk>
> ---
>  sound/soc/codecs/rt5645.c | 49 +++++++++------------------------------
>  1 file changed, 11 insertions(+), 38 deletions(-)

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH] ASoC: rt5645: Avoid upgrading static warnings to errors

[Mark Brown]

On Tue, 8 Jun 2021 17:07:13 +0100, Mark Brown wrote:
> One of the fixes reverted as part of the UMN fallout was actually fine,
> however rather than undoing the revert the process that handled all this
> stuff resulted in a patch which attempted to add extra error checks
> instead.  Unfortunately this new change wasn't really based on a good
> understanding of the subsystem APIs and bypassed the usual patch flow
> without ensuring it was reviewed by people with subsystem knowledge and
> was merged as a fix rather than during the merge window.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: rt5645: Avoid upgrading static warnings to errors
      commit: 916cccb5078eee57fce131c5fe18e417545083e2

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