On Thu, Mar 18, 2021 at 01:06:10PM +0000, Mark Brown wrote:
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).
Unfortunately I don't know which values I should validate. While the data sheet has these fields "documented" it doesn't have the units, so I don't know if I'm close in the minimum/maximum range or not. I will add documentation to the routine for each step of what I'm doing at least though. If better documentation becomes available or a second implementation presents itself we can update this to validate.
https://rockchip.fr/RK817%20datasheet%20V1.01.pdf
- 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.
Acknowledged.