Merge series "Tidy up device ID reading on legacy Cirrus parts" from Charles...

Merge series "Tidy up device ID reading on legacy Cirrus parts" from Charles Keepax <ckeepax@opensource.cirrus.com>:

Pierre requested I have a look at some cppcheck warnings in the cs42l42
driver, since it is reassigning the ret variable without ever checking
the result.  Looking a bit more broadly this happens in quite a few
legacy Cirrus parts, as they all use the same process to read the ID,
factor out a small helper so they can all share the same code. Whilst
in there fix up a couple of other trivial error path issues as well.

Thanks,
Charles

Charles Keepax (10):
  ASoC: cirrus: Add helper function for reading the device ID
  ASoC: cs35l32: Minor error paths fixups
  ASoC: cs35l33: Minor error paths fixups
  ASoC: cs35l34:  Minor error paths fixups
  ASoC: cs35l35:  Minor error paths fixups
  ASoC: cs35l35: Correct errata handling
  ASoC: cs42l42:  Minor error paths fixups
  ASoC: cs42l73:  Minor error paths fixups
  ASoC: cs43130:  Minor error paths fixups
  ASoC: cs53l30:  Minor error paths fixups

 sound/soc/codecs/cirrus_legacy.h | 21 +++++++++++++++++++++
 sound/soc/codecs/cs35l32.c       | 34 ++++++++++++++++++----------------
 sound/soc/codecs/cs35l33.c       | 15 +++++++++------
 sound/soc/codecs/cs35l34.c       | 39 ++++++++++++++++++++++-----------------
 sound/soc/codecs/cs35l35.c       | 21 ++++++++++-----------
 sound/soc/codecs/cs35l35.h       |  1 +
 sound/soc/codecs/cs42l42.c       | 18 ++++++++----------
 sound/soc/codecs/cs42l73.c       | 30 +++++++++++++++++-------------
 sound/soc/codecs/cs43130.c       | 31 +++++++++++++++++++------------
 sound/soc/codecs/cs53l30.c       | 22 +++++++++++-----------
 10 files changed, 136 insertions(+), 96 deletions(-)
 create mode 100644 sound/soc/codecs/cirrus_legacy.h

--
2.11.0

sound/soc/codecs/cirrus_legacy.h

+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Some small helpers for older Cirrus Logic parts.
+ *
+ * Copyright (C) 2021 Cirrus Logic, Inc. and
+ *                    Cirrus Logic International Semiconductor Ltd.
+ */
+
+static inline int cirrus_read_device_id(struct regmap *regmap, unsigned int reg)
+{
+	u8 devid[3];
+	int ret;
+
+	ret = regmap_bulk_read(regmap, reg, devid, ARRAY_SIZE(devid));
+	if (ret < 0)
+		return ret;
+
+	return ((devid[0] & 0xFF) << 12) |
+	       ((devid[1] & 0xFF) <<  4) |
+	       ((devid[2] & 0xF0) >>  4);
+}

sound/soc/codecs/cs35l32.c

@@ -30,6 +30,7 @@
 #include <dt-bindings/sound/cs35l32.h>
 
 #include "cs35l32.h"
