[PATCH v2 0/2] Fix error check and cleanup for JH7110 TDM

[PATCH v2 0/2] Fix error check and cleanup for JH7110 TDM

[Walker Chen]

Some minor issues were found during addtional testing and static
analysis. The first patch fix the error check for the return value of
devm_reset_control_array_get_exclusive(). The second patch drop some
unused macros.

Fixes: fd4762b6b5cf ("ASoC: starfive: Add JH7110 TDM driver")

Changes since v1:
- Fix an error check in jh7110_tdm_clk_reset_get().
- Return to use *_BIT to indicate the shift and remove some unused macros.

---
v1: https://lore.kernel.org/all/20230607081439.1517-1-walker.chen@starfivetech.com/

Walker Chen (2):
  ASoC: starfive: Fix an error check in jh7110_tdm_clk_reset_get()
  ASoC: starfive: Remove some unused macros

 sound/soc/starfive/jh7110_tdm.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

-- 
2.17.1

[PATCH v2 1/2] ASoC: starfive: Fix an error check in jh7110_tdm_clk_reset_get()

[Walker Chen]

Fix the check for devm_reset_control_array_get_exclusive() return value.
The devm_reset_control_array_get_exclusive() function may return NULL if
it's an optional request. If optional is intended then NULL should not
be treated as an error case, but as a special kind of success case. So
here the IS_ERR() is used to check better.

Signed-off-by: Walker Chen <walker.chen@starfivetech.com>
---
Fix the following issue:
https://lore.kernel.org/all/ZH7t6Nc+NTcGpq%2F3@moroto/
---
 sound/soc/starfive/jh7110_tdm.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/sound/soc/starfive/jh7110_tdm.c b/sound/soc/starfive/jh7110_tdm.c
index 973b910d2d3e..a9a3d52bdd2a 100644
--- a/sound/soc/starfive/jh7110_tdm.c
+++ b/sound/soc/starfive/jh7110_tdm.c
@@ -580,10 +580,9 @@ static int jh7110_tdm_clk_reset_get(struct platform_device *pdev,
 	}
 
 	tdm->resets = devm_reset_control_array_get_exclusive(&pdev->dev);
