mbox series

[v8,0/8] Add QPIC SPI NAND driver

Message ID 20240820104239.1774600-1-quic_mdalam@quicinc.com (mailing list archive)
Headers show
Series Add QPIC SPI NAND driver | expand

Message

Md Sadre Alam Aug. 20, 2024, 10:42 a.m. UTC
v8:
 * Fixed compilation warning reported by kernel test robot
 * Added "chip" description in nandc_set_read_loc_first()
 * Added "chip" description" in nandc_set_read_loc_last()
 * Changed data type of read_location0, read_location1,
   read_location2, read_location3, addr0, addr1, cmd, cfg0,
   cfg1, ecc_bch_cfg, ecc_buf_cfg, clrflashstatus, clrreadstatus,
   orig_cmd1, orig_vld to __le32 to fix compilation warning.
 * Included bitfield.h header file in spi-qpic-snand.c to
   fix compilation warning
 * Removed unused variable "steps" variable from 
   qcom_spi_ecc_init_ctx_pipelined()

v7:
 * Added read_oob() and write_oob() api
 * Added FIELD_PREP() in spi init
 * Made CONFIG_SPI_QPIC_SNAND and CONFIG_MTD_NAND_QCOM
   as bool type
 * Removed offset 0 in oob_ecc() layout
 * Handled multiple error condition

v6:
 * Added FIELD_PREP() and GENMASK() macro
 * Added qpic_spi_nand{..} structure for
   spi nand realted variables
 * Made qpic_common.c slectable based on
   either CONFIG_MTD_NAND_QCOM or CONFIG_SPI_QPIC_SNAND
 * Removed rawnand.h from qpic-common.h 
 * Removed partitions.h and rawnand.h form spi-qpic-snand.c
 * Added qcom_nand_unalloc() in remove()

v5:
 * Fixes nandbiterr issue
 * Added raw_read() and raw_write() API
 * Added qcom_ prefix to all the common API
 * Removed register indirection
 * Following tests for SPI-NAND devices passed

   - mtd_oobtest
   - mtd_pagetest
   - mtd_readtest
   - mtd_speedtest
   - mtd_stresstest
   - mtd_subpagetest
   - mtd_nandbiterrs
   - nandtest
   - nanddump
   - nandwrite
   - nandbiterr -i
   - mtd erase
   - mtd write
   - dd
   - hexddump

v4:
 * In this patch series fixes kernel doc for all the cmmon api
 * Also fixes dm-binding commit message
 * Fix qpic_common.c compilation based on config

v3:
 * In this patch series fixes multiple things like
   added clock-name, added _alloc_controller api instead
   of alloc_master, made common apis more generic etc.

 * Addressed all the comment from v2 patch series

v2:
 * https://lore.kernel.org/linux-arm-msm/20240215134856.1313239-1-quic_mdalam@quicinc.com/
 * In this series of patchs we have added basic working QPIC SPI NAND
   driver with READ, WRITE, ERASE etc functionality

 * Addressed all the comments given in RFC [v1] patch

v1:
 * https://lore.kernel.org/linux-arm-msm/20231031120307.1600689-1-quic_mdalam@quicinc.com/
 * Initial set of patches for handling QPIC SPI NAND.


Md Sadre Alam (8):
  spi: dt-bindings: Introduce qcom,spi-qpic-snand
  mtd: rawnand: qcom: cleanup qcom_nandc driver
  mtd: rawnand: qcom: Add qcom prefix to common api
  mtd: nand: Add qpic_common API file
  mtd: rawnand: qcom: use FIELD_PREP and GENMASK
  spi: spi-qpic: add driver for QCOM SPI NAND flash Interface
  arm64: dts: qcom: ipq9574: Add SPI nand support
  arm64: dts: qcom: ipq9574: Disable eMMC node

 .../bindings/spi/qcom,spi-qpic-snand.yaml     |   83 +
 .../boot/dts/qcom/ipq9574-rdp-common.dtsi     |   43 +
 arch/arm64/boot/dts/qcom/ipq9574-rdp433.dts   |    2 +-
 arch/arm64/boot/dts/qcom/ipq9574.dtsi         |   27 +
 drivers/mtd/nand/Makefile                     |    7 +
 drivers/mtd/nand/qpic_common.c                |  738 +++++++
 drivers/mtd/nand/raw/Kconfig                  |    2 +-
 drivers/mtd/nand/raw/qcom_nandc.c             | 1730 +++--------------
 drivers/spi/Kconfig                           |    8 +
 drivers/spi/Makefile                          |    1 +
 drivers/spi/spi-qpic-snand.c                  | 1637 ++++++++++++++++
 include/linux/mtd/nand-qpic-common.h          |  482 +++++
 12 files changed, 3331 insertions(+), 1429 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/spi/qcom,spi-qpic-snand.yaml
 create mode 100644 drivers/mtd/nand/qpic_common.c
 create mode 100644 drivers/spi/spi-qpic-snand.c
 create mode 100644 include/linux/mtd/nand-qpic-common.h

