Support boards without APLL

[aliask]

I'm currently working with an STM32 S3 board and I've run into a number of problems, including the lack of APLL support on the I2S interface.

1. I2S_CLK_SRC_APLL is hard-coded: https://github.com/CarlosDerSeher/snapclient/blob/f52f436219fbf76103a736a7d0bb418ac9cad7e4/components/lightsnapcast/player.c#L255
2. adjust_apll() calls rtc_clk_apll_coeff_set() which does not exist in my setup, breaking compilation: https://github.com/CarlosDerSeher/snapclient/blob/f52f436219fbf76103a736a7d0bb418ac9cad7e4/components/lightsnapcast/player.c#L821

I've implemented a working solution by replacing CONFIG_USE_SAMPLE_INSERTION with CONFIG_USE_APLL. Doing it this way defaults to a "safe" configuration, and only gives you the option if the underlying hardware supports the feature.

:warning: However, sdkconfig would not be backwards-compatible - I'm not sure if that's a dealbreaker.

[CarlosDerSeher]

Couldn't we keep backward compatibility by only changing those apll related things and keep sample stuffing the default setting?

[aliask]

I wasn't sure what the behaviour was when setting default true on a board which doesn't support it. Luckily it looks like the feature stays disabled due to the SOC_I2S_SUPPORTS_APLL in my testing :smile:

[CarlosDerSeher]

I meant using sample stuffing as default sync implementation as apll tuning has issues with some DACs.

[aliask]

Sample insertion is already the default today. The problem is that the APLL functions/symbols are referenced even when you're using sample stuffing, which is a problem for boards which don't have that feature. The project won't compile.

Or do you mean that we should go back to using CONFIG_USE_SAMPLE_INSERTION instead of CONFIG_USE_APLL (and ditch the depends on flag in Kconfig)? That should keep backwards compatibility, but with the disadvantage that it's possible for a user to select an invalid configuration. I can update the PR to do this if you prefer.

[CarlosDerSeher]

I'd prefer to keep backward compatibility as I think I'll ditch the APLL stuff at some point anyway.

[CarlosDerSeher]

@aliask Please check if commit 111501e6314796a2d0f407aca192a002c4aae671 makes this obsolete.

[aliask]

It doesn't appear so. Configuring ESP32 S3 target and building gives the following error (same as master):

/project/components/lightsnapcast/player.c: In function 'adjust_apll':
/project/components/lightsnapcast/player.c:787:3: error: implicit declaration of function 'rtc_clk_apll_coeff_set'; did you mean 'rtc_clk_apb_freq_get'? [-Werror=implicit-function-declaration]
  787 |   rtc_clk_apll_coeff_set(o_div, sdm0, sdm1, sdm2);
      |   ^~~~~~~~~~~~~~~~~~~~~~
      |   rtc_clk_apb_freq_get

I haven't debugged in depth but from a quick look at the changes it still seems like that function would be included regardless of the board config.

[CarlosDerSeher]

Ah OK, I forgot about this.

[luar123]

I think this can be closed, see #112

[CarlosDerSeher]

I think this will be closed as soon as we merge to master