Re: [PATCH v4 1/3] ASoC: Add Rockchip rk817 audio CODEC support

On Wed, Mar 17, 2021 at 03:21:15PM -0500, Chris Morgan wrote:

Add support for the Rockchip rk817 audio codec integrated into the rk817 PMIC. This is based on the sources provided by Rockchip from their BSP kernel.

Modulo the issue with the compatible in the binding this looks good apart from a couple of small nits:

+static int rk817_set_component_pll(struct snd_soc_component *component,

• int pll_id, int source, unsigned int freq_in,
  

• unsigned int freq_out)
  

+{

• snd_soc_component_write(component, RK817_CODEC_APLL_CFG1, 0x58);

This should really validate freq_in and freq_out, confirming that they're whatever fixed values this register sequence is for (if you don't know what freq_out actually is it's more OK to skip that than freq_in I guess since the constraints on the DAI link should ensure we end up with a sensible value).

• snd_soc_component_write(component, RK817_CODEC_APLL_CFG2, 0x2d);
• snd_soc_component_write(component, RK817_CODEC_APLL_CFG3, 0x0c);
• snd_soc_component_write(component, RK817_CODEC_APLL_CFG4, 0xa5);
• return 0;

+};

No ; at the end of the function definition.