Comments

Md Sadre Alam Sept. 3, 2024, 9:15 a.m. UTC | #1
Hi Miquel,

On 8/20/2024 4:12 PM, Md Sadre Alam wrote:
> v8:
>   * Fixed compilation warning reported by kernel test robot
>   * Added "chip" description in nandc_set_read_loc_first()
>   * Added "chip" description" in nandc_set_read_loc_last()
>   * Changed data type of read_location0, read_location1,
>     read_location2, read_location3, addr0, addr1, cmd, cfg0,
>     cfg1, ecc_bch_cfg, ecc_buf_cfg, clrflashstatus, clrreadstatus,
>     orig_cmd1, orig_vld to __le32 to fix compilation warning.
>   * Included bitfield.h header file in spi-qpic-snand.c to
>     fix compilation warning
>   * Removed unused variable "steps" variable from
>     qcom_spi_ecc_init_ctx_pipelined()
> 
     I have addressed your comments to v6 and further posted till v8.
     Could you please let me know if this is fine.
     and how to get this merged ?

Regards,
Alam.
Miquel Raynal Sept. 3, 2024, 1:08 p.m. UTC | #2
Hi,

quic_mdalam@quicinc.com wrote on Tue, 3 Sep 2024 14:45:15 +0530:

> Hi Miquel,
> 
> On 8/20/2024 4:12 PM, Md Sadre Alam wrote:
> > v8:
> >   * Fixed compilation warning reported by kernel test robot
> >   * Added "chip" description in nandc_set_read_loc_first()
> >   * Added "chip" description" in nandc_set_read_loc_last()
> >   * Changed data type of read_location0, read_location1,
> >     read_location2, read_location3, addr0, addr1, cmd, cfg0,
> >     cfg1, ecc_bch_cfg, ecc_buf_cfg, clrflashstatus, clrreadstatus,
> >     orig_cmd1, orig_vld to __le32 to fix compilation warning.
> >   * Included bitfield.h header file in spi-qpic-snand.c to
> >     fix compilation warning
> >   * Removed unused variable "steps" variable from
> >     qcom_spi_ecc_init_ctx_pipelined()
> >   
>      I have addressed your comments to v6 and further posted till v8.
>      Could you please let me know if this is fine.
>      and how to get this merged ?

There are still kernel test robot reports, so this means there are
issues in your code that I don't need to point out explicitly, but I am
actively waiting for them to be fixed.

Thanks,
Miquèl
Krzysztof Kozlowski Sept. 3, 2024, 1:44 p.m. UTC | #3
On 03/09/2024 11:15, Md Sadre Alam wrote:
> Hi Miquel,
> 
> On 8/20/2024 4:12 PM, Md Sadre Alam wrote:
>> v8:
>>   * Fixed compilation warning reported by kernel test robot
>>   * Added "chip" description in nandc_set_read_loc_first()
>>   * Added "chip" description" in nandc_set_read_loc_last()
>>   * Changed data type of read_location0, read_location1,
>>     read_location2, read_location3, addr0, addr1, cmd, cfg0,
>>     cfg1, ecc_bch_cfg, ecc_buf_cfg, clrflashstatus, clrreadstatus,
>>     orig_cmd1, orig_vld to __le32 to fix compilation warning.
>>   * Included bitfield.h header file in spi-qpic-snand.c to
>>     fix compilation warning
>>   * Removed unused variable "steps" variable from
>>     qcom_spi_ecc_init_ctx_pipelined()
>>
>      I have addressed your comments to v6 and further posted till v8.
>      Could you please let me know if this is fine.
>      and how to get this merged ?

Two weeks ago you received reports that your code does not build
properly. So no, it is not fine. Please respond to the reports and/or
send corrected code.

