Hello,
trini@konsulko.com wrote on Tue, 19 Sep 2023 11:14:56 -0400:
On Mon, Sep 18, 2023 at 07:04:41PM -0600, Simon Glass wrote:
Hi Tom, Dan,
On Tue, 8 Aug 2023 at 19:39, Tom Rini trini@konsulko.com wrote:
On Wed, Jul 26, 2023 at 09:54:08AM +0300, Dan Carpenter wrote:
The VNBYTES() macro needs to have parentheses to prevent some (harmless) macro expansion bugs. The VNBYTES() macro is used like this:
VID_TO_PIXEL(x) * VNBYTES(vid_priv->bpix)The * operation is done before the / operation. It still ends up with the same results, but it's not ideal.
Signed-off-by: Dan Carpenter dan.carpenter@linaro.org Reviewed-by: Simon Glass sjg@chromium.org
In that this seems correct:
Applied to u-boot/next, thanks!
But I want to note that with gcc-13.1 (and binutils 2.40) or more specifically the kernel.org arm/aarch64/etc toolchains, this causes the generated code to change: aarch64: (for 1/1 boards) all +8.0 text +8.0 bananapi-m5 : all +8 text +8 u-boot: add: 0/0, grow: 1/0 bytes: 8/0 (8) function old new delta video_post_probe 248 256 +8
So I don't know that this was a harmless bug.
Hmm I should have read the comment directly above...
"Note we omit the outer brackets to allow multiplication by fractional pixels."
Having said that, it doesn't seem to matter getting (if indeed we do) a less accurate result.
But we should fix the comment then, or decide no, this is intentional.
One year has passed and I just stumbled upon this incoherency. It would be nice to clarify the comment or otherwise to revert Dan's patch if judged irrelevant in the end.
Thanks! Miquèl