On Fri, Aug 19, 2011 at 10:10 AM, Simon Horman <horms@xxxxxxxxxxxx> wrote:
> This is intended to make it easier to correctly order IRQs.
>
> As suggested by Guennadi Liakhovetski.
> --- a/arch/arm/mach-shmobile/board-ag5evm.c
> +++ b/arch/arm/mach-shmobile/board-ag5evm.c
> @@ -352,15 +352,15 @@ static struct resource sdhi0_resources[] = {
> .end = 0xee1000ff,
> .flags = IORESOURCE_MEM,
> },
> - [1] = {
> + [1 + SH_MOBILE_SDHI_IRQ_CARD_DETECT] = {
> .start = gic_spi(83),
> .flags = IORESOURCE_IRQ,
> },
> - [2] = {
> + [1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
> .start = gic_spi(84),
> .flags = IORESOURCE_IRQ,
> },
> - [3] = {
> + [1 + SH_MOBILE_SDHI_IRQ_SDIO] = {
> .start = gic_spi(85),
> .flags = IORESOURCE_IRQ,
> },
Hm...
So I know you guys have been discussing this back and forth, so I'm
not sure if jumping in to this thread makes things any better. But
anyhow, here are my opinions. Feel free to ignore them. =)
First of all, having some kind of association with each IRQ is a good
thing. I am however not convinced that using the index number of the
platform device resource irq is the best option. Consider the case
when someone modifies the SDHI resource in the code above to only
include this interrupt:
[1 + SH_MOBILE_SDHI_IRQ_SDCARD] = {
.start = gic_spi(84),
.flags = IORESOURCE_IRQ,
},
>From my point of view, the common sense for this would be that only
the SDCARD interrupt would be enabled and the rest would be disabled
since they are unspecified. However, with the current code the
behavior would be something else, and since the index number of SDCARD
is not matching it will be detected as CARD_DETECT.
So isn't it really ugly to depend on the number of IRQs when they are
supposed to be used as an index? I've been toying around with this
driver for a few years now, and when I have a hard time creating
correct platform data then it's _probably_ a sign that there must be
better ways to implement this.
I would propose just adding interrupts in struct resource [] as usual,
and then have thee separate flags in the platform data for each
interrupt type. If the number of IRQ bits set in the platform data
flags doesn't match the number of interrupt resources then return
error. If they match then simply go through each flag set in the
platform data flags and assign next available interrupt resource. And
if no flags are set then go for the combined interrupt handler for all
available interrupt resources.
That's what I would do at least. Any other ideas? Perhaps just keep an
array of interrupt numbers in the platform data as the sh-sci driver
does and use the fixed indexes there if non-zero?
Thanks,
/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
