All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rob Herring <robh@kernel.org>
To: Matthias Brugger <matthias.bgg@gmail.com>
Cc: AngeloGioacchino Del Regno 
	<angelogioacchino.delregno@collabora.com>,
	Daniel Golle <daniel@makrotopia.org>,
	Sean Wang <sean.wang@mediatek.com>,
	Olivia Mackall <olivia@selenic.com>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	Conor Dooley <conor.dooley@microchip.com>,
	Mingming Su <Mingming.Su@mediatek.com>,
	linux-crypto@vger.kernel.org, linux-mediatek@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] hwrng: add driver for MediaTek TRNG SMC
Date: Mon, 20 Feb 2023 17:58:11 -0600	[thread overview]
Message-ID: <20230220235811.GA618419-robh@kernel.org> (raw)
In-Reply-To: <0d5d5d00-8569-a642-cca7-798c8d24a986@gmail.com>

On Thu, Feb 16, 2023 at 12:32:10PM +0100, Matthias Brugger wrote:
> 
> 
> On 16/02/2023 11:03, AngeloGioacchino Del Regno wrote:
> > Il 15/02/23 14:27, Daniel Golle ha scritto:
> > > Add driver providing kernel-side support for the Random Number
> > > Generator hardware found on Mediatek SoCs which have a driver in ARM
> > > TrustedFirmware-A allowing Linux to read random numbers using a
> > > non-standard vendor-defined Secure Monitor Call.
> > > 
> > > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > 
> > Hello Daniel,
> > 
> > incidentally, I've also done some research on this one some months ago, when
> > I was deep in adding support for the Helio X10 SoC (MT6795) on Xperia M5.
> > 
> > The rng-v2 is simply the same rng but hypervised by the TF-A... and the only
> > difference is, well, as you're also pointing out, that we're using secure
> > monitor calls instead of direct MMIO handling.
> > 
> > There's also not much more than what you've implemented here and the only kind
> > of addition that we will ever see on this one will be about changing the SIP
> > command (as some older SoCs use a different one)... so...
> > 
> > ...I don't think that adding an entirely new driver is worth the noise, hence
> > I propose to simply add handling for the Secure RNG to mtk-rng.c instead: it's
> > shorter and we would only need to address one if branch on that probe function
> > to set a different callback.
> > 
> > The clock should then be optional for *some* of those "v2 handling" devices,
> > as if I recall correctly, some do need the clock to be handled from Linux
> > anyway... otherwise this v2 driver will be "soon" looking bloody similar to
> > the "v1", adding a bit of code duplication around.
> > 
> > What do you think?
> > 
> 
> That was exactly what I was thinking as well when I had a look at the
> driver. I propose to add it to mtk-rng.c. I don't see any value having a
> second driver for this.

Or fix the firmware to use the already defined SMC TRNG interface...

In any case, like the SMC TRNG, you don't need a DT binding. The 
firmware interface is discoverable. Try the SMC call and if it succeeds, 
you have a TRNG.

Rob

> 
> Regards,
> Matthias
> 
> > Regards,
> > Angelo
> > 
> > > ---
> > >   MAINTAINERS                         |  1 +
> > >   drivers/char/hw_random/Kconfig      | 16 +++++++
> > >   drivers/char/hw_random/Makefile     |  1 +
> > >   drivers/char/hw_random/mtk-rng-v2.c | 74 +++++++++++++++++++++++++++++
> > >   4 files changed, 92 insertions(+)
> > >   create mode 100644 drivers/char/hw_random/mtk-rng-v2.c
> > > 
> > 

WARNING: multiple messages have this Message-ID (diff)
From: Rob Herring <robh@kernel.org>
To: Matthias Brugger <matthias.bgg@gmail.com>
Cc: AngeloGioacchino Del Regno
	<angelogioacchino.delregno@collabora.com>,
	Daniel Golle <daniel@makrotopia.org>,
	Sean Wang <sean.wang@mediatek.com>,
	Olivia Mackall <olivia@selenic.com>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	Conor Dooley <conor.dooley@microchip.com>,
	Mingming Su <Mingming.Su@mediatek.com>,
	linux-crypto@vger.kernel.org, linux-mediatek@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] hwrng: add driver for MediaTek TRNG SMC
Date: Mon, 20 Feb 2023 17:58:11 -0600	[thread overview]
Message-ID: <20230220235811.GA618419-robh@kernel.org> (raw)
In-Reply-To: <0d5d5d00-8569-a642-cca7-798c8d24a986@gmail.com>

