Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 27 Jul 2021 01:30:04 +0100
From:   Andre Przywara <andre.przywara@arm.com>
To:     Mark Brown <broonie@kernel.org>
Cc:     Matt Mackall <mpm@selenic.com>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        Mark Rutland <mark.rutland@arm.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
        Sudeep Holla <sudeep.holla@arm.com>,
        linux-crypto@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org,
        Benjamin Herrenschmidt <benh@kernel.crashing.org>,
        Ard Biesheuvel <ardb@kernel.org>,
        Will Deacon <will@kernel.org>, Ali Saidi <alisaidi@amazon.com>,
        Jon Nettleton <jon@solid-run.com>
Subject: Re: [PATCH v3 2/2] hwrng: Add Arm SMCCC TRNG based driver
Message-ID: <20210727013004.4caca28f@slackpad.fritz.box>
In-Reply-To: <20210726223738.GM4670@sirena.org.uk>
References: <20210726175610.3311-1-andre.przywara@arm.com>
        <20210726175610.3311-3-andre.przywara@arm.com>
        <20210726223738.GM4670@sirena.org.uk>
Organization: Arm Ltd.
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Mon, 26 Jul 2021 23:37:38 +0100
Mark Brown <broonie@kernel.org> wrote:

> On Mon, Jul 26, 2021 at 06:56:10PM +0100, Andre Przywara wrote:
> 
> > +static int smccc_trng_init(struct hwrng *rng)
> > +{
> > +	return 0;
> > +}  
> 
> If this can be empty (looking at the core it seems like it can) then
> best just remove it.

Ah, you are right! Actually this is explicitly mentioned in the
struct hwrng comments. Thanks for the heads up.

> 
> > +	platform_set_drvdata(pdev, trng);
> > +	ret = devm_hwrng_register(&pdev->dev, trng);
> > +	if (!ret)
> > +		dev_info(&pdev->dev,
> > +			 "ARM SMCCC TRNG firmware random number generator\n");  
> 
> Is the log message needed given that we're not announcing any version
> information here or anything?  A brief sampling of other drivers
> suggests it's not a standard thing for the subsystem.

Yeah, that was indeed more a leftover of the version print. I
thought about querying the version again explicitly, but this would
have brought back the SMCCC calls that I could so nicely delete. Plus,
the hwrng driver is just a (secondary) user of this interface, I think
announcing the version should be done in smccc.c. Which is probably
beyond the scope of this patch.

Now thinking about this, there would probably be some value in making
the TRNG UUID somehow available, as this can be used to identify flawed
implementations (general problems in the hardware or backend bugs). But
this should be some query-able interface, rather than some line in
dmesg. Any ideas? Might be beyond the scope of this series, though...

Cheers,
Andre