[PATCH 40/54] ARM: Samsung SoCs: irq_data conversion.

'Lennert Buytenhek' buytenh at wantstofly.org
Tue Dec 14 14:43:00 EST 2010


On Fri, Dec 03, 2010 at 08:56:03PM +0900, Kukjin Kim wrote:

> > Signed-off-by: Lennert Buytenhek <buytenh at secretlab.ca>
> > ---
> >  arch/arm/mach-s3c2410/bast-irq.c         |   22 ++--
> >  arch/arm/mach-s3c2412/irq.c              |   50 +++++-----
> >  arch/arm/mach-s3c2416/irq.c              |   72 +++++++-------
> >  arch/arm/mach-s3c2440/irq.c              |   18 ++--
> >  arch/arm/mach-s3c2440/s3c244x-irq.c      |   18 ++--
> >  arch/arm/mach-s3c2443/irq.c              |   90 ++++++++--------
> >  arch/arm/mach-s3c64xx/irq-eint.c         |   34 +++---
> >  arch/arm/mach-s5pv310/irq-combiner.c     |   26 +++--
> >  arch/arm/mach-s5pv310/irq-eint.c         |   55 +++++-----
> >  arch/arm/plat-s3c24xx/include/plat/irq.h |    4 +-
> >  arch/arm/plat-s3c24xx/irq-pm.c           |    9 +-
> >  arch/arm/plat-s3c24xx/irq.c              |  170
> +++++++++++++++-------------
> > --
> >  arch/arm/plat-s5p/irq-eint.c             |   84 ++++++++--------
> >  arch/arm/plat-s5p/irq-gpioint.c          |   50 +++++-----
> >  arch/arm/plat-s5p/irq-pm.c               |    6 +-
> >  arch/arm/plat-samsung/include/plat/pm.h  |    4 +-
> >  arch/arm/plat-samsung/irq-uart.c         |   44 ++++----
> >  arch/arm/plat-samsung/irq-vic-timer.c    |   22 ++--
> >  arch/arm/plat-samsung/pm.c               |    7 +-
> >  19 files changed, 396 insertions(+), 389 deletions(-)
> 
> Hi Lennert,

Hi Kukjin,


> (Added Mark Brown in Cc)
> 
> Looks ok to me but there are some comments.
> 
> Firstly, how about to separate this to regarding S3C24XX(mach-s3c24xx and
> plat-s3c24xx), S5P(mach-s5pxxxx and plat-s5p) and plat-samsung?
> (Actually, already applied S3C64XX changes so that we can drop that in your
> patch.)

The reason I originally put everything into one patch is that the
change touches s3c_irqext_wake(), which is defined in plat-samsung
and used in mach-s3c64xx, mach-s5pv310, plat-s3c24xx and plat-s5p --
so just splitting the patch across directory boundaries wouldn't work.

Since the s3c_irqext_wake() change is already in your tree, that
argument isn't valid anymore, but..


> As you know, I already applied some changes which is from Mark Brown in my
> tree.
> You can find that in
> git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git
> #for-next
> 
> So could you please re-work this based on that?

..wouldn't it just be easier if you merged the remainder of this
patch via your tree, so as not to introduce a merge dependency between
our trees?


> >  static struct irq_chip  bast_pc104_chip = {
> > -	.mask	     = bast_pc104_mask,
> > -	.unmask	     = bast_pc104_unmask,
> > -	.ack	     = bast_pc104_maskack
> > +	.irq_mask	= bast_pc104_mask,
> > +	.irq_unmask     = bast_pc104_unmask,
>                    ^^^^
> Tab is better instead of white space.

I agree, but the original version also had spaces -- and people
appear to get nervous when they perceive you to be sneaking in
'cleanup' patches along with your changes.


> > -static void s3c2416_irq_lcd_ack(unsigned int irqno)
> > +static void s3c2416_irq_lcd_ack(struct irq_data *d)
> >  {
> > -	s3c_irqsub_maskack(irqno, INTMSK_LCD, SUBMSK_LCD);
> > +	s3c_irqsub_maskack(d->irq, INTMSK_LCD, SUBMSK_LCD);
> >  }
> > 
> >  static struct irq_chip s3c2416_irq_lcd = {
> > -	.mask	    = s3c2416_irq_lcd_mask,
> > -	.unmask	    = s3c2416_irq_lcd_unmask,
> > -	.ack	    = s3c2416_irq_lcd_ack,
> > +	.irq_mask	= s3c2416_irq_lcd_mask,
> > +	.irq_unmask	= s3c2416_irq_lcd_unmask,
> > +	.irq_ack	= s3c2416_irq_lcd_ack,
> >  };
> > 
> > 
> > @@ -142,25 +142,25 @@ static void s3c2416_irq_demux_dma(unsigned int irq,
> > struct irq_desc *desc)
> >  #define SUBMSK_DMA	INTMSK(IRQ_S3C2443_DMA0, IRQ_S3C2443_DMA5)
> > 
> > 
> 
> If possible, please remove useless empty line.
> As you know, just one empty line is enough :-)

The original blank line was already there, and I left it there for
reasons stated above.


> > diff --git a/arch/arm/mach-s3c2440/s3c244x-irq.c b/arch/arm/mach-
> > s3c2440/s3c244x-irq.c
> > index a75c0c2..3878c0a 100644
> > --- a/arch/arm/mach-s3c2440/s3c244x-irq.c
> > +++ b/arch/arm/mach-s3c2440/s3c244x-irq.c
> > @@ -68,27 +68,27 @@ static void s3c_irq_demux_cam(unsigned int irq,
> >  #define INTMSK_CAM (1UL << (IRQ_CAM - IRQ_EINT0))
> > 
> >  static void
> > -s3c_irq_cam_mask(unsigned int irqno)
> > +s3c_irq_cam_mask(struct irq_data *d)
> >  {
> > -	s3c_irqsub_mask(irqno, INTMSK_CAM, 3<<11);
> > +	s3c_irqsub_mask(d->irq, INTMSK_CAM, 3<<11);
> 
> 3 << 11
> Need to add blank around "<<"

See above.


> >  static struct irq_chip s3c2443_irq_lcd = {
> > -	.mask	    = s3c2443_irq_lcd_mask,
> > -	.unmask	    = s3c2443_irq_lcd_unmask,
> > -	.ack	    = s3c2443_irq_lcd_ack,
> > +	.irq_mask	= s3c2443_irq_lcd_mask,
> > +	.irq_unmask	= s3c2443_irq_lcd_unmask,
> > +	.irq_ack	= s3c2443_irq_lcd_ack,
> >  };
> 
> Same...please remove useless empty line.

See above.


thanks,
Lennert



More information about the linux-arm-kernel mailing list