[v2,1/2] dt-bindings: memory: Add Baikal-T1 L2-cache Control Block binding

Commit Message

Serge Semin May 7, 2020, 11:07 p.m. UTC

There is a single register provided by the SoC system controller,
which can be used to tune the L2-cache RAM up. It only provides a way
to change the L2-RAM access latencies. So aside from "be,bt1-l2-ctl"
compatible string the device node can be optionally equipped with the
properties of Tag/Data/WS latencies.

Signed-off-by: Serge Semin <[e-mail]>
Cc: Alexey Malahov <[e-mail]>
Cc: Paul Burton <[e-mail]>
Cc: Ralf Baechle <[e-mail]>
Cc: Olof Johansson <[e-mail]>
Cc: Boris Brezillon <[e-mail]>
Cc: Paul Cercueil <[e-mail]>
Cc: Thomas Gleixner <[e-mail]>
Cc: Masahiro Yamada <[e-mail]>
Cc: [e-mail]
Cc: [e-mail]

---

Changelog v2:
- Move driver to the memory subsystem.
- Use dual GPL/BSD license.
- Use single lined copyright header.
- Move "allOf" restrictions to the root level of the properties.
- Discard syscon compatible string and reg property.
- The DT node is supposed to be a child of the Baikal-T1 system controller
  node.
---
 .../memory-controllers/baikal,bt1-l2-ctl.yaml | 59 +++++++++++++++++++
 1 file changed, 59 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/baikal,bt1-l2-ctl.yaml

Comments

Rob Herring May 11, 2020, 3:38 p.m. UTC | #1

On Fri, 8 May 2020 02:07:03 +0300, Serge Semin wrote:
> There is a single register provided by the SoC system controller,
> which can be used to tune the L2-cache RAM up. It only provides a way
> to change the L2-RAM access latencies. So aside from "be,bt1-l2-ctl"
> compatible string the device node can be optionally equipped with the
> properties of Tag/Data/WS latencies.
> 
> Signed-off-by: Serge Semin <[e-mail]>
> Cc: Alexey Malahov <[e-mail]>
> Cc: Paul Burton <[e-mail]>
> Cc: Ralf Baechle <[e-mail]>
> Cc: Olof Johansson <[e-mail]>
> Cc: Boris Brezillon <[e-mail]>
> Cc: Paul Cercueil <[e-mail]>
> Cc: Thomas Gleixner <[e-mail]>
> Cc: Masahiro Yamada <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]
> 
> ---
> 
> Changelog v2:
> - Move driver to the memory subsystem.
> - Use dual GPL/BSD license.
> - Use single lined copyright header.
> - Move "allOf" restrictions to the root level of the properties.
> - Discard syscon compatible string and reg property.
> - The DT node is supposed to be a child of the Baikal-T1 system controller
>   node.
> ---
>  .../memory-controllers/baikal,bt1-l2-ctl.yaml | 59 +++++++++++++++++++
>  1 file changed, 59 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/memory-controllers/baikal,bt1-l2-ctl.yaml
> 


My bot found errors running 'make dt_binding_check' on your patch:

