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/[email protected]/
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
These macros are unused and can be dropped.
Signed-off-by: Walker Chen <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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 <[email protected]>
> + return PTR_ERR(tdm->resets);
> }
>
> return 0;
> --
> 2.17.1
>
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 <[email protected]>
Reviewed-by: Claudiu Beznea <[email protected]>
> ---
> 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
>
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