Re: [PATCH] mmc: sdhci: Use "mmc" directly rather than "host->mmc"

Ulf Hansson
2021-03-11T10:58:42Z

On Thu, 11 Mar 2021 at 10:40, Jisheng Zhang <Jisheng.Zhang@synaptics.com> wrote:
>
> Clean up the code to use the "mmc" directly instead of "host->mmc".
> If the code sits in hot code path, this clean up also brings trvial
> performance improvement. Take the sdhci_post_req() for example:
>
> before the patch:
> ...
> 8d0: a9be7bfd stp x29, x30, [sp, #-32]!
> 8d4: 910003fd mov x29, sp
> 8d8: f9000bf3 str x19, [sp, #16]
> 8dc: f9400833 ldr x19, [x1, #16]
> 8e0: b9404261 ldr w1, [x19, #64]
> 8e4: 34000161 cbz w1, 910 <sdhci_post_req+0x50>
> 8e8: f9424400 ldr x0, [x0, #1160]
> 8ec: d2800004 mov x4, #0x0 // #0
> 8f0: b9401a61 ldr w1, [x19, #24]
> 8f4: b9403262 ldr w2, [x19, #48]
> 8f8: f9400000 ldr x0, [x0]
> 8fc: f278003f tst x1, #0x100
> 900: f9401e61 ldr x1, [x19, #56]
> 904: 1a9f17e3 cset w3, eq // eq = none
> 908: 11000463 add w3, w3, #0x1
> 90c: 94000000 bl 0 <dma_unmap_sg_attrs>
> ...
>
> After the patch:
> ...
> 8d0: a9be7bfd stp x29, x30, [sp, #-32]!
> 8d4: 910003fd mov x29, sp
> 8d8: f9000bf3 str x19, [sp, #16]
> 8dc: f9400833 ldr x19, [x1, #16]
> 8e0: b9404261 ldr w1, [x19, #64]
> 8e4: 34000141 cbz w1, 90c <sdhci_post_req+0x4c>
> 8e8: b9401a61 ldr w1, [x19, #24]
> 8ec: d2800004 mov x4, #0x0 // #0
> 8f0: b9403262 ldr w2, [x19, #48]
> 8f4: f9400000 ldr x0, [x0]
> 8f8: f278003f tst x1, #0x100
> 8fc: f9401e61 ldr x1, [x19, #56]
> 900: 1a9f17e3 cset w3, eq // eq = none
> 904: 11000463 add w3, w3, #0x1
> 908: 94000000 bl 0 <dma_unmap_sg_attrs>
> ...
>
> We saved one ldr instruction: "ldr x0, [x0, #1160]"

Nice!

Even if I think the cleanup of code makes sense alone.

>
> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> ---
> drivers/mmc/host/sdhci.c | 33 ++++++++++++++++-----------------
> 1 file changed, 16 insertions(+), 17 deletions(-)
>

[...]

> @@ -2489,14 +2489,14 @@ void sdhci_enable_sdio_irq(struct mmc_host *mmc, int enable)
> unsigned long flags;
>
> if (enable)
> - pm_runtime_get_noresume(host->mmc->parent);
> + pm_runtime_get_noresume(mmc->parent);

Maybe use mmc_dev(mmc) instead? At least I think I would appreciate
consistency in the entire c-file, today it seems like both
"mmc->parent" and mmc_dev(mmc) are being used.

[...]

Kind regards
Uffe