linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/2] ASoC: soc-component: add snd_soc_component_read/write_field()
@ 2021-01-26 17:17 Srinivas Kandagatla
  2021-01-26 17:17 ` [PATCH v2 2/2] ASoC: codecs: lpass-wsa-macro: make use of snd_soc_component_read_field() Srinivas Kandagatla
  2021-01-27 17:13 ` [PATCH v2 1/2] ASoC: soc-component: add snd_soc_component_read/write_field() Mark Brown
  0 siblings, 2 replies; 3+ messages in thread
From: Srinivas Kandagatla @ 2021-01-26 17:17 UTC (permalink / raw)
  To: broonie
  Cc: lgirdwood, alsa-devel, perex, tiwai, linux-kernel, vkoul,
	Srinivas Kandagatla

It's often the case that we would write or read a particular field
in register. With the current soc_component apis, reading a particular
field in register would involve first read the register and then
perform shift operations.

Ex:
to read from a field mask of 0xf0

	val = snd_soc_component_read(component, reg);
	field = ((val & 0xf0) >> 0x4);

This is sometimes prone to errors and code become less readable!

With this new api we could just do
	field = snd_soc_component_read_field(component, reg, 0xf0);

this makes it bit simple, easy to write and less error prone!

This also applies to writing!

There are various places in kernel which provides such field interfaces
however soc_component seems to be missing this.

This patch is inspired by FIELD_GET/FIELD_PREP macros in include/linux/bitfield.h

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---

Changes since v1:
	- removed open coding and call read and update_bits
	- made soc_component_field_shift an inline function

 include/sound/soc-component.h |  6 ++++
 sound/soc/soc-component.c     | 53 +++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+)

diff --git a/include/sound/soc-component.h b/include/sound/soc-component.h
index 0bce41fefd30..5b47768222b7 100644
--- a/include/sound/soc-component.h
+++ b/include/sound/soc-component.h
@@ -353,6 +353,12 @@ int snd_soc_component_test_bits(struct snd_soc_component *component,
 				unsigned int reg, unsigned int mask,
 				unsigned int value);
 