/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/memory-controllers/baikal,bt1-l2-ctl.example.dt.yaml: l2_ctl: 'baikal,l2-data-latency', 'baikal,l2-tag-latency', 'baikal,l2-ws-latency' do not match any of the regexes: '^#.*', '^(at25|devbus|dmacap|dsa|exynos|fsi[ab]|gpio-fan|gpio|gpmc|hdmi|i2c-gpio),.*', '^(keypad|m25p|max8952|max8997|max8998|mpmc),.*', '^(pinctrl-single|#pinctrl-single|PowerPC),.*', '^(pl022|pxa-mmc|rcar_sound|rotary-encoder|s5m8767|sdhci),.*', '^(simple-audio-card|simple-graph-card|st-plgpio|st-spics|ts),.*', '^GEFanuc,.*', '^ORCL,.*', '^SUNW,.*', '^[a-zA-Z0-9#_][a-zA-Z0-9+\\-._@]{0,63}$', '^[a-zA-Z0-9+\\-._]*@[0-9a-zA-Z,]*$', '^abilis,.*', '^abracon,.*', '^acme,.*', '^actions,.*', '^active-semi,.*', '^ad,.*', '^adafruit,.*', '^adapteva,.*', '^adaptrum,.*', '^adh,.*', '^adi,.*', '^advantech,.*', '^aeroflexgaisler,.*', '^al,.*', '^allegro,.*', '^allo,.*', '^allwinner,.*', '^alphascale,.*', '^altr,.*', '^amarula,.*', '^amazon,.*', '^amcc,.*', '^amd,.*', '^amediatech,.*', '^amlogic,.*', '^ampire,.*', '^ams,.*', '^amstaos,.*', '^analogix,.*', '^andestech,.*', '^anvo,.*', '^apm,.*', '^aptina,.*', '^arasan,.*', '^archermind,.*', '^arctic,.*', '^arcx,.*', '^aries,.*', '^arm,.*', '^armadeus,.*', '^arrow,.*', '^artesyn,.*', '^asahi-kasei,.*', '^asc,.*', '^aspeed,.*', '^asus,.*', '^atlas,.*', '^atmel,.*', '^auo,.*', '^auvidea,.*', '^avago,.*', '^avia,.*', '^avic,.*', '^avnet,.*', '^axentia,.*', '^axis,.*', '^azoteq,.*', '^azw,.*', '^bananapi,.*', '^beacon,.*', '^bhf,.*', '^bitmain,.*', '^boe,.*', '^bosch,.*', '^boundary,.*', '^brcm,.*', '^broadmobi,.*', '^bticino,.*', '^buffalo,.*', '^bur,.*', '^calaosystems,.*', '^calxeda,.*', '^capella,.*', '^cascoda,.*', '^catalyst,.*', '^cavium,.*', '^cdns,.*', '^cdtech,.*', '^ceva,.*', '^chipidea,.*', '^chipone,.*', '^chipspark,.*', '^chrp,.*', '^chunghwa,.*', '^chuwi,.*', '^ciaa,.*', '^cirrus,.*', '^cloudengines,.*', '^cnm,.*', '^cnxt,.*', '^colorfly,.*', '^compulab,.*', '^coreriver,.*', '^corpro,.*', '^cortina,.*', '^cosmic,.*', '^crane,.*', '^creative,.*', '^crystalfontz,.*', '^csky,.*', '^csq,.*', '^cubietech,.*', '^cypress,.*', '^cznic,.*', '^dallas,.*', '^dataimage,.*', '^davicom,.*', '^dell,.*', '^delta,.*', '^denx,.*', '^devantech,.*', '^dh,.*', '^difrnce,.*', '^digi,.*', '^digilent,.*', '^dioo,.*', '^dlc,.*', '^dlg,.*', '^dlink,.*', '^dmo,.*', '^domintech,.*', '^dongwoon,.*', '^dptechnics,.*', '^dragino,.*', '^dserve,.*', '^dynaimage,.*', '^ea,.*', '^ebs-systart,.*', '^ebv,.*', '^eckelmann,.*', '^edt,.*', '^eeti,.*', '^einfochips,.*', '^elan,.*', '^elgin,.*', '^elida,.*', '^embest,.*', '^emlid,.*', '^emmicro,.*', '^empire-electronix,.*', '^emtrion,.*', '^endless,.*', '^ene,.*', '^energymicro,.*', '^engicam,.*', '^epcos,.*', '^epfl,.*', '^epson,.*', '^esp,.*', '^est,.*', '^ettus,.*', '^eukrea,.*', '^everest,.*', '^everspin,.*', '^evervision,.*', '^exar,.*', '^excito,.*', '^ezchip,.*', '^facebook,.*', '^fairphone,.*', '^faraday,.*', '^fastrax,.*', '^fcs,.*', '^feixin,.*', '^feiyang,.*', '^firefly,.*', '^focaltech,.*', '^frida,.*', '^friendlyarm,.*', '^fsl,.*', '^fujitsu,.*', '^gardena,.*', '^gateworks,.*', '^gcw,.*', '^ge,.*', '^geekbuying,.*', '^gef,.*', '^gemei,.*', '^geniatech,.*', '^giantec,.*', '^giantplus,.*', '^globalscale,.*', '^globaltop,.*', '^gmt,.*', '^goodix,.*', '^google,.*', '^grinn,.*', '^grmn,.*', '^gumstix,.*', '^gw,.*', '^hannstar,.*', '^haoyu,.*', '^hardkernel,.*', '^hideep,.*', '^himax,.*', '^hisilicon,.*', '^hit,.*', '^hitex,.*', '^holt,.*', '^holtek,.*', '^honeywell,.*', '^hoperun,.*', '^hp,.*', '^hsg,.*', '^hugsun,.*', '^hwacom,.*', '^hydis,.*', '^hyundai,.*', '^i2se,.*', '^ibm,.*', '^icplus,.*', '^idt,.*', '^ifi,.*', '^ilitek,.*', '^img,.*', '^incircuit,.*', '^inet-tek,.*', '^infineon,.*', '^inforce,.*', '^ingenic,.*', '^innolux,.*', '^inside-secure,.*', '^inspur,.*', '^intel,.*', '^intercontrol,.*', '^invensense,.*', '^inversepath,.*', '^iom,.*', '^isee,.*', '^isil,.*', '^issi,.*', '^ite,.*', '^itead,.*', '^iwave,.*', '^jdi,.*', '^jedec,.*', '^jesurun,.*', '^jianda,.*', '^kam,.*', '^karo,.*', '^keithkoep,.*', '^keymile,.*', '^khadas,.*', '^kiebackpeter,.*', '^kinetic,.*', '^kingdisplay,.*', '^kingnovel,.*', '^kionix,.*', '^kobo,.*', '^koe,.*', '^kontron,.*', '^kosagi,.*', '^kyo,.*', '^lacie,.*', '^laird,.*', '^lamobo,.*', '^lantiq,.*', '^lattice,.*', '^leadtek,.*', '^leez,.*', '^lego,.*', '^lemaker,.*', '^lenovo,.*', '^lg,.*', '^lgphilips,.*', '^libretech,.*', '^licheepi,.*', '^linaro,.*', '^linksprite,.*', '^linksys,.*', '^linutronix,.*', '^linux,.*', '^linx,.*', '^lltc,.*', '^logicpd,.*', '^logictechno,.*', '^longcheer,.*', '^loongson,.*', '^lsi,.*', '^lwn,.*', '^macnica,.*', '^mapleboard,.*', '^marvell,.*', '^maxbotix,.*', '^maxim,.*', '^mbvl,.*', '^mcube,.*', '^meas,.*', '^mecer,.*', '^mediatek,.*', '^megachips,.*', '^mele,.*', '^melexis,.*', '^melfas,.*', '^mellanox,.*', '^memsic,.*', '^menlo,.*', '^merrii,.*', '^micrel,.*', '^microchip,.*', '^microcrystal,.*', '^micron,.*', '^microsoft,.*', '^mikroe,.*', '^miniand,.*', '^minix,.*', '^miramems,.*', '^mitsubishi,.*', '^mosaixtech,.*', '^motorola,.*', '^moxa,.*', '^mpl,.*', '^mps,.*', '^mqmaker,.*', '^mrvl,.*', '^mscc,.*', '^msi,.*', '^mti,.*', '^multi-inno,.*', '^mundoreader,.*', '^murata,.*', '^mxicy,.*', '^myir,.*', '^national,.*', '^nec,.*', '^neonode,.*', '^netgear,.*', '^netlogic,.*', '^netron-dy,.*', '^netxeon,.*', '^neweast,.*', '^newhaven,.*', '^nexbox,.*', '^nextthing,.*', '^ni,.*', '^nintendo,.*', '^nlt,.*', '^nokia,.*', '^nordic,.*', '^novtech,.*', '^nutsboard,.*', '^nuvoton,.*', '^nvd,.*', '^nvidia,.*', '^nxp,.*', '^oceanic,.*', '^okaya,.*', '^oki,.*', '^olimex,.*', '^olpc,.*', '^onion,.*', '^onnn,.*', '^ontat,.*', '^opalkelly,.*', '^opencores,.*', '^openrisc,.*', '^option,.*', '^oranth,.*', '^orisetech,.*', '^ortustech,.*', '^osddisplays,.*', '^overkiz,.*', '^ovti,.*', '^oxsemi,.*', '^ozzmaker,.*', '^panasonic,.*', '^parade,.*', '^parallax,.*', '^pda,.*', '^pericom,.*', '^pervasive,.*', '^phicomm,.*', '^phytec,.*', '^picochip,.*', '^pine64,.*', '^pineriver,.*', '^pixcir,.*', '^plantower,.*', '^plathome,.*', '^plda,.*', '^plx,.*', '^pni,.*', '^pocketbook,.*', '^polaroid,.*', '^portwell,.*', '^poslab,.*', '^pov,.*', '^powervr,.*', '^primux,.*', '^probox2,.*', '^pulsedlight,.*', '^purism,.*', '^qca,.*', '^qcom,.*', '^qemu,.*', '^qi,.*', '^qiaodian,.*', '^qihua,.*', '^qnap,.*', '^radxa,.*', '^raidsonic,.*', '^ralink,.*', '^ramtron,.*', '^raspberrypi,.*', '^raydium,.*', '^rda,.*', '^realtek,.*', '^renesas,.*', '^rervision,.*', '^richtek,.*', '^ricoh,.*', '^rikomagic,.*', '^riscv,.*', '^rockchip,.*', '^rocktech,.*', '^rohm,.*', '^ronbo,.*', '^roofull,.*', '^samsung,.*', '^samtec,.*', '^sancloud,.*', '^sandisk,.*', '^satoz,.*', '^sbs,.*', '^schindler,.*', '^seagate,.*', '^seirobotics,.*', '^semtech,.*', '^sensirion,.*', '^sensortek,.*', '^sff,.*', '^sgd,.*', '^sgmicro,.*', '^sgx,.*', '^sharp,.*', '^shimafuji,.*', '^si-en,.*', '^si-linux,.*', '^sifive,.*', '^sigma,.*', '^sii,.*', '^sil,.*', '^silabs,.*', '^silead,.*', '^silergy,.*', '^siliconmitus,.*', '^simtek,.*', '^sinlinx,.*', '^sinovoip,.*', '^sipeed,.*', '^sirf,.*', '^sis,.*', '^sitronix,.*', '^skyworks,.*', '^smsc,.*', '^snps,.*', '^sochip,.*', '^socionext,.*', '^solidrun,.*', '^solomon,.*', '^sony,.*', '^spansion,.*', '^sprd,.*', '^sst,.*', '^st,.*', '^st-ericsson,.*', '^starry,.*', '^startek,.*', '^ste,.*', '^stericsson,.*', '^summit,.*', '^sunchip,.*', '^swir,.*', '^syna,.*', '^synology,.*', '^tbs,.*', '^tbs-biometrics,.*', '^tcg,.*', '^tcl,.*', '^technexion,.*', '^technologic,.*', '^techstar,.*', '^tempo,.*', '^terasic,.*', '^tfc,.*', '^thine,.*', '^ti,.*', '^tianma,.*', '^tlm,.*', '^tmt,.*', '^topeet,.*', '^toppoly,.*', '^topwise,.*', '^toradex,.*', '^toshiba,.*', '^toumaz,.*', '^tpk,.*', '^tplink,.*', '^tpo,.*', '^tq,.*', '^tronfy,.*', '^tronsmart,.*', '^truly,.*', '^tsd,.*', '^tyan,.*', '^u-blox,.*', '^ubnt,.*', '^ucrobotics,.*', '^udoo,.*', '^ugoos,.*', '^uniwest,.*', '^upisemi,.*', '^urt,.*', '^usi,.*', '^utoo,.*', '^v3,.*', '^vamrs,.*', '^variscite,.*', '^via,.*', '^videostrong,.*', '^virtio,.*', '^vishay,.*', '^vitesse,.*', '^vivante,.*', '^vocore,.*', '^voipac,.*', '^vot,.*', '^vxt,.*', '^waveshare,.*', '^wd,.*', '^wetek,.*', '^wexler,.*', '^whwave,.*', '^wi2wi,.*', '^winbond,.*', '^winstar,.*', '^wits,.*', '^wlf,.*', '^wm,.*', '^wobo,.*', '^x-powers,.*', '^xes,.*', '^xiaomi,.*', '^xillybus,.*', '^xinpeng,.*', '^xlnx,.*', '^xunlong,.*', '^xylon,.*', '^yna,.*', '^yones-toptech,.*', '^ysoft,.*', '^zarlink,.*', '^zeitec,.*', '^zidoo,.*', '^zii,.*', '^zte,.*', '^zyxel,.*'

