Messages in this thread | | | From | Hadar Gat <> | Subject | RE: [PATCH v2 0/3] hw_random: introduce Arm CryptoCell TRNG driver | Date | Wed, 5 Feb 2020 08:56:28 +0000 |
| |
> > On Sun, Feb 2, 2020 at 2:27 PM Hadar Gat <hadar.gat@arm.com> wrote: > > > > The Arm CryptoCell is a hardware security engine. > > This patch introduces driver for its TRNG (True Random Number > > Generator) engine. > > > > Changes from previous veriosn: fixed 'make dt_bnding_check' errors. > > > > Hadar Gat (3): > > dt-bindings: add device tree binding for Arm CryptoCell trng engine > > hw_random: cctrng: introduce Arm CryptoCell driver > > MAINTAINERS: add HG as cctrng maintainer > > I looked at the patches briefly and everything makes sense to me, > > Acked-by: Arnd Bergmann <arnd@arndb.de> > > There are two style issues that you could improve: > > - The header is only included from a single file, so I would just fold > its contents into the driver itself.
Although it is included from a single file, I prefer a separated file because it is already contains more than a few definitions and it is most likely that its content will grow in next versions of Arm CryptoCell HW.
> > - You have a lot of "#ifdef CONFIG_PM" that are easy to get wrong > with various combinations of config symbols. It's often better to > leave that all compiled unconditionally and have the logic in > UNIVERSAL_DEV_PM_OPS() take care of dropping the unused > bits, with a __maybe_unused annotation on functions that cause > a warning otherwise.
Thanks Arnd. I'll remove these #ifdef as you suggested.
> > Arnd
Hadar
| |