[PATCH v1 0/8] ASoC: loongson: Simplify code formatting

[PATCH v1 0/8] ASoC: loongson: Simplify code formatting

[Binbin Zhou]

Hi all:

This patchset attempts to improve code readability by simplifying code
formatting.
No functional changes.

Original link:
https://lore.kernel.org/all/c54fbc2582702689c005e1ba528ab2318b1adde6.1725518229.git.zhoubinbin@loongson.cn/

Thanks.

Binbin Zhou (8):
  ASoC: loongson: Use BIT() macro
  ASoC: loongson: Simplify probe() with local dev variable
  ASoC: loongson: Simplify with dev_err_probe()
  ASoC: loongson: Simplify if statment in loongson_card_hw_params()
  ASoC: loongson: Simplify of_node_put() reference
  ASoC: loongson: Replace if with ternary operator
  ASoC: loongson: Factor out loongson_card_acpi_find_device() function
  ASoC: loongson: Factor out loongson i2s enable clock functions

 sound/soc/loongson/loongson_card.c    | 142 +++++++++++++-------------
 sound/soc/loongson/loongson_dma.c     |  10 +-
 sound/soc/loongson/loongson_i2s.c     | 110 ++++++++++----------
 sound/soc/loongson/loongson_i2s.h     |  24 ++---
 sound/soc/loongson/loongson_i2s_pci.c |  51 ++++-----
 5 files changed, 166 insertions(+), 171 deletions(-)

-- 
2.43.5

[PATCH v1 1/8] ASoC: loongson: Use BIT() macro

[Binbin Zhou]

Where applicable, use BIT() macro instead of shift operation to improve
readability.

Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 sound/soc/loongson/loongson_dma.c | 10 +++++-----
 sound/soc/loongson/loongson_i2s.h | 24 ++++++++++++------------
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/sound/soc/loongson/loongson_dma.c b/sound/soc/loongson/loongson_dma.c
index 0238f88bc674..20e4a0641340 100644
--- a/sound/soc/loongson/loongson_dma.c
+++ b/sound/soc/loongson/loongson_dma.c
@@ -17,11 +17,11 @@
 #include "loongson_i2s.h"
 
 /* DMA dma_order Register */
-#define DMA_ORDER_STOP          (1 << 4) /* DMA stop */
-#define DMA_ORDER_START         (1 << 3) /* DMA start */
-#define DMA_ORDER_ASK_VALID     (1 << 2) /* DMA ask valid flag */
-#define DMA_ORDER_AXI_UNCO      (1 << 1) /* Uncache access */
-#define DMA_ORDER_ADDR_64       (1 << 0) /* 64bits address support */
+#define DMA_ORDER_STOP          BIT(4) /* DMA stop */
+#define DMA_ORDER_START         BIT(3) /* DMA start */
+#define DMA_ORDER_ASK_VALID     BIT(2) /* DMA ask valid flag */
+#define DMA_ORDER_AXI_UNCO      BIT(1) /* Uncache access */
+#define DMA_ORDER_ADDR_64       BIT(0) /* 64bits address support */
 
 #define DMA_ORDER_ASK_MASK      (~0x1fUL) /* Ask addr mask */
 #define DMA_ORDER_CTRL_MASK     (0x0fUL)  /* Control mask  */
diff --git a/sound/soc/loongson/loongson_i2s.h b/sound/soc/loongson/loongson_i2s.h
index 89492eebf834..c8052a762c1b 100644
--- a/sound/soc/loongson/loongson_i2s.h
+++ b/sound/soc/loongson/loongson_i2s.h
@@ -27,18 +27,18 @@
 #define LS_I2S_RX_ORDER 0x110 /* RX DMA Order */
 
 /* Loongson I2S Control Register */
-#define I2S_CTRL_MCLK_READY	(1 << 16) /* MCLK ready */
-#define I2S_CTRL_MASTER		(1 << 15) /* Master mode */
-#define I2S_CTRL_MSB		(1 << 14) /* MSB bit order */
-#define I2S_CTRL_RX_EN		(1 << 13) /* RX enable */
-#define I2S_CTRL_TX_EN		(1 << 12) /* TX enable */
-#define I2S_CTRL_RX_DMA_EN	(1 << 11) /* DMA RX enable */
-#define I2S_CTRL_CLK_READY	(1 << 8)  /* BCLK ready */
-#define I2S_CTRL_TX_DMA_EN	(1 << 7)  /* DMA TX enable */
-#define I2S_CTRL_RESET		(1 << 4)  /* Controller soft reset */
-#define I2S_CTRL_MCLK_EN	(1 << 3)  /* Enable MCLK */
-#define I2S_CTRL_RX_INT_EN	(1 << 1)  /* RX interrupt enable */
-#define I2S_CTRL_TX_INT_EN	(1 << 0)  /* TX interrupt enable */
+#define I2S_CTRL_MCLK_READY	BIT(16) /* MCLK ready */
+#define I2S_CTRL_MASTER		BIT(15) /* Master mode */
+#define I2S_CTRL_MSB		BIT(14) /* MSB bit order */
+#define I2S_CTRL_RX_EN		BIT(13) /* RX enable */
+#define I2S_CTRL_TX_EN		BIT(12) /* TX enable */
+#define I2S_CTRL_RX_DMA_EN	BIT(11) /* DMA RX enable */
+#define I2S_CTRL_CLK_READY	BIT(8)  /* BCLK ready */
+#define I2S_CTRL_TX_DMA_EN	BIT(7)  /* DMA TX enable */
+#define I2S_CTRL_RESET		BIT(4)  /* Controller soft reset */
+#define I2S_CTRL_MCLK_EN	BIT(3)  /* Enable MCLK */
+#define I2S_CTRL_RX_INT_EN	BIT(1)  /* RX interrupt enable */
+#define I2S_CTRL_TX_INT_EN	BIT(0)  /* TX interrupt enable */
 
 #define LS_I2S_DRVNAME		"loongson-i2s"
 
