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;
Date:   Fri, 4 Mar 2022 13:23:59 -0800
From:   Brian Norris <briannorris@chromium.org>
To:     Rob Herring <robh@kernel.org>
Cc:     MyungJoo Ham <myungjoo.ham@samsung.com>,
        Kyungmin Park <kyungmin.park@samsung.com>,
        Chanwoo Choi <cw00.choi@samsung.com>,
        linux-kernel@vger.kernel.org, linux-rockchip@lists.infradead.org,
        Lin Huang <hl@rock-chips.com>,
        linux-arm-kernel@lists.infradead.org,
        Derek Basehore <dbasehore@chromium.org>,
        devicetree@vger.kernel.org, linux-pm@vger.kernel.org,
        Heiko Stuebner <heiko@sntech.de>
Subject: Re: [PATCH v2 01/15] dt-bindings: devfreq: rk3399_dmc: Convert to
 YAML
Message-ID: <YiKDb6q5k4NHpyVn@google.com>
References: <20220127230727.3369358-1-briannorris@chromium.org>
 <20220127150615.v2.1.I875ab8f28c5155a7d2f103316191954d4b07ac13@changeid>
 <YgQhXb/0BM0OFy/6@robh.at.kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YgQhXb/0BM0OFy/6@robh.at.kernel.org>
Precedence: bulk

On Wed, Feb 09, 2022 at 02:17:33PM -0600, Rob Herring wrote:
> On Thu, Jan 27, 2022 at 03:07:12PM -0800, Brian Norris wrote:
> > I want to add, deprecate, and bugfix some properties, as well as add the
> > first users. This is easier with a proper schema.
> > 
> > The transformation is mostly straightforward, plus a few notable tweaks:
> > 
> >  * Renamed rockchip,dram_speed_bin to rockchip,ddr3_speed_bin. The
> >    driver code and the example matched, but the description was
> >    different. I went with the implementation.

...

> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml
> > @@ -0,0 +1,293 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +# %YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/devfreq/rk3399_dmc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Rockchip rk3399 DMC (Dynamic Memory Controller) device
> > +
> > +maintainers:
> > +  - Brian Norris <briannorris@chromium.org>
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - rockchip,rk3399-dmc
> > +
> > +  devfreq-events:
> > +    $ref: /schemas/types.yaml#/definitions/phandle-array
> > +    minItems: 1
> 
> What's the max?
> 
> If this is just phandles (no arg cells), then you need:
> 
> items:
>   maxItems: 1
> 
> IOW, fully describe the number of entries and cells for each entry.

We only need 1, with no args. Will add |maxItems|.

> > +    description:
> > +      Node to get DDR loading. Refer to
> > +      Documentation/devicetree/bindings/devfreq/event/rockchip-dfi.txt.
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  clock-names:
> > +    items:
> > +      - const: dmc_clk
> > +
> > +  operating-points-v2: true
> > +
> > +  center-supply:
> > +    description:
> > +      DMC regulator supply.
> > +
> > +  rockchip,pmu:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description:
> > +      Phandle to the syscon managing the "PMU general register files".
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +    description:
> > +      The CPU interrupt number. It should be a DCF interrupt. When DDR DVFS
> > +      finishes, a DCF interrupt is triggered.
> > +
> > +  rockchip,ddr3_speed_bin:
> 
> Since you are changing this, s/_/-/

I'm only including this because the driver already supports the
rockchip,ddr3_speed_bin spelling. But I'm also deprecating it (because
it's not really needed) and removing it later in the series. I'd rather
not change the spelling again in the middle, when it doesn't really have
any net effect.

I can add some clarifying notes in the commit message, about impending
deprecations, so this makes a little more sense as a standalone commit.

Or if it's somehow better, I can just drop the to-be-deprecated
properties right now in the .yaml conversion? As it happens, I've seen
at least one (probably more) other YAML conversion that made breaking
changes at the same time...

> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description:
> > +      For values, reference include/dt-bindings/clock/rk3399-ddr.h. Selects the
> > +      DDR3 cl-trp-trcd type. It must be set according to "Speed Bin" in DDR3
> > +      datasheet; DO NOT use a smaller "Speed Bin" than specified for the DDR3
> > +      being used.

Brian