mbox series

[v4,0/7] clk: clk-axi-clkgen: improvements and some fixes

Message ID 20250505-dev-axi-clkgen-limits-v4-0-3ad5124e19e1@analog.com
Headers show
Series clk: clk-axi-clkgen: improvements and some fixes | expand

Message

Nuno Sá via B4 Relay May 5, 2025, 4:41 p.m. UTC
This series starts with a small fix and then a bunch of small
improvements. The main change though is to allow detecting of
struct axi_clkgen_limits during probe().

---
Changes in v4:
- Patch 3:
    * New patch to move the adi-axi-common header. 
- Link to v3: https://lore.kernel.org/r/20250421-dev-axi-clkgen-limits-v3-0-4203b4fed2c9@analog.com

Changes in v3:
- Patch 6:
    * Revert change and parenthesis back on 'if (((params->edge == 0) ^
      (frac_divider == 1))'. While checkpatch complains, it's more
      readable like this and in some configs we might even get -Wparentheses.
- Link to v2: https://lore.kernel.org/r/20250313-dev-axi-clkgen-limits-v2-0-173ae2ad6311@analog.com

Changes in v2:
- Patch 3
   * Rename adi_axi_fgpa_technology -> adi_axi_fpga_technology.

- Link to v1: https://lore.kernel.org/r/20250219-dev-axi-clkgen-limits-v1-0-26f7ef14cd9c@analog.com

---
Nuno Sá (7):
      clk: clk-axi-clkgen: fix fpfd_max frequency for zynq
      clk: clk-axi-clkgen: make sure to include mod_devicetable.h
      include: linux: move adi-axi-common.h out of fpga
      include: fpga: adi-axi-common: add new helper macros
      clk: clk-axi-clkgen: detect axi_clkgen_limits at runtime
      clk: clk-axi-clkgen move to min/max()
      clk: clk-axi-clkgen: fix coding style issues

 drivers/clk/clk-axi-clkgen.c        | 147 +++++++++++++++++++++++++-----------
 drivers/dma/dma-axi-dmac.c          |   2 +-
 drivers/hwmon/axi-fan-control.c     |   2 +-
 drivers/iio/adc/adi-axi-adc.c       |   3 +-
 drivers/iio/dac/adi-axi-dac.c       |   2 +-
 drivers/pwm/pwm-axi-pwmgen.c        |   2 +-
 drivers/spi/spi-axi-spi-engine.c    |   2 +-
 include/linux/adi-axi-common.h      |  58 ++++++++++++++
 include/linux/fpga/adi-axi-common.h |  23 ------
 9 files changed, 169 insertions(+), 72 deletions(-)
---
base-commit: 82f69876ef45ad66c0b114b786c7c6ac0f6a4580
change-id: 20250218-dev-axi-clkgen-limits-63fb0c5ec38b
--

Thanks!
- Nuno Sá

Comments

David Lechner May 5, 2025, 5:21 p.m. UTC | #1
On 5/5/25 11:41 AM, Nuno Sá via B4 Relay wrote:
> From: Nuno Sá <nuno.sa@analog.com>
> 
> Add new helper macros and enums to help identifying the platform and some
> characteristics of it at runtime.
> 
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> ---
>  include/linux/adi-axi-common.h | 35 +++++++++++++++++++++++++++++++++++

Since this file was moved in the previous patch, should we drop "fpga:" from the
subject of this patch?

>  1 file changed, 35 insertions(+)
> 
> diff --git a/include/linux/adi-axi-common.h b/include/linux/adi-axi-common.h
> index 141ac3f251e6f256526812b9d55cd440a2a46e76..a832ef9b37473ca339a2a2ff8a4a5716d428fd29 100644
> --- a/include/linux/adi-axi-common.h
> +++ b/include/linux/adi-axi-common.h
> @@ -12,6 +12,8 @@
>  #define ADI_AXI_COMMON_H_
>  
>  #define ADI_AXI_REG_VERSION			0x0000
> +#define ADI_AXI_REG_FPGA_INFO			0x001C
> +#define ADI_AXI_REG_FPGA_VOLTAGE		0x0140

