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: <3c131132-35ce-e5da-2608-36d53abc6c83@linaro.org>
Date:   Thu, 11 May 2023 19:57:40 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [PATCH v1 3/3] dt-bindings: ASoC: Add ESS ES9218P codec bindings
Content-Language: en-US
To:     Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
Cc:     lgirdwood@gmail.com, broonie@kernel.org, robh+dt@kernel.org,
        krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
        perex@perex.cz, tiwai@suse.com, alsa-devel@alsa-project.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20230510112349.939991-1-aidanmacdonald.0x0@gmail.com>
 <20230510112349.939991-3-aidanmacdonald.0x0@gmail.com>
 <7b3a37e8-0210-c539-5b5b-bf8e587707ea@linaro.org>
 <7Zga7RBqLNcaG5ylTIY4Qn7CUdE1IJsW@localhost>
 <8e98b31a-ef4d-a8bd-32f3-e6f496f2b138@linaro.org>
 <zfuvar6MiqIaW9nvjgvUIhA1t3DkZ9tE@localhost>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <zfuvar6MiqIaW9nvjgvUIhA1t3DkZ9tE@localhost>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 11/05/2023 13:26, Aidan MacDonald wrote:
> 
> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> writes:
> 
>> On 11/05/2023 12:15, Aidan MacDonald wrote:
>>>
>>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> writes:
>>>
>>>> On 10/05/2023 13:23, Aidan MacDonald wrote:
>>>>> ...
>>>>> +  ess,max-clock-div:
>>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>>> +    description:
>>>>> +      Sets the maximum MCLK divider for generating the internal CLK.
>>>>> +      CLK must be at least 20x the I2C bus speed or I2C transactions
>>>>> +      will fail. The maximum divider should be chosen to ensure that
>>>>> +      CLK will not fall below the limit.
>>>>> +    enum:
>>>>> +      - 1
>>>>> +      - 2
>>>>> +      - 4
>>>>> +      - 8
>>>>> +    default: 1
>>>>
>>>> Why do you need to customize it per board?
>>>>
>>>
>>> There's no generic API to read or change the bus speed (SCL frequency)
>>> for I2C adapters, so it's impossible to calculate a limit on the MCLK
>>> divider automatically.
>>>
>>> Defaulting to 1 is always safe, but means wasting power at lower sample
>>> rates. If you know what the bus speed is you can use a higher divider
>>> limit to save power, and it has to be done at the board/firmware level
>>> because that's the only place where the bus speed is known.
>>
>> If I understand correctly, you only miss a way to get bus_freq_hz from
>> i2c_timings to calculate the divider by yourself? This looks like Linux
>> limitation, so we shouldn't push it into DT, but rather fix Linux. The
>> I2C bus rate is known, the MCLK rate as well, so divider is possible to
>> deduce.
>>
>> I am actually surprised that I2C core does not store the timings
>> anywhere and each bus host has to deal with it on its own.
>>
>> Best regards,
>> Krzysztof
> 
> I agree it'd be better if Linux provided access the bus frequency, but
> even if that API was added it will take time for every I2C adapter to
> support it. So in that case we would still need a DT property to provide
> a safe limit or use a safe default, and miss potential power savings.
> 
> I'd prefer to add the DT property to allow power savings to be had now,
> and drop it if/when the kernel gets an API for bus frequency. That will
> be safe from a compatibility point of view -- the property won't be
> providing any useful information so it won't matter if old DTs have it.

OK, sounds good.

Best regards,
Krzysztof