linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Rob Herring <robh@kernel.org>
To: Han Xu <han.xu@nxp.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
	Michael Walle <michael@walle.cc>, Bough Chen <haibo.chen@nxp.com>,
	ashish.kumar@nxp.com, yogeshgaur.83@gmail.com,
	broonie@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
Subject: Re: [PATCH 07/11] dt-bindings: spi: spi-nxp-fspi: add a new property nxp,fspi-dll-slvdly
Date: Wed, 6 Jul 2022 10:11:10 -0600	[thread overview]
Message-ID: <20220706161110.GA123915-robh@kernel.org> (raw)
In-Reply-To: <20220705155031.zk36jsq4q2ac2ow4@umbrella>

On Tue, Jul 05, 2022 at 10:50:31AM -0500, Han Xu wrote:
> On 22/07/05 05:38PM, Krzysztof Kozlowski wrote:
> > 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?

What's a cell?

> > 
> > 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?
> 
> That make sense. I will use degree as the unit and round to register proper
> value in driver as Michael suggested. Thanks for all comments.

I'm still wondering what this is delaying? From what to what? We already 
have numerous common delay properties for SPI. If one of those doesn't 
work, then should this be a new common property? And if common, I think 
time units is better to use than degrees.

If this is vendor specific, then I'd just use the register value. 
There's not much point in using common units unless it is a common 
property.

Rob

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

  reply	other threads:[~2022-07-06 16:12 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-05  9:11 [PATCH 01/11] spi: spi-nxp-fspi: enable runtime pm for fspi haibo.chen
2022-07-05  9:11 ` [PATCH 02/11] spi: spi-nxp-fspi: change the default lut index haibo.chen
2022-07-05  9:11 ` [PATCH 03/11] spi: spi-nxp-fspi: add DTR mode support haibo.chen
2022-07-05  9:11 ` [PATCH 04/11] spi: spi-nxp-fspi: add function to select sample clock source for flash reading haibo.chen
2022-07-06 21:02   ` Michael Walle
2022-07-05  9:11 ` [PATCH 05/11] spi: spi-nxp-fspi: Add quirk to disable DTR support haibo.chen
2022-07-05 13:50   ` Michael Walle
2022-07-05  9:11 ` [PATCH 06/11] spi: spi-nxp-fspi: enable octal ddr for iMX8QM/QXP/DXL haibo.chen
2022-07-05  9:11 ` [PATCH 07/11] dt-bindings: spi: spi-nxp-fspi: add a new property nxp,fspi-dll-slvdly haibo.chen
2022-07-05  9:48   ` Krzysztof Kozlowski
2022-07-05 10:28     ` Bough Chen
2022-07-05 10:36       ` Krzysztof Kozlowski
2022-07-05 13:19         ` Han Xu
2022-07-05 13:29           ` Krzysztof Kozlowski
2022-07-05 14:00             ` Han Xu
2022-07-05 14:03               ` Krzysztof Kozlowski
2022-07-05 14:31                 ` Han Xu
2022-07-05 14:06               ` Michael Walle
2022-07-05 14:12                 ` Krzysztof Kozlowski
2022-07-05 14:52                   ` Han Xu
2022-07-05 14:58                     ` Michael Walle
2022-07-05 15:07                       ` Mark Brown
2022-07-05 15:38                     ` Krzysztof Kozlowski
2022-07-05 15:50                       ` Han Xu
2022-07-06 16:11                         ` Rob Herring [this message]
2022-07-06 20:59                           ` Michael Walle
2022-07-05  9:11 ` [PATCH 08/11] mtd: spi-nor: macronix: add support for Macronix octaflash haibo.chen
2022-07-05  9:11 ` [PATCH 09/11] mtd: spi-nor: macronix: add mx25uw51345g OPI mode support haibo.chen
2022-07-18  6:57   ` Michael Walle
2022-07-05  9:11 ` [PATCH 10/11] arm64: dts: imx8ulp: add flexspi support haibo.chen
2022-07-05  9:11 ` [PATCH 11/11] arm64: dts: imx8qm/imx8qxp: " haibo.chen
2022-07-05 14:01 ` [PATCH 01/11] spi: spi-nxp-fspi: enable runtime pm for fspi Michael Walle
2022-07-05 23:06   ` Han Xu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220706161110.GA123915-robh@kernel.org \
    --to=robh@kernel.org \
    --cc=ashish.kumar@nxp.com \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=festevam@gmail.com \
    --cc=haibo.chen@nxp.com \
    --cc=han.xu@nxp.com \
    --cc=kernel@pengutronix.de \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=krzysztof.kozlowski@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-imx@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=michael@walle.cc \
    --cc=miquel.raynal@bootlin.com \
    --cc=p.yadav@ti.com \
    --cc=richard@nod.at \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.org \
    --cc=singh.kuldeep87k@gmail.com \
    --cc=tudor.ambarus@microchip.com \
    --cc=vigneshr@ti.com \
    --cc=yogeshgaur.83@gmail.com \
    --cc=zhengxunli@mxic.com.tw \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).