Re: [PATCH] clk: rockchip: Slightly more accurate math in rockchip_mmc_get_phase()

From: Heiko Stuebner <heiko@sntech.de>
To: Douglas Anderson <dianders@chromium.org>
Cc: hal@halemmerich.com, amstan@chromium.org,
	Stephen Boyd <sboyd@kernel.org>,
	Shawn Lin <shawn.lin@rock-chips.com>,
	linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org,
	linux-rockchip@lists.infradead.org,
	Michael Turquette <mturquette@baylibre.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] clk: rockchip: Slightly more accurate math in rockchip_mmc_get_phase()
Date: Thu, 09 May 2019 10:45:56 +0200	[thread overview]
Message-ID: <1830378.zUSKOufHgj@phil> (raw)
In-Reply-To: <20190507205742.50835-1-dianders@chromium.org>

Am Dienstag, 7. Mai 2019, 22:57:42 CEST schrieb Douglas Anderson:
> There's a bit of math in rockchip_mmc_get_phase() to calculate the
> "fine delay".  This math boils down to:
> 
>  PSECS_PER_SEC = 1000000000000.
>  ROCKCHIP_MMC_DELAY_ELEMENT_PSEC = 60
>  card_clk * ROCKCHIP_MMC_DELAY_ELEMENT_PSEC * 360 * x / PSECS_PER_SEC
> 
> ...but we do it in pieces to avoid overflowing 32-bits.  Right now we
> overdo it a little bit, though, and end up getting less accurate math
> than we could.  Right now we do:
> 
>  DIV_ROUND_CLOSEST((card_clk / 1000000) *
>                    (ROCKCHIP_MMC_DELAY_ELEMENT_PSEC / 10) *
>                    (360 / 10) *
> 		   delay_num,
> 		   PSECS_PER_SEC / 1000000 / 10 / 10)
> 
> This is non-ideal because:
> A) The pins on Rockchip SoCs are rated to go at most 150 MHz, so the
>    max card clock is 150 MHz.  Even ignoring this the maximum SD card
>    clock (for SDR104) would be 208 MHz.  This means you can decrease
>    your division by 100x and still not overflow:
>      hex(208000000 / 10000 * 6 * 36 * 0xff) == 0x44497200
> B) On many Rockchip SoCs we end up with a card clock that is actually
>    148500000 because we parent off the 297 MHz PLL.  That means the
>    math we're actually doing today is less than ideal.  Specifically:
>    148500000 / 1000000 = 148
> 
> Let's fix the math to be slightly more accurate.
> 
> NOTE: no known problems are fixed by this.  It was found simply by
> code inspection.  If you want to see the difference between the old
> and the new on a 148.5 MHz clock, this python can help:
> 
>   old = [x for x in
>          (int(round(148 * 6 * 36 * x / 10000.)) for x in range(256))
> 	 if x < 90]
>   new = [x for x in
>          (int(round(1485 * 6 * 36 * x / 100000.)) for x in range(256))
> 	 if x < 90]
> 
> The only differences are:
>   delay_num=17 54=>55
>   delay_num=22 70=>71
>   delay_num=27 86=>87
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

gave this a spin on multiple socs and all of them still detected a hs200-
card, so I've applied that for 5.3

Thanks
Heiko


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel