2023-12-18 17:20:48

by Théo Lebrun

[permalink] [raw]
Subject: [PATCH 1/4] dt-bindings: pinctrl: mobileye,eyeq5-pinctrl: add bindings

Add dt-schema type bindings for the Mobileye EyeQ5 pin controller.

Signed-off-by: Théo Lebrun <[email protected]>
---
.../bindings/pinctrl/mobileye,eyeq5-pinctrl.yaml | 125 +++++++++++++++++++++
MAINTAINERS | 1 +
2 files changed, 126 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/mobileye,eyeq5-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/mobileye,eyeq5-pinctrl.yaml
new file mode 100644
index 000000000000..5faddebe2413
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/mobileye,eyeq5-pinctrl.yaml
@@ -0,0 +1,125 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/mobileye,eyeq5-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mobileye EyeQ5 pinctrl (pinmux & pinconf) controller
+
+description:
+ The EyeQ5 pin controller handles a pin bank. It is custom to this platform,
+ its registers live in a shared region called OLB.
+ There are two pin banks on the platform, each having a specific compatible.
+ Pins and groups are bijective.
+
+maintainers:
+ - Grégory Clement <[email protected]>
+ - Théo Lebrun <[email protected]>
+ - Vladimir Kondratiev <[email protected]>
+
+properties:
+ $nodename:
+ pattern: "^pinctrl([0-9]+)?$"
+ description:
+ We have no unique address, we rely on OLB; we therefore can't keep the
+ standard pattern and cannot inherit from pinctrl.yaml.
+
+ compatible:
+ enum:
+ - mobileye,eyeq5-a-pinctrl
+ - mobileye,eyeq5-b-pinctrl
+
+ "#pinctrl-cells":
+ const: 1
+
+ mobileye,olb:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description:
+ A phandle to the OLB syscon. This is a fallback to using the parent as
+ syscon node.
+
+required:
+ - compatible
+ - "#pinctrl-cells"
+
+patternProperties:
+ "-pins?$":
+ type: object
+ description: Pin muxing configuration.
+ $ref: pinmux-node.yaml#
+ additionalProperties: false
+ properties:
+ pins: true
+ function: true
+ bias-disable: true
+ bias-pull-down: true
+ bias-pull-up: true
+ drive-strength: true
+ required:
+ - pins
+ - function
+
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: mobileye,eyeq5-a-pinctrl
+ then:
+ patternProperties:
+ "-pins?$":
+ properties:
+ function:
+ enum: [gpio, timer0, timer1, timer2, timer5, uart0, uart1, can0,
+ can1, spi0, spi1, refclk0]
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: mobileye,eyeq5-b-pinctrl
+ then:
+ patternProperties:
+ "-pins?$":
+ properties:
+ function:
+ enum: [gpio, timer3, timer4, timer6, uart2, can2, spi2, spi3,
+ mclk0]
+
+additionalProperties: false
+
+examples:
+ - |
+ olb@e00000 {
+ compatible = "mobileye,eyeq5-olb", "syscon", "simple-mfd";
+ reg = <0xe00000 0x400>;
+ reg-io-width = <4>;
+
+ pinctrl0 {
+ compatible = "mobileye,eyeq5-a-pinctrl";
+ #pinctrl-cells = <1>;
+ };
+
+ pinctrl1 {
+ compatible = "mobileye,eyeq5-b-pinctrl";
+ #pinctrl-cells = <1>;
+ };
+ };
+ - |
+ olb: olb@e00000 {
+ compatible = "mobileye,eyeq5-olb", "syscon", "simple-mfd";
+ reg = <0xe00000 0x400>;
+ reg-io-width = <4>;
+ };
+
+ pinctrl0 {
+ compatible = "mobileye,eyeq5-a-pinctrl";
+ #pinctrl-cells = <1>;
+ mobileye,olb = <&olb>;
+ };
+
+ pinctrl1 {
+ compatible = "mobileye,eyeq5-b-pinctrl";
+ #pinctrl-cells = <1>;
+ mobileye,olb = <&olb>;
+ };
+
diff --git a/MAINTAINERS b/MAINTAINERS
index 138e0303c5da..d8d9583a5a06 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14553,6 +14553,7 @@ M: Théo Lebrun <[email protected]>
L: [email protected]
S: Maintained
F: Documentation/devicetree/bindings/mips/mobileye.yaml
+F: Documentation/devicetree/bindings/pinctrl/mobileye,eyeq5-pinctrl.yaml
F: Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml
F: arch/mips/boot/dts/mobileye/
F: arch/mips/configs/generic/board-eyeq5.config

--
2.43.0



2023-12-18 20:47:32

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 1/4] dt-bindings: pinctrl: mobileye,eyeq5-pinctrl: add bindings