-- 
2.43.5

[PATCH v1 2/8] ASoC: loongson: Simplify probe() with local dev variable

[Binbin Zhou]

Simplify the probe() function by using local 'dev' instead of
&pdev->dev.

Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 sound/soc/loongson/loongson_card.c    | 17 ++++++++-------
 sound/soc/loongson/loongson_i2s_pci.c | 30 +++++++++++++--------------
 2 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/sound/soc/loongson/loongson_card.c b/sound/soc/loongson/loongson_card.c
index 4d511b7589d0..d3cd23ddd027 100644
--- a/sound/soc/loongson/loongson_card.c
+++ b/sound/soc/loongson/loongson_card.c
@@ -159,40 +159,41 @@ static int loongson_card_parse_of(struct loongson_card_data *data)
 static int loongson_asoc_card_probe(struct platform_device *pdev)
 {
 	struct loongson_card_data *ls_priv;
+	struct device *dev = &pdev->dev;
 	struct snd_soc_card *card;
 	int ret;
 
-	ls_priv = devm_kzalloc(&pdev->dev, sizeof(*ls_priv), GFP_KERNEL);
+	ls_priv = devm_kzalloc(dev, sizeof(*ls_priv), GFP_KERNEL);
 	if (!ls_priv)
 		return -ENOMEM;
 
 	card = &ls_priv->snd_card;
 
-	card->dev = &pdev->dev;
+	card->dev = dev;
 	card->owner = THIS_MODULE;
 	card->dai_link = loongson_dai_links;
 	card->num_links = ARRAY_SIZE(loongson_dai_links);
 	snd_soc_card_set_drvdata(card, ls_priv);
 
-	ret = device_property_read_string(&pdev->dev, "model", &card->name);
+	ret = device_property_read_string(dev, "model", &card->name);
 	if (ret) {
-		dev_err(&pdev->dev, "Error parsing card name: %d\n", ret);
+		dev_err(dev, "Error parsing card name: %d\n", ret);
 		return ret;
 	}
-	ret = device_property_read_u32(&pdev->dev, "mclk-fs", &ls_priv->mclk_fs);
+	ret = device_property_read_u32(dev, "mclk-fs", &ls_priv->mclk_fs);
 	if (ret) {
-		dev_err(&pdev->dev, "Error parsing mclk-fs: %d\n", ret);
+		dev_err(dev, "Error parsing mclk-fs: %d\n", ret);
 		return ret;
 	}
 
-	if (has_acpi_companion(&pdev->dev))
+	if (has_acpi_companion(dev))
 		ret = loongson_card_parse_acpi(ls_priv);
 	else
 		ret = loongson_card_parse_of(ls_priv);
 	if (ret < 0)
 		return ret;
 
-	return devm_snd_soc_register_card(&pdev->dev, card);
+	return devm_snd_soc_register_card(dev, card);
 }
 
 static const struct of_device_id loongson_asoc_dt_ids[] = {
diff --git a/sound/soc/loongson/loongson_i2s_pci.c b/sound/soc/loongson/loongson_i2s_pci.c
index ec18b122cd79..e8ea28bc5a5f 100644
--- a/sound/soc/loongson/loongson_i2s_pci.c
+++ b/sound/soc/loongson/loongson_i2s_pci.c
@@ -75,32 +75,33 @@ static int loongson_i2s_pci_probe(struct pci_dev *pdev,
 {
 	const struct fwnode_handle *fwnode = pdev->dev.fwnode;
 	struct loongson_dma_data *tx_data, *rx_data;
+	struct device *dev = &pdev->dev;
 	struct loongson_i2s *i2s;
 	int ret;
 
 	if (pcim_enable_device(pdev)) {
-		dev_err(&pdev->dev, "pci_enable_device failed\n");
+		dev_err(dev, "pci_enable_device failed\n");
 		return -ENODEV;
 	}
 
-	i2s = devm_kzalloc(&pdev->dev, sizeof(*i2s), GFP_KERNEL);
+	i2s = devm_kzalloc(dev, sizeof(*i2s), GFP_KERNEL);
 	if (!i2s)
 		return -ENOMEM;
 
 	i2s->rev_id = pdev->revision;
-	i2s->dev = &pdev->dev;
+	i2s->dev = dev;
 	pci_set_drvdata(pdev, i2s);
 
-	ret = pcim_iomap_regions(pdev, 1 << 0, dev_name(&pdev->dev));
+	ret = pcim_iomap_regions(pdev, 1 << 0, dev_name(dev));
 	if (ret < 0) {
-		dev_err(&pdev->dev, "iomap_regions failed\n");
+		dev_err(dev, "iomap_regions failed\n");
 		return ret;
 	}
 	i2s->reg_base = pcim_iomap_table(pdev)[0];
-	i2s->regmap = devm_regmap_init_mmio(&pdev->dev, i2s->reg_base,
+	i2s->regmap = devm_regmap_init_mmio(dev, i2s->reg_base,
 					    &loongson_i2s_regmap_config);
 	if (IS_ERR(i2s->regmap)) {
-		dev_err(&pdev->dev, "regmap_init_mmio failed\n");
+		dev_err(dev, "regmap_init_mmio failed\n");
 		return PTR_ERR(i2s->regmap);
 	}
 
@@ -115,34 +116,33 @@ static int loongson_i2s_pci_probe(struct pci_dev *pdev,
 
 	tx_data->irq = fwnode_irq_get_byname(fwnode, "tx");
 	if (tx_data->irq < 0) {
-		dev_err(&pdev->dev, "dma tx irq invalid\n");
+		dev_err(dev, "dma tx irq invalid\n");
 		return tx_data->irq;
 	}
 
 	rx_data->irq = fwnode_irq_get_byname(fwnode, "rx");
 	if (rx_data->irq < 0) {
-		dev_err(&pdev->dev, "dma rx irq invalid\n");
+		dev_err(dev, "dma rx irq invalid\n");
 		return rx_data->irq;
 	}
 
-	device_property_read_u32(&pdev->dev, "clock-frequency", &i2s->clk_rate);
+	device_property_read_u32(dev, "clock-frequency", &i2s->clk_rate);
 	if (!i2s->clk_rate) {
-		dev_err(&pdev->dev, "clock-frequency property invalid\n");
+		dev_err(dev, "clock-frequency property invalid\n");
 		return -EINVAL;
 	}
 
-	dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
+	dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
 
 	if (i2s->rev_id == 1) {
 		regmap_write(i2s->regmap, LS_I2S_CTRL, I2S_CTRL_RESET);
 		udelay(200);
 	}
 
-	ret = devm_snd_soc_register_component(&pdev->dev,
-					      &loongson_i2s_component,
+	ret = devm_snd_soc_register_component(dev, &loongson_i2s_component,
 					      &loongson_i2s_dai, 1);
 	if (ret) {
-		dev_err(&pdev->dev, "register DAI failed %d\n", ret);
+		dev_err(dev, "register DAI failed %d\n", ret);
 		return ret;
 	}
 
-- 
2.43.5

[PATCH v1 3/8] ASoC: loongson: Simplify with dev_err_probe()

[Binbin Zhou]

Error handling in probe() can be a bit simpler with dev_err_probe().

Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 sound/soc/loongson/loongson_card.c    | 23 ++++++++-----------
 sound/soc/loongson/loongson_i2s_pci.c | 33 ++++++++++-----------------
 2 files changed, 21 insertions(+), 35 deletions(-)

diff --git a/sound/soc/loongson/loongson_card.c b/sound/soc/loongson/loongson_card.c
index d3cd23ddd027..f0ed4508f38a 100644
--- a/sound/soc/loongson/loongson_card.c
+++ b/sound/soc/loongson/loongson_card.c
@@ -176,22 +176,17 @@ static int loongson_asoc_card_probe(struct platform_device *pdev)
 	snd_soc_card_set_drvdata(card, ls_priv);
 
 	ret = device_property_read_string(dev, "model", &card->name);
-	if (ret) {
-		dev_err(dev, "Error parsing card name: %d\n", ret);
-		return ret;
-	}
+	if (ret)
+		dev_err_probe(dev, ret, "Error parsing card name\n");
+
 	ret = device_property_read_u32(dev, "mclk-fs", &ls_priv->mclk_fs);
-	if (ret) {
-		dev_err(dev, "Error parsing mclk-fs: %d\n", ret);
-		return ret;
-	}
+	if (ret)
+		dev_err_probe(dev, ret, "Error parsing mclk-fs\n");
 
-	if (has_acpi_companion(dev))
-		ret = loongson_card_parse_acpi(ls_priv);
-	else
-		ret = loongson_card_parse_of(ls_priv);
-	if (ret < 0)
-		return ret;
+	ret = has_acpi_companion(dev) ? loongson_card_parse_acpi(ls_priv)
+				      : loongson_card_parse_of(ls_priv);
+	if (ret)
+		dev_err_probe(dev, ret, "Error parsing acpi/of properties\n");
 
 	return devm_snd_soc_register_card(dev, card);
 }
diff --git a/sound/soc/loongson/loongson_i2s_pci.c b/sound/soc/loongson/loongson_i2s_pci.c
index e8ea28bc5a5f..3872b1d8fce0 100644
--- a/sound/soc/loongson/loongson_i2s_pci.c
+++ b/sound/soc/loongson/loongson_i2s_pci.c
@@ -97,13 +97,12 @@ static int loongson_i2s_pci_probe(struct pci_dev *pdev,
 		dev_err(dev, "iomap_regions failed\n");
 		return ret;
 	}
+
 	i2s->reg_base = pcim_iomap_table(pdev)[0];
 	i2s->regmap = devm_regmap_init_mmio(dev, i2s->reg_base,
 					    &loongson_i2s_regmap_config);
-	if (IS_ERR(i2s->regmap)) {
-		dev_err(dev, "regmap_init_mmio failed\n");
-		return PTR_ERR(i2s->regmap);
-	}
+	if (IS_ERR(i2s->regmap))
+		dev_err_probe(dev, PTR_ERR(i2s->regmap), "regmap_init_mmio failed\n");
 
 	tx_data = &i2s->tx_dma_data;
 	rx_data = &i2s->rx_dma_data;
@@ -115,22 +114,16 @@ static int loongson_i2s_pci_probe(struct pci_dev *pdev,
 	rx_data->order_addr = i2s->reg_base + LS_I2S_RX_ORDER;
 
 	tx_data->irq = fwnode_irq_get_byname(fwnode, "tx");
-	if (tx_data->irq < 0) {
-		dev_err(dev, "dma tx irq invalid\n");
-		return tx_data->irq;
-	}
+	if (tx_data->irq < 0)
+		dev_err_probe(dev, tx_data->irq, "dma tx irq invalid\n");
 
 	rx_data->irq = fwnode_irq_get_byname(fwnode, "rx");
-	if (rx_data->irq < 0) {
-		dev_err(dev, "dma rx irq invalid\n");
-		return rx_data->irq;
-	}
+	if (rx_data->irq < 0)
+		dev_err_probe(dev, rx_data->irq, "dma rx irq invalid\n");
 
-	device_property_read_u32(dev, "clock-frequency", &i2s->clk_rate);
-	if (!i2s->clk_rate) {
-		dev_err(dev, "clock-frequency property invalid\n");
-		return -EINVAL;
-	}
+	ret = device_property_read_u32(dev, "clock-frequency", &i2s->clk_rate);
+	if (ret)
+		dev_err_probe(dev, ret, "clock-frequency property invalid\n");
 
 	dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
 
@@ -141,10 +134,8 @@ static int loongson_i2s_pci_probe(struct pci_dev *pdev,
 
 	ret = devm_snd_soc_register_component(dev, &loongson_i2s_component,
 					      &loongson_i2s_dai, 1);
-	if (ret) {
-		dev_err(dev, "register DAI failed %d\n", ret);
-		return ret;
-	}
+	if (ret)
+		dev_err_probe(dev, ret, "register DAI failed\n");
 
 	return 0;
 }
-- 
2.43.5

Re: [PATCH v1 3/8] ASoC: loongson: Simplify with dev_err_probe()

[Krzysztof Kozlowski]

On 09/09/2024 09:19, Binbin Zhou wrote:
> Error handling in probe() can be a bit simpler with dev_err_probe().
> 
> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>

...

>  
> -	if (has_acpi_companion(dev))
> -		ret = loongson_card_parse_acpi(ls_priv);
> -	else
> -		ret = loongson_card_parse_of(ls_priv);
> -	if (ret < 0)
> -		return ret;
> +	ret = has_acpi_companion(dev) ? loongson_card_parse_acpi(ls_priv)
> +				      : loongson_card_parse_of(ls_priv);
> +	if (ret)
> +		dev_err_probe(dev, ret, "Error parsing acpi/of properties\n");

This does not make anything simpler - you don't use dev_err_probe()
features here. OTOH, you duplicate error messages.

Best regards,
Krzysztof

Re: [PATCH v1 3/8] ASoC: loongson: Simplify with dev_err_probe()

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1131 bytes --]

On Mon, Sep 09, 2024 at 03:19:07PM +0800, Binbin Zhou wrote:

>  	ret = device_property_read_string(dev, "model", &card->name);
> -	if (ret) {
> -		dev_err(dev, "Error parsing card name: %d\n", ret);
> -		return ret;
> -	}
> +	if (ret)
> +		dev_err_probe(dev, ret, "Error parsing card name\n");
> +

This means that we change from failing if there is an error to only
logging about it (and then only during probe).  Perhaps that makes sense
but the changelog should explain what's going on.  Most if not all of
the updates do this, some of them seem like clear bugs.

> -	if (has_acpi_companion(dev))
> -		ret = loongson_card_parse_acpi(ls_priv);
> -	else
> -		ret = loongson_card_parse_of(ls_priv);
> -	if (ret < 0)
> -		return ret;
> +	ret = has_acpi_companion(dev) ? loongson_card_parse_acpi(ls_priv)
> +				      : loongson_card_parse_of(ls_priv);
> +	if (ret)
> +		dev_err_probe(dev, ret, "Error parsing acpi/of properties\n");

Please just write normal conditional statements, they're far clearer -
there's applications for the ternery operator when embedded in other
things but normally it just makes things harder to read.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v1 4/8] ASoC: loongson: Simplify if statment in loongson_card_hw_params()

[Binbin Zhou]

Deal with illegal conditions first and put the normal process code
outside the if condition to improve code readability.

Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 sound/soc/loongson/loongson_card.c | 32 +++++++++++++++---------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/sound/soc/loongson/loongson_card.c b/sound/soc/loongson/loongson_card.c
index f0ed4508f38a..3dd82caaae3b 100644
--- a/sound/soc/loongson/loongson_card.c
+++ b/sound/soc/loongson/loongson_card.c
@@ -24,27 +24,27 @@ static int loongson_card_hw_params(struct snd_pcm_substream *substream,
 				   struct snd_pcm_hw_params *params)
 {
 	struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
-	struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0);
-	struct snd_soc_dai *codec_dai = snd_soc_rtd_to_codec(rtd, 0);
 	struct loongson_card_data *ls_card = snd_soc_card_get_drvdata(rtd->card);
+	struct snd_soc_dai *codec_dai = snd_soc_rtd_to_codec(rtd, 0);
+	struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0);
 	int ret, mclk;
 
-	if (ls_card->mclk_fs) {
-		mclk = ls_card->mclk_fs * params_rate(params);
-		ret = snd_soc_dai_set_sysclk(cpu_dai, 0, mclk,
-					     SND_SOC_CLOCK_OUT);
-		if (ret < 0) {
-			dev_err(codec_dai->dev, "cpu_dai clock not set\n");
-			return ret;
-		}
+	if (!ls_card->mclk_fs)
+		return 0;
 
-		ret = snd_soc_dai_set_sysclk(codec_dai, 0, mclk,
-					     SND_SOC_CLOCK_IN);
-		if (ret < 0) {
-			dev_err(codec_dai->dev, "codec_dai clock not set\n");
-			return ret;
-		}
+	mclk = ls_card->mclk_fs * params_rate(params);
+	ret = snd_soc_dai_set_sysclk(cpu_dai, 0, mclk, SND_SOC_CLOCK_OUT);
+	if (ret < 0) {
+		dev_err(codec_dai->dev, "cpu_dai clock not set\n");
+		return ret;
 	}
+
+	ret = snd_soc_dai_set_sysclk(codec_dai, 0, mclk, SND_SOC_CLOCK_IN);
+	if (ret < 0) {
+		dev_err(codec_dai->dev, "codec_dai clock not set\n");
+		return ret;
+	}
+
 	return 0;
 }
 
-- 
2.43.5

[PATCH v1 5/8] ASoC: loongson: Simplify of_node_put() reference

[Binbin Zhou]

There are multiple references to of_node_put() in
loongson_card_parse_of(). We can merge them into one place to improve
code readability.

Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 sound/soc/loongson/loongson_card.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/sound/soc/loongson/loongson_card.c b/sound/soc/loongson/loongson_card.c
index 3dd82caaae3b..4033cd7f09f6 100644
--- a/sound/soc/loongson/loongson_card.c
+++ b/sound/soc/loongson/loongson_card.c
@@ -114,45 +114,42 @@ static int loongson_card_parse_acpi(struct loongson_card_data *data)
 
 static int loongson_card_parse_of(struct loongson_card_data *data)
 {
-	struct device_node *cpu, *codec;
 	struct snd_soc_card *card = &data->snd_card;
+	struct device_node *cpu, *codec;
 	struct device *dev = card->dev;
-	int ret, i;
+	int ret = 0, i;
 
 	cpu = of_get_child_by_name(dev->of_node, "cpu");
 	if (!cpu) {
 		dev_err(dev, "platform property missing or invalid\n");
 		return -EINVAL;
 	}
+
 	codec = of_get_child_by_name(dev->of_node, "codec");
 	if (!codec) {
 		dev_err(dev, "audio-codec property missing or invalid\n");
-		of_node_put(cpu);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto cpu_put;
 	}
 
 	for (i = 0; i < card->num_links; i++) {
 		ret = snd_soc_of_get_dlc(cpu, NULL, loongson_dai_links[i].cpus, 0);
 		if (ret < 0) {
 			dev_err(dev, "getting cpu dlc error (%d)\n", ret);
-			goto err;
+			goto codec_put;
 		}
 
 		ret = snd_soc_of_get_dlc(codec, NULL, loongson_dai_links[i].codecs, 0);
 		if (ret < 0) {
 			dev_err(dev, "getting codec dlc error (%d)\n", ret);
-			goto err;
+			goto codec_put;
 		}
 	}
 
-	of_node_put(cpu);
+codec_put:
 	of_node_put(codec);
-
-	return 0;
-
-err:
+cpu_put:
 	of_node_put(cpu);
-	of_node_put(codec);
 	return ret;
 }
 
-- 
2.43.5

[PATCH v1 6/8] ASoC: loongson: Replace if with ternary operator

[Binbin Zhou]

Replace an if statement with a ternary operator, making the code a tiny
bit shorter.

Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 sound/soc/loongson/loongson_i2s.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/sound/soc/loongson/loongson_i2s.c b/sound/soc/loongson/loongson_i2s.c
index 3b9786076501..8bb38e418333 100644
--- a/sound/soc/loongson/loongson_i2s.c
+++ b/sound/soc/loongson/loongson_i2s.c
@@ -21,34 +21,30 @@
 			SNDRV_PCM_FMTBIT_S20_3LE | \
 			SNDRV_PCM_FMTBIT_S24_LE)
 
+#define LOONGSON_I2S_TX_ENABLE	(I2S_CTRL_TX_EN | I2S_CTRL_TX_DMA_EN)
+#define LOONGSON_I2S_RX_ENABLE	(I2S_CTRL_RX_EN | I2S_CTRL_RX_DMA_EN)
+
 static int loongson_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
 				struct snd_soc_dai *dai)
 {
 	struct loongson_i2s *i2s = snd_soc_dai_get_drvdata(dai);
+	unsigned int mask;
 	int ret = 0;
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
-			regmap_update_bits(i2s->regmap, LS_I2S_CTRL,
-					   I2S_CTRL_TX_EN | I2S_CTRL_TX_DMA_EN,
-					   I2S_CTRL_TX_EN | I2S_CTRL_TX_DMA_EN);
-		else
-			regmap_update_bits(i2s->regmap, LS_I2S_CTRL,
-					   I2S_CTRL_RX_EN | I2S_CTRL_RX_DMA_EN,
-					   I2S_CTRL_RX_EN | I2S_CTRL_RX_DMA_EN);
+		mask = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) ?
+		       LOONGSON_I2S_TX_ENABLE : LOONGSON_I2S_RX_ENABLE;
+		regmap_update_bits(i2s->regmap, LS_I2S_CTRL, mask, mask);
 		break;
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
-		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
-			regmap_update_bits(i2s->regmap, LS_I2S_CTRL,
-					I2S_CTRL_TX_EN | I2S_CTRL_TX_DMA_EN, 0);
-		else
-			regmap_update_bits(i2s->regmap, LS_I2S_CTRL,
-					I2S_CTRL_RX_EN | I2S_CTRL_RX_DMA_EN, 0);
+		mask = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) ?
+		       LOONGSON_I2S_TX_ENABLE : LOONGSON_I2S_RX_ENABLE;
+		regmap_update_bits(i2s->regmap, LS_I2S_CTRL, mask, 0);
 		break;
 	default:
 		ret = -EINVAL;
