Re: staging: vc04_services: Need suggestions on trying to fix sparse warning in vchiq_arm.c

Hi Ojaswin,

Am 05.06.21 um 09:23 schrieb Ojaswin Mujoo:

On Fri, Jun 04, 2021 at 08:13:06AM +0200, Stefan Wahren wrote:

quoted

Hi Ojaswin,

Am 02.06.21 um 16:50 schrieb Ojaswin Mujoo:

quoted

On Tue, Jun 01, 2021 at 11:23:07PM +0300, Dan Carpenter wrote:

quoted

The problem is not the Sparse warning, the problem is that this code is
a mess.  It used to very clearly buggy and I reported the bug.  I think
Arnd found the bug again independently and fixed it.

A couple weeks ago Al Viro looked at this code.  Here is his write up:

https://www.spinics.net/lists/kernel/msg3952745.html

It shouldn't take Al Viro dozens of pages of detailed analysis to try
figure out if the code is safe or not.  Your idea silences the warning
but would make the code even more subtle and complicated.

The right thing to do is to re-write the code to be simpler.

regards,
dan carpenter

Thank you for the prompt reply and the link, it was very insightful. You
are right, I was definitely going about this the wrong way and missing
the larger picture. I'll spend some time trying to understand this
codebase as I think that'd be a good start to understand how stuff works in
the kernel (even though some of the things in this driver are anti patterns)
and hopefully get some ideas on ways to clean this up.

Anyways, thanks again for the help, cheers!

thanks for your interest in cleaning this up. Yes, it's not clear which
points on the TODO list are the lower hanging fruits. In case you don't
want to fix checkpatch issues, maybe you can look at points 8, 9, 10, 12
and 13. Most of them require testing with a Raspberry Pi, but feel free
to ask if you have problems with it.

Regards
Stefan

Got it, Task 10 (cdev to its own file) seems like a pretty good task to
get started with. I'm planning to buy a Rpi 4 so I think I can run tests
on that. 

okay, but the AFAIK the vchiq driver in the mainline kernel doesn't work
with Rpi 4 yet. The Raspberry Pi 3 B Plus is currently the recommend
devel platform, so you can test 32 and 64 bit kernel.

Best regards

Thank you so much for the help, I'll get back incase I face any issues down 
the line.

Regards,
Ojaswin

quoted

quoted

Ojaswin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel