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 <[email protected]>
---
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
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 <[email protected]>
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 <[email protected]>
> ---
>
> 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
>
Hi David,
2017-09-06 11:09 GMT+09:00 David Daney <[email protected]>:
> 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 <[email protected]>
>
>
> 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 <[email protected]>
BTW, I could not understand your intention of
(64BIT && COMPILE_TEST)
Why can COMPILE_TEST be enabled when 64BIT?
--
Best Regards
Masahiro Yamada
On 09/05/2017 09:20 PM, Masahiro Yamada wrote:
> Hi David,
>
>
> 2017-09-06 11:09 GMT+09:00 David Daney <[email protected]>:
>> 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 <[email protected]>
>>
>>
>> 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 <[email protected]>
>
>
> 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
Hi David,
2017-09-07 2:36 GMT+09:00 David Daney <[email protected]>:
> On 09/05/2017 09:20 PM, Masahiro Yamada wrote:
>>
>> Hi David,
>>
>>
>> 2017-09-06 11:09 GMT+09:00 David Daney <[email protected]>:
>>>
>>> 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 <[email protected]>
>>>
>>>
>>>
>>> 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 <[email protected]>
>>
>>
>>
>> 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
On 09/06/2017 06:03 PM, Masahiro Yamada wrote:
> Hi David,
>
>
> 2017-09-07 2:36 GMT+09:00 David Daney <[email protected]>:
>> On 09/05/2017 09:20 PM, Masahiro Yamada wrote:
>>>
>>> Hi David,
>>>
>>>
>>> 2017-09-06 11:09 GMT+09:00 David Daney <[email protected]>:
>>>>
>>>> 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 <[email protected]>
>>>>
>>>>
>>>>
>>>> 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 <[email protected]>
>>>
>>>
>>>
>>> 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)
>
>
>
>
On Wed, Sep 6, 2017 at 3:40 AM, Masahiro Yamada
<[email protected]> 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 <[email protected]>
Patch applied with David's ACK.
Yours,
Linus Walleij