-- 
2.43.5

Re: [PATCH v1 6/8] ASoC: loongson: Replace if with ternary operator

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 209 bytes --]

On Mon, Sep 09, 2024 at 03:19:36PM +0800, Binbin Zhou wrote:

> Replace an if statement with a ternary operator, making the code a tiny
> bit shorter.

Please don't, it's shorter but not great for legibility.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v1 7/8] ASoC: loongson: Factor out loongson_card_acpi_find_device() function

[Binbin Zhou]

The operations for reading the cpu and codec nodes in
loongson_card_parse_acpi() are similar, so we convert them into a simple
helper function called loongson_card_acpi_find_device().

Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 sound/soc/loongson/loongson_card.c | 55 +++++++++++++++++-------------
 1 file changed, 31 insertions(+), 24 deletions(-)

diff --git a/sound/soc/loongson/loongson_card.c b/sound/soc/loongson/loongson_card.c
index 4033cd7f09f6..e55ad52ff43b 100644
--- a/sound/soc/loongson/loongson_card.c
+++ b/sound/soc/loongson/loongson_card.c
@@ -68,46 +68,53 @@ static struct snd_soc_dai_link loongson_dai_links[] = {
 	},
 };
 
-static int loongson_card_parse_acpi(struct loongson_card_data *data)
+static struct acpi_device *loongson_card_acpi_find_device(struct snd_soc_card *card,
+							  const char *name)
 {
-	struct snd_soc_card *card = &data->snd_card;
 	struct fwnode_handle *fwnode = card->dev->fwnode;
 	struct fwnode_reference_args args;
+	int status;
+
+	memset(&args, 0, sizeof(args));
+	status = acpi_node_get_property_reference(fwnode, name, 0, &args);
+	if (status || !is_acpi_device_node(args.fwnode)) {
+		dev_err(card->dev, "No matching phy in ACPI table\n");
+		return NULL;
+	}
+
+	return to_acpi_device_node(args.fwnode);
+}
+
+static int loongson_card_parse_acpi(struct loongson_card_data *data)
+{
+	struct snd_soc_card *card = &data->snd_card;
 	const char *codec_dai_name;
 	struct acpi_device *adev;
 	struct device *phy_dev;
-	int ret, i;
+	int i;
 
 	/* fixup platform name based on reference node */
-	memset(&args, 0, sizeof(args));
-	ret = acpi_node_get_property_reference(fwnode, "cpu", 0, &args);
-	if (ret || !is_acpi_device_node(args.fwnode)) {
-		dev_err(card->dev, "No matching phy in ACPI table\n");
-		return ret ?: -ENOENT;
-	}
-	adev = to_acpi_device_node(args.fwnode);
+	adev = loongson_card_acpi_find_device(card, "cpu");
+	if (!adev)
+		return -ENOENT;
+
 	phy_dev = acpi_get_first_physical_node(adev);
 	if (!phy_dev)
 		return -EPROBE_DEFER;
-	for (i = 0; i < card->num_links; i++)
-		loongson_dai_links[i].platforms->name = dev_name(phy_dev);
 
 	/* fixup codec name based on reference node */
-	memset(&args, 0, sizeof(args));
-	ret = acpi_node_get_property_reference(fwnode, "codec", 0, &args);
-	if (ret || !is_acpi_device_node(args.fwnode)) {
-		dev_err(card->dev, "No matching phy in ACPI table\n");
-		return ret ?: -ENOENT;
-	}
-	adev = to_acpi_device_node(args.fwnode);
+	adev = loongson_card_acpi_find_device(card, "codec");
+	if (!adev)
+		return -ENOENT;
 	snprintf(codec_name, sizeof(codec_name), "i2c-%s", acpi_dev_name(adev));
-	for (i = 0; i < card->num_links; i++)
-		loongson_dai_links[i].codecs->name = codec_name;
 
-	device_property_read_string(card->dev, "codec-dai-name",
-				    &codec_dai_name);
-	for (i = 0; i < card->num_links; i++)
+	device_property_read_string(card->dev, "codec-dai-name", &codec_dai_name);
+
+	for (i = 0; i < card->num_links; i++) {
+		loongson_dai_links[i].platforms->name = dev_name(phy_dev);
+		loongson_dai_links[i].codecs->name = codec_name;
 		loongson_dai_links[i].codecs->dai_name = codec_dai_name;
+	}
 
 	return 0;
 }
