Add support for true random number generator provided by the MCU.
New Omnia boards come without the Atmel SHA204-A chip. Instead the
crypto functionality is provided by new microcontroller, which has
a TRNG peripheral.
Signed-off-by: Marek Behún <[email protected]>
---
drivers/platform/cznic/Kconfig | 2 +
drivers/platform/cznic/Makefile | 1 +
.../platform/cznic/turris-omnia-mcu-base.c | 6 +-
.../platform/cznic/turris-omnia-mcu-gpio.c | 2 +-
.../platform/cznic/turris-omnia-mcu-trng.c | 103 ++++++++++++++++++
drivers/platform/cznic/turris-omnia-mcu.h | 8 ++
6 files changed, 120 insertions(+), 2 deletions(-)
create mode 100644 drivers/platform/cznic/turris-omnia-mcu-trng.c
diff --git a/drivers/platform/cznic/Kconfig b/drivers/platform/cznic/Kconfig
index e262930b3faf..6edac80d5fa3 100644
--- a/drivers/platform/cznic/Kconfig
+++ b/drivers/platform/cznic/Kconfig
@@ -18,6 +18,7 @@ config TURRIS_OMNIA_MCU
depends on I2C
select GPIOLIB
select GPIOLIB_IRQCHIP
+ select HW_RANDOM
select RTC_CLASS
select WATCHDOG_CORE
help
@@ -27,6 +28,7 @@ config TURRIS_OMNIA_MCU
- board poweroff into true low power mode (with voltage regulators
disabled) and the ability to configure wake up from this mode (via
rtcwake)
+ - true random number generator (if available on the MCU)
- MCU watchdog
- GPIO pins
- to get front button press events (the front button can be
diff --git a/drivers/platform/cznic/Makefile b/drivers/platform/cznic/Makefile
index 687f7718c0a1..eae4c6b341ff 100644
--- a/drivers/platform/cznic/Makefile
+++ b/drivers/platform/cznic/Makefile
@@ -4,4 +4,5 @@ obj-$(CONFIG_TURRIS_OMNIA_MCU) += turris-omnia-mcu.o
turris-omnia-mcu-y := turris-omnia-mcu-base.o
turris-omnia-mcu-y += turris-omnia-mcu-gpio.o
turris-omnia-mcu-y += turris-omnia-mcu-sys-off-wakeup.o
+turris-omnia-mcu-y += turris-omnia-mcu-trng.o
turris-omnia-mcu-y += turris-omnia-mcu-watchdog.o
diff --git a/drivers/platform/cznic/turris-omnia-mcu-base.c b/drivers/platform/cznic/turris-omnia-mcu-base.c
index f44996588d38..1d4153a96526 100644
--- a/drivers/platform/cznic/turris-omnia-mcu-base.c
+++ b/drivers/platform/cznic/turris-omnia-mcu-base.c
@@ -380,7 +380,11 @@ static int omnia_mcu_probe(struct i2c_client *client)
if (err)
return err;
- return omnia_mcu_register_gpiochip(mcu);
+ err = omnia_mcu_register_gpiochip(mcu);
+ if (err)
+ return err;
+
+ return omnia_mcu_register_trng(mcu);
}
static const struct of_device_id of_omnia_mcu_match[] = {
diff --git a/drivers/platform/cznic/turris-omnia-mcu-gpio.c b/drivers/platform/cznic/turris-omnia-mcu-gpio.c
index bc7965e6c879..972364d3d223 100644
--- a/drivers/platform/cznic/turris-omnia-mcu-gpio.c
+++ b/drivers/platform/cznic/turris-omnia-mcu-gpio.c
@@ -174,7 +174,7 @@ static const struct omnia_gpio omnia_gpios[64] = {
};
/* mapping from interrupts to indexes of GPIOs in the omnia_gpios array */
-static const u8 omnia_int_to_gpio_idx[32] = {
+const u8 omnia_int_to_gpio_idx[32] = {
[__bf_shf(OMNIA_INT_CARD_DET)] = 4,
[__bf_shf(OMNIA_INT_MSATA_IND)] = 5,
[__bf_shf(OMNIA_INT_USB30_OVC)] = 6,
diff --git a/drivers/platform/cznic/turris-omnia-mcu-trng.c b/drivers/platform/cznic/turris-omnia-mcu-trng.c
new file mode 100644
index 000000000000..fbde00f3fca1
--- /dev/null
+++ b/drivers/platform/cznic/turris-omnia-mcu-trng.c
@@ -0,0 +1,103 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * CZ.NIC's Turris Omnia MCU TRNG driver
+ *
+ * 2024 by Marek Behún <[email protected]>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/completion.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/driver.h>
+#include <linux/hw_random.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/minmax.h>
+#include <linux/module.h>
+#include <linux/string.h>
+#include <linux/turris-omnia-mcu-interface.h>
+#include <linux/types.h>
+
+#include "turris-omnia-mcu.h"
+
+#define OMNIA_CMD_TRNG_MAX_ENTROPY_LEN 64
+
+static irqreturn_t omnia_trng_irq_handler(int irq, void *dev_id)
+{
+ struct omnia_mcu *mcu = dev_id;
+
+ complete(&mcu->trng_completion);
+
+ return IRQ_HANDLED;
+}
+
+static int omnia_trng_read(struct hwrng *rng, void *data, size_t max, bool wait)
+{
+ struct omnia_mcu *mcu = (struct omnia_mcu *)rng->priv;
+ u8 reply[1 + OMNIA_CMD_TRNG_MAX_ENTROPY_LEN];
+ int err, bytes;
+
+ if (!wait && !completion_done(&mcu->trng_completion))
+ return 0;
+
+ do {
+ if (wait_for_completion_interruptible(&mcu->trng_completion))
+ return -ERESTARTSYS;
+
+ err = omnia_cmd_read(mcu->client,
+ OMNIA_CMD_TRNG_COLLECT_ENTROPY,
+ reply, sizeof(reply));
+ if (err)
+ return err;
+
+ bytes = min3(reply[0], max, OMNIA_CMD_TRNG_MAX_ENTROPY_LEN);
+ } while (wait && !bytes);
+
+ memcpy(data, &reply[1], bytes);
+
+ return bytes;
+}
+
+int omnia_mcu_register_trng(struct omnia_mcu *mcu)
+{
+ struct device *dev = &mcu->client->dev;
+ u8 irq_idx, dummy;
+ int irq, err;
+
+ if (!(mcu->features & OMNIA_FEAT_TRNG))
+ return 0;
+
+ irq_idx = omnia_int_to_gpio_idx[__bf_shf(OMNIA_INT_TRNG)];
+ irq = gpiod_to_irq(gpiochip_get_desc(&mcu->gc, irq_idx));
+ if (irq < 0)
+ return dev_err_probe(dev, irq, "Cannot get TRNG IRQ\n");
+
+ /*
+ * If someone else cleared the TRNG interrupt but did not read the
+ * entropy, a new interrupt won't be generated, and entropy collection
+ * will be stuck. Ensure an interrupt will be generated by executing
+ * the collect entropy command (and discarding the result).
+ */
+ err = omnia_cmd_read(mcu->client, OMNIA_CMD_TRNG_COLLECT_ENTROPY,
+ &dummy, 1);
+ if (err)
+ return err;
+
+ init_completion(&mcu->trng_completion);
+
+ err = devm_request_threaded_irq(dev, irq, NULL, omnia_trng_irq_handler,
+ IRQF_ONESHOT, "turris-omnia-mcu-trng",
+ mcu);
+ if (err)
+ return dev_err_probe(dev, err, "Cannot request TRNG IRQ\n");
+
+ mcu->trng.name = "turris-omnia-mcu-trng";
+ mcu->trng.read = omnia_trng_read;
+ mcu->trng.priv = (unsigned long)mcu;
+
+ err = devm_hwrng_register(dev, &mcu->trng);
+ if (err)
+ return dev_err_probe(dev, err, "Cannot register TRNG\n");
+
+ return 0;
+}
diff --git a/drivers/platform/cznic/turris-omnia-mcu.h b/drivers/platform/cznic/turris-omnia-mcu.h
index 5f846909934f..ee8d58516156 100644
--- a/drivers/platform/cznic/turris-omnia-mcu.h
+++ b/drivers/platform/cznic/turris-omnia-mcu.h
@@ -9,7 +9,9 @@
#define __TURRIS_OMNIA_MCU_H
#include <linux/bitops.h>
+#include <linux/completion.h>
#include <linux/gpio/driver.h>
+#include <linux/hw_random.h>
#include <linux/if_ether.h>
#include <linux/mutex.h>
#include <linux/rtc.h>
@@ -47,6 +49,10 @@ struct omnia_mcu {
/* MCU watchdog */
struct watchdog_device wdt;
+
+ /* true random number generator */
+ struct hwrng trng;
+ struct completion trng_completion;
};
int omnia_cmd_write_read(const struct i2c_client *client,
@@ -176,11 +182,13 @@ static inline int omnia_cmd_read_u8(const struct i2c_client *client, u8 cmd,
return omnia_cmd_read(client, cmd, reply, sizeof(*reply));
}
+extern const u8 omnia_int_to_gpio_idx[32];
extern const struct attribute_group omnia_mcu_gpio_group;
extern const struct attribute_group omnia_mcu_poweroff_group;
int omnia_mcu_register_gpiochip(struct omnia_mcu *mcu);
int omnia_mcu_register_sys_off_and_wakeup(struct omnia_mcu *mcu);
+int omnia_mcu_register_trng(struct omnia_mcu *mcu);
int omnia_mcu_register_watchdog(struct omnia_mcu *mcu);
#endif /* __TURRIS_OMNIA_MCU_H */
--
2.44.2
On Wed, Jun 5, 2024 at 7:19 PM Marek Behún <[email protected]> wrote:
>
> Add support for true random number generator provided by the MCU.
> New Omnia boards come without the Atmel SHA204-A chip. Instead the
> crypto functionality is provided by new microcontroller, which has
> a TRNG peripheral.
+Cc: Bart for gpiochip_get_desc() usage.
...
> +#include <linux/bitfield.h>
> +#include <linux/completion.h>
+ errno.h
> +#include <linux/gpio/consumer.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/hw_random.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/minmax.h>
> +#include <linux/module.h>
> +#include <linux/string.h>
> +#include <linux/turris-omnia-mcu-interface.h>
As per other patches.
> +#include <linux/types.h>
> +
> +#include "turris-omnia-mcu.h"
...
> + irq_idx = omnia_int_to_gpio_idx[__bf_shf(OMNIA_INT_TRNG)];
> + irq = gpiod_to_irq(gpiochip_get_desc(&mcu->gc, irq_idx));
> + if (irq < 0)
> + return dev_err_probe(dev, irq, "Cannot get TRNG IRQ\n");
Okay, it's a bit more complicated than that. The gpiochip_get_desc()
shouldn't be used. Bart, what can you suggest to do here? Opencoding
it doesn't sound to me a (fully) correct approach in a long term.
--
With Best Regards,
Andy Shevchenko
On Wed, 5 Jun 2024 22:00:20 +0300
Andy Shevchenko <[email protected]> wrote:
> > + irq_idx = omnia_int_to_gpio_idx[__bf_shf(OMNIA_INT_TRNG)];
> > + irq = gpiod_to_irq(gpiochip_get_desc(&mcu->gc, irq_idx));
> > + if (irq < 0)
> > + return dev_err_probe(dev, irq, "Cannot get TRNG IRQ\n");
>
> Okay, it's a bit more complicated than that. The gpiochip_get_desc()
> shouldn't be used. Bart, what can you suggest to do here? Opencoding
> it doesn't sound to me a (fully) correct approach in a long term.
Note that I can't use gpiochip_request_own_desc(), nor any other
function that calls gpio_request_commit() (like gpiod_get()), because
that checks for gpiochip_line_is_valid(), and this returns false for
the TRNG line, cause that line is not a GPIO line, but interrupt only
line.
That is why I used
irq = irq_create_mapping(dev, mcu->gc.irq.domain, irq_idx);
until v7, with no reference to gpio descriptors, since this line is not
a GPIO line.
We have discussed this back in April, in the thread
https://lore.kernel.org/soc/[email protected]/
where we concluded that
irq = gpiod_to_irq(gpiochip_get_desc(gc, irq_idx));
is better...
Marek
On Wed, 5 Jun 2024 22:00:20 +0300
Andy Shevchenko <[email protected]> wrote:
> > +#include <linux/bitfield.h>
> > +#include <linux/completion.h>
>
> + errno.h
I use -EIO, -EINVAL and -ENOMEM in turris-omnia-mcu-base.c,
-EINVAL, -ENOTSUPP in turris-omnia-mcu-gpio.c.
Should I include errno.h in those also? Or is this only needed for
-ERESTARTSYS?
Marek
On Thu, Jun 06, 2024 at 11:11:13AM +0200, Marek Beh?n wrote:
> On Wed, 5 Jun 2024 22:00:20 +0300
> Andy Shevchenko <[email protected]> wrote:
>
> > > +#include <linux/bitfield.h>
> > > +#include <linux/completion.h>
> >
> > + errno.h
>
> I use -EIO, -EINVAL and -ENOMEM in turris-omnia-mcu-base.c,
> -EINVAL, -ENOTSUPP in turris-omnia-mcu-gpio.c.
> Should I include errno.h in those also?
If you have err.h, then no (it includes asm/errno.h), otherwise, yes.
> Or is this only needed for -ERESTARTSYS?
Definitely yes for Linux internal error codes (>= 512).
Note, that ENOTSUPP is also Linux internal code.
--
With Best Regards,
Andy Shevchenko
On Thu, Jun 06, 2024 at 10:53:08AM +0200, Marek Beh?n wrote:
> On Wed, 5 Jun 2024 22:00:20 +0300
> Andy Shevchenko <[email protected]> wrote:
>
> > > + irq_idx = omnia_int_to_gpio_idx[__bf_shf(OMNIA_INT_TRNG)];
> > > + irq = gpiod_to_irq(gpiochip_get_desc(&mcu->gc, irq_idx));
> > > + if (irq < 0)
> > > + return dev_err_probe(dev, irq, "Cannot get TRNG IRQ\n");
> >
> > Okay, it's a bit more complicated than that. The gpiochip_get_desc()
> > shouldn't be used. Bart, what can you suggest to do here? Opencoding
> > it doesn't sound to me a (fully) correct approach in a long term.
>
> Note that I can't use gpiochip_request_own_desc(), nor any other
> function that calls gpio_request_commit() (like gpiod_get()), because
> that checks for gpiochip_line_is_valid(), and this returns false for
> the TRNG line, cause that line is not a GPIO line, but interrupt only
> line.
>
> That is why I used
> irq = irq_create_mapping(dev, mcu->gc.irq.domain, irq_idx);
> until v7, with no reference to gpio descriptors, since this line is not
> a GPIO line.
>
> We have discussed this back in April, in the thread
> https://lore.kernel.org/soc/[email protected]/
> where we concluded that
> irq = gpiod_to_irq(gpiochip_get_desc(gc, irq_idx));
> is better...
That's fine to not use other APIs, the problem here is with reference counting
on the GPIO device. The API you could use is gpio_device_get_desc(). But you
need to have a GPIO device pointer somewhere in your driver being available.
--
With Best Regards,
Andy Shevchenko
On Thu, 6 Jun 2024 13:11:09 +0300
Andy Shevchenko <[email protected]> wrote:
> On Thu, Jun 06, 2024 at 10:53:08AM +0200, Marek Behún wrote:
> > On Wed, 5 Jun 2024 22:00:20 +0300
> > Andy Shevchenko <[email protected]> wrote:
> >
> > > > + irq_idx = omnia_int_to_gpio_idx[__bf_shf(OMNIA_INT_TRNG)];
> > > > + irq = gpiod_to_irq(gpiochip_get_desc(&mcu->gc, irq_idx));
> > > > + if (irq < 0)
> > > > + return dev_err_probe(dev, irq, "Cannot get TRNG IRQ\n");
> > >
> > > Okay, it's a bit more complicated than that. The gpiochip_get_desc()
> > > shouldn't be used. Bart, what can you suggest to do here? Opencoding
> > > it doesn't sound to me a (fully) correct approach in a long term.
> >
> > Note that I can't use gpiochip_request_own_desc(), nor any other
> > function that calls gpio_request_commit() (like gpiod_get()), because
> > that checks for gpiochip_line_is_valid(), and this returns false for
> > the TRNG line, cause that line is not a GPIO line, but interrupt only
> > line.
> >
> > That is why I used
> > irq = irq_create_mapping(dev, mcu->gc.irq.domain, irq_idx);
> > until v7, with no reference to gpio descriptors, since this line is not
> > a GPIO line.
> >
> > We have discussed this back in April, in the thread
> > https://lore.kernel.org/soc/[email protected]/
> > where we concluded that
> > irq = gpiod_to_irq(gpiochip_get_desc(gc, irq_idx));
> > is better...
>
> That's fine to not use other APIs, the problem here is with reference counting
> on the GPIO device. The API you could use is gpio_device_get_desc(). But you
> need to have a GPIO device pointer somewhere in your driver being available.
Rewriting to gpio_device_get_desc() is simple, since
gpiochip_get_desc(gc, hwnum)
is equivalent to
gpio_device_get_desc(gc->gpiodev, hwnum)
Obviously neither of these take care of reference counting. But what
reference counting are you talking about?
Is it the
try_module_get(desc->gdev->owner)
in gpiod_request()?
Or are we talking only about the FLAG_REQUESTED flag on the descriptor
flags? (This one should not be needed since the GPIO line cannot be
requested, becuase it is not a valid GPIO line.)
Since the line is not a valid GPIO line, I thought that we don't need
to refcount in GPIO code. gpiod_to_irq() will call the gpiochip's
.to_irq() method, which will call gpiochip_to_irq(), which will call
irq_create_mapping()
gpiod_to_irq()
gpiochip_to_irq()
irq_create_mapping()
Then on gpiochip removal, the gpiochip_irqchip_remove() manually
disposes all IRQ mappings with irq_dispose_mapping().
Marek
On Fri, 7 Jun 2024 18:30:24 +0800
Herbert Xu <[email protected]> wrote:
> On Wed, Jun 05, 2024 at 06:18:49PM +0200, Marek Behún wrote:
> >
> > +static int omnia_trng_read(struct hwrng *rng, void *data, size_t max, bool wait)
> > +{
> > + struct omnia_mcu *mcu = (struct omnia_mcu *)rng->priv;
>
> Please don't cast rng->priv in this manner. Please take a look at
> drivers/char/hw_random/bcm2835-rng.c for how it should be done.
>
> Thanks,
THX, prepared for next version.