On Sat, Jan 08, 2022 at 01:26:08PM +0200, Andy Shevchenko wrote: > On Sat, Jan 8, 2022 at 3:57 AM Gabriel L. Somlo <gsomlo@xxxxxxxxx> wrote: > > On Sat, Jan 08, 2022 at 01:54:07AM +0200, Andy Shevchenko wrote: > > > On Saturday, January 8, 2022, Gabriel Somlo <gsomlo@xxxxxxxxx> wrote: > > > > + u32 div; > > > + > > > + div = freq ? host->ref_clk / freq : 256U; > > > > > > + div = roundup_pow_of_two(div); > > > + div = clamp(div, 2U, 256U); > > > > > > Logically seems to me that you may join these two together, because clamped > > > range is power-of-2 one. > > > > `div` needs to be a power-of-2 when written to the LITEX_PHY_CLOCKERDIV > > register (below). And clamp() will just enforce a min/max range, so if > > (div = ref_clk / freq) ends up e.g., 5, I need both roundup_pow_of_two() > > to bump it to 8, and clamp() to enforce that it's between 2 and 256. > > > > Unless you mean I should simply write it like: > > > > div = clamp(roundup_pow_of_two(div), 2U, 256U); > > > > ... as a single line? > > Yes, that's what I meant. Turns out, clamp really hates being passed roundup_pow_of_two() directly (see below). I think it's probably better if we leave them as-is, to avoid going the explicit cast route which Geert recommended against. I'll send out v9 later today with everything else, including devm_add_action_or_reset(). Thanks, --Gabriel drivers/mmc/host/litex_mmc.c: In function 'litex_mmc_setclk': ./include/linux/minmax.h:20:35: warning: comparison of distinct pointer types lacks a cast 20 | (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1))) | ^~ ./include/linux/minmax.h:26:18: note: in expansion of macro '__typecheck' 26 | (__typecheck(x, y) && __no_side_effects(x, y)) | ^~~~~~~~~~~ ./include/linux/minmax.h:36:31: note: in expansion of macro '__safe_cmp' 36 | __builtin_choose_expr(__safe_cmp(x, y), \ | ^~~~~~~~~~ ./include/linux/minmax.h:45:25: note: in expansion of macro '__careful_cmp' 45 | #define min(x, y) __careful_cmp(x, y, <) | ^~~~~~~~~~~~~ ./include/linux/minmax.h:89:28: note: in expansion of macro 'min' 89 | #define clamp(val, lo, hi) min((typeof(val))max(val, lo), hi) | ^~~ ./include/linux/minmax.h:26:18: note: in expansion of macro '__typecheck' 26 | (__typecheck(x, y) && __no_side_effects(x, y)) | ^~~~~~~~~~~ ./include/linux/minmax.h:36:31: note: in expansion of macro '__safe_cmp' 36 | __builtin_choose_expr(__safe_cmp(x, y), \ | ^~~~~~~~~~ ./include/linux/minmax.h:52:25: note: in expansion of macro '__careful_cmp' 52 | #define max(x, y) __careful_cmp(x, y, >) | ^~~~~~~~~~~~~ ./include/linux/minmax.h:89:45: note: in expansion of macro 'max' 89 | #define clamp(val, lo, hi) min((typeof(val))max(val, lo), hi) | ^~~ drivers/mmc/host/litex_mmc.c:448:15: note: in expansion of macro 'clamp' 448 | div = clamp(roundup_pow_of_two(div), 2U, 256U); | ^~~~~ ./include/linux/minmax.h:20:35: warning: comparison of distinct pointer types lacks a cast 20 | (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1))) | ^~ ./include/linux/minmax.h:26:18: note: in expansion of macro '__typecheck' 26 | (__typecheck(x, y) && __no_side_effects(x, y)) | ^~~~~~~~~~~ ./include/linux/minmax.h:36:31: note: in expansion of macro '__safe_cmp' 36 | __builtin_choose_expr(__safe_cmp(x, y), \ | ^~~~~~~~~~ ./include/linux/minmax.h:45:25: note: in expansion of macro '__careful_cmp' 45 | #define min(x, y) __careful_cmp(x, y, <) | ^~~~~~~~~~~~~ ./include/linux/minmax.h:89:28: note: in expansion of macro 'min' 89 | #define clamp(val, lo, hi) min((typeof(val))max(val, lo), hi) | ^~~ drivers/mmc/host/litex_mmc.c:448:15: note: in expansion of macro 'clamp' 448 | div = clamp(roundup_pow_of_two(div), 2U, 256U); | ^~~~~ In file included from ./include/linux/bits.h:5, from drivers/mmc/host/litex_mmc.c:12: ./include/linux/minmax.h:20:35: warning: comparison of distinct pointer types lacks a cast 20 | (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1))) | ^~ ./include/linux/const.h:12:55: note: in definition of macro '__is_constexpr' 12 | (sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8))) | ^ ./include/linux/minmax.h:26:39: note: in expansion of macro '__no_side_effects' 26 | (__typecheck(x, y) && __no_side_effects(x, y)) | ^~~~~~~~~~~~~~~~~ ./include/linux/minmax.h:36:31: note: in expansion of macro '__safe_cmp' 36 | __builtin_choose_expr(__safe_cmp(x, y), \ | ^~~~~~~~~~ ./include/linux/minmax.h:45:25: note: in expansion of macro '__careful_cmp' 45 | #define min(x, y) __careful_cmp(x, y, <) | ^~~~~~~~~~~~~ ./include/linux/minmax.h:89:28: note: in expansion of macro 'min' 89 | #define clamp(val, lo, hi) min((typeof(val))max(val, lo), hi) | ^~~ ./include/linux/minmax.h:26:18: note: in expansion of macro '__typecheck' 26 | (__typecheck(x, y) && __no_side_effects(x, y)) | ^~~~~~~~~~~ ./include/linux/minmax.h:36:31: note: in expansion of macro '__safe_cmp' 36 | __builtin_choose_expr(__safe_cmp(x, y), \ | ^~~~~~~~~~ ./include/linux/minmax.h:52:25: note: in expansion of macro '__careful_cmp' 52 | #define max(x, y) __careful_cmp(x, y, >) | ^~~~~~~~~~~~~ ./include/linux/minmax.h:89:45: note: in expansion of macro 'max' 89 | #define clamp(val, lo, hi) min((typeof(val))max(val, lo), hi) | ^~~ drivers/mmc/host/litex_mmc.c:448:15: note: in expansion of macro 'clamp' 448 | div = clamp(roundup_pow_of_two(div), 2U, 256U); | ^~~~~ In file included from ./include/linux/kernel.h:17, from ./include/linux/clk.h:13, from drivers/mmc/host/litex_mmc.c:13: ./include/linux/minmax.h:20:35: warning: comparison of distinct pointer types lacks a cast 20 | (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1))) | ^~ ./include/linux/minmax.h:28:27: note: in definition of macro '__cmp' 28 | #define __cmp(x, y, op) ((x) op (y) ? (x) : (y)) | ^ ./include/linux/minmax.h:45:25: note: in expansion of macro '__careful_cmp' 45 | #define min(x, y) __careful_cmp(x, y, <) | ^~~~~~~~~~~~~ ./include/linux/minmax.h:89:28: note: in expansion of macro 'min' 89 | #define clamp(val, lo, hi) min((typeof(val))max(val, lo), hi) | ^~~ ./include/linux/minmax.h:26:18: note: in expansion of macro '__typecheck' 26 | (__typecheck(x, y) && __no_side_effects(x, y)) | ^~~~~~~~~~~ ./include/linux/minmax.h:36:31: note: in expansion of macro '__safe_cmp' 36 | __builtin_choose_expr(__safe_cmp(x, y), \ | ^~~~~~~~~~ ./include/linux/minmax.h:52:25: note: in expansion of macro '__careful_cmp' 52 | #define max(x, y) __careful_cmp(x, y, >) | ^~~~~~~~~~~~~ ./include/linux/minmax.h:89:45: note: in expansion of macro 'max' 89 | #define clamp(val, lo, hi) min((typeof(val))max(val, lo), hi) | ^~~ drivers/mmc/host/litex_mmc.c:448:15: note: in expansion of macro 'clamp' 448 | div = clamp(roundup_pow_of_two(div), 2U, 256U); | ^~~~~ ./include/linux/minmax.h:20:35: warning: comparison of distinct pointer types lacks a cast 20 | (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1))) | ^~ ./include/linux/minmax.h:28:40: note: in definition of macro '__cmp' 28 | #define __cmp(x, y, op) ((x) op (y) ? (x) : (y)) | ^ ./include/linux/minmax.h:45:25: note: in expansion of macro '__careful_cmp' 45 | #define min(x, y) __careful_cmp(x, y, <) | ^~~~~~~~~~~~~ ./include/linux/minmax.h:89:28: note: in expansion of macro 'min' 89 | #define clamp(val, lo, hi) min((typeof(val))max(val, lo), hi) | ^~~ ./include/linux/minmax.h:26:18: note: in expansion of macro '__typecheck' 26 | (__typecheck(x, y) && __no_side_effects(x, y)) | ^~~~~~~~~~~ ./include/linux/minmax.h:36:31: note: in expansion of macro '__safe_cmp' 36 | __builtin_choose_expr(__safe_cmp(x, y), \ | ^~~~~~~~~~ ./include/linux/minmax.h:52:25: note: in expansion of macro '__careful_cmp' 52 | #define max(x, y) __careful_cmp(x, y, >) | ^~~~~~~~~~~~~ ./include/linux/minmax.h:89:45: note: in expansion of macro 'max' 89 | #define clamp(val, lo, hi) min((typeof(val))max(val, lo), hi) | ^~~ drivers/mmc/host/litex_mmc.c:448:15: note: in expansion of macro 'clamp' 448 | div = clamp(roundup_pow_of_two(div), 2U, 256U); | ^~~~~ ./include/linux/minmax.h:20:35: warning: comparison of distinct pointer types lacks a cast 20 | (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1))) | ^~ ./include/linux/minmax.h:31:24: note: in definition of macro '__cmp_once' 31 | typeof(x) unique_x = (x); \ | ^ ./include/linux/minmax.h:45:25: note: in expansion of macro '__careful_cmp' 45 | #define min(x, y) __careful_cmp(x, y, <) | ^~~~~~~~~~~~~ ./include/linux/minmax.h:89:28: note: in expansion of macro 'min' 89 | #define clamp(val, lo, hi) min((typeof(val))max(val, lo), hi) | ^~~ ./include/linux/minmax.h:26:18: note: in expansion of macro '__typecheck' 26 | (__typecheck(x, y) && __no_side_effects(x, y)) | ^~~~~~~~~~~ ./include/linux/minmax.h:36:31: note: in expansion of macro '__safe_cmp' 36 | __builtin_choose_expr(__safe_cmp(x, y), \ | ^~~~~~~~~~ ./include/linux/minmax.h:52:25: note: in expansion of macro '__careful_cmp' 52 | #define max(x, y) __careful_cmp(x, y, >) | ^~~~~~~~~~~~~ ./include/linux/minmax.h:89:45: note: in expansion of macro 'max' 89 | #define clamp(val, lo, hi) min((typeof(val))max(val, lo), hi) | ^~~ drivers/mmc/host/litex_mmc.c:448:15: note: in expansion of macro 'clamp' 448 | div = clamp(roundup_pow_of_two(div), 2U, 256U); | ^~~~~ ./include/linux/minmax.h:20:35: warning: comparison of distinct pointer types lacks a cast 20 | (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1))) | ^~ ./include/linux/minmax.h:31:39: note: in definition of macro '__cmp_once' 31 | typeof(x) unique_x = (x); \ | ^ ./include/linux/minmax.h:45:25: note: in expansion of macro '__careful_cmp' 45 | #define min(x, y) __careful_cmp(x, y, <) | ^~~~~~~~~~~~~ ./include/linux/minmax.h:89:28: note: in expansion of macro 'min' 89 | #define clamp(val, lo, hi) min((typeof(val))max(val, lo), hi) | ^~~ ./include/linux/minmax.h:26:18: note: in expansion of macro '__typecheck' 26 | (__typecheck(x, y) && __no_side_effects(x, y)) | ^~~~~~~~~~~ ./include/linux/minmax.h:36:31: note: in expansion of macro '__safe_cmp' 36 | __builtin_choose_expr(__safe_cmp(x, y), \ | ^~~~~~~~~~ ./include/linux/minmax.h:52:25: note: in expansion of macro '__careful_cmp' 52 | #define max(x, y) __careful_cmp(x, y, >) | ^~~~~~~~~~~~~ ./include/linux/minmax.h:89:45: note: in expansion of macro 'max' 89 | #define clamp(val, lo, hi) min((typeof(val))max(val, lo), hi) | ^~~ drivers/mmc/host/litex_mmc.c:448:15: note: in expansion of macro 'clamp' 448 | div = clamp(roundup_pow_of_two(div), 2U, 256U); | ^~~~~