See https://patchwork.ozlabs.org/patch/1285665

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure dt-schema is up to date:

pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade

Please check and re-submit.

Serge Semin May 11, 2020, 8:25 p.m. UTC | #2

On Mon, May 11, 2020 at 10:38:04AM -0500, Rob Herring wrote:
> On Fri, 8 May 2020 02:07:03 +0300, Serge Semin wrote:
> > There is a single register provided by the SoC system controller,
> > which can be used to tune the L2-cache RAM up. It only provides a way
> > to change the L2-RAM access latencies. So aside from "be,bt1-l2-ctl"
> > compatible string the device node can be optionally equipped with the
> > properties of Tag/Data/WS latencies.
> > 
> > Signed-off-by: Serge Semin <[e-mail]>
> > Cc: Alexey Malahov <[e-mail]>
> > Cc: Paul Burton <[e-mail]>
> > Cc: Ralf Baechle <[e-mail]>
> > Cc: Olof Johansson <[e-mail]>
> > Cc: Boris Brezillon <[e-mail]>
> > Cc: Paul Cercueil <[e-mail]>
> > Cc: Thomas Gleixner <[e-mail]>
> > Cc: Masahiro Yamada <[e-mail]>
> > Cc: [e-mail]
> > Cc: [e-mail]
> > 
> > ---
> > 
> > Changelog v2:
> > - Move driver to the memory subsystem.
> > - Use dual GPL/BSD license.
> > - Use single lined copyright header.
> > - Move "allOf" restrictions to the root level of the properties.
> > - Discard syscon compatible string and reg property.
> > - The DT node is supposed to be a child of the Baikal-T1 system controller
> >   node.
> > ---
> >  .../memory-controllers/baikal,bt1-l2-ctl.yaml | 59 +++++++++++++++++++
> >  1 file changed, 59 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/memory-controllers/baikal,bt1-l2-ctl.yaml
> > 
> 
> 
> My bot found errors running 'make dt_binding_check' on your patch:
> 
> [nip] ...
>
> See https://patchwork.ozlabs.org/patch/1285665
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure dt-schema is up to date:
> 
> pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade
> 
> Please check and re-submit.
> 

