All of lore.kernel.org
 help / color / mirror / Atom feed
From: Grygorii Strashko <grygorii.strashko@ti.com>
To: Tony Lindgren <tony@atomide.com>
Cc: Kevin Hilman <khilman@linaro.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-omap@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<devicetree-discuss@lists.ozlabs.org>,
	Benoit Cousson <benoit.cousson@linaro.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Stephen Warren <swarren@wwwdotorg.org>
Subject: Re: [PATCH 3/3] ARM: dts: omap4-sdp: add dynamic pin states for uart3/4
Date: Thu, 18 Jul 2013 11:54:16 +0300	[thread overview]
Message-ID: <51E7AD38.6010108@ti.com> (raw)
In-Reply-To: <20130718080953.GR7656@atomide.com>

On 07/18/2013 11:09 AM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [130717 09:48]:
>> Hi,
>>
>> On 07/17/2013 06:32 PM, Tony Lindgren wrote:
>>> * Grygorii Strashko <grygorii.strashko@ti.com> [130717 04:49]:
>>>> Add dynamic "active"/"idle" pin states for uart3/4 which will be applied
>>>> when uart3/4 state is switched from active to idle and back by Runtime
>>>> PM or during system suspend.
>>>
>>> This is good for testing code, but should not be merged because
>>> omap4 has the iopad wake-ups available for uarts. So those can
>>> be always enabled.
>>
>> In this case, 2 IRQ will be received per each UART RX event - one from
>> PRCM and from UART - and that's not good from PM perspective (It will
>> affect on CPUIdle and CPUFreq at least).
>
> Oh I see, that's because I accidentally left the debug code enabled
> to make it easier to test the wake-up events without having to
> have working off-idle. The wake flags can be kept on always for
> sure.
>
> The patch below should sort out the issue of getting wake-up interrupts
> during runtime as long as you don't have DEBUG defined.
>
> Regards,
>
> Tony
>
> --- a/drivers/pinctrl/pinctrl-single-omap.c
> +++ b/drivers/pinctrl/pinctrl-single-omap.c
> @@ -140,9 +140,17 @@ static irqreturn_t pcs_omap_handle_irq(int irq, void *data)
>   		if ((val & OMAP_WAKEUP_EVENT_MASK) == OMAP_WAKEUP_EVENT_MASK)
>   			generic_handle_irq(wakeirq);
>   	}
> -
> +#ifdef DEBUG

Don't think it's debug code - IO chain need to be rearmed after each
PRCM IO IRQ - otherwise IO wakeup events may be lost (at least on
OMAP4, OMAP5 requires more complex handling(( ).


> +	/*
> +	 * This enables wake-up interrupts during runtime also
> +	 * causing duplicate interrupts. But it also makes debugging
> +	 * the wake-up events easy as deeper idle states often are
> +	 * not working for new devices while the drivers are being
> +	 * developed.
> +	 */
>   	if (pcso->reconfigure_io_chain)
>   		pcso->reconfigure_io_chain();
> +#endif
>
>   	return IRQ_HANDLED;
>   }
>

I didn't pick up your padconf patches yet -seems i need to be in sync :)

Below the diff I used to verify IO wake up (It follows old IO daisy
chain hanlding models in hwmod before DT):


--- a/arch/arm/mach-omap2/omap_device.c
+++ b/arch/arm/mach-omap2/omap_device.c
@@ -574,7 +574,7 @@ odbs_exit:

         return ERR_PTR(ret);
  }
-
+#include "prm44xx.h"
  #ifdef CONFIG_PM_RUNTIME
  static int _od_runtime_suspend(struct device *dev)
  {
@@ -586,6 +586,7 @@ static int _od_runtime_suspend(struct device *dev)
         if (!ret) {
                 omap_device_idle(pdev);
                 pinctrl_pm_select_idle_state(dev);
+               omap44xx_prm_reconfigure_io_chain();
         }

         return ret;
@@ -596,6 +597,7 @@ static int _od_runtime_resume(struct device *dev)
         struct platform_device *pdev = to_platform_device(dev);

         pinctrl_pm_select_active_state(dev);
+       omap44xx_prm_reconfigure_io_chain();

         omap_device_enable(pdev);

diff --git a/arch/arm/mach-omap2/prm_common.c 
b/arch/arm/mach-omap2/prm_common.c
index 228b850..5db073a 100644
--- a/arch/arm/mach-omap2/prm_common.c
+++ b/arch/arm/mach-omap2/prm_common.c
@@ -79,7 +79,7 @@ static void omap_prcm_events_filter_priority(unsigned 
long *events,
                 events[i] ^= priority_events[i];
         }
  }
