linux-kernel - Re: [PATCH 0/2] Extending PLL LUT for i.MX8MP Samsung HDMI PHY

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHCN7xJD8jsqyZX1JkWxrA84XkZ8YYN19hXW6KVe+jkOFugqrw@mail.gmail.com>
Date: Tue, 10 Sep 2024 20:16:04 -0500
From: Adam Ford <aford173@...il.com>
To: Dominique Martinet <dominique.martinet@...ark-techno.com>
Cc: Frieder Schrempf <frieder@...s.de>, Kishon Vijay Abraham I <kishon@...nel.org>, linux-kernel@...r.kernel.org, 
	linux-phy@...ts.infradead.org, Vinod Koul <vkoul@...nel.org>, 
	Lucas Stach <l.stach@...gutronix.de>, Marco Felsch <m.felsch@...gutronix.de>, 
	Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
Subject: Re: [PATCH 0/2] Extending PLL LUT for i.MX8MP Samsung HDMI PHY

On Tue, Sep 10, 2024 at 7:23 PM Dominique Martinet
<dominique.martinet@...ark-techno.com> wrote:
>
> Frieder Schrempf wrote on Tue, Sep 10, 2024 at 08:14:51PM +0200:
> > [2] https://codeberg.org/fschrempf/samsung-hdmi-phy-pll-calculator/src/branch/main/pll.py
>
> Great work! Thanks!
>
> I was curious about the existing table entries, recomputing existing
> values doesn't yield the same values. For example, the first entry is
> {
>         .pixclk = 22250000,
>         .pll_div_regs = { 0xd1, 0x4b, 0xf1, 0x89, 0x88, 0x80, 0x40 },
> }
> but computing it yields
> {
>     .pixclk = 22250000,
>     .pll_div_regs = { 0xd1, 0x4a, 0xf0, 0xef, 0x10, 0x81, 0x40 },
> }
>
> I assume there just are multiple ways to generate the same frequencies,
> which is fine in itself, but it'd be great to be able to "back-compute"
> the entries as a sanity check.
>
> I've played a bit with your script and spent more time on it than I'd
> like to admit, but something like this seems to do the trick, plugging
> in the regs from the kernel:
>
> ---
> pll = FractionalNPLL(freq_ref)
>
> regs = [0xd1, 0x4b, 0xf1, 0x89, 0x88, 0x80, 0x40]
> # assume fractional
> if not regs[0] & 0xD0:
>     print("reg[0] missing 0xD0")
>     sys.exit(1)
> pll.freq_frac = True
> pll.params["p"] = regs[0] & 0x2f
> pll.params["m"] = regs[1]
> pll.params["s"] = (regs[2] >> 4) + 1
> pll.params["n2"] = ((regs[2] >> 3) & 0x1) + 1
> pll.params["n"] = (regs[2] & 0x7) + 4
> pll.params["lc"] = regs[3] & 0x7f
> if regs[4] & 0x80:
>     pll.params["lc"] = - pll.params["lc"]
> pll.params["k"] = regs[4] & 0x7f
> pll.params["lc_s"] = regs[5] & 0x7f
> pll.params["k_s"] = regs[6] & 0xbf
>
>
> f_vco = int(pll.params["m"] * pll.f_ref / pll.params["p"])
> if f_vco < 750000000 or f_vco > 3000000000:
>     print(f"f_vco {f_vco} out of range")
>     sys.exit(1)
> f_calc = f_vco / pll.params["s"] / 5
> pll.freq_int = round(f_calc)
> print(f_calc)
> sdc = pll.calc_sdc(pll.params)
> frac = pll.calc_f_frac(sdc, pll.params)
> print(frac)
> freq = pll.freq_int + frac
> print(freq)
> pll.print_reg_driver_data(freq)
> exit(0);
> ---
> yields this back (comments added manually)
> ---
> 22500000.0 (integer part)
> -250000.0 (fractional part)
> 22250000.0 (summed)
>
> PHY Driver Table Entry:
> {
>     .pixclk = 22250000.0,
>     .pll_div_regs = { 0xd1, 0x4b, 0xf1, 0x89, 0x88, 0x81, 0x40 },
> }
> ---
>
> so if I find some time I'll whip some loop to check all other values...
>
>
>
>
> That aside, I see no problem with this, just one meta-comment about
> adding a link to the script in an external repo: I see some other
> drivers have python scripts in their trees e.g.
> drivers/comedi/drivers/ni_routing/tools/*py
> drivers/gpu/drm/ci/xfails/update-xfails.py
> drivers/gpu/drm/msm/registers/gen_header.py
>
> would it make sense to commit the script here instead of linking to a
> repo that might be lost in the future?
>
> I'm not quite sure what policy the linux repo has here, so leaving that
> as an open question.

Is there a reason this couldn't be coded in C and used to expand my
integer calculator series?  With that, we could drop the lookup table.

adam
> --
> Dominique