Best regards,
Krzysztof
Md Sadre Alam Sept. 3, 2024, 2:20 p.m. UTC | #4
On 9/3/2024 6:38 PM, Miquel Raynal wrote:
> Hi,
> 
> quic_mdalam@quicinc.com wrote on Tue, 3 Sep 2024 14:45:15 +0530:
> 
>> Hi Miquel,
>>
>> On 8/20/2024 4:12 PM, Md Sadre Alam wrote:
>>> v8:
>>>    * Fixed compilation warning reported by kernel test robot
>>>    * Added "chip" description in nandc_set_read_loc_first()
>>>    * Added "chip" description" in nandc_set_read_loc_last()
>>>    * Changed data type of read_location0, read_location1,
>>>      read_location2, read_location3, addr0, addr1, cmd, cfg0,
>>>      cfg1, ecc_bch_cfg, ecc_buf_cfg, clrflashstatus, clrreadstatus,
>>>      orig_cmd1, orig_vld to __le32 to fix compilation warning.
>>>    * Included bitfield.h header file in spi-qpic-snand.c to
>>>      fix compilation warning
>>>    * Removed unused variable "steps" variable from
>>>      qcom_spi_ecc_init_ctx_pipelined()
>>>    
>>       I have addressed your comments to v6 and further posted till v8.
>>       Could you please let me know if this is fine.
>>       and how to get this merged ?
> 
> There are still kernel test robot reports, so this means there are
> issues in your code that I don't need to point out explicitly, but I am
> actively waiting for them to be fixed.

   Sorry I missed it, will fix and post next revision.
> 
> Thanks,
> Miquèl
Md Sadre Alam Sept. 3, 2024, 2:22 p.m. UTC | #5
On 9/3/2024 7:14 PM, Krzysztof Kozlowski wrote:
> On 03/09/2024 11:15, Md Sadre Alam wrote:
>> Hi Miquel,
>>
>> On 8/20/2024 4:12 PM, Md Sadre Alam wrote:
>>> v8:
>>>    * Fixed compilation warning reported by kernel test robot
>>>    * Added "chip" description in nandc_set_read_loc_first()
>>>    * Added "chip" description" in nandc_set_read_loc_last()
>>>    * Changed data type of read_location0, read_location1,
>>>      read_location2, read_location3, addr0, addr1, cmd, cfg0,
>>>      cfg1, ecc_bch_cfg, ecc_buf_cfg, clrflashstatus, clrreadstatus,
>>>      orig_cmd1, orig_vld to __le32 to fix compilation warning.
>>>    * Included bitfield.h header file in spi-qpic-snand.c to
>>>      fix compilation warning
>>>    * Removed unused variable "steps" variable from
>>>      qcom_spi_ecc_init_ctx_pipelined()
>>>
>>       I have addressed your comments to v6 and further posted till v8.
>>       Could you please let me know if this is fine.
>>       and how to get this merged ?
> 
> Two weeks ago you received reports that your code does not build
> properly. So no, it is not fine. Please respond to the reports and/or
> send corrected code.
   Sorry I missed it, will fix and post next revision.
> 
> Best regards,
> Krzysztof
>
Md Sadre Alam Sept. 10, 2024, 6:21 a.m. UTC | #6
On 9/3/2024 6:38 PM, Miquel Raynal wrote:
> Hi,
> 
> quic_mdalam@quicinc.com wrote on Tue, 3 Sep 2024 14:45:15 +0530:
> 
>> Hi Miquel,
>>
>> On 8/20/2024 4:12 PM, Md Sadre Alam wrote:
>>> v8:
>>>    * Fixed compilation warning reported by kernel test robot
>>>    * Added "chip" description in nandc_set_read_loc_first()
>>>    * Added "chip" description" in nandc_set_read_loc_last()
>>>    * Changed data type of read_location0, read_location1,
>>>      read_location2, read_location3, addr0, addr1, cmd, cfg0,
>>>      cfg1, ecc_bch_cfg, ecc_buf_cfg, clrflashstatus, clrreadstatus,
>>>      orig_cmd1, orig_vld to __le32 to fix compilation warning.
>>>    * Included bitfield.h header file in spi-qpic-snand.c to
>>>      fix compilation warning
>>>    * Removed unused variable "steps" variable from
>>>      qcom_spi_ecc_init_ctx_pipelined()
>>>    
>>       I have addressed your comments to v6 and further posted till v8.
>>       Could you please let me know if this is fine.
>>       and how to get this merged ?
> 
> There are still kernel test robot reports, so this means there are
> issues in your code that I don't need to point out explicitly, but I am
> actively waiting for them to be fixed.

