2012-11-26 21:38:56

by Grant Likely

Subject: Re: [PATCH] No need to call irq_domain_legacy_revmap() for twice

On Mon, 24 Sep 2012 17:37:55 +0800, Mike Qiu <[email protected]> wrote:
> Function irq_create_mapping() calls irq_find_mapping(). The later
> function has checked if the indicated IRQ domain has hw IRQ mapped to
> virtual IRQ through legacy mode or not and return the value of the
> legacy irq number by call irq_domain_legacy_revmap(). We needn't
> to call irq_domain_legacy_revmap() to do same check in
> irq_create_mapping() again.
>
> The patch removes the duplicate call.
>
> Signed-off-by: Mike Qiu <[email protected]>
> ---
> kernel/irq/irqdomain.c | 7 +++++--
> 1 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 49a7772..286d672 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -547,9 +547,12 @@ unsigned int irq_create_mapping(struct irq_domain *domain,
> return virq;
> }
>
> - /* Get a virtual interrupt number */
> + /*
> + * For IRQ domain with type of IRQ_DOMAIN_MAP_LEGACY, we needn't
> + * create the IRQ mapping for non-existing one, so just return 0.
> + */
> if (domain->revmap_type == IRQ_DOMAIN_MAP_LEGACY)
> - return irq_domain_legacy_revmap(domain, hwirq);
> + return 0;

But it does need to return the virq assigned to the hwirq. That is why
it has to call the revmap function.

g.

2012-11-27 01:41:57

by Mike Qiu

Subject: Re: [PATCH] No need to call irq_domain_legacy_revmap() for twice

在 2012-11-26一的 20:17 +0000，Grant Likely写道：
> On Mon, 24 Sep 2012 17:37:55 +0800, Mike Qiu <[email protected]> wrote:
> > Function irq_create_mapping() calls irq_find_mapping(). The later
> > function has checked if the indicated IRQ domain has hw IRQ mapped to
> > virtual IRQ through legacy mode or not and return the value of the
> > legacy irq number by call irq_domain_legacy_revmap(). We needn't
> > to call irq_domain_legacy_revmap() to do same check in
> > irq_create_mapping() again.
> >
> > The patch removes the duplicate call.
> >
> > Signed-off-by: Mike Qiu <[email protected]>
> > ---
> > kernel/irq/irqdomain.c | 7 +++++--
> > 1 files changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> > index 49a7772..286d672 100644
> > --- a/kernel/irq/irqdomain.c
> > +++ b/kernel/irq/irqdomain.c
> > @@ -547,9 +547,12 @@ unsigned int irq_create_mapping(struct irq_domain *domain,
> > return virq;
> > }
> >
> > - /* Get a virtual interrupt number */
> > + /*
> > + * For IRQ domain with type of IRQ_DOMAIN_MAP_LEGACY, we needn't
> > + * create the IRQ mapping for non-existing one, so just return 0.
> > + */
> > if (domain->revmap_type == IRQ_DOMAIN_MAP_LEGACY)
> > - return irq_domain_legacy_revmap(domain, hwirq);
> > + return 0;
>
> But it does need to return the virq assigned to the hwirq. That is why
> it has to call the revmap function.
>
Yes, thanks

this judgment has been done in
/*Check if mapping already exists*/
virq = irq_find_mapping(domain, hwirq);

if the virq equals none zero, the func irq_create_mapping will
return the virq value directly(already exists).

otherwise, that means virq equals zero, this has two meanings, one is
haven't find the already exist mapping, the other one is in
irq_find_mapping()
case IRQ_DOMAIN_MAP_LEGACY:
return irq_domain_legacy_revmap(domain, hwirq);
this may return zero.

So, when we check if it is a IRQ_DOMAIN_MAP_LEGACY, we just return zero
is OK. because if it is none zero, it will be return after check if
mapping already exists, and never come here. also we never try to assign
the virq for the legacy map.

I don't know if you have understand my explanation.

Thanks
Mike



> g.
>