On Mon, 18 Dec 2023 18:19:46 +0100, Théo Lebrun wrote:
> Add dt-schema type bindings for the Mobileye EyeQ5 pin controller.
>
> Signed-off-by: Théo Lebrun <[email protected]>
> ---
> .../bindings/pinctrl/mobileye,eyeq5-pinctrl.yaml | 125 +++++++++++++++++++++
> MAINTAINERS | 1 +
> 2 files changed, 126 insertions(+)
>

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/pinctrl/mobileye,eyeq5-pinctrl.example.dtb: /example-0/olb@e00000: failed to match any schema with compatible: ['mobileye,eyeq5-olb', 'syscon', 'simple-mfd']
Documentation/devicetree/bindings/pinctrl/mobileye,eyeq5-pinctrl.example.dtb: /example-1/olb@e00000: failed to match any schema with compatible: ['mobileye,eyeq5-olb', 'syscon', 'simple-mfd']
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/mobileye,eyeq5-pinctrl.example.dtb: pinctrl0: 'mobileye,olb' does not match any of the regexes: '^#.*', '^(at25|bm|devbus|dmacap|dsa|exynos|fsi[ab]|gpio-fan|gpio-key|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|st-plgpio|st-spics|ts),.*', '^100ask,.*', '^70mai,.*', '^8dev,.*', '^GEFanuc,.*', '^ORCL,.*', '^SUNW,.*', '^[a-zA-Z0-9#_][a-zA-Z0-9+\\-._@]{0,63}$', '^[a-zA-Z0-9+\\-._]*@[0-9a-zA-Z,]*$', '^abb,.*', '^abilis,.*', '^abracon,.*', '^abt,.*', '^acbel,.*', '^acer,.*', '^acme,.*', '^actions,.*', '^active-semi,.*', '^ad,.*', '^adafruit,.*', '^adapteva,.*', '^adaptrum,.*', '^adh,.*', '^adi,.*', '^adieng,.*', '^advantech,.*', '^aeroflexgaisler,.*', '^aesop,.*', '^airoha,.*', '^al,.*', '^alcatel,.*', '^aldec,.*', '^alfa-network,.*', '^allegro,.*
', '^allo,.*', '^allwinner,.*', '^alphascale,.*', '^alps,.*', '^alt,.*', '^altr,.*', '^amarula,.*', '^amazon,.*', '^amcc,.*', '^amd,.*', '^amediatech,.*', '^amlogic,.*', '^ampere,.*', '^ampire,.*', '^ams,.*', '^amstaos,.*', '^analogix,.*', '^anbernic,.*', '^andestech,.*', '^anvo,.*', '^apm,.*', '^apple,.*', '^aptina,.*', '^arasan,.*', '^archermind,.*', '^arcom,.*', '^arctic,.*', '^arcx,.*', '^aries,.*', '^arm,.*', '^armadeus,.*', '^arrow,.*', '^artesyn,.*', '^asahi-kasei,.*', '^asc,.*', '^asix,.*', '^aspeed,.*', '^asrock,.*', '^asus,.*', '^atheros,.*', '^atlas,.*', '^atmel,.*', '^auo,.*', '^auvidea,.*', '^avago,.*', '^avia,.*', '^avic,.*', '^avnet,.*', '^awinic,.*', '^axentia,.*', '^axis,.*', '^azoteq,.*', '^azw,.*', '^baikal,.*', '^bananapi,.*', '^beacon,.*', '^beagle,.*', '^belling,.*', '^bhf,.*', '^bigtreetech,.*', '^bitmain,.*', '^blutek,.*', '^boe,.*', '^bosch,.*', '^boundary,.*', '^brcm,.*', '^broadmobi,.*', '^bsh,.*', '^bticino,.*', '^buffalo,.*', '^bur,.*', '^bytedance,.*',
'^calamp,.*', '^calaosystems,.*', '^calxeda,.*', '^canaan,.*', '^caninos,.*', '^capella,.*', '^cascoda,.*', '^catalyst,.*', '^cavium,.*', '^cdns,.*', '^cdtech,.*', '^cellwise,.*', '^ceva,.*', '^chargebyte,.*', '^checkpoint,.*', '^chefree,.*', '^chipidea,.*', '^chipone,.*', '^chipspark,.*', '^chongzhou,.*', '^chrontel,.*', '^chrp,.*', '^chunghwa,.*', '^chuwi,.*', '^ciaa,.*', '^cirrus,.*', '^cisco,.*', '^clockwork,.*', '^cloos,.*', '^cloudengines,.*', '^cnm,.*', '^cnxt,.*', '^colorfly,.*', '^compulab,.*', '^congatec,.*', '^coreriver,.*', '^corpro,.*', '^cortina,.*', '^cosmic,.*', '^crane,.*', '^creative,.*', '^crystalfontz,.*', '^csky,.*', '^csq,.*', '^ctera,.*', '^ctu,.*', '^cubietech,.*', '^cui,.*', '^cypress,.*', '^cyx,.*', '^cznic,.*', '^dallas,.*', '^dataimage,.*', '^davicom,.*', '^dell,.*', '^delta,.*', '^densitron,.*', '^denx,.*', '^devantech,.*', '^dfi,.*', '^dh,.*', '^difrnce,.*', '^digi,.*', '^digilent,.*', '^diodes,.*', '^dioo,.*', '^dlc,.*', '^dlg,.*', '^dlink,.*', '^dmo,.
*', '^domintech,.*', '^dongwoon,.*', '^dptechnics,.*', '^dragino,.*', '^ds,.*', '^dserve,.*', '^dynaimage,.*', '^ea,.*', '^ebang,.*', '^ebbg,.*', '^ebs-systart,.*', '^ebv,.*', '^eckelmann,.*', '^edgeble,.*', '^edimax,.*', '^edt,.*', '^ees,.*', '^eeti,.*', '^einfochips,.*', '^eink,.*', '^elan,.*', '^element14,.*', '^elgin,.*', '^elida,.*', '^elimo,.*', '^elpida,.*', '^embedfire,.*', '^embest,.*', '^emlid,.*', '^emmicro,.*', '^empire-electronix,.*', '^emtrion,.*', '^enclustra,.*', '^endless,.*', '^ene,.*', '^energymicro,.*', '^engicam,.*', '^engleder,.*', '^epcos,.*', '^epfl,.*', '^epson,.*', '^esp,.*', '^est,.*', '^ettus,.*', '^eukrea,.*', '^everest,.*', '^everspin,.*', '^evervision,.*', '^exar,.*', '^excito,.*', '^exegin,.*', '^ezchip,.*', '^facebook,.*', '^fairphone,.*', '^faraday,.*', '^fastrax,.*', '^fcs,.*', '^feixin,.*', '^feiyang,.*', '^fii,.*', '^firefly,.*', '^focaltech,.*', '^forlinx,.*', '^freecom,.*', '^frida,.*', '^friendlyarm,.*', '^fsl,.*', '^fujitsu,.*', '^fxtec,.*',
'^gardena,.*', '^gateway,.*', '^gateworks,.*', '^gcw,.*', '^ge,.*', '^geekbuying,.*', '^gef,.*', '^gemei,.*', '^gemtek,.*', '^genesys,.*', '^geniatech,.*', '^giantec,.*', '^giantplus,.*', '^globalscale,.*', '^globaltop,.*', '^gmt,.*', '^goldelico,.*', '^goodix,.*', '^google,.*', '^goramo,.*', '^gplus,.*', '^grinn,.*', '^grmn,.*', '^gumstix,.*', '^gw,.*', '^hannstar,.*', '^haochuangyi,.*', '^haoyu,.*', '^hardkernel,.*', '^hechuang,.*', '^hideep,.*', '^himax,.*', '^hirschmann,.*', '^hisi,.*', '^hisilicon,.*', '^hit,.*', '^hitex,.*', '^holt,.*', '^holtek,.*', '^honestar,.*', '^honeywell,.*', '^hoperun,.*', '^hp,.*', '^hpe,.*', '^hsg,.*', '^huawei,.*', '^hugsun,.*', '^hwacom,.*', '^hxt,.*', '^hycon,.*', '^hydis,.*', '^hynitron,.*', '^hynix,.*', '^hyundai,.*', '^i2se,.*', '^ibm,.*', '^icplus,.*', '^idt,.*', '^ifi,.*', '^ilitek,.*', '^imagis,.*', '^img,.*', '^imi,.*', '^inanbo,.*', '^incircuit,.*', '^indiedroid,.*', '^inet-tek,.*', '^infineon,.*', '^inforce,.*', '^ingenic,.*', '^ingrasys,
.*', '^injoinic,.*', '^innocomm,.*', '^innolux,.*', '^inside-secure,.*', '^insignal,.*', '^inspur,.*', '^intel,.*', '^intercontrol,.*', '^invensense,.*', '^inventec,.*', '^inversepath,.*', '^iom,.*', '^irondevice,.*', '^isee,.*', '^isil,.*', '^issi,.*', '^ite,.*', '^itead,.*', '^itian,.*', '^ivo,.*', '^iwave,.*', '^jadard,.*', '^jasonic,.*', '^jdi,.*', '^jedec,.*', '^jesurun,.*', '^jethome,.*', '^jianda,.*', '^joz,.*', '^kam,.*', '^karo,.*', '^keithkoep,.*', '^keymile,.*', '^khadas,.*', '^kiebackpeter,.*', '^kinetic,.*', '^kingdisplay,.*', '^kingnovel,.*', '^kionix,.*', '^kobo,.*', '^kobol,.*', '^koe,.*', '^kontron,.*', '^kosagi,.*', '^kvg,.*', '^kyo,.*', '^lacie,.*', '^laird,.*', '^lamobo,.*', '^lantiq,.*', '^lattice,.*', '^lctech,.*', '^leadtek,.*', '^leez,.*', '^lego,.*', '^lemaker,.*', '^lenovo,.*', '^lg,.*', '^lgphilips,.*', '^libretech,.*', '^licheepi,.*', '^linaro,.*', '^lineartechnology,.*', '^linksprite,.*', '^linksys,.*', '^linutronix,.*', '^linux,.*', '^linx,.*', '^liteon
,.*', '^litex,.*', '^lltc,.*', '^logicpd,.*', '^logictechno,.*', '^longcheer,.*', '^lontium,.*', '^loongmasses,.*', '^loongson,.*', '^lsi,.*', '^lunzn,.*', '^lwn,.*', '^lxa,.*', '^m5stack,.*', '^macnica,.*', '^mantix,.*', '^mapleboard,.*', '^marantec,.*', '^marvell,.*', '^maxbotix,.*', '^maxim,.*', '^maxlinear,.*', '^mbvl,.*', '^mcube,.*', '^meas,.*', '^mecer,.*', '^mediatek,.*', '^megachips,.*', '^mele,.*', '^melexis,.*', '^melfas,.*', '^mellanox,.*', '^memsensing,.*', '^memsic,.*', '^menlo,.*', '^mentor,.*', '^meraki,.*', '^merrii,.*', '^methode,.*', '^micrel,.*', '^microchip,.*', '^microcrystal,.*', '^micron,.*', '^microsoft,.*', '^microsys,.*', '^mikroe,.*', '^mikrotik,.*', '^milkv,.*', '^miniand,.*', '^minix,.*', '^miramems,.*', '^mitsubishi,.*', '^mitsumi,.*', '^mixel,.*', '^miyoo,.*', '^mntre,.*', '^modtronix,.*', '^moortec,.*', '^mosaixtech,.*', '^motorcomm,.*', '^motorola,.*', '^moxa,.*', '^mpl,.*', '^mps,.*', '^mqmaker,.*', '^mrvl,.*', '^mscc,.*', '^msi,.*', '^mstar,.*', '
^mti,.*', '^multi-inno,.*', '^mundoreader,.*', '^murata,.*', '^mxic,.*', '^mxicy,.*', '^myir,.*', '^national,.*', '^nec,.*', '^neonode,.*', '^netgear,.*', '^netlogic,.*', '^netron-dy,.*', '^netronix,.*', '^netxeon,.*', '^neweast,.*', '^newhaven,.*', '^newvision,.*', '^nexbox,.*', '^nextthing,.*', '^ni,.*', '^nintendo,.*', '^nlt,.*', '^nokia,.*', '^nordic,.*', '^novatek,.*', '^novtech,.*', '^nutsboard,.*', '^nuvoton,.*', '^nvd,.*', '^nvidia,.*', '^nxp,.*', '^oceanic,.*', '^ocs,.*', '^oct,.*', '^okaya,.*', '^oki,.*', '^olimex,.*', '^olpc,.*', '^oneplus,.*', '^onie,.*', '^onion,.*', '^onnn,.*', '^ontat,.*', '^opalkelly,.*', '^openailab,.*', '^opencores,.*', '^openembed,.*', '^openpandora,.*', '^openrisc,.*', '^option,.*', '^oranth,.*', '^orisetech,.*', '^ortustech,.*', '^osddisplays,.*', '^osmc,.*', '^ouya,.*', '^overkiz,.*', '^ovti,.*', '^oxsemi,.*', '^ozzmaker,.*', '^panasonic,.*', '^parade,.*', '^parallax,.*', '^pda,.*', '^pericom,.*', '^pervasive,.*', '^phicomm,.*', '^phytec,.*', '
^picochip,.*', '^pine64,.*', '^pineriver,.*', '^pixcir,.*', '^plantower,.*', '^plathome,.*', '^plda,.*', '^plx,.*', '^ply,.*', '^pni,.*', '^pocketbook,.*', '^polaroid,.*', '^polyhex,.*', '^portwell,.*', '^poslab,.*', '^pov,.*', '^powertip,.*', '^powervr,.*', '^powkiddy,.*', '^primux,.*', '^probox2,.*', '^prt,.*', '^pulsedlight,.*', '^purism,.*', '^qca,.*', '^qcom,.*', '^qemu,.*', '^qi,.*', '^qiaodian,.*', '^qihua,.*', '^qishenglong,.*', '^qnap,.*', '^quanta,.*', '^radxa,.*', '^raidsonic,.*', '^ralink,.*', '^ramtron,.*', '^raspberrypi,.*', '^raydium,.*', '^rda,.*', '^realtek,.*', '^remarkable,.*', '^renesas,.*', '^rervision,.*', '^revotics,.*', '^rex,.*', '^richtek,.*', '^ricoh,.*', '^rikomagic,.*', '^riot,.*', '^riscv,.*', '^rockchip,.*', '^rocktech,.*', '^rohm,.*', '^ronbo,.*', '^roofull,.*', '^roseapplepi,.*', '^saef,.*', '^samsung,.*', '^samtec,.*', '^sancloud,.*', '^sandisk,.*', '^satoz,.*', '^sbs,.*', '^schindler,.*', '^seagate,.*', '^seeed,.*', '^seirobotics,.*', '^semtech,.*'
, '^senseair,.*', '^sensirion,.*', '^sensortek,.*', '^sercomm,.*', '^sff,.*', '^sgd,.*', '^sgmicro,.*', '^sgx,.*', '^sharp,.*', '^shift,.*', '^shimafuji,.*', '^shineworld,.*', '^shiratech,.*', '^si-en,.*', '^si-linux,.*', '^siemens,.*', '^sifive,.*', '^sigma,.*', '^sii,.*', '^sil,.*', '^silabs,.*', '^silan,.*', '^silead,.*', '^silergy,.*', '^silex-insight,.*', '^siliconfile,.*', '^siliconmitus,.*', '^silvaco,.*', '^simtek,.*', '^sinlinx,.*', '^sinovoip,.*', '^sinowealth,.*', '^sipeed,.*', '^sirf,.*', '^sis,.*', '^sitronix,.*', '^skov,.*', '^skyworks,.*', '^smartlabs,.*', '^smsc,.*', '^snps,.*', '^sochip,.*', '^socionext,.*', '^solidrun,.*', '^solomon,.*', '^sony,.*', '^sophgo,.*', '^sourceparts,.*', '^spansion,.*', '^sparkfun,.*', '^spinalhdl,.*', '^sprd,.*', '^square,.*', '^ssi,.*', '^sst,.*', '^sstar,.*', '^st,.*', '^st-ericsson,.*', '^starfive,.*', '^starry,.*', '^startek,.*', '^starterkit,.*', '^ste,.*', '^stericsson,.*', '^storlink,.*', '^storm,.*', '^storopack,.*', '^summit,.*
', '^sunchip,.*', '^sundance,.*', '^sunplus,.*', '^supermicro,.*', '^swir,.*', '^syna,.*', '^synology,.*', '^synopsys,.*', '^tbs,.*', '^tbs-biometrics,.*', '^tcg,.*', '^tcl,.*', '^tcs,.*', '^tdo,.*', '^team-source-display,.*', '^technexion,.*', '^technologic,.*', '^techstar,.*', '^teejet,.*', '^teltonika,.*', '^tempo,.*', '^terasic,.*', '^tesla,.*', '^tfc,.*', '^thead,.*', '^thine,.*', '^thingyjp,.*', '^thundercomm,.*', '^thwc,.*', '^ti,.*', '^tianma,.*', '^tlm,.*', '^tmt,.*', '^topeet,.*', '^topic,.*', '^toppoly,.*', '^topwise,.*', '^toradex,.*', '^toshiba,.*', '^toumaz,.*', '^tpk,.*', '^tplink,.*', '^tpo,.*', '^tq,.*', '^traverse,.*', '^tronfy,.*', '^tronsmart,.*', '^truly,.*', '^tsd,.*', '^turing,.*', '^tyan,.*', '^u-blox,.*', '^u-boot,.*', '^ubnt,.*', '^ucrobotics,.*', '^udoo,.*', '^ufispace,.*', '^ugoos,.*', '^uniwest,.*', '^upisemi,.*', '^urt,.*', '^usi,.*', '^usr,.*', '^utoo,.*', '^v3,.*', '^vaisala,.*', '^vamrs,.*', '^variscite,.*', '^vdl,.*', '^vertexcom,.*', '^via,.*', '^v
ialab,.*', '^vicor,.*', '^videostrong,.*', '^virtio,.*', '^virtual,.*', '^vishay,.*', '^visionox,.*', '^vitesse,.*', '^vivante,.*', '^vivax,.*', '^vocore,.*', '^voipac,.*', '^vot,.*', '^vxt,.*', '^wanchanglong,.*', '^wand,.*', '^waveshare,.*', '^wd,.*', '^we,.*', '^welltech,.*', '^wetek,.*', '^wexler,.*', '^whwave,.*', '^wi2wi,.*', '^widora,.*', '^wiligear,.*', '^willsemi,.*', '^winbond,.*', '^wingtech,.*', '^winlink,.*', '^winstar,.*', '^wirelesstag,.*', '^wits,.*', '^wlf,.*', '^wm,.*', '^wobo,.*', '^x-powers,.*', '^xen,.*', '^xes,.*', '^xiaomi,.*', '^xillybus,.*', '^xingbangda,.*', '^xinpeng,.*', '^xiphera,.*', '^xlnx,.*', '^xnano,.*', '^xunlong,.*', '^xylon,.*', '^yadro,.*', '^yamaha,.*', '^yes-optoelectronics,.*', '^yic,.*', '^yiming,.*', '^ylm,.*', '^yna,.*', '^yones-toptech,.*', '^ys,.*', '^ysoft,.*', '^zarlink,.*', '^zealz,.*', '^zeitec,.*', '^zidoo,.*', '^zii,.*', '^zinitix,.*', '^zkmagic,.*', '^zte,.*', '^zyxel,.*', 'pinctrl-[0-9]+'
from schema $id: http://devicetree.org/schemas/vendor-prefixes.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/mobileye,eyeq5-pinctrl.example.dtb: pinctrl1: 'mobileye,olb' does not match any of the regexes: '^#.*', '^(at25|bm|devbus|dmacap|dsa|exynos|fsi[ab]|gpio-fan|gpio-key|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|st-plgpio|st-spics|ts),.*', '^100ask,.*', '^70mai,.*', '^8dev,.*', '^GEFanuc,.*', '^ORCL,.*', '^SUNW,.*', '^[a-zA-Z0-9#_][a-zA-Z0-9+\\-._@]{0,63}$', '^[a-zA-Z0-9+\\-._]*@[0-9a-zA-Z,]*$', '^abb,.*', '^abilis,.*', '^abracon,.*', '^abt,.*', '^acbel,.*', '^acer,.*', '^acme,.*', '^actions,.*', '^active-semi,.*', '^ad,.*', '^adafruit,.*', '^adapteva,.*', '^adaptrum,.*', '^adh,.*', '^adi,.*', '^adieng,.*', '^advantech,.*', '^aeroflexgaisler,.*', '^aesop,.*', '^airoha,.*', '^al,.*', '^alcatel,.*', '^aldec,.*', '^alfa-network,.*', '^allegro,.*
', '^allo,.*', '^allwinner,.*', '^alphascale,.*', '^alps,.*', '^alt,.*', '^altr,.*', '^amarula,.*', '^amazon,.*', '^amcc,.*', '^amd,.*', '^amediatech,.*', '^amlogic,.*', '^ampere,.*', '^ampire,.*', '^ams,.*', '^amstaos,.*', '^analogix,.*', '^anbernic,.*', '^andestech,.*', '^anvo,.*', '^apm,.*', '^apple,.*', '^aptina,.*', '^arasan,.*', '^archermind,.*', '^arcom,.*', '^arctic,.*', '^arcx,.*', '^aries,.*', '^arm,.*', '^armadeus,.*', '^arrow,.*', '^artesyn,.*', '^asahi-kasei,.*', '^asc,.*', '^asix,.*', '^aspeed,.*', '^asrock,.*', '^asus,.*', '^atheros,.*', '^atlas,.*', '^atmel,.*', '^auo,.*', '^auvidea,.*', '^avago,.*', '^avia,.*', '^avic,.*', '^avnet,.*', '^awinic,.*', '^axentia,.*', '^axis,.*', '^azoteq,.*', '^azw,.*', '^baikal,.*', '^bananapi,.*', '^beacon,.*', '^beagle,.*', '^belling,.*', '^bhf,.*', '^bigtreetech,.*', '^bitmain,.*', '^blutek,.*', '^boe,.*', '^bosch,.*', '^boundary,.*', '^brcm,.*', '^broadmobi,.*', '^bsh,.*', '^bticino,.*', '^buffalo,.*', '^bur,.*', '^bytedance,.*',
'^calamp,.*', '^calaosystems,.*', '^calxeda,.*', '^canaan,.*', '^caninos,.*', '^capella,.*', '^cascoda,.*', '^catalyst,.*', '^cavium,.*', '^cdns,.*', '^cdtech,.*', '^cellwise,.*', '^ceva,.*', '^chargebyte,.*', '^checkpoint,.*', '^chefree,.*', '^chipidea,.*', '^chipone,.*', '^chipspark,.*', '^chongzhou,.*', '^chrontel,.*', '^chrp,.*', '^chunghwa,.*', '^chuwi,.*', '^ciaa,.*', '^cirrus,.*', '^cisco,.*', '^clockwork,.*', '^cloos,.*', '^cloudengines,.*', '^cnm,.*', '^cnxt,.*', '^colorfly,.*', '^compulab,.*', '^congatec,.*', '^coreriver,.*', '^corpro,.*', '^cortina,.*', '^cosmic,.*', '^crane,.*', '^creative,.*', '^crystalfontz,.*', '^csky,.*', '^csq,.*', '^ctera,.*', '^ctu,.*', '^cubietech,.*', '^cui,.*', '^cypress,.*', '^cyx,.*', '^cznic,.*', '^dallas,.*', '^dataimage,.*', '^davicom,.*', '^dell,.*', '^delta,.*', '^densitron,.*', '^denx,.*', '^devantech,.*', '^dfi,.*', '^dh,.*', '^difrnce,.*', '^digi,.*', '^digilent,.*', '^diodes,.*', '^dioo,.*', '^dlc,.*', '^dlg,.*', '^dlink,.*', '^dmo,.
*', '^domintech,.*', '^dongwoon,.*', '^dptechnics,.*', '^dragino,.*', '^ds,.*', '^dserve,.*', '^dynaimage,.*', '^ea,.*', '^ebang,.*', '^ebbg,.*', '^ebs-systart,.*', '^ebv,.*', '^eckelmann,.*', '^edgeble,.*', '^edimax,.*', '^edt,.*', '^ees,.*', '^eeti,.*', '^einfochips,.*', '^eink,.*', '^elan,.*', '^element14,.*', '^elgin,.*', '^elida,.*', '^elimo,.*', '^elpida,.*', '^embedfire,.*', '^embest,.*', '^emlid,.*', '^emmicro,.*', '^empire-electronix,.*', '^emtrion,.*', '^enclustra,.*', '^endless,.*', '^ene,.*', '^energymicro,.*', '^engicam,.*', '^engleder,.*', '^epcos,.*', '^epfl,.*', '^epson,.*', '^esp,.*', '^est,.*', '^ettus,.*', '^eukrea,.*', '^everest,.*', '^everspin,.*', '^evervision,.*', '^exar,.*', '^excito,.*', '^exegin,.*', '^ezchip,.*', '^facebook,.*', '^fairphone,.*', '^faraday,.*', '^fastrax,.*', '^fcs,.*', '^feixin,.*', '^feiyang,.*', '^fii,.*', '^firefly,.*', '^focaltech,.*', '^forlinx,.*', '^freecom,.*', '^frida,.*', '^friendlyarm,.*', '^fsl,.*', '^fujitsu,.*', '^fxtec,.*',
'^gardena,.*', '^gateway,.*', '^gateworks,.*', '^gcw,.*', '^ge,.*', '^geekbuying,.*', '^gef,.*', '^gemei,.*', '^gemtek,.*', '^genesys,.*', '^geniatech,.*', '^giantec,.*', '^giantplus,.*', '^globalscale,.*', '^globaltop,.*', '^gmt,.*', '^goldelico,.*', '^goodix,.*', '^google,.*', '^goramo,.*', '^gplus,.*', '^grinn,.*', '^grmn,.*', '^gumstix,.*', '^gw,.*', '^hannstar,.*', '^haochuangyi,.*', '^haoyu,.*', '^hardkernel,.*', '^hechuang,.*', '^hideep,.*', '^himax,.*', '^hirschmann,.*', '^hisi,.*', '^hisilicon,.*', '^hit,.*', '^hitex,.*', '^holt,.*', '^holtek,.*', '^honestar,.*', '^honeywell,.*', '^hoperun,.*', '^hp,.*', '^hpe,.*', '^hsg,.*', '^huawei,.*', '^hugsun,.*', '^hwacom,.*', '^hxt,.*', '^hycon,.*', '^hydis,.*', '^hynitron,.*', '^hynix,.*', '^hyundai,.*', '^i2se,.*', '^ibm,.*', '^icplus,.*', '^idt,.*', '^ifi,.*', '^ilitek,.*', '^imagis,.*', '^img,.*', '^imi,.*', '^inanbo,.*', '^incircuit,.*', '^indiedroid,.*', '^inet-tek,.*', '^infineon,.*', '^inforce,.*', '^ingenic,.*', '^ingrasys,
.*', '^injoinic,.*', '^innocomm,.*', '^innolux,.*', '^inside-secure,.*', '^insignal,.*', '^inspur,.*', '^intel,.*', '^intercontrol,.*', '^invensense,.*', '^inventec,.*', '^inversepath,.*', '^iom,.*', '^irondevice,.*', '^isee,.*', '^isil,.*', '^issi,.*', '^ite,.*', '^itead,.*', '^itian,.*', '^ivo,.*', '^iwave,.*', '^jadard,.*', '^jasonic,.*', '^jdi,.*', '^jedec,.*', '^jesurun,.*', '^jethome,.*', '^jianda,.*', '^joz,.*', '^kam,.*', '^karo,.*', '^keithkoep,.*', '^keymile,.*', '^khadas,.*', '^kiebackpeter,.*', '^kinetic,.*', '^kingdisplay,.*', '^kingnovel,.*', '^kionix,.*', '^kobo,.*', '^kobol,.*', '^koe,.*', '^kontron,.*', '^kosagi,.*', '^kvg,.*', '^kyo,.*', '^lacie,.*', '^laird,.*', '^lamobo,.*', '^lantiq,.*', '^lattice,.*', '^lctech,.*', '^leadtek,.*', '^leez,.*', '^lego,.*', '^lemaker,.*', '^lenovo,.*', '^lg,.*', '^lgphilips,.*', '^libretech,.*', '^licheepi,.*', '^linaro,.*', '^lineartechnology,.*', '^linksprite,.*', '^linksys,.*', '^linutronix,.*', '^linux,.*', '^linx,.*', '^liteon
,.*', '^litex,.*', '^lltc,.*', '^logicpd,.*', '^logictechno,.*', '^longcheer,.*', '^lontium,.*', '^loongmasses,.*', '^loongson,.*', '^lsi,.*', '^lunzn,.*', '^lwn,.*', '^lxa,.*', '^m5stack,.*', '^macnica,.*', '^mantix,.*', '^mapleboard,.*', '^marantec,.*', '^marvell,.*', '^maxbotix,.*', '^maxim,.*', '^maxlinear,.*', '^mbvl,.*', '^mcube,.*', '^meas,.*', '^mecer,.*', '^mediatek,.*', '^megachips,.*', '^mele,.*', '^melexis,.*', '^melfas,.*', '^mellanox,.*', '^memsensing,.*', '^memsic,.*', '^menlo,.*', '^mentor,.*', '^meraki,.*', '^merrii,.*', '^methode,.*', '^micrel,.*', '^microchip,.*', '^microcrystal,.*', '^micron,.*', '^microsoft,.*', '^microsys,.*', '^mikroe,.*', '^mikrotik,.*', '^milkv,.*', '^miniand,.*', '^minix,.*', '^miramems,.*', '^mitsubishi,.*', '^mitsumi,.*', '^mixel,.*', '^miyoo,.*', '^mntre,.*', '^modtronix,.*', '^moortec,.*', '^mosaixtech,.*', '^motorcomm,.*', '^motorola,.*', '^moxa,.*', '^mpl,.*', '^mps,.*', '^mqmaker,.*', '^mrvl,.*', '^mscc,.*', '^msi,.*', '^mstar,.*', '
^mti,.*', '^multi-inno,.*', '^mundoreader,.*', '^murata,.*', '^mxic,.*', '^mxicy,.*', '^myir,.*', '^national,.*', '^nec,.*', '^neonode,.*', '^netgear,.*', '^netlogic,.*', '^netron-dy,.*', '^netronix,.*', '^netxeon,.*', '^neweast,.*', '^newhaven,.*', '^newvision,.*', '^nexbox,.*', '^nextthing,.*', '^ni,.*', '^nintendo,.*', '^nlt,.*', '^nokia,.*', '^nordic,.*', '^novatek,.*', '^novtech,.*', '^nutsboard,.*', '^nuvoton,.*', '^nvd,.*', '^nvidia,.*', '^nxp,.*', '^oceanic,.*', '^ocs,.*', '^oct,.*', '^okaya,.*', '^oki,.*', '^olimex,.*', '^olpc,.*', '^oneplus,.*', '^onie,.*', '^onion,.*', '^onnn,.*', '^ontat,.*', '^opalkelly,.*', '^openailab,.*', '^opencores,.*', '^openembed,.*', '^openpandora,.*', '^openrisc,.*', '^option,.*', '^oranth,.*', '^orisetech,.*', '^ortustech,.*', '^osddisplays,.*', '^osmc,.*', '^ouya,.*', '^overkiz,.*', '^ovti,.*', '^oxsemi,.*', '^ozzmaker,.*', '^panasonic,.*', '^parade,.*', '^parallax,.*', '^pda,.*', '^pericom,.*', '^pervasive,.*', '^phicomm,.*', '^phytec,.*', '
^picochip,.*', '^pine64,.*', '^pineriver,.*', '^pixcir,.*', '^plantower,.*', '^plathome,.*', '^plda,.*', '^plx,.*', '^ply,.*', '^pni,.*', '^pocketbook,.*', '^polaroid,.*', '^polyhex,.*', '^portwell,.*', '^poslab,.*', '^pov,.*', '^powertip,.*', '^powervr,.*', '^powkiddy,.*', '^primux,.*', '^probox2,.*', '^prt,.*', '^pulsedlight,.*', '^purism,.*', '^qca,.*', '^qcom,.*', '^qemu,.*', '^qi,.*', '^qiaodian,.*', '^qihua,.*', '^qishenglong,.*', '^qnap,.*', '^quanta,.*', '^radxa,.*', '^raidsonic,.*', '^ralink,.*', '^ramtron,.*', '^raspberrypi,.*', '^raydium,.*', '^rda,.*', '^realtek,.*', '^remarkable,.*', '^renesas,.*', '^rervision,.*', '^revotics,.*', '^rex,.*', '^richtek,.*', '^ricoh,.*', '^rikomagic,.*', '^riot,.*', '^riscv,.*', '^rockchip,.*', '^rocktech,.*', '^rohm,.*', '^ronbo,.*', '^roofull,.*', '^roseapplepi,.*', '^saef,.*', '^samsung,.*', '^samtec,.*', '^sancloud,.*', '^sandisk,.*', '^satoz,.*', '^sbs,.*', '^schindler,.*', '^seagate,.*', '^seeed,.*', '^seirobotics,.*', '^semtech,.*'
, '^senseair,.*', '^sensirion,.*', '^sensortek,.*', '^sercomm,.*', '^sff,.*', '^sgd,.*', '^sgmicro,.*', '^sgx,.*', '^sharp,.*', '^shift,.*', '^shimafuji,.*', '^shineworld,.*', '^shiratech,.*', '^si-en,.*', '^si-linux,.*', '^siemens,.*', '^sifive,.*', '^sigma,.*', '^sii,.*', '^sil,.*', '^silabs,.*', '^silan,.*', '^silead,.*', '^silergy,.*', '^silex-insight,.*', '^siliconfile,.*', '^siliconmitus,.*', '^silvaco,.*', '^simtek,.*', '^sinlinx,.*', '^sinovoip,.*', '^sinowealth,.*', '^sipeed,.*', '^sirf,.*', '^sis,.*', '^sitronix,.*', '^skov,.*', '^skyworks,.*', '^smartlabs,.*', '^smsc,.*', '^snps,.*', '^sochip,.*', '^socionext,.*', '^solidrun,.*', '^solomon,.*', '^sony,.*', '^sophgo,.*', '^sourceparts,.*', '^spansion,.*', '^sparkfun,.*', '^spinalhdl,.*', '^sprd,.*', '^square,.*', '^ssi,.*', '^sst,.*', '^sstar,.*', '^st,.*', '^st-ericsson,.*', '^starfive,.*', '^starry,.*', '^startek,.*', '^starterkit,.*', '^ste,.*', '^stericsson,.*', '^storlink,.*', '^storm,.*', '^storopack,.*', '^summit,.*
', '^sunchip,.*', '^sundance,.*', '^sunplus,.*', '^supermicro,.*', '^swir,.*', '^syna,.*', '^synology,.*', '^synopsys,.*', '^tbs,.*', '^tbs-biometrics,.*', '^tcg,.*', '^tcl,.*', '^tcs,.*', '^tdo,.*', '^team-source-display,.*', '^technexion,.*', '^technologic,.*', '^techstar,.*', '^teejet,.*', '^teltonika,.*', '^tempo,.*', '^terasic,.*', '^tesla,.*', '^tfc,.*', '^thead,.*', '^thine,.*', '^thingyjp,.*', '^thundercomm,.*', '^thwc,.*', '^ti,.*', '^tianma,.*', '^tlm,.*', '^tmt,.*', '^topeet,.*', '^topic,.*', '^toppoly,.*', '^topwise,.*', '^toradex,.*', '^toshiba,.*', '^toumaz,.*', '^tpk,.*', '^tplink,.*', '^tpo,.*', '^tq,.*', '^traverse,.*', '^tronfy,.*', '^tronsmart,.*', '^truly,.*', '^tsd,.*', '^turing,.*', '^tyan,.*', '^u-blox,.*', '^u-boot,.*', '^ubnt,.*', '^ucrobotics,.*', '^udoo,.*', '^ufispace,.*', '^ugoos,.*', '^uniwest,.*', '^upisemi,.*', '^urt,.*', '^usi,.*', '^usr,.*', '^utoo,.*', '^v3,.*', '^vaisala,.*', '^vamrs,.*', '^variscite,.*', '^vdl,.*', '^vertexcom,.*', '^via,.*', '^v
ialab,.*', '^vicor,.*', '^videostrong,.*', '^virtio,.*', '^virtual,.*', '^vishay,.*', '^visionox,.*', '^vitesse,.*', '^vivante,.*', '^vivax,.*', '^vocore,.*', '^voipac,.*', '^vot,.*', '^vxt,.*', '^wanchanglong,.*', '^wand,.*', '^waveshare,.*', '^wd,.*', '^we,.*', '^welltech,.*', '^wetek,.*', '^wexler,.*', '^whwave,.*', '^wi2wi,.*', '^widora,.*', '^wiligear,.*', '^willsemi,.*', '^winbond,.*', '^wingtech,.*', '^winlink,.*', '^winstar,.*', '^wirelesstag,.*', '^wits,.*', '^wlf,.*', '^wm,.*', '^wobo,.*', '^x-powers,.*', '^xen,.*', '^xes,.*', '^xiaomi,.*', '^xillybus,.*', '^xingbangda,.*', '^xinpeng,.*', '^xiphera,.*', '^xlnx,.*', '^xnano,.*', '^xunlong,.*', '^xylon,.*', '^yadro,.*', '^yamaha,.*', '^yes-optoelectronics,.*', '^yic,.*', '^yiming,.*', '^ylm,.*', '^yna,.*', '^yones-toptech,.*', '^ys,.*', '^ysoft,.*', '^zarlink,.*', '^zealz,.*', '^zeitec,.*', '^zidoo,.*', '^zii,.*', '^zinitix,.*', '^zkmagic,.*', '^zte,.*', '^zyxel,.*', 'pinctrl-[0-9]+'
from schema $id: http://devicetree.org/schemas/vendor-prefixes.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]

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.


