On 11/13/21 12:34 AM, Russell King (Oracle) wrote:
On Fri, Nov 12, 2021 at 04:39:04PM -0800, Randy Dunlap wrote:
On 11/12/21 9:29 AM, Jonathan Cameron wrote:
On Tue, 9 Nov 2021 18:37:55 -0800
Randy Dunlap <rdunlap@xxxxxxxxxxxxx> wrote:
MIPS does not always provide clk*() interfaces and there are no
always-present stubs for them, so depending on "MIPS || COMPILE_TEST"
is not strong enough to prevent build errors.
Likewise MACH_INGENIC_SOC || COMPILE_TEST is not strong enough
since if only COMPILE_TEST=y (with some other MIPS MACH_ or CPU or
BOARD setting), there are still the same build errors.
It looks like depending on MACH_INGENIC is the only thing that is
sufficient here in order to prevent build errors.
mips-linux-ld: drivers/iio/adc/ingenic-adc.o: in function `jz4770_adc_init_clk_div':
ingenic-adc.c:(.text+0xe4): undefined reference to `clk_get_parent'
mips-linux-ld: drivers/iio/adc/ingenic-adc.o: in function `jz4725b_adc_init_clk_div':
ingenic-adc.c:(.text+0x1b8): undefined reference to `clk_get_parent'
Fixes: 1a78daea107d ("IIO: add Ingenic JZ47xx ADC driver.")
Signed-off-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>
Reported-by: kernel test robot <lkp@xxxxxxxxx>
Cc: Artur Rojek <contact@xxxxxxxxxxxxxx>
Cc: Paul Cercueil <paul@xxxxxxxxxxxxxxx>
Cc: linux-mips@xxxxxxxxxxxxxxx
Cc: Jonathan Cameron <jic23@xxxxxxxxxx>
Cc: Lars-Peter Clausen <lars@xxxxxxxxxx>
Cc: linux-iio@xxxxxxxxxxxxxxx
Cc: Florian Fainelli <f.fainelli@xxxxxxxxx>
Cc: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>
I'm a bit confused. There are stubs in include/linux/clk.h for these.
Why do those not apply here? Are these platforms built with CONFIG_CLK but
don't provide all the functions?
That sounds highly error prone and rather defeats the object of the
stubs. Could we either provide the missing stubs, or solve this some other
way. I'm not keen to massively cut the build coverage this driver is getting
by dropping COMPILE_TEST if there is any route to avoid doing so.
I'm all for that (above), but it's a mess.
Based on the guess than any platform with clks must be able to turn them on
I grepped for int clk_enable() and there seem to be only two possiblities
bcm63xx and lantiq as sources of the build breakage.
CONFIG_BCM63XX=y
# CONFIG_MACH_INGENIC_SOC is not set
CONFIG_INGENIC_ADC=y
CONFIG_HAVE_CLK=y
According to the build error messages (above), clk_get_parent()
is missing. Looking at <linux/clk.h>, for CONFIG_HAVE_CLK=y,
there is a prototype for clk_get_parent(), and if CONFIG_HAVE_CLK
is not set, there is a stub for it.
Now look at drivers/clk/clk.c and drivers/clk/Makefile:
clk_get_parent() is defined in clk.c, which is built when
CONFIG_COMMON_CLK=y, but that is not set in this .config file.
CONFIG_HAVE_CLK=y, but that doesn't get clk_get_parent()
compiled.
So to me it is a disparity or incongruity between HAVE_CLK and COMMON_CLK.
HAVE_CLK means we have the clk API implemented. COMMON_CLK is one such
implementation, and HAVE_LEGACY_CLK is another group of implementations.
BCM63XX has its own implementation and uses HAVE_LEGACY_CLK, which can
be found in arch/mips/bcm63xx/clk.c.
If it doesn't support parent clocks, then it should provide a stub
clk_get_parent() that returns NULL at the very least.
Russell- thanks for the explanation.
That works nicely.
Jonathan, I'll send a different patch.
--
~Randy