-
+#include "prm44xx.h"
  /*
   * PRCM Interrupt Handler
   *
@@ -144,6 +144,7 @@ static void omap_prcm_irq_handler(unsigned int irq, 
struct irq_desc *desc)
         chip->irq_unmask(&desc->irq_data);

         prcm_irq_setup->ocp_barrier(); /* avoid spurious IRQs */
+       omap44xx_prm_reconfigure_io_chain();
  }



WARNING: multiple messages have this Message-ID (diff)
From: Grygorii Strashko <grygorii.strashko@ti.com>
To: Tony Lindgren <tony@atomide.com>
Cc: Kevin Hilman <khilman@linaro.org>,
	linux-arm-kernel@lists.infradead.org, linux-omap@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	devicetree-discuss@lists.ozlabs.org,
	Benoit Cousson <benoit.cousson@linaro.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Stephen Warren <swarren@wwwdotorg.org>
Subject: Re: [PATCH 3/3] ARM: dts: omap4-sdp: add dynamic pin states for uart3/4
Date: Thu, 18 Jul 2013 11:54:16 +0300	[thread overview]
Message-ID: <51E7AD38.6010108@ti.com> (raw)
In-Reply-To: <20130718080953.GR7656@atomide.com>

On 07/18/2013 11:09 AM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [130717 09:48]:
>> Hi,
>>
>> On 07/17/2013 06:32 PM, Tony Lindgren wrote:
>>> * Grygorii Strashko <grygorii.strashko@ti.com> [130717 04:49]:
>>>> Add dynamic "active"/"idle" pin states for uart3/4 which will be applied
>>>> when uart3/4 state is switched from active to idle and back by Runtime
>>>> PM or during system suspend.
>>>
>>> This is good for testing code, but should not be merged because
>>> omap4 has the iopad wake-ups available for uarts. So those can
>>> be always enabled.
>>
>> In this case, 2 IRQ will be received per each UART RX event - one from
>> PRCM and from UART - and that's not good from PM perspective (It will
>> affect on CPUIdle and CPUFreq at least).
>
> Oh I see, that's because I accidentally left the debug code enabled
> to make it easier to test the wake-up events without having to
> have working off-idle. The wake flags can be kept on always for
> sure.
>
> The patch below should sort out the issue of getting wake-up interrupts
> during runtime as long as you don't have DEBUG defined.
>
> Regards,
>
> Tony
>
> --- a/drivers/pinctrl/pinctrl-single-omap.c
> +++ b/drivers/pinctrl/pinctrl-single-omap.c
> @@ -140,9 +140,17 @@ static irqreturn_t pcs_omap_handle_irq(int irq, void *data)
>   		if ((val & OMAP_WAKEUP_EVENT_MASK) == OMAP_WAKEUP_EVENT_MASK)
>   			generic_handle_irq(wakeirq);
>   	}
> -
> +#ifdef DEBUG

Don't think it's debug code - IO chain need to be rearmed after each
PRCM IO IRQ - otherwise IO wakeup events may be lost (at least on
OMAP4, OMAP5 requires more complex handling(( ).


> +	/*
> +	 * This enables wake-up interrupts during runtime also
> +	 * causing duplicate interrupts. But it also makes debugging
> +	 * the wake-up events easy as deeper idle states often are
> +	 * not working for new devices while the drivers are being
> +	 * developed.
> +	 */
>   	if (pcso->reconfigure_io_chain)
>   		pcso->reconfigure_io_chain();
> +#endif
>
>   	return IRQ_HANDLED;
>   }
>

I didn't pick up your padconf patches yet -seems i need to be in sync :)

Below the diff I used to verify IO wake up (It follows old IO daisy
chain hanlding models in hwmod before DT):


--- a/arch/arm/mach-omap2/omap_device.c
+++ b/arch/arm/mach-omap2/omap_device.c
@@ -574,7 +574,7 @@ odbs_exit:

         return ERR_PTR(ret);
  }
-
+#include "prm44xx.h"
  #ifdef CONFIG_PM_RUNTIME
  static int _od_runtime_suspend(struct device *dev)
  {
@@ -586,6 +586,7 @@ static int _od_runtime_suspend(struct device *dev)
         if (!ret) {
                 omap_device_idle(pdev);
                 pinctrl_pm_select_idle_state(dev);
+               omap44xx_prm_reconfigure_io_chain();
         }

         return ret;
@@ -596,6 +597,7 @@ static int _od_runtime_resume(struct device *dev)
         struct platform_device *pdev = to_platform_device(dev);

         pinctrl_pm_select_active_state(dev);
+       omap44xx_prm_reconfigure_io_chain();

         omap_device_enable(pdev);

diff --git a/arch/arm/mach-omap2/prm_common.c 
b/arch/arm/mach-omap2/prm_common.c
index 228b850..5db073a 100644
--- a/arch/arm/mach-omap2/prm_common.c
+++ b/arch/arm/mach-omap2/prm_common.c
@@ -79,7 +79,7 @@ static void omap_prcm_events_filter_priority(unsigned 
long *events,
                 events[i] ^= priority_events[i];
         }
  }
