linux-kernel - Re: [RFC V3 3/3] phy: freescale: fsl-samsung-hdmi: Support dynamic integer

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAHCN7xLVSq9gh9KbKbFBcZ7HT5vgv4JHdn3QTXFf5B9t86Wp_g@mail.gmail.com>
Date: Mon, 2 Sep 2024 20:20:50 -0500
From: Adam Ford <aford173@...il.com>
To: Dominique Martinet <dominique.martinet@...ark-techno.com>
Cc: linux-phy@...ts.infradead.org, linux-imx@....com, festevam@...il.com, 
	frieder.schrempf@...tron.de, aford@...conembedded.com, 
	Vinod Koul <vkoul@...nel.org>, Kishon Vijay Abraham I <kishon@...nel.org>, 
	Marco Felsch <m.felsch@...gutronix.de>, 
	Uwe Kleine-König <u.kleine-koenig@...gutronix.de>, 
	Lucas Stach <l.stach@...gutronix.de>, linux-kernel@...r.kernel.org, 
	Makoto Sato <makoto.sato@...ark-techno.com>
Subject: Re: [RFC V3 3/3] phy: freescale: fsl-samsung-hdmi: Support dynamic integer

On Mon, Sep 2, 2024 at 7:47 PM Dominique Martinet
<dominique.martinet@...ark-techno.com> wrote:
>
> Adam Ford wrote on Mon, Sep 02, 2024 at 04:20:11PM -0500:
> > > -       const struct phy_config *cur_cfg;
> > > +       struct phy_config cur_cfg;
> >
> > Wouldn't converting this from a pointer require me to do a memcpy
> > later?  It seems like that's more work than just pointing it to an
> > address.
> >
> > > -       phy->cur_cfg = &phy_pll_cfg[i];
> > > +       phy->cur_cfg = phy_pll_cfg[i];
> >
> > I think this is would have to be a memcpy instead of just an equal
> > statement since phy->cur_cfg  would no longer be a pointer.
>
> C allows copying structs like this, it's fine to write it as just an
> equal.
> It's not 100% equivalent, iiuc simple assignment is undefined behaviour
> if the elements aren't aligned but memcpy will work even in that case,
> but for us this is not a proble mand the generated code should be
> identical... Also note I'm only suggesting that because the struct is
> tiny (1*u32+7*u8 is less than two u64), but this code isn't meant to run
> very often anyway so we should prioritize readability -- if you think
> it's harder to understand than an extra pointer somewhere I have no
> strong opinion; as said in the previous mail if parallel uses are
> possible it'd be better kept on the stack anyway...

Dominique,

Unless someone pushes back, I think keeping it a pointer is more
consistent with what we did before, and it's more in my style of
coding.

I have two more patches in the series.
*  One updates the round and set functions to compare the closest
values between the fractional divider and the integer divider and uses
the option with the smallest difference between the achievable value
and the desired rate if the actual value is not achievable.

* The second removes the integer divider values from the LUT since
having it there just takes up extra space and makes the LUT search
longer.

I'll send it as V4 with a cover letter describing everything I have
done and the rationale behind it without the RFC designation.

For what it's worth, adding the integer calculator added 4 entries to
my modetest list, and changing the hdmi-tx to support a clock rante of
+/- 5% added another 11 possible entries.  I have not tested them all
yet.
I likely won't do a patch for the HDMI-TX, but if you want to send a
patch increasing the tolerance, I'll likely test it as I have time.

adam

>
> --
> Dominique
>
>