* [PATCH 1/3] mips: mt76x8: ddr_cal: Rename dqs_test_valid() to dqs_test_error()
@ 2020-03-06 14:14 Stefan Roese
2020-03-06 14:14 ` [PATCH 2/3] mips: mt76x8: ddr_cal: Change types from u32 to int in dqs_find_min/max Stefan Roese
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Stefan Roese @ 2020-03-06 14:14 UTC (permalink / raw)
To: u-boot
This function returns "-1" (true) upon error. So the function name does
not match its implementation which is confusing. This patch renames the
function to dqs_test_error() which makes the code easier to read.
Also change the return type to bool and return "true" or "false".
Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Weijie Gao <weijie.gao@mediatek.com>
Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
---
arch/mips/mach-mtmips/ddr_cal.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/mips/mach-mtmips/ddr_cal.c b/arch/mips/mach-mtmips/ddr_cal.c
index 0ea7c7d5e1..b934c4a854 100644
--- a/arch/mips/mach-mtmips/ddr_cal.c
+++ b/arch/mips/mach-mtmips/ddr_cal.c
@@ -32,8 +32,8 @@ static inline void pref_op(int op, const volatile void *addr)
__asm__ __volatile__("pref %0, 0(%1)" : : "i" (op), "r" (addr));
}
-static inline int dqs_test_valid(void __iomem *memc, u32 memsize, u32 dqsval,
- u32 bias)
+static inline bool dqs_test_error(void __iomem *memc, u32 memsize, u32 dqsval,
+ u32 bias)
{
u32 *nca, *ca;
u32 off;
@@ -64,11 +64,11 @@ static inline int dqs_test_valid(void __iomem *memc, u32 memsize, u32 dqsval,
for (i = 0; i < TEST_PAT_SIZE / sizeof(u32); i++) {
if (ca[i] != (u32)nca + i + bias)
- return -1;
+ return true;
}
}
- return 0;
+ return false;
}
static inline u32 dqs_find_max(void __iomem *memc, u32 memsize, u32 initval,
@@ -79,7 +79,7 @@ static inline u32 dqs_find_max(void __iomem *memc, u32 memsize, u32 initval,
do {
dqsval = regval | (fieldval << shift);
- if (dqs_test_valid(memc, memsize, dqsval, 3))
+ if (dqs_test_error(memc, memsize, dqsval, 3))
break;
fieldval++;
@@ -96,7 +96,7 @@ static inline u32 dqs_find_min(void __iomem *memc, u32 memsize, u32 initval,
while (fieldval > minval) {
dqsval = regval | (fieldval << shift);
- if (dqs_test_valid(memc, memsize, dqsval, 1)) {
+ if (dqs_test_error(memc, memsize, dqsval, 1)) {
fieldval++;
break;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] mips: mt76x8: ddr_cal: Change types from u32 to int in dqs_find_min/max
2020-03-06 14:14 [PATCH 1/3] mips: mt76x8: ddr_cal: Rename dqs_test_valid() to dqs_test_error() Stefan Roese
@ 2020-03-06 14:14 ` Stefan Roese
2020-04-15 7:21 ` Weijie Gao
2020-03-06 14:14 ` [PATCH 3/3] mips: mt76x8: ddr_cal: Correct dqs_find_min/max implementations Stefan Roese
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Stefan Roese @ 2020-03-06 14:14 UTC (permalink / raw)
To: u-boot
This change is made to enable comparison of integer variables, which
might be negative in the next patch. No functional change is intended
in this patch.
Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Weijie Gao <weijie.gao@mediatek.com>
Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
---
arch/mips/mach-mtmips/ddr_cal.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/arch/mips/mach-mtmips/ddr_cal.c b/arch/mips/mach-mtmips/ddr_cal.c
index b934c4a854..80a058d693 100644
--- a/arch/mips/mach-mtmips/ddr_cal.c
+++ b/arch/mips/mach-mtmips/ddr_cal.c
@@ -71,10 +71,11 @@ static inline bool dqs_test_error(void __iomem *memc, u32 memsize, u32 dqsval,
return false;
}
-static inline u32 dqs_find_max(void __iomem *memc, u32 memsize, u32 initval,
- u32 maxval, u32 shift, u32 regval)
+static inline int dqs_find_max(void __iomem *memc, u32 memsize, int initval,
+ int maxval, int shift, u32 regval)
{
- u32 fieldval = initval, dqsval;
+ int fieldval = initval;
+ u32 dqsval;
do {
dqsval = regval | (fieldval << shift);
@@ -88,10 +89,11 @@ static inline u32 dqs_find_max(void __iomem *memc, u32 memsize, u32 initval,
return fieldval;
}
-static inline u32 dqs_find_min(void __iomem *memc, u32 memsize, u32 initval,
- u32 minval, u32 shift, u32 regval)
+static inline int dqs_find_min(void __iomem *memc, u32 memsize, int initval,
+ int minval, int shift, u32 regval)
{
- u32 fieldval = initval, dqsval;
+ int fieldval = initval;
+ u32 dqsval;
while (fieldval > minval) {
dqsval = regval | (fieldval << shift);
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] mips: mt76x8: ddr_cal: Correct dqs_find_min/max implementations
2020-03-06 14:14 [PATCH 1/3] mips: mt76x8: ddr_cal: Rename dqs_test_valid() to dqs_test_error() Stefan Roese
2020-03-06 14:14 ` [PATCH 2/3] mips: mt76x8: ddr_cal: Change types from u32 to int in dqs_find_min/max Stefan Roese
@ 2020-03-06 14:14 ` Stefan Roese
2020-04-15 7:20 ` Weijie Gao
2020-04-15 7:19 ` [PATCH 1/3] mips: mt76x8: ddr_cal: Rename dqs_test_valid() to dqs_test_error() Weijie Gao
2020-04-27 13:38 ` Daniel Schwierzeck
3 siblings, 1 reply; 8+ messages in thread
From: Stefan Roese @ 2020-03-06 14:14 UTC (permalink / raw)
To: u-boot
The current implementations have some issues detecting the correct
values:
dqs_find_max() will return "last passing fieldval + 1" instead of
"last passing fieldval". Also it will return "maxval + 1" in the
case that all fieldvals are tested valid (without error).
dqs_find_min() will not test the "lowest" value because of using ">"
instead of ">=".
This patch now rewrites these functions to fix those issues. Also,
this patch uses the same approach of a for loop in both functions making
it easier to read and maintain.
Since the variables are integers now, we can use min()/max(), which
handles the wrap around case for fieldval=0: return (0 - 1).
Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Weijie Gao <weijie.gao@mediatek.com>
Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
---
arch/mips/mach-mtmips/ddr_cal.c | 28 ++++++++++------------------
1 file changed, 10 insertions(+), 18 deletions(-)
diff --git a/arch/mips/mach-mtmips/ddr_cal.c b/arch/mips/mach-mtmips/ddr_cal.c
index 80a058d693..71a53c3c9c 100644
--- a/arch/mips/mach-mtmips/ddr_cal.c
+++ b/arch/mips/mach-mtmips/ddr_cal.c
@@ -74,39 +74,31 @@ static inline bool dqs_test_error(void __iomem *memc, u32 memsize, u32 dqsval,
static inline int dqs_find_max(void __iomem *memc, u32 memsize, int initval,
int maxval, int shift, u32 regval)
{
- int fieldval = initval;
+ int fieldval;
u32 dqsval;
- do {
+ for (fieldval = initval; fieldval <= maxval; fieldval++) {
dqsval = regval | (fieldval << shift);
-
if (dqs_test_error(memc, memsize, dqsval, 3))
- break;
-
- fieldval++;
- } while (fieldval <= maxval);
+ return max(fieldval - 1, initval);
+ }
- return fieldval;
+ return maxval;
}
static inline int dqs_find_min(void __iomem *memc, u32 memsize, int initval,
int minval, int shift, u32 regval)
{
- int fieldval = initval;
+ int fieldval;
u32 dqsval;
- while (fieldval > minval) {
+ for (fieldval = initval; fieldval >= minval; fieldval--) {
dqsval = regval | (fieldval << shift);
-
- if (dqs_test_error(memc, memsize, dqsval, 1)) {
- fieldval++;
- break;
- }
-
- fieldval--;
+ if (dqs_test_error(memc, memsize, dqsval, 1))
+ return min(fieldval + 1, initval);
}
- return fieldval;
+ return minval;
}
void ddr_calibrate(void __iomem *memc, u32 memsize, u32 bw)
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 1/3] mips: mt76x8: ddr_cal: Rename dqs_test_valid() to dqs_test_error()
2020-03-06 14:14 [PATCH 1/3] mips: mt76x8: ddr_cal: Rename dqs_test_valid() to dqs_test_error() Stefan Roese
2020-03-06 14:14 ` [PATCH 2/3] mips: mt76x8: ddr_cal: Change types from u32 to int in dqs_find_min/max Stefan Roese
2020-03-06 14:14 ` [PATCH 3/3] mips: mt76x8: ddr_cal: Correct dqs_find_min/max implementations Stefan Roese
@ 2020-04-15 7:19 ` Weijie Gao
2020-04-15 7:46 ` Stefan Roese
2020-04-27 13:38 ` Daniel Schwierzeck
3 siblings, 1 reply; 8+ messages in thread
From: Weijie Gao @ 2020-04-15 7:19 UTC (permalink / raw)
To: u-boot
On Fri, 2020-03-06 at 15:14 +0100, Stefan Roese wrote:
> This function returns "-1" (true) upon error. So the function name does
> not match its implementation which is confusing. This patch renames the
> function to dqs_test_error() which makes the code easier to read.
>
> Also change the return type to bool and return "true" or "false".
>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Weijie Gao <weijie.gao@mediatek.com>
> Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
> ---
> arch/mips/mach-mtmips/ddr_cal.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
Reviewed-by: Weijie Gao <weijie.gao@mediatek.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/3] mips: mt76x8: ddr_cal: Correct dqs_find_min/max implementations
2020-03-06 14:14 ` [PATCH 3/3] mips: mt76x8: ddr_cal: Correct dqs_find_min/max implementations Stefan Roese
@ 2020-04-15 7:20 ` Weijie Gao
0 siblings, 0 replies; 8+ messages in thread
From: Weijie Gao @ 2020-04-15 7:20 UTC (permalink / raw)
To: u-boot
On Fri, 2020-03-06 at 15:14 +0100, Stefan Roese wrote:
> The current implementations have some issues detecting the correct
> values:
>
> dqs_find_max() will return "last passing fieldval + 1" instead of
> "last passing fieldval". Also it will return "maxval + 1" in the
> case that all fieldvals are tested valid (without error).
>
> dqs_find_min() will not test the "lowest" value because of using ">"
> instead of ">=".
>
> This patch now rewrites these functions to fix those issues. Also,
> this patch uses the same approach of a for loop in both functions making
> it easier to read and maintain.
>
> Since the variables are integers now, we can use min()/max(), which
> handles the wrap around case for fieldval=0: return (0 - 1).
>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Weijie Gao <weijie.gao@mediatek.com>
> Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
> ---
> arch/mips/mach-mtmips/ddr_cal.c | 28 ++++++++++------------------
> 1 file changed, 10 insertions(+), 18 deletions(-)
>
Reviewed-by: Weijie Gao <weijie.gao@mediatek.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/3] mips: mt76x8: ddr_cal: Change types from u32 to int in dqs_find_min/max
2020-03-06 14:14 ` [PATCH 2/3] mips: mt76x8: ddr_cal: Change types from u32 to int in dqs_find_min/max Stefan Roese
@ 2020-04-15 7:21 ` Weijie Gao
0 siblings, 0 replies; 8+ messages in thread
From: Weijie Gao @ 2020-04-15 7:21 UTC (permalink / raw)
To: u-boot
On Fri, 2020-03-06 at 15:14 +0100, Stefan Roese wrote:
> This change is made to enable comparison of integer variables, which
> might be negative in the next patch. No functional change is intended
> in this patch.
>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Weijie Gao <weijie.gao@mediatek.com>
> Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
> ---
> arch/mips/mach-mtmips/ddr_cal.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
Reviewed-by: Weijie Gao <weijie.gao@mediatek.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] mips: mt76x8: ddr_cal: Rename dqs_test_valid() to dqs_test_error()
2020-04-15 7:19 ` [PATCH 1/3] mips: mt76x8: ddr_cal: Rename dqs_test_valid() to dqs_test_error() Weijie Gao
@ 2020-04-15 7:46 ` Stefan Roese
0 siblings, 0 replies; 8+ messages in thread
From: Stefan Roese @ 2020-04-15 7:46 UTC (permalink / raw)
To: u-boot
Hi Weijie,
thanks for reviewing. Could you please also review the latest RFC
version of my "generification" of the legacy lzma support as well?
I really would like to see some review comments before sending v7
of this patchset to get it (hopefully) accepted in this merge window.
http://patchwork.ozlabs.org/project/uboot/patch/20200410110431.12256-1-sr at denx.de/
http://patchwork.ozlabs.org/project/uboot/patch/20200410110431.12256-2-sr at denx.de/
http://patchwork.ozlabs.org/project/uboot/patch/20200410110431.12256-2-sr at denx.de/
Thanks,
Stefan
On 15.04.20 09:19, Weijie Gao wrote:
> On Fri, 2020-03-06 at 15:14 +0100, Stefan Roese wrote:
>> This function returns "-1" (true) upon error. So the function name does
>> not match its implementation which is confusing. This patch renames the
>> function to dqs_test_error() which makes the code easier to read.
>>
>> Also change the return type to bool and return "true" or "false".
>>
>> Signed-off-by: Stefan Roese <sr@denx.de>
>> Cc: Weijie Gao <weijie.gao@mediatek.com>
>> Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
>> ---
>> arch/mips/mach-mtmips/ddr_cal.c | 12 ++++++------
>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>
>
> Reviewed-by: Weijie Gao <weijie.gao@mediatek.com>
>
Viele Gr??e,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] mips: mt76x8: ddr_cal: Rename dqs_test_valid() to dqs_test_error()
2020-03-06 14:14 [PATCH 1/3] mips: mt76x8: ddr_cal: Rename dqs_test_valid() to dqs_test_error() Stefan Roese
` (2 preceding siblings ...)
2020-04-15 7:19 ` [PATCH 1/3] mips: mt76x8: ddr_cal: Rename dqs_test_valid() to dqs_test_error() Weijie Gao
@ 2020-04-27 13:38 ` Daniel Schwierzeck
3 siblings, 0 replies; 8+ messages in thread
From: Daniel Schwierzeck @ 2020-04-27 13:38 UTC (permalink / raw)
To: u-boot
Am 06.03.20 um 15:14 schrieb Stefan Roese:
> This function returns "-1" (true) upon error. So the function name does
> not match its implementation which is confusing. This patch renames the
> function to dqs_test_error() which makes the code easier to read.
>
> Also change the return type to bool and return "true" or "false".
>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Weijie Gao <weijie.gao@mediatek.com>
> Cc: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>
> ---
> arch/mips/mach-mtmips/ddr_cal.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
series applied to u-boot-mips/next
--
- Daniel
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-04-27 13:38 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-06 14:14 [PATCH 1/3] mips: mt76x8: ddr_cal: Rename dqs_test_valid() to dqs_test_error() Stefan Roese
2020-03-06 14:14 ` [PATCH 2/3] mips: mt76x8: ddr_cal: Change types from u32 to int in dqs_find_min/max Stefan Roese
2020-04-15 7:21 ` Weijie Gao
2020-03-06 14:14 ` [PATCH 3/3] mips: mt76x8: ddr_cal: Correct dqs_find_min/max implementations Stefan Roese
2020-04-15 7:20 ` Weijie Gao
2020-04-15 7:19 ` [PATCH 1/3] mips: mt76x8: ddr_cal: Rename dqs_test_valid() to dqs_test_error() Weijie Gao
2020-04-15 7:46 ` Stefan Roese
2020-04-27 13:38 ` Daniel Schwierzeck
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.