All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Niebel <list-09_linux_arm-7U5DVgMjAv4@public.gmane.org>
To: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	Markus Niebel
	<Markus.Niebel-HVi9AzVXQk5Wk0Htik3J/w@public.gmane.org>,
	Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>,
	"linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org"
	<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>,
	Grant Likely
	<grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	"devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: Re: [[RFC PATCH]] gpio: gpio-mxc: make sure gpio is input when request IRQ
Date: Fri, 25 Jul 2014 14:52:05 +0200	[thread overview]
Message-ID: <53D252F5.2060601@tqsc.de> (raw)
In-Reply-To: <CACRpkdarH_-2uW55C5bAUxM1ESgSb6tJAWc4UqEoWr8uc_kQRQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Am 25.07.2014 13:38, wrote Linus Walleij:
> On Thu, Jul 24, 2014 at 10:12 AM, Markus Niebel
> <list-09_linux_arm-7U5DVgMjAv4@public.gmane.org> wrote:
>> Am 23.07.2014 18:14, wrote Linus Walleij:
> 
>>> So always prepare the hardware and make it ready for action in respective
>>> callbacks from the GPIO and irqchip APIs. Do not rely on gpiod_to_irq() having
>>> been called first.
>>>
>>
>> So a gpio driver is responsible to read status of gpio lines and flag any gpio line
>> currently configured as out (base on the information read from hardware registers)
>> on driver probe time - correct?
> 
> I don't think anyone reads that information explicitly to set up
> these flags.
> 
> Drivers just leave the pins in their power-on maiden state without
> trying to figure out how they're set-up. But as you say, if you call
> gpiod_get_direction() on them, the flag gets set up indeed.
> 
> So usually these flags are set by code, calling
> gpiod_[get/set]_direction().
> Then they do get flagged as outputs or inputs.
> 
>> If yes is the driver allowed to call
>> gpiod_get_direction() to have the FLAG_IS_OUT set in the gpiolib layer?
> 
> I don't know if it'd be a good idea to loop over all gpios in a new
> irqchip and fetch the direction just to get the flags right, so far
> we haven't done that and I don't know what the usecase would be.
> 

I've came to a situation where it would have been helpful to know - bootloader
configured a pin as output and linux tried to configure the pin as IRQ input.
*YES I KNOW THIS IS NOT CORRECT* but it took some time to see, what happened.

> If we need that we should do it in gpiolib for all drivers don't you
> think?

A pragmatic / lean solution would be to deny the IRQ configuration when
seeing the pin configured as output in hardware and print out an error
on the gpio driver level.

> 
> But then we need a rationale for doing it, other than it's nice :-)
> It is already called on-the-fly by debugfs when a user needs that
> info.

See above, I think an error on a misconfigured system would be enough.
Maybe the documentation for gpio drivers should have a hint for driver
implementors.

So first step would be to convert the driver to the irqchip helper and then
add calls to gpiod_[un]lock_as_irq at the right places.

Will try to do that after my holiday.

> 
> Yours,
> Linus Walleij
> 
Yours,

Markus Niebel
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: list-09_linux_arm@tqsc.de (Markus Niebel)
To: linux-arm-kernel@lists.infradead.org
Subject: [[RFC PATCH]] gpio: gpio-mxc: make sure gpio is input when request IRQ
Date: Fri, 25 Jul 2014 14:52:05 +0200	[thread overview]
Message-ID: <53D252F5.2060601@tqsc.de> (raw)
In-Reply-To: <CACRpkdarH_-2uW55C5bAUxM1ESgSb6tJAWc4UqEoWr8uc_kQRQ@mail.gmail.com>

Am 25.07.2014 13:38, wrote Linus Walleij:
> On Thu, Jul 24, 2014 at 10:12 AM, Markus Niebel
> <list-09_linux_arm@tqsc.de> wrote:
>> Am 23.07.2014 18:14, wrote Linus Walleij:
> 
>>> So always prepare the hardware and make it ready for action in respective
>>> callbacks from the GPIO and irqchip APIs. Do not rely on gpiod_to_irq() having
>>> been called first.
>>>
>>
>> So a gpio driver is responsible to read status of gpio lines and flag any gpio line
>> currently configured as out (base on the information read from hardware registers)
>> on driver probe time - correct?
> 
> I don't think anyone reads that information explicitly to set up
> these flags.
> 
> Drivers just leave the pins in their power-on maiden state without
> trying to figure out how they're set-up. But as you say, if you call
> gpiod_get_direction() on them, the flag gets set up indeed.
> 
> So usually these flags are set by code, calling
> gpiod_[get/set]_direction().
> Then they do get flagged as outputs or inputs.
> 
>> If yes is the driver allowed to call
>> gpiod_get_direction() to have the FLAG_IS_OUT set in the gpiolib layer?
> 
> I don't know if it'd be a good idea to loop over all gpios in a new
> irqchip and fetch the direction just to get the flags right, so far
> we haven't done that and I don't know what the usecase would be.
> 

I've came to a situation where it would have been helpful to know - bootloader
configured a pin as output and linux tried to configure the pin as IRQ input.
*YES I KNOW THIS IS NOT CORRECT* but it took some time to see, what happened.

> If we need that we should do it in gpiolib for all drivers don't you
> think?

A pragmatic / lean solution would be to deny the IRQ configuration when
seeing the pin configured as output in hardware and print out an error
on the gpio driver level.

> 
> But then we need a rationale for doing it, other than it's nice :-)
> It is already called on-the-fly by debugfs when a user needs that
> info.

See above, I think an error on a misconfigured system would be enough.
Maybe the documentation for gpio drivers should have a hint for driver
implementors.

So first step would be to convert the driver to the irqchip helper and then
add calls to gpiod_[un]lock_as_irq at the right places.

Will try to do that after my holiday.

> 
> Yours,
> Linus Walleij
> 
Yours,

Markus Niebel

  parent reply	other threads:[~2014-07-25 12:52 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-16 13:51 [[RFC PATCH]] gpio: gpio-mxc: make sure gpio is input when request IRQ Markus Niebel
2014-07-22  6:28 ` Shawn Guo
2014-07-22  7:19   ` Markus Niebel
     [not found]     ` <53CE107A.1000000-7U5DVgMjAv4@public.gmane.org>
2014-07-22  7:42       ` Shawn Guo
2014-07-22  7:42         ` Shawn Guo
2014-07-23 16:14         ` Linus Walleij
2014-07-23 16:14           ` Linus Walleij
2014-07-24  8:12           ` Markus Niebel
2014-07-24  8:12             ` Markus Niebel
     [not found]             ` <53D0C008.70305-7U5DVgMjAv4@public.gmane.org>
2014-07-25 11:38               ` Linus Walleij
2014-07-25 11:38                 ` Linus Walleij
     [not found]                 ` <CACRpkdarH_-2uW55C5bAUxM1ESgSb6tJAWc4UqEoWr8uc_kQRQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-07-25 12:52                   ` Markus Niebel [this message]
2014-07-25 12:52                     ` Markus Niebel
     [not found]                     ` <53D252F5.2060601-7U5DVgMjAv4@public.gmane.org>
2014-07-25 13:52                       ` Linus Walleij
2014-07-25 13:52                         ` Linus Walleij
2014-07-23 16:10 ` Linus Walleij

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=53D252F5.2060601@tqsc.de \
    --to=list-09_linux_arm-7u5dvgmjav4@public.gmane.org \
    --cc=Markus.Niebel-HVi9AzVXQk5Wk0Htik3J/w@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org \
    --cc=linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.