-	if (IS_ERR_OR_NULL(tdm->resets)) {
-		ret = PTR_ERR(tdm->resets);
-		dev_err(&pdev->dev, "Failed to get tdm resets");
-		return ret;
+	if (IS_ERR(tdm->resets)) {
+		dev_err(&pdev->dev, "Failed to get tdm resets\n");
+		return PTR_ERR(tdm->resets);
 	}
 
 	return 0;
-- 
2.17.1

[PATCH v2 2/2] ASoC: starfive: Remove some unused macros

[Walker Chen]

These macros are unused and can be dropped.

Signed-off-by: Walker Chen <walker.chen@starfivetech.com>
---
 sound/soc/starfive/jh7110_tdm.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/sound/soc/starfive/jh7110_tdm.c b/sound/soc/starfive/jh7110_tdm.c
index a9a3d52bdd2a..e4bdba20c499 100644
--- a/sound/soc/starfive/jh7110_tdm.c
+++ b/sound/soc/starfive/jh7110_tdm.c
@@ -25,11 +25,8 @@
 #include <sound/soc-dai.h>
 
 #define TDM_PCMGBCR			0x00
-	#define PCMGBCR_MASK		0x1e
 	#define PCMGBCR_ENABLE		BIT(0)
-	#define PCMGBCR_TRITXEN		BIT(4)
 	#define CLKPOL_BIT		5
-	#define TRITXEN_BIT		4
 	#define ELM_BIT			3
 	#define SYNCM_BIT		2
 	#define MS_BIT			1
@@ -42,11 +39,6 @@
 	#define LRJ_BIT			1
 #define TDM_PCMRXCR			0x08
 	#define PCMRXCR_RXEN		BIT(0)
-	#define PCMRXCR_RXSL_MASK	0xc
-	#define PCMRXCR_RXSL_16BIT	0x4
-	#define PCMRXCR_RXSL_32BIT	0x8
-	#define PCMRXCR_SCALE_MASK	0xf0
-	#define PCMRXCR_SCALE_1CH	0x10
 #define TDM_PCMDIV			0x0c
 
 #define JH7110_TDM_FIFO			0x170c0000
-- 
2.17.1

Re: [PATCH v2 1/2] ASoC: starfive: Fix an error check in jh7110_tdm_clk_reset_get()

[Claudiu.Beznea]

On 08.06.2023 16:57, Walker Chen wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Fix the check for devm_reset_control_array_get_exclusive() return value.
> The devm_reset_control_array_get_exclusive() function may return NULL if
> it's an optional request. If optional is intended then NULL should not
> be treated as an error case, but as a special kind of success case. So
> here the IS_ERR() is used to check better.
> 
> Signed-off-by: Walker Chen <walker.chen@starfivetech.com>
> ---
> Fix the following issue:
> https://lore.kernel.org/all/ZH7t6Nc+NTcGpq%2F3@moroto/
> ---
>  sound/soc/starfive/jh7110_tdm.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/sound/soc/starfive/jh7110_tdm.c b/sound/soc/starfive/jh7110_tdm.c
> index 973b910d2d3e..a9a3d52bdd2a 100644
> --- a/sound/soc/starfive/jh7110_tdm.c
> +++ b/sound/soc/starfive/jh7110_tdm.c
> @@ -580,10 +580,9 @@ static int jh7110_tdm_clk_reset_get(struct platform_device *pdev,
>         }
> 
>         tdm->resets = devm_reset_control_array_get_exclusive(&pdev->dev);
> -       if (IS_ERR_OR_NULL(tdm->resets)) {
> -               ret = PTR_ERR(tdm->resets);
> -               dev_err(&pdev->dev, "Failed to get tdm resets");
> -               return ret;
> +       if (IS_ERR(tdm->resets)) {
> +               dev_err(&pdev->dev, "Failed to get tdm resets\n");

There is an extra "\n" added to this to this patch which is not mentioned
anywhere. Just make sure to do one thing per patch. Other than this:

Reviewed-by: Claudiu Beznea <claudiu.beznea@microchip.com>


> +               return PTR_ERR(tdm->resets);
>         }
> 
>         return 0;
> --
> 2.17.1
> 

Re: [PATCH v2 2/2] ASoC: starfive: Remove some unused macros

[Claudiu.Beznea]

On 08.06.2023 16:57, Walker Chen wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> These macros are unused and can be dropped.
> 
> Signed-off-by: Walker Chen <walker.chen@starfivetech.com>

Reviewed-by: Claudiu Beznea <claudiu.beznea@microchip.com>


> ---
>  sound/soc/starfive/jh7110_tdm.c | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/sound/soc/starfive/jh7110_tdm.c b/sound/soc/starfive/jh7110_tdm.c
> index a9a3d52bdd2a..e4bdba20c499 100644
> --- a/sound/soc/starfive/jh7110_tdm.c
> +++ b/sound/soc/starfive/jh7110_tdm.c
> @@ -25,11 +25,8 @@
>  #include <sound/soc-dai.h>
> 
>  #define TDM_PCMGBCR                    0x00
> -       #define PCMGBCR_MASK            0x1e
>         #define PCMGBCR_ENABLE          BIT(0)
> -       #define PCMGBCR_TRITXEN         BIT(4)
>         #define CLKPOL_BIT              5
> -       #define TRITXEN_BIT             4
>         #define ELM_BIT                 3
>         #define SYNCM_BIT               2
>         #define MS_BIT                  1
> @@ -42,11 +39,6 @@
>         #define LRJ_BIT                 1
>  #define TDM_PCMRXCR                    0x08
>         #define PCMRXCR_RXEN            BIT(0)
> -       #define PCMRXCR_RXSL_MASK       0xc
> -       #define PCMRXCR_RXSL_16BIT      0x4
> -       #define PCMRXCR_RXSL_32BIT      0x8
> -       #define PCMRXCR_SCALE_MASK      0xf0
> -       #define PCMRXCR_SCALE_1CH       0x10
>  #define TDM_PCMDIV                     0x0c
> 
>  #define JH7110_TDM_FIFO                        0x170c0000
> --
> 2.17.1
> 

Re: [PATCH v2 0/2] Fix error check and cleanup for JH7110 TDM

[Mark Brown]

On Thu, 08 Jun 2023 21:57:48 +0800, Walker Chen wrote:
> Some minor issues were found during addtional testing and static
> analysis. The first patch fix the error check for the return value of
> devm_reset_control_array_get_exclusive(). The second patch drop some
> unused macros.
> 
> Fixes: fd4762b6b5cf ("ASoC: starfive: Add JH7110 TDM driver")
> 
> [...]

Applied to

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

Thanks!

[1/2] ASoC: starfive: Fix an error check in jh7110_tdm_clk_reset_get()
      commit: 3582cf94ff49469ffe78e96014550f7d4e466fbd
[2/2] ASoC: starfive: Remove some unused macros
      commit: 8bd81864533bd02d6922deadeed643c813dfe142

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