Patchwork SPARC and OF_GPIO

login
register
mail settings
Submitter David Miller
Date Nov. 6, 2012, 11:40 p.m.
Message ID <20121106.184058.636480855516538607.davem@davemloft.net>
Download mbox | patch
Permalink /patch/197582/
State RFC
Delegated to: David Miller
Headers show

Comments

David Miller - Nov. 6, 2012, 11:40 p.m.
From: Thierry Reding <thierry.reding@avionic-design.de>
Date: Mon, 5 Nov 2012 10:53:15 +0100

> 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.

> 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.

--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thierry Reding - Nov. 7, 2012, 6:52 a.m.
On Tue, Nov 06, 2012 at 06:40:58PM -0500, David Miller wrote:
> From: Thierry Reding <thierry.reding@avionic-design.de>
> Date: Mon, 5 Nov 2012 10:53:15 +0100
> 
> > 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.

> > 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

> 
> diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
> index b6b442b..f0a5391 100644
> --- a/arch/sparc/Kconfig
> +++ b/arch/sparc/Kconfig
> @@ -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
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index d055cee..f11d8e3 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -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"
>
David Miller - Nov. 7, 2012, 7:34 a.m.
From: Thierry Reding <thierry.reding@avionic-design.de>
Date: Wed, 7 Nov 2012 07:52:58 +0100

> 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.
 ...
> 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.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Grant Likely - Nov. 30, 2012, 9:35 a.m.
On Wed, 07 Nov 2012 02:34:19 -0500 (EST), David Miller <davem@davemloft.net> wrote:
> From: Thierry Reding <thierry.reding@avionic-design.de>
> Date: Wed, 7 Nov 2012 07:52:58 +0100
> 
> > 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.
>  ...
> > 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.

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.
Thierry Reding - Nov. 30, 2012, 9:40 a.m.
On Fri, Nov 30, 2012 at 09:35:20AM +0000, Grant Likely wrote:
> On Wed, 07 Nov 2012 02:34:19 -0500 (EST), David Miller <davem@davemloft.net> wrote:
> > From: Thierry Reding <thierry.reding@avionic-design.de>
> > Date: Wed, 7 Nov 2012 07:52:58 +0100
> > 
> > > 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.
> >  ...
> > > 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.
> 
> 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.

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
Grant Likely - Nov. 30, 2012, 9:58 a.m.
On Fri, Nov 30, 2012 at 9:40 AM, Thierry Reding
<thierry.reding@avionic-design.de> wrote:
> On Fri, Nov 30, 2012 at 09:35:20AM +0000, Grant Likely wrote:
>> On Wed, 07 Nov 2012 02:34:19 -0500 (EST), David Miller <davem@davemloft.net> wrote:
>> > From: Thierry Reding <thierry.reding@avionic-design.de>
>> > Date: Wed, 7 Nov 2012 07:52:58 +0100
>> >
>> > > 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.
>> >  ...
>> > > 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.
>>
>> 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.
>
> 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.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - Nov. 30, 2012, 4:46 p.m.
From: Grant Likely <grant.likely@secretlab.ca>
Date: Fri, 30 Nov 2012 09:35:20 +0000

> 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.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index b6b442b..f0a5391 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -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
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index d055cee..f11d8e3 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -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"