* [PATCH 1/2] Serial: Define IRQ flags for 8250 driver
@ 2009-06-12 0:49 Vikram Pandita
2009-06-12 1:46 ` Menon, Nishanth
0 siblings, 1 reply; 12+ messages in thread
From: Vikram Pandita @ 2009-06-12 0:49 UTC (permalink / raw)
To: linux-omap; +Cc: Vikram Pandita
Boards with serial irq High/Low/Rising/Falling IRQ requirement
do not work today
8250 serial driver does not have provision to pass on IRQ flags
from platform_device
This is requred for OMAP Zoom2 board for which Serial IRQ trigger
is IRQF_TRIGGER_HIGH
Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>
---
drivers/serial/8250.c | 10 ++++++++++
include/linux/serial_core.h | 4 ++++
2 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index bab115e..8235ef5 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -1641,6 +1641,16 @@ static int serial_link_irq_chain(struct uart_8250_port *up)
struct irq_info *i;
int ret, irq_flags = up->port.flags & UPF_SHARE_IRQ ? IRQF_SHARED : 0;
+ /* 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;
+
mutex_lock(&hash_mutex);
h = &irq_lists[up->port.irq % NR_IRQ_HASH];
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 57a97e5..07591d5 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -296,7 +296,11 @@ struct uart_port {
#define UPF_SPD_WARP ((__force upf_t) (0x1010))
#define UPF_SKIP_TEST ((__force upf_t) (1 << 6))
#define UPF_AUTO_IRQ ((__force upf_t) (1 << 7))
+#define UPF_IRQ_TRIG_RISING ((__force upf_t) (1 << 8))
+#define UPF_IRQ_TRIG_FALLING ((__force upf_t) (1 << 9))
+#define UPF_IRQ_TRIG_HIGH ((__force upf_t) (1 << 10))
#define UPF_HARDPPS_CD ((__force upf_t) (1 << 11))
+#define UPF_IRQ_TRIG_LOW ((__force upf_t) (1 << 12))
#define UPF_LOW_LATENCY ((__force upf_t) (1 << 13))
#define UPF_BUGGY_UART ((__force upf_t) (1 << 14))
#define UPF_NO_TXEN_TEST ((__force upf_t) (1 << 15))
--
1.6.0.3.613.g9f8f13
^ permalink raw reply related [flat|nested] 12+ messages in thread* RE: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver
2009-06-12 0:49 [PATCH 1/2] Serial: Define IRQ flags for 8250 driver Vikram Pandita
@ 2009-06-12 1:46 ` Menon, Nishanth
2009-06-12 6:37 ` Felipe Balbi
0 siblings, 1 reply; 12+ messages in thread
From: Menon, Nishanth @ 2009-06-12 1:46 UTC (permalink / raw)
To: Pandita, Vikram, linux-omap@vger.kernel.org
> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Pandita, Vikram
> Sent: Thursday, June 11, 2009 7:50 PM
> To: linux-omap@vger.kernel.org
> 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?
Regards,
Nishanth Menon
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver
2009-06-12 1:46 ` Menon, Nishanth
@ 2009-06-12 6:37 ` Felipe Balbi
2009-06-12 14:45 ` Menon, Nishanth
0 siblings, 1 reply; 12+ messages in thread
From: Felipe Balbi @ 2009-06-12 6:37 UTC (permalink / raw)
To: ext Menon, Nishanth; +Cc: Pandita, Vikram, linux-omap@vger.kernel.org
On Fri, Jun 12, 2009 at 03:46:37AM +0200, ext Menon, Nishanth wrote:
>
> > -----Original Message-----
> > From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> > owner@vger.kernel.org] On Behalf Of Pandita, Vikram
> > Sent: Thursday, June 11, 2009 7:50 PM
> > To: linux-omap@vger.kernel.org
> > 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;
}
???
--
balbi
^ permalink raw reply [flat|nested] 12+ messages in thread* RE: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver
2009-06-12 6:37 ` Felipe Balbi
@ 2009-06-12 14:45 ` Menon, Nishanth
2009-06-12 14:50 ` Pandita, Vikram
0 siblings, 1 reply; 12+ messages in thread
From: Menon, Nishanth @ 2009-06-12 14:45 UTC (permalink / raw)
To: felipe.balbi@nokia.com; +Cc: Pandita, Vikram, linux-omap@vger.kernel.org
> -----Original Message-----
> From: Felipe Balbi [mailto:felipe.balbi@nokia.com]
> Sent: Friday, June 12, 2009 1:38 AM
> To: Menon, Nishanth
> Cc: Pandita, Vikram; linux-omap@vger.kernel.org
> 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: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> > > owner@vger.kernel.org] On Behalf Of Pandita, Vikram
> > > Sent: Thursday, June 11, 2009 7:50 PM
> > > To: linux-omap@vger.kernel.org
> > > 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 :).
Regards,
Nishanth Menon
^ permalink raw reply [flat|nested] 12+ messages in thread* RE: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver
2009-06-12 14:45 ` Menon, Nishanth
@ 2009-06-12 14:50 ` Pandita, Vikram
2009-06-12 14:54 ` Menon, Nishanth
2009-06-12 19:01 ` Felipe Balbi
0 siblings, 2 replies; 12+ messages in thread
From: Pandita, Vikram @ 2009-06-12 14:50 UTC (permalink / raw)
To: Menon, Nishanth, felipe.balbi@nokia.com; +Cc: linux-omap@vger.kernel.org
>-----Original Message-----
>From: Menon, Nishanth
>Sent: Friday, June 12, 2009 9:46 AM
>To: felipe.balbi@nokia.com
>Cc: Pandita, Vikram; linux-omap@vger.kernel.org
>Subject: RE: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver
>
>> -----Original Message-----
>> From: Felipe Balbi [mailto:felipe.balbi@nokia.com]
>> Sent: Friday, June 12, 2009 1:38 AM
>> To: Menon, Nishanth
>> Cc: Pandita, Vikram; linux-omap@vger.kernel.org
>> 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: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
>> > > owner@vger.kernel.org] On Behalf Of Pandita, Vikram
>> > > Sent: Thursday, June 11, 2009 7:50 PM
>> > > To: linux-omap@vger.kernel.org
>> > > 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.
>
>Regards,
>Nishanth Menon
^ permalink raw reply [flat|nested] 12+ messages in thread* RE: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver
2009-06-12 14:50 ` Pandita, Vikram
@ 2009-06-12 14:54 ` Menon, Nishanth
2009-06-12 19:01 ` Felipe Balbi
1 sibling, 0 replies; 12+ messages in thread
From: Menon, Nishanth @ 2009-06-12 14:54 UTC (permalink / raw)
To: Pandita, Vikram, felipe.balbi@nokia.com; +Cc: linux-omap@vger.kernel.org
Regards,
Nishanth Menon
> -----Original Message-----
> From: Pandita, Vikram
> Sent: Friday, June 12, 2009 9:50 AM
> To: Menon, Nishanth; felipe.balbi@nokia.com
> Cc: linux-omap@vger.kernel.org
> Subject: RE: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver
>
>
>
> >-----Original Message-----
> >From: Menon, Nishanth
> >Sent: Friday, June 12, 2009 9:46 AM
> >To: felipe.balbi@nokia.com
> >Cc: Pandita, Vikram; linux-omap@vger.kernel.org
> >Subject: RE: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver
> >
> >> -----Original Message-----
> >> From: Felipe Balbi [mailto:felipe.balbi@nokia.com]
> >> Sent: Friday, June 12, 2009 1:38 AM
> >> To: Menon, Nishanth
> >> Cc: Pandita, Vikram; linux-omap@vger.kernel.org
> >> 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: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> >> > > owner@vger.kernel.org] On Behalf Of Pandita, Vikram
> >> > > Sent: Thursday, June 11, 2009 7:50 PM
> >> > > To: linux-omap@vger.kernel.org
> >> > > 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.
Replacing switch (up->port.flags & UPF_IRQ_FLAGS_MASK)
With
#define UPF_IRQ_LEVEL_FLAGS_MASK \
(UPF_IRQ_TRIG_RISING | UPF_IRQ_TRIG_FALLING |\
UPF_IRQ_TRIG_HIGH | UPF_IRQ_TRIG_LOW)
switch (up->port.flags & UPF_IRQ_LEVEL_FLAGS_MASK) {
will allow for valid -EINVAL return.
Regards,
Nishanth Menon
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver
2009-06-12 14:50 ` Pandita, Vikram
2009-06-12 14:54 ` Menon, Nishanth
@ 2009-06-12 19:01 ` Felipe Balbi
2009-06-12 19:11 ` Pandita, Vikram
2009-06-12 23:13 ` Felipe Contreras
1 sibling, 2 replies; 12+ messages in thread
From: Felipe Balbi @ 2009-06-12 19:01 UTC (permalink / raw)
To: ext Pandita, Vikram
Cc: Menon, Nishanth, Balbi Felipe (Nokia-D/Helsinki),
linux-omap@vger.kernel.org
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: felipe.balbi@nokia.com
> >Cc: Pandita, Vikram; linux-omap@vger.kernel.org
> >Subject: RE: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver
> >
> >> -----Original Message-----
> >> From: Felipe Balbi [mailto:felipe.balbi@nokia.com]
> >> Sent: Friday, June 12, 2009 1:38 AM
> >> To: Menon, Nishanth
> >> Cc: Pandita, Vikram; linux-omap@vger.kernel.org
> >> 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: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> >> > > owner@vger.kernel.org] On Behalf Of Pandita, Vikram
> >> > > Sent: Thursday, June 11, 2009 7:50 PM
> >> > > To: linux-omap@vger.kernel.org
> >> > > 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
^ permalink raw reply [flat|nested] 12+ messages in thread* RE: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver
2009-06-12 19:01 ` Felipe Balbi
@ 2009-06-12 19:11 ` Pandita, Vikram
2009-06-12 23:13 ` Felipe Contreras
1 sibling, 0 replies; 12+ messages in thread
From: Pandita, Vikram @ 2009-06-12 19:11 UTC (permalink / raw)
To: felipe.balbi@nokia.com; +Cc: Menon, Nishanth, linux-omap@vger.kernel.org
>-----Original Message-----
>From: Felipe Balbi [mailto:felipe.balbi@nokia.com]
>Sent: Friday, June 12, 2009 2:02 PM
>To: Pandita, Vikram
>Cc: Menon, Nishanth; Balbi Felipe (Nokia-D/Helsinki); linux-omap@vger.kernel.org
>Subject: Re: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver
>
>On Fri, Jun 12, 2009 at 04:50:09PM +0200, ext Pandita, Vikram wrote:
>>
>>
>
>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;
>}
Have posted the original patch to linux-serial already.
I can see your change is a replacement for if:else with switch:case
Thanks for the review. Lets see what's the response on the patch, and see when we can put these changes in.
>
>--
>balbi
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver
2009-06-12 19:01 ` Felipe Balbi
2009-06-12 19:11 ` Pandita, Vikram
@ 2009-06-12 23:13 ` Felipe Contreras
1 sibling, 0 replies; 12+ messages in thread
From: Felipe Contreras @ 2009-06-12 23:13 UTC (permalink / raw)
To: felipe.balbi
Cc: ext Pandita, Vikram, Menon, Nishanth, linux-omap@vger.kernel.org
On Fri, Jun 12, 2009 at 10:01 PM, Felipe Balbi<felipe.balbi@nokia.com> wrote:
> 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: felipe.balbi@nokia.com
>> >Cc: Pandita, Vikram; linux-omap@vger.kernel.org
>> >Subject: RE: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver
>> >
>> >> -----Original Message-----
>> >> From: Felipe Balbi [mailto:felipe.balbi@nokia.com]
>> >> Sent: Friday, June 12, 2009 1:38 AM
>> >> To: Menon, Nishanth
>> >> Cc: Pandita, Vikram; linux-omap@vger.kernel.org
>> >> 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: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
>> >> > > owner@vger.kernel.org] On Behalf Of Pandita, Vikram
>> >> > > Sent: Thursday, June 11, 2009 7:50 PM
>> >> > > To: linux-omap@vger.kernel.org
>> >> > > 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;
How about?
irq_flags |= IRQF_TRIGGER_RISING; break;
As far as I can tell Linux code-style tries to use as less lines of
code as possible while retaining sane readability.
> 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;
There's no need for the final break, is it?
> }
Cheers.
--
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] Serial: Define IRQ flags for 8250 driver
@ 2009-06-12 0:53 Vikram Pandita
2009-06-12 16:26 ` Marc Zyngier
2009-06-12 16:47 ` Kevin Hilman
0 siblings, 2 replies; 12+ messages in thread
From: Vikram Pandita @ 2009-06-12 0:53 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: linux-omap, Vikram Pandita
Boards with serial irq High/Low/Rising/Falling IRQ requirement
do not work today
8250 serial driver does not have provision to pass on IRQ flags
from platform_device
This is requred for OMAP Zoom2 board for which Serial IRQ trigger
is IRQF_TRIGGER_HIGH
Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>
---
drivers/serial/8250.c | 10 ++++++++++
include/linux/serial_core.h | 4 ++++
2 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index bab115e..8235ef5 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -1641,6 +1641,16 @@ static int serial_link_irq_chain(struct uart_8250_port *up)
struct irq_info *i;
int ret, irq_flags = up->port.flags & UPF_SHARE_IRQ ? IRQF_SHARED : 0;
+ /* 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;
+
mutex_lock(&hash_mutex);
h = &irq_lists[up->port.irq % NR_IRQ_HASH];
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 57a97e5..07591d5 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -296,7 +296,11 @@ struct uart_port {
#define UPF_SPD_WARP ((__force upf_t) (0x1010))
#define UPF_SKIP_TEST ((__force upf_t) (1 << 6))
#define UPF_AUTO_IRQ ((__force upf_t) (1 << 7))
+#define UPF_IRQ_TRIG_RISING ((__force upf_t) (1 << 8))
+#define UPF_IRQ_TRIG_FALLING ((__force upf_t) (1 << 9))
+#define UPF_IRQ_TRIG_HIGH ((__force upf_t) (1 << 10))
#define UPF_HARDPPS_CD ((__force upf_t) (1 << 11))
+#define UPF_IRQ_TRIG_LOW ((__force upf_t) (1 << 12))
#define UPF_LOW_LATENCY ((__force upf_t) (1 << 13))
#define UPF_BUGGY_UART ((__force upf_t) (1 << 14))
#define UPF_NO_TXEN_TEST ((__force upf_t) (1 << 15))
--
1.6.0.3.613.g9f8f13
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver
2009-06-12 0:53 Vikram Pandita
@ 2009-06-12 16:26 ` Marc Zyngier
2009-06-12 16:47 ` Kevin Hilman
1 sibling, 0 replies; 12+ messages in thread
From: Marc Zyngier @ 2009-06-12 16:26 UTC (permalink / raw)
To: Vikram Pandita; +Cc: linux-arm-kernel, linux-omap
On Thu, 11 Jun 2009 19:53:50 -0500
Vikram Pandita <vikram.pandita@ti.com> wrote:
> Boards with serial irq High/Low/Rising/Falling IRQ requirement
> do not work today
>
> 8250 serial driver does not have provision to pass on IRQ flags
> from platform_device
>
> This is requred for OMAP Zoom2 board for which Serial IRQ trigger
> is IRQF_TRIGGER_HIGH
>
> Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>
Arcom Viper (pxa255 based) has the same kind of requirement (triggers
on rising edge), and currently uses a couple of "set_irq_type" that I'd
love to get rid of.
So a patch like this one should definitely be considered.
Best regards,
M.
--
We don't mess around.
-------------------------------------------------------------------
List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
FAQ: http://www.arm.linux.org.uk/mailinglists/faq.php
Etiquette: http://www.arm.linux.org.uk/mailinglists/etiquette.php
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver
2009-06-12 0:53 Vikram Pandita
2009-06-12 16:26 ` Marc Zyngier
@ 2009-06-12 16:47 ` Kevin Hilman
1 sibling, 0 replies; 12+ messages in thread
From: Kevin Hilman @ 2009-06-12 16:47 UTC (permalink / raw)
To: Vikram Pandita; +Cc: linux-arm-kernel, linux-omap
Vikram Pandita <vikram.pandita@ti.com> writes:
> Boards with serial irq High/Low/Rising/Falling IRQ requirement
> do not work today
>
> 8250 serial driver does not have provision to pass on IRQ flags
> from platform_device
>
> This is requred for OMAP Zoom2 board for which Serial IRQ trigger
> is IRQF_TRIGGER_HIGH
>
> Signed-off-by: Vikram Pandita <vikram.pandita@ti.com>
Acked-by: Kevin Hilman <khilman@deeprootsystems.com>
Tested and verified on zoom2. This should to go linux-serial, and CC
LKML, linux-omap.
Before sending, I would update the subject/description slightly:
Subject: serial: 8250: add IRQ trigger support
Description:
There is currently no provision for passing IRQ trigger flags for
serial IRQs with triggering requirements (such as GPIO IRQs.)
This patch adds UPF_IRQ_TRIG_* flags which map on to IRQF_TRIGGER_*
flags.
> ---
> drivers/serial/8250.c | 10 ++++++++++
> include/linux/serial_core.h | 4 ++++
> 2 files changed, 14 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
> index bab115e..8235ef5 100644
> --- a/drivers/serial/8250.c
> +++ b/drivers/serial/8250.c
> @@ -1641,6 +1641,16 @@ static int serial_link_irq_chain(struct uart_8250_port *up)
> struct irq_info *i;
> int ret, irq_flags = up->port.flags & UPF_SHARE_IRQ ? IRQF_SHARED : 0;
>
> + /* 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;
> +
> mutex_lock(&hash_mutex);
>
> h = &irq_lists[up->port.irq % NR_IRQ_HASH];
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 57a97e5..07591d5 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -296,7 +296,11 @@ struct uart_port {
> #define UPF_SPD_WARP ((__force upf_t) (0x1010))
> #define UPF_SKIP_TEST ((__force upf_t) (1 << 6))
> #define UPF_AUTO_IRQ ((__force upf_t) (1 << 7))
> +#define UPF_IRQ_TRIG_RISING ((__force upf_t) (1 << 8))
> +#define UPF_IRQ_TRIG_FALLING ((__force upf_t) (1 << 9))
> +#define UPF_IRQ_TRIG_HIGH ((__force upf_t) (1 << 10))
> #define UPF_HARDPPS_CD ((__force upf_t) (1 << 11))
> +#define UPF_IRQ_TRIG_LOW ((__force upf_t) (1 << 12))
> #define UPF_LOW_LATENCY ((__force upf_t) (1 << 13))
> #define UPF_BUGGY_UART ((__force upf_t) (1 << 14))
> #define UPF_NO_TXEN_TEST ((__force upf_t) (1 << 15))
> --
> 1.6.0.3.613.g9f8f13
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2009-06-12 23:13 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-06-12 0:49 [PATCH 1/2] Serial: Define IRQ flags for 8250 driver Vikram Pandita
2009-06-12 1:46 ` Menon, Nishanth
2009-06-12 6:37 ` Felipe Balbi
2009-06-12 14:45 ` Menon, Nishanth
2009-06-12 14:50 ` Pandita, Vikram
2009-06-12 14:54 ` Menon, Nishanth
2009-06-12 19:01 ` Felipe Balbi
2009-06-12 19:11 ` Pandita, Vikram
2009-06-12 23:13 ` Felipe Contreras
-- strict thread matches above, loose matches on Subject: below --
2009-06-12 0:53 Vikram Pandita
2009-06-12 16:26 ` Marc Zyngier
2009-06-12 16:47 ` Kevin Hilman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox