Re: [PATCH v5 4/4] gpio: rockchip: Set input direction when request irq

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Tue, Nov 12, 2024 at 2:23 PM Sebastian Reichel
<sebastian.reichel@xxxxxxxxxxxxx> wrote:
>
> Hi,
>
> On Tue, Nov 12, 2024 at 01:53:48PM +0100, Bartosz Golaszewski wrote:
> > On Tue, Nov 12, 2024 at 1:50 PM Bartosz Golaszewski <brgl@xxxxxxxx> wrote:
> > > On Tue, Nov 12, 2024 at 11:40 AM Andy Shevchenko
> > > <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> > > > On Tue, Nov 12, 2024 at 09:48:06AM +0100, Bartosz Golaszewski wrote:
> > > > > On Tue, Nov 12, 2024 at 2:54 AM Ye Zhang <ye.zhang@xxxxxxxxxxxxxx> wrote:
> > > > > >
> > > > > > Since the GPIO can only generate interrupts when its direction is set to
> > > > > > input, it is set to input before requesting the interrupt resources.
> > > >
> > > > ...
> > > >
> > > > > This looks like a fix to me, do you want it sent for stable? If so,
> > > > > please add the Fixes tag and put it first in the series.
> > > >
> > > > Independently on the resolution on this, can the first three be applied to
> > > > for-next? I think they are valuable from the documentation perspective as
> > > > it adds the explanation of the version register bit fields.
> > > >
> > > > The last one seems to me independent (code wise, meaning no potential
> > > > conflicts) to the rest and may be applied to for-current later on.
> > > >
> > > > --
> > > > With Best Regards,
> > > > Andy Shevchenko
> > > >
> > > >
> > >
> > > There's another issue I see with this patch. It effectively changes
> > > the pin's direction behind the back of the GPIOLIB. If a GPIO is
> > > requested, its direction set to output and another orthogonal user
> > > requests the same pin as input, we'll never update the FLAG_IS_OUT
> >
> > I meant to say "same pin as interrupt". Sorry for the noise.
>
> GPIO output and interrupt at the same time looks like a misconfiguration
> to me. Maybe check for that situation and return -EBUSY?
>

Of course it is. That's why we check for it in
gpiod_direction_output() and in gpiochip_lock_as_irq(): refuse to set
direction to output if the GPIO is used as interrupt and refuse to use
it as interrupt if direction already is output respectively.

This patch however proposes to set the direction to input implicitly
which is wrong. The underlying gpiochip_reqres_irq() will check the
current direction and return -EIO if it's output.

Bart