-
+#include "prm44xx.h"
  /*
   * PRCM Interrupt Handler
   *
@@ -144,6 +144,7 @@ static void omap_prcm_irq_handler(unsigned int irq, 
struct irq_desc *desc)
         chip->irq_unmask(&desc->irq_data);

         prcm_irq_setup->ocp_barrier(); /* avoid spurious IRQs */
+       omap44xx_prm_reconfigure_io_chain();
  }

WARNING: multiple messages have this Message-ID (diff)
From: grygorii.strashko@ti.com (Grygorii Strashko)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/3] ARM: dts: omap4-sdp: add dynamic pin states for uart3/4
Date: Thu, 18 Jul 2013 11:54:16 +0300	[thread overview]
Message-ID: <51E7AD38.6010108@ti.com> (raw)
In-Reply-To: <20130718080953.GR7656@atomide.com>

On 07/18/2013 11:09 AM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [130717 09:48]:
>> Hi,
>>
>> On 07/17/2013 06:32 PM, Tony Lindgren wrote:
>>> * Grygorii Strashko <grygorii.strashko@ti.com> [130717 04:49]:
>>>> Add dynamic "active"/"idle" pin states for uart3/4 which will be applied
>>>> when uart3/4 state is switched from active to idle and back by Runtime
>>>> PM or during system suspend.
>>>
>>> This is good for testing code, but should not be merged because
>>> omap4 has the iopad wake-ups available for uarts. So those can
>>> be always enabled.
>>
>> In this case, 2 IRQ will be received per each UART RX event - one from
>> PRCM and from UART - and that's not good from PM perspective (It will
>> affect on CPUIdle and CPUFreq at least).
>
> Oh I see, that's because I accidentally left the debug code enabled
> to make it easier to test the wake-up events without having to
> have working off-idle. The wake flags can be kept on always for
> sure.
>
> The patch below should sort out the issue of getting wake-up interrupts
> during runtime as long as you don't have DEBUG defined.
>
> Regards,
>
> Tony
>
> --- a/drivers/pinctrl/pinctrl-single-omap.c
> +++ b/drivers/pinctrl/pinctrl-single-omap.c
> @@ -140,9 +140,17 @@ static irqreturn_t pcs_omap_handle_irq(int irq, void *data)
>   		if ((val & OMAP_WAKEUP_EVENT_MASK) == OMAP_WAKEUP_EVENT_MASK)
>   			generic_handle_irq(wakeirq);
>   	}
> -
> +#ifdef DEBUG

Don't think it's debug code - IO chain need to be rearmed after each
PRCM IO IRQ - otherwise IO wakeup events may be lost (at least on
OMAP4, OMAP5 requires more complex handling(( ).


> +	/*
> +	 * This enables wake-up interrupts during runtime also
> +	 * causing duplicate interrupts. But it also makes debugging
> +	 * the wake-up events easy as deeper idle states often are
> +	 * not working for new devices while the drivers are being
> +	 * developed.
> +	 */
>   	if (pcso->reconfigure_io_chain)
>   		pcso->reconfigure_io_chain();
> +#endif
>
>   	return IRQ_HANDLED;
>   }
>

I didn't pick up your padconf patches yet -seems i need to be in sync :)

Below the diff I used to verify IO wake up (It follows old IO daisy
chain hanlding models in hwmod before DT):


--- a/arch/arm/mach-omap2/omap_device.c
+++ b/arch/arm/mach-omap2/omap_device.c
@@ -574,7 +574,7 @@ odbs_exit:

         return ERR_PTR(ret);
  }
-
+#include "prm44xx.h"
  #ifdef CONFIG_PM_RUNTIME
  static int _od_runtime_suspend(struct device *dev)
  {
@@ -586,6 +586,7 @@ static int _od_runtime_suspend(struct device *dev)
         if (!ret) {
                 omap_device_idle(pdev);
                 pinctrl_pm_select_idle_state(dev);
+               omap44xx_prm_reconfigure_io_chain();
         }

         return ret;
@@ -596,6 +597,7 @@ static int _od_runtime_resume(struct device *dev)
         struct platform_device *pdev = to_platform_device(dev);

         pinctrl_pm_select_active_state(dev);
+       omap44xx_prm_reconfigure_io_chain();

         omap_device_enable(pdev);

diff --git a/arch/arm/mach-omap2/prm_common.c 
b/arch/arm/mach-omap2/prm_common.c
index 228b850..5db073a 100644
--- a/arch/arm/mach-omap2/prm_common.c
+++ b/arch/arm/mach-omap2/prm_common.c
@@ -79,7 +79,7 @@ static void omap_prcm_events_filter_priority(unsigned 
long *events,
                 events[i] ^= priority_events[i];
         }
  }
