On Mon, Jun 17, 2024 at 10:56 AM Marek Behún <kabel@xxxxxxxxxx> wrote:
>
> On Mon, 17 Jun 2024 10:38:31 +0200
> Bartosz Golaszewski <brgl@xxxxxxxx> wrote:
>
> > On Wed, Jun 5, 2024 at 9:00 PM Andy Shevchenko
> > <andy.shevchenko@xxxxxxxxx> wrote:
> > >
> > > On Wed, Jun 5, 2024 at 7:19 PM Marek Behún <kabel@xxxxxxxxxx> 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.
> > >
> >
> > Andy's worried about reference counting of the GPIO device. Maybe you
> > should just ref the GPIO device in irq_request_resources() and unref
> > it in irq_release_resources()? Then you could use gpiochip_get_desc()
> > just fine.
>
> But this is already being done.
>
> The irqchip uses GPIOCHIP_IRQ_RESOURCE_HELPERS macro in its definition:
>
> static const struct irq_chip omnia_mcu_irq_chip = {
> ...
> GPIOCHIP_IRQ_RESOURCE_HELPERS,
> };
>
> This means that gpiochip_irq_reqres() and gpiochip_irq_relres() are
> used as irq_request_resources() and irq_release_resources().
>
> The gpiochip_reqres_irq() code increases the module refcount and even
> locks the line as IRQ:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpio/gpiolib.c?h=v6.10-rc4#n3732
>
> Marek
Andy: what is the issue here then exactly?
Bart
