diff mbox series

[v5,02/10] ASoC: cs35l41: Move cs35l41_otp_unpack to shared code

Message ID 20211216114332.153409-3-tanureal@opensource.cirrus.com (mailing list archive)
State Superseded
Headers show
Series Add support for CS35L41 in HDA systems | expand

Commit Message

Lucas Tanure Dec. 16, 2021, 11:43 a.m. UTC 
ASoC and HDA will do the same cs35l41_otp_unpack, so move it
to shared code

Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com>
---
 include/sound/cs35l41.h        |   4 +-
 sound/soc/codecs/cs35l41-lib.c | 121 ++++++++++++++++++++++++++++++-
 sound/soc/codecs/cs35l41.c     | 125 +--------------------------------
 3 files changed, 122 insertions(+), 128 deletions(-)

Comments

Cezary Rojewski Dec. 17, 2021, 12:59 p.m. UTC | #1 
On 2021-12-16 12:43 PM, Lucas Tanure wrote:
> ASoC and HDA will do the same cs35l41_otp_unpack, so move it
> to shared code

...

> +static const struct cs35l41_otp_map_element_t *cs35l41_find_otp_map(u32 otp_id)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(cs35l41_otp_map_map); i++) {
> +		if (cs35l41_otp_map_map[i].id == otp_id)
> +			return &cs35l41_otp_map_map[i];
> +	}

The parenthesis could be dropped.

