Hi,

On 08.09.21 21:00, Trent Piepho wrote:
> On Sun, Sep 5, 2021 at 6:52 AM Ahmad Fatoum <[email protected]> wrote:
>>
>> Correct the value in imx8mm_fracpll_tbl to match the one expanded by
>> MHZ(266) macro, rounding it down to MHz range only.
> 
> It's not really "correcting" the value, since the value is wrong.
> 
> 24 MHz / 9 * 400 / 2^2 = 266666666.6 Hz
> 
> Maybe it would be better to say, "make the value in imx8mm_facpll_tbl
> incorrect so it matches another incorrect value used elsewhere."
> 
> In which case, one has to wonder why it would not be better to use the
> correct value everywhere?

Fair point. I just ported the patch and called it a day.
Looking further into it, the rounding is strange as well.
166.75 MHz is rounded to 167, but 266.6 is rounded to 267.
It would be cleaner to just use MHz values through out and don't use
Hz values at all and always round up. This makes the code diverge
from U-Boot, but I guess that's acceptable. Works for you?

Thanks for the reviews by the way!

Cheers,
Ahmad

> 
>>
>> Signed-off-by: Andrey Zhizhikin <[email protected]>
>> [afa: ported from U-Boot 6ac4d4480663]
>> Signed-off-by: Ahmad Fatoum <[email protected]>
>> ---
>>  drivers/ddr/imx8m/ddrphy_utils.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/ddr/imx8m/ddrphy_utils.c 
>> b/drivers/ddr/imx8m/ddrphy_utils.c
>> index 9a4e1a22ee5e..6836e7d4b351 100644
>> --- a/drivers/ddr/imx8m/ddrphy_utils.c
>> +++ b/drivers/ddr/imx8m/ddrphy_utils.c
>> @@ -198,7 +198,7 @@ static struct imx_int_pll_rate_table 
>> imx8mm_fracpll_tbl[] = {
>>         { .rate = 600000000U,  .r1 = MDIV(300) | PDIV(3) | SDIV(2), .r2 = 0 
>> },
>>         { .rate = 594000000U,  .r1 = MDIV( 99) | PDIV(1) | SDIV(2), .r2 = 0 
>> },
>>         { .rate = 400000000U,  .r1 = MDIV(300) | PDIV(9) | SDIV(1), .r2 = 0 
>> },
>> -       { .rate = 266666667U,  .r1 = MDIV(400) | PDIV(9) | SDIV(2), .r2 = 0 
>> },
>> +       { .rate = 266000000U,  .r1 = MDIV(400) | PDIV(9) | SDIV(2), .r2 = 0 
>> },
>>         { .rate = 167000000U,  .r1 = MDIV(334) | PDIV(3) | SDIV(4), .r2 = 0 
>> },
>>         { .rate = 100000000U,  .r1 = MDIV(300) | PDIV(9) | SDIV(3), .r2 = 0 
>> },
>>  };
> 
> _______________________________________________
> barebox mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/barebox
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

Reply via email to