Received-SPF: pass (google.com: domain of linux-kernel+bounces-30700-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Fri, 19 Jan 2024 10:02:33 +0800
From: Dawei Li <dawei.li@shingroup.cn>
To: Marc Zyngier <maz@kernel.org>
Cc: tglx@linutronix.de, sdonthineni@nvidia.com,
	linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
	set_pte_at@outlook.com
Subject: Re: [PATCH 2/4] irqchip/gic: Implement generic gic_irq() API for GIC
 & GIC-v3
Message-ID: <0021C3C0D197C59F+ZanYOXvAxIT1pF0h@centos8>
References: <20240118112739.2000497-1-dawei.li@shingroup.cn>
 <20240118112739.2000497-3-dawei.li@shingroup.cn>
 <87zfx2wxgj.wl-maz@kernel.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87zfx2wxgj.wl-maz@kernel.org>
Feedback-ID: bizesmtp:shingroup.cn:qybglogicsvrgz:qybglogicsvrgz5a-1

Hi Marc,
Thanks for the review.

On Thu, Jan 18, 2024 at 02:03:08PM +0000, Marc Zyngier wrote:
> On Thu, 18 Jan 2024 11:27:37 +0000,
> Dawei Li <dawei.li@shingroup.cn> wrote:
> > 
> > GIC & GIC-v3 share same gic_irq() implementations, unify them into a
> > generic API.
> > 
> > Signed-off-by: Dawei Li <dawei.li@shingroup.cn>
> > ---
> >  drivers/irqchip/irq-gic-common.h | 5 +++++
> >  drivers/irqchip/irq-gic-v3.c     | 5 -----
> >  drivers/irqchip/irq-gic.c        | 5 -----
> >  3 files changed, 5 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/irqchip/irq-gic-common.h b/drivers/irqchip/irq-gic-common.h
> > index f407cce9ecaa..ed18db4ab2c5 100644
> > --- a/drivers/irqchip/irq-gic-common.h
> > +++ b/drivers/irqchip/irq-gic-common.h
> > @@ -19,6 +19,11 @@ struct gic_quirk {
> >  	u32 mask;
> >  };
> >  
> > +static inline unsigned int gic_irq(struct irq_data *d)
> > +{
> > +	return d->hwirq;
> > +}
> > +
> >  int gic_configure_irq(unsigned int irq, unsigned int type,
> >                         void __iomem *base, void (*sync_access)(void));
> >  void gic_dist_config(void __iomem *base, int gic_irqs,
> > diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> > index b9d9375a3434..474a498a521e 100644
> > --- a/drivers/irqchip/irq-gic-v3.c
> > +++ b/drivers/irqchip/irq-gic-v3.c
> > @@ -181,11 +181,6 @@ static enum gic_intid_range get_intid_range(struct irq_data *d)
> >  	return __get_intid_range(d->hwirq);
> >  }
> >  
> > -static inline unsigned int gic_irq(struct irq_data *d)
> > -{
> > -	return d->hwirq;
> > -}
> > -
> 
> I'd rather not do that. If anything, I'd get rid of the helper
> altogether, as we have irqd_to_hwirq() that does the same job, and
> actually preserves the typing.

Yes, your solution is much better for it's truly generic, independent of
irq chip.

I will send respin of V2 as you suggested, and if I may, add your
suggested-by?

Thanks,
	Dawei

> 
> 	M.
> 
> -- 
> Without deviation from the norm, progress is not possible.
>