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;
Message-ID: <bee82951-47c9-20bc-3cb2-3a52f67a4728@sifive.com>
Date:   Fri, 29 Jul 2022 09:10:07 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.10.0
Subject: Re: [PATCH 2/3] mtd: spi-nor: add support for Quad Page Program to
 no_sfdp_flags
Content-Language: en-GB
To:     Tudor.Ambarus@microchip.com, sudip.mukherjee@sifive.com
Cc:     p.yadav@ti.com, michael@walle.cc, miquel.raynal@bootlin.com,
        richard@nod.at, vigneshr@ti.com, greentime.hu@sifive.com,
        jude.onyenegecha@sifive.com, william.salmon@sifive.com,
        adnan.chowdhury@sifive.com, linux-mtd@lists.infradead.org,
        linux-kernel@vger.kernel.org
References: <20220712163823.428126-1-sudip.mukherjee@sifive.com>
 <20220712163823.428126-3-sudip.mukherjee@sifive.com>
 <3a9877d0-3cc5-6452-764b-d07b38c72b00@microchip.com>
 <CAHyZL-fK-d+dPWG63dcFUz17uP-sG4bnSFVW_gt16snnOVTzpQ@mail.gmail.com>
 <CAHyZL-c9iSd8=CaQ00Nptea4cqp9f7oqS5VefT3vfcr2J3hmSQ@mail.gmail.com>
 <d5662307-200e-171d-58fd-8f26dc02c771@microchip.com>
From:   Ben Dooks <ben.dooks@sifive.com>
In-Reply-To: <d5662307-200e-171d-58fd-8f26dc02c771@microchip.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 29/07/2022 08:48, Tudor.Ambarus@microchip.com wrote:
> On 7/22/22 13:24, Sudip Mukherjee wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> Hi Tudor,
>>
> 
> Hi!
> 
>> On Mon, Jul 18, 2022 at 7:49 PM Sudip Mukherjee
>> <sudip.mukherjee@sifive.com> wrote:
>>>
>>> On Mon, Jul 18, 2022 at 6:02 PM <Tudor.Ambarus@microchip.com> wrote:
>>>>
>>>> On 7/12/22 19:38, Sudip Mukherjee wrote:
>>>>> [You don't often get email from sudip.mukherjee@sifive.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>>>>
>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>>
>>>>> Some flash chips which does not have a SFDP table can support Quad
>>>>> Input Page Program. Enable it in hwcaps if defined.
>>>>>
>>>
>>> <snip>
>>>
>>>>> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
>>>>> index 58fbedc94080f..dde636bdb1a7c 100644
>>>>> --- a/drivers/mtd/spi-nor/core.h
>>>>> +++ b/drivers/mtd/spi-nor/core.h
>>>>> @@ -462,6 +462,7 @@ struct spi_nor_fixups {
>>>>>    *   SPI_NOR_OCTAL_READ:      flash supports Octal Read.
>>>>>    *   SPI_NOR_OCTAL_DTR_READ:  flash supports octal DTR Read.
>>>>>    *   SPI_NOR_OCTAL_DTR_PP:    flash supports Octal DTR Page Program.
>>>>> + *   SPI_NOR_QUAD_PP:         flash supports Quad Input Page Program.
>>>>
>>>> You don't need this flag if your flash supports the 4-byte Address
>>>> Instruction Table. Does you flash support it? Can you dump all the
>>>> SFDP tables, please?
>>>
>>> Not sure what the correct way to dump sfdp is. I did this from sysfs.
>>
>> I tried decoding this SFDP table and I think the parameters table says
>> it has "3-Byte only addressing".
>> So, I guess that means it does not support 4-byte Address Instruction
>> Table. And the datasheet
>> says it supports "Quad Input Page Program (3-byte Address)".
>> My existing patchset works for Quad Input Page Program, and I can send
>> a v2 with the previous
>> patch and this merged together (as you suggested) or I can try
>> enabling sfdp for this chip and then use
> 
> You should definitely enable SFDP and get rid of the NO_SFDP_FLAGS flags,
> regardless of the 1-1-4 PP outcome.
> 
>> a fixup_flags to enable "Quad Input Page Program" which I think will
>> be more complicated.
>> Which one will you suggest?
>>
> 
> First I'd like to understand what "much better performance" means. Would
> you run some speed tests please? mtd-utils should have a speedtest, otherwise
> you can use the in kernel mtd_speedtest module. Page programs are slow anyway,
> using 4 lines may not make any difference. But let's see.
> 
> About your question, it depends on how common is 1-1-4 pp. If it's common and
> desirable we can introduce a flash info flag.

We have an issue with the SPI controller where if it isn't in the above 
1 bit modes it has to block the CPU as the feature to hold the clock is
not enabled in the 1bit "old" modes. In the old modes, if the FIFO gets
to the empty stage then the CS gets de-selected. If we can enable the
4bit mode, we can also set the clock-suspend flag which means the code
does not have to block the core it is running on to ensure it gets the
data out in time.

At the moment speed tests are not easy as we are on entirely emulated
hardware, we could try some but the results may not represent what the
real chip can perform.

-- 
Ben