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: <0bd271e9-8d9b-7388-2d9b-65cc39a54f8c@linaro.org>
Date:   Tue, 5 Jul 2022 17:38:24 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCH 07/11] dt-bindings: spi: spi-nxp-fspi: add a new property
 nxp,fspi-dll-slvdly
Content-Language: en-US
To:     Han Xu <han.xu@nxp.com>
Cc:     Michael Walle <michael@walle.cc>, Bough Chen <haibo.chen@nxp.com>,
        ashish.kumar@nxp.com, yogeshgaur.83@gmail.com, broonie@kernel.org,
        robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
        singh.kuldeep87k@gmail.com, tudor.ambarus@microchip.com,
        p.yadav@ti.com, miquel.raynal@bootlin.com, richard@nod.at,
        vigneshr@ti.com, shawnguo@kernel.org, s.hauer@pengutronix.de,
        kernel@pengutronix.de, linux-spi@vger.kernel.org,
        linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
        linux-mtd@lists.infradead.org, festevam@gmail.com,
        dl-linux-imx <linux-imx@nxp.com>,
        linux-arm-kernel@lists.infradead.org, zhengxunli@mxic.com.tw
References: <1657012303-6464-1-git-send-email-haibo.chen@nxp.com>
 <1657012303-6464-7-git-send-email-haibo.chen@nxp.com>
 <ef676df1-77e0-b8ee-3950-97eade8ddd5b@linaro.org>
 <VI1PR04MB40167A70FBE772DF91047A4190819@VI1PR04MB4016.eurprd04.prod.outlook.com>
 <59d360ef-5374-c7a7-2995-854ab3715b25@linaro.org>
 <DU2PR04MB87747C9A8F18D8300461D6B197819@DU2PR04MB8774.eurprd04.prod.outlook.com>
 <f33ad190-f5c7-d9fa-088b-5538ab1f4d59@linaro.org>
 <DU2PR04MB877492F346BAA10B2AA7428497819@DU2PR04MB8774.eurprd04.prod.outlook.com>
 <62f113a0cdb0d58bf04ab0b274912eb7@walle.cc>
 <be521f90-97ce-c61d-d7d6-8f2bde24d824@linaro.org>
 <20220705145226.tarpvub6bh67tj63@umbrella>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <20220705145226.tarpvub6bh67tj63@umbrella>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 05/07/2022 16:52, Han Xu wrote:
> On 22/07/05 04:12PM, Krzysztof Kozlowski wrote:
>> On 05/07/2022 16:06, Michael Walle wrote:
>>>
>>>>>
>>>>> I think you could use here clock cycles or clock phase, but then it 
>>>>> has to be obvious
>>>>> it is that unit.
>>>>
>>>> Hi Krzysztof,
>>>>
>>>> Let me clarify it, in the document a term "delay cell" was used to
>>>> descript this register bit. Each delay cell equals "1/32 clock phase",
>>>> so the unit of delay cell is clock phase. The value user need set in
>>>> DT just number to define how many delay cells needed.
>>>
>>> Then should the unit be "-degrees" and the possible range 0-180?
>>
>> Thanks. We don't have it documented currently, but the unit seems
>> reasonable.
> 
> IMO, use the unit "-degrees" makes it more complicate. Personaly I would
> calculate how many clock cycle delay needed, such as 1/4 clock cycle or half
> clock cycle. Using degree brings extra calculation.

And what if the next device uses a bit different divider? Like 1/16?
This is why we have standard units so people won't push register values
into the bindings.

> 
> The granularity of the clock phase change is 1/32 of 180 degree, but the range
> 0-180 make people feel it can be set in any degree in range.

Yes, because that's how the bindings are being written - allowing any
reasonable value, not register-specific value, to be used because it is
the most flexible, hardware-independent and allows further customization
of bindings (e.g. new devices). Embedding device programming model into
the bindings contradicts it.

Second, nothing stops you from narrowing the acceptable values with an
enum. This still allows extension. Your 1/32 does not.

> 
> If I describe all details of the relation between "nxp,fspi-dll-slvdly" and
> "delay cell" in patch v2, do you think it's clear for users?

1/32 could be a nice unit, but degrees is better. Just like uV is better
than 1/32 of V. Like 1 us is better than 1/32 of ms.

Do you see  in the bindings many other values like time, potential,
current or power described in 1/32 units?

Best regards,
Krzysztof