Hi Paul,
> Am 20.09.2022 um 15:33 schrieb Paul Cercueil <paul@xxxxxxxxxxxxxxx>:
>
>
>
> Le mar., sept. 20 2022 at 14:31:38 +0200, H. Nikolaus Schaller <hns@xxxxxxxxxxxxx> a écrit :
>> Hi Paul,
>>> Am 20.09.2022 um 11:09 schrieb Paul Cercueil <paul@xxxxxxxxxxxxxxx>:
>>> Hi Nikolaus,
>>> Le mar., sept. 20 2022 at 08:31:30 +0200, H. Nikolaus Schaller <hns@xxxxxxxxxxxxx> a écrit :
>>>> Hi Paul,
>>>> it seems as if there aren't many places left over where the MACH_JZ47* configs are still in use:
>>>> drivers/char/hw_ramdom/Kconfig
>>>> drivers/clk/ingenic/Kconfig
>>>> drivers/gpu/drm/ingenic/Kconfig
>>>> drivers/pinctrl/pinctrl-ingenic.c
>>>> Is it possible to get rid of them and just have CONFIG_MACH_INGENIC_GENERIC?
>>>> This might simplify my defconfig for multiple machines.
>>> CONFIG_MIPS_GENERIC_KERNEL=y
>> This breaks compilation for me, e.g.
>> arch/mips/mm/cache.c:203:6: error: 'cpu_has_tx39_cache' undeclared (first use in this function)
>
> v6.0-rc does not have 'cpu_has_tx39_cache' anywhere in that file, or in arch/mips/ for that matter. It was removed in v5.18.
>
> And a v5.17 kernel compiles fine here with these options enabled. So it's a problem on your side, I guess.
Ah, you were right.
I have a patch included which was provided by zhouyanjie@xxxxxxxxxxxxxx
("MIPS: mm: Add Ingenic XBurst SoCs specific cache driver.").
It is intended to improve caching and is part of jz4780 SMP support which we wanted to have.
AFAIR it was either not posted to lkml or rejected or superseded.
>
>>> CONFIG_BOARD_INGENIC=y
>> This config option does not exist (at least in v6.0-rc). Probably you refer to CONFIG_INGENIC_GENERIC_BOARD.
>
> No, I do not, and yes, it exists.
Ah, I grepped for CONFIG_BOARD_INGENIC but it exists only in one Kconfig as BOARD_INGENIC.
But what is then the difference to CONFIG_INGENIC_GENERIC_BOARD and CONFIG_MACH_INGENIC_GENERIC?
>
>> As far as I see, this does not choose to build any device tree blob.
>> I tried some patch to get the .dtb built, but the resulting kernel does not show any activity.
>> If I e.g. switch back from CONFIG_INGENIC_GENERIC_BOARD=y to CONFIG_JZ4780_CI20=y the kernel works.
>
> Because in the first case you build a generic kernel, which does not embed any .dtb, and you are responsible for providing the kernel with the blob at boot time; while if you build with CONFIG_JZ4780_CI20=y it embeds the .dtb inside the kernel.
>
> You can embed the .dtb into the generic kernel at compile-time too, have a look at "Kernel type -> Kernel appended dtb support." Not sure why you'd want that for a generic kernel, though.
Ah, I remember. Since I usually code 99% of my time for ARM systems with separate .dtb files chosen by the boot loader, I forgot that we have to append the .dtb on the CI20 and Alpha400. So there is no good solution for a "universal" kernel binary either.
>
>>> Then you can support all Ingenic-based boards alongside other MIPS boards.
>> Yes, I know, but why are the MACH_JZ47* not replaced by CONFIG_MACH_INGENIC_GENERIC if they are almost unused or completely removed?
>
> They *are* used.
Well, only in a handful of places as it looks like.
>
>> BTW: there are also seems to be some board specific CONFIGs in processor specific code (e.g. CONFIG_JZ4780_CI20 in irqchip code).
>
> rgrep CI20 drivers/irqchip/
>
> returns nothing for me.
Ah, again an inofficial patch which is part of the SMP stuff ("irqchip: Ingenic: Add percpu IRQ support for X2000.").
>
>> So selecting a MACH is not sufficient to get these features.
>> All this looks a little fragile and incomplete... Maybe if I find some time (which is unfortunately quite unlikely) I can propose some fixes.
>
> It is not "fragile and incomplete", it works as intended, and it's a feature I use often.
Yes, seems as if you are right. We may have added too many useful patches which did not go upstreamand get in conflict with upstream features.
BR and thanks for helping to better understand,
Nikolaus
>
> -Paul
>
