All of lore.kernel.org
 help / color / mirror / Atom feed
From: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
To: Gregory CLEMENT
	<gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Cc: "linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Jason Cooper <jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org>,
	Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>,
	Sebastian Hesselbarth
	<sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Thomas Petazzoni
	<thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>,
	"linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org"
	<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>,
	Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	"devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Nadav Haklai <nadavh-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>,
	Victor Gu <xigu-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>,
	Marcin Wojtas <mw-nYOzD4b6Jr9Wk0Htik3J/w@public.gmane.org>,
	Wilson Ding <dingwei-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>,
	Hua Jing <jinghua-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>,
	Neta Zur Hershkovits
	<neta-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
Subject: Re: [PATCH v2 5/7] pinctrl: aramda-37xx: Add irqchip support
Date: Wed, 29 Mar 2017 04:18:19 +0200	[thread overview]
Message-ID: <CACRpkdYA9xq3TmW-=0+cxdeBcq6v3HAFf3f5E-FAvUpGZqns3g@mail.gmail.com> (raw)
In-Reply-To: <87mvc5a3hh.fsf-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

On Tue, Mar 28, 2017 at 4:19 PM, Gregory CLEMENT
<gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>  On mar., mars 28 2017, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:

>> What you're doing is mocking around with core irqchip semantics.
>> Is ->mask really supposed to be played around with from the outsid
>> like this?
>
> According to the documentation mask is a "precomputed bitmask for
> accessing the chip registers" and it is exactly the way it is used in
> this driver.
>
> Moreover, currently ->mask is only used by the generic irqchip framework
> and not by the core of the irqchip subsystem. So the mask initialization
> is not done from the oustide but at the same level as the generic
> irqchip which is not used here.

OK excellent, sorry for my ignorance. We should rather point to your driver
as a good example of how to do this. Care to add some few lines of comment
as to what is going on for others that need to do the same?

Actually it would even be good to have something in
Documentation/gpio/driver.txt

>> It has irq_create_mapping(gpiochip->irqdomain, offset); that get
>> called for every IRQ, and that will eventually call irq_of_parse_and_map()
>> if the IRQs are defined in the device tree. (IIRC)
>
> When I followed the functions called I never find a call to
> irq_of_parse_and_map(), the closer things related to device tree I found
> was:
> "virq = irq_domain_alloc_descs(-1, 1, hwirq, of_node_to_nid(of_node),
> NULL);"
> http://elixir.free-electrons.com/source/kernel/irq/irqdomain.c?v=4.11-rc4#L507

I don't know if I'm rambling or what. I'm pretty sure it gets called, maybe
even earlier, like when the DT is parsed for the platform. We have so many
drivers not seemingly needing this, but if your driver needs it, all others
may need to be fixed too.

Can you put a print in irq_of_parse_and_map() and see what happens?

Yours,
Linus Walleij
--
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: Linus Walleij <linus.walleij@linaro.org>
To: Gregory CLEMENT <gregory.clement@free-electrons.com>
Cc: "linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
	Jason Cooper <jason@lakedaemon.net>, Andrew Lunn <andrew@lunn.ch>,
	Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>,
	Thomas Petazzoni <thomas.petazzoni@free-electrons.com>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	Rob Herring <robh+dt@kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Nadav Haklai <nadavh@marvell.com>, Victor Gu <xigu@marvell.com>,
	Marcin Wojtas <mw@semihalf.com>,
	Wilson Ding <dingwei@marvell.com>, Hua Jing <jinghua@marvell.com>,
	Neta Zur Hershkovits <neta@marvell.com>
Subject: Re: [PATCH v2 5/7] pinctrl: aramda-37xx: Add irqchip support
Date: Wed, 29 Mar 2017 04:18:19 +0200	[thread overview]
Message-ID: <CACRpkdYA9xq3TmW-=0+cxdeBcq6v3HAFf3f5E-FAvUpGZqns3g@mail.gmail.com> (raw)
In-Reply-To: <87mvc5a3hh.fsf@free-electrons.com>

On Tue, Mar 28, 2017 at 4:19 PM, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:
>  On mar., mars 28 2017, Linus Walleij <linus.walleij@linaro.org> wrote:

>> What you're doing is mocking around with core irqchip semantics.
>> Is ->mask really supposed to be played around with from the outsid
>> like this?
>
> According to the documentation mask is a "precomputed bitmask for
> accessing the chip registers" and it is exactly the way it is used in
> this driver.
>
> Moreover, currently ->mask is only used by the generic irqchip framework
> and not by the core of the irqchip subsystem. So the mask initialization
> is not done from the oustide but at the same level as the generic
> irqchip which is not used here.

