| Message ID | 1504662052-12478-1-git-send-email-yamada.masahiro@socionext.com |
|---|---|
| State | Accepted |
| Commit | 311de3ce96f453a5a6edb3066eb6109cc07e81d2 |
| Headers | show |
| Series | gpio: thunderx: select IRQ_DOMAIN_HIERARCHY instead of depends on | expand |
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 > -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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) > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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(-) -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html