Received-SPF: pass (google.com: domain of linux-kernel+bounces-186373-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Wed, 22 May 2024 09:39:17 -0500
From: Rob Herring <robh@kernel.org>
To: Eddie James <eajames@linux.ibm.com>
Cc: Krzysztof Kozlowski <krzk@kernel.org>, linux-fsi@lists.ozlabs.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	krzk+dt@kernel.org, conor+dt@kernel.org, joel@jms.id.au,
	andrew@codeconstruct.com.au
Subject: Re: [PATCH v5 5/9] dt-bindings: fsi: Document the FSI controller
 common properties
Message-ID: <20240522143917.GA3261343-robh@kernel.org>
References: <20240514195435.155372-1-eajames@linux.ibm.com>
 <20240514195435.155372-6-eajames@linux.ibm.com>
 <9200e46a-3cb5-4363-a560-ee3d88e05ced@kernel.org>
 <a219f01e-a856-46cb-83c4-4fde99b8addd@linux.ibm.com>
 <eb6e2b5b-f341-404b-9215-6e80f21a6842@kernel.org>
 <80713a00-9574-4ae0-8af7-3fe12affcfc1@linux.ibm.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <80713a00-9574-4ae0-8af7-3fe12affcfc1@linux.ibm.com>

On Wed, May 15, 2024 at 10:02:46AM -0500, Eddie James wrote:
> 
> On 5/15/24 09:35, Krzysztof Kozlowski wrote:
> > On 15/05/2024 16:28, Eddie James wrote:
> > > On 5/15/24 09:18, Krzysztof Kozlowski wrote:
> > > > On 14/05/2024 21:54, Eddie James wrote:
> > > > > Since there are multiple FSI controllers documented, the common
> > > > > properties should be documented separately and then referenced
> > > > > from the specific controller documentation.
> > > > > 
> > > > > Signed-off-by: Eddie James <eajames@linux.ibm.com>
> > > > > ---
> > > > > Changes since v4:
> > > > >    - Add interrupt controller properties
> > > > >    - Add clock-frequency property to FSI controller and CFAM
> > > > >    - Add detail to chip-id property description
> > > > > 
> > > > >    .../bindings/fsi/fsi-controller.yaml          | 66 +++++++++++++++++++
> > > > >    1 file changed, 66 insertions(+)
> > > > >    create mode 100644 Documentation/devicetree/bindings/fsi/fsi-controller.yaml
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/fsi/fsi-controller.yaml b/Documentation/devicetree/bindings/fsi/fsi-controller.yaml
> > > > > new file mode 100644
> > > > > index 0000000000000..8620e4da6de77
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/fsi/fsi-controller.yaml
> > > > > @@ -0,0 +1,66 @@
> > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > +%YAML 1.2
> > > > > +---
> > > > > +$id: http://devicetree.org/schemas/fsi/fsi-controller.yaml#
> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > +
> > > > > +title: FSI Controller Common Properties
> > > > > +
> > > > > +maintainers:
> > > > > +  - Eddie James <eajames@linux.ibm.com>
> > > > > +
> > > > > +description:
> > > > > +  FSI (FRU (Field Replaceable Unit) Service Interface) is a two wire bus. The
> > > > > +  FSI bus is connected to a CFAM (Common FRU Access Macro) which contains
> > > > > +  various engines such as I2C controllers, SPI controllers, etc.
> > > > > +
> > > > > +properties:
> > > > > +  "#address-cells":
> > > > > +    const: 2
> > > > > +
> > > > > +  "#size-cells":
> > > > > +    const: 0
> > > > > +
> > > > > +  '#interrupt-cells':
> > > > > +    const: 1
> > > > > +
> > > > > +  clock-frequency:
> > > > > +    minimum: 1
> > > > > +    maximum: 200000000
> > > > This is a deprecated property in general. Why did it appear? It does not
> > > > exist in current bindings and nothing in commit msg suggests changes in
> > > > the bindings themselves.
> > > 
> > > OK, is there some document that describes what properties are
> > > deprecated? Because it's used all over the place in the bindings. Anyway
> > dtschema: dtschema/schemas/clock/clock.yaml
> > 
> > buses anyway should use bus-frequency but it is also legacy one.
> > 
> > > I need this property, I can rename it if you like. I can also update the
> > Why do you need it? Why clocks cannot be chosen by drivers and initial
> > state selected by assigned-clock-rates?
> 
> 
> Well, I could use assigned-clock-rates, though it seems like I'd then have
> to implement the clock provider framework for both the FSI controller driver
> and the CFAM driver, which is a lot of extra work. FSI controller isn't
> really a clock provider, it's a bus like i2c (which uses clock-frequency),
> so it doesn't quite fit in my opinion...

'clock-frequency' was used for I2C, but it really should have been 
'bus-frequency' as it is frequency of the bus rather than input 
frequency to the controller. So if this is bus frequency, use 
'bus-frequency'.

Rob