* [PATCH V2] mmc: dw_mmc: exynos: fix the finding clock sample value
[not found] <CGME20211022082025epcas1p3a4f5908ec149414ff985d7d3ec414910@epcas1p3.samsung.com>
@ 2021-10-22 8:21 ` Jaehoon Chung
2021-10-22 10:53 ` Christian Hewitt
2021-10-26 15:40 ` Ulf Hansson
0 siblings, 2 replies; 6+ messages in thread
From: Jaehoon Chung @ 2021-10-22 8:21 UTC (permalink / raw)
To: linux-mmc
Cc: ulf.hansson, krzysztof.kozlowski, christianshewitt, mihailescu2m,
m.szyprowski, Jaehoon Chung
Even though there are candiates value if can't find best value, it's
returned -EIO. It's not proper behavior.
If there is not best value, use a first candiate value to work eMMC.
Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
Changelog V2:
- Add Marek's Tested-by tag
- Remove unnecessary code
drivers/mmc/host/dw_mmc-exynos.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index 0c75810812a0..1f8a3c0ddfe1 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -464,6 +464,18 @@ static s8 dw_mci_exynos_get_best_clksmpl(u8 candiates)
}
}
+ /*
+ * If there is no cadiates value, then it needs to return -EIO.
+ * If there are candiates values and don't find bset clk sample value,
+ * then use a first candiates clock sample value.
+ */
+ for (i = 0; i < iter; i++) {
+ __c = ror8(candiates, i);
+ if ((__c & 0x1) == 0x1) {
+ loc = i;
+ goto out;
+ }
+ }
out:
return loc;
}
@@ -494,6 +506,8 @@ static int dw_mci_exynos_execute_tuning(struct dw_mci_slot *slot, u32 opcode)
priv->tuned_sample = found;
} else {
ret = -EIO;
+ dev_warn(&mmc->class_dev,
+ "There is no candiates value about clksmpl!\n");
}
return ret;
--
2.29.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH V2] mmc: dw_mmc: exynos: fix the finding clock sample value
2021-10-22 8:21 ` [PATCH V2] mmc: dw_mmc: exynos: fix the finding clock sample value Jaehoon Chung
@ 2021-10-22 10:53 ` Christian Hewitt
2021-10-22 11:35 ` Marek Szyprowski
2021-10-26 15:40 ` Ulf Hansson
1 sibling, 1 reply; 6+ messages in thread
From: Christian Hewitt @ 2021-10-22 10:53 UTC (permalink / raw)
To: Jaehoon Chung
Cc: linux-mmc, Ulf Hansson, krzysztof.kozlowski, Marian Mihailescu,
m.szyprowski
> On 22 Oct 2021, at 12:21 pm, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>
> Even though there are candiates value if can't find best value, it's
> returned -EIO. It's not proper behavior.
> If there is not best value, use a first candiate value to work eMMC.
>
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Tested-by: Christian Hewitt <christianshewitt@gmail.com>
v2 patch working fine with the module that triggered the original report:
[ 2.902144] mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 400000Hz, actual 396825HZ div = 63)
[ 2.912118] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req 400000Hz, actual 396825HZ div = 63)
[ 3.142474] mmc_host mmc0: Bus speed (slot 0) = 200000000Hz (slot req 200000000Hz, actual 200000000HZ div = 0)
[ 3.239339] mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 52000000Hz, actual 50000000HZ div = 0)
[ 3.241388] mmc_host mmc0: Bus speed (slot 0) = 266666666Hz (slot req 200000000Hz, actual 133333333HZ div = 1)
[ 3.243310] mmc0: new HS400 MMC card at address 0001
[ 3.259191] mmcblk0: mmc0:0001 8GME4R 7.28 GiB
[ 3.302621] mmcblk0: p1 p2
[ 3.311541] mmcblk0boot0: mmc0:0001 8GME4R 4.00 MiB
[ 3.327737] mmcblk0boot1: mmc0:0001 8GME4R 4.00 MiB
[ 3.340919] mmcblk0rpmb: mmc0:0001 8GME4R 512 KiB, chardev (246:0)
Thanks!
> ---
> Changelog V2:
> - Add Marek's Tested-by tag
> - Remove unnecessary code
>
> drivers/mmc/host/dw_mmc-exynos.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
> index 0c75810812a0..1f8a3c0ddfe1 100644
> --- a/drivers/mmc/host/dw_mmc-exynos.c
> +++ b/drivers/mmc/host/dw_mmc-exynos.c
> @@ -464,6 +464,18 @@ static s8 dw_mci_exynos_get_best_clksmpl(u8 candiates)
> }
> }
>
> + /*
> + * If there is no cadiates value, then it needs to return -EIO.
> + * If there are candiates values and don't find bset clk sample value,
> + * then use a first candiates clock sample value.
> + */
> + for (i = 0; i < iter; i++) {
> + __c = ror8(candiates, i);
> + if ((__c & 0x1) == 0x1) {
> + loc = i;
> + goto out;
> + }
> + }
> out:
> return loc;
> }
> @@ -494,6 +506,8 @@ static int dw_mci_exynos_execute_tuning(struct dw_mci_slot *slot, u32 opcode)
> priv->tuned_sample = found;
> } else {
> ret = -EIO;
> + dev_warn(&mmc->class_dev,
> + "There is no candiates value about clksmpl!\n");
> }
>
> return ret;
> --
> 2.29.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V2] mmc: dw_mmc: exynos: fix the finding clock sample value
2021-10-22 10:53 ` Christian Hewitt
@ 2021-10-22 11:35 ` Marek Szyprowski
2021-10-22 11:42 ` Christian Hewitt
0 siblings, 1 reply; 6+ messages in thread
From: Marek Szyprowski @ 2021-10-22 11:35 UTC (permalink / raw)
To: Christian Hewitt, Jaehoon Chung
Cc: linux-mmc, Ulf Hansson, krzysztof.kozlowski, Marian Mihailescu
Hi,
On 22.10.2021 12:53, Christian Hewitt wrote:
>> On 22 Oct 2021, at 12:21 pm, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>
>> Even though there are candiates value if can't find best value, it's
>> returned -EIO. It's not proper behavior.
>> If there is not best value, use a first candiate value to work eMMC.
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Tested-by: Christian Hewitt <christianshewitt@gmail.com>
>
> v2 patch working fine with the module that triggered the original report:
>
> [ 2.902144] mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 400000Hz, actual 396825HZ div = 63)
> [ 2.912118] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req 400000Hz, actual 396825HZ div = 63)
> [ 3.142474] mmc_host mmc0: Bus speed (slot 0) = 200000000Hz (slot req 200000000Hz, actual 200000000HZ div = 0)
> [ 3.239339] mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 52000000Hz, actual 50000000HZ div = 0)
> [ 3.241388] mmc_host mmc0: Bus speed (slot 0) = 266666666Hz (slot req 200000000Hz, actual 133333333HZ div = 1)
I wonder why 266666666Hz bus speed is selected instead of the
400000000Hz one. Did you remove the workaround patch which changed the
divider value from your kernel tree? I didn't analyze the code, so maybe
this change is intentional result of this patch? On my XU4 I get
400000000Hz bus clock for the eMMC dw-mmc controller.
> [ 3.243310] mmc0: new HS400 MMC card at address 0001
> [ 3.259191] mmcblk0: mmc0:0001 8GME4R 7.28 GiB
> [ 3.302621] mmcblk0: p1 p2
> [ 3.311541] mmcblk0boot0: mmc0:0001 8GME4R 4.00 MiB
> [ 3.327737] mmcblk0boot1: mmc0:0001 8GME4R 4.00 MiB
> [ 3.340919] mmcblk0rpmb: mmc0:0001 8GME4R 512 KiB, chardev (246:0)
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V2] mmc: dw_mmc: exynos: fix the finding clock sample value
2021-10-22 11:35 ` Marek Szyprowski
@ 2021-10-22 11:42 ` Christian Hewitt
2021-10-27 23:03 ` Jaehoon Chung
0 siblings, 1 reply; 6+ messages in thread
From: Christian Hewitt @ 2021-10-22 11:42 UTC (permalink / raw)
To: Marek Szyprowski
Cc: Jaehoon Chung, linux-mmc, Ulf Hansson, krzysztof.kozlowski,
Marian Mihailescu
> On 22 Oct 2021, at 3:35 pm, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> Hi,
>
> On 22.10.2021 12:53, Christian Hewitt wrote:
>>> On 22 Oct 2021, at 12:21 pm, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>>
>>> Even though there are candiates value if can't find best value, it's
>>> returned -EIO. It's not proper behavior.
>>> If there is not best value, use a first candiate value to work eMMC.
>>>
>>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>>> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> Tested-by: Christian Hewitt <christianshewitt@gmail.com>
>>
>> v2 patch working fine with the module that triggered the original report:
>>
>> [ 2.902144] mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 400000Hz, actual 396825HZ div = 63)
>> [ 2.912118] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req 400000Hz, actual 396825HZ div = 63)
>> [ 3.142474] mmc_host mmc0: Bus speed (slot 0) = 200000000Hz (slot req 200000000Hz, actual 200000000HZ div = 0)
>> [ 3.239339] mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 52000000Hz, actual 50000000HZ div = 0)
>> [ 3.241388] mmc_host mmc0: Bus speed (slot 0) = 266666666Hz (slot req 200000000Hz, actual 133333333HZ div = 1)
>
> I wonder why 266666666Hz bus speed is selected instead of the
> 400000000Hz one. Did you remove the workaround patch which changed the
> divider value from your kernel tree? I didn't analyze the code, so maybe
> this change is intentional result of this patch? On my XU4 I get
> 400000000Hz bus clock for the eMMC dw-mmc controller.
Yes, I removed the workaround patch before testing. It’s delivering
the same result as the workaround so perhaps it’s normal for this
module. All the emmc modules I have (all samples from HK sent at the
same time) are identical so there’s nothing else I can test with.
Christian
>> [ 3.243310] mmc0: new HS400 MMC card at address 0001
>> [ 3.259191] mmcblk0: mmc0:0001 8GME4R 7.28 GiB
>> [ 3.302621] mmcblk0: p1 p2
>> [ 3.311541] mmcblk0boot0: mmc0:0001 8GME4R 4.00 MiB
>> [ 3.327737] mmcblk0boot1: mmc0:0001 8GME4R 4.00 MiB
>> [ 3.340919] mmcblk0rpmb: mmc0:0001 8GME4R 512 KiB, chardev (246:0)
>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V2] mmc: dw_mmc: exynos: fix the finding clock sample value
2021-10-22 8:21 ` [PATCH V2] mmc: dw_mmc: exynos: fix the finding clock sample value Jaehoon Chung
2021-10-22 10:53 ` Christian Hewitt
@ 2021-10-26 15:40 ` Ulf Hansson
1 sibling, 0 replies; 6+ messages in thread
From: Ulf Hansson @ 2021-10-26 15:40 UTC (permalink / raw)
To: Jaehoon Chung
Cc: linux-mmc, krzysztof.kozlowski, christianshewitt, mihailescu2m,
m.szyprowski
On Fri, 22 Oct 2021 at 10:20, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>
> Even though there are candiates value if can't find best value, it's
> returned -EIO. It's not proper behavior.
> If there is not best value, use a first candiate value to work eMMC.
>
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Applied for fixes and by adding the below tags, thanks!
Cc: stable@vger.kernel.org
Fixes: c537a1c5ff63 ("mmc: dw_mmc: exynos: add variable delay tuning sequence")
Please tell me if that doesn't make sense!
Kind regards
Uffe
> ---
> Changelog V2:
> - Add Marek's Tested-by tag
> - Remove unnecessary code
>
> drivers/mmc/host/dw_mmc-exynos.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
> index 0c75810812a0..1f8a3c0ddfe1 100644
> --- a/drivers/mmc/host/dw_mmc-exynos.c
> +++ b/drivers/mmc/host/dw_mmc-exynos.c
> @@ -464,6 +464,18 @@ static s8 dw_mci_exynos_get_best_clksmpl(u8 candiates)
> }
> }
>
> + /*
> + * If there is no cadiates value, then it needs to return -EIO.
> + * If there are candiates values and don't find bset clk sample value,
> + * then use a first candiates clock sample value.
> + */
> + for (i = 0; i < iter; i++) {
> + __c = ror8(candiates, i);
> + if ((__c & 0x1) == 0x1) {
> + loc = i;
> + goto out;
> + }
> + }
> out:
> return loc;
> }
> @@ -494,6 +506,8 @@ static int dw_mci_exynos_execute_tuning(struct dw_mci_slot *slot, u32 opcode)
> priv->tuned_sample = found;
> } else {
> ret = -EIO;
> + dev_warn(&mmc->class_dev,
> + "There is no candiates value about clksmpl!\n");
> }
>
> return ret;
> --
> 2.29.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V2] mmc: dw_mmc: exynos: fix the finding clock sample value
2021-10-22 11:42 ` Christian Hewitt
@ 2021-10-27 23:03 ` Jaehoon Chung
0 siblings, 0 replies; 6+ messages in thread
From: Jaehoon Chung @ 2021-10-27 23:03 UTC (permalink / raw)
To: Christian Hewitt, Marek Szyprowski
Cc: linux-mmc, Ulf Hansson, krzysztof.kozlowski, Marian Mihailescu
On 10/22/21 8:42 PM, Christian Hewitt wrote:
>
>> On 22 Oct 2021, at 3:35 pm, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>>
>> Hi,
>>
>> On 22.10.2021 12:53, Christian Hewitt wrote:
>>>> On 22 Oct 2021, at 12:21 pm, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>>>
>>>> Even though there are candiates value if can't find best value, it's
>>>> returned -EIO. It's not proper behavior.
>>>> If there is not best value, use a first candiate value to work eMMC.
>>>>
>>>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>>>> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> Tested-by: Christian Hewitt <christianshewitt@gmail.com>
>>>
>>> v2 patch working fine with the module that triggered the original report:
>>>
>>> [ 2.902144] mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 400000Hz, actual 396825HZ div = 63)
>>> [ 2.912118] mmc_host mmc1: Bus speed (slot 0) = 50000000Hz (slot req 400000Hz, actual 396825HZ div = 63)
>>> [ 3.142474] mmc_host mmc0: Bus speed (slot 0) = 200000000Hz (slot req 200000000Hz, actual 200000000HZ div = 0)
>>> [ 3.239339] mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 52000000Hz, actual 50000000HZ div = 0)
>>> [ 3.241388] mmc_host mmc0: Bus speed (slot 0) = 266666666Hz (slot req 200000000Hz, actual 133333333HZ div = 1)
>>
>> I wonder why 266666666Hz bus speed is selected instead of the
>> 400000000Hz one. Did you remove the workaround patch which changed the
>> divider value from your kernel tree? I didn't analyze the code, so maybe
>> this change is intentional result of this patch? On my XU4 I get
>> 400000000Hz bus clock for the eMMC dw-mmc controller.
>
> Yes, I removed the workaround patch before testing. It’s delivering
> the same result as the workaround so perhaps it’s normal for this
> module. All the emmc modules I have (all samples from HK sent at the
> same time) are identical so there’s nothing else I can test with.
As Marek was mentioned, I wonder why 266666666Hz is selected.
On my XU4, it's selected 400MHz as Marek's.
Best Regards,
Jaehoon Chung
>
> Christian
>
>>> [ 3.243310] mmc0: new HS400 MMC card at address 0001
>>> [ 3.259191] mmcblk0: mmc0:0001 8GME4R 7.28 GiB
>>> [ 3.302621] mmcblk0: p1 p2
>>> [ 3.311541] mmcblk0boot0: mmc0:0001 8GME4R 4.00 MiB
>>> [ 3.327737] mmcblk0boot1: mmc0:0001 8GME4R 4.00 MiB
>>> [ 3.340919] mmcblk0rpmb: mmc0:0001 8GME4R 512 KiB, chardev (246:0)
>>
>> Best regards
>> --
>> Marek Szyprowski, PhD
>> Samsung R&D Institute Poland
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-10-27 23:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20211022082025epcas1p3a4f5908ec149414ff985d7d3ec414910@epcas1p3.samsung.com>
2021-10-22 8:21 ` [PATCH V2] mmc: dw_mmc: exynos: fix the finding clock sample value Jaehoon Chung
2021-10-22 10:53 ` Christian Hewitt
2021-10-22 11:35 ` Marek Szyprowski
2021-10-22 11:42 ` Christian Hewitt
2021-10-27 23:03 ` Jaehoon Chung
2021-10-26 15:40 ` Ulf Hansson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox