[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CA+U=Dso5uCyjd8v1Xo5iWsUacchfbtuR+6NMgivEAv9TZNezcA@mail.gmail.com>
Date: Thu, 11 Feb 2021 07:32:09 +0200
From: Alexandru Ardelean <ardeleanalex@...il.com>
To: Moritz Fischer <mdf@...nel.org>
Cc: Alexandru Ardelean <alexandru.ardelean@...log.com>,
linux-clk@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>,
Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>,
Lars-Peter Clausen <lars@...afoo.de>,
linux-fpga@...r.kernel.org,
Mircea Caprioru <mircea.caprioru@...log.com>
Subject: Re: [PATCH 1/2] include: fpga: adi-axi-common.h: add definitions for
supported FPGAs
On Thu, Feb 11, 2021 at 6:00 AM Moritz Fischer <mdf@...nel.org> wrote:
>
> Alexandru,
>
> On Wed, Feb 10, 2021 at 12:15:34PM +0200, Alexandru Ardelean wrote:
> > From: Mircea Caprioru <mircea.caprioru@...log.com>
> >
> > All (newer) FPGA IP cores supported by Analog Devices, store information in
>
> Nit: extra ',' ?
> > the synthesized designs. This information describes various parameters,
> > including the family of boards on which this is deployed, speed-grade, and
> > so on.
> >
> > Currently, some of these definitions are deployed mostly on Xilinx boards,
> > but they have been considered also for FPGA boards from other vendors.
> Let's add them together with the code that uses them.
> >
> > The register definitions are described at this link:
> > https://wiki.analog.com/resources/fpga/docs/hdl/regmap
> > (the 'Base (common to all cores)' section).
> >
> > Acked-by: Moritz Fischer <mdf@...nel.org>
> This patchset is very different from the reviewed one earlier. Please
> don't just copy Acked-by's.
Apologies
I think it got some more reviews and I just kept the tag.
Sloppy on my part.
>
> > Signed-off-by: Mircea Caprioru <mircea.caprioru@...log.com>
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@...log.com>
> > ---
> >
> > This is a continuation of this old set:
> > https://lore.kernel.org/linux-clk/20200929144417.89816-1-alexandru.ardelean@analog.com/
> >
> > Particularly patches:
> > https://lore.kernel.org/linux-clk/20200929144417.89816-15-alexandru.ardelean@analog.com/
> > https://lore.kernel.org/linux-clk/20200929144417.89816-16-alexandru.ardelean@analog.com/
> >
> > That was v4, but this patchset was split away from it, to resolve
> > discussion on some other patches in that set.
> >
> > The other patches were accepted here:
> > https://lore.kernel.org/linux-clk/20210201151245.21845-1-alexandru.ardelean@analog.com/
> >
> > include/linux/fpga/adi-axi-common.h | 103 ++++++++++++++++++++++++++++
> > 1 file changed, 103 insertions(+)
> >
> > diff --git a/include/linux/fpga/adi-axi-common.h b/include/linux/fpga/adi-axi-common.h
> > index 141ac3f251e6..1a7f18e3a384 100644
> > --- a/include/linux/fpga/adi-axi-common.h
> > +++ b/include/linux/fpga/adi-axi-common.h
> > @@ -13,6 +13,9 @@
> >
> > #define ADI_AXI_REG_VERSION 0x0000
> >
> > +#define ADI_AXI_REG_FPGA_INFO 0x001C
> > +#define ADI_AXI_REG_FPGA_VOLTAGE 0x0140
> > +
> > #define ADI_AXI_PCORE_VER(major, minor, patch) \
> > (((major) << 16) | ((minor) << 8) | (patch))
> >
> > @@ -20,4 +23,104 @@
> > #define ADI_AXI_PCORE_VER_MINOR(version) (((version) >> 8) & 0xff)
> > #define ADI_AXI_PCORE_VER_PATCH(version) ((version) & 0xff)
> >
> > +#define ADI_AXI_INFO_FPGA_VOLTAGE(val) ((val) & 0xffff)
> > +
> > +#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)
> > +#define ADI_AXI_INFO_FPGA_DEV_PACKAGE(info) ((info) & 0xff)
>
> Do we really need all the macros?
No.
I can trim them to a minimum.
> > +
> > +/**
> > + * FPGA Technology definitions
> > + */
> > +#define ADI_AXI_FPGA_TECH_XILINX_UNKNOWN 0
> > +#define ADI_AXI_FPGA_TECH_XILINS_SERIES7 1
> > +#define ADI_AXI_FPGA_TECH_XILINX_ULTRASCALE 2
> > +#define ADI_AXI_FPGA_TECH_XILINX_ULTRASCALE_PLUS 3
> > +
> > +#define ADI_AXI_FPGA_TECH_INTEL_UNKNOWN 100
> > +#define ADI_AXI_FPGA_TECH_INTEL_CYCLONE_5 101
> > +#define ADI_AXI_FPGA_TECH_INTEL_CYCLONE_10 102
> > +#define ADI_AXI_FPGA_TECH_INTEL_ARRIA_10 103
> > +#define ADI_AXI_FPGA_TECH_INTEL_STRATIX_10 104
> > +
> > +/**
> > + * FPGA Family definitions
> > + */
> > +#define ADI_AXI_FPGA_FAMILY_UNKNOWN 0
> > +
> > +#define ADI_AXI_FPGA_FAMILY_XILINX_ARTIX 1
> > +#define ADI_AXI_FPGA_FAMILY_XILINX_KINTEX 2
> > +#define ADI_AXI_FPGA_FAMILY_XILINX_VIRTEX 3
> > +#define ADI_AXI_FPGA_FAMILY_XILINX_ZYNQ 4
> > +
> > +#define ADI_AXI_FPGA_FAMILY_INTEL_SX 1
> > +#define ADI_AXI_FPGA_FAMILY_INTEL_GX 2
> > +#define ADI_AXI_FPGA_FAMILY_INTEL_GT 3
> > +#define ADI_AXI_FPGA_FAMILY_INTEL_GZ 4
> > +
> > +/**
> > + * FPGA Speed-grade definitions
> > + */
> > +#define ADI_AXI_FPGA_SPEED_GRADE_UNKNOWN 0
> > +
> > +#define ADI_AXI_FPGA_SPEED_GRADE_XILINX_1 10
> > +#define ADI_AXI_FPGA_SPEED_GRADE_XILINX_1L 11
> > +#define ADI_AXI_FPGA_SPEED_GRADE_XILINX_1H 12
> > +#define ADI_AXI_FPGA_SPEED_GRADE_XILINX_1HV 13
> > +#define ADI_AXI_FPGA_SPEED_GRADE_XILINX_1LV 14
> > +#define ADI_AXI_FPGA_SPEED_GRADE_XILINX_2 20
> > +#define ADI_AXI_FPGA_SPEED_GRADE_XILINX_2L 21
> > +#define ADI_AXI_FPGA_SPEED_GRADE_XILINX_2LV 22
> > +#define ADI_AXI_FPGA_SPEED_GRADE_XILINX_3 30
> > +
> > +#define ADI_AXI_FPGA_SPEED_GRADE_INTEL_1 1
> > +#define ADI_AXI_FPGA_SPEED_GRADE_INTEL_2 2
> > +#define ADI_AXI_FPGA_SPEED_GRADE_INTEL_3 3
> > +#define ADI_AXI_FPGA_SPEED_GRADE_INTEL_4 4
> > +#define ADI_AXI_FPGA_SPEED_GRADE_INTEL_5 5
> > +#define ADI_AXI_FPGA_SPEED_GRADE_INTEL_6 6
> > +#define ADI_AXI_FPGA_SPEED_GRADE_INTEL_7 7
> > +#define ADI_AXI_FPGA_SPEED_GRADE_INTEL_8 8
> > +#define ADI_AXI_FPGA_SPEED_GRADE_INTEL_9 9
> > +
> > +/**
> > + * FPGA Device Package definitions
> > + */
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_UNKNOWN 0
> > +
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_XILINX_RF 1
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_XILINX_FL 2
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_XILINX_FF 3
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_XILINX_FB 4
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_XILINX_HC 5
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_XILINX_FH 6
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_XILINX_CS 7
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_XILINX_CP 8
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_XILINX_FT 9
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_XILINX_FG 10
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_XILINX_SB 11
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_XILINX_RB 12
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_XILINX_RS 13
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_XILINX_CL 14
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_XILINX_SF 15
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_XILINX_BA 16
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_XILINX_FA 17
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_XILINX_FS 18
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_XILINX_FI 19
> > +
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_INTEL_BGA 1
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_INTEL_PGA 2
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_INTEL_FBGA 3
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_INTEL_HBGA 4
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_INTEL_PDIP 5
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_INTEL_EQFP 6
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_INTEL_PLCC 7
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_INTEL_PQFP 8
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_INTEL_RQFP 9
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_INTEL_TQFP 10
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_INTEL_UBGA 11
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_INTEL_UFBGA 12
> > +#define ADI_AXI_FPGA_DEV_PACKAGE_INTEL_MBGA 13
>
> What is using those? Do these package impact anything behavioral?
So, these should get defined in ADI FPGA projects.
The initial idea was to be able to read this in drivers and adjust
behavior (usually speed) according to each parameter.
Then some drivers could have used this to do some tunings.
Maybe also pass these in userspace and have the application decide.
I don't think there was some finalization on the idea (yet).
[I know how this sounds, but] The more I try to explain it, the less I
get convinced by this patch.
I pick up some of the patches in our tree and try to make sense of
them through upstreaming.
Sometimes when I pick them, it's not always obvious to me whether
they're worth it or not.
¯\_(ツ)_/¯
> > +
> > #endif /* ADI_AXI_COMMON_H_ */
> > --
> > 2.17.1
> >
>
> - Moritz
>
> PS: The subject line could use a bit of work, too :)
Will think about it :)
Powered by blists - more mailing lists