The problem is due to an absent vendor prefix in the test kernel source tree
environment. As I said in the cover-letter the new vendor prefix will be added
in the framework of the next patchset:
https://lkml.org/lkml/2020/5/6/1047

Rob, please review that patchset first, merge in the corresponding patch from
there and test this binding out then.

-Sergey

Rob Herring May 11, 2020, 10:37 p.m. UTC | #3

On Mon, May 11, 2020 at 3:25 PM Serge Semin
<[e-mail]> wrote:
>
> On Mon, May 11, 2020 at 10:38:04AM -0500, Rob Herring wrote:
> > On Fri, 8 May 2020 02:07:03 +0300, Serge Semin wrote:
> > > There is a single register provided by the SoC system controller,
> > > which can be used to tune the L2-cache RAM up. It only provides a way
> > > to change the L2-RAM access latencies. So aside from "be,bt1-l2-ctl"
> > > compatible string the device node can be optionally equipped with the
> > > properties of Tag/Data/WS latencies.
> > >
> > > Signed-off-by: Serge Semin <[e-mail]>
> > > Cc: Alexey Malahov <[e-mail]>
> > > Cc: Paul Burton <[e-mail]>
> > > Cc: Ralf Baechle <[e-mail]>
> > > Cc: Olof Johansson <[e-mail]>
> > > Cc: Boris Brezillon <[e-mail]>
> > > Cc: Paul Cercueil <[e-mail]>
> > > Cc: Thomas Gleixner <[e-mail]>
> > > Cc: Masahiro Yamada <[e-mail]>
> > > Cc: [e-mail]
> > > Cc: [e-mail]
> > >
> > > ---
> > >
> > > Changelog v2:
> > > - Move driver to the memory subsystem.
> > > - Use dual GPL/BSD license.
> > > - Use single lined copyright header.
> > > - Move "allOf" restrictions to the root level of the properties.
> > > - Discard syscon compatible string and reg property.
> > > - The DT node is supposed to be a child of the Baikal-T1 system controller
> > >   node.
> > > ---
> > >  .../memory-controllers/baikal,bt1-l2-ctl.yaml | 59 +++++++++++++++++++
> > >  1 file changed, 59 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/memory-controllers/baikal,bt1-l2-ctl.yaml
> > >
> >
> >
> > My bot found errors running 'make dt_binding_check' on your patch:
> >
> > [nip] ...
> >
> > See https://patchwork.ozlabs.org/patch/1285665
> >
> > If you already ran 'make dt_binding_check' and didn't see the above
> > error(s), then make sure dt-schema is up to date:
> >
> > pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade
> >
> > Please check and re-submit.
> >
>
> The problem is due to an absent vendor prefix in the test kernel source tree
> environment. As I said in the cover-letter the new vendor prefix will be added
> in the framework of the next patchset:
> https://lkml.org/lkml/2020/5/6/1047
>
> Rob, please review that patchset first, merge in the corresponding patch from
> there and test this binding out then.