OK excellent, sorry for my ignorance. We should rather point to your driver
as a good example of how to do this. Care to add some few lines of comment
as to what is going on for others that need to do the same?

Actually it would even be good to have something in
Documentation/gpio/driver.txt

>> It has irq_create_mapping(gpiochip->irqdomain, offset); that get
>> called for every IRQ, and that will eventually call irq_of_parse_and_map()
>> if the IRQs are defined in the device tree. (IIRC)
>
> When I followed the functions called I never find a call to
> irq_of_parse_and_map(), the closer things related to device tree I found
> was:
> "virq = irq_domain_alloc_descs(-1, 1, hwirq, of_node_to_nid(of_node),
> NULL);"
> http://elixir.free-electrons.com/source/kernel/irq/irqdomain.c?v=4.11-rc4#L507

I don't know if I'm rambling or what. I'm pretty sure it gets called, maybe
even earlier, like when the DT is parsed for the platform. We have so many
drivers not seemingly needing this, but if your driver needs it, all others
may need to be fixed too.

Can you put a print in irq_of_parse_and_map() and see what happens?

Yours,
Linus Walleij

WARNING: multiple messages have this Message-ID (diff)
From: linus.walleij@linaro.org (Linus Walleij)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 5/7] pinctrl: aramda-37xx: Add irqchip support
Date: Wed, 29 Mar 2017 04:18:19 +0200	[thread overview]
Message-ID: <CACRpkdYA9xq3TmW-=0+cxdeBcq6v3HAFf3f5E-FAvUpGZqns3g@mail.gmail.com> (raw)
In-Reply-To: <87mvc5a3hh.fsf@free-electrons.com>

On Tue, Mar 28, 2017 at 4:19 PM, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:
>  On mar., mars 28 2017, Linus Walleij <linus.walleij@linaro.org> wrote:

>> What you're doing is mocking around with core irqchip semantics.
>> Is ->mask really supposed to be played around with from the outsid
>> like this?
>
> According to the documentation mask is a "precomputed bitmask for
> accessing the chip registers" and it is exactly the way it is used in
> this driver.
>
> Moreover, currently ->mask is only used by the generic irqchip framework
> and not by the core of the irqchip subsystem. So the mask initialization
> is not done from the oustide but at the same level as the generic
> irqchip which is not used here.

OK excellent, sorry for my ignorance. We should rather point to your driver
as a good example of how to do this. Care to add some few lines of comment
as to what is going on for others that need to do the same?

Actually it would even be good to have something in
Documentation/gpio/driver.txt

>> It has irq_create_mapping(gpiochip->irqdomain, offset); that get
>> called for every IRQ, and that will eventually call irq_of_parse_and_map()
>> if the IRQs are defined in the device tree. (IIRC)
>
> When I followed the functions called I never find a call to
> irq_of_parse_and_map(), the closer things related to device tree I found
> was:
> "virq = irq_domain_alloc_descs(-1, 1, hwirq, of_node_to_nid(of_node),
> NULL);"
> http://elixir.free-electrons.com/source/kernel/irq/irqdomain.c?v=4.11-rc4#L507

I don't know if I'm rambling or what. I'm pretty sure it gets called, maybe
even earlier, like when the DT is parsed for the platform. We have so many
drivers not seemingly needing this, but if your driver needs it, all others
may need to be fixed too.

Can you put a print in irq_of_parse_and_map() and see what happens?

