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

[Anton Bondarenko]

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/