-- 
2.43.5

[PATCH v1 8/8] ASoC: loongson: Factor out loongson i2s enable clock functions

[Binbin Zhou]

There are a few i2s clock enable operations in loongson_i2s_set_fmt(),
convert them to simple helper functions called
loongson_i2s_enable_mclk() and loongson_i2s_enable_bclk().

Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
---
 sound/soc/loongson/loongson_i2s.c | 86 +++++++++++++++++--------------
 1 file changed, 47 insertions(+), 39 deletions(-)

diff --git a/sound/soc/loongson/loongson_i2s.c b/sound/soc/loongson/loongson_i2s.c
index 8bb38e418333..40bbf3205391 100644
--- a/sound/soc/loongson/loongson_i2s.c
+++ b/sound/soc/loongson/loongson_i2s.c
@@ -24,6 +24,9 @@
 #define LOONGSON_I2S_TX_ENABLE	(I2S_CTRL_TX_EN | I2S_CTRL_TX_DMA_EN)
 #define LOONGSON_I2S_RX_ENABLE	(I2S_CTRL_RX_EN | I2S_CTRL_RX_DMA_EN)
 
+#define LOONGSON_I2S_DEF_DELAY		10
+#define LOONGSON_I2S_DEF_TIMEOUT	500000
+
 static int loongson_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
 				struct snd_soc_dai *dai)
 {
@@ -119,10 +122,40 @@ static int loongson_i2s_set_dai_sysclk(struct snd_soc_dai *dai, int clk_id,
 	return 0;
 }
 
+static int loongson_i2s_enable_mclk(struct loongson_i2s *i2s)
+{
+	u32 val;
+
+	if (i2s->rev_id == 0)
+		return 0;
+
+	regmap_update_bits(i2s->regmap, LS_I2S_CTRL,
+			   I2S_CTRL_MCLK_EN, I2S_CTRL_MCLK_EN);
+
+	return regmap_read_poll_timeout_atomic(i2s->regmap,
+					       LS_I2S_CTRL, val,
+					       val & I2S_CTRL_MCLK_READY,
+					       LOONGSON_I2S_DEF_DELAY,
+					       LOONGSON_I2S_DEF_TIMEOUT);
+}
+
+static int loongson_i2s_enable_bclk(struct loongson_i2s *i2s)
+{
+	u32 val;
+
+	if (i2s->rev_id == 0)
+		return 0;
+
+	return regmap_read_poll_timeout_atomic(i2s->regmap,
+					       LS_I2S_CTRL, val,
+					       val & I2S_CTRL_CLK_READY,
+					       LOONGSON_I2S_DEF_DELAY,
+					       LOONGSON_I2S_DEF_TIMEOUT);
+}
+
 static int loongson_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
 {
 	struct loongson_i2s *i2s = snd_soc_dai_get_drvdata(dai);
-	u32 val;
 	int ret;
 
 	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
@@ -144,54 +177,29 @@ static int loongson_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
 		/* Enable master mode */
 		regmap_update_bits(i2s->regmap, LS_I2S_CTRL, I2S_CTRL_MASTER,
 				   I2S_CTRL_MASTER);
-		if (i2s->rev_id == 1) {
-			ret = regmap_read_poll_timeout_atomic(i2s->regmap,
-						LS_I2S_CTRL, val,
-						val & I2S_CTRL_CLK_READY,
-						10, 500000);
-			if (ret < 0)
-				dev_warn(dai->dev, "wait BCLK ready timeout\n");
-		}
+		ret = loongson_i2s_enable_bclk(i2s);
+		if (ret < 0)
+			dev_warn(dai->dev, "wait BCLK ready timeout\n");
 		break;
 	case SND_SOC_DAIFMT_BC_FP:
 		/* Enable MCLK */
-		if (i2s->rev_id == 1) {
-			regmap_update_bits(i2s->regmap, LS_I2S_CTRL,
-					   I2S_CTRL_MCLK_EN,
-					   I2S_CTRL_MCLK_EN);
-			ret = regmap_read_poll_timeout_atomic(i2s->regmap,
-						LS_I2S_CTRL, val,
-						val & I2S_CTRL_MCLK_READY,
-						10, 500000);
-			if (ret < 0)
-				dev_warn(dai->dev, "wait MCLK ready timeout\n");
-		}
+		ret = loongson_i2s_enable_mclk(i2s);
+		if (ret < 0)
+			dev_warn(dai->dev, "wait MCLK ready timeout\n");
 		break;
 	case SND_SOC_DAIFMT_BP_FP:
 		/* Enable MCLK */
-		if (i2s->rev_id == 1) {
-			regmap_update_bits(i2s->regmap, LS_I2S_CTRL,
-					   I2S_CTRL_MCLK_EN,
-					   I2S_CTRL_MCLK_EN);
-			ret = regmap_read_poll_timeout_atomic(i2s->regmap,
-						LS_I2S_CTRL, val,
-						val & I2S_CTRL_MCLK_READY,
-						10, 500000);
-			if (ret < 0)
-				dev_warn(dai->dev, "wait MCLK ready timeout\n");
-		}
+		ret = loongson_i2s_enable_mclk(i2s);
+		if (ret < 0)
+			dev_warn(dai->dev, "wait MCLK ready timeout\n");
 
 		/* Enable master mode */
 		regmap_update_bits(i2s->regmap, LS_I2S_CTRL, I2S_CTRL_MASTER,
 				   I2S_CTRL_MASTER);
-		if (i2s->rev_id == 1) {
-			ret = regmap_read_poll_timeout_atomic(i2s->regmap,
-						LS_I2S_CTRL, val,
-						val & I2S_CTRL_CLK_READY,
-						10, 500000);
-			if (ret < 0)
-				dev_warn(dai->dev, "wait BCLK ready timeout\n");
-		}
+
+		ret = loongson_i2s_enable_bclk(i2s);
+		if (ret < 0)
+			dev_warn(dai->dev, "wait BCLK ready timeout\n");
 		break;
 	default:
 		return -EINVAL;
-- 
2.43.5

Re: (subset) [PATCH v1 0/8] ASoC: loongson: Simplify code formatting

[Mark Brown]

On Mon, 09 Sep 2024 15:19:04 +0800, Binbin Zhou wrote:
> This patchset attempts to improve code readability by simplifying code
> formatting.
> No functional changes.
> 
> Original link:
> https://lore.kernel.org/all/c54fbc2582702689c005e1ba528ab2318b1adde6.1725518229.git.zhoubinbin@loongson.cn/
> 
> [...]

Applied to

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

Thanks!

[1/8] ASoC: loongson: Use BIT() macro
      commit: f2bd6f5b3777d800fb3cad17b9509e1e2128df62
[2/8] ASoC: loongson: Simplify probe() with local dev variable
      commit: ce3997ab8b4ae2312b8cdddd7db9b15eb11004a3
[3/8] ASoC: loongson: Simplify with dev_err_probe()
      commit: 3d2528d6c021cba141a7079ae1a5937190700899
[4/8] ASoC: loongson: Simplify if statment in loongson_card_hw_params()
      commit: e28ee1b8a92e6797fc652fa64e536178dd627e89
[6/8] ASoC: loongson: Replace if with ternary operator
      commit: c7b626a8930d7029c096f0634ae6169af59d92b6
[7/8] ASoC: loongson: Factor out loongson_card_acpi_find_device() function
      commit: ddb538a3004b10a04a14a0d275c5f52a8d161e80
[8/8] ASoC: loongson: Factor out loongson i2s enable clock functions
      commit: 4c22b04e116e114f18d9cef15f913e1649ccb7ed

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