+#include "cirrus_legacy.h"
 
 #define CS35L32_NUM_SUPPLIES 2
 static const char *const cs35l32_supply_names[CS35L32_NUM_SUPPLIES] = {
@@ -348,8 +349,7 @@ static int cs35l32_i2c_probe(struct i2c_client *i2c_client,
 	struct cs35l32_private *cs35l32;
 	struct cs35l32_platform_data *pdata =
 		dev_get_platdata(&i2c_client->dev);
-	int ret, i;
-	unsigned int devid = 0;
+	int ret, i, devid;
 	unsigned int reg;
 
 	cs35l32 = devm_kzalloc(&i2c_client->dev, sizeof(*cs35l32), GFP_KERNEL);
@@ -404,40 +404,40 @@ static int cs35l32_i2c_probe(struct i2c_client *i2c_client,
 	/* Reset the Device */
 	cs35l32->reset_gpio = devm_gpiod_get_optional(&i2c_client->dev,
 		"reset", GPIOD_OUT_LOW);
-	if (IS_ERR(cs35l32->reset_gpio))
-		return PTR_ERR(cs35l32->reset_gpio);
+	if (IS_ERR(cs35l32->reset_gpio)) {
+		ret = PTR_ERR(cs35l32->reset_gpio);
+		goto err_supplies;
+	}
 
 	gpiod_set_value_cansleep(cs35l32->reset_gpio, 1);
 
 	/* initialize codec */
-	ret = regmap_read(cs35l32->regmap, CS35L32_DEVID_AB, &reg);
-	devid = (reg & 0xFF) << 12;
-
-	ret = regmap_read(cs35l32->regmap, CS35L32_DEVID_CD, &reg);
-	devid |= (reg & 0xFF) << 4;
-
-	ret = regmap_read(cs35l32->regmap, CS35L32_DEVID_E, &reg);
-	devid |= (reg & 0xF0) >> 4;
+	devid = cirrus_read_device_id(cs35l32->regmap, CS35L32_DEVID_AB);
+	if (devid < 0) {
+		ret = devid;
+		dev_err(&i2c_client->dev, "Failed to read device ID: %d\n", ret);
+		goto err_disable;
+	}
 
 	if (devid != CS35L32_CHIP_ID) {
 		ret = -ENODEV;
 		dev_err(&i2c_client->dev,
 			"CS35L32 Device ID (%X). Expected %X\n",
 			devid, CS35L32_CHIP_ID);
-		return ret;
+		goto err_disable;
 	}
 
 	ret = regmap_read(cs35l32->regmap, CS35L32_REV_ID, &reg);
 	if (ret < 0) {
 		dev_err(&i2c_client->dev, "Get Revision ID failed\n");
-		return ret;
+		goto err_disable;
 	}
 
 	ret = regmap_register_patch(cs35l32->regmap, cs35l32_monitor_patch,
 				    ARRAY_SIZE(cs35l32_monitor_patch));
 	if (ret < 0) {
 		dev_err(&i2c_client->dev, "Failed to apply errata patch\n");
-		return ret;
+		goto err_disable;
 	}
 
 	dev_info(&i2c_client->dev,
@@ -478,7 +478,7 @@ static int cs35l32_i2c_probe(struct i2c_client *i2c_client,
 			    CS35L32_PDN_AMP);
 
 	/* Clear MCLK Error Bit since we don't have the clock yet */
-	ret = regmap_read(cs35l32->regmap, CS35L32_INT_STATUS_1, &reg);
+	regmap_read(cs35l32->regmap, CS35L32_INT_STATUS_1, &reg);
 
 	ret = devm_snd_soc_register_component(&i2c_client->dev,
 			&soc_component_dev_cs35l32, cs35l32_dai,
@@ -489,6 +489,8 @@ static int cs35l32_i2c_probe(struct i2c_client *i2c_client,
 	return 0;
 
 err_disable:
+	gpiod_set_value_cansleep(cs35l32->reset_gpio, 0);
+err_supplies:
 	regulator_bulk_disable(ARRAY_SIZE(cs35l32->supplies),
 			       cs35l32->supplies);
 	return ret;

sound/soc/codecs/cs35l33.c

@@ -34,6 +34,7 @@
 #include <linux/of_irq.h>
 
 #include "cs35l33.h"
+#include "cirrus_legacy.h"
 
 #define CS35L33_BOOT_DELAY	50
 
@@ -1190,12 +1191,12 @@ static int cs35l33_i2c_probe(struct i2c_client *i2c_client,
 	regcache_cache_only(cs35l33->regmap, false);
 
 	/* initialize codec */
-	ret = regmap_read(cs35l33->regmap, CS35L33_DEVID_AB, &reg);
-	devid = (reg & 0xFF) << 12;
-	ret = regmap_read(cs35l33->regmap, CS35L33_DEVID_CD, &reg);
-	devid |= (reg & 0xFF) << 4;
-	ret = regmap_read(cs35l33->regmap, CS35L33_DEVID_E, &reg);
-	devid |= (reg & 0xF0) >> 4;
+	devid = cirrus_read_device_id(cs35l33->regmap, CS35L33_DEVID_AB);
+	if (devid < 0) {
+		ret = devid;
+		dev_err(&i2c_client->dev, "Failed to read device ID: %d\n", ret);
+		goto err_enable;
+	}
 
 	if (devid != CS35L33_CHIP_ID) {
 		dev_err(&i2c_client->dev,
@@ -1242,6 +1243,8 @@ static int cs35l33_i2c_probe(struct i2c_client *i2c_client,
 	return 0;
 
 err_enable:
+	gpiod_set_value_cansleep(cs35l33->reset_gpio, 0);
+
 	regulator_bulk_disable(cs35l33->num_core_supplies,
 			       cs35l33->core_supplies);
 

sound/soc/codecs/cs35l34.c

@@ -34,6 +34,7 @@
 #include <sound/cs35l34.h>
 
 #include "cs35l34.h"
+#include "cirrus_legacy.h"
 
 #define PDN_DONE_ATTEMPTS 10
 #define CS35L34_START_DELAY 50
@@ -996,9 +997,8 @@ static int cs35l34_i2c_probe(struct i2c_client *i2c_client,
 	struct cs35l34_private *cs35l34;
 	struct cs35l34_platform_data *pdata =
 		dev_get_platdata(&i2c_client->dev);
-	int i;
+	int i, devid;
 	int ret;
-	unsigned int devid = 0;
 	unsigned int reg;
 
 	cs35l34 = devm_kzalloc(&i2c_client->dev, sizeof(*cs35l34), GFP_KERNEL);
@@ -1039,13 +1039,15 @@ static int cs35l34_i2c_probe(struct i2c_client *i2c_client,
 	} else {
 		pdata = devm_kzalloc(&i2c_client->dev, sizeof(*pdata),
 				     GFP_KERNEL);
-		if (!pdata)
-			return -ENOMEM;
+		if (!pdata) {
+			ret = -ENOMEM;
+			goto err_regulator;
+		}
 
 		if (i2c_client->dev.of_node) {
 			ret = cs35l34_handle_of_data(i2c_client, pdata);
 			if (ret != 0)
-				return ret;
+				goto err_regulator;
 
 		}
 		cs35l34->pdata = *pdata;
@@ -1059,33 +1061,34 @@ static int cs35l34_i2c_probe(struct i2c_client *i2c_client,
 
 	cs35l34->reset_gpio = devm_gpiod_get_optional(&i2c_client->dev,
 				"reset-gpios", GPIOD_OUT_LOW);
-	if (IS_ERR(cs35l34->reset_gpio))
-		return PTR_ERR(cs35l34->reset_gpio);
+	if (IS_ERR(cs35l34->reset_gpio)) {
+		ret = PTR_ERR(cs35l34->reset_gpio);
+		goto err_regulator;
+	}
 
 	gpiod_set_value_cansleep(cs35l34->reset_gpio, 1);
 
 	msleep(CS35L34_START_DELAY);
 
-	ret = regmap_read(cs35l34->regmap, CS35L34_DEVID_AB, &reg);
-
-	devid = (reg & 0xFF) << 12;
-	ret = regmap_read(cs35l34->regmap, CS35L34_DEVID_CD, &reg);
-	devid |= (reg & 0xFF) << 4;
-	ret = regmap_read(cs35l34->regmap, CS35L34_DEVID_E, &reg);
-	devid |= (reg & 0xF0) >> 4;
+	devid = cirrus_read_device_id(cs35l34->regmap, CS35L34_DEVID_AB);
+	if (devid < 0) {
+		ret = devid;
+		dev_err(&i2c_client->dev, "Failed to read device ID: %d\n", ret);
+		goto err_reset;
+	}
 
 	if (devid != CS35L34_CHIP_ID) {
 		dev_err(&i2c_client->dev,
 			"CS35l34 Device ID (%X). Expected ID %X\n",
 			devid, CS35L34_CHIP_ID);
 		ret = -ENODEV;
-		goto err_regulator;
+		goto err_reset;
 	}
 
 	ret = regmap_read(cs35l34->regmap, CS35L34_REV_ID, &reg);
 	if (ret < 0) {
 		dev_err(&i2c_client->dev, "Get Revision ID failed\n");
-		goto err_regulator;
+		goto err_reset;
 	}
 
 	dev_info(&i2c_client->dev,
@@ -1110,11 +1113,13 @@ static int cs35l34_i2c_probe(struct i2c_client *i2c_client,
 	if (ret < 0) {
 		dev_err(&i2c_client->dev,
 			"%s: Register component failed\n", __func__);
-		goto err_regulator;
+		goto err_reset;
 	}
 
 	return 0;
 
+err_reset:
+	gpiod_set_value_cansleep(cs35l34->reset_gpio, 0);
 err_regulator:
 	regulator_bulk_disable(cs35l34->num_core_supplies,
 		cs35l34->core_supplies);

sound/soc/codecs/cs35l35.c

@@ -33,6 +33,7 @@
 #include <linux/completion.h>
 
 #include "cs35l35.h"
+#include "cirrus_legacy.h"
 
 /*
  * Some fields take zero as a valid value so use a high bit flag that won't
@@ -495,10 +496,10 @@ static int cs35l35_hw_params(struct snd_pcm_substream *substream,
 	 * the Class H algorithm does not enable weak-drive operation for
 	 * nonzero values of CH_WKFET_DELAY if SP_RATE = 01 or 10
 	 */
-	errata_chk = clk_ctl & CS35L35_SP_RATE_MASK;
+	errata_chk = (clk_ctl & CS35L35_SP_RATE_MASK) >> CS35L35_SP_RATE_SHIFT;
 
 	if (classh->classh_wk_fet_disable == 0x00 &&
-		(errata_chk == 0x01 || errata_chk == 0x03)) {
+		(errata_chk == 0x01 || errata_chk == 0x02)) {
 		ret = regmap_update_bits(cs35l35->regmap,
 					CS35L35_CLASS_H_FET_DRIVE_CTL,
 					CS35L35_CH_WKFET_DEL_MASK,
@@ -1471,9 +1472,8 @@ static int cs35l35_i2c_probe(struct i2c_client *i2c_client,
 	struct cs35l35_private *cs35l35;
 	struct device *dev = &i2c_client->dev;
 	struct cs35l35_platform_data *pdata = dev_get_platdata(dev);
-	int i;
+	int i, devid;
 	int ret;
-	unsigned int devid = 0;
 	unsigned int reg;
 
 	cs35l35 = devm_kzalloc(dev, sizeof(struct cs35l35_private), GFP_KERNEL);
@@ -1552,13 +1552,12 @@ static int cs35l35_i2c_probe(struct i2c_client *i2c_client,
 		goto err;
 	}
 	/* initialize codec */
-	ret = regmap_read(cs35l35->regmap, CS35L35_DEVID_AB, &reg);
-
-	devid = (reg & 0xFF) << 12;
-	ret = regmap_read(cs35l35->regmap, CS35L35_DEVID_CD, &reg);
-	devid |= (reg & 0xFF) << 4;
-	ret = regmap_read(cs35l35->regmap, CS35L35_DEVID_E, &reg);
-	devid |= (reg & 0xF0) >> 4;
+	devid = cirrus_read_device_id(cs35l35->regmap, CS35L35_DEVID_AB);
+	if (devid < 0) {
+		ret = devid;
+		dev_err(dev, "Failed to read device ID: %d\n", ret);
+		goto err;
+	}
 
 	if (devid != CS35L35_CHIP_ID) {
 		dev_err(dev, "CS35L35 Device ID (%X). Expected ID %X\n",

sound/soc/codecs/cs35l35.h

@@ -168,6 +168,7 @@
 #define CS35L35_SP_SCLKS_48FS		0x0B
 #define CS35L35_SP_SCLKS_64FS		0x0F
 #define CS35L35_SP_RATE_MASK		0xC0
+#define CS35L35_SP_RATE_SHIFT		6
 
 #define CS35L35_PDN_BST_MASK		0x06
 #define CS35L35_PDN_BST_FETON_SHIFT	1

sound/soc/codecs/cs42l42.c

@@ -36,6 +36,7 @@
 #include <dt-bindings/sound/cs42l42.h>
 
 #include "cs42l42.h"
+#include "cirrus_legacy.h"
 
 static const struct reg_default cs42l42_reg_defaults[] = {
 	{ CS42L42_FRZ_CTL,			0x00 },
@@ -1814,8 +1815,7 @@ static int cs42l42_i2c_probe(struct i2c_client *i2c_client,
 				       const struct i2c_device_id *id)
 {
 	struct cs42l42_private *cs42l42;
-	int ret, i;
-	unsigned int devid = 0;
+	int ret, i, devid;
 	unsigned int reg;
 
 	cs42l42 = devm_kzalloc(&i2c_client->dev, sizeof(struct cs42l42_private),
@@ -1878,14 +1878,12 @@ static int cs42l42_i2c_probe(struct i2c_client *i2c_client,
 			"Failed to request IRQ: %d\n", ret);
 
 	/* initialize codec */
-	ret = regmap_read(cs42l42->regmap, CS42L42_DEVID_AB, &reg);
-	devid = (reg & 0xFF) << 12;
-
-	ret = regmap_read(cs42l42->regmap, CS42L42_DEVID_CD, &reg);
-	devid |= (reg & 0xFF) << 4;
-
-	ret = regmap_read(cs42l42->regmap, CS42L42_DEVID_E, &reg);
-	devid |= (reg & 0xF0) >> 4;
+	devid = cirrus_read_device_id(cs42l42->regmap, CS42L42_DEVID_AB);
+	if (devid < 0) {
+		ret = devid;
+		dev_err(&i2c_client->dev, "Failed to read device ID: %d\n", ret);
+		goto err_disable;
+	}
 
 	if (devid != CS42L42_CHIP_ID) {
 		ret = -ENODEV;

sound/soc/codecs/cs42l73.c

@@ -27,6 +27,7 @@
 #include <sound/tlv.h>
 #include <sound/cs42l73.h>
 #include "cs42l73.h"
+#include "cirrus_legacy.h"
 
 struct sp_config {
 	u8 spc, mmcc, spfs;
@@ -1275,8 +1276,7 @@ static int cs42l73_i2c_probe(struct i2c_client *i2c_client,
 {
 	struct cs42l73_private *cs42l73;
 	struct cs42l73_platform_data *pdata = dev_get_platdata(&i2c_client->dev);
-	int ret;
-	unsigned int devid = 0;
+	int ret, devid;
 	unsigned int reg;
 	u32 val32;
 
@@ -1326,27 +1326,25 @@ static int cs42l73_i2c_probe(struct i2c_client *i2c_client,
 	}
 
 	/* initialize codec */
-	ret = regmap_read(cs42l73->regmap, CS42L73_DEVID_AB, &reg);
-	devid = (reg & 0xFF) << 12;
-
-	ret = regmap_read(cs42l73->regmap, CS42L73_DEVID_CD, &reg);
-	devid |= (reg & 0xFF) << 4;
-
-	ret = regmap_read(cs42l73->regmap, CS42L73_DEVID_E, &reg);
-	devid |= (reg & 0xF0) >> 4;
+	devid = cirrus_read_device_id(cs42l73->regmap, CS42L73_DEVID_AB);
+	if (devid < 0) {
+		ret = devid;
+		dev_err(&i2c_client->dev, "Failed to read device ID: %d\n", ret);
+		goto err_reset;
+	}
 
 	if (devid != CS42L73_DEVID) {
 		ret = -ENODEV;
 		dev_err(&i2c_client->dev,
 			"CS42L73 Device ID (%X). Expected %X\n",
 			devid, CS42L73_DEVID);
-		return ret;
+		goto err_reset;
 	}
 
 	ret = regmap_read(cs42l73->regmap, CS42L73_REVID, &reg);
 	if (ret < 0) {
 		dev_err(&i2c_client->dev, "Get Revision ID failed\n");
-		return ret;
+		goto err_reset;
 	}
 
 	dev_info(&i2c_client->dev,
@@ -1356,8 +1354,14 @@ static int cs42l73_i2c_probe(struct i2c_client *i2c_client,
 			&soc_component_dev_cs42l73, cs42l73_dai,
 			ARRAY_SIZE(cs42l73_dai));
 	if (ret < 0)
-		return ret;
+		goto err_reset;
+
 	return 0;
+
+err_reset:
+	gpio_set_value_cansleep(cs42l73->pdata.reset_gpio, 0);
+
+	return ret;
 }
 
 static const struct of_device_id cs42l73_of_match[] = {

sound/soc/codecs/cs43130.c

@@ -36,6 +36,7 @@
 #include <sound/jack.h>
 
 #include "cs43130.h"
+#include "cirrus_legacy.h"
 
 static const struct reg_default cs43130_reg_defaults[] = {
 	{CS43130_SYS_CLK_CTL_1, 0x06},
@@ -2424,9 +2425,8 @@ static int cs43130_i2c_probe(struct i2c_client *client,
 {
 	struct cs43130_private *cs43130;
 	int ret;
-	unsigned int devid = 0;
 	unsigned int reg;
-	int i;
+	int i, devid;
 
 	cs43130 = devm_kzalloc(&client->dev, sizeof(*cs43130), GFP_KERNEL);
 	if (!cs43130)
@@ -2464,20 +2464,21 @@ static int cs43130_i2c_probe(struct i2c_client *client,
 
 	cs43130->reset_gpio = devm_gpiod_get_optional(&client->dev,
 						      "reset", GPIOD_OUT_LOW);
-	if (IS_ERR(cs43130->reset_gpio))
-		return PTR_ERR(cs43130->reset_gpio);
+	if (IS_ERR(cs43130->reset_gpio)) {
+		ret = PTR_ERR(cs43130->reset_gpio);
+		goto err_supplies;
+	}
 
 	gpiod_set_value_cansleep(cs43130->reset_gpio, 1);
 
 	usleep_range(2000, 2050);
 
-	ret = regmap_read(cs43130->regmap, CS43130_DEVID_AB, &reg);
-
-	devid = (reg & 0xFF) << 12;
-	ret = regmap_read(cs43130->regmap, CS43130_DEVID_CD, &reg);
-	devid |= (reg & 0xFF) << 4;
-	ret = regmap_read(cs43130->regmap, CS43130_DEVID_E, &reg);
-	devid |= (reg & 0xF0) >> 4;
+	devid = cirrus_read_device_id(cs43130->regmap, CS43130_DEVID_AB);
+	if (devid < 0) {
+		ret = devid;
+		dev_err(&client->dev, "Failed to read device ID: %d\n", ret);
+		goto err;
+	}
 
 	switch (devid) {
 	case CS43130_CHIP_ID:
@@ -2517,7 +2518,7 @@ static int cs43130_i2c_probe(struct i2c_client *client,
 					"cs43130", cs43130);
 	if (ret != 0) {
 		dev_err(&client->dev, "Failed to request IRQ: %d\n", ret);
-		return ret;
+		goto err;
 	}
 
 	cs43130->mclk_int_src = CS43130_MCLK_SRC_RCO;
@@ -2576,7 +2577,13 @@ static int cs43130_i2c_probe(struct i2c_client *client,
 			   CS43130_XSP_3ST_MASK, 0);
 
 	return 0;
+
 err:
+	gpiod_set_value_cansleep(cs43130->reset_gpio, 0);
+err_supplies:
+	regulator_bulk_disable(ARRAY_SIZE(cs43130->supplies),
+			       cs43130->supplies);
+
 	return ret;
 }
 

sound/soc/codecs/cs53l30.c

@@ -20,6 +20,7 @@
 #include <sound/tlv.h>
 
 #include "cs53l30.h"
+#include "cirrus_legacy.h"
 
 #define CS53L30_NUM_SUPPLIES 2
 static const char *const cs53l30_supply_names[CS53L30_NUM_SUPPLIES] = {
@@ -920,9 +921,8 @@ static int cs53l30_i2c_probe(struct i2c_client *client,
 	const struct device_node *np = client->dev.of_node;
 	struct device *dev = &client->dev;
 	struct cs53l30_private *cs53l30;
-	unsigned int devid = 0;
 	unsigned int reg;
-	int ret = 0, i;
+	int ret = 0, i, devid;
 	u8 val;
 
 	cs53l30 = devm_kzalloc(dev, sizeof(*cs53l30), GFP_KERNEL);
@@ -951,7 +951,7 @@ static int cs53l30_i2c_probe(struct i2c_client *client,
 						      GPIOD_OUT_LOW);
 	if (IS_ERR(cs53l30->reset_gpio)) {
 		ret = PTR_ERR(cs53l30->reset_gpio);
-		goto error;
+		goto error_supplies;
 	}
 
 	gpiod_set_value_cansleep(cs53l30->reset_gpio, 1);
@@ -968,14 +968,12 @@ static int cs53l30_i2c_probe(struct i2c_client *client,
 	}
 
 	/* Initialize codec */
-	ret = regmap_read(cs53l30->regmap, CS53L30_DEVID_AB, &reg);
-	devid = reg << 12;
-
-	ret = regmap_read(cs53l30->regmap, CS53L30_DEVID_CD, &reg);
-	devid |= reg << 4;
-
-	ret = regmap_read(cs53l30->regmap, CS53L30_DEVID_E, &reg);
-	devid |= (reg & 0xF0) >> 4;
+	devid = cirrus_read_device_id(cs53l30->regmap, CS53L30_DEVID_AB);
+	if (devid < 0) {
+		ret = devid;
+		dev_err(dev, "Failed to read device ID: %d\n", ret);
+		goto error;
+	}
 
 	if (devid != CS53L30_DEVID) {
 		ret = -ENODEV;
@@ -1037,6 +1035,8 @@ static int cs53l30_i2c_probe(struct i2c_client *client,
 	return 0;
 
 error:
+	gpiod_set_value_cansleep(cs53l30->reset_gpio, 0);
+error_supplies:
 	regulator_bulk_disable(ARRAY_SIZE(cs53l30->supplies),
 			       cs53l30->supplies);
 	return ret;