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: <38205667-d36f-e7a9-21b0-2e8597a662ff@linaro.org>
Date:   Tue, 25 Oct 2022 08:19:02 -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
Content-Language: en-US
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>
 <66c1a100-922e-4a33-e80c-fc80866acf03@linaro.org>
 <jZCUALhj8PoqVkuWdtLf8LnPAj1wDakF@localhost>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <jZCUALhj8PoqVkuWdtLf8LnPAj1wDakF@localhost>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

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