Linux-ARM-MSM Archive on lore.kernel.org
 help / color / Atom feed
From: Maulik Shah <mkshah@codeaurora.org>
To: Stephen Boyd <swboyd@chromium.org>,
	bjorn.andersson@linaro.org, evgreen@chromium.org,
	linus.walleij@linaro.org, maz@kernel.org, mka@chromium.org
Cc: linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	linux-gpio@vger.kernel.org, agross@kernel.org,
	tglx@linutronix.de, jason@lakedaemon.net, dianders@chromium.org,
	rnayak@codeaurora.org, ilina@codeaurora.org,
	lsrao@codeaurora.org
Subject: Re: [PATCH v2 1/4] gpio: gpiolib: Allow GPIO IRQs to lazy disable
Date: Thu, 28 May 2020 18:41:23 +0530
Message-ID: <948defc1-5ea0-adbb-185b-5f5a81f2e28a@codeaurora.org> (raw)
In-Reply-To: <159062812628.69627.2153485337510882984@swboyd.mtv.corp.google.com>

Hi,

On 5/28/2020 6:38 AM, Stephen Boyd wrote:
> Quoting Maulik Shah (2020-05-27 04:26:14)
>> On 5/27/2020 3:14 PM, Stephen Boyd wrote:
>>> Quoting Maulik Shah (2020-05-23 10:11:10)
>>>> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
>>>> index eaa0e20..3810cd0 100644
>>>> --- a/drivers/gpio/gpiolib.c
>>>> +++ b/drivers/gpio/gpiolib.c
>>>> @@ -2465,32 +2465,37 @@ static void gpiochip_irq_relres(struct irq_data *d)
>>>>           gpiochip_relres_irq(gc, d->hwirq);
>>>>    }
>>>>    
>>>> +static void gpiochip_irq_mask(struct irq_data *d)
>>>> +{
>>>> +       struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
>>>> +
>>>> +       if (gc->irq.irq_mask)
>>>> +               gc->irq.irq_mask(d);
>>>> +       gpiochip_disable_irq(gc, d->hwirq);
>>> How does this work in the lazy case when I want to drive the GPIO? Say I
>>> have a GPIO that is also an interrupt. The code would look like
>>>
>>>    struct gpio_desc *gpio = gpiod_get(...)
>>>    unsigned int girq = gpiod_to_irq(gpio)
>>>
>>>    request_irq(girq, ...);
>>>
>>>    disable_irq(girq);
>>>    gpiod_direction_output(gpio, 1);
>>>
>>> In the lazy case genirq wouldn't call the mask function until the first
>>> interrupt arrived on the GPIO line. If that never happened then wouldn't
>>> we be blocked in gpiod_direction_output() when the test_bit() sees
>>> FLAG_USED_AS_IRQ? Or do we need irqs to be released before driving
>>> gpios?
>> The client driver can decide to unlazy disable IRQ with below API...
>>
>>    irq_set_status_flags(girq, IRQ_DISABLE_UNLAZY);
>>
>> This will immediatly invoke mask function (unlazy disable) from genirq,
>> even though irq_disable is not implemented.
>>
> Sure a consumer can disable the lazy feature, but that shouldn't be
> required to make this work. The flag was introduced in commit
> e9849777d0e2 ("genirq: Add flag to force mask in
> disable_irq[_nosync]()") specifically to help devices that can't disable
> the interrupt in their own device avoid a double interrupt.
i don't think this will be a problem.

Case 1) Client driver have locked gpio to be used as IRQ using 
gpiochip_lock_as_irq()

In this case, When client driver want to change the direction for a 
gpio, they will invoke gpiod_direction_output().
I see it checks for two flags (pasted below), if GPIO is used as IRQ and 
whether its enabled IRQ or not.

        /* GPIOs used for enabled IRQs shall not be set as output */
         if (test_bit(FLAG_USED_AS_IRQ, &desc->flags) &&
             test_bit(FLAG_IRQ_IS_ENABLED, &desc->flags)) {

The first one (FLAG_USED_AS_IRQ) is set only if client driver in past 
have locked gpio to use as IRQ with a call to gpiochip_lock_as_irq()
then it never gets unlocked until clients invoke gpiochip_unlock_as_irq().

So i presume the client driver which in past locked gpio to be used as 
IRQ, now wants to change direction then it will
a. first unlock to use as IRQ
b. then change the direction.

Once it unlocks in step (a), both these flags will be cleared and there 
won't be any error in changing direction in step (b).

Case 2) Client driver did not lock gpio to be used as IRQ.

In this case, it will be straight forward to change the direction, as 
FLAG_USED_AS_IRQ is never set.

Thanks,
Maulik

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation


  reply index

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-23 17:11 [PATCH v2 0/4] irqchip: qcom: pdc: Introduce irq_set_wake call Maulik Shah
2020-05-23 17:11 ` [PATCH v2 1/4] gpio: gpiolib: Allow GPIO IRQs to lazy disable Maulik Shah
2020-05-25 11:55   ` Linus Walleij
2020-05-25 12:22     ` Hans Verkuil
2020-05-26  4:45       ` Maulik Shah
2020-05-27 10:38     ` Hans Verkuil
2020-05-27 13:56       ` Linus Walleij
2020-05-27 14:15         ` Russell King - ARM Linux admin
2020-05-27  9:44   ` Stephen Boyd
2020-05-27 11:26     ` Maulik Shah
2020-05-28  1:08       ` Stephen Boyd
2020-05-28 13:11         ` Maulik Shah [this message]
2020-05-28 22:22           ` Stephen Boyd
2020-05-29  9:57             ` Maulik Shah
2020-05-28 21:33   ` Linus Walleij
     [not found]     ` <159070486671.69627.662167272601689396@swboyd.mtv.corp.google.com>
2020-06-03 12:28       ` Linus Walleij
2020-05-23 17:11 ` [PATCH v2 2/4] pinctrl: qcom: Remove irq_disable callback from msmgpio irqchip Maulik Shah
2020-05-23 17:11 ` [PATCH v2 3/4] pinctrl: qcom: Add msmgpio irqchip flags Maulik Shah
2020-05-27  9:47   ` Stephen Boyd
2020-05-27 12:30     ` Maulik Shah
2020-05-23 17:11 ` [PATCH v2 4/4] irqchip: qcom-pdc: Introduce irq_set_wake call Maulik Shah
2020-05-27 10:15   ` Stephen Boyd
2020-05-29  9:20     ` Maulik Shah
2020-05-30 19:26       ` Stephen Boyd
2020-06-01 11:38         ` Maulik Shah
2020-06-16 11:57           ` Stephen Boyd
2020-06-18 10:03             ` Maulik Shah
2020-06-19  9:26               ` Stephen Boyd
2020-06-22  9:19                 ` Maulik Shah

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=948defc1-5ea0-adbb-185b-5f5a81f2e28a@codeaurora.org \
    --to=mkshah@codeaurora.org \
    --cc=agross@kernel.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=dianders@chromium.org \
    --cc=evgreen@chromium.org \
    --cc=ilina@codeaurora.org \
    --cc=jason@lakedaemon.net \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lsrao@codeaurora.org \
    --cc=maz@kernel.org \
    --cc=mka@chromium.org \
    --cc=rnayak@codeaurora.org \
    --cc=swboyd@chromium.org \
    --cc=tglx@linutronix.de \
    /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

Linux-ARM-MSM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-msm/0 linux-arm-msm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-msm linux-arm-msm/ https://lore.kernel.org/linux-arm-msm \
		linux-arm-msm@vger.kernel.org
	public-inbox-index linux-arm-msm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-arm-msm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git