Did you read the part about a 'bot'? My bot doesn't read cover letters
and I only occasionally do. Do you want to write me a script that can
do this?

Rob

Rob Herring May 11, 2020, 10:43 p.m. UTC | #4

On Thu, May 7, 2020 at 6:07 PM Serge Semin
<[e-mail]> wrote:
>
> There is a single register provided by the SoC system controller,
> which can be used to tune the L2-cache RAM up. It only provides a way
> to change the L2-RAM access latencies. So aside from "be,bt1-l2-ctl"
> compatible string the device node can be optionally equipped with the
> properties of Tag/Data/WS latencies.
>
> Signed-off-by: Serge Semin <[e-mail]>
> Cc: Alexey Malahov <[e-mail]>
> Cc: Paul Burton <[e-mail]>
> Cc: Ralf Baechle <[e-mail]>
> Cc: Olof Johansson <[e-mail]>
> Cc: Boris Brezillon <[e-mail]>
> Cc: Paul Cercueil <[e-mail]>
> Cc: Thomas Gleixner <[e-mail]>
> Cc: Masahiro Yamada <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]
>
> ---
>
> Changelog v2:
> - Move driver to the memory subsystem.
> - Use dual GPL/BSD license.
> - Use single lined copyright header.
> - Move "allOf" restrictions to the root level of the properties.
> - Discard syscon compatible string and reg property.
> - The DT node is supposed to be a child of the Baikal-T1 system controller
>   node.
> ---
>  .../memory-controllers/baikal,bt1-l2-ctl.yaml | 59 +++++++++++++++++++
>  1 file changed, 59 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/memory-controllers/baikal,bt1-l2-ctl.yaml
>
> diff --git a/Documentation/devicetree/bindings/memory-controllers/baikal,bt1-l2-ctl.yaml b/Documentation/devicetree/bindings/memory-controllers/baikal,bt1-l2-ctl.yaml
> new file mode 100644
> index 000000000000..263f0cdab4e6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/memory-controllers/baikal,bt1-l2-ctl.yaml
> @@ -0,0 +1,59 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2020 BAIKAL ELECTRONICS, JSC
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/memory-controllers/baikal,bt1-l2-ctl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Baikal-T1 L2-cache Control Block
> +
> +maintainers:
> +  - Serge Semin <[e-mail]>
> +
> +description: |
> +  By means of the System Controller Baikal-T1 SoC exposes a few settings to
> +  tune the MIPS P5600 CM2 L2 cache performance up. In particular it's possible
> +  to change the Tag, Data and Way-select RAM access latencies. Baikal-T1
> +  L2-cache controller block is responsible for the tuning. Its DT node is
> +  supposed to be a child of the system controller.

Is there a register range for just the L2 registers in the system
controller. If so, please add a 'reg' property.

This should all be part of the system controller schema either as 1
file or by a $ref from the system controller to this file. That's how
we ensure "supposed to be a child of the system controller".

> +
> +properties:
> +  compatible:
> +    const: baikal,bt1-l2-ctl
> +
> +  baikal,l2-ws-latency:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Cycles of latency for Way-select RAM accesses
> +    default: 0
> +    minimum: 0
> +    maximum: 3
> +
> +  baikal,l2-tag-latency:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Cycles of latency for Tag RAM accesses
> +    default: 0
> +    minimum: 0
> +    maximum: 3
> +
> +  baikal,l2-data-latency:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Cycles of latency for Data RAM accesses
> +    default: 1
> +    minimum: 0
> +    maximum: 3
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +
> +examples:
> +  - |
> +    l2_ctl {
> +      compatible = "baikal,bt1-l2-ctl";
> +
> +      baikal,l2-ws-latency = <0>;
> +      baikal,l2-tag-latency = <0>;

0 is the default, why list it?

> +      baikal,l2-data-latency = <1>;
> +    };
> +...
> --
> 2.25.1
>

Serge Semin May 12, 2020, 6:25 p.m. UTC | #5

