[PATCH v2 0/2] Fix error check and cleanup for JH7110 TDM
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.c...
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(-)
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
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
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;
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
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
participants (3)
-
Claudiu.Beznea＠microchip.com -
Mark Brown -
Walker Chen