+unsigned int snd_soc_component_read_field(struct snd_soc_component *component,
+					  unsigned int reg, unsigned int mask);
+int snd_soc_component_write_field(struct snd_soc_component *component,
+				  unsigned int reg, unsigned int mask,
+				  unsigned int val);
+
 /* component wide operations */
 int snd_soc_component_set_sysclk(struct snd_soc_component *component,
 				 int clk_id, int source,
diff --git a/sound/soc/soc-component.c b/sound/soc/soc-component.c
index 760523382f3c..361a79d655e3 100644
--- a/sound/soc/soc-component.c
+++ b/sound/soc/soc-component.c
@@ -34,6 +34,18 @@ static inline int _soc_component_ret(struct snd_soc_component *component,
 	return ret;
 }
 
+static inline int soc_component_field_shift(struct snd_soc_component *component,
+					    unsigned int mask)
+{
+	if (!mask) {
+		dev_err(component->dev,	"ASoC: error field mask is zero for %s\n",
+			component->name);
+		return 0;
+	}
+
+	return (__builtin_ffs(mask) - 1);
+}
+
 /*
  * We might want to check substream by using list.
  * In such case, we can update these macros.
@@ -839,6 +851,47 @@ int snd_soc_component_update_bits_async(struct snd_soc_component *component,
 }
 EXPORT_SYMBOL_GPL(snd_soc_component_update_bits_async);
 
+/**
+ * snd_soc_component_read_field() - Read register field value
+ * @component: Component to read from
+ * @reg: Register to read
+ * @mask: mask of the register field
+ *
+ * Return: read value of register field.
+ */
+unsigned int snd_soc_component_read_field(struct snd_soc_component *component,
+					  unsigned int reg, unsigned int mask)
+{
+	unsigned int val;
+
+	val = snd_soc_component_read(component, reg);
+
+	val = (val & mask) >> soc_component_field_shift(component, mask);
+
+	return val;
+}
+EXPORT_SYMBOL_GPL(snd_soc_component_read_field);
+
+/**
+ * snd_soc_component_write_field() - write to register field
+ * @component: Component to write to
+ * @reg: Register to write
+ * @mask: mask of the register field to update
+ * @val: value of the field to write
+ *
+ * Return: 1 for change, otherwise 0.
+ */
+int snd_soc_component_write_field(struct snd_soc_component *component,
+				  unsigned int reg, unsigned int mask,
+				  unsigned int val)
+{
+
+	val = (val << soc_component_field_shift(component, mask)) & mask;
+
+	return snd_soc_component_update_bits(component, reg, mask, val);
+}
+EXPORT_SYMBOL_GPL(snd_soc_component_write_field);
+
 /**
  * snd_soc_component_async_complete() - Ensure asynchronous I/O has completed
  * @component: Component for which to wait
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH v2 2/2] ASoC: codecs: lpass-wsa-macro: make use of snd_soc_component_read_field()
  2021-01-26 17:17 [PATCH v2 1/2] ASoC: soc-component: add snd_soc_component_read/write_field() Srinivas Kandagatla
@ 2021-01-26 17:17 ` Srinivas Kandagatla
  2021-01-27 17:13 ` [PATCH v2 1/2] ASoC: soc-component: add snd_soc_component_read/write_field() Mark Brown
  1 sibling, 0 replies; 3+ messages in thread
From: Srinivas Kandagatla @ 2021-01-26 17:17 UTC (permalink / raw)
  To: broonie
  Cc: lgirdwood, alsa-devel, perex, tiwai, linux-kernel, vkoul,
	Srinivas Kandagatla

Make use of snd_soc_component_read_field() to make the code more readable!

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 sound/soc/codecs/lpass-wsa-macro.c | 43 ++++++++++++++----------------
 1 file changed, 20 insertions(+), 23 deletions(-)

diff --git a/sound/soc/codecs/lpass-wsa-macro.c b/sound/soc/codecs/lpass-wsa-macro.c
index 25f1df214ca5..5ebcd935ba89 100644
--- a/sound/soc/codecs/lpass-wsa-macro.c
+++ b/sound/soc/codecs/lpass-wsa-macro.c
@@ -40,9 +40,11 @@
 #define CDC_WSA_TOP_I2S_CLK			(0x00A4)
 #define CDC_WSA_TOP_I2S_RESET			(0x00A8)
 #define CDC_WSA_RX_INP_MUX_RX_INT0_CFG0		(0x0100)
-#define CDC_WSA_RX_INTX_1_MIX_INP2_SEL_MASK	GENMASK(5, 3)
-#define CDC_WSA_RX_INTX_2_SEL_MASK		GENMASK(2, 0)
+#define CDC_WSA_RX_INTX_1_MIX_INP0_SEL_MASK	GENMASK(2, 0)
+#define CDC_WSA_RX_INTX_1_MIX_INP1_SEL_MASK	GENMASK(5, 3)
 #define CDC_WSA_RX_INP_MUX_RX_INT0_CFG1		(0x0104)
+#define CDC_WSA_RX_INTX_2_SEL_MASK		GENMASK(2, 0)
+#define CDC_WSA_RX_INTX_1_MIX_INP2_SEL_MASK	GENMASK(5, 3)
 #define CDC_WSA_RX_INP_MUX_RX_INT1_CFG0		(0x0108)
 #define CDC_WSA_RX_INP_MUX_RX_INT1_CFG1		(0x010C)
 #define CDC_WSA_RX_INP_MUX_RX_MIX_CFG0		(0x0110)
@@ -229,8 +231,6 @@
 #define NUM_INTERPOLATORS 2
 #define WSA_NUM_CLKS_MAX	5
 #define WSA_MACRO_MCLK_FREQ 19200000
-#define WSA_MACRO_MUX_INP_SHFT 0x3
-#define WSA_MACRO_MUX_INP_MASK1 0x07
 #define WSA_MACRO_MUX_INP_MASK2 0x38
 #define WSA_MACRO_MUX_CFG_OFFSET 0x8
 #define WSA_MACRO_MUX_CFG1_OFFSET 0x4
@@ -843,7 +843,6 @@ static int wsa_macro_set_prim_interpolator_rate(struct snd_soc_dai *dai,
 	u32 j, port;
 	u16 int_mux_cfg0, int_mux_cfg1;
 	u16 int_fs_reg;
-	u8 int_mux_cfg0_val, int_mux_cfg1_val;
 	u8 inp0_sel, inp1_sel, inp2_sel;
 	struct snd_soc_component *component = dai->component;
 	struct wsa_macro *wsa = snd_soc_component_get_drvdata(component);
@@ -865,15 +864,13 @@ static int wsa_macro_set_prim_interpolator_rate(struct snd_soc_dai *dai,
 		 */
 		for (j = 0; j < NUM_INTERPOLATORS; j++) {
 			int_mux_cfg1 = int_mux_cfg0 + WSA_MACRO_MUX_CFG1_OFFSET;
-			int_mux_cfg0_val = snd_soc_component_read(component,
-								  int_mux_cfg0);
-			int_mux_cfg1_val = snd_soc_component_read(component,
-								  int_mux_cfg1);
-			inp0_sel = int_mux_cfg0_val & WSA_MACRO_MUX_INP_MASK1;
-			inp1_sel = (int_mux_cfg0_val >> WSA_MACRO_MUX_INP_SHFT) &
-						WSA_MACRO_MUX_INP_MASK1;
-			inp2_sel = (int_mux_cfg1_val >> WSA_MACRO_MUX_INP_SHFT) &
-						WSA_MACRO_MUX_INP_MASK1;
+			inp0_sel = snd_soc_component_read_field(component, int_mux_cfg0, 
+								CDC_WSA_RX_INTX_1_MIX_INP0_SEL_MASK);
+			inp1_sel = snd_soc_component_read_field(component, int_mux_cfg0, 
+								CDC_WSA_RX_INTX_1_MIX_INP1_SEL_MASK);
+			inp2_sel = snd_soc_component_read_field(component, int_mux_cfg1,
+								CDC_WSA_RX_INTX_1_MIX_INP2_SEL_MASK);
+
 			if ((inp0_sel == int_1_mix1_inp + INTn_1_INP_SEL_RX0) ||
 			    (inp1_sel == int_1_mix1_inp + INTn_1_INP_SEL_RX0) ||
 			    (inp2_sel == int_1_mix1_inp + INTn_1_INP_SEL_RX0)) {
@@ -912,9 +909,9 @@ static int wsa_macro_set_mix_interpolator_rate(struct snd_soc_dai *dai,
 
 		int_mux_cfg1 = CDC_WSA_RX_INP_MUX_RX_INT0_CFG1;
 		for (j = 0; j < NUM_INTERPOLATORS; j++) {
-			int_mux_cfg1_val = snd_soc_component_read(component,
-							int_mux_cfg1) &
-							WSA_MACRO_MUX_INP_MASK1;
+			int_mux_cfg1_val = snd_soc_component_read_field(component, int_mux_cfg1,
+									CDC_WSA_RX_INTX_2_SEL_MASK);
+
 			if (int_mux_cfg1_val == int_2_inp + INTn_2_INP_SEL_RX0) {
 				int_fs_reg = CDC_WSA_RX0_RX_PATH_MIX_CTL +
 					WSA_MACRO_RX_PATH_OFFSET * j;
@@ -1410,25 +1407,25 @@ static bool wsa_macro_adie_lb(struct snd_soc_component *component,
 			      int interp_idx)
 {
 	u16 int_mux_cfg0,  int_mux_cfg1;
-	u8 int_mux_cfg0_val, int_mux_cfg1_val;
 	u8 int_n_inp0, int_n_inp1, int_n_inp2;
 
 	int_mux_cfg0 = CDC_WSA_RX_INP_MUX_RX_INT0_CFG0 + interp_idx * 8;
 	int_mux_cfg1 = int_mux_cfg0 + 4;
-	int_mux_cfg0_val = snd_soc_component_read(component, int_mux_cfg0);
-	int_mux_cfg1_val = snd_soc_component_read(component, int_mux_cfg1);
 
-	int_n_inp0 = int_mux_cfg0_val & 0x0F;
+	int_n_inp0 = snd_soc_component_read_field(component, int_mux_cfg0,
+						  CDC_WSA_RX_INTX_1_MIX_INP0_SEL_MASK);
 	if (int_n_inp0 == INTn_1_INP_SEL_DEC0 ||
 		int_n_inp0 == INTn_1_INP_SEL_DEC1)
 		return true;
 
-	int_n_inp1 = int_mux_cfg0_val >> 4;
+	int_n_inp1 = snd_soc_component_read_field(component, int_mux_cfg0,
+						  CDC_WSA_RX_INTX_1_MIX_INP1_SEL_MASK);
 	if (int_n_inp1 == INTn_1_INP_SEL_DEC0 ||
 		int_n_inp1 == INTn_1_INP_SEL_DEC1)
 		return true;
 
-	int_n_inp2 = int_mux_cfg1_val >> 4;
+	int_n_inp2 = snd_soc_component_read_field(component, int_mux_cfg1,
+						  CDC_WSA_RX_INTX_1_MIX_INP2_SEL_MASK);
 	if (int_n_inp2 == INTn_1_INP_SEL_DEC0 ||
 		int_n_inp2 == INTn_1_INP_SEL_DEC1)
 		return true;
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2 1/2] ASoC: soc-component: add snd_soc_component_read/write_field()
  2021-01-26 17:17 [PATCH v2 1/2] ASoC: soc-component: add snd_soc_component_read/write_field() Srinivas Kandagatla
  2021-01-26 17:17 ` [PATCH v2 2/2] ASoC: codecs: lpass-wsa-macro: make use of snd_soc_component_read_field() Srinivas Kandagatla
@ 2021-01-27 17:13 ` Mark Brown
  1 sibling, 0 replies; 3+ messages in thread
From: Mark Brown @ 2021-01-27 17:13 UTC (permalink / raw)
  To: Srinivas Kandagatla; +Cc: tiwai, alsa-devel, lgirdwood, vkoul, linux-kernel

On Tue, 26 Jan 2021 17:17:48 +0000, Srinivas Kandagatla wrote:
> It's often the case that we would write or read a particular field
> in register. With the current soc_component apis, reading a particular
> field in register would involve first read the register and then
> perform shift operations.
> 
> Ex:
> to read from a field mask of 0xf0
> 
> [...]

Applied to

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

Thanks!

[1/2] ASoC: soc-component: add snd_soc_component_read/write_field()
      commit: 1da0b9899abdbc7103d3ec6b1a888efda41dbb59
[2/2] ASoC: codecs: lpass-wsa-macro: make use of snd_soc_component_read_field()
      commit: 7db4c4cd767e5c2418e7892c6b2f06982d34369a

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-01-27 17:17 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-26 17:17 [PATCH v2 1/2] ASoC: soc-component: add snd_soc_component_read/write_field() Srinivas Kandagatla
2021-01-26 17:17 ` [PATCH v2 2/2] ASoC: codecs: lpass-wsa-macro: make use of snd_soc_component_read_field() Srinivas Kandagatla
2021-01-27 17:13 ` [PATCH v2 1/2] ASoC: soc-component: add snd_soc_component_read/write_field() Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).