Re: [PATCH v2 2/3] bindings: sound: Add documentation for TI j721e EVM (CPB and IVI)

Peter Ujfalusi
2020-06-11T08:59:24Z

Hi Rob,

On 20/05/2020 14.50, Peter Ujfalusi wrote:
>>> + clocks:
>>> + items:
>>> + - description: PLL4 clock
>>> + - description: PLL15 clock
>>> + - description: McASP10 auxclk clock
>>> + - description: PLL4_HSDIV0 parent for McASP10 auxclk (for 48KHz)
>>> + - description: PLL15_HSDIV0 parent for McASP10 auxclk (for 44.1KHz)
>>> + - description: AUDIO_REFCLK2 clock
>>> + - description: PLL4_HSDIV2 parent for AUDIO_REFCLK2 clock (for 48KHz)
>>> + - description: PLL15_HSDIV2 parent for AUDIO_REFCLK2 clock (for 44.1KHz)
>>
>> What h/w are these connected to?
>
> These clocks are internal to the SoC with the exception of AUDIO_REFCLK2
> which is routed to SoC pin.
>
>> You have no control interface here, so how do you have clocks?
>
> I need to control these clocks, the sound card is the user of these clocks.
>
>> Defining parent clocks seems wrong, too. This seems to just be a
>> collection of clocks a driver happens to need. Really, you should be
>> able query possible parents and select one with the right frequency
>> multiple.
>
> The issue in hand is that I need to dynamically switch between certain
> parents of the cpb-mcasp (for McASP) and audio-refclk2 (for the codec)
> based on sampling rate of the stream.
>
> The McASP auxclk parent can be selected from 7 source and I must use the
> two dedicated ones.
> The REFCLK2 parent can be selected from 30 source.
>
> It is also a limitation of the system that I can not query directly the
> PLL4/PLL15 frequencies, I can only get the frequency on the HSDIVs, but
> I can not get the divider on them.
> In order to handle the constraints on clocking I need to know the source
> rate so the dividers can be taken into account. The codec is picky when
> it comes to clocking and there is a need to switch between
> 256/512/768xFS based SCKI in order to be able to support sampling rates.
>
> At the moment I have fixed clocks in place for the pll4/15 with the
> rates they are configured so the dts can switch to a real clock which I
> can use in the future.
> As things are it is unlikely that I will ever going to have access to
> them, but I wanted to avoid in the bindings:
> ti,j721e-pll4-rate = <1179648000>;
> ti,j721e-pll15-rate = <1083801600>;
>
>>> +
>>> + clock-names:
>>> + items:
>>> + - const: pll4
>>> + - const: pll15
>>> + - const: cpb-mcasp
>>> + - const: cpb-mcasp-48000
>>> + - const: cpb-mcasp-44100
>>> + - const: audio-refclk2
>>> + - const: audio-refclk2-48000
>>> + - const: audio-refclk2-44100

It should be better to document the refclk2 (and refclk0 for IVI) clocks
based on it's use, not source:
cpb-codec-scki (sourced from audio_refclk2)
cpb-codec-scki-48000/44100

For the IVI binding it is just s/cpb/ivi in the clock-names.

- Péter