[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d329ec5e035ec89944a73457bf14111a7318b6e9.camel@gmail.com>
Date: Tue, 17 Dec 2024 12:36:27 +0100
From: Nuno Sá <noname.nuno@...il.com>
To: David Lechner <dlechner@...libre.com>, Mark Brown <broonie@...nel.org>,
Jonathan Cameron
<jic23@...nel.org>, Rob Herring <robh@...nel.org>, Krzysztof Kozlowski
<krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Nuno
Sá
<nuno.sa@...log.com>
Cc: Uwe Kleine-König <ukleinek@...nel.org>, Michael
Hennerich <Michael.Hennerich@...log.com>, Lars-Peter Clausen
<lars@...afoo.de>, David Jander <david@...tonic.nl>, Martin Sperl
<kernel@...tin.sperl.org>, linux-spi@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-iio@...r.kernel.org, linux-pwm@...r.kernel.org, Jonathan Cameron
<Jonathan.Cameron@...wei.com>
Subject: Re: [PATCH v6 04/17] spi: offload-trigger: add PWM trigger driver
On Wed, 2024-12-11 at 14:54 -0600, David Lechner wrote:
> Add a new driver for a generic PWM trigger for SPI offloads.
>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>
> Signed-off-by: David Lechner <dlechner@...libre.com>
> ---
Still don't really agree with always checking against
SPI_OFFLOAD_TRIGGER_PERIODIC. But yeah...
Reviewed-by: Nuno Sa <nuno.sa@...log.com>
>
> v6 changes:
> * Use dev instead of &pdev->dev
> * Swap order of "pwm" and "trigger" in name to follow "pwm-clock"
> precedent.
>
> v5 changes:
> * Updated to accommodate changes in other patches in this series.
> * Add MAINTAINERS entry.
>
> v4 changes: new patch in v4
> ---
> MAINTAINERS | 1 +
> drivers/spi/Kconfig | 12 +++
> drivers/spi/Makefile | 3 +
> drivers/spi/spi-offload-trigger-pwm.c | 162
> ++++++++++++++++++++++++++++++++++
> 4 files changed, 178 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index
> b2aa6f37743e48353c60e5973ea8126590c7f6d5..d8d72da5ac4bcab817a515774eb8db37a7e9
> 4f25 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -22131,6 +22131,7 @@ F: include/linux/mtd/spi-nor.h
>
> SPI OFFLOAD
> R: David Lechner <dlechner@...libre.com>
> +F: drivers/spi/spi-offload-trigger-pwm.c
> F: drivers/spi/spi-offload.c
> F: include/linux/spi/spi-offload.h
> K: spi_offload
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index
> 02064a4e292815ec0213e2e446b4f90ed8855a52..2cfc14be869790f5226130428bb7cb40aadf
> b031 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -1320,4 +1320,16 @@ endif # SPI_SLAVE
> config SPI_DYNAMIC
> def_bool ACPI || OF_DYNAMIC || SPI_SLAVE
>
> +if SPI_OFFLOAD
> +
> +comment "SPI Offload triggers"
> +
> +config SPI_OFFLOAD_TRIGGER_PWM
> + tristate "SPI offload trigger using PWM"
> + depends on PWM
> + help
> + Generic SPI offload trigger implemented using PWM output.
> +
> +endif # SPI_OFFLOAD
> +
> endif # SPI
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index
> bb5fc20df21332232533c2e70c0cc230f6bcf27f..0068d170bc99c750c13376c4013991d927bb
> ac63 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -164,3 +164,6 @@ obj-$(CONFIG_SPI_AMD) += spi-amd.o
> # SPI slave protocol handlers
> obj-$(CONFIG_SPI_SLAVE_TIME) += spi-slave-time.o
> obj-$(CONFIG_SPI_SLAVE_SYSTEM_CONTROL) += spi-slave-system-control.o
> +
> +# SPI offload triggers
> +obj-$(CONFIG_SPI_OFFLOAD_TRIGGER_PWM) += spi-offload-trigger-pwm.o
> diff --git a/drivers/spi/spi-offload-trigger-pwm.c b/drivers/spi/spi-offload-
> trigger-pwm.c
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..b26d4437c589052709a8206f8314ffd08355
> 870e
> --- /dev/null
> +++ b/drivers/spi/spi-offload-trigger-pwm.c
> @@ -0,0 +1,162 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2024 Analog Devices Inc.
> + * Copyright (C) 2024 BayLibre, SAS
> + *
> + * Generic PWM trigger for SPI offload.
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/pwm.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/spi/offload/provider.h>
> +#include <linux/types.h>
> +
> +struct spi_offload_trigger_pwm_state {
> + struct device *dev;
> + struct pwm_device *pwm;
> +};
> +
> +static bool spi_offload_trigger_pwm_match(struct spi_offload_trigger
> *trigger,
> + enum spi_offload_trigger_type type,
> + u64 *args, u32 nargs)
> +{
> + if (nargs)
> + return false;
> +
> + return type == SPI_OFFLOAD_TRIGGER_PERIODIC;
> +}
> +
> +static int spi_offload_trigger_pwm_validate(struct spi_offload_trigger
> *trigger,
> + struct spi_offload_trigger_config
> *config)
> +{
> + struct spi_offload_trigger_pwm_state *st =
> spi_offload_trigger_get_priv(trigger);
> + struct spi_offload_trigger_periodic *periodic = &config->periodic;
> + struct pwm_waveform wf = { };
> + int ret;
> +
> + if (config->type != SPI_OFFLOAD_TRIGGER_PERIODIC)
> + return -EINVAL;
> +
> + if (!periodic->frequency_hz)
> + return -EINVAL;
> +
> + wf.period_length_ns = DIV_ROUND_UP_ULL(NSEC_PER_SEC, periodic-
> >frequency_hz);
> + /* REVISIT: 50% duty-cycle for now - may add config parameter later
> */
> + wf.duty_length_ns = wf.period_length_ns / 2;
> +
> + ret = pwm_round_waveform_might_sleep(st->pwm, &wf);
> + if (ret < 0)
> + return ret;
> +
> + periodic->frequency_hz = DIV_ROUND_UP_ULL(NSEC_PER_SEC,
> wf.period_length_ns);
> +
> + return 0;
> +}
> +
> +static int spi_offload_trigger_pwm_enable(struct spi_offload_trigger
> *trigger,
> + struct spi_offload_trigger_config
> *config)
> +{
> + struct spi_offload_trigger_pwm_state *st =
> spi_offload_trigger_get_priv(trigger);
> + struct spi_offload_trigger_periodic *periodic = &config->periodic;
> + struct pwm_waveform wf = { };
> +
> + if (config->type != SPI_OFFLOAD_TRIGGER_PERIODIC)
> + return -EINVAL;
> +
> + if (!periodic->frequency_hz)
> + return -EINVAL;
> +
> + wf.period_length_ns = DIV_ROUND_UP_ULL(NSEC_PER_SEC, periodic-
> >frequency_hz);
> + /* REVISIT: 50% duty-cycle for now - may add config parameter later
> */
> + wf.duty_length_ns = wf.period_length_ns / 2;
> +
> + return pwm_set_waveform_might_sleep(st->pwm, &wf, false);
> +}
> +
> +static void spi_offload_trigger_pwm_disable(struct spi_offload_trigger
> *trigger)
> +{
> + struct spi_offload_trigger_pwm_state *st =
> spi_offload_trigger_get_priv(trigger);
> + struct pwm_waveform wf;
> + int ret;
> +
> + ret = pwm_get_waveform_might_sleep(st->pwm, &wf);
> + if (ret < 0) {
> + dev_err(st->dev, "failed to get waveform: %d\n", ret);
> + return;
> + }
> +
> + wf.duty_length_ns = 0;
> +
> + ret = pwm_set_waveform_might_sleep(st->pwm, &wf, false);
> + if (ret < 0)
> + dev_err(st->dev, "failed to disable PWM: %d\n", ret);
> +}
> +
> +static const struct spi_offload_trigger_ops spi_offload_trigger_pwm_ops = {
> + .match = spi_offload_trigger_pwm_match,
> + .validate = spi_offload_trigger_pwm_validate,
> + .enable = spi_offload_trigger_pwm_enable,
> + .disable = spi_offload_trigger_pwm_disable,
> +};
> +
> +static void spi_offload_trigger_pwm_release(void *data)
> +{
> + pwm_disable(data);
> +}
> +
> +static int spi_offload_trigger_pwm_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct spi_offload_trigger_info info = {
> + .fwnode = dev_fwnode(dev),
> + .ops = &spi_offload_trigger_pwm_ops,
> + };
> + struct spi_offload_trigger_pwm_state *st;
> + struct pwm_state state;
> + int ret;
> +
> + st = devm_kzalloc(dev, sizeof(*st), GFP_KERNEL);
> + if (!st)
> + return -ENOMEM;
> +
> + info.priv = st;
> + st->dev = dev;
> +
> + st->pwm = devm_pwm_get(dev, NULL);
> + if (IS_ERR(st->pwm))
> + return dev_err_probe(dev, PTR_ERR(st->pwm), "failed to get
> PWM\n");
> +
> + /* init with duty_cycle = 0, output enabled to ensure trigger off */
> + pwm_init_state(st->pwm, &state);
> + state.enabled = true;
> +
> + ret = pwm_apply_might_sleep(st->pwm, &state);
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "failed to apply PWM
> state\n");
> +
> + ret = devm_add_action_or_reset(dev, spi_offload_trigger_pwm_release,
> st->pwm);
> + if (ret)
> + return ret;
> +
> + return devm_spi_offload_trigger_register(dev, &info);
> +}
> +
> +static const struct of_device_id spi_offload_trigger_pwm_of_match_table[] = {
> + { .compatible = "pwm-trigger" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, spi_offload_trigger_pwm_of_match_table);
> +
> +static struct platform_driver spi_offload_trigger_pwm_driver = {
> + .driver = {
> + .name = "pwm-trigger",
> + .of_match_table = spi_offload_trigger_pwm_of_match_table,
> + },
> + .probe = spi_offload_trigger_pwm_probe,
> +};
> +module_platform_driver(spi_offload_trigger_pwm_driver);
> +
> +MODULE_AUTHOR("David Lechner <dlechner@...libre.com>");
> +MODULE_DESCRIPTION("Generic PWM trigger");
> +MODULE_LICENSE("GPL");
>
Powered by blists - more mailing lists