Skip to contentSkip to conversation

SPARC and OF_GPIO

DISCUSSION Thierry Reding 3 participants 8 messages Nov 5, 2012 – Nov 30, 2012

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

[resending with David's correct email address]

Hi David,

There have been a number of reports that Linux kernel builds fail on SPARC because it doesn't support OF_GPIO, which provides the of_node field of the struct gpio_chip.

One of the drivers I wrote (gpio-adnp) accesses this unconditionally but only depends on OF and not OF_GPIO, so it fails to build on SPARC. A similar problem happens with the gpio-fan driver, which defines the OF match table only if OF_GPIO is selected, but uses it even if only OF but not OF_GPIO is selected.

While it is clearly the drivers which are at fault here it still raises the question as to why SPARC still conflicts with OF_GPIO. Over two years ago, commit 5ab5fc7 made most of the OF symbols available to all platforms except SPARC. The commit message explicitly states that this should probably be re-evaluated.

Are you aware of any reasons why this conflict would still be necessary? This is not only the case for OF_GPIO but likely also for OF_SPI, OF_I2C, OF_IRQ and OF_ADDRESS. Shouldn't those all work even on SPARC nowadays?

Thierry

maintainer
#1 lore raw

From: Thierry Reding <[email protected]>
Date: Mon, 5 Nov 2012 10:53:15 +0100

Quoted 1 line

Are you aware of any reasons why this conflict would still be necessary?

No reason that I can see, I'll push something like the patch below via the sparc tree.

Quoted 3 lines

This is not only the case for OF_GPIO but likely also for OF_SPI, OF_I2C, OF_IRQ and OF_ADDRESS. Shouldn't those all work even on SPARC nowadays?

Those also would need to be tested on an individual basis, but there are no fundamental problems that I am aware of.

arch/sparc/Kconfig +1 -0
index b6b442b..f0a5391 100644
@@ -14,6 +14,7 @@ config SPARC
 	default y
 	select OF
 	select OF_PROMTREE
+	select OF_GPIO
 	select HAVE_IDE
 	select HAVE_OPROFILE
 	select HAVE_ARCH_KGDB if !SMP || SPARC64
drivers/gpio/Kconfig +1 -1
index d055cee..f11d8e3 100644
@@ -47,7 +47,7 @@ if GPIOLIB
 
 config OF_GPIO
 	def_bool y
-	depends on OF && !SPARC
+	depends on OF
 
 config DEBUG_GPIO
 	bool "Debug GPIO calls"
#2 lore raw
Quoted 7 lines · David Miller

From: Thierry Reding <[email protected]>
Date: Mon, 5 Nov 2012 10:53:15 +0100

Quoted 1 line

Are you aware of any reasons why this conflict would still be necessary?

No reason that I can see, I'll push something like the patch below via the sparc tree.

Thanks for doing this.

Quoted 6 lines
Quoted 3 lines

This is not only the case for OF_GPIO but likely also for OF_SPI, OF_I2C, OF_IRQ and OF_ADDRESS. Shouldn't those all work even on SPARC nowadays?

Those also would need to be tested on an individual basis, but there are no fundamental problems that I am aware of.

It seems like OF_ADDRESS would be trickier. A comment around line 60 in drivers/of/platform.c says that SPARC doesn't need functions defined in the enclosing #ifdef CONFIG_OF_ADDRESS block. I'm not sure it would be acceptable to remove the conflict nonetheless, even if the functions aren't used. One benefit would be that the code could receive some extra compile coverage.

Oddly I'm no longer able to find any reference to OF_SPI, so maybe I just made that up...

The code conditionalized on OF_I2C looks very generic, so I think there shouldn't be a problem to remove that conflict either.

Finally, OF_IRQ is again just generic code to map device tree data to IRQ domains. While I didn't see the IRQ_DOMAIN symbol selected anywhere in SPARC it should still be possible to run drivers that properly implement IRQ domains on SPARC, right? Or is there any reason why they wouldn't work?

So this seems like all conflicts except the one for OF_ADDRESS can easily be removed. And even for OF_ADDRESS there may be some value in removing the conflict.

Thierry

Quoted 27 lines
arch/sparc/Kconfig +1 -0
index b6b442b..f0a5391 100644
@@ -14,6 +14,7 @@ config SPARC
 	default y
 	select OF
 	select OF_PROMTREE
+	select OF_GPIO
 	select HAVE_IDE
 	select HAVE_OPROFILE
 	select HAVE_ARCH_KGDB if !SMP || SPARC64
drivers/gpio/Kconfig +1 -1
index d055cee..f11d8e3 100644
@@ -47,7 +47,7 @@ if GPIOLIB
 
 config OF_GPIO
 	def_bool y
-	depends on OF && !SPARC
+	depends on OF
 
 config DEBUG_GPIO
 	bool "Debug GPIO calls"
maintainer
#3 lore raw

From: Thierry Reding <[email protected]>
Date: Wed, 7 Nov 2012 07:52:58 +0100

Quoted 6 lines

It seems like OF_ADDRESS would be trickier. A comment around line 60 in drivers/of/platform.c says that SPARC doesn't need functions defined in the enclosing #ifdef CONFIG_OF_ADDRESS block. I'm not sure it would be acceptable to remove the conflict nonetheless, even if the functions aren't used. One benefit would be that the code could receive some extra compile coverage.

...

Quoted 5 lines

Finally, OF_IRQ is again just generic code to map device tree data to IRQ domains. While I didn't see the IRQ_DOMAIN symbol selected anywhere in SPARC it should still be possible to run drivers that properly implement IRQ domains on SPARC, right? Or is there any reason why they wouldn't work?

These are the two most conflicted areas for Sparc.

For addresses, we fully compute the full fully resolved physical address of all registers of an OF device very early at bootup time when we first scan the device tree.

Same goes for interrupts, we fully compute them early in the bootup process. Also, we support multiple interrupts for a device.

#4 lore raw
Quoted 16 lines · David Miller

From: Thierry Reding <[email protected]>
Date: Wed, 7 Nov 2012 07:52:58 +0100

Quoted 6 lines

It seems like OF_ADDRESS would be trickier. A comment around line 60 in drivers/of/platform.c says that SPARC doesn't need functions defined in the enclosing #ifdef CONFIG_OF_ADDRESS block. I'm not sure it would be acceptable to remove the conflict nonetheless, even if the functions aren't used. One benefit would be that the code could receive some extra compile coverage.

...

Quoted 5 lines

Finally, OF_IRQ is again just generic code to map device tree data to IRQ domains. While I didn't see the IRQ_DOMAIN symbol selected anywhere in SPARC it should still be possible to run drivers that properly implement IRQ domains on SPARC, right? Or is there any reason why they wouldn't work?

Quoted 8 lines

These are the two most conflicted areas for Sparc.

For addresses, we fully compute the full fully resolved physical address of all registers of an OF device very early at bootup time when we first scan the device tree.

Same goes for interrupts, we fully compute them early in the bootup process.

Right. That's the reason I haven't tackled making all architectures do the same thing. I've not been confident that I'd get the sparc bits correct. I think it could be done, but I haven't been able to wrap my brain around it sufficiently.

On non-sparc I've actually been moving in the direction of resolving resources at .probe time to make it easier to handle deferred probing. So if, for example, a device irq line is routed to a GPIO instead of the core interrupt controller, then the irq number won't be known until after the gpio driver .probe occurs. For addresses, this situation is unlikely, but for all the other kinds of resources (gpios, regs, clocks, irqs, etc) it is a problem that we're actually seeing.

g.

Signature · 2 lines
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.
#5 lore raw
Quoted 31 lines · Grant Likely
Quoted 16 lines · David Miller

From: Thierry Reding <[email protected]>
Date: Wed, 7 Nov 2012 07:52:58 +0100

Quoted 6 lines

It seems like OF_ADDRESS would be trickier. A comment around line 60 in drivers/of/platform.c says that SPARC doesn't need functions defined in the enclosing #ifdef CONFIG_OF_ADDRESS block. I'm not sure it would be acceptable to remove the conflict nonetheless, even if the functions aren't used. One benefit would be that the code could receive some extra compile coverage.

...

Quoted 5 lines

Finally, OF_IRQ is again just generic code to map device tree data to IRQ domains. While I didn't see the IRQ_DOMAIN symbol selected anywhere in SPARC it should still be possible to run drivers that properly implement IRQ domains on SPARC, right? Or is there any reason why they wouldn't work?

Quoted 8 lines

These are the two most conflicted areas for Sparc.

For addresses, we fully compute the full fully resolved physical address of all registers of an OF device very early at bootup time when we first scan the device tree.

Same goes for interrupts, we fully compute them early in the bootup process.

Right. That's the reason I haven't tackled making all architectures do the same thing. I've not been confident that I'd get the sparc bits correct. I think it could be done, but I haven't been able to wrap my brain around it sufficiently.

Quoted 7 lines

On non-sparc I've actually been moving in the direction of resolving resources at .probe time to make it easier to handle deferred probing. So if, for example, a device irq line is routed to a GPIO instead of the core interrupt controller, then the irq number won't be known until after the gpio driver .probe occurs. For addresses, this situation is unlikely, but for all the other kinds of resources (gpios, regs, clocks, irqs, etc) it is a problem that we're actually seeing.

Interesting. I have some I2C devices that run into the problem where their interrupts cannot be resolved at instantiation time so I've had to work around it by calling irq_of_parse_and_map() at .probe() time and return -EPROBE_DEFER if that return NO_IRQ.

Are any of your plans documented somewhere? I'd be interested to know how this is supposed to be solved. irq_of_parse_and_map() is not going to work for non-DT setups so the above can't be a proper solution.

Thierry

#6 lore raw
Quoted 40 lines · Thierry Reding
Quoted 31 lines · Grant Likely
Quoted 16 lines · David Miller

From: Thierry Reding <[email protected]>
Date: Wed, 7 Nov 2012 07:52:58 +0100

Quoted 6 lines

It seems like OF_ADDRESS would be trickier. A comment around line 60 in drivers/of/platform.c says that SPARC doesn't need functions defined in the enclosing #ifdef CONFIG_OF_ADDRESS block. I'm not sure it would be acceptable to remove the conflict nonetheless, even if the functions aren't used. One benefit would be that the code could receive some extra compile coverage.

...

Quoted 5 lines

Finally, OF_IRQ is again just generic code to map device tree data to IRQ domains. While I didn't see the IRQ_DOMAIN symbol selected anywhere in SPARC it should still be possible to run drivers that properly implement IRQ domains on SPARC, right? Or is there any reason why they wouldn't work?

Quoted 8 lines

These are the two most conflicted areas for Sparc.

For addresses, we fully compute the full fully resolved physical address of all registers of an OF device very early at bootup time when we first scan the device tree.

Same goes for interrupts, we fully compute them early in the bootup process.

Right. That's the reason I haven't tackled making all architectures do the same thing. I've not been confident that I'd get the sparc bits correct. I think it could be done, but I haven't been able to wrap my brain around it sufficiently.

Quoted 7 lines

On non-sparc I've actually been moving in the direction of resolving resources at .probe time to make it easier to handle deferred probing. So if, for example, a device irq line is routed to a GPIO instead of the core interrupt controller, then the irq number won't be known until after the gpio driver .probe occurs. For addresses, this situation is unlikely, but for all the other kinds of resources (gpios, regs, clocks, irqs, etc) it is a problem that we're actually seeing.

Quoted 8 lines

Interesting. I have some I2C devices that run into the problem where their interrupts cannot be resolved at instantiation time so I've had to work around it by calling irq_of_parse_and_map() at .probe() time and return -EPROBE_DEFER if that return NO_IRQ.

Are any of your plans documented somewhere? I'd be interested to know how this is supposed to be solved. irq_of_parse_and_map() is not going to work for non-DT setups so the above can't be a proper solution.

I think there should be generic helpers for retrieving each type of resource and each data source (resource table, DT, ACPI, etc) should plug into that infrastructure. Those functions already exist for the platform bus type, but I've not gotten to the step of plugging in the decode helpers.

g.

maintainer
#7 lore raw
In reply to Grant Likely

From: Grant Likely <[email protected]>
Date: Fri, 30 Nov 2012 09:35:20 +0000

Quoted 7 lines

On non-sparc I've actually been moving in the direction of resolving resources at .probe time to make it easier to handle deferred probing. So if, for example, a device irq line is routed to a GPIO instead of the core interrupt controller, then the irq number won't be known until after the gpio driver .probe occurs. For addresses, this situation is unlikely, but for all the other kinds of resources (gpios, regs, clocks, irqs, etc) it is a problem that we're actually seeing.

Every interrupt in the device tree is resolvable with, at worst, very small bus drivers, and that's what we pack into the generic sparc OF device creation layer.

Actually much of it is generic and not bus type specific at all, and is a simply mask and match into an interrupt routing table property.