mbox series

[v2,0/6] spi: dw: Cleanup macros/funcs naming and add IP-core version support

Message ID 20211114223026.13359-1-Sergey.Semin@baikalelectronics.ru
Headers show
Series spi: dw: Cleanup macros/funcs naming and add IP-core version support | expand

Message

Serge Semin Nov. 14, 2021, 10:30 p.m. UTC
I was going to submit a cleanup patchset for the DW SSI driver for a
long time, but due to lack of free time couldn't make it so far.
Nandhini's series [1] made me to proceed with this task so mate would
finally have his patchset accepted and merged into the mainline kernel.

There are four cleanup patches here and two feature patches. In the
framework of the former patches we convert the DW SPI driver to using
module namespaces and provide a better code organization. In particular
the second part concerns the methods and macros naming unification (using
a unified prefixes of the code object names) and the CSR fields macro
implementation using the bitfield helpers available in the kernel. The
later patches introduces the DW SSI IP-core versions interface so it
could be used for a version-specific features implementation.  Nandhini
will be mostly interested in the later patches in the framework of his
series [1].

Nandhini, could you please test the patchset out on your DWC SSI hardware?
After it's merged into the spi/for-next branch of the Mark' repository you
will be able to rebase your series on top of it and use the last IP-core
version interface for your benefit.

Andy, I haven't added your tag to the new patches and to the patch
with the IP-core version interface.

[1] https://lore.kernel.org/linux-spi/20211111065201.10249-4-nandhini.srikandan@intel.com

Link: https://lore.kernel.org/linux-spi/20211112204927.8830-1-Sergey.Semin@baikalelectronics.ru
Changelog v2:
- Replace "assi" suffixes with "pssi" in the spi-dw-mmio.c methods.
  (Thanks Andy for noticing this pity mistake)
- Add new patches:
  [PATCH v2 6/6] spi: dw: Replace DWC_HSSI capability with IP-core version checker
  [PATCH v2 1/6] spi: dw: Add symbol namespaces for the core and DMA modules
- Replace the ASCII-to-integer conversion with a unified IP-core versioning
  interface (Suggested by Andy).

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: linux-spi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

Serge Semin (6):
  spi: dw: Add a symbols namespace for the core module
  spi: dw: Discard redundant DW SSI Frame Formats enumeration
  spi: dw: Put the driver entities naming in order
  spi: dw: Convert to using the Bitfield access macros
  spi: dw: Introduce Synopsys IP-core versions interface
  spi: dw: Replace DWC_HSSI capability with IP-core version checker

 drivers/spi/spi-dw-bt1.c  |   9 +-
 drivers/spi/spi-dw-core.c | 175 ++++++++++++++++++++----------------
 drivers/spi/spi-dw-dma.c  |  57 ++++++------
 drivers/spi/spi-dw-mmio.c |  22 ++---
 drivers/spi/spi-dw-pci.c  |  60 ++++++-------
 drivers/spi/spi-dw.h      | 182 ++++++++++++++++++++------------------
 6 files changed, 273 insertions(+), 232 deletions(-)

Comments

Andy Shevchenko Nov. 15, 2021, 2:32 p.m. UTC | #1
On Mon, Nov 15, 2021 at 01:30:25AM +0300, Serge Semin wrote:
> The driver currently supports two IP-core versions. It's DW APB SSI which
> is older version of the controller with APB system bus interface, and DW
> SSI controller with AHB bus interface. The later one is supposed to be a
> new generation high-speed SSI. Even though both of these IP-cores have got
> an almost identical registers space there are some differences. The driver
> differentiates these distinctions by the DW_SPI_CAP_DWC_HSSI capability
> flag. In addition to that each DW SSI IP-core is equipped with a Synopsys
> Component version register, which encodes the IP-core release ID the has
> been synthesized from. Seeing we are going to need the later one to
> differentiate some controller peculiarities it would be better to have a
> unified interface for both IP-core line and release versions instead of
> using each of them separately.
> 
> Introduced here IP-core versioning interface consists of two parts:
> 1) IDs of the IP-core (virtual) and component versions.
> 2) a set of macro helpers to identify current IP-core and component
> versions.
> 
> So the platform code is supposed to assign a proper IP-core version based
> on it's platform -knowledge. The main driver initialization method reads
> the IP-core release ID from the SSI component version register. That data
> is used by the helpers to distinguish one IP-core release from another.
> Thus the rest of the driver can use these macros to implement the
> conditional code execution based on the specified IP-core and version IDs.
> 
> Collect the IP-core versions interface and the defined capabilities at the
> top of the header file since they represent a common device description
> data and so to immediately available for the driver hackers.

Thanks! Fine with me
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
> ---
> 
> Note I intentionally haven't used the "%p4cc" format specifier due to
> three reasons. First the component version 4cc string is preserved in the
> little endian format in the DW SSI register thus having '*' in the least
> significant byte.  Such representation do look well when you print it in
> hex, but that's not what the 4cc format print implementation expects
> (fourcc_string() wants to see a most-significant char at the lowest byte).
> Second the 4cc format also prints some pixel-format specific info like
> endiannes, which is irrelevant to the synopsys component version. Finally
> a string in the "v%c.%c%c" format represents the component version better.

