Re: [PATCH v2 3/8] spi: imx: add support for all SPI word width for DMA transfer

[Anton Bondarenko <anton.bondarenko.sama@gmail.com>]

On 08.10.2015 11:51, Robin Gong wrote:
> On Thu, Oct 01, 2015 at 12:34:54AM +0000, Bondarenko, Anton wrote:
>> On 30.09.2015 10:35, Robin Gong wrote:
>>> On Fri, Sep 25, 2015 at 07:57:10PM +0200, Anton Bondarenko wrote:
>>>> @@ -91,11 +91,15 @@ struct spi_imx_data {
>>>>
>>>>   	struct completion xfer_done;
>>>>   	void __iomem *base;
>>>> +	unsigned long base_phys;
>>>> +
>>>>   	struct clk *clk_per;
>>>>   	struct clk *clk_ipg;
>>>>   	unsigned long spi_clk;
>>>>   	unsigned int spi_bus_clk;
>>>>
>>>> +	unsigned int bpw_w;
>>>> +
>>> It's better to change bytes_per_word for clear understanding,since bpw in spi means
>>> bits_per_word...
>> Agree. Fixed in V3
>>>>   	unsigned int count;
>>>>   	void (*tx)(struct spi_imx_data *);
>>>>   	void (*rx)(struct spi_imx_data *);
>>>> @@ -201,9 +205,13 @@ static bool spi_imx_can_dma(struct spi_master *master, struct spi_device *spi,
>>>>   			 struct spi_transfer *transfer)
>>>>   {
>>>>   	struct spi_imx_data *spi_imx = spi_master_get_devdata(master);
>>>> +	unsigned int bpw_w = transfer->bits_per_word;
>>>>
>>>> -	if (spi_imx->dma_is_inited &&
>>>> -	    (transfer->len > spi_imx->wml * sizeof(u32)))
>>>> +	if (!bpw_w)
>>>> +		bpw_w = spi->bits_per_word;
>>>> +
>>>> +	bpw_w = DIV_ROUND_UP(bpw_w, 8);
>>>> +	if (spi_imx->dma_is_inited && (transfer->len > spi_imx->wml * bpw_w))
>>> Please remove bpw_w here as I talked in the first patch.
>> As explained in patch1 we need to use SPI word size in calculation to decide
>> if we want to go with DMA or PIO mode. Just a short example:
>> We need to transfer 24 SPI words with BPW == 32. This will take ~ 24 PIO writes
>> to FIFO (and same for reads). But transfer->len will be 24*4=96 bytes.
>> WML is 32 SPI words. The decision will be incorrect if we do not take into account
>> SPI bits per word.
> Yes, you are right, but I'm thinking so many 'DIV_ROUND_UP()'in your patch to
> get bpw, can you centralize it or just use the 'bpw_w' in 'struct spi_imx_data'?
We can't use bpw_w from spi_imx_data because this is external callback 
for SPI core and driver does not know the purpose of this call. We also 
can't update current bpw_w for the same reason.

I'll add a function like this:
int spi_imx_get_bytes_per_word(int bpw)
{
     return DIV_ROUND_UP(bpw, BITS_PER_BYTE);
}

>>>>   		return true;
>>>>   	return false;
>>>>   }

Regards,
Anton