Message-ID: <qg7epd5xmwefp7kimlgummlktnonrd3pd7obnq3chgzrceg3lv@t4iyccqn2zdt>
Date: Fri, 5 Jul 2024 12:51:27 +0200
From: Thierry Reding <thierry.reding@...il.com>
To: Rob Herring <robh@...nel.org>
Cc: Krishna Yarlagadda <kyarlagadda@...dia.com>, 
	linux-tegra@...r.kernel.org, devicetree@...r.kernel.org, linux-doc@...r.kernel.org, 
	linux-i2c@...r.kernel.org, linux-mmc@...r.kernel.org, linux-kernel@...r.kernel.org, 
	jonathanh@...dia.com, krzk+dt@...nel.org, conor+dt@...nel.org, corbet@....net, 
	andi.shyti@...nel.org, wsa+renesas@...g-engineering.com, ulf.hansson@...aro.org, 
	adrian.hunter@...el.com, digetx@...il.com, ldewangan@...dia.com, mkumard@...dia.com
Subject: Re: [RFC PATCH V2 04/12] dt-bindings: misc: tegra-i2c: config
 settings

On Wed, Jul 03, 2024 at 02:21:04PM GMT, Rob Herring wrote:
> On Tue, Jul 2, 2024 at 4:29 AM Thierry Reding <thierry.reding@...il.com> wrote:
> >
> > On Mon, Jul 01, 2024 at 11:42:27AM GMT, Rob Herring wrote:
> > > On Mon, Jul 01, 2024 at 08:42:22PM +0530, Krishna Yarlagadda wrote:
> > > > I2C interface timing registers are configured using config setting
> > > > framework. List available field properties for Tegra I2C controllers.
> > >
> > > How is I2C bus timing parameters specific to NVIDIA? Just because you
> > > have more controls? No. That's no reason to invent a whole new way to
> > > specify parameters. Extend what's already there and make it work for
> > > anyone.
> >
> > This may be applicable to a subset of this, and yes, maybe we can find
> > generalizations for some of these parameters.
> >
> > However, we're also looking for feedback specifically on these config
> > nodes that go beyond individual timing parameters. For example in the
> > case of I2C, how should parameters for different operating modes be
> > described?
> 
> Like what? It all looks like timing to me.

The problem here isn't the individual properties but rather how to group
them. More generally the problem is that we have a set of settings that
need to be applied in different variants. Yes, they are all timings, but
the values differ based on what mode a given controller operates at.

Take for example I2C where we have things like start-hold time or stop-
setup time, which we could describe in a more generic way (i.e. leave
out the vendor prefix). However, depending on the mode that the I2C
controller runs at (could be standard mode, fast mode or fastplus mode)
these values need to be adjusted.

So it's the same set of properties but with different values for each
different operating mode. As far as I can tell there's no good construct
to describe this in DT currently.

> > Would you agree with something along the lines provided in this series?
> 
> When there are multiple users/vendors of it, maybe.
> 
> In general, it goes against the DT design of properties for foo go in
> foo's node. This looks more like how ACPI does things where it's add
> another table for this new thing we need.

Well, that's what Krishna had proposed in the first version of the
series, which you guys rejected. The problem with that is that we cannot
easily group these settings using nodes because subnodes of I2C
controllers are considered to be clients by default. This applies to SPI
and other busses as well.

This approach avoids these issues and can be more easily optimized since
settings could be shared between multiple instances of the controllers.
I have a slight preference of putting this into the controllers' device
tree nodes, but I can't think of a good way of avoiding the above child
node problem other than what we had in v1.

Thierry

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)

Powered by blists - more mailing lists