2023-12-19 07:35:06

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/4] dt-bindings: pinctrl: mobileye,eyeq5-pinctrl: add bindings

On 18/12/2023 18:19, Théo Lebrun wrote:
> Add dt-schema type bindings for the Mobileye EyeQ5 pin controller.
>
> Signed-off-by: Théo Lebrun <[email protected]>
> ---
> .../bindings/pinctrl/mobileye,eyeq5-pinctrl.yaml | 125 +++++++++++++++++++++
> MAINTAINERS | 1 +
> 2 files changed, 126 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/mobileye,eyeq5-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/mobileye,eyeq5-pinctrl.yaml
> new file mode 100644
> index 000000000000..5faddebe2413
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/mobileye,eyeq5-pinctrl.yaml
> @@ -0,0 +1,125 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/mobileye,eyeq5-pinctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Mobileye EyeQ5 pinctrl (pinmux & pinconf) controller

pinctrl means pin controller, so you basically wrote:
pin controller pinmux and pin configuration controller

Just "pin controller"


> +
> +description:
> + The EyeQ5 pin controller handles a pin bank. It is custom to this platform,

Can part of SoC be not custom to given platform? I mean... describe the
hardware, not write essay.

> + its registers live in a shared region called OLB.
> + There are two pin banks on the platform, each having a specific compatible.

