* [PATCH] gpio: thunderx: select IRQ_DOMAIN_HIERARCHY instead of depends on @ 2017-09-06 1:40 Masahiro Yamada 2017-09-06 2:09 ` David Daney 2017-09-12 9:24 ` Linus Walleij 0 siblings, 2 replies; 7+ messages in thread From: Masahiro Yamada @ 2017-09-06 1:40 UTC (permalink / raw) To: linux-gpio; +Cc: David Daney, Masahiro Yamada, Linus Walleij, linux-kernel IRQ_DOMAIN_HIERARCHY is not user-configurable, but supposed to be selected by drivers that need IRQ domain hierarchy support. GPIO_THUNDERX is the only user of "depends on IRQ_DOMAIN_HIERARCHY". This means, we can not enable GPIO_THUNDERX unless other drivers select IRQ_DOMAIN_HIERARCHY elsewhere. This is odd. Flip the logic. Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> --- drivers/gpio/Kconfig | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 3388d54..3f80f16 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -453,7 +453,8 @@ config GPIO_TS4800 config GPIO_THUNDERX tristate "Cavium ThunderX/OCTEON-TX GPIO" depends on ARCH_THUNDER || (64BIT && COMPILE_TEST) - depends on PCI_MSI && IRQ_DOMAIN_HIERARCHY + depends on PCI_MSI + select IRQ_DOMAIN_HIERARCHY select IRQ_FASTEOI_HIERARCHY_HANDLERS help Say yes here to support the on-chip GPIO lines on the ThunderX -- 2.7.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] gpio: thunderx: select IRQ_DOMAIN_HIERARCHY instead of depends on 2017-09-06 1:40 [PATCH] gpio: thunderx: select IRQ_DOMAIN_HIERARCHY instead of depends on Masahiro Yamada @ 2017-09-06 2:09 ` David Daney 2017-09-06 4:20 ` Masahiro Yamada 2017-09-12 9:24 ` Linus Walleij 1 sibling, 1 reply; 7+ messages in thread From: David Daney @ 2017-09-06 2:09 UTC (permalink / raw) To: Masahiro Yamada, linux-gpio; +Cc: David Daney, Linus Walleij, linux-kernel On 09/05/2017 06:40 PM, Masahiro Yamada wrote: > IRQ_DOMAIN_HIERARCHY is not user-configurable, but supposed to be > selected by drivers that need IRQ domain hierarchy support. > > GPIO_THUNDERX is the only user of "depends on IRQ_DOMAIN_HIERARCHY". > This means, we can not enable GPIO_THUNDERX unless other drivers > select IRQ_DOMAIN_HIERARCHY elsewhere. This is odd. Flip the logic. > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> IRQ_DOMAIN_HIERARCHY is set as a result of ARCH_THUNDER (this SoC hardware), so it actually works as-is. That said, this looks like a reasonable improvement, and will allow the COMPILE_TEST to enable it, so... Acked-by: David Daney <david.daney@cavium.com> > --- > > drivers/gpio/Kconfig | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig > index 3388d54..3f80f16 100644 > --- a/drivers/gpio/Kconfig > +++ b/drivers/gpio/Kconfig > @@ -453,7 +453,8 @@ config GPIO_TS4800 > config GPIO_THUNDERX > tristate "Cavium ThunderX/OCTEON-TX GPIO" > depends on ARCH_THUNDER || (64BIT && COMPILE_TEST) > - depends on PCI_MSI && IRQ_DOMAIN_HIERARCHY > + depends on PCI_MSI > + select IRQ_DOMAIN_HIERARCHY > select IRQ_FASTEOI_HIERARCHY_HANDLERS > help > Say yes here to support the on-chip GPIO lines on the ThunderX > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] gpio: thunderx: select IRQ_DOMAIN_HIERARCHY instead of depends on 2017-09-06 2:09 ` David Daney @ 2017-09-06 4:20 ` Masahiro Yamada 2017-09-06 17:36 ` David Daney 0 siblings, 1 reply; 7+ messages in thread From: Masahiro Yamada @ 2017-09-06 4:20 UTC (permalink / raw) To: David Daney Cc: linux-gpio, David Daney, Linus Walleij, Linux Kernel Mailing List Hi David, 2017-09-06 11:09 GMT+09:00 David Daney <ddaney@caviumnetworks.com>: > On 09/05/2017 06:40 PM, Masahiro Yamada wrote: >> >> IRQ_DOMAIN_HIERARCHY is not user-configurable, but supposed to be >> selected by drivers that need IRQ domain hierarchy support. >> >> GPIO_THUNDERX is the only user of "depends on IRQ_DOMAIN_HIERARCHY". >> This means, we can not enable GPIO_THUNDERX unless other drivers >> select IRQ_DOMAIN_HIERARCHY elsewhere. This is odd. Flip the logic. >> >> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > > > IRQ_DOMAIN_HIERARCHY is set as a result of ARCH_THUNDER (this SoC hardware), > so it actually works as-is. Right, ARCH_THUNDER does not select it directly, but does it indirectly. (this is not so clear...) ARCH_THUNDER -> ARM64 -> ARM_GIC -> IRQ_DOMAIN_HIERARCHY > That said, this looks like a reasonable > improvement, and will allow the COMPILE_TEST to enable it, so... > > Acked-by: David Daney <david.daney@cavium.com> BTW, I could not understand your intention of (64BIT && COMPILE_TEST) Why can COMPILE_TEST be enabled when 64BIT? -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] gpio: thunderx: select IRQ_DOMAIN_HIERARCHY instead of depends on 2017-09-06 4:20 ` Masahiro Yamada @ 2017-09-06 17:36 ` David Daney 2017-09-07 1:03 ` Masahiro Yamada 0 siblings, 1 reply; 7+ messages in thread From: David Daney @ 2017-09-06 17:36 UTC (permalink / raw) To: Masahiro Yamada, David Daney Cc: linux-gpio, David Daney, Linus Walleij, Linux Kernel Mailing List On 09/05/2017 09:20 PM, Masahiro Yamada wrote: > Hi David, > > > 2017-09-06 11:09 GMT+09:00 David Daney <ddaney@caviumnetworks.com>: >> On 09/05/2017 06:40 PM, Masahiro Yamada wrote: >>> >>> IRQ_DOMAIN_HIERARCHY is not user-configurable, but supposed to be >>> selected by drivers that need IRQ domain hierarchy support. >>> >>> GPIO_THUNDERX is the only user of "depends on IRQ_DOMAIN_HIERARCHY". >>> This means, we can not enable GPIO_THUNDERX unless other drivers >>> select IRQ_DOMAIN_HIERARCHY elsewhere. This is odd. Flip the logic. >>> >>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> >> >> >> IRQ_DOMAIN_HIERARCHY is set as a result of ARCH_THUNDER (this SoC hardware), >> so it actually works as-is. > > > Right, ARCH_THUNDER does not select it directly, > but does it indirectly. (this is not so clear...) > > ARCH_THUNDER -> ARM64 -> ARM_GIC -> IRQ_DOMAIN_HIERARCHY > > > >> That said, this looks like a reasonable >> improvement, and will allow the COMPILE_TEST to enable it, so... >> >> Acked-by: David Daney <david.daney@cavium.com> > > > BTW, I could not understand your intention of > (64BIT && COMPILE_TEST) > The driver uses readq()/writeq(), which are not available in some 32BIT kernels. So to ensure that it can build without error we depend on 64BIT as a proxy for the availability of readq()/writeq() David ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] gpio: thunderx: select IRQ_DOMAIN_HIERARCHY instead of depends on 2017-09-06 17:36 ` David Daney @ 2017-09-07 1:03 ` Masahiro Yamada 2017-09-07 1:18 ` David Daney 0 siblings, 1 reply; 7+ messages in thread From: Masahiro Yamada @ 2017-09-07 1:03 UTC (permalink / raw) To: David Daney Cc: linux-gpio, David Daney, Linus Walleij, Linux Kernel Mailing List Hi David, 2017-09-07 2:36 GMT+09:00 David Daney <ddaney@caviumnetworks.com>: > On 09/05/2017 09:20 PM, Masahiro Yamada wrote: >> >> Hi David, >> >> >> 2017-09-06 11:09 GMT+09:00 David Daney <ddaney@caviumnetworks.com>: >>> >>> On 09/05/2017 06:40 PM, Masahiro Yamada wrote: >>>> >>>> >>>> IRQ_DOMAIN_HIERARCHY is not user-configurable, but supposed to be >>>> selected by drivers that need IRQ domain hierarchy support. >>>> >>>> GPIO_THUNDERX is the only user of "depends on IRQ_DOMAIN_HIERARCHY". >>>> This means, we can not enable GPIO_THUNDERX unless other drivers >>>> select IRQ_DOMAIN_HIERARCHY elsewhere. This is odd. Flip the logic. >>>> >>>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> >>> >>> >>> >>> IRQ_DOMAIN_HIERARCHY is set as a result of ARCH_THUNDER (this SoC >>> hardware), >>> so it actually works as-is. >> >> >> >> Right, ARCH_THUNDER does not select it directly, >> but does it indirectly. (this is not so clear...) >> >> ARCH_THUNDER -> ARM64 -> ARM_GIC -> IRQ_DOMAIN_HIERARCHY >> >> >> >>> That said, this looks like a reasonable >>> improvement, and will allow the COMPILE_TEST to enable it, so... >>> >>> Acked-by: David Daney <david.daney@cavium.com> >> >> >> >> BTW, I could not understand your intention of >> (64BIT && COMPILE_TEST) >> > > The driver uses readq()/writeq(), which are not available in some 32BIT > kernels. So to ensure that it can build without error we depend on 64BIT as > a proxy for the availability of readq()/writeq() > > IMHO, drivers code should not depend on CPU architecture too much. Does the following make sense for your driver? - split {read,write}q into two transactions of {read,write}l or - include <linux/io-64-nonatomic-hi-lo.h> or <linux-io-64-nonatomic-lo-hi.h> (choose a suitable one for your driver) -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] gpio: thunderx: select IRQ_DOMAIN_HIERARCHY instead of depends on 2017-09-07 1:03 ` Masahiro Yamada @ 2017-09-07 1:18 ` David Daney 0 siblings, 0 replies; 7+ messages in thread From: David Daney @ 2017-09-07 1:18 UTC (permalink / raw) To: Masahiro Yamada Cc: linux-gpio, David Daney, Linus Walleij, Linux Kernel Mailing List On 09/06/2017 06:03 PM, Masahiro Yamada wrote: > Hi David, > > > 2017-09-07 2:36 GMT+09:00 David Daney <ddaney@caviumnetworks.com>: >> On 09/05/2017 09:20 PM, Masahiro Yamada wrote: >>> >>> Hi David, >>> >>> >>> 2017-09-06 11:09 GMT+09:00 David Daney <ddaney@caviumnetworks.com>: >>>> >>>> On 09/05/2017 06:40 PM, Masahiro Yamada wrote: >>>>> >>>>> >>>>> IRQ_DOMAIN_HIERARCHY is not user-configurable, but supposed to be >>>>> selected by drivers that need IRQ domain hierarchy support. >>>>> >>>>> GPIO_THUNDERX is the only user of "depends on IRQ_DOMAIN_HIERARCHY". >>>>> This means, we can not enable GPIO_THUNDERX unless other drivers >>>>> select IRQ_DOMAIN_HIERARCHY elsewhere. This is odd. Flip the logic. >>>>> >>>>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> >>>> >>>> >>>> >>>> IRQ_DOMAIN_HIERARCHY is set as a result of ARCH_THUNDER (this SoC >>>> hardware), >>>> so it actually works as-is. >>> >>> >>> >>> Right, ARCH_THUNDER does not select it directly, >>> but does it indirectly. (this is not so clear...) >>> >>> ARCH_THUNDER -> ARM64 -> ARM_GIC -> IRQ_DOMAIN_HIERARCHY >>> >>> >>> >>>> That said, this looks like a reasonable >>>> improvement, and will allow the COMPILE_TEST to enable it, so... >>>> >>>> Acked-by: David Daney <david.daney@cavium.com> >>> >>> >>> >>> BTW, I could not understand your intention of >>> (64BIT && COMPILE_TEST) >>> >> >> The driver uses readq()/writeq(), which are not available in some 32BIT >> kernels. So to ensure that it can build without error we depend on 64BIT as >> a proxy for the availability of readq()/writeq() >> >> > > > IMHO, drivers code should not depend on CPU architecture too much. > The CPU and the device are in a SoC (both on the same silicon die), there is no way they can be used apart from one another. > > Does the following make sense for your driver? > > > - split {read,write}q into two transactions of {read,write}l No. The registers must be accessed with 64-bit operations. We are not going to screw around complicating the driver so that it can work on hypothetical non-existent systems. If we ever put this GPIO hardware unit in a system with a 32-bit CPU, we will at that time, and only then, modify it to work with 32-bit operations. > > or > > - include <linux/io-64-nonatomic-hi-lo.h> or > <linux-io-64-nonatomic-lo-hi.h> > (choose a suitable one for your driver) > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] gpio: thunderx: select IRQ_DOMAIN_HIERARCHY instead of depends on 2017-09-06 1:40 [PATCH] gpio: thunderx: select IRQ_DOMAIN_HIERARCHY instead of depends on Masahiro Yamada 2017-09-06 2:09 ` David Daney @ 2017-09-12 9:24 ` Linus Walleij 1 sibling, 0 replies; 7+ messages in thread From: Linus Walleij @ 2017-09-12 9:24 UTC (permalink / raw) To: Masahiro Yamada Cc: linux-gpio@vger.kernel.org, David Daney, linux-kernel@vger.kernel.org On Wed, Sep 6, 2017 at 3:40 AM, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > IRQ_DOMAIN_HIERARCHY is not user-configurable, but supposed to be > selected by drivers that need IRQ domain hierarchy support. > > GPIO_THUNDERX is the only user of "depends on IRQ_DOMAIN_HIERARCHY". > This means, we can not enable GPIO_THUNDERX unless other drivers > select IRQ_DOMAIN_HIERARCHY elsewhere. This is odd. Flip the logic. > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> Patch applied with David's ACK. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-09-12 9:24 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-09-06 1:40 [PATCH] gpio: thunderx: select IRQ_DOMAIN_HIERARCHY instead of depends on Masahiro Yamada 2017-09-06 2:09 ` David Daney 2017-09-06 4:20 ` Masahiro Yamada 2017-09-06 17:36 ` David Daney 2017-09-07 1:03 ` Masahiro Yamada 2017-09-07 1:18 ` David Daney 2017-09-12 9:24 ` Linus Walleij
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).