> +	return NULL;
> +}
> +int cs35l41_otp_unpack(struct device *dev, struct regmap *regmap)
> +{
> +	const struct cs35l41_otp_map_element_t *otp_map_match;
> +	const struct cs35l41_otp_packed_element_t *otp_map;
> +	int bit_offset, word_offset, ret, i;
> +	unsigned int bit_sum = 8;
> +	u32 otp_val, otp_id_reg;
> +	u32 *otp_mem;
> +
> +	otp_mem = kmalloc_array(CS35L41_OTP_SIZE_WORDS, sizeof(*otp_mem), GFP_KERNEL);
> +	if (!otp_mem)
> +		return -ENOMEM;
> +
> +	ret = regmap_read(regmap, CS35L41_OTPID, &otp_id_reg);
> +	if (ret) {
> +		dev_err(dev, "Read OTP ID failed: %d\n", ret);
> +		goto err_otp_unpack;
> +	}
> +
> +	otp_map_match = cs35l41_find_otp_map(otp_id_reg);
> +
> +	if (!otp_map_match) {
> +		dev_err(dev, "OTP Map matching ID %d not found\n", otp_id_reg);
> +		ret = -EINVAL;
> +		goto err_otp_unpack;
> +	}

This block could be understood as: assign and check. Surrounding blocks 
that carry similar value do not have a newline between assignment and 
check. My suggestion is to drop that newline here so the block looks 
more cohesive when compared with the rest of the function.

> +
> +	ret = regmap_bulk_read(regmap, CS35L41_OTP_MEM0, otp_mem, CS35L41_OTP_SIZE_WORDS);
> +	if (ret) {
> +		dev_err(dev, "Read OTP Mem failed: %d\n", ret);
> +		goto err_otp_unpack;
> +	}
> +
> +	otp_map = otp_map_match->map;
> +
> +	bit_offset = otp_map_match->bit_offset;
> +	word_offset = otp_map_match->word_offset;
> +
> +	ret = regmap_write(regmap, CS35L41_TEST_KEY_CTL, 0x00000055);
> +	if (ret) {
> +		dev_err(dev, "Write Unlock key failed 1/2: %d\n", ret);
> +		goto err_otp_unpack;
> +	}
> +	ret = regmap_write(regmap, CS35L41_TEST_KEY_CTL, 0x000000AA);
> +	if (ret) {
> +		dev_err(dev, "Write Unlock key failed 2/2: %d\n", ret);
> +		goto err_otp_unpack;
> +	}
> +
> +	for (i = 0; i < otp_map_match->num_elements; i++) {
> +		dev_dbg(dev, "bitoffset= %d, word_offset=%d, bit_sum mod 32=%d\n",
> +			bit_offset, word_offset, bit_sum % 32);
> +		if (bit_offset + otp_map[i].size - 1 >= 32) {
> +			otp_val = (otp_mem[word_offset] &
> +					GENMASK(31, bit_offset)) >> bit_offset;
> +			otp_val |= (otp_mem[++word_offset] &
> +					GENMASK(bit_offset + otp_map[i].size - 33, 0)) <<
> +					(32 - bit_offset);
> +			bit_offset += otp_map[i].size - 32;
> +		} else {
> +			otp_val = (otp_mem[word_offset] &
> +				   GENMASK(bit_offset + otp_map[i].size - 1, bit_offset)
> +				  ) >> bit_offset;

The ')' looks off (the '>>' too), at least it does not match the 
convention seen in if-statement above. Choosing single convention could 
improve the readability.

> +			bit_offset += otp_map[i].size;
> +		}
> +		bit_sum += otp_map[i].size;
> +
> +		if (bit_offset == 32) {
> +			bit_offset = 0;
> +			word_offset++;
> +		}
> +
> +		if (otp_map[i].reg != 0) {
> +			ret = regmap_update_bits(regmap, otp_map[i].reg,
> +						 GENMASK(otp_map[i].shift + otp_map[i].size - 1,
> +							 otp_map[i].shift),
> +						 otp_val << otp_map[i].shift);
> +			if (ret < 0) {
> +				dev_err(dev, "Write OTP val failed: %d\n", ret);
> +				goto err_otp_unpack;
> +			}
> +		}
> +	}
> +
> +	ret = regmap_write(regmap, CS35L41_TEST_KEY_CTL, 0x000000CC);
> +	if (ret) {
> +		dev_err(dev, "Write Lock key failed 1/2: %d\n", ret);
> +		goto err_otp_unpack;
> +	}
> +	ret = regmap_write(regmap, CS35L41_TEST_KEY_CTL, 0x00000033);
> +	if (ret) {
> +		dev_err(dev, "Write Lock key failed 2/2: %d\n", ret);
> +		goto err_otp_unpack;
> +	}
> +	ret = 0;

Hmm.. maybe I'm missing something, but isn't the 'ret' already '0' by 
the time we get here?

> +err_otp_unpack:
> +	kfree(otp_mem);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(cs35l41_otp_unpack);
> +
diff mbox series

Patch

diff --git a/include/sound/cs35l41.h b/include/sound/cs35l41.h
index aac3ffb9bc89..6cf3ef02b26a 100644
--- a/include/sound/cs35l41.h
+++ b/include/sound/cs35l41.h
@@ -534,7 +534,6 @@ 
 #define CS35L41_MAX_CACHE_REG		36
 #define CS35L41_OTP_SIZE_WORDS		32
 #define CS35L41_NUM_OTP_ELEM		100
-#define CS35L41_NUM_OTP_MAPS		5
 
 #define CS35L41_VALID_PDATA		0x80000000
 #define CS35L41_NUM_SUPPLIES            2
@@ -760,8 +759,9 @@  struct cs35l41_otp_map_element_t {
 	u32 word_offset;
 };
 
-extern const struct cs35l41_otp_map_element_t cs35l41_otp_map_map[CS35L41_NUM_OTP_MAPS];
 extern struct regmap_config cs35l41_regmap_i2c;
 extern struct regmap_config cs35l41_regmap_spi;
 
+int cs35l41_otp_unpack(struct device *dev, struct regmap *regmap);
+
 #endif /* __CS35L41_H */
diff --git a/sound/soc/codecs/cs35l41-lib.c b/sound/soc/codecs/cs35l41-lib.c
index f19531ebf729..dc5f502447a2 100644
--- a/sound/soc/codecs/cs35l41-lib.c
+++ b/sound/soc/codecs/cs35l41-lib.c
@@ -7,8 +7,11 @@ 
 // Author: David Rhodes <david.rhodes@cirrus.com>
 // Author: Lucas Tanure <lucas.tanure@cirrus.com>
 
+#include <linux/dev_printk.h>
 #include <linux/module.h>
 #include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
 
 #include <sound/cs35l41.h>
 
@@ -655,7 +658,7 @@  static const struct cs35l41_otp_packed_element_t otp_map_2[CS35L41_NUM_OTP_ELEM]
 	{ 0x00017044,	0,	24 }, /*LOT_NUMBER*/
 };
 
