Received-SPF: pass (google.com: domain of linux-kernel+bounces-202691-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Feedback-ID: ifd894703:Fastmail
User-Agent: Cyrus-JMAP/3.11.0-alpha0-497-g97f96844c-fm-20240526.001-g97f96844
Precedence: bulk
MIME-Version: 1.0
Message-Id: <310c2bce-e6cb-4268-858b-8466c92266dd@app.fastmail.com>
In-Reply-To: <878qztbvb4.ffs@tglx>
References: <20240517-loongson_nodecnt-v2-1-5bd0bb20ff5f@flygoat.com>
 <87sey3b6de.ffs@tglx> <9947f7a5-1a95-48f2-b0eb-0385eb2b3d55@app.fastmail.com>
 <558b1d58-08a5-410d-97b8-e77def7c1cf8@app.fastmail.com> <878qztbvb4.ffs@tglx>
Date: Wed, 05 Jun 2024 14:17:38 +0100
From: "Jiaxun Yang" <jiaxun.yang@flygoat.com>
To: "Thomas Gleixner" <tglx@linutronix.de>,
 "Huacai Chen" <chenhuacai@kernel.org>,
 "Thomas Bogendoerfer" <tsbogend@alpha.franken.de>,
 "Daniel Lezcano" <daniel.lezcano@linaro.org>
Cc: linux-kernel@vger.kernel.org,
 "linux-mips@vger.kernel.org" <linux-mips@vger.kernel.org>
Subject: Re: [PATCH v2] clocksource: Add node counter timer driver for MIPS/Loongson64
Content-Type: text/plain;charset=utf-8
Content-Transfer-Encoding: quoted-printable


=E5=9C=A82024=E5=B9=B45=E6=9C=8828=E6=97=A5=E4=BA=94=E6=9C=88 =E4=B8=8B=E5=
=8D=888:17=EF=BC=8CThomas Gleixner=E5=86=99=E9=81=93=EF=BC=9A
> On Tue, May 28 2024 at 15:31, Jiaxun Yang wrote:
>> =E5=9C=A82024=E5=B9=B45=E6=9C=8827=E6=97=A5=E4=BA=94=E6=9C=88 =E4=B8=8B=
=E5=8D=885:32=EF=BC=8CJiaxun Yang=E5=86=99=E9=81=93=EF=BC=9A
>>> =E5=9C=A82024=E5=B9=B45=E6=9C=8827=E6=97=A5=E4=BA=94=E6=9C=88 =E4=B8=
=8B=E5=8D=884:51=EF=BC=8CThomas Gleixner=E5=86=99=E9=81=93=EF=BC=9A
>>>> Jiaxun!
>>>>
>>> Hi Thomas,
>>> [...]
>>>>
>>>> What's this indirection for? Why dont you update=20
>>> [...]
>>>>
>>>>> +static struct clocksource nodecnt_clocksource =3D {
>>>>> +	.name	=3D "nodecnt",
>>>>> +	.read	=3D nodecnt_clocksource_read,
>>>>
>>>> the read function pointer here and spare the indirection?
>>> Smart! sched_clock takes slightly different function pointer argumen=
t type
>>> but as we don't use the argument anyway, it should be safe to relax =
this
>>> indirection.
>>
>> Sadly, I'm unable to remove it with force type catsing :-/
>>
>> drivers/clocksource/loongson-nodecnt.c: In function =E2=80=98nodecnt_=
clocksource_init=E2=80=99:
>> drivers/clocksource/loongson-nodecnt.c:89:36: warning: cast between i=
ncompatible function types from =E2=80=98u64 (*)(void)=E2=80=99 {aka =E2=
=80=98long long unsigned int (*)(void)=E2=80=99} to =E2=80=98u64 (*)(str=
uct clocksource *)=E2=80=99 {aka =E2=80=98long long unsigned int (*)(str=
uct clocksource *)=E2=80=99} [-Wcast-function-type]
>>    89 |         nodecnt_clocksource.read =3D (u64 (*)(struct clocksou=
rce *))nodecnt_read_fn;
>>
>
> What about making the actual read functions have the required function
> signature? The clocksource argument is not used in those real function=
s.

Sadly sched_clock requires another signature :-/

I can see timer-clint hit into the same pitfall.

Thanks
>
> Thanks,
>
>         tglx

--=20
- Jiaxun