Doesn't the voltage register only apply to AXI CLKGEN and therefore would
belong in the clock driver rather than here? 0x1C seems to be the last of the
defined "common to all IP cores" address before we possibly get into
core-specific register definitions starting at 0x40.

I guess there are 1 or 2 other cores that define it at the same place, but it
still seems not-global.

>  
>  #define ADI_AXI_PCORE_VER(major, minor, patch)	\
>  	(((major) << 16) | ((minor) << 8) | (patch))
> @@ -20,4 +22,37 @@
>  #define ADI_AXI_PCORE_VER_MINOR(version)	(((version) >> 8) & 0xff)
>  #define ADI_AXI_PCORE_VER_PATCH(version)	((version) & 0xff)
>  
> +#define ADI_AXI_INFO_FPGA_TECH(info)            (((info) >> 24) & 0xff)
> +#define ADI_AXI_INFO_FPGA_FAMILY(info)          (((info) >> 16) & 0xff)
> +#define ADI_AXI_INFO_FPGA_SPEED_GRADE(info)     (((info) >> 8) & 0xff)

I guess we don't care about the DEV_PACKAGE field?

> +#define ADI_AXI_INFO_FPGA_VOLTAGE(val)          ((val) & 0xffff)

This VOLTAGE also goes applies to the first comment.
Jonathan Cameron May 5, 2025, 6:30 p.m. UTC | #2
On Mon, 05 May 2025 17:41:34 +0100
Nuno Sá via B4 Relay <devnull+nuno.sa.analog.com@kernel.org> wrote:

> From: Nuno Sá <nuno.sa@analog.com>
> 
> The adi-axi-common.h header has some common defines used in various ADI
> IPs. However they are not specific for any fpga manager so it's
> questionable for the header to live under include/linux/fpga. Hence
> let's just move one directory up and update all users.
> 
> Suggested-by: Xu Yilun <yilun.xu@linux.intel.com>
> Signed-off-by: Nuno Sá <nuno.sa@analog.com>

Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> # for IIO

