Re: [PATCH v3 2/8] wlcore: set irq_flags in the board files instead of hiding behind a quirk

2013-07-04 Thread Luciano Coelho
On Wed, 2013-07-03 at 17:12 +0200, Javier Martinez Canillas wrote:
 On Wed, Jul 3, 2013 at 4:15 PM, Luciano Coelho coe...@ti.com wrote:
  On Wed, 2013-07-03 at 17:03 +0300, Luciano Coelho wrote:
  The platform_quirk element in the platform data was used to change the
  way the IRQ is triggered.  When set, the EDGE_IRQ quirk would change
  the irqflags used and treat edge trigger differently from the rest.
 
  Instead of hiding this irq flag setting behind the quirk, have the
  board files set the flags during initialization.  This will be more
  meaningful than driver-specific quirks when we switch to DT.
 
  Additionally, fix missing gpio_request() calls in the boarding files
  (so that setting the flags actually works).
 
  Cc: Tony Lindgren t...@atomide.com
  Cc: Sekhar Nori nsek...@ti.com
  Signed-off-by: Luciano Coelho coe...@ti.com
  ---
 
  [...]
 
  @@ -5928,16 +5927,21 @@ static void wlcore_nvs_cb(const struct firmware 
  *fw, void *context)
wlcore_adjust_conf(wl);
 
wl-irq = platform_get_irq(pdev, 0);
  - wl-platform_quirks = pdata-platform_quirks;
wl-if_ops = pdev_data-if_ops;
 
  - if (wl-platform_quirks  WL12XX_PLATFORM_QUIRK_EDGE_IRQ)
  - irqflags = IRQF_TRIGGER_RISING;
  - else
  - irqflags = IRQF_TRIGGER_HIGH | IRQF_ONESHOT;
  + irq_data = irq_get_irq_data(wl-irq);
  + if (!irq_data) {
  + wl1271_error(couldn't get irq data for irq %d\n, wl-irq);
  + ret = -EINVAL;
  + goto out_free_nvs;
  + }
  +
  + wl-irq_flags = irqd_get_trigger_type(irq_data);
 
  BTW, there seems to be a patch on its way to make reading the flags
  easier (ie. no need to get the irq_data first):
 
  http://mid.gmane.org/1367945288-5625-1-git-send-email-jav...@dowhile0.org
 
  I'm not sure if this is going to be taken in, but if it does, it would
  be nice to change the code here to use the new irq_get_trigger_type()
  function.

 That patch has been already merged in Linus tree as commit 1f6236bf
 (genirq: Add irq_get_trigger_type() to get IRQ flags).
 
 So yes, it would be better if you can use irq_get_trigger_type()
 instead calling irq_get_irq_data() + irqd_get_trigger_type().

That's great, thanks! I'll make this change as soon as I get your patch
into my tree (ie. after the merge window closes).

--
Luca.

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH v3 2/8] wlcore: set irq_flags in the board files instead of hiding behind a quirk

2013-07-03 Thread Felipe Balbi
Hi,

On Wed, Jul 03, 2013 at 05:03:23PM +0300, Luciano Coelho wrote:
 diff --git a/arch/arm/mach-omap2/board-4430sdp.c 
 b/arch/arm/mach-omap2/board-4430sdp.c
 index 56a9a4f..953f620 100644
 --- a/arch/arm/mach-omap2/board-4430sdp.c
 +++ b/arch/arm/mach-omap2/board-4430sdp.c
 @@ -703,12 +703,30 @@ static void __init omap4_sdp4430_wifi_init(void)
  
   omap4_sdp4430_wifi_mux_init();
   omap4_sdp4430_wlan_data.irq = gpio_to_irq(GPIO_WIFI_IRQ);
 +
 + ret = gpio_request_one(GPIO_WIFI_IRQ, GPIOF_IN, GPIO_WIFI_IRQ);
 + if (ret) {
 + pr_err(error requesting wl12xx gpio: %d\n, ret);
 + goto out;
 + }
 +
 + ret = irq_set_irq_type(gpio_to_irq(GPIO_WIFI_IRQ), IRQ_TYPE_LEVEL_HIGH);
 + if (ret) {
 + pr_err(error setting wl12xx irq type: %d\n, ret);
 + goto free;
 + }
 +
   ret = wl12xx_set_platform_data(omap4_sdp4430_wlan_data);
   if (ret)
   pr_err(Error setting wl12xx data: %d\n, ret);
 +
   ret = platform_device_register(omap_vwlan_device);
   if (ret)
   pr_err(Error registering wl12xx device: %d\n, ret);
 +out:
 + return;
 +free:
 + gpio_free(GPIO_WIFI_IRQ);

