| Message ID | 20200306141405.24539-3-sr@denx.de |
|---|---|
| State | Accepted |
| Commit | 04d21a93fa36e43f992ed224dc9615492c578ce2 |
| Delegated to: | Daniel Schwierzeck |
| Headers | show |
| Series | [1/3] mips: mt76x8: ddr_cal: Rename dqs_test_valid() to dqs_test_error() | expand |
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>
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)
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(-)