Instead of repeating something obvious - visible from the binding -
explain why. Say something different than the binding is saying.


> + Pins and groups are bijective.
> +
> +maintainers:
> + - Grégory Clement <[email protected]>
> + - Théo Lebrun <[email protected]>
> + - Vladimir Kondratiev <[email protected]>
> +
> +properties:
> + $nodename:
> + pattern: "^pinctrl([0-9]+)?$"
> + description:
> + We have no unique address, we rely on OLB; we therefore can't keep the
> + standard pattern and cannot inherit from pinctrl.yaml.

No, instead fix pinctrl.yaml

> +
> + compatible:
> + enum:
> + - mobileye,eyeq5-a-pinctrl
> + - mobileye,eyeq5-b-pinctrl

Why two compatibles? Description provided no rationale for this.

> +
> + "#pinctrl-cells":
> + const: 1
> +
> + mobileye,olb:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + A phandle to the OLB syscon. This is a fallback to using the parent as
> + syscon node.

So here is the explanation for missing unit address. If all registers,
as you claim in description, belong to OLB, then this should be part of
OLB. Drop the phandle.

> +
> +required:
> + - compatible
> + - "#pinctrl-cells"

So now please test your code without olb phandle...

> +
> +patternProperties:

patternProperties go after properties

> + "-pins?$":
> + type: object
> + description: Pin muxing configuration.
> + $ref: pinmux-node.yaml#
> + additionalProperties: false

