lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 2 Apr 2024 08:48:44 +0100
From: Cristian Marussi <cristian.marussi@....com>
To: Peng Fan <peng.fan@....com>
Cc: Andy Shevchenko <andy.shevchenko@...il.com>,
	"Peng Fan (OSS)" <peng.fan@....nxp.com>,
	Sudeep Holla <sudeep.holla@....com>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Linus Walleij <linus.walleij@...aro.org>,
	Dan Carpenter <dan.carpenter@...aro.org>,
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	Oleksii Moisieiev <oleksii_moisieiev@...m.com>
Subject: Re: [PATCH v6 3/4] firmware: arm_scmi: Add SCMI v3.2 pincontrol
 protocol basic support

On Sun, Mar 31, 2024 at 01:44:28PM +0000, Peng Fan wrote:
> Hi Andy,
> 

Hi Peng,


> > Subject: Re: [PATCH v6 3/4] firmware: arm_scmi: Add SCMI v3.2 pincontrol
> > protocol basic support
> > 
> > Sat, Mar 23, 2024 at 08:15:16PM +0800, Peng Fan (OSS) kirjoitti:
> > > From: Peng Fan <peng.fan@....com>
> > >
> > > Add basic implementation of the SCMI v3.2 pincontrol protocol.
> > 
> > ...
> > 
> > >  scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += virtio.o
> > >  scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += optee.o
> > > scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o
> > > system.o voltage.o powercap.o
> > 
> > Actually you want to have := here.
> > 
> > > +scmi-protocols-y += pinctrl.o
> > 
> > 
> > 
> > >  scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y)
> > > $(scmi-transport-y)
> > 
> > Side note: The -objs has to be -y
> > 
> > ...
> > 
> > > +#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.
> 

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)

I'll made further remarks on v7 that you just posted.

Thanks,
Cristian


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