linux-kernel - Re: [PATCH] mmc: Add Synopsys DesignWare mmc cmdq host driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACT4zj8yP0DPRfQ9_=gxaM+paZLW_86b5YVyGse-WiWK_KszPw@mail.gmail.com>
Date:   Wed, 6 Sep 2023 10:13:29 +0800
From:   Ben Chuang <benchuanggli@...il.com>
To:     Jyan Chou [周芷安] <jyanchou@...ltek.com>
Cc:     "adrian.hunter@...el.com" <adrian.hunter@...el.com>,
        "ulf.hansson@...aro.org" <ulf.hansson@...aro.org>,
        "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mmc: Add Synopsys DesignWare mmc cmdq host driver

Hi Jyan,

On Thu, Aug 31, 2023 at 3:47 PM Jyan Chou [周芷安] <jyanchou@...ltek.com> 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