> ---
>  drivers/clk/clk-axi-clkgen.c              | 2 ++
>  drivers/dma/dma-axi-dmac.c                | 2 +-
>  drivers/hwmon/axi-fan-control.c           | 2 +-
>  drivers/iio/adc/adi-axi-adc.c             | 3 +--
>  drivers/iio/dac/adi-axi-dac.c             | 2 +-
>  drivers/pwm/pwm-axi-pwmgen.c              | 2 +-
>  drivers/spi/spi-axi-spi-engine.c          | 2 +-
>  include/linux/{fpga => }/adi-axi-common.h | 0
>  8 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/clk/clk-axi-clkgen.c b/drivers/clk/clk-axi-clkgen.c
> index 2a95f9b220234a1245024a821c50e1eb9c104ac9..31915f8f5565f2ef5d17c0b4a0c91a648005b3e6 100644
> --- a/drivers/clk/clk-axi-clkgen.c
> +++ b/drivers/clk/clk-axi-clkgen.c
> @@ -16,6 +16,8 @@
>  #include <linux/mod_devicetable.h>
>  #include <linux/err.h>
>  
> +#include <linux/adi-axi-common.h>
> +
>  #define AXI_CLKGEN_V2_REG_RESET		0x40
>  #define AXI_CLKGEN_V2_REG_CLKSEL	0x44
>  #define AXI_CLKGEN_V2_REG_DRP_CNTRL	0x70
> diff --git a/drivers/dma/dma-axi-dmac.c b/drivers/dma/dma-axi-dmac.c
> index 36943b0c6d603cbe38606b0d7bde02535f529a9a..5b06b0dc67ee12017c165bf815fb7c0e1bf5abd8 100644
> --- a/drivers/dma/dma-axi-dmac.c
> +++ b/drivers/dma/dma-axi-dmac.c
> @@ -6,6 +6,7 @@
>   *  Author: Lars-Peter Clausen <lars@metafoo.de>
>   */
>  
> +#include <linux/adi-axi-common.h>
>  #include <linux/bitfield.h>
>  #include <linux/clk.h>
>  #include <linux/device.h>
> @@ -22,7 +23,6 @@
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>
>  #include <linux/slab.h>
> -#include <linux/fpga/adi-axi-common.h>
>  
>  #include <dt-bindings/dma/axi-dmac.h>
>  
> diff --git a/drivers/hwmon/axi-fan-control.c b/drivers/hwmon/axi-fan-control.c
> index 35c862eb158b0909dac64c2e9f51f0f9f0e8bf72..b7bb325c3ad966ed2a93be4dfbf4e20661568509 100644
> --- a/drivers/hwmon/axi-fan-control.c
> +++ b/drivers/hwmon/axi-fan-control.c
> @@ -4,9 +4,9 @@
>   *
>   * Copyright 2019 Analog Devices Inc.
>   */
> +#include <linux/adi-axi-common.h>
>  #include <linux/bits.h>
>  #include <linux/clk.h>
> -#include <linux/fpga/adi-axi-common.h>
>  #include <linux/hwmon.h>
>  #include <linux/hwmon-sysfs.h>
>  #include <linux/interrupt.h>
> diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
> index c7357601f0f869e57636f00bb1e26c059c3ab15c..87fa18f1ec96782556bdfad08bedb5e7549fb93d 100644
> --- a/drivers/iio/adc/adi-axi-adc.c
> +++ b/drivers/iio/adc/adi-axi-adc.c
> @@ -6,6 +6,7 @@
>   * Copyright 2012-2020 Analog Devices Inc.
>   */
>  
> +#include <linux/adi-axi-common.h>
>  #include <linux/bitfield.h>
>  #include <linux/cleanup.h>
>  #include <linux/clk.h>
> @@ -20,8 +21,6 @@
>  #include <linux/regmap.h>
>  #include <linux/slab.h>
>  
> -#include <linux/fpga/adi-axi-common.h>
> -
>  #include <linux/iio/backend.h>
>  #include <linux/iio/buffer-dmaengine.h>
>  #include <linux/iio/buffer.h>
> diff --git a/drivers/iio/dac/adi-axi-dac.c b/drivers/iio/dac/adi-axi-dac.c
> index b143f7ed6847277aeb49094627d90e5d95eed71c..581a2fe55a7fb35f1a03f96f3a0e95421d1583e7 100644
> --- a/drivers/iio/dac/adi-axi-dac.c
> +++ b/drivers/iio/dac/adi-axi-dac.c
> @@ -5,6 +5,7 @@
>   *
>   * Copyright 2016-2024 Analog Devices Inc.
>   */
> +#include <linux/adi-axi-common.h>
>  #include <linux/bitfield.h>
>  #include <linux/bits.h>
>  #include <linux/cleanup.h>
> @@ -23,7 +24,6 @@
>  #include <linux/regmap.h>
>  #include <linux/units.h>
>  
> -#include <linux/fpga/adi-axi-common.h>
>  #include <linux/iio/backend.h>
>  #include <linux/iio/buffer-dmaengine.h>
>  #include <linux/iio/buffer.h>
> diff --git a/drivers/pwm/pwm-axi-pwmgen.c b/drivers/pwm/pwm-axi-pwmgen.c
> index 4259a0db9ff45808eecae28680473292d165d1f6..e720191e74558d15f1b04fa18cf2984299f88809 100644
> --- a/drivers/pwm/pwm-axi-pwmgen.c
> +++ b/drivers/pwm/pwm-axi-pwmgen.c
> @@ -18,10 +18,10 @@
>   * - Supports normal polarity. Does not support changing polarity.
>   * - On disable, the PWM output becomes low (inactive).
>   */
> +#include <linux/adi-axi-common.h>
>  #include <linux/bits.h>
>  #include <linux/clk.h>
>  #include <linux/err.h>
> -#include <linux/fpga/adi-axi-common.h>
>  #include <linux/io.h>
>  #include <linux/minmax.h>
>  #include <linux/module.h>
> diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
> index 7c252126b33ea83fe6a6e80c6cb87499243069f5..d498132f1ff6adf20639bf4a21f1687903934bec 100644
> --- a/drivers/spi/spi-axi-spi-engine.c
> +++ b/drivers/spi/spi-axi-spi-engine.c
> @@ -5,9 +5,9 @@
>   *  Author: Lars-Peter Clausen <lars@metafoo.de>
>   */
>  
> +#include <linux/adi-axi-common.h>
>  #include <linux/clk.h>
>  #include <linux/completion.h>
> -#include <linux/fpga/adi-axi-common.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
>  #include <linux/of.h>
> diff --git a/include/linux/fpga/adi-axi-common.h b/include/linux/adi-axi-common.h
> similarity index 100%
> rename from include/linux/fpga/adi-axi-common.h
> rename to include/linux/adi-axi-common.h
>
Xu Yilun May 7, 2025, 2:35 a.m. UTC | #3
On Mon, May 05, 2025 at 07:30:01PM +0100, Jonathan Cameron wrote:
> On Mon, 05 May 2025 17:41:34 +0100
> Nuno Sá via B4 Relay <devnull+nuno.sa.analog.com@kernel.org> wrote:
> 
> > From: Nuno Sá <nuno.sa@analog.com>
> > 
> > The adi-axi-common.h header has some common defines used in various ADI
> > IPs. However they are not specific for any fpga manager so it's
> > questionable for the header to live under include/linux/fpga. Hence
> > let's just move one directory up and update all users.
> > 
> > Suggested-by: Xu Yilun <yilun.xu@linux.intel.com>
> > Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> 
> Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> # for IIO