Why not unevaluatedProperties?

> + properties:
> + pins: true
> + function: true
> + bias-disable: true
> + bias-pull-down: true
> + bias-pull-up: true
> + drive-strength: true
> + required:
> + - pins
> + - function
> +
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: mobileye,eyeq5-a-pinctrl
> + then:
> + patternProperties:
> + "-pins?$":
> + properties:
> + function:
> + enum: [gpio, timer0, timer1, timer2, timer5, uart0, uart1, can0,
> + can1, spi0, spi1, refclk0]
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: mobileye,eyeq5-b-pinctrl
> + then:
> + patternProperties:
> + "-pins?$":
> + properties:
> + function:
> + enum: [gpio, timer3, timer4, timer6, uart2, can2, spi2, spi3,
> + mclk0]
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + olb@e00000 {
> + compatible = "mobileye,eyeq5-olb", "syscon", "simple-mfd";

Drop, not erlated.

> + reg = <0xe00000 0x400>;
> + reg-io-width = <4>;
> +
> + pinctrl0 {

Suffixes are always after -

> + compatible = "mobileye,eyeq5-a-pinctrl";
> + #pinctrl-cells = <1>;

Where is the phandle?

> + };
> +
> + pinctrl1 {
> + compatible = "mobileye,eyeq5-b-pinctrl";
> + #pinctrl-cells = <1>;
> + };
> + };
> + - |
> + olb: olb@e00000 {
> + compatible = "mobileye,eyeq5-olb", "syscon", "simple-mfd";
> + reg = <0xe00000 0x400>;
> + reg-io-width = <4>;
> + };
> +
> + pinctrl0 {
> + compatible = "mobileye,eyeq5-a-pinctrl";
> + #pinctrl-cells = <1>;
> + mobileye,olb = <&olb>;

Really, why? This is just confusing. There is no explanation for
supporting both. Hardware is either this or that, not both!


Best regards,
Krzysztof


2023-12-20 09:21:37

by Théo Lebrun

[permalink] [raw]
Subject: Re: [PATCH 1/4] dt-bindings: pinctrl: mobileye,eyeq5-pinctrl: add bindings

Hello,

I've seen all your comments, thanks for that. I'll answer to some.

On Tue Dec 19, 2023 at 8:34 AM CET, Krzysztof Kozlowski wrote:
> On 18/12/2023 18:19, Théo Lebrun wrote:
> > Add dt-schema type bindings for the Mobileye EyeQ5 pin controller.
> >
> > Signed-off-by: Théo Lebrun <[email protected]>
> > ---
> > .../bindings/pinctrl/mobileye,eyeq5-pinctrl.yaml | 125 +++++++++++++++++++++
> > MAINTAINERS | 1 +
> > 2 files changed, 126 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/pinctrl/mobileye,eyeq5-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/mobileye,eyeq5-pinctrl.yaml
> > new file mode 100644
> > index 000000000000..5faddebe2413
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pinctrl/mobileye,eyeq5-pinctrl.yaml
> > @@ -0,0 +1,125 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pinctrl/mobileye,eyeq5-pinctrl.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Mobileye EyeQ5 pinctrl (pinmux & pinconf) controller
>
> pinctrl means pin controller, so you basically wrote:
> pin controller pinmux and pin configuration controller
>
> Just "pin controller"
>
>
> > +
> > +description:
> > + The EyeQ5 pin controller handles a pin bank. It is custom to this platform,
>
> Can part of SoC be not custom to given platform? I mean... describe the
> hardware, not write essay.
>
> > + its registers live in a shared region called OLB.
> > + There are two pin banks on the platform, each having a specific compatible.
>
> Instead of repeating something obvious - visible from the binding -
> explain why. Say something different than the binding is saying.
>
>
> > + Pins and groups are bijective.
> > +
> > +maintainers:
> > + - Grégory Clement <[email protected]>
> > + - Théo Lebrun <[email protected]>
> > + - Vladimir Kondratiev <[email protected]>
> > +
> > +properties:
> > + $nodename:
> > + pattern: "^pinctrl([0-9]+)?$"
> > + description:
> > + We have no unique address, we rely on OLB; we therefore can't keep the
> > + standard pattern and cannot inherit from pinctrl.yaml.
>
> No, instead fix pinctrl.yaml

I've tried some things, but I'm unsure how to proceed. Options I see:

- Modify pinctrl.yaml so that if reg/ranges is required, $nodename must
be the current value ("^(pinctrl|pinmux)(@[0-9a-f]+)?$"). Else,
$nodename should be "^(pinctrl|pinmux)(-[0-9a-f]+)?$".

I've tried some things but nothing conclusive for the moment.

- Leave pinctrl.yaml alone and override $nodename from our binding.
I've not found a way to do that though.

- Use the current $nodename, ie with a unit address. With that approach
I get the "node has a unit name, but no reg or ranges property"
warning which, reading the code, I don't see a way of avoiding.

Were you thinking about option 1? Any advice on how to proceed would be
helpful, I've not been able to get a working patch to use option 1.

>
> > +
> > + compatible:
> > + enum:
> > + - mobileye,eyeq5-a-pinctrl
> > + - mobileye,eyeq5-b-pinctrl
>
> Why two compatibles? Description provided no rationale for this.

I'll add that info. The gist of it is to have one node per bank. Each
pin has two function: GPIO or pin-dependent. So we must know which bank
we are to know what each pin function can be.

Both nodes are child to the same OLB. The compatible also tells us which
registers to use.

>
> > +
> > + "#pinctrl-cells":
> > + const: 1
> > +
> > + mobileye,olb:
> > + $ref: /schemas/types.yaml#/definitions/phandle
> > + description:
> > + A phandle to the OLB syscon. This is a fallback to using the parent as
> > + syscon node.
>
> So here is the explanation for missing unit address. If all registers,
> as you claim in description, belong to OLB, then this should be part of
> OLB. Drop the phandle.

The reason I provided both options was that I see four drivers that do
this kind of fallback. I guess it was for legacy reasons. I'm dropping
the phandle and keeping only the child option.

drivers/gpio/gpio-syscon.c
drivers/phy/rockchip/phy-rockchip-usb.c
drivers/phy/samsung/phy-exynos-dp-video.c
drivers/soc/rockchip/io-domain.c

>
> > +
> > +required:
> > + - compatible
> > + - "#pinctrl-cells"
>
> So now please test your code without olb phandle...

That is the main way I am running my code.

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

2023-12-20 10:27:34

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/4] dt-bindings: pinctrl: mobileye,eyeq5-pinctrl: add bindings

