On 12/12/2024 09:59, Crystal Guo wrote: > Add devicetree binding for mediatek common-dramc driver. > > The DRAM controller of MediaTek SoC provides an interface to > get the current data rate of DRAM. Bindings are before users. A nit, subject: drop second/last, redundant "dt-bindings". The "dt-bindings" prefix is already stating that these are bindings. See also: https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18 > > Signed-off-by: Crystal Guo <crystal.guo@xxxxxxxxxxxx> > --- > .../mediatek,common-dramc.yaml | 129 ++++++++++++++++++ > 1 file changed, 129 insertions(+) > create mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,common-dramc.yaml > > diff --git a/Documentation/devicetree/bindings/memory-controllers/mediatek,common-dramc.yaml b/Documentation/devicetree/bindings/memory-controllers/mediatek,common-dramc.yaml > new file mode 100644 > index 000000000000..c9e608c7f183 > --- /dev/null > +++ b/Documentation/devicetree/bindings/memory-controllers/mediatek,common-dramc.yaml > @@ -0,0 +1,129 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +# Copyright (c) 2024 MediaTek Inc. > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/memory-controllers/mediatek,common-dramc.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: MediaTek Common DRAMC (DRAM Controller) Common? Is this a real thing? Please describe the hardware. > + > +maintainers: > + - Crystal Guo <crystal.guo@xxxxxxxxxxxx> > + > +description: | Do not need '|' unless you need to preserve formatting. > + The DRAM controller of MediaTek SoC provides an interface to > + get the current data rate of DRAM. So not common here? > + > +properties: > + compatible: > + const: mediatek,common-dramc This has to be SoC. > + > + reg: > + minItems: 9 Why this is flexible? > + items: > + - description: DRAMC_AO_CHA_BASE > + - description: DRAMC_AO_CHB_BASE > + - description: DRAMC_AO_CHC_BASE > + - description: DRAMC_AO_CHD_BASE > + - description: DRAMC_NAO_CHA_BASE > + - description: DRAMC_NAO_CHB_BASE > + - description: DRAMC_NAO_CHC_BASE > + - description: DRAMC_NAO_CHD_BASE > + - description: DDRPHY_AO_CHA_BASE > + - description: DDRPHY_AO_CHB_BASE > + - description: DDRPHY_AO_CHC_BASE > + - description: DDRPHY_AO_CHD_BASE > + - description: DDRPHY_NAO_CHA_BASE > + - description: DDRPHY_NAO_CHB_BASE > + - description: DDRPHY_NAO_CHC_BASE > + - description: DDRPHY_NAO_CHD_BASE > + - description: SLEEP_BASE Don't use some defines. Look at other bindings how they describe items. > + > + support-ch-cnt: Nope > + maxItems: 1 > + > + fmeter-version: > + maxItems: 1 > + description: > + Fmeter version for calculating dram data rate > + > + crystal-freq: > + maxItems: 1 > + description: > + Reference clock rate in MHz > + > + shu-of: > + maxItems: 1 > + > + pll-id: true > + shu-lv: true > + sdmpcw: true > + posdiv: true > + fbksel: true > + dqsopen: true > + async-ca: true > + dq-ser-mode: true This binding is terrible. Was not tested and does not follow any guidelines. Please read example schema and writing bindings document. You can also read slides from my talks... > + > +required: > + - compatible > + - reg > + - support-ch-cnt > + - fmeter-version > + - crystal-freq > + - pll-id > + - shu-lv > + - shu-of > + - sdmpcw > + - posdiv > + - fbksel > + - dqsopen > + - async-ca > + - dq-ser-mode > + > +additionalProperties: false > + > +examples: > + - | > + soc { > + #address-cells = <2>; > + #size-cells = <2>; > + > + dramc: dramc@10230000 { memory-controller@ and drop unused label. Best regards, Krzysztof
