Message ID | 20221022162742.21671-2-aidanmacdonald.0x0@gmail.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:4242:0:0:0:0:0 with SMTP id s2csp1274901wrr; Sat, 22 Oct 2022 09:31:36 -0700 (PDT) X-Google-Smtp-Source: AMsMyM79tJcmbWN8VyXs5DZQKXm8Bi6ST2DjovnAoCtvB2l0ykVz+dZUhNx0Nr7LX4piajEh5+QP X-Received: by 2002:a17:907:2daa:b0:78d:4dca:43e with SMTP id gt42-20020a1709072daa00b0078d4dca043emr20505423ejc.134.1666456296619; Sat, 22 Oct 2022 09:31:36 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1666456296; cv=none; d=google.com; s=arc-20160816; b=JxFQlTg4xMpdOiMzcORVkHI6yFkQ+nc5QdMipZpZiUNG1GzkgBiUtm9VRmfxFfN9QB ySBwivojMYg3qmdR1hnjZtKcO2koCukxDOv1rTX9DzlzFOhdKTnTGROg2UswFwNhzxRl NvoFqfeivdhopA/00QtBHxMkuDARIuCXC448kEhMcvxRUtFuZHAyIBjlgZIZcDTbaAHG 6TuF9NRtoWWahkyn+bD5ZICHvif8nUQglYmUlr7qzIyIdIxN5VGRDOxk3hFqw/tohI8p 7o4Ej+/C82gpBNCa00xORF6w8Ze0iBhDm+GiiGr+Oj6Qviumr1Ir8W+PEgXpONpEnF+P fVHg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=u3LZtQhmhTtG0av/Vhn5L+1eJ4O3uKOFFREHllICaQw=; b=oIRuWF4sZBHEzgP4qwrjikdHx21W4MqvmePloktaUPFL2QJafKmEoWshX2tT9kJwNF MN+1H5otwGFV26Ap6jnMWIbmuVhCtCS7ZGPEDnugH+IyFFwA96WeZDjmU9HsWiI3OnO4 hYg+4p5xpUTqZySpI+qsV1dH8+lYR8rliSd8eiKblY+o6FnFZVcqnBOwxjUr1ZbwDKDs t4ebt2DmlQ8hj1Bev7PFZ2D0A4tuMB5vIOU47uK5DLuduxni4cSFYUI8WQpfnf+UwW3r wDg5NsRk9ulz67wj4FCh15ibxBZbuHWVGHfbR64/3SuFkw0xI86qRm20u5Gr/Cohwhp+ ugBg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20210112 header.b=qwKBPuGa; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id n5-20020a170906700500b0077fc66b581esi18158964ejj.688.2022.10.22.09.31.11; Sat, 22 Oct 2022 09:31:36 -0700 (PDT) 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; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20210112 header.b=qwKBPuGa; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229766AbiJVQ2A (ORCPT <rfc822;pwkd43@gmail.com> + 99 others); Sat, 22 Oct 2022 12:28:00 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55884 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229515AbiJVQ16 (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Sat, 22 Oct 2022 12:27:58 -0400 Received: from mail-wr1-x42f.google.com (mail-wr1-x42f.google.com [IPv6:2a00:1450:4864:20::42f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 06C6B13588F; Sat, 22 Oct 2022 09:27:58 -0700 (PDT) Received: by mail-wr1-x42f.google.com with SMTP id n12so9269525wrp.10; Sat, 22 Oct 2022 09:27:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=u3LZtQhmhTtG0av/Vhn5L+1eJ4O3uKOFFREHllICaQw=; b=qwKBPuGa1JTF0Lz0r3+ZjdptUqdyXE/Rvkl5x/B8w7r8FeSm49MVyLrsinFqRgQXxy lOopTc/54bAT4biIxpZeI9MX7D5PBJ5AmXFxsVd+1BhbGqA8dm/hfy2CkPuQOHG8wf8V G8gwFY3mKrO+ib6aO1EE+60Ar2bFZ5GwSichX1192pkD8BP5JfUdlq6viMFDBcCRpzrX HZpgAfszR4MTEn98go/JcL0rE+5JQ/5yJSl2O0qzufwxN1Ihy0Do7Me5izt2XRi2QW/y mLidQEuvuK0ABThySmoD6VEQ2X7sWRCGRyp8ngN9uPUnw2rt+5u3xNKjGMSMB9x/7qdE gaUg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=u3LZtQhmhTtG0av/Vhn5L+1eJ4O3uKOFFREHllICaQw=; b=EKP0yjFSlzTjLnRRuJ1DwtHgGZ7gsdgNQISppfVFWRZxZJcm+zcvmZ/a6dGCBFjSXr ETB64kPTEBXuLOV6sWUwgCITQw3WPyuxaLRWYt0RFd+y1634tvci8nOeZ5f0kLOR55Pz q9743pcp9k6PN331oQV0OfnBTx9Cw4RajditONiDV+sSYOhF6fdFBQ0/t3aa4FOnzWCz s6BSnptw1hC+lFpvP/eJWSCKiGhxc1tz2EBgkLs0gtuCaedYYkdgI83wU9fnPI2UxRkH NHhVT88PvpFJOwwl7zbAVnC32YVSfkoRcyVHmvSgF4e7m1Fr111h3xVU0vQSwxpwwsXw f8Rw== X-Gm-Message-State: ACrzQf3+nD9IG38xcynUUuqCizY+VH/AC5/S8lqlNFh6GvLLtdLFmMgl UGus3EvRUgypNFt0AwxM6hY= X-Received: by 2002:a5d:6da1:0:b0:231:c189:e077 with SMTP id u1-20020a5d6da1000000b00231c189e077mr16103020wrs.114.1666456076553; Sat, 22 Oct 2022 09:27:56 -0700 (PDT) Received: from localhost (94.197.3.61.threembb.co.uk. [94.197.3.61]) by smtp.gmail.com with ESMTPSA id be5-20020a05600c1e8500b003a531c7aa66sm6141184wmb.1.2022.10.22.09.27.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 22 Oct 2022 09:27:56 -0700 (PDT) From: Aidan MacDonald <aidanmacdonald.0x0@gmail.com> To: broonie@kernel.org, lgirdwood@gmail.com, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, kuninori.morimoto.gx@renesas.com Cc: perex@perex.cz, tiwai@suse.com, alsa-devel@alsa-project.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v1 2/2] dt-bindings: ASoC: simple-card: Add system-clock-id property Date: Sat, 22 Oct 2022 17:27:42 +0100 Message-Id: <20221022162742.21671-2-aidanmacdonald.0x0@gmail.com> In-Reply-To: <20221022162742.21671-1-aidanmacdonald.0x0@gmail.com> References: <20221022162742.21671-1-aidanmacdonald.0x0@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-1.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1747406077977435505?= X-GMAIL-MSGID: =?utf-8?q?1747406077977435505?= |
Series |
[v1,1/2] ASoC: simple-card: Support custom DAI system clock IDs
|
|
Commit Message
Aidan MacDonald
Oct. 22, 2022, 4:27 p.m. UTC
This is a new per-DAI property used to specify the clock ID argument
to snd_soc_dai_set_sysclk().
Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
---
Documentation/devicetree/bindings/sound/simple-card.yaml | 8 ++++++++
1 file changed, 8 insertions(+)
Comments
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
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. Regards, Aidan
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
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. Regards, Aidan
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
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> writes: > 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 Device trees already use standardized enumerations in other areas so it isn't a new idea. Look under include/dt-bindings/clock. Every header there contains an arbitrary enumeration of a device's clocks. In fact most of include/dt-bindings is exactly for this purpose, to define standard values that are not "just numbers" but an enum, a flag, etc, with a special meaning. It is not specific to clocks. There is no dt-binding for system clock ID, because prior to this patch they were not exposed to DT in any way. But the enumerations themselves already exist, eg. the IDs for nau8821 codec: /* System Clock Source */ enum { NAU8821_CLK_DIS, NAU8821_CLK_MCLK, NAU8821_CLK_INTERNAL, NAU8821_CLK_FLL_MCLK, NAU8821_CLK_FLL_BLK, NAU8821_CLK_FLL_FS, }; We would just be moving these into dt-bindings if somebody wants to use a codec with simple-card. Future drivers would add the enum into dt-bindings from the start because that's where it belongs. Regards, Aidan
On 25/10/2022 05:14, Aidan MacDonald wrote: >> Krzysztof > > Device trees already use standardized enumerations in other areas so it > isn't a new idea. Look under include/dt-bindings/clock. Every header > there contains an arbitrary enumeration of a device's clocks. In fact > most of include/dt-bindings is exactly for this purpose, to define > standard values that are not "just numbers" but an enum, a flag, etc, > with a special meaning. It is not specific to clocks. > > There is no dt-binding for system clock ID, because prior to this patch > they were not exposed to DT in any way. But the enumerations themselves > already exist, eg. the IDs for nau8821 codec: > > /* System Clock Source */ > enum { > NAU8821_CLK_DIS, > NAU8821_CLK_MCLK, > NAU8821_CLK_INTERNAL, > NAU8821_CLK_FLL_MCLK, > NAU8821_CLK_FLL_BLK, > NAU8821_CLK_FLL_FS, > }; OK, this looks good. > > We would just be moving these into dt-bindings if somebody wants to > use a codec with simple-card. Future drivers would add the enum into > dt-bindings from the start because that's where it belongs. And the remaining piece I don't get is that these are not bindings for codec, but for sound audio card. You want to set "system-clock-id" property for audio card, while putting clock from codec, which will be used to pass back to the codec... so it is a property of the codec, not of the audio card. IOW, NAU8821_CLK_* does not configure here the clock of the system, but only, only clock of the codec. Best regards, Krzysztof
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> writes: > And the remaining piece I don't get is that these are not bindings for > codec, but for sound audio card. You want to set "system-clock-id" > property for audio card, while putting clock from codec, which will be > used to pass back to the codec... so it is a property of the codec, not > of the audio card. IOW, NAU8821_CLK_* does not configure here the clock > of the system, but only, only clock of the codec. The system clock is controlled at the DAI level, it's specific to one DAI on one component. The simple-card device node has sub-nodes for the DAI links, and each DAI link node has sub-nodes for the DAIs within the link. "system-clock-id" is a property on the DAI nodes, so it's not a card-level property, just one part of the overall card definition. Since the clock ID is something defined by the codec it would naturally be a value defined by the codec, but the *configuration* of the codec is part of the sound card because it depends on how everything is connected together. If you used the same codec in a different machine it would have a different configuration. Regards, Aidan
On 26/10/2022 10:48, Aidan MacDonald wrote: > > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> writes: > >> And the remaining piece I don't get is that these are not bindings for >> codec, but for sound audio card. You want to set "system-clock-id" >> property for audio card, while putting clock from codec, which will be >> used to pass back to the codec... so it is a property of the codec, not >> of the audio card. IOW, NAU8821_CLK_* does not configure here the clock >> of the system, but only, only clock of the codec. > > The system clock is controlled at the DAI level, it's specific to one > DAI on one component. The simple-card device node has sub-nodes for the > DAI links, and each DAI link node has sub-nodes for the DAIs within the > link. "system-clock-id" is a property on the DAI nodes, so it's not a > card-level property, just one part of the overall card definition. > > Since the clock ID is something defined by the codec it would naturally > be a value defined by the codec, but the *configuration* of the codec is > part of the sound card because it depends on how everything is connected > together. If you used the same codec in a different machine it would > have a different configuration. OK, that sounds reasonable. Thank you for explaining this. You still need to convince Mark :) Best regards, Krzysztof
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(). > > Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com> > --- > Documentation/devicetree/bindings/sound/simple-card.yaml | 8 ++++++++ > 1 file changed, 8 insertions(+) My concerns were addressed, so: Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> writes: > On 26/10/2022 10:48, Aidan MacDonald wrote: >> >> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> writes: >> >>> And the remaining piece I don't get is that these are not bindings for >>> codec, but for sound audio card. You want to set "system-clock-id" >>> property for audio card, while putting clock from codec, which will be >>> used to pass back to the codec... so it is a property of the codec, not >>> of the audio card. IOW, NAU8821_CLK_* does not configure here the clock >>> of the system, but only, only clock of the codec. >> >> The system clock is controlled at the DAI level, it's specific to one >> DAI on one component. The simple-card device node has sub-nodes for the >> DAI links, and each DAI link node has sub-nodes for the DAIs within the >> link. "system-clock-id" is a property on the DAI nodes, so it's not a >> card-level property, just one part of the overall card definition. >> >> Since the clock ID is something defined by the codec it would naturally >> be a value defined by the codec, but the *configuration* of the codec is >> part of the sound card because it depends on how everything is connected >> together. If you used the same codec in a different machine it would >> have a different configuration. > > OK, that sounds reasonable. Thank you for explaining this. You still > need to convince Mark :) No problem, thanks for bearing with all my explanations! Mark raised some good points, and I have to agree with him. This could create too many future issues, and the problem might be better solved with the clock API -- but unfortunately that's not yet feasible. Regards, Aidan
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. + Defaults to 0 if unspecified. + $ref: /schemas/types.yaml#/definitions/uint32 + mclk-fs: description: | Multiplication factor between stream rate and codec mclk. @@ -145,6 +151,8 @@ definitions: $ref: "#/definitions/system-clock-direction-out" system-clock-fixed: $ref: "#/definitions/system-clock-fixed" + system-clock-id: + $ref: "#/definitions/system-clock-id" required: - sound-dai