Yours,
Linus Walleij

  parent reply	other threads:[~2017-03-29  2:18 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-21 18:28 [PATCH v2 0/7] Hi, Gregory CLEMENT
2017-03-21 18:28 ` Gregory CLEMENT
2017-03-21 18:28 ` Gregory CLEMENT
2017-03-21 18:28 ` [PATCH v2 1/7] pinctrl: dt-bindings: Add documentation for Armada 37xx pin controllers Gregory CLEMENT
2017-03-21 18:28   ` Gregory CLEMENT
2017-03-21 18:28   ` Gregory CLEMENT
2017-03-27  9:18   ` Linus Walleij
2017-03-27  9:18     ` Linus Walleij
2017-03-27  9:18     ` Linus Walleij
     [not found]     ` <CACRpkdaP9Q=3ZeARTYizCKKtH6NdCrCnapxS7p=EdV1gvCZe8w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-03-28  9:35       ` Gregory CLEMENT
2017-03-28  9:35         ` Gregory CLEMENT
2017-03-28  9:35         ` Gregory CLEMENT
2017-03-21 18:28 ` [PATCH v2 2/7] arm64: marvell: enable the Armada 37xx pinctrl driver Gregory CLEMENT
2017-03-21 18:28   ` Gregory CLEMENT
2017-03-21 18:28   ` Gregory CLEMENT
2017-03-21 18:28 ` [PATCH v2 3/7] pinctrl: armada-37xx: Add pin controller support for Armada 37xx Gregory CLEMENT
2017-03-21 18:28   ` Gregory CLEMENT
2017-03-27  9:26   ` Linus Walleij
2017-03-27  9:26     ` Linus Walleij
2017-03-27  9:26     ` Linus Walleij
2017-03-28 10:43     ` Gregory CLEMENT
2017-03-28 10:43       ` Gregory CLEMENT
2017-03-28 10:43       ` Gregory CLEMENT
2017-03-21 18:28 ` [PATCH v2 4/7] pinctrl: armada-37xx: Add gpio support Gregory CLEMENT
2017-03-21 18:28   ` Gregory CLEMENT
2017-03-21 18:28   ` Gregory CLEMENT
2017-03-27  8:57   ` Linus Walleij
2017-03-27  8:57     ` Linus Walleij
2017-03-27  8:57     ` Linus Walleij
2017-03-27  9:46     ` Gregory CLEMENT
2017-03-27  9:46       ` Gregory CLEMENT
2017-03-27  9:46       ` Gregory CLEMENT
2017-03-21 18:28 ` [PATCH v2 5/7] pinctrl: aramda-37xx: Add irqchip support Gregory CLEMENT
2017-03-21 18:28   ` Gregory CLEMENT
2017-03-21 18:28   ` Gregory CLEMENT
2017-03-27  9:15   ` Linus Walleij
2017-03-27  9:15     ` Linus Walleij
2017-03-27  9:15     ` Linus Walleij
2017-03-28 10:36     ` Gregory CLEMENT
2017-03-28 10:36       ` Gregory CLEMENT
2017-03-28 10:36       ` Gregory CLEMENT
2017-03-28 13:04       ` Linus Walleij
2017-03-28 13:04         ` Linus Walleij
2017-03-28 13:04         ` Linus Walleij
2017-03-28 14:19         ` Gregory CLEMENT
2017-03-28 14:19           ` Gregory CLEMENT
2017-03-28 14:19           ` Gregory CLEMENT
     [not found]           ` <87mvc5a3hh.fsf-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2017-03-29  2:18             ` Linus Walleij [this message]
2017-03-29  2:18               ` Linus Walleij
2017-03-29  2:18               ` Linus Walleij
2017-03-30 16:57               ` Gregory CLEMENT
2017-03-30 16:57                 ` Gregory CLEMENT
2017-03-30 16:57                 ` Gregory CLEMENT
2017-03-21 18:28 ` [PATCH v2 6/7] ARM64: dts: marvell: Add pinctrl nodes for Armada 3700 Gregory CLEMENT
2017-03-21 18:28   ` Gregory CLEMENT
2017-03-21 18:28   ` Gregory CLEMENT
2017-03-21 18:28 ` [PATCH v2 7/7] ARM64: dts: marvell: armada37xx: add pinctrl definition Gregory CLEMENT
2017-03-21 18:28   ` Gregory CLEMENT
2017-03-21 18:28   ` Gregory CLEMENT
2017-03-21 20:50 ` [PATCH v2 0/7] Add support for pinctrl/gpio on Armada 37xx Was Re: [PATCH v2 0/7] Hi, Gregory CLEMENT
2017-03-21 20:50   ` Gregory CLEMENT
2017-03-22 11:40   ` Gregory CLEMENT
2017-03-22 11:40     ` Gregory CLEMENT

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='CACRpkdYA9xq3TmW-=0+cxdeBcq6v3HAFf3f5E-FAvUpGZqns3g@mail.gmail.com' \
    --to=linus.walleij-qsej5fyqhm4dnm+yrofe0a@public.gmane.org \
    --cc=andrew-g2DYL2Zd6BY@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=dingwei-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org \
    --cc=gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org \
    --cc=jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org \
    --cc=jinghua-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mw-nYOzD4b6Jr9Wk0Htik3J/w@public.gmane.org \
    --cc=nadavh-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org \
    --cc=neta-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org \
    --cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org \
    --cc=xigu-eYqpPyKDWXRBDgjK7y7TUQ@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.