Re: [PATCH RFC 1/3] ASoC: dt-bindings: amlogic,axg-sound-card: document clocks and clock-names

[Jerome Brunet]

On Fri 14 Jun 2024 at 18:24, Neil Armstrong <neil.armstrong@xxxxxxxxxx> wrote:

> The sound card design is based on 3 reference PLL frequencies that
> are the root of all clock rates calculations.
>
> Today, those 3 frequencies are specified in DT via assigned-clocks,
> because they correspond to the basic audio use-case.
>
> It makes no sense to setup clock rates for a sound card without
> referencing the clocks for the sound card, mainly because at
> some point more complex audio use cases will be supported
> and those root rates would need to change.
>
> To solve this situation, let's legitimize the presence of assigned-clocks
> in the sound card by documenting those clocks, as it describes a true
> dependency of the sound card and paths the way of more complex
> audio uses-cases involving those root frequencies.
>
> Signed-off-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx>
> ---
>  .../devicetree/bindings/sound/amlogic,axg-sound-card.yaml    | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/sound/amlogic,axg-sound-card.yaml b/Documentation/devicetree/bindings/sound/amlogic,axg-sound-card.yaml
> index 5db718e4d0e7..676ff2731b86 100644
> --- a/Documentation/devicetree/bindings/sound/amlogic,axg-sound-card.yaml
> +++ b/Documentation/devicetree/bindings/sound/amlogic,axg-sound-card.yaml
> @@ -26,6 +26,18 @@ properties:
>        A list off component DAPM widget. Each entry is a pair of strings,
>        the first being the widget type, the second being the widget name
>  
> +  clocks:
> +    maxItems: 3
> +    description:
> +      Base PLL clocks of audio susbsytem, used to configure base clock
> +      frequencies for different audio use-cases.
> +
> +  clock-names:
> +    items:
> +      - const: mpll0
> +      - const: mpll1
> +      - const: mpll2
> +

Thanks a lot for this series. This is going in the right direction
but requiring 3 clocks or naming them (whatever the name) is not
appropriate.

The purpose is for the sound card to get the necessary base rates it
needs for its operation.

So far it has always been 3 clocks because of the 3 usual family rates
and enough PLLs are available. But this is not required. There could be
none (very unlikely but possible if fixed clocks are or with slave setups),
one (probable on a1 from what I can tell), or even more than 3, if one
needs supports for unusual rates.

Also the PLLs are not necessarily the mplls, HiFi PLL is used on some
device. It could even be the GP0 or external slave clocks which is why
putting a limit the number of clocks would be arbitrary.

I think the following would better describe the HW:

clocks: true
assigned-clocks: true
assigned-clock-parents: true
assigned-clock-rates: true

Maybe just 'clocks: true' is enough since the presence of would allow
'assigned-clocks'

For sure, clock-names is not useful, for axg of gx compatible card at least.
All inputs are equal to the card, so index are enough if the card needed
to change rates are runtime (but it is very unlikely to happen, the
whole axg or gx system are meant to operate with fixed PLLs so it would
not be compatible)

>  patternProperties:
>    "^dai-link-[0-9]+$":
>      type: object

PS: I just noticed that my reply to your previous series was private.
    It was not meant to be. Sorry about that.

-- 
Jerome