On 3/27/2023 8:19 PM, Miquel Raynal wrote:
Hello,
+ Mark
quic_mdalam@xxxxxxxxxxx wrote on Mon, 27 Mar 2023 19:24:02 +0530:
On 3/6/2023 8:08 PM, Miquel Raynal wrote:
Hello,
quic_mdalam@xxxxxxxxxxx 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@xxxxxxxxxxxxxx> 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
