Hello, tr...@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 <tr...@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.carpen...@linaro.org> > > > > Reviewed-by: Simon Glass <s...@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
