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