actually, you should leave this GPIO requested in order to use it as
IRQ.

ditto for all others

-- 
balbi


signature.asc
Description: Digital signature
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH v3 2/8] wlcore: set irq_flags in the board files instead of hiding behind a quirk

2013-07-03 Thread Luciano Coelho
On Wed, 2013-07-03 at 17:03 +0300, Luciano Coelho wrote:
 The platform_quirk element in the platform data was used to change the
 way the IRQ is triggered.  When set, the EDGE_IRQ quirk would change
 the irqflags used and treat edge trigger differently from the rest.
 
 Instead of hiding this irq flag setting behind the quirk, have the
 board files set the flags during initialization.  This will be more
 meaningful than driver-specific quirks when we switch to DT.
 
 Additionally, fix missing gpio_request() calls in the boarding files
 (so that setting the flags actually works).
 
 Cc: Tony Lindgren t...@atomide.com
 Cc: Sekhar Nori nsek...@ti.com
 Signed-off-by: Luciano Coelho coe...@ti.com
 ---

[...]

 @@ -5928,16 +5927,21 @@ static void wlcore_nvs_cb(const struct firmware *fw, 
 void *context)
   wlcore_adjust_conf(wl);
  
   wl-irq = platform_get_irq(pdev, 0);
 - wl-platform_quirks = pdata-platform_quirks;
   wl-if_ops = pdev_data-if_ops;
  
 - if (wl-platform_quirks  WL12XX_PLATFORM_QUIRK_EDGE_IRQ)
 - irqflags = IRQF_TRIGGER_RISING;
 - else
 - irqflags = IRQF_TRIGGER_HIGH | IRQF_ONESHOT;
 + irq_data = irq_get_irq_data(wl-irq);
 + if (!irq_data) {
 + wl1271_error(couldn't get irq data for irq %d\n, wl-irq);
 + ret = -EINVAL;
 + goto out_free_nvs;
 + }
 +
 + wl-irq_flags = irqd_get_trigger_type(irq_data);

BTW, there seems to be a patch on its way to make reading the flags
easier (ie. no need to get the irq_data first):

http://mid.gmane.org/1367945288-5625-1-git-send-email-jav...@dowhile0.org

I'm not sure if this is going to be taken in, but if it does, it would
be nice to change the code here to use the new irq_get_trigger_type()
function.

--
Luca.

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH v3 2/8] wlcore: set irq_flags in the board files instead of hiding behind a quirk

