On Fri, Jun 12, 2009 at 04:50:09PM +0200, ext Pandita, Vikram wrote:
>
>
> >-----Original Message-----
> >From: Menon, Nishanth
> >Sent: Friday, June 12, 2009 9:46 AM
> >To: [email protected]
> >Cc: Pandita, Vikram; [email protected]
> >Subject: RE: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver
> >
> >> -----Original Message-----
> >> From: Felipe Balbi [mailto:[email protected]]
> >> Sent: Friday, June 12, 2009 1:38 AM
> >> To: Menon, Nishanth
> >> Cc: Pandita, Vikram; [email protected]
> >> Subject: Re: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver
> >>
> >> On Fri, Jun 12, 2009 at 03:46:37AM +0200, ext Menon, Nishanth wrote:
> >> >
> >> > > -----Original Message-----
> >> > > From: [email protected] [mailto:linux-omap-
> >> > > [email protected]] On Behalf Of Pandita, Vikram
> >> > > Sent: Thursday, June 11, 2009 7:50 PM
> >> > > To: [email protected]
> >> > > Cc: Pandita, Vikram
> >> > > Subject: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver
> >> > >
> >> > >
> >> > > + /* Get IRQ Trigger Flag */
> >> > > + if (up->port.flags & UPF_IRQ_TRIG_RISING)
> >> > > + irq_flags |= IRQF_TRIGGER_RISING;
> >> > > + else if (up->port.flags & UPF_IRQ_TRIG_FALLING)
> >> > > + irq_flags |= IRQF_TRIGGER_FALLING;
> >> > > + else if (up->port.flags & UPF_IRQ_TRIG_HIGH)
> >> > > + irq_flags |= IRQF_TRIGGER_HIGH;
> >> > > + else if (up->port.flags & UPF_IRQ_TRIG_LOW)
> >> > > + irq_flags |= IRQF_TRIGGER_LOW;
> >> > > +
> >> > Blame it on my dislike for nested if elseif, but...
> >> > irq_flags |=
> >> > (up->port.flags & UPF_IRQ_TRIG_RISING)? IRQF_TRIGGER_RISING :
> >> > (up->port.flags & UPF_IRQ_TRIG_FALLING)? IRQF_TRIGGER_FALLING :
> >> > (up->port.flags & IRQF_TRIGGER_HIGH)? IRQF_TRIGGER_HIGH :
> >> > (up->port.flags & UPF_IRQ_TRIG_LOW)? IRQF_TRIGGER_LOW : 0;
> >> >
> >> > Makes sense?
> >>
> >> switch (up->port.flags & UPF_IRQ_FLAGS_MASK) {
> >> case UPF_IRQ_TRIG_RISING:
> >> irq_flags |= IRQF_TRIGGER_RISING;
> >> break;
> >> case UPF_IRQ_TRIG_FALLING:
> >> irq_flags |= IRQF_TRIGGER_FALLING;
> >> break;
> >> case UPF_IRQ_TRIG_HIGH:
> >> irq_flags |= IRQF_TRIGGER_HIGH;
> >> break;
> >> case UPF_IRQ_TRIG_LOW:
> >> irq_flags |= IRQF_TRIGGER_LOW;
> >> break;
> >> default:
> >> printk(KERN_ERR "Unsupported flag\n");
> >> return -EINVAL;
> >> }
> >>
> >Better :) infact, if the port.flags = UPF_IRQ_TRIG_LOW | UPF_IRQ_TRIG_HIGH
> >it might be better to
> >return -EINVAL, which Felipe's change does :).
>
> Needs more investigation as to how current boards using 8250 driver do not
> pass any flag (maybe just SHARED flag) and they still work. Maybe some
> default taken by request_irq()
>
> In short NO_ need to return failure as no flag is a valid use case.
remove the default part:
switch (up->port.flags & UPF_IRQ_FLAGS_MASK) {
case UPF_IRQ_TRIG_RISING:
irq_flags |= IRQF_TRIGGER_RISING;
break;
case UPF_IRQ_TRIG_FALLING:
irq_flags |= IRQF_TRIGGER_FALLING;
break;
case UPF_IRQ_TRIG_HIGH:
irq_flags |= IRQF_TRIGGER_HIGH;
break;
case UPF_IRQ_TRIG_LOW:
irq_flags |= IRQF_TRIGGER_LOW;
break;
default:
break;
}
--
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
