lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 15 Nov 2021 16:35:58 +0200
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Serge Semin <Sergey.Semin@...kalelectronics.ru>
Cc:     Serge Semin <fancer.lancer@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Nandhini Srikandan <nandhini.srikandan@...el.com>,
        Andy Shevchenko <andy@...nel.org>, linux-spi@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 6/6] spi: dw: Replace DWC_HSSI capability with IP-core
 version checker

On Mon, Nov 15, 2021 at 01:30:26AM +0300, Serge Semin wrote:
> Since there is a common IP-core and component versions interface available
> we can use it to differentiate the DW HSSI device features in the code.
> Let's remove the corresponding DWC_HSSI capability flag then and use the
> dw_spi_ip_is() macro instead.

Reviewed-by: Andy Shevchenko <andy.shevchenko@...il.com>

But see below.

> Signed-off-by: Serge Semin <Sergey.Semin@...kalelectronics.ru>
> 
> ---
> 
> Changelog v2:
> - This is a new patch created as a logical result of a new DW SSI IP-core
>   versions internal interface introduced in the previous patch.
> ---
>  drivers/spi/spi-dw-core.c | 8 ++++----
>  drivers/spi/spi-dw-mmio.c | 5 +++--
>  drivers/spi/spi-dw.h      | 3 +--
>  3 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c
> index 42536b448ddd..934cc7a922e8 100644
> --- a/drivers/spi/spi-dw-core.c
> +++ b/drivers/spi/spi-dw-core.c
> @@ -272,7 +272,7 @@ static u32 dw_spi_prepare_cr0(struct dw_spi *dws, struct spi_device *spi)
>  {
>  	u32 cr0 = 0;
>  
> -	if (!(dws->caps & DW_SPI_CAP_DWC_HSSI)) {
> +	if (dw_spi_ip_is(dws, PSSI)) {
>  		/* CTRLR0[ 5: 4] Frame Format */
>  		cr0 |= FIELD_PREP(DW_PSSI_CTRLR0_FRF_MASK, DW_SPI_CTRLR0_FRF_MOTO_SPI);
>  
> @@ -325,7 +325,7 @@ void dw_spi_update_config(struct dw_spi *dws, struct spi_device *spi,
>  	/* CTRLR0[ 4/3: 0] or CTRLR0[ 20: 16] Data Frame Size */
>  	cr0 |= (cfg->dfs - 1) << dws->dfs_offset;
>  
> -	if (!(dws->caps & DW_SPI_CAP_DWC_HSSI))
> +	if (dw_spi_ip_is(dws, PSSI))
>  		/* CTRLR0[ 9:8] Transfer Mode */
>  		cr0 |= FIELD_PREP(DW_PSSI_CTRLR0_TMOD_MASK, cfg->tmode);
>  	else
> @@ -832,7 +832,7 @@ static void dw_spi_hw_init(struct device *dev, struct dw_spi *dws)
>  		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_ip_is(dws, PSSI) ? " APB " : " ",
>  			DW_SPI_GET_BYTE(dws->ver, 3), DW_SPI_GET_BYTE(dws->ver, 2),
>  			DW_SPI_GET_BYTE(dws->ver, 1));
>  	}
> @@ -860,7 +860,7 @@ static void dw_spi_hw_init(struct device *dev, struct dw_spi *dws)
>  	 * writability. Note DWC SSI controller also has the extended DFS, but
>  	 * with zero offset.
>  	 */
> -	if (!(dws->caps & DW_SPI_CAP_DWC_HSSI)) {
> +	if (dw_spi_ip_is(dws, PSSI)) {
>  		u32 cr0, tmp = dw_readl(dws, DW_SPI_CTRLR0);
>  
>  		dw_spi_enable_chip(dws, 0);
> diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
> index 0b37bd32b041..c0e5bb6add0a 100644
> --- a/drivers/spi/spi-dw-mmio.c
> +++ b/drivers/spi/spi-dw-mmio.c
> @@ -207,7 +207,7 @@ static int dw_spi_pssi_init(struct platform_device *pdev,
>  static int dw_spi_hssi_init(struct platform_device *pdev,
>  			    struct dw_spi_mmio *dwsmmio)
>  {
> -	dwsmmio->dws.caps = DW_SPI_CAP_DWC_HSSI;
> +	dwsmmio->dws.ip = DW_HSSI_ID;
>  
>  	dw_spi_dma_setup_generic(&dwsmmio->dws);
>  
> @@ -217,7 +217,8 @@ static int dw_spi_hssi_init(struct platform_device *pdev,
>  static int dw_spi_keembay_init(struct platform_device *pdev,
>  			       struct dw_spi_mmio *dwsmmio)
>  {
> -	dwsmmio->dws.caps = DW_SPI_CAP_KEEMBAY_MST | DW_SPI_CAP_DWC_HSSI;
> +	dwsmmio->dws.ip = DW_HSSI_ID;
> +	dwsmmio->dws.caps = DW_SPI_CAP_KEEMBAY_MST;
>  
>  	return 0;
>  }
> diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
> index 2f7d77024b48..d5ee5130601e 100644
> --- a/drivers/spi/spi-dw.h
> +++ b/drivers/spi/spi-dw.h
> @@ -32,8 +32,7 @@
>  /* 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)
> +#define DW_SPI_CAP_DFS32		BIT(2)

In one patch you move this in the file upper.
Here you reshuffling it due to dropping one bit.

Now I'm wondering if you may split these two into a separate patch, which
brings us to simple

-#define DW_SPI_CAP_DWC_HSSI		BIT(3)

here.

>  
>  /* Register offsets (Generic for both DWC APB SSI and DWC SSI IP-cores) */
>  #define DW_SPI_CTRLR0			0x00

-- 
With Best Regards,
Andy Shevchenko


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