On Tue, Apr 2, 2024 at 10:48 AM Cristian Marussi <cristian.marussi@xxxxxxx> wrote: > On Sun, Mar 31, 2024 at 01:44:28PM +0000, Peng Fan wrote: > > > Sat, Mar 23, 2024 at 08:15:16PM +0800, Peng Fan (OSS) kirjoitti: ... > > > > +#include <linux/module.h> > > > > +#include <linux/scmi_protocol.h> > > > > +#include <linux/slab.h> > > > > > > This is semi-random list of headers. Please, follow IWYU principle (include > > > what you use). There are a lot of inclusions I see missing (just in the context of > > > this page I see bits.h, types.h, and asm/byteorder.h). > > > > Is there any documentation about this requirement? > > Some headers are already included by others. The documentation here is called "a common sense". The C language is built like this and we expect that nobody will invest into the dependency hell that we have already, that's why IWYU principle, please follow it. > Andy made (mostly) the same remarks on this same patch ~1-year ago on > this same patch while it was posted by Oleksii. > > And I told that time that most of the remarks around devm_ usage were > wrong due to how the SCMI core handles protocol initialization (using a > devres group transparently). > > This is what I answered that time. > > https://lore.kernel.org/linux-arm-kernel/ZJ78hBcjAhiU+ZBO@e120937-lin/#t > > I wont repeat myself, but, in a nutshell the memory allocation like it > is now is fine: a bit happens via devm_ at protocol initialization, the > other is doe via explicit kmalloc at runtime and freed via kfree at > remove time (if needed...i.e. checking the present flag of some structs) This sounds like a mess. devm_ is expected to be used only for the ->probe() stage, otherwise you may consider cleanup.h (__free() macro) to have automatic free at the paths where memory is not needed. And the function naming doesn't suggest that you have a probe-remove pair. Moreover, if the init-deinit part is called in the probe-remove, the devm_ must not be mixed with non-devm ones, as it breaks the order and leads to subtle mistakes. > I'll made further remarks on v7 that you just posted. -- With Best Regards, Andy Shevchenko