Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <66c1a100-922e-4a33-e80c-fc80866acf03@linaro.org>
Date:   Mon, 24 Oct 2022 20:00:31 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.0
Subject: Re: [PATCH v1 2/2] dt-bindings: ASoC: simple-card: Add
 system-clock-id property
To:     Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
Cc:     broonie@kernel.org, lgirdwood@gmail.com, robh+dt@kernel.org,
        krzysztof.kozlowski+dt@linaro.org,
        kuninori.morimoto.gx@renesas.com, perex@perex.cz, tiwai@suse.com,
        alsa-devel@alsa-project.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20221022162742.21671-1-aidanmacdonald.0x0@gmail.com>
 <20221022162742.21671-2-aidanmacdonald.0x0@gmail.com>
 <ef6a326b-5c61-988b-2ec2-cd8e233e5d28@linaro.org>
 <GMvEU8xVTkjIoQ518XWAaLkhldSZHlk7@localhost>
 <4ef59d94-d045-55fc-d531-c84e7edb8333@linaro.org>
 <hXRpArckbrXUelDdaJ3Y2SErmKiuycXt@localhost>
Content-Language: en-US
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <hXRpArckbrXUelDdaJ3Y2SErmKiuycXt@localhost>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 24/10/2022 19:38, Aidan MacDonald wrote:
> 
> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> writes:
> 
>> On 23/10/2022 09:47, Aidan MacDonald wrote:
>>>
>>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 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@gmail.com>
>>>>> ---
>>>>>  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