Yeah, I guess we might need a specific FourCC for Synopsys IPs. But it's
another story.

> Changelog v2:
> - Replace the ASCII-to-integer conversion with a unified IP-core versions
>   interface.
> ---
>  drivers/spi/spi-dw-core.c | 14 ++++++++++++++
>  drivers/spi/spi-dw.h      | 36 ++++++++++++++++++++++++++++--------
>  2 files changed, 42 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> index b9f809989fda..42536b448ddd 100644
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c
> @@ -823,6 +823,20 @@ static void dw_spi_hw_init(struct device *dev, struct dw_spi *dws)
>  {
>  	dw_spi_reset_chip(dws);
>  
> +	/*
> +	 * Retrieve the Synopsys component version if it hasn't been specified
> +	 * by the platform. CoreKit version ID is encoded as a 3-chars ASCII
> +	 * code enclosed with '*' (typical for the most of Synopsys IP-cores).
> +	 */
> +	if (!dws->ver) {
> +		dws->ver = dw_readl(dws, DW_SPI_VERSION);
> +
> +		dev_dbg(dev, "Synopsys DWC%sSSI v%c.%c%c\n",
> +			(dws->caps & DW_SPI_CAP_DWC_HSSI) ? " " : " APB ",
> +			DW_SPI_GET_BYTE(dws->ver, 3), DW_SPI_GET_BYTE(dws->ver, 2),
> +			DW_SPI_GET_BYTE(dws->ver, 1));
> +	}
> +
>  	/*
>  	 * Try to detect the FIFO depth if not set by interface driver,
>  	 * the depth could be from 2 to 256 from HW spec
> diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
> index 634085eadad1..2f7d77024b48 100644
> --- a/drivers/spi/spi-dw.h
> +++ b/drivers/spi/spi-dw.h
> @@ -11,6 +11,30 @@
>  #include <linux/spi/spi-mem.h>
>  #include <linux/bitfield.h>
>  
> +/* Synopsys DW SSI IP-core virtual IDs */
> +#define DW_PSSI_ID			0
> +#define DW_HSSI_ID			1
> +
> +/* Synopsys DW SSI component versions (FourCC sequence) */
> +#define DW_HSSI_102A			0x3130322a
> +
> +/* DW SSI IP-core ID and version check helpers */
> +#define dw_spi_ip_is(_dws, _ip) \
> +	((_dws)->ip == DW_ ## _ip ## _ID)
> +
> +#define __dw_spi_ver_cmp(_dws, _ip, _ver, _op) \
> +	(dw_spi_ip_is(_dws, _ip) && (_dws)->ver _op DW_ ## _ip ## _ver)
> +
> +#define dw_spi_ver_is(_dws, _ip, _ver) __dw_spi_ver_cmp(_dws, _ip, _ver, ==)
> +
> +#define dw_spi_ver_is_ge(_dws, _ip, _ver) __dw_spi_ver_cmp(_dws, _ip, _ver, >=)
> +
> +/* DW SPI controller capabilities */
> +#define DW_SPI_CAP_CS_OVERRIDE		BIT(0)
> +#define DW_SPI_CAP_KEEMBAY_MST		BIT(1)
> +#define DW_SPI_CAP_DWC_HSSI		BIT(2)
> +#define DW_SPI_CAP_DFS32		BIT(3)
> +
>  /* Register offsets (Generic for both DWC APB SSI and DWC SSI IP-cores) */
>  #define DW_SPI_CTRLR0			0x00
>  #define DW_SPI_CTRLR1			0x04
> @@ -113,12 +137,6 @@
>  #define DW_SPI_GET_BYTE(_val, _idx) \
>  	((_val) >> (BITS_PER_BYTE * (_idx)) & 0xff)
>  
> -/* DW SPI capabilities */
> -#define DW_SPI_CAP_CS_OVERRIDE		BIT(0)
> -#define DW_SPI_CAP_KEEMBAY_MST		BIT(1)
> -#define DW_SPI_CAP_DWC_HSSI		BIT(2)
> -#define DW_SPI_CAP_DFS32		BIT(3)
> -
>  /* Slave spi_transfer/spi_mem_op related */
>  struct dw_spi_cfg {
>  	u8 tmode;
> @@ -141,6 +159,10 @@ struct dw_spi_dma_ops {
>  struct dw_spi {
>  	struct spi_controller	*master;
>  
> +	u32			ip;		/* Synopsys DW SSI IP-core ID */
> +	u32			ver;		/* Synopsys component version */
> +	u32			caps;		/* DW SPI capabilities */
> +
>  	void __iomem		*regs;
>  	unsigned long		paddr;
>  	int			irq;
> @@ -149,8 +171,6 @@ struct dw_spi {
>  	u32			max_mem_freq;	/* max mem-ops bus freq */
>  	u32			max_freq;	/* max bus freq supported */
>  
> -	u32			caps;		/* DW SPI capabilities */
> -
>  	u32			reg_io_width;	/* DR I/O width in bytes */
>  	u16			bus_num;
>  	u16			num_cs;		/* supported slave numbers */
> -- 
> 2.33.0
>