-
+#include "prm44xx.h"
  /*
   * PRCM Interrupt Handler
   *
@@ -144,6 +144,7 @@ static void omap_prcm_irq_handler(unsigned int irq, 
struct irq_desc *desc)
         chip->irq_unmask(&desc->irq_data);

         prcm_irq_setup->ocp_barrier(); /* avoid spurious IRQs */
+       omap44xx_prm_reconfigure_io_chain();
  }

  reply	other threads:[~2013-07-18  8:56 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-17 11:41 [PATCH 0/3] ARM: OMAP2+: omap_device: add dynamic pinctrl handling Grygorii Strashko
2013-07-17 11:41 ` Grygorii Strashko
2013-07-17 11:41 ` Grygorii Strashko
2013-07-17 11:41 ` [PATCH 1/3] pinctrl: rollback check for !dev->pins in pinctrl_pm_select*() APIs Grygorii Strashko
2013-07-17 11:41   ` Grygorii Strashko
2013-07-17 11:41   ` Grygorii Strashko
2013-07-17 15:40   ` Tony Lindgren
2013-07-17 15:40     ` Tony Lindgren
2013-07-26 23:22     ` Linus Walleij
2013-07-26 23:22       ` Linus Walleij
2013-07-26 23:22       ` Linus Walleij
2013-07-17 11:41 ` [PATCH 2/3] ARM: OMAP2+: omap_device: add pinctrl handling Grygorii Strashko
2013-07-17 11:41   ` Grygorii Strashko
2013-07-17 11:41   ` Grygorii Strashko
2013-07-17 15:38   ` Tony Lindgren
2013-07-17 15:38     ` Tony Lindgren
2013-07-17 16:56     ` Grygorii Strashko
2013-07-17 16:56       ` Grygorii Strashko
2013-07-17 16:56       ` Grygorii Strashko
2013-07-18  8:14       ` Tony Lindgren
2013-07-18  8:14         ` Tony Lindgren
2013-07-18 11:22         ` Grygorii Strashko
2013-07-18 11:22           ` Grygorii Strashko
2013-07-18 11:22           ` Grygorii Strashko
2013-07-17 11:41 ` [PATCH 3/3] ARM: dts: omap4-sdp: add dynamic pin states for uart3/4 Grygorii Strashko
2013-07-17 11:41   ` Grygorii Strashko
2013-07-17 11:41   ` Grygorii Strashko
2013-07-17 15:32   ` Tony Lindgren
2013-07-17 15:32     ` Tony Lindgren
2013-07-17 15:32     ` Tony Lindgren
2013-07-17 16:41     ` Grygorii Strashko
2013-07-17 16:41       ` Grygorii Strashko
2013-07-17 16:41       ` Grygorii Strashko
2013-07-18  8:09       ` Tony Lindgren
2013-07-18  8:09         ` Tony Lindgren
2013-07-18  8:09         ` Tony Lindgren
2013-07-18  8:54         ` Grygorii Strashko [this message]
2013-07-18  8:54           ` Grygorii Strashko
2013-07-18  8:54           ` Grygorii Strashko
2013-07-18  9:04           ` Tony Lindgren
2013-07-18  9:04             ` Tony Lindgren
2013-07-18 12:01             ` Grygorii Strashko
2013-07-18 12:01               ` Grygorii Strashko
2013-07-18 12:01               ` Grygorii Strashko
2013-07-17 11:57 ` [PATCH 0/3] ARM: OMAP2+: omap_device: add dynamic pinctrl handling Roger Quadros
2013-07-17 11:57   ` Roger Quadros
2013-07-17 11:57   ` Roger Quadros
2013-07-17 12:30   ` Grygorii Strashko
2013-07-17 12:30     ` Grygorii Strashko
2013-07-17 12:30     ` Grygorii Strashko
2013-07-18  6:44     ` Roger Quadros
2013-07-18  6:44       ` Roger Quadros
2013-07-18  6:44       ` Roger Quadros

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=51E7AD38.6010108@ti.com \
    --to=grygorii.strashko@ti.com \
    --cc=benoit.cousson@linaro.org \
    --cc=devicetree-discuss@lists.ozlabs.org \
    --cc=khilman@linaro.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=swarren@wwwdotorg.org \
    --cc=tony@atomide.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.