On 20/12/2023 10:21, Théo Lebrun wrote:
> Hello,
>
> I've seen all your comments, thanks for that. I'll answer to some.
>
> On Tue Dec 19, 2023 at 8:34 AM CET, Krzysztof Kozlowski wrote:
>> On 18/12/2023 18:19, Théo Lebrun wrote:
>>> Add dt-schema type bindings for the Mobileye EyeQ5 pin controller.
>>>
>>> Signed-off-by: Théo Lebrun <[email protected]>
>>> ---
>>> .../bindings/pinctrl/mobileye,eyeq5-pinctrl.yaml | 125 +++++++++++++++++++++
>>> MAINTAINERS | 1 +
>>> 2 files changed, 126 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/pinctrl/mobileye,eyeq5-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/mobileye,eyeq5-pinctrl.yaml
>>> new file mode 100644
>>> index 000000000000..5faddebe2413
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/pinctrl/mobileye,eyeq5-pinctrl.yaml
>>> @@ -0,0 +1,125 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/pinctrl/mobileye,eyeq5-pinctrl.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Mobileye EyeQ5 pinctrl (pinmux & pinconf) controller
>>
>> pinctrl means pin controller, so you basically wrote:
>> pin controller pinmux and pin configuration controller
>>
>> Just "pin controller"
>>
>>
>>> +
>>> +description:
>>> + The EyeQ5 pin controller handles a pin bank. It is custom to this platform,
>>
>> Can part of SoC be not custom to given platform? I mean... describe the
>> hardware, not write essay.
>>
>>> + its registers live in a shared region called OLB.
>>> + There are two pin banks on the platform, each having a specific compatible.
>>
>> Instead of repeating something obvious - visible from the binding -
>> explain why. Say something different than the binding is saying.
>>
>>
>>> + Pins and groups are bijective.
>>> +
>>> +maintainers:
>>> + - Grégory Clement <[email protected]>
>>> + - Théo Lebrun <[email protected]>
>>> + - Vladimir Kondratiev <[email protected]>
>>> +
>>> +properties:
>>> + $nodename:
>>> + pattern: "^pinctrl([0-9]+)?$"
>>> + description:
>>> + We have no unique address, we rely on OLB; we therefore can't keep the
>>> + standard pattern and cannot inherit from pinctrl.yaml.
>>
>> No, instead fix pinctrl.yaml
>
> I've tried some things, but I'm unsure how to proceed. Options I see:
>
> - Modify pinctrl.yaml so that if reg/ranges is required, $nodename must
> be the current value ("^(pinctrl|pinmux)(@[0-9a-f]+)?$"). Else,
> $nodename should be "^(pinctrl|pinmux)(-[0-9a-f]+)?$".

