Re: [PATCH 2/7] gpio: omap: fix error handling in omap_gpio_irq_type
Hi Linus,
On 06/02/2015 05:27 PM, grygorii.stras...@linaro.org wrote:
On 06/02/2015 12:40 PM, Javier Martinez Canillas wrote:
Hello Grygorii,
On Fri, May 22, 2015 at 4:35 PM, Grygorii Strashko
grygorii.stras...@linaro.org wrote:
The GPIO bank will be kept powered in case if input parameters
are invalid or error occurred in omap_gpio_irq_type.
Hence, fix it by ensuring that GPIO bank will be unpowered
in case of errors and add additional check of value returned
from omap_set_gpio_triggering().
Signed-off-by: Grygorii Strashko grygorii.stras...@linaro.org
---
drivers/gpio/gpio-omap.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index bb60cbc..f6cc638 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -488,9 +488,6 @@ static int omap_gpio_irq_type(struct irq_data *d,
unsigned type)
unsigned long flags;
unsigned offset = d-hwirq;
- if (!BANK_USED(bank))
- pm_runtime_get_sync(bank-dev);
-
if (type ~IRQ_TYPE_SENSE_MASK)
return -EINVAL;
@@ -498,12 +495,18 @@ static int omap_gpio_irq_type(struct irq_data
*d, unsigned type)
(type (IRQ_TYPE_LEVEL_LOW|IRQ_TYPE_LEVEL_HIGH)))
return -EINVAL;
+ if (!BANK_USED(bank))
+ pm_runtime_get_sync(bank-dev);
+
spin_lock_irqsave(bank-lock, flags);
retval = omap_set_gpio_triggering(bank, offset, type);
+ if (retval)
At this point the bank-lock spinlock is held so you need to add a
spin_unlock_irqrestore() in the error path.
Ops. Thanks for catching it.
+ goto error;
omap_gpio_init_irq(bank, offset);
if (!omap_gpio_is_input(bank, offset)) {
spin_unlock_irqrestore(bank-lock, flags);
- return -EINVAL;
+ retval = -EINVAL;
+ goto error;
}
spin_unlock_irqrestore(bank-lock, flags);
@@ -512,6 +515,11 @@ static int omap_gpio_irq_type(struct irq_data
*d, unsigned type)
else if (type (IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_EDGE_RISING))
__irq_set_handler_locked(d-irq, handle_edge_irq);
+ return 0;
+
+error:
+ if (!BANK_USED(bank))
+ pm_runtime_put(bank-dev);
return retval;
}
--
But you are correct about the runtime PM bug so after addressing the
above comment, feel free to add:
Acked-by: Javier Martinez Canillas jav...@dowhile0.org
Linus, How do prefer me to fix it?
Resend whole patch or send additional fix on top of patch 5.
Below is the additional patch to fix issue reported by Javier.
Pls. inform me if you would like me to send v2 of this patch instead.
--
regards,
-grygorii
--
From 611cdfe480f095136e55f155bb71aeb09ea994b0 Mon Sep 17 00:00:00 2001
From: Grygorii Strashko grygorii.stras...@linaro.org
Date: Wed, 3 Jun 2015 19:15:19 +0300
Subject: [PATCH] gpio: omap: add missed spin_unlock_irqrestore in
omap_gpio_irq_type
Add missed spin_unlock_irqrestore in omap_gpio_irq_type when
omap_set_gpio_triggering() is failed.
This fixes patch 'gpio: omap: fix error handling in omap_gpio_irq_type'
Reported-by: Javier Martinez Canillas jav...@dowhile0.org
Signed-off-by: Grygorii Strashko grygorii.stras...@linaro.org
---
drivers/gpio/gpio-omap.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index e26dc40..e3f8205 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -490,8 +490,10 @@ static int omap_gpio_irq_type(struct irq_data *d, unsigned
type)
spin_lock_irqsave(bank-lock, flags);
retval = omap_set_gpio_triggering(bank, offset, type);
- if (retval)
+ if (retval) {
+ spin_unlock_irqrestore(bank-lock, flags);
goto error;
+ }
spin_unlock_irqrestore(bank-lock, flags);
if (type (IRQ_TYPE_LEVEL_LOW | IRQ_TYPE_LEVEL_HIGH))
--
1.9.1
--
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 2/7] gpio: omap: fix error handling in omap_gpio_irq_type
On 06/02/2015 12:40 PM, Javier Martinez Canillas wrote:
Hello Grygorii,
On Fri, May 22, 2015 at 4:35 PM, Grygorii Strashko
grygorii.stras...@linaro.org wrote:
The GPIO bank will be kept powered in case if input parameters
are invalid or error occurred in omap_gpio_irq_type.
Hence, fix it by ensuring that GPIO bank will be unpowered
in case of errors and add additional check of value returned
from omap_set_gpio_triggering().
Signed-off-by: Grygorii Strashko grygorii.stras...@linaro.org
---
drivers/gpio/gpio-omap.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index bb60cbc..f6cc638 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -488,9 +488,6 @@ static int omap_gpio_irq_type(struct irq_data *d, unsigned
type)
unsigned long flags;
unsigned offset = d-hwirq;
- if (!BANK_USED(bank))
- pm_runtime_get_sync(bank-dev);
-
if (type ~IRQ_TYPE_SENSE_MASK)
return -EINVAL;
@@ -498,12 +495,18 @@ static int omap_gpio_irq_type(struct irq_data *d,
unsigned type)
(type (IRQ_TYPE_LEVEL_LOW|IRQ_TYPE_LEVEL_HIGH)))
return -EINVAL;
+ if (!BANK_USED(bank))
+ pm_runtime_get_sync(bank-dev);
+
spin_lock_irqsave(bank-lock, flags);
retval = omap_set_gpio_triggering(bank, offset, type);
+ if (retval)
At this point the bank-lock spinlock is held so you need to add a
spin_unlock_irqrestore() in the error path.
Ops. Thanks for catching it.
+ goto error;
omap_gpio_init_irq(bank, offset);
if (!omap_gpio_is_input(bank, offset)) {
spin_unlock_irqrestore(bank-lock, flags);
- return -EINVAL;
+ retval = -EINVAL;
+ goto error;
}
spin_unlock_irqrestore(bank-lock, flags);
@@ -512,6 +515,11 @@ static int omap_gpio_irq_type(struct irq_data *d, unsigned
type)
else if (type (IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_EDGE_RISING))
__irq_set_handler_locked(d-irq, handle_edge_irq);
+ return 0;
+
+error:
+ if (!BANK_USED(bank))
+ pm_runtime_put(bank-dev);
return retval;
}
--
But you are correct about the runtime PM bug so after addressing the
above comment, feel free to add:
Acked-by: Javier Martinez Canillas jav...@dowhile0.org
Linus, How do prefer me to fix it?
Resend whole patch or send additional fix on top of patch 5.
--
regards,
-grygorii
--
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: [RFC/RFT PATCH v2 7/7] gpio: omap: ensure that runtime pm will disable unused gpio banks
On 05/25/2015 05:46 PM, grygorii.stras...@linaro.org wrote: Hi Tony, On 05/22/2015 09:10 PM, Tony Lindgren wrote: * Grygorii Strashko grygorii.stras...@linaro.org [150522 07:37]: Now there are some points related to Runtime PM usage: 1) bank state doesn't need to be checked in places where Rintime PM is used, bacause Runtime PM maintains its own usage counter: if (!BANK_USED(bank)) pm_runtime_get_sync(bank-dev); so, it's safe to drop such checks. 2) Such implementation is racy, because check !BANK_USED(bank) is not protected, like: CPU0CPU1 gpio_request(bankX.A) ... gpio_free(bankX.A)gpio_request(bankX.Y) and bankX can be unpowered in the middle of processing gpio_request(bankX.Y) 3) There is a call of pm_runtime_get_sync() in omap_gpio_irq_type(), but no corresponding put. As result, GPIO devices could be powered on forever if at least one GPIO was used as IRQ. Hence, allow powering off GPIO banks by adding missed pm_runtime_put(bank-dev) at the end of omap_gpio_irq_type(). As, part of this change update omap2_gpio__idle() functions to use pm_runtime_force_suspend()/pm_runtime_force_resume(). Signed-off-by: Grygorii Strashko grygorii.stras...@linaro.org --- Changes in v2: - omap2_gpio__idle() functions switched to use pm_runtime_force_suspend()/pm_runtime_force_resume() API. v1: http://marc.info/?l=linux-gpiom=142567003515626w=2 This one causes an abort during boot on at least omap3. Maybe try to get a beagleboard xm to test with? It has Ethernet over USB though, so that does not work with PM over NFSroot. If you want something to test with PM with mainline over NFSroot, maybe you can get hold of some of the better supported ones out of these boards: $ git grep ethernet@gpmc arch/arm/boot/dts/*.dts* For those Ethernet has a GPIO interrupt ;) Sure, I'll try get beagleboard. But, this log is very interesting :( What I can see from it is that GPIO IRQ is triggered in the middle of omap_sram_idle() - which shouldn't happen, because this is cpuidle path and local IRQs should be disabled. Am I missing smth? Yep. I've missed this :( pm_runtime_force_suspend |- pm_runtime_disable |-__pm_runtime_disable |- spin_unlock_irq(dev-power.lock); So, Runtime PM forced API can't be used in cpuidle path :( Sorry. -- regards, -grygorii -- 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: [RFC/RFT PATCH v2 7/7] gpio: omap: ensure that runtime pm will disable unused gpio banks
Hi Tony, On 05/22/2015 09:10 PM, Tony Lindgren wrote: * Grygorii Strashko grygorii.stras...@linaro.org [150522 07:37]: Now there are some points related to Runtime PM usage: 1) bank state doesn't need to be checked in places where Rintime PM is used, bacause Runtime PM maintains its own usage counter: if (!BANK_USED(bank)) pm_runtime_get_sync(bank-dev); so, it's safe to drop such checks. 2) Such implementation is racy, because check !BANK_USED(bank) is not protected, like: CPU0 CPU1 gpio_request(bankX.A) ... gpio_free(bankX.A)gpio_request(bankX.Y) and bankX can be unpowered in the middle of processing gpio_request(bankX.Y) 3) There is a call of pm_runtime_get_sync() in omap_gpio_irq_type(), but no corresponding put. As result, GPIO devices could be powered on forever if at least one GPIO was used as IRQ. Hence, allow powering off GPIO banks by adding missed pm_runtime_put(bank-dev) at the end of omap_gpio_irq_type(). As, part of this change update omap2_gpio__idle() functions to use pm_runtime_force_suspend()/pm_runtime_force_resume(). Signed-off-by: Grygorii Strashko grygorii.stras...@linaro.org --- Changes in v2: - omap2_gpio__idle() functions switched to use pm_runtime_force_suspend()/pm_runtime_force_resume() API. v1: http://marc.info/?l=linux-gpiom=142567003515626w=2 This one causes an abort during boot on at least omap3. Maybe try to get a beagleboard xm to test with? It has Ethernet over USB though, so that does not work with PM over NFSroot. If you want something to test with PM with mainline over NFSroot, maybe you can get hold of some of the better supported ones out of these boards: $ git grep ethernet@gpmc arch/arm/boot/dts/*.dts* For those Ethernet has a GPIO interrupt ;) Sure, I'll try get beagleboard. But, this log is very interesting :( What I can see from it is that GPIO IRQ is triggered in the middle of omap_sram_idle() - which shouldn't happen, because this is cpuidle path and local IRQs should be disabled. Am I missing smth? [6.150238] Unhandled fault: external abort on non-linefetch (0x1028) at 0xfb05601c [6.158355] pgd = c0004000 [6.161224] [fb05601c] *pgd=49011452(bad) [6.165496] Internal error: : 1028 [#1] SMP ARM [6.170288] Modules linked in: [6.173522] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.1.0-rc4-next-20150522-00021-g6d8613e #94 [6.182800] Hardware name: Generic OMAP36xx (Flattened Device Tree) [6.189422] task: c09b0678 ti: c09aa000 task.ti: c09aa000 [6.195129] PC is at omap_gpio_irq_handler+0x9c/0x234 [6.200469] LR is at trace_hardirqs_off+0x14/0x18 [6.205444] pc : [c03d9b68]lr : [c0095924]psr: a193 [6.205444] sp : c09abcf0 ip : c09abca8 fp : c09abd2c [6.217529] r10: c06786c0 r9 : fb056000 r8 : [6.223052] r7 : c59fa010 r6 : c5816300 r5 : 0001 r4 : c59fa084 [6.229949] r3 : r2 : fb05601c r1 : c0a2461c r0 : 001c [6.236846] Flags: NzCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment kernel [6.244628] Control: 10c5387d Table: 80004019 DAC: 0015 [6.250701] Process swapper/0 (pid: 0, stack limit = 0xc09aa218) [6.257049] Stack: (0xc09abcf0 to 0xc09ac000) [6.261657] bce0: c09abd1c c5816300 c5802c64 fb056018 [6.270294] bd00: 0031 0031 c09adacc 0001 c5802000 c06786c0 [6.278900] bd20: c09abd44 c09abd30 c00a674c c03d9ad8 0177 c09a6110 c09abd6c c09abd48 [6.287536] bd40: c00a6aa0 c00a6720 c12536c0 c09abd98 c0a22f40 0021 0001 0001 [6.296173] bd60: c09abd94 c09abd70 c0009510 c00a6a38 1bb6 c0671c0c 2113 [6.304809] bd80: c09abdcc 0001 c09abdf4 c09abd98 c06725e4 c0009464 0001 0001 [6.313446] bda0: a113 c09c610c a113 0001 0001 0001 [6.322082] bdc0: c06786c0 c09abdf4 c09abdb0 c09abde0 c0098d44 c0671c0c 2113 [6.330718] bde0: c09c60ac c09c610c c09abe14 c09abdf8 c002cd60 c0671bd4 c59ff200 [6.339355] be00: c59ff240 c09abe2c c09abe18 c002e21c c002cd2c c5a03010 [6.347961] be20: c09abe44 c09abe30 c002e288 c002e1e0 c5a03010 c0a245dc c09abe5c c09abe48 [6.356597] be40: c0446588 c002e268 c59fa010 c0a245dc c09abe7c c09abe60 c03da610 c044654c [6.365234] be60: fa306ae0 c0a64d28 0003 c09abea4 c09abe80 c0030d4c c03da5cc [6.373870] be80: 0001 0008 0002 c0a64d68 c06786b8 0001 c09abedc c09abea8 [6.382507] bea0: c00323a4 c0030c1c 65a0bc00 0001 c0513e94 c09b3af4 6b46e923 [6.391143] bec0: 0001 c6e75380 0003 c09abf2c c09abee0 c0512550 c003229c [6.399780] bee0: c09b3af4 14a4 6b46e923 0001 c09abf00 6b46e923 0001 [6.408386] bf00: c09b3af4 c6e75380 0003 c6e75380 c09b3af4 c09adacc c0679fa8 [6.417022
Re: Calling irq_set_irq_wake() from .set_irq_wake()?
On 05/18/2015 05:31 PM, Thomas Gleixner wrote:
On Sun, 17 May 2015, Geert Uytterhoeven wrote:
At least the recursive locking message no longer appears after the revert.
[ 30.591905] PM: Syncing filesystems ... done.
[ 30.623060] Freezing user space processes ... (elapsed 0.003 seconds)
done.
[ 30.634470] Freezing remaining freezable tasks ... (elapsed 0.002
seconds) done.
[ 30.658288] sd 0:0:0:0: [sda] Synchronizing SCSI cache
[ 30.663678]
[ 30.663681] =
[ 30.663683] [ INFO: possible recursive locking detected ]
[ 30.663688] 4.1.0-rc3 #1115 Not tainted
[ 30.663693] -
[ 30.663697] suspend.sh/2319 is trying to acquire lock:
[ 30.663719] (class){..}, at: [c0096ebc]
__irq_get_desc_lock+0x48/0x88
[ 30.663722]
[ 30.663722] but task is already holding lock:
[ 30.663734] (class){..}, at: [c0096ebc]
__irq_get_desc_lock+0x48/0x88
Does this mean .set_irq_wake() cannot call irq_set_irq_wake()?
It can call it, if it's guaranteed that this wont deadlock.
To tell lockdep that you sure about that, you need to set a different
lock class for the child interrupts. irq_set_lockdep_class() is what
you want to use here.
Hm. Seems we already have corresponding call in gpiochip_irq_map:
static int gpiochip_irq_map(struct irq_domain *d, unsigned int irq,
irq_hw_number_t hwirq)
{
struct gpio_chip *chip = d-host_data;
irq_set_chip_data(irq, chip);
irq_set_lockdep_class(irq, gpiochip_irq_lock_class);
commit e45d1c80c0eee88e82751461e9cac49d9ed287bc
Author: Linus Walleij linus.wall...@linaro.org
Date: Tue Apr 22 14:01:46 2014 +0200
gpio: put GPIO IRQs into their own lock clas
added in Kernel v3.16
Roger, can you confirm that you've observed this issue with latest kernel, pls?
Many GPIO drivers do that, as they need to propagate wake-up state to the
parent interrupt controller?
As I remember, there was similar problem, so I found corresponding patch
(just FYI)
ab2b926 mfd: Fix twl6030 lockdep recursion warning on setting wake IRQs
Not sure such kind of solution is the best choice (
That looks like a convoluted solution...
Indeed. See above.
--
regards,
-grygorii
--
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/1] gpio: omap: Fix PM runtime issue and remove most BANK_USED macros
Hi Tony,
Sorry for delayed reply.
On 04/23/2015 05:39 PM, Tony Lindgren wrote:
* grygorii.stras...@linaro.org grygorii.stras...@linaro.org [150423 04:13]:
On 04/21/2015 07:08 PM, Tony Lindgren wrote:
@@ -438,11 +447,30 @@ static void omap_enable_gpio_module(struct gpio_bank
*bank, unsigned offset)
writel_relaxed(ctrl, reg);
bank-context.ctrl = ctrl;
}
+
+ if (is_irq) {
+ omap_set_gpio_direction(bank, offset, 1);
+ bank-irq_usage |= BIT(offset);
+ } else {
+ omap_set_gpio_triggering(bank, offset, IRQ_TYPE_NONE);
+ bank-mod_usage |= BIT(offset);
+ }
The OMAP GPIO driver implements two Core interfaces IRQ-chip and GPIO-chip
which, in general,
more or less independent.
So, I don't think, that it's good to mix GPIO-IRQ-chip specific code with
GPIO-chip code.
And this even don't really correspond the purpose of
omap_enable_gpio_module() :( and might
introduce misunderstanding of code. The worst thing is that future fixes in
IRQ-chip may
affect on on GPIO-chip and vise versa :(
Hmm I'm thinking omap_enable/disable_gpio_module() eventually becomes
our runtime_resume/suspend(). Currently the enabling and disabling is
buggy for GPIO for some corner cases.. AFAIK the only difference between
It might be very helpful if you'd able to share additional info about
any corner cases you know.
enabling GPIO vs GPIO-IRQ is the calling of omap_set_gpio_direction
vs omap_set_gpio_triggering. Or at least that's the way it should be
unless I'm missing something?
I think yes. what i'd like to say, first of all, is that it might be not good
idea to mix
too much functionality in omap_enable/disable_gpio_module() - especially
GPIO-IRQ vs
GPIO-chip ( Very easy to get lost ;)
For example (1) - your change of omap_gpio_request() looks like invalid:
static int omap_gpio_request(struct gpio_chip *chip, unsigned offset)
{
struct gpio_bank *bank = container_of(chip, struct gpio_bank, chip);
- unsigned long flags;
- /*
-* If this is the first gpio_request for the bank,
-* enable the bank module.
-*/
- if (!BANK_USED(bank))
- pm_runtime_get_sync(bank-dev);
-
- spin_lock_irqsave(bank-lock, flags);
- /* Set trigger to none. You need to enable the desired trigger with
-* request_irq() or set_irq_type(). Only do this if the IRQ line has
-* not already been requested.
-*/
- if (!LINE_USED(bank-irq_usage, offset)) {
- omap_set_gpio_triggering(bank, offset, IRQ_TYPE_NONE);
- omap_enable_gpio_module(bank, offset);
^^ above two line should be executed only if GPIO line was not
requested as IRQ before
- }
- bank-mod_usage |= BIT(offset);
- spin_unlock_irqrestore(bank-lock, flags);
+ omap_enable_gpio_module(bank, offset, false);
^^ after your change, it looks like omap_set_gpio_triggering(bank, offset,
IRQ_TYPE_NONE)
will be called always and GPIO triggering configuration might be lost
return 0;
}
Example (2)
I've found commit 55b6019ae294 OMAP: GPIO: clear/restore level/edge detect
settings on mask/unmask
which does the following
gpio_mask_irq()
|- _set_gpio_triggering(bank, get_gpio_index(gpio), IRQ_TYPE_NONE);
gpio_unmask_irq()
|- u32 trigger = irqd_get_trigger_type(d);
if (trigger)
omap_set_gpio_triggering(bank, offset, trigger);
As result, it seems unreasonable to physically configure IRQ triggering type
in GPIO bank registers
inside omap_gpio_irq_type(). Of course, such assumption should be double
checked taking into account that
__irq_set_trigger() can be called any time even before request_irq().
Also, seems the same could be applied to omap_enable_gpio_module and
omap_set_gpio_direction and they
could be removed from omap_gpio_irq_type().
Could we keep omap_xxx_gpio_module() functions responsible only for GPIO
bank PM and
enabling/disabling?
If you're thinking about just thinking about having separate wrappers around
it like this::
static void omap_enable_gpio_module(struct gpio_bank *bank, unsigned offset,
bool is_irq)
{
...
}
static void omap_enable_gpio((struct gpio_bank *bank, unsigned offset)
{
omap_enable_gpio_module(bpio_bank, offset, 0);
}
static void omap_enable_gpio_irq((struct gpio_bank *bank, unsigned offset)
{
omap_enable_gpio_module(bpio_bank, offset, 1);
}
Then yes makes sense to me. Or do you have something else in mind?
Yep. Commented above.
Also, it probably could work if we will set GPIO_CTRL.DISABLEMODULE=1
in omap_gpio_runtime_resume and GPIO_CTRL.DISABLEMODULE=0 in _runtime_suspend,
but it may require from us to split CPUIdle and suspend code path (
--
regards,
-grygorii
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body
Re: [PATCH] mmc: core: add missing pm event in mmc_pm_notify to fix hib restore
On 04/23/2015 04:19 PM, Ulf Hansson wrote: On 23 April 2015 at 12:43, grygorii.stras...@linaro.org wrote: From: Grygorii Strashko grygorii.stras...@linaro.org The PM_RESTORE_PREPARE is not handled now in mmc_pm_notify(), as result mmc_rescan() could be scheduled and executed at late hibernation restore stages when MMC device is suspended already - which, in turn, will lead to system crash on TI dra7-evm board: WARNING: CPU: 0 PID: 3188 at drivers/bus/omap_l3_noc.c:148 l3_interrupt_handler+0x258/0x374() 4400.ocp:L3 Custom Error: MASTER MPU TARGET L4_PER1_P3 (Idle): Data Access in User mode during Functional access Hence, add missed PM_RESTORE_PREPARE PM event in mmc_pm_notify(). Signed-off-by: Grygorii Strashko grygorii.stras...@linaro.org Huh, that was an old bug you found. :-) oh yes. it's happened thanks to HW issue on my board which generates a flood of SDCD IRQs :) It's the worst case ever to track wrong/missed PM notifiers ( I have applied it for fixes and added the below fixes tag. Fixes: 4c2ef25fe0b8 (mmc: fix all hangs related to mmc/sd card insert/removal during suspend/resume) Thanks. -- regards, -grygorii -- 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] gpio: omap: Allow building as a loadable module
On 04/24/2015 02:56 AM, Tony Lindgren wrote:
We currently get all kinds of errors building the omap gpio driver
as a module starting with:
undefined reference to `omap2_gpio_resume_after_idle'
undefined reference to `omap2_gpio_prepare_for_idle'
...
Let's fix the issue by adding inline functions to the header.
Note that we can now also remove the two unused functions for
omap_set_gpio_debounce and omap_set_gpio_debounce_time.
Then doing rmmod on the module produces further warnings
because of missing exit related functions. Let's add those.
And finally, we can make the Kconfig entry just a tristate
option that's selected for omaps.
Reviewed-by: Grygorii Strashko grygorii.stras...@linaro.org
boot tested on: dra7-evm with gpio-omap.ko
Cc: Felipe Balbi ba...@ti.com
Cc: Javier Martinez Canillas jav...@dowhile0.org
Cc: Grygorii Strashko grygorii.stras...@linaro.org
Cc: Kevin Hilman khil...@deeprootsystems.com
Cc: Nishanth Menon n...@ti.com
Cc: Santosh Shilimkar ssant...@kernel.org
Signed-off-by: Tony Lindgren t...@atomide.com
---
drivers/gpio/Kconfig| 2 +-
drivers/gpio/gpio-omap.c| 24
include/linux/platform_data/gpio-omap.h | 12 ++--
3 files changed, 35 insertions(+), 3 deletions(-)
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index caefe80..ff7df95 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -308,7 +308,7 @@ config GPIO_OCTEON
family of SOCs.
config GPIO_OMAP
- bool TI OMAP GPIO support if COMPILE_TEST !ARCH_OMAP2PLUS
+ tristate TI OMAP GPIO support if ARCH_OMAP2PLUS || COMPILE_TEST
default y if ARCH_OMAP
depends on ARM
select GENERIC_IRQ_CHIP
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index b59c3ca..384a852 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1202,6 +1202,17 @@ static int omap_gpio_probe(struct platform_device *pdev)
return 0;
}
+static int omap_gpio_remove(struct platform_device *pdev)
+{
+ struct gpio_bank *bank = platform_get_drvdata(pdev);
+
+ list_del(bank-node);
+ gpiochip_remove(bank-chip);
+ pm_runtime_disable(bank-dev);
+
+ return 0;
+}
+
#ifdef CONFIG_ARCH_OMAP2PLUS
#if defined(CONFIG_PM)
@@ -1387,6 +1398,7 @@ static int omap_gpio_runtime_resume(struct device *dev)
}
#endif /* CONFIG_PM */
+#if IS_BUILTIN(CONFIG_GPIO_OMAP)
void omap2_gpio_prepare_for_idle(int pwr_mode)
{
struct gpio_bank *bank;
@@ -1412,6 +1424,7 @@ void omap2_gpio_resume_after_idle(void)
pm_runtime_get_sync(bank-dev);
}
}
+#endif
#if defined(CONFIG_PM)
static void omap_gpio_init_context(struct gpio_bank *p)
@@ -1567,6 +1580,7 @@ MODULE_DEVICE_TABLE(of, omap_gpio_match);
static struct platform_driver omap_gpio_driver = {
.probe = omap_gpio_probe,
+ .remove = omap_gpio_remove,
.driver = {
.name = omap_gpio,
.pm = gpio_pm_ops,
@@ -1584,3 +1598,13 @@ static int __init omap_gpio_drv_reg(void)
return platform_driver_register(omap_gpio_driver);
}
postcore_initcall(omap_gpio_drv_reg);
+
+static void __exit omap_gpio_exit(void)
+{
+ platform_driver_unregister(omap_gpio_driver);
+}
+module_exit(omap_gpio_exit);
+
+MODULE_DESCRIPTION(omap gpio driver);
+MODULE_ALIAS(platform:gpio-omap);
+MODULE_LICENSE(GPL v2);
diff --git a/include/linux/platform_data/gpio-omap.h
b/include/linux/platform_data/gpio-omap.h
index 5d50b25..cb26181 100644
--- a/include/linux/platform_data/gpio-omap.h
+++ b/include/linux/platform_data/gpio-omap.h
@@ -208,9 +208,17 @@ struct omap_gpio_platform_data {
int (*get_context_loss_count)(struct device *dev);
};
+#if IS_BUILTIN(CONFIG_GPIO_OMAP)
extern void omap2_gpio_prepare_for_idle(int off_mode);
extern void omap2_gpio_resume_after_idle(void);
-extern void omap_set_gpio_debounce(int gpio, int enable);
-extern void omap_set_gpio_debounce_time(int gpio, int enable);
+#else
+static inline void omap2_gpio_prepare_for_idle(int off_mode)
+{
+}
+
+static inline void omap2_gpio_resume_after_idle(void)
+{
+}
+#endif
#endif
--
regards,
-grygorii
--
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] gpio: omap: Fix regression for MPUIO interrupts
On 04/24/2015 02:54 AM, Tony Lindgren wrote:
At some point with all the GPIO clean-up we've broken the
MPUIO interrupts. Those are just a little bit different from
the GPIO interrupts, so we can fix it up just by setting
different irqchip functions for it. And then we can just
remove all old code trying to do the same.
Reviewed-by: Grygorii Strashko grygorii.stras...@linaro.org
Cc: Aaro Koskinen aaro.koski...@iki.fi
Cc: Felipe Balbi ba...@ti.com
Cc: Javier Martinez Canillas jav...@dowhile0.org
Cc: Grygorii Strashko grygorii.stras...@linaro.org
Cc: Kevin Hilman khil...@deeprootsystems.com
Cc: Nishanth Menon n...@ti.com
Cc: Santosh Shilimkar ssant...@kernel.org
Signed-off-by: Tony Lindgren t...@atomide.com
---
drivers/gpio/gpio-omap.c | 48 +---
1 file changed, 9 insertions(+), 39 deletions(-)
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 6553361..b59c3ca 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -1023,38 +1023,8 @@ static void omap_gpio_mod_init(struct gpio_bank *bank)
dev_err(bank-dev, Could not get gpio dbck\n);
}
-static void
-omap_mpuio_alloc_gc(struct gpio_bank *bank, unsigned int irq_start,
- unsigned int num)
-{
- struct irq_chip_generic *gc;
- struct irq_chip_type *ct;
-
- gc = irq_alloc_generic_chip(MPUIO, 1, irq_start, bank-base,
- handle_simple_irq);
- if (!gc) {
- dev_err(bank-dev, Memory alloc failed for gc\n);
- return;
- }
-
- ct = gc-chip_types;
-
- /* NOTE: No ack required, reading IRQ status clears it. */
- ct-chip.irq_mask = irq_gc_mask_set_bit;
- ct-chip.irq_unmask = irq_gc_mask_clr_bit;
- ct-chip.irq_set_type = omap_gpio_irq_type;
-
- if (bank-regs-wkup_en)
- ct-chip.irq_set_wake = omap_gpio_wake_enable;
-
- ct-regs.mask = OMAP_MPUIO_GPIO_INT / bank-stride;
- irq_setup_generic_chip(gc, IRQ_MSK(num), IRQ_GC_INIT_MASK_CACHE,
- IRQ_NOREQUEST | IRQ_NOPROBE, 0);
-}
-
static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc)
{
- int j;
static int gpio;
int irq_base = 0;
int ret;
@@ -1101,6 +1071,15 @@ static int omap_gpio_chip_init(struct gpio_bank *bank,
struct irq_chip *irqc)
}
#endif
+ /* MPUIO is a bit different, reading IRQ status clears it */
+ if (bank-is_mpuio) {
+ irqc-irq_ack = dummy_irq_chip.irq_ack;
+ irqc-irq_mask = irq_gc_mask_set_bit;
+ irqc-irq_unmask = irq_gc_mask_clr_bit;
+ if (!bank-regs-wkup_en)
+ irqc-irq_set_wake = NULL;
+ }
+
ret = gpiochip_irqchip_add(bank-chip, irqc,
irq_base, omap_gpio_irq_handler,
IRQ_TYPE_NONE);
@@ -1114,15 +1093,6 @@ static int omap_gpio_chip_init(struct gpio_bank *bank,
struct irq_chip *irqc)
gpiochip_set_chained_irqchip(bank-chip, irqc,
bank-irq, omap_gpio_irq_handler);
- for (j = 0; j bank-width; j++) {
- int irq = irq_find_mapping(bank-chip.irqdomain, j);
- if (bank-is_mpuio) {
- omap_mpuio_alloc_gc(bank, irq, bank-width);
- irq_set_chip_and_handler(irq, NULL, NULL);
- set_irq_flags(irq, 0);
- }
- }
-
return 0;
}
--
regards,
-grygorii
--
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 V2] drivers/rtc/rtc-ds1307.c: Enable the mcp794xx alarm after programming time
On 04/23/2015 04:11 PM, Nishanth Menon wrote: On 04/23/2015 05:17 AM, grygorii.stras...@linaro.org wrote: On 04/23/2015 03:00 AM, Nishanth Menon wrote: On 04/22/2015 08:26 AM, grygorii.stras...@linaro.org wrote: Hi, On 04/21/2015 03:51 AM, Nishanth Menon wrote: Alarm interrupt enable register is at offset 0x7, while the time registers for the alarm follow that. When we program Alarm interrupt enable prior to programming the time, it is possible that previous time value could be close or match at the time of alarm enable resulting in interrupt trigger which is unexpected (and does not match the time we expect it to trigger). To prevent this scenario from occuring, program the ALM0_EN bit only after the alarm time is appropriately programmed. Ofcourse, I2C programming is non-atomic, so there are loopholes where the interrupt wont trigger if the time requested is in the past at the time of programming the ALM0_EN bit. However, we will not have unexpected interrupts while the time is programmed after the interrupt are enabled. I think it will be nice if you will mention that you going to follow vendor recommendations - AN1491 Configuring the MCP794XX RTCC Family http://ww1.microchip.com/downloads/en/AppNotes/01491A.pdf ;) Also, it is recommended that the alarm registers be loaded before the alarm is enabled. Hmm... i did not know that existed, thanks for digging it up.. that teaches me to look for docs before putting a scope/LA on the board (not that I regret doing that)... That said, reading the app note, I kind of realized: a) that playing with ST bit for programming time is not done, but then, that implies that oscillator will have to be restarted (upto a few seconds for certain crystals).. but that said, it does not seem mandatory or seem to (yet seen) functional issues... b) We dont have flexibility yet to describe if we do indeed have a backup battery or not - VBATEN should be set only if we have a backup battery on the platform :( - on some it might even be optional thanks to certain compliance requirements of shipping boards internationally and general unlike of lithium ion in cargo hold.. c) we dont have capability to control the alarm polarity in the driver which, by the way, we probably should also control OUT polarity (when ALARM is not asserted).. d) we dont have support for external 32k oscillator(X1 only) instead of assuming we always have a 32k crystal(X1 and X2)... Ugghhh... more cleaning up to do for the future.. that said, the sequence it does recommend (in page 4): The following steps show how the Alarm 0 is config- ured. Alarm 1 can be configured in the same manner. 1.Write 0x23 to the Alarm0 Seconds register [0x0A]. 2.Write 0x47 to the Alarm0 Minutes register [0x0B]. 3.Write 0x71 to the Alarm0 Hours register [0x0C] – 11 hours in 12-hour format. 4.Write 0x72 to the Alarm0 Day register [0x0D] – Tuesday + Alarm Polarity Low + Match on all. The Alarm0 Interrupt Flag is also cleared. 5.Write 0x14 to the Alarm0 Date register [0x0E]. 6.Write 0x08 to the Alarm0 Month register [0x0F]. With all the Alarm0 registers set we can now activate the Alarm0 on the Control register. 7.Write 0x10 to the Control register [0x07] – Alarm0 enabled no CLKOUT, Alarm1 disabled before this patch we do ( http://pastebin.ubuntu.com/10863880/) CONTROL r[7] = 0x90 (OUT=1, ALM0EN=1) OSCTRIM r[8] = 0x00 EEUNLOCK r[9] = 0x00 ALM0SEC r[A] = 0x01 ALM0MIN r[B] = 0x45 ALM0HOUR r[C] = 0x23 ALM0WKDAY r[D] = 0x75 -ALMOIF is cleared ALM0DATE r[E] = 0x09 ALM0MTH r[F] = 0x04 RSRVED r[10] = 0x01 with this patch, we do: burst( CONTROL r[7] = 0x80 (OUT=1) OSCTRIM r[8] = 0x00 EEUNLOCK r[9] = 0x00 ALM0SEC r[A] = 0x01 ALM0MIN r[B] = 0x45 ALM0HOUR r[C] = 0x23 ALM0WKDAY r[D] = 0x75 -ALMOIF is cleared ALM0DATE r[E] = 0x09 ALM0MTH r[F] = 0x04 RSRVED r[10] = 0x01 ) CONTROL r[7] = 0x90 (OUT=1, ALM0EN=1) Which is slightly unoptimal way of what the app note recommends. - as I mentioned earlier in this thread, I will try and do optimizations in a later patch. Given that Andrew had picked up this patch, I dont see a reason to respin this yet. but will include the app note for future patches - thanks for pointing it out to me. ^^ Up to you. Np, Always yours! Considering the narrow focus of the current patch (which does fix an issue that it attempts to), can I get an Ack? Reviewed-by: Grygorii Strashko grygorii.stras...@linaro.org -- regards, -grygorii -- 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 V2] drivers/rtc/rtc-ds1307.c: Enable the mcp794xx alarm after programming time
On 04/23/2015 03:00 AM, Nishanth Menon wrote: On 04/22/2015 08:26 AM, grygorii.stras...@linaro.org wrote: Hi, On 04/21/2015 03:51 AM, Nishanth Menon wrote: Alarm interrupt enable register is at offset 0x7, while the time registers for the alarm follow that. When we program Alarm interrupt enable prior to programming the time, it is possible that previous time value could be close or match at the time of alarm enable resulting in interrupt trigger which is unexpected (and does not match the time we expect it to trigger). To prevent this scenario from occuring, program the ALM0_EN bit only after the alarm time is appropriately programmed. Ofcourse, I2C programming is non-atomic, so there are loopholes where the interrupt wont trigger if the time requested is in the past at the time of programming the ALM0_EN bit. However, we will not have unexpected interrupts while the time is programmed after the interrupt are enabled. I think it will be nice if you will mention that you going to follow vendor recommendations - AN1491 Configuring the MCP794XX RTCC Family http://ww1.microchip.com/downloads/en/AppNotes/01491A.pdf ;) Also, it is recommended that the alarm registers be loaded before the alarm is enabled. Hmm... i did not know that existed, thanks for digging it up.. that teaches me to look for docs before putting a scope/LA on the board (not that I regret doing that)... That said, reading the app note, I kind of realized: a) that playing with ST bit for programming time is not done, but then, that implies that oscillator will have to be restarted (upto a few seconds for certain crystals).. but that said, it does not seem mandatory or seem to (yet seen) functional issues... b) We dont have flexibility yet to describe if we do indeed have a backup battery or not - VBATEN should be set only if we have a backup battery on the platform :( - on some it might even be optional thanks to certain compliance requirements of shipping boards internationally and general unlike of lithium ion in cargo hold.. c) we dont have capability to control the alarm polarity in the driver which, by the way, we probably should also control OUT polarity (when ALARM is not asserted).. d) we dont have support for external 32k oscillator(X1 only) instead of assuming we always have a 32k crystal(X1 and X2)... Ugghhh... more cleaning up to do for the future.. that said, the sequence it does recommend (in page 4): The following steps show how the Alarm 0 is config- ured. Alarm 1 can be configured in the same manner. 1.Write 0x23 to the Alarm0 Seconds register [0x0A]. 2.Write 0x47 to the Alarm0 Minutes register [0x0B]. 3.Write 0x71 to the Alarm0 Hours register [0x0C] – 11 hours in 12-hour format. 4.Write 0x72 to the Alarm0 Day register [0x0D] – Tuesday + Alarm Polarity Low + Match on all. The Alarm0 Interrupt Flag is also cleared. 5.Write 0x14 to the Alarm0 Date register [0x0E]. 6.Write 0x08 to the Alarm0 Month register [0x0F]. With all the Alarm0 registers set we can now activate the Alarm0 on the Control register. 7.Write 0x10 to the Control register [0x07] – Alarm0 enabled no CLKOUT, Alarm1 disabled before this patch we do ( http://pastebin.ubuntu.com/10863880/) CONTROL r[7] = 0x90 (OUT=1, ALM0EN=1) OSCTRIM r[8] = 0x00 EEUNLOCK r[9] = 0x00 ALM0SEC r[A] = 0x01 ALM0MIN r[B] = 0x45 ALM0HOUR r[C] = 0x23 ALM0WKDAY r[D] = 0x75 -ALMOIF is cleared ALM0DATE r[E] = 0x09 ALM0MTH r[F] = 0x04 RSRVED r[10] = 0x01 with this patch, we do: burst(CONTROL r[7] = 0x80 (OUT=1) OSCTRIM r[8] = 0x00 EEUNLOCK r[9] = 0x00 ALM0SEC r[A] = 0x01 ALM0MIN r[B] = 0x45 ALM0HOUR r[C] = 0x23 ALM0WKDAY r[D] = 0x75 -ALMOIF is cleared ALM0DATE r[E] = 0x09 ALM0MTH r[F] = 0x04 RSRVED r[10] = 0x01 ) CONTROL r[7] = 0x90 (OUT=1, ALM0EN=1) Which is slightly unoptimal way of what the app note recommends. - as I mentioned earlier in this thread, I will try and do optimizations in a later patch. Given that Andrew had picked up this patch, I dont see a reason to respin this yet. but will include the app note for future patches - thanks for pointing it out to me. ^^ Up to you. Np, Always yours! Signed-off-by: Nishanth Menon n...@ti.com --- Changes in v2: - minor typo fix in comments - merged up code that I missed committing in V1: https://patchwork.kernel.org/patch/6245041/ drivers/rtc/rtc-ds1307.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c index 4ffabb322a9a..3cd4783375a5 100644 --- a/drivers/rtc/rtc-ds1307.c +++ b/drivers/rtc/rtc-ds1307.c @@ -742,17 +742,17 @@ static int mcp794xx_set_alarm(struct device *dev, struct rtc_wkalrm *t) regs[6] = ~MCP794XX_BIT_ALMX_IF; /* Set alarm match: second
Re: [PATCH 1/1] gpio: omap: Fix PM runtime issue and remove most BANK_USED macros
Hi Tony,
On 04/21/2015 07:08 PM, Tony Lindgren wrote:
Looks like omap_gpio_irq_type can return early at several places
leaving a GPIO bank enabled without doing pm_runtime_put if wrong
GPIO arguments are passed.
Instead of adding more complicated BANK_USED macros, let's fix the
issue properly. We can pass is_irq flag to omap_enable_gpio_module
and omap_disble_gpio_module. And with that we can remove all the
similar code elsewhere to get rid of most BANK_USED macros.
Note that the reason for the BANK_USED macro is that we need to manage
PM runtime on per GPIO bank basis. In the long run we want to move to
using PM runtime counts for each GPIO line to determine if a GPIO
bank is used. Once we have a solution for omap_enable_gpio_module
and omap_disable_gpio_module, we can remove the remaining BANK_USED
macros.
In general, this approach is ok for me - I've had not able to test this patch,
but
I'm going to take a deeper look on it on Friday or at the beginning of next
week.
But, honestly, there is one thing I really don't like :( see below pls.
Cc: Felipe Balbi ba...@ti.com
Cc: Grygorii Strashko grygorii.stras...@linaro.org
Cc: Javier Martinez Canillas jmarti...@softcrates.net
Cc: Nishanth Menon n...@ti.com
Signed-off-by: Tony Lindgren t...@atomide.com
---
drivers/gpio/gpio-omap.c | 111
+--
1 file changed, 40 insertions(+), 71 deletions(-)
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index d44e617..39a6312 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -86,6 +86,7 @@ struct gpio_bank {
#define BANK_USED(bank) (bank-mod_usage || bank-irq_usage)
#define LINE_USED(line, offset) (line (BIT(offset)))
+static void omap_reset_gpio(struct gpio_bank *bank, unsigned offset);
static void omap_gpio_unmask_irq(struct irq_data *d);
static inline struct gpio_bank *omap_irq_data_get_bank(struct irq_data *d)
@@ -419,8 +420,16 @@ static int omap_set_gpio_triggering(struct gpio_bank
*bank, int gpio,
return 0;
}
-static void omap_enable_gpio_module(struct gpio_bank *bank, unsigned offset)
+static void omap_enable_gpio_module(struct gpio_bank *bank, unsigned offset,
+ bool is_irq)
{
+ unsigned long flags;
+
+ /* PM runtime is per bank, not per GPIO line */
+ if (!BANK_USED(bank))
+ pm_runtime_get_sync(bank-dev);
+
+ spin_lock_irqsave(bank-lock, flags);
if (bank-regs-pinctrl) {
void __iomem *reg = bank-base + bank-regs-pinctrl;
@@ -438,11 +447,30 @@ static void omap_enable_gpio_module(struct gpio_bank
*bank, unsigned offset)
writel_relaxed(ctrl, reg);
bank-context.ctrl = ctrl;
}
+
+ if (is_irq) {
+ omap_set_gpio_direction(bank, offset, 1);
+ bank-irq_usage |= BIT(offset);
+ } else {
+ omap_set_gpio_triggering(bank, offset, IRQ_TYPE_NONE);
+ bank-mod_usage |= BIT(offset);
+ }
The OMAP GPIO driver implements two Core interfaces IRQ-chip and GPIO-chip
which, in general,
more or less independent.
So, I don't think, that it's good to mix GPIO-IRQ-chip specific code with
GPIO-chip code.
And this even don't really correspond the purpose of omap_enable_gpio_module()
:( and might
introduce misunderstanding of code. The worst thing is that future fixes in
IRQ-chip may
affect on on GPIO-chip and vise versa :(
Could we keep omap_xxx_gpio_module() functions responsible only for GPIO bank
PM and
enabling/disabling?
+ spin_unlock_irqrestore(bank-lock, flags);
}
-static void omap_disable_gpio_module(struct gpio_bank *bank, unsigned offset)
+static void omap_disable_gpio_module(struct gpio_bank *bank, unsigned offset,
+ bool is_irq)
{
void __iomem *base = bank-base;
+ unsigned long flags;
+
+ spin_lock_irqsave(bank-lock, flags);
+ if (is_irq)
+ bank-irq_usage = ~(BIT(offset));
+ else
+ bank-mod_usage = ~(BIT(offset));
+
+ omap_reset_gpio(bank, offset);
if (bank-regs-wkup_en
!LINE_USED(bank-mod_usage, offset)
@@ -463,6 +491,11 @@ static void omap_disable_gpio_module(struct gpio_bank
*bank, unsigned offset)
writel_relaxed(ctrl, reg);
bank-context.ctrl = ctrl;
}
+ spin_unlock_irqrestore(bank-lock, flags);
+
+ /* PM runtime is per bank, not per GPIO line */
+ if (!BANK_USED(bank))
+ pm_runtime_put(bank-dev);
}
static int omap_gpio_is_input(struct gpio_bank *bank, unsigned offset)
@@ -472,15 +505,6 @@ static int omap_gpio_is_input(struct gpio_bank *bank,
unsigned offset)
return readl_relaxed(reg) BIT(offset);
}
-static void omap_gpio_init_irq(struct gpio_bank *bank, unsigned offset)
-{
- if (!LINE_USED(bank-mod_usage, offset
Re: [PATCH V2] drivers/rtc/rtc-ds1307.c: Enable the mcp794xx alarm after programming time
Hi, On 04/21/2015 03:51 AM, Nishanth Menon wrote: Alarm interrupt enable register is at offset 0x7, while the time registers for the alarm follow that. When we program Alarm interrupt enable prior to programming the time, it is possible that previous time value could be close or match at the time of alarm enable resulting in interrupt trigger which is unexpected (and does not match the time we expect it to trigger). To prevent this scenario from occuring, program the ALM0_EN bit only after the alarm time is appropriately programmed. Ofcourse, I2C programming is non-atomic, so there are loopholes where the interrupt wont trigger if the time requested is in the past at the time of programming the ALM0_EN bit. However, we will not have unexpected interrupts while the time is programmed after the interrupt are enabled. I think it will be nice if you will mention that you going to follow vendor recommendations - AN1491 Configuring the MCP794XX RTCC Family http://ww1.microchip.com/downloads/en/AppNotes/01491A.pdf ;) Also, it is recommended that the alarm registers be loaded before the alarm is enabled. Signed-off-by: Nishanth Menon n...@ti.com --- Changes in v2: - minor typo fix in comments - merged up code that I missed committing in V1: https://patchwork.kernel.org/patch/6245041/ drivers/rtc/rtc-ds1307.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c index 4ffabb322a9a..3cd4783375a5 100644 --- a/drivers/rtc/rtc-ds1307.c +++ b/drivers/rtc/rtc-ds1307.c @@ -742,17 +742,17 @@ static int mcp794xx_set_alarm(struct device *dev, struct rtc_wkalrm *t) regs[6] = ~MCP794XX_BIT_ALMX_IF; /* Set alarm match: second, minute, hour, day, date, month. */ regs[6] |= MCP794XX_MSK_ALMX_MATCH; - - if (t-enabled) - regs[0] |= MCP794XX_BIT_ALM0_EN; - else - regs[0] = ~MCP794XX_BIT_ALM0_EN; + /* Disable interrupt. We will not enable until completely programmed */ + regs[0] = ~MCP794XX_BIT_ALM0_EN; ret = ds1307-write_block_data(client, MCP794XX_REG_CONTROL, 10, regs); if (ret 0) return ret; - return 0; + if (!t-enabled) + return 0; + regs[0] |= MCP794XX_BIT_ALM0_EN; + return i2c_smbus_write_byte_data(client, MCP794XX_REG_CONTROL, regs[0]); So, It seems, that right sequence should be: - disable alarmX - read alarmX regs - configure alarmX regs - load alarmX regs - enable alarmX More over, looks like, alarm/alarm IRQ should be enabled/disabled separately from set_alarm/RTC_ALM_SET by RTC_AIE_ON, RTC_AIE_OFF. Should it? } static int mcp794xx_alarm_irq_enable(struct device *dev, unsigned int enabled) -- regards, -grygorii -- 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: [RFC/RFT PATCH 2/2] gpio: omap: ensure that runtime pm will disable unused gpio banks
Hi Tony,
On 04/16/2015 07:29 PM, Tony Lindgren wrote:
* Tony Lindgren t...@atomide.com [150319 16:08]:
* Tony Lindgren t...@atomide.com [150307 00:08]:
* grygorii.stras...@linaro.org grygorii.stras...@linaro.org [150306
11:27]:
From: Grygorii Strashko grygorii.stras...@linaro.org
Now there are two points related to Runtime PM usage:
1) bank state doesn't need to be checked in places where
Rintime PM is used, bacause Runtime PM maintains its
own usage counter:
if (!BANK_USED(bank))
pm_runtime_get_sync(bank-dev);
so, it's safe to drop such checks.
2) There is a call of pm_runtime_get_sync() in omap_gpio_irq_type(),
but no corresponding put. As result, GPIO devices could be
powered on forever if at least one GPIO was used as IRQ.
Hence, allow powering off GPIO banks by adding missed
pm_runtime_put(bank-dev) at the end of omap_gpio_irq_type().
Nice to see this happening, but I think before merging this we need
to test to be sure that the pm_runtime calls actually match.. I'm
not convinced right now.. We may still have uninitialized entry
points similar to 3d009c8c61f9 (gpio: omap: Fix bad device
access with setup_irq()).
OK so I finally got around testing this along with your bank
removal set. Looks like this patch causes a regression at least
with n900 keyboard LEDs with off-idle. The LED won't come back
on after restore from off-idle. Anyways, now we have something
reproducable :) So I'll try to debug it further at some point,
might be few days before I get to it.
Sorry for the delay, finally got around to this one :)
Here's what I came up with, only lightly tested so far. Note that
we need to keep the PM runtime per bank, not per GPIO. Will repost
a proper patch after some more testing.
I've had a opposite idea actually ;) - use Runtime PM on per-gpio basis as
it maintains all needed counters inside and we can be sure that
PM runtime callbacks will be called once per bank.
Also, I've thought about moving the code from omap_enable_gpio_module()
into Runtime PM callbacks.
So, final goal - get rid of BANK_USED LINE_USED.
Were you able to identify broken calls sequence?
Also, Pay attention pls, that omap2_gpio_prepare_for/resume_after_idle()
will be most probably a NOP now.
This should not cause any functional changes, mostly just removal
of code that can all be done in omap_enable/disable_gpio_module.
8 -
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -86,6 +86,7 @@ struct gpio_bank {
#define BANK_USED(bank) (bank-mod_usage || bank-irq_usage)
#define LINE_USED(line, offset) (line (BIT(offset)))
+static void omap_reset_gpio(struct gpio_bank *bank, unsigned offset);
static void omap_gpio_unmask_irq(struct irq_data *d);
static inline struct gpio_bank *omap_irq_data_get_bank(struct irq_data *d)
@@ -419,8 +420,16 @@ static int omap_set_gpio_triggering(struct gpio_bank
*bank, int gpio,
return 0;
}
-static void omap_enable_gpio_module(struct gpio_bank *bank, unsigned offset)
+static void omap_enable_gpio_module(struct gpio_bank *bank, unsigned offset,
+ bool is_irq)
{
+ unsigned long flags;
+
+ /* PM runtime is per bank, not per GPIO line */
+ if (!BANK_USED(bank))
+ pm_runtime_get_sync(bank-dev);
+
+ spin_lock_irqsave(bank-lock, flags);
if (bank-regs-pinctrl) {
void __iomem *reg = bank-base + bank-regs-pinctrl;
@@ -438,11 +447,30 @@ static void omap_enable_gpio_module(struct gpio_bank
*bank, unsigned offset)
writel_relaxed(ctrl, reg);
bank-context.ctrl = ctrl;
}
+
+ if (is_irq) {
+ omap_set_gpio_direction(bank, offset, 1);
+ bank-irq_usage |= BIT(offset);
+ } else {
+ omap_set_gpio_triggering(bank, offset, IRQ_TYPE_NONE);
+ bank-mod_usage |= BIT(offset);
+ }
+ spin_unlock_irqrestore(bank-lock, flags);
}
-static void omap_disable_gpio_module(struct gpio_bank *bank, unsigned offset)
+static void omap_disable_gpio_module(struct gpio_bank *bank, unsigned offset,
+ bool is_irq)
{
void __iomem *base = bank-base;
+ unsigned long flags;
+
+ spin_lock_irqsave(bank-lock, flags);
+ if (is_irq)
+ bank-irq_usage = ~(BIT(offset));
+ else
+ bank-mod_usage = ~(BIT(offset));
+
+ omap_reset_gpio(bank, offset);
if (bank-regs-wkup_en
!LINE_USED(bank-mod_usage, offset)
@@ -463,6 +491,11 @@ static void omap_disable_gpio_module(struct gpio_bank
*bank, unsigned offset)
writel_relaxed(ctrl, reg);
bank-context.ctrl = ctrl;
}
+ spin_unlock_irqrestore(bank-lock, flags);
+
+ /* PM runtime is per bank, not per GPIO line */
+ if (!BANK_USED(bank))
+ pm_runtime_put(bank-dev
Re: [PATCH v2 0/8] gpio: omap: cleanup: get rid of system GPIO - GPIO offset converseations
On 03/23/2015 02:18 PM, grygorii.stras...@linaro.org wrote: From: Grygorii Strashko grygorii.stras...@linaro.org Now in TI OMAP GPIO driver there are a lot of places where System GPIO number calculated and then converted to GPIO offset. What is worse is that in many place such conversation performed twice or even three times. But actually, we don't need to do that at all, because - gpiolib always passes GPIO offset to GPIO controller - OMAP GPIO driver converted to use IRQ domain, so struct irq_data-hwirq contains GPIO offset Hence, it is safe to convert all GPIO OMAP functions to use GPIO offset instead of system GPIO numbers. Also, this allows to remove unneeded conversations routines #define GPIO_INDEX(bank, gpio) #define GPIO_BIT(bank, gpio) int omap_irq_to_gpio() Tested on: - dra7-evm. - omap1 (osk5912), 770 and E3. Last two patches have to be tested on OMAP1: - gpio: omap: get rid of omap_irq_to_gpio() - gpio: omap: get rid of GPIO_INDEX() macro Based on top of Linux 4.0-rc4 plus patch '[PATCH 1/2] gpio: omap: irq_shutdown: remove unnecessary call of gpiochip_unlock_as_irq' http://www.spinics.net/lists/linux-omap/msg116482.html Changes in v2: - fixed build failure with Patch 5, no functional code changes. Tested-by: Tony Lindgren t...@atomide.com Tested-by: Aaro Koskinen aaro.koski...@iki.fi Acked-by: Santosh Shilimkar ssant...@kernel.org Acked-by: Javier Martinez Canillas jav...@dowhile0.org Thanks Linus. regards, -grygorii -- regards, -grygorii -- 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] omapdss: extend pm notifier to handle hibernation
Hi Tomi,
On 03/20/2015 02:20 PM, Tomi Valkeinen wrote:
On 25/02/15 19:03, grygorii.stras...@linaro.org wrote:
From: Grygorii Strashko grygorii.stras...@linaro.org
Add handling of missed events in omap_dss_pm_notif which are
needed to support hibernation (suspend to disk).
Signed-off-by: Grygorii Strashko grygorii.stras...@linaro.org
---
drivers/video/fbdev/omap2/dss/core.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/video/fbdev/omap2/dss/core.c
b/drivers/video/fbdev/omap2/dss/core.c
index 6b74f73..e60976a 100644
--- a/drivers/video/fbdev/omap2/dss/core.c
+++ b/drivers/video/fbdev/omap2/dss/core.c
@@ -178,11 +178,15 @@ static int omap_dss_pm_notif(struct notifier_block *b,
unsigned long v, void *d)
DSSDBG(pm notif %lu\n, v);
switch (v) {
+case PM_HIBERNATION_PREPARE:
case PM_SUSPEND_PREPARE:
+case PM_RESTORE_PREPARE:
DSSDBG(suspending displays\n);
return dss_suspend_all_devices();
case PM_POST_SUSPEND:
+case PM_POST_HIBERNATION:
+case PM_POST_RESTORE:
DSSDBG(resuming displays\n);
return dss_resume_all_devices();
Why suspend displays when PM_RESTORE_PREPARE happens? Why resume when
PM_POST_RESTORE happens?
We have following sequence when system is restored from hibernation:
- original kernel booted;
- late_initcall_sync(software_resume);
- pm_notifier_call_chain(PM_RESTORE_PREPARE);
- freeze_processes
- check read hibernation image
- suspend all devices (.freeze())
- jump to stored kernel
- restore devices
...
So, all devices should be in frozen/suspended state when we will jump to stored
kernel
(device's state should be the same as before creating hib image).
Without this patch I can see a lot of log messages like below:
[3.642499] Freezing user space processes ...
[3.647029] mmcblk1boot1: unknown partition table
[3.647043] (elapsed 0.000 seconds) done.
[3.686414] mmcblk1boot0: unknown partition table
[3.714552] PM: Using 1 thread(s) for decompression.
[3.714552] PM: Loading and decompressing image data (144641 pages)...
[4.520388] PM: Image loading progress: 0%
[5.153715] PM: Image loading progress: 10%
[5.847731] PM: Image loading progress: 20%
[6.622024] PM: Image loading progress: 30%
[7.023830] PM: Image loading progress: 40%
[7.455959] PM: Image loading progress: 50%
[8.137186] PM: Image loading progress: 60%
[8.567899] PM: Image loading progress: 70%
[9.670371] PM: Image loading progress: 80%
[ 10.130646] PM: Image loading progress: 90%
[ 10.525035] PM: Image loading progress: 100%
[ 10.529565] PM: Image loading done.
[ 10.533262] PM: Read 578564 kbytes in 6.80 seconds (85.08 MB/s)
[ 10.545313] Suspending console(s) (use no_console_suspend to debug)
[ 193.721284] PM: freeze of devices complete after 7.891 msecs
[ 193.722618] PM: late freeze of devices complete after 1.325 msecs
[ 193.723969] PM: noirq freeze of devices complete after 1.343 msecs
[ 193.724133] Disabling non-boot CPUs ...
[ 193.724792] CPU1: shutdown
[ 193.725387] PM: Creating hibernation image:
[ 193.725387] PM: Need to copy 144499 pages
[ 193.725439] Enabling non-boot CPUs ...
[ 193.725783] CPU1: smp_ops.cpu_die() returned, trying to resuscitate
[ 193.725790] CPU1: Booted secondary processor
[ 193.726069] CPU1 is up
[ 193.743772] PM: noirq restore of devices complete after 17.693 msecs
[ 193.744691] PM: early restore of devices complete after 0.634 msecs
[ 193.951382] [drm:omap_crtc_error_irq] *ERROR* tv: errors: 8000
[ 193.951389] [drm:omap_plane_error_irq] *ERROR* gfx: errors: 0040
[ 193.951402] [drm:omap_crtc_error_irq] *ERROR* tv: errors: 8000
[ 193.951413] [drm:omap_crtc_error_irq] *ERROR* tv: errors: 8000
[ 193.951424] [drm:omap_crtc_error_irq] *ERROR* tv: errors: 8000
[ 193.951435] [drm:omap_crtc_error_irq] *ERROR* tv: errors: 8000
[ 193.951445] [drm:omap_crtc_error_irq] *ERROR* tv: errors: 8000
[ 193.951455] [drm:omap_crtc_error_irq] *ERROR* tv: errors: 8000
[ 193.951465] [drm:omap_crtc_error_irq] *ERROR* tv: errors: 8000
[ 193.951475] [drm:omap_crtc_error_irq] *ERROR* tv: errors: 8000
[ 193.951484] [drm:omap_crtc_error_irq] *ERROR* tv: errors: 8000
[ 193.951567] omap_l3_noc 4400.ocp: L3 application error: target 2 mod:1
(unclearable)
[ 193.951605] omap_l3_noc 4400.ocp: L3 debug error: target 2 mod:1
(unclearable)
[ 194.293226] ata1: SATA link down (SStatus 0 SControl 300)
[ 194.560684] PM: restore of devices complete after 610.740 msecs
--
regards,
-grygorii
--
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] drm/omap: tiler: add hibernation callback
On 03/20/2015 01:43 PM, Tomi Valkeinen wrote:
On 18/03/15 16:56, grygorii.stras...@linaro.org wrote:
Hi All,
On 02/25/2015 08:08 PM, grygorii.stras...@linaro.org wrote:
From: Grygorii Strashko grygorii.stras...@linaro.org
Setting a dev_pm_ops resume callback but not a set of
hibernation handler means that pm function will not be
called upon hibernation.
Fix this by using SIMPLE_DEV_PM_OPS, which appropriately
assigns the suspend and hibernation handlers and move
omap_dmm_resume under CONFIG_PM_SLEEP to avoid build warnings.
Signed-off-by: Grygorii Strashko grygorii.stras...@linaro.org
---
drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
index 56c6055..afb8cfc 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
@@ -941,7 +941,7 @@ error:
}
#endif
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_SLEEP
static int omap_dmm_resume(struct device *dev)
{
struct tcm_area area;
@@ -965,12 +965,10 @@ static int omap_dmm_resume(struct device *dev)
return 0;
}
-
-static const struct dev_pm_ops omap_dmm_pm_ops = {
-.resume = omap_dmm_resume,
-};
#endif
+SIMPLE_DEV_PM_OPS(omap_dmm_pm_ops, NULL, omap_dmm_resume);
+
#if defined(CONFIG_OF)
static const struct of_device_id dmm_of_match[] = {
{ .compatible = ti,omap4-dmm, },
@@ -986,9 +984,7 @@ struct platform_driver omap_dmm_driver = {
.owner = THIS_MODULE,
.name = DMM_DRIVER_NAME,
.of_match_table = of_match_ptr(dmm_of_match),
-#ifdef CONFIG_PM
.pm = omap_dmm_pm_ops,
-#endif
},
};
Any comments on this?
Sorry, I missed this. I'll add it to my omapdrm branch.
Thanks.
--
regards,
-grygorii
--
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 0/8] gpio: omap: cleanup: get rid of system GPIO - GPIO offset converseations
On 03/20/2015 01:00 AM, Tony Lindgren wrote: * grygorii.stras...@linaro.org grygorii.stras...@linaro.org [150319 10:26]: From: Grygorii Strashko grygorii.stras...@linaro.org Now in TI OMAP GPIO driver there are a lot of places where System GPIO number calculated and then converted to GPIO offset. What is worse is that in many place such conversation performed twice or even three times. But actually, we don't need to do that at all, because - gpiolib always passes GPIO offset to GPIO controller - OMAP GPIO driver converted to use IRQ domain, so struct irq_data-hwirq contains GPIO offset Hence, it is safe to convert all GPIO OMAP functions to use GPIO offset instead of system GPIO numbers. Also, this allows to remove unneeded conversations routines #define GPIO_INDEX(bank, gpio) #define GPIO_BIT(bank, gpio) int omap_irq_to_gpio() Right on! This is excellent news. I've tested this set on top of -rc4 plus the patch mentioned below, and I'm not seeing regressions on the platforms I tested with. Wake up to smsc911x ping with off-idle still works on omap3. I only briefly tested on omap1 (osk5912), so I'like to wait for Aaro's ack before this gets merged. I found one build error, other than that, for the whole series please feel free to add: Tested-by: Tony Lindgren t...@atomide.com Thanks Tony. Yep. I'll wait for news from Aaro then will re-send if ok. -- regards, -grygorii -- 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 5/8] gpio: omap: convert gpio irq functions to use GPIO offset
On 03/20/2015 01:03 AM, Tony Lindgren wrote:
* grygorii.stras...@linaro.org grygorii.stras...@linaro.org [150319 10:26]:
From: Grygorii Strashko grygorii.stras...@linaro.org
Convert GPIO IRQ functions to use GPIO offset instead of system
GPIO numbers. This allows to drop unneeded conversations between
system GPIO - GPIO offset which are done in many places and
many times.
It is safe to do now because:
- gpiolib always passes GPIO offset to GPIO controller
- OMAP GPIO driver converted to use IRQ domain, so
struct irq_data-hwirq contains GPIO offset
This is preparation step before removing:
#define GPIO_INDEX(bank, gpio)
#define GPIO_BIT(bank, gpio)
int omap_irq_to_gpio()
...
static void omap_gpio_unmask_irq(struct irq_data *d)
{
struct gpio_bank *bank = omap_irq_data_get_bank(d);
- unsigned int gpio = omap_irq_to_gpio(bank, d-hwirq);
+ unsigned offset = d-hwirq;
unsigned int irq_mask = GPIO_BIT(bank, gpio);
u32 trigger = irqd_get_trigger_type(d);
unsigned long flags;
This series up to this patch produces a build error that
would break git bisect:
drivers/gpio/gpio-omap.c: In function ‘omap_gpio_unmask_irq’:
drivers/gpio/gpio-omap.c:866:37: error: ‘gpio’ undeclared (first use in this
function)
unsigned int irq_mask = GPIO_BIT(bank, gpio);
Oh. Thanks for catching that :) - splitting/rebasing issue.
--
regards,
-grygorii
--
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] omapdss: extend pm notifier to handle hibernation
On 03/20/2015 05:21 PM, Tomi Valkeinen wrote:
On 20/03/15 16:57, grygorii.stras...@linaro.org wrote:
On 03/20/2015 02:20 PM, Tomi Valkeinen wrote:
On 25/02/15 19:03, grygorii.stras...@linaro.org wrote:
From: Grygorii Strashko grygorii.stras...@linaro.org
Add handling of missed events in omap_dss_pm_notif which are
needed to support hibernation (suspend to disk).
Signed-off-by: Grygorii Strashko grygorii.stras...@linaro.org
---
drivers/video/fbdev/omap2/dss/core.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/video/fbdev/omap2/dss/core.c
b/drivers/video/fbdev/omap2/dss/core.c
index 6b74f73..e60976a 100644
--- a/drivers/video/fbdev/omap2/dss/core.c
+++ b/drivers/video/fbdev/omap2/dss/core.c
@@ -178,11 +178,15 @@ static int omap_dss_pm_notif(struct notifier_block
*b, unsigned long v, void *d)
DSSDBG(pm notif %lu\n, v);
switch (v) {
+ case PM_HIBERNATION_PREPARE:
case PM_SUSPEND_PREPARE:
+ case PM_RESTORE_PREPARE:
DSSDBG(suspending displays\n);
return dss_suspend_all_devices();
case PM_POST_SUSPEND:
+ case PM_POST_HIBERNATION:
+ case PM_POST_RESTORE:
DSSDBG(resuming displays\n);
return dss_resume_all_devices();
Why suspend displays when PM_RESTORE_PREPARE happens? Why resume when
PM_POST_RESTORE happens?
We have following sequence when system is restored from hibernation:
- original kernel booted;
- late_initcall_sync(software_resume);
- pm_notifier_call_chain(PM_RESTORE_PREPARE);
- freeze_processes
- check read hibernation image
- suspend all devices (.freeze())
- jump to stored kernel
- restore devices
...
So, all devices should be in frozen/suspended state when we will jump to
stored kernel
(device's state should be the same as before creating hib image).
Without this patch I can see a lot of log messages like below:
Yes, I am sure a fix is needed for hibernation. But I still don't quite
understand PM_RESTORE_PREPARE and PM_POST_RESTORE.
When we enter hibernation, there's only PM_HIBERNATION_PREPARE?
No. There is always PM_POST_HIBERNATION:
- original Kernel in case of failure
- restored Kernel on success
When waking from hibernation, there's first PM_RESTORE_PREPARE, where we
need to disable displays that were enabled during boot. Then either
PM_POST_HIBERNATION if all went well, or PM_POST_RESTORE if there was an
error, and in both cases we want to enable the displays?
Yes.
int hibernate(void)
error = pm_notifier_call_chain(PM_HIBERNATION_PREPARE);
...
error = hibernation_snapshot(hibernation_mode == HIBERNATION_PLATFORM);
^^^ restored kernel will start from here
if (error || freezer_test_done)
goto Free_bitmaps;
if (in_suspend) {
...
pr_debug(PM: writing image.\n);
error = swsusp_write(flags);
swsusp_free();
if (!error)
power_down();
in_suspend = 0;
pm_restore_gfp_mask();
^^ this part will be executed by Kernel requested Hibernation
} else {
pr_debug(PM: Image restored successfully.\n);
^^ this part will be executed by Kernel restored from
Hibernation
}
...
pm_notifier_call_chain(PM_POST_HIBERNATION);
^^^ Both Kernels will be notified here:
- original Kernel in case of failure
- restored Kernel on success
static int software_resume(void)
error = pm_notifier_call_chain(PM_RESTORE_PREPARE);
error = swsusp_read(flags);
swsusp_close(FMODE_READ);
if (!error)
hibernation_restore(flags SF_PLATFORM_MODE);
^^^ if ok we will never return from this function
printk(KERN_ERR PM: Failed to load hibernation image, recovering.\n);
pm_notifier_call_chain(PM_POST_RESTORE);
^^^
if fail - just continue work as usual
--
regards,
-grygorii
--
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: [RFT OMAP1 PATCH 7/8] gpio: omap: get rid of omap_irq_to_gpio()
On 03/20/2015 08:56 PM, Javier Martinez Canillas wrote: Hello Grygorii, On Thu, Mar 19, 2015 at 6:25 PM, grygorii.stras...@linaro.org wrote: From: Grygorii Strashko grygorii.stras...@linaro.org Now OMAP GPIO driver prepared for omap_irq_to_gpio() removing. Do it ;) Signed-off-by: Grygorii Strashko grygorii.stras...@linaro.org I don't have access to OMAP1 boards to test but Tony did it o an osk5912 and Aaro can confirm that it doesn't cause regressions on other OMAP1 boards. Thanks. But could you clarify one point to me please? Aaro can confirm that it doesn't cause regressions on other OMAP1 boards - Does It mean I can add his Tested-by? Or I still have to wait for his direct reply? Just to avoid any confusions ;) Acked-by: Javier Martinez Canillas jav...@dowhile0.org -- regards, -grygorii -- 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 2/2] ARM: dts: dra7: remove ti,hwmod property from pcie phy
On 03/19/2015 05:45 PM, Paul Walmsley wrote: On Thu, 19 Mar 2015, grygorii.stras...@linaro.org wrote: On 03/19/2015 04:55 PM, Paul Walmsley wrote: On Wed, 18 Mar 2015, grygorii.stras...@linaro.org wrote: On 03/18/2015 06:57 PM, Tony Lindgren wrote: * Grygorii Strashko grygorii.stras...@ti.com [150318 09:37]: As I can see Patch 1 from this series was merged in 4.0-rc4, but this patch wasn't. As result, I can see below warning all the time during boot now: [0.594591] platform 4a094000.pciephy: Cannot lookup hwmod 'pcie1-phy' OK. Is this needed as a fix for the v4.0-rc series, or can this wait for v4.1? I think, Yes. These two patches should go all together, because they are interrelated. Does the warning result in a functional problem, or is it just a warning? PCE1 PHY device is not registered any more. How does the second patch fix that? I've re-checked it - sorry for disinfo. PHY devices are created, but omap_device_fail_pm_domain is assigned to them. As result Runtime PM is not working any more. [0.592501] platform 4a094000.pciephy: Cannot lookup hwmod 'pcie1-phy' [0.597510] pinctrl-single 4a003400.pinmux: 281 pins at pa fc003400 size 1124 [0.602794] ti-pipe3 4a094000.pciephy: _od_fail_runtime_resume: FIXME: missing hwmod/omap_dev info When ti,hwmods is not present PCI PHY will be added as regular Platform device and Runtime PM will work again. -- regards, -grygorii -- 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 2/2] ARM: dts: dra7: remove ti,hwmod property from pcie phy
On 03/19/2015 04:55 PM, Paul Walmsley wrote: On Wed, 18 Mar 2015, grygorii.stras...@linaro.org wrote: On 03/18/2015 06:57 PM, Tony Lindgren wrote: * Grygorii Strashko grygorii.stras...@ti.com [150318 09:37]: As I can see Patch 1 from this series was merged in 4.0-rc4, but this patch wasn't. As result, I can see below warning all the time during boot now: [0.594591] platform 4a094000.pciephy: Cannot lookup hwmod 'pcie1-phy' OK. Is this needed as a fix for the v4.0-rc series, or can this wait for v4.1? I think, Yes. These two patches should go all together, because they are interrelated. Does the warning result in a functional problem, or is it just a warning? PCE1 PHY device is not registered any more. -- regards, -grygorii -- 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] drm/omap: tiler: add hibernation callback
Hi All,
On 02/25/2015 08:08 PM, grygorii.stras...@linaro.org wrote:
From: Grygorii Strashko grygorii.stras...@linaro.org
Setting a dev_pm_ops resume callback but not a set of
hibernation handler means that pm function will not be
called upon hibernation.
Fix this by using SIMPLE_DEV_PM_OPS, which appropriately
assigns the suspend and hibernation handlers and move
omap_dmm_resume under CONFIG_PM_SLEEP to avoid build warnings.
Signed-off-by: Grygorii Strashko grygorii.stras...@linaro.org
---
drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
index 56c6055..afb8cfc 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
@@ -941,7 +941,7 @@ error:
}
#endif
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_SLEEP
static int omap_dmm_resume(struct device *dev)
{
struct tcm_area area;
@@ -965,12 +965,10 @@ static int omap_dmm_resume(struct device *dev)
return 0;
}
-
-static const struct dev_pm_ops omap_dmm_pm_ops = {
- .resume = omap_dmm_resume,
-};
#endif
+SIMPLE_DEV_PM_OPS(omap_dmm_pm_ops, NULL, omap_dmm_resume);
+
#if defined(CONFIG_OF)
static const struct of_device_id dmm_of_match[] = {
{ .compatible = ti,omap4-dmm, },
@@ -986,9 +984,7 @@ struct platform_driver omap_dmm_driver = {
.owner = THIS_MODULE,
.name = DMM_DRIVER_NAME,
.of_match_table = of_match_ptr(dmm_of_match),
-#ifdef CONFIG_PM
.pm = omap_dmm_pm_ops,
-#endif
},
};
Any comments on this?
--
regards,
-grygorii
--
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 2/2] ARM: dts: dra7: remove ti,hwmod property from pcie phy
On 03/18/2015 06:57 PM, Tony Lindgren wrote:
* Grygorii Strashko grygorii.stras...@ti.com [150318 09:37]:
As I can see Patch 1 from this series was merged in 4.0-rc4,
but this patch wasn't. As result, I can see below warning all the time
during boot now:
[0.594591] platform 4a094000.pciephy: Cannot lookup hwmod 'pcie1-phy'
OK. Is this needed as a fix for the v4.0-rc series, or can this wait
for v4.1?
I think, Yes. These two patches should go all together, because they are
interrelated.
On 02/20/2015 10:51 AM, Kishon Vijay Abraham I wrote:
Now that we don't have hwmod entry for pcie PHY remove the
ti,hwmod property from PCIE PHY's
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
---
arch/arm/boot/dts/dra7.dtsi |2 --
1 file changed, 2 deletions(-)
diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index 5827fed..18a904d 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -,7 +,6 @@
wkupclk, refclk,
div-clk, phy-div;
#phy-cells = 0;
- ti,hwmods = pcie1-phy;
};
pcie2_phy: pciephy@4a095000 {
@@ -1130,7 +1129,6 @@
wkupclk, refclk,
div-clk, phy-div;
#phy-cells = 0;
- ti,hwmods = pcie2-phy;
status = disabled;
};
};
--
regards,
-grygorii
--
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: ARM: OMPA4+: is it expected dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64)); to fail?
Hi Arnd,
On 03/09/2015 11:33 PM, Arnd Bergmann wrote:
On Thursday 05 March 2015 20:55:07 grygorii.stras...@linaro.org wrote:
Hi All,
Now I can see very interesting behavior related to
dma_coerce_mask_and_coherent()
and friends which I'd like to explain and clarify.
Below is set of questions I have (why - I explained below):
- Is expected dma_coerce_mask_and_coherent(DMA_BIT_MASK(64)) and friends to
fail on 32 bits HW?
No. dma_coerce_mask_and_coherent() is meant to ignore the actual mask. It's
usually considered a bug to use this function for that reason.
- What is expected value for max_pfn: max_phys_pfn or max_phys_pfn + 1?
- What is expected value for struct memblock_region-size: mem_range_size or
mem_range_size - 1?
- What is expected value to be returned by memblock_end_of_DRAM():
@base + @size(max_phys_addr + 1) or @base + @size - 1(max_phys_addr)?
I'm working with BeaglBoard-X15 (AM572x/DRA7xx) board and have following
code in OMAP ASOC driver
which is failed SOMETIMES during the boot with error -EIO.
=== to omap-pcm.c:
omap_pcm_new() {
...
ret = dma_coerce_mask_and_coherent(card-dev, DMA_BIT_MASK(64));
^^ failed sometimes
if (ret)
return ret;
}
The code should be fixed to use dma_set_mask_and_coherent(), which is
expected to
fail if the bus is incapable of addressing all RAM within the mask.
I'd be very appreciated for any comments/clarification on questions I've
listed at the
beginning of my e-mail - there are no patches from my side as I'd like to
understand
expected behavior of the kernel first (especially taking into account that
any
memblock changes might affect on at least half of arches).
Is the device you have actually 64-bit capable?
Is the bus it is connected to 64-bit wide?
As I mentioned before - The device was fixed by switching to use 32 bit mask
The issue with omap-pcm was simply fixed by using DMA_BIT_MASK(32), .
Does the dma-ranges property of the parent bus reflect the correct address
width?
dma-ranges is not used and all devices are created with default mask
DMA_BIT_MASK(32);
My goal was to clarify above questions (first of all), because on my HW I can
see
different values of max_pfn, max_mapnr and memblock configuration depending on
CONFIG_ARM_LPAE=n|y and when RAM is defined as: start = 0x8000 size =
0x8000.
(and also between kernels 3.14 and LKML).
Looks like such RAM configuration is a corner case, which is not always handled
as expected
(and how is it expected to be handled?).
For example:
before commit ARM: 8025/1: Get rid of meminfo
- registered RAM start = 0x8000 size = 0x8000 will be adjusted by
arm_add_memory()
and final RAM configuration will be start = 0x8000 size = 0x7000
after this commit:
- code will try to register start = 0x8000 size = 0x8000, but memblock
will
adjust it to start = 0x8000 size = 0x7fff.
--
regards,
-grygorii
--
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: ARM: OMPA4+: is it expected dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64)); to fail?
Hi Russell,
On 03/10/2015 01:05 PM, Russell King - ARM Linux wrote:
On Fri, Mar 06, 2015 at 11:47:48PM +0200, grygorii.stras...@linaro.org wrote:
On 03/05/2015 10:17 PM, Russell King - ARM Linux wrote:
On Thu, Mar 05, 2015 at 08:55:07PM +0200, grygorii.stras...@linaro.org
wrote:
The dma_coerce_mask_and_coherent() will fail in case 'Example 3' and
succeed in cases 1,2.
dma-mapping.c -- __dma_supported()
if (sizeof(mask) != sizeof(dma_addr_t) == true for all OMAP4+
mask (dma_addr_t)~0 == true for DMA_BIT_MASK(64)
dma_to_pfn(dev, ~0) max_pfn) { == true only for Example 3
Hmm, I think this may make more sense to be max_pfn - 1 here, as
that would be better suited to our intention.
The result of dma_to_pfn(dev, ~0) is the maximum PFN which we could
address via DMA, but we're comparing it with the maximum PFN in the
system plus 1 - so we need to subtract one from it.
Ok. I'll try it.
Any news on this - I think it is a real off-by-one bug which we should
fix in any case.
Sorry for delay, there was a day-off on my side.
As per my test results - with above change
dma_coerce_mask_and_coherent(DMA_BIT_MASK(64)) and friends will succeed always.
=== Test results:
Test case 1:
Input data:
- RAM: start = 0x8000 size = 0x8000
- CONFIG_ARM_LPAE=n and sizeof(phys_addr_t) = 4
a) NO changes:
memory registered within memblock as:
memory.cnt = 0x1
memory[0x0] [0x008000-0x00fffe], 0x7fff bytes flags:
0x0
max_pfn = 0xF
max_mapnr = 0x7
dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)); -- succeeded
b) with change in __dma_supported():
if (sizeof(mask) != sizeof(dma_addr_t)
mask (dma_addr_t)~0
- dma_to_pfn(dev, ~0) max_pfn) {
+ dma_to_pfn(dev, ~0) (max_pfn - 1)) {
if (warn) {
memory registered within memblock as:
memory.cnt = 0x1
memory[0x0] [0x008000-0x00fffe], 0x7fff bytes flags:
0x0
max_pfn = 0xF
max_mapnr = 0x7
dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)); -- succeeded
Test case 2:
Input data:
- RAM: start = 0x8000 size = 0x8000
- CONFIG_ARM_LPAE=y and sizeof(phys_addr_t) = 8
a) NO changes:
memory registered within memblock as:
memory.cnt = 0x1
memory[0x0] [0x008000-0x00], 0x8000 bytes flags:
0x0
max_pfn = 0x10
max_mapnr = 0x8
dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)); -- failed
[5.468470] asoc-simple-card sound@0: Coherent DMA mask 0x
is larger than dma_addr_t allows
[5.478706] asoc-simple-card sound@0: Driver did not use or check the return
value from dma_set_coherent_mask()?
[5.496620] davinci-mcasp 48468000.mcasp: ASoC: pcm constructor failed: -5
[5.503844] asoc-simple-card sound@0: ASoC: can't create pcm
davinci-mcasp.0-tlv320aic3x-hifi :-5
b) with change in __dma_supported():
if (sizeof(mask) != sizeof(dma_addr_t)
mask (dma_addr_t)~0
- dma_to_pfn(dev, ~0) max_pfn) {
+ dma_to_pfn(dev, ~0) (max_pfn - 1)) {
if (warn) {
memory registered within memblock as:
memory.cnt = 0x1
memory[0x0] [0x008000-0x00], 0x8000 bytes flags:
0x0
max_pfn = 0x10
max_mapnr = 0x8
dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)); -- succeeded
regards,
-grygorii
--
regards,
-grygorii
--
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: ARM: OMPA4+: is it expected dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64)); to fail?
Hi Russell,
On 03/05/2015 10:17 PM, Russell King - ARM Linux wrote:
On Thu, Mar 05, 2015 at 08:55:07PM +0200, grygorii.stras...@linaro.org wrote:
Now I can see very interesting behavior related to
dma_coerce_mask_and_coherent()
and friends which I'd like to explain and clarify.
Below is set of questions I have (why - I explained below):
- Is expected dma_coerce_mask_and_coherent(DMA_BIT_MASK(64)) and friends to
fail on 32 bits HW?
Not really.
- What is expected value for max_pfn: max_phys_pfn or max_phys_pfn + 1?
mm/page_owner.c:
/* Find an allocated page */
for (; pfn max_pfn; pfn++) {
drivers/base/platform.c:u32 low_totalram = ((max_pfn - 1) PAGE_SHIFT);
drivers/base/platform.c:u32 high_totalram = ((max_pfn - 1) (32 -
PAGE_SHIFT));
So, there's ample evidence that max_pfn is one more than the greatest pfn
which may be used in the system.
- What is expected value for struct memblock_region-size: mem_range_size or
mem_range_size - 1?
A size is a size - it's a number of bytes contained within the region.
If it is value 1, then there is exactly one byte in the region. If
there are 0x7fff, then there are 2G-1 bytes in the region, not 2G.
Thanks - it seems clear now.
- What is expected value to be returned by memblock_end_of_DRAM():
@base + @size(max_phys_addr + 1) or @base + @size - 1(max_phys_addr)?
The last address plus one in the system. However, there's a problem here.
On a 32-bit system, phys_addr_t may be 32-bit. If it is 32-bit, then
last address plus one could be zero, which makes no sense. Hence, it
is artificially reduced to 0xf000, thereby omitting the final page.
^ this part seems not fully true now, because for ARM32 + DT the
fdt.c-early_init_dt_add_memory_arch() is called instead of arm_add_memory()
and it works in a different way a bit.
For example, I don't see below message when reg = 0x8000 0x8000:
Truncating memory at 0x8000 to fit in 32-bit physical address space
instead memblock silently configured as
memory.cnt = 0x1
memory[0x0].base = 0x8000
memory[0x0].size = 0x7fff
Example 3 CONFIG_ARM_LPAE=y (but system really works with 32 bit address
space):
memory {
device_type = memory;
reg = 0x8000 0x8000;
};
memblock will be configured as:
memory.cnt = 0x1
memory[0x0] [0x008000-0x00], 0x8000 bytes
flags: 0x0
^^
max_pfn = 0x0010
The dma_coerce_mask_and_coherent() will fail in case 'Example 3' and succeed
in cases 1,2.
dma-mapping.c -- __dma_supported()
if (sizeof(mask) != sizeof(dma_addr_t) == true for all OMAP4+
mask (dma_addr_t)~0 == true for DMA_BIT_MASK(64)
dma_to_pfn(dev, ~0) max_pfn) { == true only for Example 3
Hmm, I think this may make more sense to be max_pfn - 1 here, as
that would be better suited to our intention.
The result of dma_to_pfn(dev, ~0) is the maximum PFN which we could
address via DMA, but we're comparing it with the maximum PFN in the
system plus 1 - so we need to subtract one from it.
Ok. I'll try it.
Please think about this and test this out; I'm not back to normal yet
(post-op) so I could very well not be thinking straight yet.
Thanks for your comments. I hope you feel better.
--
regards,
-grygorii
--
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
ARM: OMPA4+: is it expected dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64)); to fail?
Hi All,
Now I can see very interesting behavior related to
dma_coerce_mask_and_coherent()
and friends which I'd like to explain and clarify.
Below is set of questions I have (why - I explained below):
- Is expected dma_coerce_mask_and_coherent(DMA_BIT_MASK(64)) and friends to
fail on 32 bits HW?
- What is expected value for max_pfn: max_phys_pfn or max_phys_pfn + 1?
- What is expected value for struct memblock_region-size: mem_range_size or
mem_range_size - 1?
- What is expected value to be returned by memblock_end_of_DRAM():
@base + @size(max_phys_addr + 1) or @base + @size - 1(max_phys_addr)?
I'm working with BeaglBoard-X15 (AM572x/DRA7xx) board and have following code
in OMAP ASOC driver
which is failed SOMETIMES during the boot with error -EIO.
=== to omap-pcm.c:
omap_pcm_new() {
...
ret = dma_coerce_mask_and_coherent(card-dev, DMA_BIT_MASK(64));
^^ failed sometimes
if (ret)
return ret;
}
What I can see is that dma_coerce_mask_and_coherent() and etc may fail or
succeed
depending on - max_pfn value.
- max_pfn value depends on memblock configuration
max_pfn = max_high = PFN_DOWN(memblock_end_of_DRAM());
|- PFN_DOWN(memblock.memory.regions[last_idx].base +
memblock.memory.regions[last_idx].size)
- memblock configuration depends on
a) CONFIG_ARM_LPAE=y|n (my system really works with 32 bit address space)
b) RAM configuration
Example 1 CONFIG_ARM_LPAE=n:
memory {
device_type = memory;
reg = 0x8000 0x6000; /* 1536 MB */
};
memblock will be configured as:
memory.cnt = 0x1
memory[0x0] [0x008000-0x00dfff], 0x6000 bytes
flags: 0x0
^^
max_pfn = 0x000E
Example 2 CONFIG_ARM_LPAE=n:
memory {
device_type = memory;
reg = 0x8000 0x8000;
};
memblock will be configured as:
memory.cnt = 0x1
memory[0x0] [0x008000-0x00fffe], 0x7fff bytes
flags: 0x0
^^
max_pfn = 0x000F
Example 3 CONFIG_ARM_LPAE=y (but system really works with 32 bit address space):
memory {
device_type = memory;
reg = 0x8000 0x8000;
};
memblock will be configured as:
memory.cnt = 0x1
memory[0x0] [0x008000-0x00], 0x8000 bytes
flags: 0x0
^^
max_pfn = 0x0010
The dma_coerce_mask_and_coherent() will fail in case 'Example 3' and succeed in
cases 1,2.
dma-mapping.c -- __dma_supported()
if (sizeof(mask) != sizeof(dma_addr_t) == true for all OMAP4+
mask (dma_addr_t)~0 == true for DMA_BIT_MASK(64)
dma_to_pfn(dev, ~0) max_pfn) { == true only for Example 3
I've tracked down patch which changes memblock behavior to:
commit eb18f1b5bfb99b1d7d2f5d792e6ee5c9b7d89330
Author: Tejun Heo t...@kernel.org
Date: Thu Dec 8 10:22:07 2011 -0800
memblock: Make memblock functions handle overflowing range @size
This commit is pretty old :( and it doesn't takes into account LPAE mode
where phys_addr_t is 64 bit, but physically accessible addresses = 40 bit
(memblock_cap_size()).
The issue with omap-pcm was simply fixed by using DMA_BIT_MASK(32), but It
seems problem is
wider and above behavior of dma_set_maskX() and memblock confused me a bit.
I'd be very appreciated for any comments/clarification on questions I've listed
at the
beginning of my e-mail - there are no patches from my side as I'd like to
understand
expected behavior of the kernel first (especially taking into account that any
memblock changes might affect on at least half of arches).
Thanks.
Additional info:
memblock: Make memblock functions handle overflowing range @size
https://lkml.org/lkml/2011/7/26/235
[alsa-devel] [PATCH] ASoC: omap-pcm: Lower the dma coherent mask to 32bits
http://mailman.alsa-project.org/pipermail/alsa-devel/2013-December/069817.html
--
regards,
-grygorii
--
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 14/21] drm/omap: stop connector polling during suspend
Hi Tomi,
On 02/26/2015 03:20 PM, Tomi Valkeinen wrote:
When not using proper hotplug detection, DRM polls periodically the
connectors to find out if a cable is connected. This polling can happen
at any time, even very late in the suspend process.
This causes a problem with omapdrm, when the poll happens during the
suspend process after GPIOs have been disabled, leading to a crash in
gpio_get().
This patch fixes the issue by adding suspend and resume hooks to
omapdrm, in which we disable and enable, respectively, the polling.
Signed-off-by: Tomi Valkeinen tomi.valkei...@ti.com
---
drivers/gpu/drm/omapdrm/omap_drv.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
b/drivers/gpu/drm/omapdrm/omap_drv.c
index 0ebd1315fff8..d0b1aece8cc5 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -694,9 +694,28 @@ static int pdev_remove(struct platform_device *device)
return 0;
}
+static int omap_drm_suspend(struct device *dev)
+{
+ struct drm_device *drm_dev = dev_get_drvdata(dev);
+
+ drm_kms_helper_poll_disable(drm_dev);
+
+ return 0;
+}
+
+static int omap_drm_resume(struct device *dev)
+{
+ struct drm_device *drm_dev = dev_get_drvdata(dev);
+
+ drm_kms_helper_poll_enable(drm_dev);
+
+ return omap_gem_resume(dev);
+}
+
#ifdef CONFIG_PM
static const struct dev_pm_ops omapdrm_pm_ops = {
- .resume = omap_gem_resume,
+ .suspend = omap_drm_suspend,
+ .resume = omap_drm_resume,
};
#endif
Could I ask you to update this patch as below, pls?
Regards,
-grygorii
===
drm/omap: add hibernation callbacks
Setting a dev_pm_ops suspend/resume pair but not a set of
hibernation functions means those pm functions will not be
called upon hibernation.
Fix this by using SET_SYSTEM_SLEEP_PM_OPS, which appropriately
assigns the suspend and hibernation handlers and move
omap_drm_suspend/omap_drm_resume under CONFIG_PM_SLEEP
to avoid build warnings.
Signed-off-by: Grygorii Strashko grygorii.stras...@linaro.org
---
drivers/gpu/drm/omapdrm/omap_drv.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c
b/drivers/gpu/drm/omapdrm/omap_drv.c
index 7cb1c8f..4a544e4 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -715,6 +715,7 @@ static int pdev_remove(struct platform_device *device)
return 0;
}
+#ifdef CONFIG_PM_SLEEP
static int omap_drm_suspend(struct device *dev)
{
struct drm_device *drm_dev = dev_get_drvdata(dev);
@@ -732,21 +733,15 @@ static int omap_drm_resume(struct device *dev)
return omap_gem_resume(dev);
}
-
-#ifdef CONFIG_PM
-static const struct dev_pm_ops omapdrm_pm_ops = {
- .suspend = omap_drm_suspend,
- .resume = omap_drm_resume,
-};
#endif
+static SIMPLE_DEV_PM_OPS(omapdrm_pm_ops, omap_drm_suspend, omap_drm_resume);
+
static struct platform_driver pdev = {
.driver = {
.name = DRIVER_NAME,
.owner = THIS_MODULE,
-#ifdef CONFIG_PM
.pm = omapdrm_pm_ops,
-#endif
},
.probe = pdev_probe,
.remove = pdev_remove,
-- 1.9.1
--
regards,
-grygorii
--
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] thermal: ti-soc-thermal: bandgap: Fix build warning if !CONFIG_PM_SLEEP
Hi Rui,
On 02/09/2015 05:01 PM, Nishanth Menon wrote:
On 16:55-20150206, grygorii.stras...@linaro.org wrote:
From: Grygorii Strashko grygorii.stras...@linaro.org
Fix following build warning if CONFIG_PM_SLEEP is not set:
drivers/thermal/ti-soc-thermal/ti-bandgap.c:1478:12: warning:
'ti_bandgap_suspend' defined but not used [-Wunused-function]
static int ti_bandgap_suspend(struct device *dev)
^
drivers/thermal/ti-soc-thermal/ti-bandgap.c:1492:12: warning:
'ti_bandgap_resume' defined but not used [-Wunused-function]
static int ti_bandgap_resume(struct device *dev)
^
Signed-off-by: Grygorii Strashko grygorii.stras...@linaro.org
---
drivers/thermal/ti-soc-thermal/ti-bandgap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.c
b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
index 74c0e34..5d46660 100644
--- a/drivers/thermal/ti-soc-thermal/ti-bandgap.c
+++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
@@ -1402,7 +1402,7 @@ int ti_bandgap_remove(struct platform_device *pdev)
return 0;
}
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_SLEEP
static int ti_bandgap_save_ctxt(struct ti_bandgap *bgp)
{
int i;
--
1.9.1
Suggest aligning with Paul as well:
https://patchwork.kernel.org/patch/5795391/
^Paul Walmsley wrote: Oh, nothing to be confused by, I just missed the
earlier patch. I withdraw mine.
Otherwise:
Acked-by: Nishanth Menon n...@ti.com
It looks like this patch is missed in 4.0-rc1, so could it be merged
during rc cycle or you'd like me to resend it?
(I've rechecked - it can be applied on top of Linux 4.0-rc1 without any issues)
--
regards,
-grygorii
--
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] ARM: dra7xx: hwmod: drop .modulemode from pcie1/2 hwmods
On 02/12/2015 11:08 PM, Paul Walmsley wrote: Thanks guys. On Thu, 12 Feb 2015, grygorii.stras...@linaro.org wrote: Looks good for me and seems working. Grygorii, can I add your Acked-by? - Paul There is my Signed-off-by :) -- regards, -grygorii -- 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] ARM: dra7xx: hwmod: drop .modulemode from pcie1/2 hwmods
On 02/12/2015 02:43 PM, Kishon Vijay Abraham I wrote:
On Monday 09 February 2015 08:22 PM, grygorii.stras...@linaro.org wrote:
On 02/09/2015 09:24 PM, Kishon Vijay Abraham I wrote:
Hi,
On Monday 09 February 2015 03:58 PM, grygorii.stras...@linaro.org wrote:
Hi Kishon,
On 02/09/2015 04:50 PM, Kishon Vijay Abraham I wrote:
On Tuesday 03 February 2015 09:21 PM, grygorii.stras...@linaro.org
wrote:
From: Grygorii Strashko grygorii.stras...@linaro.org
Now DRA7xx pcie1/2 hwmods define PRCM configuration as following:
.clkctrl_offs = DRA7XX_CM_PCIE_CLKSTCTRL_OFFSET,
.rstctrl_offs = DRA7XX_RM_L3INIT_RSTCTRL_OFFSET,
.modulemode = MODULEMODE_SWCTRL,
which is completely wrong because DRA7XX_CM_PCIE_CLKSTCTRL_OFFSET
is clockdomain ctrl register and NOT module ctrl register.
And they have diffrent allowed values for bits[0,1]:
CLKTRCTRL MODULEMODE
0x0: NO_SLEEP 0x0: Module is disabled by SW.
0x1: SW_SLEEP 0x1: Module is managed automatically by HW
0x2: SW_WKUP 0x2: Module is explicitly enabled.
0x3: HW_AUTO 0x3: Reserved
As result, following message can be seen during suspend:
omap_hwmod: pcie1: _wait_target_disable failed
Fix it by removing .modulemode from pcie1/2 hwmods and, in that
way, prevent clockdomain ctrl register writing from HWMOD core.
Looks correct except for one change.
Acked-by: Kishon Vijay Abraham I kis...@ti.com
Signed-off-by: Grygorii Strashko grygorii.stras...@linaro.org
---
More over, it looks like pcie1/2 hwmods are fake and have to be
dropped at all.
The real HWMODs are PCIESS1/2.
Not sure I get this. You mean dra7xx_pcie1_hwmod should be
replaced with
dra7xx_pciess1_hwmod? Or you mean an entire new hwmod is missing?
Please note we still have to enable the clock domain and main clock.
We've also
purposefully omitted sysconfig from hwmod data since pcie reset
(RM_PCIESS_RSTCTRL) should be done before accessing the syconfig
register and
the infrastructure for that is currently not present.
What I'm trying to say is that now PM control data mixed between
pcieX and pcieX-phy hwmods.
After this patch pcieX hwmods will actually do nothing (I assume
that pciex-phy will be
enabled before pcieX), and probably can be removed if pcie_clkdm
could be attached to pcieX-phy hwmod
instead.
More over, now, pcie_clkdm is connected to pcieX hwmod while
MODULEMODE register is controlled
by pciex-phy hwmod, so when pciess is going to be enabled the
l3init_clkdm will be waken-up by
hwmode core and not pcie_clkdm - as I can remember this is not good
(we should alway wake-up clockdomain
and keep it in SWSUP mode while changing MODULEMODE and SYSC
registers).
static struct omap_hwmod dra7xx_pcie1_hwmod = {
.name= pcie1,
.class= dra7xx_pcie_hwmod_class,
.clkdm_name= pcie_clkdm,
.main_clk= l4_root_clk_div,
static struct omap_hwmod dra7xx_pcie1_phy_hwmod = {
.name= pcie1-phy,
.class= dra7xx_pcie_phy_hwmod_class,
.clkdm_name= l3init_clkdm,
.main_clk= l4_root_clk_div,
So, in my opinion, some rework may be needed here.
Am I right?
you are right. We should have a single hwmod like dra7xx_pciess1_hwmod
whose
clkdm should be pcie_clkdm and whose clkctrl_offs should be
DRA7XX_CM_L3INIT_PCIESS1_CLKCTRL_OFFSET (for controlling MODULEMODE).
PCIE PHY
shouldn't have a hwmod entry at all.
Thanks
Kishon
Could this patch be applied any way? It fixes real issue for me.
A proper fix should look something like below IMO
Looks good for me and seems working.
From 1c177d37ac46885a4dc17bacec33071ac23c56da Mon Sep 17 00:00:00 2001
From: Kishon Vijay Abraham I kis...@ti.com
Date: Thu, 12 Feb 2015 11:55:08 +0530
Subject: [RFC PATCH] ARM: DRA7: hwmod_data: Fix hwmod data for pcie
Fixed hwmod data for pcie by having the correct module mode offset.
Previously this module mode offset was part of pcie PHY which was wrong.
Now this module mode offset was moved to pcie hwmod and removed the
hwmod data
for pcie phy. While at that renamed pcie_hwmod to pciess_hwmod in order
to match with the name given in TRM.
This helps to get rid of the following warning
omap_hwmod: pcie1: _wait_target_disable failed
[grygorii.stras...@linaro.org: Found the issue that actually caused
omap_hwmod: pcie1: _wait_target_disable failed]
Signed-off-by: Grygorii Strashko grygorii.stras...@linaro.org
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
---
this patch was created on 3.14 kernel after applying reset framework
patches
required for testing PCIe. I can port this to mainline if this patch is
fine.
arch/arm/boot/dts/dra7.dtsi |2 -
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 107
+++--
2 files changed, 26 insertions(+), 83 deletions(-)
diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index a4b1337..d7a1ff9 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts
Re: [PATCH] ARM: dra7xx: hwmod: drop .modulemode from pcie1/2 hwmods
Hi Kishon,
On 02/09/2015 04:50 PM, Kishon Vijay Abraham I wrote:
On Tuesday 03 February 2015 09:21 PM, grygorii.stras...@linaro.org wrote:
From: Grygorii Strashko grygorii.stras...@linaro.org
Now DRA7xx pcie1/2 hwmods define PRCM configuration as following:
.clkctrl_offs = DRA7XX_CM_PCIE_CLKSTCTRL_OFFSET,
.rstctrl_offs = DRA7XX_RM_L3INIT_RSTCTRL_OFFSET,
.modulemode = MODULEMODE_SWCTRL,
which is completely wrong because DRA7XX_CM_PCIE_CLKSTCTRL_OFFSET
is clockdomain ctrl register and NOT module ctrl register.
And they have diffrent allowed values for bits[0,1]:
CLKTRCTRL MODULEMODE
0x0: NO_SLEEP 0x0: Module is disabled by SW.
0x1: SW_SLEEP 0x1: Module is managed automatically by HW
0x2: SW_WKUP 0x2: Module is explicitly enabled.
0x3: HW_AUTO 0x3: Reserved
As result, following message can be seen during suspend:
omap_hwmod: pcie1: _wait_target_disable failed
Fix it by removing .modulemode from pcie1/2 hwmods and, in that
way, prevent clockdomain ctrl register writing from HWMOD core.
Looks correct except for one change.
Acked-by: Kishon Vijay Abraham I kis...@ti.com
Signed-off-by: Grygorii Strashko grygorii.stras...@linaro.org
---
More over, it looks like pcie1/2 hwmods are fake and have to be dropped at
all.
The real HWMODs are PCIESS1/2.
Not sure I get this. You mean dra7xx_pcie1_hwmod should be replaced with
dra7xx_pciess1_hwmod? Or you mean an entire new hwmod is missing?
Please note we still have to enable the clock domain and main clock. We've
also
purposefully omitted sysconfig from hwmod data since pcie reset
(RM_PCIESS_RSTCTRL) should be done before accessing the syconfig register and
the infrastructure for that is currently not present.
What I'm trying to say is that now PM control data mixed between pcieX and
pcieX-phy hwmods.
After this patch pcieX hwmods will actually do nothing (I assume that
pciex-phy will be
enabled before pcieX), and probably can be removed if pcie_clkdm could be
attached to pcieX-phy hwmod
instead.
More over, now, pcie_clkdm is connected to pcieX hwmod while MODULEMODE
register is controlled
by pciex-phy hwmod, so when pciess is going to be enabled the l3init_clkdm
will be waken-up by
hwmode core and not pcie_clkdm - as I can remember this is not good (we
should alway wake-up clockdomain
and keep it in SWSUP mode while changing MODULEMODE and SYSC registers).
static struct omap_hwmod dra7xx_pcie1_hwmod = {
.name = pcie1,
.class = dra7xx_pcie_hwmod_class,
.clkdm_name = pcie_clkdm,
.main_clk = l4_root_clk_div,
static struct omap_hwmod dra7xx_pcie1_phy_hwmod = {
.name = pcie1-phy,
.class = dra7xx_pcie_phy_hwmod_class,
.clkdm_name = l3init_clkdm,
.main_clk = l4_root_clk_div,
So, in my opinion, some rework may be needed here.
Am I right?
Unfortunatelly, not all information on PCIE is public, so
I could be wrong here.
---
arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
index ffd6604..a428b2d 100644
--- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
@@ -1478,7 +1478,6 @@ static struct omap_hwmod dra7xx_pcie1_hwmod = {
.prcm = {
.omap4 = {
.clkctrl_offs = DRA7XX_CM_PCIE_CLKSTCTRL_OFFSET,
-.modulemode = MODULEMODE_SWCTRL,
I think the entire .prcm can be removed here.
not sure. I've tried it and Kernel boot failed (on 3.14)
},
},
};
@@ -1492,7 +1491,6 @@ static struct omap_hwmod dra7xx_pcie2_hwmod = {
.prcm = {
.omap4 = {
.clkctrl_offs = DRA7XX_CM_PCIE_CLKSTCTRL_OFFSET,
-.modulemode = MODULEMODE_SWCTRL,
},
},
};
Thanks
Kishon
--
regards,
-grygorii
--
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] ARM: dra7xx: hwmod: drop .modulemode from pcie1/2 hwmods
On 02/09/2015 09:24 PM, Kishon Vijay Abraham I wrote:
Hi,
On Monday 09 February 2015 03:58 PM, grygorii.stras...@linaro.org wrote:
Hi Kishon,
On 02/09/2015 04:50 PM, Kishon Vijay Abraham I wrote:
On Tuesday 03 February 2015 09:21 PM, grygorii.stras...@linaro.org wrote:
From: Grygorii Strashko grygorii.stras...@linaro.org
Now DRA7xx pcie1/2 hwmods define PRCM configuration as following:
.clkctrl_offs = DRA7XX_CM_PCIE_CLKSTCTRL_OFFSET,
.rstctrl_offs = DRA7XX_RM_L3INIT_RSTCTRL_OFFSET,
.modulemode = MODULEMODE_SWCTRL,
which is completely wrong because DRA7XX_CM_PCIE_CLKSTCTRL_OFFSET
is clockdomain ctrl register and NOT module ctrl register.
And they have diffrent allowed values for bits[0,1]:
CLKTRCTRL MODULEMODE
0x0: NO_SLEEP 0x0: Module is disabled by SW.
0x1: SW_SLEEP 0x1: Module is managed automatically by HW
0x2: SW_WKUP 0x2: Module is explicitly enabled.
0x3: HW_AUTO 0x3: Reserved
As result, following message can be seen during suspend:
omap_hwmod: pcie1: _wait_target_disable failed
Fix it by removing .modulemode from pcie1/2 hwmods and, in that
way, prevent clockdomain ctrl register writing from HWMOD core.
Looks correct except for one change.
Acked-by: Kishon Vijay Abraham I kis...@ti.com
Signed-off-by: Grygorii Strashko grygorii.stras...@linaro.org
---
More over, it looks like pcie1/2 hwmods are fake and have to be dropped at all.
The real HWMODs are PCIESS1/2.
Not sure I get this. You mean dra7xx_pcie1_hwmod should be replaced with
dra7xx_pciess1_hwmod? Or you mean an entire new hwmod is missing?
Please note we still have to enable the clock domain and main clock. We've also
purposefully omitted sysconfig from hwmod data since pcie reset
(RM_PCIESS_RSTCTRL) should be done before accessing the syconfig register and
the infrastructure for that is currently not present.
What I'm trying to say is that now PM control data mixed between pcieX and
pcieX-phy hwmods.
After this patch pcieX hwmods will actually do nothing (I assume that
pciex-phy will be
enabled before pcieX), and probably can be removed if pcie_clkdm could be attached to
pcieX-phy hwmod
instead.
More over, now, pcie_clkdm is connected to pcieX hwmod while MODULEMODE
register is controlled
by pciex-phy hwmod, so when pciess is going to be enabled the l3init_clkdm
will be waken-up by
hwmode core and not pcie_clkdm - as I can remember this is not good (we
should alway wake-up clockdomain
and keep it in SWSUP mode while changing MODULEMODE and SYSC registers).
static struct omap_hwmod dra7xx_pcie1_hwmod = {
.name = pcie1,
.class = dra7xx_pcie_hwmod_class,
.clkdm_name = pcie_clkdm,
.main_clk = l4_root_clk_div,
static struct omap_hwmod dra7xx_pcie1_phy_hwmod = {
.name = pcie1-phy,
.class = dra7xx_pcie_phy_hwmod_class,
.clkdm_name = l3init_clkdm,
.main_clk = l4_root_clk_div,
So, in my opinion, some rework may be needed here.
Am I right?
you are right. We should have a single hwmod like dra7xx_pciess1_hwmod whose
clkdm should be pcie_clkdm and whose clkctrl_offs should be
DRA7XX_CM_L3INIT_PCIESS1_CLKCTRL_OFFSET (for controlling MODULEMODE). PCIE PHY
shouldn't have a hwmod entry at all.
Thanks
Kishon
Could this patch be applied any way? It fixes real issue for me.
--
regards,
-grygorii
--
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] arm: omap: reduce zImage size on omap2plus_defconfig
On 12/25/2014 12:13 PM, Igor Grinberg wrote: Hi Tony, On 12/24/14 21:04, Tony Lindgren wrote: * Felipe Balbi ba...@ti.com [141224 07:52]: Hi, On Wed, Dec 24, 2014 at 01:53:46PM +0200, Igor Grinberg wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 12/23/14 18:19, Felipe Balbi wrote: On Tue, Dec 23, 2014 at 09:30:45AM +0200, Igor Grinberg wrote: Hi Felipe, On 12/22/14 20:05, Felipe Balbi wrote: [...] CONFIG_SCSI_SCAN_ASYNC=y -CONFIG_ATA=y -CONFIG_SATA_AHCI_PLATFORM=y -CONFIG_MD=y +CONFIG_ATA=m +CONFIG_SATA_AHCI_PLATFORM=m Isn't this needed for the rootfs on SATA devices? there's no known boards with rootfs on SATA. Until then, we can reduce the size. What makes you say so? The decision for rootfs on SATA is taken dynamically. OMAP5 boards (specifically cm-t54) can have rootfs on SATA... I'll leave the decision to Tony. Even though they _can_, they really don't and IIRC, OMAP5's SATA has so many silicon errata that it'd be annoying to find that special device which works (e.g it can't negotiate lower speeds with SATA III devices and it won't support SATA I). Yet, it is not that buggy and at least until now, I di not get any reports about badly working SATA from customers... As of today, we don't know of anybody really shipping anything with rootfs on SATA and distros would rather ship initiramfs than a giant zImage anyway. So, you just continue to ignore what I'm saying... even after I point to a device... Is it SATA that makes it so giant? Because I find it worth having in SATA than spare some more k's... SATA code occupies ~179Kbytes - checked on 3.19-rc1 CONFIG_ATA=y CONFIG_SATA_AHCI_PLATFORM=y CONFIG_MD=y (seems it will give +-0K) Tony, your call May be it will be good thing to split this patch. That way more information will be stored in commit log about which set of options gives us what benefits. And also, It will allow to continue with agreed changes. ? I think we should move omap2plus_defconfig to be mostly modular and usable for distros as a base. Most distros prefer to build almost everything as loadable modules. And my preference is that we should only keep the minimum rootfs for devices and serial support as built-in and rely on initramfs for most drivers. And slowly move also the remaining built-in drivers to be loadable modules. The reasons for having drivers as loadable modules are many. It allows distros to use the same kernel for all the devices without bloating the kernel. It makes developing drivers easier as just the module needs to be reloaded. And loadable modules protect us from cross-framework spaghetti calls in the kernel as the interfaces are clearly defined. Are there people really using SATA as rootfs right now on omaps? Yes. That is exactly my point. From my side I'd like to note that I know about few ongoing projects on DRA7x EVM where SATA is used as rootfs - now It is the fast possible way to start Android. -- regards, -grygorii -- 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
