| Message ID | 20240215-mbly-i2c-v1-2-19a336e91dca@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add Mobileye EyeQ5 support to the Nomadik I2C controller & use hrtimers for timeouts | expand |
On Thu, 15 Feb 2024 17:52:09 +0100, Théo Lebrun wrote: > Add EyeQ5 bindings to the existing Nomadik I2C dt-bindings. Add the two > EyeQ5-specific properties behind a conditional. Add an example for this > compatible. > > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com> > --- > .../devicetree/bindings/i2c/st,nomadik-i2c.yaml | 44 ++++++++++++++++++++-- > 1 file changed, 40 insertions(+), 4 deletions(-) > 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: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/i2c/st,nomadik-i2c.example.dtb: i2c@300000: 'mobileye,id', 'mobileye,olb' do 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 ,.*', '^alliedvision,.*', '^allo,.*', '^allwinner,.*', '^alphascale,.*', '^alps,.*', '^alt,.*', '^altr,.*', '^amarula,.*', '^amazon,.*', '^amcc,.*', '^amd,.*', '^amediatech,.*', '^amlogic,.*', '^ampere,.*', '^ampire,.*', '^ams,.*', '^amstaos,.*', '^analogix,.*', '^anbernic,.*', '^andestech,.*', '^anvo,.*', '^aosong,.*', '^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,.*', '^buff alo,.*', '^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,.*', '^coolpi,.*', '^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,.*', '^dimonoff,.*', ' ^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,.*', '^fascontek,.*', '^fastrax,.*', '^fcs,.*', '^feixin,.*', '^feiyang,.*', '^fii,.*', '^firefly,.*', '^focaltech,.*', '^forlinx,.*', '^ freecom,.*', '^frida,.*', '^friendlyarm,.*', '^fsl,.*', '^fujitsu,.*', '^fxtec,.*', '^galaxycore,.*', '^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,.*', '^htc,.*', '^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,.*', '^panas onic,.*', '^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,.*', '^rve,.*', '^saef,.*', '^samsung,.*', '^samtec,.*', '^sanclo ud,.*', '^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,.*', '^smi,.*', '^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,.*', '^techwell,.*', '^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,.*', '^transpeed,.*', '^traverse,.*', '^tronfy,.*', '^tronsmart,.*', '^truly,.*', '^tsd,.*', '^turing,.*', '^tyan,.*', '^u-blox,.*', '^u-boot,.*', '^ubnt,.*', '^ucrobotics,.*', '^udoo,.*', '^ufispace,.*', '^ugoos,.*', '^un iwest,.*', '^upisemi,.*', '^urt,.*', '^usi,.*', '^usr,.*', '^utoo,.*', '^v3,.*', '^vaisala,.*', '^vamrs,.*', '^variscite,.*', '^vdl,.*', '^vertexcom,.*', '^via,.*', '^vialab,.*', '^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/20240215-mbly-i2c-v1-2-19a336e91dca@bootlin.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.
On Thu, Feb 15, 2024 at 05:52:09PM +0100, Théo Lebrun wrote: > Add EyeQ5 bindings to the existing Nomadik I2C dt-bindings. Add the two > EyeQ5-specific properties behind a conditional. Add an example for this > compatible. > > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com> > --- > .../devicetree/bindings/i2c/st,nomadik-i2c.yaml | 44 ++++++++++++++++++++-- > 1 file changed, 40 insertions(+), 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/i2c/st,nomadik-i2c.yaml b/Documentation/devicetree/bindings/i2c/st,nomadik-i2c.yaml > index e6b95e3765ac..eaade14b6d4c 100644 > --- a/Documentation/devicetree/bindings/i2c/st,nomadik-i2c.yaml > +++ b/Documentation/devicetree/bindings/i2c/st,nomadik-i2c.yaml > @@ -14,9 +14,6 @@ description: The Nomadik I2C host controller began its life in the ST > maintainers: > - Linus Walleij <linus.walleij@linaro.org> > > -allOf: > - - $ref: /schemas/i2c/i2c-controller.yaml# > - > # Need a custom select here or 'arm,primecell' will match on lots of nodes > select: > properties: > @@ -24,6 +21,7 @@ select: > contains: > enum: > - st,nomadik-i2c > + - mobileye,eyeq5-i2c > required: > - compatible > > @@ -39,6 +37,10 @@ properties: > - const: stericsson,db8500-i2c > - const: st,nomadik-i2c > - const: arm,primecell > + # The variant found on Mobileye EyeQ5 > + - items: > + - const: mobileye,eyeq5-i2c > + - const: arm,primecell > > reg: > maxItems: 1 > @@ -55,7 +57,7 @@ properties: > - items: > - const: mclk > - const: apb_pclk > - # Clock name in DB8500 > + # Clock name in DB8500 or EyeQ5 > - items: > - const: i2cclk > - const: apb_pclk > @@ -83,6 +85,25 @@ required: > > unevaluatedProperties: false > > +allOf: > + - $ref: /schemas/i2c/i2c-controller.yaml# > + - if: > + properties: > + compatible: > + contains: > + const: mobileye,eyeq5-i2c > + then: > + properties: > + mobileye,olb: > + $ref: /schemas/types.yaml#/definitions/phandle > + description: A phandle to the OLB syscon. Define properties at the top-level and then restrict them in if/then schemas. > + mobileye,id: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: Platform-wide controller ID (integer starting from zero). instance indexes are a NAK. You can use i2cN aliases if you must. Why do you need it? To access OLB? If so, add cell args to the OLB phandle instead. Rob
Hello, On Fri Feb 16, 2024 at 3:22 AM CET, Rob Herring wrote: > On Thu, Feb 15, 2024 at 05:52:09PM +0100, Théo Lebrun wrote: > > Add EyeQ5 bindings to the existing Nomadik I2C dt-bindings. Add the two > > EyeQ5-specific properties behind a conditional. Add an example for this > > compatible. [...] > > +allOf: > > + - $ref: /schemas/i2c/i2c-controller.yaml# > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: mobileye,eyeq5-i2c > > + then: > > + properties: > > + mobileye,olb: > > + $ref: /schemas/types.yaml#/definitions/phandle > > + description: A phandle to the OLB syscon. > > Define properties at the top-level and then restrict them in if/then > schemas. Noted, thanks. > > + mobileye,id: > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + description: Platform-wide controller ID (integer starting from zero). > > instance indexes are a NAK. You can use i2cN aliases if you must. > > Why do you need it? To access OLB? If so, add cell args to the OLB > phandle instead. Why we do what we do: I2C controller must write a 2 bit value depending on the bus speed. All I2C controllers write into the same register. Index is used to compute the shift. mobileye,olb is a phandle to a syscon. I'll be using i2cN aliases I guess. Thanks, -- Théo Lebrun, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On 16/02/2024 11:18, Théo Lebrun wrote: > >>> + mobileye,id: >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + description: Platform-wide controller ID (integer starting from zero). >> >> instance indexes are a NAK. You can use i2cN aliases if you must. >> >> Why do you need it? To access OLB? If so, add cell args to the OLB >> phandle instead. > > Why we do what we do: I2C controller must write a 2 bit value depending > on the bus speed. All I2C controllers write into the same register. Which register? Your devices do not share IO address space. > Index is used to compute the shift. > > mobileye,olb is a phandle to a syscon. I'll be using i2cN aliases I > guess. > > Thanks, > > -- > Théo Lebrun, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com Best regards, Krzysztof
Hello, On Fri Feb 16, 2024 at 11:33 AM CET, Krzysztof Kozlowski wrote: > On 16/02/2024 11:18, Théo Lebrun wrote: > > > >>> + mobileye,id: > >>> + $ref: /schemas/types.yaml#/definitions/uint32 > >>> + description: Platform-wide controller ID (integer starting from zero). > >> > >> instance indexes are a NAK. You can use i2cN aliases if you must. > >> > >> Why do you need it? To access OLB? If so, add cell args to the OLB > >> phandle instead. > > > > Why we do what we do: I2C controller must write a 2 bit value depending > > on the bus speed. All I2C controllers write into the same register. > > Which register? Your devices do not share IO address space. mobileye,olb is a prop with a phandle to a syscon. That syscon contains the register we are interested in. The Linux code side of things is in the following patch. We use syscon_regmap_lookup_by_phandle(). [PATCH 10/13] i2c: nomadik: support Mobileye EyeQ5 I2C controller https://lore.kernel.org/lkml/20240215-mbly-i2c-v1-10-19a336e91dca@bootlin.com/ Thanks, -- Théo Lebrun, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On 16/02/2024 11:40, Théo Lebrun wrote: > Hello, > > On Fri Feb 16, 2024 at 11:33 AM CET, Krzysztof Kozlowski wrote: >> On 16/02/2024 11:18, Théo Lebrun wrote: >>> >>>>> + mobileye,id: >>>>> + $ref: /schemas/types.yaml#/definitions/uint32 >>>>> + description: Platform-wide controller ID (integer starting from zero). >>>> >>>> instance indexes are a NAK. You can use i2cN aliases if you must. >>>> >>>> Why do you need it? To access OLB? If so, add cell args to the OLB >>>> phandle instead. >>> >>> Why we do what we do: I2C controller must write a 2 bit value depending >>> on the bus speed. All I2C controllers write into the same register. >> >> Which register? Your devices do not share IO address space. > > mobileye,olb is a prop with a phandle to a syscon. That syscon contains > the register we are interested in. So exactly what Rob said... I don't understand why you have chosen to go with alias. Best regards, Krzysztof
Hello, On Sat Feb 17, 2024 at 9:25 AM CET, Krzysztof Kozlowski wrote: > On 16/02/2024 11:40, Théo Lebrun wrote: > > On Fri Feb 16, 2024 at 11:33 AM CET, Krzysztof Kozlowski wrote: > >> On 16/02/2024 11:18, Théo Lebrun wrote: > >>> > >>>>> + mobileye,id: > >>>>> + $ref: /schemas/types.yaml#/definitions/uint32 > >>>>> + description: Platform-wide controller ID (integer starting from zero). > >>>> > >>>> instance indexes are a NAK. You can use i2cN aliases if you must. > >>>> > >>>> Why do you need it? To access OLB? If so, add cell args to the OLB > >>>> phandle instead. > >>> > >>> Why we do what we do: I2C controller must write a 2 bit value depending > >>> on the bus speed. All I2C controllers write into the same register. > >> > >> Which register? Your devices do not share IO address space. > > > > mobileye,olb is a prop with a phandle to a syscon. That syscon contains > > the register we are interested in. > > So exactly what Rob said... I don't understand why you have chosen to go > with alias. I had misunderstood Rob's original message. Now that I've done some tests to use cells I get what was meant. I'd have a follow-up question. What should the cells contain? I see two options: - phandle + I2C controller global index (from 0 thru 4). Then Linux (or other) driver know how to map that index to register + mask combo. ie: i2c2: i2c@500000 { compatible = "mobileye,eyeq5-i2c", "arm,primecell"; reg = <0 0x500000 0x0 0x1000>; /* ... */ mobileye,olb = <&olb 2>; }; - phandle + register offset + mask. ie: i2c2: i2c@500000 { compatible = "mobileye,eyeq5-i2c", "arm,primecell"; reg = <0 0x500000 0x0 0x1000>; /* ... */ mobileye,olb = <&olb 0xB8 0x300>; /* phandle + offset + mask */ }; I would have guessed the second approach was frown upon as DT aren't meant to contain iomem offsets. However I'm seeing quite a few drivers using this approach, and no driver doing the first approach. Maybe my instinct isn't leading me the right way. See those bindings that use the second approach. They were found because their drivers use the syscon_regmap_lookup_by_phandle_args() function call. I've added the file creation date to highlight recent bindings (that hopefully are closer to the right way). - phy/starfive,jh7110-pcie-phy.yaml 2023-06-29T15:51:12+08:00 - usb/starfive,jh7110-usb.yaml 2023-05-18T19:27:48+08:00 - net/starfive,jh7110-dwmac.yaml 2023-04-17T18:02:49+08:00 - phy/qcom,sc8280xp-qmp-pcie-phy.yaml 2022-11-05T15:59:34+01:00 - sound/snps,designware-i2s.yaml 2022-07-01T20:22:49+01:00 - pinctrl/canaan,k210-fpioa.yaml 2020-12-13T22:50:44+09:00 - media/ti,cal.yaml 2019-11-12T15:53:47+01:00 I know looking at existing drivers/bindings isn't the right way, but I have no other frame of reference. That's why I'm asking for guidance on this one. Thanks, -- Théo Lebrun, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ------------------------------------------------------------------------
On 19/02/2024 14:41, Théo Lebrun wrote: > Hello, > > On Sat Feb 17, 2024 at 9:25 AM CET, Krzysztof Kozlowski wrote: >> On 16/02/2024 11:40, Théo Lebrun wrote: >>> On Fri Feb 16, 2024 at 11:33 AM CET, Krzysztof Kozlowski wrote: >>>> On 16/02/2024 11:18, Théo Lebrun wrote: >>>>> >>>>>>> + mobileye,id: >>>>>>> + $ref: /schemas/types.yaml#/definitions/uint32 >>>>>>> + description: Platform-wide controller ID (integer starting from zero). >>>>>> >>>>>> instance indexes are a NAK. You can use i2cN aliases if you must. >>>>>> >>>>>> Why do you need it? To access OLB? If so, add cell args to the OLB >>>>>> phandle instead. >>>>> >>>>> Why we do what we do: I2C controller must write a 2 bit value depending >>>>> on the bus speed. All I2C controllers write into the same register. >>>> >>>> Which register? Your devices do not share IO address space. >>> >>> mobileye,olb is a prop with a phandle to a syscon. That syscon contains >>> the register we are interested in. >> >> So exactly what Rob said... I don't understand why you have chosen to go >> with alias. > > I had misunderstood Rob's original message. Now that I've done some > tests to use cells I get what was meant. I'd have a follow-up question. > What should the cells contain? I see two options: > > - phandle + I2C controller global index (from 0 thru 4). Then Linux > (or other) driver know how to map that index to register + mask > combo. ie: > > i2c2: i2c@500000 { > compatible = "mobileye,eyeq5-i2c", "arm,primecell"; > reg = <0 0x500000 0x0 0x1000>; > /* ... */ > mobileye,olb = <&olb 2>; > }; > > - phandle + register offset + mask. ie: > > i2c2: i2c@500000 { > compatible = "mobileye,eyeq5-i2c", "arm,primecell"; > reg = <0 0x500000 0x0 0x1000>; > /* ... */ > mobileye,olb = <&olb 0xB8 0x300>; /* phandle + offset + mask */ > }; Whichever works for your current and possibly future needs and hardware, because property should have one meaning. It's anyway specific to the property. Second option is quite popular. Please design it for entire hardware, not for this one particular case. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/i2c/st,nomadik-i2c.yaml b/Documentation/devicetree/bindings/i2c/st,nomadik-i2c.yaml index e6b95e3765ac..eaade14b6d4c 100644 --- a/Documentation/devicetree/bindings/i2c/st,nomadik-i2c.yaml +++ b/Documentation/devicetree/bindings/i2c/st,nomadik-i2c.yaml @@ -14,9 +14,6 @@ description: The Nomadik I2C host controller began its life in the ST maintainers: - Linus Walleij <linus.walleij@linaro.org> -allOf: - - $ref: /schemas/i2c/i2c-controller.yaml# - # Need a custom select here or 'arm,primecell' will match on lots of nodes select: properties: @@ -24,6 +21,7 @@ select: contains: enum: - st,nomadik-i2c + - mobileye,eyeq5-i2c required: - compatible @@ -39,6 +37,10 @@ properties: - const: stericsson,db8500-i2c - const: st,nomadik-i2c - const: arm,primecell + # The variant found on Mobileye EyeQ5 + - items: + - const: mobileye,eyeq5-i2c + - const: arm,primecell reg: maxItems: 1 @@ -55,7 +57,7 @@ properties: - items: - const: mclk - const: apb_pclk - # Clock name in DB8500 + # Clock name in DB8500 or EyeQ5 - items: - const: i2cclk - const: apb_pclk @@ -83,6 +85,25 @@ required: unevaluatedProperties: false +allOf: + - $ref: /schemas/i2c/i2c-controller.yaml# + - if: + properties: + compatible: + contains: + const: mobileye,eyeq5-i2c + then: + properties: + mobileye,olb: + $ref: /schemas/types.yaml#/definitions/phandle + description: A phandle to the OLB syscon. + mobileye,id: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Platform-wide controller ID (integer starting from zero). + required: + - mobileye,olb + - mobileye,id + examples: - | #include <dt-bindings/interrupt-controller/irq.h> @@ -116,5 +137,20 @@ examples: clocks = <&i2c0clk>, <&pclki2c0>; clock-names = "mclk", "apb_pclk"; }; + - | + #include <dt-bindings/interrupt-controller/mips-gic.h> + i2c@300000 { + compatible = "mobileye,eyeq5-i2c", "arm,primecell"; + reg = <0x300000 0x1000>; + interrupt-parent = <&gic>; + interrupts = <GIC_SHARED 1 IRQ_TYPE_LEVEL_HIGH>; + clock-frequency = <400000>; + #address-cells = <1>; + #size-cells = <0>; + clocks = <&i2c_ser_clk>, <&i2c_clk>; + clock-names = "i2cclk", "apb_pclk"; + mobileye,olb = <&olb>; + mobileye,id = <0>; + }; ...
Add EyeQ5 bindings to the existing Nomadik I2C dt-bindings. Add the two EyeQ5-specific properties behind a conditional. Add an example for this compatible. Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com> --- .../devicetree/bindings/i2c/st,nomadik-i2c.yaml | 44 ++++++++++++++++++++-- 1 file changed, 40 insertions(+), 4 deletions(-)