| Message ID | 20250227162823.3585810-8-david@protonic.nl (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add Linux Motion Control subsystem | expand |
On Thu, 27 Feb 2025 17:28:23 +0100, David Jander wrote: > Add device-tree bindings for simple Linux Motion Control devices that > are based on 1 or 2 PWM outputs. > > Signed-off-by: David Jander <david@protonic.nl> > --- > .../bindings/motion/motion-simple-pwm.yaml | 55 +++++++++++++++++++ > 1 file changed, 55 insertions(+) > create mode 100644 Documentation/devicetree/bindings/motion/motion-simple-pwm.yaml > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/motion/motion-simple-pwm.example.dtb: motion-simple-pwm0: 'motion,acceleration-conv-div', 'motion,acceleration-conv-mul', 'motion,pwm-inverted', 'motion,speed-conv-div', 'motion,speed-conv-mul' 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,.*', '^IBM,.*', '^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,.*', '^acelink,.*', '^acer,.*', '^acme,.*', '^actions,.*', '^active-semi,.*', '^ad,.*', '^adafruit,.*', '^adapteva,.*', '^adaptrum,.*', '^adh,.*', '^adi,.*', '^adieng,.*', '^admatec,.*', '^advantech,.*', '^aeroflexgaisler,.*', '^aesop,.*', '^airoha,.*', '^al,.*', '^alcatel,.*', '^aldec,.*', '^alfa-network,.*', '^allegro,.*', '^allegromicro,.*', '^alliedvision,.*', '^allo,.*', '^allwinner,.*', '^alphascale,.*', '^alps,.*', '^alt,.*', '^altr,.*', '^amarula,.*', '^amazon,.*', '^amcc,.*', '^amd,.*', '^amediatech,.*', '^amlogic,.*', '^ampere,.*', '^amphenol,.*', '^ampire,.*', '^ams,.*', '^amstaos,.*', '^analogix,.*', '^anbernic,.*', '^andestech,.*', '^anvo,.*', '^aosong,.*', '^apm,.*', '^apple,.*', '^aptina,.*', '^arasan,.*', '^archermind,.*', '^arcom,.*', '^arctic,.*', '^arcx,.*', '^aries,.*', '^arm,.*', '^armadeus,.*', '^armsom,.*', '^arrow,.*', '^artesyn,.*', '^asahi-kasei,.*', '^asc,.*', '^asix,.*', '^aspeed,.*', '^asrock,.*', '^asteralabs,.*', '^asus,.*', '^atheros,.*', '^atlas,.*', '^atmel,.*', '^auo,.*', '^auvidea,.*', '^avago,.*', '^avia,.*', '^avic,.*', '^avnet,.*', '^awinic,.*', '^axentia,.*', '^axis,.*', '^azoteq,.*', '^azw,.*', '^baikal,.*', '^bananapi,.*', '^beacon,.*', '^beagle,.*', '^belling,.*', '^bhf,.*', '^bigtreetech,.*', '^bitmain,.*', '^blaize,.*', '^blutek,.*', '^boe,.*', '^bosch,.*', '^boundary,.*', '^brcm,.*', '^broadmobi,.*', '^bsh,.*', '^bticino,.*', '^buffalo,.*', '^bur,.*', '^bytedance,.*', '^calamp,.*', '^calao,.*', '^calaosystems,.*', '^calxeda,.*', '^cameo,.*', '^canaan,.*', '^caninos,.*', '^capella,.*', '^cascoda,.*', '^catalyst,.*', '^cavium,.*', '^cct,.*', '^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,.*', '^comvetia,.*', '^congatec,.*', '^coolpi,.*', '^coreriver,.*', '^corpro,.*', '^cortina,.*', '^cosmic,.*', '^crane,.*', '^creative,.*', '^crystalfontz,.*', '^csky,.*', '^csq,.*', '^ctera,.*', '^ctu,.*', '^cubietech,.*', '^cudy,.*', '^cui,.*', '^cypress,.*', '^cyx,.*', '^cznic,.*', '^dallas,.*', '^dataimage,.*', '^davicom,.*', '^deepcomputing,.*', '^dell,.*', '^delta,.*', '^densitron,.*', '^denx,.*', '^devantech,.*', '^dfi,.*', '^dfrobot,.*', '^dh,.*', '^difrnce,.*', '^digi,.*', '^digilent,.*', '^dimonoff,.*', '^diodes,.*', '^dioo,.*', '^dlc,.*', '^dlg,.*', '^dlink,.*', '^dmo,.*', '^domintech,.*', '^dongwoon,.*', '^dptechnics,.*', '^dragino,.*', '^dream,.*', '^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,.*', '^emcraft,.*', '^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,.*', '^fairchild,.*', '^fairphone,.*', '^faraday,.*', '^fascontek,.*', '^fastrax,.*', '^fcs,.*', '^feixin,.*', '^feiyang,.*', '^fii,.*', '^firefly,.*', '^focaltech,.*', '^forlinx,.*', '^freebox,.*', '^freecom,.*', '^frida,.*', '^friendlyarm,.*', '^fsl,.*', '^fujitsu,.*', '^fxtec,.*', '^galaxycore,.*', '^gameforce,.*', '^gardena,.*', '^gateway,.*', '^gateworks,.*', '^gcw,.*', '^ge,.*', '^geekbuying,.*', '^gef,.*', '^gehc,.*', '^gemei,.*', '^gemtek,.*', '^genesys,.*', '^genexis,.*', '^geniatech,.*', '^giantec,.*', '^giantplus,.*', '^glinet,.*', '^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,.*', '^hoperf,.*', '^hoperun,.*', '^hp,.*', '^hpe,.*', '^hsg,.*', '^htc,.*', '^huawei,.*', '^hugsun,.*', '^hwacom,.*', '^hxt,.*', '^hycon,.*', '^hydis,.*', '^hynitron,.*', '^hynix,.*', '^hyundai,.*', '^i2se,.*', '^ibm,.*', '^icplus,.*', '^idt,.*', '^iei,.*', '^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,.*', '^jenson,.*', '^jesurun,.*', '^jethome,.*', '^jianda,.*', '^jide,.*', '^joz,.*', '^kam,.*', '^karo,.*', '^keithkoep,.*', '^keymile,.*', '^khadas,.*', '^kiebackpeter,.*', '^kinetic,.*', '^kingdisplay,.*', '^kingnovel,.*', '^kionix,.*', '^kobo,.*', '^kobol,.*', '^koe,.*', '^kontron,.*', '^kosagi,.*', '^kvg,.*', '^kyo,.*', '^lacie,.*', '^laird,.*', '^lamobo,.*', '^lantiq,.*', '^lattice,.*', '^lckfb,.*', '^lctech,.*', '^leadtek,.*', '^leez,.*', '^lego,.*', '^lemaker,.*', '^lenovo,.*', '^lg,.*', '^lgphilips,.*', '^libretech,.*', '^licheepi,.*', '^linaro,.*', '^lincolntech,.*', '^lineartechnology,.*', '^linksprite,.*', '^linksys,.*', '^linutronix,.*', '^linux,.*', '^linx,.*', '^liteon,.*', '^litex,.*', '^lltc,.*', '^logicpd,.*', '^logictechno,.*', '^longcheer,.*', '^lontium,.*', '^loongmasses,.*', '^loongson,.*', '^lsi,.*', '^lunzn,.*', '^luxul,.*', '^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,.*', '^microtips,.*', '^mikroe,.*', '^mikrotik,.*', '^milkv,.*', '^miniand,.*', '^minix,.*', '^mips,.*', '^miramems,.*', '^mitsubishi,.*', '^mitsumi,.*', '^mixel,.*', '^miyoo,.*', '^mntre,.*', '^mobileye,.*', '^modtronix,.*', '^moortec,.*', '^mosaixtech,.*', '^motorcomm,.*', '^motorola,.*', '^moxa,.*', '^mpl,.*', '^mps,.*', '^mqmaker,.*', '^mrvl,.*', '^mscc,.*', '^msi,.*', '^mstar,.*', '^mti,.*', '^multi-inno,.*', '^mundoreader,.*', '^murata,.*', '^mxic,.*', '^mxicy,.*', '^myir,.*', '^national,.*', '^neardi,.*', '^nec,.*', '^neofidelity,.*', '^neonode,.*', '^netgear,.*', '^netlogic,.*', '^netron-dy,.*', '^netronix,.*', '^netxeon,.*', '^neweast,.*', '^newhaven,.*', '^newvision,.*', '^nexbox,.*', '^nextthing,.*', '^ni,.*', '^nintendo,.*', '^nlt,.*', '^nokia,.*', '^nordic,.*', '^nothing,.*', '^novatek,.*', '^novtech,.*', '^numonyx,.*', '^nutsboard,.*', '^nuvoton,.*', '^nvd,.*', '^nvidia,.*', '^nxp,.*', '^oceanic,.*', '^ocs,.*', '^oct,.*', '^okaya,.*', '^oki,.*', '^olimex,.*', '^olpc,.*', '^oneplus,.*', '^onie,.*', '^onion,.*', '^onnn,.*', '^ontat,.*', '^opalkelly,.*', '^openailab,.*', '^opencores,.*', '^openembed,.*', '^openpandora,.*', '^openrisc,.*', '^openwrt,.*', '^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,.*', '^primeview,.*', '^primux,.*', '^probox2,.*', '^prt,.*', '^pulsedlight,.*', '^purism,.*', '^puya,.*', '^qca,.*', '^qcom,.*', '^qemu,.*', '^qi,.*', '^qiaodian,.*', '^qihua,.*', '^qishenglong,.*', '^qnap,.*', '^quanta,.*', '^radxa,.*', '^raidsonic,.*', '^ralink,.*', '^ramtron,.*', '^raspberrypi,.*', '^raydium,.*', '^rda,.*', '^realtek,.*', '^relfor,.*', '^remarkable,.*', '^renesas,.*', '^rervision,.*', '^revotics,.*', '^rex,.*', '^richtek,.*', '^ricoh,.*', '^rikomagic,.*', '^riot,.*', '^riscv,.*', '^rockchip,.*', '^rocktech,.*', '^rohm,.*', '^ronbo,.*', '^roofull,.*', '^roseapplepi,.*', '^rve,.*', '^saef,.*', '^samsung,.*', '^samtec,.*', '^sancloud,.*', '^sandisk,.*', '^satoz,.*', '^sbs,.*', '^schindler,.*', '^schneider,.*', '^sciosense,.*', '^seagate,.*', '^seeed,.*', '^seirobotics,.*', '^semtech,.*', '^senseair,.*', '^sensirion,.*', '^sensortek,.*', '^sercomm,.*', '^sff,.*', '^sgd,.*', '^sgmicro,.*', '^sgx,.*', '^sharp,.*', '^shift,.*', '^shimafuji,.*', '^shineworld,.*', '^shiratech,.*', '^si-en,.*', '^si-linux,.*', '^siemens,.*', '^sifive,.*', '^siflower,.*', '^sigma,.*', '^sii,.*', '^sil,.*', '^silabs,.*', '^silan,.*', '^silead,.*', '^silergy,.*', '^silex-insight,.*', '^siliconfile,.*', '^siliconmitus,.*', '^silvaco,.*', '^simtek,.*', '^sinlinx,.*', '^sinovoip,.*', '^sinowealth,.*', '^sipeed,.*', '^sirf,.*', '^sis,.*', '^sitronix,.*', '^skov,.*', '^skyworks,.*', '^smartlabs,.*', '^smartrg,.*', '^smi,.*', '^smsc,.*', '^snps,.*', '^sochip,.*', '^socionext,.*', '^solidrun,.*', '^solomon,.*', '^sony,.*', '^sophgo,.*', '^sourceparts,.*', '^spacemit,.*', '^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,.*', '^test,.*', '^tfc,.*', '^thead,.*', '^thine,.*', '^thingyjp,.*', '^thundercomm,.*', '^thwc,.*', '^ti,.*', '^tianma,.*', '^tlm,.*', '^tmt,.*', '^topeet,.*', '^topic,.*', '^topland,.*', '^toppoly,.*', '^topwise,.*', '^toradex,.*', '^toshiba,.*', '^toumaz,.*', '^tpk,.*', '^tplink,.*', '^tpo,.*', '^tq,.*', '^transpeed,.*', '^traverse,.*', '^tronfy,.*', '^tronsmart,.*', '^truly,.*', '^tsd,.*', '^turing,.*', '^tyan,.*', '^tyhx,.*', '^u-blox,.*', '^u-boot,.*', '^ubnt,.*', '^ucrobotics,.*', '^udoo,.*', '^ufispace,.*', '^ugoos,.*', '^uni-t,.*', '^uniwest,.*', '^upisemi,.*', '^urt,.*', '^usi,.*', '^usr,.*', '^utoo,.*', '^v3,.*', '^vaisala,.*', '^vamrs,.*', '^variscite,.*', '^vdl,.*', '^vertexcom,.*', '^via,.*', '^vialab,.*', '^vicor,.*', '^videostrong,.*', '^virtio,.*', '^virtual,.*', '^vishay,.*', '^visionox,.*', '^vitesse,.*', '^vivante,.*', '^vivax,.*', '^vocore,.*', '^voipac,.*', '^voltafield,.*', '^vot,.*', '^vscom,.*', '^vxt,.*', '^wacom,.*', '^wanchanglong,.*', '^wand,.*', '^waveshare,.*', '^wd,.*', '^we,.*', '^welltech,.*', '^wetek,.*', '^wexler,.*', '^whwave,.*', '^wi2wi,.*', '^widora,.*', '^wiligear,.*', '^willsemi,.*', '^winbond,.*', '^wingtech,.*', '^winlink,.*', '^winstar,.*', '^wirelesstag,.*', '^wits,.*', '^wlf,.*', '^wm,.*', '^wobo,.*', '^wolfvision,.*', '^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/20250227162823.3585810-8-david@protonic.nl 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 27, 2025 at 05:28:23PM +0100, David Jander wrote: > Add device-tree bindings for simple Linux Motion Control devices that > are based on 1 or 2 PWM outputs. > > Signed-off-by: David Jander <david@protonic.nl> > --- > .../bindings/motion/motion-simple-pwm.yaml | 55 +++++++++++++++++++ > 1 file changed, 55 insertions(+) > create mode 100644 Documentation/devicetree/bindings/motion/motion-simple-pwm.yaml > > diff --git a/Documentation/devicetree/bindings/motion/motion-simple-pwm.yaml b/Documentation/devicetree/bindings/motion/motion-simple-pwm.yaml > new file mode 100644 > index 000000000000..409e3aef6f3f > --- /dev/null > +++ b/Documentation/devicetree/bindings/motion/motion-simple-pwm.yaml > @@ -0,0 +1,55 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/motion/motion-simple-pwm.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Simple PWM based motor controller > + > +maintainers: > + - David Jander <david@protonic> > + > +description: | Do not need '|' unless you need to preserve formatting. > + Simple motor control device based on 1 or 2 PWM outputs Your schema does not allow 1. Test it. > + > +properties: > + compatible: > + enum: > + - motion-simple-pwm > + > + pwms: > + maxItems: 2 List and describe items instead. > + > + pwm-names: > + maxItems: 2 List items instead. > + > + motion,pwm-inverted: > + $ref: /schemas/types.yaml#/definitions/flag And PWM flag does not work? Anyway, there is no "motion" company. > + description: > + If present, this flag indicates that the PWM signal should be inverted. > + The duty-cycle will be scaled from 100% down to 0% instead 0% to 100%. > + > +required: > + - compatible > + - pwms > + > +allOf: > + - $ref: /schemas/motion/common.yaml# > + > +unevaluatedProperties: false > + > +examples: > + - | > + // This example shows how to use the TI DRV8873 or similar motor controllers > + // with this driver > + motion-simple-pwm0 { Node names should be generic. See also an explanation and list of examples (not exhaustive) in DT specification: https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation e.g. motor { Best regards, Krzysztof
Dear Krzysztof, Thanks for reviewing... On Fri, 28 Feb 2025 08:12:45 +0100 Krzysztof Kozlowski <krzk@kernel.org> wrote: > On Thu, Feb 27, 2025 at 05:28:23PM +0100, David Jander wrote: > [...] > > +description: | > > Do not need '|' unless you need to preserve formatting. > > > + Simple motor control device based on 1 or 2 PWM outputs > > Your schema does not allow 1. Test it. Ok, that came as a surprise to me. Thanks! > > + > > +properties: > > + compatible: > > + enum: > > + - motion-simple-pwm > > + > > + pwms: > > + maxItems: 2 > > List and describe items instead. > > > + > > + pwm-names: > > + maxItems: 2 > > List items instead. Will do in next iteration. Thanks. > > + > > + motion,pwm-inverted: > > + $ref: /schemas/types.yaml#/definitions/flag > > And PWM flag does not work? I have seen PWM controllers that don't seem to support the PWM_POLARITY_INVERTED flag and those where it just doesn't work. Should all PWM controller drivers always support the PWM_POLARITY_INVERTED flag, even if it needs to be inverted in software? If so, there are some drivers that need fixing. > Anyway, there is no "motion" company. Got it. Dropped all the "motion," prefixes. > > + description: > > + If present, this flag indicates that the PWM signal should be inverted. > > + The duty-cycle will be scaled from 100% down to 0% instead 0% to 100%. > > + > > +required: > > + - compatible > > + - pwms > > + > > +allOf: > > + - $ref: /schemas/motion/common.yaml# > > + > > +unevaluatedProperties: false > > + > > +examples: > > + - | > > + // This example shows how to use the TI DRV8873 or similar motor controllers > > + // with this driver > > + motion-simple-pwm0 { > > Node names should be generic. See also an explanation and list of > examples (not exhaustive) in DT specification: > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > > e.g. motor { Will change. Thanks. Best regards,
On 28/02/2025 10:22, David Jander wrote: > >>> + >>> + motion,pwm-inverted: >>> + $ref: /schemas/types.yaml#/definitions/flag >> >> And PWM flag does not work? > > I have seen PWM controllers that don't seem to support the > PWM_POLARITY_INVERTED flag and those where it just doesn't work. Should all Shouldn't the controllers be fixed? Or let's rephrase the question: why only this PWM consumer needs this property and none of others need it? Best regards, Krzysztof
On Fri, 28 Feb 2025 10:37:48 +0100 Krzysztof Kozlowski <krzk@kernel.org> wrote: > On 28/02/2025 10:22, David Jander wrote: > > > >>> + > >>> + motion,pwm-inverted: > >>> + $ref: /schemas/types.yaml#/definitions/flag > >> > >> And PWM flag does not work? > > > > I have seen PWM controllers that don't seem to support the > > PWM_POLARITY_INVERTED flag and those where it just doesn't work. Should all > > > Shouldn't the controllers be fixed? Or let's rephrase the question: why > only this PWM consumer needs this property and none of others need it? CCing Uwe Kleine-Koenig and linux-pwm mailing list. I know that at least in kernel 6.11 the pwm-stm32.c PWM driver doesn't properly invert the PWM signal when specifying PWM_POLARITY_INVERTED. I agree this is a probably bug that needs fixing if still present in 6.14-rc. Besides that, if linux-pwm agrees that every single PWM driver _must_ properly support this flag, I will drop this consumer flag an start fixing broken PWM drivers that I encounter. I agree that it makes more sense this way, but I wanted to be sure. Best regards,
Hey David, On Fri, Feb 28, 2025 at 11:09:31AM +0100, David Jander wrote: > On Fri, 28 Feb 2025 10:37:48 +0100 > Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > On 28/02/2025 10:22, David Jander wrote: > > > > > >>> + > > >>> + motion,pwm-inverted: > > >>> + $ref: /schemas/types.yaml#/definitions/flag > > >> > > >> And PWM flag does not work? > > > > > > I have seen PWM controllers that don't seem to support the > > > PWM_POLARITY_INVERTED flag and those where it just doesn't work. Should all > > > > > > Shouldn't the controllers be fixed? Or let's rephrase the question: why > > only this PWM consumer needs this property and none of others need it? > > CCing Uwe Kleine-Koenig and linux-pwm mailing list. > > I know that at least in kernel 6.11 the pwm-stm32.c PWM driver doesn't > properly invert the PWM signal when specifying PWM_POLARITY_INVERTED. I agree > this is a probably bug that needs fixing if still present in 6.14-rc. Besides > that, if linux-pwm agrees that every single PWM driver _must_ properly support > this flag, I will drop this consumer flag an start fixing broken PWM drivers > that I encounter. I agree that it makes more sense this way, but I wanted to > be sure. Some hardwares cannot support PWM_POLARITY_INVERTED. Affected drivers include: pwm-adp5585 pwm-ntxec pwm-raspberrypi-poe pwm-rz-mtu3 (software limitation only) pwm-sunplus pwm-twl-led (not completely sure, that one is strange) . ISTR that there is a driver that does only support inverted polarity, but I don't find it. For an overview I recommend reading through the output of: for f in drivers/pwm/pwm-*; do echo $f; sed -rn '/Limitations:/,/\*\/?$/p' $f; echo; done | less . (Note not all drivers have commentary in the right format to unveil their limitations.) For most use-cases you can just do .duty_cycle = .period - .duty_cycle instead of inverting polarity, but there is no abstraction in the PWM bindings for that and also no helpers in the PWM framework. The problem is more or less ignored, so if you have a device with pwms = <&pwm0 0 PWM_POLARITY_INVERTED>; and the PWM chip in question doesn't support that, the pwm API functions will fail. So the system designer better makes sure that the PWM hardware can cope with the needed polarity. Best regards Uwe
On 2/27/25 10:28 AM, David Jander wrote: > Add device-tree bindings for simple Linux Motion Control devices that > are based on 1 or 2 PWM outputs. > > Signed-off-by: David Jander <david@protonic.nl> > --- > .../bindings/motion/motion-simple-pwm.yaml | 55 +++++++++++++++++++ > 1 file changed, 55 insertions(+) > create mode 100644 Documentation/devicetree/bindings/motion/motion-simple-pwm.yaml > > diff --git a/Documentation/devicetree/bindings/motion/motion-simple-pwm.yaml b/Documentation/devicetree/bindings/motion/motion-simple-pwm.yaml > new file mode 100644 > index 000000000000..409e3aef6f3f > --- /dev/null > +++ b/Documentation/devicetree/bindings/motion/motion-simple-pwm.yaml > @@ -0,0 +1,55 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/motion/motion-simple-pwm.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Simple PWM based motor controller I think it has been said many times before, in DT, there is no such thing as a simple device! It will be much more future-proof if we write bindings for actual individual motor controller chips than try to generalize all in a single binding. The chip you gave as an example is far from the simplest H-bridge I have seen! > + > +maintainers: > + - David Jander <david@protonic> > + > +description: | > + Simple motor control device based on 1 or 2 PWM outputs > + > +properties: > + compatible: > + enum: > + - motion-simple-pwm This should be e.g. ti,drv8873-q1. This device has much more pins that is given in these bindings. If we find more devices that have similar functionality/pinout we can add them to the same bindings, but we will likely find that trying to cram all H-bridges into a single binding is too much. For starters, every H-bridge chip is going to have one or more power supplies. ti,drv8873-q1 would need dvdd-supply and vm-supply properties for the DVDD and VM pins. Many have inputs for disabling the chip, e.g. for power management. And some have outputs to indicate faults. The TI DRV8873 in particular has an nSLEEP, DISABLE, nOL, SR, MODE and nITRIP inputs in addition to the IN1 and IN2 that would be connected to the PWMs. So we would have properties for all of these to either say how the pin is hardwired or a *-gpios property if it needs to be controlled by the driver. The fault output would generally be an interrupts property. The IPROPI1 and IPROPI2 output pins look like they would be connected to an ADC, so it would make sense to have an io-channels property show that connection. This chip also has a SPI interface. So it needs to have the possibility of being a SPI peripheral node. And even if the Linux driver doesn't implement all of these features, we still want the DT bindings to be as complete as possible, so we shouldn't be leaving these out, at least for the trivial ones where there is an obvious correct binding (which I think is the case for most of what I suggested). > + > + pwms: > + maxItems: 2 > + > + pwm-names: > + maxItems: 2 Specifying what is wired up to the IN pins can be tricky. Using two PWMs is the most sensible. But I've also seen devices where there was a single PWM gated by two gpios. And for very basic H-bridges, there might not even be a PWM. Just gpios to turn it on or off. > + > + motion,pwm-inverted: > + $ref: /schemas/types.yaml#/definitions/flag > + description: > + If present, this flag indicates that the PWM signal should be inverted. > + The duty-cycle will be scaled from 100% down to 0% instead 0% to 100%. > + > +required: > + - compatible > + - pwms > + > +allOf: > + - $ref: /schemas/motion/common.yaml# > + > +unevaluatedProperties: false > + > +examples: > + - | > + // This example shows how to use the TI DRV8873 or similar motor controllers > + // with this driver > + motion-simple-pwm0 { > + compatible = "motion-simple-pwm"; > + pwms = <&hpdcm0_pwm 0 50000 0>, > + <&hpdcm0_pwm 1 50000 0>; > + pwm-names = "left", "right"; > + motion,pwm-inverted; > + motion,speed-conv-mul = <3600>; > + motion,speed-conv-div = <100000>; > + motion,acceleration-conv-mul = <3600>; > + motion,acceleration-conv-div = <100000>; This H-bridge controller doesn't have any kind of speed sensors that I can see so these properties don't make sense to me. The H-bridge can control the voltage sent to the motor, but there are more variables involved to convert voltage to speed. It isn't a constant. > + };
On 2/28/25 4:18 PM, Uwe Kleine-König wrote: > Hey David, > > On Fri, Feb 28, 2025 at 11:09:31AM +0100, David Jander wrote: >> On Fri, 28 Feb 2025 10:37:48 +0100 >> Krzysztof Kozlowski <krzk@kernel.org> wrote: >> >>> On 28/02/2025 10:22, David Jander wrote: >>>> >>>>>> + >>>>>> + motion,pwm-inverted: >>>>>> + $ref: /schemas/types.yaml#/definitions/flag >>>>> And PWM flag does not work? >>>> I have seen PWM controllers that don't seem to support the >>>> PWM_POLARITY_INVERTED flag and those where it just doesn't work. Should all >>> >>> Shouldn't the controllers be fixed? Or let's rephrase the question: why >>> only this PWM consumer needs this property and none of others need it? >> CCing Uwe Kleine-Koenig and linux-pwm mailing list. >> >> I know that at least in kernel 6.11 the pwm-stm32.c PWM driver doesn't >> properly invert the PWM signal when specifying PWM_POLARITY_INVERTED. I agree >> this is a probably bug that needs fixing if still present in 6.14-rc. Besides >> that, if linux-pwm agrees that every single PWM driver _must_ properly support >> this flag, I will drop this consumer flag an start fixing broken PWM drivers >> that I encounter. I agree that it makes more sense this way, but I wanted to >> be sure. > Some hardwares cannot support PWM_POLARITY_INVERTED. Affected drivers > include: > > pwm-adp5585 > pwm-ntxec > pwm-raspberrypi-poe > pwm-rz-mtu3 (software limitation only) > pwm-sunplus > pwm-twl-led (not completely sure, that one is strange) > > . ISTR that there is a driver that does only support inverted polarity, > but I don't find it. For an overview I recommend reading through the > output of: The only one that I know of is the opencores pwm driver that the starfive jh71xx uses, I remember talking with you there. That one does still need a proper review I believe: https://lore.kernel.org/all/20250106103540.10079-1-william.qiu@starfivetech.com/ It is kind of in a limbo right now > > for f in drivers/pwm/pwm-*; do > echo $f; > sed -rn '/Limitations:/,/\*\/?$/p' $f; > echo; > done | less > > . (Note not all drivers have commentary in the right format to unveil > their limitations.) > > For most use-cases you can just do > > .duty_cycle = .period - .duty_cycle > > instead of inverting polarity, but there is no abstraction in the PWM > bindings for that and also no helpers in the PWM framework. The problem > is more or less ignored, so if you have a device with > > pwms = <&pwm0 0 PWM_POLARITY_INVERTED>; > > and the PWM chip in question doesn't support that, the pwm API functions > will fail. So the system designer better makes sure that the PWM > hardware can cope with the needed polarity. > > Best regards > Uwe
Dear Uwe, Thanks for chiming in! On Fri, 28 Feb 2025 16:18:05 +0100 Uwe Kleine-König <ukleinek@kernel.org> wrote: > Hey David, > > On Fri, Feb 28, 2025 at 11:09:31AM +0100, David Jander wrote: > > On Fri, 28 Feb 2025 10:37:48 +0100 > > Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > > > On 28/02/2025 10:22, David Jander wrote: > > > > > > > >>> + > > > >>> + motion,pwm-inverted: > > > >>> + $ref: /schemas/types.yaml#/definitions/flag > > > >> > > > >> And PWM flag does not work? > > > > > > > > I have seen PWM controllers that don't seem to support the > > > > PWM_POLARITY_INVERTED flag and those where it just doesn't work. Should all > > > > > > > > > Shouldn't the controllers be fixed? Or let's rephrase the question: why > > > only this PWM consumer needs this property and none of others need it? > > > > CCing Uwe Kleine-Koenig and linux-pwm mailing list. > > > > I know that at least in kernel 6.11 the pwm-stm32.c PWM driver doesn't > > properly invert the PWM signal when specifying PWM_POLARITY_INVERTED. I agree > > this is a probably bug that needs fixing if still present in 6.14-rc. Besides > > that, if linux-pwm agrees that every single PWM driver _must_ properly support > > this flag, I will drop this consumer flag an start fixing broken PWM drivers > > that I encounter. I agree that it makes more sense this way, but I wanted to > > be sure. > > Some hardwares cannot support PWM_POLARITY_INVERTED. Affected drivers > include: > > pwm-adp5585 > pwm-ntxec > pwm-raspberrypi-poe > pwm-rz-mtu3 (software limitation only) > pwm-sunplus > pwm-twl-led (not completely sure, that one is strange) > > . ISTR that there is a driver that does only support inverted polarity, > but I don't find it. For an overview I recommend reading through the > output of: > > for f in drivers/pwm/pwm-*; do > echo $f; > sed -rn '/Limitations:/,/\*\/?$/p' $f; > echo; > done | less > > . (Note not all drivers have commentary in the right format to unveil > their limitations.) > > For most use-cases you can just do > > .duty_cycle = .period - .duty_cycle Yes, that is exactly what the relevant code in motion/simple-pwm.c does when the "pwm-inverted" flag is present in the DT node. > instead of inverting polarity, but there is no abstraction in the PWM > bindings for that and also no helpers in the PWM framework. The problem > is more or less ignored, so if you have a device with > > pwms = <&pwm0 0 PWM_POLARITY_INVERTED>; > > and the PWM chip in question doesn't support that, the pwm API functions > will fail. So the system designer better makes sure that the PWM > hardware can cope with the needed polarity. Thanks for clarifying this! @Krzysztof, do you think that given this situation it is acceptable to include the "pwm-inverted" flag in the dt-schema of the simple PWM motor driver? The need for an inverted PWM signal is something very common in the case of H-bridge motor drivers, where the PWM signal represents the actual logical output level of each of the two halves of the bridge. Often the high-side switches are used as the free-wheel position, so that 100% duty-cycle on both channels is actually standstill, while 0% duty-cycle on one channel is full speed in either direction. This isn't always the case though, hence the importance for this to be able to be selected. Best regards,
Hi David, On Fri, 28 Feb 2025 16:41:29 -0600 David Lechner <dlechner@baylibre.com> wrote: > On 2/27/25 10:28 AM, David Jander wrote: > > Add device-tree bindings for simple Linux Motion Control devices that > > are based on 1 or 2 PWM outputs. > > > > Signed-off-by: David Jander <david@protonic.nl> > > --- > > .../bindings/motion/motion-simple-pwm.yaml | 55 +++++++++++++++++++ > > 1 file changed, 55 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/motion/motion-simple-pwm.yaml > > > > diff --git a/Documentation/devicetree/bindings/motion/motion-simple-pwm.yaml b/Documentation/devicetree/bindings/motion/motion-simple-pwm.yaml > > new file mode 100644 > > index 000000000000..409e3aef6f3f > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/motion/motion-simple-pwm.yaml > > @@ -0,0 +1,55 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/motion/motion-simple-pwm.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Simple PWM based motor controller > > I think it has been said many times before, in DT, there is no such thing as > a simple device! It will be much more future-proof if we write bindings for > actual individual motor controller chips than try to generalize all in a single > binding. The chip you gave as an example is far from the simplest H-bridge I > have seen! To clarify, my intention is not to generalize all existing DC motor controllers into one driver or dt-binding. I mentioned the drv8873, but only as an example. Actually my plan is to make a separate driver and separate bindings for the drv8873, but I haven't had time for that yet, and it would be too much for the first version of LMC. There are a lot of simple "dumb" devices though, that have integrated or even discrete H-bridges with fixed dead-time, that can't do more than just 2 PWM signals to control left-/right-speed. There are also lots of places where people connect a DC motor to just a simple power-MOSFET. All of these cases can be handled by this driver. Maybe the name "simple-pwm" isn't adequate. Should we name it "pwm-motor" or something liket that instead? The intend of motion/simple-pwm.c is to be the analog of something like these other "simple" or "generic" drivers and corresponding dt-bindings, for example: gpio_keys.c: https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/input/keyboard/gpio_keys.c https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/input/gpio-keys.yaml gpio-regulator.c: https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/regulator/gpio-regulator.c https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/regulator/gpio-regulator.yaml pwm-regulator.c: https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/regulator/pwm-regulator.c https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/regulator/pwm-regulator.yaml pwm_bl.c: https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/video/backlight/pwm_bl.c https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/leds/backlight/pwm-backlight.yaml etc... Although the driver actually is simple, and intended for simple hardware. The fact that it can even be used with more complex chips, like the drv8873, if the requirements are simple enough, and as long as there is no dedicated driver yet, doesn't change that fact. > > +maintainers: > > + - David Jander <david@protonic> > > + > > +description: | > > + Simple motor control device based on 1 or 2 PWM outputs > > + > > +properties: > > + compatible: > > + enum: > > + - motion-simple-pwm > > This should be e.g. ti,drv8873-q1. This device has much more pins that is given > in these bindings. Like I said, this driver isn't intended for the drv8873. It was merely an example where as a matter of fact this driver could be used for the drv8873, but that's not the intention. Sorry for not being clear. :-) > If we find more devices that have similar functionality/pinout we can add them > to the same bindings, but we will likely find that trying to cram all H-bridges > into a single binding is too much. > > For starters, every H-bridge chip is going to have one or more power supplies. > ti,drv8873-q1 would need dvdd-supply and vm-supply properties for the DVDD and > VM pins. > > Many have inputs for disabling the chip, e.g. for power management. And some > have outputs to indicate faults. > > The TI DRV8873 in particular has an nSLEEP, DISABLE, nOL, SR, MODE and nITRIP > inputs in addition to the IN1 and IN2 that would be connected to the PWMs. > So we would have properties for all of these to either say how the pin is > hardwired or a *-gpios property if it needs to be controlled by the driver. Yes, sure. These will all be in the dt-binding for the drv8873. Our board actually uses the drv8873s, which has an SPI interface, so that will of course also be part of the bindings and of the driver. > The fault output would generally be an interrupts property. Ack. > The IPROPI1 and IPROPI2 output pins look like they would be connected to an > ADC, so it would make sense to have an io-channels property show that > connection. Ack. In fact, our board has these connected to the internal ADC of the SoC. > This chip also has a SPI interface. So it needs to have the possibility of > being a SPI peripheral node. Sure, like I said above. > And even if the Linux driver doesn't implement all of these features, we still > want the DT bindings to be as complete as possible, so we shouldn't be leaving > these out, at least for the trivial ones where there is an obvious correct > binding (which I think is the case for most of what I suggested). Completely agree. Will be done, but not for this first version. It is simply too much to review, I'm afraid. It will be a separate binding, in motion/ti,drv8873.yaml (not included in this version). > > + > > + pwms: > > + maxItems: 2 > > + > > + pwm-names: > > + maxItems: 2 > > Specifying what is wired up to the IN pins can be tricky. Using two PWMs is > the most sensible. But I've also seen devices where there was a single PWM > gated by two gpios. And for very basic H-bridges, there might not even be a > PWM. Just gpios to turn it on or off. That would be something for a future motion/gpio-motor.yaml, I guess. > > + > > + motion,pwm-inverted: > > + $ref: /schemas/types.yaml#/definitions/flag > > + description: > > + If present, this flag indicates that the PWM signal should be inverted. > > + The duty-cycle will be scaled from 100% down to 0% instead 0% to 100%. > > + > > +required: > > + - compatible > > + - pwms > > + > > +allOf: > > + - $ref: /schemas/motion/common.yaml# > > + > > +unevaluatedProperties: false > > + > > +examples: > > + - | > > + // This example shows how to use the TI DRV8873 or similar motor controllers > > + // with this driver > > + motion-simple-pwm0 { > > + compatible = "motion-simple-pwm"; > > + pwms = <&hpdcm0_pwm 0 50000 0>, > > + <&hpdcm0_pwm 1 50000 0>; > > + pwm-names = "left", "right"; > > + motion,pwm-inverted; > > > > + motion,speed-conv-mul = <3600>; > > + motion,speed-conv-div = <100000>; > > + motion,acceleration-conv-mul = <3600>; > > + motion,acceleration-conv-div = <100000>; > > This H-bridge controller doesn't have any kind of speed sensors that I can see > so these properties don't make sense to me. The H-bridge can control the voltage > sent to the motor, but there are more variables involved to convert voltage to > speed. It isn't a constant. Sure. In the most basic case, when there is no feedback signal for speed (like an encoder for example), the best thing you can do is assume an (imperfect) relation between duty-cycle and motor speed. That's what these factors are for. You could have a reduction gearbox even, so specifying some parameters for the user-space software to work with seems sensible. If you leave them out, the default values are used, which are all 1's, basically making the "speed" value equal to the (scaled) duty-cycle of the PWMs. The driver scales duty-cycle to 1/1000th of a percent, which seemed a sensible resolution to use and be intuitive enough, since due to the lack of floating point numbers in the kernel, it is common practice to scale values by a factor of 1000 to enhance resolution. If you prefer, we could also use parts-per-million, or ppm (1/1,000,000). With this in mind, setting the PWM outputs to 100% duty-cycle (0% if inverted), would correspond to setting a motor speed of 3600 in the case of the example scaling values given in the bindings. Best regards,
On 03/03/2025 12:40, David Jander wrote: >> >> Some hardwares cannot support PWM_POLARITY_INVERTED. Affected drivers >> include: >> >> pwm-adp5585 >> pwm-ntxec >> pwm-raspberrypi-poe >> pwm-rz-mtu3 (software limitation only) >> pwm-sunplus >> pwm-twl-led (not completely sure, that one is strange) >> >> . ISTR that there is a driver that does only support inverted polarity, >> but I don't find it. For an overview I recommend reading through the >> output of: >> >> for f in drivers/pwm/pwm-*; do >> echo $f; >> sed -rn '/Limitations:/,/\*\/?$/p' $f; >> echo; >> done | less >> >> . (Note not all drivers have commentary in the right format to unveil >> their limitations.) >> >> For most use-cases you can just do >> >> .duty_cycle = .period - .duty_cycle > > Yes, that is exactly what the relevant code in motion/simple-pwm.c does when > the "pwm-inverted" flag is present in the DT node. > >> instead of inverting polarity, but there is no abstraction in the PWM >> bindings for that and also no helpers in the PWM framework. The problem >> is more or less ignored, so if you have a device with >> >> pwms = <&pwm0 0 PWM_POLARITY_INVERTED>; >> >> and the PWM chip in question doesn't support that, the pwm API functions >> will fail. So the system designer better makes sure that the PWM >> hardware can cope with the needed polarity. > > Thanks for clarifying this! > > @Krzysztof, do you think that given this situation it is acceptable to include > the "pwm-inverted" flag in the dt-schema of the simple PWM motor driver? No, because that flag is already supported via PWM_POLARITY_INVERTED. Do not tie bindings to specific implementation. If PWM core is changed to always handle PWM_POLARITY_INVERTED even if controller does not support it, would the binding became outdated? > > The need for an inverted PWM signal is something very common in the case of > H-bridge motor drivers, where the PWM signal represents the actual logical > output level of each of the two halves of the bridge. Often the high-side > switches are used as the free-wheel position, so that 100% duty-cycle on both > channels is actually standstill, while 0% duty-cycle on one channel is full > speed in either direction. This isn't always the case though, hence the > importance for this to be able to be selected. Sure and use existing bindings for that. If implementation has problems, fix implementation. Best regards, Krzysztof
On Mon, 3 Mar 2025 15:18:40 +0100 Krzysztof Kozlowski <krzk@kernel.org> wrote: > On 03/03/2025 12:40, David Jander wrote: > >> > >> Some hardwares cannot support PWM_POLARITY_INVERTED. Affected drivers > >> include: > >> > >> pwm-adp5585 > >> pwm-ntxec > >> pwm-raspberrypi-poe > >> pwm-rz-mtu3 (software limitation only) > >> pwm-sunplus > >> pwm-twl-led (not completely sure, that one is strange) > >> > >> . ISTR that there is a driver that does only support inverted polarity, > >> but I don't find it. For an overview I recommend reading through the > >> output of: > >> > >> for f in drivers/pwm/pwm-*; do > >> echo $f; > >> sed -rn '/Limitations:/,/\*\/?$/p' $f; > >> echo; > >> done | less > >> > >> . (Note not all drivers have commentary in the right format to unveil > >> their limitations.) > >> > >> For most use-cases you can just do > >> > >> .duty_cycle = .period - .duty_cycle > > > > Yes, that is exactly what the relevant code in motion/simple-pwm.c does when > > the "pwm-inverted" flag is present in the DT node. > > > >> instead of inverting polarity, but there is no abstraction in the PWM > >> bindings for that and also no helpers in the PWM framework. The problem > >> is more or less ignored, so if you have a device with > >> > >> pwms = <&pwm0 0 PWM_POLARITY_INVERTED>; > >> > >> and the PWM chip in question doesn't support that, the pwm API functions > >> will fail. So the system designer better makes sure that the PWM > >> hardware can cope with the needed polarity. > > > > Thanks for clarifying this! > > > > @Krzysztof, do you think that given this situation it is acceptable to include > > the "pwm-inverted" flag in the dt-schema of the simple PWM motor driver? > > No, because that flag is already supported via PWM_POLARITY_INVERTED. > > Do not tie bindings to specific implementation. If PWM core is changed > to always handle PWM_POLARITY_INVERTED even if controller does not > support it, would the binding became outdated? Understood. I guess I will have to start fixing PWM drivers then... @uwe, do you have objections to trying to add a (very trivial) helper for PWM controllers that can't do inversion in hardware and start to patch some of them if needed? > > The need for an inverted PWM signal is something very common in the case of > > H-bridge motor drivers, where the PWM signal represents the actual logical > > output level of each of the two halves of the bridge. Often the high-side > > switches are used as the free-wheel position, so that 100% duty-cycle on both > > channels is actually standstill, while 0% duty-cycle on one channel is full > > speed in either direction. This isn't always the case though, hence the > > importance for this to be able to be selected. > > Sure and use existing bindings for that. If implementation has problems, > fix implementation. Sounds reasonable. Best regards,
diff --git a/Documentation/devicetree/bindings/motion/motion-simple-pwm.yaml b/Documentation/devicetree/bindings/motion/motion-simple-pwm.yaml new file mode 100644 index 000000000000..409e3aef6f3f --- /dev/null +++ b/Documentation/devicetree/bindings/motion/motion-simple-pwm.yaml @@ -0,0 +1,55 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/motion/motion-simple-pwm.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Simple PWM based motor controller + +maintainers: + - David Jander <david@protonic> + +description: | + Simple motor control device based on 1 or 2 PWM outputs + +properties: + compatible: + enum: + - motion-simple-pwm + + pwms: + maxItems: 2 + + pwm-names: + maxItems: 2 + + motion,pwm-inverted: + $ref: /schemas/types.yaml#/definitions/flag + description: + If present, this flag indicates that the PWM signal should be inverted. + The duty-cycle will be scaled from 100% down to 0% instead 0% to 100%. + +required: + - compatible + - pwms + +allOf: + - $ref: /schemas/motion/common.yaml# + +unevaluatedProperties: false + +examples: + - | + // This example shows how to use the TI DRV8873 or similar motor controllers + // with this driver + motion-simple-pwm0 { + compatible = "motion-simple-pwm"; + pwms = <&hpdcm0_pwm 0 50000 0>, + <&hpdcm0_pwm 1 50000 0>; + pwm-names = "left", "right"; + motion,pwm-inverted; + motion,speed-conv-mul = <3600>; + motion,speed-conv-div = <100000>; + motion,acceleration-conv-mul = <3600>; + motion,acceleration-conv-div = <100000>; + };
Add device-tree bindings for simple Linux Motion Control devices that are based on 1 or 2 PWM outputs. Signed-off-by: David Jander <david@protonic.nl> --- .../bindings/motion/motion-simple-pwm.yaml | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 Documentation/devicetree/bindings/motion/motion-simple-pwm.yaml