Thread (76 messages) 76 messages, 9 authors, 2021-09-21

Re: [PATCH 07/10] serial: mvebu-uart: implement UART clock driver for configuring UART base clock

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2021-06-25 13:35:42
Also in: oe-kbuild-all

Hi Pali,

On Fri, Jun 25, 2021 at 3:22 PM Pali Rohár [off-list ref] wrote:
On Friday 25 June 2021 14:44:36 Geert Uytterhoeven wrote:
quoted
On Fri, Jun 25, 2021 at 1:29 PM Pali Rohár [off-list ref] wrote:
quoted
On Friday 25 June 2021 13:05:32 Geert Uytterhoeven wrote:
quoted
On Fri, Jun 25, 2021 at 11:31 AM Pali Rohár [off-list ref] wrote:
quoted
On Friday 25 June 2021 16:15:38 kernel test robot wrote:
quoted
I love your patch! Yet something to improve:

[auto build test ERROR on robh/for-next]
[also build test ERROR on tty/tty-testing clk/clk-next linus/master v5.13-rc7 next-20210624]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Pali-Roh-r/serial-mvebu-uart-Fixes-and-new-support-for-higher-baudrates/20210625-065146
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: m68k-allmodconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/747483a6b8f2de98afe461dbf91227404a8e2e81
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Pali-Roh-r/serial-mvebu-uart-Fixes-and-new-support-for-higher-baudrates/20210625-065146
        git checkout 747483a6b8f2de98afe461dbf91227404a8e2e81
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=m68k

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <redacted>

All errors (new ones prefixed by >>):

   m68k-linux-ld: drivers/tty/serial/mvebu-uart.o: in function `mvebu_uart_clock_prepare':
   mvebu-uart.c:(.text+0x6d0): undefined reference to `__udivdi3'
quoted
quoted
m68k-linux-ld: mvebu-uart.c:(.text+0x78c): undefined reference to `__udivdi3'
Hello! Could you help me how to fix this issue? I'm using macro
DIV_ROUND_CLOSEST() with two u64 values in that function. And I really
do not know details about m68k arch and I never touched this arch.

There is missing number of line which caused this error. So if it is
possible I have suggestion for bot to compile kernel with -g flag. In
this case linker show exact line number (and not only hex address) which
caused that linker error. Also in future it could help identify source
of errors...
This error means your driver uses a 64-bit division without using the
proper methods from include/linux/math64.h.
Ok. So whenever I need 64-bit division should I always use macros from
this header file and also in case my driver is only for 64-bit platform?
Exactly. As SERIAL_MVEBU_UART depends on ARCH_MVEBU || COMPILE_TEST,
it can be test-compiled on other architectures.
quoted
I see that in this header file is DIV64_U64_ROUND_CLOSEST() macro which
seems to be 64-bit forced variant of DIV_ROUND_CLOSEST() which I'm
using.
Indeed.

Please also consider if you really need to do a 64-bit division, and if a
simpler and faster 64-by-32 division would be sufficient.
Good catch! I have checked this and numerator in the worst case does not
fit into 32 bits (so u64 is needed) but denominator in the worst case
fits into unsigned 32 bit integer.
Note that in some cases, the numerator is (unsigned) long, hence sometimes
32-bit, sometimes 64-bit, and there are separate helpers for that.
So what is the preferred way to do u64/u32 division with rounding to the
closest number which would work on all platforms?

In the end I need to clamp this result into range 1..1023.
Without rounding, it is div_u64().
With rounding, that would be DIV_U64_ROUND_CLOSEST(), which doesn't
exist yet. So you have to add it first, cfr. DIV_S64_ROUND_CLOSEST().

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help