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

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] [day] [month] [year] [list]

Message-ID: <5626CE25.8040706@gmail.com>
Date:	Wed, 21 Oct 2015 01:28:37 +0200
From:	Anton Bondarenko <anton.bondarenko.sama@...il.com>
To:	Robin Gong <b38343@...escale.com>
Cc:	"broonie@...nel.org" <broonie@...nel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-spi@...r.kernel.org" <linux-spi@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"Wang, Jiada (ESD)" <Jiada_Wang@...tor.com>,
	"Zapolskiy, Vladimir" <Vladimir_Zapolskiy@...tor.com>
Subject: Re: [PATCH v2 3/8] spi: imx: add support for all SPI word width for
 DMA transfer



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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/