Re: [PATCH v5 5/9] dt-bindings: fsi: Document the FSI controller common properties

[Rob Herring]

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@xxxxxxxxxxxxx>
> > > > > ---
> > > > > 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@xxxxxxxxxxxxx>
> > > > > +
> > > > > +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