| Message ID | 20241212090029.13692-3-crystal.guo@mediatek.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add MediaTek DRAMC driver support | expand |
On Thu, 12 Dec 2024 16:59:48 +0800, 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. > > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > --- > .../mediatek,common-dramc.yaml | 129 ++++++++++++++++++ > 1 file changed, 129 insertions(+) > create mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,common-dramc.yaml > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/memory-controllers/mediatek,common-dramc.yaml: support-ch-cnt: missing type definition /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/memory-controllers/mediatek,common-dramc.yaml: fmeter-version: missing type definition /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/memory-controllers/mediatek,common-dramc.yaml: crystal-freq: missing type definition /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/memory-controllers/mediatek,common-dramc.yaml: shu-of: missing type definition /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/memory-controllers/mediatek,common-dramc.yaml: pll-id: missing type definition /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/memory-controllers/mediatek,common-dramc.yaml: shu-lv: missing type definition /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/memory-controllers/mediatek,common-dramc.yaml: sdmpcw: missing type definition /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/memory-controllers/mediatek,common-dramc.yaml: posdiv: missing type definition /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/memory-controllers/mediatek,common-dramc.yaml: fbksel: missing type definition /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/memory-controllers/mediatek,common-dramc.yaml: dqsopen: missing type definition /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/memory-controllers/mediatek,common-dramc.yaml: async-ca: missing type definition /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/memory-controllers/mediatek,common-dramc.yaml: dq-ser-mode: missing type definition doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241212090029.13692-3-crystal.guo@mediatek.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
Il 12/12/24 09:59, Crystal Guo ha scritto: > 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. > > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> > --- > .../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) MediaTek DRAM Controller (DRAMC) > + > +maintainers: > + - Crystal Guo <crystal.guo@mediatek.com> > + > +description: | > + The DRAM controller of MediaTek SoC provides an interface to > + get the current data rate of DRAM. No, the DRAM Controller does much more than just that. > + > +properties: > + compatible: > + const: mediatek,common-dramc Absolutely no! Compatibles are per-soc. mediatek,mt8186-dramc mediatek,mt8188-dramc mediatek,mt8195-dramc etc > + > + reg: > + minItems: 9 > + items: > + - description: DRAMC_AO_CHA_BASE > + - description: DRAMC_AO_CHB_BASE > + - description: DRAMC_AO_CHC_BASE > + - description: DRAMC_AO_CHD_BASE All those channels are sequential in AO->NAO, in the sense that every channel is: CH0 AO: 0x10230000 len: 0x4000 CH0 NAO: 0x10234000 len: 0x2000 CH0 PHY_AO: 0x10236000 len: 0x2000 CH0 PHY_AO: 0x10238000 len: 0x2000 So the reg can be simplified as minItems: 4 items: - description: DRAM Controller Channel 0 - description: DRAM Controller Channel 1 - description: DRAM Controller Channel 2 - description: DRAM Controller Channel 3 > + - 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 You're not using the SLEEP_BASE iospace, and that's not even really specific to the DRAM Controller. Drop it. > + > + support-ch-cnt: > + maxItems: 1 Don't tell me that the DRAM Controller in MediaTek SoCs cannot see how many channels are actually occupied by a DRAM bank, because I will be really skeptical. You can autodetect that in the driver, you don't need a DT property for that. > + > + fmeter-version: > + maxItems: 1 > + description: > + Fmeter version for calculating dram data rate The Fmeter version is SoC-specific, you need platform data, not DT property. > + > + crystal-freq: > + maxItems: 1 > + description: > + Reference clock rate in MHz Is this crystal an external component, or is it integrated into the SoC? > + > + shu-of: > + maxItems: 1 There's no description, what is shu-of? > + > + pll-id: true > + shu-lv: true > + sdmpcw: true > + posdiv: true > + fbksel: true > + dqsopen: true > + async-ca: true > + dq-ser-mode: true Same for these ones, please describe them - but then remember: if those parameters are board-specific, they can stay here, otherwise those go in platform data. Besides, I doubt that those are board specific. Regards, Angelo
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@mediatek.com> > --- > .../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@mediatek.com> > + > +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
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) + +maintainers: + - Crystal Guo <crystal.guo@mediatek.com> + +description: | + The DRAM controller of MediaTek SoC provides an interface to + get the current data rate of DRAM. + +properties: + compatible: + const: mediatek,common-dramc + + reg: + minItems: 9 + 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 + + support-ch-cnt: + 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 + +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 { + compatible = "mediatek,common-dramc"; + reg = <0 0x10230000 0 0x2000>, /* DRAMC_AO_CHA_BASE */ + <0 0x10240000 0 0x2000>, /* DRAMC_AO_CHB_BASE */ + <0 0x10250000 0 0x2000>, /* DRAMC_AO_CHC_BASE */ + <0 0x10260000 0 0x2000>, /* DRAMC_AO_CHD_BASE */ + <0 0x10234000 0 0x1000>, /* DRAMC_NAO_CHA_BASE */ + <0 0x10244000 0 0x1000>, /* DRAMC_NAO_CHB_BASE */ + <0 0x10254000 0 0x1000>, /* DRAMC_NAO_CHC_BASE */ + <0 0x10264000 0 0x1000>, /* DRAMC_NAO_CHD_BASE */ + <0 0x10238000 0 0x2000>, /* DDRPHY_AO_CHA_BASE */ + <0 0x10248000 0 0x2000>, /* DDRPHY_AO_CHB_BASE */ + <0 0x10258000 0 0x2000>, /* DDRPHY_AO_CHC_BASE */ + <0 0x10268000 0 0x2000>, /* DDRPHY_AO_CHD_BASE */ + <0 0x10236000 0 0x2000>, /* DDRPHY_NAO_CHA_BASE */ + <0 0x10246000 0 0x2000>, /* DDRPHY_NAO_CHB_BASE */ + <0 0x10256000 0 0x2000>, /* DDRPHY_NAO_CHC_BASE */ + <0 0x10266000 0 0x2000>, /* DDRPHY_NAO_CHD_BASE */ + <0 0x10006000 0 0x1000>; /* SLEEP_BASE */ + support-ch-cnt = <4>; + fmeter-version = <1>; + crystal-freq = <26>; + pll-id = <0x0e98 0x02000000 25>; + shu-lv = <0x0e98 0x0000c000 14>; + shu-of = <0x700>; + sdmpcw = <0x0908 0x0007fff8 3>, + <0x0928 0x0007fff8 3>; + posdiv = <0x090c 0x00003800 11>, + <0x092c 0x00003800 11>; + fbksel = <0x0910 0x00000040 6>, + <0x0910 0x00000040 6>; + dqsopen = <0x0d94 0x04000000 26>, + <0x0d94 0x04000000 26>; + async-ca = <0x0d08 0x00000001 0>, + <0x0d08 0x00000001 0>; + dq-ser-mode = <0x0dc4 0x00000018 3>, + <0x0dc4 0x00000018 3>; + }; + };
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. Signed-off-by: Crystal Guo <crystal.guo@mediatek.com> --- .../mediatek,common-dramc.yaml | 129 ++++++++++++++++++ 1 file changed, 129 insertions(+) create mode 100644 Documentation/devicetree/bindings/memory-controllers/mediatek,common-dramc.yaml