Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 30 Dec 2022 15:30:59 +0800
From:   Xu Yilun <yilun.xu@intel.com>
To:     Ivan Bornyakov <i.bornyakov@metrotek.ru>
Cc:     linux-fpga@vger.kernel.org,
        Conor Dooley <conor.dooley@microchip.com>,
        Moritz Fischer <mdf@kernel.org>, Wu Hao <hao.wu@intel.com>,
        Tom Rix <trix@redhat.com>,
        Ilpo =?iso-8859-1?Q?J=E4rvinen?= <ilpo.jarvinen@linux.intel.com>,
        linux-kernel@vger.kernel.org, system@metrotek.ru
Subject: Re: [PATCH v4 1/3] fpga: microchip-spi: move SPI I/O buffers out of
 stack
Message-ID: <Y66Ts4SIYBE8QFF8@yilunxu-OptiPlex-7050>
References: <20221229104604.2496-1-i.bornyakov@metrotek.ru>
 <20221229104604.2496-2-i.bornyakov@metrotek.ru>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20221229104604.2496-2-i.bornyakov@metrotek.ru>
Precedence: bulk

On 2022-12-29 at 13:46:02 +0300, Ivan Bornyakov wrote:
> As spi-summary doc says:
>  > I/O buffers use the usual Linux rules, and must be DMA-safe.
>  > You'd normally allocate them from the heap or free page pool.
>  > Don't use the stack, or anything that's declared "static".
> 
> Replace spi_write() with spi_write_then_read(), which is dma-safe for
> on-stack buffers. Use allocated buffers for transfers used in

How about "Use cacheline aligned buffers for ..."

> spi_sync_transfer().
> 
> Although everything works OK with stack-located I/O buffers, better
> follow the doc to be safe.
> 
> Fixes: 5f8d4a900830 ("fpga: microchip-spi: add Microchip MPF FPGA manager")
> Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  drivers/fpga/microchip-spi.c | 93 ++++++++++++++++++------------------
>  1 file changed, 47 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/fpga/microchip-spi.c b/drivers/fpga/microchip-spi.c
> index 7436976ea904..e72fedd93a27 100644
> --- a/drivers/fpga/microchip-spi.c
> +++ b/drivers/fpga/microchip-spi.c
> @@ -42,46 +42,55 @@
>  struct mpf_priv {
>  	struct spi_device *spi;
>  	bool program_mode;
> +	u8 tx __aligned(ARCH_KMALLOC_MINALIGN);
> +	u8 rx __aligned(ARCH_KMALLOC_MINALIGN);

If the 2 buffers are used synchronously by dma, they could share a
cacheline. Just separate them from other members should be OK, like:

	u8 tx __aligned(ARCH_KMALLOC_MINALIGN);
	u8 rx;

>  };
>