On Mon, May 11, 2020 at 05:43:58PM -0500, Rob Herring wrote:
> On Thu, May 7, 2020 at 6:07 PM Serge Semin
> <[e-mail]> wrote:
> >
> > There is a single register provided by the SoC system controller,
> > which can be used to tune the L2-cache RAM up. It only provides a way
> > to change the L2-RAM access latencies. So aside from "be,bt1-l2-ctl"
> > compatible string the device node can be optionally equipped with the
> > properties of Tag/Data/WS latencies.
> >
> > Signed-off-by: Serge Semin <[e-mail]>
> > Cc: Alexey Malahov <[e-mail]>
> > Cc: Paul Burton <[e-mail]>
> > Cc: Ralf Baechle <[e-mail]>
> > Cc: Olof Johansson <[e-mail]>
> > Cc: Boris Brezillon <[e-mail]>
> > Cc: Paul Cercueil <[e-mail]>
> > Cc: Thomas Gleixner <[e-mail]>
> > Cc: Masahiro Yamada <[e-mail]>
> > Cc: [e-mail]
> > Cc: [e-mail]
> >
> > ---
> >
> > Changelog v2:
> > - Move driver to the memory subsystem.
> > - Use dual GPL/BSD license.
> > - Use single lined copyright header.
> > - Move "allOf" restrictions to the root level of the properties.
> > - Discard syscon compatible string and reg property.
> > - The DT node is supposed to be a child of the Baikal-T1 system controller
> >   node.
> > ---
> >  .../memory-controllers/baikal,bt1-l2-ctl.yaml | 59 +++++++++++++++++++
> >  1 file changed, 59 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/memory-controllers/baikal,bt1-l2-ctl.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/memory-controllers/baikal,bt1-l2-ctl.yaml b/Documentation/devicetree/bindings/memory-controllers/baikal,bt1-l2-ctl.yaml
> > new file mode 100644
> > index 000000000000..263f0cdab4e6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/memory-controllers/baikal,bt1-l2-ctl.yaml
> > @@ -0,0 +1,59 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +# Copyright (C) 2020 BAIKAL ELECTRONICS, JSC
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/memory-controllers/baikal,bt1-l2-ctl.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Baikal-T1 L2-cache Control Block
> > +
> > +maintainers:
> > +  - Serge Semin <[e-mail]>
> > +
> > +description: |
> > +  By means of the System Controller Baikal-T1 SoC exposes a few settings to
> > +  tune the MIPS P5600 CM2 L2 cache performance up. In particular it's possible
> > +  to change the Tag, Data and Way-select RAM access latencies. Baikal-T1
> > +  L2-cache controller block is responsible for the tuning. Its DT node is
> > +  supposed to be a child of the system controller.
> 
> Is there a register range for just the L2 registers in the system
> controller. If so, please add a 'reg' property.

It's just a single register, though almost fully dedicated for this feature.
Should I add the reg property anyway? Since you touched this topic, aside from
this l2-control block the system controller has also got sub-blocks of PLLs, clock
dividers, reboot, reboot-mode and indirectly addressed i2c in the same MMIO space.
These blocks all have got a dedicated registers range within the syscon regmap
space. Shall I add an optional reg property for them too? If so shall their node
names to be in the regexp-format like "^name(@[0-9a-f]+)?" ?

> 
> This should all be part of the system controller schema either as 1
> file or by a $ref from the system controller to this file. That's how
> we ensure "supposed to be a child of the system controller".

Oh, that's clever solution. I was thinking of how to signify this parent-child
dependency. I'll add the $ref in the corresponding properties of the system
controller. So this DT schema should live here, separately from the syscon DT
node. Thanks for the note.

> 
> > +
> > +properties:
> > +  compatible:
> > +    const: baikal,bt1-l2-ctl
> > +
> > +  baikal,l2-ws-latency:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: Cycles of latency for Way-select RAM accesses
> > +    default: 0
> > +    minimum: 0
> > +    maximum: 3
> > +
> > +  baikal,l2-tag-latency:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: Cycles of latency for Tag RAM accesses
> > +    default: 0
> > +    minimum: 0
> > +    maximum: 3
> > +
> > +  baikal,l2-data-latency:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: Cycles of latency for Data RAM accesses
> > +    default: 1
> > +    minimum: 0
> > +    maximum: 3
> > +
> > +additionalProperties: false
> > +
> > +required:
> > +  - compatible
> > +
> > +examples:
> > +  - |
> > +    l2_ctl {
> > +      compatible = "baikal,bt1-l2-ctl";
> > +
> > +      baikal,l2-ws-latency = <0>;
> > +      baikal,l2-tag-latency = <0>;
> 
> 0 is the default, why list it?

1 is the default for the l2-data-latency too. Why not? It's just an
example.

-Sergey

> 
> > +      baikal,l2-data-latency = <1>;
> > +    };
> > +...
> > --
> > 2.25.1
> >

Serge Semin May 12, 2020, 6:31 p.m. UTC | #6

