lkml.org 
[lkml]   [2023]   [Feb]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v3 1/3] misc: servo-pwm: driver for controlling servo motors via PWM
    On Fri, Feb 17, 2023 at 05:10:35PM +0100, Angelo Compagnucci wrote:
    > This patch adds a simple driver to control servo motor position via
    > PWM signal.
    > The driver allows to set the angle from userspace, while min/max
    > positions duty cycle and the motor degrees aperture are defined in
    > the dts.
    >
    > Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com>
    > ---
    > v2:
    > * Driver mostly rewritten for kernel 6.2
    > v3:
    > * Fixed sysfs_emit (greg k-h)
    >
    > MAINTAINERS | 6 ++
    > drivers/misc/Kconfig | 11 +++
    > drivers/misc/Makefile | 1 +
    > drivers/misc/servo-pwm.c | 149 +++++++++++++++++++++++++++++++++++++++
    > 4 files changed, 167 insertions(+)
    > create mode 100644 drivers/misc/servo-pwm.c
    >
    > diff --git a/MAINTAINERS b/MAINTAINERS
    > index 39ff1a717625..8f4af64deb1b 100644
    > --- a/MAINTAINERS
    > +++ b/MAINTAINERS
    > @@ -8737,6 +8737,12 @@ F: Documentation/devicetree/bindings/power/power?domain*
    > F: drivers/base/power/domain*.c
    > F: include/linux/pm_domain.h
    >
    > +GENERIC PWM SERVO DRIVER
    > +M: "Angelo Compagnucci" <angelo@amarulasolutions.com>
    > +L: linux-pwm@vger.kernel.org
    > +S: Maintained
    > +F: drivers/misc/servo-pwm.c
    > +
    > GENERIC RESISTIVE TOUCHSCREEN ADC DRIVER
    > M: Eugen Hristev <eugen.hristev@microchip.com>
    > L: linux-input@vger.kernel.org
    > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
    > index 9947b7892bd5..8a74087149ac 100644
    > --- a/drivers/misc/Kconfig
    > +++ b/drivers/misc/Kconfig
    > @@ -518,6 +518,17 @@ config VCPU_STALL_DETECTOR
    >
    > If you do not intend to run this kernel as a guest, say N.
    >
    > +config SERVO_PWM
    > + tristate "Servo motor positioning"
    > + depends on PWM
    > + help
    > + Driver to control generic servo motor positioning.
    > + Writing to the "angle" device attribute, the motor will move to
    > + the angle position.
    > +
    > + To compile this driver as a module, choose M here: the module
    > + will be called servo-pwm.
    > +
    > source "drivers/misc/c2port/Kconfig"
    > source "drivers/misc/eeprom/Kconfig"
    > source "drivers/misc/cb710/Kconfig"
    > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
    > index 87b54a4a4422..936629b648a9 100644
    > --- a/drivers/misc/Makefile
    > +++ b/drivers/misc/Makefile
    > @@ -64,3 +64,4 @@ obj-$(CONFIG_HI6421V600_IRQ) += hi6421v600-irq.o
    > obj-$(CONFIG_OPEN_DICE) += open-dice.o
    > obj-$(CONFIG_GP_PCI1XXXX) += mchp_pci1xxxx/
    > obj-$(CONFIG_VCPU_STALL_DETECTOR) += vcpu_stall_detector.o
    > +obj-$(CONFIG_SERVO_PWM) += servo-pwm.o
    > diff --git a/drivers/misc/servo-pwm.c b/drivers/misc/servo-pwm.c
    > new file mode 100644
    > index 000000000000..1303ddda8d07
    > --- /dev/null
    > +++ b/drivers/misc/servo-pwm.c
    > @@ -0,0 +1,149 @@
    > +// SPDX-License-Identifier: GPL-2.0-or-later
    > +/*
    > + * Copyright (c) 2023 Angelo Compagnucci <angelo@amarulasolutions.com>
    > + * servo-pwm.c - driver for controlling servo motors via pwm.
    > + */
    > +
    > +#include <linux/module.h>
    > +#include <linux/kernel.h>
    > +#include <linux/platform_device.h>
    > +#include <linux/err.h>
    > +#include <linux/pwm.h>
    > +
    > +#define DEFAULT_DUTY_MIN 500000
    > +#define DEFAULT_DUTY_MAX 2500000
    > +#define DEFAULT_DEGREES 175
    > +#define DEFAULT_ANGLE 0
    > +
    > +struct servo_pwm_data {
    > + u32 duty_min;
    > + u32 duty_max;
    > + u32 degrees;
    > + u32 angle;
    > +
    > + struct mutex lock;
    > + struct pwm_device *pwm;
    > + struct pwm_state pwmstate;
    > +};
    > +
    > +static int servo_pwm_set(struct servo_pwm_data *data, int val)
    > +{
    > + u64 new_duty = (((data->duty_max - data->duty_min) /
    > + data->degrees) * val) + data->duty_min;

    This one formula is basically the only thing that this driver adds. The
    remaining 150+ lines are essentially boilerplate to expose the "angle"
    property via sysfs.

    We can already do everything that this driver does via the PWM sysfs, so
    I wonder if we really need this.

    Also, how are other aspects of the motor (such as velocity) controlled?
    Wouldn't you want to expose these other controls as well?

    Thierry
    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2023-03-27 00:28    [W:2.193 / U:0.408 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site