2013-07-03 Thread Luciano Coelho
On Wed, 2013-07-03 at 17:13 +0300, Felipe Balbi wrote:
 Hi,
 
 On Wed, Jul 03, 2013 at 05:03:23PM +0300, Luciano Coelho wrote:
  diff --git a/arch/arm/mach-omap2/board-4430sdp.c 
  b/arch/arm/mach-omap2/board-4430sdp.c
  index 56a9a4f..953f620 100644
  --- a/arch/arm/mach-omap2/board-4430sdp.c
  +++ b/arch/arm/mach-omap2/board-4430sdp.c
  @@ -703,12 +703,30 @@ static void __init omap4_sdp4430_wifi_init(void)
   
  omap4_sdp4430_wifi_mux_init();
  omap4_sdp4430_wlan_data.irq = gpio_to_irq(GPIO_WIFI_IRQ);
  +
  +   ret = gpio_request_one(GPIO_WIFI_IRQ, GPIOF_IN, GPIO_WIFI_IRQ);
  +   if (ret) {
  +   pr_err(error requesting wl12xx gpio: %d\n, ret);
  +   goto out;
  +   }
  +
  +   ret = irq_set_irq_type(gpio_to_irq(GPIO_WIFI_IRQ), IRQ_TYPE_LEVEL_HIGH);
  +   if (ret) {
  +   pr_err(error setting wl12xx irq type: %d\n, ret);
  +   goto free;
  +   }
  +
  ret = wl12xx_set_platform_data(omap4_sdp4430_wlan_data);
  if (ret)
  pr_err(Error setting wl12xx data: %d\n, ret);
  +
  ret = platform_device_register(omap_vwlan_device);
  if (ret)
  pr_err(Error registering wl12xx device: %d\n, ret);
  +out:
  +   return;
  +free:
  +   gpio_free(GPIO_WIFI_IRQ);
 
 actually, you should leave this GPIO requested in order to use it as
 IRQ.
 
 ditto for all others

Actually, I don't need to use the GPIO if something goes wrong (ie.
setting the IRQ type or setting the platform data).  Notice that in the
normal cases (ie. without errors), I return before the gpio_free() is
called.

This is not really needed, but it's a bit cleaner and we can probably
release some resources.

--
Luca.

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH v3 2/8] wlcore: set irq_flags in the board files instead of hiding behind a quirk

2013-07-03 Thread Felipe Balbi
On Wed, Jul 03, 2013 at 05:18:14PM +0300, Luciano Coelho wrote:
 On Wed, 2013-07-03 at 17:13 +0300, Felipe Balbi wrote:
  Hi,
  
  On Wed, Jul 03, 2013 at 05:03:23PM +0300, Luciano Coelho wrote:
   diff --git a/arch/arm/mach-omap2/board-4430sdp.c 
   b/arch/arm/mach-omap2/board-4430sdp.c
   index 56a9a4f..953f620 100644
   --- a/arch/arm/mach-omap2/board-4430sdp.c
   +++ b/arch/arm/mach-omap2/board-4430sdp.c
   @@ -703,12 +703,30 @@ static void __init omap4_sdp4430_wifi_init(void)

 omap4_sdp4430_wifi_mux_init();
 omap4_sdp4430_wlan_data.irq = gpio_to_irq(GPIO_WIFI_IRQ);
   +
   + ret = gpio_request_one(GPIO_WIFI_IRQ, GPIOF_IN, GPIO_WIFI_IRQ);
   + if (ret) {
   + pr_err(error requesting wl12xx gpio: %d\n, ret);
   + goto out;
   + }
   +
   + ret = irq_set_irq_type(gpio_to_irq(GPIO_WIFI_IRQ), IRQ_TYPE_LEVEL_HIGH);
   + if (ret) {
   + pr_err(error setting wl12xx irq type: %d\n, ret);
   + goto free;
   + }
   +
 ret = wl12xx_set_platform_data(omap4_sdp4430_wlan_data);
 if (ret)
 pr_err(Error setting wl12xx data: %d\n, ret);
   +
 ret = platform_device_register(omap_vwlan_device);
 if (ret)
 pr_err(Error registering wl12xx device: %d\n, ret);
   +out:
   + return;
   +free:
   + gpio_free(GPIO_WIFI_IRQ);
  
  actually, you should leave this GPIO requested in order to use it as
  IRQ.
  
  ditto for all others
 
 Actually, I don't need to use the GPIO if something goes wrong (ie.
 setting the IRQ type or setting the platform data).  Notice that in the
 normal cases (ie. without errors), I return before the gpio_free() is
 called.

hah, missed the 'return' call, my bad :-p