Yes, but: "-[0-9]", these are not hex.

I don't understand what is the problem here. It's just a regex and there
are plenty of examples how this should look like.

>
> I've tried some things but nothing conclusive for the moment.
>
> - Leave pinctrl.yaml alone and override $nodename from our binding.
> I've not found a way to do that though.
>
> - Use the current $nodename, ie with a unit address. With that approach
> I get the "node has a unit name, but no reg or ranges property"
> warning which, reading the code, I don't see a way of avoiding.
>
> Were you thinking about option 1? Any advice on how to proceed would be
> helpful, I've not been able to get a working patch to use option 1.

Why?

>
>>
>>> +
>>> + compatible:
>>> + enum:
>>> + - mobileye,eyeq5-a-pinctrl
>>> + - mobileye,eyeq5-b-pinctrl
>>
>> Why two compatibles? Description provided no rationale for this.
>
> I'll add that info. The gist of it is to have one node per bank. Each
> pin has two function: GPIO or pin-dependent. So we must know which bank
> we are to know what each pin function can be.

OK

>
> Both nodes are child to the same OLB. The compatible also tells us which
> registers to use.
>
>>
>>> +
>>> + "#pinctrl-cells":
>>> + const: 1
>>> +
>>> + mobileye,olb:
>>> + $ref: /schemas/types.yaml#/definitions/phandle
>>> + description:
>>> + A phandle to the OLB syscon. This is a fallback to using the parent as
>>> + syscon node.
>>
>> So here is the explanation for missing unit address. If all registers,
>> as you claim in description, belong to OLB, then this should be part of
>> OLB. Drop the phandle.
>
> The reason I provided both options was that I see four drivers that do
> this kind of fallback. I guess it was for legacy reasons. I'm dropping
> the phandle and keeping only the child option.
>
> drivers/gpio/gpio-syscon.c
> drivers/phy/rockchip/phy-rockchip-usb.c
> drivers/phy/samsung/phy-exynos-dp-video.c
> drivers/soc/rockchip/io-domain.c
>


Best regards,
Krzysztof