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: <745e6bd6-96a5-ceab-1f0c-31440d34e4c8@quicinc.com>
Date:   Mon, 27 Mar 2023 19:24:02 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.9.0
Subject: Re: [PATCH 2/5] mtd: rawnand: qcom: Add initial support for qspi nand
Content-Language: en-US
To:     Miquel Raynal <miquel.raynal@bootlin.com>
CC:     Boris Brezillon <boris.brezillon@collabora.com>,
        <linux-kernel@vger.kernel.org>, <devicetree@vger.kernel.org>,
        <linux-mtd@lists.infradead.org>, <linux-arm-msm@vger.kernel.org>,
        <robh+dt@kernel.org>, <vigneshr@ti.com>, <richard@nod.at>,
        <agross@kernel.org>, <bjorn.andersson@linaro.org>,
        <quic_srichara@quicinc.com>, <qpic_varada@quicinc.com>,
        <quic_sjaganat@quicinc.com>
References: <1602307902-16761-1-git-send-email-mdalam@codeaurora.org>
 <1602307902-16761-3-git-send-email-mdalam@codeaurora.org>
 <20201029100751.713e27df@collabora.com>
 <b70ddb40-a1f1-f967-6b7b-057a39b0bcc2@quicinc.com>
 <20230306153851.0dcdda27@xps-13>
From:   Md Sadre Alam <quic_mdalam@quicinc.com>
In-Reply-To: <20230306153851.0dcdda27@xps-13>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


On 3/6/2023 8:08 PM, Miquel Raynal wrote:
> Hello,
> 
> quic_mdalam@quicinc.com wrote on Mon, 6 Mar 2023 19:45:58 +0530:
> 
>> On 10/29/2020 2:37 PM, Boris Brezillon wrote:
>>> Hello,
>>>
>>> On Sat, 10 Oct 2020 11:01:39 +0530
>>> Md Sadre Alam <mdalam@codeaurora.org> wrote:
>>>   
>>>> This change will add initial support for qspi (serial nand).
>>>>
>>>> QPIC Version v.2.0 onwards supports serial nand as well so this
>>>> change will initialize all required register to enable qspi (serial
>>>> nand).
>>>>
>>>> This change is supporting very basic functionality of qspi nand flash.
>>>>
>>>> 1. Reset device (Reset QSPI NAND device).
>>>>
>>>> 2. Device detection (Read id QSPI NAND device).
>>> Unfortunately, that's not going to work in the long term. You're
>>> basically hacking the raw NAND framework to make SPI NANDs fit. I do
>>> understand the rationale behind this decision (re-using the code for
>>> ECC and probably other things), but that's not going to work. So I'd
>>> recommend doing the following instead:
>>>
>>> 1/ implement a SPI-mem controller driver
>>> 2/ implement an ECC engine driver so the ECC logic can be shared
>>>      between the SPI controller and raw NAND controller drivers
>>> 3/ convert the raw NAND driver to the exec_op() interface (none of
>>>      this hack would have been possible if the driver was using the new
>>>      API)
>>>
>>> Regards,
>>>
>>> Boris
>>>   
>>      Sorry for late reply, again started working on this feature support.  The QPIC v2 on wards there is serial nand support got added , its not a standard SPI controller
>>
>>      its QPIC controller having support for serial nand. All SPI related configuration done by QPIC hardware and its not exposed as SPI bus to the external world. Only based on
>>
>>      QPIC_SPI_CFG = 1, serial functionality will get selected. So that no need to implement as SPI-mem controller driver, since its not a SPI controller.
>>
>>     Please check the below diagram for top view of QPIC controller.
> 
> One of the hard things in the Linux kernel is to make devices fit
> frameworks. This feature does not fit the raw NAND framework. It does
> not follow any of the conventions taken there. It is not gonna be
> accepted there. You need to expose spi-mem functionalities, even if the
> spi-proper features are not available. I believe your situation still
> fits the spi-mem abstraction.
> 
> Thanks,
> Miquèl


I have started writing the driver code for SPI NAND. Please check the below design,
is this fine as per Boris suggestion.


           |------------------------|                      |------------------------------|                        |---------------------------------|
           |qcom spi nand driver    |--------------------->|common ECC engine driver      |<-----------------------|qcom raw nand driver             |
           |                        |                      |                              |                        |                                 |
           |                        |                      |drivers/mtd/nand/ecc-qcom.c   |                        |drivers/mtd/nand/raw/qcom_nand.c |
           |                        |                      |                              |                        |                                 |
           |drivers/spi/spi-qpic.c  |                      |------------------------------|                        |                                 |
           |                        |                      |common API file:              |                        |                                 |
           |                        |                      |common API: reset, read id,   |                        |                                 |
           |                        |--------------------->|erase, read page, write page, |<-----------------------|                                 |
           |------------------------|                      |bad block check etc.          |                        |                                 |
                                                           |                              |                        |---------------------------------|
                                                           |drivers/mtd/nand/raw/qpic_comm|
                                                           |    on.c                      |
                                                           |------------------------------|


Here ECC engine driver as separate file under (drivers/mtd/nand/ecc-qcom.c) and all
common APIs like reset, read id, erase, write page, read page, check block bad etc.
as separate file under drivers/mtd/nand/raw/qpic_common.c.APIs under ECC engine drivers
and qpic_common.c will be exported and used by spi-qpic.c driver (Serial NAND) and qcom_nand.c
(raw nand driver).

Thanks,
Alam.