Messages in this thread | | | From | Jiasheng Jiang <> | Subject | Re: Re: [PATCH] mmc: sdhci-of-esdhc: Check for error num after setting mask | Date | Wed, 12 Jan 2022 16:17:15 +0800 |
| |
On Wed, Jan 12, 2022 at 02:45:13PM +0800, Adrian Hunter wrote: >> Because of the possible failure of the dma_supported(), the >> dma_set_mask_and_coherent() may return error num. >> Therefore, it should be better to check it and return the error if >> fails. >> Also, the caller, esdhc_of_resume(), should deal with the return >> value. >> Moreover, as the sdhci_esdhc_driver has not been used, it does not >> need to >> be considered. > > Apologies, but that last sentence I don't understand. Can you clarify > it a bit. > What doesn't need to be considered and why?
Thanks, because the original esdhc_of_enable_dma() only returns 0, the caller may not consider to check the return value. I also notice that the esdhc_of_enable_dma() is assigned to sdhci_esdhc_le_pdata and sdhci_esdhc_be_pdata, which is only used by sdhci_esdhc_driver. And now the sdhci_esdhc_driver only have 'probe' and 'remove', without other action. So we should not consider to check whether there is a caller for esdhc_of_enable_dma() in sdhci_esdhc_driver.
>> if (ret == 0) { >> /* Isn't this already done by sdhci_resume_host() ? >> --rmk */ >> - esdhc_of_enable_dma(host); >> + ret = esdhc_of_enable_dma(host); >> + if (ret) >> + return ret; >> + > > This is already done by sdhci_resume_host(), which assumes there can be > no > error if DMA has been enabled previously i.e. -> enable_dma() is called > at setup and the return value checked then. If it is possible that DMA > support can disappear later, then it would be better to address that in > SDHCI so that all SDHCI drivers get the benefit.
Fine, since it is already checked in setup, I think it is no need to check later.
I will send a v2 without the change of esdhc_of_resume().
Sincerely thanks, Jiang
| |