-const struct cs35l41_otp_map_element_t cs35l41_otp_map_map[CS35L41_NUM_OTP_MAPS] = {
+static const struct cs35l41_otp_map_element_t cs35l41_otp_map_map[] = {
 	{
 		.id = 0x01,
 		.map = otp_map_1,
@@ -692,7 +695,6 @@  const struct cs35l41_otp_map_element_t cs35l41_otp_map_map[CS35L41_NUM_OTP_MAPS]
 		.word_offset = 2,
 	},
 };
-EXPORT_SYMBOL_GPL(cs35l41_otp_map_map);
 
 struct regmap_config cs35l41_regmap_i2c = {
 	.reg_bits = 32,
@@ -727,6 +729,121 @@  struct regmap_config cs35l41_regmap_spi = {
 };
 EXPORT_SYMBOL_GPL(cs35l41_regmap_spi);
 
+static const struct cs35l41_otp_map_element_t *cs35l41_find_otp_map(u32 otp_id)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(cs35l41_otp_map_map); i++) {
+		if (cs35l41_otp_map_map[i].id == otp_id)
+			return &cs35l41_otp_map_map[i];
+	}
+
+	return NULL;
+}
+
+int cs35l41_otp_unpack(struct device *dev, struct regmap *regmap)
+{
+	const struct cs35l41_otp_map_element_t *otp_map_match;
+	const struct cs35l41_otp_packed_element_t *otp_map;
+	int bit_offset, word_offset, ret, i;
+	unsigned int bit_sum = 8;
+	u32 otp_val, otp_id_reg;
+	u32 *otp_mem;
+
+	otp_mem = kmalloc_array(CS35L41_OTP_SIZE_WORDS, sizeof(*otp_mem), GFP_KERNEL);
+	if (!otp_mem)
+		return -ENOMEM;
+
+	ret = regmap_read(regmap, CS35L41_OTPID, &otp_id_reg);
+	if (ret) {
+		dev_err(dev, "Read OTP ID failed: %d\n", ret);
+		goto err_otp_unpack;
+	}
+
+	otp_map_match = cs35l41_find_otp_map(otp_id_reg);
+
+	if (!otp_map_match) {
+		dev_err(dev, "OTP Map matching ID %d not found\n", otp_id_reg);
+		ret = -EINVAL;
+		goto err_otp_unpack;
+	}
+
+	ret = regmap_bulk_read(regmap, CS35L41_OTP_MEM0, otp_mem, CS35L41_OTP_SIZE_WORDS);
+	if (ret) {
+		dev_err(dev, "Read OTP Mem failed: %d\n", ret);
+		goto err_otp_unpack;
+	}
+
+	otp_map = otp_map_match->map;
+
+	bit_offset = otp_map_match->bit_offset;
+	word_offset = otp_map_match->word_offset;
+
+	ret = regmap_write(regmap, CS35L41_TEST_KEY_CTL, 0x00000055);
+	if (ret) {
+		dev_err(dev, "Write Unlock key failed 1/2: %d\n", ret);
+		goto err_otp_unpack;
+	}
+	ret = regmap_write(regmap, CS35L41_TEST_KEY_CTL, 0x000000AA);
+	if (ret) {
+		dev_err(dev, "Write Unlock key failed 2/2: %d\n", ret);
+		goto err_otp_unpack;
+	}
+
+	for (i = 0; i < otp_map_match->num_elements; i++) {
+		dev_dbg(dev, "bitoffset= %d, word_offset=%d, bit_sum mod 32=%d\n",
+			bit_offset, word_offset, bit_sum % 32);
+		if (bit_offset + otp_map[i].size - 1 >= 32) {
+			otp_val = (otp_mem[word_offset] &
+					GENMASK(31, bit_offset)) >> bit_offset;
+			otp_val |= (otp_mem[++word_offset] &
+					GENMASK(bit_offset + otp_map[i].size - 33, 0)) <<
+					(32 - bit_offset);
+			bit_offset += otp_map[i].size - 32;
+		} else {
+			otp_val = (otp_mem[word_offset] &
+				   GENMASK(bit_offset + otp_map[i].size - 1, bit_offset)
+				  ) >> bit_offset;
+			bit_offset += otp_map[i].size;
+		}
+		bit_sum += otp_map[i].size;
+
+		if (bit_offset == 32) {
+			bit_offset = 0;
+			word_offset++;
+		}
+
+		if (otp_map[i].reg != 0) {
+			ret = regmap_update_bits(regmap, otp_map[i].reg,
+						 GENMASK(otp_map[i].shift + otp_map[i].size - 1,
+							 otp_map[i].shift),
+						 otp_val << otp_map[i].shift);
+			if (ret < 0) {
+				dev_err(dev, "Write OTP val failed: %d\n", ret);
+				goto err_otp_unpack;
+			}
+		}
+	}
+
+	ret = regmap_write(regmap, CS35L41_TEST_KEY_CTL, 0x000000CC);
+	if (ret) {
+		dev_err(dev, "Write Lock key failed 1/2: %d\n", ret);
+		goto err_otp_unpack;
+	}
+	ret = regmap_write(regmap, CS35L41_TEST_KEY_CTL, 0x00000033);
+	if (ret) {
+		dev_err(dev, "Write Lock key failed 2/2: %d\n", ret);
+		goto err_otp_unpack;
+	}
+	ret = 0;
+
+err_otp_unpack:
+	kfree(otp_mem);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(cs35l41_otp_unpack);
+
 MODULE_DESCRIPTION("CS35L41 library");
 MODULE_AUTHOR("David Rhodes, Cirrus Logic Inc, <david.rhodes@cirrus.com>");
 MODULE_AUTHOR("Lucas Tanure, Cirrus Logic Inc, <tanureal@opensource.cirrus.com>");
diff --git a/sound/soc/codecs/cs35l41.c b/sound/soc/codecs/cs35l41.c
index 60332eae1162..aa57c59b334d 100644
--- a/sound/soc/codecs/cs35l41.c
+++ b/sound/soc/codecs/cs35l41.c
@@ -14,7 +14,6 @@ 
 #include <linux/moduleparam.h>
 #include <linux/of_device.h>
 #include <linux/property.h>
-#include <linux/slab.h>
 #include <sound/initval.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
@@ -420,128 +419,6 @@  static const struct snd_kcontrol_new cs35l41_aud_controls[] = {
 	WM_ADSP_FW_CONTROL("DSP1", 0),
 };
 
-static const struct cs35l41_otp_map_element_t *cs35l41_find_otp_map(u32 otp_id)
-{
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(cs35l41_otp_map_map); i++) {
-		if (cs35l41_otp_map_map[i].id == otp_id)
-			return &cs35l41_otp_map_map[i];
-	}
-
-	return NULL;
-}
-
-static int cs35l41_otp_unpack(void *data)
-{
-	const struct cs35l41_otp_map_element_t *otp_map_match;
-	const struct cs35l41_otp_packed_element_t *otp_map;
-	struct cs35l41_private *cs35l41 = data;
-	int bit_offset, word_offset, ret, i;
-	unsigned int bit_sum = 8;
-	u32 otp_val, otp_id_reg;
-	u32 *otp_mem;
-
-	otp_mem = kmalloc_array(CS35L41_OTP_SIZE_WORDS, sizeof(*otp_mem), GFP_KERNEL);
-	if (!otp_mem)
-		return -ENOMEM;
-
-	ret = regmap_read(cs35l41->regmap, CS35L41_OTPID, &otp_id_reg);
-	if (ret < 0) {
-		dev_err(cs35l41->dev, "Read OTP ID failed: %d\n", ret);
-		goto err_otp_unpack;
-	}
-
-	otp_map_match = cs35l41_find_otp_map(otp_id_reg);
-
-	if (!otp_map_match) {
-		dev_err(cs35l41->dev, "OTP Map matching ID %d not found\n",
-			otp_id_reg);
-		ret = -EINVAL;
-		goto err_otp_unpack;
-	}
-
-	ret = regmap_bulk_read(cs35l41->regmap, CS35L41_OTP_MEM0, otp_mem,
-			       CS35L41_OTP_SIZE_WORDS);
-	if (ret < 0) {
-		dev_err(cs35l41->dev, "Read OTP Mem failed: %d\n", ret);
-		goto err_otp_unpack;
-	}
-
-	otp_map = otp_map_match->map;
-
-	bit_offset = otp_map_match->bit_offset;
-	word_offset = otp_map_match->word_offset;
-
-	ret = regmap_write(cs35l41->regmap, CS35L41_TEST_KEY_CTL, 0x00000055);
-	if (ret < 0) {
-		dev_err(cs35l41->dev, "Write Unlock key failed 1/2: %d\n", ret);
-		goto err_otp_unpack;
-	}
-	ret = regmap_write(cs35l41->regmap, CS35L41_TEST_KEY_CTL, 0x000000AA);
-	if (ret < 0) {
-		dev_err(cs35l41->dev, "Write Unlock key failed 2/2: %d\n", ret);
-		goto err_otp_unpack;
-	}
-
-	for (i = 0; i < otp_map_match->num_elements; i++) {
-		dev_dbg(cs35l41->dev,
-			"bitoffset= %d, word_offset=%d, bit_sum mod 32=%d\n",
-			bit_offset, word_offset, bit_sum % 32);
-		if (bit_offset + otp_map[i].size - 1 >= 32) {
-			otp_val = (otp_mem[word_offset] &
-					GENMASK(31, bit_offset)) >>
-					bit_offset;
-			otp_val |= (otp_mem[++word_offset] &
-					GENMASK(bit_offset +
-						otp_map[i].size - 33, 0)) <<
-					(32 - bit_offset);
-			bit_offset += otp_map[i].size - 32;
-		} else {
-			otp_val = (otp_mem[word_offset] &
-				GENMASK(bit_offset + otp_map[i].size - 1,
-					bit_offset)) >>	bit_offset;
-			bit_offset += otp_map[i].size;
-		}
-		bit_sum += otp_map[i].size;
-
-		if (bit_offset == 32) {
-			bit_offset = 0;
-			word_offset++;
-		}
-
-		if (otp_map[i].reg != 0) {
-			ret = regmap_update_bits(cs35l41->regmap,
-						 otp_map[i].reg,
-						 GENMASK(otp_map[i].shift +
-							 otp_map[i].size - 1,
-						 otp_map[i].shift),
-						 otp_val << otp_map[i].shift);
-			if (ret < 0) {
-				dev_err(cs35l41->dev, "Write OTP val failed: %d\n",
-					ret);
-				goto err_otp_unpack;
-			}
-		}
-	}
-
-	ret = regmap_write(cs35l41->regmap, CS35L41_TEST_KEY_CTL, 0x000000CC);
-	if (ret < 0) {
-		dev_err(cs35l41->dev, "Write Lock key failed 1/2: %d\n", ret);
-		goto err_otp_unpack;
-	}
-	ret = regmap_write(cs35l41->regmap, CS35L41_TEST_KEY_CTL, 0x00000033);
-	if (ret < 0) {
-		dev_err(cs35l41->dev, "Write Lock key failed 2/2: %d\n", ret);
-		goto err_otp_unpack;
-	}
-	ret = 0;
-
-err_otp_unpack:
-	kfree(otp_mem);
-	return ret;
-}
-
 static irqreturn_t cs35l41_irq(int irq, void *data)
 {
 	struct cs35l41_private *cs35l41 = data;
@@ -1667,7 +1544,7 @@  int cs35l41_probe(struct cs35l41_private *cs35l41,
 		goto err;
 	}
 
-	ret = cs35l41_otp_unpack(cs35l41);
+	ret = cs35l41_otp_unpack(cs35l41->dev, cs35l41->regmap);
 	if (ret < 0) {
 		dev_err(cs35l41->dev, "OTP Unpack failed: %d\n", ret);
 		goto err;