Linux-PWM Archive mirror
 help / color / mirror / Atom feed
From: Trevor Gamblin <tgamblin@baylibre.com>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: linux-pwm@vger.kernel.org, linux-kernel@vger.kernel.org,
	michael.hennerich@analog.com, nuno.sa@analog.com,
	devicetree@vger.kernel.org, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	Drew Fustini <dfustini@baylibre.com>,
	Sergiu Cuciurean <sergiu.cuciurean@analog.com>,
	David Lechner <dlechner@baylibre.com>
Subject: Re: [PATCH 2/2 v3] pwm: Add driver for AXI PWM generator
Date: Tue, 27 Feb 2024 13:54:15 -0500	[thread overview]
Message-ID: <6aca1272-8a39-4b8a-b09a-79bc085362dc@baylibre.com> (raw)
In-Reply-To: <rqozm7c4xixq3mb7sod7uslceieme7jrtdj2yelrekrqnttch2@vpsbe2nqmdsd>


On 2024-02-26 05:13, Uwe Kleine-König wrote:
> On Wed, Jan 31, 2024 at 04:40:41PM -0500, Trevor Gamblin wrote:
>> From: Drew Fustini <dfustini@baylibre.com>
>>
>> Add support for the Analog Devices AXI PWM Generator. This device is an
>> FPGA-implemented peripheral used as PWM signal generator and can be
>> interfaced with AXI4. The register map of this peripheral makes it
>> possible to configure the period and duty cycle of the output signal.
>>
>> Link: https://wiki.analog.com/resources/fpga/docs/axi_pwm_gen
>> Co-developed-by: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
>> Signed-off-by: Sergiu Cuciurean <sergiu.cuciurean@analog.com>
>> Co-developed-by: David Lechner <dlechner@baylibre.com>
>> Signed-off-by: David Lechner <dlechner@baylibre.com>
>> Signed-off-by: Drew Fustini <dfustini@baylibre.com>
>> Co-developed-by: Trevor Gamblin <tgamblin@baylibre.com>
>> Acked-by: Nuno Sa <nuno.sa@analog.com>
>> Signed-off-by: Trevor Gamblin <tgamblin@baylibre.com>
>> ---
>> v3 changes:
>> * Address feedback for driver in v2:
>>    * Remove unnecessary blank line in axi_pwmgen_apply
>>    * Use macros already defined in <linux/fpga/adi-axi-common.h> for
>>      version checking
>>
>> v2 changes:
>> * Address feedback for driver and device tree in v1:
>>    * Use more reasonable Kconfig approach
>>    * Use common prefixes for all functions
>>    * Rename axi_pwmgen struct to axi_pwmgen_ddata
>>    * Change use of "pwm" to "ddata"
>>    * Set and check state->polarity
>>    * Multiply safely with mul_u64_u64_div_u64()
>>    * Improve handling of max and zero periods
>>    * Error if clk_rate_hz > NSEC_PER_SEC
>>    * Add "Limitations" section at top of pwm-axi-pwmgen.c
>>    * Don't disable outputs by default
>>    * Remove unnecessary macros for period, duty, offset
>>    * Fix axi_pwmgen_ddata alignment
>>    * Don't artificially limit npwm to four
>>    * Use clk_rate_exclusive_get(), balance with clk_rate_exclusive_put()
>>    * Cache clk rate in axi_pwmgen_ddata
>>    * Don't assign pwm->chip.base, do assign pwm->chip.atomic
>> * Remove redundant calls to clk_get_rate
>> * Test contents of AXI_PWMGEN_REG_CORE_MAGIC instead of
>>    arbitrary AXI_PWMGEN_TEST_DATA in AXI_PWMGEN_REG_SCRATCHPAD
>> * Remove redundant clk struct from axi_pwmgen_ddata
>> * Add self as module author
>> * Add major version check for IP core
>>
>> ---
>>   MAINTAINERS                  |   1 +
>>   drivers/pwm/Kconfig          |  13 ++
>>   drivers/pwm/Makefile         |   1 +
>>   drivers/pwm/pwm-axi-pwmgen.c | 242 +++++++++++++++++++++++++++++++++++
>>   4 files changed, 257 insertions(+)
>>   create mode 100644 drivers/pwm/pwm-axi-pwmgen.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 8a4ed5545680..2baa7a0a1c8c 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -3438,6 +3438,7 @@ L:	linux-pwm@vger.kernel.org
>>   S:	Supported
>>   W:	https://ez.analog.com/linux-software-drivers
>>   F:	Documentation/devicetree/bindings/pwm/adi,axi-pwmgen.yaml
>> +F:	drivers/pwm/pwm-axi-pwmgen.c
>>   
>>   AXXIA I2C CONTROLLER
>>   M:	Krzysztof Adamski <krzysztof.adamski@nokia.com>
>> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
>> index 4b956d661755..d44b0e86adee 100644
>> --- a/drivers/pwm/Kconfig
>> +++ b/drivers/pwm/Kconfig
>> @@ -98,6 +98,19 @@ config PWM_ATMEL_TCB
>>   	  To compile this driver as a module, choose M here: the module
>>   	  will be called pwm-atmel-tcb.
>>   
>> +config PWM_AXI_PWMGEN
>> +	tristate "Analog Devices AXI PWM generator"
>> +	depends on MICROBLAZE || NIOS2 || ARCH_ZYNQ || ARCH_ZYNQMP || ARCH_INTEL_SOCFPGA || COMPILE_TEST
>> +	select REGMAP_MMIO
>> +	help
>> +	  This enables support for the Analog Devices AXI PWM generator.
>> +
>> +	  This is a configurable PWM generator with variable pulse width and
>> +	  period.
>> +
>> +	  To compile this driver as a module, choose M here: the module will be
>> +	  called pwm-axi-pwmgen.
>> +
>>   config PWM_BCM_IPROC
>>   	tristate "iProc PWM support"
>>   	depends on ARCH_BCM_IPROC || COMPILE_TEST
>> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
>> index c5ec9e168ee7..8322089954e9 100644
>> --- a/drivers/pwm/Makefile
>> +++ b/drivers/pwm/Makefile
>> @@ -6,6 +6,7 @@ obj-$(CONFIG_PWM_APPLE)		+= pwm-apple.o
>>   obj-$(CONFIG_PWM_ATMEL)		+= pwm-atmel.o
>>   obj-$(CONFIG_PWM_ATMEL_HLCDC_PWM)	+= pwm-atmel-hlcdc.o
>>   obj-$(CONFIG_PWM_ATMEL_TCB)	+= pwm-atmel-tcb.o
>> +obj-$(CONFIG_PWM_AXI_PWMGEN)	+= pwm-axi-pwmgen.o
>>   obj-$(CONFIG_PWM_BCM_IPROC)	+= pwm-bcm-iproc.o
>>   obj-$(CONFIG_PWM_BCM_KONA)	+= pwm-bcm-kona.o
>>   obj-$(CONFIG_PWM_BCM2835)	+= pwm-bcm2835.o
>> diff --git a/drivers/pwm/pwm-axi-pwmgen.c b/drivers/pwm/pwm-axi-pwmgen.c
>> new file mode 100644
>> index 000000000000..44e62e90b227
>> --- /dev/null
>> +++ b/drivers/pwm/pwm-axi-pwmgen.c
>> @@ -0,0 +1,242 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Analog Devices AXI PWM generator
>> + *
>> + * Copyright 2024 Analog Devices Inc.
>> + * Copyright 2024 Baylibre SAS
>> + *
>> + * Limitations:
>> + * - The writes to registers for period and duty are shadowed until
>> + *   LOAD_CONFIG is written to AXI_PWMGEN_REG_CONFIG at the end of the
>> + *   current period.
>> + * - Writing LOAD_CONFIG also has the effect of re-synchronizing all
>> + *   enabled channels, which could cause glitching on other channels. It
>> + *   is therefore expected that channels are assigned harmonic periods
>> + *   and all have a single user coordinating this.
> That means that when I reconfigure pwm2 the currently running period of
> pwm2 is completed and then all pwmX's counters are reset to zero?
Right.
>
> The patch looks good to me now. It needs a rework to use
> pwmchip_alloc(). If you could care for that, that would be great.
> Otherwise I'd pick up your patch later and add the conversion myself.
Will do. Working on the change now. Once I've done some testing I'll 
submit the v4.
>
> Best regards
> Uwe
>

      reply	other threads:[~2024-02-27 18:54 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-31 21:40 [PATCH 0/2 v3] pwm: add axi-pwm-gen driver Trevor Gamblin
2024-01-31 21:40 ` [PATCH 1/2 v3] dt-bindings: pwm: Add AXI PWM generator Trevor Gamblin
2024-01-31 21:40 ` [PATCH 2/2 v3] pwm: Add driver for " Trevor Gamblin
2024-02-26 10:13   ` Uwe Kleine-König
2024-02-27 18:54     ` Trevor Gamblin [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6aca1272-8a39-4b8a-b09a-79bc085362dc@baylibre.com \
    --to=tgamblin@baylibre.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dfustini@baylibre.com \
    --cc=dlechner@baylibre.com \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=michael.hennerich@analog.com \
    --cc=nuno.sa@analog.com \
    --cc=robh+dt@kernel.org \
    --cc=sergiu.cuciurean@analog.com \
    --cc=u.kleine-koenig@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).