Acked-by: Xu Yilun <yilun.xu@intel.com>

> 
> > ---
> >  drivers/clk/clk-axi-clkgen.c              | 2 ++
> >  drivers/dma/dma-axi-dmac.c                | 2 +-
> >  drivers/hwmon/axi-fan-control.c           | 2 +-
> >  drivers/iio/adc/adi-axi-adc.c             | 3 +--
> >  drivers/iio/dac/adi-axi-dac.c             | 2 +-
> >  drivers/pwm/pwm-axi-pwmgen.c              | 2 +-
> >  drivers/spi/spi-axi-spi-engine.c          | 2 +-
> >  include/linux/{fpga => }/adi-axi-common.h | 0
> >  8 files changed, 8 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/clk/clk-axi-clkgen.c b/drivers/clk/clk-axi-clkgen.c
> > index 2a95f9b220234a1245024a821c50e1eb9c104ac9..31915f8f5565f2ef5d17c0b4a0c91a648005b3e6 100644
> > --- a/drivers/clk/clk-axi-clkgen.c
> > +++ b/drivers/clk/clk-axi-clkgen.c
> > @@ -16,6 +16,8 @@
> >  #include <linux/mod_devicetable.h>
> >  #include <linux/err.h>
> >  
> > +#include <linux/adi-axi-common.h>
> > +
> >  #define AXI_CLKGEN_V2_REG_RESET		0x40
> >  #define AXI_CLKGEN_V2_REG_CLKSEL	0x44
> >  #define AXI_CLKGEN_V2_REG_DRP_CNTRL	0x70
> > diff --git a/drivers/dma/dma-axi-dmac.c b/drivers/dma/dma-axi-dmac.c
> > index 36943b0c6d603cbe38606b0d7bde02535f529a9a..5b06b0dc67ee12017c165bf815fb7c0e1bf5abd8 100644
> > --- a/drivers/dma/dma-axi-dmac.c
> > +++ b/drivers/dma/dma-axi-dmac.c
> > @@ -6,6 +6,7 @@
> >   *  Author: Lars-Peter Clausen <lars@metafoo.de>
> >   */
> >  
> > +#include <linux/adi-axi-common.h>
> >  #include <linux/bitfield.h>
> >  #include <linux/clk.h>
> >  #include <linux/device.h>
> > @@ -22,7 +23,6 @@
> >  #include <linux/platform_device.h>
> >  #include <linux/regmap.h>
> >  #include <linux/slab.h>
> > -#include <linux/fpga/adi-axi-common.h>
> >  
> >  #include <dt-bindings/dma/axi-dmac.h>
> >  
> > diff --git a/drivers/hwmon/axi-fan-control.c b/drivers/hwmon/axi-fan-control.c
> > index 35c862eb158b0909dac64c2e9f51f0f9f0e8bf72..b7bb325c3ad966ed2a93be4dfbf4e20661568509 100644
> > --- a/drivers/hwmon/axi-fan-control.c
> > +++ b/drivers/hwmon/axi-fan-control.c
> > @@ -4,9 +4,9 @@
> >   *
> >   * Copyright 2019 Analog Devices Inc.
> >   */
> > +#include <linux/adi-axi-common.h>
> >  #include <linux/bits.h>
> >  #include <linux/clk.h>
> > -#include <linux/fpga/adi-axi-common.h>
> >  #include <linux/hwmon.h>
> >  #include <linux/hwmon-sysfs.h>
> >  #include <linux/interrupt.h>
> > diff --git a/drivers/iio/adc/adi-axi-adc.c b/drivers/iio/adc/adi-axi-adc.c
> > index c7357601f0f869e57636f00bb1e26c059c3ab15c..87fa18f1ec96782556bdfad08bedb5e7549fb93d 100644
> > --- a/drivers/iio/adc/adi-axi-adc.c
> > +++ b/drivers/iio/adc/adi-axi-adc.c
> > @@ -6,6 +6,7 @@
> >   * Copyright 2012-2020 Analog Devices Inc.
> >   */
> >  
> > +#include <linux/adi-axi-common.h>
> >  #include <linux/bitfield.h>
> >  #include <linux/cleanup.h>
> >  #include <linux/clk.h>
> > @@ -20,8 +21,6 @@
> >  #include <linux/regmap.h>
> >  #include <linux/slab.h>
> >  
> > -#include <linux/fpga/adi-axi-common.h>
> > -
> >  #include <linux/iio/backend.h>
> >  #include <linux/iio/buffer-dmaengine.h>
> >  #include <linux/iio/buffer.h>
> > diff --git a/drivers/iio/dac/adi-axi-dac.c b/drivers/iio/dac/adi-axi-dac.c
> > index b143f7ed6847277aeb49094627d90e5d95eed71c..581a2fe55a7fb35f1a03f96f3a0e95421d1583e7 100644
> > --- a/drivers/iio/dac/adi-axi-dac.c
> > +++ b/drivers/iio/dac/adi-axi-dac.c
> > @@ -5,6 +5,7 @@
> >   *
> >   * Copyright 2016-2024 Analog Devices Inc.
> >   */
> > +#include <linux/adi-axi-common.h>
> >  #include <linux/bitfield.h>
> >  #include <linux/bits.h>
> >  #include <linux/cleanup.h>
> > @@ -23,7 +24,6 @@
> >  #include <linux/regmap.h>
> >  #include <linux/units.h>
> >  
> > -#include <linux/fpga/adi-axi-common.h>
> >  #include <linux/iio/backend.h>
> >  #include <linux/iio/buffer-dmaengine.h>
> >  #include <linux/iio/buffer.h>
> > diff --git a/drivers/pwm/pwm-axi-pwmgen.c b/drivers/pwm/pwm-axi-pwmgen.c
> > index 4259a0db9ff45808eecae28680473292d165d1f6..e720191e74558d15f1b04fa18cf2984299f88809 100644
> > --- a/drivers/pwm/pwm-axi-pwmgen.c
> > +++ b/drivers/pwm/pwm-axi-pwmgen.c
> > @@ -18,10 +18,10 @@
> >   * - Supports normal polarity. Does not support changing polarity.
> >   * - On disable, the PWM output becomes low (inactive).
> >   */
> > +#include <linux/adi-axi-common.h>
> >  #include <linux/bits.h>
> >  #include <linux/clk.h>
> >  #include <linux/err.h>
> > -#include <linux/fpga/adi-axi-common.h>
> >  #include <linux/io.h>
> >  #include <linux/minmax.h>
> >  #include <linux/module.h>
> > diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
> > index 7c252126b33ea83fe6a6e80c6cb87499243069f5..d498132f1ff6adf20639bf4a21f1687903934bec 100644
> > --- a/drivers/spi/spi-axi-spi-engine.c
> > +++ b/drivers/spi/spi-axi-spi-engine.c
> > @@ -5,9 +5,9 @@
> >   *  Author: Lars-Peter Clausen <lars@metafoo.de>
> >   */
> >  
> > +#include <linux/adi-axi-common.h>
> >  #include <linux/clk.h>
> >  #include <linux/completion.h>
> > -#include <linux/fpga/adi-axi-common.h>
> >  #include <linux/interrupt.h>
> >  #include <linux/io.h>
> >  #include <linux/of.h>
> > diff --git a/include/linux/fpga/adi-axi-common.h b/include/linux/adi-axi-common.h
> > similarity index 100%
> > rename from include/linux/fpga/adi-axi-common.h
> > rename to include/linux/adi-axi-common.h
> > 
>
Nuno Sá May 7, 2025, 6:11 a.m. UTC | #4
On Mon, 2025-05-05 at 12:21 -0500, David Lechner wrote:
> On 5/5/25 11:41 AM, Nuno Sá via B4 Relay wrote:
> > From: Nuno Sá <nuno.sa@analog.com>
> > 
> > Add new helper macros and enums to help identifying the platform and some
> > characteristics of it at runtime.
> > 
> > Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> > ---
> >  include/linux/adi-axi-common.h | 35 +++++++++++++++++++++++++++++++++++
> 
> Since this file was moved in the previous patch, should we drop "fpga:" from the
> subject of this patch?

