Hi Ben, Thanks for your comment and suggestion. > Apart from the difference in register definitions and the addition of cmdq, is there any other behavior that is different from dw_mmc.c? > I recommend using a patch series and describing the differences from dw_mmc in your cover letter, for an example as follows. We had modified our patch into a patch series and fixed compile error. Please refer to the commits below. > Do you forget to add dw_mmc_cqe.o and dw_mmc_cqe-rtk.o to Makefile? Thanks for your remind, we had added dw_mmc_cqe.o, dw_mmc_cqe-rtk.o into new version patch. https://patchwork.kernel.org/project/linux-mmc/patch/da1f7fbae1dd34cfc5d4bcecf3a2323f382ffd3a.1693991785.git.jyanchou@xxxxxxxxxxx/ https://patchwork.kernel.org/project/linux-mmc/patch/9617f04133ba8b6907b253c4154083f75956a341.1693991785.git.jyanchou@xxxxxxxxxxx/ https://patchwork.kernel.org/project/linux-mmc/patch/9cc6c51d8513c0dca5399420d753825183aa98f4.1693991785.git.jyanchou@xxxxxxxxxxx/ Best regards, Jyan Chou -----Original Message----- From: Ben Chuang <benchuanggli@xxxxxxxxx> Sent: Wednesday, September 6, 2023 10:13 AM To: Jyan Chou [周芷安] <jyanchou@xxxxxxxxxxx> Cc: adrian.hunter@xxxxxxxxx; ulf.hansson@xxxxxxxxxx; linux-mmc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx Subject: Re: [PATCH] mmc: Add Synopsys DesignWare mmc cmdq host driver External mail. Hi Jyan, On Thu, Aug 31, 2023 at 3:47 PM Jyan Chou [周芷安] <jyanchou@xxxxxxxxxxx> wrote: > > Hi Ben, > Thanks for your suggestion. > > > The patch includes two parts: a dw_mmc_cqe driver and dw_mmc_cqe-rtk driver. > > Adrian and Ulf's comments[1][2] don't seem to be addressed. > > [1] The reason why we added many changes was because we found out that > synopsys IP data book's register and user guide with cmdq support were > different from non cmdq's , so we referred to dw_mmc.c coding style > and push dw_mmc_cqe.c to support version after 5.1 JEDEC Standard. > Apart from the difference in register definitions and the addition of cmdq, is there any other behavior that is different from dw_mmc.c? I recommend using a patch series and describing the differences from dw_mmc in your cover letter, for an example as follows [00/04] cover letter - Add DesignWare Mobile mmc driver [01/04] Introduce a setup_tran_desc ops ... [02/04] Add dw mobile_mmc driver ..... [03/04] Add command queue to dw mobile_mmc driver ..... [04/04] Add dw mobile mmc rtk driver ..... And please read patiently https://www.kernel.org/doc/html/latest/process/submitting-patches.html. >>--- >> drivers/mmc/host/Kconfig | 22 + Do you forget to add dw_mmc_cqe.o and dw_mmc_cqe-rtk.o to Makefile? >> drivers/mmc/host/cqhci-core.c | 5 + >> drivers/mmc/host/cqhci.h | 2 + >> drivers/mmc/host/dw_mmc_cqe-rtk.c | 999 ++++++++++++++++++ >> drivers/mmc/host/dw_mmc_cqe-rtk.h | 160 +++ >> drivers/mmc/host/dw_mmc_cqe.c | 1633 +++++++++++++++++++++++++++++ >> drivers/mmc/host/dw_mmc_cqe.h | 442 ++++++++ >> 7 files changed, 3263 insertions(+) And some compile complains for your reference, --- drivers/mmc/host/dw_mmc_cqe.c: In function 'dw_mci_cqe_err_handle': drivers/mmc/host/dw_mmc_cqe.c:723:41: warning: this 'if' clause does not guard... [-Wmisleading-indentation] 723 | if (err == -DW_MCI_NOT_READY) | ^~ drivers/mmc/host/dw_mmc_cqe.c:726:49: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if' 726 | break; | ^~~~~ ---- In file included from drivers/mmc/host/dw_mmc_cqe-rtk.c:23: drivers/mmc/host/dw_mmc_cqe-rtk.h:155:5: error: conflicting types for 'mmc_hw_reset'; have 'int(struct mmc_host *)' 155 | int mmc_hw_reset(struct mmc_host *host); | ^~~~~~~~~~~~ In file included from drivers/mmc/host/dw_mmc_cqe-rtk.c:11: ./include/linux/mmc/core.h:178:5: note: previous declaration of 'mmc_hw_reset' with type 'int(struct mmc_card *)' 178 | int mmc_hw_reset(struct mmc_card *card); ---- Best regards, Ben Chuang ------Please consider the environment before printing this e-mail.
