Re: [PATCH 1/1] mmc: sdhci: check host->cmd is null or not in sdhci_cqe_irq()

[Adrian Hunter <adrian.hunter@intel.com>]

On 7/09/22 13:03, Ulf Hansson wrote:
> Hi Adrian,
> 
> On Wed, 7 Sept 2022 at 09:06, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>
>> On 31/08/22 15:43, Ying-Chun Liu (PaulLiu) wrote:
>>> From: "Ying-Chun Liu (PaulLiu)" <paul.liu@linaro.org>
>>>
>>> If we don't check host->cmd. We will get the following oops
>>> when booting from eMMC on RockPi4.
>>>
>>>  Unable to handle kernel read from unreadable memory at
>>>    virtual address 0000000000000000
>>>  Call trace:
>>>    sdhci_cqe_irq+0x40/0x254
>>
>> Thank you!  However, I prefer this patch:
>>
>> https://lore.kernel.org/linux-mmc/20220907035847.13783-1-wenchao.chen666@gmail.com/
> 
> That patch means that we need to do a sdhci_readw() to figure out what
> command we are running, rather than just checking for a valid pointer.
> Maybe it's not a big deal as this code isn't executed that often?

That was my thinking, but CQE/CQHCI does not set host->cmd either.
It is the same approach taken in sdhci_data_irq() so consistent at
least.

> 
> Anyway, it's your call to make, I pick the patch that you prefer.
> 
> Kind regards
> Uffe
> 
>>
>>>
>>> Signed-off-by: Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org>
>>> Signed-off-by: Ram Muthiah <rammuthiah@google.com>
>>> Cc: Adrian Hunter <adrian.hunter@intel.com>
>>> Cc: Ritesh Harjani <riteshh@codeaurora.org>
>>> Cc: Asutosh Das <asutoshd@codeaurora.org>
>>> Cc: Ulf Hansson <ulf.hansson@linaro.org>
>>> Cc: linux-mmc@vger.kernel.org
>>> ---
>>>  drivers/mmc/host/sdhci.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index 7689ffec5ad1..e0e7768bd6c0 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -3928,7 +3928,7 @@ bool sdhci_cqe_irq(struct sdhci_host *host, u32 intmask, int *cmd_error,
>>>
>>>       if (intmask & (SDHCI_INT_INDEX | SDHCI_INT_END_BIT | SDHCI_INT_CRC)) {
>>>               *cmd_error = -EILSEQ;
>>> -             if (!mmc_op_tuning(host->cmd->opcode))
>>> +             if (host->cmd && !mmc_op_tuning(host->cmd->opcode))
>>>                       sdhci_err_stats_inc(host, CMD_CRC);
>>>       } else if (intmask & SDHCI_INT_TIMEOUT) {
>>>               *cmd_error = -ETIMEDOUT;
>>> @@ -3938,7 +3938,7 @@ bool sdhci_cqe_irq(struct sdhci_host *host, u32 intmask, int *cmd_error,
>>>
>>>       if (intmask & (SDHCI_INT_DATA_END_BIT | SDHCI_INT_DATA_CRC)) {
>>>               *data_error = -EILSEQ;
>>> -             if (!mmc_op_tuning(host->cmd->opcode))
>>> +             if (host->cmd && !mmc_op_tuning(host->cmd->opcode))
>>>                       sdhci_err_stats_inc(host, DAT_CRC);
>>>       } else if (intmask & SDHCI_INT_DATA_TIMEOUT) {
>>>               *data_error = -ETIMEDOUT;
>>