I have fixed most of the sparse warnings after converting __le32 to u32.
However am not able to address the following sparse warnings

	drivers/mtd/nand/raw/qcom_nandc.c:1401:29: sparse: warning: cast to restricted __le32
	drivers/mtd/nand/raw/qcom_nandc.c:1587:30: sparse: warning: cast to restricted __le32
	drivers/mtd/nand/raw/qcom_nandc.c:1588:31: sparse: warning: cast to restricted __le32
	drivers/mtd/nand/raw/qcom_nandc.c:1589:34: sparse: warning: cast to restricted __le32
	drivers/mtd/nand/raw/qcom_nandc.c:2479:47: sparse:    got restricted __le32 [usertype]
	drivers/mtd/nand/raw/qcom_nandc.c:2480:47: sparse:    got restricted __le32 [usertype]
	drivers/mtd/nand/raw/qcom_nandc.c:2616:25: sparse: warning: cast to restricted __le32
	drivers/mtd/nand/raw/qcom_nandc.c:2672:32: sparse: warning: cast to restricted __le32

These warnings are seen with existing kernel code too. For example

	drivers/mtd/ftl.c:299:39: sparse: warning: cast to restricted __le32
	drivers/mtd/ftl.c:387:23: sparse:    got restricted __le32 [usertype]

Hence, can these be ignored as false positives and post the next version of the patches.
Kindly advice.

Thanks
Alam
Miquel Raynal Sept. 10, 2024, 7:41 a.m. UTC | #7
Hi,

> >>>    >>       I have addressed your comments to v6 and further posted till v8.  
> >>       Could you please let me know if this is fine.
> >>       and how to get this merged ?  
> > 
> > There are still kernel test robot reports, so this means there are
> > issues in your code that I don't need to point out explicitly, but I am
> > actively waiting for them to be fixed.  
> 
> I have fixed most of the sparse warnings after converting __le32 to u32.
> However am not able to address the following sparse warnings
> 
> 	drivers/mtd/nand/raw/qcom_nandc.c:1401:29: sparse: warning: cast to restricted __le32
> 	drivers/mtd/nand/raw/qcom_nandc.c:1587:30: sparse: warning: cast to restricted __le32
> 	drivers/mtd/nand/raw/qcom_nandc.c:1588:31: sparse: warning: cast to restricted __le32
> 	drivers/mtd/nand/raw/qcom_nandc.c:1589:34: sparse: warning: cast to restricted __le32
> 	drivers/mtd/nand/raw/qcom_nandc.c:2479:47: sparse:    got restricted __le32 [usertype]
> 	drivers/mtd/nand/raw/qcom_nandc.c:2480:47: sparse:    got restricted __le32 [usertype]
> 	drivers/mtd/nand/raw/qcom_nandc.c:2616:25: sparse: warning: cast to restricted __le32
> 	drivers/mtd/nand/raw/qcom_nandc.c:2672:32: sparse: warning: cast to restricted __le32

The rule is: you cannot add new warnings.

For existing warnings in the driver, I'd anyway advise to solve them.
Without the actual code I cannot help.

Thanks,
Miquèl
Md Sadre Alam Sept. 12, 2024, 6:18 a.m. UTC | #8
On 9/10/2024 1:11 PM, Miquel Raynal wrote:
> Hi,
> 
>>>>>     >>       I have addressed your comments to v6 and further posted till v8.
>>>>        Could you please let me know if this is fine.
>>>>        and how to get this merged ?
>>>
>>> There are still kernel test robot reports, so this means there are
>>> issues in your code that I don't need to point out explicitly, but I am
>>> actively waiting for them to be fixed.
>>
>> I have fixed most of the sparse warnings after converting __le32 to u32.
>> However am not able to address the following sparse warnings
>>
>> 	drivers/mtd/nand/raw/qcom_nandc.c:1401:29: sparse: warning: cast to restricted __le32
>> 	drivers/mtd/nand/raw/qcom_nandc.c:1587:30: sparse: warning: cast to restricted __le32
>> 	drivers/mtd/nand/raw/qcom_nandc.c:1588:31: sparse: warning: cast to restricted __le32
>> 	drivers/mtd/nand/raw/qcom_nandc.c:1589:34: sparse: warning: cast to restricted __le32
>> 	drivers/mtd/nand/raw/qcom_nandc.c:2479:47: sparse:    got restricted __le32 [usertype]
>> 	drivers/mtd/nand/raw/qcom_nandc.c:2480:47: sparse:    got restricted __le32 [usertype]
>> 	drivers/mtd/nand/raw/qcom_nandc.c:2616:25: sparse: warning: cast to restricted __le32
>> 	drivers/mtd/nand/raw/qcom_nandc.c:2672:32: sparse: warning: cast to restricted __le32
> 
> The rule is: you cannot add new warnings.
> 
> For existing warnings in the driver, I'd anyway advise to solve them.
> Without the actual code I cannot help.
I have fixed all the warnings and posted next revision.
> 
> Thanks,
> Miquèl