Reviewed-by: Felipe Balbi ba...@ti.com
Corrected-my-broken-eye-sight-by: Luciano Coelho coe...@ti.com

-- 
balbi


signature.asc
Description: Digital signature
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH v3 2/8] wlcore: set irq_flags in the board files instead of hiding behind a quirk

2013-07-03 Thread Javier Martinez Canillas
On Wed, Jul 3, 2013 at 4:15 PM, Luciano Coelho coe...@ti.com wrote:
 On Wed, 2013-07-03 at 17:03 +0300, Luciano Coelho wrote:
 The platform_quirk element in the platform data was used to change the
 way the IRQ is triggered.  When set, the EDGE_IRQ quirk would change
 the irqflags used and treat edge trigger differently from the rest.

 Instead of hiding this irq flag setting behind the quirk, have the
 board files set the flags during initialization.  This will be more
 meaningful than driver-specific quirks when we switch to DT.

 Additionally, fix missing gpio_request() calls in the boarding files
 (so that setting the flags actually works).

 Cc: Tony Lindgren t...@atomide.com
 Cc: Sekhar Nori nsek...@ti.com
 Signed-off-by: Luciano Coelho coe...@ti.com
 ---

 [...]

 @@ -5928,16 +5927,21 @@ static void wlcore_nvs_cb(const struct firmware *fw, 
 void *context)
   wlcore_adjust_conf(wl);

   wl-irq = platform_get_irq(pdev, 0);
 - wl-platform_quirks = pdata-platform_quirks;
   wl-if_ops = pdev_data-if_ops;

 - if (wl-platform_quirks  WL12XX_PLATFORM_QUIRK_EDGE_IRQ)
 - irqflags = IRQF_TRIGGER_RISING;
 - else
 - irqflags = IRQF_TRIGGER_HIGH | IRQF_ONESHOT;
 + irq_data = irq_get_irq_data(wl-irq);
 + if (!irq_data) {
 + wl1271_error(couldn't get irq data for irq %d\n, wl-irq);
 + ret = -EINVAL;
 + goto out_free_nvs;
 + }
 +
 + wl-irq_flags = irqd_get_trigger_type(irq_data);

 BTW, there seems to be a patch on its way to make reading the flags
 easier (ie. no need to get the irq_data first):

 http://mid.gmane.org/1367945288-5625-1-git-send-email-jav...@dowhile0.org

 I'm not sure if this is going to be taken in, but if it does, it would
 be nice to change the code here to use the new irq_get_trigger_type()
 function.

 --
 Luca.


Hi Luca

That patch has been already merged in Linus tree as commit 1f6236bf
(genirq: Add irq_get_trigger_type() to get IRQ flags).

So yes, it would be better if you can use irq_get_trigger_type()
instead calling irq_get_irq_data() + irqd_get_trigger_type().

Best regards,
Javier
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH v3 2/8] wlcore: set irq_flags in the board files instead of hiding behind a quirk

2013-07-03 Thread Sekhar Nori
On 7/3/2013 7:33 PM, Luciano Coelho wrote:
 The platform_quirk element in the platform data was used to change the
 way the IRQ is triggered.  When set, the EDGE_IRQ quirk would change
 the irqflags used and treat edge trigger differently from the rest.
 
 Instead of hiding this irq flag setting behind the quirk, have the
 board files set the flags during initialization.  This will be more
 meaningful than driver-specific quirks when we switch to DT.
 
 Additionally, fix missing gpio_request() calls in the boarding files
 (so that setting the flags actually works).
 
 Cc: Tony Lindgren t...@atomide.com
 Cc: Sekhar Nori nsek...@ti.com
 Signed-off-by: Luciano Coelho coe...@ti.com
 ---
  arch/arm/mach-davinci/board-da850-evm.c  |8 +-

For the board-da850-evm.c change,

Acked-by: Sekhar Nori nsek...@ti.com

Thanks,
Sekhar
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss