Re: [PATCH v6 1/3] reset: mediatek: Move MediaTek system clock reset to reset/mediatek

["Nícolas F. R. A. Prado" <nfraprado@collabora.com>]

On Mon, Oct 31, 2022 at 05:14:40AM +0000, Rex-BC Chen (陳柏辰) wrote:
> On Tue, 2022-10-25 at 12:36 +0200, AngeloGioacchino Del Regno wrote:
> > Il 21/10/22 12:48, Bo-Chen Chen ha scritto:
[..]
> > > --- a/drivers/clk/mediatek/Kconfig
> > > +++ b/drivers/clk/mediatek/Kconfig
> > > @@ -8,6 +8,7 @@ menu "Clock driver for MediaTek SoC"
> > >   config COMMON_CLK_MEDIATEK
> > >   	tristate
> > >   	select RESET_CONTROLLER
> > > +	select RESET_MEDIATEK_SYSCLK
> > >   	help
> > >   	  MediaTek SoCs' clock support.
> > >   
> > 
> > ..snip..
> > 
> > > diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> > > index 3e7e5fd633a8..5cef7ccc9a7d 100644
> > > --- a/drivers/reset/Makefile
> > > +++ b/drivers/reset/Makefile
> > > @@ -1,6 +1,7 @@
> > >   # SPDX-License-Identifier: GPL-2.0
> > >   obj-y += core.o
> > >   obj-y += hisilicon/
> > > +obj-y += mediatek/
> > 
> > I'd be more for
> > 
> > obj-$(CONFIG_ARCH_MEDIATEK) += mediatek/
> > 
> > as there's no reason to even compile these if MTK support isn't
> > enabled at all.
> > 
> 
> Hello Angelo,
> 
> thanks for your review.
> I obj-y += mediatek/ because if I don't write like this, it will build
> fail for x86.
> Is there any suggestion for this?
> 
> /tmp/src_kernel/prebuilt/toolchain/0day/gcc/x86_64-linux/bin/x86_64-
> linux-ld: vmlinux.o: in function `mtk_pericfg_init':
> clk-mt8135.c:(.init.text+0x12a2b7): undefined reference to
> `mtk_reset_init_with_node'
> /tmp/src_kernel/prebuilt/toolchain/0day/gcc/x86_64-linux/bin/x86_64-
> linux-ld: vmlinux.o: in function `mtk_infrasys_init':
> clk-mt8135.c:(.init.text+0x12a3bb): undefined reference to
> `mtk_reset_init_with_node'
> /tmp/src_kernel/prebuilt/toolchain/0day/gcc/x86_64-linux/bin/x86_64-
> linux-ld: vmlinux.o: in function `mtk_infrasys_init':
> clk-mt8173.c:(.init.text+0x12ac47): undefined reference to
> `mtk_reset_init_with_node'
> /tmp/src_kernel/prebuilt/toolchain/0day/gcc/x86_64-linux/bin/x86_64-
> linux-ld: vmlinux.o: in function `mtk_pericfg_init':
> clk-mt8173.c:(.init.text+0x12ad25): undefined reference to
> `mtk_reset_init_with_node'
> /tmp/src_kernel/kernel/mediatek/scripts/Makefile.vmlinux:34: recipe for
> target 'vmlinux' failed
> make[3]: *** [vmlinux] Error 1
> make[3]: Target '__default' not remade because of errors.
> /tmp/src_kernel/kernel/mediatek/Makefile:1236: recipe for target
> 'vmlinux' failed
> make[2]: *** [vmlinux] Error 2
> make[2]: Target '__all' not remade because of errors.
> make[2]: Leaving directory '/tmp/out_kernel/out/allyesconfig.x86_64'
> Makefile:231: recipe for target '__sub-make' failed
> make[1]: *** [__sub-make] Error 2
> make[1]: Target '__all' not remade because of errors.
> make[1]: Leaving directory '/tmp/src_kernel/kernel/mediatek'
> build/core/kbuild_test.mk:61: recipe for target 'all' failed
> make: *** [all] Error 2
> [11:44:04] Error: failed to build allyesconfig.x86_64

From the errors it seems like you're compiling the clock drivers (like
clk-mt8135.c) but not the sysclk reset driver (reset-mediatek-sysclk.c). Given
that this happened when you added CONFIG_ARCH_MEDIATEK to that Makefile, then it
must mean that that config is disabled, which would make sense for a x86 build.
But then the fact that the clock driver was built even in this case must mean
that you have COMPILE_TEST enabled.

I guess in order to have a working COMPILE_TEST=y build, you do need to have
obj-y += mediatek/, and on the Kconfig below...

> 
> > >   obj-$(CONFIG_ARCH_STI) += sti/
> > >   obj-$(CONFIG_ARCH_TEGRA) += tegra/
> > >   obj-$(CONFIG_RESET_A10SR) += reset-a10sr.o
> > > diff --git a/drivers/reset/mediatek/Kconfig
> > > b/drivers/reset/mediatek/Kconfig
> > > new file mode 100644
> > > index 000000000000..a416cb938753
> > > --- /dev/null
> > > +++ b/drivers/reset/mediatek/Kconfig
> > > @@ -0,0 +1,5 @@
> > > +# SPDX-License-Identifier: GPL-2.0-only
> > 
> > Similarly, we should at this point also do....
> > 
> > if ARCH_MEDIATEK

... have instead

if ARCH_MEDIATEK || COMPILE_TEST

I think that should fix the issue.

Thanks,
Nícolas

> > 
> > > +config RESET_MEDIATEK_SYSCLK
> > > +	tristate "MediaTek System Clock Reset Driver"
> > > +	help
> > > +	  This enables the system clock reset driver for MediaTek SoCs.
> > 
> > endif # ARCH_MEDIATEK