Hi David, On Fri, 28 Feb 2025 16:41:29 -0600 David Lechner <dlechner@xxxxxxxxxxxx> 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@xxxxxxxxxxx> > > --- > > .../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, -- David Jander
