Messages in this thread | | | Date | Tue, 28 Mar 2023 16:48:59 +0530 | Subject | Re: [PATCH 2/5] mtd: rawnand: qcom: Add initial support for qspi nand | From | Md Sadre Alam <> |
| |
On 3/27/2023 8:19 PM, Miquel Raynal wrote: > Hello, > > + Mark > > quic_mdalam@quicinc.com wrote on Mon, 27 Mar 2023 19:24:02 +0530: > >> 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. > > Thanks. > > >> |------------------------| |------------------------------| |---------------------------------| >> |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. > > I'm not sure how generic these APIs are, please try to not put anything > raw NAND or SPI-NAND specific in the common file. I don't want to see > "if (raw)" or "if (spi)" conditions there.
I agree , I will not try to put any condition like if(raw) or if(spi).
I am doing in the below sequence.
1) exec->op() in raw nand driver
2) Move ecc related code to new ecc engine driver under ./drivers/mtd/nand/ecc-qcom.c and make raw nand driver to work with this ecc engine driver.
3) Write separate SPI-mem driver under ./drivers/spi/spi-qpic.c
4) Move common code under qpic_common.c under ./driver/mtd/nand/raw/qpic_common.c
> >> 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). > > On my side, I don't have a strong opinion yet but it looks good to me. > > Can you confirm you are considering switching to the ->exec_op() API in > the raw NAND driver?
Yes, first priority is switching to ->exec_op() API in raw nand driver. I have already started and sent couple of patches, will address latest comment send again.
> > Thanks, > Miquèl
| |