sure

> 
> >  1 file changed, 35 insertions(+)
> > 
> > diff --git a/include/linux/adi-axi-common.h b/include/linux/adi-axi-common.h
> > index
> > 141ac3f251e6f256526812b9d55cd440a2a46e76..a832ef9b37473ca339a2a2ff8a4a5716d428fd2
> > 9 100644
> > --- a/include/linux/adi-axi-common.h
> > +++ b/include/linux/adi-axi-common.h
> > @@ -12,6 +12,8 @@
> >  #define ADI_AXI_COMMON_H_
> >  
> >  #define ADI_AXI_REG_VERSION			0x0000
> > +#define ADI_AXI_REG_FPGA_INFO			0x001C
> > +#define ADI_AXI_REG_FPGA_VOLTAGE		0x0140
> 
> Doesn't the voltage register only apply to AXI CLKGEN and therefore would
> belong in the clock driver rather than here? 0x1C seems to be the last of the
> defined "common to all IP cores" address before we possibly get into
> core-specific register definitions starting at 0x40.
> 
> I guess there are 1 or 2 other cores that define it at the same place, but it
> still seems not-global.


Hmm, to be honest I did not checked. This patch was out of tree and I blindly pushed
it (as its straightforward). Will drop the above.

> 
> >  
> >  #define ADI_AXI_PCORE_VER(major, minor, patch)	\
> >  	(((major) << 16) | ((minor) << 8) | (patch))
> > @@ -20,4 +22,37 @@
> >  #define ADI_AXI_PCORE_VER_MINOR(version)	(((version) >> 8) & 0xff)
> >  #define ADI_AXI_PCORE_VER_PATCH(version)	((version) & 0xff)
> >  
> > +#define ADI_AXI_INFO_FPGA_TECH(info)            (((info) >> 24) & 0xff)
> > +#define ADI_AXI_INFO_FPGA_FAMILY(info)          (((info) >> 16) & 0xff)
> > +#define ADI_AXI_INFO_FPGA_SPEED_GRADE(info)     (((info) >> 8) & 0xff)
> 
> I guess we don't care about the DEV_PACKAGE field?

ack

> 
> > +#define ADI_AXI_INFO_FPGA_VOLTAGE(val)          ((val) & 0xffff)
> 
> This VOLTAGE also goes applies to the first comment.
> 

ack