subject:"\[PATCH 1\/2\] Serial\: Define IRQ flags for 8250 driver"

Re: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver

2009-06-12 Thread Felipe Balbi

On Fri, Jun 12, 2009 at 03:46:37AM +0200, ext Menon, Nishanth wrote:
 
  -Original Message-
  From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap-
  ow...@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
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver

2009-06-12 Thread Menon, Nishanth

 -Original Message-
 From: Felipe Balbi [mailto:felipe.ba...@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-ow...@vger.kernel.org [mailto:linux-omap-
   ow...@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

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver

2009-06-12 Thread Pandita, Vikram



-Original Message-
From: Menon, Nishanth
Sent: Friday, June 12, 2009 9:46 AM
To: felipe.ba...@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.ba...@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-ow...@vger.kernel.org [mailto:linux-omap-
   ow...@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

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver

2009-06-12 Thread Menon, Nishanth



Regards,
Nishanth Menon

 -Original Message-
 From: Pandita, Vikram
 Sent: Friday, June 12, 2009 9:50 AM
 To: Menon, Nishanth; felipe.ba...@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.ba...@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.ba...@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-ow...@vger.kernel.org [mailto:linux-omap-
ow...@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
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver

2009-06-12 Thread Kevin Hilman

Vikram Pandita vikram.pand...@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.pand...@ti.com

Acked-by: Kevin Hilman khil...@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 majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver

2009-06-12 Thread Marc Zyngier

On Thu, 11 Jun 2009 19:53:50 -0500
Vikram Pandita vikram.pand...@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.pand...@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.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver

2009-06-12 Thread Felipe Balbi

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.ba...@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.ba...@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-ow...@vger.kernel.org [mailto:linux-omap-
ow...@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
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver

2009-06-12 Thread Pandita, Vikram



-Original Message-
From: Felipe Balbi [mailto:felipe.ba...@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

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver

2009-06-12 Thread Felipe Contreras

On Fri, Jun 12, 2009 at 10:01 PM, Felipe Balbifelipe.ba...@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.ba...@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.ba...@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-ow...@vger.kernel.org [mailto:linux-omap-
ow...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] Serial: Define IRQ flags for 8250 driver

2009-06-11 Thread 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.pand...@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

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] Serial: Define IRQ flags for 8250 driver

2009-06-11 Thread 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.pand...@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

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver

RE: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver

RE: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver

RE: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver

Re: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver

Re: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver

Re: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver

RE: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver

Re: [PATCH 1/2] Serial: Define IRQ flags for 8250 driver

[PATCH 1/2] Serial: Define IRQ flags for 8250 driver

[PATCH 1/2] Serial: Define IRQ flags for 8250 driver

11 matches

Site Navigation

Mail list logo

Footer information