[v2] sparc: Add sparc support for platform_get_irq()

This adds sparc support for platform_get_irq that in the normal case use
platform_get_resource() to get an irq. This standard approach fails for sparc as
there are no resources of type IORESOURCE_IRQ for irqs for sparc.

Cross platform drivers can then use this standard platform function and work on
sparc instead of having to have a special case for sparc.

Signed-off-by: Andreas Larsson <andreas@gaisler.com>
---
 drivers/base/platform.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

On 2012-10-18 09:42, Andreas Larsson wrote:
> This adds sparc support for platform_get_irq that in the normal case use
> platform_get_resource() to get an irq. This standard approach fails for sparc as
> there are no resources of type IORESOURCE_IRQ for irqs for sparc.
>
> Cross platform drivers can then use this standard platform function and work on
> sparc instead of having to have a special case for sparc.

Any comments? Is this a good idea, or do anyone see any problems with 
this change?

It would make life easier when adapting device drivers to work on a 
sparc platform.

Cheers,
Andreas Larsson


> Signed-off-by: Andreas Larsson <andreas@gaisler.com>
> ---
>   drivers/base/platform.c |    6 ++++++
>   1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index ddeca14..a17199a 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -82,9 +82,15 @@ EXPORT_SYMBOL_GPL(platform_get_resource);
>    */
>   int platform_get_irq(struct platform_device *dev, unsigned int num)
>   {
> +#ifdef CONFIG_SPARC
> +	if (!dev || num >= dev->archdata.num_irqs)
> +		return -ENXIO;
> +	return dev->archdata.irqs[num];
> +#else
>   	struct resource *r = platform_get_resource(dev, IORESOURCE_IRQ, num);
>
>   	return r ? r->start : -ENXIO;
> +#endif
>   }
>   EXPORT_SYMBOL_GPL(platform_get_irq);
>

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

From: Andreas Larsson <andreas@gaisler.com>
Date: Tue, 23 Oct 2012 12:16:50 +0200

> On 2012-10-18 09:42, Andreas Larsson wrote:
>> This adds sparc support for platform_get_irq that in the normal case
>> use
>> platform_get_resource() to get an irq. This standard approach fails
>> for sparc as
>> there are no resources of type IORESOURCE_IRQ for irqs for sparc.
>>
>> Cross platform drivers can then use this standard platform function
>> and work on
>> sparc instead of having to have a special case for sparc.
> 
> Any comments? Is this a good idea, or do anyone see any problems with
> this change?
> 
> It would make life easier when adapting device drivers to work on a
> sparc platform.

I'm just busy and haven't gotten to review this change yet, be patient.
--
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

Hi Andreas.

On Tue, Oct 23, 2012 at 12:16:50PM +0200, Andreas Larsson wrote:
> On 2012-10-18 09:42, Andreas Larsson wrote:
>> This adds sparc support for platform_get_irq that in the normal case use
>> platform_get_resource() to get an irq. This standard approach fails for sparc as
>> there are no resources of type IORESOURCE_IRQ for irqs for sparc.
>>
>> Cross platform drivers can then use this standard platform function and work on
>> sparc instead of having to have a special case for sparc.
>
> Any comments? Is this a good idea, or do anyone see any problems with  
> this change?

First off - seeing a SPARC ifdef in base/drivers make me think
that something is wrong. And there is not even a comment why SPARC is so
special.

And secondly - what is need for SPARC to support IORESOURCE_IRQ?
I took a very quick look - and I did not understand all the
logic regarding IORESOURCE_IRQ.
But I could see linux/ioport.h being included in several places
in sparc..

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

From: Sam Ravnborg <sam@ravnborg.org>
Date: Wed, 24 Oct 2012 21:37:43 +0200

> First off - seeing a SPARC ifdef in base/drivers make me think
> that something is wrong. And there is not even a comment why SPARC is so
> special.

The problem is that, on sparc, we store the IRQs in the device
archdata area.  Whereas other platforms do this in a different
way, mostly via IORESOURCE_IRQ in the platform 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

From: David Miller:
> From: Sam Ravnborg <sam@ravnborg.org>
>> First off - seeing a SPARC ifdef in base/drivers make me think that
>> something is wrong. And there is not even a comment why SPARC is so
>> special.
>
> The problem is that, on sparc, we store the IRQs in the device
> archdata area. Whereas other platforms do this in a different
> way, mostly via IORESOURCE_IRQ in the platform device

I agree that there should be a comment in the code about the reason
for an #ifdef CONFIG_SPARC to be there.

 From the point of view of getting cross platform drivers working on
sparc, I think it would be very nice if irqs were available as
platform resources just as for so many other platforms. This however
is a larger change than getting platform_get_irq to work.

I have lookined into the alternative of providing IORESOURCE_IRQ for
sparc in parallel with the archdata representation, as a first
step. Then drivers that relies upon platform resources could work at
the same time as the code that relies on irqs being in archdata
continues to work. In arch/sparc some changes needs to be done to code
that assumes that all resources are "register" resources.

The problem with this approach however, is that there are a number of
existing drivers (and some other code) that rely on specific number of
resources for a device. All such code for devices that has irqs that
run in a sparc context needs to be changed to take the new irq
resources into account and care needs to be taken that code that is
not in a sparc context is not changed. It is not always totally
self-evident without further investigation which of the affected
drivers might run in a sparc context.

The other problem of providing irq resources for sparc is the flags
variable that comes with the resource. There seems to be no
corresponding information for sparc devices so that the flag variable
can be filled in properly.

Because of all this my proposition is to get platform_get_irq to
work on sparc. Then cross platform drivers can be made to work under
sparc in a smooth manner. As it is now, platform_get_irq is avalable
to call in sparc, but the call will always fail.

Cheers,
Andreas

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

From: Andreas Larsson <andreas@gaisler.com>
Date: Thu, 18 Oct 2012 09:42:44 +0200

> This adds sparc support for platform_get_irq that in the normal case use
> platform_get_resource() to get an irq. This standard approach fails for sparc as
> there are no resources of type IORESOURCE_IRQ for irqs for sparc.
> 
> Cross platform drivers can then use this standard platform function and work on
> sparc instead of having to have a special case for sparc.
> 
> Signed-off-by: Andreas Larsson <andreas@gaisler.com>

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