On Mon, May 11, 2020 at 05:37:41PM -0500, Rob Herring wrote:
> On Mon, May 11, 2020 at 3:25 PM Serge Semin
> <[e-mail]> wrote:
> >
> > On Mon, May 11, 2020 at 10:38:04AM -0500, Rob Herring wrote:
> > > On Fri, 8 May 2020 02:07:03 +0300, Serge Semin wrote:
> > > > There is a single register provided by the SoC system controller,
> > > > which can be used to tune the L2-cache RAM up. It only provides a way
> > > > to change the L2-RAM access latencies. So aside from "be,bt1-l2-ctl"
> > > > compatible string the device node can be optionally equipped with the
> > > > properties of Tag/Data/WS latencies.
> > > >
> > > > Signed-off-by: Serge Semin <[e-mail]>
> > > > Cc: Alexey Malahov <[e-mail]>
> > > > Cc: Paul Burton <[e-mail]>
> > > > Cc: Ralf Baechle <[e-mail]>
> > > > Cc: Olof Johansson <[e-mail]>
> > > > Cc: Boris Brezillon <[e-mail]>
> > > > Cc: Paul Cercueil <[e-mail]>
> > > > Cc: Thomas Gleixner <[e-mail]>
> > > > Cc: Masahiro Yamada <[e-mail]>
> > > > Cc: [e-mail]
> > > > Cc: [e-mail]
> > > >
> > > > ---
> > > >
> > > > Changelog v2:
> > > > - Move driver to the memory subsystem.
> > > > - Use dual GPL/BSD license.
> > > > - Use single lined copyright header.
> > > > - Move "allOf" restrictions to the root level of the properties.
> > > > - Discard syscon compatible string and reg property.
> > > > - The DT node is supposed to be a child of the Baikal-T1 system controller
> > > >   node.
> > > > ---
> > > >  .../memory-controllers/baikal,bt1-l2-ctl.yaml | 59 +++++++++++++++++++
> > > >  1 file changed, 59 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/memory-controllers/baikal,bt1-l2-ctl.yaml
> > > >
> > >
> > >
> > > My bot found errors running 'make dt_binding_check' on your patch:
> > >
> > > [nip] ...
> > >
> > > See https://patchwork.ozlabs.org/patch/1285665
> > >
> > > If you already ran 'make dt_binding_check' and didn't see the above
> > > error(s), then make sure dt-schema is up to date:
> > >
> > > pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade
> > >
> > > Please check and re-submit.
> > >
> >
> > The problem is due to an absent vendor prefix in the test kernel source tree
> > environment. As I said in the cover-letter the new vendor prefix will be added
> > in the framework of the next patchset:
> > https://lkml.org/lkml/2020/5/6/1047
> >
> > Rob, please review that patchset first, merge in the corresponding patch from
> > there and test this binding out then.
> 
> Did you read the part about a 'bot'? My bot doesn't read cover letters
> and I only occasionally do. Do you want to write me a script that can
> do this?

A script, that would read a cover-letter for you? =)
Anyway I wasn't talking to the bot, but to you explaining why the problem
happened and of how to solve it. I didn't mean to blame you or the 'bot'
for not reading the letter. Sorry, if it seemed like I did.

-Sergey

> 
> Rob

Serge Semin May 19, 2020, 12:27 p.m. UTC | #7

Rob,

Could you take a look at this patch?
Since you've accepted and merged in the patch:
https://lore.kernel.org/linux-devicetree/[e-mail]/

It's safe to perform the dt_binding_check of this one.

-Sergey 

On Fri, May 08, 2020 at 02:07:03AM +0300, Serge Semin wrote:
> There is a single register provided by the SoC system controller,
> which can be used to tune the L2-cache RAM up. It only provides a way
> to change the L2-RAM access latencies. So aside from "be,bt1-l2-ctl"
> compatible string the device node can be optionally equipped with the
> properties of Tag/Data/WS latencies.
> 
> Signed-off-by: Serge Semin <[e-mail]>
> Cc: Alexey Malahov <[e-mail]>
> Cc: Paul Burton <[e-mail]>
> Cc: Ralf Baechle <[e-mail]>
> Cc: Olof Johansson <[e-mail]>
> Cc: Boris Brezillon <[e-mail]>
> Cc: Paul Cercueil <[e-mail]>
> Cc: Thomas Gleixner <[e-mail]>
> Cc: Masahiro Yamada <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]
> 
> ---
> 
> Changelog v2:
> - Move driver to the memory subsystem.
> - Use dual GPL/BSD license.
> - Use single lined copyright header.
> - Move "allOf" restrictions to the root level of the properties.
> - Discard syscon compatible string and reg property.
> - The DT node is supposed to be a child of the Baikal-T1 system controller
>   node.
> ---
>  .../memory-controllers/baikal,bt1-l2-ctl.yaml | 59 +++++++++++++++++++
>  1 file changed, 59 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/memory-controllers/baikal,bt1-l2-ctl.yaml
> 
> diff --git a/Documentation/devicetree/bindings/memory-controllers/baikal,bt1-l2-ctl.yaml b/Documentation/devicetree/bindings/memory-controllers/baikal,bt1-l2-ctl.yaml
> new file mode 100644
> index 000000000000..263f0cdab4e6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/memory-controllers/baikal,bt1-l2-ctl.yaml
> @@ -0,0 +1,59 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2020 BAIKAL ELECTRONICS, JSC
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/memory-controllers/baikal,bt1-l2-ctl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Baikal-T1 L2-cache Control Block
> +
> +maintainers:
> +  - Serge Semin <[e-mail]>
> +
> +description: |
> +  By means of the System Controller Baikal-T1 SoC exposes a few settings to
> +  tune the MIPS P5600 CM2 L2 cache performance up. In particular it's possible
> +  to change the Tag, Data and Way-select RAM access latencies. Baikal-T1
> +  L2-cache controller block is responsible for the tuning. Its DT node is
> +  supposed to be a child of the system controller.
> +
> +properties:
> +  compatible:
> +    const: baikal,bt1-l2-ctl
> +
> +  baikal,l2-ws-latency:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Cycles of latency for Way-select RAM accesses
> +    default: 0
> +    minimum: 0
> +    maximum: 3
> +
> +  baikal,l2-tag-latency:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Cycles of latency for Tag RAM accesses
> +    default: 0
> +    minimum: 0
> +    maximum: 3
> +
> +  baikal,l2-data-latency:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Cycles of latency for Data RAM accesses
> +    default: 1
> +    minimum: 0
> +    maximum: 3
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +
> +examples:
> +  - |
> +    l2_ctl {
> +      compatible = "baikal,bt1-l2-ctl";
> +
> +      baikal,l2-ws-latency = <0>;
> +      baikal,l2-tag-latency = <0>;
> +      baikal,l2-data-latency = <1>;
> +    };
> +...
> -- 
> 2.25.1
>

