Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH v3 3/3] mtd: spi-nor: cadence-quadspi: disable the
 auto-poll for Intel LGM
To:     Vignesh Raghavendra <vigneshr@ti.com>,
        linux-mtd@lists.infradead.org
Cc:     linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
        dwmw2@infradead.org, computersforpeace@gmail.com, richard@nod.at,
        jwboyer@gmail.com, boris.brezillon@free-electrons.com,
        cyrille.pitchen@atmel.com, david.oberhollenzer@sigma-star.at,
        miquel.raynal@bootlin.com, tudor.ambarus@gmail.com,
        andriy.shevchenko@intel.com, cheol.yong.kim@intel.com,
        qi-ming.wu@intel.com
References: <20190909104733.14273-1-vadivel.muruganx.ramuthevar@linux.intel.com>
 <20190909104733.14273-4-vadivel.muruganx.ramuthevar@linux.intel.com>
 <a4d45efe-907f-6c87-c650-5ad19942f0e4@ti.com>
From:   "Ramuthevar, Vadivel MuruganX" 
        <vadivel.muruganx.ramuthevar@linux.intel.com>
Message-ID: <888a5cfa-7ded-938a-cdd6-cc11068117e4@linux.intel.com>
Date:   Wed, 16 Oct 2019 16:53:17 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101
 Thunderbird/60.9.0
MIME-Version: 1.0
In-Reply-To: <a4d45efe-907f-6c87-c650-5ad19942f0e4@ti.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Vignesh,

      Thank you for the review comments.

On 16/10/2019 4:40 PM, Vignesh Raghavendra wrote:
>
> On 09/09/19 4:17 PM, Ramuthevar,Vadivel MuruganX wrote:
>> From: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>
>>
>> On Intel's Lightning Mountain(LGM) SoC QSPI controller do not auto-poll.
>> This patch introduces to properly disable the auto-polling feature to
> This patch disables auto polling when direct access mode is disabled
> which should be noted in the commit message.
will add it.
>> improve the performance of cadence-quadspi.
> How does this improve performance of cadence-quadspi? I would expect HW
> auto-polling to be faster than SW polling.
During the bring-up time observed this, once again verify it on my setup.
Agreed, you are correct HW auto-polling is faster than SW polling.
>> Signed-off-by: Ramuthevar Vadivel Murugan <vadivel.muruganx.ramuthevar@linux.intel.com>
>> ---
>>   drivers/mtd/spi-nor/cadence-quadspi.c | 24 ++++++++++++++++++++++++
>>   1 file changed, 24 insertions(+)
>>
>> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
>> index 73b9fbd1508a..60998eaad1cc 100644
>> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
>> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
>> @@ -135,6 +135,8 @@ struct cqspi_driver_platdata {
>>   #define CQSPI_REG_RD_INSTR_TYPE_DATA_MASK	0x3
>>   #define CQSPI_REG_RD_INSTR_DUMMY_MASK		0x1F
>>   
>> +#define CQSPI_REG_WR_COMPLETION_CTRL		0x38
>> +#define CQSPI_REG_WR_COMPLETION_DISABLE_AUTO_POLL	BIT(14)
>>   #define CQSPI_REG_WR_INSTR			0x08
>>   #define CQSPI_REG_WR_INSTR_OPCODE_LSB		0
>>   #define CQSPI_REG_WR_INSTR_TYPE_ADDR_LSB	12
>> @@ -471,6 +473,18 @@ static int cqspi_command_write_addr(struct spi_nor *nor,
>>   	return cqspi_exec_flash_cmd(cqspi, reg);
>>   }
>>   
>> +static int cqspi_disable_auto_poll(struct cqspi_st *cqspi)
>> +{
>> +	void __iomem *reg_base = cqspi->iobase;
>> +	unsigned int reg;
>> +
>> +	reg = readl(reg_base + CQSPI_REG_WR_COMPLETION_CTRL);
>> +	reg |= CQSPI_REG_WR_COMPLETION_DISABLE_AUTO_POLL;
>> +	writel(reg, reg_base + CQSPI_REG_WR_COMPLETION_CTRL);
>> +
>> +	return 0;
>> +}
>> +
>>   static int cqspi_read_setup(struct spi_nor *nor)
>>   {
>>   	struct cqspi_flash_pdata *f_pdata = nor->priv;
>> @@ -508,6 +522,11 @@ static int cqspi_read_setup(struct spi_nor *nor)
>>   	reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK;
>>   	reg |= (nor->addr_width - 1);
>>   	writel(reg, reg_base + CQSPI_REG_SIZE);
>> +
>> +	/* Disable auto-polling */
>> +	if (!f_pdata->use_direct_mode)
>> +		cqspi_disable_auto_poll(cqspi);
>> +
>>   	return 0;
>>   }
>>   
> Hmmm.. There is no need to disable polling for every read/write
> operation. It should be enough to do it once in cqspi_controller_init()
sure, move to cqspi_controller_init() .
---
Regards
Vadivel
>
>
>> @@ -627,6 +646,11 @@ static int cqspi_write_setup(struct spi_nor *nor)
>>   	reg &= ~CQSPI_REG_SIZE_ADDRESS_MASK;
>>   	reg |= (nor->addr_width - 1);
>>   	writel(reg, reg_base + CQSPI_REG_SIZE);
>> +
>> +	/* Disable auto-polling */
>> +	if (!f_pdata->use_direct_mode)
>> +		cqspi_disable_auto_poll(cqspi);
>> +
>>   	return 0;
>>   }
>>   
>>