On Thu, Feb 16, 2023 at 12:32:10PM +0100, Matthias Brugger wrote:
> 
> 
> On 16/02/2023 11:03, AngeloGioacchino Del Regno wrote:
> > Il 15/02/23 14:27, Daniel Golle ha scritto:
> > > Add driver providing kernel-side support for the Random Number
> > > Generator hardware found on Mediatek SoCs which have a driver in ARM
> > > TrustedFirmware-A allowing Linux to read random numbers using a
> > > non-standard vendor-defined Secure Monitor Call.
> > > 
> > > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > 
> > Hello Daniel,
> > 
> > incidentally, I've also done some research on this one some months ago, when
> > I was deep in adding support for the Helio X10 SoC (MT6795) on Xperia M5.
> > 
> > The rng-v2 is simply the same rng but hypervised by the TF-A... and the only
> > difference is, well, as you're also pointing out, that we're using secure
> > monitor calls instead of direct MMIO handling.
> > 
> > There's also not much more than what you've implemented here and the only kind
> > of addition that we will ever see on this one will be about changing the SIP
> > command (as some older SoCs use a different one)... so...
> > 
> > ...I don't think that adding an entirely new driver is worth the noise, hence
> > I propose to simply add handling for the Secure RNG to mtk-rng.c instead: it's
> > shorter and we would only need to address one if branch on that probe function
> > to set a different callback.
> > 
> > The clock should then be optional for *some* of those "v2 handling" devices,
> > as if I recall correctly, some do need the clock to be handled from Linux
> > anyway... otherwise this v2 driver will be "soon" looking bloody similar to
> > the "v1", adding a bit of code duplication around.
> > 
> > What do you think?
> > 
> 
> That was exactly what I was thinking as well when I had a look at the
> driver. I propose to add it to mtk-rng.c. I don't see any value having a
> second driver for this.

Or fix the firmware to use the already defined SMC TRNG interface...

In any case, like the SMC TRNG, you don't need a DT binding. The 
firmware interface is discoverable. Try the SMC call and if it succeeds, 
you have a TRNG.

Rob

> 
> Regards,
> Matthias
> 
> > Regards,
> > Angelo
> > 
> > > ---
> > >   MAINTAINERS                         |  1 +
> > >   drivers/char/hw_random/Kconfig      | 16 +++++++
> > >   drivers/char/hw_random/Makefile     |  1 +
> > >   drivers/char/hw_random/mtk-rng-v2.c | 74 +++++++++++++++++++++++++++++
> > >   4 files changed, 92 insertions(+)
> > >   create mode 100644 drivers/char/hw_random/mtk-rng-v2.c
> > > 
> > 

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

  reply	other threads:[~2023-02-20 23:58 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-15 13:27 [PATCH 1/2] hwrng: add driver for MediaTek TRNG SMC Daniel Golle
2023-02-15 13:27 ` Daniel Golle
2023-02-15 13:27 ` [PATCH 2/2] dt-bindings: rng: Add MediaTek MT7981 TRNG Daniel Golle
2023-02-15 13:27   ` Daniel Golle
2023-02-16  9:14   ` Krzysztof Kozlowski
2023-02-16  9:14     ` Krzysztof Kozlowski
2023-02-16  9:19     ` Daniel Golle
2023-02-16  9:19       ` Daniel Golle
2023-02-16 10:03 ` [PATCH 1/2] hwrng: add driver for MediaTek TRNG SMC AngeloGioacchino Del Regno
2023-02-16 10:03   ` AngeloGioacchino Del Regno
2023-02-16 11:32   ` Matthias Brugger
2023-02-16 11:32     ` Matthias Brugger
2023-02-20 23:58     ` Rob Herring [this message]
2023-02-20 23:58       ` Rob Herring
2023-02-21  0:14       ` Daniel Golle
2023-02-21  0:14         ` Daniel Golle

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230220235811.GA618419-robh@kernel.org \
    --to=robh@kernel.org \
    --cc=Mingming.Su@mediatek.com \
    --cc=angelogioacchino.delregno@collabora.com \
    --cc=conor.dooley@microchip.com \
    --cc=daniel@makrotopia.org \
    --cc=herbert@gondor.apana.org.au \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=matthias.bgg@gmail.com \
    --cc=olivia@selenic.com \
    --cc=sean.wang@mediatek.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.