Serge Semin May 21, 2020, 9:46 p.m. UTC | #8

On Mon, May 11, 2020 at 05:43:58PM -0500, Rob Herring wrote:
> On Thu, May 7, 2020 at 6:07 PM Serge Semin
> <[e-mail]> wrote:
> >

[nip]

> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/memory-controllers/baikal,bt1-l2-ctl.yaml
> > @@ -0,0 +1,59 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +# Copyright (C) 2020 BAIKAL ELECTRONICS, JSC
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/memory-controllers/baikal,bt1-l2-ctl.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Baikal-T1 L2-cache Control Block
> > +
> > +maintainers:
> > +  - Serge Semin <[e-mail]>
> > +
> > +description: |
> > +  By means of the System Controller Baikal-T1 SoC exposes a few settings to
> > +  tune the MIPS P5600 CM2 L2 cache performance up. In particular it's possible
> > +  to change the Tag, Data and Way-select RAM access latencies. Baikal-T1
> > +  L2-cache controller block is responsible for the tuning. Its DT node is
> > +  supposed to be a child of the system controller.
> 
> Is there a register range for just the L2 registers in the system
> controller. If so, please add a 'reg' property.

Rob, could you clarify whether the reg property is supposed to be in the required
list in this case? I've got a few more bindings with the same issue. It would be
great to fix all of them at once and resend.

-Sergey

> 
> This should all be part of the system controller schema either as 1
> file or by a $ref from the system controller to this file. That's how
> we ensure "supposed to be a child of the system controller".
> 
> > +
> > +properties:
> > +  compatible:
> > +    const: baikal,bt1-l2-ctl
> > +
> > +  baikal,l2-ws-latency:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: Cycles of latency for Way-select RAM accesses
> > +    default: 0
> > +    minimum: 0
> > +    maximum: 3
> > +
> > +  baikal,l2-tag-latency:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: Cycles of latency for Tag RAM accesses
> > +    default: 0
> > +    minimum: 0
> > +    maximum: 3
> > +
> > +  baikal,l2-data-latency:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: Cycles of latency for Data RAM accesses
> > +    default: 1
> > +    minimum: 0
> > +    maximum: 3
> > +
> > +additionalProperties: false
> > +
> > +required:
> > +  - compatible
> > +
> > +examples:
> > +  - |
> > +    l2_ctl {
> > +      compatible = "baikal,bt1-l2-ctl";
> > +
> > +      baikal,l2-ws-latency = <0>;
> > +      baikal,l2-tag-latency = <0>;
> 
> 0 is the default, why list it?
> 
> > +      baikal,l2-data-latency = <1>;
> > +    };
> > +...
> > --
> > 2.25.1
> >

Patch

diff --git a/Documentation/devicetree/bindings/memory-controllers/baikal,bt1-l2-ctl.yaml b/Documentation/devicetree/bindings/memory-controllers/baikal,bt1-l2-ctl.yaml
new file mode 100644
index 000000000000..263f0cdab4e6
--- /dev/null
+++ b/Documentation/devicetree/bindings/memory-controllers/baikal,bt1-l2-ctl.yaml
@@ -0,0 +1,59 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2020 BAIKAL ELECTRONICS, JSC
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/memory-controllers/baikal,bt1-l2-ctl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Baikal-T1 L2-cache Control Block
+
+maintainers:
+  - Serge Semin <[e-mail]>
+
+description: |
+  By means of the System Controller Baikal-T1 SoC exposes a few settings to
+  tune the MIPS P5600 CM2 L2 cache performance up. In particular it's possible
+  to change the Tag, Data and Way-select RAM access latencies. Baikal-T1
+  L2-cache controller block is responsible for the tuning. Its DT node is
+  supposed to be a child of the system controller.
+
+properties:
+  compatible:
+    const: baikal,bt1-l2-ctl
+
+  baikal,l2-ws-latency:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Cycles of latency for Way-select RAM accesses
+    default: 0
+    minimum: 0
+    maximum: 3
+
+  baikal,l2-tag-latency:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Cycles of latency for Tag RAM accesses
+    default: 0
+    minimum: 0
+    maximum: 3
+
+  baikal,l2-data-latency:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Cycles of latency for Data RAM accesses
+    default: 1
+    minimum: 0
+    maximum: 3
+
+additionalProperties: false
+
+required:
+  - compatible
+
+examples:
+  - |
+    l2_ctl {
+      compatible = "baikal,bt1-l2-ctl";
+
+      baikal,l2-ws-latency = <0>;
+      baikal,l2-tag-latency = <0>;
+      baikal,l2-data-latency = <1>;
+    };
+...