Re: how to request gpiochip line which is only valid as an interrupt?

[Marek Behún <kabel@xxxxxxxxxx>]

On Wed, 12 Jun 2024 14:30:58 +0200
Bartosz Golaszewski <brgl@xxxxxxxx> wrote:

> On Wed, Jun 12, 2024 at 11:03 AM Marek Behún <kabel@xxxxxxxxxx> wrote:
> >
> > On Wed, 12 Jun 2024 10:55:55 +0200
> > Marek Behún <kabel@xxxxxxxxxx> wrote:
> >  
> > > > Users can still read the value of this pin but won't be able to set
> > > > direction to output.  
> > >
> > > Users are not supposed to read value of this pin, because it is not a
> > > GPIO pin. The corresponding bit is not set in gpiochip.valid_mask.
> > > It is for example impossible to export it in /sys/class/gpio.
> > >
> > > This line is valid only as an IRQ (the
> > > corresponding bit is set in gpiochip.irq.valid_mask).  
> >  
> 
> Ah, I missed the bit about these pins not being marked as valid.
> 
> > I am starting to thing that this might be the problem, that the line is
> > not valid as GPIO, only as an IRQ. gpiolib seems to be unable to handle
> > that.
> >
> > Indeed, the definition of the function
> >   gpiochip_irqchip_irq_valid()
> > first checks if the line is valid as gpio:
> >
> >   static bool gpiochip_irqchip_irq_valid(const struct gpio_chip *gc,
> >                                          unsigned int offset)
> >   {
> >           if (!gpiochip_line_is_valid(gc, offset))
> >                   return false;
> >           ...
> >  
> 
> TBH Maybe using the GPIO provider APIs for something that isn't a GPIO
> doesn't make much sense? They are called GPIO irqchips for a reason
> after all? How about handling the non-GPIO interrupts with regular
> irqchip code and only exposing actual GPIOs using gpiolib?
> 
> What is the exact layout of these pins? Are the GPIOs and non-GPIOs
> somehow grouped together? Maybe export several separate GPIO banks?

It is a little bit more complicated.

The MCU has some pins configured in GPIO mode, and the purpose of this
driver is to expose them to the operating system.
Their direction cannot be changed (with one exception).

Because of different MCU firmware versions, different GPIO groups are
readable/writable via different MCU commands. The driver exposes the
GPIOs via 3 banks:
 bank  cmd width  read command             write command
  1st  16 bits    GET_STATUS_WORD          GENERAL_CONTROL
  2nd  32 bits    GET_EXT_STATUS_DWORD     n/a
  3rd  16 bits    GET_EXT_CONTROL_STATUS   EXT_CONTROL

So for example the 1st bank has 16 lines, because the I2C command is 16
bits wide. But of these 16 bits, not all bits correspond to a real GPIO.
But the binding was created as if the bank has 16 lines to make things
simpler, and they can potentially been filled with firmware upgrade.
(More importantly we want to be compaitble with real U-Boot binaries
which already use this binding.).

Now, interrupt support was added to the MCU firmware later, and reading
interrupt status is done by a different MCU command (GET_INT_AND_CLEAR).
This command supports 32 interrupts, of which some map to the GPIOs,
but the mapping is not 1 to 1, since there is only one interrupt
command (32 bits wide) and three GPIO commands (64 bits wide in total).

So the driver exposes a gpiochip with 64 lines (of which not all are
valid), and to make it simple, it maps the GPIO lines that support
interrupts to the corresponding interrupt bit in the MCU interrupt
command. This way it is possible to use the gpiolib's internal irqchip.

Example:

  GPIO bank/line  IRQ line  Description
          0/   4         4  MiniPCIe0 Card Detect
          0/   5         5  MiniPCIe0 mSATA Indicator
          0/  12        12  Front Button

But the MCU also provides three interrupts that do not map to a real
GPIO, they are semantically different. For example the trng interrupt
is fired when MCU has generated entropy, to inform the system that it
can collect it.

I have mapped these 3 interrupts onto bits 11, 13 and 14 of bank 0,
because bank 0 corresponds to the GET_STATUS_WORD command, and these
bits of the GET_STATUS_WORD command reply can never correspond to a
GPIO (they have different meaning).

I do realize that this is rather complicated, but it is properly
documented in the device-tree binding, and is already being used this
way in the wild...

Marek