Re: [PATCH] mmc: sdhci: Use "mmc" directly rather than "host->mmc"
On Thu, 11 Mar 2021 at 10:40, Jisheng Zhang 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: a9be7bfdstp x29, x30, [sp, #-32]!
> 8d4: 910003fdmov x29, sp
> 8d8: f9000bf3str x19, [sp, #16]
> 8dc: f9400833ldr x19, [x1, #16]
> 8e0: b9404261ldr w1, [x19, #64]
> 8e4: 34000161cbz w1, 910
> 8e8: f9424400ldr x0, [x0, #1160]
> 8ec: d284mov x4, #0x0// #0
> 8f0: b9401a61ldr w1, [x19, #24]
> 8f4: b9403262ldr w2, [x19, #48]
> 8f8: f940ldr x0, [x0]
> 8fc: f278003ftst x1, #0x100
> 900: f9401e61ldr x1, [x19, #56]
> 904: 1a9f17e3csetw3, eq // eq = none
> 908: 11000463add w3, w3, #0x1
> 90c: 9400bl 0
> ...
>
> After the patch:
> ...
> 8d0: a9be7bfdstp x29, x30, [sp, #-32]!
> 8d4: 910003fdmov x29, sp
> 8d8: f9000bf3str x19, [sp, #16]
> 8dc: f9400833ldr x19, [x1, #16]
> 8e0: b9404261ldr w1, [x19, #64]
> 8e4: 34000141cbz w1, 90c
> 8e8: b9401a61ldr w1, [x19, #24]
> 8ec: d284mov x4, #0x0// #0
> 8f0: b9403262ldr w2, [x19, #48]
> 8f4: f940ldr x0, [x0]
> 8f8: f278003ftst x1, #0x100
> 8fc: f9401e61ldr x1, [x19, #56]
> 900: 1a9f17e3csetw3, eq // eq = none
> 904: 11000463add w3, w3, #0x1
> 908: 9400bl 0
> ...
>
> We saved one ldr instruction: "ldr x0, [x0, #1160]"
>
> Signed-off-by: Jisheng Zhang
Applied for next, thanks!
Kind regards
Uffe
> ---
> drivers/mmc/host/sdhci.c | 33 -
> 1 file changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 2d73407ee52e..b1afabab9cae 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -907,7 +907,7 @@ static void sdhci_calc_sw_timeout(struct sdhci_host *host,
>
> if (data) {
> blksz = data->blksz;
> - freq = host->mmc->actual_clock ? : host->clock;
> + freq = mmc->actual_clock ? : host->clock;
> transfer_time = (u64)blksz * NSEC_PER_SEC * (8 / bus_width);
> do_div(transfer_time, freq);
> /* multiply by '2' to account for any unknowns */
> @@ -2269,14 +2269,14 @@ void sdhci_set_ios(struct mmc_host *mmc, struct
> mmc_ios *ios)
>
> if (host->quirks & SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK &&
> host->clock) {
> - host->timeout_clk = host->mmc->actual_clock ?
> - host->mmc->actual_clock /
> 1000 :
> + host->timeout_clk = mmc->actual_clock ?
> + mmc->actual_clock / 1000 :
> host->clock / 1000;
> - host->mmc->max_busy_timeout =
> + mmc->max_busy_timeout =
> host->ops->get_max_timeout_count ?
> host->ops->get_max_timeout_count(host) :
> 1 << 27;
> - host->mmc->max_busy_timeout /= host->timeout_clk;
> + mmc->max_busy_timeout /= host->timeout_clk;
> }
> }
>
> @@ -2399,7 +2399,7 @@ static int sdhci_get_cd(struct mmc_host *mmc)
> return 0;
>
> /* If nonremovable, assume that the card is always present. */
> - if (!mmc_card_is_removable(host->mmc))
> + if (!mmc_card_is_removable(mmc))
> return 1;
>
> /*
> @@ -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);
>
> spin_lock_irqsave(&host->lock, flags);
> sdhci_enable_sdio_irq_nolock(host, enable);
> spin_unlock_irqrestore(&host->lock, flags);
>
> if (!enable)
> - pm_runtime_put_noidle(host->mmc->parent);
> + pm_runtime_put_noidle(mmc->parent);
> }
> EXPORT_SYMBOL_GPL(sdhci_enable_sdio_irq);
>
> @@ -
Re: [PATCH] mmc: sdhci: Use "mmc" directly rather than "host->mmc"
On 11/03/21 11:40 am, Jisheng Zhang 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: a9be7bfdstp x29, x30, [sp, #-32]!
> 8d4: 910003fdmov x29, sp
> 8d8: f9000bf3str x19, [sp, #16]
> 8dc: f9400833ldr x19, [x1, #16]
> 8e0: b9404261ldr w1, [x19, #64]
> 8e4: 34000161cbz w1, 910
> 8e8: f9424400ldr x0, [x0, #1160]
> 8ec: d284mov x4, #0x0// #0
> 8f0: b9401a61ldr w1, [x19, #24]
> 8f4: b9403262ldr w2, [x19, #48]
> 8f8: f940ldr x0, [x0]
> 8fc: f278003ftst x1, #0x100
> 900: f9401e61ldr x1, [x19, #56]
> 904: 1a9f17e3csetw3, eq // eq = none
> 908: 11000463add w3, w3, #0x1
> 90c: 9400bl 0
> ...
>
> After the patch:
> ...
> 8d0: a9be7bfdstp x29, x30, [sp, #-32]!
> 8d4: 910003fdmov x29, sp
> 8d8: f9000bf3str x19, [sp, #16]
> 8dc: f9400833ldr x19, [x1, #16]
> 8e0: b9404261ldr w1, [x19, #64]
> 8e4: 34000141cbz w1, 90c
> 8e8: b9401a61ldr w1, [x19, #24]
> 8ec: d284mov x4, #0x0// #0
> 8f0: b9403262ldr w2, [x19, #48]
> 8f4: f940ldr x0, [x0]
> 8f8: f278003ftst x1, #0x100
> 8fc: f9401e61ldr x1, [x19, #56]
> 900: 1a9f17e3csetw3, eq // eq = none
> 904: 11000463add w3, w3, #0x1
> 908: 9400bl 0
> ...
>
> We saved one ldr instruction: "ldr x0, [x0, #1160]"
>
> Signed-off-by: Jisheng Zhang
Acked-by: Adrian Hunter
> ---
> drivers/mmc/host/sdhci.c | 33 -
> 1 file changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 2d73407ee52e..b1afabab9cae 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -907,7 +907,7 @@ static void sdhci_calc_sw_timeout(struct sdhci_host *host,
>
> if (data) {
> blksz = data->blksz;
> - freq = host->mmc->actual_clock ? : host->clock;
> + freq = mmc->actual_clock ? : host->clock;
> transfer_time = (u64)blksz * NSEC_PER_SEC * (8 / bus_width);
> do_div(transfer_time, freq);
> /* multiply by '2' to account for any unknowns */
> @@ -2269,14 +2269,14 @@ void sdhci_set_ios(struct mmc_host *mmc, struct
> mmc_ios *ios)
>
> if (host->quirks & SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK &&
> host->clock) {
> - host->timeout_clk = host->mmc->actual_clock ?
> - host->mmc->actual_clock / 1000 :
> + host->timeout_clk = mmc->actual_clock ?
> + mmc->actual_clock / 1000 :
> host->clock / 1000;
> - host->mmc->max_busy_timeout =
> + mmc->max_busy_timeout =
> host->ops->get_max_timeout_count ?
> host->ops->get_max_timeout_count(host) :
> 1 << 27;
> - host->mmc->max_busy_timeout /= host->timeout_clk;
> + mmc->max_busy_timeout /= host->timeout_clk;
> }
> }
>
> @@ -2399,7 +2399,7 @@ static int sdhci_get_cd(struct mmc_host *mmc)
> return 0;
>
> /* If nonremovable, assume that the card is always present. */
> - if (!mmc_card_is_removable(host->mmc))
> + if (!mmc_card_is_removable(mmc))
> return 1;
>
> /*
> @@ -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);
>
> spin_lock_irqsave(&host->lock, flags);
> sdhci_enable_sdio_irq_nolock(host, enable);
> spin_unlock_irqrestore(&host->lock, flags);
>
> if (!enable)
> - pm_runtime_put_noidle(host->mmc->parent);
> + pm_runtime_put_noidle(mmc->parent);
> }
> EXPORT_SYMBOL_GPL(sdhci_enable_sdio_irq);
>
> @@ -2837,7 +2837,7 @@ int sdhci_execute_tuning(struct mmc_host *mmc, u32
> opcode)
> goto out;
> }
>
> - host->mmc->retune_period
Re: [PATCH] mmc: sdhci: Use "mmc" directly rather than "host->mmc"
On Thu, 11 Mar 2021 at 10:40, Jisheng Zhang 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: a9be7bfdstp x29, x30, [sp, #-32]! > 8d4: 910003fdmov x29, sp > 8d8: f9000bf3str x19, [sp, #16] > 8dc: f9400833ldr x19, [x1, #16] > 8e0: b9404261ldr w1, [x19, #64] > 8e4: 34000161cbz w1, 910 > 8e8: f9424400ldr x0, [x0, #1160] > 8ec: d284mov x4, #0x0// #0 > 8f0: b9401a61ldr w1, [x19, #24] > 8f4: b9403262ldr w2, [x19, #48] > 8f8: f940ldr x0, [x0] > 8fc: f278003ftst x1, #0x100 > 900: f9401e61ldr x1, [x19, #56] > 904: 1a9f17e3csetw3, eq // eq = none > 908: 11000463add w3, w3, #0x1 > 90c: 9400bl 0 > ... > > After the patch: > ... > 8d0: a9be7bfdstp x29, x30, [sp, #-32]! > 8d4: 910003fdmov x29, sp > 8d8: f9000bf3str x19, [sp, #16] > 8dc: f9400833ldr x19, [x1, #16] > 8e0: b9404261ldr w1, [x19, #64] > 8e4: 34000141cbz w1, 90c > 8e8: b9401a61ldr w1, [x19, #24] > 8ec: d284mov x4, #0x0// #0 > 8f0: b9403262ldr w2, [x19, #48] > 8f4: f940ldr x0, [x0] > 8f8: f278003ftst x1, #0x100 > 8fc: f9401e61ldr x1, [x19, #56] > 900: 1a9f17e3csetw3, eq // eq = none > 904: 11000463add w3, w3, #0x1 > 908: 9400bl 0 > ... > > 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 > --- > 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
[PATCH] mmc: sdhci: Use "mmc" directly rather than "host->mmc"
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: a9be7bfdstp x29, x30, [sp, #-32]!
8d4: 910003fdmov x29, sp
8d8: f9000bf3str x19, [sp, #16]
8dc: f9400833ldr x19, [x1, #16]
8e0: b9404261ldr w1, [x19, #64]
8e4: 34000161cbz w1, 910
8e8: f9424400ldr x0, [x0, #1160]
8ec: d284mov x4, #0x0// #0
8f0: b9401a61ldr w1, [x19, #24]
8f4: b9403262ldr w2, [x19, #48]
8f8: f940ldr x0, [x0]
8fc: f278003ftst x1, #0x100
900: f9401e61ldr x1, [x19, #56]
904: 1a9f17e3csetw3, eq // eq = none
908: 11000463add w3, w3, #0x1
90c: 9400bl 0
...
After the patch:
...
8d0: a9be7bfdstp x29, x30, [sp, #-32]!
8d4: 910003fdmov x29, sp
8d8: f9000bf3str x19, [sp, #16]
8dc: f9400833ldr x19, [x1, #16]
8e0: b9404261ldr w1, [x19, #64]
8e4: 34000141cbz w1, 90c
8e8: b9401a61ldr w1, [x19, #24]
8ec: d284mov x4, #0x0// #0
8f0: b9403262ldr w2, [x19, #48]
8f4: f940ldr x0, [x0]
8f8: f278003ftst x1, #0x100
8fc: f9401e61ldr x1, [x19, #56]
900: 1a9f17e3csetw3, eq // eq = none
904: 11000463add w3, w3, #0x1
908: 9400bl 0
...
We saved one ldr instruction: "ldr x0, [x0, #1160]"
Signed-off-by: Jisheng Zhang
---
drivers/mmc/host/sdhci.c | 33 -
1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 2d73407ee52e..b1afabab9cae 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -907,7 +907,7 @@ static void sdhci_calc_sw_timeout(struct sdhci_host *host,
if (data) {
blksz = data->blksz;
- freq = host->mmc->actual_clock ? : host->clock;
+ freq = mmc->actual_clock ? : host->clock;
transfer_time = (u64)blksz * NSEC_PER_SEC * (8 / bus_width);
do_div(transfer_time, freq);
/* multiply by '2' to account for any unknowns */
@@ -2269,14 +2269,14 @@ void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios
*ios)
if (host->quirks & SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK &&
host->clock) {
- host->timeout_clk = host->mmc->actual_clock ?
- host->mmc->actual_clock / 1000 :
+ host->timeout_clk = mmc->actual_clock ?
+ mmc->actual_clock / 1000 :
host->clock / 1000;
- host->mmc->max_busy_timeout =
+ mmc->max_busy_timeout =
host->ops->get_max_timeout_count ?
host->ops->get_max_timeout_count(host) :
1 << 27;
- host->mmc->max_busy_timeout /= host->timeout_clk;
+ mmc->max_busy_timeout /= host->timeout_clk;
}
}
@@ -2399,7 +2399,7 @@ static int sdhci_get_cd(struct mmc_host *mmc)
return 0;
/* If nonremovable, assume that the card is always present. */
- if (!mmc_card_is_removable(host->mmc))
+ if (!mmc_card_is_removable(mmc))
return 1;
/*
@@ -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);
spin_lock_irqsave(&host->lock, flags);
sdhci_enable_sdio_irq_nolock(host, enable);
spin_unlock_irqrestore(&host->lock, flags);
if (!enable)
- pm_runtime_put_noidle(host->mmc->parent);
+ pm_runtime_put_noidle(mmc->parent);
}
EXPORT_SYMBOL_GPL(sdhci_enable_sdio_irq);
@@ -2837,7 +2837,7 @@ int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
goto out;
}
- host->mmc->retune_period = tuning_count;
+ mmc->retune_period = tuning_count;
if (host->tuning_delay < 0)
host->tuning_delay = opcode == MMC_SEND_TUN
