| 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
| ||
|
Message-ID: <bbf4ec7e-ef77-485b-b748-4bb3a6baba59@kontron.de>
Date: Wed, 11 Sep 2024 09:01:13 +0200
From: Frieder Schrempf <frieder.schrempf@...tron.de>
To: Adam Ford <aford173@...il.com>, linux-phy@...ts.infradead.org
Cc: dominique.martinet@...ark-techno.com, linux-imx@....com,
festevam@...il.com, aford@...conembedded.com, Sandor.yu@....com,
Vinod Koul <vkoul@...nel.org>, Kishon Vijay Abraham I <kishon@...nel.org>,
Marco Felsch <m.felsch@...gutronix.de>, Lucas Stach
<l.stach@...gutronix.de>, Uwe Kleine-König
<u.kleine-koenig@...gutronix.de>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V7 3/5] xphy: freescale: fsl-samsung-hdmi: Support dynamic
integer
On 11.09.24 3:28 AM, Adam Ford wrote:
> There is currently a look-up table for a variety of resolutions.
> Since the phy has the ability to dynamically calculate the values
> necessary to use the intger divider which should allow more
^ integer
> resolutions without having to update the look-up-table.
>
> If the lookup table cannot find an exact match, fall back to the
> dynamic calculator of the integer divider.
>
> Previously, the value of P was hard-coded to 1, this required an
> update to the phy_pll_cfg table to add in the extra value into the
> table, so if the value of P is calculated to be something else
> by the PMS calculator, the calculated_phy_pll_cfg structure
> can be used instead without having to keep track of which method
> was used.
>
> Signed-off-by: Adam Ford <aford173@...il.com>
> ---
> V7: Refactored much of the code to create smaller helper functions
> to eliminate redundant code and improve code flow and comment
> readability. Any t-b and s-o-b tags removed due to the
> extent of the changes.
>
> V6: Fix comment typos and remove an unnecesary extra calculation
> by using the cached value.
> V5: No Change
> V4: No Change
> V3: Change size of pll_div_regs to include PHY_REG01 (P)
> Create calculated_phy_pll_cfg to containe the values
> Eliminate the PMS calculation from fsl_samsung_hdmi_phy_configure
> Make the LUT primary and fall back to integer calculator in
> phy_clk_round_rate.
> Check the range right away to ensure it's reaonsable rather than
> trying to find a clock only to learn it's outside the range.
> Overall added notes and comments where stuff may not be intuitive.
>
> V2: Update phy_clk_round_rate and phy_clk_set_rate to both support
> the integer clock PMS calculator.
> ---
> drivers/phy/freescale/phy-fsl-samsung-hdmi.c | 371 +++++++++++++------
> 1 file changed, 265 insertions(+), 106 deletions(-)
>
> diff --git a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
> index 4f6874226f9a..49317a96f767 100644
[...]
> +
> + /* The ref manual states the values of 'P' range from 1 to 11 */
> + for (_p = 1; _p <= 11; ++_p) {
> + for (_s = 1; _s <= 16; ++_s) {
> + u64 tmp;
> + u32 delta;
> +
> + /* s must be one or even */
> + if (_s > 1 && (_s & 0x01) == 1)
> + _s++;
> +
> + /* _s cannot be 14 per the TRM */
> + if (_s == 14)
> + continue;
> +
> + /*
> + * TODO: Ref Manual doesn't state the range of _m
> + * so this should be further refined if possible.
> + * This range was set based on the original values
> + * in the lookup table
> + */
> + tmp = (u64)fout * (_p * _s);
> + do_div(tmp, 24 * MHZ);
> + _m = tmp;
> + if (_m < 0x30 || _m > 0x7b)
> + continue;
> +
> + /*
> + * Rev 2 of the Ref Manual states the
> + * VCO can range between 750MHz and
> + * 3GHz. The VCO is assumed to be
^ whitespace
> + * is assumed to be (M * f_ref) / P,
^ duplicate "is assumed to be"
> + * where f_ref is 24MHz.
> + */
> + tmp = (u64)_m * 24 * MHZ;
> + do_div(tmp, _p);
> + if (tmp < 750 * MHZ ||
> + tmp > 3000 * MHZ)
> + continue;
> +
> + /* Final frequency after post-divider */
> + do_div(tmp, _s);
> +
> + delta = abs(fout - tmp);
> + if (delta < min_delta) {
> + best_p = _p;
> + best_s = _s;
> + best_m = _m;
> + min_delta = delta;
> + best_freq = tmp;
> + }
> + }
> + }
> +
> + if (best_freq) {
> + *p = best_p;
> + *m = best_m;
> + *s = best_s;
> + }
> +
> + return best_freq / 5;
> +}
> +
[...]
>
> static int phy_clk_set_rate(struct clk_hw *hw,
> unsigned long rate, unsigned long parent_rate)
> {
> struct fsl_samsung_hdmi_phy *phy = to_fsl_samsung_hdmi_phy(hw);
> - int i;
> + const struct phy_config *fract_div_phy;
> + u32 int_div_clk;
> + u16 m;
> + u8 p, s;
>
> - for (i = ARRAY_SIZE(phy_pll_cfg) - 1; i >= 0; i--)
> - if (phy_pll_cfg[i].pixclk <= rate)
> - break;
> + /* Search the fractional divider lookup table */
> + fract_div_phy = fsl_samsung_hdmi_phy_lookup_rate(rate);
>
> - if (i < 0)
> - return -EINVAL;
> + /* If the rate is an exact match, use that value */
> + if (fract_div_phy->pixclk == rate)
> + goto use_fract_div;
Actually I don't really like the jump here and the even more extensive
jumping in the next patch.
Why not simply create another function and do "return use_fract_div()"
here and below at the end of phy_clk_set_rate().
>
> - phy->cur_cfg = &phy_pll_cfg[i];
> + /*
> + * If the rate from the fractional divder is not exact, check the integer divider,
> + * and use it if that value is an exact match.
> + */
> + int_div_clk = fsl_samsung_hdmi_phy_find_pms(rate, &p, &m, &s);
> + if (int_div_clk == rate) {
> + dev_dbg(phy->dev, "fsl_samsung_hdmi_phy: integer divider rate = %u\n",
> + int_div_clk);
> +
> + fsl_samsung_hdmi_calculate_phy(&calculated_phy_pll_cfg, int_div_clk, p, m, s);
> + phy->cur_cfg = &calculated_phy_pll_cfg;
> + return fsl_samsung_hdmi_phy_configure(phy, phy->cur_cfg);
> + }
>
> + /*
> + * If neither the fractional divder nor the integer divder can find an exact value
^ divider ^ divider
> + * fall back to using the fractional divider
> + */
> +use_fract_div:
> + phy->cur_cfg = fract_div_phy;
> + dev_dbg(phy->dev, "fsl_samsung_hdmi_phy: using fractional divider rate = %u\n",
> + phy->cur_cfg->pixclk);
> return fsl_samsung_hdmi_phy_configure(phy, phy->cur_cfg);
> }
>
Powered by blists - more mailing lists