lkml.org 
[lkml]   [2023]   [Jan]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] watchdog: ixp4xx: Use devm_clk_get_enabled() helper
From
Le 01/01/2023 à 16:07, Guenter Roeck a écrit :
> On Sun, Jan 01, 2023 at 10:35:59AM +0100, Christophe JAILLET wrote:
>> Le 01/01/2023 à 00:14, Guenter Roeck a écrit :
>>> On Sat, Dec 31, 2022 at 12:07:27PM +0100, Christophe JAILLET wrote:
>>>> The devm_clk_get_enabled() helper:
>>>> - calls devm_clk_get()
>>>> - calls clk_prepare_enable() and registers what is needed in order to
>>>> call clk_disable_unprepare() when needed, as a managed resource.
>>>>
>>>> This simplifies the code and avoids the need of a dedicated function used
>>>> with devm_add_action_or_reset().
>>>>
>>>> Signed-off-by: Christophe JAILLET <christophe.jaillet-39ZsbGIQGT5GWvitb5QawA@public.gmane.org>
>>>> ---
>>>> Note that I get a compilation error because read_cpuid_id() is not defined
>>>> on my system (x86_64).
>>>> So I think that a "depends on ARM<something>" in missing in a KConfig file.
>>>
>>> It has
>>>
>>> depends on ARCH_IXP4XX
>>>
>>> and CONFIG_IXP4XX_WATCHDOG is not set for me after "make allmodconfig".
>>
>> Here is what do.
>>
>> make allmodconfig
>> make -j8 drivers/watchdog/ixp4xx_wdt.o
>>
>> And I get:
>> DESCEND objtool
>> CALL scripts/checksyscalls.sh
>> CC drivers/watchdog/ixp4xx_wdt.o
>> drivers/watchdog/ixp4xx_wdt.c: In function ‘ixp4xx_wdt_probe’:
>> drivers/watchdog/ixp4xx_wdt.c:122:15: error: implicit declaration of
>> function ‘read_cpuid_id’ [-Werror=implicit-function-declaration]
>> 122 | if (!(read_cpuid_id() & 0xf) && !cpu_is_ixp46x()) {
>> | ^~~~~~~~~~~~~
>> cc1: all warnings being treated as errors
>> make[3]: *** [scripts/Makefile.build:252 : drivers/watchdog/ixp4xx_wdt.o]
>> Erreur 1
>> make[2]: *** [scripts/Makefile.build:504 : drivers/watchdog] Erreur 2
>> make[1]: *** [scripts/Makefile.build:504 : drivers] Erreur 2
>> make: *** [Makefile:2011 : .] Erreur 2
>>
>>
>> I do agree with you that:
>>
>> - Kconfig looks fine
>> config IXP4XX_WATCHDOG
>> tristate "IXP4xx Watchdog"
>> depends on ARCH_IXP4XX
>>
>> - Makefile looks fine
>> obj-$(CONFIG_IXP4XX_WATCHDOG) += ixp4xx_wdt.o
>>
>> - .config looks fine
>> IXP4XX_WATCHDOG is NOT defined
>>
>> - make drivers/watchdog/ looks fine
>> No error and ixp4xx_wdt.o is NOT built.
>>
>>
>> However, in the past (if I recollect correctly :) ), a "make <something_that
>> depends_on_a_config_variable_that_is_not_defined>" returned an error stating
>> that no rule existed to build the specified target.
>>
>
> This is not correct. It only applies if the target directory Makefile is
> excluded by the make flags, or possibly if the target file is a complex
> one build from various source files.
>
>> I sometimes needed to tweak the Kconfig files to force some compilation when
>> I didn't have the right tool chain or configuration.
>> It was maybe not the best practice, but it worked most of the time.
>>
>>
>> Now, with the example above, such a compilation attempt is possible. It is
>> maybe normal (because of a change somewhere in the way the kernel is built,
>> because of an updated toolchain on my machine, ...)
>> This is just fine for me, but looked really surprising.
>>
>> That is why I first thought that something was missing in a Kconfig file.
>>
>>
>> So my comments are just a surprise to me to something that seems not to
>> behave the same as before.
>>
> I don't think anything changed. It always worked like that for me.
> I would suggest to go back to an older kernel and try it there.
> You'll see exactly the same error. Maybe you just never encountered
> a file like that.

git reset --hard next-20210111 (randomly chosen date)
make allmodconfig
make clean
make -j7 drivers/watchdog/ixp4xx_wdt.o (too build most of the needed
stuff to build a kernel)
touch drivers/watchdog/ixp4xx_wdt.c
make -j7 drivers/watchdog/ixp4xx_wdt.o (too build this file only)

DESCEND objtool
CALL scripts/atomic/check-atomics.sh
CALL scripts/checksyscalls.sh
make[3]: *** Aucune règle pour fabriquer la cible «
drivers/watchdog/ixp4xx_wdt.o ». Arrêt.
make[2]: *** [scripts/Makefile.build:471 : __build] Erreur 2
make[1]: *** [scripts/Makefile.build:496 : drivers/watchdog] Erreur 2
make: *** [Makefile:1805 : drivers] Erreur 2


This is what I had in mind.
(Aucune règle pour fabriquer la cible... = no rule to build...)


So something changed somewhere. Maybe in the way Makefile are now
included or not in the build process, as you suggest above.

>
> So, in other words, what you should have said is "not compile tested".
> Alternatively, you could install cross compilers and compile test the
> patches with those.
>

No. The patches HAVE been cross compiled after my initial attempt with
my default x86_64 failing built.


This one was successfully built using:
make ARCH=arm CROSS_COMPILE=/usr/bin/arm-linux-gnueabi- allmodconfig
make ARCH=arm CROSS_COMPILE=/usr/bin/arm-linux-gnueabi- -j7
drivers/watchdog/ixp4xx_wdt.o


The other one was successfully built using:
make ARCH=mips CROSS_COMPILE=mips64-linux-gnuabi64- allmodconfig
Changing CONFIG_MACH_LOONGSON32 to y, instead of
CONFIG_MIPS_GENERIC_KERNEL
make ARCH=mips CROSS_COMPILE=mips64-linux-gnuabi64- -j7
drivers/watchdog/loongson1_wdt.o

I've long been reluctant to use cross-compiler because of low disk space
on my system. But I've changed my mind recently and now I do cross
compile. (see [1] if needed as example also with ARCH=arm)


My comments below the --- in the patches should not be taken as "I've
not managed to build with the patch", but "I've been surprised to get an
issue with x86_64, then cross-compiled with the relevant toolchain.
Then, I reported what looked like a potential issue when building with
x86_64."

CJ

[1]:
https://lore.kernel.org/all/dc5a2157-19c8-4498-6288-d7513ad3dde2@wanadoo.fr/

>
> Thanks,
> Guenter
>

\
 
 \ /
  Last update: 2023-03-26 23:24    [W:0.059 / U:0.212 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site