On 24/10/2022 19:38, Aidan MacDonald wrote: > > Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> writes: > >> On 23/10/2022 09:47, Aidan MacDonald wrote: >>> >>> Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> writes: >>> >>>> On 22/10/2022 12:27, Aidan MacDonald wrote: >>>>> This is a new per-DAI property used to specify the clock ID argument >>>>> to snd_soc_dai_set_sysclk(). >>>> >>>> You did no show the use of this property and here you refer to some >>>> specific Linux driver implementation, so in total this does no look like >>>> a hardware property. >>>> >>>> You also did not explain why do you need it (the most important piece of >>>> commit msg). >>>> >>>>> >>>>> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@xxxxxxxxx> >>>>> --- >>>>> Documentation/devicetree/bindings/sound/simple-card.yaml | 8 ++++++++ >>>>> 1 file changed, 8 insertions(+) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/sound/simple-card.yaml b/Documentation/devicetree/bindings/sound/simple-card.yaml >>>>> index ed19899bc94b..cb7774e235d0 100644 >>>>> --- a/Documentation/devicetree/bindings/sound/simple-card.yaml >>>>> +++ b/Documentation/devicetree/bindings/sound/simple-card.yaml >>>>> @@ -57,6 +57,12 @@ definitions: >>>>> single fixed sampling rate. >>>>> $ref: /schemas/types.yaml#/definitions/flag >>>>> >>>>> + system-clock-id: >>>>> + description: | >>>>> + Specify the clock ID used for setting the DAI system clock. >>>> >>>> >>>> With lack of explanation above, I would say - use common clock framework >>>> to choose a clock... >>>> >>>> >>>> Best regards, >>>> Krzysztof >>> >>> Sorry, I didn't explain things very well. The system clock ID is indeed >>> a property of the DAI hardware. The ID is not specific to Linux in any >>> way, and really it's an enumeration that requires a dt-binding. >>> >>> A DAI may support multiple system clock inputs or outputs identified by >>> the clock ID. In the case of outputs, these could be distinct clocks >>> that have their own I/O pins, or the clock ID could select the internal >>> source clock used for a clock generator. For inputs, the system clock ID >>> may inform the DAI how or where the system clock is being provided so >>> hardware registers can be configured appropriately. >>> >>> Really the details do not matter, except that in a particular DAI link >>> configuration a specific clock ID must be used. This is determined by >>> the actual hardware connection between the DAIs; if the wrong clock is >>> used, the DAI may not function correctly. >>> >>> Currently the device tree is ambiguous as to which system clock should >>> be used when the DAI supports more than one, because there is no way to >>> specify which clock was intended. Linux just treats the ID as zero, but >>> that's currently a Linux-specific numbering so there's guarantee that >>> another OS would choose the same clock as Linux. >>> >>> The system-clock-id property is therefore necessary to fully describe >>> the hardware connection between DAIs in a DAI link when a DAI offers >>> more than one choice of system clock. >>> >>> I will resend the patch with the above in the commit message. >> >> For example if you want to define which input pin to use (so you have >> internal mux), it's quite unspecific to give them some indexes. What is >> 0? What is 1? Number of pin? Number of pin counting from where? >> >> Since this is unanswered, the IDs are also driver and implementation >> dependent, thus you still have the same problem - another OS can choose >> different clock. That's not then a hardware description, but software >> configuration. >> >> Best regards, >> Krzysztof > > I answered this already. The enumeration is arbitrary. Create some > dt-bindings and voila, it becomes standardized and OS-independent. Hm, then I missed something. Can you point me to DTS and bindings (patches or in-tree) which show this standardized indices of clock inputs? Best regards, Krzysztof
